Update const and variable names. strip_stage_param default to false

silverstripe · Aug 10, 2023 · e5d596b · e5d596b
1 parent dca9eb9
commit e5d596b
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 61 deletions.
diff --git a/src/Extension/Publishable/PublishableSiteTree.php b/src/Extension/Publishable/PublishableSiteTree.php
@@ -23,13 +23,13 @@
  */
 class PublishableSiteTree extends DataExtension implements StaticallyPublishable, StaticPublishingTrigger
 {
-    public const RELATION_INCLUDE_NONE = 'none';
-    public const RELATION_INCLUDE_DIRECT = 'direct';
-    public const RELATION_INCLUDE_RECURSIVE = 'recursive';
+    public const REGENERATE_RELATIONS_NONE = 'none';
+    public const REGENERATE_RELATIONS_DIRECT = 'direct';
+    public const REGENERATE_RELATIONS_RECURSIVE = 'recursive';
 
-    private static string $regenerate_children = self::RELATION_INCLUDE_NONE;
+    private static string $regenerate_children = self::REGENERATE_RELATIONS_NONE;
 
-    private static string $regenerate_parents = self::RELATION_INCLUDE_DIRECT;
+    private static string $regenerate_parents = self::REGENERATE_RELATIONS_DIRECT;
 
     public function getMyVirtualPages()
     {
@@ -59,30 +59,30 @@ public function objectsToUpdate($context)
 
             // For the 'publish' action, we will update children when we are configured to do so. Any config value other
             // than 'none' means that we want to regenerate children at some level
-            $childInclusion = $siteTree->config()->get('regenerate_children');
+            $regenerateChildren = SiteTree::config()->get('regenerate_children');
             // When the context of urlSegmentChanged has been provided we *must* update children - because all of their
             // URLs will have just changed
-            $forceRecursiveInclusion = $context['urlSegmentChanged'] ?? false;
+            $forceRecursiveRegeneration = $context['urlSegmentChanged'] ?? false;
 
             // We've either been configured to regenerate (some level) of children, or the above context has been set
-            if ($childInclusion !== self::RELATION_INCLUDE_NONE || $forceRecursiveInclusion) {
+            if ($regenerateChildren !== self::REGENERATE_RELATIONS_NONE || $forceRecursiveRegeneration) {
                 // We will want to recursively add all children if our regenerate_children config was set to Recursive,
-                // or if $forceRecursiveInclusion was set to true
+                // or if $forceRecursiveRegeneration was set to true
                 // If neither of those conditions are true, then we will only be adding the direct children of this
                 // parent page
-                $recursive = $childInclusion === self::RELATION_INCLUDE_RECURSIVE || $forceRecursiveInclusion;
+                $recursive = $regenerateChildren === self::REGENERATE_RELATIONS_RECURSIVE || $forceRecursiveRegeneration;
 
                 $this->addChildren($list, $siteTree, $recursive);
             }
         }
 
         // For any of our defined actions, we will update parents when configured to do so. Any config value other than
         // 'none' means that we want to include children at some level
-        $parentInclusion = $siteTree->config()->get('regenerate_parents');
+        $regenerateParents = SiteTree::config()->get('regenerate_parents');
 
-        if ($parentInclusion !== self::RELATION_INCLUDE_NONE) {
+        if ($regenerateParents !== self::REGENERATE_RELATIONS_NONE) {
             // You can also choose whether to update only the direct parent, or the entire tree
-            $recursive = $parentInclusion === self::RELATION_INCLUDE_RECURSIVE;
+            $recursive = $regenerateParents === self::REGENERATE_RELATIONS_RECURSIVE;
 
             $this->addParents($list, $siteTree, $recursive);
         }
@@ -119,20 +119,20 @@ public function objectsToDelete($context)
 
         // Check if you specifically want children regenerated in all actions. Any config value other than 'none' means
         // that we want to regenerate children at some level
-        $childInclusion = $siteTree->config()->get('regenerate_children');
+        $regenerateChildren = SiteTree::config()->get('regenerate_children');
         // Check to see if SiteTree enforces strict hierarchy (that being, parents must be published in order for
         // children to be viewed)
         // If strict hierarchy is being used, then we *must* purge all child pages recursively, as they are no longer
         // available for frontend users to view
-        $forceRecursiveInclusion = $siteTree->config()->get('enforce_strict_hierarchy');
+        $forceRecursiveRegeneration = SiteTree::config()->get('enforce_strict_hierarchy');
 
         // We've either been configured to include (some level) of children, or enforce_strict_hierarchy was true
-        if ($childInclusion !== self::RELATION_INCLUDE_NONE || $forceRecursiveInclusion) {
+        if ($regenerateChildren !== self::REGENERATE_RELATIONS_NONE || $forceRecursiveRegeneration) {
             // We will want to recursively add all children if our regenerate_children config was set to Recursive,
-            // or if $forceRecursiveInclusion was set to true
+            // or if $forceRecursiveRegeneration was set to true
             // If neither of those conditions are true, then we will only be adding the direct children of this
             // parent page
-            $recursive = $childInclusion === self::RELATION_INCLUDE_RECURSIVE || $forceRecursiveInclusion;
+            $recursive = $regenerateChildren === self::REGENERATE_RELATIONS_RECURSIVE || $forceRecursiveRegeneration;
 
             $this->addChildren($list, $siteTree, $recursive);
         }

diff --git a/src/Service/UrlBundleService.php b/src/Service/UrlBundleService.php
@@ -21,7 +21,7 @@ class UrlBundleService implements UrlBundleInterface
     use Injectable;
     use Configurable;
 
-    private static bool $strip_stage_param = true;
+    private static bool $strip_stage_param = false;
 
     protected $urls = [];
 
@@ -61,21 +61,6 @@ public function getJobsForUrls(string $jobClass, ?string $message = null, ?DataO
         return $jobs;
     }
 
-    /**
-     * Any URL that we attempt to process through static publisher should always have any stage=* param removed
-     */
-    protected function stripStageParam(string $url): string
-    {
-        // This will safely remove "stage" params, but keep any others. It doesn't matter where in the string "stage="
-        // exists
-        $url = preg_replace('/([?&])stage=[^&]+(&|$)/', '$1', $url);
-        // Trim any trailing "?" or "&".
-        $url = rtrim($url, '&');
-        $url = rtrim($url, '?');
-
-        return $url;
-    }
-
     /**
      * Get URLs for further processing
      */
@@ -107,7 +92,7 @@ protected function getUrls(): array
      */
     protected function formatUrl(string $url): ?string
     {
-        if ($this->config()->get('strip_stage_param')) {
+        if (UrlBundleService::config()->get('strip_stage_param')) {
             $url = $this->stripStageParam($url);
         }
 
@@ -132,4 +117,19 @@ protected function assignPriorityToUrls(array $urls): array
 
         return $priorityUrls;
     }
+
+    /**
+     * Any URL that we attempt to process through static publisher should always have any stage=* param removed
+     */
+    private function stripStageParam(string $url): string
+    {
+        // This will safely remove "stage" params, but keep any others. It doesn't matter where in the string "stage="
+        // exists
+        $url = preg_replace('/([?&])stage=[^&]+(&|$)/', '$1', $url);
+        // Trim any trailing "?" or "&".
+        $url = rtrim($url, '&');
+        $url = rtrim($url, '?');
+
+        return $url;
+    }
 }
diff --git a/tests/php/Extension/Engine/SiteTreePublishingEngineTest.php b/tests/php/Extension/Engine/SiteTreePublishingEngineTest.php
@@ -12,6 +12,7 @@
 use SilverStripe\StaticPublishQueue\Extension\Publishable\PublishableSiteTree;
 use SilverStripe\StaticPublishQueue\Job\DeleteStaticCacheJob;
 use SilverStripe\StaticPublishQueue\Job\GenerateStaticCacheJob;
+use SilverStripe\StaticPublishQueue\Service\UrlBundleService;
 use SilverStripe\StaticPublishQueue\Test\QueuedJobsTestService;
 use Symbiote\QueuedJobs\DataObjects\QueuedJobDescriptor;
 use Symbiote\QueuedJobs\Services\QueuedJob;
@@ -33,8 +34,8 @@ class SiteTreePublishingEngineTest extends SapphireTest
     public function testPublishRecursive(): void
     {
         // Inclusion of parent/child is tested in PublishableSiteTreeTest
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         /** @var QueuedJobsTestService $service */
         $service = QueuedJobService::singleton();
@@ -63,8 +64,8 @@ public function testPublishRecursive(): void
     public function testPublishRecursiveWhenParentIsDraft(): void
     {
         // Inclusion of parent/child is tested in PublishableSiteTreeTest
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         /** @var QueuedJobsTestService $service */
         $service = QueuedJobService::singleton();
@@ -102,8 +103,8 @@ public function testPublishRecursiveWhenParentIsDraft(): void
     public function testPublishRecursiveUrlChange(): void
     {
         // Inclusion of parent/child is tested in PublishableSiteTreeTest
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         /** @var QueuedJobsTestService $service */
         $service = Injector::inst()->get(QueuedJobService::class);
@@ -150,8 +151,8 @@ public function testPublishRecursiveUrlChange(): void
     public function testPublishRecursiveParentChange(): void
     {
         // Inclusion of parent/child is tested in PublishableSiteTreeTest
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         /** @var QueuedJobsTestService $service */
         $service = Injector::inst()->get(QueuedJobService::class);
@@ -200,9 +201,9 @@ public function testPublishRecursiveParentChange(): void
     public function testDoUnpublish(): void
     {
         // Inclusion of parent/child is tested in PublishableSiteTreeTest
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
         // Because this is an unpublish, we'll expect all the children to be present as well (regardless of config)
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         /** @var QueuedJobsTestService $service */
         $service = Injector::inst()->get(QueuedJobService::class);
@@ -258,6 +259,9 @@ protected function setUp(): void
         // Set up our base URL so that it's always consistent for our tests
         Config::modify()->set(Director::class, 'alternate_base_url', 'http://example.com/');
 
+        // Performing all tests with stage params removed
+        UrlBundleService::config()->set('strip_stage_param', true);
+
         parent::setUp();
 
         $pages = [

diff --git a/tests/php/Extension/Publishable/PublishableSiteTreeTest.php b/tests/php/Extension/Publishable/PublishableSiteTreeTest.php
@@ -34,8 +34,8 @@ protected function setUp(): void
     public function testObjectsActionPublishNoInclusion(): void
     {
         // Test that only the actioned page and its virtual page are added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $virtualPage = $this->objFromFixture(VirtualPage::class, 'page1');
@@ -65,8 +65,8 @@ public function testObjectsActionPublishNoInclusion(): void
     public function testObjectsActionPublishDirectInclusion(): void
     {
         // Check that direct only parent/child are added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_DIRECT);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_DIRECT);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_DIRECT);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_DIRECT);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $virtualPage = $this->objFromFixture(VirtualPage::class, 'page1');
@@ -100,8 +100,8 @@ public function testObjectsActionPublishDirectInclusion(): void
     public function testObjectsActionPublishRecursiveInclusion(): void
     {
         // Check that recursive parents and children are added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_RECURSIVE);
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_RECURSIVE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_RECURSIVE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_RECURSIVE);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $virtualPage = $this->objFromFixture(VirtualPage::class, 'page1');
@@ -139,9 +139,9 @@ public function testObjectsActionPublishRecursiveInclusion(): void
     public function testObjectsActionPublishUrlChangeNoInclusion(): void
     {
         // Test that only the actioned page and its virtual page are added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
         // Given the context of a URL change, we will expect all children to be added regardless of config
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $virtualPage = $this->objFromFixture(VirtualPage::class, 'page1');
@@ -176,9 +176,9 @@ public function testObjectsActionPublishUrlChangeNoInclusion(): void
     public function testObjectsActionUnpublishNoInclusion(): void
     {
         // Test that only the actioned page and its virtual page are added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
         // Regardless of configuration, we expect children to be added in objectsToDelete() on unpublish action
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $childPage = $this->objFromFixture(SiteTree::class, 'page4');
@@ -214,9 +214,9 @@ public function testObjectsActionUnpublishNoStrictHierarchy(): void
         // Disable strict hierarchy, meaning that children can remain published even when a parent is unpublished
         SiteTree::config()->set('enforce_strict_hierarchy', false);
         // Test that only the actioned page and its virtual page are added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
         // We are disabling child inclusion through our config, as well as strict hierarchy being disabled
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_NONE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_NONE);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $virtualPage = $this->objFromFixture(VirtualPage::class, 'page1');
@@ -246,9 +246,9 @@ public function testObjectsActionUnpublishNoStrictHierarchy(): void
     public function testObjectsActionUnpublishDirectInclusion(): void
     {
         // Test that direct parent is added
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_DIRECT);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_DIRECT);
         // Regardless of configuration, we expect children to be added in objectsToDelete() on unpublish action
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_DIRECT);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_DIRECT);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $childPage = $this->objFromFixture(SiteTree::class, 'page4');
@@ -290,9 +290,9 @@ public function testObjectsActionUnpublishDirectInclusion(): void
     public function testObjectsActionUnpublishRecursiveInclusion(): void
     {
         // Test that recursive parents and children
-        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::RELATION_INCLUDE_RECURSIVE);
+        SiteTree::config()->set('regenerate_parents', PublishableSiteTree::REGENERATE_RELATIONS_RECURSIVE);
         // Regardless of configuration, we expect children to be added in objectsToDelete() on unpublish action
-        SiteTree::config()->set('regenerate_children', PublishableSiteTree::RELATION_INCLUDE_RECURSIVE);
+        SiteTree::config()->set('regenerate_children', PublishableSiteTree::REGENERATE_RELATIONS_RECURSIVE);
 
         $page = $this->objFromFixture(SiteTree::class, 'page3');
         $childPage = $this->objFromFixture(SiteTree::class, 'page4');

diff --git a/tests/php/Service/UrlBundleServiceTest.php b/tests/php/Service/UrlBundleServiceTest.php
@@ -131,6 +131,8 @@ public function chunkCases(): array
 
     public function testGetUrls(): void
     {
+        UrlBundleService::config()->set('strip_stage_param', true);
+
         $urls = [
             'http://www.test.com?stage=Stage',
             'https://www.test.com?test1=1&stage=Live&test2=2'
@@ -172,6 +174,8 @@ public function testGetUrlsDontStripStage(): void
      */
     public function testStripStageParam(string $url, string $expectedUrl): void
     {
+        UrlBundleService::config()->set('strip_stage_param', true);
+
         $urlService = UrlBundleService::create();
         $method = new ReflectionMethod($urlService, 'stripStageParam');
         $method->setAccessible(true);