Another issue found during our tests with JSON API and JSON API Extras - we have an entity with a relationship to another entity, both of which are using JSON API Extras to override their URL paths and field names. When testing POSTs and PATCHes of this entity, we found that since the incoming data had a relationship type that respected the overrides (i.e. "module" instead of "config_entity--module"), it was throwing an exception as it tried to use the entityTypeManager->getStorage() with the overridden relationship type string.

I have a patch that resolves the issue by fetching the originalID and replacing the relationship data with the correct string if they don't match. There's probably a better way to address this, but it does allow for our records to be POSTed and PATCHed with the same data as they output from a GET.

CommentFileSizeAuthor
#22 2886618--rel-type-conversion.patch5.39 KBdpolant
#18 jsonapi-n2886618-18.interdiff.txt3.43 KBdamienmckenna
#18 jsonapi-n2886618-18.patch7.05 KBdamienmckenna
#17 jsonapi-n2886618-16.interdiff.txt1.95 KBdamienmckenna
#17 jsonapi-n2886618-16.patch4.95 KBdamienmckenna
#14 jsonapi-n2886618-14.interdiff.txt3.46 KBdamienmckenna
#14 jsonapi-n2886618-14.patch4.34 KBdamienmckenna
#12 jsonapi-n2886618-12.interdiff.txt2.79 KBdamienmckenna
#12 jsonapi-n2886618-12.patch4.79 KBdamienmckenna
#9 2886618--overridden-relationships-fail-post--8.patch3.87 KBlogickal
#7 2886618--overridden-relationships-fail-post--7.patch3.6 KBlogickal
#6 2886618--overridden-relationships-fail-post--6.patch1.19 KBlogickal
#2 2886618--overridden-relationships-fail-post--2.patch1.22 KBlogickal

Comments

logickal created an issue. See original summary.

logickal’s picture

logickal commented
StatusFileSize
new 2886618--overridden-relationships-fail-post--2.patch1.22 KB
logickal’s picture

logickal commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 2886618--overridden-relationships-fail-post--2.patch, failed testing. View results

logickal’s picture

logickal commented

Oooops. I used getJsonapiResourceConfig(), which comes from jsonapi_extras. I'll have to find a better way to get the original entity ID back.

logickal’s picture

logickal commented
StatusFileSize
new 2886618--overridden-relationships-fail-post--6.patch1.19 KB

I'm sure there's a better way to do this, but this is at least more straightforward and doesn't require jsonapi_extras.

logickal’s picture

logickal commented
Status: Needs work » Needs review
StatusFileSize
new 2886618--overridden-relationships-fail-post--7.patch3.6 KB

Worked on the unit test to more accurately reflect the case, but feeling like this is a very naive solution and could use more thought.

Status: Needs review » Needs work

The last submitted patch, 7: 2886618--overridden-relationships-fail-post--7.patch, failed testing. View results

logickal’s picture

logickal commented
StatusFileSize
new 2886618--overridden-relationships-fail-post--8.patch3.87 KB

Ok, this patch is passing tests on my local, but it's really only good enough to get our specific implementation unblocked. It seems to me we have an interesting problem to solve here in that if someone uses jsonapi_extras to override type names, it not only breaks the ability to post content with those relationships, but there doesn't seem to be any good way to expose that change back to jsonapi.

I'll freely admit to possibly being dense here, so any additional ideas around this would be greatly appreciated - I'm not sure if perhaps this needs to roll back down to jsonapi_extras to implement its own deserialization functions that would be able to handle any override mappings properly...

mrjmd’s picture

mrjmd commented
Status: Needs work » Needs review
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Needs work

Sorry for dropping the ball here.

+++ b/src/Normalizer/JsonApiDocumentTopLevelNormalizer.php
@@ -79,6 +79,18 @@ class JsonApiDocumentTopLevelNormalizer extends NormalizerBase implements Denorm
+          if (!strpos($relationship['data']['type'], '--')) {
+            $resource_type = $this->currentContext->getResourceType();

Did you try using the $resource_type to see if the name is changed? It should have a name translation method you can use.

You can also get the $type_id from there (I think).

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new jsonapi-n2886618-12.patch4.79 KB
new jsonapi-n2886618-12.interdiff.txt2.79 KB

Loop over each $relationship['data'] item.

Status: Needs review » Needs work

The last submitted patch, 12: jsonapi-n2886618-12.patch, failed testing. View results

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new jsonapi-n2886618-14.patch4.34 KB
new jsonapi-n2886618-14.interdiff.txt3.46 KB

The $resource_type points to the primary object, not the referenced object, as can be seen from this patch. So that doesn't work.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 14: jsonapi-n2886618-14.patch, failed testing. View results

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
StatusFileSize
new jsonapi-n2886618-16.patch4.95 KB
new jsonapi-n2886618-16.interdiff.txt1.95 KB

I missed some changes for that last patch. This better illustrates the issue - $resource_type doesn't point to the referenced entity type.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
StatusFileSize
new jsonapi-n2886618-18.patch7.05 KB
new jsonapi-n2886618-18.interdiff.txt3.43 KB

This appears to work. A key problem I had was that when examining a relationship I didn't know what relationship was currently being processed, so by changing from array_map() to a foreach() I can find that out, and then match it up against the "publicName" of the fields, then identify the correct entity type based upon the settings of the entityreference field.

Food for thought.

Status: Needs review » Needs work

The last submitted patch, 18: jsonapi-n2886618-18.patch, failed testing. View results

logickal’s picture

logickal commented

The big problem with #18 is that it again relies on getJsonapiResourceConfig(), which comes from jsonapi_extras. I ran into this on #2. Maybe there is a case to be made to move that method into the main jsonapi module?

I also know that there are some other relationship issues with metadata that probably need to be solved in parallel with this. so It might make sense to solve both problems with one refactor of denormalize().

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Overridden relationships fail to POST » Overridden relationship fields fail to POST
Project: JSON:API » JSON:API Extras

Moving this to JSON API Extras, because that's where the "override" functionality comes from.

dpolant’s picture

dpolant commented
Status: Needs work » Needs review
StatusFileSize
new 2886618--rel-type-conversion.patch5.39 KB

This patch should take care of translating the relationship > data > type into a viable entity type without relying on methods from JSONAPI Extras.

dpolant’s picture

dpolant commented
Project: JSON:API Extras » JSON:API

I believe this work needs to be done in JSONAPI core, so moving it back.

logickal’s picture

logickal commented

We have this working and fully tested in our stack - @e0ipso, let us know your thoughts on the approach here.

  • e0ipso committed a14dc04 on 8.x-1.x authored by dpolant 
    fix(Selializer): Overridden relationship fields fail to POST (#2886618...
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Fixed

Thanks everyone for the work and the patience. This is a superb patch!

Status: Fixed » Closed (fixed)

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