Support inheritance in document-summary

4 files changed, 164 insertions, 6 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaryFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaryFields.java
index 0d99c698aca..8db4ec432c5 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaryFields.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImplicitSummaryFields.java
@@ -23,6 +23,7 @@ public class ImplicitSummaryFields extends Processor {
     @Override
     public void process(boolean validate, boolean documentsOnly) {
         for (DocumentSummary docsum : search.getSummaries().values()) {
+            if (docsum.getInherited() != null) continue; // Implicit fields are added to inheriting summaries through their parent
             addField(docsum, new SummaryField("rankfeatures", DataType.STRING, SummaryTransform.RANKFEATURES), validate);
             addField(docsum, new SummaryField("summaryfeatures", DataType.STRING, SummaryTransform.SUMMARYFEATURES), validate);
         }
diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java
index 8dc5356026b..3c6aa881af4 100644
--- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java
+++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/DocumentSummary.java
@@ -1,8 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.documentmodel;
 
-import com.yahoo.document.Field;
-
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
@@ -16,6 +14,7 @@ import java.util.List;
 public class DocumentSummary extends FieldView {
 
     private boolean fromDisk = false;
+    private DocumentSummary inherited;
 
     /**
      * Creates a DocumentSummary with the given name.
@@ -44,13 +43,21 @@ public class DocumentSummary extends FieldView {
     }
 
     public SummaryField getSummaryField(String name) {
+        var parent = getInherited();
+        if (parent != null) {
+            return parent.getSummaryField(name);
+        }
         return (SummaryField) get(name);
     }
 
     public Collection<SummaryField> getSummaryFields() {
-        ArrayList<SummaryField> fields = new ArrayList<>(getFields().size());
-        for(Field f : getFields()) {
-            fields.add((SummaryField) f);
+        var fields = new ArrayList<SummaryField>(getFields().size());
+        var parent = getInherited();
+        if (parent != null) {
+            fields.addAll(parent.getSummaryFields());
+        }
+        for (var field : getFields()) {
+            fields.add((SummaryField) field);
         }
         return fields;
     }
@@ -80,8 +87,19 @@ public class DocumentSummary extends FieldView {
         }
     }
 
+    /** Sets the parent of this. Both summaries must be present in the same search definition */
+    public void setInherited(DocumentSummary inherited) {
+        this.inherited = inherited;
+    }
+
+    /** Returns the parent of this, or null if none is inherited */
+    public DocumentSummary getInherited() {
+        return inherited;
+    }
+
     public String toString() {
-        return "document summary '" + getName() + "'";
+        return "document summary '" + getName() + "'" +
+               (inherited == null ? "" : " inheriting from '" + inherited.getName() + "'");
     }
 
 }
diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj
index e560d78a116..0e9a47145f2 100644
--- a/config-model/src/main/javacc/SDParser.jj
+++ b/config-model/src/main/javacc/SDParser.jj
@@ -1706,6 +1706,7 @@ Object documentSummary(Search search) :
 {
     ( <DOCUMENTSUMMARY>
       name = identifierWithDash() { search.addSummary(summary = new DocumentSummary(name)); }
+      [inheritsDocumentSummary(summary, search)]
       lbrace()
          (
            <FROMDISK> { summary.setFromDisk(true); } |
@@ -1718,6 +1719,23 @@ Object documentSummary(Search search) :
 }
 
 /**
+ * This rule consumes an inherits statement of a document summary.
+ *
+ * @param documentSummary The document summary to modify.
+ * @param search The search object documentSummary is being added to.
+ */
+void inheritsDocumentSummary(DocumentSummary documentSummary, Search search) :
+{
+    String name;
+}
+{
+    <INHERITS> name = identifierWithDash()
+    {
+        documentSummary.setInherited(search.getSummaries().get(name));
+    }
+}
+
+/**
  * Consumes a single document-summary item.
  *
  * @param summary The document summary to modify.
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SummaryTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SummaryTestCase.java
index 686fb2173da..91599e6f607 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/SummaryTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/SummaryTestCase.java
@@ -2,13 +2,19 @@
 package com.yahoo.searchdefinition;
 
 import com.yahoo.searchdefinition.parser.ParseException;
+import com.yahoo.vespa.documentmodel.DocumentSummary;
 import com.yahoo.vespa.model.test.utils.DeployLoggerStub;
+import com.yahoo.vespa.objects.FieldBase;
 import org.junit.Test;
 
+import java.util.Collection;
+import java.util.List;
 import java.util.logging.Level;
+import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Tests summary validation
@@ -130,4 +136,119 @@ public class SummaryTestCase {
         assertTrue(logger.entries.isEmpty());
     }
 
+    @Test
+    public void testInheritance() throws Exception {
+        String sd =
+                "search music {\n" +
+                "\n" +
+                "  document music {\n" +
+                "    field title type string {\n" +
+                "      indexing: summary | attribute | index\n" +
+                "    }\n" +
+                "    \n" +
+                "    field artist type string {\n" +
+                "      indexing: summary | attribute | index\n" +
+                "    }\n" +
+                "    \n" +
+                "    field album type string {\n" +
+                "      indexing: summary | attribute | index\n" +
+                "    }\n" +
+                "  }\n" +
+                "  \n" +
+                "  document-summary title {\n" +
+                "    summary title type string {\n" +
+                "      source: title\n" +
+                "    }\n" +
+                "  }\n" +
+                "  document-summary title_artist inherits title {\n" +
+                "    summary artist type string {\n" +
+                "      source: artist\n" +
+                "    }\n" +
+                "  }\n" +
+                "  document-summary everything inherits title_artist {\n" +
+                "    summary album type string {\n" +
+                "      source: album\n" +
+                "    }\n" +
+                "  }\n" +
+                "\n" +
+                "}";
+        var logger = new DeployLoggerStub();
+        var search = SearchBuilder.createFromString(sd, logger).getSearch();
+        assertEquals(List.of(), logger.entries);
+
+        var titleField = "title";
+        var artistField = "artist";
+        var albumField = "album";
+        var titleSummary = search.getSummary(titleField);
+        var titleArtistSummary = search.getSummary(titleField + "_" + artistField);
+        var everythingSummary = search.getSummary("everything");
+
+        var implicitFields = List.of("rankfeatures", "summaryfeatures");
+        var tests = List.of(
+                new TestValue(titleSummary, null, List.of(List.of(titleField), implicitFields)),
+                new TestValue(titleArtistSummary, titleSummary, List.of(List.of(titleField), implicitFields, List.of(artistField))),
+                new TestValue(everythingSummary, titleArtistSummary, List.of(List.of(titleField), implicitFields, List.of(artistField, albumField)))
+        );
+        tests.forEach(testValue -> {
+            var actualFields = testValue.summary.getSummaryFields().stream()
+                                                .map(FieldBase::getName)
+                                                .collect(Collectors.toList());
+            assertEquals(testValue.summary.getName() + (testValue.parent == null ? " does not inherit anything" : " inherits " + testValue.parent.getName()),
+                         testValue.parent,
+                         testValue.summary.getInherited());
+            assertEquals("Summary " + testValue.summary.getName() + " has expected fields", testValue.fields, actualFields);
+        });
+    }
+
+    @Test
+    public void testRedeclaringInheritedFieldFails() throws Exception {
+        String sd =
+                "search music {\n" +
+                "\n" +
+                "  document music {\n" +
+                "    field title type string {\n" +
+                "      indexing: summary | attribute | index\n" +
+                "    }\n" +
+                "    field title_short type string {\n" +
+                "      indexing: summary | attribute | index\n" +
+                "    }\n" +
+                "  }\n" +
+                "  \n" +
+                "  document-summary title {\n" +
+                "    summary title type string {\n" +
+                "      source: title\n" +
+                "    }\n" +
+                "  }\n" +
+                "  document-summary title2 inherits title {\n" +
+                "    summary title type string {\n" +
+                "      source: title_short\n" +
+                "    }\n" +
+                "  }\n" +
+                "  \n" +
+                "}";
+        var logger = new DeployLoggerStub();
+        try {
+            SearchBuilder.createFromString(sd, logger);
+            fail("Expected exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("For search 'music', summary class 'title2', summary field 'title': Can not use " +
+                         "source 'title_short' for this summary field, an equally named field in summary class 'title' " +
+                         "uses a different source: 'title'.", e.getMessage());
+        }
+    }
+
+    private static class TestValue {
+
+        private final DocumentSummary summary;
+        private final DocumentSummary parent;
+        private final List<String> fields;
+
+        public TestValue(DocumentSummary summary, DocumentSummary parent, List<List<String>> fields) {
+            this.summary = summary;
+            this.parent = parent;
+            this.fields = fields.stream().flatMap(Collection::stream).collect(Collectors.toList());;
+        }
+
+    }
+
 }