Only call doCache one per request

CHANGELOG.md

@@ -16,6 +16,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 
 ### Bug Fixes
 - Fix the issue of unprocessed X-Request-Id ([#5954](https://github.com/opensearch-project/security/pull/5954))
+- Skip logic in doCache if index request cache is disabled ([#6001](https://github.com/opensearch-project/security/pull/6001))
+
 ### Refactoring
 
 ### Maintenance

src/integrationTest/java/org/opensearch/security/dlsfls/DoCacheMemoizationTest.java

@@ -0,0 +1,107 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+package org.opensearch.security.dlsfls;
+
+import java.util.Map;
+import java.util.stream.IntStream;
+
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.opensearch.action.index.IndexRequest;
+import org.opensearch.action.search.SearchRequest;
+import org.opensearch.action.search.SearchResponse;
+import org.opensearch.client.RestHighLevelClient;
+import org.opensearch.index.query.QueryBuilders;
+import org.opensearch.search.builder.SearchSourceBuilder;
+import org.opensearch.test.framework.TestSecurityConfig;
+import org.opensearch.test.framework.cluster.ClusterManager;
+import org.opensearch.test.framework.cluster.LocalCluster;
+import org.opensearch.test.framework.log.LogCapturingAppender;
+import org.opensearch.test.framework.log.LogsRule;
+import org.opensearch.transport.client.Client;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.lessThan;
+import static org.opensearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE;
+import static org.opensearch.client.RequestOptions.DEFAULT;
+import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
+
+/**
+ * Regression test verifying that hasFlsOrFieldMasking() is memoized per request in doCache().
+ *
+ * Without the fix, doCache() calls hasFlsOrFieldMasking() once per BooleanQuery filter clause
+ * per shard. With the fix, the result is cached in the ThreadContext for the duration of the
+ * request so only 1 call is made per shard per request, and subsequent calls hit the memoized path.
+ */
+public class DoCacheMemoizationTest {
+
+    static final String INDEX_NAME = "test-index";
+    static final int NUM_FILTER_CLAUSES = 50;
+
+    // Field masking causes hasFlsOrFieldMasking() to return true, exercising the memoization path.
+    static final TestSecurityConfig.User MASKED_USER = new TestSecurityConfig.User("masked_user").roles(
+        new TestSecurityConfig.Role("masked_role").clusterPermissions("cluster_composite_ops_ro")
+            .indexPermissions("read")
+            .maskedFields("field_a")
+            .on(INDEX_NAME)
+    );
+
+    @ClassRule
+    public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
+        .anonymousAuth(false)
+        .authc(AUTHC_HTTPBASIC_INTERNAL)
+        .users(MASKED_USER)
+        .build();
+
+    @Rule
+    public LogsRule logsRule = new LogsRule("org.opensearch.security.OpenSearchSecurityPlugin");
+
+    @BeforeClass
+    public static void setupIndex() {
+        try (Client client = cluster.getInternalNodeClient()) {
+            client.index(new IndexRequest(INDEX_NAME).setRefreshPolicy(IMMEDIATE).source(Map.of("field_a", "value", "field_b", "other")))
+                .actionGet();
+        }
+    }
+
+    /**
+     * filter clauses → scoreMode=COMPLETE_NO_SCORES → doCache() called per clause per shard.
+     * should/must clauses would set needsScores=true and bypass doCache() entirely.
+     */
+    private SearchRequest buildFilterQuery() {
+        var bool = QueryBuilders.boolQuery();
+        IntStream.range(0, NUM_FILTER_CLAUSES).forEach(i -> bool.filter(QueryBuilders.termQuery("field_b", "other_" + i)));
+        return new SearchRequest(INDEX_NAME).source(new SearchSourceBuilder().query(bool).size(10));
+    }
+
+    @Test
+    public void doCacheUsedMemoizedValueForSubsequentClauses() throws Exception {
+        try (RestHighLevelClient client = cluster.getRestHighLevelClient(MASKED_USER)) {
+            SearchResponse response = client.search(buildFilterQuery(), DEFAULT);
+            assertThat("search must succeed", response.getFailedShards(), lessThan(1));
+        }
+
+        // First clause evaluates fresh and populates the ThreadContext transient
+        logsRule.assertThatContain("doCache: evaluated hasFlsOrFieldMasking(test-index)=true");
+        // Subsequent clauses (49 of them) hit the memoized value instead of re-evaluating
+        logsRule.assertThatContain("doCache: memoized hasFlsOrFieldMasking(test-index)=true");
+        // Exactly 1 fresh evaluation and NUM_FILTER_CLAUSES-1 memoized hits
+        long evaluatedCount = LogCapturingAppender.getLogMessagesAsString()
+            .stream()
+            .filter(m -> m.contains("doCache: evaluated hasFlsOrFieldMasking"))
+            .count();
+        assertThat("expected exactly 1 fresh evaluation per request", evaluatedCount, equalTo(1L));
+    }
+}

src/integrationTest/resources/log4j2-test.properties

@@ -53,3 +53,8 @@ logger.securenetty4transport.name = org.opensearch.transport.netty4.ssl.SecureNe
 logger.securenetty4transport.level = error
 logger.securenetty4transport.appenderRef.capturing.ref = logCapturingAppender
 
+# Logger required by test org.opensearch.security.dlsfls.DoCacheMemoizationTest
+logger.opensearchsecurityplugin.name = org.opensearch.security.OpenSearchSecurityPlugin
+logger.opensearchsecurityplugin.level = debug
+logger.opensearchsecurityplugin.appenderRef.capturing.ref = logCapturingAppender
+

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

@@ -794,6 +794,9 @@ public void onIndexModule(IndexModule indexModule) {
             }
 
             indexModule.forceQueryCacheProvider((indexSettings, nodeCache) -> new QueryCache() {
+                // Transient key used to cache the hasFlsOrFieldMasking result for the duration of a single request,
+                // avoiding repeated alias traversal for every sub-clause of a BooleanQuery.
+                private final String cacheKey = "_opendistro_security_flsfm_" + indexSettings.getIndex().getName();
 
                 @Override
                 public Index index() {
@@ -813,7 +816,15 @@ public void clear(String reason) {
                 @Override
                 public Weight doCache(Weight weight, QueryCachingPolicy policy) {
                     try {
-                        if (dlsFlsValve.hasFlsOrFieldMasking(index().getName())) {
+                        Boolean cached = threadPool.getThreadContext().getTransient(cacheKey);
+                        if (cached == null) {
+                            cached = dlsFlsValve.hasFlsOrFieldMasking(index().getName());
+                            threadPool.getThreadContext().putTransient(cacheKey, cached);
+                            log.debug("doCache: evaluated hasFlsOrFieldMasking({})={}", index().getName(), cached);
+                        } else {
+                            log.debug("doCache: memoized hasFlsOrFieldMasking({})={}", index().getName(), cached);
+                        }
+                        if (cached) {
                             // Do not cache
                             return weight;
                         } else {