You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Woonsan Ko (JIRA)" <ji...@apache.org> on 2016/01/05 01:09:39 UTC

[jira] [Commented] (SCXML-242) Provide JSON based datamodel as replacement for XML/XPath

    [ 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)