mirror of
https://github.com/outline/outline.git
synced 2026-06-13 11:25:03 +03:00
perf: Avoid empty webhook processor work via cached subscription lookup (#12593)
* Avoid empty webhook processor work via cached subscription lookup
WebhookProcessor ran for every event but most teams have no matching
webhook subscription, costing an empty processor job and a database query
per event.
Cache a team's enabled subscriptions ({ id, events }) in Redis, invalidated
by model lifecycle hooks, and add an optional BaseProcessor.shouldQueue hook
consulted by the global event queue so the webhook processor only enqueues a
job when a matching subscription exists.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feedback
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,94 @@
|
||||
import { buildTeam, buildWebhookSubscription } from "@server/test/factories";
|
||||
import WebhookSubscription from "./WebhookSubscription";
|
||||
|
||||
describe("WebhookSubscription", () => {
|
||||
describe("matchEvent", () => {
|
||||
it("matches everything for a wildcard subscription", () => {
|
||||
expect(WebhookSubscription.matchEvent(["*"], "users.signin")).toBe(true);
|
||||
});
|
||||
|
||||
it("matches an exact event name", () => {
|
||||
expect(
|
||||
WebhookSubscription.matchEvent(["users.signin"], "users.signin")
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("matches a namespace prefix", () => {
|
||||
expect(WebhookSubscription.matchEvent(["users"], "users.signin")).toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it("does not match unrelated events", () => {
|
||||
expect(
|
||||
WebhookSubscription.matchEvent(["documents"], "users.signin")
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("findEnabledByTeamId", () => {
|
||||
it("returns only enabled subscriptions for the team", async () => {
|
||||
const subscription = await buildWebhookSubscription({
|
||||
events: ["users"],
|
||||
});
|
||||
const disabled = await buildWebhookSubscription({
|
||||
teamId: subscription.teamId,
|
||||
events: ["documents"],
|
||||
});
|
||||
await disabled.disable();
|
||||
|
||||
const result = await WebhookSubscription.findEnabledByTeamId(
|
||||
subscription.teamId
|
||||
);
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].id).toEqual(subscription.id);
|
||||
expect(result[0].events).toEqual(["users"]);
|
||||
});
|
||||
|
||||
it("returns an empty array when the team has no subscriptions", async () => {
|
||||
const team = await buildTeam();
|
||||
|
||||
const result = await WebhookSubscription.findEnabledByTeamId(team.id);
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it("reflects changes after a subscription is disabled", async () => {
|
||||
const subscription = await buildWebhookSubscription({
|
||||
events: ["users"],
|
||||
});
|
||||
|
||||
// prime the cache
|
||||
const before = await WebhookSubscription.findEnabledByTeamId(
|
||||
subscription.teamId
|
||||
);
|
||||
expect(before).toHaveLength(1);
|
||||
|
||||
await subscription.disable();
|
||||
|
||||
const after = await WebhookSubscription.findEnabledByTeamId(
|
||||
subscription.teamId
|
||||
);
|
||||
expect(after).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("reflects changes after a subscription is destroyed", async () => {
|
||||
const subscription = await buildWebhookSubscription({
|
||||
events: ["users"],
|
||||
});
|
||||
|
||||
const before = await WebhookSubscription.findEnabledByTeamId(
|
||||
subscription.teamId
|
||||
);
|
||||
expect(before).toHaveLength(1);
|
||||
|
||||
await subscription.destroy();
|
||||
|
||||
const after = await WebhookSubscription.findEnabledByTeamId(
|
||||
subscription.teamId
|
||||
);
|
||||
expect(after).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -4,6 +4,7 @@ import type {
|
||||
InferAttributes,
|
||||
InferCreationAttributes,
|
||||
InstanceUpdateOptions,
|
||||
Transaction,
|
||||
} from "sequelize";
|
||||
import {
|
||||
Column,
|
||||
@@ -14,12 +15,19 @@ import {
|
||||
DataType,
|
||||
IsUrl,
|
||||
BeforeCreate,
|
||||
AfterCreate,
|
||||
AfterUpdate,
|
||||
AfterDestroy,
|
||||
AfterRestore,
|
||||
DefaultScope,
|
||||
AllowNull,
|
||||
} from "sequelize-typescript";
|
||||
import { Hour } from "@shared/utils/time";
|
||||
import { WebhookSubscriptionValidation } from "@shared/validations";
|
||||
import { ValidationError } from "@server/errors";
|
||||
import type { Event } from "@server/types";
|
||||
import { CacheHelper } from "@server/utils/CacheHelper";
|
||||
import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper";
|
||||
import Team from "./Team";
|
||||
import User from "./User";
|
||||
import ParanoidModel from "./base/ParanoidModel";
|
||||
@@ -47,6 +55,60 @@ class WebhookSubscription extends ParanoidModel<
|
||||
> {
|
||||
static eventNamespace = "webhookSubscriptions";
|
||||
|
||||
/**
|
||||
* Returns the enabled webhook subscriptions for a team, caching the
|
||||
* lightweight { id, events } projection in Redis to avoid a database query on
|
||||
* every event. The cache is invalidated by model lifecycle hooks whenever a
|
||||
* team's subscriptions change.
|
||||
*
|
||||
* @param teamId The team to load subscriptions for.
|
||||
* @returns the enabled subscriptions' ids and subscribed event names.
|
||||
*/
|
||||
public static async findEnabledByTeamId(
|
||||
teamId: string
|
||||
): Promise<Array<{ id: string; events: string[] }>> {
|
||||
return (
|
||||
(await CacheHelper.getDataOrSet<Array<{ id: string; events: string[] }>>(
|
||||
RedisPrefixHelper.getWebhookSubscriptionsKey(teamId),
|
||||
async () => {
|
||||
const subscriptions = await this.unscoped().findAll({
|
||||
attributes: ["id", "events"],
|
||||
where: { enabled: true, teamId },
|
||||
raw: true,
|
||||
});
|
||||
return subscriptions.map((subscription) => ({
|
||||
id: subscription.id,
|
||||
events: subscription.events,
|
||||
}));
|
||||
},
|
||||
Hour.seconds
|
||||
)) ?? []
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines whether a subscription configured for the given event names
|
||||
* should receive an event with the given name. Pure so it can run against the
|
||||
* cached projection as well as model instances.
|
||||
*
|
||||
* @param events The event names a subscription is configured for.
|
||||
* @param eventName The name of the event being processed.
|
||||
* @returns true if the event matches the subscription configuration.
|
||||
*/
|
||||
public static matchEvent(events: string[], eventName: string): boolean {
|
||||
if (events.length === 1 && events[0] === "*") {
|
||||
return true;
|
||||
}
|
||||
|
||||
for (const e of events) {
|
||||
if (e === eventName || eventName.startsWith(e + ".")) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
@NotEmpty
|
||||
@Length({
|
||||
max: WebhookSubscriptionValidation.maxNameLength,
|
||||
@@ -105,6 +167,31 @@ class WebhookSubscription extends ParanoidModel<
|
||||
}
|
||||
}
|
||||
|
||||
@AfterCreate
|
||||
@AfterUpdate
|
||||
@AfterDestroy
|
||||
@AfterRestore
|
||||
static async invalidateCache(
|
||||
model: WebhookSubscription,
|
||||
options: { transaction?: Transaction | null }
|
||||
) {
|
||||
const invalidate = () =>
|
||||
CacheHelper.removeData(
|
||||
RedisPrefixHelper.getWebhookSubscriptionsKey(model.teamId)
|
||||
);
|
||||
|
||||
// Defer invalidation until after the transaction commits so that a rollback
|
||||
// does not leave the cache out of sync, and so a stale pre-commit read is
|
||||
// not re-cached by a concurrent reader. Walk to the parent transaction when
|
||||
// nested so the callback isn't lost when a savepoint releases.
|
||||
if (options.transaction) {
|
||||
const transaction = options.transaction.parent || options.transaction;
|
||||
transaction.afterCommit(invalidate);
|
||||
} else {
|
||||
await invalidate();
|
||||
}
|
||||
}
|
||||
|
||||
// instance methods
|
||||
|
||||
/**
|
||||
@@ -130,22 +217,11 @@ class WebhookSubscription extends ParanoidModel<
|
||||
* Determines if an event should be processed for this webhook subscription
|
||||
* based on the event configuration.
|
||||
*
|
||||
* @param event Event to ceck
|
||||
* @param event Event to check
|
||||
* @returns true if event is valid
|
||||
*/
|
||||
public validForEvent = (event: Event): boolean => {
|
||||
if (this.events.length === 1 && this.events[0] === "*") {
|
||||
return true;
|
||||
}
|
||||
|
||||
for (const e of this.events) {
|
||||
if (e === event.name || event.name.startsWith(e + ".")) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
};
|
||||
public validForEvent = (event: Event): boolean =>
|
||||
WebhookSubscription.matchEvent(this.events, event.name);
|
||||
|
||||
/**
|
||||
* Calculates the signature for a webhook payload if the webhook subscription
|
||||
|
||||
@@ -1,8 +1,31 @@
|
||||
import type { Event } from "@server/types";
|
||||
|
||||
export default abstract class BaseProcessor {
|
||||
/**
|
||||
* The event names this processor handles. The global event queue only creates
|
||||
* a job for the processor when an event's name is listed here, or when it
|
||||
* contains the `"*"` wildcard to match every event.
|
||||
*/
|
||||
static applicableEvents: (Event["name"] | "*")[] = [];
|
||||
|
||||
/**
|
||||
* Optional hook run in the global event queue before a job is created for this
|
||||
* processor. Implement it to cheaply opt out of events the processor will not
|
||||
* act on and avoid the cost of an empty job. When omitted, every applicable
|
||||
* event is queued.
|
||||
*
|
||||
* @param event The event about to be queued.
|
||||
* @returns true if a job should be queued for this processor.
|
||||
*/
|
||||
static shouldQueue?: (event: Event) => Promise<boolean>;
|
||||
|
||||
/**
|
||||
* Handle an applicable event. Called once per queued job, with retries on
|
||||
* failure.
|
||||
*
|
||||
* @param event The event to process.
|
||||
* @returns A promise that resolves once the event has been processed.
|
||||
*/
|
||||
public abstract perform(event: Event): Promise<void>;
|
||||
|
||||
/**
|
||||
|
||||
@@ -57,7 +57,14 @@ export default async function init() {
|
||||
ProcessorClass.applicableEvents.includes(event.name) ||
|
||||
ProcessorClass.applicableEvents.includes("*")
|
||||
) {
|
||||
await processorEventQueue().add({ event, name });
|
||||
// A processor may optionally opt out of an event before a job is
|
||||
// created, avoiding the cost of an empty job.
|
||||
if (
|
||||
!ProcessorClass.shouldQueue ||
|
||||
(await ProcessorClass.shouldQueue(event))
|
||||
) {
|
||||
await processorEventQueue().add({ event, name });
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
Logger.error(
|
||||
|
||||
@@ -43,6 +43,16 @@ export class RedisPrefixHelper {
|
||||
return `uc:${userId}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets key for caching a team's enabled webhook subscriptions.
|
||||
*
|
||||
* @param teamId The team ID to generate a key for.
|
||||
* @returns the cache key string.
|
||||
*/
|
||||
public static getWebhookSubscriptionsKey(teamId: string) {
|
||||
return `whs:${teamId}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets key for caching the count of a relationship managed by the
|
||||
* `CounterCache` decorator.
|
||||
|
||||
Reference in New Issue
Block a user