[
https://issues.apache.org/jira/browse/SCXML-242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15082053#comment-15082053
]
Woonsan Ko commented on SCXML-242:
----------------------------------
Great improvements and simplifications!
I have some remarks below:
First, {{ContentParser}} seems to be used privately by {{SCXMLReader}} at the
moment. If it's intended to be used that way, would it be better to mark it as
package private?
Second,
{code:title=SCXMLReader.java}
private static void readData(final XMLStreamReader reader, final
Configuration configuration, final Datamodel dm)
throws XMLStreamException, ModelException {
// SNIP
if (node.hasChildNodes()) {
NodeList children = node.getChildNodes();
if (children.getLength() == 1 && children.item(0).getNodeType() ==
Node.TEXT_NODE) {
String text =
configuration.contentParser.trimContent(children.item(0).getNodeValue());
if (configuration.contentParser.hasJsonSignature(text)) {
{code}
- Would it be an idea to simply use {{Node#getTextContent()}} instead? Then it
can allow CDATA_NODE as well with excluding comment or processing instruction.
- The {{#hasJsonSignature(String)}} is static, so
{{ContentParser.hasJsonSignature(text)}} looks better.
Finally, one minor thing is, maybe can we add commons-lang3 dependency as well?
We already have commons-io and commons-jexl. So if we can add commons-lang3,
then maybe we can replace {{ContentParser#trimContent(String)}} by
{{StringUtils#strip(String)}}, replace
{{ContentParser#spaceNormalizeContent(String)}} by
{{StringUtils#replacePattern("\s+", " ")}} and remove
{{ContentParser#isWhiteSpace(char)}}.
Anyway, this is a great improvement and thanks a lot for caring all of these.
Cheers,
Woonsan
> Provide JSON based datamodel as replacement for XML/XPath
> ---------------------------------------------------------
>
> Key: SCXML-242
> URL: https://issues.apache.org/jira/browse/SCXML-242
> Project: Commons SCXML
> Issue Type: New Feature
> Reporter: Ate Douma
> Assignee: Woonsan Ko
> Fix For: 2.0
>
>
> The XML/XPath datamodel, with 'convenient' support through Data() and
> Location() operations for usage within the Jexl, Javascript and Groovy SCXML
> languages has been the root and cause of many invasive and complicating
> problems in the implementation of Commons SCXML (2.0).
> The [SCXML 1.0 specification|http://www.w3.org/TR/2015/REC-scxml-20150901/]
> has dropped the xpath datamodel from the specification because of
> similar/same complications and (also thereof) lack of 'reference'
> implementations.
> Further maintaining and trying to 'improve' on the XML/XPath datamodel
> support in Commons SCXML therefore will be stopped and removed from the
> implementation all together.
> Instead and as replacement new JSON datamodel support will be added, which
> also aligns with the (optional) Ecmascript datamodel as described in the
> SCXML specification.
> The JSON based datamodel will be parsed (in Java) into a 'raw' object model,
> consisting of plain Maps (JSON Object) and Lists (JSON Array), and thus
> trivially to be used from the Commons SCXML supported languages (Jexl, Groovy
> and Javascript).
> As this new JSON datamodel will replace all XML/XPath usages, part of this
> new feature implementation will be updating and replacing all the
> example/unit-test usages in the codebase, and thereby also removing all
> usages of the custom Data() and Location() functions.
> Once these changes are in place, the Data() and Location() custom functions
> will then be removed as well.
> Finally, the XPath language itself then can be removed, including all the
> supporting logic scattered throughout the engine implementation itself, but
> this will be done as a separate issue afterwards.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
