Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 25, 2024

Conversation

jaxwilko
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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 jaxwilko self-assigned this Oct 4, 2024
@jaxwilko
Copy link
Member Author

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

@LukeTowers LukeTowers merged commit 9c91169 into main Oct 25, 2024
6 checks passed
@LukeTowers LukeTowers deleted the fix/new-file-caching branch October 25, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants