Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hybrid properties with custom setter not supported #345

Open
digitalkaoz opened this issue Sep 20, 2020 · 2 comments
Open

hybrid properties with custom setter not supported #345

digitalkaoz opened this issue Sep 20, 2020 · 2 comments

Comments

@digitalkaoz
Copy link

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
Copy link

watchsteve commented Dec 17, 2020

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
Copy link
Contributor

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")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants