From 784babcf457bc5fee22ea1d59361a5f9d9318d02 Mon Sep 17 00:00:00 2001 From: Prem Palanisamy Date: Mon, 27 Apr 2026 09:13:35 +0100 Subject: [PATCH] fix: address Greptile review on PR #12062 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three P1 issues flagged on the initial commit: 1. Lock key in updateProjectService used "platform:project:{id}" — missing the "lock:" namespace prefix and using singular "project" instead of the conventional plural collection name. The factory's `lockTargetOf` extracts segment [2] as the telemetry target, so the broken key was emitting the project ID itself as the target attribute (cardinality blowup, broken dashboards). Fixed to "lock:platform:projects:{id}" matching the convention used in shared/api.php. 2. The 409 contention exception embedded the raw Redis lock key in its user-facing message, leaking internal collection names and the locking namespace to API clients. Removed the custom message so the catalog default ("The requested resource is currently being modified...") is used. Telemetry already carries the target collection for operator-side observability. 3. _APP_LOCKING_ENABLED variable doc had `introduction: '1.10.0'` on a 1.9.x-targeted PR. Corrected to '1.9.3' (next 1.9.x patch). --- app/config/variables.php | 2 +- app/init/resources.php | 10 ++++++---- .../Modules/Project/Http/Project/Services/Update.php | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/config/variables.php b/app/config/variables.php index 5dd7196266..46a6c7d1e2 100644 --- a/app/config/variables.php +++ b/app/config/variables.php @@ -37,7 +37,7 @@ return [ [ 'name' => '_APP_LOCKING_ENABLED', 'description' => 'Enable distributed locking for platform writes. Locks coordinate concurrent updates across API pods so that read-modify-write operations on shared documents do not lose updates. By default, set to \'enabled\'. Set to \'disabled\' as an emergency kill switch — locks become no-ops (fail-open) and concurrent writes will race.', - 'introduction' => '1.10.0', + 'introduction' => '1.9.3', 'default' => 'enabled', 'required' => false, 'question' => '', diff --git a/app/init/resources.php b/app/init/resources.php index 4c3a684d4e..d3443ee337 100644 --- a/app/init/resources.php +++ b/app/init/resources.php @@ -325,10 +325,12 @@ $container->set('distributedLockOrFail', function (\Redis $redis, Telemetry $tel if (! $acquired) { $attempts->add(1, ['outcome' => 'contended', 'target' => $target]); - throw new AppwriteException( - AppwriteException::GENERAL_RESOURCE_LOCKED, - "Resource '{$key}' is currently being modified by another request. Please retry." - ); + // Don't pass a custom message — the catalog message in + // app/config/errors.php is reused so we don't leak the internal + // lock key (which embeds collection name and document id) into a + // user-facing 409 response. The telemetry attribute already + // carries the target collection for operator-side observability. + throw new AppwriteException(AppwriteException::GENERAL_RESOURCE_LOCKED); } $attempts->add(1, ['outcome' => 'acquired', 'target' => $target]); diff --git a/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php b/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php index e7b048b734..1126da8897 100644 --- a/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php +++ b/src/Appwrite/Platform/Modules/Project/Http/Project/Services/Update.php @@ -81,7 +81,7 @@ class Update extends Action // the read-modify-write window; re-read inside the lock so the // baseline reflects any update that landed between request init // and lock acquisition. - $project = $distributedLockOrFail("platform:project:{$project->getId()}", function () use ($project, $serviceId, $enabled, $dbForPlatform, $authorization) { + $project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $serviceId, $enabled, $dbForPlatform, $authorization) { $project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId())); $services = $project->getAttribute('services', []);