fix(ServiceBus): Replace amqp protocol by sb for connection string#1675
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughServiceBusContainer.GetConnectionString() now constructs the Endpoint URI using the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Testcontainers.ServiceBus/ServiceBusContainer.cs (1)
20-45: Consider extracting shared connection string logic to reduce duplication.Both
GetConnectionString()andGetHttpConnectionString()share identical logic except for the port number. You could extract a private helper method:♻️ Suggested refactor
public string GetConnectionString() { - var properties = new Dictionary<string, string>(); - properties.Add("Endpoint", new UriBuilder("sb", Hostname, GetMappedPublicPort(ServiceBusBuilder.ServiceBusPort)).ToString()); - properties.Add("SharedAccessKeyName", "RootManageSharedAccessKey"); - properties.Add("SharedAccessKey", "SAS_KEY_VALUE"); - properties.Add("UseDevelopmentEmulator", "true"); - return string.Join(";", properties.Select(property => string.Join("=", property.Key, property.Value))); + return BuildConnectionString(ServiceBusBuilder.ServiceBusPort); } public string GetHttpConnectionString() { - var properties = new Dictionary<string, string>(); - properties.Add("Endpoint", new UriBuilder("sb", Hostname, GetMappedPublicPort(ServiceBusBuilder.ServiceBusHttpPort)).ToString()); - properties.Add("SharedAccessKeyName", "RootManageSharedAccessKey"); - properties.Add("SharedAccessKey", "SAS_KEY_VALUE"); - properties.Add("UseDevelopmentEmulator", "true"); - return string.Join(";", properties.Select(property => string.Join("=", property.Key, property.Value))); + return BuildConnectionString(ServiceBusBuilder.ServiceBusHttpPort); } + +private string BuildConnectionString(ushort port) +{ + var properties = new Dictionary<string, string> + { + { "Endpoint", new UriBuilder("sb", Hostname, GetMappedPublicPort(port)).ToString() }, + { "SharedAccessKeyName", "RootManageSharedAccessKey" }, + { "SharedAccessKey", "SAS_KEY_VALUE" }, + { "UseDevelopmentEmulator", "true" } + }; + return string.Join(";", properties.Select(property => string.Join("=", property.Key, property.Value))); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.ServiceBus/ServiceBusContainer.cs` around lines 20 - 45, Both GetConnectionString and GetHttpConnectionString duplicate the same dictionary-building and join logic; extract that into a private helper (e.g., BuildConnectionString) that takes the varying port (use ServiceBusBuilder.ServiceBusPort and ServiceBusBuilder.ServiceBusHttpPort) and returns the joined string. Replace GetConnectionString and GetHttpConnectionString to call this helper with the appropriate port (and keep the same keys "Endpoint","SharedAccessKeyName","SharedAccessKey","UseDevelopmentEmulator"), so only the port differs and duplication is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Testcontainers.ServiceBus/ServiceBusContainer.cs`:
- Around line 20-45: Both GetConnectionString and GetHttpConnectionString
duplicate the same dictionary-building and join logic; extract that into a
private helper (e.g., BuildConnectionString) that takes the varying port (use
ServiceBusBuilder.ServiceBusPort and ServiceBusBuilder.ServiceBusHttpPort) and
returns the joined string. Replace GetConnectionString and
GetHttpConnectionString to call this helper with the appropriate port (and keep
the same keys
"Endpoint","SharedAccessKeyName","SharedAccessKey","UseDevelopmentEmulator"), so
only the port differs and duplication is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3c25386e-6cbc-4b34-abfb-c21625efbc0e
📒 Files selected for processing (1)
src/Testcontainers.ServiceBus/ServiceBusContainer.cs
What does this PR do?
This PR fixes the incorrect amqp protocol used in the connection string for Azure Service Bus, to replace it with the expected sb protocol.
Why is it important?
Endpoint=sb://<namespace>/;SharedAccessKeyName=<keyname>;SharedAccessKey=<key>amqps://<keyname>:<key>@<namespace>.servicebus.windows.net:5671/?verify=verify_noneEndpoint=amqp://127.0.0.1:33036/;SharedAccessKeyName=RootManageSharedAccessKey;SharedAccessKey=SAS_KEY_VALUE;UseDevelopmentEmulator=trueRelated issues
Summary by CodeRabbit