From e89b11b1fa75501f6cd75ac3fda4a45ba0af7e48 Mon Sep 17 00:00:00 2001
From: mpostma <postma.marin@protonmail.com>
Date: Tue, 9 Feb 2021 11:41:26 +0100
Subject: [PATCH] create IndexSetting struct

need to stabilize the create index trait interface
---
 src/data/mod.rs                               |  9 +++--
 .../local_index_controller/mod.rs             | 11 +++---
 src/index_controller/mod.rs                   | 35 +++++++++++++++++--
 3 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/src/data/mod.rs b/src/data/mod.rs
index 65364df00..8c512895c 100644
--- a/src/data/mod.rs
+++ b/src/data/mod.rs
@@ -9,7 +9,7 @@ use std::sync::Arc;
 
 use sha2::Digest;
 
-use crate::index_controller::{IndexController, LocalIndexController, IndexMetadata, Settings};
+use crate::index_controller::{IndexController, LocalIndexController, IndexMetadata, Settings, IndexSettings};
 use crate::option::Opt;
 
 #[derive(Clone)]
@@ -126,7 +126,12 @@ impl Data {
     }
 
     pub fn create_index(&self, name: impl AsRef<str>, primary_key: Option<impl AsRef<str>>) -> anyhow::Result<IndexMetadata> {
-        let meta = self.index_controller.create_index(name, primary_key)?;
+        let settings = IndexSettings {
+            name: Some(name.as_ref().to_string()),
+            primary_key: primary_key.map(|s| s.as_ref().to_string()),
+        };
+
+        let meta = self.index_controller.create_index(settings)?;
         Ok(meta)
     }
 
diff --git a/src/index_controller/local_index_controller/mod.rs b/src/index_controller/local_index_controller/mod.rs
index 9624a9c64..60a02573f 100644
--- a/src/index_controller/local_index_controller/mod.rs
+++ b/src/index_controller/local_index_controller/mod.rs
@@ -13,7 +13,7 @@ use crate::option::IndexerOpts;
 use index_store::IndexStore;
 use super::IndexController;
 use super::updates::UpdateStatus;
-use super::{UpdateMeta, UpdateResult, IndexMetadata};
+use super::{UpdateMeta, UpdateResult, IndexMetadata, IndexSettings};
 
 pub struct LocalIndexController {
     indexes: IndexStore,
@@ -58,9 +58,10 @@ impl IndexController for LocalIndexController {
         Ok(pending.into())
     }
 
-    fn create_index(&self, index_name: impl AsRef<str>, primary_key: Option<impl AsRef<str>>) -> anyhow::Result<IndexMetadata> {
+    fn create_index(&self, index_settings: IndexSettings) -> anyhow::Result<IndexMetadata> {
+        let index_name = index_settings.name.context("Missing name for index")?;
         let (index, _, meta) = self.indexes.create_index(&index_name, self.update_db_size, self.index_db_size)?;
-        if let Some(ref primary_key) = primary_key {
+        if let Some(ref primary_key) = index_settings.primary_key {
             if let Err(e) = update_primary_key(index, primary_key).context("error creating index") {
                 // TODO: creating index could not be completed, delete everything.
                 Err(e)?
@@ -68,11 +69,11 @@ impl IndexController for LocalIndexController {
         }
 
         let meta = IndexMetadata {
-            name: index_name.as_ref().to_owned(),
+            name: index_name,
             uuid: meta.uuid.clone(),
             created_at: meta.created_at,
             updated_at: meta.created_at,
-            primary_key: primary_key.map(|n| n.as_ref().to_owned()),
+            primary_key: index_settings.primary_key,
         };
 
         Ok(meta)
diff --git a/src/index_controller/mod.rs b/src/index_controller/mod.rs
index d5bf873ee..2c8dd1226 100644
--- a/src/index_controller/mod.rs
+++ b/src/index_controller/mod.rs
@@ -97,6 +97,11 @@ pub enum UpdateResult {
     Other,
 }
 
+pub struct IndexSettings {
+    pub name: Option<String>,
+    pub primary_key: Option<String>,
+}
+
 /// The `IndexController` is in charge of the access to the underlying indices. It splits the logic
 /// for read access which is provided thanks to an handle to the index, and write access which must
 /// be provided. This allows the implementer to define the behaviour of write accesses to the
@@ -127,7 +132,7 @@ pub trait IndexController {
     fn update_settings<S: AsRef<str>>(&self, index_uid: S, settings: Settings) -> anyhow::Result<UpdateStatus>;
 
     /// Create an index with the given `index_uid`.
-    fn create_index(&self, index_uid: impl AsRef<str>, primary_key: Option<impl AsRef<str>>) -> Result<IndexMetadata>;
+    fn create_index(&self, index_settings: IndexSettings) -> Result<IndexMetadata>;
 
     /// Delete index with the given `index_uid`, attempting to close it beforehand.
     fn delete_index<S: AsRef<str>>(&self, index_uid: S) -> Result<()>;
@@ -169,16 +174,40 @@ pub(crate) mod test {
             fn test_create_and_list_indexes() {
                 crate::index_controller::test::create_and_list_indexes($controller_buider);
             }
+
+            #[test]
+            fn test_create_index_with_no_name_is_error() {
+                crate::index_controller::test::create_index_with_no_name_is_error($controller_buider);
+            }
         };
     }
 
     pub(crate) fn create_and_list_indexes<S: IndexController>(controller: S) {
-        controller.create_index("test_index").unwrap();
-        controller.create_index("test_index2").unwrap();
+        let settings1 = IndexSettings {
+            name: Some(String::from("test_index")),
+            primary_key: None,
+        };
+
+        let settings2 = IndexSettings {
+            name: Some(String::from("test_index2")),
+            primary_key: Some(String::from("foo")),
+        };
+
+        controller.create_index(settings1).unwrap();
+        controller.create_index(settings2).unwrap();
 
         let indexes = controller.list_indexes().unwrap();
         assert_eq!(indexes.len(), 2);
         assert_eq!(indexes[0].name, "test_index");
         assert_eq!(indexes[1].name, "test_index2");
+        assert_eq!(indexes[1].primary_key.clone().unwrap(), "foo");
+    }
+
+    pub(crate) fn create_index_with_no_name_is_error<S: IndexController>(controller: S) {
+        let settings = IndexSettings {
+            name: None,
+            primary_key: None,
+        };
+        assert!(controller.create_index(settings).is_err());
     }
 }