marshmallow-code / marshmallow-sqlalchemy

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

hybrid properties with custom setter not supported #345

Open digitalkaoz opened 3 years ago

digitalkaoz commented 3 years ago

Hey, it seems custom hybrid property setters arent called when deserializing data:

from marshmallow_sqlalchemy import SQLAlchemyAutoSchema
from sqlalchemy import Column, String, Integer
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.hybrid import hybrid_property

base = declarative_base()

#define our Model
class User(base):
    __tablename__ = "users"

    id = Column(Integer, autoincrement=True, primary_key=True)
    username = Column(String(100), nullable=False)
    _password = Column(String(100), nullable=False)

    @hybrid_property
    def password(self):
        return self._password

    @password.setter
    def password(self, plaintext):
        self._password = hash(plaintext)

# create the AutoSchema
class UserSchema(SQLAlchemyAutoSchema):
    class Meta(object):
        model = User
        load_instance = True
        transient = True
        ordered = True

# test serialize
u = User(id=1337, username="admin", password="admin")

assert u.id == 1337
assert u.username == "admin"
assert u.password != "admin" # its not admin, bc. the custom setter was called and the password was hashed

assert UserSchema(exclude=["_password"]).dump(u) == {
    "id": 1337,
    "username": "admin",
}

# test deserialize
u2: User = UserSchema().load({
    "id": 1337,
    "password": "admin", # gives exception bc the _password field isnt set
    "username": "admin",
})

assert u2.password != "admin"

the problem is here:

u2: User = UserSchema().load({
    "id": 1337,
    "password": "admin", #with password the deserialization works but we set the underlying prop
    "username": "admin",
})

what did i do wrong? Or is my understanding wrong? i would except the following:

u2: User = UserSchema().load({
    "id": 1337,
    "_password": "someSaltyHa5h"
    "username": "admin",
})

this should set the underlying prop directly (bypassing the hybrid property+setter, and this should work too:

u2: User = UserSchema().load({
    "id": 1337,
    "password": "plainTextPassword"
    "username": "admin",
})

this should call the the custom setter for password and write hashed value to the underlying _password property.

any ideas on how to achieve this?

watchsteve commented 3 years ago

I ran into the same issue. I solved this by using a pre load hook to do the logic on the schema instead of the ORM. Not ideal ofc but it works. I just copied the custom setter code over since it's only one line for me, but you could probably call the method specifically to keep it DRY. If you had a lot of these hybrid properties/custom setters you could probably create a BaseSchema class with that inspects the ORM class and dynamically generates these pre load or pre dump hooks.

class TokenSchema(ma.SQLAlchemyAutoSchema):
  class Meta:
    model = Token
    include_fk = True
    include_relationships = True
    load_instance = True

 @marsh.pre_load
 def encrypt_token(self, item, many, **kwargs):
    if item.get('access_token'):
      item['_access_token'] = fernet.encrypt(str.encode(item['access_token']))
      item.pop('access_token')
    return item
peterschutt commented 3 years ago

By explicitly setting the data_key and attribute args to the _password I was able to get all of your tests to pass:

from marshmallow_sqlalchemy import SQLAlchemyAutoSchema, auto_field
from sqlalchemy import Column, String, Integer
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.ext.hybrid import hybrid_property

base = declarative_base()

#define our Model
class User(base):
    __tablename__ = "users"

    id = Column(Integer, autoincrement=True, primary_key=True)
    username = Column(String(100), nullable=False)
    _password = Column(String(100), nullable=False)

    @hybrid_property
    def password(self):
        return self._password

    @password.setter
    def password(self, plaintext):
        self._password = hash(plaintext)

# create the AutoSchema
class UserSchema(SQLAlchemyAutoSchema):
    class Meta(object):
        model = User
        load_instance = True
        transient = True
        ordered = True

    _password = auto_field(data_key="password", attribute="password")

# test serialize
u = User(id=1337, username="admin", password="admin")

assert u.id == 1337
assert u.username == "admin"
assert u.password != "admin" # its not admin, bc. the custom setter was called and the password was hashed

assert UserSchema(exclude=["_password"]).dump(u) == {
    "id": 1337,
    "username": "admin",
}

# test deserialize
u2: User = UserSchema().load({
    "id": 1337,
    "password": "admin", # gives exception bc the _password field isnt set
    "username": "admin",
})

assert u2.password != "admin"

Here's the diff:

diff --git a/orig.py b/main.py
index 7d9986e..babfa6b 100644
--- a/orig.py
+++ b/main.py
@@ -1,4 +1,4 @@
-from marshmallow_sqlalchemy import SQLAlchemyAutoSchema
+from marshmallow_sqlalchemy import SQLAlchemyAutoSchema, auto_field
 from sqlalchemy import Column, String, Integer
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.hybrid import hybrid_property
@@ -30,6 +30,8 @@ class UserSchema(SQLAlchemyAutoSchema):
         transient = True
         ordered = True

+    _password = auto_field(data_key="password", attribute="password")
+

 # test serialize
 u = User(id=1337, username="admin", password="admin")