Merge pull request #3119 from vespa-engine/geirst/add-imported-attribute-fields-to-attributes-config

Geirst/add imported attribute fields to attributes config

6 files changed, 64 insertions, 49 deletions

diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp
index e6200338047..0347da0c52c 100644
--- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp
+++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp
@@ -30,7 +30,6 @@ using config::IConfigContext;
 using config::InvalidConfigException;
 using proton::matching::IConstantValueRepo;
 using vespa::config::search::AttributesConfig;
-using vespa::config::search::ImportedFieldsConfig;
 using vespa::config::search::IndexschemaConfig;
 using vespa::config::search::RankProfilesConfig;
 using vespa::config::search::core::RankingConstantsConfig;
@@ -54,8 +53,7 @@ public:
     bool verifyConfig(const RankProfilesConfig &rankCfg,
                       const IndexschemaConfig &schemaCfg,
                       const AttributesConfig &attributeCfg,
-                      const RankingConstantsConfig &constantsCfg,
-                      const ImportedFieldsConfig &importedFieldsCfg);
+                      const RankingConstantsConfig &constantsCfg);
 
     int usage();
     int Main() override;
@@ -108,14 +106,12 @@ bool
 App::verifyConfig(const RankProfilesConfig &rankCfg,
                   const IndexschemaConfig &schemaCfg,
                   const AttributesConfig &attributeCfg,
-                  const RankingConstantsConfig &constantsCfg,
-                  const ImportedFieldsConfig &importedFieldsCfg)
+                  const RankingConstantsConfig &constantsCfg)
 {
     bool ok = true;
     search::index::Schema schema;
     search::index::SchemaBuilder::build(schemaCfg, schema);
     search::index::SchemaBuilder::build(attributeCfg, schema);
-    search::index::SchemaBuilder::build(importedFieldsCfg, schema);
     DummyConstantValueRepo repo(constantsCfg);
     for(size_t i = 0; i < rankCfg.rankprofile.size(); i++) {
         search::fef::Properties properties;
@@ -161,14 +157,12 @@ App::Main()
         ConfigHandle<AttributesConfig>::UP attributesHandle = subscriber.subscribe<AttributesConfig>(cfgId);
         ConfigHandle<IndexschemaConfig>::UP schemaHandle = subscriber.subscribe<IndexschemaConfig>(cfgId);
         ConfigHandle<RankingConstantsConfig>::UP constantsHandle = subscriber.subscribe<RankingConstantsConfig>(cfgId);
-        ConfigHandle<ImportedFieldsConfig>::UP importedFieldsHandle = subscriber.subscribe<ImportedFieldsConfig>(cfgId);
 
         subscriber.nextConfig();
         ok = verifyConfig(*rankHandle->getConfig(),
                           *schemaHandle->getConfig(),
                           *attributesHandle->getConfig(),
-                          *constantsHandle->getConfig(),
-                          *importedFieldsHandle->getConfig());
+                          *constantsHandle->getConfig());
     } catch (ConfigRuntimeException & e) {
         LOG(error, "Unable to subscribe to config: %s", e.getMessage().c_str());
     } catch (InvalidConfigException & e) {
diff --git a/searchcore/src/tests/proton/proton_config_fetcher/proton_config_fetcher_test.cpp b/searchcore/src/tests/proton/proton_config_fetcher/proton_config_fetcher_test.cpp
index fb37fa7916a..6c682ea33e9 100644
--- a/searchcore/src/tests/proton/proton_config_fetcher/proton_config_fetcher_test.cpp
+++ b/searchcore/src/tests/proton/proton_config_fetcher/proton_config_fetcher_test.cpp
@@ -240,17 +240,26 @@ TEST_FF("require that documentdb config manager subscribes for config",
     ASSERT_TRUE(f1.configEqual("typea", getDocumentDBConfig(f1, f2)));
 }
 
-TEST_FF("require that documentdb config manager builds schema with imported attribute fields",
+TEST_FF("require that documentdb config manager builds schema with imported attribute fields"
+        " and that they are filtered from resulting attribute config",
         ConfigTestFixture("search"),
         DocumentDBConfigManager(f1.configId + "/typea", "typea"))
 {
     auto *docType = f1.addDocType("typea");
-    docType->importedFieldsBuilder.attribute.resize(1);
-    docType->importedFieldsBuilder.attribute[0].name = "imported";
+    docType->attributesBuilder.attribute.resize(2);
+    docType->attributesBuilder.attribute[0].name = "imported";
+    docType->attributesBuilder.attribute[0].imported = true;
+    docType->attributesBuilder.attribute[1].name = "regular";
 
     const auto &schema = getDocumentDBConfig(f1, f2)->getSchemaSP();
     EXPECT_EQUAL(1u, schema->getNumImportedAttributeFields());
     EXPECT_EQUAL("imported", schema->getImportedAttributeFields()[0].getName());
+    EXPECT_EQUAL(1u, schema->getNumAttributeFields());
+    EXPECT_EQUAL("regular", schema->getAttributeFields()[0].getName());
+
+    const auto &attrCfg = getDocumentDBConfig(f1, f2)->getAttributesConfig();
+    EXPECT_EQUAL(1u, attrCfg.attribute.size());
+    EXPECT_EQUAL("regular", attrCfg.attribute[0].name);
 }
 
 TEST_FFF("require that proton config fetcher follows changes to bootstrap",
diff --git a/searchcore/src/tests/proton/proton_configurer/proton_configurer_test.cpp b/searchcore/src/tests/proton/proton_configurer/proton_configurer_test.cpp
index c3b1247a425..757a9c19f29 100644
--- a/searchcore/src/tests/proton/proton_configurer/proton_configurer_test.cpp
+++ b/searchcore/src/tests/proton/proton_configurer/proton_configurer_test.cpp
@@ -57,7 +57,6 @@ struct DBConfigFixture {
         SchemaBuilder::build(_attributesBuilder, *schema);
         SchemaBuilder::build(_summaryBuilder, *schema);
         SchemaBuilder::build(_indexschemaBuilder, *schema);
-        SchemaBuilder::build(_importedFieldsBuilder, *schema);
         return schema;
     }
 
diff --git a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp
index b15e64ae41f..ca871499151 100644
--- a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp
+++ b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp
@@ -47,13 +47,23 @@ void verify_dir() {
 
 //-----------------------------------------------------------------------------
 
+struct Attribute {
+    std::string dataType;
+    std::string collectionType;
+    std::string imported;
+    Attribute(const std::string &dataType_,
+              const std::string &collectionType_,
+              const std::string &imported_)
+        : dataType(dataType_), collectionType(collectionType_), imported(imported_)
+    {}
+};
+
 struct Model {
     std::map<std::string,std::pair<std::string,std::string> > indexes;
-    std::map<std::string,std::pair<std::string,std::string> > attributes;
+    std::map<std::string,Attribute>                           attributes;
     std::map<std::string,std::string>                         properties;
     std::map<std::string,std::string>                         constants;
     std::vector<bool>                                         extra_profiles;
-    std::vector<std::string>                                  imported_attributes;
     Model();
     ~Model();
     void index(const std::string &name, schema::DataType data_type,
@@ -63,10 +73,11 @@ struct Model {
         indexes[name].second = schema::getTypeName(collection_type);
     }
     void attribute(const std::string &name, schema::DataType data_type,
-                   schema::CollectionType collection_type)
+                   schema::CollectionType collection_type, bool imported = false)
     {
-        attributes[name].first = schema::getTypeName(data_type);
-        attributes[name].second = schema::getTypeName(collection_type);
+        attributes.emplace(name, Attribute(schema::getTypeName(data_type),
+                                           schema::getTypeName(collection_type),
+                                           (imported ? "true" : "false")));
     }
     void property(const std::string &name, const std::string &val) {
         properties[name] = val;
@@ -89,16 +100,14 @@ struct Model {
     void bad_profile() {
         extra_profiles.push_back(false);
     }
-    void imported_attribute(const std::string &name) {
-        imported_attributes.emplace_back(name);
-    }
     void write_attributes(const Writer &out) {
         out.fmt("attribute[%zu]\n", attributes.size());
-        std::map<std::string,std::pair<std::string,std::string> >::const_iterator pos = attributes.begin();
+        auto pos = attributes.begin();
         for (size_t i = 0; pos != attributes.end(); ++pos, ++i) {
             out.fmt("attribute[%zu].name \"%s\"\n", i, pos->first.c_str());
-            out.fmt("attribute[%zu].datatype %s\n", i, pos->second.first.c_str());
-            out.fmt("attribute[%zu].collectiontype %s\n", i, pos->second.second.c_str());
+            out.fmt("attribute[%zu].datatype %s\n", i, pos->second.dataType.c_str());
+            out.fmt("attribute[%zu].collectiontype %s\n", i, pos->second.collectionType.c_str());
+            out.fmt("attribute[%zu].imported %s\n", i, pos->second.imported.c_str());
         }
     }
     void write_indexschema(const Writer &out) {
@@ -134,21 +143,11 @@ struct Model {
             ++idx;
         }
     }
-    void write_imported_attributes(const Writer &out) {
-        size_t idx = 0;
-        for (const auto &attr : imported_attributes) {
-            out.fmt("attribute[%zu].name \"%s\"\n", idx, attr.c_str());
-            out.fmt("attribute[%zu].referencefield \"%s_ref\"\n", idx, attr.c_str());
-            out.fmt("attribute[%zu].targetfield \"%s_target\"\n", idx, attr.c_str());
-            ++idx;
-        }
-    }
     void generate() {
         write_attributes(Writer(gen_dir + "/attributes.cfg"));
         write_indexschema(Writer(gen_dir + "/indexschema.cfg"));
         write_rank_profiles(Writer(gen_dir + "/rank-profiles.cfg"));
         write_ranking_constants(Writer(gen_dir + "/ranking-constants.cfg"));
-        write_imported_attributes(Writer(gen_dir + "/imported-fields.cfg"));
     }
     bool verify() {
         generate();
@@ -192,7 +191,7 @@ struct SimpleModel : Model {
         index("list", DataType::STRING, CollectionType::ARRAY);
         index("keywords", DataType::STRING, CollectionType::WEIGHTEDSET);
         attribute("date", DataType::INT32, CollectionType::SINGLE);
-        imported_attribute("imported_attr");
+        attribute("imported_attr", DataType::INT32, CollectionType::SINGLE, true);
         constants["my_tensor"] = "tensor(x{},y{})";
     }
 };
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp
index 3bcc9ce8ac2..a1cdad5bd22 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp
@@ -52,14 +52,12 @@ namespace {
 Schema::SP
 buildNewSchema(const AttributesConfig &newAttributesConfig,
                const SummaryConfig &newSummaryConfig,
-               const IndexschemaConfig &newIndexschemaConfig,
-               const ImportedFieldsConfig &newImportedFieldsConfig)
+               const IndexschemaConfig &newIndexschemaConfig)
 {
     Schema::SP schema = std::make_shared<Schema>();
     SchemaBuilder::build(newAttributesConfig, *schema);
     SchemaBuilder::build(newSummaryConfig, *schema);
     SchemaBuilder::build(newIndexschemaConfig, *schema);
-    SchemaBuilder::build(newImportedFieldsConfig, *schema);
     return schema;
 }
 
@@ -68,8 +66,7 @@ buildNewSchema(const AttributesConfig &newAttributesConfig,
 Schema::SP
 DocumentDBConfigManager::buildSchema(const AttributesConfig &newAttributesConfig,
                                      const SummaryConfig &newSummaryConfig,
-                                     const IndexschemaConfig &newIndexschemaConfig,
-                                     const ImportedFieldsConfig &newImportedFieldsConfig)
+                                     const IndexschemaConfig &newIndexschemaConfig)
 {
     // Called with lock held
     Schema::SP oldSchema;
@@ -77,16 +74,14 @@ DocumentDBConfigManager::buildSchema(const AttributesConfig &newAttributesConfig
         oldSchema = _pendingConfigSnapshot->getSchemaSP();
     }
     if (oldSchema.get() == NULL) {
-        return buildNewSchema(newAttributesConfig, newSummaryConfig,
-                              newIndexschemaConfig, newImportedFieldsConfig);
+        return buildNewSchema(newAttributesConfig, newSummaryConfig, newIndexschemaConfig);
     }
     const DocumentDBConfig &old = *_pendingConfigSnapshot;
     if (old.getAttributesConfig() != newAttributesConfig ||
         old.getSummaryConfig() != newSummaryConfig ||
         old.getIndexschemaConfig() != newIndexschemaConfig)
     {
-        Schema::SP schema(buildNewSchema(newAttributesConfig, newSummaryConfig,
-                                         newIndexschemaConfig, newImportedFieldsConfig));
+        Schema::SP schema(buildNewSchema(newAttributesConfig, newSummaryConfig, newIndexschemaConfig));
         return (*oldSchema == *schema) ? oldSchema : schema;
     }
     return oldSchema;
@@ -138,6 +133,26 @@ buildMaintenanceConfig(const BootstrapConfig::SP &bootstrapConfig,
                     proton.maintenancejobs.maxoutstandingmoveops));
 }
 
+namespace {
+
+using AttributesConfigSP = DocumentDBConfig::AttributesConfigSP;
+using AttributesConfigBuilder = vespa::config::search::AttributesConfigBuilder;
+using AttributesConfigBuilderSP = std::shared_ptr<AttributesConfigBuilder>;
+
+AttributesConfigSP
+filterImportedAttributes(const AttributesConfigSP &attrCfg)
+{
+    AttributesConfigBuilderSP result = std::make_shared<AttributesConfigBuilder>();
+    result->attribute.reserve(attrCfg->attribute.size());
+    for (const auto &attr : attrCfg->attribute) {
+        if (!attr.imported) {
+            result->attribute.push_back(attr);
+        }
+    }
+    return result;
+}
+
+}
 
 void
 DocumentDBConfigManager::update(const ConfigSnapshot &snapshot)
@@ -259,8 +274,7 @@ DocumentDBConfigManager::update(const ConfigSnapshot &snapshot)
 
     Schema::SP schema(buildSchema(*newAttributesConfig,
                                   *newSummaryConfig,
-                                  *newIndexschemaConfig,
-                                  *newImportedFieldsConfig));
+                                  *newIndexschemaConfig));
     newMaintenanceConfig = buildMaintenanceConfig(_bootstrapConfig,
                                                   _docTypeName);
     if (newMaintenanceConfig.get() != NULL &&
@@ -274,7 +288,7 @@ DocumentDBConfigManager::update(const ConfigSnapshot &snapshot)
                                  newRankProfilesConfig,
                                  newRankingConstants,
                                  newIndexschemaConfig,
-                                 newAttributesConfig,
+                                 filterImportedAttributes(newAttributesConfig),
                                  newSummaryConfig,
                                  newSummarymapConfig,
                                  newJuniperrcConfig,
diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.h b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.h
index 989433248e1..45259fe32a3 100644
--- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.h
+++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.h
@@ -31,8 +31,8 @@ private:
     search::index::Schema::SP
     buildSchema(const DocumentDBConfig::AttributesConfig & newAttributesConfig,
                 const DocumentDBConfig::SummaryConfig & newSummaryConfig,
-                const DocumentDBConfig::IndexschemaConfig & newIndexschemaConfig,
-                const DocumentDBConfig::ImportedFieldsConfig &newImportedFieldsConfig);
+                const DocumentDBConfig::IndexschemaConfig & newIndexschemaConfig);
+
 public:
     DocumentDBConfigManager(const vespalib::string &configId,
                             const vespalib::string &docTypeName);