From 591f3373084cd46935c47c06e7892ce4df13293d Mon Sep 17 00:00:00 2001 From: Benjamin Kott Date: Tue, 12 Jul 2022 19:33:40 +0200 Subject: [PATCH] [BUGFIX] Do not cache public json endpoint (#337) * [BUGFIX] Do not cache public json endpoint * Require symfony/cache-contracts * [TASK] Remove not needed constructor parameter and string check * Check returned types * Apply CS rules * Reintroduce string check * Type check on retrival * Fix sqlite3 installation * Remove string check Co-authored-by: Simon Gilli <25326036+gilbertsoft@users.noreply.github.com> --- .github/workflows/continuous-integration.yml | 18 ++--- composer.json | 1 + composer.lock | 14 ++-- src/Controller/Api/AbstractController.php | 7 ++ src/Controller/Api/CacheController.php | 6 +- src/Controller/DefaultController.php | 19 ------ src/Controller/JsonController.php | 49 ++++++++++++++ src/Service/LegacyDataService.php | 31 +++------ .../Controller/Web/JsonControllerTest.php | 65 +++++++++++++++++++ 9 files changed, 147 insertions(+), 63 deletions(-) create mode 100644 src/Controller/JsonController.php create mode 100644 tests/Functional/Controller/Web/JsonControllerTest.php diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 0ce0311e..42dcb541 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -77,7 +77,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: latest tools: composer:2 @@ -115,7 +115,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: latest tools: composer:2 @@ -166,7 +166,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: ${{ matrix.php-version }} tools: composer:2 @@ -214,7 +214,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: latest tools: composer:2 @@ -254,7 +254,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: latest tools: composer:2 @@ -294,7 +294,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: latest tools: composer:2 @@ -334,7 +334,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: latest tools: composer:2 @@ -398,7 +398,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: none - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1 php-version: ${{ matrix.php-version }} tools: composer:2 @@ -487,7 +487,7 @@ jobs: uses: shivammathur/setup-php@v2 with: coverage: xdebug - extensions: ctype, iconv, json, sqlit3, tokenizer, zip, zlib + extensions: ctype, iconv, json, sqlite3, tokenizer, zip, zlib ini-values: memory_limit=-1, error_reporting=E_ALL, display_errors=On php-version: ${{ matrix.php-version }} tools: composer:2 diff --git a/composer.json b/composer.json index 3130fb07..da7e90b4 100644 --- a/composer.json +++ b/composer.json @@ -54,6 +54,7 @@ "sensio/framework-extra-bundle": "^6.1", "symfony/asset": "5.4.*", "symfony/cache": "5.4.*", + "symfony/cache-contracts": "^2.5", "symfony/console": "5.4.*", "symfony/dependency-injection": "5.4.*", "symfony/dotenv": "5.4.*", diff --git a/composer.lock b/composer.lock index 164dd363..8d6b7761 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "bd26499090833ebe3c6a94ffc865c313", + "content-hash": "da72aa9dfdaaa54bdf1e5b86b75c55b7", "packages": [ { "name": "composer/ca-bundle", @@ -3917,16 +3917,16 @@ }, { "name": "symfony/cache-contracts", - "version": "v2.5.0", + "version": "v2.5.2", "source": { "type": "git", "url": "https://github.com/symfony/cache-contracts.git", - "reference": "ac2e168102a2e06a2624f0379bde94cd5854ced2" + "reference": "64be4a7acb83b6f2bf6de9a02cee6dad41277ebc" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/cache-contracts/zipball/ac2e168102a2e06a2624f0379bde94cd5854ced2", - "reference": "ac2e168102a2e06a2624f0379bde94cd5854ced2", + "url": "https://api.github.com/repos/symfony/cache-contracts/zipball/64be4a7acb83b6f2bf6de9a02cee6dad41277ebc", + "reference": "64be4a7acb83b6f2bf6de9a02cee6dad41277ebc", "shasum": "" }, "require": { @@ -3976,7 +3976,7 @@ "standards" ], "support": { - "source": "https://github.com/symfony/cache-contracts/tree/v2.5.0" + "source": "https://github.com/symfony/cache-contracts/tree/v2.5.2" }, "funding": [ { @@ -3992,7 +3992,7 @@ "type": "tidelift" } ], - "time": "2021-08-17T14:20:01+00:00" + "time": "2022-01-02T09:53:40+00:00" }, { "name": "symfony/config", diff --git a/src/Controller/Api/AbstractController.php b/src/Controller/Api/AbstractController.php index 3064acc3..0a47e36b 100644 --- a/src/Controller/Api/AbstractController.php +++ b/src/Controller/Api/AbstractController.php @@ -36,10 +36,12 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Validator\ConstraintViolationInterface; +use Symfony\Contracts\Cache\TagAwareCacheInterface; class AbstractController extends \Symfony\Bundle\FrameworkBundle\Controller\AbstractController { public function __construct( + private readonly TagAwareCacheInterface $cache, private readonly \JMS\Serializer\SerializerInterface $serializer, private readonly \Doctrine\Persistence\ManagerRegistry $managerRegistry, private readonly \App\Repository\MajorVersionRepository $majorVersions, @@ -49,6 +51,11 @@ public function __construct( ) { } + protected function getCache(): TagAwareCacheInterface + { + return $this->cache; + } + protected function getSerializer(): SerializerInterface { return $this->serializer; diff --git a/src/Controller/Api/CacheController.php b/src/Controller/Api/CacheController.php index bfad0696..6d8e7408 100644 --- a/src/Controller/Api/CacheController.php +++ b/src/Controller/Api/CacheController.php @@ -26,7 +26,6 @@ use Nelmio\ApiDocBundle\Annotation\Security as DocSecurity; use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted; use Swagger\Annotations as SWG; -use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Annotation\Route; @@ -156,10 +155,7 @@ private function getPurgeUrlsForMajorVersion(float $version): array */ private function deleteReleases(): void { - $filesystemAdapter = new FilesystemAdapter(); - if ($filesystemAdapter->hasItem('releases.json')) { - $filesystemAdapter->delete('releases.json'); - } + $this->getCache()->invalidateTags(['releases']); } /** diff --git a/src/Controller/DefaultController.php b/src/Controller/DefaultController.php index 786b460e..9194bbb8 100644 --- a/src/Controller/DefaultController.php +++ b/src/Controller/DefaultController.php @@ -27,7 +27,6 @@ use App\Entity\Release; use App\Utility\VersionUtility; use InvalidArgumentException; -use Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -39,8 +38,6 @@ /** * Regular content and download pages - * - * @Cache(maxage="3600", public=true) */ class DefaultController extends AbstractController { @@ -82,22 +79,6 @@ public function apiDoc(): RedirectResponse return $this->redirectToRoute('app.swagger_ui'); } - /** - * Outputs the JSON file - * /json - * Legacy end point - */ - #[Route(path: '/json', methods: ['GET'], name: 'legacy-releases-json')] - public function releaseJson(): Response - { - $content = $this->legacyDataService->getReleaseJson(); - $headers = [ - 'Content-type' => 'application/json', - 'Access-Control-Allow-Origin' => '*', - ]; - return new Response($content, \Symfony\Component\HttpFoundation\Response::HTTP_OK, $headers); - } - /** * Display release notes for a version */ diff --git a/src/Controller/JsonController.php b/src/Controller/JsonController.php new file mode 100644 index 00000000..26a9697d --- /dev/null +++ b/src/Controller/JsonController.php @@ -0,0 +1,49 @@ +legacyDataService->getReleaseJson(); + $headers = [ + 'Content-type' => 'application/json', + 'Access-Control-Allow-Origin' => '*', + ]; + return new Response($content, \Symfony\Component\HttpFoundation\Response::HTTP_OK, $headers); + } +} diff --git a/src/Service/LegacyDataService.php b/src/Service/LegacyDataService.php index b8ed1150..2bcdc0a3 100644 --- a/src/Service/LegacyDataService.php +++ b/src/Service/LegacyDataService.php @@ -23,43 +23,28 @@ namespace App\Service; -use App\Entity\MajorVersion; use App\Repository\MajorVersionRepository; -use Doctrine\ORM\EntityManagerInterface; -use Symfony\Component\Cache\Adapter\FilesystemAdapter; use Symfony\Contracts\Cache\ItemInterface; +use Symfony\Contracts\Cache\TagAwareCacheInterface; class LegacyDataService { - public function __construct(private readonly EntityManagerInterface $entityManager) - { + public function __construct( + private readonly TagAwareCacheInterface $cache, + private readonly MajorVersionRepository $majorVersionRepository, + ) { } public function getReleaseJson(): string { - /* - $cache = new FilesystemAdapter(); - $result = $cache->get('releases.json', function (ItemInterface $item): string { - /** @var MajorVersionRepository $majorVersions * / - $majorVersions = $this->entityManager->getRepository(MajorVersion::class); - $content = json_encode($majorVersions->findAllPreparedForJson(), JSON_THROW_ON_ERROR); + $result = $this->cache->get('releases.json', function (ItemInterface $item): string { + $item->tag(['releases']); + $content = json_encode($this->majorVersionRepository->findAllPreparedForJson(), JSON_THROW_ON_ERROR); $content = $content != false ? $content : ''; // remove version suffix only used for version sorting return str_replace('.0000', '', $content); }); - if (!is_string($result)) { - throw new \RuntimeException(sprintf('String expected but %s given.', gettype($result))); - } - */ - - /** @var MajorVersionRepository $majorVersions */ - $majorVersions = $this->entityManager->getRepository(MajorVersion::class); - $content = json_encode($majorVersions->findAllPreparedForJson(), JSON_THROW_ON_ERROR); - $content = $content != false ? $content : ''; - // remove version suffix only used for version sorting - $result = str_replace('.0000', '', $content); - return $result; } } diff --git a/tests/Functional/Controller/Web/JsonControllerTest.php b/tests/Functional/Controller/Web/JsonControllerTest.php new file mode 100644 index 00000000..e73c4a9e --- /dev/null +++ b/tests/Functional/Controller/Web/JsonControllerTest.php @@ -0,0 +1,65 @@ +addFixture(new MajorVersionFixtures()); + $this->addFixture(new ReleaseFixtures()); + $this->addFixture(new RequirementFixtures()); + $this->executeFixtures(); + } + + /** + * @test + */ + public function index(): void + { + $this->client->request('GET', '/json'); + $response = $this->client->getResponse(); + + if (($json = $response->getContent()) === false) { + throw new RuntimeException('Error no response content.', 1_657_642_832); + } + + if (!\is_array($content = json_decode($json, true, 512, JSON_THROW_ON_ERROR))) { + throw new RuntimeException('Error array expected.', 1_657_642_833); + } + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + self::assertArrayHasKey('10', $content); + self::assertArrayHasKey('latest_stable', $content); + self::assertArrayHasKey('latest_old_stable', $content); + self::assertArrayHasKey('latest_lts', $content); + self::assertArrayHasKey('latest_old_lts', $content); + } +}