Merge pull request #18167 from vespa-engine/hmusum/move-sd-files-to-schemas-dir

Move files from searchdefinitions/ to schemas/ when deploying

6 files changed, 177 insertions, 1 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
index ac350db5c21..9a0fd8bf12b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
@@ -36,13 +36,14 @@ import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry;
 import com.yahoo.vespa.config.server.monitoring.MetricUpdater;
 import com.yahoo.vespa.config.server.monitoring.Metrics;
 import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
-import com.yahoo.vespa.config.server.tenant.TenantListener;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.config.server.zookeeper.ConfigCurator;
 import com.yahoo.vespa.config.server.zookeeper.SessionCounter;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.defaults.Defaults;
+import com.yahoo.vespa.flags.BooleanFlag;
 import com.yahoo.vespa.flags.FlagSource;
+import com.yahoo.vespa.flags.Flags;
 import com.yahoo.yolean.Exceptions;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.recipes.cache.ChildData;
@@ -58,6 +59,7 @@ import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -121,6 +123,7 @@ public class SessionRepository {
     private final Zone zone;
     private final ModelFactoryRegistry modelFactoryRegistry;
     private final ConfigDefinitionRepo configDefinitionRepo;
+    private final BooleanFlag rewriteSearchDefinitions;
 
     public SessionRepository(TenantName tenantName,
                              TenantApplications applicationRepo,
@@ -161,6 +164,7 @@ public class SessionRepository {
         this.zone = zone;
         this.modelFactoryRegistry = modelFactoryRegistry;
         this.configDefinitionRepo = configDefinitionRepo;
+        this.rewriteSearchDefinitions = Flags.MOVE_SEARCH_DEFINITIONS_TO_SCHEMAS_DIR.bindTo(flagSource);
 
         loadSessions(); // Needs to be done before creating cache below
         this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, zkCacheExecutor);
@@ -676,6 +680,9 @@ public class SessionRepository {
             tempDestinationDir = Files.createTempDirectory(destinationDir.getParentFile().toPath(), "app-package");
             log.log(Level.FINE, "Copying dir " + sourceDir.getAbsolutePath() + " to " + tempDestinationDir.toFile().getAbsolutePath());
             IOUtils.copyDirectory(sourceDir, tempDestinationDir.toFile());
+            if (rewriteSearchDefinitions.value())
+                moveSearchDefinitionsToSchemasDir(tempDestinationDir);
+
             log.log(Level.FINE, "Moving " + tempDestinationDir + " to " + destinationDir.getAbsolutePath());
             Files.move(tempDestinationDir, destinationDir.toPath(), StandardCopyOption.ATOMIC_MOVE);
         } finally {
@@ -685,6 +692,24 @@ public class SessionRepository {
         }
     }
 
+    // TODO: Remove in Vespa 8 (when we don't allow files in SEARCH_DEFINITIONS_DIR)
+    // Copies schemas from searchdefinitions/ to schemas/ if searchdefinitions/ exists
+    private void moveSearchDefinitionsToSchemasDir(java.nio.file.Path applicationDir) throws IOException {
+        File schemasDir = applicationDir.resolve(ApplicationPackage.SCHEMAS_DIR.getRelative()).toFile();
+        File sdDir = applicationDir.resolve(ApplicationPackage.SEARCH_DEFINITIONS_DIR.getRelative()).toFile();
+        if (sdDir.exists() && sdDir.isDirectory()) {
+            File[] sdFiles = sdDir.listFiles();
+            if (sdFiles != null) {
+                Files.createDirectories(schemasDir.toPath());
+                Arrays.asList(sdFiles).forEach(file -> Exceptions.uncheck(
+                        () -> Files.move(file.toPath(),
+                                         schemasDir.toPath().resolve(file.toPath().getFileName()),
+                                         StandardCopyOption.REPLACE_EXISTING)));
+            }
+            Files.delete(sdDir.toPath());
+        }
+    }
+
     /**
      * Returns a new session instance for the given session id.
      */
diff --git a/configserver/src/test/apps/deprecated-features-app/hosts.xml b/configserver/src/test/apps/deprecated-features-app/hosts.xml
new file mode 100644
index 00000000000..f4256c9fc81
--- /dev/null
+++ b/configserver/src/test/apps/deprecated-features-app/hosts.xml
@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -->
+<hosts>
+  <host name="mytesthost">
+    <alias>node1</alias>
+  </host>
+</hosts>
diff --git a/configserver/src/test/apps/deprecated-features-app/searchdefinitions/music.sd b/configserver/src/test/apps/deprecated-features-app/searchdefinitions/music.sd
new file mode 100644
index 00000000000..a2d4614c657
--- /dev/null
+++ b/configserver/src/test/apps/deprecated-features-app/searchdefinitions/music.sd
@@ -0,0 +1,50 @@
+# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+# A basic search definition - called music, should be saved to music.sd
+search music {
+
+  # It contains one document type only - called music as well
+  document music {
+
+    field title type string {
+      indexing: summary | index   # How this field should be indexed
+      # index-to: title, default    # Create two indexes
+      weight: 75 # Ranking importancy of this field, used by the built in nativeRank feature
+    }
+
+    field artist type string {
+      indexing: summary | attribute | index
+      # index-to: artist, default
+
+      weight: 25
+    }
+
+    field year type int {
+      indexing: summary | attribute
+    }
+
+    # Increase query
+    field popularity type int {
+      indexing: summary | attribute
+    }
+
+    field url type uri {
+      indexing: summary | index
+    }
+
+  }
+
+  rank-profile default inherits default {
+    first-phase {
+      expression: nativeRank(title,artist) + attribute(popularity)
+    }
+
+  }
+
+  rank-profile textmatch inherits default {
+    first-phase {
+      expression: nativeRank(title,artist)
+    }
+
+  }
+
+}
diff --git a/configserver/src/test/apps/deprecated-features-app/services.xml b/configserver/src/test/apps/deprecated-features-app/services.xml
new file mode 100644
index 00000000000..509d7786be0
--- /dev/null
+++ b/configserver/src/test/apps/deprecated-features-app/services.xml
@@ -0,0 +1,38 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<!-- Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -->
+<services version="1.0">
+
+    <admin version="2.0">
+        <adminserver hostalias="node1"/>
+        <logserver hostalias="node1" />
+    </admin>
+
+    <content version="1.0">
+        <redundancy>2</redundancy>
+        <documents>
+            <document type="music" mode="index"/>
+        </documents>
+        <nodes>
+            <node hostalias="node1" distribution-key="0"/>
+        </nodes>
+
+    </content>
+
+    <container version="1.0">
+        <document-processing compressdocuments="true">
+            <chain id="ContainerWrapperTest">
+                <documentprocessor id="com.yahoo.vespa.config.AppleDocProc"/>
+            </chain>
+        </document-processing>
+
+        <config name="project.specific">
+            <value>someval</value>
+        </config>
+
+        <nodes>
+            <node hostalias="node1" />
+        </nodes>
+
+    </container>
+
+</services>
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
index a3025cbf364..31d29e7b78e 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.session;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
+import com.yahoo.config.application.api.ApplicationFile;
 import com.yahoo.config.application.api.ApplicationPackage;
 import com.yahoo.config.model.NullConfigModelRegistry;
 import com.yahoo.config.model.api.Model;
@@ -14,6 +15,8 @@ import com.yahoo.config.model.deploy.DeployState;
 import com.yahoo.config.model.test.MockApplicationPackage;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.TenantName;
+import com.yahoo.io.reader.NamedReader;
+import com.yahoo.path.Path;
 import com.yahoo.text.Utf8;
 import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.MockProvisioner;
@@ -28,6 +31,8 @@ import com.yahoo.vespa.config.server.zookeeper.ConfigCurator;
 import com.yahoo.vespa.config.util.ConfigUtils;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.VespaModelFactory;
 import org.junit.Rule;
@@ -42,15 +47,18 @@ import java.time.LocalDate;
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.LongPredicate;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * @author Ulf Lilleengen
@@ -66,6 +74,7 @@ public class SessionRepositoryTest {
     private TenantRepository tenantRepository;
     private ApplicationRepository applicationRepository;
     private SessionRepository sessionRepository;
+    private final InMemoryFlagSource flagSource = new InMemoryFlagSource();
 
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -86,6 +95,7 @@ public class SessionRepositoryTest {
         tenantRepository = new TestTenantRepository.Builder()
                 .withConfigserverConfig(configserverConfig)
                 .withCurator(curator)
+                .withFlagSource(flagSource)
                 .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig))
                 .withModelFactoryRegistry(modelFactoryRegistry)
                 .build();
@@ -94,6 +104,7 @@ public class SessionRepositoryTest {
                 .withTenantRepository(tenantRepository)
                 .withProvisioner(new MockProvisioner())
                 .withOrchestrator(new OrchestratorMock())
+                .withFlagSource(flagSource)
                 .build();
         sessionRepository = tenantRepository.getTenant(tenantName).getSessionRepository();
     }
@@ -113,6 +124,10 @@ public class SessionRepositoryTest {
         assertEquals(applicationId.application(), applicationSet.getForVersionOrLatest(Optional.empty(), Instant.now()).getId().application());
         assertNotNull(applicationSet.getForVersionOrLatest(Optional.empty(), Instant.now()).getModel());
 
+        LocalSession session = sessionRepository.getLocalSession(secondSessionId);
+        Collection<NamedReader> a = session.applicationPackage.get().getSchemas();
+        assertEquals(1, a.size());
+
         sessionRepository.close();
         // All created sessions are deleted
         assertNull(sessionRepository.getLocalSession(firstSessionId));
@@ -241,6 +256,40 @@ public class SessionRepositoryTest {
         // Does not cause an error because model version 3 is skipped
     }
 
+    @Test
+    public void require_that_searchdefinitions_are_written_to_schemas_dir() throws Exception {
+        setup();
+
+        // App has schemas in searchdefinitions/, should NOT be moved to schemas/ on deploy
+        flagSource.withBooleanFlag(Flags.MOVE_SEARCH_DEFINITIONS_TO_SCHEMAS_DIR.id(), false);
+        long sessionId = deploy(applicationId, new File("src/test/apps/deprecated-features-app"));
+        LocalSession session = sessionRepository.getLocalSession(sessionId);
+
+        assertEquals(1, session.applicationPackage.get().getSchemas().size());
+
+        ApplicationFile schema = getSchema(session, "schemas");
+        assertFalse(schema.exists());
+        ApplicationFile sd = getSchema(session, "searchdefinitions");
+        assertTrue(sd.exists());
+
+
+        // App has schemas in searchdefinitions/, should be moved to schemas/ on deploy
+        flagSource.withBooleanFlag(Flags.MOVE_SEARCH_DEFINITIONS_TO_SCHEMAS_DIR.id(), true);
+        sessionId = deploy(applicationId, new File("src/test/apps/deprecated-features-app"));
+        session = sessionRepository.getLocalSession(sessionId);
+
+        assertEquals(1, session.applicationPackage.get().getSchemas().size());
+
+        schema = getSchema(session, "schemas");
+        assertTrue(schema.exists());
+        sd = getSchema(session, "searchdefinitions");
+        assertFalse(sd.exists());
+    }
+
+    ApplicationFile getSchema(Session session, String subDirectory) {
+        return session.applicationPackage.get().getFile(Path.fromString(subDirectory).append("music.sd"));
+    }
+
     private void createSession(long sessionId, boolean wait) {
         SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator,
                                                                 ConfigCurator.create(curator),
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index 136f64abec6..698ed616804 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -273,6 +273,13 @@ public class Flags {
             "Takes effect on next deployment through controller",
             APPLICATION_ID);
 
+    public static final UnboundBooleanFlag MOVE_SEARCH_DEFINITIONS_TO_SCHEMAS_DIR = defineFeatureFlag(
+            "move-search-definitions-to-schemas-dir", false,
+            List.of("hmusum"), "2021-06-09", "2021-08-09",
+            "Whether to move files in searchdefinitions/ to schemas/ when deploying an application",
+            "Takes effect on next deployment",
+            ZONE_ID, APPLICATION_ID);
+
     /** WARNING: public for testing: All flags should be defined in {@link Flags}. */
     public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners,
                                                        String createdAt, String expiresAt, String description,