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
