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

Chris M. Hostetter commented on SOLR-16203:
-------------------------------------------

{quote}As a simplistic and conservative option, does it make sense to forbid 
the combination of ClassicIndexSchemaFactory and the SPI lookups (if it's 
possible to detect which SchemaFactory is used in the FieldTypePluginLoader)?
{quote}
That seems like a step backwards instead of a step forwards?

This doesn't seem like a "serious" enough bug that we need to rush out a patch 
release that rips out functionality (ie: it doesn't cause data loss or any 
glaring security holes) ... once you understand what's going "wrong" it's easy 
to fix/workaround ... it's just not immediately obvious what's going wrong 
since the "failure" doesn't happen until you try to use the analyzer.
{quote}I think the fix would be to correctly call the inform(ResourceLoader). 
Where's the problem?
{quote}
Probably, yes ... I think there may be some other solr specific things that the 
SolrResourceLoader does when it instantiates things that would also need to be 
audited (Which is really what confuses me: how/why are those things happening 
when ManagedIndexSchemaFactory is used ... is ManagedIndexSchemaFactory 
duplicating some code that is in SolrResourceLoader .. is it doing things 
inconsistently?)
{quote}In general I was talking with Robert already: We should change the 
Analysis factories in Lucene to get rid of this inform() calls. As with the 
parameter map, it should give a ResourceLoader instance as constructor 
parameter. This is the correct way to handle this.
{quote}
That seems like a new LUCENE Jira – but FWIW I'm not really sure where/how you 
would make that work with "factory factories" like 
{{TokenFilterFactory.forName()}} since not all analysis components need/care 
about {{ResourceLoader}}  ... would you add a {{ResourceLoader}} argument to 
those {{forName}} methods and use it if and only if the SPI class that get 
resolved takes a {{ResourceLoader}} as one of it's constructor args?  (That's 
why {{ResourceLoaderAware}} was created in the first place – so that _after_ 
you resolved the String -> Class mapping you could inspect the class to know if 
it needs a way to load resource files)
{quote}Perhaps the real long-term solution here should be to scrap our aged 
home-grown dependency injection mechanisms ...
{quote}
No objection, but that certainly seems like something that would have t wait 
for a Solr 10 – not something we can do in a bug fix release.

> Using SPI lookups of analysis components results in factories that don't get 
> ResourceLoaderAware.inform called on them (When using 
> ClassicIndexSchemaFactory)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-16203
>                 URL: https://issues.apache.org/jira/browse/SOLR-16203
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>    Affects Versions: 9.0
>            Reporter: Chris M. Hostetter
>            Priority: Major
>         Attachments: SOLR-16203_test.patch
>
>
> Ok ... this is a weird one ...
> SOLR-13593 modified {{FieldTypePluginLoader}} to add the ability for analysis 
> Factories to be resolved by SPI name (ex: {{{}name="stop"{}}}, instead of by 
> "short class name" (ex: {{{}class="solr.StopFilterFactory"{}}}.
> The code that deals with this just calls (for example) 
> {{TokenFilterFactory.forName(name, params);}} and uses the resulting factory 
> "as is" – But these means that nothing calls the {{inform(ResourceLoader)}} 
> on any factories that implement {{ResourceLoaderAware}} (such as 
> {{StopFilterFactory}} or {{{}SynonymGraphFilterFactory{}}})
> In the "short class name" code path (the only option prior to 9.0), the 
> {{SolrResourceLoader}} is used to initialize the Factory, and 
> {{SolrResourceLoader..newInstance(...)}} takes responsibility of calling 
> {{factory.inform(this)}} on everything it instantiates that implements 
> {{ResourceLoaderAware}} (as well as some other checks: like 
> {{{}SolrCoreAware{}}})
> This discrepancy means that when using the {{name="foo"}} syntax, many 
> factories won't be fully initialized – or fail on invalid input – during 
> schema initialization. In the case of things like {{StopFilterFactory}} or 
> {{SynonymGraphFilterFactory}}) the problem will manifest as some type of 
> runtime error when the factory's {{create(...)}} method is called as part of 
> creating a new index or query Analyzer.
> ----
> _*...BUT...*_
> ----
> This problem only seems to manifest itself when using 
> {{ClassicIndexSchemaFactory}} -- which is why it's not readily apparent when 
> using the default configset, or something like {{bin/solr -e techproducts}} 
> (but is trivial to reproduce in testcases (since almost every "test" 
> solrconfig uses {{ClassicIndexSchemaFactory}})
> I have no idea _why_ using {{ManagedIndexSchemaFactory}} doesn't manifest the 
> same problem -- it should be using the same {{FieldTypePluginLoader}} under 
> the covers -- but clearly something specific to {{ManagedIndexSchema}} is 
> taking responsibility for calling {{ResourceLoaderAware.inform(...)}}



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

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

Reply via email to