Merge pull request #574 from tom-smalls/fix-appendable-decoder-version

[Java] Invalid code generated for newly added string field

Author: mjpt777

sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/java/JavaGenerator.java

@@ -827,7 +827,7 @@ private void generateDataDecodeMethods(
                     indent + "        }\n" +
                     indent + "    }\n",
                     Generators.toUpperFirstChar(propertyName),
-                    generateStringNotPresentCondition(token.version(), indent),
+                    generateStringNotPresentConditionForAppendable(token.version(), indent),
                     sizeOfLengthField,
                     generateGet(lengthType, "limit", byteOrderStr),
                     byteOrderStr));
@@ -1734,6 +1734,22 @@ private static CharSequence generateArrayFieldNotPresentCondition(final int sinc
             sinceVersion);
     }
 
+    private static CharSequence generateStringNotPresentConditionForAppendable(
+        final int sinceVersion, final String indent)
+    {
+        if (0 == sinceVersion)
+        {
+            return "";
+        }
+
+        return String.format(
+            indent + "        if (parentMessage.actingVersion < %d)\n" +
+            indent + "        {\n" +
+            indent + "            return;\n" +
+            indent + "        }\n\n",
+            sinceVersion);
+    }
+
     private static CharSequence generateStringNotPresentCondition(final int sinceVersion, final String indent)
     {
         if (0 == sinceVersion)

sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/java/SchemaExtensionTest.java

@@ -61,6 +61,7 @@ public void setup() throws Exception
         generator().generate();
     }
 
+    @SuppressWarnings("MethodLength")
     @Test
     public void testMessage1() throws Exception
     {
@@ -81,6 +82,8 @@ public void testMessage1() throws Exception
             final Object setEncoder = encoder.getClass().getMethod("tag5").invoke(encoder);
             set(setEncoder, "firstChoice", boolean.class, false);
             set(setEncoder, "secondChoice", boolean.class, true);
+
+            set(encoder, "tag6", String.class, "This is some variable length data");
         }
 
         { // Decode version 0
@@ -90,12 +93,16 @@ public void testMessage1() throws Exception
             assertNull(get(decoderVersion0, "tag3"));
             assertThat(get(decoderVersion0, "tag4").toString(), is("NULL_VAL"));
             assertNull(get(decoderVersion0, "tag5"));
+            final StringBuilder tag6Value = new StringBuilder();
+            get(decoderVersion0, "tag6", tag6Value);
+            assertThat(tag6Value.length(), is(0));
 
             assertEquals(0, decoderVersion0.getClass().getMethod("tag1SinceVersion").invoke(null));
             assertEquals(1, decoderVersion0.getClass().getMethod("tag2SinceVersion").invoke(null));
             assertEquals(2, decoderVersion0.getClass().getMethod("tag3SinceVersion").invoke(null));
             assertEquals(3, decoderVersion0.getClass().getMethod("tag4SinceVersion").invoke(null));
             assertEquals(4, decoderVersion0.getClass().getMethod("tag5SinceVersion").invoke(null));
+            assertEquals(5, decoderVersion0.getClass().getMethod("tag6SinceVersion").invoke(null));
         }
 
         { // Decode version 1
@@ -105,6 +112,9 @@ public void testMessage1() throws Exception
             assertNull(get(decoderVersion1, "tag3"));
             assertThat(get(decoderVersion1, "tag4").toString(), is("NULL_VAL"));
             assertNull(get(decoderVersion1, "tag5"));
+            final StringBuilder tag6Value = new StringBuilder();
+            get(decoderVersion1, "tag6", tag6Value);
+            assertThat(tag6Value.length(), is(0));
         }
 
         { // Decode version 2
@@ -116,6 +126,9 @@ public void testMessage1() throws Exception
             assertEquals(300, get(compositeDecoder2, "value"));
             assertThat(get(decoderVersion2, "tag4").toString(), is("NULL_VAL"));
             assertNull(get(decoderVersion2, "tag5"));
+            final StringBuilder tag6Value = new StringBuilder();
+            get(decoderVersion2, "tag6", tag6Value);
+            assertThat(tag6Value.length(), is(0));
         }
 
         { // Decode version 3
@@ -128,6 +141,9 @@ public void testMessage1() throws Exception
             final Object enumConstant = getAEnumConstant(decoderVersion3, "AEnum", 1);
             assertEquals(enumConstant, get(decoderVersion3, "tag4"));
             assertNull(get(decoderVersion3, "tag5"));
+            final StringBuilder tag6Value = new StringBuilder();
+            get(decoderVersion3, "tag6", tag6Value);
+            assertThat(tag6Value.length(), is(0));
         }
 
         { // Decode version 4
@@ -143,6 +159,27 @@ public void testMessage1() throws Exception
             assertNotNull(setDecoder);
             assertEquals(false, get(setDecoder, "firstChoice"));
             assertEquals(true, get(setDecoder, "secondChoice"));
+            final StringBuilder tag6Value = new StringBuilder();
+            get(decoderVersion4, "tag6", tag6Value);
+            assertThat(tag6Value.length(), is(0));
+        }
+
+        { // Decode version 5
+            final Object decoderVersion5 = getMessage1Decoder(buffer, 14, 5);
+            assertEquals(100, get(decoderVersion5, "tag1"));
+            assertEquals(200, get(decoderVersion5, "tag2"));
+            final Object compositeDecoder4 = get(decoderVersion5, "tag3");
+            assertNotNull(compositeDecoder4);
+            assertEquals(300, get(compositeDecoder4, "value"));
+            final Object enumConstant = getAEnumConstant(decoderVersion5, "AEnum", 1);
+            assertEquals(enumConstant, get(decoderVersion5, "tag4"));
+            final Object setDecoder = get(decoderVersion5, "tag5");
+            assertNotNull(setDecoder);
+            assertEquals(false, get(setDecoder, "firstChoice"));
+            assertEquals(true, get(setDecoder, "secondChoice"));
+            final StringBuilder tag6Value = new StringBuilder();
+            get(decoderVersion5, "tag6", tag6Value);
+            assertThat(tag6Value.toString(), is("This is some variable length data"));
         }
     }
 

sbe-tool/src/test/resources/extension-schema.xml

@@ -2,7 +2,7 @@
 <sbe:messageSchema xmlns:sbe="http://fixprotocol.io/2016/sbe"
                    package="code.generation.test"
                    id="1"
-                   version="4"
+                   version="5"
                    description="Unit Test">
     <types>
         <composite name="messageHeader" description="Message identifiers and length of message root">
@@ -19,6 +19,10 @@
         <composite name="AComposite" sinceVersion="1">
             <type name="value" primitiveType="int32"/>
         </composite>
+        <composite name="varDataEncoding" semanticType="Length" sinceVersion="5">
+            <type name="length" primitiveType="uint8" semanticType="Length"/>
+            <type name="varData" primitiveType="char" semanticType="data"/>
+        </composite>
         <enum name="AEnum" encodingType="uint8" sinceVersion="3">
             <validValue name="FirstValue">0</validValue>
             <validValue name="SecondValue">1</validValue>
@@ -34,13 +38,15 @@
         Version 2: tag3 added to TestMessage1, tag2 added to TestMessage2
         Version 3: AEnum, ASet introduced; tag4 added to TestMessage1, tag5 added to TestMessage2
         Version 4: tag5 added to TestMessage1, tag4 added to TestMessage2
+        Version 5: tag6 added to TestMessage1
     -->
     <sbe:message name="TestMessage1" id="1">
         <field name="tag1" id="1" type="int32"/>
         <field name="tag2" id="2" type="AType" sinceVersion="1"/>
         <field name="tag3" id="3" type="AComposite" sinceVersion="2"/>
         <field name="tag4" id="4" type="AEnum" sinceVersion="3"/>
         <field name="tag5" id="5" type="ASet" sinceVersion="4"/>
+        <data name="tag6" type="varDataEncoding" id="6" sinceVersion="5"/>
     </sbe:message>
     <sbe:message name="TestMessage2" id="2">
         <field name="tag1" id="1" type="int32"/>