Skip to content

Commit 2821be8

Browse files
committed
[Java] Support arbitrary char arrays in PBTs without data after null.
I spotted a failure in the DTO round-trip test locally where the DTO's (and flyweight codec's) String-typed accessor would return `""` if the first character in an array was the `null` character. Therefore, it would not round-trip the "hidden bytes" after the `null`. In this commit, I've added a generation mode that disables the generation of strings with this format. Given the representation of char[N] in DTOs it would not be sensible to support round-tripping these "hidden bytes".
1 parent a1560e5 commit 2821be8

File tree

3 files changed

+106
-48
lines changed

3 files changed

+106
-48
lines changed

sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import uk.co.real_logic.sbe.properties.arbitraries.SbeArbitraries;
3232
import uk.co.real_logic.sbe.properties.utils.InMemoryOutputManager;
3333
import org.agrona.*;
34-
import org.agrona.concurrent.UnsafeBuffer;
3534
import org.agrona.io.DirectBufferInputStream;
3635

3736
import java.io.IOException;
@@ -115,10 +114,7 @@ void javaDtoEncodeShouldBeTheInverseOfDtoDecode(
115114
encodedMessage.buffer(), MessageHeaderDecoder.ENCODED_LENGTH, blockLength, actingVersion);
116115
outputBuffer.setMemory(0, outputBuffer.capacity(), (byte)0);
117116
final int outputLength = (int)encodeWith.invoke(null, dto, outputBuffer, 0);
118-
if (!areEqual(inputBuffer, inputLength, outputBuffer, outputLength))
119-
{
120-
fail("Input and output differ");
121-
}
117+
assertEqual(inputBuffer, inputLength, outputBuffer, outputLength);
122118
}
123119
}
124120
catch (final Throwable throwable)
@@ -319,9 +315,9 @@ private static void execute(
319315
@Provide
320316
Arbitrary<SbeArbitraries.EncodedMessage> encodedMessage()
321317
{
322-
final SbeArbitraries.CharGenerationMode mode =
323-
SbeArbitraries.CharGenerationMode.JSON_PRINTER_COMPATIBLE;
324-
return SbeArbitraries.encodedMessage(mode);
318+
final SbeArbitraries.CharGenerationConfig config =
319+
SbeArbitraries.CharGenerationConfig.jsonPrinterCompatibleAndNullTerminates();
320+
return SbeArbitraries.encodedMessage(config);
325321
}
326322

327323
private static void copyResourceToFile(
@@ -346,13 +342,33 @@ private static void copyResourceToFile(
346342
}
347343
}
348344

349-
private boolean areEqual(
345+
private void assertEqual(
350346
final ExpandableArrayBuffer inputBuffer,
351347
final int inputLength,
352348
final ExpandableArrayBuffer outputBuffer,
353349
final int outputLength)
354350
{
355-
return new UnsafeBuffer(inputBuffer, 0, inputLength).equals(new UnsafeBuffer(outputBuffer, 0, outputLength));
351+
final boolean lengthsDiffer = inputLength != outputLength;
352+
final int minLength = Math.min(inputLength, outputLength);
353+
354+
for (int i = 0; i < minLength; i++)
355+
{
356+
if (inputBuffer.getByte(i) != outputBuffer.getByte(i))
357+
{
358+
throw new AssertionError(
359+
"Input and output differ at byte " + i + ".\n" +
360+
"Input length: " + inputLength + ", Output length: " + outputLength + "\n" +
361+
"Input: " + inputBuffer.getByte(i) + ", Output: " + outputBuffer.getByte(i) +
362+
(lengthsDiffer ? "\nLengths differ." : ""));
363+
}
364+
}
365+
366+
if (lengthsDiffer)
367+
{
368+
throw new AssertionError(
369+
"Input and output differ in length.\n" +
370+
"Input length: " + inputLength + ", Output length: " + outputLength);
371+
}
356372
}
357373

358374
private void addGeneratedSourcesFootnotes(

sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ void shouldGenerateValidJson(@ForAll("encodedMessage") final SbeArbitraries.Enco
4646
@Provide
4747
Arbitrary<SbeArbitraries.EncodedMessage> encodedMessage()
4848
{
49-
final SbeArbitraries.CharGenerationMode mode =
50-
SbeArbitraries.CharGenerationMode.JSON_PRINTER_COMPATIBLE;
51-
return SbeArbitraries.encodedMessage(mode);
49+
final SbeArbitraries.CharGenerationConfig config =
50+
SbeArbitraries.CharGenerationConfig.jsonPrinterCompatibleAndNullTerminates();
51+
return SbeArbitraries.encodedMessage(config);
5252
}
5353
}

sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java

Lines changed: 77 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -114,10 +114,38 @@ private static Arbitrary<EncodedDataTypeSchema> encodedDataTypeSchema()
114114
).as(EncodedDataTypeSchema::new);
115115
}
116116

117-
public enum CharGenerationMode
117+
public static class CharGenerationConfig
118118
{
119-
UNRESTRICTED,
120-
JSON_PRINTER_COMPATIBLE
119+
private final boolean alphaOnly;
120+
private final boolean nullCharTerminatesData;
121+
122+
public CharGenerationConfig(
123+
final boolean alphaOnly,
124+
final boolean nullCharTerminatesData)
125+
{
126+
this.alphaOnly = alphaOnly;
127+
this.nullCharTerminatesData = nullCharTerminatesData;
128+
}
129+
130+
boolean alphaOnly()
131+
{
132+
return alphaOnly;
133+
}
134+
135+
boolean nullCharTerminatesData()
136+
{
137+
return nullCharTerminatesData;
138+
}
139+
140+
public static CharGenerationConfig unrestricted()
141+
{
142+
return new CharGenerationConfig(false, false);
143+
}
144+
145+
public static CharGenerationConfig jsonPrinterCompatibleAndNullTerminates()
146+
{
147+
return new CharGenerationConfig(true, true);
148+
}
121149
}
122150

123151
private static Arbitrary<EnumTypeSchema> enumTypeSchema()
@@ -343,27 +371,24 @@ private static Arbitrary<Encoder> combineArbitraryEncoders(final List<Arbitrary<
343371
}
344372
}
345373

346-
public static CharacterArbitrary chars(final CharGenerationMode mode)
374+
public static CharacterArbitrary chars(final CharGenerationConfig config)
347375
{
348-
switch (mode)
376+
if (config.alphaOnly())
349377
{
350-
case UNRESTRICTED:
351-
return Arbitraries.chars();
352-
353-
case JSON_PRINTER_COMPATIBLE:
354-
return Arbitraries.chars().alpha();
355-
356-
default:
357-
throw new IllegalArgumentException("Unsupported mode: " + mode);
378+
return Arbitraries.chars().alpha();
379+
}
380+
else
381+
{
382+
return Arbitraries.chars();
358383
}
359384
}
360385

361386
private static Arbitrary<Encoder> encodedTypeEncoder(
362387
final Encoding encoding,
363388
final boolean isOptional,
364-
final CharGenerationMode charGenerationMode)
389+
final CharGenerationConfig charGenerationConfig)
365390
{
366-
final Arbitrary<Encoder> inRangeEncoder = encodedTypeEncoder(encoding, charGenerationMode);
391+
final Arbitrary<Encoder> inRangeEncoder = encodedTypeEncoder(encoding, charGenerationConfig);
367392

368393
if (isOptional)
369394
{
@@ -378,7 +403,7 @@ private static Arbitrary<Encoder> encodedTypeEncoder(
378403

379404
private static Arbitrary<Encoder> encodedTypeEncoder(
380405
final Encoding encoding,
381-
final CharGenerationMode charGenerationMode)
406+
final CharGenerationConfig charGenerationConfig)
382407
{
383408
final PrimitiveValue minValue = encoding.applicableMinValue();
384409
final PrimitiveValue maxValue = encoding.applicableMaxValue();
@@ -387,7 +412,7 @@ private static Arbitrary<Encoder> encodedTypeEncoder(
387412
{
388413
case CHAR:
389414
assert minValue.longValue() <= maxValue.longValue();
390-
return chars(charGenerationMode).map(c ->
415+
return chars(charGenerationConfig).map(c ->
391416
(builder, buffer, offset, limit) ->
392417
{
393418
builder.appendLine().append(c).append(" @ ").append(offset)
@@ -558,13 +583,13 @@ private static Arbitrary<Encoder> encodedTypeEncoder(
558583
final int offset,
559584
final Token memberToken,
560585
final Token typeToken,
561-
final CharGenerationMode charGenerationMode)
586+
final CharGenerationConfig charGenerationConfig)
562587
{
563588
final Encoding encoding = typeToken.encoding();
564589
final Arbitrary<Encoder> arbEncoder = encodedTypeEncoder(
565590
encoding,
566591
memberToken.isOptionalEncoding(),
567-
charGenerationMode
592+
charGenerationConfig
568593
);
569594

570595
if (typeToken.arrayLength() == 1)
@@ -577,11 +602,28 @@ private static Arbitrary<Encoder> encodedTypeEncoder(
577602
return arbEncoder.list().ofSize(typeToken.arrayLength())
578603
.map(encoders -> (builder, buffer, bufferOffset, limit) ->
579604
{
605+
boolean hasNullTerminated = false;
606+
580607
for (int i = 0; i < typeToken.arrayLength(); i++)
581608
{
582609
builder.beginScope("[" + i + "]");
583610
final int elementOffset = bufferOffset + offset + i * encoding.primitiveType().size();
584-
encoders.get(i).encode(builder, buffer, elementOffset, limit);
611+
if (hasNullTerminated)
612+
{
613+
buffer.putByte(elementOffset, (byte)0);
614+
}
615+
else
616+
{
617+
encoders.get(i).encode(builder, buffer, elementOffset, limit);
618+
}
619+
620+
if (encoding.primitiveType() == PrimitiveType.CHAR &&
621+
charGenerationConfig.nullCharTerminatesData() &&
622+
buffer.getByte(elementOffset) == 0)
623+
{
624+
hasNullTerminated = true;
625+
}
626+
585627
builder.endScope();
586628
}
587629
});
@@ -786,7 +828,7 @@ private static Arbitrary<Encoder> fieldsEncoder(
786828
final MutableInteger cursor,
787829
final int endIdxInclusive,
788830
final boolean expectFields,
789-
final CharGenerationMode charGenerationMode)
831+
final CharGenerationConfig charGenerationConfig)
790832
{
791833
final List<Arbitrary<Encoder>> encoders = new ArrayList<>();
792834
while (cursor.get() <= endIdxInclusive)
@@ -820,7 +862,7 @@ else if (expectFields)
820862
final int endCompositeTokenCount = 1;
821863
final int lastMemberIdx = nextFieldIdx - endCompositeTokenCount - endFieldTokenCount - 1;
822864
final Arbitrary<Encoder> encoder = fieldsEncoder(
823-
tokens, cursor, lastMemberIdx, false, charGenerationMode);
865+
tokens, cursor, lastMemberIdx, false, charGenerationConfig);
824866
fieldEncoder = encoder.map(e ->
825867
(builder, buffer, bufferOffset, limit) ->
826868
e.encode(builder, buffer, bufferOffset + offset, limit));
@@ -839,7 +881,7 @@ else if (expectFields)
839881
break;
840882

841883
case ENCODING:
842-
fieldEncoder = encodedTypeEncoder(offset, memberToken, typeToken, charGenerationMode);
884+
fieldEncoder = encodedTypeEncoder(offset, memberToken, typeToken, charGenerationConfig);
843885
break;
844886

845887
default:
@@ -868,7 +910,7 @@ private static Arbitrary<Encoder> groupsEncoder(
868910
final List<Token> tokens,
869911
final MutableInteger cursor,
870912
final int endIdxInclusive,
871-
final CharGenerationMode charGenerationMode)
913+
final CharGenerationConfig charGenerationConfig)
872914
{
873915
final List<Arbitrary<Encoder>> encoders = new ArrayList<>();
874916

@@ -894,9 +936,9 @@ private static Arbitrary<Encoder> groupsEncoder(
894936

895937

896938
final Arbitrary<Encoder> groupElement = Combinators.combine(
897-
fieldsEncoder(tokens, cursor, nextFieldIdx - 1, true, charGenerationMode),
898-
groupsEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationMode),
899-
varDataEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationMode)
939+
fieldsEncoder(tokens, cursor, nextFieldIdx - 1, true, charGenerationConfig),
940+
groupsEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationConfig),
941+
varDataEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationConfig)
900942
).as((fieldsEncoder, groupsEncoder, varDataEncoder) ->
901943
(builder, buffer, ignored, limit) ->
902944
{
@@ -946,7 +988,7 @@ private static Arbitrary<Encoder> varDataEncoder(
946988
final List<Token> tokens,
947989
final MutableInteger cursor,
948990
final int endIdxInclusive,
949-
final CharGenerationMode charGenerationMode)
991+
final CharGenerationConfig charGenerationConfig)
950992
{
951993
final List<Arbitrary<Encoder>> encoders = new ArrayList<>();
952994

@@ -969,7 +1011,7 @@ private static Arbitrary<Encoder> varDataEncoder(
9691011
final String characterEncoding = varDataToken.encoding().characterEncoding();
9701012
final Arbitrary<Byte> arbitraryByte = null == characterEncoding ?
9711013
Arbitraries.bytes() :
972-
chars(charGenerationMode).map(c -> (byte)c.charValue());
1014+
chars(charGenerationConfig).map(c -> (byte)c.charValue());
9731015
encoders.add(arbitraryByte.list()
9741016
.ofMaxSize((int)Math.min(lengthToken.encoding().applicableMaxValue().longValue(), 260L))
9751017
.map(bytes -> (builder, buffer, ignored, limit) ->
@@ -1003,7 +1045,7 @@ private static Arbitrary<Encoder> varDataEncoder(
10031045
private static Arbitrary<Encoder> messageValueEncoder(
10041046
final Ir ir,
10051047
final short messageId,
1006-
final CharGenerationMode charGenerationMode)
1048+
final CharGenerationConfig charGenerationConfig)
10071049
{
10081050
final List<Token> tokens = ir.getMessage(messageId);
10091051
final MutableInteger cursor = new MutableInteger(1);
@@ -1015,11 +1057,11 @@ private static Arbitrary<Encoder> messageValueEncoder(
10151057
}
10161058

10171059
final Arbitrary<Encoder> fieldsEncoder = fieldsEncoder(
1018-
tokens, cursor, tokens.size() - 1, true, charGenerationMode);
1060+
tokens, cursor, tokens.size() - 1, true, charGenerationConfig);
10191061
final Arbitrary<Encoder> groupsEncoder = groupsEncoder(
1020-
tokens, cursor, tokens.size() - 1, charGenerationMode);
1062+
tokens, cursor, tokens.size() - 1, charGenerationConfig);
10211063
final Arbitrary<Encoder> varDataEncoder = varDataEncoder(
1022-
tokens, cursor, tokens.size() - 1, charGenerationMode);
1064+
tokens, cursor, tokens.size() - 1, charGenerationConfig);
10231065
return Combinators.combine(fieldsEncoder, groupsEncoder, varDataEncoder)
10241066
.as((fields, groups, varData) -> (builder, buffer, offset, limit) ->
10251067
{
@@ -1087,7 +1129,7 @@ public String encodingLog()
10871129
}
10881130
}
10891131

1090-
public static Arbitrary<EncodedMessage> encodedMessage(final CharGenerationMode mode)
1132+
public static Arbitrary<EncodedMessage> encodedMessage(final CharGenerationConfig charGenerationConfig)
10911133
{
10921134
return SbeArbitraries.messageSchema().flatMap(testSchema ->
10931135
{
@@ -1101,7 +1143,7 @@ public static Arbitrary<EncodedMessage> encodedMessage(final CharGenerationMode
11011143
.build();
11021144
final uk.co.real_logic.sbe.xml.MessageSchema parsedSchema = parse(in, options);
11031145
final Ir ir = new IrGenerator().generate(parsedSchema);
1104-
return SbeArbitraries.messageValueEncoder(ir, testSchema.templateId(), mode)
1146+
return SbeArbitraries.messageValueEncoder(ir, testSchema.templateId(), charGenerationConfig)
11051147
.map(encoder ->
11061148
{
11071149
final EncodingLogger logger = new EncodingLogger();

0 commit comments

Comments
 (0)