Skip to content

Make the current client the source of truth for the client id - #21

Open
loevgaard wants to merge 1 commit into
2.xfrom
client-resolution
Open

loevgaard wants to merge 1 commit into
2.xfrom
client-resolution

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Summary

The current client is now the source of truth for the client id. The client cookie always holds the id of the client that ClientContextInterface returns, decorators included. See ADR 0001.

 ClientContextInterface
-  [app decorators, priority 0: outside the memo]
-  CachedClientContext                memo cleared by kernel.reset
-    CookieBasedClientContext
-      CachedCookieProvider           WeakMap per Request
-        RequestBasedCookieProvider   parses + validates the cookie
-      DefaultClientContext           new client
+  CachedClientContext                WeakMap per Request, priority -1024: wraps app decorators
+    [app decorators]
+      ClientContext                  cookie → client, else new client; no request → anonymous client
+        ClientCookieJar::read()      @internal, owns name/format/validation/expiry

 StoreCookieSubscriber (kernel.response)
   PreStoreCookieEvent veto
-  cookie = cookieProvider.getCookie() ?? new Cookie(context.getClient().id)   // cookie wins
-  write cookie (bump last seen)
+  cookieJar.write(request, response, context.getClient())                    // current client wins
+    same id as the request's cookie → keep first seen, bump last seen
+    other id                        → fresh timestamps
+    empty or > 255 bytes            → LogicException

StoreCookieSubscriber, StoreMetadataSubscriber and ClientValueResolver now inject the interface alias, so they see app decorators. Also adds GLOSSARY.md, docs/adr/0001-… and UPGRADE-2.0.md, updates the README and CLAUDE.md, and drops symfony/service-contracts.

Evidence

  • Before (1.x): DecoratedClientContextTest, run against 1.x with a decorator that names the client by a query parameter:
    ✘ gives the request the client of the decorator            cookie id '01a19ca1-…' ≠ 'decorated-client-id'
    ✘ stores the decorator's client id in place of the cookie   cookie id 'cookie-client-id' ≠ 'decorated-client-id'
    ✘ stores metadata set on a decorator's per-call client      no row stored
    ✘ refuses a client id the cookie can't hold                 no exception
    
    After: all 4 pass. Moving the memo inside the decorator (priority 1024) makes the metadata test fail again.
  • AnonymousClientTest: outside a request, two calls give two anonymous clients.
  • Full suite on PHP 8.4: 112 tests OK, PHPStan max clean, ECS clean, Infection MSI 100% (95/95 mutants killed), composer validate --strict and composer normalize --dry-run clean, dependency analyser clean. The CI matrix (8.1/8.2, lowest) wasn't run locally.

Merge Danger

Door: two-way

It targets the unreleased 2.x line, so a revert undoes it. The BC check is skipped for PRs into 2.x.

Blast Radius: consumers

When 2.0.0 ships, apps that use CookieProviderInterface or the removed context classes break. Apps whose decorators return a different client id will see the cookie rewritten. Code outside a request that relied on getting the same anonymous client from repeated calls gets a new one each call. UPGRADE-2.0.md covers each of these.

The client cookie now always holds the client id of the current client,
i.e. what ClientContextInterface returns, decorators included. Before, a
request with a cookie kept the cookie's id and the cookie writer never
asked the client context.

ClientContext recognizes the client by the cookie of the main request and
replaces CookieBasedClientContext and DefaultClientContext.
CachedClientContext decorates the interface with priority -1024, so it
wraps the application's decorators, and keeps the clients in a WeakMap
keyed by the request, so it needs no kernel.reset. An internal
ClientCookieJar owns reading and writing the cookie and replaces the
cookie providers and CookieProviderInterface. Without a request, every
call gets an anonymous client.

Adds GLOSSARY.md, ADR 0001 and UPGRADE-2.0.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant