-
Notifications
You must be signed in to change notification settings - Fork 649
Analyzer to ensure AddHandler<T> isn't called on a saga type #7559
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
Conversation
| public class OrderPolicy : Saga<OrderPolicyData>, | ||
| IAmStartedByMessages<OrderPlaced>, | ||
| IAmStartedByMessages<OrderBilled>, | ||
| IHandleTimeouts<OrderPlaced> // Should not also use a message as timeout state in real life! |
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.
We have docs that promote this https://docs.particular.net/nservicebus/sagas/timeouts#timeout-state-using-the-incoming-message-as-a-timeout-state
| } | ||
|
|
||
| [Test] | ||
| public void SagaWithInappropriateDoubleMessageMapping() |
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.
Should the test name be "SagaThatIsResusingMessageAsTimeoutState"?
|
|
||
| var handlerFullyQualifiedName = handlerType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); | ||
| return new HandlerSpec(InterceptLocationSpec.From(location), handlerType.Name, handlerFullyQualifiedName, registrations); | ||
| var handlerTypeSpec = HandlerTypeSpec.From(handlerType); |
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.
I like this change
| } | ||
|
|
||
| (string methodName, HandlerSpec handlerSpec) = first; | ||
| HandlerSpec first = group.First(); |
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.
I originally went with avoiding LINQ for efficiency reasons. It seems, though, LINQ is smart enough in this case to see that the enumerable is an array and therefore always accesses the first element so I'm OK leaving it as is
src/NServiceBus.Core.Analyzer/Handlers/AddHandlerInterceptor.cs
Outdated
Show resolved
Hide resolved
|
I would wait with merging until we discussed #7560 |
No description provided.