From 4ff614161f6a2654a59706f4f72b5fbb614e70ec Mon Sep 17 00:00:00 2001 From: Andrew Gorcester Date: Thu, 10 Dec 2020 16:17:57 -0800 Subject: [PATCH] fix: fix conditional retry handling of camelCase query params (#340) --- google/cloud/storage/retry.py | 4 ++-- tests/unit/test_retry.py | 38 +++++++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/google/cloud/storage/retry.py b/google/cloud/storage/retry.py index c1f1ad10d..e9a9eeb2f 100644 --- a/google/cloud/storage/retry.py +++ b/google/cloud/storage/retry.py @@ -99,13 +99,13 @@ def get_retry_policy_if_conditions_met(self, **kwargs): def is_generation_specified(query_params): """Return True if generation or if_generation_match is specified.""" generation = query_params.get("generation") is not None - if_generation_match = query_params.get("if_generation_match") is not None + if_generation_match = query_params.get("ifGenerationMatch") is not None return generation or if_generation_match def is_metageneration_specified(query_params): """Return True if if_metageneration_match is specified.""" - if_metageneration_match = query_params.get("if_metageneration_match") is not None + if_metageneration_match = query_params.get("ifMetagenerationMatch") is not None return if_metageneration_match diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 06780362e..d9a773cf9 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -14,6 +14,8 @@ import unittest +from google.cloud.storage import _helpers + import mock @@ -112,7 +114,7 @@ def test_w_generation(self): self.assertTrue(self._call_fut(query_params)) def test_wo_generation_w_if_generation_match(self): - query_params = {"if_generation_match": 123} + query_params = {"ifGenerationMatch": 123} self.assertTrue(self._call_fut(query_params)) @@ -129,7 +131,7 @@ def test_w_empty(self): self.assertFalse(self._call_fut(query_params)) def test_w_if_metageneration_match(self): - query_params = {"if_metageneration_match": 123} + query_params = {"ifMetagenerationMatch": 123} self.assertTrue(self._call_fut(query_params)) @@ -163,48 +165,62 @@ def test_w_empty_data(self): class Test_default_conditional_retry_policies(unittest.TestCase): - def test_is_generation_specified_match_metageneration(self): + def test_is_generation_specified_match_generation_match(self): from google.cloud.storage import retry + query_dict = {} + _helpers._add_generation_match_parameters(query_dict, if_generation_match=1) + conditional_policy = retry.DEFAULT_RETRY_IF_GENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_generation_match": 1} + query_params=query_dict ) self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_generation_specified_match_generation(self): from google.cloud.storage import retry + query_dict = {"generation": 1} + conditional_policy = retry.DEFAULT_RETRY_IF_GENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"generation": 1} + query_params=query_dict ) self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_generation_specified_mismatch(self): from google.cloud.storage import retry + query_dict = {} + _helpers._add_generation_match_parameters(query_dict, if_metageneration_match=1) + conditional_policy = retry.DEFAULT_RETRY_IF_GENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_metageneration_match": 1} + query_params=query_dict ) self.assertEqual(policy, None) def test_is_metageneration_specified_match(self): from google.cloud.storage import retry + query_dict = {} + _helpers._add_generation_match_parameters(query_dict, if_metageneration_match=1) + conditional_policy = retry.DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_metageneration_match": 1} + query_params=query_dict ) self.assertEqual(policy, retry.DEFAULT_RETRY) def test_is_metageneration_specified_mismatch(self): from google.cloud.storage import retry + query_dict = {} + _helpers._add_generation_match_parameters(query_dict, if_generation_match=1) + conditional_policy = retry.DEFAULT_RETRY_IF_METAGENERATION_SPECIFIED policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_generation_match": 1} + query_params=query_dict ) self.assertEqual(policy, None) @@ -213,7 +229,7 @@ def test_is_etag_in_json_etag_match(self): conditional_policy = retry.DEFAULT_RETRY_IF_ETAG_IN_JSON policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_generation_match": 1}, data='{"etag": "12345678"}' + query_params={"ifGenerationMatch": 1}, data='{"etag": "12345678"}' ) self.assertEqual(policy, retry.DEFAULT_RETRY) @@ -222,7 +238,7 @@ def test_is_etag_in_json_mismatch(self): conditional_policy = retry.DEFAULT_RETRY_IF_ETAG_IN_JSON policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_generation_match": 1}, data="{}" + query_params={"ifGenerationMatch": 1}, data="{}" ) self.assertEqual(policy, None) @@ -231,6 +247,6 @@ def test_is_meta_or_etag_in_json_invalid(self): conditional_policy = retry.DEFAULT_RETRY_IF_ETAG_IN_JSON policy = conditional_policy.get_retry_policy_if_conditions_met( - query_params={"if_generation_match": 1}, data="I am invalid JSON!" + query_params={"ifGenerationMatch": 1}, data="I am invalid JSON!" ) self.assertEqual(policy, None)