inveniosoftware / flask-resources

REST APIs for Flask
https://flask-resources.readthedocs.io
MIT License
3 stars 21 forks source link

loader: class structure #40

Closed ppanero closed 4 years ago

ppanero commented 4 years ago

Instead of defining one deserializer + arg parser per mimetype, @fenekku proposes:

Having RequestLoader know the different mimetype deserializations. Also we could call args_parsing, deserializing bc it deserializes the querystring in a way:


request_loader = RequestLoader(
    body_deserializers={
        "application/json": JSONDeserializer,
        "application/marcxml": MARCXMLDeserializer
    },
    args_deserializers={...}
)   
ppanero commented 4 years ago

The structure you propose for the RequestLoader could be an option... But it breaks the symmetry with the output (Response) where there is only one serializer. In addition it requires custom logic and might make testing a bit more complex, not a lot. But I think is simpler the way it is. I think this will become clearer once we start using it.

fenekku commented 4 years ago

breaks the symmetry with the output (Response) where there is only one serializer.

Yeah... I am thinking more and more that body serialization/deserialiation and maybe header extraction/injection are the only symmetrical parts

Request coming in --> route parsing -> querystring parsing -> (header parsing ->) body parsing (mimetype dependent) i.e. deserializing -> business
Response coming out <-- -- <- default http code if not overridden before <- header making <- body serializing business

Agreed this will become clearer once we use it.

ppanero commented 4 years ago

Nice diagram! I sort of agree, but I think there are corner cases, which in the end will define how we can do things (e.g. some will not need querystring parsing. They can still pass by the step and return nothing, but the issue is how we do that clean in a programatic manner).

fenekku commented 4 years ago

This was originally created from a comment I made and things have changed since. I am closing it in favour of: https://github.com/inveniosoftware/flask-resources/issues/73