Verified Commit 3624059e authored by Dave Long's avatar Dave Long
Browse files

fix: #3555974 HttpExceptionNormalizer sets cache-max to 0 unnecessarily sometimes 

By: tstoeckler
By: bbrala
(cherry picked from commit e1e87688)
parent 35860d3f

core/modules/jsonapi/src/Normalizer/HttpExceptionNormalizer.php

+6 −1
Original line number Diff line number Diff line
@@ -51,8 +51,13 @@ public function normalize($object, $format = NULL, array $context = []): array|s 
      $cacheability->setCacheMaxAge(0);

    }



    $is_verbose_reporting = \Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE;

    $cacheability->addCacheTags(['config:system.logging']);

    if (\Drupal::config('system.logging')->get('error_level') === ERROR_REPORTING_DISPLAY_VERBOSE) {



    $site_report_access = $this->currentUser->hasPermission('access site reports');

    $cacheability->addCacheContexts(['user.permissions']);



    if ($site_report_access && $is_verbose_reporting) {

      $cacheability->setCacheMaxAge(0);

    }

core/modules/jsonapi/tests/src/Functional/BlockTest.php

+1 −1
Original line number Diff line number Diff line
@@ -186,7 +186,7 @@ protected function getExpectedUnauthorizedAccessCacheability() { 
        'http_response',

        'user:2',

      ])

      ->setCacheContexts(['url.query_args', 'url.site', 'user.roles']);

      ->setCacheContexts(['url.query_args', 'url.site', 'user.permissions', 'user.roles']);

  }



  /**

core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php

+6 −1
Original line number Diff line number Diff line
@@ -604,6 +604,7 @@ protected static function getExpectedCollectionCacheability(AccountInterface $ac 
      'url.query_args',

      // Drupal defaults.

      'url.site',

      'user.permissions',

    ];

    // If the entity type is revisionable, add a resource version cache context.

    $cache_contexts = Cache::mergeContexts($cache_contexts, $entity_type->isRevisionable() ? ['url.query_args:resourceVersion'] : []);
@@ -983,7 +984,7 @@ public function testGetIndividual(): void { 
      $expected_document,

      $response,

      ['4xx-response', 'http_response'],

      ['url.query_args', 'url.site'],

      ['url.query_args', 'url.site', 'user.permissions'],

      'UNCACHEABLE (request policy)',

      TRUE,

    );
@@ -1097,6 +1098,7 @@ public function testGetIndividual(): void { 
      [

        'url.query_args',

        'url.site',

        'user.permissions',

      ],

      'UNCACHEABLE (request policy)',

      'UNCACHEABLE (poor cacheability)',
@@ -1118,6 +1120,7 @@ public function testGetIndividual(): void { 
      [

        'url.query_args',

        'url.site',

        'user.permissions',

      ],

      'UNCACHEABLE (request policy)',

      'UNCACHEABLE (poor cacheability)',
@@ -3086,6 +3089,7 @@ public function testRevisions(): void { 
      'url.path',

      'url.query_args',

      'url.site',

      'user.permissions',

    ];

    $this->assertResourceErrorResponse(501, 'JSON:API does not support filtering on revisions other than the latest version because a secure Drupal core API does not yet exist to do so.', $rel_working_copy_collection_url_filtered, $actual_response, FALSE, ['http_response'], $filtered_collection_expected_cache_contexts);

    // Fetch the collection URL using an invalid version identifier.
@@ -3094,6 +3098,7 @@ public function testRevisions(): void { 
      'url.path',

      'url.query_args',

      'url.site',

      'user.permissions',

    ];

    $this->assertResourceErrorResponse(

      400,

core/modules/jsonapi/tests/src/Functional/UserTest.php

+1 −1
Original line number Diff line number Diff line
@@ -475,7 +475,7 @@ public function testQueryInvolvingRoles(): void { 
    $this->grantPermissionsToTestedRole(['administer users']);



    $response = $this->request('GET', $collection_url, $request_options);

    $expected_cache_contexts = ['url.path', 'url.query_args', 'url.site'];

    $expected_cache_contexts = ['url.path', 'url.query_args', 'url.site', 'user.permissions'];

    $this->assertResourceErrorResponse(

      400,

      "Filtering on config entities is not supported by Drupal's entity API. You tried to filter on a Role config entity.",

core/modules/jsonapi/tests/src/Unit/Normalizer/HttpExceptionNormalizerTest.php

+14 −2
Original line number Diff line number Diff line
@@ -4,17 +4,21 @@ 


namespace Drupal\Tests\jsonapi\Unit\Normalizer;



use Drupal\Core\Cache\Cache;

use Drupal\Core\Cache\Context\CacheContextsManager;

use Drupal\Core\Config\ConfigFactory;

use Drupal\Core\Config\ImmutableConfig;

use Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException;

use Drupal\Core\Session\AccountInterface;

use Drupal\jsonapi\Normalizer\HttpExceptionNormalizer;

use Drupal\jsonapi\Normalizer\Value\HttpExceptionNormalizerValue;

use Drupal\Tests\Core\Render\TestCacheableDependency;

use Drupal\Tests\UnitTestCase;

use PHPUnit\Framework\Attributes\CoversClass;

use PHPUnit\Framework\Attributes\Group;

use Symfony\Component\DependencyInjection\ContainerInterface;

use Symfony\Component\HttpFoundation\Request;

use Symfony\Component\HttpFoundation\RequestStack;

use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;



/**

 * Tests Drupal\jsonapi\Normalizer\HttpExceptionNormalizer.
@@ -40,12 +44,18 @@ public function testNormalize(): void { 
    $config_factory = $this->prophesize(ConfigFactory::class);

    $config_factory->get('system.logging')->willReturn($config->reveal());

    $container->get('config.factory')->willReturn($config_factory->reveal());

    $cache_contexts_manager = $this->prophesize(CacheContextsManager::class);

    $cache_contexts_manager->assertValidTokens(['user.permissions'])->willReturn(TRUE);

    $container->get('cache_contexts_manager')->willReturn($cache_contexts_manager->reveal());

    \Drupal::setContainer($container->reveal());

    $exception = new AccessDeniedHttpException('lorem', NULL, 13);

    $cacheability = new TestCacheableDependency([], [], Cache::PERMANENT);

    $exception = new CacheableAccessDeniedHttpException($cacheability, 'lorem', NULL, 13);

    $current_user = $this->prophesize(AccountInterface::class);

    $current_user->hasPermission('access site reports')->willReturn(TRUE);

    $normalizer = new HttpExceptionNormalizer($current_user->reveal());

    $normalized = $normalizer->normalize($exception, 'api_json');

    $this->assertInstanceOf(HttpExceptionNormalizerValue::class, $normalized);

    $this->assertEquals(0, $normalized->getCacheMaxAge());

    $normalized = $normalized->getNormalization();

    $error = $normalized[0];

    $this->assertNotEmpty($error['meta']);
@@ -61,6 +71,8 @@ public function testNormalize(): void { 
    $current_user->hasPermission('access site reports')->willReturn(FALSE);

    $normalizer = new HttpExceptionNormalizer($current_user->reveal());

    $normalized = $normalizer->normalize($exception, 'api_json');

    $this->assertInstanceOf(HttpExceptionNormalizerValue::class, $normalized);

    $this->assertEquals(Cache::PERMANENT, $normalized->getCacheMaxAge());

    $normalized = $normalized->getNormalization();

    $error = $normalized[0];

    $this->assertArrayNotHasKey('meta', $error);