pynamodb / PynamoDB

A pythonic interface to Amazon's DynamoDB
http://pynamodb.readthedocs.io
MIT License
2.45k stars 427 forks source link

Polymorphic attribute support for from_json #1005

Closed lyang closed 2 years ago

lyang commented 2 years ago

For the recently added from_json function from #857 , it seems polymorphic attributes aren't properly deserialized. For e.g.:

def test_from_json_polymorphic(self):
    class MyPet(MapAttribute):
        cls = DiscriminatorAttribute()
        name = UnicodeAttribute()

    class MyCat(MyPet, discriminator="cat"):
        pass

    class MyModel(Model):
        pet = MyPet()

    json_string = (
        '{'
            '"pet": {"cls": "cat", "name": "Garfield"}'
        '}')

    my_model = MyModel().from_json(json_string)
    assert isinstance(my_model.pet, MyCat)
    assert my_model.pet.name == "Garfield"
>       assert isinstance(my_model.pet, MyCat)
E       AttributeError: 'NoneType' object has no attribute 'pet'

FYI: @jpinner-lyft

lyang commented 2 years ago

Not knowing #857 was already in PR, I had a different implementation that tries to do similar things, i.e. make the model objects serializable to json. And it supports polymorphic models and attributes.

Here's my implementation: https://github.com/lyang/pynamodb-encoder , if you are interested in it.

I chose to only convert model object to/from dict instead of json str, in the hope to make the usage more flexible. Converting str to/from dict is very straight forward, but dict is much easier to manipulate.

For e.g. sensitive attributes can be easily removed before serializing to json, without changing the model object itself.

Another e.g. is json API request usually contains other fields not tied to model object, and those request are most likely already deserialized to dict for pre-processing / format validation etc. So, it would be simpler to just convert the already deserialized dict to model object.

jpinner-lyft commented 2 years ago

@lyang You need to call from_json on a separate line. The return value of from_json is None. (This matches the signature of deserialize.)

>>> from pynamodb.attributes import DiscriminatorAttribute, MapAttribute, UnicodeAttribute
>>> from pynamodb.models import Model
>>> class MyPet(MapAttribute):
...     cls = DiscriminatorAttribute()
...     name = UnicodeAttribute()
... 
>>> class MyCat(MyPet, discriminator="cat"):
...     pass
... 
>>> class MyModel(Model):
...     pet = MyPet()
... 
>>> mm1 = MyModel()
>>> mm1.pet = MyCat()
>>> mm1.pet.name = 'Garfield'
>>> 
>>> mm2 = MyModel()
>>> mm2.from_json(mm1.to_json())
>>> mm2.pet
<__main__.MyCat object at 0x10b412160>
lyang commented 2 years ago

Ah, that solves the problem for polymorphic attributes. Thanks @jpinner-lyft !

However, it still doesn't have great solution for polymorphic model deserialization, unless I missed something obvious?

One would have to inspect the json string first to know the discriminator value, then find the right class for the deserialization.

jpinner-lyft commented 2 years ago

@lyang You are correct that from_json is not a great solution for model instantiation. It purely address serialization/deserialization to a known instance type. It does not handle any of the instantiation logic used to read from dynamodb: https://github.com/pynamodb/PynamoDB/blob/4929aee/pynamodb/attributes.py#L421-L429

A possible option would be to introduce a class method similar to from_raw_data which took the json string and returned a model instance that handled the polymorphism.

lyang commented 2 years ago

@jpinner-lyft Thanks!