marshmallow-code / marshmallow-sqlalchemy

SQLAlchemy integration with marshmallow
https://marshmallow-sqlalchemy.readthedocs.io
MIT License
558 stars 95 forks source link

Foreign key value disregarded during .load() #20

Open sssilver opened 9 years ago

sssilver commented 9 years ago

Hello,

With roughly the following models:

class Tariff(rod.model.db.Model, rod.model.PersistentMixin):
    id = sqlalchemy.schema.Column(sqlalchemy.types.Integer, primary_key=True)
    title = sqlalchemy.schema.Column(sqlalchemy.types.String())

    # Course this tariff belongs to
    course_id = sqlalchemy.schema.Column(sqlalchemy.types.Integer, sqlalchemy.schema.ForeignKey('course.id'))
    course = sqlalchemy.orm.relationship(
        'Course',
        back_populates='tariffs'
    )

    # Price of this payment plan
    price = sqlalchemy.schema.Column(sqlalchemy.types.Integer)
class Course(rod.model.db.Model, rod.model.PersistentMixin):
    id = sqlalchemy.schema.Column(sqlalchemy.types.Integer, primary_key=True)
    title = sqlalchemy.schema.Column(sqlalchemy.types.String())

    levels = sqlalchemy.orm.relationship(
        'Level',
        back_populates='course'
    )

    tariffs = sqlalchemy.orm.relationship(
        'Tariff',
        back_populates='course'
    )

And the following schemas:


class CourseSchema(rod.model.BaseSchema):
    class Meta(rod.model.BaseSchema.Meta):
        model = rod.model.course.Course

    tariffs = marshmallow.fields.Nested('TariffSchema', many=True, exclude=('course',))

class TariffSchema(rod.model.BaseSchema):
    class Meta(rod.model.BaseSchema.Meta):
        model = rod.model.tariff.Tariff

    course = marshmallow.fields.Nested(CourseSchema)

I'm POSTing the following body: {"course":{"id":1},"course_id":1,"title":"uu","price":"222"}

To my handler:

    tariff_obj = rod.model.schemas.TariffSchema().load(flask.request.json).data
    rod.model.db.session.add(tariff_obj)
    rod.model.db.session.commit()

    return flask.jsonify(rod.model.schemas.TariffSchema().dump(tariff_obj).data)

However, for some reason after load()ing the JSON dump, my tariff_obj.course_id is None, and so it tariff_obj.course.id, even though the rest of the tariff_obj.course looks like a proper (albeit new) Course class instance.

Is this a bug?

Thanks.

dpwrussell commented 9 years ago

What is happening here is that you are running into the fact that primary_keys are by default in marshmallow-sqlalchemy set to dump_only.

So when you deserialize your data, primary key fields will be silently ignored and set to None.

I've formulated this into something runnable:

import sqlalchemy
from sqlalchemy.ext.declarative import declarative_base
import marshmallow
from marshmallow_sqlalchemy import ModelSchema, field_for

Base = declarative_base()

class Tariff(Base):
    __tablename__ = 'tariff'

    id = sqlalchemy.schema.Column(sqlalchemy.types.Integer, primary_key=True)
    title = sqlalchemy.schema.Column(sqlalchemy.types.String())

    # Course this tariff belongs to
    course_id = sqlalchemy.schema.Column(sqlalchemy.types.Integer, sqlalchemy.schema.ForeignKey('course.id'))
    course = sqlalchemy.orm.relationship(
        'Course',
        back_populates='tariffs'
    )

    # Price of this payment plan
    price = sqlalchemy.schema.Column(sqlalchemy.types.Integer)

class Course(Base):
    __tablename__ = 'course'

    id = sqlalchemy.schema.Column(sqlalchemy.types.Integer, primary_key=True)
    title = sqlalchemy.schema.Column(sqlalchemy.types.String())

    # levels = sqlalchemy.orm.relationship(
    #     'Level',
    #     back_populates='course'
    # )

    tariffs = sqlalchemy.orm.relationship(
        'Tariff',
        back_populates='course'
    )

engine = sqlalchemy.create_engine('sqlite:///:memory:', echo=False)
Base.metadata.create_all(engine)
Session = sqlalchemy.orm.sessionmaker(bind=engine)
session = Session()

class CourseSchema(ModelSchema):
    class Meta:
        model = Course
        sqla_session = session

    tariffs = marshmallow.fields.Nested('TariffSchema', many=True, exclude=('course',))
    # id = field_for(Course, 'id', dump_only=False)

class TariffSchema(ModelSchema):
    class Meta:
        model = Tariff
        sqla_session = session

    course = marshmallow.fields.Nested(CourseSchema)
    # id = field_for(Tariff, 'id', dump_only=False)

# I removed 'course_id:999' as that does nothing.
j = {"course":{"id":999},"title":"uu","price":"222"}

tariff_obj = TariffSchema().load(j).data
print 'Before Commit', tariff_obj.course_id, tariff_obj.course.id

session.add(tariff_obj)
session.commit()

print 'After Commit', tariff_obj.course_id, tariff_obj.course.id

This will output:

Before Commit None None
After Commit 1 1

I changed the id's in the j data to have a course_id of 999 so that after the commit it was obvious that this had been ignored in favour of the auto-generated ids.

If you comment in # id = field_for(Course, 'id', dump_only=False) then you can make that field be deserialized.

Before Commit None 999
After Commit 999 999

This is something that I have been thinking about myself as it makes 'edit' functionality difficult. There is a ticket over at ColanderAlchemy with the idea that there should be a toggle between edit/add. This makes a lot of sense to me and I think it is worth a discussion with @sloria about potentially implementing in Marshmallow.

dpwrussell commented 9 years ago

For reference:

https://github.com/marshmallow-code/marshmallow-sqlalchemy/blob/dev/marshmallow_sqlalchemy/convert.py#L153-L155

sssilver commented 9 years ago

Apparently passing in an ID value for the field itself works alright:

{"course":999,"title":"uu","price":"222"}

I suppose this should be somehow mentioned in the documentation.

uralbash commented 9 years ago

I have a similar example with m2m relation. My task is to create a new record like {"group_id": 2, "user_id": 1} but it's not work. Can I do this without creating relationships in Group2User.


from sqlalchemy import Column, String, Integer, ForeignKey, create_engine
from sqlalchemy.orm import relationship, sessionmaker, scoped_session
from sqlalchemy.ext.declarative import declarative_base

Base = declarative_base()
DBSession = scoped_session(sessionmaker())

class Group2User(Base):
    __tablename__ = 'group2user'
    user_id = Column(Integer, ForeignKey('user.id'), primary_key=True)
    group_id = Column(Integer, ForeignKey('groups.id'), primary_key=True)

class User(Base):
    __tablename__ = 'user'
    id = Column(Integer, primary_key=True)
    name = Column(String(30))
    groups = relationship("Group", secondary="group2user", backref="users")

    def __repr__(self):
        return self.name

class Group(Base):
    __tablename__ = 'groups'
    id = Column(Integer, primary_key=True)
    name = Column(String(30))

    def __repr__(self):
        return self.name

engine = create_engine('sqlite:///:memory:')

for t in [User, Group, Group2User]:
    t.metadata.create_all(engine)

from sqlalchemy import orm
session = orm.scoped_session(orm.sessionmaker())
session.configure(bind=engine)

users = [User(id=i + 1, name=i + 1) for i in range(20)]
session.add_all(users)
groups = [Group(id=i + 1, name=i + 1) for i in range(20)]
session.add_all(groups)
session.commit()

print(session.query(User).get(1).id)   # 1
print(session.query(Group).get(2).id)  # 2

from marshmallow_sqlalchemy import ModelSchema

class Schema(ModelSchema):
    class Meta:
        model = Group2User

schema = Schema()
schema.session = session
data = {
    "user_id": 1,
    "group_id": 2
}
instance = schema.load(data).data

# session.add(instance)
# session.commit() 
# it raises sqlalchemy.exc.IntegrityError: (sqlite3.IntegrityError)
# NOT NULL constraint failed: group2user.user_id
# [SQL: u'INSERT INTO group2user DEFAULT VALUES']

print(instance.user_id)   # None but why???
print(instance.group_id)  # None but why???

class Schema(ModelSchema):
    class Meta:
        model = User

schema = Schema()
schema.session = session
data = {
    "id": 100500,
    "name": "foo"
}
instance = schema.load(data).data
print(instance.id)    # 100500
print(instance.name)  # foo
YKdvd commented 7 years ago

It looks like this issue has been resolved at some point? Running dpwrussell's test code above with the current version 0.12 now seems to produce the "correct" output with the "999" ids, not the problematic behaviour. Closing out this bug or indicating what problems might remain would be nice - this use case is something I was about to use mm-sqla for, and seeing this was a bit scary!