You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by lippertto <gi...@git.apache.org> on 2016/07/31 17:55:28 UTC
[GitHub] commons-scxml pull request #2: SCXML-251: Parse src attribute of data elemen...
GitHub user lippertto opened a pull request:
https://github.com/apache/commons-scxml/pull/2
SCXML-251: Parse src attribute of data elements
Also: reject data elements which have src and data defined.
According to the scxml standard only either src or data are allowed.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lippertto/commons-scxml master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-scxml/pull/2.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2
----
commit 6f6f0f698dd81a0220cc74d6c10f545f23374bf4
Author: Tobias Lippert <li...@fastmail.com>
Date: 2016-07-31T17:41:22Z
SCXML-251: Parse src attribute of data elements
Also: reject data elements which have src and data defined.
According to the scmxl standard only src or data are allowed.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-scxml pull request #2: SCXML-251: Parse src attribute of data elemen...
Posted by woonsan <gi...@git.apache.org>.
Github user woonsan commented on a diff in the pull request:
https://github.com/apache/commons-scxml/pull/2#discussion_r72977944
--- Diff: src/test/java/org/apache/commons/scxml2/io/SCXMLReaderTest.java ---
@@ -278,6 +278,23 @@ public void testSCXMLReaderGroovyClosure() throws Exception {
Assert.assertNotNull(scxml.getGlobalScript());
}
+ @Test(expected=org.apache.commons.scxml2.model.ModelException.class)
+ public void dataWithSrcAndExprIsRejected() throws Exception {
+ SCXMLTestHelper.parse("org/apache/commons/scxml2/io/data-with-src-and-expr.xml");
+ }
+
+ @Test
+ public void srcAttributeOfDataIsParsed() throws Exception {
+ SCXML scxml = SCXMLTestHelper.parse("org/apache/commons/scxml2/io/data-with-src.xml");
+ Assert.assertNotNull(scxml);
+ Assert.assertNotNull(scxml.getDatamodel());
+ Assert.assertNotNull(scxml.getDatamodel().getData());
+ Assert.assertEquals("Exactly one data element parsed.", 1, scxml.getDatamodel().getData().size());
+ Data data = scxml.getDatamodel().getData().get(0);
+ Assert.assertNotNull(data);
+ Assert.assertEquals("http://www.w3.org/TR/sxcml", data.getSrc());
+ }
--- End diff --
Could you also add a test to read Data#getExpr()?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-scxml pull request #2: SCXML-251: Parse src attribute of data elemen...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/commons-scxml/pull/2
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-scxml issue #2: SCXML-251: Parse src attribute of data elements
Posted by lippertto <gi...@git.apache.org>.
Github user lippertto commented on the issue:
https://github.com/apache/commons-scxml/pull/2
Hello Woonsan,
thanks for the review. I added another commit which addresses your comments.
Is there anything else?
Tobias
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-scxml pull request #2: SCXML-251: Parse src attribute of data elemen...
Posted by woonsan <gi...@git.apache.org>.
Github user woonsan commented on a diff in the pull request:
https://github.com/apache/commons-scxml/pull/2#discussion_r72977373
--- Diff: src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java ---
@@ -1094,7 +1094,21 @@ private static void readData(final XMLStreamReader reader, final Configuration c
Data datum = new Data();
datum.setId(readRequiredAV(reader, ELEM_DATA, ATTR_ID));
- datum.setExpr(readAV(reader, ATTR_EXPR));
+ final String expr = readAV(reader, ATTR_EXPR);
+ final String src = readAV(reader, ATTR_SRC);
+
+ if (expr != null && src != null) {
+ LogFactory.getLog(SCXMLReader.class).error(
+ "Found src and expr attributes for data node '" + datum.getId() + "', which is not valid SCXML.");
+ throw new ModelException();
--- End diff --
I think you should invoke #reportConflictingAttribute(reader, configuration, ...) instead here. That method leaves logs in a consistent manner and can handle strict or non-strict mode properly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-scxml pull request #2: SCXML-251: Parse src attribute of data elemen...
Posted by woonsan <gi...@git.apache.org>.
Github user woonsan commented on a diff in the pull request:
https://github.com/apache/commons-scxml/pull/2#discussion_r72977815
--- Diff: src/main/java/org/apache/commons/scxml2/io/SCXMLReader.java ---
@@ -1094,7 +1094,21 @@ private static void readData(final XMLStreamReader reader, final Configuration c
Data datum = new Data();
datum.setId(readRequiredAV(reader, ELEM_DATA, ATTR_ID));
- datum.setExpr(readAV(reader, ATTR_EXPR));
+ final String expr = readAV(reader, ATTR_EXPR);
+ final String src = readAV(reader, ATTR_SRC);
+
+ if (expr != null && src != null) {
+ LogFactory.getLog(SCXMLReader.class).error(
+ "Found src and expr attributes for data node '" + datum.getId() + "', which is not valid SCXML.");
+ throw new ModelException();
+ }
+ if (expr != null) {
+ datum.setExpr(expr);
+ }
+ if (src != null) {
+ datum.setSrc(src);
--- End diff --
it's minor, but our indentation rule is 4 spaces. ;-)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---