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

Introduce new format for saleor schema version #416

Merged
merged 1 commit into from
Feb 28, 2025
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/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 @@
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 @@
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]),

Check warning on line 20 in src/headers.ts

View check run for this annotation

Codecov / codecov/patch

src/headers.ts#L20

Added line #L20 was not covered by tests
});

/**
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