From 38f53168fa2b8e181df1643effc208bb6d3869b6 Mon Sep 17 00:00:00 2001 From: simojenki Date: Fri, 8 Jul 2022 15:24:07 +1000 Subject: [PATCH] refactor --- src/http.ts | 79 +++++++++++------- src/server.ts | 26 +++--- src/subsonic/index.ts | 27 +++---- src/subsonic/library.ts | 30 ++----- src/subsonic/subsonic_http.ts | 53 ++++++++----- src/utils.ts | 18 ++++- tests/http.test.ts | 141 ++++++++++++++++++++++++++++++++- tests/subsonic/generic.test.ts | 4 +- tests/utils.test.ts | 31 +++++++- 9 files changed, 300 insertions(+), 109 deletions(-) diff --git a/src/http.ts b/src/http.ts index 0a6e561..0dd6154 100644 --- a/src/http.ts +++ b/src/http.ts @@ -1,8 +1,16 @@ -import { AxiosPromise, AxiosRequestConfig, ResponseType } from "axios"; +import { + AxiosPromise, + AxiosRequestConfig, + Method, + ResponseType, +} from "axios"; export interface Http { (config: AxiosRequestConfig): AxiosPromise; } +export interface Http2 extends Http { + with: (defaults: Partial) => Http2; +} export type RequestParams = { baseURL: string; @@ -10,32 +18,49 @@ export type RequestParams = { params: any; headers: any; responseType: ResponseType; + method: Method; +}; + +const wrap = (http2: Http2, defaults: Partial): Http2 => { + const f = ((config: AxiosRequestConfig) => http2(merge(defaults, config))) as Http2; + f.with = (defaults: Partial) => wrap(f, defaults); + return f; +}; + +export const http2From = (http: Http): Http2 => { + const f = ((config: AxiosRequestConfig) => http(config)) as Http2; + f.with = (defaults: Partial) => wrap(f, defaults); + return f; +} + +const merge = ( + defaults: Partial, + config: AxiosRequestConfig +) => { + let toApply = { + ...defaults, + ...config, + }; + if (defaults.params) { + toApply = { + ...toApply, + params: { + ...defaults.params, + ...config.params, + }, + }; + } + if (defaults.headers) { + toApply = { + ...toApply, + headers: { + ...defaults.headers, + ...config.headers, + }, + }; + } + return toApply; }; export const http = - (base: Http, defaults: Partial): Http => - (config: AxiosRequestConfig) => { - let toApply = { - ...defaults, - ...config, - }; - if (defaults.params) { - toApply = { - ...toApply, - params: { - ...defaults.params, - ...config.params, - }, - }; - } - if (defaults.headers) { - toApply = { - ...toApply, - headers: { - ...defaults.headers, - ...config.headers, - }, - }; - } - return base(toApply); - }; + (base: Http, defaults: Partial): Http => (config: AxiosRequestConfig) => base(merge(defaults, config)); diff --git a/src/server.ts b/src/server.ts index 34d4f55..f5d56c5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -33,13 +33,10 @@ import makeI8N, { asLANGs, KEY, keys as i8nKeys, LANG } from "./i8n"; import { Icon, ICONS, festivals, features } from "./icon"; import _, { shuffle } from "underscore"; import morgan from "morgan"; -import { takeWithRepeats } from "./utils"; +import { mask, takeWithRepeats } from "./utils"; import { parse } from "./burn"; import { axiosImageFetcher, ImageFetcher } from "./images"; -import { - JWTSmapiLoginTokens, - SmapiAuthTokens, -} from "./smapi_auth"; +import { JWTSmapiLoginTokens, SmapiAuthTokens } from "./smapi_auth"; export const BONOB_ACCESS_TOKEN_HEADER = "bat"; @@ -377,23 +374,28 @@ function server( logger.info( `${trace} bnb<- ${req.method} ${req.path}?${JSON.stringify( req.query - )}, headers=${JSON.stringify({ ...req.headers, "bnbt": "*****", "bnbk": "*****" })}` + )}, headers=${JSON.stringify(mask(req.headers, ["bnbt", "bnbk"]))}` ); const serviceToken = pipe( E.fromNullable("Missing bnbt header")(req.headers["bnbt"] as string), - E.chain(token => pipe( - E.fromNullable("Missing bnbk header")(req.headers["bnbk"] as string), - E.map(key => ({ token, key })) - )), + E.chain((token) => + pipe( + E.fromNullable("Missing bnbk header")(req.headers["bnbk"] as string), + E.map((key) => ({ token, key })) + ) + ), E.chain((auth) => pipe( smapiAuthTokens.verify(auth), E.mapLeft((_) => "Auth token failed to verify") ) ), - E.getOrElseW(() => undefined) - ) + E.getOrElseW((e: string) => { + logger.error(`Failed to get serviceToken for stream: ${e}`); + return undefined; + }) + ); if (!serviceToken) { return res.status(401).send(); diff --git a/src/subsonic/index.ts b/src/subsonic/index.ts index d6cd1c8..729e4b7 100644 --- a/src/subsonic/index.ts +++ b/src/subsonic/index.ts @@ -5,7 +5,7 @@ import axios from "axios"; import randomstring from "randomstring"; import _ from "underscore"; // todo: rename http2 to http -import { Http, http as http2, RequestParams } from "../http"; +import { Http2, http2From } from "../http"; import { Credentials, @@ -17,7 +17,7 @@ import { import { b64Encode, b64Decode } from "../b64"; import { axiosImageFetcher, ImageFetcher } from "../images"; import { navidromeMusicLibrary, SubsonicGenericMusicLibrary } from "./library"; -import { getJSON as getJSON } from "./subsonic_http"; +import { client } from "./subsonic_http"; export const t = (password: string, s: string) => Md5.hashStr(`${password}${s}`); @@ -101,7 +101,7 @@ export class Subsonic implements MusicService { // todo: why is this in here? externalImageFetcher: ImageFetcher; - subsonicHttp: Http; + subsonic: Http2; constructor( url: string, @@ -111,29 +111,22 @@ export class Subsonic implements MusicService { this.url = url; this.streamClientApplication = streamClientApplication; this.externalImageFetcher = externalImageFetcher; - this.subsonicHttp = http2(axios, { + this.subsonic = http2From(axios).with({ baseURL: this.url, params: { v: "1.16.1", c: DEFAULT_CLIENT_APPLICATION }, headers: { "User-Agent": "bonob" }, }); } - authenticatedSubsonicHttp = (credentials: Credentials) => - http2(this.subsonicHttp, { - params: { - u: credentials.username, - ...t_and_s(credentials.password), - }, - }); - - GET = (query: Partial) => ({ - asJSON: () => getJSON(http2(this.subsonicHttp, query)), - }); + asAuthParams = (credentials: Credentials) => ({ + u: credentials.username, + ...t_and_s(credentials.password), + }) generateToken = (credentials: Credentials) => pipe( TE.tryCatch( - () => getJSON(http2(this.authenticatedSubsonicHttp(credentials), { url: "/rest/ping.view" })), + () => client(this.subsonic.with({ params: this.asAuthParams(credentials) } ))({ method: 'get', url: "/rest/ping.view" }).asJSON(), (e) => new AuthFailure(e as string) ), TE.chain(({ type }) => @@ -168,7 +161,7 @@ export class Subsonic implements MusicService { ): Promise => { const subsonicGenericLibrary = new SubsonicGenericMusicLibrary( this.streamClientApplication, - this.authenticatedSubsonicHttp(credentials) + this.subsonic.with({ params: this.asAuthParams(credentials) } ) ); if (credentials.type == "navidrome") { return Promise.resolve( diff --git a/src/subsonic/library.ts b/src/subsonic/library.ts index 8866a70..8a929bc 100644 --- a/src/subsonic/library.ts +++ b/src/subsonic/library.ts @@ -38,8 +38,8 @@ import { import axios from "axios"; import { asURLSearchParams } from "../utils"; import { artistSummaryFromNDArtist, NDArtist } from "./navidrome"; -import { Http, http as newHttp, RequestParams } from "../http"; -import { getRaw2, getJSON } from "./subsonic_http"; +import { Http2, RequestParams } from "../http"; +import { client } from "./subsonic_http"; type album = { id: string; @@ -276,20 +276,17 @@ const maybeAsGenre = (genreName: string | undefined): Genre | undefined => export class SubsonicGenericMusicLibrary implements SubsonicMusicLibrary { streamClientApplication: StreamClientApplication; - subsonicHttp: Http; + subsonicHttp: Http2; constructor( streamClientApplication: StreamClientApplication, - subsonicHttp: Http + subsonicHttp: Http2 ) { this.streamClientApplication = streamClientApplication; this.subsonicHttp = subsonicHttp; } - GET = (query: Partial) => ({ - asRAW: () => getRaw2(newHttp(this.subsonicHttp, query)), - asJSON: () => getJSON(newHttp(this.subsonicHttp, query)), - }); + GET = (query: Partial) => client(this.subsonicHttp)({ method: 'get', ...query }); flavour = () => "subsonic"; @@ -390,7 +387,6 @@ export class SubsonicGenericMusicLibrary implements SubsonicMusicLibrary { trackId: string; range: string | undefined; }) => - // todo: all these headers and stuff can be rolled into httpeee this.getTrack(trackId).then((track) => this.GET({ url: "/rest/stream", @@ -398,20 +394,10 @@ export class SubsonicGenericMusicLibrary implements SubsonicMusicLibrary { id: trackId, c: this.streamClientApplication(track), }, - headers: pipe( - range, - O.fromNullable, - O.map((range) => ({ - // "User-Agent": USER_AGENT, - Range: range, - })), - O.getOrElse(() => ({ - // "User-Agent": USER_AGENT, - })) - ), + headers: range != undefined ? { Range: range } : {}, responseType: "stream", }) - .asRAW() + .asRaw() .then((res) => ({ status: res.status, headers: { @@ -717,7 +703,7 @@ export class SubsonicGenericMusicLibrary implements SubsonicMusicLibrary { url: "/rest/getCoverArt", params: { id, size }, responseType: "arraybuffer", - }).asRAW(); + }).asRaw(); private getTrack = (id: string) => this.GET({ diff --git a/src/subsonic/subsonic_http.ts b/src/subsonic/subsonic_http.ts index bb57e41..390566e 100644 --- a/src/subsonic/subsonic_http.ts +++ b/src/subsonic/subsonic_http.ts @@ -1,9 +1,7 @@ -import { - isError, - SubsonicEnvelope, -} from "."; +import { AxiosResponse } from "axios"; +import { isError, SubsonicEnvelope } from "."; // todo: rename http2 to http -import { Http, http as newHttp } from "../http"; +import { Http2, RequestParams } from "../http"; export type HttpResponse = { data: any; @@ -11,21 +9,7 @@ export type HttpResponse = { headers: any; }; -export const getRaw2 = (http: Http) => - http({ method: "get" }) - .catch((e) => { - throw `Subsonic failed with: ${e}`; - }) - .then((response) => { - if (response.status != 200 && response.status != 206) { - throw `Subsonic failed with a ${response.status || "no!"} status`; - } else return response; - }); - -export const getJSON = async (http: Http): Promise => - getRaw2(newHttp(http, { params: { f: "json" } })).then(asJSON) as Promise; - -export const asJSON = (response: HttpResponse): T => { +const asJSON = (response: HttpResponse): T => { const subsonicResponse = (response.data as SubsonicEnvelope)[ "subsonic-response" ]; @@ -33,6 +17,35 @@ export const asJSON = (response: HttpResponse): T => { throw `Subsonic error:${subsonicResponse.error.message}`; else return subsonicResponse as unknown as T; }; +const throwUp = (error: any) => { + throw `Subsonic failed with: ${error}`; +}; +const verifyResponse = (response: AxiosResponse) => { + if (response.status != 200 && response.status != 206) { + throw `Subsonic failed with a ${response.status || "no!"} status`; + } else return response; +}; +export interface SubsonicHttpResponse { + asRaw(): Promise>; + asJSON(): Promise; +} +export interface SubsonicHttp { + (query: Partial): SubsonicHttpResponse; +} +export const client = (http: Http2): SubsonicHttp => { + return (query: Partial): SubsonicHttpResponse => { + return { + asRaw: () => http(query).catch(throwUp).then(verifyResponse), + + asJSON: () => + http + .with({ params: { f: "json" } })(query) + .catch(throwUp) + .then(verifyResponse) + .then(asJSON) as Promise, + }; + }; +}; diff --git a/src/utils.ts b/src/utils.ts index 3eba2c1..0315b0f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -22,11 +22,21 @@ export const asURLSearchParams = (q: any) => { return urlSearchParams; }; - -export function takeWithRepeats(things:T[], count: number) { +export function takeWithRepeats(things: T[], count: number) { const result = []; - for(let i = 0; i < count; i++) { - result.push(things[i % things.length]) + for (let i = 0; i < count; i++) { + result.push(things[i % things.length]); } return result; } + +export const mask = (thing: any, fields: string[]) => + fields.reduce( + (res: any, key: string) => { + if (Object.keys(res).includes(key)) { + res[key] = "****"; + } + return res; + }, + { ...thing } + ); diff --git a/tests/http.test.ts b/tests/http.test.ts index 85f69c6..5e2acf8 100644 --- a/tests/http.test.ts +++ b/tests/http.test.ts @@ -1,4 +1,4 @@ -import { http, } from "../src/http"; +import { http, http2From, } from "../src/http"; describe("http", () => { const mockAxios = jest.fn(); @@ -11,6 +11,7 @@ describe("http", () => { describe.each([ ["baseURL"], ["url"], + ["method"], ])('%s', (field) => { const getValue = (value: string) => { const thing = {} as any; @@ -136,3 +137,141 @@ describe("http", () => { }); }) }); + +describe("http2", () => { + const mockAxios = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + jest.resetAllMocks(); + }); + + describe.each([ + ["baseURL"], + ["url"], + ["method"], + ])('%s', (field) => { + const fieldWithValue = (value: string) => { + const thing = {} as any; + thing[field] = value; + return thing; + }; + + const base = http2From(mockAxios).with(fieldWithValue('default')); + + describe("using default", () => { + it("should use the default", () => { + base({}) + expect(mockAxios).toHaveBeenCalledWith(fieldWithValue('default')); + }); + }); + + describe("overriding", () => { + it("should use the override", () => { + base(fieldWithValue('override')) + expect(mockAxios).toHaveBeenCalledWith(fieldWithValue('override')); + }); + }); + + describe("wrapping", () => { + const firstLayer = http2From(base).with(fieldWithValue('level1')); + const secondLayer = firstLayer.with(fieldWithValue('level2')); + + describe("when the outter call provides a value", () => { + it("should apply it", () => { + secondLayer(fieldWithValue('outter')) + expect(mockAxios).toHaveBeenCalledWith(fieldWithValue('outter')); + }); + }); + + describe("when the outter call does not provide a value", () => { + it("should use the second layer", () => { + secondLayer({ }) + expect(mockAxios).toHaveBeenCalledWith(fieldWithValue('level2')); + }); + }); + }); + }); + + describe("requestType", () => { + const base = http2From(mockAxios).with({ responseType: 'stream' }); + + describe("using default", () => { + it("should use the default", () => { + base({}) + expect(mockAxios).toHaveBeenCalledWith({ responseType: 'stream' }); + }); + }); + + describe("overriding", () => { + it("should use the override", () => { + base({ responseType: 'arraybuffer' }) + expect(mockAxios).toHaveBeenCalledWith({ responseType: 'arraybuffer' }); + }); + }); + + describe("wrapping", () => { + const firstLayer = base.with({ responseType: 'arraybuffer' }); + const secondLayer = firstLayer.with({ responseType: 'blob' }); + + describe("when the outter call provides a value", () => { + it("should apply it", () => { + secondLayer({ responseType: 'text' }) + expect(mockAxios).toHaveBeenCalledWith({ responseType: 'text' }); + }); + }); + + describe("when the outter call does not provide a value", () => { + it("should use the second layer", () => { + secondLayer({ }) + expect(mockAxios).toHaveBeenCalledWith({ responseType: 'blob' }); + }); + }); + }); + }); + + describe.each([ + ["params"], + ["headers"], + ])('%s', (field) => { + const fieldWithValues = (values: any) => { + const thing = {} as any; + thing[field] = values; + return thing; + } + const base = http2From(mockAxios).with(fieldWithValues({ a: 1, b: 2, c: 3, d: 4 })); + + describe("using default", () => { + it("should use the default", () => { + base({}); + expect(mockAxios).toHaveBeenCalledWith(fieldWithValues({ a: 1, b: 2, c: 3, d: 4 })); + }); + }); + + describe("overriding", () => { + it("should use the override", () => { + base(fieldWithValues({ b: 22, e: 5 })); + expect(mockAxios).toHaveBeenCalledWith(fieldWithValues({ a: 1, b: 22, c: 3, d: 4, e: 5 })); + }); + }); + + describe("wrapping", () => { + const firstLayer = base.with(fieldWithValues({ b: 22 })); + const secondLayer = firstLayer.with(fieldWithValues({ c: 33 })); + + describe("when the outter call provides a value", () => { + it("should apply it", () => { + secondLayer(fieldWithValues({ a: 11, e: 5 })); + expect(mockAxios).toHaveBeenCalledWith(fieldWithValues({ a: 11, b: 22, c: 33, d: 4, e: 5 })); + }); + }); + + describe("when the outter call does not provide a value", () => { + it("should use the second layer", () => { + secondLayer({ }); + expect(mockAxios).toHaveBeenCalledWith(fieldWithValues({ a: 1, b: 22, c: 33, d: 4 })); + }); + }); + }); + }) +}); diff --git a/tests/subsonic/generic.test.ts b/tests/subsonic/generic.test.ts index e2039d7..0cc9dd9 100644 --- a/tests/subsonic/generic.test.ts +++ b/tests/subsonic/generic.test.ts @@ -46,7 +46,7 @@ import { import { EMPTY, error, FAILURE, subsonicOK, ok } from "../subsonic.test"; import { DODGY_IMAGE_NAME, t } from "../../src/subsonic"; import { b64Encode } from "../../src/b64"; -import { http as http2 } from "../../src/http"; +import { http2From } from "../../src/http"; const maybeIdFromCoverArtUrn = (coverArt: BUrn | undefined) => pipe( @@ -511,7 +511,7 @@ describe("SubsonicGenericMusicLibrary", () => { const generic = new SubsonicGenericMusicLibrary( streamClientApplication, // todo: all this stuff doesnt need to be defaulted in here. - http2(mockAxios, { + http2From(mockAxios).with({ baseURL, params: authParams, headers diff --git a/tests/utils.test.ts b/tests/utils.test.ts index f494703..3d1517a 100644 --- a/tests/utils.test.ts +++ b/tests/utils.test.ts @@ -1,4 +1,4 @@ -import { asURLSearchParams, takeWithRepeats } from "../src/utils"; +import { asURLSearchParams, mask, takeWithRepeats } from "../src/utils"; describe("asURLSearchParams", () => { describe("empty q", () => { @@ -46,8 +46,6 @@ describe("asURLSearchParams", () => { }); }); - - describe("takeWithRepeat", () => { describe("when there is nothing in the input", () => { it("should return an array of undefineds", () => { @@ -77,7 +75,32 @@ describe("takeWithRepeat", () => { describe("when there more than the amount required", () => { it("should return the first n items", () => { expect(takeWithRepeats(["a", "b", "c"], 2)).toEqual(["a", "b"]); - expect(takeWithRepeats(["a", undefined, "c"], 2)).toEqual(["a", undefined]); + expect(takeWithRepeats(["a", undefined, "c"], 2)).toEqual([ + "a", + undefined, + ]); }); }); }); + +describe("mask", () => { + it.each([ + [{}, ["a", "b"], {}], + [{ foo: "bar" }, ["a", "b"], { foo: "bar" }], + [{ a: 1 }, ["a", "b"], { a: "****" }], + [{ a: 1, b: "dog" }, ["a", "b"], { a: "****", b: "****" }], + [ + { a: 1, b: "dog", foo: "bar" }, + ["a", "b"], + { a: "****", b: "****", foo: "bar" }, + ], + ])( + "masking of %s, keys = %s, should result in %s", + (original: any, keys: string[], expected: any) => { + const copyOfOrig = JSON.parse(JSON.stringify(original)); + const masked = mask(original, keys); + expect(masked).toEqual(expected); + expect(original).toEqual(copyOfOrig); + } + ); +});