Outbox prisma adapter #231

kamilwylegala · 2024-11-26T14:30:53Z

Transactional outbox pattern adapter for Prisma.

kibertoad · 2024-11-26T15:20:22Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+    const prismaModel: PrismaClient[typeof this.modelName] = this.prisma[this.modelName]
+
+    for (const entry of entries) {
+      await prismaModel.upsert({


upserts often have suboptimal performance with plenty of locking, can we somehow simplify this to be bulk inserts and bulk updates?

Definitely. I'm working on making tests green. Once I cover all cases, Bulk update/insert would do the job.

Actually upsert has some weird behavior, maybe let's switch to bulk inserts/updates right away.

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

CarlosGamero · 2025-01-23T08:51:15Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+type ModelDelegate = {
+  // biome-ignore lint/suspicious/noExplicitAny: <explanation>
+  create: (args: any) => Promise<any>
+  // biome-ignore lint/suspicious/noExplicitAny: <explanation>
+  findMany: (args: any) => Promise<any>
+  // biome-ignore lint/suspicious/noExplicitAny: <explanation>
+  createMany: (args: any) => Promise<any>
+  // biome-ignore lint/suspicious/noExplicitAny: <explanation>
+  updateMany: (args: any) => Promise<any>
+}


🟡 I am not really happy with using any here, as we know the method signature could we define input and output types to have type check and IDE help?

type OutboxEntryDelegate<Event extends CommonEventDefinition> = { create: (args: { data: OutboxEntry<Event> }) => Promise<OutboxEntry<Event>> findMany: (args: { where?: Partial<OutboxEntry<Event>> }) => Promise<OutboxEntry<Event>[]> .... }

Thanks for pointing this out. It's still a work in progress. I did any here to move quicker. I fully agree, it needs to be addressed. I will work on it.

Fixed, please take a look 😁

CarlosGamero · 2025-01-23T08:54:26Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+export class OutboxPrismaAdapter<
+  SupportedEvents extends CommonEventDefinition[],
+  ModelName extends keyof PrismaClient & string,
+> implements OutboxStorage<SupportedEvents>
+{
+  constructor(
+    private readonly prisma: PrismaClient,
+    private readonly modelName: ModelName,
+  ) {}


🟡 I remember having issue with passing PrismaClient directly as a parameter on prima-utils package, I don't remember exactly the reason, but it was because the object we are using is the autogenerate one and not the default coming from the Prisma package. I fixed it by doing something like:

export class OutboxPrismaAdapter< SupportedEvents extends CommonEventDefinition[], Prisma extends PrismaClient, ... > implements OutboxStorage<SupportedEvents> { constructor( private readonly prisma: Prisma, private readonly modelName: ModelName, ) {} ... }

Also, I am not sure if the definition of the Model name will work to have IDE autocompletion, wondering if we can make it a but more specific with something like:

type PrismaModelName<T extends PrismaClient> = { [K in keyof T]: T[K] extends { create: Function; findMany: Function } ? K : never }[keyof T] export class OutboxPrismaAdapter< SupportedEvents extends CommonEventDefinition[], Prisma extends PrismaClient, ModelName extends PrismaModelName<Prisma>, > implements OutboxStorage<SupportedEvents> { constructor( private readonly prisma: Prisma, private readonly modelName: ModelName, ) {}

CarlosGamero · 2025-01-23T08:55:42Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+  createEntry(
+    outboxEntry: OutboxEntry<SupportedEvents[number]>,
+  ): Promise<OutboxEntry<SupportedEvents[number]>> {
+    const prismaModel = this.prisma[this.modelName] as unknown as ModelDelegate


🟢 I see we are doing this on all methods, could we maybe create a private method to retrieve the model delegate so we can have the casting in a single place

CarlosGamero · 2025-01-23T08:56:51Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+  async flush(outboxAccumulator: OutboxAccumulator<SupportedEvents>): Promise<void> {
+    const entries = await outboxAccumulator.getEntries()
+    const failedEntries = await outboxAccumulator.getFailedEntries()
+    const prismaModel = this.prisma[this.modelName] as unknown as ModelDelegate
+
+    const existingEntries = await prismaModel.findMany({
+      where: {
+        id: {
+          in: [...entries.map((entry) => entry.id), ...failedEntries.map((entry) => entry.id)],
+        },
+      },
+    })
+
+    await this.prisma.$transaction(async (prisma) => {
+      const prismaModel = prisma[this.modelName] as ModelDelegate
+      await this.handleSuccesses(prismaModel, entries, existingEntries)
+      await this.handleFailures(prismaModel, failedEntries, existingEntries)
+    })


🟡 How many entries do we expect here? wondering if chunking could be beneficial

CarlosGamero · 2025-01-23T08:58:34Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+    prismaModel: ModelDelegate,
+    entries: OutboxEntry<SupportedEvents[number]>[],
+    existingEntries: OutboxEntry<SupportedEvents[number]>[],
+  ) {


🟢 This method is almost the same as handleSuccesses wondering if we can combine them by adding another param like isSuccess

kamilwylegala · 2025-01-23T08:58:45Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+    const prismaModel = this.prisma[this.modelName] as unknown as ModelDelegate
+
+    // @ts-ignore
+    const messageType = getMessageType(outboxEntry.event)


@CarlosGamero While you're looking at the PR, could you check this line? For some reason, TSLint complains about type mismatch here. I believe, the code is exactly the same as in the other packages, thus there are still compilation errors.

Of course! let me have a look :)

Discussed in Slack, For visibility:

Most likely we don't know to save the type in a separate filed and we can save it with the event itself, but just for visibility the issue was that getMessageType is expecting a message type and we are passing an event, if I am reading it fine on code, a message is an extension of event and that's why it was failing

CarlosGamero · 2025-01-23T09:00:19Z

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts

+      },
+    })
+
+    await this.prisma.$transaction(async (prisma) => {


🟡 What do you think about using prismaTransaction (from @lokalise/prisma-utils) here so we can benefit from teh retry mechanism implemented there

I think I discussed it with Igor, it's kibertoad namespace, so the preference is to not depend on any lokalise pacakges.

I don't remember this conversation, but if it happened, then I disagree with Igor from the past. We use lokalise-namespaced packages liberally in node-service-template, there is nothing wrong with it, and I don't think that @lokalise/prisma-utils is coupled to our internal specifics - as long as it supports key DBs (PostgreSQL and MySQL), it should be fine to use it.
Since this is a dedicated package for prisma adapter, having prisma-related dependencies in it should be fine

Sure, let's use prisma utils.

Back then, it was about using @lokalise/id-utils:

#204 (comment)

right, it was a slightly different case, we only needed a subset of the library, of which our id-utils is basically an opinionated wrapper

kamilwylegala added 5 commits November 26, 2024 15:32

AP-5046 WIP prisma adapter.

0353847

WIP prisma adapter.

14fbfba

Working test.

d55405e

WIP

e93bdad

Working test for saving outbox entries.

963014a

kamilwylegala force-pushed the AP-5046-outbox-prisma-adapter branch from 9ffbf8c to 963014a Compare November 26, 2024 14:33

failing test for updating.

7444fab

kibertoad reviewed Nov 26, 2024

View reviewed changes

kamilwylegala added 18 commits November 27, 2024 10:38

Bulk update + insert.

9c971fc

Failed entries handling.

5ad155f

fetching entries up to the retry count limit.

d9e3d41

Narrowed down types.

c61533d

lint fix

552e0b6

Use generated db client from test dir.

f143384

Build includes building test prisma client.

432875b

Fixed import.

ccf29a6

prisma main dependency.

3f39f92

prisma client dev dependency.

0b7d718

Build before lint.

c2108e3

Ignore db client in biome.

a7a9cb0

Peer prisma.

1b3c72c

inferred type

77b1d26

debugging ci

856ee17

keep prisma outside test folder in root

45d7310

Fixed import in spec.

00ca6d0

temp ts ignore.

33b954b

kamilwylegala commented Nov 28, 2024

View reviewed changes

packages/outbox-prisma-adapter/lib/outbox-prisma-adapter.ts Outdated Show resolved Hide resolved

kamilwylegala added 3 commits November 28, 2024 13:07

wait for db.

829c9ac

wait for db.

1f39dbf

wait for db.

8b818c1

kamilwylegala marked this pull request as ready for review November 28, 2024 12:20

kamilwylegala requested a review from kibertoad November 28, 2024 12:20

Redundant docker start.

ab58167

kibertoad requested review from CarlosGamero and Drodevbar January 21, 2025 22:51

CarlosGamero reviewed Jan 23, 2025

View reviewed changes

kamilwylegala commented Jan 23, 2025

View reviewed changes

CarlosGamero reviewed Jan 23, 2025

View reviewed changes

kamilwylegala added 2 commits January 23, 2025 14:34

Simplified outbox entry.

ed4ce02

Fixed tests to new version of outbox entry.

bc80d7d

kamilwylegala force-pushed the AP-5046-outbox-prisma-adapter branch from 965cc31 to bc80d7d Compare January 24, 2025 09:31

kamilwylegala added 3 commits January 24, 2025 10:50

Working on ModelDelegate

55bdd41

Fixed model delegate.

01975c0

Type fix.

0fc3e7f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Outbox prisma adapter #231

Outbox prisma adapter #231

kamilwylegala commented Nov 26, 2024

kibertoad Nov 26, 2024 •

edited

Loading

kamilwylegala Nov 26, 2024

CarlosGamero Jan 23, 2025 •

edited

Loading

kamilwylegala Jan 23, 2025

kamilwylegala Jan 24, 2025

CarlosGamero Jan 23, 2025 •

edited

Loading

CarlosGamero Jan 23, 2025

CarlosGamero Jan 23, 2025

CarlosGamero Jan 23, 2025 •

edited

Loading

CarlosGamero Jan 23, 2025

kamilwylegala Jan 23, 2025

CarlosGamero Jan 23, 2025

CarlosGamero Jan 23, 2025

CarlosGamero Jan 23, 2025

kamilwylegala Jan 23, 2025

kibertoad Jan 23, 2025

kamilwylegala Jan 23, 2025

kibertoad Jan 23, 2025

Outbox prisma adapter #231

Are you sure you want to change the base?

Outbox prisma adapter #231

Conversation

kamilwylegala commented Nov 26, 2024

kibertoad Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarlosGamero Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarlosGamero Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CarlosGamero Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibertoad Nov 26, 2024 •

edited

Loading

CarlosGamero Jan 23, 2025 •

edited

Loading

CarlosGamero Jan 23, 2025 •

edited

Loading

CarlosGamero Jan 23, 2025 •

edited

Loading