diff --git a/dev/test-distributed-lock.sh b/dev/test-distributed-lock.sh new file mode 100755 index 0000000000..6a740a4169 --- /dev/null +++ b/dev/test-distributed-lock.sh @@ -0,0 +1,157 @@ +#!/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= \ +# APPWRITE_API_KEY= \ +# ./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 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" diff --git a/tests/e2e/Client.php b/tests/e2e/Client.php index 4358058fe3..84c937d2d1 100644 --- a/tests/e2e/Client.php +++ b/tests/e2e/Client.php @@ -211,7 +211,13 @@ class Client } } - curl_setopt($ch, CURLOPT_PATH_AS_IS, 1); + // 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_CUSTOMREQUEST, $method); curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1); curl_setopt($ch, CURLOPT_FOLLOWLOCATION, $followRedirects); @@ -237,9 +243,12 @@ 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); diff --git a/tests/e2e/Services/Project/ServicesBase.php b/tests/e2e/Services/Project/ServicesBase.php index b5f94f8181..c8c7bd173f 100644 --- a/tests/e2e/Services/Project/ServicesBase.php +++ b/tests/e2e/Services/Project/ServicesBase.php @@ -266,6 +266,94 @@ 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