Apply code review feedback

GoogleCloudPlatform · May 31, 2023 · 9423f87 · 9423f87
1 parent 342a0cb
commit 9423f87
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 86 deletions.
diff --git a/docs/generated/api.json b/docs/generated/api.json
@@ -851,8 +851,8 @@
             },
             {
               "kind": "Reference",
-              "text": "TypedHandlerFunction",
-              "canonicalReference": "@google-cloud/functions-framework!TypedHandlerFunction:interface"
+              "text": "TypedFunction",
+              "canonicalReference": "@google-cloud/functions-framework!TypedFunction:interface"
             },
             {
               "kind": "Content",
@@ -1009,7 +1009,7 @@
         {
           "kind": "Interface",
           "canonicalReference": "@google-cloud/functions-framework!InvocationFormat:interface",
-          "docComment": "/**\n * The contract for a request deserializer and response serializer.\n */\n",
+          "docComment": "/**\n * The contract for a request deserializer and response serializer.\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -1048,7 +1048,7 @@
             {
               "kind": "MethodSignature",
               "canonicalReference": "@google-cloud/functions-framework!InvocationFormat#deserializeRequest:member(1)",
-              "docComment": "/**\n * @param request - the request body as raw bytes\n *\n * @param headers - the headers received on the HTTP request as a map\n */\n",
+              "docComment": "/**\n * Creates an instance of the request type from an invocation request.\n *\n * @param request - the request body as raw bytes\n */\n",
               "excerptTokens": [
                 {
                   "kind": "Content",
@@ -1103,7 +1103,7 @@
             {
               "kind": "MethodSignature",
               "canonicalReference": "@google-cloud/functions-framework!InvocationFormat#serializeResponse:member(1)",
-              "docComment": "/**\n * @param response - \n *\n * @param responseHeaders - mutable object providing headers that will be set on the response\n */\n",
+              "docComment": "/**\n * Writes the response type to the invocation result.\n *\n * @param responseWriter - interface for writing to the invocation result\n *\n * @param response - the response object\n */\n",
               "excerptTokens": [
                 {
                   "kind": "Content",
@@ -1177,7 +1177,7 @@
         {
           "kind": "Interface",
           "canonicalReference": "@google-cloud/functions-framework!InvocationRequest:interface",
-          "docComment": "/**\n * InvocationRequest represents the properties of an invocation over HTTP.\n */\n",
+          "docComment": "/**\n * InvocationRequest represents the properties of an invocation over HTTP.\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -1273,7 +1273,7 @@
         {
           "kind": "Interface",
           "canonicalReference": "@google-cloud/functions-framework!InvocationResponse:interface",
-          "docComment": "/**\n * InvocationResponse interface describes the properties that can be set on an invocation response.\n */\n",
+          "docComment": "/**\n * InvocationResponse interface describes the properties that can be set on an invocation response.\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -1452,7 +1452,7 @@
         {
           "kind": "Class",
           "canonicalReference": "@google-cloud/functions-framework!JsonInvocationFormat:class",
-          "docComment": "/**\n * Default invocation format for JSON requests.\n */\n",
+          "docComment": "/**\n * Default invocation format for JSON requests.\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -1796,7 +1796,7 @@
         {
           "kind": "Variable",
           "canonicalReference": "@google-cloud/functions-framework!typed:var",
-          "docComment": "/**\n * Register a function that handles strongly typed invocations.\n *\n * @param functionName - the name of the function\n *\n * @param handler - the function to trigger.\n */\n",
+          "docComment": "/**\n * Register a function that handles strongly typed invocations.\n *\n * @param functionName - the name of the function\n *\n * @param handler - the function to trigger\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
@@ -1808,8 +1808,8 @@
             },
             {
               "kind": "Reference",
-              "text": "TypedHandlerFunction",
-              "canonicalReference": "@google-cloud/functions-framework!TypedHandlerFunction:interface"
+              "text": "TypedFunction",
+              "canonicalReference": "@google-cloud/functions-framework!TypedFunction:interface"
             },
             {
               "kind": "Content",
@@ -1836,12 +1836,12 @@
         },
         {
           "kind": "Interface",
-          "canonicalReference": "@google-cloud/functions-framework!TypedHandlerFunction:interface",
-          "docComment": "/**\n * A Typed function handler that may return a value or a promise.\n */\n",
+          "canonicalReference": "@google-cloud/functions-framework!TypedFunction:interface",
+          "docComment": "/**\n * A Typed function handler that may return a value or a promise.\n *\n * @public\n */\n",
           "excerptTokens": [
             {
               "kind": "Content",
-              "text": "export interface TypedHandlerFunction<T = "
+              "text": "export interface TypedFunction<T = "
             },
             {
               "kind": "Content",
@@ -1886,12 +1886,12 @@
               }
             }
           ],
-          "name": "TypedHandlerFunction",
+          "name": "TypedFunction",
           "preserveMemberOrder": false,
           "members": [
             {
               "kind": "PropertySignature",
-              "canonicalReference": "@google-cloud/functions-framework!TypedHandlerFunction#format:member",
+              "canonicalReference": "@google-cloud/functions-framework!TypedFunction#format:member",
               "docComment": "",
               "excerptTokens": [
                 {
@@ -1923,7 +1923,7 @@
             },
             {
               "kind": "PropertySignature",
-              "canonicalReference": "@google-cloud/functions-framework!TypedHandlerFunction#handler:member",
+              "canonicalReference": "@google-cloud/functions-framework!TypedFunction#handler:member",
               "docComment": "",
               "excerptTokens": [
                 {

diff --git a/docs/generated/api.md b/docs/generated/api.md
@@ -59,7 +59,7 @@ export interface EventFunctionWithCallback {
 }
 
 // @public
-export type HandlerFunction<T = unknown, U = unknown> = HttpFunction | EventFunction | EventFunctionWithCallback | CloudEventFunction<T> | CloudEventFunctionWithCallback<T> | TypedHandlerFunction<T, U>;
+export type HandlerFunction<T = unknown, U = unknown> = HttpFunction | EventFunction | EventFunctionWithCallback | CloudEventFunction<T> | CloudEventFunctionWithCallback<T> | TypedFunction<T, U>;
 
 // @public
 export const http: (functionName: string, handler: HttpFunction) => void;
@@ -72,9 +72,7 @@ export interface HttpFunction {
 
 // @public
 export interface InvocationFormat<T, U> {
-    // (undocumented)
     deserializeRequest(request: InvocationRequest): T | Promise<T>;
-    // (undocumented)
     serializeResponse(responseWriter: InvocationResponse, response: U): void | Promise<void>;
 }
 
@@ -121,10 +119,10 @@ export { Request_2 as Request }
 export { Response_2 as Response }
 
 // @public
-export const typed: <T, U>(functionName: string, handler: TypedHandlerFunction<T, U> | ((req: T) => U | Promise<U>)) => void;
+export const typed: <T, U>(functionName: string, handler: TypedFunction<T, U> | ((req: T) => U | Promise<U>)) => void;
 
 // @public
-export interface TypedHandlerFunction<T = unknown, U = unknown> {
+export interface TypedFunction<T = unknown, U = unknown> {
     // (undocumented)
     format: InvocationFormat<T, U>;
     // (undocumented)

diff --git a/src/function_registry.ts b/src/function_registry.ts
@@ -16,7 +16,7 @@ import {
   HttpFunction,
   CloudEventFunction,
   HandlerFunction,
-  TypedHandlerFunction,
+  TypedFunction,
   JsonInvocationFormat,
 } from './functions';
 import {SignatureType} from './types';
@@ -104,11 +104,11 @@ export const cloudEvent = <T = unknown>(
 /**
  * Register a function that handles strongly typed invocations.
  * @param functionName - the name of the function
- * @param handler - the function to trigger.
+ * @param handler - the function to trigger
  */
 export const typed = <T, U>(
   functionName: string,
-  handler: TypedHandlerFunction<T, U>['handler'] | TypedHandlerFunction<T, U>
+  handler: TypedFunction<T, U>['handler'] | TypedFunction<T, U>
 ): void => {
   if (handler instanceof Function) {
     handler = {

diff --git a/src/function_wrappers.ts b/src/function_wrappers.ts
@@ -26,7 +26,7 @@ import {
   CloudEventFunction,
   CloudEventFunctionWithCallback,
   HandlerFunction,
-  TypedHandlerFunction,
+  TypedFunction,
   InvocationRequest,
   InvocationResponse,
 } from './functions';
@@ -206,35 +206,7 @@ const wrapEventFunctionWithCallback = (
  * @param userFunction User's function
  * @return An Express handler function that invokes the user function
  */
-const wrapTypedFunction = (
-  typedFunction: TypedHandlerFunction
-): RequestHandler => {
-  class InvocationRequestImpl implements InvocationRequest {
-    constructor(private req: Request) {}
-
-    body(): string | Buffer {
-      return this.req.body;
-    }
-
-    header(header: string): string | undefined {
-      return this.req.header(header);
-    }
-  }
-
-  class InvocationResponseImpl implements InvocationResponse {
-    constructor(private res: Response) {}
-
-    setHeader(key: string, value: string): void {
-      this.res.set(key, value);
-    }
-    write(data: string | Buffer): void {
-      this.res.write(data);
-    }
-    end(data: string | Buffer): void {
-      this.res.end(data);
-    }
-  }
-
+const wrapTypedFunction = (typedFunction: TypedFunction): RequestHandler => {
   const typedHandlerWrapper: HttpFunction = async (
     req: Request,
     res: Response
@@ -298,6 +270,38 @@ export const wrapUserFunction = <T = unknown>(
       }
       return wrapCloudEventFunction(userFunction as CloudEventFunction);
     case 'typed':
-      return wrapTypedFunction(userFunction as TypedHandlerFunction);
+      return wrapTypedFunction(userFunction as TypedFunction);
   }
 };
+
+/**
+ * @private
+ */
+class InvocationRequestImpl implements InvocationRequest {
+  constructor(private req: Request) {}
+
+  body(): string | Buffer {
+    return this.req.body;
+  }
+
+  header(header: string): string | undefined {
+    return this.req.header(header);
+  }
+}
+
+/**
+ * @private
+ */
+class InvocationResponseImpl implements InvocationResponse {
+  constructor(private res: Response) {}
+
+  setHeader(key: string, value: string): void {
+    this.res.set(key, value);
+  }
+  write(data: string | Buffer): void {
+    this.res.write(data);
+  }
+  end(data: string | Buffer): void {
+    this.res.end(data);
+  }
+}
diff --git a/src/functions.ts b/src/functions.ts
@@ -77,8 +77,9 @@ export interface CloudEventFunctionWithCallback<T = unknown> {
 
 /**
  * A Typed function handler that may return a value or a promise.
+ * @public
  */
-export interface TypedHandlerFunction<T = unknown, U = unknown> {
+export interface TypedFunction<T = unknown, U = unknown> {
   handler: (req: T) => U | Promise<U>;
   format: InvocationFormat<T, U>;
 }
@@ -93,7 +94,7 @@ export type HandlerFunction<T = unknown, U = unknown> =
   | EventFunctionWithCallback
   | CloudEventFunction<T>
   | CloudEventFunctionWithCallback<T>
-  | TypedHandlerFunction<T, U>;
+  | TypedFunction<T, U>;
 
 /**
  * A legacy event.
@@ -150,6 +151,7 @@ export type Context = CloudFunctionsContext | CloudEvent<unknown>;
 
 /**
  * InvocationRequest represents the properties of an invocation over HTTP.
+ * @public
  */
 export interface InvocationRequest {
   /** Returns the request body as either a string or a Buffer if the body is binary. */
@@ -161,6 +163,7 @@ export interface InvocationRequest {
 /**
  * InvocationResponse interface describes the properties that can be set on
  * an invocation response.
+ * @public
  */
 export interface InvocationResponse {
   /** Sets a header on the response. */
@@ -173,18 +176,21 @@ export interface InvocationResponse {
 
 /**
  * The contract for a request deserializer and response serializer.
+ * @public
  */
 export interface InvocationFormat<T, U> {
   /**
+   * Creates an instance of the request type from an invocation request.
    *
    * @param request the request body as raw bytes
-   * @param headers the headers received on the HTTP request as a map
    */
   deserializeRequest(request: InvocationRequest): T | Promise<T>;
+
   /**
+   * Writes the response type to the invocation result.
    *
-   * @param response
-   * @param responseHeaders mutable object providing headers that will be set on the response
+   * @param responseWriter interface for writing to the invocation result
+   * @param response the response object
    */
   serializeResponse(
     responseWriter: InvocationResponse,
@@ -194,11 +200,13 @@ export interface InvocationFormat<T, U> {
 
 /**
  * Default invocation format for JSON requests.
+ * @public
  */
 export class JsonInvocationFormat<T, U> implements InvocationFormat<T, U> {
   deserializeRequest(request: InvocationRequest): T {
     const body = request.body();
     if (typeof body !== 'string') {
+      console.log(typeof body);
       throw new Error('Unsupported Content-Type, expected application/json');
     }
     try {

diff --git a/src/server.ts b/src/server.ts
@@ -85,24 +85,18 @@ export function getServer(
   };
 
   // Apply middleware
-  if (functionSignatureType === 'typed') {
-    app.use(
-      bodyParser.text({
-        limit: requestLimit,
-        type: '*/json',
-      })
-    );
-    app.use(
-      bodyParser.text({
-        limit: requestLimit,
-        type: 'text/*',
-      })
-    );
-  } else {
+  if (functionSignatureType !== 'typed') {
+    // If the function is not typed then JSON parsing can be done automatically, otherwise the
+    // functions format must determine deserialization.
     app.use(bodyParser.json(cloudEventsBodySavingOptions));
     app.use(bodyParser.json(defaultBodySavingOptions));
-    app.use(bodyParser.text(defaultBodySavingOptions));
+  } else {
+    const jsonParserOptions = Object.assign({}, defaultBodySavingOptions, {
+      type: 'application/json',
+    });
+    app.use(bodyParser.text(jsonParserOptions));
   }
+  app.use(bodyParser.text(defaultBodySavingOptions));
   app.use(bodyParser.urlencoded(urlEncodedOptions));
   // The parser will process ALL content types so MUST come last.
   // Subsequent parsers will be skipped when one is matched.
@@ -134,7 +128,7 @@ export function getServer(
   }
 
   if (functionSignatureType === 'http') {
-    app.use('/favicon.ico|/robots.txt', (req: express.Request, res) => {
+    app.use('/favicon.ico|/robots.txt', (req, res) => {
       // Neither crawlers nor browsers attempting to pull the icon find the body
       // contents particularly useful, so we send nothing in the response body.
       res.status(404).send(null);