Skip to content

Commit

Permalink
fix: fix conditional retry handling of camelCase query params (#340)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewsg authored Dec 11, 2020
1 parent 910e34c commit 4ff6141
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
4 changes: 2 additions & 2 deletions google/cloud/storage/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
38 changes: 27 additions & 11 deletions tests/unit/test_retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

import unittest

from google.cloud.storage import _helpers

import mock


Expand Down Expand Up @@ -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))

Expand All @@ -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))

Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)

0 comments on commit 4ff6141

Please sign in to comment.