Skip to content

Commit

Permalink
new schema version format (#416)
Browse files Browse the repository at this point in the history
  • Loading branch information
lkostrowski authored Feb 28, 2025
1 parent 5b0f7a8 commit 3dfb91c
Show file tree
Hide file tree
Showing 20 changed files with 59 additions and 49 deletions.
5 changes: 5 additions & 0 deletions .changeset/fluffy-ties-stop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@saleor/app-sdk": major
---

`requiredEnvVars` param was removed trom SaleorApp. Field was not used internally. Apps should validate it's envs.
5 changes: 5 additions & 0 deletions .changeset/smooth-wolves-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@saleor/app-sdk": major
---

Saleor version that was previously represented as a floating number (eg Saleor 3.20 was represented as 3.2) is now a `SaleorSchemaVersion` which is a tuple `major: number, minor: number`. This format is now passed to Manifest handler and webhooks handler
11 changes: 1 addition & 10 deletions src/APL/vercel-kv/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
import { VercelKvApl } from "./vercel-kv-apl";

/**
* @deprecated - use VercelKvApl
*/
const _experimental_VercelKvApl = VercelKvApl;

export {
// TODO Remove in next minor
_experimental_VercelKvApl,
VercelKvApl,
};
export { VercelKvApl };
2 changes: 1 addition & 1 deletion src/handlers/actions/manifest-action-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe("ManifestActionHandler", () => {
expect(manifestFactory).toHaveBeenCalledWith({
appBaseUrl: "http://example.com",
request: {},
schemaVersion: "3.20",
schemaVersion: [3, 20],
});
});

Expand Down
8 changes: 5 additions & 3 deletions src/handlers/actions/manifest-action-handler.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { createDebug } from "@/debug";
import { AppManifest } from "@/types";
import { AppManifest, SaleorSchemaVersion } from "@/types";
import { parseSchemaVersion } from "@/util";

import {
ActionHandlerInterface,
Expand All @@ -19,7 +20,7 @@ export type CreateManifestHandlerOptions<T> = {
* so manifest can be generated according to the version. But it may
* be also requested from plain GET from the browser, so it may not be available
*/
schemaVersion?: string;
schemaVersion?: SaleorSchemaVersion;
}): AppManifest | Promise<AppManifest>;
};

Expand All @@ -30,6 +31,7 @@ export class ManifestActionHandler<I> implements ActionHandlerInterface {

async handleAction(options: CreateManifestHandlerOptions<I>): Promise<ActionHandlerResult> {
const { schemaVersion } = this.requestProcessor.getSaleorHeaders();
const parsedSchemaVersion = parseSchemaVersion(schemaVersion) ?? undefined;
const baseURL = this.adapter.getBaseUrl();

debug("Received request with schema version \"%s\" and base URL \"%s\"", schemaVersion, baseURL);
Expand All @@ -44,7 +46,7 @@ export class ManifestActionHandler<I> implements ActionHandlerInterface {
const manifest = await options.manifestFactory({
appBaseUrl: baseURL,
request: this.adapter.request,
schemaVersion,
schemaVersion: parsedSchemaVersion,
});

debug("Executed manifest file");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe("AWS Lambda createManifestHandler", () => {
expect.objectContaining({
appBaseUrl: expectedBaseUrl,
request: event,
schemaVersion: "3.20",
schemaVersion: [3, 20],
}),
);
expect(response.statusCode).toBe(200);
Expand Down Expand Up @@ -98,7 +98,7 @@ describe("AWS Lambda createManifestHandler", () => {
expect.objectContaining({
appBaseUrl: expectedBaseUrl,
request: event,
schemaVersion: "3.20",
schemaVersion: [3, 20],
}),
);
expect(response.statusCode).toBe(200);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe("AWS Lambda SaleorSyncWebhook", () => {
baseUrl: "example.com",
event: "CHECKOUT_CALCULATE_TAXES",
payload: { data: "test_payload" },
schemaVersion: 3.19,
schemaVersion: [3, 19],
authData: {
token: webhookConfig.apl.mockToken,
jwks: webhookConfig.apl.mockJwks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("Fetch API createManifestHandler", () => {
expect(mockManifestFactory).toHaveBeenCalledWith({
appBaseUrl: baseUrl,
request,
schemaVersion: "3.20",
schemaVersion: [3, 20],
});
expect(response.status).toBe(200);
await expect(response.json()).resolves.toStrictEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("Web API SaleorAsyncWebhook", () => {
baseUrl: "example.com",
event: "product_updated",
payload: { data: "test_payload" },
schemaVersion: 3.2,
schemaVersion: [3, 20],
authData: {
saleorApiUrl: mockAPL.workingSaleorApiUrl,
token: mockAPL.mockToken,
Expand Down Expand Up @@ -62,7 +62,7 @@ describe("Web API SaleorAsyncWebhook", () => {
authData: expect.objectContaining({
saleorApiUrl: mockAPL.workingSaleorApiUrl,
}),
})
}),
);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("Web API SaleorSyncWebhook", () => {
baseUrl: "example.com",
event: "checkout_calculate_taxes",
payload: { data: "test_payload" },
schemaVersion: 3.19,
schemaVersion: [3, 19],
authData: {
token: webhookConfiguration.apl.mockToken,
jwks: webhookConfiguration.apl.mockJwks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe("Next.js createManifestHandler", () => {
expect(mockManifestFactory).toHaveBeenCalledWith({
appBaseUrl: baseUrl,
request: req,
schemaVersion: "3.20",
schemaVersion: [3, 20],
});

expect(res.statusCode).toBe(200);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe("Next.js SaleorAsyncWebhook", () => {
expect(saleorAsyncWebhook.getWebhookManifest(baseUrl)).toEqual(
expect.objectContaining({
targetUrl: `${baseUrl}/${webhookPath}`,
})
}),
);
});

Expand All @@ -56,7 +56,7 @@ describe("Next.js SaleorAsyncWebhook", () => {
baseUrl: "example.com",
event: "product_updated",
payload: { data: "test_payload" },
schemaVersion: 3.2,
schemaVersion: [3, 20],
authData: {
saleorApiUrl: mockAPL.workingSaleorApiUrl,
token: mockAPL.mockToken,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe("Next.js SaleorSyncWebhook", () => {
baseUrl: "example.com",
event: "checkout_calculate_taxes",
payload: { data: "test_payload" },
schemaVersion: 3.19,
schemaVersion: [3, 19],
authData: {
token: validSyncWebhookConfiguration.apl.mockToken,
jwks: validSyncWebhookConfiguration.apl.mockJwks,
Expand Down
18 changes: 11 additions & 7 deletions src/handlers/shared/saleor-webhook-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { APL } from "@/APL";
import { createDebug } from "@/debug";
import { fetchRemoteJwks } from "@/fetch-remote-jwks";
import { getOtelTracer } from "@/open-telemetry";
import { SaleorSchemaVersion } from "@/types";
import { parseSchemaVersion } from "@/util";
import { verifySignatureWithJwks } from "@/verify-signature";

Expand Down Expand Up @@ -94,7 +95,7 @@ export class SaleorWebhookValidator {

throw new WebhookError(
`Wrong incoming request event: ${event}. Expected: ${expected}`,
"WRONG_EVENT"
"WRONG_EVENT",
);
}

Expand All @@ -121,7 +122,10 @@ export class SaleorWebhookValidator {
throw new WebhookError("Request body can't be parsed", "CANT_BE_PARSED");
}

let parsedSchemaVersion: number | null = null;
/**
* Can be undefined - subscription must contain "version", otherwise nothing to parse
*/
let parsedSchemaVersion: SaleorSchemaVersion | null = null;

try {
parsedSchemaVersion = parseSchemaVersion(parsedBody.version);
Expand All @@ -139,7 +143,7 @@ export class SaleorWebhookValidator {

throw new WebhookError(
`Can't find auth data for ${saleorApiUrl}. Please register the application`,
"NOT_REGISTERED"
"NOT_REGISTERED",
);
}

Expand All @@ -162,15 +166,15 @@ export class SaleorWebhookValidator {

throw new WebhookError(
"Fetching remote JWKS failed",
"SIGNATURE_VERIFICATION_FAILED"
"SIGNATURE_VERIFICATION_FAILED",
);
});

this.debug("Fetched refreshed JWKS");

try {
this.debug(
"Second attempt to validate the signature JWKS, using fresh tokens from the API"
"Second attempt to validate the signature JWKS, using fresh tokens from the API",
);

await verifySignatureWithJwks(newJwks, signature, rawBody);
Expand All @@ -183,7 +187,7 @@ export class SaleorWebhookValidator {

throw new WebhookError(
"Request signature check failed",
"SIGNATURE_VERIFICATION_FAILED"
"SIGNATURE_VERIFICATION_FAILED",
);
}
}
Expand Down Expand Up @@ -211,7 +215,7 @@ export class SaleorWebhookValidator {
} finally {
span.end();
}
}
},
);
}
}
10 changes: 7 additions & 3 deletions src/handlers/shared/saleor-webhook.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { SaleorSchemaVersion } from "@/types";

import { AuthData } from "../../APL";

export const WebhookErrorCodeMap: Record<SaleorWebhookError, number> = {
Expand Down Expand Up @@ -50,9 +52,11 @@ export type WebhookContext<TPayload> = {
event: string;
payload: TPayload;
authData: AuthData;
// TODO: Make this required
/** Added in Saleor 3.15 */
schemaVersion: number | null;
/**
* Schema version is passed in subscription payload. Webhook must request it, otherwise it will be null.
* If subscription contains version, it will be parsed to SaleorSchemaVersion
*/
schemaVersion: SaleorSchemaVersion | null;
};

export type FormatWebhookErrorResult = {
Expand Down
5 changes: 1 addition & 4 deletions src/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@ import {
const toStringOrUndefined = (value: string | string[] | undefined) =>
value ? value.toString() : undefined;

const toFloatOrNull = (value: string | string[] | undefined) =>
value ? parseFloat(value.toString()) : null;

/**
* Extracts Saleor-specific headers from the response.
*/
Expand All @@ -20,7 +17,7 @@ export const getSaleorHeaders = (headers: { [name: string]: string | string[] |
signature: toStringOrUndefined(headers[SALEOR_SIGNATURE_HEADER]),
event: toStringOrUndefined(headers[SALEOR_EVENT_HEADER]),
saleorApiUrl: toStringOrUndefined(headers[SALEOR_API_URL_HEADER]),
schemaVersion: toFloatOrNull(headers[SALEOR_SCHEMA_VERSION]),
schemaVersion: toStringOrUndefined(headers[SALEOR_SCHEMA_VERSION]),
});

/**
Expand Down
4 changes: 0 additions & 4 deletions src/saleor-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,12 @@ export interface HasAPL {

export interface SaleorAppParams {
apl: APL;
requiredEnvVars?: string[];
}

export class SaleorApp implements HasAPL {
readonly apl: APL;

readonly requiredEnvVars: string[];

constructor(options: SaleorAppParams) {
this.apl = options.apl;
this.requiredEnvVars = options.requiredEnvVars ?? [];
}
}
2 changes: 2 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,5 @@ export interface AppManifest {
};
};
}

export type SaleorSchemaVersion = [major: number, minor: number];
8 changes: 4 additions & 4 deletions src/util/schema-version.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ describe("parseSchemaVersion", () => {
},
{
rawVersion: "3.19",
parsedVersion: 3.19,
parsedVersion: [3, 19],
},
{
rawVersion: "3.19.1",
parsedVersion: 3.19,
parsedVersion: [3, 19],
},
{
rawVersion: "malformed",
Expand All @@ -39,7 +39,7 @@ describe("parseSchemaVersion", () => {
])(
"Parses version string from: $rawVersion to: $parsedVersion",
({ rawVersion, parsedVersion }) => {
expect(parseSchemaVersion(rawVersion)).toBe(parsedVersion);
}
expect(parseSchemaVersion(rawVersion)).toEqual(parsedVersion);
},
);
});
8 changes: 6 additions & 2 deletions src/util/schema-version.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
export const parseSchemaVersion = (rawVersion: string | undefined | null): number | null => {
import { SaleorSchemaVersion } from "@/types";

export const parseSchemaVersion = (
rawVersion: string | undefined | null,
): SaleorSchemaVersion | null => {
if (!rawVersion) {
return null;
}
Expand All @@ -8,7 +12,7 @@ export const parseSchemaVersion = (rawVersion: string | undefined | null): numbe
const minor = parseInt(minorString, 10);

if (major && minor) {
return parseFloat(`${major}.${minor}`);
return [major, minor];
}

return null;
Expand Down

0 comments on commit 3dfb91c

Please sign in to comment.