From 7a5459da73410e8950ae10cfa2c9ca19ee43a4c7 Mon Sep 17 00:00:00 2001
From: Yuce Tekol <yucetekol@gmail.com>
Date: Sat, 11 Nov 2017 00:49:37 +0300
Subject: [PATCH] input defs: Throw an error if pirmary key is not given
 instead of adding it auto.

---
 docs/input-definition.md |  3 +++
 handler_test.go          | 37 +++++++++++++++++++++++++++++++++++++
 input_definition.go      | 21 +--------------------
 input_definition_test.go |  7 +++++--
 4 files changed, 46 insertions(+), 22 deletions(-)

diff --git a/docs/input-definition.md b/docs/input-definition.md
index f7571b5cc..381388198 100644
--- a/docs/input-definition.md
+++ b/docs/input-definition.md
@@ -43,6 +43,9 @@ curl localhost:10101/index/repository/input-definition/stargazer \
                  }
              ],
              "fields": [
+                 {
+                     "primaryKey": true
+                 },
                  {
                      "actions": [
                          {
diff --git a/handler_test.go b/handler_test.go
index b66cdf830..280467cfc 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1309,6 +1309,43 @@ func TestHandler_DuplicatePrimaryKey(t *testing.T) {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
+	// Ensure throwing error if there's no primary key
+	hldr.MustCreateIndexIfNotExists("i1", pilosa.IndexOptions{ColumnLabel: "id"})
+	unmatchColumnBody := []byte(`
+			{
+			"frames":[{
+				"name":"event-time",
+				"options":{
+					"timeQuantum": "YMD",
+					"inverseEnabled": false,
+					"cacheType": "ranked"
+				}
+			}],
+			"fields": [
+				{
+					"name": "foo",
+					"actions": [
+						{
+							"frame": "cab-type",
+							"valueDestination": "mapping",
+							"valueMap": {
+								"Green": 1,
+								"Yellow": 2
+							}
+						}
+					]
+				}
+			]
+		}`)
+
+	w = httptest.NewRecorder()
+	h.ServeHTTP(w, test.MustNewHTTPRequest("POST", "/index/i1/input-definition/input1", bytes.NewBuffer(unmatchColumnBody)))
+	if w.Code != http.StatusBadRequest {
+		t.Fatalf("unexpected status code: %d", w.Code)
+	} else if body := w.Body.String(); body != pilosa.ErrInputDefinitionHasPrimaryKey.Error()+"\n" {
+		t.Fatalf("unexpected body: %s", body)
+	}
+
 	// Eusure throwing error if request body is invalid.
 	jsonErrorBody := []byte(`
 			{
diff --git a/input_definition.go b/input_definition.go
index 3bb55ba79..0c5088614 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -131,9 +131,7 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error {
 	}
 
 	if len(pb.Fields) > 0 && !primaryKeyGiven {
-		// primary field is required if there are other fields.
-		// add it if it doesn't exist.
-		i.fields = append(i.fields, InputDefDefaultPrimaryKeyField())
+		return ErrInputDefinitionHasPrimaryKey
 	}
 
 	return nil
@@ -335,16 +333,7 @@ func (i *InputDefinitionInfo) Encode() *internal.InputDefinition {
 	for _, f := range i.Frames {
 		def.Frames = append(def.Frames, f.Encode())
 	}
-	primaryKeyGiven := false
 	for _, f := range i.Fields {
-		if f.PrimaryKey {
-			f.Name = DefaultColumnLabel
-			primaryKeyGiven = true
-		}
-		def.Fields = append(def.Fields, f.Encode())
-	}
-	if len(i.Fields) > 0 && !primaryKeyGiven {
-		f := InputDefDefaultPrimaryKeyField()
 		def.Fields = append(def.Fields, f.Encode())
 	}
 	return &def
@@ -402,11 +391,3 @@ func HandleAction(a Action, value interface{}, colID uint64, timestamp int64) (*
 	}
 	return &bit, err
 }
-
-func InputDefDefaultPrimaryKeyField() InputDefinitionField {
-	return InputDefinitionField{
-		Name:       DefaultColumnLabel,
-		PrimaryKey: true,
-		Actions:    []Action{},
-	}
-}
diff --git a/input_definition_test.go b/input_definition_test.go
index 1912043f0..4daf7ce89 100644
--- a/input_definition_test.go
+++ b/input_definition_test.go
@@ -63,6 +63,9 @@ func TestInputDefinition_Encoding(t *testing.T) {
 				}
 			}],
 			"fields": [
+				{
+					"primaryKey": true
+				},
 				{
 					"name": "cabType",
 					"actions": [
@@ -92,9 +95,9 @@ func TestInputDefinition_Encoding(t *testing.T) {
 		t.Fatalf("unexpected frame meta data: %v", internalDef)
 	} else if len(internalDef.Fields) != 2 {
 		t.Fatalf("unexpected number of Fields: %d", len(internalDef.Fields))
-	} else if len(internalDef.Fields[0].InputDefinitionActions) != 1 {
+	} else if len(internalDef.Fields[1].InputDefinitionActions) != 1 {
 		t.Fatalf("unexpected number of Actions: %v", internalDef.Fields[1].InputDefinitionActions)
-	} else if internalDef.Fields[0].InputDefinitionActions[0].ValueDestination != "mapping" {
+	} else if internalDef.Fields[1].InputDefinitionActions[0].ValueDestination != "mapping" {
 		t.Fatalf("unexpected ValueDestination: %v", internalDef.Fields[1].InputDefinitionActions[0])
 	}
 }