fix: skip empty policy bindings in len() and iter() (#159)

busunkim96 · web-flow · commit 9eaa7868164a · 2021-03-25T11:50:03.000Z
Exclude empty policy bindings (bindings with no members) in `Policy.__iter__()` and `Policy.__len__()` Follow up to #155
diff --git a/google/api_core/iam.py b/google/api_core/iam.py
@@ -125,18 +125,22 @@ def __init__(self, etag=None, version=None):
 
     def __iter__(self):
         self.__check_version__()
-        return (binding["role"] for binding in self._bindings)
+        # Exclude bindings with no members
+        return (binding["role"] for binding in self._bindings if binding["members"])
 
     def __len__(self):
         self.__check_version__()
-        return len(self._bindings)
+        # Exclude bindings with no members
+        return len(list(self.__iter__()))
 
     def __getitem__(self, key):
         self.__check_version__()
         for b in self._bindings:
             if b["role"] == key:
                 return b["members"]
-        # binding does not yet exist, create one
+        # If the binding does not yet exist, create one
+        # NOTE: This will create bindings with no members
+        # which are ignored by __iter__ and __len__
         new_binding = {"role": key, "members": set()}
         self._bindings.append(new_binding)
         return new_binding["members"]
diff --git a/tests/unit/test_iam.py b/tests/unit/test_iam.py
@@ -32,11 +32,11 @@ def test_ctor_defaults(self):
         policy = self._make_one()
         assert policy.etag is None
         assert policy.version is None
-        assert len(policy) == 0
-        assert dict(policy) == {}
         assert policy.owners == empty
         assert policy.editors == empty
         assert policy.viewers == empty
+        assert len(policy) == 0
+        assert dict(policy) == {}
 
     def test_ctor_explicit(self):
         VERSION = 1
@@ -45,11 +45,11 @@ def test_ctor_explicit(self):
         policy = self._make_one(ETAG, VERSION)
         assert policy.etag == ETAG
         assert policy.version == VERSION
-        assert len(policy) == 0
-        assert dict(policy) == {}
         assert policy.owners == empty
         assert policy.editors == empty
         assert policy.viewers == empty
+        assert len(policy) == 0
+        assert dict(policy) == {}
 
     def test___getitem___miss(self):
         policy = self._make_one()
@@ -301,10 +301,10 @@ def test_from_api_repr_only_etag(self):
         policy = klass.from_api_repr(RESOURCE)
         assert policy.etag == "ACAB"
         assert policy.version is None
-        assert dict(policy) == {}
         assert policy.owners == empty
         assert policy.editors == empty
         assert policy.viewers == empty
+        assert dict(policy) == {}
 
     def test_from_api_repr_complete(self):
         from google.api_core.iam import OWNER_ROLE, EDITOR_ROLE, VIEWER_ROLE
