Leverage updates in Drupal 8.8

Added link to the patch again in the summary.

So:

1. Commit made to this module to reduce boilerplate code
2. The quick fix is the one-line patch
3. The full fix is to tackle #3070204: Refactor the JSON:API FieldResolver to use a resource type instead of an entity type ID and bundle ID pair
4. Then #3031173: FieldResolver::resolveInternalEntityQueryPath should accept an array of resource types, not a single resource type (which is in the contrib queue not core?)

Correct?

Cool :) Thanks @gabesullice

#3070204: Refactor the JSON:API FieldResolver to use a resource type instead of an entity type ID and bundle ID pair landed! 🥳

I tried to use the module with 8.8. @mglaman pointed me to this issue, thank you!

When trying to use this I run into:

The website encountered an unexpected error. Please try again later.
Error: Call to undefined method Drupal\jsonapi\ResourceType\ResourceTypeAttribute::withRelatableResourceTypes() in Drupal\jsonapi\ResourceType\ResourceType->Drupal\jsonapi\ResourceType\{closure}() (line 385 of core/modules/jsonapi/src/ResourceType/ResourceType.php).
Drupal\jsonapi\ResourceType\ResourceType->Drupal\jsonapi\ResourceType\{closure}(Array, 'contact_form')
array_reduce(Array, Object, Array) (Line: 387)
Drupal\jsonapi\ResourceType\ResourceType->setRelatableResourceTypes(Array) (Line: 59)
Drupal\jsonapi_cross_bundles\ResourceType\CrossBundleResourceTypeRepository->all() (Line: 115)
Drupal\jsonapi\Routing\Routes->routes()
call_user_func(Array) (Line: 146)
Drupal\Core\Routing\RouteBuilder->rebuild() (Line: 83)

After talking with @Wim Leers we figured out this is caused by the BC layer in \Drupal\jsonapi\ResourceType\ResourceType::updateDeprecatedFieldMapping. The problem in there is caused by the old problem that field definitions are per bundle. The patch I've attached copies the core function for now into \Drupal\jsonapi_cross_bundles\ResourceType\CrossBundlesResourceType to see whether we
can use a superset somehow, see patch.

1. +++ b/src/ResourceType/CrossBundlesResourceType.php
   @@ -16,6 +24,48 @@ final class CrossBundlesResourceType extends ResourceType {
   +    if (!empty($fields) && !reset($fields) instanceof ResourceTypeField) {
   +      $fields = $this->updateDeprecatedFieldMapping($fields, $entity_type_id, $entity_type_id === $bundle ? NULL : $bundle);
   +    }
   

   If the only change here is to pass NULL or not, should we just handle that in updateDeprecatedFieldMapping so that we don't need to override the constructor?

2. +++ b/src/ResourceType/CrossBundlesResourceType.php
   @@ -63,4 +113,88 @@ final class CrossBundlesResourceType extends ResourceType {
   +  private function updateDeprecatedFieldMapping(array $field_mapping, $entity_type_id, $bundle) {
   +    $class_name = self::class;
   

   We can have this assert $bundle is NULL, or make it be null. Since we know this class expects it to be null.

Wait. I actually have a better idea. Let's make updateDeprecatedFieldMapping a no-op in our class. Override the parent so it does nothing.

We then update the deprecated method in \Drupal\jsonapi_cross_bundles\ResourceType\CrossBundleResourceTypeRepository::all for our resource type. Then there's no need to update deprecated values.

Note: This patch doesn't take into account comment 10 yet, sorry :)

I tried to implement generating a superset of all the field definitions, see interdiff. This resulted in another problem:

The website encountered an unexpected error. Please try again later.

LogicException: withRelatableResourceTypes() must be called before getting relatable resource types. in Drupal\jsonapi\ResourceType\ResourceTypeRelationship->getRelatableResourceTypes() (line 48 of core/modules/jsonapi/src/ResourceType/ResourceTypeRelationship.php).
Drupal\jsonapi\ResourceType\ResourceType->Drupal\jsonapi\ResourceType\{closure}(Object)
array_map(Object, Array) (Line: 404)
Drupal\jsonapi\ResourceType\ResourceType->getRelatableResourceTypes() (Line: 313)
Drupal\jsonapi\Routing\Routes::getIndividualRoutesForResourceType(Object) (Line: 183)
Drupal\jsonapi\Routing\Routes::getRoutesForResourceType(Object, '/jsonapi') (Line: 116)
Drupal\jsonapi\Routing\Routes->routes()
call_user_func(Array) (Line: 146)

So in my debugging, the error comes from ResourceTypeRelationship

  public function getRelatableResourceTypes() {
    if (!isset($this->relatableResourceTypesByField)) {
      $this->relatableResourceTypesByField = array_reduce(array_map(function (ResourceTypeRelationship $field) {
        return [$field->getPublicName() => $field->getRelatableResourceTypes()];
      }, array_filter($this->fields, function (ResourceTypeField $field) {
        return $field instanceof ResourceTypeRelationship;
      })), 'array_merge', []);
    }
    return $this->relatableResourceTypesByField;
  }

For me this errors when trying to get the relationship to commerce_product_variation_type , as I"m testing on my Commerce install.

This module was written before ResourceTypeRelationship was introduced.

This addresses the comments of @mglaman in comment number 10.

+++ b/src/ResourceType/CrossBundlesResourceType.php
@@ -16,6 +24,48 @@ final class CrossBundlesResourceType extends ResourceType {
+    if (!empty($fields) && !reset($fields) instanceof ResourceTypeField) {
+      $fields = $this->updateDeprecatedFieldMapping($fields, $entity_type_id);
+    }

If \Drupal\jsonapi_cross_bundles\ResourceType\CrossBundleResourceTypeRepository::all is updated to use getFields, this is never invoked. Which means the processing can all live in CrossBundleResourceTypeRepository, still.

$field_mapping_superset should be a grouped array of each bundle's relationship resources.

Here's my go.

The problem is \Drupal\jsonapi_cross_bundles\ResourceType\CrossBundleResourceTypeRepository::getFieldMappingSuperset returns relationship resources with no related resource types set.

😬yeah, \Drupal\jsonapi\ResourceType\ResourceTypeRepository::getFields is returning relationship resources without related types set, even though we pass in a valid resource type with a bundle.

Drupal core has a bug in \Drupal\jsonapi\ResourceType\ResourceTypeRepository::getFields.

In the following code, it never sets the relatable types for ResourceTypeRelationship, if it is used. That's why we crash.

      $fields[$field_name] = $is_relationship_field
        ? new ResourceTypeRelationship($field_name, $alias, TRUE, $has_one)
        : new ResourceTypeAttribute($field_name, $alias, TRUE, $has_one);

Actually, this identifies where the bug might keep going.

        $cross_bundle_resource_type->setBundleResourceTypes($resource_types);
        $cross_bundle_resource_type->setRelatableResourceTypes(static::getRelatableResourceTypesSuperset($resource_types, $field_mapping_superset));

In setRelatableResourceTypes it should set the relatable types for all ResourceTypeRelationship.

      $internal_field_name = $this->fieldMapping[$public_field_name];
      $field = $fields[$internal_field_name];
      assert($field instanceof ResourceTypeRelationship);
      $fields[$internal_field_name] = $field->withRelatableResourceTypes($relatable_resource_types[$public_field_name]);
      return $fields;

So we might need to fix \Drupal\jsonapi_cross_bundles\ResourceType\CrossBundleResourceTypeRepository::getRelatableResourceTypesSuperset

Okay, this fixes errors when building caches. I need to run for an appointment, let's see what the test suite says.

Fix param conversion.

- $cross_bundle_resource_types[] = $cross_bundle_resource_type;
+ $cross_bundle_resource_types[$cross_bundle_resource_type->getTypeName()] = $cross_bundle_resource_type;

@mglaman Thanks a lot! Manually testing this patch worked great!

Great! The tests just need to be updated for the new route name

This should resolve the test failures.

Renaming the issue. It's more than just a patch. Cleaned up the resource name and how we calculated related resources.

Exciting to see this green! 🥳

🙌committed! thanks for the assist @dawehner

Thank you @mglaman!