Skip to content

Commit

Permalink
Some internal refactorings in AnalysisRegistry (#40609)
Browse files Browse the repository at this point in the history 
Reducing some methods scope and marking them as static where possible. Removing
"alias" support from AnalysisRegistry#produceAnalyze and changing that method to
return a NamedAnalyzer instead of having a side effect on the analyzer map passed in.
Also, CustomAnalyzerProvider doesn't seem to need the `environment` field.
  • Loading branch information
Christoph Büscher authored Apr 8, 2019
1 parent f49429b commit c95a95b
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 40 deletions.
36 changes: 14 additions & 22 deletions server/src/main/java/org/elasticsearch/index/analysis/AnalysisRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@

import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.core.WhitespaceTokenizer;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.env.Environment;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexSettings;
Expand Down Expand Up @@ -290,13 +290,13 @@ private <T> Map<String, T> buildMapping(Component component, IndexSettings setti
T factory = null;
if (typeName == null) {
if (currentSettings.get("tokenizer") != null) {
factory = (T) new CustomAnalyzerProvider(settings, name, currentSettings, environment);
factory = (T) new CustomAnalyzerProvider(settings, name, currentSettings);
} else {
throw new IllegalArgumentException(component + " [" + name + "] " +
"must specify either an analyzer type, or a tokenizer");
}
} else if (typeName.equals("custom")) {
factory = (T) new CustomAnalyzerProvider(settings, name, currentSettings, environment);
factory = (T) new CustomAnalyzerProvider(settings, name, currentSettings);
}
if (factory != null) {
factories.put(name, factory);
Expand Down Expand Up @@ -427,8 +427,10 @@ public IndexAnalyzers build(IndexSettings indexSettings,
Map<String, NamedAnalyzer> normalizers = new HashMap<>();
Map<String, NamedAnalyzer> whitespaceNormalizers = new HashMap<>();
for (Map.Entry<String, AnalyzerProvider<?>> entry : analyzerProviders.entrySet()) {
processAnalyzerFactory(indexSettings, entry.getKey(), entry.getValue(), analyzers,
tokenFilterFactoryFactories, charFilterFactoryFactories, tokenizerFactoryFactories);
analyzers.merge(entry.getKey(), produceAnalyzer(entry.getKey(), entry.getValue(), tokenFilterFactoryFactories,
charFilterFactoryFactories, tokenizerFactoryFactories), (k, v) -> {
throw new IllegalStateException("already registered analyzer with name: " + entry.getKey());
});
}
for (Map.Entry<String, AnalyzerProvider<?>> entry : normalizerProviders.entrySet()) {
processNormalizerFactory(entry.getKey(), entry.getValue(), normalizers, "keyword",
Expand All @@ -438,9 +440,9 @@ public IndexAnalyzers build(IndexSettings indexSettings,
}

if (!analyzers.containsKey("default")) {
processAnalyzerFactory(indexSettings, "default", new StandardAnalyzerProvider(indexSettings, null,
"default", Settings.Builder.EMPTY_SETTINGS),
analyzers, tokenFilterFactoryFactories, charFilterFactoryFactories, tokenizerFactoryFactories);
NamedAnalyzer defaultAnalyzer = produceAnalyzer("default", new StandardAnalyzerProvider(indexSettings, null, "default",
Settings.Builder.EMPTY_SETTINGS), tokenFilterFactoryFactories, charFilterFactoryFactories, tokenizerFactoryFactories);
analyzers.put("default", defaultAnalyzer);
}
if (!analyzers.containsKey("default_search")) {
analyzers.put("default_search", analyzers.get("default"));
Expand Down Expand Up @@ -470,11 +472,9 @@ public IndexAnalyzers build(IndexSettings indexSettings,
whitespaceNormalizers);
}

private void processAnalyzerFactory(IndexSettings indexSettings,
String name,
AnalyzerProvider<?> analyzerFactory,
Map<String, NamedAnalyzer> analyzers, Map<String, TokenFilterFactory> tokenFilters,
Map<String, CharFilterFactory> charFilters, Map<String, TokenizerFactory> tokenizers) {
private static NamedAnalyzer produceAnalyzer(String name, AnalyzerProvider<?> analyzerFactory,
Map<String, TokenFilterFactory> tokenFilters, Map<String, CharFilterFactory> charFilters,
Map<String, TokenizerFactory> tokenizers) {
/*
* Lucene defaults positionIncrementGap to 0 in all analyzers but
* Elasticsearch defaults them to 0 only before version 2.0
Expand Down Expand Up @@ -508,15 +508,7 @@ private void processAnalyzerFactory(IndexSettings indexSettings,
} else {
analyzer = new NamedAnalyzer(name, analyzerFactory.scope(), analyzerF, overridePositionIncrementGap);
}
if (analyzers.containsKey(name)) {
throw new IllegalStateException("already registered analyzer with name: " + name);
}
analyzers.put(name, analyzer);
// TODO: remove alias support completely when we no longer support pre 5.0 indices
final String analyzerAliasKey = "index.analysis.analyzer." + analyzerFactory.name() + ".alias";
if (indexSettings.getSettings().get(analyzerAliasKey) != null) {
throw new IllegalArgumentException("setting [" + analyzerAliasKey + "] is not supported");
}
return analyzer;
}

private void processNormalizerFactory(
Expand Down
7 changes: 2 additions & 5 deletions server/src/main/java/org/elasticsearch/index/analysis/CustomAnalyzerProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.elasticsearch.index.analysis;

import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.mapper.TextFieldMapper;

Expand All @@ -35,18 +34,16 @@
public class CustomAnalyzerProvider extends AbstractIndexAnalyzerProvider<CustomAnalyzer> {

private final Settings analyzerSettings;
private final Environment environment;

private CustomAnalyzer customAnalyzer;

public CustomAnalyzerProvider(IndexSettings indexSettings,
String name, Settings settings, Environment environment) {
String name, Settings settings) {
super(indexSettings, name, settings);
this.analyzerSettings = settings;
this.environment = environment;
}

public void build(final Map<String, TokenizerFactory> tokenizers, final Map<String, CharFilterFactory> charFilters,
void build(final Map<String, TokenizerFactory> tokenizers, final Map<String, CharFilterFactory> charFilters,
final Map<String, TokenFilterFactory> tokenFilters) {
String tokenizerName = analyzerSettings.get("tokenizer");
if (tokenizerName == null) {
Expand Down
13 changes: 0 additions & 13 deletions server/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,6 @@ public void testSimpleConfigurationYaml() throws IOException {
testSimpleConfiguration(settings);
}

public void testAnalyzerAliasNotAllowedPost5x() throws IOException {
Settings settings = Settings.builder()
.put("index.analysis.analyzer.foobar.type", "standard")
.put("index.analysis.analyzer.foobar.alias","foobaz")
// analyzer aliases were removed in v5.0.0 alpha6
.put(IndexMetaData.SETTING_VERSION_CREATED, VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, null))
.put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
.build();
AnalysisRegistry registry = getNewRegistry(settings);
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> getIndexAnalyzers(registry, settings));
assertEquals("setting [index.analysis.analyzer.foobar.alias] is not supported", e.getMessage());
}

public void testVersionedAnalyzers() throws Exception {
String yaml = "/org/elasticsearch/index/analysis/test1.yml";
Settings settings2 = Settings.builder()
Expand Down

0 comments on commit c95a95b

Please sign in to comment.