wintercms / wn-blocks-plugin

Block based content management plugin for Winter CMS.
MIT License
21 stars 5 forks source link

[FIX] Added fix for new files not being detected automatically #37

Closed jaxwilko closed 3 weeks ago

jaxwilko commented 2 months ago

This PR adds a fix to ensure that all blocks are considered when use generating the cache key.

The issue was caused by BlocksDatasource::getPathsCacheKey() not producing a cache key that included all file paths as some paths were being cached by said key. The Block CmsCompoundObject was able to detect the new files that were added but was unable to report them correctly as the hash used for caching was not being updated when the available paths were changed.

To fix this, during the BlocksDatasource init, I've added a check to ensure that both registered blocks and blocks detected by the datasource are correctly considered when generating the cache key.

This should resolve any issues of files not being available when they are available (i.e. adding a new file without clearing the cache).

jaxwilko commented 2 months ago

Really, we should only do this check when not running in production mode, if in production we should cache this list and only fetch it when it's missing (i.e. fresh deploy / cache busted)

jaxwilko commented 2 months ago
diff --git a/classes/BlocksDatasource.php b/classes/BlocksDatasource.php
index b1ea356..9440312 100644
--- a/classes/BlocksDatasource.php
+++ b/classes/BlocksDatasource.php
@@ -2,11 +2,18 @@

 namespace Winter\Blocks\Classes;

+use Illuminate\Support\Facades\Cache;
 use Winter\Storm\Exception\SystemException;
 use Winter\Storm\Halcyon\Datasource\Datasource;
+use Winter\Storm\Support\Facades\Config;

 class BlocksDatasource extends Datasource
 {
+    /**
+     * @var string Key used to cache the available block paths
+     */
+    protected string $registrationCacheKey = 'halcyon-datastore-blocks-registered-blocks';
+
     /**
      * @var array [key => path] List of blocks managed by the BlockManager
      */
@@ -15,14 +22,9 @@ class BlocksDatasource extends Datasource
     public function __construct()
     {
         $this->processor = new BlockProcessor();
-        $this->blocks = array_merge(
-            // Get blocks registered via plugins
-            BlockManager::instance()->getRegisteredBlocks(),
-            // Get blocks existing in the autodatasource
-            BlockManager::instance()->getBlocks()->map(function ($block) {
-                return ['name' => $block->name, 'path' => $block->getFilePath()];
-            })->pluck('path', 'name')->toArray()
-        );
+        $this->blocks = Config::get('app.env') === 'production'
+            ? Cache::rememberForever($this->registrationCacheKey, fn () => $this->getRegisteredBlocks())
+            : $this->getRegisteredBlocks();
     }

     /**
@@ -148,4 +150,16 @@ class BlocksDatasource extends Datasource
         }
         return $paths;
     }
+
+    public function getRegisteredBlocks(): array
+    {
+        return array_merge(
+            // Get blocks registered via plugins
+            BlockManager::instance()->getRegisteredBlocks(),
+            // Get blocks existing in the autodatasource
+            BlockManager::instance()->getBlocks()->map(function ($block) {
+                return ['name' => $block->name, 'path' => $block->getFilePath()];
+            })->pluck('path', 'name')->toArray()
+        );
+    }
 }
jaxwilko commented 3 weeks ago

@LukeTowers what else is required for this to be merged? Just wasted 5 minutes debugging because I thought it was already merged :joy: :cry: