Gracefully handle no data

[amw]

Currently the parser fails with "AttributeError: 'list' object has no attribute 'get'" when the passed JSON is not a dictionary. This causes Server Error (500) response.

The diff looks big, but I've just added the type check and moved the error from the bottom to the beginning. The rest is the same code but unindented.

[codecov-io]

Current coverage is 90.96% (diff: 96.77%)

Merging #290 into develop will decrease coverage by 0.19%

@@            develop       #290   diff @@
==========================================
  Files            49         49          
  Lines          2295       2302     +7   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2092       2094     +2   
- Misses          203        208     +5   
  Partials          0          0          

Powered by Codecov. Last update 1623942...0cfe2e1

[chrisclark]

@amw this seems extremely reasonable, but for completeness what do you think of doing the check like:

if not isinstance(result, dict) or not isinstance(result.get('data'), dict):
    raise ParseError('Received document does not contain primary data')

Then this situation gets handled appropriately:

{
    "data": []
}

Yeah, it's a little reductio-ad-absurdum, but the reason I bring it up is because, without that additional check, the behavior of your code is actually slightly different than the previous behavior. The above payload would appropriately raise a ParseError in the current codebase (because of the check for the truthiness of result.get('data')) whereas it would bomb with another AttributeError with your change.

What do you think? If you can update, I'll merge.

[amw]

I actually have a much larger branch that changes this parser's logic, but I haven't sent it as PR yet as I was awaiting some of my other PRs to be merged.

The purpose of that other branch is to stop special-casing of RelationshipView. I want to be able to implement my own views that take arrays of resources or resource-identifiers, e.g. to write a batch processing endpoint.

I want {"data": []} to be a valid input for these views.

If you want to take a look on that branch you can find it here:
amw@4f77ee1

[chrisclark]

Hm, yeah, that makes sense...but it's not actually part of the spec is it? The spec says:

Resource linkage MUST be represented as one of the following:

• null for empty to-one relationships.
• an empty array ([]) for empty to-many relationships.
• a single resource identifier object for non-empty to-one relationships.
• an array of resource identifier objects for non-empty to-many relationships.

It doesn't appear to allow an array of resource objects. It seems totally reasonable to allow that, but I don't think that change would get merged as it's not part of the spec. Let me know if I've misunderstood. It'd be great to get that as part of the spec, though.

[chrisclark]

To expand just a bit further, I think the way the spec would recommend doing this is to first bulk create the objects at the 'other' end of the relationship (which is supported), and then bulk-create the relationships themselves. So do it in 2 steps, instead of one request to create both the relationships and the objects on other side in 1 go.

You can definitely bulk create resources, just not as part of a relationship payload.

[amw]

You can definitely bulk create resources, just not as part of a relationship payload.

Current parser does not allow bulk creating resources. It expects the data to be dict. It will actually cause 500 HTTP errors when data is an array (data.get('type')).

JSON-API allows extensions, but these are experimental for now. Bulk operations used to be one of the official extensions before the whole extension system was sent back to the drawing board.

You can see the original bulk spec here:
https://github.com/json-api/json-api/blob/9c7a03dbc37f80f6ca81b16d444c960e96dd7a57/extensions/bulk/index.md

I have a need for bulk operations in my app and I've implemented a few custom views to handle them. I don't mind that django-rest-framework-json-api does not come with these views out of the box, but I would like the parser not to reject these types of requests.

Even though there's no spec for handling bulk operations yet, the official spec does allow this kind of payload:

Primary data MUST be either:

• a single resource object, a single resource identifier object, or null, for requests that target single resources
• an array of resource objects, an array of resource identifier objects, or an empty array ([]), for requests that target resource collections

[chrisclark]

Sorry - got sidetracked here with the discussion of your other branch. The reasoning for this change in isolation is sound. Merged.