Skip to content

Commit

Permalink
[core-https] Hide client constructors (Azure#13859)
Browse files Browse the repository at this point in the history
Migrate DefaultHttpsClient from a constructor pattern to a factory.
  • Loading branch information
xirzec authored Feb 19, 2021
1 parent 38a2bdd commit 7a7c173
Show file tree
Hide file tree
Showing 16 changed files with 62 additions and 40 deletions.
2 changes: 1 addition & 1 deletion sdk/core/core-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"dependencies": {
"@azure/abort-controller": "^1.0.0",
"@azure/core-auth": "^1.2.0",
"@azure/core-https": "1.0.0-beta.1",
"@azure/core-https": "1.0.0-beta.2",
"@azure/core-tracing": "1.0.0-preview.9",
"@opentelemetry/api": "^0.10.2",
"tslib": "^2.0.0"
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/core-client/src/httpClientCache.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { HttpsClient, DefaultHttpsClient } from "@azure/core-https";
import { HttpsClient, createDefaultHttpsClient } from "@azure/core-https";

let cachedHttpsClient: HttpsClient | undefined;

export function getCachedDefaultHttpsClient(): HttpsClient {
if (!cachedHttpsClient) {
cachedHttpsClient = new DefaultHttpsClient();
cachedHttpsClient = createDefaultHttpsClient();
}

return cachedHttpsClient;
Expand Down
2 changes: 2 additions & 0 deletions sdk/core/core-https/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## 1.0.0-beta.2 (Unreleased)

- Changed from exposing `DefaultHttpsClient` as a class directly, to providing `createDefaultHttpsClient()` to instantiate the appropriate runtime class.

## 1.0.0-beta.1 (2021-02-04)

- Changes from `core-http`:
Expand Down
8 changes: 3 additions & 5 deletions sdk/core/core-https/review/core-https.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ export interface BearerTokenAuthenticationPolicyOptions {
scopes: string | string[];
}

// @public
export function createDefaultHttpsClient(): HttpsClient;

// @public
export function createEmptyPipeline(): Pipeline;

Expand All @@ -47,11 +50,6 @@ export function decompressResponsePolicy(): PipelinePolicy;
// @public
export const decompressResponsePolicyName = "decompressResponsePolicy";

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

// @public
export function exponentialRetryPolicy(options?: ExponentialRetryPolicyOptions): PipelinePolicy;

Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-https/src/accessTokenCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface AccessTokenCache {
* Provides an AccessTokenCache implementation which clears
* the cached AccessToken's after the expiresOnTimestamp has
* passed.
* @internal
*/
export class ExpiringAccessTokenCache implements AccessTokenCache {
private tokenRefreshBufferMs: number;
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-https/src/defaultHttpsClient.browser.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export { XhrHttpsClient as DefaultHttpsClient } from "./xhrHttpsClient";
export { createXhrHttpsClient as createDefaultHttpsClient } from "./xhrHttpsClient";
2 changes: 1 addition & 1 deletion sdk/core/core-https/src/defaultHttpsClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

export { NodeHttpsClient as DefaultHttpsClient } from "./nodeHttpsClient";
export { createNodeHttpsClient as createDefaultHttpsClient } from "./nodeHttpsClient";
2 changes: 1 addition & 1 deletion sdk/core/core-https/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export {
PipelineOptions,
createPipelineFromOptions
} from "./pipeline";
export { DefaultHttpsClient } from "./defaultHttpsClient";
export { createDefaultHttpsClient } from "./defaultHttpsClient";
export { createHttpHeaders } from "./httpHeaders";
export { createPipelineRequest, PipelineRequestOptions } from "./pipelineRequest";
export { RestError, RestErrorOptions } from "./restError";
Expand Down
10 changes: 9 additions & 1 deletion sdk/core/core-https/src/nodeHttpsClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ class ReportTransform extends Transform {

/**
* A HttpsClient implementation that uses Node's "https" module to send HTTPS requests.
* @internal
*/
export class NodeHttpsClient implements HttpsClient {
class NodeHttpsClient implements HttpsClient {
private keepAliveAgent?: https.Agent;
private proxyAgent?: https.Agent;

Expand Down Expand Up @@ -306,3 +307,10 @@ function getBodyLength(body: RequestBodyType): number | null {
return null;
}
}

/**
* Create a new HttpsClient instance for the NodeJS environment.
*/
export function createNodeHttpsClient(): HttpsClient {
return new NodeHttpsClient();
}
10 changes: 9 additions & 1 deletion sdk/core/core-https/src/xhrHttpsClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ function isReadableStream(body: any): body is NodeJS.ReadableStream {

/**
* A HttpsClient implementation that uses XMLHttpRequest to send HTTPS requests.
* @internal
*/
export class XhrHttpsClient implements HttpsClient {
class XhrHttpsClient implements HttpsClient {
/**
* Makes a request over an underlying transport layer and returns the response.
* @param request - The request to be made.
Expand Down Expand Up @@ -188,3 +189,10 @@ function rejectOnTerminalEvent(
xhr.addEventListener("abort", () => reject(abortError));
xhr.addEventListener("timeout", () => reject(abortError));
}

/**
* Create a new HttpsClient instance for the browser environment.
*/
export function createXhrHttpsClient(): HttpsClient {
return new XhrHttpsClient();
}
20 changes: 10 additions & 10 deletions sdk/core/core-https/test/browser/xhrHttpsClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { assert } from "chai";
import * as sinon from "sinon";
import { AbortController } from "@azure/abort-controller";
import { DefaultHttpsClient, createPipelineRequest } from "../../src";
import { createDefaultHttpsClient, createPipelineRequest } from "../../src";

describe("XhrHttpsClient", function() {
let xhrMock: sinon.SinonFakeXMLHttpRequestStatic;
Expand All @@ -27,7 +27,7 @@ describe("XhrHttpsClient", function() {
});

it("shouldn't throw on 404", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const request = createPipelineRequest({ url: "https://example.com" });
const promise = client.sendRequest(request);
assert.equal(requests.length, 1);
Expand All @@ -37,7 +37,7 @@ describe("XhrHttpsClient", function() {
});

it("should allow canceling of requests", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const controller = new AbortController();
const request = createPipelineRequest({
url: "https://example.com",
Expand All @@ -54,7 +54,7 @@ describe("XhrHttpsClient", function() {
});

it("should allow canceling of requests before the request is made", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const controller = new AbortController();
controller.abort();
const request = createPipelineRequest({
Expand All @@ -71,7 +71,7 @@ describe("XhrHttpsClient", function() {
});

it("should report upload and download progress", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
let downloadCalled = false;
let uploadCalled = false;
const request = createPipelineRequest({
Expand All @@ -97,7 +97,7 @@ describe("XhrHttpsClient", function() {
});

it("should honor timeout", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();

const timeoutLength = 2000;
const request = createPipelineRequest({
Expand All @@ -115,7 +115,7 @@ describe("XhrHttpsClient", function() {
});

it("parses headers", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const request = createPipelineRequest({ url: "https://example.com" });
const promise = client.sendRequest(request);
assert.equal(requests.length, 1);
Expand All @@ -127,7 +127,7 @@ describe("XhrHttpsClient", function() {
});

it("parses empty string headers", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const request = createPipelineRequest({ url: "https://example.com" });
const promise = client.sendRequest(request);
assert.equal(requests.length, 1);
Expand All @@ -139,7 +139,7 @@ describe("XhrHttpsClient", function() {
});

it("should stream response body on matching status code", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const request = createPipelineRequest({
url: "https://example.com",
streamResponseStatusCodes: new Set([200])
Expand All @@ -154,7 +154,7 @@ describe("XhrHttpsClient", function() {
});

it("should not stream response body on non-matching status code", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const request = createPipelineRequest({
url: "https://example.com",
streamResponseStatusCodes: new Set([200])
Expand Down
18 changes: 9 additions & 9 deletions sdk/core/core-https/test/node/nodeHttpsClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { PassThrough } from "stream";
import { IncomingMessage, ClientRequest, IncomingHttpHeaders } from "http";
import * as https from "https";
import { AbortController } from "@azure/abort-controller";
import { DefaultHttpsClient, createPipelineRequest } from "../../src";
import { createDefaultHttpsClient, createPipelineRequest } from "../../src";

class FakeResponse extends PassThrough {
public statusCode?: number;
Expand Down Expand Up @@ -51,7 +51,7 @@ describe("NodeHttpsClient", function() {
});

it("shouldn't throw on 404", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
stubbedRequest.returns(createRequest());
const request = createPipelineRequest({ url: "https://example.com" });
const promise = client.sendRequest(request);
Expand All @@ -61,7 +61,7 @@ describe("NodeHttpsClient", function() {
});

it("should allow canceling of requests", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const controller = new AbortController();
stubbedRequest.returns(createRequest());
const request = createPipelineRequest({
Expand All @@ -79,7 +79,7 @@ describe("NodeHttpsClient", function() {
});

it("should allow canceling of requests before the request is made", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
const controller = new AbortController();
controller.abort();
stubbedRequest.returns(createRequest());
Expand All @@ -97,7 +97,7 @@ describe("NodeHttpsClient", function() {
});

it("should report upload and download progress", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
stubbedRequest.returns(createRequest());
let downloadCalled = false;
let uploadCalled = false;
Expand All @@ -123,7 +123,7 @@ describe("NodeHttpsClient", function() {
});

it("should fail if progress callbacks throw", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
stubbedRequest.returns(createRequest());
const errorMessage = "it failed horribly!";
const request = createPipelineRequest({
Expand All @@ -143,7 +143,7 @@ describe("NodeHttpsClient", function() {
});

it("should honor timeout", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();

const timeoutLength = 2000;
stubbedRequest.returns(createRequest());
Expand All @@ -162,7 +162,7 @@ describe("NodeHttpsClient", function() {
});

it("should stream response body on matching status code", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
stubbedRequest.returns(createRequest());
const request = createPipelineRequest({
url: "https://example.com",
Expand All @@ -176,7 +176,7 @@ describe("NodeHttpsClient", function() {
});

it("should not stream response body on non-matching status code", async function() {
const client = new DefaultHttpsClient();
const client = createDefaultHttpsClient();
stubbedRequest.returns(createRequest());
const request = createPipelineRequest({
url: "https://example.com",
Expand Down
4 changes: 2 additions & 2 deletions sdk/eventgrid/eventgrid/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@
"autoPublish": false,
"dependencies": {
"@azure/core-auth": "^1.2.0",
"@azure/core-client": "1.0.0-beta.1",
"@azure/core-https": "1.0.0-beta.1",
"@azure/core-client": "1.0.0-beta.2",
"@azure/core-https": "1.0.0-beta.2",
"@azure/core-tracing": "1.0.0-preview.9",
"@azure/logger": "^1.0.0",
"@opentelemetry/api": "^0.10.2",
Expand Down
2 changes: 1 addition & 1 deletion sdk/storage/storage-blob/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@
"tslib": "^2.0.0"
},
"devDependencies": {
"@azure/core-https": "1.0.0-beta.1",
"@azure/core-https": "1.0.0-beta.2",
"@azure/dev-tool": "^1.0.0",
"@azure/eslint-plugin-azure-sdk": "^3.0.0",
"@azure/identity": "^1.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,20 @@
// Licensed under the MIT license.

import { StorageBlobDownloadWithSASTest } from "./dowloadWithSAS.spec";
import { DefaultHttpsClient, createPipelineRequest, PipelineRequest } from "@azure/core-https";
import {
createDefaultHttpsClient,
HttpsClient,
createPipelineRequest,
PipelineRequest
} from "@azure/core-https";
import { drainStream } from "@azure/test-utils-perfstress";

export class CoreHTTPSDownloadWithSASTest extends StorageBlobDownloadWithSASTest {
client: DefaultHttpsClient;
client: HttpsClient;
request: PipelineRequest;
constructor() {
super();
this.client = new DefaultHttpsClient();
this.client = createDefaultHttpsClient();
this.request = createPipelineRequest({
url: this.sasUrl,
streamResponseStatusCodes: new Set([200, 206]),
Expand Down
4 changes: 2 additions & 2 deletions sdk/tables/data-tables/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@
"sideEffects": false,
"prettier": "@azure/eslint-plugin-azure-sdk/prettier.json",
"dependencies": {
"@azure/core-client": "1.0.0-beta.1",
"@azure/core-https": "1.0.0-beta.1",
"@azure/core-client": "1.0.0-beta.2",
"@azure/core-https": "1.0.0-beta.2",
"@azure/core-paging": "^1.1.1",
"@azure/core-xml": "1.0.0-beta.1",
"@azure/logger": "^1.0.0",
Expand Down

0 comments on commit 7a7c173

Please sign in to comment.