- Test that deprecation warning is given if specified inside match-phase {}.

- Test that exception is thrown if specified twice.

2 files changed, 51 insertions, 18 deletions

diff --git a/config-model/src/main/javacc/SchemaParser.jj b/config-model/src/main/javacc/SchemaParser.jj
index a386b63cb58..c9eff88764f 100644
--- a/config-model/src/main/javacc/SchemaParser.jj
+++ b/config-model/src/main/javacc/SchemaParser.jj
@@ -1892,7 +1892,7 @@ void matchPhaseItem(ParsedRankProfile profile, MatchPhaseSettings settings) :
 }
 {
     ( <ATTRIBUTE> <COLON> str = identifier() { settings.setAttribute(str); }
-      | diversity(profile)
+      | diversityDeprecated(profile)
       | <ORDER> <COLON> ( <ASCENDING> { settings.setAscending(true); } 
                           | <DESCENDING> { settings.setAscending(false); } )
       | <MAX_HITS> <COLON> num = integer() { settings.setMaxHits(num); }
@@ -1918,6 +1918,18 @@ void diversity(ParsedRankProfile profile) :
     }
 }
 
+void diversityDeprecated(ParsedRankProfile profile) :
+{
+    DiversitySettings settings = new DiversitySettings();
+}
+{
+    <DIVERSITY> lbrace() (diversityItem(settings) (<NL>)*)* <RBRACE>
+    {
+ 	    profile.setDiversity(settings);
+        deployLogger.logApplicationPackage(Level.WARNING, "'diversity is deprecated inside 'match-phase'. Specify it at 'rank-profile' level.");
+    }
+}
+
 void diversityItem(DiversitySettings settings) :
 {
     String str;
diff --git a/config-model/src/test/java/com/yahoo/schema/DiversityTestCase.java b/config-model/src/test/java/com/yahoo/schema/DiversityTestCase.java
index 2c03bab8144..4026341464f 100644
--- a/config-model/src/test/java/com/yahoo/schema/DiversityTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/DiversityTestCase.java
@@ -3,19 +3,28 @@ package com.yahoo.schema;
 
 import com.yahoo.search.query.ranking.Diversity;
 import com.yahoo.schema.parser.ParseException;
+import com.yahoo.vespa.model.test.utils.DeployLoggerStub;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author baldersheim
  */
 public class DiversityTestCase {
-    @Test
-    void testDiversity() throws ParseException {
+    private static void verifyDiversity(DeployLoggerStub logger, boolean atRankProfile, boolean atMatchPhase) throws ParseException {
         RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
-        ApplicationBuilder builder = new ApplicationBuilder(rankProfileRegistry);
+        ApplicationBuilder builder = new ApplicationBuilder(logger, rankProfileRegistry);
+        String diversitySpec = """
+                diversity {
+                    attribute: b
+                    min-groups: 74
+                    cutoff-factor: 17.3
+                    cutoff-strategy: strict
+                }
+                """;
         builder.addSchema(
                 """
                         search test {
@@ -29,17 +38,15 @@ public class DiversityTestCase {
                                 }
                             }
                             rank-profile parent {
-                                match-phase {
+                                match-phase {""" +
+                        (atMatchPhase ? diversitySpec : "") +
+                        """
                                     attribute: a
                                     max-hits: 120
                                     max-filter-coverage: 0.065
-                                }
-                                diversity {
-                                    attribute: b
-                                    min-groups: 74
-                                    cutoff-factor: 17.3
-                                    cutoff-strategy: strict
-                                }
+                                }""" +
+                        (atRankProfile ? diversitySpec : "") +
+                                """
                             }
                         }
                         """);
@@ -56,6 +63,21 @@ public class DiversityTestCase {
         assertEquals("a", matchPhase.getAttribute());
         assertEquals(0.065, matchPhase.getMaxFilterCoverage(), 1e-16);
     }
+    @Test
+    void testDiversity() throws ParseException {
+        DeployLoggerStub logger = new DeployLoggerStub();
+        verifyDiversity(logger, true, false);
+        assertTrue(logger.entries.isEmpty());
+        verifyDiversity(logger, false, true);
+        assertEquals(1, logger.entries.size());
+        assertEquals("'diversity is deprecated inside 'match-phase'. Specify it at 'rank-profile' level.", logger.entries.get(0).message);
+        try {
+            verifyDiversity(logger, true, true);
+            fail("Should throw.");
+        } catch (Exception e) {
+            assertEquals("rank-profile 'parent' error: already has diversity", e.getMessage());
+        }
+    }
 
     private static String getMessagePrefix() {
         return "In search definition 'test', rank-profile 'parent': diversity attribute 'b' ";
@@ -85,8 +107,7 @@ public class DiversityTestCase {
         }
     }
     private ApplicationBuilder getSearchBuilder(String diversityField) throws ParseException {
-        RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
-        ApplicationBuilder builder = new ApplicationBuilder(rankProfileRegistry);
+        ApplicationBuilder builder = new ApplicationBuilder(new RankProfileRegistry());
         builder.addSchema("""
                         search test {
                             document test {
@@ -99,13 +120,13 @@ public class DiversityTestCase {
                             }
                             rank-profile parent {
                                 match-phase {
-                                    diversity {
-                                        attribute: b
-                                        min-groups: 74
-                                    }
                                     attribute: a
                                     max-hits: 120
                                 }
+                                diversity {
+                                    attribute: b
+                                    min-groups: 74
+                                }
                             }
                         }""");
         return builder;