[jira] Created: (SOLR-78) [PATCH] Improved config loading

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
[PATCH] Improved config loading
-------------------------------

                 Key: SOLR-78
                 URL: http://issues.apache.org/jira/browse/SOLR-78
             Project: Solr
          Issue Type: Improvement
            Reporter: Alexander Saar
            Priority: Minor


Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.

A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
     [ http://issues.apache.org/jira/browse/SOLR-78?page=all ]

Alexander Saar updated SOLR-78:
-------------------------------

    Attachment: fixed-config-loading-with-older-parser-versions.patch

Patch that fixes the problem described by this issue.

> [PATCH] Improved config loading
> -------------------------------
>
>                 Key: SOLR-78
>                 URL: http://issues.apache.org/jira/browse/SOLR-78
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Alexander Saar
>            Priority: Minor
>         Attachments: fixed-config-loading-with-older-parser-versions.patch
>
>
> Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.
> A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
In reply to this post by Tim Allison (Jira)
    [ http://issues.apache.org/jira/browse/SOLR-78?page=comments#action_12456987 ]
           
Hoss Man commented on SOLR-78:
------------------------------

FWIW: I don't know a lot about DOM2 vs DOM3, but loking over the documentation on the meaning of textContent I see...

      http://www.w3.org/TR/DOM-Level-3-Core/core.html
      ...
      [forsome node types]
      concatenation of the textContent attribute value of every child node, excluding COMMENT_NODE and
      PROCESSING_INSTRUCTION_NODE nodes. This is the empty string if the node has no children.
      [for other node types including comments and processing instructions]
      nodeValue

If I'm reading that right, then if we replace usages of getTextContent with our own method for walking the child nodes and pulling out the nodeValue, we need to explicitly watch out for processing instructions and comments or we will start slurping them in my accident right?

either way: DOMUtil.getText(Node) should have some javadoc clarifying it's purpsoe (support DOM 2) so it doesn't accidently get optimized away by someone down the road who says "hey, we can just use getTextContent()!"

> [PATCH] Improved config loading
> -------------------------------
>
>                 Key: SOLR-78
>                 URL: http://issues.apache.org/jira/browse/SOLR-78
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Alexander Saar
>            Priority: Minor
>         Attachments: fixed-config-loading-with-older-parser-versions.patch
>
>
> Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.
> A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] Assigned: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
In reply to this post by Tim Allison (Jira)
     [ http://issues.apache.org/jira/browse/SOLR-78?page=all ]

Hoss Man reassigned SOLR-78:
----------------------------

    Assignee: Hoss Man

I tried to produce an example of the type of thing i'm worried about with the body of a comment getting slurped up ... but i could not -- as far as i can tell that's actually a defect of the DOM parser, the spec says it should be there.

Either way it should be safe and easy to check the Node type and only include nodes that aren't comments or processing instructions, i'll try to do that tomorow.

> [PATCH] Improved config loading
> -------------------------------
>
>                 Key: SOLR-78
>                 URL: http://issues.apache.org/jira/browse/SOLR-78
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Alexander Saar
>         Assigned To: Hoss Man
>            Priority: Minor
>         Attachments: fixed-config-loading-with-older-parser-versions.patch
>
>
> Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.
> A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
In reply to this post by Tim Allison (Jira)
     [ http://issues.apache.org/jira/browse/SOLR-78?page=all ]

Hoss Man updated SOLR-78:
-------------------------

    Attachment: fixed-config-loading-with-older-parser-versions.patch

revised version of patch that attempts to impliment the DOM Spec for "textContent" as precicesly as possible....

http://www.w3.org/TR/DOM-Level-3-Core/core.html#Node3-textContent

It seems to work properly, but I'd appreciate some eyeballs on this before I commit.

> [PATCH] Improved config loading
> -------------------------------
>
>                 Key: SOLR-78
>                 URL: http://issues.apache.org/jira/browse/SOLR-78
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Alexander Saar
>         Assigned To: Hoss Man
>            Priority: Minor
>         Attachments: fixed-config-loading-with-older-parser-versions.patch, fixed-config-loading-with-older-parser-versions.patch
>
>
> Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.
> A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
In reply to this post by Tim Allison (Jira)
    [ http://issues.apache.org/jira/browse/SOLR-78?page=comments#action_12458600 ]
           
Yonik Seeley commented on SOLR-78:
----------------------------------

I'm no DOM expert, but it looks OK.
If it works on all our current tests/configs then it's a low risk change since that's all it's used for.

Hmmm, but *how* do we know it's reading all the values OK, instead of getting a default or something.
Perhaps run a startup test with the debugging level pumped all the way up, one before the change, and one after, then diff the logs (minus timestamps/linenumbers)?

Oh, and StringBuffer could be StringBuilder, but it's private so I don't really care.

> [PATCH] Improved config loading
> -------------------------------
>
>                 Key: SOLR-78
>                 URL: http://issues.apache.org/jira/browse/SOLR-78
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Alexander Saar
>         Assigned To: Hoss Man
>            Priority: Minor
>         Attachments: fixed-config-loading-with-older-parser-versions.patch, fixed-config-loading-with-older-parser-versions.patch
>
>
> Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.
> A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] Resolved: (SOLR-78) [PATCH] Improved config loading

Tim Allison (Jira)
In reply to this post by Tim Allison (Jira)
     [ http://issues.apache.org/jira/browse/SOLR-78?page=all ]

Hoss Man resolved SOLR-78.
--------------------------

    Resolution: Fixed

Changed StringBuffer to StringBuilder (i keep forgetting about that class) and then tried out Yonik's suggestion (using the full test suite we have since some config access is done on an as needed basis) and the only diffs were in default object toStrings, and the names of temp directories created for indexes (AbstractSolrTestcase puts the time and a random number in the name when making the directory)

commited.

(Thanks for the initial patch Alexander, this is the kind of thing that never would have occured to me, but shoudl really help reduce frustrations new users might have)



> [PATCH] Improved config loading
> -------------------------------
>
>                 Key: SOLR-78
>                 URL: http://issues.apache.org/jira/browse/SOLR-78
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Alexander Saar
>         Assigned To: Hoss Man
>            Priority: Minor
>         Attachments: fixed-config-loading-with-older-parser-versions.patch, fixed-config-loading-with-older-parser-versions.patch
>
>
> Solr configuration throws an UnsupportedConfigurationException for Node.getTextContent() if it is started in server environments that uses older implementations of the DOM API. This can be improved by checking wich node type is actually handled an calling the appropriate methods.
> A patch that fixes this problem is attached.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira