From 5ff249f5e14f938700e01772aa4607c4882b4a92 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 30 Aug 2021 16:04:38 +0100 Subject: [PATCH] Issue #2997123 by jibran, Wim Leers, bbrala, gabesullice, quietone, axle_foley00, KapilV, acbramley, dpi, mglaman, tstoeckler, e0ipso, Sam152: Cacheability of normalized computed fields' properties is not captured during serialization --- .../EntityTestComputedFieldTest.php | 187 ++++++++++++++++++ .../Normalizer/PrimitiveDataNormalizer.php | 3 + .../src/Kernel/EntitySerializationTest.php | 19 ++ .../entity_test/entity_test.routing.yml | 8 + .../src/Entity/EntityTestComputedField.php | 12 +- .../DataType/ComputedTestCacheableString.php | 24 +++ .../ComputedTestCacheableStringItemList.php | 30 +++ .../ComputedTestCacheableStringItem.php | 37 ++++ .../EntityTestComputedFieldNormalizerTest.php | 107 ++++++++++ .../Rest/EntityTestResourceTestBase.php | 11 +- 10 files changed, 432 insertions(+), 6 deletions(-) create mode 100644 core/modules/jsonapi/tests/src/Functional/EntityTestComputedFieldTest.php create mode 100644 core/modules/system/tests/modules/entity_test/src/Plugin/DataType/ComputedTestCacheableString.php create mode 100644 core/modules/system/tests/modules/entity_test/src/Plugin/Field/ComputedTestCacheableStringItemList.php create mode 100644 core/modules/system/tests/modules/entity_test/src/Plugin/Field/FieldType/ComputedTestCacheableStringItem.php create mode 100644 core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestComputedFieldNormalizerTest.php diff --git a/core/modules/jsonapi/tests/src/Functional/EntityTestComputedFieldTest.php b/core/modules/jsonapi/tests/src/Functional/EntityTestComputedFieldTest.php new file mode 100644 index 000000000000..8cc53d049a9c --- /dev/null +++ b/core/modules/jsonapi/tests/src/Functional/EntityTestComputedFieldTest.php @@ -0,0 +1,187 @@ +<?php + +namespace Drupal\Tests\jsonapi\Functional; + +use Drupal\Core\Cache\Cache; +use Drupal\Core\Url; +use Drupal\entity_test\Entity\EntityTestComputedField; +use Drupal\user\Entity\User; + +/** + * JSON:API integration test for the "EntityTestComputedField" content entity type. + * + * @group jsonapi + */ +class EntityTestComputedFieldTest extends ResourceTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = ['entity_test']; + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; + + /** + * {@inheritdoc} + */ + protected static $entityTypeId = 'entity_test_computed_field'; + + /** + * {@inheritdoc} + */ + protected static $resourceTypeName = 'entity_test_computed_field--entity_test_computed_field'; + + /** + * {@inheritdoc} + */ + protected static $patchProtectedFieldNames = []; + + /** + * {@inheritdoc} + * + * @var \Drupal\entity_test\Entity\EntityTestComputedField + */ + protected $entity; + + /** + * {@inheritdoc} + */ + protected function setUpAuthorization($method) { + $this->grantPermissionsToTestedRole(['administer entity_test content']); + + switch ($method) { + case 'GET': + $this->grantPermissionsToTestedRole(['view test entity']); + break; + + case 'POST': + $this->grantPermissionsToTestedRole(['create entity_test entity_test_with_bundle entities']); + break; + + case 'PATCH': + case 'DELETE': + $this->grantPermissionsToTestedRole(['administer entity_test content']); + break; + } + } + + /** + * {@inheritdoc} + */ + protected function createEntity() { + $entity_test = EntityTestComputedField::create([ + 'name' => 'Llama', + 'type' => 'entity_test_computed_field', + ]); + + $entity_test->setOwnerId(0); + $entity_test->save(); + + return $entity_test; + } + + /** + * {@inheritdoc} + */ + protected function getExpectedDocument() { + $self_url = Url::fromUri('base:/jsonapi/entity_test_computed_field/entity_test_computed_field/' . $this->entity->uuid())->setAbsolute()->toString(TRUE)->getGeneratedUrl(); + $author = User::load(0); + return [ + 'jsonapi' => [ + 'meta' => [ + 'links' => [ + 'self' => ['href' => 'http://jsonapi.org/format/1.0/'], + ], + ], + 'version' => '1.0', + ], + 'links' => [ + 'self' => ['href' => $self_url], + ], + 'data' => [ + 'id' => $this->entity->uuid(), + 'type' => 'entity_test_computed_field--entity_test_computed_field', + 'links' => [ + 'self' => ['href' => $self_url], + ], + 'attributes' => [ + 'created' => (new \DateTime())->setTimestamp($this->entity->get('created')->value)->setTimezone(new \DateTimeZone('UTC'))->format(\DateTime::RFC3339), + 'name' => 'Llama', + 'drupal_internal__id' => 1, + 'computed_string_field' => NULL, + 'computed_test_cacheable_string_field' => 'computed test cacheable string field', + ], + 'relationships' => [ + 'computed_reference_field' => [ + 'data' => NULL, + 'links' => [ + 'related' => ['href' => $self_url . '/computed_reference_field'], + 'self' => ['href' => $self_url . '/relationships/computed_reference_field'], + ], + ], + 'user_id' => [ + 'data' => [ + 'id' => $author->uuid(), + 'meta' => [ + 'drupal_internal__target_id' => (int) $author->id(), + ], + 'type' => 'user--user', + ], + 'links' => [ + 'related' => ['href' => $self_url . '/user_id'], + 'self' => ['href' => $self_url . '/relationships/user_id'], + ], + ], + ], + ], + ]; + } + + /** + * {@inheritdoc} + */ + protected function getPostDocument() { + return [ + 'data' => [ + 'type' => 'entity_test_computed_field--entity_test_computed_field', + 'attributes' => [ + 'name' => 'Dramallama', + ], + ], + ]; + } + + /** + * {@inheritdoc} + */ + protected function getSparseFieldSets() { + // EntityTest's owner field name is `user_id`, not `uid`, which breaks + // nested sparse fieldset tests. + return array_diff_key(parent::getSparseFieldSets(), array_flip([ + 'nested_empty_fieldset', + 'nested_fieldset_with_owner_fieldset', + ])); + } + + protected function getExpectedCacheContexts(array $sparse_fieldset = NULL) { + $cache_contexts = parent::getExpectedCacheContexts($sparse_fieldset); + if ($sparse_fieldset === NULL || in_array('computed_test_cacheable_string_field', $sparse_fieldset)) { + $cache_contexts = Cache::mergeContexts($cache_contexts, ['url.query_args:computed_test_cacheable_string_field']); + } + + return $cache_contexts; + } + + protected function getExpectedCacheTags(array $sparse_fieldset = NULL) { + $expected_cache_tags = parent::getExpectedCacheTags($sparse_fieldset); + if ($sparse_fieldset === NULL || in_array('computed_test_cacheable_string_field', $sparse_fieldset)) { + $expected_cache_tags = Cache::mergeTags($expected_cache_tags, ['field:computed_test_cacheable_string_field']); + } + + return $expected_cache_tags; + } + +} diff --git a/core/modules/serialization/src/Normalizer/PrimitiveDataNormalizer.php b/core/modules/serialization/src/Normalizer/PrimitiveDataNormalizer.php index a594757e7dca..873fbbb11392 100644 --- a/core/modules/serialization/src/Normalizer/PrimitiveDataNormalizer.php +++ b/core/modules/serialization/src/Normalizer/PrimitiveDataNormalizer.php @@ -21,6 +21,9 @@ class PrimitiveDataNormalizer extends NormalizerBase { * {@inheritdoc} */ public function normalize($object, $format = NULL, array $context = []) { + // Add cacheability if applicable. + $this->addCacheableDependency($context, $object); + $parent = $object->getParent(); if ($parent instanceof FieldItemInterface && $object->getValue()) { $serialized_property_names = $this->getCustomSerializedPropertyNames($parent); diff --git a/core/modules/serialization/tests/src/Kernel/EntitySerializationTest.php b/core/modules/serialization/tests/src/Kernel/EntitySerializationTest.php index 2a09246428c4..47e8ca222b06 100644 --- a/core/modules/serialization/tests/src/Kernel/EntitySerializationTest.php +++ b/core/modules/serialization/tests/src/Kernel/EntitySerializationTest.php @@ -2,11 +2,15 @@ namespace Drupal\Tests\serialization\Kernel; +use Drupal\Core\Cache\CacheableDependencyInterface; +use Drupal\Core\Cache\CacheableMetadata; +use Drupal\entity_test\Entity\EntityTestComputedField; use Drupal\Component\Serialization\Json; use Drupal\Component\Datetime\DateTimePlus; use Drupal\entity_test\Entity\EntitySerializedField; use Drupal\entity_test\Entity\EntityTestMulRev; use Drupal\filter\Entity\FilterFormat; +use Drupal\serialization\Normalizer\CacheableNormalizerInterface; /** * Tests that entities can be serialized to supported core formats. @@ -365,4 +369,19 @@ public function testDenormalizeStringValue() { ], EntitySerializedField::class); } + /** + * Tests normalizing cacheable computed field. + */ + public function testCacheableComputedField() { + $context[CacheableNormalizerInterface::SERIALIZATION_CONTEXT_CACHEABILITY] = new CacheableMetadata(); + $entity = EntityTestComputedField::create(); + $normalized = $this->serializer->normalize($entity, NULL, $context); + $this->assertEquals('computed test cacheable string field', $normalized['computed_test_cacheable_string_field'][0]['value']); + $this->assertInstanceOf(CacheableDependencyInterface::class, $context[CacheableNormalizerInterface::SERIALIZATION_CONTEXT_CACHEABILITY]); + // See \Drupal\entity_test\Plugin\Field\ComputedTestCacheableStringItemList::computeValue(). + $this->assertEquals($context[CacheableNormalizerInterface::SERIALIZATION_CONTEXT_CACHEABILITY]->getCacheContexts(), ['url.query_args:computed_test_cacheable_string_field']); + $this->assertEquals($context[CacheableNormalizerInterface::SERIALIZATION_CONTEXT_CACHEABILITY]->getCacheTags(), ['field:computed_test_cacheable_string_field']); + $this->assertEquals($context[CacheableNormalizerInterface::SERIALIZATION_CONTEXT_CACHEABILITY]->getCacheMaxAge(), 800); + } + } diff --git a/core/modules/system/tests/modules/entity_test/entity_test.routing.yml b/core/modules/system/tests/modules/entity_test/entity_test.routing.yml index 5b6d37b946a4..f08c0902d8ec 100644 --- a/core/modules/system/tests/modules/entity_test/entity_test.routing.yml +++ b/core/modules/system/tests/modules/entity_test/entity_test.routing.yml @@ -102,5 +102,13 @@ entity.entity_test_view_builder.canonical: requirements: _access: 'TRUE' +entity.entity_test_computed_field.canonical: + path: '/entity_test_computed_field/{entity_test_computed_field}' + defaults: + _entity_view: 'entity_test_computed_field.full' + _title: 'Test full view mode' + requirements: + _entity_access: 'entity_test_computed_field.view' + route_callbacks: - '\Drupal\entity_test\Routing\EntityTestRoutes::routes' diff --git a/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestComputedField.php b/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestComputedField.php index 187effe50456..6cf035700278 100644 --- a/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestComputedField.php +++ b/core/modules/system/tests/modules/entity_test/src/Entity/EntityTestComputedField.php @@ -4,7 +4,9 @@ use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\BaseFieldDefinition; +use Drupal\Core\StringTranslation\TranslatableMarkup; use Drupal\entity_test\Plugin\Field\ComputedReferenceTestFieldItemList; +use Drupal\entity_test\Plugin\Field\ComputedTestCacheableStringItemList; use Drupal\entity_test\Plugin\Field\ComputedTestFieldItemList; /** @@ -19,11 +21,12 @@ * }, * entity_keys = { * "id" = "id", + * "uuid" = "uuid", * "label" = "name", * }, * admin_permission = "administer entity_test content", * links = { - * "add-form" = "/entity_test_computed_field/add", + * "canonical" = "/entity_test_computed_field/{entity_test_computed_field}", * }, * ) */ @@ -46,6 +49,13 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) { ->setSetting('target_type', 'entity_test') ->setClass(ComputedReferenceTestFieldItemList::class); + $fields['computed_test_cacheable_string_field'] = BaseFieldDefinition::create('computed_test_cacheable_string_item') + ->setLabel(new TranslatableMarkup('Computed Cacheable String Field Test')) + ->setComputed(TRUE) + ->setClass(ComputedTestCacheableStringItemList::class) + ->setReadOnly(FALSE) + ->setInternal(FALSE); + return $fields; } diff --git a/core/modules/system/tests/modules/entity_test/src/Plugin/DataType/ComputedTestCacheableString.php b/core/modules/system/tests/modules/entity_test/src/Plugin/DataType/ComputedTestCacheableString.php new file mode 100644 index 000000000000..2bb7ca821c84 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test/src/Plugin/DataType/ComputedTestCacheableString.php @@ -0,0 +1,24 @@ +<?php + +namespace Drupal\entity_test\Plugin\DataType; + +use Drupal\Core\Cache\RefinableCacheableDependencyInterface; +use Drupal\Core\Cache\RefinableCacheableDependencyTrait; +use Drupal\Core\TypedData\Plugin\DataType\StringData; + +/** + * The string data type with cacheability metadata. + * + * The plain value of a string is a regular PHP string. For setting the value + * any PHP variable that casts to a string may be passed. + * + * @DataType( + * id = "computed_test_cacheable_string", + * label = @Translation("Computed Test Cacheable String") + * ) + */ +class ComputedTestCacheableString extends StringData implements RefinableCacheableDependencyInterface { + + use RefinableCacheableDependencyTrait; + +} diff --git a/core/modules/system/tests/modules/entity_test/src/Plugin/Field/ComputedTestCacheableStringItemList.php b/core/modules/system/tests/modules/entity_test/src/Plugin/Field/ComputedTestCacheableStringItemList.php new file mode 100644 index 000000000000..21aaa9f7b0c7 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test/src/Plugin/Field/ComputedTestCacheableStringItemList.php @@ -0,0 +1,30 @@ +<?php + +namespace Drupal\entity_test\Plugin\Field; + +use Drupal\Core\Cache\CacheableMetadata; +use Drupal\Core\Field\FieldItemList; +use Drupal\Core\TypedData\ComputedItemListTrait; + +/** + * Item list class for computed cacheable string field. + */ +class ComputedTestCacheableStringItemList extends FieldItemList { + + use ComputedItemListTrait; + + /** + * {@inheritdoc} + */ + protected function computeValue() { + /** @var \Drupal\entity_test\Plugin\Field\FieldType\ComputedTestCacheableStringItem $item */ + $item = $this->createItem(0, 'computed test cacheable string field'); + $cacheability = (new CacheableMetadata()) + ->setCacheContexts(['url.query_args:computed_test_cacheable_string_field']) + ->setCacheTags(['field:computed_test_cacheable_string_field']) + ->setCacheMaxAge(800); + $item->get('value')->addCacheableDependency($cacheability); + $this->list[0] = $item; + } + +} diff --git a/core/modules/system/tests/modules/entity_test/src/Plugin/Field/FieldType/ComputedTestCacheableStringItem.php b/core/modules/system/tests/modules/entity_test/src/Plugin/Field/FieldType/ComputedTestCacheableStringItem.php new file mode 100644 index 000000000000..06da5ea3ab56 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test/src/Plugin/Field/FieldType/ComputedTestCacheableStringItem.php @@ -0,0 +1,37 @@ +<?php + +namespace Drupal\entity_test\Plugin\Field\FieldType; + +use Drupal\Core\Field\FieldStorageDefinitionInterface; +use Drupal\Core\Field\Plugin\Field\FieldType\StringItem; +use Drupal\Core\StringTranslation\TranslatableMarkup; +use Drupal\Core\TypedData\DataDefinition; + +/** + * Defines the 'string' entity field type with cacheability metadata. + * + * @FieldType( + * id = "computed_test_cacheable_string_item", + * label = @Translation("Test Text (plain string with cacheability)"), + * description = @Translation("A test field containing a plain string value and cacheability metadata."), + * category = @Translation("Text"), + * no_ui = TRUE, + * default_widget = "string_textfield", + * default_formatter = "string" + * ) + */ +class ComputedTestCacheableStringItem extends StringItem { + + /** + * {@inheritdoc} + */ + public static function propertyDefinitions(FieldStorageDefinitionInterface $field_definition) { + $properties['value'] = DataDefinition::create('computed_test_cacheable_string') + ->setLabel(new TranslatableMarkup('Text value')) + ->setSetting('case_sensitive', $field_definition->getSetting('case_sensitive')) + ->setRequired(TRUE); + + return $properties; + } + +} diff --git a/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestComputedFieldNormalizerTest.php b/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestComputedFieldNormalizerTest.php new file mode 100644 index 000000000000..b6c914e32d09 --- /dev/null +++ b/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestComputedFieldNormalizerTest.php @@ -0,0 +1,107 @@ +<?php + +namespace Drupal\Tests\entity_test\Functional\Rest; + +use Drupal\Core\Cache\Cache; +use Drupal\Tests\rest\Functional\AnonResourceTestTrait; + +/** + * Test normalization of computed field. + * + * @group rest + */ +class EntityTestComputedFieldNormalizerTest extends EntityTestResourceTestBase { + + use AnonResourceTestTrait; + + /** + * {@inheritdoc} + */ + protected static $entityTypeId = 'entity_test_computed_field'; + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'stark'; + + /** + * {@inheritdoc} + */ + protected function setUpAuthorization($method) { + $this->grantPermissionsToTestedRole(['administer entity_test content']); + } + + /** + * {@inheritdoc} + */ + protected function getExpectedUnauthorizedAccessMessage($method) { + if ($this->config('rest.settings')->get('bc_entity_resource_permissions')) { + return parent::getExpectedUnauthorizedAccessMessage($method); + } + + return "The 'administer entity_test content' permission is required."; + } + + /** + * {@inheritdoc} + */ + protected function getExpectedNormalizedEntity() { + $expected = parent::getExpectedNormalizedEntity(); + $expected['computed_reference_field'] = []; + $expected['computed_string_field'] = []; + unset($expected['field_test_text'], $expected['langcode'], $expected['type'], $expected['uuid']); + // @see \Drupal\entity_test\Plugin\Field\ComputedTestCacheableStringItemList::computeValue(). + $expected['computed_test_cacheable_string_field'] = [ + [ + 'value' => 'computed test cacheable string field', + ], + ]; + + $expected['uuid'] = [ + 0 => [ + 'value' => $this->entity->uuid(), + ], + ]; + + return $expected; + } + + /** + * {@inheritdoc} + */ + protected function getExpectedCacheContexts() { + return Cache::mergeContexts(parent::getExpectedCacheContexts(), ['url.query_args:computed_test_cacheable_string_field']); + } + + /** + * {@inheritdoc} + */ + protected function getExpectedCacheTags() { + return Cache::mergeTags(parent::getExpectedCacheTags(), ['field:computed_test_cacheable_string_field']); + } + + /** + * {@inheritdoc} + */ + public function testPost() { + // Post test not required. + $this->markTestSkipped(); + } + + /** + * {@inheritdoc} + */ + public function testPatch() { + // Patch test not required. + $this->markTestSkipped(); + } + + /** + * {@inheritdoc} + */ + public function testDelete() { + // Delete test not required. + $this->markTestSkipped(); + } + +} diff --git a/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestResourceTestBase.php b/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestResourceTestBase.php index 836fb3e6e030..621ae5adb74a 100644 --- a/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestResourceTestBase.php +++ b/core/modules/system/tests/modules/entity_test/tests/src/Functional/Rest/EntityTestResourceTestBase.php @@ -2,7 +2,6 @@ namespace Drupal\Tests\entity_test\Functional\Rest; -use Drupal\entity_test\Entity\EntityTest; use Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase; use Drupal\Tests\system\Functional\Entity\Traits\EntityDefinitionTestTrait; use Drupal\user\Entity\User; @@ -60,9 +59,11 @@ protected function createEntity() { $this->container->get('state')->set('entity_test.internal_field', TRUE); $this->applyEntityUpdates('entity_test'); - $entity_test = EntityTest::create([ + $entity_test = \Drupal::entityTypeManager() + ->getStorage(static::$entityTypeId) + ->create([ 'name' => 'Llama', - 'type' => 'entity_test', + 'type' => static::$entityTypeId, // Set a value for the internal field to confirm that it will not be // returned in normalization. // @see entity_test_entity_base_field_info(). @@ -99,7 +100,7 @@ protected function getExpectedNormalizedEntity() { ], 'type' => [ [ - 'value' => 'entity_test', + 'value' => static::$entityTypeId, ], ], 'name' => [ @@ -134,7 +135,7 @@ protected function getNormalizedPostEntity() { return [ 'type' => [ [ - 'value' => 'entity_test', + 'value' => static::$entityTypeId, ], ], 'name' => [ -- GitLab