From 12b47612f2cd98fecde5514454a93d257fb5f4a8 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Fri, 20 Sep 2019 10:34:40 +0100 Subject: [PATCH] Issue #3070204 by gabesullice, ravi.shankar, Wim Leers, mglaman, catch, larowlan, jibran: Refactor the JSON:API FieldResolver to use a resource type instead of an entity type ID and bundle ID pair --- .../jsonapi/src/Context/FieldResolver.php | 22 ++++++++++++----- .../jsonapi/src/Controller/EntityResource.php | 2 +- core/modules/jsonapi/src/Query/Filter.php | 2 +- .../src/Kernel/Context/FieldResolverTest.php | 6 +++-- .../tests/src/Kernel/Query/FilterTest.php | 24 ++++++++++++------- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/core/modules/jsonapi/src/Context/FieldResolver.php b/core/modules/jsonapi/src/Context/FieldResolver.php index e41b8527d1b6..ef58a086ddab 100644 --- a/core/modules/jsonapi/src/Context/FieldResolver.php +++ b/core/modules/jsonapi/src/Context/FieldResolver.php @@ -251,10 +251,8 @@ public static function resolveInternalIncludePath(ResourceType $resource_type, a * 'uid.field_first_name' -> 'uid.entity.field_first_name'. * 'author.firstName' -> 'field_author.entity.field_first_name' * - * @param string $entity_type_id - * The type of the entity for which to resolve the field name. - * @param string $bundle - * The bundle of the entity for which to resolve the field name. + * @param \Drupal\jsonapi\ResourceType\ResourceType $resource_type + * The JSON:API resource type from which to resolve the field name. * @param string $external_field_name * The public field name to map to a Drupal field name. * @@ -263,9 +261,21 @@ public static function resolveInternalIncludePath(ResourceType $resource_type, a * * @throws \Drupal\Core\Http\Exception\CacheableBadRequestHttpException */ - public function resolveInternalEntityQueryPath($entity_type_id, $bundle, $external_field_name) { + public function resolveInternalEntityQueryPath($resource_type, $external_field_name) { + $function_args = func_get_args(); + // @todo Remove this conditional block in drupal:9.0.0 and add a type hint + // to the first argument of this method. + // @see https://www.drupal.org/project/drupal/issues/3078045 + if (count($function_args) === 3) { + @trigger_error('Passing the entity type ID and bundle to ' . __METHOD__ . ' is deprecated in drupal:8.8.0 and will throw a fatal error in drupal:9.0.0. Pass a JSON:API resource type instead. See https://www.drupal.org/node/3078036', E_USER_DEPRECATED); + list($entity_type_id, $bundle, $external_field_name) = $function_args; + $resource_type = $this->resourceTypeRepository->get($entity_type_id, $bundle); + } + elseif (!$resource_type instanceof ResourceType) { + throw new \InvalidArgumentException("The first argument to " . __METHOD__ . " should be an instance of \Drupal\jsonapi\ResourceType\ResourceType, " . gettype($resource_type) . " given."); + } + $cacheability = (new CacheableMetadata())->addCacheContexts(['url.query_args:filter', 'url.query_args:sort']); - $resource_type = $this->resourceTypeRepository->get($entity_type_id, $bundle); if (empty($external_field_name)) { throw new CacheableBadRequestHttpException($cacheability, 'No field name was provided for the filter.'); } diff --git a/core/modules/jsonapi/src/Controller/EntityResource.php b/core/modules/jsonapi/src/Controller/EntityResource.php index ca76ab31adf4..24c0dadf7a4c 100644 --- a/core/modules/jsonapi/src/Controller/EntityResource.php +++ b/core/modules/jsonapi/src/Controller/EntityResource.php @@ -877,7 +877,7 @@ protected function getCollectionQuery(ResourceType $resource_type, array $params // Apply any sorts to the entity query. if (isset($params[Sort::KEY_NAME]) && $sort = $params[Sort::KEY_NAME]) { foreach ($sort->fields() as $field) { - $path = $this->fieldResolver->resolveInternalEntityQueryPath($resource_type->getEntityTypeId(), $resource_type->getBundle(), $field[Sort::PATH_KEY]); + $path = $this->fieldResolver->resolveInternalEntityQueryPath($resource_type, $field[Sort::PATH_KEY]); $direction = isset($field[Sort::DIRECTION_KEY]) ? $field[Sort::DIRECTION_KEY] : 'ASC'; $langcode = isset($field[Sort::LANGUAGE_KEY]) ? $field[Sort::LANGUAGE_KEY] : NULL; $query->sort($path, $direction, $langcode); diff --git a/core/modules/jsonapi/src/Query/Filter.php b/core/modules/jsonapi/src/Query/Filter.php index 58d398582d05..e87f0081a45a 100644 --- a/core/modules/jsonapi/src/Query/Filter.php +++ b/core/modules/jsonapi/src/Query/Filter.php @@ -157,7 +157,7 @@ public static function createFromQueryParameter($parameter, ResourceType $resour foreach ($expanded as &$filter_item) { if (isset($filter_item[static::CONDITION_KEY][EntityCondition::PATH_KEY])) { $unresolved = $filter_item[static::CONDITION_KEY][EntityCondition::PATH_KEY]; - $filter_item[static::CONDITION_KEY][EntityCondition::PATH_KEY] = $field_resolver->resolveInternalEntityQueryPath($resource_type->getEntityTypeId(), $resource_type->getBundle(), $unresolved); + $filter_item[static::CONDITION_KEY][EntityCondition::PATH_KEY] = $field_resolver->resolveInternalEntityQueryPath($resource_type, $unresolved); } } return new static(static::buildEntityConditionGroup($expanded)); diff --git a/core/modules/jsonapi/tests/src/Kernel/Context/FieldResolverTest.php b/core/modules/jsonapi/tests/src/Kernel/Context/FieldResolverTest.php index e1ebc3c57a67..62fb3befa01d 100644 --- a/core/modules/jsonapi/tests/src/Kernel/Context/FieldResolverTest.php +++ b/core/modules/jsonapi/tests/src/Kernel/Context/FieldResolverTest.php @@ -168,7 +168,8 @@ public function resolveInternalIncludePathErrorProvider() { * @dataProvider resolveInternalEntityQueryPathProvider */ public function testResolveInternalEntityQueryPath($expect, $external_path, $entity_type_id = 'entity_test_with_bundle', $bundle = 'bundle1') { - $this->assertEquals($expect, $this->sut->resolveInternalEntityQueryPath($entity_type_id, $bundle, $external_path)); + $resource_type = $this->resourceTypeRepository->get($entity_type_id, $bundle); + $this->assertEquals($expect, $this->sut->resolveInternalEntityQueryPath($resource_type, $external_path)); } /** @@ -252,7 +253,8 @@ public function testResolveInternalEntityQueryPathError($entity_type, $bundle, $ if (!empty($expected_message)) { $this->expectExceptionMessage($expected_message); } - $this->sut->resolveInternalEntityQueryPath($entity_type, $bundle, $external_path); + $resource_type = $this->resourceTypeRepository->get($entity_type, $bundle); + $this->sut->resolveInternalEntityQueryPath($resource_type, $external_path); } /** diff --git a/core/modules/jsonapi/tests/src/Kernel/Query/FilterTest.php b/core/modules/jsonapi/tests/src/Kernel/Query/FilterTest.php index 74e9c291a4f2..2a644615a244 100644 --- a/core/modules/jsonapi/tests/src/Kernel/Query/FilterTest.php +++ b/core/modules/jsonapi/tests/src/Kernel/Query/FilterTest.php @@ -46,6 +46,13 @@ class FilterTest extends JsonapiKernelTestBase { */ protected $nodeStorage; + /** + * The JSON:API resource type repository. + * + * @var \Drupal\jsonapi\ResourceType\ResourceTypeRepositoryInterface + */ + protected $resourceTypeRepository; + /** * {@inheritdoc} */ @@ -68,6 +75,7 @@ public function setUp() { $this->nodeStorage = $this->container->get('entity_type.manager')->getStorage('node'); $this->fieldResolver = $this->container->get('jsonapi.field_resolver'); + $this->resourceTypeRepository = $this->container->get('jsonapi.resource_type.repository'); } /** @@ -76,7 +84,7 @@ public function setUp() { public function testInvalidFilterPathDueToMissingPropertyName() { $this->expectException(CacheableBadRequestHttpException::class); $this->expectExceptionMessage('Invalid nested filtering. The field `colors`, given in the path `colors` is incomplete, it must end with one of the following specifiers: `value`, `format`, `processed`.'); - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); Filter::createFromQueryParameter(['colors' => ''], $resource_type, $this->fieldResolver); } @@ -86,7 +94,7 @@ public function testInvalidFilterPathDueToMissingPropertyName() { public function testInvalidFilterPathDueToMissingPropertyNameReferenceFieldWithMetaProperties() { $this->expectException(CacheableBadRequestHttpException::class); $this->expectExceptionMessage('Invalid nested filtering. The field `photo`, given in the path `photo` is incomplete, it must end with one of the following specifiers: `id`, `meta.alt`, `meta.title`, `meta.width`, `meta.height`.'); - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); Filter::createFromQueryParameter(['photo' => ''], $resource_type, $this->fieldResolver); } @@ -96,7 +104,7 @@ public function testInvalidFilterPathDueToMissingPropertyNameReferenceFieldWithM public function testInvalidFilterPathDueMissingMetaPrefixReferenceFieldWithMetaProperties() { $this->expectException(CacheableBadRequestHttpException::class); $this->expectExceptionMessage('Invalid nested filtering. The property `alt`, given in the path `photo.alt` belongs to the meta object of a relationship and must be preceded by `meta`.'); - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); Filter::createFromQueryParameter(['photo.alt' => ''], $resource_type, $this->fieldResolver); } @@ -106,7 +114,7 @@ public function testInvalidFilterPathDueMissingMetaPrefixReferenceFieldWithMetaP public function testInvalidFilterPathDueToMissingPropertyNameReferenceFieldWithoutMetaProperties() { $this->expectException(CacheableBadRequestHttpException::class); $this->expectExceptionMessage('Invalid nested filtering. The field `uid`, given in the path `uid` is incomplete, it must end with one of the following specifiers: `id`.'); - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); Filter::createFromQueryParameter(['uid' => ''], $resource_type, $this->fieldResolver); } @@ -116,7 +124,7 @@ public function testInvalidFilterPathDueToMissingPropertyNameReferenceFieldWitho public function testInvalidFilterPathDueToNonexistentProperty() { $this->expectException(CacheableBadRequestHttpException::class); $this->expectExceptionMessage('Invalid nested filtering. The property `foobar`, given in the path `colors.foobar`, does not exist. Must be one of the following property names: `value`, `format`, `processed`.'); - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); Filter::createFromQueryParameter(['colors.foobar' => ''], $resource_type, $this->fieldResolver); } @@ -126,7 +134,7 @@ public function testInvalidFilterPathDueToNonexistentProperty() { public function testInvalidFilterPathDueToElidedSoleProperty() { $this->expectException(CacheableBadRequestHttpException::class); $this->expectExceptionMessage('Invalid nested filtering. The property `value`, given in the path `promote.value`, does not exist. Filter by `promote`, not `promote.value` (the JSON:API module elides property names from single-property fields).'); - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); Filter::createFromQueryParameter(['promote.value' => ''], $resource_type, $this->fieldResolver); } @@ -155,7 +163,7 @@ public function testQueryCondition() { return (string) $p->getValue($entity_query); }; - $resource_type = new ResourceType('node', 'painting', NULL); + $resource_type = $this->resourceTypeRepository->get('node', 'painting'); foreach ($data as $case) { $parameter = $case[0]; $expected_query = $case[1]; @@ -405,7 +413,7 @@ public function testCreateFromQueryParameterNested() { */ protected function getFieldResolverMock(ResourceType $resource_type) { $field_resolver = $this->prophesize(FieldResolver::class); - $field_resolver->resolveInternalEntityQueryPath($resource_type->getEntityTypeId(), $resource_type->getBundle(), Argument::any())->willReturnArgument(2); + $field_resolver->resolveInternalEntityQueryPath($resource_type, Argument::any())->willReturnArgument(1); return $field_resolver->reveal(); } -- GitLab