revert: scope distributed-lock PR to thundering-herd sites only

Drop the 18 lost-update endpoint locks (Project/* settings + Projects
team/update). Those address a different bug class (read-modify-write
races) than the manager-flagged production problem (regions slow queries
to platform from thundering herd on accessedAt writes).

Kept:
- distributedLock + distributedLockOrFail factories on the per-request
  container, GENERAL_RESOURCE_LOCKED exception, 409 mapping
- 4 thundering-herd sites: cache-invalidation in shared/api.php (3) +
  router projects.accessedAt in general.php (1)

Dropped:
- 18 endpoint OrFail wires
- testConcurrentTogglesAllPersist + Swoole-cURL test client patches
- dev/test-distributed-lock.sh smoke script
This commit is contained in:
Prem Palanisamy
2026-04-29 05:31:18 +01:00
parent da5382d58a
commit fce2abfd4c
22 changed files with 290 additions and 647 deletions
-157
View File
@@ -1,157 +0,0 @@
#!/usr/bin/env bash
#
# Manual smoke test for the distributed-lock pilot on `updateProjectService`.
#
# Fires N concurrent PATCH /project/services/:serviceId requests against the
# same project, each toggling a different service to "enabled=true". Then
# refetches the project and counts how many of the targeted services actually
# persisted.
#
# Usage:
# APPWRITE_ENDPOINT=http://localhost \
# APPWRITE_PROJECT_ID=<id> \
# APPWRITE_API_KEY=<key with project.write scope> \
# ./dev/test-distributed-lock.sh
#
# Required scopes on the API key:
# - project.write (to toggle services)
# - projects.read (to refetch project state via GET /v1/projects/:id)
#
# To prove the lock fixes the bug, run twice:
# 1. With `_APP_LOCKING_ENABLED=enabled` (default): expect successes == enabled
# 2. With `_APP_LOCKING_ENABLED=disabled` : expect successes > enabled (lost updates)
#
# Set `_APP_LOCKING_ENABLED` in `.env` and `docker compose up -d --force-recreate`
# between runs. Use `--parallelism N` to tune the concurrency level.
set -eu
# --- Configuration ---------------------------------------------------------
ENDPOINT="${APPWRITE_ENDPOINT:?set APPWRITE_ENDPOINT, e.g. http://localhost}"
PROJECT_ID="${APPWRITE_PROJECT_ID:?set APPWRITE_PROJECT_ID}"
API_KEY="${APPWRITE_API_KEY:?set APPWRITE_API_KEY}"
PARALLELISM="${PARALLELISM:-5}"
# Services to toggle concurrently — must be in the optional-services list of
# the project. These are the same set used by the e2e ServicesBase trait.
SERVICES=(teams storage functions sites messaging)
# Trim or extend SERVICES to match PARALLELISM.
SERVICES=("${SERVICES[@]:0:$PARALLELISM}")
if [ "${#SERVICES[@]}" -lt 2 ]; then
echo "ERROR: PARALLELISM must be >= 2 to detect contention" >&2
exit 1
fi
# --- Helpers ---------------------------------------------------------------
curl_appwrite() {
local method="$1"
local path="$2"
shift 2
curl -sS -o /tmp/lock-smoke-body.$$ -w '%{http_code}' \
-X "$method" \
-H "Content-Type: application/json" \
-H "X-Appwrite-Project: $PROJECT_ID" \
-H "X-Appwrite-Key: $API_KEY" \
"$ENDPOINT/v1$path" \
"$@"
}
toggle_service() {
local service="$1"
local enabled="$2"
local code
code=$(curl_appwrite PATCH "/project/services/$service" \
-d "{\"enabled\": $enabled}")
echo "$code"
}
get_project_state() {
curl -sS \
-H "Content-Type: application/json" \
-H "X-Appwrite-Project: console" \
-H "X-Appwrite-Key: $API_KEY" \
"$ENDPOINT/v1/projects/$PROJECT_ID"
}
# --- Run -------------------------------------------------------------------
echo "==> Distributed-lock smoke test"
echo " endpoint: $ENDPOINT"
echo " project: $PROJECT_ID"
echo " parallelism: $PARALLELISM"
echo " services: ${SERVICES[*]}"
echo
# 1. Baseline — disable all targeted services sequentially.
echo "==> Baseline: disabling ${SERVICES[*]}"
for svc in "${SERVICES[@]}"; do
code=$(toggle_service "$svc" false)
if [ "$code" != "200" ]; then
echo " WARN: baseline disable of $svc returned $code (expected 200)"
fi
done
# 2. Fire concurrent toggles to enabled=true. Capture each child's HTTP status.
echo
echo "==> Firing ${#SERVICES[@]} concurrent toggle requests..."
RESULTS_FILE=$(mktemp -t lock-smoke-results.XXXXXX)
for svc in "${SERVICES[@]}"; do
(
code=$(toggle_service "$svc" true)
printf '%s %s\n' "$svc" "$code" >> "$RESULTS_FILE"
) &
done
wait
# 3. Tally responses.
SUCCESS_COUNT=$(awk '$2 == 200' "$RESULTS_FILE" | wc -l | tr -d ' ')
CONFLICT_COUNT=$(awk '$2 == 409' "$RESULTS_FILE" | wc -l | tr -d ' ')
OTHER_COUNT=$(awk '$2 != 200 && $2 != 409' "$RESULTS_FILE" | wc -l | tr -d ' ')
echo
echo "==> Child responses:"
sort "$RESULTS_FILE"
echo
echo " successes (200): $SUCCESS_COUNT"
echo " conflicts (409): $CONFLICT_COUNT"
echo " other: $OTHER_COUNT"
rm -f "$RESULTS_FILE"
# 4. Refetch project; count how many targeted services are enabled.
PROJECT_JSON=$(get_project_state)
ENABLED_COUNT=0
for svc in "${SERVICES[@]}"; do
# Capitalize first letter to form serviceStatusFor<Svc> key.
Cap="$(echo "$svc" | awk '{print toupper(substr($1,1,1)) substr($1,2)}')"
val=$(echo "$PROJECT_JSON" | sed -nE "s/.*\"serviceStatusFor${Cap}\":[[:space:]]*(true|false).*/\1/p" | head -n1)
if [ "$val" = "true" ]; then
ENABLED_COUNT=$((ENABLED_COUNT + 1))
fi
done
echo " enabled in project state: $ENABLED_COUNT"
echo
# 5. Verdict.
if [ "$SUCCESS_COUNT" -eq "$ENABLED_COUNT" ]; then
echo "PASS: every successful toggle persisted (no lost updates)."
EXIT=0
else
echo "FAIL: lost updates detected. successes=$SUCCESS_COUNT enabled=$ENABLED_COUNT"
echo " Locking is either disabled or not effective on this endpoint."
EXIT=1
fi
# 6. Cleanup — re-enable all targeted services.
echo
echo "==> Cleanup: re-enabling ${SERVICES[*]}"
for svc in "${SERVICES[@]}"; do
toggle_service "$svc" true >/dev/null || true
done
rm -f /tmp/lock-smoke-body.$$
exit "$EXIT"
@@ -60,7 +60,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -71,22 +70,17 @@ class Update extends Action
Database $dbForPlatform,
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
Event $queueForEvents
): void {
$auth = Config::getParam('auth')[$methodId] ?? [];
$authKey = $auth['key'] ?? '';
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $authKey, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths[$authKey] = $enabled;
$auths = $project->getAttribute('auths', []);
$auths[$authKey] = $enabled;
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
$queueForEvents->setParam('methodId', $methodId);
@@ -59,7 +59,6 @@ class Create extends Action
->inject('project')
->inject('dbForPlatform')
->inject('authorization')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -71,8 +70,21 @@ class Create extends Action
Document $project,
Database $dbForPlatform,
Authorization $authorization,
callable $distributedLockOrFail,
) {
$auths = $project->getAttribute('auths', []);
$mockNumbers = $auths['mockNumbers'] ?? [];
if (\count($mockNumbers) >= APP_LIMIT_COUNT) {
throw new Exception(Exception::MOCK_NUMBER_LIMIT_EXCEEDED);
}
foreach ($mockNumbers as $mockNumber) {
if ($mockNumber['phone'] === $number) {
throw new Exception(Exception::MOCK_NUMBER_ALREADY_EXISTS);
}
}
// Set to now date
$mockNumber = [
'phone' => $number,
@@ -81,29 +93,14 @@ class Create extends Action
'$updatedAt' => DateTime::now(),
];
$distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $number, $mockNumber, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$mockNumbers[] = $mockNumber;
$auths['mockNumbers'] = $mockNumbers;
$auths = $project->getAttribute('auths', []);
$mockNumbers = $auths['mockNumbers'] ?? [];
$updates = new Document([
'auths' => $auths,
]);
if (\count($mockNumbers) >= APP_LIMIT_COUNT) {
throw new Exception(Exception::MOCK_NUMBER_LIMIT_EXCEEDED);
}
foreach ($mockNumbers as $existing) {
if ($existing['phone'] === $number) {
throw new Exception(Exception::MOCK_NUMBER_ALREADY_EXISTS);
}
}
$mockNumbers[] = $mockNumber;
$auths['mockNumbers'] = $mockNumbers;
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents->setParam('number', $number);
@@ -58,7 +58,6 @@ class Delete extends Action
->inject('project')
->inject('dbForPlatform')
->inject('authorization')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -69,33 +68,33 @@ class Delete extends Action
Document $project,
Database $dbForPlatform,
Authorization $authorization,
callable $distributedLockOrFail,
) {
$distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $number, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths = $project->getAttribute('auths', []);
$mockNumbers = $auths['mockNumbers'] ?? [];
$mockNumbers = $auths['mockNumbers'] ?? [];
$mockNumberIndex = null;
foreach ($mockNumbers as $index => $mock) {
if ($mock['phone'] === $number) {
$mockNumberIndex = $index;
break;
}
$mockNumberIndex = null;
foreach ($mockNumbers as $index => $mock) {
if ($mock['phone'] === $number) {
$mockNumberIndex = $index;
break;
}
}
if (\is_null($mockNumberIndex)) {
throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND);
}
if (\is_null($mockNumberIndex)) {
throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND);
}
unset($mockNumbers[$mockNumberIndex]);
$auths['mockNumbers'] = array_values($mockNumbers);
unset($mockNumbers[$mockNumberIndex]);
$mockNumbers = array_values($mockNumbers);
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$auths['mockNumbers'] = $mockNumbers;
$updates = new Document([
'auths' => $auths,
]);
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents->setParam('number', $number);
@@ -59,7 +59,6 @@ class Update extends Action
->inject('project')
->inject('dbForPlatform')
->inject('authorization')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -71,41 +70,38 @@ class Update extends Action
Document $project,
Database $dbForPlatform,
Authorization $authorization,
callable $distributedLockOrFail,
) {
$mockNumber = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $number, $otp, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths = $project->getAttribute('auths', []);
$mockNumbers = $auths['mockNumbers'] ?? [];
$mockNumbers = $auths['mockNumbers'] ?? [];
$mockNumberIndex = null;
foreach ($mockNumbers as $index => $mock) {
if ($mock['phone'] === $number) {
$mockNumberIndex = $index;
break;
}
$mockNumberIndex = null;
foreach ($mockNumbers as $index => $mock) {
if ($mock['phone'] === $number) {
$mockNumberIndex = $index;
break;
}
}
if (\is_null($mockNumberIndex)) {
throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND);
}
if (\is_null($mockNumberIndex)) {
throw new Exception(Exception::MOCK_NUMBER_NOT_FOUND);
}
$mockNumbers[$mockNumberIndex]['otp'] = $otp;
$mockNumbers[$mockNumberIndex]['$updatedAt'] = DateTime::now();
$auths['mockNumbers'] = $mockNumbers;
$mockNumbers[$mockNumberIndex]['otp'] = $otp;
$mockNumbers[$mockNumberIndex]['$updatedAt'] = DateTime::now();
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
$auths['mockNumbers'] = $mockNumbers;
return $mockNumbers[$mockNumberIndex];
});
$updates = new Document([
'auths' => $auths,
]);
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents->setParam('number', $number);
$response
->setStatusCode(Response::STATUS_CODE_OK)
->dynamic(new Document($mockNumber), Response::MODEL_MOCK_NUMBER);
->dynamic(new Document($mockNumbers[$mockNumberIndex]), Response::MODEL_MOCK_NUMBER);
}
}
@@ -60,7 +60,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -75,33 +74,30 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $userId, $userEmail, $userPhone, $userName, $userMFA, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths = $project->getAttribute('auths', []);
if ($userId !== null) {
$auths['membershipsUserId'] = $userId;
}
if ($userEmail !== null) {
$auths['membershipsUserEmail'] = $userEmail;
}
if ($userPhone !== null) {
$auths['membershipsUserPhone'] = $userPhone;
}
if ($userName !== null) {
$auths['membershipsUserName'] = $userName;
}
if ($userMFA !== null) {
$auths['membershipsMfa'] = $userMFA;
}
if ($userId !== null) {
$auths['membershipsUserId'] = $userId;
}
if ($userEmail !== null) {
$auths['membershipsUserEmail'] = $userEmail;
}
if ($userPhone !== null) {
$auths['membershipsUserPhone'] = $userPhone;
}
if ($userName !== null) {
$auths['membershipsUserName'] = $userName;
}
if ($userMFA !== null) {
$auths['membershipsMfa'] = $userMFA;
}
$updates = new Document([
'auths' => $auths,
]);
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -56,7 +56,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -67,18 +66,15 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths['passwordDictionary'] = $enabled;
$auths = $project->getAttribute('auths', []);
$auths['passwordDictionary'] = $enabled;
$updates = new Document([
'auths' => $auths,
]);
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -59,7 +59,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -70,18 +69,20 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $total, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths = $project->getAttribute('auths', []);
$auths['passwordHistory'] = \is_null($total) ? 0 : $total;
if (\is_null($total)) {
$auths['passwordHistory'] = 0;
} else {
$auths['passwordHistory'] = $total;
}
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$updates = new Document([
'auths' => $auths,
]);
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -57,7 +57,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -68,18 +67,15 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths['personalDataCheck'] = $enabled;
$auths = $project->getAttribute('auths', []);
$auths['personalDataCheck'] = $enabled;
$updates = new Document([
'auths' => $auths,
]);
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -56,7 +56,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -67,18 +66,15 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths['sessionAlerts'] = $enabled;
$auths = $project->getAttribute('auths', []);
$auths['sessionAlerts'] = $enabled;
$updates = new Document([
'auths' => $auths,
]);
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -56,7 +56,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -67,18 +66,15 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $duration, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths['duration'] = $duration;
$auths = $project->getAttribute('auths', []);
$auths['duration'] = $duration;
$updates = new Document([
'auths' => $auths,
]);
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -56,7 +56,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -67,18 +66,15 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths['invalidateSessions'] = $enabled;
$auths = $project->getAttribute('auths', []);
$auths['invalidateSessions'] = $enabled;
$updates = new Document([
'auths' => $auths,
]);
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -57,7 +57,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -68,18 +67,20 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $total, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths = $project->getAttribute('auths', []);
$auths['maxSessions'] = \is_null($total) ? 0 : $total;
if (\is_null($total)) {
$auths['maxSessions'] = 0;
} else {
$auths['maxSessions'] = $total;
}
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$updates = new Document([
'auths' => $auths,
]);
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -57,7 +57,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -68,18 +67,20 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $total, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$auths = $project->getAttribute('auths', []);
$auths = $project->getAttribute('auths', []);
$auths['limit'] = \is_null($total) ? 0 : $total;
if (\is_null($total)) {
$auths['limit'] = 0;
} else {
$auths['limit'] = $total;
}
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'auths' => $auths,
])));
});
$updates = new Document([
'auths' => $auths,
]);
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents
->setParam('projectId', $project->getId())
@@ -60,7 +60,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -72,18 +71,13 @@ class Update extends Action
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
): void {
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $protocolId, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
$protocols = $project->getAttribute('apis', []);
$protocols[$protocolId] = $enabled;
$protocols = $project->getAttribute('apis', []);
$protocols[$protocolId] = $enabled;
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'apis' => $protocols,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'apis' => $protocols,
])));
$queueForEvents->setParam('protocolId', $protocolId);
@@ -72,7 +72,6 @@ class Update extends Action
->inject('dbForPlatform')
->inject('project')
->inject('authorization')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -91,101 +90,83 @@ class Update extends Action
Response $response,
Database $dbForPlatform,
Document $project,
Authorization $authorization,
callable $distributedLockOrFail,
Authorization $authorization
): void {
$inputs = [
'host' => $host,
'port' => $port,
'username' => $username,
'password' => $password,
'senderEmail' => $senderEmail,
'senderName' => $senderName,
'replyToEmail' => $replyToEmail,
'replyToName' => $replyToName,
'secure' => $secure,
'enabled' => $enabled,
];
// Fetch current configuration
$smtp = $project->getAttribute('smtp', []);
// The SMTP test (PHPMailer SmtpConnect with Timeout=5) runs inside the
// lock so two concurrent SMTP updates don't validate against the same
// baseline and overwrite each other's secrets. The 10s default lock
// TTL covers the worst-case 5s connection probe with margin.
$project = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $inputs, $enabled, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
// Apply changes
$keys = ['host', 'port', 'username', 'password', 'senderEmail', 'senderName', 'replyToEmail', 'replyToName', 'secure', 'enabled'];
foreach ($keys as $key) {
if (!\is_null(${$key})) {
$smtp[$key] = ${$key};
}
}
// Fetch current configuration
$smtp = $project->getAttribute('smtp', []);
// Backwards compatibility
$smtp['replyToEmail'] = $smtp['replyToEmail'] ?? $smtp['replyTo'] ?? '';
// Apply changes
foreach ($inputs as $key => $value) {
if (!\is_null($value)) {
$smtp[$key] = $value;
if (($smtp['enabled'] ?? false) === true) {
// Ensure required fields are set
$requiredKeys = ['host', 'port', 'senderEmail'];
foreach ($requiredKeys as $key) {
if (empty($smtp[$key])) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.');
}
}
}
// Backwards compatibility
$smtp['replyToEmail'] = $smtp['replyToEmail'] ?? $smtp['replyTo'] ?? '';
// Validate SMTP credentials
// Validate when the caller is explicitly enabling or hasn't expressed a preference
// (so a credentials-only PATCH can auto-enable). Skip only when the caller is
// explicitly keeping/turning SMTP off.
if (\is_null($enabled) || $enabled === true) {
$mail = new PHPMailer(true);
$mail->isSMTP();
if (($smtp['enabled'] ?? false) === true) {
// Ensure required fields are set
$requiredKeys = ['host', 'port', 'senderEmail'];
foreach ($requiredKeys as $key) {
if (empty($smtp[$key])) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.');
}
}
$mail->Host = $smtp['host'] ?? '';
$mail->Port = $smtp['port'] ?? '';
$mail->SMTPSecure = $smtp['secure'] ?? '';
$mail->setFrom($smtp['senderEmail'], $smtp['senderName'] ?? '');
if (!empty($smtp['username'] ?? '')) {
$mail->SMTPAuth = true;
$mail->Username = $smtp['username'];
$mail->Password = $smtp['password'] ?? '';
}
// Validate SMTP credentials
// Validate when the caller is explicitly enabling or hasn't expressed a preference
// (so a credentials-only PATCH can auto-enable). Skip only when the caller is
// explicitly keeping/turning SMTP off.
if (\is_null($enabled) || $enabled === true) {
$mail = new PHPMailer(true);
$mail->isSMTP();
$mail->Host = $smtp['host'] ?? '';
$mail->Port = $smtp['port'] ?? '';
$mail->SMTPSecure = $smtp['secure'] ?? '';
$mail->setFrom($smtp['senderEmail'], $smtp['senderName'] ?? '');
if (!empty($smtp['username'] ?? '')) {
$mail->SMTPAuth = true;
$mail->Username = $smtp['username'];
$mail->Password = $smtp['password'] ?? '';
}
if (!empty($smtp['replyToEmail'] ?? '')) {
$mail->addReplyTo($smtp['replyToEmail'], $smtp['replyToName'] ?? '');
}
$mail->SMTPAutoTLS = false;
$mail->Timeout = 5;
try {
$valid = $mail->SmtpConnect();
if (!$valid) {
throw new \Exception('Connection is not valid.');
}
// Auto-enable if configuration is valid
// Dont do this if specifically request to mark disabled
if (\is_null($enabled)) {
$smtp['enabled'] = true;
}
} catch (Throwable $error) {
if (($smtp['enabled'] ?? null) === true) {
throw new Exception(Exception::PROJECT_SMTP_CONFIG_INVALID, $error->getMessage());
}
}
if (!empty($smtp['replyToEmail'] ?? '')) {
$mail->addReplyTo($smtp['replyToEmail'], $smtp['replyToName'] ?? '');
}
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'smtp' => $smtp,
])));
});
$mail->SMTPAutoTLS = false;
$mail->Timeout = 5;
try {
$valid = $mail->SmtpConnect();
if (!$valid) {
throw new \Exception('Connection is not valid.');
}
// Auto-enable if configuration is valid
// Dont do this if specifically request to mark disabled
if (\is_null($enabled)) {
$smtp['enabled'] = true;
}
} catch (Throwable $error) {
if (($smtp['enabled'] ?? null) === true) {
throw new Exception(Exception::PROJECT_SMTP_CONFIG_INVALID, $error->getMessage());
}
}
}
// Save configuration
$updates = new Document([
'smtp' => $smtp,
]);
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$response->dynamic($project, Response::MODEL_PROJECT);
}
@@ -60,7 +60,6 @@ class Update extends Action
->inject('project')
->inject('authorization')
->inject('queueForEvents')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -71,26 +70,14 @@ class Update extends Action
Database $dbForPlatform,
Document $project,
Authorization $authorization,
Event $queueForEvents,
callable $distributedLockOrFail,
Event $queueForEvents
): void {
// The services map is a JSON object on the project document. Two
// concurrent service toggles read the same baseline, each set their
// own key, and the second updateDocument() overwrites the first —
// silent lost-update on a sparse write. Lock the project doc for
// 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("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', []);
$services[$serviceId] = $enabled;
$services = $project->getAttribute('services', []);
$services[$serviceId] = $enabled;
return $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'services' => $services,
])));
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'services' => $services,
])));
$queueForEvents->setParam('serviceId', $serviceId);
@@ -69,7 +69,6 @@ class Update extends Action
->inject('dbForPlatform')
->inject('authorization')
->inject('project')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
@@ -87,61 +86,47 @@ class Update extends Action
Database $dbForPlatform,
Authorization $authorization,
Document $project,
callable $distributedLockOrFail,
) {
$locale = $locale ?: System::getEnv('_APP_LOCALE', 'en');
$inputs = [
'senderName' => $senderName,
'senderEmail' => $senderEmail,
'replyToEmail' => $replyToEmail,
'replyToName' => $replyToName,
'message' => $message,
'subject' => $subject,
];
// Prevent template update if custom SMTP is not configured
$smtp = $project->getAttribute('smtp', []);
if (($smtp['enabled'] ?? false) !== true) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'SMTP must be enabled on the project to configure custom email templates.');
}
$template = $distributedLockOrFail("lock:platform:projects:{$project->getId()}", function () use ($project, $templateId, $locale, $inputs, $dbForPlatform, $authorization) {
$project = $authorization->skip(fn () => $dbForPlatform->getDocument('projects', $project->getId()));
// Fetch current configuration
$templates = $project->getAttribute('templates', []);
$template = $templates['email.' . $templateId . '-' . $locale] ?? [];
// Prevent template update if custom SMTP is not configured
$smtp = $project->getAttribute('smtp', []);
if (($smtp['enabled'] ?? false) !== true) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'SMTP must be enabled on the project to configure custom email templates.');
// Apply changes
$keys = ['senderName', 'senderEmail', 'replyToEmail', 'replyToName', 'message', 'subject'];
foreach ($keys as $key) {
if (!\is_null(${$key})) {
$template[$key] = ${$key};
}
}
// Fetch current configuration
$templates = $project->getAttribute('templates', []);
$template = $templates['email.' . $templateId . '-' . $locale] ?? [];
// Backwards compatibility
if (!\is_null($template['replyTo'] ?? null)) {
$template['replyToEmail'] = $template['replyToEmail'] ?? $template['replyTo'] ?? '';
}
// Apply changes
foreach ($inputs as $key => $value) {
if (!\is_null($value)) {
$template[$key] = $value;
}
// Ensure required fields are set
$requiredKeys = ['subject', 'message'];
foreach ($requiredKeys as $key) {
if (empty($template[$key])) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.');
}
}
// Backwards compatibility
if (!\is_null($template['replyTo'] ?? null)) {
$template['replyToEmail'] = $template['replyToEmail'] ?? $template['replyTo'] ?? '';
}
// Save configuration
$templates['email.' . $templateId . '-' . $locale] = $template;
$updates = new Document([
'templates' => $templates,
]);
// Ensure required fields are set
$requiredKeys = ['subject', 'message'];
foreach ($requiredKeys as $key) {
if (empty($template[$key])) {
throw new Exception(Exception::GENERAL_ARGUMENT_INVALID, 'Param "' . $key . '" is not optional.');
}
}
// Save configuration
$templates['email.' . $templateId . '-' . $locale] = $template;
$authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'templates' => $templates,
])));
return $template;
});
$project = $authorization->skip(fn () => $dbForPlatform->updateDocument('projects', $project->getId(), $updates));
$queueForEvents->setParam('templateId', $templateId);
@@ -55,37 +55,29 @@ class Update extends Action
->param('teamId', '', new UID(), 'Team ID of the team to transfer project to.')
->inject('response')
->inject('dbForPlatform')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
public function action(string $projectId, string $teamId, Response $response, Database $dbForPlatform, callable $distributedLockOrFail)
public function action(string $projectId, string $teamId, Response $response, Database $dbForPlatform)
{
// Lock around the project doc RMW. Cascade fan-out to installations,
// repositories and vcsComments runs after the lock is released —
// those write to separate collections, not the project doc.
[$project, $permissions] = $distributedLockOrFail("lock:platform:projects:{$projectId}", function () use ($projectId, $teamId, $dbForPlatform) {
$project = $dbForPlatform->getDocument('projects', $projectId);
$team = $dbForPlatform->getDocument('teams', $teamId);
$project = $dbForPlatform->getDocument('projects', $projectId);
$team = $dbForPlatform->getDocument('teams', $teamId);
if ($project->isEmpty()) {
throw new Exception(Exception::PROJECT_NOT_FOUND);
}
if ($project->isEmpty()) {
throw new Exception(Exception::PROJECT_NOT_FOUND);
}
if ($team->isEmpty()) {
throw new Exception(Exception::TEAM_NOT_FOUND);
}
if ($team->isEmpty()) {
throw new Exception(Exception::TEAM_NOT_FOUND);
}
$permissions = $this->getPermissions($teamId, $projectId);
$permissions = $this->getPermissions($teamId, $projectId);
$project = $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'teamId' => $teamId,
'teamInternalId' => $team->getSequence(),
'$permissions' => $permissions,
]));
return [$project, $permissions];
});
$project = $dbForPlatform->updateDocument('projects', $project->getId(), new Document([
'teamId' => $teamId,
'teamInternalId' => $team->getSequence(),
'$permissions' => $permissions,
]));
$installations = $dbForPlatform->find('installations', [
Query::equal('projectInternalId', [$project->getSequence()]),
@@ -65,36 +65,29 @@ class Update extends Action
->param('legalTaxId', '', new Text(256), 'Project legal tax ID. Max length: 256 chars.', true)
->inject('response')
->inject('dbForPlatform')
->inject('distributedLockOrFail')
->callback($this->action(...));
}
public function action(string $projectId, string $name, string $description, string $logo, string $url, string $legalName, string $legalCountry, string $legalState, string $legalCity, string $legalAddress, string $legalTaxId, Response $response, Database $dbForPlatform, callable $distributedLockOrFail)
public function action(string $projectId, string $name, string $description, string $logo, string $url, string $legalName, string $legalCountry, string $legalState, string $legalCity, string $legalAddress, string $legalTaxId, Response $response, Database $dbForPlatform)
{
// Re-fetch and write the full project doc inside the lock. This is the
// worst RMW window in the projects API — the endpoint passes the entire
// document back to updateDocument(), so a concurrent write to *any*
// attribute (services, auths, smtp, ...) would be silently overwritten.
$project = $distributedLockOrFail("lock:platform:projects:{$projectId}", function () use ($projectId, $name, $description, $logo, $url, $legalName, $legalCountry, $legalState, $legalCity, $legalAddress, $legalTaxId, $dbForPlatform) {
$project = $dbForPlatform->getDocument('projects', $projectId);
$project = $dbForPlatform->getDocument('projects', $projectId);
if ($project->isEmpty()) {
throw new Exception(Exception::PROJECT_NOT_FOUND);
}
if ($project->isEmpty()) {
throw new Exception(Exception::PROJECT_NOT_FOUND);
}
return $dbForPlatform->updateDocument('projects', $project->getId(), $project
->setAttribute('name', $name)
->setAttribute('description', $description)
->setAttribute('logo', $logo)
->setAttribute('url', $url)
->setAttribute('legalName', $legalName)
->setAttribute('legalCountry', $legalCountry)
->setAttribute('legalState', $legalState)
->setAttribute('legalCity', $legalCity)
->setAttribute('legalAddress', $legalAddress)
->setAttribute('legalTaxId', $legalTaxId)
->setAttribute('search', implode(' ', [$projectId, $name])));
});
$project = $dbForPlatform->updateDocument('projects', $project->getId(), $project
->setAttribute('name', $name)
->setAttribute('description', $description)
->setAttribute('logo', $logo)
->setAttribute('url', $url)
->setAttribute('legalName', $legalName)
->setAttribute('legalCountry', $legalCountry)
->setAttribute('legalState', $legalState)
->setAttribute('legalCity', $legalCity)
->setAttribute('legalAddress', $legalAddress)
->setAttribute('legalTaxId', $legalTaxId)
->setAttribute('search', implode(' ', [$projectId, $name])));
$response->dynamic($project, Response::MODEL_PROJECT);
}
+3 -12
View File
@@ -211,13 +211,7 @@ class Client
}
}
// CURLOPT_PATH_AS_IS isn't supported by Swoole's emulated cURL when the
// SWOOLE_HOOK_CURL coroutine hook is active. Skip it in that case so
// tests that need real parallel HTTP (Swoole\Coroutine\run + cURL hook)
// don't fatal-error here. Native (non-hooked) cURL keeps the option.
if (! \extension_loaded('swoole') || ! (\Swoole\Runtime::getHookFlags() & SWOOLE_HOOK_CURL)) {
curl_setopt($ch, CURLOPT_PATH_AS_IS, 1);
}
curl_setopt($ch, CURLOPT_PATH_AS_IS, 1);
curl_setopt($ch, CURLOPT_CUSTOMREQUEST, $method);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, $followRedirects);
@@ -243,12 +237,9 @@ class Client
if ($method === self::METHOD_HEAD) {
curl_setopt($ch, CURLOPT_NOBODY, true); // This is crucial for HEAD requests
curl_setopt($ch, CURLOPT_HEADER, false);
} else {
curl_setopt($ch, CURLOPT_NOBODY, false);
}
// Note: explicit CURLOPT_NOBODY=false on non-HEAD requests is redundant
// (false is cURL's default) and actively breaks Swoole's emulated cURL
// on PATCH-with-body — Swoole strips the body and the request reaches
// the server as a method-without-body, hitting the framework's 404
// fallback. Just skip the redundant set.
if ($method != self::METHOD_GET && $method != self::METHOD_HEAD) {
curl_setopt($ch, CURLOPT_POSTFIELDS, $query);
@@ -266,94 +266,6 @@ trait ServicesBase
$this->assertSame(true, $response['body']['serviceStatusForTeams']);
}
/**
* Concurrency test for the distributed-lock pilot on `updateProjectService`.
*
* Without locking, two concurrent toggles to different services on the
* same project both read the same baseline `services` map, each set their
* own key, and the second sparse `updateDocument()` overwrites the first
* — silent lost-update.
*
* The test fires N parallel PATCH calls via Swoole coroutines (with the
* SWOOLE_HOOK_CURL runtime hook enabled so the test Client's cURL calls
* yield to the scheduler). After all coroutines complete the project is
* refetched and the count of enabled services is compared against the
* count of HTTP 200 responses.
*
* To verify the test catches the bug:
* - `_APP_LOCKING_ENABLED=enabled` → must pass
* - `_APP_LOCKING_ENABLED=disabled` → must FAIL (lost updates)
*/
public function testConcurrentTogglesAllPersist(): void
{
$services = ['teams', 'storage', 'functions', 'sites', 'messaging'];
// Baseline: disable everything so the toggle direction is unambiguous.
// Done outside the coroutine context so it stays sequential and the
// resulting project state is a clean known-zero before the race.
foreach ($services as $service) {
$this->updateServiceStatus($service, false);
}
// Enable Swoole's cURL hook so the test Client's HTTP calls yield
// to the scheduler and the foreach below actually runs in parallel.
// Without this, coroutines serialize on cURL and the negative-case
// run (locking disabled) would falsely pass.
\Swoole\Runtime::enableCoroutine(\SWOOLE_HOOK_CURL);
$results = [];
try {
\Swoole\Coroutine\run(function () use ($services, &$results): void {
foreach ($services as $service) {
\Swoole\Coroutine::create(function () use ($service, &$results): void {
$response = $this->updateServiceStatus($service, true);
$results[$service] = $response['headers']['status-code'];
});
}
});
} finally {
// SWOOLE_HOOK_NONE isn't defined in some Swoole builds; pass 0
// (the integer value of "no hooks") to disable any hooks this
// test enabled so subsequent tests run with native cURL.
\Swoole\Runtime::enableCoroutine(0);
}
$successCount = count(array_filter($results, fn ($code) => $code === 200));
// Refetch the project and count which of the targeted services ended up
// enabled. The endpoint returns serviceStatusFor{Service} keys.
$project = $this->client->call(Client::METHOD_GET, '/projects/' . $this->getProject()['$id'], [
'content-type' => 'application/json',
'x-appwrite-project' => 'console',
'cookie' => 'a_session_console=' . $this->getRoot()['session'],
]);
$enabledCount = 0;
foreach ($services as $service) {
$key = 'serviceStatusFor' . ucfirst($service);
if (($project['body'][$key] ?? false) === true) {
$enabledCount++;
}
}
$this->assertGreaterThan(0, $successCount, 'At least one concurrent toggle should succeed');
$this->assertSame(
$successCount,
$enabledCount,
sprintf(
'Each successful concurrent toggle must persist. successCount=%d enabledCount=%d (lost-update detected — distributed lock not effective)',
$successCount,
$enabledCount,
),
);
// Cleanup: leave all targeted services enabled so subsequent tests
// see a known-good baseline.
foreach ($services as $service) {
$this->updateServiceStatus($service, true);
}
}
// Helpers
protected function updateServiceStatus(string $serviceId, bool $enabled, bool $authenticated = true): mixed