[RFC] Webhook factory abstraction for webhook handlers

[krzysztofzuraw]

Context

We should update Saleor webhooks, such as adding new fields, in apps without waiting for clients to update to the Saleor version that introduces the change.

Example: Saleor adds new field for OrderCreatedsubscription in release 3.15. We want to support it without waiting for our clients to update to 3.15.

Proposed solution

Introduce new abstraction above SaleorAsyncWebhook and SaleorSyncWebhook classes from @saleor/app-sdk : webhook factory handler.

Continuing with example from previous paragraph:

1. We create 2 subscriptions:
   1. One for version older than 3.15
   2. Another for version 3.15 and above with newField in subscription body
2. We get saleor schema version from headers (this will tell us in which Saleor version app in being installed)
3. Selecting what subscription will be used based on Saleor version will be hidden under OrderCreatedWebhookFactory:

type CombinedFragments = OrderCreatedFullFragment | OrderIdFragment;

export class OrderCreatedWebhookFactory {
  private webhook = new SaleorAsyncWebhook<CombinedFragments>({
    name: "Order Created in Saleor",
    webhookPath: "api/webhooks/order-created",
    event: "ORDER_CREATED",
    apl: saleorApp.apl,
    query: OrderCreatedDocument,
  });

  getWebhookManifest(apiBaseURL: string, saleorVersion: number) {
    if (saleorVersion >= 3.15) {
      this.webhook.query = OrderCreatedDocument;
      return this.webhook.getWebhookManifest(apiBaseURL);
    }

    this.webhook.query = OrderCreated314Document;
    return this.webhook.getWebhookManifest(apiBaseURL);
  }

  createHandler(handlerFn: NextWebhookApiHandler<CombinedFragments, {}>) {
    return this.webhook.createHandler(handlerFn);
  }
}

1. Factory then can be used in src/pages/api/webhooks/HANDLER:

export const orderCreatedWebhook = new OrderCreatedWebhookFactory();

export default orderCreatedWebhook.createHandler((req, res, ctx) => {
  const { payload } = ctx;

  console.log(`Order created: ${JSON.stringify(payload)}`);

  return res.status(200).end();
});

export const config = {
  api: {
    bodyParser: false,
  },
};

1. As well as in src/pages/api/manifest.ts:

export default createManifestHandler({
  async manifestFactory({ appBaseUrl, request }) {

    const saleorVersion = getSaleorVersion(
      request.headers ? request.headers["saleor-schema-version"] : ""
    );

    const manifest: AppManifest = {
      webhooks: [orderCreatedWebhookFactory.getWebhookManifest(apiBaseURL, saleorVersion)],
      // ...rest of manifest here
    };

    return manifest;
  },
});

Thanks to that, we can prevent logic from leaking into other system parts. When the client migrates to version 3.15, we can eliminate duplicate subscription handling in one location.

Potential problem - header saleor-schema-version is present from Saleor 3.15.

Repository with example code: https://github.com/krzysztofzuraw/webhook-factory-app

Other solutions

• Keeping multiple schemas
• Having branch per Saleor env (like in dashboard)

[witoszekdev]

This looks neat 🚀

I'm wondering how will the event type look like on createHandler? Will it be a union of OrderCreatedDocument and OrderCreated314Document? We should probably wrap the payload so that it's a discriminated union in order to make it easy to write a specific logic for certain Saleor versions, e.g.

// ctx.payload is of type OrderCreatedDocument | OrderCreated314Document
if (ctx.version >= 3.15) {
  // ctx.payload is of type OrderCreatedDocument
}
// ctx.payload is of type OrderCreated314Document

[lkostrowski]

Potential problem - header saleor-schema-version is present from Saleor 3.15.

I think its fine, we will add new features >=3.15, older ones are soon going to be EOL anyway

---

I agree with @witoszekdev that we should ensure we have union of types and result is type safe

---

I'd also mention that problem is not only subscriptions/webhooks. The same issue is classic query/mutation calls on the app runtime. And you make the POC more generic and also create a solution for graphql calls?

[krzysztofzuraw]

@witoszekdev good idea - I will add version to context in app-sdk then. We can reuse version to keep handlers typesafe and rely on version instead of field being present or not. After the change we can he something like this:

type CombinedFragments = OrderCreatedFullFragment | OrderIdFragment;

// type guard
export const isFullOrderWithVersion = (
  version: number,
  data: CombinedFragments
): data is OrderCreatedFullFragment => version >= 3.15;

// usage in handler
export default orderCreatedWebhook.createHandler((req, res, ctx) => {
  const { payload } = ctx;

  if (isFullOrder(ctx.version, payload)) {
    console.log(`Full order: ${payload.order?.userEmail}`);
  }

  console.log(`Id order only: ${JSON.stringify(payload.order?.id)}`);

  return res.status(200).end();
});

@lkostrowski sure - but to clarify do you want generic factory for all GraphQL requests or maybe additional example in example app and then usage in pattern description?

[lkostrowski]

@krzysztofzuraw I'd like the POC to include scope of the queries too. I think there will be an intersection. If we aim to look for a standard pattern, we can cover both use cases. Otherwise we will have a second RFC soon about queries.

I suggest to create abstraction on query, something like

interface QueryFactory<X, Y, Z, ...N> {
  resolveQuery(version: number): X | Y | Z
}

[krzysztofzuraw]

Based on discussion I've updated POC with new examples:

// saleor-order-api-factory.tsx

export class SaleorOrderAPIFactory {
  private client: Client;

  constructor(public apiUrl: string, public token: string) {
    this.client = createClient(apiUrl, async () => ({ token }));
  }

  private prepareQuery = (version: number | null, id: string, externalReference: string) => {
    if (version && version >= 3.15) {
      return this.client.query(OrderByIdDocument, { id });
    }
    return this.client.query(OrderByExternalRefDocument, { externalReference });
  };

  public async getOrder<T extends unknown>({
    version,
    id = "",
    externalReference = "",
  }: {
    version: number | null;
    id?: string;
    externalReference?: string;
  }) {
    return (await this.prepareQuery(version, id, externalReference).toPromise()) as T;
  }

  public updateMetadata = ({
    version,
    id,
    input,
  }: {
    version: number | null;
    id: string;
    input: { key: string; value: string }[];
  }) => {
    console.log("Version can be used to determine which mutation to use", version);
    return this.client.mutation(UpdateOrderMetadataDocument, { id, input }).toPromise();
  };

// order-created-handler.ts

export const isFullOrder = (
  version: number | null,
  data: CombinedFragments
): data is OrderCreatedFullFragment => (version ? version >= 3.15 : false);

export default orderCreatedWebhook.createHandler(async (req, res, ctx) => {
  const { payload, authData, schemaVersion } = ctx;

  const saleorAPI = new SaleorOrderAPIFactory(authData.saleorApiUrl, authData.token);

  if (isFullOrder(schemaVersion, payload)) {
    console.log(`Full order: ${payload.order?.userEmail}`);
    const orderByExternalRef = await saleorAPI.getOrder<OrderByExternalRefQuery>({
      version: schemaVersion,
      externalReference: payload.order?.externalReference ?? "",
    });
    console.log("BillingAddress", orderByExternalRef.order?.billingAddress);
  }

  console.log(`Id order only: ${JSON.stringify(payload.order?.id)}`);
  const orderById = await saleorAPI.getOrder<OrderByIdQuery>({
    version: schemaVersion,
    id: payload.order?.id ?? "",
  });
  console.log("ShippingAddress", orderById.order?.shippingAddress);

  saleorAPI.updateMetadata({
    version: schemaVersion,
    id: payload.order?.id ?? "",
    input: [{ key: "webhook", value: "order-created" }],
  });

  return res.status(200).end();
});

See the last commit on krzysztofzuraw/webhook-factory-app@a4d90d7

[lkostrowski]

export class SaleorOrderAPIFactory {

I don't think this is factory, it contains entire client inside. I think factory is a good pattern usage when you only wrap which query will be returned (or which client, etc)

---

Question: have you tried other abstractions, like wrapping query only? I'm not saying this one is bad, but its quite a boilerplate. I think it will not be common, and maybe its a good choice (we can enrich this with more methods, eg fallback missing value etc), but I'd like to compare possibilities

also "isFullOrder" would be good if embedded inside the class. Afair it doesn't work with class methods in TS, but I may be wrong. Anyway, we can store this assertion function near the class/factory

[krzysztofzuraw]

Yes - you are right with this one (name of the class) - I will update the example. I've tried with wrapper:

export class OrderQueryFactory {
  constructor(public client: Client, public version: number | null) {
    this.client = client;
    this.version = version;
  }

  public async fetch<T extends unknown>({
    id = "",
    externalReference = "",
  }: {
    id?: string;
    externalReference?: string;
  }) {
    if (this.version && this.version >= 3.15) {
      return (await this.client.query(OrderByIdDocument, { id }).toPromise()) as T;
    }
    return (await this.client
      .query(OrderByExternalRefDocument, { externalReference })
      .toPromise()) as T;
  }
}

const client = createClient();

const query = new OrderQueryFactory(client, schemaVersion);

const orderByExternalRef = await query.fetch<OrderByExternalRefQuery>({
  externalReference,
});

const orderById = await query.fetch<OrderByIdQuery>({
  id,
});

About isFullOrder - it turns out it can be static method on class 😄

[lkostrowski]

Have you tried constructing just a query (not client calling query)?

Like

client.query(
  OrderQueryFactory.resolveQuery(3.15)
).toPromise()

?

[krzysztofzuraw]

Checked and we can but we need to help TS compiler and use generic:

export class OrderQueryFactory {
  public static resolveQuery<T extends unknown>(version: number | null) {
    if (version && version >= 3.15) {
      return OrderByIdDocument as T;
    }
    return OrderByExternalRefDocument as T;
  }
}

const query = OrderQueryFactory.resolveQuery<typeof OrderByExternalRefDocument>(schemaVersion);

const result = await client.query(query, { externalReference }).toPromise();