From 4907f4e83975e9c8ebb1264b4b1f88be06a0984d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Fri, 28 Feb 2025 14:08:14 +0100 Subject: [PATCH] Remove session injection in session storage --- .github/workflows/build.yaml | 222 +++++++++++++++--- LICENSE | 2 +- composer.json | 27 ++- ecs.php | 2 +- phpspec.yml.dist | 4 - phpstan.neon | 3 - psalm.xml | 26 ++ .../AddPathsToPhpTemplatesEnginePass.php | 4 +- .../Compiler/AddPathsToTwigPass.php | 4 +- .../Compiler/RegisterRenderersPass.php | 7 +- .../SetonoTagBagExtension.php | 2 +- src/EventListener/RestoreTagBagSubscriber.php | 5 +- src/EventListener/StoreTagBagSubscriber.php | 5 +- src/Resources/config/services/storage.xml | 2 +- src/Storage/SessionStorage.php | 37 ++- .../StoreTagBagSubscriberTest.php | 8 +- tests/SetonoTagBagBundleTest.php | 4 - 17 files changed, 280 insertions(+), 84 deletions(-) delete mode 100644 phpspec.yml.dist delete mode 100644 phpstan.neon create mode 100644 psalm.xml diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 165cd30..1f61d5d 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -1,42 +1,196 @@ -name: build +name: "build" on: - push: ~ + push: + branches: + - "2.x" pull_request: ~ workflow_dispatch: ~ jobs: - checks: - name: 'PHP ${{ matrix.php-versions }} with composer args: ${{ matrix.composer-args }}' - runs-on: ubuntu-latest + coding-standards: + name: "Coding Standards" + + runs-on: "ubuntu-latest" + strategy: matrix: - php-versions: ['7.4', '8.0', '8.1'] - composer-args: ['--prefer-lowest --prefer-stable', ''] + php-version: + - "7.4" + + dependencies: + - "highest" + steps: - - name: Checkout - uses: actions/checkout@v1 - - name: Setup PHP, with composer and extensions - uses: shivammathur/setup-php@v2 - with: - php-version: ${{ matrix.php-versions }} - extensions: intl - tools: composer-require-checker, composer-unused - coverage: none - - name: Install Composer dependencies - run: composer update --no-progress --no-suggest --prefer-dist --no-interaction ${{ matrix.composer-args }} - - name: Validate composer - run: composer validate --strict - - name: Check composer normalized - run: composer normalize --dry-run - - name: Check style - run: composer check-style - - name: Static analysis - run: composer analyse - - name: Run phpunit - run: composer phpunit - - name: Composer require checker - if: matrix.php-versions == '7.4' && matrix.composer-args == '' - run: composer-require-checker - - name: Composer unused checker - if: matrix.php-versions == '7.4' && matrix.composer-args == '' - run: composer-unused + - name: "Checkout" + uses: "actions/checkout@v4" + + - name: "Setup PHP, with composer and extensions" + uses: "shivammathur/setup-php@v2" + with: + php-version: "${{ matrix.php-version }}" + extensions: "${{ env.PHP_EXTENSIONS }}" + coverage: "none" + + - name: "Install composer dependencies" + uses: "ramsey/composer-install@v3" + with: + dependency-versions: "${{ matrix.dependencies }}" + + - name: "Validate composer" + run: "composer validate --strict" + + - name: "Check composer normalized" + run: "composer normalize --dry-run" + + - name: "Check style" + run: "composer check-style" + + dependency-analysis: + name: "Dependency Analysis" + + runs-on: "ubuntu-latest" + + strategy: + matrix: + php-version: + - "7.4" + - "8.0" + - "8.1" + - "8.2" + - "8.3" + + dependencies: + - "lowest" + - "highest" + + symfony: + - "~5.4.0" + - "~6.4.0" + + exclude: + - php-version: "7.4" + symfony: "~6.4.0" + - php-version: "8.0" + symfony: "~6.4.0" + + steps: + - name: "Checkout" + uses: "actions/checkout@v4" + + - name: "Setup PHP, with composer and extensions" + uses: "shivammathur/setup-php@v2" + with: + coverage: "none" + extensions: "${{ env.PHP_EXTENSIONS }}" + php-version: "${{ matrix.php-version }}" + tools: "flex" + + - name: "Install composer dependencies" + uses: "ramsey/composer-install@v3" + env: + SYMFONY_REQUIRE: "${{ matrix.symfony }}" + with: + dependency-versions: "${{ matrix.dependencies }}" + + - name: "Dependency analysis" + run: "vendor/bin/composer-dependency-analyser" + + static-code-analysis: + name: "Static Code Analysis" + + runs-on: "ubuntu-latest" + + strategy: + matrix: + php-version: + - "7.4" + - "8.0" + - "8.1" + - "8.2" + - "8.3" + + dependencies: + - "lowest" + - "highest" + + symfony: + - "~5.4.0" + - "~6.4.0" + + exclude: + - php-version: "7.4" + symfony: "~6.4.0" + - php-version: "8.0" + symfony: "~6.4.0" + + steps: + - name: "Checkout" + uses: "actions/checkout@v4" + + - name: "Setup PHP, with composer and extensions" + uses: "shivammathur/setup-php@v2" + with: + php-version: "${{ matrix.php-version }}" + extensions: "${{ env.PHP_EXTENSIONS }}" + coverage: "none" + tools: "flex" + + - name: "Install composer dependencies" + uses: "ramsey/composer-install@v3" + env: + SYMFONY_REQUIRE: "${{ matrix.symfony }}" + with: + dependency-versions: "${{ matrix.dependencies }}" + + - name: "Static analysis" + run: "vendor/bin/psalm --php-version=${{ matrix.php-version }}" + + unit-tests: + name: "Unit tests" + + runs-on: "ubuntu-latest" + + strategy: + matrix: + php-version: + - "7.4" + - "8.0" + - "8.1" + - "8.2" + - "8.3" + + dependencies: + - "lowest" + - "highest" + + symfony: + - "~5.4.0" + - "~6.4.0" + + exclude: + - php-version: "7.4" + symfony: "~6.4.0" + - php-version: "8.0" + symfony: "~6.4.0" + + steps: + - name: "Checkout" + uses: "actions/checkout@v4" + + - name: "Setup PHP, with composer and extensions" + uses: "shivammathur/setup-php@v2" + with: + php-version: "${{ matrix.php-version }}" + extensions: "${{ env.PHP_EXTENSIONS }}" + coverage: "none" + tools: "flex" + + - name: "Install composer dependencies" + uses: "ramsey/composer-install@v3" + env: + SYMFONY_REQUIRE: "${{ matrix.symfony }}" + with: + dependency-versions: "${{ matrix.dependencies }}" + + - name: "Run phpunit" + run: "composer phpunit" diff --git a/LICENSE b/LICENSE index e987d35..e4c0848 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2020 Setono +Copyright (c) 2025 Setono Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/composer.json b/composer.json index 2f419cf..58b336a 100644 --- a/composer.json +++ b/composer.json @@ -12,25 +12,26 @@ "require": { "php": ">=7.4", "setono/tag-bag": "^1.4", - "symfony/config": "^4.4 || ^5.4 || ^6.0", - "symfony/dependency-injection": "^4.4 || ^5.4 || ^6.0", - "symfony/event-dispatcher": "^4.4 || ^5.4 || ^6.0", - "symfony/http-foundation": "^4.4 || ^5.4 || ^6.0", - "symfony/http-kernel": "^4.4 || ^5.4 || ^6.0", + "symfony/config": "^5.4 || ^6.4", + "symfony/dependency-injection": "^5.4 || ^6.4", + "symfony/deprecation-contracts": "^2.0 || ^3.0", + "symfony/event-dispatcher": "^5.4 || ^6.4", + "symfony/http-foundation": "^5.4 || ^6.4", + "symfony/http-kernel": "^5.4 || ^6.4", "twig/twig": "^2.0 || ^3.0", "webmozart/assert": "^1.11" }, "require-dev": { "matthiasnoback/symfony-dependency-injection-test": "^4.2", "nyholm/symfony-bundle-test": "^1.7", + "phpspec/prophecy-phpunit": "^2.3", "phpunit/phpunit": "^9.5", - "roave/security-advisories": "dev-latest", - "setono/code-quality-pack": "^2.2", - "setono/php-templates-bundle": "^1.0", + "psalm/plugin-phpunit": "^0.19.0", + "setono/code-quality-pack": "^2.9", "setono/tag-bag-gtag": "^1.0", - "setono/tag-bag-php-templates": "^1.2", "setono/tag-bag-twig": "^1.1", - "symfony/twig-bundle": "^4.4 || ^5.4 || ^6.0" + "shipmonk/composer-dependency-analyser": "^1.8.2", + "symfony/twig-bundle": "^5.4 || ^6.4" }, "prefer-stable": true, "autoload": { @@ -45,14 +46,14 @@ }, "config": { "allow-plugins": { - "phpstan/extension-installer": true, + "dealerdirect/phpcodesniffer-composer-installer": false, "ergebnis/composer-normalize": true, - "dealerdirect/phpcodesniffer-composer-installer": false + "phpstan/extension-installer": true }, "sort-packages": true }, "scripts": { - "analyse": "phpstan analyse -c phpstan.neon -l max src/", + "analyse": "psalm", "check-style": "ecs check src/ tests/", "fix-style": "ecs check --fix src/ tests/", "phpunit": "phpunit" diff --git a/ecs.php b/ecs.php index 2c950d2..9fe162b 100644 --- a/ecs.php +++ b/ecs.php @@ -5,5 +5,5 @@ use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; return static function (ContainerConfigurator $containerConfigurator): void { - $containerConfigurator->import(__DIR__ . '/vendor/setono/coding-standard/easy-coding-standard.php'); + $containerConfigurator->import('vendor/sylius-labs/coding-standard/ecs.php'); }; diff --git a/phpspec.yml.dist b/phpspec.yml.dist deleted file mode 100644 index e894041..0000000 --- a/phpspec.yml.dist +++ /dev/null @@ -1,4 +0,0 @@ -suites: - main: - namespace: Setono\TagBagBundle - psr4_prefix: Setono\TagBagBundle diff --git a/phpstan.neon b/phpstan.neon deleted file mode 100644 index 43ce7e5..0000000 --- a/phpstan.neon +++ /dev/null @@ -1,3 +0,0 @@ -parameters: - reportUnmatchedIgnoredErrors: true - checkMissingIterableValueType: false diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 0000000..7c5886c --- /dev/null +++ b/psalm.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + diff --git a/src/DependencyInjection/Compiler/AddPathsToPhpTemplatesEnginePass.php b/src/DependencyInjection/Compiler/AddPathsToPhpTemplatesEnginePass.php index c66971b..37c1e8f 100644 --- a/src/DependencyInjection/Compiler/AddPathsToPhpTemplatesEnginePass.php +++ b/src/DependencyInjection/Compiler/AddPathsToPhpTemplatesEnginePass.php @@ -6,7 +6,6 @@ use InvalidArgumentException; use ReflectionClass; -use function Safe\sprintf; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -32,7 +31,8 @@ private static function addGtagPaths(ContainerBuilder $container): void $filename = (new ReflectionClass($interface))->getFileName(); if (false === $filename) { throw new InvalidArgumentException(sprintf( - 'The filename of interface %s could not be deduced', $interface + 'The filename of interface %s could not be deduced', + $interface )); } diff --git a/src/DependencyInjection/Compiler/AddPathsToTwigPass.php b/src/DependencyInjection/Compiler/AddPathsToTwigPass.php index 63a57bd..d0d75f3 100644 --- a/src/DependencyInjection/Compiler/AddPathsToTwigPass.php +++ b/src/DependencyInjection/Compiler/AddPathsToTwigPass.php @@ -6,7 +6,6 @@ use InvalidArgumentException; use ReflectionClass; -use function Safe\sprintf; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -35,7 +34,8 @@ private static function addGtagPaths(Definition $twigLoader): void $filename = (new ReflectionClass($interface))->getFileName(); if (false === $filename) { throw new InvalidArgumentException(sprintf( - 'The filename of interface %s could not be deduced', $interface + 'The filename of interface %s could not be deduced', + $interface )); } diff --git a/src/DependencyInjection/Compiler/RegisterRenderersPass.php b/src/DependencyInjection/Compiler/RegisterRenderersPass.php index bc712f7..649d645 100644 --- a/src/DependencyInjection/Compiler/RegisterRenderersPass.php +++ b/src/DependencyInjection/Compiler/RegisterRenderersPass.php @@ -18,9 +18,14 @@ public function process(ContainerBuilder $container): void $renderer = $container->getDefinition('setono_tag_bag.renderer.composite'); + /** + * @var string $id + * @var array $tags + */ foreach ($container->findTaggedServiceIds('setono_tag_bag.renderer') as $id => $tags) { + /** @var array $tag */ foreach ($tags as $tag) { - $priority = $tag['priority'] ?? 0; + $priority = (int) ($tag['priority'] ?? 0); $renderer->addMethodCall('addRenderer', [new Reference($id), $priority]); } diff --git a/src/DependencyInjection/SetonoTagBagExtension.php b/src/DependencyInjection/SetonoTagBagExtension.php index 59f1c57..5bb4513 100644 --- a/src/DependencyInjection/SetonoTagBagExtension.php +++ b/src/DependencyInjection/SetonoTagBagExtension.php @@ -12,7 +12,7 @@ final class SetonoTagBagExtension extends Extension { - public function load(array $config, ContainerBuilder $container): void + public function load(array $configs, ContainerBuilder $container): void { $container->registerForAutoconfiguration(RendererInterface::class) ->addTag('setono_tag_bag.renderer', [ diff --git a/src/EventListener/RestoreTagBagSubscriber.php b/src/EventListener/RestoreTagBagSubscriber.php index 712f6aa..d4325b6 100644 --- a/src/EventListener/RestoreTagBagSubscriber.php +++ b/src/EventListener/RestoreTagBagSubscriber.php @@ -11,8 +11,7 @@ final class RestoreTagBagSubscriber implements EventSubscriberInterface { - /** @var TagBagInterface */ - private $tagBag; + private TagBagInterface $tagBag; public function __construct(TagBagInterface $tagBag) { @@ -33,7 +32,7 @@ public static function getSubscribedEvents(): array public function onKernelRequest(RequestEvent $event): void { - if (!$event->isMasterRequest()) { + if (!$event->isMainRequest()) { return; } diff --git a/src/EventListener/StoreTagBagSubscriber.php b/src/EventListener/StoreTagBagSubscriber.php index b17d7c6..5a41b48 100644 --- a/src/EventListener/StoreTagBagSubscriber.php +++ b/src/EventListener/StoreTagBagSubscriber.php @@ -11,8 +11,7 @@ final class StoreTagBagSubscriber implements EventSubscriberInterface { - /** @var TagBagInterface */ - private $tagBag; + private TagBagInterface $tagBag; public function __construct(TagBagInterface $tagBag) { @@ -33,7 +32,7 @@ public static function getSubscribedEvents(): array public function onKernelResponse(ResponseEvent $event): void { - if (!$event->isMasterRequest()) { + if (!$event->isMainRequest()) { return; } diff --git a/src/Resources/config/services/storage.xml b/src/Resources/config/services/storage.xml index ce5fabf..3868c7a 100644 --- a/src/Resources/config/services/storage.xml +++ b/src/Resources/config/services/storage.xml @@ -14,7 +14,7 @@ --> - + diff --git a/src/Storage/SessionStorage.php b/src/Storage/SessionStorage.php index 5aad762..6aa037d 100644 --- a/src/Storage/SessionStorage.php +++ b/src/Storage/SessionStorage.php @@ -5,30 +5,57 @@ namespace Setono\TagBagBundle\Storage; use Setono\TagBag\Storage\StorageInterface; +use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\SessionInterface; +use Webmozart\Assert\Assert; final class SessionStorage implements StorageInterface { - /** @var SessionInterface */ + /** @var SessionInterface|RequestStack */ private $session; - public function __construct(SessionInterface $session) + /** + * @param SessionInterface|RequestStack $session + */ + public function __construct($session) { + if ($session instanceof SessionInterface) { + trigger_deprecation('setono/tag-bag-bundle', '2.3', 'Passing a SessionInterface to the constructor is deprecated, pass a RequestStack instead'); + } + $this->session = $session; } public function store(string $data): void { - $this->session->set(self::DATA_KEY, $data); + $this->getSession()->set(self::DATA_KEY, $data); } public function restore(): ?string { - return $this->session->get(self::DATA_KEY); + $data = $this->getSession()->get(self::DATA_KEY); + Assert::nullOrString($data); + + return $data; } public function remove(): void { - $this->session->remove(self::DATA_KEY); + $this->getSession()->remove(self::DATA_KEY); + } + + private function getSession(): SessionInterface + { + if ($this->session instanceof SessionInterface) { + return $this->session; + } + + $request = $this->session->getMainRequest(); + Assert::notNull($request); + + $session = $request->getSession(); + Assert::isInstanceOf($session, SessionInterface::class); + + return $session; } } diff --git a/tests/EventListener/StoreTagBagSubscriberTest.php b/tests/EventListener/StoreTagBagSubscriberTest.php index 1a7454c..ace57f9 100644 --- a/tests/EventListener/StoreTagBagSubscriberTest.php +++ b/tests/EventListener/StoreTagBagSubscriberTest.php @@ -4,7 +4,6 @@ namespace Setono\TagBagBundle\Tests\EventListener; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Setono\TagBag\TagBagInterface; @@ -68,15 +67,12 @@ public function it_has_the_correct_priority(): void $this->assertGreaterThan($sessionListenerPriority, $priority); } - /** - * @return ResponseEvent|MockObject - */ - private function getResponseEvent(bool $masterRequest): ResponseEvent + private function getResponseEvent(bool $mainRequest): ResponseEvent { $kernel = $this->createMock(HttpKernelInterface::class); $request = $this->createMock(Request::class); $response = $this->createMock(Response::class); - return new ResponseEvent($kernel, $request, $masterRequest ? HttpKernelInterface::MASTER_REQUEST : HttpKernelInterface::SUB_REQUEST, $response); + return new ResponseEvent($kernel, $request, $mainRequest ? HttpKernelInterface::MAIN_REQUEST : HttpKernelInterface::SUB_REQUEST, $response); } } diff --git a/tests/SetonoTagBagBundleTest.php b/tests/SetonoTagBagBundleTest.php index 17868ee..feb3d73 100644 --- a/tests/SetonoTagBagBundleTest.php +++ b/tests/SetonoTagBagBundleTest.php @@ -6,11 +6,9 @@ use Nyholm\BundleTest\BaseBundleTestCase; use Nyholm\BundleTest\CompilerPass\PublicServicePass; -use Setono\PhpTemplatesBundle\SetonoPhpTemplatesBundle; use Setono\TagBag\Generator\ValueBasedFingerprintGenerator; use Setono\TagBag\Renderer\CompositeRenderer; use Setono\TagBag\Renderer\ContentRenderer; -use Setono\TagBag\Renderer\PhpTemplatesRenderer; use Setono\TagBag\Renderer\ScriptRenderer; use Setono\TagBag\Renderer\StyleRenderer; use Setono\TagBag\Renderer\TwigRenderer; @@ -46,7 +44,6 @@ public function it_inits(): void { $kernel = $this->createKernel(); $kernel->addBundle(TwigBundle::class); - $kernel->addBundle(SetonoPhpTemplatesBundle::class); $this->bootKernel(); @@ -70,7 +67,6 @@ public function it_inits(): void ['id' => 'setono_tag_bag.renderer.script', 'class' => ScriptRenderer::class], ['id' => 'setono_tag_bag.renderer.style', 'class' => StyleRenderer::class], ['id' => 'setono_tag_bag.renderer.twig', 'class' => TwigRenderer::class], - ['id' => 'setono_tag_bag.renderer.php_templates', 'class' => PhpTemplatesRenderer::class], // storage //['id' => StorageInterface::class, 'class' => SessionStorage::class], // todo why doesn't this work?