Merge pull request #18303 from vespa-engine/bratseth/struct-type-collision

Internal structs take precedence over external documents

7 files changed, 156 insertions, 52 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 38d831a0b28..da338ad3107 100644
--- a/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java
+++ b/config-model/src/main/java/com/yahoo/documentmodel/NewDocumentType.java
@@ -3,7 +3,6 @@ package com.yahoo.documentmodel;
 
 import com.yahoo.document.DataType;
 import com.yahoo.document.Document;
-import com.yahoo.document.DocumentId;
 import com.yahoo.document.Field;
 import com.yahoo.document.StructDataType;
 import com.yahoo.document.StructuredDataType;
@@ -32,34 +31,6 @@ import static java.util.Collections.emptySet;
  */
 public final class NewDocumentType extends StructuredDataType implements DataTypeCollection {
 
-    public static final class Name {
-
-        private final String name;
-        private final int id;
-
-        public Name(String name) {
-            this(name.hashCode(), name);
-        }
-
-        public Name(int id, String name) {
-            this.id = id;
-            this.name = name;
-        }
-
-        public String toString() { return name; }
-
-        public final String getName() { return name; }
-
-        public final int getId() { return id; }
-
-        public int hashCode() { return name.hashCode(); }
-
-        public boolean equals(Object other) {
-            if ( ! (other instanceof Name)) return false;
-            return name.equals(((Name)other).getName());
-        }
-    }
-
     private final Name name;
     private final DataTypeRepo dataTypes = new DataTypeRepo();
     private final Map<Integer, NewDocumentType> inherits = new LinkedHashMap<>();
@@ -139,7 +110,7 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp
     }
 
     @Override
-    public Class getValueClass() {
+    public Class<Document> getValueClass() {
         return Document.class;
     }
 
@@ -148,7 +119,8 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp
         if (!(value instanceof Document)) {
             return false;
         }
-        /** Temporary disabled  due to clash with document and covariant return type
+        /*
+         Temporary disabled  due to clash with document and covariant return type
          Document doc = (Document) value;
          if (((NewDocumentType) doc.getDataType()).inherits(this)) {
          //the value is of this type; or the supertype of the value is of this type, etc....
@@ -162,28 +134,31 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp
         for (Field f : getFields()) {
             Field inhF = inherited.getField(f.getName());
             if (inhF != null && !inhF.equals(f)) {
-                 throw new IllegalArgumentException("Inherited document '" + inherited.toString() + "' already contains field '" +
-                         inhF.getName() + "'. Can not override with '" + f.getName() + "'.");
+                 throw new IllegalArgumentException("Inherited document '" + inherited + "' already contains field '" +
+                                                    inhF.getName() + "'. Can not override with '" + f.getName() + "'.");
             }
         }
         for (Field f : inherited.getAllFields()) {
             for (NewDocumentType side : inherits.values()) {
                 Field sideF = side.getField(f.getName());
                 if (sideF != null && !sideF.equals(f)) {
-                    throw new IllegalArgumentException("Inherited document '" + side.toString() + "' already contains field '" +
-                            sideF.getName() + "'. Document '" + inherited.toString() + "' also defines field '" + f.getName() +
-                            "'.Multiple inheritance must be disjunctive.");
+                    throw new IllegalArgumentException("Inherited document '" + side + "' already contains field '" +
+                                                       sideF.getName() + "'. Document '" + inherited +
+                                                       "' also defines field '" + f.getName() +
+                                                       "'.Multiple inheritance must be disjunctive.");
                 }
             }
         }
         return true;
     }
+
     public void inherit(NewDocumentType inherited) {
         if ( ! inherits.containsKey(inherited.getId())) {
             verifyInheritance(inherited);
             inherits.put(inherited.getId(), inherited);
         }
     }
+
     public boolean inherits(NewDocumentType superType) {
         if (getId() == superType.getId()) return true;
         for (NewDocumentType type : inherits.values()) {
@@ -243,7 +218,7 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp
 
     @Override
     public Document createFieldValue() {
-        return new Document(null, (DocumentId)null);
+        throw new RuntimeException("Cannot create an instance of " + this);
     }
 
     @Override
@@ -375,4 +350,36 @@ public final class NewDocumentType extends StructuredDataType implements DataTyp
         return importedFieldNames;
     }
 
+    public static final class Name {
+
+        private final String name;
+        private final int id;
+
+        public Name(String name) {
+            this(name.hashCode(), name);
+        }
+
+        public Name(int id, String name) {
+            this.id = id;
+            this.name = name;
+        }
+
+        @Override
+        public String toString() { return name; }
+
+        public final String getName() { return name; }
+
+        public final int getId() { return id; }
+
+        @Override
+        public int hashCode() { return name.hashCode(); }
+
+        @Override
+        public boolean equals(Object other) {
+            if ( ! (other instanceof Name)) return false;
+            return name.equals(((Name)other).getName());
+        }
+
+    }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java
index fed35382b21..9b752c4179f 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentModelBuilder.java
@@ -209,17 +209,13 @@ public class DocumentModelBuilder {
     private static DataType resolveTemporariesRecurse(DataType type, DataTypeCollection repo,
                                                       Collection<NewDocumentType> docs) {
         if (type instanceof TemporaryStructuredDataType) {
-            NewDocumentType docType = getDocumentType(docs, type.getId());
-            if (docType != null) {
-                type = docType;
-                return type;
-            }
-            DataType real = repo.getDataType(type.getId());
-            if (real == null) {
-                throw new NullPointerException("Can not find type '" + type.toString() + "', impossible.");
-            }
-            type = real;
-        } else if (type instanceof StructDataType) {
+            DataType struct = repo.getDataType(type.getId());
+            if (struct != null)
+                type = struct;
+            else
+                type = getDocumentType(docs, type.getId());
+        }
+        else if (type instanceof StructDataType) {
             StructDataType dt = (StructDataType) type;
             for (com.yahoo.document.Field field : dt.getFields()) {
                 if (field.getDataType() != type) {
@@ -227,14 +223,17 @@ public class DocumentModelBuilder {
                     field.setDataType(resolveTemporariesRecurse(field.getDataType(), repo, docs));
                 }
             }
-        } else if (type instanceof MapDataType) {
+        }
+        else if (type instanceof MapDataType) {
             MapDataType t = (MapDataType) type;
             t.setKeyType(resolveTemporariesRecurse(t.getKeyType(), repo, docs));
             t.setValueType(resolveTemporariesRecurse(t.getValueType(), repo, docs));
-        } else if (type instanceof CollectionDataType) {
+        }
+        else if (type instanceof CollectionDataType) {
             CollectionDataType t = (CollectionDataType) type;
             t.setNestedType(resolveTemporariesRecurse(t.getNestedType(), repo, docs));
-        } else if (type instanceof ReferenceDataType) {
+        }
+        else if (type instanceof ReferenceDataType) {
             ReferenceDataType t = (ReferenceDataType) type;
             if (t.getTargetType() instanceof TemporaryStructuredDataType) {
                 DataType targetType = resolveTemporariesRecurse(t.getTargetType(), repo, docs);
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Search.java b/config-model/src/main/java/com/yahoo/searchdefinition/Search.java
index 4b7b1625a01..9ce1b8bb330 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/Search.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/Search.java
@@ -98,7 +98,7 @@ public class Search implements ImmutableSearch {
     private final DeployLogger deployLogger;
     private final ModelContext.Properties properties;
 
-    /** Testin only */
+    /** Testing only */
     public Search(String name) {
         this(name, null, new BaseDeployLogger(), new TestProperties());
     }
diff --git a/config-model/src/test/derived/namecollision/collision.sd b/config-model/src/test/derived/namecollision/collision.sd
new file mode 100644
index 00000000000..43dd4830204
--- /dev/null
+++ b/config-model/src/test/derived/namecollision/collision.sd
@@ -0,0 +1,8 @@
+# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+search collision {
+
+  document collision {
+
+  }
+
+}
diff --git a/config-model/src/test/derived/namecollision/collisionstruct.sd b/config-model/src/test/derived/namecollision/collisionstruct.sd
new file mode 100644
index 00000000000..c98efb0b582
--- /dev/null
+++ b/config-model/src/test/derived/namecollision/collisionstruct.sd
@@ -0,0 +1,15 @@
+# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+search collisionstruct {
+
+    document collisionstruct {
+
+        struct collision {
+        }
+
+        field structarray type array<collision> {
+            indexing: summary
+        }
+
+    }
+
+}
diff --git a/config-model/src/test/derived/namecollision/documentmanager.cfg b/config-model/src/test/derived/namecollision/documentmanager.cfg
new file mode 100644
index 00000000000..8d0d89dde35
--- /dev/null
+++ b/config-model/src/test/derived/namecollision/documentmanager.cfg
@@ -0,0 +1,55 @@
+enablecompression false
+datatype[].id 1381038251
+datatype[].structtype[].name "position"
+datatype[].structtype[].version 0
+datatype[].structtype[].compresstype NONE
+datatype[].structtype[].compresslevel 0
+datatype[].structtype[].compressthreshold 95
+datatype[].structtype[].compressminsize 800
+datatype[].structtype[].field[].name "x"
+datatype[].structtype[].field[].datatype 0
+datatype[].structtype[].field[].detailedtype ""
+datatype[].structtype[].field[].name "y"
+datatype[].structtype[].field[].datatype 0
+datatype[].structtype[].field[].detailedtype ""
+datatype[].id -379118517
+datatype[].structtype[].name "collision.header"
+datatype[].structtype[].version 0
+datatype[].structtype[].compresstype NONE
+datatype[].structtype[].compresslevel 0
+datatype[].structtype[].compressthreshold 95
+datatype[].structtype[].compressminsize 800
+datatype[].id 1557022836
+datatype[].documenttype[].name "collision"
+datatype[].documenttype[].version 0
+datatype[].documenttype[].inherits[].name "document"
+datatype[].documenttype[].inherits[].version 0
+datatype[].documenttype[].headerstruct -379118517
+datatype[].documenttype[].bodystruct 0
+datatype[].id 1557022836
+datatype[].structtype[].name "collision"
+datatype[].structtype[].version 0
+datatype[].structtype[].compresstype NONE
+datatype[].structtype[].compresslevel 0
+datatype[].structtype[].compressthreshold 95
+datatype[].structtype[].compressminsize 800
+datatype[].id -1730522993
+datatype[].arraytype[].datatype 1557022836
+datatype[].id -1270379114
+datatype[].structtype[].name "collisionstruct.header"
+datatype[].structtype[].version 0
+datatype[].structtype[].compresstype NONE
+datatype[].structtype[].compresslevel 0
+datatype[].structtype[].compressthreshold 95
+datatype[].structtype[].compressminsize 800
+datatype[].structtype[].field[].name "structarray"
+datatype[].structtype[].field[].datatype -1730522993
+datatype[].structtype[].field[].detailedtype ""
+datatype[].id -1723079287
+datatype[].documenttype[].name "collisionstruct"
+datatype[].documenttype[].version 0
+datatype[].documenttype[].inherits[].name "document"
+datatype[].documenttype[].inherits[].version 0
+datatype[].documenttype[].headerstruct -1270379114
+datatype[].documenttype[].bodystruct 0
+datatype[].documenttype[].fieldsets{[]}.fields[] "structarray"
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/derived/NameCollisionTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/derived/NameCollisionTestCase.java
new file mode 100644
index 00000000000..fda9e6327ce
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/derived/NameCollisionTestCase.java
@@ -0,0 +1,20 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+package com.yahoo.searchdefinition.derived;
+
+import org.junit.Test;
+
+/**
+ * Verifies that a struct in a document type is preferred over another dopcument type
+ * of the same name.
+ *
+ * @author bratseth
+ */
+public class NameCollisionTestCase extends AbstractExportingTestCase {
+
+    @Test
+    public void testNameCollision() throws Exception {
+        assertCorrectDeriving("namecollision", "collisionstruct", new TestableDeployLogger());
+    }
+
+}