From e7c911a523f1c339ab40ed5846b6be860aad6d1f Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:11:56 +0100 Subject: [PATCH 01/10] Format with prettier --- src/APL/saleor-cloud/saleor-cloud-apl.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.ts b/src/APL/saleor-cloud/saleor-cloud-apl.ts index 1592398f..5870ba73 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.ts @@ -38,7 +38,7 @@ const validateResponseStatus = (response: Response) => { throw new SaleorCloudAplError( CloudAplError.RESPONSE_NON_200, - `Fetch returned with non 200 status code ${response.status}` + `Fetch returned with non 200 status code ${response.status}`, ); } }; @@ -161,7 +161,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.FAILED_TO_REACH_API, - `${extractErrorMessage(error)}` + `${extractErrorMessage(error)}`, ); }); @@ -184,7 +184,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.FAILED_TO_REACH_API, - "Response couldn't be resolved" + "Response couldn't be resolved", ); } @@ -254,7 +254,7 @@ export class SaleorCloudAPL implements APL { span.end(); return authData; - } + }, ); } @@ -289,7 +289,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.ERROR_SAVING_DATA, - `Error during saving the data: ${extractErrorMessage(e)}` + `Error during saving the data: ${extractErrorMessage(e)}`, ); }); @@ -305,7 +305,7 @@ export class SaleorCloudAPL implements APL { span.end(); return undefined; - } + }, ); } @@ -329,7 +329,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.ERROR_DELETING_DATA, - `Error during deleting the data: ${errorMessage}` + `Error during deleting the data: ${errorMessage}`, ); } } @@ -346,7 +346,7 @@ export class SaleorCloudAPL implements APL { debug(`Get all responded with ${response.status} code`); return ((await response.json()) as GetAllAplResponseShape).results.map( - mapAPIResponseToAuthData + mapAPIResponseToAuthData, ); } catch (error) { const errorMessage = extractErrorMessage(error); From a7faf73601b583c67aa1514ff980bda914452d11 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:12:09 +0100 Subject: [PATCH 02/10] Add limit=1000 for getAll requests in cloud apl --- src/APL/saleor-cloud/saleor-cloud-apl.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.ts b/src/APL/saleor-cloud/saleor-cloud-apl.ts index 5870ba73..861b3fe9 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.ts @@ -338,7 +338,7 @@ export class SaleorCloudAPL implements APL { debug("Get all data from SaleorCloud"); try { - const response = await fetch(this.resourceUrl, { + const response = await fetch(`${this.resourceUrl}?limit=1000`, { method: "GET", headers: { "Content-Type": "application/json", ...this.headers }, }); From e7f46ab656d052e7435d21335850523105870631 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:39:38 +0100 Subject: [PATCH 03/10] Add paginator and option to configure limit --- src/APL/saleor-cloud/paginator.ts | 54 ++++++++++++++++++++++++ src/APL/saleor-cloud/saleor-cloud-apl.ts | 16 ++++--- 2 files changed, 63 insertions(+), 7 deletions(-) create mode 100644 src/APL/saleor-cloud/paginator.ts diff --git a/src/APL/saleor-cloud/paginator.ts b/src/APL/saleor-cloud/paginator.ts new file mode 100644 index 00000000..742edd19 --- /dev/null +++ b/src/APL/saleor-cloud/paginator.ts @@ -0,0 +1,54 @@ +import { createAPLDebug } from "../apl-debug"; + +const debug = createAPLDebug("Paginator"); + +interface PaginationResponse { + next: string | null; + previous: string | null; + results: ResultType[]; +} + +export class Paginator { + constructor( + private url: string, + private fetchOptions: RequestInit, + private fetchFn = fetch, + ) {} + + public async fetchAll() { + debug("Fetching all pages for url", this.url); + const response = await this.fetchFn(this.url, this.fetchOptions); + debug("%0", response); + + const json = (await response.json()) as PaginationResponse; + + if (json.next) { + const remainingPages = await this.fetchNext(json.next); + const allResults = [...json.results, ...remainingPages.flatMap((page) => page.results)]; + debug("Fetched all pages, total length: %d", allResults.length); + + return { + next: null, + previous: null, + count: allResults.length, + results: allResults, + }; + } + + debug("No more pages to fetch, returning first page"); + return json; + } + + private async fetchNext(nextUrl: string): Promise>> { + debug("Fetching next page with url %s", nextUrl); + const response = await this.fetchFn(nextUrl, this.fetchOptions); + debug("%0", response); + + const json = (await response.json()) as PaginationResponse; + + if (json.next) { + return [json, ...(await this.fetchNext(json.next))]; + } + return [json]; + } +} diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.ts b/src/APL/saleor-cloud/saleor-cloud-apl.ts index 861b3fe9..e0785ad9 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.ts @@ -7,6 +7,7 @@ import { APL, AplConfiguredResult, AplReadyResult, AuthData } from "../apl"; import { createAPLDebug } from "../apl-debug"; import { authDataFromObject } from "../auth-data-from-object"; import { CloudAplError, SaleorCloudAplError } from "./saleor-cloud-apl-errors"; +import { Paginator } from "./paginator"; const debug = createAPLDebug("SaleorCloudAPL"); @@ -16,6 +17,7 @@ export type SaleorCloudAPLConfig = { experimental?: { cacheManager?: Map; }; + pageLimit?: number; }; type CloudAPLAuthDataShape = { @@ -104,6 +106,10 @@ export class SaleorCloudAPL implements APL { return `${this.resourceUrl}/${Buffer.from(saleorApiUrl).toString("base64url")}`; } + private getUrlWithLimit() { + return `${this.resourceUrl}?limit=${this.pageLimit ?? 1000}`; + } + private setToCacheIfExists(saleorApiUrl: string, authData: AuthData) { if (!this.cacheManager) { return; @@ -338,16 +344,12 @@ export class SaleorCloudAPL implements APL { debug("Get all data from SaleorCloud"); try { - const response = await fetch(`${this.resourceUrl}?limit=1000`, { + const paginator = new Paginator(this.getUrlWithLimit(), { method: "GET", headers: { "Content-Type": "application/json", ...this.headers }, }); - - debug(`Get all responded with ${response.status} code`); - - return ((await response.json()) as GetAllAplResponseShape).results.map( - mapAPIResponseToAuthData, - ); + const responses = await paginator.fetchAll(); + return responses.results.map(mapAPIResponseToAuthData); } catch (error) { const errorMessage = extractErrorMessage(error); From 30485b766e9448c12c877f80809a1c838b27211a Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:46:17 +0100 Subject: [PATCH 04/10] Add changeset --- .changeset/violet-donuts-begin.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/violet-donuts-begin.md diff --git a/.changeset/violet-donuts-begin.md b/.changeset/violet-donuts-begin.md new file mode 100644 index 00000000..abbf7613 --- /dev/null +++ b/.changeset/violet-donuts-begin.md @@ -0,0 +1,5 @@ +--- +"@saleor/app-sdk": minor +--- + +Added pagination for CloudAPL. Previously if there were more than 100 results it would return only first 100. This change adds an option to configure the page size and automatically paginates through the responses until `next` property is set to null on the response From 0c9ef34459282c4f78eac5cb0fa0611cfa3ac944 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:48:07 +0100 Subject: [PATCH 05/10] Fix TS error --- src/APL/saleor-cloud/saleor-cloud-apl.ts | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.ts b/src/APL/saleor-cloud/saleor-cloud-apl.ts index e0785ad9..4dd2e9d5 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.ts @@ -6,8 +6,8 @@ import { getOtelTracer, OTEL_APL_SERVICE_NAME } from "../../open-telemetry"; import { APL, AplConfiguredResult, AplReadyResult, AuthData } from "../apl"; import { createAPLDebug } from "../apl-debug"; import { authDataFromObject } from "../auth-data-from-object"; -import { CloudAplError, SaleorCloudAplError } from "./saleor-cloud-apl-errors"; import { Paginator } from "./paginator"; +import { CloudAplError, SaleorCloudAplError } from "./saleor-cloud-apl-errors"; const debug = createAPLDebug("SaleorCloudAPL"); @@ -40,7 +40,7 @@ const validateResponseStatus = (response: Response) => { throw new SaleorCloudAplError( CloudAplError.RESPONSE_NON_200, - `Fetch returned with non 200 status code ${response.status}`, + `Fetch returned with non 200 status code ${response.status}` ); } }; @@ -91,6 +91,8 @@ export class SaleorCloudAPL implements APL { private cacheManager?: Map; + private readonly pageLimit: number; + constructor(config: SaleorCloudAPLConfig) { this.resourceUrl = config.resourceUrl; this.headers = { @@ -99,6 +101,7 @@ export class SaleorCloudAPL implements APL { this.tracer = getOtelTracer(); this.cacheManager = config?.experimental?.cacheManager; + this.pageLimit = config.pageLimit ?? 1000; } private getUrlForDomain(saleorApiUrl: string) { @@ -107,7 +110,7 @@ export class SaleorCloudAPL implements APL { } private getUrlWithLimit() { - return `${this.resourceUrl}?limit=${this.pageLimit ?? 1000}`; + return `${this.resourceUrl}?limit=${this.pageLimit}`; } private setToCacheIfExists(saleorApiUrl: string, authData: AuthData) { @@ -167,7 +170,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.FAILED_TO_REACH_API, - `${extractErrorMessage(error)}`, + `${extractErrorMessage(error)}` ); }); @@ -190,7 +193,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.FAILED_TO_REACH_API, - "Response couldn't be resolved", + "Response couldn't be resolved" ); } @@ -260,7 +263,7 @@ export class SaleorCloudAPL implements APL { span.end(); return authData; - }, + } ); } @@ -295,7 +298,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.ERROR_SAVING_DATA, - `Error during saving the data: ${extractErrorMessage(e)}`, + `Error during saving the data: ${extractErrorMessage(e)}` ); }); @@ -311,7 +314,7 @@ export class SaleorCloudAPL implements APL { span.end(); return undefined; - }, + } ); } @@ -335,7 +338,7 @@ export class SaleorCloudAPL implements APL { throw new SaleorCloudAplError( CloudAplError.ERROR_DELETING_DATA, - `Error during deleting the data: ${errorMessage}`, + `Error during deleting the data: ${errorMessage}` ); } } From 925591a35e6f0034d32ca196712e4f6982b508f7 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:52:52 +0100 Subject: [PATCH 06/10] Fix issues with "useless constructor" error when using TypeScript shorthand --- .eslintrc | 1 + package.json | 5 +++-- pnpm-lock.yaml | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/.eslintrc b/.eslintrc index 64841845..f7c26f4e 100644 --- a/.eslintrc +++ b/.eslintrc @@ -6,6 +6,7 @@ }, "extends": [ "airbnb", + "airbnb-typescript", "plugin:@typescript-eslint/recommended", "prettier" // prettier *has* to be the last one, to avoid conflicting rules ], diff --git a/package.json b/package.json index c36d55bd..63b6ad3a 100644 --- a/package.json +++ b/package.json @@ -19,11 +19,11 @@ "author": "", "license": "ISC", "peerDependencies": { + "@vercel/kv": "^1.0.0", "graphql": ">=16.6.0", "next": ">=12", "react": ">=17", - "react-dom": ">=17", - "@vercel/kv": "^1.0.0" + "react-dom": ">=17" }, "dependencies": { "@opentelemetry/api": "^1.7.0", @@ -51,6 +51,7 @@ "clean-publish": "^4.0.1", "eslint": "8.23.0", "eslint-config-airbnb": "^19.0.4", + "eslint-config-airbnb-typescript": "^17.1.0", "eslint-config-prettier": "^8.5.0", "eslint-import-resolver-typescript": "^3.5.0", "eslint-plugin-import": "^2.26.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ceb5315c..53539fcc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -76,6 +76,9 @@ devDependencies: eslint-config-airbnb: specifier: ^19.0.4 version: 19.0.4(eslint-plugin-import@2.26.0)(eslint-plugin-jsx-a11y@6.6.1)(eslint-plugin-react-hooks@4.6.0)(eslint-plugin-react@7.31.6)(eslint@8.23.0) + eslint-config-airbnb-typescript: + specifier: ^17.1.0 + version: 17.1.0(@typescript-eslint/eslint-plugin@5.36.1)(@typescript-eslint/parser@5.36.1)(eslint-plugin-import@2.26.0)(eslint@8.23.0) eslint-config-prettier: specifier: ^8.5.0 version: 8.5.0(eslint@8.23.0) @@ -2737,6 +2740,21 @@ packages: semver: 6.3.0 dev: true + /eslint-config-airbnb-typescript@17.1.0(@typescript-eslint/eslint-plugin@5.36.1)(@typescript-eslint/parser@5.36.1)(eslint-plugin-import@2.26.0)(eslint@8.23.0): + resolution: {integrity: sha512-GPxI5URre6dDpJ0CtcthSZVBAfI+Uw7un5OYNVxP2EYi3H81Jw701yFP7AU+/vCE7xBtFmjge7kfhhk4+RAiig==} + peerDependencies: + '@typescript-eslint/eslint-plugin': ^5.13.0 || ^6.0.0 + '@typescript-eslint/parser': ^5.0.0 || ^6.0.0 + eslint: ^7.32.0 || ^8.2.0 + eslint-plugin-import: ^2.25.3 + dependencies: + '@typescript-eslint/eslint-plugin': 5.36.1(@typescript-eslint/parser@5.36.1)(eslint@8.23.0)(typescript@4.9.5) + '@typescript-eslint/parser': 5.36.1(eslint@8.23.0)(typescript@4.9.5) + eslint: 8.23.0 + eslint-config-airbnb-base: 15.0.0(eslint-plugin-import@2.26.0)(eslint@8.23.0) + eslint-plugin-import: 2.26.0(@typescript-eslint/parser@5.36.1)(eslint-import-resolver-typescript@3.5.0)(eslint@8.23.0) + dev: true + /eslint-config-airbnb@19.0.4(eslint-plugin-import@2.26.0)(eslint-plugin-jsx-a11y@6.6.1)(eslint-plugin-react-hooks@4.6.0)(eslint-plugin-react@7.31.6)(eslint@8.23.0): resolution: {integrity: sha512-T75QYQVQX57jiNgpF9r1KegMICE94VYwoFQyMGhrvc+lB8YF2E/M/PYDaQe1AJcWaEgqLE+ErXV1Og/+6Vyzew==} engines: {node: ^10.12.0 || ^12.22.0 || ^14.17.0 || >=16.0.0} From bf5b63e205550b9b7d9b7768f3b185e29cdb5a51 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 15:53:15 +0100 Subject: [PATCH 07/10] Add paginator --- src/APL/saleor-cloud/paginator.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/APL/saleor-cloud/paginator.ts b/src/APL/saleor-cloud/paginator.ts index 742edd19..172ffb35 100644 --- a/src/APL/saleor-cloud/paginator.ts +++ b/src/APL/saleor-cloud/paginator.ts @@ -10,9 +10,9 @@ interface PaginationResponse { export class Paginator { constructor( - private url: string, - private fetchOptions: RequestInit, - private fetchFn = fetch, + private readonly url: string, + private readonly fetchOptions: RequestInit, + private readonly fetchFn = fetch ) {} public async fetchAll() { From f4748bc0b5a59b927e259f7c721f6ea321616209 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 16:04:34 +0100 Subject: [PATCH 08/10] Add tests --- src/APL/saleor-cloud/paginator.test.ts | 64 ++++++++++++++++++ src/APL/saleor-cloud/saleor-cloud-apl.test.ts | 66 +++++++++++++++++++ src/APL/saleor-cloud/saleor-cloud-apl.ts | 2 + 3 files changed, 132 insertions(+) create mode 100644 src/APL/saleor-cloud/paginator.test.ts diff --git a/src/APL/saleor-cloud/paginator.test.ts b/src/APL/saleor-cloud/paginator.test.ts new file mode 100644 index 00000000..c732e36f --- /dev/null +++ b/src/APL/saleor-cloud/paginator.test.ts @@ -0,0 +1,64 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { Paginator } from "./paginator"; + +const fetchMock = vi.fn(); + +describe("Paginator", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("Returns single page when there is no `next` property", async () => { + fetchMock.mockResolvedValue({ + status: 200, + json: async () => ({ count: 1, next: null, previous: null, results: [{ ok: "yes" }] }), + ok: true, + }); + const paginator = new Paginator("https://test.com", {}, fetchMock); + const result = await paginator.fetchAll(); + + expect(fetchMock).toHaveBeenCalledOnce(); + expect(result).toStrictEqual({ + count: 1, + next: null, + previous: null, + results: [{ ok: "yes" }], + }); + }); + + it("Returns all pages when there is `next` property", async () => { + fetchMock + .mockResolvedValueOnce({ + status: 200, + json: async () => ({ + next: "https://test.com?page=2", + previous: null, + count: 2, + results: [{ ok: "1" }], + }), + ok: true, + }) + .mockResolvedValueOnce({ + status: 200, + json: async () => ({ + next: null, + previous: "https://test.com?page=1", + count: 2, + results: [{ ok: "2" }], + }), + ok: true, + }); + const paginator = new Paginator("https://test.com", {}, fetchMock); + const result = await paginator.fetchAll(); + + expect(fetchMock).toHaveBeenLastCalledWith("https://test.com?page=2", {}); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(result).toStrictEqual({ + count: 2, + next: null, + previous: null, + results: [{ ok: "1" }, { ok: "2" }], + }); + }); +}); diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.test.ts b/src/APL/saleor-cloud/saleor-cloud-apl.test.ts index 69bb0928..3b8c9dfe 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.test.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.test.ts @@ -249,6 +249,72 @@ describe("APL", () => { }, ]); }); + + it("Handles paginated response", async () => { + fetchMock + .mockResolvedValueOnce({ + status: 200, + ok: true, + json: async () => { + const mockData: GetAllAplResponseShape = { + count: 2, + next: "https://example.com?page=2", + previous: null, + results: [ + { + domain: "example.com", + jwks: "{}", + token: "token1", + saleor_api_url: "https://example.com/graphql/", + saleor_app_id: "x", + }, + ], + }; + return mockData; + }, + }) + .mockResolvedValueOnce({ + status: 200, + ok: true, + json: async () => { + const mockData: GetAllAplResponseShape = { + count: 2, + next: null, + previous: "https://example.com?page=1", + results: [ + { + domain: "example2.com", + jwks: "{}", + token: "token2", + saleor_api_url: "https://example2.com/graphql/", + saleor_app_id: "y", + }, + ], + }; + + return mockData; + }, + }); + + const apl = new SaleorCloudAPL(aplConfig); + + expect(await apl.getAll()).toStrictEqual([ + { + appId: "x", + domain: "example.com", + jwks: "{}", + saleorApiUrl: "https://example.com/graphql/", + token: "token1", + }, + { + appId: "y", + domain: "example2.com", + jwks: "{}", + saleorApiUrl: "https://example2.com/graphql/", + token: "token2", + }, + ]); + }); }); }); }); diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.ts b/src/APL/saleor-cloud/saleor-cloud-apl.ts index 4dd2e9d5..0c749713 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.ts @@ -30,6 +30,8 @@ type CloudAPLAuthDataShape = { export type GetAllAplResponseShape = { count: number; + next: string | null; + previous: string | null; results: CloudAPLAuthDataShape[]; }; From 76029cb8ef67de1487cfbf392cebfc2b150da316 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 16:05:56 +0100 Subject: [PATCH 09/10] Fix TS error --- src/APL/saleor-cloud/saleor-cloud-apl.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/APL/saleor-cloud/saleor-cloud-apl.test.ts b/src/APL/saleor-cloud/saleor-cloud-apl.test.ts index 3b8c9dfe..e151b16e 100644 --- a/src/APL/saleor-cloud/saleor-cloud-apl.test.ts +++ b/src/APL/saleor-cloud/saleor-cloud-apl.test.ts @@ -208,6 +208,8 @@ describe("APL", () => { json: async () => { const mockData: GetAllAplResponseShape = { count: 2, + next: null, + previous: null, results: [ { domain: "example.com", From f2844ec22dcf8644ad099468d0e06782a370e4e7 Mon Sep 17 00:00:00 2001 From: Jonatan Witoszek Date: Wed, 3 Jan 2024 16:08:36 +0100 Subject: [PATCH 10/10] Disable rules that cause errors in unchanged files --- .eslintrc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.eslintrc b/.eslintrc index f7c26f4e..a9957b26 100644 --- a/.eslintrc +++ b/.eslintrc @@ -60,7 +60,9 @@ "@typescript-eslint/no-misused-promises": ["error"], "@typescript-eslint/no-floating-promises": ["error"], "class-methods-use-this": "off", - "no-new": "off" + "no-new": "off", + "@typescript-eslint/no-redeclare": "off", + "@typescript-eslint/naming-convention": "off" }, "settings": { "import/parsers": {