Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pagination and option to configure limit for CloudAPL #333

Merged
merged 10 commits into from
Jan 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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