diff --git a/core/modules/jsonapi/src/Controller/EntityResource.php b/core/modules/jsonapi/src/Controller/EntityResource.php index 73c7f3ea10..561294f229 100644 --- a/core/modules/jsonapi/src/Controller/EntityResource.php +++ b/core/modules/jsonapi/src/Controller/EntityResource.php @@ -981,7 +981,7 @@ protected function buildWrappedResponse($data, Request $request, IncludedData $i assert($data instanceof Data || $data instanceof FieldItemListInterface); $links = ($links ?: new LinkCollection([])); if (!$links->hasLinkWithKey('self')) { - $self_link = new Link(new CacheableMetadata(), self::getRequestLink($request), ['self']); + $self_link = new Link(new CacheableMetadata(), self::getRequestLink($request), 'self'); $links = $links->withLink('self', $self_link); } $response = new ResourceResponse(new JsonApiDocumentTopLevel($data, $includes, $links, $meta), $response_code, $headers); @@ -1264,20 +1264,20 @@ protected static function getPagerLinks(Request $request, OffsetPage $page_param // Check if this is not the last page. if ($link_context['has_next_page']) { $next_url = static::getRequestLink($request, static::getPagerQueries('next', $offset, $size, $query)); - $pager_links = $pager_links->withLink('next', new Link(new CacheableMetadata(), $next_url, ['next'])); + $pager_links = $pager_links->withLink('next', new Link(new CacheableMetadata(), $next_url, 'next')); if (!empty($total)) { $last_url = static::getRequestLink($request, static::getPagerQueries('last', $offset, $size, $query, $total)); - $pager_links = $pager_links->withLink('last', new Link(new CacheableMetadata(), $last_url, ['last'])); + $pager_links = $pager_links->withLink('last', new Link(new CacheableMetadata(), $last_url, 'last')); } } // Check if this is not the first page. if ($offset > 0) { $first_url = static::getRequestLink($request, static::getPagerQueries('first', $offset, $size, $query)); - $pager_links = $pager_links->withLink('first', new Link(new CacheableMetadata(), $first_url, ['first'])); + $pager_links = $pager_links->withLink('first', new Link(new CacheableMetadata(), $first_url, 'first')); $prev_url = static::getRequestLink($request, static::getPagerQueries('prev', $offset, $size, $query)); - $pager_links = $pager_links->withLink('prev', new Link(new CacheableMetadata(), $prev_url, ['prev'])); + $pager_links = $pager_links->withLink('prev', new Link(new CacheableMetadata(), $prev_url, 'prev')); } return $pager_links; diff --git a/core/modules/jsonapi/src/Controller/EntryPoint.php b/core/modules/jsonapi/src/Controller/EntryPoint.php index bbbc2d56d1..12d5efab0c 100644 --- a/core/modules/jsonapi/src/Controller/EntryPoint.php +++ b/core/modules/jsonapi/src/Controller/EntryPoint.php @@ -82,7 +82,7 @@ public function index() { return !$resource->isInternal(); }); - $self_link = new Link(new CacheableMetadata(), Url::fromRoute('jsonapi.resource_list'), ['self']); + $self_link = new Link(new CacheableMetadata(), Url::fromRoute('jsonapi.resource_list'), 'self'); $urls = array_reduce($resources, function (LinkCollection $carry, ResourceType $resource_type) { if ($resource_type->isLocatable() || $resource_type->isMutable()) { $route_suffix = $resource_type->isLocatable() ? 'collection' : 'collection.post'; diff --git a/core/modules/jsonapi/src/Controller/FileUpload.php b/core/modules/jsonapi/src/Controller/FileUpload.php index 27032eaf3a..eb9fd6cd44 100644 --- a/core/modules/jsonapi/src/Controller/FileUpload.php +++ b/core/modules/jsonapi/src/Controller/FileUpload.php @@ -178,7 +178,7 @@ public function handleFileUploadForNewResource(Request $request, ResourceType $r } // @todo Remove line below in favor of commented line in https://www.drupal.org/project/jsonapi/issues/2878463. - $self_link = new Link(new CacheableMetadata(), Url::fromRoute('jsonapi.file--file.individual', ['entity' => $file->uuid()]), ['self']); + $self_link = new Link(new CacheableMetadata(), Url::fromRoute('jsonapi.file--file.individual', ['entity' => $file->uuid()]), 'self'); /* $self_link = new Link(new CacheableMetadata(), $this->entity->toUrl('jsonapi'), ['self']); */ $links = new LinkCollection(['self' => $self_link]); diff --git a/core/modules/jsonapi/src/JsonApiResource/Link.php b/core/modules/jsonapi/src/JsonApiResource/Link.php index 5634f983df..8aad002058 100644 --- a/core/modules/jsonapi/src/JsonApiResource/Link.php +++ b/core/modules/jsonapi/src/JsonApiResource/Link.php @@ -3,6 +3,7 @@ namespace Drupal\jsonapi\JsonApiResource; use Drupal\Component\Assertion\Inspector; +use Drupal\Component\Serialization\Json; use Drupal\Core\Cache\CacheableDependencyInterface; use Drupal\Core\Cache\CacheableDependencyTrait; use Drupal\Core\Cache\CacheableMetadata; @@ -41,6 +42,8 @@ * The link relation types. * * @var string[] + * + * @todo: change this type documentation to be a single string in https://www.drupal.org/project/drupal/issues/3080467. */ protected $rel; @@ -64,16 +67,24 @@ * entity on which the link will appear. * @param \Drupal\Core\Url $url * The Url object for the link. - * @param string[] $link_relation_types + * @param string $link_relation_type * An array of registered or extension RFC8288 link relation types. * @param array $target_attributes * An associative array of target attributes for the link. * * @see https://tools.ietf.org/html/rfc8288#section-2.1 */ - public function __construct(CacheableMetadata $cacheability, Url $url, array $link_relation_types, array $target_attributes = []) { + public function __construct(CacheableMetadata $cacheability, Url $url, $link_relation_type, array $target_attributes = []) { + // @todo Remove this conditional block in drupal:9.0.0 and add a type hint to the $link_relation_type argument of this method in https://www.drupal.org/project/drupal/issues/3080467. + // @see https://www.drupal.org/project/drupal/issues/3080259 + if (is_array($link_relation_type)) { + @trigger_error('Constructing a ' . self::class . ' with an array of link relation types is deprecated in drupal:8.8.0 and will throw a fatal error in drupal:9.0.0. Pass a single string instead. See https://www.drupal.org/project/drupal/issues/3080259.', E_USER_DEPRECATED); + } + else { + $link_relation_type = [$link_relation_type]; + } // @todo: uncomment the extra assertion below when JSON:API begins to use its own extension relation types. - assert(/* !empty($link_relation_types) && */Inspector::assertAllStrings($link_relation_types)); + assert(/* !empty($link_relation_types) && */Inspector::assertAllStrings($link_relation_type)); assert(Inspector::assertAllStrings(array_keys($target_attributes))); assert(Inspector::assertAll(function ($target_attribute_value) { return is_string($target_attribute_value) || is_array($target_attribute_value); @@ -81,7 +92,7 @@ public function __construct(CacheableMetadata $cacheability, Url $url, array $li $generated_url = $url->setAbsolute()->toString(TRUE); $this->href = $generated_url->getGeneratedUrl(); $this->uri = $url; - $this->rel = $link_relation_types; + $this->rel = $link_relation_type; $this->attributes = $target_attributes; $this->setCacheability($cacheability->addCacheableDependency($generated_url)); } @@ -106,14 +117,32 @@ public function getHref() { return $this->href; } + /** + * Gets the link's relation types. + * + * @return string|string[] + * The link's relation type. For BC reasons, this method might return an + * array of strings. + * + * @todo: update the return type docs to only have a single string in https://www.drupal.org/project/drupal/issues/3080467. + */ + public function getLinkRelationType() { + // @todo: uncomment the following line and remove the one after it in https://www.drupal.org/project/drupal/issues/3080467. + //return $this->rel; + return count($this->rel) > 1 ? $this->rel : reset($this->rel); + } + /** * Gets the link's relation types. * * @return string[] * The link's relation types. + * + * @todo: remove this method in https://www.drupal.org/project/drupal/issues/3080467. */ public function getLinkRelationTypes() { - return $this->rel; + @trigger_error(self::class . '::' . __METHOD__ . ' is deprecated in drupal:8.8.0 and will be removed in drupal:9.0.0. Use ' . self::class . '::' . 'getLinkRelationType' . ' instead. See https://www.drupal.org/project/drupal/issues/3080259.', E_USER_DEPRECATED); + return (array) $this->getLinkRelationType(); } /** @@ -127,7 +156,7 @@ public function getTargetAttributes() { } /** - * Compares two links by their href. + * Compares two links. * * @param \Drupal\jsonapi\JsonApiResource\Link $a * The first link. @@ -135,16 +164,31 @@ public function getTargetAttributes() { * The second link. * * @return int - * The result of strcmp() on the links' hrefs. + * 0 if the links can be considered identical, an integer greater than or + * less than 0 otherwise. */ public static function compare(Link $a, Link $b) { - return strcmp($a->getHref(), $b->getHref()); + // Sort the link relation type array so that order does not matter during + // string comparison. + sort($a->rel); + sort($b->rel); + // Any string concatenation would work, but a Link header-like format makes + // it clear what is being compared. + $a_string = sprintf('<%s>;rel="%s"', $a->getHref(), implode(' ', $a->rel)); + $b_string = sprintf('<%s>;rel="%s"', $b->getHref(), implode(' ', $b->rel)); + $cmp = strcmp($a_string, $b_string); + // This ternary avoids unnecessarily encoding target attributes on non- + // equivalent links. + $json_cmp = strcmp(Json::encode($a->getTargetAttributes()), Json::encode($b->getTargetAttributes())); + return $cmp === 0 + ? $json_cmp + : $cmp; } /** - * Merges two link objects' relation types and target attributes. + * Merges two equivalent links into one link with the merged cacheability. * - * The links must share the same URI. + * The links must share the same URI, link relation type and attributes. * * @param \Drupal\jsonapi\JsonApiResource\Link $a * The first link. @@ -152,15 +196,14 @@ public static function compare(Link $a, Link $b) { * The second link. * * @return static - * A new JSON:API Link object with the link relation type and target - * attributes merged. + * A new JSON:API Link object with the cacheability of both links merged. */ public static function merge(Link $a, Link $b) { - assert(static::compare($a, $b) === 0); - $merged_rels = array_unique(array_merge($a->getLinkRelationTypes(), $b->getLinkRelationTypes())); - $merged_attributes = array_merge_recursive($a->getTargetAttributes(), $b->getTargetAttributes()); + if (static::compare($a, $b) !== 0) { + throw new \LogicException('Only equivalent links can be merged.'); + } $merged_cacheability = (new CacheableMetadata())->addCacheableDependency($a)->addCacheableDependency($b); - return new static($merged_cacheability, $a->getUri(), $merged_rels, $merged_attributes); + return new static($merged_cacheability, $a->getUri(), $a->getLinkRelationType(), $a->getTargetAttributes()); } } diff --git a/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php b/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php index 0540248c9b..f870368e86 100644 --- a/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php +++ b/core/modules/jsonapi/src/JsonApiResource/ResourceObject.php @@ -243,19 +243,19 @@ protected static function buildLinksFromEntity(ResourceType $resource_type, Enti // revision changes and to disambiguate resource objects with the same // `type` and `id` in a `version-history` collection. $self_with_version_url = $self_url->setOption('query', [JsonApiSpec::VERSION_QUERY_PARAMETER => 'id:' . $entity->getRevisionId()]); - $links = $links->withLink('self', new Link(new CacheableMetadata(), $self_with_version_url, ['self'])); + $links = $links->withLink('self', new Link(new CacheableMetadata(), $self_with_version_url, 'self')); } if (!$entity->isDefaultRevision()) { $latest_version_url = $self_url->setOption('query', [JsonApiSpec::VERSION_QUERY_PARAMETER => 'rel:' . VersionByRel::LATEST_VERSION]); - $links = $links->withLink(VersionByRel::LATEST_VERSION, new Link(new CacheableMetadata(), $latest_version_url, [VersionByRel::LATEST_VERSION])); + $links = $links->withLink(VersionByRel::LATEST_VERSION, new Link(new CacheableMetadata(), $latest_version_url, VersionByRel::LATEST_VERSION)); } if (!$entity->isLatestRevision()) { $working_copy_url = $self_url->setOption('query', [JsonApiSpec::VERSION_QUERY_PARAMETER => 'rel:' . VersionByRel::WORKING_COPY]); - $links = $links->withLink(VersionByRel::WORKING_COPY, new Link(new CacheableMetadata(), $working_copy_url, [VersionByRel::WORKING_COPY])); + $links = $links->withLink(VersionByRel::WORKING_COPY, new Link(new CacheableMetadata(), $working_copy_url, VersionByRel::WORKING_COPY)); } } if (!$links->hasLinkWithKey('self')) { - $links = $links->withLink('self', new Link(new CacheableMetadata(), $self_url, ['self'])); + $links = $links->withLink('self', new Link(new CacheableMetadata(), $self_url, 'self')); } } return $links; diff --git a/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php b/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php index 6257adb4d8..fece326ab5 100644 --- a/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php +++ b/core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php @@ -380,7 +380,7 @@ protected function normalize(EntityInterface $entity, Url $url) { // Don't use cached normalizations in tests. $this->container->get('cache.jsonapi_normalizations')->deleteAll(); - $self_link = new Link(new CacheableMetadata(), $url, ['self']); + $self_link = new Link(new CacheableMetadata(), $url, 'self'); $resource_type = $this->container->get('jsonapi.resource_type.repository')->getByTypeName(static::$resourceTypeName); $doc = new JsonApiDocumentTopLevel(new ResourceObjectData([ResourceObject::createFromEntity($resource_type, $entity)], 1), new NullIncludedData(), new LinkCollection(['self' => $self_link])); return $this->serializer->normalize($doc, 'api_json', [ diff --git a/core/modules/jsonapi/tests/src/Unit/JsonApiResource/LinkTest.php b/core/modules/jsonapi/tests/src/Unit/JsonApiResource/LinkTest.php index 91441a8ea5..ee60421371 100644 --- a/core/modules/jsonapi/tests/src/Unit/JsonApiResource/LinkTest.php +++ b/core/modules/jsonapi/tests/src/Unit/JsonApiResource/LinkTest.php @@ -19,67 +19,99 @@ class LinkTest extends UnitTestCase { /** - * @covers ::merge - * @dataProvider mergeTargetAttributesProvider + * @covers ::compare + * @dataProvider linkComparisonProvider */ - public function testMergeTargetAttributes($a, $b, $expected) { - $this->assertSame($expected->getTargetAttributes(), Link::merge($a, $b)->getTargetAttributes()); + public function testLinkComparison(Link $a, Link $b, $expected) { + $actual = Link::compare($a, $b); + $this->assertSame($expected, $actual === 0); } /** - * Provides test data for link merging. + * Provides test data for link comparison. */ - public function mergeTargetAttributesProvider() { - $cases = [ - 'strings' => [ - ['key' => 'foo'], - ['key' => 'bar'], - ['key' => ['foo', 'bar']], + public function linkComparisonProvider() { + $this->mockUrlAssembler(); + return [ + 'same href and same link relation type' => [ + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self'), + TRUE, + ], + 'different href and same link relation type' => [ + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/bar'), 'self'), + FALSE, + ], + 'same href and different link relation type' => [ + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'related'), + FALSE, ], - 'string and array' => [ - ['key' => 'foo'], - ['key' => ['bar', 'baz']], - ['key' => ['foo', 'bar', 'baz']], + 'same href and same link relation type and empty target attributes' => [ + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self', []), + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self', []), + TRUE, ], - 'one-dimensional indexed arrays' => [ - ['key' => ['foo']], - ['key' => ['bar']], - ['key' => ['foo', 'bar']], + 'same href and same link relation type and same target attributes' => [ + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self', ['anchor' => 'https://jsonapi.org']), + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self', ['anchor' => 'https://jsonapi.org']), + TRUE, ], - 'one-dimensional keyed arrays' => [ - ['key' => ['foo' => 'tball']], - ['key' => ['bar' => 'ista']], - [ - 'key' => [ - 'foo' => 'tball', - 'bar' => 'ista', - ], - ], + 'same href and same link relation type and different target attributes' => [ + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self', ['anchor' => 'https://jsonapi.org/foo']), + new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/foo'), 'self', ['anchor' => 'https://jsonapi.org/bar']), + FALSE, ], - 'two-dimensional indexed arrays' => [ - ['one' => ['two' => ['foo']]], - ['one' => ['two' => ['bar']]], - ['one' => ['two' => ['foo', 'bar']]], + 'same href and same link relation type and same nested target attributes' => [ + new link(new cacheablemetadata(), url::fromuri('https://jsonapi.org/foo'), 'self', ['data' => ['foo' => 'bar']]), + new link(new cacheablemetadata(), url::fromuri('https://jsonapi.org/foo'), 'self', ['data' => ['foo' => 'bar']]), + TRUE, ], - 'two-dimensional keyed arrays' => [ - ['one' => ['two' => ['foo' => 'tball']]], - ['one' => ['two' => ['bar' => 'ista']]], - [ - 'one' => [ - 'two' => [ - 'foo' => 'tball', - 'bar' => 'ista', - ], - ], - ], + 'same href and same link relation type and different nested target attributes' => [ + new link(new cacheablemetadata(), url::fromuri('https://jsonapi.org/foo'), 'self', ['data' => ['foo' => 'bar']]), + new link(new cacheablemetadata(), url::fromuri('https://jsonapi.org/foo'), 'self', ['data' => ['foo' => 'baz']]), + FALSE, ], ]; + } + + /** + * @covers ::merge + * @dataProvider linkMergeProvider + */ + public function testLinkMerge(Link $a, Link $b, $expected) { + if ($expected instanceof Link) { + $this->assertSame($expected->getCacheTags(), Link::merge($a, $b)->getCacheTags()); + } + else { + $this->expectException($expected); + Link::merge($a, $b); + } + } + + /** + * Provides test data for link merging. + */ + public function linkMergeProvider() { $this->mockUrlAssembler(); - return array_map(function ($arguments) { - return array_map(function ($attributes) { - return new Link(new CacheableMetadata(), Url::fromUri('https://jsonapi.org/'), ['item'], $attributes); - }, $arguments); - }, $cases); + return [ + 'same everything' => [ + new Link((new CacheableMetadata())->addCacheTags(['foo']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link((new CacheableMetadata())->addCacheTags(['foo']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link((new CacheableMetadata())->addCacheTags(['foo']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + ], + 'different cache tags' => [ + new Link((new CacheableMetadata())->addCacheTags(['foo']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link((new CacheableMetadata())->addCacheTags(['bar']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link((new CacheableMetadata())->addCacheTags(['foo', 'bar']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + ], + 'same cache tags, non-equivalent links' => [ + new Link((new CacheableMetadata())->addCacheTags(['foo']), Url::fromUri('https://jsonapi.org/foo'), 'self'), + new Link((new CacheableMetadata())->addCacheTags(['foo']), Url::fromUri('https://jsonapi.org/bar'), 'self'), + \LogicException::class, + ], + ]; } /** @@ -89,7 +121,9 @@ protected function mockUrlAssembler() { $url_assembler = $this->getMockBuilder(UnroutedUrlAssemblerInterface::class) ->disableOriginalConstructor() ->getMock(); - $url_assembler->method('assemble')->willReturn((new GeneratedUrl())->setGeneratedUrl('https://jsonapi.org/')); + $url_assembler->method('assemble')->will($this->returnCallback(function ($uri) { + return (new GeneratedUrl())->setGeneratedUrl($uri); + })); $container = new ContainerBuilder(); $container->set('unrouted_url_assembler', $url_assembler);