diff --git a/src/Access/RelationshipFieldAccess.php b/src/Access/RelationshipFieldAccess.php index eefb7f0..0be984c 100644 --- a/src/Access/RelationshipFieldAccess.php +++ b/src/Access/RelationshipFieldAccess.php @@ -41,8 +41,8 @@ class RelationshipFieldAccess implements AccessInterface { */ public function access(Request $request, Route $route, AccountInterface $account) { $relationship_field_name = $route->getRequirement(static::ROUTE_REQUIREMENT_KEY); - $field_operation = $request->isMethodSafe() ? 'view' : 'edit'; - $entity_operation = $request->isMethodSafe() ? 'view' : 'update'; + $field_operation = $request->isMethodCacheable() ? 'view' : 'edit'; + $entity_operation = $request->isMethodCacheable() ? 'view' : 'update'; if ($resource_type = $request->get(Routes::RESOURCE_TYPE_KEY)) { $entity = $request->get($resource_type->getEntityTypeId()); if ($entity instanceof FieldableEntityInterface && $entity->hasField($relationship_field_name)) { @@ -50,7 +50,11 @@ class RelationshipFieldAccess implements AccessInterface { $field_access = $entity->get($relationship_field_name)->access($field_operation, $account, TRUE); $access_result = $entity_access->andIf($field_access); if ($access_result instanceof AccessResultReasonInterface) { - $access_result->setReason("The current user is not allowed to $field_operation this relationship. " . $access_result->getReason()); + $reason = "The current user is not allowed to {$field_operation} this relationship."; + if ($access_reason = $access_result->getReason()) { + $reason .= $access_reason; + }; + $access_result->setReason($reason); } return $access_result; } diff --git a/src/Controller/EntityResource.php b/src/Controller/EntityResource.php index 2d6235b..f171f8e 100644 --- a/src/Controller/EntityResource.php +++ b/src/Controller/EntityResource.php @@ -844,9 +844,6 @@ class EntityResource { // @todo Is this really the right path? throw new EntityAccessDeniedHttpException($entity, $combined_access, $related_field, "The current user is not allowed to $operation this relationship."); } - if (!($field_list = $entity->get($related_field)) || !$this->isRelationshipField($field_list)) { - throw new NotFoundHttpException(sprintf('The relationship %s is not present in this resource.', $related_field)); - } } /** @@ -930,25 +927,6 @@ class EntityResource { throw new EntityAccessDeniedHttpException($original_field->getEntity(), $field_edit_access, '/data/attributes/' . $field_name, sprintf('The current user is not allowed to PATCH the selected field (%s).', $field_name)); } - /** - * Checks if is a relationship field. - * - * @param \Drupal\Core\Field\FieldItemListInterface $entity_field - * Entity field. - * - * @return bool - * Returns TRUE if entity field is a relationship field with non-internal - * target resource types, FALSE otherwise. - */ - protected function isRelationshipField(FieldItemListInterface $entity_field) { - $resource_types = $this->resourceType->getRelatableResourceTypesByField( - $this->resourceType->getInternalName($entity_field->getName()) - ); - return !empty($resource_types) && array_reduce($resource_types, function ($has_external, $resource_type) { - return $has_external ? TRUE : !$resource_type->isInternal(); - }, FALSE); - } - /** * Build a collection of the entities to respond with and access objects. * diff --git a/src/Routing/Routes.php b/src/Routing/Routes.php index 2412f4d..764a35b 100644 --- a/src/Routing/Routes.php +++ b/src/Routing/Routes.php @@ -231,7 +231,13 @@ class Routes implements ContainerInjectionInterface { $routes->add(static::getRouteName($resource_type, 'individual.delete'), $individual_remove_route); } - foreach ($resource_type->getRelatableResourceTypes() as $relationship_field_name => $target_resource_type) { + foreach ($resource_type->getRelatableResourceTypes() as $relationship_field_name => $target_resource_types) { + // Do not create routes for relationships that only target internal + // resource types. + if (!static::hasNonInternalTargetResourceTypes($target_resource_types)) { + continue; + } + // Get an individual resource's related resources. $related_route = new Route("/{$path}/{{$entity_type_id}}/{$relationship_field_name}"); $related_route->setMethods(['GET']); @@ -313,6 +319,22 @@ class Routes implements ContainerInjectionInterface { return sprintf('jsonapi.%s.%s', $resource_type->getTypeName(), $route_type); } + /** + * Determines if an array of resource types has any non-internal ones. + * + * @param \Drupal\jsonapi\ResourceType\ResourceType[] $resource_types + * The resource types to check. + * + * @return bool + * TRUE if there is at least one non-internal resource type in the given + * array; FALSE otherwise. + */ + protected static function hasNonInternalTargetResourceTypes(array $resource_types) { + return array_reduce($resource_types, function ($carry, ResourceType $target) { + return $carry || !$target->isInternal(); + }, FALSE); + } + /** * Gets the resource type from a route or request's parameters. * diff --git a/tests/src/Unit/Routing/RoutesTest.php b/tests/src/Unit/Routing/RoutesTest.php index ba678dd..726bd08 100644 --- a/tests/src/Unit/Routing/RoutesTest.php +++ b/tests/src/Unit/Routing/RoutesTest.php @@ -66,9 +66,15 @@ class RoutesTest extends UnitTestCase { // Get the route collection and start making assertions. $routes = $this->routes['ok']->routes(); - // Make sure that there are 7 routes for the non-internal resource and 1 for - // the entry point. - $this->assertEquals(8, $routes->count()); + // - 2 collection routes; GET & POST for the non-internal resource type. + // - 3 individual routes; GET, PATCH & DELETE for the non-internal resource + // type. + // - 2 related routes; GET for the non-internal resource type relationships + // fields: external & both. + // - 2 relationship routes; all HTTP methods handled by the one route for + // the non-internal resource type. + // - 1 for the JSON API entry point. + $this->assertEquals(10, $routes->count()); $iterator = $routes->getIterator(); // Check the collection route. @@ -142,8 +148,8 @@ class RoutesTest extends UnitTestCase { // Check the related route. /** @var \Symfony\Component\Routing\Route $route */ - $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.related'); - $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/{related}', $route->getPath()); + $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.external.related'); + $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/external', $route->getPath()); $this->assertSame('entity_type_1--bundle_1_1', $route->getDefault(Routes::RESOURCE_TYPE_KEY)); $this->assertEquals(['GET'], $route->getMethods()); $this->assertSame(Routes::FRONT_CONTROLLER, $route->getDefault(RouteObjectInterface::CONTROLLER_NAME)); @@ -163,8 +169,8 @@ class RoutesTest extends UnitTestCase { // Check the relationships route. /** @var \Symfony\Component\Routing\Route $route */ - $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.relationship'); - $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/relationships/{related}', $route->getPath()); + $route = $iterator->offsetGet('jsonapi.entity_type_1--bundle_1_1.both.relationship'); + $this->assertSame('/jsonapi/entity_type_1/bundle_1_1/{entity_type_1}/relationships/both', $route->getPath()); $this->assertSame('entity_type_1--bundle_1_1', $route->getDefault(Routes::RESOURCE_TYPE_KEY)); $this->assertEquals(['GET', 'POST', 'PATCH', 'DELETE'], $route->getMethods()); $this->assertSame(Routes::FRONT_CONTROLLER, $route->getDefault(RouteObjectInterface::CONTROLLER_NAME)); @@ -192,8 +198,10 @@ class RoutesTest extends UnitTestCase { return [ ['jsonapi.entity_type_1--bundle_1_1.individual'], ['jsonapi.entity_type_1--bundle_1_1.collection'], - ['jsonapi.entity_type_1--bundle_1_1.related'], - ['jsonapi.entity_type_1--bundle_1_1.relationship'], + ['jsonapi.entity_type_1--bundle_1_1.external.related'], + ['jsonapi.entity_type_1--bundle_1_1.external.relationship'], + ['jsonapi.entity_type_1--bundle_1_1.both.related'], + ['jsonapi.entity_type_1--bundle_1_1.both.relationship'], ['jsonapi.resource_list'], ]; } @@ -215,8 +223,8 @@ class RoutesTest extends UnitTestCase { ['jsonapi.entity_type_2--bundle_2_1.individual'], ['jsonapi.entity_type_2--bundle_2_1.collection'], ['jsonapi.entity_type_2--bundle_2_1.collection.post'], - ['jsonapi.entity_type_2--bundle_2_1.related'], - ['jsonapi.entity_type_2--bundle_2_1.relationship'], + ['jsonapi.entity_type_2--bundle_2_1.internal.related'], + ['jsonapi.entity_type_2--bundle_2_1.internal.relationship'], ]; }