Make spending a public link's view the thing that guards it

[blaipr]

A public link issued for a single view could be followed twice.

The counter increment was already atomic — countViews + 1 in SQL, which is what stops a lost update — but the limit was checked in PHP, against a row that had already been read:

if ($publicLink !== null
    && time() < $publicLink->getDateExpire()
    && $publicLink->getCountViews() < $publicLink->getMaxCountViews()   // read
) {
    $this->publicLinkService->addLinkView($publicLink);                 // then write

Two requests arriving together both read countViews = max - 1, both pass, and both are served. An atomic increment prevents the counter going wrong; it never prevented exceeding the limit.

The guard moves into the update

addLinkView() now carries both conditions:

WHERE hash = :hash AND COALESCE(countViews, 0) < maxCountViews AND dateExpire > :now

and reports whether a row was affected. The controller serves only if the view was actually spent, so the moment the link is charged for and the moment it is handed over are the same moment.

COALESCE because countViews is nullable: a NULL would make the comparison NULL and refuse every link nobody had followed yet.

The password-reset flow already does exactly this — toggleUsedByHash() is a conditional update (… AND used = 0) that throws when it affects nothing, before the password is changed. The pattern was in the codebase; this path predated it.

Tests

The race is pinned deterministically, without concurrency. A second simultaneous request is precisely a process holding a stale copy of the row, so the test reads the link, exhausts it through the normal path, and then presents that copy. Under the old code the copy still said "views remaining" and spent the link past its limit.

The same for an expiry that passes while a request is in flight.

followLink() in the round-trip test replicates ViewLinkController, so it follows the controller's new shape — the guard is the return value of spending the view, not a pair of comparisons before it.

Reverting the SQL guard fails four tests, one of which shows an exhausted link handing back the plaintext password.

A harness note

Moving the guard out of PHP broke two refusal tests in ViewLinkRefusalsTest, correctly: they run on the mocked-database harness, which answers every update with "one row affected", so with the check now in SQL the double stopped refusing anything. The double now answers the way a server would. The link read had to move into the same closure, because databaseQueryResolver is consulted first and short-circuits the mapper resolvers.

Known limit, left in place

The useInfo usage list is still assembled from the row as it was read, so two views landing together record one entry between them. The counter is exact — that is what decides access — while the list is a log. Recorded at the call site.

3965 unit tests pass; PHPStan level 6 on src and PHPCS clean.