feat: [#730] add Listen and Dispatch to the event module - #1541
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1541 +/- ##
==========================================
+ Coverage 72.41% 72.60% +0.19%
==========================================
Files 409 412 +3
Lines 26475 26741 +266
==========================================
+ Hits 19172 19416 +244
- Misses 7301 7323 +22
Partials 2 2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
00ce3a2 to
489ddad
Compare
Introduce a Laravel style event dispatcher next to the existing Register and Job flow, which is now deprecated but keeps working unchanged. Listen registers one or more listeners for one or more events, accepting strings, wildcard patterns, event.Event values and slices of any of them. Listeners can be event.QueueListener implementations, plain func(event any, args ...any) error closures, or typed func(event *SomeEvent) error closures whose event is resolved from the parameter type. Dispatch fires an event, runs every matching listener and returns an event.Result carrying the errors of the ones that failed, joined through Error(), listed by Errors() and summarised by Failed(). Every accepted listener form is normalised once at registration into a single internal representation, so dispatching itself needs neither reflection nor type switches. Listeners implementing event.QueueListener are registered with the queue once per signature and pushed onto it when Queue().Enable is true, carrying the event name as their first argument because only scalars survive the queue boundary. A panicking listener fails on its own without breaking the rest of the dispatch. Also update the make:listener stub to the new interface.
489ddad to
3f89025
Compare
…h flow QueueListener.Handle now takes the canonical event name rather than the event itself. Previously it received the event when running in process and the name when running from the queue, so the type of its first argument depended on the transport, and Queue() can depend on the payload, so one listener could alternate between the two at runtime. The data a queued listener needs travels in args, since only scalars survive the queue boundary. The wildcard cache is gone. It stored an entry for every dispatched event name, including when no wildcard was registered at all, so an application using dynamic event names grew a map for the lifetime of the process. Wildcards are now kept in registration order and matched with an allocation free matcher, which also makes the invocation order of overlapping patterns deterministic rather than dependent on map iteration. Dispatching an event name never seen before now costs the same as dispatching a familiar one. Dispatch and the deprecated Task share one pipeline, parameterised by the two behaviours that genuinely differ: Task requires listeners and stops at the first error, so that a failing listener still prevents the jobs behind it from being queued, while Dispatch runs everything and collects. An event.Event prepares its payload in both, once, and only when there is a listener to prepare it for. Two listeners can no longer claim one queue signature. The queue resolves a job by signature alone, so the second registration would silently run the first listener's code. Listen now rejects the conflict, and rejects value listeners outright, since two values of the same type cannot be told apart. A panic inside a queue worker no longer escapes: queueJob.Handle recovers so the failure goes through the queue's own retry and failure handling instead of taking the process down. Also: queue registration no longer runs while the registry is locked, Listen validates a whole request before registering any of it, and a typed closure can no longer be registered on an event it could never be called for.
Review — event
|
|
@ahmed3mar About |
Listener now takes the canonical event name, and QueueListener is gone, so there is a single listener interface behind both Listen and the deprecated Register: Handle(eventName string, args ...any) error A listener no longer satisfies queue.Job, so Register wraps each one the way Listen already did, which means both flows put the same job on the queue with the event name as its first argument. Queueing is now decided the same way everywhere: Queue().Enable pushes the listener onto the queue, otherwise it runs in process. A listener registered through Register used to run through the queue facade even when it did not enable queueing, that round trip is gone. Register resolves the signatures and builds the wrappers before locking the registry. Resolving a signature calls into the listener, and doing so under the lock meant a panicking listener left the mutex held for the rest of the process. Wildcard patterns keep one entry per registration. Folding a repeated pattern into its earlier entry ran its later listeners before the patterns registered in between.
eventArgsToQueueArgs lost its last caller when the queue arguments were unified behind queueArgs, which always prepends the event name.
…the event error list A queue job now carries the signature it was built with, so the worker and its panic recovery report a job without calling back into the listener. A listener whose Signature panics can no longer defeat the recovery meant to contain it. The signature travels from the job to everything that needs it, rather than being asked of the listener again at each step. The event errors are named for the cases the module is growing into, and the block is back in alphabetical order.
An event that panics while preparing its payload is now contained the same way a panicking listener is, so a faulty event fails its own dispatch instead of unwinding the caller. Its listeners do not run, the payload was never prepared. Dispatch reports a second payload rather than silently dropping it. Job builds its task from the same registry Dispatch uses, so it reaches the listeners registered through Listen and the ones registered on a matching wildcard pattern. It used to look them up by the identity of the event value, which only ever found them when the event was a zero size struct. The queue branch that ran a job synchronously is gone, a listener that doesn't enable queueing never reached it. The wildcard benchmark now registers patterns that actually match the names it dispatches, and builds those names before the loop, so it measures dispatching rather than the harness.
hwbrzzl
left a comment
hwbrzzl
left a comment
There was a problem hiding this comment.
Thanks! Great job 👍 Could you add full tests in goravel/example as well?
3 participants
📑 Description
Closes goravel/goravel#730
Reimplementation of #1133 from scratch against current
master, addressing the review comments left on that PR.This adds a Laravel style event dispatcher —
Listen()andDispatch()— next to the existingRegister()/Job()flow, which is now deprecated but keeps working.ListenDispatchEvery listener runs, and
event.Resultcarries the errors of the ones that failed:Listener interface
There is one listener interface. It replaces the old one rather than sitting beside it, so
Listenand the deprecatedRegistertake the same type:Queue().Enabledecides whether the listener is pushed onto the queue or run in process, so no separateShouldQueueconcept is introduced. Listeners must be non-nil pointers: the queue resolves a job by its signature alone, and two values of the same type cannot be told apart, so a value listener could be enqueued and a different one executed.Design notes
queue/utils/convert.goconverts only scalars, so an event object cannot survive the queue. Rather than hand the listener an object in process and a string from a worker — a first argument whose type depends on the transport — aListeneralways receives the name. Data a queued listener needs travels inargs. Note this is not Laravel's behaviour: Laravel serialises the event object, which this queue cannot do.event/application_dispatch_benchmark_test.go.Dispatchand the deprecatedTaskdiffer only in the two behaviours that genuinely differ:Taskrequires listeners and stops at the first error, so a failing listener still prevents the jobs behind it from being queued;Dispatchruns everything and collects. Anevent.Eventprepares its payload in both, once, and only when a listener exists to prepare it for.Listenerand afunc(evt any, args ...any) errorare matched by type assertion and invoked directly. Typedfunc(e *UserCreated) errorclosures need onereflect.Call; they are validated at registration and rejected on an event they could never be called for.Handleare each contained, so one faulty implementation fails itself instead of unwinding the caller or killing a queue worker.errors/list.go.Migration
event.Listenerchanges shape, so every existing listener needs a mechanical edit. The steps below are written to be applied automatically.1.
Handletakes the event name. Add a leadingeventName stringparameter to every type implementingevent.Listener.Signature()andQueue()are unchanged. The payload keeps its positions: what used to beargs[0]is stillargs[0].2. Non-queued listeners registered through
Registernow run in process. A listener whoseQueue().Enableis false used to be handed to the queue facade and run through its sync driver; it is now called directly. Behaviour to check for: anything that relied on the queue's job wrapping, its failure recording, or a custom sync driver's instrumentation for these listeners.3.
Jobresolves listeners by event name. A fresh event instance now finds the listeners registered for that event, where it previously matched by the identity of the value and so only worked for zero-size structs.4. Type-derived event names are fully qualified. An event that is not a string is named by its import path plus its type name,
github.com/acme/app/events.UserCreated, so two packages both calledeventsno longer collide. That name is what a wildcard pattern is matched against, what aListenerreceives aseventName, and the first argument of a queued job. Anything that hardcoded the shortevents.UserCreatedform must be updated.5.
Listenvalidates strictly, and returns an error rather than registering something that could never fire. Applies to: closures, whose parameter must be a named struct or a pointer to one, and which cannot be registered on an event they do not name; slice kinds other than[]string,[]event.Eventand[]any; events that are neither a non-empty string nor a named struct; nil or non-pointer listeners; and listeners whoseSignature()is empty. Check the returned error at every call site.6. Deploy note — drain the event queues first. A queued event job pushed by a pre-upgrade binary carries no event name, and the new worker reads the first argument as one. Let the event queues empty before rolling out, or the in-flight jobs are misread.
Backward compatibility
Register,Job,GetEventsandevent.Taskkeep working, now markedDeprecated:.Registerare reachable byDispatch, and both paths run through the same pipeline.event.Listeneris a breaking change, see the migration above.InstancegainsListenandDispatch. That is source breaking for anything outside the framework that implementsevent.Instance; callers are unaffected.CI
The Test In Example jobs fail, and will keep failing until a companion PR lands: goravel/example implements the old
Handle(args ...any) error, so it no longer satisfiesevent.Listener. That PR will be opened separately. Every job that builds only this repository is green.Open question for the maintainer
make:listenergenerates the newevent.Listener, butWithEventsisfunc() map[event.Event][]event.Listener, whose key is anevent.Event, so it cannot express a string event, a wildcard pattern or a closure. A generated listener does fit it now that there is a single interface, but the rest of theListensurface does not. Options:WithEventsand addWithEventListeners(func(dispatcher event.Instance) error), run at the event configuration lifecycle point, legacy registrations first, a returned error failing bootstrap.Happy to do either; say which you prefer.
✅ Checks