Fixed get and find methods in SecurityGroupService to return an object

and list respectively. Closes issue:
https://github.com/gvlproject/cloudbridge/issues/5

cloudbridge/cloud/interfaces/services.py

@@ -698,6 +698,24 @@ class SecurityGroupService(PageableObjectMixin, ProviderService):
     """
     __metaclass__ = ABCMeta
 
+    @abstractmethod
+    def get(self, security_group_id):
+        """
+        Returns a SecurityGroup given its ID. Returns ``None`` if the
+        SecurityGroup does not exist.
+
+        Example:
+
+        .. code-block:: python
+
+            sg = provider.security.security_groups.get('my_sg_id')
+            print(sg.id, sg.name)
+
+        :rtype: :class:`.SecurityGroup`
+        :return:  a SecurityGroup instance
+        """
+        pass
+
     @abstractmethod
     def list(self, limit=None, marker=None):
         """
@@ -725,19 +743,12 @@ class SecurityGroupService(PageableObjectMixin, ProviderService):
         pass
 
     @abstractmethod
-    def get(self, group_names=None, group_ids=None):
+    def find(self, name, limit=None, marker=None):
         """
         Get all security groups associated with your account.
 
-        :type group_names: list
-        :param group_names: A list of the names of security groups to retrieve.
-                           If not provided, all security groups will be
-                           returned.
-
-        :type group_ids: list
-        :param group_ids: A list of IDs of security groups to retrieve.
-                          If not provided, all security groups will be
-                          returned.
+        :type name: str
+        :param name: The name of the security group to retrieve.
 
         :rtype: list of :class:`SecurityGroup`
         :return: A list of SecurityGroup objects or an empty list if none

cloudbridge/cloud/providers/aws/resources.py

@@ -287,8 +287,8 @@ class AWSInstance(BaseInstance):
         # boto instance.groups field returns a ``Group`` object so need to
         # convert that into a ``SecurityGroup`` object before creating a
         # cloudbridge SecurityGroup object
-        names = [group.name for group in self._ec2_instance.groups]
-        return self._provider.security.security_groups.get(names)
+        return [self._provider.security.security_groups.get(group.id)
+                for group in self._ec2_instance.groups]
 
     @property
     def key_pair_name(self):

cloudbridge/cloud/providers/aws/services.py

@@ -137,6 +137,17 @@ class AWSSecurityGroupService(BaseSecurityGroupService):
     def __init__(self, provider):
         super(AWSSecurityGroupService, self).__init__(provider)
 
+    def get(self, sg_id):
+        """
+        Returns a SecurityGroup given its id.
+        """
+        try:
+            sgs = self.provider.ec2_conn.get_all_security_groups(
+                group_ids=[sg_id])
+            return AWSSecurityGroup(self.provider, sgs[0]) if sgs else None
+        except EC2ResponseError:
+            return None
+
     def list(self, limit=None, marker=None):
         """
         List all security groups associated with this account.
@@ -168,27 +179,13 @@ class AWSSecurityGroupService(BaseSecurityGroupService):
             return AWSSecurityGroup(self.provider, sg)
         return None
 
-    def get(self, group_names=None, group_ids=None):
+    def find(self, name, limit=None, marker=None):
         """
         Get all security groups associated with your account.
-
-        :type group_names: list
-        :param group_names: A list of the names of security groups to retrieve.
-                           If not provided, all security groups will be
-                           returned.
-
-        :type group_ids: list
-        :param group_ids: A list of IDs of security groups to retrieve.
-                          If not provided, all security groups will be
-                          returned.
-
-        :rtype: list of :class:`SecurityGroup`
-        :return: A list of SecurityGroup objects or an empty list if none
-        found.
         """
         try:
             security_groups = self.provider.ec2_conn.get_all_security_groups(
-                groupnames=group_names, group_ids=group_ids)
+                groupnames=[name])
         except EC2ResponseError:
             security_groups = []
         return [AWSSecurityGroup(self.provider, sg) for sg in security_groups]
@@ -299,7 +296,7 @@ class AWSSnapshotService(BaseSnapshotService):
 
     def find(self, name, limit=None, marker=None):
         """
-        Searches for a volume by a given list of attributes.
+        Searches for a snapshot by a given list of attributes.
         """
         filtr = {'Name': name}
         snaps = [AWSSnapshot(self.provider, snap) for snap in

cloudbridge/cloud/providers/openstack/resources.py

@@ -299,9 +299,8 @@ class OpenStackInstance(BaseInstance):
         """
         Get the security groups associated with this instance.
         """
-        names = [group.get('name')
-                 for group in self._os_instance.security_groups]
-        return self._provider.security.security_groups.get(names)
+        return [self._provider.security.security_groups.find(group['name'])[0]
+                for group in self._os_instance.security_groups]
 
     @property
     def key_pair_name(self):

cloudbridge/cloud/providers/openstack/services.py

@@ -133,6 +133,16 @@ class OpenStackSecurityGroupService(BaseSecurityGroupService):
     def __init__(self, provider):
         super(OpenStackSecurityGroupService, self).__init__(provider)
 
+    def get(self, sg_id):
+        """
+        Returns a SecurityGroup given its id.
+        """
+        try:
+            return OpenStackSecurityGroup(
+                self.provider, self.provider.nova.security_groups.get(sg_id))
+        except NovaNotFound:
+            return None
+
     def list(self, limit=None, marker=None):
         """
         List all security groups associated with this account.
@@ -165,39 +175,15 @@ class OpenStackSecurityGroupService(BaseSecurityGroupService):
             return OpenStackSecurityGroup(self.provider, sg)
         return None
 
-    def get(self, group_names=None, group_ids=None):
+    def find(self, name, limit=None, marker=None):
         """
         Get all security groups associated with your account.
-
-        :type group_names: list
-        :param group_names: A list of strings of the names of security groups
-                           to retrieve. If not provided, all security groups
-                           will be returned.
-
-        :type group_ids: list
-        :param group_ids: A list of string IDs of security groups to retrieve.
-                          If not provided, all security groups will be
-                          returned.
-
-        :rtype: list of :class:`SecurityGroup`
-        :return: A list of SecurityGroup objects or an empty list if none
-        found.
-        """
-        if not group_names:
-            group_names = []
-        if not group_ids:
-            group_ids = []
-        security_groups = self.provider.nova.security_groups.list()
-        filtered = []
-        for sg in security_groups:
-            if sg.name in group_names:
-                filtered.append(sg)
-            if sg.id in group_ids:
-                filtered.append(sg)
-        # If a filter was specified, use the filtered list; otherwise, get all
-        return [OpenStackSecurityGroup(self.provider, sg)
-                for sg in (filtered
-                           if (group_names or group_ids) else security_groups)]
+        """
+        sgs = self.provider.nova.security_groups.findall(name=name)
+        results = [OpenStackSecurityGroup(self.provider, sg)
+                   for sg in sgs]
+        return ClientPagedResultList(self.provider, results,
+                                     limit=limit, marker=marker)
 
     def delete(self, group_id):
         """
@@ -212,9 +198,9 @@ class OpenStackSecurityGroupService(BaseSecurityGroupService):
                   been deleted by this method but instead has not existed in
                   the first place.
         """
-        sg = self.get(group_ids=[group_id])
+        sg = self.get(group_id)
         if sg:
-            sg[0].delete()
+            sg.delete()
         return True
 
 

test/test_block_store_service.py

@@ -160,7 +160,7 @@ class CloudBlockStoreServiceTestCase(ProviderTestBase):
                 find_snap = self.provider.block_store.snapshots.find(name=name)
                 self.assertTrue(
                     len(find_snap) == 1,
-                    "Find snaps does not return the expected volume %s" %
+                    "Find snaps does not return the expected snapshot %s" %
                     name)
 
                 get_snap = self.provider.block_store.snapshots.get(

test/test_security_service.py

@@ -116,14 +116,20 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
                 "Iter security groups does not return the expected group %s" %
                 name)
 
-            sgl = self.provider.security.security_groups.get(
-                group_names=[
-                    sg.name])
-            found_sg = [g for g in sgl if g.name == name]
+            # check find
+            find_sg = self.provider.security.security_groups.find(name=sg.name)
             self.assertTrue(
-                len(found_sg) == 1,
-                "List security groups did not return the expected group {0}."
+                len(find_sg) == 1,
+                "List security groups returned {0} when expected was: {1}."
+                .format(find_sg, sg.name))
+
+            # check get
+            get_sg = self.provider.security.security_groups.get(sg.id)
+            self.assertTrue(
+                get_sg == sg,
+                "Get SecurityGroup did not return the expected key {0}."
                 .format(name))
+
             self.assertTrue(
                 sg.id in repr(sg),
                 "repr(obj) should contain the object id so that the object"
@@ -134,8 +140,7 @@ class CloudSecurityServiceTestCase(ProviderTestBase):
             len(found_sg) == 0,
             "Security group {0} should have been deleted but still exists."
             .format(name))
-        no_sg = self.provider.security.security_groups.get(
-            group_ids=['bogus_sg'])
+        no_sg = self.provider.security.security_groups.find(name='bogus_sg')
         self.assertTrue(
             len(no_sg) == 0,
             "Found a bogus security group?!?".format(no_sg))