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

HY-51: decoder reset missing to set next to null in groups decoder,… #528

Merged
merged 1 commit into from
Nov 7, 2024
Merged

HY-51: decoder reset missing to set next to null in groups decoder,… #528

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions ...o-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/DecoderGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,21 +416,28 @@ protected String resetGroup(final Entry entry)
" {\n" +
" for (final %2$s %6$s : %5$s.iterator())\n" +
" {\n" +
" %6$s.reset();\n" +
" if (%6$s.next() == null)\n" +
" {\n" +
" %6$s.reset();\n" +
" break;\n" +
" }\n" +
" else\n" +
" {\n" +
" %6$s.reset();\n" +
" }\n" +
" }\n" +
" %3$s = MISSING_INT;\n" +
" has%4$s = false;\n" +
" %7$s = null;\n" +
" }\n\n",
resetMethod,
decoderClassName(name),
formatPropertyName(numberField.name()),
numberField.name(),
iteratorFieldName(group),
formatPropertyName(decoderClassName(name)));
formatPropertyName(decoderClassName(name)),
formatPropertyName(name)
);
}
}

Expand Down Expand Up @@ -484,6 +491,9 @@ private String additionalReset(final boolean isGroup)
{
return
" buffer = null;\n" +
(isGroup ?
" next = null;\n" : ""
) +
" if (" + CODEC_VALIDATION_ENABLED + ")\n" +
" {\n" +
" invalidTagId = Decoder.NO_ERROR;\n" +
Expand Down
2 changes: 1 addition & 1 deletion artio-codecs/src/test/java/uk/co/real_logic/artio/dictionary/ExampleDictionary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public final class ExampleDictionary
"8=FIX.4.4\0019=77\00135=0\001115=abc\001116=2\001117=1.1\001127=19700101-00:00:00.001" +
"\001120=2\001121=1\001122=2\001123=1\001123=2\001121=2\001122=2\001123=3\001123=4\00110=063\001";

public static final String MULTI_ENTRY_NESTED_GROUP_MESSAGE_WITHOUT_NESTED_FIELDS =
public static final String MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS =
"8=FIX.4.4\0019=77\00135=0\001115=abc\001116=2\001117=1.1\001127=19700101-00:00:00.001" +
"\001120=2\001121=1\001121=2\00110=063\001";

Expand Down
48 changes: 35 additions & 13 deletions .../test/java/uk/co/real_logic/artio/dictionary/generation/AbstractDecoderGeneratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.function.Consumer;
import java.util.function.Function;

import static java.lang.reflect.Modifier.isAbstract;
Expand Down Expand Up @@ -838,14 +839,21 @@ public void shouldReasonablyValidateGroupNumbersLessThanTheNumberOfElementsInThe
assertInvalid(decoder, INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP, 120);
}

//TODO this test was added to guide the bugfix
//this test was added for issue #525
@Test
public void shouldReasonablyValidateGroupNumbersLessThanTheNumberOfElementsInTheGroupList() throws Exception
{
final Decoder decoder = decodeHeartbeatWithRejectingUnknownFields(
REPEATING_GROUP_MESSAGE_WITH_THREE, REPEATING_GROUP_MESSAGE_WITH_TOO_HIGH_NUMBER_FIELD);
final Decoder decoder = (Decoder)heartbeatWithRejectingUnknownFields.getConstructor().newInstance();

// assertInvalid(decoder, INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP, 120);
decodeHeartbeatWithRejectingUnknownFields(
decoder, this::assertValid, REPEATING_GROUP_MESSAGE_WITH_THREE);
decodeHeartbeatWithRejectingUnknownFields(
decoder, decoder1 ->
assertInvalid(decoder1,
INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP,
120), REPEATING_GROUP_MESSAGE_WITH_TOO_HIGH_NUMBER_FIELD);
decodeHeartbeatWithRejectingUnknownFields(
decoder, this::assertValid, REPEATING_GROUP_MESSAGE_WITH_THREE);
}

@Test
Expand Down Expand Up @@ -1167,15 +1175,29 @@ public void shouldResetAllNestedRepeatingGroupEntries() throws Exception

decoder.reset();

decode(MULTI_ENTRY_NESTED_GROUP_MESSAGE_WITHOUT_NESTED_FIELDS, decoder);
decode(MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS, decoder);
assertEquals(2, getNoEgGroupGroupCounter(decoder));

group = getEgGroup(decoder);
assertNull(getNestedGroup(group));

group = next(group);
assertNull(getNestedGroup(group));
}

@Test
public void shouldHaveAllNestedRepeatingGroupEntriesNullifiedWhenMessageDoesNotHaveIt() throws Exception
{
Object group;

assertNestedRepeating(group, 1, CodecUtil.MISSING_INT, CodecUtil.MISSING_INT);
final Decoder decoder = decodeHeartbeat(MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS);
assertEquals(2, getNoEgGroupGroupCounter(decoder));

group = getEgGroup(decoder);
assertNull(getNestedGroup(group));

group = next(group);
assertNestedRepeating(group, 2, CodecUtil.MISSING_INT, CodecUtil.MISSING_INT);
assertNull(getNestedGroup(group));
}

@Test
Expand Down Expand Up @@ -1889,13 +1911,12 @@ Decoder decodeHeartbeatWithoutValidation(final String example) throws Exception
return decoder;
}

private Decoder decodeHeartbeatWithRejectingUnknownFields(final String... example) throws Exception
private Decoder decodeHeartbeatWithRejectingUnknownFields(final Decoder decoder,
final Consumer<Decoder> consumer,
final String example)
{
final Decoder decoder = (Decoder)heartbeatWithRejectingUnknownFields.getConstructor().newInstance();
for (final String s : example)
{
decode(s, decoder);
}
decode(example, decoder);
consumer.accept(decoder);
return decoder;
}

Expand All @@ -1909,6 +1930,7 @@ private Decoder decodeHeartbeatWithRejectingUnknownFields(final String example)
void decode(final String example, final Decoder decoder)
{
buffer.putAscii(1, example);
decoder.reset();
decoder.decode(buffer, 1, example.length());
}

Expand Down