Simplify instance concretization & add tests.

objectrocket/acls.py

@@ -104,7 +104,7 @@ def _concrete_acl(self, acl_doc):
         try:
             return Acl(document=acl_doc, acls=self)
 
-        # If service is not recognized, log a warning and return None.
+        # If construction fails, log the exception and return None.
         except Exception as ex:
             logger.exception(ex)
             logger.error('Could not instantiate ACL document. You probably need to upgrade to a '

objectrocket/instances/__init__.py

@@ -102,23 +102,21 @@ def _concrete_instance(self, instance_doc):
         if not isinstance(instance_doc, dict):
             return None
 
-        service = instance_doc.setdefault('service', 'unknown')
-        inst = None
-
-        # If service key is a recognized service type, instantiate its respective instance.
-        if service in self._service_class_map:
+        # Attempt to instantiate the appropriate class for the given instance document.
+        try:
+            service = instance_doc['service']
             cls = self._service_class_map[service]
-            inst = cls(instance_document=instance_doc, instances=self)
+            return cls(instance_document=instance_doc, instances=self)
 
-        # If service is not recognized, log a warning and return None.
-        else:
-            logger.warning(
-                'Could not determine instance service. You probably need to upgrade to a more '
+        # If construction fails, log the exception and return None.
+        except Exception as ex:
+            logger.exception(ex)
+            logger.error(
+                'Instance construction failed. You probably need to upgrade to a more '
                 'recent version of the client. Instance document which generated this '
                 'warning: {}'.format(instance_doc)
             )
-
-        return inst
+            return None
 
     def _concrete_instance_list(self, instance_docs):
         """Concretize a list of instance documents.
@@ -130,7 +128,7 @@ def _concrete_instance_list(self, instance_docs):
         if not instance_docs:
             return []
 
-        return filter(None, [self._concrete_instance(instance_doc=doc) for doc in instance_docs])
+        return list(filter(None, [self._concrete_instance(instance_doc=doc) for doc in instance_docs]))
 
     @property
     def _default_request_kwargs(self):

tests/test_instances.py

@@ -0,0 +1,40 @@
+"""Tests for the objectrocket.instances module."""
+from objectrocket.instances.mongodb import MongodbInstance
+
+
+##########################################
+# Tests for Instances private interface. #
+##########################################
+def test_concrete_instance_returns_none_if_dict_not_given(client):
+    output = client.instances._concrete_instance([])
+    assert output is None
+
+
+def test_concrete_instance_returns_none_if_doc_is_missing_needed_field(client, mongodb_sharded_doc):
+    mongodb_sharded_doc.pop('name')  # Will cause a constructor error.
+    output = client.instances._concrete_instance(mongodb_sharded_doc)
+    assert output is None
+
+
+def test_concrete_instance_returns_expected_instance_object(client, mongodb_sharded_doc):
+    output = client.instances._concrete_instance(mongodb_sharded_doc)
+    assert isinstance(output, MongodbInstance)
+    assert output._instance_document is mongodb_sharded_doc
+
+
+def test_concrete_instance_list_returns_empty_list_if_empty_list_provided(client):
+    output = client.instances._concrete_instance_list([])
+    assert output == []
+
+
+def test_concrete_instance_list_returns_empty_list_if_constructor_error_for_doc(client, mongodb_sharded_doc):
+    mongodb_sharded_doc.pop('name')  # Will cause a constructor error.
+    output = client.instances._concrete_instance_list([mongodb_sharded_doc])
+    assert output == []
+
+
+def test_concrete_instance_list_returns_expected_list(client, mongodb_sharded_doc):
+    output = client.instances._concrete_instance_list([mongodb_sharded_doc])
+    assert isinstance(output, list)
+    assert len(output) == 1
+    assert output[0]._instance_document is mongodb_sharded_doc