Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core-https][core-client] Core v2 cleanup work #13266

Merged
merged 9 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sdk/core/core-client/review/core-client.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export interface OperationQueryParameter extends OperationParameter {
}

// @public
export type OperationRequest = PipelineRequest<OperationRequestInfo>;
export type OperationRequest = PipelineRequest;

// @public
export interface OperationRequestInfo {
Expand Down
14 changes: 9 additions & 5 deletions sdk/core/core-client/src/deserializationPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from "./interfaces";
import { MapperTypeNames } from "./serializer";
import { isStreamOperation } from "./interfaceHelpers";
import { getOperationRequestInfo } from "./operationHelpers";

const defaultJsonContentTypes = ["application/json", "text/json"];
const defaultXmlContentTypes = ["application/xml", "application/atom+xml"];
Expand Down Expand Up @@ -104,20 +105,22 @@ function getOperationResponseMap(
): undefined | OperationResponseMap {
let result: OperationResponseMap | undefined;
const request: OperationRequest = parsedResponse.request;
const operationSpec = request.additionalInfo?.operationSpec;
const operationInfo = getOperationRequestInfo(request);
const operationSpec = operationInfo?.operationSpec;
if (operationSpec) {
if (!request.additionalInfo?.operationResponseGetter) {
if (!operationInfo?.operationResponseGetter) {
result = operationSpec.responses[parsedResponse.status];
} else {
result = request.additionalInfo?.operationResponseGetter(operationSpec, parsedResponse);
result = operationInfo?.operationResponseGetter(operationSpec, parsedResponse);
}
}
return result;
}

function shouldDeserializeResponse(parsedResponse: PipelineResponse): boolean {
const request: OperationRequest = parsedResponse.request;
const shouldDeserialize = request.additionalInfo?.shouldDeserialize;
const operationInfo = getOperationRequestInfo(request);
const shouldDeserialize = operationInfo?.shouldDeserialize;
let result: boolean;
if (shouldDeserialize === undefined) {
result = true;
Expand Down Expand Up @@ -147,7 +150,8 @@ async function deserializeResponseBody(
return parsedResponse;
}

const operationSpec = parsedResponse.request.additionalInfo?.operationSpec;
const operationInfo = getOperationRequestInfo(parsedResponse.request);
const operationSpec = operationInfo?.operationSpec;
if (!operationSpec || !operationSpec.responses) {
return parsedResponse;
}
Expand Down
5 changes: 2 additions & 3 deletions sdk/core/core-client/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ export type RequiredSerializerOptions = {
};

/**
* This interface extends a generic `PipelineRequest` to include
* additional metadata about the request.
* A type alias for future proofing.
*/
export type OperationRequest = PipelineRequest<OperationRequestInfo>;
export type OperationRequest = PipelineRequest;

/**
* Metadata that is used to properly parse a response.
Expand Down
16 changes: 15 additions & 1 deletion sdk/core/core-client/src/operationHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import {
OperationParameter,
Mapper,
CompositeMapper,
ParameterPath
ParameterPath,
OperationRequestInfo,
OperationRequest
} from "./interfaces";

/**
Expand Down Expand Up @@ -103,3 +105,15 @@ function getPropertyFromParameterPath(
}
return result;
}

const operationRequestMap = new WeakMap<OperationRequest, OperationRequestInfo>();

export function getOperationRequestInfo(request: OperationRequest): OperationRequestInfo {
let info = operationRequestMap.get(request);

if (!info) {
info = {};
operationRequestMap.set(request, info);
}
return info;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you setting info anywhere else? it looks like it will always be {}?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We assign properties to this object in serviceClient.ts lines 153 and 154.

}
10 changes: 7 additions & 3 deletions sdk/core/core-client/src/serializationPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import {
} from "./interfaces";
import { MapperTypeNames } from "./serializer";
import { getPathStringFromParameter } from "./interfaceHelpers";
import { getOperationArgumentValueFromParameter } from "./operationHelpers";
import {
getOperationArgumentValueFromParameter,
getOperationRequestInfo
} from "./operationHelpers";

/**
* The programmatic identifier of the serializationPolicy.
Expand Down Expand Up @@ -47,8 +50,9 @@ export function serializationPolicy(options: serializationPolicyOptions = {}): P
return {
name: serializationPolicyName,
async sendRequest(request: OperationRequest, next: SendRequest): Promise<PipelineResponse> {
const operationSpec = request.additionalInfo?.operationSpec;
const operationArguments = request.additionalInfo?.operationArguments;
const operationInfo = getOperationRequestInfo(request);
const operationSpec = operationInfo?.operationSpec;
const operationArguments = operationInfo?.operationArguments;
if (operationSpec && operationArguments) {
serializeHeaders(request, operationArguments, operationSpec);
serializeRequestBody(request, operationArguments, operationSpec, stringifyXML);
Expand Down
9 changes: 5 additions & 4 deletions sdk/core/core-client/src/serviceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import { deserializationPolicy, DeserializationPolicyOptions } from "./deseriali
import { URL } from "./url";
import { serializationPolicy, serializationPolicyOptions } from "./serializationPolicy";
import { getCachedDefaultHttpsClient } from "./httpClientCache";
import { getOperationRequestInfo } from "./operationHelpers";

/**
* Options to be provided while creating the client.
Expand Down Expand Up @@ -148,9 +149,9 @@ export class ServiceClient {
url
});
request.method = operationSpec.httpMethod;
request.additionalInfo = {};
request.additionalInfo.operationSpec = operationSpec;
request.additionalInfo.operationArguments = operationArguments;
const operationInfo = getOperationRequestInfo(request);
operationInfo.operationSpec = operationSpec;
operationInfo.operationArguments = operationArguments;

const contentType = operationSpec.contentType || this._requestContentType;
if (contentType && operationSpec.requestBody) {
Expand Down Expand Up @@ -181,7 +182,7 @@ export class ServiceClient {
}

if (requestOptions.shouldDeserialize !== undefined) {
request.additionalInfo.shouldDeserialize = requestOptions.shouldDeserialize;
operationInfo.shouldDeserialize = requestOptions.shouldDeserialize;
}
}

Expand Down
6 changes: 3 additions & 3 deletions sdk/core/core-client/test/deserializationPolicy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
RawHttpHeaders
} from "@azure/core-https";
import { parseXML } from "@azure/core-xml";
import { getOperationRequestInfo } from "../src/operationHelpers";

describe("deserializationPolicy", function() {
it(`should not modify a request that has no request body mapper`, async function() {
Expand Down Expand Up @@ -709,9 +710,8 @@ async function getDeserializedResponse(
serializerOptions: options.serializerOptions
});
const request: OperationRequest = createPipelineRequest({ url: "https://example.com" });
request.additionalInfo = {
operationSpec: options.operationSpec
};
const operationInfo = getOperationRequestInfo(request);
operationInfo.operationSpec = options.operationSpec;
request.body = options.requestBody;

const res: PipelineResponse = {
Expand Down
10 changes: 6 additions & 4 deletions sdk/core/core-client/test/serviceClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import {
createPipelineRequest
} from "@azure/core-https";

import { getOperationArgumentValueFromParameter } from "../src/operationHelpers";
import {
getOperationArgumentValueFromParameter,
getOperationRequestInfo
} from "../src/operationHelpers";
import { deserializationPolicy } from "../src/deserializationPolicy";
import { TokenCredential } from "@azure/core-auth";
import { getCachedDefaultHttpsClient } from "../src/httpClientCache";
Expand Down Expand Up @@ -785,9 +788,8 @@ describe("ServiceClient", function() {
};

let request: OperationRequest = createPipelineRequest({ url: "https://example.com" });
request.additionalInfo = {
operationSpec
};
const operationInfo = getOperationRequestInfo(request);
operationInfo.operationSpec = operationSpec;

const httpsClient: HttpsClient = {
sendRequest: (req) => {
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-https/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"module": "dist-esm/src/index.js",
"browser": {
"./dist-esm/src/defaultHttpsClient.js": "./dist-esm/src/defaultHttpsClient.browser.js",
"./dist-esm/src/policies/disableResponseDecompressionPolicy.js": "./dist-esm/src/policies/disableResponseDecompressionPolicy.browser.js",
"./dist-esm/src/policies/decompressResponsePolicy.js": "./dist-esm/src/policies/decompressResponsePolicy.browser.js",
"./dist-esm/src/policies/formDataPolicy.js": "./dist-esm/src/policies/formDataPolicy.browser.js",
"./dist-esm/src/policies/proxyPolicy.js": "./dist-esm/src/policies/proxyPolicy.browser.js",
"./dist-esm/src/util/inspect.js": "./dist-esm/src/util/inspect.browser.js",
Expand Down
40 changes: 8 additions & 32 deletions sdk/core/core-https/review/core-https.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,15 @@ export function createPipelineFromOptions(options: InternalPipelineOptions): Pip
export function createPipelineRequest(options: PipelineRequestOptions): PipelineRequest;

// @public
export class DefaultHttpsClient implements HttpsClient {
sendRequest(request: PipelineRequest): Promise<PipelineResponse>;
}
export function decompressResponsePolicy(): PipelinePolicy;

// @public
export function disableResponseDecompressionPolicy(): PipelinePolicy;
export const decompressResponsePolicyName = "decompressResponsePolicy";

// @public
export const disableResponseDecompressionPolicyName = "disableResponseDecompressionPolicy";
export class DefaultHttpsClient implements HttpsClient {
sendRequest(request: PipelineRequest): Promise<PipelineResponse>;
}

// @public
export function exponentialRetryPolicy(options?: ExponentialRetryPolicyOptions): PipelinePolicy;
Expand Down Expand Up @@ -84,7 +84,6 @@ export function getDefaultProxySettings(proxyUrl?: string): ProxySettings | unde

// @public
export interface HttpHeaders extends Iterable<[string, string]> {
clone(): HttpHeaders;
delete(name: string): void;
get(name: string): string | undefined;
has(name: string): boolean;
Expand All @@ -102,20 +101,7 @@ export interface HttpsClient {

// @public
export interface InternalPipelineOptions extends PipelineOptions {
decompressResponse?: boolean;
loggingOptions?: LogPolicyOptions;
sendStreamingJson?: boolean;
}

// @public
export function keepAlivePolicy(options?: KeepAlivePolicyOptions): PipelinePolicy;

// @public
export const keepAlivePolicyName = "keepAlivePolicy";

// @public
export interface KeepAlivePolicyOptions {
enable?: boolean;
}

// @public
Expand Down Expand Up @@ -152,9 +138,8 @@ export interface Pipeline {
// @public
export interface PipelineOptions {
httpsClient?: HttpsClient;
keepAliveOptions?: KeepAlivePolicyOptions;
proxyOptions?: ProxySettings;
redirectOptions?: PipelineRedirectOptions;
redirectOptions?: RedirectPolicyOptions;
retryOptions?: ExponentialRetryPolicyOptions;
userAgentOptions?: UserAgentPolicyOptions;
}
Expand All @@ -169,16 +154,9 @@ export interface PipelinePolicy {
}

// @public
export interface PipelineRedirectOptions extends RedirectPolicyOptions {
disable?: boolean;
}

// @public
export interface PipelineRequest<AdditionalInfo = any> {
export interface PipelineRequest {
abortSignal?: AbortSignalLike;
additionalInfo?: AdditionalInfo;
body?: RequestBodyType;
clone(): PipelineRequest;
formData?: FormDataMap;
headers: HttpHeaders;
keepAlive?: boolean;
Expand All @@ -187,7 +165,6 @@ export interface PipelineRequest<AdditionalInfo = any> {
onUploadProgress?: (progress: TransferProgressEvent) => void;
proxySettings?: ProxySettings;
requestId: string;
skipDecompressResponse?: boolean;
spanOptions?: SpanOptions;
streamResponseBody?: boolean;
timeout: number;
Expand All @@ -196,9 +173,8 @@ export interface PipelineRequest<AdditionalInfo = any> {
}

// @public
export interface PipelineRequestOptions<AdditionalInfo = any> {
export interface PipelineRequestOptions {
abortSignal?: AbortSignalLike;
additionalInfo?: AdditionalInfo;
body?: RequestBodyType;
formData?: FormDataMap;
headers?: HttpHeaders;
Expand Down
7 changes: 0 additions & 7 deletions sdk/core/core-https/src/httpHeaders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,6 @@ class HttpHeadersImpl implements HttpHeaders {
return JSON.stringify(this.toJSON());
}

/**
* Create a deep clone/copy of this HttpHeaders collection.
*/
public clone(): HttpHeaders {
return new HttpHeadersImpl(this.toJSON());
}

/**
* Iterate over tuples of header [name, value] pairs.
*/
Expand Down
12 changes: 3 additions & 9 deletions sdk/core/core-https/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,16 @@ export {
createEmptyPipeline,
InternalPipelineOptions,
PipelineOptions,
PipelineRedirectOptions,
createPipelineFromOptions
} from "./pipeline";
export { DefaultHttpsClient } from "./defaultHttpsClient";
export { createHttpHeaders } from "./httpHeaders";
export { createPipelineRequest, PipelineRequestOptions } from "./pipelineRequest";
export { RestError, RestErrorOptions } from "./restError";
export {
disableResponseDecompressionPolicy,
disableResponseDecompressionPolicyName
} from "./policies/disableResponseDecompressionPolicy";
decompressResponsePolicy,
decompressResponsePolicyName
} from "./policies/decompressResponsePolicy";
export {
exponentialRetryPolicy,
ExponentialRetryPolicyOptions,
Expand All @@ -43,11 +42,6 @@ export {
setClientRequestIdPolicy,
setClientRequestIdPolicyName
} from "./policies/setClientRequestIdPolicy";
export {
keepAlivePolicy,
keepAlivePolicyName,
KeepAlivePolicyOptions
} from "./policies/keepAlivePolicy";
export { logPolicy, logPolicyName, LogPolicyOptions } from "./policies/logPolicy";
export { proxyPolicy, proxyPolicyName, getDefaultProxySettings } from "./policies/proxyPolicy";
export {
Expand Down
Loading