Suppress "this-escape" warnings from generated encoders and decoders

artio-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/CodecConfiguration.java

@@ -52,6 +52,7 @@ public final class CodecConfiguration
      * without checking whether the field is set.
      */
     public static final String WRAP_EMPTY_BUFFER = "fix.codecs.wrap_empty_buffer";
+    public static final String ALLOW_EMPTY_TAGS = "fix.codecs.allow_empty_tags";
     public static final String PARENT_PACKAGE_PROPERTY = "fix.codecs.parent_package";
     public static final String FLYWEIGHTS_ENABLED_PROPERTY = "fix.codecs.flyweight";
     public static final String REJECT_UNKNOWN_ENUM_VALUE_PROPERTY = "reject.unknown.enum.value";
@@ -63,6 +64,7 @@ public final class CodecConfiguration
     private String parentPackage = System.getProperty(PARENT_PACKAGE_PROPERTY, DEFAULT_PARENT_PACKAGE);
     private boolean flyweightsEnabled = Boolean.getBoolean(FLYWEIGHTS_ENABLED_PROPERTY);
     private boolean wrapEmptyBuffer = Boolean.getBoolean(WRAP_EMPTY_BUFFER);
+    private boolean allowEmptyTags = Boolean.getBoolean(ALLOW_EMPTY_TAGS);
     private boolean fixTagsInJavadoc = Boolean.parseBoolean(System.getProperty(
         FIX_TAGS_IN_JAVADOC, DEFAULT_FIX_TAGS_IN_JAVADOC));
     private SharedCodecConfiguration sharedCodecConfiguration;
@@ -129,6 +131,21 @@ public CodecConfiguration wrapEmptyBuffer(final boolean wrapEmptyBuffer)
         return this;
     }
 
+    /**
+     * Suppresses checks for empty tags, eg |1234=|
+     * instead of rejecting when tag is empty, treat the tag as absent
+     *
+     * Defaults to the value of {@link #ALLOW_EMPTY_TAGS} system property.
+     *
+     * @param allowEmptyTags true to suppress check of empty tags, false to keep checking (default)
+     * @return this
+     */
+    public CodecConfiguration allowEmptyTags(final boolean allowEmptyTags)
+    {
+        this.allowEmptyTags = allowEmptyTags;
+        return this;
+    }
+
     /**
      * Allow duplicate fields. Executable documentation can be found in the test "DuplicateFieldsTest".
      *
@@ -248,6 +265,11 @@ boolean wrapEmptyBuffer()
         return wrapEmptyBuffer;
     }
 
+    public boolean allowEmptyTags()
+    {
+        return allowEmptyTags;
+    }
+
     String codecRejectUnknownEnumValueEnabled()
     {
         return codecRejectUnknownEnumValueEnabled;

artio-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/CodecGenerator.java

@@ -180,6 +180,7 @@ private static void generateDictionary(
             RejectUnknownEnumValue.class,
             false,
             configuration.wrapEmptyBuffer(),
+            configuration.allowEmptyTags(),
             codecRejectUnknownEnumValueEnabled,
             configuration.fixTagsInJavadoc()).generate();
 
@@ -202,6 +203,7 @@ private static void generateDictionary(
                 RejectUnknownEnumValue.class,
                 true,
                 configuration.wrapEmptyBuffer(),
+                configuration.allowEmptyTags(),
                 codecRejectUnknownEnumValueEnabled,
                 configuration.fixTagsInJavadoc()).generate();
         }

artio-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/DecoderGenerator.java

@@ -119,6 +119,11 @@ static String decoderClassName(final String name)
      * Wrap empty buffer instead of throwing an exception if an optional string is unset.
      */
     private final boolean wrapEmptyBuffer;
+    /**
+     * Do not reject messages when there's an empty tag: instead, treat the field as absent
+     */
+    private final boolean allowEmptyTags;
+
 
     DecoderGenerator(
         final Dictionary dictionary,
@@ -132,6 +137,7 @@ static String decoderClassName(final String name)
         final Class<?> rejectUnknownEnumValueClass,
         final boolean flyweightsEnabled,
         final boolean wrapEmptyBuffer,
+        final boolean allowEmptyTags,
         final String codecRejectUnknownEnumValueEnabled,
         final boolean fixTagsInJavadoc)
     {
@@ -140,6 +146,7 @@ static String decoderClassName(final String name)
         this.initialBufferSize = initialBufferSize;
         this.encoderPackage = encoderPackage;
         this.wrapEmptyBuffer = wrapEmptyBuffer;
+        this.allowEmptyTags = allowEmptyTags;
     }
 
     public void generate()
@@ -311,6 +318,7 @@ private String classDeclaration(
         return String.format(
             "\n" +
             GENERATED_ANNOTATION +
+            SUPPRESS_THIS_ESCAPE_ANNOTATION +
             "public %3$s%4$sclass %1$s extends %5$s%2$s\n" +
             "{\n",
             className,
@@ -416,28 +424,21 @@ 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(name)
-            );
+                formatPropertyName(decoderClassName(name)));
         }
     }
 
@@ -491,9 +492,6 @@ 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" +
@@ -1770,11 +1768,7 @@ private String generateDecodePrefix(
             "                    invalidTagId = tag;\n" +
             "                    rejectReason = " + INVALID_TAG_NUMBER + ";\n" +
             "                }\n" +
-            "                else if (valueLength == 0)\n" +
-            "                {\n" +
-            "                    invalidTagId = tag;\n" +
-            "                    rejectReason = " + TAG_SPECIFIED_WITHOUT_A_VALUE + ";\n" +
-            "                }\n" +
+            emptyTagValidation() +
             headerValidation(isHeader) +
             (isGroup ? "" :
             "                if (!alreadyVisitedFields.add(tag))\n" +
@@ -1789,6 +1783,22 @@ private String generateDecodePrefix(
             "            {\n";
     }
 
+    private String emptyTagValidation()
+    {
+        if (allowEmptyTags)
+        {
+            return "";
+        }
+        else
+        {
+            return "                else if (valueLength == 0)\n" +
+                   "                {\n" +
+                   "                    invalidTagId = tag;\n" +
+                   "                    rejectReason = " + TAG_SPECIFIED_WITHOUT_A_VALUE + ";\n" +
+                   "                }\n";
+        }
+    }
+
     private String decodeTrailerOrReturn(final boolean hasCommonCompounds, final int indent)
     {
         return (hasCommonCompounds ?

artio-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/EncoderGenerator.java

@@ -42,6 +42,7 @@
 import static uk.co.real_logic.artio.dictionary.generation.AggregateType.HEADER;
 import static uk.co.real_logic.artio.dictionary.generation.EnumGenerator.enumName;
 import static uk.co.real_logic.artio.dictionary.generation.GenerationUtil.GENERATED_ANNOTATION;
+import static uk.co.real_logic.artio.dictionary.generation.GenerationUtil.SUPPRESS_THIS_ESCAPE_ANNOTATION;
 import static uk.co.real_logic.artio.dictionary.generation.GenerationUtil.fileHeader;
 import static uk.co.real_logic.artio.dictionary.generation.GenerationUtil.importFor;
 import static uk.co.real_logic.artio.dictionary.generation.OptionalSessionFields.ENCODER_OPTIONAL_SESSION_FIELDS;
@@ -347,6 +348,7 @@ private String classDeclaration(
         return String.format(
             "\n" +
             GENERATED_ANNOTATION +
+            SUPPRESS_THIS_ESCAPE_ANNOTATION +
             "public %3$s%4$sclass %1$s%5$s%2$s\n" +
             "{\n",
             className,

artio-codecs/src/main/java/uk/co/real_logic/artio/dictionary/generation/GenerationUtil.java

@@ -30,6 +30,7 @@ public final class GenerationUtil
 
     public static final String INDENT = "    ";
     public static final String GENERATED_ANNOTATION = "@Generated(\"uk.co.real_logic.artio\")\n";
+    public static final String SUPPRESS_THIS_ESCAPE_ANNOTATION = "@SuppressWarnings(\"this-escape\")\n";
 
     private GenerationUtil()
     {

artio-codecs/src/test/java/uk/co/real_logic/artio/dictionary/ExampleDictionary.java

@@ -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_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS =
+    public static final String MULTI_ENTRY_NESTED_GROUP_MESSAGE_WITHOUT_NESTED_FIELDS =
         "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";
 
@@ -347,6 +347,10 @@ public final class ExampleDictionary
         "8=FIX.4.4\0019=0027\00135=0\001115=abc\001116=2\001116=1\001117=1.1\001127=19700101-00:00:00.001" +
         "\00110=161\001";
 
+    public static final String EMPTY_FIX_TAG_FOR_STRING_FIELD_MESSAGE =
+        "8=FIX.4.4\0019=81\00135=0\001115=abc\001112=\001116=2\001117=1.1" +
+        "\001118=Y\001200=3\001119=123\001127=19700101-00:00:00.001\00110=199\001";
+
     public static final String DERIVED_FIELDS_MESSAGE =
         "8=FIX.4.4\0019=53\00135=0\001115=abc\001116=2\001117=1.1\001127=19700101-00:00:00.001" +
         "\00110=043\001";

artio-codecs/src/test/java/uk/co/real_logic/artio/dictionary/generation/AbstractDecoderGeneratorTest.java

@@ -33,7 +33,6 @@
 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;
@@ -72,6 +71,7 @@
     private static Class<?> heartbeatWithoutValidation;
     private static Class<?> heartbeatWithoutEnumValueValidation;
     private static Class<?> heartbeatWithRejectingUnknownFields;
+    private static Class<?> heartbeatAllowingEmptyTags;
     private static Class<?> heartbeat;
     private static Class<?> component;
     private static Class<?> otherMessage;
@@ -85,13 +85,15 @@
     static void generate(final boolean flyweightStringsEnabled) throws Exception
     {
         sourcesWithValidation = generateSources(
-            true, false, true, flyweightStringsEnabled, false);
+            true, false, true, flyweightStringsEnabled, false, false);
         final Map<String, CharSequence> sourcesWithNoEnumValueValidation = generateSources(
-            true, false, false, flyweightStringsEnabled, false);
+            true, false, false, flyweightStringsEnabled, false, false);
         final Map<String, CharSequence> sourcesWithoutValidation = generateSources(
-            false, false, true, flyweightStringsEnabled, true);
+            false, false, true, flyweightStringsEnabled, true, false);
         final Map<String, CharSequence> sourcesRejectingUnknownFields = generateSources(
-            true, true, true, flyweightStringsEnabled, false);
+            true, true, true, flyweightStringsEnabled, false, false);
+        final Map<String, CharSequence> sourcesAllowingEmptyTags = generateSources(
+            true, false, true, flyweightStringsEnabled, false, true);
         heartbeat = compileInMemory(HEARTBEAT_DECODER, sourcesWithValidation);
         if (heartbeat == null || CODEC_LOGGING)
         {
@@ -109,6 +111,7 @@
         heartbeatWithoutValidation = compileInMemory(HEARTBEAT_DECODER, sourcesWithoutValidation);
         heartbeatWithoutEnumValueValidation = compileInMemory(HEARTBEAT_DECODER, sourcesWithNoEnumValueValidation);
         heartbeatWithRejectingUnknownFields = compileInMemory(HEARTBEAT_DECODER, sourcesRejectingUnknownFields);
+        heartbeatAllowingEmptyTags = compileInMemory(HEARTBEAT_DECODER, sourcesAllowingEmptyTags);
         allReqFieldTypesMessage = compileInMemory(ALL_REQ_FIELD_TYPES_MESSAGE_DECODER, sourcesWithoutValidation);
         if (heartbeatWithoutValidation == null || CODEC_LOGGING)
         {
@@ -118,7 +121,8 @@
 
     private static Map<String, CharSequence> generateSources(
         final boolean validation, final boolean rejectingUnknownFields, final boolean rejectingUnknownEnumValue,
-        final boolean flyweightStringsEnabled, final boolean wrapEmptyBuffer)
+        final boolean flyweightStringsEnabled, final boolean wrapEmptyBuffer, final boolean allowEmptyTags
+    )
     {
         final Class<?> validationClass = validation ? ValidationOn.class : ValidationOff.class;
         final Class<?> rejectUnknownField = rejectingUnknownFields ?
@@ -132,7 +136,7 @@
         final DecoderGenerator decoderGenerator = new DecoderGenerator(
             MESSAGE_EXAMPLE, 1, TEST_PACKAGE, TEST_PARENT_PACKAGE, TEST_PACKAGE,
             outputManager, validationClass, rejectUnknownField,
-            rejectUnknownEnumValue, flyweightStringsEnabled, wrapEmptyBuffer,
+            rejectUnknownEnumValue, flyweightStringsEnabled, wrapEmptyBuffer, allowEmptyTags,
             String.valueOf(rejectingUnknownEnumValue), true);
         final EncoderGenerator encoderGenerator = new EncoderGenerator(MESSAGE_EXAMPLE, TEST_PACKAGE,
             TEST_PARENT_PACKAGE, outputManager, ValidationOn.class, RejectUnknownFieldOn.class,
@@ -838,22 +842,15 @@
 
         assertInvalid(decoder, INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP, 120);
     }
 
-    //this test was added for issue #525
+    //TODO this test was added to guide the bugfix
     @Test
     public void shouldReasonablyValidateGroupNumbersLessThanTheNumberOfElementsInTheGroupList() throws Exception
     {
-        final Decoder decoder = (Decoder)heartbeatWithRejectingUnknownFields.getConstructor().newInstance();
+        final Decoder decoder = decodeHeartbeatWithRejectingUnknownFields(
+            REPEATING_GROUP_MESSAGE_WITH_THREE, REPEATING_GROUP_MESSAGE_WITH_TOO_HIGH_NUMBER_FIELD);
 
-        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);
+//        assertInvalid(decoder, INCORRECT_NUMINGROUP_COUNT_FOR_REPEATING_GROUP, 120);
     }
 
     @Test
@@ -967,6 +964,14 @@
         assertEquals("Wrong reject reason", TAG_NOT_DEFINED_FOR_THIS_MESSAGE_TYPE, decoder.rejectReason());
     }
 
+    @Test
+    public void shouldAllowMessagesWithEmptyTagsForStringPropertyWhenAllowTagsPropIsSet() throws Exception
+    {
+        final Decoder decoder = decodeHeartbeatAllowingEmptyTags(EMPTY_FIX_TAG_FOR_STRING_FIELD_MESSAGE);
+
+        assertTrue("Failed validation with empty fix tag", decoder.validate());
+    }
+
     @Test
     public void shouldSkipFieldUnknownToMessageButDefinedInFIXSpec() throws Exception
     {
@@ -1175,29 +1180,15 @@
 
         decoder.reset();
 
-        decode(MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS, decoder);
+        decode(MULTI_ENTRY_NESTED_GROUP_MESSAGE_WITHOUT_NESTED_FIELDS, 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;
 
-        final Decoder decoder = decodeHeartbeat(MULTI_ENTRY_EG_GROUP_MESSAGE_WITHOUT_NESTED_GROUPS);
-        assertEquals(2, getNoEgGroupGroupCounter(decoder));
-
-        group = getEgGroup(decoder);
-        assertNull(getNestedGroup(group));
+        assertNestedRepeating(group, 1, CodecUtil.MISSING_INT, CodecUtil.MISSING_INT);
 
         group = next(group);
-        assertNull(getNestedGroup(group));
+        assertNestedRepeating(group, 2, CodecUtil.MISSING_INT, CodecUtil.MISSING_INT);
     }
 
     @Test
@@ -1911,12 +1902,13 @@
         return decoder;
     }
 
-    private Decoder decodeHeartbeatWithRejectingUnknownFields(final Decoder decoder,
-        final Consumer<Decoder> consumer,
-        final String example)
+    private Decoder decodeHeartbeatWithRejectingUnknownFields(final String... example) throws Exception
     {
-        decode(example, decoder);
-        consumer.accept(decoder);
+        final Decoder decoder = (Decoder)heartbeatWithRejectingUnknownFields.getConstructor().newInstance();
+        for (final String s : example)
+        {
+            decode(s, decoder);
+        }
         return decoder;
     }
 
@@ -1927,10 +1919,16 @@
         return decoder;
     }
 
+    private Decoder decodeHeartbeatAllowingEmptyTags(final String example) throws Exception
+    {
+        final Decoder decoder = (Decoder)heartbeatAllowingEmptyTags.getConstructor().newInstance();
+        decode(example, decoder);
+        return decoder;
+    }
+
     void decode(final String example, final Decoder decoder)
     {
         buffer.putAscii(1, example);
-        decoder.reset();
         decoder.decode(buffer, 1, example.length());
     }