You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2020/05/14 16:30:32 UTC

[GitHub] [incubator-daffodil] jadams-tresys opened a new pull request #383: Deep copy VariableMap when DataProcessor is copied

jadams-tresys opened a new pull request #383:
URL: https://github.com/apache/incubator-daffodil/pull/383


   This copy needs to happen now that VariableMap is mutable, so sharing
   the VariableMap with other processors that use/set the same variables
   can cause issues. Deep copying the VariableMap resolves this issue.
   
   DAFFODIL-2342


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] jadams-tresys commented on a change in pull request #383: Deep copy VariableMap when DataProcessor is copied

Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on a change in pull request #383:
URL: https://github.com/apache/incubator-daffodil/pull/383#discussion_r426183402



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestExternalVariablesNew.scala
##########
@@ -288,4 +301,45 @@ class TestExternalVariablesNew {
     }
   }
 
+  @Test def test_data_processor_vmap_copy() {
+    // Here we want to test that even when multiple var2's
+    // are defined with different namespaces that we can
+    // set the correct one.
+    //
+    val tla = {
+      <dfdl:format ref="tns:GeneralFormat"/>
+      <dfdl:defineVariable name="var1" type="xs:string" external="true" defaultValue="default1"/>
+    }
+    val sch = generateTestSchemaVmap(tla, XMLUtils.EXAMPLE_NAMESPACE)
+    val source = UnitTestSchemaSource(sch, "test_data_processor_vmap_copy")
+
+    val vars = Map(("{http://example.com}var1", "value1"))
+
+    val variables = ExternalVariablesLoader.mapToBindings(vars)
+
+    val c = Compiler(validateDFDLSchemas = false)
+
+    val pf = c.compileSource(source)
+    val sset = pf.sset
+
+    val dp1 = pf.onPath("/")
+    val dp2 = pf.onPath("/").withExternalVariables(variables)
+
+    val outputter = new ScalaXMLInfosetOutputter()
+    val input = InputSourceDataInputStream(Channels.newInputStream(Misc.stringToReadableByteChannel("")))
+
+    val res1 = dp1.parse(input, outputter)
+    assertTrue(outputter.getResult.toString.contains("default1"))
+
+    val res2 = dp2.parse(input, outputter)
+    assertTrue(outputter.getResult.toString.contains("value1"))
+
+    val res3 = dp1.parse(input, outputter)
+    assertTrue(outputter.getResult.toString.contains("default1"))
+
+    //checkResult(dp1.variableMap, "{http://example.com}var1", "default1")

Review comment:
       These were actually uncommented on my file, but I didn't write the file to disk before committing. New push has them uncommented.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] jadams-tresys commented on pull request #383: Deep copy VariableMap when DataProcessor is copied

Posted by GitBox <gi...@apache.org>.
jadams-tresys commented on pull request #383:
URL: https://github.com/apache/incubator-daffodil/pull/383#issuecomment-629414300


   Test has been added and another similar potential issue was discovered by the test and fixed


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] jadams-tresys merged pull request #383: Deep copy VariableMap when DataProcessor is copied

Posted by GitBox <gi...@apache.org>.
jadams-tresys merged pull request #383:
URL: https://github.com/apache/incubator-daffodil/pull/383


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #383: Deep copy VariableMap when DataProcessor is copied

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #383:
URL: https://github.com/apache/incubator-daffodil/pull/383#discussion_r426013001



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestExternalVariablesNew.scala
##########
@@ -288,4 +301,45 @@ class TestExternalVariablesNew {
     }
   }
 
+  @Test def test_data_processor_vmap_copy() {
+    // Here we want to test that even when multiple var2's
+    // are defined with different namespaces that we can
+    // set the correct one.
+    //
+    val tla = {
+      <dfdl:format ref="tns:GeneralFormat"/>
+      <dfdl:defineVariable name="var1" type="xs:string" external="true" defaultValue="default1"/>
+    }
+    val sch = generateTestSchemaVmap(tla, XMLUtils.EXAMPLE_NAMESPACE)
+    val source = UnitTestSchemaSource(sch, "test_data_processor_vmap_copy")
+
+    val vars = Map(("{http://example.com}var1", "value1"))
+
+    val variables = ExternalVariablesLoader.mapToBindings(vars)
+
+    val c = Compiler(validateDFDLSchemas = false)
+
+    val pf = c.compileSource(source)
+    val sset = pf.sset
+
+    val dp1 = pf.onPath("/")
+    val dp2 = pf.onPath("/").withExternalVariables(variables)
+
+    val outputter = new ScalaXMLInfosetOutputter()
+    val input = InputSourceDataInputStream(Channels.newInputStream(Misc.stringToReadableByteChannel("")))
+
+    val res1 = dp1.parse(input, outputter)
+    assertTrue(outputter.getResult.toString.contains("default1"))
+
+    val res2 = dp2.parse(input, outputter)
+    assertTrue(outputter.getResult.toString.contains("value1"))
+
+    val res3 = dp1.parse(input, outputter)
+    assertTrue(outputter.getResult.toString.contains("default1"))
+
+    //checkResult(dp1.variableMap, "{http://example.com}var1", "default1")

Review comment:
       why keep these and the definition of the checkResult method if they're not being called?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org