diff --git a/csharp/sbe-tests/FieldAccessOrderCheckTests.cs b/csharp/sbe-tests/FieldAccessOrderCheckTests.cs index d0b7e1699..1ee456964 100644 --- a/csharp/sbe-tests/FieldAccessOrderCheckTests.cs +++ b/csharp/sbe-tests/FieldAccessOrderCheckTests.cs @@ -3159,6 +3159,25 @@ public void DisallowsIncompleteMessagesDueToMissingNestedGroup1(int bCount, stri var exception = Assert.ThrowsException(encoder.CheckEncodingIsComplete); StringAssert.Contains(exception.Message, $"Not fully encoded, current state: {expectedState}"); } + + [TestMethod] + public void AllowsSkippingFutureGroupWhenDecodingFromVersionWithNoChangesInMessage() + { + var encoder = new AddGroupBeforeVarDataV0() + .WrapForEncodeAndApplyHeader(_buffer, Offset, _messageHeader); + + _messageHeader.TemplateId = SkipVersionAddGroupBeforeVarDataV2.TemplateId; + _messageHeader.Version = 1; + + encoder.A = 42; + encoder.SetB("abc"); + + var decoder = new SkipVersionAddGroupBeforeVarDataV2() + .WrapForDecodeAndApplyHeader(_buffer, Offset, _messageHeader); + + Assert.AreEqual(42, decoder.A); + Assert.AreEqual(decoder.GetB(), "abc"); + } private void ModifyHeaderToLookLikeVersion0() { diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/FieldPrecedenceModel.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/FieldPrecedenceModel.java index 3122926ba..58a68c17f 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/FieldPrecedenceModel.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/FieldPrecedenceModel.java @@ -46,7 +46,7 @@ public final class FieldPrecedenceModel new CodecInteraction.CodecInteractionFactory(groupPathsByField, topLevelBlockFields); private final Map> transitionsByInteraction = new LinkedHashMap<>(); private final Map> transitionsByState = new HashMap<>(); - private final Int2ObjectHashMap versionWrappedStates = new Int2ObjectHashMap<>(); + private final TreeMap versionWrappedStates = new TreeMap<>(); private final State notWrappedState = allocateState("NOT_WRAPPED"); private final String generatedRepresentationClassName; private State encoderWrappedState; @@ -103,13 +103,11 @@ public State latestVersionWrappedState() * Iterates over the states after a codec is wrapped over a particular version of data. * @param consumer the consumer of the states. */ - public void forEachWrappedStateByVersion(final IntObjConsumer consumer) + public void forEachWrappedStateByVersionDesc(final IntObjConsumer consumer) { - final Int2ObjectHashMap.EntryIterator iterator = versionWrappedStates.entrySet().iterator(); - while (iterator.hasNext()) + for (final Map.Entry entry : versionWrappedStates.descendingMap().entrySet()) { - iterator.next(); - consumer.accept(iterator.getIntKey(), iterator.getValue()); + consumer.accept(entry.getKey(), entry.getValue()); } } diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java index 54a168337..5b5e6f072 100755 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java @@ -3055,22 +3055,37 @@ private static CharSequence generateDecoderWrapListener(final FieldPrecedenceMod final StringBuilder sb = new StringBuilder(); sb.append(INDENT).append("void onWrapForDecode(std::uint64_t actingVersion)\n") - .append(INDENT).append("{\n") - .append(INDENT).append(INDENT).append("switch(actingVersion)\n") - .append(INDENT).append(INDENT).append("{\n"); - - fieldPrecedenceModel.forEachWrappedStateByVersion((version, state) -> - sb.append(INDENT).append(TWO_INDENT).append("case ").append(version).append(":\n") - .append(INDENT).append(THREE_INDENT).append("codecState(") - .append(qualifiedStateCase(state)).append(");\n") - .append(INDENT).append(THREE_INDENT).append("break;\n")); - - sb.append(INDENT).append(TWO_INDENT).append("default:\n") - .append(INDENT).append(THREE_INDENT).append("codecState(") - .append(qualifiedStateCase(fieldPrecedenceModel.latestVersionWrappedState())).append(");\n") - .append(INDENT).append(THREE_INDENT).append("break;\n") - .append(INDENT).append(INDENT).append("}\n") - .append(INDENT).append("}\n\n"); + .append(INDENT).append("{\n"); + + final MutableBoolean actingVersionCanBeTooLowToBeValid = new MutableBoolean(true); + + fieldPrecedenceModel.forEachWrappedStateByVersionDesc((version, state) -> + { + if (version == 0) + { + actingVersionCanBeTooLowToBeValid.set(false); + + sb.append(INDENT).append(" codecState(") + .append(qualifiedStateCase(state)).append(");\n"); + } + else + { + sb.append(INDENT).append(" if (actingVersion >= ").append(version).append(")\n") + .append(INDENT).append(" {\n") + .append(INDENT).append(" codecState(") + .append(qualifiedStateCase(state)).append(");\n") + .append(INDENT).append(" return;\n") + .append(INDENT).append(" }\n\n"); + } + }); + + if (actingVersionCanBeTooLowToBeValid.get()) + { + sb.append(INDENT) + .append(" throw std::runtime_error(\"Unsupported acting version: \" + actingVersion);\n"); + } + + sb.append(INDENT).append("}\n\n"); return sb; } diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpGenerator.java index f141c7876..8d1502181 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpGenerator.java @@ -1912,22 +1912,20 @@ private static CharSequence generateDecoderWrapListener( } final StringBuilder sb = new StringBuilder(); + sb.append(indent).append("private void OnWrapForDecode(int actingVersion)\n") - .append(indent).append("{\n") - .append(indent).append(INDENT).append("switch(actingVersion)\n") - .append(indent).append(INDENT).append("{\n"); + .append(indent).append("{\n"); - fieldPrecedenceModel.forEachWrappedStateByVersion((version, state) -> - sb.append(indent).append(TWO_INDENT).append("case ").append(version).append(":\n") - .append(indent).append(THREE_INDENT).append("codecState(") + fieldPrecedenceModel.forEachWrappedStateByVersionDesc((version, state) -> + sb.append(indent).append(" if (actingVersion >= ").append(version).append(")\n") + .append(indent).append(" {\n") + .append(indent).append(" codecState(") .append(qualifiedStateCase(state)).append(");\n") - .append(indent).append(THREE_INDENT).append("break;\n")); + .append(indent).append(" return;\n") + .append(indent).append(" }\n\n")); - sb.append(indent).append(TWO_INDENT).append("default:\n") - .append(indent).append(THREE_INDENT).append("codecState(") - .append(qualifiedStateCase(fieldPrecedenceModel.latestVersionWrappedState())).append(");\n") - .append(indent).append(THREE_INDENT).append("break;\n") - .append(indent).append(INDENT).append("}\n") + sb.append(indent) + .append(" throw new InvalidOperationException(\"Unsupported acting version: \" + actingVersion);\n") .append(indent).append("}\n\n"); return sb; diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java index 8009f7a07..1a5e17bb2 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java @@ -762,21 +762,18 @@ private static CharSequence generateDecoderWrapListener( final StringBuilder sb = new StringBuilder(); sb.append(indent).append("private void onWrap(final int actingVersion)\n") - .append(indent).append("{\n") - .append(indent).append(" switch(actingVersion)\n") - .append(indent).append(" {\n"); + .append(indent).append("{\n"); - fieldPrecedenceModel.forEachWrappedStateByVersion((version, state) -> - sb.append(indent).append(" case ").append(version).append(":\n") - .append(indent).append(" codecState(") + fieldPrecedenceModel.forEachWrappedStateByVersionDesc((version, state) -> + sb.append(indent).append(" if (actingVersion >= ").append(version).append(")\n") + .append(indent).append(" {\n") + .append(indent).append(" codecState(") .append(qualifiedStateCase(state)).append(");\n") - .append(indent).append(" break;\n")); + .append(indent).append(" return;\n") + .append(indent).append(" }\n\n")); - sb.append(indent).append(" default:\n") - .append(indent).append(" codecState(") - .append(qualifiedStateCase(fieldPrecedenceModel.latestVersionWrappedState())).append(");\n") - .append(indent).append(" break;\n") - .append(indent).append(" }\n") + sb.append(indent) + .append(" throw new IllegalStateException(\"Unsupported acting version: \" + actingVersion);\n") .append(indent).append("}\n\n"); return sb; diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java index 12bdde9ae..356354065 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java @@ -139,15 +139,13 @@ public int offset() private void onWrap(final int actingVersion) { - switch(actingVersion) + if (actingVersion >= 0) { - case 0: - codecState(CodecStates.V0_BLOCK); - break; - default: - codecState(CodecStates.V0_BLOCK); - break; + codecState(CodecStates.V0_BLOCK); + return; } + + throw new IllegalStateException("Unsupported acting version: " + actingVersion); } public FrameCodecDecoder wrap( diff --git a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java index e6e0fba7b..7b2e69831 100644 --- a/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java +++ b/sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java @@ -186,15 +186,13 @@ public int offset() private void onWrap(final int actingVersion) { - switch(actingVersion) + if (actingVersion >= 0) { - case 0: - codecState(CodecStates.V0_BLOCK); - break; - default: - codecState(CodecStates.V0_BLOCK); - break; + codecState(CodecStates.V0_BLOCK); + return; } + + throw new IllegalStateException("Unsupported acting version: " + actingVersion); } public TokenCodecDecoder wrap( diff --git a/sbe-tool/src/test/cpp/FieldAccessOrderCheckTest.cpp b/sbe-tool/src/test/cpp/FieldAccessOrderCheckTest.cpp index ecd3df133..ef2b58d75 100644 --- a/sbe-tool/src/test/cpp/FieldAccessOrderCheckTest.cpp +++ b/sbe-tool/src/test/cpp/FieldAccessOrderCheckTest.cpp @@ -51,6 +51,7 @@ #include "order_check/NoBlock.h" #include "order_check/GroupWithNoBlock.h" #include "order_check/NestedGroupWithVarLength.h" +#include "order_check/SkipVersionAddGroupBeforeVarDataV2.h" using namespace order::check; using ::testing::HasSubstr; @@ -4935,3 +4936,22 @@ INSTANTIATE_TEST_SUITE_P( std::make_tuple(2, 2, "V0_B_N_D_N_BLOCK") ) ); + +TEST_F(FieldAccessOrderCheckTest, allowsSkippingFutureGroupWhenDecodingFromVersionWithNoChangesInMessagePart1) +{ + AddGroupBeforeVarDataV0 encoder; + encoder.wrapForEncode(m_buffer, OFFSET, BUFFER_LEN); + encoder.a(42).putB("abc"); + + SkipVersionAddGroupBeforeVarDataV2 decoder; + decoder.wrapForDecode( + m_buffer, + OFFSET, + AddGroupBeforeVarDataV0::sbeBlockLength(), + 1, + BUFFER_LEN + ); + + EXPECT_EQ(decoder.a(), 42); + EXPECT_EQ(decoder.getBAsString(), "abc"); +} diff --git a/sbe-tool/src/test/java/uk/co/real_logic/sbe/FieldAccessOrderCheckTest.java b/sbe-tool/src/test/java/uk/co/real_logic/sbe/FieldAccessOrderCheckTest.java index 80a4ba441..6ac554b8a 100644 --- a/sbe-tool/src/test/java/uk/co/real_logic/sbe/FieldAccessOrderCheckTest.java +++ b/sbe-tool/src/test/java/uk/co/real_logic/sbe/FieldAccessOrderCheckTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; import java.nio.charset.StandardCharsets; import java.util.Random; @@ -3407,6 +3408,116 @@ void disallowsIncompleteMessagesDueToMissingVarDataInNestedGroup( assertThat(exception.getMessage(), containsString("Not fully encoded, current state: " + expectedState)); } + @Test + void decodesNullValueForFutureVersionBlockFieldWhenCurrentVersionHasNoChangesInMessage() + { + final MessageHeaderEncoder headerEncoder = new MessageHeaderEncoder(); + final SkipVersionAddPrimitiveV1Encoder encoder = new SkipVersionAddPrimitiveV1Encoder() + .wrapAndApplyHeader(buffer, OFFSET, headerEncoder); + + headerEncoder + .templateId(SkipVersionAddPrimitiveV2Encoder.TEMPLATE_ID) + .version(1); + + encoder.b("abc"); + + final SkipVersionAddPrimitiveV2Decoder decoder = new SkipVersionAddPrimitiveV2Decoder() + .wrapAndApplyHeader(buffer, OFFSET, messageHeaderDecoder); + + assertThat(decoder.a(), equalTo(SkipVersionAddPrimitiveV2Decoder.aNullValue())); + assertThat(decoder.b(), equalTo("abc")); + } + + @Test + void decodesEmptyFutureGroupWhenDecodingFromVersionWithNoChangesInMessage() + { + final MessageHeaderEncoder headerEncoder = new MessageHeaderEncoder(); + final SkipVersionAddGroupV1Encoder encoder = new SkipVersionAddGroupV1Encoder() + .wrapAndApplyHeader(buffer, OFFSET, headerEncoder); + + headerEncoder + .templateId(SkipVersionAddGroupV2Decoder.TEMPLATE_ID) + .version(1); + + encoder.a(42); + + final SkipVersionAddGroupV2Decoder decoder = new SkipVersionAddGroupV2Decoder() + .wrapAndApplyHeader(buffer, OFFSET, messageHeaderDecoder); + + assertThat(decoder.a(), equalTo(42)); + assertThat(decoder.b().count(), equalTo(0)); + } + + @Test + void decodesNullValueForFutureVarDataFieldWhenDecodingFromVersionWithNoChangesInMessage() + { + final MessageHeaderEncoder headerEncoder = new MessageHeaderEncoder(); + final SkipVersionAddVarDataV1Encoder encoder = new SkipVersionAddVarDataV1Encoder() + .wrapAndApplyHeader(buffer, OFFSET, headerEncoder); + + headerEncoder + .templateId(SkipVersionAddVarDataV2Decoder.TEMPLATE_ID) + .version(1); + + encoder.a(42); + + final SkipVersionAddVarDataV2Decoder decoder = new SkipVersionAddVarDataV2Decoder() + .wrapAndApplyHeader(buffer, OFFSET, messageHeaderDecoder); + + assertThat(decoder.a(), equalTo(42)); + assertThat(decoder.b(), equalTo("")); + } + + @Test + void allowsDecodingUnknownFutureVersions() + { + final MessageHeaderEncoder headerEncoder = new MessageHeaderEncoder(); + final SkipVersionAddPrimitiveV2Encoder encoder = new SkipVersionAddPrimitiveV2Encoder() + .wrapAndApplyHeader(buffer, OFFSET, headerEncoder); + + headerEncoder + .templateId(SkipVersionAddPrimitiveV2Encoder.TEMPLATE_ID) + .version(42); + + encoder.a(43).b("abc"); + + final SkipVersionAddPrimitiveV2Decoder decoder = new SkipVersionAddPrimitiveV2Decoder() + .wrapAndApplyHeader(buffer, OFFSET, messageHeaderDecoder); + + assertThat(decoder.a(), equalTo(43)); + assertThat(decoder.b(), equalTo("abc")); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void allowsSkippingFutureGroupWhenDecodingFromVersionWithNoChangesInMessage(final boolean decodeGroup) + { + final MessageHeaderEncoder headerEncoder = new MessageHeaderEncoder(); + final AddGroupBeforeVarDataV0Encoder encoder = new AddGroupBeforeVarDataV0Encoder() + .wrapAndApplyHeader(buffer, OFFSET, headerEncoder); + + headerEncoder + .templateId(SkipVersionAddGroupBeforeVarDataV2Decoder.TEMPLATE_ID) + .version(1); + + encoder.a(42).b("abc"); + + final SkipVersionAddGroupBeforeVarDataV2Decoder decoder = new SkipVersionAddGroupBeforeVarDataV2Decoder() + .wrapAndApplyHeader(buffer, OFFSET, messageHeaderDecoder); + + assertThat(decoder.a(), equalTo(42)); + + if (decodeGroup) + { + for (final SkipVersionAddGroupBeforeVarDataV2Decoder.CDecoder group : decoder.c()) + { + group.sbeSkip(); + } + } + + assertThat(decoder.b(), equalTo("abc")); + } + private void modifyHeaderToLookLikeVersion0() { messageHeaderDecoder.wrap(buffer, OFFSET); @@ -3418,9 +3529,9 @@ private void modifyHeaderToLookLikeVersion0() private void modifyHeaderToLookLikeVersion1() { messageHeaderDecoder.wrap(buffer, OFFSET); - assert messageHeaderDecoder.version() == 1; + assert messageHeaderDecoder.version() >= 1; final int v0TemplateId = messageHeaderDecoder.templateId() - 1_000; messageHeaderEncoder.wrap(buffer, OFFSET); - messageHeaderEncoder.templateId(v0TemplateId); + messageHeaderEncoder.templateId(v0TemplateId).version(1); } } diff --git a/sbe-tool/src/test/resources/field-order-check-schema.xml b/sbe-tool/src/test/resources/field-order-check-schema.xml index e4d62ebfd..8bedfd4b3 100644 --- a/sbe-tool/src/test/resources/field-order-check-schema.xml +++ b/sbe-tool/src/test/resources/field-order-check-schema.xml @@ -4,7 +4,7 @@ id="1" description="Unit Test" byteOrder="littleEndian" - version="1"> + version="3"> @@ -141,6 +141,7 @@ + @@ -289,4 +290,43 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +