issue connamara#134 - DD "required=N" assumed if "required" is not pr…

…esent
wywialm · Dec 12, 2012 · bfb34f7 · bfb34f7
1 parent 7ea8357
commit bfb34f7
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 8 deletions.
diff --git a/NEXT_VERSION.md b/NEXT_VERSION.md
@@ -17,6 +17,7 @@ Changes since the last version:
 -------------------------------
 * (major/minor/patch) desc of fixed issue (contributor)
 
+* (patch) issue #134 - if DD field/group/component is missing "required" attribute, treat it as "required=N" (gbirchmeier)
 * (minor) issue #101 - better exception for when group doesn't use proper delimiter (gbirchmeier)
 * (minor) issue #28 - rename interfaces to start with "I" (gbirchmeier)
 * (patch) issue #128 - simplify/improve TradeClient example app (gbirchmeier)

diff --git a/QuickFIXn/DataDictionary/DataDictionary.cs b/QuickFIXn/DataDictionary/DataDictionary.cs
@@ -510,23 +510,40 @@ private void parseMsgEl(XmlNode node, DDMap ddmap)
         /// </param>
 		private void parseMsgEl(XmlNode node, DDMap ddmap, bool? componentRequired)
 		{
+            ///* 
+            // This code is great for debugging DD parsing issues.
+            string s = "+ " + node.Name;
+            if (node.Attributes["name"] != null)
+                s += " | " + node.Attributes["name"].Value;
+            Console.WriteLine(s);
+            //*/
+
 			if (!node.HasChildNodes) { return; }
 			foreach (XmlNode childNode in node.ChildNodes)
 			{
+                ///*
+                // Continuation of code that's great for debugging DD parsing issues.
+                s = "    + " + childNode.Name;
+                if (node.Attributes["name"] != null)
+                    s += " | " + childNode.Attributes["name"].Value;
+                Console.WriteLine(s);
+                //*/
+
 				if( childNode.Name == "field" )
 				{
 					DDField fld = FieldsByName[childNode.Attributes["name"].Value];
-					if (childNode.Attributes["required"].Value == "Y"
-                        && (componentRequired==null || componentRequired.Value==true))
-					{
-						ddmap.ReqFields.Add(fld.Tag);
-					}
+                    XmlAttribute req = childNode.Attributes["required"];
+                    if (req != null && req.Value == "Y"
+                        && (componentRequired == null || componentRequired.Value == true))
+                    {
+                        ddmap.ReqFields.Add(fld.Tag);
+                    }
 					if (!ddmap.IsField(fld.Tag))
 					{
 						ddmap.Fields.Add(fld.Tag, fld);
 					}
 
-					// if first field in group, make it the DELIM <3 FIX
+					// if first field in group, make it the DELIM
 					if ((ddmap.GetType() == typeof(DDGrp) && ((DDGrp)ddmap).Delim == 0))
 					{
 						((DDGrp)ddmap).Delim = fld.Tag;
@@ -536,7 +553,8 @@ private void parseMsgEl(XmlNode node, DDMap ddmap, bool? componentRequired)
 				{
 					DDField fld = FieldsByName[childNode.Attributes["name"].Value];
 					DDGrp grp = new DDGrp();
-					if (childNode.Attributes["required"].Value == "Y"
+                    XmlAttribute req = childNode.Attributes["required"];
+                    if (req != null && req.Value == "Y"
                         && (componentRequired == null || componentRequired.Value == true))
 					{
 						ddmap.ReqFields.Add(fld.Tag);
@@ -554,7 +572,8 @@ private void parseMsgEl(XmlNode node, DDMap ddmap, bool? componentRequired)
                 {
                     String name = childNode.Attributes["name"].Value;
                     XmlNode compNode = RootDoc.SelectSingleNode("//components/component[@name='" + name + "']");
-                    bool? compRequired = (childNode.Attributes["required"].Value == "Y");
+                    XmlAttribute req = childNode.Attributes["required"];
+                    bool? compRequired = (req != null && req.Value == "Y");
                     parseMsgEl(compNode, ddmap, compRequired);
                 }
 			}

diff --git a/UnitTests/DataDictionaryTests.cs b/UnitTests/DataDictionaryTests.cs
@@ -422,5 +422,17 @@ public void ComponentFieldsRequirements()
             Assert.False(dd.Messages["AD"].ReqFields.Contains(55));
             Assert.True(dd.Messages["7"].ReqFields.Contains(55));
         }
+
+        [Test]
+        public void Issue134_RequiredIsOptional()
+        {
+            QuickFix.DataDictionary.DataDictionary dd = new QuickFix.DataDictionary.DataDictionary("../../../spec/test/required_is_optional.xml");
+            Assert.True(dd.Messages["magic"].ReqFields.Contains(1111));  //base required field
+            Assert.False(dd.Messages["magic"].ReqFields.Contains(5555)); //base optional field
+            Assert.False(dd.Messages["magic"].ReqFields.Contains(5556)); //component optional field
+
+            Assert.False(dd.Messages["magic"].Groups[6660].Required); // group isn't required
+            Assert.False(dd.Messages["magic"].Groups[6660].ReqFields.Contains(6662)); // group optional field
+        }
     }
 }
diff --git a/spec/test/required_is_optional.xml b/spec/test/required_is_optional.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  This dictionary lacks 'required' attributes for fields and components.
+  Per issue 134, the lack of 'required' should imply 'required=N'.
+-->
+<fix minor="9" major="9">
+  <header/>
+  <trailer/>
+  <messages>
+    <message name="MagicMessage" msgtype="magic" msgcat="app">
+      <field name="MundaneField" required="Y"/>
+      <field name="MagicField"/>
+      <component name="MagicComponent"/>
+      <group name="NoMagicGroups">
+        <field name="Delim"/>
+        <field name="Blah"/>
+      </group>
+    </message>
+  </messages>
+
+  <components>
+    <component name="MagicComponent">
+      <field name="MagicComponentField"/>
+    </component>
+  </components>
+
+  <fields>
+    <field number="1111" name="MundaneField" type="STRING"/>
+    <field number="5555" name="MagicField" type="STRING"/>
+    <field number="5556" name="MagicComponentField" type="STRING"/>
+    <field number="6660" name="NoMagicGroups" type="NUMINGROUP"/>
+    <field number="6661" name="Delim" type="STRING"/>
+    <field number="6662" name="Blah" type="STRING"/>
+  </fields>
+</fix>