[core-client-rest] Addressing feedback from ArchBoard

sdk/core/core-client-rest/CHANGELOG.md

@@ -5,6 +5,12 @@
 
 - Add options skipUrlEncoding to support skip path parameter encoding.
 
+## 1.0.0-beta.8 (UNRELEASED)
+
+- Adding more robust handling of request and response body. [#18478](https://github.com/Azure/azure-sdk-for-js/pull/18478)
+
+### Other Changes
+
 ## 1.0.0-beta.7 (2021-09-02)
 
 ### Other Changes

sdk/core/core-client-rest/review/core-client.api.md

@@ -22,16 +22,7 @@ export interface CertificateCredential {
 // @public
 export interface Client {
     path: Function;
-    pathUnchecked: (path: string, ...args: Array<any>) => {
-        get: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        post: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        put: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        patch: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        delete: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        head: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        options: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-        trace: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    };
+    pathUnchecked: PathUnchecked;
     pipeline: Pipeline;
 }
 
@@ -46,9 +37,6 @@ export type ClientOptions = PipelineOptions & {
     allowInsecureConnection?: boolean;
 };
 
-// @public
-export function createDefaultPipeline(baseUrl: string, credential?: TokenCredential | KeyCredential, options?: ClientOptions): Pipeline;
-
 // @public
 export function createRestError(message: string, response: PathUncheckedResponse): RestError;
 
@@ -69,6 +57,16 @@ export type HttpResponse = {
 // @public
 export function isCertificateCredential(credential: unknown): credential is CertificateCredential;
 
+// @public
+export type PathParameters<TRoute extends string> = TRoute extends `${infer _Head}/{${infer _Param}}${infer Tail}` ? [
+pathParameter: string,
+...pathParameters: PathParameters<Tail>
+] : [
+];
+
+// @public
+export type PathUnchecked = <TPath extends string>(path: TPath, ...args: PathParameters<TPath>) => ResourceMethods;
+
 // @public
 export type PathUncheckedResponse = HttpResponse & {
     body: any;
@@ -86,10 +84,15 @@ export type RequestParameters = {
 };
 
 // @public
-export type RouteParams<TRoute extends string> = TRoute extends `${infer _Head}/{${infer _Param}}${infer Tail}` ? [
-pathParam: string,
-...pathParams: RouteParams<Tail>
-] : [
-];
+export interface ResourceMethods {
+    delete: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    get: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    head: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    options: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    patch: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    post: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    put: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+    trace: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+}
 
 ```

sdk/core/core-client-rest/src/common.ts

@@ -1,7 +1,124 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PipelineOptions, PipelineRequest, RawHttpHeaders } from "@azure/core-rest-pipeline";
+import {
+  Pipeline,
+  PipelineOptions,
+  PipelineRequest,
+  RawHttpHeaders,
+} from "@azure/core-rest-pipeline";
+import { RawHttpHeadersInput } from "@azure/core-rest-pipeline";
+
+/**
+ * Shape of the default request parameters, this may be overriden by the specific
+ * request types to provide strong types
+ */
+export type RequestParameters = {
+  /**
+   * Headers to send along with the request
+   */
+  headers?: RawHttpHeadersInput;
+  /**
+   * Sets the accept header to send to the service
+   * defaults to 'application/json'. If also a header "accept" is set
+   * this property will take precedence.
+   */
+  accept?: string;
+  /**
+   * Body to send with the request
+   */
+  body?: unknown;
+  /**
+   * Query parameters to send with the request
+   */
+  queryParameters?: Record<string, unknown>;
+  /**
+   * Set an explicit content-type to send with the request. If also a header "content-type" is set
+   * this property will take precedence.
+   */
+  contentType?: string;
+  /** Set to true if the request is sent over HTTP instead of HTTPS */
+  allowInsecureConnection?: boolean;
+  /** Set to true if you want to skip encoding the path parameters */
+  skipUrlEncoding?: boolean;
+};
+
+/**
+ * Type to use with pathUnchecked, overrides the body type to any to allow flexibility
+ */
+export type PathUncheckedResponse = HttpResponse & { body: any };
+
+/**
+ * Shape of a Rest Level Client
+ */
+export interface Client {
+  /**
+   * The pipeline used by this client to make requests
+   */
+  pipeline: Pipeline;
+  /**
+   * This method will be used to send request that would check the path to provide
+   * strong types. When used by the codegen this type gets overriden wit the generated
+   * types. For example:
+   * ```typescript
+   * export type MyClient = Client & {
+   *    path: Routes;
+   * }
+   * ```
+   */
+  // eslint-disable-next-line @typescript-eslint/ban-types
+  path: Function;
+  /**
+   * This method allows arbitrary paths and doesn't provide strong types
+   */
+  pathUnchecked: PathUnchecked;
+}
+
+/**
+ * Defines the signature for pathUnchecked.
+ */
+export type PathUnchecked = <TPath extends string>(
+  path: TPath,
+  ...args: PathParameters<TPath>
+) => ResourceMethods;
+
+/**
+ * Defines the methods that can be called on a resource
+ */
+export interface ResourceMethods {
+  /**
+   * Definition of the GET HTTP method for a resource
+   */
+  get: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the POST HTTP method for a resource
+   */
+  post: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the PUT HTTP method for a resource
+   */
+  put: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the PATCH HTTP method for a resource
+   */
+  patch: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the DELETE HTTP method for a resource
+   */
+  delete: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the HEAD HTTP method for a resource
+   */
+  head: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the OPTIONS HTTP method for a resource
+   */
+  options: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+  /**
+   * Definition of the TRACE HTTP method for a resource
+   */
+  trace: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
+}
 
 /**
  * General options that a Rest Level Client can take
@@ -55,3 +172,27 @@ export type HttpResponse = {
    */
   status: string;
 };
+
+/**
+ * Helper type used to detect parameters in a path template
+ * text surrounded by \{\} will be considered a path parameter
+ */
+export type PathParameters<
+  TRoute extends string
+  // This is trying to match the string in TRoute with a template where HEAD/{PARAM}/TAIL
+  // for example in the followint path: /foo/{fooId}/bar/{barId}/baz the template will infer
+  // HEAD: /foo
+  // Param: fooId
+  // Tail: /bar/{barId}/baz
+  // The above sample path would return [pathParam: string, pathParam: string]
+> = TRoute extends `${infer _Head}/{${infer _Param}}${infer Tail}`
+  ? // In case we have a match for the template above we know for sure
+    // that we have at least one pathParameter, that's why we set the first pathParam
+    // in the tuple. At this point we have only matched up until param, if we want to identify
+    // additional parameters we can call RouteParameters recursively on the Tail to match the remaining parts,
+    // in case the Tail has more parameters, it will return a tuple with the parameters found in tail.
+    // We spread the second path params to end up with a single dimension tuple at the end.
+    [pathParameter: string, ...pathParameters: PathParameters<Tail>]
+  : // When the path doesn't match the template, it means that we have no path parameters so we return
+    // an empty tuple.
+    [];

sdk/core/core-client-rest/src/getClient.ts

@@ -5,48 +5,10 @@ import { isTokenCredential, KeyCredential, TokenCredential } from "@azure/core-a
 import { isCertificateCredential } from "./certificateCredential";
 import { HttpMethods, Pipeline, PipelineOptions } from "@azure/core-rest-pipeline";
 import { createDefaultPipeline } from "./clientHelpers";
-import { ClientOptions, HttpResponse } from "./common";
-import { RequestParameters } from "./pathClientTypes";
+import { Client, ClientOptions, HttpResponse, RequestParameters } from "./common";
 import { sendRequest } from "./sendRequest";
 import { buildRequestUrl } from "./urlHelpers";
 
-/**
- * Type to use with pathUnchecked, overrides the body type to any to allow flexibility
- */
-export type PathUncheckedResponse = HttpResponse & { body: any };
-
-/**
- * Shape of a Rest Level Client
- */
-export interface Client {
-  /**
-   * The pipeline used by this client to make requests
-   */
-  pipeline: Pipeline;
-  /**
-   * This method will be used to send request that would check the path to provide
-   * strong types
-   */
-  // eslint-disable-next-line @typescript-eslint/ban-types
-  path: Function;
-  /**
-   * This method allows arbitrary paths and doesn't provide strong types
-   */
-  pathUnchecked: (
-    path: string,
-    ...args: Array<any>
-  ) => {
-    get: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    post: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    put: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    patch: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    delete: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    head: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    options: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-    trace: (options?: RequestParameters) => Promise<PathUncheckedResponse>;
-  };
-}
-
 /**
  * Creates a client with a default pipeline
  * @param baseUrl - Base endpoint for the client

sdk/core/core-client-rest/src/index.ts

@@ -6,9 +6,7 @@
  * @packageDocumentation
  */
 
-export { createDefaultPipeline } from "./clientHelpers";
 export { CertificateCredential, isCertificateCredential } from "./certificateCredential";
 export { createRestError } from "./restError";
-export * from "./common";
 export * from "./getClient";
-export * from "./pathClientTypes";
+export * from "./common";

sdk/core/core-client-rest/src/restError.ts

@@ -1,8 +1,8 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
-import { PathUncheckedResponse } from "./getClient";
 import { RestError, PipelineResponse, createHttpHeaders } from "@azure/core-rest-pipeline";
+import { PathUncheckedResponse } from "./common";
 
 /**
  * Creates a rest error from a PathUnchecked response