From ac20030332ded4ff103dad31cf69460c3766c0fe Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Tue, 15 Sep 2026 14:58:53 -0400 Subject: [PATCH 1/6] feat: enhance ProcessVariableController and add ProcessVariableDiscoveryService for improved variable handling --- .../Api/V1_1/ProcessVariableController.php | 178 +++++----- .../ProcessVariableDiscoveryService.php | 331 ++++++++++++++++++ 2 files changed, 422 insertions(+), 87 deletions(-) create mode 100644 ProcessMaker/Services/ProcessVariableDiscoveryService.php diff --git a/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php b/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php index 2b5f462923..94afcbdf22 100644 --- a/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php +++ b/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php @@ -4,17 +4,17 @@ namespace ProcessMaker\Http\Controllers\Api\V1_1; +use Illuminate\Database\QueryException; use Illuminate\Http\Request; use Illuminate\Pagination\LengthAwarePaginator; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Schema; use ProcessMaker\Http\Controllers\Controller; -use ProcessMaker\Managers\ExportManager; -use ProcessMaker\Models\Process; -use ProcessMaker\Models\Screen; use ProcessMaker\Package\SavedSearch\Models\SavedSearch; use ProcessMaker\Package\VariableFinder\Models\ProcessVariable; +use ProcessMaker\Services\ProcessVariableDiscoveryService; class ProcessVariableController extends Controller { @@ -119,12 +119,19 @@ public function index(Request $request) $processIds = !empty($validated['processIds']) ? array_map('intval', explode(',', $validated['processIds'])) : []; - $perPage = $validated['per_page'] ?? 20; - $page = $validated['page'] ?? 1; + $perPage = (int) ($validated['per_page'] ?? 20); + $page = (int) ($validated['page'] ?? 1); $excludeSavedSearch = $validated['savedSearchId'] ?? 0; - // Generate mock data - if (static::$mockData) { + // Available columns and process variables have independent pagination. + if ($request->has('onlyAvailable')) { + $paginator = $this->getAvailableColumnsPaginator( + $excludeSavedSearch, + $page, + $perPage, + $request + ); + } elseif (static::$mockData) { $paginator = $this->getProcessesVariablesFromMock($processIds, $excludeSavedSearch, $page, $perPage, $request); } else { $paginator = $this->getProcessesVariables($processIds, $excludeSavedSearch, $page, $perPage, $request); @@ -204,13 +211,11 @@ private function getProcessesVariablesFromMock(array $processIds, $excludeSavedS * @param int $page The page number for pagination. * @param int $perPage The number of items per page for pagination. * @param Request $request The HTTP request instance. - * @return \Illuminate\Http\JsonResponse JSON response containing the process variables. + * @return LengthAwarePaginator */ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $page, $perPage, $request) { - // Determine which columns to exclude based on the saved search $activeColumns = []; - $savedSearch = null; if ($excludeSavedSearch) { $savedSearch = SavedSearch::find($excludeSavedSearch); if ($savedSearch && $savedSearch->current_columns) { @@ -218,26 +223,20 @@ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $p } } - // If the classes or tables do not exist, fallback to a saved search approach. if ( !class_exists(ProcessVariable::class) || !Schema::hasTable('process_variables') || !self::$useVarFinder + || $processIds === [] ) { - $paginator = $this->getProcessesVariablesFrom($processIds); - if ($request->has('onlyAvailable')) { - return $this->mergeOnlyAvailableColumns($paginator, $savedSearch, $activeColumns); - } - - return $paginator; + return $this->getProcessesVariablesFrom($processIds, $activeColumns, $page, $perPage, $request); } - // Build a single query that joins asset_variables, and var_finder_variables - // and applies filtering for excluded fields. $query = DB::table('asset_variables as av') ->join('var_finder_variables as vfv', 'av.id', '=', 'vfv.asset_variable_id') ->whereIn('av.process_id', $processIds) ->groupBy('vfv.field', 'vfv.label') + ->orderBy('vfv.field') ->select([ DB::raw('MAX(vfv.id) as id'), DB::raw("CONCAT('data.', vfv.field) as field"), @@ -249,33 +248,54 @@ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $p DB::raw('NULL AS `default`'), ]); - // Return the paginated result - $paginator = $query->paginate($perPage, ['*'], 'page', $page); - - if ($request->has('onlyAvailable')) { - return $this->mergeOnlyAvailableColumns($paginator, $savedSearch, $activeColumns); + $activeDataColumns = collect($activeColumns) + ->filter(fn ($column) => is_string($column) && str_starts_with($column, 'data.')) + ->map(fn ($column) => substr($column, 5)) + ->values() + ->all(); + if ($activeDataColumns !== []) { + $query->whereNotIn('vfv.field', $activeDataColumns); } - return $query->paginate($perPage, ['*'], 'page', $page); + try { + return $query->paginate($perPage, ['*'], 'page', $page); + } catch (QueryException $exception) { + if ((int) ($exception->errorInfo[1] ?? 0) !== 1038) { + throw $exception; + } + + Log::warning('Variable Finder exceeded MySQL sort memory; using screen variables', [ + 'process_count' => count($processIds), + 'page' => $page, + 'per_page' => $perPage, + ]); + + return $this->getProcessesVariablesFrom($processIds, $activeColumns, $page, $perPage, $request); + } } /** - * Merge only available columns with collection items - * - * @param LengthAwarePaginator $paginator - * @param SavedSearch|null $savedSearch - * @param array $activeColumns - * - * @return LengthAwarePaginator + * Paginate the saved search columns separately from process variables. */ - private function mergeOnlyAvailableColumns($paginator, $savedSearch, $activeColumns) + private function getAvailableColumnsPaginator($savedSearchId, int $page, int $perPage, Request $request): LengthAwarePaginator { - $availableColumns = $this->mergeAvailableColumns($savedSearch); - $availableColumns = $availableColumns->merge($paginator->items()); - $availableColumns = $this->filterActiveColumns($availableColumns, $activeColumns); - $paginator->setCollection($availableColumns); + $savedSearch = $savedSearchId ? SavedSearch::find($savedSearchId) : null; + $activeColumns = $savedSearch?->current_columns?->pluck('field')->toArray() ?? []; + $availableColumns = $this->filterActiveColumns( + $this->mergeAvailableColumns($savedSearch), + $activeColumns + )->unique('field')->values(); - return $paginator; + return new LengthAwarePaginator( + $availableColumns->forPage($page, $perPage)->values(), + $availableColumns->count(), + $perPage, + $page, + [ + 'path' => $request->url(), + 'query' => $request->query(), + ] + ); } /** @@ -285,15 +305,13 @@ private function mergeOnlyAvailableColumns($paginator, $savedSearch, $activeColu */ private function mergeAvailableColumns(?SavedSearch $savedSearch = null) { - $availableColumns = collect(); - - if ($savedSearch?->available_columns) { - $availableColumns = $savedSearch->available_columns->merge( - $savedSearch->getDataColumnsAttribute() ?? collect() - ); + if (!$savedSearch) { + return collect(); } - return $availableColumns; + return $savedSearch->available_columns->merge( + $savedSearch->getDataColumnsAttribute() ?? collect() + ); } /** @@ -330,50 +348,36 @@ public static function useVarFinder(bool $value = true) } /** - * Retrieve process variables from its screens. + * Retrieve process variables from Variable Finder or process screens. * - * @param array $processIds - * - * @return LengthAwarePaginator + * @param list $processIds + * @param list $activeColumns */ - private function getProcessesVariablesFrom(array $processIds) - { - $perPage = request()->get('per_page', 20); - // Validate processIds input is required - if (empty($processIds)) { - return new LengthAwarePaginator([], 0, $perPage, 1); - } - - // Get screens used in the processes - $processes = Process::whereIn('id', $processIds)->get(); - $ids = collect([]); - foreach ($processes as $process) { - $manager = app(ExportManager::class); - try { - $ids = $ids->merge($manager->getDependenciesOfType(Screen::class, $process)); - } catch (\Exception $e) { - $ids = collect([]); - } - } + private function getProcessesVariablesFrom( + array $processIds, + array $activeColumns, + int $page, + int $perPage, + Request $request + ): LengthAwarePaginator { + $service = app(ProcessVariableDiscoveryService::class); + $useVariableFinder = self::$useVarFinder; + $columns = $processIds === [] + ? $service->unscoped($useVariableFinder) + : $service->forProcessIds($processIds, $useVariableFinder); + $columns = $this->filterActiveColumns($columns, $activeColumns) + ->unique('field') + ->values(); - // Get columns from screens - $columns = collect([]); - $screens = Screen::whereIn('id', $ids->unique())->where('type', '!=', 'DISPLAY')->get(); - foreach ($screens as $screen) { - $screenColumns = $screen->fields->map(function ($item) { - $item->field = "data.{$item->field}"; - - return $item; - }); - - $columns = $columns->merge($screenColumns); - } - - // Paginate the result - $page = request()->get('page', 1); - $total = $columns->count(); - $items = $columns->forPage($page, $perPage); - - return new LengthAwarePaginator($items, $total, $perPage, $page); + return new LengthAwarePaginator( + $columns->forPage($page, $perPage)->values(), + $columns->count(), + $perPage, + $page, + [ + 'path' => $request->url(), + 'query' => $request->query(), + ] + ); } } diff --git a/ProcessMaker/Services/ProcessVariableDiscoveryService.php b/ProcessMaker/Services/ProcessVariableDiscoveryService.php new file mode 100644 index 0000000000..457320eb8b --- /dev/null +++ b/ProcessMaker/Services/ProcessVariableDiscoveryService.php @@ -0,0 +1,331 @@ + $processIds + */ + public function forProcessIds(array $processIds, bool $useVariableFinder = true): Collection + { + $processIds = array_values(array_unique(array_filter(array_map('intval', $processIds)))); + + if ($processIds === []) { + return $this->unscoped($useVariableFinder); + } + + return $this->remember( + $this->scopedCacheKey($processIds, $useVariableFinder), + fn () => $this->resolveForProcessIds($processIds, $useVariableFinder) + ); + } + + /** + * Resolve data.* columns across the catalog when a Saved Search has no process scope. + */ + public function unscoped(bool $useVariableFinder = true): Collection + { + return $this->remember( + $this->unscopedCacheKey($useVariableFinder), + fn () => $this->resolveUnscoped($useVariableFinder) + ); + } + + /** + * @param list $processIds + * @return list> + */ + private function resolveForProcessIds(array $processIds, bool $useVariableFinder): array + { + if ($useVariableFinder) { + $fromFinder = $this->columnsFromVariableFinder($processIds); + if ($fromFinder !== null) { + return $fromFinder; + } + } + + return $this->columnsFromScreens( + Process::whereIn('id', $processIds)->orderBy('id')->get() + ); + } + + /** + * @return list> + */ + private function resolveUnscoped(bool $useVariableFinder): array + { + if ($useVariableFinder) { + $fromFinder = $this->columnsFromVariableFinder([]); + if ($fromFinder !== null) { + return $fromFinder; + } + } + + return $this->columnsFromScreens( + Process::query() + ->orderByDesc('updated_at') + ->limit(self::UNSCOPED_SCREEN_PROCESS_LIMIT) + ->get() + ); + } + + /** + * @param list $processIds + * @return list>|null + */ + private function columnsFromVariableFinder(array $processIds): ?array + { + if (!$this->variableFinderAvailable()) { + return null; + } + + try { + $query = DB::table('asset_variables as av') + ->join('var_finder_variables as vfv', 'av.id', '=', 'vfv.asset_variable_id') + ->groupBy('vfv.field', 'vfv.label') + ->orderBy('vfv.field') + ->select([ + 'vfv.field', + 'vfv.label', + DB::raw('MAX(vfv.data_type) as format'), + ]); + + if ($processIds !== []) { + $query->whereIn('av.process_id', $processIds); + } + + return $query->get() + ->map(fn ($row) => $this->columnArray( + $this->prefixedField((string) $row->field), + $row->label, + $row->format + )) + ->unique('field') + ->values() + ->all(); + } catch (QueryException $exception) { + if (!$this->isMysqlSortMemoryError($exception)) { + throw $exception; + } + + Log::warning('Variable Finder exceeded MySQL sort memory; using screen variables', [ + 'process_count' => count($processIds), + ]); + + return null; + } + } + + /** + * @return list> + */ + private function columnsFromScreens(Collection $processes): array + { + $resolver = new ScreensInProcess(); + $screenIds = collect(); + + foreach ($processes as $process) { + try { + foreach ($resolver->referencesToExport($process) as [$class, $id]) { + if ($class === Screen::class) { + $screenIds->push($id); + } + } + } catch (Throwable $exception) { + Log::warning('Unable to resolve process screens for variable discovery', [ + 'process_id' => $process->id, + 'message' => $exception->getMessage(), + ]); + } + } + + $columns = collect(); + $screens = Screen::whereIn('id', $screenIds->unique()) + ->where('type', '!=', 'DISPLAY') + ->get(); + + foreach ($screens as $screen) { + try { + $columns = $columns->merge( + $screen->fields->map(fn ($column) => $this->columnArray( + $this->prefixedField((string) $column->field), + $column->label, + $column->format + )) + ); + } catch (Throwable $exception) { + Log::warning('Unable to resolve screen fields for variable discovery', [ + 'screen_id' => $screen->id, + 'message' => $exception->getMessage(), + ]); + } + } + + return $columns->unique('field')->values()->all(); + } + + /** + * @param callable(): list> $resolver + */ + private function remember(string $cacheKey, callable $resolver): Collection + { + $lastKnownKey = $cacheKey . ':last-known-good'; + $payload = Cache::flexible( + $cacheKey, + [self::CACHE_FRESH_SECONDS, self::CACHE_STALE_SECONDS], + fn () => $this->refresh($cacheKey, $lastKnownKey, $resolver) + ); + + return collect(is_array($payload) ? $payload : []) + ->map(fn (array $column) => new Column($column)); + } + + /** + * @param callable(): list> $resolver + * @return list> + */ + private function refresh(string $cacheKey, string $lastKnownKey, callable $resolver): array + { + $lastKnown = Cache::get($lastKnownKey); + $lock = Cache::lock($cacheKey . ':lock', self::CACHE_LOCK_SECONDS); + + if (!$lock->get()) { + return $this->waitForLock($lock, $lastKnownKey, $lastKnown); + } + + try { + $payload = $resolver(); + Cache::put($lastKnownKey, $payload, self::LAST_KNOWN_SECONDS); + + return $payload; + } catch (Throwable $exception) { + Log::warning('Process variable discovery failed', [ + 'message' => $exception->getMessage(), + ]); + + return is_array($lastKnown) ? $lastKnown : []; + } finally { + $lock->release(); + } + } + + /** + * @param mixed $lastKnown + * @return list> + */ + private function waitForLock($lock, string $lastKnownKey, $lastKnown): array + { + try { + return $lock->block( + 5, + fn () => Cache::get($lastKnownKey, is_array($lastKnown) ? $lastKnown : []) + ); + } catch (LockTimeoutException) { + return is_array($lastKnown) ? $lastKnown : []; + } + } + + /** + * @param list $processIds + */ + private function scopedCacheKey(array $processIds, bool $useVariableFinder): string + { + sort($processIds); + $processStamp = Process::whereIn('id', $processIds)->pluck('updated_at', 'id')->toJson(); + $finderStamp = $useVariableFinder ? $this->variableFinderFingerprint() : 'screens'; + + return 'process-variables:scoped:v1:' . sha1($processStamp . '|' . $finderStamp); + } + + private function unscopedCacheKey(bool $useVariableFinder): string + { + if ($useVariableFinder && $this->variableFinderAvailable()) { + return 'process-variables:unscoped:v1:' . sha1($this->variableFinderFingerprint()); + } + + $processStamp = Process::query() + ->orderByDesc('updated_at') + ->limit(self::UNSCOPED_SCREEN_PROCESS_LIMIT) + ->pluck('updated_at', 'id') + ->toJson(); + + return 'process-variables:unscoped-screens:v1:' . sha1($processStamp); + } + + private function variableFinderFingerprint(): string + { + if (!$this->variableFinderAvailable()) { + return ''; + } + + $fingerprint = DB::table('var_finder_variables') + ->selectRaw('COUNT(*) as aggregate_count, MAX(updated_at) as max_updated') + ->first(); + + return json_encode($fingerprint) ?: ''; + } + + private function variableFinderAvailable(): bool + { + return class_exists(ProcessVariable::class) + && Schema::hasTable('process_variables') + && Schema::hasTable('var_finder_variables') + && Schema::hasTable('asset_variables'); + } + + private function isMysqlSortMemoryError(QueryException $exception): bool + { + return (int) ($exception->errorInfo[1] ?? 0) === self::MYSQL_SORT_MEMORY_ERROR; + } + + private function prefixedField(string $field): string + { + return str_starts_with($field, 'data.') ? $field : 'data.' . $field; + } + + /** + * @return array + */ + private function columnArray(string $field, mixed $label, mixed $format): array + { + return [ + 'label' => $label, + 'field' => $field, + 'sortable' => true, + 'default' => false, + 'format' => $format, + 'mask' => null, + ]; + } +} From 8812834719562c68e316a73d07c25e8f7ab3bb58 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Tue, 15 Sep 2026 15:05:35 -0400 Subject: [PATCH 2/6] test: add comprehensive tests for ProcessVariableController, including pagination and variable filtering scenarios --- .../V1_1/ProcessVariableControllerTest.php | 254 +++++++++++++++++- 1 file changed, 246 insertions(+), 8 deletions(-) diff --git a/tests/Feature/Api/V1_1/ProcessVariableControllerTest.php b/tests/Feature/Api/V1_1/ProcessVariableControllerTest.php index 0b013681de..9032172fd5 100644 --- a/tests/Feature/Api/V1_1/ProcessVariableControllerTest.php +++ b/tests/Feature/Api/V1_1/ProcessVariableControllerTest.php @@ -2,17 +2,22 @@ namespace Tests\Feature\Api\V1_1; +use Illuminate\Database\QueryException; use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Schema; use Illuminate\Support\Str; use ProcessMaker\Http\Controllers\Api\V1_1\ProcessVariableController; +use ProcessMaker\Models\Column; use ProcessMaker\Models\Process; +use ProcessMaker\Models\ProcessRequest; use ProcessMaker\Models\Screen; -use ProcessMaker\Models\User; +use ProcessMaker\Models\Setting; use ProcessMaker\Package\SavedSearch\Models\SavedSearch; use ProcessMaker\Package\VariableFinder\Models\AssetVariable; use ProcessMaker\Package\VariableFinder\Models\ProcessVariable; use ProcessMaker\Package\VariableFinder\Models\VarFinderVariable; +use ProcessMaker\Services\ProcessVariableDiscoveryService; use Tests\Feature\Shared\RequestHelper; use Tests\TestCase; @@ -23,15 +28,14 @@ class ProcessVariableControllerTest extends TestCase private bool $isVariablesFinderEnabled; /** - * Set up test environment by creating a test user and authenticating as them + * Load Variable Finder fixtures after the test user exists. + * + * Named withUserSetup so RequestHelper invokes it after the user is created. * * @return void */ - public function setupCreateUser() + public function withUserSetup() { - $this->user = User::factory()->create(); - $this->actingAs($this->user); - // Check if the VariableFinder package is enabled $this->isVariablesFinderEnabled = class_exists(ProcessVariable::class) && Schema::hasTable('process_variables'); @@ -243,7 +247,7 @@ private function loadVariableFinderData(array $processIds) // Generate data similarly to mockVariableFinder $format = $this->getRandomDataType(); $label = "Variable {$i} for Process {$processId}"; - $field = "data.var_{$processId}_{$i}"; + $field = "var_{$processId}_{$i}"; // 3. Create the VarFinderVariable record linked to the same AssetVariable VarFinderVariable::create([ @@ -362,7 +366,7 @@ public function test_saved_search_id_filtering(): void $this->assertFalse($filteredFields->contains('data.var_1_2')); // Check that the total count matches the actual number of variables - $this->assertEquals(10, $responseData['meta']['total']); // Total number of variables + $this->assertEquals(8, $responseData['meta']['total']); // Total number of variables } /** @@ -544,4 +548,238 @@ public function test_saved_search_with_no_available_columns(): void $this->assertFalse($filteredFields->contains('initiated_at')); $this->assertFalse($filteredFields->contains('completed_at')); } + + public function test_fulltext_saved_search_with_empty_process_ids_loads_available_columns(): void + { + ProcessVariableController::mock(false); + ProcessVariableController::useVarFinder(false); + Setting::updateOrCreate( + ['key' => 'indexed-search'], + ['config' => ['enabled' => false]] + ); + + $process = Process::factory()->create(); + ProcessRequest::factory()->create([ + 'process_id' => $process->id, + 'data' => ['four_33159_variable' => 'value'], + ]); + $savedSearch = SavedSearch::factory()->create([ + 'type' => SavedSearch::TYPE_REQUEST, + 'meta' => [ + 'icon' => 'search', + 'columns' => [], + ], + 'pmql' => '(fulltext LIKE "%test%")', + ]); + + $url = '/api/1.1/processes/variables?processIds=&savedSearchId=' . $savedSearch->id + . '&onlyAvailable=&per_page=100&page='; + $fields = collect(); + $page = 1; + + do { + $response = $this->apiCall('GET', $url . $page); + $response->assertStatus(200); + $fields = $fields->merge(collect($response->json('data'))->pluck('field')); + $lastPage = (int) $response->json('meta.last_page'); + $page++; + } while ($page <= $lastPage); + + $this->assertContains('case_number', $fields); + $this->assertContains('data.four_33159_variable', $fields); + } + + public function test_only_available_columns_are_paginated_without_losing_fields(): void + { + ProcessVariableController::mock(false); + ProcessVariableController::useVarFinder(false); + + $process = Process::factory()->create(); + $data = collect(range(1, 125))->mapWithKeys(function ($index) { + return ["four_33159_paginated_{$index}" => 'value']; + })->all(); + ProcessRequest::factory()->create([ + 'process_id' => $process->id, + 'data' => $data, + ]); + $savedSearch = SavedSearch::factory()->create([ + 'type' => SavedSearch::TYPE_REQUEST, + 'meta' => [ + 'icon' => 'search', + 'columns' => [ + [ + 'label' => 'Active paginated variable', + 'field' => 'data.four_33159_paginated_1', + ], + ], + ], + 'pmql' => '', + ]); + $url = '/api/1.1/processes/variables?processIds=' . $process->id + . '&savedSearchId=' . $savedSearch->id + . '&onlyAvailable=&per_page=50&page='; + + $firstPage = $this->apiCall('GET', $url . '1'); + $firstPage->assertStatus(200); + $total = $firstPage->json('meta.total'); + $lastPage = $firstPage->json('meta.last_page'); + $fields = collect($firstPage->json('data'))->pluck('field'); + + $this->assertGreaterThan(100, $total); + $this->assertCount(50, $firstPage->json('data')); + $this->assertSame(50, $firstPage->json('meta.per_page')); + $this->assertSame(1, $firstPage->json('meta.from')); + $this->assertSame(50, $firstPage->json('meta.to')); + $this->assertSame((int) ceil($total / 50), $lastPage); + + for ($page = 2; $page <= $lastPage; $page++) { + $response = $this->apiCall('GET', $url . $page); + $expectedCount = min(50, $total - (($page - 1) * 50)); + + $response->assertStatus(200); + $this->assertCount($expectedCount, $response->json('data')); + $this->assertSame($page, $response->json('meta.current_page')); + $this->assertSame($total, $response->json('meta.total')); + $this->assertSame((($page - 1) * 50) + 1, $response->json('meta.from')); + $this->assertSame(($page - 1) * 50 + $expectedCount, $response->json('meta.to')); + $fields = $fields->merge(collect($response->json('data'))->pluck('field')); + } + + $this->assertCount($total, $fields); + $this->assertCount($total, $fields->unique()); + $this->assertNotContains('data.four_33159_paginated_1', $fields); + foreach (range(2, 125) as $index) { + $this->assertContains("data.four_33159_paginated_{$index}", $fields); + } + } + + public function test_process_screen_variable_pages_exclude_active_columns(): void + { + ProcessVariableController::mock(false); + ProcessVariableController::useVarFinder(false); + + $bpmn = file_get_contents(base_path('tests/Feature/Api/bpmnPatterns/SimpleTaskProcess.bpmn')); + $screen = $this->createScreenWithFields(91, 12); + $process = Process::factory()->create([ + 'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen->id . '"', $bpmn), + ]); + $savedSearch = SavedSearch::factory()->create([ + 'type' => SavedSearch::TYPE_REQUEST, + 'meta' => [ + 'icon' => 'search', + 'columns' => [ + [ + 'label' => 'Variable 1 for Process 91', + 'field' => 'data.var_91_1', + ], + ], + ], + 'pmql' => '', + ]); + $url = '/api/1.1/processes/variables?processIds=' . $process->id + . '&savedSearchId=' . $savedSearch->id + . '&per_page=5&page='; + $fields = collect(); + + for ($page = 1; $page <= 3; $page++) { + $response = $this->apiCall('GET', $url . $page); + + $response->assertStatus(200); + $this->assertLessThanOrEqual(5, count($response->json('data'))); + $this->assertSame(11, $response->json('meta.total')); + $fields = $fields->merge(collect($response->json('data'))->pluck('field')); + } + + $this->assertCount(11, $fields); + $this->assertCount(11, $fields->unique()); + $this->assertNotContains('data.var_91_1', $fields); + } + + public function test_only_available_without_saved_search_returns_an_empty_typed_page(): void + { + $response = $this->apiCall( + 'GET', + '/api/1.1/processes/variables?processIds=1&page=2&per_page=7&onlyAvailable=' + ); + + $response->assertStatus(200); + $this->assertSame([], $response->json('data')); + $this->assertSame(2, $response->json('meta.current_page')); + $this->assertSame(7, $response->json('meta.per_page')); + $this->assertSame(0, $response->json('meta.total')); + $this->assertSame(1, $response->json('meta.last_page')); + $this->assertNull($response->json('meta.from')); + $this->assertNull($response->json('meta.to')); + $this->assertNull($response->json('meta.links.next')); + } + + public function test_only_available_does_not_scan_process_request_data(): void + { + $savedSearch = SavedSearch::factory()->create([ + 'type' => SavedSearch::TYPE_REQUEST, + 'meta' => ['columns' => []], + 'pmql' => '(fulltext LIKE "%test%")', + ]); + DB::flushQueryLog(); + DB::enableQueryLog(); + + try { + $response = $this->apiCall( + 'GET', + '/api/1.1/processes/variables?processIds=&savedSearchId=' . $savedSearch->id + . '&page=1&per_page=5&onlyAvailable=' + ); + $queries = collect(DB::getQueryLog())->pluck('query')->implode("\n"); + } finally { + DB::disableQueryLog(); + } + + $response->assertStatus(200); + $this->assertStringNotContainsString('lower(', strtolower($queries)); + $this->assertStringNotContainsString('LOWER(', $queries); + } + + public function test_variable_finder_sort_memory_error_falls_back_to_screen_variables(): void + { + if (!$this->isVariablesFinderEnabled) { + $this->markTestSkipped('Variable Finder is not enabled.'); + } + + ProcessVariableController::mock(false); + ProcessVariableController::useVarFinder(true); + $this->mock(ProcessVariableDiscoveryService::class, function ($mock) { + $mock->shouldReceive('forProcessIds') + ->once() + ->andReturn(collect([ + new Column([ + 'label' => 'Fallback Field', + 'field' => 'data.fallback_field', + 'sortable' => true, + 'default' => false, + 'format' => 'string', + 'mask' => null, + ]), + ])); + }); + DB::beforeExecuting(function ($query, $bindings) { + if (!str_contains($query, 'var_finder_variables')) { + return; + } + + $previous = new \PDOException('Out of sort memory', 1038); + $previous->errorInfo = ['HY001', 1038, 'Out of sort memory']; + + throw new QueryException('processmaker', $query, $bindings, $previous); + }); + + $response = $this->apiCall( + 'GET', + '/api/1.1/processes/variables?processIds=1&page=1&per_page=5' + ); + + $response->assertStatus(200); + $this->assertSame(['data.fallback_field'], collect($response->json('data'))->pluck('field')->all()); + $this->assertSame(1, $response->json('meta.total')); + $this->assertSame(5, $response->json('meta.per_page')); + } } From 138c08d280a5f9d3ef240f19bb97687032eabd9c Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Tue, 15 Sep 2026 15:21:10 -0400 Subject: [PATCH 3/6] refactor(sonarqube): replace sha1 with xxh128 for improved hash performance --- ProcessMaker/Services/ProcessVariableDiscoveryService.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ProcessMaker/Services/ProcessVariableDiscoveryService.php b/ProcessMaker/Services/ProcessVariableDiscoveryService.php index 457320eb8b..91ca3a2982 100644 --- a/ProcessMaker/Services/ProcessVariableDiscoveryService.php +++ b/ProcessMaker/Services/ProcessVariableDiscoveryService.php @@ -265,13 +265,13 @@ private function scopedCacheKey(array $processIds, bool $useVariableFinder): str $processStamp = Process::whereIn('id', $processIds)->pluck('updated_at', 'id')->toJson(); $finderStamp = $useVariableFinder ? $this->variableFinderFingerprint() : 'screens'; - return 'process-variables:scoped:v1:' . sha1($processStamp . '|' . $finderStamp); + return 'process-variables:scoped:v1:' . hash('xxh128', $processStamp . '|' . $finderStamp); } private function unscopedCacheKey(bool $useVariableFinder): string { if ($useVariableFinder && $this->variableFinderAvailable()) { - return 'process-variables:unscoped:v1:' . sha1($this->variableFinderFingerprint()); + return 'process-variables:unscoped:v1:' . hash('xxh128', $this->variableFinderFingerprint()); } $processStamp = Process::query() @@ -280,7 +280,7 @@ private function unscopedCacheKey(bool $useVariableFinder): string ->pluck('updated_at', 'id') ->toJson(); - return 'process-variables:unscoped-screens:v1:' . sha1($processStamp); + return 'process-variables:unscoped-screens:v1:' . hash('xxh128', $processStamp); } private function variableFinderFingerprint(): string From eff5d27012d3574e983a5dac3342d7bc86859da0 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Tue, 15 Sep 2026 15:26:02 -0400 Subject: [PATCH 4/6] fix: prevent caching of empty results and ensure last-known-good payload is returned --- .../Services/ProcessVariableDiscoveryService.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ProcessMaker/Services/ProcessVariableDiscoveryService.php b/ProcessMaker/Services/ProcessVariableDiscoveryService.php index 91ca3a2982..9e29e8b817 100644 --- a/ProcessMaker/Services/ProcessVariableDiscoveryService.php +++ b/ProcessMaker/Services/ProcessVariableDiscoveryService.php @@ -221,7 +221,9 @@ private function refresh(string $cacheKey, string $lastKnownKey, callable $resol $lock = Cache::lock($cacheKey . ':lock', self::CACHE_LOCK_SECONDS); if (!$lock->get()) { - return $this->waitForLock($lock, $lastKnownKey, $lastKnown); + // Never return an empty list here: Cache::flexible would persist it + // as the fresh payload and hide variables for the full cache window. + return $this->waitForLock($lock, $lastKnownKey, $lastKnown, $resolver); } try { @@ -242,9 +244,10 @@ private function refresh(string $cacheKey, string $lastKnownKey, callable $resol /** * @param mixed $lastKnown + * @param callable(): list> $resolver * @return list> */ - private function waitForLock($lock, string $lastKnownKey, $lastKnown): array + private function waitForLock($lock, string $lastKnownKey, $lastKnown, callable $resolver): array { try { return $lock->block( @@ -252,7 +255,12 @@ private function waitForLock($lock, string $lastKnownKey, $lastKnown): array fn () => Cache::get($lastKnownKey, is_array($lastKnown) ? $lastKnown : []) ); } catch (LockTimeoutException) { - return is_array($lastKnown) ? $lastKnown : []; + if (is_array($lastKnown) && $lastKnown !== []) { + return $lastKnown; + } + + // No last-known-good payload: resolve directly so we do not cache an empty result. + return $resolver(); } } From acedb6323420bbc83e59dfbab79d68194465cb87 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Tue, 15 Sep 2026 15:34:50 -0400 Subject: [PATCH 5/6] fix: update getProcessesVariablesFrom to include useVariableFinder parameter for improved variable handling --- .../Controllers/Api/V1_1/ProcessVariableController.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php b/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php index 94afcbdf22..5d3550bdc9 100644 --- a/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php +++ b/ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php @@ -270,7 +270,7 @@ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $p 'per_page' => $perPage, ]); - return $this->getProcessesVariablesFrom($processIds, $activeColumns, $page, $perPage, $request); + return $this->getProcessesVariablesFrom($processIds, $activeColumns, $page, $perPage, $request, false); } } @@ -358,10 +358,11 @@ private function getProcessesVariablesFrom( array $activeColumns, int $page, int $perPage, - Request $request + Request $request, + ?bool $useVariableFinder = null ): LengthAwarePaginator { $service = app(ProcessVariableDiscoveryService::class); - $useVariableFinder = self::$useVarFinder; + $useVariableFinder = $useVariableFinder ?? self::$useVarFinder; $columns = $processIds === [] ? $service->unscoped($useVariableFinder) : $service->forProcessIds($processIds, $useVariableFinder); From 2d4f89b259f08c9fdac6d19ccb3df16e9dca7c84 Mon Sep 17 00:00:00 2001 From: Miguel Angel Date: Tue, 15 Sep 2026 15:38:13 -0400 Subject: [PATCH 6/6] refactor: extract resolveAndRemember method for improved readability and error handling --- .../ProcessVariableDiscoveryService.php | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/ProcessMaker/Services/ProcessVariableDiscoveryService.php b/ProcessMaker/Services/ProcessVariableDiscoveryService.php index 9e29e8b817..3a8fdb6660 100644 --- a/ProcessMaker/Services/ProcessVariableDiscoveryService.php +++ b/ProcessMaker/Services/ProcessVariableDiscoveryService.php @@ -227,16 +227,7 @@ private function refresh(string $cacheKey, string $lastKnownKey, callable $resol } try { - $payload = $resolver(); - Cache::put($lastKnownKey, $payload, self::LAST_KNOWN_SECONDS); - - return $payload; - } catch (Throwable $exception) { - Log::warning('Process variable discovery failed', [ - 'message' => $exception->getMessage(), - ]); - - return is_array($lastKnown) ? $lastKnown : []; + return $this->resolveAndRemember($lastKnownKey, $lastKnown, $resolver); } finally { $lock->release(); } @@ -260,7 +251,28 @@ private function waitForLock($lock, string $lastKnownKey, $lastKnown, callable $ } // No last-known-good payload: resolve directly so we do not cache an empty result. - return $resolver(); + return $this->resolveAndRemember($lastKnownKey, $lastKnown, $resolver); + } + } + + /** + * @param mixed $lastKnown + * @param callable(): list> $resolver + * @return list> + */ + private function resolveAndRemember(string $lastKnownKey, $lastKnown, callable $resolver): array + { + try { + $payload = $resolver(); + Cache::put($lastKnownKey, $payload, self::LAST_KNOWN_SECONDS); + + return $payload; + } catch (Throwable $exception) { + Log::warning('Process variable discovery failed', [ + 'message' => $exception->getMessage(), + ]); + + return is_array($lastKnown) ? $lastKnown : []; } }