From fdda71848f65f7e710cf200dbff67ff248cd869c Mon Sep 17 00:00:00 2001 From: lacatoire Date: Fri, 17 Apr 2026 21:05:58 +0200 Subject: [PATCH] Skip asset copy when destination already has the file When an RST project references the same image from multiple documents, AssetsExtension::copyAsset hits the Flysystem Local adapter on every invocation. On second and subsequent copies the adapter re-enters ensureDirectory() and, on Flysystem v1, logs a noisy 'Impossible to create the root directory ... mkdir(): File exist' error even though the HTML output itself is fine. Short-circuit the copy when the destination filesystem already reports the target path: one stat per asset reference, no redundant write, no noisy mkdir attempt on repeated renders. A new AssetsExtension unit test covers the skip path, the first-copy path, and idempotency across two consecutive asset() calls. refs https://github.com/phpDocumentor/phpDocumentor/issues/3635 --- packages/guides/src/Twig/AssetsExtension.php | 7 +- .../tests/unit/Twig/AssetsExtensionTest.php | 127 ++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 packages/guides/tests/unit/Twig/AssetsExtensionTest.php diff --git a/packages/guides/src/Twig/AssetsExtension.php b/packages/guides/src/Twig/AssetsExtension.php index 87da94d81..100d578fc 100644 --- a/packages/guides/src/Twig/AssetsExtension.php +++ b/packages/guides/src/Twig/AssetsExtension.php @@ -178,6 +178,11 @@ private function copyAsset( return $outputPath; } + $destination = $renderContext->getDestination(); + if ($destination->has($outputPath)) { + return $outputPath; + } + $fileContents = $renderContext->getOrigin()->read($normalizedSourcePath); if ($fileContents === false) { $this->logger->error( @@ -188,7 +193,7 @@ private function copyAsset( return $outputPath; } - $result = $renderContext->getDestination()->put($outputPath, $fileContents); + $result = $destination->put($outputPath, $fileContents); if ($result === false) { $this->logger->error( sprintf('Unable to write file "%s"', $outputPath), diff --git a/packages/guides/tests/unit/Twig/AssetsExtensionTest.php b/packages/guides/tests/unit/Twig/AssetsExtensionTest.php new file mode 100644 index 000000000..bcd089806 --- /dev/null +++ b/packages/guides/tests/unit/Twig/AssetsExtensionTest.php @@ -0,0 +1,127 @@ +documentNameResolver = self::createMock(DocumentNameResolverInterface::class); + $this->urlGenerator = self::createMock(UrlGeneratorInterface::class); + $this->logger = self::createMock(LoggerInterface::class); + $this->extension = new AssetsExtension( + $this->logger, + self::createMock(NodeRenderer::class), + $this->documentNameResolver, + $this->urlGenerator, + ); + } + + private function stubRenderContext(): RenderContext&MockObject + { + $renderContext = self::createMock(RenderContext::class); + $renderContext->method('getDirName')->willReturn('docs/cli'); + $renderContext->method('getDestinationPath')->willReturn('/guides'); + $this->documentNameResolver->method('canonicalUrl') + ->willReturn('docs/cli/first-docs.png'); + $this->documentNameResolver->method('absoluteUrl') + ->willReturn('/guides/docs/cli/first-docs.png'); + $this->urlGenerator->method('generateInternalUrl') + ->willReturn('docs/cli/first-docs.png'); + + return $renderContext; + } + + /** @link https://github.com/phpDocumentor/phpDocumentor/issues/3635 */ + public function testAssetSkipsCopyWhenDestinationAlreadyHasTheFile(): void + { + $renderContext = $this->stubRenderContext(); + $origin = self::createMock(FilesystemInterface::class); + $destination = self::createMock(FilesystemInterface::class); + $renderContext->method('getOrigin')->willReturn($origin); + $renderContext->method('getDestination')->willReturn($destination); + + $origin->method('has')->with('docs/cli/first-docs.png')->willReturn(true); + $destination->method('has')->with('/guides/docs/cli/first-docs.png')->willReturn(true); + + $origin->expects(self::never())->method('read'); + $destination->expects(self::never())->method('put'); + $this->logger->expects(self::never())->method('error'); + + $this->extension->asset(['env' => $renderContext], 'first-docs.png'); + } + + public function testAssetCopiesWhenDestinationDoesNotHaveTheFile(): void + { + $renderContext = $this->stubRenderContext(); + $origin = self::createMock(FilesystemInterface::class); + $destination = self::createMock(FilesystemInterface::class); + $renderContext->method('getOrigin')->willReturn($origin); + $renderContext->method('getDestination')->willReturn($destination); + + $origin->method('has')->with('docs/cli/first-docs.png')->willReturn(true); + $origin->method('read')->with('docs/cli/first-docs.png')->willReturn('PNG BYTES'); + $destination->method('has')->with('/guides/docs/cli/first-docs.png')->willReturn(false); + + $destination->expects(self::once()) + ->method('put') + ->with('/guides/docs/cli/first-docs.png', 'PNG BYTES') + ->willReturn(true); + $this->logger->expects(self::never())->method('error'); + + $this->extension->asset(['env' => $renderContext], 'first-docs.png'); + } + + /** @link https://github.com/phpDocumentor/phpDocumentor/issues/3635 */ + public function testAssetIsIdempotentAcrossConsecutiveCalls(): void + { + $renderContext = $this->stubRenderContext(); + $origin = self::createMock(FilesystemInterface::class); + $destination = self::createMock(FilesystemInterface::class); + $renderContext->method('getOrigin')->willReturn($origin); + $renderContext->method('getDestination')->willReturn($destination); + + $origin->method('has')->with('docs/cli/first-docs.png')->willReturn(true); + $origin->expects(self::once()) + ->method('read') + ->with('docs/cli/first-docs.png') + ->willReturn('PNG BYTES'); + $destination->method('has') + ->with('/guides/docs/cli/first-docs.png') + ->willReturnOnConsecutiveCalls(false, true); + + $destination->expects(self::once()) + ->method('put') + ->with('/guides/docs/cli/first-docs.png', 'PNG BYTES') + ->willReturn(true); + $this->logger->expects(self::never())->method('error'); + + $this->extension->asset(['env' => $renderContext], 'first-docs.png'); + $this->extension->asset(['env' => $renderContext], 'first-docs.png'); + } +}