diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 3be46fb..0c69cb4 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -132,7 +132,7 @@ jobs: SYMFONY_REQUIRE: "${{ matrix.symfony }}" - name: "Static analysis" - run: "vendor/bin/psalm --php-version=${{ matrix.php-version }}" + run: "vendor/bin/phpstan analyse --no-progress" unit-tests: name: "Unit tests" @@ -179,6 +179,39 @@ jobs: - name: "Run phpunit" run: "composer phpunit" + mutation-testing: + name: "Mutation Testing" + + runs-on: "ubuntu-latest" + + strategy: + matrix: + php-version: + - "8.2" + + dependencies: + - "highest" + + steps: + - name: "Checkout" + uses: "actions/checkout@v4" + + - name: "Setup PHP, with composer and extensions" + uses: "shivammathur/setup-php@v2" + with: + coverage: "pcov" + extensions: "${{ env.PHP_EXTENSIONS }}" + php-version: "${{ matrix.php-version }}" + tools: "flex" + + - name: "Install composer dependencies" + uses: "ramsey/composer-install@v3" + with: + dependency-versions: "${{ matrix.dependencies }}" + + - name: "Run Infection" + run: "vendor/bin/infection --ansi --no-interaction --no-progress --show-mutations --min-msi=100 --min-covered-msi=100" + code-coverage: name: "Code Coverage" diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..b63c3b0 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,72 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +A Symfony bundle (`setono/client-bundle`) that integrates the `setono/client` library into a Symfony app to **track users between visits**. It stores a cookie (`setono_client_id` by default) holding a client id plus first/last-seen timestamps, and optionally persists per-client key/value **metadata** to a database table. Metadata is lazy: nothing is queried or written unless the application actually reads or mutates it. + +This is a library/bundle — there is no runnable application here. It is exercised through its test suite and consumed by host Symfony apps. + +## Commands + +PHP 8.1+ is required (CI runs 8.1 and 8.2). Use the `8.1`/`8.2` shell switchers if needed. The dev tooling is the individually-pinned set from `setono/code-quality-pack` (inlined into `require-dev` rather than depending on the meta-package); PHPUnit is held at `^10.5` so PHP 8.1 stays supported (PHPUnit 11 needs 8.2). + +- Tests: `composer phpunit` (or `vendor/bin/phpunit`) +- Single test: `vendor/bin/phpunit --filter after_loading_the_correct_parameter_has_been_set` or by path `vendor/bin/phpunit tests/DependencyInjection/SetonoClientExtensionTest.php` +- Static analysis: `composer analyse` (PHPStan at `level: max`, config in `phpstan.neon.dist`; auto-loads the doctrine/symfony/phpunit/strict-rules extensions via `phpstan/extension-installer`) +- Check style: `composer check-style` (ECS, Sylius Labs ruleset) +- Fix style: `composer fix-style` +- Mutation testing: `composer infection` (config in `infection.json.dist`; needs a coverage driver — pcov/xdebug). CI gates on `--min-msi=100 --min-covered-msi=100`. One equivalent mutant (the defensive `instanceof ClassMetadata` in `ConvertToEntityListener`) is excluded in the config. +- Dependency analysis: `vendor/bin/composer-dependency-analyser` (no composer script; see `composer-dependency-analyser.php`) +- Rector (dry-run): `vendor/bin/rector process --dry-run` (config in `rector.php`, `withPhpSets(php81: true)`; not run in CI) + +CI (`.github/workflows/build.yaml`) additionally runs `composer validate --strict` and `composer normalize --dry-run` across the PHP 8.1/8.2 × Symfony 6.4/7.0 matrix. + +> Local note (PHP 8.4): the transitive dev dep `thecodingmachine/safe` emits a flood of implicit-nullable `E_DEPRECATED` notices on PHP 8.4 (they don't exist on CI's 8.1/8.2). This is harmless for PHPStan/PHPUnit but breaks Infection's initial test run — run it with `vendor/bin/infection --initial-tests-php-options="-d error_reporting=24575"` locally on 8.4. + +## Architecture + +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) + +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: + +- **Client context** — runtime order outermost→innermost: `CachedClientContext` (prio 32) → `CookieBasedClientContext` (prio 64) → `DefaultClientContext`. + - `Cached` memoizes the `Client` for the request. + - `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` via `spl_object_hash`) → `RequestBasedCookieProvider` (parses the cookie off the request). +- **Metadata provider** — `DoctrineOrmBasedMetadataProvider` (prio 64) → `EmptyMetadataProvider` (default). Doctrine loads from DB, falling back to the decorated provider when no row exists. + +### 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/StoreMetadataSubscriber` (on `kernel.finish_request`): calls the metadata persister for the current client. + +### Lazy metadata (why writes are cheap) + +`Client/ChangeAwareMetadata` extends `setono/client`'s `Metadata` and flips a `dirty` flag on `set`/`remove`. `Client/LazyChangeAwareMetadata` adds Symfony VarExporter's `LazyGhostTrait`, so `DoctrineOrmBasedMetadataProvider` returns a lazy ghost whose DB query only fires on first access. `DoctrineOrmBasedMetadataPersister::persist()` then **short-circuits** in two cases: the metadata is a lazy ghost that was never initialized (never read), or it is not dirty (never mutated). This is what makes "lazy loaded metadata" in the README real — no SELECT and no flush unless the app touched metadata. + +### Entity mapping + +`Entity/Metadata` (table `setono_client__metadata`, string id `clientId`, nullable `json` `metadata`) is mapped in `config/doctrine-mapping/Metadata.orm.xml` as a **mapped-superclass**. `EventListener/Doctrine/ConvertToEntityListener` listens on Doctrine's `loadClassMetadata` and flips `isMappedSuperclass = false` so the bundle's own `Metadata` becomes a concrete entity when the app provides no replacement. Apps can swap in a custom class via the `metadata_class` config option; `SetonoClientExtension::load()` enforces that it implements `Entity/MetadataInterface` and they are responsible for mapping it. + +### Configuration + +Defined in `DependencyInjection/Configuration.php`, mapped to container parameters in `SetonoClientExtension`: + +```yaml +setono_client: + cookie: + name: setono_client_id # changing this makes existing clients appear new + expiration: '+365 days' # any strtotime-parsable string + metadata_class: Setono\ClientBundle\Entity\Metadata +``` + +## Conventions + +- Strict types everywhere (`declare(strict_types=1)`); concrete classes are `final`, except the metadata/entity classes (`ChangeAwareMetadata`, `LazyChangeAwareMetadata`, `Entity\Metadata`) which are intentionally extensible. +- New services go in `config/services.xml` using the `setono_client..` id scheme, with a `setono_client..default` alias and an interface alias for autowiring. To insert behavior into a chain, add a decorator with an appropriate `decoration-priority` rather than editing existing classes. +- Tests use `matthiasnoback/symfony-dependency-injection-test` (e.g. `AbstractExtensionTestCase`) and Prophecy (`ProphecyTrait`), with the `@test` annotation and snake_case method-name style. Keep the suite at 100% MSI — when adding logic, add a test that kills the corresponding mutant. diff --git a/LICENSE b/LICENSE index ddee45d..84b974a 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2024 Setono +Copyright (c) 2026 Setono Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/composer.json b/composer.json index c637ba1..9f340eb 100644 --- a/composer.json +++ b/composer.json @@ -27,13 +27,23 @@ "symfony/var-exporter": "^6.4 || ^7.0" }, "require-dev": { + "ergebnis/composer-normalize": "^2.50", + "infection/infection": "^0.27 || ^0.28", + "jangregor/phpstan-prophecy": "^2.3", "matthiasnoback/symfony-dependency-injection-test": "^4.3.1 || ^5.1", - "phpspec/prophecy-phpunit": "^2.2", + "phpspec/prophecy": "^1.20", + "phpspec/prophecy-phpunit": "^2.5", + "phpstan/extension-installer": "^1.4.3", + "phpstan/phpstan": "^2.1.46", + "phpstan/phpstan-doctrine": "^2.0", + "phpstan/phpstan-phpunit": "^2.0.16", + "phpstan/phpstan-strict-rules": "^2.0.10", + "phpstan/phpstan-symfony": "^2.0", + "phpstan/phpstan-webmozart-assert": "^2.0", "phpunit/phpunit": "^10.5", - "psalm/plugin-phpunit": "^0.19", - "psalm/plugin-symfony": "^5.1", - "setono/code-quality-pack": "^2.7", - "shipmonk/composer-dependency-analyser": "^1.8.2" + "rector/rector": "^2.4.1", + "shipmonk/composer-dependency-analyser": "^1.8.4", + "sylius-labs/coding-standard": "^4.5" }, "prefer-stable": true, "autoload": { @@ -49,14 +59,22 @@ "config": { "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": false, - "ergebnis/composer-normalize": true + "ergebnis/composer-normalize": true, + "infection/extension-installer": true, + "phpstan/extension-installer": true + }, + "policy": { + "advisories": { + "block": false + } }, "sort-packages": true }, "scripts": { - "analyse": "psalm", + "analyse": "phpstan analyse", "check-style": "ecs check", "fix-style": "ecs check --fix", + "infection": "infection", "phpunit": "phpunit" } } diff --git a/infection.json.dist b/infection.json.dist new file mode 100644 index 0000000..4fe82dd --- /dev/null +++ b/infection.json.dist @@ -0,0 +1,16 @@ +{ + "$schema": "vendor/infection/infection/resources/schema.json", + "source": { + "directories": [ + "src" + ] + }, + "mutators": { + "@default": true, + "InstanceOf_": { + "ignore": [ + "Setono\\ClientBundle\\EventListener\\Doctrine\\ConvertToEntityListener::loadClassMetadata" + ] + } + } +} diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 0000000..0d978a3 --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,14 @@ +parameters: + level: max + phpVersion: 80100 + paths: + - src + - tests + ignoreErrors: + # Symfony's LazyGhostTrait initializer must call __construct() on the ghost + # instance to populate it. This is the documented idiom for lazy ghosts, so + # phpstan-strict-rules' ban on calling __construct() does not apply here. + - + identifier: constructor.call + path: src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php + count: 1 diff --git a/psalm.xml b/psalm.xml deleted file mode 100644 index 8afdc20..0000000 --- a/psalm.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - - - - - - - - - diff --git a/rector.php b/rector.php index b8f240b..f7c0a70 100644 --- a/rector.php +++ b/rector.php @@ -3,15 +3,10 @@ declare(strict_types=1); use Rector\Config\RectorConfig; -use Rector\Set\ValueObject\LevelSetList; -return static function (RectorConfig $rectorConfig): void { - $rectorConfig->paths([ +return RectorConfig::configure() + ->withPaths([ __DIR__ . '/src', - __DIR__ . '/tests/Unit', - ]); - - $rectorConfig->sets([ - LevelSetList::UP_TO_PHP_74 - ]); -}; + __DIR__ . '/tests', + ]) + ->withPhpSets(php81: true); diff --git a/src/Client/ChangeAwareMetadata.php b/src/Client/ChangeAwareMetadata.php index 458b0c0..84c393a 100644 --- a/src/Client/ChangeAwareMetadata.php +++ b/src/Client/ChangeAwareMetadata.php @@ -15,7 +15,7 @@ public function isDirty(): bool return $this->dirty; } - public function set(string $key, mixed $value, int $ttl = null): void + public function set(string $key, mixed $value, ?int $ttl = null): void { parent::set($key, $value, $ttl); diff --git a/src/Client/LazyChangeAwareMetadata.php b/src/Client/LazyChangeAwareMetadata.php index 6f65daa..b1726b1 100644 --- a/src/Client/LazyChangeAwareMetadata.php +++ b/src/Client/LazyChangeAwareMetadata.php @@ -6,9 +6,6 @@ use Symfony\Component\VarExporter\LazyGhostTrait; -/** - * @psalm-suppress PropertyNotSetInConstructor - */ class LazyChangeAwareMetadata extends ChangeAwareMetadata { use LazyGhostTrait; diff --git a/src/CookieProvider/CachedCookieProvider.php b/src/CookieProvider/CachedCookieProvider.php index ae9c589..828e5e3 100644 --- a/src/CookieProvider/CachedCookieProvider.php +++ b/src/CookieProvider/CachedCookieProvider.php @@ -19,9 +19,9 @@ public function __construct( ) { } - public function getCookie(Request $request = null): ?Cookie + public function getCookie(?Request $request = null): ?Cookie { - $request = $request ?? $this->requestStack->getMainRequest(); + $request ??= $this->requestStack->getMainRequest(); if (null === $request) { return null; } diff --git a/src/CookieProvider/CookieProviderInterface.php b/src/CookieProvider/CookieProviderInterface.php index 2a0d2b6..7cd0ef4 100644 --- a/src/CookieProvider/CookieProviderInterface.php +++ b/src/CookieProvider/CookieProviderInterface.php @@ -12,5 +12,5 @@ interface CookieProviderInterface /** * @param Request|null $request if null, the main request will be used */ - public function getCookie(Request $request = null): ?Cookie; + public function getCookie(?Request $request = null): ?Cookie; } diff --git a/src/CookieProvider/RequestBasedCookieProvider.php b/src/CookieProvider/RequestBasedCookieProvider.php index 580d7a0..4b9292d 100644 --- a/src/CookieProvider/RequestBasedCookieProvider.php +++ b/src/CookieProvider/RequestBasedCookieProvider.php @@ -16,7 +16,7 @@ public function __construct( ) { } - public function getCookie(Request $request = null): ?Cookie + public function getCookie(?Request $request = null): ?Cookie { $cookieValue = ($request ?? $this->requestStack->getMainRequest())?->cookies->get($this->cookieName); if (!is_string($cookieValue) || '' === $cookieValue) { diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 5bbd499..9f9c238 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -18,7 +18,6 @@ public function getConfigTreeBuilder(): TreeBuilder /** @var ArrayNodeDefinition $rootNode */ $rootNode = $treeBuilder->getRootNode(); - /** @psalm-suppress MixedMethodCall, UndefinedInterfaceMethod, PossiblyNullReference */ $rootNode ->addDefaultsIfNotSet() ->children() diff --git a/src/DependencyInjection/SetonoClientExtension.php b/src/DependencyInjection/SetonoClientExtension.php index 68f9efc..4af2c3c 100644 --- a/src/DependencyInjection/SetonoClientExtension.php +++ b/src/DependencyInjection/SetonoClientExtension.php @@ -15,8 +15,6 @@ final class SetonoClientExtension extends Extension public function load(array $configs, ContainerBuilder $container): void { /** - * @psalm-suppress PossiblyNullArgument - * * @var array{cookie: array{name: string, expiration: string}, metadata_class: class-string} $config */ $config = $this->processConfiguration($this->getConfiguration([], $container), $configs); diff --git a/src/Entity/Metadata.php b/src/Entity/Metadata.php index 9686a97..9be4679 100644 --- a/src/Entity/Metadata.php +++ b/src/Entity/Metadata.php @@ -8,7 +8,7 @@ class Metadata implements MetadataInterface { protected ?string $clientId = null; - /** @var null|array{__expires?: array, ...} */ + /** @var array|null */ protected ?array $metadata = []; public function getClientId(): ?string @@ -26,6 +26,9 @@ public function getMetadata(): array return $this->metadata ?? []; } + /** + * @param array $metadata + */ public function setMetadata(array $metadata): void { $this->metadata = [] === $metadata ? null : $metadata; diff --git a/src/Entity/MetadataInterface.php b/src/Entity/MetadataInterface.php index 57bf575..b39f407 100644 --- a/src/Entity/MetadataInterface.php +++ b/src/Entity/MetadataInterface.php @@ -11,7 +11,7 @@ public function getClientId(): ?string; public function setClientId(string $clientId): void; /** - * @return array{__expires?: array, ...} + * @return array */ public function getMetadata(): array; diff --git a/src/EventListener/Doctrine/ConvertToEntityListener.php b/src/EventListener/Doctrine/ConvertToEntityListener.php index cda6a88..2708c82 100644 --- a/src/EventListener/Doctrine/ConvertToEntityListener.php +++ b/src/EventListener/Doctrine/ConvertToEntityListener.php @@ -14,6 +14,9 @@ */ final class ConvertToEntityListener { + /** + * @param LoadClassMetadataEventArgs<\Doctrine\Persistence\Mapping\ClassMetadata, \Doctrine\Persistence\ObjectManager> $eventArgs + */ public function loadClassMetadata(LoadClassMetadataEventArgs $eventArgs): void { if (!is_a($eventArgs->getClassMetadata()->getName(), Metadata::class, true)) { diff --git a/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php b/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php index 2f92cb1..e5aa281 100644 --- a/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php +++ b/src/MetadataProvider/DoctrineOrmBasedMetadataProvider.php @@ -39,11 +39,6 @@ public function getMetadata(string $clientId): Metadata $metadata = $entity->getMetadata(); } - /** - * todo remove the MixedArgumentTypeCoercion suppression when this issue is fixed: https://github.com/vimeo/psalm/issues/10964 - * - * @psalm-suppress DirectConstructorCall,MixedArgumentTypeCoercion - */ $instance->__construct($metadata); }); } diff --git a/tests/Client/ChangeAwareMetadataTest.php b/tests/Client/ChangeAwareMetadataTest.php new file mode 100644 index 0000000..a3f161e --- /dev/null +++ b/tests/Client/ChangeAwareMetadataTest.php @@ -0,0 +1,46 @@ +isDirty()); + self::assertFalse((new ChangeAwareMetadata(['foo' => 'bar']))->isDirty()); + } + + /** + * @test + */ + public function it_becomes_dirty_when_setting_a_value(): void + { + $metadata = new ChangeAwareMetadata(); + $metadata->set('foo', 'bar'); + + self::assertTrue($metadata->isDirty()); + self::assertSame('bar', $metadata->get('foo')); + } + + /** + * @test + */ + public function it_becomes_dirty_when_removing_a_value(): void + { + $metadata = new ChangeAwareMetadata(['foo' => 'bar']); + self::assertFalse($metadata->isDirty()); + + $metadata->remove('foo'); + + self::assertTrue($metadata->isDirty()); + self::assertFalse($metadata->has('foo')); + } +} diff --git a/tests/Context/CachedClientContextTest.php b/tests/Context/CachedClientContextTest.php new file mode 100644 index 0000000..fe92ba0 --- /dev/null +++ b/tests/Context/CachedClientContextTest.php @@ -0,0 +1,42 @@ +prophesize(ClientContextInterface::class); + $decorated->getClient()->willReturn($client); + + self::assertSame($client, (new CachedClientContext($decorated->reveal()))->getClient()); + } + + /** + * @test + */ + public function it_only_resolves_the_client_once(): void + { + $decorated = $this->prophesize(ClientContextInterface::class); + $decorated->getClient()->willReturn(new Client('id'))->shouldBeCalledOnce(); + + $context = new CachedClientContext($decorated->reveal()); + + self::assertSame($context->getClient(), $context->getClient()); + } +} diff --git a/tests/Context/CookieBasedClientContextTest.php b/tests/Context/CookieBasedClientContextTest.php new file mode 100644 index 0000000..6e6a1ad --- /dev/null +++ b/tests/Context/CookieBasedClientContextTest.php @@ -0,0 +1,74 @@ +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/Context/DefaultClientContextTest.php b/tests/Context/DefaultClientContextTest.php new file mode 100644 index 0000000..a646776 --- /dev/null +++ b/tests/Context/DefaultClientContextTest.php @@ -0,0 +1,25 @@ +getClient(); + $metadata = $client->metadata; + + self::assertNotSame('', $client->id); + self::assertInstanceOf(ChangeAwareMetadata::class, $metadata); + self::assertFalse($metadata->isDirty()); + } +} diff --git a/tests/Controller/ClientValueResolverTest.php b/tests/Controller/ClientValueResolverTest.php index 6192dfe..8665f48 100644 --- a/tests/Controller/ClientValueResolverTest.php +++ b/tests/Controller/ClientValueResolverTest.php @@ -28,6 +28,20 @@ public function it_resolves(): void $resolved = $resolver->resolve(new Request(), new ArgumentMetadata('foo', Client::class, false, false, null)); self::assertCount(1, $resolved); - self::assertInstanceOf(Client::class, $resolved[0]); + self::assertSame('id', $resolved[0]->id); + } + + /** + * @test + */ + public function it_does_not_resolve_arguments_that_are_not_clients(): void + { + $clientContext = $this->prophesize(ClientContextInterface::class); + $clientContext->getClient()->shouldNotBeCalled(); + + $resolver = new ClientValueResolver($clientContext->reveal()); + $resolved = $resolver->resolve(new Request(), new ArgumentMetadata('foo', \stdClass::class, false, false, null)); + + self::assertSame([], $resolved); } } diff --git a/tests/CookieProvider/CachedCookieProviderTest.php b/tests/CookieProvider/CachedCookieProviderTest.php new file mode 100644 index 0000000..e55618a --- /dev/null +++ b/tests/CookieProvider/CachedCookieProviderTest.php @@ -0,0 +1,68 @@ +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_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/CookieProvider/RequestBasedCookieProviderTest.php b/tests/CookieProvider/RequestBasedCookieProviderTest.php new file mode 100644 index 0000000..eb4b28e --- /dev/null +++ b/tests/CookieProvider/RequestBasedCookieProviderTest.php @@ -0,0 +1,107 @@ +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_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/DependencyInjection/SetonoClientExtensionTest.php b/tests/DependencyInjection/SetonoClientExtensionTest.php index 6392c48..47778c3 100644 --- a/tests/DependencyInjection/SetonoClientExtensionTest.php +++ b/tests/DependencyInjection/SetonoClientExtensionTest.php @@ -5,6 +5,9 @@ namespace Setono\ClientBundle\Tests\DependencyInjection; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; +use Setono\ClientBundle\Context\ClientContextInterface; +use Setono\ClientBundle\Context\DefaultClientContext; +use Setono\ClientBundle\CookieProvider\RequestBasedCookieProvider; use Setono\ClientBundle\DependencyInjection\SetonoClientExtension; use Setono\ClientBundle\Entity\Metadata; @@ -28,4 +31,26 @@ public function after_loading_the_correct_parameter_has_been_set(): void $this->assertContainerBuilderHasParameter('setono_client.cookie.expiration', '+365 days'); $this->assertContainerBuilderHasParameter('setono_client.metadata_class', Metadata::class); } + + /** + * @test + */ + 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->assertContainerBuilderHasAlias(ClientContextInterface::class, 'setono_client.client_context.default'); + } + + /** + * @test + */ + public function it_throws_when_the_metadata_class_does_not_implement_the_interface(): void + { + $this->expectException(\InvalidArgumentException::class); + + $this->load(['metadata_class' => \stdClass::class]); + } } diff --git a/tests/Entity/MetadataTest.php b/tests/Entity/MetadataTest.php new file mode 100644 index 0000000..1aab1f7 --- /dev/null +++ b/tests/Entity/MetadataTest.php @@ -0,0 +1,54 @@ +getClientId()); + + $metadata->setClientId('client-id'); + self::assertSame('client-id', $metadata->getClientId()); + } + + /** + * @test + */ + public function it_returns_an_empty_array_by_default(): void + { + self::assertSame([], (new Metadata())->getMetadata()); + } + + /** + * @test + */ + public function it_stores_metadata(): void + { + $metadata = new Metadata(); + $metadata->setMetadata(['foo' => 'bar']); + + self::assertSame(['foo' => 'bar'], $metadata->getMetadata()); + } + + /** + * @test + */ + public function it_normalizes_empty_metadata_to_an_empty_array(): void + { + $metadata = new Metadata(); + $metadata->setMetadata(['foo' => 'bar']); + $metadata->setMetadata([]); + + self::assertSame([], $metadata->getMetadata()); + } +} diff --git a/tests/EventListener/Doctrine/ConvertToEntityListenerTest.php b/tests/EventListener/Doctrine/ConvertToEntityListenerTest.php new file mode 100644 index 0000000..a7457ac --- /dev/null +++ b/tests/EventListener/Doctrine/ConvertToEntityListenerTest.php @@ -0,0 +1,54 @@ +isMappedSuperclass = true; + + (new ConvertToEntityListener())->loadClassMetadata($this->createEventArgs($classMetadata)); + + self::assertFalse($classMetadata->isMappedSuperclass); + } + + /** + * @test + */ + public function it_leaves_other_classes_untouched(): void + { + $classMetadata = new ClassMetadata(\stdClass::class); + $classMetadata->isMappedSuperclass = true; + + (new ConvertToEntityListener())->loadClassMetadata($this->createEventArgs($classMetadata)); + + self::assertTrue($classMetadata->isMappedSuperclass); + } + + /** + * @param ClassMetadata $classMetadata + * + * @return LoadClassMetadataEventArgs, EntityManagerInterface> + */ + private function createEventArgs(ClassMetadata $classMetadata): LoadClassMetadataEventArgs + { + return new LoadClassMetadataEventArgs($classMetadata, $this->prophesize(EntityManagerInterface::class)->reveal()); + } +} diff --git a/tests/EventSubscriber/StoreCookieSubscriberTest.php b/tests/EventSubscriber/StoreCookieSubscriberTest.php new file mode 100644 index 0000000..8a5c4bf --- /dev/null +++ b/tests/EventSubscriber/StoreCookieSubscriberTest.php @@ -0,0 +1,150 @@ + 'store'], StoreCookieSubscriber::getSubscribedEvents()); + } + + /** + * @test + */ + public function it_stores_a_new_cookie_when_none_exists(): 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); + + $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(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_does_not_store_a_cookie_for_sub_requests(): void + { + $clientContext = $this->prophesize(ClientContextInterface::class); + $cookieProvider = $this->prophesize(CookieProviderInterface::class); + $cookieProvider->getCookie(Argument::cetera())->shouldNotBeCalled(); + + $response = new Response(); + $this->createSubscriber($clientContext->reveal(), $cookieProvider->reveal())->store( + $this->createResponseEvent(new Request(), $response, HttpKernelInterface::SUB_REQUEST), + ); + + self::assertCount(0, $response->headers->getCookies()); + } + + /** + * @test + */ + 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(); + + $eventDispatcher = new EventDispatcher(); + $eventDispatcher->addListener(PreStoreCookieEvent::class, static function (PreStoreCookieEvent $event): void { + $event->store = false; + }); + + $response = new Response(); + $subscriber = new StoreCookieSubscriber( + $clientContext->reveal(), + $cookieProvider->reveal(), + $eventDispatcher, + self::COOKIE_NAME, + '+365 days', + ); + $subscriber->store($this->createResponseEvent(new Request(), $response)); + + self::assertCount(0, $response->headers->getCookies()); + } + + private function createSubscriber( + ClientContextInterface $clientContext, + CookieProviderInterface $cookieProvider, + ): StoreCookieSubscriber { + return new StoreCookieSubscriber( + $clientContext, + $cookieProvider, + new EventDispatcher(), + self::COOKIE_NAME, + '+365 days', + ); + } + + private function createResponseEvent( + Request $request, + Response $response, + int $requestType = HttpKernelInterface::MAIN_REQUEST, + ): ResponseEvent { + return new ResponseEvent( + $this->prophesize(HttpKernelInterface::class)->reveal(), + $request, + $requestType, + $response, + ); + } +} diff --git a/tests/EventSubscriber/StoreMetadataSubscriberTest.php b/tests/EventSubscriber/StoreMetadataSubscriberTest.php new file mode 100644 index 0000000..b5cb0cd --- /dev/null +++ b/tests/EventSubscriber/StoreMetadataSubscriberTest.php @@ -0,0 +1,71 @@ + 'store'], StoreMetadataSubscriber::getSubscribedEvents()); + } + + /** + * @test + */ + public function it_persists_the_metadata_of_the_current_client(): void + { + $client = new Client('client-id'); + + $clientContext = $this->prophesize(ClientContextInterface::class); + $clientContext->getClient()->willReturn($client); + + $persister = $this->prophesize(MetadataPersisterInterface::class); + $persister->persist($client)->shouldBeCalledOnce(); + + $subscriber = new StoreMetadataSubscriber($clientContext->reveal(), $persister->reveal()); + $subscriber->store($this->createFinishRequestEvent(HttpKernelInterface::MAIN_REQUEST)); + } + + /** + * @test + */ + public function it_does_nothing_for_sub_requests(): void + { + $clientContext = $this->prophesize(ClientContextInterface::class); + $clientContext->getClient()->shouldNotBeCalled(); + + $persister = $this->prophesize(MetadataPersisterInterface::class); + $persister->persist(Argument::cetera())->shouldNotBeCalled(); + + $subscriber = new StoreMetadataSubscriber($clientContext->reveal(), $persister->reveal()); + $subscriber->store($this->createFinishRequestEvent(HttpKernelInterface::SUB_REQUEST)); + } + + private function createFinishRequestEvent(int $requestType): FinishRequestEvent + { + return new FinishRequestEvent( + $this->prophesize(HttpKernelInterface::class)->reveal(), + new Request(), + $requestType, + ); + } +} diff --git a/tests/MetadataPersister/DoctrineOrmBasedMetadataPersisterTest.php b/tests/MetadataPersister/DoctrineOrmBasedMetadataPersisterTest.php new file mode 100644 index 0000000..452b279 --- /dev/null +++ b/tests/MetadataPersister/DoctrineOrmBasedMetadataPersisterTest.php @@ -0,0 +1,182 @@ +prophesize(EntityManagerInterface::class); + $manager->find(Argument::cetera())->shouldNotBeCalled(); + $manager->persist(Argument::cetera())->shouldNotBeCalled(); + $manager->flush()->shouldNotBeCalled(); + + $managerRegistry = $this->createManagerRegistry($manager); + + $metadata = (new DoctrineOrmBasedMetadataProvider( + new EmptyMetadataProvider(), + $managerRegistry->reveal(), + MetadataEntity::class, + ))->getMetadata('client-id'); + + self::assertInstanceOf(LazyChangeAwareMetadata::class, $metadata); + self::assertFalse($metadata->isLazyObjectInitialized()); + + $this->createPersister($managerRegistry)->persist(new Client('client-id', $metadata)); + + self::assertFalse($metadata->isLazyObjectInitialized()); + } + + /** + * @test + */ + public function it_does_nothing_when_the_metadata_is_not_dirty(): void + { + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->find(Argument::cetera())->shouldNotBeCalled(); + $manager->persist(Argument::cetera())->shouldNotBeCalled(); + $manager->flush()->shouldNotBeCalled(); + + $managerRegistry = $this->createManagerRegistry($manager); + + $metadata = new ChangeAwareMetadata(); + $this->createPersister($managerRegistry)->persist(new Client('client-id', $metadata)); + + self::assertFalse($metadata->isDirty()); + } + + /** + * @test + */ + public function it_persists_a_new_entity(): void + { + $persistedEntity = null; + + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->find(MetadataEntity::class, 'client-id')->willReturn(null); + $manager->persist(Argument::type(MetadataEntity::class))->will( + function (array $args) use (&$persistedEntity): void { + $persistedEntity = $args[0]; + }, + )->shouldBeCalledOnce(); + $manager->flush()->shouldBeCalledOnce(); + + $metadata = new ChangeAwareMetadata(); + $metadata->set('foo', 'bar'); + + $this->createPersister($this->createManagerRegistry($manager))->persist(new Client('client-id', $metadata)); + + self::assertInstanceOf(MetadataEntity::class, $persistedEntity); + self::assertSame('client-id', $persistedEntity->getClientId()); + self::assertSame(['foo' => 'bar'], $persistedEntity->getMetadata()); + } + + /** + * @test + */ + public function it_persists_metadata_that_does_not_track_changes(): void + { + $persistedEntity = null; + + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->find(MetadataEntity::class, 'client-id')->willReturn(null); + $manager->persist(Argument::type(MetadataEntity::class))->will( + function (array $args) use (&$persistedEntity): void { + $persistedEntity = $args[0]; + }, + )->shouldBeCalledOnce(); + $manager->flush()->shouldBeCalledOnce(); + + $client = new Client('client-id', new Metadata(['foo' => 'bar'])); + + $this->createPersister($this->createManagerRegistry($manager))->persist($client); + + self::assertInstanceOf(MetadataEntity::class, $persistedEntity); + self::assertSame(['foo' => 'bar'], $persistedEntity->getMetadata()); + } + + /** + * @test + */ + public function it_updates_an_existing_entity(): void + { + $existing = new MetadataEntity(); + $existing->setClientId('client-id'); + + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->find(MetadataEntity::class, 'client-id')->willReturn($existing); + $manager->persist(Argument::cetera())->shouldNotBeCalled(); + $manager->flush()->shouldBeCalledOnce(); + + $metadata = new ChangeAwareMetadata(); + $metadata->set('foo', 'bar'); + + $this->createPersister($this->createManagerRegistry($manager))->persist(new Client('client-id', $metadata)); + + self::assertSame(['foo' => 'bar'], $existing->getMetadata()); + } + + /** + * @test + */ + public function it_swallows_unique_constraint_violations(): void + { + $exception = (new \ReflectionClass(UniqueConstraintViolationException::class))->newInstanceWithoutConstructor(); + + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->find(MetadataEntity::class, 'client-id')->willReturn(null); + $manager->persist(Argument::type(MetadataEntity::class))->shouldBeCalledOnce(); + $manager->flush()->willThrow($exception); + + $metadata = new ChangeAwareMetadata(); + $metadata->set('foo', 'bar'); + + $this->createPersister($this->createManagerRegistry($manager))->persist(new Client('client-id', $metadata)); + + self::assertTrue($metadata->isDirty()); + } + + /** + * @param ObjectProphecy $manager + * + * @return ObjectProphecy + */ + private function createManagerRegistry(ObjectProphecy $manager): ObjectProphecy + { + $managerRegistry = $this->prophesize(ManagerRegistry::class); + $managerRegistry->getManagerForClass(MetadataEntity::class)->willReturn($manager); + + return $managerRegistry; + } + + /** + * @param ObjectProphecy $managerRegistry + */ + private function createPersister(ObjectProphecy $managerRegistry): DoctrineOrmBasedMetadataPersister + { + return new DoctrineOrmBasedMetadataPersister($managerRegistry->reveal(), MetadataEntity::class); + } +} diff --git a/tests/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php b/tests/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php index c45abfb..d9feaa3 100644 --- a/tests/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php +++ b/tests/MetadataProvider/DoctrineOrmBasedMetadataProviderTest.php @@ -7,6 +7,7 @@ use Doctrine\ORM\EntityManagerInterface; use Doctrine\Persistence\ManagerRegistry; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; use Setono\Client\Metadata; use Setono\ClientBundle\Client\LazyChangeAwareMetadata; @@ -69,4 +70,33 @@ public function it_initializes_metadata_when_accessed(): void self::assertTrue($metadata->isDirty()); self::assertSame('some-value', $metadata->get('some-key')); } + + /** + * @test + */ + public function it_loads_the_metadata_from_an_existing_entity(): void + { + $entity = new MetadataEntity(); + $entity->setClientId('some-client-id'); + $entity->setMetadata(['foo' => 'bar']); + + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->find(MetadataEntity::class, 'some-client-id')->willReturn($entity); + + $managerRegistry = $this->prophesize(ManagerRegistry::class); + $managerRegistry->getManagerForClass(MetadataEntity::class)->willReturn($manager); + + $decorated = $this->prophesize(MetadataProviderInterface::class); + $decorated->getMetadata(Argument::any())->shouldNotBeCalled(); + + $provider = new DoctrineOrmBasedMetadataProvider( + $decorated->reveal(), + $managerRegistry->reveal(), + MetadataEntity::class, + ); + + $metadata = $provider->getMetadata('some-client-id'); + + self::assertSame('bar', $metadata->get('foo')); + } } diff --git a/tests/MetadataProvider/EmptyMetadataProviderTest.php b/tests/MetadataProvider/EmptyMetadataProviderTest.php new file mode 100644 index 0000000..304dbdd --- /dev/null +++ b/tests/MetadataProvider/EmptyMetadataProviderTest.php @@ -0,0 +1,22 @@ +getMetadata('client-id'); + + self::assertCount(0, $metadata); + self::assertFalse($metadata->isDirty()); + } +} diff --git a/tests/SetonoClientBundleTest.php b/tests/SetonoClientBundleTest.php new file mode 100644 index 0000000..9e2f03c --- /dev/null +++ b/tests/SetonoClientBundleTest.php @@ -0,0 +1,45 @@ +getPath()); + } + + /** + * @test + */ + public function it_registers_the_doctrine_mapping_for_the_metadata_entity(): void + { + $container = new ContainerBuilder(); + (new SetonoClientBundle())->build($container); + + $pass = null; + foreach ($container->getCompilerPassConfig()->getBeforeOptimizationPasses() as $candidate) { + if ($candidate instanceof DoctrineOrmMappingsPass) { + $pass = $candidate; + + break; + } + } + + self::assertInstanceOf(DoctrineOrmMappingsPass::class, $pass); + self::assertSame( + [\dirname(__DIR__) . '/src/../config/doctrine-mapping' => 'Setono\ClientBundle\Entity'], + (new \ReflectionProperty($pass, 'namespaces'))->getValue($pass), + ); + } +}