Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Java, C#, C++] Fix precedence checks to account for non-contiguous versions. #989

Merged
merged 1 commit into from Apr 11, 2024
Merged

[Java, C#, C++] Fix precedence checks to account for non-contiguous versions. #989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions csharp/sbe-tests/FieldAccessOrderCheckTests.cs
View file
Open in desktop
Expand Up @@ -3159,6 +3159,25 @@ public void DisallowsIncompleteMessagesDueToMissingNestedGroup1(int bCount, stri
var exception = Assert.ThrowsException<InvalidOperationException>(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()
{
Expand Down
10 changes: 4 additions & 6 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/common/FieldPrecedenceModel.java
View file
Open in desktop
Expand Up @@ -46,7 +46,7 @@ public final class FieldPrecedenceModel
new CodecInteraction.CodecInteractionFactory(groupPathsByField, topLevelBlockFields);
private final Map<CodecInteraction, List<TransitionGroup>> transitionsByInteraction = new LinkedHashMap<>();
private final Map<State, List<TransitionGroup>> transitionsByState = new HashMap<>();
private final Int2ObjectHashMap<State> versionWrappedStates = new Int2ObjectHashMap<>();
private final TreeMap<Integer, State> versionWrappedStates = new TreeMap<>();
private final State notWrappedState = allocateState("NOT_WRAPPED");
private final String generatedRepresentationClassName;
private State encoderWrappedState;
Expand Down Expand Up @@ -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<State> consumer)
public void forEachWrappedStateByVersionDesc(final IntObjConsumer<State> consumer)
{
final Int2ObjectHashMap<State>.EntryIterator iterator = versionWrappedStates.entrySet().iterator();
while (iterator.hasNext())
for (final Map.Entry<Integer, State> entry : versionWrappedStates.descendingMap().entrySet())
{
iterator.next();
consumer.accept(iterator.getIntKey(), iterator.getValue());
consumer.accept(entry.getKey(), entry.getValue());
}
}

Expand Down
47 changes: 31 additions & 16 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/cpp/CppGenerator.java
View file
Open in desktop
Expand Up @@ -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;
}
Expand Down
22 changes: 10 additions & 12 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/csharp/CSharpGenerator.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down
21 changes: 9 additions & 12 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down
12 changes: 5 additions & 7 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/FrameCodecDecoder.java
View file
Open in desktop
Expand Up @@ -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(
Expand Down
12 changes: 5 additions & 7 deletions sbe-tool/src/main/java/uk/co/real_logic/sbe/ir/generated/TokenCodecDecoder.java
View file
Open in desktop
Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions sbe-tool/src/test/cpp/FieldAccessOrderCheckTest.cpp
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
}
115 changes: 113 additions & 2 deletions sbe-tool/src/test/java/uk/co/real_logic/sbe/FieldAccessOrderCheckTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}