From 6d052a9cde69353e51d9c0c52eb9ce2b500565ed Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Fri, 31 Jul 2026 15:55:57 -0400 Subject: [PATCH 1/2] =?UTF-8?q?FOUR-32497:=20[Octane]=20MEDIUM=20=E2=80=94?= =?UTF-8?q?=20Accumulating=20State=20"pmFunctions"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ProcessMaker/Models/FormalExpression.php | 10 ++ .../Models/FormalExpressionOctaneTest.php | 162 ++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 tests/unit/ProcessMaker/Models/FormalExpressionOctaneTest.php diff --git a/ProcessMaker/Models/FormalExpression.php b/ProcessMaker/Models/FormalExpression.php index 838944d60e..e91bbb4eac 100644 --- a/ProcessMaker/Models/FormalExpression.php +++ b/ProcessMaker/Models/FormalExpression.php @@ -56,6 +56,16 @@ protected function initFormalExpression() public function registerPMFunction($name, callable $callable) { static::$pmFunctions[$name] = $callable; + + // Also register into the current ExpressionLanguage instance if initialized + if ($this->feelExpression instanceof ExpressionLanguage) { + $this->feelExpression->register( + $name, + function () { + }, + $callable + ); + } } /** diff --git a/tests/unit/ProcessMaker/Models/FormalExpressionOctaneTest.php b/tests/unit/ProcessMaker/Models/FormalExpressionOctaneTest.php new file mode 100644 index 0000000000..6a5eaf5ce1 --- /dev/null +++ b/tests/unit/ProcessMaker/Models/FormalExpressionOctaneTest.php @@ -0,0 +1,162 @@ +setLanguage('FEEL'); + $formalExp1->setBody('customFn("test") == "test"'); + + // Use reflection to call registerPMFunction (simulating a package registering) + $reflection = new \ReflectionClass($formalExp1); + $method = $reflection->getMethod('registerPMFunction'); + $method->setAccessible(true); + $method->invoke($formalExp1, 'customFn', function ($arguments, $arg) { + return (string) $arg; + }); + + // Verify the function was registered + $staticProperty = $reflection->getProperty('pmFunctions'); + $staticProperty->setAccessible(true); + $this->assertArrayHasKey('customFn', $staticProperty->getValue()); + $initialCount = count($staticProperty->getValue()); + + // Simulate request 2: create a new instance in the same worker + // The static $pmFunctions should still contain the previously registered function + // but should not grow unbounded + $formalExp2 = new FormalExpression(); + $formalExp2->setLanguage('FEEL'); + + $reflection2 = new \ReflectionClass($formalExp2); + $staticProperty2 = $reflection2->getProperty('pmFunctions'); + $staticProperty2->setAccessible(true); + $pmFunctionsAfterRequest2 = $staticProperty2->getValue(); + + // The functions should persist (static across instances in same worker) + $this->assertArrayHasKey('customFn', $pmFunctionsAfterRequest2); + + // But the count should not have grown unexpectedly + $this->assertCount($initialCount, $pmFunctionsAfterRequest2); + + // Simulate request 3: register another custom function + $method2 = $reflection2->getMethod('registerPMFunction'); + $method2->setAccessible(true); + $method2->invoke($formalExp2, 'anotherFn', function ($arguments, $arg) { + return strtoupper((string) $arg); + }); + + $countAfterRequest3 = count($staticProperty2->getValue()); + $this->assertArrayHasKey('anotherFn', $staticProperty2->getValue()); + + // The registry grew by exactly 1 + $this->assertEquals($initialCount + 1, $countAfterRequest3); + } + + /** + * Test that the built-in system functions are always registered and + * available regardless of static state. + */ + public function test_built_in_functions_are_always_available(): void + { + $formalExp = new FormalExpression(); + $formalExp->setLanguage('FEEL'); + $formalExp->setBody('date("Y") > 1900'); + $this->assertTrue($formalExp([])); + } + + /** + * Test that a custom registered function works correctly. + */ + public function test_custom_pm_function_is_evaluable(): void + { + $formalExp = new FormalExpression(); + $formalExp->setLanguage('FEEL'); + $formalExp->setBody('greet("World") == "Hello, World!"'); + + $reflection = new \ReflectionClass($formalExp); + $method = $reflection->getMethod('registerPMFunction'); + $method->setAccessible(true); + $method->invoke($formalExp, 'greet', function ($arguments, $name) { + return 'Hello, ' . $name . '!'; + }); + + $this->assertTrue($formalExp([])); + } + + /** + * Test that duplicate function registration overwrites the previous one + * rather than causing conflicts. + */ + public function test_registering_same_function_twice_overwrites(): void + { + $formalExp = new FormalExpression(); + $formalExp->setLanguage('FEEL'); + + $reflection = new \ReflectionClass($formalExp); + $method = $reflection->getMethod('registerPMFunction'); + $method->setAccessible(true); + + // Register with first implementation + $method->invoke($formalExp, 'double', function ($arguments, $x) { + return $x * 2; + }); + + // Overwrite with second implementation + $method->invoke($formalExp, 'double', function ($arguments, $x) { + return $x * 3; + }); + + $formalExp->setBody('double(2) == 6'); + $this->assertTrue($formalExp([])); + } + + /** + * Test that pmFunctions remains bounded and does not grow with each + * FormalExpression instantiation. + */ + public function test_pm_functions_static_array_does_not_grow_with_instances(): void + { + $reflection = new \ReflectionClass(FormalExpression::class); + $staticProperty = $reflection->getProperty('pmFunctions'); + $staticProperty->setAccessible(true); + + // Clear any existing functions to get a clean baseline + $staticProperty->setValue([]); + + // Create multiple instances - the static array should not grow just + // from instantiating FormalExpression (only when registerPMFunction is called) + $initialCount = count($staticProperty->getValue()); + + for ($i = 0; $i < 10; $i++) { + $exp = new FormalExpression(); + $exp->setLanguage('FEEL'); + } + + $this->assertCount($initialCount, $staticProperty->getValue()); + } +} From 5a1e6a4cfc8a3abfa4103add1df1a1a3d4860751 Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Mon, 3 Aug 2026 08:55:17 -0400 Subject: [PATCH 2/2] fix: code review notes --- ProcessMaker/Models/FormalExpression.php | 19 ++- ProcessMaker/Octane/ResetRequestState.php | 2 + ProcessMaker/Support/PmFunctionRegistry.php | 50 ++++++ .../Models/FormalExpressionOctaneTest.php | 152 ++++++++++-------- 4 files changed, 152 insertions(+), 71 deletions(-) create mode 100644 ProcessMaker/Support/PmFunctionRegistry.php diff --git a/ProcessMaker/Models/FormalExpression.php b/ProcessMaker/Models/FormalExpression.php index e91bbb4eac..12cc91aad0 100644 --- a/ProcessMaker/Models/FormalExpression.php +++ b/ProcessMaker/Models/FormalExpression.php @@ -9,6 +9,7 @@ use ProcessMaker\Exception\SyntaxErrorException; use ProcessMaker\Nayra\Bpmn\BaseTrait; use ProcessMaker\Nayra\Contracts\Bpmn\FormalExpressionInterface; +use ProcessMaker\Support\PmFunctionRegistry; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; use Symfony\Component\ExpressionLanguage\SyntaxError; use Throwable; @@ -31,8 +32,6 @@ class FormalExpression implements FormalExpressionInterface 'FEEL' => ['feelExpression', 'feelEncode'], ]; - private static $pmFunctions = []; - /** * FEEL expression object to be used to evaluate * @var ExpressionLanguage @@ -55,7 +54,7 @@ protected function initFormalExpression() */ public function registerPMFunction($name, callable $callable) { - static::$pmFunctions[$name] = $callable; + PmFunctionRegistry::register($name, $callable); // Also register into the current ExpressionLanguage instance if initialized if ($this->feelExpression instanceof ExpressionLanguage) { @@ -68,6 +67,18 @@ function () { } } + /** + * Reset runtime-registered PM functions. + * + * Called by Octane's RequestTerminated listener to clear request-scoped + * functions and prevent accumulation across requests. Boot-time functions + * are preserved in the registry and re-registered on the next request. + */ + public static function resetPmFunctions(): void + { + PmFunctionRegistry::reset(); + } + /** * Register system functions */ @@ -284,7 +295,7 @@ function ($arguments, $array, $key, $default) { ); // Register global PM functions from packages - foreach (static::$pmFunctions as $name => $callable) { + foreach (PmFunctionRegistry::all() as $name => $callable) { $this->feelExpression->register( $name, function () { diff --git a/ProcessMaker/Octane/ResetRequestState.php b/ProcessMaker/Octane/ResetRequestState.php index 45071e97ad..0defa5b56a 100644 --- a/ProcessMaker/Octane/ResetRequestState.php +++ b/ProcessMaker/Octane/ResetRequestState.php @@ -5,6 +5,7 @@ namespace ProcessMaker\Octane; use ProcessMaker\Listeners\HandleRedirectListener; +use ProcessMaker\Models\FormalExpression; use ProcessMaker\Providers\ProcessMakerServiceProvider; final class ResetRequestState @@ -13,5 +14,6 @@ public function handle(): void { ProcessMakerServiceProvider::beginRequestTiming(); HandleRedirectListener::reset(); + FormalExpression::resetPmFunctions(); } } diff --git a/ProcessMaker/Support/PmFunctionRegistry.php b/ProcessMaker/Support/PmFunctionRegistry.php new file mode 100644 index 0000000000..0b8352ad9a --- /dev/null +++ b/ProcessMaker/Support/PmFunctionRegistry.php @@ -0,0 +1,50 @@ +make(PmFunctionRegistry::class); + } + public function test_pm_functions_do_not_accumulate_across_requests(): void { - // Simulate request 1: register a custom PM function + PmFunctionRegistry::clear(); + FormalExpression::resetPmFunctions(); + + // Request 1: register a custom PM function (runtime registration) $formalExp1 = new FormalExpression(); $formalExp1->setLanguage('FEEL'); $formalExp1->setBody('customFn("test") == "test"'); - // Use reflection to call registerPMFunction (simulating a package registering) $reflection = new \ReflectionClass($formalExp1); $method = $reflection->getMethod('registerPMFunction'); $method->setAccessible(true); @@ -40,47 +37,31 @@ public function test_pm_functions_do_not_accumulate_across_requests(): void return (string) $arg; }); - // Verify the function was registered - $staticProperty = $reflection->getProperty('pmFunctions'); - $staticProperty->setAccessible(true); - $this->assertArrayHasKey('customFn', $staticProperty->getValue()); - $initialCount = count($staticProperty->getValue()); + $this->assertCount(1, $this->registry()->all()); + + // Simulate Octane RequestTerminated reset + $resetState = new ResetRequestState(); + $resetState->handle(); - // Simulate request 2: create a new instance in the same worker - // The static $pmFunctions should still contain the previously registered function - // but should not grow unbounded + // After reset, runtime-registered functions should be cleared + $this->assertCount(0, $this->registry()->all()); + + // Request 2: register another custom function after reset $formalExp2 = new FormalExpression(); $formalExp2->setLanguage('FEEL'); $reflection2 = new \ReflectionClass($formalExp2); - $staticProperty2 = $reflection2->getProperty('pmFunctions'); - $staticProperty2->setAccessible(true); - $pmFunctionsAfterRequest2 = $staticProperty2->getValue(); - - // The functions should persist (static across instances in same worker) - $this->assertArrayHasKey('customFn', $pmFunctionsAfterRequest2); - - // But the count should not have grown unexpectedly - $this->assertCount($initialCount, $pmFunctionsAfterRequest2); - - // Simulate request 3: register another custom function $method2 = $reflection2->getMethod('registerPMFunction'); $method2->setAccessible(true); $method2->invoke($formalExp2, 'anotherFn', function ($arguments, $arg) { return strtoupper((string) $arg); }); - $countAfterRequest3 = count($staticProperty2->getValue()); - $this->assertArrayHasKey('anotherFn', $staticProperty2->getValue()); - - // The registry grew by exactly 1 - $this->assertEquals($initialCount + 1, $countAfterRequest3); + // Only the new function should exist after reset + $this->assertCount(1, $this->registry()->all()); + $this->assertArrayHasKey('anotherFn', $this->registry()->all()); } - /** - * Test that the built-in system functions are always registered and - * available regardless of static state. - */ public function test_built_in_functions_are_always_available(): void { $formalExp = new FormalExpression(); @@ -89,9 +70,6 @@ public function test_built_in_functions_are_always_available(): void $this->assertTrue($formalExp([])); } - /** - * Test that a custom registered function works correctly. - */ public function test_custom_pm_function_is_evaluable(): void { $formalExp = new FormalExpression(); @@ -108,10 +86,6 @@ public function test_custom_pm_function_is_evaluable(): void $this->assertTrue($formalExp([])); } - /** - * Test that duplicate function registration overwrites the previous one - * rather than causing conflicts. - */ public function test_registering_same_function_twice_overwrites(): void { $formalExp = new FormalExpression(); @@ -121,12 +95,10 @@ public function test_registering_same_function_twice_overwrites(): void $method = $reflection->getMethod('registerPMFunction'); $method->setAccessible(true); - // Register with first implementation $method->invoke($formalExp, 'double', function ($arguments, $x) { return $x * 2; }); - // Overwrite with second implementation $method->invoke($formalExp, 'double', function ($arguments, $x) { return $x * 3; }); @@ -135,28 +107,74 @@ public function test_registering_same_function_twice_overwrites(): void $this->assertTrue($formalExp([])); } - /** - * Test that pmFunctions remains bounded and does not grow with each - * FormalExpression instantiation. - */ public function test_pm_functions_static_array_does_not_grow_with_instances(): void { - $reflection = new \ReflectionClass(FormalExpression::class); - $staticProperty = $reflection->getProperty('pmFunctions'); - $staticProperty->setAccessible(true); + PmFunctionRegistry::clear(); + FormalExpression::resetPmFunctions(); - // Clear any existing functions to get a clean baseline - $staticProperty->setValue([]); - - // Create multiple instances - the static array should not grow just - // from instantiating FormalExpression (only when registerPMFunction is called) - $initialCount = count($staticProperty->getValue()); + $initialCount = count($this->registry()->all()); for ($i = 0; $i < 10; $i++) { $exp = new FormalExpression(); $exp->setLanguage('FEEL'); } - $this->assertCount($initialCount, $staticProperty->getValue()); + $this->assertCount($initialCount, $this->registry()->all()); + } + + public function test_reset_pm_functions_restores_boot_time_state(): void + { + PmFunctionRegistry::clear(); + + // Register a boot-time function + $formalExp = new FormalExpression(); + $reflection = new \ReflectionClass($formalExp); + $method = $reflection->getMethod('registerPMFunction'); + $method->setAccessible(true); + $method->invoke($formalExp, 'bootFn', function () { + }); + + // Mark current state as boot baseline + FormalExpression::resetPmFunctions(); + + // Register a runtime function after the boot baseline + $method->invoke($formalExp, 'runtimeFn', function () { + }); + + $this->assertCount(2, $this->registry()->all()); + + // Reset should clear runtime functions, keeping only boot-time + FormalExpression::resetPmFunctions(); + + $this->assertCount(1, $this->registry()->all()); + $this->assertArrayHasKey('bootFn', $this->registry()->all()); + $this->assertArrayNotHasKey('runtimeFn', $this->registry()->all()); + } + + public function test_boot_time_functions_survive_reset(): void + { + PmFunctionRegistry::clear(); + + // Register a boot-time function + $formalExp = new FormalExpression(); + $reflection = new \ReflectionClass($formalExp); + $method = $reflection->getMethod('registerPMFunction'); + $method->setAccessible(true); + $bootFn = function () { + }; + $method->invoke($formalExp, 'bootFn', $bootFn); + + // Mark current state as boot baseline + FormalExpression::resetPmFunctions(); + + // Register a runtime function after the boot baseline + $method->invoke($formalExp, 'runtimeFn', function () { + }); + + // Reset should preserve bootFn + FormalExpression::resetPmFunctions(); + + $this->assertCount(1, $this->registry()->all()); + $this->assertSame($bootFn, $this->registry()->all()['bootFn']); } }