From 9eaa7868164a7e98792de24d2be97f79fba22322 Mon Sep 17 00:00:00 2001
From: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>
Date: Thu, 25 Mar 2021 05:50:03 -0600
Subject: [PATCH] fix: skip empty policy bindings in `len()` and `iter()`
 (#159)

Exclude empty policy bindings (bindings with no members) in
`Policy.__iter__()` and `Policy.__len__()`

Follow up to #155
---
 google/api_core/iam.py | 10 +++++++---
 tests/unit/test_iam.py | 10 +++++-----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/google/api_core/iam.py b/google/api_core/iam.py
index d83cbf35..c498c685 100644
--- 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
index f9771f0f..0da9b231 100644
--- 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