refactor and simplify renderURLSearchParams & flattenObject

generator/template.go

@@ -3,6 +3,7 @@ package generator
 import (
 	"bytes"
 	"fmt"
+	"net/url"
 	"regexp"
 	"strings"
 	"text/template"
@@ -196,89 +197,127 @@ function getNotifyEntityArrivalSink<T>(notifyCallback: NotifyStreamEntityArrival
   })
 }
 
+type Primitive = string | boolean | number;
+type RequestPayload = Record<string, unknown>;
+type FlattenedRequestPayload = Record<string, Primitive | Array<Primitive>>;
+
 /**
- * Flattens a deeply nested object
- *
- * @param  {Object} deeplyNestedObject
+ * Checks if given value is a plain object
+ * source: https://github.com/char0n/ramda-adjunct/blob/master/src/isPlainObj.js
+ * @param  {unknown} value
+ * @return {boolean}
+ */
+function isPlainObject(value: unknown): boolean {
+  const isObject =
+    Object.prototype.toString.call(value).slice(8, -1) === "Object";
+  const isObjLike = value !== null && isObject;
+
+  if (!isObjLike || !isObject) {
+    return false;
+  }
+
+  const proto = Object.getPrototypeOf(value);
+
+  const hasObjectConstructor =
+    typeof proto === "object" &&
+    proto.constructor === Object.prototype.constructor;
+
+  return hasObjectConstructor;
+}
+
+/**
+ * Checks if given value is of a primitive type
+ * @param  {unknown} value
+ * @return {boolean}
+ */
+function isPrimitive(value: unknown): boolean {
+  return ["string", "number", "boolean"].some(t => typeof value === t);
+}
+
+/**
+ * Checks if given primitive is zero-value
+ * @param  {Primitive} value
+ * @return {boolean}
+ */
+function isZeroValuePrimitive(value: Primitive): boolean {
+  return ["", "0", "false"].some(zv => value.toString() === zv);
+}
+
+/**
+ * Flattens a deeply nested request payload and returns an object
+ * with only primitive values and non-empty array of primitive values
+ * as per https://github.com/googleapis/googleapis/blob/master/google/api/http.proto
+ * @param  {RequestPayload} requestPayload
  * @param  {String} path
- * @return {Record<string, unknown>}
+ * @return {FlattenedRequestPayload>}
  */
-function flattenObject<T extends Record<string, unknown>>(
-  deeplyNestedObject: T,
+function flattenRequestPayload<T extends RequestPayload>(
+  requestPayload: T,
   path: string = ""
-): T {
-  return Object.keys(deeplyNestedObject).reduce(
+): FlattenedRequestPayload {
+  return Object.keys(requestPayload).reduce(
     (acc: T, key: string): T => {
-      const value = deeplyNestedObject[key];
-
-      const newPath = Array.isArray(deeplyNestedObject)
-        ? path + "." + key
-        : [path, key].filter(Boolean).join(".");
-      const isObject = [
-        typeof value === "object",
-        value !== null,
-        !(value instanceof Date),
-        !(value instanceof RegExp),
-        !(Array.isArray(value) && value.length === 0)
-      ].every(Boolean);
-
-      return isObject
-        ? { ...acc, ...flattenObject(value as Record<string, unknown>, newPath) }
-        : { ...acc, [newPath]: value };
+      const value = requestPayload[key];
+      const newPath = path ? [path, key].join(".") : key;
+
+      const isNonEmptyPrimitiveArray =
+        Array.isArray(value) &&
+        value.every(v => isPrimitive(v)) &&
+        value.length > 0;
+
+      let objectToMerge = {};
+
+      if (isPlainObject(value)) {
+        objectToMerge = flattenRequestPayload(value as RequestPayload, newPath);
+      } else if (
+        isPrimitive(value) &&
+        !isZeroValuePrimitive(value as Primitive)
+      ) {
+        objectToMerge = { [newPath]: value };
+      } else if (isNonEmptyPrimitiveArray) {
+        const nonEmptyPrimitiveArray = value as Primitive[];
+        objectToMerge = {
+          [newPath]: nonEmptyPrimitiveArray.filter(
+            m => !isZeroValuePrimitive(m)
+          )
+        };
+      }
+
+      return { ...acc, ...objectToMerge };
     },
     {} as T
-  );
+  ) as FlattenedRequestPayload;
 }
 
 /**
- * Renders a deeply nested object into a string of URL search parameters
- * by first flattening the object and then removing non-primitive array keys,
- * non-primitive values and keys which are already present in the URL path.
- * @param  {Object} deeplyNestedObject
- * @param  {Array<string>} urlPathParams
- * @return {String}
+ * Renders a deeply nested request payload into a string of URL search
+ * parameters by first flattening the request payload and then removing keys
+ * which are already present in the URL path.
+ * @param  {RequestPayload} requestPayload
+ * @param  {string[]} urlPathParams
+ * @return {string}
  */
-export function renderURLSearchParams<T extends Record<string, unknown>>(
-  deeplyNestedObject: T,
+export function renderURLSearchParams<T extends RequestPayload>(
+  requestPayload: T,
   urlPathParams: string[] = []
 ): string {
-  const flattenedObject = flattenObject(deeplyNestedObject);
+  const flattenedRequestPayload = flattenRequestPayload(requestPayload);
 
-  const urlSearchParams = Object.keys(flattenedObject).reduce(
+  const urlSearchParams = Object.keys(flattenedRequestPayload).reduce(
     (acc: string[][], key: string): string[][] => {
-      const parts = key.split(".");
-      const index = parts.findIndex(f => Number(f) || f === "0");
-
-      // if key does not contain only one numeric index as the last element
-      // then it is an array of objects
-      if (parts.length === index + 1 || index === -1) {
-        // remove array index to render url search params properly
-        // according to http.proto
-        // https://github.com/googleapis/googleapis/blob/master/google/api/http.proto
-        const keyWithoutArrayIndex =
-          index > -1 ? parts.slice(0, index).join(".") : key;
-
-        // values should only be primitive types and key should not be
-        // present in the url path as a parameter
-        const value = flattenedObject[key];
-        if (
-          (typeof value === "string" ||
-            typeof value === "boolean" ||
-            typeof value === "number") &&
-          !urlPathParams.find(f => f === key)
-        ) {
-          acc = [...acc, [keyWithoutArrayIndex, value.toString()]];
-        }
+      // key should not be present in the url path as a parameter
+      const value = flattenedRequestPayload[key];
+      if (urlPathParams.find(f => f === key)) {
+        return acc;
       }
-
-      return acc;
+      return Array.isArray(value)
+        ? [...acc, ...value.map(m => [key, m.toString()])]
+        : (acc = [...acc, [key, value.toString()]]);
     },
     [] as string[][]
   );
 
-  return urlSearchParams.length > 0
-    ? "?" + new URLSearchParams(urlSearchParams).toString()
-    : "";
+  return new URLSearchParams(urlSearchParams).toString();
 }
 `
 
@@ -314,27 +353,39 @@ func fieldName(r *registry.Registry) func(name string) string {
 func renderURL(r *registry.Registry) func(method data.Method) string {
 	fieldNameFn := fieldName(r)
 	return func(method data.Method) string {
-		url := method.URL
+		methodURL := method.URL
 		reg := regexp.MustCompile("{([^}]+)}")
-		matches := reg.FindAllStringSubmatch(url, -1)
-		fieldList := "["
+		matches := reg.FindAllStringSubmatch(methodURL, -1)
+		fieldsInPath := "["
 		if len(matches) > 0 {
 			log.Debugf("url matches %v", matches)
 			for _, m := range matches {
 				expToReplace := m[0]
 				fieldName := fieldNameFn(m[1])
 				part := fmt.Sprintf(`${req["%s"]}`, fieldName)
-				url = strings.ReplaceAll(url, expToReplace, part)
-				fieldList += fmt.Sprintf(`"%s", `, fieldName)
+				methodURL = strings.ReplaceAll(methodURL, expToReplace, part)
+				fieldsInPath += fmt.Sprintf(`"%s", `, fieldName)
 			}
 		}
-		fieldList = strings.TrimRight(fieldList, ", ") + "]"
+		fieldsInPath = strings.TrimRight(fieldsInPath, ", ") + "]"
 
 		if !method.ClientStreaming && method.HTTPMethod == "GET" {
-			url += fmt.Sprintf("${fm.renderURLSearchParams(req, %s)}", fieldList)
+			// parse the url to check for query string
+			parsedURL, err := url.Parse(methodURL)
+			if err != nil {
+				return methodURL
+			}
+			renderURLSearchParamsFn := fmt.Sprintf("${fm.renderURLSearchParams(req, %s)}", fieldsInPath)
+			// prepend "&" if query string is present otherwise prepend "?"
+			// trim leading "&" if present before prepending it
+			if parsedURL.RawQuery != "" {
+				methodURL += "&" + strings.TrimRight(renderURLSearchParamsFn, "&")
+			} else {
+				methodURL += "?" + renderURLSearchParamsFn
+			}
 		}
 
-		return url
+		return methodURL
 	}
 }
 

integration_tests/integration_test.ts

@@ -53,7 +53,7 @@ describe("test grpc-gateway-ts communication", () => {
   })
 
   it('http get request with url search parameters', async () => {
-    const result = await CounterService.HTTPGetWithURLSearchParams({a: 10, [getFieldName('post_req')]: {b: 11}, c: [23, 25], [getFieldName('ext_msg')]: {d: 12}}, {pathPrefix: "http://localhost:8081"})
-    expect(getField(result, 'url_search_params_result')).to.equal(81)
+    const result = await CounterService.HTTPGetWithURLSearchParams({a: 10, [getFieldName('post_req')]: {b: 0}, c: [23, 25], [getFieldName('ext_msg')]: {d: 12}}, {pathPrefix: "http://localhost:8081"})
+    expect(getField(result, 'url_search_params_result')).to.equal(70)
   })
 })

integration_tests/service.go

@@ -64,10 +64,10 @@ func (r *RealCounterService) HTTPPatch(ctx context.Context, in *HttpPatchRequest
 
 func (r *RealCounterService) HTTPGetWithURLSearchParams(ctx context.Context, in *HTTPGetWithURLSearchParamsRequest) (*HTTPGetWithURLSearchParamsResponse, error) {
 	totalC := 0
-	for _, c := range in.C {
+	for _, c := range in.GetC() {
 		totalC += int(c)
 	}
 	return &HTTPGetWithURLSearchParamsResponse{
-		UrlSearchParamsResult: in.A + in.PostReq.B + in.ExtMsg.D + int32(totalC),
+		UrlSearchParamsResult: in.GetA() + in.PostReq.GetB() + in.ExtMsg.GetD() + int32(totalC),
 	}, nil
 }