Skip to content
This repository has been archived by the owner on Jun 6, 2024. It is now read-only.

Confusion around the type for included data. #133

Closed
anthony-arnold opened this issue Jul 3, 2018 · 4 comments
Closed

Confusion around the type for included data. #133

anthony-arnold opened this issue Jul 3, 2018 · 4 comments

Comments

@anthony-arnold
Copy link

There seems to be a little confusion around the correct type for included data.

To quote the JSON API spec:

In a compound document, all included resources MUST be represented as an array of resource objects in a top-level included member.

In marshmalllow_jsonapi.schema.Schema.__init__, the included_data member is initialised as a dict.

In _do_load, the included_data member defaults to a dict. However, it is iterated as a list inside _extract_from_included which would be correct. This works if the payload is an array or undefined, because then _extract_from_included will either iterate the list or iterate the keys of an empty dict.

In render_included_data, the included_data member is expected to have a values method (i.e. dict-like).

The reason I think this has been working so far is because most people are not deserialising payloads with 'included' data. This is because the current spec has no provision for compound document requests (but there are some in draft), only responses.

The spec clearly states the the top-level included member is an array. It should be treated as a list everywhere.

@anthony-arnold
Copy link
Author

anthony-arnold commented Jul 4, 2018

OK, with a little more investigation it seems that during dump, included_data is expected by fields to be a dict keyed by an (id,type) tuple.

During load, included_data takes the value from the input data['included'], which will be a list if it's a compliant JSON API document; which is good because _extract_from_included is expecting it to be a list.

The main problem arises when you use the same Schema instance to do a load of a payload with included data, then a dump. The included_data attribute is not reset, leading to Schema expecting to find a dict, but getting a list instead.

I think that marshmallow_jsonapi.schema.Schema should have a pre_dump method which resets the included_data back to an empty dict.

@AdrianVandierAst
Copy link

This bug arises when using flask-rest-jsonapi. I've done a correction to have a consistent type for included_data. Pull request coming soon.

@AdrianVandierAst
Copy link

The promised pull request: #268
Don't know how to link it :/

@multimeric
Copy link

multimeric commented Nov 8, 2019

So if I understand right, the behaviour is consistent for each task: when dumping, included_data is consistently a dict, and when loading it's consistently a list. But it's only if you re-use a schema for both tasks this causes issues?

@AdrianVandierAst if you write "closes X" anywhere in a comment on your PR, it will link them: https://help.github.com/en/github/managing-your-work-on-github/closing-issues-using-keywords#about-issue-references

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants