-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Implement comprehensive JMS ConnectionFactory configuration for Service Bus #47950
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
base: main
Are you sure you want to change the base?
Conversation
…iver ConnectionFactory Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
...azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsContainerConfiguration.java
Show resolved
Hide resolved
The receiver now conditionally uses the ConnectionFactory bean when it's JmsPoolConnectionFactory or CachingConnectionFactory, otherwise creates a dedicated ServiceBusJmsConnectionFactory. This matches the expected behavior where the receiver should use the same pooled/cached connection factory as the sender when those features are enabled. Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
...azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsContainerConfiguration.java
Outdated
Show resolved
Hide resolved
The receiver now only uses the shared ConnectionFactory bean when pooling or caching is EXPLICITLY enabled. When properties are not set, the receiver creates a dedicated ServiceBusJmsConnectionFactory instance, even though the sender defaults to CachingConnectionFactory. This matches the expected behavior table from the issue. Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Table
Explanation
@copilot , According to this table and explanation:
|
- Updated logic to follow the complete configuration table with all 9 scenarios - Added JavaDoc tables to both sender and receiver configurations - Added comprehensive test coverage for all scenarios - Updated CHANGELOG with detailed configuration table and rules - Both properties set to true now uses CachingConnectionFactory (cache takes precedence) - Added proper fallback logic when required classes are not in classpath Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Done in commit 29cb8b3:
Key changes:
|
|
@copilot Fix the lint error: |
Fixed OperatorWrapCheck violations by moving && operators from end of line to the beginning of the next line, following the project's code style rules. Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
...azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsContainerConfiguration.java
Show resolved
Hide resolved
...azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsContainerConfiguration.java
Show resolved
Hide resolved
...azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsContainerConfiguration.java
Show resolved
Hide resolved
|
@copilot apply changes based on the comments in this thread |
1. Fixed CHANGELOG typo: Removed extra comma after "Note:" 2. Added lifecycle management: Implemented DisposableBean to properly close dedicated ConnectionFactory instances on shutdown 3. Memoized all dedicated factory types: Added volatile fields and synchronized getter methods for CachingConnectionFactory, JmsPoolConnectionFactory, and ServiceBusJmsConnectionFactory to prevent duplicate instances 4. Added receiver tests: Added 4 comprehensive tests verifying receiver uses correct ConnectionFactory type in different configuration scenarios This ensures proper resource cleanup and validates receiver behavior. Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Applied all changes in commit 93ee8fa:
All 77 JMS tests passing, checkstyle 0 violations. |
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
...azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsAutoConfigurationTests.java
Show resolved
Hide resolved
|
@copilot apply changes based on the comments in this thread |
1. Fixed CHANGELOG typo: Removed extra comma after "Note:" 2. Enhanced receiver tests to verify actual ConnectionFactory instances: - Create DefaultMessageListenerContainer from factories - Assert ConnectionFactory type using getConnectionFactory() - Verify dedicated instances are NOT same as sender bean - Verify shared beans ARE same as sender bean - Verify both listener containers share same dedicated instance (memoization) Tests now properly validate that receiver uses correct ConnectionFactory type and that memoization prevents duplicate instances. Co-authored-by: rujche <171773178+rujche@users.noreply.github.com>
Applied in commit 0b12dc2. Enhanced all 4 receiver tests to:
Tests now properly validate receiver ConnectionFactory behavior and instance sharing. |
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.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
|
/azp run java - spring - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Plan: Change sender's default JmsConnectionFactory to CachingConnectionFactory
Understanding:
spring.jms.servicebus.pool.enabledandspring.jms.cache.enabledare not set, both sender and receiver useServiceBusJmsConnectionFactoryJmsTemplateas each send creates and closes a connectionCachingConnectionFactoryby default for the sender sideImplementation Strategy:
ServiceBusJmsConnectionFactoryConfiguration.javato implement complete configuration matrixServiceBusJmsContainerConfiguration.javato match all scenariosOriginal prompt
This section details on the original issue you should resolve
<issue_title>[FEATURE REQ] Change sender's default JmsConnectionFactory from ServiceBusJmsConnectionFactory to CachingConnectionFactory</issue_title>
<issue_description>## Current behavior
Expected behavior
Comment from @vinaysurya
Relate codes
azure-sdk-for-java/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsContainerConfiguration.java
Lines 29 to 48 in d9d47b6
azure-sdk-for-java/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/jms/ServiceBusJmsConnectionFactoryConfiguration.java
Lines 59 to 72 in 1ab5874
Comments on the Issue (you are @copilot in this section)
@rujche ## Current statusJmsTemplatebean.JmsTemplatebean will only be configured when there is only one bean with typeConnectionFactory.@ConditionalOnSingleCandidate(ConnectionFactory.class).ConnectionFactory, it's the same one used in spring-boot/../JmsClientConfigurations.javaImplementation suggestion
ConnectionFactory, to make the spring-boot/../JmsClientConfigurations.java can still work.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.