From 3d51e846059e20f53784b1b7fe679dcc65adcd29 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 09:46:47 -0800 Subject: [PATCH 01/11] 75 Associations - Interim Commit 1 - HasOne Draft Implementation --- src/protean/core/entity.py | 5 +- src/protean/core/exceptions.py | 4 +- src/protean/core/field/association.py | 64 ++++++++++++++++++++++++-- src/protean/core/repository/factory.py | 13 +++++- tests/conftest.py | 16 ++++++- tests/core/test_entity.py | 19 ++++++-- tests/support/dog.py | 50 +++++++++++++++++++- tests/support/human.py | 35 ++++++++++++++ 8 files changed, 191 insertions(+), 15 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 5af8640d..7d9a2601 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -6,6 +6,7 @@ from protean.core.exceptions import ValidationError from protean.core.field import Auto from protean.core.field import Field, Reference +from protean.core.field.association import HasOne from protean.utils.generic import classproperty from protean.utils.query import Q @@ -81,7 +82,7 @@ def _load_fields(new_class, attrs): is set up in this method, while `parent_id` is set up in `_set_up_reference_fields()`. """ for attr_name, attr_obj in attrs.items(): - if isinstance(attr_obj, (Field, Reference)): + if isinstance(attr_obj, (Field, Reference)) and not isinstance(attr_obj, HasOne): setattr(new_class, attr_name, attr_obj) new_class._meta.declared_fields[attr_name] = attr_obj @@ -458,7 +459,7 @@ def __init__(self, *template, **kwargs): # Now load the remaining fields with a None value, which will fail # for required fields for field_name, field_obj in self._meta.declared_fields.items(): - if field_name not in loaded_fields: + if field_name not in loaded_fields and not isinstance(field_obj, HasOne): # Check that the field is not set already, which would happen if we are # dealing with reference fields if getattr(self, field_name, None) is None: diff --git a/src/protean/core/exceptions.py b/src/protean/core/exceptions.py index 0f5ccd95..7de24031 100644 --- a/src/protean/core/exceptions.py +++ b/src/protean/core/exceptions.py @@ -11,8 +11,8 @@ class ObjectNotFoundError(Exception): """Object was not found, can raise 404""" -class ValueError(Exception): - """Object of incorrect type, or with invalid state was assigned""" +class NotSupportedError(Exception): + """Object does not support the operation being performed""" class ValidationError(Exception): diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index f0d5d00e..d11d3f84 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -2,6 +2,7 @@ from .mixins import FieldCacheMixin from protean.core import exceptions +from protean.utils import inflection class ReferenceField(Field): @@ -51,10 +52,10 @@ def _reset_values(self, instance): class Reference(FieldCacheMixin, Field): """ - Provide a many-to-one relation by adding a column to the local entity + Provide a many-to-one relation by adding an attribute to the local entity to hold the remote value. - By default ForeignKey will target the pk of the remote model but this + By default ForeignKey will target the `id` column of the remote model but this behavior can be changed by using the ``via`` argument. """ @@ -84,7 +85,7 @@ def __set__(self, instance, value): if value: # Check if the reference object has been saved. Otherwise, throw ValueError if value.id is None: # FIXME not a comprehensive check. Should refer to state - raise exceptions.ValueError( + raise ValueError( "Target Object must be saved before being referenced", self.field_name) else: @@ -111,3 +112,60 @@ def _cast_to_type(self, value): def get_cache_name(self): return self.name + + +class HasOne(FieldCacheMixin, Field): + """ + Provide a HasOne relation to a remote entity. + + By default, the query will lookup an attribute of the form `_id` + to fetch and populate. This behavior can be changed by using the `via` argument. + """ + + def __init__(self, to_cls, via=None, **kwargs): + super().__init__(**kwargs) + self.to_cls = to_cls + self.via = via + + def __set__(self, instance, value): + """Cannot set values through the HasOne association""" + raise exceptions.NotSupportedError( + "Object does not support the operation being performed", + self.field_name + ) + + def _cast_to_type(self, value): + """Verify type of value assigned to the association field""" + # FIXME Verify that the value being assigned is compatible with the associated Entity + return value + + def _linked_attribute(self, owner): + """Choose the Linkage attribute between `via` and own `id_field`""" + return self.via or (inflection.underscore(owner.__name__) + '_id') + + def _fetch_to_cls_from_registry(self, entity): + if isinstance(entity, str): + from protean.core.repository import repo_factory # FIXME Move to a better placement + + try: + return repo_factory.get_entity(self.to_cls) + except AssertionError: + # Entity has not been registered (yet) + # FIXME print a helpful debug message + raise + else: + return self.to_cls + + def __get__(self, instance, owner): + """Retrieve associated objects""" + if isinstance(self.to_cls, str): + self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) + + # Fetch target object by own Identifier + id_value = getattr(instance, instance.id_field.field_name) + reference_obj = self.to_cls.find_by(**{self._linked_attribute(owner): id_value}) + if reference_obj: + self.value = reference_obj + else: + # No Objects were found in the remote entity with this Entity's ID + pass diff --git a/src/protean/core/repository/factory.py b/src/protean/core/repository/factory.py index b4883c3f..8f3c5f67 100644 --- a/src/protean/core/repository/factory.py +++ b/src/protean/core/repository/factory.py @@ -23,6 +23,7 @@ class RepositoryFactory: def __init__(self): """"Initialize repository factory""" self._registry = {} + self._entity_registry = {} self._model_registry = {} self._connections = local() self._repositories = None @@ -94,6 +95,7 @@ def register(self, model_cls, repo_cls=None): self._registry[entity_name] = \ repo_cls(self.connections[model_cls.opts_.bind], model_cls) self._model_registry[entity_name] = model_cls + self._entity_registry[entity_name] = model_cls.Meta.entity logger.debug( f'Registered model {model_name} for entity {entity_name} with repository provider ' f'{conn_info["PROVIDER"]}.') @@ -103,13 +105,20 @@ def get_model(self, entity_name): try: return self._model_registry[entity_name] except KeyError: - raise AssertionError('No Model registered for {entity_name}') + raise AssertionError('No Model registered for {entity_name}'.format(entity_name)) + + def get_entity(self, entity_name): + """Retrieve Entity class registered by `entity_name`""" + try: + return self._entity_registry[entity_name] + except KeyError: + raise AssertionError('No Entity registered with name {entity_name}'.format(entity_name)) def __getattr__(self, entity_name): try: return self._registry[entity_name] except KeyError: - raise AssertionError('No Model registered for {entity_name}') + raise AssertionError('No Model registered for {entity_name}'.format(entity_name)) def close_connections(self): """ Close all connections registered with the repository """ diff --git a/tests/conftest.py b/tests/conftest.py index 14fbb80e..6ba78bb4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,13 +10,19 @@ def run_around_tests(): """Initialize DogModel with Dict Repo""" from protean.core.repository import repo_factory - from tests.support.dog import DogModel, RelatedDogModel, DogRelatedByEmailModel - from tests.support.human import HumanModel + from tests.support.dog import (DogModel, RelatedDogModel, DogRelatedByEmailModel, + HasOneDog1Model, HasOneDog2Model, HasOneDog3Model) + from tests.support.human import HumanModel, HasOneHuman1Model, HasOneHuman2Model repo_factory.register(DogModel) repo_factory.register(RelatedDogModel) repo_factory.register(DogRelatedByEmailModel) + repo_factory.register(HasOneDog1Model) + repo_factory.register(HasOneDog2Model) + repo_factory.register(HasOneDog3Model) repo_factory.register(HumanModel) + repo_factory.register(HasOneHuman1Model) + repo_factory.register(HasOneHuman2Model) # A test function will be run at this point yield @@ -24,4 +30,10 @@ def run_around_tests(): repo_factory.Dog.delete_all() repo_factory.RelatedDog.delete_all() repo_factory.DogRelatedByEmail.delete_all() + repo_factory.HasOneDog1.delete_all() + repo_factory.HasOneDog2.delete_all() + repo_factory.HasOneDog3.delete_all() repo_factory.Human.delete_all() + repo_factory.HasOneHuman1.delete_all() + repo_factory.HasOneHuman2.delete_all() + diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index e29db319..87075e51 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -1,13 +1,14 @@ """Tests for Entity Functionality and Base Classes""" import pytest -from tests.support.dog import Dog, RelatedDog, DogRelatedByEmail -from tests.support.human import Human +from tests.support.dog import (Dog, RelatedDog, DogRelatedByEmail, + HasOneDog1, HasOneDog2, HasOneDog3) +from tests.support.human import Human, HasOneHuman1, HasOneHuman2 from protean.core import field from protean.core.entity import Entity, QuerySet from protean.core.exceptions import ObjectNotFoundError -from protean.core.exceptions import ValidationError, ValueError +from protean.core.exceptions import ValidationError from protean.utils.query import Q @@ -1327,3 +1328,15 @@ def test_via_with_shadow_attribute_assign(self): dog.save() assert hasattr(dog, 'owner_email') assert dog.owner_email == human.email + + class TestHasOne: + """Class to test HasOne Association""" + + def test_init(self): + """Test successful HasOne initialization""" + human = HasOneHuman1.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = HasOneDog1.create(id=1, name='John Doe', age=10, has_one_human1=human) + assert dog.has_one_human1 == human + assert human.dog == dog diff --git a/tests/support/dog.py b/tests/support/dog.py index d5c9bc0a..4243a885 100644 --- a/tests/support/dog.py +++ b/tests/support/dog.py @@ -1,6 +1,6 @@ """Support Classes for Test Cases""" -from tests.support.human import Human +from tests.support.human import Human, HasOneHuman1, HasOneHuman2 from protean.core import field from protean.core.entity import Entity @@ -53,3 +53,51 @@ class Meta: """ Meta class for model options""" entity = DogRelatedByEmail model_name = 'related_dogs_by_email' + + +class HasOneDog1(Entity): + """This is a dummy Dog Entity class to test HasOne Association""" + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + has_one_human1 = field.Reference(HasOneHuman1) + + +class HasOneDog1Model(DictModel): + """ Model for the HasOneDog1 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasOneDog1 + model_name = 'has_one_dogs1' + + +class HasOneDog2(Entity): + """This is a dummy Dog Entity class to test HasOne Association""" + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + human = field.Reference(HasOneHuman2) + + +class HasOneDog2Model(DictModel): + """ Model for the HasOneDog2 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasOneDog2 + model_name = 'has_one_dogs2' + + +class HasOneDog3(Entity): + """This is a dummy Dog Entity class to test HasOne Association""" + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + human_id = field.Integer() + + +class HasOneDog3Model(DictModel): + """ Model for the HasOneDog3 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasOneDog3 + model_name = 'has_one_dogs3' diff --git a/tests/support/human.py b/tests/support/human.py index 463d114c..2dd856b2 100644 --- a/tests/support/human.py +++ b/tests/support/human.py @@ -1,6 +1,7 @@ """Human Support Class for Test Cases""" from protean.core import field +from protean.core.field import association from protean.core.entity import Entity from protean.impl.repository.dict_repo import DictModel @@ -19,3 +20,37 @@ class Meta: """ Meta class for model options""" entity = Human model_name = 'humans' + + +class HasOneHuman1(Entity): + """This is a dummy Human Entity class to test HasOne association""" + first_name = field.String(required=True, unique=True, max_length=50) + last_name = field.String(required=True, unique=True, max_length=50) + email = field.String(required=True, unique=True, max_length=50) + dog = association.HasOne('HasOneDog1') + + +class HasOneHuman1Model(DictModel): + """ Model for the HasOneHuman1 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasOneHuman1 + model_name = 'has_one_humans1' + + +class HasOneHuman2(Entity): + """This is a dummy Human Entity class to test HasOne association""" + first_name = field.String(required=True, unique=True, max_length=50) + last_name = field.String(required=True, unique=True, max_length=50) + email = field.String(required=True, unique=True, max_length=50) + dog = association.HasOne('HasOneDog2', via='human_id') + + +class HasOneHuman2Model(DictModel): + """ Model for the HasOneHuman2 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasOneHuman2 + model_name = 'has_one_humans1' \ No newline at end of file From e843e730931d7a3aea4c0979239ae6c1e6d87987 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 10:32:26 -0800 Subject: [PATCH 02/11] 75 Associations - Interim Commit 2 - HasOne Working Implementation --- src/protean/core/entity.py | 14 +++++++++ src/protean/core/field/association.py | 3 +- src/protean/impl/repository/dict_repo.py | 2 +- tests/core/test_entity.py | 40 ++++++++++++++++++++++-- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 7d9a2601..9c110206 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -54,6 +54,9 @@ def __new__(mcs, name, bases, attrs, **kwargs): # Lookup an already defined ID field or create an `Auto` field new_class._set_id_field() + # Load list of Attributes from declared fields, depending on type of fields + new_class._load_attributes() + # Construct an empty QuerySet associated with this Entity class new_class.query = QuerySet(name) @@ -119,6 +122,11 @@ def _create_id_field(new_class): new_class._meta.declared_fields['id'] = id_field new_class._meta.id_field = id_field + def _load_attributes(new_class): + """Load list of attributes from declared fields""" + for field_name, field_obj in new_class._meta.declared_fields.items(): + new_class._meta.attributes[field_obj.get_attribute_name()] = field_obj + class EntityMeta: """ Metadata information for the entity including any options defined.""" @@ -129,6 +137,7 @@ def __init__(self, meta): # Initialize Options self.entity_cls = None self.declared_fields = {} + self.attributes = {} self.id_field = None @property @@ -523,6 +532,11 @@ def declared_fields(cls): """Pass through method to retrieve declared fields defined for entity""" return cls._meta.declared_fields + @classproperty + def attributes(cls): + """Pass through method to retrieve attributes defined for entity""" + return cls._meta.attributes + @classproperty def auto_fields(cls): """Pass through method to retrieve `Auto` fields defined for entity""" diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index d11d3f84..eb552f99 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -166,6 +166,7 @@ def __get__(self, instance, owner): reference_obj = self.to_cls.find_by(**{self._linked_attribute(owner): id_value}) if reference_obj: self.value = reference_obj + return reference_obj else: # No Objects were found in the remote entity with this Entity's ID - pass + return None diff --git a/src/protean/impl/repository/dict_repo.py b/src/protean/impl/repository/dict_repo.py index 66fe818b..c69c4b30 100644 --- a/src/protean/impl/repository/dict_repo.py +++ b/src/protean/impl/repository/dict_repo.py @@ -298,7 +298,7 @@ class DictModel(BaseModel): def from_entity(cls, entity): """ Convert the entity to a dictionary record """ dict_obj = {} - for field_name in entity.__class__.declared_fields: + for field_name in entity.__class__.attributes: dict_obj[field_name] = getattr(entity, field_name) return dict_obj diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index 87075e51..b5c031f3 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -1,5 +1,7 @@ """Tests for Entity Functionality and Base Classes""" +from collections import OrderedDict + import pytest from tests.support.dog import (Dog, RelatedDog, DogRelatedByEmail, HasOneDog1, HasOneDog2, HasOneDog3) @@ -498,6 +500,40 @@ def test_filter_returns_q_object(self): assert isinstance(query, QuerySet) +class TestEntityMetaAttributes: + """Class that holds testcases for Entity's meta attributes""" + + def test_meta_on_init(self): + """Test that `meta` attribute is available after initialization""" + dog = Dog(id=1, name='John Doe', age=10, owner='Jimmy') + assert hasattr(dog, '_meta') + + def test_declared_fields_normal(self): + """Test declared fields on an entity without references""" + dog = Dog(id=1, name='John Doe', age=10, owner='Jimmy') + + attribute_keys = list(OrderedDict(sorted(dog.attributes.items())).keys()) + assert attribute_keys == ['age', 'id', 'name', 'owner'] + + def test_declared_fields_with_reference(self): + """Test declared fields on an entity with references""" + human = Human.create(first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = RelatedDog(id=1, name='John Doe', age=10, owner=human) + + attribute_keys = list(OrderedDict(sorted(dog.attributes.items())).keys()) + assert attribute_keys == ['age', 'id', 'name', 'owner_id'] + + def test_declared_fields_with_hasone_association(self): + """Test declared fields on an entity with a HasOne association""" + human = HasOneHuman1.create(first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = HasOneDog1.create(id=1, name='John Doe', age=10, has_one_human1=human) + + assert all(key in dog.attributes for key in ['age', 'has_one_human1_id', 'id', 'name']) + assert all(key in human.attributes for key in ['first_name', 'id', 'last_name', 'email']) + + class TestQuerySet: """Class that holds Tests for QuerySet""" @@ -1337,6 +1373,6 @@ def test_init(self): human = HasOneHuman1.create( first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') - dog = HasOneDog1.create(id=1, name='John Doe', age=10, has_one_human1=human) + dog = HasOneDog1.create(id=101, name='John Doe', age=10, has_one_human1=human) assert dog.has_one_human1 == human - assert human.dog == dog + assert human.dog.id == dog.id From e18a7dbf2a4dc5dc5b16b819138aa8628168d5e5 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 11:14:54 -0800 Subject: [PATCH 03/11] 75 Associations - Interim Commit 3 - HasOne Additional test cases --- src/protean/impl/repository/dict_repo.py | 1 - tests/conftest.py | 6 ++++-- tests/core/test_entity.py | 21 +++++++++++++++++++- tests/support/human.py | 25 ++++++++++++++++++++++-- 4 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/protean/impl/repository/dict_repo.py b/src/protean/impl/repository/dict_repo.py index c69c4b30..922717e3 100644 --- a/src/protean/impl/repository/dict_repo.py +++ b/src/protean/impl/repository/dict_repo.py @@ -7,7 +7,6 @@ from protean.core.entity import Q from protean.core.exceptions import ObjectNotFoundError -from protean.core.field import Auto from protean.core.repository import BaseAdapter from protean.core.repository import BaseConnectionHandler from protean.core.repository import BaseModel diff --git a/tests/conftest.py b/tests/conftest.py index 6ba78bb4..24513288 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -12,7 +12,8 @@ def run_around_tests(): from protean.core.repository import repo_factory from tests.support.dog import (DogModel, RelatedDogModel, DogRelatedByEmailModel, HasOneDog1Model, HasOneDog2Model, HasOneDog3Model) - from tests.support.human import HumanModel, HasOneHuman1Model, HasOneHuman2Model + from tests.support.human import (HumanModel, HasOneHuman1Model, + HasOneHuman2Model, HasOneHuman3Model) repo_factory.register(DogModel) repo_factory.register(RelatedDogModel) @@ -23,6 +24,7 @@ def run_around_tests(): repo_factory.register(HumanModel) repo_factory.register(HasOneHuman1Model) repo_factory.register(HasOneHuman2Model) + repo_factory.register(HasOneHuman3Model) # A test function will be run at this point yield @@ -36,4 +38,4 @@ def run_around_tests(): repo_factory.Human.delete_all() repo_factory.HasOneHuman1.delete_all() repo_factory.HasOneHuman2.delete_all() - + repo_factory.HasOneHuman3.delete_all() diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index b5c031f3..d9a22062 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -5,7 +5,7 @@ import pytest from tests.support.dog import (Dog, RelatedDog, DogRelatedByEmail, HasOneDog1, HasOneDog2, HasOneDog3) -from tests.support.human import Human, HasOneHuman1, HasOneHuman2 +from tests.support.human import Human, HasOneHuman1, HasOneHuman2, HasOneHuman3 from protean.core import field from protean.core.entity import Entity, QuerySet @@ -1376,3 +1376,22 @@ def test_init(self): dog = HasOneDog1.create(id=101, name='John Doe', age=10, has_one_human1=human) assert dog.has_one_human1 == human assert human.dog.id == dog.id + + def test_init_with_via(self): + """Test successful HasOne initialization with a class containing via""" + human = HasOneHuman2.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = HasOneDog2.create(id=101, name='John Doe', age=10, human=human) + assert dog.human == human + assert human.dog.id == dog.id + + def test_init_with_no_reference(self): + """Test successful HasOne initialization with a class containing via""" + human = HasOneHuman3.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = HasOneDog3.create(id=101, name='John Doe', age=10, human_id=human.id) + assert dog.human_id == human.id + assert not hasattr(dog, 'human') + assert human.dog.id == dog.id diff --git a/tests/support/human.py b/tests/support/human.py index 2dd856b2..52f3e597 100644 --- a/tests/support/human.py +++ b/tests/support/human.py @@ -40,7 +40,9 @@ class Meta: class HasOneHuman2(Entity): - """This is a dummy Human Entity class to test HasOne association""" + """This is a dummy Human Entity class to test HasOne association + with a custom attribute defined in `via` argument to field + """ first_name = field.String(required=True, unique=True, max_length=50) last_name = field.String(required=True, unique=True, max_length=50) email = field.String(required=True, unique=True, max_length=50) @@ -53,4 +55,23 @@ class HasOneHuman2Model(DictModel): class Meta: """ Meta class for model options""" entity = HasOneHuman2 - model_name = 'has_one_humans1' \ No newline at end of file + model_name = 'has_one_humans2' + + +class HasOneHuman3(Entity): + """This is a dummy Human Entity class to test HasOne association + when there is no corresponding Reference defined in the target class + """ + first_name = field.String(required=True, unique=True, max_length=50) + last_name = field.String(required=True, unique=True, max_length=50) + email = field.String(required=True, unique=True, max_length=50) + dog = association.HasOne('HasOneDog3', via='human_id') + + +class HasOneHuman3Model(DictModel): + """ Model for the HasOneHuman3 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasOneHuman3 + model_name = 'has_one_humans3' From f08e2a4eb140873667019597970ec1de39ddc16e Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 15:32:39 -0800 Subject: [PATCH 04/11] 75 Associations - Interim Commit 4 - Support Reference initialization with Entity Name --- src/protean/core/entity.py | 6 +-- src/protean/core/field/association.py | 56 +++++++++++++++++++++++++-- src/protean/core/field/base.py | 6 +++ tests/core/test_entity.py | 9 ++++- tests/support/dog.py | 18 +++++++++ 5 files changed, 87 insertions(+), 8 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 9c110206..8326e3dd 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -48,12 +48,12 @@ def __new__(mcs, name, bases, attrs, **kwargs): # Load declared fields from Base class, in case this Entity is subclassing another new_class._load_base_class_fields(bases, attrs) - # Set up Relation Fields - new_class._set_up_reference_fields() - # Lookup an already defined ID field or create an `Auto` field new_class._set_id_field() + # Set up Relation Fields + new_class._set_up_reference_fields() + # Load list of Attributes from declared fields, depending on type of fields new_class._load_attributes() diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index eb552f99..423172a9 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -60,13 +60,11 @@ class Reference(FieldCacheMixin, Field): """ def __init__(self, to_cls, via=None, **kwargs): + # FIXME ensure `via` argument is of type `str` super().__init__(**kwargs) self.to_cls = to_cls self.via = via - # Choose the Linkage attribute between `via` and `id` - self.linked_attribute = self.via or 'id' - self.relation = ReferenceField(self) def get_attribute_name(self): @@ -78,8 +76,49 @@ def get_shadow_field(self): Primarily used during Entity initialization to register shadow field""" return (self.attribute_name, self.relation) + @property + def linked_attribute(self): + """Choose the Linkage attribute between `via` and designated `id_field` of the target class + + This method is initially called from `__set_name__()` -> `get_attribute_name()` + at which point, the `to_cls` has not been initialized properly. We simply default + the linked attribute to 'id' in that case. + + Eventually, when setting value the first time, the `to_cls` entity is initialized + and the attribute name is reset correctly. + """ + if isinstance(self.to_cls, str): + return 'id' + else: + return self.via or self.to_cls.id_field.attribute_name + + def _fetch_to_cls_from_registry(self, entity): + """Private Method to fetch an Entity class from an entity's name + + FIXME Move this method into utils + """ + # Defensive check to ensure we only process if `to_cls` is a string + if isinstance(entity, str): + from protean.core.repository import repo_factory # FIXME Move to a better placement + + try: + return repo_factory.get_entity(self.to_cls) + except AssertionError: + # Entity has not been registered (yet) + # FIXME print a helpful debug message + raise + else: + return self.to_cls + def __set__(self, instance, value): """Override `__set__` to coordinate between relation field and its shadow attribute""" + if isinstance(self.to_cls, str): + self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) + + # Refresh attribute name, now that we know `to_cls` Entity and it has been + # initialized with `id_field` + self.attribute_name = self.get_attribute_name() + value = self._load(value) if value: @@ -140,10 +179,16 @@ def _cast_to_type(self, value): return value def _linked_attribute(self, owner): - """Choose the Linkage attribute between `via` and own `id_field`""" + """Choose the Linkage attribute between `via` and own entity's `id_field` + + FIXME Explore converting this method into an attribute, and treating it + uniformly at `association` level. + """ return self.via or (inflection.underscore(owner.__name__) + '_id') def _fetch_to_cls_from_registry(self, entity): + """Private Method to fetch an Entity class from an entity's name""" + # Defensive check to ensure we only process if `to_cls` is a string if isinstance(entity, str): from protean.core.repository import repo_factory # FIXME Move to a better placement @@ -158,6 +203,9 @@ def _fetch_to_cls_from_registry(self, entity): def __get__(self, instance, owner): """Retrieve associated objects""" + + # If `to_cls` was specified as a string, take this opportunity to fetch + # and update the correct entity class against it, if not already done if isinstance(self.to_cls, str): self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) diff --git a/src/protean/core/field/base.py b/src/protean/core/field/base.py index c91b8bef..aa35ad15 100644 --- a/src/protean/core/field/base.py +++ b/src/protean/core/field/base.py @@ -76,6 +76,9 @@ def __init__(self, identifier: bool = False, default: Any = None, # Value holder self._value = value + # Hold a reference to Entity registering the field + self._entity_cls = None + # These are set up when the owner (Entity class) adds the field to itself self.field_name = None self.attribute_name = None @@ -91,6 +94,9 @@ def __set_name__(self, entity_cls, name): self.field_name = name self.attribute_name = self.get_attribute_name() + # Record Entity setting up the field + self._entity_cls = entity_cls + # `self.label` should default to being based on the field name. if self.label is None: self.label = self.field_name.replace('_', ' ').capitalize() diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index d9a22062..cf111054 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -3,7 +3,7 @@ from collections import OrderedDict import pytest -from tests.support.dog import (Dog, RelatedDog, DogRelatedByEmail, +from tests.support.dog import (Dog, RelatedDog, RelatedDog2, DogRelatedByEmail, HasOneDog1, HasOneDog2, HasOneDog3) from tests.support.human import Human, HasOneHuman1, HasOneHuman2, HasOneHuman3 @@ -1235,6 +1235,13 @@ def test_init(self): dog = RelatedDog(id=1, name='John Doe', age=10, owner=human) assert dog.owner == human + def test_init_with_string_reference(self): + """Test successful RelatedDog initialization""" + human = Human.create(first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = RelatedDog2(id=1, name='John Doe', age=10, owner=human) + assert dog.owner == human + def test_save(self): """Test successful RelatedDog save""" human = Human.create(first_name='Jeff', last_name='Kennedy', diff --git a/tests/support/dog.py b/tests/support/dog.py index 4243a885..77ebf22e 100644 --- a/tests/support/dog.py +++ b/tests/support/dog.py @@ -39,6 +39,24 @@ class Meta: model_name = 'related_dogs' +class RelatedDog2(Entity): + """This is a dummy RelatedDog2 Entity class with reference definition + containing the target class name as string + """ + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + owner = field.Reference('Human') + + +class RelatedDog2Model(DictModel): + """ Model for the RelatedDog2 Entity""" + + class Meta: + """ Meta class for model options""" + entity = RelatedDog2 + model_name = 'related_dogs2' + + class DogRelatedByEmail(Entity): """This is a dummy Dog Entity class with an association""" name = field.String(required=True, unique=True, max_length=50) From c2ce5bfb72708a06056bcd01cbfb40a0ccc156ad Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 16:04:29 -0800 Subject: [PATCH 05/11] 75 Associations - Interim Commit 5 - Implement caching --- src/protean/core/field/association.py | 55 +++++++++++++++++---------- src/protean/core/field/base.py | 3 ++ tests/core/test_entity.py | 25 ++++++++++++ 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index 423172a9..e6adddc8 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -14,7 +14,9 @@ def __init__(self, reference, **kwargs): super().__init__(**kwargs) def __set__(self, instance, value): - """Override `__set__` to update relation field""" + """Override `__set__` to update relation field and keep it in sync with the shadow + attribute's value + """ value = self._load(value) if value: @@ -35,6 +37,7 @@ def __set__(self, instance, value): self._reset_values(instance) def __delete__(self, instance): + """Nullify values and linkages""" self._reset_values(instance) def _cast_to_type(self, value): @@ -149,9 +152,6 @@ def _cast_to_type(self, value): self.fail('invalid', value=value) return value - def get_cache_name(self): - return self.name - class HasOne(FieldCacheMixin, Field): """ @@ -166,13 +166,6 @@ def __init__(self, to_cls, via=None, **kwargs): self.to_cls = to_cls self.via = via - def __set__(self, instance, value): - """Cannot set values through the HasOne association""" - raise exceptions.NotSupportedError( - "Object does not support the operation being performed", - self.field_name - ) - def _cast_to_type(self, value): """Verify type of value assigned to the association field""" # FIXME Verify that the value being assigned is compatible with the associated Entity @@ -209,12 +202,34 @@ def __get__(self, instance, owner): if isinstance(self.to_cls, str): self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) - # Fetch target object by own Identifier - id_value = getattr(instance, instance.id_field.field_name) - reference_obj = self.to_cls.find_by(**{self._linked_attribute(owner): id_value}) - if reference_obj: - self.value = reference_obj - return reference_obj - else: - # No Objects were found in the remote entity with this Entity's ID - return None + try: + reference_obj = self.get_cached_value(instance) + except KeyError: + # Fetch target object by own Identifier + id_value = getattr(instance, instance.id_field.field_name) + reference_obj = self.to_cls.find_by(**{self._linked_attribute(owner): id_value}) + if reference_obj: + self.value = reference_obj + self.set_cached_value(instance, reference_obj) + else: + # No Objects were found in the remote entity with this Entity's ID + reference_obj = None + + return reference_obj + + def __set__(self, instance, value): + """Cannot set values through the HasOne association""" + raise exceptions.NotSupportedError( + "Object does not support the operation being performed", + self.field_name + ) + + def __delete__(self, instance): + """Cannot pop values for a HasOne association""" + raise exceptions.NotSupportedError( + "Object does not support the operation being performed", + self.field_name + ) + + def get_cache_name(self): + return self.field_name diff --git a/src/protean/core/field/base.py b/src/protean/core/field/base.py index aa35ad15..83e498de 100644 --- a/src/protean/core/field/base.py +++ b/src/protean/core/field/base.py @@ -222,3 +222,6 @@ def _load(self, value: Any): self._run_validators(value) return value + + def get_cache_name(self): + return self.field_name diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index cf111054..7982926e 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -2,6 +2,7 @@ from collections import OrderedDict +import mock import pytest from tests.support.dog import (Dog, RelatedDog, RelatedDog2, DogRelatedByEmail, HasOneDog1, HasOneDog2, HasOneDog3) @@ -1241,6 +1242,7 @@ def test_init_with_string_reference(self): email='jeff.kennedy@presidents.com') dog = RelatedDog2(id=1, name='John Doe', age=10, owner=human) assert dog.owner == human + assert not hasattr(human, 'dog') # Reverse linkages are not provided by default def test_save(self): """Test successful RelatedDog save""" @@ -1372,6 +1374,17 @@ def test_via_with_shadow_attribute_assign(self): assert hasattr(dog, 'owner_email') assert dog.owner_email == human.email + @mock.patch('protean.core.entity.Entity.find_by') + def test_caching(self, find_by_mock): + """Test that subsequent accesses after first retrieval don't fetch record again""" + human = Human.create(first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = RelatedDog(id=1, name='John Doe', age=10, owner_id=human.id) + + for _ in range(3): + getattr(dog, 'owner') + assert find_by_mock.call_count == 1 + class TestHasOne: """Class to test HasOne Association""" @@ -1402,3 +1415,15 @@ def test_init_with_no_reference(self): assert dog.human_id == human.id assert not hasattr(dog, 'human') assert human.dog.id == dog.id + + @mock.patch('protean.core.entity.Entity.find_by') + def test_caching(self, find_by_mock): + """Test that subsequent accesses after first retrieval don't fetch record again""" + human = HasOneHuman3.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + HasOneDog3.create(id=101, name='John Doe', age=10, human_id=human.id) + + for _ in range(3): + getattr(human, 'dog') + assert find_by_mock.call_count == 1 From 93e38be487fddb2530c6a237922f235e60d97576 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 17:05:43 -0800 Subject: [PATCH 06/11] 75 Associations - Interim Commit 6 - HasMany Implementation --- src/protean/core/entity.py | 7 ++- src/protean/core/field/association.py | 82 +++++++++++++++++++++++++++ src/protean/core/field/base.py | 1 - tests/conftest.py | 19 ++++++- tests/core/test_entity.py | 61 +++++++++++++++++++- tests/support/dog.py | 56 +++++++++++++++++- tests/support/human.py | 55 ++++++++++++++++++ 7 files changed, 271 insertions(+), 10 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 8326e3dd..b1946828 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -6,7 +6,7 @@ from protean.core.exceptions import ValidationError from protean.core.field import Auto from protean.core.field import Field, Reference -from protean.core.field.association import HasOne +from protean.core.field.association import HasOne, HasMany from protean.utils.generic import classproperty from protean.utils.query import Q @@ -85,7 +85,8 @@ def _load_fields(new_class, attrs): is set up in this method, while `parent_id` is set up in `_set_up_reference_fields()`. """ for attr_name, attr_obj in attrs.items(): - if isinstance(attr_obj, (Field, Reference)) and not isinstance(attr_obj, HasOne): + if (isinstance(attr_obj, (Field, Reference)) and + not isinstance(attr_obj, (HasOne, HasMany))): setattr(new_class, attr_name, attr_obj) new_class._meta.declared_fields[attr_name] = attr_obj @@ -468,7 +469,7 @@ def __init__(self, *template, **kwargs): # Now load the remaining fields with a None value, which will fail # for required fields for field_name, field_obj in self._meta.declared_fields.items(): - if field_name not in loaded_fields and not isinstance(field_obj, HasOne): + if field_name not in loaded_fields and not isinstance(field_obj, (HasOne, HasMany)): # Check that the field is not set already, which would happen if we are # dealing with reference fields if getattr(self, field_name, None) is None: diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index e6adddc8..59f75ee3 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -233,3 +233,85 @@ def __delete__(self, instance): def get_cache_name(self): return self.field_name + + +class HasMany(FieldCacheMixin, Field): + """ + Provide a HasMany relation to a remote entity. + + By default, the query will lookup an attribute of the form `_id` + to fetch and populate. This behavior can be changed by using the `via` argument. + """ + + def __init__(self, to_cls, via=None, **kwargs): + super().__init__(**kwargs) + self.to_cls = to_cls + self.via = via + + def _cast_to_type(self, value): + """Verify type of value assigned to the association field""" + # FIXME Verify that the value being assigned is compatible with the associated Entity + return value + + def _linked_attribute(self, owner): + """Choose the Linkage attribute between `via` and own entity's `id_field` + + FIXME Explore converting this method into an attribute, and treating it + uniformly at `association` level. + """ + return self.via or (inflection.underscore(owner.__name__) + '_id') + + def _fetch_to_cls_from_registry(self, entity): + """Private Method to fetch an Entity class from an entity's name""" + # Defensive check to ensure we only process if `to_cls` is a string + if isinstance(entity, str): + from protean.core.repository import repo_factory # FIXME Move to a better placement + + try: + return repo_factory.get_entity(self.to_cls) + except AssertionError: + # Entity has not been registered (yet) + # FIXME print a helpful debug message + raise + else: + return self.to_cls + + def __get__(self, instance, owner): + """Retrieve associated objects""" + + # If `to_cls` was specified as a string, take this opportunity to fetch + # and update the correct entity class against it, if not already done + if isinstance(self.to_cls, str): + self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) + + try: + reference_obj = self.get_cached_value(instance) + except KeyError: + # Fetch target object by own Identifier + id_value = getattr(instance, instance.id_field.field_name) + reference_obj = self.to_cls.query.filter(**{self._linked_attribute(owner): id_value}) + if reference_obj: + self.value = reference_obj + self.set_cached_value(instance, reference_obj) + else: + # No Objects were found in the remote entity with this Entity's ID + reference_obj = None + + return reference_obj + + def __set__(self, instance, value): + """Cannot set values through the HasMany association""" + raise exceptions.NotSupportedError( + "Object does not support the operation being performed", + self.field_name + ) + + def __delete__(self, instance): + """Cannot pop values for a HasMany association""" + raise exceptions.NotSupportedError( + "Object does not support the operation being performed", + self.field_name + ) + + def get_cache_name(self): + return self.field_name diff --git a/src/protean/core/field/base.py b/src/protean/core/field/base.py index 83e498de..a31e15f1 100644 --- a/src/protean/core/field/base.py +++ b/src/protean/core/field/base.py @@ -139,7 +139,6 @@ def fail(self, key, **kwargs): raise AssertionError(msg) if isinstance(msg, str): msg = msg.format(**kwargs) - raise exceptions.ValidationError(msg, self.field_name) @property diff --git a/tests/conftest.py b/tests/conftest.py index 24513288..a21a9759 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,9 +11,12 @@ def run_around_tests(): """Initialize DogModel with Dict Repo""" from protean.core.repository import repo_factory from tests.support.dog import (DogModel, RelatedDogModel, DogRelatedByEmailModel, - HasOneDog1Model, HasOneDog2Model, HasOneDog3Model) + HasOneDog1Model, HasOneDog2Model, HasOneDog3Model, + HasManyDog1Model, HasManyDog2Model, HasManyDog3Model) from tests.support.human import (HumanModel, HasOneHuman1Model, - HasOneHuman2Model, HasOneHuman3Model) + HasOneHuman2Model, HasOneHuman3Model, + HasManyHuman1Model, HasManyHuman2Model, + HasManyHuman3Model) repo_factory.register(DogModel) repo_factory.register(RelatedDogModel) @@ -21,10 +24,16 @@ def run_around_tests(): repo_factory.register(HasOneDog1Model) repo_factory.register(HasOneDog2Model) repo_factory.register(HasOneDog3Model) + repo_factory.register(HasManyDog1Model) + repo_factory.register(HasManyDog2Model) + repo_factory.register(HasManyDog3Model) repo_factory.register(HumanModel) repo_factory.register(HasOneHuman1Model) repo_factory.register(HasOneHuman2Model) repo_factory.register(HasOneHuman3Model) + repo_factory.register(HasManyHuman1Model) + repo_factory.register(HasManyHuman2Model) + repo_factory.register(HasManyHuman3Model) # A test function will be run at this point yield @@ -35,7 +44,13 @@ def run_around_tests(): repo_factory.HasOneDog1.delete_all() repo_factory.HasOneDog2.delete_all() repo_factory.HasOneDog3.delete_all() + repo_factory.HasManyDog1.delete_all() + repo_factory.HasManyDog2.delete_all() + repo_factory.HasManyDog3.delete_all() repo_factory.Human.delete_all() repo_factory.HasOneHuman1.delete_all() repo_factory.HasOneHuman2.delete_all() repo_factory.HasOneHuman3.delete_all() + repo_factory.HasManyHuman1.delete_all() + repo_factory.HasManyHuman2.delete_all() + repo_factory.HasManyHuman3.delete_all() diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index 7982926e..154aaff3 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -5,8 +5,11 @@ import mock import pytest from tests.support.dog import (Dog, RelatedDog, RelatedDog2, DogRelatedByEmail, - HasOneDog1, HasOneDog2, HasOneDog3) -from tests.support.human import Human, HasOneHuman1, HasOneHuman2, HasOneHuman3 + HasOneDog1, HasOneDog2, HasOneDog3, + HasManyDog1, HasManyDog2, HasManyDog3) +from tests.support.human import (Human, HasOneHuman1, HasOneHuman2, HasOneHuman3, + HasManyHuman1, HasManyHuman2, HasManyHuman3) + from protean.core import field from protean.core.entity import Entity, QuerySet @@ -1427,3 +1430,57 @@ def test_caching(self, find_by_mock): for _ in range(3): getattr(human, 'dog') assert find_by_mock.call_count == 1 + + class TestHasMany: + """Class to test HasMany Association""" + + def test_init(self): + """Test successful HasOne initialization""" + human = HasManyHuman1.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog1 = HasManyDog1.create(id=101, name='John Doe', age=10, has_many_human1=human) + dog2 = HasManyDog1.create(id=102, name='Jane Doe', age=10, has_many_human1=human) + + assert dog1.has_many_human1.id == human.id + assert dog2.has_many_human1.id == human.id + assert len(human.dogs) == 2 + + def test_init_with_via(self): + """Test successful HasMany initialization with a class containing via""" + human = HasManyHuman2.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog1 = HasManyDog2.create(id=101, name='John Doe', age=10, human=human) + dog2 = HasManyDog2.create(id=102, name='Jane Doe', age=10, human=human) + + assert dog1.human.id == human.id + assert dog2.human.id == human.id + + assert len(human.dogs) == 2 + + def test_init_with_no_reference(self): + """Test successful HasOne initialization with a class containing via""" + human = HasManyHuman3.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog1 = HasManyDog3.create(id=101, name='John Doe', age=10, human_id=human.id) + + assert dog1.human_id == human.id + assert not hasattr(dog1, 'human') + + @mock.patch('protean.core.entity.QuerySet.filter') + @mock.patch('protean.core.entity.Entity.exists') + def test_caching(self, exists_mock, filter_mock): + """Test that subsequent accesses after first retrieval don't fetch record again""" + exists_mock.return_value = False + human = HasManyHuman3.create( + first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + HasManyDog3.create(id=101, name='John Doe', human_id=human.id) + HasManyDog3.create(id=102, name='Jane Doe', human_id=human.id) + HasManyDog3.create(id=103, name='Jinny Doe', human_id=human.id) + + for _ in range(3): + getattr(human, 'dogs') + assert filter_mock.call_count == 1 diff --git a/tests/support/dog.py b/tests/support/dog.py index 77ebf22e..51373b81 100644 --- a/tests/support/dog.py +++ b/tests/support/dog.py @@ -1,6 +1,6 @@ """Support Classes for Test Cases""" -from tests.support.human import Human, HasOneHuman1, HasOneHuman2 +from tests.support.human import Human, HasOneHuman1, HasOneHuman2, HasManyHuman1, HasManyHuman2 from protean.core import field from protean.core.entity import Entity @@ -90,7 +90,9 @@ class Meta: class HasOneDog2(Entity): - """This is a dummy Dog Entity class to test HasOne Association""" + """This is a dummy Dog Entity class to test HasOne Association, where the associated + has defined a `via` attribute to finetune linkage + """ name = field.String(required=True, unique=True, max_length=50) age = field.Integer(default=5) human = field.Reference(HasOneHuman2) @@ -119,3 +121,53 @@ class Meta: """ Meta class for model options""" entity = HasOneDog3 model_name = 'has_one_dogs3' + + +class HasManyDog1(Entity): + """This is a dummy Dog Entity class to test HasMany Association""" + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + has_many_human1 = field.Reference(HasManyHuman1) + + +class HasManyDog1Model(DictModel): + """ Model for the HasManyDog1 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasManyDog1 + model_name = 'has_many_dogs1' + + +class HasManyDog2(Entity): + """This is a dummy Dog Entity class to test HasMany Association, where the associated + has defined a `via` attribute to finetune linkage + """ + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + human = field.Reference(HasManyHuman2) + + +class HasManyDog2Model(DictModel): + """ Model for the HasManyDog2 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasManyDog2 + model_name = 'has_many_dogs2' + + +class HasManyDog3(Entity): + """This is a dummy Dog Entity class to test HasMany Association""" + name = field.String(required=True, unique=True, max_length=50) + age = field.Integer(default=5) + human_id = field.Integer() + + +class HasManyDog3Model(DictModel): + """ Model for the HasManyDog3 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasManyDog3 + model_name = 'has_many_dogs3' diff --git a/tests/support/human.py b/tests/support/human.py index 52f3e597..4fac0481 100644 --- a/tests/support/human.py +++ b/tests/support/human.py @@ -75,3 +75,58 @@ class Meta: """ Meta class for model options""" entity = HasOneHuman3 model_name = 'has_one_humans3' + + +class HasManyHuman1(Entity): + """This is a dummy Human Entity class to test HasMany association""" + first_name = field.String(required=True, unique=True, max_length=50) + last_name = field.String(required=True, unique=True, max_length=50) + email = field.String(required=True, unique=True, max_length=50) + dogs = association.HasMany('HasManyDog1') + + +class HasManyHuman1Model(DictModel): + """ Model for the HasManyHuman1 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasManyHuman1 + model_name = 'has_many_humans1' + + +class HasManyHuman2(Entity): + """This is a dummy Human Entity class to test HasMany association + with a custom attribute defined in `via` argument to field + """ + first_name = field.String(required=True, unique=True, max_length=50) + last_name = field.String(required=True, unique=True, max_length=50) + email = field.String(required=True, unique=True, max_length=50) + dogs = association.HasMany('HasManyDog2', via='human_id') + + +class HasManyHuman2Model(DictModel): + """ Model for the HasManyHuman2 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasManyHuman2 + model_name = 'has_many_humans2' + + +class HasManyHuman3(Entity): + """This is a dummy Human Entity class to test HasMany association + when there is no corresponding Reference defined in the target class + """ + first_name = field.String(required=True, unique=True, max_length=50) + last_name = field.String(required=True, unique=True, max_length=50) + email = field.String(required=True, unique=True, max_length=50) + dogs = association.HasMany('HasManyDog3', via='human_id') + + +class HasManyHuman3Model(DictModel): + """ Model for the HasManyHuman3 Entity""" + + class Meta: + """ Meta class for model options""" + entity = HasManyHuman3 + model_name = 'has_many_humans3' From efeaaf6ab8f38e4b5c56f9f5625cd637785eb86a Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Tue, 26 Feb 2019 17:13:59 -0800 Subject: [PATCH 07/11] 75 Associations - Interim Commit 7 - DRY association code --- src/protean/core/field/association.py | 102 ++++++-------------------- 1 file changed, 24 insertions(+), 78 deletions(-) diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index 59f75ee3..fdd725c7 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -1,3 +1,5 @@ +from abc import abstractmethod + from .base import Field from .mixins import FieldCacheMixin @@ -153,13 +155,8 @@ def _cast_to_type(self, value): return value -class HasOne(FieldCacheMixin, Field): - """ - Provide a HasOne relation to a remote entity. - - By default, the query will lookup an attribute of the form `_id` - to fetch and populate. This behavior can be changed by using the `via` argument. - """ +class Association(FieldCacheMixin, Field): + """Base class for all association classes""" def __init__(self, to_cls, via=None, **kwargs): super().__init__(**kwargs) @@ -207,7 +204,7 @@ def __get__(self, instance, owner): except KeyError: # Fetch target object by own Identifier id_value = getattr(instance, instance.id_field.field_name) - reference_obj = self.to_cls.find_by(**{self._linked_attribute(owner): id_value}) + reference_obj = self._fetch_objects(self._linked_attribute(owner), id_value) if reference_obj: self.value = reference_obj self.set_cached_value(instance, reference_obj) @@ -217,6 +214,11 @@ def __get__(self, instance, owner): return reference_obj + @abstractmethod + def _fetch_objects(self, key, value): + """Placeholder method for customized Association query methods""" + raise NotImplementedError + def __set__(self, instance, value): """Cannot set values through the HasOne association""" raise exceptions.NotSupportedError( @@ -235,83 +237,27 @@ def get_cache_name(self): return self.field_name -class HasMany(FieldCacheMixin, Field): +class HasOne(Association): """ - Provide a HasMany relation to a remote entity. + Provide a HasOne relation to a remote entity. By default, the query will lookup an attribute of the form `_id` to fetch and populate. This behavior can be changed by using the `via` argument. """ - def __init__(self, to_cls, via=None, **kwargs): - super().__init__(**kwargs) - self.to_cls = to_cls - self.via = via - - def _cast_to_type(self, value): - """Verify type of value assigned to the association field""" - # FIXME Verify that the value being assigned is compatible with the associated Entity - return value - - def _linked_attribute(self, owner): - """Choose the Linkage attribute between `via` and own entity's `id_field` - - FIXME Explore converting this method into an attribute, and treating it - uniformly at `association` level. - """ - return self.via or (inflection.underscore(owner.__name__) + '_id') - - def _fetch_to_cls_from_registry(self, entity): - """Private Method to fetch an Entity class from an entity's name""" - # Defensive check to ensure we only process if `to_cls` is a string - if isinstance(entity, str): - from protean.core.repository import repo_factory # FIXME Move to a better placement - - try: - return repo_factory.get_entity(self.to_cls) - except AssertionError: - # Entity has not been registered (yet) - # FIXME print a helpful debug message - raise - else: - return self.to_cls - - def __get__(self, instance, owner): - """Retrieve associated objects""" - - # If `to_cls` was specified as a string, take this opportunity to fetch - # and update the correct entity class against it, if not already done - if isinstance(self.to_cls, str): - self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) - - try: - reference_obj = self.get_cached_value(instance) - except KeyError: - # Fetch target object by own Identifier - id_value = getattr(instance, instance.id_field.field_name) - reference_obj = self.to_cls.query.filter(**{self._linked_attribute(owner): id_value}) - if reference_obj: - self.value = reference_obj - self.set_cached_value(instance, reference_obj) - else: - # No Objects were found in the remote entity with this Entity's ID - reference_obj = None + def _fetch_objects(self, key, value): + """Fetch Multiple linked objects""" + return self.to_cls.find_by(**{key: value}) - return reference_obj - def __set__(self, instance, value): - """Cannot set values through the HasMany association""" - raise exceptions.NotSupportedError( - "Object does not support the operation being performed", - self.field_name - ) +class HasMany(Association): + """ + Provide a HasMany relation to a remote entity. - def __delete__(self, instance): - """Cannot pop values for a HasMany association""" - raise exceptions.NotSupportedError( - "Object does not support the operation being performed", - self.field_name - ) + By default, the query will lookup an attribute of the form `_id` + to fetch and populate. This behavior can be changed by using the `via` argument. + """ - def get_cache_name(self): - return self.field_name + def _fetch_objects(self, key, value): + """Fetch Multiple linked objects""" + return self.to_cls.query.filter(**{key: value}) \ No newline at end of file From f795469d53adfe00e8bf5167d0e38a0e933b8547 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Wed, 27 Feb 2019 09:52:31 -0800 Subject: [PATCH 08/11] 75 Associations - Interim Commit 8 - Move Descriptor code into a Mixin --- src/protean/core/entity.py | 6 ++-- src/protean/core/field/association.py | 9 +++--- src/protean/core/field/base.py | 30 ++++-------------- src/protean/core/field/mixins.py | 44 ++++++++++++++++++++++++++- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index b1946828..339a11a3 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -6,7 +6,6 @@ from protean.core.exceptions import ValidationError from protean.core.field import Auto from protean.core.field import Field, Reference -from protean.core.field.association import HasOne, HasMany from protean.utils.generic import classproperty from protean.utils.query import Q @@ -85,8 +84,7 @@ def _load_fields(new_class, attrs): is set up in this method, while `parent_id` is set up in `_set_up_reference_fields()`. """ for attr_name, attr_obj in attrs.items(): - if (isinstance(attr_obj, (Field, Reference)) and - not isinstance(attr_obj, (HasOne, HasMany))): + if isinstance(attr_obj, (Field, Reference)): setattr(new_class, attr_name, attr_obj) new_class._meta.declared_fields[attr_name] = attr_obj @@ -469,7 +467,7 @@ def __init__(self, *template, **kwargs): # Now load the remaining fields with a None value, which will fail # for required fields for field_name, field_obj in self._meta.declared_fields.items(): - if field_name not in loaded_fields and not isinstance(field_obj, (HasOne, HasMany)): + if field_name not in loaded_fields: # Check that the field is not set already, which would happen if we are # dealing with reference fields if getattr(self, field_name, None) is None: diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index fdd725c7..92cf7b2e 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -1,7 +1,7 @@ from abc import abstractmethod from .base import Field -from .mixins import FieldCacheMixin +from .mixins import FieldCacheMixin, FieldDescriptorMixin from protean.core import exceptions from protean.utils import inflection @@ -155,11 +155,12 @@ def _cast_to_type(self, value): return value -class Association(FieldCacheMixin, Field): +class Association(FieldDescriptorMixin, FieldCacheMixin): """Base class for all association classes""" def __init__(self, to_cls, via=None, **kwargs): super().__init__(**kwargs) + self.to_cls = to_cls self.via = via @@ -220,14 +221,14 @@ def _fetch_objects(self, key, value): raise NotImplementedError def __set__(self, instance, value): - """Cannot set values through the HasOne association""" + """Cannot set values through an association""" raise exceptions.NotSupportedError( "Object does not support the operation being performed", self.field_name ) def __delete__(self, instance): - """Cannot pop values for a HasOne association""" + """Cannot pop values for an association""" raise exceptions.NotSupportedError( "Object does not support the operation being performed", self.field_name diff --git a/src/protean/core/field/base.py b/src/protean/core/field/base.py index a31e15f1..1c987fc6 100644 --- a/src/protean/core/field/base.py +++ b/src/protean/core/field/base.py @@ -6,6 +6,8 @@ from typing import Any from typing import Iterable +from .mixins import FieldDescriptorMixin + from protean.core import exceptions MISSING_ERROR_MESSAGE = ( @@ -14,7 +16,7 @@ ) -class Field(metaclass=ABCMeta): +class Field(FieldDescriptorMixin, metaclass=ABCMeta): """Abstract field from which other fields should extend. :param default: If set, this value will be used during entity loading @@ -50,6 +52,9 @@ def __init__(self, identifier: bool = False, default: Any = None, label: str = None, choices: enum.Enum = None, validators: Iterable = (), value=None, error_messages: dict = None): + # Nothing to be passed into FieldCacheMixin for initialization + super().__init__(**{}) + self.identifier = identifier self.default = default @@ -79,10 +84,6 @@ def __init__(self, identifier: bool = False, default: Any = None, # Hold a reference to Entity registering the field self._entity_cls = None - # These are set up when the owner (Entity class) adds the field to itself - self.field_name = None - self.attribute_name = None - # Collect default error message from self and parent classes messages = {} for cls in reversed(self.__class__.__mro__): @@ -90,17 +91,6 @@ def __init__(self, identifier: bool = False, default: Any = None, messages.update(error_messages or {}) self.error_messages = messages - def __set_name__(self, entity_cls, name): - self.field_name = name - self.attribute_name = self.get_attribute_name() - - # Record Entity setting up the field - self._entity_cls = entity_cls - - # `self.label` should default to being based on the field name. - if self.label is None: - self.label = self.field_name.replace('_', ' ').capitalize() - def __get__(self, instance, owner): return instance.__dict__.get(self.field_name, self.value) @@ -119,14 +109,6 @@ def value(self): def value(self, value): self._value = value if value else None - def get_attribute_name(self): - """Return Attribute name for the attribute. - - Defaults to the field name in this base class, but can be overridden. - Handy when defining complex objects with shadow attributes, like Foreign keys. - """ - return self.field_name - def fail(self, key, **kwargs): """A helper method that simply raises a `ValidationError`. """ diff --git a/src/protean/core/field/mixins.py b/src/protean/core/field/mixins.py index c6dbc4ec..1a07d7a8 100644 --- a/src/protean/core/field/mixins.py +++ b/src/protean/core/field/mixins.py @@ -2,7 +2,7 @@ class FieldCacheMixin: - """Provide an API for working with the model's fields value cache.""" + """Provide an API for working with the entity's fields value cache.""" def get_cache_name(self): raise NotImplementedError @@ -24,3 +24,45 @@ def set_cached_value(self, instance, value): def delete_cached_value(self, instance): del instance._state.fields_cache[self.get_cache_name()] + + +class FieldDescriptorMixin: + """Provide basic implementation to treat the Field as a descriptor""" + + def __init__(self, *args, **kwargs): + """Initialize common Field Attributes""" + # These are set up when the owner (Entity class) adds the field to itself + self.field_name = None + self.attribute_name = None + self.label = None + + def __set_name__(self, entity_cls, name): + self.field_name = name + self.attribute_name = self.get_attribute_name() + + # Record Entity setting up the field + self._entity_cls = entity_cls + + # `self.label` should default to being based on the field name. + if self.label is None: + self.label = self.field_name.replace('_', ' ').capitalize() + + def get_attribute_name(self): + """Return Attribute name for the attribute. + + Defaults to the field name in this base class, but can be overridden. + Handy when defining complex objects with shadow attributes, like Foreign keys. + """ + return self.field_name + + def __get__(self, instance, owner): + """Placeholder for handling `getattr` operations on attributes""" + raise NotImplementedError + + def __set__(self, instance, value): + """Placeholder for handling `setattr` operations on attributes""" + raise NotImplementedError + + def __delete__(self, instance): + """Placeholder for handling `del` operations on attributes""" + raise NotImplementedError From 9fd640ab31cd96cbb6a0b2918b6716ba616501d3 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Wed, 27 Feb 2019 15:15:07 -0800 Subject: [PATCH 09/11] 75 Associations - Interim Commit 9 - Do not autoload Reference too --- src/protean/core/entity.py | 6 +- src/protean/core/field/__init__.py | 4 +- src/protean/core/field/association.py | 96 ++++++++++++++++++--------- src/protean/core/field/base.py | 5 +- src/protean/core/field/mixins.py | 3 +- tests/core/test_entity.py | 51 +++++++++++++- 6 files changed, 122 insertions(+), 43 deletions(-) diff --git a/src/protean/core/entity.py b/src/protean/core/entity.py index 8442f3ff..4e443bfb 100644 --- a/src/protean/core/entity.py +++ b/src/protean/core/entity.py @@ -6,7 +6,7 @@ from protean.core.exceptions import ObjectNotFoundError from protean.core.exceptions import ValidationError from protean.core.field import Auto -from protean.core.field import Field, Reference +from protean.core.field import Field, Reference, ReferenceField from protean.utils.generic import classproperty from protean.utils.query import Q @@ -497,9 +497,7 @@ def __init__(self, *template, **kwargs): # for required fields for field_name, field_obj in self._meta.declared_fields.items(): if field_name not in loaded_fields: - # Check that the field is not set already, which would happen if we are - # dealing with reference fields - if getattr(self, field_name, None) is None: + if not isinstance(field_obj, (Reference, ReferenceField)): setattr(self, field_name, None) # Raise any errors found during load diff --git a/src/protean/core/field/__init__.py b/src/protean/core/field/__init__.py index 35fb5750..f5839f53 100644 --- a/src/protean/core/field/__init__.py +++ b/src/protean/core/field/__init__.py @@ -1,6 +1,6 @@ """Package for defining Field type and its implementations""" -from .association import Reference +from .association import Reference, ReferenceField from .base import Field from .basic import Auto from .basic import Boolean @@ -18,4 +18,4 @@ __all__ = ('Field', 'String', 'Boolean', 'Integer', 'Float', 'List', 'Dict', 'Auto', 'Date', 'DateTime', 'Text', 'StringShort', 'StringMedium', - 'StringLong', 'Reference') + 'StringLong', 'Reference', 'ReferenceField') diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index 92cf7b2e..1af1b3c0 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -23,18 +23,6 @@ def __set__(self, instance, value): if value: instance.__dict__[self.field_name] = value - - # Fetch target object and refresh the reference field value - reference_obj = self.reference.to_cls.find_by( - **{self.reference.linked_attribute: value}) - if reference_obj: - self.reference.value = reference_obj - instance.__dict__[self.reference.field_name] = reference_obj - else: - # Object was not found in the database - raise exceptions.ValueError( - "Target Object not found", - self.reference.field_name) else: self._reset_values(instance) @@ -49,10 +37,11 @@ def _cast_to_type(self, value): def _reset_values(self, instance): """Reset all associated values and clean up dictionary items""" - instance.__dict__.pop(self.field_name) - instance.__dict__.pop(self.reference.field_name) - self.reference.value = None self.value = None + self.reference.value = None + instance.__dict__.pop(self.field_name, None) + instance.__dict__.pop(self.reference.field_name, None) + self.reference.delete_cached_value(instance) class Reference(FieldCacheMixin, Field): @@ -81,6 +70,9 @@ def get_shadow_field(self): Primarily used during Entity initialization to register shadow field""" return (self.attribute_name, self.relation) + def get_cache_name(self): + return self.field_name + @property def linked_attribute(self): """Choose the Linkage attribute between `via` and designated `id_field` of the target class @@ -115,8 +107,11 @@ def _fetch_to_cls_from_registry(self, entity): else: return self.to_cls - def __set__(self, instance, value): - """Override `__set__` to coordinate between relation field and its shadow attribute""" + def __get__(self, instance, owner): + """Retrieve associated objects""" + + # If `to_cls` was specified as a string, take this opportunity to fetch + # and update the correct entity class against it, if not already done if isinstance(self.to_cls, str): self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) @@ -124,30 +119,66 @@ def __set__(self, instance, value): # initialized with `id_field` self.attribute_name = self.get_attribute_name() - value = self._load(value) + reference_obj = None + if hasattr(instance, '_state'): + try: + reference_obj = self.get_cached_value(instance) + except KeyError: + # Fetch target object by own Identifier + id_value = getattr(instance, self.get_attribute_name()) + if id_value: + reference_obj = self._fetch_objects(self.linked_attribute, id_value) + if reference_obj: + self.value = reference_obj + instance.__dict__[self.field_name] = reference_obj + self.set_cached_value(instance, reference_obj) + else: + # No Objects were found in the remote entity with this Entity's ID + pass + + return reference_obj + + def _fetch_objects(self, key, value): + """Fetch Multiple linked objects""" + return self.to_cls.find_by(**{key: value}) + def __set__(self, instance, value): + """Override `__set__` to coordinate between relation field and its shadow attribute""" if value: - # Check if the reference object has been saved. Otherwise, throw ValueError - if value.id is None: # FIXME not a comprehensive check. Should refer to state - raise ValueError( - "Target Object must be saved before being referenced", - self.field_name) - else: - self.relation.value = value.id - instance.__dict__[self.field_name] = value - instance.__dict__[self.attribute_name] = getattr(value, self.linked_attribute) + if isinstance(self.to_cls, str): + self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) + + # Refresh attribute name, now that we know `to_cls` Entity and it has been + # initialized with `id_field` + self.attribute_name = self.get_attribute_name() + + value = self._load(value) + + if value: + # Check if the reference object has been saved. Otherwise, throw ValueError + if value.id is None: # FIXME not a comprehensive check. Should refer to state + raise ValueError( + "Target Object must be saved before being referenced", + self.field_name) + else: + self.value = value + self.relation.value = getattr(value, self.linked_attribute) + instance.__dict__[self.field_name] = value + instance.__dict__[self.attribute_name] = getattr(value, self.linked_attribute) + self.set_cached_value(instance, value) else: - self._reset_values(instance) + self.value = None + self.relation.value = None + instance.__dict__.pop(self.field_name, None) + instance.__dict__.pop(self.attribute_name, None) + self.delete_cached_value(instance) def __delete__(self, instance): - self._reset_values(instance) - - def _reset_values(self, instance): - """Reset all associated values and clean up dictionary items""" self.value = None self.relation.value = None instance.__dict__.pop(self.field_name, None) instance.__dict__.pop(self.attribute_name, None) + self.delete_cached_value(instance) def _cast_to_type(self, value): if not isinstance(value, self.to_cls): @@ -208,6 +239,7 @@ def __get__(self, instance, owner): reference_obj = self._fetch_objects(self._linked_attribute(owner), id_value) if reference_obj: self.value = reference_obj + instance.__dict__[self.field_name] = reference_obj self.set_cached_value(instance, reference_obj) else: # No Objects were found in the remote entity with this Entity's ID diff --git a/src/protean/core/field/base.py b/src/protean/core/field/base.py index fb90d838..750e94a6 100644 --- a/src/protean/core/field/base.py +++ b/src/protean/core/field/base.py @@ -92,7 +92,10 @@ def __init__(self, identifier: bool = False, default: Any = None, self.error_messages = messages def __get__(self, instance, owner): - return instance.__dict__.get(self.field_name, self.value) + if hasattr(instance, '__dict__'): + return instance.__dict__.get(self.field_name, self.value) + else: + return None def __set__(self, instance, value): value = self._load(value) diff --git a/src/protean/core/field/mixins.py b/src/protean/core/field/mixins.py index 1a07d7a8..5e314bfd 100644 --- a/src/protean/core/field/mixins.py +++ b/src/protean/core/field/mixins.py @@ -23,7 +23,8 @@ def set_cached_value(self, instance, value): instance._state.fields_cache[self.get_cache_name()] = value def delete_cached_value(self, instance): - del instance._state.fields_cache[self.get_cache_name()] + if self.get_cache_name() in instance._state.fields_cache: + del instance._state.fields_cache[self.get_cache_name()] class FieldDescriptorMixin: diff --git a/tests/core/test_entity.py b/tests/core/test_entity.py index 9867dd3b..a303332e 100644 --- a/tests/core/test_entity.py +++ b/tests/core/test_entity.py @@ -1313,14 +1313,18 @@ def test_init(self): human = Human.create(first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = RelatedDog(id=1, name='John Doe', age=10, owner=human) - assert dog.owner == human + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) + assert dog.owner.id == human.id + assert dog.owner_id == human.id def test_init_with_string_reference(self): """Test successful RelatedDog initialization""" human = Human.create(first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = RelatedDog2(id=1, name='John Doe', age=10, owner=human) - assert dog.owner == human + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) + assert dog.owner.id == human.id + assert dog.owner_id == human.id assert not hasattr(human, 'dog') # Reverse linkages are not provided by default def test_save(self): @@ -1328,8 +1332,10 @@ def test_save(self): human = Human.create(first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = RelatedDog(id=1, name='John Doe', age=10, owner=human) + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) dog.save() assert dog.id is not None + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) def test_unsaved_entity_init(self): """Test that initialization fails when an unsaved entity is assigned to a relation""" @@ -1342,7 +1348,9 @@ def test_unsaved_entity_assign(self): with pytest.raises(ValueError): human = Human(first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') + dog = RelatedDog(id=1, name='John Doe', age=10) + assert any(key in dog.__dict__ for key in ['owner', 'owner_id']) is False dog.owner = human def test_invalid_entity_type(self): @@ -1357,25 +1365,49 @@ def test_shadow_attribute(self): human = Human.create(first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = RelatedDog(id=1, name='John Doe', age=10, owner=human) + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) assert human.id is not None assert dog.owner_id == human.id def test_save_after_assign(self): - """Test identifier backing the association""" + """Test saving after assignment (post init)""" human = Human.create(id=101, first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = RelatedDog(id=1, name='John Doe', age=10) + assert any(key in dog.__dict__ for key in ['owner', 'owner_id']) is False dog.owner = human + dog.save() + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) assert dog.owner_id == human.id + def test_fetch_after_save(self): + """Test fetch after save and ensure reference is not auto-loaded""" + human = Human.create(id=101, first_name='Jeff', last_name='Kennedy', + email='jeff.kennedy@presidents.com') + dog = RelatedDog(id=1, name='John Doe', age=10) + dog.owner = human + dog.save() + + dog2 = RelatedDog.get(dog.id) + # Reference attribute is not loaded automatically + assert 'owner' not in dog2.__dict__ + assert dog2.owner_id == human.id + + # Accessing attribute shows it up in __dict__ + getattr(dog2, 'owner') + assert 'owner' in dog2.__dict__ + def test_shadow_attribute_init(self): """Test identifier backing the association""" human = Human.create(id=101, first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = RelatedDog(id=1, name='John Doe', age=10, owner_id=human.id) + assert 'owner_id' in dog.__dict__ + assert 'owner' not in dog.__dict__ dog.save() assert dog.owner_id == human.id assert dog.owner.id == human.id + assert all(key in dog.__dict__ for key in ['owner', 'owner_id']) def test_shadow_attribute_assign(self): """Test identifier backing the association""" @@ -1383,9 +1415,11 @@ def test_shadow_attribute_assign(self): email='jeff.kennedy@presidents.com') dog = RelatedDog(id=1, name='John Doe', age=10) dog.owner_id = human.id + assert 'owner' not in dog.__dict__ dog.save() assert dog.owner_id == human.id assert dog.owner.id == human.id + assert 'owner' in dog.__dict__ def test_reference_reset_association_to_None(self): """Test that the reference field and shadow attribute are reset together""" @@ -1396,6 +1430,7 @@ def test_reference_reset_association_to_None(self): assert dog.owner.id == human.id dog.owner = None + assert any(key in dog.__dict__ for key in ['owner', 'owner_id']) is False assert dog.owner is None assert dog.owner_id is None @@ -1408,6 +1443,7 @@ def test_reference_reset_shadow_field_to_None(self): assert dog.owner.id == human.id dog.owner_id = None + assert any(key in dog.__dict__ for key in ['owner', 'owner_id']) is False assert dog.owner is None assert dog.owner_id is None @@ -1420,6 +1456,7 @@ def test_reference_reset_association_by_del(self): assert dog.owner.id == human.id del dog.owner + assert any(key in dog.__dict__ for key in ['owner', 'owner_id']) is False assert dog.owner is None assert dog.owner_id is None @@ -1432,6 +1469,7 @@ def test_reference_reset_shadow_field_by_del(self): assert dog.owner.id == human.id del dog.owner_id + assert any(key in dog.__dict__ for key in ['owner', 'owner_id']) is False assert dog.owner is None assert dog.owner_id is None @@ -1440,6 +1478,7 @@ def test_via(self): human = Human.create(first_name='Jeff', last_name='Kennedy', email='jeff.kennedy@presidents.com') dog = DogRelatedByEmail.create(id=1, name='John Doe', age=10, owner=human) + assert all(key in dog.__dict__ for key in ['owner', 'owner_email']) assert hasattr(dog, 'owner_email') assert dog.owner_email == human.email @@ -1449,6 +1488,7 @@ def test_via_with_shadow_attribute_assign(self): email='jeff.kennedy@presidents.com') dog = DogRelatedByEmail(id=1, name='John Doe', age=10) dog.owner_email = human.email + assert 'owner' not in dog.__dict__ dog.save() assert hasattr(dog, 'owner_email') assert dog.owner_email == human.email @@ -1483,7 +1523,9 @@ def test_init_with_via(self): email='jeff.kennedy@presidents.com') dog = HasOneDog2.create(id=101, name='John Doe', age=10, human=human) assert dog.human == human + assert 'dog' not in human.__dict__ assert human.dog.id == dog.id + assert 'dog' in human.__dict__ def test_init_with_no_reference(self): """Test successful HasOne initialization with a class containing via""" @@ -1493,6 +1535,7 @@ def test_init_with_no_reference(self): dog = HasOneDog3.create(id=101, name='John Doe', age=10, human_id=human.id) assert dog.human_id == human.id assert not hasattr(dog, 'human') + assert 'human' not in dog.__dict__ assert human.dog.id == dog.id @mock.patch('protean.core.entity.Entity.find_by') @@ -1520,7 +1563,9 @@ def test_init(self): assert dog1.has_many_human1.id == human.id assert dog2.has_many_human1.id == human.id + assert 'dogs' not in human.__dict__ assert len(human.dogs) == 2 + assert 'dogs' in human.__dict__ # Avaiable after access def test_init_with_via(self): """Test successful HasMany initialization with a class containing via""" From e6e899ee88540c32a2df89ede3b0affe7a105761 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Wed, 27 Feb 2019 15:26:42 -0800 Subject: [PATCH 10/11] 75 Associations - Interim Commit 10 - DRY set/reset methods for Association and Reference --- src/protean/core/field/association.py | 49 ++++++++++++++++----------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index 1af1b3c0..7a1deb57 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -129,9 +129,7 @@ def __get__(self, instance, owner): if id_value: reference_obj = self._fetch_objects(self.linked_attribute, id_value) if reference_obj: - self.value = reference_obj - instance.__dict__[self.field_name] = reference_obj - self.set_cached_value(instance, reference_obj) + self._set_own_value(instance, reference_obj) else: # No Objects were found in the remote entity with this Entity's ID pass @@ -161,24 +159,34 @@ def __set__(self, instance, value): "Target Object must be saved before being referenced", self.field_name) else: - self.value = value - self.relation.value = getattr(value, self.linked_attribute) - instance.__dict__[self.field_name] = value - instance.__dict__[self.attribute_name] = getattr(value, self.linked_attribute) - self.set_cached_value(instance, value) + self._set_own_value(instance, value) + self._set_relation_value(instance, getattr(value, self.linked_attribute)) else: - self.value = None - self.relation.value = None + self._reset_values(instance) + + def _set_own_value(self, instance, value): + self.value = value + if value is None: instance.__dict__.pop(self.field_name, None) - instance.__dict__.pop(self.attribute_name, None) self.delete_cached_value(instance) + else: + instance.__dict__[self.field_name] = value + self.set_cached_value(instance, value) + + def _set_relation_value(self, instance, value): + self.relation.value = value + if value is None: + instance.__dict__.pop(self.attribute_name, None) + else: + instance.__dict__[self.attribute_name] = value def __delete__(self, instance): - self.value = None - self.relation.value = None - instance.__dict__.pop(self.field_name, None) - instance.__dict__.pop(self.attribute_name, None) - self.delete_cached_value(instance) + self._reset_values(instance) + + def _reset_values(self, instance): + """Reset all associated values and clean up dictionary items""" + self._set_own_value(instance, None) + self._set_relation_value(instance, None) def _cast_to_type(self, value): if not isinstance(value, self.to_cls): @@ -238,15 +246,18 @@ def __get__(self, instance, owner): id_value = getattr(instance, instance.id_field.field_name) reference_obj = self._fetch_objects(self._linked_attribute(owner), id_value) if reference_obj: - self.value = reference_obj - instance.__dict__[self.field_name] = reference_obj - self.set_cached_value(instance, reference_obj) + self._set_own_value(instance, reference_obj) else: # No Objects were found in the remote entity with this Entity's ID reference_obj = None return reference_obj + def _set_own_value(self, instance, value): + self.value = value + instance.__dict__[self.field_name] = value + self.set_cached_value(instance, value) + @abstractmethod def _fetch_objects(self, key, value): """Placeholder method for customized Association query methods""" From 934a66bd340129a9489e3677b3c8e1aa06065b49 Mon Sep 17 00:00:00 2001 From: Subhash Bhushan Date: Wed, 27 Feb 2019 15:35:11 -0800 Subject: [PATCH 11/11] 75 Associations - Interim Commit 11 - DRY by moving generic method into utils --- src/protean/core/field/association.py | 26 ++++---------------------- src/protean/utils/__init__.py | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/protean/core/field/association.py b/src/protean/core/field/association.py index 7a1deb57..ac7939e9 100644 --- a/src/protean/core/field/association.py +++ b/src/protean/core/field/association.py @@ -4,7 +4,7 @@ from .mixins import FieldCacheMixin, FieldDescriptorMixin from protean.core import exceptions -from protean.utils import inflection +from protean import utils class ReferenceField(Field): @@ -89,31 +89,13 @@ def linked_attribute(self): else: return self.via or self.to_cls.id_field.attribute_name - def _fetch_to_cls_from_registry(self, entity): - """Private Method to fetch an Entity class from an entity's name - - FIXME Move this method into utils - """ - # Defensive check to ensure we only process if `to_cls` is a string - if isinstance(entity, str): - from protean.core.repository import repo_factory # FIXME Move to a better placement - - try: - return repo_factory.get_entity(self.to_cls) - except AssertionError: - # Entity has not been registered (yet) - # FIXME print a helpful debug message - raise - else: - return self.to_cls - def __get__(self, instance, owner): """Retrieve associated objects""" # If `to_cls` was specified as a string, take this opportunity to fetch # and update the correct entity class against it, if not already done if isinstance(self.to_cls, str): - self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) + self.to_cls = utils.fetch_entity_cls_from_registry(self.to_cls) # Refresh attribute name, now that we know `to_cls` Entity and it has been # initialized with `id_field` @@ -144,7 +126,7 @@ def __set__(self, instance, value): """Override `__set__` to coordinate between relation field and its shadow attribute""" if value: if isinstance(self.to_cls, str): - self.to_cls = self._fetch_to_cls_from_registry(self.to_cls) + self.to_cls = utils.fetch_entity_cls_from_registry(self.to_cls) # Refresh attribute name, now that we know `to_cls` Entity and it has been # initialized with `id_field` @@ -214,7 +196,7 @@ def _linked_attribute(self, owner): FIXME Explore converting this method into an attribute, and treating it uniformly at `association` level. """ - return self.via or (inflection.underscore(owner.__name__) + '_id') + return self.via or (utils.inflection.underscore(owner.__name__) + '_id') def _fetch_to_cls_from_registry(self, entity): """Private Method to fetch an Entity class from an entity's name""" diff --git a/src/protean/utils/__init__.py b/src/protean/utils/__init__.py index e69de29b..a7fcfa81 100644 --- a/src/protean/utils/__init__.py +++ b/src/protean/utils/__init__.py @@ -0,0 +1,14 @@ +def fetch_entity_cls_from_registry(entity): + """Util Method to fetch an Entity class from an entity's name""" + # Defensive check to ensure we only process if `to_cls` is a string + if isinstance(entity, str): + from protean.core.repository import repo_factory # FIXME Move to a better placement + + try: + return repo_factory.get_entity(entity) + except AssertionError: + # Entity has not been registered (yet) + # FIXME print a helpful debug message + raise + else: + return entity