Skip to content

Commit

Permalink
[Messaging] Allow relative URI property values (#44583)
Browse files Browse the repository at this point in the history
The focus of these changes is to fix a but in the
AMQP converter that assumed all URI described types
represented an absolute URI. If a relative URI was
provided, conversion would fail.
  • Loading branch information
jsquire committed Jun 14, 2024
1 parent 717ba30 commit b9b2c60
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,11 @@ public static bool TryCreateAmqpPropertyValueFromNetProperty(
break;

case AmqpType.Uri:
amqpPropertyValue = new DescribedType((AmqpSymbol)AmqpMessageConstants.Uri, ((Uri)propertyValue).AbsoluteUri);
amqpPropertyValue = new DescribedType((AmqpSymbol)AmqpMessageConstants.Uri, propertyValue switch
{
Uri uriValue when uriValue.IsAbsoluteUri => uriValue.AbsoluteUri,
_ => propertyValue.ToString()
});
break;

case AmqpType.DateTimeOffset:
Expand Down Expand Up @@ -819,7 +823,7 @@ private static void ThrowSerializationFailed(string propertyName, KeyValuePair<s
{
if (symbol.Equals(AmqpMessageConstants.Uri))
{
return new Uri((string)value);
return new Uri((string)value, UriKind.RelativeOrAbsolute);
}

if (symbol.Equals(AmqpMessageConstants.TimeSpan))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.IO;
using System.Linq;
using System.Text;
Expand Down Expand Up @@ -211,6 +212,30 @@ public void ToAmqpMessagePopulatesSimpleApplicationProperties()
}
}

[Test]
[TestCase("http://www.server.com/path/stuff/thing.json")]
[TestCase("/path/stuff/thing.json")]
public void ToAmqpMessageHandlesRelativeAndAbsoluteUris(string uriValue)
{
var key = "UriProperty";
var expectedUri = new Uri(uriValue, UriKind.RelativeOrAbsolute);

var annotatedMessage = new AmqpAnnotatedMessage(AmqpMessageBody.FromData(new ReadOnlyMemory<byte>[] { new byte[] { 0x11, 0x22, 0x33 }}));
annotatedMessage.ApplicationProperties.Add(key, expectedUri);

using AmqpMessage message = ToAmqpMessage(annotatedMessage);

Assert.IsNotNull(message, "The AMQP message should have been created.");
Assert.IsNotNull(message.ApplicationProperties, "The AMQP message should have a set of application properties.");

var containsValue = annotatedMessage.ApplicationProperties.TryGetValue(key, out object value);
var uriProperty = value as Uri;

Assert.IsTrue(containsValue, $"The message properties did not contain the Uri property");
Assert.IsNotNull(uriProperty, "The property value was not a Uri.");
Assert.AreEqual(expectedUri, uriProperty, "The property value did not match.");
}

[Test]
public void FromAmqpMessagePopulatesSimpleApplicationProperties()
{
Expand Down Expand Up @@ -242,6 +267,31 @@ public void FromAmqpMessagePopulatesSimpleApplicationProperties()
}
}

[Test]
[TestCase("http://www.server.com/path/stuff/thing.json")]
[TestCase("/path/stuff/thing.json")]
public void FromAmqpMessageHandlesRelativeAndAbsoluteUris(string uriValue)
{
var key = "UriProperty";
var expectedUri = new Uri(uriValue, UriKind.RelativeOrAbsolute);
var dataBody = new Data { Value = new byte[] { 0x11, 0x22, 0x33 } };

using var message = AmqpMessage.Create(dataBody);
message.ApplicationProperties.Map.Add(key, new DescribedType((AmqpSymbol)AmqpMessageConstants.Uri, uriValue));

var annotatedMessage = FromAmqpMessage(message);

Assert.NotNull(annotatedMessage, "The message should have been created.");
Assert.IsTrue(annotatedMessage.ApplicationProperties.Any(), "The message should have a set of application properties.");

var containsValue = annotatedMessage.ApplicationProperties.TryGetValue(key, out object value);
var uriProperty = value as Uri;

Assert.IsTrue(containsValue, $"The message properties did not contain the Uri property");
Assert.IsNotNull(uriProperty, "The property value was not a Uri.");
Assert.AreEqual(expectedUri, uriProperty, "The property value did not match.");
}

[Test]
[TestCaseSource(nameof(DescribedTypePropertyTestCases))]
public void ToAmqpMessageTranslatesDescribedApplicationProperties(object typeDescriptor, object propertyValueRaw, Func<object, object> propertyValueAccessor)
Expand Down
4 changes: 4 additions & 0 deletions sdk/eventhub/Azure.Messaging.EventHubs.Processor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@

### Bugs Fixed

- Fixed an error that prevented relative URIs from being used with [application properties](https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-application-properties) in the `EventData.Properties` collection.

### Other Changes

- The processor will now refresh the maximum message size each time a new AMQP link is opened; this is necessary for large message support, where the maximum message size for entities can be reconfigured and adjusted on the fly. Because the client had cached the value, it would not be aware of the change and would enforce the wrong size for batch creation.

## 5.12.0-beta.1 (2024-05-17)

### Features Added
Expand Down
4 changes: 3 additions & 1 deletion sdk/eventhub/Azure.Messaging.EventHubs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@

### Bugs Fixed

- Fixed an error that prevented relative URIs from being used with [application properties](https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-application-properties) in the `EventData.Properties` collection.

### Other Changes

- The client will now refresh the maximum message size each time a new AMQP link is opened; this is necessary for large message support, where the maximum message size for entities can be reconfigureed adjusted on the fly. Because the client had cached the value, it would not be aware of the change and would enforce the wrong size for batch creation.
- The client will now refresh the maximum message size each time a new AMQP link is opened; this is necessary for large message support, where the maximum message size for entities can be reconfigured and adjusted on the fly. Because the client had cached the value, it would not be aware of the change and would enforce the wrong size for batch creation.

- The `PluggableCheckpointStoreEventProcessor` will now emit a diagnostic span when a checkpoint is created/updated. While this span is not defined by the Open Telemetry specification, this change aligns diagnostic spans with those emitted by `EventProcessorClient`.

Expand Down
2 changes: 2 additions & 0 deletions sdk/servicebus/Azure.Messaging.ServiceBus/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

- Fixed an error that caused connection strings using host names without a scheme to fail parsing and be considered invalid.

- Fixed an error that prevented relative URIs from being used with [application properties](https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-application-properties) in the `ServiceBusMessage.ApplicationProperties` and `ServiceBusReceivedMessage.ApplicationProperties` collections.

### Other Changes

- The client will now refresh the maximum message size each time a new AMQP link is opened; this is necessary for large message support, where the maximum message size for entities can be reconfigureed adjusted on the fly. Because the client had cached the value, it would not be aware of the change and would enforce the wrong size for batch creation.
Expand Down

0 comments on commit b9b2c60

Please sign in to comment.