Add system tests for storage retention policy (#450)

storage/google/cloud/storage/bucket.py

@@ -1346,7 +1346,9 @@ def retention_period(self, value):
         :raises ValueError: if the bucket's retention policy is locked.
         """
         policy = self._properties.setdefault('retentionPolicy', {})
-        policy['retentionPeriod'] = str(value)
+        if value is not None:
+            value = str(value)
+        policy['retentionPeriod'] = value
         self._patch_property('retentionPolicy', policy)
 
     @property
@@ -1805,7 +1807,7 @@ def lock_retention_policy(self, client=None):
 
         client = self._require_client(client)
 
-        query_params = {'metageneration': self.metageneration}
+        query_params = {'ifMetagenerationMatch': self.metageneration}
 
         if self.user_project is not None:
             query_params['userProject'] = self.user_project

storage/tests/system.py

@@ -1240,3 +1240,176 @@ def test_rewrite_rotate_csek_to_cmek(self):
         self.assertEqual(total, len(source_data))
 
         self.assertEqual(dest.download_as_string(), source_data)
+
+
+class TestRetentionPolicy(unittest.TestCase):
+
+    def setUp(self):
+        self.case_buckets_to_delete = []
+
+    def tearDown(self):
+        for bucket_name in self.case_buckets_to_delete:
+            bucket = Config.CLIENT.bucket(bucket_name)
+            retry_429(bucket.delete)()
+
+    def test_bucket_w_retention_period(self):
+        import datetime
+        from google.api_core import exceptions
+
+        period_secs = 10
+
+        new_bucket_name = 'w-retention-period' + unique_resource_id('-')
+        bucket = Config.CLIENT.create_bucket(new_bucket_name)
+        self.case_buckets_to_delete.append(new_bucket_name)
+
+        bucket.retention_period = period_secs
+        bucket.default_event_based_hold = False
+        bucket.patch()
+
+        self.assertEqual(bucket.retention_period, period_secs)
+        self.assertIsInstance(
+            bucket.retention_policy_effective_time, datetime.datetime)
+        self.assertFalse(bucket.default_event_based_hold)
+        self.assertFalse(bucket.retention_policy_locked)
+
+        blob_name = 'test-blob'
+        payload = b'DEADBEEF'
+        blob = bucket.blob(blob_name)
+        blob.upload_from_string(payload)
+
+        other = bucket.get_blob(blob_name)
+
+        self.assertFalse(other.event_based_hold)
+        self.assertFalse(other.temporary_hold)
+        self.assertIsInstance(
+            other.retention_expiration_time, datetime.datetime)
+
+        with self.assertRaises(exceptions.Forbidden):
+            other.delete()
+
+        bucket.retention_period = None
+        bucket.patch()
+
+        self.assertIsNone(bucket.retention_period)
+        self.assertIsNone(bucket.retention_policy_effective_time)
+        self.assertFalse(bucket.default_event_based_hold)
+        self.assertFalse(bucket.retention_policy_locked)
+
+        other.reload()
+
+        self.assertFalse(other.event_based_hold)
+        self.assertFalse(other.temporary_hold)
+        self.assertIsNone(other.retention_expiration_time)
+
+        other.delete()
+
+    def test_bucket_w_default_event_based_hold(self):
+        from google.api_core import exceptions
+
+        new_bucket_name = 'w-def-ebh' + unique_resource_id('-')
+        self.assertRaises(exceptions.NotFound,
+                          Config.CLIENT.get_bucket, new_bucket_name)
+        bucket = Config.CLIENT.create_bucket(new_bucket_name)
+        self.case_buckets_to_delete.append(new_bucket_name)
+
+        bucket.default_event_based_hold = True
+        bucket.patch()
+
+        self.assertTrue(bucket.default_event_based_hold)
+        self.assertIsNone(bucket.retention_period)
+        self.assertIsNone(bucket.retention_policy_effective_time)
+        self.assertFalse(bucket.retention_policy_locked)
+
+        blob_name = 'test-blob'
+        payload = b'DEADBEEF'
+        blob = bucket.blob(blob_name)
+        blob.upload_from_string(payload)
+
+        other = bucket.get_blob(blob_name)
+
+        self.assertTrue(other.event_based_hold)
+        self.assertFalse(other.temporary_hold)
+        self.assertIsNone(other.retention_expiration_time)
+
+        with self.assertRaises(exceptions.Forbidden):
+            other.delete()
+
+        other.event_based_hold = False
+        other.patch()
+
+        other.delete()
+
+        bucket.default_event_based_hold = False
+        bucket.patch()
+
+        self.assertFalse(bucket.default_event_based_hold)
+        self.assertIsNone(bucket.retention_period)
+        self.assertIsNone(bucket.retention_policy_effective_time)
+        self.assertFalse(bucket.retention_policy_locked)
+
+        blob.upload_from_string(payload)
+        self.assertFalse(other.event_based_hold)
+        self.assertFalse(other.temporary_hold)
+        self.assertIsNone(other.retention_expiration_time)
+
+        blob.delete()
+
+    def test_blob_w_temporary_hold(self):
+        from google.api_core import exceptions
+
+        new_bucket_name = 'w-tmp-hold' + unique_resource_id('-')
+        self.assertRaises(exceptions.NotFound,
+                          Config.CLIENT.get_bucket, new_bucket_name)
+        bucket = Config.CLIENT.create_bucket(new_bucket_name)
+        self.case_buckets_to_delete.append(new_bucket_name)
+
+        blob_name = 'test-blob'
+        payload = b'DEADBEEF'
+        blob = bucket.blob(blob_name)
+        blob.upload_from_string(payload)
+
+        other = bucket.get_blob(blob_name)
+        other.temporary_hold = True
+        other.patch()
+
+        self.assertTrue(other.temporary_hold)
+        self.assertFalse(other.event_based_hold)
+        self.assertIsNone(other.retention_expiration_time)
+
+        with self.assertRaises(exceptions.Forbidden):
+            other.delete()
+
+        other.temporary_hold = False
+        other.patch()
+
+        other.delete()
+
+    def test_bucket_lock_retention_policy(self):
+        import datetime
+        from google.api_core import exceptions
+
+        period_secs = 10
+
+        new_bucket_name = 'loc-ret-policy' + unique_resource_id('-')
+        self.assertRaises(exceptions.NotFound,
+                          Config.CLIENT.get_bucket, new_bucket_name)
+        bucket = Config.CLIENT.create_bucket(new_bucket_name)
+        self.case_buckets_to_delete.append(new_bucket_name)
+
+        bucket.retention_period = period_secs
+        bucket.patch()
+
+        self.assertEqual(bucket.retention_period, period_secs)
+        self.assertIsInstance(
+            bucket.retention_policy_effective_time, datetime.datetime)
+        self.assertFalse(bucket.default_event_based_hold)
+        self.assertFalse(bucket.retention_policy_locked)
+
+        bucket.lock_retention_policy()
+
+        bucket.reload()
+        self.assertTrue(bucket.retention_policy_locked)
+
+        bucket.retention_period = None
+        with self.assertRaises(exceptions.Forbidden):
+            bucket.patch()

storage/tests/unit/test_bucket.py

@@ -1570,7 +1570,18 @@ def test_retention_period_getter(self):
 
         self.assertEqual(bucket.retention_period, period)
 
-    def test_retention_period_setter(self):
+    def test_retention_period_setter_w_none(self):
+        period = 86400 * 100  # 100 days
+        bucket = self._make_one()
+        policy = bucket._properties['retentionPolicy'] = {}
+        policy['retentionPeriod'] = period
+
+        bucket.retention_period = None
+
+        self.assertIsNone(
+            bucket._properties['retentionPolicy']['retentionPeriod'])
+
+    def test_retention_period_setter_w_int(self):
         period = 86400 * 100  # 100 days
         bucket = self._make_one()
 
@@ -2485,7 +2496,7 @@ def test_lock_retention_policy_ok(self):
         kw, = connection._requested
         self.assertEqual(kw['method'], 'POST')
         self.assertEqual(kw['path'], '/b/{}/lockRetentionPolicy'.format(name))
-        self.assertEqual(kw['query_params'], {'metageneration': 1234})
+        self.assertEqual(kw['query_params'], {'ifMetagenerationMatch': 1234})
 
     def test_lock_retention_policy_w_user_project(self):
         name = 'name'
@@ -2518,7 +2529,7 @@ def test_lock_retention_policy_w_user_project(self):
         self.assertEqual(kw['path'], '/b/{}/lockRetentionPolicy'.format(name))
         self.assertEqual(
             kw['query_params'], {
-                'metageneration': 1234,
+                'ifMetagenerationMatch': 1234,
                 'userProject': user_project,
             })