[ 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