shayelkin commented on code in PR #23717:
URL: https://github.com/apache/kafka/pull/23717#discussion_r4277074349
##########
clients/src/test/java/org/apache/kafka/common/message/MessageTest.java:
##########
@@ -858,6 +861,77 @@ public void testUnknownTaggedFields() {
verifyWriteSucceeds((short) 6, createTopics);
}
+ @Test
+ public void testTaggedFieldsWrittenInAscendingTagOrder() {
+ // KIP-482 requires tagged fields in strictly ascending tag order.
+ // Setters for SimpleExampleMessage's known tagged fields, indexed by
tag.
+ List<Consumer<SimpleExampleMessageData>> knownTagSetters = List.of(
+ m -> m.setMyTaggedIntArray(List.of(1)),
+ m -> m.setMyNullableString("a"),
+ m -> m.setMyInt16((short) 1),
+ m -> m.setMyFloat64(1.0),
+ m -> m.setMyString("b"),
+ m -> m.setMyBytes(new byte[] {0x1}),
+ m -> m.setTaggedUuid(new Uuid(1L, 2L)),
+ m -> m.setTaggedLong(1L),
+ m -> m.setMyTaggedStruct(new
SimpleExampleMessageData.TaggedStruct().setStructId("c")),
+ m -> m.setTaggedLongFlexibleVersionSubset(1L));
+ for (int run = 0; run < 100; run++) {
+ long seed = System.nanoTime() + run;
+ Random random = new Random(seed);
+ short version = (short) (1 + random.nextInt(2));
+ // Tags 8 and 9 are only valid from version 2.
+ int numKnownTags = version == 1 ? 8 : knownTagSetters.size();
+ SimpleExampleMessageData message = new SimpleExampleMessageData();
+ List<Integer> expectedTags = new ArrayList<>();
+ for (int tag = 0; tag < numKnownTags; tag++) {
+ if (random.nextBoolean()) {
+ expectedTags.add(tag);
+ }
+ }
+ // Set the known tagged fields in random order, so the written
order cannot come from it.
+ Collections.shuffle(expectedTags, random);
+ for (int tag : expectedTags) {
+ knownTagSetters.get(tag).accept(message);
+ }
+ // Unknown tags are the ones the schema does not define, so they
are above all known tags.
+ // They must be added in ascending order, since
RawTaggedFieldWriter rejects an unsorted list.
Review Comment:
> since RawTaggedFieldWriter rejects an unsorted list.
If we don't have an explicit test for that, it'd be useful to add one as a
follow up PR.
--
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]
