r75462 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r75461‎ | r75462 | r75463 >
Date:19:45, 26 October 2010
Author:reedy
Status:deferred (Comments)
Tags:
Comment:
Split FeedSitemapItem and SitemapFeed to own file

Also add classes to $wgAutoloadClasses
Modified paths:
  • /trunk/extensions/GoogleNewsSitemap/FeedSitemapItem.php (added) (history)
  • /trunk/extensions/GoogleNewsSitemap/GoogleNewsSitemap.php (modified) (history)
  • /trunk/extensions/GoogleNewsSitemap/GoogleNewsSitemap_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/GoogleNewsSitemap/GoogleNewsSitemap_body.php
——@@ -500,144 +500,3 @@
501501 return $str;
502502 }
503503 }
504 -
505 -/**
506 - * FeedSitemapItem Class
507 - **
508 - * Base class for basic SiteMap support, for building url containers.
509 - **/
510 -class FeedSitemapItem {
511 - /**
512 - * Var string
513 - **/
514 - var $url, $pubDate, $keywords, $lastMod, $priority;
515 -
516 - function __construct( $url, $pubDate, $keywords = '', $lastMod = '', $priority = '' ) {
517 - $this->url = $url;
518 - $this->pubDate = $pubDate;
519 - $this->keywords = $keywords;
520 - $this->lastMod = $lastMod;
521 - $this->priority = $priority;
522 - }
523 -
524 - public function xmlEncode( $string ) {
525 - $string = str_replace( "\r\n", "\n", $string );
526 - $string = preg_replace( '/[\x00-\x08\x0b\x0c\x0e-\x1f]/', '', $string );
527 - return htmlspecialchars( $string );
528 - }
529 -
530 - public function getUrl() {
531 - return $this->url;
532 - }
533 -
534 - public function getPriority() {
535 - return $this->priority;
536 - }
537 -
538 - public function getLastMod() {
539 - return $this->lastMod;
540 - }
541 -
542 - public function getKeywords () {
543 - return $this->xmlEncode( $this->keywords );
544 - }
545 -
546 - public function getPubDate() {
547 - return $this->pubDate;
548 - }
549 -
550 - function formatTime( $ts ) {
551 - // need to use RFC 822 time format at least for rss2.0
552 - return gmdate( 'Y-m-d\TH:i:s', wfTimestamp( TS_UNIX, $ts ) );
553 - }
554 -
555 - /**
556 - * Setup and send HTTP headers. Don't send any content;
557 - * content might end up being cached and re-sent with
558 - * these same headers later.
559 - *
560 - * This should be called from the outHeader() method,
561 - * but can also be called separately.
562 - *
563 - * @public
564 - **/
565 - function httpHeaders() {
566 - global $wgOut;
567 - # We take over from $wgOut, excepting its cache header info
568 - $wgOut->disable();
569 - $mimetype = $this->contentType();
570 - header( "Content-type: $mimetype; charset=UTF-8" );
571 - $wgOut->sendCacheControl();
572 -
573 - }
574 -
575 - function outXmlHeader() {
576 - $this->httpHeaders();
577 - echo '<?xml version="1.0" encoding="UTF-8"?>' . "\n";
578 - }
579 -
580 - /**
581 - * Return an internet media type to be sent in the headers.
582 - *
583 - * @return string
584 - * @private
585 - **/
586 - function contentType() {
587 - global $wgRequest;
588 - $ctype = $wgRequest->getVal( 'ctype', 'application/xml' );
589 - $allowedctypes = array( 'application/xml', 'text/xml', 'application/rss+xml', 'application/atom+xml' );
590 - return ( in_array( $ctype, $allowedctypes ) ? $ctype : 'application/xml' );
591 - }
592 -}
593 -
594 -class SitemapFeed extends FeedSitemapItem {
595 - /**
596 - * Output feed headers
597 - **/
598 - function outHeader() {
599 - $this->outXmlHeader();
600 - ?>
601 -<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"
602 - xmlns:news="http://www.google.com/schemas/sitemap-news/0.9">
603 - <?php
604 - }
605 - /**
606 - * Output a SiteMap 0.9 item
607 - * @param FeedSitemapItem item to be output
608 - **/
609 - function outItem( $item ) {
610 - ?>
611 -<url>
612 -<loc>
613 - <?php print $item->getUrl() ?>
614 -</loc>
615 -<news:news>
616 - <news:publication_date>
617 - <?php print $item->getPubDate() ?>
618 - </news:publication_date>
619 - <?php if ( $item->getKeywords() ) {
620 - echo '<news:keywords>' . $item->getKeywords() . "</news:keywords>\n";
621 - }
622 - ?>
623 -</news:news>
624 - <?php if ( $item->getLastMod() ) { ?>
625 -<lastmod>
626 - <?php print $item->getLastMod(); ?>
627 -</lastmod>
628 - <?php } ?>
629 - <?php if ( $item->getPriority() ) { ?>
630 -<priority>
631 - <? print $item->getPriority(); ?>
632 -</priority>
633 - <?php } ?>
634 -</url>
635 - <?php
636 - }
637 -
638 - /**
639 - * Output SiteMap 0.9 footer
640 - **/
641 - function outFooter() {
642 - echo '</urlset>';
643 - }
644 -}
Index: trunk/extensions/GoogleNewsSitemap/GoogleNewsSitemap.php
——@@ -65,4 +65,6 @@
6666 $wgExtensionMessagesFiles['GoogleNewsSitemap'] = $dir . 'GoogleNewsSitemap.i18n.php';
6767 $wgExtensionAliasesFiles['GoogleNewsSitemap'] = $dir . 'GoogleNewsSitemap.alias.php';
6868 $wgAutoloadClasses['GoogleNewsSitemap'] = $dir . 'GoogleNewsSitemap_body.php';
 69+$wgAutoloadClasses['FeedSitemapItem'] = $dir . 'FeedSitemapItem.php';
 70+$wgAutoloadClasses['SitemapFeed'] = $dir . 'FeedSitemapItem.php';
6971 $wgSpecialPages['GoogleNewsSitemap'] = 'GoogleNewsSitemap';
Index: trunk/extensions/GoogleNewsSitemap/FeedSitemapItem.php
——@@ -0,0 +1,143 @@
 2+<?php
 3+if ( !defined( 'MEDIAWIKI' ) ) die();
 4+
 5+/**
 6+ * FeedSitemapItem Class
 7+ **
 8+ * Base class for basic SiteMap support, for building url containers.
 9+ **/
 10+class FeedSitemapItem {
 11+ /**
 12+ * Var string
 13+ **/
 14+ var $url, $pubDate, $keywords, $lastMod, $priority;
 15+
 16+ function __construct( $url, $pubDate, $keywords = '', $lastMod = '', $priority = '' ) {
 17+ $this->url = $url;
 18+ $this->pubDate = $pubDate;
 19+ $this->keywords = $keywords;
 20+ $this->lastMod = $lastMod;
 21+ $this->priority = $priority;
 22+ }
 23+
 24+ public function xmlEncode( $string ) {
 25+ $string = str_replace( "\r\n", "\n", $string );
 26+ $string = preg_replace( '/[\x00-\x08\x0b\x0c\x0e-\x1f]/', '', $string );
 27+ return htmlspecialchars( $string );
 28+ }
 29+
 30+ public function getUrl() {
 31+ return $this->url;
 32+ }
 33+
 34+ public function getPriority() {
 35+ return $this->priority;
 36+ }
 37+
 38+ public function getLastMod() {
 39+ return $this->lastMod;
 40+ }
 41+
 42+ public function getKeywords () {
 43+ return $this->xmlEncode( $this->keywords );
 44+ }
 45+
 46+ public function getPubDate() {
 47+ return $this->pubDate;
 48+ }
 49+
 50+ function formatTime( $ts ) {
 51+ // need to use RFC 822 time format at least for rss2.0
 52+ return gmdate( 'Y-m-d\TH:i:s', wfTimestamp( TS_UNIX, $ts ) );
 53+ }
 54+
 55+ /**
 56+ * Setup and send HTTP headers. Don't send any content;
 57+ * content might end up being cached and re-sent with
 58+ * these same headers later.
 59+ *
 60+ * This should be called from the outHeader() method,
 61+ * but can also be called separately.
 62+ *
 63+ * @public
 64+ **/
 65+ function httpHeaders() {
 66+ global $wgOut;
 67+ # We take over from $wgOut, excepting its cache header info
 68+ $wgOut->disable();
 69+ $mimetype = $this->contentType();
 70+ header( "Content-type: $mimetype; charset=UTF-8" );
 71+ $wgOut->sendCacheControl();
 72+
 73+ }
 74+
 75+ function outXmlHeader() {
 76+ $this->httpHeaders();
 77+ echo '<?xml version="1.0" encoding="UTF-8"?>' . "\n";
 78+ }
 79+
 80+ /**
 81+ * Return an internet media type to be sent in the headers.
 82+ *
 83+ * @return string
 84+ * @private
 85+ **/
 86+ function contentType() {
 87+ global $wgRequest;
 88+ $ctype = $wgRequest->getVal( 'ctype', 'application/xml' );
 89+ $allowedctypes = array( 'application/xml', 'text/xml', 'application/rss+xml', 'application/atom+xml' );
 90+ return ( in_array( $ctype, $allowedctypes ) ? $ctype : 'application/xml' );
 91+ }
 92+}
 93+
 94+class SitemapFeed extends FeedSitemapItem {
 95+ /**
 96+ * Output feed headers
 97+ **/
 98+ function outHeader() {
 99+ $this->outXmlHeader();
 100+ ?>
 101+<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"
 102+ xmlns:news="http://www.google.com/schemas/sitemap-news/0.9">
 103+ <?php
 104+ }
 105+ /**
 106+ * Output a SiteMap 0.9 item
 107+ * @param FeedSitemapItem item to be output
 108+ **/
 109+ function outItem( $item ) {
 110+ ?>
 111+<url>
 112+<loc>
 113+ <?php print $item->getUrl() ?>
 114+</loc>
 115+<news:news>
 116+ <news:publication_date>
 117+ <?php print $item->getPubDate() ?>
 118+ </news:publication_date>
 119+ <?php if ( $item->getKeywords() ) {
 120+ echo '<news:keywords>' . $item->getKeywords() . "</news:keywords>\n";
 121+ }
 122+ ?>
 123+</news:news>
 124+ <?php if ( $item->getLastMod() ) { ?>
 125+<lastmod>
 126+ <?php print $item->getLastMod(); ?>
 127+</lastmod>
 128+ <?php } ?>
 129+ <?php if ( $item->getPriority() ) { ?>
 130+<priority>
 131+ <? print $item->getPriority(); ?>
 132+</priority>
 133+ <?php } ?>
 134+</url>
 135+ <?php
 136+ }
 137+
 138+ /**
 139+ * Output SiteMap 0.9 footer
 140+ **/
 141+ function outFooter() {
 142+ echo '</urlset>';
 143+ }
 144+}
Property changes on: trunk/extensions/GoogleNewsSitemap/FeedSitemapItem.php
___________________________________________________________________
Added: svn:eol-style
1145 + native

Comments

#Comment by MarkAHershberger (talk | contribs)   21:10, 23 November 2010

Since this commit involves a reorg of the extension, (at least part of) the FIXME and some of the comments from r76544 rightfully apply here instead of there. I'm copying those that I think are pertinent, things that I think you should have addressed before committing this code -- or at that should have at least been acknowledged as future areas where work was needed.

  • echo and print?!?! Don't do it. That and direct output (e.g. closing ?>) should not be done. Note that this is (part of) what is causing using the template format not to work.
  • Don't try to create XML yourself, it doesn't work. Use XMLWriter or DOM to create XML. That way you don't have do xmlEncode().
#Comment by Reedy (talk | contribs)   08:33, 29 November 2010

Marking this one new as this information (and a lot more) is on r76544

Status & tagging log