Make Application immutable

7 files changed, 94 insertions, 85 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
index 5822b1bca05..e4790a16f86 100644
--- a/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
+++ b/config-model/src/main/java/com/yahoo/config/model/test/MockApplicationPackage.java
@@ -126,7 +126,7 @@ public class MockApplicationPackage implements ApplicationPackage {
                                                         queryProfileRegistry);
         for (String sd : schemas) {
             try  {
-                String name = schemaBuilder.importString(sd);
+                String name = schemaBuilder.addSchema(sd).getName();
                 readers.add(new NamedReader(name + ApplicationPackage.SD_NAME_SUFFIX, new StringReader(sd)));
             } catch (ParseException e) {
                 throw new RuntimeException(e);
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/Application.java b/config-model/src/main/java/com/yahoo/searchdefinition/Application.java
index fe29c9cf313..9c1cc839092 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/Application.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/Application.java
@@ -20,23 +20,28 @@ import java.util.Map;
 public class Application {
 
     private final ApplicationPackage applicationPackage;
-    private final Map<String, Schema> schemas = new LinkedHashMap<>();
+    private final Map<String, Schema> schemas;
     private final DocumentModel documentModel = new DocumentModel();
 
-    public Application(ApplicationPackage applicationPackage) {
+    public Application(ApplicationPackage applicationPackage, List<Schema> schemas, DeployLogger logger) {
         this.applicationPackage = applicationPackage;
-    }
 
-    public ApplicationPackage applicationPackage() { return applicationPackage; }
+        Map<String, Schema> schemaMap = new LinkedHashMap<>();
+        for (Schema schema : schemas) {
+            if (schemaMap.containsKey(schema.getName()))
+                throw new IllegalArgumentException("Duplicate schema '" + schema.getName() + "' in " + this);
+            schemaMap.put(schema.getName(), schema);
+        }
+        this.schemas = Collections.unmodifiableMap(schemaMap);
 
-    public void add(Schema schema) {
-        if (schemas.containsKey(schema.getName()))
-            throw new IllegalArgumentException("Duplicate schema '" + schema.getName() + "' in " + this);
-        schemas.put(schema.getName(), schema);
+        schemas.forEach(schema -> schema.setOwner(this));
+        schemas.forEach(schema -> schema.validate(logger));
     }
 
+    public ApplicationPackage applicationPackage() { return applicationPackage; }
+
     /** Returns an unmodifiable list of the schemas of this application */
-    public Map<String, Schema> schemas() { return Collections.unmodifiableMap(schemas); }
+    public Map<String, Schema> schemas() { return schemas; }
 
     public void buildDocumentModel(List<Schema> schemasSomewhatOrdered) {
         var builder = new DocumentModelBuilder(documentModel);
@@ -45,11 +50,6 @@ public class Application {
 
     public DocumentModel documentModel() { return documentModel; }
 
-    /** Validates this. Must be called after all content is added to it. */
-    public void validate(DeployLogger logger) {
-        schemas.values().forEach(schema -> schema.validate(logger));
-    }
-
     @Override
     public String toString() { return "application " + applicationPackage.getApplicationId(); }
 
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentOnlySchema.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentOnlySchema.java
index c672b662874..1d71a9f1494 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentOnlySchema.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentOnlySchema.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.searchdefinition;
 
+import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.application.api.FileRegistry;
 import com.yahoo.config.model.api.ModelContext;
@@ -14,8 +15,11 @@ import com.yahoo.searchdefinition.document.SDDocumentType;
  */
 public class DocumentOnlySchema extends Schema {
 
-    public DocumentOnlySchema(Application application, FileRegistry fileRegistry, DeployLogger deployLogger, ModelContext.Properties properties) {
-        super(application, fileRegistry, deployLogger, properties);
+    public DocumentOnlySchema(ApplicationPackage applicationPackage,
+                              FileRegistry fileRegistry,
+                              DeployLogger deployLogger,
+                              ModelContext.Properties properties) {
+        super(applicationPackage, fileRegistry, deployLogger, properties);
     }
 
     @Override
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 6ef4084e5b7..4df118545eb 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/Schema.java
@@ -55,6 +55,9 @@ public class Schema implements ImmutableSchema {
     /** The unique name of this schema */
     private String name;
 
+    /** The application package this is constructed from */
+    private final ApplicationPackage applicationPackage;
+
     /** The name of the schema this should inherit all the content of, if any */
     private final Optional<String> inherited;
 
@@ -93,21 +96,22 @@ public class Schema implements ImmutableSchema {
     /** The resulting processed field */
     private Optional<ImportedFields> importedFields = Optional.empty();
 
-    private final Application owner;
     private final DeployLogger deployLogger;
     private final ModelContext.Properties properties;
 
+    private Application owner;
+
     /** Testing only */
-    public Schema(String name) {
-        this(name, Optional.empty(), null, null, new BaseDeployLogger(), new TestProperties());
+    public Schema(String name, ApplicationPackage applicationPackage) {
+        this(name, applicationPackage, Optional.empty(), null, new BaseDeployLogger(), new TestProperties());
     }
 
     public Schema(String name,
-                  Application application,
+                  ApplicationPackage applicationPackage,
                   FileRegistry fileRegistry,
                   DeployLogger deployLogger,
                   ModelContext.Properties properties) {
-        this(name, Optional.empty(), application, fileRegistry, deployLogger, properties);
+        this(name, applicationPackage, Optional.empty(), fileRegistry, deployLogger, properties);
     }
 
     /**
@@ -115,30 +119,30 @@ public class Schema implements ImmutableSchema {
      *
      * @param name of the schema
      * @param inherited the schema this inherits, if any
-     * @param application the application containing this
      */
     public Schema(String name,
+                  ApplicationPackage applicationPackage,
                   Optional<String> inherited,
-                  Application application,
                   FileRegistry fileRegistry,
                   DeployLogger deployLogger,
                   ModelContext.Properties properties) {
-        this(inherited, application, fileRegistry, deployLogger, properties, false);
-        this.name = name;
+        this(inherited, applicationPackage, fileRegistry, deployLogger, properties, false);
+        this.name = Objects.requireNonNull(name, "A schema must have a name");
     }
 
-    protected Schema(Application application, FileRegistry fileRegistry, DeployLogger deployLogger, ModelContext.Properties properties) {
-        this(Optional.empty(), application, fileRegistry, deployLogger, properties, true);
+    protected Schema(ApplicationPackage applicationPackage, FileRegistry fileRegistry,
+                     DeployLogger deployLogger, ModelContext.Properties properties) {
+        this(Optional.empty(), applicationPackage, fileRegistry, deployLogger, properties, true);
     }
 
     private Schema(Optional<String> inherited,
-                   Application application,
+                   ApplicationPackage applicationPackage,
                    FileRegistry fileRegistry,
                    DeployLogger deployLogger,
                    ModelContext.Properties properties,
                    boolean documentsOnly) {
         this.inherited = inherited;
-        this.owner = application;
+        this.applicationPackage = applicationPackage;
         this.deployLogger = deployLogger;
         this.properties = properties;
         this.documentsOnly = documentsOnly;
@@ -147,6 +151,17 @@ public class Schema implements ImmutableSchema {
         onnxModels = new OnnxModels(fileRegistry, Optional.of(this));
     }
 
+    /**
+     * Assigns the owner of this
+     *
+     * @throws IllegalStateException if an owner is already assigned
+     */
+    public void setOwner(Application owner) {
+        if (this.owner != null)
+            throw new IllegalStateException("Cannot reassign the owner of " + this);
+        this.owner = owner;
+    }
+
     protected void setName(String name) { this.name = name; }
 
     @Override
@@ -307,16 +322,13 @@ public class Schema implements ImmutableSchema {
      */
     @Override
     public Reader getRankingExpression(String fileName) {
-        return owner.applicationPackage().getRankingExpression(fileName);
+        return applicationPackage.getRankingExpression(fileName);
     }
 
     public Application application() { return owner; }
 
     @Override
-    public ApplicationPackage applicationPackage() {
-        if (owner == null) return null;
-        return owner.applicationPackage();
-    }
+    public ApplicationPackage applicationPackage() { return applicationPackage; }
 
     @Override
     public DeployLogger getDeployLogger() { return deployLogger; }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java
index 8b22612108f..efa0d0784bb 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/SchemaBuilder.java
@@ -48,8 +48,9 @@ import java.util.Set;
 // NOTE: Since this was created we have added Application, and much of the content in this should migrate there.
 public class SchemaBuilder {
 
-    private final DocumentTypeManager docTypeMgr = new DocumentTypeManager();
-    private final Application application;
+    private final ApplicationPackage applicationPackage;
+    private final List<Schema> schemas = new ArrayList<>();
+    private final DocumentTypeManager documentTypeManager = new DocumentTypeManager();
     private final RankProfileRegistry rankProfileRegistry;
     private final QueryProfileRegistry queryProfileRegistry;
     private final FileRegistry fileRegistry;
@@ -58,7 +59,7 @@ public class SchemaBuilder {
     /** True to build the document aspect only, skipping instantiation of rank profiles */
     private final boolean documentsOnly;
 
-    private boolean isBuilt = false;
+    private Application application;
 
     private final Set<Class<? extends Processor>> processorsToSkip = new HashSet<>();
 
@@ -122,7 +123,7 @@ public class SchemaBuilder {
                           RankProfileRegistry rankProfileRegistry,
                           QueryProfileRegistry queryProfileRegistry,
                           boolean documentsOnly) {
-        this.application = new Application(applicationPackage);
+        this.applicationPackage = applicationPackage;
         this.rankProfileRegistry = rankProfileRegistry;
         this.queryProfileRegistry = queryProfileRegistry;
         this.fileRegistry = fileRegistry;
@@ -139,17 +140,17 @@ public class SchemaBuilder {
      * @throws IOException    thrown if the file can not be read for some reason
      * @throws ParseException thrown if the file does not contain a valid search definition
      */
-    public String importFile(String fileName) throws IOException, ParseException {
+    public Schema addSchemaFile(String fileName) throws IOException, ParseException {
         File file = new File(fileName);
-        return importString(IOUtils.readFile(file), file.getAbsoluteFile().getParent());
+        return addSchema(IOUtils.readFile(file), file.getAbsoluteFile().getParent());
     }
 
-    private String importFile(Path file) throws IOException, ParseException {
-        return importFile(file.toString());
+    private Schema addSchemaFile(Path file) throws IOException, ParseException {
+        return addSchemaFile(file.toString());
     }
 
     public void importFromApplicationPackage() {
-        for (NamedReader reader : application.applicationPackage().getSchemas()) {
+        for (NamedReader reader : applicationPackage.getSchemas()) {
             importFrom(reader);
         }
     }
@@ -158,11 +159,11 @@ public class SchemaBuilder {
      * Reads and parses the schema string provided by the given reader. Once all schemas have been
      * imported, call {@link #build()}.
      *
-     * @param reader       the reader whose content to import
+     * @param reader  the reader whose content to import
      */
     private void importFrom(NamedReader reader) {
         try {
-            String schemaName = importString(IOUtils.readAll(reader), reader.getName());
+            String schemaName = addSchema(IOUtils.readAll(reader), reader.getName()).getName();
             String schemaFileName = stripSuffix(reader.getName(), ApplicationPackage.SD_NAME_SUFFIX);
             if ( ! schemaFileName.equals(schemaName)) {
                 throw new IllegalArgumentException("The file containing schema '" + schemaName + "' must be named '" +
@@ -184,22 +185,24 @@ public class SchemaBuilder {
     }
 
     /**
-     * Import search definition.
+     * Adds a schema to this.
      *
-     * @param str the string to parse
-     * @return the name of the imported object
+     * @param string the string to parse
+     * @return the schema
      * @throws ParseException thrown if the file does not contain a valid search definition
      */
-    public String importString(String str) throws ParseException {
-        return importString(str, null);
+    public Schema addSchema(String string) throws ParseException {
+        return addSchema(string, null);
     }
 
-    private String importString(String str, String schemaDir) throws ParseException {
+    private Schema addSchema(String str, String schemaDir) throws ParseException {
         SimpleCharStream stream = new SimpleCharStream(str);
         try {
-            return importRawSchema(new SDParser(stream, fileRegistry, deployLogger, properties, application,
-                                                rankProfileRegistry, documentsOnly)
-                                           .schema(docTypeMgr, schemaDir));
+            Schema schema = new SDParser(stream, applicationPackage, fileRegistry, deployLogger, properties,
+                                         rankProfileRegistry, documentsOnly)
+                    .schema(documentTypeManager, schemaDir);
+            addSchemaFile(schema);
+            return schema;
         } catch (TokenMgrException e) {
             throw new ParseException("Unknown symbol: " + e.getMessage());
         } catch (ParseException pe) {
@@ -213,15 +216,12 @@ public class SchemaBuilder {
      * programmatically constructed schemas used in unit tests.
      *
      * @param schema the object to import
-     * @return the name of the imported object
      * @throws IllegalArgumentException if the given search object has already been processed
      */
-    public String importRawSchema(Schema schema) {
+    public void addSchemaFile(Schema schema) {
         if (schema.getName() == null)
             throw new IllegalArgumentException("Schema has no name");
-        String rawName = schema.getName();
-        application.add(schema);
-        return rawName;
+        schemas.add(schema);
     }
 
     /**
@@ -240,16 +240,14 @@ public class SchemaBuilder {
      * @throws IllegalStateException thrown if this method has already been called
      */
     public void build(boolean validate) {
-        if (isBuilt) throw new IllegalStateException("Application already built");
+        if (application != null) throw new IllegalStateException("Application already built");
 
-        new TemporarySDTypeResolver(application.schemas().values(), deployLogger).process();
-
-        if (validate)
-            application.validate(deployLogger);
+        application = new Application(applicationPackage, schemas, deployLogger);
+        new TemporarySDTypeResolver(schemas, deployLogger).process();
 
         List<SDDocumentType> sdocs = new ArrayList<>();
         sdocs.add(SDDocumentType.VESPA_DOCUMENT);
-        for (Schema schema : application.schemas().values()) {
+        for (Schema schema : schemas) {
             if (schema.hasDocument()) {
                 sdocs.add(schema.getDocument());
             }
@@ -262,22 +260,21 @@ public class SchemaBuilder {
             new FieldOperationApplier().process(sdoc);
         }
 
-        var resolver = new DocumentReferenceResolver(application.schemas().values());
+        var resolver = new DocumentReferenceResolver(schemas);
         sdocs.forEach(resolver::resolveReferences);
         sdocs.forEach(resolver::resolveInheritedReferences);
-        var importedFieldsEnumerator = new ImportedFieldsEnumerator(application.schemas().values());
+        var importedFieldsEnumerator = new ImportedFieldsEnumerator(schemas);
         sdocs.forEach(importedFieldsEnumerator::enumerateImportedFields);
 
         if (validate)
             new DocumentGraphValidator().validateDocumentGraph(sdocs);
 
-        List<Schema> schemasSomewhatOrdered = new ArrayList<>(application.schemas().values());
+        List<Schema> schemasSomewhatOrdered = new ArrayList<>(schemas);
         for (Schema schema : new SearchOrderer().order(schemasSomewhatOrdered)) {
             new FieldOperationApplierForSearch().process(schema); // TODO: Why is this not in the regular list?
             process(schema, new QueryProfiles(queryProfileRegistry, deployLogger), validate);
         }
         application.buildDocumentModel(schemasSomewhatOrdered);
-        isBuilt = true;
     }
 
     /** Returns a modifiable set of processors we should skip for these schemas. Useful for testing. */
@@ -302,7 +299,7 @@ public class SchemaBuilder {
      * @throws IllegalStateException if there is not exactly one search.
      */
     public Schema getSchema() {
-        if ( ! isBuilt)  throw new IllegalStateException("Application not built.");
+        if (application == null)  throw new IllegalStateException("Application not built");
         if (application.schemas().size() != 1)
             throw new IllegalStateException("This call only works if we have 1 schema. Schemas: " +
                                             application.schemas().values());
@@ -322,7 +319,7 @@ public class SchemaBuilder {
      * @throws IllegalStateException if {@link #build()} has not been called.
      */
     public Schema getSchema(String name) {
-        if ( ! isBuilt)  throw new IllegalStateException("Application not built.");
+        if (application == null)  throw new IllegalStateException("Application not built");
         if (name == null) return getSchema();
         return application.schemas().get(name);
     }
@@ -351,7 +348,7 @@ public class SchemaBuilder {
 
     public static SchemaBuilder createFromString(String sd, DeployLogger logger) throws ParseException {
         SchemaBuilder builder = new SchemaBuilder(logger);
-        builder.importString(sd);
+        builder.addSchema(sd);
         builder.build(true);
         return builder;
     }
@@ -359,7 +356,7 @@ public class SchemaBuilder {
     public static SchemaBuilder createFromStrings(DeployLogger logger, String ... schemas) throws ParseException {
         SchemaBuilder builder = new SchemaBuilder(logger);
         for (var schema : schemas)
-            builder.importString(schema);
+            builder.addSchema(schema);
         builder.build(true);
         return builder;
     }
@@ -427,7 +424,7 @@ public class SchemaBuilder {
                                                   rankProfileRegistry,
                                                   queryprofileRegistry);
         for (String fileName : fileNames) {
-            builder.importFile(fileName);
+            builder.addSchemaFile(fileName);
         }
         builder.build(true);
         return builder;
@@ -468,7 +465,7 @@ public class SchemaBuilder {
                                                   rankProfileRegistry,
                                                   queryProfileRegistry);
         for (Iterator<Path> i = Files.list(new File(dir).toPath()).filter(p -> p.getFileName().toString().endsWith(".sd")).iterator(); i.hasNext(); ) {
-            builder.importFile(i.next());
+            builder.addSchemaFile(i.next());
         }
         builder.build(true);
         return builder;
@@ -533,13 +530,13 @@ public class SchemaBuilder {
      *
      * @param rawSchema the raw object to build from
      * @return the built {@link SchemaBuilder} object
-     * @see #importRawSchema(Schema)
+     * @see #addSchemaFile(Schema)
      */
     public static SchemaBuilder createFromRawSchema(Schema rawSchema,
                                                     RankProfileRegistry rankProfileRegistry,
                                                     QueryProfileRegistry queryProfileRegistry) {
         SchemaBuilder builder = new SchemaBuilder(rankProfileRegistry, queryProfileRegistry);
-        builder.importRawSchema(rawSchema);
+        builder.addSchemaFile(rawSchema);
         builder.build();
         return builder;
     }
@@ -549,7 +546,7 @@ public class SchemaBuilder {
      *
      * @param rawSchema the raw object to build from
      * @return the built {@link Schema} object
-     * @see #importRawSchema(Schema)
+     * @see #addSchemaFile(Schema)
      */
     public static Schema buildFromRawSchema(Schema rawSchema,
                                             RankProfileRegistry rankProfileRegistry,
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java b/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java
index b1fe4942f4a..15cf7c36208 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/derived/Deriver.java
@@ -21,7 +21,7 @@ public class Deriver {
         SchemaBuilder builder = new SchemaBuilder();
         try {
             for (String schema : schemas)
-                builder.importFile(schema);
+                builder.addSchemaFile(schema);
         } catch (ParseException | IOException e) {
             throw new IllegalArgumentException(e);
         }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporaryImportedFields.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporaryImportedFields.java
index 96f2f2f1d24..b4d76445507 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporaryImportedFields.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/TemporaryImportedFields.java
@@ -29,11 +29,7 @@ public class TemporaryImportedFields {
     }
 
     public boolean hasField(String fieldName) {
-        boolean has = fields.get(fieldName) != null;
-        if (has) return true;
-        if (owner.inherited().isEmpty()) return false;
-        if (owner.inherited().get().temporaryImportedFields().isEmpty()) return false;
-        return owner.inherited().get().temporaryImportedFields().get().hasField(fieldName);
+        return fields.get(fieldName) != null;
     }
 
     public Map<String, TemporaryImportedField> fields() {