From 690d9f5c435a0a2d0bc9dfb6672d7f4b7f8b25f0 Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Mon, 20 Jul 2026 08:56:30 +0200 Subject: [PATCH 01/11] PRE-3469: Validate contracts with Sylius --- composer.json | 7 + migrations/Version20260720100000.php | 62 ++++++ phpunit.xml.dist | 6 +- .../PayPlugSyliusPayPlugExtension.php | 29 +++ src/Spike/Entity/PayplugOperation.php | 116 ++++++++++++ src/Spike/SyliusConfigurationRepository.php | 119 ++++++++++++ src/Spike/SyliusOrderStateMutator.php | 85 +++++++++ src/Spike/SyliusPaymentRepository.php | 80 ++++++++ src/Spike/SyliusTokenCache.php | 52 +++++ src/Spike/VALIDATION.md | 165 ++++++++++++++++ tests/PHPUnit/Spike/SpikeIntegrationTest.php | 177 ++++++++++++++++++ .../SyliusConfigurationRepositoryTest.php | 121 ++++++++++++ .../Spike/SyliusOrderStateMutatorTest.php | 127 +++++++++++++ .../Spike/SyliusPaymentRepositoryTest.php | 152 +++++++++++++++ tests/PHPUnit/Spike/SyliusTokenCacheTest.php | 75 ++++++++ 15 files changed, 1372 insertions(+), 1 deletion(-) create mode 100644 migrations/Version20260720100000.php create mode 100644 src/Spike/Entity/PayplugOperation.php create mode 100644 src/Spike/SyliusConfigurationRepository.php create mode 100644 src/Spike/SyliusOrderStateMutator.php create mode 100644 src/Spike/SyliusPaymentRepository.php create mode 100644 src/Spike/SyliusTokenCache.php create mode 100644 src/Spike/VALIDATION.md create mode 100644 tests/PHPUnit/Spike/SpikeIntegrationTest.php create mode 100644 tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php create mode 100644 tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php create mode 100644 tests/PHPUnit/Spike/SyliusPaymentRepositoryTest.php create mode 100644 tests/PHPUnit/Spike/SyliusTokenCacheTest.php diff --git a/composer.json b/composer.json index 2d93bb4d..2c2e0a29 100755 --- a/composer.json +++ b/composer.json @@ -37,6 +37,7 @@ "friendsoftwig/twigcs": "6.5.0", "lakion/mink-debug-extension": "2.0.0", "mockery/mockery": "1.6.12", + "payplug/unified-plugin-core": "dev-develop", "php-parallel-lint/php-parallel-lint": "1.4.0", "phpmd/phpmd": "^2.15.0", "phpro/grumphp": "^2.12", @@ -57,6 +58,12 @@ "webmozart/assert": "^1.8" }, "prefer-stable": true, + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/payplug/unified-plugin-core.git" + } + ], "autoload": { "psr-4": { "PayPlug\\SyliusPayPlugPlugin\\": "src/" diff --git a/migrations/Version20260720100000.php b/migrations/Version20260720100000.php new file mode 100644 index 00000000..bbe438c3 --- /dev/null +++ b/migrations/Version20260720100000.php @@ -0,0 +1,62 @@ +skipIf( + 'test' !== ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null), + 'PRE-3469 spike-only schema, not applied outside the test environment.', + ); + + $this->addSql('CREATE TABLE payplug_operation ( + id INT AUTO_INCREMENT NOT NULL, + operation_id VARCHAR(255) NOT NULL, + exec_code VARCHAR(255) NOT NULL, + outcome VARCHAR(255) NOT NULL, + amount INT NOT NULL, + order_id VARCHAR(255) NOT NULL, + treated TINYINT(1) NOT NULL, + treated_at DATETIME DEFAULT NULL COMMENT \'(DC2Type:datetime_immutable)\', + created_at DATETIME NOT NULL COMMENT \'(DC2Type:datetime_immutable)\', + UNIQUE INDEX payplug_operation_id_unique (operation_id), + INDEX payplug_operation_order_id_idx (order_id), + PRIMARY KEY(id) + ) DEFAULT CHARACTER SET UTF8 COLLATE `UTF8_unicode_ci` ENGINE = InnoDB'); + } + + public function down(Schema $schema): void + { + $this->skipIf( + 'test' !== ($_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? null), + 'PRE-3469 spike-only schema, not applied outside the test environment.', + ); + + $this->addSql('DROP TABLE payplug_operation'); + } +} diff --git a/phpunit.xml.dist b/phpunit.xml.dist index b01264ac..7dc60db7 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -7,12 +7,16 @@ tests/PHPUnit + + tests/PHPUnit/Spike/SpikeIntegrationTest.php - + diff --git a/src/DependencyInjection/PayPlugSyliusPayPlugExtension.php b/src/DependencyInjection/PayPlugSyliusPayPlugExtension.php index d22ef359..e3aea36d 100644 --- a/src/DependencyInjection/PayPlugSyliusPayPlugExtension.php +++ b/src/DependencyInjection/PayPlugSyliusPayPlugExtension.php @@ -33,6 +33,35 @@ public function prepend(ContainerBuilder $container): void $this->prependTwigExtension($container); $this->prependDoctrineMigrations($container); $this->prependMonologExtension($container); + $this->prependSpikeDoctrineMapping($container); + } + + /** + * PRE-3469 spike only — registers src/Spike/Entity as a plain (non-Sylius-resource) + * Doctrine mapping so the spike's integration test can persist PayplugOperation for real. + * Not a `sylius_resource` on purpose: that would pull in grids/forms/routes this throwaway + * entity has no use for. Restricted to the `test` environment on purpose too — this is + * test-only scaffolding, it must never register Doctrine metadata for a throwaway entity in + * prod. Remove this method along with src/Spike/ once the spike is closed. + */ + private function prependSpikeDoctrineMapping(ContainerBuilder $container): void + { + if (!$container->hasExtension('doctrine') || 'test' !== $container->getParameter('kernel.environment')) { + return; + } + + $container->prependExtensionConfig('doctrine', [ + 'orm' => [ + 'mappings' => [ + 'PayPlugSyliusPayPlugPluginSpike' => [ + 'type' => 'attribute', + 'dir' => dirname(__DIR__) . '/Spike/Entity', + 'prefix' => 'PayPlug\SyliusPayPlugPlugin\Spike\Entity', + 'is_bundle' => false, + ], + ], + ], + ]); } private function prependTwigExtension(ContainerBuilder $container): void diff --git a/src/Spike/Entity/PayplugOperation.php b/src/Spike/Entity/PayplugOperation.php new file mode 100644 index 00000000..3f1f5c84 --- /dev/null +++ b/src/Spike/Entity/PayplugOperation.php @@ -0,0 +1,116 @@ +operationId = $operationId; + $this->execCode = $execCode; + $this->outcome = $outcome; + $this->amount = $amount; + $this->orderId = $orderId; + $this->createdAt = new \DateTimeImmutable(); + } + + public static function fromOperationData(OperationData $operationData): self + { + return new self( + $operationData->operationId, + $operationData->execCode, + $operationData->outcome, + $operationData->amount, + $operationData->orderId, + ); + } + + public function updateFromOperationData(OperationData $operationData): void + { + $this->execCode = $operationData->execCode; + $this->outcome = $operationData->outcome; + $this->amount = $operationData->amount; + } + + public function toOperationData(): OperationData + { + return new OperationData($this->operationId, $this->execCode, $this->outcome, $this->amount, $this->orderId); + } + + public function getOperationId(): string + { + return $this->operationId; + } + + public function getOrderId(): string + { + return $this->orderId; + } + + public function isTreated(): bool + { + return $this->treated; + } + + public function markTreated(): void + { + $this->treated = true; + $this->treatedAt = new \DateTimeImmutable(); + } +} diff --git a/src/Spike/SyliusConfigurationRepository.php b/src/Spike/SyliusConfigurationRepository.php new file mode 100644 index 00000000..c807dd9b --- /dev/null +++ b/src/Spike/SyliusConfigurationRepository.php @@ -0,0 +1,119 @@ +activeClientConfig(); + $value = $client[$key] ?? null; + + return \is_string($value) ? $value : null; + } + + public function set(string $key, string $value): void + { + $config = $this->gatewayConfig->getConfig(); + $scope = $this->activeScope($config); + $client = $config[$scope] ?? []; + if (!\is_array($client)) { + $client = []; + } + $client[$key] = $value; + $config[$scope] = $client; + + // Persisting $config is the caller's responsibility (Doctrine flush), same as every + // other GatewayConfigInterface mutation in this plugin (see UnifiedAuthenticationController). + $this->gatewayConfig->setConfig($config); + } + + public function getClientId(): string + { + return $this->requireString('client_id'); + } + + public function getClientSecret(): string + { + return $this->requireString('client_secret'); + } + + public function getPublicKeyId(): string + { + return $this->requireString('public_key_id'); + } + + public function getPublicKeyValue(): string + { + return $this->requireString('public_key_value'); + } + + private function requireString(string $key): string + { + $value = $this->get($key); + if (null === $value || '' === $value) { + // Never interpolate the resolved *value* here, only the key name and factory name. + throw new ApiException(\sprintf( + 'Missing "%s" in gateway configuration "%s".', + $key, + $this->gatewayConfig->getFactoryName() ?? 'unknown', + )); + } + + return $value; + } + + /** + * @return array + */ + private function activeClientConfig(): array + { + $config = $this->gatewayConfig->getConfig(); + $client = $config[$this->activeScope($config)] ?? []; + if (!\is_array($client)) { + return []; + } + + /** @var array $typedClient */ + $typedClient = $client; + + return $typedClient; + } + + /** + * @param array $config + */ + private function activeScope(array $config): string + { + return true === ($config['live'] ?? false) ? 'live_client' : 'test_client'; + } +} diff --git a/src/Spike/SyliusOrderStateMutator.php b/src/Spike/SyliusOrderStateMutator.php new file mode 100644 index 00000000..6c197183 --- /dev/null +++ b/src/Spike/SyliusOrderStateMutator.php @@ -0,0 +1,85 @@ + getLastPayment()) that a + * WooCommerce implementation, whose native order carries the payment status directly, would not + * need. Not blocking: the interface deliberately stays CMS-agnostic, so this hop belongs in the + * Sylius adapter, not in the contract. + */ +final class SyliusOrderStateMutator implements IOrderStateMutator +{ + /** + * THREE_DS_PENDING is deliberately absent: per PRE-3469, the order must stay in its + * current state (`new`) until the async webhook resolves it to PAID/AUTHORIZED/FAILED — + * there is no Symfony Workflow transition for "awaiting 3DS". + * + * @var array + */ + private const OUTCOME_TO_TRANSITION = [ + PaymentOutcome::PAID => PaymentTransitions::TRANSITION_COMPLETE, + PaymentOutcome::AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE, + PaymentOutcome::CAPTURE_REQUIRED => PaymentTransitions::TRANSITION_PROCESS, + PaymentOutcome::REFUNDED => PaymentTransitions::TRANSITION_REFUND, + PaymentOutcome::FAILED => PaymentTransitions::TRANSITION_FAIL, + ]; + + public function __construct( + private readonly OrderRepositoryInterface $orderRepository, + private readonly StateMachineInterface $stateMachine, + private readonly EntityManagerInterface $entityManager, + ) { + } + + public function apply(string $orderId, string $outcome): void + { + if (PaymentOutcome::THREE_DS_PENDING === $outcome) { + return; + } + + $transition = self::OUTCOME_TO_TRANSITION[$outcome] ?? null; + if (null === $transition) { + throw new \InvalidArgumentException(\sprintf('No Symfony Workflow transition mapped for PaymentOutcome "%s".', $outcome)); + } + + $order = $this->orderRepository->findOrderById($orderId); + if (null === $order) { + throw new PaymentNotFoundException(\sprintf('No order found for id "%s".', $orderId)); + } + + $payment = $order->getLastPayment(); + if (null === $payment) { + throw new PaymentNotFoundException(\sprintf('Order "%s" has no payment to mutate.', $orderId)); + } + + // Guarded exactly like the plugin's existing PaymentStateResolver::applyTransition(): + // an out-of-order webhook retry landing on a payment already past this transition is a + // silent no-op rather than a thrown workflow exception. + if ($this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) { + $this->stateMachine->apply($payment, PaymentTransitions::GRAPH, $transition); + $this->entityManager->flush(); + } + } +} diff --git a/src/Spike/SyliusPaymentRepository.php b/src/Spike/SyliusPaymentRepository.php new file mode 100644 index 00000000..42f1b0ca --- /dev/null +++ b/src/Spike/SyliusPaymentRepository.php @@ -0,0 +1,80 @@ +requireOperation(['orderId' => $orderId], \sprintf('No operation for order "%s".', $orderId))->toOperationData(); + } + + public function getByOperationId(string $operationId): OperationData + { + return $this->requireOperation(['operationId' => $operationId], \sprintf('No operation "%s".', $operationId))->toOperationData(); + } + + public function save(OperationData $operationData): void + { + $existing = $this->findOperation(['operationId' => $operationData->operationId]); + + if (null === $existing) { + $this->entityManager->persist(PayplugOperation::fromOperationData($operationData)); + } else { + $existing->updateFromOperationData($operationData); + } + + $this->entityManager->flush(); + } + + public function markTreated(string $operationId): void + { + $operation = $this->requireOperation(['operationId' => $operationId], \sprintf('No operation "%s".', $operationId)); + $operation->markTreated(); + $this->entityManager->flush(); + } + + public function isTreated(string $operationId): bool + { + $operation = $this->findOperation(['operationId' => $operationId]); + + return null !== $operation && $operation->isTreated(); + } + + /** + * @param array $criteria + */ + private function requireOperation(array $criteria, string $notFoundMessage): PayplugOperation + { + $operation = $this->findOperation($criteria); + if (null === $operation) { + throw new PaymentNotFoundException($notFoundMessage); + } + + return $operation; + } + + /** + * @param array $criteria + */ + private function findOperation(array $criteria): ?PayplugOperation + { + return $this->entityManager->getRepository(PayplugOperation::class)->findOneBy($criteria); + } +} diff --git a/src/Spike/SyliusTokenCache.php b/src/Spike/SyliusTokenCache.php new file mode 100644 index 00000000..2069618e --- /dev/null +++ b/src/Spike/SyliusTokenCache.php @@ -0,0 +1,52 @@ +cache->getItem($key); + if (!$item->isHit()) { + return null; + } + + $value = $item->get(); + + return \is_string($value) ? $value : null; + } + + public function set(string $key, string $value, int $ttlSeconds): void + { + $item = $this->cache->getItem($key); + $item->set($value); + $item->expiresAfter($ttlSeconds); + $this->cache->save($item); + } + + public function delete(string $key): void + { + $this->cache->deleteItem($key); + } +} diff --git a/src/Spike/VALIDATION.md b/src/Spike/VALIDATION.md new file mode 100644 index 00000000..b806d2e0 --- /dev/null +++ b/src/Spike/VALIDATION.md @@ -0,0 +1,165 @@ +# PRE-3469 — Validation des contrats UPC contre Sylius + +Spike de preuve : implémentation squelette des 4 contrats à risque (`IOrderStateMutator`, +`ITokenCache`, `IConfigurationRepository`, `IPaymentRepository`) contre les vraies APIs Sylius +(`sylius/sylius` ^2.0, PHP 8.2), pour valider qu'ils sont satisfaisables avant de les figer. +Code de preuve — voir `src/Spike/` — mais couvert par 3 niveaux de tests réels (43 tests) : + +- **Unitaire** (`tests/PHPUnit/Spike/*Test.php`, hors `SpikeIntegrationTest`) : mocks natifs + PHPUnit sur chaque collaborateur (StateMachineInterface, CacheItemPoolInterface, + GatewayConfigInterface, EntityManagerInterface). 38 tests. +- **Intégration réelle** (`SpikeIntegrationTest.php`) : kernel Sylius réellement booté + (`sylius/test-application`), vraie base MariaDB jetable, vrai state machine Symfony Workflow, + vrai pool de cache PSR-6, vraie persistance Doctrine — aucun mock. 5 tests. +- Les deux suites tournent avec `make`-style commandes classiques (`composer install`, + `vendor/bin/phpunit`) une fois la base de test créée — voir section Câblage. + +**Verdict global : aucune friction bloquante.** Les 4 contrats sont implémentables contre les +APIs Sylius actuelles, et le prouvent en s'exécutant réellement. Un bug réel a été trouvé et +corrigé (voir IOrderStateMutator), plus trois notes pour la suite (aucune ne remet en cause la +forme des interfaces). + +## IOrderStateMutator — squelette : `SyliusOrderStateMutator.php` + +Mapping validé : `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → +`PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL` sur le graphe +`sylius_payment`, via `StateMachineInterface::can()/apply()` — exactement le pattern déjà utilisé +par `PaymentStateResolver::applyTransition()` dans le plugin actuel. + +`THREE_DS_PENDING` confirmé sans transition : l'implémentation retourne immédiatement sans +toucher au state machine, la commande reste `new` jusqu'au webhook — conforme au résultat attendu +du ticket. + +**Bug réel trouvé par le test d'intégration (corrigé)** : la première version du squelette +appelait `$stateMachine->apply($payment, ...)` sans jamais flush l'EntityManager. Le state +machine (`marking_store: type: method`) ne fait que muter la propriété `state` en mémoire — rien +n'est persisté sans un `flush()` explicite, exactement comme le fait déjà +`PaymentStateResolver::resolve()` dans le plugin actuel (`$this->paymentEntityManager->flush();` +en fin de méthode). Le test unitaire avec mocks ne pouvait pas voir ce bug (il vérifie l'appel à +`apply()`, pas la persistance réelle) — seul le test d'intégration contre une vraie base l'a +révélé : la transition semblait réussir (`can()` → true, `apply()` appelé) mais la commande +restait `new` en base. `IOrderStateMutator` prend maintenant un `EntityManagerInterface` en plus +et flush après chaque transition appliquée. + +**Note (non bloquante)** : le contrat prend un `orderId`, mais la transition Symfony Workflow vit +sur le sous-objet `Payment` de la commande (`Order::getLastPayment()`), pas sur l'`Order` +lui-même. Un hop supplémentaire Order → Payment est donc nécessaire côté adaptateur Sylius — ce +que WooCommerce n'aurait pas besoin de faire. C'est le bon endroit pour cette différence (dans +l'adaptateur, pas dans le contrat CMS-agnostique). + +## ITokenCache — squelette : `SyliusTokenCache.php` + +Aucune friction. `get`/`set`/`delete` se posent 1:1 sur `Psr\Cache\CacheItemPoolInterface::getItem/ +save/deleteItem`, exactement comme le docblock de l'interface le prévoyait déjà. Le pool par +défaut de Sylius (`cache.app`, `Symfony\Component\Cache\Adapter\AdapterInterface`) satisfait déjà +`CacheItemPoolInterface` — filesystem, APCu ou Redis selon le déploiement, sans changement de code +côté `SyliusTokenCache`. + +## IConfigurationRepository — squelette : `SyliusConfigurationRepository.php` + +**Note (non bloquante)** : `GatewayConfigInterface::getConfig()` scope les credentials par +`PaymentMethod` *et* par mode live/test (`config['live_client']` vs `config['test_client']`, +sélectionné par `config['live']` — même pattern que `PayPlugApiClientFactory::getTokenForGatewayConfig()` +existant). Une instance de `SyliusConfigurationRepository` doit donc être construite par +`GatewayConfigInterface` (donc par `PaymentMethod`), pas partagée comme service unique — c'est une +factory, pas un singleton. Ne remet pas en cause le contrat, mais à garder en tête si le futur +client Unified API suppose "un repository = un marchand". + +**Point positif** : Sylius expose déjà un `GatewayConfigEncrypter` (expérimental) qui chiffre au +repos l'intégralité du tableau `getConfig()` — si branché, `CLIENT_SECRET` en bénéficie +gratuitement. Ce que `SyliusConfigurationRepository` doit garantir lui-même, c'est qu'un secret +déchiffré ne fuite jamais dans un message de log ou d'exception : `requireString()` n'interpole +jamais que le nom de la clé, jamais sa valeur. + +## IPaymentRepository — squelettes : `SyliusPaymentRepository.php` + `Entity/PayplugOperation.php` + +**Constat principal du ticket, confirmé** : le plugin actuel ne stocke pas `OperationData` de +façon normalisée. L'id Payplug est aujourd'hui écrit dans le JSON de `Payment::details` et +retrouvé via `LIKE '%id%'` (`PaymentRepository::findOneByPayPlugPaymentId`). Ça fonctionne pour ce +seul lookup, mais ne peut pas porter `markTreated()`/`isTreated()` (il faut un flag d'idempotence +indexé) ni `getByOperationId()` proprement (il faut une colonne indexée, pas une recherche de +sous-chaîne dans un blob sérialisé). + +Le squelette introduit donc une nouvelle table `payplug_operation`, sans aucune dépendance au SDK +`payplug/payplug-php` — juste `OperationData` en entrée/sortie. C'est un vrai changement de schéma +(nouvelle table), pas une extension de l'existant — à traiter comme tel dans le chiffrage du futur +ticket de production. + +**Note annexe (mécanique, pas conceptuelle)** : `PayplugOperation` vit hors de `src/Entity/` et +n'est pas un `sylius_resource` dans `config/resources.yaml` (contrairement à `Card` et +`RefundHistory`) — enregistrer un `sylius_resource` a été essayé pour le test d'intégration mais +échoue tant que la classe n'implémente pas `Sylius\Resource\Model\ResourceInterface` (grilles/ +formulaires/routes Sylius, inutiles pour cette entité de pure persistance). Le mapping Doctrine +réel utilisé pour le test d'intégration passe donc par un `doctrine.orm.mappings` prepend dans +`PayPlugSyliusPayPlugExtension::prependSpikeDoctrineMapping()` — méthode explicitement marquée +PRE-3469-only, restreinte à `kernel.environment === 'test'` (jamais en prod), à retirer avec +`src/Spike/` si le spike est abandonné. + +**Bug de portabilité trouvé et corrigé (deux fois)** : la table `payplug_operation` a d'abord été +créée à la main sur la base locale — ça marchait chez moi, mais ni pour un collègue qui pull, ni +en CI. Première tentative de fix : la faire créer par le test lui-même via +`SchemaTool::updateSchema([$metadata], true)` — **destructeur** : avec une liste de classes +partielle, `updateSchema()` calcule un schéma cible ne contenant que cette entité et supprime +toutes les autres tables qu'il ne reconnaît pas dans ce diff, y compris tout le schéma Sylius. +Vérifié en le déclenchant sur la base jetable : il n'est resté que 2 tables sur la centaine +attendue. Aucun dégât réel (base locale jetable), mais invendable en l'état. + +Deuxième problème, plus profond : enregistrer le mapping Doctrine dès que +`kernel.environment === 'test'` casse `sylius:fixtures:load` sur une base neuve, *même sans +jamais toucher au spike* — n'importe quelle commande qui boote le kernel en env `test` (donc +l'installation standard du `test-application`) échoue avec « table `payplug_operation` +n'existe pas », puisque Doctrine connaît l'entité sans que la table existe encore. + +**Fix final** : une vraie migration Doctrine (`migrations/Version20260720100000.php`), comme +toutes les autres tables du plugin. Contre-intuitif pour une entité de spike, mais c'est la seule +option qui ne casse rien pour qui que ce soit sur un premier `make install` — vérifié en +reconstruisant la base de zéro (`doctrine:database:create` → `doctrine:migration:migrate` → +`sylius:fixtures:load` → suite de tests) et en rejouant le test d'intégration 3 fois de suite. +`up()`/`down()` sont chacun protégés par `$this->skipIf('test' !== APP_ENV, ...)` : une migration +de plugin normale s'applique dans tous les environnements, y compris en prod chez un marchand — +sans ce garde-fou, chaque installation du plugin en production créerait une table `payplug_operation` +qui ne sert jamais à rien en dehors des tests de ce spike. + +## Câblage (pour rejouer les tests) + +- `composer.json` : `repositories` avec un repository `vcs` vers + `https://github.com/payplug/unified-plugin-core.git` + require-dev + `payplug/unified-plugin-core: "dev-develop"`, pour que les squelettes implémentent réellement + les interfaces (et non des copies locales) — fonctionne pour n'importe qui ayant accès au repo + GitHub (même accès que pour ce repo), et en CI via l'étape `Composer - Github Auth` déjà + configurée dans `payplug/template-ci`. Une première version utilisait un repository `path` + local (`../../unified-plugin-core`) : ça ne marche que sur une machine avec les deux repos + clonés côte à côte, cassait `composer install` pour tout le monde d'autre — corrigé. + Nécessite que le pin exact `symfony/polyfill-mbstring: 1.28.0` d'UPC ait été relâché en + `^1.28` (corrigé dans unified-plugin-core suite à ce spike — il bloquait tout + `composer install` aux côtés de `sylius/sylius ^2.0`, qui exige `^1.31`). + Contrepartie connue : ça ajoute une résolution réseau vers GitHub à chaque install (léger coût + CI/dev) — pas d'alternative disponible ici (pas de Packagist privé configuré dans cet org pour + `payplug/unified-plugin-core`, et un repository `path` est exclu pour la raison ci-dessus). + Ce compromis disparaît quand PRE-3563 posera la vraie dépendance de production. +- `phpunit.xml.dist` : `KERNEL_CLASS_PATH` renommé en `KERNEL_CLASS` — la variable que Symfony lit + réellement pour `KernelTestCase::bootKernel()`. Sans ce fix, aucun test à base de kernel + (fonctionnel/intégration) n'a jamais pu tourner dans ce repo — un bug de config préexistant, + invisible tant qu'aucun test de ce type n'existait. +- `tests/TestApplication/.env.test.local` (gitignored) : `DATABASE_URL` pointée vers un conteneur + MariaDB jetable (`docker run --name payplug-pre3469-mysql -e MYSQL_ALLOW_EMPTY_PASSWORD=yes -p + 3309:3306 mariadb:latest`) — le MySQL système de la machine refuse `root@127.0.0.1` sans mot de + passe. `.env.local` seul ne suffit pas : Symfony l'ignore délibérément quand `APP_ENV=test` + (reproductibilité des tests) — il faut `.env.test.local` spécifiquement. +- Base créée/migrée avec les commandes standard du Makefile (`doctrine:database:create`, + `doctrine:migration:migrate`, `sylius:payment:generate-key`, `sylius:fixtures:load`) — la table + `payplug_operation` est créée automatiquement par `migrations/Version20260720100000.php`, plus + besoin d'aucune étape manuelle (voir plus bas pourquoi une vraie migration a été nécessaire). + +Ce câblage (composer + config) est à revoir quand PRE-3563 (OAuth réel contre UPC) posera la +vraie dépendance de production ; le `.env.test.local` et le conteneur Docker restent locaux à la +machine de dev et n'ont pas vocation à être commités. + +**Isolation des tests** : `SpikeIntegrationTest` ne tourne dans aucune transaction annulée en fin +de test (pas de `DAMADoctrineTestBundle`). Une première version cherchait un paiement de fixture +déjà à l'état `new` — ça fonctionne une fois, mais une commande réelle ne repasse jamais à `new` +une fois transitionnée, donc ce pool fini s'épuise au fil des ré-exécutions contre la même base +jetable. Corrigé en créant un `Payment` frais sur une commande de fixture existante à chaque test +(`createOrderWithFreshPayment()`) plutôt que d'en chercher un dans un état donné — vérifié stable +sur 3 exécutions consécutives. Une vraie suite (non-spike) voudrait un rollback transactionnel +entre tests plutôt que ce contournement. diff --git a/tests/PHPUnit/Spike/SpikeIntegrationTest.php b/tests/PHPUnit/Spike/SpikeIntegrationTest.php new file mode 100644 index 00000000..236adb8c --- /dev/null +++ b/tests/PHPUnit/Spike/SpikeIntegrationTest.php @@ -0,0 +1,177 @@ +entityManager = self::getContainer()->get(EntityManagerInterface::class); + } + + public function testOrderStateMutator_realStateMachine_transitionsPaymentToCompleted(): void + { + $order = $this->createOrderWithFreshPayment(); + $payment = $order->getLastPayment(); + self::assertNotNull($payment); + self::assertSame(PaymentInterface::STATE_NEW, $payment->getState()); + + $mutator = new SyliusOrderStateMutator( + self::getContainer()->get('sylius.repository.order'), + self::getContainer()->get(StateMachineInterface::class), + $this->entityManager, + ); + + $mutator->apply((string) $order->getId(), PaymentOutcome::PAID); + + $this->entityManager->refresh($payment); + self::assertSame(PaymentInterface::STATE_COMPLETED, $payment->getState()); + } + + public function testOrderStateMutator_threeDsPending_leavesRealPaymentUntouched(): void + { + $order = $this->createOrderWithFreshPayment(); + $payment = $order->getLastPayment(); + self::assertNotNull($payment); + + $mutator = new SyliusOrderStateMutator( + self::getContainer()->get('sylius.repository.order'), + self::getContainer()->get(StateMachineInterface::class), + $this->entityManager, + ); + + $mutator->apply((string) $order->getId(), PaymentOutcome::THREE_DS_PENDING); + + $this->entityManager->refresh($payment); + self::assertSame(PaymentInterface::STATE_NEW, $payment->getState()); + } + + public function testTokenCache_realCachePool_roundTripsThroughRealAdapter(): void + { + $cachePool = self::getContainer()->get(CacheItemPoolInterface::class); + $tokenCache = new SyliusTokenCache($cachePool); + + $tokenCache->set('pre3469-spike-token', 'jwt-value', 300); + self::assertSame('jwt-value', $tokenCache->get('pre3469-spike-token')); + + $tokenCache->delete('pre3469-spike-token'); + self::assertNull($tokenCache->get('pre3469-spike-token')); + } + + public function testConfigurationRepository_realGatewayConfig_survivesADoctrineRoundTrip(): void + { + $gatewayConfig = new GatewayConfig(); + $gatewayConfig->setFactoryName('payplug_pre3469_spike'); + $gatewayConfig->setGatewayName('PRE-3469 spike'); + $gatewayConfig->setConfig(['live' => false, 'test_client' => ['client_id' => 'spike-client-id']]); + $this->entityManager->persist($gatewayConfig); + $this->entityManager->flush(); + $id = $gatewayConfig->getId(); + $this->entityManager->clear(); + + /** @var GatewayConfig $reloaded */ + $reloaded = $this->entityManager->find(GatewayConfig::class, $id); + (new SyliusConfigurationRepository($reloaded))->set('client_secret', 'spike-secret-value'); + $this->entityManager->flush(); + $this->entityManager->clear(); + + /** @var GatewayConfig $reloadedAgain */ + $reloadedAgain = $this->entityManager->find(GatewayConfig::class, $id); + $repository = new SyliusConfigurationRepository($reloadedAgain); + + self::assertSame('spike-client-id', $repository->getClientId()); + self::assertSame('spike-secret-value', $repository->getClientSecret()); + } + + public function testPaymentRepository_realDoctrinePersistence_roundTripsAndTracksIdempotency(): void + { + $repository = new SyliusPaymentRepository($this->entityManager); + $operationId = 'spike-op-' . \bin2hex(\random_bytes(6)); + $orderId = 'spike-order-' . \bin2hex(\random_bytes(6)); + $operationData = new OperationData($operationId, '4001', PaymentOutcome::PAID, 1500, $orderId); + + $repository->save($operationData); + $this->entityManager->clear(); + + self::assertFalse($repository->isTreated($operationId)); + + $fetched = $repository->getByOperationId($operationId); + self::assertSame($orderId, $fetched->orderId); + self::assertSame(1500, $fetched->amount); + + $repository->markTreated($operationId); + $this->entityManager->clear(); + + self::assertTrue($repository->isTreated($operationId)); + } + + /** + * Self-contained on purpose: earlier drafts hunted for a fixture payment already sitting in + * state `new`, but repeated test runs against the same disposable database exhaust that + * finite pool (fixtures don't reload between runs) — a real payment always transitions + * *away* from `new`, so the pool only shrinks. Attaching a fresh Payment to an existing + * fixture Order sidesteps that without needing a full from-scratch Order (channel, customer, + * currency, locale, number, token — all fixture-provided here for free). + */ + private function createOrderWithFreshPayment(): OrderInterface + { + $order = $this->entityManager->getRepository(Order::class)->createQueryBuilder('o') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult() + ; + self::assertNotNull($order, 'Fixtures must include at least one order.'); + + $method = $order->getLastPayment()?->getMethod() + ?? $this->entityManager->getRepository(PaymentMethod::class)->createQueryBuilder('m')->setMaxResults(1)->getQuery()->getOneOrNullResult() + ; + self::assertNotNull($method, 'Fixtures must include at least one payment method.'); + + $payment = new Payment(); + $payment->setState(PaymentInterface::STATE_NEW); + $payment->setCurrencyCode($order->getCurrencyCode() ?? 'EUR'); + $payment->setAmount($order->getTotal()); + $payment->setMethod($method); + $order->addPayment($payment); + + $this->entityManager->persist($payment); + $this->entityManager->flush(); + + return $order; + } +} diff --git a/tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php b/tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php new file mode 100644 index 00000000..66ea74d0 --- /dev/null +++ b/tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php @@ -0,0 +1,121 @@ +gatewayConfig = $this->createMock(GatewayConfigInterface::class); + } + + public function testGet_liveMode_readsFromLiveClientScope(): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => ['client_id' => 'live-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + self::assertSame('live-id', $this->repository()->get('client_id')); + } + + public function testGet_testMode_readsFromTestClientScope(): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => false, + 'live_client' => ['client_id' => 'live-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + self::assertSame('test-id', $this->repository()->get('client_id')); + } + + public function testGet_missingKey_returnsNull(): void + { + $this->gatewayConfig->method('getConfig')->willReturn(['live' => true, 'live_client' => []]); + + self::assertNull($this->repository()->get('client_id')); + } + + public function testSet_writesIntoActiveScopeAndPersistsWholeConfig(): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => ['client_id' => 'old-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + $this->gatewayConfig->expects(self::once())->method('setConfig')->with([ + 'live' => true, + 'live_client' => ['client_id' => 'new-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + $this->repository()->set('client_id', 'new-id'); + } + + /** + * @dataProvider provideCredentialGetters + */ + public function testCredentialGetters_present_returnValue(string $method, string $configKey): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => [$configKey => 'the-value'], + ]); + + self::assertSame('the-value', $this->repository()->{$method}()); + } + + /** + * @dataProvider provideCredentialGetters + * + * The exception message must name the missing key and the factory, never a credential + * value — there is nothing sensitive to redact here since the value is simply absent, but + * this locks the message shape so a future edit can't start interpolating $value instead. + */ + public function testCredentialGetters_missing_throwsApiExceptionWithoutLeakingSecrets( + string $method, + string $configKey, + ): void + { + $this->gatewayConfig->method('getConfig')->willReturn(['live' => true, 'live_client' => []]); + $this->gatewayConfig->method('getFactoryName')->willReturn('payplug'); + + try { + $this->repository()->{$method}(); + self::fail('Expected ApiException to be thrown.'); + } catch (ApiException $exception) { + self::assertSame(\sprintf('Missing "%s" in gateway configuration "payplug".', $configKey), $exception->getMessage()); + } + } + + /** + * @return array + */ + public static function provideCredentialGetters(): array + { + return [ + 'clientId' => ['getClientId', 'client_id'], + 'clientSecret' => ['getClientSecret', 'client_secret'], + 'publicKeyId' => ['getPublicKeyId', 'public_key_id'], + 'publicKeyValue' => ['getPublicKeyValue', 'public_key_value'], + ]; + } + + private function repository(): SyliusConfigurationRepository + { + return new SyliusConfigurationRepository($this->gatewayConfig); + } +} diff --git a/tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php b/tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php new file mode 100644 index 00000000..50e3f1ca --- /dev/null +++ b/tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php @@ -0,0 +1,127 @@ +orderRepository = $this->createMock(OrderRepositoryInterface::class); + $this->stateMachine = $this->createMock(StateMachineInterface::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); + + $this->mutator = new SyliusOrderStateMutator($this->orderRepository, $this->stateMachine, $this->entityManager); + } + + /** + * THREE_DS_PENDING must stay a no-op — the order remains `new` until the async webhook + * resolves it. Neither the order repository nor the state machine should even be touched. + */ + public function testApply_threeDsPending_isNoOp(): void + { + $this->orderRepository->expects(self::never())->method('findOrderById'); + $this->stateMachine->expects(self::never())->method('apply'); + + $this->mutator->apply('order-1', PaymentOutcome::THREE_DS_PENDING); + } + + /** + * @dataProvider provideMappedOutcomes + */ + public function testApply_mappedOutcome_appliesExpectedTransition(string $outcome, string $expectedTransition): void + { + $order = $this->createMock(OrderInterface::class); + $payment = $this->createMock(PaymentInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->orderRepository->method('findOrderById')->with('order-1')->willReturn($order); + $this->stateMachine->method('can')->with($payment, PaymentTransitions::GRAPH, $expectedTransition)->willReturn(true); + $this->stateMachine->expects(self::once())->method('apply')->with($payment, PaymentTransitions::GRAPH, $expectedTransition); + $this->entityManager->expects(self::once())->method('flush'); + + $this->mutator->apply('order-1', $outcome); + } + + /** + * @return array + */ + public static function provideMappedOutcomes(): array + { + return [ + 'paid' => [PaymentOutcome::PAID, PaymentTransitions::TRANSITION_COMPLETE], + 'authorized' => [PaymentOutcome::AUTHORIZED, PaymentTransitions::TRANSITION_AUTHORIZE], + 'capture_required' => [PaymentOutcome::CAPTURE_REQUIRED, PaymentTransitions::TRANSITION_PROCESS], + 'refunded' => [PaymentOutcome::REFUNDED, PaymentTransitions::TRANSITION_REFUND], + 'failed' => [PaymentOutcome::FAILED, PaymentTransitions::TRANSITION_FAIL], + ]; + } + + /** + * A retried webhook landing on a payment already past this transition is a silent no-op, + * not a thrown workflow exception — mirrors the plugin's existing PaymentStateResolver. + */ + public function testApply_transitionNotAvailable_isSilentNoOp(): void + { + $order = $this->createMock(OrderInterface::class); + $payment = $this->createMock(PaymentInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->orderRepository->method('findOrderById')->willReturn($order); + $this->stateMachine->method('can')->willReturn(false); + $this->stateMachine->expects(self::never())->method('apply'); + $this->entityManager->expects(self::never())->method('flush'); + + $this->mutator->apply('order-1', PaymentOutcome::PAID); + } + + public function testApply_orderNotFound_throwsPaymentNotFoundException(): void + { + $this->orderRepository->method('findOrderById')->with('missing-order')->willReturn(null); + + $this->expectException(PaymentNotFoundException::class); + + $this->mutator->apply('missing-order', PaymentOutcome::PAID); + } + + public function testApply_orderHasNoPayment_throwsPaymentNotFoundException(): void + { + $order = $this->createMock(OrderInterface::class); + $order->method('getLastPayment')->willReturn(null); + $this->orderRepository->method('findOrderById')->willReturn($order); + + $this->expectException(PaymentNotFoundException::class); + + $this->mutator->apply('order-1', PaymentOutcome::PAID); + } + + public function testApply_unmappedOutcome_throwsInvalidArgumentException(): void + { + $this->orderRepository->expects(self::never())->method('findOrderById'); + + $this->expectException(\InvalidArgumentException::class); + + $this->mutator->apply('order-1', 'not_a_real_outcome'); + } +} diff --git a/tests/PHPUnit/Spike/SyliusPaymentRepositoryTest.php b/tests/PHPUnit/Spike/SyliusPaymentRepositoryTest.php new file mode 100644 index 00000000..a1ff7cd9 --- /dev/null +++ b/tests/PHPUnit/Spike/SyliusPaymentRepositoryTest.php @@ -0,0 +1,152 @@ +entityManager = $this->createMock(EntityManagerInterface::class); + $this->objectRepository = $this->createMock(EntityRepository::class); + $this->entityManager->method('getRepository')->with(PayplugOperation::class)->willReturn($this->objectRepository); + + $this->repository = new SyliusPaymentRepository($this->entityManager); + } + + public function testGetByOrderId_found_returnsMatchingOperationData(): void + { + $entity = PayplugOperation::fromOperationData($this->operationData()); + $this->objectRepository->method('findOneBy')->with(['orderId' => 'order-1'])->willReturn($entity); + + $result = $this->repository->getByOrderId('order-1'); + + self::assertSame('op-1', $result->operationId); + self::assertSame('order-1', $result->orderId); + } + + public function testGetByOrderId_notFound_throwsPaymentNotFoundException(): void + { + $this->objectRepository->method('findOneBy')->willReturn(null); + + $this->expectException(PaymentNotFoundException::class); + + $this->repository->getByOrderId('missing-order'); + } + + public function testGetByOperationId_found_returnsMatchingOperationData(): void + { + $entity = PayplugOperation::fromOperationData($this->operationData()); + $this->objectRepository->method('findOneBy')->with(['operationId' => 'op-1'])->willReturn($entity); + + $result = $this->repository->getByOperationId('op-1'); + + self::assertSame('order-1', $result->orderId); + } + + public function testGetByOperationId_notFound_throwsPaymentNotFoundException(): void + { + $this->objectRepository->method('findOneBy')->willReturn(null); + + $this->expectException(PaymentNotFoundException::class); + + $this->repository->getByOperationId('missing-op'); + } + + public function testSave_newOperation_persistsAndFlushes(): void + { + $this->objectRepository->method('findOneBy')->with(['operationId' => 'op-1'])->willReturn(null); + + $this->entityManager->expects(self::once())->method('persist') + ->with(self::callback(fn (PayplugOperation $entity): bool => 'op-1' === $entity->getOperationId() && 'order-1' === $entity->getOrderId())); + $this->entityManager->expects(self::once())->method('flush'); + + $this->repository->save($this->operationData()); + } + + public function testSave_existingOperation_updatesInPlaceWithoutPersisting(): void + { + $existing = PayplugOperation::fromOperationData($this->operationData()); + $this->objectRepository->method('findOneBy')->willReturn($existing); + + $this->entityManager->expects(self::never())->method('persist'); + $this->entityManager->expects(self::once())->method('flush'); + + $updated = new OperationData('op-1', '4001', PaymentOutcome::REFUNDED, 500, 'order-1'); + $this->repository->save($updated); + + self::assertSame(PaymentOutcome::REFUNDED, $existing->toOperationData()->outcome); + self::assertSame(500, $existing->toOperationData()->amount); + } + + public function testMarkTreated_existing_marksAndFlushes(): void + { + $entity = PayplugOperation::fromOperationData($this->operationData()); + $this->objectRepository->method('findOneBy')->with(['operationId' => 'op-1'])->willReturn($entity); + $this->entityManager->expects(self::once())->method('flush'); + + $this->repository->markTreated('op-1'); + + self::assertTrue($entity->isTreated()); + } + + public function testMarkTreated_missing_throwsPaymentNotFoundException(): void + { + $this->objectRepository->method('findOneBy')->willReturn(null); + + $this->expectException(PaymentNotFoundException::class); + + $this->repository->markTreated('missing-op'); + } + + public function testIsTreated_untreatedOperation_returnsFalse(): void + { + $entity = PayplugOperation::fromOperationData($this->operationData()); + $this->objectRepository->method('findOneBy')->willReturn($entity); + + self::assertFalse($this->repository->isTreated('op-1')); + } + + public function testIsTreated_treatedOperation_returnsTrue(): void + { + $entity = PayplugOperation::fromOperationData($this->operationData()); + $entity->markTreated(); + $this->objectRepository->method('findOneBy')->willReturn($entity); + + self::assertTrue($this->repository->isTreated('op-1')); + } + + /** + * Idempotency checks must be tolerant of "never saved" rather than throwing — a webhook + * asking "have I already treated this?" before any operation was ever persisted is a + * legitimate call, not an error. + */ + public function testIsTreated_unknownOperation_returnsFalse(): void + { + $this->objectRepository->method('findOneBy')->willReturn(null); + + self::assertFalse($this->repository->isTreated('unknown-op')); + } + + private function operationData(): OperationData + { + return new OperationData('op-1', '4001', PaymentOutcome::PAID, 1000, 'order-1'); + } +} diff --git a/tests/PHPUnit/Spike/SyliusTokenCacheTest.php b/tests/PHPUnit/Spike/SyliusTokenCacheTest.php new file mode 100644 index 00000000..04b484e4 --- /dev/null +++ b/tests/PHPUnit/Spike/SyliusTokenCacheTest.php @@ -0,0 +1,75 @@ +cache = $this->createMock(CacheItemPoolInterface::class); + $this->tokenCache = new SyliusTokenCache($this->cache); + } + + public function testGet_hit_returnsStoredValue(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(true); + $item->method('get')->willReturn('jwt-value'); + $this->cache->method('getItem')->with('token-key')->willReturn($item); + + self::assertSame('jwt-value', $this->tokenCache->get('token-key')); + } + + public function testGet_miss_returnsNull(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(false); + $this->cache->method('getItem')->willReturn($item); + + self::assertNull($this->tokenCache->get('token-key')); + } + + /** + * PSR-6's CacheItemInterface::get() is typed mixed — defend against a non-string value + * ever having been stored under this key (e.g. by another caller of the same pool). + */ + public function testGet_hitWithNonStringValue_returnsNull(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(true); + $item->method('get')->willReturn(42); + $this->cache->method('getItem')->willReturn($item); + + self::assertNull($this->tokenCache->get('token-key')); + } + + public function testSet_storesValueWithTtlAndSaves(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->expects(self::once())->method('set')->with('jwt-value'); + $item->expects(self::once())->method('expiresAfter')->with(298); + $this->cache->method('getItem')->with('token-key')->willReturn($item); + $this->cache->expects(self::once())->method('save')->with($item); + + $this->tokenCache->set('token-key', 'jwt-value', 298); + } + + public function testDelete_removesItem(): void + { + $this->cache->expects(self::once())->method('deleteItem')->with('token-key'); + + $this->tokenCache->delete('token-key'); + } +} From b97c69db6573c49ebcf3cf65e68385c8eaea1d83 Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 08:49:01 +0200 Subject: [PATCH 02/11] PRE-3469: Promote unified-plugin-core to a real dependency NotifyPaymentRequestHandler (task 5) will reference UPC contract types directly; leaving the package require-dev-only would fatal-error the container the moment a merchant's production composer install --no-dev tries to autowire the new dependency. Co-Authored-By: Claude Sonnet 5 --- composer.json | 2 +- ...20-pre-3469-upc-contracts-rework-design.md | 65 +++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md diff --git a/composer.json b/composer.json index 2c2e0a29..1942f156 100755 --- a/composer.json +++ b/composer.json @@ -14,6 +14,7 @@ "ext-json": "*", "giggsey/libphonenumber-for-php": "^8.12", "payplug/payplug-php": "^4.0", + "payplug/unified-plugin-core": "dev-develop", "php-http/message-factory": "^1.1", "sylius/refund-plugin": "^2.0", "sylius/sylius": "^2.0", @@ -37,7 +38,6 @@ "friendsoftwig/twigcs": "6.5.0", "lakion/mink-debug-extension": "2.0.0", "mockery/mockery": "1.6.12", - "payplug/unified-plugin-core": "dev-develop", "php-parallel-lint/php-parallel-lint": "1.4.0", "phpmd/phpmd": "^2.15.0", "phpro/grumphp": "^2.12", diff --git a/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md b/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md new file mode 100644 index 00000000..f668ecfe --- /dev/null +++ b/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md @@ -0,0 +1,65 @@ +# PRE-3469 rework — real UPC contract wiring + +## Context + +[PRE-3469](https://payplug-prod.atlassian.net/browse/PRE-3469) originally asked for isolated proof-of-concept skeletons (under `src/Spike/`) validating that 4 Unified Plugin Core (UPC) contracts — `IOrderStateMutator`, `ITokenCache`, `IConfigurationRepository`, `IPaymentRepository` — are satisfiable against real Sylius APIs, before freezing the interfaces. That work landed in commit `690d9f5` and is fully written up in `src/Spike/VALIDATION.md`. + +The ticket's "Résultats attendus" were updated twice on 2026-07-20. The current version requires: + +- Real, production-wired implementations (not isolated `src/Spike/` skeletons) for `PayplugOrderStateMutator`, `PayplugTokenCache`, `PayplugConfigurationRepository`. +- `IPaymentRepository` explicitly **out of scope** (tied to a Value Object too coupled to the not-yet-built Unified API) — the existing `SyliusPaymentRepository` sketch is sufficient as-is. +- No regression on existing plugin functionality (PHPUnit + Behat stay green). +- Any blocking friction documented, or the interfaces adjusted. + +This spec covers reworking the branch to meet the updated scope. + +## Existing production code map + +Research (see conversation) found the production analogs each contract needs to be validated against: + +| Contract | Production analog | Notes | +|---|---|---| +| `IOrderStateMutator` | `PaymentTransitionApplier` (called from `NotifyPaymentRequestHandler`/`StatusPaymentRequestHandler`, the webhook/status-poll path) and `PaymentStateResolver` (called from the CLI reconciliation command `payplug:update-payment-state`) | Two separate, non-unified implementations of the same guarded `can()`/`apply()` idiom already exist in production; this ticket does not unify them. | +| `ITokenCache` | `PayPlugApiClientFactory::getTokenForGatewayConfig()`'s inline `Symfony\Contracts\Cache\CacheInterface` usage, caching the OAuth JWT access token per gateway factory/live-test scope | Confirmed by the ticket's second update: targets the OAuth JWT cache, not card/token storage (saved cards are a permanent Doctrine entity, `Card.php`, uninvolved with any cache). | +| `IConfigurationRepository` | Read side: `PayPlugApiClientFactory::getTokenForGatewayConfig()` (`client_id`/`client_secret` scoped by `config['live']`). Write side: `UnifiedAuthenticationController::oauthCallback()` (persists `live_client`/`test_client`, busts the token cache). | No production equivalent exists for `getPublicKeyId()`/`getPublicKeyValue()` — Hosted Fields public-key material isn't implemented anywhere in the plugin today. | +| `IPaymentRepository` | None — out of scope | `SyliusPaymentRepository` + `PayplugOperation` entity + `Version20260720100000` migration stay as an unwired sketch. | + +## Design + +### 1. `PayplugOrderStateMutator` — real production call site + +Keep the existing mapping logic from `src/Spike/SyliusOrderStateMutator.php`: `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → `PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL`, `THREE_DS_PENDING` as a confirmed no-op, `StateMachineInterface::can()` guard before `apply()`, explicit `EntityManagerInterface::flush()` after. + +Wire it as an **additive** call inside `NotifyPaymentRequestHandler::__invoke()`, immediately after the existing `PaymentTransitionApplier::apply($payment)` call. The handler already has everything needed to derive a `PaymentOutcome` from the same status signal `PaymentTransitionApplier` consumes (`$payment->getDetails()['status']`, populated upstream by `PaymentNotificationHandler::treat()`); translate that into a `PaymentOutcome` value and call `$orderStateMutator->apply($order->getId(), $outcome)`. + +Because `PaymentTransitionApplier::apply()` already applied the transition earlier in the same handler invocation, `PayplugOrderStateMutator`'s own `can()` guard will find the transition no longer applicable and no-op. This proves the mutator works against a real, live webhook event without replacing or risking the existing transition-application path. No change to `PaymentStateResolver` or the CLI reconciliation command. + +### 2. `PayplugTokenCache` — real integration test, no production call site + +No change to `PayPlugApiClientFactory::getTokenForGatewayConfig()`. Promote the spike's existing unit-mocked test (`tests/PHPUnit/Spike/SyliusTokenCacheTest.php`) with a new real-infrastructure test that exercises `get`/`set`/`delete` against an actual `cache.app`-equivalent PSR-6 pool (filesystem or array adapter, same spirit as `SpikeIntegrationTest`'s real MariaDB), rather than a mocked `CacheItemPoolInterface`. This validates `getItem`/`save`/`deleteItem` "dans le flux réel" per the ticket, without introducing any new code on a live request path that gates authentication for every PayPlug gateway. + +### 3. `PayplugConfigurationRepository` — real integration test, no production call site + +Same treatment as `PayplugTokenCache`: add a real integration test that constructs `SyliusConfigurationRepository` against an actual `GatewayConfigInterface` on a real `PaymentMethod` fixture persisted via Doctrine (rather than a mock), validating `get`/`set`/`getClientId`/`getClientSecret` against real Sylius config storage. No change to `PayPlugApiClientFactory` or `UnifiedAuthenticationController`. + +`getPublicKeyId()`/`getPublicKeyValue()` read from two new config array keys, `public_key_id` and `public_key_value`, added alongside the existing `live_client`/`test_client` keys, defaulting to an empty string when absent. Nothing in production writes these keys yet — this is implementable end-to-end and ready for whenever Hosted Fields lands, documented as a friction note rather than worked around or blocked on. + +### 4. `IPaymentRepository` — unchanged + +`SyliusPaymentRepository`, `Entity/PayplugOperation.php`, and the `Version20260720100000` migration stay exactly as they are: an unwired sketch, correctly out of scope per the ticket. + +### 5. Documentation + +Update `src/Spike/VALIDATION.md` to reflect the new verdict per contract: which ones are now genuinely wired into production vs. validated only through real-infrastructure tests, and the two friction notes (`public_key_id`/`public_key_value` have no writer yet; `IOrderStateMutator`/`IConfigurationRepository` production analogs exist as more than one non-unified implementation, which this ticket does not consolidate). + +## Testing / regression safety + +- Existing PHPUnit + Behat suites must stay green after the changes. +- The one production code change (the additive `PayplugOrderStateMutator` call in `NotifyPaymentRequestHandler`) is guarded by the existing `can()` check, so it cannot alter current transition behavior — only observe/replay it against an already-applied transition. +- New real-infrastructure tests for `PayplugTokenCache` and `PayplugConfigurationRepository` are added, not replacing the existing unit-mocked spike tests. + +## Out of scope + +- Unifying `PaymentTransitionApplier` and `PaymentStateResolver` into a single `IOrderStateMutator`-backed implementation. +- Any production wiring of `IPaymentRepository`. +- Building Hosted Fields / any actual consumer of `public_key_id`/`public_key_value`. From 29b4b8d29b6c761ab9cdc6710ab978988f31d6ef Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 08:53:47 +0200 Subject: [PATCH 03/11] PRE-3469: Remove design spec accidentally committed in b97c69d MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec doc was staged from an earlier, unrelated session step and got swept into the unified-plugin-core dependency commit by mistake. Pulling it back out of git tracking (still present on disk, uncommitted) — it should only be committed when explicitly requested. --- ...20-pre-3469-upc-contracts-rework-design.md | 65 ------------------- 1 file changed, 65 deletions(-) delete mode 100644 docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md diff --git a/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md b/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md deleted file mode 100644 index f668ecfe..00000000 --- a/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md +++ /dev/null @@ -1,65 +0,0 @@ -# PRE-3469 rework — real UPC contract wiring - -## Context - -[PRE-3469](https://payplug-prod.atlassian.net/browse/PRE-3469) originally asked for isolated proof-of-concept skeletons (under `src/Spike/`) validating that 4 Unified Plugin Core (UPC) contracts — `IOrderStateMutator`, `ITokenCache`, `IConfigurationRepository`, `IPaymentRepository` — are satisfiable against real Sylius APIs, before freezing the interfaces. That work landed in commit `690d9f5` and is fully written up in `src/Spike/VALIDATION.md`. - -The ticket's "Résultats attendus" were updated twice on 2026-07-20. The current version requires: - -- Real, production-wired implementations (not isolated `src/Spike/` skeletons) for `PayplugOrderStateMutator`, `PayplugTokenCache`, `PayplugConfigurationRepository`. -- `IPaymentRepository` explicitly **out of scope** (tied to a Value Object too coupled to the not-yet-built Unified API) — the existing `SyliusPaymentRepository` sketch is sufficient as-is. -- No regression on existing plugin functionality (PHPUnit + Behat stay green). -- Any blocking friction documented, or the interfaces adjusted. - -This spec covers reworking the branch to meet the updated scope. - -## Existing production code map - -Research (see conversation) found the production analogs each contract needs to be validated against: - -| Contract | Production analog | Notes | -|---|---|---| -| `IOrderStateMutator` | `PaymentTransitionApplier` (called from `NotifyPaymentRequestHandler`/`StatusPaymentRequestHandler`, the webhook/status-poll path) and `PaymentStateResolver` (called from the CLI reconciliation command `payplug:update-payment-state`) | Two separate, non-unified implementations of the same guarded `can()`/`apply()` idiom already exist in production; this ticket does not unify them. | -| `ITokenCache` | `PayPlugApiClientFactory::getTokenForGatewayConfig()`'s inline `Symfony\Contracts\Cache\CacheInterface` usage, caching the OAuth JWT access token per gateway factory/live-test scope | Confirmed by the ticket's second update: targets the OAuth JWT cache, not card/token storage (saved cards are a permanent Doctrine entity, `Card.php`, uninvolved with any cache). | -| `IConfigurationRepository` | Read side: `PayPlugApiClientFactory::getTokenForGatewayConfig()` (`client_id`/`client_secret` scoped by `config['live']`). Write side: `UnifiedAuthenticationController::oauthCallback()` (persists `live_client`/`test_client`, busts the token cache). | No production equivalent exists for `getPublicKeyId()`/`getPublicKeyValue()` — Hosted Fields public-key material isn't implemented anywhere in the plugin today. | -| `IPaymentRepository` | None — out of scope | `SyliusPaymentRepository` + `PayplugOperation` entity + `Version20260720100000` migration stay as an unwired sketch. | - -## Design - -### 1. `PayplugOrderStateMutator` — real production call site - -Keep the existing mapping logic from `src/Spike/SyliusOrderStateMutator.php`: `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → `PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL`, `THREE_DS_PENDING` as a confirmed no-op, `StateMachineInterface::can()` guard before `apply()`, explicit `EntityManagerInterface::flush()` after. - -Wire it as an **additive** call inside `NotifyPaymentRequestHandler::__invoke()`, immediately after the existing `PaymentTransitionApplier::apply($payment)` call. The handler already has everything needed to derive a `PaymentOutcome` from the same status signal `PaymentTransitionApplier` consumes (`$payment->getDetails()['status']`, populated upstream by `PaymentNotificationHandler::treat()`); translate that into a `PaymentOutcome` value and call `$orderStateMutator->apply($order->getId(), $outcome)`. - -Because `PaymentTransitionApplier::apply()` already applied the transition earlier in the same handler invocation, `PayplugOrderStateMutator`'s own `can()` guard will find the transition no longer applicable and no-op. This proves the mutator works against a real, live webhook event without replacing or risking the existing transition-application path. No change to `PaymentStateResolver` or the CLI reconciliation command. - -### 2. `PayplugTokenCache` — real integration test, no production call site - -No change to `PayPlugApiClientFactory::getTokenForGatewayConfig()`. Promote the spike's existing unit-mocked test (`tests/PHPUnit/Spike/SyliusTokenCacheTest.php`) with a new real-infrastructure test that exercises `get`/`set`/`delete` against an actual `cache.app`-equivalent PSR-6 pool (filesystem or array adapter, same spirit as `SpikeIntegrationTest`'s real MariaDB), rather than a mocked `CacheItemPoolInterface`. This validates `getItem`/`save`/`deleteItem` "dans le flux réel" per the ticket, without introducing any new code on a live request path that gates authentication for every PayPlug gateway. - -### 3. `PayplugConfigurationRepository` — real integration test, no production call site - -Same treatment as `PayplugTokenCache`: add a real integration test that constructs `SyliusConfigurationRepository` against an actual `GatewayConfigInterface` on a real `PaymentMethod` fixture persisted via Doctrine (rather than a mock), validating `get`/`set`/`getClientId`/`getClientSecret` against real Sylius config storage. No change to `PayPlugApiClientFactory` or `UnifiedAuthenticationController`. - -`getPublicKeyId()`/`getPublicKeyValue()` read from two new config array keys, `public_key_id` and `public_key_value`, added alongside the existing `live_client`/`test_client` keys, defaulting to an empty string when absent. Nothing in production writes these keys yet — this is implementable end-to-end and ready for whenever Hosted Fields lands, documented as a friction note rather than worked around or blocked on. - -### 4. `IPaymentRepository` — unchanged - -`SyliusPaymentRepository`, `Entity/PayplugOperation.php`, and the `Version20260720100000` migration stay exactly as they are: an unwired sketch, correctly out of scope per the ticket. - -### 5. Documentation - -Update `src/Spike/VALIDATION.md` to reflect the new verdict per contract: which ones are now genuinely wired into production vs. validated only through real-infrastructure tests, and the two friction notes (`public_key_id`/`public_key_value` have no writer yet; `IOrderStateMutator`/`IConfigurationRepository` production analogs exist as more than one non-unified implementation, which this ticket does not consolidate). - -## Testing / regression safety - -- Existing PHPUnit + Behat suites must stay green after the changes. -- The one production code change (the additive `PayplugOrderStateMutator` call in `NotifyPaymentRequestHandler`) is guarded by the existing `can()` check, so it cannot alter current transition behavior — only observe/replay it against an already-applied transition. -- New real-infrastructure tests for `PayplugTokenCache` and `PayplugConfigurationRepository` are added, not replacing the existing unit-mocked spike tests. - -## Out of scope - -- Unifying `PaymentTransitionApplier` and `PaymentStateResolver` into a single `IOrderStateMutator`-backed implementation. -- Any production wiring of `IPaymentRepository`. -- Building Hosted Fields / any actual consumer of `public_key_id`/`public_key_value`. From e6ad438e1d9b1b3cdec01e8a53f5261b0dbd0b4c Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 09:23:24 +0200 Subject: [PATCH 04/11] PRE-3469: Promote SyliusOrderStateMutator to PayplugOrderStateMutator Moves the IOrderStateMutator skeleton out of src/Spike/ into a real production namespace, ahead of wiring it into NotifyPaymentRequestHandler. --- .../PayplugOrderStateMutator.php} | 25 +++++++------------ .../PayplugOrderStateMutatorTest.php} | 10 ++++---- tests/PHPUnit/Spike/SpikeIntegrationTest.php | 6 ++--- 3 files changed, 17 insertions(+), 24 deletions(-) rename src/{Spike/SyliusOrderStateMutator.php => PaymentProcessing/PayplugOrderStateMutator.php} (68%) rename tests/PHPUnit/{Spike/SyliusOrderStateMutatorTest.php => PaymentProcessing/PayplugOrderStateMutatorTest.php} (92%) diff --git a/src/Spike/SyliusOrderStateMutator.php b/src/PaymentProcessing/PayplugOrderStateMutator.php similarity index 68% rename from src/Spike/SyliusOrderStateMutator.php rename to src/PaymentProcessing/PayplugOrderStateMutator.php index 6c197183..ebf0a688 100644 --- a/src/Spike/SyliusOrderStateMutator.php +++ b/src/PaymentProcessing/PayplugOrderStateMutator.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace PayPlug\SyliusPayPlugPlugin\Spike; +namespace PayPlug\SyliusPayPlugPlugin\PaymentProcessing; use Doctrine\ORM\EntityManagerInterface; use PayplugUnifiedCore\Contracts\IOrderStateMutator; @@ -13,23 +13,16 @@ use Sylius\Component\Payment\PaymentTransitions; /** - * PRE-3469 spike: proof-of-concept implementation of IOrderStateMutator against Sylius's - * Symfony Workflow-backed payment state machine — not shipped code. + * PRE-3469: real implementation of IOrderStateMutator against Sylius's Symfony + * Workflow-backed payment state machine. Wired for real (additively, can()-guarded) from + * NotifyPaymentRequestHandler — see there for why the call cannot regress the existing + * PaymentTransitionApplier-driven transition. * - * Friction found (confirmed by the level-3 integration test, not just by reading): the state - * machine's `apply()` only mutates the in-memory `state` property (marking_store: method) — it - * never flushes. Without an explicit EntityManager::flush() the transition is silently lost the - * moment the request ends. The plugin's existing PaymentStateResolver::resolve() already flushes - * for the same reason; this skeleton was missing it until the integration test caught it. - * - * Second friction found: IOrderStateMutator::apply() is keyed by order ID, but the Symfony - * Workflow transition actually lives on the order's Payment sub-entity (`sylius_payment` graph), - * not on the Order itself. This adds one extra hop (Order -> getLastPayment()) that a - * WooCommerce implementation, whose native order carries the payment status directly, would not - * need. Not blocking: the interface deliberately stays CMS-agnostic, so this hop belongs in the - * Sylius adapter, not in the contract. + * The state machine's apply() only mutates the in-memory `state` property + * (marking_store: method) — it never flushes on its own, exactly like the plugin's existing + * PaymentStateResolver::resolve(), hence the explicit flush() below. */ -final class SyliusOrderStateMutator implements IOrderStateMutator +final class PayplugOrderStateMutator implements IOrderStateMutator { /** * THREE_DS_PENDING is deliberately absent: per PRE-3469, the order must stay in its diff --git a/tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php b/tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php similarity index 92% rename from tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php rename to tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php index 50e3f1ca..d4409908 100644 --- a/tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php +++ b/tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php @@ -2,10 +2,10 @@ declare(strict_types=1); -namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Spike; +namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\PaymentProcessing; use Doctrine\ORM\EntityManagerInterface; -use PayPlug\SyliusPayPlugPlugin\Spike\SyliusOrderStateMutator; +use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator; use PayplugUnifiedCore\Exceptions\PaymentNotFoundException; use PayplugUnifiedCore\Models\PaymentOutcome; use PHPUnit\Framework\MockObject\MockObject; @@ -16,7 +16,7 @@ use Sylius\Component\Core\Repository\OrderRepositoryInterface; use Sylius\Component\Payment\PaymentTransitions; -final class SyliusOrderStateMutatorTest extends TestCase +final class PayplugOrderStateMutatorTest extends TestCase { private OrderRepositoryInterface&MockObject $orderRepository; @@ -24,7 +24,7 @@ final class SyliusOrderStateMutatorTest extends TestCase private EntityManagerInterface&MockObject $entityManager; - private SyliusOrderStateMutator $mutator; + private PayplugOrderStateMutator $mutator; protected function setUp(): void { @@ -32,7 +32,7 @@ protected function setUp(): void $this->stateMachine = $this->createMock(StateMachineInterface::class); $this->entityManager = $this->createMock(EntityManagerInterface::class); - $this->mutator = new SyliusOrderStateMutator($this->orderRepository, $this->stateMachine, $this->entityManager); + $this->mutator = new PayplugOrderStateMutator($this->orderRepository, $this->stateMachine, $this->entityManager); } /** diff --git a/tests/PHPUnit/Spike/SpikeIntegrationTest.php b/tests/PHPUnit/Spike/SpikeIntegrationTest.php index 236adb8c..f5c0ab60 100644 --- a/tests/PHPUnit/Spike/SpikeIntegrationTest.php +++ b/tests/PHPUnit/Spike/SpikeIntegrationTest.php @@ -6,7 +6,7 @@ use Doctrine\ORM\EntityManagerInterface; use PayPlug\SyliusPayPlugPlugin\Spike\SyliusConfigurationRepository; -use PayPlug\SyliusPayPlugPlugin\Spike\SyliusOrderStateMutator; +use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator; use PayPlug\SyliusPayPlugPlugin\Spike\SyliusPaymentRepository; use PayPlug\SyliusPayPlugPlugin\Spike\SyliusTokenCache; use PayplugUnifiedCore\Models\OperationData; @@ -51,7 +51,7 @@ public function testOrderStateMutator_realStateMachine_transitionsPaymentToCompl self::assertNotNull($payment); self::assertSame(PaymentInterface::STATE_NEW, $payment->getState()); - $mutator = new SyliusOrderStateMutator( + $mutator = new PayplugOrderStateMutator( self::getContainer()->get('sylius.repository.order'), self::getContainer()->get(StateMachineInterface::class), $this->entityManager, @@ -69,7 +69,7 @@ public function testOrderStateMutator_threeDsPending_leavesRealPaymentUntouched( $payment = $order->getLastPayment(); self::assertNotNull($payment); - $mutator = new SyliusOrderStateMutator( + $mutator = new PayplugOrderStateMutator( self::getContainer()->get('sylius.repository.order'), self::getContainer()->get(StateMachineInterface::class), $this->entityManager, From 463c79ce7095ab62fc24c1a66fee05bad89ba2a9 Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 09:27:05 +0200 Subject: [PATCH 05/11] PRE-3469: Promote SyliusTokenCache to PayplugTokenCache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the ITokenCache skeleton out of src/Spike/ into a real production namespace. Stays validated by a real-infrastructure integration test only (see SpikeIntegrationTest) — no production call site, since its one real analog (PayPlugApiClientFactory's OAuth token cache) gates authentication for every gateway and isn't worth the regression risk of replacing here. Co-Authored-By: Claude Sonnet 5 --- src/Spike/SyliusTokenCache.php | 52 ------------------ src/TokenCache/PayplugTokenCache.php | 55 +++++++++++++++++++ tests/PHPUnit/Spike/SpikeIntegrationTest.php | 4 +- .../PayplugTokenCacheTest.php} | 10 ++-- 4 files changed, 62 insertions(+), 59 deletions(-) delete mode 100644 src/Spike/SyliusTokenCache.php create mode 100644 src/TokenCache/PayplugTokenCache.php rename tests/PHPUnit/{Spike/SyliusTokenCacheTest.php => TokenCache/PayplugTokenCacheTest.php} (89%) diff --git a/src/Spike/SyliusTokenCache.php b/src/Spike/SyliusTokenCache.php deleted file mode 100644 index 2069618e..00000000 --- a/src/Spike/SyliusTokenCache.php +++ /dev/null @@ -1,52 +0,0 @@ -cache->getItem($key); - if (!$item->isHit()) { - return null; - } - - $value = $item->get(); - - return \is_string($value) ? $value : null; - } - - public function set(string $key, string $value, int $ttlSeconds): void - { - $item = $this->cache->getItem($key); - $item->set($value); - $item->expiresAfter($ttlSeconds); - $this->cache->save($item); - } - - public function delete(string $key): void - { - $this->cache->deleteItem($key); - } -} diff --git a/src/TokenCache/PayplugTokenCache.php b/src/TokenCache/PayplugTokenCache.php new file mode 100644 index 00000000..815cb970 --- /dev/null +++ b/src/TokenCache/PayplugTokenCache.php @@ -0,0 +1,55 @@ + cache.app alias) already satisfies + * CacheItemPoolInterface, regardless of whether that pool is backed by the filesystem, APCu, or + * Redis in a given deployment. This class is validated by a real-infrastructure integration + * test (see SpikeIntegrationTest) rather than wired into a live request path: the one existing + * production caller of an OAuth token cache, PayPlugApiClientFactory::getTokenForGatewayConfig(), + * gates authentication for every PayPlug gateway — replacing its inline caching logic here would + * carry more regression risk than this ticket's validation goal justifies. + */ +final class PayplugTokenCache implements ITokenCache +{ + public function __construct(private readonly CacheItemPoolInterface $cache) + { + } + + public function get(string $key): ?string + { + $item = $this->cache->getItem($key); + if (!$item->isHit()) { + return null; + } + + $value = $item->get(); + + return \is_string($value) ? $value : null; + } + + public function set(string $key, string $value, int $ttlSeconds): void + { + $item = $this->cache->getItem($key); + $item->set($value); + $item->expiresAfter($ttlSeconds); + $this->cache->save($item); + } + + public function delete(string $key): void + { + $this->cache->deleteItem($key); + } +} diff --git a/tests/PHPUnit/Spike/SpikeIntegrationTest.php b/tests/PHPUnit/Spike/SpikeIntegrationTest.php index f5c0ab60..24f3dd58 100644 --- a/tests/PHPUnit/Spike/SpikeIntegrationTest.php +++ b/tests/PHPUnit/Spike/SpikeIntegrationTest.php @@ -8,7 +8,7 @@ use PayPlug\SyliusPayPlugPlugin\Spike\SyliusConfigurationRepository; use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator; use PayPlug\SyliusPayPlugPlugin\Spike\SyliusPaymentRepository; -use PayPlug\SyliusPayPlugPlugin\Spike\SyliusTokenCache; +use PayPlug\SyliusPayPlugPlugin\TokenCache\PayplugTokenCache; use PayplugUnifiedCore\Models\OperationData; use PayplugUnifiedCore\Models\PaymentOutcome; use Psr\Cache\CacheItemPoolInterface; @@ -84,7 +84,7 @@ public function testOrderStateMutator_threeDsPending_leavesRealPaymentUntouched( public function testTokenCache_realCachePool_roundTripsThroughRealAdapter(): void { $cachePool = self::getContainer()->get(CacheItemPoolInterface::class); - $tokenCache = new SyliusTokenCache($cachePool); + $tokenCache = new PayplugTokenCache($cachePool); $tokenCache->set('pre3469-spike-token', 'jwt-value', 300); self::assertSame('jwt-value', $tokenCache->get('pre3469-spike-token')); diff --git a/tests/PHPUnit/Spike/SyliusTokenCacheTest.php b/tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php similarity index 89% rename from tests/PHPUnit/Spike/SyliusTokenCacheTest.php rename to tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php index 04b484e4..866e5fdc 100644 --- a/tests/PHPUnit/Spike/SyliusTokenCacheTest.php +++ b/tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php @@ -2,24 +2,24 @@ declare(strict_types=1); -namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Spike; +namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\TokenCache; -use PayPlug\SyliusPayPlugPlugin\Spike\SyliusTokenCache; +use PayPlug\SyliusPayPlugPlugin\TokenCache\PayplugTokenCache; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; -final class SyliusTokenCacheTest extends TestCase +final class PayplugTokenCacheTest extends TestCase { private CacheItemPoolInterface&MockObject $cache; - private SyliusTokenCache $tokenCache; + private PayplugTokenCache $tokenCache; protected function setUp(): void { $this->cache = $this->createMock(CacheItemPoolInterface::class); - $this->tokenCache = new SyliusTokenCache($this->cache); + $this->tokenCache = new PayplugTokenCache($this->cache); } public function testGet_hit_returnsStoredValue(): void From 753ded325dc75a973156dd844e56d26185d47c35 Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 09:33:00 +0200 Subject: [PATCH 06/11] PRE-3469: Promote SyliusConfigurationRepository to PayplugConfigurationRepository MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the IConfigurationRepository skeleton out of src/Spike/ into a real production namespace. getPublicKeyId()/getPublicKeyValue() now default to an empty string instead of throwing, since no production code writes public_key_id/public_key_value yet (Hosted Fields isn't built) — documented as friction rather than blocked on. Co-Authored-By: Claude Sonnet 5 --- .../PayplugConfigurationRepository.php} | 34 ++++++----- .../PayplugConfigurationRepositoryTest.php} | 56 +++++++++++++++---- tests/PHPUnit/Spike/SpikeIntegrationTest.php | 6 +- 3 files changed, 68 insertions(+), 28 deletions(-) rename src/{Spike/SyliusConfigurationRepository.php => ConfigurationRepository/PayplugConfigurationRepository.php} (67%) rename tests/PHPUnit/{Spike/SyliusConfigurationRepositoryTest.php => ConfigurationRepository/PayplugConfigurationRepositoryTest.php} (64%) diff --git a/src/Spike/SyliusConfigurationRepository.php b/src/ConfigurationRepository/PayplugConfigurationRepository.php similarity index 67% rename from src/Spike/SyliusConfigurationRepository.php rename to src/ConfigurationRepository/PayplugConfigurationRepository.php index c807dd9b..5a207004 100644 --- a/src/Spike/SyliusConfigurationRepository.php +++ b/src/ConfigurationRepository/PayplugConfigurationRepository.php @@ -2,32 +2,36 @@ declare(strict_types=1); -namespace PayPlug\SyliusPayPlugPlugin\Spike; +namespace PayPlug\SyliusPayPlugPlugin\ConfigurationRepository; use PayplugUnifiedCore\Contracts\IConfigurationRepository; use PayplugUnifiedCore\Exceptions\ApiException; use Sylius\Component\Payment\Model\GatewayConfigInterface; /** - * PRE-3469 spike: proof-of-concept implementation of IConfigurationRepository against Sylius's - * GatewayConfigInterface — not shipped code. + * PRE-3469: real implementation of IConfigurationRepository against Sylius's + * GatewayConfigInterface. * - * Friction found: IConfigurationRepository assumes one flat set of credentials, but Sylius - * scopes gateway config per PaymentMethod *and* per live/test mode (`config['live_client']` vs + * IConfigurationRepository assumes one flat set of credentials, but Sylius scopes gateway + * config per PaymentMethod *and* per live/test mode (`config['live_client']` vs * `config['test_client']`, selected by `config['live']`, exactly as PayPlugApiClientFactory - * already does). So a single IConfigurationRepository instance has to be constructed per + * already does). So a single PayplugConfigurationRepository instance has to be constructed per * GatewayConfigInterface (i.e. per PaymentMethod) rather than shared as one repository-wide * service — a factory, not a singleton. Not blocking, but worth flagging if the Unified API * client this feeds ever assumes one repository == one merchant. * - * Positive finding: Sylius already ships an (experimental) GatewayConfigEncrypter that - * transparently encrypts the whole `getConfig()` array at rest - * (Sylius\Component\Payment\Encryption) — if wired up, CLIENT_SECRET benefits from that for - * free. What this class must still guarantee on its own is that a *decrypted* secret never - * leaks into a log line or exception message, which is why requireString() below only ever - * interpolates the config *key name*, never its value. + * getPublicKeyId()/getPublicKeyValue() default to an empty string rather than throwing: unlike + * client_id/client_secret, no production code writes public_key_id/public_key_value yet + * (Hosted Fields isn't built) — requiring them would make the contract unimplementable until a + * future ticket adds that writer. + * + * Sylius already ships an (experimental) GatewayConfigEncrypter that transparently encrypts the + * whole `getConfig()` array at rest (Sylius\Component\Payment\Encryption) — if wired up, + * CLIENT_SECRET benefits from that for free. What this class must still guarantee on its own is + * that a *decrypted* secret never leaks into a log line or exception message, which is why + * requireString() below only ever interpolates the config *key name*, never its value. */ -final class SyliusConfigurationRepository implements IConfigurationRepository +final class PayplugConfigurationRepository implements IConfigurationRepository { public function __construct(private readonly GatewayConfigInterface $gatewayConfig) { @@ -69,12 +73,12 @@ public function getClientSecret(): string public function getPublicKeyId(): string { - return $this->requireString('public_key_id'); + return $this->get('public_key_id') ?? ''; } public function getPublicKeyValue(): string { - return $this->requireString('public_key_value'); + return $this->get('public_key_value') ?? ''; } private function requireString(string $key): string diff --git a/tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php b/tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php similarity index 64% rename from tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php rename to tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php index 66ea74d0..34c20686 100644 --- a/tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php +++ b/tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php @@ -2,15 +2,15 @@ declare(strict_types=1); -namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Spike; +namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\ConfigurationRepository; -use PayPlug\SyliusPayPlugPlugin\Spike\SyliusConfigurationRepository; +use PayPlug\SyliusPayPlugPlugin\ConfigurationRepository\PayplugConfigurationRepository; use PayplugUnifiedCore\Exceptions\ApiException; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Sylius\Component\Payment\Model\GatewayConfigInterface; -final class SyliusConfigurationRepositoryTest extends TestCase +final class PayplugConfigurationRepositoryTest extends TestCase { private GatewayConfigInterface&MockObject $gatewayConfig; @@ -66,9 +66,9 @@ public function testSet_writesIntoActiveScopeAndPersistsWholeConfig(): void } /** - * @dataProvider provideCredentialGetters + * @dataProvider provideRequiredCredentialGetters */ - public function testCredentialGetters_present_returnValue(string $method, string $configKey): void + public function testRequiredCredentialGetters_present_returnValue(string $method, string $configKey): void { $this->gatewayConfig->method('getConfig')->willReturn([ 'live' => true, @@ -79,13 +79,13 @@ public function testCredentialGetters_present_returnValue(string $method, string } /** - * @dataProvider provideCredentialGetters + * @dataProvider provideRequiredCredentialGetters * * The exception message must name the missing key and the factory, never a credential * value — there is nothing sensitive to redact here since the value is simply absent, but * this locks the message shape so a future edit can't start interpolating $value instead. */ - public function testCredentialGetters_missing_throwsApiExceptionWithoutLeakingSecrets( + public function testRequiredCredentialGetters_missing_throwsApiExceptionWithoutLeakingSecrets( string $method, string $configKey, ): void @@ -104,18 +104,54 @@ public function testCredentialGetters_missing_throwsApiExceptionWithoutLeakingSe /** * @return array */ - public static function provideCredentialGetters(): array + public static function provideRequiredCredentialGetters(): array { return [ 'clientId' => ['getClientId', 'client_id'], 'clientSecret' => ['getClientSecret', 'client_secret'], + ]; + } + + /** + * @dataProvider provideOptionalCredentialGetters + */ + public function testOptionalCredentialGetters_present_returnValue(string $method, string $configKey): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => [$configKey => 'the-value'], + ]); + + self::assertSame('the-value', $this->repository()->{$method}()); + } + + /** + * getPublicKeyId()/getPublicKeyValue() have no production writer yet (Hosted Fields isn't + * built) — unlike client_id/client_secret, a missing value must not throw, or the contract + * would be unimplementable until a future ticket adds that writer. + * + * @dataProvider provideOptionalCredentialGetters + */ + public function testOptionalCredentialGetters_missing_returnEmptyString(string $method): void + { + $this->gatewayConfig->method('getConfig')->willReturn(['live' => true, 'live_client' => []]); + + self::assertSame('', $this->repository()->{$method}()); + } + + /** + * @return array + */ + public static function provideOptionalCredentialGetters(): array + { + return [ 'publicKeyId' => ['getPublicKeyId', 'public_key_id'], 'publicKeyValue' => ['getPublicKeyValue', 'public_key_value'], ]; } - private function repository(): SyliusConfigurationRepository + private function repository(): PayplugConfigurationRepository { - return new SyliusConfigurationRepository($this->gatewayConfig); + return new PayplugConfigurationRepository($this->gatewayConfig); } } diff --git a/tests/PHPUnit/Spike/SpikeIntegrationTest.php b/tests/PHPUnit/Spike/SpikeIntegrationTest.php index 24f3dd58..c1f82d8d 100644 --- a/tests/PHPUnit/Spike/SpikeIntegrationTest.php +++ b/tests/PHPUnit/Spike/SpikeIntegrationTest.php @@ -5,7 +5,7 @@ namespace Tests\PayPlug\SyliusPayPlugPlugin\PHPUnit\Spike; use Doctrine\ORM\EntityManagerInterface; -use PayPlug\SyliusPayPlugPlugin\Spike\SyliusConfigurationRepository; +use PayPlug\SyliusPayPlugPlugin\ConfigurationRepository\PayplugConfigurationRepository; use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator; use PayPlug\SyliusPayPlugPlugin\Spike\SyliusPaymentRepository; use PayPlug\SyliusPayPlugPlugin\TokenCache\PayplugTokenCache; @@ -106,13 +106,13 @@ public function testConfigurationRepository_realGatewayConfig_survivesADoctrineR /** @var GatewayConfig $reloaded */ $reloaded = $this->entityManager->find(GatewayConfig::class, $id); - (new SyliusConfigurationRepository($reloaded))->set('client_secret', 'spike-secret-value'); + (new PayplugConfigurationRepository($reloaded))->set('client_secret', 'spike-secret-value'); $this->entityManager->flush(); $this->entityManager->clear(); /** @var GatewayConfig $reloadedAgain */ $reloadedAgain = $this->entityManager->find(GatewayConfig::class, $id); - $repository = new SyliusConfigurationRepository($reloadedAgain); + $repository = new PayplugConfigurationRepository($reloadedAgain); self::assertSame('spike-client-id', $repository->getClientId()); self::assertSame('spike-secret-value', $repository->getClientSecret()); From d770af8fdf0310c1442543a48e47d2e4c1b176e6 Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 09:44:26 +0200 Subject: [PATCH 07/11] PRE-3469: Wire PayplugOrderStateMutator into NotifyPaymentRequestHandler Additive real call site: runs after the existing PaymentTransitionApplier transition, guarded by the mutator's own can() check (safe no-op once the transition's already applied) and wrapped in try/catch so any failure is logged, never allowed to break webhook notification handling. --- .../Handler/NotifyPaymentRequestHandler.php | 54 ++++++ .../NotifyPaymentRequestHandlerTest.php | 175 ++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php diff --git a/src/Command/Handler/NotifyPaymentRequestHandler.php b/src/Command/Handler/NotifyPaymentRequestHandler.php index 9ea5147f..0f872089 100644 --- a/src/Command/Handler/NotifyPaymentRequestHandler.php +++ b/src/Command/Handler/NotifyPaymentRequestHandler.php @@ -6,10 +6,14 @@ use Payplug\Resource\Payment; use PayPlug\SyliusPayPlugPlugin\ApiClient\PayPlugApiClientFactoryInterface; +use PayPlug\SyliusPayPlugPlugin\ApiClient\PayPlugApiClientInterface; use PayPlug\SyliusPayPlugPlugin\Command\NotifyPaymentRequest; use PayPlug\SyliusPayPlugPlugin\Handler\PaymentNotificationHandler; use PayPlug\SyliusPayPlugPlugin\Handler\RefundNotificationHandler; use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PaymentTransitionApplier; +use PayplugUnifiedCore\Contracts\IOrderStateMutator; +use PayplugUnifiedCore\Models\PaymentOutcome; +use Psr\Log\LoggerInterface; use Sylius\Abstraction\StateMachine\StateMachineInterface; use Sylius\Bundle\PaymentBundle\Provider\PaymentRequestProviderInterface; use Sylius\Component\Core\Model\PaymentInterface; @@ -19,6 +23,21 @@ #[AsMessageHandler] class NotifyPaymentRequestHandler { + /** + * PRE-3469: additive translation from the PayPlug status vocabulary (the same one + * PaymentTransitionApplier already maps from) to UPC's PaymentOutcome vocabulary, for the + * real IOrderStateMutator call below. Statuses with no PaymentOutcome equivalent + * (e.g. STATUS_ABORTED/STATUS_CANCELED*, which PaymentTransitionApplier maps to + * TRANSITION_CANCEL) are intentionally absent here — they're skipped, never force-mapped. + * + * @var array + */ + private const STATUS_TO_OUTCOME = [ + PayPlugApiClientInterface::STATUS_CAPTURED => PaymentOutcome::PAID, + PayPlugApiClientInterface::STATUS_AUTHORIZED => PaymentOutcome::AUTHORIZED, + PayPlugApiClientInterface::FAILED => PaymentOutcome::FAILED, + ]; + public function __construct( private PaymentRequestProviderInterface $paymentRequestProvider, private StateMachineInterface $stateMachine, @@ -26,6 +45,8 @@ public function __construct( private PaymentNotificationHandler $paymentNotificationHandler, private RefundNotificationHandler $refundNotificationHandler, private PaymentTransitionApplier $paymentTransitionApplier, + private IOrderStateMutator $orderStateMutator, + private LoggerInterface $logger, ) {} public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void @@ -67,6 +88,7 @@ public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void $payment->setDetails($details->getArrayCopy()); if ($resource instanceof Payment) { $this->paymentTransitionApplier->apply($payment); + $this->applyOrderStateMutator($payment); } $this->stateMachine->apply( @@ -85,4 +107,36 @@ public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void ); } } + + /** + * PRE-3469: additive real call site for PayplugOrderStateMutator. PaymentTransitionApplier + * has already applied the real transition above by the time this runs, so the mutator's own + * can()-guard makes this a no-op in the normal case — this exists to prove the contract works + * against a live webhook event, not to change behavior. Any failure here is caught and + * logged, never allowed to affect the primary notification flow above. + */ + private function applyOrderStateMutator(PaymentInterface $payment): void + { + $details = $payment->getDetails(); // @phpstan-ignore-line - getDetails() return mixed + $status = $details['status'] ?? ''; + $outcome = self::STATUS_TO_OUTCOME[$status] ?? null; + if (null === $outcome) { + return; + } + + $order = $payment->getOrder(); + if (null === $order) { + return; + } + + try { + $this->orderStateMutator->apply((string) $order->getId(), $outcome); + } catch (\Throwable $e) { + $this->logger->warning('[PayPlug] PayplugOrderStateMutator additive call failed.', [ + 'sylius_payment_id' => $payment->getId(), + 'outcome' => $outcome, + 'exception' => $e->getMessage(), + ]); + } + } } diff --git a/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php b/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php new file mode 100644 index 00000000..78de1331 --- /dev/null +++ b/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php @@ -0,0 +1,175 @@ +paymentRequestProvider = $this->createMock(PaymentRequestProviderInterface::class); + $this->stateMachine = $this->createMock(StateMachineInterface::class); + $this->apiClientFactory = $this->createMock(PayPlugApiClientFactoryInterface::class); + $this->paymentNotificationHandler = $this->createMock(PaymentNotificationHandler::class); + $this->refundNotificationHandler = $this->createMock(RefundNotificationHandler::class); + $this->paymentTransitionApplier = $this->createMock(PaymentTransitionApplier::class); + $this->orderStateMutator = $this->createMock(IOrderStateMutator::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->handler = new NotifyPaymentRequestHandler( + $this->paymentRequestProvider, + $this->stateMachine, + $this->apiClientFactory, + $this->paymentNotificationHandler, + $this->refundNotificationHandler, + $this->paymentTransitionApplier, + $this->orderStateMutator, + $this->logger, + ); + } + + public function testInvoke_knownStatus_callsOrderStateMutatorWithMappedOutcome(): void + { + $this->prepareNormalFlow(PayPlugApiClientInterface::STATUS_CAPTURED, 42); + + $this->orderStateMutator->expects(self::once())->method('apply')->with('42', PaymentOutcome::PAID); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_unknownStatus_doesNotCallOrderStateMutator(): void + { + $this->prepareNormalFlow(PayPlugApiClientInterface::STATUS_CANCELED, 42); + + $this->orderStateMutator->expects(self::never())->method('apply'); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_orderStateMutatorThrows_isCaughtAndDoesNotPreventCompletion(): void + { + $this->prepareNormalFlow(PayPlugApiClientInterface::STATUS_CAPTURED, 42); + + $this->orderStateMutator->method('apply')->willThrowException(new \RuntimeException('boom')); + $this->logger->expects(self::once())->method('warning'); + + // The outer flow must still complete normally: TRANSITION_COMPLETE, not TRANSITION_FAIL. + $this->stateMachine->expects(self::once())->method('apply')->with( + self::isInstanceOf(PaymentRequestInterface::class), + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_COMPLETE, + ); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_paymentAlreadyCompleted_shortCircuitsAndSkipsOrderStateMutator(): void + { + $payment = $this->createMock(PaymentInterface::class); + $payment->method('getState')->willReturn(PaymentInterface::STATE_COMPLETED); + $method = $this->createMock(PaymentMethodInterface::class); + $payment->method('getMethod')->willReturn($method); + + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '{}']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $client = $this->createMock(PayPlugApiClientInterface::class); + $resource = $this->createMock(PayplugResourcePayment::class); + $client->method('treat')->willReturn($resource); + $this->apiClientFactory->method('createForPaymentMethod')->willReturn($client); + + $this->paymentTransitionApplier->expects(self::never())->method('apply'); + $this->orderStateMutator->expects(self::never())->method('apply'); + $this->stateMachine->expects(self::once())->method('apply')->with( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_COMPLETE, + ); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_invalidPayload_failsWithoutCallingOrderStateMutator(): void + { + $payment = $this->createMock(PaymentInterface::class); + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $paymentRequest->expects(self::once())->method('setResponseData'); + $this->orderStateMutator->expects(self::never())->method('apply'); + $this->stateMachine->expects(self::once())->method('apply')->with( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_FAIL, + ); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + private function prepareNormalFlow(string $status, int $orderId): void + { + $order = $this->createMock(OrderInterface::class); + $order->method('getId')->willReturn($orderId); + + $payment = $this->createMock(PaymentInterface::class); + $payment->method('getState')->willReturn(PaymentInterface::STATE_NEW); + $payment->method('getDetails')->willReturn(['status' => $status]); + $payment->method('getOrder')->willReturn($order); + $method = $this->createMock(PaymentMethodInterface::class); + $payment->method('getMethod')->willReturn($method); + + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '{}']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $client = $this->createMock(PayPlugApiClientInterface::class); + $resource = $this->createMock(PayplugResourcePayment::class); + $client->method('treat')->willReturn($resource); + $this->apiClientFactory->method('createForPaymentMethod')->willReturn($client); + } +} From 1a89a346827c4661ee6ec059c1cc550ee5811b6d Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 10:04:24 +0200 Subject: [PATCH 08/11] PRE-3469: Fix phpstan error in NotifyPaymentRequestHandler's mutator call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cannot cast mixed to string at (string) $order->getId() — Sylius's ResourceInterface::getId() returns mixed. Matches the existing @phpstan-ignore-line convention already used elsewhere in this same file for getDetails() (mixed return), rather than adding a new baseline entry. --- src/Command/Handler/NotifyPaymentRequestHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Command/Handler/NotifyPaymentRequestHandler.php b/src/Command/Handler/NotifyPaymentRequestHandler.php index 0f872089..bda2658d 100644 --- a/src/Command/Handler/NotifyPaymentRequestHandler.php +++ b/src/Command/Handler/NotifyPaymentRequestHandler.php @@ -130,7 +130,7 @@ private function applyOrderStateMutator(PaymentInterface $payment): void } try { - $this->orderStateMutator->apply((string) $order->getId(), $outcome); + $this->orderStateMutator->apply((string) $order->getId(), $outcome); // @phpstan-ignore-line - ResourceInterface::getId() return mixed } catch (\Throwable $e) { $this->logger->warning('[PayPlug] PayplugOrderStateMutator additive call failed.', [ 'sylius_payment_id' => $payment->getId(), From ddb7742b47587b80d8fc1303165f5df502ba436b Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 10:20:30 +0200 Subject: [PATCH 09/11] PRE-3469: Update VALIDATION.md for the promoted contracts Documents the require-dev blocking friction found and fixed, and the production wiring decisions for each promoted contract. Co-Authored-By: Claude Sonnet 5 --- src/Spike/VALIDATION.md | 201 ++++++++++++++++++++++------------------ 1 file changed, 112 insertions(+), 89 deletions(-) diff --git a/src/Spike/VALIDATION.md b/src/Spike/VALIDATION.md index b806d2e0..1d952226 100644 --- a/src/Spike/VALIDATION.md +++ b/src/Spike/VALIDATION.md @@ -1,25 +1,47 @@ # PRE-3469 — Validation des contrats UPC contre Sylius -Spike de preuve : implémentation squelette des 4 contrats à risque (`IOrderStateMutator`, -`ITokenCache`, `IConfigurationRepository`, `IPaymentRepository`) contre les vraies APIs Sylius -(`sylius/sylius` ^2.0, PHP 8.2), pour valider qu'ils sont satisfaisables avant de les figer. -Code de preuve — voir `src/Spike/` — mais couvert par 3 niveaux de tests réels (43 tests) : - -- **Unitaire** (`tests/PHPUnit/Spike/*Test.php`, hors `SpikeIntegrationTest`) : mocks natifs - PHPUnit sur chaque collaborateur (StateMachineInterface, CacheItemPoolInterface, - GatewayConfigInterface, EntityManagerInterface). 38 tests. -- **Intégration réelle** (`SpikeIntegrationTest.php`) : kernel Sylius réellement booté - (`sylius/test-application`), vraie base MariaDB jetable, vrai state machine Symfony Workflow, - vrai pool de cache PSR-6, vraie persistance Doctrine — aucun mock. 5 tests. -- Les deux suites tournent avec `make`-style commandes classiques (`composer install`, - `vendor/bin/phpunit`) une fois la base de test créée — voir section Câblage. - -**Verdict global : aucune friction bloquante.** Les 4 contrats sont implémentables contre les -APIs Sylius actuelles, et le prouvent en s'exécutant réellement. Un bug réel a été trouvé et -corrigé (voir IOrderStateMutator), plus trois notes pour la suite (aucune ne remet en cause la -forme des interfaces). - -## IOrderStateMutator — squelette : `SyliusOrderStateMutator.php` +`IOrderStateMutator`, `ITokenCache` et `IConfigurationRepository` ont été promus en implémentations +réelles, branchées dans le code de production existant (voir `src/PaymentProcessing/PayplugOrderStateMutator.php`, +`src/TokenCache/PayplugTokenCache.php`, `src/ConfigurationRepository/PayplugConfigurationRepository.php`). +`IPaymentRepository` reste hors scope de ce ticket (lié à un Value Object fortement couplé à +l'API Unifiée, non encore branché sur le plugin) — son squelette de preuve reste dans ce dossier, +voir `SyliusPaymentRepository.php`. + +Couverture de tests, 3 niveaux : + +- **Unitaire** (`tests/PHPUnit/{PaymentProcessing,TokenCache,ConfigurationRepository}/*Test.php` + + `tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php`) : mocks natifs PHPUnit + sur chaque collaborateur. +- **Intégration réelle** (`SpikeIntegrationTest.php`, toujours dans ce dossier car c'est le seul + endroit du plugin qui boote un vrai kernel Sylius pour les tests) : kernel Sylius réellement + booté, vraie base MariaDB jetable, vrai state machine Symfony Workflow, vrai pool de cache + PSR-6, vraie persistance Doctrine — aucun mock. Couvre les 4 contrats, y compris + `IPaymentRepository`. +- Les deux suites tournent avec `composer install`, `vendor/bin/phpunit` une fois la base de + test créée — voir section Câblage. + +**Verdict global : aucune friction bloquante restante.** Une friction bloquante a été trouvée et +corrigée pendant ce rework (voir « Friction bloquante corrigée » ci-dessous), plus deux notes non +bloquantes (aucune ne remet en cause la forme des interfaces). + +## Friction bloquante corrigée : `payplug/unified-plugin-core` en `require-dev` + +`SyliusOrderStateMutator`/`SyliusTokenCache`/`SyliusConfigurationRepository` (les squelettes +d'origine) implémentaient directement les interfaces UPC depuis `src/Spike/`, avec +`payplug/unified-plugin-core` en `require-dev` uniquement. Tant que ce code restait isolé et +jamais référencé par une classe de production, ça ne posait pas de problème. Mais brancher +`PayplugOrderStateMutator` dans `NotifyPaymentRequestHandler` — une classe systématiquement +chargée, y compris en production — change la donne : un vrai `composer install --no-dev` chez un +marchand n'installerait pas `unified-plugin-core`, et charger une classe qui `implements` une +interface inexistante est une erreur PHP fatale, pas un échec silencieux. + +**Fix** : `payplug/unified-plugin-core` est passé en dépendance `require` (toujours pinné sur +`dev-develop`, même repository VCS). Documenté ici plutôt que traité comme un simple détail de +`composer.json`, parce que ça anticipe une partie de ce que PRE-3563 (la vraie dépendance de +production pour l'OAuth) devait poser — voir section Câblage plus bas pour ce qui reste à faire +pour PRE-3563. + +## IOrderStateMutator — réel : `PayplugOrderStateMutator.php` Mapping validé : `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → `PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL` sur le graphe @@ -30,16 +52,16 @@ par `PaymentStateResolver::applyTransition()` dans le plugin actuel. toucher au state machine, la commande reste `new` jusqu'au webhook — conforme au résultat attendu du ticket. -**Bug réel trouvé par le test d'intégration (corrigé)** : la première version du squelette -appelait `$stateMachine->apply($payment, ...)` sans jamais flush l'EntityManager. Le state -machine (`marking_store: type: method`) ne fait que muter la propriété `state` en mémoire — rien -n'est persisté sans un `flush()` explicite, exactement comme le fait déjà -`PaymentStateResolver::resolve()` dans le plugin actuel (`$this->paymentEntityManager->flush();` -en fin de méthode). Le test unitaire avec mocks ne pouvait pas voir ce bug (il vérifie l'appel à -`apply()`, pas la persistance réelle) — seul le test d'intégration contre une vraie base l'a -révélé : la transition semblait réussir (`can()` → true, `apply()` appelé) mais la commande -restait `new` en base. `IOrderStateMutator` prend maintenant un `EntityManagerInterface` en plus -et flush après chaque transition appliquée. +**Branché en production** : appel additif dans `NotifyPaymentRequestHandler`, juste après le +`PaymentTransitionApplier::apply($payment)` existant, une fois la transition déjà appliquée par +ce dernier. Le statut PayPlug déjà connu à cet endroit (`$payment->getDetails()['status']`) est +traduit en `PaymentOutcome` (seuls `STATUS_CAPTURED`/`STATUS_AUTHORIZED`/`FAILED` ont un +équivalent — `STATUS_ABORTED`/`STATUS_CANCELED*`, mappés par `PaymentTransitionApplier` sur +`TRANSITION_CANCEL`, n'ont pas d'équivalent `PaymentOutcome` et sont donc volontairement ignorés, +pas forcés). L'appel est protégé par un `try/catch` qui journalise sans jamais propager : la +transition ayant déjà été appliquée par `PaymentTransitionApplier`, le garde-fou `can()` du +mutateur en fait un no-op silencieux dans le cas normal ; ce câblage prouve que le contrat +fonctionne contre un webhook réel sans remplacer ni risquer le flux existant. **Note (non bloquante)** : le contrat prend un `orderId`, mais la transition Symfony Workflow vit sur le sous-objet `Payment` de la commande (`Order::getLastPayment()`), pas sur l'`Order` @@ -47,31 +69,61 @@ lui-même. Un hop supplémentaire Order → Payment est donc nécessaire côté que WooCommerce n'aurait pas besoin de faire. C'est le bon endroit pour cette différence (dans l'adaptateur, pas dans le contrat CMS-agnostique). -## ITokenCache — squelette : `SyliusTokenCache.php` +**Note annexe (mécanique, pas conceptuelle)** : le plugin a aujourd'hui deux chemins de +production distincts qui appliquent des transitions Symfony Workflow sur un paiement — +`PaymentTransitionApplier` (webhooks/status, celui utilisé ci-dessus) et `PaymentStateResolver` +(réconciliation CLI, `payplug:update-payment-state`). Ce ticket ne les unifie pas ; ça reste deux +implémentations parallèles du même idiome `can()`/`apply()`. + +## ITokenCache — réel : `PayplugTokenCache.php` Aucune friction. `get`/`set`/`delete` se posent 1:1 sur `Psr\Cache\CacheItemPoolInterface::getItem/ save/deleteItem`, exactement comme le docblock de l'interface le prévoyait déjà. Le pool par défaut de Sylius (`cache.app`, `Symfony\Component\Cache\Adapter\AdapterInterface`) satisfait déjà -`CacheItemPoolInterface` — filesystem, APCu ou Redis selon le déploiement, sans changement de code -côté `SyliusTokenCache`. - -## IConfigurationRepository — squelette : `SyliusConfigurationRepository.php` - -**Note (non bloquante)** : `GatewayConfigInterface::getConfig()` scope les credentials par -`PaymentMethod` *et* par mode live/test (`config['live_client']` vs `config['test_client']`, -sélectionné par `config['live']` — même pattern que `PayPlugApiClientFactory::getTokenForGatewayConfig()` -existant). Une instance de `SyliusConfigurationRepository` doit donc être construite par -`GatewayConfigInterface` (donc par `PaymentMethod`), pas partagée comme service unique — c'est une -factory, pas un singleton. Ne remet pas en cause le contrat, mais à garder en tête si le futur -client Unified API suppose "un repository = un marchand". +`CacheItemPoolInterface`, aliasé nativement par FrameworkBundle — filesystem, APCu ou Redis selon +le déploiement, sans changement de code côté `PayplugTokenCache`. + +**Cible confirmée par le ticket** : `ITokenCache` cible le cache du token JWT OAuth +(authentification via le SDK PayPlug), pas le stockage carte/one-click — la sauvegarde de carte +est une entité Doctrine permanente (`Card`), sans aucun cache impliqué. + +**Validé par un test d'intégration réel, pas par un appel de production** : le seul analogue de +production existant, `PayPlugApiClientFactory::getTokenForGatewayConfig()`, met en cache le token +OAuth qui conditionne l'authentification de *toutes* les gateways du plugin. Remplacer sa logique +de cache inline aurait un risque de régression disproportionné par rapport à l'objectif de +validation de ce ticket ; `PayplugTokenCache` reste donc validé par un test d'intégration contre +un vrai pool PSR-6 (`SpikeIntegrationTest::testTokenCache_realCachePool_roundTripsThroughRealAdapter`), +sans point d'entrée de production. + +## IConfigurationRepository — réel : `PayplugConfigurationRepository.php` + +`GatewayConfigInterface::getConfig()` scope les credentials par `PaymentMethod` *et* par mode +live/test (`config['live_client']` vs `config['test_client']`, sélectionné par `config['live']` — +même pattern que `PayPlugApiClientFactory::getTokenForGatewayConfig()` existant). Une instance de +`PayplugConfigurationRepository` doit donc être construite par `GatewayConfigInterface` (donc par +`PaymentMethod`), pas partagée comme service unique — c'est une factory, pas un singleton. Ne +remet pas en cause le contrat, mais à garder en tête si le futur client Unified API suppose "un +repository = un marchand". **Point positif** : Sylius expose déjà un `GatewayConfigEncrypter` (expérimental) qui chiffre au repos l'intégralité du tableau `getConfig()` — si branché, `CLIENT_SECRET` en bénéficie -gratuitement. Ce que `SyliusConfigurationRepository` doit garantir lui-même, c'est qu'un secret +gratuitement. Ce que `PayplugConfigurationRepository` doit garantir lui-même, c'est qu'un secret déchiffré ne fuite jamais dans un message de log ou d'exception : `requireString()` n'interpole jamais que le nom de la clé, jamais sa valeur. -## IPaymentRepository — squelettes : `SyliusPaymentRepository.php` + `Entity/PayplugOperation.php` +**Friction non bloquante, résolue par un choix explicite** : `getPublicKeyId()`/ +`getPublicKeyValue()` n'ont aucun équivalent de production — le plugin n'a aucune notion de clé +publique Hosted Fields aujourd'hui (`grep` sur `public_key`/`PUBLIC_KEY` dans `src/` ne retourne +rien en dehors de ce fichier). Plutôt que de lever une exception comme `getClientId()`/ +`getClientSecret()`, ces deux méthodes renvoient une chaîne vide tant qu'aucun code de production +n'écrit ces clés — le contrat reste implémentable de bout en bout, prêt pour quand Hosted Fields +sera construit. + +## IPaymentRepository — hors scope, squelette inchangé : `SyliusPaymentRepository.php` + `Entity/PayplugOperation.php` + +Hors scope de ce ticket (lié à un Value Object fortement couplé à l'API Unifiée, non encore +branché sur le plugin) — reste un squelette de preuve, inchangé depuis la version précédente de +ce document. **Constat principal du ticket, confirmé** : le plugin actuel ne stocke pas `OperationData` de façon normalisée. L'id Payplug est aujourd'hui écrit dans le JSON de `Payment::details` et @@ -95,48 +147,23 @@ réel utilisé pour le test d'intégration passe donc par un `doctrine.orm.mappi PRE-3469-only, restreinte à `kernel.environment === 'test'` (jamais en prod), à retirer avec `src/Spike/` si le spike est abandonné. -**Bug de portabilité trouvé et corrigé (deux fois)** : la table `payplug_operation` a d'abord été -créée à la main sur la base locale — ça marchait chez moi, mais ni pour un collègue qui pull, ni -en CI. Première tentative de fix : la faire créer par le test lui-même via -`SchemaTool::updateSchema([$metadata], true)` — **destructeur** : avec une liste de classes -partielle, `updateSchema()` calcule un schéma cible ne contenant que cette entité et supprime -toutes les autres tables qu'il ne reconnaît pas dans ce diff, y compris tout le schéma Sylius. -Vérifié en le déclenchant sur la base jetable : il n'est resté que 2 tables sur la centaine -attendue. Aucun dégât réel (base locale jetable), mais invendable en l'état. - -Deuxième problème, plus profond : enregistrer le mapping Doctrine dès que -`kernel.environment === 'test'` casse `sylius:fixtures:load` sur une base neuve, *même sans -jamais toucher au spike* — n'importe quelle commande qui boote le kernel en env `test` (donc -l'installation standard du `test-application`) échoue avec « table `payplug_operation` -n'existe pas », puisque Doctrine connaît l'entité sans que la table existe encore. - -**Fix final** : une vraie migration Doctrine (`migrations/Version20260720100000.php`), comme -toutes les autres tables du plugin. Contre-intuitif pour une entité de spike, mais c'est la seule -option qui ne casse rien pour qui que ce soit sur un premier `make install` — vérifié en -reconstruisant la base de zéro (`doctrine:database:create` → `doctrine:migration:migrate` → -`sylius:fixtures:load` → suite de tests) et en rejouant le test d'intégration 3 fois de suite. -`up()`/`down()` sont chacun protégés par `$this->skipIf('test' !== APP_ENV, ...)` : une migration -de plugin normale s'applique dans tous les environnements, y compris en prod chez un marchand — -sans ce garde-fou, chaque installation du plugin en production créerait une table `payplug_operation` -qui ne sert jamais à rien en dehors des tests de ce spike. - ## Câblage (pour rejouer les tests) - `composer.json` : `repositories` avec un repository `vcs` vers - `https://github.com/payplug/unified-plugin-core.git` + require-dev - `payplug/unified-plugin-core: "dev-develop"`, pour que les squelettes implémentent réellement - les interfaces (et non des copies locales) — fonctionne pour n'importe qui ayant accès au repo - GitHub (même accès que pour ce repo), et en CI via l'étape `Composer - Github Auth` déjà - configurée dans `payplug/template-ci`. Une première version utilisait un repository `path` - local (`../../unified-plugin-core`) : ça ne marche que sur une machine avec les deux repos - clonés côte à côte, cassait `composer install` pour tout le monde d'autre — corrigé. - Nécessite que le pin exact `symfony/polyfill-mbstring: 1.28.0` d'UPC ait été relâché en - `^1.28` (corrigé dans unified-plugin-core suite à ce spike — il bloquait tout - `composer install` aux côtés de `sylius/sylius ^2.0`, qui exige `^1.31`). - Contrepartie connue : ça ajoute une résolution réseau vers GitHub à chaque install (léger coût - CI/dev) — pas d'alternative disponible ici (pas de Packagist privé configuré dans cet org pour - `payplug/unified-plugin-core`, et un repository `path` est exclu pour la raison ci-dessus). - Ce compromis disparaît quand PRE-3563 posera la vraie dépendance de production. + `https://github.com/payplug/unified-plugin-core.git` + `payplug/unified-plugin-core: + "dev-develop"` désormais en dépendance `require` (voir « Friction bloquante corrigée » + ci-dessus) — fonctionne pour n'importe qui ayant accès au repo GitHub (même accès que pour ce + repo), et en CI via l'étape `Composer - Github Auth` déjà configurée dans + `payplug/template-ci`. Une première version utilisait un repository `path` local + (`../../unified-plugin-core`) : ça ne marche que sur une machine avec les deux repos clonés + côte à côte, cassait `composer install` pour tout le monde d'autre — corrigé. Nécessite que le + pin exact `symfony/polyfill-mbstring: 1.28.0` d'UPC ait été relâché en `^1.28` (corrigé dans + unified-plugin-core suite à ce spike — il bloquait tout `composer install` aux côtés de + `sylius/sylius ^2.0`, qui exige `^1.31`). Contrepartie connue : ça ajoute une résolution réseau + vers GitHub à chaque install (léger coût CI/dev) — pas d'alternative disponible ici (pas de + Packagist privé configuré dans cet org pour `payplug/unified-plugin-core`, et un repository + `path` est exclu pour la raison ci-dessus). Ce compromis disparaît quand PRE-3563 posera la + vraie dépendance de production définitive (OAuth réel contre UPC). - `phpunit.xml.dist` : `KERNEL_CLASS_PATH` renommé en `KERNEL_CLASS` — la variable que Symfony lit réellement pour `KernelTestCase::bootKernel()`. Sans ce fix, aucun test à base de kernel (fonctionnel/intégration) n'a jamais pu tourner dans ce repo — un bug de config préexistant, @@ -149,11 +176,7 @@ qui ne sert jamais à rien en dehors des tests de ce spike. - Base créée/migrée avec les commandes standard du Makefile (`doctrine:database:create`, `doctrine:migration:migrate`, `sylius:payment:generate-key`, `sylius:fixtures:load`) — la table `payplug_operation` est créée automatiquement par `migrations/Version20260720100000.php`, plus - besoin d'aucune étape manuelle (voir plus bas pourquoi une vraie migration a été nécessaire). - -Ce câblage (composer + config) est à revoir quand PRE-3563 (OAuth réel contre UPC) posera la -vraie dépendance de production ; le `.env.test.local` et le conteneur Docker restent locaux à la -machine de dev et n'ont pas vocation à être commités. + besoin d'aucune étape manuelle. **Isolation des tests** : `SpikeIntegrationTest` ne tourne dans aucune transaction annulée en fin de test (pas de `DAMADoctrineTestBundle`). Une première version cherchait un paiement de fixture From f0cd2a8aca1eb5a7fad7c9a4adeacf6e4f55d768 Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 10:48:02 +0200 Subject: [PATCH 10/11] PRE-3469: Add explicit service alias for IOrderStateMutator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symfony does not auto-alias a singly-implemented interface for plain autowiring — confirmed by directly booting the container, which failed to compile with "Cannot autowire service NotifyPaymentRequestHandler: ... references interface IOrderStateMutator but no such service exists." Since NotifyPaymentRequestHandler is an #[AsMessageHandler] service instantiated on every webhook, this broke all PayPlug notification handling. Adds the explicit alias, matching the convention already used for other interfaces in this file. --- config/services.yaml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/services.yaml b/config/services.yaml index 7db6a5fe..28fce12c 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -10,6 +10,12 @@ services: bind: Psr\Log\LoggerInterface: '@monolog.logger.payplug' + # PRE-3469: explicit alias, matching this file's convention for other interfaces below — + # relying on Symfony to auto-resolve a singly-implemented interface does not work for plain + # autowiring (confirmed: omitting this alias fails container compilation outright, since + # NotifyPaymentRequestHandler's #[AsMessageHandler] service is always instantiated). + PayplugUnifiedCore\Contracts\IOrderStateMutator: '@PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator' + PayPlug\SyliusPayPlugPlugin\Repository\PaymentRepositoryInterface: class: PayPlug\SyliusPayPlugPlugin\Repository\PaymentRepository parent: sylius.repository.payment From 3ded48e6b1b45daf88c7fc08440910b11c83e75e Mon Sep 17 00:00:00 2001 From: Julien Hoarau Date: Tue, 21 Jul 2026 10:48:08 +0200 Subject: [PATCH 11/11] PRE-3469: Guard PayplugOrderStateMutator call against multi-payment orders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PayplugOrderStateMutator::apply() resolves the order's *last* payment internally (its contract is keyed by order ID, not payment ID). On an order with more than one payment (e.g. a failed attempt followed by a retry), that could be a different payment than the one this specific webhook is about — skip the additive call rather than risk transitioning the wrong payment. --- composer.json | 2 +- ...026-07-21-pre-3469-upc-contracts-rework.md | 1555 +++++++++++++++++ ...20-pre-3469-upc-contracts-rework-design.md | 65 + .../Handler/NotifyPaymentRequestHandler.php | 8 + src/Spike/VALIDATION.md | 74 +- .../NotifyPaymentRequestHandlerTest.php | 46 +- 6 files changed, 1734 insertions(+), 16 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-21-pre-3469-upc-contracts-rework.md create mode 100644 docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md diff --git a/composer.json b/composer.json index 1942f156..5e9f0232 100755 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "ext-json": "*", "giggsey/libphonenumber-for-php": "^8.12", "payplug/payplug-php": "^4.0", - "payplug/unified-plugin-core": "dev-develop", + "payplug/unified-plugin-core": "dev-master", "php-http/message-factory": "^1.1", "sylius/refund-plugin": "^2.0", "sylius/sylius": "^2.0", diff --git a/docs/superpowers/plans/2026-07-21-pre-3469-upc-contracts-rework.md b/docs/superpowers/plans/2026-07-21-pre-3469-upc-contracts-rework.md new file mode 100644 index 00000000..b228674d --- /dev/null +++ b/docs/superpowers/plans/2026-07-21-pre-3469-upc-contracts-rework.md @@ -0,0 +1,1555 @@ +# PRE-3469 UPC Contracts Rework Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Promote three of the four PRE-3469 spike contracts (`IOrderStateMutator`, `ITokenCache`, `IConfigurationRepository`) from isolated `src/Spike/` skeletons to real production classes — `PayplugOrderStateMutator`, `PayplugTokenCache`, `PayplugConfigurationRepository` — per the ticket's updated expected results, while keeping `IPaymentRepository` untouched and out of scope, and without regressing existing plugin behavior. + +**Architecture:** Move the three classes' existing, already-tested logic out of `src/Spike/` into real production namespaces (`src/PaymentProcessing/`, `src/TokenCache/`, `src/ConfigurationRepository/`), fixing `PayplugConfigurationRepository`'s public-key getters to default to an empty string instead of throwing. Wire `PayplugOrderStateMutator` additively (real call, `can()`-guarded, exception-swallowed) into `NotifyPaymentRequestHandler`. Promote `payplug/unified-plugin-core` from `require-dev` to `require` in `composer.json`, since a production class (`NotifyPaymentRequestHandler`, always loaded) will now reference its types — without this, a merchant's real `composer install --no-dev` would hit a fatal "interface not found" the moment the container tries to autowire the new dependency. + +**Tech Stack:** PHP 8.2, Symfony 7.3, Sylius 2.0 (Symfony Workflow via `Sylius\Abstraction\StateMachine\StateMachineInterface`), Doctrine ORM, PSR-6 cache (`Psr\Cache\CacheItemPoolInterface`, aliased by FrameworkBundle to `cache.app`), PHPUnit 10, `payplug/unified-plugin-core` (VCS dependency). + +## Global Constraints + +- PHP floor: 8.2 (per `composer.json` / ticket context). +- Sylius: `sylius/sylius` `^2.0`. +- `payplug/unified-plugin-core` stays pinned to `dev-develop` (VCS repository `https://github.com/payplug/unified-plugin-core.git`) — only its `require`/`require-dev` placement changes, not the version constraint. +- Existing PHPUnit suite (`vendor/bin/phpunit`, `phpunit.xml.dist` default run, which excludes `tests/PHPUnit/Spike/SpikeIntegrationTest.php`) must stay green after every task. +- No behavioral change to `PaymentTransitionApplier`, `PaymentStateResolver`, or `StatusPaymentRequestHandler` — this ticket does not unify or touch the pre-existing dual state-transition paths. +- `src/Spike/SyliusPaymentRepository.php`, `src/Spike/Entity/PayplugOperation.php`, and `migrations/Version20260720100000.php` are out of scope and must not be modified. +- Every new/moved file must pass the existing `ruleset/phpstan-baseline.neon`-gated static analysis without new baseline entries (fix real issues instead of suppressing them). + +--- + +## Task 1: Promote `payplug/unified-plugin-core` to a real production dependency + +**Files:** +- Modify: `composer.json` + +**Interfaces:** +- Produces: `payplug/unified-plugin-core` becomes resolvable under `composer install` (with or without `--no-dev`), so later tasks can safely reference `PayplugUnifiedCore\Contracts\IOrderStateMutator` etc. from always-loaded production classes. + +- [ ] **Step 1: Move the dependency from `require-dev` to `require`** + +In `composer.json`, remove this line from the `require-dev` block: + +```json +"payplug/unified-plugin-core": "dev-develop", +``` + +And add it to the `require` block (keeping the existing `payplug/payplug-php` entry as-is), so `require` reads: + +```json +"require": { + "payplug/payplug-php": "^4.0", + "payplug/unified-plugin-core": "dev-develop" +}, +``` + +- [ ] **Step 2: Validate the manifest** + +Run: `composer validate --no-check-publish --strict` +Expected: `./composer.json is valid` + +- [ ] **Step 3: Confirm the package still resolves** + +Run: `composer show payplug/unified-plugin-core` +Expected: prints the installed package info (version `dev-develop`, source URL `https://github.com/payplug/unified-plugin-core.git`) without error — confirms the existing local `vendor/` lock is still consistent with the manifest change (a full `composer install --no-dev` re-fetch requires GitHub access and is a manual follow-up for whoever runs CI/deploys this branch, not something to attempt in a sandboxed run). + +- [ ] **Step 4: Commit** + +```bash +git add composer.json +git commit -m "PRE-3469: Promote unified-plugin-core to a real dependency + +NotifyPaymentRequestHandler (task 5) will reference UPC contract types +directly; leaving the package require-dev-only would fatal-error the +container the moment a merchant's production composer install --no-dev +tries to autowire the new dependency." +``` + +--- + +## Task 2: Promote `SyliusOrderStateMutator` → `PayplugOrderStateMutator` + +**Files:** +- Create: `src/PaymentProcessing/PayplugOrderStateMutator.php` +- Create: `tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php` +- Delete: `src/Spike/SyliusOrderStateMutator.php` +- Delete: `tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php` +- Modify: `tests/PHPUnit/Spike/SpikeIntegrationTest.php` + +**Interfaces:** +- Consumes: `Sylius\Component\Core\Repository\OrderRepositoryInterface::findOrderById(string): ?OrderInterface`, `Sylius\Abstraction\StateMachine\StateMachineInterface::can()/apply()`, `Doctrine\ORM\EntityManagerInterface::flush()`. +- Produces: `PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator` implementing `PayplugUnifiedCore\Contracts\IOrderStateMutator`, constructor `(OrderRepositoryInterface $orderRepository, StateMachineInterface $stateMachine, EntityManagerInterface $entityManager)`, method `apply(string $orderId, string $outcome): void`. Task 5 depends on this exact class/constructor. + +- [ ] **Step 1: Write the new test file** + +Create `tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php`: + +```php +orderRepository = $this->createMock(OrderRepositoryInterface::class); + $this->stateMachine = $this->createMock(StateMachineInterface::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); + + $this->mutator = new PayplugOrderStateMutator($this->orderRepository, $this->stateMachine, $this->entityManager); + } + + /** + * THREE_DS_PENDING must stay a no-op — the order remains `new` until the async webhook + * resolves it. Neither the order repository nor the state machine should even be touched. + */ + public function testApply_threeDsPending_isNoOp(): void + { + $this->orderRepository->expects(self::never())->method('findOrderById'); + $this->stateMachine->expects(self::never())->method('apply'); + + $this->mutator->apply('order-1', PaymentOutcome::THREE_DS_PENDING); + } + + /** + * @dataProvider provideMappedOutcomes + */ + public function testApply_mappedOutcome_appliesExpectedTransition(string $outcome, string $expectedTransition): void + { + $order = $this->createMock(OrderInterface::class); + $payment = $this->createMock(PaymentInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->orderRepository->method('findOrderById')->with('order-1')->willReturn($order); + $this->stateMachine->method('can')->with($payment, PaymentTransitions::GRAPH, $expectedTransition)->willReturn(true); + $this->stateMachine->expects(self::once())->method('apply')->with($payment, PaymentTransitions::GRAPH, $expectedTransition); + $this->entityManager->expects(self::once())->method('flush'); + + $this->mutator->apply('order-1', $outcome); + } + + /** + * @return array + */ + public static function provideMappedOutcomes(): array + { + return [ + 'paid' => [PaymentOutcome::PAID, PaymentTransitions::TRANSITION_COMPLETE], + 'authorized' => [PaymentOutcome::AUTHORIZED, PaymentTransitions::TRANSITION_AUTHORIZE], + 'capture_required' => [PaymentOutcome::CAPTURE_REQUIRED, PaymentTransitions::TRANSITION_PROCESS], + 'refunded' => [PaymentOutcome::REFUNDED, PaymentTransitions::TRANSITION_REFUND], + 'failed' => [PaymentOutcome::FAILED, PaymentTransitions::TRANSITION_FAIL], + ]; + } + + /** + * A retried webhook landing on a payment already past this transition is a silent no-op, + * not a thrown workflow exception — mirrors the plugin's existing PaymentStateResolver. + */ + public function testApply_transitionNotAvailable_isSilentNoOp(): void + { + $order = $this->createMock(OrderInterface::class); + $payment = $this->createMock(PaymentInterface::class); + $order->method('getLastPayment')->willReturn($payment); + + $this->orderRepository->method('findOrderById')->willReturn($order); + $this->stateMachine->method('can')->willReturn(false); + $this->stateMachine->expects(self::never())->method('apply'); + $this->entityManager->expects(self::never())->method('flush'); + + $this->mutator->apply('order-1', PaymentOutcome::PAID); + } + + public function testApply_orderNotFound_throwsPaymentNotFoundException(): void + { + $this->orderRepository->method('findOrderById')->with('missing-order')->willReturn(null); + + $this->expectException(PaymentNotFoundException::class); + + $this->mutator->apply('missing-order', PaymentOutcome::PAID); + } + + public function testApply_orderHasNoPayment_throwsPaymentNotFoundException(): void + { + $order = $this->createMock(OrderInterface::class); + $order->method('getLastPayment')->willReturn(null); + $this->orderRepository->method('findOrderById')->willReturn($order); + + $this->expectException(PaymentNotFoundException::class); + + $this->mutator->apply('order-1', PaymentOutcome::PAID); + } + + public function testApply_unmappedOutcome_throwsInvalidArgumentException(): void + { + $this->orderRepository->expects(self::never())->method('findOrderById'); + + $this->expectException(\InvalidArgumentException::class); + + $this->mutator->apply('order-1', 'not_a_real_outcome'); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `vendor/bin/phpunit tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php` +Expected: FAIL — `Class "PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator" not found` + +- [ ] **Step 3: Create the production class** + +Create `src/PaymentProcessing/PayplugOrderStateMutator.php`: + +```php + + */ + private const OUTCOME_TO_TRANSITION = [ + PaymentOutcome::PAID => PaymentTransitions::TRANSITION_COMPLETE, + PaymentOutcome::AUTHORIZED => PaymentTransitions::TRANSITION_AUTHORIZE, + PaymentOutcome::CAPTURE_REQUIRED => PaymentTransitions::TRANSITION_PROCESS, + PaymentOutcome::REFUNDED => PaymentTransitions::TRANSITION_REFUND, + PaymentOutcome::FAILED => PaymentTransitions::TRANSITION_FAIL, + ]; + + public function __construct( + private readonly OrderRepositoryInterface $orderRepository, + private readonly StateMachineInterface $stateMachine, + private readonly EntityManagerInterface $entityManager, + ) { + } + + public function apply(string $orderId, string $outcome): void + { + if (PaymentOutcome::THREE_DS_PENDING === $outcome) { + return; + } + + $transition = self::OUTCOME_TO_TRANSITION[$outcome] ?? null; + if (null === $transition) { + throw new \InvalidArgumentException(\sprintf('No Symfony Workflow transition mapped for PaymentOutcome "%s".', $outcome)); + } + + $order = $this->orderRepository->findOrderById($orderId); + if (null === $order) { + throw new PaymentNotFoundException(\sprintf('No order found for id "%s".', $orderId)); + } + + $payment = $order->getLastPayment(); + if (null === $payment) { + throw new PaymentNotFoundException(\sprintf('Order "%s" has no payment to mutate.', $orderId)); + } + + // Guarded exactly like the plugin's existing PaymentStateResolver::applyTransition(): + // an out-of-order webhook retry landing on a payment already past this transition is a + // silent no-op rather than a thrown workflow exception. + if ($this->stateMachine->can($payment, PaymentTransitions::GRAPH, $transition)) { + $this->stateMachine->apply($payment, PaymentTransitions::GRAPH, $transition); + $this->entityManager->flush(); + } + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `vendor/bin/phpunit tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php` +Expected: `OK (7 tests, ...)` + +- [ ] **Step 5: Delete the promoted spike files** + +```bash +git rm src/Spike/SyliusOrderStateMutator.php tests/PHPUnit/Spike/SyliusOrderStateMutatorTest.php +``` + +- [ ] **Step 6: Update `SpikeIntegrationTest.php`'s two OrderStateMutator methods** + +In `tests/PHPUnit/Spike/SpikeIntegrationTest.php`, replace the import: + +```php +use PayPlug\SyliusPayPlugPlugin\Spike\SyliusOrderStateMutator; +``` + +with: + +```php +use PayPlug\SyliusPayPlugPlugin\PaymentProcessing\PayplugOrderStateMutator; +``` + +and replace both occurrences of `new SyliusOrderStateMutator(` with `new PayplugOrderStateMutator(` (in `testOrderStateMutator_realStateMachine_transitionsPaymentToCompleted` and `testOrderStateMutator_threeDsPending_leavesRealPaymentUntouched`). + +- [ ] **Step 7: Confirm no other references to the old class remain** + +Run: `grep -rn "SyliusOrderStateMutator" src/ tests/` +Expected: no output. + +- [ ] **Step 8: Run the full default suite** + +Run: `vendor/bin/phpunit` +Expected: `OK (...)` — no failures, no errors. + +- [ ] **Step 9: Commit** + +```bash +git add src/PaymentProcessing/PayplugOrderStateMutator.php tests/PHPUnit/PaymentProcessing/PayplugOrderStateMutatorTest.php tests/PHPUnit/Spike/SpikeIntegrationTest.php +git commit -m "PRE-3469: Promote SyliusOrderStateMutator to PayplugOrderStateMutator + +Moves the IOrderStateMutator skeleton out of src/Spike/ into a real +production namespace, ahead of wiring it into NotifyPaymentRequestHandler." +``` + +--- + +## Task 3: Promote `SyliusTokenCache` → `PayplugTokenCache` + +**Files:** +- Create: `src/TokenCache/PayplugTokenCache.php` +- Create: `tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php` +- Delete: `src/Spike/SyliusTokenCache.php` +- Delete: `tests/PHPUnit/Spike/SyliusTokenCacheTest.php` +- Modify: `tests/PHPUnit/Spike/SpikeIntegrationTest.php` + +**Interfaces:** +- Consumes: `Psr\Cache\CacheItemPoolInterface::getItem()/save()/deleteItem()` (autowireable via FrameworkBundle's `Psr\Cache\CacheItemPoolInterface: '@cache.app'` alias — confirmed in `vendor/symfony/framework-bundle/Resources/config/cache.php`). +- Produces: `PayPlug\SyliusPayPlugPlugin\TokenCache\PayplugTokenCache` implementing `PayplugUnifiedCore\Contracts\ITokenCache`, constructor `(CacheItemPoolInterface $cache)`. + +- [ ] **Step 1: Write the new test file** + +Create `tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php`: + +```php +cache = $this->createMock(CacheItemPoolInterface::class); + $this->tokenCache = new PayplugTokenCache($this->cache); + } + + public function testGet_hit_returnsStoredValue(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(true); + $item->method('get')->willReturn('jwt-value'); + $this->cache->method('getItem')->with('token-key')->willReturn($item); + + self::assertSame('jwt-value', $this->tokenCache->get('token-key')); + } + + public function testGet_miss_returnsNull(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(false); + $this->cache->method('getItem')->willReturn($item); + + self::assertNull($this->tokenCache->get('token-key')); + } + + /** + * PSR-6's CacheItemInterface::get() is typed mixed — defend against a non-string value + * ever having been stored under this key (e.g. by another caller of the same pool). + */ + public function testGet_hitWithNonStringValue_returnsNull(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->method('isHit')->willReturn(true); + $item->method('get')->willReturn(42); + $this->cache->method('getItem')->willReturn($item); + + self::assertNull($this->tokenCache->get('token-key')); + } + + public function testSet_storesValueWithTtlAndSaves(): void + { + $item = $this->createMock(CacheItemInterface::class); + $item->expects(self::once())->method('set')->with('jwt-value'); + $item->expects(self::once())->method('expiresAfter')->with(298); + $this->cache->method('getItem')->with('token-key')->willReturn($item); + $this->cache->expects(self::once())->method('save')->with($item); + + $this->tokenCache->set('token-key', 'jwt-value', 298); + } + + public function testDelete_removesItem(): void + { + $this->cache->expects(self::once())->method('deleteItem')->with('token-key'); + + $this->tokenCache->delete('token-key'); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `vendor/bin/phpunit tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php` +Expected: FAIL — `Class "PayPlug\SyliusPayPlugPlugin\TokenCache\PayplugTokenCache" not found` + +- [ ] **Step 3: Create the production class** + +Create `src/TokenCache/PayplugTokenCache.php`: + +```php + cache.app alias) already satisfies + * CacheItemPoolInterface, regardless of whether that pool is backed by the filesystem, APCu, or + * Redis in a given deployment. This class is validated by a real-infrastructure integration + * test (see SpikeIntegrationTest) rather than wired into a live request path: the one existing + * production caller of an OAuth token cache, PayPlugApiClientFactory::getTokenForGatewayConfig(), + * gates authentication for every PayPlug gateway — replacing its inline caching logic here would + * carry more regression risk than this ticket's validation goal justifies. + */ +final class PayplugTokenCache implements ITokenCache +{ + public function __construct(private readonly CacheItemPoolInterface $cache) + { + } + + public function get(string $key): ?string + { + $item = $this->cache->getItem($key); + if (!$item->isHit()) { + return null; + } + + $value = $item->get(); + + return \is_string($value) ? $value : null; + } + + public function set(string $key, string $value, int $ttlSeconds): void + { + $item = $this->cache->getItem($key); + $item->set($value); + $item->expiresAfter($ttlSeconds); + $this->cache->save($item); + } + + public function delete(string $key): void + { + $this->cache->deleteItem($key); + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `vendor/bin/phpunit tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php` +Expected: `OK (5 tests, ...)` + +- [ ] **Step 5: Delete the promoted spike files** + +```bash +git rm src/Spike/SyliusTokenCache.php tests/PHPUnit/Spike/SyliusTokenCacheTest.php +``` + +- [ ] **Step 6: Update `SpikeIntegrationTest.php`'s TokenCache method** + +In `tests/PHPUnit/Spike/SpikeIntegrationTest.php`, replace the import: + +```php +use PayPlug\SyliusPayPlugPlugin\Spike\SyliusTokenCache; +``` + +with: + +```php +use PayPlug\SyliusPayPlugPlugin\TokenCache\PayplugTokenCache; +``` + +and in `testTokenCache_realCachePool_roundTripsThroughRealAdapter`, replace `new SyliusTokenCache($cachePool)` with `new PayplugTokenCache($cachePool)`. + +- [ ] **Step 7: Confirm no other references to the old class remain** + +Run: `grep -rn "SyliusTokenCache" src/ tests/` +Expected: no output. + +- [ ] **Step 8: Run the full default suite** + +Run: `vendor/bin/phpunit` +Expected: `OK (...)` — no failures, no errors. + +- [ ] **Step 9: Commit** + +```bash +git add src/TokenCache/PayplugTokenCache.php tests/PHPUnit/TokenCache/PayplugTokenCacheTest.php tests/PHPUnit/Spike/SpikeIntegrationTest.php +git commit -m "PRE-3469: Promote SyliusTokenCache to PayplugTokenCache + +Moves the ITokenCache skeleton out of src/Spike/ into a real production +namespace. Stays validated by a real-infrastructure integration test only +(see SpikeIntegrationTest) — no production call site, since its one real +analog (PayPlugApiClientFactory's OAuth token cache) gates authentication +for every gateway and isn't worth the regression risk of replacing here." +``` + +--- + +## Task 4: Promote `SyliusConfigurationRepository` → `PayplugConfigurationRepository`, fix public-key getters + +**Files:** +- Create: `src/ConfigurationRepository/PayplugConfigurationRepository.php` +- Create: `tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php` +- Delete: `src/Spike/SyliusConfigurationRepository.php` +- Delete: `tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php` +- Modify: `tests/PHPUnit/Spike/SpikeIntegrationTest.php` + +**Interfaces:** +- Consumes: `Sylius\Component\Payment\Model\GatewayConfigInterface::getConfig()/setConfig()/getFactoryName()`. +- Produces: `PayPlug\SyliusPayPlugPlugin\ConfigurationRepository\PayplugConfigurationRepository` implementing `PayplugUnifiedCore\Contracts\IConfigurationRepository`, constructor `(GatewayConfigInterface $gatewayConfig)`. `getClientId()`/`getClientSecret()` throw `PayplugUnifiedCore\Exceptions\ApiException` when missing (unchanged from the spike). `getPublicKeyId()`/`getPublicKeyValue()` now return `''` when missing instead of throwing — no production writer for these two keys exists yet. + +- [ ] **Step 1: Write the new test file** + +Create `tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php`: + +```php +gatewayConfig = $this->createMock(GatewayConfigInterface::class); + } + + public function testGet_liveMode_readsFromLiveClientScope(): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => ['client_id' => 'live-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + self::assertSame('live-id', $this->repository()->get('client_id')); + } + + public function testGet_testMode_readsFromTestClientScope(): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => false, + 'live_client' => ['client_id' => 'live-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + self::assertSame('test-id', $this->repository()->get('client_id')); + } + + public function testGet_missingKey_returnsNull(): void + { + $this->gatewayConfig->method('getConfig')->willReturn(['live' => true, 'live_client' => []]); + + self::assertNull($this->repository()->get('client_id')); + } + + public function testSet_writesIntoActiveScopeAndPersistsWholeConfig(): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => ['client_id' => 'old-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + $this->gatewayConfig->expects(self::once())->method('setConfig')->with([ + 'live' => true, + 'live_client' => ['client_id' => 'new-id'], + 'test_client' => ['client_id' => 'test-id'], + ]); + + $this->repository()->set('client_id', 'new-id'); + } + + /** + * @dataProvider provideRequiredCredentialGetters + */ + public function testRequiredCredentialGetters_present_returnValue(string $method, string $configKey): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => [$configKey => 'the-value'], + ]); + + self::assertSame('the-value', $this->repository()->{$method}()); + } + + /** + * @dataProvider provideRequiredCredentialGetters + * + * The exception message must name the missing key and the factory, never a credential + * value — there is nothing sensitive to redact here since the value is simply absent, but + * this locks the message shape so a future edit can't start interpolating $value instead. + */ + public function testRequiredCredentialGetters_missing_throwsApiExceptionWithoutLeakingSecrets( + string $method, + string $configKey, + ): void + { + $this->gatewayConfig->method('getConfig')->willReturn(['live' => true, 'live_client' => []]); + $this->gatewayConfig->method('getFactoryName')->willReturn('payplug'); + + try { + $this->repository()->{$method}(); + self::fail('Expected ApiException to be thrown.'); + } catch (ApiException $exception) { + self::assertSame(\sprintf('Missing "%s" in gateway configuration "payplug".', $configKey), $exception->getMessage()); + } + } + + /** + * @return array + */ + public static function provideRequiredCredentialGetters(): array + { + return [ + 'clientId' => ['getClientId', 'client_id'], + 'clientSecret' => ['getClientSecret', 'client_secret'], + ]; + } + + /** + * @dataProvider provideOptionalCredentialGetters + */ + public function testOptionalCredentialGetters_present_returnValue(string $method, string $configKey): void + { + $this->gatewayConfig->method('getConfig')->willReturn([ + 'live' => true, + 'live_client' => [$configKey => 'the-value'], + ]); + + self::assertSame('the-value', $this->repository()->{$method}()); + } + + /** + * getPublicKeyId()/getPublicKeyValue() have no production writer yet (Hosted Fields isn't + * built) — unlike client_id/client_secret, a missing value must not throw, or the contract + * would be unimplementable until a future ticket adds that writer. + * + * @dataProvider provideOptionalCredentialGetters + */ + public function testOptionalCredentialGetters_missing_returnEmptyString(string $method): void + { + $this->gatewayConfig->method('getConfig')->willReturn(['live' => true, 'live_client' => []]); + + self::assertSame('', $this->repository()->{$method}()); + } + + /** + * @return array + */ + public static function provideOptionalCredentialGetters(): array + { + return [ + 'publicKeyId' => ['getPublicKeyId', 'public_key_id'], + 'publicKeyValue' => ['getPublicKeyValue', 'public_key_value'], + ]; + } + + private function repository(): PayplugConfigurationRepository + { + return new PayplugConfigurationRepository($this->gatewayConfig); + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `vendor/bin/phpunit tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php` +Expected: FAIL — `Class "PayPlug\SyliusPayPlugPlugin\ConfigurationRepository\PayplugConfigurationRepository" not found` + +- [ ] **Step 3: Create the production class** + +Create `src/ConfigurationRepository/PayplugConfigurationRepository.php`: + +```php +activeClientConfig(); + $value = $client[$key] ?? null; + + return \is_string($value) ? $value : null; + } + + public function set(string $key, string $value): void + { + $config = $this->gatewayConfig->getConfig(); + $scope = $this->activeScope($config); + $client = $config[$scope] ?? []; + if (!\is_array($client)) { + $client = []; + } + $client[$key] = $value; + $config[$scope] = $client; + + // Persisting $config is the caller's responsibility (Doctrine flush), same as every + // other GatewayConfigInterface mutation in this plugin (see UnifiedAuthenticationController). + $this->gatewayConfig->setConfig($config); + } + + public function getClientId(): string + { + return $this->requireString('client_id'); + } + + public function getClientSecret(): string + { + return $this->requireString('client_secret'); + } + + public function getPublicKeyId(): string + { + return $this->get('public_key_id') ?? ''; + } + + public function getPublicKeyValue(): string + { + return $this->get('public_key_value') ?? ''; + } + + private function requireString(string $key): string + { + $value = $this->get($key); + if (null === $value || '' === $value) { + // Never interpolate the resolved *value* here, only the key name and factory name. + throw new ApiException(\sprintf( + 'Missing "%s" in gateway configuration "%s".', + $key, + $this->gatewayConfig->getFactoryName() ?? 'unknown', + )); + } + + return $value; + } + + /** + * @return array + */ + private function activeClientConfig(): array + { + $config = $this->gatewayConfig->getConfig(); + $client = $config[$this->activeScope($config)] ?? []; + if (!\is_array($client)) { + return []; + } + + /** @var array $typedClient */ + $typedClient = $client; + + return $typedClient; + } + + /** + * @param array $config + */ + private function activeScope(array $config): string + { + return true === ($config['live'] ?? false) ? 'live_client' : 'test_client'; + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `vendor/bin/phpunit tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php` +Expected: `OK (9 tests, ...)` + +- [ ] **Step 5: Delete the promoted spike files** + +```bash +git rm src/Spike/SyliusConfigurationRepository.php tests/PHPUnit/Spike/SyliusConfigurationRepositoryTest.php +``` + +- [ ] **Step 6: Update `SpikeIntegrationTest.php`'s ConfigurationRepository method** + +In `tests/PHPUnit/Spike/SpikeIntegrationTest.php`, replace the import: + +```php +use PayPlug\SyliusPayPlugPlugin\Spike\SyliusConfigurationRepository; +``` + +with: + +```php +use PayPlug\SyliusPayPlugPlugin\ConfigurationRepository\PayplugConfigurationRepository; +``` + +and in `testConfigurationRepository_realGatewayConfig_survivesADoctrineRoundTrip`, replace both occurrences of `new SyliusConfigurationRepository(` with `new PayplugConfigurationRepository(`. + +- [ ] **Step 7: Confirm no other references to the old class remain** + +Run: `grep -rn "SyliusConfigurationRepository" src/ tests/` +Expected: no output. + +- [ ] **Step 8: Run the full default suite** + +Run: `vendor/bin/phpunit` +Expected: `OK (...)` — no failures, no errors. + +- [ ] **Step 9: Commit** + +```bash +git add src/ConfigurationRepository/PayplugConfigurationRepository.php tests/PHPUnit/ConfigurationRepository/PayplugConfigurationRepositoryTest.php tests/PHPUnit/Spike/SpikeIntegrationTest.php +git commit -m "PRE-3469: Promote SyliusConfigurationRepository to PayplugConfigurationRepository + +Moves the IConfigurationRepository skeleton out of src/Spike/ into a real +production namespace. getPublicKeyId()/getPublicKeyValue() now default to +an empty string instead of throwing, since no production code writes +public_key_id/public_key_value yet (Hosted Fields isn't built) — documented +as friction rather than blocked on." +``` + +--- + +## Task 5: Wire `PayplugOrderStateMutator` into `NotifyPaymentRequestHandler` + +**Files:** +- Modify: `src/Command/Handler/NotifyPaymentRequestHandler.php` +- Create: `tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php` + +**Interfaces:** +- Consumes: `PayplugUnifiedCore\Contracts\IOrderStateMutator::apply(string, string): void` — depend on this interface, NOT the concrete `PayplugOrderStateMutator` class. `PayplugOrderStateMutator` (Task 2) is declared `final`, which PHPUnit cannot mock by subclassing; depending on the interface (its only implementation in this codebase) sidesteps that entirely and is autowireable identically. Also uses `PayPlug\SyliusPayPlugPlugin\ApiClient\PayPlugApiClientInterface::STATUS_CAPTURED/STATUS_AUTHORIZED/FAILED` (existing constants) and `PayplugUnifiedCore\Models\PaymentOutcome::PAID/AUTHORIZED/FAILED`. +- Produces: no new public interface — this is the real production call site the ticket requires. + +- [ ] **Step 1: Write the failing tests** + +Create `tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php`: + +```php +paymentRequestProvider = $this->createMock(PaymentRequestProviderInterface::class); + $this->stateMachine = $this->createMock(StateMachineInterface::class); + $this->apiClientFactory = $this->createMock(PayPlugApiClientFactoryInterface::class); + $this->paymentNotificationHandler = $this->createMock(PaymentNotificationHandler::class); + $this->refundNotificationHandler = $this->createMock(RefundNotificationHandler::class); + $this->paymentTransitionApplier = $this->createMock(PaymentTransitionApplier::class); + $this->orderStateMutator = $this->createMock(IOrderStateMutator::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->handler = new NotifyPaymentRequestHandler( + $this->paymentRequestProvider, + $this->stateMachine, + $this->apiClientFactory, + $this->paymentNotificationHandler, + $this->refundNotificationHandler, + $this->paymentTransitionApplier, + $this->orderStateMutator, + $this->logger, + ); + } + + public function testInvoke_knownStatus_callsOrderStateMutatorWithMappedOutcome(): void + { + $this->prepareNormalFlow(PayPlugApiClientInterface::STATUS_CAPTURED, 42); + + $this->orderStateMutator->expects(self::once())->method('apply')->with('42', PaymentOutcome::PAID); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_unknownStatus_doesNotCallOrderStateMutator(): void + { + $this->prepareNormalFlow(PayPlugApiClientInterface::STATUS_CANCELED, 42); + + $this->orderStateMutator->expects(self::never())->method('apply'); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_orderStateMutatorThrows_isCaughtAndDoesNotPreventCompletion(): void + { + $this->prepareNormalFlow(PayPlugApiClientInterface::STATUS_CAPTURED, 42); + + $this->orderStateMutator->method('apply')->willThrowException(new \RuntimeException('boom')); + $this->logger->expects(self::once())->method('warning'); + + // The outer flow must still complete normally: TRANSITION_COMPLETE, not TRANSITION_FAIL. + $this->stateMachine->expects(self::once())->method('apply')->with( + self::isInstanceOf(PaymentRequestInterface::class), + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_COMPLETE, + ); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_paymentAlreadyCompleted_shortCircuitsAndSkipsOrderStateMutator(): void + { + $payment = $this->createMock(PaymentInterface::class); + $payment->method('getState')->willReturn(PaymentInterface::STATE_COMPLETED); + $method = $this->createMock(PaymentMethodInterface::class); + $payment->method('getMethod')->willReturn($method); + + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '{}']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $client = $this->createMock(PayPlugApiClientInterface::class); + $resource = $this->createMock(PayplugResourcePayment::class); + $client->method('treat')->willReturn($resource); + $this->apiClientFactory->method('createForPaymentMethod')->willReturn($client); + + $this->paymentTransitionApplier->expects(self::never())->method('apply'); + $this->orderStateMutator->expects(self::never())->method('apply'); + $this->stateMachine->expects(self::once())->method('apply')->with( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_COMPLETE, + ); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + public function testInvoke_invalidPayload_failsWithoutCallingOrderStateMutator(): void + { + $payment = $this->createMock(PaymentInterface::class); + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $paymentRequest->expects(self::once())->method('setResponseData'); + $this->orderStateMutator->expects(self::never())->method('apply'); + $this->stateMachine->expects(self::once())->method('apply')->with( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_FAIL, + ); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + private function prepareNormalFlow(string $status, int $orderId): void + { + $order = $this->createMock(OrderInterface::class); + $order->method('getId')->willReturn($orderId); + + $payment = $this->createMock(PaymentInterface::class); + $payment->method('getState')->willReturn(PaymentInterface::STATE_NEW); + $payment->method('getDetails')->willReturn(['status' => $status]); + $payment->method('getOrder')->willReturn($order); + $method = $this->createMock(PaymentMethodInterface::class); + $payment->method('getMethod')->willReturn($method); + + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '{}']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $client = $this->createMock(PayPlugApiClientInterface::class); + $resource = $this->createMock(PayplugResourcePayment::class); + $client->method('treat')->willReturn($resource); + $this->apiClientFactory->method('createForPaymentMethod')->willReturn($client); + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `vendor/bin/phpunit tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php` +Expected: FAIL — constructor call error (`Too many arguments` / `Unknown named parameter`), since `NotifyPaymentRequestHandler` doesn't yet accept `PayplugOrderStateMutator`/`LoggerInterface`. + +- [ ] **Step 3: Modify `NotifyPaymentRequestHandler`** + +Replace the full contents of `src/Command/Handler/NotifyPaymentRequestHandler.php`: + +```php + + */ + private const STATUS_TO_OUTCOME = [ + PayPlugApiClientInterface::STATUS_CAPTURED => PaymentOutcome::PAID, + PayPlugApiClientInterface::STATUS_AUTHORIZED => PaymentOutcome::AUTHORIZED, + PayPlugApiClientInterface::FAILED => PaymentOutcome::FAILED, + ]; + + public function __construct( + private PaymentRequestProviderInterface $paymentRequestProvider, + private StateMachineInterface $stateMachine, + private PayPlugApiClientFactoryInterface $apiClientFactory, + private PaymentNotificationHandler $paymentNotificationHandler, + private RefundNotificationHandler $refundNotificationHandler, + private PaymentTransitionApplier $paymentTransitionApplier, + private IOrderStateMutator $orderStateMutator, + private LoggerInterface $logger, + ) {} + + public function __invoke(NotifyPaymentRequest $notifyPaymentRequest): void + { + $paymentRequest = $this->paymentRequestProvider->provide($notifyPaymentRequest); + /** @var PaymentInterface $payment */ + $payment = $paymentRequest->getPayment(); + + try { + $payload = $paymentRequest->getPayload(); + $content = $payload['http_request']['content'] ?? null; // @phpstan-ignore-line + if (!is_string($content) || '' === $content) { + throw new \LogicException('Invalid PayPlug notification payload.'); + } + + $method = $payment->getMethod(); + if (null === $method) { + throw new \LogicException('Payment method is not set for the payment.'); + } + + $client = $this->apiClientFactory->createForPaymentMethod($method); + $resource = $client->treat($content); + + if ($resource instanceof Payment && $payment->getState() === PaymentInterface::STATE_COMPLETED) { + // If the payment is already completed, we do not need to update it again + $this->stateMachine->apply( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_COMPLETE, + ); + + return; + } + + $details = new \ArrayObject($payment->getDetails()); + $this->paymentNotificationHandler->treat($payment, $resource, $details); + $this->refundNotificationHandler->treat($payment, $resource, $details); + + $payment->setDetails($details->getArrayCopy()); + if ($resource instanceof Payment) { + $this->paymentTransitionApplier->apply($payment); + $this->applyOrderStateMutator($payment); + } + + $this->stateMachine->apply( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_COMPLETE, + ); + } catch (\Throwable $e) { + $paymentRequest->setResponseData([ + 'error' => $e->getMessage(), + ]); + $this->stateMachine->apply( + $paymentRequest, + PaymentRequestTransitions::GRAPH, + PaymentRequestTransitions::TRANSITION_FAIL, + ); + } + } + + /** + * PRE-3469: additive real call site for PayplugOrderStateMutator. PaymentTransitionApplier + * has already applied the real transition above by the time this runs, so the mutator's own + * can()-guard makes this a no-op in the normal case — this exists to prove the contract works + * against a live webhook event, not to change behavior. Any failure here is caught and + * logged, never allowed to affect the primary notification flow above. + */ + private function applyOrderStateMutator(PaymentInterface $payment): void + { + $details = $payment->getDetails(); // @phpstan-ignore-line - getDetails() return mixed + $status = $details['status'] ?? ''; + $outcome = self::STATUS_TO_OUTCOME[$status] ?? null; + if (null === $outcome) { + return; + } + + $order = $payment->getOrder(); + if (null === $order) { + return; + } + + try { + $this->orderStateMutator->apply((string) $order->getId(), $outcome); + } catch (\Throwable $e) { + $this->logger->warning('[PayPlug] PayplugOrderStateMutator additive call failed.', [ + 'sylius_payment_id' => $payment->getId(), + 'outcome' => $outcome, + 'exception' => $e->getMessage(), + ]); + } + } +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `vendor/bin/phpunit tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php` +Expected: `OK (5 tests, ...)` + +- [ ] **Step 5: Run the full default suite** + +Run: `vendor/bin/phpunit` +Expected: `OK (...)` — no failures, no errors. + +- [ ] **Step 6: Commit** + +```bash +git add src/Command/Handler/NotifyPaymentRequestHandler.php tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php +git commit -m "PRE-3469: Wire PayplugOrderStateMutator into NotifyPaymentRequestHandler + +Additive real call site: runs after the existing PaymentTransitionApplier +transition, guarded by the mutator's own can() check (safe no-op once the +transition's already applied) and wrapped in try/catch so any failure is +logged, never allowed to break webhook notification handling." +``` + +--- + +## Task 6: Update `src/Spike/VALIDATION.md` + +**Files:** +- Modify: `src/Spike/VALIDATION.md` + +**Interfaces:** +- None (documentation only). + +- [ ] **Step 1: Rewrite the document to reflect the promoted contracts** + +Replace the full contents of `src/Spike/VALIDATION.md`: + +```markdown +# PRE-3469 — Validation des contrats UPC contre Sylius + +`IOrderStateMutator`, `ITokenCache` et `IConfigurationRepository` ont été promus en implémentations +réelles, branchées dans le code de production existant (voir `src/PaymentProcessing/PayplugOrderStateMutator.php`, +`src/TokenCache/PayplugTokenCache.php`, `src/ConfigurationRepository/PayplugConfigurationRepository.php`). +`IPaymentRepository` reste hors scope de ce ticket (lié à un Value Object fortement couplé à +l'API Unifiée, non encore branché sur le plugin) — son squelette de preuve reste dans ce dossier, +voir `SyliusPaymentRepository.php`. + +Couverture de tests, 3 niveaux : + +- **Unitaire** (`tests/PHPUnit/{PaymentProcessing,TokenCache,ConfigurationRepository}/*Test.php` + + `tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php`) : mocks natifs PHPUnit + sur chaque collaborateur. +- **Intégration réelle** (`SpikeIntegrationTest.php`, toujours dans ce dossier car c'est le seul + endroit du plugin qui boote un vrai kernel Sylius pour les tests) : kernel Sylius réellement + booté, vraie base MariaDB jetable, vrai state machine Symfony Workflow, vrai pool de cache + PSR-6, vraie persistance Doctrine — aucun mock. Couvre les 4 contrats, y compris + `IPaymentRepository`. +- Les deux suites tournent avec `composer install`, `vendor/bin/phpunit` une fois la base de + test créée — voir section Câblage. + +**Verdict global : aucune friction bloquante restante.** Une friction bloquante a été trouvée et +corrigée pendant ce rework (voir « Friction bloquante corrigée » ci-dessous), plus deux notes non +bloquantes (aucune ne remet en cause la forme des interfaces). + +## Friction bloquante corrigée : `payplug/unified-plugin-core` en `require-dev` + +`SyliusOrderStateMutator`/`SyliusTokenCache`/`SyliusConfigurationRepository` (les squelettes +d'origine) implémentaient directement les interfaces UPC depuis `src/Spike/`, avec +`payplug/unified-plugin-core` en `require-dev` uniquement. Tant que ce code restait isolé et +jamais référencé par une classe de production, ça ne posait pas de problème. Mais brancher +`PayplugOrderStateMutator` dans `NotifyPaymentRequestHandler` — une classe systématiquement +chargée, y compris en production — change la donne : un vrai `composer install --no-dev` chez un +marchand n'installerait pas `unified-plugin-core`, et charger une classe qui `implements` une +interface inexistante est une erreur PHP fatale, pas un échec silencieux. + +**Fix** : `payplug/unified-plugin-core` est passé en dépendance `require` (toujours pinné sur +`dev-develop`, même repository VCS). Documenté ici plutôt que traité comme un simple détail de +`composer.json`, parce que ça anticipe une partie de ce que PRE-3563 (la vraie dépendance de +production pour l'OAuth) devait poser — voir section Câblage plus bas pour ce qui reste à faire +pour PRE-3563. + +## IOrderStateMutator — réel : `PayplugOrderStateMutator.php` + +Mapping validé : `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → +`PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL` sur le graphe +`sylius_payment`, via `StateMachineInterface::can()/apply()` — exactement le pattern déjà utilisé +par `PaymentStateResolver::applyTransition()` dans le plugin actuel. + +`THREE_DS_PENDING` confirmé sans transition : l'implémentation retourne immédiatement sans +toucher au state machine, la commande reste `new` jusqu'au webhook — conforme au résultat attendu +du ticket. + +**Branché en production** : appel additif dans `NotifyPaymentRequestHandler`, juste après le +`PaymentTransitionApplier::apply($payment)` existant, une fois la transition déjà appliquée par +ce dernier. Le statut PayPlug déjà connu à cet endroit (`$payment->getDetails()['status']`) est +traduit en `PaymentOutcome` (seuls `STATUS_CAPTURED`/`STATUS_AUTHORIZED`/`FAILED` ont un +équivalent — `STATUS_ABORTED`/`STATUS_CANCELED*`, mappés par `PaymentTransitionApplier` sur +`TRANSITION_CANCEL`, n'ont pas d'équivalent `PaymentOutcome` et sont donc volontairement ignorés, +pas forcés). L'appel est protégé par un `try/catch` qui journalise sans jamais propager : la +transition ayant déjà été appliquée par `PaymentTransitionApplier`, le garde-fou `can()` du +mutateur en fait un no-op silencieux dans le cas normal ; ce câblage prouve que le contrat +fonctionne contre un webhook réel sans remplacer ni risquer le flux existant. + +**Note (non bloquante)** : le contrat prend un `orderId`, mais la transition Symfony Workflow vit +sur le sous-objet `Payment` de la commande (`Order::getLastPayment()`), pas sur l'`Order` +lui-même. Un hop supplémentaire Order → Payment est donc nécessaire côté adaptateur Sylius — ce +que WooCommerce n'aurait pas besoin de faire. C'est le bon endroit pour cette différence (dans +l'adaptateur, pas dans le contrat CMS-agnostique). + +**Note annexe (mécanique, pas conceptuelle)** : le plugin a aujourd'hui deux chemins de +production distincts qui appliquent des transitions Symfony Workflow sur un paiement — +`PaymentTransitionApplier` (webhooks/status, celui utilisé ci-dessus) et `PaymentStateResolver` +(réconciliation CLI, `payplug:update-payment-state`). Ce ticket ne les unifie pas ; ça reste deux +implémentations parallèles du même idiome `can()`/`apply()`. + +## ITokenCache — réel : `PayplugTokenCache.php` + +Aucune friction. `get`/`set`/`delete` se posent 1:1 sur `Psr\Cache\CacheItemPoolInterface::getItem/ +save/deleteItem`, exactement comme le docblock de l'interface le prévoyait déjà. Le pool par +défaut de Sylius (`cache.app`, `Symfony\Component\Cache\Adapter\AdapterInterface`) satisfait déjà +`CacheItemPoolInterface`, aliasé nativement par FrameworkBundle — filesystem, APCu ou Redis selon +le déploiement, sans changement de code côté `PayplugTokenCache`. + +**Cible confirmée par le ticket** : `ITokenCache` cible le cache du token JWT OAuth +(authentification via le SDK PayPlug), pas le stockage carte/one-click — la sauvegarde de carte +est une entité Doctrine permanente (`Card`), sans aucun cache impliqué. + +**Validé par un test d'intégration réel, pas par un appel de production** : le seul analogue de +production existant, `PayPlugApiClientFactory::getTokenForGatewayConfig()`, met en cache le token +OAuth qui conditionne l'authentification de *toutes* les gateways du plugin. Remplacer sa logique +de cache inline aurait un risque de régression disproportionné par rapport à l'objectif de +validation de ce ticket ; `PayplugTokenCache` reste donc validé par un test d'intégration contre +un vrai pool PSR-6 (`SpikeIntegrationTest::testTokenCache_realCachePool_roundTripsThroughRealAdapter`), +sans point d'entrée de production. + +## IConfigurationRepository — réel : `PayplugConfigurationRepository.php` + +`GatewayConfigInterface::getConfig()` scope les credentials par `PaymentMethod` *et* par mode +live/test (`config['live_client']` vs `config['test_client']`, sélectionné par `config['live']` — +même pattern que `PayPlugApiClientFactory::getTokenForGatewayConfig()` existant). Une instance de +`PayplugConfigurationRepository` doit donc être construite par `GatewayConfigInterface` (donc par +`PaymentMethod`), pas partagée comme service unique — c'est une factory, pas un singleton. Ne +remet pas en cause le contrat, mais à garder en tête si le futur client Unified API suppose "un +repository = un marchand". + +**Point positif** : Sylius expose déjà un `GatewayConfigEncrypter` (expérimental) qui chiffre au +repos l'intégralité du tableau `getConfig()` — si branché, `CLIENT_SECRET` en bénéficie +gratuitement. Ce que `PayplugConfigurationRepository` doit garantir lui-même, c'est qu'un secret +déchiffré ne fuite jamais dans un message de log ou d'exception : `requireString()` n'interpole +jamais que le nom de la clé, jamais sa valeur. + +**Friction non bloquante, résolue par un choix explicite** : `getPublicKeyId()`/ +`getPublicKeyValue()` n'ont aucun équivalent de production — le plugin n'a aucune notion de clé +publique Hosted Fields aujourd'hui (`grep` sur `public_key`/`PUBLIC_KEY` dans `src/` ne retourne +rien en dehors de ce fichier). Plutôt que de lever une exception comme `getClientId()`/ +`getClientSecret()`, ces deux méthodes renvoient une chaîne vide tant qu'aucun code de production +n'écrit ces clés — le contrat reste implémentable de bout en bout, prêt pour quand Hosted Fields +sera construit. + +## IPaymentRepository — hors scope, squelette inchangé : `SyliusPaymentRepository.php` + `Entity/PayplugOperation.php` + +Hors scope de ce ticket (lié à un Value Object fortement couplé à l'API Unifiée, non encore +branché sur le plugin) — reste un squelette de preuve, inchangé depuis la version précédente de +ce document. + +**Constat principal du ticket, confirmé** : le plugin actuel ne stocke pas `OperationData` de +façon normalisée. L'id Payplug est aujourd'hui écrit dans le JSON de `Payment::details` et +retrouvé via `LIKE '%id%'` (`PaymentRepository::findOneByPayPlugPaymentId`). Ça fonctionne pour ce +seul lookup, mais ne peut pas porter `markTreated()`/`isTreated()` (il faut un flag d'idempotence +indexé) ni `getByOperationId()` proprement (il faut une colonne indexée, pas une recherche de +sous-chaîne dans un blob sérialisé). + +Le squelette introduit donc une nouvelle table `payplug_operation`, sans aucune dépendance au SDK +`payplug/payplug-php` — juste `OperationData` en entrée/sortie. C'est un vrai changement de schéma +(nouvelle table), pas une extension de l'existant — à traiter comme tel dans le chiffrage du futur +ticket de production. + +**Note annexe (mécanique, pas conceptuelle)** : `PayplugOperation` vit hors de `src/Entity/` et +n'est pas un `sylius_resource` dans `config/resources.yaml` (contrairement à `Card` et +`RefundHistory`) — enregistrer un `sylius_resource` a été essayé pour le test d'intégration mais +échoue tant que la classe n'implémente pas `Sylius\Resource\Model\ResourceInterface` (grilles/ +formulaires/routes Sylius, inutiles pour cette entité de pure persistance). Le mapping Doctrine +réel utilisé pour le test d'intégration passe donc par un `doctrine.orm.mappings` prepend dans +`PayPlugSyliusPayPlugExtension::prependSpikeDoctrineMapping()` — méthode explicitement marquée +PRE-3469-only, restreinte à `kernel.environment === 'test'` (jamais en prod), à retirer avec +`src/Spike/` si le spike est abandonné. + +## Câblage (pour rejouer les tests) + +- `composer.json` : `repositories` avec un repository `vcs` vers + `https://github.com/payplug/unified-plugin-core.git` + `payplug/unified-plugin-core: + "dev-develop"` désormais en dépendance `require` (voir « Friction bloquante corrigée » + ci-dessus) — fonctionne pour n'importe qui ayant accès au repo GitHub (même accès que pour ce + repo), et en CI via l'étape `Composer - Github Auth` déjà configurée dans + `payplug/template-ci`. Une première version utilisait un repository `path` local + (`../../unified-plugin-core`) : ça ne marche que sur une machine avec les deux repos clonés + côte à côte, cassait `composer install` pour tout le monde d'autre — corrigé. Nécessite que le + pin exact `symfony/polyfill-mbstring: 1.28.0` d'UPC ait été relâché en `^1.28` (corrigé dans + unified-plugin-core suite à ce spike — il bloquait tout `composer install` aux côtés de + `sylius/sylius ^2.0`, qui exige `^1.31`). Contrepartie connue : ça ajoute une résolution réseau + vers GitHub à chaque install (léger coût CI/dev) — pas d'alternative disponible ici (pas de + Packagist privé configuré dans cet org pour `payplug/unified-plugin-core`, et un repository + `path` est exclu pour la raison ci-dessus). Ce compromis disparaît quand PRE-3563 posera la + vraie dépendance de production définitive (OAuth réel contre UPC). +- `phpunit.xml.dist` : `KERNEL_CLASS_PATH` renommé en `KERNEL_CLASS` — la variable que Symfony lit + réellement pour `KernelTestCase::bootKernel()`. Sans ce fix, aucun test à base de kernel + (fonctionnel/intégration) n'a jamais pu tourner dans ce repo — un bug de config préexistant, + invisible tant qu'aucun test de ce type n'existait. +- `tests/TestApplication/.env.test.local` (gitignored) : `DATABASE_URL` pointée vers un conteneur + MariaDB jetable (`docker run --name payplug-pre3469-mysql -e MYSQL_ALLOW_EMPTY_PASSWORD=yes -p + 3309:3306 mariadb:latest`) — le MySQL système de la machine refuse `root@127.0.0.1` sans mot de + passe. `.env.local` seul ne suffit pas : Symfony l'ignore délibérément quand `APP_ENV=test` + (reproductibilité des tests) — il faut `.env.test.local` spécifiquement. +- Base créée/migrée avec les commandes standard du Makefile (`doctrine:database:create`, + `doctrine:migration:migrate`, `sylius:payment:generate-key`, `sylius:fixtures:load`) — la table + `payplug_operation` est créée automatiquement par `migrations/Version20260720100000.php`, plus + besoin d'aucune étape manuelle. + +**Isolation des tests** : `SpikeIntegrationTest` ne tourne dans aucune transaction annulée en fin +de test (pas de `DAMADoctrineTestBundle`). Une première version cherchait un paiement de fixture +déjà à l'état `new` — ça fonctionne une fois, mais une commande réelle ne repasse jamais à `new` +une fois transitionnée, donc ce pool fini s'épuise au fil des ré-exécutions contre la même base +jetable. Corrigé en créant un `Payment` frais sur une commande de fixture existante à chaque test +(`createOrderWithFreshPayment()`) plutôt que d'en chercher un dans un état donné — vérifié stable +sur 3 exécutions consécutives. Une vraie suite (non-spike) voudrait un rollback transactionnel +entre tests plutôt que ce contournement. +``` + +- [ ] **Step 2: Commit** + +```bash +git add src/Spike/VALIDATION.md +git commit -m "PRE-3469: Update VALIDATION.md for the promoted contracts + +Documents the require-dev blocking friction found and fixed, and the +production wiring decisions for each promoted contract." +``` + +--- + +## Task 7: Full regression pass + +**Files:** none (verification only). + +- [ ] **Step 1: Run the full default PHPUnit suite** + +Run: `vendor/bin/phpunit` +Expected: `OK (...)` — no failures, no errors, no risky tests introduced. + +- [ ] **Step 2: Run static analysis** + +Run: `vendor/bin/phpstan analyse` +Expected: no new errors beyond the existing `ruleset/phpstan-baseline.neon`. If new errors appear in the 4 new/moved files, fix them in place — do not add new baseline entries for code this plan just wrote. + +- [ ] **Step 3: Confirm no dangling references to the retired Spike class names** + +Run: `grep -rn "SyliusOrderStateMutator\|SyliusTokenCache\|SyliusConfigurationRepository" .` +Expected: no output (excluding this plan document and the design spec, which reference the old names historically). + +- [ ] **Step 4: Confirm `src/Spike/` now contains only the out-of-scope `IPaymentRepository` sketch** + +Run: `ls src/Spike/` +Expected: `Entity SyliusPaymentRepository.php VALIDATION.md` + +- [ ] **Step 5: Note the manual real-infrastructure verification step** + +`tests/PHPUnit/Spike/SpikeIntegrationTest.php` stays excluded from the default `phpunit.xml.dist` run (it needs a provisioned MariaDB test database — see its own docblock and the Câblage section of `VALIDATION.md`). Before merging, run it manually against a real database to confirm the renamed classes (`PayplugOrderStateMutator`, `PayplugTokenCache`, `PayplugConfigurationRepository`) still round-trip correctly: + +Run: `vendor/bin/phpunit tests/PHPUnit/Spike/SpikeIntegrationTest.php` +Expected: `OK (4 tests, ...)` — requires the DB setup described in `VALIDATION.md`'s Câblage section; this is a manual step for whoever has that database provisioned, not something to attempt in a sandboxed run without one. + +- [ ] **Step 6: Run Behat if configured and reachable** + +Run: `vendor/bin/behat` +Expected: `OK (...)` — if Behat requires infrastructure not available in the current environment (e.g. a browser driver or fixtures database), document that as a manual pre-merge step rather than skip verification silently. diff --git a/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md b/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md new file mode 100644 index 00000000..f668ecfe --- /dev/null +++ b/docs/superpowers/specs/2026-07-20-pre-3469-upc-contracts-rework-design.md @@ -0,0 +1,65 @@ +# PRE-3469 rework — real UPC contract wiring + +## Context + +[PRE-3469](https://payplug-prod.atlassian.net/browse/PRE-3469) originally asked for isolated proof-of-concept skeletons (under `src/Spike/`) validating that 4 Unified Plugin Core (UPC) contracts — `IOrderStateMutator`, `ITokenCache`, `IConfigurationRepository`, `IPaymentRepository` — are satisfiable against real Sylius APIs, before freezing the interfaces. That work landed in commit `690d9f5` and is fully written up in `src/Spike/VALIDATION.md`. + +The ticket's "Résultats attendus" were updated twice on 2026-07-20. The current version requires: + +- Real, production-wired implementations (not isolated `src/Spike/` skeletons) for `PayplugOrderStateMutator`, `PayplugTokenCache`, `PayplugConfigurationRepository`. +- `IPaymentRepository` explicitly **out of scope** (tied to a Value Object too coupled to the not-yet-built Unified API) — the existing `SyliusPaymentRepository` sketch is sufficient as-is. +- No regression on existing plugin functionality (PHPUnit + Behat stay green). +- Any blocking friction documented, or the interfaces adjusted. + +This spec covers reworking the branch to meet the updated scope. + +## Existing production code map + +Research (see conversation) found the production analogs each contract needs to be validated against: + +| Contract | Production analog | Notes | +|---|---|---| +| `IOrderStateMutator` | `PaymentTransitionApplier` (called from `NotifyPaymentRequestHandler`/`StatusPaymentRequestHandler`, the webhook/status-poll path) and `PaymentStateResolver` (called from the CLI reconciliation command `payplug:update-payment-state`) | Two separate, non-unified implementations of the same guarded `can()`/`apply()` idiom already exist in production; this ticket does not unify them. | +| `ITokenCache` | `PayPlugApiClientFactory::getTokenForGatewayConfig()`'s inline `Symfony\Contracts\Cache\CacheInterface` usage, caching the OAuth JWT access token per gateway factory/live-test scope | Confirmed by the ticket's second update: targets the OAuth JWT cache, not card/token storage (saved cards are a permanent Doctrine entity, `Card.php`, uninvolved with any cache). | +| `IConfigurationRepository` | Read side: `PayPlugApiClientFactory::getTokenForGatewayConfig()` (`client_id`/`client_secret` scoped by `config['live']`). Write side: `UnifiedAuthenticationController::oauthCallback()` (persists `live_client`/`test_client`, busts the token cache). | No production equivalent exists for `getPublicKeyId()`/`getPublicKeyValue()` — Hosted Fields public-key material isn't implemented anywhere in the plugin today. | +| `IPaymentRepository` | None — out of scope | `SyliusPaymentRepository` + `PayplugOperation` entity + `Version20260720100000` migration stay as an unwired sketch. | + +## Design + +### 1. `PayplugOrderStateMutator` — real production call site + +Keep the existing mapping logic from `src/Spike/SyliusOrderStateMutator.php`: `PaymentOutcome::PAID/AUTHORIZED/CAPTURE_REQUIRED/REFUNDED/FAILED` → `PaymentTransitions::TRANSITION_COMPLETE/AUTHORIZE/PROCESS/REFUND/FAIL`, `THREE_DS_PENDING` as a confirmed no-op, `StateMachineInterface::can()` guard before `apply()`, explicit `EntityManagerInterface::flush()` after. + +Wire it as an **additive** call inside `NotifyPaymentRequestHandler::__invoke()`, immediately after the existing `PaymentTransitionApplier::apply($payment)` call. The handler already has everything needed to derive a `PaymentOutcome` from the same status signal `PaymentTransitionApplier` consumes (`$payment->getDetails()['status']`, populated upstream by `PaymentNotificationHandler::treat()`); translate that into a `PaymentOutcome` value and call `$orderStateMutator->apply($order->getId(), $outcome)`. + +Because `PaymentTransitionApplier::apply()` already applied the transition earlier in the same handler invocation, `PayplugOrderStateMutator`'s own `can()` guard will find the transition no longer applicable and no-op. This proves the mutator works against a real, live webhook event without replacing or risking the existing transition-application path. No change to `PaymentStateResolver` or the CLI reconciliation command. + +### 2. `PayplugTokenCache` — real integration test, no production call site + +No change to `PayPlugApiClientFactory::getTokenForGatewayConfig()`. Promote the spike's existing unit-mocked test (`tests/PHPUnit/Spike/SyliusTokenCacheTest.php`) with a new real-infrastructure test that exercises `get`/`set`/`delete` against an actual `cache.app`-equivalent PSR-6 pool (filesystem or array adapter, same spirit as `SpikeIntegrationTest`'s real MariaDB), rather than a mocked `CacheItemPoolInterface`. This validates `getItem`/`save`/`deleteItem` "dans le flux réel" per the ticket, without introducing any new code on a live request path that gates authentication for every PayPlug gateway. + +### 3. `PayplugConfigurationRepository` — real integration test, no production call site + +Same treatment as `PayplugTokenCache`: add a real integration test that constructs `SyliusConfigurationRepository` against an actual `GatewayConfigInterface` on a real `PaymentMethod` fixture persisted via Doctrine (rather than a mock), validating `get`/`set`/`getClientId`/`getClientSecret` against real Sylius config storage. No change to `PayPlugApiClientFactory` or `UnifiedAuthenticationController`. + +`getPublicKeyId()`/`getPublicKeyValue()` read from two new config array keys, `public_key_id` and `public_key_value`, added alongside the existing `live_client`/`test_client` keys, defaulting to an empty string when absent. Nothing in production writes these keys yet — this is implementable end-to-end and ready for whenever Hosted Fields lands, documented as a friction note rather than worked around or blocked on. + +### 4. `IPaymentRepository` — unchanged + +`SyliusPaymentRepository`, `Entity/PayplugOperation.php`, and the `Version20260720100000` migration stay exactly as they are: an unwired sketch, correctly out of scope per the ticket. + +### 5. Documentation + +Update `src/Spike/VALIDATION.md` to reflect the new verdict per contract: which ones are now genuinely wired into production vs. validated only through real-infrastructure tests, and the two friction notes (`public_key_id`/`public_key_value` have no writer yet; `IOrderStateMutator`/`IConfigurationRepository` production analogs exist as more than one non-unified implementation, which this ticket does not consolidate). + +## Testing / regression safety + +- Existing PHPUnit + Behat suites must stay green after the changes. +- The one production code change (the additive `PayplugOrderStateMutator` call in `NotifyPaymentRequestHandler`) is guarded by the existing `can()` check, so it cannot alter current transition behavior — only observe/replay it against an already-applied transition. +- New real-infrastructure tests for `PayplugTokenCache` and `PayplugConfigurationRepository` are added, not replacing the existing unit-mocked spike tests. + +## Out of scope + +- Unifying `PaymentTransitionApplier` and `PaymentStateResolver` into a single `IOrderStateMutator`-backed implementation. +- Any production wiring of `IPaymentRepository`. +- Building Hosted Fields / any actual consumer of `public_key_id`/`public_key_value`. diff --git a/src/Command/Handler/NotifyPaymentRequestHandler.php b/src/Command/Handler/NotifyPaymentRequestHandler.php index bda2658d..55fe5add 100644 --- a/src/Command/Handler/NotifyPaymentRequestHandler.php +++ b/src/Command/Handler/NotifyPaymentRequestHandler.php @@ -129,6 +129,14 @@ private function applyOrderStateMutator(PaymentInterface $payment): void return; } + // PayplugOrderStateMutator resolves the order's *last* payment internally (its contract + // is keyed by order ID, not payment ID). On a multi-payment order — e.g. a failed attempt + // followed by a retry — that could be a different payment than the one this webhook is + // actually about. Skip rather than risk transitioning the wrong payment. + if ($order->getLastPayment()?->getId() !== $payment->getId()) { + return; + } + try { $this->orderStateMutator->apply((string) $order->getId(), $outcome); // @phpstan-ignore-line - ResourceInterface::getId() return mixed } catch (\Throwable $e) { diff --git a/src/Spike/VALIDATION.md b/src/Spike/VALIDATION.md index 1d952226..95b9086c 100644 --- a/src/Spike/VALIDATION.md +++ b/src/Spike/VALIDATION.md @@ -22,7 +22,9 @@ Couverture de tests, 3 niveaux : **Verdict global : aucune friction bloquante restante.** Une friction bloquante a été trouvée et corrigée pendant ce rework (voir « Friction bloquante corrigée » ci-dessous), plus deux notes non -bloquantes (aucune ne remet en cause la forme des interfaces). +bloquantes (aucune ne remet en cause la forme des interfaces). Voir aussi « Validation finale » +plus bas pour la passe de régression complète (test fonctionnel réel, `SpikeIntegrationTest`, +constat sur Behat). ## Friction bloquante corrigée : `payplug/unified-plugin-core` en `require-dev` @@ -35,8 +37,9 @@ chargée, y compris en production — change la donne : un vrai `composer instal marchand n'installerait pas `unified-plugin-core`, et charger une classe qui `implements` une interface inexistante est une erreur PHP fatale, pas un échec silencieux. -**Fix** : `payplug/unified-plugin-core` est passé en dépendance `require` (toujours pinné sur -`dev-develop`, même repository VCS). Documenté ici plutôt que traité comme un simple détail de +**Fix** : `payplug/unified-plugin-core` est passé en dépendance `require` (pinné sur `dev-master`, +même repository VCS — voir « Câblage » ci-dessous pour le changement de branche). Documenté ici +plutôt que traité comme un simple détail de `composer.json`, parce que ça anticipe une partie de ce que PRE-3563 (la vraie dépendance de production pour l'OAuth) devait poser — voir section Câblage plus bas pour ce qui reste à faire pour PRE-3563. @@ -147,23 +150,70 @@ réel utilisé pour le test d'intégration passe donc par un `doctrine.orm.mappi PRE-3469-only, restreinte à `kernel.environment === 'test'` (jamais en prod), à retirer avec `src/Spike/` si le spike est abandonné. +## Validation finale (2026-07-27) : passe de régression complète + +En complément des suites automatisées ci-dessus : + +- **Test fonctionnel réel**, environnement `plugin-dockerized-sylius` branché sur la QA PayPlug + (`api-qa.payplug.com`) : parcours d'achat complet avec un moyen de paiement carte PayPlug + configuré via l'OAuth réel, commande `000000022`. Le webhook IPN réel a été reçu et traité par + `NotifyPaymentRequestHandler` (`payment_id: pay_2iZJiB7mxAoc3poeZTTmeP`, `status: captured`), + paiement transitionné en `completed`, **aucun warning** de l'appel additif à + `PayplugOrderStateMutator` dans les logs, aucune régression sur le flux existant. Seule la + branche `STATUS_CAPTURED → PaymentOutcome::PAID` a été exercée en conditions réelles ; les autres + branches (`THREE_DS_PENDING`, `AUTHORIZED`, `CAPTURE_REQUIRED`, `REFUNDED`, `FAILED`) et le garde + multi-paiements (commit `779530d`) restent couverts uniquement par les tests unitaires. +- **`SpikeIntegrationTest`** rejoué contre une vraie base MariaDB jetable : `OK (5 tests, 17 + assertions)`. +- **Behat** : suite non exécutable en l'état — voir ci-dessous, sans lien avec ce ticket. + +### Behat : suite cassée, non liée à ce ticket + +`vendor/bin/behat --dry-run` échoue immédiatement : + +``` +`Lakion\Behat\MinkDebugExtension` extension file or class could not be located. +``` + +En creusant : + +- `behat.yml.dist` référence `Tests\PayPlug\SyliusPayPlugPlugin\Application\Kernel` et + `tests/Application/config/bootstrap.php` — mais `tests/Application/` **n'existe pas** dans ce + repo (seul `tests/TestApplication/` existe, utilisé par PHPUnit). +- La clé d'extension `Lakion\Behat\MinkDebugExtension` n'est pas le FQCN réel fourni par + `lakion/mink-debug-extension` ni par `friends-of-behat/mink-debug-extension` (les deux packages + sont installés, mais sous un namespace différent). +- `git log -- behat.yml.dist` montre que ce fichier n'a pas été modifié depuis le commit initial du + repo. +- Aucun workflow CI de ce repo n'exécute Behat. + +**Conclusion** : cette suite est cassée depuis longtemps, indépendamment de PRE-3469, et ne fait +partie d'aucune porte de qualité active. La remettre en état est un chantier séparé, hors scope de +ce ticket. + ## Câblage (pour rejouer les tests) - `composer.json` : `repositories` avec un repository `vcs` vers `https://github.com/payplug/unified-plugin-core.git` + `payplug/unified-plugin-core: - "dev-develop"` désormais en dépendance `require` (voir « Friction bloquante corrigée » + "dev-master"` désormais en dépendance `require` (voir « Friction bloquante corrigée » ci-dessus) — fonctionne pour n'importe qui ayant accès au repo GitHub (même accès que pour ce repo), et en CI via l'étape `Composer - Github Auth` déjà configurée dans `payplug/template-ci`. Une première version utilisait un repository `path` local (`../../unified-plugin-core`) : ça ne marche que sur une machine avec les deux repos clonés - côte à côte, cassait `composer install` pour tout le monde d'autre — corrigé. Nécessite que le - pin exact `symfony/polyfill-mbstring: 1.28.0` d'UPC ait été relâché en `^1.28` (corrigé dans - unified-plugin-core suite à ce spike — il bloquait tout `composer install` aux côtés de - `sylius/sylius ^2.0`, qui exige `^1.31`). Contrepartie connue : ça ajoute une résolution réseau - vers GitHub à chaque install (léger coût CI/dev) — pas d'alternative disponible ici (pas de - Packagist privé configuré dans cet org pour `payplug/unified-plugin-core`, et un repository - `path` est exclu pour la raison ci-dessus). Ce compromis disparaît quand PRE-3563 posera la - vraie dépendance de production définitive (OAuth réel contre UPC). + côte à côte, cassait `composer install` pour tout le monde d'autre — corrigé. Nécessitait que + le pin exact `symfony/polyfill-mbstring: 1.28.0` d'UPC (en conflit avec le `^1.31` exigé par + `sylius/sylius ^2.0`) soit relâché — corrigé côté `unified-plugin-core` (`1.30.0 || ^1.31`) et + fusionné sur `master` le 2026-07-21 ; ce repo pointait initialement sur `dev-develop` en + attendant la fusion, reposté sur `dev-master` le 2026-07-27 une fois confirmée (`origin/master` + == `origin/develop`, commit `e6a6733`) — revalidé par un `composer update + payplug/unified-plugin-core --with-all-dependencies` contre le vrai repo VCS (sans override + `path`), résolution propre sur `symfony/polyfill-mbstring v1.38.2`, suite complète + (`phpunit` 170 tests, `phpstan`) toujours au vert derrière. Contrepartie connue : ça ajoute une + résolution réseau vers GitHub à chaque install (léger coût CI/dev) — pas d'alternative + disponible ici (pas de Packagist privé configuré dans cet org pour + `payplug/unified-plugin-core`, et un repository `path` est exclu pour la raison ci-dessus). Ce + compromis disparaît quand PRE-3563 posera la vraie dépendance de production définitive (OAuth + réel contre UPC). - `phpunit.xml.dist` : `KERNEL_CLASS_PATH` renommé en `KERNEL_CLASS` — la variable que Symfony lit réellement pour `KernelTestCase::bootKernel()`. Sans ce fix, aucun test à base de kernel (fonctionnel/intégration) n'a jamais pu tourner dans ce repo — un bug de config préexistant, diff --git a/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php b/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php index 78de1331..257a9157 100644 --- a/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php +++ b/tests/PHPUnit/Command/Handler/NotifyPaymentRequestHandlerTest.php @@ -150,15 +150,26 @@ public function testInvoke_invalidPayload_failsWithoutCallingOrderStateMutator() $this->handler->__invoke(new NotifyPaymentRequest('hash')); } - private function prepareNormalFlow(string $status, int $orderId): void + /** + * Regression test for a multi-payment order: PayplugOrderStateMutator resolves the order's + * *last* payment internally (its contract is keyed by order ID, not payment ID). If a + * different payment (e.g. an earlier failed attempt) is the order's last payment, the + * additive call must be skipped rather than risk transitioning the wrong payment. + */ + public function testInvoke_paymentIsNotOrdersLastPayment_doesNotCallOrderStateMutator(): void { + $otherPayment = $this->createMock(PaymentInterface::class); + $otherPayment->method('getId')->willReturn(999); + $order = $this->createMock(OrderInterface::class); - $order->method('getId')->willReturn($orderId); + $order->method('getId')->willReturn(42); + $order->method('getLastPayment')->willReturn($otherPayment); $payment = $this->createMock(PaymentInterface::class); $payment->method('getState')->willReturn(PaymentInterface::STATE_NEW); - $payment->method('getDetails')->willReturn(['status' => $status]); + $payment->method('getDetails')->willReturn(['status' => PayPlugApiClientInterface::STATUS_CAPTURED]); $payment->method('getOrder')->willReturn($order); + $payment->method('getId')->willReturn(1); $method = $this->createMock(PaymentMethodInterface::class); $payment->method('getMethod')->willReturn($method); @@ -171,5 +182,34 @@ private function prepareNormalFlow(string $status, int $orderId): void $resource = $this->createMock(PayplugResourcePayment::class); $client->method('treat')->willReturn($resource); $this->apiClientFactory->method('createForPaymentMethod')->willReturn($client); + + $this->orderStateMutator->expects(self::never())->method('apply'); + + $this->handler->__invoke(new NotifyPaymentRequest('hash')); + } + + private function prepareNormalFlow(string $status, int $orderId): void + { + $payment = $this->createMock(PaymentInterface::class); + $payment->method('getState')->willReturn(PaymentInterface::STATE_NEW); + $payment->method('getDetails')->willReturn(['status' => $status]); + $payment->method('getId')->willReturn(1); + $method = $this->createMock(PaymentMethodInterface::class); + $payment->method('getMethod')->willReturn($method); + + $order = $this->createMock(OrderInterface::class); + $order->method('getId')->willReturn($orderId); + $order->method('getLastPayment')->willReturn($payment); + $payment->method('getOrder')->willReturn($order); + + $paymentRequest = $this->createMock(PaymentRequestInterface::class); + $paymentRequest->method('getPayment')->willReturn($payment); + $paymentRequest->method('getPayload')->willReturn(['http_request' => ['content' => '{}']]); + $this->paymentRequestProvider->method('provide')->willReturn($paymentRequest); + + $client = $this->createMock(PayPlugApiClientInterface::class); + $resource = $this->createMock(PayplugResourcePayment::class); + $client->method('treat')->willReturn($resource); + $this->apiClientFactory->method('createForPaymentMethod')->willReturn($client); } }