Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 28 additions & 2 deletions src/Orleans.Core.Abstractions/IDs/IdSpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,20 @@ private IdSpan(SerializationInfo info, StreamingContext context)
public override bool Equals(object? obj) => obj is IdSpan kind && Equals(kind);

/// <inheritdoc/>
public bool Equals(IdSpan obj) => _value == obj._value || _value.AsSpan().SequenceEqual(obj._value);
public bool Equals(IdSpan obj)
{
if (_value == obj._value)
{
return true;
}

if (_value is null || obj._value is null)
{
return false;
}

return _value.AsSpan().SequenceEqual(obj._value);
}
Comment on lines +97 to +110

Copilot AI Feb 13, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IdSpanCodec currently serializes based on value.AsSpan(). Since AsSpan() returns an empty span for both _value == null and _value.Length == 0, an IdSpan created from "" (empty byte[]) will be serialized the same as default and will deserialize back to default. With the new equality semantics (null != empty), that means serialization no longer round-trips the value. Consider updating IdSpanCodec to encode empty arrays distinctly from null/default (and add a regression test for that).

Copilot uses AI. Check for mistakes.

/// <inheritdoc/>
public override int GetHashCode() => _hashCode;
Expand Down Expand Up @@ -139,7 +152,20 @@ public void GetObjectData(SerializationInfo info, StreamingContext context)
public static byte[]? UnsafeGetArray(IdSpan id) => id._value;

/// <inheritdoc/>
public int CompareTo(IdSpan other) => _value.AsSpan().SequenceCompareTo(other._value.AsSpan());
public int CompareTo(IdSpan other)
{
if (_value is null)
Comment thread
ReubenBond marked this conversation as resolved.
{
return other._value is null ? 0 : -1;
}

if (other._value is null)
{
return 1;
}

return _value.AsSpan().SequenceCompareTo(other._value.AsSpan());
}

/// <summary>
/// Returns a string representation of this instance, decoding the value as UTF8.
Expand Down
123 changes: 123 additions & 0 deletions test/NonSilo.Tests/IdSpanTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
using Orleans.Runtime;
using Xunit;

namespace NonSilo.Tests
{
/// <summary>
/// Tests for IdSpan, a primitive type for identities representing a sequence of bytes.
/// Validates equality, hash code consistency, and proper handling of null vs empty arrays.
/// </summary>
[TestCategory("BVT")]
public class IdSpanTests
{
/// <summary>
/// Tests that IdSpan.Create(string.Empty) and default(IdSpan) are NOT equal.
/// They should have different internal states (empty array vs null) and should not be considered equal.
/// </summary>
[Fact]
public void IdSpan_CreateEmptyString_NotEqualToDefault()
{
IdSpan createdFromEmptyString = IdSpan.Create(string.Empty);
IdSpan defaultIdSpan = default;

Assert.NotEqual(createdFromEmptyString, defaultIdSpan);
Assert.False(createdFromEmptyString.Equals(defaultIdSpan));
Assert.False(createdFromEmptyString == defaultIdSpan);
Assert.True(createdFromEmptyString != defaultIdSpan);
}

/// <summary>
/// Tests that hash codes are consistent with equality.
/// If two IdSpans are equal, they must have the same hash code.
/// </summary>
[Fact]
public void IdSpan_HashCode_ConsistentWithEquality()
{
IdSpan id1 = IdSpan.Create("test");
IdSpan id2 = IdSpan.Create("test");
IdSpan id3 = IdSpan.Create("different");

// Equal objects must have equal hash codes
Assert.Equal(id1, id2);
Assert.Equal(id1.GetHashCode(), id2.GetHashCode());

// Not equal objects may have different hash codes (not required but expected)
Assert.NotEqual(id1, id3);
}

/// <summary>
/// Tests that default IdSpan has expected properties.
/// </summary>
[Fact]
public void IdSpan_Default_HasExpectedProperties()
{
IdSpan defaultIdSpan = default;

Assert.True(defaultIdSpan.IsDefault);
Assert.Equal(0, defaultIdSpan.GetHashCode());
Assert.Equal("", defaultIdSpan.ToString());
}

/// <summary>
/// Tests that IdSpan created from empty string has expected properties.
/// </summary>
[Fact]
public void IdSpan_CreateEmptyString_HasExpectedProperties()
{
IdSpan emptyStringIdSpan = IdSpan.Create(string.Empty);

Assert.True(emptyStringIdSpan.IsDefault);
Assert.Equal("", emptyStringIdSpan.ToString());
// Hash code should be computed from empty byte array, not 0
Assert.NotEqual(0, emptyStringIdSpan.GetHashCode());
Comment on lines +68 to +72

Copilot AI Feb 13, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion bakes in an implementation detail that the stable hash of an empty byte array is non-zero. To make the test resilient to future hash algorithm changes while still validating the contract, consider asserting that IdSpan.Create(string.Empty) has a different hash code than default(IdSpan) (and/or that a.Equals(b) implies equal hash codes), rather than asserting != 0 specifically.

Suggested change
Assert.True(emptyStringIdSpan.IsDefault);
Assert.Equal("", emptyStringIdSpan.ToString());
// Hash code should be computed from empty byte array, not 0
Assert.NotEqual(0, emptyStringIdSpan.GetHashCode());
IdSpan defaultIdSpan = default;
Assert.True(emptyStringIdSpan.IsDefault);
Assert.Equal("", emptyStringIdSpan.ToString());
// Hash code should differ from default(IdSpan) while still representing an "empty" value
Assert.NotEqual(defaultIdSpan.GetHashCode(), emptyStringIdSpan.GetHashCode());

Copilot uses AI. Check for mistakes.
}

/// <summary>
/// Tests that IdSpans with same content are equal.
/// </summary>
[Fact]
public void IdSpan_SameContent_AreEqual()
{
IdSpan id1 = IdSpan.Create("test123");
IdSpan id2 = IdSpan.Create("test123");

Assert.Equal(id1, id2);
Assert.True(id1 == id2);
Assert.False(id1 != id2);
Assert.Equal(id1.GetHashCode(), id2.GetHashCode());
}

/// <summary>
/// Tests that IdSpans with different content are not equal.
/// </summary>
[Fact]
public void IdSpan_DifferentContent_AreNotEqual()
{
IdSpan id1 = IdSpan.Create("test1");
IdSpan id2 = IdSpan.Create("test2");

Assert.NotEqual(id1, id2);
Assert.False(id1 == id2);
Assert.True(id1 != id2);
}

/// <summary>
/// Tests CompareTo behavior with null and empty arrays.
/// </summary>
[Fact]
public void IdSpan_CompareTo_HandlesNullAndEmpty()
{
IdSpan defaultIdSpan = default;
IdSpan emptyStringIdSpan = IdSpan.Create(string.Empty);
IdSpan normalIdSpan = IdSpan.Create("test");

// Default (null) should compare differently than empty string
Assert.NotEqual(0, defaultIdSpan.CompareTo(emptyStringIdSpan));
Assert.NotEqual(0, emptyStringIdSpan.CompareTo(defaultIdSpan));

// Both should be less than a normal span
Assert.True(defaultIdSpan.CompareTo(normalIdSpan) < 0);
Assert.True(emptyStringIdSpan.CompareTo(normalIdSpan) < 0);
}
}
}
Loading