OAK-12353: Group Elasticsearch dynamic boost nested documents by boost score

oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexProviderService.java

@@ -22,6 +22,7 @@
 import org.apache.jackrabbit.oak.plugins.index.AsyncIndexInfoService;
 import org.apache.jackrabbit.oak.plugins.index.IndexEditorProvider;
 import org.apache.jackrabbit.oak.plugins.index.IndexInfoProvider;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticDocument;
 import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticIndexEditorProvider;
 import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticRetryPolicy;
 import org.apache.jackrabbit.oak.plugins.index.elastic.query.ElasticIndexProvider;
@@ -236,6 +237,9 @@ private void activate(BundleContext bundleContext, Config config) {
         oakRegs.add(whiteboard.register(FeatureToggle.class,
                 new FeatureToggle(ElasticIndexStatistics.FT_OAK_12248, ElasticIndexStatistics.FT_OAK_12248_ENABLE),
                 emptyMap()));
+        oakRegs.add(whiteboard.register(FeatureToggle.class,
+                new FeatureToggle(ElasticDocument.FT_OAK_12353, ElasticDocument.FT_OAK_12353_ENABLE),
+                emptyMap()));
         if (System.getProperty(QueryEngineSettings.OAK_INFERENCE_ENABLED) != null) {
             this.isInferenceEnabled = Boolean.parseBoolean(System.getProperty(QueryEngineSettings.OAK_INFERENCE_ENABLED));
         } else {

oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticDocument.java

@@ -35,13 +35,27 @@
 import java.util.Map;
 import java.util.HashMap;
 import java.util.Set;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
 
 import static org.apache.jackrabbit.oak.plugins.index.elastic.util.ElasticIndexUtils.toFloats;
 
 @JsonInclude(JsonInclude.Include.NON_EMPTY)
 public class ElasticDocument {
 
+    public static final String FT_OAK_12353 = "FT_OAK-12353";
+    /**
+     * When {@code true}, dynamic boost values sharing the same boost score are grouped into a
+     * single nested document per property, with {@code value} holding an array of the grouped
+     * values, instead of one nested document per value. This reduces the number of nested
+     * documents generated for properties with many dynamic-boost values that share a boost score.
+     * Default is {@code true} (feature enabled); set to {@code false} via the feature toggle to
+     * revert to the pre-fix behaviour of one nested document per value.
+     */
+    public static final AtomicBoolean FT_OAK_12353_ENABLE = new AtomicBoolean(true);
+
     @JsonProperty(FieldNames.PATH)
     public final String path;
     @JsonProperty(ElasticIndexDefinition.PATH_RANDOM_VALUE)
@@ -66,6 +80,12 @@ public class ElasticDocument {
     @JsonProperty(ElasticIndexDefinition.LAST_UPDATED)
     private long lastUpdated;
 
+    // fieldName -> boost -> values sharing that boost. Only populated when FT_OAK_12353_ENABLE is
+    // true, in which case it replaces the corresponding entries that would otherwise be added to
+    // "properties" directly by addDynamicBoostField.
+    @JsonIgnore
+    private final Map<String, LinkedHashMap<Double, LinkedHashSet<String>>> dynamicBoostGroups;
+
     // Internal set with properties that need to be removed from the document on update operations
     @JsonIgnore
     private final Set<String> propertiesToRemove;
@@ -87,6 +107,7 @@ public class ElasticDocument {
         this.dbFullText = new LinkedHashSet<>();
         this.similarityTags = new LinkedHashSet<>();
         this.propertiesToRemove = new HashSet<>();
+        this.dynamicBoostGroups = new LinkedHashMap<>();
     }
 
     void addFulltext(String value) {
@@ -175,12 +196,18 @@ void indexAncestors(String path) {
     }
 
     void addDynamicBoostField(String fieldName, String value, double boost) {
-        addProperty(fieldName,
-                Map.of(
-                        ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE, value,
-                        ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST, boost
-                )
-        );
+        if (FT_OAK_12353_ENABLE.get()) {
+            dynamicBoostGroups.computeIfAbsent(fieldName, k -> new LinkedHashMap<>())
+                    .computeIfAbsent(boost, k -> new LinkedHashSet<>())
+                    .add(value);
+        } else {
+            addProperty(fieldName,
+                    Map.of(
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE, value,
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST, boost
+                    )
+            );
+        }
 
         // add value into the dynamic boost specific fulltext field. We cannot add this in the standard
         // field since dynamic boosted terms require lower weight compared to standard terms
@@ -197,7 +224,21 @@ void setLastUpdated(long lastUpdated) {
 
     @JsonAnyGetter
     public Map<String, Object> getProperties() {
-        return properties;
+        if (dynamicBoostGroups.isEmpty()) {
+            return properties;
+        }
+        Map<String, Object> merged = new LinkedHashMap<>(properties);
+        dynamicBoostGroups.forEach((fieldName, boostToValues) -> {
+            Set<Object> nestedDocs = boostToValues.entrySet().stream()
+                    .map(entry -> Map.of(
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE,
+                            entry.getValue().size() == 1 ? entry.getValue().iterator().next() : entry.getValue(),
+                            ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST, entry.getKey()
+                    ))
+                    .collect(Collectors.toCollection(LinkedHashSet::new));
+            merged.put(fieldName, nestedDocs);
+        });
+        return merged;
     }
 
     public void removeProperty(String fieldName) {
@@ -222,6 +263,9 @@ public String toString() {
         if (!dynamicProperties.isEmpty()) {
             buff.append("dynamicProperties:").append(dynamicProperties).append('\n');
         }
+        if (!dynamicBoostGroups.isEmpty()) {
+            buff.append("dynamicBoostGroups:").append(dynamicBoostGroups).append('\n');
+        }
         return buff.toString();
     }
 

oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexHelper.java

@@ -393,7 +393,7 @@ private static void checkIndexRules(ElasticIndexDefinition indexDefinition) {
                 .stream()
                 .filter(e -> e.getValue().size() > 1)
                 .filter(e -> e.getValue().stream().map(PropertyDefinition::getType).distinct().count() > 1)
-                .collect(Collectors.toList());
+                .toList();
 
         if (!multiTypesFields.isEmpty()) {
             String fields = multiTypesFields.stream().map(Map.Entry::getKey).collect(Collectors.joining(", ", "[", "]"));
@@ -417,7 +417,7 @@ protected static String convertUpperCamelToLowerUnderscore(String string) {
         StringBuilder result = new StringBuilder();
         for (char c : string.toCharArray()) {
             // start?
-            if (result.length() == 0) {
+            if (result.isEmpty()) {
                 result.append(Character.toLowerCase(c));
             } else {
                 if (Character.isUpperCase(c)) {

oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticDynamicBoostTest.java

@@ -20,20 +20,48 @@
 import org.apache.jackrabbit.oak.api.ContentRepository;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.plugins.index.DynamicBoostCommonTest;
+import org.apache.jackrabbit.oak.plugins.index.elastic.index.ElasticDocument;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 
 import java.util.List;
 
+/**
+ * Runs every test twice, once with {@link ElasticDocument#FT_OAK_12353_ENABLE} enabled and once
+ * with it disabled, so both dynamic-boost grouping code paths get the same coverage.
+ */
+@RunWith(Parameterized.class)
 public class ElasticDynamicBoostTest extends DynamicBoostCommonTest {
 
     @ClassRule
     public static final ElasticConnectionRule elasticRule = new ElasticConnectionRule();
 
+    @Parameterized.Parameters(name = "dynamicBoostGroupingEnabled={0}")
+    public static Iterable<Object[]> fixtures() {
+        return List.of(new Object[]{true}, new Object[]{false});
+    }
+
+    @Parameterized.Parameter
+    public boolean dynamicBoostGroupingEnabled;
+
     public ElasticDynamicBoostTest() {
         this.indexOptions = new ElasticIndexOptions();
     }
 
+    @Before
+    public void setDynamicBoostGroupingToggle() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(dynamicBoostGroupingEnabled);
+    }
+
+    @After
+    public void resetDynamicBoostGroupingToggle() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(true);
+    }
+
     @Override
     protected ContentRepository createRepository() {
         repositoryOptionsUtil = new ElasticTestRepositoryBuilder(elasticRule).build();
@@ -85,6 +113,34 @@ public void dynamicBoostAnalyzed() throws Exception {
         });
     }
 
+    /**
+     * Predicted tags sharing the same boost score are grouped into a single nested document
+     * (see {@link ElasticDocument#FT_OAK_12353_ENABLE}). This verifies that querying still
+     * matches on any of the grouped values, both with the grouping enabled and disabled
+     * (see {@link #dynamicBoostGroupingEnabled}).
+     */
+    @Test
+    public void dynamicBoostQueriesGroupedValuesSharingSameBoostScore() throws Exception {
+        createAssetsIndexAndProperties(false, false);
+
+        Tree testParent = createNodeWithType(root.getTree("/"), "test", JcrConstants.NT_UNSTRUCTURED, "");
+
+        Tree predicted1 = createAssetNodeWithPredicted(testParent, "asset1", "flower with a lot of red and a bit of blue");
+        createPredictedTag(predicted1, "red", 5.0);
+        createPredictedTag(predicted1, "blue", 5.0);
+        createPredictedTag(predicted1, "green", 5.0);
+        createPredictedTag(predicted1, "special", 9.0);
+
+        root.commit();
+
+        assertEventually(() -> {
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'red')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'blue')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'green')]", XPATH, List.of("/test/asset1"));
+            assertQuery("//element(*, dam:Asset)[jcr:contains(., 'special')]", XPATH, List.of("/test/asset1"));
+        });
+    }
+
     @Test
     public void dynamicBoostNotIncludedInFullText() throws Exception {
         createAssetsIndexAndProperties(false, false, false);
@@ -119,4 +175,27 @@ public void dynamicBoostNotIncludedInFullText() throws Exception {
         });
 
     }
+
+    @Test
+    public void ranking() throws Exception {
+        createAssetsIndexAndProperties(false, false);
+        Tree test = createNodeWithType(root.getTree("/"), "test", JcrConstants.NT_UNSTRUCTURED, "");
+
+        // asset1: three tags sharing one boost group (boost 1)
+        Tree many = createAssetNodeWithPredicted(test, "asset1", "titleone");
+        createPredictedTag(many, "red", 1.0);
+        createPredictedTag(many, "blue", 1.0);
+        createPredictedTag(many, "green", 1.0);
+
+        // asset2: one high-boost tag in its own group, the other two effectively zero
+        Tree single = createAssetNodeWithPredicted(test, "asset2", "titletwo");
+        createPredictedTag(single, "red", 4.0);
+        createPredictedTag(single, "blue", 0.01);
+        createPredictedTag(single, "green", 0.01);
+
+        root.commit();
+        String query =
+                "select [jcr:path] from [dam:Asset] where contains(*, 'red blue green')";
+        assertEventually(() -> assertOrderedQuery(query, List.of("/test/asset2", "/test/asset1")));
+    }
 }

oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticDocumentTest.java

@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.index.elastic.index;
+
+import org.junit.After;
+import org.junit.Test;
+
+import java.time.LocalDate;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class ElasticDocumentTest {
+
+    @After
+    public void resetToggle() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(true);
+    }
+
+    @Test
+    public void dynamicBoostValuesAreNotGroupedWhenToggleDisabled() {
+        ElasticDocument.FT_OAK_12353_ENABLE.set(false);
+
+        ElasticDocument doc = new ElasticDocument("/test");
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Replacement Cost", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Theft", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "GENERAL INSURANCE COMPANY", 0.988);
+
+        Object value = doc.getProperties().get("predictedTagsDynamicBoost");
+        assertTrue(value instanceof Set);
+        @SuppressWarnings("unchecked")
+        Set<Map<String, Object>> nestedDocs = (Set<Map<String, Object>>) value;
+        assertEquals(3, nestedDocs.size());
+        for (Map<String, Object> nestedDoc : nestedDocs) {
+            assertTrue(nestedDoc.get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE) instanceof String);
+        }
+    }
+
+    @Test
+    public void dynamicBoostValuesAreGroupedByBoostByDefault() {
+        ElasticDocument doc = new ElasticDocument("/test");
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Replacement Cost", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Theft", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "Alberta", 1.0);
+        doc.addDynamicBoostField("predictedTagsDynamicBoost", "GENERAL INSURANCE COMPANY", 0.988);
+
+        Object value = doc.getProperties().get("predictedTagsDynamicBoost");
+        assertTrue(value instanceof Set);
+        @SuppressWarnings("unchecked")
+        Set<Map<String, Object>> nestedDocs = (Set<Map<String, Object>>) value;
+        // one nested doc for the 3 values sharing boost=1.0, one for the distinct boost=0.988
+        assertEquals(2, nestedDocs.size());
+
+        boolean foundGrouped = false;
+        boolean foundSingle = false;
+        for (Map<String, Object> nestedDoc : nestedDocs) {
+            Object boost = nestedDoc.get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_BOOST);
+            Object nestedValue = nestedDoc.get(ElasticIndexHelper.DYNAMIC_BOOST_NESTED_VALUE);
+            if (Double.valueOf(1.0).equals(boost)) {
+                assertTrue(nestedValue instanceof Collection);
+                @SuppressWarnings("unchecked")
+                Collection<String> values = (Collection<String>) nestedValue;
+                assertEquals(List.of("Replacement Cost", "Theft", "Alberta"), new ArrayList<>(values));
+                foundGrouped = true;
+            } else if (Double.valueOf(0.988).equals(boost)) {
+                assertEquals("GENERAL INSURANCE COMPANY", nestedValue);
+                foundSingle = true;
+            }
+        }
+        assertTrue(foundGrouped);
+        assertTrue(foundSingle);
+    }
+
+    @Test
+    public void ft_oak_12353_toggleShouldBeRemoved() {
+        // Time-bombed: if this test fails, the feature toggle FT_OAK-12353 and its guard in
+        // ElasticDocument#addDynamicBoostField/#getProperties should be removed — the grouping
+        // has been enabled by default in production long enough.
+        assertTrue("Feature toggle " + ElasticDocument.FT_OAK_12353 + " is overdue for removal",
+                LocalDate.now().isBefore(LocalDate.of(2027, 8, 12)));
+    }
+}

oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/FulltextIndexConstants.java

@@ -485,6 +485,6 @@ public static IndexingMode from(String indexingMode) {
      * needed from an outside process that does not have visibility to the specific index module.
      */
     Map<String, String> INDEX_VERSION_BY_TYPE = Map.of(
-            "elasticsearch", "1.4.0"
+            "elasticsearch", "1.5.0"
     );
 }