Skip to content

Commit

Permalink
[Messaging] Allow binary application properties (#43181)
Browse files Browse the repository at this point in the history
* [Messaging] Allow binary application properties

The focus of these changes is to update AMQP conversion
to remove the validation that disallows `byte[]` and
`ArraySegment<byte>` values in application properties.
These are confirmed to be encoded as `binary` by
`Microsoft.Azure.Amqp`, making them allowable values
according to section 3.2.5 of the AMQP specification.

The Event Hubs documentation for `EventData` has been
updated to reflect this change.  Due to a known bug
in the Service Bus service, the service will reject
messages with `byte[]` application properties.  Until
this is confirmed fixed, the Service Bus documentation
still reflects `byte[]` as an invalid type for the
application properties.
  • Loading branch information
jsquire authored Apr 4, 2024
1 parent 5b3a404 commit 5f28227
Show file tree
Hide file tree
Showing 11 changed files with 550 additions and 384 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -522,10 +522,14 @@ public static bool TryCreateAmqpPropertyValueFromNetProperty(
amqpPropertyValue = new DescribedType((AmqpSymbol)AmqpMessageConstants.TimeSpan, ((TimeSpan)propertyValue).Ticks);
break;

case AmqpType.Unknown when allowBodyTypes && propertyValue is byte[] byteArray:
case AmqpType.Unknown when propertyValue is byte[] byteArray:
amqpPropertyValue = new ArraySegment<byte>(byteArray);
break;

case AmqpType.Unknown when propertyValue is ArraySegment<byte> byteSegment:
amqpPropertyValue = byteSegment;
break;

case AmqpType.Unknown when allowBodyTypes && propertyValue is IDictionary dict:
amqpPropertyValue = new AmqpMap(dict);
break;
Expand Down Expand Up @@ -811,8 +815,7 @@ private static void ThrowSerializationFailed(string propertyName, KeyValuePair<s
///
/// <returns>The typed value of the symbol, if it belongs to the well-known set; otherwise, <c>null</c>.</returns>
///
private static object? TranslateSymbol(AmqpSymbol symbol,
object value)
private static object? TranslateSymbol(AmqpSymbol symbol, object value)
{
if (symbol.Equals(AmqpMessageConstants.Uri))
{
Expand Down
281 changes: 270 additions & 11 deletions sdk/core/Azure.Core.Amqp/tests/AmqpAnnotatedMessageConverterTests.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Licensed under the MIT License.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace Azure.Messaging.EventHubs.Tests
Expand Down Expand Up @@ -115,7 +117,33 @@ public static bool IsEquivalentTo(this EventData instance,
return false;
}

return instance.Properties.OrderBy(kvp => kvp.Key).SequenceEqual(other.Properties.OrderBy(kvp => kvp.Key));
foreach (var key in instance.Properties.Keys)
{
if (!other.Properties.TryGetValue(key, out object otherValue))
{
return false;
}

// Properties can contain byte[] or ArraySegment<byte> values, which need to be compared
// as a sequence rather than by strict equality. Both forms implement IList<byte>, so they
// can be normalized for comparison.

if ((instance.Properties[key] is IList<byte> instanceList) && (otherValue is IList<byte> otherList))
{
if (!instanceList.SequenceEqual(otherList))
{
return false;
}
}
else if (!instance.Properties[key].Equals(otherValue))
{
return false;
}
}

// No inequalities were found, so the events are equal.

return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,114 @@ public void IsEquivalentToDetectsDifferentProperties()
Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.False);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
/// </summary>
///
[Test]
public void IsEquivalentToComparesByteArrayPropertiesBySequence()
{
var body = new byte[] { 0x22, 0x44, 0x88 };
var firstEvent = new EventData((byte[])body.Clone());
var secondEvent = new EventData((byte[])body.Clone());

firstEvent.Properties["test"] = new byte[] { 0x1, 0x2, 0x3 };
secondEvent.Properties["test"] = new byte[] { 0x1, 0x2, 0x3 };

Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.True);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
/// </summary>
///
[Test]
public void IsEquivalentToDetectsDifferentArrayProperties()
{
var body = new byte[] { 0x22, 0x44, 0x88 };
var firstEvent = new EventData((byte[])body.Clone());
var secondEvent = new EventData((byte[])body.Clone());

firstEvent.Properties["test"] = new byte[] { 0x1, 0x2, 0x3 };
secondEvent.Properties["test"] = new byte[] { 0x2, 0x3, 0x4 };

Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.False);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
/// </summary>
///
[Test]
public void IsEquivalentToComparesArraySegmentPropertiesBySequence()
{
var body = new byte[] { 0x22, 0x44, 0x88 };
var firstEvent = new EventData((byte[])body.Clone());
var secondEvent = new EventData((byte[])body.Clone());

firstEvent.Properties["test"] = new ArraySegment<byte>(new byte[] { 0x1, 0x2, 0x3 });
secondEvent.Properties["test"] = new ArraySegment<byte>(new byte[] { 0x1, 0x2, 0x3 });

Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.True);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
/// </summary>
///
[Test]
public void IsEquivalentToDetectsDifferentArraySegmentProperties()
{
var body = new byte[] { 0x22, 0x44, 0x88 };
var firstEvent = new EventData((byte[])body.Clone());
var secondEvent = new EventData((byte[])body.Clone());

firstEvent.Properties["test"] = new ArraySegment<byte>(new byte[] { 0x1, 0x2, 0x3 });
secondEvent.Properties["test"] = new ArraySegment<byte>(new byte[] { 0x2, 0x3, 0x4 });

Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.False);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
/// </summary>
///
[Test]
public void IsEquivalentToComparesMixedBinaryPropertiesBySequence()
{
var body = new byte[] { 0x22, 0x44, 0x88 };
var firstEvent = new EventData((byte[])body.Clone());
var secondEvent = new EventData((byte[])body.Clone());

firstEvent.Properties["test"] = new ArraySegment<byte>(new byte[] { 0x1, 0x2, 0x3 });
secondEvent.Properties["test"] = new byte[] { 0x1, 0x2, 0x3 };

Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.True);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
/// </summary>
///
[Test]
public void IsEquivalentToDetectsMixedBinaryProperties()
{
var body = new byte[] { 0x22, 0x44, 0x88 };
var firstEvent = new EventData((byte[])body.Clone());
var secondEvent = new EventData((byte[])body.Clone());

firstEvent.Properties["test"] = new byte[] { 0x1, 0x2, 0x3 };
secondEvent.Properties["test"] = new ArraySegment<byte>(new byte[] { 0x2, 0x3, 0x4 });

Assert.That(firstEvent.IsEquivalentTo(secondEvent), Is.False);
}

/// <summary>
/// Verifies functionality of the <see cref="EventDataExtensions.IsEquivalentTo" /> test
/// helper.
Expand Down
2 changes: 2 additions & 0 deletions sdk/eventhub/Azure.Messaging.EventHubs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

### Other Changes

- It is now possible to set `byte[]` values as [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.

## 5.11.1 (2024-03-05)

### Other Changes
Expand Down
Loading

0 comments on commit 5f28227

Please sign in to comment.