Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/spring/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This section includes changes in `spring-cloud-azure-autoconfigure` module.

- Added `AzureServiceBusJmsConnectionFactoryFactory` to allow applications to customize how `ServiceBusJmsConnectionFactory` instances are created, including support for custom subclasses ([#49676](https://github.com/Azure/azure-sdk-for-java/pull/49676)).
- Added support for constructing `AadB2cAuthorizationRequestResolver` with a custom `authorizationRequestBaseUri`, aligning Azure AD B2C authorization request resolution with the configurability already available for AAD. ([#49674](https://github.com/Azure/azure-sdk-for-java/pull/49674))
- Added a new Service Bus `inherit-configuration` property (`spring.cloud.azure.servicebus[.producer|.consumer|.processor].inherit-configuration`). It defaults to `true`, preserving the existing behavior. Set it to `false` to prevent the sender, receiver or processor sub-client builder from overwriting configuration already set on the shared `ServiceBusClientBuilder` (for example the `ClientOptions` carrying `TracingOptions` set through an `AzureServiceClientBuilderCustomizer<ServiceBusClientBuilder>`) ([#49742](https://github.com/Azure/azure-sdk-for-java/issues/49742)).
Comment thread
Copilot marked this conversation as resolved.
Outdated

#### Breaking Changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ public class AzureMessagingListenerAutoConfiguration {
@Configuration
@ConditionalOnBean(EventHubsProcessorFactory.class)
static class EventHubsConfiguration {
@Bean(name = "azureEventHubsListenerContainerFactory")
@ConditionalOnMissingBean(name = "azureEventHubsListenerContainerFactory")
@Bean(name = EventHubsListenerAnnotationBeanPostProcessor.DEFAULT_EVENT_HUBS_LISTENER_CONTAINER_FACTORY_BEAN_NAME)
@ConditionalOnMissingBean(name = EventHubsListenerAnnotationBeanPostProcessor.DEFAULT_EVENT_HUBS_LISTENER_CONTAINER_FACTORY_BEAN_NAME)
Comment thread
moarychan marked this conversation as resolved.
Outdated
public MessageListenerContainerFactory<? extends MessageListenerContainer> azureEventHubsListenerContainerFactory(
EventHubsProcessorFactory eventHubsProcessorFactory,
ObjectProvider<AzureMessageConverter<EventData, EventData>> messageConverterProvider) {
Expand All @@ -73,8 +73,8 @@ static class EnableEventHubsConfiguration {
@Configuration
@ConditionalOnBean(ServiceBusProcessorFactory.class)
static class ServiceBusConfiguration {
@Bean(name = "azureServiceBusListenerContainerFactory")
@ConditionalOnMissingBean(name = "azureServiceBusListenerContainerFactory")
@Bean(name = ServiceBusListenerAnnotationBeanPostProcessor.DEFAULT_SERVICE_BUS_LISTENER_CONTAINER_FACTORY_BEAN_NAME)
@ConditionalOnMissingBean(name = ServiceBusListenerAnnotationBeanPostProcessor.DEFAULT_SERVICE_BUS_LISTENER_CONTAINER_FACTORY_BEAN_NAME)
Comment thread
moarychan marked this conversation as resolved.
Outdated
public MessageListenerContainerFactory<? extends MessageListenerContainer> azureServiceBusListenerContainerFactory(
ServiceBusProcessorFactory serviceBusProcessorFactory,
ObjectProvider<AzureMessageConverter<ServiceBusReceivedMessage, ServiceBusMessage>> messageConverterProvider) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ public abstract class AzureServiceBusCommonProperties extends AbstractAzureAmqpC
*/
private String customEndpointAddress;

/**
* Whether the shared ServiceBusClientBuilder should inherit the configuration derived from the sender, receiver or
* processor properties. Defaults to true (existing behavior). Set to false to preserve configuration already set on
* the shared builder, for example the ClientOptions carrying TracingOptions set through an
* AzureServiceClientBuilderCustomizer.
Comment thread
Copilot marked this conversation as resolved.
Outdated
*/
private Boolean inheritConfiguration;

private String extractFqdnFromConnectionString() {
if (this.connectionString == null) {
return null;
Expand Down Expand Up @@ -107,4 +115,12 @@ public String getCustomEndpointAddress() {
public void setCustomEndpointAddress(String customEndpointAddress) {
this.customEndpointAddress = customEndpointAddress;
}

public Boolean getInheritConfiguration() {
return inheritConfiguration;
}

public void setInheritConfiguration(Boolean inheritConfiguration) {
this.inheritConfiguration = inheritConfiguration;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,15 @@ public Producer buildProducerProperties() {
propertyMapper.from(this.getConnectionString()).to(properties::setConnectionString);
propertyMapper.from(this.getEntityName()).to(properties::setEntityName);
propertyMapper.from(this.getEntityType()).to(properties::setEntityType);
propertyMapper.from(this.getInheritConfiguration()).to(properties::setInheritConfiguration);

propertyMapper.from(this.producer.getDomainName()).to(properties::setDomainName);
propertyMapper.from(this.producer.getCustomEndpointAddress()).to(properties::setCustomEndpointAddress);
propertyMapper.from(this.producer.getNamespace()).to(properties::setNamespace);
propertyMapper.from(this.producer.getConnectionString()).to(properties::setConnectionString);
propertyMapper.from(this.producer.getEntityType()).to(properties::setEntityType);
propertyMapper.from(this.producer.getEntityName()).to(properties::setEntityName);
propertyMapper.from(this.producer.getInheritConfiguration()).to(properties::setInheritConfiguration);

return properties;
}
Expand All @@ -93,13 +95,15 @@ public Consumer buildConsumerProperties() {
propertyMapper.from(this.getConnectionString()).to(properties::setConnectionString);
propertyMapper.from(this.getEntityName()).to(properties::setEntityName);
propertyMapper.from(this.getEntityType()).to(properties::setEntityType);
propertyMapper.from(this.getInheritConfiguration()).to(properties::setInheritConfiguration);

propertyMapper.from(this.consumer.getDomainName()).to(properties::setDomainName);
propertyMapper.from(this.consumer.getCustomEndpointAddress()).to(properties::setCustomEndpointAddress);
propertyMapper.from(this.consumer.getNamespace()).to(properties::setNamespace);
propertyMapper.from(this.consumer.getConnectionString()).to(properties::setConnectionString);
propertyMapper.from(this.consumer.getEntityType()).to(properties::setEntityType);
propertyMapper.from(this.consumer.getEntityName()).to(properties::setEntityName);
propertyMapper.from(this.consumer.getInheritConfiguration()).to(properties::setInheritConfiguration);

propertyMapper.from(this.consumer.getSessionEnabled()).to(properties::setSessionEnabled);
propertyMapper.from(this.consumer.getAutoComplete()).to(properties::setAutoComplete);
Expand All @@ -125,13 +129,15 @@ public Processor buildProcessorProperties() {
propertyMapper.from(this.getConnectionString()).to(properties::setConnectionString);
propertyMapper.from(this.getEntityName()).to(properties::setEntityName);
propertyMapper.from(this.getEntityType()).to(properties::setEntityType);
propertyMapper.from(this.getInheritConfiguration()).to(properties::setInheritConfiguration);

propertyMapper.from(this.processor.getDomainName()).to(properties::setDomainName);
propertyMapper.from(this.processor.getCustomEndpointAddress()).to(properties::setCustomEndpointAddress);
propertyMapper.from(this.processor.getNamespace()).to(properties::setNamespace);
propertyMapper.from(this.processor.getConnectionString()).to(properties::setConnectionString);
propertyMapper.from(this.processor.getEntityType()).to(properties::setEntityType);
propertyMapper.from(this.processor.getEntityName()).to(properties::setEntityName);
propertyMapper.from(this.processor.getInheritConfiguration()).to(properties::setInheritConfiguration);

propertyMapper.from(this.processor.getSessionEnabled()).to(properties::setSessionEnabled);
propertyMapper.from(this.processor.getAutoComplete()).to(properties::setAutoComplete);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,30 @@ void childrenWillInheritParent() {
assertEquals(customEndpoint, processor.getCustomEndpointAddress());
}

@Test
void inheritConfigurationDefaultsToNull() {
AzureServiceBusProperties serviceBusProperties = new AzureServiceBusProperties();
assertNull(serviceBusProperties.getInheritConfiguration());
assertNull(serviceBusProperties.buildProducerProperties().getInheritConfiguration());
assertNull(serviceBusProperties.buildConsumerProperties().getInheritConfiguration());
assertNull(serviceBusProperties.buildProcessorProperties().getInheritConfiguration());
}

@Test
void inheritConfigurationIsInheritedByChildrenAndOverridableByChild() {
AzureServiceBusProperties serviceBusProperties = new AzureServiceBusProperties();
serviceBusProperties.setInheritConfiguration(true);

// Children inherit the namespace-level inherit-configuration value.
assertEquals(Boolean.TRUE, serviceBusProperties.buildProducerProperties().getInheritConfiguration());
assertEquals(Boolean.TRUE, serviceBusProperties.buildConsumerProperties().getInheritConfiguration());
assertEquals(Boolean.TRUE, serviceBusProperties.buildProcessorProperties().getInheritConfiguration());

// A child-level value takes precedence over the namespace-level value.
serviceBusProperties.getProcessor().setInheritConfiguration(false);
assertEquals(Boolean.FALSE, serviceBusProperties.buildProcessorProperties().getInheritConfiguration());
}

@Test
void buildProducerPropertiesUseParent() {
AzureServiceBusProperties serviceBusProperties = new AzureServiceBusProperties();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,22 @@ protected boolean isShareServiceBusClientBuilder() {
return shareServiceBusClientBuilder;
}

private boolean isInheritConfiguration() {
// Not set defaults to true, preserving the existing behavior.
return this.properties.getInheritConfiguration() == null || this.properties.getInheritConfiguration();
}

// In the non-shared path the sub-client builder applies its property-derived configuration to the underlying
// ServiceBusClientBuilder when inherit-configuration is true (the default). Set it to false to preserve configuration
// already set on the shared builder (e.g. ClientOptions carrying TracingOptions from a customizer). See #49742.
Comment thread
Copilot marked this conversation as resolved.
Outdated
private boolean shouldConfigureServiceBusClientBuilder() {
return !isShareServiceBusClientBuilder() && isInheritConfiguration();
}

@Override
protected BiConsumer<T, ProxyOptions> consumeProxyOptions() {
return (builder, proxy) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().proxyOptions(proxy);
}
};
Expand All @@ -121,7 +133,7 @@ protected BiConsumer<T, ProxyOptions> consumeProxyOptions() {
@Override
protected BiConsumer<T, AmqpTransportType> consumeAmqpTransportType() {
return (builder, t) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().transportType(t);
}
};
Expand All @@ -130,7 +142,7 @@ protected BiConsumer<T, AmqpTransportType> consumeAmqpTransportType() {
@Override
protected BiConsumer<T, AmqpRetryOptions> consumeAmqpRetryOptions() {
return (builder, retry) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().retryOptions(retry);
}
};
Expand All @@ -139,7 +151,7 @@ protected BiConsumer<T, AmqpRetryOptions> consumeAmqpRetryOptions() {
@Override
protected BiConsumer<T, ClientOptions> consumeClientOptions() {
return (builder, client) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().clientOptions(client);
}
};
Expand All @@ -154,17 +166,17 @@ protected AzureProperties getAzureProperties() {
protected List<AuthenticationDescriptor<?>> getAuthenticationDescriptors(T builder) {
return Arrays.asList(
new NamedKeyAuthenticationDescriptor(credential -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().credential(credential);
}
}),
new SasAuthenticationDescriptor(credential -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().credential(credential);
}
}),
new TokenAuthenticationDescriptor(this.tokenCredentialResolver, credential -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().credential(credential);
}
})
Expand All @@ -174,7 +186,7 @@ protected List<AuthenticationDescriptor<?>> getAuthenticationDescriptors(T build
@Override
protected BiConsumer<T, Configuration> consumeConfiguration() {
return (builder, configuration) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().configuration(configuration);
}
};
Expand All @@ -183,7 +195,7 @@ protected BiConsumer<T, Configuration> consumeConfiguration() {
@Override
protected BiConsumer<T, TokenCredential> consumeDefaultTokenCredential() {
return (builder, credential) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().credential(credential);
}
};
Expand All @@ -192,15 +204,15 @@ protected BiConsumer<T, TokenCredential> consumeDefaultTokenCredential() {
@Override
protected BiConsumer<T, String> consumeConnectionString() {
return (builder, connectionString) -> {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().connectionString(connectionString);
}
};
}

@Override
protected void configureService(T builder) {
if (!isShareServiceBusClientBuilder()) {
if (shouldConfigureServiceBusClientBuilder()) {
getServiceBusClientBuilder().fullyQualifiedNamespace(properties.getFullyQualifiedNamespace());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* Helper class to control the lifecycle of a {@link ServiceBusProcessorClient}.
* This class implements {@code SmartLifecycle} to auto start {@link ServiceBusProcessorClient} when the Spring Application Context starts,
* And stop the {@link ServiceBusProcessorClient} when the Spring Application Context stops.
* NOTE, there is not need to call {@link #start()} or {@link #stop()} explicitly, as the {@link ServiceBusProcessorClient} will be started and stopped automatically.
* NOTE, there is no need to call {@link #start()} or {@link #stop()} explicitly, as the {@link ServiceBusProcessorClient} will be started and stopped automatically.
* And since the {@link ServiceBusProcessorClient} is a {@link AutoCloseable}, there is no need to call {@link ServiceBusProcessorClient#close()} explicitly.
Comment thread
moarychan marked this conversation as resolved.
Outdated
*/
public class ServiceBusProcessorClientLifecycleManager implements SmartLifecycle {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,13 @@ public interface ServiceBusClientCommonProperties extends AzureProperties, Retry
* @return the custom endpoint address.
*/
String getCustomEndpointAddress();

/**
* Whether the shared {@code ServiceBusClientBuilder} should inherit the configuration derived from this sub-client's
* properties. When {@code null} (the default) it is treated as {@code true}, preserving the existing behavior. Set to
* {@code false} to keep configuration already present on the shared builder, for example {@code ClientOptions}
* carrying {@code TracingOptions} set through an {@code AzureServiceClientBuilderCustomizer}.
Comment thread
Copilot marked this conversation as resolved.
Outdated
* @return whether the shared {@code ServiceBusClientBuilder} inherits this sub-client's property configuration.
*/
Boolean getInheritConfiguration();
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -135,6 +136,44 @@ void connectionStringConfigured(boolean isShareServiceClientBuilder) {
verify(factory.getServiceBusClientBuilder(), times(1)).connectionString(connectionString);
}

@Test
void inheritConfigurationFalseSkipsConfiguringServiceBusClientBuilder() {
P properties = createMinimalServiceProperties();
properties.setShareServiceBusClientBuilder(false);
properties.setInheritConfiguration(false);
final F factory = createClientBuilderFactoryWithMockBuilder(properties);
doReturn(false).when(factory).isShareServiceBusClientBuilder();
ServiceBusClientBuilder rootBuilder = mock(ServiceBusClientBuilder.class);
doReturn(rootBuilder).when(factory).getServiceBusClientBuilder();

B builder = factory.build();
buildClient(builder);

// With inherit-configuration=false, the sub-client builder factory must not overwrite the shared builder, so
// that a customizer-configured builder (e.g. ClientOptions carrying TracingOptions) is preserved. See #49742.
verify(rootBuilder, times(0)).fullyQualifiedNamespace(any());
verify(rootBuilder, times(0)).clientOptions(any());
verify(rootBuilder, times(0)).proxyOptions(any());
verify(rootBuilder, times(0)).retryOptions(any());
verify(rootBuilder, times(0)).transportType(any());
}

@Test
void inheritConfigurationNotSetConfiguresServiceBusClientBuilder() {
P properties = createMinimalServiceProperties();
properties.setShareServiceBusClientBuilder(false);
final F factory = createClientBuilderFactoryWithMockBuilder(properties);
doReturn(false).when(factory).isShareServiceBusClientBuilder();
ServiceBusClientBuilder rootBuilder = mock(ServiceBusClientBuilder.class);
doReturn(rootBuilder).when(factory).getServiceBusClientBuilder();

B builder = factory.build();
buildClient(builder);

// By default (unset, treated as true) the behavior is unchanged: the shared builder is configured.
verify(rootBuilder, atLeast(1)).fullyQualifiedNamespace(properties.getFullyQualifiedNamespace());
}

private void verifyFqdnConfigured(boolean isShareServiceClientBuilder) {
P properties = createMinimalServiceProperties();
properties.setShareServiceBusClientBuilder(isShareServiceClientBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class ServiceBusClientCommonTestProperties extends AzureAmqpSdkProperties
private String customEndpointAddress;

private boolean shareServiceBusClientBuilder;
private Boolean inheritConfiguration;

private String extractFqdnFromConnectionString() {
if (this.connectionString == null) {
Expand Down Expand Up @@ -93,4 +94,13 @@ public boolean isShareServiceBusClientBuilder() {
public void setShareServiceBusClientBuilder(boolean shareServiceBusClientBuilder) {
this.shareServiceBusClientBuilder = shareServiceBusClientBuilder;
}

@Override
public Boolean getInheritConfiguration() {
return inheritConfiguration;
}

public void setInheritConfiguration(Boolean inheritConfiguration) {
this.inheritConfiguration = inheritConfiguration;
}
}
Loading
Loading