From bfb17881a32e7eb97898f191abed72b1a8630706 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Fri, 14 Aug 2026 09:40:46 +0200 Subject: [PATCH 01/23] STEP 1: add pause and resume button + warning in module + release manager gate --- Classes/ContentReleaseManager.php | 25 ++++ Classes/Controller/BackendController.php | 39 ++++++ .../Core/AutomaticReleaseSwitchService.php | 81 +++++++++++++ .../AutomaticReleasePauseState.php | 75 ++++++++++++ Configuration/Policy.yaml | 10 ++ .../Integration/Backend.Index.fusion | 52 ++++++++ Tests/Unit/ContentReleaseManagerTest.php | 102 ++++++++++++++++ .../AutomaticReleaseSwitchServiceTest.php | 112 ++++++++++++++++++ .../AutomaticReleasePauseStateTest.php | 57 +++++++++ 9 files changed, 553 insertions(+) create mode 100644 Classes/Core/AutomaticReleaseSwitchService.php create mode 100644 Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php create mode 100644 Tests/Unit/ContentReleaseManagerTest.php create mode 100644 Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php create mode 100644 Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php diff --git a/Classes/ContentReleaseManager.php b/Classes/ContentReleaseManager.php index f7f2a5f..e9997a9 100644 --- a/Classes/ContentReleaseManager.php +++ b/Classes/ContentReleaseManager.php @@ -4,6 +4,7 @@ namespace Flowpack\DecoupledContentStore; +use Flowpack\DecoupledContentStore\Core\AutomaticReleaseSwitchService; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; @@ -12,7 +13,9 @@ use Flowpack\Prunner\ValueObject\PipelineName; use Neos\ContentRepository\Domain\Model\Workspace; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Log\Utility\LogEnvironment; use Neos\Flow\Security\Context; +use Psr\Log\LoggerInterface; /** * @Flow\Scope("singleton") @@ -43,9 +46,22 @@ class ContentReleaseManager */ protected $securityContext; + #[Flow\Inject] + protected AutomaticReleaseSwitchService $automaticReleaseSwitchService; + + #[Flow\Inject] + protected LoggerInterface $logger; + const REDIS_CURRENT_RELEASE_KEY = 'contentStore:current'; const NO_PREVIOUS_RELEASE = 'NO_PREVIOUS_RELEASE'; + /** + * All automatic release triggers (workspace publish, asset change, re-release after a rendering error) run through + * this method, so it is the single place where the pause switch takes effect. + * + * While automatic releases are paused, the returned identifier belongs to a release which was never scheduled. + * No caller uses the return value today; the signature is kept so existing callers do not have to change. + */ public function startIncrementalContentRelease( ?string $currentContentReleaseId = null, ?Workspace $workspace = null, @@ -53,6 +69,13 @@ public function startIncrementalContentRelease( ): ContentReleaseIdentifier { $contentReleaseId = ContentReleaseIdentifier::create(); + if ($this->automaticReleaseSwitchService->isPaused()) { + $this->automaticReleaseSwitchService->countSuppressedRelease(); + $this->logger->info(sprintf('Automatic content releases are paused, so content release %s was not scheduled.', $contentReleaseId->getIdentifier()), LogEnvironment::fromMethodName(__METHOD__)); + + return $contentReleaseId; + } + // the currentContentReleaseId is not used in any pipeline step in this package, but is a common need in other // use cases in extensions, e.g. calculating the differences between current and new release $this->prunnerApiService->schedulePipeline( @@ -71,6 +94,8 @@ public function startIncrementalContentRelease( } // the validate parameter can be used to intentionally skip the validation step for this release + // + // This is the explicitly requested release ("Publish All"), so it is deliberately not affected by the pause switch. public function startFullContentRelease( bool $validate = true, ?string $currentContentReleaseId = null, diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index 9172773..a5f7899 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -7,6 +7,7 @@ use Flowpack\DecoupledContentStore\BackendUi\BackendUiDataService; use Flowpack\DecoupledContentStore\BackendUi\WorkerErrorLogAggregator; use Flowpack\DecoupledContentStore\ContentReleaseManager; +use Flowpack\DecoupledContentStore\Core\AutomaticReleaseSwitchService; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\PrunnerJobId; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; @@ -85,6 +86,9 @@ class BackendController extends \Neos\Flow\Mvc\Controller\ActionController */ protected $redisPruneService; + #[Flow\Inject] + protected AutomaticReleaseSwitchService $automaticReleaseSwitchService; + /** * @Flow\InjectConfiguration("redisContentStores") * @var array @@ -121,6 +125,7 @@ public function indexAction(?string $contentStore = null) $configEpochRedis === $currentConfigEpoch ? $previousConfigEpoch : $currentConfigEpoch ); $this->view->assign('showToggleConfigEpochButton', $showToggleConfigEpochButton); + $this->view->assign('automaticReleasePauseState', $this->automaticReleaseSwitchService->getPauseState()); } public function detailsAction( @@ -248,6 +253,40 @@ public function cancelRunningReleaseAction(string $redisInstanceIdentifier) $this->redirect('index', null, null, ['contentStore' => $redisInstanceIdentifier->getIdentifier()]); } + public function pauseAutomaticReleasesAction(?string $contentStore = null): ?string + { + if ($this->request->getHttpRequest()->getMethod() !== 'POST') { + $this->response->setStatusCode(405); + return 'Method not allowed'; + } + + $this->automaticReleaseSwitchService->pause(); + $this->addFlashMessage( + 'Automatic content releases are paused. Editor publishes will not go live until you resume them.' + ); + + $this->redirect('index', null, null, $contentStore !== null ? ['contentStore' => $contentStore] : []); + + return null; + } + + public function resumeAutomaticReleasesAction(?string $contentStore = null): ?string + { + if ($this->request->getHttpRequest()->getMethod() !== 'POST') { + $this->response->setStatusCode(405); + return 'Method not allowed'; + } + + $this->automaticReleaseSwitchService->resume(); + $this->addFlashMessage( + 'Automatic content releases are enabled again. Changes published while they were paused go live with the next release.' + ); + + $this->redirect('index', null, null, $contentStore !== null ? ['contentStore' => $contentStore] : []); + + return null; + } + public function toggleConfigEpochAction(string $redisInstanceIdentifier) { $redisInstanceIdentifier = RedisInstanceIdentifier::fromString($redisInstanceIdentifier); diff --git a/Classes/Core/AutomaticReleaseSwitchService.php b/Classes/Core/AutomaticReleaseSwitchService.php new file mode 100644 index 0000000..f82fd5e --- /dev/null +++ b/Classes/Core/AutomaticReleaseSwitchService.php @@ -0,0 +1,81 @@ +redisClientManager->getPrimaryRedis()->hExists(self::REDIS_KEY, 'pausedAt'); + } + + public function getPauseState(): ?AutomaticReleasePauseState + { + $redisHash = $this->redisClientManager->getPrimaryRedis()->hGetAll(self::REDIS_KEY); + if (!array_key_exists('pausedAt', $redisHash)) { + return null; + } + + return AutomaticReleasePauseState::fromRedisHash($redisHash); + } + + public function pause(): void + { + if ($this->isPaused()) { + return; + } + + $this->redisClientManager->getPrimaryRedis()->hMset(self::REDIS_KEY, [ + 'pausedAt' => (new DateTimeImmutable())->format(DateTimeInterface::ATOM), + 'accountId' => $this->getAccountId() ?? '', + 'suppressedReleaseCount' => 0, + ]); + } + + public function resume(): void + { + $this->redisClientManager->getPrimaryRedis()->del(self::REDIS_KEY); + } + + public function countSuppressedRelease(): void + { + $this->redisClientManager->getPrimaryRedis()->hIncrBy(self::REDIS_KEY, 'suppressedReleaseCount', 1); + } + + private function getAccountId(): ?string + { + // getAccount() is documented as always returning an Account, but returns NULL whenever nothing is + // authenticated - which is the normal case for the CLI triggers + $account = $this->securityContext->isInitialized() ? $this->securityContext->getAccount() : null; + + return $account?->getAccountIdentifier(); + } +} diff --git a/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php b/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php new file mode 100644 index 0000000..4b9160f --- /dev/null +++ b/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php @@ -0,0 +1,75 @@ +pausedAt = $pausedAt; + $this->accountId = $accountId; + $this->suppressedReleaseCount = $suppressedReleaseCount; + } + + /** + * "pausedAt" is what marks the switch as set, so a hash without it is not a pause state - see + * AutomaticReleaseSwitchService::isPaused(). Rejected rather than defaulted, because inventing a timestamp would + * hide the inconsistency behind a banner claiming the pause started just now. + * + * @param array $redisHash + * @throws DateMalformedStringException + */ + public static function fromRedisHash(array $redisHash): self + { + if (!array_key_exists('pausedAt', $redisHash)) { + throw new InvalidArgumentException( + 'The automatic release pause state must contain a "pausedAt" field.', + 1786706446 + ); + } + + return new self( + new DateTimeImmutable($redisHash['pausedAt']), + ($redisHash['accountId'] ?? '') !== '' ? $redisHash['accountId'] : null, + (int)($redisHash['suppressedReleaseCount'] ?? 0) + ); + } + + public function getPausedAt(): DateTimeImmutable + { + return $this->pausedAt; + } + + /** + * The account which paused the automatic releases, if it could be determined. + */ + public function getAccountId(): ?string + { + return $this->accountId; + } + + /** + * How many automatically triggered releases have been suppressed since the pause started. + */ + public function getSuppressedReleaseCount(): int + { + return $this->suppressedReleaseCount; + } +} diff --git a/Configuration/Policy.yaml b/Configuration/Policy.yaml index f9f57fe..e8ed1ce 100644 --- a/Configuration/Policy.yaml +++ b/Configuration/Policy.yaml @@ -3,9 +3,19 @@ privilegeTargets: 'Flowpack.DecoupledContentStore:BackendModule': matcher: 'administration/contentstore' + 'Neos\Flow\Security\Authorization\Privilege\Method\MethodPrivilege': + # Operating the switch which suppresses automatically triggered content releases. Separate from the module + # privilege above, so an installation which lets editors watch the module can still restrict who may pause + # releases and thereby stop everybody's publishes from going live. + 'Flowpack.DecoupledContentStore:ReleaseControl': + matcher: 'method(Flowpack\DecoupledContentStore\Controller\BackendController->(pauseAutomaticReleases|resumeAutomaticReleases)Action())' + roles: 'Neos.Neos:Administrator': privileges: - privilegeTarget: 'Flowpack.DecoupledContentStore:BackendModule' permission: GRANT + - + privilegeTarget: 'Flowpack.DecoupledContentStore:ReleaseControl' + permission: GRANT diff --git a/Resources/Private/BackendFusion/Integration/Backend.Index.fusion b/Resources/Private/BackendFusion/Integration/Backend.Index.fusion index 37ede51..6792ef2 100644 --- a/Resources/Private/BackendFusion/Integration/Backend.Index.fusion +++ b/Resources/Private/BackendFusion/Integration/Backend.Index.fusion @@ -8,6 +8,7 @@ Flowpack.DecoupledContentStore.BackendController.index = Neos.Fusion:Component { // - toggleFromConfigEpoch: string, e.g. "2" // - toggleToConfigEpoch: string, e.g. "1" // - showToggleConfigEpochButton: boolean + // - automaticReleasePauseState: AutomaticReleasePauseState object, or NULL if automatic releases are not paused renderer = Neos.Fusion:Component { _renderedTableBody = Neos.Fusion:Loop { @@ -64,6 +65,7 @@ Flowpack.DecoupledContentStore.BackendController.index = Neos.Fusion:Component {
+ Content Store Releases
@@ -102,6 +104,27 @@ Flowpack.DecoupledContentStore.BackendController.index = Neos.Fusion:Component { } } +// Rendered whenever automatic releases are paused, for everybody who can see the module - the pause stops all editor +// publishes from going live, so it must not be visible only to those allowed to lift it. +prototype(Flowpack.DecoupledContentStore:AutomaticReleasesPausedBanner) < prototype(Neos.Fusion:Component) { + pauseState = ${automaticReleasePauseState} + + renderer = afx` +
+
Automatic content releases are paused
+
+ Nothing editors publish goes live until an administrator resumes them. + Paused on {Date.format(props.pauseState.pausedAt, 'd.m.Y H:i:s')} + by {props.pauseState.accountId}, + {props.pauseState.suppressedReleaseCount} release(s) suppressed since then. +
+
+ ` +} + prototype(Flowpack.DecoupledContentStore:ContentStoreActions) < prototype(Neos.Fusion:Join) { @process.wrap = afx`
@@ -109,6 +132,35 @@ prototype(Flowpack.DecoupledContentStore:ContentStoreActions) < prototype(Neos.F
` + toggleAutomaticReleases = Neos.Fusion:Component { + @if.hasAccess = ${Security.hasAccess('Flowpack.DecoupledContentStore:ReleaseControl')} + + isPaused = ${automaticReleasePauseState ? true : false} + + _pauseUri = Neos.Fusion:UriBuilder { + action = 'pauseAutomaticReleases' + arguments = Neos.Fusion:DataStructure { + contentStore = ${contentStore} + } + } + + _resumeUri = Neos.Fusion:UriBuilder { + action = 'resumeAutomaticReleases' + arguments = Neos.Fusion:DataStructure { + contentStore = ${contentStore} + } + } + + renderer = afx` + + + ` + } + publishAllWithoutValidation = Neos.Fusion:Component { _publishAllWithoutValidationUri = Neos.Fusion:UriBuilder { action = 'publishAllWithoutValidation' diff --git a/Tests/Unit/ContentReleaseManagerTest.php b/Tests/Unit/ContentReleaseManagerTest.php new file mode 100644 index 0000000..d9edd91 --- /dev/null +++ b/Tests/Unit/ContentReleaseManagerTest.php @@ -0,0 +1,102 @@ +prunnerApiService = $this->createMock(PrunnerApiService::class); + $this->prunnerApiService->method('schedulePipeline')->willReturn(JobId::create('job-id')); + + $this->automaticReleaseSwitchService = $this->createMock(AutomaticReleaseSwitchService::class); + } + + public function testAnAutomaticReleaseIsNotScheduledWhilePaused(): void + { + $this->automaticReleaseSwitchService->method('isPaused')->willReturn(true); + $this->prunnerApiService->expects(self::never())->method('schedulePipeline'); + + $this->buildContentReleaseManager()->startIncrementalContentRelease(); + } + + public function testASuppressedReleaseIsCountedSoTheBackendCanShowHowMuchIsWaiting(): void + { + $this->automaticReleaseSwitchService->method('isPaused')->willReturn(true); + $this->automaticReleaseSwitchService->expects(self::once())->method('countSuppressedRelease'); + + $this->buildContentReleaseManager()->startIncrementalContentRelease(); + } + + public function testAnAutomaticReleaseIsScheduledWhileNotPaused(): void + { + $this->automaticReleaseSwitchService->method('isPaused')->willReturn(false); + $this->automaticReleaseSwitchService->expects(self::never())->method('countSuppressedRelease'); + $this->prunnerApiService->expects(self::once())->method('schedulePipeline'); + + $this->buildContentReleaseManager()->startIncrementalContentRelease(); + } + + public function testPublishAllIsScheduledEvenWhilePaused(): void + { + $this->automaticReleaseSwitchService->method('isPaused')->willReturn(true); + $this->prunnerApiService->expects(self::once())->method('schedulePipeline'); + + $this->buildContentReleaseManager()->startFullContentRelease(); + } + + private function buildContentReleaseManager(): ContentReleaseManager + { + $redis = $this->createMock(\Redis::class); + $redis->method('get')->willReturn(false); + + $redisClientManager = $this->createMock(RedisClientManager::class); + $redisClientManager->method('getPrimaryRedis')->willReturn($redis); + + $securityContext = $this->createMock(Context::class); + $securityContext->method('isInitialized')->willReturn(false); + + $contentReleaseManager = new ContentReleaseManager(); + self::injectDependency($contentReleaseManager, 'prunnerApiService', $this->prunnerApiService); + self::injectDependency( + $contentReleaseManager, + 'automaticReleaseSwitchService', + $this->automaticReleaseSwitchService + ); + self::injectDependency($contentReleaseManager, 'redisClientManager', $redisClientManager); + self::injectDependency($contentReleaseManager, 'securityContext', $securityContext); + self::injectDependency($contentReleaseManager, 'logger', new NullLogger()); + + return $contentReleaseManager; + } + + private static function injectDependency(object $target, string $propertyName, object $dependency): void + { + // the class uses Flow property injection, which is not available outside a Flow bootstrap + (new ReflectionProperty($target, $propertyName))->setValue($target, $dependency); + } +} diff --git a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php new file mode 100644 index 0000000..93032c0 --- /dev/null +++ b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php @@ -0,0 +1,112 @@ +buildRedis(); + $redis->method('hExists')->with(self::REDIS_KEY, 'pausedAt')->willReturn(false); + + self::assertFalse($this->buildService($redis)->isPaused()); + } + + public function testPausingWhileAlreadyPausedKeepsTheOriginalState(): void + { + // otherwise the second pause would reset both the timestamp and the count of suppressed releases + $redis = $this->buildRedis(); + $redis->method('hExists')->willReturn(true); + $redis->expects(self::never())->method('hMSet'); + + $this->buildService($redis)->pause(); + } + + public function testPausingRecordsTheTimestampAndAnEmptyCounter(): void + { + $redis = $this->buildRedis(); + $redis->method('hExists')->willReturn(false); + $redis->expects(self::once())->method('hMSet')->with( + self::REDIS_KEY, + self::callback(static function (array $hash): bool { + return $hash['accountId'] === '' + && $hash['suppressedReleaseCount'] === 0 + && \DateTimeImmutable::createFromFormat(\DateTimeInterface::ATOM, $hash['pausedAt']) !== false; + }) + ); + + $this->buildService($redis)->pause(); + } + + public function testThereIsNoPauseStateWhileTheSwitchIsNotSet(): void + { + $redis = $this->buildRedis(); + $redis->method('hGetAll')->willReturn([]); + + self::assertNull($this->buildService($redis)->getPauseState()); + } + + public function testThePauseStateIsReadFromTheHash(): void + { + $redis = $this->buildRedis(); + $redis->method('hGetAll')->willReturn([ + 'pausedAt' => '2026-08-13T09:15:00+02:00', + 'accountId' => 'admin', + 'suppressedReleaseCount' => '4', + ]); + + $pauseState = $this->buildService($redis)->getPauseState(); + + self::assertNotNull($pauseState); + self::assertSame('admin', $pauseState->getAccountId()); + self::assertSame(4, $pauseState->getSuppressedReleaseCount()); + } + + /** + * @return \Redis&MockObject + */ + private function buildRedis(): \Redis + { + return $this->createMock(\Redis::class); + } + + private function buildService(\Redis $redis): AutomaticReleaseSwitchService + { + $redisClientManager = $this->createMock(RedisClientManager::class); + $redisClientManager->method('getPrimaryRedis')->willReturn($redis); + + $securityContext = $this->createMock(Context::class); + $securityContext->method('isInitialized')->willReturn(false); + + $service = new AutomaticReleaseSwitchService(); + self::injectDependency($service, 'redisClientManager', $redisClientManager); + self::injectDependency($service, 'securityContext', $securityContext); + + return $service; + } + + private static function injectDependency(object $target, string $propertyName, object $dependency): void + { + // the class uses Flow property injection, which is not available outside a Flow bootstrap + (new ReflectionProperty($target, $propertyName))->setValue($target, $dependency); + } +} diff --git a/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php b/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php new file mode 100644 index 0000000..5eaa2f1 --- /dev/null +++ b/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php @@ -0,0 +1,57 @@ + '2026-08-13T09:15:00+02:00', + 'accountId' => 'admin', + 'suppressedReleaseCount' => '7', + ]); + + self::assertSame('2026-08-13T09:15:00+02:00', $pauseState->getPausedAt()->format(\DateTimeInterface::ATOM)); + self::assertSame('admin', $pauseState->getAccountId()); + self::assertSame(7, $pauseState->getSuppressedReleaseCount()); + } + + public function testAnEmptyAccountIdBecomesNull(): void + { + // pause() writes an empty string when no account could be determined, because a Redis hash has no null + $pauseState = AutomaticReleasePauseState::fromRedisHash([ + 'pausedAt' => '2026-08-13T09:15:00+02:00', + 'accountId' => '', + 'suppressedReleaseCount' => '0', + ]); + + self::assertNull($pauseState->getAccountId()); + } + + public function testTheCounterDefaultsToZero(): void + { + $pauseState = AutomaticReleasePauseState::fromRedisHash([ + 'pausedAt' => '2026-08-13T09:15:00+02:00', + ]); + + self::assertSame(0, $pauseState->getSuppressedReleaseCount()); + self::assertNull($pauseState->getAccountId()); + } + + public function testAHashWithoutPausedAtIsRejected(): void + { + $this->expectException(\InvalidArgumentException::class); + + AutomaticReleasePauseState::fromRedisHash(['suppressedReleaseCount' => '3']); + } +} From a345f5121f2f0fe49b8078502fb8590a2dfb04a0 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 09:53:23 +0200 Subject: [PATCH 02/23] STEP 2: add warning in content module when releases are paused --- .gitignore | 1 + .../AutomaticReleaseStatusDataSource.php | 69 +++++++++++ Classes/BackendUi/BackendDateFormatter.php | 36 ++++++ Classes/Controller/BackendController.php | 43 +++++-- Configuration/Settings.yaml | 15 +++ .../Integration/Backend.Index.fusion | 34 ++++-- Resources/Private/Translations/de/Main.xlf | 47 ++++++++ Resources/Private/Translations/en/Main.xlf | 38 ++++++ Resources/Public/BackendCompiled/out.css | 10 +- .../ContentModule/AutomaticReleaseWarning.css | 26 +++++ .../ContentModule/AutomaticReleaseWarning.js | 109 ++++++++++++++++++ .../AutomaticReleaseStatusDataSourceTest.php | 101 ++++++++++++++++ 12 files changed, 508 insertions(+), 21 deletions(-) create mode 100644 Classes/BackendUi/AutomaticReleaseStatusDataSource.php create mode 100644 Classes/BackendUi/BackendDateFormatter.php create mode 100644 Resources/Private/Translations/de/Main.xlf create mode 100644 Resources/Private/Translations/en/Main.xlf create mode 100644 Resources/Public/ContentModule/AutomaticReleaseWarning.css create mode 100644 Resources/Public/ContentModule/AutomaticReleaseWarning.js create mode 100644 Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php diff --git a/.gitignore b/.gitignore index 66f204b..5c16bce 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ node_modules composer.lock +package-lock.json vendor Packages .phpunit.cache diff --git a/Classes/BackendUi/AutomaticReleaseStatusDataSource.php b/Classes/BackendUi/AutomaticReleaseStatusDataSource.php new file mode 100644 index 0000000..714b0f2 --- /dev/null +++ b/Classes/BackendUi/AutomaticReleaseStatusDataSource.php @@ -0,0 +1,69 @@ + $arguments + * @return array + */ + public function getData(?NodeInterface $node = null, array $arguments = []): array + { + $pauseState = $this->automaticReleaseSwitchService->getPauseState(); + + if ($pauseState === null) { + return ['paused' => false]; + } + + return [ + 'paused' => true, + 'message' => (string)$this->translator->translateById( + 'automaticReleases.paused.contentModuleWarning', + [ + $this->backendDateFormatter->format($pauseState->getPausedAt()), + $pauseState->getSuppressedReleaseCount(), + ], + null, + null, + 'Main', + 'Flowpack.DecoupledContentStore' + ), + ]; + } +} diff --git a/Classes/BackendUi/BackendDateFormatter.php b/Classes/BackendUi/BackendDateFormatter.php new file mode 100644 index 0000000..267a291 --- /dev/null +++ b/Classes/BackendUi/BackendDateFormatter.php @@ -0,0 +1,36 @@ +datetimeFormatter->formatDateTime( + $dateTime, + $this->localizationService->getConfiguration()->getCurrentLocale(), + DatesReader::FORMAT_LENGTH_MEDIUM + ); + } +} diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index a5f7899..29dea28 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -4,6 +4,7 @@ namespace Flowpack\DecoupledContentStore\Controller; +use Flowpack\DecoupledContentStore\BackendUi\BackendDateFormatter; use Flowpack\DecoupledContentStore\BackendUi\BackendUiDataService; use Flowpack\DecoupledContentStore\BackendUi\WorkerErrorLogAggregator; use Flowpack\DecoupledContentStore\ContentReleaseManager; @@ -21,11 +22,16 @@ use Flowpack\Prunner\PrunnerApiService; use Flowpack\Prunner\ValueObject\PipelineName; use Neos\Flow\Annotations as Flow; +use Neos\Flow\I18n\Translator; +use Neos\Flow\Mvc\Controller\ActionController; use Neos\Fusion\View\FusionView; +use Neos\Neos\Controller\BackendUserTranslationTrait; use Symfony\Component\Console\Output\BufferedOutput; -class BackendController extends \Neos\Flow\Mvc\Controller\ActionController +class BackendController extends ActionController { + use BackendUserTranslationTrait; + /** * @Flow\Inject * @var PrunnerApiService @@ -89,6 +95,12 @@ class BackendController extends \Neos\Flow\Mvc\Controller\ActionController #[Flow\Inject] protected AutomaticReleaseSwitchService $automaticReleaseSwitchService; + #[Flow\Inject] + protected Translator $translator; + + #[Flow\Inject] + protected BackendDateFormatter $backendDateFormatter; + /** * @Flow\InjectConfiguration("redisContentStores") * @var array @@ -125,7 +137,14 @@ public function indexAction(?string $contentStore = null) $configEpochRedis === $currentConfigEpoch ? $previousConfigEpoch : $currentConfigEpoch ); $this->view->assign('showToggleConfigEpochButton', $showToggleConfigEpochButton); - $this->view->assign('automaticReleasePauseState', $this->automaticReleaseSwitchService->getPauseState()); + $automaticReleasePauseState = $this->automaticReleaseSwitchService->getPauseState(); + $this->view->assign('automaticReleasePauseState', $automaticReleasePauseState); + $this->view->assign( + 'automaticReleasePausedAt', + $automaticReleasePauseState !== null + ? $this->backendDateFormatter->format($automaticReleasePauseState->getPausedAt()) + : null + ); } public function detailsAction( @@ -261,9 +280,7 @@ public function pauseAutomaticReleasesAction(?string $contentStore = null): ?str } $this->automaticReleaseSwitchService->pause(); - $this->addFlashMessage( - 'Automatic content releases are paused. Editor publishes will not go live until you resume them.' - ); + $this->addFlashMessage($this->translateById('automaticReleases.paused.flashMessage')); $this->redirect('index', null, null, $contentStore !== null ? ['contentStore' => $contentStore] : []); @@ -278,9 +295,7 @@ public function resumeAutomaticReleasesAction(?string $contentStore = null): ?st } $this->automaticReleaseSwitchService->resume(); - $this->addFlashMessage( - 'Automatic content releases are enabled again. Changes published while they were paused go live with the next release.' - ); + $this->addFlashMessage($this->translateById('automaticReleases.resumed.flashMessage')); $this->redirect('index', null, null, $contentStore !== null ? ['contentStore' => $contentStore] : []); @@ -294,4 +309,16 @@ public function toggleConfigEpochAction(string $redisInstanceIdentifier) $this->redirect('index', null, null, ['contentStore' => $redisInstanceIdentifier->getIdentifier()]); } + + private function translateById(string $labelId): string + { + return (string)$this->translator->translateById( + $labelId, + [], + null, + null, + 'Main', + 'Flowpack.DecoupledContentStore' + ); + } } diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index e61895b..a5d4d91 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -196,6 +196,21 @@ Neos: icon: 'fas fa-exchange-alt' mainStylesheet: 'Lite' + Ui: + resources: + # Warns editors that publishing does not reach the live site while automatic releases are paused. Both files + # are served as they are, so registering them here needs no build step. + javascript: + 'Flowpack.DecoupledContentStore:AutomaticReleaseWarning': + resource: 'resource://Flowpack.DecoupledContentStore/Public/ContentModule/AutomaticReleaseWarning.js' + # Before the UI host and without "defer", because the host reads the inlined _NEOS_UI_* globals once and + # deletes them - the script has to capture the route table before that happens. + position: 'before Neos.Neos.UI:Host' + stylesheets: + 'Flowpack.DecoupledContentStore:AutomaticReleaseWarning': + resource: 'resource://Flowpack.DecoupledContentStore/Public/ContentModule/AutomaticReleaseWarning.css' + position: 'end' + Flow: resource: targets: diff --git a/Resources/Private/BackendFusion/Integration/Backend.Index.fusion b/Resources/Private/BackendFusion/Integration/Backend.Index.fusion index 6792ef2..302930f 100644 --- a/Resources/Private/BackendFusion/Integration/Backend.Index.fusion +++ b/Resources/Private/BackendFusion/Integration/Backend.Index.fusion @@ -9,6 +9,7 @@ Flowpack.DecoupledContentStore.BackendController.index = Neos.Fusion:Component { // - toggleToConfigEpoch: string, e.g. "1" // - showToggleConfigEpochButton: boolean // - automaticReleasePauseState: AutomaticReleasePauseState object, or NULL if automatic releases are not paused + // - automaticReleasePausedAt: the pause timestamp, formatted for the backend user's language renderer = Neos.Fusion:Component { _renderedTableBody = Neos.Fusion:Loop { @@ -108,23 +109,42 @@ Flowpack.DecoupledContentStore.BackendController.index = Neos.Fusion:Component { // publishes from going live, so it must not be visible only to those allowed to lift it. prototype(Flowpack.DecoupledContentStore:AutomaticReleasesPausedBanner) < prototype(Neos.Fusion:Component) { pauseState = ${automaticReleasePauseState} + // formatted by the controller, so that the timestamp reads the same here and in the content module warning + pausedAt = ${automaticReleasePausedAt} renderer = afx`
-
Automatic content releases are paused
+
+ +
- Nothing editors publish goes live until an administrator resumes them. - Paused on {Date.format(props.pauseState.pausedAt, 'd.m.Y H:i:s')} - by {props.pauseState.accountId}, - {props.pauseState.suppressedReleaseCount} release(s) suppressed since then. + + {' '} + +
` } +prototype(Flowpack.DecoupledContentStore:Translate) < prototype(Neos.Fusion:Value) { + id = null + arguments = ${[]} + + value = ${I18n.id(this.id).package('Flowpack.DecoupledContentStore').source('Main').arguments(this.arguments).translate()} +} + prototype(Flowpack.DecoupledContentStore:ContentStoreActions) < prototype(Neos.Fusion:Join) { @process.wrap = afx`
@@ -153,10 +173,10 @@ prototype(Flowpack.DecoupledContentStore:ContentStoreActions) < prototype(Neos.F renderer = afx` ` } diff --git a/Resources/Private/Translations/de/Main.xlf b/Resources/Private/Translations/de/Main.xlf new file mode 100644 index 0000000..3e55899 --- /dev/null +++ b/Resources/Private/Translations/de/Main.xlf @@ -0,0 +1,47 @@ + + + + + + + Pause automatic releases + Automatische Releases pausieren + + + Resume automatic releases + Automatische Releases fortsetzen + + + Automatic content releases are paused. Editor publishes will not go live until you resume them. + Automatische Content-Releases sind pausiert. Veröffentlichungen der Redakteure gehen erst live, wenn Sie die Releases wieder fortsetzen. + + + Automatic content releases are enabled again. Changes published while they were paused go live with the next release. + Automatische Content-Releases sind wieder aktiv. Änderungen, die während der Pause veröffentlicht wurden, gehen mit dem nächsten Release live. + + + Automatic content releases are paused + Automatische Content-Releases sind pausiert + + + Nothing editors publish goes live until an administrator resumes the releases. + Was Redakteure veröffentlichen, geht erst live, wenn ein Administrator die Releases wieder fortsetzt. + + + + Paused on {0} by {1}, {2} release(s) suppressed since then. + Pausiert am {0} von {1}, seitdem {2} Release(s) unterdrückt. + + + + Paused on {0}, {1} release(s) suppressed since then. + Pausiert am {0}, seitdem {1} Release(s) unterdrückt. + + + + Automatic content releases have been paused on {0}, so your changes do not go live yet. {1} release(s) are waiting. An administrator has to resume them in the Content Store module. + Automatische Content-Releases wurden am {0} pausiert, Ihre Änderungen gehen daher noch nicht live. {1} Release(s) warten. Ein Administrator muss sie im Modul „Content Store“ wieder fortsetzen. + + + + diff --git a/Resources/Private/Translations/en/Main.xlf b/Resources/Private/Translations/en/Main.xlf new file mode 100644 index 0000000..feacfbe --- /dev/null +++ b/Resources/Private/Translations/en/Main.xlf @@ -0,0 +1,38 @@ + + + + + + + Pause automatic releases + + + Resume automatic releases + + + Automatic content releases are paused. Editor publishes will not go live until you resume them. + + + Automatic content releases are enabled again. Changes published while they were paused go live with the next release. + + + Automatic content releases are paused + + + Nothing editors publish goes live until an administrator resumes the releases. + + + + Paused on {0} by {1}, {2} release(s) suppressed since then. + + + + Paused on {0}, {1} release(s) suppressed since then. + + + + Automatic content releases have been paused on {0}, so your changes do not go live yet. {1} release(s) are waiting. An administrator has to resume them in the Content Store module. + + + + diff --git a/Resources/Public/BackendCompiled/out.css b/Resources/Public/BackendCompiled/out.css index b2df9df..bd407e1 100644 --- a/Resources/Public/BackendCompiled/out.css +++ b/Resources/Public/BackendCompiled/out.css @@ -1,4 +1,4 @@ -/* ../../../../../../../../private/var/folders/fd/vt4bk4351l97704fxm4zmmyr0000gn/T/tmp-47815-zvlMZoCh5PYD/Flowpack.DecoupledContentStore/Resources/Private/Js/1778224314767-styles.css */ +/* ../../../../../../../../private/var/folders/fd/vt4bk4351l97704fxm4zmmyr0000gn/T/tmp-30026-6RnCASmiDb7G/Flowpack.DecoupledContentStore/Resources/Private/Js/1786952756520-styles.css */ #app *, #app ::before, #app ::after { @@ -11,6 +11,9 @@ #app ::after { --tw-content: ""; } +#app .visible { + visibility: visible; +} #app .absolute { position: absolute; } @@ -155,11 +158,6 @@ #app .overflow-x-scroll { overflow-x: scroll; } -#app .truncate { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -} #app .whitespace-pre { white-space: pre; } diff --git a/Resources/Public/ContentModule/AutomaticReleaseWarning.css b/Resources/Public/ContentModule/AutomaticReleaseWarning.css new file mode 100644 index 0000000..2309db4 --- /dev/null +++ b/Resources/Public/ContentModule/AutomaticReleaseWarning.css @@ -0,0 +1,26 @@ +/* + * The warning painted by AutomaticReleaseWarning.js. + * + * It is the first element in the document flow, above everything Neos renders, so it can never cover a control. + */ +#flowpack-decoupledcontentstore-release-warning { + padding: 12px 24px; + background-color: #ff460d; + color: #fff; + font-family: "Noto Sans", sans-serif; + font-size: 15px; + font-weight: bold; + line-height: 1.4; + text-align: center; +} + +/* + * The Neos chrome is positioned fixed and therefore ignores the document flow - without this it would start at the + * top of the viewport, behind the warning. Any transform makes the application container the containing block for + * those fixed children, so they lay themselves out inside the remaining height instead of the whole viewport. + */ +html.flowpack-decoupledcontentstore-release-warning-visible #appContainer { + position: relative; + height: calc(100vh - var(--flowpack-decoupledcontentstore-release-warning-height, 0px)); + transform: translate(0); +} diff --git a/Resources/Public/ContentModule/AutomaticReleaseWarning.js b/Resources/Public/ContentModule/AutomaticReleaseWarning.js new file mode 100644 index 0000000..3a8574b --- /dev/null +++ b/Resources/Public/ContentModule/AutomaticReleaseWarning.js @@ -0,0 +1,109 @@ +/** + * Warns editors in the content module while automatic content releases are paused, because publishing still works + * then - the change just does not reach the live site until somebody resumes the releases. + * + * Plain ES5, served as it is: this file is not part of the package's esbuild bundle, so it can be registered under + * Neos.Neos.Ui.resources.javascript without a build step. + */ +(function () { + 'use strict'; + + const DATA_SOURCE_IDENTIFIER = 'flowpack-decoupledcontentstore-automatic-release-status'; + const POLL_INTERVAL_IN_MS = 30000; + const ELEMENT_ID = 'flowpack-decoupledcontentstore-release-warning'; + const VISIBLE_CLASS = 'flowpack-decoupledcontentstore-release-warning-visible'; + const HEIGHT_PROPERTY = '--flowpack-decoupledcontentstore-release-warning-height'; + + // Read now, not when polling: the UI host consumes the inlined _NEOS_UI_* globals with a "delete" as soon as it + // boots, so this script is registered before the host and without "defer" purely to get here first. + const routes = window._NEOS_UI_routes; + let warned = false; + + function statusUri() { + const dataSourceUri = routes && routes.core && routes.core.service && routes.core.service.dataSource; + + return dataSourceUri ? dataSourceUri + '/' + DATA_SOURCE_IDENTIFIER : null; + } + + // once, not on every poll: an editor cannot act on this, but without it a broken endpoint looks exactly like + // "no releases are paused" + function warn(reason) { + if (!warned) { + warned = true; + console.warn('[Flowpack.DecoupledContentStore] cannot read the automatic release status: ' + reason); + } + } + + // The warning sits in the document flow above the Neos chrome, which is positioned fixed and would otherwise + // ignore it. Publishing its height lets the stylesheet shrink the application container by exactly that much. + function publishHeight() { + const element = document.getElementById(ELEMENT_ID); + + if (element) { + document.documentElement.style.setProperty(HEIGHT_PROPERTY, element.offsetHeight + 'px'); + } + } + + function render(status) { + let element = document.getElementById(ELEMENT_ID); + + if (!status.paused) { + if (element) { + element.parentNode.removeChild(element); + document.documentElement.classList.remove(VISIBLE_CLASS); + document.documentElement.style.removeProperty(HEIGHT_PROPERTY); + } + return; + } + + if (!element) { + element = document.createElement('div'); + element.id = ELEMENT_ID; + element.setAttribute('role', 'alert'); + document.body.insertBefore(element, document.body.firstChild); + document.documentElement.classList.add(VISIBLE_CLASS); + } + + element.textContent = status.message; + publishHeight(); + } + + function poll() { + const uri = statusUri(); + if (!uri) { + warn('_NEOS_UI_routes.core.service.dataSource is not available'); + return; + } + + fetch(uri, {credentials: 'same-origin'}) + .then(function (response) { + if (!response.ok) { + warn(uri + ' answered ' + response.status); + return null; + } + return response.json(); + }) + .then(function (status) { + if (status) { + render(status); + } + }) + .catch(function (error) { + warn(String(error)); + }); + } + + function start() { + poll(); + window.setInterval(poll, POLL_INTERVAL_IN_MS); + // the message wraps to a different number of lines as the window gets narrower + window.addEventListener('resize', publishHeight); + } + + // running before the UI host means running before exists + if (document.readyState === 'loading') { + document.addEventListener('DOMContentLoaded', start); + } else { + start(); + } +})(); diff --git a/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php b/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php new file mode 100644 index 0000000..c266bdb --- /dev/null +++ b/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php @@ -0,0 +1,101 @@ + false], $this->buildDataSource(null)->getData()); + } + + public function testTheWarningIsPublishedAsAReadyMadeMessage(): void + { + // the script has no translation API available, so it prints what it gets + $pauseState = AutomaticReleasePauseState::fromRedisHash([ + 'pausedAt' => '2026-08-13T09:15:00+02:00', + 'accountId' => 'admin', + 'suppressedReleaseCount' => '4', + ]); + + self::assertSame([ + 'paused' => true, + 'message' => 'translated: automaticReleases.paused.contentModuleWarning', + ], $this->buildDataSource($pauseState)->getData()); + } + + public function testTheTimestampAndTheWaitingCountAreHandedToTheTranslation(): void + { + $pauseState = AutomaticReleasePauseState::fromRedisHash([ + 'pausedAt' => '2026-08-13T09:15:00+02:00', + 'suppressedReleaseCount' => '4', + ]); + + $translator = $this->createMock(Translator::class); + $translator->expects(self::once())->method('translateById')->with( + 'automaticReleases.paused.contentModuleWarning', + ['formatted date', 4], + null, + null, + 'Main', + 'Flowpack.DecoupledContentStore' + ); + + $this->buildDataSource($pauseState, $translator)->getData(); + } + + private function buildDataSource( + ?AutomaticReleasePauseState $pauseState, + ?Translator $translator = null + ): AutomaticReleaseStatusDataSource { + $automaticReleaseSwitchService = $this->createMock(AutomaticReleaseSwitchService::class); + $automaticReleaseSwitchService->method('getPauseState')->willReturn($pauseState); + + if ($translator === null) { + $translator = $this->createMock(Translator::class); + $translator->method('translateById')->willReturnCallback( + static fn(string $labelId): string => 'translated: ' . $labelId + ); + } + + $backendDateFormatter = $this->createMock(BackendDateFormatter::class); + $backendDateFormatter->method('format')->willReturn('formatted date'); + + $dataSource = new AutomaticReleaseStatusDataSource(); + // the class uses Flow property injection, which is not available outside a Flow bootstrap + self::injectDependency($dataSource, 'automaticReleaseSwitchService', $automaticReleaseSwitchService); + self::injectDependency($dataSource, 'translator', $translator); + self::injectDependency($dataSource, 'backendDateFormatter', $backendDateFormatter); + + return $dataSource; + } + + private static function injectDependency(object $target, string $propertyName, object $dependency): void + { + (new ReflectionProperty($target, $propertyName))->setValue($target, $dependency); + } +} From 7d3cae7b1f1a048486326ec01bad32c24d471a8f Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 11:15:43 +0200 Subject: [PATCH 03/23] STEP 3: add copy command --- ...ntReleaseQuickPublishCommandController.php | 53 +++++ .../RedisReleaseCopyService.php | 225 ++++++++++++++++++ .../Dto/RedisKeyPostfixForEachRelease.php | 28 ++- .../Dto/RedisKeyPostfixesForEachRelease.php | 14 ++ Configuration/Settings.yaml | 19 ++ Configuration/Testing/Behat/Settings.yaml | 4 +- Resources/Public/BackendCompiled/out.css | 2 +- .../Features/Bootstrap/FeatureContext.php | 48 ++++ .../ContentStore/QuickRelease.feature | 56 +++++ .../AutomaticReleaseStatusDataSourceTest.php | 17 +- Tests/Unit/ContentReleaseManagerTest.php | 25 +- .../AutomaticReleaseSwitchServiceTest.php | 15 +- .../AutomaticReleasePauseStateTest.php | 4 +- .../RedisReleaseCopyServiceTest.php | 215 +++++++++++++++++ .../RedisKeyPostfixesForEachReleaseTest.php | 64 +++++ 15 files changed, 741 insertions(+), 48 deletions(-) create mode 100644 Classes/Command/ContentReleaseQuickPublishCommandController.php create mode 100644 Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php create mode 100644 Tests/Behavior/Features/ContentStore/QuickRelease.feature create mode 100644 Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php create mode 100644 Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php diff --git a/Classes/Command/ContentReleaseQuickPublishCommandController.php b/Classes/Command/ContentReleaseQuickPublishCommandController.php new file mode 100644 index 0000000..020a504 --- /dev/null +++ b/Classes/Command/ContentReleaseQuickPublishCommandController.php @@ -0,0 +1,53 @@ +output, $targetIdentifier); + + try { + $this->redisReleaseCopyService->copyReleaseWithin( + $redisInstanceIdentifier, + $sourceIdentifier, + $targetIdentifier, + $logger + ); + } catch (Exception $exception) { + // the pipeline shows the task log, so an uncaught exception would bury the reason under a stack trace + $logger->error($exception->getMessage()); + $this->quit(1); + } + } +} diff --git a/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php b/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php new file mode 100644 index 0000000..d13aa8c --- /dev/null +++ b/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php @@ -0,0 +1,225 @@ + + */ + #[Flow\InjectConfiguration('redisKeyPostfixesForEachRelease')] + protected array $redisKeyPostfixesForEachReleaseConfiguration; + + /** + * @throws Exception if the source release cannot be built upon, or the server is too old for COPY + */ + public function copyReleaseWithin( + RedisInstanceIdentifier $redisInstanceIdentifier, + ContentReleaseIdentifier $sourceContentReleaseIdentifier, + ContentReleaseIdentifier $targetContentReleaseIdentifier, + ContentReleaseLogger $contentReleaseLogger + ): void { + if ($sourceContentReleaseIdentifier->equals($targetContentReleaseIdentifier)) { + throw new InvalidReleaseException( + sprintf( + 'Cannot copy content release %s onto itself.', + $sourceContentReleaseIdentifier->getIdentifier() + ), 1786953585 + ); + } + + $redis = $this->redisClientManager->getRedis($redisInstanceIdentifier); + $this->assertServerSupportsCopy($redis); + $this->assertSourceReleaseCanBeBuiltUpon($redis, $redisInstanceIdentifier, $sourceContentReleaseIdentifier); + + $contentReleaseLogger->info( + sprintf( + 'Copying content release %s to %s within redis %s', + $sourceContentReleaseIdentifier->getIdentifier(), + $targetContentReleaseIdentifier->getIdentifier(), + $redisInstanceIdentifier->getIdentifier() + ) + ); + + $redisKeyPostfixesForEachRelease = RedisKeyPostfixesForEachRelease::fromArray( + $this->redisKeyPostfixesForEachReleaseConfiguration + ); + $startTime = microtime(true); + $copiedKeyCount = 0; + + foreach ($redisKeyPostfixesForEachRelease->getKeysToCopyOnQuickRelease() as $redisKeyPostfix) { + $sourceKey = $this->redisKeyService->getRedisKeyForPostfix( + $sourceContentReleaseIdentifier, + $redisKeyPostfix->getRedisKeyPostfix() + ); + $targetKey = $this->redisKeyService->getRedisKeyForPostfix( + $targetContentReleaseIdentifier, + $redisKeyPostfix->getRedisKeyPostfix() + ); + + if (!$redis->exists($sourceKey)) { + $contentReleaseLogger->info('COPY: Skipping ' . $sourceKey . ', as it does not exist.'); + continue; + } + + if ($redis->exists($targetKey)) { + $contentReleaseLogger->warn( + 'COPY: ' . $targetKey . ' already exists and is replaced - ' + . 'the release was copied into after something already wrote to it.' + ); + } + + $keyStartTime = microtime(true); + if ($redis->copy($sourceKey, $targetKey, ['replace' => true]) !== true) { + throw new InvalidReleaseException( + 'COPY: Could not copy ' . $sourceKey . ' to ' . $targetKey . '.', + 1786953586 + ); + } + $copiedKeyCount++; + + $contentReleaseLogger->info( + sprintf( + 'COPY: Copied key %s (time: %2.3f)', + $targetKey, + microtime(true) - $keyStartTime + ) + ); + } + + $contentReleaseLogger->info( + sprintf( + 'COPY: Copied %d keys from content release %s (total time: %2.3f)', + $copiedKeyCount, + $sourceContentReleaseIdentifier->getIdentifier(), + microtime(true) - $startTime + ) + ); + } + + /** + * @throws Exception + */ + private function assertServerSupportsCopy(Redis $redis): void + { + $serverInfo = $redis->info('server'); + $redisVersion = is_array($serverInfo) && array_key_exists('redis_version', $serverInfo) + ? (string)$serverInfo['redis_version'] + : ''; + + if ($redisVersion === '' || version_compare($redisVersion, self::MINIMUM_REDIS_VERSION, '<')) { + throw new Exception( + sprintf( + 'Copying a content release needs the redis COPY command, which requires redis %s or newer. ' + . 'This server reports version "%s".', + self::MINIMUM_REDIS_VERSION, + $redisVersion + ), 1786953587 + ); + } + } + + /** + * A quick release inherits everything it does not render itself, so a source release which never finished would + * be published as if it had. The status is checked rather than assumed because an administrator can switch to an + * arbitrary release by hand, so being the currently active release does not mean a release completed. + * + * @throws Exception + */ + private function assertSourceReleaseCanBeBuiltUpon( + Redis $redis, + RedisInstanceIdentifier $redisInstanceIdentifier, + ContentReleaseIdentifier $sourceContentReleaseIdentifier + ): void { + $metadata = $this->redisContentReleaseService->fetchMetadataForContentRelease( + $sourceContentReleaseIdentifier, + $redisInstanceIdentifier + ); + + if ($metadata === null) { + throw new InvalidReleaseException( + sprintf( + 'Content release %s does not exist in redis %s, so it cannot be copied. Run a full content release ' + . 'instead.', + $sourceContentReleaseIdentifier->getIdentifier(), + $redisInstanceIdentifier->getIdentifier() + ), 1786953588 + ); + } + + if (!$metadata->getStatus()->isSuccessful()) { + throw new InvalidReleaseException( + sprintf( + 'Content release %s has the status "%s" instead of "success", so it cannot be copied. Run a full ' + . 'content release instead.', + $sourceContentReleaseIdentifier->getIdentifier(), + $metadata->getStatus()->getStatus() + ), 1786953589 + ); + } + + $redisKeyPostfixesForEachRelease = RedisKeyPostfixesForEachRelease::fromArray( + $this->redisKeyPostfixesForEachReleaseConfiguration + ); + + // only the inherited keys have to exist - the rest is built by the quick release itself. isRequired alone is + // not enough of a filter: the other places reading it check it per transfer target, so a key an installation + // does not write at all is switched off there with "transfer: false" and stays required here. + foreach ($redisKeyPostfixesForEachRelease->getKeysToCopyOnQuickRelease() as $requiredPostfix) { + if (!$requiredPostfix->isRequired()) { + continue; + } + + $requiredKey = $this->redisKeyService->getRedisKeyForPostfix( + $sourceContentReleaseIdentifier, + $requiredPostfix->getRedisKeyPostfix() + ); + if (!$redis->exists($requiredKey)) { + throw new InvalidReleaseException( + sprintf( + 'Required redis key %s does not exist, so content release %s cannot be copied. Run a full ' + . 'content release instead.', + $requiredKey, + $sourceContentReleaseIdentifier->getIdentifier() + ), 1786953590 + ); + } + } + } +} diff --git a/Classes/Transfer/Dto/RedisKeyPostfixForEachRelease.php b/Classes/Transfer/Dto/RedisKeyPostfixForEachRelease.php index e412766..bafe7f8 100644 --- a/Classes/Transfer/Dto/RedisKeyPostfixForEachRelease.php +++ b/Classes/Transfer/Dto/RedisKeyPostfixForEachRelease.php @@ -20,15 +20,22 @@ final class RedisKeyPostfixForEachRelease protected array $transfer; protected string $transferMode; protected bool $isRequired; + protected bool $copyOnQuickRelease; /** * @param string $redisKeyPostfix * @param bool|array $transfer * @param string $transferMode * @param bool $isRequired + * @param bool $copyOnQuickRelease */ - private function __construct(string $redisKeyPostfix, $transfer, string $transferMode, bool $isRequired) - { + private function __construct( + string $redisKeyPostfix, + $transfer, + string $transferMode, + bool $isRequired, + bool $copyOnQuickRelease + ) { if (!in_array($transferMode, [self::TRANSFER_MODE_HASH_INCREMENTAL, self::TRANSFER_MODE_DUMP])) { throw new \RuntimeException('TransferMode ' . $transferMode . ' not supported.'); } @@ -44,11 +51,21 @@ private function __construct(string $redisKeyPostfix, $transfer, string $transfe $this->redisKeyPostfix = $redisKeyPostfix; $this->transferMode = $transferMode; $this->isRequired = $isRequired; + $this->copyOnQuickRelease = $copyOnQuickRelease; } public static function fromArray(array $in): self { - return new self($in['redisKeyPostfix'], $in['transfer'], $in['transferMode'], $in['isRequired']); + return new self( + $in['redisKeyPostfix'], + $in['transfer'], + $in['transferMode'], + $in['isRequired'], + // keys registered before quick releases existed do not carry the flag, and not copying them is the safe + // default: a key which should have been copied shows up as missing content, a key which should not have + // been copied describes a different release + $in['copyOnQuickRelease'] ?? false + ); } /** @@ -70,6 +87,11 @@ public function isRequired(): bool return $this->isRequired; } + public function shouldCopyOnQuickRelease(): bool + { + return $this->copyOnQuickRelease; + } + public function getRedisKeyPostfix(): string { return $this->redisKeyPostfix; diff --git a/Classes/Transfer/Dto/RedisKeyPostfixesForEachRelease.php b/Classes/Transfer/Dto/RedisKeyPostfixesForEachRelease.php index 900c2ce..3f098b4 100644 --- a/Classes/Transfer/Dto/RedisKeyPostfixesForEachRelease.php +++ b/Classes/Transfer/Dto/RedisKeyPostfixesForEachRelease.php @@ -47,6 +47,20 @@ public function getKeysToTransfer(RedisInstanceIdentifier $redisInstanceIdentifi } } + /** + * The keys a quick content release takes over from the release it is built on. + * + * @return iterable|RedisKeyPostfixForEachRelease[] + */ + public function getKeysToCopyOnQuickRelease(): iterable + { + foreach ($this->redisKeyPostfixes as $redisKeyPostfix) { + if ($redisKeyPostfix->shouldCopyOnQuickRelease()) { + yield $redisKeyPostfix; + } + } + } + /** * @return iterable|RedisKeyPostfixForEachRelease[] */ diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index a5d4d91..5c1a156 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -104,66 +104,85 @@ Flowpack: # transfer: # target_live: false # '*': true + # + # copyOnQuickRelease decides whether a quick content release takes this key over from the release it is built + # on, instead of producing it again. Set it for everything which carries content the consuming side reads, and + # leave it off for keys which describe the build of a single release - a quick release enumerates and renders + # only the documents which changed, so its own enumeration, queue and statistics have to stay its own. + # It defaults to `false`, so keys registered by a site package are never copied unless they opt in. data: redisKeyPostfix: 'data' transfer: true transferMode: 'hash_incremental' isRequired: true + copyOnQuickRelease: true enumerationDocumentNodes: redisKeyPostfix: 'enumeration:documentNodes' transfer: true transferMode: 'dump' isRequired: true + copyOnQuickRelease: false inProgressRenderings: redisKeyPostfix: 'inProgressRenderings' transfer: false transferMode: 'dump' isRequired: false + copyOnQuickRelease: false metainfo: redisKeyPostfix: 'meta:info' transfer: true transferMode: 'dump' isRequired: true + # written when the release is created, and it describes this release - never the one it was copied from + copyOnQuickRelease: false metaUrls: redisKeyPostfix: 'meta:urls' transfer: true transferMode: 'dump' isRequired: true + copyOnQuickRelease: true renderedDocuments: redisKeyPostfix: 'renderedDocuments' transfer: true transferMode: 'hash_incremental' isRequired: true + copyOnQuickRelease: true renderingErrors: redisKeyPostfix: 'renderingErrors' transfer: true transferMode: 'dump' isRequired: false + copyOnQuickRelease: false renderingJobQueue: redisKeyPostfix: 'renderingJobQueue' transfer: false transferMode: 'dump' isRequired: false + copyOnQuickRelease: false renderAttempts: redisKeyPostfix: 'renderAttempts' transfer: false transferMode: 'dump' isRequired: false + copyOnQuickRelease: false renderedMetadata: redisKeyPostfix: 'renderedMetadata' transfer: true transferMode: 'hash_incremental' isRequired: true + copyOnQuickRelease: true renderingStatistics: redisKeyPostfix: 'renderingStatistics' transfer: true transferMode: 'dump' isRequired: true + copyOnQuickRelease: false statisticsEvents: redisKeyPostfix: 'statisticsEvents' transfer: false transferMode: 'dump' isRequired: false + copyOnQuickRelease: false # can be used on the consuming site to ensure non-breaking deployments for changes in the config configEpoch: diff --git a/Configuration/Testing/Behat/Settings.yaml b/Configuration/Testing/Behat/Settings.yaml index 97915be..0bbff78 100644 --- a/Configuration/Testing/Behat/Settings.yaml +++ b/Configuration/Testing/Behat/Settings.yaml @@ -18,4 +18,6 @@ Flowpack: redisKeyPostfix: 'renderedMetadata' transfer: true transferMode: 'hash_incremental' - isRequired: true + # the key is written by documentMetadataGenerators, and the test setup registers none - so nothing that + # requires every registered key to exist could ever pass here + isRequired: false diff --git a/Resources/Public/BackendCompiled/out.css b/Resources/Public/BackendCompiled/out.css index bd407e1..d94deb7 100644 --- a/Resources/Public/BackendCompiled/out.css +++ b/Resources/Public/BackendCompiled/out.css @@ -1,4 +1,4 @@ -/* ../../../../../../../../private/var/folders/fd/vt4bk4351l97704fxm4zmmyr0000gn/T/tmp-30026-6RnCASmiDb7G/Flowpack.DecoupledContentStore/Resources/Private/Js/1786952756520-styles.css */ +/* ../../../../../../../../private/var/folders/fd/vt4bk4351l97704fxm4zmmyr0000gn/T/tmp-48963-VFAUMDKqBjLj/Flowpack.DecoupledContentStore/Resources/Private/Js/1786957409186-styles.css */ #app *, #app ::before, #app ::after { diff --git a/Tests/Behavior/Features/Bootstrap/FeatureContext.php b/Tests/Behavior/Features/Bootstrap/FeatureContext.php index 67336b9..035fd37 100644 --- a/Tests/Behavior/Features/Bootstrap/FeatureContext.php +++ b/Tests/Behavior/Features/Bootstrap/FeatureContext.php @@ -6,6 +6,9 @@ use Flowpack\DecoupledContentStore\Core\ConcurrentBuildLockService; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\PrunnerJobId; +use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; +use Flowpack\DecoupledContentStore\Exception as DecoupledContentStoreException; +use Flowpack\DecoupledContentStore\QuickPublish\Infrastructure\RedisReleaseCopyService; use Flowpack\DecoupledContentStore\Core\RedisKeyService; use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; @@ -173,6 +176,51 @@ public function iEnumerateAllNodesForContentRelease($contentReleaseIdentifier) echo $bufferedOutput->fetch(); } + /** + * @When I copy the content release :sourceContentReleaseIdentifier to the content release :targetContentReleaseIdentifier + */ + public function iCopyTheContentRelease($sourceContentReleaseIdentifier, $targetContentReleaseIdentifier) + { + $this->copyContentRelease($sourceContentReleaseIdentifier, $targetContentReleaseIdentifier); + } + + /** + * @Then copying the content release :sourceContentReleaseIdentifier to the content release :targetContentReleaseIdentifier is refused + */ + public function copyingTheContentReleaseIsRefused($sourceContentReleaseIdentifier, $targetContentReleaseIdentifier) + { + try { + $this->copyContentRelease($sourceContentReleaseIdentifier, $targetContentReleaseIdentifier); + } catch (DecoupledContentStoreException $exception) { + return; + } + + Assert::fail('Copying content release ' . $sourceContentReleaseIdentifier . ' should have been refused.'); + } + + private function copyContentRelease($sourceContentReleaseIdentifier, $targetContentReleaseIdentifier): void + { + $sourceContentReleaseIdentifier = ContentReleaseIdentifier::fromString($sourceContentReleaseIdentifier); + $targetContentReleaseIdentifier = ContentReleaseIdentifier::fromString($targetContentReleaseIdentifier); + $redisReleaseCopyService = $this->getObjectManager()->get(RedisReleaseCopyService::class); + $bufferedOutput = new BufferedOutput(); + $contentReleaseLogger = ContentReleaseLogger::fromSymfonyOutput( + $bufferedOutput, + $targetContentReleaseIdentifier + ); + + try { + $redisReleaseCopyService->copyReleaseWithin( + RedisInstanceIdentifier::primary(), + $sourceContentReleaseIdentifier, + $targetContentReleaseIdentifier, + $contentReleaseLogger + ); + } finally { + echo $bufferedOutput->fetch(); + } + } + /** * @Then the enumeration for content release :contentReleaseIdentifier contains :expectedCount node * @Then the enumeration for content release :contentReleaseIdentifier contains :expectedCount nodes diff --git a/Tests/Behavior/Features/ContentStore/QuickRelease.feature b/Tests/Behavior/Features/ContentStore/QuickRelease.feature new file mode 100644 index 0000000..44e8e28 --- /dev/null +++ b/Tests/Behavior/Features/ContentStore/QuickRelease.feature @@ -0,0 +1,56 @@ +@fixtures +@resetRedis +Feature: Quick Release + + Background: + Given I have the following NodeTypes configuration: + """ + Flowpack.DecoupledContentStore.Test:Document.StartPage: + superTypes: + 'Neos.Neos:Document': true + + Flowpack.DecoupledContentStore.Test:Content.Text: + superTypes: + 'Neos.Neos:Content': true + properties: + text: + type: string + + """ + Given I am authenticated with role "Neos.Neos:Editor" + Given I have a site for Site Node "test" with site package key "Flowpack.DecoupledContentStore" with domain "test.de" + And I have the following nodes: + | Path | Node Type | Properties | HiddenInIndex | Language | + | /sites | unstructured | [] | false | de | + | /sites/test | Flowpack.DecoupledContentStore.Test:Document.StartPage | {"title":"Startseite","uriPathSegment":"startseite"} | false | de | + | /sites/test/main | Neos.Neos:ContentCollection | {} | false | de | + | /sites/test/main/t1 | Flowpack.DecoupledContentStore.Test:Content.Text | {"text": "Hallo - this is rendered."} | false | de | + And I flush the content cache depending on the modified nodes + + Scenario: A finished release is copied forward instead of being rendered again + When I create a content release "5" + And I enumerate all nodes for content release "5" + And I run the render-orchestrator control loop once for content release "5" + And I run the renderer for content release "5" until the queue is empty + Then during rendering of content release "5", no errors occured + When I continue running the render-orchestrator control loop + Then I expect the render-orchestrator control loop to exit with status code 0 + And I expect the content release "5" to have the completion status success + + # nothing is rendered for the new release, and it holds the content of its predecessor anyway + When I create a content release "6" + And I copy the content release "5" to the content release "6" + Then I expect the content release "6" to contain the following content for URI "http://test.de/de" at CSS selector "body .neos-contentcollection": + """ + BEFOREHallo - this is rendered.AFTER + """ + # the enumeration says what a release renders, so a quick release brings its own instead of inheriting one + And the enumeration for content release "6" contains 0 nodes + + Scenario: A release which has not finished is not copied forward + # everything the copy does not overwrite is published as if it had been rendered, so an unfinished release + # must not be built upon + When I create a content release "5" + And I enumerate all nodes for content release "5" + When I create a content release "6" + Then copying the content release "5" to the content release "6" is refused diff --git a/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php b/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php index c266bdb..199456b 100644 --- a/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php +++ b/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php @@ -9,8 +9,7 @@ use Flowpack\DecoupledContentStore\Core\AutomaticReleaseSwitchService; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\AutomaticReleasePauseState; use Neos\Flow\I18n\Translator; -use PHPUnit\Framework\TestCase; -use ReflectionProperty; +use Neos\Flow\Tests\UnitTestCase; /** * Tests the payload the content module warning is painted from. @@ -18,7 +17,7 @@ * Both the identifier and the field names are a contract with * Resources/Public/ContentModule/AutomaticReleaseWarning.js, which has no way of noticing that they changed. */ -final class AutomaticReleaseStatusDataSourceTest extends TestCase +final class AutomaticReleaseStatusDataSourceTest extends UnitTestCase { public function testTheIdentifierIsTheOneTheContentModuleScriptRequests(): void { @@ -86,16 +85,10 @@ private function buildDataSource( $backendDateFormatter->method('format')->willReturn('formatted date'); $dataSource = new AutomaticReleaseStatusDataSource(); - // the class uses Flow property injection, which is not available outside a Flow bootstrap - self::injectDependency($dataSource, 'automaticReleaseSwitchService', $automaticReleaseSwitchService); - self::injectDependency($dataSource, 'translator', $translator); - self::injectDependency($dataSource, 'backendDateFormatter', $backendDateFormatter); + $this->inject($dataSource, 'automaticReleaseSwitchService', $automaticReleaseSwitchService); + $this->inject($dataSource, 'translator', $translator); + $this->inject($dataSource, 'backendDateFormatter', $backendDateFormatter); return $dataSource; } - - private static function injectDependency(object $target, string $propertyName, object $dependency): void - { - (new ReflectionProperty($target, $propertyName))->setValue($target, $dependency); - } } diff --git a/Tests/Unit/ContentReleaseManagerTest.php b/Tests/Unit/ContentReleaseManagerTest.php index d9edd91..4015562 100644 --- a/Tests/Unit/ContentReleaseManagerTest.php +++ b/Tests/Unit/ContentReleaseManagerTest.php @@ -10,10 +10,9 @@ use Flowpack\Prunner\PrunnerApiService; use Flowpack\Prunner\ValueObject\JobId; use Neos\Flow\Security\Context; +use Neos\Flow\Tests\UnitTestCase; use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; -use ReflectionProperty; /** * Tests the effect of the pause switch on scheduling. @@ -22,7 +21,7 @@ * startIncrementalContentRelease(), so that method is the only gate. "Publish All" is an explicit request and must * stay unaffected - otherwise the pause could not be used to prepare a release by hand. */ -class ContentReleaseManagerTest extends TestCase +class ContentReleaseManagerTest extends UnitTestCase { private PrunnerApiService&MockObject $prunnerApiService; @@ -81,22 +80,12 @@ private function buildContentReleaseManager(): ContentReleaseManager $securityContext->method('isInitialized')->willReturn(false); $contentReleaseManager = new ContentReleaseManager(); - self::injectDependency($contentReleaseManager, 'prunnerApiService', $this->prunnerApiService); - self::injectDependency( - $contentReleaseManager, - 'automaticReleaseSwitchService', - $this->automaticReleaseSwitchService - ); - self::injectDependency($contentReleaseManager, 'redisClientManager', $redisClientManager); - self::injectDependency($contentReleaseManager, 'securityContext', $securityContext); - self::injectDependency($contentReleaseManager, 'logger', new NullLogger()); + $this->inject($contentReleaseManager, 'prunnerApiService', $this->prunnerApiService); + $this->inject($contentReleaseManager, 'automaticReleaseSwitchService', $this->automaticReleaseSwitchService); + $this->inject($contentReleaseManager, 'redisClientManager', $redisClientManager); + $this->inject($contentReleaseManager, 'securityContext', $securityContext); + $this->inject($contentReleaseManager, 'logger', new NullLogger()); return $contentReleaseManager; } - - private static function injectDependency(object $target, string $propertyName, object $dependency): void - { - // the class uses Flow property injection, which is not available outside a Flow bootstrap - (new ReflectionProperty($target, $propertyName))->setValue($target, $dependency); - } } diff --git a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php index 93032c0..630b8ca 100644 --- a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php +++ b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php @@ -7,9 +7,8 @@ use Flowpack\DecoupledContentStore\Core\AutomaticReleaseSwitchService; use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; use Neos\Flow\Security\Context; +use Neos\Flow\Tests\UnitTestCase; use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; -use ReflectionProperty; /** * Tests the switch which suppresses automatically triggered content releases. @@ -17,7 +16,7 @@ * The state lives in a single Redis hash, so the interesting behaviour is which field decides that the switch is * set, and that pausing twice does not wipe what the first pause recorded. */ -final class AutomaticReleaseSwitchServiceTest extends TestCase +final class AutomaticReleaseSwitchServiceTest extends UnitTestCase { private const REDIS_KEY = 'contentStore:automaticReleasesPaused'; @@ -98,15 +97,9 @@ private function buildService(\Redis $redis): AutomaticReleaseSwitchService $securityContext->method('isInitialized')->willReturn(false); $service = new AutomaticReleaseSwitchService(); - self::injectDependency($service, 'redisClientManager', $redisClientManager); - self::injectDependency($service, 'securityContext', $securityContext); + $this->inject($service, 'redisClientManager', $redisClientManager); + $this->inject($service, 'securityContext', $securityContext); return $service; } - - private static function injectDependency(object $target, string $propertyName, object $dependency): void - { - // the class uses Flow property injection, which is not available outside a Flow bootstrap - (new ReflectionProperty($target, $propertyName))->setValue($target, $dependency); - } } diff --git a/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php b/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php index 5eaa2f1..71c3b36 100644 --- a/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php +++ b/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php @@ -5,13 +5,13 @@ namespace Flowpack\DecoupledContentStore\Tests\Unit\Core\Domain\ValueObject; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\AutomaticReleasePauseState; -use PHPUnit\Framework\TestCase; +use Neos\Flow\Tests\UnitTestCase; /** * Tests the mapping of the "contentStore:automaticReleasesPaused" Redis hash onto the pause state shown in the * backend module. Everything arrives as a string, and only "pausedAt" is guaranteed to be there. */ -final class AutomaticReleasePauseStateTest extends TestCase +final class AutomaticReleasePauseStateTest extends UnitTestCase { public function testAllFieldsAreReadFromTheHash(): void { diff --git a/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php b/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php new file mode 100644 index 0000000..d374bea --- /dev/null +++ b/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php @@ -0,0 +1,215 @@ + + */ + private array $copiedKeys = []; + + public function testOnlyTheFlaggedKeysAreCopied(): void + { + $this->copyRelease($this->buildRedis(), $this->buildRedisContentReleaseService()); + + // renderingJobQueue exists on the source, but describes the build of that release rather than its content + self::assertSame([ + ['contentStore:5:data', 'contentStore:6:data'], + ['contentStore:5:meta:urls', 'contentStore:6:meta:urls'], + ], $this->copiedKeys); + } + + public function testAKeyWhichDoesNotExistOnTheSourceIsSkipped(): void + { + $redis = $this->buildRedis(['contentStore:5:data', 'contentStore:5:meta:urls']); + + $this->copyRelease($redis, $this->buildRedisContentReleaseService()); + + self::assertCount(2, $this->copiedKeys); + } + + public function testCopyingIsRefusedOnAServerWithoutTheCopyCommand(): void + { + $redis = $this->buildRedis(self::SOURCE_KEYS, '6.0.20'); + + $this->expectException(Exception::class); + $this->expectExceptionCode(1786953587); + + $this->copyRelease($redis, $this->buildRedisContentReleaseService()); + } + + public function testAReleaseWhichDidNotFinishIsNotCopied(): void + { + // the source release is the one which is currently live, and an administrator can switch to any release + $redisContentReleaseService = $this->buildRedisContentReleaseService(NodeRenderingCompletionStatus::running()); + + $this->expectException(Exception::class); + $this->expectExceptionCode(1786953589); + + $this->copyRelease($this->buildRedis(), $redisContentReleaseService); + } + + public function testAReleaseWhichDoesNotExistIsNotCopied(): void + { + $redisContentReleaseService = $this->createMock(RedisContentReleaseService::class); + $redisContentReleaseService->method('fetchMetadataForContentRelease')->willReturn(null); + + $this->expectException(Exception::class); + $this->expectExceptionCode(1786953588); + + $this->copyRelease($this->buildRedis(), $redisContentReleaseService); + } + + public function testAReleaseMissingOneOfItsRequiredKeysIsNotCopied(): void + { + $redis = $this->buildRedis(['contentStore:5:data']); + + $this->expectException(Exception::class); + $this->expectExceptionCode(1786953590); + + $this->copyRelease($redis, $this->buildRedisContentReleaseService()); + } + + public function testARequiredKeyWhichIsNotCopiedMayBeMissingOnTheSource(): void + { + // enumeration:documentNodes is required, but a quick release enumerates the given nodes itself - and a key + // an installation does not write at all is registered as required as well + $this->copyRelease($this->buildRedis(), $this->buildRedisContentReleaseService()); + + self::assertCount(2, $this->copiedKeys); + } + + public function testAReleaseIsNotCopiedOntoItself(): void + { + $this->expectException(Exception::class); + $this->expectExceptionCode(1786953585); + + $this->copyRelease($this->buildRedis(), $this->buildRedisContentReleaseService(), '5'); + } + + private function copyRelease( + \Redis $redis, + RedisContentReleaseService $redisContentReleaseService, + string $targetContentReleaseIdentifier = '6' + ): void { + $redisClientManager = $this->createMock(RedisClientManager::class); + $redisClientManager->method('getRedis')->willReturn($redis); + + $redisKeyService = new RedisKeyService(); + $this->inject($redisKeyService, 'redisKeyPostfixesForEachReleaseConfiguration', self::keyConfiguration()); + + $service = new RedisReleaseCopyService(); + $this->inject($service, 'redisClientManager', $redisClientManager); + $this->inject($service, 'redisKeyService', $redisKeyService); + $this->inject($service, 'redisContentReleaseService', $redisContentReleaseService); + $this->inject($service, 'redisKeyPostfixesForEachReleaseConfiguration', self::keyConfiguration()); + + $service->copyReleaseWithin( + RedisInstanceIdentifier::primary(), + ContentReleaseIdentifier::fromString('5'), + ContentReleaseIdentifier::fromString($targetContentReleaseIdentifier), + ContentReleaseLogger::fromSymfonyOutput(new BufferedOutput(), ContentReleaseIdentifier::fromString('6')) + ); + } + + /** + * @param array $existingKeys + * @return \Redis&MockObject + */ + private function buildRedis(array $existingKeys = self::SOURCE_KEYS, string $redisVersion = '7.2.4'): \Redis + { + $redis = $this->createMock(\Redis::class); + $redis->method('info')->willReturn(['redis_version' => $redisVersion]); + $redis->method('exists')->willReturnCallback( + static fn(string $key): int => in_array($key, $existingKeys, true) ? 1 : 0 + ); + $redis->method('copy')->willReturnCallback(function (string $sourceKey, string $targetKey): bool { + $this->copiedKeys[] = [$sourceKey, $targetKey]; + return true; + }); + + return $redis; + } + + /** + * @return RedisContentReleaseService&MockObject + */ + private function buildRedisContentReleaseService( + ?NodeRenderingCompletionStatus $status = null + ): RedisContentReleaseService { + $metadata = ContentReleaseMetadata::create(PrunnerJobId::fromString('job'), new \DateTimeImmutable()) + ->withStatus($status ?? NodeRenderingCompletionStatus::success()); + + $redisContentReleaseService = $this->createMock(RedisContentReleaseService::class); + $redisContentReleaseService->method('fetchMetadataForContentRelease')->willReturn($metadata); + + return $redisContentReleaseService; + } + + /** + * @return array> + */ + private static function keyConfiguration(): array + { + return [ + 'data' => [ + 'redisKeyPostfix' => 'data', + 'transfer' => true, + 'transferMode' => 'hash_incremental', + 'isRequired' => true, + 'copyOnQuickRelease' => true, + ], + 'metaUrls' => [ + 'redisKeyPostfix' => 'meta:urls', + 'transfer' => true, + 'transferMode' => 'dump', + 'isRequired' => true, + 'copyOnQuickRelease' => true, + ], + 'renderingJobQueue' => [ + 'redisKeyPostfix' => 'renderingJobQueue', + 'transfer' => false, + 'transferMode' => 'dump', + 'isRequired' => false, + 'copyOnQuickRelease' => false, + ], + 'enumerationDocumentNodes' => [ + 'redisKeyPostfix' => 'enumeration:documentNodes', + 'transfer' => true, + 'transferMode' => 'dump', + 'isRequired' => true, + 'copyOnQuickRelease' => false + ] + ]; + } +} diff --git a/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php b/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php new file mode 100644 index 0000000..fc1bf37 --- /dev/null +++ b/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php @@ -0,0 +1,64 @@ + self::keyConfiguration('renderedDocuments', true), + 'renderingJobQueue' => self::keyConfiguration('renderingJobQueue', false), + 'metaUrls' => self::keyConfiguration('meta:urls', true), + ]); + + self::assertSame(['renderedDocuments', 'meta:urls'], self::copiedPostfixes($redisKeyPostfixes)); + } + + public function testAKeyWhichDoesNotKnowAboutQuickReleasesIsNotCopied(): void + { + // site packages register their own keys, and those configurations predate quick releases + $configurationWithoutTheFlag = self::keyConfiguration('renderedDocuments', true); + unset($configurationWithoutTheFlag['copyOnQuickRelease']); + + $redisKeyPostfixes = RedisKeyPostfixesForEachRelease::fromArray([ + 'renderedDocuments' => $configurationWithoutTheFlag, + ]); + + self::assertSame([], self::copiedPostfixes($redisKeyPostfixes)); + } + + /** + * @return array + */ + private static function copiedPostfixes(RedisKeyPostfixesForEachRelease $redisKeyPostfixes): array + { + $result = []; + foreach ($redisKeyPostfixes->getKeysToCopyOnQuickRelease() as $redisKeyPostfix) { + $result[] = $redisKeyPostfix->getRedisKeyPostfix(); + } + return $result; + } + + /** + * @return array + */ + private static function keyConfiguration(string $redisKeyPostfix, bool $copyOnQuickRelease): array + { + return [ + 'redisKeyPostfix' => $redisKeyPostfix, + 'transfer' => true, + 'transferMode' => 'dump', + 'isRequired' => true, + 'copyOnQuickRelease' => $copyOnQuickRelease, + ]; + } +} From f90e70fa229db4c3657aae623579be79df56bd4f Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 11:42:41 +0200 Subject: [PATCH 04/23] STEP 4: add quick publish enumeration --- ...ntReleaseQuickPublishCommandController.php | 29 +++ .../Domain/Service/DocumentNodeFilter.php | 179 ++++++++++++++ Classes/NodeEnumeration/NodeEnumerator.php | 93 ++----- Classes/QuickPublish/Dto/NodeIdentifiers.php | 93 +++++++ .../QuickPublishNodeEnumerator.php | 226 ++++++++++++++++++ .../Features/Bootstrap/FeatureContext.php | 63 +++++ .../ContentStore/QuickRelease.feature | 9 + .../Service/DocumentNodeFilterTest.php} | 8 +- .../QuickPublish/Dto/NodeIdentifiersTest.php | 96 ++++++++ phpstan-baseline.neon | 18 -- 10 files changed, 718 insertions(+), 96 deletions(-) create mode 100644 Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php create mode 100644 Classes/QuickPublish/Dto/NodeIdentifiers.php create mode 100644 Classes/QuickPublish/QuickPublishNodeEnumerator.php rename Tests/Unit/NodeEnumeration/{NodeEnumeratorTest.php => Domain/Service/DocumentNodeFilterTest.php} (93%) create mode 100644 Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php diff --git a/Classes/Command/ContentReleaseQuickPublishCommandController.php b/Classes/Command/ContentReleaseQuickPublishCommandController.php index 020a504..3bb080e 100644 --- a/Classes/Command/ContentReleaseQuickPublishCommandController.php +++ b/Classes/Command/ContentReleaseQuickPublishCommandController.php @@ -8,7 +8,9 @@ use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; use Flowpack\DecoupledContentStore\Exception; +use Flowpack\DecoupledContentStore\QuickPublish\Dto\NodeIdentifiers; use Flowpack\DecoupledContentStore\QuickPublish\Infrastructure\RedisReleaseCopyService; +use Flowpack\DecoupledContentStore\QuickPublish\QuickPublishNodeEnumerator; use Neos\Flow\Annotations as Flow; use Neos\Flow\Cli\CommandController; @@ -20,6 +22,9 @@ final class ContentReleaseQuickPublishCommandController extends CommandControlle #[Flow\Inject] protected RedisReleaseCopyService $redisReleaseCopyService; + #[Flow\Inject] + protected QuickPublishNodeEnumerator $quickPublishNodeEnumerator; + /** * Take the content of a finished content release over into a new one, within the same redis instance. * @@ -50,4 +55,28 @@ public function copyReleaseWithinCommand( $this->quit(1); } } + + /** + * Enumerate the given document nodes for rendering. Everything else in the release comes from the release it + * was copied from. + * + * @param string $contentReleaseIdentifier the release being built + * @param string $nodeIdentifiers the node identifiers to publish, separated by commas + */ + public function enumerateGivenNodesCommand(string $contentReleaseIdentifier, string $nodeIdentifiers): void + { + $releaseIdentifier = ContentReleaseIdentifier::fromString($contentReleaseIdentifier); + $logger = ContentReleaseLogger::fromConsoleOutput($this->output, $releaseIdentifier); + + try { + $this->quickPublishNodeEnumerator->enumerateGivenNodesAndStoreInRedis( + NodeIdentifiers::fromCommaSeparatedString($nodeIdentifiers), + $logger, + $releaseIdentifier + ); + } catch (Exception $exception) { + $logger->error($exception->getMessage()); + $this->quit(1); + } + } } diff --git a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php new file mode 100644 index 0000000..03192fc --- /dev/null +++ b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php @@ -0,0 +1,179 @@ + + */ + #[Flow\InjectConfiguration('nodeRendering.nodeTypeWhitelist')] + protected array $nodeTypeWhitelist; + + /** + * Builds a FlowQuery filter string from the node type whitelist, + * where entries prefixed with "!" are excluded. + * + * The filter parts must be concatenated without a separator: FlowQuery parses + * comma-separated filter groups independently, and a group consisting only of + * "[!instanceof ...]" makes find() throw "find() needs an identifier, path or + * instanceof filter for the first filter part" (exception 1436884196). For the same + * reason, the positive "[instanceof ...]" filters are put first. + * + * If the whitelist configures exclusions only, the default node type is used as the + * positive filter - otherwise find() would run into the very same exception. + */ + public function flowQueryNodeTypeFilter(): string + { + return self::buildNodeTypeFilter($this->nodeTypeWhitelist); + } + + /** + * Whether a single node passes the very filter {@see flowQueryNodeTypeFilter()} expresses. + * + * Concatenated FlowQuery filter parts are ANDed, so a node has to be of every included node type - not of any + * of them. + */ + public function matchesNodeTypeWhitelist(NodeInterface $node): bool + { + $nodeType = $node->getNodeType(); + $partitionedNodeTypes = self::partitionNodeTypeWhitelist($this->nodeTypeWhitelist); + + foreach ($partitionedNodeTypes['excludes'] as $excludedNodeType) { + if ($nodeType->isOfType($excludedNodeType)) { + return false; + } + } + + foreach ($partitionedNodeTypes['includes'] as $includedNodeType) { + if (!$nodeType->isOfType($includedNodeType)) { + return false; + } + } + + return true; + } + + /** + * Why a node must not go into a content release, for the log - or NULL if it may. + * + * The node type is deliberately not checked here: the full enumeration adds the site node to its result without + * passing it through the FlowQuery filter, and both enumerators would change behaviour if that were tightened + * as a side effect. Callers which resolve nodes by identifier check {@see matchesNodeTypeWhitelist()} themselves. + */ + public function skipReason(NodeInterface $node, NodeInterface $siteNode): ?string + { + // the site node has no parent inside the site, so it can never be orphaned + if ($node !== $siteNode && self::isOrphaned($node, $siteNode)) { + return 'orphaned'; + } + + if ($node->isHidden()) { + return 'hidden'; + } + + return null; + } + + private static function isOrphaned(NodeInterface $node, NodeInterface $siteNode): bool + { + $parentNode = self::getParentNodeOrNull($node); + while ($parentNode !== $siteNode) { + if ($parentNode === null) { + return true; + } + $parentNode = self::getParentNodeOrNull($parentNode); + } + + return false; + } + + /** + * The parent of a node, or NULL where the walk up leaves the tree. + * + * Walking up needs findParentNode(), which belongs to TraversableNodeInterface - a different interface from the + * NodeInterface the rest of this class works with, implemented side by side by the content repository's node + * class. Both conversions live here so that callers compare nodes of one type, and so that "left the tree" + * stays a NULL rather than an exception. + */ + private static function getParentNodeOrNull(NodeInterface $node): ?NodeInterface + { + if (!$node instanceof TraversableNodeInterface) { + return null; + } + + try { + $parentNode = $node->findParentNode(); + } catch (NodeException) { + return null; + } + + return $parentNode instanceof NodeInterface ? $parentNode : null; + } + + /** + * @param array $nodeTypeWhitelist + */ + private static function buildNodeTypeFilter(array $nodeTypeWhitelist): string + { + $partitionedNodeTypes = self::partitionNodeTypeWhitelist($nodeTypeWhitelist); + + $filterParts = []; + foreach ($partitionedNodeTypes['includes'] as $includedNodeType) { + $filterParts[] = '[instanceof ' . $includedNodeType . ']'; + } + foreach ($partitionedNodeTypes['excludes'] as $excludedNodeType) { + $filterParts[] = '[!instanceof ' . $excludedNodeType . ']'; + } + + return implode('', $filterParts); + } + + /** + * @param array $nodeTypeWhitelist + * @return array{includes: array, excludes: array} + */ + private static function partitionNodeTypeWhitelist(array $nodeTypeWhitelist): array + { + $includes = []; + $excludes = []; + + foreach ($nodeTypeWhitelist as $nodeType) { + $nodeType = trim($nodeType); + if ($nodeType === '') { + continue; + } + if ($nodeType[0] === '!') { + $excludes[] = substr($nodeType, 1); + continue; + } + $includes[] = $nodeType; + } + + if ($includes === []) { + $includes[] = self::DEFAULT_NODE_TYPE; + } + + return ['includes' => $includes, 'excludes' => $excludes]; + } +} diff --git a/Classes/NodeEnumeration/NodeEnumerator.php b/Classes/NodeEnumeration/NodeEnumerator.php index 3d0db8c..6a72c52 100644 --- a/Classes/NodeEnumeration/NodeEnumerator.php +++ b/Classes/NodeEnumeration/NodeEnumerator.php @@ -10,6 +10,7 @@ use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Dto\EnumeratedNode; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Repository\RedisEnumerationRepository; +use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\DocumentNodeFilter; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\NodeContextCombinator; use Flowpack\DecoupledContentStore\NodeRendering\Dto\NodeRenderingCompletionStatus; use Flowpack\DecoupledContentStore\NodeRendering\Extensibility\NodeRenderingExtensionManager; @@ -23,10 +24,8 @@ class NodeEnumerator { - /** - * Used when "nodeRendering.nodeTypeWhitelist" configures no node type to include. - */ - private const DEFAULT_NODE_TYPE = 'Neos.Neos:Document'; + #[Flow\Inject] + protected DocumentNodeFilter $documentNodeFilter; /** * @Flow\Inject @@ -52,12 +51,6 @@ class NodeEnumerator */ protected $nodeRenderingExtensionManager; - /** - * @Flow\InjectConfiguration("nodeRendering.nodeTypeWhitelist") - * @var array - */ - protected $nodeTypeWhitelist; - public function enumerateAndStoreInRedis( ?Site $site, ContentReleaseLogger $contentReleaseLogger, @@ -93,40 +86,6 @@ public function enumerateAndStoreInRedis( } } - /** - * Builds a FlowQuery filter string from the node type whitelist, - * where entries prefixed with "!" are excluded. - * - * The filter parts must be concatenated without a separator: FlowQuery parses - * comma-separated filter groups independently, and a group consisting only of - * "[!instanceof ...]" makes find() throw "find() needs an identifier, path or - * instanceof filter for the first filter part" (exception 1436884196). For the same - * reason, the positive "[instanceof ...]" filters are put first. - * - * If the whitelist configures exclusions only, the default node type is used as the - * positive filter - otherwise find() would run into the very same exception. - */ - private static function buildNodeTypeFilter(array $nodeTypeWhitelist): string - { - $includes = []; - $excludes = []; - foreach ($nodeTypeWhitelist as $nodeType) { - $nodeType = trim($nodeType); - if ($nodeType === '') { - continue; - } - if ($nodeType[0] === '!') { - $excludes[] = '[!instanceof ' . substr($nodeType, 1) . ']'; - continue; - } - $includes[] = '[instanceof ' . $nodeType . ']'; - } - if ($includes === []) { - $includes[] = '[instanceof ' . self::DEFAULT_NODE_TYPE . ']'; - } - return implode('', array_merge($includes, $excludes)); - } - /** * @return iterable * @throws Exception @@ -138,8 +97,7 @@ private function enumerateAll( ): iterable { $combinator = new NodeContextCombinator(); - // an empty whitelist falls back to the default node type in buildNodeTypeFilter() - $nodeTypeFilter = self::buildNodeTypeFilter($this->nodeTypeWhitelist); + $nodeTypeFilter = $this->documentNodeFilter->flowQueryNodeTypeFilter(); $queueSite = function (Site $site) use ($combinator, $nodeTypeFilter, $contentReleaseLogger, $workspaceName) { $contentReleaseLogger->debug('Publishing site', [ @@ -162,36 +120,23 @@ private function enumerateAll( foreach ($matchingNodes as $nodeToEnumerate) { $contextPath = $nodeToEnumerate->getContextPath(); - // BUGFIX: the site node has no parent but must NOT be recognized as orphaned - if ($nodeToEnumerate !== $siteNode) { - // Verify that the node is not orphaned - $parentNode = $nodeToEnumerate->getParent(); - while ($parentNode !== $siteNode) { - if ($parentNode === null) { - $contentReleaseLogger->debug('Skipping node from publishing, because it is orphaned', [ - 'node' => $contextPath - ]); - // Continue with the next document - continue 2; - } - $parentNode = $parentNode->getParent(); - } + $skipReason = $this->documentNodeFilter->skipReason($nodeToEnumerate, $siteNode); + if ($skipReason !== null) { + $contentReleaseLogger->debug( + 'Skipping node from publishing, because it is ' . $skipReason, + ['node' => $contextPath] + ); + continue; } - if ($nodeToEnumerate->isHidden()) { - $contentReleaseLogger->debug('Skipping node from publishing, because it is hidden', [ - 'node' => $contextPath - ]); - } else { - $contentReleaseLogger->debug('Registering node for publishing', [ - 'node' => $contextPath - ]); - - foreach ($this->nodeRenderingExtensionManager->enumerateDocumentNode( - $nodeToEnumerate - ) as $enumeratedNode) { - yield $enumeratedNode; - } + $contentReleaseLogger->debug('Registering node for publishing', [ + 'node' => $contextPath + ]); + + foreach ($this->nodeRenderingExtensionManager->enumerateDocumentNode( + $nodeToEnumerate + ) as $enumeratedNode) { + yield $enumeratedNode; } } } diff --git a/Classes/QuickPublish/Dto/NodeIdentifiers.php b/Classes/QuickPublish/Dto/NodeIdentifiers.php new file mode 100644 index 0000000..c788644 --- /dev/null +++ b/Classes/QuickPublish/Dto/NodeIdentifiers.php @@ -0,0 +1,93 @@ + + */ +#[Flow\Proxy(false)] +final class NodeIdentifiers implements \IteratorAggregate, \JsonSerializable +{ + private const IDENTIFIER_PATTERN = '/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i'; + + /** + * @var array + */ + private array $identifiers; + + /** + * @param array $identifiers + */ + private function __construct(array $identifiers) + { + $this->identifiers = $identifiers; + } + + /** + * @throws Exception if the list is empty or holds anything which is not a node identifier + */ + public static function fromCommaSeparatedString(string $nodeIdentifiers): self + { + $identifiers = []; + + foreach (explode(',', $nodeIdentifiers) as $identifier) { + $identifier = trim($identifier); + if ($identifier === '') { + continue; + } + if (preg_match(self::IDENTIFIER_PATTERN, $identifier) !== 1) { + throw new Exception( + sprintf('"%s" is not a node identifier.', $identifier), + 1786958510 + ); + } + // an identifier given twice would be rendered twice + if (!in_array($identifier, $identifiers, true)) { + $identifiers[] = $identifier; + } + } + + if ($identifiers === []) { + throw new Exception('No node identifiers given.', 1786958511); + } + + return new self($identifiers); + } + + /** + * @return \Traversable + */ + public function getIterator(): \Traversable + { + yield from $this->identifiers; + } + + /** + * @return array + */ + public function jsonSerialize(): array + { + return $this->identifiers; + } + + public function count(): int + { + return count($this->identifiers); + } + + public function __toString(): string + { + return implode(',', $this->identifiers); + } +} diff --git a/Classes/QuickPublish/QuickPublishNodeEnumerator.php b/Classes/QuickPublish/QuickPublishNodeEnumerator.php new file mode 100644 index 0000000..4d114b8 --- /dev/null +++ b/Classes/QuickPublish/QuickPublishNodeEnumerator.php @@ -0,0 +1,226 @@ +info('Starting quick content release', [ + 'contentReleaseIdentifier' => $releaseIdentifier->jsonSerialize(), + 'nodeIdentifiers' => $nodeIdentifiers->jsonSerialize() + ]); + + $currentMetadata = $this->redisContentReleaseService->fetchMetadataForContentRelease($releaseIdentifier); + if ($currentMetadata === null) { + throw new InvalidReleaseException( + sprintf( + 'Content release %s does not exist, so its nodes cannot be enumerated.', + $releaseIdentifier->getIdentifier() + ), 1786958512 + ); + } + + $newMetadata = $currentMetadata->withStatus(NodeRenderingCompletionStatus::running()); + $this->redisContentReleaseService->setContentReleaseMetadata( + $releaseIdentifier, + $newMetadata, + RedisInstanceIdentifier::primary() + ); + + $this->redisEnumerationRepository->clearDocumentNodesEnumeration($releaseIdentifier); + + $enumeratedNodeCount = 0; + foreach ( + GeneratorUtility::createArrayBatch( + $this->enumerateGivenNodes( + $nodeIdentifiers, + $contentReleaseLogger, + $newMetadata->getWorkspaceName() ?? 'live' + ), + 100 + ) as $enumeration + ) { + $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($releaseIdentifier); + $this->redisEnumerationRepository->addDocumentNodesToEnumeration($releaseIdentifier, ...$enumeration); + $enumeratedNodeCount += count($enumeration); + } + + // a quick release which renders nothing publishes exactly the release it was copied from - which looks like + // a successful publish to everybody watching, while the change the editor asked for is nowhere + if ($enumeratedNodeCount === 0) { + throw new Exception( + sprintf( + 'None of the given nodes can be published (%s), so content release %s would only repeat the release ' + . 'it was built on.', + (string)$nodeIdentifiers, + $releaseIdentifier->getIdentifier() + ), 1786958513 + ); + } + + $contentReleaseLogger->info(sprintf('Enumerated %d node variants for rendering', $enumeratedNodeCount)); + } + + /** + * @return iterable + * @throws Exception + */ + private function enumerateGivenNodes( + NodeIdentifiers $nodeIdentifiers, + ContentReleaseLogger $contentReleaseLogger, + string $workspaceName + ): iterable { + foreach ($nodeIdentifiers as $nodeIdentifier) { + $contentCacheFlushed = false; + + foreach ($this->nodeVariants($nodeIdentifier, $workspaceName) as [$siteNode, $nodeToEnumerate]) { + if (!$contentCacheFlushed) { + // the tags carry the node identifier and the workspace, so one flush covers every dimension + $this->flushContentCacheForNode($nodeToEnumerate, $nodeIdentifier, $contentReleaseLogger); + $contentCacheFlushed = true; + } + + $contextPath = $nodeToEnumerate->getContextPath(); + + $skipReason = $this->documentNodeFilter->skipReason($nodeToEnumerate, $siteNode); + if ($skipReason === null && !$this->documentNodeFilter->matchesNodeTypeWhitelist($nodeToEnumerate)) { + $skipReason = 'not of a node type which is published'; + } + if ($skipReason !== null) { + // warn rather than debug: somebody asked for this node by hand and will not see it change + $contentReleaseLogger->warn( + 'Skipping node from publishing, because it is ' . $skipReason, + ['node' => $contextPath] + ); + continue; + } + + $contentReleaseLogger->info('Registering node for publishing', ['node' => $contextPath]); + + yield from $this->nodeRenderingExtensionManager->enumerateDocumentNode($nodeToEnumerate); + } + + if (!$contentCacheFlushed) { + throw new NodeNotFoundException( + sprintf( + 'Could not find node %s in any site and dimension, so it cannot be published.', + $nodeIdentifier + ), 1786958514 + ); + } + } + } + + /** + * The node in every dimension it exists in, together with the site node it belongs to. + * + * {@see NodeContextCombinator::nodeInContexts()} hands out the same variants, but not the site node the orphan + * check needs - and it reports "not found" per site, while a node not being part of a site is the normal case + * for all but one of them. + * + * @return \Generator + */ + private function nodeVariants(string $nodeIdentifier, string $workspaceName): \Generator + { + foreach ($this->nodeContextCombinator->sites() as $site) { + $nodeFound = false; + + foreach ($this->nodeContextCombinator->siteNodeInContexts($site, $workspaceName) as $siteNode) { + $node = $siteNode->getContext()->getNodeByIdentifier($nodeIdentifier); + if ($node instanceof NodeInterface) { + $nodeFound = true; + yield [$siteNode, $node]; + } + } + + if ($nodeFound) { + // getNodeByIdentifier() looks the node up in the whole content repository rather than inside the + // site, so every further site would hand out the very same variants again + return; + } + } + } + + /** + * A quick release renders a handful of documents into a copy of a finished release. If their cache entries are + * still valid, that rendering is served straight from the content cache and the release publishes exactly what + * it copied - the editor's change would be missing without a single error anywhere. + */ + private function flushContentCacheForNode( + NodeInterface $node, + string $nodeIdentifier, + ContentReleaseLogger $contentReleaseLogger + ): void { + $flushedEntriesCount = 0; + foreach ($this->cachingHelper->nodeTag($node) as $tag) { + $flushedEntriesCount += $this->contentCache->flushByTag($tag); + } + + $contentReleaseLogger->info( + sprintf( + 'Flushed %d content cache entries for node %s before re-rendering it', + $flushedEntriesCount, + $nodeIdentifier + ) + ); + } +} diff --git a/Tests/Behavior/Features/Bootstrap/FeatureContext.php b/Tests/Behavior/Features/Bootstrap/FeatureContext.php index 035fd37..c02813b 100644 --- a/Tests/Behavior/Features/Bootstrap/FeatureContext.php +++ b/Tests/Behavior/Features/Bootstrap/FeatureContext.php @@ -8,12 +8,15 @@ use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\PrunnerJobId; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; use Flowpack\DecoupledContentStore\Exception as DecoupledContentStoreException; +use Flowpack\DecoupledContentStore\QuickPublish\Dto\NodeIdentifiers; use Flowpack\DecoupledContentStore\QuickPublish\Infrastructure\RedisReleaseCopyService; +use Flowpack\DecoupledContentStore\QuickPublish\QuickPublishNodeEnumerator; use Flowpack\DecoupledContentStore\Core\RedisKeyService; use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; use Flowpack\DecoupledContentStore\IncrementalContentReleaseHandler; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Repository\RedisEnumerationRepository; +use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\NodeContextCombinator; use Flowpack\DecoupledContentStore\NodeEnumeration\NodeEnumerator; use Flowpack\DecoupledContentStore\NodeRendering\Dto\RendererIdentifier; use Flowpack\DecoupledContentStore\NodeRendering\Infrastructure\RedisRenderingErrorManager; @@ -30,6 +33,7 @@ use Flowpack\DecoupledContentStore\PrepareContentRelease\Infrastructure\RedisContentReleaseService; use Flowpack\DecoupledContentStore\Tests\Behavior\Fixtures\StubPrunnerApiService; use Neos\Behat\Tests\Behat\FlowContextTrait; +use Neos\ContentRepository\Domain\Model\NodeInterface; use Neos\ContentRepository\Domain\Repository\WorkspaceRepository; use Neos\ContentRepository\Domain\Service\NodeTypeManager; use Neos\ContentRepository\Tests\Behavior\Features\Bootstrap\NodeOperationsTrait; @@ -221,6 +225,65 @@ private function copyContentRelease($sourceContentReleaseIdentifier, $targetCont } } + /** + * @When I enumerate the node at path :path for content release :contentReleaseIdentifier + */ + public function iEnumerateTheNodeAtPathForContentRelease($path, $contentReleaseIdentifier) + { + $this->enumerateGivenNodes($this->nodeIdentifierForPath($path), $contentReleaseIdentifier); + } + + /** + * @Then enumerating the node :nodeIdentifiers for content release :contentReleaseIdentifier is refused + */ + public function enumeratingTheNodeIsRefused($nodeIdentifiers, $contentReleaseIdentifier) + { + try { + $this->enumerateGivenNodes($nodeIdentifiers, $contentReleaseIdentifier); + } catch (DecoupledContentStoreException $exception) { + return; + } + + Assert::fail('Enumerating ' . $nodeIdentifiers . ' should have been refused.'); + } + + private function enumerateGivenNodes($nodeIdentifiers, $contentReleaseIdentifier): void + { + $contentReleaseIdentifier = ContentReleaseIdentifier::fromString($contentReleaseIdentifier); + $quickPublishNodeEnumerator = $this->getObjectManager()->get(QuickPublishNodeEnumerator::class); + $bufferedOutput = new BufferedOutput(); + $contentReleaseLogger = ContentReleaseLogger::fromSymfonyOutput($bufferedOutput, $contentReleaseIdentifier); + + try { + $quickPublishNodeEnumerator->enumerateGivenNodesAndStoreInRedis( + NodeIdentifiers::fromCommaSeparatedString($nodeIdentifiers), + $contentReleaseLogger, + $contentReleaseIdentifier + ); + } finally { + echo $bufferedOutput->fetch(); + } + } + + /** + * The fixtures do not spell out node identifiers, and a quick release is asked for exactly those. + */ + private function nodeIdentifierForPath(string $path): string + { + $combinator = $this->getObjectManager()->get(NodeContextCombinator::class); + + foreach ($combinator->sites() as $site) { + foreach ($combinator->siteNodeInContexts($site, 'live') as $siteNode) { + $node = $siteNode->getContext()->getNode($path); + if ($node instanceof NodeInterface) { + return $node->getIdentifier(); + } + } + } + + Assert::fail('Could not find a node at path ' . $path); + } + /** * @Then the enumeration for content release :contentReleaseIdentifier contains :expectedCount node * @Then the enumeration for content release :contentReleaseIdentifier contains :expectedCount nodes diff --git a/Tests/Behavior/Features/ContentStore/QuickRelease.feature b/Tests/Behavior/Features/ContentStore/QuickRelease.feature index 44e8e28..383c4a3 100644 --- a/Tests/Behavior/Features/ContentStore/QuickRelease.feature +++ b/Tests/Behavior/Features/ContentStore/QuickRelease.feature @@ -47,6 +47,15 @@ Feature: Quick Release # the enumeration says what a release renders, so a quick release brings its own instead of inheriting one And the enumeration for content release "6" contains 0 nodes + # and that enumeration holds nothing but the nodes the quick release was asked to publish + When I enumerate the node at path "/sites/test" for content release "6" + Then the enumeration for content release "6" contains 1 node + + Scenario: A node which cannot be found is not published + # a quick release which renders nothing would publish the release it was copied from, and look successful + When I create a content release "5" + Then enumerating the node "3239baee-3e7f-785c-0853-f4302ef32570" for content release "5" is refused + Scenario: A release which has not finished is not copied forward # everything the copy does not overwrite is published as if it had been rendered, so an unfinished release # must not be built upon diff --git a/Tests/Unit/NodeEnumeration/NodeEnumeratorTest.php b/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php similarity index 93% rename from Tests/Unit/NodeEnumeration/NodeEnumeratorTest.php rename to Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php index 4e6bcee..3323936 100644 --- a/Tests/Unit/NodeEnumeration/NodeEnumeratorTest.php +++ b/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php @@ -2,9 +2,9 @@ declare(strict_types=1); -namespace Flowpack\DecoupledContentStore\Tests\Unit\NodeEnumeration; +namespace Flowpack\DecoupledContentStore\Tests\Unit\NodeEnumeration\Domain\Service; -use Flowpack\DecoupledContentStore\NodeEnumeration\NodeEnumerator; +use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\DocumentNodeFilter; use PHPUnit\Framework\TestCase; /** @@ -15,14 +15,14 @@ * - 'Neos.Neos:Document' * - '!My.Package:Bar' */ -class NodeEnumeratorTest extends TestCase +class DocumentNodeFilterTest extends TestCase { /** * @param array $nodeTypeWhitelist the setting as it arrives from the YAML configuration */ private static function buildNodeTypeFilter(array $nodeTypeWhitelist): string { - $method = new \ReflectionMethod(NodeEnumerator::class, 'buildNodeTypeFilter'); + $method = new \ReflectionMethod(DocumentNodeFilter::class, 'buildNodeTypeFilter'); return $method->invoke(null, $nodeTypeWhitelist); } diff --git a/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php b/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php new file mode 100644 index 0000000..bee2cbe --- /dev/null +++ b/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php @@ -0,0 +1,96 @@ +jsonSerialize()); + } + + public function testSurroundingWhitespaceAndEmptyEntriesAreIgnored(): void + { + // the identifiers arrive from a textarea, one per line + $nodeIdentifiers = NodeIdentifiers::fromCommaSeparatedString( + " " . self::IDENTIFIER . " ,\n,\t" . self::OTHER_IDENTIFIER . "," + ); + + self::assertSame([self::IDENTIFIER, self::OTHER_IDENTIFIER], $nodeIdentifiers->jsonSerialize()); + } + + public function testUppercaseIdentifiersAreAccepted(): void + { + $identifier = strtoupper(self::IDENTIFIER); + + self::assertSame([$identifier], NodeIdentifiers::fromCommaSeparatedString($identifier)->jsonSerialize()); + } + + public function testTheSameIdentifierIsPublishedOnlyOnce(): void + { + $nodeIdentifiers = NodeIdentifiers::fromCommaSeparatedString(self::IDENTIFIER . ',' . self::IDENTIFIER); + + self::assertSame(1, $nodeIdentifiers->count()); + } + + /** + * @dataProvider notAnIdentifier + */ + public function testAnythingWhichIsNotAnIdentifierIsRefused(string $nodeIdentifiers): void + { + $this->expectException(Exception::class); + $this->expectExceptionCode(1786958510); + + NodeIdentifiers::fromCommaSeparatedString($nodeIdentifiers); + } + + /** + * @return array> + */ + public static function notAnIdentifier(): array + { + return [ + 'a shell command' => [self::IDENTIFIER . '; rm -rf /'], + 'a node path' => ['/sites/test/products'], + 'too short' => ['3239baee-3e7f-785c-0853-f4302ef325'], + 'no hyphens' => ['3239baee3e7f785c0853f4302ef32570'], + 'a quoted identifier' => ['"' . self::IDENTIFIER . '"'], + ]; + } + + public function testAnEmptyListIsRefused(): void + { + $this->expectException(Exception::class); + $this->expectExceptionCode(1786958511); + + NodeIdentifiers::fromCommaSeparatedString(' , '); + } + + public function testTheListIsHandedToThePipelineAsItWasRead(): void + { + // the pipeline passes it on as a prunner variable + self::assertSame( + self::IDENTIFIER . ',' . self::OTHER_IDENTIFIER, + (string) NodeIdentifiers::fromCommaSeparatedString(self::IDENTIFIER . ' , ' . self::OTHER_IDENTIFIER) + ); + } +} diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4c2f675..dacb8e4 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -798,30 +798,12 @@ parameters: count: 1 path: Classes/NodeEnumeration/NodeEnumerator.php - - - message: '#^Method Flowpack\\DecoupledContentStore\\NodeEnumeration\\NodeEnumerator\:\:buildNodeTypeFilter\(\) has parameter \$nodeTypeWhitelist with no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: Classes/NodeEnumeration/NodeEnumerator.php - - message: '#^Parameter \#3 \$workspaceName of method Flowpack\\DecoupledContentStore\\NodeEnumeration\\NodeEnumerator\:\:enumerateAll\(\) expects string, string\|null given\.$#' identifier: argument.type count: 1 path: Classes/NodeEnumeration/NodeEnumerator.php - - - message: '#^Property Flowpack\\DecoupledContentStore\\NodeEnumeration\\NodeEnumerator\:\:\$nodeTypeWhitelist type has no value type specified in iterable type array\.$#' - identifier: missingType.iterableValue - count: 1 - path: Classes/NodeEnumeration/NodeEnumerator.php - - - - message: '#^Strict comparison using \=\=\= between Neos\\ContentRepository\\Domain\\Model\\NodeInterface and null will always evaluate to false\.$#' - identifier: identical.alwaysFalse - count: 1 - path: Classes/NodeEnumeration/NodeEnumerator.php - - message: '#^Variable \$startTime might not be defined\.$#' identifier: variable.undefined From 8e43be2a0bc9c570d03ca90ecd1c63cd97a4bd6d Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 11:52:21 +0200 Subject: [PATCH 05/23] STEP 5: add scoped validation --- ...tentReleaseValidationCommandController.php | 29 +++-- Classes/QuickPublish/ContentReleaseScope.php | 79 ++++++++++++ .../QuickPublishNodeEnumerator.php | 84 ++++++++++--- Configuration/Settings.yaml | 8 ++ .../Features/Bootstrap/FeatureContext.php | 29 +++++ .../ContentStore/QuickRelease.feature | 52 +++++--- .../QuickPublish/ContentReleaseScopeTest.php | 118 ++++++++++++++++++ 7 files changed, 355 insertions(+), 44 deletions(-) create mode 100644 Classes/QuickPublish/ContentReleaseScope.php create mode 100644 Tests/Unit/QuickPublish/ContentReleaseScopeTest.php diff --git a/Classes/Command/ContentReleaseValidationCommandController.php b/Classes/Command/ContentReleaseValidationCommandController.php index 40a5f57..2791a4f 100644 --- a/Classes/Command/ContentReleaseValidationCommandController.php +++ b/Classes/Command/ContentReleaseValidationCommandController.php @@ -4,14 +4,15 @@ namespace Flowpack\DecoupledContentStore\Command; -use Flowpack\DecoupledContentStore\Exception; +use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; +use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; +use Flowpack\DecoupledContentStore\Exception; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Repository\RedisEnumerationRepository; use Flowpack\DecoupledContentStore\NodeRendering\Infrastructure\RedisRenderingErrorManager; +use Flowpack\DecoupledContentStore\QuickPublish\ContentReleaseScope; use Flowpack\DecoupledContentStore\ReleaseSwitch\Infrastructure\RedisReleaseSwitchService; use Neos\Flow\Annotations as Flow; -use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; -use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; use Neos\Flow\Cli\CommandController; /** @@ -37,12 +38,13 @@ class ContentReleaseValidationCommandController extends CommandController */ protected $redisEnumerationRepository; + #[Flow\Inject] + protected ContentReleaseScope $contentReleaseScope; + /** * Factor between 0 and 1 for the amount of URLs a new release needs to include to be valid - * - * @var float */ - protected $validReleaseUrlCountThreshold = 0.7; + protected float $validReleaseUrlCountThreshold = 0.7; public function validateCommand(string $contentReleaseIdentifier) { @@ -66,8 +68,19 @@ public function validateCommand(string $contentReleaseIdentifier) } $logger->info('Previous Content Release: ' . $currentlyLiveReleaseIdentifier->getIdentifier()); - $currentUrlsCount = $this->redisEnumerationRepository->count($currentlyLiveReleaseIdentifier); - $newUrlsCount = $this->redisEnumerationRepository->count($contentReleaseIdentifier); + if ($this->contentReleaseScope->getChangedUrls($contentReleaseIdentifier) !== null) { + // A quick release enumerates the handful of documents it re-renders and copies the rest, so its + // enumeration is smaller than the live one by design and would fail this check every single time. + // Its published URLs are the comparable number: after the copy they equal the release it was built on. + $logger->info( + 'Content release is a quick release, so its published URLs are counted instead of its enumeration.' + ); + $currentUrlsCount = $this->contentReleaseScope->countPublishedUrls($currentlyLiveReleaseIdentifier); + $newUrlsCount = $this->contentReleaseScope->countPublishedUrls($contentReleaseIdentifier); + } else { + $currentUrlsCount = $this->redisEnumerationRepository->count($currentlyLiveReleaseIdentifier); + $newUrlsCount = $this->redisEnumerationRepository->count($contentReleaseIdentifier); + } $minimumUrlsCount = (int) ceil($this->validReleaseUrlCountThreshold * $currentUrlsCount); $logger->info('Previous URL Count: ' . $currentUrlsCount); diff --git a/Classes/QuickPublish/ContentReleaseScope.php b/Classes/QuickPublish/ContentReleaseScope.php new file mode 100644 index 0000000..dc79883 --- /dev/null +++ b/Classes/QuickPublish/ContentReleaseScope.php @@ -0,0 +1,79 @@ +|null + */ + public function getChangedUrls(ContentReleaseIdentifier $contentReleaseIdentifier): ?array + { + $changedUrls = $this->redisClientManager->getPrimaryRedis()->sMembers( + $this->redisKeyService->getRedisKeyForPostfix($contentReleaseIdentifier, self::CHANGED_URLS_POSTFIX) + ); + + // a quick release which changed nothing is never published, so an empty set means there is no scope + if (!is_array($changedUrls) || $changedUrls === []) { + return null; + } + + return $changedUrls; + } + + /** + * @param array $changedUrls + */ + public function setChangedUrls(ContentReleaseIdentifier $contentReleaseIdentifier, array $changedUrls): void + { + if ($changedUrls === []) { + return; + } + + $this->redisClientManager->getPrimaryRedis()->sAdd( + $this->redisKeyService->getRedisKeyForPostfix($contentReleaseIdentifier, self::CHANGED_URLS_POSTFIX), + ...$changedUrls + ); + } + + /** + * How many URLs the release holds. + * + * This is what makes two releases comparable in size: the enumeration of a quick release only covers what it + * re-rendered, while every release - copied or rendered - carries the full list of URLs it publishes. + */ + public function countPublishedUrls(ContentReleaseIdentifier $contentReleaseIdentifier): int + { + return (int) $this->redisClientManager->getPrimaryRedis()->zCard( + $this->redisKeyService->getRedisKeyForPostfix($contentReleaseIdentifier, self::URLS_POSTFIX) + ); + } +} diff --git a/Classes/QuickPublish/QuickPublishNodeEnumerator.php b/Classes/QuickPublish/QuickPublishNodeEnumerator.php index 4d114b8..8c1a991 100644 --- a/Classes/QuickPublish/QuickPublishNodeEnumerator.php +++ b/Classes/QuickPublish/QuickPublishNodeEnumerator.php @@ -17,6 +17,7 @@ use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\NodeContextCombinator; use Flowpack\DecoupledContentStore\NodeRendering\Dto\NodeRenderingCompletionStatus; use Flowpack\DecoupledContentStore\NodeRendering\Extensibility\NodeRenderingExtensionManager; +use Flowpack\DecoupledContentStore\NodeRendering\NodeRenderingUriService; use Flowpack\DecoupledContentStore\PrepareContentRelease\Infrastructure\RedisContentReleaseService; use Flowpack\DecoupledContentStore\QuickPublish\Dto\NodeIdentifiers; use Flowpack\DecoupledContentStore\Utility\GeneratorUtility; @@ -56,6 +57,12 @@ final class QuickPublishNodeEnumerator #[Flow\Inject] protected CachingHelper $cachingHelper; + #[Flow\Inject] + protected NodeRenderingUriService $nodeRenderingUriService; + + #[Flow\Inject] + protected ContentReleaseScope $contentReleaseScope; + /** * @throws Exception if a given node cannot be published, or nothing is left to render */ @@ -88,25 +95,15 @@ public function enumerateGivenNodesAndStoreInRedis( $this->redisEnumerationRepository->clearDocumentNodesEnumeration($releaseIdentifier); - $enumeratedNodeCount = 0; - foreach ( - GeneratorUtility::createArrayBatch( - $this->enumerateGivenNodes( - $nodeIdentifiers, - $contentReleaseLogger, - $newMetadata->getWorkspaceName() ?? 'live' - ), - 100 - ) as $enumeration - ) { - $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($releaseIdentifier); - $this->redisEnumerationRepository->addDocumentNodesToEnumeration($releaseIdentifier, ...$enumeration); - $enumeratedNodeCount += count($enumeration); - } + $nodesToRender = $this->enumerateGivenNodes( + $nodeIdentifiers, + $contentReleaseLogger, + $newMetadata->getWorkspaceName() ?? 'live' + ); // a quick release which renders nothing publishes exactly the release it was copied from - which looks like // a successful publish to everybody watching, while the change the editor asked for is nowhere - if ($enumeratedNodeCount === 0) { + if ($nodesToRender === []) { throw new Exception( sprintf( 'None of the given nodes can be published (%s), so content release %s would only repeat the release ' @@ -117,18 +114,59 @@ public function enumerateGivenNodesAndStoreInRedis( ); } - $contentReleaseLogger->info(sprintf('Enumerated %d node variants for rendering', $enumeratedNodeCount)); + foreach ( + GeneratorUtility::createArrayBatch( + array_map(static fn(array $nodeToRender): EnumeratedNode => $nodeToRender[1], $nodesToRender), + 100 + ) as $enumeration + ) { + $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($releaseIdentifier); + $this->redisEnumerationRepository->addDocumentNodesToEnumeration($releaseIdentifier, ...$enumeration); + } + + $this->writeChangedUrls($nodesToRender, $releaseIdentifier, $contentReleaseLogger); + + $contentReleaseLogger->info(sprintf('Enumerated %d node variants for rendering', count($nodesToRender))); } /** - * @return iterable + * The URLs of everything this release renders, for the validators which check a quick release instead of the + * whole content store. + * + * They are built here rather than while enumerating, because {@see NodeRenderingUriService::buildNodeUri()} + * marks the security context as initialized as a side effect, which would change what the node lookups of the + * remaining identifiers are allowed to see. + * + * @param array $nodesToRender + */ + private function writeChangedUrls( + array $nodesToRender, + ContentReleaseIdentifier $releaseIdentifier, + ContentReleaseLogger $contentReleaseLogger + ): void { + $changedUrls = []; + foreach ($nodesToRender as [$node, $enumeratedNode]) { + $changedUrls[] = $this->nodeRenderingUriService->buildNodeUri($node, $enumeratedNode->getArguments()); + } + + $this->contentReleaseScope->setChangedUrls($releaseIdentifier, $changedUrls); + + $contentReleaseLogger->info('Content release is scoped to the URLs it renders', [ + 'changedUrls' => $changedUrls + ]); + } + + /** + * @return array * @throws Exception */ private function enumerateGivenNodes( NodeIdentifiers $nodeIdentifiers, ContentReleaseLogger $contentReleaseLogger, string $workspaceName - ): iterable { + ): array { + $nodesToRender = []; + foreach ($nodeIdentifiers as $nodeIdentifier) { $contentCacheFlushed = false; @@ -156,7 +194,11 @@ private function enumerateGivenNodes( $contentReleaseLogger->info('Registering node for publishing', ['node' => $contextPath]); - yield from $this->nodeRenderingExtensionManager->enumerateDocumentNode($nodeToEnumerate); + foreach ( + $this->nodeRenderingExtensionManager->enumerateDocumentNode($nodeToEnumerate) as $enumeratedNode + ) { + $nodesToRender[] = [$nodeToEnumerate, $enumeratedNode]; + } } if (!$contentCacheFlushed) { @@ -168,6 +210,8 @@ private function enumerateGivenNodes( ); } } + + return $nodesToRender; } /** diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index 5c1a156..d9ea77b 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -141,6 +141,14 @@ Flowpack: transferMode: 'dump' isRequired: true copyOnQuickRelease: true + quickPublishChangedUrls: + redisKeyPostfix: 'quickPublish:changedUrls' + transfer: false + transferMode: 'dump' + isRequired: false + # the key exists for a quick release only, and says which URLs it re-rendered - validators use it to check + # those instead of the whole release. It describes this release, so it is not copied and not transferred. + copyOnQuickRelease: false renderedDocuments: redisKeyPostfix: 'renderedDocuments' transfer: true diff --git a/Tests/Behavior/Features/Bootstrap/FeatureContext.php b/Tests/Behavior/Features/Bootstrap/FeatureContext.php index c02813b..f8193ca 100644 --- a/Tests/Behavior/Features/Bootstrap/FeatureContext.php +++ b/Tests/Behavior/Features/Bootstrap/FeatureContext.php @@ -2,6 +2,7 @@ use Behat\Behat\Context\Context; use Behat\Gherkin\Node\PyStringNode; +use Flowpack\DecoupledContentStore\Command\ContentReleaseValidationCommandController; use Flowpack\DecoupledContentStore\ContentReleaseManager; use Flowpack\DecoupledContentStore\Core\ConcurrentBuildLockService; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; @@ -180,6 +181,34 @@ public function iEnumerateAllNodesForContentRelease($contentReleaseIdentifier) echo $bufferedOutput->fetch(); } + /** + * Sets what the pipeline reads as the currently live release, without going through a switch - which validates + * a lot more than this is about. + * + * @Given the currently live content release is :contentReleaseIdentifier + */ + public function theCurrentlyLiveContentReleaseIs($contentReleaseIdentifier) + { + $redisClientManager = $this->getObjectManager()->get(RedisClientManager::class); + $redisClientManager->getPrimaryRedis()->set('contentStore:current', $contentReleaseIdentifier); + } + + /** + * @Then validating content release :contentReleaseIdentifier succeeds + */ + public function validatingContentReleaseSucceeds($contentReleaseIdentifier) + { + // the command ends the process when it considers the release invalid, so getting past this line is half of + // the assertion - the other half is that it did not mark the release as broken on the way + $validationCommandController = $this->getObjectManager()->get(ContentReleaseValidationCommandController::class); + $validationCommandController->validateCommand($contentReleaseIdentifier); + + $redisRenderingErrorManager = $this->getObjectManager()->get(RedisRenderingErrorManager::class); + Assert::assertCount(0, $redisRenderingErrorManager->getRenderingErrors( + ContentReleaseIdentifier::fromString($contentReleaseIdentifier) + )); + } + /** * @When I copy the content release :sourceContentReleaseIdentifier to the content release :targetContentReleaseIdentifier */ diff --git a/Tests/Behavior/Features/ContentStore/QuickRelease.feature b/Tests/Behavior/Features/ContentStore/QuickRelease.feature index 383c4a3..2f89b0c 100644 --- a/Tests/Behavior/Features/ContentStore/QuickRelease.feature +++ b/Tests/Behavior/Features/ContentStore/QuickRelease.feature @@ -9,6 +9,10 @@ Feature: Quick Release superTypes: 'Neos.Neos:Document': true + Flowpack.DecoupledContentStore.Test:Document.Page: + superTypes: + 'Neos.Neos:Document': true + Flowpack.DecoupledContentStore.Test:Content.Text: superTypes: 'Neos.Neos:Content': true @@ -20,46 +24,62 @@ Feature: Quick Release Given I am authenticated with role "Neos.Neos:Editor" Given I have a site for Site Node "test" with site package key "Flowpack.DecoupledContentStore" with domain "test.de" And I have the following nodes: - | Path | Node Type | Properties | HiddenInIndex | Language | - | /sites | unstructured | [] | false | de | - | /sites/test | Flowpack.DecoupledContentStore.Test:Document.StartPage | {"title":"Startseite","uriPathSegment":"startseite"} | false | de | - | /sites/test/main | Neos.Neos:ContentCollection | {} | false | de | - | /sites/test/main/t1 | Flowpack.DecoupledContentStore.Test:Content.Text | {"text": "Hallo - this is rendered."} | false | de | + | Path | Node Type | Properties | HiddenInIndex | Language | + | /sites | unstructured | [] | false | de | + | /sites/test | Flowpack.DecoupledContentStore.Test:Document.StartPage | {"title":"Startseite","uriPathSegment":"startseite"} | false | de | + | /sites/test/main | Neos.Neos:ContentCollection | {} | false | de | + | /sites/test/main/t1 | Flowpack.DecoupledContentStore.Test:Content.Text | {"text": "Hallo - this is rendered."} | false | de | + | /sites/test/sub | Flowpack.DecoupledContentStore.Test:Document.Page | {"title":"Subpage","uriPathSegment":"nested"} | false | de | + | /sites/test/sub/main | Neos.Neos:ContentCollection | {} | false | de | + | /sites/test/sub/main/t1 | Flowpack.DecoupledContentStore.Test:Content.Text | {"text": "Unterseite"} | false | de | + | /sites/test/sub2 | Flowpack.DecoupledContentStore.Test:Document.Page | {"title":"Subpage2","uriPathSegment":"nested2"} | false | de | + | /sites/test/sub2/main | Neos.Neos:ContentCollection | {} | false | de | + | /sites/test/sub2/main/t1 | Flowpack.DecoupledContentStore.Test:Content.Text | {"text": "Unterseite2"} | false | de | And I flush the content cache depending on the modified nodes - Scenario: A finished release is copied forward instead of being rendered again + # the release a quick release is built on When I create a content release "5" And I enumerate all nodes for content release "5" - And I run the render-orchestrator control loop once for content release "5" + Then the enumeration for content release "5" contains 3 nodes + When I run the render-orchestrator control loop once for content release "5" And I run the renderer for content release "5" until the queue is empty Then during rendering of content release "5", no errors occured When I continue running the render-orchestrator control loop Then I expect the render-orchestrator control loop to exit with status code 0 And I expect the content release "5" to have the completion status success + Scenario: A finished release is copied forward instead of being rendered again # nothing is rendered for the new release, and it holds the content of its predecessor anyway When I create a content release "6" And I copy the content release "5" to the content release "6" - Then I expect the content release "6" to contain the following content for URI "http://test.de/de" at CSS selector "body .neos-contentcollection": + Then I expect the content release "6" to contain the following content for URI "http://test.de/de/nested" at CSS selector "body .neos-contentcollection": """ - BEFOREHallo - this is rendered.AFTER + BEFOREUnterseiteAFTER """ # the enumeration says what a release renders, so a quick release brings its own instead of inheriting one And the enumeration for content release "6" contains 0 nodes # and that enumeration holds nothing but the nodes the quick release was asked to publish - When I enumerate the node at path "/sites/test" for content release "6" + When I enumerate the node at path "/sites/test/sub" for content release "6" Then the enumeration for content release "6" contains 1 node + Scenario: A quick release is not rejected for enumerating only what it changed + # the URL count check compares the new release against the live one, and the enumeration of a quick release is + # smaller than that by design - it has to be measured by the URLs it publishes instead + Given the currently live content release is "5" + When I create a content release "6" + And I copy the content release "5" to the content release "6" + And I enumerate the node at path "/sites/test/sub" for content release "6" + Then validating content release "6" succeeds + Scenario: A node which cannot be found is not published # a quick release which renders nothing would publish the release it was copied from, and look successful - When I create a content release "5" - Then enumerating the node "3239baee-3e7f-785c-0853-f4302ef32570" for content release "5" is refused + When I create a content release "6" + Then enumerating the node "3239baee-3e7f-785c-0853-f4302ef32570" for content release "6" is refused Scenario: A release which has not finished is not copied forward # everything the copy does not overwrite is published as if it had been rendered, so an unfinished release # must not be built upon - When I create a content release "5" - And I enumerate all nodes for content release "5" - When I create a content release "6" - Then copying the content release "5" to the content release "6" is refused + When I create a content release "7" + And I create a content release "8" + Then copying the content release "7" to the content release "8" is refused diff --git a/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php b/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php new file mode 100644 index 0000000..91698f8 --- /dev/null +++ b/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php @@ -0,0 +1,118 @@ +createMock(\Redis::class); + $redis->method('sMembers')->with(self::CHANGED_URLS_KEY)->willReturn([]); + + self::assertNull($this->buildContentReleaseScope($redis)->getChangedUrls($this->contentReleaseIdentifier())); + } + + public function testAQuickReleaseIsScopedToTheUrlsItRendered(): void + { + $redis = $this->createMock(\Redis::class); + $redis->method('sMembers')->with(self::CHANGED_URLS_KEY)->willReturn([ + 'http://test.de/de', + 'http://test.de/de/nested', + ]); + + self::assertSame( + ['http://test.de/de', 'http://test.de/de/nested'], + $this->buildContentReleaseScope($redis)->getChangedUrls($this->contentReleaseIdentifier()) + ); + } + + public function testTheScopeIsStoredWithTheReleaseItBelongsTo(): void + { + $redis = $this->createMock(\Redis::class); + $redis->expects(self::once())->method('sAdd')->with( + self::CHANGED_URLS_KEY, + 'http://test.de/de', + 'http://test.de/de/nested' + ); + + $this->buildContentReleaseScope($redis)->setChangedUrls( + $this->contentReleaseIdentifier(), + ['http://test.de/de', 'http://test.de/de/nested'] + ); + } + + public function testAnEmptyScopeIsNotStored(): void + { + // it would be indistinguishable from a full release, which is validated as a whole + $redis = $this->createMock(\Redis::class); + $redis->expects(self::never())->method('sAdd'); + + $this->buildContentReleaseScope($redis)->setChangedUrls($this->contentReleaseIdentifier(), []); + } + + public function testPublishedUrlsAreCountedFromTheUrlIndexRatherThanTheEnumeration(): void + { + $redis = $this->createMock(\Redis::class); + $redis->method('zCard')->with('contentStore:5:meta:urls')->willReturn(18015); + + self::assertSame( + 18015, + $this->buildContentReleaseScope($redis)->countPublishedUrls($this->contentReleaseIdentifier()) + ); + } + + private function contentReleaseIdentifier(): ContentReleaseIdentifier + { + return ContentReleaseIdentifier::fromString('5'); + } + + /** + * @param \Redis&MockObject $redis + */ + private function buildContentReleaseScope(\Redis $redis): ContentReleaseScope + { + $redisClientManager = $this->createMock(RedisClientManager::class); + $redisClientManager->method('getPrimaryRedis')->willReturn($redis); + + $redisKeyService = new RedisKeyService(); + $this->inject($redisKeyService, 'redisKeyPostfixesForEachReleaseConfiguration', [ + 'metaUrls' => [ + 'redisKeyPostfix' => 'meta:urls', + 'transfer' => true, + 'transferMode' => 'dump', + 'isRequired' => true, + 'copyOnQuickRelease' => true, + ], + 'quickPublishChangedUrls' => [ + 'redisKeyPostfix' => 'quickPublish:changedUrls', + 'transfer' => false, + 'transferMode' => 'dump', + 'isRequired' => false, + 'copyOnQuickRelease' => false, + ], + ]); + + $contentReleaseScope = new ContentReleaseScope(); + $this->inject($contentReleaseScope, 'redisClientManager', $redisClientManager); + $this->inject($contentReleaseScope, 'redisKeyService', $redisKeyService); + + return $contentReleaseScope; + } +} From d40be6b25cc0c2a58e7bebfa617391dd5e9aca12 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 12:06:26 +0200 Subject: [PATCH 06/23] STEP 6: add pipeline config and usage --- Classes/ContentReleaseManager.php | 83 +++++++++-- ...uickContentReleaseNotPossibleException.php | 14 ++ Tests/Unit/ContentReleaseManagerTest.php | 103 +++++++++++++- pipelines_template.yml | 132 ++++++++++++++++-- 4 files changed, 309 insertions(+), 23 deletions(-) create mode 100644 Classes/Exception/QuickContentReleaseNotPossibleException.php diff --git a/Classes/ContentReleaseManager.php b/Classes/ContentReleaseManager.php index e9997a9..7988be1 100644 --- a/Classes/ContentReleaseManager.php +++ b/Classes/ContentReleaseManager.php @@ -8,6 +8,9 @@ use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\ContentReleaseIdentifier; use Flowpack\DecoupledContentStore\Core\Domain\ValueObject\RedisInstanceIdentifier; use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; +use Flowpack\DecoupledContentStore\Exception\QuickContentReleaseNotPossibleException; +use Flowpack\DecoupledContentStore\QuickPublish\Dto\NodeIdentifiers; +use Flowpack\Prunner\Dto\Job; use Flowpack\Prunner\PrunnerApiService; use Flowpack\Prunner\ValueObject\JobId; use Flowpack\Prunner\ValueObject\PipelineName; @@ -54,6 +57,8 @@ class ContentReleaseManager const REDIS_CURRENT_RELEASE_KEY = 'contentStore:current'; const NO_PREVIOUS_RELEASE = 'NO_PREVIOUS_RELEASE'; + const CONTENT_RELEASE_PIPELINE_NAME = 'do_content_release'; + const QUICK_CONTENT_RELEASE_PIPELINE_NAME = 'do_quick_content_release'; /** * All automatic release triggers (workspace publish, asset change, re-release after a rendering error) run through @@ -79,7 +84,7 @@ public function startIncrementalContentRelease( // the currentContentReleaseId is not used in any pipeline step in this package, but is a common need in other // use cases in extensions, e.g. calculating the differences between current and new release $this->prunnerApiService->schedulePipeline( - PipelineName::create('do_content_release'), + PipelineName::create(self::CONTENT_RELEASE_PIPELINE_NAME), array_merge($additionalVariables, [ 'contentReleaseId' => $contentReleaseId, 'currentContentReleaseId' => $this->resolveCurrentContentReleaseId($currentContentReleaseId), @@ -104,7 +109,7 @@ public function startFullContentRelease( ): ContentReleaseIdentifier { $contentReleaseId = ContentReleaseIdentifier::create(); $this->prunnerApiService->schedulePipeline( - PipelineName::create('do_content_release'), + PipelineName::create(self::CONTENT_RELEASE_PIPELINE_NAME), array_merge($additionalVariables, [ 'contentReleaseId' => $contentReleaseId, 'currentContentReleaseId' => $this->resolveCurrentContentReleaseId($currentContentReleaseId), @@ -118,11 +123,60 @@ public function startFullContentRelease( return $contentReleaseId; } + /** + * Publish the given document nodes into a copy of the release which is currently live, instead of rendering + * every document again. + * + * This is an explicitly requested release, so like "Publish All" it is deliberately not affected by the pause + * switch - a paused automatic release is in fact the situation this exists for. + * + * @param array $additionalVariables additional prunner variables, e.g. for added pipeline tasks + * @throws QuickContentReleaseNotPossibleException if there is no release to build upon, or another quick release + * is still on its way + */ + public function startQuickContentRelease( + NodeIdentifiers $nodeIdentifiers, + ?Workspace $workspace = null, + array $additionalVariables = [] + ): ContentReleaseIdentifier { + $currentContentReleaseId = $this->resolveCurrentContentReleaseId(null); + if ($currentContentReleaseId === self::NO_PREVIOUS_RELEASE) { + throw new QuickContentReleaseNotPossibleException( + 'There is no content release live at the moment, so there is nothing to publish the given nodes into. ' + . 'Run a full content release instead.', + 1786963710 + ); + } + + // a quick release copies the release which is live when it is scheduled, so a second one queued behind the + // first would build on the release the first is about to replace - and drop that first change without a word + $quickReleaseJobs = $this->prunnerApiService->loadPipelinesAndJobs()->getJobs() + ->forPipeline(PipelineName::create(self::QUICK_CONTENT_RELEASE_PIPELINE_NAME)); + if ($quickReleaseJobs->running()->getArray() !== [] || $quickReleaseJobs->waiting()->getArray() !== []) { + throw new QuickContentReleaseNotPossibleException( + 'Another quick content release is still on its way. Wait for it to go live, then publish these nodes.', + 1786963711 + ); + } + + $contentReleaseId = ContentReleaseIdentifier::create(); + $this->prunnerApiService->schedulePipeline( + PipelineName::create(self::QUICK_CONTENT_RELEASE_PIPELINE_NAME), + array_merge($additionalVariables, [ + 'contentReleaseId' => $contentReleaseId, + 'currentContentReleaseId' => $currentContentReleaseId, + 'quickPublishNodeIdentifiers' => (string)$nodeIdentifiers, + 'workspaceName' => $workspace !== null ? $workspace->getName() : 'live', + 'accountId' => $this->getAccountId() + ]) + ); + + return $contentReleaseId; + } + public function cancelAllRunningContentReleases(): void { - $result = $this->prunnerApiService->loadPipelinesAndJobs(); - $runningJobs = $result->getJobs()->forPipeline(PipelineName::create('do_content_release'))->running(); - foreach ($runningJobs as $job) { + foreach ($this->runningContentReleaseJobs() as $job) { $this->prunnerApiService->cancelJob($job); } } @@ -132,9 +186,7 @@ public function cancelAllRunningContentReleases(): void */ public function cancelRunningContentRelease(JobId $jobId): void { - $result = $this->prunnerApiService->loadPipelinesAndJobs(); - $runningJobs = $result->getJobs()->forPipeline(PipelineName::create('do_content_release'))->running(); - foreach ($runningJobs as $job) { + foreach ($this->runningContentReleaseJobs() as $job) { if ($job->getId() === $jobId) { $this->prunnerApiService->cancelJob($job); break; @@ -142,6 +194,21 @@ public function cancelRunningContentRelease(JobId $jobId): void } } + /** + * Both pipelines build a release which ends up being switched live, so both are cancelled. + * + * @return Job[] + */ + private function runningContentReleaseJobs(): array + { + $jobs = $this->prunnerApiService->loadPipelinesAndJobs()->getJobs(); + + return array_merge( + $jobs->forPipeline(PipelineName::create(self::CONTENT_RELEASE_PIPELINE_NAME))->running()->getArray(), + $jobs->forPipeline(PipelineName::create(self::QUICK_CONTENT_RELEASE_PIPELINE_NAME))->running()->getArray() + ); + } + public function toggleConfigEpoch(RedisInstanceIdentifier $redisInstanceIdentifier): void { $currentConfigEpochConfig = $this->configEpochSettings['current'] ?? null; diff --git a/Classes/Exception/QuickContentReleaseNotPossibleException.php b/Classes/Exception/QuickContentReleaseNotPossibleException.php new file mode 100644 index 0000000..fce4571 --- /dev/null +++ b/Classes/Exception/QuickContentReleaseNotPossibleException.php @@ -0,0 +1,14 @@ +prunnerApiService = $this->createMock(PrunnerApiService::class); @@ -68,10 +80,93 @@ public function testPublishAllIsScheduledEvenWhilePaused(): void $this->buildContentReleaseManager()->startFullContentRelease(); } + public function testAQuickReleaseIsScheduledEvenWhilePaused(): void + { + // pausing the automatic release is what quick releases exist for, so the pause must not block them + $this->currentContentReleaseId = '5'; + $this->automaticReleaseSwitchService->method('isPaused')->willReturn(true); + $this->prunnerApiService->expects(self::once())->method('schedulePipeline')->with( + self::equalTo(PipelineName::create('do_quick_content_release')), + self::callback(static fn(array $variables): bool => + $variables['currentContentReleaseId'] === '5' + && $variables['quickPublishNodeIdentifiers'] === self::NODE_IDENTIFIER) + ); + + $this->buildContentReleaseManager()->startQuickContentRelease($this->nodeIdentifiers()); + } + + public function testAQuickReleaseIsRefusedWhileNoReleaseIsLive(): void + { + // there is nothing to copy, so the release would hold the given nodes and nothing else + $this->currentContentReleaseId = false; + $this->prunnerApiService->expects(self::never())->method('schedulePipeline'); + + $this->expectException(QuickContentReleaseNotPossibleException::class); + $this->buildContentReleaseManager()->startQuickContentRelease($this->nodeIdentifiers()); + } + + /** + * @dataProvider quickReleasesOnTheirWay + */ + public function testAQuickReleaseIsRefusedWhileAnotherOneIsOnItsWay(bool $started): void + { + // the second one copies the release the first is about to replace, so it would undo the first change + $this->currentContentReleaseId = '5'; + $this->prunnerApiService->method('loadPipelinesAndJobs') + ->willReturn($this->jobsResponse('do_quick_content_release', $started)); + $this->prunnerApiService->expects(self::never())->method('schedulePipeline'); + + $this->expectException(QuickContentReleaseNotPossibleException::class); + $this->buildContentReleaseManager()->startQuickContentRelease($this->nodeIdentifiers()); + } + + /** + * @return array + */ + public static function quickReleasesOnTheirWay(): array + { + return ['running' => [true], 'waiting in the queue' => [false]]; + } + + public function testARunningQuickReleaseIsCancelledAlongWithTheOtherContentReleases(): void + { + // it ends up being switched live just like a full release does, so "cancel" has to reach it + $this->prunnerApiService->method('loadPipelinesAndJobs') + ->willReturn($this->jobsResponse('do_quick_content_release', true)); + $this->prunnerApiService->expects(self::once())->method('cancelJob'); + + $this->buildContentReleaseManager()->cancelAllRunningContentReleases(); + } + + private function nodeIdentifiers(): NodeIdentifiers + { + return NodeIdentifiers::fromCommaSeparatedString(self::NODE_IDENTIFIER); + } + + private function jobsResponse(string $pipeline, bool $started): PipelinesAndJobsResponse + { + return PipelinesAndJobsResponse::fromJsonArray([ + 'pipelines' => [], + 'jobs' => [ + [ + 'id' => 'job-id', + 'pipeline' => $pipeline, + 'tasks' => [], + 'completed' => false, + 'canceled' => false, + 'errored' => false, + 'created' => '2026-08-17T10:00:00+02:00', + 'start' => $started ? '2026-08-17T10:00:01+02:00' : null, + 'user' => 'test', + ], + ], + ]); + } + private function buildContentReleaseManager(): ContentReleaseManager { $redis = $this->createMock(\Redis::class); - $redis->method('get')->willReturn(false); + $redis->method('get')->willReturn($this->currentContentReleaseId); $redisClientManager = $this->createMock(RedisClientManager::class); $redisClientManager->method('getPrimaryRedis')->willReturn($redis); diff --git a/pipelines_template.yml b/pipelines_template.yml index 3d46b88..9fbb990 100644 --- a/pipelines_template.yml +++ b/pipelines_template.yml @@ -61,23 +61,26 @@ pipelines: ################################################################################ # 2) RENDERING ################################################################################ - render_orchestrator: + # the tasks anchored here (&name) are reused by do_quick_content_release below through a YAML alias (*name), + # so the two pipelines cannot drift apart. Editing one of them changes both - which is the intention; a task + # which has to differ is written out again down there instead. + render_orchestrator: &render_orchestrator script: - ./flow nodeRendering:orchestrateRendering {{ .contentReleaseId }} depends_on: [enumerate_finished] - render_1: + render_1: &render_1 script: - Packages/Application/Flowpack.DecoupledContentStore/Scripts/renderWorker.sh {{ .contentReleaseId }} w1 depends_on: [enumerate_finished] - render_2: + render_2: &render_2 script: - Packages/Application/Flowpack.DecoupledContentStore/Scripts/renderWorker.sh {{ .contentReleaseId }} w2 depends_on: [enumerate_finished] - render_3: + render_3: &render_3 script: - Packages/Application/Flowpack.DecoupledContentStore/Scripts/renderWorker.sh {{ .contentReleaseId }} w3 depends_on: [enumerate_finished] - render_4: + render_4: &render_4 script: - Packages/Application/Flowpack.DecoupledContentStore/Scripts/renderWorker.sh {{ .contentReleaseId }} w4 depends_on: [enumerate_finished] @@ -141,7 +144,7 @@ pipelines: # 3) the task name must be included in validate_finished.depends_on # marker task to depend on all validation jobs; as separation between validation and transfer stages - validate_finished: + validate_finished: &validate_finished script: - ./flow contentReleaseValidation:ensureNoValidationErrorsExist {{ .contentReleaseId }} depends_on: @@ -151,7 +154,7 @@ pipelines: # 4) TRANSFER ################################################################################ # By default, the transfer phase is empty. ("Minimal Setup" in the README) - transfer_content: + transfer_content: &transfer_content script: # in case you want to use additional content stores, you need to enable the command # below (see "Copy Content Releases to a different Redis instance" in README) @@ -162,7 +165,7 @@ pipelines: # - ./flow contentReleaseTransfer:transferToContentStore target_live {{ .contentReleaseId }} depends_on: [validate_finished] - transfer_resources: + transfer_resources: &transfer_resources script: # in case you need to manually sync assets using rsync, you need to enable the command # below (see "Manually Sync Assets to the Delivery Layer via RSync" in README) @@ -177,7 +180,7 @@ pipelines: # 3) the task name must be included in transfer_finished.depends_on # task to depend on all transfer jobs; as separation between transfer and switch stages - transfer_finished: + transfer_finished: &transfer_finished script: - ./flow contentReleaseTransfer:removeOldReleases primary {{ .contentReleaseId }} depends_on: @@ -187,7 +190,7 @@ pipelines: ################################################################################ # 5) SWITCH ################################################################################ - switch_primary: + switch_primary: &switch_primary script: - ./flow contentReleaseSwitch:switchActiveContentRelease primary {{ .contentReleaseId }} # in case you need want to switch the active content release in another content store, @@ -197,11 +200,118 @@ pipelines: depends_on: - transfer_finished - switch_finished: + switch_finished: &switch_finished script: [""] depends_on: - switch_primary + # A quick content release publishes a handful of named document nodes without rendering everything else: it copies + # the release which is currently live and re-renders only the given nodes into that copy. Everything after the + # enumeration is the pipeline above, unchanged. + # + # ARG: contentReleaseId, currentContentReleaseId, quickPublishNodeIdentifiers, workspaceName, accountId + # the currentContentReleaseId is the release being copied here, so unlike in do_content_release it is required + do_quick_content_release: + + # crucial settings for incremental rendering to work + concurrency: 1 + + # deliberately NOT `queue_strategy: replace` - a quick release publishes exactly the nodes somebody asked for, so + # a queued one must not be thrown away by the next one. ContentReleaseManager::startQuickContentRelease() refuses + # to schedule a second one while the first is still on its way, because its copy source is resolved when it is + # scheduled and would be the release the first one is about to replace. + + # see the note on do_content_release above + retention_count: 10 + + tasks: + ################################################################################ + # 0) PREPARE + ################################################################################ + # there is no flushContentCacheIfRequired here: this pipeline renders a few documents into a copy of a finished + # release, and contentReleaseQuickPublish:enumerateGivenNodes flushes the content cache for exactly those. + prepare_content_release: + script: + - ./flow contentReleasePrepare:createContentRelease {{ .contentReleaseId }} {{ .__jobID }} --workspaceName {{ .workspaceName }} --accountId {{ .accountId }} + - ./flow contentReleasePrepare:ensureAllOtherInProgressContentReleasesWillBeTerminated {{ .contentReleaseId }} + + prepare_copy_previous_release: + script: + - ./flow contentReleaseQuickPublish:copyReleaseWithin primary {{ .currentContentReleaseId }} {{ .contentReleaseId }} + depends_on: [prepare_content_release] + + # marker task to depend on all preparation jobs; as separation between preparation and enumeration stages + prepare_finished: + script: [""] + depends_on: + - prepare_copy_previous_release + + ################################################################################ + # 1) ENUMERATION + ################################################################################ + enumerate_nodes: + script: + - ./flow contentReleaseQuickPublish:enumerateGivenNodes {{ .contentReleaseId }} {{ .quickPublishNodeIdentifiers }} + depends_on: [prepare_finished] + + # Extension Point: see do_content_release above. Note that anything enumerated here is *added* to a copy of the + # previous release, so an enumeration which produces nothing leaves the copy as it is. + + # marker task to depend on all enumeration jobs; as separation between enumeration and rendering stages + enumerate_finished: + script: [""] + depends_on: + - enumerate_nodes + + ################################################################################ + # 2) RENDERING + ################################################################################ + # fewer workers than do_content_release: a quick release enumerates a handful of documents, so further workers + # would only add Flow bootstraps to the pipeline. `*name` is a YAML alias of the task anchored as `&name` in + # do_content_release above - depends_on names a task, and is resolved inside this pipeline + render_orchestrator: *render_orchestrator + render_1: *render_1 + render_2: *render_2 + render_3: *render_3 + render_4: *render_4 + + # marker task to depend on all rendering jobs; as separation between rendering and validation stages + render_finished: + script: [""] + depends_on: + - render_orchestrator + - render_1 + - render_2 + - render_3 + - render_4 + + ################################################################################ + # 3) VALIDATION + ################################################################################ + # unconditional, unlike in do_content_release: validation of a quick release is scoped to the URLs it rendered + # (see Flowpack\DecoupledContentStore\QuickPublish\ContentReleaseScope), so there is nothing to save by skipping it + validate_content: + script: + - ./flow contentReleaseValidation:validate {{ .contentReleaseId }} + depends_on: [render_finished] + + validate_finished: *validate_finished + + ################################################################################ + # 4) TRANSFER + ################################################################################ + # identical to do_content_release: the release is complete after the copy, so it is transferred as a whole - + # which also means the content stores you comment in up there are transferred to from here as well + transfer_content: *transfer_content + transfer_resources: *transfer_resources + transfer_finished: *transfer_finished + + ################################################################################ + # 5) SWITCH + ################################################################################ + switch_primary: *switch_primary + switch_finished: *switch_finished + # this subset of the pipeline above is running when a release is manually transfered and switched live # from the primary content store to any other content store # ARG: contentReleaseId, currentContentReleaseId, redisInstanceId From d494d3ff709e1e7a2145700676a62fac12b5b0f2 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 12:52:29 +0200 Subject: [PATCH 07/23] STEP 7: add backend ui for quick publishing --- Classes/Controller/BackendController.php | 88 +++++++++- .../Domain/Service/DocumentNodeFilter.php | 16 ++ .../Domain/Service/NodeContextCombinator.php | 44 ++++- Classes/QuickPublish/Dto/NodeIdentifiers.php | 25 ++- .../Dto/QuickPublishPreviewRow.php | 106 ++++++++++++ .../QuickPublishNodeEnumerator.php | 38 +---- .../QuickPublishPreviewService.php | 89 ++++++++++ Configuration/Policy.yaml | 9 +- .../Integration/Backend.Index.fusion | 17 ++ .../Integration/Backend.QuickPublish.fusion | 157 ++++++++++++++++++ Resources/Private/Translations/de/Main.xlf | 47 ++++++ Resources/Private/Translations/en/Main.xlf | 36 ++++ .../QuickPublish/Dto/NodeIdentifiersTest.php | 25 +++ 13 files changed, 652 insertions(+), 45 deletions(-) create mode 100644 Classes/QuickPublish/Dto/QuickPublishPreviewRow.php create mode 100644 Classes/QuickPublish/QuickPublishPreviewService.php create mode 100644 Resources/Private/BackendFusion/Integration/Backend.QuickPublish.fusion diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index 29dea28..9eb7155 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -16,11 +16,15 @@ use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; use Flowpack\DecoupledContentStore\Core\RedisKeyService; use Flowpack\DecoupledContentStore\Core\RedisPruneService; +use Flowpack\DecoupledContentStore\Exception; use Flowpack\DecoupledContentStore\PrepareContentRelease\Infrastructure\RedisContentReleaseService; +use Flowpack\DecoupledContentStore\QuickPublish\Dto\NodeIdentifiers; +use Flowpack\DecoupledContentStore\QuickPublish\QuickPublishPreviewService; use Flowpack\DecoupledContentStore\ReleaseSwitch\Infrastructure\RedisReleaseSwitchService; use Flowpack\DecoupledContentStore\Transfer\ContentReleaseCleaner; use Flowpack\Prunner\PrunnerApiService; use Flowpack\Prunner\ValueObject\PipelineName; +use Neos\Error\Messages\Message; use Neos\Flow\Annotations as Flow; use Neos\Flow\I18n\Translator; use Neos\Flow\Mvc\Controller\ActionController; @@ -101,6 +105,9 @@ class BackendController extends ActionController #[Flow\Inject] protected BackendDateFormatter $backendDateFormatter; + #[Flow\Inject] + protected QuickPublishPreviewService $quickPublishPreviewService; + /** * @Flow\InjectConfiguration("redisContentStores") * @var array @@ -310,11 +317,88 @@ public function toggleConfigEpochAction(string $redisInstanceIdentifier) $this->redirect('index', null, null, ['contentStore' => $redisInstanceIdentifier->getIdentifier()]); } - private function translateById(string $labelId): string + /** + * Where the documents of a quick release are named. The index page offers it only while automatic releases are + * paused, which is the situation a quick release exists for. + */ + public function quickPublishFormAction(?string $contentStore = null, string $nodeIdentifiers = ''): void + { + $this->view->assign('contentStore', $contentStore); + $this->view->assign('nodeIdentifiers', $nodeIdentifiers); + } + + /** + * What the given identifiers resolve to, before anything is published. The identifiers end up in a shell command + * in the pipeline, so this is also where anything which is not a node identifier is rejected. + */ + public function quickPublishPreviewAction(string $nodeIdentifiers, ?string $contentStore = null): ?string + { + if ($this->request->getHttpRequest()->getMethod() !== 'POST') { + $this->response->setStatusCode(405); + return 'Method not allowed'; + } + + try { + $identifiers = NodeIdentifiers::fromUserInput($nodeIdentifiers); + } catch (Exception $exception) { + $this->addFlashMessage($exception->getMessage(), '', Message::SEVERITY_ERROR); + $this->redirect('quickPublishForm', null, null, [ + 'contentStore' => $contentStore, + 'nodeIdentifiers' => $nodeIdentifiers + ]); + + return null; + } + + $previewRows = $this->quickPublishPreviewService->preview($identifiers, $this->controllerContext); + + $this->view->assign('contentStore', $contentStore); + $this->view->assign('nodeIdentifiers', (string)$identifiers); + $this->view->assign('previewRows', $previewRows); + $this->view->assign('publishedRowCount', $this->quickPublishPreviewService->countPublishedRows($previewRows)); + + return null; + } + + public function quickPublishAction(string $nodeIdentifiers, ?string $contentStore = null): ?string + { + if ($this->request->getHttpRequest()->getMethod() !== 'POST') { + $this->response->setStatusCode(405); + return 'Method not allowed'; + } + + try { + $contentReleaseIdentifier = $this->contentReleaseManager->startQuickContentRelease( + NodeIdentifiers::fromUserInput($nodeIdentifiers) + ); + } catch (Exception $exception) { + // both the identifier check and the manager phrase their messages for the person reading this page + $this->addFlashMessage($exception->getMessage(), '', Message::SEVERITY_ERROR); + $this->redirect('quickPublishForm', null, null, [ + 'contentStore' => $contentStore, + 'nodeIdentifiers' => $nodeIdentifiers + ]); + + return null; + } + + $this->addFlashMessage($this->translateById( + 'quickPublish.scheduled.flashMessage', + [$contentReleaseIdentifier->getIdentifier()] + )); + $this->redirect('index', null, null, $contentStore !== null ? ['contentStore' => $contentStore] : []); + + return null; + } + + /** + * @param array $arguments + */ + private function translateById(string $labelId, array $arguments = []): string { return (string)$this->translator->translateById( $labelId, - [], + $arguments, null, null, 'Main', diff --git a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php index 03192fc..c7c48f8 100644 --- a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php +++ b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php @@ -95,6 +95,22 @@ public function skipReason(NodeInterface $node, NodeInterface $siteNode): ?strin return null; } + /** + * Why a node somebody named by identifier must not go into a content release, or NULL if it may. + * + * The node type is part of the answer here, unlike in {@see skipReason()}: a node which was named by hand did + * not come out of the FlowQuery filter, so nothing else checked it. + */ + public function skipReasonForNamedNode(NodeInterface $node, NodeInterface $siteNode): ?string + { + $skipReason = $this->skipReason($node, $siteNode); + if ($skipReason !== null) { + return $skipReason; + } + + return $this->matchesNodeTypeWhitelist($node) ? null : 'not of a node type which is published'; + } + private static function isOrphaned(NodeInterface $node, NodeInterface $siteNode): bool { $parentNode = self::getParentNodeOrNull($node); diff --git a/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php b/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php index 1228b51..e62bb54 100644 --- a/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php +++ b/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php @@ -66,6 +66,40 @@ public function nodeInContexts(string $nodeIdentifier, Site $site, string $works } } + /** + * Iterate over the node with the given identifier in every dimension it exists in, together with the site node + * it belongs to. + * + * Unlike {@see nodeInContexts()} this searches every site and hands out the site node as well, which callers + * need for the orphan check. It also does not report "not found" per site: a node is part of one site, so all + * the others not having it is the normal case rather than an error. + * + * @return \Generator the site node and the node, in that order + */ + public function nodeVariantsWithSiteNode(string $nodeIdentifier, string $workspaceName = 'live'): \Generator + { + foreach ($this->sites() as $site) { + $nodeFound = false; + + // hidden nodes are shown here regardless of "recurseHiddenContent": somebody named this node by its + // identifier, and "it is hidden" is the answer they need - a context which filters it away could only + // report that it does not exist + foreach ($this->siteNodeInContexts($site, $workspaceName, true) as $siteNode) { + $node = $siteNode->getContext()->getNodeByIdentifier($nodeIdentifier); + if ($node instanceof NodeInterface) { + $nodeFound = true; + yield [$siteNode, $node]; + } + } + + if ($nodeFound) { + // getNodeByIdentifier() looks the node up in the whole content repository rather than inside the + // site, so every further site would hand out the very same variants again + return; + } + } + } + /** * Iterate over all sites * @@ -83,10 +117,14 @@ public function sites(): \Generator /** * Iterate over the site node in all available presets (if it exists) * + * @param bool|null $invisibleContentShown NULL follows the "nodeRendering.recurseHiddenContent" setting * @return \Generator */ - public function siteNodeInContexts(Site $site, string $workspaceName = 'live'): \Generator - { + public function siteNodeInContexts( + Site $site, + string $workspaceName = 'live', + ?bool $invisibleContentShown = null + ): \Generator { $allowedContextCombinations = $this->contentDimensionCombinator->getAllAllowedCombinations(); foreach ($allowedContextCombinations as $dimensionContextCombination) { @@ -95,7 +133,7 @@ public function siteNodeInContexts(Site $site, string $workspaceName = 'live'): 'workspaceName' => $workspaceName, 'dimensions' => $dimensionContextCombination, 'targetDimensions' => [], - 'invisibleContentShown' => $this->recurseHiddenContent + 'invisibleContentShown' => $invisibleContentShown ?? $this->recurseHiddenContent )); $siteNode = $contentContext->getNode('/sites/' . $site->getNodeName()); diff --git a/Classes/QuickPublish/Dto/NodeIdentifiers.php b/Classes/QuickPublish/Dto/NodeIdentifiers.php index c788644..7020f92 100644 --- a/Classes/QuickPublish/Dto/NodeIdentifiers.php +++ b/Classes/QuickPublish/Dto/NodeIdentifiers.php @@ -35,13 +35,36 @@ private function __construct(array $identifiers) } /** + * The form the pipeline passes them in. + * * @throws Exception if the list is empty or holds anything which is not a node identifier */ public static function fromCommaSeparatedString(string $nodeIdentifiers): self + { + return self::fromTokens(explode(',', $nodeIdentifiers)); + } + + /** + * What somebody pasted into the backend form - one identifier per line, or separated by commas, or both. + * + * @throws Exception if the list is empty or holds anything which is not a node identifier + */ + public static function fromUserInput(string $nodeIdentifiers): self + { + $tokens = preg_split('/[\s,;]+/', $nodeIdentifiers); + + return self::fromTokens($tokens === false ? [] : $tokens); + } + + /** + * @param array $tokens + * @throws Exception if the list is empty or holds anything which is not a node identifier + */ + private static function fromTokens(array $tokens): self { $identifiers = []; - foreach (explode(',', $nodeIdentifiers) as $identifier) { + foreach ($tokens as $identifier) { $identifier = trim($identifier); if ($identifier === '') { continue; diff --git a/Classes/QuickPublish/Dto/QuickPublishPreviewRow.php b/Classes/QuickPublish/Dto/QuickPublishPreviewRow.php new file mode 100644 index 0000000..d694677 --- /dev/null +++ b/Classes/QuickPublish/Dto/QuickPublishPreviewRow.php @@ -0,0 +1,106 @@ +nodeIdentifier = $nodeIdentifier; + $this->title = $title; + $this->nodePath = $nodePath; + $this->dimensions = $dimensions; + $this->nodeTypeName = $nodeTypeName; + $this->backendUri = $backendUri; + $this->skipReason = $skipReason; + } + + public static function forNode( + string $nodeIdentifier, + string $title, + string $nodePath, + string $dimensions, + string $nodeTypeName, + ?string $backendUri, + ?string $skipReason + ): self { + return new self($nodeIdentifier, $title, $nodePath, $dimensions, $nodeTypeName, $backendUri, $skipReason); + } + + public static function forNodeWhichCannotBeFound(string $nodeIdentifier): self + { + return new self($nodeIdentifier, '', '', '', '', null, 'not found in any site and dimension'); + } + + public function getNodeIdentifier(): string + { + return $this->nodeIdentifier; + } + + public function getTitle(): string + { + return $this->title; + } + + public function getNodePath(): string + { + return $this->nodePath; + } + + public function getDimensions(): string + { + return $this->dimensions; + } + + public function getNodeTypeName(): string + { + return $this->nodeTypeName; + } + + public function getBackendUri(): ?string + { + return $this->backendUri; + } + + public function getSkipReason(): ?string + { + return $this->skipReason; + } + + public function isPublished(): bool + { + return $this->skipReason === null; + } +} diff --git a/Classes/QuickPublish/QuickPublishNodeEnumerator.php b/Classes/QuickPublish/QuickPublishNodeEnumerator.php index 8c1a991..587d3df 100644 --- a/Classes/QuickPublish/QuickPublishNodeEnumerator.php +++ b/Classes/QuickPublish/QuickPublishNodeEnumerator.php @@ -170,7 +170,8 @@ private function enumerateGivenNodes( foreach ($nodeIdentifiers as $nodeIdentifier) { $contentCacheFlushed = false; - foreach ($this->nodeVariants($nodeIdentifier, $workspaceName) as [$siteNode, $nodeToEnumerate]) { + $variants = $this->nodeContextCombinator->nodeVariantsWithSiteNode($nodeIdentifier, $workspaceName); + foreach ($variants as [$siteNode, $nodeToEnumerate]) { if (!$contentCacheFlushed) { // the tags carry the node identifier and the workspace, so one flush covers every dimension $this->flushContentCacheForNode($nodeToEnumerate, $nodeIdentifier, $contentReleaseLogger); @@ -179,10 +180,7 @@ private function enumerateGivenNodes( $contextPath = $nodeToEnumerate->getContextPath(); - $skipReason = $this->documentNodeFilter->skipReason($nodeToEnumerate, $siteNode); - if ($skipReason === null && !$this->documentNodeFilter->matchesNodeTypeWhitelist($nodeToEnumerate)) { - $skipReason = 'not of a node type which is published'; - } + $skipReason = $this->documentNodeFilter->skipReasonForNamedNode($nodeToEnumerate, $siteNode); if ($skipReason !== null) { // warn rather than debug: somebody asked for this node by hand and will not see it change $contentReleaseLogger->warn( @@ -214,36 +212,6 @@ private function enumerateGivenNodes( return $nodesToRender; } - /** - * The node in every dimension it exists in, together with the site node it belongs to. - * - * {@see NodeContextCombinator::nodeInContexts()} hands out the same variants, but not the site node the orphan - * check needs - and it reports "not found" per site, while a node not being part of a site is the normal case - * for all but one of them. - * - * @return \Generator - */ - private function nodeVariants(string $nodeIdentifier, string $workspaceName): \Generator - { - foreach ($this->nodeContextCombinator->sites() as $site) { - $nodeFound = false; - - foreach ($this->nodeContextCombinator->siteNodeInContexts($site, $workspaceName) as $siteNode) { - $node = $siteNode->getContext()->getNodeByIdentifier($nodeIdentifier); - if ($node instanceof NodeInterface) { - $nodeFound = true; - yield [$siteNode, $node]; - } - } - - if ($nodeFound) { - // getNodeByIdentifier() looks the node up in the whole content repository rather than inside the - // site, so every further site would hand out the very same variants again - return; - } - } - } - /** * A quick release renders a handful of documents into a copy of a finished release. If their cache entries are * still valid, that rendering is served straight from the content cache and the release publishes exactly what diff --git a/Classes/QuickPublish/QuickPublishPreviewService.php b/Classes/QuickPublish/QuickPublishPreviewService.php new file mode 100644 index 0000000..cfe15f2 --- /dev/null +++ b/Classes/QuickPublish/QuickPublishPreviewService.php @@ -0,0 +1,89 @@ + one row per dimension variant of every given identifier + */ + public function preview(NodeIdentifiers $nodeIdentifiers, ControllerContext $controllerContext): array + { + $rows = []; + + foreach ($nodeIdentifiers as $nodeIdentifier) { + $nodeFound = false; + + foreach ($this->nodeContextCombinator->nodeVariantsWithSiteNode($nodeIdentifier) as [$siteNode, $node]) { + $nodeFound = true; + $rows[] = QuickPublishPreviewRow::forNode( + $nodeIdentifier, + $node->getLabel(), + $node->getPath(), + self::describeDimensions($node), + $node->getNodeType()->getName(), + $this->backendUri($node, $controllerContext), + $this->documentNodeFilter->skipReasonForNamedNode($node, $siteNode) + ); + } + + if (!$nodeFound) { + $rows[] = QuickPublishPreviewRow::forNodeWhichCannotBeFound($nodeIdentifier); + } + } + + return $rows; + } + + /** + * @param array $rows + */ + public function countPublishedRows(array $rows): int + { + return count(array_filter($rows, static fn(QuickPublishPreviewRow $row): bool => $row->isPublished())); + } + + private function backendUri(NodeInterface $node, ControllerContext $controllerContext): string + { + return $controllerContext->getUriBuilder()->reset()->uriFor( + 'index', + ['node' => $node->getContextPath()], + 'Backend', + 'Neos.Neos.Ui' + ); + } + + private static function describeDimensions(NodeInterface $node): string + { + $dimensions = []; + foreach ($node->getContext()->getDimensions() as $dimensionName => $dimensionValues) { + $dimensions[] = $dimensionName . ': ' . implode(', ', $dimensionValues); + } + + return implode(' | ', $dimensions); + } +} diff --git a/Configuration/Policy.yaml b/Configuration/Policy.yaml index e8ed1ce..075d3cf 100644 --- a/Configuration/Policy.yaml +++ b/Configuration/Policy.yaml @@ -4,11 +4,12 @@ privilegeTargets: matcher: 'administration/contentstore' 'Neos\Flow\Security\Authorization\Privilege\Method\MethodPrivilege': - # Operating the switch which suppresses automatically triggered content releases. Separate from the module - # privilege above, so an installation which lets editors watch the module can still restrict who may pause - # releases and thereby stop everybody's publishes from going live. + # Operating the switch which suppresses automatically triggered content releases, and publishing single + # documents while it is off. Separate from the module privilege above, so an installation which lets editors + # watch the module can still restrict who may pause releases and thereby stop everybody's publishes from going + # live. Quick publish sits behind the same target because it is the other half of that workflow. 'Flowpack.DecoupledContentStore:ReleaseControl': - matcher: 'method(Flowpack\DecoupledContentStore\Controller\BackendController->(pauseAutomaticReleases|resumeAutomaticReleases)Action())' + matcher: 'method(Flowpack\DecoupledContentStore\Controller\BackendController->(pauseAutomaticReleases|resumeAutomaticReleases|quickPublishForm|quickPublishPreview|quickPublish)Action())' roles: 'Neos.Neos:Administrator': diff --git a/Resources/Private/BackendFusion/Integration/Backend.Index.fusion b/Resources/Private/BackendFusion/Integration/Backend.Index.fusion index 302930f..eb4dd06 100644 --- a/Resources/Private/BackendFusion/Integration/Backend.Index.fusion +++ b/Resources/Private/BackendFusion/Integration/Backend.Index.fusion @@ -181,6 +181,23 @@ prototype(Flowpack.DecoupledContentStore:ContentStoreActions) < prototype(Neos.F ` } + // a quick release publishes into a copy of the release which is live, so it only makes sense while the automatic + // ones are paused - otherwise the next automatic release renders everything anyway + quickPublish = Neos.Fusion:Component { + @if.isPaused = ${automaticReleasePauseState} + @if.hasAccess = ${Security.hasAccess('Flowpack.DecoupledContentStore:ReleaseControl')} + + renderer = afx` + + + + ` + } + publishAllWithoutValidation = Neos.Fusion:Component { _publishAllWithoutValidationUri = Neos.Fusion:UriBuilder { action = 'publishAllWithoutValidation' diff --git a/Resources/Private/BackendFusion/Integration/Backend.QuickPublish.fusion b/Resources/Private/BackendFusion/Integration/Backend.QuickPublish.fusion new file mode 100644 index 0000000..ea11b6a --- /dev/null +++ b/Resources/Private/BackendFusion/Integration/Backend.QuickPublish.fusion @@ -0,0 +1,157 @@ +// A backend module is dispatched as a sub-request whose arguments Neos reads from "moduleArguments" alone (see +// Neos\Neos\Controller\Backend\ModuleController::indexAction), so every form field an action expects has to be +// named inside that namespace - a field posted at the top level never reaches the action. __csrfToken is a Flow +// argument of the outer request and stays where it is. The other buttons in this module do not run into this, +// because they carry their parameters in the formaction URI, which the UriBuilder namespaces already. + +Flowpack.DecoupledContentStore.BackendController.quickPublishForm = Neos.Fusion:Component { + + // Context Variables: + // - contentStore: string content store identifier, or NULL + // - nodeIdentifiers: what was typed in before, when the form is shown again after an error + + _previewUri = Neos.Fusion:UriBuilder { + action = 'quickPublishPreview' + } + + renderer = afx` +
+ +
+ + +
+

+
+ + + + + + +
+
+
+ +
+ +
+ ` +} + +Flowpack.DecoupledContentStore.BackendController.quickPublishPreview = Neos.Fusion:Component { + + // Context Variables: + // - contentStore: string content store identifier, or NULL + // - nodeIdentifiers: the validated identifiers, comma separated + // - previewRows: list of QuickPublishPreviewRow objects, one per dimension variant + // - publishedRowCount: how many of them a quick release would actually render + + renderer = Neos.Fusion:Component { + _quickPublishUri = Neos.Fusion:UriBuilder { + action = 'quickPublish' + } + + _renderedTableBody = Neos.Fusion:Loop { + items = ${previewRows} + itemRenderer = afx` +
+ + + + + + + ` + } + + renderer = afx` +
+ +
+ + +
+

+
+ {item.title} + {item.title} + {' '} + {item.skipReason} + {item.nodePath}{item.dimensions}{item.nodeTypeName}{item.nodeIdentifier}
+ + + + + + + + + + + {props._renderedTableBody} + +
TitlePathDimensionsNode TypeIdentifier
+ +

+ +

+
0} + method="post" + action={props._quickPublishUri} + style="margin-top: 2rem;" + > + + + + +
+
+
+ + +
+ + + + +
+ ` + } +} + +prototype(Flowpack.DecoupledContentStore:QuickPublishBackLink) < prototype(Neos.Fusion:Component) { + renderer = afx` + + Content Store + + ` +} diff --git a/Resources/Private/Translations/de/Main.xlf b/Resources/Private/Translations/de/Main.xlf index 3e55899..395e70d 100644 --- a/Resources/Private/Translations/de/Main.xlf +++ b/Resources/Private/Translations/de/Main.xlf @@ -42,6 +42,53 @@ Automatic content releases have been paused on {0}, so your changes do not go live yet. {1} release(s) are waiting. An administrator has to resume them in the Content Store module. Automatische Content-Releases wurden am {0} pausiert, Ihre Änderungen gehen daher noch nicht live. {1} Release(s) warten. Ein Administrator muss sie im Modul „Content Store“ wieder fortsetzen. + + + Quick publish pages + Seiten schnell veröffentlichen + + + Quick publish pages + Seiten schnell veröffentlichen + + + A quick release takes the content release which is live now, re-renders the pages you name here into a copy of it and publishes that. Everything else goes live exactly as it is now: pages which embed the ones you publish keep their old teasers, titles and navigation entries until the next full release. + Ein schnelles Release übernimmt das aktuell veröffentlichte Content-Release, rendert die hier genannten Seiten in eine Kopie davon neu und veröffentlicht diese. Alles andere geht unverändert live: Seiten, die die veröffentlichten Seiten einbinden, behalten ihre alten Teaser, Titel und Navigationseinträge bis zum nächsten vollständigen Release. + + + Node identifiers, one per line or comma separated list: + Node-Identifier, einer pro Zeile oder mit Komma separierte Liste: + + + Show what would be published + Anzeigen, was veröffentlicht würde + + + These pages would be published + Diese Seiten würden veröffentlicht + + + One row per dimension. Rows marked in red are not published - check them before you continue, because a page you meant to fix would stay as it is. + Eine Zeile pro Dimension. Rot markierte Zeilen werden nicht veröffentlicht – prüfen Sie diese, bevor Sie fortfahren, denn eine Seite, die Sie korrigieren wollten, bliebe unverändert. + + + + Publish {0} page(s) now + {0} Seite(n) jetzt veröffentlichen + + + Change the list + Liste ändern + + + None of these pages can be published, so there is nothing a quick release would change. + Keine dieser Seiten kann veröffentlicht werden, ein schnelles Release würde also nichts ändern. + + + + Content release {0} was started and publishes the pages you named. + Content-Release {0} wurde gestartet und veröffentlicht die genannten Seiten. + diff --git a/Resources/Private/Translations/en/Main.xlf b/Resources/Private/Translations/en/Main.xlf index feacfbe..7ef2cc4 100644 --- a/Resources/Private/Translations/en/Main.xlf +++ b/Resources/Private/Translations/en/Main.xlf @@ -33,6 +33,42 @@ Automatic content releases have been paused on {0}, so your changes do not go live yet. {1} release(s) are waiting. An administrator has to resume them in the Content Store module. + + + Quick publish pages + + + Quick publish pages + + + A quick release takes the content release which is live now, re-renders the pages you name here into a copy of it and publishes that. Everything else goes live exactly as it is now: pages which embed the ones you publish keep their old teasers, titles and navigation entries until the next full release. + + + Node identifiers, one per line or comma separated list: + + + Show what would be published + + + These pages would be published + + + One row per dimension. Rows marked in red are not published - check them before you continue, because a page you meant to fix would stay as it is. + + + + Publish {0} page(s) now + + + Change the list + + + None of these pages can be published, so there is nothing a quick release would change. + + + + Content release {0} was started and publishes the pages you named. + diff --git a/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php b/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php index bee2cbe..ad2458c 100644 --- a/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php +++ b/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php @@ -85,6 +85,31 @@ public function testAnEmptyListIsRefused(): void NodeIdentifiers::fromCommaSeparatedString(' , '); } + public function testTheBackendFormAcceptsOneIdentifierPerLine(): void + { + $nodeIdentifiers = NodeIdentifiers::fromUserInput( + " " . self::IDENTIFIER . "\r\n\n" . self::OTHER_IDENTIFIER . " \n" + ); + + self::assertSame([self::IDENTIFIER, self::OTHER_IDENTIFIER], $nodeIdentifiers->jsonSerialize()); + } + + public function testTheBackendFormAlsoAcceptsSeparatorsSomebodyPastedIn(): void + { + // an identifier list copied out of a log or a spreadsheet arrives with commas or semicolons + $nodeIdentifiers = NodeIdentifiers::fromUserInput(self::IDENTIFIER . '; ' . self::OTHER_IDENTIFIER . ','); + + self::assertSame([self::IDENTIFIER, self::OTHER_IDENTIFIER], $nodeIdentifiers->jsonSerialize()); + } + + public function testTheBackendFormRefusesAnythingWhichIsNotAnIdentifier(): void + { + $this->expectException(Exception::class); + $this->expectExceptionCode(1786958510); + + NodeIdentifiers::fromUserInput("/sites/test/products\n" . self::IDENTIFIER); + } + public function testTheListIsHandedToThePipelineAsItWasRead(): void { // the pipeline passes it on as a prunner variable From 7919930d162e51ef5849f14f8e91e0a625129ffe Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 12:55:52 +0200 Subject: [PATCH 08/23] STEP 8: update readme --- .../Concepts/QuickContentReleases.md | 327 ++++++++++++++++++ README.md | 166 ++++++++- 2 files changed, 492 insertions(+), 1 deletion(-) create mode 100644 Documentation/Concepts/QuickContentReleases.md diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md new file mode 100644 index 0000000..95d5d8a --- /dev/null +++ b/Documentation/Concepts/QuickContentReleases.md @@ -0,0 +1,327 @@ +# Quick Content Releases + +Getting a single fixed page live in minutes instead of waiting for a full publish: pause the automatic release, copy +the last release forward, re-render only the pages you name. + +This document is about *why* the feature looks the way it does — the measurement it is based on, the design decisions +and the constraints they follow from. For how to use and configure it, see the +[Quick Content Releases](../../README.md#quick-content-releases) chapter of the README. + +## 1. The problem: rendering is the release + +A release is nothing but a set of Redis keys named `contentStore::`: hashes keyed by URL for +`renderedDocuments` and `renderedMetadata`, a sorted set `meta:urls`, plus whatever an installation registers under +`Flowpack.DecoupledContentStore.redisKeyPostfixesForEachRelease`. Everything the delivery layer reads is in there. + +`NodeRenderOrchestrator` already implements a "check whether it is already rendered" loop — but it checks the *Neos +content cache*, per node, for the whole enumeration. On a large site that per-node work over every page in every +dimension is most of the release, and a release which changes a single page still produces a set of keys that is +identical to the previous release everywhere else. + +### Measured: rendering is 92% of a full release + +A full release of an installation with 18 015 documents and two renderers per document (36 030 render jobs, ~320 MB of +release data), triggered as *Publish All without validation* so the content cache was flushed first — the worst case, +and exactly the situation a bugfix release runs into. Full breakdown in section 9. + +| Phase | Duration | Share | +|---|---:|---:| +| prepare | 2 s | 0.1 % | +| enumerate | 47 s | 2.0 % | +| **render** | **2 183 s** | **91.6 %** | +| validate | 145 s | 6.1 % | +| transfer (two additional content stores) | 3 s | 0.1 % | +| switch | 1 s | 0.0 % | +| **total** | **2 384 s ≈ 40 min** | | + +Two conclusions come out of this table, and both shaped the design: + +* **The copy-forward is aimed at the right thing.** An earlier draft also planned a "transfer only the changed + documents" optimisation. Transfer is three seconds; that work was dropped. +* **Removing rendering moves the goalposts once.** With rendering gone, the validators become the entire remaining + cost, which is why validation is scoped to the changed URLs as well (section 6). + +## 2. The approach: copy the last release forward, re-render the exceptions + +The obvious formulation — "iterate every node; if it was named, re-render it, otherwise take the page from the old +release" — is right, and there is a cheaper way to express it. Copy the entire previous release to the new release ID +at the Redis level first, then enumerate *only* the named nodes. `NodeRenderOrchestrator` then does its normal job on +a two-item enumeration and overwrites those hash fields. + +Everything downstream stays as it is. The new release is a complete, valid release; it simply differs from the +previous one in a handful of fields. + +| Pipeline stage | Full release | Quick release | +|---|---|---| +| prepare | create release, terminate others | unchanged | +| copy | — | server-side copy of the registered release keys from the live release | +| enumerate | all documents × all dimensions × all renderers | only the named node identifiers | +| render | all workers plus any project-specific render tasks | orchestrator + a few workers, nothing else | +| validate | full consistency validators | scoped to the changed URLs, see section 6 | +| transfer | full copy to every configured content store | unchanged (3 s measured) | +| switch | unchanged | unchanged | + +### Measured: a quick release + +The same installation, one document changed, published as a quick release: + +| | full release | quick release | +|---|---:|---:| +| `enumeration:documentNodes` | 36 030 entries | **56** (28 dimension variants × 2 renderers) | +| copy of 13 registered keys | — | **0.28 s** | +| rendering | 2 183 s | **10 s** | +| schema validation of the documents | 145 s | **2 s** | +| total | ~40 min | **~107 s**, 80 s of it in project-specific validators which still walk the whole release | + +Comparing the two releases key by key afterwards: every content key is identical to the byte except inside the +re-rendered URLs, and the cardinality of every hash matches. Two details are worth knowing because they look like +bugs and are not: + +* A quick release reports a **smaller release size**. `RedisContentReleaseSizeService` sums `MEMORY USAGE` over *all* + keys of a release, including the bookkeeping — and the two keys with one entry per enumerated node + (`renderAttempts`, `enumeration:documentNodes`) are 21 MB for 36 030 documents against 20 KB for 56. The content is + identical; the number is not comparable between the two pipelines. +* Copied keys can occupy slightly *less* memory than their source. `COPY` rebuilds the value, so it lands in a + freshly sized encoding without the rehash slack the original accumulated while it was being filled. + +## 3. Which keys are copied is configuration + +`RedisReleaseCopyService` copies every release key registered with `copyOnQuickRelease: true`. This flag is the main +reason the copy belongs in this package rather than in a site package: only the installation knows which of its keys +carry content the delivery layer reads and which describe the build of one particular release. + +The default is `false`, and an absent flag means `false`. That is the safe direction: a key which should have been +copied shows up as missing content, while a key which should not have been copied describes a *different* release and +is much harder to notice. The package's own content keys (`data`, `meta:urls`, `renderedDocuments`, +`renderedMetadata`) are `true`; its enumeration, job queue and statistics keys are `false`, because the quick +pipeline writes those itself. + +The copy is the native `COPY` command (Redis 6.2+) — server-side, nothing over the wire, no Lua. Since the package +cannot assume 6.2, the service reads `INFO server` and aborts with a message naming the required version rather than +failing cryptically on an older server. + +### The source release is the live one, and it is verified + +The copy source is read from `contentStore:current`: by definition a release which passed validation and was switched +live. That alone is not enough, because switching to an arbitrary release by hand is possible, so before copying +anything the service checks that the source's `meta:info` status is `success` and that the required keys it is about +to inherit exist. If either check fails, it aborts and tells the operator to run a normal release — it never falls +back to "the newest release we can find". + +That second check is narrower than it first looks, and deliberately so: it covers the keys marked **both** +`isRequired` and `copyOnQuickRelease`. Keys the quick release builds itself do not have to exist in the source, and +more importantly, `isRequired` means "must exist if it is transferred" everywhere else in this package — +`RedisReleaseSwitchService` and `ContentReleaseSynchronizer` both test it only for keys they actually transfer. An +installation which retires a key by setting `transfer: false` therefore leaves it registered as required, and a copy +which demanded every required key would refuse to build on a perfectly good release. + +## 4. A separate pipeline + +`do_quick_content_release` in `pipelines_template.yml` is a pipeline of its own rather than a flag on +`do_content_release`, because that one is configured with `queue_strategy: replace` — a queued automatic release +would silently replace a quick release. + +For the same reason the quick pipeline does **not** use `queue_strategy: replace` either: a quick release publishes +exactly the documents somebody named, so a queued one must not be thrown away by the next one. + +Details of the pipeline which are not obvious from reading it: + +* The copy task is called **`prepare_copy_previous_release`**, not `copy_previous_release`. The backend module's + details view groups tasks by name prefix (`prepare_`, `enumerate_`, `render_`, `validate_`, `transfer_`, + `switch_`), so a task outside those prefixes is not rendered anywhere. Every step the UI shows is prefix-driven, + which is why adding a phase needed no UI change at all. +* **Fewer render workers** than a full release. A quick release enumerates a handful of documents; further workers + only add Flow bootstraps. +* **No `flushContentCacheIfRequired`.** It flushes the whole content cache, while the quick enumerator flushes + exactly the nodes it re-renders. +* **Validation runs unconditionally**, not behind a `validate` variable. Scoped validation of a quick release costs + nothing worth skipping. +* Every task which is identical to the one in `do_content_release` should be a **YAML alias** of it, so the two + pipelines cannot drift apart. `depends_on` names a task and resolves per pipeline, which is what makes aliasing + whole tasks across pipelines safe. + +### Scheduling guards + +`ContentReleaseManager::startQuickContentRelease()` refuses in two cases, both with a message written to be read by +the person who pressed the button: + +* **Nothing is live to copy.** The release would hold the named documents and nothing else. +* **Another quick release is running or queued.** This is the subtle one: the copy source is resolved when the + release is *scheduled*, so a second quick release queued behind the first would copy the release the first is + about to replace, and silently undo it. + +Like *Publish All*, a quick release is an explicit request and is deliberately **not** blocked by the pause switch — +a paused automatic release is the situation it exists for. `cancelAllRunningContentReleases()` and +`cancelRunningContentRelease()` cover both pipelines, since a running quick release ends up switched live just like a +full one. + +## 5. Enumeration of named nodes + +`QuickPublishNodeEnumerator` flushes the content cache for each target node first, then writes the enumeration for +those nodes only, through `NodeRenderingExtensionManager::enumerateDocumentNode()` so that every configured renderer +is covered. + +The hidden / orphaned / node-type guards are shared with the full enumeration through +`NodeEnumeration/Domain/Service/DocumentNodeFilter`, which expresses `nodeTypeWhitelist` twice: as the FlowQuery +filter string the full enumeration passes to `find()`, and as a check for a single node, which is what an enumerator +starting from identifiers needs. The node-type check is deliberately **not** part of the shared `skipReason()`: +`NodeEnumerator` adds the site node to its result without passing it through the FlowQuery filter, so folding the +node type into the shared guard would silently drop site nodes whose type is excluded. + +Two failures abort the task rather than shrinking the release quietly: an identifier which resolves in no site and +dimension, and an enumeration which ends up empty. A quick release which renders nothing would publish the release it +copied and look like a successful publish while the change is nowhere. Everything it skips is logged as a warning +rather than at debug level, because somebody asked for those documents by hand. + +The identifiers are a value object, `QuickPublish/Dto/NodeIdentifiers`, which rejects anything that is not a UUID. +This is not cosmetic: the list travels through a pipeline variable into a shell command, so unvalidated input is a +command-injection hole. The check lives at the point where the list is read, not only in the backend form. + +Variants of a node are resolved through `NodeContextCombinator::nodeVariantsWithSiteNode()` — via `sites()` and +`siteNodeInContexts()` rather than `nodeInContexts()`, because the orphan check needs the site node, and +`getNodeByIdentifier()` searches the whole content repository rather than one site, so iterating every site would +hand out the same variants once per site. + +That lookup shows invisible content regardless of `nodeRendering.recurseHiddenContent`, which defaults to `false`. +The setting is about recursing into hidden content while walking the tree; applied to a lookup by identifier it makes +a hidden page indistinguishable from one which does not exist, and the first hidden page tried reported "not found in +any site and dimension". `siteNodeInContexts()` therefore takes an `$invisibleContentShown` override, `NULL` keeping +the configured behaviour for the full enumeration. Hidden pages are still not published — the skip reason says +"hidden", on the confirmation page and in the pipeline log alike. + +## 6. Validation scoped to the changed URLs + +With rendering gone, validation is the whole cost of a quick release. It is also almost entirely wasted work: after a +copy-forward, every document except the handful just re-rendered is byte-for-byte what the previous release was +validated on. + +* The enumerator writes the URLs it produced into the release key `quickPublish:changedUrls`, registered with + `transfer: false` and `isRequired: false`. It is written with `sAdd`, and an empty set is never written, so "the key + exists" and "this is a quick release" are the same statement. +* `QuickPublish/ContentReleaseScope` is the one accessor the whole pipeline shares. `getChangedUrls()` returns `NULL` + for an ordinary release, meaning "validate everything", and the URL list for a quick release. Validators which know + nothing about quick releases keep working unchanged; validators which opt in narrow their read from `hGetAll` to + `hMGet`. `countPublishedUrls()` — the `meta:urls` cardinality — is there for the threshold check, which needs it + for the live release as well as the new one. +* The URLs are built with `NodeRenderingUriService::buildNodeUri()`, the same call `DocumentRenderer` makes before + rendering, so the strings match the keys the release is written under, per renderer and per dimension. They are + built in a **second pass** after every identifier has been resolved, not while enumerating: `buildNodeUri()` marks + the security context as initialized as a side effect, which would change what the node lookups of the remaining + identifiers are allowed to see. A URL which cannot be built aborts the task rather than being left out, because a + missing entry would silently exclude a changed document from validation. + +`NULL` versus an empty list is the trap in this API. A validator which reads "no scope" as "no URLs to check" waves +every ordinary release through, so the two cases have to be handled explicitly. + +### `contentReleaseValidation:validate` had to be adapted, not just scoped + +This is a trap rather than an optimisation. The validator compares the enumeration count of the new release against +the live one and aborts below 70%. A quick release deliberately enumerates a handful of documents instead of all of +them, so the check would fail every single time. For a quick release it compares the number of *published* URLs +instead, which after a copy-forward equals the previous release. + +Any project validator which reasons about the size of the enumeration has the same problem, and the failure mode is +the good one — the release is refused rather than published wrongly — but it needs the same treatment. + +## 7. The pause switch + +A quick release only makes sense while ordinary releases are held back, so the pause is part of the same design. + +The state is a Redis hash on the primary instance, `contentStore:automaticReleasesPaused`, outside the per-release +key space so pruning never touches it, holding `pausedAt`, `accountId` and `suppressedReleaseCount`. A hash rather +than a JSON string so the counter can be raised with `HINCRBY`, without a read-modify-write race against a +concurrent publish. `isPaused()` tests the `pausedAt` field, not the key: an increment racing a resume would +otherwise re-create the key with nothing but its counter and read as paused forever. + +`ContentReleaseManager::startIncrementalContentRelease()` is the single entry point for **all** automatic releases — +workspace publish, asset change, re-render after a rendering error — so one gate there covers every trigger. +`startFullContentRelease()` is deliberately not gated, so *Publish All* keeps working. + +Pause, resume and quick publish sit behind `Flowpack.DecoupledContentStore:ReleaseControl`, separate from the module +privilege so that an installation which lets editors watch the module can still restrict who may stop everybody's +publishes. The buttons are hidden with `Security.hasAccess()` rather than only protected, so nobody is offered a +control which then throws. + +### Why the content-module warning is a data source + +A pause stops *everybody's* publishes, so the warning has to reach editors inside the Neos content module, not only +administrators inside the Content Store module. The state is published as a Neos **data source**, not as an action on +the backend module: `ModulePrivilege` expands internally into a method privilege over every action of the module +controller, so a status action there would be readable only by the people who already have module access — precisely +not the editors the warning is for. Data sources are granted to `Neos.Neos:AbstractEditor` by Neos itself, need no +route of their own, and the script reaches them through `_NEOS_UI_routes.core.service.dataSource`, so no URI is +hardcoded. The data source returns nothing but the flag, the timestamp, the account and the counter, so widening its +access costs nothing. It must not be `final`: Neos instantiates data sources with `new`, and an unproxied class gets +no property injection. + +Two constraints on the script which registers under `Neos.Neos.Ui.resources.javascript`, both learned the hard way: +it must run **before** `Neos.Neos.UI:Host` and **without `defer`**. The UI host reads the inlined `_NEOS_UI_*` +globals through `getInlinedData()`, which does `delete window[...]` immediately after reading, so a deferred script +finds `_NEOS_UI_routes` already gone and cannot build the endpoint URI. The script therefore captures the route table +at evaluation time and waits for `DOMContentLoaded` before touching the DOM. + +The warning is translated **server-side in the data source**: Neos exposes no translation API to plain JavaScript, +and the service controllers already set the current locale from the backend user's interface language, so the message +arrives in the language the reader chose and the script only prints it. Timestamps go through +`BackendUi/BackendDateFormatter` on both paths, so the same moment does not read differently depending on which +screen shows it. + +## 8. The backend UI + +Three actions — the form, the confirmation page, the release — rendered through the package's existing `FusionView` +setup. + +The confirmation page deliberately does **not** show the live URL of a document. +`NodeRenderingUriService::buildNodeUri()` is the only thing which can produce the real URL — routing turns dimension +values into path prefixes, so a URL assembled from `uriPathSegment` properties would be wrong on any multi-language +site — and it marks the security context as initialized and swaps in a fake `ActionRequest` as a side effect. That is +fine in a CLI render and not fine in a backend request, where the same singleton is still serving the page. The row +shows title, node path, dimensions, node type, identifier and a link into the Neos backend instead, which answers +"are these the documents I mean?" without touching the request's security context. + +The reason a row gives for not being published comes from `DocumentNodeFilter::skipReasonForNamedNode()`, the same +method the enumerator calls, so what the confirmation page says will be skipped is exactly what the pipeline then +skips rather than a second implementation of the same rules. An identifier which resolves nowhere becomes a row +rather than an error: somebody who pasted five identifiers needs to see which one is wrong. + +One trap for anyone adding forms to this module: Neos dispatches a backend module as a sub-request and reads its +arguments from the `moduleArguments` namespace alone, so a field posted at the top level never reaches the action and +fails with "required argument is missing". Both forms name their fields `moduleArguments[…]`; `__csrfToken` belongs +to the outer request and stays where it is. The module's other buttons never hit this, because they carry their +parameters in a `formaction` URI, which the UriBuilder namespaces already. + +## 9. Appendix: the measured full release + +18 015 documents, two renderers per document, ~320 MB, triggered as *Publish All without validation* +(`flushContentCache: true`, `validate: false`). Task start offsets and durations, project-specific tasks folded into +one line: + +``` +task start+s dur s +prepare_finished 0 2.0 +enumerate_nodes 2 47.2 +project-specific enumerate/render tasks 2 ~219 (longest) +enumerate_finished 49 0.0 +render_orchestrator 49 2181.2 +render_1 … render_20 49 ~2182 +render_finished 2232 0.0 +validate_content 2232 0.0 (skipped, validate=false) +validate_documents_against_schema 2232 144.3 +validate_finished 2377 0.9 +transfer_content (two content stores) 2378 2.8 +transfer_finished 2380 0.6 +switch_* 2381 1.1 +``` + +Notes that matter for the design: + +* The orchestrator needed **two iterations**: 2 142 s for the first, 34 s for the second. The second iteration is the + copy-into-release pass over everything the first one rendered — which is exactly the work a copy-forward replaces. +* All project-specific render tasks finish inside the document-rendering window, so they never sit on the critical + path and cost nothing extra in a full release. In a quick release they are not run at all, which is why the keys + they write need `copyOnQuickRelease: true`. +* Transfer moved the whole release to both additional content stores in 2.8 s each. That was a local Docker network, + so a production network is slower — but with rendering at 2 183 s, transfer would have to get two orders of + magnitude worse before it mattered. +* Schema validation at 144 s is untouched by the rendering work and becomes the dominant cost of a quick release, + hence section 6. diff --git a/README.md b/README.md index 880ab8f..56128a9 100644 --- a/README.md +++ b/README.md @@ -53,13 +53,17 @@ delivery layer part in another software (e.g. a shop system) as an extension. - Allows rsyncing persistent assets around (should you need it) - Backend module with overview of _content releases_ (current release, switching releases, manual publish) +- Pausing the automatic releases, so nothing goes live while a change is being prepared +- *Quick content releases*: publish single documents into a copy of the release which is live, instead of + re-rendering everything This project is using the go-package [prunner](https://github.com/Flowpack/prunner) and [its Flow Package wrapper](https://github.com/Flowpack/Flowpack.Prunner) as the basis for orchestrating and executing a content release. ## Requirements -- Redis +- Redis — 6.2 or newer if you want to use [Quick Content Releases](#quick-content-releases), which copy a release + with the server-side `COPY` command. Everything else works with older versions. - Prunner Start up prunner via the following command: @@ -317,6 +321,163 @@ by the newer rendering task. in the wait-list waiting to be rendered.** Additionally, we can be sure that scheduled content releases will be eventually executed, because that's prunner's job. +## Quick Content Releases + +Rendering dominates the runtime of a content release: on a big site, a release which changes a single page still +re-renders every other page to produce a release which is identical to the previous one everywhere else. + +A *quick content release* is the shortcut for that case. It copies the content release which is currently live, +re-renders only the documents you name into that copy, and publishes the result. From the enumeration onwards it is +the ordinary pipeline, so validation, transfer and switching behave exactly as they always do — the release which +goes live is a complete, ordinary content release, not a patch. + +It is deliberately manual and explicit. Nothing starts a quick release automatically, and the backend offers it only +while automatic releases are paused, because that is the situation it exists for: something has to go live now, and +waiting for a full release is not an option. + +The reasoning behind the design — the measurement it is based on, why the copy is configured per key, and which +limitations follow from copying a release forward — is written up in +[Documentation/Concepts/QuickContentReleases.md](Documentation/Concepts/QuickContentReleases.md). + +### Requirements + +- **Redis 6.2 or newer** on the primary content store. The copy is the server-side `COPY` command, so nothing travels + over the wire. The command checks the server version before it copies anything and aborts with a message naming the + required version, rather than failing cryptically on an older server. +- **The `do_quick_content_release` pipeline** from `pipelines_template.yml` in your own `pipelines.yml`. +- **`copyOnQuickRelease: true` on every custom release key** you write (see below). This is the part which is easy to + miss, and getting it wrong is not subtle: a key which is `isRequired` and is neither copied nor written by the + quick pipeline makes the switch abort. + +### Registering your own keys for the copy + +Which keys a quick release carries over is configuration, not a hardcoded list. Extend the registration described +under [Writing Custom Data to the Content Release](#writing-custom-data-to-the-content-release): + +```yaml +Flowpack: + DecoupledContentStore: + redisKeyPostfixesForEachRelease: + foo: + transfer: true + # true for content which a quick release does not rebuild, and which therefore has to come along from the + # release being copied. Defaults to false. + copyOnQuickRelease: true +``` + +The default is `false`, which is the safe direction: a key which should have been copied shows up as missing content, +while a key which should not have been copied describes a *different* release and is much harder to notice. The +package's own content keys (`data`, `meta:urls`, `renderedDocuments`, `renderedMetadata`) are `true`; its enumeration, +job queue and statistics keys are `false`, because the quick pipeline writes those itself. + +Set it to `true` for anything your pipeline exports in a task the quick pipeline does not run — brand data, redirect +exports, reusable snippets and the like. Those then go live exactly as they were in the copied release. + +### Pausing the automatic releases + +The Content Store module has a *Pause automatic releases* button. While the pause is on: + +- every automatic trigger (workspace publish, asset change, re-render after a rendering error) is suppressed and + counted, so the module can show how much is waiting; +- *Publish All* still works — it is an explicit request, and the pause exists to let you prepare a release by hand; +- editors see a warning in the Neos content module telling them their changes are not going live yet, because a pause + stops everybody's publishes, not just yours. + +Resuming only lifts the switch. It does not start a release, so the suppressed changes go live with the next release +that is triggered — start one yourself if you do not want to wait for the next editor publish. + +Pause, resume and quick publish sit behind the `Flowpack.DecoupledContentStore:ReleaseControl` privilege target, +which the package grants to `Neos.Neos:Administrator`. It is separate from the module privilege, so an installation +which lets editors watch the module can still restrict who may stop everybody's publishes from going live. The +read-only status the content-module warning uses is outside the target, so editors can see the banner. + +### Publishing single documents + +With automatic releases paused, the module offers *Quick publish pages*. Paste the node identifiers of the documents +to publish, one per line. The confirmation page then shows one row per dimension variant with its title, path, +dimensions, node type and a link into the Neos backend, and flags every row which will **not** be published — a page +which is hidden, orphaned, of a node type outside `nodeRendering.nodeTypeWhitelist`, or an identifier which resolves +nowhere at all. Check those before you continue: a page you meant to fix would otherwise silently stay as it is. + +The identifiers are checked against the identifier format before they are used anywhere. They end up inside a shell +command in the pipeline, so anything else is refused outright. + +Two situations are refused with an explanation instead of a release: + +- **No release is live.** There is nothing to copy, so run a full release instead. +- **Another quick release is still running or queued.** Its copy source is resolved when it is scheduled, so a second + one queued behind the first would build on the release the first is about to replace — and drop that change without + a word. + +### What a quick release cannot do + +These follow from copying the previous release forward. They are not gaps to be closed later. + +- **Only the named pages change.** Anything on *other* pages which embeds them — navigation titles, teasers, sitemap + entries, search indexes, and whatever your own export tasks produce — stays as it was until the next normal release. +- **Adding or removing pages is out of scope.** A deleted page still sits in the copied `meta:urls` and + `renderedDocuments`. Quick publish is for fixing pages which already exist. +- **The release being copied has to still exist**, so it must not have been pruned by + `contentReleaseRetentionCount`. +- **Pausing means editor changes stop going live.** The banner and the counter are the whole mitigation, which is why + resuming is a deliberate manual step. +- **Concurrency.** The quick pipeline takes the same concurrent build lock as any other release, so it terminates an + in-flight full release, and a full release started afterwards terminates it. Correct, but pressing *Publish All* + during a quick release throws the quick release away. + +### Scoping your own validators + +Validation is what is left of the runtime once rendering is gone, and after a copy-forward almost all of it is wasted: +every document except the handful just re-rendered is byte-for-byte what the previous release was already validated +on. + +`Flowpack\DecoupledContentStore\QuickPublish\ContentReleaseScope` is the hook for that. A validator which knows +nothing about quick releases keeps working unchanged; one which opts in asks for the scope and narrows its read: + +```php +$changedUrls = $this->contentReleaseScope->getChangedUrls($contentReleaseIdentifier); +if ($changedUrls === null) { + // an ordinary release: validate everything, as before +} else { + // a quick release: only these URLs were rendered, everything else was validated in the release we copied +} +``` + +`NULL` means "this release was rendered as a whole" — a validator which reads it as "no URLs to check" would wave +everything through, so treat the two cases explicitly. The typical win is turning an `hGetAll` over the whole +document hash into an `hMGet` for the changed URLs. + +The package's own `contentReleaseValidation:validate` already does this, and it had to: it compares the enumeration +of the new release against the live one and aborts below 70%, while a quick release deliberately enumerates a handful +of documents instead of all of them. For a quick release it compares the number of published URLs instead, which +after a copy-forward equals the previous release. + +### The commands + +Both are pipeline steps and are not meant to be called by hand, but they are useful to know when reading a failed +job log: + +```bash +# copy every key registered with copyOnQuickRelease from one release to another, within one content store +./flow contentReleaseQuickPublish:copyReleaseWithin primary + +# write the enumeration of a quick release: these documents, and nothing else +./flow contentReleaseQuickPublish:enumerateGivenNodes --nodeIdentifiers +``` + +The copy refuses a source release whose status is not `success` or which is missing a required key, because +switching a release live by hand is possible and "currently live" alone does not guarantee a clean release. The +enumeration refuses an identifier which resolves nowhere, and refuses to end up empty — a quick release which renders +nothing would publish the release it copied and look like a successful publish while the change is nowhere. + +To start one from your own code: + +```php +$this->contentReleaseManager->startQuickContentRelease( + NodeIdentifiers::fromCommaSeparatedString(',') +); +``` + ## Extensibility ### Custom `pipelines.yml` @@ -390,6 +551,9 @@ Flowpack: This is needed so that the system knows which keys should be synchronized between the different content stores, and what data to delete if a release is removed. +If you use [Quick Content Releases](#quick-content-releases), decide here whether the key travels into one — see +[Registering your own keys for the copy](#registering-your-own-keys-for-the-copy). + ### Rendering additional nodes with arguments (e.g. pagination or filters) If you render a paginated list or have filters (with a predictable list of values) that can be From 8801509d950bca73fc14f34fc311ba9c5b7e02ac Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 13:00:15 +0200 Subject: [PATCH 09/23] format code according to psr12 --- .../AutomaticReleaseStatusDataSource.php | 6 +- Classes/ContentReleaseManager.php | 14 +++- Classes/Controller/BackendController.php | 15 ++-- .../Core/AutomaticReleaseSwitchService.php | 6 +- .../AutomaticReleasePauseState.php | 4 +- Classes/NodeEnumeration/NodeEnumerator.php | 7 +- Classes/QuickPublish/ContentReleaseScope.php | 15 ++-- Classes/QuickPublish/Dto/NodeIdentifiers.php | 5 +- .../RedisReleaseCopyService.php | 69 +++++++++---------- .../QuickPublishNodeEnumerator.php | 46 ++++++------- .../QuickPublishPreviewService.php | 10 ++- .../Features/Bootstrap/FeatureContext.php | 9 ++- .../AutomaticReleaseStatusDataSourceTest.php | 40 ++++++----- Tests/Unit/ContentReleaseManagerTest.php | 30 +++++--- .../AutomaticReleaseSwitchServiceTest.php | 28 ++++---- .../AutomaticReleasePauseStateTest.php | 6 +- .../QuickPublish/ContentReleaseScopeTest.php | 32 +++++---- .../QuickPublish/Dto/NodeIdentifiersTest.php | 10 ++- .../RedisReleaseCopyServiceTest.php | 41 ++++++----- .../RedisKeyPostfixesForEachReleaseTest.php | 6 +- 20 files changed, 212 insertions(+), 187 deletions(-) diff --git a/Classes/BackendUi/AutomaticReleaseStatusDataSource.php b/Classes/BackendUi/AutomaticReleaseStatusDataSource.php index 714b0f2..3e3150a 100644 --- a/Classes/BackendUi/AutomaticReleaseStatusDataSource.php +++ b/Classes/BackendUi/AutomaticReleaseStatusDataSource.php @@ -53,17 +53,17 @@ public function getData(?NodeInterface $node = null, array $arguments = []): arr return [ 'paused' => true, - 'message' => (string)$this->translator->translateById( + 'message' => (string) $this->translator->translateById( 'automaticReleases.paused.contentModuleWarning', [ $this->backendDateFormatter->format($pauseState->getPausedAt()), - $pauseState->getSuppressedReleaseCount(), + $pauseState->getSuppressedReleaseCount() ], null, null, 'Main', 'Flowpack.DecoupledContentStore' - ), + ) ]; } } diff --git a/Classes/ContentReleaseManager.php b/Classes/ContentReleaseManager.php index 7988be1..765e402 100644 --- a/Classes/ContentReleaseManager.php +++ b/Classes/ContentReleaseManager.php @@ -76,7 +76,13 @@ public function startIncrementalContentRelease( if ($this->automaticReleaseSwitchService->isPaused()) { $this->automaticReleaseSwitchService->countSuppressedRelease(); - $this->logger->info(sprintf('Automatic content releases are paused, so content release %s was not scheduled.', $contentReleaseId->getIdentifier()), LogEnvironment::fromMethodName(__METHOD__)); + $this->logger->info( + sprintf( + 'Automatic content releases are paused, so content release %s was not scheduled.', + $contentReleaseId->getIdentifier() + ), + LogEnvironment::fromMethodName(__METHOD__) + ); return $contentReleaseId; } @@ -150,7 +156,9 @@ public function startQuickContentRelease( // a quick release copies the release which is live when it is scheduled, so a second one queued behind the // first would build on the release the first is about to replace - and drop that first change without a word - $quickReleaseJobs = $this->prunnerApiService->loadPipelinesAndJobs()->getJobs() + $quickReleaseJobs = $this->prunnerApiService + ->loadPipelinesAndJobs() + ->getJobs() ->forPipeline(PipelineName::create(self::QUICK_CONTENT_RELEASE_PIPELINE_NAME)); if ($quickReleaseJobs->running()->getArray() !== [] || $quickReleaseJobs->waiting()->getArray() !== []) { throw new QuickContentReleaseNotPossibleException( @@ -165,7 +173,7 @@ public function startQuickContentRelease( array_merge($additionalVariables, [ 'contentReleaseId' => $contentReleaseId, 'currentContentReleaseId' => $currentContentReleaseId, - 'quickPublishNodeIdentifiers' => (string)$nodeIdentifiers, + 'quickPublishNodeIdentifiers' => (string) $nodeIdentifiers, 'workspaceName' => $workspace !== null ? $workspace->getName() : 'live', 'accountId' => $this->getAccountId() ]) diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index 9eb7155..5cce8c9 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -353,7 +353,7 @@ public function quickPublishPreviewAction(string $nodeIdentifiers, ?string $cont $previewRows = $this->quickPublishPreviewService->preview($identifiers, $this->controllerContext); $this->view->assign('contentStore', $contentStore); - $this->view->assign('nodeIdentifiers', (string)$identifiers); + $this->view->assign('nodeIdentifiers', (string) $identifiers); $this->view->assign('previewRows', $previewRows); $this->view->assign('publishedRowCount', $this->quickPublishPreviewService->countPublishedRows($previewRows)); @@ -368,9 +368,9 @@ public function quickPublishAction(string $nodeIdentifiers, ?string $contentStor } try { - $contentReleaseIdentifier = $this->contentReleaseManager->startQuickContentRelease( - NodeIdentifiers::fromUserInput($nodeIdentifiers) - ); + $contentReleaseIdentifier = $this->contentReleaseManager->startQuickContentRelease(NodeIdentifiers::fromUserInput( + $nodeIdentifiers + )); } catch (Exception $exception) { // both the identifier check and the manager phrase their messages for the person reading this page $this->addFlashMessage($exception->getMessage(), '', Message::SEVERITY_ERROR); @@ -382,10 +382,7 @@ public function quickPublishAction(string $nodeIdentifiers, ?string $contentStor return null; } - $this->addFlashMessage($this->translateById( - 'quickPublish.scheduled.flashMessage', - [$contentReleaseIdentifier->getIdentifier()] - )); + $this->addFlashMessage($this->translateById('quickPublish.scheduled.flashMessage', [$contentReleaseIdentifier->getIdentifier()])); $this->redirect('index', null, null, $contentStore !== null ? ['contentStore' => $contentStore] : []); return null; @@ -396,7 +393,7 @@ public function quickPublishAction(string $nodeIdentifiers, ?string $contentStor */ private function translateById(string $labelId, array $arguments = []): string { - return (string)$this->translator->translateById( + return (string) $this->translator->translateById( $labelId, $arguments, null, diff --git a/Classes/Core/AutomaticReleaseSwitchService.php b/Classes/Core/AutomaticReleaseSwitchService.php index f82fd5e..87cb89b 100644 --- a/Classes/Core/AutomaticReleaseSwitchService.php +++ b/Classes/Core/AutomaticReleaseSwitchService.php @@ -34,7 +34,7 @@ public function isPaused(): bool { // the "pausedAt" field, not the key itself: countSuppressedRelease() can re-create the key with only its // counter field if a resume happens in between. - return (bool)$this->redisClientManager->getPrimaryRedis()->hExists(self::REDIS_KEY, 'pausedAt'); + return (bool) $this->redisClientManager->getPrimaryRedis()->hExists(self::REDIS_KEY, 'pausedAt'); } public function getPauseState(): ?AutomaticReleasePauseState @@ -54,9 +54,9 @@ public function pause(): void } $this->redisClientManager->getPrimaryRedis()->hMset(self::REDIS_KEY, [ - 'pausedAt' => (new DateTimeImmutable())->format(DateTimeInterface::ATOM), + 'pausedAt' => new DateTimeImmutable()->format(DateTimeInterface::ATOM), 'accountId' => $this->getAccountId() ?? '', - 'suppressedReleaseCount' => 0, + 'suppressedReleaseCount' => 0 ]); } diff --git a/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php b/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php index 4b9160f..47a466c 100644 --- a/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php +++ b/Classes/Core/Domain/ValueObject/AutomaticReleasePauseState.php @@ -47,8 +47,8 @@ public static function fromRedisHash(array $redisHash): self return new self( new DateTimeImmutable($redisHash['pausedAt']), - ($redisHash['accountId'] ?? '') !== '' ? $redisHash['accountId'] : null, - (int)($redisHash['suppressedReleaseCount'] ?? 0) + ( $redisHash['accountId'] ?? '' ) !== '' ? $redisHash['accountId'] : null, + (int) ( $redisHash['suppressedReleaseCount'] ?? 0 ) ); } diff --git a/Classes/NodeEnumeration/NodeEnumerator.php b/Classes/NodeEnumeration/NodeEnumerator.php index 6a72c52..6bc4283 100644 --- a/Classes/NodeEnumeration/NodeEnumerator.php +++ b/Classes/NodeEnumeration/NodeEnumerator.php @@ -122,10 +122,9 @@ private function enumerateAll( $skipReason = $this->documentNodeFilter->skipReason($nodeToEnumerate, $siteNode); if ($skipReason !== null) { - $contentReleaseLogger->debug( - 'Skipping node from publishing, because it is ' . $skipReason, - ['node' => $contextPath] - ); + $contentReleaseLogger->debug('Skipping node from publishing, because it is ' . $skipReason, [ + 'node' => $contextPath + ]); continue; } diff --git a/Classes/QuickPublish/ContentReleaseScope.php b/Classes/QuickPublish/ContentReleaseScope.php index dc79883..fc8229d 100644 --- a/Classes/QuickPublish/ContentReleaseScope.php +++ b/Classes/QuickPublish/ContentReleaseScope.php @@ -37,9 +37,12 @@ final class ContentReleaseScope */ public function getChangedUrls(ContentReleaseIdentifier $contentReleaseIdentifier): ?array { - $changedUrls = $this->redisClientManager->getPrimaryRedis()->sMembers( - $this->redisKeyService->getRedisKeyForPostfix($contentReleaseIdentifier, self::CHANGED_URLS_POSTFIX) - ); + $changedUrls = $this->redisClientManager + ->getPrimaryRedis() + ->sMembers($this->redisKeyService->getRedisKeyForPostfix( + $contentReleaseIdentifier, + self::CHANGED_URLS_POSTFIX + )); // a quick release which changed nothing is never published, so an empty set means there is no scope if (!is_array($changedUrls) || $changedUrls === []) { @@ -72,8 +75,8 @@ public function setChangedUrls(ContentReleaseIdentifier $contentReleaseIdentifie */ public function countPublishedUrls(ContentReleaseIdentifier $contentReleaseIdentifier): int { - return (int) $this->redisClientManager->getPrimaryRedis()->zCard( - $this->redisKeyService->getRedisKeyForPostfix($contentReleaseIdentifier, self::URLS_POSTFIX) - ); + return (int) $this->redisClientManager + ->getPrimaryRedis() + ->zCard($this->redisKeyService->getRedisKeyForPostfix($contentReleaseIdentifier, self::URLS_POSTFIX)); } } diff --git a/Classes/QuickPublish/Dto/NodeIdentifiers.php b/Classes/QuickPublish/Dto/NodeIdentifiers.php index 7020f92..a5bcd74 100644 --- a/Classes/QuickPublish/Dto/NodeIdentifiers.php +++ b/Classes/QuickPublish/Dto/NodeIdentifiers.php @@ -70,10 +70,7 @@ private static function fromTokens(array $tokens): self continue; } if (preg_match(self::IDENTIFIER_PATTERN, $identifier) !== 1) { - throw new Exception( - sprintf('"%s" is not a node identifier.', $identifier), - 1786958510 - ); + throw new Exception(sprintf('"%s" is not a node identifier.', $identifier), 1786958510); } // an identifier given twice would be rendered twice if (!in_array($identifier, $identifiers, true)) { diff --git a/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php b/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php index d13aa8c..ac5a887 100644 --- a/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php +++ b/Classes/QuickPublish/Infrastructure/RedisReleaseCopyService.php @@ -60,7 +60,8 @@ public function copyReleaseWithin( sprintf( 'Cannot copy content release %s onto itself.', $sourceContentReleaseIdentifier->getIdentifier() - ), 1786953585 + ), + 1786953585 ); } @@ -68,18 +69,14 @@ public function copyReleaseWithin( $this->assertServerSupportsCopy($redis); $this->assertSourceReleaseCanBeBuiltUpon($redis, $redisInstanceIdentifier, $sourceContentReleaseIdentifier); - $contentReleaseLogger->info( - sprintf( - 'Copying content release %s to %s within redis %s', - $sourceContentReleaseIdentifier->getIdentifier(), - $targetContentReleaseIdentifier->getIdentifier(), - $redisInstanceIdentifier->getIdentifier() - ) - ); + $contentReleaseLogger->info(sprintf( + 'Copying content release %s to %s within redis %s', + $sourceContentReleaseIdentifier->getIdentifier(), + $targetContentReleaseIdentifier->getIdentifier(), + $redisInstanceIdentifier->getIdentifier() + )); - $redisKeyPostfixesForEachRelease = RedisKeyPostfixesForEachRelease::fromArray( - $this->redisKeyPostfixesForEachReleaseConfiguration - ); + $redisKeyPostfixesForEachRelease = RedisKeyPostfixesForEachRelease::fromArray($this->redisKeyPostfixesForEachReleaseConfiguration); $startTime = microtime(true); $copiedKeyCount = 0; @@ -100,7 +97,9 @@ public function copyReleaseWithin( if ($redis->exists($targetKey)) { $contentReleaseLogger->warn( - 'COPY: ' . $targetKey . ' already exists and is replaced - ' + 'COPY: ' + . $targetKey + . ' already exists and is replaced - ' . 'the release was copied into after something already wrote to it.' ); } @@ -114,23 +113,19 @@ public function copyReleaseWithin( } $copiedKeyCount++; - $contentReleaseLogger->info( - sprintf( - 'COPY: Copied key %s (time: %2.3f)', - $targetKey, - microtime(true) - $keyStartTime - ) - ); + $contentReleaseLogger->info(sprintf( + 'COPY: Copied key %s (time: %2.3f)', + $targetKey, + microtime(true) - $keyStartTime + )); } - $contentReleaseLogger->info( - sprintf( - 'COPY: Copied %d keys from content release %s (total time: %2.3f)', - $copiedKeyCount, - $sourceContentReleaseIdentifier->getIdentifier(), - microtime(true) - $startTime - ) - ); + $contentReleaseLogger->info(sprintf( + 'COPY: Copied %d keys from content release %s (total time: %2.3f)', + $copiedKeyCount, + $sourceContentReleaseIdentifier->getIdentifier(), + microtime(true) - $startTime + )); } /** @@ -140,7 +135,7 @@ private function assertServerSupportsCopy(Redis $redis): void { $serverInfo = $redis->info('server'); $redisVersion = is_array($serverInfo) && array_key_exists('redis_version', $serverInfo) - ? (string)$serverInfo['redis_version'] + ? (string) $serverInfo['redis_version'] : ''; if ($redisVersion === '' || version_compare($redisVersion, self::MINIMUM_REDIS_VERSION, '<')) { @@ -150,7 +145,8 @@ private function assertServerSupportsCopy(Redis $redis): void . 'This server reports version "%s".', self::MINIMUM_REDIS_VERSION, $redisVersion - ), 1786953587 + ), + 1786953587 ); } } @@ -179,7 +175,8 @@ private function assertSourceReleaseCanBeBuiltUpon( . 'instead.', $sourceContentReleaseIdentifier->getIdentifier(), $redisInstanceIdentifier->getIdentifier() - ), 1786953588 + ), + 1786953588 ); } @@ -190,13 +187,12 @@ private function assertSourceReleaseCanBeBuiltUpon( . 'content release instead.', $sourceContentReleaseIdentifier->getIdentifier(), $metadata->getStatus()->getStatus() - ), 1786953589 + ), + 1786953589 ); } - $redisKeyPostfixesForEachRelease = RedisKeyPostfixesForEachRelease::fromArray( - $this->redisKeyPostfixesForEachReleaseConfiguration - ); + $redisKeyPostfixesForEachRelease = RedisKeyPostfixesForEachRelease::fromArray($this->redisKeyPostfixesForEachReleaseConfiguration); // only the inherited keys have to exist - the rest is built by the quick release itself. isRequired alone is // not enough of a filter: the other places reading it check it per transfer target, so a key an installation @@ -217,7 +213,8 @@ private function assertSourceReleaseCanBeBuiltUpon( . 'content release instead.', $requiredKey, $sourceContentReleaseIdentifier->getIdentifier() - ), 1786953590 + ), + 1786953590 ); } } diff --git a/Classes/QuickPublish/QuickPublishNodeEnumerator.php b/Classes/QuickPublish/QuickPublishNodeEnumerator.php index 587d3df..be4dd0a 100644 --- a/Classes/QuickPublish/QuickPublishNodeEnumerator.php +++ b/Classes/QuickPublish/QuickPublishNodeEnumerator.php @@ -82,7 +82,8 @@ public function enumerateGivenNodesAndStoreInRedis( sprintf( 'Content release %s does not exist, so its nodes cannot be enumerated.', $releaseIdentifier->getIdentifier() - ), 1786958512 + ), + 1786958512 ); } @@ -108,18 +109,17 @@ public function enumerateGivenNodesAndStoreInRedis( sprintf( 'None of the given nodes can be published (%s), so content release %s would only repeat the release ' . 'it was built on.', - (string)$nodeIdentifiers, + (string) $nodeIdentifiers, $releaseIdentifier->getIdentifier() - ), 1786958513 + ), + 1786958513 ); } - foreach ( - GeneratorUtility::createArrayBatch( - array_map(static fn(array $nodeToRender): EnumeratedNode => $nodeToRender[1], $nodesToRender), - 100 - ) as $enumeration - ) { + foreach (GeneratorUtility::createArrayBatch( + array_map(static fn(array $nodeToRender): EnumeratedNode => $nodeToRender[1], $nodesToRender), + 100 + ) as $enumeration) { $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($releaseIdentifier); $this->redisEnumerationRepository->addDocumentNodesToEnumeration($releaseIdentifier, ...$enumeration); } @@ -183,18 +183,17 @@ private function enumerateGivenNodes( $skipReason = $this->documentNodeFilter->skipReasonForNamedNode($nodeToEnumerate, $siteNode); if ($skipReason !== null) { // warn rather than debug: somebody asked for this node by hand and will not see it change - $contentReleaseLogger->warn( - 'Skipping node from publishing, because it is ' . $skipReason, - ['node' => $contextPath] - ); + $contentReleaseLogger->warn('Skipping node from publishing, because it is ' . $skipReason, [ + 'node' => $contextPath + ]); continue; } $contentReleaseLogger->info('Registering node for publishing', ['node' => $contextPath]); - foreach ( - $this->nodeRenderingExtensionManager->enumerateDocumentNode($nodeToEnumerate) as $enumeratedNode - ) { + foreach ($this->nodeRenderingExtensionManager->enumerateDocumentNode( + $nodeToEnumerate + ) as $enumeratedNode) { $nodesToRender[] = [$nodeToEnumerate, $enumeratedNode]; } } @@ -204,7 +203,8 @@ private function enumerateGivenNodes( sprintf( 'Could not find node %s in any site and dimension, so it cannot be published.', $nodeIdentifier - ), 1786958514 + ), + 1786958514 ); } } @@ -227,12 +227,10 @@ private function flushContentCacheForNode( $flushedEntriesCount += $this->contentCache->flushByTag($tag); } - $contentReleaseLogger->info( - sprintf( - 'Flushed %d content cache entries for node %s before re-rendering it', - $flushedEntriesCount, - $nodeIdentifier - ) - ); + $contentReleaseLogger->info(sprintf( + 'Flushed %d content cache entries for node %s before re-rendering it', + $flushedEntriesCount, + $nodeIdentifier + )); } } diff --git a/Classes/QuickPublish/QuickPublishPreviewService.php b/Classes/QuickPublish/QuickPublishPreviewService.php index cfe15f2..b387a0c 100644 --- a/Classes/QuickPublish/QuickPublishPreviewService.php +++ b/Classes/QuickPublish/QuickPublishPreviewService.php @@ -69,12 +69,10 @@ public function countPublishedRows(array $rows): int private function backendUri(NodeInterface $node, ControllerContext $controllerContext): string { - return $controllerContext->getUriBuilder()->reset()->uriFor( - 'index', - ['node' => $node->getContextPath()], - 'Backend', - 'Neos.Neos.Ui' - ); + return $controllerContext + ->getUriBuilder() + ->reset() + ->uriFor('index', ['node' => $node->getContextPath()], 'Backend', 'Neos.Neos.Ui'); } private static function describeDimensions(NodeInterface $node): string diff --git a/Tests/Behavior/Features/Bootstrap/FeatureContext.php b/Tests/Behavior/Features/Bootstrap/FeatureContext.php index f8193ca..4c720e8 100644 --- a/Tests/Behavior/Features/Bootstrap/FeatureContext.php +++ b/Tests/Behavior/Features/Bootstrap/FeatureContext.php @@ -204,9 +204,12 @@ public function validatingContentReleaseSucceeds($contentReleaseIdentifier) $validationCommandController->validateCommand($contentReleaseIdentifier); $redisRenderingErrorManager = $this->getObjectManager()->get(RedisRenderingErrorManager::class); - Assert::assertCount(0, $redisRenderingErrorManager->getRenderingErrors( - ContentReleaseIdentifier::fromString($contentReleaseIdentifier) - )); + Assert::assertCount( + 0, + $redisRenderingErrorManager->getRenderingErrors(ContentReleaseIdentifier::fromString( + $contentReleaseIdentifier + )) + ); } /** diff --git a/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php b/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php index 199456b..ea8a7df 100644 --- a/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php +++ b/Tests/Unit/BackendUi/AutomaticReleaseStatusDataSourceTest.php @@ -38,31 +38,37 @@ public function testTheWarningIsPublishedAsAReadyMadeMessage(): void $pauseState = AutomaticReleasePauseState::fromRedisHash([ 'pausedAt' => '2026-08-13T09:15:00+02:00', 'accountId' => 'admin', - 'suppressedReleaseCount' => '4', + 'suppressedReleaseCount' => '4' ]); - self::assertSame([ - 'paused' => true, - 'message' => 'translated: automaticReleases.paused.contentModuleWarning', - ], $this->buildDataSource($pauseState)->getData()); + self::assertSame( + [ + 'paused' => true, + 'message' => 'translated: automaticReleases.paused.contentModuleWarning' + ], + $this->buildDataSource($pauseState)->getData() + ); } public function testTheTimestampAndTheWaitingCountAreHandedToTheTranslation(): void { $pauseState = AutomaticReleasePauseState::fromRedisHash([ 'pausedAt' => '2026-08-13T09:15:00+02:00', - 'suppressedReleaseCount' => '4', + 'suppressedReleaseCount' => '4' ]); $translator = $this->createMock(Translator::class); - $translator->expects(self::once())->method('translateById')->with( - 'automaticReleases.paused.contentModuleWarning', - ['formatted date', 4], - null, - null, - 'Main', - 'Flowpack.DecoupledContentStore' - ); + $translator + ->expects(self::once()) + ->method('translateById') + ->with( + 'automaticReleases.paused.contentModuleWarning', + ['formatted date', 4], + null, + null, + 'Main', + 'Flowpack.DecoupledContentStore' + ); $this->buildDataSource($pauseState, $translator)->getData(); } @@ -76,9 +82,9 @@ private function buildDataSource( if ($translator === null) { $translator = $this->createMock(Translator::class); - $translator->method('translateById')->willReturnCallback( - static fn(string $labelId): string => 'translated: ' . $labelId - ); + $translator + ->method('translateById') + ->willReturnCallback(static fn(string $labelId): string => 'translated: ' . $labelId); } $backendDateFormatter = $this->createMock(BackendDateFormatter::class); diff --git a/Tests/Unit/ContentReleaseManagerTest.php b/Tests/Unit/ContentReleaseManagerTest.php index bea43f2..7018842 100644 --- a/Tests/Unit/ContentReleaseManagerTest.php +++ b/Tests/Unit/ContentReleaseManagerTest.php @@ -85,12 +85,18 @@ public function testAQuickReleaseIsScheduledEvenWhilePaused(): void // pausing the automatic release is what quick releases exist for, so the pause must not block them $this->currentContentReleaseId = '5'; $this->automaticReleaseSwitchService->method('isPaused')->willReturn(true); - $this->prunnerApiService->expects(self::once())->method('schedulePipeline')->with( - self::equalTo(PipelineName::create('do_quick_content_release')), - self::callback(static fn(array $variables): bool => - $variables['currentContentReleaseId'] === '5' - && $variables['quickPublishNodeIdentifiers'] === self::NODE_IDENTIFIER) - ); + $this->prunnerApiService + ->expects(self::once()) + ->method('schedulePipeline') + ->with( + self::equalTo(PipelineName::create('do_quick_content_release')), + self::callback( + static fn(array $variables): bool => ( + $variables['currentContentReleaseId'] === '5' + && $variables['quickPublishNodeIdentifiers'] === self::NODE_IDENTIFIER + ) + ) + ); $this->buildContentReleaseManager()->startQuickContentRelease($this->nodeIdentifiers()); } @@ -112,7 +118,8 @@ public function testAQuickReleaseIsRefusedWhileAnotherOneIsOnItsWay(bool $starte { // the second one copies the release the first is about to replace, so it would undo the first change $this->currentContentReleaseId = '5'; - $this->prunnerApiService->method('loadPipelinesAndJobs') + $this->prunnerApiService + ->method('loadPipelinesAndJobs') ->willReturn($this->jobsResponse('do_quick_content_release', $started)); $this->prunnerApiService->expects(self::never())->method('schedulePipeline'); @@ -131,7 +138,8 @@ public static function quickReleasesOnTheirWay(): array public function testARunningQuickReleaseIsCancelledAlongWithTheOtherContentReleases(): void { // it ends up being switched live just like a full release does, so "cancel" has to reach it - $this->prunnerApiService->method('loadPipelinesAndJobs') + $this->prunnerApiService + ->method('loadPipelinesAndJobs') ->willReturn($this->jobsResponse('do_quick_content_release', true)); $this->prunnerApiService->expects(self::once())->method('cancelJob'); @@ -157,9 +165,9 @@ private function jobsResponse(string $pipeline, bool $started): PipelinesAndJobs 'errored' => false, 'created' => '2026-08-17T10:00:00+02:00', 'start' => $started ? '2026-08-17T10:00:01+02:00' : null, - 'user' => 'test', - ], - ], + 'user' => 'test' + ] + ] ]); } diff --git a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php index 630b8ca..a541c07 100644 --- a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php +++ b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php @@ -44,14 +44,16 @@ public function testPausingRecordsTheTimestampAndAnEmptyCounter(): void { $redis = $this->buildRedis(); $redis->method('hExists')->willReturn(false); - $redis->expects(self::once())->method('hMSet')->with( - self::REDIS_KEY, - self::callback(static function (array $hash): bool { - return $hash['accountId'] === '' + $redis + ->expects(self::once()) + ->method('hMSet') + ->with(self::REDIS_KEY, self::callback(static function (array $hash): bool { + return ( + $hash['accountId'] === '' && $hash['suppressedReleaseCount'] === 0 - && \DateTimeImmutable::createFromFormat(\DateTimeInterface::ATOM, $hash['pausedAt']) !== false; - }) - ); + && \DateTimeImmutable::createFromFormat(\DateTimeInterface::ATOM, $hash['pausedAt']) !== false + ); + })); $this->buildService($redis)->pause(); } @@ -67,11 +69,13 @@ public function testThereIsNoPauseStateWhileTheSwitchIsNotSet(): void public function testThePauseStateIsReadFromTheHash(): void { $redis = $this->buildRedis(); - $redis->method('hGetAll')->willReturn([ - 'pausedAt' => '2026-08-13T09:15:00+02:00', - 'accountId' => 'admin', - 'suppressedReleaseCount' => '4', - ]); + $redis + ->method('hGetAll') + ->willReturn([ + 'pausedAt' => '2026-08-13T09:15:00+02:00', + 'accountId' => 'admin', + 'suppressedReleaseCount' => '4' + ]); $pauseState = $this->buildService($redis)->getPauseState(); diff --git a/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php b/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php index 71c3b36..a708512 100644 --- a/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php +++ b/Tests/Unit/Core/Domain/ValueObject/AutomaticReleasePauseStateTest.php @@ -18,7 +18,7 @@ public function testAllFieldsAreReadFromTheHash(): void $pauseState = AutomaticReleasePauseState::fromRedisHash([ 'pausedAt' => '2026-08-13T09:15:00+02:00', 'accountId' => 'admin', - 'suppressedReleaseCount' => '7', + 'suppressedReleaseCount' => '7' ]); self::assertSame('2026-08-13T09:15:00+02:00', $pauseState->getPausedAt()->format(\DateTimeInterface::ATOM)); @@ -32,7 +32,7 @@ public function testAnEmptyAccountIdBecomesNull(): void $pauseState = AutomaticReleasePauseState::fromRedisHash([ 'pausedAt' => '2026-08-13T09:15:00+02:00', 'accountId' => '', - 'suppressedReleaseCount' => '0', + 'suppressedReleaseCount' => '0' ]); self::assertNull($pauseState->getAccountId()); @@ -41,7 +41,7 @@ public function testAnEmptyAccountIdBecomesNull(): void public function testTheCounterDefaultsToZero(): void { $pauseState = AutomaticReleasePauseState::fromRedisHash([ - 'pausedAt' => '2026-08-13T09:15:00+02:00', + 'pausedAt' => '2026-08-13T09:15:00+02:00' ]); self::assertSame(0, $pauseState->getSuppressedReleaseCount()); diff --git a/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php b/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php index 91698f8..1775589 100644 --- a/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php +++ b/Tests/Unit/QuickPublish/ContentReleaseScopeTest.php @@ -32,10 +32,13 @@ public function testAReleaseWhichWasRenderedAsAWholeHasNoScope(): void public function testAQuickReleaseIsScopedToTheUrlsItRendered(): void { $redis = $this->createMock(\Redis::class); - $redis->method('sMembers')->with(self::CHANGED_URLS_KEY)->willReturn([ - 'http://test.de/de', - 'http://test.de/de/nested', - ]); + $redis + ->method('sMembers') + ->with(self::CHANGED_URLS_KEY) + ->willReturn([ + 'http://test.de/de', + 'http://test.de/de/nested' + ]); self::assertSame( ['http://test.de/de', 'http://test.de/de/nested'], @@ -46,16 +49,15 @@ public function testAQuickReleaseIsScopedToTheUrlsItRendered(): void public function testTheScopeIsStoredWithTheReleaseItBelongsTo(): void { $redis = $this->createMock(\Redis::class); - $redis->expects(self::once())->method('sAdd')->with( - self::CHANGED_URLS_KEY, + $redis + ->expects(self::once()) + ->method('sAdd') + ->with(self::CHANGED_URLS_KEY, 'http://test.de/de', 'http://test.de/de/nested'); + + $this->buildContentReleaseScope($redis)->setChangedUrls($this->contentReleaseIdentifier(), [ 'http://test.de/de', 'http://test.de/de/nested' - ); - - $this->buildContentReleaseScope($redis)->setChangedUrls( - $this->contentReleaseIdentifier(), - ['http://test.de/de', 'http://test.de/de/nested'] - ); + ]); } public function testAnEmptyScopeIsNotStored(): void @@ -98,15 +100,15 @@ private function buildContentReleaseScope(\Redis $redis): ContentReleaseScope 'transfer' => true, 'transferMode' => 'dump', 'isRequired' => true, - 'copyOnQuickRelease' => true, + 'copyOnQuickRelease' => true ], 'quickPublishChangedUrls' => [ 'redisKeyPostfix' => 'quickPublish:changedUrls', 'transfer' => false, 'transferMode' => 'dump', 'isRequired' => false, - 'copyOnQuickRelease' => false, - ], + 'copyOnQuickRelease' => false + ] ]); $contentReleaseScope = new ContentReleaseScope(); diff --git a/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php b/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php index ad2458c..aa327ad 100644 --- a/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php +++ b/Tests/Unit/QuickPublish/Dto/NodeIdentifiersTest.php @@ -21,9 +21,7 @@ final class NodeIdentifiersTest extends UnitTestCase public function testIdentifiersAreReadOnePerEntry(): void { - $nodeIdentifiers = NodeIdentifiers::fromCommaSeparatedString( - self::IDENTIFIER . ',' . self::OTHER_IDENTIFIER - ); + $nodeIdentifiers = NodeIdentifiers::fromCommaSeparatedString(self::IDENTIFIER . ',' . self::OTHER_IDENTIFIER); self::assertSame([self::IDENTIFIER, self::OTHER_IDENTIFIER], $nodeIdentifiers->jsonSerialize()); } @@ -32,7 +30,7 @@ public function testSurroundingWhitespaceAndEmptyEntriesAreIgnored(): void { // the identifiers arrive from a textarea, one per line $nodeIdentifiers = NodeIdentifiers::fromCommaSeparatedString( - " " . self::IDENTIFIER . " ,\n,\t" . self::OTHER_IDENTIFIER . "," + ' ' . self::IDENTIFIER . " ,\n,\t" . self::OTHER_IDENTIFIER . ',' ); self::assertSame([self::IDENTIFIER, self::OTHER_IDENTIFIER], $nodeIdentifiers->jsonSerialize()); @@ -73,7 +71,7 @@ public static function notAnIdentifier(): array 'a node path' => ['/sites/test/products'], 'too short' => ['3239baee-3e7f-785c-0853-f4302ef325'], 'no hyphens' => ['3239baee3e7f785c0853f4302ef32570'], - 'a quoted identifier' => ['"' . self::IDENTIFIER . '"'], + 'a quoted identifier' => ['"' . self::IDENTIFIER . '"'] ]; } @@ -88,7 +86,7 @@ public function testAnEmptyListIsRefused(): void public function testTheBackendFormAcceptsOneIdentifierPerLine(): void { $nodeIdentifiers = NodeIdentifiers::fromUserInput( - " " . self::IDENTIFIER . "\r\n\n" . self::OTHER_IDENTIFIER . " \n" + ' ' . self::IDENTIFIER . "\r\n\n" . self::OTHER_IDENTIFIER . " \n" ); self::assertSame([self::IDENTIFIER, self::OTHER_IDENTIFIER], $nodeIdentifiers->jsonSerialize()); diff --git a/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php b/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php index d374bea..3d11337 100644 --- a/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php +++ b/Tests/Unit/QuickPublish/Infrastructure/RedisReleaseCopyServiceTest.php @@ -30,7 +30,7 @@ final class RedisReleaseCopyServiceTest extends UnitTestCase private const SOURCE_KEYS = [ 'contentStore:5:data', 'contentStore:5:meta:urls', - 'contentStore:5:renderingJobQueue', + 'contentStore:5:renderingJobQueue' ]; /** @@ -43,10 +43,13 @@ public function testOnlyTheFlaggedKeysAreCopied(): void $this->copyRelease($this->buildRedis(), $this->buildRedisContentReleaseService()); // renderingJobQueue exists on the source, but describes the build of that release rather than its content - self::assertSame([ - ['contentStore:5:data', 'contentStore:6:data'], - ['contentStore:5:meta:urls', 'contentStore:6:meta:urls'], - ], $this->copiedKeys); + self::assertSame( + [ + ['contentStore:5:data', 'contentStore:6:data'], + ['contentStore:5:meta:urls', 'contentStore:6:meta:urls'] + ], + $this->copiedKeys + ); } public function testAKeyWhichDoesNotExistOnTheSourceIsSkipped(): void @@ -150,13 +153,15 @@ private function buildRedis(array $existingKeys = self::SOURCE_KEYS, string $red { $redis = $this->createMock(\Redis::class); $redis->method('info')->willReturn(['redis_version' => $redisVersion]); - $redis->method('exists')->willReturnCallback( - static fn(string $key): int => in_array($key, $existingKeys, true) ? 1 : 0 - ); - $redis->method('copy')->willReturnCallback(function (string $sourceKey, string $targetKey): bool { - $this->copiedKeys[] = [$sourceKey, $targetKey]; - return true; - }); + $redis->method('exists')->willReturnCallback(static fn(string $key): int => in_array($key, $existingKeys, true) + ? 1 + : 0); + $redis + ->method('copy') + ->willReturnCallback(function (string $sourceKey, string $targetKey): bool { + $this->copiedKeys[] = [$sourceKey, $targetKey]; + return true; + }); return $redis; } @@ -167,8 +172,10 @@ private function buildRedis(array $existingKeys = self::SOURCE_KEYS, string $red private function buildRedisContentReleaseService( ?NodeRenderingCompletionStatus $status = null ): RedisContentReleaseService { - $metadata = ContentReleaseMetadata::create(PrunnerJobId::fromString('job'), new \DateTimeImmutable()) - ->withStatus($status ?? NodeRenderingCompletionStatus::success()); + $metadata = ContentReleaseMetadata::create( + PrunnerJobId::fromString('job'), + new \DateTimeImmutable() + )->withStatus($status ?? NodeRenderingCompletionStatus::success()); $redisContentReleaseService = $this->createMock(RedisContentReleaseService::class); $redisContentReleaseService->method('fetchMetadataForContentRelease')->willReturn($metadata); @@ -187,21 +194,21 @@ private static function keyConfiguration(): array 'transfer' => true, 'transferMode' => 'hash_incremental', 'isRequired' => true, - 'copyOnQuickRelease' => true, + 'copyOnQuickRelease' => true ], 'metaUrls' => [ 'redisKeyPostfix' => 'meta:urls', 'transfer' => true, 'transferMode' => 'dump', 'isRequired' => true, - 'copyOnQuickRelease' => true, + 'copyOnQuickRelease' => true ], 'renderingJobQueue' => [ 'redisKeyPostfix' => 'renderingJobQueue', 'transfer' => false, 'transferMode' => 'dump', 'isRequired' => false, - 'copyOnQuickRelease' => false, + 'copyOnQuickRelease' => false ], 'enumerationDocumentNodes' => [ 'redisKeyPostfix' => 'enumeration:documentNodes', diff --git a/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php b/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php index fc1bf37..7eb1a58 100644 --- a/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php +++ b/Tests/Unit/Transfer/Dto/RedisKeyPostfixesForEachReleaseTest.php @@ -17,7 +17,7 @@ public function testOnlyTheFlaggedKeysAreCopied(): void $redisKeyPostfixes = RedisKeyPostfixesForEachRelease::fromArray([ 'renderedDocuments' => self::keyConfiguration('renderedDocuments', true), 'renderingJobQueue' => self::keyConfiguration('renderingJobQueue', false), - 'metaUrls' => self::keyConfiguration('meta:urls', true), + 'metaUrls' => self::keyConfiguration('meta:urls', true) ]); self::assertSame(['renderedDocuments', 'meta:urls'], self::copiedPostfixes($redisKeyPostfixes)); @@ -30,7 +30,7 @@ public function testAKeyWhichDoesNotKnowAboutQuickReleasesIsNotCopied(): void unset($configurationWithoutTheFlag['copyOnQuickRelease']); $redisKeyPostfixes = RedisKeyPostfixesForEachRelease::fromArray([ - 'renderedDocuments' => $configurationWithoutTheFlag, + 'renderedDocuments' => $configurationWithoutTheFlag ]); self::assertSame([], self::copiedPostfixes($redisKeyPostfixes)); @@ -58,7 +58,7 @@ private static function keyConfiguration(string $redisKeyPostfix, bool $copyOnQu 'transfer' => true, 'transferMode' => 'dump', 'isRequired' => true, - 'copyOnQuickRelease' => $copyOnQuickRelease, + 'copyOnQuickRelease' => $copyOnQuickRelease ]; } } From 660dc1baec942fb26ab0ab728730331a7f0b797e Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Mon, 17 Aug 2026 14:23:23 +0200 Subject: [PATCH 10/23] fix phpunit, bugs and i18n --- .github/workflows/ci.yml | 2 +- .../Core/AutomaticReleaseSwitchService.php | 2 +- .../Domain/Service/NodeContextCombinator.php | 3 +++ Resources/Private/Translations/de/Main.xlf | 6 ++--- .../AutomaticReleaseSwitchServiceTest.php | 26 +++++++------------ composer.json | 3 ++- 6 files changed, 20 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0134467..d6aaae6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: uses: php-actions/phpunit@v4 with: php_version: ${{ matrix.php_version }} - php_extensions: xdebug + php_extensions: xdebug redis bootstrap: vendor/autoload.php configuration: phpunit.xml coverage_text: true diff --git a/Classes/Core/AutomaticReleaseSwitchService.php b/Classes/Core/AutomaticReleaseSwitchService.php index 87cb89b..035a18f 100644 --- a/Classes/Core/AutomaticReleaseSwitchService.php +++ b/Classes/Core/AutomaticReleaseSwitchService.php @@ -54,7 +54,7 @@ public function pause(): void } $this->redisClientManager->getPrimaryRedis()->hMset(self::REDIS_KEY, [ - 'pausedAt' => new DateTimeImmutable()->format(DateTimeInterface::ATOM), + 'pausedAt' => (new DateTimeImmutable())->format(DateTimeInterface::ATOM), 'accountId' => $this->getAccountId() ?? '', 'suppressedReleaseCount' => 0 ]); diff --git a/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php b/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php index e62bb54..9694881 100644 --- a/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php +++ b/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php @@ -79,6 +79,9 @@ public function nodeInContexts(string $nodeIdentifier, Site $site, string $works public function nodeVariantsWithSiteNode(string $nodeIdentifier, string $workspaceName = 'live'): \Generator { foreach ($this->sites() as $site) { + // a flag rather than a plain return after the inner loop: a site which does not contain the node + // yields nothing and has to fall through to the next site, and returning from inside the inner loop + // would cut off the node's remaining dimension variants $nodeFound = false; // hidden nodes are shown here regardless of "recurseHiddenContent": somebody named this node by its diff --git a/Resources/Private/Translations/de/Main.xlf b/Resources/Private/Translations/de/Main.xlf index 395e70d..a512b12 100644 --- a/Resources/Private/Translations/de/Main.xlf +++ b/Resources/Private/Translations/de/Main.xlf @@ -13,7 +13,7 @@ Automatic content releases are paused. Editor publishes will not go live until you resume them. - Automatische Content-Releases sind pausiert. Veröffentlichungen der Redakteure gehen erst live, wenn Sie die Releases wieder fortsetzen. + Automatische Content-Releases sind pausiert. Veröffentlichungen der Redakteure gehen erst live, wenn Releases wieder fortgesetzt werden. Automatic content releases are enabled again. Changes published while they were paused go live with the next release. @@ -40,7 +40,7 @@ Automatic content releases have been paused on {0}, so your changes do not go live yet. {1} release(s) are waiting. An administrator has to resume them in the Content Store module. - Automatische Content-Releases wurden am {0} pausiert, Ihre Änderungen gehen daher noch nicht live. {1} Release(s) warten. Ein Administrator muss sie im Modul „Content Store“ wieder fortsetzen. + Automatische Content-Releases wurden am {0} pausiert, Änderungen gehen daher noch nicht live. {1} Release(s) warten. Ein Administrator muss sie im Modul „Content Store“ wieder fortsetzen. @@ -69,7 +69,7 @@ One row per dimension. Rows marked in red are not published - check them before you continue, because a page you meant to fix would stay as it is. - Eine Zeile pro Dimension. Rot markierte Zeilen werden nicht veröffentlicht – prüfen Sie diese, bevor Sie fortfahren, denn eine Seite, die Sie korrigieren wollten, bliebe unverändert. + Eine Zeile pro Dimension. Rot markierte Zeilen werden nicht veröffentlicht – diese bitte prüfen, denn diese werden nicht neu gerendert. diff --git a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php index a541c07..cb56367 100644 --- a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php +++ b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php @@ -4,11 +4,13 @@ namespace Flowpack\DecoupledContentStore\Tests\Unit\Core; +use DateTimeImmutable; +use DateTimeInterface; use Flowpack\DecoupledContentStore\Core\AutomaticReleaseSwitchService; use Flowpack\DecoupledContentStore\Core\Infrastructure\RedisClientManager; use Neos\Flow\Security\Context; use Neos\Flow\Tests\UnitTestCase; -use PHPUnit\Framework\MockObject\MockObject; +use Redis; /** * Tests the switch which suppresses automatically triggered content releases. @@ -24,7 +26,7 @@ public function testTheSwitchIsSetOnlyIfThePausedAtFieldExists(): void { // countSuppressedRelease() re-creates the key with nothing but its counter if it races a resume, so the // existence of the key itself says nothing - $redis = $this->buildRedis(); + $redis = $this->createMock(Redis::class); $redis->method('hExists')->with(self::REDIS_KEY, 'pausedAt')->willReturn(false); self::assertFalse($this->buildService($redis)->isPaused()); @@ -33,7 +35,7 @@ public function testTheSwitchIsSetOnlyIfThePausedAtFieldExists(): void public function testPausingWhileAlreadyPausedKeepsTheOriginalState(): void { // otherwise the second pause would reset both the timestamp and the count of suppressed releases - $redis = $this->buildRedis(); + $redis = $this->createMock(Redis::class); $redis->method('hExists')->willReturn(true); $redis->expects(self::never())->method('hMSet'); @@ -42,7 +44,7 @@ public function testPausingWhileAlreadyPausedKeepsTheOriginalState(): void public function testPausingRecordsTheTimestampAndAnEmptyCounter(): void { - $redis = $this->buildRedis(); + $redis = $this->createMock(Redis::class); $redis->method('hExists')->willReturn(false); $redis ->expects(self::once()) @@ -51,7 +53,7 @@ public function testPausingRecordsTheTimestampAndAnEmptyCounter(): void return ( $hash['accountId'] === '' && $hash['suppressedReleaseCount'] === 0 - && \DateTimeImmutable::createFromFormat(\DateTimeInterface::ATOM, $hash['pausedAt']) !== false + && DateTimeImmutable::createFromFormat(DateTimeInterface::ATOM, $hash['pausedAt']) !== false ); })); @@ -60,7 +62,7 @@ public function testPausingRecordsTheTimestampAndAnEmptyCounter(): void public function testThereIsNoPauseStateWhileTheSwitchIsNotSet(): void { - $redis = $this->buildRedis(); + $redis = $this->createMock(Redis::class); $redis->method('hGetAll')->willReturn([]); self::assertNull($this->buildService($redis)->getPauseState()); @@ -68,7 +70,7 @@ public function testThereIsNoPauseStateWhileTheSwitchIsNotSet(): void public function testThePauseStateIsReadFromTheHash(): void { - $redis = $this->buildRedis(); + $redis = $this->createMock(Redis::class); $redis ->method('hGetAll') ->willReturn([ @@ -84,15 +86,7 @@ public function testThePauseStateIsReadFromTheHash(): void self::assertSame(4, $pauseState->getSuppressedReleaseCount()); } - /** - * @return \Redis&MockObject - */ - private function buildRedis(): \Redis - { - return $this->createMock(\Redis::class); - } - - private function buildService(\Redis $redis): AutomaticReleaseSwitchService + private function buildService(Redis $redis): AutomaticReleaseSwitchService { $redisClientManager = $this->createMock(RedisClientManager::class); $redisClientManager->method('getPrimaryRedis')->willReturn($redis); diff --git a/composer.json b/composer.json index aa2c1b6..fa22422 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,8 @@ }, "autoload-dev": { "psr-4": { - "Flowpack\\DecoupledContentStore\\Tests\\": "Tests/" + "Flowpack\\DecoupledContentStore\\Tests\\": "Tests/", + "Neos\\Flow\\Tests\\": "Packages/Framework/Neos.Flow/Tests/" } }, "extra": { From b23dbb14824b21fac764b4654324a03ae7d1c884 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:35:30 +0200 Subject: [PATCH 11/23] fix bug 1 --- .../Domain/Service/NodeContextCombinator.php | 50 +++++++++++++------ 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php b/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php index 9694881..4fdfb41 100644 --- a/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php +++ b/Classes/NodeEnumeration/Domain/Service/NodeContextCombinator.php @@ -5,11 +5,13 @@ namespace Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service; use Flowpack\DecoupledContentStore\Exception\NodeNotFoundException; +use Generator; +use Neos\ContentRepository\Domain\Model\NodeInterface; +use Neos\ContentRepository\Domain\Projection\Content\TraversableNodeInterface; use Neos\ContentRepository\Domain\Service\ContentDimensionCombinator; use Neos\ContentRepository\Domain\Service\ContextFactoryInterface; use Neos\Flow\Annotations as Flow; use Neos\Neos\Domain\Model\Site; -use Neos\ContentRepository\Domain\Model\NodeInterface; use Neos\Neos\Domain\Repository\SiteRepository; class NodeContextCombinator @@ -41,10 +43,10 @@ class NodeContextCombinator /** * Iterate over the node with the given identifier and site in contexts for all available presets (if it exists as a variant) * - * @return \Generator + * @return Generator * @throws NodeNotFoundException */ - public function nodeInContexts(string $nodeIdentifier, Site $site, string $workspaceName = 'live'): \Generator + public function nodeInContexts(string $nodeIdentifier, Site $site, string $workspaceName = 'live'): Generator { $nodeFound = false; @@ -74,9 +76,9 @@ public function nodeInContexts(string $nodeIdentifier, Site $site, string $works * need for the orphan check. It also does not report "not found" per site: a node is part of one site, so all * the others not having it is the normal case rather than an error. * - * @return \Generator the site node and the node, in that order + * @return Generator the site node and the node, in that order */ - public function nodeVariantsWithSiteNode(string $nodeIdentifier, string $workspaceName = 'live'): \Generator + public function nodeVariantsWithSiteNode(string $nodeIdentifier, string $workspaceName = 'live'): Generator { foreach ($this->sites() as $site) { // a flag rather than a plain return after the inner loop: a site which does not contain the node @@ -89,26 +91,45 @@ public function nodeVariantsWithSiteNode(string $nodeIdentifier, string $workspa // report that it does not exist foreach ($this->siteNodeInContexts($site, $workspaceName, true) as $siteNode) { $node = $siteNode->getContext()->getNodeByIdentifier($nodeIdentifier); - if ($node instanceof NodeInterface) { + // getNodeByIdentifier() looks the node up in the whole content repository rather than inside the + // site, so it answers for every site alike and the node has to be matched against the site itself + if ($node instanceof NodeInterface && self::isWithinSiteNode($node, $siteNode)) { $nodeFound = true; yield [$siteNode, $node]; } } if ($nodeFound) { - // getNodeByIdentifier() looks the node up in the whole content repository rather than inside the - // site, so every further site would hand out the very same variants again + // a node belongs to exactly one site, so the remaining ones cannot contribute further variants return; } } } + /** + * Whether the node is the site node itself or lives below it. + * + * Compared by path rather than by walking up the parents: the walk gives the same answer, while this runs for + * every site the node is looked up in. Nodes without a path cannot be placed in a site and count as outside it. + */ + private static function isWithinSiteNode(NodeInterface $node, NodeInterface $siteNode): bool + { + if (!$node instanceof TraversableNodeInterface || !$siteNode instanceof TraversableNodeInterface) { + return false; + } + + $nodePath = $node->findNodePath(); + $siteNodePath = $siteNode->findNodePath(); + + return $nodePath->equals($siteNodePath) || str_starts_with((string)$nodePath, $siteNodePath . '/'); + } + /** * Iterate over all sites * - * @return \Generator + * @return Generator */ - public function sites(): \Generator + public function sites(): Generator { $sites = $this->siteRepository->findAll(); @@ -121,13 +142,14 @@ public function sites(): \Generator * Iterate over the site node in all available presets (if it exists) * * @param bool|null $invisibleContentShown NULL follows the "nodeRendering.recurseHiddenContent" setting - * @return \Generator + * @return Generator */ public function siteNodeInContexts( Site $site, string $workspaceName = 'live', ?bool $invisibleContentShown = null - ): \Generator { + ): Generator + { $allowedContextCombinations = $this->contentDimensionCombinator->getAllAllowedCombinations(); foreach ($allowedContextCombinations as $dimensionContextCombination) { @@ -150,9 +172,9 @@ public function siteNodeInContexts( /** * Iterate over the given node and all document child nodes recursively * - * @return \Generator + * @return Generator */ - public function recurseDocumentChildNodes(NodeInterface $node): \Generator + public function recurseDocumentChildNodes(NodeInterface $node): Generator { yield $node; From 111a11375704157ff8f65506db27bdd5e96cf69c Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:42:08 +0200 Subject: [PATCH 12/23] fix bug 2 --- ...tentReleaseValidationCommandController.php | 38 ++++++++++++------- .../Concepts/QuickContentReleases.md | 6 ++- README.md | 6 ++- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/Classes/Command/ContentReleaseValidationCommandController.php b/Classes/Command/ContentReleaseValidationCommandController.php index 2791a4f..13d151b 100644 --- a/Classes/Command/ContentReleaseValidationCommandController.php +++ b/Classes/Command/ContentReleaseValidationCommandController.php @@ -68,19 +68,8 @@ public function validateCommand(string $contentReleaseIdentifier) } $logger->info('Previous Content Release: ' . $currentlyLiveReleaseIdentifier->getIdentifier()); - if ($this->contentReleaseScope->getChangedUrls($contentReleaseIdentifier) !== null) { - // A quick release enumerates the handful of documents it re-renders and copies the rest, so its - // enumeration is smaller than the live one by design and would fail this check every single time. - // Its published URLs are the comparable number: after the copy they equal the release it was built on. - $logger->info( - 'Content release is a quick release, so its published URLs are counted instead of its enumeration.' - ); - $currentUrlsCount = $this->contentReleaseScope->countPublishedUrls($currentlyLiveReleaseIdentifier); - $newUrlsCount = $this->contentReleaseScope->countPublishedUrls($contentReleaseIdentifier); - } else { - $currentUrlsCount = $this->redisEnumerationRepository->count($currentlyLiveReleaseIdentifier); - $newUrlsCount = $this->redisEnumerationRepository->count($contentReleaseIdentifier); - } + $currentUrlsCount = $this->countUrls($currentlyLiveReleaseIdentifier, $logger, 'Currently live release'); + $newUrlsCount = $this->countUrls($contentReleaseIdentifier, $logger, 'Content release'); $minimumUrlsCount = (int) ceil($this->validReleaseUrlCountThreshold * $currentUrlsCount); $logger->info('Previous URL Count: ' . $currentUrlsCount); @@ -123,6 +112,29 @@ public function validateCommand(string $contentReleaseIdentifier) $this->logCompletion($logger, $startedAt); } + /** + * How many URLs a content release covers, measured so that two releases are comparable. + * + * A quick release enumerates only the handful of documents it re-renders and copies the rest, so its enumeration + * describes neither what it publishes nor what a later release has to live up to - taken as the baseline it would + * put the threshold at a handful of URLs and wave through any release which lost most of the site. Its published + * URLs are the comparable number: after the copy they equal the release it was built on. Each release is + * therefore measured on its own terms, whichever side of the comparison it is on. + */ + private function countUrls( + ContentReleaseIdentifier $contentReleaseIdentifier, + ContentReleaseLogger $logger, + string $label + ): int { + if ($this->contentReleaseScope->getChangedUrls($contentReleaseIdentifier) === null) { + return $this->redisEnumerationRepository->count($contentReleaseIdentifier); + } + + $logger->info($label . ' is a quick release, so its published URLs are counted instead of its enumeration.'); + + return $this->contentReleaseScope->countPublishedUrls($contentReleaseIdentifier); + } + /** * Final log line of the command. If it is the last line you see while the task is still marked as * "running" in the UI, the remaining time is NOT spent in this command, but in another script line diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index 95d5d8a..c23860c 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -217,8 +217,10 @@ every ordinary release through, so the two cases have to be handled explicitly. This is a trap rather than an optimisation. The validator compares the enumeration count of the new release against the live one and aborts below 70%. A quick release deliberately enumerates a handful of documents instead of all of -them, so the check would fail every single time. For a quick release it compares the number of *published* URLs -instead, which after a copy-forward equals the previous release. +them, so it is counted by its number of *published* URLs instead, which after a copy-forward equals the release it +was built on. Both sides of the comparison are measured that way, each release on its own terms: as the new release a +quick one would fail the check every single time, and as the currently live release it would put the threshold at a +handful of URLs and let the next full release pass no matter how much of the site that one lost. Any project validator which reasons about the size of the enumeration has the same problem, and the failure mode is the good one — the release is refused rather than published wrongly — but it needs the same treatment. diff --git a/README.md b/README.md index 56128a9..ff01be1 100644 --- a/README.md +++ b/README.md @@ -449,8 +449,10 @@ document hash into an `hMGet` for the changed URLs. The package's own `contentReleaseValidation:validate` already does this, and it had to: it compares the enumeration of the new release against the live one and aborts below 70%, while a quick release deliberately enumerates a handful -of documents instead of all of them. For a quick release it compares the number of published URLs instead, which -after a copy-forward equals the previous release. +of documents instead of all of them. A quick release is therefore counted by its number of published URLs — which +after a copy-forward equals the release it was built on — whichever side of the comparison it stands on. As the new +release its enumeration would fail the check every single time; as the currently live one it would put the threshold +at a handful of URLs and wave the next full release through however much of the site that one lost. ### The commands From ba0f0634a9b1310f97567650b506c9ca1c29577b Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:43:32 +0200 Subject: [PATCH 13/23] remove ununsed file --- .../Render/NodeContextCombinator.php | 134 ------------------ phpstan-baseline.neon | 18 --- 2 files changed, 152 deletions(-) delete mode 100644 Classes/NodeRendering/Render/NodeContextCombinator.php diff --git a/Classes/NodeRendering/Render/NodeContextCombinator.php b/Classes/NodeRendering/Render/NodeContextCombinator.php deleted file mode 100644 index a892324..0000000 --- a/Classes/NodeRendering/Render/NodeContextCombinator.php +++ /dev/null @@ -1,134 +0,0 @@ -siteNodeInContexts($site) as $siteNode) { - $node = $siteNode->getContext()->getNodeByIdentifier($nodeIdentifier); - - if ($node instanceof NodeInterface) { - $nodeFound = true; - yield $node; - } - } - - if (!$nodeFound) { - throw new Exception\NodeNotFoundException( - 'Could not find node by identifier ' . $nodeIdentifier . ' in any context', - 1467285561 - ); - } - } - - /** - * Iterate over all sites - * - * @return Site[] - */ - public function sites() - { - $sites = $this->siteRepository->findAll(); - - foreach ($sites as $site) { - yield $site; - } - } - - /** - * Iterate over the site node in all available presets (if it exists) - * - * @param Site $site - * @return NodeInterface[] - */ - public function siteNodeInContexts(Site $site) - { - $presets = $this->dimensionPresetSource->getAllPresets(); - if ($presets === []) { - $contentContext = $this->contextFactory->create(array( - 'currentSite' => $site, - 'workspaceName' => 'live', - 'dimensions' => [], - 'targetDimensions' => [] - )); - - $siteNode = $contentContext->getNode('/sites/' . $site->getNodeName()); - - yield $siteNode; - } else { - foreach ($presets as $dimensionIdentifier => $presetsConfiguration) { - foreach ($presetsConfiguration['presets'] as $presetIdentifier => $presetConfiguration) { - $dimensions = [$dimensionIdentifier => $presetConfiguration['values']]; - - $contentContext = $this->contextFactory->create(array( - 'currentSite' => $site, - 'workspaceName' => 'live', - 'dimensions' => $dimensions, - 'targetDimensions' => [] - )); - - $siteNode = $contentContext->getNode('/sites/' . $site->getNodeName()); - - if ($siteNode instanceof NodeInterface) { - yield $siteNode; - } - } - } - } - } - - /** - * Iterate over the given node and all document child nodes recursively - * - * @param NodeInterface $node - * @return NodeInterface[] - */ - public function recurseDocumentChildNodes(NodeInterface $node) - { - yield $node; - - foreach ($node->getChildNodes('Neos.Neos:Document') as $node) { - foreach ($this->recurseDocumentChildNodes($node) as $childNode) { - yield $childNode; - } - } - } -} diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index dacb8e4..0484788 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1410,24 +1410,6 @@ parameters: count: 1 path: Classes/NodeRendering/Render/ExtractedExceptionDto.php - - - message: '#^Foreach overwrites \$node with its value variable\.$#' - identifier: foreach.valueOverwrite - count: 1 - path: Classes/NodeRendering/Render/NodeContextCombinator.php - - - - message: '#^Instanceof between Neos\\ContentRepository\\Domain\\Model\\NodeInterface and Neos\\ContentRepository\\Domain\\Model\\NodeInterface will always evaluate to true\.$#' - identifier: instanceof.alwaysTrue - count: 1 - path: Classes/NodeRendering/Render/NodeContextCombinator.php - - - - message: '#^Yield can be used only with these return types\: Generator, Iterator, Traversable, iterable\.$#' - identifier: generator.returnType - count: 5 - path: Classes/NodeRendering/Render/NodeContextCombinator.php - - message: '#^Method Flowpack\\DecoupledContentStore\\NodeRendering\\Render\\RenderExceptionExtractor\:\:extractRenderingException\(\) should return Flowpack\\DecoupledContentStore\\NodeRendering\\Render\\ExtractedExceptionDto but returns null\.$#' identifier: return.type From 5a5c9b1f2514b01e8e37a16a5dc0804345534cf0 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:48:26 +0200 Subject: [PATCH 14/23] fix bug 3 --- .../QuickPublishNodeEnumerator.php | 19 ++++++++++--------- .../Concepts/QuickContentReleases.md | 7 +++++++ README.md | 5 +++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Classes/QuickPublish/QuickPublishNodeEnumerator.php b/Classes/QuickPublish/QuickPublishNodeEnumerator.php index be4dd0a..f949c02 100644 --- a/Classes/QuickPublish/QuickPublishNodeEnumerator.php +++ b/Classes/QuickPublish/QuickPublishNodeEnumerator.php @@ -10,7 +10,6 @@ use Flowpack\DecoupledContentStore\Core\Infrastructure\ContentReleaseLogger; use Flowpack\DecoupledContentStore\Exception; use Flowpack\DecoupledContentStore\Exception\InvalidReleaseException; -use Flowpack\DecoupledContentStore\Exception\NodeNotFoundException; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Dto\EnumeratedNode; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Repository\RedisEnumerationRepository; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\DocumentNodeFilter; @@ -64,7 +63,7 @@ final class QuickPublishNodeEnumerator protected ContentReleaseScope $contentReleaseScope; /** - * @throws Exception if a given node cannot be published, or nothing is left to render + * @throws Exception if nothing is left to render */ public function enumerateGivenNodesAndStoreInRedis( NodeIdentifiers $nodeIdentifiers, @@ -157,8 +156,9 @@ private function writeChangedUrls( } /** + * The node variants to render, skipping every given identifier which must not or cannot be published. + * * @return array - * @throws Exception */ private function enumerateGivenNodes( NodeIdentifiers $nodeIdentifiers, @@ -199,12 +199,13 @@ private function enumerateGivenNodes( } if (!$contentCacheFlushed) { - throw new NodeNotFoundException( - sprintf( - 'Could not find node %s in any site and dimension, so it cannot be published.', - $nodeIdentifier - ), - 1786958514 + // skipped like every other unpublishable node rather than failing the task: the confirmation page + // lists an identifier which resolves nowhere as one row among the others, so somebody publishing + // five documents of which one was deleted in the meantime gets the other four. A list in which + // nothing at all can be published still fails, in enumerateGivenNodesAndStoreInRedis() + $contentReleaseLogger->warn( + 'Skipping node from publishing, because it is not found in any site and dimension', + ['node' => $nodeIdentifier] ); } } diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index c23860c..183e7ad 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -286,6 +286,13 @@ method the enumerator calls, so what the confirmation page says will be skipped skips rather than a second implementation of the same rules. An identifier which resolves nowhere becomes a row rather than an error: somebody who pasted five identifiers needs to see which one is wrong. +The enumeration skips such an identifier as well, instead of failing the task. Otherwise the page's promise would +not hold in the one case it cannot rule out: a document deleted between the preview and the confirmation would take +the whole release down with it — with the release already marked `running` and its enumeration cleared — rather than +publishing the other four documents the editor asked for. `DocumentNodeFilter::NOT_FOUND_SKIP_REASON` is the wording +both sides use. What still fails the task is a list in which *nothing* can be published, because the release would +then be a copy of the live one under a new identifier. + One trap for anyone adding forms to this module: Neos dispatches a backend module as a sub-request and reads its arguments from the `moduleArguments` namespace alone, so a field posted at the top level never reaches the action and fails with "required argument is missing". Both forms name their fields `moduleArguments[…]`; `__csrfToken` belongs diff --git a/README.md b/README.md index ff01be1..83e1703 100644 --- a/README.md +++ b/README.md @@ -469,8 +469,9 @@ job log: The copy refuses a source release whose status is not `success` or which is missing a required key, because switching a release live by hand is possible and "currently live" alone does not guarantee a clean release. The -enumeration refuses an identifier which resolves nowhere, and refuses to end up empty — a quick release which renders -nothing would publish the release it copied and look like a successful publish while the change is nowhere. +enumeration skips an identifier which resolves nowhere — with a warning in the job log, like every other node it +cannot publish — and refuses to end up empty: a quick release which renders nothing would publish the release it +copied and look like a successful publish while the change is nowhere. To start one from your own code: From 7433ee0d7280b0c08fa5c481687004f656e8e113 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:50:02 +0200 Subject: [PATCH 15/23] remove deprecated function usage --- Classes/QuickPublish/QuickPublishPreviewService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Classes/QuickPublish/QuickPublishPreviewService.php b/Classes/QuickPublish/QuickPublishPreviewService.php index b387a0c..b01bcbf 100644 --- a/Classes/QuickPublish/QuickPublishPreviewService.php +++ b/Classes/QuickPublish/QuickPublishPreviewService.php @@ -9,6 +9,7 @@ use Flowpack\DecoupledContentStore\QuickPublish\Dto\NodeIdentifiers; use Flowpack\DecoupledContentStore\QuickPublish\Dto\QuickPublishPreviewRow; use Neos\ContentRepository\Domain\Model\NodeInterface; +use Neos\ContentRepository\Domain\Projection\Content\TraversableNodeInterface; use Neos\Flow\Annotations as Flow; use Neos\Flow\Mvc\Controller\ControllerContext; @@ -43,7 +44,7 @@ public function preview(NodeIdentifiers $nodeIdentifiers, ControllerContext $con $rows[] = QuickPublishPreviewRow::forNode( $nodeIdentifier, $node->getLabel(), - $node->getPath(), + $node instanceof TraversableNodeInterface ? (string)$node->findNodePath() : '', self::describeDimensions($node), $node->getNodeType()->getName(), $this->backendUri($node, $controllerContext), From 6b682619361f6e7a8a79132604e2b7af63929ecc Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:54:02 +0200 Subject: [PATCH 16/23] fix bug 4 --- Classes/ContentReleaseManager.php | 8 +++++- .../Concepts/QuickContentReleases.md | 8 +++++- Tests/Unit/ContentReleaseManagerTest.php | 26 ++++++++++++++++--- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/Classes/ContentReleaseManager.php b/Classes/ContentReleaseManager.php index 765e402..e1bdfb0 100644 --- a/Classes/ContentReleaseManager.php +++ b/Classes/ContentReleaseManager.php @@ -160,7 +160,13 @@ public function startQuickContentRelease( ->loadPipelinesAndJobs() ->getJobs() ->forPipeline(PipelineName::create(self::QUICK_CONTENT_RELEASE_PIPELINE_NAME)); - if ($quickReleaseJobs->running()->getArray() !== [] || $quickReleaseJobs->waiting()->getArray() !== []) { + // Jobs::waiting() means "never started", which is true of a job cancelled while it was still queued as well. + // Such a job stays in prunner's list until it falls out of the pipeline's retention_count - a window only + // quick releases consume - so without the isCompleted() guard one cancelled job blocks them all until then. + $queuedQuickReleaseJobs = $quickReleaseJobs + ->waiting() + ->filter(static fn(Job $job): bool => !$job->isCompleted()); + if ($quickReleaseJobs->running()->getArray() !== [] || $queuedQuickReleaseJobs->getArray() !== []) { throw new QuickContentReleaseNotPossibleException( 'Another quick content release is still on its way. Wait for it to go live, then publish these nodes.', 1786963711 diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index 183e7ad..307623c 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -122,7 +122,13 @@ which demanded every required key would refuse to build on a perfectly good rele would silently replace a quick release. For the same reason the quick pipeline does **not** use `queue_strategy: replace` either: a quick release publishes -exactly the documents somebody named, so a queued one must not be thrown away by the next one. +exactly the documents somebody named, so a queued one must not be thrown away by the next one. Instead +`ContentReleaseManager::startQuickContentRelease()` refuses to schedule a second one while the first has not gone +live, because the copy source is resolved when the release is scheduled: a queued release would build on the release +the first one is about to replace and drop that change without a word. `Jobs::waiting()` alone is not that check — +it means "never started", which a job cancelled while it was still queued never was either, and such a job stays in +prunner's list until it falls out of `retention_count`. It is therefore combined with `isCompleted()`, or one +cancelled job would block every quick release for as long as it is kept. Details of the pipeline which are not obvious from reading it: diff --git a/Tests/Unit/ContentReleaseManagerTest.php b/Tests/Unit/ContentReleaseManagerTest.php index 7018842..e613bcf 100644 --- a/Tests/Unit/ContentReleaseManagerTest.php +++ b/Tests/Unit/ContentReleaseManagerTest.php @@ -135,6 +135,18 @@ public static function quickReleasesOnTheirWay(): array return ['running' => [true], 'waiting in the queue' => [false]]; } + public function testAQuickReleaseIsScheduledAfterAnEarlierOneWasCancelledBeforeItStarted(): void + { + // such a job has no start time, so it counts as waiting for as long as prunner keeps it in its job list + $this->currentContentReleaseId = '5'; + $this->prunnerApiService + ->method('loadPipelinesAndJobs') + ->willReturn($this->jobsResponse('do_quick_content_release', false, true)); + $this->prunnerApiService->expects(self::once())->method('schedulePipeline'); + + $this->buildContentReleaseManager()->startQuickContentRelease($this->nodeIdentifiers()); + } + public function testARunningQuickReleaseIsCancelledAlongWithTheOtherContentReleases(): void { // it ends up being switched live just like a full release does, so "cancel" has to reach it @@ -151,8 +163,14 @@ private function nodeIdentifiers(): NodeIdentifiers return NodeIdentifiers::fromCommaSeparatedString(self::NODE_IDENTIFIER); } - private function jobsResponse(string $pipeline, bool $started): PipelinesAndJobsResponse - { + /** + * @param bool $canceled a cancelled job is a completed one as well, whether or not it ever started + */ + private function jobsResponse( + string $pipeline, + bool $started, + bool $canceled = false + ): PipelinesAndJobsResponse { return PipelinesAndJobsResponse::fromJsonArray([ 'pipelines' => [], 'jobs' => [ @@ -160,8 +178,8 @@ private function jobsResponse(string $pipeline, bool $started): PipelinesAndJobs 'id' => 'job-id', 'pipeline' => $pipeline, 'tasks' => [], - 'completed' => false, - 'canceled' => false, + 'completed' => $canceled, + 'canceled' => $canceled, 'errored' => false, 'created' => '2026-08-17T10:00:00+02:00', 'start' => $started ? '2026-08-17T10:00:01+02:00' : null, From 9af5c9df0ae53222da4c5b9a8a30a1e8f799e376 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 12:59:34 +0200 Subject: [PATCH 17/23] fix bug 5 --- .../Domain/Service/DocumentNodeFilter.php | 32 ++++++++- .../Concepts/QuickContentReleases.md | 24 +++++-- .../Domain/Service/DocumentNodeFilterTest.php | 66 +++++++++++++++++++ 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php index c7c48f8..51cc455 100644 --- a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php +++ b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php @@ -98,8 +98,9 @@ public function skipReason(NodeInterface $node, NodeInterface $siteNode): ?strin /** * Why a node somebody named by identifier must not go into a content release, or NULL if it may. * - * The node type is part of the answer here, unlike in {@see skipReason()}: a node which was named by hand did - * not come out of the FlowQuery filter, so nothing else checked it. + * The node type and the pages above the node are part of the answer here, unlike in {@see skipReason()}: a node + * which was named by hand did not come out of the FlowQuery filter and was not reached by descending from the + * site node, so nothing else checked either of them. */ public function skipReasonForNamedNode(NodeInterface $node, NodeInterface $siteNode): ?string { @@ -108,9 +109,36 @@ public function skipReasonForNamedNode(NodeInterface $node, NodeInterface $siteN return $skipReason; } + if (self::hasHiddenAncestor($node)) { + return 'below a hidden page'; + } + return $this->matchesNodeTypeWhitelist($node) ? null : 'not of a node type which is published'; } + /** + * Whether one of the pages above the node is hidden. + * + * Only asked about a node named by identifier, which is resolved in a context that shows hidden nodes so that + * "it is hidden" can be reported instead of "it does not exist". The full enumeration needs no such check: it + * descends from the site node in a context which hides them, and therefore never reaches a document below a + * hidden page - publishing one from a quick release would put a page live which the next full release removes + * again. The walk goes past the site node, because a hidden site node keeps its whole site out of a full release + * just as well. + */ + private static function hasHiddenAncestor(NodeInterface $node): bool + { + $parentNode = self::getParentNodeOrNull($node); + while ($parentNode !== null) { + if ($parentNode->isHidden()) { + return true; + } + $parentNode = self::getParentNodeOrNull($parentNode); + } + + return false; + } + private static function isOrphaned(NodeInterface $node, NodeInterface $siteNode): bool { $parentNode = self::getParentNodeOrNull($node); diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index 307623c..4233539 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -174,19 +174,22 @@ starting from identifiers needs. The node-type check is deliberately **not** par `NodeEnumerator` adds the site node to its result without passing it through the FlowQuery filter, so folding the node type into the shared guard would silently drop site nodes whose type is excluded. -Two failures abort the task rather than shrinking the release quietly: an identifier which resolves in no site and -dimension, and an enumeration which ends up empty. A quick release which renders nothing would publish the release it -copied and look like a successful publish while the change is nowhere. Everything it skips is logged as a warning -rather than at debug level, because somebody asked for those documents by hand. +One failure aborts the task rather than shrinking the release quietly: an enumeration which ends up empty. A quick +release which renders nothing would publish the release it copied and look like a successful publish while the change +is nowhere. An identifier which resolves in no site and dimension is a skip like any other, because a document can be +deleted between the confirmation page and the confirmation itself. Everything skipped is logged as a warning rather +than at debug level, because somebody asked for those documents by hand. The identifiers are a value object, `QuickPublish/Dto/NodeIdentifiers`, which rejects anything that is not a UUID. This is not cosmetic: the list travels through a pipeline variable into a shell command, so unvalidated input is a command-injection hole. The check lives at the point where the list is read, not only in the backend form. Variants of a node are resolved through `NodeContextCombinator::nodeVariantsWithSiteNode()` — via `sites()` and -`siteNodeInContexts()` rather than `nodeInContexts()`, because the orphan check needs the site node, and -`getNodeByIdentifier()` searches the whole content repository rather than one site, so iterating every site would -hand out the same variants once per site. +`siteNodeInContexts()` rather than `nodeInContexts()`, because the orphan check needs the site node. +`getNodeByIdentifier()` searches the whole content repository rather than one site, so it answers for every site +alike: each candidate is matched against the site node by its `findNodePath()` before it is handed out, or a +multi-site installation would pair every node with the first site and report it as orphaned. Once a site has claimed +the node the remaining ones are skipped, since a node belongs to exactly one site. That lookup shows invisible content regardless of `nodeRendering.recurseHiddenContent`, which defaults to `false`. The setting is about recursing into hidden content while walking the tree; applied to a lookup by identifier it makes @@ -195,6 +198,13 @@ any site and dimension". `siteNodeInContexts()` therefore takes an `$invisibleCo the configured behaviour for the full enumeration. Hidden pages are still not published — the skip reason says "hidden", on the confirmation page and in the pipeline log alike. +Because of that override, `skipReasonForNamedNode()` also has to walk the pages **above** the node, which +`skipReason()` does not: the full enumeration descends from the site node in a context which hides them, so a +document below a hidden page is not in the content store at all. Naming it in a quick release would resolve, render +and publish it — a page live until the next full release quietly removes it again. The skip reason for that is +"below a hidden page". It is the node's own `hidden` flag which is checked, so a page hidden by +`hiddenBeforeDateTime` / `hiddenAfterDateTime` is not covered on either level. + ## 6. Validation scoped to the changed URLs With rendering gone, validation is the whole cost of a quick release. It is also almost entirely wasted work: after a diff --git a/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php b/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php index 3323936..f78989a 100644 --- a/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php +++ b/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php @@ -5,6 +5,10 @@ namespace Flowpack\DecoupledContentStore\Tests\Unit\NodeEnumeration\Domain\Service; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\DocumentNodeFilter; +use Neos\ContentRepository\Domain\Model\Node; +use Neos\ContentRepository\Domain\Model\NodeType; +use Neos\ContentRepository\Exception\NodeException; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; /** @@ -14,6 +18,9 @@ * nodeTypeWhitelist: * - 'Neos.Neos:Document' * - '!My.Package:Bar' + * + * ... and the checks a node named by identifier for a quick release has to pass, which the FlowQuery filter and the + * traversal of the full enumeration answer implicitly. */ class DocumentNodeFilterTest extends TestCase { @@ -101,4 +108,63 @@ public function testEmptyWhitelistFallsBackToTheDefaultNodeType(): void { self::assertSame('[instanceof Neos.Neos:Document]', self::buildNodeTypeFilter([])); } + + public function testADocumentBelowAHiddenPageIsNotPublished(): void + { + // the full enumeration descends in a context which hides them and never reaches such a document, so a quick + // release publishing it would put a page live which the next full release removes again + $siteNode = $this->nodeMock(false, null); + $hiddenParentNode = $this->nodeMock(true, $siteNode); + $node = $this->nodeMock(false, $hiddenParentNode); + + self::assertSame( + 'below a hidden page', + $this->buildDocumentNodeFilter()->skipReasonForNamedNode($node, $siteNode) + ); + } + + public function testADocumentBelowVisiblePagesIsPublished(): void + { + $siteNode = $this->nodeMock(false, null); + $parentNode = $this->nodeMock(false, $siteNode); + $node = $this->nodeMock(false, $parentNode); + + self::assertNull($this->buildDocumentNodeFilter()->skipReasonForNamedNode($node, $siteNode)); + } + + private function buildDocumentNodeFilter(): DocumentNodeFilter + { + $documentNodeFilter = new DocumentNodeFilter(); + + // the setting arrives by InjectConfiguration, which no container assembles in a unit test + $nodeTypeWhitelist = new \ReflectionProperty(DocumentNodeFilter::class, 'nodeTypeWhitelist'); + $nodeTypeWhitelist->setValue($documentNodeFilter, ['Neos.Neos:Document']); + + return $documentNodeFilter; + } + + /** + * The content repository's node class is mocked rather than one of the two interfaces, because the walk up needs + * TraversableNodeInterface while everything else is NodeInterface, and that class is what implements both. + * + * @param Node|null $parentNode NULL where the walk up leaves the tree, as it does above the root node + * @return Node&MockObject + */ + private function nodeMock(bool $hidden, ?Node $parentNode): Node + { + $nodeType = $this->createMock(NodeType::class); + $nodeType->method('isOfType')->willReturn(true); + + $node = $this->createMock(Node::class); + $node->method('isHidden')->willReturn($hidden); + $node->method('getNodeType')->willReturn($nodeType); + + if ($parentNode === null) { + $node->method('findParentNode')->willThrowException(new NodeException()); + } else { + $node->method('findParentNode')->willReturn($parentNode); + } + + return $node; + } } From c5d5985b2de4374afc7eb9c1d07eaf954c235f2a Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 13:03:11 +0200 Subject: [PATCH 18/23] fix bug 6 --- Classes/NodeEnumeration/NodeEnumerator.php | 27 ++++++++++++++++--- .../QuickPublishNodeEnumerator.php | 9 +++++++ .../Concepts/QuickContentReleases.md | 8 ++++++ README.md | 5 ++++ 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Classes/NodeEnumeration/NodeEnumerator.php b/Classes/NodeEnumeration/NodeEnumerator.php index 6bc4283..d34f610 100644 --- a/Classes/NodeEnumeration/NodeEnumerator.php +++ b/Classes/NodeEnumeration/NodeEnumerator.php @@ -79,10 +79,29 @@ public function enumerateAndStoreInRedis( $this->redisEnumerationRepository->addDocumentNodesToEnumeration($releaseIdentifier, ...$enumeration); - // DEPRECATED: use extensions.documentRenderers.[...].enumeratorClassName instead - foreach ($enumeration as $enumeratedNode) { - $this->emitNodeEnumerated($enumeratedNode, $releaseIdentifier, $contentReleaseLogger); - } + $this->emitNodesEnumerated($enumeration, $releaseIdentifier, $contentReleaseLogger); + } + } + + /** + * Emit {@see emitNodeEnumerated()} for a batch of nodes, including one another enumerator wrote. + * + * A signal is identified by the class which declares it, so a slot connected to this one hears nothing about the + * nodes a quick release enumerates unless that release emits it from here as well - and the extra variants such + * a slot adds for a document (pagination, filter arguments) would keep the rendering of the release which was + * copied while the document itself is re-rendered. + * + * DEPRECATED: use extensions.documentRenderers.[...].enumeratorClassName instead + * + * @param array $enumeration + */ + public function emitNodesEnumerated( + array $enumeration, + ContentReleaseIdentifier $releaseIdentifier, + ContentReleaseLogger $contentReleaseLogger + ): void { + foreach ($enumeration as $enumeratedNode) { + $this->emitNodeEnumerated($enumeratedNode, $releaseIdentifier, $contentReleaseLogger); } } diff --git a/Classes/QuickPublish/QuickPublishNodeEnumerator.php b/Classes/QuickPublish/QuickPublishNodeEnumerator.php index f949c02..c2007ab 100644 --- a/Classes/QuickPublish/QuickPublishNodeEnumerator.php +++ b/Classes/QuickPublish/QuickPublishNodeEnumerator.php @@ -14,6 +14,7 @@ use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Repository\RedisEnumerationRepository; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\DocumentNodeFilter; use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\NodeContextCombinator; +use Flowpack\DecoupledContentStore\NodeEnumeration\NodeEnumerator; use Flowpack\DecoupledContentStore\NodeRendering\Dto\NodeRenderingCompletionStatus; use Flowpack\DecoupledContentStore\NodeRendering\Extensibility\NodeRenderingExtensionManager; use Flowpack\DecoupledContentStore\NodeRendering\NodeRenderingUriService; @@ -47,6 +48,9 @@ final class QuickPublishNodeEnumerator #[Flow\Inject] protected NodeContextCombinator $nodeContextCombinator; + #[Flow\Inject] + protected NodeEnumerator $nodeEnumerator; + #[Flow\Inject] protected DocumentNodeFilter $documentNodeFilter; @@ -121,6 +125,11 @@ public function enumerateGivenNodesAndStoreInRedis( ) as $enumeration) { $this->concurrentBuildLockService->assertNoOtherContentReleaseWasStarted($releaseIdentifier); $this->redisEnumerationRepository->addDocumentNodesToEnumeration($releaseIdentifier, ...$enumeration); + + // through the full enumerator, because that is the class the deprecated signal is declared in: a slot + // which adds further variants of a document would otherwise not hear about a quick release, and those + // variants would stay at the rendering of the release this one was copied from + $this->nodeEnumerator->emitNodesEnumerated($enumeration, $releaseIdentifier, $contentReleaseLogger); } $this->writeChangedUrls($nodesToRender, $releaseIdentifier, $contentReleaseLogger); diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index 4233539..2831253 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -167,6 +167,14 @@ full one. those nodes only, through `NodeRenderingExtensionManager::enumerateDocumentNode()` so that every configured renderer is covered. +The deprecated `nodeEnumerated` signal is emitted for that enumeration too, through +`NodeEnumerator::emitNodesEnumerated()` — a signal is identified by the class which declares it, so emitting it from +the quick enumerator would reach nobody. A slot on it derives further variants of a document (pagination, filter +arguments) and writes them into the enumeration itself; without the signal those variants would keep the rendering of +the release which was copied while the document itself is re-rendered, so a page and its own paginated variants would +go live disagreeing with each other. They are not written into `quickPublish:changedUrls`, though, which only knows +the documents that were named — a scoped validator does not see them. + The hidden / orphaned / node-type guards are shared with the full enumeration through `NodeEnumeration/Domain/Service/DocumentNodeFilter`, which expresses `nodeTypeWhitelist` twice: as the FlowQuery filter string the full enumeration passes to `find()`, and as a check for a single node, which is what an enumerator diff --git a/README.md b/README.md index 83e1703..85736d9 100644 --- a/README.md +++ b/README.md @@ -615,6 +615,11 @@ class NodeListsEnumerator The actual logic will depend on your use of the node. Having the actual filtering logic implemented in PHP is beneficial, because it allows you to use it in the rendering process as well as in the additional enumeration. +A [quick content release](#quick-content-releases) emits the signal for the documents it re-renders, so the extra +nodes a slot adds are re-rendered along with them instead of staying at the rendering of the release which was +copied. They are not part of the release's [scope](#scoping-your-own-validators), though: a validator which +reads `getChangedUrls()` sees the documents that were named, not the variants a slot derived from them. + ### Extending the backend module - You need a Views.yaml in your package, looking like this: From 3710c3f1c056fa2e75285d96dee1b0d5324c437a Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 13:07:21 +0200 Subject: [PATCH 19/23] fix bug 7 --- .../Core/AutomaticReleaseSwitchService.php | 23 ++++++++++++++++--- .../Concepts/QuickContentReleases.md | 6 +++-- .../AutomaticReleaseSwitchServiceTest.php | 21 +++++++++++++++-- 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/Classes/Core/AutomaticReleaseSwitchService.php b/Classes/Core/AutomaticReleaseSwitchService.php index 035a18f..697c057 100644 --- a/Classes/Core/AutomaticReleaseSwitchService.php +++ b/Classes/Core/AutomaticReleaseSwitchService.php @@ -24,6 +24,19 @@ class AutomaticReleaseSwitchService { private const REDIS_KEY = 'contentStore:automaticReleasesPaused'; + /** + * HINCRBY creates the hash it counts in, so counting has to be conditional on the switch still being set: a + * resume between the caller's isPaused() check and the count would otherwise leave a key behind which holds + * nothing but a counter. That key lives outside the per-release key space, so pruning never clears it. + */ + private const COUNT_SUPPRESSED_RELEASE_LUA_SCRIPT = ' + local pauseStateKey = KEYS[1] + + if redis.call("HEXISTS", pauseStateKey, "pausedAt") == 1 then + redis.call("HINCRBY", pauseStateKey, "suppressedReleaseCount", 1) + end + '; + #[Flow\Inject] protected RedisClientManager $redisClientManager; @@ -32,8 +45,8 @@ class AutomaticReleaseSwitchService public function isPaused(): bool { - // the "pausedAt" field, not the key itself: countSuppressedRelease() can re-create the key with only its - // counter field if a resume happens in between. + // the "pausedAt" field rather than the key itself, because that field is what records the pause - the key + // holds the counter of what the pause suppressed beside it return (bool) $this->redisClientManager->getPrimaryRedis()->hExists(self::REDIS_KEY, 'pausedAt'); } @@ -67,7 +80,11 @@ public function resume(): void public function countSuppressedRelease(): void { - $this->redisClientManager->getPrimaryRedis()->hIncrBy(self::REDIS_KEY, 'suppressedReleaseCount', 1); + $this->redisClientManager->getPrimaryRedis()->eval( + self::COUNT_SUPPRESSED_RELEASE_LUA_SCRIPT, + [self::REDIS_KEY], + 1 + ); } private function getAccountId(): ?string diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index 2831253..79a6bf8 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -256,8 +256,10 @@ A quick release only makes sense while ordinary releases are held back, so the p The state is a Redis hash on the primary instance, `contentStore:automaticReleasesPaused`, outside the per-release key space so pruning never touches it, holding `pausedAt`, `accountId` and `suppressedReleaseCount`. A hash rather than a JSON string so the counter can be raised with `HINCRBY`, without a read-modify-write race against a -concurrent publish. `isPaused()` tests the `pausedAt` field, not the key: an increment racing a resume would -otherwise re-create the key with nothing but its counter and read as paused forever. +concurrent publish. `HINCRBY` creates the hash it counts in, though, so `countSuppressedRelease()` runs it from a Lua +script guarded by `HEXISTS … pausedAt`: an increment racing a resume would otherwise leave a key behind holding +nothing but a counter, and that key is outside the space pruning cleans. `isPaused()` tests the `pausedAt` field +rather than the key for the same reason — it is that field which records the pause. `ContentReleaseManager::startIncrementalContentRelease()` is the single entry point for **all** automatic releases — workspace publish, asset change, re-render after a rendering error — so one gate there covers every trigger. diff --git a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php index cb56367..b573ac1 100644 --- a/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php +++ b/Tests/Unit/Core/AutomaticReleaseSwitchServiceTest.php @@ -24,14 +24,31 @@ final class AutomaticReleaseSwitchServiceTest extends UnitTestCase public function testTheSwitchIsSetOnlyIfThePausedAtFieldExists(): void { - // countSuppressedRelease() re-creates the key with nothing but its counter if it races a resume, so the - // existence of the key itself says nothing + // the key also holds the counter of what the pause suppressed, so it is that field which says it is set $redis = $this->createMock(Redis::class); $redis->method('hExists')->with(self::REDIS_KEY, 'pausedAt')->willReturn(false); self::assertFalse($this->buildService($redis)->isPaused()); } + public function testASuppressedReleaseIsCountedWithoutEverCreatingTheKey(): void + { + // HINCRBY would create the hash, leaving a key holding nothing but a counter behind if a resume happened + // between the caller's isPaused() check and the count - and pruning never reaches that key + $redis = $this->createMock(Redis::class); + $redis->expects(self::never())->method('hIncrBy'); + $redis + ->expects(self::once()) + ->method('eval') + ->with( + self::stringContains('HEXISTS'), + self::equalTo([self::REDIS_KEY]), + self::equalTo(1) + ); + + $this->buildService($redis)->countSuppressedRelease(); + } + public function testPausingWhileAlreadyPausedKeepsTheOriginalState(): void { // otherwise the second pause would reset both the timestamp and the count of suppressed releases From 21dd24e522ff7012074de3763a6ffa15c8342e52 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 13:15:10 +0200 Subject: [PATCH 20/23] fix bug 8 --- .../Domain/Service/DocumentNodeFilter.php | 23 ++++++--- .../Concepts/QuickContentReleases.md | 18 +++++-- .../Domain/Service/DocumentNodeFilterTest.php | 50 +++++++++++++++---- 3 files changed, 68 insertions(+), 23 deletions(-) diff --git a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php index 51cc455..927636e 100644 --- a/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php +++ b/Classes/NodeEnumeration/Domain/Service/DocumentNodeFilter.php @@ -30,6 +30,9 @@ final class DocumentNodeFilter #[Flow\InjectConfiguration('nodeRendering.nodeTypeWhitelist')] protected array $nodeTypeWhitelist; + #[Flow\InjectConfiguration('nodeRendering.recurseHiddenContent')] + protected bool $recurseHiddenContent; + /** * Builds a FlowQuery filter string from the node type whitelist, * where entries prefixed with "!" are excluded. @@ -88,7 +91,9 @@ public function skipReason(NodeInterface $node, NodeInterface $siteNode): ?strin return 'orphaned'; } - if ($node->isHidden()) { + // isVisible() rather than isHidden(), so that a page hidden by "hiddenBeforeDateTime" or + // "hiddenAfterDateTime" counts as hidden as well - it is hidden to a visitor just the same + if (!$node->isVisible()) { return 'hidden'; } @@ -109,7 +114,9 @@ public function skipReasonForNamedNode(NodeInterface $node, NodeInterface $siteN return $skipReason; } - if (self::hasHiddenAncestor($node)) { + // "recurseHiddenContent" is what the full enumeration descends into hidden pages with, so where it is set a + // document below one is part of an ordinary release and has to be part of a quick release just the same + if (!$this->recurseHiddenContent && self::hasHiddenAncestor($node)) { return 'below a hidden page'; } @@ -117,20 +124,20 @@ public function skipReasonForNamedNode(NodeInterface $node, NodeInterface $siteN } /** - * Whether one of the pages above the node is hidden. + * Whether one of the pages above the node is hidden, by its flag or by its hidden-before/after dates. * * Only asked about a node named by identifier, which is resolved in a context that shows hidden nodes so that * "it is hidden" can be reported instead of "it does not exist". The full enumeration needs no such check: it - * descends from the site node in a context which hides them, and therefore never reaches a document below a - * hidden page - publishing one from a quick release would put a page live which the next full release removes - * again. The walk goes past the site node, because a hidden site node keeps its whole site out of a full release - * just as well. + * descends from the site node in a context built from "recurseHiddenContent", and therefore never reaches a + * document below a hidden page unless that setting says it should - publishing one from a quick release would + * put a page live which the next full release removes again. The walk goes past the site node, because a hidden + * site node keeps its whole site out of a full release just as well. */ private static function hasHiddenAncestor(NodeInterface $node): bool { $parentNode = self::getParentNodeOrNull($node); while ($parentNode !== null) { - if ($parentNode->isHidden()) { + if (!$parentNode->isVisible()) { return true; } $parentNode = self::getParentNodeOrNull($parentNode); diff --git a/Documentation/Concepts/QuickContentReleases.md b/Documentation/Concepts/QuickContentReleases.md index 79a6bf8..5102aed 100644 --- a/Documentation/Concepts/QuickContentReleases.md +++ b/Documentation/Concepts/QuickContentReleases.md @@ -207,11 +207,19 @@ the configured behaviour for the full enumeration. Hidden pages are still not pu "hidden", on the confirmation page and in the pipeline log alike. Because of that override, `skipReasonForNamedNode()` also has to walk the pages **above** the node, which -`skipReason()` does not: the full enumeration descends from the site node in a context which hides them, so a -document below a hidden page is not in the content store at all. Naming it in a quick release would resolve, render -and publish it — a page live until the next full release quietly removes it again. The skip reason for that is -"below a hidden page". It is the node's own `hidden` flag which is checked, so a page hidden by -`hiddenBeforeDateTime` / `hiddenAfterDateTime` is not covered on either level. +`skipReason()` does not: with `recurseHiddenContent` at its default the full enumeration descends from the site node +in a context which hides them, so a document below a hidden page is not in the content store at all. Naming it in a +quick release would resolve, render and publish it — a page live until the next full release quietly removes it +again. The skip reason for that is "below a hidden page". + +That walk is therefore gated on the very setting which decides the traversal: where `recurseHiddenContent` is `true` +the full enumeration does reach such a document, and skipping it in a quick release would be the same mismatch the +other way round. The hidden page itself stays unpublished either way — that is `skipReason()`, which the setting does +not touch. + +Both levels ask `isVisible()` rather than `isHidden()`, so a page hidden by `hiddenBeforeDateTime` / +`hiddenAfterDateTime` counts as hidden as well: it is hidden to a visitor just the same, and the context of the full +enumeration filters it on exactly that measure. ## 6. Validation scoped to the changed URLs diff --git a/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php b/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php index f78989a..5225144 100644 --- a/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php +++ b/Tests/Unit/NodeEnumeration/Domain/Service/DocumentNodeFilterTest.php @@ -113,9 +113,9 @@ public function testADocumentBelowAHiddenPageIsNotPublished(): void { // the full enumeration descends in a context which hides them and never reaches such a document, so a quick // release publishing it would put a page live which the next full release removes again - $siteNode = $this->nodeMock(false, null); - $hiddenParentNode = $this->nodeMock(true, $siteNode); - $node = $this->nodeMock(false, $hiddenParentNode); + $siteNode = $this->nodeMock(true, null); + $hiddenParentNode = $this->nodeMock(false, $siteNode); + $node = $this->nodeMock(true, $hiddenParentNode); self::assertSame( 'below a hidden page', @@ -125,21 +125,50 @@ public function testADocumentBelowAHiddenPageIsNotPublished(): void public function testADocumentBelowVisiblePagesIsPublished(): void { - $siteNode = $this->nodeMock(false, null); - $parentNode = $this->nodeMock(false, $siteNode); - $node = $this->nodeMock(false, $parentNode); + $siteNode = $this->nodeMock(true, null); + $parentNode = $this->nodeMock(true, $siteNode); + $node = $this->nodeMock(true, $parentNode); self::assertNull($this->buildDocumentNodeFilter()->skipReasonForNamedNode($node, $siteNode)); } - private function buildDocumentNodeFilter(): DocumentNodeFilter + public function testAPageHiddenByItsDatesRatherThanByItsFlagHidesWhatIsBelowItToo(): void + { + // a page whose "hiddenBeforeDateTime" / "hiddenAfterDateTime" hides it has its "hidden" flag unset, which is + // what isVisible() adds over isHidden() - the node data behind those dates is the node class' own business + $siteNode = $this->nodeMock(true, null); + $parentNode = $this->nodeMock(false, $siteNode); + $parentNode->method('isHidden')->willReturn(false); + $node = $this->nodeMock(true, $parentNode); + + self::assertSame( + 'below a hidden page', + $this->buildDocumentNodeFilter()->skipReasonForNamedNode($node, $siteNode) + ); + } + + public function testADocumentBelowAHiddenPageIsPublishedWhereTheFullEnumerationRecursesIntoOne(): void + { + // with "recurseHiddenContent" the full enumeration reaches such a document, so skipping it in a quick + // release would be the same mismatch the other way round + $siteNode = $this->nodeMock(true, null); + $hiddenParentNode = $this->nodeMock(false, $siteNode); + $node = $this->nodeMock(true, $hiddenParentNode); + + self::assertNull($this->buildDocumentNodeFilter(true)->skipReasonForNamedNode($node, $siteNode)); + } + + private function buildDocumentNodeFilter(bool $recurseHiddenContent = false): DocumentNodeFilter { $documentNodeFilter = new DocumentNodeFilter(); - // the setting arrives by InjectConfiguration, which no container assembles in a unit test + // the settings arrive by InjectConfiguration, which no container assembles in a unit test $nodeTypeWhitelist = new \ReflectionProperty(DocumentNodeFilter::class, 'nodeTypeWhitelist'); $nodeTypeWhitelist->setValue($documentNodeFilter, ['Neos.Neos:Document']); + $recurseHiddenContentProperty = new \ReflectionProperty(DocumentNodeFilter::class, 'recurseHiddenContent'); + $recurseHiddenContentProperty->setValue($documentNodeFilter, $recurseHiddenContent); + return $documentNodeFilter; } @@ -147,16 +176,17 @@ private function buildDocumentNodeFilter(): DocumentNodeFilter * The content repository's node class is mocked rather than one of the two interfaces, because the walk up needs * TraversableNodeInterface while everything else is NodeInterface, and that class is what implements both. * + * @param bool $visible what isVisible() answers - the flag and the hidden-before/after dates together * @param Node|null $parentNode NULL where the walk up leaves the tree, as it does above the root node * @return Node&MockObject */ - private function nodeMock(bool $hidden, ?Node $parentNode): Node + private function nodeMock(bool $visible, ?Node $parentNode): Node { $nodeType = $this->createMock(NodeType::class); $nodeType->method('isOfType')->willReturn(true); $node = $this->createMock(Node::class); - $node->method('isHidden')->willReturn($hidden); + $node->method('isVisible')->willReturn($visible); $node->method('getNodeType')->willReturn($nodeType); if ($parentNode === null) { From fe8340dd0390758ca40f673a26cfdfe12a461a7e Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 15:20:25 +0200 Subject: [PATCH 21/23] remove not needed aspect --- ...inkHandlingInContentCacheFlusherAspect.php | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 Classes/Aspects/FixedNodeLinkHandlingInContentCacheFlusherAspect.php diff --git a/Classes/Aspects/FixedNodeLinkHandlingInContentCacheFlusherAspect.php b/Classes/Aspects/FixedNodeLinkHandlingInContentCacheFlusherAspect.php deleted file mode 100644 index c9fb9d6..0000000 --- a/Classes/Aspects/FixedNodeLinkHandlingInContentCacheFlusherAspect.php +++ /dev/null @@ -1,47 +0,0 @@ -registerNodeChange())") - */ - public function registerNodeChange(JoinPointInterface $joinPoint) - { - $node = $joinPoint->getMethodArgument('node'); - $tagName = 'NodeDynamicTag_' . $node->getIdentifier(); - $contentCacheFlusher = $joinPoint->getProxy(); - $tagsToFlush = ObjectAccess::getProperty($contentCacheFlusher, 'tagsToFlush', true); - $tagsToFlush[$tagName] = sprintf( - 'which were tagged with "%s" because node "%s" has changed.', - $tagName, - $node->getIdentifier() - ); - ObjectAccess::setProperty($contentCacheFlusher, 'tagsToFlush', $tagsToFlush, true); - } -} From 2160f634f487ee9fb74bdc049c535bffd87fa445 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 15:20:33 +0200 Subject: [PATCH 22/23] fix bug --- .../Extensibility/NodeRenderingExtensionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/NodeRendering/Extensibility/NodeRenderingExtensionManager.php b/Classes/NodeRendering/Extensibility/NodeRenderingExtensionManager.php index 690a5df..e5043d6 100644 --- a/Classes/NodeRendering/Extensibility/NodeRenderingExtensionManager.php +++ b/Classes/NodeRendering/Extensibility/NodeRenderingExtensionManager.php @@ -94,7 +94,7 @@ public function renderDocumentNodeVariant( protected function rendererFor(EnumeratedNode $enumeratedNode): DocumentRendererInterface { - if (!isset($this->documentEnumerators)) { + if (!isset($this->documentRenderers)) { $this->documentRenderers = self::instantiateExtensions( $this->configuredDocumentRenderers, DocumentRendererInterface::class, From fde8ebd0fd73586aaad31b57f9c1c269d4366e19 Mon Sep 17 00:00:00 2001 From: Timon Heuser Date: Tue, 18 Aug 2026 15:20:52 +0200 Subject: [PATCH 23/23] phpstan baseline update --- phpstan-baseline.neon | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 0484788..9861488 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -114,12 +114,6 @@ parameters: count: 1 path: Classes/Aspects/FixedAssetHandlingInContentCacheFlusherAspect.php - - - message: '#^Method Flowpack\\DecoupledContentStore\\Aspects\\FixedNodeLinkHandlingInContentCacheFlusherAspect\:\:registerNodeChange\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: Classes/Aspects/FixedNodeLinkHandlingInContentCacheFlusherAspect.php - - message: '#^Method Flowpack\\DecoupledContentStore\\BackendUi\\BackendUiDataService\:\:loadBackendOverviewData\(\) has no return type specified\.$#' identifier: missingType.return @@ -1029,7 +1023,13 @@ parameters: - message: '#^Property Flowpack\\DecoupledContentStore\\NodeRendering\\Extensibility\\NodeRenderingExtensionManager\:\:\$documentEnumerators \(array\\) in isset\(\) is not nullable\.$#' identifier: isset.property - count: 2 + count: 1 + path: Classes/NodeRendering/Extensibility/NodeRenderingExtensionManager.php + + - + message: '#^Property Flowpack\\DecoupledContentStore\\NodeRendering\\Extensibility\\NodeRenderingExtensionManager\:\:\$documentRenderers \(array\\) in isset\(\) is not nullable\.$#' + identifier: isset.property + count: 1 path: Classes/NodeRendering/Extensibility/NodeRenderingExtensionManager.php -