Skip to content

Commit 61edbd6

Browse files
software-dovsoftware-dov
authored
Fix for samplegen bugs discovered from the test harness (#220)
Implement partial fix for #216 quotes not handled correctly FIx for #216 : lower case (snake) manifest file name
1 parent 0e1149b commit 61edbd6

7 files changed

Lines changed: 49 additions & 34 deletions

File tree

‎packages/gapic-generator/gapic/samplegen/manifest.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from typing import Tuple
1818

1919
from gapic.samplegen_utils import (types, yaml)
20+
from gapic.utils import case
2021

2122
BASE_PATH_KEY = "base_path"
2223
DEFAULT_SAMPLE_DIR = "samples"
@@ -111,7 +112,7 @@ def transform_path(fpath):
111112
)
112113

113114
manifest_fname = manifest_fname_template.format(
114-
api=api_schema.naming.name,
115+
api=case.to_snake_case(api_schema.naming.name),
115116
version=api_schema.naming.version,
116117
language=environment.name,
117118
year=dt.tm_year,

‎packages/gapic-generator/gapic/samplegen/samplegen.py‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import dataclasses
1616
import itertools
1717
import jinja2
18+
import json
1819
import keyword
1920
import os
2021
import re
@@ -275,10 +276,6 @@ def validate_and_transform_request(self,
275276
if not witness:
276277
raise types.InvalidEnumVariant(
277278
"Invalid variant for enum {}: '{}'".format(attr, val))
278-
# Python code can set protobuf enums from strings.
279-
# This is preferable to adding the necessary import statement
280-
# and requires less munging of the assigned value
281-
duplicate["value"] = f"'{val}'"
282279
break
283280
elif attr.is_primitive:
284281
# Only valid if this is the last attribute in the chain.
@@ -305,6 +302,17 @@ def validate_and_transform_request(self,
305302
attr_chain[0]))
306303
del duplicate["field"]
307304

305+
if isinstance(duplicate["value"], str):
306+
# Passing value through json is a safe and simple way of
307+
# making sure strings are properly wrapped and quotes escaped.
308+
# This statement both wraps enums in quotes and escapes quotes
309+
# in string values passed as parameters.
310+
#
311+
# Python code can set protobuf enums from strings.
312+
# This is preferable to adding the necessary import statement
313+
# and requires less munging of the assigned value
314+
duplicate["value"] = json.dumps(duplicate["value"])
315+
308316
# Mypy isn't smart enough to handle dictionary unpacking,
309317
# so disable it for the AttributeRequestSetup ctor call.
310318
base_param_to_attrs[attr_chain[0]].append(

‎packages/gapic-generator/gapic/templates/examples/feature_fragments.j2‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ There is a little, but not enough for it to be important because
4141

4242
{% macro print_string_formatting(string_list) %}
4343
{% if string_list|length == 1 %}
44-
"{{ string_list[0]|replace("%s", "{}") }}"
44+
"{{ string_list[0]|replace("%s", "{}")|replace('\"', '\\\"') }}"
4545
{% else %}
46-
"{{ string_list[0]|replace("%s", "{}") }}".format({{ string_list[1:]|map("coerce_response_name")|join(", ") }})
46+
"{{ string_list[0]|replace("%s", "{}")|replace('\"', '\\\"') }}".format({{ string_list[1:]|map("coerce_response_name")|join(", ") }})
4747
{% endif %}
4848
{% endmacro %}
4949

@@ -177,14 +177,14 @@ with open({{ attr.input_parameter }}, "rb") as f:
177177
{% endmacro %}
178178

179179
{% macro render_method_call(sample, calling_form, calling_form_enum) %}
180-
{# Note: this doesn't deal with enums or unions #}
180+
{# Note: this doesn't deal with enums or unions #}
181181
{% if calling_form in [calling_form_enum.RequestStreamingBidi,
182-
calling_form_enum.RequestStreamingClient] %}
183-
client.{{ sample.rpc|snake_case }}([{{ render_request_params(sample.request) }}])
184-
{% else %}
182+
calling_form_enum.RequestStreamingClient] -%}
183+
client.{{ sample.rpc|snake_case }}([{{ render_request_params(sample.request)|trim -}}])
184+
{% else -%}
185185
{# TODO: set up client streaming once some questions are answered #}
186-
client.{{ sample.rpc|snake_case }}({{ render_request_params(sample.request) }})
187-
{% endif %}
186+
client.{{ sample.rpc|snake_case }}({{ render_request_params(sample.request)|trim -}})
187+
{% endif -%}
188188
{% endmacro %}
189189

190190
{# Setting up the method invocation is the responsibility of the caller: #}

‎packages/gapic-generator/tests/unit/generator/test_generator.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ def test_samplegen_config_to_output_files(
351351
content="\n",
352352
),
353353
CodeGeneratorResponse.File(
354-
name="samples/Mollusc.v6.python.21120601.131313.manifest.yaml",
354+
name="samples/mollusc.v6.python.21120601.131313.manifest.yaml",
355355
content=dedent("""\
356356
---
357357
type: manifest/samples
@@ -441,7 +441,7 @@ def test_samplegen_id_disambiguation(mock_gmtime, mock_generate_sample, fs):
441441
content="\n",
442442
),
443443
CodeGeneratorResponse.File(
444-
name="samples/Mollusc.v6.python.21120601.131313.manifest.yaml",
444+
name="samples/mollusc.v6.python.21120601.131313.manifest.yaml",
445445
content=dedent("""\
446446
---
447447
type: manifest/samples

‎packages/gapic-generator/tests/unit/samplegen/test_integration.py‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,14 @@ def test_generate_sample_basic():
9191
"description": "Determine the full taxonomy of input mollusc",
9292
"request": [
9393
{"field": "classify_request.video",
94-
"value": "'path/to/mollusc/video.mkv'",
94+
"value": "path/to/mollusc/video.mkv",
9595
"input_parameter": "video",
9696
"value_is_file": True},
9797
{"field": "classify_request.location_annotation",
98-
"value": "'New Zealand'",
98+
"value": "New Zealand",
9999
"input_parameter": "location"}
100100
],
101-
"response": [{"print": ["Mollusc is a %s", "$resp.taxonomy"]}]}
101+
"response": [{"print": ['Mollusc is a "%s"', "$resp.taxonomy"]}]}
102102

103103
sample_str = samplegen.generate_sample(
104104
sample,
@@ -130,16 +130,16 @@ def sample_classify(video, location):
130130
)
131131
132132
classify_request = {}
133-
# video = 'path/to/mollusc/video.mkv'
133+
# video = "path/to/mollusc/video.mkv"
134134
with open(video, "rb") as f:
135135
classify_request["video"] = f.read()
136136
137-
# location = 'New Zealand'
137+
# location = "New Zealand"
138138
classify_request["location_annotation"] = location
139139
140140
141141
response = client.classify(classify_request)
142-
print("Mollusc is a {}".format(response.taxonomy))
142+
print("Mollusc is a \\"{}\\"".format(response.taxonomy))
143143
144144
# [END %s]
145145
@@ -149,10 +149,10 @@ def main():
149149
parser = argparse.ArgumentParser()
150150
parser.add_argument("--video",
151151
type=str,
152-
default='path/to/mollusc/video.mkv')
152+
default="path/to/mollusc/video.mkv")
153153
parser.add_argument("--location",
154154
type=str,
155-
default='New Zealand')
155+
default="New Zealand")
156156
args = parser.parse_args()
157157
158158
sample_classify(args.video, args.location)

‎packages/gapic-generator/tests/unit/samplegen/test_manifest.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def test_generate_manifest():
3838
manifest_time=4486525628
3939
)
4040

41-
assert fname == "Mollusc.v1.python.21120304.090708.manifest.yaml"
41+
assert fname == "mollusc.v1.python.21120304.090708.manifest.yaml"
4242

4343
doc = gapic_yaml.Doc([
4444
gapic_yaml.KeyVal("type", "manifest/samples"),

‎packages/gapic-generator/tests/unit/samplegen/test_samplegen.py‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -878,7 +878,10 @@ def test_validate_request_basic():
878878
"mantle_length": DummyField(
879879
message=DummyMessage(type="LENGTH_TYPE")),
880880
"mantle_mass": DummyField(
881-
message=DummyMessage(type="MASS_TYPE"))},
881+
message=DummyMessage(type="MASS_TYPE")),
882+
"num_tentacles": DummyField(
883+
message=DummyMessage(type="MASS_TYPE"))
884+
},
882885
type="SQUID_TYPE"
883886
)
884887
)
@@ -890,17 +893,20 @@ def test_validate_request_basic():
890893
actual = v.validate_and_transform_request(
891894
types.CallingForm.Request,
892895
[
893-
{"field": "squid.mantle_length", "value": "100 cm"},
896+
{"field": "squid.mantle_length", "value": '100 "cm'},
894897
{"field": "squid.mantle_mass", "value": "10 kg"},
898+
{"field": "squid.num_tentacles", "value": 10},
895899
],
896900
)
897901
expected = [samplegen.TransformedRequest(
898902
base="squid",
899903
body=[
900904
samplegen.AttributeRequestSetup(field="mantle_length",
901-
value="100 cm"),
905+
value='"100 \\"cm"'),
902906
samplegen.AttributeRequestSetup(field="mantle_mass",
903-
value="10 kg"),
907+
value='"10 kg"'),
908+
samplegen.AttributeRequestSetup(field="num_tentacles",
909+
value=10)
904910
],
905911
single=None
906912
)]
@@ -940,7 +946,7 @@ def test_validate_request_top_level_field():
940946
samplegen.TransformedRequest(base="squid",
941947
body=None,
942948
single=samplegen.AttributeRequestSetup(
943-
value="humboldt"
949+
value='"humboldt"'
944950
))
945951
]
946952

@@ -1033,15 +1039,15 @@ def test_validate_request_multiple_arguments():
10331039
base="squid",
10341040
body=[samplegen.AttributeRequestSetup(
10351041
field="mantle_length",
1036-
value="100 cm",
1042+
value='"100 cm"',
10371043
value_is_file=True)],
10381044
single=None
10391045
),
10401046
samplegen.TransformedRequest(
10411047
base="clam",
10421048
body=[samplegen.AttributeRequestSetup(
10431049
field="shell_mass",
1044-
value="100 kg",
1050+
value='"100 kg"',
10451051
comment="Clams can be large")],
10461052
single=None
10471053
),
@@ -1520,7 +1526,7 @@ def test_validate_request_enum():
15201526
expected = [samplegen.TransformedRequest(
15211527
"cephalopod",
15221528
body=[samplegen.AttributeRequestSetup(field="subclass",
1523-
value="'COLEOIDEA'")],
1529+
value='"COLEOIDEA"')],
15241530
single=None)]
15251531
assert actual == expected
15261532

@@ -1536,7 +1542,7 @@ def test_validate_request_enum_top_level():
15361542
)
15371543
expected = [samplegen.TransformedRequest(
15381544
"subclass",
1539-
single=samplegen.AttributeRequestSetup(value="'COLEOIDEA'"),
1545+
single=samplegen.AttributeRequestSetup(value='"COLEOIDEA"'),
15401546
body=None)]
15411547
assert actual == expected
15421548

@@ -1598,7 +1604,7 @@ def test_validate_request_primitive_field():
15981604
base="species",
15991605
body=None,
16001606
single=samplegen.AttributeRequestSetup(
1601-
value="Architeuthis dux"
1607+
value='"Architeuthis dux"'
16021608
)
16031609
)
16041610
]

0 commit comments

Comments
 (0)