BUGFIX: url comparison fails for quick release after a full release

Classes/Command/ContentReleaseValidationCommandController.php

@@ -8,7 +8,6 @@
 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;
@@ -32,12 +31,6 @@ class ContentReleaseValidationCommandController extends CommandController
      */
     protected $redisReleaseSwitchService;
 
-    /**
-     * @Flow\Inject
-     * @var RedisEnumerationRepository
-     */
-    protected $redisEnumerationRepository;
-
     #[Flow\Inject]
     protected ContentReleaseScope $contentReleaseScope;
 
@@ -68,8 +61,12 @@ public function validateCommand(string $contentReleaseIdentifier)
         }
         $logger->info('Previous Content Release: ' . $currentlyLiveReleaseIdentifier->getIdentifier());
 
-        $currentUrlsCount = $this->countUrls($currentlyLiveReleaseIdentifier, $logger, 'Currently live release');
-        $newUrlsCount = $this->countUrls($contentReleaseIdentifier, $logger, 'Content release');
+        // Both sides are counted by the URLs they publish, never by their enumeration: the enumeration holds one
+        // entry per document *and renderer*, so with a second configured document renderer it is a multiple of the
+        // URL count - and the enumeration of a quick release covers only the documents it re-rendered, while it
+        // publishes everything it copied along. `meta:urls` is the one measure which means the same on both sides.
+        $currentUrlsCount = $this->contentReleaseScope->countPublishedUrls($currentlyLiveReleaseIdentifier);
+        $newUrlsCount = $this->contentReleaseScope->countPublishedUrls($contentReleaseIdentifier);
         $minimumUrlsCount = (int) ceil($this->validReleaseUrlCountThreshold * $currentUrlsCount);
 
         $logger->info('Previous URL Count: ' . $currentUrlsCount);
@@ -112,29 +109,6 @@ 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

Classes/QuickPublish/ContentReleaseScope.php

@@ -70,8 +70,9 @@ public function setChangedUrls(ContentReleaseIdentifier $contentReleaseIdentifie
     /**
      * 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.
+     * This is what makes two releases comparable in size, and the only measure which does: every release - copied or
+     * rendered - carries the full list of URLs it publishes, while the enumeration of a quick release covers only
+     * what it re-rendered, and the enumeration of any release counts documents per renderer rather than URLs.
      */
     public function countPublishedUrls(ContentReleaseIdentifier $contentReleaseIdentifier): int
     {

Documentation/Concepts/QuickContentReleases.md

@@ -247,15 +247,21 @@ 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 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.
+This is a trap rather than an optimisation. The validator compared the enumeration count of the new release against
+the live one and aborted below 70%. A quick release deliberately enumerates a handful of documents instead of all of
+them, so as the new release it would fail that 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.
+
+It therefore counts what a release *publishes* — the `meta:urls` cardinality — on **both** sides. Measuring each
+release on its own terms, the enumeration for a full one and the published URLs for a quick one, is not enough: the
+two are different units. The enumeration holds one entry per document **and renderer**, so an installation with a
+second document renderer (Louis renders every page as HTML and as headless JSON) enumerates twice as many entries as
+it has URLs. A quick release compared against a full one that way reports 50% and is refused, and a full release
+compared against a quick one reports 200% and is waved through whatever it lost — the check disabled in the direction
+where it matters.
 
 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.
+not always the good one — see above for the direction in which the comparison silently passes.
 
 ## 7. The pause switch
 

README.md

@@ -447,12 +447,16 @@ if ($changedUrls === null) {
 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. 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 package's own `contentReleaseValidation:validate` had to be adapted as well: it aborts a release below 70% of the
+size of the live one, while a quick release deliberately enumerates a handful of documents instead of all of them. As
+the new release its enumeration would fail that 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.
+
+It therefore counts the URLs a release *publishes* (`ContentReleaseScope::countPublishedUrls()`, the `meta:urls`
+cardinality) on **both** sides, which after a copy-forward equals the release the quick one was built on. Do the same
+in a size check of your own, and do not mix the two measures: the enumeration holds one entry per document **and
+renderer**, so with a second document renderer configured it is a multiple of the URL count, and comparing one against
+the other refuses every quick release while letting a full release which lost half the site pass.
 
 ### The commands
 

Tests/Behavior/Features/Bootstrap/FeatureContext.php

@@ -16,6 +16,7 @@
 use Flowpack\DecoupledContentStore\Core\RedisKeyService;
 use Flowpack\DecoupledContentStore\Exception as DecoupledContentStoreException;
 use Flowpack\DecoupledContentStore\IncrementalContentReleaseHandler;
+use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Dto\EnumeratedNode;
 use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Repository\RedisEnumerationRepository;
 use Flowpack\DecoupledContentStore\NodeEnumeration\Domain\Service\NodeContextCombinator;
 use Flowpack\DecoupledContentStore\NodeEnumeration\NodeEnumerator;
@@ -409,6 +410,33 @@ public function theEnumerationContainsNode($contentReleaseIdentifier, $expectedC
         Assert::assertCount((int) $expectedCount, $enumerationAsArray);
     }
 
+    /**
+     * The enumeration is written per document *and renderer*, so a second document renderer doubles it while the
+     * release still publishes one URL per document. That is the one setup in which the length of the enumeration and
+     * the number of published URLs differ for an ordinary release, and rendering a page twice - as HTML and as JSON,
+     * say - is a common enough reason to configure one.
+     *
+     * @Given the enumeration of content release :contentReleaseIdentifier is duplicated for a second document renderer
+     */
+    public function theEnumerationIsDuplicatedForASecondDocumentRenderer($contentReleaseIdentifier)
+    {
+        $contentReleaseIdentifier = ContentReleaseIdentifier::fromString($contentReleaseIdentifier);
+        $redisEnumerationRepository = $this->getObjectManager()->get(RedisEnumerationRepository::class);
+
+        $enumerationOfSecondRenderer = [];
+        foreach ($redisEnumerationRepository->findAll($contentReleaseIdentifier) as $enumeratedNode) {
+            $enumerationOfSecondRenderer[] = EnumeratedNode::fromJsonString((string) json_encode(array_merge(
+                $enumeratedNode->jsonSerialize(),
+                ['rendererId' => 'secondRenderer'],
+            )));
+        }
+
+        $redisEnumerationRepository->addDocumentNodesToEnumeration(
+            $contentReleaseIdentifier,
+            ...$enumerationOfSecondRenderer,
+        );
+    }
+
     /**
      * @When I run the render-orchestrator control loop once for content release :contentReleaseIdentifier
      */

Tests/Behavior/Features/ContentStore/QuickRelease.feature

@@ -66,13 +66,24 @@ Feature: Quick Release
 
   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
+    # smaller than that by design - both sides are therefore measured by the URLs they publish
     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 quick release is not rejected for the size of a release rendered by two renderers
+    # the enumeration counts documents per renderer, the released URLs count documents - so the two measures must not
+    # be compared against each other. A live release enumerated by two renderers is twice its own URL count, which
+    # puts the threshold above what any quick release publishes, and it refuses one which holds the whole site.
+    Given the currently live content release is "5"
+    And the enumeration of content release "5" is duplicated for a second document renderer
+    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 "6"