Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add queryString to snake_case when using the fallback REST Method. #1312

Closed
sckimynwa opened this issue Aug 16, 2022 · 2 comments
Closed

add queryString to snake_case when using the fallback REST Method. #1312

sckimynwa opened this issue Aug 16, 2022 · 2 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@sckimynwa
Copy link
Contributor

I'm using gapic-generator-typescript to generate client-side REST fallback methods from protobuf.
which internally depends on gax-nodejs.

when requesting "Post" method with body, it works fine. but when i use "Get" method with query string
(to support pagination features like page_size, page_token), it sends camelCased query string.

https://my-domain.com/api/v1/taskGroups/-/tasks?pageSize=10

so when i checked implemented code on this module, it works like this.
in comment, it says "use camel case for query string".

// transcoding.ts
export function transcode(
  request: JSONObject,
  parsedOptions: ParsedOptionsType,
  requestFields?: {[k: string]: Field}
): TranscodedRequest | undefined {
...
const queryStringObject = deepCopy(request); // use camel case for query string
...
const queryStringComponents =
        buildQueryStringComponents(queryStringObject);
const queryString = queryStringComponents.join('&');
...
}

I'm wondering why gax-nodejs formats query string as "camelCase". because in google aip examples,
it seems like query string would be "snake_case". (https://google.aip.dev/127)

// The user-specified ID for the book.
// When using HTTP/JSON, this field is populated based on a query string
// argument, such as `?book_id=foo`. This is the fallback for fields that
// are not included in either the URI or the body.
string book_id = 3;

If you have any ideas of supporting snake_case in queryString?
(By the way, i forked this repository and use camelToSnakeCase Method to make query string snake _case)

@sckimynwa sckimynwa added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Aug 16, 2022
@sckimynwa
Copy link
Contributor Author

[Additional Comment]

It seems like Google pub/sub GET API is using camelCase for queryString 🤔
https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.topics.subscriptions/list

@sofisl
Copy link
Contributor

sofisl commented Aug 18, 2022

Hi @sckimynwa, gax-nodejs explicitly changes snake case query string paramaters to camel case. However, you're right the AIP comments are misleading:

  // The user-specified ID for the book.
  // When using HTTP/JSON, this field is populated based on a query string
  // argument, such as `?book_id=foo`. This is the fallback for fields that
  // are not included in either the URI or the body.

We've created a PR to update the documentation. Thanks for submitting the bug!

@sofisl sofisl closed this as completed Aug 18, 2022
mscheong01 added a commit to mscheong01/armeria that referenced this issue Sep 13, 2022
Currently, googlers confirmed that camelCase should be used for Http Json transcoded apis
googleapis/gax-nodejs#1312
aip-dev/google.aip.dev#934

add a way to serve HttpJsonTranscoding with camelCase parameter keys
mscheong01 added a commit to mscheong01/armeria that referenced this issue Sep 13, 2022
Currently, googlers confirmed that camelCase should be used for Http Json transcoded apis
googleapis/gax-nodejs#1312
aip-dev/google.aip.dev#934

add a way to serve HttpJsonTranscoding with camelCase parameter keys
mscheong01 added a commit to mscheong01/armeria that referenced this issue Sep 13, 2022
Currently, googlers confirmed that camelCase should be used for Http Json transcoded apis
googleapis/gax-nodejs#1312
aip-dev/google.aip.dev#934

add a way to serve HttpJsonTranscoding with camelCase parameter keys
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants