jfinkels / flask-restless

NO LONGER MAINTAINED - A Flask extension for creating simple ReSTful JSON APIs from SQLAlchemy models.
https://flask-restless.readthedocs.io
GNU Affero General Public License v3.0
1.02k stars 301 forks source link

Regression: GET_SINGLE requests to parent class do not find instances of child classes #614

Open Natureshadow opened 7 years ago

Natureshadow commented 7 years ago

Considering having a model called A and a model called B, where B is a sublcass of A, there is an issue with getting single resources. A and B are polymorphic in SQLAlchemy.

GETing /api/A returns a list of all instances of A and B, but GETing /api/A/1 returns 404 if the object with id 1 is an instance of class B.

It should be possible to get resources of a child model by GETing it through the parent model endpoint.

Natureshadow commented 7 years ago

This used to work in 0.17.0 by the way.

jfinkels commented 7 years ago

This is a design decision I made in order for the different request methods (GET, POST, etc.) to be more uniform and to fit more cleanly with the requirements of the JSON API specification. See https://github.com/jfinkels/flask-restless/issues/200#issuecomment-220523164 for a summary of what is supported and what is not supported.

Basically, if we allow fetching from /api/A/1 then the response will be a JSON API resource object of type B: {'id': 1, 'type': 'B'}. Now what should its self URL be? Should requests to /api/B/1 be allowed? etc. There are many small issues like this, and I'm happy to receive advice on how to handle them.

Natureshadow commented 7 years ago

I think as /api/A lists the resource, then it should also be available as /api/A/1. You either recognize as also being an instance of A or you don't, but you don't accept that it is of class A in GET_MANY, then deny it in GET_SINGLE.

its self link should probably point to the real type URL (it does so when being listed in /api/A, right?).

That would also match the polymorphism in SQLAlchemy.

Natureshadow commented 7 years ago

Maybe it's even simpler, and you should really just rely on how SQLAlchemy is configured. That is, if the resource can be found by A.get(1) in Flask-SQLAlchemy, then Flask-Restless should deliver it. As this is nothing that would violate JSONAPI, Flask-Restless should choose to just not divert from Flask-SQLAlchemy's behaviour.

jfinkels commented 7 years ago

I guess following SQLAlchemy's lead makes sense. Let's first locate the relevant test cases that need to change in tests/test_polymorphism.py, then work from there.