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

QueryStringManager use wrong schema from marshmallow `class_registry` #21

Closed pacoyang closed 2 years ago

pacoyang commented 4 years ago

I define a schema use in admin api:

class PostSchema(Schema):
    class Meta:
        type_ = 'posts'
        strict = True

    id = fields.Int(as_string=True)
    title = fields.Str()
    tags = fields.List(fields.Str())

And then I define another schema use in web api which has the same type_ name, but exclude the field tags.

class PostSchema(Schema):
    class Meta:
        type_ = 'posts'
        strict = True

    id = fields.Int(as_string=True)
    title = fields.Str()

When I use ResourceDetail in admin api, request with param fields[posts]=title,tags, the method get_schema_from_type will return the schema define in web api which is not my expect. This issue because of get_schema_from_type find schema by it's type name and return the first match schema. Refer #21 .

multimeric commented 4 years ago

Can you not solve this issue using exclude? I've done a similar thing with my own project: https://github.com/ewels/MegaQC/blob/59bc2a0d0b05458af2934740ea5bcea3590bd78d/megaqc/rest_api/views.py#L238-L245

pacoyang commented 4 years ago

Yes, I can solve this issue in some way like using exclude or use different type_ name in admin api schema, but I think use type_ to locate the schema is not strict.

I will use the exclude to fix this issue in current 😄.

multimeric commented 4 years ago

I don't think it's unreasonable to assume that there is only one schema per type_. Otherwise, how can we possibly decide which schema to use in this situation?

pacoyang commented 4 years ago

We have already define which schema use in resource and relationship field.

class PostList(ResourceList):
    schema = PostSchema
    ...
class PostSchema(Schema):
    class Meta:
        type_ = 'posts'
        strict = True

    id = fields.Int(as_string=True)
    title = fields.Str()
    tags = Relationship(TagSchema, type_='tags', many=True)
multimeric commented 4 years ago

Hmm that's true. In that case I suppose you could first check the current self.schema and its immediate relationships for a more specific schema to use, and otherwise fall back on the schema registry.

I don't particularly like the idea of having multiple schemas with the same name in the same API, but I suppose if you really need that use-case I would accept a PR adding this functionality.