Bugfix: Set reading mode when reading/writing CacheKeys (#65)

Author: chrispenny

src/Extensions/CacheKeyExtension.php

@@ -40,6 +40,7 @@ public function updateCMSFields(FieldList $fields): void
             return;
         }
 
+        // The configuration for this DataObject has specified that it does not use CacheKeys
         if (!$this->owner->config()->get('has_cache_key')) {
             return;
         }
@@ -81,6 +82,7 @@ public function onAfterDelete(): void
         // We will want to publish changes to the CacheKey onAfterWrite if the instance triggering this event is *not*
         // Versioned (the changes should be seen immediately even though the object wasn't Published)
         $publishUpdates = !$this->owner->hasExtension(Versioned::class);
+        // Note: doArchive will call deleteFromStage() which will in turn trigger this extension hook
         $this->owner->triggerCacheEvent($publishUpdates);
         CacheKey::remove($this->owner);
     }
@@ -90,8 +92,17 @@ public function onAfterPublish(): void
         $this->owner->triggerCacheEvent(true);
     }
 
+    public function onAfterPublishRecursive(): void
+    {
+        // This can sometimes be called in the same request as onAfterPublish(), but the duplication of effort is
+        // minimal since we keep track of which records have been processed
+        $this->owner->triggerCacheEvent(true);
+    }
+
     public function onAfterUnpublish(): void
     {
+        // Note: doArchive() will call doUnpublish(), so this extension hook is called when published records are
+        // archived
         $this->owner->triggerCacheEvent(true);
     }
 
@@ -117,47 +128,68 @@ private function findCacheKeyHash(): ?string
             return null;
         }
 
-        $hasCacheKey = $this->owner->config()->get('has_cache_key');
-
-        // You have requested that this DataObject class does not use cache keys
-        if (!$hasCacheKey) {
+        // The configuration for this DataObject has specified that it does not use CacheKeys
+        if (!$this->owner->config()->get('has_cache_key')) {
             return null;
         }
 
-        // Find an existing CacheKey, or create a new one for this DataObject
+        // First we'll try to find this CacheKey in whatever your current Stage is (IE: We'll fetch LIVE records when
+        // you're in the LIVE reading_mode, and DRAFT records when you're in the DRAFT reading_mode)
+        $cacheKey = CacheKey::findInStage($this->owner);
+
+        // A key exists in your active Stage, so we can return it immediately
+        if ($cacheKey) {
+            return $cacheKey->KeyHash;
+        }
+
+        // We know that a CacheKey did not exist in your active Stage (that could have been DRAFT or LIVE). We'll now
+        // attempt to find an existing CacheKey (specifically in DRAFT), or we'll create a new one
+
+        // Given this context, our goal is now to make sure that we have a CacheKey for this record in the appropriate
+        // Stage. EG: If you are browsing this record in LIVE, then we'd expect to have a published CacheKey
         $cacheKey = CacheKey::findOrCreate($this->owner);
 
-        // In this context (that being, in a time where we are not actively generating Cache Keys, and are instead just
-        // trying to find them) we will not perform a write() when/if the StagingState indicates that we should not
+        // Safety first, but there shouldn't really have been a reason for this to be null
+        if (!$cacheKey) {
+            return null;
+        }
+
+        // In this context (that being, in a time when we are not actively generating Cache Keys, and are instead just
+        // trying to find them) we will not write() when/if the StagingState indicates that we should not
+        // One example is when browsing CMS Previews. We do not save CacheKeys in that context
         if (!StagingState::singleton()->canWrite()) {
             return $cacheKey->KeyHash;
         }
 
-        // Check that our CacheKey has been saved to the Database
+        // Make sure that our CacheKey is saved to the Database
         if (!$cacheKey->isInDB()) {
-            $cacheKey->write();
+            // We need to make sure that we are specifically writing this with reading mode set to DRAFT. If we write()
+            // while a user is browsing in a LIVE reading mode, then this CacheKey will be "live" immediately
+            // @see https://github.com/silverstripe/silverstripe-versioned/issues/382
+            Versioned::withVersionedMode(static function () use ($cacheKey): void {
+                Versioned::set_stage(Versioned::DRAFT);
+
+                $cacheKey->write();
+            });
         }
 
-        // The Cache Key is already published, so there is nothing left for us to do except return the KeyHash
-        if ($cacheKey->isPublished()) {
+        // In this context we will not publish() when/if the StagingState indicates that we should not
+        // Generally, any time we're in a DRAFT reading_mode, we will not publish
+        if (!StagingState::singleton()->canPublish()) {
             return $cacheKey->KeyHash;
         }
 
-        // In this context we will not perform a publish() when/if the StagingState indicates that we should not
-        if (!StagingState::singleton()->canPublish()) {
+        // The Cache Key is already published, so there is nothing left for us to do except return the KeyHash
+        if ($cacheKey->isPublished()) {
             return $cacheKey->KeyHash;
         }
 
-        // If the owner is not Versioned (essentially meaning that it is *always* published), or if the owner is
-        // currently published, then we want to make sure we publish our CacheKey as well
-        if (!$this->owner->hasExtension(Versioned::class) || $this->owner->isPublished()) {
-            // Default behaviour is that publish_recursive is disabled. There is only value in using publishRecursive()
-            // if you decide that your CacheKey model needs to $own something
-            if (CacheKey::config()->get('publish_recursive')) {
-                $cacheKey->publishRecursive();
-            } else {
-                $cacheKey->publishSingle();
-            }
+        // Default behaviour is that publish_recursive is disabled. There is only value in using publishRecursive()
+        // if you decide that your CacheKey model needs to $own something
+        if (CacheKey::config()->get('publish_recursive')) {
+            $cacheKey->publishRecursive();
+        } else {
+            $cacheKey->publishSingle();
         }
 
         return $cacheKey->KeyHash;

src/Extensions/StagingExtension.php

@@ -4,7 +4,6 @@
 
 use SilverStripe\CMS\Controllers\ContentController;
 use SilverStripe\CMS\Model\SiteTreeExtension;
-use SilverStripe\Versioned\Versioned;
 use Terraformers\KeysForCache\State\StagingState;
 
 /**
@@ -15,16 +14,9 @@ class StagingExtension extends SiteTreeExtension
     public function contentcontrollerInit(ContentController $controller): void
     {
         // If we are currently browsing in a CMSPreview, then we do not want to write or publish any CacheKeys
-        if ($controller->getRequest()->getVar('CMSPreview')) {
+        if ($controller->getRequest()->getVar('CMSPreview')) { // phpcs:ignore
             StagingState::singleton()->disableWrite();
             StagingState::singleton()->disablePublish();
-
-            return;
-        }
-
-        // If we are browsing in stage=Stage, then we do not want to publish any CacheKeys
-        if ($controller->getRequest()->getVar('stage') === Versioned::DRAFT) { // phpcs:ignore
-            StagingState::singleton()->disablePublish();
         }
     }
 }

src/Models/CacheKey.php

@@ -51,16 +51,29 @@ public static function updateOrCreateKey(DataObject $dataObject): ?CacheKey
         return $cacheKey;
     }
 
+    public static function findInStage(DataObject $dataObject): ?CacheKey
+    {
+        // The configuration for this DataObject has specified that it does not use CacheKeys
+        if (!$dataObject->config()->get('has_cache_key')) {
+            return null;
+        }
+
+        // This search will be performed in whatever your current Stage is
+        return static::get()->filter([
+            'RecordClass' => $dataObject->ClassName,
+            'RecordID' => $dataObject->ID,
+        ])->first();
+    }
+
     /**
      * @param DataObject|CacheKeyExtension $dataObject
      * @return CacheKey|null
      * @throws ValidationException
      */
     public static function findOrCreate(DataObject $dataObject): ?CacheKey
     {
-        $hasCacheKey = $dataObject->config()->get('has_cache_key');
-
-        if (!$hasCacheKey) {
+        // The configuration for this DataObject has specified that it does not use CacheKeys
+        if (!$dataObject->config()->get('has_cache_key')) {
             return null;
         }
 

src/Services/CacheProcessingService.php

@@ -114,8 +114,14 @@ private function updateInstance(DataObject $instance): array
             return $this->createEdges($instance);
         }
 
-        // Make sure that our CacheKey record is saved
-        $cacheKey->write();
+        // We need to make sure that we are specifically writing this with reading mode set to DRAFT. If we write()
+        // while a user is browsing in a LIVE reading mode, then this CacheKey will be "live" immediately
+        // @see https://github.com/silverstripe/silverstripe-versioned/issues/382
+        Versioned::withVersionedMode(static function () use ($cacheKey): void {
+            Versioned::set_stage(Versioned::DRAFT);
+
+            $cacheKey->write();
+        });
 
         // Check to see if we need to publish this CacheKey
         if ($this->shouldPublishUpdates()) {

src/State/StagingState.php

@@ -3,6 +3,7 @@
 namespace Terraformers\KeysForCache\State;
 
 use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Versioned\Versioned;
 
 class StagingState
 {
@@ -40,7 +41,11 @@ public function disablePublish(): void
 
     public function canPublish(): bool
     {
-        return $this->publishEnabled;
+        if (!$this->publishEnabled) {
+            return false;
+        }
+
+        return Versioned::get_stage() === Versioned::LIVE;
     }
 
 }