Handle rendering of URL Query Parameters in renderURL

[atreya2011]

Resolves #9

TODO:

• Create a TypeScript function in template to build URL query parameters
• Handle nested message types.
• Handle primitive repeated fields.

[atreya2011]

@lyonlai

What I finally ended up doing was:

1. Create a TypeScript function, in the template to build URL query parameters, that handles nested message types & primitive repeated fields along with optional fields.
2. First I pass the deeply nested object and the list of fields present in the URL path to renderURLSearchParams function present within the template.
3. In this function, the deeply nested object is flattened using the flattenObject function and then the query string is built using the rules present in the http.proto while excluding the list of fields present in the URL path as parameters.

It returns an empty string if there are no URL query parameters to be rendered.

I have also written a small integration test to check if the query parameters are being rendered and passed properly.

Looking forward to your feedback 🙏🏼

P.S. I have added the following to gen-server-proto.sh to ensure that the proper tool binaries will always be installed before generating the server-side go files.

# remove binaries to ensure that binaries present in tools.go are installed
rm -f $GOBIN/protoc-gen-go $GOBIN/protoc-gen-grpc-gateway $GOBIN/protoc-gen-swagger

go install \
	github.com/golang/protobuf/protoc-gen-go \
	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway \
	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger

[lyonlai]

I like the flattenObject idea, this makes runtime to pickup the fields being set. And not to grow the generated asset size too much.

Suggestions have been made. Looking forward to it.

[atreya2011]

@lyonlai
I have revised the code based on your feedback and suggestions. However, I had difficulty in implementing Object.keys(flattenedObject).filter(keysInPath).map() because unfortunately, it becomes complicated to return an array of type string[][] to be used in new URLSearchParams(), so I simplified the Object.reduce function.

For example, lets take the following request payload where we filter out key a assuming it is present in the path as a parameter:

const req = {
  a: 10,
  postReq: {
    b: 11
  },
  c: [23, 25],
  extMsg: {
    d: 12
  }
};

After flattening and filtering key a it becomes like this:

{
  "postReq.b": 11,
  c: [23, 25],
  "extMsg.d": 12
};

The above flattened request payload needs to be converted like this to use as input in new URLSearchParams():

[["postReq.b", 11], ["c", 23], ["c", 25], ["extMsg.d", 12]]

However, it becomes complicated to use just Array.map on the filtered and flattened to return an array of type string[][] like above.

This is the implementation I tried which does not return [["postReq.b", 11], ["c", 23], ["c", 25], ["extMsg.d", 12]]:
It returns [["postReq.b", 11], [["c", 23], ["c", 25]], ["extMsg.d", 12]] which is not useable in new URLSearchParams()

Object.keys(flattenedRequestPayload)
    .filter(key => !urlPathParams.includes(key))
    .map(key => {
      const value = flattenedRequestPayload[key];
      if (Array.isArray(value)) {
        return value.map(v => [key, v.toString()]);
      }
      return [key, value.toString()];
    });

Looking forward to your feedback 🙇🏼

[lyonlai]

it definitely looks nicer on this pass. there are few changes & cases to cover as per comment. Good job.

[atreya2011]

I have refactored the code according to your suggestions and added a test for checking falsy values as well 🙇🏼

[atreya2011]

Thank you once again for taking your time in reviewing this. I have furthered fixed the code according to your suggestions 🙇🏼
Let me know if I have missed anything 🙏🏼

[lyonlai]

Last two comments. This is looking really good. sorry for missing the fieldsInPath construction before.

Also when you finished, can you also please rebase to a single commit and push up?

[atreya2011]

I have added the notes here. Looking forward to your feedback 🙇🏼

[lyonlai]

This is too long. The note section is supposed to note the things people unexpected to find, and also supposed to be concise. In the http get case, it is about the field value matches the default value won't be presented in url params. and a short example like the following will be enough .

{fieldA: "A", fieldB: "" fieldC:1} will turned in the /path?filedA=A&fieldC=1

[atreya2011]

Hmm... ok I will try my best to make it more concise. Thank you for the feedback 🙏🏼

[atreya2011]

Sorry for the delay. I have simplified the notes section 🙇🏼

[lyonlai]

This is too long. The note section is supposed to note the things people unexpected to find, and also supposed to be concise. In the http get case, it is about the field value matches the default value won't be presented in url params. and a short example like the following will be enough .

{fieldA: "A", fieldB: "" fieldC:1} will turned in the /path?filedA=A&fieldC=1

[lyonlai]

protos in the integration test already covers this

[atreya2011]

So maybe we can point out in the notes to look for the integration test example?

[lyonlai]

I think so. just point a link to the integration test service.proto will do the work.

[atreya2011]

Understood. Thanks for the feedback 🙇🏼

[atreya2011]

Completed 🙏🏼

[lyonlai]

It will be good to have the url example here

[atreya2011]

Ah ok, you mean the URL example from the notes section in the README?

[lyonlai]

just a url example will be ok. no need to move the whole thing here.

[atreya2011]

I understand, just a simple request and response message and a simple RPC implementation.

[atreya2011]

I have modified the tests with the URL example. Please let me know if this good or if I have missed something 🙇🏼

[atreya2011]

Last two comments. This is looking really good. sorry for missing the fieldsInPath construction before.

Also when you finished, can you also please rebase to a single commit and push up?

Also rebased to a single commit 🙇🏼

[lyonlai]

LGTM, thanks for the contribution @atreya2011

[atreya2011]

LGTM, thanks for the contribution @atreya2011

@lyonlai
Likewise, thank you so much for your patience in reviewing my PR and letting me make this contribution 🙇🏼