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

move file cache to generated/metadata #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fsw
Copy link
Collaborator

@fsw fsw commented Nov 24, 2022

Move file cache to generated/metadata to fix issues in magento cloud and use standard location

note: tests are required before merging this.

move file cache to generated/metadata to fix issue sin magento cloud and use standard location
@igorwulff
Copy link

igorwulff commented Nov 25, 2022

This is not only an issue with Magento Cloud, but also with the --keep-generated parameter for bin/magento setup:upgrade. I think for now, we should also just change the clean() method in vendor/creatuity/magento2-interceptors/Generator/FileCache.php and let it return true or false or base it if we are on production mode or not:

    public function clean($mode = \Zend_Cache::CLEANING_MODE_ALL, array $tags = [])
    {
        if ($this->cachePath) {
            foreach (glob($this->cachePath . '/*') as $file) {
                if (is_file($file)) {
                    unlink($file);
                }
            }
            return true;
        }
        return false;
    }

Unless we want these generated plugin files to be removed? But then I fear they might not get regenerated when in production mode, so after the usage of bin/magento setup:upgrade --keep-generated, which we also use in our deploy scripts next to a regular bin/magento setup:upgrade before that.

In order to check for PROD mode or not, you probably need to change the following?

Index: vendor/creatuity/magento2-interceptors/Generator/CompiledPluginList.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vendor/creatuity/magento2-interceptors/Generator/CompiledPluginList.php b/vendor/creatuity/magento2-interceptors/Generator/CompiledPluginList.php
--- a/vendor/creatuity/magento2-interceptors/Generator/CompiledPluginList.php	
+++ b/vendor/creatuity/magento2-interceptors/Generator/CompiledPluginList.php	(date 1669382668937)
@@ -9,6 +9,7 @@
 use Magento\Framework\Interception\ObjectManager\ConfigInterface;
 use Magento\Framework\Interception\PluginListGenerator;
 use Magento\Framework\ObjectManager\Config\Reader\Dom;
+use Magento\Framework\App\State;
 use Magento\Framework\ObjectManager\Relations\Runtime as ObjectManagerRelationsRuntime;
 use Magento\Framework\Interception\Definition\Runtime as InterceptionDefinitionRuntime;
 use Magento\Framework\ObjectManager\Definition\Runtime as ObjectManagerDefinitionRuntime;
@@ -22,6 +23,7 @@
      * CompiledPluginList constructor.
      * @param ObjectManager $objectManager
      * @param ScopeInterface $scope
+     * @param State $appState
      * @param null|ReaderInterface $reader
      * @param null|ConfigInterface $omConfig
      * @param null|string $cachePath
@@ -29,10 +31,15 @@
     public function __construct(
         ObjectManager $objectManager,
         ScopeInterface $scope,
+        State $appState = null,
         ReaderInterface $reader = null,
         ConfigInterface $omConfig = null,
         $cachePath = null
     ) {
+        if (!$appState) {
+            $appState = $objectManager->get(State::class);
+        }
+
         if (!$reader || !$omConfig) {
             $reader = $objectManager->get(Dom::class);
             $omConfig = $objectManager->get(ConfigInterface::class);
@@ -40,7 +47,7 @@
         parent::__construct(
             $reader,
             $scope,
-            new FileCache($cachePath),
+            new FileCache($appState, $cachePath),
             new ObjectManagerRelationsRuntime(),
             $omConfig,
             new InterceptionDefinitionRuntime(),

and

Index: vendor/creatuity/magento2-interceptors/Generator/FileCache.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/vendor/creatuity/magento2-interceptors/Generator/FileCache.php b/vendor/creatuity/magento2-interceptors/Generator/FileCache.php
--- a/vendor/creatuity/magento2-interceptors/Generator/FileCache.php	
+++ b/vendor/creatuity/magento2-interceptors/Generator/FileCache.php	(date 1669382086131)
@@ -7,6 +7,7 @@
 
 use Magento\Framework\App\Filesystem\DirectoryList;
 use Magento\Framework\Config\CacheInterface;
+use Magento\Framework\App\State;
 
 /**
  * Class FileCache
@@ -16,19 +17,26 @@
 
     private $cachePath;
 
+    private State $appState;
+
     /**
      * FileCache constructor.
+     * @param State $appState
      * @param null|string $cachePath
      */
-    public function __construct($cachePath = null)
-    {
+    public function __construct(
+        State $appState,
+        $cachePath = null
+    ) {
         if ($cachePath === null) {
             $this->cachePath = BP . DIRECTORY_SEPARATOR .
                 DirectoryList::GENERATED . DIRECTORY_SEPARATOR .
-                'staticcache';
+                'metadata';
         } else {
             $this->cachePath = $cachePath;
         }
+
+        $this->appState = $appState;
     }
 
     /**
@@ -115,7 +123,9 @@
      */
     public function clean($mode = \Zend_Cache::CLEANING_MODE_ALL, array $tags = [])
     {
-        if ($this->cachePath) {
+        if ($this->cachePath
+            && $this->appState->getMode() !== State::MODE_PRODUCTION
+        ) {
             foreach (glob($this->cachePath . '/*') as $file) {
                 if (is_file($file)) {
                     unlink($file);


I will do some tests with this.

@norgeindian
Copy link

@igorwulff , as we're facing the same issue: Were you able to test this and find a good solution?

@igorwulff
Copy link

Hi @norgeindian I haven't tested this in production unfortunately. I still think it is an good way to approach the problem.

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.

3 participants