From a1560e5e3b8b903383ce98e95421e2d1f9b6fa59 Mon Sep 17 00:00:00 2001 From: Zach Bray Date: Mon, 16 Jun 2025 12:30:03 +0100 Subject: [PATCH 1/4] [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) ``` --- .../arbitraries/SbeArbitraries.java | 110 +++++++++++++++++- 1 file changed, 106 insertions(+), 4 deletions(-) diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java index c1dc232ee3..72a70ee0b0 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java @@ -358,6 +358,24 @@ public static CharacterArbitrary chars(final CharGenerationMode mode) } } + private static Arbitrary encodedTypeEncoder( + final Encoding encoding, + final boolean isOptional, + final CharGenerationMode charGenerationMode) + { + final Arbitrary inRangeEncoder = encodedTypeEncoder(encoding, charGenerationMode); + + if (isOptional) + { + final Arbitrary nullEncoder = nullEncoder(encoding); + return Arbitraries.oneOf(inRangeEncoder, nullEncoder); + } + else + { + return inRangeEncoder; + } + } + private static Arbitrary encodedTypeEncoder( final Encoding encoding, final CharGenerationMode charGenerationMode) @@ -458,13 +476,96 @@ private static Arbitrary encodedTypeEncoder( } } + private static Arbitrary nullEncoder(final Encoding encoding) + { + final PrimitiveValue nullValue = encoding.applicableNullValue(); + + switch (encoding.primitiveType()) + { + case CHAR: + return Arbitraries.just((char)nullValue.longValue()).map(c -> + (builder, buffer, offset, limit) -> + { + builder.appendLine().append(c).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_BYTE).append("]"); + buffer.putChar(offset, c, encoding.byteOrder()); + }); + + case UINT8: + case INT8: + return Arbitraries.just((short)nullValue.longValue()) + .map(b -> (builder, buffer, offset, limit) -> + { + builder.appendLine().append((byte)(short)b).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_BYTE).append("]"); + buffer.putByte(offset, (byte)(short)b); + }); + + case UINT16: + case INT16: + return Arbitraries.just((int)nullValue.longValue()) + .map(s -> (builder, buffer, offset, limit) -> + { + builder.appendLine().append((short)(int)s).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_SHORT).append("]"); + buffer.putShort(offset, (short)(int)s, encoding.byteOrder()); + }); + + case UINT32: + case INT32: + return Arbitraries.just(nullValue.longValue()) + .map(i -> (builder, buffer, offset, limit) -> + { + builder.appendLine().append((int)(long)i).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_INT).append("]"); + buffer.putInt(offset, (int)(long)i, encoding.byteOrder()); + }); + + case UINT64: + case INT64: + return Arbitraries.just(nullValue.longValue()) + .map(l -> (builder, buffer, offset, limit) -> + { + builder.appendLine().append(l).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_LONG).append("]"); + buffer.putLong(offset, l, encoding.byteOrder()); + }); + + case FLOAT: + return Arbitraries.just((float)nullValue.doubleValue()) + .map(f -> (builder, buffer, offset, limit) -> + { + builder.appendLine().append(f).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_FLOAT).append("]"); + buffer.putFloat(offset, f, encoding.byteOrder()); + }); + + case DOUBLE: + return Arbitraries.just(nullValue.doubleValue()) + .map(d -> (builder, buffer, offset, limit) -> + { + builder.appendLine().append(d).append(" @ ").append(offset) + .append("[").append(BitUtil.SIZE_OF_DOUBLE).append("]"); + buffer.putDouble(offset, d, encoding.byteOrder()); + }); + + default: + throw new IllegalArgumentException("Unsupported type: " + encoding.primitiveType()); + } + } + private static Arbitrary encodedTypeEncoder( final int offset, + final Token memberToken, final Token typeToken, final CharGenerationMode charGenerationMode) { final Encoding encoding = typeToken.encoding(); - final Arbitrary arbEncoder = encodedTypeEncoder(encoding, charGenerationMode); + final Arbitrary arbEncoder = encodedTypeEncoder( + encoding, + memberToken.isOptionalEncoding(), + charGenerationMode + ); if (typeToken.arrayLength() == 1) { @@ -548,6 +649,7 @@ private static Encoder integerValueEncoder(final Encoding encoding, final long v private static Arbitrary enumEncoder( final int offset, final List tokens, + final Token memberToken, final Token typeToken, final MutableInteger cursor, final int endIdxInclusive) @@ -569,7 +671,7 @@ private static Arbitrary enumEncoder( encoders.add(caseEncoder); } - if (encoders.isEmpty()) + if (memberToken.isOptionalEncoding() || encoders.isEmpty()) { final Encoder nullEncoder = integerValueEncoder( typeToken.encoding(), @@ -727,7 +829,7 @@ else if (expectFields) case BEGIN_ENUM: final int endEnumTokenCount = 1; final int lastValidValueIdx = nextFieldIdx - endFieldTokenCount - endEnumTokenCount - 1; - fieldEncoder = enumEncoder(offset, tokens, typeToken, cursor, lastValidValueIdx); + fieldEncoder = enumEncoder(offset, tokens, memberToken, typeToken, cursor, lastValidValueIdx); break; case BEGIN_SET: @@ -737,7 +839,7 @@ else if (expectFields) break; case ENCODING: - fieldEncoder = encodedTypeEncoder(offset, typeToken, charGenerationMode); + fieldEncoder = encodedTypeEncoder(offset, memberToken, typeToken, charGenerationMode); break; default: From 2821be8197ac99dbdef305b9d55d33fb9f32b2a8 Mon Sep 17 00:00:00 2001 From: Zach Bray Date: Tue, 17 Jun 2025 17:24:29 +0100 Subject: [PATCH 2/4] [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". --- .../sbe/properties/DtosPropertyTest.java | 36 ++++-- .../sbe/properties/JsonPropertyTest.java | 6 +- .../arbitraries/SbeArbitraries.java | 112 ++++++++++++------ 3 files changed, 106 insertions(+), 48 deletions(-) diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java index 04e1d8fdae..c7399b4ef3 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java @@ -31,7 +31,6 @@ import uk.co.real_logic.sbe.properties.arbitraries.SbeArbitraries; import uk.co.real_logic.sbe.properties.utils.InMemoryOutputManager; import org.agrona.*; -import org.agrona.concurrent.UnsafeBuffer; import org.agrona.io.DirectBufferInputStream; import java.io.IOException; @@ -115,10 +114,7 @@ void javaDtoEncodeShouldBeTheInverseOfDtoDecode( encodedMessage.buffer(), MessageHeaderDecoder.ENCODED_LENGTH, blockLength, actingVersion); outputBuffer.setMemory(0, outputBuffer.capacity(), (byte)0); final int outputLength = (int)encodeWith.invoke(null, dto, outputBuffer, 0); - if (!areEqual(inputBuffer, inputLength, outputBuffer, outputLength)) - { - fail("Input and output differ"); - } + assertEqual(inputBuffer, inputLength, outputBuffer, outputLength); } } catch (final Throwable throwable) @@ -319,9 +315,9 @@ private static void execute( @Provide Arbitrary encodedMessage() { - final SbeArbitraries.CharGenerationMode mode = - SbeArbitraries.CharGenerationMode.JSON_PRINTER_COMPATIBLE; - return SbeArbitraries.encodedMessage(mode); + final SbeArbitraries.CharGenerationConfig config = + SbeArbitraries.CharGenerationConfig.jsonPrinterCompatibleAndNullTerminates(); + return SbeArbitraries.encodedMessage(config); } private static void copyResourceToFile( @@ -346,13 +342,33 @@ private static void copyResourceToFile( } } - private boolean areEqual( + private void assertEqual( final ExpandableArrayBuffer inputBuffer, final int inputLength, final ExpandableArrayBuffer outputBuffer, final int outputLength) { - return new UnsafeBuffer(inputBuffer, 0, inputLength).equals(new UnsafeBuffer(outputBuffer, 0, outputLength)); + final boolean lengthsDiffer = inputLength != outputLength; + final int minLength = Math.min(inputLength, outputLength); + + for (int i = 0; i < minLength; i++) + { + if (inputBuffer.getByte(i) != outputBuffer.getByte(i)) + { + throw new AssertionError( + "Input and output differ at byte " + i + ".\n" + + "Input length: " + inputLength + ", Output length: " + outputLength + "\n" + + "Input: " + inputBuffer.getByte(i) + ", Output: " + outputBuffer.getByte(i) + + (lengthsDiffer ? "\nLengths differ." : "")); + } + } + + if (lengthsDiffer) + { + throw new AssertionError( + "Input and output differ in length.\n" + + "Input length: " + inputLength + ", Output length: " + outputLength); + } } private void addGeneratedSourcesFootnotes( diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java index ebac19703e..a20deb3ee5 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java @@ -46,8 +46,8 @@ void shouldGenerateValidJson(@ForAll("encodedMessage") final SbeArbitraries.Enco @Provide Arbitrary encodedMessage() { - final SbeArbitraries.CharGenerationMode mode = - SbeArbitraries.CharGenerationMode.JSON_PRINTER_COMPATIBLE; - return SbeArbitraries.encodedMessage(mode); + final SbeArbitraries.CharGenerationConfig config = + SbeArbitraries.CharGenerationConfig.jsonPrinterCompatibleAndNullTerminates(); + return SbeArbitraries.encodedMessage(config); } } diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java index 72a70ee0b0..dcabb47806 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java @@ -114,10 +114,38 @@ private static Arbitrary encodedDataTypeSchema() ).as(EncodedDataTypeSchema::new); } - public enum CharGenerationMode + public static class CharGenerationConfig { - UNRESTRICTED, - JSON_PRINTER_COMPATIBLE + private final boolean alphaOnly; + private final boolean nullCharTerminatesData; + + public CharGenerationConfig( + final boolean alphaOnly, + final boolean nullCharTerminatesData) + { + this.alphaOnly = alphaOnly; + this.nullCharTerminatesData = nullCharTerminatesData; + } + + boolean alphaOnly() + { + return alphaOnly; + } + + boolean nullCharTerminatesData() + { + return nullCharTerminatesData; + } + + public static CharGenerationConfig unrestricted() + { + return new CharGenerationConfig(false, false); + } + + public static CharGenerationConfig jsonPrinterCompatibleAndNullTerminates() + { + return new CharGenerationConfig(true, true); + } } private static Arbitrary enumTypeSchema() @@ -343,27 +371,24 @@ private static Arbitrary combineArbitraryEncoders(final List encodedTypeEncoder( final Encoding encoding, final boolean isOptional, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { - final Arbitrary inRangeEncoder = encodedTypeEncoder(encoding, charGenerationMode); + final Arbitrary inRangeEncoder = encodedTypeEncoder(encoding, charGenerationConfig); if (isOptional) { @@ -378,7 +403,7 @@ private static Arbitrary encodedTypeEncoder( private static Arbitrary encodedTypeEncoder( final Encoding encoding, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { final PrimitiveValue minValue = encoding.applicableMinValue(); final PrimitiveValue maxValue = encoding.applicableMaxValue(); @@ -387,7 +412,7 @@ private static Arbitrary encodedTypeEncoder( { case CHAR: assert minValue.longValue() <= maxValue.longValue(); - return chars(charGenerationMode).map(c -> + return chars(charGenerationConfig).map(c -> (builder, buffer, offset, limit) -> { builder.appendLine().append(c).append(" @ ").append(offset) @@ -558,13 +583,13 @@ private static Arbitrary encodedTypeEncoder( final int offset, final Token memberToken, final Token typeToken, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { final Encoding encoding = typeToken.encoding(); final Arbitrary arbEncoder = encodedTypeEncoder( encoding, memberToken.isOptionalEncoding(), - charGenerationMode + charGenerationConfig ); if (typeToken.arrayLength() == 1) @@ -577,11 +602,28 @@ private static Arbitrary encodedTypeEncoder( return arbEncoder.list().ofSize(typeToken.arrayLength()) .map(encoders -> (builder, buffer, bufferOffset, limit) -> { + boolean hasNullTerminated = false; + for (int i = 0; i < typeToken.arrayLength(); i++) { builder.beginScope("[" + i + "]"); final int elementOffset = bufferOffset + offset + i * encoding.primitiveType().size(); - encoders.get(i).encode(builder, buffer, elementOffset, limit); + if (hasNullTerminated) + { + buffer.putByte(elementOffset, (byte)0); + } + else + { + encoders.get(i).encode(builder, buffer, elementOffset, limit); + } + + if (encoding.primitiveType() == PrimitiveType.CHAR && + charGenerationConfig.nullCharTerminatesData() && + buffer.getByte(elementOffset) == 0) + { + hasNullTerminated = true; + } + builder.endScope(); } }); @@ -786,7 +828,7 @@ private static Arbitrary fieldsEncoder( final MutableInteger cursor, final int endIdxInclusive, final boolean expectFields, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { final List> encoders = new ArrayList<>(); while (cursor.get() <= endIdxInclusive) @@ -820,7 +862,7 @@ else if (expectFields) final int endCompositeTokenCount = 1; final int lastMemberIdx = nextFieldIdx - endCompositeTokenCount - endFieldTokenCount - 1; final Arbitrary encoder = fieldsEncoder( - tokens, cursor, lastMemberIdx, false, charGenerationMode); + tokens, cursor, lastMemberIdx, false, charGenerationConfig); fieldEncoder = encoder.map(e -> (builder, buffer, bufferOffset, limit) -> e.encode(builder, buffer, bufferOffset + offset, limit)); @@ -839,7 +881,7 @@ else if (expectFields) break; case ENCODING: - fieldEncoder = encodedTypeEncoder(offset, memberToken, typeToken, charGenerationMode); + fieldEncoder = encodedTypeEncoder(offset, memberToken, typeToken, charGenerationConfig); break; default: @@ -868,7 +910,7 @@ private static Arbitrary groupsEncoder( final List tokens, final MutableInteger cursor, final int endIdxInclusive, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { final List> encoders = new ArrayList<>(); @@ -894,9 +936,9 @@ private static Arbitrary groupsEncoder( final Arbitrary groupElement = Combinators.combine( - fieldsEncoder(tokens, cursor, nextFieldIdx - 1, true, charGenerationMode), - groupsEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationMode), - varDataEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationMode) + fieldsEncoder(tokens, cursor, nextFieldIdx - 1, true, charGenerationConfig), + groupsEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationConfig), + varDataEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationConfig) ).as((fieldsEncoder, groupsEncoder, varDataEncoder) -> (builder, buffer, ignored, limit) -> { @@ -946,7 +988,7 @@ private static Arbitrary varDataEncoder( final List tokens, final MutableInteger cursor, final int endIdxInclusive, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { final List> encoders = new ArrayList<>(); @@ -969,7 +1011,7 @@ private static Arbitrary varDataEncoder( final String characterEncoding = varDataToken.encoding().characterEncoding(); final Arbitrary arbitraryByte = null == characterEncoding ? Arbitraries.bytes() : - chars(charGenerationMode).map(c -> (byte)c.charValue()); + chars(charGenerationConfig).map(c -> (byte)c.charValue()); encoders.add(arbitraryByte.list() .ofMaxSize((int)Math.min(lengthToken.encoding().applicableMaxValue().longValue(), 260L)) .map(bytes -> (builder, buffer, ignored, limit) -> @@ -1003,7 +1045,7 @@ private static Arbitrary varDataEncoder( private static Arbitrary messageValueEncoder( final Ir ir, final short messageId, - final CharGenerationMode charGenerationMode) + final CharGenerationConfig charGenerationConfig) { final List tokens = ir.getMessage(messageId); final MutableInteger cursor = new MutableInteger(1); @@ -1015,11 +1057,11 @@ private static Arbitrary messageValueEncoder( } final Arbitrary fieldsEncoder = fieldsEncoder( - tokens, cursor, tokens.size() - 1, true, charGenerationMode); + tokens, cursor, tokens.size() - 1, true, charGenerationConfig); final Arbitrary groupsEncoder = groupsEncoder( - tokens, cursor, tokens.size() - 1, charGenerationMode); + tokens, cursor, tokens.size() - 1, charGenerationConfig); final Arbitrary varDataEncoder = varDataEncoder( - tokens, cursor, tokens.size() - 1, charGenerationMode); + tokens, cursor, tokens.size() - 1, charGenerationConfig); return Combinators.combine(fieldsEncoder, groupsEncoder, varDataEncoder) .as((fields, groups, varData) -> (builder, buffer, offset, limit) -> { @@ -1087,7 +1129,7 @@ public String encodingLog() } } - public static Arbitrary encodedMessage(final CharGenerationMode mode) + public static Arbitrary encodedMessage(final CharGenerationConfig charGenerationConfig) { return SbeArbitraries.messageSchema().flatMap(testSchema -> { @@ -1101,7 +1143,7 @@ public static Arbitrary encodedMessage(final CharGenerationMode .build(); final uk.co.real_logic.sbe.xml.MessageSchema parsedSchema = parse(in, options); final Ir ir = new IrGenerator().generate(parsedSchema); - return SbeArbitraries.messageValueEncoder(ir, testSchema.templateId(), mode) + return SbeArbitraries.messageValueEncoder(ir, testSchema.templateId(), charGenerationConfig) .map(encoder -> { final EncodingLogger logger = new EncodingLogger(); From ddd42b93d2829e70c862a2b300e58eaef443d874 Mon Sep 17 00:00:00 2001 From: Zach Bray Date: Tue, 17 Jun 2025 18:41:45 +0100 Subject: [PATCH 3/4] [Java,C++,C#] Improve DTO value validation. Previously, in Java, the generated validation methods would not permit the null value, even for fields with optional presence. Now, we do allow these correctly. In this commit, I've also attempted to clean up, centralise, and test the logic that decides when validation for a particular side of a range needs to be included. We omit the validation when the native type cannot represent values outside of the range. There are still some gaps around validation, e.g., we do not validate array values. --- .../generation/common/DtoValidationUtil.java | 224 ++++++++++++++++++ .../sbe/generation/cpp/CppDtoGenerator.java | 10 +- .../generation/csharp/CSharpDtoGenerator.java | 11 +- .../sbe/generation/java/JavaDtoGenerator.java | 168 ++++++++++--- .../common/DtoValidationUtilTest.java | 105 ++++++++ 5 files changed, 482 insertions(+), 36 deletions(-) create mode 100644 sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtil.java create mode 100644 sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtilTest.java diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtil.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtil.java new file mode 100644 index 0000000000..1e522c232b --- /dev/null +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtil.java @@ -0,0 +1,224 @@ +/* + * Copyright 2013-2025 Real Logic Limited. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.co.real_logic.sbe.generation.common; + +import uk.co.real_logic.sbe.PrimitiveType; +import uk.co.real_logic.sbe.PrimitiveValue; +import uk.co.real_logic.sbe.ir.Encoding; +import uk.co.real_logic.sbe.ir.Token; + +/** + * Helpers for generating value validation code. + */ +public final class DtoValidationUtil +{ + private DtoValidationUtil() + { + } + + /** + * What support the target language has for native integer types. + */ + public enum NativeIntegerSupport + { + /** + * The target language supports both signed and unsigned integers natively and the generated code uses + * these to represent the SBE types. + */ + SIGNED_AND_UNSIGNED, + + /** + * The target language only supports signed integers natively and the generated code uses the next biggest + * signed integer type to represent the unsigned SBE types, except for UINT64 which is always represented + * as a signed long. + */ + SIGNED_ONLY + } + + /** + * Checks if the native type can represent values less than the valid range of the SBE type. + * + * @param fieldToken the field token to check if it is optional. + * @param encoding the encoding of the field to check the applicable minimum and null values. + * @param integerSupport the support for native integer types in the target language. + * @return true if the native type can represent values less than the valid range of the SBE type, + * false otherwise. + */ + public static boolean nativeTypeRepresentsValuesLessThanValidRange( + final Token fieldToken, + final Encoding encoding, + final NativeIntegerSupport integerSupport) + { + final PrimitiveType primitiveType = encoding.primitiveType(); + final PrimitiveValue minValue = encoding.applicableMinValue(); + + switch (minValue.representation()) + { + case LONG: + final long nativeMinValue = nativeTypeMinValue(primitiveType, integerSupport); + final PrimitiveValue nullValue = encoding.applicableNullValue(); + final boolean gapBefore = minValue.longValue() > nativeMinValue; + final boolean nullFillsGap = fieldToken.isOptionalEncoding() && + nullValue.longValue() == nativeMinValue && + minValue.longValue() == nativeMinValue + 1L; + return gapBefore && !nullFillsGap; + + case DOUBLE: + switch (primitiveType) + { + case FLOAT: + return minValue.doubleValue() > -Float.MAX_VALUE; + case DOUBLE: + return minValue.doubleValue() > -Double.MAX_VALUE; + default: + throw new IllegalArgumentException( + "Type did not have a double representation: " + primitiveType); + } + + default: + throw new IllegalArgumentException( + "Cannot understand the range of a type with representation: " + minValue.representation()); + } + } + + /** + * Checks if the native type can represent values greater than the valid range of the SBE type. + * + * @param fieldToken the field token to check if it is optional. + * @param encoding the encoding of the field to check the applicable maximum and null values. + * @param integerSupport the support for native integer types in the target language. + * @return true if the native type can represent values greater than the valid range of the SBE type, + * false otherwise. + */ + public static boolean nativeTypeRepresentsValuesGreaterThanValidRange( + final Token fieldToken, + final Encoding encoding, + final NativeIntegerSupport integerSupport) + { + final PrimitiveType primitiveType = encoding.primitiveType(); + final PrimitiveValue maxValue = encoding.applicableMaxValue(); + + switch (maxValue.representation()) + { + case LONG: + final long nativeMaxValue = nativeTypeMaxValue(primitiveType, integerSupport); + final PrimitiveValue nullValue = encoding.applicableNullValue(); + final boolean gapAfter = maxValue.longValue() < nativeMaxValue; + final boolean nullFillsGap = fieldToken.isOptionalEncoding() && + nullValue.longValue() == nativeMaxValue && + maxValue.longValue() + 1L == nativeMaxValue; + return gapAfter && !nullFillsGap; + + case DOUBLE: + switch (primitiveType) + { + case FLOAT: + return maxValue.doubleValue() < Float.MAX_VALUE; + case DOUBLE: + return maxValue.doubleValue() < Double.MAX_VALUE; + default: + throw new IllegalArgumentException( + "Type did not have a double representation: " + primitiveType); + } + + default: + throw new IllegalArgumentException( + "Cannot understand the range of a type with representation: " + maxValue.representation()); + } + } + + private static long nativeTypeMinValue( + final PrimitiveType primitiveType, + final NativeIntegerSupport integerSupport) + { + switch (primitiveType) + { + case CHAR: + return Character.MIN_VALUE; + case INT8: + return Byte.MIN_VALUE; + case INT16: + return Short.MIN_VALUE; + case INT32: + return Integer.MIN_VALUE; + case INT64: + return Long.MIN_VALUE; + case UINT8: + if (integerSupport == NativeIntegerSupport.SIGNED_ONLY) + { + return Short.MIN_VALUE; + } + return 0L; + case UINT16: + if (integerSupport == NativeIntegerSupport.SIGNED_ONLY) + { + return Integer.MIN_VALUE; + } + return 0L; + case UINT32: + if (integerSupport == NativeIntegerSupport.SIGNED_ONLY) + { + return Long.MIN_VALUE; + } + return 0L; + case UINT64: + return 0L; + default: + throw new IllegalArgumentException("Type did not have a long representation: " + primitiveType); + } + } + + private static long nativeTypeMaxValue( + final PrimitiveType primitiveType, + final NativeIntegerSupport integerSupport) + { + switch (primitiveType) + { + case CHAR: + return Character.MAX_VALUE; + case INT8: + return Byte.MAX_VALUE; + case INT16: + return Short.MAX_VALUE; + case INT32: + return Integer.MAX_VALUE; + case INT64: + return Long.MAX_VALUE; + case UINT8: + if (integerSupport == NativeIntegerSupport.SIGNED_ONLY) + { + return Short.MAX_VALUE; + } + return 0xFFL; + case UINT16: + if (integerSupport == NativeIntegerSupport.SIGNED_ONLY) + { + return Integer.MAX_VALUE; + } + return 0xFFFFL; + case UINT32: + if (integerSupport == NativeIntegerSupport.SIGNED_ONLY) + { + return Long.MAX_VALUE; + } + return 0xFFFFFFFFL; + case UINT64: + return 0xFFFFFFFFFFFFFFFFL; + default: + throw new IllegalArgumentException("Type did not have a long representation: " + primitiveType); + } + } +} diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppDtoGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppDtoGenerator.java index 05d214d751..baf01b639e 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppDtoGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppDtoGenerator.java @@ -35,6 +35,9 @@ import static uk.co.real_logic.sbe.generation.Generators.toLowerFirstChar; import static uk.co.real_logic.sbe.generation.Generators.toUpperFirstChar; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.NativeIntegerSupport.SIGNED_AND_UNSIGNED; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesGreaterThanValidRange; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesLessThanValidRange; import static uk.co.real_logic.sbe.generation.cpp.CppUtil.*; import static uk.co.real_logic.sbe.ir.GenerationUtil.collectFields; import static uk.co.real_logic.sbe.ir.GenerationUtil.collectGroups; @@ -1591,8 +1594,11 @@ private static void generateSingleValuePropertyValidateMethod( String value = "value"; - final boolean mustPreventLesser = !encoding.applicableMinValue().equals(encoding.primitiveType().minValue()); - final boolean mustPreventGreater = !encoding.applicableMaxValue().equals(encoding.primitiveType().maxValue()); + final boolean mustPreventLesser = + nativeTypeRepresentsValuesLessThanValidRange(fieldToken, encoding, SIGNED_AND_UNSIGNED); + + final boolean mustPreventGreater = + nativeTypeRepresentsValuesGreaterThanValidRange(fieldToken, encoding, SIGNED_AND_UNSIGNED); if (fieldToken.isOptionalEncoding()) { diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpDtoGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpDtoGenerator.java index 47bef5a349..ed55bae37b 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpDtoGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpDtoGenerator.java @@ -35,6 +35,9 @@ import java.util.Set; import java.util.function.Predicate; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.NativeIntegerSupport.SIGNED_AND_UNSIGNED; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesGreaterThanValidRange; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesLessThanValidRange; import static uk.co.real_logic.sbe.generation.csharp.CSharpUtil.*; import static uk.co.real_logic.sbe.ir.GenerationUtil.collectFields; import static uk.co.real_logic.sbe.ir.GenerationUtil.collectGroups; @@ -1285,7 +1288,9 @@ private void generateSingleValueProperty( .append("}\n"); } - final boolean mustPreventLesser = !encoding.applicableMinValue().equals(encoding.primitiveType().minValue()); + final boolean mustPreventLesser = + nativeTypeRepresentsValuesLessThanValidRange(fieldToken, typeToken.encoding(), SIGNED_AND_UNSIGNED); + if (mustPreventLesser) { sb.append(indent).append(INDENT) @@ -1299,7 +1304,9 @@ private void generateSingleValueProperty( .append("}\n"); } - final boolean mustPreventGreater = !encoding.applicableMaxValue().equals(encoding.primitiveType().maxValue()); + final boolean mustPreventGreater = + nativeTypeRepresentsValuesGreaterThanValidRange(fieldToken, typeToken.encoding(), SIGNED_AND_UNSIGNED); + if (mustPreventGreater) { sb.append(indent).append(INDENT) diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtoGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtoGenerator.java index 65d7bbf6a8..bb7da8df34 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtoGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaDtoGenerator.java @@ -19,6 +19,7 @@ import uk.co.real_logic.sbe.PrimitiveType; import uk.co.real_logic.sbe.generation.CodeGenerator; import uk.co.real_logic.sbe.generation.Generators; +import uk.co.real_logic.sbe.ir.Encoding; import uk.co.real_logic.sbe.ir.Ir; import uk.co.real_logic.sbe.ir.Signal; import uk.co.real_logic.sbe.ir.Token; @@ -36,6 +37,9 @@ import static uk.co.real_logic.sbe.generation.Generators.toLowerFirstChar; import static uk.co.real_logic.sbe.generation.Generators.toUpperFirstChar; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.NativeIntegerSupport.SIGNED_ONLY; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesGreaterThanValidRange; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesLessThanValidRange; import static uk.co.real_logic.sbe.generation.java.JavaUtil.*; import static uk.co.real_logic.sbe.ir.GenerationUtil.collectFields; import static uk.co.real_logic.sbe.ir.GenerationUtil.collectGroups; @@ -1485,57 +1489,157 @@ private void generateSingleValueProperty( final String typeName = javaTypeName(typeToken.encoding().primitiveType()); final String formattedPropertyName = formatPropertyName(propertyName); final String fieldName = formatFieldName(propertyName); + + final String validateMethod = generateSingleValueValidateMethod( + classBuilder, + decoderClassName, + propertyName, + fieldToken, + typeToken, + indent, + typeName, + formattedPropertyName + ); + + classBuilder.appendField() + .append(indent).append("private ").append(typeName).append(" ").append(fieldName).append(";\n"); + + classBuilder.appendPublic().append("\n") + .append(generateDocumentation(indent, fieldToken)) + .append(indent).append("public ").append(typeName).append(" ") + .append(formattedPropertyName).append("()\n") + .append(indent).append("{\n") + .append(indent).append(INDENT).append("return this.").append(fieldName).append(";\n") + .append(indent).append("}\n"); + + final StringBuilder setterBuilder = classBuilder.appendPublic().append("\n") + .append(generateDocumentation(indent, fieldToken)) + .append(indent).append("public void ").append(formattedPropertyName).append("(") + .append(typeName).append(" value)\n") + .append(indent).append("{\n"); + + if (null != validateMethod) + { + setterBuilder.append(indent).append(INDENT).append(validateMethod).append("(value);\n"); + } + + setterBuilder + .append(indent).append(INDENT).append("this.").append(fieldName).append(" = value;\n") + .append(indent).append("}\n"); + } + + private static String generateSingleValueValidateMethod( + final ClassBuilder classBuilder, + final String decoderClassName, + final String propertyName, + final Token fieldToken, + final Token typeToken, + final String indent, + final String typeName, + final String formattedPropertyName) + { + final boolean mustPreventLesser = + nativeTypeRepresentsValuesLessThanValidRange(fieldToken, typeToken.encoding(), SIGNED_ONLY); + + final boolean mustPreventGreater = + nativeTypeRepresentsValuesGreaterThanValidRange(fieldToken, typeToken.encoding(), SIGNED_ONLY); + + if (!mustPreventLesser && !mustPreventGreater) + { + return null; + } + final String validateMethod = "validate" + toUpperFirstChar(propertyName); - final boolean representedWithinJavaType = typeToken.encoding().primitiveType() != PrimitiveType.UINT64; + final StringBuilder validateBuilder = classBuilder.appendPrivate().append("\n") + .append(indent).append("private static void ").append(validateMethod).append("(") + .append(typeName).append(" value)\n") + .append(indent).append("{\n"); - final StringBuilder validationCall = new StringBuilder(); + final boolean allowsNull = fieldToken.isOptionalEncoding(); - if (representedWithinJavaType) + if (allowsNull) { - final StringBuilder validateBuilder = classBuilder.appendPrivate().append("\n") - .append(indent).append("private static void ").append(validateMethod).append("(") - .append(typeName).append(" value)\n") - .append(indent).append("{\n"); + validateBuilder.append(indent).append(INDENT) + .append("if (value == ") + .append(decoderClassName).append(".").append(formattedPropertyName).append("NullValue())\n") + .append(indent).append(INDENT) + .append("{\n") + .append(indent).append(INDENT).append(INDENT) + .append("return;\n") + .append(indent).append(INDENT) + .append("}\n\n"); + } + if (mustPreventLesser) + { validateBuilder.append(indent).append(INDENT) - .append("if (value < ") - .append(decoderClassName).append(".").append(formattedPropertyName).append("MinValue() || ") - .append("value").append(" > ") - .append(decoderClassName).append(".").append(formattedPropertyName).append("MaxValue())\n") + .append("if ("); + + generateGreaterThanExpression( + validateBuilder, + typeToken.encoding(), + decoderClassName + "." + formattedPropertyName + "MinValue()", + "value" + ); + + validateBuilder + .append(")\n") .append(indent).append(INDENT) .append("{\n") .append(indent).append(INDENT).append(INDENT) .append("throw new IllegalArgumentException(\"") .append(propertyName) - .append(": value is out of allowed range: \" + ") + .append(": value is below allowed minimum: \" + ") .append("value").append(");\n") .append(indent).append(INDENT) - .append("}\n") - .append(indent).append("}\n"); + .append("}\n\n"); + } - validationCall.append(indent).append(INDENT).append(validateMethod).append("(value);\n"); + if (mustPreventGreater) + { + validateBuilder.append(indent).append(INDENT) + .append("if ("); + + generateGreaterThanExpression( + validateBuilder, + typeToken.encoding(), + "value", + decoderClassName + "." + formattedPropertyName + "MaxValue()" + ); + + validateBuilder + .append(")\n") + .append(indent).append(INDENT) + .append("{\n") + .append(indent).append(INDENT).append(INDENT) + .append("throw new IllegalArgumentException(\"") + .append(propertyName) + .append(": value is above allowed maximum: \" + ") + .append("value").append(");\n") + .append(indent).append(INDENT) + .append("}\n\n"); } - classBuilder.appendField() - .append(indent).append("private ").append(typeName).append(" ").append(fieldName).append(";\n"); + validateBuilder.append(indent).append("}\n"); - classBuilder.appendPublic().append("\n") - .append(generateDocumentation(indent, fieldToken)) - .append(indent).append("public ").append(typeName).append(" ") - .append(formattedPropertyName).append("()\n") - .append(indent).append("{\n") - .append(indent).append(INDENT).append("return this.").append(fieldName).append(";\n") - .append(indent).append("}\n"); + return validateMethod; + } - classBuilder.appendPublic().append("\n") - .append(generateDocumentation(indent, fieldToken)) - .append(indent).append("public void ").append(formattedPropertyName).append("(") - .append(typeName).append(" value)\n") - .append(indent).append("{\n") - .append(validationCall) - .append(indent).append(INDENT).append("this.").append(fieldName).append(" = value;\n") - .append(indent).append("}\n"); + private static void generateGreaterThanExpression( + final StringBuilder builder, + final Encoding encoding, + final String lhs, + final String rhs) + { + if (encoding.primitiveType() == PrimitiveType.UINT64) + { + builder.append("Long.compareUnsigned(").append(lhs).append(", ").append(rhs).append(") > 0"); + } + else + { + builder.append(lhs).append(" > ").append(rhs); + } } private void generateConstPropertyMethods( diff --git a/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtilTest.java b/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtilTest.java new file mode 100644 index 0000000000..ea6d19e5fa --- /dev/null +++ b/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/common/DtoValidationUtilTest.java @@ -0,0 +1,105 @@ +/* + * Copyright 2013-2025 Real Logic Limited. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package uk.co.real_logic.sbe.generation.common; + +import uk.co.real_logic.sbe.PrimitiveType; +import uk.co.real_logic.sbe.PrimitiveValue; +import uk.co.real_logic.sbe.ir.Encoding; +import uk.co.real_logic.sbe.ir.Token; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesGreaterThanValidRange; +import static uk.co.real_logic.sbe.generation.common.DtoValidationUtil.nativeTypeRepresentsValuesLessThanValidRange; + +public class DtoValidationUtilTest +{ + @ParameterizedTest + @CsvSource({ + "int8,SIGNED_AND_UNSIGNED,false,-128,-127,127,true,false", + "int8,SIGNED_AND_UNSIGNED,true,-128,-127,127,false,false", + "int8,SIGNED_ONLY,false,-128,-127,127,true,false", + "int8,SIGNED_ONLY,true,-128,-127,127,false,false", + + "int8,SIGNED_AND_UNSIGNED,false,127,-128,126,false,true", + "int8,SIGNED_AND_UNSIGNED,true,127,-128,126,false,false", + "int8,SIGNED_ONLY,false,127,-128,126,false,true", + "int8,SIGNED_ONLY,true,127,-128,126,false,false", + + "int8,SIGNED_ONLY,true,-128,-100,127,true,false", + "int8,SIGNED_ONLY,true,127,-128,100,false,true", + + "int8,SIGNED_ONLY,true,0,-128,127,false,false", + "int8,SIGNED_ONLY,true,0,-127,127,true,false", + "int8,SIGNED_ONLY,true,0,-128,126,false,true", + "int8,SIGNED_ONLY,true,0,-127,126,true,true", + + "uint8,SIGNED_AND_UNSIGNED,false,255,0,254,false,true", + "uint8,SIGNED_AND_UNSIGNED,true,255,0,254,false,false", + "uint8,SIGNED_ONLY,false,255,0,254,true,true", + "uint8,SIGNED_ONLY,true,255,0,254,true,true", + + "float,SIGNED_AND_UNSIGNED,false,-2,-1,1,true,true", + "float,SIGNED_AND_UNSIGNED,true,-2,-1,1,true,true", + "float,SIGNED_ONLY,false,-2,-1,1,true,true", + "float,SIGNED_ONLY,true,-2,-1,1,true,true", + + "uint64,SIGNED_AND_UNSIGNED,true,18446744073709551615,0,18446744073709551614,false,false", + "uint64,SIGNED_AND_UNSIGNED,true,18446744073709551615,1,18446744073709551614,true,false", + "uint64,SIGNED_AND_UNSIGNED,true,18446744073709551615,0,18446744073709551613,false,true", + "uint64,SIGNED_AND_UNSIGNED,true,18446744073709551615,1,18446744073709551613,true,true", + "uint64,SIGNED_ONLY,true,18446744073709551615,0,18446744073709551614,false,false", + "uint64,SIGNED_ONLY,true,18446744073709551615,1,18446744073709551614,true,false", + "uint64,SIGNED_ONLY,true,18446744073709551615,0,18446744073709551613,false,true", + "uint64,SIGNED_ONLY,true,18446744073709551615,1,18446744073709551613,true,true", + }) + void shouldGenerateValidationBasedOnNativeRangeVersusSbeTypeRange( + final String type, + final DtoValidationUtil.NativeIntegerSupport integerSupport, + final boolean isOptional, + final String nullValue, + final String minValue, + final String maxValue, + final boolean shouldValidateBelow, + final boolean shouldValidateAbove) + { + final Token fieldToken = mock(Token.class); + when(fieldToken.isOptionalEncoding()).thenReturn(isOptional); + final Encoding encoding = mock(Encoding.class); + final PrimitiveType primitiveType = PrimitiveType.get(type); + when(encoding.primitiveType()).thenReturn(primitiveType); + when(encoding.applicableNullValue()).thenReturn(PrimitiveValue.parse(nullValue, primitiveType)); + when(encoding.applicableMinValue()).thenReturn(PrimitiveValue.parse(minValue, primitiveType)); + when(encoding.applicableMaxValue()).thenReturn(PrimitiveValue.parse(maxValue, primitiveType)); + + final boolean validatesBelow = + nativeTypeRepresentsValuesLessThanValidRange(fieldToken, encoding, integerSupport); + + final boolean validatesAbove = + nativeTypeRepresentsValuesGreaterThanValidRange(fieldToken, encoding, integerSupport); + + assertEquals( + shouldValidateBelow, validatesBelow, + shouldValidateBelow ? "should" : "should not" + " validate below"); + + assertEquals( + shouldValidateAbove, validatesAbove, + shouldValidateAbove ? "should" : "should not" + " validate above"); + } +} From 735eeb3787d1a3f7f7f6f9c9888b3464e5da94eb Mon Sep 17 00:00:00 2001 From: Zach Bray Date: Thu, 19 Jun 2025 13:40:37 +0100 Subject: [PATCH 4/4] [Java] Fix JSON escaping of string characters, delimeters, etc. Previously, there were several problems: - (delimeters) single `char` values were output with single quotes, but this is not valid JSON. - (escaping) escaping was not implemented as per the specification in Section 7 of RFC 8259. For example, special-cased control codes, e.g., `\b` were encoded as `\\` followed by `\b` rather than `\\` followed by `b`. Also, the non-special-cased control characters were not encoded using JSON's mechanism for doing so, e.g., `\u0020`. - (numbers) Section 6 of the specification says "Numeric values that cannot be represented in the grammar below (such as Infinity and NaN) are not permitted." However, these were encoded as expressions of numbers with multiple terms, e.g., `-1/0` for positive infinity. While these are quite logical encodings of such "numbers", it is not valid JSON. Therefore, I have kept the expressions, but enclosed them within quotes. Also, in this commit: - Replaced custom compilation logic with Agrona's CompilerUtil. Note that `CompilerUtil#compileOnDisk` is broken. I attempted to use it to see if I could see classes in IntelliJ rather than just stack frames, but it fails with an NPE. --- .../sbe/json/JsonTokenListener.java | 30 +--- .../java/uk/co/real_logic/sbe/otf/Types.java | 105 ++++++++++++-- .../sbe/properties/DtosPropertyTest.java | 101 +++++++------- .../sbe/properties/JsonPropertyTest.java | 14 +- .../sbe/properties/PropertyTestUtil.java | 43 ++++++ .../arbitraries/SbeArbitraries.java | 111 ++++++++++----- .../schema/TestXmlSchemaWriter.java | 16 +-- .../utils/InMemoryOutputManager.java | 132 +++--------------- 8 files changed, 303 insertions(+), 249 deletions(-) create mode 100644 sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/PropertyTestUtil.java diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/json/JsonTokenListener.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/json/JsonTokenListener.java index 4d81dc47b9..18001ae9fa 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/json/JsonTokenListener.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/json/JsonTokenListener.java @@ -15,13 +15,13 @@ */ package uk.co.real_logic.sbe.json; -import org.agrona.DirectBuffer; -import org.agrona.PrintBufferUtil; import uk.co.real_logic.sbe.PrimitiveValue; import uk.co.real_logic.sbe.ir.Encoding; import uk.co.real_logic.sbe.ir.Token; import uk.co.real_logic.sbe.otf.TokenListener; import uk.co.real_logic.sbe.otf.Types; +import org.agrona.DirectBuffer; +import org.agrona.PrintBufferUtil; import java.io.UnsupportedEncodingException; import java.util.List; @@ -236,7 +236,7 @@ public void onVarData( final String str = charsetName != null ? new String(tempBuffer, 0, length, charsetName) : PrintBufferUtil.hexDump(tempBuffer); - escape(str); + Types.jsonEscape(str, output); doubleQuote(); next(); @@ -290,7 +290,7 @@ private void appendEncodingAsString( final long longValue = constOrNotPresentValue.longValue(); if (PrimitiveValue.NULL_VALUE_CHAR != longValue) { - escape(new String(new byte[]{ (byte)longValue }, characterEncoding)); + Types.jsonEscape(new String(new byte[] {(byte)longValue}, characterEncoding), output); } } catch (final UnsupportedEncodingException ex) @@ -300,7 +300,7 @@ private void appendEncodingAsString( } else { - escape(constOrNotPresentValue.toString()); + Types.jsonEscape(constOrNotPresentValue.toString(), output); } doubleQuote(); @@ -371,7 +371,7 @@ private void escapePrintableChar(final DirectBuffer buffer, final int index, fin final byte c = buffer.getByte(index + (i * elementSize)); if (c > 0) { - escape((char)c); + Types.jsonEscape((char)c, output); } else { @@ -467,22 +467,4 @@ private static long readEncodingAsLong( return Types.getLong(buffer, bufferIndex, typeToken.encoding()); } - - private void escape(final String str) - { - for (int i = 0, length = str.length(); i < length; i++) - { - escape(str.charAt(i)); - } - } - - private void escape(final char c) - { - if ('"' == c || '\\' == c || '\b' == c || '\f' == c || '\n' == c || '\r' == c || '\t' == c) - { - output.append('\\'); - } - - output.append(c); - } } diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/otf/Types.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/otf/Types.java index d9c64a2a2f..2d41cf22ba 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/otf/Types.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/otf/Types.java @@ -15,10 +15,10 @@ */ package uk.co.real_logic.sbe.otf; -import org.agrona.DirectBuffer; import uk.co.real_logic.sbe.PrimitiveType; import uk.co.real_logic.sbe.PrimitiveValue; import uk.co.real_logic.sbe.ir.Encoding; +import org.agrona.DirectBuffer; import java.nio.ByteOrder; @@ -27,6 +27,11 @@ */ public class Types { + private static final char[] HEX_DIGIT = { + '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' + }; + /** * Get an integer value from a buffer at a given index for a {@link PrimitiveType}. * @@ -187,7 +192,9 @@ public static void appendAsJsonString( switch (encoding.primitiveType()) { case CHAR: - sb.append('\'').append((char)buffer.getByte(index)).append('\''); + sb.append('\"'); + jsonEscape((char)buffer.getByte(index), sb); + sb.append('\"'); break; case INT8: @@ -227,15 +234,15 @@ public static void appendAsJsonString( final float value = buffer.getFloat(index, encoding.byteOrder()); if (Float.isNaN(value)) { - sb.append("0/0"); + sb.append("\"0/0\""); } else if (value == Float.POSITIVE_INFINITY) { - sb.append("1/0"); + sb.append("\"1/0\""); } else if (value == Float.NEGATIVE_INFINITY) { - sb.append("-1/0"); + sb.append("\"-1/0\""); } else { @@ -249,15 +256,15 @@ else if (value == Float.NEGATIVE_INFINITY) final double value = buffer.getDouble(index, encoding.byteOrder()); if (Double.isNaN(value)) { - sb.append("0/0"); + sb.append("\"0/0\""); } else if (value == Double.POSITIVE_INFINITY) { - sb.append("1/0"); + sb.append("\"1/0\""); } else if (value == Double.NEGATIVE_INFINITY) { - sb.append("-1/0"); + sb.append("\"-1/0\""); } else { @@ -280,7 +287,9 @@ public static void appendAsJsonString(final StringBuilder sb, final PrimitiveVal switch (encoding.primitiveType()) { case CHAR: - sb.append('\'').append((char)value.longValue()).append('\''); + sb.append('\"'); + jsonEscape((char)value.longValue(), sb); + sb.append('\"'); break; case INT8: @@ -299,15 +308,15 @@ public static void appendAsJsonString(final StringBuilder sb, final PrimitiveVal final float floatValue = (float)value.doubleValue(); if (Float.isNaN(floatValue)) { - sb.append("0/0"); + sb.append("\"0/0\""); } else if (floatValue == Float.POSITIVE_INFINITY) { - sb.append("1/0"); + sb.append("\"1/0\""); } else if (floatValue == Float.NEGATIVE_INFINITY) { - sb.append("-1/0"); + sb.append("\"-1/0\""); } else { @@ -321,15 +330,15 @@ else if (floatValue == Float.NEGATIVE_INFINITY) final double doubleValue = value.doubleValue(); if (Double.isNaN(doubleValue)) { - sb.append("0/0"); + sb.append("\"0/0\""); } else if (doubleValue == Double.POSITIVE_INFINITY) { - sb.append("1/0"); + sb.append("\"1/0\""); } else if (doubleValue == Double.NEGATIVE_INFINITY) { - sb.append("-1/0"); + sb.append("\"-1/0\""); } else { @@ -339,4 +348,70 @@ else if (doubleValue == Double.NEGATIVE_INFINITY) } } } + + /** + * Escape a string for use in a JSON string. + * + * @param str the string to escape + * @param output to append the escaped string to + */ + public static void jsonEscape(final String str, final StringBuilder output) + { + for (int i = 0, length = str.length(); i < length; i++) + { + jsonEscape(str.charAt(i), output); + } + } + + /** + * Escape a character for use in a JSON string. + * + * @param c the character to escape + * @param output to append the escaped character to + */ + public static void jsonEscape(final char c, final StringBuilder output) + { + if ('"' == c || '\\' == c) + { + output.append('\\'); + output.append(c); + } + else if ('\b' == c) + { + output.append("\\b"); + } + else if ('\f' == c) + { + output.append("\\f"); + } + else if ('\n' == c) + { + output.append("\\n"); + } + else if ('\r' == c) + { + output.append("\\r"); + } + else if ('\t' == c) + { + output.append("\\t"); + } + else if (c <= 0x1F || Character.isHighSurrogate(c) || Character.isLowSurrogate(c)) + { + jsonUnicodeEncode(c, output); + } + else + { + output.append(c); + } + } + + private static void jsonUnicodeEncode(final char c, final StringBuilder output) + { + output.append('\\').append('u') + .append(HEX_DIGIT[(c >>> 12) & 0x0F]) + .append(HEX_DIGIT[(c >>> 8) & 0x0F]) + .append(HEX_DIGIT[(c >>> 4) & 0x0F]) + .append(HEX_DIGIT[c & 0x0F]); + } } diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java index c7399b4ef3..ba37558242 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/DtosPropertyTest.java @@ -36,19 +36,17 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.Base64; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.fail; import static uk.co.real_logic.sbe.SbeTool.JAVA_DEFAULT_DECODING_BUFFER_TYPE; import static uk.co.real_logic.sbe.SbeTool.JAVA_DEFAULT_ENCODING_BUFFER_TYPE; +import static uk.co.real_logic.sbe.properties.PropertyTestUtil.addSchemaAndInputMessageFootnotes; @SuppressWarnings("ReadWriteStringCanBeUsed") @EnableFootnotes @@ -64,8 +62,7 @@ public class DtosPropertyTest void javaDtoEncodeShouldBeTheInverseOfDtoDecode( @ForAll("encodedMessage") final SbeArbitraries.EncodedMessage encodedMessage, final Footnotes footnotes) - throws IOException, ClassNotFoundException, NoSuchMethodException, InvocationTargetException, - IllegalAccessException + throws Exception { final String packageName = encodedMessage.ir().applicableNamespace(); final InMemoryOutputManager outputManager = new InMemoryOutputManager(packageName); @@ -94,36 +91,36 @@ void javaDtoEncodeShouldBeTheInverseOfDtoDecode( fail("Code generation failed.", generationException); } - try (URLClassLoader generatedClassLoader = outputManager.compileGeneratedSources()) - { - final Class dtoClass = - generatedClassLoader.loadClass(packageName + ".TestMessageDto"); - - final Method decodeFrom = - dtoClass.getMethod("decodeFrom", DirectBuffer.class, int.class, int.class, int.class); - - final Method encodeWith = - dtoClass.getMethod("encodeWithHeaderWith", dtoClass, MutableDirectBuffer.class, int.class); - - final int inputLength = encodedMessage.length(); - final ExpandableArrayBuffer inputBuffer = encodedMessage.buffer(); - final MessageHeaderDecoder header = new MessageHeaderDecoder().wrap(inputBuffer, 0); - final int blockLength = header.blockLength(); - final int actingVersion = header.version(); - final Object dto = decodeFrom.invoke(null, - encodedMessage.buffer(), MessageHeaderDecoder.ENCODED_LENGTH, blockLength, actingVersion); - outputBuffer.setMemory(0, outputBuffer.capacity(), (byte)0); - final int outputLength = (int)encodeWith.invoke(null, dto, outputBuffer, 0); - assertEqual(inputBuffer, inputLength, outputBuffer, outputLength); - } + final Class dtoClass = outputManager.compileAndLoad(packageName + ".TestMessageDto"); + + final Method decodeFrom = + dtoClass.getMethod("decodeFrom", DirectBuffer.class, int.class, int.class, int.class); + + final Method encodeWith = + dtoClass.getMethod("encodeWithHeaderWith", dtoClass, MutableDirectBuffer.class, int.class); + + final int inputLength = encodedMessage.length(); + final DirectBuffer inputBuffer = encodedMessage.buffer(); + final MessageHeaderDecoder header = new MessageHeaderDecoder().wrap(inputBuffer, 0); + final int blockLength = header.blockLength(); + final int actingVersion = header.version(); + final Object dto = decodeFrom.invoke( + null, + encodedMessage.buffer(), MessageHeaderDecoder.ENCODED_LENGTH, blockLength, actingVersion); + outputBuffer.setMemory(0, outputBuffer.capacity(), (byte)0); + final int outputLength = (int)encodeWith.invoke(null, dto, outputBuffer, 0); + assertEqual(inputBuffer, inputLength, outputBuffer, outputLength); } catch (final Throwable throwable) { - addInputFootnotes(footnotes, encodedMessage); + if (null != footnotes) + { + addSchemaAndInputMessageFootnotes(footnotes, encodedMessage); - final StringBuilder generatedSources = new StringBuilder(); - outputManager.dumpSources(generatedSources); - footnotes.addFootnote(generatedSources.toString()); + final StringBuilder generatedSources = new StringBuilder(); + outputManager.dumpSources(generatedSources); + footnotes.addFootnote(generatedSources.toString()); + } throw throwable; } @@ -176,14 +173,12 @@ void csharpDtoEncodeShouldBeTheInverseOfDtoDecode( if (!Arrays.equals(inputBytes, outputBytes)) { throw new AssertionError( - "Input and output files differ\n\n" + - "DIR:" + tempDir + "\n\n" + - "SCHEMA:\n" + encodedMessage.schema()); + "Input and output files differ\n\nDIR:" + tempDir); } } catch (final Throwable throwable) { - addInputFootnotes(footnotes, encodedMessage); + addSchemaAndInputMessageFootnotes(footnotes, encodedMessage); addGeneratedSourcesFootnotes(footnotes, tempDir, ".cs"); throw throwable; @@ -235,14 +230,12 @@ void cppDtoEncodeShouldBeTheInverseOfDtoDecode( final byte[] outputBytes = Files.readAllBytes(tempDir.resolve("output.dat")); if (!Arrays.equals(inputBytes, outputBytes)) { - throw new AssertionError( - "Input and output files differ\n\n" + - "SCHEMA:\n" + encodedMessage.schema()); + throw new AssertionError("Input and output files differ"); } } catch (final Throwable throwable) { - addInputFootnotes(footnotes, encodedMessage); + addSchemaAndInputMessageFootnotes(footnotes, encodedMessage); addGeneratedSourcesFootnotes(footnotes, tempDir, ".cpp"); throw throwable; @@ -316,7 +309,7 @@ private static void execute( Arbitrary encodedMessage() { final SbeArbitraries.CharGenerationConfig config = - SbeArbitraries.CharGenerationConfig.jsonPrinterCompatibleAndNullTerminates(); + SbeArbitraries.CharGenerationConfig.firstNullTerminatesCharArray(); return SbeArbitraries.encodedMessage(config); } @@ -342,10 +335,23 @@ private static void copyResourceToFile( } } + private static String readResourceFileAsString(final String resourcePath) throws IOException + { + try (InputStream inputStream = DtosPropertyTest.class.getResourceAsStream(resourcePath)) + { + if (inputStream == null) + { + throw new IllegalArgumentException("Resource not found: " + resourcePath); + } + + return new String(inputStream.readAllBytes(), StandardCharsets.UTF_8); + } + } + private void assertEqual( - final ExpandableArrayBuffer inputBuffer, + final DirectBuffer inputBuffer, final int inputLength, - final ExpandableArrayBuffer outputBuffer, + final DirectBuffer outputBuffer, final int outputLength) { final boolean lengthsDiffer = inputLength != outputLength; @@ -399,15 +405,4 @@ private void addGeneratedSourcesFootnotes( LangUtil.rethrowUnchecked(exn); } } - - public void addInputFootnotes(final Footnotes footnotes, final SbeArbitraries.EncodedMessage encodedMessage) - { - final byte[] messageBytes = new byte[encodedMessage.length()]; - encodedMessage.buffer().getBytes(0, messageBytes); - final byte[] base64EncodedMessageBytes = Base64.getEncoder().encode(messageBytes); - - footnotes.addFootnote("Schema:" + System.lineSeparator() + encodedMessage.schema()); - footnotes.addFootnote("Input Message:" + System.lineSeparator() + - new String(base64EncodedMessageBytes, StandardCharsets.UTF_8)); - } } diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java index a20deb3ee5..4837d267c7 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/JsonPropertyTest.java @@ -19,16 +19,23 @@ import net.jqwik.api.ForAll; import net.jqwik.api.Property; import net.jqwik.api.Provide; +import net.jqwik.api.footnotes.EnableFootnotes; +import net.jqwik.api.footnotes.Footnotes; import uk.co.real_logic.sbe.json.JsonPrinter; import uk.co.real_logic.sbe.properties.arbitraries.SbeArbitraries; import org.agrona.concurrent.UnsafeBuffer; import org.json.JSONException; import org.json.JSONObject; +import static uk.co.real_logic.sbe.properties.PropertyTestUtil.addSchemaAndInputMessageFootnotes; + public class JsonPropertyTest { @Property - void shouldGenerateValidJson(@ForAll("encodedMessage") final SbeArbitraries.EncodedMessage encodedMessage) + @EnableFootnotes + void shouldGenerateValidJson( + @ForAll("encodedMessage") final SbeArbitraries.EncodedMessage encodedMessage, + final Footnotes footnotes) { final StringBuilder output = new StringBuilder(); final JsonPrinter printer = new JsonPrinter(encodedMessage.ir()); @@ -39,7 +46,8 @@ void shouldGenerateValidJson(@ForAll("encodedMessage") final SbeArbitraries.Enco } catch (final JSONException e) { - throw new AssertionError("Invalid JSON: " + output + "\n\nSchema:\n" + encodedMessage.schema(), e); + addSchemaAndInputMessageFootnotes(footnotes, encodedMessage); + throw new AssertionError("Invalid JSON: " + output); } } @@ -47,7 +55,7 @@ void shouldGenerateValidJson(@ForAll("encodedMessage") final SbeArbitraries.Enco Arbitrary encodedMessage() { final SbeArbitraries.CharGenerationConfig config = - SbeArbitraries.CharGenerationConfig.jsonPrinterCompatibleAndNullTerminates(); + SbeArbitraries.CharGenerationConfig.unrestricted(); return SbeArbitraries.encodedMessage(config); } } diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/PropertyTestUtil.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/PropertyTestUtil.java new file mode 100644 index 0000000000..e3c8c4e292 --- /dev/null +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/PropertyTestUtil.java @@ -0,0 +1,43 @@ +/* + * Copyright 2013-2025 Real Logic Limited. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package uk.co.real_logic.sbe.properties; + +import net.jqwik.api.footnotes.Footnotes; +import uk.co.real_logic.sbe.properties.arbitraries.SbeArbitraries; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; + +final class PropertyTestUtil +{ + private PropertyTestUtil() + { + } + + static void addSchemaAndInputMessageFootnotes( + final Footnotes footnotes, + final SbeArbitraries.EncodedMessage encodedMessage) + { + final byte[] messageBytes = new byte[encodedMessage.length()]; + encodedMessage.buffer().getBytes(0, messageBytes); + final byte[] base64EncodedMessageBytes = Base64.getEncoder().encode(messageBytes); + + footnotes.addFootnote("Schema:" + System.lineSeparator() + encodedMessage.schema()); + footnotes.addFootnote("Input Message:" + System.lineSeparator() + + new String(base64EncodedMessageBytes, StandardCharsets.UTF_8)); + } +} diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java index dcabb47806..74bd2cc1da 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/arbitraries/SbeArbitraries.java @@ -29,9 +29,11 @@ import uk.co.real_logic.sbe.xml.IrGenerator; import uk.co.real_logic.sbe.xml.ParserOptions; import org.agrona.BitUtil; +import org.agrona.DirectBuffer; import org.agrona.ExpandableArrayBuffer; import org.agrona.MutableDirectBuffer; import org.agrona.collections.MutableInteger; +import org.agrona.concurrent.UnsafeBuffer; import java.io.ByteArrayInputStream; import java.io.InputStream; @@ -116,22 +118,14 @@ private static Arbitrary encodedDataTypeSchema() public static class CharGenerationConfig { - private final boolean alphaOnly; private final boolean nullCharTerminatesData; public CharGenerationConfig( - final boolean alphaOnly, final boolean nullCharTerminatesData) { - this.alphaOnly = alphaOnly; this.nullCharTerminatesData = nullCharTerminatesData; } - boolean alphaOnly() - { - return alphaOnly; - } - boolean nullCharTerminatesData() { return nullCharTerminatesData; @@ -139,12 +133,12 @@ boolean nullCharTerminatesData() public static CharGenerationConfig unrestricted() { - return new CharGenerationConfig(false, false); + return new CharGenerationConfig(false); } - public static CharGenerationConfig jsonPrinterCompatibleAndNullTerminates() + public static CharGenerationConfig firstNullTerminatesCharArray() { - return new CharGenerationConfig(true, true); + return new CharGenerationConfig(true); } } @@ -371,24 +365,28 @@ private static Arbitrary combineArbitraryEncoders(final List encodedTypeEncoder( final Encoding encoding, - final boolean isOptional, - final CharGenerationConfig charGenerationConfig) + final boolean isOptional) { - final Arbitrary inRangeEncoder = encodedTypeEncoder(encoding, charGenerationConfig); + final Arbitrary inRangeEncoder = encodedTypeEncoder(encoding); if (isOptional) { @@ -402,8 +400,7 @@ private static Arbitrary encodedTypeEncoder( } private static Arbitrary encodedTypeEncoder( - final Encoding encoding, - final CharGenerationConfig charGenerationConfig) + final Encoding encoding) { final PrimitiveValue minValue = encoding.applicableMinValue(); final PrimitiveValue maxValue = encoding.applicableMaxValue(); @@ -412,7 +409,7 @@ private static Arbitrary encodedTypeEncoder( { case CHAR: assert minValue.longValue() <= maxValue.longValue(); - return chars(charGenerationConfig).map(c -> + return chars(encoding).map(c -> (builder, buffer, offset, limit) -> { builder.appendLine().append(c).append(" @ ").append(offset) @@ -588,8 +585,7 @@ private static Arbitrary encodedTypeEncoder( final Encoding encoding = typeToken.encoding(); final Arbitrary arbEncoder = encodedTypeEncoder( encoding, - memberToken.isOptionalEncoding(), - charGenerationConfig + memberToken.isOptionalEncoding() ); if (typeToken.arrayLength() == 1) @@ -938,7 +934,7 @@ private static Arbitrary groupsEncoder( final Arbitrary groupElement = Combinators.combine( fieldsEncoder(tokens, cursor, nextFieldIdx - 1, true, charGenerationConfig), groupsEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationConfig), - varDataEncoder(tokens, cursor, nextFieldIdx - 1, charGenerationConfig) + varDataEncoder(tokens, cursor, nextFieldIdx - 1) ).as((fieldsEncoder, groupsEncoder, varDataEncoder) -> (builder, buffer, ignored, limit) -> { @@ -987,8 +983,7 @@ private static Arbitrary groupsEncoder( private static Arbitrary varDataEncoder( final List tokens, final MutableInteger cursor, - final int endIdxInclusive, - final CharGenerationConfig charGenerationConfig) + final int endIdxInclusive) { final List> encoders = new ArrayList<>(); @@ -1011,7 +1006,7 @@ private static Arbitrary varDataEncoder( final String characterEncoding = varDataToken.encoding().characterEncoding(); final Arbitrary arbitraryByte = null == characterEncoding ? Arbitraries.bytes() : - chars(charGenerationConfig).map(c -> (byte)c.charValue()); + chars(varDataToken.encoding()).map(c -> (byte)c.charValue()); encoders.add(arbitraryByte.list() .ofMaxSize((int)Math.min(lengthToken.encoding().applicableMaxValue().longValue(), 260L)) .map(bytes -> (builder, buffer, ignored, limit) -> @@ -1061,7 +1056,7 @@ private static Arbitrary messageValueEncoder( final Arbitrary groupsEncoder = groupsEncoder( tokens, cursor, tokens.size() - 1, charGenerationConfig); final Arbitrary varDataEncoder = varDataEncoder( - tokens, cursor, tokens.size() - 1, charGenerationConfig); + tokens, cursor, tokens.size() - 1); return Combinators.combine(fieldsEncoder, groupsEncoder, varDataEncoder) .as((fields, groups, varData) -> (builder, buffer, offset, limit) -> { @@ -1085,14 +1080,14 @@ public static final class EncodedMessage { private final String schema; private final Ir ir; - private final ExpandableArrayBuffer buffer; + private final DirectBuffer buffer; private final int length; private final String encodingLog; private EncodedMessage( final String schema, final Ir ir, - final ExpandableArrayBuffer buffer, + final DirectBuffer buffer, final int length, final String encodingLog) { @@ -1113,7 +1108,7 @@ public Ir ir() return ir; } - public ExpandableArrayBuffer buffer() + public DirectBuffer buffer() { return buffer; } @@ -1123,10 +1118,58 @@ public int length() return length; } + /** + * A log of the steps taken to encode the input message. This information is useful for debugging problems in + * the arbitrary message generator. + * + * @return a log of the steps taken to encode the message + */ + @SuppressWarnings("unused") public String encodingLog() { return encodingLog; } + + /** + * Takes the XML schema and Base64 input message from a (possibly shrunken) example and creates an + * {@link EncodedMessage} instance that can be used to drive property-based test logic. This data is output + * when a test fails. + * + * @param schemaXml the schema to use + * @param inputMessage the input message (as a base64 encoded string) + * @return an encoded message for use with property-based tests + */ + @SuppressWarnings("unused") + public static EncodedMessage fromDebugOutput( + final String schemaXml, + final String inputMessage) + { + try (InputStream in = new ByteArrayInputStream(schemaXml.getBytes(StandardCharsets.UTF_8))) + { + final ParserOptions options = ParserOptions.builder() + .suppressOutput(false) + .warningsFatal(true) + .stopOnError(true) + .build(); + final uk.co.real_logic.sbe.xml.MessageSchema parsedSchema = parse(in, options); + final Ir ir = new IrGenerator().generate(parsedSchema); + + final byte[] messageBytes = Base64.getDecoder().decode(inputMessage); + final UnsafeBuffer buffer = new UnsafeBuffer(messageBytes); + + return new EncodedMessage( + schemaXml, + ir, + buffer, + buffer.capacity(), + "" + ); + } + catch (final Exception exception) + { + throw new RuntimeException(exception); + } + } } public static Arbitrary encodedMessage(final CharGenerationConfig charGenerationConfig) diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/schema/TestXmlSchemaWriter.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/schema/TestXmlSchemaWriter.java index 5dc4569db2..141e7e469b 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/schema/TestXmlSchemaWriter.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/schema/TestXmlSchemaWriter.java @@ -15,12 +15,17 @@ */ package uk.co.real_logic.sbe.properties.schema; +import uk.co.real_logic.sbe.ir.Encoding; import org.agrona.collections.MutableInteger; import org.w3c.dom.Document; import org.w3c.dom.Element; import org.w3c.dom.Node; -import uk.co.real_logic.sbe.ir.Encoding; +import java.io.File; +import java.io.StringWriter; +import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.OutputKeys; @@ -28,11 +33,6 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; -import java.io.File; -import java.io.StringWriter; -import java.util.*; -import java.util.function.Function; -import java.util.stream.Collectors; import static java.util.Objects.requireNonNull; @@ -355,7 +355,6 @@ private static void appendTypes( } } - @SuppressWarnings("EnhancedSwitchMigration") private static final class TypeSchemaConverter implements TypeSchemaVisitor { private final Document document; @@ -465,7 +464,8 @@ public Node convert(final VarDataSchema varData) final Element varDataElement = createTypeElement(document, "varData", "uint8"); varDataElement.setAttribute("length", "0"); - if (varData.dataEncoding().equals(VarDataSchema.Encoding.ASCII)) + final VarDataSchema.Encoding encoding = varData.dataEncoding(); + if (encoding.equals(VarDataSchema.Encoding.ASCII)) { varDataElement.setAttribute("characterEncoding", "US-ASCII"); } diff --git a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/utils/InMemoryOutputManager.java b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/utils/InMemoryOutputManager.java index 065737c57e..403c64b068 100644 --- a/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/utils/InMemoryOutputManager.java +++ b/sbe-tool/src/propertyTest/java/uk/co/real_logic/sbe/properties/utils/InMemoryOutputManager.java @@ -15,17 +15,14 @@ */ package uk.co.real_logic.sbe.properties.utils; +import org.agrona.generation.CompilerUtil; import org.agrona.generation.DynamicPackageOutputManager; -import javax.tools.*; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.StringWriter; import java.io.Writer; -import java.net.URI; -import java.net.URL; -import java.net.URLClassLoader; -import java.util.*; +import java.util.HashMap; +import java.util.Map; /** * An implementation of {@link DynamicPackageOutputManager} that stores generated source code in memory and compiles it @@ -34,7 +31,7 @@ public class InMemoryOutputManager implements DynamicPackageOutputManager { private final String packageName; - private final Map sourceFiles = new HashMap<>(); + private final Map sources = new HashMap<>(); private String packageNameOverride; public InMemoryOutputManager(final String packageName) @@ -53,42 +50,32 @@ public void setPackageName(final String packageName) } /** - * Compile the generated sources and return a {@link URLClassLoader} that can be used to load the generated classes. + * Compile the generated sources and return a {@link Class} matching the supplied fully-qualified name. * - * @return a {@link URLClassLoader} that can be used to load the generated classes + * @param fqClassName the fully-qualified class name to compile and load. + * @return a {@link Class} matching the supplied fully-qualified name. */ - public URLClassLoader compileGeneratedSources() + public Class compileAndLoad(final String fqClassName) { - final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); - final StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, null); - final InMemoryFileManager fileManager = new InMemoryFileManager(standardFileManager); - final JavaCompiler.CompilationTask task = compiler.getTask( - null, - fileManager, - null, - null, - null, - sourceFiles.values()); - - if (!task.call()) + try { - throw new IllegalStateException("Compilation failed"); + return CompilerUtil.compileInMemory(fqClassName, sources); + } + catch (final Exception exception) + { + throw new RuntimeException(exception); } - - final GeneratedCodeLoader classLoader = new GeneratedCodeLoader(getClass().getClassLoader()); - classLoader.defineClasses(fileManager); - return classLoader; } public void dumpSources(final StringBuilder builder) { - builder.append(System.lineSeparator()).append("Generated sources file count: ").append(sourceFiles.size()) + builder.append(System.lineSeparator()).append("Generated sources file count: ").append(sources.size()) .append(System.lineSeparator()); - sourceFiles.forEach((qualifiedName, file) -> + sources.forEach((qualifiedName, source) -> { builder.append(System.lineSeparator()).append("Source file: ").append(qualifiedName) - .append(System.lineSeparator()).append(file.sourceCode) + .append(System.lineSeparator()).append(source) .append(System.lineSeparator()); }); } @@ -109,93 +96,14 @@ public void close() throws IOException packageNameOverride = null; final String qualifiedName = actingPackageName + "." + name; - final InMemoryJavaFileObject sourceFile = - new InMemoryJavaFileObject(qualifiedName, getBuffer().toString()); - final InMemoryJavaFileObject existingFile = sourceFiles.putIfAbsent(qualifiedName, sourceFile); + final String source = getBuffer().toString(); + final CharSequence existingSource = sources.putIfAbsent(qualifiedName, source); - if (existingFile != null && !Objects.equals(existingFile.sourceCode, sourceFile.sourceCode)) + if (null != existingSource && 0 != CharSequence.compare(existingSource, source)) { throw new IllegalStateException("Duplicate (but different) class: " + qualifiedName); } } } - - static class InMemoryFileManager extends ForwardingJavaFileManager - { - private final List outputFiles = new ArrayList<>(); - - InMemoryFileManager(final StandardJavaFileManager fileManager) - { - super(fileManager); - } - - public JavaFileObject getJavaFileForOutput( - final Location location, - final String className, - final JavaFileObject.Kind kind, - final FileObject sibling) - { - final InMemoryJavaFileObject outputFile = new InMemoryJavaFileObject(className, kind); - outputFiles.add(outputFile); - return outputFile; - } - - public Collection outputFiles() - { - return outputFiles; - } - } - - static class InMemoryJavaFileObject extends SimpleJavaFileObject - { - private final String sourceCode; - private final ByteArrayOutputStream outputStream; - - InMemoryJavaFileObject(final String className, final String sourceCode) - { - super(URI.create("string:///" + className.replace('.', '/') + Kind.SOURCE.extension), Kind.SOURCE); - this.sourceCode = sourceCode; - this.outputStream = new ByteArrayOutputStream(); - } - - InMemoryJavaFileObject(final String className, final Kind kind) - { - super(URI.create("mem:///" + className.replace('.', '/') + kind.extension), kind); - this.sourceCode = null; - this.outputStream = new ByteArrayOutputStream(); - } - - public CharSequence getCharContent(final boolean ignoreEncodingErrors) - { - return sourceCode; - } - - public ByteArrayOutputStream openOutputStream() - { - return outputStream; - } - - public byte[] getClassBytes() - { - return outputStream.toByteArray(); - } - } - - static class GeneratedCodeLoader extends URLClassLoader - { - GeneratedCodeLoader(final ClassLoader parent) - { - super(new URL[0], parent); - } - - void defineClasses(final InMemoryFileManager fileManager) - { - fileManager.outputFiles().forEach(file -> - { - final byte[] classBytes = file.getClassBytes(); - super.defineClass(null, classBytes, 0, classBytes.length); - }); - } - } }