Fix issue updating UserMetadata in subscriptions.. (Azure#42332)

* Do not set values of readonly properties on the Subscription.

* Adding test case to update properties.

* Update assets folder.

* Update CHANGELOG

sdk/servicebus/azure-messaging-servicebus/CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Bugs Fixed
 
 - Fixes the thread unsafe use of javax.crypto.Mac instance in ServiceBusSharedKeyCredential. ([42353](https://github.com/Azure/azure-sdk-for-java/pull/42353))
+- Fixed issue where `SubscriptionProperties.UserMetadata` was set to `null` when updating its value. ([#42332](https://github.com/Azure/azure-sdk-for-java/pull/42332))
 
 ### Other Changes
 

sdk/servicebus/azure-messaging-servicebus/assets.json

@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "java",
   "TagPrefix": "java/servicebus/azure-messaging-servicebus",
-  "Tag": "java/servicebus/azure-messaging-servicebus_2f19e2f937"
+  "Tag": "java/servicebus/azure-messaging-servicebus_b03e11cf36"
 }

sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/administration/AdministrationModelConverter.java

@@ -159,6 +159,7 @@ CreateSubscriptionBodyImpl getCreateSubscriptionBody(SubscriptionDescriptionImpl
         final CreateSubscriptionBodyContentImpl content = new CreateSubscriptionBodyContentImpl()
             .setType(CONTENT_TYPE)
             .setSubscriptionDescription(subscriptionDescription);
+
         return new CreateSubscriptionBodyImpl().setContent(content);
     }
 
@@ -257,7 +258,19 @@ CreateSubscriptionBodyImpl getUpdateSubscriptionBody(SubscriptionProperties subs
             subscription.setForwardDeadLetteredMessagesTo(forwardDlq);
         }
 
-        return getCreateSubscriptionBody(EntityHelper.toImplementation(subscription));
+        // Set read-only properties on the subscription to null so they are not serialized.  The service will not
+        // properly update fields if it encounters MessageCountDetails in the serialized XML.  Mirrors behaviour in
+        // Track 1 library.
+        final SubscriptionDescriptionImpl implementation = EntityHelper.toImplementation(subscription)
+            .setDefaultMessageTimeToLive(null)
+            .setMessageCount(null)
+            .setCreatedAt(null)
+            .setUpdatedAt(null)
+            .setAccessedAt(null)
+            .setMessageCountDetails(null)
+            .setEntityAvailabilityStatus(null);
+
+        return getCreateSubscriptionBody(implementation);
     }
 
     CreateTopicBodyImpl getUpdateTopicBody(TopicProperties topic) {

sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/administration/ServiceBusAdministrationAsyncClientIntegrationTest.java

@@ -36,6 +36,7 @@
 import com.azure.messaging.servicebus.administration.models.SharedAccessAuthorizationRule;
 import com.azure.messaging.servicebus.administration.models.SqlRuleAction;
 import com.azure.messaging.servicebus.administration.models.SqlRuleFilter;
+import com.azure.messaging.servicebus.administration.models.SubscriptionProperties;
 import com.azure.messaging.servicebus.administration.models.SubscriptionRuntimeProperties;
 import com.azure.messaging.servicebus.administration.models.TopicProperties;
 import com.azure.messaging.servicebus.administration.models.TopicRuntimeProperties;
@@ -64,6 +65,7 @@
 import static com.azure.messaging.servicebus.TestUtils.getTopicBaseName;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
@@ -154,7 +156,7 @@ void azureClientSecretCredential(HttpClient httpClient) {
             .verify(DEFAULT_TIMEOUT);
     }
 
-    //region Create tests
+    //region Create Entity tests
 
     @ParameterizedTest
     @MethodSource("createHttpClients")
@@ -536,7 +538,7 @@ void createTopicExistingName(HttpClient httpClient) {
 
     //endregion
 
-    //region Delete tests
+    //region Delete Entity tests
 
     @ParameterizedTest
     @MethodSource("createHttpClients")
@@ -664,7 +666,7 @@ void deleteTopicDoesNotExist(HttpClient httpClient) {
 
     //endregion
 
-    //region Get and exists tests
+    //region Get & Exists Entity tests
 
     @ParameterizedTest
     @MethodSource("createHttpClients")
@@ -1061,7 +1063,7 @@ void getSubscriptionRuntimePropertiesUnauthorizedClient(HttpClient httpClient) {
 
     //endregion
 
-    //region List tests
+    //region List Entity tests
 
     @ParameterizedTest
     @MethodSource("createHttpClients")
@@ -1145,6 +1147,8 @@ void listTopics(HttpClient httpClient) {
 
     //endregion
 
+    //region Update Entity tests
+
     @ParameterizedTest
     @MethodSource("createHttpClients")
     void updateRuleResponse(HttpClient httpClient) {
@@ -1180,6 +1184,81 @@ void updateRuleResponse(HttpClient httpClient) {
             .verify(DEFAULT_TIMEOUT);
     }
 
+    @ParameterizedTest
+    @MethodSource("createHttpClients")
+    void updateSubscriptionWithRule(HttpClient httpClient) {
+        // Arrange
+        final String userMetadata = "some-metadata-for-testing-subscriptions";
+        final String updatedUserMetadata = "updated-metadata: 1728929824";
+        final Duration updatedAutoDeleteOnIdle = Duration.ofDays(6);
+
+        final ServiceBusAdministrationAsyncClient client = createClient(httpClient);
+        final String topicName = getEntityName(getTopicBaseName(), 0);
+        final String subscriptionName = testResourceNamer.randomName("sub", 10);
+        final CreateSubscriptionOptions subscriptionOptions = new CreateSubscriptionOptions()
+            .setMaxDeliveryCount(7)
+            .setLockDuration(Duration.ofSeconds(45))
+            .setUserMetadata(userMetadata);
+
+        final String ruleName = testResourceNamer.randomName("rule", 10);
+        final SqlRuleFilter ruleFilter = new SqlRuleFilter("color='red'");
+        final SqlRuleAction ruleAction = new SqlRuleAction("SET MessageId = 'is-red'");
+        final CreateRuleOptions ruleOptions = new CreateRuleOptions(ruleFilter)
+            .setAction(ruleAction);
+
+        final SubscriptionProperties createdSubscription = client.createSubscription(topicName, subscriptionName,
+            ruleName, subscriptionOptions, ruleOptions).block(DEFAULT_TIMEOUT);
+
+        // Assert created options are correct.
+        assertNotNull(createdSubscription);
+        assertEquals(userMetadata, createdSubscription.getUserMetadata());
+
+        final SubscriptionProperties existing = client.getSubscription(topicName, subscriptionName)
+            .block(DEFAULT_TIMEOUT);
+
+        assertNotNull(existing);
+
+        // Updated existing properties.
+        existing.setUserMetadata(updatedUserMetadata)
+            .setAutoDeleteOnIdle(updatedAutoDeleteOnIdle);
+
+        // Act & Assert
+        StepVerifier.create(client.updateSubscription(existing))
+                .assertNext(contents -> {
+                    assertAreEquals(existing, contents);
+                })
+                .expectComplete()
+                .verify(DEFAULT_TIMEOUT);
+
+        StepVerifier.create(client.getSubscription(topicName, subscriptionName))
+            .assertNext(contents -> {
+                assertAreEquals(existing, contents);
+            })
+            .expectComplete()
+            .verify(DEFAULT_TIMEOUT);
+
+        StepVerifier.create(client.getRule(topicName, subscriptionName, ruleName))
+            .assertNext(contents -> {
+                assertEquals(ruleName, contents.getName());
+
+                assertNotNull(contents.getFilter());
+                assertInstanceOf(SqlRuleFilter.class, contents.getFilter());
+
+                final SqlRuleFilter actualFilter = (SqlRuleFilter) contents.getFilter();
+                assertEquals(ruleFilter.getSqlExpression(), actualFilter.getSqlExpression());
+
+                assertNotNull(contents.getAction());
+                assertInstanceOf(SqlRuleAction.class, contents.getAction());
+
+                final SqlRuleAction actualAction = (SqlRuleAction) contents.getAction();
+                assertEquals(ruleAction.getSqlExpression(), actualAction.getSqlExpression());
+            })
+            .expectComplete()
+            .verify(DEFAULT_TIMEOUT);
+    }
+
+    //endregion
+
     private ServiceBusAdministrationAsyncClient createClient(HttpClient httpClient) {
         final ServiceBusAdministrationClientBuilder builder = new ServiceBusAdministrationClientBuilder()
             .httpLogOptions(new HttpLogOptions().setLogLevel(HttpLogDetailLevel.BODY_AND_HEADERS));
@@ -1219,4 +1298,22 @@ static void configure(ServiceBusAdministrationClientBuilder builder,
             interceptorManager.addMatchers(TEST_PROXY_REQUEST_MATCHERS);
         }
     }
+
+    private static void assertAreEquals(SubscriptionProperties expected, SubscriptionProperties actual) {
+        assertEquals(expected.getLockDuration(), actual.getLockDuration());
+        assertEquals(expected.isSessionRequired(), actual.isSessionRequired());
+
+        assertEquals(expected.isDeadLetteringOnMessageExpiration(), actual.isDeadLetteringOnMessageExpiration());
+        assertEquals(expected.isDeadLetteringOnFilterEvaluationExceptions(),
+            actual.isDeadLetteringOnFilterEvaluationExceptions());
+
+        assertEquals(expected.getMaxDeliveryCount(), actual.getMaxDeliveryCount());
+        assertEquals(expected.isBatchedOperationsEnabled(), actual.isBatchedOperationsEnabled());
+
+        assertEquals(expected.getUserMetadata(), actual.getUserMetadata());
+
+        assertEquals(expected.getForwardTo(), actual.getForwardTo());
+        assertEquals(expected.getForwardDeadLetteredMessagesTo(), actual.getForwardDeadLetteredMessagesTo());
+        assertEquals(expected.getAutoDeleteOnIdle(), actual.getAutoDeleteOnIdle());
+    }
 }