-
Notifications
You must be signed in to change notification settings - Fork 78
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
[Host.HybridMessageSerialzier] Resolve HybridMessageSerializer dependencies from IServiceProvider #239
[Host.HybridMessageSerialzier] Resolve HybridMessageSerializer dependencies from IServiceProvider #239
Conversation
9361734
to
f29f09e
Compare
Removed some syntactic sugar that SonarCloud wasn't a fan of. |
src/SlimMessageBus.Host.Serialization.Hybrid/MessageBusBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/Tests/SlimMessageBus.Host.Serialization.Hybrid.Test/Helpers/SampleMessages.cs
Outdated
Show resolved
Hide resolved
src/Tests/SlimMessageBus.Host.Serialization.Hybrid.Test/HybridMessageSerializerTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EtherZa have a look at a few minor things.
I am also wondering if there is a way we could not require hybrid to be configured as the last (while ensuring backward compatibility with the overall serializer configuration experience).
Looks good overall!
d5ecb88
to
d0e60b1
Compare
I have added a potential option that you may prefer in the issue. |
cbbe35e
to
807368f
Compare
807368f
to
4e7700b
Compare
…Provider Signed-off-by: Richard Pringle <richardpringle@gmail.com>
4e7700b
to
2829d04
Compare
Quality Gate failedFailed conditions See analysis details on SonarCloud Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
@EtherZa thanks for your contribution looks good! If you have a moment would be good to tweak these 4 sonar warnings (in another PR). Else I can look at them in a few days. Cheers. |
Signed-off-by: Richard Pringle <richardpringle@gmail.com>
Fantastic!
#243 fixes the issues. Thanks for the support/ideas in getting it across the line. |
Signed-off-by: Richard Pringle <richardpringle@gmail.com>
#238 PR to allow for serializers to be added and then resolved by the hybrid message serializer.
By resolving the specialized serializers from the DI container, registration of those serializers can be performed via their included builders. This will then allow for the expected dependency resolution path to be followed for each of the specialized serializers ie. JsonMessageSerializer can use a supplied JsonSerializerOptions instance or resolve one from the DI container directly
Caveat:
The HybridMessageSerializer must be registered last so that it is able to remove previous IMessageSerializer registrations in order to make itself the designated serializer.