Fix JSON encoding of nested message fields corresponding to query params. (#321)
* Add example of a nested message field query param to samples.
* Fix JSON encoding of nested message fields corresponding to query params.
Note that apitools already has logic to add custom JSON encodings in the
generated messages file for message fields whose names differ from their
eventual JSON representations:
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/gen/extended_descriptor.py#L220-L225
Examples can be found throughout the generated message files, e.g.
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/samples/iam_sample/iam_v1/iam_v1_messages.py#L955-L956.
This happens here when writing the file:
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/gen/extended_descriptor.py#L162-L171
Specifically, in AddDescriptorFromSchema, the ExtendedMessageDescriptor
objects that are instantiated will contain field_mappings for any field
whose converted Python attribute name in the message differs from its
name in the discovery doc:
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/gen/message_registry.py#L278-L281
This all works correctly for explicit message types defined in the API
protos (which end up in the "schemas" field of the discovery doc).
However, apitools also uses AddDescriptorFromSchema to generate
top-level request message types (which don't correspond to any
underlying discovery doc schema message):
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/gen/service_registry.py#L273
To do so, it basically creates a schema dictionary on the fly that
matches the one used in the discovery doc JSON and includes the
properties for each field, and then passes that to
AddDescriptorFromSchema. The problem is that when creating this
pseudo-schema, it determines the field name property for query params by
calling self.__names.CleanName() on the parameter name:
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/gen/service_registry.py#L288
But this means that the field name is already in its converted Python
form when AddDescriptorFromSchema sees it, so the generated field name
matches the field name passed to it, and no custom field mapping
gets added even if there should be one. In particular, for nested
message query params, the JSON parameter will be something like e.g.
options.requestedPolicyVersion, whereas the Python name will be e.g.
options_requestedPolicyVersion, so a custom JSON encoding *should* be
generated for that field, but isn't (similarly for query params that
conflict with Python keywords e.g. 'continue' <-> 'continue_').
The fix here is just deleting the call to self.__names.CleanName() in
__CreateRequestType; AddDescriptorFromSchema already calls that when
constructing the field
(https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/gen/message_registry.py#L329-L332)
and thus there's no need to call it ahead of time. This way,
AddDescriptorFromSchema will receive the original JSON parameter name,
and can see that it differs from the generated Python field name,
so it will now add a custom field mapping for any such query param
fields in the request message.
Note that this bug has been present in apitools for as long as it has
existed, dating back to the very first version. This has necessitated a
lot of manual calls to encoding.AddCustomJsonFieldMapping() to work
around it in consumers of apitools; these can now all be cleaned up
since they'll be included in the generated messages file directly. In
theory this shouldn't break anything either; the behavior already was
broken and requests would fail without the workaround, and the
workarounds themselves won't affect anything even if they're not cleaned
up after this fix (it's okay to add a custom JSON field mapping twice as
long as both mappings are the same:
https://github.com/google/apitools/blob/c7d44d6bef513e82fe2b8740352e5e2290dd2e5c/apitools/base/py/encoding_helper.py#L676-L685).
4 files changed
