From 6ad57bf393bda06da1c463d7c276d4a61d3cb299 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 16 Jul 2024 08:16:11 -0400 Subject: [PATCH] Add matchesPluginSystemIndexPattern to SystemIndexRegistry (#14750) * Add matchesPluginSystemIndexPattern to SystemIndexRegistry Signed-off-by: Craig Perkins * Add to CHANGELOG Signed-off-by: Craig Perkins * Use single data structure to keep track of system indices Signed-off-by: Craig Perkins * Address code review comments Signed-off-by: Craig Perkins * Add test for getAllDescriptors Signed-off-by: Craig Perkins * Update server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java Co-authored-by: Andriy Redko Signed-off-by: Craig Perkins --------- Signed-off-by: Craig Perkins Signed-off-by: Craig Perkins Co-authored-by: Andriy Redko --- CHANGELOG.md | 1 + .../indices/SystemIndexRegistry.java | 36 ++++--- .../org/opensearch/indices/SystemIndices.java | 5 +- .../main/java/org/opensearch/node/Node.java | 5 +- .../indices/SystemIndicesTests.java | 100 ++++++++++++++---- 5 files changed, 112 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f666dbf3b8d5..7050e7d23b24d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Workload Management] add queryGroupId header propagator across requests and nodes ([#14614](https://github.com/opensearch-project/OpenSearch/pull/14614)) - Create SystemIndexRegistry with helper method matchesSystemIndex ([#14415](https://github.com/opensearch-project/OpenSearch/pull/14415)) - Print reason why parent task was cancelled ([#14604](https://github.com/opensearch-project/OpenSearch/issues/14604)) +- Add matchesPluginSystemIndexPattern to SystemIndexRegistry ([#14750](https://github.com/opensearch-project/OpenSearch/pull/14750)) ### Dependencies - Bump `org.gradle.test-retry` from 1.5.8 to 1.5.9 ([#13442](https://github.com/opensearch-project/OpenSearch/pull/13442)) diff --git a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java index d9608e220d924..ab2cbd4ef1a73 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndexRegistry.java @@ -15,13 +15,13 @@ import org.opensearch.common.regex.Regex; import org.opensearch.tasks.TaskResultsService; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; import static java.util.Collections.singletonList; @@ -45,25 +45,35 @@ public class SystemIndexRegistry { ); private volatile static String[] SYSTEM_INDEX_PATTERNS = new String[0]; - volatile static Collection SYSTEM_INDEX_DESCRIPTORS = Collections.emptyList(); + private volatile static Map> SYSTEM_INDEX_DESCRIPTORS_MAP = Collections.emptyMap(); static void register(Map> pluginAndModulesDescriptors) { final Map> descriptorsMap = buildSystemIndexDescriptorMap(pluginAndModulesDescriptors); checkForOverlappingPatterns(descriptorsMap); - List descriptors = pluginAndModulesDescriptors.values() - .stream() - .flatMap(Collection::stream) - .collect(Collectors.toList()); - descriptors.add(TASK_INDEX_DESCRIPTOR); - SYSTEM_INDEX_DESCRIPTORS = descriptors.stream().collect(Collectors.toUnmodifiableList()); - SYSTEM_INDEX_PATTERNS = descriptors.stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); + SYSTEM_INDEX_DESCRIPTORS_MAP = descriptorsMap; + SYSTEM_INDEX_PATTERNS = getAllDescriptors().stream().map(SystemIndexDescriptor::getIndexPattern).toArray(String[]::new); } - public static List matchesSystemIndexPattern(String... indexExpressions) { - return Arrays.stream(indexExpressions) - .filter(pattern -> Regex.simpleMatch(SYSTEM_INDEX_PATTERNS, pattern)) - .collect(Collectors.toList()); + public static Set matchesSystemIndexPattern(Set indexExpressions) { + return indexExpressions.stream().filter(pattern -> Regex.simpleMatch(SYSTEM_INDEX_PATTERNS, pattern)).collect(Collectors.toSet()); + } + + public static Set matchesPluginSystemIndexPattern(String pluginClassName, Set indexExpressions) { + if (!SYSTEM_INDEX_DESCRIPTORS_MAP.containsKey(pluginClassName)) { + return Collections.emptySet(); + } + String[] pluginSystemIndexPatterns = SYSTEM_INDEX_DESCRIPTORS_MAP.get(pluginClassName) + .stream() + .map(SystemIndexDescriptor::getIndexPattern) + .toArray(String[]::new); + return indexExpressions.stream() + .filter(pattern -> Regex.simpleMatch(pluginSystemIndexPatterns, pattern)) + .collect(Collectors.toSet()); + } + + static List getAllDescriptors() { + return SYSTEM_INDEX_DESCRIPTORS_MAP.values().stream().flatMap(Collection::stream).collect(Collectors.toList()); } /** diff --git a/server/src/main/java/org/opensearch/indices/SystemIndices.java b/server/src/main/java/org/opensearch/indices/SystemIndices.java index bbf58fe91512f..6e9e5e7707877 100644 --- a/server/src/main/java/org/opensearch/indices/SystemIndices.java +++ b/server/src/main/java/org/opensearch/indices/SystemIndices.java @@ -63,7 +63,7 @@ public class SystemIndices { public SystemIndices(Map> pluginAndModulesDescriptors) { SystemIndexRegistry.register(pluginAndModulesDescriptors); - this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS); + this.runAutomaton = buildCharacterRunAutomaton(SystemIndexRegistry.getAllDescriptors()); } /** @@ -91,7 +91,8 @@ public boolean isSystemIndex(String indexName) { * @throws IllegalStateException if multiple descriptors match the name */ public @Nullable SystemIndexDescriptor findMatchingDescriptor(String name) { - final List matchingDescriptors = SystemIndexRegistry.SYSTEM_INDEX_DESCRIPTORS.stream() + final List matchingDescriptors = SystemIndexRegistry.getAllDescriptors() + .stream() .filter(descriptor -> descriptor.matchesIndexPattern(name)) .collect(Collectors.toList()); diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 96a716af7f1a1..281f697d9bb79 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -699,7 +699,10 @@ protected Node( pluginsService.filterPlugins(SystemIndexPlugin.class) .stream() .collect( - Collectors.toMap(plugin -> plugin.getClass().getSimpleName(), plugin -> plugin.getSystemIndexDescriptors(settings)) + Collectors.toMap( + plugin -> plugin.getClass().getCanonicalName(), + plugin -> plugin.getSystemIndexDescriptors(settings) + ) ) ); final SystemIndices systemIndices = new SystemIndices(systemIndexDescriptorMap); diff --git a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java index 8ac457c32d53a..ca9370645dec3 100644 --- a/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java +++ b/server/src/test/java/org/opensearch/indices/SystemIndicesTests.java @@ -44,6 +44,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; @@ -155,32 +157,61 @@ public void testSystemIndexMatching() { ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index2"), - equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1", ".system-index2")), + equalTo(Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2)) ); - assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".system-index1"), equalTo(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1))); - assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".system-index2"), equalTo(List.of(SystemIndexPlugin2.SYSTEM_INDEX_2))); - assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern1"), equalTo(List.of(".system-index-pattern1"))); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern-sub*"), - equalTo(List.of(".system-index-pattern-sub*")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1")), + equalTo(Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1)) ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index-pattern1", ".system-index-pattern2"), - equalTo(List.of(".system-index-pattern1", ".system-index-pattern2")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index2")), + equalTo(Set.of(SystemIndexPlugin2.SYSTEM_INDEX_2)) ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1"), - equalTo(List.of(".system-index1", ".system-index-pattern1")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index-pattern1")), + equalTo(Set.of(".system-index-pattern1")) ); assertThat( - SystemIndexRegistry.matchesSystemIndexPattern(".system-index1", ".system-index-pattern1", ".not-system"), - equalTo(List.of(".system-index1", ".system-index-pattern1")) + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index-pattern-sub*")), + equalTo(Set.of(".system-index-pattern-sub*")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index-pattern1", ".system-index-pattern2")), + equalTo(Set.of(".system-index-pattern1", ".system-index-pattern2")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1", ".system-index-pattern1")), + equalTo(Set.of(".system-index1", ".system-index-pattern1")) + ); + assertThat( + SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".system-index1", ".system-index-pattern1", ".not-system")), + equalTo(Set.of(".system-index1", ".system-index-pattern1")) + ); + assertThat(SystemIndexRegistry.matchesSystemIndexPattern(Set.of(".not-system")), equalTo(Collections.emptySet())); + } + + public void testRegisteredSystemIndexGetAllDescriptors() { + SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); + SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndices pluginSystemIndices = new SystemIndices( + Map.of( + SystemIndexPlugin1.class.getCanonicalName(), + plugin1.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPlugin2.class.getCanonicalName(), + plugin2.getSystemIndexDescriptors(Settings.EMPTY) + ) + ); + assertEquals( + SystemIndexRegistry.getAllDescriptors() + .stream() + .map(SystemIndexDescriptor::getIndexPattern) + .collect(Collectors.toUnmodifiableList()), + List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2, TASK_INDEX + "*") ); - assertThat(SystemIndexRegistry.matchesSystemIndexPattern(".not-system"), equalTo(Collections.emptyList())); } - public void testRegisteredSystemIndexExpansion() { + public void testRegisteredSystemIndexMatching() { SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); SystemIndices pluginSystemIndices = new SystemIndices( @@ -191,12 +222,43 @@ public void testRegisteredSystemIndexExpansion() { plugin2.getSystemIndexDescriptors(Settings.EMPTY) ) ); - List systemIndices = SystemIndexRegistry.matchesSystemIndexPattern( - SystemIndexPlugin1.SYSTEM_INDEX_1, - SystemIndexPlugin2.SYSTEM_INDEX_2 + Set systemIndices = SystemIndexRegistry.matchesSystemIndexPattern( + Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2) ); assertEquals(2, systemIndices.size()); - assertTrue(systemIndices.containsAll(List.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); + assertTrue(systemIndices.containsAll(Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2))); + } + + public void testRegisteredSystemIndexMatchingForPlugin() { + SystemIndexPlugin plugin1 = new SystemIndexPlugin1(); + SystemIndexPlugin plugin2 = new SystemIndexPlugin2(); + SystemIndices pluginSystemIndices = new SystemIndices( + Map.of( + SystemIndexPlugin1.class.getCanonicalName(), + plugin1.getSystemIndexDescriptors(Settings.EMPTY), + SystemIndexPlugin2.class.getCanonicalName(), + plugin2.getSystemIndexDescriptors(Settings.EMPTY) + ) + ); + Set systemIndicesForPlugin1 = SystemIndexRegistry.matchesPluginSystemIndexPattern( + SystemIndexPlugin1.class.getCanonicalName(), + Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2, "other-index") + ); + assertEquals(1, systemIndicesForPlugin1.size()); + assertTrue(systemIndicesForPlugin1.contains(SystemIndexPlugin1.SYSTEM_INDEX_1)); + + Set systemIndicesForPlugin2 = SystemIndexRegistry.matchesPluginSystemIndexPattern( + SystemIndexPlugin2.class.getCanonicalName(), + Set.of(SystemIndexPlugin1.SYSTEM_INDEX_1, SystemIndexPlugin2.SYSTEM_INDEX_2, "other-index") + ); + assertEquals(1, systemIndicesForPlugin2.size()); + assertTrue(systemIndicesForPlugin2.contains(SystemIndexPlugin2.SYSTEM_INDEX_2)); + + Set noMatchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern( + SystemIndexPlugin2.class.getCanonicalName(), + Set.of("other-index") + ); + assertEquals(0, noMatchingSystemIndices.size()); } static final class SystemIndexPlugin1 extends Plugin implements SystemIndexPlugin {