diff --git a/sdk/servicebus/service-bus/CHANGELOG.md b/sdk/servicebus/service-bus/CHANGELOG.md index 1a593c82a6a9..412d1559d4e4 100644 --- a/sdk/servicebus/service-bus/CHANGELOG.md +++ b/sdk/servicebus/service-bus/CHANGELOG.md @@ -6,6 +6,10 @@ These credential types support rotation via their `update` methods and are an alternative to using the `SharedAccessKeyName/SharedAccessKey` or `SharedAccessSignature` properties in a connection string. Resolves [#11891](https://github.com/Azure/azure-sdk-for-js/issues/11891). +### Bug fixes + +- Some of the queue properties such as "forwardTo" and "autoDeleteOnIdle" were not being set as requested through the `ServiceBusAdministrationClient.createQueue` method because of a bug w.r.t the ordering of XML properties. The issue has been fixed in [#14692](https://github.com/Azure/azure-sdk-for-js/pull/14692). + ## 7.0.4 (2021-03-31) ### Bug fixes diff --git a/sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts b/sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts index 216d867363c4..1f5397293b03 100644 --- a/sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts +++ b/sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts @@ -31,6 +31,13 @@ import { */ export function buildQueueOptions(queue: CreateQueueOptions): InternalQueueOptions { return { + // NOTE: this ordering is extremely important. As an example, misordering of the ForwardTo property + // resulted in a customer bug where the Forwarding attributes appeared to be set but the portal was + // not picking up on it. + // + // The authority on this ordering is here: + // https://github.com/Azure/azure-sdk-for-net/blob/8af2dfc32c96ef3e340f9d20013bf588d97ea756/sdk/servicebus/Azure.Messaging.ServiceBus/src/Administration/QueuePropertiesExtensions.cs#L20 + LockDuration: queue.lockDuration, MaxSizeInMegabytes: getStringOrUndefined(queue.maxSizeInMegabytes), RequiresDuplicateDetection: getStringOrUndefined(queue.requiresDuplicateDetection), @@ -42,11 +49,11 @@ export function buildQueueOptions(queue: CreateQueueOptions): InternalQueueOptio EnableBatchedOperations: getStringOrUndefined(queue.enableBatchedOperations), AuthorizationRules: getRawAuthorizationRules(queue.authorizationRules), Status: getStringOrUndefined(queue.status), + ForwardTo: getStringOrUndefined(queue.forwardTo), + UserMetadata: getStringOrUndefined(queue.userMetadata), AutoDeleteOnIdle: getStringOrUndefined(queue.autoDeleteOnIdle), EnablePartitioning: getStringOrUndefined(queue.enablePartitioning), ForwardDeadLetteredMessagesTo: getStringOrUndefined(queue.forwardDeadLetteredMessagesTo), - ForwardTo: getStringOrUndefined(queue.forwardTo), - UserMetadata: getStringOrUndefined(queue.userMetadata), EntityAvailabilityStatus: getStringOrUndefined(queue.availabilityStatus), EnableExpress: getStringOrUndefined(queue.enableExpress) }; diff --git a/sdk/servicebus/service-bus/src/serializers/subscriptionResourceSerializer.ts b/sdk/servicebus/service-bus/src/serializers/subscriptionResourceSerializer.ts index 6c3d6c9badde..fa84808980eb 100644 --- a/sdk/servicebus/service-bus/src/serializers/subscriptionResourceSerializer.ts +++ b/sdk/servicebus/service-bus/src/serializers/subscriptionResourceSerializer.ts @@ -36,6 +36,12 @@ export function buildSubscriptionOptions( subscription: CreateSubscriptionOptions ): InternalSubscriptionOptions { return { + // NOTE: this ordering is extremely important. As an example, misordering of the ForwardTo property + // resulted in a customer bug where the Forwarding attributes appeared to be set but the portal was + // not picking up on it. + // + // The authority on this ordering is here: + // https://github.com/Azure/azure-sdk-for-net/blob/8af2dfc32c96ef3e340f9d20013bf588d97ea756/sdk/servicebus/Azure.Messaging.ServiceBus/src/Administration/SubscriptionPropertiesExtensions.cs#L191 LockDuration: subscription.lockDuration, RequiresSession: getStringOrUndefined(subscription.requiresSession), DefaultMessageTimeToLive: getStringOrUndefined(subscription.defaultMessageTimeToLive), diff --git a/sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts b/sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts index f0816413e8b8..339f844ee07d 100644 --- a/sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts +++ b/sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts @@ -31,6 +31,13 @@ import { */ export function buildTopicOptions(topic: CreateTopicOptions): InternalTopicOptions { return { + // NOTE: this ordering is extremely important. As an example, misordering of the ForwardTo property + // resulted in a customer bug where the Forwarding attributes appeared to be set but the portal was + // not picking up on it. + // + // The authority on this ordering is here: + // https://github.com/Azure/azure-sdk-for-net/blob/8af2dfc32c96ef3e340f9d20013bf588d97ea756/sdk/servicebus/Azure.Messaging.ServiceBus/src/Administration/TopicPropertiesExtensions.cs#L175 + DefaultMessageTimeToLive: topic.defaultMessageTimeToLive, MaxSizeInMegabytes: getStringOrUndefined(topic.maxSizeInMegabytes), RequiresDuplicateDetection: getStringOrUndefined(topic.requiresDuplicateDetection), diff --git a/sdk/servicebus/service-bus/test/public/atomManagement.spec.ts b/sdk/servicebus/service-bus/test/public/atomManagement.spec.ts index 168c9ef0dcfc..76d5327ef3e1 100644 --- a/sdk/servicebus/service-bus/test/public/atomManagement.spec.ts +++ b/sdk/servicebus/service-bus/test/public/atomManagement.spec.ts @@ -17,9 +17,10 @@ import { ServiceBusAdministrationClient, WithResponse } from "../../src"; import { EntityStatus, EntityAvailabilityStatus } from "../../src"; import { EnvVarNames, getEnvVars } from "./utils/envVarUtils"; import { recreateQueue, recreateSubscription, recreateTopic } from "./utils/managementUtils"; -import { EntityNames } from "./utils/testUtils"; +import { EntityNames, TestClientType } from "./utils/testUtils"; import { TestConstants } from "./testConstants"; import { AzureNamedKeyCredential } from "@azure/core-auth"; +import { createServiceBusClientForTests, ServiceBusClientForTests } from "./utils/testutils2"; chai.use(chaiAsPromised); chai.use(chaiExclude); @@ -60,6 +61,83 @@ const newManagementEntity2 = EntityNames.MANAGEMENT_NEW_ENTITY_2; type AccessRights = ("Manage" | "Send" | "Listen")[]; const randomDate = new Date(); +/** + * These tests are just a sanity check that our updates are actually + * _doing_ something. We've run into some bugs where we've done things like + * enabled forwarding only to find that forwarding isn't happening even though + * we can _read_ the attributes back. + */ +describe("Atom management - forwarding", () => { + let serviceBusClient: ServiceBusClientForTests; + + before(() => { + serviceBusClient = createServiceBusClientForTests(); + }); + + after(() => serviceBusClient.test.after()); + + afterEach(async () => { + serviceBusClient.test.afterEach(); + }); + + it("queue: forwarding", async () => { + const willForward = await serviceBusClient.test.createTestEntities( + TestClientType.PartitionedQueue + ); + const willBeForwardedTo = await serviceBusClient.test.createTestEntities( + TestClientType.UnpartitionedQueue + ); + + // make it so all messages from `willForward` are forwarded to `willBeForwardedTo` + const queueProperties = await serviceBusAtomManagementClient.getQueue(willForward.queue!); + queueProperties.forwardTo = willBeForwardedTo.queue!; + await serviceBusAtomManagementClient.updateQueue(queueProperties); + + const receiver = await serviceBusClient.test.createReceiveAndDeleteReceiver(willBeForwardedTo); + const sender = await serviceBusClient.test.createSender(willForward); + + await sender.sendMessages({ + body: "forwarded message with queues!" + }); + + const messages = await receiver.receiveMessages(1); + assert.deepEqual( + [{ body: "forwarded message with queues!" }], + messages.map((m) => ({ body: m.body })) + ); + }); + + it("subscription: forwarding", async () => { + const willForward = await serviceBusClient.test.createTestEntities( + TestClientType.PartitionedSubscription + ); + const willBeForwardedTo = await serviceBusClient.test.createTestEntities( + TestClientType.UnpartitionedQueue + ); + + // make it so all messages from `willForward` are forwarded to `willBeForwardedTo` + const subscriptionProperties = await serviceBusAtomManagementClient.getSubscription( + willForward.topic!, + willForward.subscription! + ); + subscriptionProperties.forwardTo = willBeForwardedTo.queue!; + await serviceBusAtomManagementClient.updateSubscription(subscriptionProperties); + + const receiver = await serviceBusClient.test.createReceiveAndDeleteReceiver(willBeForwardedTo); + const sender = await serviceBusClient.test.createSender(willForward); + + await sender.sendMessages({ + body: "forwarded message with subscriptions!" + }); + + const messages = await receiver.receiveMessages(1); + assert.deepEqual( + [{ body: "forwarded message with subscriptions!" }], + messages.map((m) => ({ body: m.body })) + ); + }); +}); + describe("Atom management - Namespace", function(): void { it("Get namespace properties", async () => { const namespaceProperties = await serviceBusAtomManagementClient.getNamespaceProperties();