[ 
https://issues.apache.org/jira/browse/SOLR-5526?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Vitaliy Zhovtyuk updated SOLR-5526:
-----------------------------------

    Attachment: SOLR-5526.patch

minor fixes:
{quote} QParserPlugin.standardPlugins's javadoc needs to point out the 
importance of these names being static & final so people aren't surprised by 
these  tests when new parsers are added in the future.{quote}
Added relevant javadocs.
{quote}
TestStandardQParsers is doing something sufficiently odd that it really needs 
some javadocs explaining why it exists (ie: mention the class loading problems 
associated if there is a standardPlugin that has a non-static, non-final name, 
with an {{@see}} this issue, {{@see QParserPlugin.standardPlugins}}, 
etc...){quote}
Added javadocs
{quote} we should probably make TestStandardQParsers assert that the static & 
final name it finds in each class matches the name associated in 
QParserPlugin.standardPlugins.{quote}
Thats actually what TestStandardQParsers does. Unit test takes classes 
registered in QParserPlugin.standardPlugins and ensure that each class has 
final and static NAME field.
Added relevant javadocs to TestStandardQParsers.
{quote} solrconfig-query-parser-init.xml has a cut & paste comment referring to 
an unrelated test.{quote}
Fixed, added relevant comments.

{quote} TestInitQParser should have a javadoc comment explaining what the point 
of the test is{quote}
Fixed, added relevant comments.

{quote}TestInitQParser should actaully do a query using the "fail" parser 
registered in the config, to help future-proof us against someone unwittingly 
changing the test config in a way that defeats the point of the test.{quote}
This test alerady does query using defType=fail, so i expect this registered 
QParser used and return result.

> Query parser extends standard cause NPE on Solr startup
> -------------------------------------------------------
>
>                 Key: SOLR-5526
>                 URL: https://issues.apache.org/jira/browse/SOLR-5526
>             Project: Solr
>          Issue Type: Bug
>          Components: query parsers
>    Affects Versions: 4.5.1, 4.6, 5.0
>         Environment: Linux, Java 1.7.0_45
>            Reporter: Nikolay Khitrin
>            Priority: Blocker
>         Attachments: NPE_load_trace, SOLR-5526-final-names.patch, 
> SOLR-5526-final-names.patch, SOLR-5526-tests.patch, SOLR-5526.patch, 
> SOLR-5526.patch, SOLR-5526.patch
>
>
> Adding any custom query parser extending standard one with non-final field 
> {{NAME}} lead to messy {{NullPointerException}} during Solr startup.
> Definition of standard parsers is located in  
> {{QParserPlugin.standardPlugins}} static array. The array contains names from 
> static {{NAME}} fields and classes for each plugin.                       
> But all of listed parsers are derived from {{QParserPlugin}}, so we have 
> circular dependency of static fields.
> Normally, class loader start initializing {{QParserPlugin}} before all listed 
> plugins in {{SolrCore.initQParsers}}, and array elements referenced to 
> {{NAME}} plugins' fields are filled properly.
> Custom parsers are instantiated before standard parsers. And when we subclass 
> plugin with non-final {{NAME}} field and add it to Solr via 
> {{solrconfig.xml}}, class loader start loading our class before 
> {{QParserPlugin}}. Because {{QParserPlugin}} is a superclass for plugin, it 
> must be initialized before subclasses, and static dereferencing cause null 
> elements in {{standardPlugins}} array because it filled before {{NAME}} field 
> of loading plugin's superclass.
> How to reproduce:
> # Checkout Solr (trunk or stable)
> # Add the following line to solr/example/solr/collection1/conf/solrconfig.xml
>   {{<queryParser name="fail" class="solr.search.LuceneQParserPlugin"/>}}
> # Call ant run-example in solr folder
> Possible workarounds:
> * dev-workaround: add {{int workaround = 
> QParserPlugin.standardPlugins.length;}} as a first line to
>   {{SolrCore.initQParsers}}
> * user-workaround: add plugin with final {{NAME}} field (edismax) to 
> solrconfig.xml  before subclasses of standard plugins. 
>   {{<queryParser name="workaround" 
> class="solr.search.ExtendedDismaxQParserPlugin"/>}}
>   
> Possible fix:
> Move {{standardPlugins}} to new final class to break circular dependency.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

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

Reply via email to