Fix: Exclude questionmark from URL when not sending request params (#140

) * split tests between unit and integration * rename test files to include '.test.' * use mocks in tests * use separate tsconfig for tests * add (failing) tests to assert desired behavior * fix implementation * prettier-ignore the output files
lionralfs · Apr 2, 2023 · 158c468 · 158c468
1 parent f24b539
commit 158c468
Show file tree

Hide file tree

Showing 31 changed files with 423 additions and 173 deletions.
diff --git a/.prettierignore b/.prettierignore
@@ -1,2 +1,4 @@
 commonjs/**/*.js
-coverage/
+coverage/
+browser/index.js
+node-esm/index.js
diff --git a/lib/collection.ts b/lib/collection.ts
@@ -184,7 +184,7 @@ export default function (client: DiscogsClient) {
                 SortParameters<'label' | 'artist' | 'title' | 'catno' | 'format' | 'rating' | 'added' | 'year'>
         ): Promise<RateLimitedResponse<GetReleasesResponse & PaginationResponse>> {
             if (client.authenticated(2) || Number(folder) === 0) {
-                const path = `/users/${escape(user)}/collection/folders/${folder}/releases?${toQueryString(params)}`;
+                const path = `/users/${escape(user)}/collection/folders/${folder}/releases${toQueryString(params)}`;
                 return client.get(path) as Promise<RateLimitedResponse<GetReleasesResponse & PaginationResponse>>;
             }
             return Promise.reject(new AuthError());

diff --git a/lib/database.ts b/lib/database.ts
@@ -210,7 +210,7 @@ export default function (client: DiscogsClient) {
             artist: number | string,
             params?: PaginationParameters & SortParameters<'year' | 'title' | 'format'>
         ): Promise<RateLimitedResponse<GetArtistReleasesResponses & PaginationResponse>> {
-            const path = `/artists/${artist}/releases?${toQueryString(params)}`;
+            const path = `/artists/${artist}/releases${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<GetArtistReleasesResponses & PaginationResponse>>;
         },
 
@@ -234,7 +234,7 @@ export default function (client: DiscogsClient) {
         ): Promise<RateLimitedResponse<GetReleaseResponse>> {
             let path = `/releases/${release}`;
             if (currency !== undefined) {
-                path += `?${toQueryString({ curr_abbr: currency })}`;
+                path += `${toQueryString({ curr_abbr: currency })}`;
             }
             return client.get(path) as Promise<RateLimitedResponse<GetReleaseResponse>>;
         },
@@ -366,7 +366,7 @@ export default function (client: DiscogsClient) {
                     >
                 >
         ): Promise<RateLimitedResponse<GetMasterVersionsResponse & PaginationResponse>> {
-            const path = `/masters/${master}/versions?${toQueryString(params)}`;
+            const path = `/masters/${master}/versions${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<GetMasterVersionsResponse & PaginationResponse>>;
         },
 
@@ -399,7 +399,7 @@ export default function (client: DiscogsClient) {
             label: number | string,
             params?: PaginationParameters
         ): Promise<RateLimitedResponse<GetLabelReleasesResponse & PaginationResponse>> {
-            const path = `/labels/${label}/releases?${toQueryString(params)}`;
+            const path = `/labels/${label}/releases${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<GetLabelReleasesResponse & PaginationResponse>>;
         },
 
@@ -436,14 +436,9 @@ export default function (client: DiscogsClient) {
         search: function (
             params: PaginationParameters & Partial<SearchParameters> = {}
         ): Promise<RateLimitedResponse<SearchResponse & PaginationResponse>> {
-            const args = { ...params };
-            if (args.query) {
-                // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-                // @ts-ignore
-                args.q = args.query;
-                delete args.query;
-            }
-            return client.get({ url: `/database/search?${toQueryString(args)}`, authLevel: 1 }) as Promise<
+            const { query, ...inputArgs } = params;
+            const args = query ? Object.assign(inputArgs, { q: query }) : inputArgs;
+            return client.get({ url: `/database/search${toQueryString(args)}`, authLevel: 1 }) as Promise<
                 RateLimitedResponse<SearchResponse & PaginationResponse>
             >;
         },

diff --git a/lib/marketplace.ts b/lib/marketplace.ts
@@ -101,7 +101,7 @@ export default function (client: DiscogsClient) {
         getListing: function (listing: number, currency?: Currency): Promise<RateLimitedResponse<Listing>> {
             let path = `/marketplace/listings/${listing}`;
             if (currency !== undefined) {
-                path += `?${toQueryString({ curr_abbr: currency })}`;
+                path += `${toQueryString({ curr_abbr: currency })}`;
             }
             return client.get(path) as Promise<RateLimitedResponse<Listing>>;
         },
@@ -229,7 +229,7 @@ export default function (client: DiscogsClient) {
                 PaginationParameters &
                 SortParameters<'id' | 'buyer' | 'created' | 'status' | 'last_activity'>
         ): Promise<RateLimitedResponse<PaginationResponse & { orders: Array<GetOrderResponse> }>> {
-            const path = `/marketplace/orders?${toQueryString(params)}`;
+            const path = `/marketplace/orders${toQueryString(params)}`;
             return client.get({ url: path, authLevel: 2 }) as Promise<
                 RateLimitedResponse<PaginationResponse & { orders: Array<GetOrderResponse> }>
             >;
@@ -286,7 +286,7 @@ export default function (client: DiscogsClient) {
             order: number,
             params?: PaginationParameters
         ): Promise<RateLimitedResponse<PaginationResponse & { messages: Array<OrderMessage> }>> {
-            const path = `/marketplace/orders/${order}/messages?${toQueryString(params)}`;
+            const path = `/marketplace/orders/${order}/messages${toQueryString(params)}`;
             return client.get({ url: path, authLevel: 2 }) as Promise<
                 RateLimitedResponse<PaginationResponse & { messages: Array<OrderMessage> }>
             >;
@@ -370,9 +370,9 @@ export default function (client: DiscogsClient) {
         ): Promise<RateLimitedResponse<GetReleaseStatsResponse>> {
             let path = `/marketplace/stats/${release}`;
             if (currency) {
-                path += `?${toQueryString({ curr_abbr: currency })}`;
+                path += `${toQueryString({ curr_abbr: currency })}`;
             }
-            return client.get({ url: path }) as Promise<RateLimitedResponse<GetReleaseStatsResponse>>;
+            return client.get(path) as Promise<RateLimitedResponse<GetReleaseStatsResponse>>;
         },
     };
 }
diff --git a/lib/user.ts b/lib/user.ts
@@ -144,7 +144,7 @@ export default function (client: DiscogsClient) {
                     'listed' | 'price' | 'item' | 'artist' | 'label' | 'catno' | 'audio' | 'status' | 'location'
                 >
         ): Promise<RateLimitedResponse<GetInventoryResponse & PaginationResponse>> {
-            const path = `/users/${escape(user)}/inventory?${toQueryString(params)}`;
+            const path = `/users/${escape(user)}/inventory${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<GetInventoryResponse & PaginationResponse>>;
         },
 
@@ -198,7 +198,7 @@ export default function (client: DiscogsClient) {
             params?: PaginationParameters &
                 SortParameters<'label' | 'artist' | 'title' | 'catno' | 'format' | 'rating' | 'year' | 'added'>
         ): Promise<RateLimitedResponse<GetContributionsResponse & PaginationResponse>> {
-            const path = `/users/${escape(user)}/contributions?${toQueryString(params)}`;
+            const path = `/users/${escape(user)}/contributions${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<GetContributionsResponse & PaginationResponse>>;
         },
 
@@ -217,7 +217,7 @@ export default function (client: DiscogsClient) {
             user: string,
             params?: PaginationParameters
         ): Promise<RateLimitedResponse<PaginationResponse & GetSubmissionsResponse>> {
-            const path = `/users/${escape(user)}/submissions?${toQueryString(params)}`;
+            const path = `/users/${escape(user)}/submissions${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<PaginationResponse & GetSubmissionsResponse>>;
         },
 
@@ -236,7 +236,7 @@ export default function (client: DiscogsClient) {
             user: string,
             params?: PaginationParameters
         ): Promise<RateLimitedResponse<PaginationResponse & GetListsResponse>> {
-            const path = `/users/${escape(user)}/lists?${toQueryString(params)}`;
+            const path = `/users/${escape(user)}/lists${toQueryString(params)}`;
             return client.get(path) as Promise<RateLimitedResponse<PaginationResponse & GetListsResponse>>;
         },
     };

diff --git a/lib/util.ts b/lib/util.ts
@@ -8,20 +8,20 @@ export function stripVariation(name: string): string {
 }
 
 /**
- * Turns a simple key-value object into a query string
+ * Turns a simple key-value object into a query string (including the leading questionmark)
  * @param {Record<string, string | number>} data - Data object containing the params
  * @returns {string}
  */
 export function toQueryString(data?: Record<PropertyKey, string | number | boolean>): string {
-    if (!data) {
+    if (!data || !Object.keys(data).length) {
         return '';
     }
 
     const searchParams = new URLSearchParams();
     for (const [key, value] of Object.entries(data)) {
         searchParams.set(key, value.toString());
     }
-    return searchParams.toString();
+    return '?' + searchParams.toString();
 }
 
 /**

diff --git a/lib/wantlist.ts b/lib/wantlist.ts
@@ -45,9 +45,9 @@ export default function (client: DiscogsClient) {
          */
         getReleases: function (
             user: string,
-            params: PaginationParameters
+            params?: PaginationParameters
         ): Promise<RateLimitedResponse<PaginationResponse & { wants: Array<WantlistEntryResponse> }>> {
-            const path = `/users/${escape(user)}/wants?${toQueryString(params)}`;
+            const path = `/users/${escape(user)}/wants${toQueryString(params)}`;
 
             return client.get(path) as Promise<
                 RateLimitedResponse<PaginationResponse & { wants: Array<WantlistEntryResponse> }>

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -38,6 +38,7 @@
     },
     "devDependencies": {
         "@esbuild-kit/esm-loader": "^2.5.0",
+        "@fluffy-spoon/substitute": "^1.208.0",
         "@typescript-eslint/eslint-plugin": "^5.41.0",
         "@typescript-eslint/parser": "^5.41.0",
         "ava": "^5.0.1",

diff --git a/test/error.ts b/test/error.ts
diff --git a/test/_setup.ts → test/integration/_setup.test.ts b/test/_setup.ts → test/integration/_setup.test.ts
diff --git a/test/client.ts → test/integration/client.test.ts b/test/client.ts → test/integration/client.test.ts
@@ -1,36 +1,10 @@
 import test from 'ava';
 import { rest } from 'msw';
-import { DiscogsClient } from '../lib/client.js';
-import { setupMockAPI } from './_setup.js';
+import { DiscogsClient } from '@lib/client.js';
+import { setupMockAPI } from './_setup.test.js';
 
 const server = setupMockAPI();
 
-test('DiscogsClient: Test instance', t => {
-    t.true(new DiscogsClient() instanceof DiscogsClient);
-});
-
-test('DiscogsClient: Test authenticated()', t => {
-    t.false(new DiscogsClient().authenticated(1), 'Authentication level 1 === false');
-});
-
-test('DiscogsClient: Test setConfig with exponential backoff parameters', t => {
-    // Given
-    const client = new DiscogsClient();
-
-    // When
-    client.setConfig({
-        exponentialBackoffMaxRetries: 333,
-        exponentialBackoffIntervalMs: 444,
-        exponentialBackoffRate: 555,
-    });
-
-    // Then
-
-    t.is(client['config'].exponentialBackoffMaxRetries, 333);
-    t.is(client['config'].exponentialBackoffIntervalMs, 444);
-    t.is(client['config'].exponentialBackoffRate, 555);
-});
-
 test.serial('DiscogsClient: Test get()', async t => {
     t.plan(1);
     server.use(
@@ -127,44 +101,6 @@ test.serial('DiscogsClient: Auth (userToken)', async t => {
     await client.getIdentity();
 });
 
-test('DiscogsClient: Auth (Full OAuth)', t => {
-    const client = new DiscogsClient({
-        auth: {
-            method: 'oauth',
-            consumerKey: 'consumerKey',
-            consumerSecret: 'consumerSecret',
-            accessToken: 'accessToken',
-            accessTokenSecret: 'accessTokenSecret',
-        },
-    });
-
-    t.deepEqual(client['auth'], {
-        method: 'oauth',
-        level: 2,
-        consumerKey: 'consumerKey',
-        consumerSecret: 'consumerSecret',
-        accessToken: 'accessToken',
-        accessTokenSecret: 'accessTokenSecret',
-    });
-});
-
-test('DiscogsClient: Auth (OAuth without tokens)', t => {
-    const client = new DiscogsClient({
-        auth: {
-            method: 'oauth',
-            consumerKey: 'consumerKey',
-            consumerSecret: 'consumerSecret',
-        },
-    });
-
-    t.deepEqual(client['auth'], {
-        method: 'oauth',
-        level: 1,
-        consumerKey: 'consumerKey',
-        consumerSecret: 'consumerSecret',
-    });
-});
-
 test.serial('DiscogsClient: Sends OAuth header', async t => {
     t.plan(1);
 

diff --git a/test/collection.ts → test/integration/collection.test.ts b/test/collection.ts → test/integration/collection.test.ts
@@ -1,7 +1,7 @@
 import test from 'ava';
 import { rest } from 'msw';
-import { DiscogsClient } from '../lib/client.js';
-import { setupMockAPI } from './_setup.js';
+import { DiscogsClient } from '@lib/client.js';
+import { setupMockAPI } from './_setup.test.js';
 
 const server = setupMockAPI();
 
@@ -53,11 +53,6 @@ test.serial('Collection: Get folder metadata (no auth required for public folder
     await client.user().collection().getFolder('rodneyfool', 0);
 });
 
-test('Collection: Get folder metadata (throws auth error)', async t => {
-    const client = new DiscogsClient();
-    await t.throwsAsync(client.user().collection().getFolder('rodneyfool', 1234));
-});
-
 test.serial('Collection: Edit folder name', async t => {
     t.plan(1);
     server.use(
@@ -130,14 +125,6 @@ test.serial('Collection: Collection items by folder (default doesnt need auth)',
     await client.user().collection().getReleases('rodneyfool', '0', { sort: 'artist', sort_order: 'desc' });
 });
 
-test('Collection: Collection items by folder (throws auth error)', async t => {
-    t.plan(1);
-    const client = new DiscogsClient();
-    await t.throwsAsync(
-        client.user().collection().getReleases('rodneyfool', '1234', { sort: 'artist', sort_order: 'desc' })
-    );
-});
-
 test.serial('Collection: Add release to folder', async t => {
     t.plan(1);
     server.use(