From 263974e1aee89b3cc6fb96537acebfb765404dd0 Mon Sep 17 00:00:00 2001 From: Ben Schwartz Date: Fri, 13 May 2022 11:58:52 -0400 Subject: [PATCH] Refactor --- .npmrc | 4 + .../electron_app/build.action.sh | 2 +- .../electron_app/{http => }/fetch.spec.ts | 0 .../electron_app/{http => }/fetch.ts | 2 +- src/server_manager/electron_app/http/types.ts | 34 -------- src/server_manager/electron_app/index.ts | 4 +- src/server_manager/electron_app/preload.ts | 2 +- src/server_manager/electron_app/tsconfig.json | 4 +- src/server_manager/infrastructure/errors.ts | 2 +- .../infrastructure/path_api.spec.ts | 67 +++++++++++++++ .../{web_app => infrastructure}/path_api.ts | 50 +++++------- src/server_manager/types/preload.d.ts | 4 +- .../web_app/digitalocean_server.ts | 2 +- src/server_manager/web_app/fetcher.spec.ts | 28 +++++++ src/server_manager/web_app/fetcher.ts | 43 ++++++++++ src/server_manager/web_app/gcp_server.ts | 2 +- src/server_manager/web_app/manual_server.ts | 2 +- src/server_manager/web_app/path_api.spec.ts | 81 ------------------- .../web_app/shadowbox_server.ts | 2 +- 19 files changed, 177 insertions(+), 158 deletions(-) rename src/server_manager/electron_app/{http => }/fetch.spec.ts (100%) rename src/server_manager/electron_app/{http => }/fetch.ts (96%) delete mode 100644 src/server_manager/electron_app/http/types.ts create mode 100644 src/server_manager/infrastructure/path_api.spec.ts rename src/server_manager/{web_app => infrastructure}/path_api.ts (77%) create mode 100644 src/server_manager/web_app/fetcher.spec.ts create mode 100644 src/server_manager/web_app/fetcher.ts delete mode 100644 src/server_manager/web_app/path_api.spec.ts diff --git a/.npmrc b/.npmrc index 656f264c..35d740d1 100644 --- a/.npmrc +++ b/.npmrc @@ -1,2 +1,6 @@ +; Enforces that the user is `npm install`ing with the correct node version. engine-strict=true + +; Workaround for conflict between the default location(s) of node-forge and the +; location expected by Typescript, Jasmine, and Electron. prefer-dedupe=true diff --git a/src/server_manager/electron_app/build.action.sh b/src/server_manager/electron_app/build.action.sh index 128d263b..410f4cd9 100755 --- a/src/server_manager/electron_app/build.action.sh +++ b/src/server_manager/electron_app/build.action.sh @@ -51,7 +51,7 @@ tsc -p src/server_manager/electron_app/tsconfig.json --outDir build/server_manag readonly STATIC_DIR="${OUT_DIR}/static" mkdir -p "${STATIC_DIR}" mkdir -p "${STATIC_DIR}/server_manager" -cp -r "${OUT_DIR}/js/"* "${STATIC_DIR}" +cp -r "${OUT_DIR}/js/electron_app/"* "${STATIC_DIR}" cp -r "${BUILD_DIR}/server_manager/web_app/static" "${STATIC_DIR}/server_manager/web_app/" # Electron requires a package.json file for the app's name, etc. diff --git a/src/server_manager/electron_app/http/fetch.spec.ts b/src/server_manager/electron_app/fetch.spec.ts similarity index 100% rename from src/server_manager/electron_app/http/fetch.spec.ts rename to src/server_manager/electron_app/fetch.spec.ts diff --git a/src/server_manager/electron_app/http/fetch.ts b/src/server_manager/electron_app/fetch.ts similarity index 96% rename from src/server_manager/electron_app/http/fetch.ts rename to src/server_manager/electron_app/fetch.ts index 8417d9cb..d6631616 100644 --- a/src/server_manager/electron_app/http/fetch.ts +++ b/src/server_manager/electron_app/fetch.ts @@ -18,7 +18,7 @@ import {urlToHttpOptions} from 'url'; import type {IncomingMessage} from 'http'; -import type {HttpRequest, HttpResponse} from './types'; +import type {HttpRequest, HttpResponse} from '../infrastructure/path_api'; export const fetchWithPin = async ( req: HttpRequest, diff --git a/src/server_manager/electron_app/http/types.ts b/src/server_manager/electron_app/http/types.ts deleted file mode 100644 index 3f39f2b2..00000000 --- a/src/server_manager/electron_app/http/types.ts +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2022 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// This file is imported by both the Electron and Renderer process code, -// so it cannot contain any imports that are not available in both -// environments. - -// These type definitions are designed to bridge the differences between -// the Fetch API and the Node.JS HTTP API, while also being compatible -// with the Structured Clone algorithm so that they can be passed between -// the Electron and Renderer processes. - -export interface HttpRequest { - url: string; - method: string; - headers?: Record; - body?: string; -} - -export interface HttpResponse { - status: number; - body?: string; -} diff --git a/src/server_manager/electron_app/index.ts b/src/server_manager/electron_app/index.ts index a698b290..c47ce8e7 100644 --- a/src/server_manager/electron_app/index.ts +++ b/src/server_manager/electron_app/index.ts @@ -19,8 +19,8 @@ import {autoUpdater} from 'electron-updater'; import * as path from 'path'; import {URL, URLSearchParams} from 'url'; -import type {HttpRequest, HttpResponse} from './http/types'; -import {fetchWithPin} from './http/fetch'; +import type {HttpRequest, HttpResponse} from '../infrastructure/path_api'; +import {fetchWithPin} from './fetch'; import * as menu from './menu'; const app = electron.app; diff --git a/src/server_manager/electron_app/preload.ts b/src/server_manager/electron_app/preload.ts index a158944f..56592fb7 100644 --- a/src/server_manager/electron_app/preload.ts +++ b/src/server_manager/electron_app/preload.ts @@ -18,7 +18,7 @@ import {URL} from 'url'; import * as digitalocean_oauth from './digitalocean_oauth'; import * as gcp_oauth from './gcp_oauth'; -import {HttpRequest, HttpResponse} from './http/types'; +import {HttpRequest, HttpResponse} from '../infrastructure/path_api'; import {redactManagerUrl} from './util'; // This file is run in the renderer process *before* nodeIntegration is disabled. diff --git a/src/server_manager/electron_app/tsconfig.json b/src/server_manager/electron_app/tsconfig.json index fb3d3aa7..b4dcbff7 100644 --- a/src/server_manager/electron_app/tsconfig.json +++ b/src/server_manager/electron_app/tsconfig.json @@ -4,10 +4,10 @@ "removeComments": false, "noImplicitAny": true, "module": "commonjs", - "rootDir": ".", + "rootDir": "..", "lib": ["dom", "es2021"] }, - "include": ["*.ts", "../types/*.d.ts"], + "include": ["*.ts", "../infrastructure/path_api.ts", "../types/*.d.ts"], "exclude": ["node_modules"], "compileOnSave": true } diff --git a/src/server_manager/infrastructure/errors.ts b/src/server_manager/infrastructure/errors.ts index 0cd528f9..e598465a 100644 --- a/src/server_manager/infrastructure/errors.ts +++ b/src/server_manager/infrastructure/errors.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import type {HttpResponse} from '../electron_app/http/types'; +import type {HttpResponse} from './path_api'; export class OutlineError extends Error { constructor(message?: string) { diff --git a/src/server_manager/infrastructure/path_api.spec.ts b/src/server_manager/infrastructure/path_api.spec.ts new file mode 100644 index 00000000..1e809727 --- /dev/null +++ b/src/server_manager/infrastructure/path_api.spec.ts @@ -0,0 +1,67 @@ +// Copyright 2022 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {PathApiClient} from './path_api'; + +describe('PathApi', () => { + // Mock fetcher + let lastRequest: HttpRequest; + let nextResponse: Promise; + + const fetcher = (request: HttpRequest) => { + lastRequest = request; + return nextResponse; + }; + + beforeEach(() => { + lastRequest = undefined; + nextResponse = undefined; + }); + + const api = new PathApiClient('https://asdf.test/foo', fetcher); + + it('GET', async () => { + const response = {status: 200, body: '{"asdf": true}'}; + nextResponse = Promise.resolve(response); + expect(await api.request('bar')).toEqual({asdf: true}); + expect(lastRequest).toEqual({ + url: 'https://asdf.test/foo/bar', + method: 'GET', + }); + }); + + it('PUT form data', async () => { + const response = {status: 200, body: '{"asdf": true}'}; + nextResponse = Promise.resolve(response); + expect(await api.requestForm('bar', 'PUT', {name: 'value'})).toEqual({asdf: true}); + expect(lastRequest).toEqual({ + url: 'https://asdf.test/foo/bar', + method: 'PUT', + headers: {'Content-Type': 'application/x-www-form-urlencoded'}, + body: 'name=value', + }); + }); + + it('POST JSON data', async () => { + const response = {status: 200, body: '{"asdf": true}'}; + nextResponse = Promise.resolve(response); + expect(await api.requestJson('bar', 'POST', {key: 'value'})).toEqual({asdf: true}); + expect(lastRequest).toEqual({ + url: 'https://asdf.test/foo/bar', + method: 'POST', + headers: {'Content-Type': 'application/json'}, + body: '{"key":"value"}', + }); + }); +}); diff --git a/src/server_manager/web_app/path_api.ts b/src/server_manager/infrastructure/path_api.ts similarity index 77% rename from src/server_manager/web_app/path_api.ts rename to src/server_manager/infrastructure/path_api.ts index 65303d51..ee6dd00e 100644 --- a/src/server_manager/web_app/path_api.ts +++ b/src/server_manager/infrastructure/path_api.ts @@ -12,39 +12,31 @@ // See the License for the specific language governing permissions and // limitations under the License. -import * as errors from '../infrastructure/errors'; -import {HttpRequest, HttpResponse} from '../electron_app/http/types'; +// This file is imported by both the Electron and Renderer process code, +// so it cannot contain any imports that are not available in both +// environments. -async function fetchWrapper(request: HttpRequest): Promise { - const response = await fetch(request.url, request); - return { - status: response.status, - body: await response.text(), - }; +// These type definitions are designed to bridge the differences between +// the Fetch API and the Node.JS HTTP API, while also being compatible +// with the Structured Clone algorithm so that they can be passed between +// the Electron and Renderer processes. + +import * as errors from './errors'; + +export interface HttpRequest { + url: string; + method: string; + headers?: Record; + body?: string; +} + +export interface HttpResponse { + status: number; + body?: string; } // A Fetcher provides the HTTP client functionality for PathApi. -export type Fetcher = typeof fetchWrapper; - -/** - * @param fingerprint A SHA-256 hash of the expected leaf certificate, in binary encoding. - * @returns An HTTP client that enforces `fingerprint`, if set. - */ -function makeFetcher(fingerprint?: string): Fetcher { - if (fingerprint) { - return (request) => fetchWithPin(request, fingerprint); - } - return fetchWrapper; -} - -/** - * @param base A valid URL - * @param fingerprint A SHA-256 hash of the expected leaf certificate, in binary encoding. - * @returns A fully initialized API client. - */ -export function makePathApiClient(base: string, fingerprint?: string): PathApiClient { - return new PathApiClient(base, makeFetcher(fingerprint)); -} +export type Fetcher = (request: HttpRequest) => Promise; /** * Provides access to an HTTP API of the kind exposed by the Shadowbox server. diff --git a/src/server_manager/types/preload.d.ts b/src/server_manager/types/preload.d.ts index d5bfc2bd..b50fc808 100644 --- a/src/server_manager/types/preload.d.ts +++ b/src/server_manager/types/preload.d.ts @@ -14,8 +14,8 @@ // Functions made available to the renderer process via preload.ts. -type HttpRequest = import('../electron_app/http/types').HttpRequest; -type HttpResponse = import('../electron_app/http/types').HttpResponse; +type HttpRequest = import('../infrastructure/path_api').HttpRequest; +type HttpResponse = import('../infrastructure/path_api').HttpResponse; declare function fetchWithPin(request: HttpRequest, fingerprint: string): Promise; declare function openImage(basename: string): void; diff --git a/src/server_manager/web_app/digitalocean_server.ts b/src/server_manager/web_app/digitalocean_server.ts index 367bad0b..cdf6fcbf 100644 --- a/src/server_manager/web_app/digitalocean_server.ts +++ b/src/server_manager/web_app/digitalocean_server.ts @@ -19,7 +19,7 @@ import {sleep} from '../infrastructure/sleep'; import {ValueStream} from '../infrastructure/value_stream'; import {Region} from '../model/digitalocean'; import * as server from '../model/server'; -import {makePathApiClient} from './path_api'; +import {makePathApiClient} from './fetcher'; import {ShadowboxServer} from './shadowbox_server'; diff --git a/src/server_manager/web_app/fetcher.spec.ts b/src/server_manager/web_app/fetcher.spec.ts new file mode 100644 index 00000000..48489718 --- /dev/null +++ b/src/server_manager/web_app/fetcher.spec.ts @@ -0,0 +1,28 @@ +// Copyright 2022 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {makePathApiClient} from './fetcher'; + +describe('makePathApiClient', () => { + const api = makePathApiClient('https://api.github.com/repos/Jigsaw-Code/'); + + if (process?.versions?.node) { + // This test relies on fetch(), which doesn't exist in Node (yet). + return; + } + it('GET', async () => { + const response = await api.request<{name: string}>('outline-server'); + expect(response.name).toEqual('outline-server'); + }); +}); diff --git a/src/server_manager/web_app/fetcher.ts b/src/server_manager/web_app/fetcher.ts new file mode 100644 index 00000000..0dfb0915 --- /dev/null +++ b/src/server_manager/web_app/fetcher.ts @@ -0,0 +1,43 @@ +// Copyright 2022 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import {Fetcher, PathApiClient} from '../infrastructure/path_api'; + +async function fetchWrapper(request: HttpRequest): Promise { + const response = await fetch(request.url, request); + return { + status: response.status, + body: await response.text(), + }; +} + +/** + * @param fingerprint A SHA-256 hash of the expected leaf certificate, in binary encoding. + * @returns An HTTP client that enforces `fingerprint`, if set. + */ +function makeFetcher(fingerprint?: string): Fetcher { + if (fingerprint) { + return (request) => fetchWithPin(request, fingerprint); + } + return fetchWrapper; +} + +/** + * @param base A valid URL + * @param fingerprint A SHA-256 hash of the expected leaf certificate, in binary encoding. + * @returns A fully initialized API client. + */ +export function makePathApiClient(base: string, fingerprint?: string): PathApiClient { + return new PathApiClient(base, makeFetcher(fingerprint)); +} diff --git a/src/server_manager/web_app/gcp_server.ts b/src/server_manager/web_app/gcp_server.ts index 8af4abb4..32a6dead 100644 --- a/src/server_manager/web_app/gcp_server.ts +++ b/src/server_manager/web_app/gcp_server.ts @@ -19,7 +19,7 @@ import {ValueStream} from '../infrastructure/value_stream'; import {Zone} from '../model/gcp'; import * as server from '../model/server'; import {DataAmount, ManagedServerHost, MonetaryCost} from '../model/server'; -import {makePathApiClient} from './path_api'; +import {makePathApiClient} from './fetcher'; import {ShadowboxServer} from './shadowbox_server'; diff --git a/src/server_manager/web_app/manual_server.ts b/src/server_manager/web_app/manual_server.ts index 077cdaf9..65b20642 100644 --- a/src/server_manager/web_app/manual_server.ts +++ b/src/server_manager/web_app/manual_server.ts @@ -14,7 +14,7 @@ import {hexToString} from '../infrastructure/hex_encoding'; import * as server from '../model/server'; -import {makePathApiClient} from './path_api'; +import {makePathApiClient} from './fetcher'; import {ShadowboxServer} from './shadowbox_server'; diff --git a/src/server_manager/web_app/path_api.spec.ts b/src/server_manager/web_app/path_api.spec.ts deleted file mode 100644 index 8165b47f..00000000 --- a/src/server_manager/web_app/path_api.spec.ts +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2022 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -import {makePathApiClient, PathApiClient} from './path_api'; - -describe('PathApi', () => { - describe('local', () => { - // Mock fetcher - let lastRequest: HttpRequest; - let nextResponse: Promise; - - const fetcher = (request: HttpRequest) => { - lastRequest = request; - return nextResponse; - }; - - beforeEach(() => { - lastRequest = undefined; - nextResponse = undefined; - }); - - const api = new PathApiClient('https://asdf.test/foo', fetcher); - - it('GET', async () => { - const response = {status: 200, body: '{"asdf": true}'}; - nextResponse = Promise.resolve(response); - expect(await api.request('bar')).toEqual({asdf: true}); - expect(lastRequest).toEqual({ - url: 'https://asdf.test/foo/bar', - method: 'GET', - }); - }); - - it('PUT form data', async () => { - const response = {status: 200, body: '{"asdf": true}'}; - nextResponse = Promise.resolve(response); - expect(await api.requestForm('bar', 'PUT', {name: 'value'})).toEqual({asdf: true}); - expect(lastRequest).toEqual({ - url: 'https://asdf.test/foo/bar', - method: 'PUT', - headers: {'Content-Type': 'application/x-www-form-urlencoded'}, - body: 'name=value', - }); - }); - - it('POST JSON data', async () => { - const response = {status: 200, body: '{"asdf": true}'}; - nextResponse = Promise.resolve(response); - expect(await api.requestJson('bar', 'POST', {key: 'value'})).toEqual({asdf: true}); - expect(lastRequest).toEqual({ - url: 'https://asdf.test/foo/bar', - method: 'POST', - headers: {'Content-Type': 'application/json'}, - body: '{"key":"value"}', - }); - }); - }); - - // These tests rely on the fetch() function, which is not available in Node (yet). - if (!process?.versions?.node) { - describe('remote', () => { - const api = makePathApiClient('https://api.github.com/repos/Jigsaw-Code/'); - - it('GET', async () => { - const response = await api.request<{name: string}>('outline-server'); - expect(response.name).toEqual('outline-server'); - }); - }); - } -}); diff --git a/src/server_manager/web_app/shadowbox_server.ts b/src/server_manager/web_app/shadowbox_server.ts index c90addfc..d4bbd098 100644 --- a/src/server_manager/web_app/shadowbox_server.ts +++ b/src/server_manager/web_app/shadowbox_server.ts @@ -15,7 +15,7 @@ import * as semver from 'semver'; import * as server from '../model/server'; -import {PathApiClient} from './path_api'; +import {PathApiClient} from '../infrastructure/path_api'; interface AccessKeyJson { id: string;