Skip to content

Commit 60bd119

Browse files
ZachBrayZachBray
committed
[Java] Fix remaining DTO issues uncovered with PBT.
The main issue was around determining when a field might not be present. We were checking `token.version() > 0`, which isn't accurate in composites or groups. In this commit, I've also started to use the `Footnotes` API in JQWIK, which allows useful debug information to be captured _only_ for failing attempts. It then prints out the shrunken footnotes upon a failure.
1 parent 75df462 commit 60bd119

File tree

5 files changed

+184
-130
lines changed

5 files changed

+184
-130
lines changed

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtoGenerator.java

Lines changed: 83 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.io.Writer;
3131
import java.util.ArrayList;
3232
import java.util.List;
33+
import java.util.function.Predicate;
3334

3435
import static uk.co.real_logic.sbe.generation.Generators.toLowerFirstChar;
3536
import static uk.co.real_logic.sbe.generation.Generators.toUpperFirstChar;
@@ -43,6 +44,7 @@
4344
*/
4445
public class JavaDtoGenerator implements CodeGenerator
4546
{
47+
private static final Predicate<Token> ALWAYS_FALSE_PREDICATE = ignored -> false;
4648
private static final String INDENT = " ";
4749
private static final String BASE_INDENT = "";
4850

@@ -97,7 +99,7 @@ public void generate() throws IOException
9799
generateVarData(classBuilder, varData, BASE_INDENT + INDENT);
98100

99101
generateDecodeWith(classBuilder, dtoClassName, decoderClassName, fields,
100-
groups, varData, BASE_INDENT + INDENT);
102+
groups, varData, BASE_INDENT + INDENT, fieldToken -> fieldToken.version() > msgToken.version());
101103
generateDecodeFrom(classBuilder, dtoClassName, decoderClassName, BASE_INDENT + INDENT);
102104
generateEncodeWith(classBuilder, dtoClassName, encoderClassName, fields, groups, varData,
103105
BASE_INDENT + INDENT);
@@ -196,6 +198,13 @@ private void generateGroups(
196198
final String groupClassName = formatDtoClassName(groupName);
197199
final String qualifiedDtoClassName = qualifiedParentDtoClassName + "." + groupClassName;
198200

201+
final Token dimToken = tokens.get(i + 1);
202+
if (dimToken.signal() != Signal.BEGIN_COMPOSITE)
203+
{
204+
throw new IllegalStateException("groups must start with BEGIN_COMPOSITE: token=" + dimToken);
205+
}
206+
final int sinceVersion = dimToken.version();
207+
199208
final String fieldName = formatFieldName(groupName);
200209
final String formattedPropertyName = formatPropertyName(groupName);
201210

@@ -226,10 +235,22 @@ private void generateGroups(
226235
i = collectVarData(tokens, i, varData);
227236
generateVarData(groupClassBuilder, varData, indent + INDENT);
228237

238+
final Predicate<Token> wasAddedAfterGroup = token ->
239+
{
240+
final boolean addedAfterParent = token.version() > sinceVersion;
241+
242+
if (addedAfterParent && token.signal() == Signal.BEGIN_VAR_DATA)
243+
{
244+
throw new IllegalStateException("Cannot extend var data inside a group.");
245+
}
246+
247+
return addedAfterParent;
248+
};
249+
229250
generateDecodeListWith(
230251
groupClassBuilder, groupClassName, qualifiedDecoderClassName, indent + INDENT);
231252
generateDecodeWith(groupClassBuilder, groupClassName, qualifiedDecoderClassName,
232-
fields, groups, varData, indent + INDENT);
253+
fields, groups, varData, indent + INDENT, wasAddedAfterGroup);
233254
generateEncodeWith(
234255
groupClassBuilder, groupClassName, qualifiedEncoderClassName, fields, groups, varData, indent + INDENT);
235256
generateComputeEncodedLength(groupClassBuilder, qualifiedDecoderClassName,
@@ -324,8 +345,10 @@ private void generateComputeEncodedLength(
324345
.append(qualifiedDecoderClassName).append(".")
325346
.append(formatPropertyName(propertyName)).append("HeaderLength();\n");
326347

348+
final String characterEncoding = varDataToken.encoding().characterEncoding();
349+
final String lengthAccessor = characterEncoding == null ? ".length" : ".length()";
327350
lengthBuilder.append(indent).append(INDENT).append("encodedLength += ")
328-
.append(fieldName).append(".length()");
351+
.append(fieldName).append(lengthAccessor);
329352

330353
final int elementByteLength = varDataToken.encoding().primitiveType().size();
331354
if (elementByteLength != 1)
@@ -358,7 +381,7 @@ private void generateCompositeDecodeWith(
358381
final Token token = tokens.get(i);
359382

360383
generateFieldDecodeWith(
361-
decodeBuilder, token, token, decoderClassName, indent + INDENT);
384+
decodeBuilder, token, token, decoderClassName, indent + INDENT, ALWAYS_FALSE_PREDICATE);
362385

363386
i += tokens.get(i).componentTokenCount();
364387
}
@@ -426,16 +449,17 @@ private void generateDecodeWith(
426449
final List<Token> fields,
427450
final List<Token> groups,
428451
final List<Token> varData,
429-
final String indent)
452+
final String indent,
453+
final Predicate<Token> wasAddedAfterParent)
430454
{
431455
final StringBuilder decodeBuilder = classBuilder.appendPublic().append("\n")
432456
.append(indent).append("public static void decodeWith(").append(decoderClassName).append(" decoder, ")
433457
.append(dtoClassName).append(" dto)\n")
434458
.append(indent).append("{\n");
435459

436-
generateMessageFieldsDecodeWith(decodeBuilder, fields, decoderClassName, indent + INDENT);
460+
generateMessageFieldsDecodeWith(decodeBuilder, fields, decoderClassName, indent + INDENT, wasAddedAfterParent);
437461
generateGroupsDecodeWith(decodeBuilder, groups, indent + INDENT);
438-
generateVarDataDecodeWith(decodeBuilder, varData, indent + INDENT);
462+
generateVarDataDecodeWith(decodeBuilder, decoderClassName, varData, indent + INDENT, wasAddedAfterParent);
439463
decodeBuilder.append(indent).append("}\n");
440464
}
441465

@@ -466,7 +490,8 @@ private void generateMessageFieldsDecodeWith(
466490
final StringBuilder sb,
467491
final List<Token> tokens,
468492
final String decoderClassName,
469-
final String indent)
493+
final String indent,
494+
final Predicate<Token> wasAddedAfterParent)
470495
{
471496
for (int i = 0, size = tokens.size(); i < size; i++)
472497
{
@@ -475,7 +500,7 @@ private void generateMessageFieldsDecodeWith(
475500
{
476501
final Token encodingToken = tokens.get(i + 1);
477502

478-
generateFieldDecodeWith(sb, signalToken, encodingToken, decoderClassName, indent);
503+
generateFieldDecodeWith(sb, signalToken, encodingToken, decoderClassName, indent, wasAddedAfterParent);
479504
}
480505
}
481506
}
@@ -485,17 +510,18 @@ private void generateFieldDecodeWith(
485510
final Token fieldToken,
486511
final Token typeToken,
487512
final String decoderClassName,
488-
final String indent)
513+
final String indent,
514+
final Predicate<Token> wasAddedAfterParent)
489515
{
490516
switch (typeToken.signal())
491517
{
492518
case ENCODING:
493-
generatePrimitiveDecodeWith(sb, fieldToken, typeToken, decoderClassName, indent);
519+
generatePrimitiveDecodeWith(sb, fieldToken, typeToken, decoderClassName, indent, wasAddedAfterParent);
494520
break;
495521

496522
case BEGIN_SET:
497523
final String bitSetName = formatDtoClassName(typeToken.applicableTypeName());
498-
generateBitSetDecodeWith(sb, fieldToken, bitSetName, indent);
524+
generateBitSetDecodeWith(sb, decoderClassName, fieldToken, bitSetName, indent, wasAddedAfterParent);
499525
break;
500526

501527
case BEGIN_ENUM:
@@ -516,7 +542,8 @@ private void generatePrimitiveDecodeWith(
516542
final Token fieldToken,
517543
final Token typeToken,
518544
final String decoderClassName,
519-
final String indent)
545+
final String indent,
546+
final Predicate<Token> wasAddedAfterParent)
520547
{
521548
if (typeToken.isConstantEncoding())
522549
{
@@ -543,12 +570,13 @@ private void generatePrimitiveDecodeWith(
543570
indent,
544571
"decoder.actingVersion() >= " + decoderClassName + "." + formattedPropertyName + "SinceVersion()",
545572
"decoder." + formattedPropertyName + "()",
546-
decoderNullValue
573+
decoderNullValue,
574+
wasAddedAfterParent
547575
);
548576
}
549577
else if (arrayLength > 1)
550578
{
551-
generateArrayDecodeWith(sb, decoderClassName, fieldToken, typeToken, indent);
579+
generateArrayDecodeWith(sb, decoderClassName, fieldToken, typeToken, indent, wasAddedAfterParent);
552580
}
553581
}
554582

@@ -557,7 +585,8 @@ private void generateArrayDecodeWith(
557585
final String decoderClassName,
558586
final Token fieldToken,
559587
final Token typeToken,
560-
final String indent)
588+
final String indent,
589+
final Predicate<Token> wasAddedAfterParent)
561590
{
562591
if (fieldToken.isConstantEncoding())
563592
{
@@ -566,22 +595,24 @@ private void generateArrayDecodeWith(
566595

567596
final String propertyName = fieldToken.name();
568597
final String formattedPropertyName = formatPropertyName(propertyName);
598+
final PrimitiveType primitiveType = typeToken.encoding().primitiveType();
569599

570-
if (typeToken.encoding().primitiveType() == PrimitiveType.CHAR)
600+
if (primitiveType == PrimitiveType.CHAR)
571601
{
572602
generateRecordPropertyAssignment(
573603
sb,
574604
fieldToken,
575605
indent,
576606
"decoder.actingVersion() >= " + decoderClassName + "." + formattedPropertyName + "SinceVersion()",
577607
"decoder." + formattedPropertyName + "()",
578-
"\"\""
608+
"\"\"",
609+
wasAddedAfterParent
579610
);
580611
}
581612
else
582613
{
583614
final StringBuilder initializerList = new StringBuilder();
584-
final String elementType = javaTypeName(typeToken.encoding().primitiveType());
615+
final String elementType = javaTypeName(primitiveType);
585616
initializerList.append("new ").append(elementType).append("[] { ");
586617
final int arrayLength = typeToken.arrayLength();
587618
for (int i = 0; i < arrayLength; i++)
@@ -598,16 +629,19 @@ private void generateArrayDecodeWith(
598629
indent,
599630
"decoder.actingVersion() >= " + decoderClassName + "." + formattedPropertyName + "SinceVersion()",
600631
initializerList,
601-
null
632+
"new " + elementType + "[0]",
633+
wasAddedAfterParent
602634
);
603635
}
604636
}
605637

606638
private void generateBitSetDecodeWith(
607639
final StringBuilder sb,
640+
final String decoderClassName,
608641
final Token fieldToken,
609642
final String dtoTypeName,
610-
final String indent)
643+
final String indent,
644+
final Predicate<Token> wasAddedAfterParent)
611645
{
612646
if (fieldToken.isConstantEncoding())
613647
{
@@ -617,11 +651,11 @@ private void generateBitSetDecodeWith(
617651
final String propertyName = fieldToken.name();
618652
final String formattedPropertyName = formatPropertyName(propertyName);
619653

620-
if (fieldToken.isOptionalEncoding())
654+
if (wasAddedAfterParent.test(fieldToken))
621655
{
622-
sb.append(indent).append("if (decoder.").append(formattedPropertyName).append("InActingVersion()");
623-
624-
sb.append(")\n")
656+
sb.append(indent).append("if (decoder.actingVersion() >= ")
657+
.append(decoderClassName).append(".")
658+
.append(formattedPropertyName).append("SinceVersion())\n")
625659
.append(indent).append("{\n");
626660

627661
sb.append(indent).append(INDENT).append(dtoTypeName).append(".decodeWith(decoder.")
@@ -710,8 +744,10 @@ private void generateGroupsDecodeWith(
710744

711745
private void generateVarDataDecodeWith(
712746
final StringBuilder sb,
747+
final String decoderClassName,
713748
final List<Token> tokens,
714-
final String indent)
749+
final String indent,
750+
final Predicate<Token> wasAddedAfterParent)
715751
{
716752
for (int i = 0; i < tokens.size(); i++)
717753
{
@@ -723,7 +759,7 @@ private void generateVarDataDecodeWith(
723759
final Token varDataToken = Generators.findFirst("varData", tokens, i);
724760
final String characterEncoding = varDataToken.encoding().characterEncoding();
725761

726-
final boolean isOptional = token.version() > 0;
762+
final boolean isOptional = wasAddedAfterParent.test(token);
727763
final String blockIndent = isOptional ? indent + INDENT : indent;
728764

729765
final String dataVar = toLowerFirstChar(propertyName) + "Data";
@@ -734,7 +770,7 @@ private void generateVarDataDecodeWith(
734770
{
735771
decoderValueExtraction.append(blockIndent).append("byte[] ").append(dataVar)
736772
.append(" = new byte[decoder.").append(formattedPropertyName).append("Length()];\n")
737-
.append(blockIndent).append("decoder.get").append(formattedPropertyName)
773+
.append(blockIndent).append("decoder.get").append(toUpperFirstChar(formattedPropertyName))
738774
.append("(").append(dataVar).append(", 0, decoder.").append(formattedPropertyName)
739775
.append("Length());\n");
740776
}
@@ -746,17 +782,17 @@ private void generateVarDataDecodeWith(
746782

747783
if (isOptional)
748784
{
749-
sb.append(indent).append("if (decoder.").append(formattedPropertyName).append("InActingVersion()");
750-
751-
sb.append(")\n")
785+
sb.append(indent).append("if (decoder.actingVersion() >= ")
786+
.append(decoderClassName).append(".")
787+
.append(formattedPropertyName).append("SinceVersion())\n")
752788
.append(indent).append("{\n");
753789

754790
sb.append(decoderValueExtraction);
755791

756792
sb.append(indent).append(INDENT).append("dto.").append(formattedPropertyName).append("(")
757793
.append(dataVar).append(");\n");
758794

759-
final String nullDtoValue = "\"\"";
795+
final String nullDtoValue = characterEncoding == null ? "new byte[0]" : "\"\"";
760796

761797
sb.append(indent).append("}\n")
762798
.append(indent).append("else\n")
@@ -782,27 +818,25 @@ private void generateRecordPropertyAssignment(
782818
final String indent,
783819
final String presenceExpression,
784820
final CharSequence getExpression,
785-
final String nullDecoderValueOrNull)
821+
final String nullDecoderValue,
822+
final Predicate<Token> wasAddedAfterParent)
786823
{
787824
final String propertyName = token.name();
788825
final String formattedPropertyName = formatPropertyName(propertyName);
789826

790-
if (token.isOptionalEncoding())
827+
if (wasAddedAfterParent.test(token))
791828
{
792-
793829
sb.append(indent).append("if (").append(presenceExpression).append(")\n")
794830
.append(indent).append("{\n");
795831

796832
sb.append(indent).append(INDENT).append("dto.").append(formattedPropertyName).append("(")
797833
.append(getExpression).append(");\n");
798834

799-
final String nullValue = nullDecoderValueOrNull == null ? "null" : nullDecoderValueOrNull;
800-
801835
sb.append(indent).append("}\n")
802836
.append(indent).append("else\n")
803837
.append(indent).append("{\n")
804838
.append(indent).append(INDENT).append("dto.").append(formattedPropertyName).append("(")
805-
.append(nullValue).append(");\n")
839+
.append(nullDecoderValue).append(");\n")
806840
.append(indent).append("}\n");
807841
}
808842
else
@@ -966,19 +1000,16 @@ private void generateArrayEncodeWith(
9661000
final String propertyName = fieldToken.name();
9671001
final String formattedPropertyName = formatPropertyName(propertyName);
9681002

969-
if (typeToken.encoding().primitiveType() == PrimitiveType.CHAR)
1003+
final PrimitiveType primitiveType = typeToken.encoding().primitiveType();
1004+
1005+
if (primitiveType == PrimitiveType.CHAR)
9701006
{
9711007
sb.append(indent).append("encoder.").append(toLowerFirstChar(propertyName)).append("(")
9721008
.append("dto.").append(formattedPropertyName).append("());\n");
9731009
}
974-
else if (typeToken.encoding().primitiveType() == PrimitiveType.UINT8)
975-
{
976-
sb.append(indent).append("encoder.put").append(toUpperFirstChar(propertyName)).append("(")
977-
.append("dto.").append(formattedPropertyName).append("());\n");
978-
}
9791010
else
9801011
{
981-
final String javaTypeName = javaTypeName(typeToken.encoding().primitiveType());
1012+
final String javaTypeName = javaTypeName(primitiveType);
9821013
sb.append(indent).append(javaTypeName).append("[] ").append(formattedPropertyName).append(" = ")
9831014
.append("dto.").append(formattedPropertyName).append("();\n")
9841015
.append(indent).append("for (int i = 0; i < ").append(formattedPropertyName).append(".length; i++)\n")
@@ -1103,7 +1134,9 @@ private void generateVarDataEncodeWith(
11031134
if (characterEncoding == null)
11041135
{
11051136
sb.append(indent).append("encoder.put").append(toUpperFirstChar(propertyName)).append("(")
1106-
.append("dto.").append(formattedPropertyName).append("());\n");
1137+
.append("dto.").append(formattedPropertyName).append("(),")
1138+
.append("0,")
1139+
.append("dto.").append(formattedPropertyName).append("().length);\n");
11071140
}
11081141
else
11091142
{
@@ -1310,7 +1343,9 @@ private void generateArrayProperty(
13101343
final String fieldName = formatFieldName(propertyName);
13111344
final String validateMethod = "validate" + toUpperFirstChar(propertyName);
13121345

1313-
if (typeToken.encoding().primitiveType() == PrimitiveType.CHAR)
1346+
final PrimitiveType primitiveType = typeToken.encoding().primitiveType();
1347+
1348+
if (primitiveType == PrimitiveType.CHAR)
13141349
{
13151350
final CharSequence typeName = "String";
13161351

@@ -1345,7 +1380,7 @@ private void generateArrayProperty(
13451380
}
13461381
else
13471382
{
1348-
final String elementTypeName = javaTypeName(typeToken.encoding().primitiveType());
1383+
final String elementTypeName = javaTypeName(primitiveType);
13491384
final String typeName = elementTypeName + "[]";
13501385

13511386
classBuilder.appendField()

‎sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,6 +1034,10 @@ private void generateGroupDecoderClassHeader(
10341034
.append(indent).append(" {\n")
10351035
.append(indent).append(" return blockLength;\n")
10361036
.append(indent).append(" }\n\n")
1037+
.append(indent).append(" public int actingVersion()\n")
1038+
.append(indent).append(" {\n")
1039+
.append(indent).append(" return parentMessage.actingVersion;\n")
1040+
.append(indent).append(" }\n\n")
10371041
.append(indent).append(" public int count()\n")
10381042
.append(indent).append(" {\n")
10391043
.append(indent).append(" return count;\n")

0 commit comments

Comments
 (0)