search

symfony / mercure

The Mercure Component allows to easily push updates to web browsers and other HTTP clients using the Mercure protocol.
https://symfony.com/doc/current/components/mercure.html
MIT License
413 stars 39 forks source link

Add event on Update publish to Hub #86

Closed tostiheld closed 9 months ago

tostiheld commented 2 years ago

For debugging purposes, we maintain an API log. We have a listener on KernelEvents::TERMINATE that writes any relevant request/response information to a log file. However we do not have this for SSE's. That's why we now decorate the Publisher/Hub to add this functionality. I thought it might be nice to submit this change as a pull request, as I can imagine it might be useful for others as well.

dunglas commented 2 years ago

The code looks good to me, however I'm unsure if this use case is frequent enough to be covered directly in the core component, as it's already possible to use the current extension points. WDYT @symfony/mergers?

tostiheld commented 2 years ago

By other extension points you mean decorating the Hub? Or is there some other extension point I am not aware of?

dunglas commented 2 years ago

Yes, decorating the hub.

wouterj commented 2 years ago

Adding such event is consistent with e.g. the event dispatched by the Workflow or Console components. That said, we're not very consistent with events in components (e.g. HttpClient and Mailer don't dispatch lifecycle events).

Decoration does add some cognitive overhead, as you need to look at the DI configuration file in order to discover that the default hub service is decorated with one of your own classes.

So I'm leaning towards +1 for this PR. Can we think of other use-cases of this event? Otherwise, we maybe can add optional support for psr/log directly in hub instead of dispatching an event?

dunglas commented 1 year ago

I would prefer adding optional support psr/log than maintaining another extension point.

tostiheld commented 9 months ago

Closed in favour of #110