Issue472 Handle presence="optional"

Add intialization to NullValue for optional fields. Allow NullValue for optional fields in RangeCheck. Add test case.

Author: billsegall

build.gradle

@@ -523,6 +523,7 @@ task(generateGolangCodecsWithXSD, type: JavaExec) {
  args = ['sbe-tool/src/test/resources/group-with-data-schema.xml',
  'sbe-tool/src/test/resources/FixBinary.xml',
  'sbe-tool/src/test/resources/issue435.xml',
+ 'sbe-tool/src/test/resources/issue472.xml',
  'sbe-tool/src/test/resources/since-deprecated-test-schema.xml',
  'sbe-tool/src/test/resources/example-bigendian-test-schema.xml',
  'gocode/resources/example-composite.xml',

gocode/Makefile

@@ -68,7 +68,7 @@ test: $(DEP)
 go install \
 ;done))
 (export GOPATH=$(GOPATH) && \
-(for t in baseline-bignendian mktdata group_with_data group_with_data_extension composite_elements composite since-deprecated simple issue435; do \
+(for t in baseline-bigendian mktdata group_with_data group_with_data_extension composite_elements composite since-deprecated simple issue435 issue472; do \
 cd $(GOPATH)/src/$$t && \
 go build && \
 go fmt && \

gocode/src/issue472/Issue472_test.go

@@ -0,0 +1,59 @@
+package issue472
+
+import (
+"bytes"
+"testing"
+)
+
+// Optional values should allow NullValue in RangeCheck
+func TestOptionalRangeCheck(t *testing.T) {
+var in Issue472
+Issue472Init(&in)
+if err := in.RangeCheck(in.SbeSchemaVersion(), in.SbeSchemaVersion()); err != nil {
+t.Log("Issue472 RangeCheck() did not accept a NullValue for an optional field", err)
+t.Fail()
+}
+}
+
+// Optional values should be set to Null Value by the Init() method
+func TestOptionalInit(t *testing.T) {
+var in Issue472
+Issue472Init(&in)
+if in.Optional != in.OptionalNullValue() {
+t.Log("Issue472Init() failed to initialize optional field to Null Value")
+t.Fail()
+}
+}
+
+func TestEncodeDecode(t *testing.T) {
+
+m := NewSbeGoMarshaller()
+var in Issue472
+Issue472Init(&in)
+var buf = new(bytes.Buffer)
+
+// in contains a single optional field which is not initialized
+if err := in.Encode(m, buf, true); err != nil {
+t.Log("Encoding Error", err)
+t.Fail()
+}
+
+// Create a new Issue472 and decode into it
+out := *new(Issue472)
+
+if err := out.Decode(m, buf, in.SbeSchemaVersion(), in.SbeBlockLength(), true); err != nil {
+t.Log("Decoding Error", err)
+t.Fail()
+}
+t.Log("Issue472 decodes as: ", out)
+
+// Sanity checks
+if buf.Len() != 0 {
+t.Logf("buffer not drained")
+t.Fail()
+}
+if in != out {
+t.Logf("in != out\n%s\n%s", in, out)
+t.Fail()
+}
+}

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

@@ -422,16 +422,23 @@ private void generateDecodePrimitive(
  private void generateRangeCheckPrimitive(
  final StringBuilder sb,
  final String varName,
- final Token token)
+ final Token token,
+ final Boolean isOptional)
  {
+ // Note that constant encoding is a property of the type
+ // and so works on signal/encoding tokens but optionality is
+ // a property of the field and so is not set on the encoding token.
+ // For this reason we can use the token to check for constancy
+ // but pass in optionality as a parameter
+
  // Constant values don't need checking
  if (token.isConstantEncoding())
  {
  return;
  }
 
  // If this field is unknown then we have nothing to check
- // Otherwise do a The Min,MaxValue checks (possibly for arrays)
+ // Otherwise do the Min,MaxValue checks (possibly for arrays)
  this.imports.add("fmt");
  if (token.arrayLength() > 1)
  {
@@ -449,9 +456,22 @@ private void generateRangeCheckPrimitive(
  }
  else
  {
+ // Optional fields can be NullValue which may be outside the
+ // range of Min->Max so we need a special case.
+ // Structured this way for go fmt sanity on both cases.
+ final String check;
+ if (isOptional)
+ {
+ check = "\t\tif %1$s != %1$sNullValue() && (%1$s < %1$sMinValue() || %1$s > %1$sMaxValue()) {\n";
+ }
+ else
+ {
+ check = "\t\tif %1$s < %1$sMinValue() || %1$s > %1$sMaxValue() {\n";
+ }
+
  sb.append(String.format(
  "\tif %1$sInActingVersion(actingVersion) {\n" +
- "\t\tif %1$s < %1$sMinValue() || %1$s > %1$sMaxValue() {\n" +
+ check +
  "\t\t\treturn fmt.Errorf(\"Range check failed on %1$s " +
  "(%%d < %%d > %%d)\", %1$sMinValue(), %1$s, %1$sMaxValue())\n" +
  "\t\t}\n" +
@@ -468,40 +488,56 @@ private void generateRangeCheckPrimitive(
  }
  }
 
- private void generateInitPrimitive(
+ private void generateOptionalInitPrimitive(
  final StringBuilder sb,
  final String varName,
  final Token token)
  {
+ final Encoding encoding = token.encoding();
+
+ // Optional items get initialized to their NullValue
+ sb.append(String.format(
+ "\t%1$s = %2$s\n",
+ varName,
+ generateNullValueLiteral(encoding.primitiveType(), encoding)));
+ }
+
+ private void generateConstantInitPrimitive(
+ final StringBuilder sb,
+ final String varName,
+ final Token token)
+ {
+ final Encoding encoding = token.encoding();
+
  // Decode of a constant is simply assignment
  if (token.isConstantEncoding())
  {
  // if primitiveType="char" this is a character array
- if (token.encoding().primitiveType() == CHAR)
+ if (encoding.primitiveType() == CHAR)
  {
- if (token.encoding().constValue().size() > 1)
+ if (encoding.constValue().size() > 1)
  {
  // constValue is a string
  sb.append(String.format(
  "\tcopy(%1$s[:], \"%2$s\")\n",
  varName,
- token.encoding().constValue()));
+ encoding.constValue()));
  }
  else
  {
  // constValue is a char
  sb.append(String.format(
  "\t%1$s[0] = %2$s\n",
  varName,
- token.encoding().constValue()));
+ encoding.constValue()));
  }
  }
  else
  {
  sb.append(String.format(
  "\t%1$s = %2$s\n",
  varName,
- generateLiteral(token.encoding().primitiveType(), token.encoding().constValue().toString())));
+ generateLiteral(encoding.primitiveType(), encoding.constValue().toString())));
  }
  }
  }
@@ -652,17 +688,28 @@ private int generateEncodeDecode(
  encode.append(generateEncodeOffset(gap, ""));
  decode.append(generateDecodeOffset(gap, ""));
  currentOffset += signalToken.encodedLength() + gap;
+ final String primitive = Character.toString(varName) + "." + propertyName;
 
- // Encode of a constant is a nullop
- if (!signalToken.isConstantEncoding())
+ // Encode of a constant is a nullop and we want to
+ // initialize constant values.
+ if (signalToken.isConstantEncoding())
+ {
+ generateConstantInitPrimitive(init, primitive, signalToken);
+ }
+ else
  {
  generateEncodePrimitive(encode, varName, formatPropertyName(signalToken.name()), signalToken);
+ }
 
+ // Optional tokens also get initialized
+ if (signalToken.isOptionalEncoding())
+ {
+ generateOptionalInitPrimitive(init, primitive, signalToken);
  }
- final String primitive = Character.toString(varName) + "." + propertyName;
+
  generateDecodePrimitive(decode, primitive, signalToken);
- generateRangeCheckPrimitive(rangeCheck, primitive, signalToken);
- generateInitPrimitive(init, primitive, signalToken);
+ generateRangeCheckPrimitive(rangeCheck, primitive, signalToken, signalToken.isOptionalEncoding());
+
  break;
 
  case BEGIN_GROUP:
@@ -1004,16 +1051,29 @@ private int generateFieldEncodeDecode(
  gap = encodingToken.offset() - currentOffset;
  encode.append(generateEncodeOffset(gap, ""));
  decode.append(generateDecodeOffset(gap, ""));
+ final String primitive = Character.toString(varName) + "." + propertyName;
 
- // Encode of a constant is a nullop
- if (!encodingToken.isConstantEncoding())
+ // Encode of a constant is a nullop and we want to
+ // initialize constant values.
+ // (note: constancy is determined by the type's token)
+ if (encodingToken.isConstantEncoding())
+ {
+ generateConstantInitPrimitive(init, primitive, encodingToken);
+ }
+ else
  {
  generateEncodePrimitive(encode, varName, formatPropertyName(signalToken.name()), encodingToken);
  }
- final String primitive = Character.toString(varName) + "." + propertyName;
+
+ // Optional tokens get initialized to NullValue
+ // (note: optionality is determined by the field's token)
+ if (signalToken.isOptionalEncoding())
+ {
+ generateOptionalInitPrimitive(init, primitive, encodingToken);
+ }
+
  generateDecodePrimitive(decode, primitive, encodingToken);
- generateRangeCheckPrimitive(rc, primitive, encodingToken);
- generateInitPrimitive(init, primitive, encodingToken);
+ generateRangeCheckPrimitive(rc, primitive, encodingToken, signalToken.isOptionalEncoding());
  break;
  }
 

sbe-tool/src/main/resources/golang/templates/SbeMarshallingBigEndian.go

@@ -84,7 +84,6 @@ func (m *SbeGoMessageHeader) Decode(_m *SbeGoMarshaller, _r io.Reader) error {
 return nil
 }
 
-
 func (m *SbeGoMarshaller) WriteUint8(w io.Writer, v uint8) error {
 m.b1[0] = byte(v)
 _, err := w.Write(m.b1)

sbe-tool/src/test/resources/issue472.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<sbe:messageSchema xmlns:sbe="http://fixprotocol.io/2016/sbe"
+ package="issue472"
+ id="472"
+ version="0"
+ semanticVersion="1.0"
+ description="issue 472 test case"
+ byteOrder="bigEndian">
+ <types>
+ <composite name="messageHeader" description="Message identifiers and length of message root">
+ <type name="blockLength" primitiveType="uint16"/>
+ <type name="templateId" primitiveType="uint16"/>
+ <type name="schemaId" primitiveType="uint16"/>
+ <type name="version" primitiveType="uint16"/>
+ </composite>
+ </types>
+ <sbe:message name="issue472" id="1" description="issue 472 test">
+ <field name="optional" type="uint64" id="2" presence="optional"/>
+ </sbe:message>
+</sbe:messageSchema>