From 090a1cf06496b66d3150086d9985bb32c8fc5476 Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Tue, 17 Nov 2020 10:07:24 +0000 Subject: [PATCH] Issue #2996114 by BR0kEN, gabesullice, jungle, ARUN AK, tengoku, Wim Leers, nishantkumar155, lawxen, bbrala, mglaman, Morbus Iff, larowlan, joachim, mxr576: Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, NULL given (cherry picked from commit e82556c355334744641f38b8eb356ed0c30c4430) --- .../ResourceType/ResourceTypeRepository.php | 67 +++++++++++--- ...onapi_test_resource_type_aliasing.info.yml | 3 + ...i_test_resource_type_aliasing.services.yml | 5 ++ .../src/ResourceType/AliasedResourceType.php | 33 +++++++ .../AliasingResourceTypeRepository.php | 24 +++++ .../ResourceType/RelatedResourceTypesTest.php | 32 +++++++ .../ResourceTypeNameAliasTest.php | 87 +++++++++++++++++++ .../ResourceTypeRepositoryTest.php | 10 +++ 8 files changed, 250 insertions(+), 11 deletions(-) create mode 100644 core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.info.yml create mode 100644 core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.services.yml create mode 100644 core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasedResourceType.php create mode 100644 core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasingResourceTypeRepository.php create mode 100644 core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeNameAliasTest.php diff --git a/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php index c2d306d4b9f1..fe508b970806 100644 --- a/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php @@ -14,6 +14,7 @@ use Drupal\Core\Entity\EntityTypeManagerInterface; use Drupal\Core\Entity\FieldableEntityInterface; use Drupal\Core\Field\FieldDefinitionInterface; +use Drupal\Core\Installer\InstallerKernel; use Drupal\Core\TypedData\DataReferenceTargetDefinition; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpKernel\Exception\PreconditionFailedHttpException; @@ -179,7 +180,7 @@ public function get($entity_type_id, $bundle) { throw new PreconditionFailedHttpException('Server error. The current route is malformed.'); } - return $this->getByTypeName("$entity_type_id--$bundle"); + return static::lookupResourceType($this->all(), $entity_type_id, $bundle); } /** @@ -423,19 +424,34 @@ protected function calculateRelatableResourceTypes(ResourceType $resource_type, */ protected function getRelatableResourceTypesFromFieldDefinition(FieldDefinitionInterface $field_definition, array $resource_types) { $item_definition = $field_definition->getItemDefinition(); - $entity_type_id = $item_definition->getSetting('target_type'); $handler_settings = $item_definition->getSetting('handler_settings'); + $target_bundles = empty($handler_settings['target_bundles']) ? $this->getAllBundlesForEntityType($entity_type_id) : $handler_settings['target_bundles']; + $relatable_resource_types = []; - $has_target_bundles = isset($handler_settings['target_bundles']) && !empty($handler_settings['target_bundles']); - $target_bundles = $has_target_bundles ? - $handler_settings['target_bundles'] - : $this->getAllBundlesForEntityType($entity_type_id); + foreach ($target_bundles as $target_bundle) { + if ($resource_type = static::lookupResourceType($resource_types, $entity_type_id, $target_bundle)) { + $relatable_resource_types[] = $resource_type; + } + // Do not warn during the site installation since system integrity + // is not guaranteed in this period and the warnings may pop up falsy, + // adding confusion to the process. + elseif (!InstallerKernel::installationAttempted()) { + trigger_error( + sprintf( + 'The "%s" at "%s:%s" references the "%s:%s" entity type that does not exist. Please take action.', + $field_definition->getName(), + $field_definition->getTargetEntityTypeId(), + $field_definition->getTargetBundle(), + $entity_type_id, + $target_bundle + ), + E_USER_WARNING + ); + } + } - return array_map(function ($target_bundle) use ($entity_type_id, $resource_types) { - $type_name = "$entity_type_id--$target_bundle"; - return isset($resource_types[$type_name]) ? $resource_types[$type_name] : NULL; - }, $target_bundles); + return $relatable_resource_types; } /** @@ -473,7 +489,36 @@ protected function isReferenceFieldDefinition(FieldDefinitionInterface $field_de * The bundle IDs. */ protected function getAllBundlesForEntityType($entity_type_id) { - return array_keys($this->entityTypeBundleInfo->getBundleInfo($entity_type_id)); + // Ensure all keys are strings because numeric values are allowed as bundle + // names and "array_keys()" casts "42" to 42. + return array_map('strval', array_keys($this->entityTypeBundleInfo->getBundleInfo($entity_type_id))); + } + + /** + * Lookup a resource type by entity type ID and bundle name. + * + * @param \Drupal\jsonapi\ResourceType\ResourceType[] $resource_types + * The list of resource types to do a lookup. + * @param string $entity_type_id + * The entity type of a seekable resource type. + * @param string $bundle + * The entity bundle of a seekable resource type. + * + * @return \Drupal\jsonapi\ResourceType\ResourceType|null + * The resource type or NULL if one cannot be found. + */ + protected static function lookupResourceType(array $resource_types, $entity_type_id, $bundle) { + if (isset($resource_types["$entity_type_id--$bundle"])) { + return $resource_types["$entity_type_id--$bundle"]; + } + + foreach ($resource_types as $resource_type) { + if ($resource_type->getEntityTypeId() === $entity_type_id && $resource_type->getBundle() === $bundle) { + return $resource_type; + } + } + + return NULL; } } diff --git a/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.info.yml b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.info.yml new file mode 100644 index 000000000000..828588185d23 --- /dev/null +++ b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.info.yml @@ -0,0 +1,3 @@ +name: 'JSON:API test resource type aliasing' +type: module +package: Testing diff --git a/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.services.yml b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.services.yml new file mode 100644 index 000000000000..9c02e50e3461 --- /dev/null +++ b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/jsonapi_test_resource_type_aliasing.services.yml @@ -0,0 +1,5 @@ +services: + Drupal\jsonapi_test_resource_type_aliasing\ResourceType\AliasingResourceTypeRepository: + decorates: jsonapi.resource_type.repository + parent: jsonapi.resource_type.repository + public: false diff --git a/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasedResourceType.php b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasedResourceType.php new file mode 100644 index 000000000000..761dc8ee59d1 --- /dev/null +++ b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasedResourceType.php @@ -0,0 +1,33 @@ +<?php + +namespace Drupal\jsonapi_test_resource_type_aliasing\ResourceType; + +use Drupal\jsonapi\ResourceType\ResourceType; +use Drupal\jsonapi\ResourceType\ResourceType as ResourceTypeBase; + +/** + * Resource type with uses an alternative type name. + * + * @todo remove this class in https://www.drupal.org/project/drupal/issues/3105318 + */ +class AliasedResourceType extends ResourceTypeBase { + + /** + * {@inheritdoc} + */ + public function __construct(ResourceType $resource_type, string $alias) { + parent::__construct( + $resource_type->getEntityTypeId(), + $resource_type->getBundle(), + $resource_type->getDeserializationTargetClass(), + $resource_type->isInternal(), + $resource_type->isLocatable(), + $resource_type->isMutable(), + $resource_type->isVersionable(), + $resource_type->getFields() + ); + // Alias the resource type name with an alternative pattern. + $this->typeName = $alias; + } + +} diff --git a/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasingResourceTypeRepository.php b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasingResourceTypeRepository.php new file mode 100644 index 000000000000..6b8bc234db62 --- /dev/null +++ b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_aliasing/src/ResourceType/AliasingResourceTypeRepository.php @@ -0,0 +1,24 @@ +<?php + +namespace Drupal\jsonapi_test_resource_type_aliasing\ResourceType; + +use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\jsonapi\ResourceType\ResourceTypeRepository as ResourceTypeRepositoryBase; + +/** + * Provides JSON:API resource types with a different type name pattern. + * + * @todo remove this class in https://www.drupal.org/project/drupal/issues/3105318 + */ +class AliasingResourceTypeRepository extends ResourceTypeRepositoryBase { + + /** + * {@inheritdoc} + */ + protected function createResourceType(EntityTypeInterface $entity_type, $bundle) { + $alias = sprintf('%s==%s', $entity_type->id(), $bundle); + $base_resource_type = parent::createResourceType($entity_type, $bundle); + return new AliasedResourceType($base_resource_type, $alias); + } + +} diff --git a/core/modules/jsonapi/tests/src/Kernel/ResourceType/RelatedResourceTypesTest.php b/core/modules/jsonapi/tests/src/Kernel/ResourceType/RelatedResourceTypesTest.php index c8afcce764ef..3ee33a9c6e40 100644 --- a/core/modules/jsonapi/tests/src/Kernel/ResourceType/RelatedResourceTypesTest.php +++ b/core/modules/jsonapi/tests/src/Kernel/ResourceType/RelatedResourceTypesTest.php @@ -4,6 +4,7 @@ use Drupal\Tests\jsonapi\Kernel\JsonapiKernelTestBase; use Drupal\node\Entity\NodeType; +use PHPUnit\Framework\Error\Warning; /** * @coversDefaultClass \Drupal\jsonapi\ResourceType\ResourceType @@ -179,4 +180,35 @@ public function getRelatableResourceTypesByFieldProvider() { ]; } + /** + * Ensure a graceful failure when a field can references a missing bundle. + * + * @covers \Drupal\jsonapi\ResourceType\ResourceTypeRepository::all + * @covers \Drupal\jsonapi\ResourceType\ResourceTypeRepository::calculateRelatableResourceTypes + * @covers \Drupal\jsonapi\ResourceType\ResourceTypeRepository::getRelatableResourceTypesFromFieldDefinition + * + * @link https://www.drupal.org/project/drupal/issues/2996114 + */ + public function testGetRelatableResourceTypesFromFieldDefinition() { + $field_config_storage = $this->container->get('entity_type.manager')->getStorage('field_config'); + + static::assertCount(0, $this->resourceTypeRepository->get('node', 'foo')->getRelatableResourceTypesByField('field_relationship')); + $this->createEntityReferenceField('node', 'foo', 'field_ref_with_missing_bundle', 'Related entity', 'node', 'default', [ + 'target_bundles' => ['missing_bundle'], + ]); + $fields = $field_config_storage->loadByProperties(['field_name' => 'field_ref_with_missing_bundle']); + static::assertSame(['missing_bundle'], $fields['node.foo.field_ref_with_missing_bundle']->getItemDefinition()->getSetting('handler_settings')['target_bundles']); + + try { + $this->resourceTypeRepository->get('node', 'foo')->getRelatableResourceTypesByField('field_ref_with_missing_bundle'); + static::fail('The above code must produce a warning since the "missing_bundle" does not exist.'); + } + catch (Warning $e) { + static::assertSame( + 'The "field_ref_with_missing_bundle" at "node:foo" references the "node:missing_bundle" entity type that does not exist. Please take action.', + $e->getMessage() + ); + } + } + } diff --git a/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeNameAliasTest.php b/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeNameAliasTest.php new file mode 100644 index 000000000000..b2b506c44985 --- /dev/null +++ b/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeNameAliasTest.php @@ -0,0 +1,87 @@ +<?php + +namespace Drupal\Tests\jsonapi\Kernel\ResourceType; + +use Drupal\jsonapi\ResourceType\ResourceType; +use Drupal\Tests\jsonapi\Kernel\JsonapiKernelTestBase; + +/** + * Ensures that JSON:API doesn't malfunction on aliased resource type names. + * + * Despite JSON:API having a limited public PHP API, contrib and custom + * modules have found ways to rename resource types (e.g. "user--user" to + * "user") which causes JSON:API to malfunction. This results in confusion and + * bug reports. Since aliasing resource type names is a feature that JSON:API + * will eventually support, this test will help prevent future regressions and + * lower the present support burden. + * + * @coversDefaultClass \Drupal\jsonapi\ResourceType\ResourceTypeRepository + * @group jsonapi + * + * @internal + * + * @link https://www.drupal.org/project/drupal/issues/2996114 + * + * @todo move this test coverage to ResourceTypeRepositoryTest::testResourceTypeNameAliasing in https://www.drupal.org/project/drupal/issues/3105318 + */ +class ResourceTypeNameAliasTest extends JsonapiKernelTestBase { + + /** + * {@inheritdoc} + */ + protected static $modules = [ + 'node', + 'user', + 'field', + 'system', + 'serialization', + 'jsonapi_test_resource_type_aliasing', + ]; + + /** + * {@inheritdoc} + */ + protected function setUp(): void { + parent::setUp(); + + $this->container + ->get('entity_type.manager') + ->getStorage('node_type') + ->create(['type' => 'page']) + ->save(); + } + + /** + * Ensures resource repository works with publicly renamed resource types. + * + * @covers ::get + * @covers ::all + * @covers ::getByTypeName + */ + public function testRepositoryResourceTypeNameAliasing() { + $repository = $this->container->get('jsonapi.resource_type.repository'); + + static::assertInstanceOf(ResourceType::class, $repository->get('user', 'user')); + static::assertNull($repository->getByTypeName('user--user')); + static::assertInstanceOf(ResourceType::class, $repository->getByTypeName('user==user')); + + static::assertInstanceOf(ResourceType::class, $repository->get('node', 'page')); + static::assertNull($repository->getByTypeName('node--page')); + static::assertInstanceOf(ResourceType::class, $repository->getByTypeName('node==page')); + + foreach ($repository->all() as $id => $resource_type) { + static::assertSame( + $resource_type->getTypeName(), + $id, + 'The key is always equal to the type name.' + ); + + static::assertNotSame( + sprintf('%s--%s', $resource_type->getEntityTypeId(), $resource_type->getBundle()), + $id, + 'The type name can be renamed so it differs from the internal.' + ); + } + } + +} diff --git a/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php b/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php index d4461fe9c956..c8f2ab2aeff1 100644 --- a/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php +++ b/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php @@ -187,6 +187,16 @@ public function testResourceTypeFieldAliasing() { $this->assertSame($this->resourceTypeRepository->getByTypeName('node--page')->getPublicName('uid'), 'owner'); } + /** + * Tests that resource type fields can be aliased per resource type. + */ + public function testResourceTypeNameAliasing() { + // When this test is implemented, ensure the the tested behaviors in + // ResourceTypeNameAliasTest have been covered and remove it. Then remove + // the jsonapi_test_resource_type_aliasing test module. + $this->markTestSkipped('Remove in https://www.drupal.org/project/drupal/issues/3105318'); + } + /** * Tests that resource type fields can be disabled per resource type. */ -- GitLab