Add a public API for aliasing resource type names

I'm not sure if the "public" vs "internal" distinction that we have for fields makes sense here too. For now, going with it... although much if not all of the infrastructure does assume the entity type and bundle can be parsed from the JSON:API resource type name

We do not need it. The infrastructure does not assume that, at least not on purpose. We have ResourceType::getEntityTypeId() and ResourceType::getBundle() for that. That's because JSON:API Extras has always allowed resource types to be renamed. Parsing the resource type name would be a bug IMO.

So, then shall we go with just setResourceTypeName()?

Works for me!

Renamed the method per #9 and #10. Added a test method to ResourceTypeRepositoryTest.

This doesn't work. \Drupal\jsonapi\ResourceType\ResourceType::__construct still builds the typeName based off of the entity type ID and bundle. We'd need to modify the constructor to accept the resource type name. Or a method for overriding it.

    $this->typeName = $this->bundle === '?'
      ? 'unknown'
      : sprintf('%s--%s', $this->entityTypeId, $this->bundle);

Here's a stab at changing the constructor and test changes. We'll see what we do.

I just realized this doesn't entirely solve our problem for the Commerce API.

  public function getPath() {
    return sprintf('/%s/%s', $this->getEntityTypeId(), $this->getBundle());
  }

The resource path is not built off of the resource type name. So you're going to have a product--default type available at /commerce_product/default.

I don't know if this is scope creep, but I feel like the path should reflect the resource type name as it currently does. So I've changed the path output to

  public function getPath() {
    return '/' . implode('/', explode('--', $this->typeName));
  }

Also:

+++ b/core/lib/Drupal/Core/Security/PharExtensionInterceptor.php
@@ -31,7 +31,7 @@ class PharExtensionInterceptor implements Assertable {
-  public function assert(string $path, string $command): bool {
+  public function assert($path, $command) {

Whoops. I couldn't run tests locally without modifying this. Didn't mean to patch.

So I ran into a side effect here. There is no way to retrieve an aliased resource type for an entity type. Given an entity type ID you have to know how it was aliased so you can still run `\Drupal\jsonapi\ResourceType\ResourceTypeRepository::get`.

`get()` does the following:

return $this->getByTypeName("$entity_type_id--$bundle");

Currently, as a workaround, I am using something like this:

  private function getResourceTypesForEntityType(string $entity_type_id): array {
    if (!isset($this->entityTypeResourceTypes[$entity_type_id])) {
      $this->entityTypeResourceTypes[$entity_type_id] = array_filter($this->resourceTypeRepository->all(), static function (ResourceType $resource_type) use ($entity_type_id) {
        return $resource_type->getEntityTypeId() === $entity_type_id;
      });
    }
    return $this->entityTypeResourceTypes[$entity_type_id];
  }

Oh, this totally breaks everything, like related fields

\Drupal\jsonapi\ResourceType\ResourceTypeRepository::getRelatableResourceTypesFromFieldDefinition

    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);

I think aliasing resource type names is going to be near impossible. For Commerce, all we want is to change the paths. We want to control `type` and paths :/

It looks like some of the blockers are due to reverts in resource-type matching logic from #3018287: ResourceTypeRepository computes ResourceType value objects on *every request*

This reverts changes in ResourceTypeRepository caused by #3018287 that break renamed resource types. It should cause this patch to pass. It also means JSON:API Extras is probably broken in 8.8.x as well if you rename resource types.

Edit: not a problem, I messed up my assertion code. 🤦‍♂️

1. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -339,7 +341,7 @@ public function isVersionable() {
   -  public function __construct($entity_type_id, $bundle, $deserialization_target_class, $internal = FALSE, $is_locatable = TRUE, $is_mutable = TRUE, $is_versionable = FALSE, array $fields = []) {
   +  public function __construct($entity_type_id, $bundle, $deserialization_target_class, $type_name = NULL, $internal = FALSE, $is_locatable = TRUE, $is_mutable = TRUE, $is_versionable = FALSE, array $fields = []) {
   

   If you move $type_name to the end, we can skip the whole BC song and dance. However, if we do want to do that song and dance, let's go all-in. Make it the first argument so that $entity_type_id and $bundle can more easily be made optional in the future.

   ... Actually, thinking about it more. Let's do that. Then we can move all the $this->typeName logic out of the constructor into the repository where you have $type_name = NULL;.

2. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -425,7 +430,7 @@ public function getRelatableResourceTypesByField($field_name) {
      public function getPath() {
   -    return sprintf('/%s/%s', $this->getEntityTypeId(), $this->getBundle());
   +    return '/' . implode('/', explode('--', $this->typeName));
      }
   

   I like this reformulation. It gets us further away from entities.

3. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
   @@ -433,8 +441,12 @@ protected function getRelatableResourceTypesFromFieldDefinition(FieldDefinitionI
        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;
   +      foreach ($resource_types as $resource_type) {
   +        if ($resource_type->getEntityTypeId() === $entity_type_id && $resource_type->getBundle() === $target_bundle) {
   +          return $resource_type;
   +        }
   +      }
   +      return NULL;
        }, $target_bundles);
   

   Let's convert this to an array_reduce and simply drop the NULL values. This is a bug waiting to happen (it's probably already there).

4. +++ b/core/modules/jsonapi/tests/modules/jsonapi_test_resource_type_building/src/EventSubscriber/ResourceTypeBuildEventSubscriber.php
   @@ -22,6 +22,7 @@ public static function getSubscribedEvents() {
   +        ['renamingResourceTypes'],
   
   @@ -75,4 +76,18 @@ public function disableResourceTypeFields(ResourceTypeBuildEvent $event) {
   +  public function renamingResourceTypes(ResourceTypeBuildEvent $event) {
   

   Nit: s/renamingResourceTypes/renameResourceType

Let's convert this to an array_reduce and simply drop the NULL values. This is a bug waiting to happen (it's probably already there).

I was pretty sure that this was probably a bug and I had a vague feeling that I had realized it before. I tracked that feeling down to this comment #3034786-37: ResourceIdentifier::getVirtualOrMissingResourceIdentifier() ignores field aliases; causes $relatable_resource_types field to be empty and results in an error.

I think we can/should just fix it here for efficiency's sake.

Edit: Actually, it's entirely appropriate to fix that bug here. The bug could only have been encountered if one were using JSON:API Extras. JSON:API core has no official way to alias resource types in JSON:API so, in theory, the bug doesn't exist and there was no good way to write a test for it. (JSON:API Extras does some tricky things to alias resource types and it wasn't ever "officially" supported)

Since this issue is all about creating a public, official API to alias resource types, it makes complete sense that the bug should be prevented in this issue.

Picking this back up

• #25.1: done, made it a mandatory parameter and the first one. The resource type repository is now what defines the default type name to be {$entity_type_id}--{$bundle}
• #25.3 wouldn't that be a performance waste? array_reduce would go through every resource type, even after we have determined the proper one. Or I'm not following the suggestion.

#26 (follow up to 25.3) so we should do an array_filter to remove any keys from getRelatableResourceTypesFromFieldDefinition which may be null? I don't see the gains in array_reduce, as we know it should exit once we find the first (and only) match.

This should address the test failures.

This should fix the remaining tests – forgot to persist "unknown" as the type name for dangling references. Also the decorated ResourceTypeRepository in jsonapi_test_collection_count needed to be updated.

Failures were completely unrelated, queued the tests again and it passed.

Awesome patch, would love to see this in core. I went through the code and I have one concern. As mentioned in our discussion in slack about this patch.

+++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
@@ -179,7 +182,12 @@ public function get($entity_type_id, $bundle) {
+    foreach ($this->all() as $resource) {
+      if ($resource->getEntityTypeId() === $entity_type_id && $resource->getBundle() === $bundle) {
+        return $resource;
+      }
+    }
+    return NULL;

When looking at this code i'm worried about the performance implications if removing the lookup table for finding the correct resources.

This would mean when serving a request with a set of resources the repository will loop through all resources for every access check for the set. This would possibly be quite a lot and probably even includes the related resources.

Perhaps we do need a static lookup table in some form again. Not sure how right now though.

#36 has a great point. This patch adds a static mapping, which prevents duplicated loops to find a resource type. It was modified in #3018287: ResourceTypeRepository computes ResourceType value objects on *every request* to improve performance but broke the aliasing of resource types.

It leverages the memory cache so that its caches can be invalidated.

Edit:

+++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
@@ -104,12 +111,13 @@ class ResourceTypeRepository implements ResourceTypeRepositoryInterface {
    * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $dispatcher
    *   The event dispatcher.
    */
-  public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityTypeBundleInfoInterface $entity_bundle_info, EntityFieldManagerInterface $entity_field_manager, CacheBackendInterface $cache, EventDispatcherInterface $dispatcher) {
+  public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityTypeBundleInfoInterface $entity_bundle_info, EntityFieldManagerInterface $entity_field_manager, CacheBackendInterface $cache, EventDispatcherInterface $dispatcher, CacheBackendInterface $resource_type_mapping) {

Forgot to update a doc block.

Do we need a change notice here or need to amend an existing one?

+++ b/core/modules/jsonapi/jsonapi.services.yml
@@ -82,7 +82,7 @@ services:
+    arguments: ['@entity_type.manager', '@entity_type.bundle.info', '@entity_field.manager', '@cache.jsonapi_resource_types', '@event_dispatcher', '@cache.jsonapi_memory']

Why not use cache.jsonapi_resource_types which is a chained cache with memory first and then the default cache which has been introduced for the ResourceRepository and is already included?

This cache could do the same, but with some persistence?

The patch in #2996114-167: Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, NULL given references this issue as a follow up since it is introducing a test related to resource type aliasing.

Before this patch lands, we will need to clean up the @todos that it introduced (assuming it lands).

I was working on extra's and using build events there and came across issue again. I think it is time to get this moving again since #2996114: Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, NULL given has landed.

I've resolled the patch for 9.3 as a start.

Missing docblock for ResourceTypeRepository contructor.

Remove tests that were added in #2996114: Argument 2 passed to Drupal\jsonapi\Routing\Routes::Drupal\jsonapi\Routing\{closure}() must be an instance of Drupal\jsonapi\ResourceType\ResourceType, NULL given temporarily. The coverage for the API is already contained in this issue.

Fixed Kernel/Normalizer/LinkCollectionNormalizerTest since it instantiates a ResourceType and was missing an argument.

Invalidated.

Invalidated.

(I give up on commit style patches)

Cleaned up the code to lookup resources since we added a lookupResourceType method in a related issue. Made cache lookups consistent with the code in the all() method.

Also removed the extra cache from the service and class. We should just use the cache already present in the repository, no need to change the service. The CID can't overlap so it is fine to save it in permanent cache.

1. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -339,7 +341,7 @@ public function isVersionable() {
   -  public function __construct($entity_type_id, $bundle, $deserialization_target_class, $internal = FALSE, $is_locatable = TRUE, $is_mutable = TRUE, $is_versionable = FALSE, array $fields = []) {
   +  public function __construct($type_name, $entity_type_id, $bundle, $deserialization_target_class, $internal = FALSE, $is_locatable = TRUE, $is_mutable = TRUE, $is_versionable = FALSE, array $fields = []) {
   

   I think this is the correct thing to do, but I suspect committers will ask us to put $type_name as a final, optional argument to maintain BC on this internal API on a "best effort" basis.

   In another issue, we could consider adding a static ResourceType::create method in order to have a more elegant method signature.

2. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -425,7 +424,7 @@ public function getRelatableResourceTypesByField($field_name) {
   -    return sprintf('/%s/%s', $this->getEntityTypeId(), $this->getBundle());
   +    return '/' . implode('/', explode('--', $this->typeName));
   

   This looks like a bugfix? E.g. if $this->bundle === '?' then the path would contain an illegal character in the path segment of the URL.

3. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
   @@ -153,13 +153,16 @@ protected function createResourceType(EntityTypeInterface $entity_type, $bundle)
   +    $type_name = $bundle === '?' ? 'unknown' : sprintf('%s--%s', $entity_type->id(), $bundle);
        if (!$internalize_resource_type) {
          $event = ResourceTypeBuildEvent::createFromEntityTypeAndBundle($entity_type, $bundle, $fields);
          $this->eventDispatcher->dispatch($event, ResourceTypeBuildEvents::BUILD);
          $internalize_resource_type = $event->resourceTypeShouldBeDisabled();
          $fields = $event->getFields();
   +      $type_name = $event->getResourceTypeName();
        }
        return new ResourceType(
   +      $type_name,
   

   Yeah, I think the way to do the BC dance is to keep the fallback inside the ResourceType class and make $type_name and optional argument with NULL as the default.

4. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
   @@ -180,7 +183,14 @@ public function get($entity_type_id, $bundle) {
   -    return static::lookupResourceType($this->all(), $entity_type_id, $bundle);
   +    $map_id = sprintf('jsonapi.resource_type.%s.%s', $entity_type_id, $bundle);
   +    $cached = $this->cache->get($map_id);
   +    if ($cached === FALSE) {
   +      $resource_type = static::lookupResourceType($this->all(), $entity_type_id, $bundle);
   +      $this->cache->set($map_id, $resource_type, Cache::PERMANENT, $this->cacheTags);
   +    }
   +
   +    return $cached ? $cached->data : $resource_type;
   

   Could we add this caching in a separate issue?

5. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
   @@ -188,7 +198,7 @@ public function get($entity_type_id, $bundle) {
   -    return isset($resource_types[$type_name]) ? $resource_types[$type_name] : NULL;
   +    return $resource_types[$type_name] ?? NULL;
   

   Fine, but could be a Task issue.

6. +++ b/core/modules/jsonapi/tests/modules/jsonapi_test_collection_count/src/ResourceType/CountableResourceTypeRepository.php
   @@ -14,16 +14,17 @@ class CountableResourceTypeRepository extends ResourceTypeRepository {
   -      $entity_type->id(),
   -      $bundle,
   -      $entity_type->getClass(),
   -      $entity_type->isInternal(),
   -      static::isLocatableResourceType($entity_type, $bundle),
   -      static::isMutableResourceType($entity_type, $bundle),
   -      static::isVersionableResourceType($entity_type),
   -      static::getFields($raw_fields, $entity_type, $bundle)
   +      $resource_type->getTypeName(),
   +      $resource_type->getEntityTypeId(),
   +      $resource_type->getBundle(),
   +      $resource_type->getDeserializationTargetClass(),
   +      $resource_type->isInternal(),
   +      $resource_type->isLocatable(),
   +      $resource_type->isMutable(),
   +      $resource_type->isVersionable(),
   +      $resource_type->getFields()
   

   😍

7. +++ /dev/null
   @@ -1,33 +0,0 @@
   - * @todo remove this class in https://www.drupal.org/project/drupal/issues/3105318
   

   👏👏👏

8. +++ b/core/modules/jsonapi/tests/src/Kernel/Controller/EntityResourceTest.php
   @@ -218,7 +218,7 @@ public function testGetEmptyCollection() {
   -    $resource_type = new ResourceType('node', 'article', NULL);
   +    $resource_type = new ResourceType('node--article', 'node', 'article', NULL);
   
   +++ b/core/modules/jsonapi/tests/src/Kernel/Normalizer/LinkCollectionNormalizerTest.php
   @@ -76,7 +76,7 @@ protected function setUp(): void {
   -    $link_context = new ResourceObject(new CacheableMetadata(), new ResourceType('n/a', 'n/a', 'n/a'), 'n/a', NULL, [], new LinkCollection([]));
   +    $link_context = new ResourceObject(new CacheableMetadata(), new ResourceType('n/a', 'n/a', 'n/a', 'n/a'), 'n/a', NULL, [], new LinkCollection([]));
   
   +++ b/core/modules/jsonapi/tests/src/Kernel/Query/FilterTest.php
   @@ -311,7 +311,7 @@ protected function savePaintings($paintings) {
   -    $resource_type = new ResourceType('foo', 'bar', NULL);
   +    $resource_type = new ResourceType('foo--bar', 'foo', 'bar', NULL);
   
   @@ -378,7 +378,7 @@ public function testCreateFromQueryParameterNested() {
   -    $resource_type = new ResourceType('foo', 'bar', NULL);
   +    $resource_type = new ResourceType('foo--bar', 'foo', 'bar', NULL);
   

   These changes could be avoided by making $type_name optional.

9. +++ /dev/null
   @@ -1,87 +0,0 @@
   - * 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.
   

   This is basically covering JSON:API Extras. Which, happily, you maintain! I'm not worried about removing this test, but I do think we should have a change record that describes the upgrade path from an aliasing repository to an event subscriber.

Thanks for the review, at first glance they all make perfect sense. I'll get this cleaned up and perhaps split off some parts to keep it as small as possible.

1. Ok, i'll go with your defensive approach. Perhaps a create method somewhere is something for the future, but i wont make this patch even bigger right now at least.
2. See #16 and #25. Mostly to keep things consistent really.
3. Yeah, seems like the way to go. I'll move and change default for the ResourceTypeBuildEvent.
4. I think if we do not do this we get possible performance issues. This code will be run a LOT as noted comment #36 (by me ;))
5. Yeah might as well, ill create a new task and revert this
6. :)
7. :)
8. Yeah i've gone through and checked all calls, lot of less changes now. While doing that i noticed a place where it wasnt changed also $type_3 = new ResourceType('entity_type_3', '123', EntityInterface::class, NULL, TRUE);. Guess that call was forgotten.
9. Yeah change record should be present.

If this issue is fixed we should look into #3171154: Cannot upload files if file--file resource type has been aliased..

Reverted accidental remove of jsonapi_test_field_aliasing module.

Looks good! My only reservation after #60 was still the cache. I was worried that going over the network to a database, even one like Redis, would be slower than a loop in PHP. However, I confirmed that the cache back end is an in-memory one, which mitigates my concern.

Seeing this RTBC brings tears of joy to my eyes 🥲

Draft change record available here.

Looks good! Thank you.

This is looking great, just a couple of suggestions

1. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -338,8 +338,10 @@ public function isVersionable() {
   +   * @param null|string $type_name
   +   *   The resource type name.
   

   I think we should document that two hyphens will be used to construct the path, e.g -- becomes /

2. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
   @@ -153,11 +153,13 @@ protected function createResourceType(EntityTypeInterface $entity_type, $bundle)
   +    $type_name = $bundle === '?' ? 'unknown' : sprintf('%s--%s', $entity_type->id(), $bundle);
   
   @@ -167,7 +169,8 @@ protected function createResourceType(EntityTypeInterface $entity_type, $bundle)
   +      $type_name
   

   I don't think we need to initialise this - as passing null to the constructor will take care of that

3. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeRepository.php
   @@ -180,7 +183,14 @@ public function get($entity_type_id, $bundle) {
   +    if ($cached === FALSE) {
   +      $resource_type = static::lookupResourceType($this->all(), $entity_type_id, $bundle);
   +      $this->cache->set($map_id, $resource_type, Cache::PERMANENT, $this->cacheTags);
   ...
   +    return $cached ? $cached->data : $resource_type;
   

   this would read nicer if you flipped the logic, you'd avoid the second ternary

   if ($cached) {
     return $cached->data;
   }
   
   ...
   

1. #66.1 You are right. This is should also be included in the change record.
2. #66.2 Good point! But in my opinion it should be initialized as NULL to be explicit about it.
3. #66. OK, that is better, i agree. Will open a follow-up task then since the RecourseTypeRepository::all method uses exactly this logic. I chose consistency over readability here.

New patch incoming after I eat ;)

I've been thinking and #66.2 does mean some extra breaks in contrib since this would mean anyone who now uses double dashes in a renamed resource would end up with a new path so i think something like

  public function getPath() {
    $defaultPath = sprintf('/%s/%s', $this->getEntityTypeId(), $this->getBundle());
    if($this->typeName === sprintf('%s--%s', $this->entityTypeId, $this->bundle)){
        return '/' . implode('/', explode('--', $this->typeName));
    }
    return '/' . $this->typeName;
  }

This would mean less breakage through contrib, but would mean it is impossible to rename a bundle only.

Pros:
- Less breakage in contrib with names containing double dashes.

Cons:
- Less flexible renaming of resources since you cannot rename something like node/page to node/content-page.

Another option is adding an option to enable/disable adding path separators. I'm looking for some extra opinions here I think.

On mobile, so not s thorough review of recent updates. But we should not consider contrib here, in my opinion. This was never a public API and Amy contrib was hacks.

The only two contrib to worry about is JSONAPI Extras and Commerce API (which maintains it's kwk version of this code)

I like `--` becoming a standard constant to identify a split (like `:` for plugin derivatives)

After a discussion on Slack with @larowan, @mglaman and @e0ipso i decided to go through with this patch.

1. I created #3225034: Simplify ResourceTypeRepository control flow for returning cached data to simplify some control flows in the ResourceTypeRepository.
2. I updated the changerecord to include the new constant Drupal\jsonapi\ResourceType::TYPE_NAME_URI_PATH_SEPARATOR that defines how resource names are split to uri paths.
3. As requested, the $typeName variable is now initilized as NULL in ResourceTypeRepository::createResourceType

Putting this back to RTBC as discussed with @larowan on Slack.

1. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -355,7 +362,7 @@ public function __construct($entity_type_id, $bundle, $deserialization_target_cl
   +        : sprintf('%s' . self::TYPE_NAME_URI_PATH_SEPARATOR . '%s', $this->entityTypeId, $this->bundle);
   

   Suggestion: $this->entityTypeId . self::TYPE_NAME_URI_PATH_SEPARATOR . $this->bundle.

2. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -425,12 +432,13 @@ public function getRelatableResourceTypesByField($field_name) {
   +   *   The path to access this resource type. This function replaces "--" with
   +   *   a "/" in the uri path. Example: "node--article" -> "node/article"
   

   Suggestion: The path to access this resource type. This function replaces {@see TYPE_NAME_URI_PATH_SEPARATOR} with a "/" in the URI path. Example: "node--article" -> "node/article".

3. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeBuildEvent.php
   @@ -67,7 +67,7 @@ protected function __construct($resource_type_name, array $fields) {
   +    return new static(sprintf('%s' . ResourceType::TYPE_NAME_URI_PATH_SEPARATOR . '%s', $entity_type->id(), $bundle), $fields);
   

   Suggestion: return new static($entity_type->id() . ResourceType::TYPE_NAME_URI_PATH_SEPARATOR . $bundle, $fields);

4. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeBuildEvent.php
   @@ -84,7 +84,8 @@ public function getResourceTypeName() {
   +   *   The resource type name. The resource replaces "--" with
   +   *   a "/" in the uri path. Example: "node--article" -> "node/article"
   

   Suggestion: The resource type name. The {@see TYPE_NAME_URI_PATH_SEPARATOR} is replaced by a "/". Example: "node--article" -> "node/article".

I'll have a look, thanks.

#72.1: Ok, the current contruction is indeed weird. But i think i rather sprintf('%s%s%s', ...args) then.
#72.2: Good point.
#72.3: See 1.
#72.4: See 2.

Updated docblocks a bit more to be more readable. Also replaces sprinf with normal string concatting.

1. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -349,9 +358,12 @@ public function __construct($entity_type_id, $bundle, $deserialization_target_cl
   +    $this->typeName = $type_name;
   +    if ($type_name === NULL) {
   +      $this->typeName = $this->bundle === '?'
   +        ? 'unknown'
   +        : $this->entityTypeId . self::TYPE_NAME_URI_PATH_SEPARATOR . $this->bundle;
   +    }
   

   Assigning a value and then reassign it in case it's NULL seem useless. Can we do the following?

   if ($type_name === NULL) {
     $this->typeName = $this->bundle === '?'
       ? 'unknown'
       : $this->entityTypeId . self::TYPE_NAME_URI_PATH_SEPARATOR . $this->bundle;
   }
   else {
     $this->typeName = $type_name;
   }
   

2. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeBuildEvent.php
   @@ -17,9 +17,9 @@ class ResourceTypeBuildEvent extends Event {
   +  protected $resourceTypeName = NULL;
   

   It's NULL by default, no need to explicitly declare that.

3. +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeBuildEvent.php
   @@ -80,6 +80,17 @@ public function getResourceTypeName() {
   +  public function setResourceTypeName(string $resource_type_name) {
   

   Since this is the new public method, can we explicitly declare it should have no return statement by adding the void return type annotation?

1. I think the current way is more readable in my opinion.
2. This is true technically, sure.
3. Yes we actually can, and should.

The cli tool seems to have rendered a broken interdiff.

😍 loving the progress here.

Just a couple of minor things here.
It might be worth asking in #documentation for some clarification on the best way to handle these @see references, but it seems like they're not compliant with the standard

1. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -18,6 +18,13 @@
   +   * @see getPath()
   

   According to the docs, this needs to be fully qualified

   I don't claim to be an expert, but the docs seems to indicate that's the case

2. +++ b/core/modules/jsonapi/src/ResourceType/ResourceType.php
   @@ -420,12 +432,14 @@ public function getRelatableResourceTypesByField($field_name) {
   +   *   {@see TYPE_NAME_URI_PATH_SEPARATOR} with a "/" in the URI path.
   
   +++ b/core/modules/jsonapi/src/ResourceType/ResourceTypeBuildEvent.php
   @@ -80,6 +80,17 @@ public function getResourceTypeName() {
   +   *   {@see \Drupal\jsonapi\ResourceType\ResourceType::getPath()}.
   

   According to the standard, @see needs to be on it's own line, with a space

   https://www.drupal.org/docs/develop/standards/api-documentation-and-comm...

Made changes as per comment #81, please review.

That is exactly what i wouldv'e done. I've updated the remaining docblock with the reference to the constant.

Setting to rtbc again since the changes are so minimal.

+++ b/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php
@@ -217,4 +207,19 @@ public function testResourceTypeFieldDisabling() {
diff --git a/core/scripts/dev/commit-code-check.sh b/core/scripts/dev/commit-code-check.sh

diff --git a/core/scripts/dev/commit-code-check.sh b/core/scripts/dev/commit-code-check.sh
old mode 100755
new mode 100644
diff --git a/core/scripts/drupal.sh b/core/scripts/drupal.sh

diff --git a/core/scripts/drupal.sh b/core/scripts/drupal.sh
old mode 100755
new mode 100644
diff --git a/core/scripts/password-hash.sh b/core/scripts/password-hash.sh

diff --git a/core/scripts/password-hash.sh b/core/scripts/password-hash.sh
old mode 100755
new mode 100644
diff --git a/core/scripts/rebuild_token_calculator.sh b/core/scripts/rebuild_token_calculator.sh

diff --git a/core/scripts/rebuild_token_calculator.sh b/core/scripts/rebuild_token_calculator.sh
old mode 100755
new mode 100644
diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh

diff --git a/core/scripts/run-tests.sh b/core/scripts/run-tests.sh
old mode 100755
new mode 100644
diff --git a/core/scripts/update-countries.sh b/core/scripts/update-countries.sh

diff --git a/core/scripts/update-countries.sh b/core/scripts/update-countries.sh
old mode 100755
new mode 100644
diff --git a/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/fixtures/scripts/disable-git-bin/git b/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/fixtures/scripts/disable-git-bin/git

diff --git a/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/fixtures/scripts/disable-git-bin/git b/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/fixtures/scripts/disable-git-bin/git
old mode 100755
new mode 100644

all these file permissions changes are unintended, can you please revert these

Sorry, forgot to add the local branch while generating the diff. Interdiff is empty for some reason, but when manualy comaring the patch had just the last set of lines removed.

Committed fc51b4b and pushed to 9.3.x. Thanks!

Published the change record.

Nice work folks.

Awesome! Thanks everyone!