Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Register stable plugins in ActionModule #90067

Merged
merged 21 commits into from
Sep 21, 2022
Merged

Register stable plugins in ActionModule #90067

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
2a2179b
Scanning for Stable Plugin components
pgomulka Sep 9, 2022
f59692a
small cleanup
pgomulka Sep 9, 2022
c508977
remove example use
pgomulka Sep 9, 2022
515071b
Update docs/changelog/89969.yaml
pgomulka Sep 9, 2022
58726df
fix javadoc
pgomulka Sep 9, 2022
5927de1
Merge branch 'plugin_scanning_just_files' of github.com:pgomulka/elas…
pgomulka Sep 9, 2022
e722af2
code review followup
pgomulka Sep 12, 2022
fadaf34
path to resources
pgomulka Sep 12, 2022
6534f34
move file
pgomulka Sep 12, 2022
bfc4275
char filter working
pgomulka Sep 13, 2022
9d6583f
precommit
pgomulka Sep 13, 2022
e0162d8
registration for all components
pgomulka Sep 13, 2022
a7bc36c
tests in progress
pgomulka Sep 13, 2022
d3e102a
tests finished without normalizer
pgomulka Sep 14, 2022
c468474
Merge remote-tracking branch 'upstream/main' into register_plugins
pgomulka Sep 14, 2022
2e4a471
fix tests
pgomulka Sep 15, 2022
cfe4ec4
Merge remote-tracking branch 'upstream/main' into register_plugins
pgomulka Sep 15, 2022
c289eca
Update docs/changelog/90067.yaml
pgomulka Sep 15, 2022
2a81e5a
normalizer tests
pgomulka Sep 15, 2022
e932f91
Merge remote-tracking branch 'pgomulka/register_plugins' into registe…
pgomulka Sep 15, 2022
e25c5b1
foreach
pgomulka Sep 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/90067.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 90067
summary: Register stable plugins in `ActionModule`
area: Infra/Plugins
type: enhancement
issues: []
3 changes: 2 additions & 1 deletion ...nalysis-common/src/test/java/org/elasticsearch/analysis/common/CompoundAnalysisTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.indices.analysis.AnalysisModule.AnalysisProvider;
import org.elasticsearch.plugins.AnalysisPlugin;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.IndexSettingsModule;
import org.hamcrest.MatcherAssert;
Expand Down Expand Up @@ -85,7 +86,7 @@ private AnalysisModule createAnalysisModule(Settings settings) throws IOExceptio
public Map<String, AnalysisProvider<TokenFilterFactory>> getTokenFilters() {
return singletonMap("myfilter", MyFilterTokenFilterFactory::new);
}
}));
}), new StablePluginsRegistry());
}

private Settings getJsonSettings() throws IOException {
Expand Down
9 changes: 6 additions & 3 deletions ...lysis-common/src/test/java/org/elasticsearch/analysis/common/EdgeNGramTokenizerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
import org.elasticsearch.test.ESTokenStreamTestCase;
import org.elasticsearch.test.IndexSettingsModule;
import org.elasticsearch.test.VersionUtils;
Expand All @@ -36,9 +37,11 @@ private IndexAnalyzers buildAnalyzers(Version version, String tokenizer) throws
.put("index.analysis.analyzer.my_analyzer.tokenizer", tokenizer)
.build();
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("index", indexSettings);
return new AnalysisModule(TestEnvironment.newEnvironment(settings), Collections.singletonList(new CommonAnalysisPlugin()))
.getAnalysisRegistry()
.build(idxSettings);
return new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(new CommonAnalysisPlugin()),
new StablePluginsRegistry()
).getAnalysisRegistry().build(idxSettings);
}

public void testPreConfiguredTokenizer() throws IOException {
Expand Down
7 changes: 5 additions & 2 deletions ...s-common/src/test/java/org/elasticsearch/analysis/common/MultiplexerTokenFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
import org.elasticsearch.test.ESTokenStreamTestCase;
import org.elasticsearch.test.IndexSettingsModule;

Expand All @@ -41,7 +42,8 @@ public void testMultiplexingFilter() throws IOException {

IndexAnalyzers indexAnalyzers = new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(new CommonAnalysisPlugin())
Collections.singletonList(new CommonAnalysisPlugin()),
new StablePluginsRegistry()
).getAnalysisRegistry().build(idxSettings);

try (NamedAnalyzer analyzer = indexAnalyzers.get("myAnalyzer")) {
Expand Down Expand Up @@ -75,7 +77,8 @@ public void testMultiplexingNoOriginal() throws IOException {

IndexAnalyzers indexAnalyzers = new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(new CommonAnalysisPlugin())
Collections.singletonList(new CommonAnalysisPlugin()),
new StablePluginsRegistry()
).getAnalysisRegistry().build(idxSettings);

try (NamedAnalyzer analyzer = indexAnalyzers.get("myAnalyzer")) {
Expand Down
7 changes: 6 additions & 1 deletion ...mmon/src/test/java/org/elasticsearch/analysis/common/PredicateTokenScriptFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
Expand Down Expand Up @@ -60,7 +61,11 @@ public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryTyp

CommonAnalysisPlugin plugin = new CommonAnalysisPlugin();
plugin.createComponents(null, null, null, null, scriptService, null, null, null, null, null, null, Tracer.NOOP, null);
AnalysisModule module = new AnalysisModule(TestEnvironment.newEnvironment(settings), Collections.singletonList(plugin));
AnalysisModule module = new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(plugin),
new StablePluginsRegistry()
);

IndexAnalyzers analyzers = module.getAnalysisRegistry().build(idxSettings);

Expand Down
7 changes: 6 additions & 1 deletion ...on/src/test/java/org/elasticsearch/analysis/common/ScriptedConditionTokenFilterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.index.analysis.IndexAnalyzers;
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
Expand Down Expand Up @@ -60,7 +61,11 @@ public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryTyp

CommonAnalysisPlugin plugin = new CommonAnalysisPlugin();
plugin.createComponents(null, null, null, null, scriptService, null, null, null, null, null, null, Tracer.NOOP, null);
AnalysisModule module = new AnalysisModule(TestEnvironment.newEnvironment(settings), Collections.singletonList(plugin));
AnalysisModule module = new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(plugin),
new StablePluginsRegistry()
);

IndexAnalyzers analyzers = module.getAnalysisRegistry().build(idxSettings);

Expand Down
7 changes: 5 additions & 2 deletions ...est/java/org/elasticsearch/analysis/common/WordDelimiterGraphTokenFilterFactoryTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.elasticsearch.index.analysis.NamedAnalyzer;
import org.elasticsearch.index.analysis.TokenFilterFactory;
import org.elasticsearch.indices.analysis.AnalysisModule;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.IndexSettingsModule;
import org.elasticsearch.test.VersionUtils;
Expand Down Expand Up @@ -193,7 +194,8 @@ public void testPreconfiguredFilter() throws IOException {
try (
IndexAnalyzers indexAnalyzers = new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(new CommonAnalysisPlugin())
Collections.singletonList(new CommonAnalysisPlugin()),
new StablePluginsRegistry()
).getAnalysisRegistry().build(idxSettings)
) {

Expand All @@ -217,7 +219,8 @@ public void testPreconfiguredFilter() throws IOException {
try (
IndexAnalyzers indexAnalyzers = new AnalysisModule(
TestEnvironment.newEnvironment(settings),
Collections.singletonList(new CommonAnalysisPlugin())
Collections.singletonList(new CommonAnalysisPlugin()),
new StablePluginsRegistry()
).getAnalysisRegistry().build(idxSettings)
) {

Expand Down
6 changes: 6 additions & 0 deletions server/src/main/java/org/elasticsearch/common/NamedRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,10 @@ public <P> void extractAndRegister(List<P> plugins, Function<P, Map<String, T>>
}
}
}

public void register(Map<String, T> collect) {
for (Map.Entry<String, T> entry : collect.entrySet()) {
thecoop marked this conversation as resolved.
Show resolved Hide resolved
register(entry.getKey(), entry.getValue());
}
}
}
41 changes: 32 additions & 9 deletions server/src/main/java/org/elasticsearch/indices/analysis/AnalysisModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@
import org.elasticsearch.index.analysis.TokenFilterFactory;
import org.elasticsearch.index.analysis.TokenizerFactory;
import org.elasticsearch.index.analysis.WhitespaceAnalyzerProvider;
import org.elasticsearch.indices.analysis.wrappers.StableApiWrappers;
import org.elasticsearch.plugins.AnalysisPlugin;
import org.elasticsearch.plugins.scanners.StablePluginsRegistry;

import java.io.IOException;
import java.util.List;
Expand Down Expand Up @@ -68,13 +70,18 @@ public final class AnalysisModule {
private final HunspellService hunspellService;
private final AnalysisRegistry analysisRegistry;

public AnalysisModule(Environment environment, List<AnalysisPlugin> plugins) throws IOException {
NamedRegistry<AnalysisProvider<CharFilterFactory>> charFilters = setupCharFilters(plugins);
public AnalysisModule(Environment environment, List<AnalysisPlugin> plugins, StablePluginsRegistry stablePluginRegistry)
thecoop marked this conversation as resolved.
Show resolved Hide resolved
throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to retain the 2-arg constructor, which can delegate to the 3-arg with a new StablePluginsRegistry(), as the third arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from the "production" code point of view it makes no sense. Using the 2arg constructor would be wrong (but analysisModule is created in one place only anyway).
It would simplify the testing though. Like in this PR a big chunk of changes are test changes due to this constructor.

Do you think we should have a 2arg constructor for the sake of these tests?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

na! just a q.

NamedRegistry<AnalysisProvider<CharFilterFactory>> charFilters = setupCharFilters(plugins, stablePluginRegistry);
NamedRegistry<org.apache.lucene.analysis.hunspell.Dictionary> hunspellDictionaries = setupHunspellDictionaries(plugins);
hunspellService = new HunspellService(environment.settings(), environment, hunspellDictionaries.getRegistry());
NamedRegistry<AnalysisProvider<TokenFilterFactory>> tokenFilters = setupTokenFilters(plugins, hunspellService);
NamedRegistry<AnalysisProvider<TokenizerFactory>> tokenizers = setupTokenizers(plugins);
NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> analyzers = setupAnalyzers(plugins);
NamedRegistry<AnalysisProvider<TokenFilterFactory>> tokenFilters = setupTokenFilters(
plugins,
hunspellService,
stablePluginRegistry
);
NamedRegistry<AnalysisProvider<TokenizerFactory>> tokenizers = setupTokenizers(plugins, stablePluginRegistry);
NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> analyzers = setupAnalyzers(plugins, stablePluginRegistry);
NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> normalizers = setupNormalizers(plugins);

Map<String, PreConfiguredCharFilter> preConfiguredCharFilters = setupPreConfiguredCharFilters(plugins);
Expand Down Expand Up @@ -104,9 +111,14 @@ public AnalysisRegistry getAnalysisRegistry() {
return analysisRegistry;
}

private static NamedRegistry<AnalysisProvider<CharFilterFactory>> setupCharFilters(List<AnalysisPlugin> plugins) {
private static NamedRegistry<AnalysisProvider<CharFilterFactory>> setupCharFilters(
List<AnalysisPlugin> plugins,
StablePluginsRegistry stablePluginRegistry
) {
NamedRegistry<AnalysisProvider<CharFilterFactory>> charFilters = new NamedRegistry<>("char_filter");
charFilters.extractAndRegister(plugins, AnalysisPlugin::getCharFilters);

charFilters.register(StableApiWrappers.oldApiForStableCharFilterFactory(stablePluginRegistry));
return charFilters;
}

Expand All @@ -118,7 +130,8 @@ public static NamedRegistry<org.apache.lucene.analysis.hunspell.Dictionary> setu

private static NamedRegistry<AnalysisProvider<TokenFilterFactory>> setupTokenFilters(
List<AnalysisPlugin> plugins,
HunspellService hunspellService
HunspellService hunspellService,
StablePluginsRegistry stablePluginRegistry
) {
NamedRegistry<AnalysisProvider<TokenFilterFactory>> tokenFilters = new NamedRegistry<>("token_filter");
tokenFilters.register("stop", StopTokenFilterFactory::new);
Expand Down Expand Up @@ -157,6 +170,8 @@ public boolean requiresAnalysisSettings() {
);

tokenFilters.extractAndRegister(plugins, AnalysisPlugin::getTokenFilters);
tokenFilters.register(StableApiWrappers.oldApiForTokenFilterFactory(stablePluginRegistry));

return tokenFilters;
}

Expand Down Expand Up @@ -241,14 +256,21 @@ static Map<String, PreConfiguredTokenizer> setupPreConfiguredTokenizers(List<Ana
return unmodifiableMap(preConfiguredTokenizers.getRegistry());
}

private static NamedRegistry<AnalysisProvider<TokenizerFactory>> setupTokenizers(List<AnalysisPlugin> plugins) {
private static NamedRegistry<AnalysisProvider<TokenizerFactory>> setupTokenizers(
List<AnalysisPlugin> plugins,
StablePluginsRegistry stablePluginRegistry
) {
NamedRegistry<AnalysisProvider<TokenizerFactory>> tokenizers = new NamedRegistry<>("tokenizer");
tokenizers.register("standard", StandardTokenizerFactory::new);
tokenizers.extractAndRegister(plugins, AnalysisPlugin::getTokenizers);
tokenizers.register(StableApiWrappers.oldApiForTokenizerFactory(stablePluginRegistry));
return tokenizers;
}

private static NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> setupAnalyzers(List<AnalysisPlugin> plugins) {
private static NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> setupAnalyzers(
List<AnalysisPlugin> plugins,
StablePluginsRegistry stablePluginRegistry
) {
NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> analyzers = new NamedRegistry<>("analyzer");
analyzers.register("default", StandardAnalyzerProvider::new);
analyzers.register("standard", StandardAnalyzerProvider::new);
Expand All @@ -257,6 +279,7 @@ private static NamedRegistry<AnalysisProvider<AnalyzerProvider<?>>> setupAnalyze
analyzers.register("whitespace", WhitespaceAnalyzerProvider::new);
analyzers.register("keyword", KeywordAnalyzerProvider::new);
analyzers.extractAndRegister(plugins, AnalysisPlugin::getAnalyzers);
analyzers.register(StableApiWrappers.oldApiForAnalyzerFactory(stablePluginRegistry));
return analyzers;
}

Expand Down
Loading