Making a fix to compare sessionid on the message and the sessionid on the link filter in a case-insensitive way.#48759
Conversation
…in a case insensite way.
yvgopal
requested review from
a team,
EldertGrootenboer and
skarri-microsoft
as code owners
There was a problem hiding this comment.
Pull request overview
Updates Service Bus session ID matching to be case-insensitive (Service Bus session IDs are not case-sensitive) to avoid disposition/link lookup failures when the broker returns a different session ID casing than the message carries.
Changes:
- Normalize session ID keys (lowercased with
Locale.ROOT) inSessionsMessagePumpandServiceBusSessionManagerlookup maps. - Switch
ServiceBusSingleSessionManagersession ID comparisons toequalsIgnoreCase. - Add/extend tests to cover case-differing session ID scenarios for message settlement and single-session manager behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/SessionsMessagePump.java | Normalizes tracked session receiver keys and disposition lookups to be case-insensitive. |
| sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSingleSessionManager.java | Uses case-insensitive session ID comparison for link name/disposition routing. |
| sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusSessionManager.java | Normalizes session receiver map keys and lookups for case-insensitive session ID handling. |
| sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/SessionsMessagePumpIsolatedTest.java | Adds a test ensuring settlement succeeds when accepted vs message session ID casing differs. |
| sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusSingleSessionManagerTest.java | Adds new unit tests for case-insensitive link name lookup and disposition update. |
| import reactor.test.StepVerifier; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; |
There was a problem hiding this comment.
Unused static import assertNotNull is never referenced in this test class and will fail compilation (and Checkstyle UnusedImports). Remove the unused import or add an assertion that uses it.
| import static org.junit.jupiter.api.Assertions.assertNotNull; |
| private Flux<ServiceBusMessageContext> getSession(Scheduler scheduler, boolean disposeOnIdle) { | ||
| return getActiveLink().flatMap( | ||
| link -> link.getSessionId().map(sessionId -> sessionReceivers.compute(sessionId, (key, existing) -> { | ||
| link -> link.getSessionId().map(sessionId -> sessionReceivers.compute(sessionId.toLowerCase(Locale.ROOT), (key, existing) -> { |
There was a problem hiding this comment.
This line exceeds the 120-character Checkstyle limit (eng/lintingconfigs/checkstyle/track2/checkstyle.xml enforces max=120) and is likely to fail CI. Please wrap the nested lambdas/compute call across multiple lines for readability and to satisfy LineLength.
| public String getLinkName(String sessionId) { | ||
| final ServiceBusSessionReceiver receiver = sessionReceivers.get(sessionId); | ||
| final ServiceBusSessionReceiver receiver = sessionReceivers.get(sessionId.toLowerCase(Locale.ROOT)); | ||
| return receiver != null ? receiver.getLinkName() : null; |
There was a problem hiding this comment.
ServiceBusSessionManager now normalizes sessionId keys using toLowerCase(Locale.ROOT) for case-insensitive lookups, but there’s no unit test in ServiceBusSessionManagerTest asserting that getLinkName(...)/updateDisposition(...) work when the caller passes a differently-cased session ID than the stored/accepted one. Adding a focused test would prevent regressions in this behavior.
|
@copilot apply changes based on the comments in this thread |
Service Bus sessionid is not case sensitive. We shouldn't be doing case sensitive comparison between sessionId property on the message and sessionid property on the link.