Express BPO as scalar properties, rather than a helper object.

tseaver · tseaver · commit 6814a4f480cf · 2019-01-17T13:29:01.000-05:00
diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py
@@ -272,82 +272,6 @@ def from_api_repr(cls, resource):
         return instance
 
 
-class IAMConfiguration(dict):
-    """Map a bucket's IAM configuration.
-
-    :type bucket: :class:`Bucket`
-    :params bucket: Bucket for which this instance is the policy.
-
-    :type enabled: bool
-    :params enabled: (optional) whether the IAM-only policy is enabled for the bucket.
-
-    :type locked_time: :class:`datetime.datetime`
-    :params locked_time: (optional) When the bucket's IAM-only policy was ehabled.  This value should normally only be set by the back-end API.
-    """
-
-    def __init__(self, bucket, enabled=False, locked_time=None):
-        data = {"bucketPolicyOnly": {"enabled": enabled}}
-        if locked_time is not None:
-            data["bucketPolicyOnly"]["lockedTime"] = _datetime_to_rfc3339(locked_time)
-        super(IAMConfiguration, self).__init__(data)
-        self._bucket = bucket
-
-    @classmethod
-    def from_api_repr(cls, resource, bucket):
-        """Factory:  construct instance from resource.
-
-        :type bucket: :class:`Bucket`
-        :params bucket: Bucket for which this instance is the policy.
-
-        :type resource: dict
-        :param resource: mapping as returned from API call.
-
-        :rtype: :class:`IAMConfiguration`
-        :returns: Instance created from resource.
-        """
-        instance = cls(bucket)
-        instance.update(resource)
-        return instance
-
-    @property
-    def bucket(self):
-        """Bucket for which this instance is the policy.
-
-        :rtype: :class:`Bucket`
-        :returns: the instance's bucket.
-        """
-        return self._bucket
-
-    @property
-    def bucket_policy_only(self):
-        """Is the bucket configured to allow only IAM policy?
-
-        :rtype: bool
-        :returns: whether the bucket is configured to allow only IAM.
-        """
-        bpo = self.get("bucketPolicyOnly", {})
-        return bpo.get("enabled", False)
-
-    @bucket_policy_only.setter
-    def bucket_policy_only(self, value):
-        bpo = self.setdefault("bucketPolicyOnly", {})
-        bpo["enabled"] = bool(value)
-        self.bucket._patch_property("iamConfiguration", self)
-
-    @property
-    def locked_time(self):
-        """When was the bucket configured to allow only IAM policy?
-
-        :rtype: Union[:class:`datetime.datetime`, None]
-        :returns:  (readonly) the time the bucket's IAM-only policy was set.
-        """
-        bpo = self.get("bucketPolicyOnly", {})
-        stamp = bpo.get("lockedTime")
-        if stamp is not None:
-            stamp = _rfc3339_to_datetime(stamp)
-        return stamp
-
-
 class Bucket(_PropertyMixin):
     """A class representing a Bucket on Cloud Storage.
 
@@ -1210,16 +1134,6 @@ def id(self):
         """
         return self._properties.get("id")
 
-    @property
-    def iam_configuration(self):
-        """Retrieve IAM configuration for this bucket.
-
-        :rtype: :class:`IAMConfiguration`
-        :returns: an instance for managing the bucket's IAM configuration.
-        """
-        info = self._properties.get("iamConfiguration", {})
-        return IAMConfiguration.from_api_repr(info, self)
-
     @property
     def lifecycle_rules(self):
         """Retrieve or set lifecycle rules configured for this bucket.
@@ -1482,6 +1396,48 @@ def retention_period(self, value):
             policy = None
         self._patch_property("retentionPolicy", policy)
 
+    @property
+    def iam_configuration_bucket_policy_only(self):
+        """Get/set whether the bucket is configured to allow only bucket IAM.
+
+        If set, then ACLs are no longer in effect for the bucket or its
+        blobs.
+
+        :rtype: bool
+        :returns: True if the bucket allows only IAM, else false.
+        """
+        iam_config = self._properties.get("iamConfiguration", {})
+        bpo = iam_config.get("bucketPolicyOnly", {})
+        return bpo.get("enabled")
+
+    @iam_configuration_bucket_policy_only.setter
+    def iam_configuration_bucket_policy_only(self, value):
+        """Set whether the bucket is configured to allow only bucket IAM.
+
+        :type value: bool
+        :param value:
+            If true, the bucket will allow only IAM; if false, then the
+            ACLs for the bucket and its blobs will be effective.
+        """
+        iam_config = self._properties.setdefault("iamConfiguration", {})
+        iam_config["bucketPolicyOnly"] = {"enabled": bool(value)}
+        self._patch_property("iamConfiguration", iam_config)
+
+    @property
+    def iam_configuration_locked_time(self):
+        """Time when the bucket was configured to allow only bucket IAM.
+
+        :rtype: datetime.datetime or None
+        :returns:
+            point in time when the bucket was configured to allow only IAM.
+        """
+        iam_config = self._properties.get("iamConfiguration", {})
+        bpo = iam_config.get("bucketPolicyOnly", {})
+        stamp = bpo.get('lockedTime')
+        if stamp is not None:
+            stamp = _rfc3339_to_datetime(stamp)
+        return stamp
+
     @property
     def self_link(self):
         """Retrieve the URI for the bucket.
diff --git a/storage/tests/system.py b/storage/tests/system.py
@@ -1487,7 +1487,7 @@ def test_new_bucket_w_bpo(self):
             exceptions.NotFound, Config.CLIENT.get_bucket, new_bucket_name
         )
         bucket = Config.CLIENT.bucket(new_bucket_name)
-        bucket.iam_configuration.bucket_policy_only = True
+        bucket.iam_configuration_bucket_policy_only = True
         retry_429(bucket.create)()
         self.case_buckets_to_delete.append(new_bucket_name)
 
@@ -1536,7 +1536,7 @@ def test_bpo_set_unset_preserves_acls(self):
         blob_acl_before = list(bucket.acl)
 
         # Set BPO
-        bucket.iam_configuration.bucket_policy_only = True
+        bucket.iam_configuration_bucket_policy_only = True
         bucket.patch()
 
         # While BPO is set, cannot get / set ACLs
@@ -1557,7 +1557,7 @@ def test_bpo_set_unset_preserves_acls(self):
             blob.acl.clear()
 
         # Clear BPO
-        bucket.iam_configuration.bucket_policy_only = False
+        bucket.iam_configuration_bucket_policy_only = False
         bucket.patch()
 
         # Query ACLs after clearing BPO
diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py
