diff --git a/CLAUDE.md b/CLAUDE.md index 2548ae2..56218bf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,20 +33,21 @@ Branches: `2.x` (the default) is the next major and may break BC; `1.x` is the 1 The core abstraction is `ClientContextInterface::getClient(): Client`. Controllers obtain the current `Setono\Client\Client` either by autowiring `ClientContextInterface`, or — more commonly — by type-hinting `Setono\Client\Client` directly on a controller argument, which `Controller/ClientValueResolver` resolves via the context. -### Decorator chains (the key structural idea) +### Service wiring (decoration) -Three concerns are each built as a **Symfony service-decoration chain** in `config/services.xml`. Higher `decoration-priority` is applied first and ends up *innermost*; the public alias resolves to the outermost decorator. Understanding the resolution order is essential before changing any service wiring: +Two concerns are built with **Symfony service decoration** in `config/services.xml`. Higher `decoration-priority` is applied first and ends up *innermost*; the decorated id resolves to the outermost decorator. Understanding the resolution order is essential before changing any service wiring: -- **Client context** — runtime order outermost→innermost: `CachedClientContext` (prio 32) → `CookieBasedClientContext` (prio 64) → `DefaultClientContext`. - - `Cached` memoizes the `Client` for the request. It implements `ResetInterface` and is tagged `kernel.reset`, so long-running runtimes (FrankenPHP worker mode, RoadRunner) don't hand one visitor's client to the next request; `tests/Functional/LongRunningProcessTest.php` covers this. Any new service that holds per-request state needs the same, unless it keys the state by the request object like `CachedCookieProvider`. - - `CookieBased` reads the cookie; if present, builds `Client(cookieId, metadataProvider->getMetadata(...))`; otherwise delegates. - - `Default` returns an anonymous `new Client(null, new ChangeAwareMetadata())`. -- **Cookie provider** — `CachedCookieProvider` (prio 32, memoizes per `Request` in a `WeakMap` keyed by the request object, so it needs no reset; not `spl_object_hash()`, which PHP hands to a new request once the old one is freed) → `RequestBasedCookieProvider` (parses the cookie off the request). The test kernel keeps its requests alive, so only `CachedCookieProviderTest` can reproduce that handle reuse. -- **Metadata provider** — `DoctrineOrmBasedMetadataProvider` (prio 64) → `EmptyMetadataProvider` (default). Doctrine loads from DB, falling back to the decorated provider when no row exists. +- **Client context**: runtime order outermost→innermost: `CachedClientContext` → the application's decorators → `ClientContext` (`setono_client.client_context.default`). The current client is the source of truth for the client id; see `docs/adr/0001-the-current-client-is-the-source-of-truth-for-the-client-id.md`. + - `CachedClientContext` decorates the **interface alias** `ClientContextInterface` at priority `-1024`, so it wraps every application decorator, whether that decorates the interface (e.g. `#[AsDecorator(ClientContextInterface::class)]`) or `setono_client.client_context.default`. It gives a request one client, even when a decorator creates a client on every call (`tests/Functional/DecoratedClientContextTest.php` uses such a decorator, `tests/Functional/App/Context/RequestedClientContext.php`). It keeps the clients in a `WeakMap` keyed by the main `Request` object, so it needs no `kernel.reset` in long-running runtimes (FrankenPHP worker mode, RoadRunner; `tests/Functional/LongRunningProcessTest.php`). Don't key by `spl_object_hash()`: PHP hands a freed request's handle to the next request. Without a request it passes every call on. Any new service that holds per-request state should key it by the request object the same way. + - `ClientContext` reads the client cookie of the main request with `Cookie/ClientCookieJar`. A usable cookie gives `Client(cookieId, metadataProvider->getMetadata(...))`; otherwise it's a new client, `new Client(null, new ChangeAwareMetadata())`. Without a request, every call gets an anonymous client that is never stored. + - The bundle's own consumers (`StoreCookieSubscriber`, `StoreMetadataSubscriber`, `ClientValueResolver`) inject the interface alias, never `setono_client.client_context.default`, so they get the memo and the application's decorators. +- **Metadata provider**: `DoctrineOrmBasedMetadataProvider` (prio 64) → `EmptyMetadataProvider` (default). Doctrine loads from DB, falling back to the decorated provider when no row exists. + +`Cookie/ClientCookieJar` (`@internal`) owns everything about the client cookie: its name, format, validation (a non-empty client id of at most 255 bytes) and expiry. `read()` returns null for a missing or unusable cookie. `write()` stores the current client's id, keeps `firstSeenAt` only when the request's cookie holds the same id (otherwise fresh timestamps), and throws a `LogicException` for a client id that `read()` would reject. ### Request lifecycle -- `EventSubscriber/StoreCookieSubscriber` (on `kernel.response`): dispatches `Event/PreStoreCookieEvent` (an app can set `$store = false` to veto), then writes/refreshes the cookie with an updated `lastSeenAt`. The cookie is set `HttpOnly(false)` on purpose so client-side JS can read it. +- `EventSubscriber/StoreCookieSubscriber` (on `kernel.response`): dispatches `Event/PreStoreCookieEvent` (an app can set `$store = false` to veto), then writes the client cookie of the current client with `ClientCookieJar::write()`. The cookie is set `HttpOnly(false)` on purpose so client-side JS can read it. - `EventSubscriber/StoreMetadataSubscriber` (on `kernel.finish_request`): calls the metadata persister for the current client. ### Lazy metadata (why writes are cheap) diff --git a/GLOSSARY.md b/GLOSSARY.md new file mode 100644 index 0000000..0c46125 --- /dev/null +++ b/GLOSSARY.md @@ -0,0 +1,39 @@ +# Client tracking + +Recognizes the same browser across visits and keeps key/value data about it between visits. + +## Language + +**Client**: +A browser the application recognizes across visits, identified by its client id. +_Avoid_: user, customer, session + +**Client id**: +The identifier of a client. It's opaque to the application and the same on every visit. +_Avoid_: visitor id, tracking id + +**Current client**: +The client the request being handled belongs to. It's the source of truth for the client id, and the client cookie always follows it. +_Avoid_: resolved client, active client + +**New client**: +A client that the request introduced: the request had no valid client cookie, so it got a new client id. +_Avoid_: anonymous client (that's a different term), first-time visitor + +**Anonymous client**: +A client created where there is no request, e.g. in a console command or a worker. Its client id is never stored. +_Avoid_: new client, default client + +**Client cookie**: +The cookie that carries a client's id together with its first-seen and last-seen timestamps. +_Avoid_: client id cookie, tracking cookie + +**First seen**: +When the client id in the client cookie was first issued. It starts over when the current client's id differs from the cookie's. + +**Last seen**: +When the client was last seen, i.e. the latest request that refreshed its client cookie. + +**Metadata**: +Key/value data kept per client between visits, each key with an optional expiry. +_Avoid_: attributes, properties, client data diff --git a/README.md b/README.md index 7762aab..ded1401 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Typical use cases: first‑party analytics, marketing attribution, returning‑v - [Get the current client](#get-the-current-client) - [Read and write metadata](#read-and-write-metadata) - [Set metadata from a request](#set-metadata-from-a-request) - - [Access the cookie directly](#access-the-cookie-directly) + - [Read the cookie in JavaScript](#read-the-cookie-in-javascript) - [Skip storing the cookie (consent)](#skip-storing-the-cookie-consent) - [Configuration](#configuration) - [How it works](#how-it-works) @@ -109,6 +109,9 @@ final class SomeService } ``` +A request gets one client, however often you ask for it. Outside a request (console commands, workers) there is no +visitor to recognize, so every call returns an *anonymous* client with a new client id that is never stored. + ### Read and write metadata `$client->metadata` is a key/value store. Reading a key for the first time lazily loads the metadata from the @@ -164,35 +167,11 @@ final class GoogleClickIdSubscriber implements EventSubscriberInterface } ``` -### Access the cookie directly - -The cookie stores the client id plus the first/last seen timestamps. Read it through `CookieProviderInterface`: - -```php -use Setono\ClientBundle\CookieProvider\CookieProviderInterface; - -final class SomeService -{ - public function __construct(private readonly CookieProviderInterface $cookieProvider) - { - } - - public function __invoke(): void - { - $cookie = $this->cookieProvider->getCookie(); - if (null === $cookie) { - return; // visitor has no cookie yet - } +### Read the cookie in JavaScript - $cookie->clientId; // the client id - $cookie->firstSeenAt; // unix timestamp of first visit - $cookie->lastSeenAt; // unix timestamp of the previous visit - } -} -``` - -The cookie is intentionally **not** `HttpOnly`, so you can also read it from JavaScript (e.g. to send the client id -to a third‑party tag). +The cookie is intentionally **not** `HttpOnly`, so you can read it from JavaScript (e.g. to send the client id to a +third‑party tag). Its value is `...`, with the timestamps as unix timestamps, +e.g. `2.1700000000.1700086400.0190c3a2-...`. ### Skip storing the cookie (consent) @@ -247,13 +226,14 @@ setono_client: ## How it works -- **Cookie** — on each main response, `StoreCookieSubscriber` writes/refreshes the cookie and bumps `lastSeenAt`. - A new client id is generated only when no valid cookie is present. +- **Current client** — `ClientContext` recognizes the client by the cookie of the main request. A request without a + valid cookie gets a *new client*, with a new client id. `CachedClientContext` wraps the client context, including + your decorators, and gives each request one client. +- **Cookie** — on each main response, `StoreCookieSubscriber` writes the cookie of the current client and bumps + `lastSeenAt`. The cookie always holds the client id of the current client: when that differs from the id in the + request's cookie (e.g. because a decorator chose another client), the cookie gets the new id and fresh timestamps. - **Lazy metadata** — `$client->metadata` is a lazy ghost object. The database is queried only the first time you read a value, and a row is written only at the end of the request, and only if the metadata was actually changed. -- **Resolution chain** — `ClientContextInterface` is built from a chain of decorating services - (`CachedClientContext` → `CookieBasedClientContext` → `DefaultClientContext`). The cached layer memoizes the client - per request; the cookie‑based layer builds it from the cookie; the default layer creates a fresh, anonymous client. ## Extending the bundle @@ -262,7 +242,6 @@ Every moving part is a service behind an interface, so you can swap or decorate | Concern | Interface | | --- | --- | | Resolving the current client | `Setono\ClientBundle\Context\ClientContextInterface` | -| Reading the cookie | `Setono\ClientBundle\CookieProvider\CookieProviderInterface` | | Loading metadata | `Setono\ClientBundle\MetadataProvider\MetadataProviderInterface` | | Persisting metadata | `Setono\ClientBundle\MetadataPersister\MetadataPersisterInterface` | | Metadata entity | `Setono\ClientBundle\Entity\MetadataInterface` | @@ -270,10 +249,33 @@ Every moving part is a service behind an interface, so you can swap or decorate To add behaviour, [decorate](https://symfony.com/doc/current/service_container/service_decoration.html) the relevant service rather than replacing it outright — that is exactly how the bundle composes its own defaults. -Decorate the `.default` service of a chain, e.g. `setono_client.client_context.default`, and choose its place with the -decoration priority. Don't decorate `setono_client.client_context.cached` directly: it is reset between requests in -long-running runtimes like FrankenPHP's worker mode, and Symfony would then reset your decorator instead. Your decorator -would have to implement `Symfony\Contracts\Service\ResetInterface` and pass the `reset()` call on. +To change which client a request gets, e.g. to recognize a logged‑in user's client on another device, decorate +`ClientContextInterface`: + +```php +use Setono\Client\Client; +use Setono\ClientBundle\Context\ClientContextInterface; +use Symfony\Component\DependencyInjection\Attribute\AsDecorator; +use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated; + +#[AsDecorator(ClientContextInterface::class)] +final class UserClientContext implements ClientContextInterface +{ + public function __construct(#[AutowireDecorated] private readonly ClientContextInterface $decorated) + { + } + + public function getClient(): Client + { + // return another client, or the decorated one + return $this->decorated->getClient(); + } +} +``` + +The bundle's `CachedClientContext` decorates the interface with the lowest priority (`-1024`), so it wraps your +decorator and a request gets one client however often it's asked for, even if your decorator creates a client on every +call. The cookie then stores the client id of your client. It must be a non‑empty string of at most 255 bytes. ## Contributing diff --git a/UPGRADE-2.0.md b/UPGRADE-2.0.md new file mode 100644 index 0000000..3442ff6 --- /dev/null +++ b/UPGRADE-2.0.md @@ -0,0 +1,28 @@ +# Upgrade from 1.x to 2.0 + +## Client resolution + +The client context and the cookie are now one module: `ClientContext` recognizes the client, `CachedClientContext` gives a request one client, and an internal `ClientCookieJar` reads and writes the cookie. + +### Removed + +- `Setono\ClientBundle\CookieProvider\CookieProviderInterface`, `RequestBasedCookieProvider` and `CachedCookieProvider`, with the services `setono_client.cookie_provider.default` and `setono_client.cookie_provider.cached`. To read the cookie's timestamps, parse the cookie of the request yourself: + + ```php + $cookie = \Setono\Client\Cookie::fromString($request->cookies->getString('setono_client_id')); // throws on an invalid cookie + ``` + +- `Setono\ClientBundle\Context\CookieBasedClientContext` and `DefaultClientContext`, with the service `setono_client.client_context.cookie_based`. `Setono\ClientBundle\Context\ClientContext` (`setono_client.client_context.default`) replaces both. +- `CachedClientContext` no longer implements `Symfony\Contracts\Service\ResetInterface` and is no longer tagged `kernel.reset`. Its constructor takes a `RequestStack` as its second argument. +- The dependency on `symfony/service-contracts`. + +### Changed + +- `StoreCookieSubscriber::__construct()` takes `ClientContextInterface`, `ClientCookieJar` and the event dispatcher. The cookie name and expiration moved to `ClientCookieJar`. +- The cookie always holds the client id of the current client, i.e. what `ClientContextInterface::getClient()` returns, decorators included. In 1.x, a request with a cookie kept the cookie's client id. When the ids differ, the cookie gets the new client id and fresh first and last seen timestamps. +- The cookie writer throws a `LogicException` when the current client's id is empty or longer than 255 bytes, because the next request couldn't read it back. +- Outside a request (console commands, workers), every `getClient()` call returns a new anonymous client. In 1.x, the first one was kept until the container was reset. + +### Decorating the client context + +Decorate `Setono\ClientBundle\Context\ClientContextInterface`, e.g. with `#[AsDecorator(ClientContextInterface::class)]`. Decorating `setono_client.client_context.default` works too. You no longer need to pick a priority or implement `ResetInterface`. `CachedClientContext` decorates the interface with priority `-1024`, so it wraps your decorator, and a request gets one client even if your decorator creates a client on every call. diff --git a/composer.json b/composer.json index 733e184..3e837a9 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,6 @@ "symfony/event-dispatcher": "^6.4 || ^7.0", "symfony/http-foundation": "^6.4 || ^7.0", "symfony/http-kernel": "^6.4 || ^7.0", - "symfony/service-contracts": "^2.5 || ^3.0", "symfony/uid": "^6.4 || ^7.0", "symfony/var-exporter": "^6.4 || ^7.0" }, diff --git a/config/services.xml b/config/services.xml index 78c7ed7..9f86587 100644 --- a/config/services.xml +++ b/config/services.xml @@ -7,45 +7,32 @@ - - - + class="Setono\ClientBundle\Context\ClientContext"> + + - + + decorates="Setono\ClientBundle\Context\ClientContextInterface" decoration-priority="-1024"> - - - - - - - - - %setono_client.cookie.name% - - - + + + %setono_client.cookie.name% + %setono_client.cookie.expiration% - + @@ -59,18 +46,16 @@ - - + + - %setono_client.cookie.name% - %setono_client.cookie.expiration% - + diff --git a/docs/adr/0001-the-current-client-is-the-source-of-truth-for-the-client-id.md b/docs/adr/0001-the-current-client-is-the-source-of-truth-for-the-client-id.md new file mode 100644 index 0000000..e42fd81 --- /dev/null +++ b/docs/adr/0001-the-current-client-is-the-source-of-truth-for-the-client-id.md @@ -0,0 +1,11 @@ +# The current client is the source of truth for the client id + +In 2.0, the client cookie always follows the current client, which is the client that `ClientContextInterface` returns, decorators included. When the current client's id differs from the one in the request's client cookie, the cookie is rewritten with the new id and fresh timestamps. In 1.x, the cookie writer read the cookie and ignored the client context whenever a cookie existed, so a decorated context and the cookie could disagree. The bundle hands a request one client by memoizing it per `Request` object, outermost around any decorators, so it needs no `kernel.reset` in long-running runtimes. Without a request there is no visitor to recognize, so every call gets a new anonymous client that is never stored. + +## Considered Options + +- **The cookie wins** (the 1.x behaviour): rejected. A decorator, the documented way to choose another client, couldn't change the id stored in the cookie. +- **A memo cleared by `kernel.reset`** (1.x): rejected. Symfony resets whichever service decorates the reset one, and decorators had to sit inside the memo by picking a priority. Keying by the `Request` object needs no reset. +- **A memo inside the bundle's client context, with decorators outside it**: rejected. A decorator that creates a client on every call would give a request several clients, and metadata set on one of them would be lost. +- **Throw without a request**: rejected. Callers like a wishlist factory run in fixtures and console commands. +- **One anonymous client per process**: rejected. Messenger workers would share it across messages. diff --git a/src/Context/CachedClientContext.php b/src/Context/CachedClientContext.php index 9cf69ea..3ee3dc1 100644 --- a/src/Context/CachedClientContext.php +++ b/src/Context/CachedClientContext.php @@ -5,27 +5,38 @@ namespace Setono\ClientBundle\Context; use Setono\Client\Client; -use Symfony\Contracts\Service\ResetInterface; - -final class CachedClientContext implements ClientContextInterface, ResetInterface +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; + +/** + * Gives a request one client. It decorates ClientContextInterface with the lowest priority, so it wraps the decorators + * of the application too, and a decorator that creates a client on every call still gives the request one client. + * Without a request, every call gets a client of its own + */ +final class CachedClientContext implements ClientContextInterface { - private ?Client $client = null; - - public function __construct(private readonly ClientContextInterface $decorated) - { + /** + * Keyed by the request object, so an entry goes away with its request, and a new request never gets the client of + * an earlier one. That's why this service needs no reset between requests in long-running runtimes + * + * @var \WeakMap + */ + private readonly \WeakMap $clients; + + public function __construct( + private readonly ClientContextInterface $decorated, + private readonly RequestStack $requestStack, + ) { + $this->clients = new \WeakMap(); } public function getClient(): Client { - if (null === $this->client) { - $this->client = $this->decorated->getClient(); + $request = $this->requestStack->getMainRequest(); + if (null === $request) { + return $this->decorated->getClient(); } - return $this->client; - } - - public function reset(): void - { - $this->client = null; + return $this->clients[$request] ??= $this->decorated->getClient(); } } diff --git a/src/Context/ClientContext.php b/src/Context/ClientContext.php new file mode 100644 index 0000000..affbe84 --- /dev/null +++ b/src/Context/ClientContext.php @@ -0,0 +1,37 @@ +requestStack->getMainRequest(); + $cookie = null === $request ? null : $this->cookieJar->read($request); + + if (null === $cookie) { + return new Client(null, new ChangeAwareMetadata()); + } + + return new Client($cookie->clientId, $this->metadataProvider->getMetadata($cookie->clientId)); + } +} diff --git a/src/Context/CookieBasedClientContext.php b/src/Context/CookieBasedClientContext.php deleted file mode 100644 index 86182a3..0000000 --- a/src/Context/CookieBasedClientContext.php +++ /dev/null @@ -1,29 +0,0 @@ -clientCookieProvider->getCookie(); - if (null === $cookie) { - return $this->decorated->getClient(); - } - - return new Client($cookie->clientId, $this->metadataProvider->getMetadata($cookie->clientId)); - } -} diff --git a/src/Context/DefaultClientContext.php b/src/Context/DefaultClientContext.php deleted file mode 100644 index e9a4b53..0000000 --- a/src/Context/DefaultClientContext.php +++ /dev/null @@ -1,16 +0,0 @@ -cookies->get($this->name); + if (!is_string($value) || '' === $value) { + return null; + } + + try { + $cookie = Cookie::fromString($value); + } catch (\InvalidArgumentException) { + return null; + } + + return self::isUsable($cookie->clientId) ? $cookie : null; + } + + /** + * Sets the client cookie of the given client on the response. When the request's client cookie holds the same + * client id, the first seen timestamp carries over. Otherwise, the client id is new to this cookie, and so are its timestamps + * + * @throws \LogicException if the client id is one that read() would reject on the next request + */ + public function write(Request $request, Response $response, Client $client): void + { + if (!self::isUsable($client->id)) { + throw new \LogicException(sprintf( + 'The client id must be a non-empty string of at most %d bytes to be stored in the client cookie, but the current client has a client id of %d bytes. Check the services that decorate %s', + self::MAX_CLIENT_ID_LENGTH, + strlen($client->id), + ClientContextInterface::class, + )); + } + + $cookie = $this->read($request); + if (null === $cookie || $cookie->clientId !== $client->id) { + $cookie = new Cookie($client->id); + } + + $response->headers->setCookie( + HttpCookie::create($this->name, $cookie->withLastSeenAt(time())->toString(), $this->expiration) + ->withHttpOnly(false), // this allows javascript to read the cookie + ); + } + + /** + * All clients with an empty client id would share one client, and the database column holds 255 characters + */ + private static function isUsable(string $clientId): bool + { + return '' !== $clientId && strlen($clientId) <= self::MAX_CLIENT_ID_LENGTH; + } +} diff --git a/src/CookieProvider/CachedCookieProvider.php b/src/CookieProvider/CachedCookieProvider.php deleted file mode 100644 index ca6766c..0000000 --- a/src/CookieProvider/CachedCookieProvider.php +++ /dev/null @@ -1,40 +0,0 @@ - - */ - private readonly \WeakMap $cache; - - public function __construct( - private readonly CookieProviderInterface $decorated, - private readonly RequestStack $requestStack, - ) { - $this->cache = new \WeakMap(); - } - - public function getCookie(?Request $request = null): ?Cookie - { - $request ??= $this->requestStack->getMainRequest(); - if (null === $request) { - return null; - } - - $this->cache[$request] ??= ['cookie' => $this->decorated->getCookie($request)]; - - return $this->cache[$request]['cookie']; - } -} diff --git a/src/CookieProvider/CookieProviderInterface.php b/src/CookieProvider/CookieProviderInterface.php deleted file mode 100644 index 7cd0ef4..0000000 --- a/src/CookieProvider/CookieProviderInterface.php +++ /dev/null @@ -1,16 +0,0 @@ -requestStack->getMainRequest())?->cookies->get($this->cookieName); - if (!is_string($cookieValue) || '' === $cookieValue) { - return null; - } - - try { - $cookie = Cookie::fromString($cookieValue); - } catch (\InvalidArgumentException) { - return null; - } - - // All visitors with an empty client id would share one client, and the database column holds 255 characters - if ('' === $cookie->clientId || strlen($cookie->clientId) > self::MAX_CLIENT_ID_LENGTH) { - return null; - } - - return $cookie; - } -} diff --git a/src/EventSubscriber/StoreCookieSubscriber.php b/src/EventSubscriber/StoreCookieSubscriber.php index 6ac8f61..1ddf882 100644 --- a/src/EventSubscriber/StoreCookieSubscriber.php +++ b/src/EventSubscriber/StoreCookieSubscriber.php @@ -5,23 +5,22 @@ namespace Setono\ClientBundle\EventSubscriber; use Psr\EventDispatcher\EventDispatcherInterface; -use Setono\Client\Cookie; use Setono\ClientBundle\Context\ClientContextInterface; -use Setono\ClientBundle\CookieProvider\CookieProviderInterface; +use Setono\ClientBundle\Cookie\ClientCookieJar; use Setono\ClientBundle\Event\PreStoreCookieEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Symfony\Component\HttpFoundation\Cookie as HttpCookie; use Symfony\Component\HttpKernel\Event\ResponseEvent; use Symfony\Component\HttpKernel\KernelEvents; +/** + * Stores the client cookie of the current client, unless a listener of PreStoreCookieEvent vetoes it + */ final class StoreCookieSubscriber implements EventSubscriberInterface { public function __construct( private readonly ClientContextInterface $clientContext, - private readonly CookieProviderInterface $cookieProvider, + private readonly ClientCookieJar $cookieJar, private readonly EventDispatcherInterface $eventDispatcher, - private readonly string $cookieName, - private readonly string $cookieExpiration, ) { } @@ -45,15 +44,6 @@ public function store(ResponseEvent $event): void return; } - $cookie = $this->cookieProvider->getCookie($event->getRequest()); - - if (null === $cookie) { - $cookie = new Cookie($this->clientContext->getClient()->id); - } - - $event->getResponse()->headers->setCookie( - HttpCookie::create($this->cookieName, $cookie->withLastSeenAt(time())->toString(), $this->cookieExpiration) - ->withHttpOnly(false), // this allows javascript to read the cookie - ); + $this->cookieJar->write($event->getRequest(), $event->getResponse(), $this->clientContext->getClient()); } } diff --git a/tests/Functional/AnonymousClientTest.php b/tests/Functional/AnonymousClientTest.php new file mode 100644 index 0000000..45b1601 --- /dev/null +++ b/tests/Functional/AnonymousClientTest.php @@ -0,0 +1,28 @@ +get(ClientContextInterface::class); + self::assertInstanceOf(ClientContextInterface::class, $clientContext); + + $client = $clientContext->getClient(); + + self::assertTrue(Uuid::isValid($client->id)); + self::assertNotSame($client->id, $clientContext->getClient()->id); + } +} diff --git a/tests/Functional/App/Context/RequestedClientContext.php b/tests/Functional/App/Context/RequestedClientContext.php new file mode 100644 index 0000000..09c6209 --- /dev/null +++ b/tests/Functional/App/Context/RequestedClientContext.php @@ -0,0 +1,38 @@ +requestStack->getMainRequest()?->query->get('client_id'); + if (!is_string($clientId)) { + return $this->decorated->getClient(); + } + + return new Client($clientId, new ChangeAwareMetadata()); + } +} diff --git a/tests/Functional/App/Kernel.php b/tests/Functional/App/Kernel.php index 99d39a1..e7f403b 100644 --- a/tests/Functional/App/Kernel.php +++ b/tests/Functional/App/Kernel.php @@ -105,6 +105,7 @@ protected function configureContainer(ContainerConfigurator $container): void $services ->defaults()->autowire()->autoconfigure() ->load(__NAMESPACE__ . '\\Controller\\', __DIR__ . '/Controller')->tag('controller.service_arguments') + ->load(__NAMESPACE__ . '\\Context\\', __DIR__ . '/Context') ->load(__NAMESPACE__ . '\\EventSubscriber\\', __DIR__ . '/EventSubscriber') ; diff --git a/tests/Functional/DecoratedClientContextTest.php b/tests/Functional/DecoratedClientContextTest.php new file mode 100644 index 0000000..c2762e1 --- /dev/null +++ b/tests/Functional/DecoratedClientContextTest.php @@ -0,0 +1,62 @@ +browser->request('GET', '/?client_id=decorated-client-id'); + + self::assertSame('decorated-client-id', $this->browser->getResponse()->getContent()); + self::assertSame('decorated-client-id', $this->getClientId()); + } + + /** + * @test + */ + public function it_stores_the_client_id_of_the_decorator_with_fresh_timestamps_in_place_of_the_cookie(): void + { + $this->browser->getCookieJar()->set(new BrowserCookie(self::COOKIE_NAME, (new Cookie('cookie-client-id', 2, 1_000, 2_000))->toString(), null, '/', 'localhost')); + + $before = time(); + $this->browser->request('GET', '/?client_id=decorated-client-id'); + + $cookie = $this->getCookie(); + self::assertSame('decorated-client-id', $cookie->clientId); + self::assertGreaterThanOrEqual($before, $cookie->firstSeenAt); + } + + /** + * @test + */ + public function it_stores_the_metadata_set_on_the_client_of_a_decorator_that_creates_a_client_on_every_call(): void + { + $this->browser->request('GET', '/metadata/set?key=foo&value=bar&client_id=decorated-client-id'); + + self::assertSame(['foo' => 'bar'], self::fetchStoredMetadata('decorated-client-id')); + } + + /** + * @test + */ + public function it_refuses_to_store_a_client_id_that_the_cookie_cannot_hold(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The client id must be a non-empty string'); + + $this->browser->request('GET', '/?client_id='); + } +} diff --git a/tests/Unit/Context/CachedClientContextTest.php b/tests/Unit/Context/CachedClientContextTest.php index 20ac50a..c2fb451 100644 --- a/tests/Unit/Context/CachedClientContextTest.php +++ b/tests/Unit/Context/CachedClientContextTest.php @@ -9,6 +9,8 @@ use Setono\Client\Client; use Setono\ClientBundle\Context\CachedClientContext; use Setono\ClientBundle\Context\ClientContextInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; final class CachedClientContextTest extends TestCase { @@ -17,46 +19,80 @@ final class CachedClientContextTest extends TestCase /** * @test */ - public function it_returns_the_decorated_client(): void + public function it_gives_a_request_one_client(): void { - $client = new Client('id'); + $client = new Client('client-id'); $decorated = $this->prophesize(ClientContextInterface::class); - $decorated->getClient()->willReturn($client); + $decorated->getClient()->willReturn($client)->shouldBeCalledOnce(); - self::assertSame($client, (new CachedClientContext($decorated->reveal()))->getClient()); + $requestStack = new RequestStack(); + $requestStack->push(new Request()); + + $context = new CachedClientContext($decorated->reveal(), $requestStack); + + self::assertSame($client, $context->getClient()); + self::assertSame($client, $context->getClient()); } /** * @test */ - public function it_only_resolves_the_client_once(): void + public function it_gives_a_sub_request_the_client_of_the_main_request(): void { + $client = new Client('client-id'); + $decorated = $this->prophesize(ClientContextInterface::class); - $decorated->getClient()->willReturn(new Client('id'))->shouldBeCalledOnce(); + $decorated->getClient()->willReturn($client)->shouldBeCalledOnce(); + + $requestStack = new RequestStack(); + $requestStack->push(new Request()); + + $context = new CachedClientContext($decorated->reveal(), $requestStack); + $context->getClient(); - $context = new CachedClientContext($decorated->reveal()); + $requestStack->push(new Request()); - self::assertSame($context->getClient(), $context->getClient()); + self::assertSame($client, $context->getClient()); } /** * @test */ - public function it_resolves_the_client_again_after_a_reset(): void + public function it_gives_the_next_request_a_client_of_its_own(): void { - $firstClient = new Client('first-id'); - $secondClient = new Client('second-id'); + $firstClient = new Client('first-client-id'); + $secondClient = new Client('second-client-id'); $decorated = $this->prophesize(ClientContextInterface::class); $decorated->getClient()->willReturn($firstClient, $secondClient); - $context = new CachedClientContext($decorated->reveal()); + // Like a long-running runtime, which handles many requests with the same service + $requestStack = new RequestStack(); + $context = new CachedClientContext($decorated->reveal(), $requestStack); + $requestStack->push(new Request()); self::assertSame($firstClient, $context->getClient()); + $requestStack->pop(); - $context->reset(); + $requestStack->push(new Request()); + self::assertSame($secondClient, $context->getClient()); + } + /** + * @test + */ + public function it_passes_every_call_without_a_request_on(): void + { + $firstClient = new Client('first-client-id'); + $secondClient = new Client('second-client-id'); + + $decorated = $this->prophesize(ClientContextInterface::class); + $decorated->getClient()->willReturn($firstClient, $secondClient); + + $context = new CachedClientContext($decorated->reveal(), new RequestStack()); + + self::assertSame($firstClient, $context->getClient()); self::assertSame($secondClient, $context->getClient()); } } diff --git a/tests/Unit/Context/ClientContextTest.php b/tests/Unit/Context/ClientContextTest.php new file mode 100644 index 0000000..27d8a91 --- /dev/null +++ b/tests/Unit/Context/ClientContextTest.php @@ -0,0 +1,121 @@ + 'bar']); + + $metadataProvider = $this->prophesize(MetadataProviderInterface::class); + $metadataProvider->getMetadata('client-id')->willReturn($metadata); + + $client = $this->createContext(self::createRequest('client-id'), $metadataProvider)->getClient(); + + self::assertSame('client-id', $client->id); + self::assertSame($metadata, $client->metadata); + } + + /** + * @test + */ + public function it_recognizes_the_client_by_the_client_cookie_of_the_main_request(): void + { + $requestStack = new RequestStack(); + $requestStack->push(self::createRequest('main-client-id')); + $requestStack->push(self::createRequest('sub-client-id')); + + $metadataProvider = $this->prophesize(MetadataProviderInterface::class); + $metadataProvider->getMetadata(Argument::type('string'))->willReturn(new Metadata()); + + $context = new ClientContext($requestStack, new ClientCookieJar(self::COOKIE_NAME, '+365 days'), $metadataProvider->reveal()); + + self::assertSame('main-client-id', $context->getClient()->id); + } + + /** + * @test + */ + public function it_gives_a_request_without_a_client_cookie_a_new_client(): void + { + $metadataProvider = $this->prophesize(MetadataProviderInterface::class); + $metadataProvider->getMetadata(Argument::any())->shouldNotBeCalled(); + + $client = $this->createContext(new Request(), $metadataProvider)->getClient(); + + self::assertTrue(Uuid::isValid($client->id)); + self::assertInstanceOf(ChangeAwareMetadata::class, $client->metadata); + self::assertCount(0, $client->metadata); + } + + /** + * @test + */ + public function it_gives_a_request_with_an_unusable_client_cookie_a_new_client(): void + { + $metadataProvider = $this->prophesize(MetadataProviderInterface::class); + $metadataProvider->getMetadata(Argument::any())->shouldNotBeCalled(); + + $client = $this->createContext(new Request([], [], [], [self::COOKIE_NAME => '1.2.3']), $metadataProvider)->getClient(); + + self::assertTrue(Uuid::isValid($client->id)); + } + + /** + * @test + */ + public function it_gives_every_call_without_a_request_an_anonymous_client(): void + { + $metadataProvider = $this->prophesize(MetadataProviderInterface::class); + $metadataProvider->getMetadata(Argument::any())->shouldNotBeCalled(); + + $context = $this->createContext(null, $metadataProvider); + $client = $context->getClient(); + + self::assertTrue(Uuid::isValid($client->id)); + self::assertInstanceOf(ChangeAwareMetadata::class, $client->metadata); + self::assertNotSame($client->id, $context->getClient()->id); + } + + /** + * @param ObjectProphecy $metadataProvider + */ + private function createContext(?Request $request, ObjectProphecy $metadataProvider): ClientContext + { + $requestStack = new RequestStack(); + if (null !== $request) { + $requestStack->push($request); + } + + return new ClientContext($requestStack, new ClientCookieJar(self::COOKIE_NAME, '+365 days'), $metadataProvider->reveal()); + } + + private static function createRequest(string $clientId): Request + { + return new Request([], [], [], [self::COOKIE_NAME => (new Cookie($clientId))->toString()]); + } +} diff --git a/tests/Unit/Context/CookieBasedClientContextTest.php b/tests/Unit/Context/CookieBasedClientContextTest.php deleted file mode 100644 index f1ad93f..0000000 --- a/tests/Unit/Context/CookieBasedClientContextTest.php +++ /dev/null @@ -1,74 +0,0 @@ -prophesize(ClientContextInterface::class); - $decorated->getClient()->willReturn($decoratedClient); - - $metadataProvider = $this->prophesize(MetadataProviderInterface::class); - $metadataProvider->getMetadata(Argument::any())->shouldNotBeCalled(); - - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie()->willReturn(null); - - $context = new CookieBasedClientContext( - $decorated->reveal(), - $metadataProvider->reveal(), - $cookieProvider->reveal(), - ); - - self::assertSame($decoratedClient, $context->getClient()); - } - - /** - * @test - */ - public function it_builds_a_client_from_the_cookie(): void - { - $decorated = $this->prophesize(ClientContextInterface::class); - $decorated->getClient()->shouldNotBeCalled(); - - $metadata = new Metadata(['foo' => 'bar']); - - $metadataProvider = $this->prophesize(MetadataProviderInterface::class); - $metadataProvider->getMetadata('client-id')->willReturn($metadata); - - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie()->willReturn(new Cookie('client-id')); - - $context = new CookieBasedClientContext( - $decorated->reveal(), - $metadataProvider->reveal(), - $cookieProvider->reveal(), - ); - - $client = $context->getClient(); - - self::assertSame('client-id', $client->id); - self::assertSame($metadata, $client->metadata); - } -} diff --git a/tests/Unit/Context/DefaultClientContextTest.php b/tests/Unit/Context/DefaultClientContextTest.php deleted file mode 100644 index a165f18..0000000 --- a/tests/Unit/Context/DefaultClientContextTest.php +++ /dev/null @@ -1,25 +0,0 @@ -getClient(); - $metadata = $client->metadata; - - self::assertNotSame('', $client->id); - self::assertInstanceOf(ChangeAwareMetadata::class, $metadata); - self::assertFalse($metadata->isDirty()); - } -} diff --git a/tests/Unit/Cookie/ClientCookieJarTest.php b/tests/Unit/Cookie/ClientCookieJarTest.php new file mode 100644 index 0000000..622dd81 --- /dev/null +++ b/tests/Unit/Cookie/ClientCookieJarTest.php @@ -0,0 +1,238 @@ +read(self::createRequest((new Cookie('client-id', 2, 1_000, 2_000))->toString())); + + self::assertNotNull($cookie); + self::assertSame('client-id', $cookie->clientId); + self::assertSame(1_000, $cookie->firstSeenAt); + self::assertSame(2_000, $cookie->lastSeenAt); + } + + /** + * @test + */ + public function it_reads_a_version_1_cookie(): void + { + self::assertSame('client-id', self::createJar()->read(self::createRequest('client-id'))?->clientId); + } + + /** + * @test + */ + public function it_reads_the_cookie_with_the_configured_name(): void + { + $request = new Request([], [], [], ['custom_name' => (new Cookie('client-id'))->toString()]); + + self::assertNull(self::createJar()->read($request)); + self::assertSame('client-id', self::createJar('custom_name')->read($request)?->clientId); + } + + /** + * @test + */ + public function it_reads_nothing_when_the_request_has_no_client_cookie(): void + { + self::assertNull(self::createJar()->read(new Request())); + } + + /** + * @test + */ + public function it_reads_nothing_when_the_cookie_is_empty(): void + { + self::assertNull(self::createJar()->read(self::createRequest(''))); + } + + /** + * @test + */ + public function it_reads_nothing_when_the_cookie_is_not_a_string(): void + { + self::assertNull(self::createJar()->read(new Request([], [], [], [self::COOKIE_NAME => 1]))); + } + + /** + * @test + */ + public function it_reads_nothing_when_the_cookie_is_invalid(): void + { + self::assertNull(self::createJar()->read(self::createRequest('1.2.3'))); + } + + /** + * @test + */ + public function it_reads_nothing_when_the_client_id_is_empty(): void + { + self::assertNull(self::createJar()->read(self::createRequest('2.1.1.'))); + } + + /** + * @test + */ + public function it_reads_nothing_when_the_client_id_is_longer_than_255_characters(): void + { + self::assertNull(self::createJar()->read(self::createRequest((new Cookie(str_repeat('a', 256)))->toString()))); + } + + /** + * @test + */ + public function it_reads_a_client_id_of_255_characters(): void + { + $clientId = str_repeat('a', 255); + + self::assertSame($clientId, self::createJar()->read(self::createRequest((new Cookie($clientId))->toString()))?->clientId); + } + + /** + * @test + */ + public function it_writes_a_cookie_that_javascript_can_read(): void + { + $response = new Response(); + self::createJar()->write(new Request(), $response, new Client('client-id')); + + $httpCookie = self::getWrittenCookie($response); + self::assertSame(self::COOKIE_NAME, $httpCookie->getName()); + self::assertFalse($httpCookie->isHttpOnly()); + } + + /** + * @test + */ + public function it_writes_the_cookie_with_the_configured_name_and_expiration(): void + { + $before = time(); + $response = new Response(); + self::createJar('custom_name', '+30 days')->write(new Request(), $response, new Client('client-id')); + + $httpCookie = self::getWrittenCookie($response); + self::assertSame('custom_name', $httpCookie->getName()); + self::assertGreaterThanOrEqual(strtotime('+30 days', $before), $httpCookie->getExpiresTime()); + self::assertLessThanOrEqual(strtotime('+30 days'), $httpCookie->getExpiresTime()); + } + + /** + * @test + */ + public function it_gives_a_new_client_fresh_timestamps(): void + { + $before = time(); + $response = new Response(); + self::createJar()->write(new Request(), $response, new Client('client-id')); + + $cookie = Cookie::fromString((string) self::getWrittenCookie($response)->getValue()); + self::assertSame('client-id', $cookie->clientId); + self::assertGreaterThanOrEqual($before, $cookie->firstSeenAt); + self::assertGreaterThanOrEqual($before, $cookie->lastSeenAt); + } + + /** + * @test + */ + public function it_keeps_the_first_seen_timestamp_and_refreshes_the_last_seen_timestamp_of_the_same_client_id(): void + { + $request = self::createRequest((new Cookie('client-id', 2, 1_000, 2_000))->toString()); + + $before = time(); + $response = new Response(); + self::createJar()->write($request, $response, new Client('client-id')); + + $cookie = Cookie::fromString((string) self::getWrittenCookie($response)->getValue()); + self::assertSame('client-id', $cookie->clientId); + self::assertSame(1_000, $cookie->firstSeenAt); + self::assertGreaterThanOrEqual($before, $cookie->lastSeenAt); + self::assertLessThanOrEqual(time(), $cookie->lastSeenAt); + } + + /** + * @test + */ + public function it_writes_the_client_id_of_the_client_and_starts_over_the_timestamps_when_it_differs_from_the_cookie(): void + { + $request = self::createRequest((new Cookie('cookie-client-id', 2, 1_000, 2_000))->toString()); + + $before = time(); + $response = new Response(); + self::createJar()->write($request, $response, new Client('current-client-id')); + + $cookie = Cookie::fromString((string) self::getWrittenCookie($response)->getValue()); + self::assertSame('current-client-id', $cookie->clientId); + self::assertGreaterThanOrEqual($before, $cookie->firstSeenAt); + } + + /** + * @test + */ + public function it_writes_a_client_id_of_255_characters(): void + { + $clientId = str_repeat('a', 255); + + $response = new Response(); + self::createJar()->write(new Request(), $response, new Client($clientId)); + + self::assertSame($clientId, Cookie::fromString((string) self::getWrittenCookie($response)->getValue())->clientId); + } + + /** + * @test + */ + public function it_refuses_to_write_an_empty_client_id(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('The client id must be a non-empty string of at most 255 bytes to be stored in the client cookie, but the current client has a client id of 0 bytes'); + + self::createJar()->write(new Request(), new Response(), new Client('')); + } + + /** + * @test + */ + public function it_refuses_to_write_a_client_id_longer_than_255_characters(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('client id of 256 bytes'); + + self::createJar()->write(new Request(), new Response(), new Client(str_repeat('a', 256))); + } + + private static function createJar(string $name = self::COOKIE_NAME, string $expiration = '+365 days'): ClientCookieJar + { + return new ClientCookieJar($name, $expiration); + } + + private static function createRequest(string $cookieValue): Request + { + return new Request([], [], [], [self::COOKIE_NAME => $cookieValue]); + } + + private static function getWrittenCookie(Response $response): HttpCookie + { + $cookies = $response->headers->getCookies(); + self::assertCount(1, $cookies); + + return $cookies[0]; + } +} diff --git a/tests/Unit/CookieProvider/CachedCookieProviderTest.php b/tests/Unit/CookieProvider/CachedCookieProviderTest.php deleted file mode 100644 index efc26b3..0000000 --- a/tests/Unit/CookieProvider/CachedCookieProviderTest.php +++ /dev/null @@ -1,134 +0,0 @@ -prophesize(CookieProviderInterface::class); - $decorated->getCookie(Argument::any())->shouldNotBeCalled(); - - $provider = new CachedCookieProvider($decorated->reveal(), new RequestStack()); - - self::assertNull($provider->getCookie()); - } - - /** - * @test - */ - public function it_resolves_the_cookie_once_per_request(): void - { - $request = new Request(); - $cookie = new Cookie('client-id'); - - $decorated = $this->prophesize(CookieProviderInterface::class); - $decorated->getCookie($request)->willReturn($cookie)->shouldBeCalledOnce(); - - $provider = new CachedCookieProvider($decorated->reveal(), new RequestStack()); - - self::assertSame($cookie, $provider->getCookie($request)); - self::assertSame($cookie, $provider->getCookie($request)); - } - - /** - * @test - */ - public function it_caches_a_missing_cookie(): void - { - $request = new Request(); - - $decorated = $this->prophesize(CookieProviderInterface::class); - $decorated->getCookie($request)->willReturn(null)->shouldBeCalledOnce(); - - $provider = new CachedCookieProvider($decorated->reveal(), new RequestStack()); - - self::assertNull($provider->getCookie($request)); - self::assertNull($provider->getCookie($request)); - } - - /** - * @test - */ - public function it_resolves_the_cookie_of_each_request_separately(): void - { - // The requests are equal, so only their identity tells them apart. Prophecy compares arguments with ==, so the stub checks the identity itself - $firstRequest = new Request(); - $firstCookie = new Cookie('first-client-id'); - $secondRequest = new Request(); - $secondCookie = new Cookie('second-client-id'); - - $decorated = $this->prophesize(CookieProviderInterface::class); - $decorated->getCookie(Argument::type(Request::class))->will( - static fn (array $arguments): Cookie => $arguments[0] === $firstRequest ? $firstCookie : $secondCookie, - ); - - $provider = new CachedCookieProvider($decorated->reveal(), new RequestStack()); - - self::assertSame($firstCookie, $provider->getCookie($firstRequest)); - self::assertSame($secondCookie, $provider->getCookie($secondRequest)); - } - - /** - * @test - */ - public function it_does_not_give_a_new_request_the_cookie_of_a_freed_request(): void - { - // A double that doesn't hold on to the requests it receives, unlike a Prophecy double - $decorated = new class() implements CookieProviderInterface { - private int $calls = 0; - - public function getCookie(?Request $request = null): Cookie - { - return new Cookie(sprintf('client-id-%d', ++$this->calls)); - } - }; - - $provider = new CachedCookieProvider($decorated, new RequestStack()); - - $firstRequest = new Request(); - $firstRequestId = spl_object_id($firstRequest); - self::assertSame('client-id-1', $provider->getCookie($firstRequest)?->clientId); - unset($firstRequest); - - // PHP reuses the freed object's handle, so this request gets the same spl_object_id() and spl_object_hash() - $secondRequest = new Request(); - self::assertSame($firstRequestId, spl_object_id($secondRequest), 'The new request does not reuse the handle of the freed one, so this test proves nothing'); - self::assertSame('client-id-2', $provider->getCookie($secondRequest)?->clientId); - } - - /** - * @test - */ - public function it_falls_back_to_the_main_request(): void - { - $request = new Request(); - $requestStack = new RequestStack(); - $requestStack->push($request); - - $cookie = new Cookie('client-id'); - - $decorated = $this->prophesize(CookieProviderInterface::class); - $decorated->getCookie($request)->willReturn($cookie); - - $provider = new CachedCookieProvider($decorated->reveal(), $requestStack); - - self::assertSame($cookie, $provider->getCookie()); - } -} diff --git a/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php b/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php deleted file mode 100644 index d749510..0000000 --- a/tests/Unit/CookieProvider/RequestBasedCookieProviderTest.php +++ /dev/null @@ -1,140 +0,0 @@ -getCookie()); - } - - /** - * @test - */ - public function it_returns_null_when_the_cookie_is_not_present(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - - self::assertNull($provider->getCookie(new Request())); - } - - /** - * @test - */ - public function it_returns_null_when_the_cookie_value_is_invalid(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - $request = new Request([], [], [], [self::COOKIE_NAME => '1.2.3']); - - self::assertNull($provider->getCookie($request)); - } - - /** - * @test - */ - public function it_returns_null_for_an_empty_cookie_value(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - $request = new Request([], [], [], [self::COOKIE_NAME => '']); - - self::assertNull($provider->getCookie($request)); - } - - /** - * @test - */ - public function it_returns_null_when_the_client_id_is_empty(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - $request = new Request([], [], [], [self::COOKIE_NAME => '2.1.1.']); - - self::assertNull($provider->getCookie($request)); - } - - /** - * @test - */ - public function it_returns_null_when_the_client_id_is_longer_than_255_characters(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie(str_repeat('a', 256)))->toString()]); - - self::assertNull($provider->getCookie($request)); - } - - /** - * @test - */ - public function it_accepts_a_client_id_of_255_characters(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie(str_repeat('a', 255)))->toString()]); - - self::assertSame(str_repeat('a', 255), $provider->getCookie($request)?->clientId); - } - - /** - * @test - */ - public function it_prefers_the_given_request_over_the_main_request(): void - { - $mainRequest = new Request([], [], [], [self::COOKIE_NAME => (new Cookie('main-id'))->toString()]); - $requestStack = new RequestStack(); - $requestStack->push($mainRequest); - - $given = new Request([], [], [], [self::COOKIE_NAME => (new Cookie('given-id'))->toString()]); - - $provider = new RequestBasedCookieProvider($requestStack, self::COOKIE_NAME); - $cookie = $provider->getCookie($given); - - self::assertNotNull($cookie); - self::assertSame('given-id', $cookie->clientId); - } - - /** - * @test - */ - public function it_parses_the_cookie_from_the_given_request(): void - { - $provider = new RequestBasedCookieProvider(new RequestStack(), self::COOKIE_NAME); - $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie('client-id'))->toString()]); - - $cookie = $provider->getCookie($request); - - self::assertNotNull($cookie); - self::assertSame('client-id', $cookie->clientId); - } - - /** - * @test - */ - public function it_falls_back_to_the_main_request(): void - { - $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie('client-id'))->toString()]); - $requestStack = new RequestStack(); - $requestStack->push($request); - - $provider = new RequestBasedCookieProvider($requestStack, self::COOKIE_NAME); - - $cookie = $provider->getCookie(); - - self::assertNotNull($cookie); - self::assertSame('client-id', $cookie->clientId); - } -} diff --git a/tests/Unit/DependencyInjection/SetonoClientExtensionTest.php b/tests/Unit/DependencyInjection/SetonoClientExtensionTest.php index 88e1e6d..5860d80 100644 --- a/tests/Unit/DependencyInjection/SetonoClientExtensionTest.php +++ b/tests/Unit/DependencyInjection/SetonoClientExtensionTest.php @@ -5,9 +5,9 @@ namespace Setono\ClientBundle\Tests\Unit\DependencyInjection; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; +use Setono\ClientBundle\Context\ClientContext; use Setono\ClientBundle\Context\ClientContextInterface; -use Setono\ClientBundle\Context\DefaultClientContext; -use Setono\ClientBundle\CookieProvider\RequestBasedCookieProvider; +use Setono\ClientBundle\Cookie\ClientCookieJar; use Setono\ClientBundle\DependencyInjection\SetonoClientExtension; use Setono\ClientBundle\Entity\Metadata; use Setono\ClientBundle\EventListener\Doctrine\ConvertToEntityListener; @@ -60,19 +60,19 @@ public function after_loading_the_services_are_registered(): void { $this->load(); - $this->assertContainerBuilderHasService('setono_client.client_context.default', DefaultClientContext::class); - $this->assertContainerBuilderHasService('setono_client.cookie_provider.default', RequestBasedCookieProvider::class); + $this->assertContainerBuilderHasService('setono_client.client_context.default', ClientContext::class); + $this->assertContainerBuilderHasService('setono_client.client_cookie_jar', ClientCookieJar::class); $this->assertContainerBuilderHasAlias(ClientContextInterface::class, 'setono_client.client_context.default'); } /** * @test */ - public function after_loading_the_cached_client_context_is_reset_between_requests(): void + public function after_loading_the_cached_client_context_wraps_every_other_client_context(): void { $this->load(); - $this->assertContainerBuilderHasServiceDefinitionWithTag('setono_client.client_context.cached', 'kernel.reset', ['method' => 'reset']); + $this->assertContainerBuilderServiceDecoration('setono_client.client_context.cached', ClientContextInterface::class, null, -1024); } /** diff --git a/tests/Unit/EventSubscriber/StoreCookieSubscriberTest.php b/tests/Unit/EventSubscriber/StoreCookieSubscriberTest.php index cc9ef5d..08d08ea 100644 --- a/tests/Unit/EventSubscriber/StoreCookieSubscriberTest.php +++ b/tests/Unit/EventSubscriber/StoreCookieSubscriberTest.php @@ -5,13 +5,12 @@ namespace Setono\ClientBundle\Tests\Unit\EventSubscriber; use PHPUnit\Framework\TestCase; -use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Psr\EventDispatcher\EventDispatcherInterface; use Setono\Client\Client; use Setono\Client\Cookie; use Setono\ClientBundle\Context\ClientContextInterface; -use Setono\ClientBundle\CookieProvider\CookieProviderInterface; +use Setono\ClientBundle\Cookie\ClientCookieJar; use Setono\ClientBundle\Event\PreStoreCookieEvent; use Setono\ClientBundle\EventSubscriber\StoreCookieSubscriber; use Symfony\Component\EventDispatcher\EventDispatcher; @@ -38,94 +37,19 @@ public function it_subscribes_to_the_response_event(): void /** * @test */ - public function it_stores_a_new_cookie_when_none_exists(): void + public function it_stores_the_client_cookie_of_the_current_client(): void { $clientContext = $this->prophesize(ClientContextInterface::class); - $clientContext->getClient()->willReturn(new Client('client-id')); - - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie(Argument::type(Request::class))->willReturn(null); + $clientContext->getClient()->willReturn(new Client('current-client-id')); + $request = new Request([], [], [], [self::COOKIE_NAME => (new Cookie('cookie-client-id'))->toString()]); $response = new Response(); - $this->createSubscriber($clientContext->reveal(), $cookieProvider->reveal())->store( - $this->createResponseEvent(new Request(), $response), - ); + $this->createSubscriber($clientContext->reveal())->store($this->createResponseEvent($request, $response)); $cookies = $response->headers->getCookies(); self::assertCount(1, $cookies); self::assertSame(self::COOKIE_NAME, $cookies[0]->getName()); - self::assertFalse($cookies[0]->isHttpOnly()); - self::assertSame('client-id', Cookie::fromString((string) $cookies[0]->getValue())->clientId); - } - - /** - * @test - */ - public function it_reuses_the_client_id_from_an_existing_cookie(): void - { - $clientContext = $this->prophesize(ClientContextInterface::class); - $clientContext->getClient()->shouldNotBeCalled(); - - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie(Argument::type(Request::class))->willReturn(new Cookie('existing-id')); - - $response = new Response(); - $this->createSubscriber($clientContext->reveal(), $cookieProvider->reveal())->store( - $this->createResponseEvent(new Request(), $response), - ); - - $cookies = $response->headers->getCookies(); - self::assertCount(1, $cookies); - self::assertSame('existing-id', Cookie::fromString((string) $cookies[0]->getValue())->clientId); - } - - /** - * @test - */ - public function it_refreshes_the_last_seen_timestamp_and_keeps_the_first_seen_timestamp(): void - { - $clientContext = $this->prophesize(ClientContextInterface::class); - - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie(Argument::type(Request::class))->willReturn(new Cookie('existing-id', 2, 1_000, 2_000)); - - $before = time(); - $response = new Response(); - $this->createSubscriber($clientContext->reveal(), $cookieProvider->reveal())->store( - $this->createResponseEvent(new Request(), $response), - ); - - $cookie = Cookie::fromString((string) $response->headers->getCookies()[0]->getValue()); - self::assertSame(1_000, $cookie->firstSeenAt); - self::assertGreaterThanOrEqual($before, $cookie->lastSeenAt); - self::assertLessThanOrEqual(time(), $cookie->lastSeenAt); - } - - /** - * @test - */ - public function it_uses_the_configured_cookie_name_and_expiration(): void - { - $clientContext = $this->prophesize(ClientContextInterface::class); - $clientContext->getClient()->willReturn(new Client('client-id')); - - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie(Argument::type(Request::class))->willReturn(null); - - $before = time(); - $response = new Response(); - $this->createSubscriber( - $clientContext->reveal(), - $cookieProvider->reveal(), - cookieName: 'custom_name', - cookieExpiration: '+30 days', - )->store($this->createResponseEvent(new Request(), $response)); - - $cookies = $response->headers->getCookies(); - self::assertCount(1, $cookies); - self::assertSame('custom_name', $cookies[0]->getName()); - self::assertGreaterThanOrEqual(strtotime('+30 days', $before), $cookies[0]->getExpiresTime()); - self::assertLessThanOrEqual(strtotime('+30 days'), $cookies[0]->getExpiresTime()); + self::assertSame('current-client-id', Cookie::fromString((string) $cookies[0]->getValue())->clientId); } /** @@ -134,11 +58,10 @@ public function it_uses_the_configured_cookie_name_and_expiration(): void public function it_does_not_store_a_cookie_for_sub_requests(): void { $clientContext = $this->prophesize(ClientContextInterface::class); - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie(Argument::cetera())->shouldNotBeCalled(); + $clientContext->getClient()->shouldNotBeCalled(); $response = new Response(); - $this->createSubscriber($clientContext->reveal(), $cookieProvider->reveal())->store( + $this->createSubscriber($clientContext->reveal())->store( $this->createResponseEvent(new Request(), $response, HttpKernelInterface::SUB_REQUEST), ); @@ -151,30 +74,29 @@ public function it_does_not_store_a_cookie_for_sub_requests(): void public function it_does_not_store_a_cookie_when_the_pre_store_event_vetoes_it(): void { $clientContext = $this->prophesize(ClientContextInterface::class); - $cookieProvider = $this->prophesize(CookieProviderInterface::class); - $cookieProvider->getCookie(Argument::cetera())->shouldNotBeCalled(); + $clientContext->getClient()->shouldNotBeCalled(); + + $request = new Request(); + $response = new Response(); $eventDispatcher = new EventDispatcher(); - $eventDispatcher->addListener(PreStoreCookieEvent::class, static function (PreStoreCookieEvent $event): void { + $eventDispatcher->addListener(PreStoreCookieEvent::class, static function (PreStoreCookieEvent $event) use ($request, $response): void { + self::assertSame($request, $event->request); + self::assertSame($response, $event->response); + $event->store = false; }); - $response = new Response(); - $this->createSubscriber($clientContext->reveal(), $cookieProvider->reveal(), $eventDispatcher)->store( - $this->createResponseEvent(new Request(), $response), - ); + $this->createSubscriber($clientContext->reveal(), $eventDispatcher)->store($this->createResponseEvent($request, $response)); self::assertCount(0, $response->headers->getCookies()); } private function createSubscriber( ClientContextInterface $clientContext, - CookieProviderInterface $cookieProvider, EventDispatcherInterface $eventDispatcher = new EventDispatcher(), - string $cookieName = self::COOKIE_NAME, - string $cookieExpiration = '+365 days', ): StoreCookieSubscriber { - return new StoreCookieSubscriber($clientContext, $cookieProvider, $eventDispatcher, $cookieName, $cookieExpiration); + return new StoreCookieSubscriber($clientContext, new ClientCookieJar(self::COOKIE_NAME, '+365 days'), $eventDispatcher); } private function createResponseEvent(