Remove singleton pattern breakage and mark as experimental

Signed-off-by: Atri Sharma <atri.jiit@gmail.com>

Author: atris

server/src/main/java/org/opensearch/search/SearchService.java

@@ -532,7 +532,7 @@ public SearchService(
 
         // Initialize QueryRewriterRegistry with cluster settings so TermsMergingRewriter
         // can register its settings update consumer
-        QueryRewriterRegistry.initialize(settings, clusterService.getClusterSettings());
+        QueryRewriterRegistry.INSTANCE.initialize(settings, clusterService.getClusterSettings());
     }
 
     private void validateKeepAlives(TimeValue defaultKeepAlive, TimeValue maxKeepAlive) {
@@ -1517,7 +1517,7 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
             QueryBuilder query = source.query();
 
             // Apply query rewriting optimizations
-            query = QueryRewriterRegistry.rewrite(query, queryShardContext);
+            query = QueryRewriterRegistry.INSTANCE.rewrite(query, queryShardContext);
 
             InnerHitContextBuilder.extractInnerHits(query, innerHitBuilders);
             context.parsedQuery(queryShardContext.toQuery(query));

server/src/main/java/org/opensearch/search/query/QueryRewriter.java

@@ -8,15 +8,17 @@
 
 package org.opensearch.search.query;
 
+import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.index.query.QueryBuilder;
 import org.opensearch.index.query.QueryShardContext;
 
 /**
  * Interface for query rewriting implementations that optimize query structure
  * before conversion to Lucene queries.
  *
- * @opensearch.internal
+ * @opensearch.experimental
  */
+@ExperimentalApi
 public interface QueryRewriter {
 
     /**

server/src/main/java/org/opensearch/search/query/QueryRewriterRegistry.java

@@ -35,7 +35,7 @@ public final class QueryRewriterRegistry {
 
     private static final Logger logger = LogManager.getLogger(QueryRewriterRegistry.class);
 
-    private static final QueryRewriterRegistry INSTANCE = new QueryRewriterRegistry();
+    public static final QueryRewriterRegistry INSTANCE = new QueryRewriterRegistry();
 
     /**
      * Default rewriters.
@@ -59,13 +59,6 @@ private QueryRewriterRegistry() {
         registerRewriter(TermsMergingRewriter.INSTANCE);
     }
 
-    /**
-     * Get the singleton instance of the registry.
-     */
-    public static QueryRewriterRegistry getInstance() {
-        return INSTANCE;
-    }
-
     /**
      * Register a custom query rewriter.
      *
@@ -86,23 +79,21 @@ public void registerRewriter(QueryRewriter rewriter) {
      * @param settings Initial cluster settings
      * @param clusterSettings Cluster settings for registering update consumers
      */
-    public static void initialize(Settings settings, ClusterSettings clusterSettings) {
-        QueryRewriterRegistry instance = getInstance();
+    public void initialize(Settings settings, ClusterSettings clusterSettings) {
         TermsMergingRewriter.INSTANCE.initialize(settings, clusterSettings);
-        instance.enabled = SearchService.QUERY_REWRITING_ENABLED_SETTING.get(settings);
+        this.enabled = SearchService.QUERY_REWRITING_ENABLED_SETTING.get(settings);
         clusterSettings.addSettingsUpdateConsumer(
             SearchService.QUERY_REWRITING_ENABLED_SETTING,
-            (Boolean enabled) -> instance.enabled = enabled
+            (Boolean enabled) -> this.enabled = enabled
         );
     }
 
-    public static QueryBuilder rewrite(QueryBuilder query, QueryShardContext context) {
-        QueryRewriterRegistry registry = getInstance();
-        if (!registry.enabled || query == null) {
+    public QueryBuilder rewrite(QueryBuilder query, QueryShardContext context) {
+        if (!enabled || query == null) {
             return query;
         }
 
-        List<QueryRewriter> sortedRewriters = new ArrayList<>(registry.rewriters);
+        List<QueryRewriter> sortedRewriters = new ArrayList<>(rewriters);
         sortedRewriters.sort(Comparator.comparingInt(QueryRewriter::priority));
 
         QueryBuilder current = query;

server/src/test/java/org/opensearch/search/query/QueryRewriterRegistryTests.java

@@ -37,7 +37,7 @@ public void setUp() throws Exception {
             .put(SearchService.QUERY_REWRITING_TERMS_THRESHOLD_SETTING.getKey(), 16)
             .build();
         ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
-        QueryRewriterRegistry.initialize(settings, clusterSettings);
+        QueryRewriterRegistry.INSTANCE.initialize(settings, clusterSettings);
     }
 
     public void testCompleteRewritingPipeline() {
@@ -53,7 +53,7 @@ public void testCompleteRewritingPipeline() {
             .filter(QueryBuilders.termQuery("type", "product"))
             .filter(QueryBuilders.termQuery("type", "service"));
 
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertThat(rewritten, instanceOf(BoolQueryBuilder.class));
         BoolQueryBuilder rewrittenBool = (BoolQueryBuilder) rewritten;
 
@@ -76,26 +76,26 @@ public void testDisabledRewriting() {
         ClusterSettings clusterSettings = new ClusterSettings(settings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
 
         // Initialize with disabled setting
-        QueryRewriterRegistry.initialize(settings, clusterSettings);
+        QueryRewriterRegistry.INSTANCE.initialize(settings, clusterSettings);
 
         QueryBuilder query = QueryBuilders.boolQuery()
             .must(QueryBuilders.matchAllQuery())
             .filter(QueryBuilders.termQuery("field", "value"));
 
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertSame(query, rewritten);
 
         // Enable via settings update
         clusterSettings.applySettings(Settings.builder().put(SearchService.QUERY_REWRITING_ENABLED_SETTING.getKey(), true).build());
 
         // Now it should rewrite
-        QueryBuilder rewritten2 = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten2 = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertNotSame(query, rewritten2);
     }
 
     public void testNullQuery() {
         // Null query should return null
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(null, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(null, context);
         assertNull(rewritten);
     }
 
@@ -110,7 +110,7 @@ public void testRewriterPriorityOrder() {
                     .must(QueryBuilders.termQuery("field", "value2"))
             );
 
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(deeplyNested, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(deeplyNested, context);
         assertThat(rewritten, instanceOf(BoolQueryBuilder.class));
         BoolQueryBuilder rewrittenBool = (BoolQueryBuilder) rewritten;
 
@@ -141,7 +141,7 @@ public void testComplexRealWorldQuery() {
             .must(QueryBuilders.rangeQuery("price").gte(500).lte(2000))
             .mustNot(QueryBuilders.termQuery("status", "discontinued"));
 
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertThat(rewritten, instanceOf(BoolQueryBuilder.class));
         BoolQueryBuilder rewrittenBool = (BoolQueryBuilder) rewritten;
 
@@ -183,7 +183,7 @@ public void testPerformanceMetrics() {
             .filter(QueryBuilders.matchAllQuery());
 
         // Should not throw any exceptions
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertNotNull(rewritten);
     }
 
@@ -195,7 +195,7 @@ public void testRewriterErrorHandling() {
             .filter(QueryBuilders.matchAllQuery());
 
         // Even if one rewriter fails, others should still be applied
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertNotNull(rewritten);
     }
 
@@ -230,7 +230,7 @@ public void testVeryComplexMixedQuery() {
             .mustNot(QueryBuilders.boolQuery().must(QueryBuilders.termQuery("archived", "true")))
             .minimumShouldMatch(1);
 
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertThat(rewritten, instanceOf(BoolQueryBuilder.class));
         BoolQueryBuilder result = (BoolQueryBuilder) rewritten;
 
@@ -307,14 +307,14 @@ public String name() {
         };
 
         // Register the custom rewriter
-        QueryRewriterRegistry.getInstance().registerRewriter(customRewriter);
+        QueryRewriterRegistry.INSTANCE.registerRewriter(customRewriter);
 
         // Test that it's applied
         QueryBuilder query = QueryBuilders.boolQuery()
             .must(QueryBuilders.termQuery("test_field", "test_value"))
             .filter(QueryBuilders.termQuery("other_field", "other_value"));
 
-        QueryBuilder rewritten = QueryRewriterRegistry.rewrite(query, context);
+        QueryBuilder rewritten = QueryRewriterRegistry.INSTANCE.rewrite(query, context);
         assertThat(rewritten, instanceOf(BoolQueryBuilder.class));
         BoolQueryBuilder rewrittenBool = (BoolQueryBuilder) rewritten;