From 7558d32da6d8229b9c8664b881fb3a008ca42704 Mon Sep 17 00:00:00 2001
From: Will Rossiter <will.rossiter@gmail.com>
Date: Tue, 31 Jul 2012 20:38:12 +1200
Subject: [PATCH] FIX: use standard template rendering process for RSS feeds

This changes the behaviour of output to browser to use the standard SilverStripe rendering process rather than an echo statement to enable easier testability.
---
 api/RSSFeed.php           | 32 ++++++++++++--------------------
 tests/api/RSSFeedTest.php | 19 +++++++++++--------
 2 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/api/RSSFeed.php b/api/RSSFeed.php
index d1da508b4..b0263d86e 100644
--- a/api/RSSFeed.php
+++ b/api/RSSFeed.php
@@ -144,6 +144,7 @@ class RSSFeed extends ViewableData {
 	 */
 	function Entries() {
 		$output = new ArrayList();
+
 		if(isset($this->entries)) {
 			foreach($this->entries as $entry) {
 				$output->push(new RSSFeed_Entry($entry, $this->titleField, $this->descriptionField, $this->authorField));
@@ -183,7 +184,10 @@ class RSSFeed extends ViewableData {
 	/**
 	 * Output the feed to the browser
 	 */
-	function outputToBrowser() {
+	public function outputToBrowser() {
+		$prevState = SSViewer::get_source_file_comments();
+		SSViewer::set_source_file_comments(false);
+
 		if(is_int($this->lastModified)) {
 			HTTP::register_modification_timestamp($this->lastModified);
 			header('Last-Modified: ' . gmdate("D, d M Y H:i:s", $this->lastModified) . ' GMT');
@@ -192,26 +196,14 @@ class RSSFeed extends ViewableData {
 			HTTP::register_etag($this->etag);
 		}
 
-		$body = $this->feedContent();
-		HTTP::add_cache_headers();
-		header("Content-type: text/xml");
-		echo $body;
-	}
-	
-	/**
-	 * Return the content of the RSS feed.
-	 * 
-	 * Also temporarily disabled source file comments, and restores
-	 * to previous state once content has been rendered.
-	 * 
-	 * @return string
-	 */
-	function feedContent() {
-		$prevState = SSViewer::get_source_file_comments();
-		SSViewer::set_source_file_comments(false);
-		$content = str_replace('&nbsp;', '&#160;', $this->renderWith($this->getTemplate()));
+		if(!headers_sent()) {
+			HTTP::add_cache_headers();
+			header("Content-type: text/xml");
+		}
+
 		SSViewer::set_source_file_comments($prevState);
-		return $content;
+
+		return $this->renderWith($this->getTemplate());
 	}
 
 	/**
diff --git a/tests/api/RSSFeedTest.php b/tests/api/RSSFeedTest.php
index fc84b8d41..97261d549 100644
--- a/tests/api/RSSFeedTest.php
+++ b/tests/api/RSSFeedTest.php
@@ -14,7 +14,7 @@ class RSSFeedTest extends SapphireTest {
 		$list->push(new RSSFeedTest_ItemC());
 
 		$rssFeed = new RSSFeed($list, "http://www.example.com", "Test RSS Feed", "Test RSS Feed Description");
-		$content = $rssFeed->feedContent();
+		$content = $rssFeed->outputToBrowser();
 
 		//Debug::message($content);
 		$this->assertContains('<link>http://www.example.org/item-a/</link>', $content);
@@ -32,7 +32,7 @@ class RSSFeedTest extends SapphireTest {
 
 		// Feed #2 - put Content() into <title> and AltContent() into <description>
 		$rssFeed = new RSSFeed($list, "http://www.example.com", "Test RSS Feed", "Test RSS Feed Description", "Content", "AltContent");
-		$content = $rssFeed->feedContent();
+		$content = $rssFeed->outputToBrowser();
 
 		$this->assertContains('<title>ItemA Content</title>', $content);
 		$this->assertContains('<title>ItemB Content</title>', $content);
@@ -47,13 +47,11 @@ class RSSFeedTest extends SapphireTest {
 		$rssFeed = new RSSFeed(new ArrayList(), "", "", "");
 		$rssFeed->setTemplate('RSSFeedTest');
 
-		$content = $rssFeed->feedContent();
-
+		$content = $rssFeed->outputToBrowser();
 		$this->assertContains('<title>Test Custom Template</title>', $content);
 
 		$rssFeed->setTemplate('RSSFeed');
-		$content = $rssFeed->feedContent();
-		
+		$content = $rssFeed->outputToBrowser();
 		$this->assertNotContains('<title>Test Custom Template</title>', $content);
 	}
 
@@ -69,7 +67,6 @@ class RSSFeedTest extends SapphireTest {
 		Director::setBaseURL(null);
 		$_SERVER['HTTP_HOST'] = self::$original_host;
 	}
-
 }
 
 class RSSFeedTest_ItemA extends ViewableData {
@@ -83,9 +80,11 @@ class RSSFeedTest_ItemA extends ViewableData {
 	function getTitle() {
 		return "ItemA";
 	}
+
 	function getContent() {
 		return "ItemA Content";
 	}
+
 	function getAltContent() {
 		return "ItemA AltContent";
 	}
@@ -101,12 +100,15 @@ class RSSFeedTest_ItemB extends ViewableData {
 	function Title() {
 		return "ItemB";
 	}
+
 	function AbsoluteLink() {
 		return "http://www.example.com/item-b.html";
 	}
+
 	function Content() {
 		return "ItemB Content";
 	}
+
 	function AltContent() {
 		return "ItemB AltContent";
 	}
@@ -114,7 +116,7 @@ class RSSFeedTest_ItemB extends ViewableData {
 
 class RSSFeedTest_ItemC extends ViewableData {
 	// ItemC tests fields - Title has casting, Content doesn't.
-	static $casting = array(
+	public static $casting = array(
 		'Title' => 'Varchar',
 		'AltContent' => 'Text',
 	);
@@ -126,6 +128,7 @@ class RSSFeedTest_ItemC extends ViewableData {
 	function Link() {
 		return "item-c.html";
 	}
+
 	function AbsoluteLink() {
 		return "http://www.example.com/item-c.html";
 	}