Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

Commit cdbc221

Browse files
software-dovsoftware-dov
authored
fix: remove duplicate assignment of certain flattened, repeated fields (#760)
Fix for #756. Under certain circumstances, flattened, repeated fields could be duplicated during request construction.
1 parent 944136c commit cdbc221

4 files changed

Lines changed: 25 additions & 26 deletions

File tree

‎gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -341,17 +341,17 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
341341
raise ValueError('If the `request` argument is set, then none of '
342342
'the individual field arguments should be set.')
343343

344-
{% endif -%}
344+
{% endif -%} {# method.flattened_fields #}
345345
{% if method.input.ident.package != method.ident.package -%} {# request lives in a different package, so there is no proto wrapper #}
346-
# The request isn't a proto-plus wrapped type,
346+
# The request isn't a proto-plus wrapped type.
347347
# so it must be constructed via keyword expansion.
348348
if isinstance(request, dict):
349349
request = {{ method.input.ident }}(**request)
350350
{% if method.flattened_fields -%}{# Cross-package req and flattened fields #}
351351
elif not request:
352352
request = {{ method.input.ident }}({% if method.input.ident.package != method.ident.package %}{% for f in method.flattened_fields.values() %}{{ f.name }}={{ f.name }}, {% endfor %}{% endif %})
353353
{% endif -%}{# Cross-package req and flattened fields #}
354-
{%- else %}
354+
{%- else %} {# Request is in _our_ package #}
355355
# Minor optimization to avoid making a copy if the user passes
356356
# in a {{ method.input.ident }}.
357357
# There's no risk of modifying the input as we've already verified
@@ -364,22 +364,22 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
364364
# If we have keyword arguments corresponding to fields on the
365365
# request, apply these.
366366
{% endif -%}
367-
{%- for key, field in method.flattened_fields.items() if not(field.repeated and method.input.ident.package != method.ident.package) %}
367+
{%- for key, field in method.flattened_fields.items() if not field.repeated or method.input.ident.package == method.ident.package %}
368368
if {{ field.name }} is not None:
369369
request.{{ key }} = {{ field.name }}
370370
{%- endfor %}
371371
{# Map-y fields can be _updated_, however #}
372-
{%- for key, field in method.flattened_fields.items() if field.map and method.input.ident.package == method.ident.package %}
373-
372+
{%- for key, field in method.flattened_fields.items() if field.repeated and method.input.ident.package != method.ident.package %}
373+
{%- if field.map %} {# map implies repeated, but repeated does NOT imply map#}
374374
if {{ field.name }}:
375375
request.{{ key }}.update({{ field.name }})
376-
{%- endfor %}
376+
{%- else %}
377377
{# And list-y fields can be _extended_ -#}
378-
{%- for key, field in method.flattened_fields.items() if field.repeated and not field.map and method.input.ident.package == method.ident.package %}
379378
if {{ field.name }}:
380379
request.{{ key }}.extend({{ field.name }})
381-
{%- endfor %}
382-
{%- endif %}
380+
{%- endif %} {# field.map #}
381+
{%- endfor %} {# key, field in method.flattened_fields.items() #}
382+
{%- endif %} {# method.client_streaming #}
383383

384384
# Wrap the RPC method; this adds retry and timeout information,
385385
# and friendly error handling.
@@ -397,15 +397,15 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
397397
{%- endfor %}
398398
)),
399399
)
400-
{%- endif %}
400+
{%- endif %} {# method.field_headers #}
401401

402402
# Send the request.
403403
{% if not method.void %}response = {% endif %}rpc(
404404
{%- if not method.client_streaming %}
405405
request,
406406
{%- else %}
407407
requests,
408-
{%- endif %}
408+
{%- endif %} {# method.client_streaming #}
409409
retry=retry,
410410
timeout=timeout,
411411
metadata=metadata,
@@ -429,12 +429,12 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
429429
response=response,
430430
metadata=metadata,
431431
)
432-
{%- endif %}
432+
{%- endif %} {# method.lro #}
433433
{%- if not method.void %}
434434

435435
# Done; return the response.
436436
return response
437-
{%- endif %}
437+
{%- endif %} {# method.void #}
438438
{{ '\n' }}
439439
{% endfor %}
440440

‎gapic/ads-templates/noxfile.py.j2‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import os
66
import nox # type: ignore
77

88

9-
@nox.session(python=['3.6', '3.7'])
9+
@nox.session(python=['3.7', '3.8'])
1010
def unit(session):
1111
"""Run the unit test suite."""
1212

@@ -24,7 +24,7 @@ def unit(session):
2424
)
2525

2626

27-
@nox.session(python=['3.6', '3.7'])
27+
@nox.session(python=['3.7', '3.8'])
2828
def mypy(session):
2929
"""Run the type checker."""
3030
session.install('mypy')

‎gapic/ads-templates/setup.py.j2‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ setuptools.setup(
3535
'Development Status :: 3 - Alpha',
3636
'Intended Audience :: Developers',
3737
'Operating System :: OS Independent',
38-
'Programming Language :: Python :: 3.6',
3938
'Programming Language :: Python :: 3.7',
4039
'Programming Language :: Python :: 3.8',
4140
'Topic :: Internet',

‎gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
244244
if use_client_cert:
245245
if client_options.client_cert_source:
246246
is_mtls = True
247-
client_cert_source_func = client_options.client_cert_source
247+
client_cert_source_func = client_options.client_cert_source
248248
else:
249249
is_mtls = mtls.has_default_client_cert_source()
250250
client_cert_source_func = mtls.default_client_cert_source() if is_mtls else None
@@ -381,22 +381,22 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
381381
# If we have keyword arguments corresponding to fields on the
382382
# request, apply these.
383383
{% endif -%}
384-
{%- for key, field in method.flattened_fields.items() if not field.repeated and method.input.ident.package == method.ident.package %}
384+
{%- for key, field in method.flattened_fields.items() if not field.repeated or method.input.ident.package == method.ident.package %}
385385
if {{ field.name }} is not None:
386386
request.{{ key }} = {{ field.name }}
387387
{%- endfor %}
388-
{# Map-y fields can be _updated_, however #}
389-
{%- for key, field in method.flattened_fields.items() if field.map and method.input.ident.package == method.ident.package %}
390-
388+
{# Map-y fields can be _updated_, however #}
389+
{%- for key, field in method.flattened_fields.items() if field.repeated and method.input.ident.package != method.ident.package %}
390+
{%- if field.map %} {# map implies repeated, but repeated does NOT imply map#}
391391
if {{ field.name }}:
392392
request.{{ key }}.update({{ field.name }})
393-
{%- endfor %}
393+
{%- else %}
394394
{# And list-y fields can be _extended_ -#}
395-
{%- for key, field in method.flattened_fields.items() if field.repeated and not field.map and method.input.ident.package == method.ident.package %}
396395
if {{ field.name }}:
397396
request.{{ key }}.extend({{ field.name }})
398-
{%- endfor %}
399-
{%- endif %}
397+
{%- endif %} {# field.map #}
398+
{%- endfor %} {# method.flattened_fields.items() #}
399+
{%- endif %} {# method.client_streaming #}
400400

401401
# Wrap the RPC method; this adds retry and timeout information,
402402
# and friendly error handling.

0 commit comments

Comments
 (0)