Handle relationships when creating resource objects from JSON:API request data

Nice work, @sam711!

The key thing that's missing here, is test coverage.

1. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +185,87 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +    if (!empty($data['data']['relationships'])) {
   +      // Turn all single object relationship data fields into an array of
   +      // objects.
   +      $relationships = array_map(function ($relationship) {
   

   This is exactly like \Drupal\jsonapi\Normalizer\JsonApiDocumentTopLevelNormalizer::denormalize()!

   Therefore … this is also related to and would benefit from #3017295: Refactor/clean up JsonApiDocumentTopLevelNormalizer::denormalize() and EntityNormalizer::denormalize().

   Pinging @gabesullice.

2. +++ b/src/Unstable/Value/NewResourceObject.php
   @@ -37,8 +37,7 @@ final class NewResourceObject extends ResourceObject {
   -      // @todo: handle relationships.
   

   🥳

Superb, thank you so much! 😃 🙏

Will review :) The changes in #3104972: Move extracting documents from request objects to a service would cause a lot of disruption on this patch (everything moves to a new file.) So it would be easier to review and get this landed first.

1. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -115,6 +139,9 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +    $primary_data['relationships'] = $this->handleRelationships($decoded);
   
   @@ -158,4 +185,87 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +  protected function handleRelationships(array $data): array {
   +    if (!empty($data['data']['relationships'])) {
   ...
   +      }, $data['data']['relationships']);
   

   It looks like we only ever use $data['data']. Perhaps we should only pass the document data? handleRelationships shouldn't need anything else from the top level document.

2. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +185,87 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +      // Get an array of ids for every relationship.
   +      $relationships = array_map(function ($relationship) {
   

   There's a lot of logic happening in this closure. Should we move to a regular foreach()?

3. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +185,87 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +        if (empty($relationship['data'][0]['id'])) {
   +          throw new BadRequestHttpException("No ID specified for related resource");
   +        }
   +        $id_list = array_column($relationship['data'], 'id');
   +        if (empty($relationship['data'][0]['type'])) {
   +          throw new BadRequestHttpException("No type specified for related resource");
   +        }
   +        if (!$resource_type = $this->resourceTypeRepository->getByTypeName($relationship['data'][0]['type'])) {
   +          throw new BadRequestHttpException("Invalid type specified for related resource: '" . $relationship['data'][0]['type'] . "'");
   +        }
   

   What if another delta is malformed? Would this catch it?

4. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +185,87 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +        return array_filter($canonical_ids);
   

   I don't think we'd ever have a nil reference item value to require an array filter

5. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +185,87 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +    return $relationships ?? [];
   

   Relationships should always be an array if it values is determined from an array map

Here's my initial review. Not enough to warrant "Needs work", but general thoughts.

Thanks @sam711! I'll eyeball it in full later today. But it looks great, we have tests written and I think anything else can be discovered in follow ups.

1. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +187,85 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +      $id_list = array_column($relationship['data'], 'id');
   

   I totally missed this before, which is how we get all of the IDs for the relationships.

2. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +187,85 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +        throw new BadRequestHttpException("Invalid type specified for related resource: '" . $relationship['data'][0]['type'] . "'");
   ...
   +        throw new BadRequestHttpException("Invalid type specified for related resource: '" . $relationship['data'][0]['type'] . "'");
   

   nit on commit, probably move to sprintf

3. +++ b/src/Unstable/Controller/ArgumentResolver/DocumentResolver.php
   @@ -158,4 +187,85 @@ final class DocumentResolver implements ArgumentValueResolverInterface {
   +      $uuid_key = $this->entityTypeManager
   +        ->getDefinition($entity_type_id)->getKey('uuid');
   +      $related_entities = array_values($entity_storage->loadByProperties([$uuid_key => $id_list]));
   

   Hm. Actually, if we don't need the entity manager for anything but this, we could just inject the entity_repository which has loadByUuid.

Hm. Actually, if we don't need the entity manager for anything but this, we could just inject the entity_repository which has loadByUuid.

Oh duh, we need to load multiple.

Making sure to credit @Wim Leers

Woo! Thank you @sam711!

Don't the changes in #19 mean that #7.1 is no longer true, and hence #3017295: Refactor/clean up JsonApiDocumentTopLevelNormalizer::denormalize() and EntityNormalizer::denormalize() just became more difficult? 🤔