[jira] [Commented] (SOLR-5746) solr.xml parsing of "str" vs "int" vs "bool" is brittle; fails silently; expects odd type for "shareSchema"

["Hoss Man (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-5746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14057675#comment-14057675 ] 

Hoss Man commented on SOLR-5746:
--------------------------------

Maciej:

At first glance this looks awsome -- i'll try to review it more closely in the next few days.

A few quick things i noticed:

* can you update your tests to use the frameworks randomization when picking the boolean/numeric values that you put into the config strings -- instead of using hardcoded values?  that way we reduce the risk of false positives due to the code using defaults instead of the value you intended (even if the defaults change)
* can you add some asserts regarding the error message included in the SolrExceptions that are caught by the tests, so we verify that the user is getting a useful message?
* in the case where there might be multiple unexpected config keys found, can you add logging of each of the unexpected keys, and then make the exception thrown something like "Found 5 unexpected config options in solr.xml: foo, bar, baz, yak, zot"




> solr.xml parsing of "str" vs "int" vs "bool" is brittle; fails silently; expects odd type for "shareSchema"   
> --------------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-5746
>                 URL: https://issues.apache.org/jira/browse/SOLR-5746
>             Project: Solr
>          Issue Type: Bug
>    Affects Versions: 4.3, 4.4, 4.5, 4.6
>            Reporter: Hoss Man
>         Attachments: SOLR-5746.patch
>
>
> A comment in the ref guide got me looking at ConfigSolrXml.java and noticing that the parsing of solr.xml options here is very brittle and confusing.  In particular:
> * if a boolean option "foo" is expected along the lines of {{<bool name="foo">true</bool>}} it will silently ignore {{<str name="foo">true</str>}}
> * likewise for an int option {{<int name="bar">32</int>}} vs {{<str name="bar">32</str>}}
> ... this is inconsistent with the way solrconfig.xml is parsed.  In solrconfig.xml, the xml nodes are parsed into a NamedList, and the above options will work in either form, but an invalid value such as {{<bool name="foo">NOT A BOOLEAN</bool>}} will generate an error earlier (when parsing config) then {{<str name="foo">NOT A BOOLEAN</str>}} (attempt to parse the string as a bool the first time the config value is needed)
> In addition, i notice this really confusing line...
> {code}
>     propMap.put(CfgProp.SOLR_SHARESCHEMA, doSub("solr/str[@name='shareSchema']"));
> {code}
> "shareSchema" is used internally as a boolean option, but as written the parsing code will ignore it unless the user explicitly configures it as a {{<str/>}}



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org