Skip to content

Conversation

@factyy
Copy link

@factyy factyy commented Aug 31, 2023

This PR enables rendering multiple included resources together by grouping them in arrays first (if their jsonapi_type and jsonapi_id are the same) and then rendering them one-by-one while merging the rendering results.

Rationale: we use Rails and Graphiti and encountered the following problem: we added a few relationships (e.g. specific) that are not represented in corresponding models to Graphiti Resources (that are basically JSON:API wrappers). Then we have a child resource (that obviously goes into @included array) that contains these specific relationships and is referenced by two different paths from a parent resource (e.g. path1 and path2). Then we have a complex request for the parent resource, including two references to the same child object with the first reference requesting the specific resource while the second reference is not (something like include=path1.specific,path2). When Graphiti is processing the request it enriches the first referenced object with these specific relationships while leaving the second object alone with no modifications. In this case we have two different objects with the same jsonapi_type and jsonapi_id but with slightly different behavior. Sadly when squashing the references tree into simple @primary and @included objects (in traverse_resources) we enrich all includes (by merging includes in the @include_rels) but we leave only one resource (which may miss these specific relationships). And so when trying to process includes from this resource we end up with a relationship with data: nil since this resource misses the specific relationship.

BTW it is interesting that somehow the rightmost reference in the include parameter is processed first, so the result is dependent on the order of includes. If we use include=path2,path1.specific we will have proper data in the response.

Edit: it is not the rightmost reference that matters but a combination of a field (relationship) definition inside the resource and the include reference (which field is used in the include section)

@beauby
Copy link
Member

beauby commented Aug 31, 2023

In this case we have two different objects with the same jsonapi_type and jsonapi_id but with slightly different behavior.

This does not seem in line with the spec:

Within a given API, each resource object’s type and id pair MUST identify a single, unique resource.

@factyy
Copy link
Author

factyy commented Aug 31, 2023

Correct me if I'm wrong, the following:

resource object’s type and id pair MUST identify a single, unique resource

is achieved by squashing the resource tree into the @included array with unique resources which is then rendered (with no duplicates). But we are talking right now about the internals (the internal resource tree, not the API itself) and internals are not covered by the spec.

Quite frankly I don't know whether it is better to solve this problem when the request tree is constructed (i.e. store a reference to a single resource on duplicates) or when rendered (which this PR do, we render duplicates as a single unique resource, just as the spec says, not changing the behavior).

Edit: fixed a typo and added additional clarification.

@factyy
Copy link
Author

factyy commented Sep 2, 2023

@beauby , don't want to bother, but perhaps I should have rephrased the description since it looks like I somehow missed the main point: this PR doesn't enable rendering duplicates (which contradicts the spec), it only allows to render one unique resource with all required relationships.

Please take a look at the previous comment too.

Thanks

@factyy
Copy link
Author

factyy commented Jul 8, 2024

Decided to move forward without changing the rendering mechanics

@factyy factyy closed this Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants