diff --git a/sdk/core/core-client/review/core-client.api.md b/sdk/core/core-client/review/core-client.api.md index fa6bd200e2c2..12381c2c7424 100644 --- a/sdk/core/core-client/review/core-client.api.md +++ b/sdk/core/core-client/review/core-client.api.md @@ -7,6 +7,7 @@ import { AbortSignalLike } from '@azure/abort-controller'; import { HttpMethods } from '@azure/core-https'; import { HttpsClient } from '@azure/core-https'; +import { InternalPipelineOptions } from '@azure/core-https'; import { OperationTracingOptions } from '@azure/core-tracing'; import { Pipeline } from '@azure/core-https'; import { PipelinePolicy } from '@azure/core-https'; @@ -34,6 +35,15 @@ export interface BaseMapper { xmlNamespacePrefix?: string; } +// @public +export interface ClientPipelineOptions extends InternalPipelineOptions { + credentialOptions?: { + baseUri?: string; + credential?: TokenCredential; + }; + deserializationOptions?: DeserializationPolicyOptions; +} + // @public (undocumented) export interface CompositeMapper extends BaseMapper { // (undocumented) @@ -58,6 +68,9 @@ export interface CompositeMapperType { uberParent?: string; } +// @public +export function createClientPipeline(options?: ClientPipelineOptions): Pipeline; + // @public export function createSerializer(modelMappers?: { [key: string]: any; @@ -324,6 +337,9 @@ export interface ServiceClientOptions { baseUri?: string; credential?: TokenCredential; httpsClient?: HttpsClient; + parseXML?: (str: string, opts?: { + includeRoot?: boolean; + }) => Promise; pipeline?: Pipeline; requestContentType?: string; stringifyXML?: (obj: any, opts?: { diff --git a/sdk/core/core-client/src/index.ts b/sdk/core/core-client/src/index.ts index dc2fb189dd03..b57bab8d70cb 100644 --- a/sdk/core/core-client/src/index.ts +++ b/sdk/core/core-client/src/index.ts @@ -3,7 +3,12 @@ export { createSerializer, MapperTypeNames } from "./serializer"; export { createSpanFunction } from "./createSpan"; -export { ServiceClient, ServiceClientOptions } from "./serviceClient"; +export { + ServiceClient, + ServiceClientOptions, + createClientPipeline, + ClientPipelineOptions +} from "./serviceClient"; export { OperationSpec, OperationArguments, diff --git a/sdk/core/core-client/src/serviceClient.ts b/sdk/core/core-client/src/serviceClient.ts index fba4f29eed04..59c3949fc1cc 100644 --- a/sdk/core/core-client/src/serviceClient.ts +++ b/sdk/core/core-client/src/serviceClient.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { TokenCredential, isTokenCredential } from "@azure/core-auth"; +import { TokenCredential } from "@azure/core-auth"; import { DefaultHttpsClient, HttpsClient, @@ -10,7 +10,8 @@ import { Pipeline, createPipelineRequest, createPipelineFromOptions, - bearerTokenAuthenticationPolicy + bearerTokenAuthenticationPolicy, + InternalPipelineOptions } from "@azure/core-https"; import { OperationResponse, @@ -27,7 +28,7 @@ import { MapperTypeNames } from "./serializer"; import { getRequestUrl } from "./urlHelpers"; import { isPrimitiveType } from "./utils"; import { getOperationArgumentValueFromParameter } from "./operationHelpers"; -import { deserializationPolicy } from "./deserializationPolicy"; +import { deserializationPolicy, DeserializationPolicyOptions } from "./deserializationPolicy"; /** * Options to be provided while creating the client. @@ -60,6 +61,11 @@ export interface ServiceClientOptions { * A method that is able to turn an XML object model into a string. */ stringifyXML?: (obj: any, opts?: { rootName?: string }) => string; + + /** + * A method that is able to parse XML. + */ + parseXML?: (str: string, opts?: { includeRoot?: boolean }) => Promise; } /** @@ -100,10 +106,14 @@ export class ServiceClient { this._requestContentType = options.requestContentType; this._baseUri = options.baseUri; this._httpsClient = options.httpsClient || new DefaultHttpsClient(); + this._stringifyXML = options.stringifyXML; this._pipeline = options.pipeline || - createDefaultPipeline({ baseUri: this._baseUri, credential: options.credential }); - this._stringifyXML = options.stringifyXML; + createDefaultPipeline({ + baseUri: this._baseUri, + credential: options.credential, + parseXML: options.parseXML + }); } /** @@ -351,22 +361,56 @@ function getXmlValueWithNamespace( } function createDefaultPipeline( - options: { baseUri?: string; credential?: TokenCredential } = {} + options: { + baseUri?: string; + credential?: TokenCredential; + parseXML?: (str: string, opts?: { includeRoot?: boolean }) => Promise; + } = {} ): Pipeline { - const pipeline = createPipelineFromOptions({}); + return createClientPipeline({ + credentialOptions: options, + deserializationOptions: { + parseXML: options.parseXML + } + }); +} - const credential = options.credential; +/** + * Options for creating a Pipeline to use with ServiceClient. + * Mostly for customizing the auth policy (if using token auth) or + * the deserialization options when using XML. + */ +export interface ClientPipelineOptions extends InternalPipelineOptions { + /** + * Options to customize bearerTokenAuthenticationPolicy. + */ + credentialOptions?: { baseUri?: string; credential?: TokenCredential }; + /** + * Options to customize deserializationPolicy. + */ + deserializationOptions?: DeserializationPolicyOptions; +} + +/** + * Creates a new Pipeline for use with a Service Client. + * Adds in deserializationPolicy by default. + * Also adds in bearerTokenAuthenticationPolicy if passed a TokenCredential. + * @param options Options to customize the created pipeline. + */ +export function createClientPipeline(options: ClientPipelineOptions = {}): Pipeline { + const pipeline = createPipelineFromOptions(options ?? {}); + + const credential = options.credentialOptions?.credential; if (credential) { - if (isTokenCredential(credential)) { - pipeline.addPolicy( - bearerTokenAuthenticationPolicy({ credential, scopes: `${options.baseUri || ""}/.default` }) - ); - } else { - throw new Error("The credential argument must implement the TokenCredential interface"); - } + pipeline.addPolicy( + bearerTokenAuthenticationPolicy({ + credential, + scopes: `${options.credentialOptions?.baseUri || ""}/.default` + }) + ); } - pipeline.addPolicy(deserializationPolicy(), { phase: "Serialize" }); + pipeline.addPolicy(deserializationPolicy(options.deserializationOptions), { phase: "Serialize" }); return pipeline; } diff --git a/sdk/core/core-client/src/urlHelpers.ts b/sdk/core/core-client/src/urlHelpers.ts index faf818fca661..855a2e576ad2 100644 --- a/sdk/core/core-client/src/urlHelpers.ts +++ b/sdk/core/core-client/src/urlHelpers.ts @@ -34,7 +34,7 @@ export function getRequestUrl( if (isAbsoluteUrl(path)) { requestUrl = path; } else { - requestUrl = appendPath(requestUrl, operationSpec.path); + requestUrl = appendPath(requestUrl, path); } } @@ -87,21 +87,27 @@ function isAbsoluteUrl(url: string): boolean { return url.includes("://"); } -function appendPath(url: string, path?: string): string { - let result = url; - let toAppend = path; - if (toAppend) { - if (!result.endsWith("/")) { - result = `${result}/`; - } +function appendPath(url: string, pathToAppend?: string): string { + if (!pathToAppend) { + return url; + } - if (toAppend.startsWith("/")) { - toAppend = toAppend.substring(1); - } + const parsedUrl = new URL(url); + let newPath = parsedUrl.pathname; - result = result + toAppend; + if (!newPath.endsWith("/")) { + newPath = `${newPath}/`; } - return result; + + if (pathToAppend.startsWith("/")) { + pathToAppend = pathToAppend.substring(1); + } + + newPath = newPath + pathToAppend; + + parsedUrl.pathname = newPath; + + return parsedUrl.toString(); } function calculateQueryParameters( @@ -176,17 +182,41 @@ function calculateQueryParameters( return result; } +function simpleParseQueryParams(queryString: string): Array<[string, string]> { + if (!queryString || queryString[0] !== "?") { + return []; + } + + // remove the leading ? + queryString = queryString.slice(1); + + const pairs = queryString.split("&"); + + return pairs.map((pair) => { + const [name, value] = pair.split("=", 2); + return [name, value]; + }); +} + function appendQueryParams(url: string, queryParams: Map): string { + if (queryParams.size === 0) { + return url; + } + const parsedUrl = new URL(url); - const combinedParams = new Map(queryParams); + // QUIRK: parsedUrl.searchParams will have their name/value pairs decoded, which + // can change their meaning to the server, such as in the case of a SAS signature. + // To avoid accidentally un-encoding a query param, we parse the key/values ourselves + const existingParams = simpleParseQueryParams(parsedUrl.search); + const combinedParams = new Map(existingParams); - for (const [name, value] of parsedUrl.searchParams) { + for (const [name, value] of queryParams) { const existingValue = combinedParams.get(name); if (Array.isArray(existingValue)) { - existingValue.push(value); + existingValue.push(...value); } else if (existingValue) { - combinedParams.set(name, [existingValue, value]); + combinedParams.set(name, [existingValue, ...value]); } else { combinedParams.set(name, value); } diff --git a/sdk/core/core-client/test/serviceClient.spec.ts b/sdk/core/core-client/test/serviceClient.spec.ts index 2911e6415752..821f39881dac 100644 --- a/sdk/core/core-client/test/serviceClient.spec.ts +++ b/sdk/core/core-client/test/serviceClient.spec.ts @@ -157,7 +157,7 @@ describe("ServiceClient", function() { await testSendOperationRequest(["1", "2", "3"], "Multi", false, "?q=1&q=2&q=3"); await testSendOperationRequest(["1,2", "3,4", "5"], "Multi", false, "?q=1%2C2&q=3%2C4&q=5"); await testSendOperationRequest(["1,2", "3,4", "5"], "Multi", true, "?q=1,2&q=3,4&q=5"); - await testSendOperationRequest([], "Multi", true, "https://example.com/"); + await testSendOperationRequest([], "Multi", true, "https://example.com"); }); it("should deserialize response bodies", async function() { diff --git a/sdk/core/core-client/test/urlHelpers.spec.ts b/sdk/core/core-client/test/urlHelpers.spec.ts new file mode 100644 index 000000000000..14b3b0715d31 --- /dev/null +++ b/sdk/core/core-client/test/urlHelpers.spec.ts @@ -0,0 +1,86 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { assert } from "chai"; +import { getRequestUrl } from "../src/urlHelpers"; +import { OperationSpec, OperationURLParameter, createSerializer } from "../src"; + +describe("getRequestUrl", function() { + const urlParameter: OperationURLParameter = { + parameterPath: "url", + mapper: { + serializedName: "url", + required: true, + xmlName: "url", + type: { + name: "String" + } + }, + skipEncoding: true + }; + + const tableParameter: OperationURLParameter = { + parameterPath: "table", + mapper: { + serializedName: "table", + required: true, + xmlName: "table", + type: { + name: "String" + } + } + }; + + const serializer = createSerializer({}, false); + + const operationSpec: OperationSpec = { + path: "/Tables('{table}')", + httpMethod: "DELETE", + responses: {}, + urlParameters: [urlParameter, tableParameter], + serializer + }; + + it("should handle nested replacements", function() { + const result = getRequestUrl( + "{url}", + operationSpec, + { table: "TestTable" }, + { url: "https://test.com" } + ); + assert.strictEqual(result, "https://test.com/Tables('TestTable')"); + }); + + it("should handle query parameters on the base url", function() { + const result = getRequestUrl( + "{url}", + operationSpec, + { table: "TestTable" }, + { url: "https://test.com?superSecretKey=awesome" } + ); + assert.strictEqual(result, "https://test.com/Tables('TestTable')?superSecretKey=awesome"); + }); + + it("should not modify needlessly encoded query parameters", function() { + const specWithQueryParams: OperationSpec = { + ...operationSpec, + queryParameters: [ + { + parameterPath: "extraValue", + mapper: { type: { name: "String" }, serializedName: "extraValue", required: true }, + skipEncoding: true + } + ] + }; + const result = getRequestUrl( + "{url}", + specWithQueryParams, + { table: "TestTable", extraValue: "%27blah%27" }, + { url: "https://test.com?superSecretKey=Qai%2B4%2FIM%3D" } + ); + assert.strictEqual( + result, + "https://test.com/Tables('TestTable')?superSecretKey=Qai%2B4%2FIM%3D&extraValue=%27blah%27" + ); + }); +});