multimeric / flapison

Flask extension to build REST APIs around JSONAPI 1.0 specification.
http://flask-rest-jsonapi.readthedocs.io
MIT License
13 stars 4 forks source link

fix(schema): Sparse fieldsets broken on Marshmallow 3.x #17

Closed Leechael closed 4 years ago

Leechael commented 4 years ago

In previous Marshmallow 2.x version, we can using sparse fieldsets restrict the fields returned by API:

GET /persons?fields[person]=display_name HTTP/1.1

Then API will only return id and display_name for schema person. But this feature broken when flapison upgrade to marshmallow 3.x.

This patch fixed this problem.

multimeric commented 4 years ago

Ah thanks for pointing this out, I don't think I've actually used fields since upgrading Marshmallow. Can you add a test for this? We definitely need one if this regression wasn't detected by the test suite. You can probably copy another test and just use a small number of fields.

This is a neat solution, but I'm wondering if it might be simpler to just compile the only dictionary before we create the schema here, rather than editing it after and then re-initializing: https://github.com/TMiguelT/flapison/blob/99fa534a730d90ce3c5c0cafa9789175cd0c11fb/flapison/schema.py#L55 This might save us relying on an internal method which is generally good to avoid.

Leechael commented 4 years ago
schema = schema_cls(**schema_kwargs) 

An broken case: Foo.city , Foo.province.city. Using sparse fieldset specified return only city name and using Foo.city.name,Foo.city.region_code will confuse here.

multimeric commented 4 years ago

Closed in favour of #19