Fix "synonym_graph filter fails with word_delimiter_graph when using whitespace or classic tokenizer in synonym_analyzer" bug #18037

add 'analyzersBuiltSoFar' to getChainAwareTokenFilterFactory to build custom analyzers depending on other (already built) analyzers
The analyzers are built following the order of precedence specified in the settings

Signed-off-by: Lamine Idjeraoui <lidjeraoui@apple.com>

Author: Lamine Idjeraoui

modules/analysis-common/src/main/java/org/opensearch/analysis/common/MultiplexerTokenFilterFactory.java

@@ -32,6 +32,7 @@
 
 package org.opensearch.analysis.common;
 
+import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.TokenFilter;
 import org.apache.lucene.analysis.TokenStream;
 import org.apache.lucene.analysis.miscellaneous.ConditionalTokenFilter;
@@ -77,7 +78,8 @@ public TokenFilterFactory getChainAwareTokenFilterFactory(
         TokenizerFactory tokenizer,
         List<CharFilterFactory> charFilters,
         List<TokenFilterFactory> previousTokenFilters,
-        Function<String, TokenFilterFactory> allFilters
+        Function<String, TokenFilterFactory> allFilters,
+        Function<String, Analyzer> analyzersBuiltSoFar
     ) {
         List<TokenFilterFactory> filters = new ArrayList<>();
         if (preserveOriginal) {
@@ -89,15 +91,27 @@ public TokenFilterFactory getChainAwareTokenFilterFactory(
             String[] parts = Strings.tokenizeToStringArray(filter, ",");
             if (parts.length == 1) {
                 TokenFilterFactory factory = resolveFilterFactory(allFilters, parts[0]);
-                factory = factory.getChainAwareTokenFilterFactory(tokenizer, charFilters, previousTokenFilters, allFilters);
+                factory = factory.getChainAwareTokenFilterFactory(
+                    tokenizer,
+                    charFilters,
+                    previousTokenFilters,
+                    allFilters,
+                    analyzersBuiltSoFar
+                );
                 filters.add(factory);
                 mode = mode.merge(factory.getAnalysisMode());
             } else {
                 List<TokenFilterFactory> existingChain = new ArrayList<>(previousTokenFilters);
                 List<TokenFilterFactory> chain = new ArrayList<>();
                 for (String subfilter : parts) {
                     TokenFilterFactory factory = resolveFilterFactory(allFilters, subfilter);
-                    factory = factory.getChainAwareTokenFilterFactory(tokenizer, charFilters, existingChain, allFilters);
+                    factory = factory.getChainAwareTokenFilterFactory(
+                        tokenizer,
+                        charFilters,
+                        existingChain,
+                        allFilters,
+                        analyzersBuiltSoFar
+                    );
                     chain.add(factory);
                     existingChain.add(factory);
                     mode = mode.merge(factory.getAnalysisMode());

modules/analysis-common/src/main/java/org/opensearch/analysis/common/ScriptedConditionTokenFilterFactory.java

@@ -32,6 +32,7 @@
 
 package org.opensearch.analysis.common;
 
+import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.TokenStream;
 import org.apache.lucene.analysis.miscellaneous.ConditionalTokenFilter;
 import org.opensearch.common.settings.Settings;
@@ -84,7 +85,8 @@ public TokenFilterFactory getChainAwareTokenFilterFactory(
         TokenizerFactory tokenizer,
         List<CharFilterFactory> charFilters,
         List<TokenFilterFactory> previousTokenFilters,
-        Function<String, TokenFilterFactory> allFilters
+        Function<String, TokenFilterFactory> allFilters,
+        Function<String, Analyzer> analyzersBuiltSoFar
     ) {
         List<TokenFilterFactory> filters = new ArrayList<>();
         List<TokenFilterFactory> existingChain = new ArrayList<>(previousTokenFilters);
@@ -95,7 +97,7 @@ public TokenFilterFactory getChainAwareTokenFilterFactory(
                     "ScriptedConditionTokenFilter [" + name() + "] refers to undefined token filter [" + filter + "]"
                 );
             }
-            tff = tff.getChainAwareTokenFilterFactory(tokenizer, charFilters, existingChain, allFilters);
+            tff = tff.getChainAwareTokenFilterFactory(tokenizer, charFilters, existingChain, allFilters, analyzersBuiltSoFar);
             filters.add(tff);
             existingChain.add(tff);
         }

modules/analysis-common/src/main/java/org/opensearch/analysis/common/SynonymGraphTokenFilterFactory.java

@@ -70,9 +70,10 @@ public TokenFilterFactory getChainAwareTokenFilterFactory(
         TokenizerFactory tokenizer,
         List<CharFilterFactory> charFilters,
         List<TokenFilterFactory> previousTokenFilters,
-        Function<String, TokenFilterFactory> allFilters
+        Function<String, TokenFilterFactory> allFilters,
+        Function<String, Analyzer> analyzersBuiltSoFar
     ) {
-        final Analyzer analyzer = buildSynonymAnalyzer(tokenizer, charFilters, previousTokenFilters, allFilters);
+        final Analyzer analyzer = buildSynonymAnalyzer(tokenizer, charFilters, previousTokenFilters, allFilters, analyzersBuiltSoFar);
         final SynonymMap synonyms = buildSynonyms(analyzer, getRulesFromSettings(environment));
         final String name = name();
         return new TokenFilterFactory() {

modules/analysis-common/src/main/java/org/opensearch/analysis/common/SynonymTokenFilterFactory.java

@@ -112,9 +112,10 @@ public TokenFilterFactory getChainAwareTokenFilterFactory(
         TokenizerFactory tokenizer,
         List<CharFilterFactory> charFilters,
         List<TokenFilterFactory> previousTokenFilters,
-        Function<String, TokenFilterFactory> allFilters
+        Function<String, TokenFilterFactory> allFilters,
+        Function<String, Analyzer> analyzersBuiltSoFar
     ) {
-        final Analyzer analyzer = buildSynonymAnalyzer(tokenizer, charFilters, previousTokenFilters, allFilters);
+        final Analyzer analyzer = buildSynonymAnalyzer(tokenizer, charFilters, previousTokenFilters, allFilters, analyzersBuiltSoFar);
         final SynonymMap synonyms = buildSynonyms(analyzer, getRulesFromSettings(environment));
         final String name = name();
         return new TokenFilterFactory() {
@@ -147,10 +148,15 @@ Analyzer buildSynonymAnalyzer(
         TokenizerFactory tokenizer,
         List<CharFilterFactory> charFilters,
         List<TokenFilterFactory> tokenFilters,
-        Function<String, TokenFilterFactory> allFilters
+        Function<String, TokenFilterFactory> allFilters,
+        Function<String, Analyzer> analyzersBuiltSoFar
     ) {
         if (synonymAnalyzerName != null) {
-            Analyzer customSynonymAnalyzer;
+            // first, check settings analyzers
+            Analyzer customSynonymAnalyzer = analyzersBuiltSoFar.apply(synonymAnalyzerName);
+            if (customSynonymAnalyzer != null) {
+                return customSynonymAnalyzer;
+            }
             try {
                 customSynonymAnalyzer = analysisRegistry.getAnalyzer(synonymAnalyzerName);
             } catch (IOException e) {

modules/analysis-common/src/test/java/org/opensearch/analysis/common/EdgeNGramTokenizerTests.java

@@ -49,11 +49,9 @@
 
 import java.io.IOException;
 import java.io.StringReader;
+import java.util.Arrays;
 import java.util.Collections;
 
-import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.hasToString;
-
 public class EdgeNGramTokenizerTests extends OpenSearchTokenStreamTestCase {
 
     private IndexAnalyzers buildAnalyzers(Version version, String tokenizer) throws IOException {
@@ -99,7 +97,14 @@ public void testPreConfiguredTokenizer() throws IOException {
                 IllegalArgumentException.class,
                 () -> buildAnalyzers(VersionUtils.randomVersionBetween(random(), Version.V_3_0_0, Version.CURRENT), "edgeNGram")
             );
-            assertThat(e, hasToString(containsString("The [edgeNGram] tokenizer name was deprecated pre 1.0.")));
+
+            boolean found = Arrays.stream(e.getSuppressed())
+                .map(org.opensearch.ExceptionsHelper::unwrapCause)
+                .map(Throwable::getMessage)
+                .findFirst()
+                .get()
+                .contains("The [edgeNGram] tokenizer name was deprecated pre 1.0.");
+            assertTrue("expected deprecation message in suppressed causes", found);
         }
     }
 

modules/analysis-common/src/test/java/org/opensearch/analysis/common/SynonymsAnalysisTests.java

@@ -120,7 +120,7 @@ public void testSynonymWordDeleteByAnalyzer() throws IOException {
             fail("fail! due to synonym word deleted by analyzer");
         } catch (Exception e) {
             assertThat(e, instanceOf(IllegalArgumentException.class));
-            assertThat(e.getMessage(), startsWith("Failed to build synonyms"));
+            assertThat(e.getMessage(), startsWith("Failed to build analyzers: [synonymAnalyzerWithStopSynonymBeforeSynonym]"));
         }
     }
 
@@ -141,7 +141,7 @@ public void testExpandSynonymWordDeleteByAnalyzer() throws IOException {
             fail("fail! due to synonym word deleted by analyzer");
         } catch (Exception e) {
             assertThat(e, instanceOf(IllegalArgumentException.class));
-            assertThat(e.getMessage(), startsWith("Failed to build synonyms"));
+            assertThat(e.getMessage(), startsWith("Failed to build analyzers: [synonymAnalyzerExpandWithStopBeforeSynonym]"));
         }
     }
 
@@ -269,7 +269,7 @@ public void testTokenFiltersBypassSynonymAnalysis() throws IOException {
             TokenFilterFactory tff = plugin.getTokenFilters(analysisModule).get(factory).get(idxSettings, environment, factory, settings);
             TokenizerFactory tok = new KeywordTokenizerFactory(idxSettings, environment, "keyword", settings);
             SynonymTokenFilterFactory stff = new SynonymTokenFilterFactory(idxSettings, environment, "synonym", settings, analysisRegistry);
-            Analyzer analyzer = stff.buildSynonymAnalyzer(tok, Collections.emptyList(), Collections.singletonList(tff), null);
+            Analyzer analyzer = stff.buildSynonymAnalyzer(tok, Collections.emptyList(), Collections.singletonList(tff), null, null);
 
             try (TokenStream ts = analyzer.tokenStream("field", "text")) {
                 assertThat(ts, instanceOf(KeywordTokenizer.class));
@@ -350,7 +350,7 @@ public void testDisallowedTokenFilters() throws IOException {
             IllegalArgumentException e = expectThrows(
                 IllegalArgumentException.class,
                 "Expected IllegalArgumentException for factory " + factory,
-                () -> stff.buildSynonymAnalyzer(tok, Collections.emptyList(), Collections.singletonList(tff), null)
+                () -> stff.buildSynonymAnalyzer(tok, Collections.emptyList(), Collections.singletonList(tff), null, null)
             );
 
             assertEquals(factory, "Token filter [" + factory + "] cannot be used to parse synonyms", e.getMessage());
@@ -443,4 +443,48 @@ public void testSynonymAnalyzerWithWordDelimiter() throws IOException {
             assertTokenStreamContents(ts, new String[] { "notebook" }, new int[] { 0 }, new int[] { 6 });
         }
     }
+
+    /**
+     * Test the core dependency resolution issue from GitHub #18037:
+     * synonym_graph with custom synonym_analyzer should work even when
+     * the main analyzer contains word_delimiter_graph that would normally
+     * cause "cannot be used to parse synonyms" error.
+     */
+    public void testSynonymAnalyzerDependencyResolution() throws IOException {
+        Settings settings = Settings.builder()
+            .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString())
+            .put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
+
+            // Main analyzer with word_delimiter. order=2
+            .put("index.analysis.analyzer.main_analyzer.type", "custom")
+            .put("index.analysis.analyzer.main_analyzer.order", "2")
+            .put("index.analysis.analyzer.main_analyzer.tokenizer", "standard")
+            .putList("index.analysis.analyzer.main_analyzer.filter", "lowercase", "test_word_delimiter", "test_synonyms")
+
+            // Problematic filter for synonym parsing
+            .put("index.analysis.filter.test_word_delimiter.type", "word_delimiter_graph")
+            .put("index.analysis.filter.test_word_delimiter.generate_word_parts", true)
+
+            // Custom analyzer dependency. order=1 (built before main_analyzer whose order=2)
+            .put("index.analysis.analyzer.simple_synonym_analyzer.type", "custom")
+            .put("index.analysis.analyzer.simple_synonym_analyzer.order", "1")
+            .put("index.analysis.analyzer.simple_synonym_analyzer.tokenizer", "standard")
+
+            // Synonym filter that depends on custom analyzer
+            .put("index.analysis.filter.test_synonyms.type", "synonym_graph")
+            .putList("index.analysis.filter.test_synonyms.synonyms", "laptop,notebook")
+            .put("index.analysis.filter.test_synonyms.synonym_analyzer", "simple_synonym_analyzer")
+            .build();
+
+        IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("test_index", settings);
+
+        // Should succeed with the fix (would fail before due to registration order)
+        IndexAnalyzers analyzers = new AnalysisModule(
+            TestEnvironment.newEnvironment(settings),
+            Collections.singletonList(new CommonAnalysisModulePlugin())
+        ).getAnalysisRegistry().build(idxSettings);
+
+        assertNotNull("main_analyzer should be created", analyzers.get("main_analyzer"));
+        assertNotNull("simple_synonym_analyzer should be created", analyzers.get("simple_synonym_analyzer"));
+    }
 }

server/src/main/java/org/opensearch/index/analysis/AnalysisRegistry.java

@@ -54,6 +54,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -305,7 +306,7 @@ public NamedAnalyzer buildCustomAnalyzer(
                 } catch (IOException e) {
                     throw new UncheckedIOException(e);
                 }
-            });
+            }, null);
             tokenFilterFactories.add(tff);
         }
 
@@ -362,7 +363,30 @@ public Map<String, CharFilterFactory> buildCharFilterFactories(IndexSettings ind
 
     private Map<String, AnalyzerProvider<?>> buildAnalyzerFactories(IndexSettings indexSettings) throws IOException {
         final Map<String, Settings> analyzersSettings = indexSettings.getSettings().getGroups("index.analysis.analyzer");
-        return buildMapping(Component.ANALYZER, indexSettings, analyzersSettings, analyzers, prebuiltAnalysis.analyzerProviderFactories);
+
+        // Some analyzers depend on others that need to be built first
+        // Sort by 'order', default to 1000
+        List<Map.Entry<String, Settings>> sortedEntries = analyzersSettings.entrySet().stream().sorted((a, b) -> {
+            int orderA = a.getValue().getAsInt("order", 100);
+            int orderB = b.getValue().getAsInt("order", 100);
+            if (orderA != orderB) {
+                return Integer.compare(orderA, orderB);
+            }
+            return a.getKey().compareTo(b.getKey());
+        }).collect(Collectors.toList());
+
+        Map<String, Settings> sortedAnalyzersSettings = new LinkedHashMap<>();
+        for (Map.Entry<String, Settings> entry : sortedEntries) {
+            sortedAnalyzersSettings.put(entry.getKey(), entry.getValue());
+        }
+
+        return buildMapping(
+            Component.ANALYZER,
+            indexSettings,
+            sortedAnalyzersSettings,
+            analyzers,
+            prebuiltAnalysis.analyzerProviderFactories
+        );
     }
 
     private Map<String, AnalyzerProvider<?>> buildNormalizerFactories(IndexSettings indexSettings) throws IOException {
@@ -486,7 +510,7 @@ private <T> Map<String, T> buildMapping(
         Map<String, ? extends AnalysisModule.AnalysisProvider<T>> defaultInstance
     ) throws IOException {
         Settings defaultSettings = Settings.builder().put(IndexMetadata.SETTING_VERSION_CREATED, settings.getIndexVersionCreated()).build();
-        Map<String, T> factories = new HashMap<>();
+        Map<String, T> factories = new LinkedHashMap<>(); // keep insertion order
         for (Map.Entry<String, Settings> entry : settingsMap.entrySet()) {
             String name = entry.getKey();
             Settings currentSettings = entry.getValue();
@@ -637,21 +661,27 @@ public IndexAnalyzers build(
         Map<String, NamedAnalyzer> analyzers = new HashMap<>();
         Map<String, NamedAnalyzer> normalizers = new HashMap<>();
         Map<String, NamedAnalyzer> whitespaceNormalizers = new HashMap<>();
+        Map<String, Exception> buildErrors = new LinkedHashMap<>();
+        Map<String, Analyzer> analyzersBuiltSoFar = new HashMap<>();
         for (Map.Entry<String, AnalyzerProvider<?>> entry : analyzerProviders.entrySet()) {
-            analyzers.merge(
-                entry.getKey(),
-                produceAnalyzer(
+            try {
+                NamedAnalyzer namedAnalyzer = produceAnalyzer(
                     entry.getKey(),
                     entry.getValue(),
                     tokenFilterFactoryFactories,
                     charFilterFactoryFactories,
-                    tokenizerFactoryFactories
-                ),
-                (k, v) -> {
+                    tokenizerFactoryFactories,
+                    analyzersBuiltSoFar
+                );
+                analyzers.merge(entry.getKey(), namedAnalyzer, (k, v) -> {
                     throw new IllegalStateException("already registered analyzer with name: " + entry.getKey());
-                }
-            );
+                });
+                analyzersBuiltSoFar.put(entry.getKey(), namedAnalyzer);
+            } catch (Exception e) {
+                buildErrors.put(entry.getKey(), e);
+            }
         }
+
         for (Map.Entry<String, AnalyzerProvider<?>> entry : normalizerProviders.entrySet()) {
             processNormalizerFactory(
                 entry.getKey(),
@@ -707,6 +737,14 @@ public IndexAnalyzers build(
                 throw new IllegalArgumentException("analyzer name must not start with '_'. got \"" + analyzer.getKey() + "\"");
             }
         }
+
+        if (!buildErrors.isEmpty()) {
+            IllegalArgumentException aggregated = new IllegalArgumentException("Failed to build analyzers: " + buildErrors.keySet());
+            buildErrors.forEach(
+                (name, ex) -> aggregated.addSuppressed(new IllegalArgumentException("[" + name + "] " + ex.getMessage(), ex))
+            );
+            throw aggregated;
+        }
         return new IndexAnalyzers(analyzers, normalizers, whitespaceNormalizers);
     }
 
@@ -716,6 +754,17 @@ private static NamedAnalyzer produceAnalyzer(
         Map<String, TokenFilterFactory> tokenFilters,
         Map<String, CharFilterFactory> charFilters,
         Map<String, TokenizerFactory> tokenizers
+    ) {
+        return produceAnalyzer(name, analyzerFactory, tokenFilters, charFilters, tokenizers, Collections.emptyMap());
+    }
+
+    private static NamedAnalyzer produceAnalyzer(
+        String name,
+        AnalyzerProvider<?> analyzerFactory,
+        Map<String, TokenFilterFactory> tokenFilters,
+        Map<String, CharFilterFactory> charFilters,
+        Map<String, TokenizerFactory> tokenizers,
+        Map<String, Analyzer> analyzersBuiltSoFar
     ) {
         /*
          * Lucene defaults positionIncrementGap to 0 in all analyzers but
@@ -725,7 +774,7 @@ private static NamedAnalyzer produceAnalyzer(
          */
         int overridePositionIncrementGap = TextFieldMapper.Defaults.POSITION_INCREMENT_GAP;
         if (analyzerFactory instanceof CustomAnalyzerProvider) {
-            ((CustomAnalyzerProvider) analyzerFactory).build(tokenizers, charFilters, tokenFilters);
+            ((CustomAnalyzerProvider) analyzerFactory).build(tokenizers, charFilters, tokenFilters, analyzersBuiltSoFar);
             /*
              * Custom analyzers already default to the correct, version
              * dependent positionIncrementGap and the user is be able to