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

Introduce DynamicMapAttribute #868

Merged
merged 10 commits into from
Jun 25, 2021
Merged

Introduce DynamicMapAttribute #868

merged 10 commits into from
Jun 25, 2021

Conversation

jpinner-lyft
Copy link
Contributor

DynamicMapAttribute is a map attribute that supports declaring attributes (like an AttributeContainer),
but will also store any other values that are set on it (like a raw MapAttribute).

This class lets you create a "partially typed" map attribute so that it can store items that are not just primitive python types.

>>> from datetime import datetime
>>> from pynamodb.attributes import DynamicMapAttribute
>>> from pynamodb.attributes import UTCDateTimeAttribute
>>> from pynamodb.models import Model
>>> 
>>> class CreatedAtMap(DynamicMapAttribute):
...     created_at = UTCDateTimeAttribute()
... 
>>> class MyModel(Model):
...     my_map = CreatedAtMap(default={})
... 
>>> my_model = MyModel()
>>> my_model.my_map.created_at = datetime.utcnow()
>>> my_model.my_map.foo = 'bar'
>>> my_model.serialize()
{'my_map': {'M': {'created_at': {'S': '2020-10-14T16:39:15.077254+0000'}, 'foo': {'S': 'bar'}}}}
>>> 
>>> my_model_2 = MyModel()
>>> my_model_2.deserialize({'my_map': {'M': {'created_at': {'S': '2020-10-14T16:39:15.077254+0000'}, 'foo': {'S': 'bar'}}}})
>>> my_model_2.my_map.created_at
datetime.datetime(2020, 10, 14, 16, 39, 15, 77254, tzinfo=datetime.timezone.utc)
>>> my_model_2.my_map.foo
'bar'

object.__setattr__(self, name, value) if name in self.get_attributes() else super().__setattr__(name, value)

def serialize(self, values):
if not isinstance(values, type(self)):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this blob transforms a raw dict into a map attr object right? is this not shared with raw map attrs?

instance.attribute_values = {} # clear any defaults
instance._set_attributes(**values)
values = instance
rval = AttributeContainer.serialize(values)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this serializes every defined attribute

instance._set_attributes(**values)
values = instance
rval = AttributeContainer.serialize(values)
for attr_name in values:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this loop serializes the left over, undefined attrs

values = instance
rval = AttributeContainer.serialize(values)
for attr_name in values:
if attr_name not in self.get_attributes():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we dedup this with MapAttribute.serialize?

@garrettheel
Copy link
Member

We might also want to add a mention here: https://pynamodb.readthedocs.io/en/latest/attributes.html#map-attributes

@jmphilli
Copy link
Contributor

add discriminator + dynamicmapattr test


car = CarInfo(make='Make-A', model='Model-A', year=1975)
other_car = CarInfo(make='Make-A', model='Model-A', year=1975, seats=3)

`As with a model and its top-level attributes <https://github.com/pynamodb/PynamoDB/blob/master/docs/quickstart.rst#changing-items>`_, a PynamoDB MapAttribute will ignore sub-attributes it does not know about during deserialization. As a result, if the item in DynamoDB contains sub-attributes not declared as properties of the corresponding MapAttribute, save() will cause those sub-attributes to be deleted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This applies only to MapAttribute subclasses, right? Might want to reorder the DynamicMapAttribute content underneath so as not to confuse

class TestDynamicMapAttribute:

class CreatedAtTestModel(Model):
class CreatedAtMap(DynamicMapAttribute):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also test with a subclass of CreatedAtMap to test one more level of inheritance?

# Continue to serialize NULL values in "raw" map attributes for backwards compatibility.
# This special case behavior for "raw" attributes should be removed in the future.
for attr_name in values:
if attr_name not in self.get_attributes():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is a new branch that should always return True for MapAttribute and sometimes return True for DynamicMapAttribute. Calling it out because there's some perf impact, though I think it should be marginal

@jmphilli jmphilli merged commit 57a1ccb into master Jun 25, 2021
@garrettheel garrettheel deleted the dynamic-map-attribute branch June 25, 2021 22:00
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

Successfully merging this pull request may close these issues.

None yet

3 participants