Test adding to inherited collections

6 files changed, 138 insertions, 40 deletions

diff --git a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java
index 301141d0465..08a0f8b9882 100644
--- a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java
+++ b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java
@@ -22,6 +22,7 @@ import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 import static java.util.Collections.emptySet;
@@ -53,7 +54,7 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp
         this(
                 name,
                 new StructDataType(name.getName() + ".header"),
-                new FieldSets(),
+                new FieldSets(Optional.empty()),
                 documentReferences,
                 importedFieldNames);
     }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/FieldSets.java b/config-model/src/main/java/com/yahoo/searchdefinition/FieldSets.java
index fa220003adc..065ade9b094 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/FieldSets.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/FieldSets.java
@@ -4,6 +4,8 @@ package com.yahoo.searchdefinition;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
 
 import com.yahoo.searchdefinition.document.FieldSet;
 
@@ -15,24 +17,16 @@ import com.yahoo.searchdefinition.document.FieldSet;
  */
 public class FieldSets {
 
+    private final Optional<Schema> owner;
     private final Map<String, FieldSet> userFieldSets;
     private final Map<String, FieldSet> builtInFieldSets;
 
-    public FieldSets() {
+    public FieldSets(Optional<Schema> owner) {
+        this.owner = owner;
         userFieldSets = new LinkedHashMap<>();
         builtInFieldSets = new LinkedHashMap<>();
     }
 
-    FieldSets(FieldSets other) {
-        userFieldSets = new LinkedHashMap<>(other.userFieldSets);
-        builtInFieldSets = new LinkedHashMap<>(other.builtInFieldSets);
-    }
-
-    void add(FieldSets other) {
-        userFieldSets.putAll(other.userFieldSets);
-        builtInFieldSets.putAll(other.builtInFieldSets);
-    }
-
     /**
      * Adds an entry to user field sets, creating entries as needed
      *
@@ -63,12 +57,22 @@ public class FieldSets {
 
     /** Returns the built in field sets, unmodifiable */
     public Map<String, FieldSet> builtInFieldSets() {
-        return Collections.unmodifiableMap(builtInFieldSets);
+        if (owner.isEmpty() || owner.get().inherited().isEmpty()) return Collections.unmodifiableMap(builtInFieldSets);
+        if (builtInFieldSets.isEmpty()) return owner.get().inherited().get().fieldSets().builtInFieldSets();
+
+        var fieldSets = new LinkedHashMap<>(owner.get().inherited().get().fieldSets().builtInFieldSets());
+        fieldSets.putAll(builtInFieldSets);
+        return Collections.unmodifiableMap(fieldSets);
     }
     
     /** Returns the user defined field sets, unmodifiable */
     public Map<String, FieldSet> userFieldSets() {
-        return Collections.unmodifiableMap(userFieldSets);
+        if (owner.isEmpty() || owner.get().inherited().isEmpty()) return Collections.unmodifiableMap(userFieldSets);
+        if (userFieldSets.isEmpty()) return owner.get().inherited().get().fieldSets().userFieldSets();
+
+        var fieldSets = new LinkedHashMap<>(owner.get().inherited().get().fieldSets().userFieldSets());
+        fieldSets.putAll(userFieldSets);
+        return Collections.unmodifiableMap(fieldSets);
     }
     
 }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
index e8e7c999c07..b2d4f0592fe 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
@@ -67,7 +67,7 @@ public class Schema implements ImmutableSchema {
     /** The stemming setting of this schema. Default is BEST. */
     private Stemming stemming = null;
 
-    private final FieldSets fieldSets = new FieldSets();
+    private final FieldSets fieldSets = new FieldSets(Optional.of(this));
 
     /** The document contained in this schema */
     private SDDocumentType documentType;
@@ -697,13 +697,7 @@ public class Schema implements ImmutableSchema {
         return false;
     }
 
-    public FieldSets fieldSets() {
-        if (inherited.isEmpty()) return fieldSets;
-
-        var fieldSets = new FieldSets(requireInherited().fieldSets());
-        fieldSets.add(this.fieldSets);
-        return fieldSets;
-    }
+    public FieldSets fieldSets() {  return fieldSets; }
 
     /** Returns the schema inherited by this, or throws if none */
     private Schema requireInherited() { return owner.schemas().get(inherited.get()); }
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 1ebaf5d65e6..967e662dc6b 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
@@ -55,19 +55,13 @@ public class DocumentSummary extends FieldView {
     }
 
     public SummaryField getSummaryField(String name) {
-        // TODO: This before parent?
-        var parent = getInherited();
-        if (parent != null) {
-            return parent.getSummaryField(name);
-        }
-        return (SummaryField) get(name);
-    }
-
-    /** Returns a summary field if defined in this, not any parent */
-    public SummaryField getSummaryFieldInThis(String name) {
-        return (SummaryField) get(name);
+        var field = (SummaryField)get(name);
+        if (field != null) return field;
+        if (getInherited() == null)  return null;
+        return getInherited().getSummaryField(name);
     }
 
+    // TODO: This does not handle overriding in child summaries correctly
     public Collection<SummaryField> getSummaryFields() {
         var fields = new ArrayList<SummaryField>(getFields().size());
         var parent = getInherited();
@@ -115,11 +109,6 @@ public class DocumentSummary extends FieldView {
         return owner.getSummary(inherited);
     }
 
-    /** Returns the name of the summary this was declared to inherit, or null if not set to inherit anything */
-    public String getInheritedName() {
-        return inherited;
-    }
-
     @Override
     public String toString() {
         return "document summary '" + getName() + "'";
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java
index 1813ee1f636..0f9951b707b 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/SchemaTestCase.java
@@ -109,6 +109,116 @@ public class SchemaTestCase {
                 "      indexing: summary" +
                 "    }" +
                 "  }" +
+                "  fieldset child_set {" +
+                "    fields: cf1, pf1" +
+                "  }" +
+                "  stemming: shortest" +
+                "  index child_index {" +
+                "    stemming: shortest" +
+                "  }" +
+                "  field child_field type string {" +
+                "      indexing: input pf1 | lowercase | index | attribute | summary" +
+                "  }" +
+                "  rank-profile child_profile inherits parent_profile {" +
+                "  }" +
+                "  constant child_constant {" +
+                "    file: constants/my_constant_tensor_file.json" +
+                "    type: tensor<float>(x{},y{})" +
+                "  }" +
+                "  onnx-model child_model {" +
+                "    file: models/my_model.onnx" +
+                "  }" +
+                "  document-summary child_summary inherits parent_summary {" +
+                "    summary cf1 type string {}" +
+                "  }" +
+                "  import field parentschema_ref.name as child_imported {}" +
+                "}");
+
+        SearchBuilder builder = new SearchBuilder(new DeployLoggerStub());
+        builder.processorsToSkip().add(OnnxModelTypeResolver.class); // Avoid discovering the Onnx model referenced does not exist
+        builder.processorsToSkip().add(ImportedFieldsResolver.class); // Avoid discovering the document reference leads nowhere
+        builder.importString(parentLines);
+        builder.importString(childLines);
+        builder.build(true);
+        var application = builder.application();
+
+        var child = application.schemas().get("child");
+        assertEquals("pf1", child.fieldSets().userFieldSets().get("parent_set").getFieldNames().stream().findFirst().get());
+        assertEquals("[cf1, pf1]", child.fieldSets().userFieldSets().get("child_set").getFieldNames().toString());
+        assertEquals(Stemming.SHORTEST, child.getStemming());
+        assertEquals(Stemming.BEST, child.getIndex("parent_index").getStemming());
+        assertEquals(Stemming.SHORTEST, child.getIndex("child_index").getStemming());
+        assertNotNull(child.getField("parent_field"));
+        assertNotNull(child.getField("child_field"));
+        assertNotNull(child.getExtraField("parent_field"));
+        assertNotNull(child.getExtraField("child_field"));
+        assertNotNull(application.rankProfileRegistry().get(child, "parent_profile"));
+        assertNotNull(application.rankProfileRegistry().get(child, "child_profile"));
+        assertEquals("parent_profile", application.rankProfileRegistry().get(child, "child_profile").getInheritedName());
+        assertNotNull(child.rankingConstants().get("parent_constant"));
+        assertNotNull(child.rankingConstants().get("child_constant"));
+        assertTrue(child.rankingConstants().asMap().containsKey("parent_constant"));
+        assertTrue(child.rankingConstants().asMap().containsKey("child_constant"));
+        assertNotNull(child.onnxModels().get("parent_model"));
+        assertNotNull(child.onnxModels().get("child_model"));
+        assertTrue(child.onnxModels().asMap().containsKey("parent_model"));
+        assertTrue(child.onnxModels().asMap().containsKey("child_model"));
+        assertNotNull(child.getSummary("parent_summary"));
+        assertNotNull(child.getSummary("child_summary"));
+        assertEquals("parent_summary", child.getSummary("child_summary").getInherited().getName());
+        assertTrue(child.getSummaries().containsKey("parent_summary"));
+        assertTrue(child.getSummaries().containsKey("child_summary"));
+        assertNotNull(child.getSummaryField("pf1"));
+        assertNotNull(child.getSummaryField("cf1"));
+        assertNotNull(child.getExplicitSummaryField("pf1"));
+        assertNotNull(child.getExplicitSummaryField("cf1"));
+        assertNotNull(child.getUniqueNamedSummaryFields().get("pf1"));
+        assertNotNull(child.getUniqueNamedSummaryFields().get("cf1"));
+        assertNotNull(child.temporaryImportedFields().get().fields().get("parent_imported"));
+        assertNotNull(child.temporaryImportedFields().get().fields().get("child_imported"));
+    }
+
+    @Test
+    public void testSchemaInheritanceEmptyChildren() throws ParseException {
+        String parentLines = joinLines(
+                "schema parent {" +
+                "  document parent {" +
+                "    field pf1 type string {" +
+                "      indexing: summary" +
+                "    }" +
+                "  }" +
+                "  fieldset parent_set {" +
+                "    fields: pf1" +
+                "  }" +
+                "  stemming: none" +
+                "  index parent_index {" +
+                "    stemming: best" +
+                "  }" +
+                "  field parent_field type string {" +
+                "      indexing: input pf1 | lowercase | index | attribute | summary" +
+                "  }" +
+                "  rank-profile parent_profile {" +
+                "  }" +
+                "  constant parent_constant {" +
+                "    file: constants/my_constant_tensor_file.json" +
+                "    type: tensor<float>(x{},y{})" +
+                "  }" +
+                "  onnx-model parent_model {" +
+                "    file: models/my_model.onnx" +
+                "  }" +
+                "  document-summary parent_summary {" +
+                "    summary pf1 type string {}" +
+                "  }" +
+                "  import field parentschema_ref.name as parent_imported {}" +
+                "  raw-as-base64-in-summary" +
+                "}");
+        String childLines = joinLines(
+                "schema child inherits parent {" +
+                "  document child inherits parent {" +
+                "    field cf1 type string {" +
+                "      indexing: summary" +
+                "    }" +
+                "  }" +
                 "}");
         String grandchildLines = joinLines(
                 "schema grandchild inherits child {" +
@@ -148,7 +258,6 @@ public class SchemaTestCase {
         assertNotNull(schema.getSummaryField("pf1"));
         assertNotNull(schema.getExplicitSummaryField("pf1"));
         assertNotNull(schema.getUniqueNamedSummaryFields().get("pf1"));
-        assertTrue(schema.temporaryImportedFields().isPresent());
         assertNotNull(schema.temporaryImportedFields().get().fields().get("parent_imported"));
         assertTrue(schema.isRawAsBase64());
     }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java
index be47f448e40..0e96420f03c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java
@@ -17,6 +17,7 @@ import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 import static com.yahoo.vespa.model.application.validation.change.ConfigChangeTestUtils.newRefeedAction;
 import static org.junit.Assert.assertEquals;
@@ -208,7 +209,7 @@ public class DocumentTypeChangeValidatorTest {
         return new NewDocumentType(
                 new NewDocumentType.Name("mydoc"),
                 headerfields,
-                new FieldSets(),
+                new FieldSets(Optional.empty()),
                 Collections.emptySet(),
                 Collections.emptySet());
     }