This is already the case for related responses with cardinality one. This would unify that behavior and validate the idea of using the Data class as the "primary data" for a response.

Comments

gabesullice created an issue. See original summary.

gabesullice’s picture

Assigned: Unassigned » gabesullice
Status: Active » Needs work
StatusFileSize
new9.96 KB

WIP

gabesullice’s picture

Status: Needs work » Needs review
StatusFileSize
new3.2 KB
new11.76 KB

Still ongoing. Just getting a test run.

gabesullice’s picture

StatusFileSize
new454 bytes
new11.32 KB

Whoops

The last submitted patch, 3: 3035149-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 4: 3035149-4.patch, failed testing. View results

gabesullice’s picture

Status: Needs work » Needs review
StatusFileSize
new4.47 KB
new14.54 KB

Status: Needs review » Needs work

The last submitted patch, 7: 3035149-7.patch, failed testing. View results

gabesullice’s picture

Assigned: gabesullice » Unassigned
Status: Needs work » Needs review
StatusFileSize
new3.22 KB
new17.76 KB

Ready for review.

gabesullice’s picture

This 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.

wim leers’s picture

  1. +++ b/src/Controller/EntityResource.php
    @@ -918,6 +922,7 @@ class EntityResource {
    +    assert($data instanceof Data || $data instanceof FieldItemListInterface);
    

    The second case would disappear once Relationship exists, i.e. once #3036285: Add a \JsonApiResource\Relationship object to carry relationship data, metadata and a link collection. is done?

  2. +++ b/src/Controller/EntityResource.php
    @@ -1024,6 +1030,7 @@ class EntityResource {
       public function getIncludes(Request $request, $data) {
    +    assert($data instanceof ResourceObject || $data instanceof ResourceObjectData);
    

    Why can this be both?

  3. +++ b/src/IncludeResolver.php
    @@ -64,9 +64,8 @@ class IncludeResolver {
       public function resolve($data, $include_parameter) {
    -    assert($data instanceof ResourceIdentifierInterface || $data instanceof ResourceObjectData);
    ...
    +    assert($data instanceof ResourceObject || $data instanceof ResourceObjectData);
    

    Same question here.

  4. +++ b/src/JsonApiResource/LinkCollection.php
    @@ -24,7 +24,7 @@ final class LinkCollection implements \IteratorAggregate {
    -   * @var \Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel
    +   * @var \Drupal\jsonapi\JsonApiResource\JsonApiDocumentTopLevel|\Drupal\jsonapi\JsonApiResource\ResourceObject
    

    👍Fixes small oversight in #3015438: Wrap entity objects in a ResourceObject which carries a ResourceType.

gabesullice’s picture

1. Yes.
2. Because of EntityResource::getRelationship. The context of the relationship is an individual resource object. This will become assert(ResourceObjectData || Relationship) in [##3036285] too.
3. idem
4. :)

wim leers’s picture

I 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.

+++ b/src/Controller/FileUpload.php
@@ -184,7 +185,8 @@ class FileUpload {
-    return new ResourceResponse(new JsonApiDocumentTopLevel(new ResourceObject($file_resource_type, $file), new NullData(), $links), 201, []);
+    $resource_object = new ResourceObject($file_resource_type, $file);
+    return new ResourceResponse(new JsonApiDocumentTopLevel(new ResourceObjectData([$resource_object], 1), new NullData(), $links), 201, []);

+++ b/src/Normalizer/DataNormalizer.php
@@ -0,0 +1,34 @@
+class DataNormalizer extends NormalizerBase {

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 EntityCollection to 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.

gabesullice’s picture

Status: Needs review » Reviewed & tested by the community

#13 is correct. My only quibble would be that I think the indirection is there, regardless. This is only making it more explicit.

wim leers’s picture

gabesullice’s picture

Status: Reviewed & tested by the community » Needs work

JUST realized a really stupid mistake.

The DataNormalizer is not in jsonapi.services.yml.

gabesullice’s picture

Status: Needs work » Needs review
StatusFileSize
new1.15 KB
new16.63 KB

The whole DataNormalizer class was supposed to be in #3036286: Clean-up: JsonApiDocumentTopLevelNormalizer and children to prep for further clean-ups, which makes this a much smaller change.

wim leers’s picture

Status: Needs review » Reviewed & tested by the community

Ahhh, that settles my one concern! Will commit when green :) (And it obviously should be, but I'll still wait, because typos are easily introduced.)

wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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