From 07555d09df48926e60f1f9a1a72094d3251391b5 Mon Sep 17 00:00:00 2001
From: Aaron Sutula <hi@asutula.com>
Date: Fri, 27 Mar 2020 14:55:01 -0600
Subject: [PATCH] persist all index configs and add tests (#288)

Signed-off-by: Aaron Sutula <hi@asutula.com>
---
 db/collection.go      | 44 ++++++++++++++++++++++++++++++++++++++++---
 db/collection_test.go | 27 ++++++++++++++++++++++++++
 db/db.go              | 30 ++++++++++++-----------------
 3 files changed, 80 insertions(+), 21 deletions(-)

diff --git a/db/collection.go b/db/collection.go
index 31e50e3a..fa968598 100644
--- a/db/collection.go
+++ b/db/collection.go
@@ -93,8 +93,46 @@ func (c *Collection) Indexes() map[string]Index {
 // Adding an index will override any overlapping index values if they already exist.
 // @note: This does NOT currently build the index. If items have been added prior to adding
 // a new index, they will NOT be indexed a posteriori.
-func (c *Collection) AddIndex(path string, unique bool) error {
-	c.indexes[path] = Index{
+func (c *Collection) AddIndex(config IndexConfig) error {
+	indexKey := dsDBIndexes.ChildString(c.name)
+	exists, err := c.db.datastore.Has(indexKey)
+	if err != nil {
+		return err
+	}
+
+	indexes := map[string]IndexConfig{}
+
+	if exists {
+		indexesBytes, err := c.db.datastore.Get(indexKey)
+		if err != nil {
+			return err
+		}
+		if err = json.Unmarshal(indexesBytes, &indexes); err != nil {
+			return err
+		}
+	}
+
+	// if the index being added is for path ID
+	if config.Path == idFieldName {
+		// and there already is and index on ID
+		if _, exists := indexes[idFieldName]; exists {
+			// just return gracefully
+			return nil
+		}
+	}
+
+	indexes[config.Path] = config
+
+	indexBytes, err := json.Marshal(indexes)
+	if err != nil {
+		return err
+	}
+
+	if err := c.db.datastore.Put(indexKey, indexBytes); err != nil {
+		return err
+	}
+
+	c.indexes[config.Path] = Index{
 		IndexFunc: func(field string, value []byte) (ds.Key, error) {
 			result := gjson.GetBytes(value, field)
 			if !result.Exists() {
@@ -102,7 +140,7 @@ func (c *Collection) AddIndex(path string, unique bool) error {
 			}
 			return ds.NewKey(result.String()), nil
 		},
-		Unique: unique,
+		Unique: config.Unique,
 	}
 	return nil
 }
diff --git a/db/collection_test.go b/db/collection_test.go
index 881b52bf..1c707666 100644
--- a/db/collection_test.go
+++ b/db/collection_test.go
@@ -101,6 +101,33 @@ func TestNewCollection(t *testing.T) {
 	})
 }
 
+func TestAddIndex(t *testing.T) {
+	t.Parallel()
+	t.Run("CreateDBAndCollection", func(t *testing.T) {
+		t.Parallel()
+		db, clean := createTestDB(t)
+		defer clean()
+		collection, err := db.NewCollection(CollectionConfig{
+			Name:   "Person",
+			Schema: util.SchemaFromInstance(&Person{}, false),
+		})
+		checkErr(t, err)
+
+		t.Run("AddNameUniqueIndex", func(t *testing.T) {
+			err := collection.AddIndex(IndexConfig{Path: "Name", Unique: true})
+			checkErr(t, err)
+		})
+		t.Run("AddAgeNonUniqueIndex", func(t *testing.T) {
+			err := collection.AddIndex(IndexConfig{Path: "Age", Unique: false})
+			checkErr(t, err)
+		})
+		t.Run("AddIDIndex", func(t *testing.T) {
+			err := collection.AddIndex(IndexConfig{Path: "ID", Unique: true})
+			checkErr(t, err)
+		})
+	})
+}
+
 func TestCreateInstance(t *testing.T) {
 	t.Parallel()
 	t.Run("Single", func(t *testing.T) {
diff --git a/db/db.go b/db/db.go
index 43d964a4..23aecfba 100644
--- a/db/db.go
+++ b/db/db.go
@@ -176,16 +176,21 @@ func (d *DB) reCreateCollections() error {
 			return err
 		}
 
-		var indexes []IndexConfig
+		var indexes map[string]IndexConfig
 		index, err := d.datastore.Get(dsDBIndexes.ChildString(name))
 		if err == nil && index != nil {
 			_ = json.Unmarshal(index, &indexes)
 		}
 
+		indexValues := make([]IndexConfig, len(indexes))
+		for _, value := range indexes {
+			indexValues = append(indexValues, value)
+		}
+
 		if _, err := d.NewCollection(CollectionConfig{
 			Name:    name,
 			Schema:  schema,
-			Indexes: indexes,
+			Indexes: indexValues,
 		}); err != nil {
 			return err
 		}
@@ -228,24 +233,13 @@ func (d *DB) NewCollection(config CollectionConfig) (*Collection, error) {
 		}
 	}
 
-	for _, cfg := range config.Indexes {
-		// @todo: Should check to make sure this is a valid field path for this schema
-		if err := c.AddIndex(cfg.Path, cfg.Unique); err != nil {
-			return nil, err
-		}
-	}
-
-	indexBytes, err := json.Marshal(config.Indexes)
-	if err != nil {
-		return nil, err
-	}
-	indexKey := dsDBIndexes.ChildString(config.Name)
-	exists, err = d.datastore.Has(indexKey)
-	if err != nil {
+	if err := c.AddIndex(IndexConfig{Path: idFieldName, Unique: true}); err != nil {
 		return nil, err
 	}
-	if !exists {
-		if err := d.datastore.Put(indexKey, indexBytes); err != nil {
+
+	for _, cfg := range config.Indexes {
+		// @todo: Should check to make sure this is a valid field path for this schema
+		if err := c.AddIndex(cfg); err != nil {
 			return nil, err
 		}
 	}