mirror of
https://github.com/solidtime-io/solidtime.git
synced 2026-10-10 06:43:18 +01:00
Lock creation of running time entries to prevent race condition
Concurrent requests could both pass the running time entry check and create more than one running time entry for the same member.
This commit is contained in:
@@ -51,6 +51,7 @@ use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Collection;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
use Illuminate\Support\Facades\Blade;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
@@ -613,6 +614,22 @@ class TimeEntryController extends Controller
|
||||
$this->checkPermission($organization, 'time-entries:create:all');
|
||||
}
|
||||
|
||||
// Lock the creation of running time entries per user, so that concurrent requests can not create more than one running time entry
|
||||
$lock = $request->input('end') === null ? Cache::lock('time-entries:running:'.$member->user_id, 10) : null;
|
||||
$lock?->block(5);
|
||||
|
||||
try {
|
||||
return $this->storeTimeEntry($organization, $member, $request);
|
||||
} finally {
|
||||
$lock?->release();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @throws TimeEntryStillRunningApiException
|
||||
*/
|
||||
private function storeTimeEntry(Organization $organization, Member $member, TimeEntryStoreRequest $request): JsonResource
|
||||
{
|
||||
if ($request->input('end') === null && TimeEntry::query()->whereBelongsTo($member, 'member')->where('end', null)->exists()) {
|
||||
throw new TimeEntryStillRunningApiException;
|
||||
}
|
||||
|
||||
@@ -24,7 +24,9 @@ use App\Models\Task;
|
||||
use App\Models\TimeEntry;
|
||||
use App\Models\User;
|
||||
use App\Service\TimeEntryFilter;
|
||||
use Illuminate\Contracts\Cache\LockTimeoutException;
|
||||
use Illuminate\Support\Carbon;
|
||||
use Illuminate\Support\Facades\Cache;
|
||||
use Illuminate\Support\Facades\Config;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
@@ -2444,6 +2446,52 @@ class TimeEntryEndpointTest extends ApiEndpointTestAbstract
|
||||
$response->assertJsonPath('error', true);
|
||||
}
|
||||
|
||||
public function test_store_endpoint_releases_running_time_entry_lock_after_request(): void
|
||||
{
|
||||
// Arrange
|
||||
$data = $this->createUserWithPermission([
|
||||
'time-entries:create:own',
|
||||
]);
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
$response = $this->postJson(route('api.v1.time-entries.store', [$data->organization->getKey()]), [
|
||||
'billable' => true,
|
||||
'start' => Carbon::now()->toIso8601ZuluString(),
|
||||
'end' => null,
|
||||
'member_id' => $data->member->getKey(),
|
||||
]);
|
||||
|
||||
// Assert
|
||||
$response->assertStatus(201);
|
||||
$this->assertTrue(Cache::lock('time-entries:running:'.$data->user->getKey(), 10)->get());
|
||||
}
|
||||
|
||||
public function test_store_endpoint_waits_for_running_time_entry_lock_and_fails_if_it_is_not_released(): void
|
||||
{
|
||||
// Arrange
|
||||
$data = $this->createUserWithPermission([
|
||||
'time-entries:create:own',
|
||||
]);
|
||||
Cache::lock('time-entries:running:'.$data->user->getKey(), 10)->get();
|
||||
$this->withoutExceptionHandling();
|
||||
Passport::actingAs($data->user);
|
||||
|
||||
// Act
|
||||
try {
|
||||
$this->postJson(route('api.v1.time-entries.store', [$data->organization->getKey()]), [
|
||||
'billable' => true,
|
||||
'start' => Carbon::now()->toIso8601ZuluString(),
|
||||
'end' => null,
|
||||
'member_id' => $data->member->getKey(),
|
||||
]);
|
||||
$this->fail('Expected LockTimeoutException');
|
||||
} catch (LockTimeoutException) {
|
||||
// Assert
|
||||
$this->assertSame(0, TimeEntry::query()->count());
|
||||
}
|
||||
}
|
||||
|
||||
public function test_store_endpoint_validation_fails_if_task_id_does_not_belong_to_project_id(): void
|
||||
{
|
||||
// Arrange
|
||||
|
||||
Reference in New Issue
Block a user