Problem/Motivation

When responding to a request on a "relationships" endpoint with an include query parameter, the included objects in the response have their attributes inside an extraneous "data" key. This violates spec.

For example, a request to /jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/relationships/uid?_format=api_json&include=uid returns the following response:

{
    "data": {
        "type": "user--user",
        "id": "ba316f6a-143a-4263-9841-b66628564e93"
    },
    "links": {
        "self": "http://drupal-8x.dd:8080/jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/relationships/uid?_format=api_json",
        "related": "http://drupal-8x.dd:8080/jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/uid?_format=api_json"
    },
    "included": [
        {
            "data": {
                "type": "user--user",
                "id": "ba316f6a-143a-4263-9841-b66628564e93",
                "attributes": {
                    "uid": "1",
                    "uuid": "ba316f6a-143a-4263-9841-b66628564e93",
                    "langcode": "en",
                    "name": "admin",
                    "created": "1483459313",
                    "changed": "1483459313",
                    "default_langcode": "1"
                },
                "relationships": {
                    "user_picture": {
                        "data": null
                    }
                },
                "links": {
                    "self": "http://drupal-8x.dd:8080/jsonapi/user/user/ba316f6a-143a-4263-9841-b66628564e93?_format=api_json"
                }
            },
            "links": {
                "self": "http://drupal-8x.dd:8080/jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/relationships/uid?_format=api_json&include=uid&XDEBUG_SESSION_START=PHPSTORM"
            }
        }
    ]
}

Proposed resolution

Don't generate the "data" level when serializing these objects.

So, this instead:

{
    "data": {
        "type": "user--user",
        "id": "ba316f6a-143a-4263-9841-b66628564e93"
    },
    "links": {
        "self": "http://drupal-8x.dd:8080/jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/relationships/uid?_format=api_json",
        "related": "http://drupal-8x.dd:8080/jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/uid?_format=api_json"
    },
    "included": [
            {
                "type": "user--user",
                "id": "ba316f6a-143a-4263-9841-b66628564e93",
                "attributes": {
                    "uid": "1",
                    "uuid": "ba316f6a-143a-4263-9841-b66628564e93",
                    "langcode": "en",
                    "name": "admin",
                    "created": "1483459313",
                    "changed": "1483459313",
                    "default_langcode": "1"
                },
                "relationships": {
                    "user_picture": {
                        "data": null
                    }
                },
                "links": {
                    "self": "http://drupal-8x.dd:8080/jsonapi/user/user/ba316f6a-143a-4263-9841-b66628564e93?_format=api_json"
                }
            },
            "links": {
                "self": "http://drupal-8x.dd:8080/jsonapi/node/article/4f342419-e668-4b76-9f87-7ce20c436169/relationships/uid?_format=api_json&include=uid&XDEBUG_SESSION_START=PHPSTORM"
            }
    ]
}

Remaining tasks

CommentFileSizeAuthor
#16 included_objects-2841096-16.patch12.29 KBhampercm
#10 interdiff-2841096-6-10.txt11.22 KBhampercm
#10 included_objects-2841096-10.patch13.14 KBhampercm
#6 included_objects_2841096-6.patch2.44 KBhampercm
#4 included_objects_2841096-4.patch2.44 KBhampercm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hampercm created an issue. See original summary.

hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
Issue summary: View changes
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
Related issues: +#2829762: Remove wrapping "data" element from the compound docs section
hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
Status: Active » Needs review
FileSize
included_objects_2841096-4.patch2.44 KB

Status: Needs review » Needs work

The last submitted patch, 4: included_objects_2841096-4.patch, failed testing.

hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
FileSize
included_objects_2841096-6.patch2.44 KB
hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: included_objects_2841096-6.patch, failed testing.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented

I believe there is something going on with the testbot when applying patches.

I know this was not introduced in this issue, but it seems a relevant place to fix it. Can you rename DocumentRootNormalizerValue to JsonApiDocumentTopLevelNormalizerValue for consistency?

hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
FileSize
included_objects-2841096-10.patch13.14 KB
interdiff-2841096-6-10.txt11.22 KB

Renamed DocumentRootNormalizerValue and DocumentRootNormalizerValueInterface

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue summary: View changes
Status: Needs work » Reviewed & tested by the community

I've updated the IS to show the before/after. 

+++ b/tests/src/Functional/JsonApiFunctionalTest.php
@@ -296,10 +296,11 @@ class JsonApiFunctionalTest extends BrowserTestBase {
-      $first_include['data']['type']
+      $first_include['type']

This proves that that change is indeed made by this patch.

Looks ready to me!

Can you rename DocumentRootNormalizerValue to JsonApiDocumentTopLevelNormalizerValue for consistency?

+1 to this. Although I would argue it's out of scope. The patch in #6 fixes the actual bug. It's 2.5 K. Doing this too makes the patch 11 K.

Of course, e0ips0's call :)

Also, this is going to clash quite epically with #2842378: Remove all remaining interfaces in the JSON API module that have only one implementation. This one is much smaller and will be easier to reroll, so I'd prefer that one to go in first. But, if this goes in first, that's fine too — it's not that big of a deal.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: included_objects-2841096-10.patch, failed testing.

hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
Related issues: +#2842378: Remove all remaining interfaces in the JSON API module that have only one implementation

I'm fine with postponing and re-rolling after #2842378: Remove all remaining interfaces in the JSON API module that have only one implementation goes through.

e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented

#2842378: Remove all remaining interfaces in the JSON API module that have only one implementation is merged and this should be unblocked now.

hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
Assigned: Unassigned » hampercm

Rerolling the patch for this...

hampercm’s picture

hampercm CreditAttribution: hampercm at Acquia commented
Assigned: hampercm » Unassigned
Status: Needs work » Needs review
FileSize
included_objects-2841096-16.patch12.29 KB

Rerolled the patch. No interdiff due to rebase.

  • e0ipso committed ea8129f on 8.x-1.x authored by hampercm 
    fix(Serialization) Discard "data" key for relationship enpoint includes...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
CreditAttribution: e0ipso as a volunteer and at Lullabot commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.