fix(cloud): deterministic event ordering + zone concurrent-delete detection
Self-review (Task 16) follow-up fixes: - PdoEventRepository::recent(): add event_id DESC as a secondary sort key after occurred_at DESC so ties (equal device-reported timestamps) resolve deterministically, matching InMemoryEventRepository's insertion-order tie-break. - PdoZoneRepository::updateOwned(): after the ownership-scoped UPDATE, disambiguate a 0-rowCount() result (MySQL reports "changed" rows, not "matched" rows) with a targeted existence check, so a genuine no-op resubmit (identical level/polygon) still succeeds while a zone deleted concurrently between the ownership check and the UPDATE now throws instead of save() silently reporting success for a zone that no longer exists. Full suite still green: 59 tests, 114 assertions, 4 skipped, 0 failures, 0 warnings/deprecations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ne4rU4FToTLHcBUMUvUnT1
This commit is contained in:
parent
9aa7f44ea4
commit
c1fb22ee16
2 changed files with 21 additions and 3 deletions
|
|
@ -42,7 +42,7 @@ final class PdoEventRepository implements EventRepository
|
||||||
$st = $this->pdo->prepare(
|
$st = $this->pdo->prepare(
|
||||||
'SELECT ' . self::SELECT_COLUMNS . '
|
'SELECT ' . self::SELECT_COLUMNS . '
|
||||||
FROM events WHERE severity = ?
|
FROM events WHERE severity = ?
|
||||||
ORDER BY occurred_at DESC LIMIT ?'
|
ORDER BY occurred_at DESC, event_id DESC LIMIT ?'
|
||||||
);
|
);
|
||||||
$st->bindValue(1, $severity, PDO::PARAM_STR);
|
$st->bindValue(1, $severity, PDO::PARAM_STR);
|
||||||
$st->bindValue(2, $limit, PDO::PARAM_INT);
|
$st->bindValue(2, $limit, PDO::PARAM_INT);
|
||||||
|
|
@ -50,7 +50,7 @@ final class PdoEventRepository implements EventRepository
|
||||||
$st = $this->pdo->prepare(
|
$st = $this->pdo->prepare(
|
||||||
'SELECT ' . self::SELECT_COLUMNS . '
|
'SELECT ' . self::SELECT_COLUMNS . '
|
||||||
FROM events
|
FROM events
|
||||||
ORDER BY occurred_at DESC LIMIT ?'
|
ORDER BY occurred_at DESC, event_id DESC LIMIT ?'
|
||||||
);
|
);
|
||||||
$st->bindValue(1, $limit, PDO::PARAM_INT);
|
$st->bindValue(1, $limit, PDO::PARAM_INT);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -64,7 +64,12 @@ final class PdoZoneRepository implements ZoneRepository
|
||||||
* before the UPDATE (rather than trusting UPDATE's rowCount()) because
|
* before the UPDATE (rather than trusting UPDATE's rowCount()) because
|
||||||
* MySQL reports "changed rows", not "matched rows" -- a no-op update
|
* MySQL reports "changed rows", not "matched rows" -- a no-op update
|
||||||
* (identical level/polygon resubmitted) would otherwise report 0 affected
|
* (identical level/polygon resubmitted) would otherwise report 0 affected
|
||||||
* rows and be indistinguishable from an ownership mismatch.
|
* rows and be indistinguishable from an ownership mismatch. The UPDATE's
|
||||||
|
* rowCount() is still checked afterwards (0 there is unambiguous, since
|
||||||
|
* we already know the row existed a moment ago) to catch the narrow race
|
||||||
|
* where the zone is deleted between the ownership check and the UPDATE --
|
||||||
|
* without this, save() would return success for a zone that no longer
|
||||||
|
* exists.
|
||||||
*/
|
*/
|
||||||
private function updateOwned(
|
private function updateOwned(
|
||||||
string $zoneId,
|
string $zoneId,
|
||||||
|
|
@ -86,6 +91,19 @@ final class PdoZoneRepository implements ZoneRepository
|
||||||
WHERE zone_id = ? AND device_id = ? AND camera_id = ?'
|
WHERE zone_id = ? AND device_id = ? AND camera_id = ?'
|
||||||
);
|
);
|
||||||
$st->execute([$level, $polygonJson, time(), $zoneId, $deviceId, $cameraId]);
|
$st->execute([$level, $polygonJson, time(), $zoneId, $deviceId, $cameraId]);
|
||||||
|
if ($st->rowCount() === 0) {
|
||||||
|
// Ambiguous on its own: MySQL's rowCount() reports "changed" rows,
|
||||||
|
// not "matched" rows, so 0 here could mean either (a) a genuine
|
||||||
|
// no-op update (identical level/polygon resubmitted -- not an
|
||||||
|
// error) or (b) the zone was deleted between the ownership check
|
||||||
|
// above and this UPDATE. Disambiguate with a targeted existence
|
||||||
|
// check instead of assuming either case.
|
||||||
|
$stillExists = $this->pdo->prepare('SELECT 1 FROM zones WHERE zone_id = ? LIMIT 1');
|
||||||
|
$stillExists->execute([$zoneId]);
|
||||||
|
if ($stillExists->fetchColumn() === false) {
|
||||||
|
throw new RuntimeException('zone was deleted concurrently');
|
||||||
|
}
|
||||||
|
}
|
||||||
return $zoneId;
|
return $zoneId;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue