From 1d268be4f817484047d9a9dad934d15b3e7473a9 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 11 Jan 2019 09:52:00 -0500 Subject: [PATCH 01/22] WIP - re-adding caching of Metas --- lib/Builder.php | 25 +++++++++++++++++++++++++ lib/Builder/Scanner.php | 2 +- lib/Meta/Metas.php | 8 ++++++++ tests/Builder/BuilderTest.php | 23 +++++++++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/Builder.php b/lib/Builder.php index c03ca035..48ea605f 100644 --- a/lib/Builder.php +++ b/lib/Builder.php @@ -13,6 +13,7 @@ use Doctrine\RST\Event\PreBuildParseEvent; use Doctrine\RST\Event\PreBuildRenderEvent; use Doctrine\RST\Event\PreBuildScanEvent; +use Doctrine\RST\Meta\MetaEntry; use Doctrine\RST\Meta\Metas; use Symfony\Component\Filesystem\Filesystem; use function is_dir; @@ -126,11 +127,15 @@ public function build( $this->filesystem->mkdir($targetDirectory, 0755); } + $this->loadCachedMetas($targetDirectory); + $this->scan($directory, $targetDirectory); $this->parse($directory, $targetDirectory); $this->render($directory, $targetDirectory); + + $this->saveMetas($targetDirectory); } public function copy(string $source, ?string $destination = null) : self @@ -198,4 +203,24 @@ private function render(string $directory, string $targetDirectory) : void new PostBuildRenderEvent($this, $directory, $targetDirectory) ); } + + private function loadCachedMetas(string $targetDirectory) : void + { + $metaCachePath = $this->getMetaCachePath($targetDirectory); + if (!file_exists($metaCachePath)) { + return; + } + + $this->metas->setMetaEntries(unserialize(file_get_contents($metaCachePath))); + } + + private function saveMetas(string $targetDirectory) : void + { + file_put_contents($this->getMetaCachePath($targetDirectory), serialize($this->metas->getAll())); + } + + private function getMetaCachePath(string $targetDirectory) : string + { + return $targetDirectory.'/metas.php'; + } } diff --git a/lib/Builder/Scanner.php b/lib/Builder/Scanner.php index 205cdf1e..7ce3b1c4 100644 --- a/lib/Builder/Scanner.php +++ b/lib/Builder/Scanner.php @@ -33,7 +33,7 @@ public function scan(string $directory, string $file) : void $entry = $this->metas->get($file); - $rst = $directory . '/' . $file; + $rst = $directory . '/' . $file . '.rst'; if ($entry === null || ! file_exists($rst) || $entry->getCtime() < filectime($rst)) { // File was never seen or changed and thus need to be parsed diff --git a/lib/Meta/Metas.php b/lib/Meta/Metas.php index 3b618128..f12319af 100644 --- a/lib/Meta/Metas.php +++ b/lib/Meta/Metas.php @@ -97,6 +97,14 @@ public function get(string $url) : ?MetaEntry return null; } + /** + * @param MetaEntry[] $metaEntries + */ + public function setMetaEntries(array $metaEntries) + { + $this->entries = $metaEntries; + } + /** * @param string[] $links */ diff --git a/tests/Builder/BuilderTest.php b/tests/Builder/BuilderTest.php index f1a0c6cb..e19f0762 100644 --- a/tests/Builder/BuilderTest.php +++ b/tests/Builder/BuilderTest.php @@ -4,6 +4,7 @@ namespace Doctrine\Tests\RST\Builder; +use Doctrine\RST\Builder; use Doctrine\Tests\RST\BaseBuilderTest; use function file_exists; use function is_dir; @@ -38,6 +39,28 @@ public function testBuild() : void self::assertTrue(file_exists($this->targetFile('subdir/file.html'))); } + public function testCachedMetas() : void + { + // check that metas were cached + self::assertTrue(file_exists($this->targetFile('metas.php'))); + $cachedContents = file_get_contents($this->targetFile('metas.php')); + $metaEntries = unserialize($cachedContents); + self::assertArrayHasKey('index', $metaEntries); + self::assertSame('Summary', $metaEntries['index']->getTitle()); + + // update meta cache to see that it was used + file_put_contents( + $this->targetFile('metas.php'), + str_replace('Summary', 'Sumario', $cachedContents) + ); + + $builder = new Builder(); + $builder->build($this->sourceFile(), $this->targetFile()); + + $contents = $this->getFileContents($this->targetFile('index.html')); + self::assertContains('Sumario', $contents); + } + /** * Tests the ..url :: directive */ From 9977ce2a4313474d6477c5f8340d479be832421a Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Tue, 15 Jan 2019 22:01:09 -0500 Subject: [PATCH 02/22] Cleaning up Environment - giving it proper required args --- lib/Builder/ParseQueueProcessor.php | 18 +++++++------ lib/Environment.php | 41 +++++++++-------------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/lib/Builder/ParseQueueProcessor.php b/lib/Builder/ParseQueueProcessor.php index 2ea35efc..8c766ccc 100644 --- a/lib/Builder/ParseQueueProcessor.php +++ b/lib/Builder/ParseQueueProcessor.php @@ -4,6 +4,7 @@ namespace Doctrine\RST\Builder; +use Doctrine\RST\Environment; use Doctrine\RST\ErrorManager; use Doctrine\RST\Kernel; use Doctrine\RST\Meta\Metas; @@ -105,14 +106,15 @@ private function processFile(string $file) : void private function createFileParser(string $file) : Parser { - $parser = new Parser($this->kernel); - - $environment = $parser->getEnvironment(); - $environment->setMetas($this->metas); - $environment->setCurrentFileName($file); - $environment->setCurrentDirectory($this->directory); - $environment->setTargetDirectory($this->targetDirectory); - $environment->setErrorManager($this->errorManager); + $environment = new Environment( + $this->kernel->getConfiguration(), + $file, + $this->metas, + $this->directory, + $this->targetDirectory, + $this->errorManager + ); + $parser = new Parser($this->kernel, $environment); return $parser; } diff --git a/lib/Environment.php b/lib/Environment.php index 23ab13cd..2ad23865 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -76,14 +76,24 @@ class Environment /** @var InvalidLink[] */ private $invalidLinks = []; - public function __construct(Configuration $configuration) + public function __construct( + Configuration $configuration, + string $currentFileName, + Metas $metas, + string $currentDirectory, + string $targetDirectory, + ErrorManager $errorManager + ) { $this->configuration = $configuration; - $this->errorManager = new ErrorManager($this->configuration); + $this->currentFileName = $currentFileName; + $this->metas = $metas; + $this->currentDirectory = $currentDirectory; + $this->targetDirectory = $targetDirectory; + $this->errorManager = $errorManager; $this->urlGenerator = new UrlGenerator( $this->configuration ); - $this->metas = new Metas(); $this->reset(); } @@ -111,16 +121,6 @@ public function getErrorManager() : ErrorManager return $this->errorManager; } - public function setErrorManager(ErrorManager $errorManager) : void - { - $this->errorManager = $errorManager; - } - - public function setMetas(Metas $metas) : void - { - $this->metas = $metas; - } - public function getNodeFactory() : NodeFactory { return $this->configuration->getNodeFactory($this); @@ -336,21 +336,11 @@ public function getDirName() : string return $dirname; } - public function setCurrentFileName(string $filename) : void - { - $this->currentFileName = $filename; - } - public function getCurrentFileName() : string { return $this->currentFileName; } - public function setCurrentDirectory(string $directory) : void - { - $this->currentDirectory = $directory; - } - public function getCurrentDirectory() : string { return $this->currentDirectory; @@ -361,11 +351,6 @@ public function absoluteRelativePath(string $url) : string return $this->currentDirectory . '/' . $this->getDirName() . '/' . $this->relativeUrl($url); } - public function setTargetDirectory(string $directory) : void - { - $this->targetDirectory = $directory; - } - public function getTargetDirectory() : string { return $this->targetDirectory; From 31b577dfac8f44e72a6aabf0b45fd970d9e87892 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Tue, 15 Jan 2019 22:02:20 -0500 Subject: [PATCH 03/22] Removing unused method --- lib/Builder.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/Builder.php b/lib/Builder.php index 48ea605f..a4530899 100644 --- a/lib/Builder.php +++ b/lib/Builder.php @@ -96,11 +96,6 @@ public function getDocuments() : Documents return $this->documents; } - public function getParseQueue() : Builder\ParseQueue - { - return $this->parseQueue; - } - public function getErrorManager() : ErrorManager { return $this->errorManager; From 655bd0133007f183de39c4e68c8bab16b6b8e0d8 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 18 Jan 2019 14:21:16 -0500 Subject: [PATCH 04/22] Refactoring how the Scanner system works It's much simpler: Sphinx finds all files in the given directory, recursively, that have the .rst file extension. And it parses all of them. There is no need to follow "dependencies" to find more things to parse: just parse all files you find. Sure, then later you might be able to give a warning if some documents you rendered were not included in any toctree's - but you will still render those orphan documents. --- lib/Builder.php | 31 +-- lib/Builder/ParseQueue.php | 61 +++-- lib/Builder/ParseQueueProcessor.php | 35 +-- lib/Builder/Scanner.php | 182 +++++++++++---- lib/Builder/State.php | 15 -- lib/Configuration.php | 8 + tests/Builder/ParseQueueProcessorTest.php | 21 +- tests/Builder/ParseQueueTest.php | 56 ++--- tests/Builder/ScannerTest.php | 268 ++++++++++++++++------ 9 files changed, 405 insertions(+), 272 deletions(-) delete mode 100644 lib/Builder/State.php diff --git a/lib/Builder.php b/lib/Builder.php index a4530899..79fccec6 100644 --- a/lib/Builder.php +++ b/lib/Builder.php @@ -13,7 +13,6 @@ use Doctrine\RST\Event\PreBuildParseEvent; use Doctrine\RST\Event\PreBuildRenderEvent; use Doctrine\RST\Event\PreBuildScanEvent; -use Doctrine\RST\Meta\MetaEntry; use Doctrine\RST\Meta\Metas; use Symfony\Component\Filesystem\Filesystem; use function is_dir; @@ -38,12 +37,6 @@ class Builder /** @var Documents */ private $documents; - /** @var ParseQueue */ - private $parseQueue; - - /** @var Scanner */ - private $scanner; - /** @var Copier */ private $copier; @@ -67,10 +60,6 @@ public function __construct(?Kernel $kernel = null) $this->metas ); - $this->parseQueue = new Builder\ParseQueue($this->documents); - - $this->scanner = new Builder\Scanner($this->parseQueue, $this->metas); - $this->copier = new Builder\Copier($this->filesystem); $this->kernel->initBuilder($this); @@ -124,9 +113,9 @@ public function build( $this->loadCachedMetas($targetDirectory); - $this->scan($directory, $targetDirectory); + $parseQueue = $this->scan($directory, $targetDirectory); - $this->parse($directory, $targetDirectory); + $this->parse($directory, $targetDirectory, $parseQueue); $this->render($directory, $targetDirectory); @@ -147,19 +136,23 @@ public function mkdir(string $directory) : self return $this; } - private function scan(string $directory, string $targetDirectory) : void + private function scan(string $directory, string $targetDirectory) : ParseQueue { $this->configuration->dispatchEvent( PreBuildScanEvent::PRE_BUILD_SCAN, new PreBuildScanEvent($this, $directory, $targetDirectory) ); - $this->scanner->scan($directory, $this->getIndexName()); + $scanner = new Scanner( + $this->configuration->getSourceFileExtension(), + $directory, + $this->metas + ); - $this->scanner->scanMetas($directory); + return $scanner->scan($directory, $this->metas); } - private function parse(string $directory, string $targetDirectory) : void + private function parse(string $directory, string $targetDirectory, ParseQueue $parseQueue) : void { $this->configuration->dispatchEvent( PreBuildParseEvent::PRE_BUILD_PARSE, @@ -169,16 +162,14 @@ private function parse(string $directory, string $targetDirectory) : void $parseQueueProcessor = new ParseQueueProcessor( $this->kernel, $this->errorManager, - $this->parseQueue, $this->metas, $this->documents, - $this->scanner, $directory, $targetDirectory, $this->configuration->getFileExtension() ); - $parseQueueProcessor->process(); + $parseQueueProcessor->process($parseQueue); } private function render(string $directory, string $targetDirectory) : void diff --git a/lib/Builder/ParseQueue.php b/lib/Builder/ParseQueue.php index ff7f39d7..a3dfb571 100644 --- a/lib/Builder/ParseQueue.php +++ b/lib/Builder/ParseQueue.php @@ -4,51 +4,46 @@ namespace Doctrine\RST\Builder; -use function array_shift; - -class ParseQueue +final class ParseQueue { - /** @var Documents */ - private $documents; - - /** @var string[] */ - private $parseQueue = []; - - /** @var int[] */ - private $states = []; - - public function __construct(Documents $documents) + /** + * An array where each key is the filename and the value is a + * boolean indicating if the file needs to be parsed or not. + * + * @var bool[] + */ + private $fileStatuses = []; + + public function addFile(string $filename, bool $parseNeeded) : void { - $this->documents = $documents; - } - - public function getState(string $file) : ?int - { - return $this->states[$file] ?? null; + if (isset($this->fileStatuses[$filename])) { + throw new \InvalidArgumentException(sprintf('File "%s" is already in the parse queue', $filename)); + } + + $this->fileStatuses[$filename] = $parseNeeded; } - public function setState(string $file, int $state) : void + public function isFileKnownToParseQueue(string $filename) : bool { - $this->states[$file] = $state; + return array_key_exists($filename, $this->fileStatuses); } - public function getFileToParse() : ?string + public function doesFileRequireParsing(string $filename) : bool { - if ($this->parseQueue !== []) { - return array_shift($this->parseQueue); + if (!$this->isFileKnownToParseQueue($filename)) { + throw new \InvalidArgumentException(sprintf('File "%s" is not known to the parse queue', $filename)); } - return null; + return $this->fileStatuses[$filename]; } - public function addToParseQueue(string $file) : void + /** + * @return string[] + */ + public function getAllFilesThatRequireParsing() : array { - $this->states[$file] = State::PARSE; - - if ($this->documents->hasDocument($file)) { - return; - } - - $this->parseQueue[$file] = $file; + return array_keys(array_filter($this->fileStatuses, function(bool $parseNeeded) { + return $parseNeeded; + })); } } diff --git a/lib/Builder/ParseQueueProcessor.php b/lib/Builder/ParseQueueProcessor.php index 8c766ccc..b8b0b971 100644 --- a/lib/Builder/ParseQueueProcessor.php +++ b/lib/Builder/ParseQueueProcessor.php @@ -7,6 +7,7 @@ use Doctrine\RST\Environment; use Doctrine\RST\ErrorManager; use Doctrine\RST\Kernel; +use Doctrine\RST\Meta\DocumentDependency; use Doctrine\RST\Meta\Metas; use Doctrine\RST\Nodes\DocumentNode; use Doctrine\RST\Parser; @@ -22,18 +23,12 @@ class ParseQueueProcessor /** @var ErrorManager */ private $errorManager; - /** @var ParseQueue */ - private $parseQueue; - /** @var Metas */ private $metas; /** @var Documents */ private $documents; - /** @var Scanner */ - private $scanner; - /** @var string */ private $directory; @@ -46,28 +41,24 @@ class ParseQueueProcessor public function __construct( Kernel $kernel, ErrorManager $errorManager, - ParseQueue $parseQueue, Metas $metas, Documents $documents, - Scanner $scanner, string $directory, string $targetDirectory, string $fileExtension ) { $this->kernel = $kernel; $this->errorManager = $errorManager; - $this->parseQueue = $parseQueue; $this->metas = $metas; $this->documents = $documents; - $this->scanner = $scanner; $this->directory = $directory; $this->targetDirectory = $targetDirectory; $this->fileExtension = $fileExtension; } - public function process() : void + public function process(ParseQueue $parseQueue) : void { - while ($file = $this->parseQueue->getFileToParse()) { + foreach ($parseQueue->getAllFilesThatRequireParsing() as $file) { $this->processFile($file); } } @@ -86,12 +77,6 @@ private function processFile(string $file) : void $this->kernel->postParse($document); - $dependencies = $environment->getDependencies(); - - foreach ($this->buildDependenciesToScan($dependencies) as $dependency) { - $this->scanner->scan($this->directory, $dependency); - } - $this->metas->set( $file, $this->buildDocumentUrl($document), @@ -99,7 +84,7 @@ private function processFile(string $file) : void $document->getTitles(), $document->getTocs(), (int) filectime($fileAbsolutePath), - $dependencies, + $environment->getDependencies(), $environment->getLinks() ); } @@ -119,18 +104,6 @@ private function createFileParser(string $file) : Parser return $parser; } - /** - * @param string[] $dependencies - * - * @return string[] - */ - private function buildDependenciesToScan(array $dependencies) : array - { - return array_filter($dependencies, function (string $dependency) : bool { - return file_exists($this->buildFileAbsolutePath($dependency)); - }); - } - private function buildFileAbsolutePath(string $file) : string { return $this->directory . '/' . $file . '.rst'; diff --git a/lib/Builder/Scanner.php b/lib/Builder/Scanner.php index 7ce3b1c4..0b6ebed1 100644 --- a/lib/Builder/Scanner.php +++ b/lib/Builder/Scanner.php @@ -4,76 +4,178 @@ namespace Doctrine\RST\Builder; -use Doctrine\RST\Meta\MetaEntry; use Doctrine\RST\Meta\Metas; -use function file_exists; -use function filectime; +use Symfony\Component\Finder\Finder; +use Symfony\Component\Finder\SplFileInfo; class Scanner { - /** @var ParseQueue */ - private $parseQueue; + private $fileExtension; + + private $directory; - /** @var Metas */ private $metas; - public function __construct(ParseQueue $parseQueue, Metas $metas) + private $finder; + + /** @var SplFileInfo[] */ + private $fileInfos = []; + + public function __construct(string $fileExtension, string $directory, Metas $metas, Finder $finder = null) { - $this->parseQueue = $parseQueue; - $this->metas = $metas; + $this->fileExtension = $fileExtension; + $this->directory = $directory; + $this->metas = $metas; + + $this->finder = $finder ?: new Finder(); + $this->finder->in($this->directory) + ->files() + ->name('*.'.$this->fileExtension); } - public function scan(string $directory, string $file) : void + /** + * Scans a directory recursively looking for all files to parse. + * + * This takes into account the presence of cached & fresh MetaEntry + * objects, and avoids adding files to the parse queue that have + * not changed and whose dependencies have not changed. + */ + public function scan() : ParseQueue { - if ($this->parseQueue->getState($file) !== null) { - return; + // completely populate the splFileInfos property + $this->fileInfos = []; + foreach ($this->finder as $fileInfo) { + $relativeFilename = $fileInfo->getRelativePathname(); + // strip off the extension + $documentPath = substr($relativeFilename, 0, -(strlen($this->fileExtension) + 1)); + + $this->fileInfos[$documentPath] = $fileInfo; } - $this->parseQueue->setState($file, State::NO_PARSE); + $parseQueue = new ParseQueue(); + foreach ($this->fileInfos as $filename => $fileInfo) { + $parseQueue->addFile( + $filename, + $this->doesFileRequireParsing($filename, $parseQueue) + ); + } - $entry = $this->metas->get($file); + return $parseQueue; + + /* + * 1) See if meta exists. If it does not, add it to ParseQueue + * as needing to PARSE_NEEDED + * 2) If meta DOES exist + * a) if stale, PARSE_NEEDED + * b) if not stale, loop over each dependency and + * do the exact same check + * -> how can we avoid circular issues? + * -> basically, each iteration of the loop + * going deeper just needs to know that + * if it hits its parent caller/file, it + * should take no information from this, + * but not try to follow its dependencies. + * -> could possibly do this with a flag called + * PARSE_PROCESSING, which is released after + * you follow all of your dependencies. If + * you hit a PARSE_PROCESSING on one of your + * dependencies, you just exist and re-set + * your status back to some unknown. If you + * ARE able to determine from your dependencies + * if you need to be parsed, then you set to whatever + * that status is. + * -> this probably means adding everything to the + * ParseQueue in the beginning. And maybe this + * just becomes a utility class of the Scanner, + * and an array of filenames or array of SourceDocument + * objects is returned. Maybe Scanner can be more + * stateless, and a lot of the recursive logic is + * moved into this temporary, stateful ParseQueue. + * i) if no dependencies need to be re-parsed, don't parse + * ii) if any need re-parsing, re-parse + * + * We will use ParseQueue in an intelligent way - actually asking + * it if it is aware of a file yet, instead of using all this getState() + * garbage + */ + } - $rst = $directory . '/' . $file . '.rst'; + private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue, array $filesAlreadyBeingChecked = []) : bool + { + if (!isset($this->fileInfos[$filename])) { + throw new \InvalidArgumentException(sprintf('No file info found for "%s" - file does not exist.', $filename)); + } + + $file = $this->fileInfos[$filename]; + + $documentFilename = $this->getFilenameFromFile($file); + $entry = $this->metas->get($documentFilename); - if ($entry === null || ! file_exists($rst) || $entry->getCtime() < filectime($rst)) { - // File was never seen or changed and thus need to be parsed - $this->parseQueue->addToParseQueue($file); - } else { - $this->scanMetaEntry($entry, $directory, $file); + if ($entry === null || $entry->getCtime() < $file->getCTime()) { + // File is new or changed and thus need to be parsed + return true; } - } - public function scanMetaEntry(MetaEntry $entry, string $directory, string $file) : void - { - // Have a look to the file dependencies to know if you need to parse - // it or not - $depends = $entry->getDepends(); + // Look to the file's dependencies to know if you need to parse it or not + $dependencies = $entry->getDepends(); $parent = $entry->getParent(); - if ($parent !== null) { - $depends[] = $parent; + $dependencies[] = $parent; } - foreach ($depends as $dependency) { - $this->scan($directory, $dependency); + $filesAlreadyBeingChecked[] = $documentFilename; + + foreach ($dependencies as $dependency) { + if (in_array($dependency, $filesAlreadyBeingChecked, true)) { + /* + * File is already being checked. For example, consider + * this dependency tree: + * + * DocA (depends on)-> + * DocB (depends on)-> + * DocC (depends on)-> DocB & DocD + * + * And assume only DocD has changed. + * The method will be called recursively for DocB, then DocC. + * When that happens, it needs to realize that we're already + * checking to see if DocB has changed. And so, we should not + * recursively check DocB again. It's a no-op: we don't know + * if DocB has changed yet or not. So, we skip, and check DocD. + */ + + continue; + } + + // if the parseQueue already knows about this file, just ask it + if ($parseQueue->isFileKnownToParseQueue($dependency)) { + if ($parseQueue->doesFileRequireParsing($dependency)) { + return true; + } - // If any dependency needs to be parsed, this file needs also to be - // parsed - if ($this->parseQueue->getState($dependency) !== State::PARSE) { continue; } - $this->parseQueue->addToParseQueue($file); + // dependency no longer exists? We should re-parse this file + if (!isset($this->fileInfos[$dependency])) { + return true; + } + + // finally, we need to recursively ask if this file needs parsing + if ($this->doesFileRequireParsing($dependency, $parseQueue, $filesAlreadyBeingChecked)) { + return true; + } } + + // Meta is fresh and no dependencies need parsing + return false; } - public function scanMetas(string $directory) : void + /** + * Converts foo/bar.rst to foo/bar (the document filename) + */ + private function getFilenameFromFile(SplFileInfo $file) : string { - $entries = $this->metas->getAll(); - - foreach ($entries as $file => $infos) { - $this->scan($directory, $file); - } + return substr($file->getRelativePathname(), 0, -(strlen($this->fileExtension) + 1)); } } diff --git a/lib/Builder/State.php b/lib/Builder/State.php deleted file mode 100644 index 3020c5e6..00000000 --- a/lib/Builder/State.php +++ /dev/null @@ -1,15 +0,0 @@ -formats[$this->fileExtension]; } + public function getSourceFileExtension() : string + { + return $this->sourceFileExtension; + } + private function createNodeInstantiator(Environment $environment, string $type, string $nodeClassName) : NodeInstantiator { return new NodeInstantiator( diff --git a/tests/Builder/ParseQueueProcessorTest.php b/tests/Builder/ParseQueueProcessorTest.php index eed54c2c..7bfb236f 100644 --- a/tests/Builder/ParseQueueProcessorTest.php +++ b/tests/Builder/ParseQueueProcessorTest.php @@ -24,18 +24,12 @@ class ParseQueueProcessorTest extends TestCase /** @var ErrorManager|MockObject */ private $errorManager; - /** @var ParseQueue|MockObject */ - private $parseQueue; - /** @var Metas|MockObject */ private $metas; /** @var Documents|MockObject */ private $documents; - /** @var Scanner|MockObject */ - private $scanner; - /** @var string */ private $directory; @@ -52,13 +46,8 @@ public function testProcess() : void { touch($this->directory . '/file.rst'); - $this->parseQueue->expects(self::at(0)) - ->method('getFileToParse') - ->willReturn('file'); - - $this->parseQueue->expects(self::at(1)) - ->method('getFileToParse') - ->willReturn(null); + $parseQueue = new ParseQueue(); + $parseQueue->addFile('file', true); $this->documents->expects(self::once()) ->method('addDocument') @@ -70,17 +59,15 @@ public function testProcess() : void $this->metas->expects(self::once()) ->method('set'); - $this->parseQueueProcessor->process(); + $this->parseQueueProcessor->process($parseQueue); } protected function setUp() : void { $this->kernel = $this->createMock(Kernel::class); $this->errorManager = $this->createMock(ErrorManager::class); - $this->parseQueue = $this->createMock(ParseQueue::class); $this->metas = $this->createMock(Metas::class); $this->documents = $this->createMock(Documents::class); - $this->scanner = $this->createMock(Scanner::class); $this->directory = sys_get_temp_dir(); $this->targetDirectory = '/target'; $this->fileExtension = 'rst'; @@ -88,10 +75,8 @@ protected function setUp() : void $this->parseQueueProcessor = new ParseQueueProcessor( $this->kernel, $this->errorManager, - $this->parseQueue, $this->metas, $this->documents, - $this->scanner, $this->directory, $this->targetDirectory, $this->fileExtension diff --git a/tests/Builder/ParseQueueTest.php b/tests/Builder/ParseQueueTest.php index eb107d3e..e361ba61 100644 --- a/tests/Builder/ParseQueueTest.php +++ b/tests/Builder/ParseQueueTest.php @@ -12,49 +12,19 @@ class ParseQueueTest extends TestCase { - /** @var Documents|MockObject */ - private $documents; - - /** @var ParseQueue */ - private $parseQueue; - - public function testGetSetState() : void - { - self::assertNull($this->parseQueue->getState('file')); - - $this->parseQueue->setState('file', State::NO_PARSE); - - self::assertSame(State::NO_PARSE, $this->parseQueue->getState('file')); - } - - public function testGetFileToParse() : void - { - self::assertNull($this->parseQueue->getFileToParse()); - - $this->parseQueue->addToParseQueue('file1'); - $this->parseQueue->addToParseQueue('file2'); - - self::assertSame('file1', $this->parseQueue->getFileToParse()); - self::assertSame('file2', $this->parseQueue->getFileToParse()); - self::assertNull($this->parseQueue->getFileToParse()); - } - - public function testAddToParseQueue() : void + public function testAddingFiles() { - $this->documents->expects(self::once()) - ->method('hasDocument') - ->with('file') - ->willReturn(true); - - $this->parseQueue->addToParseQueue('file'); - - self::assertNull($this->parseQueue->getFileToParse()); - } - - protected function setUp() : void - { - $this->documents = $this->createMock(Documents::class); - - $this->parseQueue = new ParseQueue($this->documents); + $parseQueue = new ParseQueue(); + $parseQueue->addFile('file_needs_parsing1', true); + $parseQueue->addFile('file_no_parsing1', false); + + $this->assertTrue($parseQueue->isFileKnownToParseQueue('file_needs_parsing1')); + $this->assertTrue($parseQueue->isFileKnownToParseQueue('file_no_parsing1')); + $this->assertFalse($parseQueue->isFileKnownToParseQueue('other_file')); + + $this->assertTrue($parseQueue->doesFileRequireParsing('file_needs_parsing1')); + $this->assertFalse($parseQueue->doesFileRequireParsing('file_no_parsing1')); + + $this->assertSame(['file_needs_parsing1'], $parseQueue->getAllFilesThatRequireParsing()); } } diff --git a/tests/Builder/ScannerTest.php b/tests/Builder/ScannerTest.php index 6002b0bf..b4205fd6 100644 --- a/tests/Builder/ScannerTest.php +++ b/tests/Builder/ScannerTest.php @@ -4,18 +4,19 @@ namespace Doctrine\Tests\RST\Builder; -use Doctrine\RST\Builder\ParseQueue; +use ArrayIterator; use Doctrine\RST\Builder\Scanner; -use Doctrine\RST\Builder\State; use Doctrine\RST\Meta\MetaEntry; use Doctrine\RST\Meta\Metas; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Component\Finder\Finder; +use Symfony\Component\Finder\SplFileInfo; class ScannerTest extends TestCase { - /** @var ParseQueue|MockObject */ - private $parseQueue; + /** @var Finder|MockObject */ + private $finder; /** @var Metas|MockObject */ private $metas; @@ -23,96 +24,219 @@ class ScannerTest extends TestCase /** @var Scanner */ private $scanner; - public function testScan() : void - { - $this->parseQueue->expects(self::once()) - ->method('getState') - ->with('file') - ->willReturn(null); - - $this->parseQueue->expects(self::once()) - ->method('setState') - ->with('file', State::NO_PARSE); + /** @var SplFileInfo[]|MockObject[]|ArrayIterator */ + private $fileMocks = []; - $metaEntry = $this->createMock(MetaEntry::class); + private $metaEntryMocks = []; - $this->metas->expects(self::once()) + public function testScanWithNoMetas() : void + { + $this->metas->expects($this->any()) ->method('get') - ->with('file') - ->willReturn($metaEntry); - - $this->parseQueue->expects(self::once()) - ->method('addToParseQueue') - ->with('file'); + ->willReturn(null); - $this->scanner->scan('/directory', 'file'); + $this->addFileMockToFinder('file1.rst'); + $this->addFileMockToFinder('file2.rst'); + $this->addFileMockToFinder('file3.rst'); + $this->addFileMockToFinder('subdir/file4.rst'); + $this->addFileMockToFinder('subdir/file5.rst'); + + $parseQueue = $this->scanner->scan(); + $this->assertSame([ + 'file1', + 'file2', + 'file3', + 'subdir/file4', + 'subdir/file5' + ], $parseQueue->getAllFilesThatRequireParsing()); } - public function testScanMetaEntry() : void + public function testScanWithNonFreshMetas() { - $metaEntry = $this->createMock(MetaEntry::class); - - $metaEntry->expects(self::once()) + $file1InfoMock = $this->addFileMockToFinder('file1.rst'); + $file1MetaMock = $this->createMetaEntryMock('file1'); + + // file1.rst was modified 50 seconds ago + $file1InfoMock->method('getCTime')->willReturn(time() - 50); + // but file1 MetaEntry was modified 100 seconds ago (is out of date) + $file1MetaMock->method('getCTime')->willReturn(time() - 100); + // should never be called because the meta is definitely not fresh + $file1MetaMock->expects($this->never())->method('getDepends'); + + $file2InfoMock = $this->addFileMockToFinder('file2.rst'); + $file2MetaMock = $this->createMetaEntryMock('file2'); + + // file2.rst was modified 50 seconds ago + $lastModifiedTime = time() - 50; + $file2InfoMock->method('getCTime')->willReturn($lastModifiedTime); + // and file2 MetaEntry was also 50 seconds ago, fresh + $file2MetaMock->method('getCTime')->willReturn($lastModifiedTime); + // ignore dependencies for this test + $file2MetaMock->expects($this->once()) ->method('getDepends') - ->willReturn(['dependency']); - - $metaEntry->expects(self::once()) - ->method('getParent') - ->willReturn('parent'); - - $this->parseQueue->expects(self::at(0)) - ->method('getState') - ->with('dependency') - ->willReturn(State::PARSE); + ->willReturn([]); - $this->parseQueue->expects(self::at(1)) - ->method('getState') - ->with('dependency') - ->willReturn(State::PARSE); - - $this->parseQueue->expects(self::at(2)) - ->method('addToParseQueue') - ->with('file'); - - $this->scanner->scanMetaEntry($metaEntry, '/directory', 'file'); + $parseQueue = $this->scanner->scan(); + $this->assertSame(['file1'], $parseQueue->getAllFilesThatRequireParsing()); + $this->assertFalse($parseQueue->doesFileRequireParsing('file2')); } - public function testScanMetas() : void + public function testScanWithDependencies() { - $metaEntry = $this->createMock(MetaEntry::class); + /* + * Here is the dependency tree and results: + * * file1 (unmodified) + * depends on: file2 + * * file2 (unmodified) + * depends on: file3, file1 + * * file3 (unmodified) + * depends on: file4, file5, file3, file2 + * * file4 (unmodified) + * depends on: nothing + * * file5 (MODIFIED) + * depends on: file3, file6 + * * file6 (unmodified) + * depends on: file4 + * + * Result is that only file4 and file 6 are fresh + */ + + $metaCTime = time() - 50; + + $file1InfoMock = $this->addFileMockToFinder('file1.rst'); + $file1InfoMock->method('getCTime')->willReturn($metaCTime); + $file1MetaMock = $this->createMetaEntryMock('file1'); + $file1MetaMock->method('getDepends') + ->willReturn(['file2']); + $file1MetaMock->method('getCTime')->willReturn($metaCTime); + + $file2InfoMock = $this->addFileMockToFinder('file2.rst'); + $file2InfoMock->method('getCTime')->willReturn($metaCTime); + $file2MetaMock = $this->createMetaEntryMock('file2'); + $file2MetaMock->method('getDepends') + ->willReturn(['file2', 'file3']); + $file2MetaMock->method('getCTime')->willReturn($metaCTime); + + $file3InfoMock = $this->addFileMockToFinder('file3.rst'); + $file3InfoMock->method('getCTime')->willReturn($metaCTime); + $file3MetaMock = $this->createMetaEntryMock('file3'); + $file3MetaMock->method('getDepends') + ->willReturn(['file4', 'file5', 'file3', 'file2']); + $file3MetaMock->method('getCTime')->willReturn($metaCTime); + + $file4InfoMock = $this->addFileMockToFinder('file4.rst'); + $file4InfoMock->method('getCTime')->willReturn($metaCTime); + $file4MetaMock = $this->createMetaEntryMock('file4'); + $file4MetaMock->method('getDepends') + ->willReturn([]); + $file4MetaMock->method('getCTime')->willReturn($metaCTime); + + $file5InfoMock = $this->addFileMockToFinder('file5.rst'); + // THIS file is the one file that's modified + $file5InfoMock->method('getCTime')->willReturn(time() - 10); + $file5MetaMock = $this->createMetaEntryMock('file5'); + $file5MetaMock->method('getDepends') + ->willReturn(['file3', 'file6']); + $file5MetaMock->method('getCTime')->willReturn($metaCTime); + + $file6InfoMock = $this->addFileMockToFinder('file6.rst'); + $file6InfoMock->method('getCTime')->willReturn($metaCTime); + $file6MetaMock = $this->createMetaEntryMock('file6'); + $file6MetaMock->method('getDepends') + ->willReturn(['file4']); + $file6MetaMock->method('getCTime')->willReturn($metaCTime); + + $parseQueue = $this->scanner->scan(); + $this->assertSame([ + 'file1', + 'file2', + 'file3', + 'file5', + ], $parseQueue->getAllFilesThatRequireParsing()); + } - $metaEntries = ['file' => $metaEntry]; + public function testScanWithNonExistentDependency() + { + /* + * * file1 (unmodified) + * depends on: file2 + * * file2 (does not exist) + * depends on: file3, file1 + * + * Result is that file 1 DOES need to be parsed + */ + + $metaCTime = time() - 50; + + $file1InfoMock = $this->addFileMockToFinder('file1.rst'); + $file1InfoMock->method('getCTime')->willReturn($metaCTime); + $file1MetaMock = $this->createMetaEntryMock('file1'); + $file1MetaMock->method('getDepends') + ->willReturn(['file2']); + $file1MetaMock->method('getCTime')->willReturn($metaCTime); + + // no file info made for file2 + + $parseQueue = $this->scanner->scan(); + $this->assertSame(['file1'], $parseQueue->getAllFilesThatRequireParsing()); + } - $this->metas->expects(self::once()) - ->method('getAll') - ->willReturn($metaEntries); + // test dependency does not exist anymore - $this->parseQueue->expects(self::once()) - ->method('getState') - ->with('file') - ->willReturn(null); + protected function setUp() : void + { + $this->fileMocks = new \ArrayIterator(); + $this->finder = $this->createMock(Finder::class); + $this->finder->expects($this->any()) + ->method('getIterator') + ->willReturn($this->fileMocks); + $this->finder->expects($this->once()) + ->method('in') + ->with('/directory') + ->willReturnSelf(); + $this->finder->expects($this->once()) + ->method('files') + ->with() + ->willReturnSelf(); + $this->finder->expects($this->once()) + ->method('name') + ->with('*.rst') + ->willReturnSelf(); + + $this->metas = $this->createMock(Metas::class); + $this->metas->expects($this->any()) + ->method('get') + ->willReturnCallback(function($filename) { + return $this->metaEntryMocks[$filename] ?? null; + }); - $this->parseQueue->expects(self::once()) - ->method('setState') - ->with('file', State::NO_PARSE); + $this->scanner = new Scanner('rst', '/directory', $this->metas, $this->finder); + } - $this->metas->expects(self::once()) - ->method('get') - ->with('file') - ->willReturn($metaEntry); + /** + * @return MockObject|SplFileInfo + */ + private function addFileMockToFinder(string $relativePath) + { + $fileInfo = $this->createMock(SplFileInfo::class); + $fileInfo->expects($this->any()) + ->method('getRelativePathname') + ->willReturn($relativePath); - $this->parseQueue->expects(self::once()) - ->method('addToParseQueue') - ->with('file'); + $this->fileMocks[$relativePath] = $fileInfo; - $this->scanner->scanMetas('/directory'); + return $fileInfo; } - protected function setUp() : void + /** + * @return MockObject|MetaEntry + */ + private function createMetaEntryMock(string $filename) { - $this->parseQueue = $this->createMock(ParseQueue::class); - $this->metas = $this->createMock(Metas::class); + $meta = $this->createMock(MetaEntry::class); + + $this->metaEntryMocks[$filename] = $meta; - $this->scanner = new Scanner($this->parseQueue, $this->metas); + return $meta; } } From d482a22613dcdd4f42d00cc6e552312bf0e10544 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 18 Jan 2019 15:28:53 -0500 Subject: [PATCH 05/22] Fixing a bug where :ref: would be added as dependencies The problem is that a ref called "some-link" is not a document filename, but it is/was being added into dependencies. The solution is to track which dependencies are unresolved, and resolve them later once we can. --- lib/Environment.php | 18 +++++++++++++- lib/Kernel.php | 2 +- lib/Meta/MetaEntry.php | 36 +++++++++++++++++++++++++++- lib/References/Doc.php | 14 +++++++++-- lib/References/ResolvedReference.php | 11 ++++++++- lib/References/Resolver.php | 6 +++-- tests/Builder/BuilderTest.php | 24 ++++++++++++++++++- 7 files changed, 102 insertions(+), 9 deletions(-) diff --git a/lib/Environment.php b/lib/Environment.php index 2ad23865..f987e4d0 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -58,6 +58,9 @@ class Environment /** @var string[] */ private $dependencies = []; + /** @var string[] */ + private $unresolvedDependencies = []; + /** @var string[] */ private $variables = []; @@ -148,6 +151,16 @@ public function resolve(string $section, string $data) : ?ResolvedReference if ($resolvedReference === null) { $this->addInvalidLink(new InvalidLink($data)); + $this->getMetaEntry()->removeDependency($data); + + return null; + } + + if (in_array($data, $this->unresolvedDependencies, true)) { + $this->getMetaEntry()->resolveDependency( + $data, + $resolvedReference->getFile() + ); } return $resolvedReference; @@ -279,7 +292,7 @@ public function getLink(string $name, bool $relative = true) : string return ''; } - public function addDependency(string $dependency) : void + public function addDependency(string $dependency, bool $requiresResolving = false) : void { $dependency = $this->canonicalUrl($dependency); @@ -291,6 +304,9 @@ public function addDependency(string $dependency) : void } $this->dependencies[] = $dependency; + if ($requiresResolving) { + $this->unresolvedDependencies[] = $dependency; + } } /** diff --git a/lib/Kernel.php b/lib/Kernel.php index 6a18f0d8..38fde6fb 100644 --- a/lib/Kernel.php +++ b/lib/Kernel.php @@ -42,7 +42,7 @@ public function __construct( $this->references = array_merge([ new References\Doc(), - new References\Doc('ref'), + new References\Doc('ref', true), ], $this->createReferences(), $references); } diff --git a/lib/Meta/MetaEntry.php b/lib/Meta/MetaEntry.php index fc12be5f..43963a31 100644 --- a/lib/Meta/MetaEntry.php +++ b/lib/Meta/MetaEntry.php @@ -32,6 +32,9 @@ class MetaEntry /** @var string[] */ private $depends; + /** @var string[] */ + private $resolvedDependencies = []; + /** @var string[] */ private $links; @@ -115,7 +118,38 @@ public function getTocs() : array */ public function getDepends() : array { - return $this->depends; + return array_values(array_unique($this->depends)); + } + + /** + * Call to replace a dependency with the resolved, real filename. + */ + public function resolveDependency(string $originalDependency, string $newDependency) : void + { + // we only need to resolve a dependency one time + if (in_array($originalDependency, $this->resolvedDependencies)) { + return; + } + + $key = array_search($originalDependency, $this->depends); + + if (false === $key) { + throw new \LogicException(sprintf('Could not found dependency "%s" in MetaEntry for "%s"', $originalDependency, $this->file)); + } + + $this->depends[$key] = $newDependency; + $this->resolvedDependencies[] = $originalDependency; + } + + public function removeDependency(string $dependency) : void + { + $key = array_search($dependency, $this->depends); + + if (false === $key) { + throw new \LogicException(sprintf('Could not found dependency "%s" in MetaEntry for "%s"', $dependency, $this->file)); + } + + unset($this->depends[$key]); } /** diff --git a/lib/References/Doc.php b/lib/References/Doc.php index e75f4454..69f07f26 100644 --- a/lib/References/Doc.php +++ b/lib/References/Doc.php @@ -14,10 +14,20 @@ class Doc extends Reference /** @var Resolver */ private $resolver; - public function __construct(string $name = 'doc') + /** + * Used with "ref" - it means the dependencies added in found() + * must be resolved to their final path later (they are not + * already document names). + * + * @var bool + */ + private $dependenciesMustBeResolved; + + public function __construct(string $name = 'doc', $dependenciesMustBeResolved = false) { $this->name = $name; $this->resolver = new Resolver(); + $this->dependenciesMustBeResolved = $dependenciesMustBeResolved; } public function getName() : string @@ -32,6 +42,6 @@ public function resolve(Environment $environment, string $data) : ?ResolvedRefer public function found(Environment $environment, string $data) : void { - $environment->addDependency($data); + $environment->addDependency($data, $this->dependenciesMustBeResolved); } } diff --git a/lib/References/ResolvedReference.php b/lib/References/ResolvedReference.php index 90f7e951..a31f3684 100644 --- a/lib/References/ResolvedReference.php +++ b/lib/References/ResolvedReference.php @@ -11,6 +11,9 @@ class ResolvedReference { + /** @var string */ + private $file; + /** @var string|null */ private $title; @@ -27,8 +30,9 @@ class ResolvedReference * @param string[][]|string[][][] $titles * @param string[] $attributes */ - public function __construct(?string $title, ?string $url, array $titles = [], array $attributes = []) + public function __construct(string $file, ?string $title, ?string $url, array $titles = [], array $attributes = []) { + $this->file = $file; $this->title = $title; $this->url = $url; $this->titles = $titles; @@ -37,6 +41,11 @@ public function __construct(?string $title, ?string $url, array $titles = [], ar $this->attributes = $attributes; } + public function getFile() : string + { + return $this->file; + } + public function getTitle() : ?string { return $this->title; diff --git a/lib/References/Resolver.php b/lib/References/Resolver.php index e790769a..49ddbb4e 100644 --- a/lib/References/Resolver.php +++ b/lib/References/Resolver.php @@ -52,7 +52,7 @@ private function resolveFileReference( return null; } - return $this->createResolvedReference($environment, $entry, $attributes); + return $this->createResolvedReference($file, $environment, $entry, $attributes); } /** @@ -66,7 +66,7 @@ private function resolveAnchorReference( $entry = $environment->getMetas()->findLinkMetaEntry($data); if ($entry !== null) { - return $this->createResolvedReference($environment, $entry, $attributes, $data); + return $this->createResolvedReference($entry->getFile(), $environment, $entry, $attributes, $data); } return null; @@ -76,6 +76,7 @@ private function resolveAnchorReference( * @param string[] $attributes */ private function createResolvedReference( + string $file, Environment $environment, MetaEntry $entry, array $attributes = [], @@ -88,6 +89,7 @@ private function createResolvedReference( } return new ResolvedReference( + $file, $entry->getTitle(), $url, $entry->getTitles(), diff --git a/tests/Builder/BuilderTest.php b/tests/Builder/BuilderTest.php index e19f0762..4c20441c 100644 --- a/tests/Builder/BuilderTest.php +++ b/tests/Builder/BuilderTest.php @@ -5,6 +5,7 @@ namespace Doctrine\Tests\RST\Builder; use Doctrine\RST\Builder; +use Doctrine\RST\Meta\MetaEntry; use Doctrine\Tests\RST\BaseBuilderTest; use function file_exists; use function is_dir; @@ -44,20 +45,41 @@ public function testCachedMetas() : void // check that metas were cached self::assertTrue(file_exists($this->targetFile('metas.php'))); $cachedContents = file_get_contents($this->targetFile('metas.php')); + /** @var MetaEntry[] $metaEntries */ $metaEntries = unserialize($cachedContents); self::assertArrayHasKey('index', $metaEntries); self::assertSame('Summary', $metaEntries['index']->getTitle()); + // look at all the other documents this document depends + // on, like :doc: and :ref: + $this->assertSame([ + 'index', + 'toc-glob', + 'subdir/index' + ], $metaEntries['introduction']->getDepends()); + // update meta cache to see that it was used + // Summary is the main header in "index.rst" + // we reference it in introduction.rst + // it should cause introduction.rst to re-render with the new + // title as the link file_put_contents( $this->targetFile('metas.php'), str_replace('Summary', 'Sumario', $cachedContents) ); + // also we need to trigger the introduction.rst as looking updated + sleep(1); + file_put_contents( + __DIR__.'/input/introduction.rst', + file_get_contents(__DIR__.'/input/introduction.rst') . ' ' + ); + $builder = new Builder(); + $builder->pleaseLog = true; $builder->build($this->sourceFile(), $this->targetFile()); - $contents = $this->getFileContents($this->targetFile('index.html')); + $contents = $this->getFileContents($this->targetFile('introduction.html')); self::assertContains('Sumario', $contents); } From 0033d35b7241ec588b84e1388f524e3dc210a76f Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 18 Jan 2019 15:29:49 -0500 Subject: [PATCH 06/22] These test cases were actually wrong - these are all :ref: --- tests/Builder/input/introduction.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Builder/input/introduction.rst b/tests/Builder/input/introduction.rst index 662806e3..64106b3f 100644 --- a/tests/Builder/input/introduction.rst +++ b/tests/Builder/input/introduction.rst @@ -13,8 +13,8 @@ Reference to the :doc:`Index, paragraph toc ` Reference to the :doc:`Glob TOC ` -Reference to the :doc:`Summary Reference ` +Reference to the :ref:`Summary Reference ` -Reference to the :doc:`Test Reference ` +Reference to the :ref:`Test Reference ` -Reference to the :doc:`Camel Case Reference ` +Reference to the :ref:`Camel Case Reference ` From b49928bd8405e6b3d119ee236b142cf06f10446b Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 18 Jan 2019 15:30:10 -0500 Subject: [PATCH 07/22] Forgotten from earlier - making Parser more explicit --- lib/Parser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Parser.php b/lib/Parser.php index b910157b..4d0ba221 100644 --- a/lib/Parser.php +++ b/lib/Parser.php @@ -43,7 +43,7 @@ class Parser public function __construct( ?Kernel $kernel = null, - ?Environment $environment = null + Environment $environment ) { if ($kernel === null) { $kernel = new Kernel(); @@ -51,7 +51,7 @@ public function __construct( $this->configuration = $kernel->getConfiguration(); $this->kernel = $kernel; - $this->environment = $environment ?: new Environment($this->configuration); + $this->environment = $environment; $this->initDirectives(); $this->initReferences(); From ecc21aa2e5b62ad7fd7ed33bc5180f1cb7e192a4 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 27 Jan 2019 21:37:33 -0500 Subject: [PATCH 08/22] Simplifying dependencies further - they do not need to be recursive --- lib/Builder/Scanner.php | 98 +++++++++++------------------------ tests/Builder/ScannerTest.php | 8 +-- 2 files changed, 34 insertions(+), 72 deletions(-) diff --git a/lib/Builder/Scanner.php b/lib/Builder/Scanner.php index 0b6ebed1..21e6258c 100644 --- a/lib/Builder/Scanner.php +++ b/lib/Builder/Scanner.php @@ -38,7 +38,7 @@ public function __construct(string $fileExtension, string $directory, Metas $met * * This takes into account the presence of cached & fresh MetaEntry * objects, and avoids adding files to the parse queue that have - * not changed and whose dependencies have not changed. + * not changed and whose direct dependencies have not changed. */ public function scan() : ParseQueue { @@ -61,46 +61,9 @@ public function scan() : ParseQueue } return $parseQueue; - - /* - * 1) See if meta exists. If it does not, add it to ParseQueue - * as needing to PARSE_NEEDED - * 2) If meta DOES exist - * a) if stale, PARSE_NEEDED - * b) if not stale, loop over each dependency and - * do the exact same check - * -> how can we avoid circular issues? - * -> basically, each iteration of the loop - * going deeper just needs to know that - * if it hits its parent caller/file, it - * should take no information from this, - * but not try to follow its dependencies. - * -> could possibly do this with a flag called - * PARSE_PROCESSING, which is released after - * you follow all of your dependencies. If - * you hit a PARSE_PROCESSING on one of your - * dependencies, you just exist and re-set - * your status back to some unknown. If you - * ARE able to determine from your dependencies - * if you need to be parsed, then you set to whatever - * that status is. - * -> this probably means adding everything to the - * ParseQueue in the beginning. And maybe this - * just becomes a utility class of the Scanner, - * and an array of filenames or array of SourceDocument - * objects is returned. Maybe Scanner can be more - * stateless, and a lot of the recursive logic is - * moved into this temporary, stateful ParseQueue. - * i) if no dependencies need to be re-parsed, don't parse - * ii) if any need re-parsing, re-parse - * - * We will use ParseQueue in an intelligent way - actually asking - * it if it is aware of a file yet, instead of using all this getState() - * garbage - */ } - private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue, array $filesAlreadyBeingChecked = []) : bool + private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue) : bool { if (!isset($this->fileInfos[$filename])) { throw new \InvalidArgumentException(sprintf('No file info found for "%s" - file does not exist.', $filename)); @@ -111,7 +74,7 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue $documentFilename = $this->getFilenameFromFile($file); $entry = $this->metas->get($documentFilename); - if ($entry === null || $entry->getCtime() < $file->getCTime()) { + if ($this->hasFileBeenUpdated($filename)) { // File is new or changed and thus need to be parsed return true; } @@ -127,34 +90,20 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue $filesAlreadyBeingChecked[] = $documentFilename; foreach ($dependencies as $dependency) { - if (in_array($dependency, $filesAlreadyBeingChecked, true)) { - /* - * File is already being checked. For example, consider - * this dependency tree: - * - * DocA (depends on)-> - * DocB (depends on)-> - * DocC (depends on)-> DocB & DocD - * - * And assume only DocD has changed. - * The method will be called recursively for DocB, then DocC. - * When that happens, it needs to realize that we're already - * checking to see if DocB has changed. And so, we should not - * recursively check DocB again. It's a no-op: we don't know - * if DocB has changed yet or not. So, we skip, and check DocD. - */ - - continue; - } - - // if the parseQueue already knows about this file, just ask it - if ($parseQueue->isFileKnownToParseQueue($dependency)) { - if ($parseQueue->doesFileRequireParsing($dependency)) { - return true; - } - - continue; - } + /* + * The dependency check is NOT recursive on purpose. + * If fileA has a link to fileB that uses its "headline", + * for example, then fileA is "dependent" on fileB. If + * fileB changes, it means that its MetaEntry needs to + * be updated. And because fileA gets the headline from + * the MetaEntry, it means that fileA must also be re-parsed. + * However, if fileB depends on fileC and file C only is + * updated, fileB *does* need to be re-parsed, but fileA + * does not, because the MetaEntry for fileB IS still + * "fresh" - fileB did not actually change, so any metadata + * about headlines, etc, is still fresh. Therefore, fileA + * does not need to be parsed. + */ // dependency no longer exists? We should re-parse this file if (!isset($this->fileInfos[$dependency])) { @@ -162,7 +111,7 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue } // finally, we need to recursively ask if this file needs parsing - if ($this->doesFileRequireParsing($dependency, $parseQueue, $filesAlreadyBeingChecked)) { + if ($this->hasFileBeenUpdated($dependency)) { return true; } } @@ -171,6 +120,17 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue return false; } + private function hasFileBeenUpdated(string $filename): bool + { + $file = $this->fileInfos[$filename]; + + $documentFilename = $this->getFilenameFromFile($file); + $entry = $this->metas->get($documentFilename); + + // File is new or changed + return ($entry === null || $entry->getCtime() < $file->getCTime()); + } + /** * Converts foo/bar.rst to foo/bar (the document filename) */ diff --git a/tests/Builder/ScannerTest.php b/tests/Builder/ScannerTest.php index b4205fd6..3dd95c3a 100644 --- a/tests/Builder/ScannerTest.php +++ b/tests/Builder/ScannerTest.php @@ -98,7 +98,11 @@ public function testScanWithDependencies() * * file6 (unmodified) * depends on: file4 * - * Result is that only file4 and file 6 are fresh + * Result is that the following files are fresh: + * * file1 + * * file2 + * * file4 + * * file6 */ $metaCTime = time() - 50; @@ -148,8 +152,6 @@ public function testScanWithDependencies() $parseQueue = $this->scanner->scan(); $this->assertSame([ - 'file1', - 'file2', 'file3', 'file5', ], $parseQueue->getAllFilesThatRequireParsing()); From 066445a02432f288d24c2d9ebffe6e3865e8a96b Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 27 Jan 2019 22:26:29 -0500 Subject: [PATCH 09/22] Fixing a bug with multiple :ref: not being resolved Before this commit, if there were 2 :ref: to the same spot, because "references" were not stored uniquely on Environment, we only resolved the first, not the second. But, if we simply stored $this->dependencies as a unique array on Environment, we could have a collision between a :doc: and a :ref: with the same name. The hack of prefixing the unresolved dependencies was added to work around this. --- lib/Environment.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/Environment.php b/lib/Environment.php index f987e4d0..3fd5c4a8 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -156,9 +156,9 @@ public function resolve(string $section, string $data) : ?ResolvedReference return null; } - if (in_array($data, $this->unresolvedDependencies, true)) { + if (isset($this->unresolvedDependencies[$data])) { $this->getMetaEntry()->resolveDependency( - $data, + $this->unresolvedDependencies[$data], $resolvedReference->getFile() ); } @@ -303,9 +303,16 @@ public function addDependency(string $dependency, bool $requiresResolving = fals )); } - $this->dependencies[] = $dependency; if ($requiresResolving) { - $this->unresolvedDependencies[] = $dependency; + // a hack to avoid collisions between resolved and unresolved dependencies + $dependencyName = 'UNRESOLVED__'.$dependency; + $this->unresolvedDependencies[$dependency] = $dependencyName; + } else { + $dependencyName = $dependency; + } + + if (!in_array($dependencyName, $this->dependencies)) { + $this->dependencies[] = $dependencyName; } } From 9bb7c9b994d3eed03c9c2a2bcf151d712177a081 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 27 Jan 2019 22:29:32 -0500 Subject: [PATCH 10/22] Fixing bug where :ref: would be made into a URL For example, if we had :ref:`foo` in subdir/index, then it would be changed to `subdir/foo` and we wouldn't be able to resolve the `foo` reference later. --- lib/Environment.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Environment.php b/lib/Environment.php index 3fd5c4a8..64d8edcf 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -294,7 +294,11 @@ public function getLink(string $name, bool $relative = true) : string public function addDependency(string $dependency, bool $requiresResolving = false) : void { - $dependency = $this->canonicalUrl($dependency); + if (!$requiresResolving) { + // the dependency is already a filename, probably a :doc: + // or from a toc-tree - change it to the canonical URL + $dependency = $this->canonicalUrl($dependency); + } if ($dependency === null) { throw new InvalidArgumentException(sprintf( From e100e3950c4879741b9a61e487d52764c8e43e32 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 27 Jan 2019 22:30:25 -0500 Subject: [PATCH 11/22] Finishing test for cached metadata --- tests/Builder/BuilderTest.php | 25 +++++++++++++++++++------ tests/Builder/input/link-to-index.rst | 5 +++++ 2 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 tests/Builder/input/link-to-index.rst diff --git a/tests/Builder/BuilderTest.php b/tests/Builder/BuilderTest.php index 4c20441c..d71c0665 100644 --- a/tests/Builder/BuilderTest.php +++ b/tests/Builder/BuilderTest.php @@ -58,28 +58,41 @@ public function testCachedMetas() : void 'subdir/index' ], $metaEntries['introduction']->getDepends()); + // assert the self-refs don't mess up dependencies + $this->assertSame([ + 'subdir/index', + 'index', + 'subdir/file' + ], $metaEntries['subdir/index']->getDepends()); + // update meta cache to see that it was used // Summary is the main header in "index.rst" - // we reference it in introduction.rst - // it should cause introduction.rst to re-render with the new + // we reference it in link-to-index.rst + // it should cause link-to-index.rst to re-render with the new // title as the link file_put_contents( $this->targetFile('metas.php'), str_replace('Summary', 'Sumario', $cachedContents) ); - // also we need to trigger the introduction.rst as looking updated + // also we need to trigger the link-to-index.rst as looking updated sleep(1); + $contents = file_get_contents(__DIR__.'/input/link-to-index.rst'); file_put_contents( - __DIR__.'/input/introduction.rst', - file_get_contents(__DIR__.'/input/introduction.rst') . ' ' + __DIR__.'/input/link-to-index.rst', + $contents. ' ' + ); + // change it back + file_put_contents( + __DIR__.'/input/link-to-index.rst', + $contents ); $builder = new Builder(); $builder->pleaseLog = true; $builder->build($this->sourceFile(), $this->targetFile()); - $contents = $this->getFileContents($this->targetFile('introduction.html')); + $contents = $this->getFileContents($this->targetFile('link-to-index.html')); self::assertContains('Sumario', $contents); } diff --git a/tests/Builder/input/link-to-index.rst b/tests/Builder/input/link-to-index.rst new file mode 100644 index 00000000..9a17aa76 --- /dev/null +++ b/tests/Builder/input/link-to-index.rst @@ -0,0 +1,5 @@ +Document with a Link to Index +----------------------------- + +I have a reference to :doc:`index` and this link +is dependent on the title of that page. From 2c1b625dc74df227f9cc130e6b286a44c9ccb7a7 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 27 Jan 2019 22:32:42 -0500 Subject: [PATCH 12/22] Making cached metas disable-able --- lib/Builder.php | 4 +++- lib/Configuration.php | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/Builder.php b/lib/Builder.php index 79fccec6..46362e1d 100644 --- a/lib/Builder.php +++ b/lib/Builder.php @@ -111,7 +111,9 @@ public function build( $this->filesystem->mkdir($targetDirectory, 0755); } - $this->loadCachedMetas($targetDirectory); + if ($this->configuration->getUseCachedMetas()) { + $this->loadCachedMetas($targetDirectory); + } $parseQueue = $this->scan($directory, $targetDirectory); diff --git a/lib/Configuration.php b/lib/Configuration.php index 5b3870d1..9ab583f2 100644 --- a/lib/Configuration.php +++ b/lib/Configuration.php @@ -49,6 +49,9 @@ class Configuration /** @var bool */ private $indentHTML = false; + /** @var bool */ + private $useCachedMetas = true; + /** @var string */ private $fileExtension = Format::HTML; @@ -200,6 +203,16 @@ public function getIndentHTML() : bool return $this->indentHTML; } + public function setUseCachedMetas(bool $useCachedMetas) : void + { + $this->useCachedMetas = $useCachedMetas; + } + + public function getUseCachedMetas() : bool + { + return $this->useCachedMetas; + } + public function getFileExtension() : string { return $this->fileExtension; From 784398ee05a4a1a4a19a9852af958ac84db97cd0 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Sun, 27 Jan 2019 22:43:26 -0500 Subject: [PATCH 13/22] Making all the Builders in tests not use caching --- tests/BaseBuilderTest.php | 1 + tests/Builder/BuilderTest.php | 1 + tests/BuilderInvalidReferences/BuilderInvalidReferencesTest.php | 1 + tests/BuilderUrl/BuilderUrlTest.php | 1 + tests/LiteralNestedInDirective/BuilderTest.php | 1 + tests/RefInsideDirective/BuilderTest.php | 1 + 6 files changed, 6 insertions(+) diff --git a/tests/BaseBuilderTest.php b/tests/BaseBuilderTest.php index ea2f7368..2d0860b1 100644 --- a/tests/BaseBuilderTest.php +++ b/tests/BaseBuilderTest.php @@ -22,6 +22,7 @@ protected function setUp() : void shell_exec('rm -rf ' . $this->targetFile()); $this->builder = new Builder(); + $this->builder->getConfiguration()->setUseCachedMetas(false); $this->builder->build($this->sourceFile(), $this->targetFile()); } diff --git a/tests/Builder/BuilderTest.php b/tests/Builder/BuilderTest.php index d71c0665..7d383f11 100644 --- a/tests/Builder/BuilderTest.php +++ b/tests/Builder/BuilderTest.php @@ -88,6 +88,7 @@ public function testCachedMetas() : void $contents ); + // new builder, which will use cached metas $builder = new Builder(); $builder->pleaseLog = true; $builder->build($this->sourceFile(), $this->targetFile()); diff --git a/tests/BuilderInvalidReferences/BuilderInvalidReferencesTest.php b/tests/BuilderInvalidReferences/BuilderInvalidReferencesTest.php index cb1d23f0..269b10c5 100644 --- a/tests/BuilderInvalidReferences/BuilderInvalidReferencesTest.php +++ b/tests/BuilderInvalidReferences/BuilderInvalidReferencesTest.php @@ -18,6 +18,7 @@ class BuilderInvalidReferencesTest extends BaseBuilderTest protected function setUp() : void { $this->configuration = new Configuration(); + $this->configuration->setUseCachedMetas(false); $this->builder = new Builder(new Kernel($this->configuration)); } diff --git a/tests/BuilderUrl/BuilderUrlTest.php b/tests/BuilderUrl/BuilderUrlTest.php index 5713346a..ac29ebc6 100644 --- a/tests/BuilderUrl/BuilderUrlTest.php +++ b/tests/BuilderUrl/BuilderUrlTest.php @@ -127,6 +127,7 @@ public function testRelativeUrl() : void protected function setUp() : void { $this->configuration = new Configuration(); + $this->configuration->setUseCachedMetas(false); $this->builder = new Builder(new Kernel($this->configuration)); } diff --git a/tests/LiteralNestedInDirective/BuilderTest.php b/tests/LiteralNestedInDirective/BuilderTest.php index f3c7a169..e4a27c5f 100644 --- a/tests/LiteralNestedInDirective/BuilderTest.php +++ b/tests/LiteralNestedInDirective/BuilderTest.php @@ -25,6 +25,7 @@ protected function setUp() : void ); $this->builder = new Builder($kernel); + $this->builder->getConfiguration()->setUseCachedMetas(false); $this->builder->build($this->sourceFile(), $this->targetFile()); } diff --git a/tests/RefInsideDirective/BuilderTest.php b/tests/RefInsideDirective/BuilderTest.php index ada48982..600441ce 100644 --- a/tests/RefInsideDirective/BuilderTest.php +++ b/tests/RefInsideDirective/BuilderTest.php @@ -15,6 +15,7 @@ public function testRefInsideDirective() : void { $kernel = new Kernel(null, [new VersionAddedDirective()]); $builder = new Builder($kernel); + $builder->getConfiguration()->setUseCachedMetas(false); $builder->build( __DIR__ . '/input', From 0f786928de6d25b5b42be07df780cc9f79be79fe Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:17:01 -0500 Subject: [PATCH 14/22] Fixing bug where unresolved name was used to remove dependency --- lib/Environment.php | 6 +++++- lib/Meta/MetaEntry.php | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/Environment.php b/lib/Environment.php index 64d8edcf..ae6e5c56 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -151,13 +151,17 @@ public function resolve(string $section, string $data) : ?ResolvedReference if ($resolvedReference === null) { $this->addInvalidLink(new InvalidLink($data)); - $this->getMetaEntry()->removeDependency($data); + $this->getMetaEntry()->removeDependency( + // use the unique, unresolved name + $this->unresolvedDependencies[$data] ?? $data + ); return null; } if (isset($this->unresolvedDependencies[$data])) { $this->getMetaEntry()->resolveDependency( + // use the unique, unresolved name $this->unresolvedDependencies[$data], $resolvedReference->getFile() ); diff --git a/lib/Meta/MetaEntry.php b/lib/Meta/MetaEntry.php index 43963a31..97509e33 100644 --- a/lib/Meta/MetaEntry.php +++ b/lib/Meta/MetaEntry.php @@ -134,7 +134,7 @@ public function resolveDependency(string $originalDependency, string $newDepende $key = array_search($originalDependency, $this->depends); if (false === $key) { - throw new \LogicException(sprintf('Could not found dependency "%s" in MetaEntry for "%s"', $originalDependency, $this->file)); + throw new \LogicException(sprintf('Could not find dependency "%s" in MetaEntry for "%s"', $originalDependency, $this->file)); } $this->depends[$key] = $newDependency; @@ -146,7 +146,7 @@ public function removeDependency(string $dependency) : void $key = array_search($dependency, $this->depends); if (false === $key) { - throw new \LogicException(sprintf('Could not found dependency "%s" in MetaEntry for "%s"', $dependency, $this->file)); + throw new \LogicException(sprintf('Could not find dependency "%s" in MetaEntry for "%s"', $dependency, $this->file)); } unset($this->depends[$key]); From 4419383d2ac867065d448131881cd003ce89b01c Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:18:59 -0500 Subject: [PATCH 15/22] updating test - all files ARE now parsed, even if orphaned --- tests/BuilderToctree/BuilderTocTreeTest.php | 2 +- tests/BuilderToctree/input/{not-parsed => orphaned}/file.rst | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename tests/BuilderToctree/input/{not-parsed => orphaned}/file.rst (100%) diff --git a/tests/BuilderToctree/BuilderTocTreeTest.php b/tests/BuilderToctree/BuilderTocTreeTest.php index a07d3abd..ec556b25 100644 --- a/tests/BuilderToctree/BuilderTocTreeTest.php +++ b/tests/BuilderToctree/BuilderTocTreeTest.php @@ -12,7 +12,7 @@ class BuilderTocTreeTest extends BaseBuilderTest public function testTocTreeGlob() : void { self::assertTrue(file_exists($this->targetFile('subdir/toctree.html'))); - self::assertFalse(file_exists($this->targetFile('not-parsed/file.html'))); + self::assertTrue(file_exists($this->targetFile('orphaned/file.html'))); } public function testMaxDepth() : void diff --git a/tests/BuilderToctree/input/not-parsed/file.rst b/tests/BuilderToctree/input/orphaned/file.rst similarity index 100% rename from tests/BuilderToctree/input/not-parsed/file.rst rename to tests/BuilderToctree/input/orphaned/file.rst From b374797443108879bd1dacec307dc4af7cd9da0f Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:25:00 -0500 Subject: [PATCH 16/22] reverting cleanups that were BC breaks --- lib/Builder/ParseQueueProcessor.php | 17 ++++++------ lib/Environment.php | 41 ++++++++++++++++++++--------- lib/Parser.php | 4 +-- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/lib/Builder/ParseQueueProcessor.php b/lib/Builder/ParseQueueProcessor.php index b8b0b971..94708b1e 100644 --- a/lib/Builder/ParseQueueProcessor.php +++ b/lib/Builder/ParseQueueProcessor.php @@ -91,15 +91,14 @@ private function processFile(string $file) : void private function createFileParser(string $file) : Parser { - $environment = new Environment( - $this->kernel->getConfiguration(), - $file, - $this->metas, - $this->directory, - $this->targetDirectory, - $this->errorManager - ); - $parser = new Parser($this->kernel, $environment); + $parser = new Parser($this->kernel); + + $environment = $parser->getEnvironment(); + $environment->setMetas($this->metas); + $environment->setCurrentFileName($file); + $environment->setCurrentDirectory($this->directory); + $environment->setTargetDirectory($this->targetDirectory); + $environment->setErrorManager($this->errorManager); return $parser; } diff --git a/lib/Environment.php b/lib/Environment.php index ae6e5c56..2e11cbce 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -79,24 +79,14 @@ class Environment /** @var InvalidLink[] */ private $invalidLinks = []; - public function __construct( - Configuration $configuration, - string $currentFileName, - Metas $metas, - string $currentDirectory, - string $targetDirectory, - ErrorManager $errorManager - ) + public function __construct(Configuration $configuration) { $this->configuration = $configuration; - $this->currentFileName = $currentFileName; - $this->metas = $metas; - $this->currentDirectory = $currentDirectory; - $this->targetDirectory = $targetDirectory; - $this->errorManager = $errorManager; + $this->errorManager = new ErrorManager($this->configuration); $this->urlGenerator = new UrlGenerator( $this->configuration ); + $this->metas = new Metas(); $this->reset(); } @@ -124,6 +114,16 @@ public function getErrorManager() : ErrorManager return $this->errorManager; } + public function setErrorManager(ErrorManager $errorManager) : void + { + $this->errorManager = $errorManager; + } + + public function setMetas(Metas $metas) : void + { + $this->metas = $metas; + } + public function getNodeFactory() : NodeFactory { return $this->configuration->getNodeFactory($this); @@ -367,11 +367,21 @@ public function getDirName() : string return $dirname; } + public function setCurrentFileName(string $filename) : void + { + $this->currentFileName = $filename; + } + public function getCurrentFileName() : string { return $this->currentFileName; } + public function setCurrentDirectory(string $directory) : void + { + $this->currentDirectory = $directory; + } + public function getCurrentDirectory() : string { return $this->currentDirectory; @@ -382,6 +392,11 @@ public function absoluteRelativePath(string $url) : string return $this->currentDirectory . '/' . $this->getDirName() . '/' . $this->relativeUrl($url); } + public function setTargetDirectory(string $directory) : void + { + $this->targetDirectory = $directory; + } + public function getTargetDirectory() : string { return $this->targetDirectory; diff --git a/lib/Parser.php b/lib/Parser.php index 4d0ba221..b910157b 100644 --- a/lib/Parser.php +++ b/lib/Parser.php @@ -43,7 +43,7 @@ class Parser public function __construct( ?Kernel $kernel = null, - Environment $environment + ?Environment $environment = null ) { if ($kernel === null) { $kernel = new Kernel(); @@ -51,7 +51,7 @@ public function __construct( $this->configuration = $kernel->getConfiguration(); $this->kernel = $kernel; - $this->environment = $environment; + $this->environment = $environment ?: new Environment($this->configuration); $this->initDirectives(); $this->initReferences(); From 28cd5606b7f0241715c4e28558c0790c1cacff6c Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:39:29 -0500 Subject: [PATCH 17/22] allowing for the current MetaEntry to not be set --- lib/Environment.php | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/lib/Environment.php b/lib/Environment.php index 2e11cbce..aee5b3fa 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -151,20 +151,25 @@ public function resolve(string $section, string $data) : ?ResolvedReference if ($resolvedReference === null) { $this->addInvalidLink(new InvalidLink($data)); - $this->getMetaEntry()->removeDependency( - // use the unique, unresolved name - $this->unresolvedDependencies[$data] ?? $data - ); + + if ($this->getMetaEntry()) { + $this->getMetaEntry()->removeDependency( + // use the unique, unresolved name + $this->unresolvedDependencies[$data] ?? $data + ); + } return null; } if (isset($this->unresolvedDependencies[$data])) { - $this->getMetaEntry()->resolveDependency( - // use the unique, unresolved name - $this->unresolvedDependencies[$data], - $resolvedReference->getFile() - ); + if ($this->getMetaEntry()) { + $this->getMetaEntry()->resolveDependency( + // use the unique, unresolved name + $this->unresolvedDependencies[$data], + $resolvedReference->getFile() + ); + } } return $resolvedReference; From fb5425b2a07e5cdcccf9a7905d6305b40cdebd8a Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:41:40 -0500 Subject: [PATCH 18/22] updating test for new constructor arg of ResolvedReference --- tests/References/ResolvedReferenceTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/References/ResolvedReferenceTest.php b/tests/References/ResolvedReferenceTest.php index 2cccb7cb..5486dcc0 100644 --- a/tests/References/ResolvedReferenceTest.php +++ b/tests/References/ResolvedReferenceTest.php @@ -19,7 +19,7 @@ class ResolvedReferenceTest extends TestCase */ public function testCreateResolvedReference(array $attributes) : void { - $resolvedReference = new ResolvedReference('title', 'url', [['title' => 'title']], $attributes); + $resolvedReference = new ResolvedReference('file', 'title', 'url', [['title' => 'title']], $attributes); self::assertSame('title', $resolvedReference->getTitle()); self::assertSame('url', $resolvedReference->getUrl()); @@ -67,7 +67,7 @@ public function testCreateResolvedReferenceWithAttributesInvalid(array $attribut self::expectException(RuntimeException::class); self::expectExceptionMessage(sprintf('Attribute with name "%s" is not allowed', key($attributes))); - new ResolvedReference('title', 'url', [], $attributes); + new ResolvedReference('file', 'title', 'url', [], $attributes); } /** From 0093f34c67b4f25935e5352bedf8f1ea3128fe0d Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:46:17 -0500 Subject: [PATCH 19/22] fixing tests --- tests/GlobSearcherTest.php | 2 +- tests/References/ResolverTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/GlobSearcherTest.php b/tests/GlobSearcherTest.php index 6887abe3..9194bbdf 100644 --- a/tests/GlobSearcherTest.php +++ b/tests/GlobSearcherTest.php @@ -34,7 +34,7 @@ public function testGlobSearch() : void self::assertCount(3, $files); $expected = [ - '/not-parsed/file', + '/orphaned/file', '/index', '/subdir/toctree', ]; diff --git a/tests/References/ResolverTest.php b/tests/References/ResolverTest.php index b7472a13..03824d32 100644 --- a/tests/References/ResolverTest.php +++ b/tests/References/ResolverTest.php @@ -66,7 +66,7 @@ public function testResolveFileReference() : void ->willReturn('/url'); self::assertEquals( - new ResolvedReference('title', '/url', [], ['attr' => 'value']), + new ResolvedReference('file', 'title', '/url', [], ['attr' => 'value']), $this->resolver->resolve($this->environment, 'url', ['attr' => 'value']) ); } @@ -86,7 +86,7 @@ public function testResolveAnchorReference() : void ->willReturn('/url'); self::assertEquals( - new ResolvedReference('title', '/url#anchor', [], ['attr' => 'value']), + new ResolvedReference('', 'title', '/url#anchor', [], ['attr' => 'value']), $this->resolver->resolve($this->environment, 'anchor', ['attr' => 'value']) ); } From 27979a96746bd938ce406446048fb42ea8385ada Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 31 Jan 2019 21:49:54 -0500 Subject: [PATCH 20/22] fixing cs --- lib/Builder.php | 19 +++++++-- lib/Builder/ParseQueue.php | 16 +++++--- lib/Builder/ParseQueueProcessor.php | 4 -- lib/Builder/Scanner.php | 39 ++++++++++-------- lib/Environment.php | 27 +++++++------ lib/Meta/MetaEntry.php | 28 ++++++++----- lib/Meta/Metas.php | 2 +- lib/References/Doc.php | 6 +-- lib/References/ResolvedReference.php | 6 +-- lib/References/Resolver.php | 2 +- tests/Builder/BuilderTest.php | 24 +++++++----- tests/Builder/ParseQueueProcessorTest.php | 1 - tests/Builder/ParseQueueTest.php | 19 ++++----- tests/Builder/ScannerTest.php | 48 +++++++++++------------ tests/BuilderUrl/BuilderUrlTest.php | 2 +- 15 files changed, 136 insertions(+), 107 deletions(-) diff --git a/lib/Builder.php b/lib/Builder.php index 46362e1d..eae8f605 100644 --- a/lib/Builder.php +++ b/lib/Builder.php @@ -15,7 +15,12 @@ use Doctrine\RST\Event\PreBuildScanEvent; use Doctrine\RST\Meta\Metas; use Symfony\Component\Filesystem\Filesystem; +use function file_exists; +use function file_get_contents; +use function file_put_contents; use function is_dir; +use function serialize; +use function unserialize; class Builder { @@ -151,7 +156,7 @@ private function scan(string $directory, string $targetDirectory) : ParseQueue $this->metas ); - return $scanner->scan($directory, $this->metas); + return $scanner->scan(); } private function parse(string $directory, string $targetDirectory, ParseQueue $parseQueue) : void @@ -195,11 +200,17 @@ private function render(string $directory, string $targetDirectory) : void private function loadCachedMetas(string $targetDirectory) : void { $metaCachePath = $this->getMetaCachePath($targetDirectory); - if (!file_exists($metaCachePath)) { + if (! file_exists($metaCachePath)) { return; } - $this->metas->setMetaEntries(unserialize(file_get_contents($metaCachePath))); + $contents = file_get_contents($metaCachePath); + + if ($contents === false) { + return; + } + + $this->metas->setMetaEntries(unserialize($contents)); } private function saveMetas(string $targetDirectory) : void @@ -209,6 +220,6 @@ private function saveMetas(string $targetDirectory) : void private function getMetaCachePath(string $targetDirectory) : string { - return $targetDirectory.'/metas.php'; + return $targetDirectory . '/metas.php'; } } diff --git a/lib/Builder/ParseQueue.php b/lib/Builder/ParseQueue.php index a3dfb571..a72d788c 100644 --- a/lib/Builder/ParseQueue.php +++ b/lib/Builder/ParseQueue.php @@ -4,6 +4,12 @@ namespace Doctrine\RST\Builder; +use InvalidArgumentException; +use function array_filter; +use function array_key_exists; +use function array_keys; +use function sprintf; + final class ParseQueue { /** @@ -17,9 +23,9 @@ final class ParseQueue public function addFile(string $filename, bool $parseNeeded) : void { if (isset($this->fileStatuses[$filename])) { - throw new \InvalidArgumentException(sprintf('File "%s" is already in the parse queue', $filename)); + throw new InvalidArgumentException(sprintf('File "%s" is already in the parse queue', $filename)); } - + $this->fileStatuses[$filename] = $parseNeeded; } @@ -30,8 +36,8 @@ public function isFileKnownToParseQueue(string $filename) : bool public function doesFileRequireParsing(string $filename) : bool { - if (!$this->isFileKnownToParseQueue($filename)) { - throw new \InvalidArgumentException(sprintf('File "%s" is not known to the parse queue', $filename)); + if (! $this->isFileKnownToParseQueue($filename)) { + throw new InvalidArgumentException(sprintf('File "%s" is not known to the parse queue', $filename)); } return $this->fileStatuses[$filename]; @@ -42,7 +48,7 @@ public function doesFileRequireParsing(string $filename) : bool */ public function getAllFilesThatRequireParsing() : array { - return array_keys(array_filter($this->fileStatuses, function(bool $parseNeeded) { + return array_keys(array_filter($this->fileStatuses, static function (bool $parseNeeded) { return $parseNeeded; })); } diff --git a/lib/Builder/ParseQueueProcessor.php b/lib/Builder/ParseQueueProcessor.php index 94708b1e..cb5adab3 100644 --- a/lib/Builder/ParseQueueProcessor.php +++ b/lib/Builder/ParseQueueProcessor.php @@ -4,15 +4,11 @@ namespace Doctrine\RST\Builder; -use Doctrine\RST\Environment; use Doctrine\RST\ErrorManager; use Doctrine\RST\Kernel; -use Doctrine\RST\Meta\DocumentDependency; use Doctrine\RST\Meta\Metas; use Doctrine\RST\Nodes\DocumentNode; use Doctrine\RST\Parser; -use function array_filter; -use function file_exists; use function filectime; class ParseQueueProcessor diff --git a/lib/Builder/Scanner.php b/lib/Builder/Scanner.php index 21e6258c..df477686 100644 --- a/lib/Builder/Scanner.php +++ b/lib/Builder/Scanner.php @@ -5,32 +5,40 @@ namespace Doctrine\RST\Builder; use Doctrine\RST\Meta\Metas; +use InvalidArgumentException; use Symfony\Component\Finder\Finder; use Symfony\Component\Finder\SplFileInfo; +use function sprintf; +use function strlen; +use function substr; class Scanner { + /** @var string */ private $fileExtension; + /** @var string */ private $directory; + /** @var Metas */ private $metas; + /** @var Finder */ private $finder; /** @var SplFileInfo[] */ private $fileInfos = []; - public function __construct(string $fileExtension, string $directory, Metas $metas, Finder $finder = null) + public function __construct(string $fileExtension, string $directory, Metas $metas, ?Finder $finder = null) { $this->fileExtension = $fileExtension; - $this->directory = $directory; - $this->metas = $metas; + $this->directory = $directory; + $this->metas = $metas; $this->finder = $finder ?: new Finder(); $this->finder->in($this->directory) ->files() - ->name('*.'.$this->fileExtension); + ->name('*.' . $this->fileExtension); } /** @@ -65,14 +73,14 @@ public function scan() : ParseQueue private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue) : bool { - if (!isset($this->fileInfos[$filename])) { - throw new \InvalidArgumentException(sprintf('No file info found for "%s" - file does not exist.', $filename)); + if (! isset($this->fileInfos[$filename])) { + throw new InvalidArgumentException(sprintf('No file info found for "%s" - file does not exist.', $filename)); } $file = $this->fileInfos[$filename]; $documentFilename = $this->getFilenameFromFile($file); - $entry = $this->metas->get($documentFilename); + $entry = $this->metas->get($documentFilename); if ($this->hasFileBeenUpdated($filename)) { // File is new or changed and thus need to be parsed @@ -80,15 +88,12 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue } // Look to the file's dependencies to know if you need to parse it or not - $dependencies = $entry->getDepends(); + $dependencies = $entry !== null ? $entry->getDepends() : []; - $parent = $entry->getParent(); - if ($parent !== null) { - $dependencies[] = $parent; + if ($entry !== null && $entry->getParent() !== null) { + $dependencies[] = $entry->getParent(); } - $filesAlreadyBeingChecked[] = $documentFilename; - foreach ($dependencies as $dependency) { /* * The dependency check is NOT recursive on purpose. @@ -106,7 +111,7 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue */ // dependency no longer exists? We should re-parse this file - if (!isset($this->fileInfos[$dependency])) { + if (! isset($this->fileInfos[$dependency])) { return true; } @@ -120,15 +125,15 @@ private function doesFileRequireParsing(string $filename, ParseQueue $parseQueue return false; } - private function hasFileBeenUpdated(string $filename): bool + private function hasFileBeenUpdated(string $filename) : bool { $file = $this->fileInfos[$filename]; $documentFilename = $this->getFilenameFromFile($file); - $entry = $this->metas->get($documentFilename); + $entry = $this->metas->get($documentFilename); // File is new or changed - return ($entry === null || $entry->getCtime() < $file->getCTime()); + return $entry === null || $entry->getCtime() < $file->getCTime(); } /** diff --git a/lib/Environment.php b/lib/Environment.php index aee5b3fa..937297f5 100644 --- a/lib/Environment.php +++ b/lib/Environment.php @@ -15,6 +15,7 @@ use function dirname; use function iconv; use function implode; +use function in_array; use function preg_replace; use function sprintf; use function strtolower; @@ -152,7 +153,7 @@ public function resolve(string $section, string $data) : ?ResolvedReference if ($resolvedReference === null) { $this->addInvalidLink(new InvalidLink($data)); - if ($this->getMetaEntry()) { + if ($this->getMetaEntry() !== null) { $this->getMetaEntry()->removeDependency( // use the unique, unresolved name $this->unresolvedDependencies[$data] ?? $data @@ -162,14 +163,12 @@ public function resolve(string $section, string $data) : ?ResolvedReference return null; } - if (isset($this->unresolvedDependencies[$data])) { - if ($this->getMetaEntry()) { - $this->getMetaEntry()->resolveDependency( - // use the unique, unresolved name - $this->unresolvedDependencies[$data], - $resolvedReference->getFile() - ); - } + if (isset($this->unresolvedDependencies[$data]) && $this->getMetaEntry() !== null) { + $this->getMetaEntry()->resolveDependency( + // use the unique, unresolved name + $this->unresolvedDependencies[$data], + $resolvedReference->getFile() + ); } return $resolvedReference; @@ -303,7 +302,7 @@ public function getLink(string $name, bool $relative = true) : string public function addDependency(string $dependency, bool $requiresResolving = false) : void { - if (!$requiresResolving) { + if (! $requiresResolving) { // the dependency is already a filename, probably a :doc: // or from a toc-tree - change it to the canonical URL $dependency = $this->canonicalUrl($dependency); @@ -318,15 +317,17 @@ public function addDependency(string $dependency, bool $requiresResolving = fals if ($requiresResolving) { // a hack to avoid collisions between resolved and unresolved dependencies - $dependencyName = 'UNRESOLVED__'.$dependency; + $dependencyName = 'UNRESOLVED__' . $dependency; $this->unresolvedDependencies[$dependency] = $dependencyName; } else { $dependencyName = $dependency; } - if (!in_array($dependencyName, $this->dependencies)) { - $this->dependencies[] = $dependencyName; + if (in_array($dependencyName, $this->dependencies, true)) { + return; } + + $this->dependencies[] = $dependencyName; } /** diff --git a/lib/Meta/MetaEntry.php b/lib/Meta/MetaEntry.php index 97509e33..fa2a83a9 100644 --- a/lib/Meta/MetaEntry.php +++ b/lib/Meta/MetaEntry.php @@ -5,9 +5,15 @@ namespace Doctrine\RST\Meta; use Doctrine\RST\Environment; +use LogicException; use function array_merge; +use function array_search; +use function array_unique; +use function array_values; +use function in_array; use function is_array; use function is_string; +use function sprintf; class MetaEntry { @@ -124,29 +130,33 @@ public function getDepends() : array /** * Call to replace a dependency with the resolved, real filename. */ - public function resolveDependency(string $originalDependency, string $newDependency) : void + public function resolveDependency(string $originalDependency, ?string $newDependency) : void { + if ($newDependency === null) { + return; + } + // we only need to resolve a dependency one time - if (in_array($originalDependency, $this->resolvedDependencies)) { + if (in_array($originalDependency, $this->resolvedDependencies, true)) { return; } - $key = array_search($originalDependency, $this->depends); + $key = array_search($originalDependency, $this->depends, true); - if (false === $key) { - throw new \LogicException(sprintf('Could not find dependency "%s" in MetaEntry for "%s"', $originalDependency, $this->file)); + if ($key === false) { + throw new LogicException(sprintf('Could not find dependency "%s" in MetaEntry for "%s"', $originalDependency, $this->file)); } - $this->depends[$key] = $newDependency; + $this->depends[$key] = $newDependency; $this->resolvedDependencies[] = $originalDependency; } public function removeDependency(string $dependency) : void { - $key = array_search($dependency, $this->depends); + $key = array_search($dependency, $this->depends, true); - if (false === $key) { - throw new \LogicException(sprintf('Could not find dependency "%s" in MetaEntry for "%s"', $dependency, $this->file)); + if ($key === false) { + throw new LogicException(sprintf('Could not find dependency "%s" in MetaEntry for "%s"', $dependency, $this->file)); } unset($this->depends[$key]); diff --git a/lib/Meta/Metas.php b/lib/Meta/Metas.php index f12319af..0626c464 100644 --- a/lib/Meta/Metas.php +++ b/lib/Meta/Metas.php @@ -100,7 +100,7 @@ public function get(string $url) : ?MetaEntry /** * @param MetaEntry[] $metaEntries */ - public function setMetaEntries(array $metaEntries) + public function setMetaEntries(array $metaEntries) : void { $this->entries = $metaEntries; } diff --git a/lib/References/Doc.php b/lib/References/Doc.php index 69f07f26..05f3bbaf 100644 --- a/lib/References/Doc.php +++ b/lib/References/Doc.php @@ -23,10 +23,10 @@ class Doc extends Reference */ private $dependenciesMustBeResolved; - public function __construct(string $name = 'doc', $dependenciesMustBeResolved = false) + public function __construct(string $name = 'doc', bool $dependenciesMustBeResolved = false) { - $this->name = $name; - $this->resolver = new Resolver(); + $this->name = $name; + $this->resolver = new Resolver(); $this->dependenciesMustBeResolved = $dependenciesMustBeResolved; } diff --git a/lib/References/ResolvedReference.php b/lib/References/ResolvedReference.php index a31f3684..09feafc9 100644 --- a/lib/References/ResolvedReference.php +++ b/lib/References/ResolvedReference.php @@ -11,7 +11,7 @@ class ResolvedReference { - /** @var string */ + /** @var ?string */ private $file; /** @var string|null */ @@ -30,7 +30,7 @@ class ResolvedReference * @param string[][]|string[][][] $titles * @param string[] $attributes */ - public function __construct(string $file, ?string $title, ?string $url, array $titles = [], array $attributes = []) + public function __construct(?string $file, ?string $title, ?string $url, array $titles = [], array $attributes = []) { $this->file = $file; $this->title = $title; @@ -41,7 +41,7 @@ public function __construct(string $file, ?string $title, ?string $url, array $t $this->attributes = $attributes; } - public function getFile() : string + public function getFile() : ?string { return $this->file; } diff --git a/lib/References/Resolver.php b/lib/References/Resolver.php index 49ddbb4e..b568a39c 100644 --- a/lib/References/Resolver.php +++ b/lib/References/Resolver.php @@ -76,7 +76,7 @@ private function resolveAnchorReference( * @param string[] $attributes */ private function createResolvedReference( - string $file, + ?string $file, Environment $environment, MetaEntry $entry, array $attributes = [], diff --git a/tests/Builder/BuilderTest.php b/tests/Builder/BuilderTest.php index 7d383f11..5ea312b3 100644 --- a/tests/Builder/BuilderTest.php +++ b/tests/Builder/BuilderTest.php @@ -8,10 +8,15 @@ use Doctrine\RST\Meta\MetaEntry; use Doctrine\Tests\RST\BaseBuilderTest; use function file_exists; +use function file_get_contents; +use function file_put_contents; use function is_dir; use function range; +use function sleep; use function sprintf; +use function str_replace; use function substr_count; +use function unserialize; /** * Unit testing for RST @@ -44,7 +49,7 @@ public function testCachedMetas() : void { // check that metas were cached self::assertTrue(file_exists($this->targetFile('metas.php'))); - $cachedContents = file_get_contents($this->targetFile('metas.php')); + $cachedContents = (string) file_get_contents($this->targetFile('metas.php')); /** @var MetaEntry[] $metaEntries */ $metaEntries = unserialize($cachedContents); self::assertArrayHasKey('index', $metaEntries); @@ -52,17 +57,17 @@ public function testCachedMetas() : void // look at all the other documents this document depends // on, like :doc: and :ref: - $this->assertSame([ + self::assertSame([ 'index', 'toc-glob', - 'subdir/index' + 'subdir/index', ], $metaEntries['introduction']->getDepends()); // assert the self-refs don't mess up dependencies - $this->assertSame([ + self::assertSame([ 'subdir/index', 'index', - 'subdir/file' + 'subdir/file', ], $metaEntries['subdir/index']->getDepends()); // update meta cache to see that it was used @@ -77,20 +82,19 @@ public function testCachedMetas() : void // also we need to trigger the link-to-index.rst as looking updated sleep(1); - $contents = file_get_contents(__DIR__.'/input/link-to-index.rst'); + $contents = file_get_contents(__DIR__ . '/input/link-to-index.rst'); file_put_contents( - __DIR__.'/input/link-to-index.rst', - $contents. ' ' + __DIR__ . '/input/link-to-index.rst', + $contents . ' ' ); // change it back file_put_contents( - __DIR__.'/input/link-to-index.rst', + __DIR__ . '/input/link-to-index.rst', $contents ); // new builder, which will use cached metas $builder = new Builder(); - $builder->pleaseLog = true; $builder->build($this->sourceFile(), $this->targetFile()); $contents = $this->getFileContents($this->targetFile('link-to-index.html')); diff --git a/tests/Builder/ParseQueueProcessorTest.php b/tests/Builder/ParseQueueProcessorTest.php index 7bfb236f..f8a79013 100644 --- a/tests/Builder/ParseQueueProcessorTest.php +++ b/tests/Builder/ParseQueueProcessorTest.php @@ -7,7 +7,6 @@ use Doctrine\RST\Builder\Documents; use Doctrine\RST\Builder\ParseQueue; use Doctrine\RST\Builder\ParseQueueProcessor; -use Doctrine\RST\Builder\Scanner; use Doctrine\RST\ErrorManager; use Doctrine\RST\Kernel; use Doctrine\RST\Meta\Metas; diff --git a/tests/Builder/ParseQueueTest.php b/tests/Builder/ParseQueueTest.php index e361ba61..3b3263a6 100644 --- a/tests/Builder/ParseQueueTest.php +++ b/tests/Builder/ParseQueueTest.php @@ -4,27 +4,24 @@ namespace Doctrine\Tests\RST\Builder; -use Doctrine\RST\Builder\Documents; use Doctrine\RST\Builder\ParseQueue; -use Doctrine\RST\Builder\State; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; class ParseQueueTest extends TestCase { - public function testAddingFiles() + public function testAddingFiles() : void { $parseQueue = new ParseQueue(); $parseQueue->addFile('file_needs_parsing1', true); $parseQueue->addFile('file_no_parsing1', false); - - $this->assertTrue($parseQueue->isFileKnownToParseQueue('file_needs_parsing1')); - $this->assertTrue($parseQueue->isFileKnownToParseQueue('file_no_parsing1')); - $this->assertFalse($parseQueue->isFileKnownToParseQueue('other_file')); - $this->assertTrue($parseQueue->doesFileRequireParsing('file_needs_parsing1')); - $this->assertFalse($parseQueue->doesFileRequireParsing('file_no_parsing1')); + self::assertTrue($parseQueue->isFileKnownToParseQueue('file_needs_parsing1')); + self::assertTrue($parseQueue->isFileKnownToParseQueue('file_no_parsing1')); + self::assertFalse($parseQueue->isFileKnownToParseQueue('other_file')); - $this->assertSame(['file_needs_parsing1'], $parseQueue->getAllFilesThatRequireParsing()); + self::assertTrue($parseQueue->doesFileRequireParsing('file_needs_parsing1')); + self::assertFalse($parseQueue->doesFileRequireParsing('file_no_parsing1')); + + self::assertSame(['file_needs_parsing1'], $parseQueue->getAllFilesThatRequireParsing()); } } diff --git a/tests/Builder/ScannerTest.php b/tests/Builder/ScannerTest.php index 3dd95c3a..e5d88a3e 100644 --- a/tests/Builder/ScannerTest.php +++ b/tests/Builder/ScannerTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Finder\Finder; use Symfony\Component\Finder\SplFileInfo; +use function time; class ScannerTest extends TestCase { @@ -25,13 +26,14 @@ class ScannerTest extends TestCase private $scanner; /** @var SplFileInfo[]|MockObject[]|ArrayIterator */ - private $fileMocks = []; + private $fileMocks; + /** @var MetaEntry[]|MockObject[] */ private $metaEntryMocks = []; public function testScanWithNoMetas() : void { - $this->metas->expects($this->any()) + $this->metas->expects(self::any()) ->method('get') ->willReturn(null); @@ -42,16 +44,16 @@ public function testScanWithNoMetas() : void $this->addFileMockToFinder('subdir/file5.rst'); $parseQueue = $this->scanner->scan(); - $this->assertSame([ + self::assertSame([ 'file1', 'file2', 'file3', 'subdir/file4', - 'subdir/file5' + 'subdir/file5', ], $parseQueue->getAllFilesThatRequireParsing()); } - public function testScanWithNonFreshMetas() + public function testScanWithNonFreshMetas() : void { $file1InfoMock = $this->addFileMockToFinder('file1.rst'); $file1MetaMock = $this->createMetaEntryMock('file1'); @@ -61,7 +63,7 @@ public function testScanWithNonFreshMetas() // but file1 MetaEntry was modified 100 seconds ago (is out of date) $file1MetaMock->method('getCTime')->willReturn(time() - 100); // should never be called because the meta is definitely not fresh - $file1MetaMock->expects($this->never())->method('getDepends'); + $file1MetaMock->expects(self::never())->method('getDepends'); $file2InfoMock = $this->addFileMockToFinder('file2.rst'); $file2MetaMock = $this->createMetaEntryMock('file2'); @@ -72,16 +74,16 @@ public function testScanWithNonFreshMetas() // and file2 MetaEntry was also 50 seconds ago, fresh $file2MetaMock->method('getCTime')->willReturn($lastModifiedTime); // ignore dependencies for this test - $file2MetaMock->expects($this->once()) + $file2MetaMock->expects(self::once()) ->method('getDepends') ->willReturn([]); $parseQueue = $this->scanner->scan(); - $this->assertSame(['file1'], $parseQueue->getAllFilesThatRequireParsing()); - $this->assertFalse($parseQueue->doesFileRequireParsing('file2')); + self::assertSame(['file1'], $parseQueue->getAllFilesThatRequireParsing()); + self::assertFalse($parseQueue->doesFileRequireParsing('file2')); } - public function testScanWithDependencies() + public function testScanWithDependencies() : void { /* * Here is the dependency tree and results: @@ -151,13 +153,13 @@ public function testScanWithDependencies() $file6MetaMock->method('getCTime')->willReturn($metaCTime); $parseQueue = $this->scanner->scan(); - $this->assertSame([ + self::assertSame([ 'file3', 'file5', ], $parseQueue->getAllFilesThatRequireParsing()); } - public function testScanWithNonExistentDependency() + public function testScanWithNonExistentDependency() : void { /* * * file1 (unmodified) @@ -180,35 +182,33 @@ public function testScanWithNonExistentDependency() // no file info made for file2 $parseQueue = $this->scanner->scan(); - $this->assertSame(['file1'], $parseQueue->getAllFilesThatRequireParsing()); + self::assertSame(['file1'], $parseQueue->getAllFilesThatRequireParsing()); } - // test dependency does not exist anymore - protected function setUp() : void { - $this->fileMocks = new \ArrayIterator(); - $this->finder = $this->createMock(Finder::class); - $this->finder->expects($this->any()) + $this->fileMocks = new ArrayIterator(); + $this->finder = $this->createMock(Finder::class); + $this->finder->expects(self::any()) ->method('getIterator') ->willReturn($this->fileMocks); - $this->finder->expects($this->once()) + $this->finder->expects(self::once()) ->method('in') ->with('/directory') ->willReturnSelf(); - $this->finder->expects($this->once()) + $this->finder->expects(self::once()) ->method('files') ->with() ->willReturnSelf(); - $this->finder->expects($this->once()) + $this->finder->expects(self::once()) ->method('name') ->with('*.rst') ->willReturnSelf(); $this->metas = $this->createMock(Metas::class); - $this->metas->expects($this->any()) + $this->metas->expects(self::any()) ->method('get') - ->willReturnCallback(function($filename) { + ->willReturnCallback(function ($filename) { return $this->metaEntryMocks[$filename] ?? null; }); @@ -221,7 +221,7 @@ protected function setUp() : void private function addFileMockToFinder(string $relativePath) { $fileInfo = $this->createMock(SplFileInfo::class); - $fileInfo->expects($this->any()) + $fileInfo->expects(self::any()) ->method('getRelativePathname') ->willReturn($relativePath); diff --git a/tests/BuilderUrl/BuilderUrlTest.php b/tests/BuilderUrl/BuilderUrlTest.php index ac29ebc6..1fed78d6 100644 --- a/tests/BuilderUrl/BuilderUrlTest.php +++ b/tests/BuilderUrl/BuilderUrlTest.php @@ -128,7 +128,7 @@ protected function setUp() : void { $this->configuration = new Configuration(); $this->configuration->setUseCachedMetas(false); - $this->builder = new Builder(new Kernel($this->configuration)); + $this->builder = new Builder(new Kernel($this->configuration)); } protected function getFixturesDirectory() : string From 2c3d13a052a31072dddfa6311f0b47f93f4a297a Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 1 Feb 2019 21:37:17 -0500 Subject: [PATCH 21/22] removing array_unique --- lib/Meta/MetaEntry.php | 4 +--- tests/Builder/BuilderTest.php | 6 ++++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Meta/MetaEntry.php b/lib/Meta/MetaEntry.php index fa2a83a9..807acd8d 100644 --- a/lib/Meta/MetaEntry.php +++ b/lib/Meta/MetaEntry.php @@ -8,8 +8,6 @@ use LogicException; use function array_merge; use function array_search; -use function array_unique; -use function array_values; use function in_array; use function is_array; use function is_string; @@ -124,7 +122,7 @@ public function getTocs() : array */ public function getDepends() : array { - return array_values(array_unique($this->depends)); + return $this->depends; } /** diff --git a/tests/Builder/BuilderTest.php b/tests/Builder/BuilderTest.php index 5ea312b3..9e59f4d5 100644 --- a/tests/Builder/BuilderTest.php +++ b/tests/Builder/BuilderTest.php @@ -7,6 +7,8 @@ use Doctrine\RST\Builder; use Doctrine\RST\Meta\MetaEntry; use Doctrine\Tests\RST\BaseBuilderTest; +use function array_unique; +use function array_values; use function file_exists; use function file_get_contents; use function file_put_contents; @@ -61,14 +63,14 @@ public function testCachedMetas() : void 'index', 'toc-glob', 'subdir/index', - ], $metaEntries['introduction']->getDepends()); + ], array_values(array_unique($metaEntries['introduction']->getDepends()))); // assert the self-refs don't mess up dependencies self::assertSame([ 'subdir/index', 'index', 'subdir/file', - ], $metaEntries['subdir/index']->getDepends()); + ], array_values(array_unique($metaEntries['subdir/index']->getDepends()))); // update meta cache to see that it was used // Summary is the main header in "index.rst" From a128be2211ddaae57c8102621107e318851483c0 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Fri, 1 Feb 2019 21:49:31 -0500 Subject: [PATCH 22/22] Splitting cached meta saving/loader into separate class --- lib/Builder.php | 41 ++++++--------------------- lib/Meta/CachedMetasLoader.php | 42 ++++++++++++++++++++++++++++ tests/Meta/CachedMetasLoaderTest.php | 29 +++++++++++++++++++ 3 files changed, 79 insertions(+), 33 deletions(-) create mode 100644 lib/Meta/CachedMetasLoader.php create mode 100644 tests/Meta/CachedMetasLoaderTest.php diff --git a/lib/Builder.php b/lib/Builder.php index eae8f605..e53379ed 100644 --- a/lib/Builder.php +++ b/lib/Builder.php @@ -13,14 +13,10 @@ use Doctrine\RST\Event\PreBuildParseEvent; use Doctrine\RST\Event\PreBuildRenderEvent; use Doctrine\RST\Event\PreBuildScanEvent; +use Doctrine\RST\Meta\CachedMetasLoader; use Doctrine\RST\Meta\Metas; use Symfony\Component\Filesystem\Filesystem; -use function file_exists; -use function file_get_contents; -use function file_put_contents; use function is_dir; -use function serialize; -use function unserialize; class Builder { @@ -39,6 +35,9 @@ class Builder /** @var Metas */ private $metas; + /** @var CachedMetasLoader */ + private $cachedMetasLoader; + /** @var Documents */ private $documents; @@ -60,6 +59,8 @@ public function __construct(?Kernel $kernel = null) $this->metas = new Metas(); + $this->cachedMetasLoader = new CachedMetasLoader(); + $this->documents = new Builder\Documents( $this->filesystem, $this->metas @@ -117,7 +118,7 @@ public function build( } if ($this->configuration->getUseCachedMetas()) { - $this->loadCachedMetas($targetDirectory); + $this->cachedMetasLoader->loadCachedMetaEntries($targetDirectory, $this->metas); } $parseQueue = $this->scan($directory, $targetDirectory); @@ -126,7 +127,7 @@ public function build( $this->render($directory, $targetDirectory); - $this->saveMetas($targetDirectory); + $this->cachedMetasLoader->cacheMetaEntries($targetDirectory, $this->metas); } public function copy(string $source, ?string $destination = null) : self @@ -196,30 +197,4 @@ private function render(string $directory, string $targetDirectory) : void new PostBuildRenderEvent($this, $directory, $targetDirectory) ); } - - private function loadCachedMetas(string $targetDirectory) : void - { - $metaCachePath = $this->getMetaCachePath($targetDirectory); - if (! file_exists($metaCachePath)) { - return; - } - - $contents = file_get_contents($metaCachePath); - - if ($contents === false) { - return; - } - - $this->metas->setMetaEntries(unserialize($contents)); - } - - private function saveMetas(string $targetDirectory) : void - { - file_put_contents($this->getMetaCachePath($targetDirectory), serialize($this->metas->getAll())); - } - - private function getMetaCachePath(string $targetDirectory) : string - { - return $targetDirectory . '/metas.php'; - } } diff --git a/lib/Meta/CachedMetasLoader.php b/lib/Meta/CachedMetasLoader.php new file mode 100644 index 00000000..6c157d6f --- /dev/null +++ b/lib/Meta/CachedMetasLoader.php @@ -0,0 +1,42 @@ +getMetaCachePath($targetDirectory); + if (! file_exists($metaCachePath)) { + return; + } + + $contents = file_get_contents($metaCachePath); + + if ($contents === false) { + throw new LogicException(sprintf('Could not load file "%s"', $contents)); + } + + $metas->setMetaEntries(unserialize($contents)); + } + + public function cacheMetaEntries(string $targetDirectory, Metas $metas) : void + { + file_put_contents($this->getMetaCachePath($targetDirectory), serialize($metas->getAll())); + } + + private function getMetaCachePath(string $targetDirectory) : string + { + return $targetDirectory . '/metas.php'; + } +} diff --git a/tests/Meta/CachedMetasLoaderTest.php b/tests/Meta/CachedMetasLoaderTest.php new file mode 100644 index 00000000..3e126b0a --- /dev/null +++ b/tests/Meta/CachedMetasLoaderTest.php @@ -0,0 +1,29 @@ +cacheMetaEntries($targetDir, $metasBefore); + $loader->loadCachedMetaEntries($targetDir, $metasAfter); + self::assertEquals([$meta1, $meta2], $metasAfter->getAll()); + } +}