fix: address Greptile review on PR #12062

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).
This commit is contained in:
Prem Palanisamy
2026-04-27 17:24:50 +01:00
parent fb0d43daf3
commit 784babcf45
3 changed files with 8 additions and 6 deletions
+1 -1
View File
@@ -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' => '',
+6 -4
View File
@@ -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]);
@@ -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', []);