From dacea904cc875c3917e0faece0d41ca52efeae93 Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Fri, 20 Apr 2018 10:14:54 -0700 Subject: [PATCH] privatize IConnection and HttpConnection (#2105) --- .../ts/FunctionalTests/ts/ConnectionTests.ts | 5 ++- clients/ts/FunctionalTests/ts/index.ts | 1 - clients/ts/signalr/spec/HttpClient.spec.ts | 2 +- .../ts/signalr/spec/HttpConnection.spec.ts | 33 ++++++++++--------- clients/ts/signalr/spec/HubConnection.spec.ts | 5 +-- .../signalr/spec/HubConnectionBuilder.spec.ts | 12 +++---- clients/ts/signalr/src/HttpConnection.ts | 10 +----- clients/ts/signalr/src/HubConnection.ts | 15 ++++++--- .../ts/signalr/src/HubConnectionBuilder.ts | 8 +++-- .../ts/signalr/src/IHttpConnectionOptions.ts | 15 +++++++++ clients/ts/signalr/src/index.ts | 4 +-- 11 files changed, 65 insertions(+), 45 deletions(-) create mode 100644 clients/ts/signalr/src/IHttpConnectionOptions.ts diff --git a/clients/ts/FunctionalTests/ts/ConnectionTests.ts b/clients/ts/FunctionalTests/ts/ConnectionTests.ts index 05143c0938..6c5351c6fd 100644 --- a/clients/ts/FunctionalTests/ts/ConnectionTests.ts +++ b/clients/ts/FunctionalTests/ts/ConnectionTests.ts @@ -1,10 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -import { HttpConnection, HttpTransportType, IHttpConnectionOptions, LogLevel, TransferFormat } from "@aspnet/signalr"; +import { HttpTransportType, IHttpConnectionOptions, LogLevel, TransferFormat } from "@aspnet/signalr"; import { eachTransport, ECHOENDPOINT_URL } from "./Common"; import { TestLogger } from "./TestLogger"; +// We want to continue testing HttpConnection, but we don't export it anymore. So just pull it in directly from the source file. +import { HttpConnection } from "../../signalr/src/HttpConnection"; + const commonOptions: IHttpConnectionOptions = { logMessageContent: true, logger: TestLogger.instance, diff --git a/clients/ts/FunctionalTests/ts/index.ts b/clients/ts/FunctionalTests/ts/index.ts index 6a7008345a..9fd847cb12 100644 --- a/clients/ts/FunctionalTests/ts/index.ts +++ b/clients/ts/FunctionalTests/ts/index.ts @@ -4,7 +4,6 @@ console.log("SignalR Functional Tests Loaded"); import "es6-promise/dist/es6-promise.auto.js"; -import "./ConnectionTests"; import "./HubConnectionTests"; import "./WebDriverReporter"; import "./WebSocketTests"; diff --git a/clients/ts/signalr/spec/HttpClient.spec.ts b/clients/ts/signalr/spec/HttpClient.spec.ts index f4af2473a2..7c5c272024 100644 --- a/clients/ts/signalr/spec/HttpClient.spec.ts +++ b/clients/ts/signalr/spec/HttpClient.spec.ts @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -import { HttpRequest } from "../src/index"; +import { HttpRequest } from "../src/HttpClient"; import { TestHttpClient } from "./TestHttpClient"; import { asyncit as it } from "./Utils"; diff --git a/clients/ts/signalr/spec/HttpConnection.spec.ts b/clients/ts/signalr/spec/HttpConnection.spec.ts index 6c45e76213..853faf739c 100644 --- a/clients/ts/signalr/spec/HttpConnection.spec.ts +++ b/clients/ts/signalr/spec/HttpConnection.spec.ts @@ -1,9 +1,9 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +import { HttpResponse } from "../src/HttpClient"; import { HttpConnection } from "../src/HttpConnection"; -import { IHttpConnectionOptions } from "../src/HttpConnection"; -import { HttpResponse } from "../src/index"; +import { IHttpConnectionOptions } from "../src/IHttpConnectionOptions"; import { HttpTransportType, ITransport, TransferFormat } from "../src/ITransport"; import { eachEndpointUrl, eachTransport } from "./Common"; import { TestHttpClient } from "./TestHttpClient"; @@ -435,16 +435,26 @@ describe("HttpConnection", () => { it("authorization header removed when token factory returns null and using LongPolling", async (done) => { const availableTransport = { transport: "LongPolling", transferFormats: ["Text"] }; - var httpClientGetCount = 0; - var accessTokenFactoryCount = 0; + let httpClientGetCount = 0; + let accessTokenFactoryCount = 0; const options: IHttpConnectionOptions = { ...commonOptions, + accessTokenFactory: () => { + accessTokenFactoryCount++; + if (accessTokenFactoryCount === 1) { + return "A token value"; + } else { + // Return a null value after the first call to test the header being removed + return null; + } + }, httpClient: new TestHttpClient() .on("POST", (r) => ({ connectionId: "42", availableTransports: [availableTransport] })) .on("GET", (r) => { httpClientGetCount++; + // tslint:disable-next-line:no-string-literal const authorizationValue = r.headers["Authorization"]; - if (httpClientGetCount == 1) { + if (httpClientGetCount === 1) { if (authorizationValue) { fail("First long poll request should have a authorization header."); } @@ -458,15 +468,6 @@ describe("HttpConnection", () => { throw new Error("fail"); } }), - accessTokenFactory: () => { - accessTokenFactoryCount++; - if (accessTokenFactoryCount == 1) { - return "A token value"; - } else { - // Return a null value after the first call to test the header being removed - return null; - } - }, } as IHttpConnectionOptions; const connection = new HttpConnection("http://tempuri.org", options); @@ -485,14 +486,14 @@ describe("HttpConnection", () => { it("sets inherentKeepAlive feature when using LongPolling", async (done) => { const availableTransport = { transport: "LongPolling", transferFormats: ["Text"] }; - var httpClientGetCount = 0; + let httpClientGetCount = 0; const options: IHttpConnectionOptions = { ...commonOptions, httpClient: new TestHttpClient() .on("POST", (r) => ({ connectionId: "42", availableTransports: [availableTransport] })) .on("GET", (r) => { httpClientGetCount++; - if (httpClientGetCount == 1) { + if (httpClientGetCount === 1) { // First long polling request must succeed so start completes return ""; } else { diff --git a/clients/ts/signalr/spec/HubConnection.spec.ts b/clients/ts/signalr/spec/HubConnection.spec.ts index a438bb8721..4e17b1bba4 100644 --- a/clients/ts/signalr/spec/HubConnection.spec.ts +++ b/clients/ts/signalr/spec/HubConnection.spec.ts @@ -1,11 +1,12 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -import { HubConnection, JsonHubProtocol } from "../src/HubConnection"; +import { HubConnection } from "../src/HubConnection"; import { IConnection } from "../src/IConnection"; import { HubMessage, IHubProtocol, MessageType } from "../src/IHubProtocol"; import { ILogger, LogLevel } from "../src/ILogger"; import { HttpTransportType, ITransport, TransferFormat } from "../src/ITransport"; +import { JsonHubProtocol } from "../src/JsonHubProtocol"; import { NullLogger } from "../src/Loggers"; import { IStreamSubscriber } from "../src/Stream"; import { TextMessageFormat } from "../src/TextMessageFormat"; @@ -13,7 +14,7 @@ import { TextMessageFormat } from "../src/TextMessageFormat"; import { asyncit as it, captureException, delay, PromiseSource } from "./Utils"; function createHubConnection(connection: IConnection, logger?: ILogger, protocol?: IHubProtocol) { - return new HubConnection(connection, logger || NullLogger.instance, protocol || new JsonHubProtocol()); + return HubConnection.create(connection, logger || NullLogger.instance, protocol || new JsonHubProtocol()); } describe("HubConnection", () => { diff --git a/clients/ts/signalr/spec/HubConnectionBuilder.spec.ts b/clients/ts/signalr/spec/HubConnectionBuilder.spec.ts index 45035ca414..1adbe925cd 100644 --- a/clients/ts/signalr/spec/HubConnectionBuilder.spec.ts +++ b/clients/ts/signalr/spec/HubConnectionBuilder.spec.ts @@ -3,13 +3,14 @@ import { HubConnectionBuilder } from "../src/HubConnectionBuilder"; -import { HubConnection } from "../src"; import { HttpRequest, HttpResponse } from "../src/HttpClient"; -import { IHttpConnectionOptions } from "../src/HttpConnection"; +import { HubConnection } from "../src/HubConnection"; +import { IHttpConnectionOptions } from "../src/IHttpConnectionOptions"; import { HubMessage, IHubProtocol } from "../src/IHubProtocol"; import { ILogger, LogLevel } from "../src/ILogger"; -import { TransferFormat, HttpTransportType } from "../src/ITransport"; +import { HttpTransportType, TransferFormat } from "../src/ITransport"; import { NullLogger } from "../src/Loggers"; + import { TestHttpClient } from "./TestHttpClient"; import { asyncit as it, PromiseSource } from "./Utils"; @@ -123,14 +124,13 @@ describe("HubConnectionBuilder", () => { await closed; }); - it("allows logger to be replaced", async () => { let loggedMessages = 0; const logger = { log() { loggedMessages += 1; - } - } + }, + }; const connection = createConnectionBuilder(logger) .withUrl("http://example.com") .build(); diff --git a/clients/ts/signalr/src/HttpConnection.ts b/clients/ts/signalr/src/HttpConnection.ts index 3a9e905d4a..615cb07f70 100644 --- a/clients/ts/signalr/src/HttpConnection.ts +++ b/clients/ts/signalr/src/HttpConnection.ts @@ -3,6 +3,7 @@ import { DefaultHttpClient, HttpClient } from "./HttpClient"; import { IConnection } from "./IConnection"; +import { IHttpConnectionOptions } from "./IHttpConnectionOptions"; import { ILogger, LogLevel } from "./ILogger"; import { HttpTransportType, ITransport, TransferFormat } from "./ITransport"; import { LongPollingTransport } from "./LongPollingTransport"; @@ -10,15 +11,6 @@ import { ServerSentEventsTransport } from "./ServerSentEventsTransport"; import { Arg, createLogger } from "./Utils"; import { WebSocketTransport } from "./WebSocketTransport"; -export interface IHttpConnectionOptions { - httpClient?: HttpClient; - transport?: HttpTransportType | ITransport; - logger?: ILogger | LogLevel; - accessTokenFactory?: () => string | Promise; - logMessageContent?: boolean; - skipNegotiation?: boolean; -} - const enum ConnectionState { Connecting, Connected, diff --git a/clients/ts/signalr/src/HubConnection.ts b/clients/ts/signalr/src/HubConnection.ts index 1ed9ab125e..875217ded9 100644 --- a/clients/ts/signalr/src/HubConnection.ts +++ b/clients/ts/signalr/src/HubConnection.ts @@ -2,7 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. import { HandshakeProtocol, HandshakeRequestMessage, HandshakeResponseMessage } from "./HandshakeProtocol"; -import { HttpConnection, IHttpConnectionOptions } from "./HttpConnection"; +import { HttpConnection } from "./HttpConnection"; import { IConnection } from "./IConnection"; import { CancelInvocationMessage, CompletionMessage, HubMessage, IHubProtocol, InvocationMessage, MessageType, StreamInvocationMessage, StreamItemMessage } from "./IHubProtocol"; import { ILogger, LogLevel } from "./ILogger"; @@ -12,8 +12,6 @@ import { IStreamResult } from "./Stream"; import { TextMessageFormat } from "./TextMessageFormat"; import { Arg, createLogger, Subject } from "./Utils"; -export { JsonHubProtocol }; - const DEFAULT_TIMEOUT_IN_MS: number = 30 * 1000; export class HubConnection { @@ -30,7 +28,16 @@ export class HubConnection { public serverTimeoutInMilliseconds: number; - constructor(connection: IConnection, logger: ILogger, protocol: IHubProtocol) { + /** @internal */ + // Using a public static factory method means we can have a private constructor and an _internal_ + // create method that can be used by HubConnectionBuilder. An "internal" constructor would just + // be stripped away and the '.d.ts' file would have no constructor, which is interpreted as a + // public parameter-less constructor. + public static create(connection: IConnection, logger: ILogger, protocol: IHubProtocol): HubConnection { + return new HubConnection(connection, logger, protocol); + } + + private constructor(connection: IConnection, logger: ILogger, protocol: IHubProtocol) { Arg.isRequired(connection, "connection"); Arg.isRequired(logger, "logger"); Arg.isRequired(protocol, "protocol"); diff --git a/clients/ts/signalr/src/HubConnectionBuilder.ts b/clients/ts/signalr/src/HubConnectionBuilder.ts index 9dca525333..b5a4f42b93 100644 --- a/clients/ts/signalr/src/HubConnectionBuilder.ts +++ b/clients/ts/signalr/src/HubConnectionBuilder.ts @@ -1,11 +1,13 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -import { HttpConnection, IHttpConnectionOptions } from "./HttpConnection"; -import { HubConnection, JsonHubProtocol } from "./HubConnection"; +import { HttpConnection } from "./HttpConnection"; +import { HubConnection } from "./HubConnection"; +import { IHttpConnectionOptions } from "./IHttpConnectionOptions"; import { IHubProtocol } from "./IHubProtocol"; import { ILogger, LogLevel } from "./ILogger"; import { HttpTransportType } from "./ITransport"; +import { JsonHubProtocol } from "./JsonHubProtocol"; import { NullLogger } from "./Loggers"; import { Arg, ConsoleLogger } from "./Utils"; @@ -76,7 +78,7 @@ export class HubConnectionBuilder { } const connection = new HttpConnection(this.url, httpConnectionOptions); - return new HubConnection( + return HubConnection.create( connection, this.logger || NullLogger.instance, this.protocol || new JsonHubProtocol()); diff --git a/clients/ts/signalr/src/IHttpConnectionOptions.ts b/clients/ts/signalr/src/IHttpConnectionOptions.ts new file mode 100644 index 0000000000..cdb6ad73f3 --- /dev/null +++ b/clients/ts/signalr/src/IHttpConnectionOptions.ts @@ -0,0 +1,15 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +import { HttpClient } from "./HttpClient"; +import { ILogger, LogLevel } from "./ILogger"; +import { HttpTransportType, ITransport } from "./ITransport"; + +export interface IHttpConnectionOptions { + httpClient?: HttpClient; + transport?: HttpTransportType | ITransport; + logger?: ILogger | LogLevel; + accessTokenFactory?: () => string | Promise; + logMessageContent?: boolean; + skipNegotiation?: boolean; +} diff --git a/clients/ts/signalr/src/index.ts b/clients/ts/signalr/src/index.ts index 792b68f8ae..f1aee7490f 100644 --- a/clients/ts/signalr/src/index.ts +++ b/clients/ts/signalr/src/index.ts @@ -4,12 +4,12 @@ // Everything that users need to access must be exported here. Including interfaces. export * from "./Errors"; export * from "./HttpClient"; -export * from "./HttpConnection"; +export * from "./IHttpConnectionOptions"; export * from "./HubConnection"; export * from "./HubConnectionBuilder"; -export * from "./IConnection"; export * from "./IHubProtocol"; export * from "./ILogger"; export * from "./ITransport"; export * from "./Stream"; export * from "./Loggers"; +export * from "./JsonHubProtocol";