diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index bc27b0abc0d..d6dd8377a5a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -78,6 +78,8 @@ Other Changes * SOLR-13655:Upgrade Collections.unModifiableSet to Set.of and Set.copyOf (Atri Sharma via Tomás Fernández Löbbe) +* SOLR-13797: SolrResourceLoader no longer caches bad results when asked for wrong type (Mike Drob) + ================== 8.3.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java index b3dc5e450d9..f19268e9326 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java +++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java @@ -52,6 +52,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import com.google.common.annotations.VisibleForTesting; import org.apache.lucene.analysis.WordlistLoader; import org.apache.lucene.analysis.util.CharFilterFactory; import org.apache.lucene.analysis.util.ResourceLoader; @@ -479,6 +480,11 @@ public class SolrResourceLoader implements ResourceLoader,Closeable */ private static final Map classNameCache = new ConcurrentHashMap<>(); + @VisibleForTesting + static void clearCache() { + classNameCache.clear(); + } + // Using this pattern, legacy analysis components from previous Solr versions are identified and delegated to SPI loader: private static final Pattern legacyAnalysisPattern = Pattern.compile("((\\Q"+base+".analysis.\\E)|(\\Q"+project+".\\E))([\\p{L}_$][\\p{L}\\p{N}_$]+?)(TokenFilter|Filter|Tokenizer|CharFilter)Factory"); @@ -506,11 +512,11 @@ public class SolrResourceLoader implements ResourceLoader,Closeable if(c != null) { try { return Class.forName(c, true, classLoader).asSubclass(expectedType); - } catch (ClassNotFoundException e) { - //this is unlikely - log.error("Unable to load cached class-name : "+ c +" for shortname : "+cname + e); + } catch (ClassNotFoundException | ClassCastException e) { + // this can happen if the legacyAnalysisPattern below caches the wrong thing + log.warn("Unable to load cached class, attempting lookup. name={} shortname={} reason={}", c, cname, e); + classNameCache.remove(cname); } - } } diff --git a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java index eb88dbb3bb2..0c175537713 100644 --- a/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java +++ b/solr/core/src/test/org/apache/solr/core/ResourceLoaderTest.java @@ -34,6 +34,7 @@ import org.apache.lucene.analysis.core.KeywordTokenizerFactory; import org.apache.lucene.analysis.ngram.NGramFilterFactory; import org.apache.lucene.analysis.util.ResourceLoaderAware; import org.apache.lucene.analysis.util.TokenFilterFactory; +import org.apache.lucene.analysis.util.TokenizerFactory; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.SolrException; import org.apache.solr.handler.admin.LukeRequestHandler; @@ -42,6 +43,7 @@ import org.apache.solr.response.JSONResponseWriter; import org.apache.solr.util.plugin.SolrCoreAware; import static org.apache.solr.core.SolrResourceLoader.assertAwareCompatibility; +import static org.apache.solr.core.SolrResourceLoader.clearCache; import static org.hamcrest.core.Is.is; public class ResourceLoaderTest extends SolrTestCaseJ4 { @@ -206,4 +208,21 @@ public class ResourceLoaderTest extends SolrTestCaseJ4 { // TODO: How to check that a warning was printed to log file? loader.close(); } + + public void testCacheWrongType() { + clearCache(); + + SolrResourceLoader loader = new SolrResourceLoader(); + Class[] params = { Map.class }; + Map args = Map.of("minGramSize", "1", "maxGramSize", "2"); + final String className = "solr.NGramTokenizerFactory"; + + // We could fail here since the class name and expected type don't match, but instead we try to infer what the user actually meant + TokenFilterFactory tff = loader.newInstance(className, TokenFilterFactory.class, new String[0], params, new Object[]{new HashMap<>(args)}); + assertNotNull("Did not load TokenFilter when asking for corresponding Tokenizer", tff); + + // This should work, but won't if earlier call succeeding corrupting the cache + TokenizerFactory tf = loader.newInstance(className, TokenizerFactory.class, new String[0], params, new Object[]{new HashMap<>(args)}); + assertNotNull("Did not load Tokenizer after bad call earlier", tf); + } }