Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions ProcessMaker/Jobs/BpmnAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,13 @@ private function lockInstance($instanceId)
for ($tries = 0; $tries < $maxRetries; $tries++) {
$currentLock = $this->currentLock($ids);
if (!$currentLock) {
if (ProcessRequest::find($instanceId)) {
if (ProcessRequest::query()->whereKey($instanceId)->exists()) {
$lock = $this->requestLock($ids);
} else {
throw new Exception('Unable to lock instance #' . $this->instanceId . ': Request does not exists');
}
} elseif ($lock->id == $currentLock->id) {
$instance = ProcessRequest::findOrFail($instanceId);
$instance = $this->findProcessRequestForBpmnAction($instanceId);
$this->activateLock($lock);

return $instance;
Expand Down Expand Up @@ -231,7 +231,7 @@ private function findInstanceWithRetry($instanceId)

for ($attempt = 0; $attempt < $totalAttempts; $attempt++) {
try {
$instance = ProcessRequest::findOrFail($instanceId);
$instance = $this->findProcessRequestForBpmnAction($instanceId);

return $instance;
} catch (ModelNotFoundException $e) {
Expand Down Expand Up @@ -344,6 +344,23 @@ private function mSleep($milliseconds)
usleep($microseconds);
}

/**
* Load ProcessRequest with relations used when wiring the BPMN engine (reduces N+1 during completeTask / other BPMN jobs).
*
* @param int|string $instanceId
*/
private function findProcessRequestForBpmnAction($instanceId): ProcessRequest
{
return ProcessRequest::query()
->with([
'process',
'processVersion',
'collaboration',
])
->whereKey($instanceId)
->firstOrFail();
}

public function __destruct()
{
$this->instance = null;
Expand Down
8 changes: 6 additions & 2 deletions ProcessMaker/Models/ProcessRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -848,8 +848,12 @@ public function updateCatchEvents()
public function mergeLatestStoredData()
{
$store = $this->getDataStore();
$latest = self::select('data')->find($this->getId());
$this->data = $store->updateArray($latest->data);
// Read only the JSON column without hydrating a full ProcessRequest row (called often during BPMN transitions).
$latestData = static::query()->whereKey($this->getKey())->value('data');
if (!is_array($latestData)) {
$latestData = [];
}
$this->data = $store->updateArray($latestData);

return $this->data;
}
Expand Down
92 changes: 92 additions & 0 deletions tests/Feature/Api/TaskControllerUpdateResponseTimeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

namespace Tests\Feature\Api;

use Illuminate\Support\Facades\Notification;
use Mockery;
use ProcessMaker\Facades\WorkflowManager;
use ProcessMaker\Models\ProcessRequestToken;
use ProcessMaker\Models\User;
use Tests\Feature\Shared\RequestHelper;
use Tests\TestCase;

/**
* Response-time assertions for TaskController::update (COMPLETED and reassign paths).
*
* Set TASK_UPDATE_MAX_RESPONSE_MS in the environment (milliseconds) to tighten or relax the limit.
* Default is 20000 ms so CI stays stable; use a lower value locally to catch regressions.
*
* @group process_tests
* @group task_update_timing
*/
class TaskControllerUpdateResponseTimeTest extends TestCase
{
use RequestHelper;

private function maxResponseTimeMsForTaskUpdate(): int
{
$env = getenv('TASK_UPDATE_MAX_RESPONSE_MS');
if ($env !== false && $env !== '' && is_numeric($env)) {
return (int) $env;
}

return 20000;
}

private function assertResponseWithinMs(float $elapsedMs, int $maxMs, string $label): void
{
$this->assertLessThanOrEqual(
$maxMs,
$elapsedMs,
"{$label}: request took " . round($elapsedMs) . "ms, limit {$maxMs}ms (set TASK_UPDATE_MAX_RESPONSE_MS to adjust)"
);
}

public function testCompleteTaskUpdateResponseTime(): void
{
$maxMs = $this->maxResponseTimeMsForTaskUpdate();

$token = ProcessRequestToken::factory()->create([
'user_id' => $this->user->id,
'status' => 'ACTIVE',
]);

WorkflowManager::shouldReceive('completeTask')
->once()
->with(Mockery::any(), Mockery::any(), Mockery::any(), Mockery::any());

$params = ['status' => 'COMPLETED', 'data' => ['foo' => 'bar']];

$started = microtime(true);
$response = $this->apiCall('PUT', '/tasks/' . $token->id, $params);
$elapsedMs = (microtime(true) - $started) * 1000;

$response->assertStatus(200);
$this->assertResponseWithinMs($elapsedMs, $maxMs, 'PUT api/tasks/{id} (status=COMPLETED)');
}

public function testReassignTaskUpdateResponseTime(): void
{
$maxMs = $this->maxResponseTimeMsForTaskUpdate();

$assignee = User::factory()->create();
$token = ProcessRequestToken::factory()->create([
'user_id' => $this->user->id,
'status' => 'ACTIVE',
]);

// Prevent notification errors by faking the notification system
// The factory generates random element_ids that don't exist in the BPMN document
Notification::fake();

$started = microtime(true);
$response = $this->apiCall('PUT', '/tasks/' . $token->id, [
'user_id' => $assignee->id,
'comments' => 'response time test',
]);
$elapsedMs = (microtime(true) - $started) * 1000;

$response->assertStatus(200);
$this->assertResponseWithinMs($elapsedMs, $maxMs, 'PUT api/tasks/{id} (reassign)');
}
}
Loading