Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix conditional retry handling of camelCase query params #340

Merged
merged 1 commit into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)