Switch preserve_acl default to True.

daspecster · daspecster · commit 8e85462e35a6 · 2016-09-25T21:29:33.000-04:00
diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py
@@ -440,7 +440,7 @@ def delete_blobs(self, blobs, on_error=None, client=None):
                     raise
 
     def copy_blob(self, blob, destination_bucket, new_name=None,
-                  client=None, preserve_acl=False):
+                  client=None, preserve_acl=True):
         """Copy the given blob to the given bucket, optionally with a new name.
 
         :type blob: :class:`google.cloud.storage.blob.Blob`
@@ -460,7 +460,7 @@ def copy_blob(self, blob, destination_bucket, new_name=None,
 
         :type preserve_acl: bool
         :param preserve_acl: Optional. Copies ACL from old blob to new blob.
-                             Default: False.
+                             Default: True.
 
         :rtype: :class:`google.cloud.storage.blob.Blob`
         :returns: The new Blob.
@@ -470,14 +470,15 @@ def copy_blob(self, blob, destination_bucket, new_name=None,
             new_name = blob.name
         new_blob = Blob(bucket=destination_bucket, name=new_name)
         api_path = blob.path + '/copyTo' + new_blob.path
+        if not preserve_acl:
+            new_blob.acl.reset()
+            new_blob.acl.save()
         copy_result = client.connection.api_request(
             method='POST', path=api_path, _target_object=new_blob)
         new_blob._set_properties(copy_result)
-        if preserve_acl:
-            new_blob.acl.save(blob.acl)
         return new_blob
 
-    def rename_blob(self, blob, new_name, client=None, preserve_acl=False):
+    def rename_blob(self, blob, new_name, client=None):
         """Rename the given blob using copy and delete operations.
 
         Effectively, copies blob to the same bucket with a new name, then
@@ -500,15 +501,10 @@ def rename_blob(self, blob, new_name, client=None, preserve_acl=False):
         :param client: Optional. The client to use.  If not passed, falls back
                        to the ``client`` stored on the current bucket.
 
-        :type preserve_acl: bool
-        :param preserve_acl: Optional. Copies ACL from old blob to renamed
-                             blob. Default: False.
-
         :rtype: :class:`Blob`
         :returns: The newly-renamed blob.
         """
-        new_blob = self.copy_blob(blob, self, new_name, client=client,
-                                  preserve_acl=preserve_acl)
+        new_blob = self.copy_blob(blob, self, new_name, client=client)
         blob.delete(client=client)
         return new_blob
 
diff --git a/unit_tests/storage/test_bucket.py b/unit_tests/storage/test_bucket.py
@@ -543,7 +543,7 @@ def test_copy_blobs_preserve_acl(self):
         BLOB_NAME = 'blob-name'
         NEW_NAME = 'new_name'
 
-        def save(self, client):  # pylint: disable=unused-argument
+        def save(self):  # pylint: disable=unused-argument
             return
 
         class _Blob(object):
@@ -552,6 +552,8 @@ class _Blob(object):
 
             def __init__(self):
                 self.acl = ACL()
+                self.acl.loaded = True
+                self.acl.entity('type', 'id')
 
         connection = _Connection({})
         client = _Client(connection)
@@ -560,10 +562,12 @@ def __init__(self):
         blob = _Blob()
         with _Monkey(ACL, save=save):
             new_blob = source.copy_blob(blob, dest, NEW_NAME, client=client,
-                                        preserve_acl=True)
+                                        preserve_acl=False)
             self.assertIs(new_blob.bucket, dest)
             self.assertEqual(new_blob.name, NEW_NAME)
             self.assertIsInstance(new_blob.acl, ObjectACL)
+            self.assertFalse(new_blob.acl.loaded)
+            self.assertEqual(new_blob.acl.entities, {})
 
     def test_copy_blobs_w_name(self):
         SOURCE = 'source'
