Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WithMessageHandler overload not helpful #484

Closed
AnthonySteele opened this issue Dec 18, 2018 · 2 comments
Closed

WithMessageHandler overload not helpful #484

AnthonySteele opened this issue Dec 18, 2018 · 2 comments
Milestone

Comments

@AnthonySteele
Copy link
Contributor

AnthonySteele commented Dec 18, 2018

Having both of these overloads to register a handler makes some proposed new features harder:

        WithMessageHandler<T>(IHandlerAsync<T> handler) where T : Message;

        WithMessageHandler<T>(IHandlerResolver handlerResolver) where T : Message;

The first can't be used with any system that passes transient data to the handler instance constructor.

A new handler instance each time seems safer in general, otherwise it requires handler authors coding for re-entrancy, and all dependencies being long-lived, which is all not obvious.

Describe the solution you'd like

Remove the overload that takes a handler instance.
It is used in tests, but a dummy IHandlerResolver that only returns a supplied instance could be used instead there.

Additional context

Considered as useful for some kind of message context accessor injected into handler constructor.

@adammorr
Copy link
Contributor

I believe WithMessageHandler<T>(...) isn't a thing anymore with the changes @martincostello made to the fluent API

@AnthonySteele AnthonySteele changed the title WithMessageHandler WithMessageHandler overload not helpful Dec 18, 2018
@AnthonySteele AnthonySteele added this to the v7.0.0 milestone Dec 18, 2018
@martincostello
Copy link
Member

Yeah, this is an artifact of the design we want to move away from. The new API internally just uses the variant that uses the resolver.

https://github.com/justeat/JustSaying/blob/ef0e4a6b16cdef4388a948650866e79c351da48a/JustSaying/Fluent/TopicSubscriptionBuilder%601.cs#L104

This should get tidied up as part of #471.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants