From 6625c9bc48e3cdbb39f87e53c1865e35f19381df Mon Sep 17 00:00:00 2001 From: dswbx Date: Wed, 15 Jan 2025 17:21:28 +0100 Subject: [PATCH 1/5] Refactor event system to support returnable events Added support for validating and managing return values in events. Implemented `validate` and `clone` methods in the event base class for event mutation and return handling. Additionally, enhanced error handling, introduced "once" listeners, and improved async execution management in the `EventManager`. --- app/__test__/core/EventManager.spec.ts | 135 +++++++++++++++++++--- app/src/core/events/Event.ts | 24 +++- app/src/core/events/EventListener.ts | 7 +- app/src/core/events/EventManager.ts | 92 ++++++++++++--- app/src/core/events/index.ts | 4 +- tmp/event_manager_returning_test.patch | 150 ------------------------- 6 files changed, 227 insertions(+), 185 deletions(-) delete mode 100644 tmp/event_manager_returning_test.patch diff --git a/app/__test__/core/EventManager.spec.ts b/app/__test__/core/EventManager.spec.ts index 3327449..4f11d19 100644 --- a/app/__test__/core/EventManager.spec.ts +++ b/app/__test__/core/EventManager.spec.ts @@ -1,8 +1,18 @@ -import { describe, expect, test } from "bun:test"; -import { Event, EventManager, NoParamEvent } from "../../src/core/events"; +import { afterAll, beforeAll, describe, expect, mock, test } from "bun:test"; +import { + Event, + EventManager, + InvalidEventReturn, + type ListenerHandler, + NoParamEvent +} from "../../src/core/events"; +import { disableConsoleLog, enableConsoleLog } from "../helper"; + +beforeAll(disableConsoleLog); +afterAll(enableConsoleLog); class SpecialEvent extends Event<{ foo: string }> { - static slug = "special-event"; + static override slug = "special-event"; isBar() { return this.params.foo === "bar"; @@ -10,37 +20,136 @@ class SpecialEvent extends Event<{ foo: string }> { } class InformationalEvent extends NoParamEvent { - static slug = "informational-event"; + static override slug = "informational-event"; +} + +class ReturnEvent extends Event<{ foo: string }, string> { + static override slug = "return-event"; + + override validate(value: string) { + if (typeof value !== "string") { + throw new InvalidEventReturn("string", typeof value); + } + + return this.clone({ + foo: [this.params.foo, value].join("-") + }); + } } describe("EventManager", async () => { - test("test", async () => { + test("executes", async () => { + const call = mock(() => null); + const delayed = mock(() => null); + const emgr = new EventManager(); emgr.registerEvents([SpecialEvent, InformationalEvent]); + expect(emgr.eventExists("special-event")).toBe(true); + expect(emgr.eventExists("informational-event")).toBe(true); + expect(emgr.eventExists("unknown-event")).toBe(false); + emgr.onEvent( SpecialEvent, async (event, name) => { - console.log("Event: ", name, event.params.foo, event.isBar()); - console.log("wait..."); - - await new Promise((resolve) => setTimeout(resolve, 100)); - console.log("done waiting"); + expect(name).toBe("special-event"); + expect(event.isBar()).toBe(true); + call(); + await new Promise((resolve) => setTimeout(resolve, 50)); + delayed(); }, "sync" ); emgr.onEvent(InformationalEvent, async (event, name) => { - console.log("Event: ", name, event.params); + call(); + expect(name).toBe("informational-event"); }); await emgr.emit(new SpecialEvent({ foo: "bar" })); - console.log("done"); + await emgr.emit(new InformationalEvent()); // expect construct signatures to not cause ts errors new SpecialEvent({ foo: "bar" }); new InformationalEvent(); - expect(true).toBe(true); + expect(call).toHaveBeenCalledTimes(2); + expect(delayed).toHaveBeenCalled(); + }); + + test("custom async executor", async () => { + const call = mock(() => null); + const asyncExecutor = (p: Promise[]) => { + call(); + return Promise.all(p); + }; + const emgr = new EventManager( + { InformationalEvent }, + { + asyncExecutor + } + ); + + emgr.onEvent(InformationalEvent, async () => {}); + await emgr.emit(new InformationalEvent()); + expect(call).toHaveBeenCalled(); + }); + + test("piping", async () => { + const onInvalidReturn = mock(() => null); + const asyncEventCallback = mock(() => null); + const emgr = new EventManager( + { ReturnEvent, InformationalEvent }, + { + onInvalidReturn + } + ); + + // @ts-expect-error InformationalEvent has no return value + emgr.onEvent(InformationalEvent, async () => { + asyncEventCallback(); + return 1; + }); + + emgr.onEvent(ReturnEvent, async () => "1", "sync"); + emgr.onEvent(ReturnEvent, async () => "0", "sync"); + + // @ts-expect-error must be string + emgr.onEvent(ReturnEvent, async () => 0, "sync"); + + // return is not required + emgr.onEvent(ReturnEvent, async () => {}, "sync"); + + // was "async", will not return + const e1 = await emgr.emit(new InformationalEvent()); + expect(e1.returned).toBe(false); + + const e2 = await emgr.emit(new ReturnEvent({ foo: "bar" })); + expect(e2.returned).toBe(true); + expect(e2.params.foo).toBe("bar-1-0"); + expect(onInvalidReturn).toHaveBeenCalled(); + expect(asyncEventCallback).toHaveBeenCalled(); + }); + + test("once", async () => { + const call = mock(() => null); + const emgr = new EventManager({ InformationalEvent }); + + emgr.onEventOnce( + InformationalEvent, + async (event, slug) => { + expect(event).toBeInstanceOf(InformationalEvent); + expect(slug).toBe("informational-event"); + call(); + }, + "sync" + ); + + expect(emgr.getListeners().length).toBe(1); + await emgr.emit(new InformationalEvent()); + expect(emgr.getListeners().length).toBe(0); + await emgr.emit(new InformationalEvent()); + expect(emgr.getListeners().length).toBe(0); + expect(call).toHaveBeenCalledTimes(1); }); }); diff --git a/app/src/core/events/Event.ts b/app/src/core/events/Event.ts index 247c7a5..734c7c2 100644 --- a/app/src/core/events/Event.ts +++ b/app/src/core/events/Event.ts @@ -1,17 +1,31 @@ -export abstract class Event { +export abstract class Event { + _returning!: Returning; + /** * Unique event slug * Must be static, because registering events is done by class */ static slug: string = "untitled-event"; params: Params; + returned: boolean = false; + + validate(value: Returning): Event | void {} + + protected clone = Event>( + this: This, + params: Params + ): This { + const cloned = new (this.constructor as any)(params); + cloned.returned = true; + return cloned as This; + } constructor(params: Params) { this.params = params; } } -// @todo: current workaround: potentially there is none and that's the way +// @todo: current workaround: potentially there is "none" and that's the way export class NoParamEvent extends Event { static override slug: string = "noparam-event"; @@ -19,3 +33,9 @@ export class NoParamEvent extends Event { super(null); } } + +export class InvalidEventReturn extends Error { + constructor(expected: string, given: string) { + super(`Expected "${expected}", got "${given}"`); + } +} diff --git a/app/src/core/events/EventListener.ts b/app/src/core/events/EventListener.ts index 951fce8..fc677ed 100644 --- a/app/src/core/events/EventListener.ts +++ b/app/src/core/events/EventListener.ts @@ -4,15 +4,16 @@ import type { EventClass } from "./EventManager"; export const ListenerModes = ["sync", "async"] as const; export type ListenerMode = (typeof ListenerModes)[number]; -export type ListenerHandler = ( +export type ListenerHandler> = ( event: E, - slug: string, -) => Promise | void; + slug: string +) => E extends Event ? R | Promise : never; export class EventListener { mode: ListenerMode = "async"; event: EventClass; handler: ListenerHandler; + once: boolean = false; constructor(event: EventClass, handler: ListenerHandler, mode: ListenerMode = "async") { this.event = event; diff --git a/app/src/core/events/EventManager.ts b/app/src/core/events/EventManager.ts index 9233666..6e48224 100644 --- a/app/src/core/events/EventManager.ts +++ b/app/src/core/events/EventManager.ts @@ -1,4 +1,4 @@ -import type { Event } from "./Event"; +import { type Event, InvalidEventReturn } from "./Event"; import { EventListener, type ListenerHandler, type ListenerMode } from "./EventListener"; export interface EmitsEvents { @@ -6,7 +6,7 @@ export interface EmitsEvents { } export type EventClass = { - new (params: any): Event; + new (params: any): Event; slug: string; }; @@ -17,16 +17,20 @@ export class EventManager< protected listeners: EventListener[] = []; enabled: boolean = true; - constructor(events?: RegisteredEvents, listeners?: EventListener[]) { + constructor( + events?: RegisteredEvents, + private options?: { + listeners?: EventListener[]; + onError?: (event: Event, e: unknown) => void; + onInvalidReturn?: (event: Event, e: InvalidEventReturn) => void; + asyncExecutor?: typeof Promise.all; + } + ) { if (events) { this.registerEvents(events); } - if (listeners) { - for (const listener of listeners) { - this.addListener(listener); - } - } + options?.listeners?.forEach((l) => this.addListener(l)); } enable() { @@ -128,6 +132,18 @@ export class EventManager< this.addListener(listener as any); } + onEventOnce>( + event: ActualEvent, + handler: ListenerHandler, + mode: ListenerMode = "async" + ) { + this.throwIfEventNotRegistered(event); + + const listener = new EventListener(event, handler, mode); + listener.once = true; + this.addListener(listener as any); + } + on( slug: string, handler: ListenerHandler>, @@ -145,27 +161,73 @@ export class EventManager< this.events.forEach((event) => this.onEvent(event, handler, mode)); } - async emit(event: Event) { + protected executeAsyncs(promises: (() => Promise)[]) { + const executor = this.options?.asyncExecutor ?? ((e) => Promise.all(e)); + executor(promises.map((p) => p())).then(() => void 0); + } + + async emit>(event: Actual): Promise { // @ts-expect-error slug is static const slug = event.constructor.slug; if (!this.enabled) { console.log("EventManager disabled, not emitting", slug); - return; + return event; } if (!this.eventExists(event)) { throw new Error(`Event "${slug}" not registered`); } - const listeners = this.listeners.filter((listener) => listener.event.slug === slug); - //console.log("---!-- emitting", slug, listeners.length); + const syncs: EventListener[] = []; + const asyncs: (() => Promise)[] = []; + + this.listeners = this.listeners.filter((listener) => { + // if no match, keep and ignore + if (listener.event.slug !== slug) return true; - for (const listener of listeners) { if (listener.mode === "sync") { - await listener.handler(event, listener.event.slug); + syncs.push(listener); } else { - listener.handler(event, listener.event.slug); + asyncs.push(async () => await listener.handler(event, listener.event.slug)); + } + // Remove if `once` is true, otherwise keep + return !listener.once; + }); + + // execute asyncs + this.executeAsyncs(asyncs); + + // execute syncs + let _event: Actual = event; + for (const listener of syncs) { + try { + const return_value = (await listener.handler(_event, listener.event.slug)) as any; + + if (typeof return_value !== "undefined") { + const newEvent = _event.validate(return_value); + // @ts-expect-error slug is static + if (newEvent && newEvent.constructor.slug === slug) { + if (!newEvent.returned) { + throw new Error( + // @ts-expect-error slug is static + `Returned event ${newEvent.constructor.slug} must be marked as returned.` + ); + } + _event = newEvent as Actual; + } + } + } catch (e) { + if (e instanceof InvalidEventReturn) { + this.options?.onInvalidReturn?.(_event, e); + console.warn(`Invalid return of event listener for "${slug}": ${e.message}`); + } else if (this.options?.onError) { + this.options.onError(_event, e); + } else { + throw e; + } } } + + return _event; } } diff --git a/app/src/core/events/index.ts b/app/src/core/events/index.ts index b823edf..1edb065 100644 --- a/app/src/core/events/index.ts +++ b/app/src/core/events/index.ts @@ -1,8 +1,8 @@ -export { Event, NoParamEvent } from "./Event"; +export { Event, NoParamEvent, InvalidEventReturn } from "./Event"; export { EventListener, ListenerModes, type ListenerMode, - type ListenerHandler, + type ListenerHandler } from "./EventListener"; export { EventManager, type EmitsEvents, type EventClass } from "./EventManager"; diff --git a/tmp/event_manager_returning_test.patch b/tmp/event_manager_returning_test.patch deleted file mode 100644 index 1e194ef..0000000 --- a/tmp/event_manager_returning_test.patch +++ /dev/null @@ -1,150 +0,0 @@ -Subject: [PATCH] event manager returning test ---- -Index: app/__test__/core/EventManager.spec.ts -IDEA additional info: -Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP -<+>UTF-8 -=================================================================== -diff --git a/app/__test__/core/EventManager.spec.ts b/app/__test__/core/EventManager.spec.ts ---- a/app/__test__/core/EventManager.spec.ts (revision f06777256f332766de4bc76c23183725c8c7d310) -+++ b/app/__test__/core/EventManager.spec.ts (date 1731498680965) -@@ -1,8 +1,8 @@ - import { describe, expect, test } from "bun:test"; --import { Event, EventManager, NoParamEvent } from "../../src/core/events"; -+import { Event, EventManager, type ListenerHandler, NoParamEvent } from "../../src/core/events"; - - class SpecialEvent extends Event<{ foo: string }> { -- static slug = "special-event"; -+ static override slug = "special-event"; - - isBar() { - return this.params.foo === "bar"; -@@ -10,7 +10,19 @@ - } - - class InformationalEvent extends NoParamEvent { -- static slug = "informational-event"; -+ static override slug = "informational-event"; -+} -+ -+class ReturnEvent extends Event<{ foo: string }, number> { -+ static override slug = "return-event"; -+ static override returning = true; -+ -+ override setValidatedReturn(value: number) { -+ if (typeof value !== "number") { -+ throw new Error("Invalid return value"); -+ } -+ this.params.foo = value.toString(); -+ } - } - - describe("EventManager", async () => { -@@ -43,4 +55,22 @@ - - expect(true).toBe(true); - }); -+ -+ test.only("piping", async () => { -+ const emgr = new EventManager(); -+ emgr.registerEvents([ReturnEvent, InformationalEvent]); -+ -+ type T = ListenerHandler; -+ -+ // @ts-expect-error InformationalEvent has no return value -+ emgr.onEvent(InformationalEvent, async (event, name) => { -+ console.log("Event: ", name, event.params); -+ return 1; -+ }); -+ -+ emgr.onEvent(ReturnEvent, async (event, name) => { -+ console.log("Event: ", name, event.params); -+ return 1; -+ }); -+ }); - }); -Index: app/src/core/events/EventManager.ts -IDEA additional info: -Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP -<+>UTF-8 -=================================================================== -diff --git a/app/src/core/events/EventManager.ts b/app/src/core/events/EventManager.ts ---- a/app/src/core/events/EventManager.ts (revision f06777256f332766de4bc76c23183725c8c7d310) -+++ b/app/src/core/events/EventManager.ts (date 1731498680971) -@@ -6,7 +6,7 @@ - } - - export type EventClass = { -- new (params: any): Event; -+ new (params: any): Event; - slug: string; - }; - -@@ -137,6 +137,9 @@ - throw new Error(`Event "${slug}" not registered`); - } - -+ // @ts-expect-error returning is static -+ const returning = Boolean(event.constructor.returning); -+ - const listeners = this.listeners.filter((listener) => listener.event.slug === slug); - //console.log("---!-- emitting", slug, listeners.length); - -Index: app/src/core/events/EventListener.ts -IDEA additional info: -Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP -<+>UTF-8 -=================================================================== -diff --git a/app/src/core/events/EventListener.ts b/app/src/core/events/EventListener.ts ---- a/app/src/core/events/EventListener.ts (revision f06777256f332766de4bc76c23183725c8c7d310) -+++ b/app/src/core/events/EventListener.ts (date 1731498680968) -@@ -4,10 +4,10 @@ - export const ListenerModes = ["sync", "async"] as const; - export type ListenerMode = (typeof ListenerModes)[number]; - --export type ListenerHandler = ( -+export type ListenerHandler> = ( - event: E, -- slug: string, --) => Promise | void; -+ slug: string -+) => E extends Event ? R | Promise : never; - - export class EventListener { - mode: ListenerMode = "async"; -Index: app/src/core/events/Event.ts -IDEA additional info: -Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP -<+>UTF-8 -=================================================================== -diff --git a/app/src/core/events/Event.ts b/app/src/core/events/Event.ts ---- a/app/src/core/events/Event.ts (revision f06777256f332766de4bc76c23183725c8c7d310) -+++ b/app/src/core/events/Event.ts (date 1731498680973) -@@ -1,17 +1,25 @@ --export abstract class Event { -+export abstract class Event { - /** - * Unique event slug - * Must be static, because registering events is done by class - */ - static slug: string = "untitled-event"; - params: Params; -+ _returning!: Returning; -+ static returning: boolean = false; -+ -+ setValidatedReturn(value: Returning): void { -+ if (typeof value !== "undefined") { -+ throw new Error("Invalid event return value"); -+ } -+ } - - constructor(params: Params) { - this.params = params; - } - } - --// @todo: current workaround: potentially there is none and that's the way -+// @todo: current workaround: potentially there is "none" and that's the way - export class NoParamEvent extends Event { - static override slug: string = "noparam-event"; - From 438e36f185ff0199d8a030600527b38649478aa2 Mon Sep 17 00:00:00 2001 From: dswbx Date: Wed, 15 Jan 2025 17:46:41 +0100 Subject: [PATCH 2/5] refactor event listener registration unified listener creation logic under `createEventListener`, adding support for a flexible `RegisterListenerConfig`. Updated associated tests and improved error handling for unregistered events. --- app/__test__/core/EventManager.spec.ts | 7 +++- app/src/core/events/EventManager.ts | 56 ++++++++++++++------------ 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/app/__test__/core/EventManager.spec.ts b/app/__test__/core/EventManager.spec.ts index 4f11d19..ba5db93 100644 --- a/app/__test__/core/EventManager.spec.ts +++ b/app/__test__/core/EventManager.spec.ts @@ -61,6 +61,9 @@ describe("EventManager", async () => { "sync" ); + // don't allow unknown + expect(() => emgr.on("unknown", () => void 0)).toThrow(); + emgr.onEvent(InformationalEvent, async (event, name) => { call(); expect(name).toBe("informational-event"); @@ -135,14 +138,14 @@ describe("EventManager", async () => { const call = mock(() => null); const emgr = new EventManager({ InformationalEvent }); - emgr.onEventOnce( + emgr.onEvent( InformationalEvent, async (event, slug) => { expect(event).toBeInstanceOf(InformationalEvent); expect(slug).toBe("informational-event"); call(); }, - "sync" + { mode: "sync", once: true } ); expect(emgr.getListeners().length).toBe(1); diff --git a/app/src/core/events/EventManager.ts b/app/src/core/events/EventManager.ts index 6e48224..ec271a4 100644 --- a/app/src/core/events/EventManager.ts +++ b/app/src/core/events/EventManager.ts @@ -1,6 +1,13 @@ import { type Event, InvalidEventReturn } from "./Event"; import { EventListener, type ListenerHandler, type ListenerMode } from "./EventListener"; +export type RegisterListenerConfig = + | ListenerMode + | { + mode?: ListenerMode; + once?: boolean; + }; + export interface EmitsEvents { emgr: EventManager; } @@ -86,9 +93,11 @@ export class EventManager< return !!this.events.find((e) => slug === e.slug); } - protected throwIfEventNotRegistered(event: EventClass) { - if (!this.eventExists(event)) { - throw new Error(`Event "${event.slug}" not registered`); + protected throwIfEventNotRegistered(event: EventClass | Event | string) { + if (!this.eventExists(event as any)) { + // @ts-expect-error + const name = event.constructor?.slug ?? event.slug ?? event; + throw new Error(`Event "${name}" not registered`); } } @@ -121,44 +130,39 @@ export class EventManager< return this; } - onEvent>( - event: ActualEvent, - handler: ListenerHandler, - mode: ListenerMode = "async" + protected createEventListener( + _event: EventClass | string, + handler: ListenerHandler, + _config: RegisterListenerConfig = "async" ) { - this.throwIfEventNotRegistered(event); - - const listener = new EventListener(event, handler, mode); + const event = + typeof _event === "string" ? this.events.find((e) => e.slug === _event)! : _event; + const config = typeof _config === "string" ? { mode: _config } : _config; + const listener = new EventListener(event, handler, config.mode); + if (config.once) { + listener.once = true; + } this.addListener(listener as any); } - onEventOnce>( + onEvent>( event: ActualEvent, handler: ListenerHandler, - mode: ListenerMode = "async" + config?: RegisterListenerConfig ) { - this.throwIfEventNotRegistered(event); - - const listener = new EventListener(event, handler, mode); - listener.once = true; - this.addListener(listener as any); + this.createEventListener(event, handler, config); } on( slug: string, handler: ListenerHandler>, - mode: ListenerMode = "async" + config?: RegisterListenerConfig ) { - const event = this.events.find((e) => e.slug === slug); - if (!event) { - throw new Error(`Event "${slug}" not registered`); - } - - this.onEvent(event, handler, mode); + this.createEventListener(slug, handler, config); } - onAny(handler: ListenerHandler>, mode: ListenerMode = "async") { - this.events.forEach((event) => this.onEvent(event, handler, mode)); + onAny(handler: ListenerHandler>, config?: RegisterListenerConfig) { + this.events.forEach((event) => this.onEvent(event, handler, config)); } protected executeAsyncs(promises: (() => Promise)[]) { From 6c9707d12c4ac88a4e4a5f2b97b45f87ba9f68ed Mon Sep 17 00:00:00 2001 From: dswbx Date: Thu, 16 Jan 2025 10:10:47 +0100 Subject: [PATCH 3/5] refactored mutator to listen for returned data from event listeners --- app/src/core/config.ts | 9 +++++-- app/src/core/events/Event.ts | 19 ++++++++++++- app/src/core/events/EventManager.ts | 8 +++--- app/src/data/entities/Entity.ts | 20 ++++++++++++-- app/src/data/entities/Mutator.ts | 20 ++++++++------ app/src/data/events/index.ts | 42 ++++++++++++++++++++++++----- app/src/ui/client/api/use-entity.ts | 6 ++--- 7 files changed, 96 insertions(+), 28 deletions(-) diff --git a/app/src/core/config.ts b/app/src/core/config.ts index a99d549..2f2cf06 100644 --- a/app/src/core/config.ts +++ b/app/src/core/config.ts @@ -5,8 +5,13 @@ import type { Generated } from "kysely"; export type PrimaryFieldType = number | Generated; -// biome-ignore lint/suspicious/noEmptyInterface: -export interface DB {} +export interface DB { + // make sure to make unknown as "any" + [key: string]: { + id: PrimaryFieldType; + [key: string]: any; + }; +} export const config = { server: { diff --git a/app/src/core/events/Event.ts b/app/src/core/events/Event.ts index 734c7c2..8073c12 100644 --- a/app/src/core/events/Event.ts +++ b/app/src/core/events/Event.ts @@ -1,3 +1,8 @@ +export type EventClass = { + new (params: any): Event; + slug: string; +}; + export abstract class Event { _returning!: Returning; @@ -9,7 +14,9 @@ export abstract class Event { params: Params; returned: boolean = false; - validate(value: Returning): Event | void {} + validate(value: Returning): Event | void { + throw new EventReturnedWithoutValidation(this as any, value); + } protected clone = Event>( this: This, @@ -39,3 +46,13 @@ export class InvalidEventReturn extends Error { super(`Expected "${expected}", got "${given}"`); } } + +export class EventReturnedWithoutValidation extends Error { + constructor( + event: EventClass, + public data: any + ) { + // @ts-expect-error slug is static + super(`Event "${event.constructor.slug}" returned without validation`); + } +} diff --git a/app/src/core/events/EventManager.ts b/app/src/core/events/EventManager.ts index ec271a4..73764ea 100644 --- a/app/src/core/events/EventManager.ts +++ b/app/src/core/events/EventManager.ts @@ -1,4 +1,4 @@ -import { type Event, InvalidEventReturn } from "./Event"; +import { type Event, type EventClass, InvalidEventReturn } from "./Event"; import { EventListener, type ListenerHandler, type ListenerMode } from "./EventListener"; export type RegisterListenerConfig = @@ -12,10 +12,8 @@ export interface EmitsEvents { emgr: EventManager; } -export type EventClass = { - new (params: any): Event; - slug: string; -}; +// for compatibility, moved it to Event.ts +export type { EventClass }; export class EventManager< RegisteredEvents extends Record = Record diff --git a/app/src/data/entities/Entity.ts b/app/src/data/entities/Entity.ts index a87d609..4665322 100644 --- a/app/src/data/entities/Entity.ts +++ b/app/src/data/entities/Entity.ts @@ -192,10 +192,26 @@ export class Entity< this.data = data; } + // @todo: add tests isValidData(data: EntityData, context: TActionContext, explain?: boolean): boolean { + if (typeof data !== "object") { + if (explain) { + throw new Error(`Entity "${this.name}" data must be an object`); + } + } + const fields = this.getFillableFields(context, false); - //const fields = this.fields; - //console.log("data", data); + const field_names = fields.map((f) => f.name); + const given_keys = Object.keys(data); + + if (given_keys.some((key) => !field_names.includes(key))) { + if (explain) { + throw new Error( + `Entity "${this.name}" data must only contain known keys, got: "${given_keys}"` + ); + } + } + for (const field of fields) { if (!field.isValid(data[field.name], context)) { console.log("Entity.isValidData:invalid", context, field.name, data[field.name]); diff --git a/app/src/data/entities/Mutator.ts b/app/src/data/entities/Mutator.ts index d9bff38..15760bc 100644 --- a/app/src/data/entities/Mutator.ts +++ b/app/src/data/entities/Mutator.ts @@ -132,14 +132,17 @@ export class Mutator< throw new Error(`Creation of system entity "${entity.name}" is disabled`); } - // @todo: establish the original order from "data" + const result = await this.emgr.emit( + new Mutator.Events.MutatorInsertBefore({ entity, data: data as any }) + ); + + // if listener returned, take what's returned + const _data = result.returned ? result.params.data : data; const validatedData = { ...entity.getDefaultObject(), - ...(await this.getValidatedData(data, "create")) + ...(await this.getValidatedData(_data, "create")) }; - await this.emgr.emit(new Mutator.Events.MutatorInsertBefore({ entity, data: validatedData })); - // check if required fields are present const required = entity.getRequiredFields(); for (const field of required) { @@ -169,16 +172,17 @@ export class Mutator< throw new Error("ID must be provided for update"); } - const validatedData = await this.getValidatedData(data, "update"); - - await this.emgr.emit( + const result = await this.emgr.emit( new Mutator.Events.MutatorUpdateBefore({ entity, entityId: id, - data: validatedData as any + data }) ); + const _data = result.returned ? result.params.data : data; + const validatedData = await this.getValidatedData(_data, "update"); + const query = this.conn .updateTable(entity.name) .set(validatedData as any) diff --git a/app/src/data/events/index.ts b/app/src/data/events/index.ts index 01311d8..3ea038c 100644 --- a/app/src/data/events/index.ts +++ b/app/src/data/events/index.ts @@ -1,20 +1,48 @@ import type { PrimaryFieldType } from "core"; -import { Event } from "core/events"; +import { Event, InvalidEventReturn } from "core/events"; import type { Entity, EntityData } from "../entities"; import type { RepoQuery } from "../server/data-query-impl"; -export class MutatorInsertBefore extends Event<{ entity: Entity; data: EntityData }> { +export class MutatorInsertBefore extends Event<{ entity: Entity; data: EntityData }, EntityData> { static override slug = "mutator-insert-before"; + + override validate(data: EntityData) { + const { entity } = this.params; + if (!entity.isValidData(data, "create")) { + throw new InvalidEventReturn("EntityData", "invalid"); + } + + return this.clone({ + entity, + data + }); + } } export class MutatorInsertAfter extends Event<{ entity: Entity; data: EntityData }> { static override slug = "mutator-insert-after"; } -export class MutatorUpdateBefore extends Event<{ - entity: Entity; - entityId: PrimaryFieldType; - data: EntityData; -}> { +export class MutatorUpdateBefore extends Event< + { + entity: Entity; + entityId: PrimaryFieldType; + data: EntityData; + }, + EntityData +> { static override slug = "mutator-update-before"; + + override validate(data: EntityData) { + const { entity, ...rest } = this.params; + if (!entity.isValidData(data, "update")) { + throw new InvalidEventReturn("EntityData", "invalid"); + } + + return this.clone({ + ...rest, + entity, + data + }); + } } export class MutatorUpdateAfter extends Event<{ entity: Entity; diff --git a/app/src/ui/client/api/use-entity.ts b/app/src/ui/client/api/use-entity.ts index 5f4da99..fba6a45 100644 --- a/app/src/ui/client/api/use-entity.ts +++ b/app/src/ui/client/api/use-entity.ts @@ -109,7 +109,7 @@ export const useEntityQuery = < options?: SWRConfiguration & { enabled?: boolean; revalidateOnMutate?: boolean } ) => { const api = useApi().data; - const key = makeKey(api, entity, id, query); + const key = makeKey(api, entity as string, id, query); const { read, ...actions } = useEntity(entity, id); const fetcher = () => read(query); @@ -121,7 +121,7 @@ export const useEntityQuery = < }); const mutateAll = async () => { - const entityKey = makeKey(api, entity); + const entityKey = makeKey(api, entity as string); return mutate((key) => typeof key === "string" && key.startsWith(entityKey), undefined, { revalidate: true }); @@ -167,7 +167,7 @@ export async function mutateEntityCache< return prev; } - const entityKey = makeKey(api, entity); + const entityKey = makeKey(api, entity as string); return mutate( (key) => typeof key === "string" && key.startsWith(entityKey), From aa4aca1a9044c3c3e078218174b7ca9887330f0a Mon Sep 17 00:00:00 2001 From: dswbx Date: Thu, 16 Jan 2025 10:11:10 +0100 Subject: [PATCH 4/5] added event related tests to mutator, fixed tests --- app/__test__/core/EventManager.spec.ts | 8 +- app/__test__/data/specs/Mutator.spec.ts | 74 ++++++++++++++++++- app/__test__/data/specs/Repository.spec.ts | 5 +- app/__test__/data/specs/WithBuilder.spec.ts | 4 +- .../data/specs/fields/EnumField.spec.ts | 4 - app/__test__/data/specs/fields/Field.spec.ts | 4 +- .../data/specs/fields/JsonField.spec.ts | 2 +- .../specs/relations/EntityRelation.spec.ts | 4 +- .../adapters/StorageCloudinaryAdapter.spec.ts | 2 +- .../adapters/StorageLocalAdapter.spec.ts | 2 +- .../media/adapters/StorageS3Adapter.spec.ts | 6 +- app/bunfig.toml | 5 +- app/package.json | 1 + 13 files changed, 89 insertions(+), 32 deletions(-) diff --git a/app/__test__/core/EventManager.spec.ts b/app/__test__/core/EventManager.spec.ts index ba5db93..e332439 100644 --- a/app/__test__/core/EventManager.spec.ts +++ b/app/__test__/core/EventManager.spec.ts @@ -1,11 +1,5 @@ import { afterAll, beforeAll, describe, expect, mock, test } from "bun:test"; -import { - Event, - EventManager, - InvalidEventReturn, - type ListenerHandler, - NoParamEvent -} from "../../src/core/events"; +import { Event, EventManager, InvalidEventReturn, NoParamEvent } from "../../src/core/events"; import { disableConsoleLog, enableConsoleLog } from "../helper"; beforeAll(disableConsoleLog); diff --git a/app/__test__/data/specs/Mutator.spec.ts b/app/__test__/data/specs/Mutator.spec.ts index 5552543..47134e8 100644 --- a/app/__test__/data/specs/Mutator.spec.ts +++ b/app/__test__/data/specs/Mutator.spec.ts @@ -10,6 +10,7 @@ import { RelationMutator, TextField } from "../../../src/data"; +import * as proto from "../../../src/data/prototype"; import { getDummyConnection } from "../helper"; const { dummyConnection, afterAllCleanup } = getDummyConnection(); @@ -83,14 +84,12 @@ describe("[data] Mutator (ManyToOne)", async () => { // persisting reference should ... expect( - postRelMutator.persistReference(relations[0], "users", { + postRelMutator.persistReference(relations[0]!, "users", { $set: { id: userData.data.id } }) ).resolves.toEqual(["users_id", userData.data.id]); // @todo: add what methods are allowed to relation, like $create should not be allowed for post<>users - process.exit(0); - const userRelMutator = new RelationMutator(users, em); expect(userRelMutator.getRelationalKeys()).toEqual(["posts"]); }); @@ -99,7 +98,7 @@ describe("[data] Mutator (ManyToOne)", async () => { expect( em.mutator(posts).insertOne({ title: "post1", - users_id: 1 // user does not exist yet + users_id: 100 // user does not exist yet }) ).rejects.toThrow(); }); @@ -299,4 +298,71 @@ describe("[data] Mutator (Events)", async () => { expect(events.has(MutatorEvents.MutatorDeleteBefore.slug)).toBeTrue(); expect(events.has(MutatorEvents.MutatorDeleteAfter.slug)).toBeTrue(); }); + + /*test("insertOne event return is respected", async () => { + const posts = proto.entity("posts", { + title: proto.text(), + views: proto.number() + }); + + const conn = getDummyConnection(); + const em = new EntityManager([posts], conn.dummyConnection); + await em.schema().sync({ force: true }); + + const emgr = em.emgr as EventManager; + + emgr.onEvent( + // @ts-ignore + EntityManager.Events.MutatorInsertBefore, + async (event) => { + return { + ...event.params.data, + views: 2 + }; + }, + "sync" + ); + + const mutator = em.mutator("posts"); + const result = await mutator.insertOne({ title: "test", views: 1 }); + expect(result.data).toEqual({ + id: 1, + title: "test", + views: 2 + }); + }); + + test("updateOne event return is respected", async () => { + const posts = proto.entity("posts", { + title: proto.text(), + views: proto.number() + }); + + const conn = getDummyConnection(); + const em = new EntityManager([posts], conn.dummyConnection); + await em.schema().sync({ force: true }); + + const emgr = em.emgr as EventManager; + + emgr.onEvent( + // @ts-ignore + EntityManager.Events.MutatorUpdateBefore, + async (event) => { + return { + ...event.params.data, + views: event.params.data.views + 1 + }; + }, + "sync" + ); + + const mutator = em.mutator("posts"); + const created = await mutator.insertOne({ title: "test", views: 1 }); + const result = await mutator.updateOne(created.data.id, { views: 2 }); + expect(result.data).toEqual({ + id: 1, + title: "test", + views: 3 + }); + });*/ }); diff --git a/app/__test__/data/specs/Repository.spec.ts b/app/__test__/data/specs/Repository.spec.ts index 0ce8da1..d873389 100644 --- a/app/__test__/data/specs/Repository.spec.ts +++ b/app/__test__/data/specs/Repository.spec.ts @@ -1,7 +1,6 @@ import { afterAll, describe, expect, test } from "bun:test"; -// @ts-ignore -import { Perf } from "@bknd/core/utils"; import type { Kysely, Transaction } from "kysely"; +import { Perf } from "../../../src/core/utils"; import { Entity, EntityManager, @@ -24,7 +23,7 @@ async function sleep(ms: number) { } describe("[Repository]", async () => { - test("bulk", async () => { + test.skip("bulk", async () => { //const connection = dummyConnection; //const connection = getLocalLibsqlConnection(); const credentials = null as any; // @todo: determine what to do here diff --git a/app/__test__/data/specs/WithBuilder.spec.ts b/app/__test__/data/specs/WithBuilder.spec.ts index 9141b62..367d3f0 100644 --- a/app/__test__/data/specs/WithBuilder.spec.ts +++ b/app/__test__/data/specs/WithBuilder.spec.ts @@ -36,7 +36,7 @@ describe("[data] WithBuilder", async () => { const res = qb.compile(); expect(res.sql).toBe( - 'select (select coalesce(json_group_array(json_object(\'id\', "agg"."id", \'content\', "agg"."content", \'author_id\', "agg"."author_id")), \'[]\') from (select "posts"."id" as "id", "posts"."content" as "content", "posts"."author_id" as "author_id" from "posts" where "users"."id" = "posts"."author_id" limit ?) as agg) as "posts" from "users"' + 'select (select coalesce(json_group_array(json_object(\'id\', "agg"."id", \'content\', "agg"."content", \'author_id\', "agg"."author_id")), \'[]\') from (select "posts"."id" as "id", "posts"."content" as "content", "posts"."author_id" as "author_id" from "posts" as "posts" where "posts"."author_id" = "users"."id" limit ?) as agg) as "posts" from "users"' ); expect(res.parameters).toEqual([5]); @@ -50,7 +50,7 @@ describe("[data] WithBuilder", async () => { const res2 = qb2.compile(); expect(res2.sql).toBe( - 'select (select json_object(\'id\', "obj"."id", \'username\', "obj"."username") from (select "users"."id" as "id", "users"."username" as "username" from "users" where "posts"."author_id" = "users"."id" limit ?) as obj) as "author" from "posts"' + 'select (select json_object(\'id\', "obj"."id", \'username\', "obj"."username") from (select "author"."id" as "id", "author"."username" as "username" from "users" as "author" where "author"."id" = "posts"."author_id" limit ?) as obj) as "author" from "posts"' ); expect(res2.parameters).toEqual([1]); }); diff --git a/app/__test__/data/specs/fields/EnumField.spec.ts b/app/__test__/data/specs/fields/EnumField.spec.ts index d60f2e7..7cde4eb 100644 --- a/app/__test__/data/specs/fields/EnumField.spec.ts +++ b/app/__test__/data/specs/fields/EnumField.spec.ts @@ -13,10 +13,6 @@ describe("[data] EnumField", async () => { { options: options(["a", "b", "c"]) } ); - test("yields if no options", async () => { - expect(() => new EnumField("test", { options: options([]) })).toThrow(); - }); - test("yields if default value is not a valid option", async () => { expect( () => new EnumField("test", { options: options(["a", "b"]), default_value: "c" }) diff --git a/app/__test__/data/specs/fields/Field.spec.ts b/app/__test__/data/specs/fields/Field.spec.ts index 6fd8e04..77eb3fd 100644 --- a/app/__test__/data/specs/fields/Field.spec.ts +++ b/app/__test__/data/specs/fields/Field.spec.ts @@ -15,11 +15,9 @@ describe("[data] Field", async () => { runBaseFieldTests(FieldSpec, { defaultValue: "test", schemaType: "text" }); - test.only("default config", async () => { - const field = new FieldSpec("test"); + test("default config", async () => { const config = Default(baseFieldConfigSchema, {}); expect(stripMark(new FieldSpec("test").config)).toEqual(config); - console.log("config", new TextField("test", { required: true }).toJSON()); }); test("transformPersist (specific)", async () => { diff --git a/app/__test__/data/specs/fields/JsonField.spec.ts b/app/__test__/data/specs/fields/JsonField.spec.ts index f13968a..17fdaaa 100644 --- a/app/__test__/data/specs/fields/JsonField.spec.ts +++ b/app/__test__/data/specs/fields/JsonField.spec.ts @@ -32,7 +32,7 @@ describe("[data] JsonField", async () => { }); test("getValue", async () => { - expect(field.getValue({ test: 1 }, "form")).toBe('{"test":1}'); + expect(field.getValue({ test: 1 }, "form")).toBe('{\n "test": 1\n}'); expect(field.getValue("string", "form")).toBe('"string"'); expect(field.getValue(1, "form")).toBe("1"); diff --git a/app/__test__/data/specs/relations/EntityRelation.spec.ts b/app/__test__/data/specs/relations/EntityRelation.spec.ts index 989b4f9..92c50e3 100644 --- a/app/__test__/data/specs/relations/EntityRelation.spec.ts +++ b/app/__test__/data/specs/relations/EntityRelation.spec.ts @@ -70,9 +70,9 @@ describe("[data] EntityRelation", async () => { it("required", async () => { const relation1 = new TestEntityRelation(); - expect(relation1.config.required).toBe(false); + expect(relation1.required).toBe(false); const relation2 = new TestEntityRelation({ required: true }); - expect(relation2.config.required).toBe(true); + expect(relation2.required).toBe(true); }); }); diff --git a/app/__test__/media/adapters/StorageCloudinaryAdapter.spec.ts b/app/__test__/media/adapters/StorageCloudinaryAdapter.spec.ts index 1294275..e2457b2 100644 --- a/app/__test__/media/adapters/StorageCloudinaryAdapter.spec.ts +++ b/app/__test__/media/adapters/StorageCloudinaryAdapter.spec.ts @@ -3,7 +3,7 @@ import { randomString } from "../../../src/core/utils"; import { StorageCloudinaryAdapter } from "../../../src/media"; import { config } from "dotenv"; -const dotenvOutput = config({ path: `${import.meta.dir}/../../.env` }); +const dotenvOutput = config({ path: `${import.meta.dir}/../../../.env` }); const { CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, diff --git a/app/__test__/media/adapters/StorageLocalAdapter.spec.ts b/app/__test__/media/adapters/StorageLocalAdapter.spec.ts index 29746d1..a7c6d79 100644 --- a/app/__test__/media/adapters/StorageLocalAdapter.spec.ts +++ b/app/__test__/media/adapters/StorageLocalAdapter.spec.ts @@ -15,7 +15,7 @@ describe("StorageLocalAdapter", () => { test("puts an object", async () => { objects = (await adapter.listObjects()).length; - expect(await adapter.putObject(filename, await file.arrayBuffer())).toBeString(); + expect(await adapter.putObject(filename, file)).toBeString(); }); test("lists objects", async () => { diff --git a/app/__test__/media/adapters/StorageS3Adapter.spec.ts b/app/__test__/media/adapters/StorageS3Adapter.spec.ts index d6274dc..7ea77b1 100644 --- a/app/__test__/media/adapters/StorageS3Adapter.spec.ts +++ b/app/__test__/media/adapters/StorageS3Adapter.spec.ts @@ -3,14 +3,14 @@ import { randomString } from "../../../src/core/utils"; import { StorageS3Adapter } from "../../../src/media"; import { config } from "dotenv"; -const dotenvOutput = config({ path: `${import.meta.dir}/../../.env` }); +const dotenvOutput = config({ path: `${import.meta.dir}/../../../.env` }); const { R2_ACCESS_KEY, R2_SECRET_ACCESS_KEY, R2_URL, AWS_ACCESS_KEY, AWS_SECRET_KEY, AWS_S3_URL } = dotenvOutput.parsed!; // @todo: mock r2/s3 responses for faster tests -const ALL_TESTS = process.env.ALL_TESTS; +const ALL_TESTS = !!process.env.ALL_TESTS; -describe("Storage", async () => { +describe.skipIf(ALL_TESTS)("StorageS3Adapter", async () => { console.log("ALL_TESTS", process.env.ALL_TESTS); const versions = [ [ diff --git a/app/bunfig.toml b/app/bunfig.toml index 82e1cd0..6f4fe9a 100644 --- a/app/bunfig.toml +++ b/app/bunfig.toml @@ -1,2 +1,5 @@ [install] -registry = "http://localhost:4873" \ No newline at end of file +#registry = "http://localhost:4873" + +[test] +coverageSkipTestFiles = true \ No newline at end of file diff --git a/app/package.json b/app/package.json index 8baeefd..d8b73d8 100644 --- a/app/package.json +++ b/app/package.json @@ -7,6 +7,7 @@ "scripts": { "dev": "vite", "test": "ALL_TESTS=1 bun test --bail", + "test:coverage": "ALL_TESTS=1 bun test --bail --coverage", "build": "NODE_ENV=production bun run build.ts --minify --types", "build:all": "rm -rf dist && bun run build:static && NODE_ENV=production bun run build.ts --minify --types --clean && bun run build:cli", "build:cli": "bun build src/cli/index.ts --target node --outdir dist/cli --minify", From f47218708a8453b8ab0088056a90e47cf6cc4df2 Mon Sep 17 00:00:00 2001 From: dswbx Date: Thu, 16 Jan 2025 10:13:54 +0100 Subject: [PATCH 5/5] added event related tests to mutator, fixed tests --- app/__test__/data/specs/Mutator.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/__test__/data/specs/Mutator.spec.ts b/app/__test__/data/specs/Mutator.spec.ts index 47134e8..6493d52 100644 --- a/app/__test__/data/specs/Mutator.spec.ts +++ b/app/__test__/data/specs/Mutator.spec.ts @@ -1,4 +1,5 @@ import { afterAll, describe, expect, test } from "bun:test"; +import type { EventManager } from "../../../src/core/events"; import { Entity, EntityManager, @@ -299,7 +300,7 @@ describe("[data] Mutator (Events)", async () => { expect(events.has(MutatorEvents.MutatorDeleteAfter.slug)).toBeTrue(); }); - /*test("insertOne event return is respected", async () => { + test("insertOne event return is respected", async () => { const posts = proto.entity("posts", { title: proto.text(), views: proto.number() @@ -364,5 +365,5 @@ describe("[data] Mutator (Events)", async () => { title: "test", views: 3 }); - });*/ + }); });