From a735dc742b98f934e9ee675f6805b8a41eabc7af Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Tue, 2 Jan 2018 11:42:44 +0000 Subject: [PATCH 1/3] Changes to ensure stable ordering in messages --- .../main/java/quickfix/DataDictionary.java | 72 +++++++++++++++++-- .../src/main/java/quickfix/FieldMap.java | 6 +- .../src/main/java/quickfix/Message.java | 2 +- .../java/quickfix/DataDictionaryTest.java | 24 +++++-- .../src/test/java/quickfix/FieldMapTest.java | 9 ++- 5 files changed, 100 insertions(+), 13 deletions(-) diff --git a/quickfixj-core/src/main/java/quickfix/DataDictionary.java b/quickfixj-core/src/main/java/quickfix/DataDictionary.java index a8732dec06..b2f9300f98 100644 --- a/quickfixj-core/src/main/java/quickfix/DataDictionary.java +++ b/quickfixj-core/src/main/java/quickfix/DataDictionary.java @@ -87,6 +87,8 @@ public class DataDictionary { private final IntegerStringMap valueNames = new IntegerStringMap<>(); private final StringIntegerMap groups = new StringIntegerMap<>(); private final Map components = new HashMap<>(); + private int[] orderedFieldsArray; + private DataDictionary() { } @@ -243,7 +245,7 @@ public boolean isAppMessage(String msgType) { } private void addMsgField(String msgType, int field) { - messageFields.computeIfAbsent(msgType, k -> new HashSet<>()).add(field); + messageFields.computeIfAbsent(msgType, k -> new LinkedHashSet<>()).add(field); } /** @@ -294,7 +296,7 @@ public int getFieldTag(String name) { } private void addRequiredField(String msgType, int field) { - requiredFields.computeIfAbsent(msgType, k -> new HashSet<>()).add(field); + requiredFields.computeIfAbsent(msgType, k -> new LinkedHashSet<>()).add(field); } /** @@ -330,7 +332,7 @@ public boolean isRequiredTrailerField(int field) { } private void addFieldValue(int field, String value) { - fieldValues.computeIfAbsent(field, k -> new HashSet<>()).add(value); + fieldValues.computeIfAbsent(field, k -> new LinkedHashSet<>()).add(value); } /** @@ -1069,8 +1071,6 @@ private void load(Document document, String msgtype, Node node) throws ConfigErr } } - private int[] orderedFieldsArray; - public int[] getOrderedFields() { if (orderedFieldsArray == null) { orderedFieldsArray = new int[fields.size()]; @@ -1083,6 +1083,68 @@ public int[] getOrderedFields() { return orderedFieldsArray; } + /** + * Concatenates the Integer Sets into a single int[], with the header followed + * by the fields and finally the trailer. + * @param fieldsCollection + * @param headerCollection + * @param trailerCollection + * @return + */ + private int[] getOrderedFieldsFrom(Set fieldsCollection, + Set headerCollection, Set trailerCollection) { + + if (fieldsCollection == null) { + fieldsCollection = new LinkedHashSet(); + } + if (headerCollection == null) { + headerCollection = new LinkedHashSet(); + } + if (trailerCollection == null) { + trailerCollection = new LinkedHashSet(); + } + + int[] fields = new int[fieldsCollection.size() + headerCollection.size() + trailerCollection.size()]; + Integer[] headerArray = headerCollection.toArray(new Integer[headerCollection.size()]); + Integer[] fieldsArray = fieldsCollection.toArray(new Integer[fieldsCollection.size()]); + Integer[] trailerArray = trailerCollection.toArray(new Integer[trailerCollection.size()]); + + int overallIndex = 0; + + for (; overallIndex < headerArray.length; overallIndex++) + fields[overallIndex] = headerArray[overallIndex].intValue(); + + for (int i = 0; i < fieldsArray.length; i++, overallIndex++) + fields[overallIndex] = fieldsArray[i].intValue(); + + for (int i = 0; i < trailerArray.length; i++, overallIndex++) + fields[overallIndex] = trailerArray[i].intValue(); + + return fields; + } + + /** + * Returns the required ordered fields for a message type, including the Header and Trailer. + * @param messageType + * @return + */ + public int[] getOrderedRequiredFieldsForMessage(String messageType){ + return getOrderedFieldsFrom( + requiredFields.get(messageType), requiredFields.get(HEADER_ID), requiredFields.get(TRAILER_ID)); + } + + /** + * Returns the ordered fields for a message type, including the Header and Trailer. + * @param messageType + * @return + */ + public int[] getOrderedFieldsForMessage(String messageType){ + return getOrderedFieldsFrom( + messageFields.get(messageType), messageFields.get(HEADER_ID), messageFields.get(TRAILER_ID)); + } + + + private int lookupXMLFieldNumber(Document document, Node node) throws ConfigError { final Element element = (Element) node; if (!element.hasAttribute("name")) { diff --git a/quickfixj-core/src/main/java/quickfix/FieldMap.java b/quickfixj-core/src/main/java/quickfix/FieldMap.java index fc87e66c04..e4b8a6b994 100644 --- a/quickfixj-core/src/main/java/quickfix/FieldMap.java +++ b/quickfixj-core/src/main/java/quickfix/FieldMap.java @@ -50,7 +50,7 @@ public abstract class FieldMap implements Serializable { private final int[] fieldOrder; - private final TreeMap> fields; + private final Map> fields; private final TreeMap> groups = new TreeMap<>(); @@ -62,7 +62,9 @@ public abstract class FieldMap implements Serializable { */ protected FieldMap(int[] fieldOrder) { this.fieldOrder = fieldOrder; - fields = new TreeMap<>(fieldOrder != null ? new FieldOrderComparator() : null); + fields = (fieldOrder != null) ? + new TreeMap<>(new FieldOrderComparator()) : + new LinkedHashMap<>(); } protected FieldMap() { diff --git a/quickfixj-core/src/main/java/quickfix/Message.java b/quickfixj-core/src/main/java/quickfix/Message.java index 301ff802c8..7b7529b16e 100644 --- a/quickfixj-core/src/main/java/quickfix/Message.java +++ b/quickfixj-core/src/main/java/quickfix/Message.java @@ -85,7 +85,7 @@ public Message() { initializeHeader(); } - protected Message(int[] fieldOrder) { + public Message(int[] fieldOrder) { super(fieldOrder); initializeHeader(); } diff --git a/quickfixj-core/src/test/java/quickfix/DataDictionaryTest.java b/quickfixj-core/src/test/java/quickfix/DataDictionaryTest.java index 117d908116..5b5f413d08 100644 --- a/quickfixj-core/src/test/java/quickfix/DataDictionaryTest.java +++ b/quickfixj-core/src/test/java/quickfix/DataDictionaryTest.java @@ -55,13 +55,11 @@ import java.math.BigDecimal; import java.net.URL; import java.net.URLClassLoader; +import java.util.Arrays; 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 { @@ -105,6 +103,24 @@ public void testDictionary() throws Exception { assertFalse(dd.isMsgField("UNKNOWN_TYPE", 1)); } + @Test + public void testGetOrderedRequiredFieldsForMessage() throws Exception { + DataDictionary dictionary = getDictionary(); + assertArrayEquals("incorrect field ordering", new int[]{8, 9, 35, 49, 56, 34, 52, 98, 108, 10}, + dictionary.getOrderedRequiredFieldsForMessage("A")); + } + + @Test + public void testGetOrderedFieldsForMessage() throws Exception { + DataDictionary dictionary = getDictionary(); + assertArrayEquals("incorrect field ordering", + new int[]{8, 9, 35, 49, 56, 115, 128, 90, 91, 34, 50, 142, 57, 143, 116, 144, 129, 145, + 43, 97, 52, 122, 212, 213, 347, 369, 627, 98, 108, 95, 96, 141, 789, 383, 384, + 464, 553, 554, 93, 89, 10}, + dictionary.getOrderedFieldsForMessage("A")); + } + + @Test public void testMissingFieldAttributeForRequired() throws Exception { String data = ""; diff --git a/quickfixj-core/src/test/java/quickfix/FieldMapTest.java b/quickfixj-core/src/test/java/quickfix/FieldMapTest.java index 9eb376a5c0..860fa9a8a6 100644 --- a/quickfixj-core/src/test/java/quickfix/FieldMapTest.java +++ b/quickfixj-core/src/test/java/quickfix/FieldMapTest.java @@ -30,6 +30,13 @@ public static Test suite() { return new TestSuite(FieldMapTest.class); } + public void testDefaultFieldOrderIsInsertionOrdering() { + FieldMap map = new Message(); + map.setString(2, "A"); + map.setString(1, "B"); + assertEquals("9=82=A1=B10=017",map.toString()); + } + public void testSetUtcTimeStampField() throws Exception { FieldMap map = new Message(); LocalDateTime aDate = LocalDateTime.now(); @@ -82,7 +89,7 @@ private void testOrdering(int[] vals, int[] order, int[] expected) { public void testOrdering() { testOrdering(new int[] { 1, 2, 3 }, null, new int[] { 1, 2, 3 }); - testOrdering(new int[] { 3, 2, 1 }, null, new int[] { 1, 2, 3 }); + testOrdering(new int[] { 3, 2, 1 }, null, new int[] { 3, 2, 1 }); testOrdering(new int[] { 1, 2, 3 }, new int[] { 1, 2, 3 }, new int[] { 1, 2, 3 }); testOrdering(new int[] { 3, 2, 1 }, new int[] { 1, 2, 3 }, new int[] { 1, 2, 3 }); testOrdering(new int[] { 1, 2, 3 }, new int[] { 1, 3, 2 }, new int[] { 1, 3, 2 }); From 3494f21ef508013ecc30f811ce8f761e8f227e08 Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Thu, 22 Nov 2018 12:24:45 +0000 Subject: [PATCH 2/3] Keep field order when cloning --- quickfixj-core/src/main/java/quickfix/FieldMap.java | 1 + 1 file changed, 1 insertion(+) diff --git a/quickfixj-core/src/main/java/quickfix/FieldMap.java b/quickfixj-core/src/main/java/quickfix/FieldMap.java index e4b8a6b994..c0db63e4b1 100644 --- a/quickfixj-core/src/main/java/quickfix/FieldMap.java +++ b/quickfixj-core/src/main/java/quickfix/FieldMap.java @@ -442,6 +442,7 @@ public Iterator> iterator() { } protected void initializeFrom(FieldMap source) { + fieldOrder = source.getFieldOrder(); fields.clear(); fields.putAll(source.fields); for (Entry> entry : source.groups.entrySet()) { From 73aa45c28baa96ee8650220e52a0371956c3dc27 Mon Sep 17 00:00:00 2001 From: Philip Whitehouse Date: Fri, 16 Aug 2019 16:51:14 +0100 Subject: [PATCH 3/3] Remove final modifier from fieldOrder --- quickfixj-core/src/main/java/quickfix/FieldMap.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/quickfixj-core/src/main/java/quickfix/FieldMap.java b/quickfixj-core/src/main/java/quickfix/FieldMap.java index c0db63e4b1..a65adfac31 100644 --- a/quickfixj-core/src/main/java/quickfix/FieldMap.java +++ b/quickfixj-core/src/main/java/quickfix/FieldMap.java @@ -48,7 +48,7 @@ public abstract class FieldMap implements Serializable { static final long serialVersionUID = -3193357271891865972L; - private final int[] fieldOrder; + private int[] fieldOrder; private final Map> fields;