Closed (fixed)
Project:
JSON:API
Version:
8.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
22 Feb 2019 at 19:29 UTC
Updated:
14 Mar 2019 at 17:19 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gabesulliceWIP
Comment #3
gabesulliceStill ongoing. Just getting a test run.
Comment #4
gabesulliceWhoops
Comment #7
gabesulliceComment #9
gabesulliceReady for review.
Comment #10
gabesulliceThis would be ever so slightly easier if #3036286 landed. But it's not worth postponing. It'd be an extremely simple re-roll in both directions.
Comment #11
wim leersThe second case would disappear once
Relationshipexists, i.e. once #3036285: Add a \JsonApiResource\Relationship object to carry relationship data, metadata and a link collection. is done?Why can this be both?
Same question here.
👍Fixes small oversight in #3015438: Wrap entity objects in a ResourceObject which carries a ResourceType.
Comment #12
gabesullice1. Yes.
2. Because of
EntityResource::getRelationship. The context of the relationship is an individual resource object. This will becomeassert(ResourceObjectData || Relationship)in [##3036285] too.3. idem
4. :)
Comment #13
wim leersI find this issue a bit too light on the explanation and justification side. So I'm RTBC'ing this with my understanding of the rationale, if it's wrong, please un-RTBC. Ideally we'd also get @effulgentsia to +1, since he was involved with related/preceding issues.
This normalizer is now necessary because there is now one extra level of indirection in the data structure, right?
That extra level of indirection buys us the benefit of dealing with "data" generically: be it includes, resource objects or relationships.
This paves the path to let contrib modules in the future provide access to non-entity data in JSON:API-compliant responses (see #3032787: [META] Start creating the public PHP API of the JSON:API module).
I first started this by opening #3032679: Clean-up: rename EntityCollection to Data to rename
EntityCollectionto something that is not tightly coupled to entities, and that itself was triggered by #3015438: Wrap entity objects in a ResourceObject which carries a ResourceType, which was the first essential step.Assuming this is correct, RTBC.
Comment #14
gabesullice#13 is correct. My only quibble would be that I think the indirection is there, regardless. This is only making it more explicit.
Comment #16
wim leersComment #17
gabesulliceJUST realized a really stupid mistake.
The
DataNormalizeris not injsonapi.services.yml.Comment #18
gabesulliceThe whole
DataNormalizerclass was supposed to be in #3036286: Clean-up: JsonApiDocumentTopLevelNormalizer and children to prep for further clean-ups, which makes this a much smaller change.Comment #19
wim leersAhhh, that settles my one concern! Will commit when green :) (And it obviously should be, but I'll still wait, because typos are easily introduced.)
Comment #21
wim leers