Merge pull request #369 from wikimedia/cache-dir

Remove $wsexportConfig global and inject FileCache
wikimedia · Jun 25, 2021 · f472444 · f472444
2 parents 2d6cd01 + f15f6fd
commit f472444
Show file tree

Hide file tree

Showing 25 changed files with 128 additions and 130 deletions.
diff --git a/bin/console b/bin/console
@@ -30,8 +30,6 @@ if ($input->hasParameterOption('--no-debug', true)) {
 
 (new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
 
-require dirname( __DIR__ ) . '/config/bootstrap.php';
-
 if ($_SERVER['APP_DEBUG']) {
     umask(0000);
 

diff --git a/config/bootstrap.php b/config/bootstrap.php
diff --git a/config/services.yaml b/config/services.yaml
@@ -61,5 +61,9 @@ services:
         arguments:
             $timeout: '%env(default::int:APP_TIMEOUT)%'
 
+    App\FileCache:
+        arguments:
+            $projectDir: '%kernel.project_dir%'
+
     # add more service definitions when explicit configuration is needed
     # please note that last definitions always *replace* previous ones
diff --git a/config/services_test.yaml b/config/services_test.yaml
@@ -64,5 +64,9 @@ services:
     App\Repository\CreditRepository:
         public: true
 
+    App\FileCache:
+        arguments:
+            $projectDir: '%kernel.project_dir%'
+
     # add more service definitions when explicit configuration is needed
     # please note that last definitions always *replace* previous ones
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -4,8 +4,8 @@
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/4.8/phpunit.xsd"
          colors="true"
+         backupGlobals="true"
          bootstrap="tests/bootstrap.php">
-<!-- @TODO: Add backupGlobals="true" once the $wsexportConfig issue has been resolved. -->
 
     <php>
         <ini name="error_reporting" value="-1" />

diff --git a/public/index.php b/public/index.php
@@ -28,8 +28,6 @@
 	Request::setTrustedHosts( [ $trustedHosts ] );
 }
 
-require dirname( __DIR__ ) . '/config/bootstrap.php';
-
 $kernel = new Kernel( $_SERVER[ 'APP_ENV' ], (bool)$_SERVER[ 'APP_DEBUG' ] );
 $request = Request::createFromGlobals();
 $response = $kernel->handle( $request );

diff --git a/src/BookCreator.php b/src/BookCreator.php
@@ -21,14 +21,14 @@ class BookCreator {
 	/** @var string Full filesystem path to the created book. */
 	private $filePath;
 
-	public static function forApi( Api $api, $format, $options, GeneratorSelector $generatorSelector, CreditRepository $creditRepo ) {
+	public static function forApi( Api $api, $format, $options, GeneratorSelector $generatorSelector, CreditRepository $creditRepo, FileCache $fileCache ) {
 		if ( !in_array( $format, GeneratorSelector::getAllFormats() ) ) {
 			$list = '"' . implode( '", "', GeneratorSelector::getValidFormats() ) . '"';
 			throw new WsExportException( 'invalid-format', [ $format, $list ], 400 );
 		}
 
 		return new BookCreator(
-			new BookProvider( $api, $options, $creditRepo ),
+			new BookProvider( $api, $options, $creditRepo, $fileCache ),
 			$generatorSelector->getGenerator( $format )
 		);
 	}

diff --git a/src/BookProvider.php b/src/BookProvider.php
@@ -19,14 +19,18 @@ class BookProvider {
 	];
 	private $creditRepo;
 
+	/** @var FileCache */
+	private $fileCache;
+
 	/**
 	 * @param $api Api
 	 * @param bool[] $options
 	 */
-	public function __construct( Api $api, array $options, CreditRepository $creditRepo ) {
+	public function __construct( Api $api, array $options, CreditRepository $creditRepo, FileCache $fileCache ) {
 		$this->api = $api;
 		$this->options = array_merge( $this->options, $options );
 		$this->creditRepo = $creditRepo;
+		$this->fileCache = $fileCache;
 	}
 
 	/**
@@ -208,9 +212,8 @@ protected function getPages( $pages ) {
 	 * @return Picture[]
 	 */
 	protected function getPicturesData( array $pictures ) {
-		$cache = FileCache::singleton();
 		$client = $this->api->getClient();
-		$requests = function () use ( $client, $pictures, $cache ) {
+		$requests = function () use ( $client, $pictures ) {
 			foreach ( $pictures as $picture ) {
 				$url = $picture->url;
 				yield function () use ( $client, $url ) {
@@ -220,11 +223,11 @@ protected function getPicturesData( array $pictures ) {
 			}
 		};
 		$pool = new Pool( $client, $requests(), [
-			'fulfilled' => function ( Response $response, $index ) use ( $cache, $pictures ) {
+			'fulfilled' => function ( Response $response, $index ) use ( $pictures ) {
 				$pictureIndex = array_keys( $pictures )[ $index ];
 
 				// Write the temp file and store its path.
-				$tempFile = $cache->getDirectory() . '/' . uniqid( 'pic-' );
+				$tempFile = $this->fileCache->getDirectory() . '/' . uniqid( 'pic-' );
 				file_put_contents( $tempFile, $response->getBody()->getContents() );
 				$pictures[$pictureIndex]->tempFile = $tempFile;
 

diff --git a/src/Command/CheckCommand.php b/src/Command/CheckCommand.php
@@ -3,6 +3,7 @@
 namespace App\Command;
 
 use App\BookCreator;
+use App\FileCache;
 use App\GeneratorSelector;
 use App\Repository\CreditRepository;
 
@@ -30,14 +31,18 @@ class CheckCommand extends Command {
 	/** @var CreditRepository */
 	private $creditRepo;
 
+	/** @var FileCache */
+	private $fileCache;
+
 	/** @var SymfonyStyle */
 	private $io;
 
-	public function __construct( Api $api, GeneratorSelector $generatorSelector, CreditRepository $creditRepo ) {
+	public function __construct( Api $api, GeneratorSelector $generatorSelector, CreditRepository $creditRepo, FileCache $fileCache ) {
 		parent::__construct();
 		$this->api = $api;
 		$this->generatorSelector = $generatorSelector;
 		$this->creditRepo = $creditRepo;
+		$this->fileCache = $fileCache;
 	}
 
 	protected function configure() {
@@ -125,7 +130,7 @@ private function getPages( InputInterface $input ) {
 	 */
 	private function check( string $page ) {
 		$this->io->section( 'https://' . $this->api->getDomainName() . '/wiki/' . str_replace( ' ', '_', $page ) );
-		$creator = BookCreator::forApi( $this->api, 'epub-3', [ 'credits' => false ], $this->generatorSelector, $this->creditRepo );
+		$creator = BookCreator::forApi( $this->api, 'epub-3', [ 'credits' => false ], $this->generatorSelector, $this->creditRepo, $this->fileCache );
 		$creator->create( $page );
 		$jsonOutput = $creator->getFilePath() . '_epubcheck.json';
 		$process = new Process( [ 'epubcheck', $creator->getFilePath(), '--json', $jsonOutput ] );

diff --git a/src/Command/ExportCommand.php b/src/Command/ExportCommand.php
@@ -3,6 +3,7 @@
 namespace App\Command;
 
 use App\BookCreator;
+use App\FileCache;
 use App\GeneratorSelector;
 use App\Repository\CreditRepository;
 use App\Util\Api;
@@ -29,12 +30,16 @@ class ExportCommand extends Command {
 	/** @var bool */
 	private $enableCache;
 
-	public function __construct( GeneratorSelector $generatorSelector, CreditRepository $creditRepo, Api $api, bool $enableCache ) {
+	/** @var FileCache */
+	private $fileCache;
+
+	public function __construct( GeneratorSelector $generatorSelector, CreditRepository $creditRepo, Api $api, bool $enableCache, FileCache $fileCache ) {
 		parent::__construct();
 		$this->generatorSelector = $generatorSelector;
 		$this->creditRepo = $creditRepo;
 		$this->api = $api;
 		$this->enableCache = $enableCache;
+		$this->fileCache = $fileCache;
 	}
 
 	protected function configure() {
@@ -80,7 +85,7 @@ protected function execute( InputInterface $input, OutputInterface $output ): in
 			$io->writeln( 'Caching is disabled.' );
 			$this->api->disableCache();
 		}
-		$creator = BookCreator::forApi( $this->api, $input->getOption( 'format' ), $options, $this->generatorSelector, $this->creditRepo );
+		$creator = BookCreator::forApi( $this->api, $input->getOption( 'format' ), $options, $this->generatorSelector, $this->creditRepo, $this->fileCache );
 		$creator->create( $input->getOption( 'title' ), $input->getOption( 'path' ) );
 
 		$io->success( [

diff --git a/src/Command/OpdsCommand.php b/src/Command/OpdsCommand.php
@@ -3,6 +3,7 @@
 namespace App\Command;
 
 use App\BookProvider;
+use App\FileCache;
 use App\OpdsBuilder;
 use App\Repository\CreditRepository;
 use App\Util\Api;
@@ -22,10 +23,14 @@ class OpdsCommand extends Command {
 	/** @var CreditRepository */
 	private $creditRepo;
 
-	public function __construct( Api $api, CreditRepository $creditRepo ) {
+	/** @var FileCache */
+	private $fileCache;
+
+	public function __construct( Api $api, CreditRepository $creditRepo, FileCache $fileCache ) {
 		parent::__construct();
 		$this->api = $api;
 		$this->creditRepo = $creditRepo;
+		$this->fileCache = $fileCache;
 	}
 
 	protected function configure() {
@@ -52,10 +57,10 @@ protected function execute( InputInterface $input, OutputInterface $output ): in
 
 		date_default_timezone_set( 'UTC' );
 		$this->api->setLang( $lang );
-		$provider = new BookProvider( $this->api, [ 'categories' => false, 'images' => false ], $this->creditRepo );
+		$provider = new BookProvider( $this->api, [ 'categories' => false, 'images' => false ], $this->creditRepo, $this->fileCache );
 
 		$exportPath = 'https://ws-export.wmcloud.org/';
-		$atomGenerator = new OpdsBuilder( $provider, $this->api, $lang, $exportPath );
+		$atomGenerator = new OpdsBuilder( $provider, $this->api, $lang, $this->fileCache, $exportPath );
 		$outputFile = dirname( __DIR__, 2 ) . "/public/opds/$lang/$category.xml";
 		if ( !is_dir( dirname( $outputFile ) ) ) {
 			mkdir( dirname( $outputFile ), 0755, true );

diff --git a/src/Controller/ExportController.php b/src/Controller/ExportController.php
@@ -5,6 +5,7 @@
 use App\BookCreator;
 use App\Entity\GeneratedBook;
 use App\Exception\WsExportException;
+use App\FileCache;
 use App\FontProvider;
 use App\GeneratorSelector;
 use App\Refresh;
@@ -81,7 +82,8 @@ public function home(
 		Api $api,
 		FontProvider $fontProvider,
 		GeneratorSelector $generatorSelector,
-		CreditRepository $creditRepo
+		CreditRepository $creditRepo,
+		FileCache $fileCache
 	) {
 		// Handle ?refresh=1 for backwards compatibility.
 		if ( $request->get( 'refresh', false ) !== false ) {
@@ -100,7 +102,7 @@ public function home(
 		$response = new Response();
 		if ( $request->get( 'page' ) ) {
 			try {
-				return $this->export( $request, $api, $fontProvider, $generatorSelector, $creditRepo );
+				return $this->export( $request, $api, $fontProvider, $generatorSelector, $creditRepo, $fileCache );
 			} catch ( WsExportException $ex ) {
 				$exception = $ex;
 				$response->setStatusCode( $ex->getResponseCode() );
@@ -131,7 +133,8 @@ private function export(
 		Api $api,
 		FontProvider $fontProvider,
 		GeneratorSelector $generatorSelector,
-		CreditRepository $creditRepo
+		CreditRepository $creditRepo,
+		FileCache $fileCache
 	) {
 		// Get params.
 		$page = $request->get( 'page' );
@@ -149,7 +152,7 @@ private function export(
 
 		// Generate ebook.
 		$options = [ 'images' => $images, 'fonts' => $font, 'credits' => $credits ];
-		$creator = BookCreator::forApi( $api, $format, $options, $generatorSelector, $creditRepo );
+		$creator = BookCreator::forApi( $api, $format, $options, $generatorSelector, $creditRepo, $fileCache );
 		$creator->create( $page );
 
 		// Send file.

diff --git a/src/FileCache.php b/src/FileCache.php
@@ -2,6 +2,7 @@
 
 namespace App;
 
+use App\Util\Util;
 use DirectoryIterator;
 use Exception;
 
@@ -22,9 +23,10 @@ class FileCache {
 	private static $instance;
 
 	/**
-	 * @param string $path Directory for temp files
+	 * @param string $projectDir
 	 */
-	private function __construct( string $path ) {
+	public function __construct( string $projectDir ) {
+		$path = $projectDir . '/var/file-cache/';
 		$this->dir = $this->makePrivateDirectory( $path );
 
 		// Randomly clean up the cache
@@ -36,17 +38,21 @@ private function __construct( string $path ) {
 	}
 
 	/**
-	 * Returns a global instance of this class
-	 * @return FileCache
+	 * Builds a unique temporary file name for a given title and extension.
+	 *
+	 * @param string $title
+	 * @param string $extension
+	 * @return string
 	 */
-	public static function singleton(): FileCache {
-		global $wsexportConfig;
-
-		if ( !self::$instance ) {
-			self::$instance = new FileCache( $wsexportConfig['tempPath'] );
+	public function buildTemporaryFileName( $title, $extension ) {
+		for ( $i = 0; $i < 100; $i++ ) {
+			$path = $this->getDirectory() . '/' . 'ws-' . Util::encodeString( $title ) . '-' . getmypid() . rand() . '.' . $extension;
+			if ( !file_exists( $path ) ) {
+				return $path;
+			}
 		}
 
-		return self::$instance;
+		throw new Exception( 'Unable to create temporary file' );
 	}
 
 	/**
@@ -71,11 +77,11 @@ private function makePrivateDirectory( string $path ): string {
 		$user = preg_replace( '/^tools\./', '', $user );
 		$dir = rtrim( $path, '/' ) . '/' . $user;
 
-		// Guard against realpth() returning false sometimes
+		// Guard against realpath() returning false sometimes
 		$dir = realpath( $dir ) ?: $dir;
 
 		if ( !is_dir( $dir ) ) {
-			if ( !mkdir( $dir, 0755 ) ) {
+			if ( !mkdir( $dir, 0755, true ) ) {
 				throw new Exception( "Couldn't create temporary directory $dir" );
 			}
 		}