SOLR-13797 SolrResourceLoader no longer caches bad results when asked for wrong type

This commit is contained in:
Mike Drob 2019-09-26 17:21:22 -05:00
parent a57ec148e5
commit 2d3baf6e8f
3 changed files with 31 additions and 4 deletions

View File

@ -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.

View File

@ -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<String, String> 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);
}
}
}

View File

@ -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<String,String> 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);
}
}