do not throw IncorrectDataFormat exception if value is empty and ValidateFieldsHaveValues=N

quickfixj-core/pom.xml

@@ -56,6 +56,19 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>org.quickfixj</groupId>
+            <artifactId>quickfixj-codegenerator</artifactId>
+            <version>${project.version}</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.jooq</groupId>
+            <artifactId>joor-java-8</artifactId>
+            <version>0.9.9</version>
+            <scope>test</scope>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.mina</groupId>
             <artifactId>mina-core</artifactId>

quickfixj-core/src/main/java/quickfix/DataDictionary.java

@@ -48,10 +48,7 @@
 import java.util.Map;
 import java.util.Set;
 
-import static quickfix.FileUtil.Location.CLASSLOADER_RESOURCE;
-import static quickfix.FileUtil.Location.CONTEXT_RESOURCE;
-import static quickfix.FileUtil.Location.FILESYSTEM;
-import static quickfix.FileUtil.Location.URL;
+import static quickfix.FileUtil.Location.*;
 
 /**
  * Provide the message metadata for various versions of FIX.
@@ -697,6 +694,9 @@ private void checkValidFormat(StringField field) throws IncorrectDataFormat {
         if (fieldType == null) {
             return;
         }
+        if (field.getValue().length() == 0 && !checkFieldsHaveValues) {
+            return;
+        }
         try {
             switch (fieldType) {
                 case STRING:

quickfixj-core/src/test/java/org/quickfixj/codegenerator/Compiler.java

@@ -0,0 +1,92 @@
+package org.quickfixj.codegenerator;
+
+import org.joor.Reflect;
+
+import javax.tools.*;
+import java.io.ByteArrayOutputStream;
+import java.io.OutputStream;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+class Compiler {
+    private Compiler() {
+    }
+
+    static Map<String, Reflect> compile(final Map<String, String> classNameToSourceMap) {
+        final MethodHandles.Lookup lookup = MethodHandles.lookup();
+        final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+
+        final ClassFileManager fileManager = new ClassFileManager(compiler.getStandardFileManager(null, null, null));
+
+        final List<CharSequenceJavaFileObject> files = new ArrayList<>();
+        for (final Map.Entry<String, String> entry : classNameToSourceMap.entrySet()) {
+            files.add(new CharSequenceJavaFileObject(entry.getKey(), entry.getValue()));
+        }
+
+        final StringWriter out = new StringWriter();
+        compiler.getTask(out, fileManager, null, null, null, files).call();
+
+        if (!fileManager.output.keySet().containsAll(classNameToSourceMap.keySet())) {
+            throw new RuntimeException("Compilation error:\n" + out.toString());
+        }
+
+        final ClassLoader cl = lookup.lookupClass().getClassLoader();
+        final Map<String, Reflect> instances = new LinkedHashMap<>();
+        for (final Map.Entry<String, JavaFileObject> output : fileManager.output.entrySet()) {
+            final String className = output.getKey();
+            final byte[] b = output.getValue().getBytes();
+            final Class<?> clazz = Reflect.on(cl).call("defineClass", className, b, 0, b.length).get();
+            instances.put(className, Reflect.on(clazz));
+        }
+        return instances;
+    }
+
+    private static final class ClassFileManager extends ForwardingJavaFileManager<StandardJavaFileManager> {
+        private final Map<String, JavaFileObject> output = new LinkedHashMap<>();
+
+        ClassFileManager(final StandardJavaFileManager standardManager) {
+            super(standardManager);
+        }
+
+        @Override
+        public JavaFileObject getJavaFileForOutput(final JavaFileManager.Location location, final String className, final JavaFileObject.Kind kind, final FileObject sibling) {
+            return output.computeIfAbsent(className, (cn) -> new JavaFileObject(cn, kind));
+        }
+    }
+
+    private static final class JavaFileObject extends SimpleJavaFileObject {
+        private final ByteArrayOutputStream os = new ByteArrayOutputStream();
+
+        JavaFileObject(final String name, final JavaFileObject.Kind kind) {
+            super(URI.create("string:///" + name.replace('.', '/') + kind.extension), kind);
+        }
+
+        byte[] getBytes() {
+            return os.toByteArray();
+        }
+
+        @Override
+        public OutputStream openOutputStream() {
+            return os;
+        }
+    }
+
+    private static final class CharSequenceJavaFileObject extends SimpleJavaFileObject {
+        private final CharSequence content;
+
+        CharSequenceJavaFileObject(final String className, final CharSequence content) {
+            super(URI.create("string:///" + className.replace('.', '/') + JavaFileObject.Kind.SOURCE.extension), JavaFileObject.Kind.SOURCE);
+            this.content = content;
+        }
+
+        @Override
+        public CharSequence getCharContent(final boolean ignoreEncodingErrors) {
+            return content;
+        }
+    }
+}

quickfixj-core/src/test/java/org/quickfixj/codegenerator/MessageCodeGeneratorTest.java

@@ -0,0 +1,173 @@
+package org.quickfixj.codegenerator;
+
+import org.joor.Reflect;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import quickfix.*;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class MessageCodeGeneratorTest {
+    @Rule
+    public TemporaryFolder folder = new TemporaryFolder();
+
+    @Test
+    public void generateFromBasicFixDictionary() throws Exception {
+        final File schema = new File(MessageCodeGeneratorTest.class.getResource("/org/quickfixj/codegenerator/MessageFactory.xsl").getFile());
+        final File spec = new File(MessageCodeGeneratorTest.class.getResource("/basic.xml").getFile());
+
+        final MessageCodeGenerator.Task task = new MessageCodeGenerator.Task();
+        task.setName("basic");
+        task.setSpecification(spec);
+        task.setTransformDirectory(schema.getParentFile());
+        task.setMessagePackage("basic");
+        task.setOutputBaseDirectory(folder.getRoot());
+        task.setFieldPackage("field");
+        task.setOverwrite(true);
+        task.setOrderedFields(true);
+        task.setDecimalGenerated(false);
+        final MessageCodeGenerator generator = new MessageCodeGenerator();
+        generator.generate(task);
+
+        final File fieldDir = new File(folder.getRoot(), "field");
+        final File messageDir = new File(folder.getRoot(), "basic");
+
+        final Map<String, String> classNameToSourceMap = new LinkedHashMap<>();
+        classNameToSourceMap.put("field.BeginString", getSource(new File(fieldDir, "BeginString.java")));
+        classNameToSourceMap.put("field.BodyLength", getSource(new File(fieldDir, "BodyLength.java")));
+        classNameToSourceMap.put("field.CheckSum", getSource(new File(fieldDir, "CheckSum.java")));
+        classNameToSourceMap.put("field.MsgType", getSource(new File(fieldDir, "MsgType.java")));
+        classNameToSourceMap.put("field.Signature", getSource(new File(fieldDir, "Signature.java")));
+        classNameToSourceMap.put("field.SignatureLength", getSource(new File(fieldDir, "SignatureLength.java")));
+        classNameToSourceMap.put("field.TestReqID", getSource(new File(fieldDir, "TestReqID.java")));
+
+        classNameToSourceMap.put("basic.Message", getSource(new File(messageDir, "Message.java")));
+        classNameToSourceMap.put("basic.TestRequest", getSource(new File(messageDir, "TestRequest.java")));
+        classNameToSourceMap.put("basic.MessageCracker", getSource(new File(messageDir, "MessageCracker.java")));
+        classNameToSourceMap.put("basic.MessageFactory", getSource(new File(messageDir, "MessageFactory.java")));
+        final Map<String, Reflect> classes = Compiler.compile(classNameToSourceMap);
+
+        final Map<String, FieldDef> fieldDefs = new LinkedHashMap<>();
+        fieldDefs.put("BeginString", new FieldDef(8, StringField.class));
+        fieldDefs.put("BodyLength", new FieldDef(9, IntField.class));
+        fieldDefs.put("CheckSum", new FieldDef(10, StringField.class));
+        fieldDefs.put("MsgType", new FieldDef(35, StringField.class));
+        fieldDefs.put("Signature", new FieldDef(89, StringField.class));
+        fieldDefs.put("SignatureLength", new FieldDef(93, IntField.class));
+        fieldDefs.put("TestReqID", new FieldDef(112, StringField.class));
+        validateFields(classes, fieldDefs);
+
+        final Map<String, MessageDef> messageDefs = new LinkedHashMap<>();
+        messageDefs.put("TestRequest", new MessageDef("1"));
+        validateMessages(classes, messageDefs);
+    }
+
+    /**
+     * This test is based on the FXAll FIX spec post MiFID II which has the same group in different locations within a
+     * message based on the context of the message.  At present this generates Java code which does not compile due to
+     * duplicate case labels.
+     */
+    @Test(expected = RuntimeException.class)
+    public void generateFromFixDictionaryWithNestedGroups() throws Exception {
+        final File schema = new File(MessageCodeGeneratorTest.class.getResource("/org/quickfixj/codegenerator/MessageFactory.xsl").getFile());
+        final File spec = new File(MessageCodeGeneratorTest.class.getResource("/nested-group.xml").getFile());
+
+        final MessageCodeGenerator.Task task = new MessageCodeGenerator.Task();
+        task.setName("nested");
+        task.setSpecification(spec);
+        task.setTransformDirectory(schema.getParentFile());
+        task.setMessagePackage("nested");
+        task.setOutputBaseDirectory(folder.getRoot());
+        task.setFieldPackage("field");
+        task.setOverwrite(true);
+        task.setOrderedFields(true);
+        task.setDecimalGenerated(false);
+        final MessageCodeGenerator generator = new MessageCodeGenerator();
+        generator.generate(task);
+
+        final File fieldDir = new File(folder.getRoot(), "field");
+        final File messageDir = new File(folder.getRoot(), "nested");
+
+        final Map<String, String> classNameToSourceMap = new LinkedHashMap<>();
+        classNameToSourceMap.put("field.BeginString", getSource(new File(fieldDir, "BeginString.java")));
+        classNameToSourceMap.put("field.BodyLength", getSource(new File(fieldDir, "BodyLength.java")));
+        classNameToSourceMap.put("field.CheckSum", getSource(new File(fieldDir, "CheckSum.java")));
+        classNameToSourceMap.put("field.MsgType", getSource(new File(fieldDir, "MsgType.java")));
+        classNameToSourceMap.put("field.Signature", getSource(new File(fieldDir, "Signature.java")));
+        classNameToSourceMap.put("field.SignatureLength", getSource(new File(fieldDir, "SignatureLength.java")));
+        classNameToSourceMap.put("field.TestReqID", getSource(new File(fieldDir, "TestReqID.java")));
+        classNameToSourceMap.put("field.NoFoos", getSource(new File(fieldDir, "NoFoos.java")));
+        classNameToSourceMap.put("field.NoBars", getSource(new File(fieldDir, "NoBars.java")));
+        classNameToSourceMap.put("field.Foo", getSource(new File(fieldDir, "Foo.java")));
+
+        classNameToSourceMap.put("basic.Message", getSource(new File(messageDir, "Message.java")));
+        classNameToSourceMap.put("basic.TestRequest", getSource(new File(messageDir, "TestRequest.java")));
+        classNameToSourceMap.put("basic.MessageCracker", getSource(new File(messageDir, "MessageCracker.java")));
+        classNameToSourceMap.put("basic.MessageFactory", getSource(new File(messageDir, "MessageFactory.java")));
+        final Map<String, Reflect> classes = Compiler.compile(classNameToSourceMap);
+
+        final Map<String, FieldDef> fieldDefs = new LinkedHashMap<>();
+        fieldDefs.put("BeginString", new FieldDef(8, StringField.class));
+        fieldDefs.put("BodyLength", new FieldDef(9, IntField.class));
+        fieldDefs.put("CheckSum", new FieldDef(10, StringField.class));
+        fieldDefs.put("MsgType", new FieldDef(35, StringField.class));
+        fieldDefs.put("Signature", new FieldDef(89, StringField.class));
+        fieldDefs.put("SignatureLength", new FieldDef(93, IntField.class));
+        fieldDefs.put("TestReqID", new FieldDef(112, StringField.class));
+        fieldDefs.put("NoFoos", new FieldDef(112, IntField.class));
+        fieldDefs.put("NoBars", new FieldDef(112, IntField.class));
+        fieldDefs.put("Foo", new FieldDef(112, StringField.class));
+        validateFields(classes, fieldDefs);
+
+        final Map<String, MessageDef> messageDefs = new LinkedHashMap<>();
+        messageDefs.put("TestRequest", new MessageDef("1"));
+        validateMessages(classes, messageDefs);
+    }
+
+    private String getSource(final File file) throws IOException {
+        return new String(Files.readAllBytes(file.toPath()));
+    }
+
+    private void validateFields(final Map<String, Reflect> classes, final Map<String, FieldDef> fieldDefs) {
+        for (final Map.Entry<String, FieldDef> fieldDef : fieldDefs.entrySet()) {
+            final String fieldName = fieldDef.getKey();
+            final Field<?> fieldInstance = classes.get("field." + fieldName).create().get();
+            assertEquals(String.format("Mismatch on field number for %s", fieldName), fieldDef.getValue().fieldNumber, fieldInstance.getField());
+            assertTrue(String.format("Expected %s to be an instance of %s", fieldName, fieldDef.getValue().clazz.getSimpleName()), fieldDef.getValue().clazz.isAssignableFrom(fieldInstance.getClass()));
+        }
+    }
+
+    private void validateMessages(final Map<String, Reflect> classes,  final Map<String, MessageDef> messageDefs) throws FieldNotFound {
+        for (final Map.Entry<String, MessageDef> messageDef : messageDefs.entrySet()) {
+            final String messageName = messageDef.getKey();
+            final Message messageInstance = classes.get("basic." + messageName).create().get();
+            assertEquals(String.format("Mismatch on message type for %s", messageName), messageDef.getValue().messageType, messageInstance.getHeader().getString(35));
+        }
+    }
+
+    private final class FieldDef {
+        private final int fieldNumber;
+        private final Class<? extends Field<?>> clazz;
+
+        FieldDef(final int fieldNumber, final Class<? extends Field<?>> clazz) {
+            this.fieldNumber = fieldNumber;
+            this.clazz = clazz;
+        }
+    }
+
+    private final class MessageDef {
+        private final String messageType;
+
+        MessageDef(final String messageType) {
+            this.messageType = messageType;
+        }
+    }
+}

quickfixj-core/src/test/java/quickfix/DataDictionaryTest.java

@@ -22,31 +22,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-import quickfix.field.Account;
-import quickfix.field.AvgPx;
-import quickfix.field.BodyLength;
-import quickfix.field.CheckSum;
-import quickfix.field.ClOrdID;
-import quickfix.field.HandlInst;
-import quickfix.field.LastMkt;
-import quickfix.field.MsgSeqNum;
-import quickfix.field.MsgType;
-import quickfix.field.NoHops;
-import quickfix.field.NoPartyIDs;
-import quickfix.field.NoRelatedSym;
-import quickfix.field.OrdType;
-import quickfix.field.OrderQty;
-import quickfix.field.Price;
-import quickfix.field.QuoteReqID;
-import quickfix.field.SenderCompID;
-import quickfix.field.SenderSubID;
-import quickfix.field.SendingTime;
-import quickfix.field.SessionRejectReason;
-import quickfix.field.Side;
-import quickfix.field.Symbol;
-import quickfix.field.TargetCompID;
-import quickfix.field.TimeInForce;
-import quickfix.field.TransactTime;
+import quickfix.field.*;
 import quickfix.fix44.NewOrderSingle;
 import quickfix.test.util.ExpectedTestFailure;
 
@@ -57,10 +33,7 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.hasProperty;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class DataDictionaryTest {
 
@@ -1275,6 +1248,30 @@ public void testGroupWithReqdComponentWithReqdFieldValidation() throws Exception
         dictionary.validate(quoteRequest, true);
     }
 
+    /**
+     * Field EffectiveTime(168) is defined as UTCTIMESTAMP so an empty string value is invalid but if we allow blank values that should not fail
+     * validation
+     * @throws Exception
+     */
+    @Test
+    public void testAllowingBlankValuesDisablesFieldValidation() throws Exception {
+        final DataDictionary dictionary = getDictionary();
+        dictionary.setCheckFieldsHaveValues(false);
+
+        final quickfix.fix44.NewOrderSingle newSingle = new quickfix.fix44.NewOrderSingle(
+                new ClOrdID("123"), new Side(Side.BUY), new TransactTime(), new OrdType(OrdType.LIMIT)
+        );
+        newSingle.setField(new OrderQty(42));
+        newSingle.setField(new Price(42.37));
+        newSingle.setField(new HandlInst());
+        newSingle.setField(new Symbol("QFJ"));
+        newSingle.setField(new HandlInst(HandlInst.MANUAL_ORDER_BEST_EXECUTION));
+        newSingle.setField(new TimeInForce(TimeInForce.DAY));
+        newSingle.setField(new Account("testAccount"));
+        newSingle.setField(new StringField(EffectiveTime.FIELD));
+        dictionary.validate(newSingle, true);
+    }
+
     //
     // Group Validation Tests in RepeatingGroupTest
     //