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

Many to many key error #596

Open upmauro opened 8 years ago

upmauro commented 8 years ago

API

manager.create_api(Vehicle, methods=['GET', 'POST', 'DELETE'], preprocessors=dict(GET_SINGLE=[auth_func], GET_MANY=[auth_func]), primary_key='id')

MODELS

class Vehicle(db.Model):
    __tablename__ = 'vehicle'

    id = db.Column(db.Integer, primary_key=True)
    plate = db.Column(db.String(8))
    title = db.Column(db.String(255))
    fleet_number = db.Column(db.Integer)
    fleet_id = db.Column(db.ForeignKey(u'fleet.id'), index=True)
    vehicle_model_id = db.Column(db.ForeignKey(u'vehicle_model.id'), index=True)
    color = db.Column(db.Integer)

    fleet = db.relationship(u'Fleet', primaryjoin='Vehicle.fleet_id == Fleet.id', backref=u'vehicles')
    vehicle_model = db.relationship(u'VehicleModel', primaryjoin='Vehicle.vehicle_model_id == VehicleModel.id', backref=u'vehicles')

class VehicleDevice(db.Model):
    __tablename__ = 'vehicle_device'

    id = db.Column(db.Integer, primary_key=True)
    device_id = db.Column(db.ForeignKey(u'device.id'), nullable=False, index=True)
    vehicle_id = db.Column(db.ForeignKey(u'vehicle.id'), nullable=False, index=True)

    device = db.relationship(u'Device', primaryjoin='VehicleDevice.device_id == Device.id', backref=u'vehicle_devices')
    vehicle = db.relationship(u'Vehicle', primaryjoin='VehicleDevice.vehicle_id == Vehicle.id', backref=u'vehicle_devices')

Error

File "/home/upmauro/Work/journey/wr-platform-backend/env/src/flask-restless/flask_restless/serialization/serializers.py", line 427, in _dump for rel in relations) File "/home/upmauro/Work/journey/wr-platform-backend/env/src/flask-restless/flask_restless/serialization/serializers.py", line 427, in for rel in relations) File "/home/upmauro/Work/journey/wr-platform-backend/env/src/flask-restless/flask_restless/serialization/serializers.py", line 107, in create_relationship related_model = get_related_model(model, relation) File "/home/upmauro/Work/journey/wr-platform-backend/env/src/flask-restless/flask_restless/helpers.py", line 146, in get_related_model attribute = mapper.all_orm_descriptors[relationname] File "/home/upmauro/Work/journey/wr-platform-backend/env/lib/python2.7/site-packages/sqlalchemy/util/_collections.py", line 195, in getitem return self._data[key] KeyError: u'vehicle_devices'

jfinkels commented 8 years ago

Hi @upmauro! Thanks for the issue report. Can you provide a minimal working example (i.e. without the irrelevant columns, preprocessors, etc.) that demonstrates this bug, so that I can run the code on my own machine to try and debug it?

upmauro commented 8 years ago

First of all, thanks for your great project.

I created snippet to reproduce standalone (sorry my bad code, i'm studying).

Source

https://gist.github.com/upmauro/3534c43ab8688bee9d2a86438f23958e

Error (when try GET /api/table_a)

File "/home/upmauro/Work/journey/wr-platform-backend/env/src/flask-restless/flask_restless/serialization/serializers.py", line 107, in create_relationship related_model = get_related_model(model, relation) File "/home/upmauro/Work/journey/wr-platform-backend/env/src/flask-restless/flask_restless/helpers.py", line 146, in get_related_model attribute = mapper.all_orm_descriptors[relationname] File "/home/upmauro/Work/journey/wr-platform-backend/env/lib/python2.7/site-packages/sqlalchemy/util/_collections.py", line 194, in getitem return self._data[key] KeyError: u'table_abs'

jfinkels commented 8 years ago

This definitely seems to be a bug. Many-to-many relationships are basically untested in the development version of Flask-Restless right now. See also issue #480.

I will work on debugging this. In the minimal working example you provided (thanks!), what kind of response would you expect from the request GET /api/table_a/1? Specifically, which attributes and relationships would you expect to see by default (see the documentation section on fetching resources for an example response)?

upmauro commented 8 years ago

Hi!

I can't find specs about this situation on http://jsonapi.org/ but in my opnion when you GET "TableA" a element what the schema is referenced by a many to many schema "TableAB", return just information about the main element "TableA". If you need informations about both, call GET on "TableAB"

What do you think?

Sorry my bad english.

jfinkels commented 8 years ago

Let me clarify my question. On a request GET /api/table_a/1, there are two possible responses I can envision, and Flask-Restless needs to decide on which one to provide by default. One includes the related instances of TableAB, and one includes the remote related instances of TableB. For example

HTTP/1.1 200 OK
Content-Type: application/vnd.api+json

{
  "data": {
    "id": "1",
    "links": {
      "self": "http://example.com/api/table_a/1"
    },
    "relationships": {
      "table_abs": {
        "data": [
          {
            "id": 1,
            "type": "table_ab"
          },
          ...
        ],
        "links": {
          "related": "http://example.com/api/table_a/1/",
          "self": "http://example.com/api/table_a/1/relationships/table_abs"
        }
      }
    },
    "type": "person"
  }
}

or

HTTP/1.1 200 OK
Content-Type: application/vnd.api+json

{
  "data": {
    "id": "1",
    "links": {
      "self": "http://example.com/api/table_a/1"
    },
    "relationships": {
      "tableb": {
        "data": [
          {
            "id": 1,
            "type": "table_b"
          },
          ...
        ],
        "links": {
          "related": "http://example.com/api/table_a/1/",
          "self": "http://example.com/api/table_a/1/relationships/tableb"
        }
      }
    },
    "type": "person"
  }
}

Perhaps in most cases the information that the client really cares about is in TableB, not in the association table, so the latter type of response makes more sense than the former. What do you think?

upmauro commented 8 years ago

@jfinkels , second option looks perfect to me.