real-logic / simple-binary-encoding

Simple Binary Encoding (SBE) - High Performance Message Codec
Apache License 2.0
3.08k stars 519 forks source link

1.20.4 fails to parse cme's schema file #833

Closed pastorom closed 3 years ago

pastorom commented 3 years ago

Could someone help with what i'm doing wrong (if at all) here? The schema works with 1.19.0, so i doubt the issue is with the schema file. Zipped schema file attached.

Previously, with no issues:

java -Dsbe.target.language=Cpp -Dsbe.output.dir=./cme_iLinkBinary -Dsbe.validation.stop.on.error=true -jar sbe-all-1.19.0.jar ilinkbinary.xml

With the latest version:

java -Dsbe.target.language=Cpp -Dsbe.output.dir=./cme_iLinkBinary -Dsbe.validation.stop.on.error=true -jar sbe-all-1.20.4.jar ilinkbinary.xml

ERROR: at validValue Null uses nullValue: 255 Exception in thread "main" java.lang.IllegalArgumentException: at validValue Null uses nullValue: 255 at uk.co.real_logic.sbe.xml.ErrorHandler.error(ErrorHandler.java:72) at uk.co.real_logic.sbe.xml.XmlSchemaParser.handleError(XmlSchemaParser.java:252) at uk.co.real_logic.sbe.xml.EnumType.(EnumType.java:160) at uk.co.real_logic.sbe.xml.EnumType.(EnumType.java:51) at uk.co.real_logic.sbe.xml.XmlSchemaParser.lambda$findTypes$2(XmlSchemaParser.java:203) at uk.co.real_logic.sbe.xml.XmlSchemaParser.forEach(XmlSchemaParser.java:464) at uk.co.real_logic.sbe.xml.XmlSchemaParser.findTypes(XmlSchemaParser.java:202) at uk.co.real_logic.sbe.xml.XmlSchemaParser.parse(XmlSchemaParser.java:137) at uk.co.real_logic.sbe.SbeTool.parseSchema(SbeTool.java:297) at uk.co.real_logic.sbe.SbeTool.main(SbeTool.java:204)

ilinkbinary.zip

tmontgomery commented 3 years ago

The schema is not correct. The nullValue is implied and should not be used in the validValue. SBE just did not check in previous releases. Now it does.

pastorom commented 3 years ago

Fantastic - IIRC this schema goes into production over the weekend. Ok, thanks - i'll reach out to the CME. Odd that no one else has encountered this yet, or brought it up to them.

steve-lorimer commented 3 years ago

@pastorom Did you get anywhere with this? I just came across the same issue. Any info from the CME re the invalid schema file?

@tmontgomery Do you have a link to the spec which details that null values are implicit please?

Thanks

steve-lorimer commented 3 years ago

I read the following in this schema specification document (fixtrading.org/standards/sbe-online):

For optional fields, a special null value is used to indicate that a field value is not set. The default null indicator may also be overridden by a message schema.

Would I be correct in saying that CME's definition file is therefore correct, and the sbe tool is incorrect?

Edit

Sorry, I see in the schema XSD here, that whilst overriding the null value is possible, the field name should be called nullValue, whereas CME is using null, which is incorrect.

I will chase CME

mjpt777 commented 3 years ago

The CME are not correctly using the null value to indicate optionality. The field should be defined as presence=optional and not presence=required. SbeTool generates a null value for the enum which can be used for optional.

Here is a quote from their own documentation.

https://www.fixtrading.org/standards/sbe-online/

presence=required The field must always be set. This is the default presence. Mutually exclusive with nullValue.

steve-lorimer commented 3 years ago

@mjpt777 the error is thrown when parsing the following enum:

<enum name="TimeInForce" encodingType="uInt8">
  <validValue name="Day" description="Day">0</validValue>
  <validValue name="GoodTillCancel" description="Good Till Cancel">1</validValue>
  <validValue name="FillAndKill" description="Fill And Kill">3</validValue>
  <validValue name="FillOrKill" description="Fill Or Kill">4</validValue>
  <validValue name="GoodTillDate" description="Good Till Date">6</validValue>
  <validValue name="GoodForSession" description="Good For Session" sinceVersion="6">99</validValue>
  <validValue name="Null" description="Null" sinceVersion="6">255</validValue>                 <--- this field
</enum>

The following exception is thrown:

ERROR: at <types><enum name="TimeInForce"> validValue Null uses nullValue: 255
Exception in thread "main" java.lang.IllegalArgumentException: at <types><enum name="TimeInForce"> validValue Null uses nullValue: 255
        at uk.co.real_logic.sbe.xml.ErrorHandler.error(ErrorHandler.java:72)
        at uk.co.real_logic.sbe.xml.XmlSchemaParser.handleError(XmlSchemaParser.java:252)
        at uk.co.real_logic.sbe.xml.EnumType.<init>(EnumType.java:160)
        at uk.co.real_logic.sbe.xml.EnumType.<init>(EnumType.java:51)
        at uk.co.real_logic.sbe.xml.XmlSchemaParser.lambda$findTypes$2(XmlSchemaParser.java:203)
        at uk.co.real_logic.sbe.xml.XmlSchemaParser.forEach(XmlSchemaParser.java:464)
        at uk.co.real_logic.sbe.xml.XmlSchemaParser.findTypes(XmlSchemaParser.java:202)
        at uk.co.real_logic.sbe.xml.XmlSchemaParser.parse(XmlSchemaParser.java:137)
        at uk.co.real_logic.sbe.SbeTool.parseSchema(SbeTool.java:297)
        at uk.co.real_logic.sbe.SbeTool.main(SbeTool.java:204)

However, if you look at the encoding type for TimeInForce it's of type uInt8, not uInt8NULL.

Here is the schema for these encoding types:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<ns2:messageSchema xmlns:ns2="http://www.fixprotocol.org/ns/simple/1.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" package="iLinkBinary" id="8" version="6" semanticVersion="FIX5.0" description="20201013" byteOrder="littleEndian" xsi:schemaLocation="http://www.fixtradingcommunity.org/pg/file/fplpo/read/1196759/simple-binary-encoding-rc2xsd SimpleBinary-RC2.xsd">
    <types>
        ...
        <type name="uInt8" description="uInt8" primitiveType="uint8"/>
        <type name="uInt8NULL" description="uInt8NULL" presence="optional" nullValue="255" primitiveType="uint8"/>

If the encoding type for TimeInForce was uInt8NULL then I would agree the exception makes sense, but in this case it's not, it's uInt8, which doesn't have a null value - so the enumeration "Null" should be accepted don't you think?

mjpt777 commented 3 years ago

@skebanga The presence on a type can be overridden on a field therefore the null value must exist. Presence on a type is just a convenience to simplify the field declarations at times.

steve-lorimer commented 3 years ago

@mjpt777 got it, thanks!

pastorom commented 3 years ago

@skebanga I emailed them Friday (1/8) afternoon CT. Please email them as well - they're likelier to take a look if more than one person complains.

steve-lorimer commented 3 years ago

@pastorom no probs - I have emailed our technical account manager already! :)

tmontgomery commented 3 years ago

@tmontgomery Do you have a link to the spec which details that null values are implicit please?

Just to follow up.

All types in SBE have an implicit min/max/null value.

https://github.com/FIXTradingCommunity/fix-simple-binary-encoding/blob/master/v1-0-STANDARD/doc/02FieldEncoding.md#range-attributes-for-integer-fields

steve-lorimer commented 3 years ago

@pastorom I received this from our tech support manager:

My iLink APi team have replied and confirmed we are aware of this issue. This is due to the third-party decoder’s latest version is incompatible with the iLink3 Schema v6. We will cancel v6 production launch and release v7 to address this issue. The tentative NR date for V7 is 4-Feb and we hope to announce this on Thursday in the Globex notice so please keep an eye on the notices.

pastorom commented 3 years ago

@skebanga great, thanks for the update! yeah we were conveyed this verbally over a phone call as well. Appreciate you following up!