From 99ce5399418e13719cd3de70c83aac90e4b3544e Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Sat, 28 Mar 2026 10:20:43 +0600 Subject: [PATCH] fix(Workspace): purge/remove transport files using package workspace paths Resolve transport zip and unpacked directory paths from the package workspace (aligned with modTransportPackage transport layout), with fallback to core packages when no workspace row exists. Share filesystem cleanup via TransportPackageFilesystemTrait; add PHPUnit coverage for path resolution and purge/remove flows. Resolves modxcms/revolution#16929 --- .../PackagesPurgeRemoveProcessorsTest.php | 284 ++++++++++++++++++ .../Support/PurgeProcessorTestDouble.php | 35 +++ .../Support/RemoveProcessorTestDouble.php | 49 +++ ...ransportPackageFilesystemPathTestProxy.php | 40 +++ .../TransportPackageFilesystemTraitTest.php | 157 ++++++++++ _build/test/phpunit.xml | 1 + .../Processors/Workspace/Packages/Purge.php | 107 ++++--- .../Processors/Workspace/Packages/Remove.php | 74 ++--- .../TransportPackageFilesystemTrait.php | 95 ++++++ 9 files changed, 752 insertions(+), 90 deletions(-) create mode 100644 _build/test/Tests/Processors/Workspace/Packages/PackagesPurgeRemoveProcessorsTest.php create mode 100644 _build/test/Tests/Processors/Workspace/Packages/Support/PurgeProcessorTestDouble.php create mode 100644 _build/test/Tests/Processors/Workspace/Packages/Support/RemoveProcessorTestDouble.php create mode 100644 _build/test/Tests/Processors/Workspace/Packages/Support/TransportPackageFilesystemPathTestProxy.php create mode 100644 _build/test/Tests/Processors/Workspace/Packages/TransportPackageFilesystemTraitTest.php create mode 100644 core/src/Revolution/Processors/Workspace/Packages/TransportPackageFilesystemTrait.php diff --git a/_build/test/Tests/Processors/Workspace/Packages/PackagesPurgeRemoveProcessorsTest.php b/_build/test/Tests/Processors/Workspace/Packages/PackagesPurgeRemoveProcessorsTest.php new file mode 100644 index 00000000000..147831a32d9 --- /dev/null +++ b/_build/test/Tests/Processors/Workspace/Packages/PackagesPurgeRemoveProcessorsTest.php @@ -0,0 +1,284 @@ + */ + private array $tempRoots = []; + + protected function tearDown(): void + { + foreach ($this->tempRoots as $root) { + $this->deleteDirectory($root); + } + $this->tempRoots = []; + parent::tearDown(); + } + + private function deleteDirectory(string $path): void + { + if (!is_dir($path)) { + return; + } + $path = rtrim($path, '/'); + foreach (glob($path . '/*') ?: [] as $item) { + is_dir($item) ? $this->deleteDirectory($item) : @unlink($item); + } + @rmdir($path); + } + + /** + * Creates a workspace directory layout and a mocked package pointing at it. + * + * @return array{0: modTransportPackage, 1: string, 2: string} package, zip path, unpacked dir path + */ + private function makeWorkspaceWithPackageFiles( + bool $withZip, + bool $withDir, + string $zipFileName = 'demo-1.0.0-pl.transport.zip' + ): array { + $base = sys_get_temp_dir() . '/modx_pkg_ws_' . uniqid('', true); + $workspacePath = $base . '/ws/'; + $packagesDir = $workspacePath . 'packages'; + mkdir($packagesDir, 0777, true); + $zipPath = $packagesDir . '/' . $zipFileName; + $dirPath = $packagesDir . '/' . basename($zipFileName, '.transport.zip') . '/'; + if ($withZip) { + touch($zipPath); + } + if ($withDir) { + mkdir($dirPath, 0777, true); + } + $this->tempRoots[] = $base; + + $workspace = $this->createMock(modWorkspace::class); + $workspace->method('get')->with('path')->willReturn($workspacePath); + + $signature = basename($zipFileName, '.transport.zip'); + $package = $this->createMock(modTransportPackage::class); + $package->method('getOne')->with('Workspace')->willReturn($workspace); + $package->method('get')->willReturnCallback(static function ($key) use ($zipFileName, $signature) { + if ($key === 'source') { + return $zipFileName; + } + if ($key === 'signature') { + return $signature; + } + + return null; + }); + $package->method('getPrimaryKey')->willReturn($signature); + + return [$package, $zipPath, $dirPath]; + } + + private function createModxShell(?modError $error = null): modX + { + $modx = $this->createMock(modX::class); + $modx->method('lexicon')->willReturnCallback(static fn ($key) => $key); + $modx->method('log')->willReturn(null); + if ($error !== null) { + $modx->error = $error; + } + + return $modx; + } + + // --- Remove::initialize --- + + public function testRemoveInitializeFailsWhenSignatureMissing(): void + { + $modx = $this->createModxShell($this->createMock(modError::class)); + $remove = new Remove($modx, ['signature' => '']); + + $this->assertSame('package_err_ns', $remove->initialize()); + } + + public function testRemoveInitializeFailsWhenPackageNotFound(): void + { + $modx = $this->createModxShell($this->createMock(modError::class)); + $modx->expects($this->once()) + ->method('getObject') + ->with(modTransportPackage::class, 'missing') + ->willReturn(null); + $remove = new Remove($modx, ['signature' => 'missing']); + + $this->assertSame('package_err_nf', $remove->initialize()); + } + + public function testRemoveInitializeLoadsPackageAndNormalizesForceTrueString(): void + { + $modx = $this->createModxShell($this->createMock(modError::class)); + $pkg = $this->createMock(modTransportPackage::class); + $modx->method('getObject')->willReturn($pkg); + $remove = new Remove($modx, ['signature' => 'demo-1.0.0-pl', 'force' => 'true']); + + $this->assertTrue($remove->initialize()); + $this->assertSame($pkg, $remove->package); + $this->assertTrue($remove->getProperty('force')); + } + + // --- Remove::process --- + + public function testRemoveProcessReturnsFailureWhenBothRemovalsFailWithZipAndDirPresent(): void + { + [$pkg, ,] = $this->makeWorkspaceWithPackageFiles(true, true); + $pkg->expects($this->once())->method('removePackage')->with(false)->willReturn(false); + $pkg->expects($this->once())->method('remove')->willReturn(false); + + $error = $this->createMock(modError::class); + $error->expects($this->once())->method('failure')->willReturn(['success' => false, 'failed' => true]); + + $modx = $this->createModxShell($error); + $modx->expects($this->never())->method('invokeEvent'); + + $remove = new RemoveProcessorTestDouble($modx, ['signature' => 'x', 'force' => false]); + $remove->package = $pkg; + + $result = $remove->process(); + + $this->assertIsArray($result); + $this->assertFalse($result['success']); + $this->assertSame([], $remove->filesystemOperations); + } + + public function testRemoveProcessSucceedsWhenRemovePackageFailsButRemoveSucceeds(): void + { + [$pkg, $zipPath, $dirPath] = $this->makeWorkspaceWithPackageFiles(true, true); + $pkg->expects($this->once())->method('removePackage')->with(false)->willReturn(false); + $pkg->expects($this->once())->method('remove')->willReturn(true); + + $error = $this->createMock(modError::class); + $error->method('success')->willReturn(['success' => true]); + + $modx = $this->createModxShell($error); + $modx->expects($this->once())->method('invokeEvent')->with( + 'OnPackageRemove', + $this->callback(static function (array $payload) use ($pkg) { + return isset($payload['package']) && $payload['package'] === $pkg; + }) + ); + + $remove = new RemoveProcessorTestDouble($modx, ['signature' => 'x', 'force' => false]); + $remove->package = $pkg; + + $result = $remove->process(); + + $this->assertIsArray($result); + $this->assertTrue($result['success']); + $this->assertSame( + [['zip', $zipPath], ['dir', $dirPath]], + $remove->filesystemOperations + ); + } + + public function testRemoveProcessSucceedsWhenZipAndDirMissingAndRemoveSucceeds(): void + { + [$pkg, $zipPath, $dirPath] = $this->makeWorkspaceWithPackageFiles(false, false); + $pkg->expects($this->never())->method('removePackage'); + $pkg->expects($this->once())->method('remove')->willReturn(true); + + $error = $this->createMock(modError::class); + $error->method('success')->willReturn(['success' => true]); + + $modx = $this->createModxShell($error); + $modx->expects($this->once())->method('invokeEvent'); + + $remove = new RemoveProcessorTestDouble($modx, ['signature' => 'x', 'force' => false]); + $remove->package = $pkg; + + $remove->process(); + + $this->assertSame( + [['zip', $zipPath], ['dir', $dirPath]], + $remove->filesystemOperations + ); + } + + // --- Purge::removePackage --- + + public function testPurgeRemovePackageSkipsFilesystemWhenZipDirPresentAndBothDatabaseRemovalsFail(): void + { + [$pkg, ,] = $this->makeWorkspaceWithPackageFiles(true, true); + $pkg->expects($this->once())->method('removePackage')->with(true)->willReturn(false); + $pkg->expects($this->once())->method('remove')->willReturn(false); + + $modx = $this->createModxShell($this->createMock(modError::class)); + $modx->expects($this->never())->method('invokeEvent'); + + $purge = new PurgeProcessorTestDouble($modx, []); + $purge->removePackage($pkg); + + $this->assertSame([], $purge->filesystemOperations); + } + + public function testPurgeRemovePackageRunsFilesystemCleanupWhenOnlyRemovePackageFailsThenRemoveSucceeds(): void + { + [$pkg, $zipPath, $dirPath] = $this->makeWorkspaceWithPackageFiles(true, true); + $pkg->expects($this->once())->method('removePackage')->with(true)->willReturn(false); + $pkg->expects($this->once())->method('remove')->willReturn(true); + + $modx = $this->createModxShell($this->createMock(modError::class)); + $modx->expects($this->once())->method('invokeEvent')->with( + 'OnPackageRemove', + $this->callback(static function (array $payload) use ($pkg) { + return isset($payload['package']) && $payload['package'] === $pkg; + }) + ); + + $purge = new PurgeProcessorTestDouble($modx, []); + $purge->removePackage($pkg); + + $this->assertSame( + [['zip', $zipPath], ['dir', $dirPath]], + $purge->filesystemOperations + ); + } + + public function testPurgeRemovePackageRunsFilesystemCleanupWhenZipAndDirMissing(): void + { + [$pkg, $zipPath, $dirPath] = $this->makeWorkspaceWithPackageFiles(false, false); + $pkg->expects($this->never())->method('removePackage'); + $pkg->expects($this->once())->method('remove')->willReturn(true); + + $modx = $this->createModxShell($this->createMock(modError::class)); + $modx->expects($this->once())->method('invokeEvent'); + + $purge = new PurgeProcessorTestDouble($modx, []); + $purge->removePackage($pkg); + + $this->assertSame( + [['zip', $zipPath], ['dir', $dirPath]], + $purge->filesystemOperations + ); + } +} diff --git a/_build/test/Tests/Processors/Workspace/Packages/Support/PurgeProcessorTestDouble.php b/_build/test/Tests/Processors/Workspace/Packages/Support/PurgeProcessorTestDouble.php new file mode 100644 index 00000000000..c43a6e85150 --- /dev/null +++ b/_build/test/Tests/Processors/Workspace/Packages/Support/PurgeProcessorTestDouble.php @@ -0,0 +1,35 @@ +filesystemOperations[] = ['zip', $transportZip]; + } + + public function removeTransportDirectory(string $transportDir): void + { + $this->filesystemOperations[] = ['dir', $transportDir]; + } +} diff --git a/_build/test/Tests/Processors/Workspace/Packages/Support/RemoveProcessorTestDouble.php b/_build/test/Tests/Processors/Workspace/Packages/Support/RemoveProcessorTestDouble.php new file mode 100644 index 00000000000..66108431132 --- /dev/null +++ b/_build/test/Tests/Processors/Workspace/Packages/Support/RemoveProcessorTestDouble.php @@ -0,0 +1,49 @@ +filesystemOperations[] = ['zip', $transportZip]; + } + + public function removeTransportDirectory(string $transportDir): void + { + $this->filesystemOperations[] = ['dir', $transportDir]; + } + + public function clearCache(): void + { + /* Parent calls sleep(2); skipped for fast unit tests. */ + } + + public function cleanup(): array + { + $this->modx->invokeEvent('OnPackageRemove', [ + 'package' => $this->package, + ]); + + return $this->success(); + } +} diff --git a/_build/test/Tests/Processors/Workspace/Packages/Support/TransportPackageFilesystemPathTestProxy.php b/_build/test/Tests/Processors/Workspace/Packages/Support/TransportPackageFilesystemPathTestProxy.php new file mode 100644 index 00000000000..25d9c3fdd45 --- /dev/null +++ b/_build/test/Tests/Processors/Workspace/Packages/Support/TransportPackageFilesystemPathTestProxy.php @@ -0,0 +1,40 @@ +resolveTransportPaths($package); + } +} diff --git a/_build/test/Tests/Processors/Workspace/Packages/TransportPackageFilesystemTraitTest.php b/_build/test/Tests/Processors/Workspace/Packages/TransportPackageFilesystemTraitTest.php new file mode 100644 index 00000000000..c9ad2413256 --- /dev/null +++ b/_build/test/Tests/Processors/Workspace/Packages/TransportPackageFilesystemTraitTest.php @@ -0,0 +1,157 @@ +createMock(modX::class); + if ($corePathForFallback !== null) { + $modx->method('getOption')->with('core_path', null, '')->willReturn($corePathForFallback); + } + + return $modx; + } + + /** + * @param string|null $source value from package get('source'); null means omit from callback (simulate empty) + * @param string $signature package signature when source is empty + */ + private function mockPackage( + ?modWorkspace $workspace, + ?string $source, + string $signature = 'foo-1.0.0-pl' + ): modTransportPackage { + $package = $this->createMock(modTransportPackage::class); + $package->method('getOne')->with('Workspace')->willReturn($workspace); + $package->method('get')->willReturnCallback(function ($key) use ($source, $signature) { + if ($key === 'source') { + return $source; + } + if ($key === 'signature') { + return $signature; + } + + return null; + }); + + return $package; + } + + private function mockWorkspace(string $path): modWorkspace + { + $workspace = $this->createMock(modWorkspace::class); + $workspace->method('get')->with('path')->willReturn($path); + + return $workspace; + } + + public function testResolvePathsUsesWorkspaceBaseAndNormalizesTrailingSlash(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx()); + $ws = $this->mockWorkspace('/var/modx/ws/'); + $pkg = $this->mockPackage($ws, 'bar-2.0.0-pl.transport.zip'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/var/modx/ws/packages/bar-2.0.0-pl.transport.zip', $paths['transportZip']); + $this->assertSame('/var/modx/ws/packages/bar-2.0.0-pl/', $paths['transportDir']); + } + + public function testResolvePathsAddsPackagesSegmentWhenWorkspacePathHasNoTrailingSlash(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx()); + $ws = $this->mockWorkspace('/var/modx/ws'); + $pkg = $this->mockPackage($ws, 'baz-1.0.0-pl.transport.zip'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/var/modx/ws/packages/baz-1.0.0-pl.transport.zip', $paths['transportZip']); + $this->assertSame('/var/modx/ws/packages/baz-1.0.0-pl/', $paths['transportDir']); + } + + public function testResolvePathsUsesBasenameWhenSourceContainsSubdirectory(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx()); + $ws = $this->mockWorkspace('/ws/'); + $pkg = $this->mockPackage($ws, 'nested/dir/pkg-1.0.0-pl.transport.zip'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/ws/packages/pkg-1.0.0-pl.transport.zip', $paths['transportZip']); + $this->assertSame('/ws/packages/pkg-1.0.0-pl/', $paths['transportDir']); + } + + public function testResolvePathsDerivesZipNameFromSignatureWhenSourceEmpty(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx()); + $ws = $this->mockWorkspace('/ws/'); + $pkg = $this->mockPackage($ws, '', 'myext-3.1.0-pl'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/ws/packages/myext-3.1.0-pl.transport.zip', $paths['transportZip']); + $this->assertSame('/ws/packages/myext-3.1.0-pl/', $paths['transportDir']); + } + + public function testResolvePathsFallsBackToCorePathWhenNoWorkspace(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx('/test/core')); + $pkg = $this->mockPackage(null, 'orphan-1.0.0-pl.transport.zip'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/test/core/packages/orphan-1.0.0-pl.transport.zip', $paths['transportZip']); + $this->assertSame('/test/core/packages/orphan-1.0.0-pl/', $paths['transportDir']); + } + + public function testResolvePathsCorePathFallbackNormalizesTrailingSlash(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx('/test/core/')); + $pkg = $this->mockPackage(null, 'x-1.0.0-pl.transport.zip'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/test/core/packages/x-1.0.0-pl.transport.zip', $paths['transportZip']); + } + + public function testResolvePathsNullSourceUsesSignature(): void + { + $proxy = new TransportPackageFilesystemPathTestProxy($this->mockModx()); + $ws = $this->mockWorkspace('/ws/'); + $pkg = $this->mockPackage($ws, null, 'sig-1.0.0-pl'); + + $paths = $proxy->resolvePaths($pkg); + + $this->assertSame('/ws/packages/sig-1.0.0-pl.transport.zip', $paths['transportZip']); + $this->assertSame('/ws/packages/sig-1.0.0-pl/', $paths['transportDir']); + } +} diff --git a/_build/test/phpunit.xml b/_build/test/phpunit.xml index 1eaade10d49..7a7cb29133e 100644 --- a/_build/test/phpunit.xml +++ b/_build/test/phpunit.xml @@ -45,6 +45,7 @@ Tests/Processors/Context Tests/Processors/Element Tests/Processors/Resource + Tests/Processors/Workspace Tests/Transport diff --git a/core/src/Revolution/Processors/Workspace/Packages/Purge.php b/core/src/Revolution/Processors/Workspace/Packages/Purge.php index 02d3ca4159b..62143add823 100644 --- a/core/src/Revolution/Processors/Workspace/Packages/Purge.php +++ b/core/src/Revolution/Processors/Workspace/Packages/Purge.php @@ -1,4 +1,5 @@ modx->log(xPDO::LOG_LEVEL_INFO, - $this->modx->lexicon('packages_purge_info_gpurge', ['signature' => $package->signature])); - - $transportZip = $this->modx->getOption('core_path') . 'packages/' . $package->signature . '.transport.zip'; - $transportDir = $this->modx->getOption('core_path') . 'packages/' . $package->signature . '/'; - if (file_exists($transportZip) && file_exists($transportDir)) { - /* remove transport package */ - if ($package->remove() === false) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, - $this->modx->lexicon('package_err_remove', ['signature' => $package->getPrimaryKey()])); - $this->failure($this->modx->lexicon('package_err_remove', ['signature' => $package->getPrimaryKey()])); - return; - } - } else { - /* for some reason the files were removed, so just remove the DB object instead */ - $package->remove(); + $this->modx->log( + xPDO::LOG_LEVEL_INFO, + $this->modx->lexicon('packages_purge_info_gpurge', ['signature' => $package->get('signature')]) + ); + + $paths = $this->resolveTransportPaths($package); + $transportZip = $paths['transportZip']; + $transportDir = $paths['transportDir']; + + $zipExists = file_exists($transportZip); + $dirExists = is_dir($transportDir); + + $result = $this->attemptRemovePackageFromDatabase($package, $zipExists, $dirExists); + + if ($result['skipFilesystemCleanup']) { + return; } $this->removeTransportZip($transportZip); @@ -143,37 +150,45 @@ public function removePackage($package) } /** - * Remove the transport package archive - * @param string $transportZip - * @return void + * When both zip and unpacked dir exist: removePackage(true), then remove() if needed; if both fail, + * skip disk cleanup (legacy purge behaviour). + * + * When either artifact is missing: only remove(). If that fails, disk cleanup still runs so orphan + * transport files under resolved paths are removed even though the DB row may remain. + * + * @return array{skipFilesystemCleanup: bool} */ - public function removeTransportZip($transportZip) - { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tzip_start')); - if (!file_exists($transportZip)) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tzip_nf')); - } else if (!@unlink($transportZip)) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tzip')); - } else { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tzip')); + private function attemptRemovePackageFromDatabase( + modTransportPackage $package, + bool $zipExists, + bool $dirExists + ): array { + if ($zipExists && $dirExists) { + if ($package->removePackage(true) !== false) { + return ['skipFilesystemCleanup' => false]; + } + $this->logPackageRemoveError($package); + if ($package->remove() !== false) { + return ['skipFilesystemCleanup' => false]; + } + + return ['skipFilesystemCleanup' => true]; } + + if ($package->remove() !== false) { + return ['skipFilesystemCleanup' => false]; + } + $this->logPackageRemoveError($package); + + return ['skipFilesystemCleanup' => false]; } - /** - * Remove the transport package directory - * @param string $transportDir - * @return void - */ - public function removeTransportDirectory($transportDir) + private function logPackageRemoveError(modTransportPackage $package): void { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tdir_start')); - if (!file_exists($transportDir)) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tdir_nf')); - } else if (!$this->modx->cacheManager->deleteTree($transportDir, true, false, [])) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tdir')); - } else { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tdir')); - } + $this->modx->log( + xPDO::LOG_LEVEL_ERROR, + $this->modx->lexicon('package_err_remove', ['signature' => $package->getPrimaryKey()]) + ); } /** diff --git a/core/src/Revolution/Processors/Workspace/Packages/Remove.php b/core/src/Revolution/Processors/Workspace/Packages/Remove.php index b2919170429..e54ba8656d9 100644 --- a/core/src/Revolution/Processors/Workspace/Packages/Remove.php +++ b/core/src/Revolution/Processors/Workspace/Packages/Remove.php @@ -1,4 +1,5 @@ modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_gpack')); - $transportZip = $this->modx->getOption('core_path') . 'packages/' . $this->package->signature . '.transport.zip'; - $transportDir = $this->modx->getOption('core_path') . 'packages/' . $this->package->signature . '/'; - if (file_exists($transportZip) && file_exists($transportDir)) { - /* remove transport package */ + $paths = $this->resolveTransportPaths($this->package); + $transportZip = $paths['transportZip']; + $transportDir = $paths['transportDir']; + + $zipExists = file_exists($transportZip); + $dirExists = is_dir($transportDir); + + $removeFailed = false; + if ($zipExists && $dirExists) { if ($this->package->removePackage($this->getProperty('force')) === false) { $packageSignature = $this->package->getPrimaryKey(); - $this->modx->log(xPDO::LOG_LEVEL_ERROR, - $this->modx->lexicon('package_err_remove', ['signature' => $packageSignature])); - return $this->failure($this->modx->lexicon('package_err_remove', ['signature' => $packageSignature])); + $this->modx->log( + xPDO::LOG_LEVEL_ERROR, + $this->modx->lexicon('package_err_remove', ['signature' => $packageSignature]) + ); + if ($this->package->remove() === false) { + $removeFailed = true; + } } - } else { - /* for some reason the files were removed, so just remove the DB object instead */ - $this->package->remove(); + } elseif ($this->package->remove() === false) { + $removeFailed = true; + } + + if ($removeFailed) { + return $this->failure($this->modx->lexicon('package_err_remove', [ + 'signature' => $this->package->getPrimaryKey(), + ])); } $this->clearCache(); @@ -110,40 +130,6 @@ public function clearCache() sleep(2); } - /** - * Remove the transport package archive - * @param string $transportZip - * @return void - */ - public function removeTransportZip($transportZip) - { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tzip_start')); - if (!file_exists($transportZip)) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tzip_nf')); - } else if (!@unlink($transportZip)) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tzip')); - } else { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tzip')); - } - } - - /** - * Remove the transport package directory - * @param string $transportDir - * @return void - */ - public function removeTransportDirectory($transportDir) - { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tdir_start')); - if (!file_exists($transportDir)) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tdir_nf')); - } else if (!$this->modx->cacheManager->deleteTree($transportDir, true, false, [])) { - $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tdir')); - } else { - $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tdir')); - } - } - /** * Cleanup and return the result * @return array diff --git a/core/src/Revolution/Processors/Workspace/Packages/TransportPackageFilesystemTrait.php b/core/src/Revolution/Processors/Workspace/Packages/TransportPackageFilesystemTrait.php new file mode 100644 index 00000000000..a51a233a730 --- /dev/null +++ b/core/src/Revolution/Processors/Workspace/Packages/TransportPackageFilesystemTrait.php @@ -0,0 +1,95 @@ +modx (modX) from Processor. + */ +trait TransportPackageFilesystemTrait +{ + /** + * Absolute paths to the .transport.zip and unpacked transport directory for this package row. + * + * Unpacked folder name follows xPDOTransport: basename of archive without .transport.zip suffix. + * + * @return array{transportZip: string, transportDir: string} + */ + protected function resolveTransportPaths(modTransportPackage $package): array + { + $workspace = $package->getOne('Workspace'); + if ($workspace !== null) { + $base = rtrim($workspace->get('path'), '/') . '/packages/'; + } else { + /* No workspace: still attempt cleanup under core packages dir (orphan / legacy rows). */ + $base = rtrim((string)$this->modx->getOption('core_path', null, ''), '/') . '/packages/'; + } + + $source = $package->get('source'); + if ($source === null || $source === '') { + $source = $package->get('signature') . '.transport.zip'; + } + $zipName = basename($source); + $transportZip = $base . $zipName; + $dirName = basename($zipName, '.transport.zip'); + $transportDir = $base . $dirName . '/'; + + return [ + 'transportZip' => $transportZip, + 'transportDir' => $transportDir, + ]; + } + + /** + * Remove the transport package archive + * + * Public for backward compatibility with callers that invoked these helpers on Purge/Remove. + */ + public function removeTransportZip(string $transportZip): void + { + $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tzip_start')); + if (!file_exists($transportZip)) { + $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tzip_nf')); + } elseif (!@unlink($transportZip)) { + $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tzip')); + } else { + $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tzip')); + } + } + + /** + * Remove the transport package directory + * + * Public for backward compatibility with callers that invoked these helpers on Purge/Remove. + */ + public function removeTransportDirectory(string $transportDir): void + { + /* Same pattern as clearCache(): ensure cacheManager exists before deleteTree. */ + $this->modx->getCacheManager(); + $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tdir_start')); + if (!is_dir($transportDir)) { + $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tdir_nf')); + } elseif (!$this->modx->cacheManager->deleteTree($transportDir, true, false, [])) { + $this->modx->log(xPDO::LOG_LEVEL_ERROR, $this->modx->lexicon('package_remove_err_tdir')); + } else { + $this->modx->log(xPDO::LOG_LEVEL_INFO, $this->modx->lexicon('package_remove_info_tdir')); + } + } +}