diff --git a/packages/pulp/CHANGELOG.md b/packages/pulp/CHANGELOG.md index 1a9090c..fbb9eaf 100644 --- a/packages/pulp/CHANGELOG.md +++ b/packages/pulp/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to `mapsight/pulp` are documented here. ## Unreleased +### Fixed + +- Remove the temp file created by `srcHttp` `sink => true` when the last path-backed `File` is released, including clones. A failed request removes it immediately. A caller-supplied sink path is left in place. + ## 1.2.0 - 2026-09-02 ### Added diff --git a/packages/pulp/Pulp.php b/packages/pulp/Pulp.php index 0c2dbe1..cf34264 100644 --- a/packages/pulp/Pulp.php +++ b/packages/pulp/Pulp.php @@ -60,7 +60,9 @@ public static function srcFile( /** * ## Options * * `sink`: `true` writes the body to a temp file and emits a path-backed - * `File` (use this for large responses). A string path writes there. + * `File` (use this for large responses). The temp file is removed when + * the last `File` that shares it is released, including clones. A string + * path writes there and is left in place. * (default: `null` — load the body as a string) * * `client`: Guzzle `Client` instance. Tests inject a mock handler here. * (default: `new Client()`) diff --git a/packages/pulp/README.md b/packages/pulp/README.md index a54aba1..a78fec2 100644 --- a/packages/pulp/README.md +++ b/packages/pulp/README.md @@ -112,7 +112,7 @@ Pulp::start() ->run(); ``` -- `sink => true` writes the body to a temp file and emits a path-backed `File`, so a ~100 MB response does not have to live as a PHP string. Pass a path string to choose the file. +- `sink => true` writes the body to a temp file and emits a path-backed `File`, so a ~100 MB response does not have to live as a PHP string. The temp file is removed when the last `File` that shares it is released, including clones from `split`, `shadow`, and `results`. Pass a path string to keep the body at a path you manage. - `client` injects a Guzzle `Client` (tests use a mock handler). - `successStatuses` throws unless the status is in the list. Pair with Guzzle `http_errors => false` when 304/204 are first-class. - The file also carries `httpStatus`, `httpLastModified`, `httpEtag`, and `httpType`. diff --git a/packages/pulp/src/File.php b/packages/pulp/src/File.php index 378ca55..6e3b03b 100644 --- a/packages/pulp/src/File.php +++ b/packages/pulp/src/File.php @@ -18,6 +18,11 @@ class File private array $___data = []; private ?string $___contentPath = null; + /** + * Shared by clones. The path is removed when the last File releases it. + */ + private ?SharedTempPath $___tempPath = null; + /** * File constructor. */ @@ -26,7 +31,11 @@ public function __construct(public $fileName, ?string $srcFileName = null) $this->srcFileName = empty($srcFileName) ? $this->fileName : $srcFileName; } - public static function fromPath(string $path, ?string $fileName = null): File + /** + * @param bool $unlinkOnDestruct Remove `$path` when the last File that shares + * it is released, including clones. Caller-supplied paths stay. + */ + public static function fromPath(string $path, ?string $fileName = null, bool $unlinkOnDestruct = false): File { $file = new File( $fileName ?: $path, @@ -38,6 +47,9 @@ public static function fromPath(string $path, ?string $fileName = null): File } $file->___contentPath = $path; + if ($unlinkOnDestruct) { + $file->___tempPath = new SharedTempPath($path); + } $stats = @stat($path); if ($stats !== false) { diff --git a/packages/pulp/src/SharedTempPath.php b/packages/pulp/src/SharedTempPath.php new file mode 100644 index 0000000..63507f3 --- /dev/null +++ b/packages/pulp/src/SharedTempPath.php @@ -0,0 +1,20 @@ +path); + } +} diff --git a/packages/pulp/src/SrcHttpHandler.php b/packages/pulp/src/SrcHttpHandler.php index 3f9012a..07ab810 100644 --- a/packages/pulp/src/SrcHttpHandler.php +++ b/packages/pulp/src/SrcHttpHandler.php @@ -10,6 +10,9 @@ class SrcHttpHandler extends AbstractHandler { + /** Temp file created for `sink => true`. Null once a File owns it, or after it is removed. */ + private ?string $ephemeralSinkPath = null; + protected function getConstructorParamDefs(): array { return ['method', 'uri', 'guzzleOptions', 'aliasFileName', 'options']; @@ -39,7 +42,13 @@ public function onEnd(): void $this->assertSuccessStatus($statusCode); if ($sinkPath !== null) { - $tmpFile = File::fromPath($sinkPath, $this->cp->aliasFileName); + $tmpFile = File::fromPath( + $sinkPath, + $this->cp->aliasFileName, + $this->ephemeralSinkPath !== null, + ); + // File removes this on release. A caller-supplied path is not ephemeral. + $this->ephemeralSinkPath = null; } else { $tmpFile = new File($this->cp->aliasFileName); $tmpFile->content = (string) $res->getBody(); @@ -64,6 +73,8 @@ public function onEnd(): void } else { throw $err; } + } finally { + $this->discardEphemeralSink(); } if ($file instanceof File) { @@ -91,6 +102,8 @@ private function resolveSinkPath(): ?string throw new RuntimeException('Unable to create HTTP sink temp file'); } + $this->ephemeralSinkPath = $path; + return $path; } if (is_string($sink) && $sink !== '') { @@ -102,6 +115,17 @@ private function resolveSinkPath(): ?string return is_string($guzzleSink) && $guzzleSink !== '' ? $guzzleSink : null; } + private function discardEphemeralSink(): void + { + if ($this->ephemeralSinkPath === null) { + return; + } + + $path = $this->ephemeralSinkPath; + $this->ephemeralSinkPath = null; + @unlink($path); + } + private function assertSuccessStatus(int $statusCode): void { $allowed = $this->cp->options['successStatuses'] ?? null; diff --git a/packages/pulp/test/FileTest.php b/packages/pulp/test/FileTest.php index 6f2b55c..525eb67 100644 --- a/packages/pulp/test/FileTest.php +++ b/packages/pulp/test/FileTest.php @@ -77,6 +77,39 @@ public function testStreamReadsFromPathWithoutTouchingContent(): void } } + public function testFromPathCanUnlinkWhenReleased(): void + { + $tmpFile = tempnam(sys_get_temp_dir(), 'pulp-file-test-'); + $this->assertIsString($tmpFile); + file_put_contents($tmpFile, 'owned'); + + $f = File::fromPath($tmpFile, 'file.txt', true); + $this->assertSame('owned', $f->content); + $this->assertFileExists($tmpFile); + + unset($f); + + $this->assertFileDoesNotExist($tmpFile); + } + + public function testOwnedPathSurvivesUntilTheLastCloneIsReleased(): void + { + $tmpFile = tempnam(sys_get_temp_dir(), 'pulp-file-test-'); + $this->assertIsString($tmpFile); + file_put_contents($tmpFile, 'owned'); + + $original = File::fromPath($tmpFile, 'file.txt', true); + $clone = clone $original; + + unset($original); + $this->assertFileExists($tmpFile); + $this->assertSame('owned', $clone->content); + + unset($clone); + + $this->assertFileDoesNotExist($tmpFile); + } + public function testStreamFromGeneratedContent(): void { $f = new File('file.txt'); diff --git a/packages/pulp/test/SrcHttpHandlerTest.php b/packages/pulp/test/SrcHttpHandlerTest.php index 7bfaef7..c4d3eb4 100644 --- a/packages/pulp/test/SrcHttpHandlerTest.php +++ b/packages/pulp/test/SrcHttpHandlerTest.php @@ -60,6 +60,124 @@ public function testSinkWritesPathBackedFile(): void } } + public function testEphemeralSinkStaysUntilTheFileIsReleased(): void + { + $result = Pulp::start() + ->pipe(Pulp::srcHttp( + 'GET', + 'https://example.test/large.json', + [], + 'large.json', + [ + 'client' => $this->client(new Response(200, [], '{"sites":true}')), + 'sink' => true, + ] + )) + ->run(); + + $path = $result[0]->srcFileName; + $this->assertIsString($path); + $this->assertMatchesRegularExpression('#/pulp-http-[^/]+$#', $path); + $this->assertFileExists($path); + $this->assertSame('{"sites":true}', $result[0]->content); + $this->assertFileExists($path); + + unset($result); + gc_collect_cycles(); + + $this->assertFileDoesNotExist($path); + } + + public function testEphemeralSinkSurvivesClonesUntilTheLastFileIsReleased(): void + { + $kept = []; + $result = Pulp::start() + ->pipe(Pulp::srcHttp( + 'GET', + 'https://example.test/large.json', + [], + 'large.json', + [ + 'client' => $this->client(new Response(200, [], '{"sites":true}')), + 'sink' => true, + ] + )) + ->pipe(Pulp::results(static function (array $files) use (&$kept): void { + $kept = $files; + })) + ->run(); + + $path = $result[0]->srcFileName; + $this->assertIsString($path); + $this->assertFileExists($path); + + unset($result); + gc_collect_cycles(); + + $this->assertFileExists($path); + $this->assertSame('{"sites":true}', $kept[0]->content); + $this->assertFileExists($path); + + unset($kept); + gc_collect_cycles(); + + $this->assertFileDoesNotExist($path); + } + + public function testEphemeralSinkIsRemovedWhenTheRequestFails(): void + { + $before = $this->ephemeralSinks(); + + try { + Pulp::start() + ->pipe(Pulp::srcHttp( + 'GET', + 'https://example.test/status', + ['http_errors' => false], + 'status.json', + [ + 'client' => $this->client(new Response(500, [], 'nope')), + 'sink' => true, + 'successStatuses' => [200], + ] + )) + ->run(); + $this->fail('Expected the failed response to throw'); + } catch (RuntimeException $err) { + $this->assertStringContainsString('returned 500', $err->getMessage()); + } + + $this->assertSame($before, $this->ephemeralSinks()); + } + + public function testCallerSinkPathIsKeptWhenTheRequestFails(): void + { + $sink = tempnam(sys_get_temp_dir(), 'pulp-http-sink-'); + $this->assertIsString($sink); + + try { + Pulp::start() + ->pipe(Pulp::srcHttp( + 'GET', + 'https://example.test/status', + ['http_errors' => false], + 'status.json', + [ + 'client' => $this->client(new Response(500, [], 'nope')), + 'sink' => $sink, + 'successStatuses' => [200], + ] + )) + ->run(); + $this->fail('Expected the failed response to throw'); + } catch (RuntimeException $err) { + $this->assertStringContainsString('returned 500', $err->getMessage()); + $this->assertFileExists($sink); + } finally { + @unlink($sink); + } + } + public function testSuccessStatusesRejectUnexpectedCodes(): void { $this->expectException(RuntimeException::class); @@ -101,6 +219,16 @@ public function testNotModifiedStillEmitsFileWithStatus(): void $this->assertSame('', $result[0]->content); } + /** + * @return list + */ + private function ephemeralSinks(): array + { + $paths = glob(sys_get_temp_dir() . '/pulp-http-*'); + + return is_array($paths) ? $paths : []; + } + private function client(Response $response): Client { return new Client([