spetz commented on code in PR #4072:
URL: https://github.com/apache/iggy/pull/4072#discussion_r3943410814
##########
foreign/csharp/Iggy_SDK/Kinds/Partitioning.cs:
##########
@@ -33,12 +34,24 @@ public readonly struct Partitioning
/// <summary>
/// Length of the partitioning value.
/// </summary>
- public required int Length { get; init; }
+ public int Length => Value.Length;
Review Comment:
Removing `init` breaks existing object initializers that supplied the
previously required `Length`. Recompiling produces `CS0200`, while previously
compiled consumers throw `MissingMethodException` for `set_Length`. Both are
reproduced. Keep the getter derived from `Value`, but preserve a compatible
initializer so this serialization fix does not require downstream source
changes. It also relates to `Identifier`.
##########
foreign/csharp/Iggy_SDK_Tests/UtilityTests/IdentifiersByteSerializationTests.cs:
##########
@@ -30,6 +31,27 @@ public void
StringIdentifier_WithInvalidLength_ShouldThrowArgumentException()
Assert.Throws<ArgumentException>(() => Identifier.String(val));
}
+ [Theory]
+ [InlineData("café", 5)]
+ [InlineData("naïve-café", 12)]
+ [InlineData("日本語", 9)]
+ public void StringIdentifier_WithNonAscii_ShouldUseUtf8ByteLength(string
value, int expectedLength)
Review Comment:
These assertions check identifier properties, while the new contract matrix
uses numeric identifiers. Add exact-byte assertions for
`GetUser(Identifier.String("café"))` and
`UpdateStream(Identifier.String("café"), "topic")`. These cover the original
tight-buffer failure and corruption of the following field.
##########
foreign/csharp/Iggy_SDK/Identifier.cs:
##########
@@ -151,6 +160,9 @@ public override bool Equals(object? obj)
/// <inheritdoc />
public override int GetHashCode()
{
- return HashCode.Combine((int)Kind, Value);
+ var hash = new HashCode();
+ hash.Add(Kind);
+ hash.AddBytes(Value);
Review Comment:
Content-based hashing now depends on a publicly mutable array. After
inserting an identifier into a dictionary, changing `id.Value[0]` makes lookup
of that same identifier fail. Consider protecting the backing bytes from
mutation through both the initializer input and the getter.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
