tec: Refactor the export feature (#3045)

Even if the issue #3035 seemed pretty simple at a first glance, it was
more complicated than I expected. Because we send CSP headers AFTER
running the controller actions, it means we can't "echo" any content
from the controller. It's in fact a good practice, but it was easier at
the time we developed the feature.

To fix that, the only thing I had to do was to move the `print()` and
`readfile()` function into the view. The problem was that we needed to
output the content from the CLI too. Then, things became more
complicated. I decided to extract the export-related methods in a
`FreshRSS_Export_Service` class, in order to use it from both the
controller and the CLI. It was an opportunity to refactor the whole
feature in order to make it a bit more linear and easy to read.

Reference: https://github.com/FreshRSS/FreshRSS/issues/3035

app/Controllers/importExportController.php

@@ -720,76 +720,6 @@ class FreshRSS_importExport_Controller extends Minz_ActionController {
 		return $return;
 	}
 
-	/**
-	 * Export the files of a user and send them to the user by HTTP
-	 *
-	 * @param string $username
-	 * @param boolean $export_opml
-	 * @param boolean $export_starred
-	 * @param boolean $export_labelled
-	 * @param array|boolean $export_feeds The list of feeds ids to export, true to export all the feeds
-	 * @param integer $maxFeedEntries Limit the number of entries to export (default is 50)
-	 *
-	 * @throws FreshRSS_ZipMissing_Exception if we try to export more than two files and the zip extension doesn't exist
-	 *
-	 * @return integer The number of exported files
-	 */
-	public function exportFile($username, $export_opml = true, $export_starred = false, $export_labelled = false, $export_feeds = array(), $maxFeedEntries = 50) {
-		require_once(LIB_PATH . '/lib_opml.php');
-
-		$this->catDAO = new FreshRSS_CategoryDAO($username);
-		$this->entryDAO = FreshRSS_Factory::createEntryDao($username);
-		$this->feedDAO = FreshRSS_Factory::createFeedDao($username);
-
-		if ($export_feeds === true) {
-			//All feeds
-			$export_feeds = $this->feedDAO->listFeedsIds();
-		}
-		if (!is_array($export_feeds)) {
-			$export_feeds = array();
-		}
-
-		$day = date('Y-m-d');
-
-		$export_files = array();
-		if ($export_opml) {
-			$export_files["feeds_${day}.opml.xml"] = $this->generateOpml();
-		}
-
-		if ($export_starred || $export_labelled) {
-			$export_files["starred_${day}.json"] = $this->generateEntries(
-				($export_starred ? 'S' : '') .
-				($export_labelled ? 'T' : '')
-			);
-		}
-
-		foreach ($export_feeds as $feed_id) {
-			$feed = $this->feedDAO->searchById($feed_id);
-			if ($feed) {
-				$filename = "feed_${day}_" . $feed->category() . '_'
-				          . $feed->id() . '.json';
-				$export_files[$filename] = $this->generateEntries('f', $feed, $maxFeedEntries);
-			}
-		}
-
-		$nb_files = count($export_files);
-		if ($nb_files > 1) {
-			// If there are more than 1 file to export, we need a ZIP archive.
-			$filename = 'freshrss_' . $username . '_' . $day . '_export.zip';
-			try {
-				$this->sendZip($filename, $export_files);
-			} catch (Exception $e) {
-				throw new FreshRSS_ZipMissing_Exception($e);
-			}
-		} elseif ($nb_files === 1) {
-			// Only one file? Guess its type and export it.
-			$filename = key($export_files);
-			$type = self::guessFileType($filename);
-			$this->sendFile('freshrss_' . $username . '_' . $filename, $export_files[$filename], $type);
-		}
-		return $nb_files;
-	}
-
 	/**
 	 * This action handles export action.
 	 *
@@ -798,135 +728,113 @@ class FreshRSS_importExport_Controller extends Minz_ActionController {
 	 * Parameters are:
 	 *   - export_opml (default: false)
 	 *   - export_starred (default: false)
+	 *   - export_labelled (default: false)
 	 *   - export_feeds (default: array()) a list of feed ids
 	 */
 	public function exportAction() {
 		if (!Minz_Request::isPost()) {
-			Minz_Request::forward(array('c' => 'importExport', 'a' => 'index'), true);
-		}
-		$this->view->_layout(false);
-
-		$nb_files = 0;
-		try {
-			$nb_files = $this->exportFile(
-				Minz_Session::param('currentUser'),
-				Minz_Request::param('export_opml', false),
-				Minz_Request::param('export_starred', false),
-				Minz_Request::param('export_labelled', false),
-				Minz_Request::param('export_feeds', array())
+			return Minz_Request::forward(
+				array('c' => 'importExport', 'a' => 'index'),
+				true
 			);
-		} catch (FreshRSS_ZipMissing_Exception $zme) {
-			# Oops, there is no ZIP extension!
-			Minz_Request::bad(_t('feedback.import_export.export_no_zip_extension'),
-			                  array('c' => 'importExport', 'a' => 'index'));
 		}
 
-		if ($nb_files < 1) {
-			// Nothing to do...
-			Minz_Request::forward(array('c' => 'importExport', 'a' => 'index'), true);
-		}
-	}
+		$username = Minz_Session::param('currentUser');
+		$export_service = new FreshRSS_Export_Service($username);
 
-	/**
-	 * This method returns the OPML file based on user subscriptions.
-	 *
-	 * @return string the OPML file content.
-	 */
-	private function generateOpml() {
-		$list = array();
-		foreach ($this->catDAO->listCategories() as $key => $cat) {
-			$list[$key]['name'] = $cat->name();
-			$list[$key]['feeds'] = $this->feedDAO->listByCategory($cat->id());
+		$export_opml = Minz_Request::param('export_opml', false);
+		$export_starred = Minz_Request::param('export_starred', false);
+		$export_labelled = Minz_Request::param('export_labelled', false);
+		$export_feeds = Minz_Request::param('export_feeds', array());
+		$max_number_entries = 50;
+
+		$exported_files = [];
+
+		if ($export_opml) {
+			list($filename, $content) = $export_service->generateOpml();
+			$exported_files[$filename] = $content;
 		}
 
-		$this->view->categories = $list;
-		return $this->view->helperToString('export/opml');
-	}
+		// Starred and labelled entries are merged in the same `starred` file
+		// to avoid duplication of content.
+		if ($export_starred && $export_labelled) {
+			list($filename, $content) = $export_service->generateStarredEntries('ST');
+			$exported_files[$filename] = $content;
+		} elseif ($export_starred) {
+			list($filename, $content) = $export_service->generateStarredEntries('S');
+			$exported_files[$filename] = $content;
+		} elseif ($export_labelled) {
+			list($filename, $content) = $export_service->generateStarredEntries('T');
+			$exported_files[$filename] = $content;
+		}
 
-	/**
-	 * This method returns a JSON file content.
-	 *
-	 * @param string $type must be one of:
-	 * 	'S' (starred/favourite), 'f' (feed), 'T' (taggued/labelled), 'ST' (starred or labelled)
-	 * @param FreshRSS_Feed $feed feed of which we want to get entries.
-	 * @return string the JSON file content.
-	 */
-	private function generateEntries($type, $feed = null, $maxFeedEntries = 50) {
-		$this->view->categories = $this->catDAO->listCategories();
-		$tagDAO = FreshRSS_Factory::createTagDao();
+		foreach ($export_feeds as $feed_id) {
+			$result = $export_service->generateFeedEntries($feed_id, $max_number_entries);
+			if (!$result) {
+				// It means the actual feed_id doesn't correspond to any existing feed
+				continue;
+			}
 
-		if ($type === 's' || $type === 'S' || $type === 'T' || $type === 'ST') {
-			$this->view->list_title = _t('sub.import_export.starred_list');
-			$this->view->type = 'starred';
-			$this->view->entriesId = $this->entryDAO->listIdsWhere($type, '', FreshRSS_Entry::STATE_ALL, 'ASC', -1);
-			$this->view->entryIdsTagNames = $tagDAO->getEntryIdsTagNames($this->view->entriesId);
-			//The following is a streamable query, i.e. must be last
-			$this->view->entriesRaw = $this->entryDAO->listWhereRaw($type, '', FreshRSS_Entry::STATE_ALL, 'ASC', -1);
-		} elseif ($type === 'f' && $feed != null) {
-			$this->view->list_title = _t('sub.import_export.feed_list', $feed->name());
-			$this->view->type = 'feed/' . $feed->id();
-			$this->view->entriesId = $this->entryDAO->listIdsWhere($type, $feed->id(), FreshRSS_Entry::STATE_ALL, 'ASC', $maxFeedEntries);
-			$this->view->entryIdsTagNames = $tagDAO->getEntryIdsTagNames($this->view->entriesId);
-			//The following is a streamable query, i.e. must be last
-			$this->view->entriesRaw = $this->entryDAO->listWhereRaw($type, $feed->id(), FreshRSS_Entry::STATE_ALL, 'ASC', $maxFeedEntries);
-			$this->view->feed = $feed;
-		}
-
-		return $this->view->helperToString('export/articles');
-	}
+			list($filename, $content) = $result;
+			$exported_files[$filename] = $content;
+		}
 
-	/**
-	 * This method zips a list of files and returns it by HTTP.
-	 *
-	 * @param string $export_filename The name of the file to export
-	 * @param array $files list of files where key is filename and value the content.
-	 * @throws Exception if Zip extension is not loaded.
-	 */
-	private function sendZip($export_filename, $files) {
-		if (!extension_loaded('zip')) {
-			throw new Exception();
+		$nb_files = count($exported_files);
+		if ($nb_files <= 0) {
+			// There's nothing to do, there're no files to export
+			return Minz_Request::forward(
+				array('c' => 'importExport', 'a' => 'index'),
+				true
+			);
 		}
 
-		// From https://stackoverflow.com/questions/1061710/php-zip-files-on-the-fly
-		$zip_file = @tempnam('/tmp', 'zip');
-		$zip = new ZipArchive();
-		$zip->open($zip_file, ZipArchive::OVERWRITE);
+		if ($nb_files === 1) {
+			// If we only have one file, we just export it as it is
+			$filename = key($exported_files);
+			$content = $exported_files[$filename];
+		} else {
+			// More files? Let's compress them in a Zip archive
+			if (!extension_loaded('zip')) {
+				// Oops, there is no ZIP extension!
+				return Minz_Request::bad(
+					_t('feedback.import_export.export_no_zip_extension'),
+					array('c' => 'importExport', 'a' => 'index')
+				);
+			}
 
-		foreach ($files as $filename => $content) {
-			$zip->addFromString($filename, $content);
+			list($filename, $content) = $export_service->zip($exported_files);
 		}
 
-		// Close and send to user
-		$zip->close();
-		header('Content-Type: application/zip');
-		header('Content-Length: ' . filesize($zip_file));
-		header('Content-Disposition: attachment; filename="' . $export_filename . '"');
-		readfile($zip_file);
-		unlink($zip_file);
+		$content_type = self::filenameToContentType($filename);
+		header('Content-Type: ' . $content_type);
+		header('Content-disposition: attachment; filename="' . $filename . '"');
+
+		$this->view->_layout(false);
+		$this->view->content = $content;
 	}
 
 	/**
-	 * This method returns a single file (OPML or JSON) by HTTP.
+	 * Return the Content-Type corresponding to a filename.
+	 *
+	 * If the type of the filename is not supported, it returns
+	 * `application/octet-stream` by default.
 	 *
 	 * @param string $filename
-	 * @param string $content
-	 * @param string $type the file type (opml, json_feed or json_starred).
-	 *                     If equals to unknown, nothing happens.
+	 *
+	 * @return string
 	 */
-	private function sendFile($filename, $content, $type) {
-		if ($type === 'unknown') {
-			return;
-		}
-
-		$content_type = '';
-		if ($type === 'opml') {
-			$content_type = 'application/xml';
-		} elseif ($type === 'json_feed' || $type === 'json_starred') {
-			$content_type = 'application/json';
+	private static function filenameToContentType($filename) {
+		$filetype = self::guessFileType($filename);
+		switch ($filetype) {
+		case 'zip':
+			return 'application/zip';
+		case 'opml':
+			return 'application/xml; charset=utf-8';
+		case 'json_starred':
+		case 'json_feed':
+			return 'application/json; charset=utf-8';
+		default:
+			return 'application/octet-stream';
 		}
-
-		header('Content-Type: ' . $content_type . '; charset=utf-8');
-		header('Content-disposition: attachment; filename=' . $filename);
-		print($content);
 	}
 }

app/Services/ExportService.php

@@ -0,0 +1,189 @@
+<?php
+
+/**
+ * Provide useful methods to generate files to export.
+ */
+class FreshRSS_Export_Service {
+	/** @var string */
+	private $username;
+
+	/** @var FreshRSS_CategoryDAO */
+	private $category_dao;
+
+	/** @var FreshRSS_FeedDAO */
+	private $feed_dao;
+
+	/** @var FreshRSS_EntryDAO */
+	private $entry_dao;
+
+	/** @var FreshRSS_TagDAO */
+	private $tag_dao;
+
+	/**
+	 * Initialize the service for the given user.
+	 *
+	 * @param string $username
+	 */
+	public function __construct($username) {
+		$this->username = $username;
+
+		$this->category_dao = new FreshRSS_CategoryDAO($username);
+		$this->feed_dao = FreshRSS_Factory::createFeedDao($username);
+		$this->entry_dao = FreshRSS_Factory::createEntryDao($username);
+		$this->tag_dao = FreshRSS_Factory::createTagDao();
+	}
+
+	/**
+	 * Generate OPML file content.
+	 *
+	 * @return array First item is the filename, second item is the content
+	 */
+	public function generateOpml() {
+		require_once(LIB_PATH . '/lib_opml.php');
+
+		$view = new Minz_View();
+		$day = date('Y-m-d');
+		$categories = [];
+
+		foreach ($this->category_dao->listCategories() as $key => $category) {
+			$categories[$key]['name'] = $category->name();
+			$categories[$key]['feeds'] = $this->feed_dao->listByCategory($category->id());
+		}
+
+		$view->categories = $categories;
+
+		return [
+			"feeds_{$day}.opml.xml",
+			$view->helperToString('export/opml')
+		];
+	}
+
+	/**
+	 * Generate the starred and labelled entries file content.
+	 *
+	 * Both starred and labelled entries are put into a "starred" file, that's
+	 * why there is only one method for both.
+	 *
+	 * @param string $type must be one of:
+	 *     'S' (starred/favourite),
+	 *     'T' (taggued/labelled),
+	 *     'ST' (starred or labelled)
+	 *
+	 * @return array First item is the filename, second item is the content
+	 */
+	public function generateStarredEntries($type) {
+		$view = new Minz_View();
+		$view->categories = $this->category_dao->listCategories();
+		$day = date('Y-m-d');
+
+		$view->list_title = _t('sub.import_export.starred_list');
+		$view->type = 'starred';
+		$view->entriesId = $this->entry_dao->listIdsWhere(
+			$type, '', FreshRSS_Entry::STATE_ALL, 'ASC', -1
+		);
+		$view->entryIdsTagNames = $this->tag_dao->getEntryIdsTagNames($view->entriesId);
+		// The following is a streamable query, i.e. must be last
+		$view->entriesRaw = $this->entry_dao->listWhereRaw(
+			$type, '', FreshRSS_Entry::STATE_ALL, 'ASC', -1
+		);
+
+		return [
+			"starred_{$day}.json",
+			$view->helperToString('export/articles')
+		];
+	}
+
+	/**
+	 * Generate the entries file content for the given feed.
+	 *
+	 * @param integer $feed_id
+	 * @param integer $max_number_entries
+	 *
+	 * @return array|null First item is the filename, second item is the content.
+	 *                    It also can return null if the feed doesn't exist.
+	 */
+	public function generateFeedEntries($feed_id, $max_number_entries) {
+		$feed = $this->feed_dao->searchById($feed_id);
+		if (!$feed) {
+			return null;
+		}
+
+		$view = new Minz_View();
+		$view->categories = $this->category_dao->listCategories();
+		$view->feed = $feed;
+		$day = date('Y-m-d');
+		$filename = "feed_{$day}_" . $feed->category() . '_' . $feed->id() . '.json';
+
+		$view->list_title = _t('sub.import_export.feed_list', $feed->name());
+		$view->type = 'feed/' . $feed->id();
+		$view->entriesId = $this->entry_dao->listIdsWhere(
+			'f', $feed->id(), FreshRSS_Entry::STATE_ALL, 'ASC', $max_number_entries
+		);
+		$view->entryIdsTagNames = $this->tag_dao->getEntryIdsTagNames($view->entriesId);
+		// The following is a streamable query, i.e. must be last
+		$view->entriesRaw = $this->entry_dao->listWhereRaw(
+			'f', $feed->id(), FreshRSS_Entry::STATE_ALL, 'ASC', $max_number_entries
+		);
+
+		return [
+			$filename,
+			$view->helperToString('export/articles')
+		];
+	}
+
+	/**
+	 * Generate the entries file content for all the feeds.
+	 *
+	 * @param integer $max_number_entries
+	 *
+	 * @return array Keys are filenames and values are contents.
+	 */
+	public function generateAllFeedEntries($max_number_entries) {
+		$feed_ids = $this->feed_dao->listFeedsIds();
+
+		$exported_files = [];
+		foreach ($feed_ids as $feed_id) {
+			$result = $this->generateFeedEntries($feed_id, $max_number_entries);
+			if (!$result) {
+				continue;
+			}
+
+			list($filename, $content) = $result;
+			$exported_files[$filename] = $content;
+		}
+
+		return $exported_files;
+	}
+
+	/**
+	 * Compress several files in a Zip file.
+	 *
+	 * @param array $files where first item is the filename, second item is the content
+	 *
+	 * @return array First item is the zip filename, second item is the zip content
+	 */
+	public function zip($files) {
+		$day = date('Y-m-d');
+		$zip_filename = 'freshrss_' . $this->username . '_' . $day . '_export.zip';
+
+		// From https://stackoverflow.com/questions/1061710/php-zip-files-on-the-fly
+		$zip_file = @tempnam('/tmp', 'zip');
+		$zip_archive = new ZipArchive();
+		$zip_archive->open($zip_file, ZipArchive::OVERWRITE);
+
+		foreach ($files as $filename => $content) {
+			$zip_archive->addFromString($filename, $content);
+		}
+
+		$zip_archive->close();
+
+		$content = file_get_contents($zip_file);
+
+		unlink($zip_file);
+
+		return [
+			$zip_filename,
+			$content,
+		];
+	}
+}

app/views/importExport/export.phtml

@@ -0,0 +1 @@
+<?= $this->content ?>

cli/export-opml-for-user.php

@@ -16,11 +16,10 @@ $username = cliInitUser($options['user']);
 
 fwrite(STDERR, 'FreshRSS exporting OPML for user “' . $username . "”…\n");
 
-$importController = new FreshRSS_importExport_Controller();
-
-$ok = false;
-$ok = $importController->exportFile($username, true, false, false, array(), 0);
+$export_service = new FreshRSS_Export_Service($username);
+list($filename, $content) = $export_service->generateOpml();
+echo $content;
 
 invalidateHttpCache($username);
 
-done($ok);
+done();

cli/export-zip-for-user.php

@@ -13,19 +13,35 @@ if (!validateOptions($argv, $params) || empty($options['user'])) {
 	fail('Usage: ' . basename(__FILE__) . " --user username ( --max-feed-entries 100 ) > /path/to/file.zip");
 }
 
+if (!extension_loaded('zip')) {
+	fail('FreshRSS error: Lacking php-zip extension!');
+}
+
 $username = cliInitUser($options['user']);
 
 fwrite(STDERR, 'FreshRSS exporting ZIP for user “' . $username . "”…\n");
 
-$importController = new FreshRSS_importExport_Controller();
-
-$ok = false;
+$export_service = new FreshRSS_Export_Service($username);
 $number_entries = empty($options['max-feed-entries']) ? 100 : intval($options['max-feed-entries']);
-try {
-	$ok = $importController->exportFile($username, true, true, true, true, $number_entries);
-} catch (FreshRSS_ZipMissing_Exception $zme) {
-	fail('FreshRSS error: Lacking php-zip extension!');
-}
+$exported_files = [];
+
+// First, we generate the OPML file
+list($filename, $content) = $export_service->generateOpml();
+$exported_files[$filename] = $content;
+
+// Then, labelled and starred entries
+list($filename, $content) = $export_service->generateStarredEntries('ST');
+$exported_files[$filename] = $content;
+
+// And a list of entries based on the complete list of feeds
+$feeds_exported_files = $export_service->generateAllFeedEntries($number_entries);
+$exported_files = array_merge($exported_files, $feeds_exported_files);
+
+// Finally, we compress all these files into a single Zip archive and we output
+// the content
+list($filename, $content) = $export_service->zip($exported_files);
+echo $content;
+
 invalidateHttpCache($username);
 
-done($ok);
+done();