| @@ -6,55 +6,28 @@ | ||
| 6 | 6 | |
| 7 | 7 | defined('ABSPATH') || exit; |
| 8 | 8 | |
| 9 | 9 | /** |
| 10 | - * Safety rails for mutating MCP tools. Annotations are UX hints, not safety — | |
| 11 | - * this is where real protection lives for the writes that touch someone's | |
| 12 | - * calendar (create, reschedule, cancel). | |
| 10 | + * Safety rails for destructive MCP writes. Tool annotations are only hints; | |
| 11 | + * the real protection is here. | |
| 13 | 12 | * |
| 14 | - * Two mechanisms: | |
| 13 | + * 1. Confirm tokens. A dry_run returns a preview and a token bound to the | |
| 14 | + * record's current state and to the exact parameters previewed. Executing | |
| 15 | + * needs the token back with the same parameters, so an agent can't act on a | |
| 16 | + * record that has since changed, or run a different change than the one | |
| 17 | + * that was approved. | |
| 18 | + * 2. Idempotency keys. A retry with the same key returns the first result | |
| 19 | + * instead of booking twice. This must wrap the token check, see idempotent(). | |
| 15 | 20 | * |
| 16 | - * 1. Dry-run + confirmation token. A destructive action called with | |
| 17 | - * dry_run:true computes the effect, binds it to BOTH the target's current | |
| 18 | - * state (a fingerprint) AND the exact parameters that were previewed (a | |
| 19 | - * parameter digest), stashes a short-lived record, and returns a preview. | |
| 20 | - * To execute, the caller passes that confirm_token back with the SAME | |
| 21 | - * parameters. If the record changed in the meantime the fingerprint no | |
| 22 | - * longer matches; if the caller changed what it is asking for, the digest | |
| 23 | - * no longer matches. Either way we force a fresh preview — so an agent can | |
| 24 | - * neither act on a booking somebody else already moved, nor execute a | |
| 25 | - * different change from the one a human approved. | |
| 21 | + * Records live in wp_options rather than transients: INSERT IGNORE gives us an | |
| 22 | + * atomic claim (get + delete transient is a race), and an object-cache flush | |
| 23 | + * can't drop an idempotency record and let a duplicate write through. | |
| 26 | 24 | * |
| 27 | - * 2. Idempotency keys. The caller passes an idempotency_key; the first | |
| 28 | - * execution for that key is recorded, and a retry with the same key returns | |
| 29 | - * the first result instead of booking or emailing twice. This is the guard | |
| 30 | - * against an agent re-issuing a create after a timeout, so it MUST wrap the | |
| 31 | - * confirm-token check rather than sit inside it — see idempotent(). | |
| 25 | + * Contract: every ability annotated `destructive => true`, including Pro's, | |
| 26 | + * must treat dry_run as non-mutating and pass each write through confirm(). | |
| 27 | + * create-booking and manage-booking are the reference. The mcp:permissions | |
| 28 | + * gate dry-runs every destructive action and fails if any row count moves. | |
| 32 | 29 | * |
| 33 | - * Storage is a dedicated options-backed store rather than transients. Two | |
| 34 | - * reasons, both of which the transient API cannot give us: | |
| 35 | - * | |
| 36 | - * - Atomic claim. `INSERT IGNORE` against the unique index on `option_name` | |
| 37 | - * lets exactly one of N concurrent requests claim a key — the primitive core | |
| 38 | - * uses for its own locks. `get_transient()` followed by `delete_transient()` | |
| 39 | - * is a read-then-write race: two agents holding the same token both read it | |
| 40 | - * before either deletes, and both execute. (`add_option()` is not a | |
| 41 | - * substitute; see claim().) | |
| 42 | - * - Durability. An object-cache flush drops transients. Losing a confirm token | |
| 43 | - * degrades safely (a fresh dry-run is required); losing an idempotency | |
| 44 | - * record does not — it degrades into the duplicate write the key existed to | |
| 45 | - * prevent. | |
| 46 | - * | |
| 47 | - * CONTRACT (enforced by scripts/check-mcp-budget.php and | |
| 48 | - * scripts/check-mcp-permissions.php): every ability whose annotations include | |
| 49 | - * `destructive => true` MUST treat `dry_run` as non-mutating for EVERY action it | |
| 50 | - * exposes, and MUST route each mutating action through confirm() before | |
| 51 | - * mutating. `create-booking` and `manage-booking` are the reference | |
| 52 | - * implementations. When adding a new destructive ability — here or in Pro, which | |
| 53 | - * registers under the same namespace via fluent_booking/mcp_loaded — follow this | |
| 54 | - * contract; the permission-matrix gate calls every action of every destructive | |
| 55 | - * tool with dry_run:true and fails the build if any row count moves. | |
| 56 | - * | |
| 57 | 30 | * @since 2.2.6 |
| 58 | 31 | */ |
| 59 | 32 | class WriteGuard |
| 60 | 33 | { |
| @@ -61,39 +34,27 @@ | ||
| 61 | 34 | const CONFIRM_TTL = 300; // 5 minutes to confirm a previewed action. |
| 62 | 35 | |
| 63 | 36 | const IDEM_TTL = 86400; // remember an idempotency key for a day. |
| 64 | 37 | |
| 65 | - /** | |
| 66 | - * Option-name prefix for the record store. Kept short: option_name is | |
| 67 | - * indexed at 191 characters and every key here ends in an md5. | |
| 68 | - */ | |
| 38 | + // Kept short: option_name is indexed at 191 chars and every key ends in an md5. | |
| 69 | 39 | const STORE_PREFIX = 'fcal_mcp_g_'; |
| 70 | 40 | |
| 71 | 41 | const CONFIRM_NEXT_STEP = 'Call this tool again with EXACTLY the same parameters plus confirm_token (and an idempotency_key) to execute. Changing any parameter invalidates the token.'; |
| 72 | 42 | |
| 73 | 43 | /** |
| 74 | - * Build a dry-run preview response with a confirmation token bound to both | |
| 75 | - * the target's current state and the parameters being previewed. | |
| 44 | + * Build a dry-run preview with a confirm token. | |
| 76 | 45 | * |
| 77 | - * @param string $tool Ability name (namespacing the token). | |
| 78 | - * @param string $entityKey Stable id of the target, e.g. "booking:42". | |
| 79 | - * @param string $fingerprint A string capturing the mutable state we care | |
| 80 | - * about (e.g. "scheduled|2026-09-01 14:00:00"). | |
| 81 | - * If this differs at execute time, the token is | |
| 82 | - * rejected. | |
| 83 | - * @param array $preview The human/agent-facing preview payload. | |
| 84 | - * @param string $paramsDigest Digest of the parameters this preview | |
| 85 | - * describes, from paramsDigest(). If the caller | |
| 86 | - * executes with different parameters, the token | |
| 87 | - * is rejected. | |
| 46 | + * @param string $tool Ability name. | |
| 47 | + * @param string $entityKey Target id, e.g. "booking:42". | |
| 48 | + * @param string $fingerprint The target's mutable state; a change rejects the token. | |
| 49 | + * @param array $preview The preview payload. | |
| 50 | + * @param string $paramsDigest From paramsDigest(); different parameters reject the token. | |
| 88 | 51 | * |
| 89 | 52 | * @return array |
| 90 | 53 | */ |
| 91 | 54 | public static function preview($tool, $entityKey, $fingerprint, array $preview, $paramsDigest = '') |
| 92 | 55 | { |
| 93 | - // wp_generate_password draws from wp_rand, which prefers random_int; | |
| 94 | - // wp_generate_uuid4 falls back to mt_rand. For a token that gates a | |
| 95 | - // write, take the stronger source. | |
| 56 | + // wp_generate_password() uses random_int; wp_generate_uuid4() can fall back to mt_rand. | |
| 96 | 57 | $token = substr(wp_hash($tool . '|' . $entityKey . '|' . $fingerprint . '|' . wp_generate_password(32, false, false)), 0, 32); |
| 97 | 58 | |
| 98 | 59 | self::write(self::confirmKey($tool, $entityKey), [ |
| 99 | 60 | 'token' => $token, |
| @@ -109,10 +70,9 @@ | ||
| 109 | 70 | ]; |
| 110 | 71 | } |
| 111 | 72 | |
| 112 | 73 | /** |
| 113 | - * Validate a confirm_token against the target's current fingerprint and the | |
| 114 | - * parameters it was minted for. | |
| 74 | + * Check a confirm_token against the target's current state and parameters. | |
| 115 | 75 | * |
| 116 | 76 | * @param string $tool |
| 117 | 77 | * @param string $entityKey |
| 118 | 78 | * @param string $currentFingerprint |
| @@ -158,13 +118,10 @@ | ||
| 158 | 118 | ['next_step' => 'set dry_run:true'] |
| 159 | 119 | ); |
| 160 | 120 | } |
| 161 | 121 | |
| 162 | - // The token authorises the change that was PREVIEWED, not merely the | |
| 163 | - // record it was previewed against. Without this an agent could preview a | |
| 164 | - // cancellation with no refund, then execute the same cancellation with | |
| 165 | - // refund_payment:true on the strength of the operator's approval of the | |
| 166 | - // first one. | |
| 122 | + // The token approves the previewed change, not just the record. Otherwise | |
| 123 | + // a preview without a refund could be executed with refund_payment:true. | |
| 167 | 124 | if ((string) $stored['params'] !== (string) $paramsDigest) { |
| 168 | 125 | self::delete($key); |
| 169 | 126 | return MCPHelper::error( |
| 170 | 127 | 'parameters_changed', |
| @@ -172,17 +129,11 @@ | ||
| 172 | 129 | ['next_step' => 'set dry_run:true'] |
| 173 | 130 | ); |
| 174 | 131 | } |
| 175 | 132 | |
| 176 | - // One-shot, and atomically so: two concurrent requests holding the same | |
| 177 | - // token both reach this line, and claim() lets exactly one through. | |
| 178 | - // | |
| 179 | - // Keyed on the TOKEN, not on the entity. Keying it on the entity would | |
| 180 | - // make the marker outlive the token it describes and block the next | |
| 181 | - // legitimately-minted token for the rest of the TTL — so an agent that | |
| 182 | - // previewed and cancelled one booking could not preview and reschedule | |
| 183 | - // the same booking for another five minutes, and would be told its fresh | |
| 184 | - // token was "already used". | |
| 133 | + // Single use, claimed atomically so only one of two concurrent requests | |
| 134 | + // gets through. Keyed on the token rather than the booking, so a fresh | |
| 135 | + // token for the same booking isn't blocked for the rest of the TTL. | |
| 185 | 136 | if (!self::claim(self::usedKey($token), 1, self::CONFIRM_TTL)) { |
| 186 | 137 | return MCPHelper::error( |
| 187 | 138 | 'confirmation_expired', |
| 188 | 139 | __('That confirm_token has already been used. Run a fresh dry_run.', 'fluent-booking'), |
| @@ -195,39 +146,25 @@ | ||
| 195 | 146 | return true; |
| 196 | 147 | } |
| 197 | 148 | |
| 198 | 149 | /** |
| 199 | - * Run $fn at most once per idempotency key (per user + tool + entity). A | |
| 200 | - * repeat call with the same key on the SAME entity returns the first | |
| 201 | - * result. If no key is supplied, $fn runs normally (no dedupe) — keys are | |
| 202 | - * recommended but not forced. | |
| 150 | + * Run $fn at most once per idempotency key, scoped to user, tool and | |
| 151 | + * entity. Without a key, $fn just runs. | |
| 203 | 152 | * |
| 204 | - * ORDERING MATTERS. This must be the OUTERMOST wrapper on a destructive | |
| 205 | - * write, with the confirm() check inside $fn. The reverse — confirm() first, | |
| 206 | - * idempotency inside — cannot work: confirm() consumes the token, so the | |
| 207 | - * retry this method exists to absorb is rejected as `confirmation_expired` | |
| 208 | - * before the cached result is ever consulted, and the agent's recovery path | |
| 209 | - * is a fresh dry_run and a second booking. | |
| 153 | + * This must be the outermost wrapper, with confirm() inside $fn. The other | |
| 154 | + * way round, confirm() consumes the token and the retry fails before it | |
| 155 | + * ever reaches the cached result. | |
| 210 | 156 | * |
| 211 | - * The key is entity-scoped so reusing one idempotency_key across different | |
| 212 | - * records (e.g. "cancel-1" for two bookings) can't replay the first | |
| 213 | - * booking's result and silently skip the second mutation. | |
| 157 | + * Only a reference to the result is stored, never the response itself: the | |
| 158 | + * response holds attendee PII, and wp_options ends up in exports and | |
| 159 | + * staging clones. $replay rebuilds the response from the live record. | |
| 214 | 160 | * |
| 215 | - * WHAT IS STORED is a reference, never the response. The response carries | |
| 216 | - * BookingProjector::full() — unmasked email, phone, country, internal note | |
| 217 | - * and every answer the attendee gave — and wp_options is the table most | |
| 218 | - * likely to end up in a support export or a staging clone, where no | |
| 219 | - * exporter or eraser keyed on the booking tables would ever find it. The | |
| 220 | - * replay callback rebuilds the response from the live record instead. | |
| 221 | - * | |
| 222 | 161 | * @param string $tool |
| 223 | 162 | * @param string $entityKey |
| 224 | 163 | * @param string $key |
| 225 | 164 | * @param callable $fn |
| 226 | 165 | * @param string $paramsDigest |
| 227 | - * @param callable|null $replay Rebuilds the response from the stored | |
| 228 | - * reference. Without one a replay returns the | |
| 229 | - * reference itself. | |
| 166 | + * @param callable|null $replay Rebuilds the response from the stored reference. | |
| 230 | 167 | * |
| 231 | 168 | * @return mixed |
| 232 | 169 | */ |
| 233 | 170 | public static function idempotent($tool, $entityKey, $key, callable $fn, $paramsDigest = '', $replay = null) |
| @@ -241,14 +178,10 @@ | ||
| 241 | 178 | |
| 242 | 179 | $cached = self::read($cacheKey); |
| 243 | 180 | |
| 244 | 181 | if (is_array($cached) && array_key_exists('ref', $cached)) { |
| 245 | - // A key identifies one attempt at one change, not a licence to skip | |
| 246 | - // any later change. Reusing a key with DIFFERENT parameters — say a | |
| 247 | - // second reschedule of the same booking to a new time — would | |
| 248 | - // otherwise return the first call's success and quietly perform no | |
| 249 | - // move at all, which is the worst of both worlds: the agent is told | |
| 250 | - // it worked and nothing happened. | |
| 182 | + // Same key, different parameters: refuse, or a second reschedule | |
| 183 | + // would report success without moving anything. | |
| 251 | 184 | if ((string) Arr::get($cached, 'params', '') !== (string) $paramsDigest) { |
| 252 | 185 | return MCPHelper::error( |
| 253 | 186 | 'idempotency_conflict', |
| 254 | 187 | __('This idempotency_key was already used for a different request. Use a fresh key for a new change; reuse a key only when retrying the identical call.', 'fluent-booking'), |
| @@ -268,11 +201,9 @@ | ||
| 268 | 201 | |
| 269 | 202 | return self::flagReplay(MCPHelper::success($ref)); |
| 270 | 203 | } |
| 271 | 204 | |
| 272 | - // Claim the key before running, not after. get-then-set would let two | |
| 273 | - // concurrent retries of the same request both miss and both execute, | |
| 274 | - // which is the failure the key exists to prevent. | |
| 205 | + // Claim before running, so two concurrent retries can't both execute. | |
| 275 | 206 | if (!self::claim($lockKey, 1, 120)) { |
| 276 | 207 | return MCPHelper::error( |
| 277 | 208 | 'in_progress', |
| 278 | 209 | __('Another call with this idempotency_key is still running. Wait for it to finish rather than retrying — retrying is what this key exists to make safe.', 'fluent-booking'), |
| @@ -279,18 +210,14 @@ | ||
| 279 | 210 | ['next_step' => 'poll with get-booking, or retry in a few seconds'] |
| 280 | 211 | ); |
| 281 | 212 | } |
| 282 | 213 | |
| 283 | - // Everything from here to the release is inside try/finally, recording | |
| 284 | - // the result included: a mutation that succeeded and a record that was | |
| 285 | - // never written is exactly the divergence the key exists to prevent, so | |
| 286 | - // a failure to persist has to be reported rather than swallowed. | |
| 287 | 214 | try { |
| 288 | 215 | $result = $fn(); |
| 289 | 216 | |
| 217 | + // Only successes are recorded, so a failure stays retryable. If the | |
| 218 | + // record can't be written, say so rather than invite a duplicate retry. | |
| 290 | 219 | if (!is_wp_error($result)) { |
| 291 | - // Only successful results are recorded — a failure should stay | |
| 292 | - // retryable with the same key. | |
| 293 | 220 | if (!self::write($cacheKey, ['ref' => self::resultRef($result), 'params' => (string) $paramsDigest], self::IDEM_TTL) |
| 294 | 221 | && is_array($result)) { |
| 295 | 222 | $result['idempotency_warning'] = __('This change was applied, but the idempotency record could not be stored. Do not retry with the same key — check the result before acting again.', 'fluent-booking'); |
| 296 | 223 | } |
| @@ -302,15 +229,10 @@ | ||
| 302 | 229 | } |
| 303 | 230 | } |
| 304 | 231 | |
| 305 | 232 | /** |
| 306 | - * Mark a response as a replay, in `meta` and nowhere else. | |
| 233 | + * Mark a response as a replay. Always in `meta`, so agents check one place. | |
| 307 | 234 | * |
| 308 | - * The two return paths above used to place it differently — the rebuilt one | |
| 309 | - * merged into the envelope, the reference one into `data` — so an agent | |
| 310 | - * checking one place missed the other and re-issued a write it had already | |
| 311 | - * made, which is the failure the key exists to prevent. | |
| 312 | - * | |
| 313 | 235 | * @param array $response |
| 314 | 236 | * |
| 315 | 237 | * @return array |
| 316 | 238 | */ |
| @@ -339,11 +261,9 @@ | ||
| 339 | 261 | } |
| 340 | 262 | |
| 341 | 263 | $ref = []; |
| 342 | 264 | |
| 343 | - // A whitelist of identity and outcome fields, all scalar and none of | |
| 344 | - // them attendee data. Anything richer is rebuilt by the replay | |
| 345 | - // callback from the live record. | |
| 265 | + // Identity and outcome fields only, no attendee data. | |
| 346 | 266 | foreach (['id', 'action', 'created', 'message'] as $key) { |
| 347 | 267 | if (isset($result['data'][$key]) && is_scalar($result['data'][$key])) { |
| 348 | 268 | $ref[$key] = $result['data'][$key]; |
| 349 | 269 | } |
| @@ -358,15 +278,12 @@ | ||
| 358 | 278 | return $ref; |
| 359 | 279 | } |
| 360 | 280 | |
| 361 | 281 | /** |
| 362 | - * A stable digest of the parameters that actually change what a call does. | |
| 282 | + * A stable digest of the parameters that change what a call does. The | |
| 283 | + * control parameters are left out, since they differ between a preview, | |
| 284 | + * its execution and a retry. | |
| 363 | 285 | * |
| 364 | - * The three control parameters are excluded by definition: `dry_run` differs | |
| 365 | - * between the preview and the execution, `confirm_token` is absent from the | |
| 366 | - * preview, and `idempotency_key` legitimately varies between a call and its | |
| 367 | - * retry. Everything else is binding. | |
| 368 | - * | |
| 369 | 286 | * @param array $params |
| 370 | 287 | * @param array $ignore Extra keys to exclude. |
| 371 | 288 | * |
| 372 | 289 | * @return string |
| @@ -382,10 +299,9 @@ | ||
| 382 | 299 | return md5((string) wp_json_encode(self::canonicalize($params))); |
| 383 | 300 | } |
| 384 | 301 | |
| 385 | 302 | /** |
| 386 | - * Recursively sort keys so an agent that emits the same parameters in a | |
| 387 | - * different order still matches its own preview. | |
| 303 | + * Sort keys recursively so parameter order doesn't break the match. | |
| 388 | 304 | * |
| 389 | 305 | * @param mixed $value |
| 390 | 306 | * @return mixed |
| 391 | 307 | */ |
| @@ -409,11 +325,10 @@ | ||
| 409 | 325 | return $out; |
| 410 | 326 | } |
| 411 | 327 | |
| 412 | 328 | /** |
| 413 | - * Fingerprint for a booking: everything a caller could act on stale. | |
| 414 | - * Deliberately includes updated_at so an edit we do not otherwise model | |
| 415 | - * (a note change, a payment transition) still invalidates a pending token. | |
| 329 | + * A booking's fingerprint. updated_at catches edits we don't track | |
| 330 | + * separately, like a note or payment change. | |
| 416 | 331 | * |
| 417 | 332 | * @param \FluentBooking\App\Models\Booking $booking |
| 418 | 333 | * |
| 419 | 334 | * @return string |
| @@ -428,21 +343,15 @@ | ||
| 428 | 343 | : $booking->updated_at, |
| 429 | 344 | ]); |
| 430 | 345 | } |
| 431 | 346 | |
| 432 | - /** | |
| 433 | - * How many rows one SELECT of the purge reads, and how many such passes it | |
| 434 | - * makes before giving up for the day. The product is the ceiling on a | |
| 435 | - * single run: enough for a busy site, bounded enough that the daily task | |
| 436 | - * cannot outrun a 30-second Action Scheduler tick and be killed mid-sweep. | |
| 437 | - */ | |
| 347 | + // Batch size x passes caps one run, to stay inside a 30s Action Scheduler tick. | |
| 438 | 348 | const PURGE_BATCH = 500; |
| 439 | 349 | |
| 440 | 350 | const PURGE_MAX_PASSES = 40; |
| 441 | 351 | |
| 442 | 352 | /** |
| 443 | - * Drop every expired record. Wired to the daily scheduler — the store is | |
| 444 | - * options-backed, so unlike transients nothing prunes it for us. | |
| 353 | + * Delete expired records. Runs daily, since nothing else prunes options. | |
| 445 | 354 | * |
| 446 | 355 | * @return int rows removed |
| 447 | 356 | */ |
| 448 | 357 | public static function purgeExpired() |
| @@ -453,11 +362,9 @@ | ||
| 453 | 362 | $now = time(); |
| 454 | 363 | $offset = 0; |
| 455 | 364 | |
| 456 | 365 | for ($pass = 0; $pass < self::PURGE_MAX_PASSES; $pass++) { |
| 457 | - // option_value comes back with the name. Reading it here rather | |
| 458 | - // than calling get_option() per row turns three queries a row into | |
| 459 | - // one query a batch. | |
| 366 | + // Read values in the same query instead of get_option() per row. | |
| 460 | 367 | $rows = $wpdb->get_results( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching |
| 461 | 368 | $wpdb->prepare( |
| 462 | 369 | "SELECT option_name, option_value FROM {$wpdb->options} WHERE option_name LIKE %s OR option_name LIKE %s ORDER BY option_id ASC LIMIT %d OFFSET %d", |
| 463 | 370 | $wpdb->esc_like(self::STORE_PREFIX) . '%', |
| @@ -476,20 +383,14 @@ | ||
| 476 | 383 | |
| 477 | 384 | foreach ($rows as $row) { |
| 478 | 385 | $record = maybe_unserialize($row['option_value']); |
| 479 | 386 | |
| 480 | - // Delete only rows whose stored expiry is genuinely in the | |
| 481 | - // past, and re-check the value we just read rather than | |
| 482 | - // trusting the name alone: a preview that renewed the record | |
| 483 | - // between the SELECT and the DELETE would otherwise have its | |
| 484 | - // fresh token swept away. | |
| 387 | + // Keep live records. Rows without a valid envelope can never | |
| 388 | + // be used, so they go too. | |
| 485 | 389 | if (is_array($record) && !empty($record['expires']) && $record['expires'] >= $now) { |
| 486 | 390 | continue; |
| 487 | 391 | } |
| 488 | 392 | |
| 489 | - // A row with no usable envelope is a leftover from an older | |
| 490 | - // format or a partial write; it can never be honoured, so it | |
| 491 | - // goes too. | |
| 492 | 393 | $expired[] = $row['option_name']; |
| 493 | 394 | } |
| 494 | 395 | |
| 495 | 396 | if ($expired) { |
| @@ -503,10 +404,9 @@ | ||
| 503 | 404 | self::forgetCached($name); |
| 504 | 405 | } |
| 505 | 406 | } |
| 506 | 407 | |
| 507 | - // Rows that survived stay in the table, so the next batch has to | |
| 508 | - // start past them rather than re-reading the same live records. | |
| 408 | + // Skip past the rows we kept. | |
| 509 | 409 | $offset += count($rows) - count($expired); |
| 510 | 410 | |
| 511 | 411 | if (count($rows) < self::PURGE_BATCH) { |
| 512 | 412 | break; |
| @@ -518,23 +418,26 @@ | ||
| 518 | 418 | return $removed; |
| 519 | 419 | } |
| 520 | 420 | |
| 521 | 421 | /** |
| 522 | - * Atomic claim: exactly one of N concurrent callers gets true. | |
| 422 | + * Take a short exclusive window for one repeatable action. | |
| 523 | 423 | * |
| 524 | - * `INSERT IGNORE` against the unique index on `option_name`, which is the | |
| 525 | - * primitive WordPress core itself uses for locking | |
| 526 | - * (`WP_Upgrader::create_lock()`). Notably NOT `add_option()`: that looks | |
| 527 | - * atomic and is not. Core checks existence first and then issues | |
| 424 | + * @param string $key caller-scoped identifier | |
| 425 | + * @param int $ttl seconds the window lasts | |
| 528 | 426 | * |
| 529 | - * INSERT ... ON DUPLICATE KEY UPDATE option_value = VALUES(option_value) | |
| 427 | + * @return bool true when the caller may proceed | |
| 428 | + */ | |
| 429 | + public static function cooldown($key, $ttl) | |
| 430 | + { | |
| 431 | + return self::claim(self::STORE_PREFIX . 'cd_' . md5($key), 1, $ttl); | |
| 432 | + } | |
| 433 | + | |
| 434 | + /** | |
| 435 | + * Atomic claim: exactly one of N concurrent callers gets true. | |
| 530 | 436 | * |
| 531 | - * so a second caller whose row already exists performs an UPDATE, changes | |
| 532 | - * the value (our expiry differs), gets a non-zero affected-row count, and is | |
| 533 | - * told it took the claim. Two callers, two `true`s, no mutual exclusion — | |
| 534 | - * and for a token consumption or a refund that is the whole ballgame. | |
| 535 | - * `INSERT IGNORE` returns 0 rows when the key exists, which is the answer we | |
| 536 | - * actually need. | |
| 437 | + * Uses INSERT IGNORE, as core's WP_Upgrader::create_lock() does. Not | |
| 438 | + * add_option(): it runs INSERT ... ON DUPLICATE KEY UPDATE, so a second | |
| 439 | + * caller updates the row and is also told it succeeded. | |
| 537 | 440 | * |
| 538 | 441 | * @param string $key |
| 539 | 442 | * @param mixed $value |
| 540 | 443 | * @param int $ttl |
| @@ -540,26 +443,12 @@ | ||
| 540 | 443 | * @param int $ttl |
| 541 | 444 | * |
| 542 | 445 | * @return bool true when this caller took the claim |
| 543 | 446 | */ |
| 544 | - /** | |
| 545 | - * Take a short exclusive window for one repeatable action. | |
| 546 | - * | |
| 547 | - * @param string $key caller-scoped identifier | |
| 548 | - * @param int $ttl seconds the window lasts | |
| 549 | - * | |
| 550 | - * @return bool true when the caller may proceed | |
| 551 | - */ | |
| 552 | - public static function cooldown($key, $ttl) | |
| 553 | - { | |
| 554 | - return self::claim(self::STORE_PREFIX . 'cd_' . md5($key), 1, $ttl); | |
| 555 | - } | |
| 556 | - | |
| 557 | 447 | private static function claim($key, $value, $ttl) |
| 558 | 448 | { |
| 559 | - // A stale claim must not block forever: clear an expired one, then try. | |
| 560 | - // Deliberately before the insert and never after — stealing a claim we | |
| 561 | - // did not place is how one caller frees another caller's live lock. | |
| 449 | + // Clear an expired claim before inserting, never after, so we can't | |
| 450 | + // remove a lock someone else just took. | |
| 562 | 451 | $existing = self::readRaw($key); |
| 563 | 452 | |
| 564 | 453 | if (is_array($existing) && !empty($existing['expires']) && $existing['expires'] < time()) { |
| 565 | 454 | self::delete($key); |
| @@ -584,10 +473,9 @@ | ||
| 584 | 473 | maybe_serialize($record) |
| 585 | 474 | ) |
| 586 | 475 | ); |
| 587 | 476 | |
| 588 | - // The row went in behind the options cache's back, so a `notoptions` | |
| 589 | - // entry saying it does not exist has to go. | |
| 477 | + // We bypassed the options API, so clear any stale `notoptions` entry. | |
| 590 | 478 | self::forgetCached($key); |
| 591 | 479 | |
| 592 | 480 | return (bool) $inserted; |
| 593 | 481 | } |
| @@ -609,10 +497,9 @@ | ||
| 609 | 497 | } |
| 610 | 498 | } |
| 611 | 499 | |
| 612 | 500 | /** |
| 613 | - * The stored record with its envelope, without the expiry check read() | |
| 614 | - * applies. Used where the expiry itself is the thing being inspected. | |
| 501 | + * The stored record and its envelope, without read()'s expiry check. | |
| 615 | 502 | * |
| 616 | 503 | * @param string $key |
| 617 | 504 | * @return array|null |
| 618 | 505 | */ |
| @@ -659,11 +546,9 @@ | ||
| 659 | 546 | $updated = update_option($key, $record, false); |
| 660 | 547 | |
| 661 | 548 | self::forgetCached($key); |
| 662 | 549 | |
| 663 | - // update_option() returns false when the stored value is already | |
| 664 | - // identical, which is a success for our purposes — so confirm by | |
| 665 | - // reading rather than trusting the return. | |
| 550 | + // update_option() returns false for an unchanged value, so read it back. | |
| 666 | 551 | if ($updated) { |
| 667 | 552 | return true; |
| 668 | 553 | } |
| 669 | 554 | |
| @@ -683,10 +568,9 @@ | ||
| 683 | 568 | } |
| 684 | 569 | |
| 685 | 570 | private static function confirmKey($tool, $entityKey) |
| 686 | 571 | { |
| 687 | - // User-scoped: a token minted by one operator/session can't be consumed | |
| 688 | - // by another, even for the same booking. | |
| 572 | + // Per user, so one user can't consume another user's token. | |
| 689 | 573 | return self::STORE_PREFIX . 'c' . get_current_user_id() . '_' . md5($tool . '|' . $entityKey); |
| 690 | 574 | } |
| 691 | 575 | |
| 692 | 576 | private static function idemKey($tool, $entityKey, $key) |
| @@ -693,12 +577,9 @@ | ||
| 693 | 577 | { |
| 694 | 578 | return self::STORE_PREFIX . 'i' . get_current_user_id() . '_' . md5($tool . '|' . $entityKey . '|' . $key); |
| 695 | 579 | } |
| 696 | 580 | |
| 697 | - /** | |
| 698 | - * The one-shot marker for a single token. Tokens are already unguessable and | |
| 699 | - * user-scoped, so the token alone identifies the consumption. | |
| 700 | - */ | |
| 581 | + // Tokens are unguessable and per user, so the token alone is a unique key. | |
| 701 | 582 | private static function usedKey($token) |
| 702 | 583 | { |
| 703 | 584 | return self::STORE_PREFIX . 'u_' . md5((string) $token); |
| 704 | 585 | } |
