Skip to content

Commit

Permalink
Merge pull request #333 from saleor/fix-cloud-apl-limit
Browse files Browse the repository at this point in the history
Add pagination and option to configure limit for CloudAPL
  • Loading branch information
witoszekdev authored Jan 3, 2024
2 parents d2f19da + f2844ec commit 9845d88
Show file tree
Hide file tree
Showing 8 changed files with 230 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changeset/violet-donuts-begin.md
Original file line number Diff line number Diff line change
@@ -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
5 changes: 4 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
},
"extends": [
"airbnb",
"airbnb-typescript",
"plugin:@typescript-eslint/recommended",
"prettier" // prettier *has* to be the last one, to avoid conflicting rules
],
Expand Down Expand Up @@ -59,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": {
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
18 changes: 18 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

64 changes: 64 additions & 0 deletions src/APL/saleor-cloud/paginator.test.ts
Original file line number Diff line number Diff line change
@@ -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" }],
});
});
});
54 changes: 54 additions & 0 deletions src/APL/saleor-cloud/paginator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { createAPLDebug } from "../apl-debug";

const debug = createAPLDebug("Paginator");

interface PaginationResponse<ResultType> {
next: string | null;
previous: string | null;
results: ResultType[];
}

export class Paginator<ResultType> {
constructor(
private readonly url: string,
private readonly fetchOptions: RequestInit,
private readonly 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<ResultType>;

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<Array<PaginationResponse<ResultType>>> {
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<ResultType>;

if (json.next) {
return [json, ...(await this.fetchNext(json.next))];
}
return [json];
}
}
68 changes: 68 additions & 0 deletions src/APL/saleor-cloud/saleor-cloud-apl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ describe("APL", () => {
json: async () => {
const mockData: GetAllAplResponseShape = {
count: 2,
next: null,
previous: null,
results: [
{
domain: "example.com",
Expand Down Expand Up @@ -249,6 +251,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",
},
]);
});
});
});
});
21 changes: 14 additions & 7 deletions src/APL/saleor-cloud/saleor-cloud-apl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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 { Paginator } from "./paginator";
import { CloudAplError, SaleorCloudAplError } from "./saleor-cloud-apl-errors";

const debug = createAPLDebug("SaleorCloudAPL");
Expand All @@ -16,6 +17,7 @@ export type SaleorCloudAPLConfig = {
experimental?: {
cacheManager?: Map<string, AuthData>;
};
pageLimit?: number;
};

type CloudAPLAuthDataShape = {
Expand All @@ -28,6 +30,8 @@ type CloudAPLAuthDataShape = {

export type GetAllAplResponseShape = {
count: number;
next: string | null;
previous: string | null;
results: CloudAPLAuthDataShape[];
};

Expand Down Expand Up @@ -89,6 +93,8 @@ export class SaleorCloudAPL implements APL {

private cacheManager?: Map<string, AuthData>;

private readonly pageLimit: number;

constructor(config: SaleorCloudAPLConfig) {
this.resourceUrl = config.resourceUrl;
this.headers = {
Expand All @@ -97,13 +103,18 @@ export class SaleorCloudAPL implements APL {

this.tracer = getOtelTracer();
this.cacheManager = config?.experimental?.cacheManager;
this.pageLimit = config.pageLimit ?? 1000;
}

private getUrlForDomain(saleorApiUrl: string) {
// API URL has to be base64url encoded
return `${this.resourceUrl}/${Buffer.from(saleorApiUrl).toString("base64url")}`;
}

private getUrlWithLimit() {
return `${this.resourceUrl}?limit=${this.pageLimit}`;
}

private setToCacheIfExists(saleorApiUrl: string, authData: AuthData) {
if (!this.cacheManager) {
return;
Expand Down Expand Up @@ -338,16 +349,12 @@ export class SaleorCloudAPL implements APL {
debug("Get all data from SaleorCloud");

try {
const response = await fetch(this.resourceUrl, {
const paginator = new Paginator<CloudAPLAuthDataShape>(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);

Expand Down

0 comments on commit 9845d88

Please sign in to comment.