From 0ebbe414bf3beb3c9224a6b28dbdbe3df509f6ae Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 7 Mar 2023 11:34:37 +0000 Subject: [PATCH 01/14] fix: remove trailing underscore in transcoding --- google/api_core/path_template.py | 17 ++++++++++++++++- tests/unit/test_path_template.py | 7 +++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index b8ebb2af..c8933acd 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -308,9 +308,24 @@ def transcode(http_options, message=None, **request_kwargs): else: try: if message: - request["body"] = getattr(leftovers, body) + try: + request["body"] = getattr(leftovers, body) + except AttributeError as e: + # gapic-generator-python appends underscores to field names + # that collide with python keywords. + # `_` is stripped away as it is not possible to + # natively define a field with a trailing underscore in protobuf. + # See related issue + # https://github.com/googleapis/python-api-core/issues/227 + if hasattr(leftovers, body + "_"): + request["body"] = getattr(leftovers, body + "_") + else: + raise e delete_field(leftovers, body) else: + # gapic-generator-python appends underscores to field names + # that collide with python keywords. + leftovers = {key.rstrip('_') : val for key, val in leftovers.items()} request["body"] = leftovers.pop(body) except (KeyError, AttributeError): continue diff --git a/tests/unit/test_path_template.py b/tests/unit/test_path_template.py index 808b36f3..431f0463 100644 --- a/tests/unit/test_path_template.py +++ b/tests/unit/test_path_template.py @@ -413,6 +413,13 @@ def test_transcode_with_wildcard( {"data": {"id": 1, "info": "some info"}, "foo": "bar"}, ["post", "/v1/no/template", {"id": 1, "info": "some info"}, {"foo": "bar"}], ], + # Single field body with trailing underscore + [ + [["post", "/v1/no/template", "data"]], + None, + {"data": {"id": 1, "info": "some info"}, "foo_": "bar"}, + ["post", "/v1/no/template", {"id": 1, "info": "some info"}, {"foo": "bar"}], + ], [ [["post", "/v1/no/template", "oauth"]], auth_pb2.AuthenticationRule( From 262c60e2f440d2e9809a1d324e1a098bf32d36b3 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 7 Mar 2023 11:50:25 +0000 Subject: [PATCH 02/14] lint --- google/api_core/path_template.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index c8933acd..80c98cc2 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -325,7 +325,9 @@ def transcode(http_options, message=None, **request_kwargs): else: # gapic-generator-python appends underscores to field names # that collide with python keywords. - leftovers = {key.rstrip('_') : val for key, val in leftovers.items()} + leftovers = { + key.rstrip("_"): val for key, val in leftovers.items() + } request["body"] = leftovers.pop(body) except (KeyError, AttributeError): continue From f192273d2f9c483104016a84b096bc9a5bd74921 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 7 Mar 2023 11:51:04 +0000 Subject: [PATCH 03/14] add test --- tests/unit/test_path_template.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/unit/test_path_template.py b/tests/unit/test_path_template.py index 431f0463..881ea15b 100644 --- a/tests/unit/test_path_template.py +++ b/tests/unit/test_path_template.py @@ -626,6 +626,14 @@ def test_transcode_with_additional_bindings( auth_pb2.AuthenticationRule(selector="first"), {}, ], + # Test trailing underscore in body is removed during transcoding + # See https://github.com/googleapis/python-api-core/issues/490 + [[["post", "/v1/{name}", "data"]], None, {"name": "first/last"}], + [ + [["post", "/v1/{selector}", "data_"]], + auth_pb2.AuthenticationRule(selector="first"), + {}, + ], [[["post", "/v1/{first_name}", "data"]], None, {"last_name": "last"}], [ [["post", "/v1/{first_name}", ""]], From 3ba7c348bf39c26e2412636240c52195b8e1ff95 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 7 Mar 2023 12:23:30 +0000 Subject: [PATCH 04/14] revert test --- tests/unit/test_path_template.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/unit/test_path_template.py b/tests/unit/test_path_template.py index 881ea15b..431f0463 100644 --- a/tests/unit/test_path_template.py +++ b/tests/unit/test_path_template.py @@ -626,14 +626,6 @@ def test_transcode_with_additional_bindings( auth_pb2.AuthenticationRule(selector="first"), {}, ], - # Test trailing underscore in body is removed during transcoding - # See https://github.com/googleapis/python-api-core/issues/490 - [[["post", "/v1/{name}", "data"]], None, {"name": "first/last"}], - [ - [["post", "/v1/{selector}", "data_"]], - auth_pb2.AuthenticationRule(selector="first"), - {}, - ], [[["post", "/v1/{first_name}", "data"]], None, {"last_name": "last"}], [ [["post", "/v1/{first_name}", ""]], From 71810fd5ea71baf887cde9fcf0923c416d387605 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 7 Mar 2023 15:53:49 +0000 Subject: [PATCH 05/14] add test --- google/api_core/path_template.py | 3 ++- tests/unit/test_path_template.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index 80c98cc2..ac54e365 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -310,6 +310,7 @@ def transcode(http_options, message=None, **request_kwargs): if message: try: request["body"] = getattr(leftovers, body) + delete_field(leftovers, body) except AttributeError as e: # gapic-generator-python appends underscores to field names # that collide with python keywords. @@ -319,9 +320,9 @@ def transcode(http_options, message=None, **request_kwargs): # https://github.com/googleapis/python-api-core/issues/227 if hasattr(leftovers, body + "_"): request["body"] = getattr(leftovers, body + "_") + delete_field(leftovers, body + "_") else: raise e - delete_field(leftovers, body) else: # gapic-generator-python appends underscores to field names # that collide with python keywords. diff --git a/tests/unit/test_path_template.py b/tests/unit/test_path_template.py index 431f0463..0b93f4d6 100644 --- a/tests/unit/test_path_template.py +++ b/tests/unit/test_path_template.py @@ -17,10 +17,20 @@ import mock import pytest +import proto + from google.api import auth_pb2 from google.api_core import path_template +class Breakpoint(proto.Message): + name = proto.Field(proto.STRING, number=1) + +class SomeMessage(proto.Message): + breakpoint_ = proto.Field(Breakpoint, number=1) + debuggee_id = proto.Field(proto.STRING, number=2) + + @pytest.mark.parametrize( "tmpl, args, kwargs, expected_result", [ @@ -420,6 +430,13 @@ def test_transcode_with_wildcard( {"data": {"id": 1, "info": "some info"}, "foo_": "bar"}, ["post", "/v1/no/template", {"id": 1, "info": "some info"}, {"foo": "bar"}], ], + # Single field body with reserved keyword, using message where field name has trailing underscore + [ + [["post", "/v1/no/template", "breakpoint"]], + SomeMessage(breakpoint_=Breakpoint(name="test"),debuggee_id="test")._pb, + {}, + ["post", "/v1/no/template", Breakpoint(name="test")._pb, SomeMessage(debuggee_id="test")._pb], + ], [ [["post", "/v1/no/template", "oauth"]], auth_pb2.AuthenticationRule( From 9d730851a876f8019c4ea4a76921e19b8dce1771 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 7 Mar 2023 15:56:50 +0000 Subject: [PATCH 06/14] lint --- tests/unit/test_path_template.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_path_template.py b/tests/unit/test_path_template.py index 0b93f4d6..edbe9ed5 100644 --- a/tests/unit/test_path_template.py +++ b/tests/unit/test_path_template.py @@ -26,6 +26,7 @@ class Breakpoint(proto.Message): name = proto.Field(proto.STRING, number=1) + class SomeMessage(proto.Message): breakpoint_ = proto.Field(Breakpoint, number=1) debuggee_id = proto.Field(proto.STRING, number=2) @@ -433,9 +434,14 @@ def test_transcode_with_wildcard( # Single field body with reserved keyword, using message where field name has trailing underscore [ [["post", "/v1/no/template", "breakpoint"]], - SomeMessage(breakpoint_=Breakpoint(name="test"),debuggee_id="test")._pb, + SomeMessage(breakpoint_=Breakpoint(name="test"), debuggee_id="test")._pb, {}, - ["post", "/v1/no/template", Breakpoint(name="test")._pb, SomeMessage(debuggee_id="test")._pb], + [ + "post", + "/v1/no/template", + Breakpoint(name="test")._pb, + SomeMessage(debuggee_id="test")._pb, + ], ], [ [["post", "/v1/no/template", "oauth"]], From e3778923771a9f7568bd716b467c68c4effd41fc Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Wed, 8 Mar 2023 10:31:56 +0000 Subject: [PATCH 07/14] Address review comments --- google/api_core/path_template.py | 38 ++++++++++++++++---------------- tests/unit/test_path_template.py | 9 +++++--- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index ac54e365..ec795688 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -298,6 +298,15 @@ def transcode(http_options, message=None, **request_kwargs): # Assign body and query params body = http_option.get("body") + # gapic-generator-python appends underscores to field names + # that collide with python keywords. + # `_` is stripped away as it is not possible to + # natively define a field with a trailing underscore in protobuf. + # See related issue + # https://github.com/googleapis/python-api-core/issues/227 + if isinstance(leftovers, dict): + leftovers = {key.rstrip("_"): val for key, val in leftovers.items()} + if body: if body == "*": request["body"] = leftovers @@ -308,27 +317,18 @@ def transcode(http_options, message=None, **request_kwargs): else: try: if message: - try: - request["body"] = getattr(leftovers, body) - delete_field(leftovers, body) - except AttributeError as e: - # gapic-generator-python appends underscores to field names - # that collide with python keywords. - # `_` is stripped away as it is not possible to - # natively define a field with a trailing underscore in protobuf. - # See related issue - # https://github.com/googleapis/python-api-core/issues/227 - if hasattr(leftovers, body + "_"): - request["body"] = getattr(leftovers, body + "_") - delete_field(leftovers, body + "_") - else: - raise e - else: # gapic-generator-python appends underscores to field names # that collide with python keywords. - leftovers = { - key.rstrip("_"): val for key, val in leftovers.items() - } + # `_` is stripped away as it is not possible to + # natively define a field with a trailing underscore in protobuf. + # See related issue + # https://github.com/googleapis/python-api-core/issues/227 + field_suffix = "" + if hasattr(leftovers, body + "_"): + field_suffix = "_" + request["body"] = getattr(leftovers, f"{body}{field_suffix}") + delete_field(leftovers, f"{body}{field_suffix}") + else: request["body"] = leftovers.pop(body) except (KeyError, AttributeError): continue diff --git a/tests/unit/test_path_template.py b/tests/unit/test_path_template.py index edbe9ed5..148e2936 100644 --- a/tests/unit/test_path_template.py +++ b/tests/unit/test_path_template.py @@ -30,6 +30,7 @@ class Breakpoint(proto.Message): class SomeMessage(proto.Message): breakpoint_ = proto.Field(Breakpoint, number=1) debuggee_id = proto.Field(proto.STRING, number=2) + stacktrace_ = proto.Field(proto.STRING, number=3) @pytest.mark.parametrize( @@ -434,13 +435,15 @@ def test_transcode_with_wildcard( # Single field body with reserved keyword, using message where field name has trailing underscore [ [["post", "/v1/no/template", "breakpoint"]], - SomeMessage(breakpoint_=Breakpoint(name="test"), debuggee_id="test")._pb, + SomeMessage( + breakpoint_=Breakpoint(name="foo"), debuggee_id="bar", stacktrace_="baz" + )._pb, {}, [ "post", "/v1/no/template", - Breakpoint(name="test")._pb, - SomeMessage(debuggee_id="test")._pb, + Breakpoint(name="foo")._pb, + SomeMessage(debuggee_id="bar", stacktrace_="baz")._pb, ], ], [ From 1c568945277c464ef2a77643470135365d0c083b Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Wed, 8 Mar 2023 10:36:46 +0000 Subject: [PATCH 08/14] clarify comment --- google/api_core/path_template.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index ec795688..9da29563 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -298,7 +298,7 @@ def transcode(http_options, message=None, **request_kwargs): # Assign body and query params body = http_option.get("body") - # gapic-generator-python appends underscores to field names + # gapic-generator-python appends an underscore to field names # that collide with python keywords. # `_` is stripped away as it is not possible to # natively define a field with a trailing underscore in protobuf. @@ -317,7 +317,7 @@ def transcode(http_options, message=None, **request_kwargs): else: try: if message: - # gapic-generator-python appends underscores to field names + # gapic-generator-python append an underscores to field names # that collide with python keywords. # `_` is stripped away as it is not possible to # natively define a field with a trailing underscore in protobuf. From eb3b50e105987d6942098aa1670a896aeadb2e50 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 9 Mar 2023 16:16:55 +0000 Subject: [PATCH 09/14] address review feedback --- google/api_core/path_template.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index 9da29563..6a260ebf 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -317,12 +317,10 @@ def transcode(http_options, message=None, **request_kwargs): else: try: if message: - # gapic-generator-python append an underscores to field names - # that collide with python keywords. - # `_` is stripped away as it is not possible to - # natively define a field with a trailing underscore in protobuf. - # See related issue - # https://github.com/googleapis/python-api-core/issues/227 + # See above comment where gapic-generator-python appends + # underscores to field names that are python reserved words + # If the message has an attribute with an underscore suffix, + # use that instead. field_suffix = "" if hasattr(leftovers, body + "_"): field_suffix = "_" From 3e2834c88bf1145edc0c9368e56736ffc8370dc1 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 9 Mar 2023 16:17:24 +0000 Subject: [PATCH 10/14] typo --- google/api_core/path_template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index 6a260ebf..cfa8cc31 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -318,7 +318,7 @@ def transcode(http_options, message=None, **request_kwargs): try: if message: # See above comment where gapic-generator-python appends - # underscores to field names that are python reserved words + # underscores to field names that are python reserved words. # If the message has an attribute with an underscore suffix, # use that instead. field_suffix = "" From dd317a9b185330a6ee9f3226efdbab0cd3310d98 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Thu, 9 Mar 2023 16:20:04 +0000 Subject: [PATCH 11/14] lint --- google/api_core/path_template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index cfa8cc31..4edee03b 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -319,7 +319,7 @@ def transcode(http_options, message=None, **request_kwargs): if message: # See above comment where gapic-generator-python appends # underscores to field names that are python reserved words. - # If the message has an attribute with an underscore suffix, + # If the message has an attribute with an underscore suffix, # use that instead. field_suffix = "" if hasattr(leftovers, body + "_"): From 0918c7a8a8d3ff711923de1bdab3036260c0a826 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Thu, 9 Mar 2023 16:21:05 +0000 Subject: [PATCH 12/14] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- google/api_core/path_template.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index cfa8cc31..4edee03b 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -319,7 +319,7 @@ def transcode(http_options, message=None, **request_kwargs): if message: # See above comment where gapic-generator-python appends # underscores to field names that are python reserved words. - # If the message has an attribute with an underscore suffix, + # If the message has an attribute with an underscore suffix, # use that instead. field_suffix = "" if hasattr(leftovers, body + "_"): From 5504d0ad2112f328d42af3eba5ed3a0a98c2a292 Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 14 Mar 2023 13:47:56 +0000 Subject: [PATCH 13/14] address review feedback --- google/api_core/path_template.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index 4edee03b..1108bedb 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -299,13 +299,22 @@ def transcode(http_options, message=None, **request_kwargs): body = http_option.get("body") # gapic-generator-python appends an underscore to field names - # that collide with python keywords. - # `_` is stripped away as it is not possible to - # natively define a field with a trailing underscore in protobuf. - # See related issue + # that collide with python keywords. See related issue # https://github.com/googleapis/python-api-core/issues/227 + # `leftovers` can either be a dict or protobuf message. + # When `leftovers` is a dict, the `_` suffix in each key + # is stripped away as it is not possible to natively define a field + # with a trailing underscore in protobuf. + # When `leftovers` is a protobuf message, we need to use an underscore + # suffix when accessing the field in the protobuf message when the + # field has an underscore suffix. + field_suffix = "" + if isinstance(leftovers, dict): leftovers = {key.rstrip("_"): val for key, val in leftovers.items()} + elif body: + if hasattr(leftovers, body + "_"): + field_suffix = "_" if body: if body == "*": @@ -317,13 +326,6 @@ def transcode(http_options, message=None, **request_kwargs): else: try: if message: - # See above comment where gapic-generator-python appends - # underscores to field names that are python reserved words. - # If the message has an attribute with an underscore suffix, - # use that instead. - field_suffix = "" - if hasattr(leftovers, body + "_"): - field_suffix = "_" request["body"] = getattr(leftovers, f"{body}{field_suffix}") delete_field(leftovers, f"{body}{field_suffix}") else: From 6558cc66b0082f67ba70cc1c96491d1d79d4ef1b Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Tue, 14 Mar 2023 14:35:36 +0000 Subject: [PATCH 14/14] clarify comment --- google/api_core/path_template.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/google/api_core/path_template.py b/google/api_core/path_template.py index 1108bedb..2b90a710 100644 --- a/google/api_core/path_template.py +++ b/google/api_core/path_template.py @@ -301,6 +301,8 @@ def transcode(http_options, message=None, **request_kwargs): # gapic-generator-python appends an underscore to field names # that collide with python keywords. See related issue # https://github.com/googleapis/python-api-core/issues/227 + # Make sure that when the fields are REST encoded, the + # Python-specific trailing underscore is removed before going on the wire. # `leftovers` can either be a dict or protobuf message. # When `leftovers` is a dict, the `_` suffix in each key # is stripped away as it is not possible to natively define a field