Expose underlying storage session #6529
Conversation
…specific session) Co-authored-by: Daniel Marbach <daniel.marbach@particular.net>
lailabougria
requested review from
danielmarbach,
tmasternak and
timbussmann
| @@ -102,7 +102,7 @@ public static IIncomingLogicalMessageContext CreateIncomingLogicalMessageContext | |||
| /// <summary> | |||
| /// Creates a <see cref="IInvokeHandlerContext" /> based on the current context. | |||
| /// </summary> | |||
| public static IInvokeHandlerContext CreateInvokeHandlerContext(this StageConnector<IIncomingLogicalMessageContext, IInvokeHandlerContext> stageConnector, MessageHandler messageHandler, CompletableSynchronizedStorageSession storageSession, IIncomingLogicalMessageContext sourceContext) | |||
| public static IInvokeHandlerContext CreateInvokeHandlerContext(this StageConnector<IIncomingLogicalMessageContext, IInvokeHandlerContext> stageConnector, MessageHandler messageHandler, SynchronizedStorageSession storageSession, IIncomingLogicalMessageContext sourceContext) | |||
There was a problem hiding this comment.
V8 has it still on the other interface. Should we change that there too? https://github.com/Particular/NServiceBus/blob/master/src/NServiceBus.Core/Pipeline/ConnectorContextExtensions.cs#L106
There was a problem hiding this comment.
Yes, if we align on this change, we definitely should.
|
I'm not really sure I understand how this addresses/replaces #6526, can you elaborate? |
|
Well, the weirdness was based on the fact that you have: public static CompletableSynchronizedStorageSession GetAdaptedSession(this CompletableSynchronizedStorageSession session) =>
((CompletableSynchronizedStorageSessionAdapter)session).AdaptedSession;which is basically a method accepting a CompletableSynchronizedStorageSession which again returns a CompletableSynchronizedStorageSession, which looks off. I do agree that it looks off because the API doesn't convey that it's being unwrapped. However, if we change the signature, that's not true anymore, and it better conveys that it's being unwrapped. Or do you have other concerns? |
|
For me this would be the perfect middle ground between exposing the adapter as suggested in the other PR and leaving status quo. The only thing that makes me a bit hesitant is that we would have to change the extension method on v8 too (which has already shipped as RC2, that being said I'd be happy to go this way) |
I don't think this really addresses the issue: |
|
Closing in favour of #6526 |
Alternative to #6526
Although this does in theory break SemVer, it's a very low degree of impact.