Commit c9b15c8c authored by catch's avatar catch
Browse files

Issue #3444257 by bbrala, mxr576, quietone, acbramley, larowlan, casey, enyug,...

Issue #3444257 by bbrala, mxr576, quietone, acbramley, larowlan, casey, enyug, mstrelan, nick_schuch: ResourceObjectNormalizer::getNormalization can result in max-age drift when different sets of fields are requested

(cherry picked from commit 9bfcb369)
parent 2cc3c276
Loading
Loading
Loading
Loading
Loading
+34 −1
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Cache\VariationCacheInterface;
use Drupal\jsonapi\JsonApiResource\ResourceObject;
use Drupal\jsonapi\Normalizer\Value\CacheableNormalization;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\HttpKernel\Event\TerminateEvent;
@@ -102,7 +103,39 @@ public function get(ResourceObject $object) {
    }

    $cached = $this->variationCache->get($this->generateCacheKeys($object), new CacheableMetadata());
    return $cached ? $cached->data : FALSE;
    if (!$cached) {
      return FALSE;
    }

    // When a cache hit occurs, we first calculate the remaining time before the
    // cached record expires, ensuring that we do not reset the expiration with
    // one that might have been generated on an earlier timestamp. This is done
    // by subtracting the current timestamp from the cached record's expiration
    // timestamp. If the max-age is set, we adjust it by merging the calculated
    // remaining time with the original max-age of the cached item, ensuring
    // that the expiration remains accurate based on the current cache state
    // and timestamp.
    $normalizer_values = $cached->data;
    assert(is_array($normalizer_values));
    if ($cached->expire >= 0) {
      $max_age = max($cached->expire - $this->requestStack->getCurrentRequest()->server->get('REQUEST_TIME'), 0);
      $cacheability = new CacheableMetadata();
      $cacheability->setCacheMaxAge($max_age);

      $subsets = [
        ResourceObjectNormalizationCacher::RESOURCE_CACHE_SUBSET_BASE,
        ResourceObjectNormalizationCacher::RESOURCE_CACHE_SUBSET_FIELDS,
      ];
      foreach ($subsets as $subset) {
        foreach ($normalizer_values[$subset] as $name => $normalization) {
          assert($normalization instanceof CacheableNormalization);
          if ($normalization->getCacheMaxAge() > 0) {
            $normalizer_values[$subset][$name] = $normalization->withCacheableDependency($cacheability);
          }
        }
      }
    }
    return $normalizer_values;
  }

  /**
+65 −0
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@

use Drupal\Core\Cache\Cache;
use Drupal\Core\Cache\CacheableMetadata;
use Drupal\entity_test\Entity\EntityTestComputedField;
use Drupal\jsonapi\EventSubscriber\ResourceObjectNormalizationCacher;
use Drupal\jsonapi\JsonApiResource\ResourceObject;
use Drupal\jsonapi\Normalizer\Value\CacheableNormalization;
@@ -28,9 +29,11 @@ class ResourceObjectNormalizerCacherTest extends KernelTestBase {
   * {@inheritdoc}
   */
  protected static $modules = [
    'entity_test',
    'file',
    'system',
    'serialization',
    'text',
    'jsonapi',
    'user',
  ];
@@ -61,6 +64,7 @@ class ResourceObjectNormalizerCacherTest extends KernelTestBase {
   */
  protected function setUp(): void {
    parent::setUp();

    // Add the entity schemas.
    $this->installEntitySchema('user');
    // Add the additional table schemas.
@@ -108,4 +112,65 @@ public function testLinkNormalizationCacheability(): void {
    $this->assertFalse((bool) $this->cacher->get($resource_object));
  }

  /**
   * Tests that normalization max-age is correct.
   *
   * When max-age for a cached record is set the expiry is set accordingly. But
   * if the cached normalization is partially used in a later normalization the
   * max-age should be adjusted to a new timestamp.
   *
   * If we don't do this the expires of the cache record will be reset based on
   * the original max age. This leads to a drift in the expiry time of the
   * record.
   *
   * If a field tells the cache it should expire in exactly 1 hour, then if the
   * cached data is used 10 minutes later in another resource, that cache should
   * expire in 50 minutes and not reset to 60 minutes.
   */
  public function testMaxAgeCorrection(): void {
    $this->installEntitySchema('entity_test_computed_field');

    // Use EntityTestComputedField since ComputedTestCacheableStringItemList has a max age of 800
    $baseMaxAge = 800;
    $entity = EntityTestComputedField::create([]);
    $entity->save();
    $resource_type = $this->resourceTypeRepository->get($entity->getEntityTypeId(), $entity->bundle());
    $resource_object = ResourceObject::createFromEntity($resource_type, $entity);

    $resource_normalization = $this->serializer
      ->normalize($resource_object, 'api_json', ['account' => NULL]);
    $this->assertEquals($baseMaxAge, $resource_normalization->getCacheMaxAge());

    // Save the normalization to cache, this is done at TerminateEvent.
    $http_kernel = $this->prophesize(HttpKernelInterface::class);
    $request = $this->prophesize(Request::class);
    $response = $this->prophesize(Response::class);
    $event = new TerminateEvent($http_kernel->reveal(), $request->reveal(), $response->reveal());
    $this->cacher->onTerminate($event);

    // Change request time to 500 seconds later
    $current_request = \Drupal::requestStack()->getCurrentRequest();
    $current_request->server->set('REQUEST_TIME', $current_request->server->get('REQUEST_TIME') + 500);
    $resource_normalization = $this->serializer
      ->normalize($resource_object, 'api_json', ['account' => NULL]);
    $this->assertEquals($baseMaxAge - 500, $resource_normalization->getCacheMaxAge(), 'Max age should be 300 since 500 seconds has passed');

    // Change request time to 800 seconds later, this is the last second the
    // cache backend would return cached data. The max-age at that time should
    // be 0 which is the same as the expire time of the cache entry.
    $current_request->server->set('REQUEST_TIME', $current_request->server->get('REQUEST_TIME') + 800);
    $resource_normalization = $this->serializer
      ->normalize($resource_object, 'api_json', ['account' => NULL]);
    $this->assertEquals(0, $resource_normalization->getCacheMaxAge(), 'Max age should be 0 since max-age has passed');

    // Change request time to 801 seconds later. This validates that max-age
    // never becomes negative. This should never happen as the cache entry
    // is expired at this time and the cache backend would not return data.
    $current_request->server->set('REQUEST_TIME', $current_request->server->get('REQUEST_TIME') + 801);
    $resource_normalization = $this->serializer
      ->normalize($resource_object, 'api_json', ['account' => NULL]);
    $this->assertEquals(0, $resource_normalization->getCacheMaxAge(), 'Max age should be 0 since max-age has passed a second ago');

  }

}