Change BNB_SUBSONIC_URL to be of type URLBuilder to better handle URL…

… construction rather than string concat, should addresse #169
simojenki · Oct 9, 2023 · 0c70c5a · 0c70c5a
1 parent 378346a
commit 0c70c5a
Show file tree

Hide file tree

Showing 4 changed files with 150 additions and 151 deletions.
diff --git a/src/config.ts b/src/config.ts
@@ -58,8 +58,6 @@ const asBoolean = (value: string) => value == "true";
 
 const asInt = (value: string) => Number.parseInt(value);
 
-const asStringNoTrailingSlash = (value: string) => value.replace(/\/$/, "");
-
 export default function () {
   const port = bnbEnvVar<number>("PORT", { default: 4534, parser: asInt })!;
   const bonobUrl = bnbEnvVar("URL", {
@@ -100,8 +98,7 @@ export default function () {
       sid: bnbEnvVar<number>("SONOS_SERVICE_ID", { default: 246, parser: asInt }),
     },
     subsonic: {
-      // todo: why isnt this a url like bonobUrl above?
-      url: bnbEnvVar("SUBSONIC_URL", { legacy: ["BONOB_NAVIDROME_URL"], default: `http://${hostname()}:4533`, parser: asStringNoTrailingSlash })!,
+      url: url(bnbEnvVar("SUBSONIC_URL", { legacy: ["BONOB_NAVIDROME_URL"], default: `http://${hostname()}:4533` })!),
       customClientsFor: bnbEnvVar<string>("SUBSONIC_CUSTOM_CLIENTS", { legacy: ["BONOB_NAVIDROME_CUSTOM_CLIENTS"] }),
       artistImageCache: bnbEnvVar<string>("SUBSONIC_ARTIST_IMAGE_CACHE"),
     },

diff --git a/src/subsonic.ts b/src/subsonic.ts
@@ -32,6 +32,7 @@ import { b64Encode, b64Decode } from "./b64";
 import logger from "./logger";
 import { assertSystem, BUrn } from "./burn";
 import { artist } from "./smapi";
+import { URLBuilder } from "./url_builder";
 
 export const BROWSER_HEADERS = {
   accept:
@@ -344,28 +345,28 @@ export type ImageFetcher = (url: string) => Promise<CoverArt | undefined>;
 
 export const cachingImageFetcher =
   (cacheDir: string, delegate: ImageFetcher) =>
-  async (url: string): Promise<CoverArt | undefined> => {
-    const filename = path.join(cacheDir, `${Md5.hashStr(url)}.png`);
-    return fse
-      .readFile(filename)
-      .then((data) => ({ contentType: "image/png", data }))
-      .catch(() =>
-        delegate(url).then((image) => {
-          if (image) {
-            return sharp(image.data)
-              .png()
-              .toBuffer()
-              .then((png) => {
-                return fse
-                  .writeFile(filename, png)
-                  .then(() => ({ contentType: "image/png", data: png }));
-              });
-          } else {
-            return undefined;
-          }
-        })
-      );
-  };
+    async (url: string): Promise<CoverArt | undefined> => {
+      const filename = path.join(cacheDir, `${Md5.hashStr(url)}.png`);
+      return fse
+        .readFile(filename)
+        .then((data) => ({ contentType: "image/png", data }))
+        .catch(() =>
+          delegate(url).then((image) => {
+            if (image) {
+              return sharp(image.data)
+                .png()
+                .toBuffer()
+                .then((png) => {
+                  return fse
+                    .writeFile(filename, png)
+                    .then(() => ({ contentType: "image/png", data: png }));
+                });
+            } else {
+              return undefined;
+            }
+          })
+        );
+    };
 
 export const axiosImageFetcher = (url: string): Promise<CoverArt | undefined> =>
   axios
@@ -412,12 +413,12 @@ interface SubsonicMusicLibrary extends MusicLibrary {
 }
 
 export class Subsonic implements MusicService {
-  url: string;
+  url: URLBuilder;
   streamClientApplication: StreamClientApplication;
   externalImageFetcher: ImageFetcher;
 
   constructor(
-    url: string,
+    url: URLBuilder,
     streamClientApplication: StreamClientApplication = DEFAULT,
     externalImageFetcher: ImageFetcher = axiosImageFetcher
   ) {
@@ -433,7 +434,7 @@ export class Subsonic implements MusicService {
     config: AxiosRequestConfig | undefined = {}
   ) =>
     axios
-      .get(`${this.url}${path}`, {
+      .get(this.url.append({ pathname: path }).href(), {
         params: asURLSearchParams({
           u: username,
           v: "1.16.1",

diff --git a/tests/config.test.ts b/tests/config.test.ts
@@ -374,31 +374,31 @@ describe("config", () => {
         "BONOB_NAVIDROME_URL",
       ])("%s", (k) => {
         describe(`when ${k} is not specified`, () => {
-          it(`should default to http://${hostname()}:4533`, () => {
-            expect(config().subsonic.url).toEqual(`http://${hostname()}:4533`);
+          it(`should default to http://${hostname()}:4533/`, () => {
+            expect(config().subsonic.url.href()).toEqual(`http://${hostname()}:4533/`);
           });
         });
 
         describe(`when ${k} is ''`, () => {
-          it(`should default to http://${hostname()}:4533`, () => {
+          it(`should default to http://${hostname()}:4533/`, () => {
             process.env[k] = "";
-            expect(config().subsonic.url).toEqual(`http://${hostname()}:4533`);
+            expect(config().subsonic.url.href()).toEqual(`http://${hostname()}:4533/`);
           });
         });
 
         describe(`when ${k} is specified`, () => {
           it(`should use it for ${k}`, () => {
-            const url = "http://navidrome.example.com:1234";
+            const url = "http://navidrome.example.com:1234/some-context-path";
             process.env[k] = url;
-            expect(config().subsonic.url).toEqual(url);
+            expect(config().subsonic.url.href()).toEqual(url);
           });
         });
 
         describe(`when ${k} is specified with trailing slash`, () => {
-          it(`should remove the trailing slash and use it for ${k}`, () => {
-            const url = "http://navidrome.example.com:1234";
-            process.env[k] = `${url}/`;
-            expect(config().subsonic.url).toEqual(url);
+          it(`should maintain the trailing slash as URLBuilder will remove it when required ${k}`, () => {
+            const url = "http://navidrome.example.com:1234/";
+            process.env[k] = url;
+            expect(config().subsonic.url.href()).toEqual(url);
           });
         });
       });