[Mocha] Fix message identity collisions for nested types - #10466
Merged
Merged
Conversation
Message identities and the send queue and publish topic names derived from message types ignored declaring types, so two nested types with the same name in the same namespace (for example CreateAccountResponse.UnexpectedError and DeleteAccountResponse.UnexpectedError) got the same identity and the bus failed to start with an ArgumentException. Nested types are now prefixed with their declaring types at any depth, for example urn:message:ns:create-account-response.unexpected-error, send queue create-account-response.unexpected-error and publish topic ns.create-account-response.unexpected-error. Generic arguments and generic declaring types are formatted recursively. Names of non-nested types are unchanged. Handler, consumer and saga names are unchanged. A duplicate message identity now fails with an error that names both CLR types and the identity.
Contributor
Patch coverage88.1% of changed lines covered (37/42)
Uncovered changed lines (JSON){
"sha": "d3a13ce43fae6a3ba0fdd36a1a6fdb7da87856a0",
"files": [
{ "path": "src/Mocha/src/Mocha/ThrowHelper.cs", "ranges": [[35, 38]] },
{ "path": "src/Mocha/src/Mocha/Serialization/MessageTypeRegistry.cs", "ranges": [[73, 73]] }
]
}Project coverage: 57.8% (299448/518177 lines) |
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The change intentionally alters wire identities and broker entity names, requiring human confirmation of compatibility expectations.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes collisions between nested message types by incorporating declaring types into message identities and endpoint names.
Changes:
- Adds nested and generic-nested naming support.
- Rejects duplicate message identities with a clear error.
- Adds behavioral, naming, topology, and snapshot coverage.
| File | Description |
|---|---|
src/Mocha/src/Mocha/Naming/DefaultNamingConventions.cs |
Includes declaring types in generated names. |
src/Mocha/src/Mocha/Serialization/MessageTypeRegistry.cs |
Detects duplicate identities. |
src/Mocha/src/Mocha/ThrowHelper.cs |
Centralizes new registry exceptions. |
src/Mocha/test/Mocha.Tests/Conventions/NamingTestTypes.cs |
Adds nested naming fixtures. |
src/Mocha/test/Mocha.Tests/Conventions/DefaultNamingConventionsTests.cs |
Moves non-nested fixtures to preserve coverage. |
src/Mocha/test/Mocha.Tests/Conventions/DefaultNamingConventionsNestedTypeTests.cs |
Tests nested and generic naming. |
src/Mocha/test/Mocha.Tests/MessageTypes/MessageBusConfigurationValidationTests.cs |
Tests duplicate identity diagnostics. |
src/Mocha/test/Mocha.Tests/MessageTypes/__snapshots__/MessageBusConfigurationValidationTests.Build_Should_ReportNoTransportHint_When_HandlerRegisteredWithoutTransport.snap |
Updates generated identities. |
src/Mocha/test/Mocha.Tests/MessageTypes/__snapshots__/MessageBusConfigurationValidationTests.Build_Should_ReportAlsoBoundElsewhere_When_SameMessageHasAnotherBoundRoute.snap |
Updates generated identities. |
src/Mocha/test/Mocha.Tests/MessageTypes/__snapshots__/MessageBusConfigurationValidationTests.Build_Should_ReportAllUnboundInboundRoutes_When_ExplicitBindLeavesHandlersUnbound.snap |
Updates generated identities. |
src/Mocha/test/Mocha.Tests/MessageTypes/__snapshots__/MessageBusConfigurationValidationTests.Build_Should_NotUseReplyRouteAsDuplicate_When_NormalRouteWithSameMessageIsUnbound.snap |
Updates generated identities. |
src/Mocha/test/Mocha.Transport.InMemory.Tests/InMemoryTransportTests.cs |
Moves a fixture out of a declaring type. |
src/Mocha/test/Mocha.Transport.InMemory.Tests/Behaviors/NestedMessageTypeTests.cs |
Verifies nested-type dispatch and requests. |
src/Mocha/test/Mocha.Transport.InMemory.Tests/Behaviors/__snapshots__/NestedMessageTypeTests.Topology_Should_UseDistinctTopicsAndQueues_When_NestedTypesShareName.snap |
Captures distinct nested topology. |
src/Mocha/test/Mocha.Transport.InMemory.Tests/Topology/__snapshots__/InMemoryTopologyConventionTests.Topology_Should_OmitConventionBinding_When_SagaHasOnReplyTransition.snap |
Updates in-memory topology names. |
src/Mocha/test/Mocha.Transport.RabbitMQ.Tests/Topology/__snapshots__/RabbitMQExplicitTopologyTests.Describe_Should_OmitConventionChain_When_SagaHasOnReplyTransition.snap |
Updates RabbitMQ topology names. |
src/Mocha/test/Mocha.Transport.RabbitMQ.Tests/Routing/__snapshots__/RabbitMQReceiveTopologyConventionTests.DiscoverTopology_Should_OmitConventionChain_When_RouteIsReply.snap |
Updates RabbitMQ routing names. |
src/Mocha/test/Mocha.Transport.RabbitMQ.Tests/Descriptors/__snapshots__/RabbitMQUnifiedQueueTests.SagaEndpoint_Should_Describe_When_ConfiguredViaUnifiedQueue.snap |
Updates unified-queue names. |
src/Mocha/test/Mocha.Transport.Postgres.Tests/Conventions/__snapshots__/PostgresReceiveEndpointTopologyConventionTests.DiscoverTopology_Should_OmitConventionChain_When_RouteIsReply.snap |
Updates Postgres topic names. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
With explicit binding, Handler<T>() named the receive endpoint after the handler type, while senders on the InMemory and Azure Service Bus transports target a queue named after the message. Requests only arrived when the two names happened to match, which no longer holds for nested message types now that their send endpoint names include declaring types. On these transports, Handler<T>() now names the endpoint after the send endpoint of the handler's message when the handler has a single send or request route, the same name implicit binding uses. Subscribe, batch and multi-route handlers keep the handler-derived name. Postgres and RabbitMQ are unchanged.
…ints GetHandlerEndpointName now resolves the handler's consumer and uses the router's per-consumer index instead of copying and scanning all inbound routes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CreateAccountResponse.UnexpectedErrorandDeleteAccountResponse.UnexpectedErrorboth becameurn:message:ns:unexpected-error. Nested types now include their declaring types at any depth, including inside generics (urn:message:ns:create-account-response.unexpected-error).