Skip to content

Commit a1560e5

Browse files
committed
[Java] Cover null values in PBTs.
We had a recent bug report where the generated Java DTOs were not correctly handling null values for optional fields. It turns out the property-based tests were not encoding these values. Now, they do, and they fail with messages like this one: ``` java.lang.IllegalArgumentException: member1: value is out of allowed range: 255 at uk.co.real_logic.sbe.properties.TestMessageDto.validateMember1(TestMessageDto.java:33) at uk.co.real_logic.sbe.properties.TestMessageDto.member1(TestMessageDto.java:55) at uk.co.real_logic.sbe.properties.TestMessageDto.decodeWith(TestMessageDto.java:112) at uk.co.real_logic.sbe.properties.TestMessageDto.decodeFrom(TestMessageDto.java:135) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at uk.co.real_logic.sbe.properties.DtosPropertyTest.javaDtoEncodeShouldBeTheInverseOfDtoDecode(DtosPropertyTest.java:114) at java.base/java.lang.reflect.Method.invoke(Method.java:568) ```
1 parent 13f9636 commit a1560e5

File tree

1 file changed

+106
-4
lines changed

1 file changed

+106
-4
lines changed

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

Lines changed: 106 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,24 @@ public static CharacterArbitrary chars(final CharGenerationMode mode)
358358
}
359359
}
360360

361+
private static Arbitrary<Encoder> encodedTypeEncoder(
362+
final Encoding encoding,
363+
final boolean isOptional,
364+
final CharGenerationMode charGenerationMode)
365+
{
366+
final Arbitrary<Encoder> inRangeEncoder = encodedTypeEncoder(encoding, charGenerationMode);
367+
368+
if (isOptional)
369+
{
370+
final Arbitrary<Encoder> nullEncoder = nullEncoder(encoding);
371+
return Arbitraries.oneOf(inRangeEncoder, nullEncoder);
372+
}
373+
else
374+
{
375+
return inRangeEncoder;
376+
}
377+
}
378+
361379
private static Arbitrary<Encoder> encodedTypeEncoder(
362380
final Encoding encoding,
363381
final CharGenerationMode charGenerationMode)
@@ -458,13 +476,96 @@ private static Arbitrary<Encoder> encodedTypeEncoder(
458476
}
459477
}
460478

479+
private static Arbitrary<Encoder> nullEncoder(final Encoding encoding)
480+
{
481+
final PrimitiveValue nullValue = encoding.applicableNullValue();
482+
483+
switch (encoding.primitiveType())
484+
{
485+
case CHAR:
486+
return Arbitraries.just((char)nullValue.longValue()).map(c ->
487+
(builder, buffer, offset, limit) ->
488+
{
489+
builder.appendLine().append(c).append(" @ ").append(offset)
490+
.append("[").append(BitUtil.SIZE_OF_BYTE).append("]");
491+
buffer.putChar(offset, c, encoding.byteOrder());
492+
});
493+
494+
case UINT8:
495+
case INT8:
496+
return Arbitraries.just((short)nullValue.longValue())
497+
.map(b -> (builder, buffer, offset, limit) ->
498+
{
499+
builder.appendLine().append((byte)(short)b).append(" @ ").append(offset)
500+
.append("[").append(BitUtil.SIZE_OF_BYTE).append("]");
501+
buffer.putByte(offset, (byte)(short)b);
502+
});
503+
504+
case UINT16:
505+
case INT16:
506+
return Arbitraries.just((int)nullValue.longValue())
507+
.map(s -> (builder, buffer, offset, limit) ->
508+
{
509+
builder.appendLine().append((short)(int)s).append(" @ ").append(offset)
510+
.append("[").append(BitUtil.SIZE_OF_SHORT).append("]");
511+
buffer.putShort(offset, (short)(int)s, encoding.byteOrder());
512+
});
513+
514+
case UINT32:
515+
case INT32:
516+
return Arbitraries.just(nullValue.longValue())
517+
.map(i -> (builder, buffer, offset, limit) ->
518+
{
519+
builder.appendLine().append((int)(long)i).append(" @ ").append(offset)
520+
.append("[").append(BitUtil.SIZE_OF_INT).append("]");
521+
buffer.putInt(offset, (int)(long)i, encoding.byteOrder());
522+
});
523+
524+
case UINT64:
525+
case INT64:
526+
return Arbitraries.just(nullValue.longValue())
527+
.map(l -> (builder, buffer, offset, limit) ->
528+
{
529+
builder.appendLine().append(l).append(" @ ").append(offset)
530+
.append("[").append(BitUtil.SIZE_OF_LONG).append("]");
531+
buffer.putLong(offset, l, encoding.byteOrder());
532+
});
533+
534+
case FLOAT:
535+
return Arbitraries.just((float)nullValue.doubleValue())
536+
.map(f -> (builder, buffer, offset, limit) ->
537+
{
538+
builder.appendLine().append(f).append(" @ ").append(offset)
539+
.append("[").append(BitUtil.SIZE_OF_FLOAT).append("]");
540+
buffer.putFloat(offset, f, encoding.byteOrder());
541+
});
542+
543+
case DOUBLE:
544+
return Arbitraries.just(nullValue.doubleValue())
545+
.map(d -> (builder, buffer, offset, limit) ->
546+
{
547+
builder.appendLine().append(d).append(" @ ").append(offset)
548+
.append("[").append(BitUtil.SIZE_OF_DOUBLE).append("]");
549+
buffer.putDouble(offset, d, encoding.byteOrder());
550+
});
551+
552+
default:
553+
throw new IllegalArgumentException("Unsupported type: " + encoding.primitiveType());
554+
}
555+
}
556+
461557
private static Arbitrary<Encoder> encodedTypeEncoder(
462558
final int offset,
559+
final Token memberToken,
463560
final Token typeToken,
464561
final CharGenerationMode charGenerationMode)
465562
{
466563
final Encoding encoding = typeToken.encoding();
467-
final Arbitrary<Encoder> arbEncoder = encodedTypeEncoder(encoding, charGenerationMode);
564+
final Arbitrary<Encoder> arbEncoder = encodedTypeEncoder(
565+
encoding,
566+
memberToken.isOptionalEncoding(),
567+
charGenerationMode
568+
);
468569

469570
if (typeToken.arrayLength() == 1)
470571
{
@@ -548,6 +649,7 @@ private static Encoder integerValueEncoder(final Encoding encoding, final long v
548649
private static Arbitrary<Encoder> enumEncoder(
549650
final int offset,
550651
final List<Token> tokens,
652+
final Token memberToken,
551653
final Token typeToken,
552654
final MutableInteger cursor,
553655
final int endIdxInclusive)
@@ -569,7 +671,7 @@ private static Arbitrary<Encoder> enumEncoder(
569671
encoders.add(caseEncoder);
570672
}
571673

572-
if (encoders.isEmpty())
674+
if (memberToken.isOptionalEncoding() || encoders.isEmpty())
573675
{
574676
final Encoder nullEncoder = integerValueEncoder(
575677
typeToken.encoding(),
@@ -727,7 +829,7 @@ else if (expectFields)
727829
case BEGIN_ENUM:
728830
final int endEnumTokenCount = 1;
729831
final int lastValidValueIdx = nextFieldIdx - endFieldTokenCount - endEnumTokenCount - 1;
730-
fieldEncoder = enumEncoder(offset, tokens, typeToken, cursor, lastValidValueIdx);
832+
fieldEncoder = enumEncoder(offset, tokens, memberToken, typeToken, cursor, lastValidValueIdx);
731833
break;
732834

733835
case BEGIN_SET:
@@ -737,7 +839,7 @@ else if (expectFields)
737839
break;
738840

739841
case ENCODING:
740-
fieldEncoder = encodedTypeEncoder(offset, typeToken, charGenerationMode);
842+
fieldEncoder = encodedTypeEncoder(offset, memberToken, typeToken, charGenerationMode);
741843
break;
742844

743845
default:

0 commit comments

Comments
 (0)