From b761e2496cae47948d4f544c0c62ef4d7234cffb Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 16 Sep 2021 01:26:24 +0000 Subject: [PATCH 1/7] feat: Add helper function to format query_params for rest transport. --- google/api_core/rest_helpers.py | 76 +++++++++++++++++++++++++++++++++ tests/unit/test_rest_helpers.py | 61 ++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 google/api_core/rest_helpers.py create mode 100644 tests/unit/test_rest_helpers.py diff --git a/google/api_core/rest_helpers.py b/google/api_core/rest_helpers.py new file mode 100644 index 00000000..2bf99f15 --- /dev/null +++ b/google/api_core/rest_helpers.py @@ -0,0 +1,76 @@ +# Copyright 2017 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Helpers for rest transports.""" + +import itertools + + +def flatten_query_params(obj, key_path=[]): + """Flatten a nested dict into a list of (name,value) tuples. + + The result is suitable for setting query params on an http request. + + .. code-block:: python + + >>> obj = {'a': + ... {'b': + ... {'c': ['x', 'y', 'z']} }, + ... 'd': 'uvw', } + >>> flatten_query_params(obj) + [('a.b.c', 'x'), ('a.b.c', 'y'), ('a.b.c', 'z'), ('d', 'uvw')] + + Args: + obj: a nested dictionary (from json) + key_path: a list of name segments, representing levels above this obj. + + Returns: a list of tuples, with each tuple having a (possibly) multi-part name + and a scalar value. + """ + + if obj is None: + return [] + if isinstance(obj, dict): + return _flatten_dict(obj, key_path=key_path) + if isinstance(obj, list): + return _flatten_list(obj, key_path=key_path) + return _flatten_value(obj, key_path=key_path) + + +def _is_value(obj): + if obj is None: + return False + return not (isinstance(obj, list) or isinstance(obj, dict)) + + +def _flatten_value(obj, key_path=[]): + if not key_path: + # There must be a key. + return [] + return [('.'.join(key_path), obj)] + + +def _flatten_dict(obj, key_path=[]): + return list( + itertools.chain(*(flatten_query_params(v, key_path=key_path + [k]) + for k, v in obj.items()))) + + +def _flatten_list(l, key_path=[]): + # Only lists of scalar values are supported. + # The name (key_path) is repeated for each value. + return list( + itertools.chain(*(_flatten_value(elem, key_path=key_path) + for elem in l + if _is_value(elem)))) diff --git a/tests/unit/test_rest_helpers.py b/tests/unit/test_rest_helpers.py new file mode 100644 index 00000000..8bffcbd4 --- /dev/null +++ b/tests/unit/test_rest_helpers.py @@ -0,0 +1,61 @@ +# Copyright 2017 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from google.api_core import rest_helpers + + +def test_flatten_none(): + assert rest_helpers.flatten_query_params(None) == [] + + +def test_flatten_empty_dict(): + assert rest_helpers.flatten_query_params({}) == [] + + +def test_flatten_simple_dict(): + assert rest_helpers.flatten_query_params({'a': 'abc', 'b': 'def'}) == [ + ('a', 'abc'), ('b', 'def')] + + +def test_flatten_repeated_field(): + assert rest_helpers.flatten_query_params({'a': ['x', 'y', 'z']}) == [ + ('a', 'x'), ('a', 'y'), ('a', 'z')] + + +def test_flatten_nested_dict(): + obj = {'a': + {'b': + {'c': ['x', 'y', 'z']}}, + 'd': + {'e': 'uvw'}} + expected_result = [('a.b.c', 'x'), + ('a.b.c', 'y'), + ('a.b.c', 'z'), + ('d.e', 'uvw')] + + assert rest_helpers.flatten_query_params(obj) == expected_result + + +def test_flatten_ignore_repeated_dict(): + obj = {'a': + {'b': + {'c': + [{'v': 1}, {'v': 2}] + } + }, + 'd': 'uvw', } + # a.b.c is a repeated dict - ignored + expected_result = [('d', 'uvw')] + + assert rest_helpers.flatten_query_params(obj) == expected_result From 9f84b87c77a661f983ed234aaebe21942a9d0210 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 16 Sep 2021 02:21:34 +0000 Subject: [PATCH 2/7] fix: updated copyright date in file headers. --- google/api_core/rest_helpers.py | 2 +- tests/unit/test_rest_helpers.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google/api_core/rest_helpers.py b/google/api_core/rest_helpers.py index 2bf99f15..d2e92b21 100644 --- a/google/api_core/rest_helpers.py +++ b/google/api_core/rest_helpers.py @@ -1,4 +1,4 @@ -# Copyright 2017 Google LLC +# Copyright 2021 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/tests/unit/test_rest_helpers.py b/tests/unit/test_rest_helpers.py index 8bffcbd4..75063686 100644 --- a/tests/unit/test_rest_helpers.py +++ b/tests/unit/test_rest_helpers.py @@ -1,4 +1,4 @@ -# Copyright 2017 Google LLC +# Copyright 2021 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From ee9cd18941311c42ce27df900014987fedf3d896 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 16 Sep 2021 21:39:03 +0000 Subject: [PATCH 3/7] fix: correct some error handling and address style issues. --- google/api_core/rest_helpers.py | 52 ++++++++++++++++++++++++--------- tests/unit/test_rest_helpers.py | 32 +++++++++++++++++--- 2 files changed, 66 insertions(+), 18 deletions(-) diff --git a/google/api_core/rest_helpers.py b/google/api_core/rest_helpers.py index d2e92b21..db58e872 100644 --- a/google/api_core/rest_helpers.py +++ b/google/api_core/rest_helpers.py @@ -14,7 +14,8 @@ """Helpers for rest transports.""" -import itertools +import functools +import operator def flatten_query_params(obj, key_path=[]): @@ -31,14 +32,30 @@ def flatten_query_params(obj, key_path=[]): >>> flatten_query_params(obj) [('a.b.c', 'x'), ('a.b.c', 'y'), ('a.b.c', 'z'), ('d', 'uvw')] + Note that, as described in + https://github.com/googleapis/googleapis/blob/48d9fb8c8e287c472af500221c6450ecd45d7d39/google/api/http.proto#L117, + repeated fields (i.e. list-valued fields) may only contain primitive types (not lists or dicts). + This is enforced in this function. + Args: - obj: a nested dictionary (from json) + obj: a nested dictionary (from json), or None key_path: a list of name segments, representing levels above this obj. Returns: a list of tuples, with each tuple having a (possibly) multi-part name and a scalar value. + + Raises: + TypeError if obj is not a dict or None + ValueError if obj contains a list of non-primitive values. """ + if obj is not None and not isinstance(obj, dict): + raise TypeError("flatten_query_params must be called with dict object") + + return _flatten(obj, key_path=key_path) + + +def _flatten(obj, key_path): if obj is None: return [] if isinstance(obj, dict): @@ -48,29 +65,36 @@ def flatten_query_params(obj, key_path=[]): return _flatten_value(obj, key_path=key_path) -def _is_value(obj): +def _is_primitive_value(obj): if obj is None: return False - return not (isinstance(obj, list) or isinstance(obj, dict)) + + if isinstance(obj, (list, dict)): + raise ValueError("query params may not contain repeated dicts or lists") + + return True -def _flatten_value(obj, key_path=[]): +def _flatten_value(obj, key_path): if not key_path: # There must be a key. return [] return [('.'.join(key_path), obj)] -def _flatten_dict(obj, key_path=[]): - return list( - itertools.chain(*(flatten_query_params(v, key_path=key_path + [k]) - for k, v in obj.items()))) +def _flatten_dict(obj, key_path): + items = ( + _flatten(value, key_path=key_path + [key]) + for key, value in obj.items() + ) + return functools.reduce(operator.concat, items, []) -def _flatten_list(l, key_path=[]): +def _flatten_list(elems, key_path): # Only lists of scalar values are supported. # The name (key_path) is repeated for each value. - return list( - itertools.chain(*(_flatten_value(elem, key_path=key_path) - for elem in l - if _is_value(elem)))) + items = ( + _flatten_value(elem, key_path=key_path) for elem in elems + if _is_primitive_value(elem) + ) + return functools.reduce(operator.concat, items, []) diff --git a/tests/unit/test_rest_helpers.py b/tests/unit/test_rest_helpers.py index 75063686..2c09da3d 100644 --- a/tests/unit/test_rest_helpers.py +++ b/tests/unit/test_rest_helpers.py @@ -12,9 +12,21 @@ # See the License for the specific language governing permissions and # limitations under the License. +import pytest + from google.api_core import rest_helpers +def test_flatten_simple_value(): + with pytest.raises(TypeError): + rest_helpers.flatten_query_params('abc') + + +def test_flatten_list(): + with pytest.raises(TypeError): + rest_helpers.flatten_query_params(['abc', 'def']) + + def test_flatten_none(): assert rest_helpers.flatten_query_params(None) == [] @@ -47,7 +59,7 @@ def test_flatten_nested_dict(): assert rest_helpers.flatten_query_params(obj) == expected_result -def test_flatten_ignore_repeated_dict(): +def test_flatten_repeated_dict(): obj = {'a': {'b': {'c': @@ -55,7 +67,19 @@ def test_flatten_ignore_repeated_dict(): } }, 'd': 'uvw', } - # a.b.c is a repeated dict - ignored - expected_result = [('d', 'uvw')] - assert rest_helpers.flatten_query_params(obj) == expected_result + with pytest.raises(ValueError): + rest_helpers.flatten_query_params(obj) + + +def test_flatten_repeated_list(): + obj = {'a': + {'b': + {'c': + [['e', 'f'], ['g', 'h']] + } + }, + 'd': 'uvw', } + + with pytest.raises(ValueError): + rest_helpers.flatten_query_params(obj) From 72a8025a544c6b04ead7b6c092c4207377bf5a63 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 16 Sep 2021 21:43:55 +0000 Subject: [PATCH 4/7] fix: removed unneeded parameter from top-level function. --- google/api_core/rest_helpers.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/google/api_core/rest_helpers.py b/google/api_core/rest_helpers.py index db58e872..6edd35c7 100644 --- a/google/api_core/rest_helpers.py +++ b/google/api_core/rest_helpers.py @@ -18,7 +18,7 @@ import operator -def flatten_query_params(obj, key_path=[]): +def flatten_query_params(obj): """Flatten a nested dict into a list of (name,value) tuples. The result is suitable for setting query params on an http request. @@ -39,7 +39,6 @@ def flatten_query_params(obj, key_path=[]): Args: obj: a nested dictionary (from json), or None - key_path: a list of name segments, representing levels above this obj. Returns: a list of tuples, with each tuple having a (possibly) multi-part name and a scalar value. @@ -52,7 +51,7 @@ def flatten_query_params(obj, key_path=[]): if obj is not None and not isinstance(obj, dict): raise TypeError("flatten_query_params must be called with dict object") - return _flatten(obj, key_path=key_path) + return _flatten(obj, key_path=[]) def _flatten(obj, key_path): From 06d113c900ccdc399793ee79fb83dc21e31d4021 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Thu, 16 Sep 2021 21:48:12 +0000 Subject: [PATCH 5/7] fix: removed handling of impossible case in helper function. --- google/api_core/rest_helpers.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/google/api_core/rest_helpers.py b/google/api_core/rest_helpers.py index 6edd35c7..592bdeae 100644 --- a/google/api_core/rest_helpers.py +++ b/google/api_core/rest_helpers.py @@ -75,9 +75,6 @@ def _is_primitive_value(obj): def _flatten_value(obj, key_path): - if not key_path: - # There must be a key. - return [] return [('.'.join(key_path), obj)] From 96509cb649afa8d6cd46a3923168af561d7515b7 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Fri, 17 Sep 2021 14:05:53 +0000 Subject: [PATCH 6/7] fix: improve test coverage --- tests/unit/test_rest_helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_rest_helpers.py b/tests/unit/test_rest_helpers.py index 2c09da3d..fc2cc18a 100644 --- a/tests/unit/test_rest_helpers.py +++ b/tests/unit/test_rest_helpers.py @@ -41,7 +41,7 @@ def test_flatten_simple_dict(): def test_flatten_repeated_field(): - assert rest_helpers.flatten_query_params({'a': ['x', 'y', 'z']}) == [ + assert rest_helpers.flatten_query_params({'a': ['x', 'y', 'z', None]}) == [ ('a', 'x'), ('a', 'y'), ('a', 'z')] From 3c9f52922a1cd03403f11dc5d782a539b94ef717 Mon Sep 17 00:00:00 2001 From: Kenneth Bandes Date: Fri, 17 Sep 2021 14:34:32 +0000 Subject: [PATCH 7/7] fix: reformat according to black (which is different from autopep8). --- google/api_core/rest_helpers.py | 10 +++---- tests/unit/test_rest_helpers.py | 50 ++++++++++++++------------------- 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/google/api_core/rest_helpers.py b/google/api_core/rest_helpers.py index 592bdeae..23fb614f 100644 --- a/google/api_core/rest_helpers.py +++ b/google/api_core/rest_helpers.py @@ -75,14 +75,11 @@ def _is_primitive_value(obj): def _flatten_value(obj, key_path): - return [('.'.join(key_path), obj)] + return [(".".join(key_path), obj)] def _flatten_dict(obj, key_path): - items = ( - _flatten(value, key_path=key_path + [key]) - for key, value in obj.items() - ) + items = (_flatten(value, key_path=key_path + [key]) for key, value in obj.items()) return functools.reduce(operator.concat, items, []) @@ -90,7 +87,8 @@ def _flatten_list(elems, key_path): # Only lists of scalar values are supported. # The name (key_path) is repeated for each value. items = ( - _flatten_value(elem, key_path=key_path) for elem in elems + _flatten_value(elem, key_path=key_path) + for elem in elems if _is_primitive_value(elem) ) return functools.reduce(operator.concat, items, []) diff --git a/tests/unit/test_rest_helpers.py b/tests/unit/test_rest_helpers.py index fc2cc18a..5932fa55 100644 --- a/tests/unit/test_rest_helpers.py +++ b/tests/unit/test_rest_helpers.py @@ -19,12 +19,12 @@ def test_flatten_simple_value(): with pytest.raises(TypeError): - rest_helpers.flatten_query_params('abc') + rest_helpers.flatten_query_params("abc") def test_flatten_list(): with pytest.raises(TypeError): - rest_helpers.flatten_query_params(['abc', 'def']) + rest_helpers.flatten_query_params(["abc", "def"]) def test_flatten_none(): @@ -36,50 +36,42 @@ def test_flatten_empty_dict(): def test_flatten_simple_dict(): - assert rest_helpers.flatten_query_params({'a': 'abc', 'b': 'def'}) == [ - ('a', 'abc'), ('b', 'def')] + assert rest_helpers.flatten_query_params({"a": "abc", "b": "def"}) == [ + ("a", "abc"), + ("b", "def"), + ] def test_flatten_repeated_field(): - assert rest_helpers.flatten_query_params({'a': ['x', 'y', 'z', None]}) == [ - ('a', 'x'), ('a', 'y'), ('a', 'z')] + assert rest_helpers.flatten_query_params({"a": ["x", "y", "z", None]}) == [ + ("a", "x"), + ("a", "y"), + ("a", "z"), + ] def test_flatten_nested_dict(): - obj = {'a': - {'b': - {'c': ['x', 'y', 'z']}}, - 'd': - {'e': 'uvw'}} - expected_result = [('a.b.c', 'x'), - ('a.b.c', 'y'), - ('a.b.c', 'z'), - ('d.e', 'uvw')] + obj = {"a": {"b": {"c": ["x", "y", "z"]}}, "d": {"e": "uvw"}} + expected_result = [("a.b.c", "x"), ("a.b.c", "y"), ("a.b.c", "z"), ("d.e", "uvw")] assert rest_helpers.flatten_query_params(obj) == expected_result def test_flatten_repeated_dict(): - obj = {'a': - {'b': - {'c': - [{'v': 1}, {'v': 2}] - } - }, - 'd': 'uvw', } + obj = { + "a": {"b": {"c": [{"v": 1}, {"v": 2}]}}, + "d": "uvw", + } with pytest.raises(ValueError): rest_helpers.flatten_query_params(obj) def test_flatten_repeated_list(): - obj = {'a': - {'b': - {'c': - [['e', 'f'], ['g', 'h']] - } - }, - 'd': 'uvw', } + obj = { + "a": {"b": {"c": [["e", "f"], ["g", "h"]]}}, + "d": "uvw", + } with pytest.raises(ValueError): rest_helpers.flatten_query_params(obj)