diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index e971a8c762cf..ff3c9c1d7e8e 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1343,7 +1343,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 @@ -1802,7 +1804,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 diff --git a/storage/tests/system.py b/storage/tests/system.py index 41abfa1e3d34..3e444f67b8e3 100644 --- a/storage/tests/system.py +++ b/storage/tests/system.py @@ -1239,3 +1239,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() diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 7530c9c4e8f9..bf9a17189885 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -1564,7 +1564,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() @@ -2479,7 +2490,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' @@ -2512,7 +2523,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, })