Skip to content

Commit

Permalink
[core-client-rest] Addressing feedback from ArchBoard (#18478)
Browse files Browse the repository at this point in the history
* Multiple media type handling

* Add Template for path

* update api-extractor

* Add comments to Resource interface

* Revert path type changes

* Add comment about path type

* Omit lint check for Function type

* rebase main

* Revert confidential-ledger dependency  bump
  • Loading branch information
joheredi authored Nov 3, 2021
1 parent d85d19a commit a097f10
Show file tree
Hide file tree
Showing 10 changed files with 445 additions and 151 deletions.
6 changes: 6 additions & 0 deletions sdk/core/core-client-rest/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 21 additions & 18 deletions sdk/core/core-client-rest/review/core-client.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;

Expand All @@ -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;
Expand All @@ -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>;
}

```
143 changes: 142 additions & 1 deletion sdk/core/core-client-rest/src/common.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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.
[];
40 changes: 1 addition & 39 deletions sdk/core/core-client-rest/src/getClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions sdk/core/core-client-rest/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
60 changes: 0 additions & 60 deletions sdk/core/core-client-rest/src/pathClientTypes.ts

This file was deleted.

2 changes: 1 addition & 1 deletion sdk/core/core-client-rest/src/restError.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading

0 comments on commit a097f10

Please sign in to comment.