TASK: Cache expressions as strings

This might seem counter-intuitive at first but serves two purposes:

1) Incremental change to the expressions is possible now, before
a change in policy protected classes would usually lead to an exception
and required you to flush the full cache, this prevents the issue and
therefore makes developing much smoother.

2) Caches can be moved to redis or any other backend as it's only
strings now.

The second part also makes a further separation of EEL and Flow
possible in the future, eventually allowing EEL to be come a package
that could be used independently from Flow.

Author: kitsunet

Neos.Eel/Classes/CompilingEvaluator.php

@@ -13,6 +13,7 @@
 
 use Neos\Flow\Annotations as Flow;
 use Neos\Cache\Frontend\PhpFrontend;
+use Opis\Closure\SerializableClosure;
 
 /**
  * An evaluator that compiles expressions down to PHP code
@@ -25,50 +26,18 @@
 class CompilingEvaluator implements EelEvaluatorInterface
 {
     /**
-     * @var array
+     * Runtime cache of execution ready closures.
+     *
+     * @var \closure[]
      */
-    protected $newExpressions = [];
+    protected $evaluatedExpressions = [];
 
     /**
      * @Flow\Inject(lazy=false)
-     * @var PhpFrontend
+     * @var StringFrontend
      */
     protected $expressionCache;
 
-    /**
-     * Initialize the Evaluator
-     */
-    public function initializeObject()
-    {
-        $this->expressionCache->requireOnce('cachedExpressionClosures');
-    }
-
-    /**
-     * Shutdown the Evaluator
-     */
-    public function shutdownObject()
-    {
-        if (count($this->newExpressions) > 0) {
-            $changesToPersist = false;
-            $codeToBeCached = $this->expressionCache->get('cachedExpressionClosures');
-            /**
-             * At this point a race condition could happen, that we try to prevent with an additional check.
-             * So we compare the evaluated expressions during this request with the methods the cache has at
-             * this point and only add methods that are not present. Only if we added anything we write the cache.
-             */
-            foreach ($this->newExpressions as $functionName => $newExpression) {
-                if (strpos($codeToBeCached, $functionName) === false) {
-                    $codeToBeCached .= 'if (!function_exists(\'' . $functionName . '\')) { ' . $newExpression . ' }' . chr(10);
-                    $changesToPersist = true;
-                }
-            }
-
-            if ($changesToPersist) {
-                $this->expressionCache->set('cachedExpressionClosures', $codeToBeCached);
-            }
-        }
-    }
-
     /**
      * Evaluate an expression under a given context
      *
@@ -81,20 +50,34 @@ public function evaluate($expression, Context $context)
         $expression = trim($expression);
         $identifier = md5($expression);
         $functionName = 'expression_' . $identifier;
+        if (isset($this->evaluatedExpressions[$functionName])) {
+            return $this->evaluateAndUnwrap($this->evaluatedExpressions[$functionName], $context);
+        }
 
-        if (!function_exists($functionName)) {
-            $code = $this->generateEvaluatorCode($expression);
-            $functionDeclaration = 'function ' . $functionName . '($context){return ' . $code . ';}';
-            $this->newExpressions[$functionName] = $functionDeclaration;
-            eval($functionDeclaration);
+        $functionDeclaration = $this->expressionCache->get($functionName);
+        if (!$functionDeclaration) {
+            $functionDeclaration = $this->generateEvaluatorCode($expression);
+            $this->expressionCache->set($functionName, $functionDeclaration);
         }
 
-        $result = $functionName($context);
+        $expressionFunction = eval($functionDeclaration);
+        $this->evaluatedExpressions[$functionName] = $expressionFunction;
+        return $this->evaluateAndUnwrap($expressionFunction, $context);
+    }
+
+    /**
+     * @param \closure $expressionFunction
+     * @param Context $context
+     * @return mixed
+     */
+    protected function evaluateAndUnwrap(\closure $expressionFunction, Context $context)
+    {
+        $result = $expressionFunction($context);
         if ($result instanceof Context) {
             return $result->unwrap();
-        } else {
-            return $result;
         }
+
+        return $result;
     }
 
     /**
@@ -118,6 +101,7 @@ protected function generateEvaluatorCode($expression)
         } elseif (!array_key_exists('code', $result)) {
             throw new ParserException(sprintf('Parser error, no code in result %s ', json_encode($result)), 1334491498);
         }
-        return $result['code'];
+
+        return 'return function ($context) {return ' . $result['code'] . ';};';
     }
 }

Neos.Eel/Configuration/Caches.yaml

@@ -1,3 +1,3 @@
 Eel_Expression_Code:
-  frontend: Neos\Cache\Frontend\PhpFrontend
+  frontend: Neos\Cache\Frontend\StringFrontend
   backend: Neos\Cache\Backend\SimpleFileBackend

Neos.Flow/Classes/Aop/Pointcut/PointcutFilterComposite.php

@@ -173,14 +173,14 @@ public function getRuntimeEvaluationsClosureCode()
         $conditionCode = $this->buildRuntimeEvaluationsConditionCode('', $this->getRuntimeEvaluationsDefinition(), $useGlobalObjects);
 
         if ($conditionCode !== '') {
-            $code = "\n\t\t\t\t\t\tfunction(\\Neos\\Flow\\Aop\\JoinPointInterface \$joinPoint, \$objectManager) {\n" .
-                    "\t\t\t\t\t\t\t\$currentObject = \$joinPoint->getProxy();\n";
+            $code = "function(\\Neos\\Flow\\Aop\\JoinPointInterface \$joinPoint, \$objectManager) {\n" .
+                    "    \$currentObject = \$joinPoint->getProxy();\n";
             if ($useGlobalObjects) {
-                $code .= "\t\t\t\t\t\t\t\$globalObjectNames = \$objectManager->getSettingsByPath(array('Neos', 'Flow', 'aop', 'globalObjects'));\n";
-                $code .= "\t\t\t\t\t\t\t\$globalObjects = array_map(function(\$objectName) use (\$objectManager) { return \$objectManager->get(\$objectName); }, \$globalObjectNames);\n";
+                $code .= "    \$globalObjectNames = \$objectManager->getSettingsByPath(array('Neos', 'Flow', 'aop', 'globalObjects'));\n";
+                $code .= "    \$globalObjects = array_map(function(\$objectName) use (\$objectManager) { return \$objectManager->get(\$objectName); }, \$globalObjectNames);\n";
             }
-            $code .= "\t\t\t\t\t\t\treturn " . $conditionCode . ';' .
-                    "\n\t\t\t\t\t\t}";
+            $code .= "    return " . $conditionCode . ';' .
+                    "\n}";
             return $code;
         } else {
             return 'NULL';

Neos.Flow/Classes/Aop/Pointcut/RuntimeExpressionEvaluator.php

@@ -28,7 +28,7 @@
 class RuntimeExpressionEvaluator
 {
     /**
-     * @var PhpFrontend
+     * @var StringFrontend
      */
     protected $runtimeExpressionsCache;
 
@@ -44,13 +44,6 @@ class RuntimeExpressionEvaluator
      */
     protected $runtimeExpressions = [];
 
-    /**
-     * Newly added expressions.
-     *
-     * @var array
-     */
-    protected $newExpressions = [];
-
     /**
      * This object is created very early and is part of the blacklisted "Neos\Flow\Aop" namespace so we can't rely on AOP for the property injection.
      *
@@ -64,30 +57,9 @@ public function injectObjectManager(ObjectManagerInterface $objectManager)
             /** @var CacheManager $cacheManager */
             $cacheManager = $this->objectManager->get(CacheManager::class);
             $this->runtimeExpressionsCache = $cacheManager->getCache('Flow_Aop_RuntimeExpressions');
-            $this->runtimeExpressions = $this->runtimeExpressionsCache->requireOnce('Flow_Aop_RuntimeExpressions');
         }
     }
 
-    /**
-     * Shutdown the Evaluator and save created expressions overwriting any existing expressions
-     *
-     * @return void
-     */
-    public function saveRuntimeExpressions()
-    {
-        if ($this->newExpressions === []) {
-            return;
-        }
-
-        $codeToBeCached = 'return array (' . chr(10);
-
-        foreach ($this->newExpressions as $name => $function) {
-            $codeToBeCached .= "'" . $name . "' => " . $function . ',' . chr(10);
-        }
-        $codeToBeCached .= ');';
-        $this->runtimeExpressionsCache->set('Flow_Aop_RuntimeExpressions', $codeToBeCached);
-    }
-
     /**
      * Evaluate an expression with the given JoinPoint
      *
@@ -99,11 +71,17 @@ public function saveRuntimeExpressions()
     public function evaluate($privilegeIdentifier, JoinPointInterface $joinPoint)
     {
         $functionName = $this->generateExpressionFunctionName($privilegeIdentifier);
+        if (isset($this->runtimeExpressions[$functionName])) {
+            return $this->runtimeExpressions[$functionName]->__invoke($joinPoint, $this->objectManager);
+        }
 
-        if (!isset($this->runtimeExpressions[$functionName]) || !$this->runtimeExpressions[$functionName] instanceof \Closure) {
+        $expression = $this->runtimeExpressionsCache->get($functionName);
+
+        if (!$expression) {
             throw new Exception('Runtime expression "' . $functionName . '" does not exist. Flushing the code caches may help to solve this.', 1428694144);
         }
 
+        $this->runtimeExpressions[$functionName] = eval($expression);
         return $this->runtimeExpressions[$functionName]->__invoke($joinPoint, $this->objectManager);
     }
 
@@ -116,7 +94,10 @@ public function evaluate($privilegeIdentifier, JoinPointInterface $joinPoint)
      */
     public function addExpression($privilegeIdentifier, $expression)
     {
-        $this->newExpressions[$this->generateExpressionFunctionName($privilegeIdentifier)] = $expression;
+        $functionName = $this->generateExpressionFunctionName($privilegeIdentifier);
+        $wrappedExpression = 'return ' . $expression . ';';
+        $this->runtimeExpressionsCache->set($functionName, $wrappedExpression);
+        $this->runtimeExpressions[$functionName] = eval($wrappedExpression);
     }
 
     /**

Neos.Flow/Classes/Package.php

@@ -95,7 +95,6 @@ public function boot(Core\Bootstrap $bootstrap)
         $dispatcher->connect(Core\Bootstrap::class, 'bootstrapShuttingDown', Reflection\ReflectionService::class, 'saveToCache');
 
         $dispatcher->connect(Command\CoreCommandController::class, 'finishedCompilationRun', Security\Authorization\Privilege\Method\MethodPrivilegePointcutFilter::class, 'savePolicyCache');
-        $dispatcher->connect(Command\CoreCommandController::class, 'finishedCompilationRun', Aop\Pointcut\RuntimeExpressionEvaluator::class, 'saveRuntimeExpressions');
 
         $dispatcher->connect(Security\Authentication\AuthenticationProviderManager::class, 'authenticatedToken', function () use ($bootstrap) {
             $session = $bootstrap->getObjectManager()->get(Session\SessionInterface::class);

Neos.Flow/Configuration/Caches.yaml

@@ -128,7 +128,7 @@ Flow_Session_Storage:
 
 #  Aop filter expressions that need to be evaluated at runtime.
 Flow_Aop_RuntimeExpressions:
-  frontend: Neos\Cache\Frontend\PhpFrontend
+  frontend: Neos\Cache\Frontend\StringFrontend
   backend: Neos\Cache\Backend\SimpleFileBackend
 
 # Caches the map of possible PropertyMappers for source and target types