From b5ec05106a75376392670180f17cc9c58fa3ec8e Mon Sep 17 00:00:00 2001 From: kyleknap Date: Fri, 4 Sep 2015 15:50:37 -0700 Subject: [PATCH 1/3] Make resource identifiers immutable --- boto3/resources/base.py | 4 ++-- boto3/resources/factory.py | 13 ++++++++++++- tests/unit/resources/test_factory.py | 7 +++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/boto3/resources/base.py b/boto3/resources/base.py index f1d9b77acd..b3b902594f 100644 --- a/boto3/resources/base.py +++ b/boto3/resources/base.py @@ -99,7 +99,7 @@ def __init__(self, *args, **kwargs): # Allow setting identifiers as positional arguments in the order # in which they were defined in the ResourceJSON. for i, value in enumerate(args): - setattr(self, self.meta.identifiers[i], value) + setattr(self, '_' + self.meta.identifiers[i], value) # Allow setting identifiers via keyword arguments. Here we need # extra logic to ignore other keyword arguments like ``client``. @@ -110,7 +110,7 @@ def __init__(self, *args, **kwargs): if name not in self.meta.identifiers: raise ValueError('Unknown keyword argument: {0}'.format(name)) - setattr(self, name, value) + setattr(self, '_' + name, value) # Validate that all identifiers have been set. for identifier in self.meta.identifiers: diff --git a/boto3/resources/factory.py b/boto3/resources/factory.py index 198e78d34e..be24a14baa 100644 --- a/boto3/resources/factory.py +++ b/boto3/resources/factory.py @@ -112,7 +112,7 @@ def _load_identifiers(self, attrs, meta, model): """ for identifier in model.identifiers: meta.identifiers.append(identifier.name) - attrs[identifier.name] = None + attrs[identifier.name] = self._create_identifier(identifier.name) def _load_actions(self, attrs, model, resource_defs, service_model): """ @@ -194,6 +194,17 @@ def _load_waiters(self, attrs, model): for waiter in model.waiters: attrs[waiter.name] = self._create_waiter(waiter) + def _create_identifier(factory_self, name): + """ + Creates a read-only property for identifier attributes. + """ + def identifier(self): + return getattr(self, '_' + name, None) + + identifier.__name__ = str(name) + identifier.__doc__ = 'TODO' + return property(identifier) + def _create_autoload_property(factory_self, name, snake_cased): """ Creates a new property on the resource to lazy-load its value diff --git a/tests/unit/resources/test_factory.py b/tests/unit/resources/test_factory.py index 2d9578f18f..5df0585f2e 100644 --- a/tests/unit/resources/test_factory.py +++ b/tests/unit/resources/test_factory.py @@ -648,6 +648,13 @@ def test_dangling_resource_raises_for_unknown_arg(self): with self.assertRaises(ValueError): resource.Queue(url='foo', bar='baz') + def test_dangling_resource_identifier_is_immutable(self): + resource = self.load('test', 'test', self.model, self.defs, None)() + queue = resource.Queue('url') + # We should not be able to change the identifier's value + with self.assertRaises(AttributeError): + queue.url = 'foo' + def test_dangling_resource_equality(self): resource = self.load('test', 'test', self.model, self.defs, None)() From d17938fc4820f535c121ecc34324f6d952941be9 Mon Sep 17 00:00:00 2001 From: kyleknap Date: Fri, 4 Sep 2015 15:59:28 -0700 Subject: [PATCH 2/3] Update CHANGELOG --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d1c775c7e8..aa74844268 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,13 @@ Changelog ========= +Next Release - (TBD) +-------------------- + +* bugfix:Identifier: Make resource identifiers immutable. + (`issue 246 `__) + + 1.1.3 - 2015-09-03 ------------------ From 58acb9eae42e5227ff69ad8adc4985d18497d4a0 Mon Sep 17 00:00:00 2001 From: kyleknap Date: Fri, 11 Sep 2015 13:35:10 -0700 Subject: [PATCH 3/3] Add a comment about the identifier getter --- boto3/resources/factory.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/boto3/resources/factory.py b/boto3/resources/factory.py index be24a14baa..57b2294025 100644 --- a/boto3/resources/factory.py +++ b/boto3/resources/factory.py @@ -199,6 +199,12 @@ def _create_identifier(factory_self, name): Creates a read-only property for identifier attributes. """ def identifier(self): + # The default value is set to ``None`` instead of + # raising an AttributeError because when resources are + # instantiated a check is made such that none of the + # identifiers have a value ``None``. If any are ``None``, + # a more informative user error than a generic AttributeError + # is raised. return getattr(self, '_' + name, None) identifier.__name__ = str(name)