From 6028b50b067874a2e117cf708fcc2f5ad1e2eedd Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Wed, 28 Jun 2017 16:33:42 -0500
Subject: [PATCH 01/12] Validate incoming input definitions through the
 InputDefinitionInfo Validate rather than through Encode and piecemeal
 throughout the code.

---
 handler.go               |  28 ++------
 handler_test.go          |   2 +-
 input_definition.go      | 140 +++++++++++++++++++++++++--------------
 input_definition_test.go | 110 ++++++++++++++++++------------
 pilosa.go                |  23 ++++---
 5 files changed, 174 insertions(+), 129 deletions(-)

diff --git a/handler.go b/handler.go
index 7e1ce9066..ed34f19d0 100644
--- a/handler.go
+++ b/handler.go
@@ -1520,34 +1520,16 @@ func (h *Handler) handlePostInputDefinition(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	// TODO: validation before/after encode?
+	// validation definition before/after encode?
+	if err := req.Validate(index.ColumnLabel()); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
 
 	// Encode InputDefinition to its internal representation.
 	def := req.Encode()
-	/*
-		if err != nil {
-			http.Error(w, err.Error(), http.StatusInternalServerError)
-			return
-		}
-	*/
 	def.Name = inputDefName
 
-	// Validate columnLabel and duplicate primaryKey.
-	numPrimaryKey := 0
-	for _, field := range def.Fields {
-		if field.PrimaryKey {
-			numPrimaryKey += 1
-			if field.Name != index.columnLabel {
-				http.Error(w, ErrInputDefinitionColumnLabel.Error(), http.StatusBadRequest)
-				return
-			}
-		}
-	}
-	if numPrimaryKey > 1 {
-		http.Error(w, ErrInputDefinitionPrimaryKey.Error(), http.StatusBadRequest)
-		return
-	}
-
 	// Create InputDefinition.
 	_, err = index.CreateInputDefinition(def)
 	if err == ErrInputDefinitionExists {
diff --git a/handler_test.go b/handler_test.go
index b5cb4b661..e47e04cc0 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1159,7 +1159,7 @@ func TestHandler_DuplicatePrimaryKey(t *testing.T) {
 	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/i0/input-definition/input2", bytes.NewBuffer(invalidPrimaryKey)))
 	if w.Code != http.StatusBadRequest {
 		t.Fatalf("unexpected status code: %d", w.Code)
-	} else if body := w.Body.String(); body != pilosa.ErrInputDefinitionPrimaryKey.Error()+"\n" {
+	} else if body := w.Body.String(); body != pilosa.ErrInputDefinitionDupePrimaryKey.Error()+"\n" {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
diff --git a/input_definition.go b/input_definition.go
index c27b2911d..32e61effa 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -15,13 +15,12 @@
 package pilosa
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 
-	"errors"
-
 	"github.com/gogo/protobuf/proto"
 	"github.com/pilosa/pilosa/internal"
 )
@@ -101,20 +100,9 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error {
 		i.frames = append(i.frames, inputFrame)
 	}
 
-	accountRowID := make(map[string]uint64)
 	for _, field := range pb.Fields {
 		var actions []Action
 		for _, action := range field.InputDefinitionActions {
-			if err := i.ValidateAction(action); err != nil {
-				return err
-			}
-			if action.ValueDestination == InputSingleRowBool && action.Frame != "" {
-				val, ok := accountRowID[action.Frame]
-				if ok && val == action.RowID {
-					return fmt.Errorf("duplicate rowID with other field: %v", action.RowID)
-				}
-				accountRowID[action.Frame] = action.RowID
-			}
 			actions = append(actions, Action{
 				Frame:            action.Frame,
 				ValueDestination: action.ValueDestination,
@@ -209,19 +197,34 @@ type Action struct {
 	RowID            *uint64           `json:"rowID,omitempty"`
 }
 
-// Encode converts Action into its internal representation.
-func (o *Action) Encode() *internal.InputDefinitionAction {
-	// TODO: this check needs to happen somewhere other than Encode()
-	/*
-		if o.RowID == nil && o.ValueDestination == InputSingleRowBool {
-			return nil, errors.New("rowID required for single-row-boolean")
+// Validate ensures the input definition action conforms to our specification.
+func (a *Action) Validate() error {
+	if a.Frame == "" {
+		return ErrFrameRequired
+	}
+	validValues := make(map[string]bool)
+	for _, val := range validValueDestination {
+		validValues[val] = true
+	}
+	if _, ok := validValues[a.ValueDestination]; !ok {
+		return fmt.Errorf("invalid ValueDestination: %s", a.ValueDestination)
+	}
+	switch a.ValueDestination {
+	case InputMapping:
+		if len(a.ValueMap) == 0 {
+			return errors.New("valueMap required for map")
 		}
-	*/
+	}
+	return nil
+}
+
+// Encode converts Action into its internal representation.
+func (a *Action) Encode() *internal.InputDefinitionAction {
 	return &internal.InputDefinitionAction{
-		Frame:            o.Frame,
-		ValueDestination: o.ValueDestination,
-		ValueMap:         o.ValueMap,
-		RowID:            convert(o.RowID),
+		Frame:            a.Frame,
+		ValueDestination: a.ValueDestination,
+		ValueMap:         a.ValueMap,
+		RowID:            convert(a.RowID),
 	}
 }
 
@@ -239,13 +242,21 @@ type InputFrame struct {
 	Options FrameOptions `json:"options,omitempty"`
 }
 
-// Encode converts InputFrame into its internal representation.
-func (f *InputFrame) Encode() *internal.Frame {
-	return &internal.Frame{
-		Name: f.Name,
-		Meta: f.Options.Encode(),
+// Validate the InputFrame data
+func (i *InputFrame) Validate() error {
+	if err := ValidateName(i.Name); err != nil {
+		return err
 	}
 
+	return nil
+}
+
+// Encode converts InputFrame into its internal representation.
+func (i *InputFrame) Encode() *internal.Frame {
+	return &internal.Frame{
+		Name: i.Name,
+		Meta: i.Options.Encode(),
+	}
 }
 
 // InputDefinitionInfo represents the json message format needed to create an InputDefinition.
@@ -254,6 +265,56 @@ type InputDefinitionInfo struct {
 	Fields []InputDefinitionField `json:"fields"`
 }
 
+// Validate the InputDefinitionInfo data
+func (i *InputDefinitionInfo) Validate(columnLabel string) error {
+	numPrimaryKey := 0
+	accountRowID := make(map[string]uint64)
+
+	if len(i.Frames) == 0 {
+		return fmt.Errorf("At least one frame is required per Input Definition")
+	}
+
+	for _, frame := range i.Frames {
+		if err := frame.Validate(); err != nil {
+			return err
+		}
+		// TODO frame option validation
+	}
+
+	// Validate columnLabel and duplicate primaryKey.
+	for _, field := range i.Fields {
+		if field.PrimaryKey {
+			numPrimaryKey++
+			if field.Name != columnLabel {
+				return ErrInputDefinitionColumnLabel
+			}
+		}
+		for _, action := range field.Actions {
+			if err := action.Validate(); err != nil {
+				return err
+			}
+			if action.ValueDestination == InputSingleRowBool && action.Frame != "" {
+				if action.RowID == nil {
+					return fmt.Errorf("rowID required for single-row-boolean Field %s", field.Name)
+				}
+				val, ok := accountRowID[action.Frame]
+				if ok && val == convert(action.RowID) {
+					return fmt.Errorf("duplicate rowID with other field: %v", action.RowID)
+				}
+				accountRowID[action.Frame] = convert(action.RowID)
+			}
+		}
+	}
+	if len(i.Fields) > 0 && numPrimaryKey == 0 {
+		return ErrInputDefinitionHasPrimaryKey
+	}
+	if numPrimaryKey > 1 {
+		return ErrInputDefinitionDupePrimaryKey
+	}
+
+	return nil
+}
+
 // Encode converts InputDefinitionInfo into its internal representation.
 func (i *InputDefinitionInfo) Encode() *internal.InputDefinition {
 	var def internal.InputDefinition
@@ -275,27 +336,6 @@ func (i *InputDefinition) AddFrame(frame InputFrame) error {
 	return nil
 }
 
-// ValidateAction ensures the input definition action conforms to our specification.
-func (i *InputDefinition) ValidateAction(action *internal.InputDefinitionAction) error {
-	if action.Frame == "" {
-		return ErrFrameRequired
-	}
-	validValues := make(map[string]bool)
-	for _, val := range validValueDestination {
-		validValues[val] = true
-	}
-	if _, ok := validValues[action.ValueDestination]; !ok {
-		return fmt.Errorf("invalid ValueDestination: %s", action.ValueDestination)
-	}
-	switch action.ValueDestination {
-	case InputMapping:
-		if len(action.ValueMap) == 0 {
-			return errors.New("valueMap required for map")
-		}
-	}
-	return nil
-}
-
 // HandleAction Process the input data with its action and return a bit to be imported later
 // Note: if the Bit should not be set then nil is returned with no error
 // From the JSON marshalling the possible types are: float64, boolean, string
diff --git a/input_definition_test.go b/input_definition_test.go
index 612dbcbce..6e007fc32 100644
--- a/input_definition_test.go
+++ b/input_definition_test.go
@@ -102,64 +102,86 @@ func TestInputDefinition_Encoding(t *testing.T) {
 	}
 }
 
-func TestInputDefinition_LoadDefinition(t *testing.T) {
-	index := MustOpenIndex()
-	defer index.Close()
+// Test The Action validation cases
+func TestActionValidation(t *testing.T) {
+	rowID := uint64(100)
 
-	// Create Input Definition.
-	input := pilosa.InputDefinition{}
-	frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
-	action := internal.InputDefinitionAction{Frame: "f", ValueDestination: "value-to-ROW", ValueMap: map[string]uint64{"Green": 1}}
-	field := internal.InputDefinitionField{Name: "id", PrimaryKey: true, InputDefinitionActions: []*internal.InputDefinitionAction{&action}}
-	def := &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}}
-	err := input.LoadDefinition(def)
+	action := pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, ValueMap: map[string]uint64{"Green": 1}}
+	field := pilosa.InputDefinitionField{Name: "id", PrimaryKey: false, Actions: []pilosa.Action{action}}
+	info := pilosa.InputDefinitionInfo{Fields: []pilosa.InputDefinitionField{field}}
+	err := info.Validate("id")
+	if !strings.Contains(err.Error(), "one frame is required per Input Definition") {
+		t.Fatalf("Expected frame required error, actual error: %s", err)
+	}
+
+	frame := pilosa.InputFrame{Name: "f", Options: pilosa.FrameOptions{RowLabel: "row"}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("id")
+	if !strings.Contains(err.Error(), "rowID required for single-row-boolean") {
+		t.Fatalf("Expected rowID required for single-row-boolean error, actual error: %s", err)
+	}
+
+	frame = pilosa.InputFrame{Name: "^", Options: pilosa.FrameOptions{RowLabel: "row"}}
+	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
+	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("id")
+	if !strings.Contains(err.Error(), "invalid index or frame's name") {
+		t.Fatalf("Expected iinvalid index or frame's name error, actual error: %s", err)
+	}
+
+	frame = pilosa.InputFrame{Name: "f", Options: pilosa.FrameOptions{RowLabel: "row"}}
+	action = pilosa.Action{ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
+	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("id")
+	if !strings.Contains(err.Error(), "frame required") {
+		t.Fatalf("Expected frame required error, actual error: %s", err)
+	}
+
+	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
+	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("test")
+	if !strings.Contains(err.Error(), "PrimaryKey field name does not match columnLabel") {
+		t.Fatalf("Expected PrimaryKey field name does not match columnLabel error, actual error: %s", err)
+	}
+
+	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
+	field = pilosa.InputDefinitionField{Name: "x", PrimaryKey: false, Actions: []pilosa.Action{action}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("id")
+	if !strings.Contains(err.Error(), "input-definition must contain one PrimaryKey") {
+		t.Fatalf("Expected input-definition must contain one PrimaryKey error, actual error: %s", err)
+	}
+
+	action = pilosa.Action{Frame: "f", ValueDestination: "value-to-ROW", ValueMap: map[string]uint64{"Green": 1}}
+	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("id")
 	if !strings.Contains(err.Error(), "invalid ValueDestination") {
 		t.Fatalf("Expected invalid ValueDestination error, actual error: %s", err)
 	}
 
-	action = internal.InputDefinitionAction{Frame: "f", ValueDestination: pilosa.InputMapping, RowID: 100}
-	field = internal.InputDefinitionField{Name: "id", PrimaryKey: true, InputDefinitionActions: []*internal.InputDefinitionAction{&action}}
-	def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}}
-	err = input.LoadDefinition(def)
+	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputMapping, RowID: &rowID}
+	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
+	err = info.Validate("id")
 	if !strings.Contains(err.Error(), "valueMap required for map") {
 		t.Fatalf("Expected valueMap required for map error, actual error: %s", err)
 	}
 
-	action = internal.InputDefinitionAction{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: 100}
-	action1 := internal.InputDefinitionAction{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: 100}
-	field1 := internal.InputDefinitionField{Name: "id", PrimaryKey: true, InputDefinitionActions: []*internal.InputDefinitionAction{&action1}}
-	def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field, &field1}}
-	err = input.LoadDefinition(def)
+	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
+	field = pilosa.InputDefinitionField{Name: "test", PrimaryKey: false, Actions: []pilosa.Action{action}}
+	action1 := pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
+	field1 := pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action1}}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field, field1}}
+	err = info.Validate("id")
 	if !strings.Contains(err.Error(), "duplicate rowID with other field") {
 		t.Fatalf("Expected duplicate rowID with other field error, actual error: %s", err)
 	}
-
-	action = internal.InputDefinitionAction{ValueDestination: pilosa.InputSingleRowBool, RowID: 100}
-	def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}}
-	err = input.LoadDefinition(def)
-	if !strings.Contains(err.Error(), "frame required") {
-		t.Fatalf("Expected frame required error, actual error: %s", err)
-	}
 }
 
-/*
-// TODO: handle validation outside of the Encode()
-func TestActionEncoding(t *testing.T) {
-	action := pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, ValueMap: map[string]uint64{"Green": 1}}
-	_, err := action.Encode()
-	if !strings.Contains(err.Error(), "rowID required for single-row-boolean") {
-		t.Fatalf("Expected rowID required for single-row-boolean error, actual error: %s", err)
-	}
-
-	field := pilosa.InputDefinitionField{Name: "id", PrimaryKey: false, Actions: []pilosa.Action{action}}
-	info := pilosa.InputDefinitionInfo{Fields: []pilosa.InputDefinitionField{field}}
-	_, err = info.Encode()
-	if !strings.Contains(err.Error(), "rowID required for single-row-boolean") {
-		t.Fatalf("Expected rowID required for single-row-boolean error, actual error: %s", err)
-	}
-}
-*/
-
 func TestHandleAction(t *testing.T) {
 	var value interface{}
 	colID := uint64(0)
diff --git a/pilosa.go b/pilosa.go
index bde8cc2c7..cb650e02d 100644
--- a/pilosa.go
+++ b/pilosa.go
@@ -36,18 +36,19 @@ var (
 	ErrFrameInverseDisabled = errors.New("frame inverse disabled")
 	ErrColumnRowLabelEqual  = errors.New("column and row labels cannot be equal")
 
-	ErrInputDefinitionExists        = errors.New("input-definition already exists")
-	ErrInputDefinitionPrimaryKey    = errors.New("input-definition can only contain one PrimaryKey")
-	ErrInputDefinitionColumnLabel   = errors.New("PrimaryKey field name does not match columnLabel")
-	ErrInputDefinitionNameRequired  = errors.New("input-definition name required")
-	ErrInputDefinitionAttrsRequired = errors.New("frames and fields are required")
+	ErrInputDefinitionExists         = errors.New("input-definition already exists")
+	ErrInputDefinitionHasPrimaryKey  = errors.New("input-definition must contain one PrimaryKey")
+	ErrInputDefinitionDupePrimaryKey = errors.New("input-definition can only contain one PrimaryKey")
+	ErrInputDefinitionColumnLabel    = errors.New("PrimaryKey field name does not match columnLabel")
+	ErrInputDefinitionNameRequired   = errors.New("input-definition name required")
+	ErrInputDefinitionAttrsRequired  = errors.New("frames and fields are required")
 
-	ErrFieldNameRequired      = errors.New("field name required")
-	ErrInvalidFieldType       = errors.New("invalid field type")
-	ErrInvalidFieldRange      = errors.New("invalid field range")
-	ErrInverseRangeNotAllowed = errors.New("inverse range not allowed")
-	ErrRangeCacheNotAllowed   = errors.New("range cache not allowed")
-	ErrFrameFieldsNotAllowed  = errors.New("frame fields not allowed")
+	ErrFieldNameRequired       = errors.New("field name required")
+	ErrInvalidFieldType        = errors.New("invalid field type")
+	ErrInvalidFieldRange       = errors.New("invalid field range")
+	ErrInverseRangeNotAllowed  = errors.New("inverse range not allowed")
+	ErrRangeCacheNotAllowed    = errors.New("range cache not allowed")
+	ErrFrameFieldsNotAllowed   = errors.New("frame fields not allowed")
 	ErrInputDefinitionNotFound = errors.New("input-definition not found")
 
 	ErrInvalidView      = errors.New("invalid view")

From 3e2bbc3717db8af9e7ff92e0c68def18aee6e965 Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 08:50:53 -0500
Subject: [PATCH 02/12] require fields and frames in definition

---
 input_definition.go      | 11 +++++------
 input_definition_test.go |  2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/input_definition.go b/input_definition.go
index 32e61effa..acfe3b40b 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -247,7 +247,7 @@ func (i *InputFrame) Validate() error {
 	if err := ValidateName(i.Name); err != nil {
 		return err
 	}
-
+	// TODO frame option validation
 	return nil
 }
 
@@ -270,15 +270,14 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 	numPrimaryKey := 0
 	accountRowID := make(map[string]uint64)
 
-	if len(i.Frames) == 0 {
-		return fmt.Errorf("At least one frame is required per Input Definition")
+	if len(i.Frames) == 0 || len(i.Fields) == 0 {
+		return ErrInputDefinitionAttrsRequired
 	}
 
 	for _, frame := range i.Frames {
 		if err := frame.Validate(); err != nil {
 			return err
 		}
-		// TODO frame option validation
 	}
 
 	// Validate columnLabel and duplicate primaryKey.
@@ -293,7 +292,7 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 			if err := action.Validate(); err != nil {
 				return err
 			}
-			if action.ValueDestination == InputSingleRowBool && action.Frame != "" {
+			if action.ValueDestination == InputSingleRowBool {
 				if action.RowID == nil {
 					return fmt.Errorf("rowID required for single-row-boolean Field %s", field.Name)
 				}
@@ -305,13 +304,13 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 			}
 		}
 	}
+
 	if len(i.Fields) > 0 && numPrimaryKey == 0 {
 		return ErrInputDefinitionHasPrimaryKey
 	}
 	if numPrimaryKey > 1 {
 		return ErrInputDefinitionDupePrimaryKey
 	}
-
 	return nil
 }
 
diff --git a/input_definition_test.go b/input_definition_test.go
index 6e007fc32..32219b68c 100644
--- a/input_definition_test.go
+++ b/input_definition_test.go
@@ -110,7 +110,7 @@ func TestActionValidation(t *testing.T) {
 	field := pilosa.InputDefinitionField{Name: "id", PrimaryKey: false, Actions: []pilosa.Action{action}}
 	info := pilosa.InputDefinitionInfo{Fields: []pilosa.InputDefinitionField{field}}
 	err := info.Validate("id")
-	if !strings.Contains(err.Error(), "one frame is required per Input Definition") {
+	if err != pilosa.ErrInputDefinitionAttrsRequired {
 		t.Fatalf("Expected frame required error, actual error: %s", err)
 	}
 

From 56f8705407d0f31d46e72017611829dc3cf78e8a Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 08:51:29 -0500
Subject: [PATCH 03/12] return an error if the definition does not exist

---
 index.go      | 20 +++++++++-----------
 index_test.go | 25 ++++++++++++-------------
 2 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/index.go b/index.go
index 62fd87410..f155bc991 100644
--- a/index.go
+++ b/index.go
@@ -349,13 +349,13 @@ func (i *Index) Frame(name string) *Frame {
 }
 
 // InputDefinition returns an input definition in the index by name.
-func (i *Index) InputDefinition(name string) *InputDefinition {
+func (i *Index) InputDefinition(name string) (*InputDefinition, error) {
 	i.mu.Lock()
 	defer i.mu.Unlock()
 	if inputDef, ok := i.inputDefinitions[name]; ok {
-		return inputDef
+		return inputDef, nil
 	}
-	return nil
+	return nil, ErrInputDefinitionNotFound
 }
 
 func (i *Index) frame(name string) *Frame { return i.frames[name] }
@@ -657,8 +657,6 @@ func (i *Index) CreateInputDefinition(pb *internal.InputDefinition) (*InputDefin
 func (i *Index) createInputDefinition(pb *internal.InputDefinition) (*InputDefinition, error) {
 	if pb.Name == "" {
 		return nil, ErrInputDefinitionNameRequired
-	} else if len(pb.Frames) == 0 || len(pb.Fields) == 0 {
-		return nil, ErrInputDefinitionAttrsRequired
 	}
 
 	for _, fr := range pb.Frames {
@@ -704,15 +702,15 @@ func (i *Index) newInputDefinition(name string) (*InputDefinition, error) {
 
 // DeleteInputDefinition removes an input definition from the index.
 func (i *Index) DeleteInputDefinition(name string) error {
+	// Fail if input definition doesn't exist.
+	_, err := i.InputDefinition(name)
+	if err != nil {
+		return err
+	}
+
 	i.mu.Lock()
 	defer i.mu.Unlock()
 
-	// Ignore if input definition doesn't exist.
-	inputDef := i.inputDefinition(name)
-	if inputDef == nil {
-		return nil
-	}
-
 	// Delete input definition file.
 	if err := os.Remove(filepath.Join(i.InputDefinitionPath(), name)); err != nil {
 		return err
diff --git a/index_test.go b/index_test.go
index aaebe288d..9a3f4133c 100644
--- a/index_test.go
+++ b/index_test.go
@@ -384,16 +384,9 @@ func TestIndex_CreateExistingInputDefinition(t *testing.T) {
 	index := MustOpenIndex()
 	defer index.Close()
 
-	// Test frames and fields are required
-	def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{}, Fields: []*internal.InputDefinitionField{}}
-	_, err := index.CreateInputDefinition(&def)
-	if err != pilosa.ErrInputDefinitionAttrsRequired {
-		t.Fatal(err)
-	}
-
 	//Test input definition name is required
-	def = internal.InputDefinition{Name: "", Frames: []*internal.Frame{}, Fields: []*internal.InputDefinitionField{}}
-	_, err = index.CreateInputDefinition(&def)
+	def := internal.InputDefinition{Name: "", Frames: []*internal.Frame{}, Fields: []*internal.InputDefinitionField{}}
+	_, err := index.CreateInputDefinition(&def)
 	if err != pilosa.ErrInputDefinitionNameRequired {
 		t.Fatal(err)
 	}
@@ -426,15 +419,21 @@ func TestIndex_DeleteInputDefinition(t *testing.T) {
 	_, err := index.CreateInputDefinition(&def)
 	if err != nil {
 		t.Fatal(err)
-	} else if index.InputDefinition("test") == nil {
-		t.Fatal("No input definition created")
+	}
+
+	_, err = index.InputDefinition("test")
+	if err != nil {
+		t.Fatal(err)
 	}
 
 	err = index.DeleteInputDefinition("test")
 	if err != nil {
 		t.Fatal(err)
-	} else if index.InputDefinition("test") != nil {
-		t.Fatal("input definition isn't deleted")
+	}
+
+	_, err = index.InputDefinition("test")
+	if err != pilosa.ErrInputDefinitionNotFound {
+		t.Fatal(err)
 	}
 }
 

From 8e8796993334dd8f3ee3ff4eee4d2e8a5fdca9a7 Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 08:52:18 -0500
Subject: [PATCH 04/12] Input definition GET and DELETE handle the case when
 the definition does not exist

---
 handler.go      | 21 +++++++++++++--------
 handler_test.go | 13 +++++++++++--
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/handler.go b/handler.go
index ed34f19d0..63ecffac0 100644
--- a/handler.go
+++ b/handler.go
@@ -1566,8 +1566,13 @@ func (h *Handler) handleGetInputDefinition(w http.ResponseWriter, r *http.Reques
 		return
 	}
 
-	inputDef, _ := index.inputDefinitions[inputDefName]
-	if err := json.NewEncoder(w).Encode(InputDefinitionInfo{
+	inputDef, err := index.InputDefinition(inputDefName)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusNotFound)
+		return
+	}
+
+	if err = json.NewEncoder(w).Encode(InputDefinitionInfo{
 		Frames: inputDef.frames,
 		Fields: inputDef.fields,
 	}); err != nil {
@@ -1629,7 +1634,7 @@ func (h *Handler) handlePostInput(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	for _, req := range reqs {
-		bits, err := h.InputJsonDataParser(req.(map[string]interface{}), index, inputDefName)
+		bits, err := h.InputJSONDataParser(req.(map[string]interface{}), index, inputDefName)
 		if err == ErrInputDefinitionNotFound {
 			http.Error(w, err.Error(), http.StatusNotFound)
 			return
@@ -1650,11 +1655,11 @@ func (h *Handler) handlePostInput(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// InputJsonDataParser validate input json file and execute SetBit
-func (h *Handler) InputJsonDataParser(req map[string]interface{}, index *Index, name string) (map[string][]*Bit, error) {
-	inputDef := index.inputDefinition(name)
-	if inputDef == nil {
-		return nil, ErrInputDefinitionNotFound
+// InputJSONDataParser validate input json file and execute SetBit
+func (h *Handler) InputJSONDataParser(req map[string]interface{}, index *Index, name string) (map[string][]*Bit, error) {
+	inputDef, err := index.InputDefinition(name)
+	if err != nil {
+		return nil, err
 	}
 	// if field in input data is not in defined definition, return error
 	var columnLabel string
diff --git a/handler_test.go b/handler_test.go
index e47e04cc0..a88ab1f64 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1253,8 +1253,10 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected status code: %d", w.Code)
 	} else if body := w.Body.String(); body != `{}`+"\n" {
 		t.Fatalf("unexpected body: %s", body)
-	} else if index.InputDefinition("test") != nil {
-		t.Fatalf("unexpected result: %v", index.InputDefinition("test"))
+	}
+	_, err = index.InputDefinition("test")
+	if err != pilosa.ErrInputDefinitionNotFound {
+		t.Fatal(err)
 	}
 }
 
@@ -1299,6 +1301,13 @@ func TestHandler_GetInputDefinition(t *testing.T) {
 	} else if body := w.Body.String(); body != string(expect)+"\n" {
 		t.Fatalf("unexpected body: %s, expect: %s", body, string(expect))
 	}
+
+	// Check non existant definition
+	w = httptest.NewRecorder()
+	h.ServeHTTP(w, MustNewHTTPRequest("GET", "/index/i0/input-definition/foo", strings.NewReader("")))
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("unexpected status code: %d", w.Code)
+	}
 }
 
 var defaultBody = `

From 681c48fd12cd3bf2f61227ac3543890c7279936e Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 09:31:21 -0500
Subject: [PATCH 05/12] add more tests for index and input-definition not found

---
 handler.go      |  2 +-
 handler_test.go | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/handler.go b/handler.go
index 63ecffac0..88f1e3fa6 100644
--- a/handler.go
+++ b/handler.go
@@ -1595,7 +1595,7 @@ func (h *Handler) handleDeleteInputDefinition(w http.ResponseWriter, r *http.Req
 
 	// Delete input definition from the index.
 	if err := index.DeleteInputDefinition(inputDefName); err != nil {
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		http.Error(w, err.Error(), http.StatusNotFound)
 		return
 	}
 
diff --git a/handler_test.go b/handler_test.go
index a88ab1f64..a0654adb4 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1121,6 +1121,15 @@ func TestHandler_CreateInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
+	// Test index not found
+	w = httptest.NewRecorder()
+	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/foo/input-definition/input2", bytes.NewBuffer(inputBody)))
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("unexpected status code: %d", w.Code)
+	} else if body := w.Body.String(); body != pilosa.ErrIndexNotFound.Error()+"\n" {
+		t.Fatalf("unexpected body: %s", body)
+	}
+
 }
 
 // Ensure throwing error if there's duplicated primaryKey field.
@@ -1247,6 +1256,14 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	// Test definition not found
+	w = httptest.NewRecorder()
+	h.ServeHTTP(w, MustNewHTTPRequest("DELETE", "/index/i0/input-definition/foo", strings.NewReader("")))
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("unexpected status code: %d", w.Code)
+	}
+
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("DELETE", "/index/i0/input-definition/test", strings.NewReader("")))
 	if w.Code != http.StatusOK {
@@ -1402,7 +1419,24 @@ func TestHandler_CreateInput(t *testing.T) {
 	h := NewHandler()
 	h.Holder = hldr.Holder
 	h.Cluster = NewCluster(1)
+
+	// Return error if index does not exist
 	w := httptest.NewRecorder()
+	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/foo/input/input1", bytes.NewBuffer(inputBody)))
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("unexpected status code: %d", w.Code)
+	} else if body := w.Body.String(); body != pilosa.ErrIndexNotFound.Error()+"\n" {
+		t.Fatalf("unexpected body: %s, expect: %s", body, pilosa.ErrIndexNotFound)
+	}
+
+	// Check non existant definition
+	w = httptest.NewRecorder()
+	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/i0/input/input2", bytes.NewBuffer(inputBody)))
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("unexpected status code: %d", w.Code)
+	}
+
+	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/i0/input/input1", bytes.NewBuffer(inputBody)))
 	if w.Code != http.StatusOK {
 		t.Fatalf("unexpected status code: %d", w.Code)

From a80be54a60e0db6ffddbe8362c97fbfd324bd512 Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 10:13:47 -0500
Subject: [PATCH 06/12] validate missing frame from input data

---
 handler_test.go | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/handler_test.go b/handler_test.go
index a0654adb4..33c537fa8 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1389,6 +1389,16 @@ var defaultBody = `
 						   "frame":"distance-miles",
 						   "valueDestination":"value-to-row"
 
+						}
+					 ]
+				  },
+				  {
+					 "name":"noFrame",
+					 "actions":[
+						{
+						   "frame":"foo",
+						   "valueDestination":"value-to-row"
+
 						}
 					 ]
 				  }
@@ -1509,6 +1519,20 @@ func TestInput_JSON(t *testing.T) {
 				"withPet": true
 				}]`,
 			err: "columnLabel required"},
+		{json: `[{
+				"id": 1,
+				"cabType": "yellow",
+				"distanceMiles": 8,
+				"withPet": true
+				}`,
+			err: "unexpected EOF"},
+		{json: `[{
+				"id": 1,
+				"cabType": "yellow",
+				"distanceMiles": 8,
+				"noFrame": 1
+				}]`,
+			err: "Frame not found: foo"},
 	}
 	h := NewHandler()
 	h.Holder = hldr.Holder

From c47fffebd36d6e7d30279787323904c8214d8b12 Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 11:11:53 -0500
Subject: [PATCH 07/12] find string in slice

---
 input_definition.go | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/input_definition.go b/input_definition.go
index acfe3b40b..5f748f571 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -202,13 +202,10 @@ func (a *Action) Validate() error {
 	if a.Frame == "" {
 		return ErrFrameRequired
 	}
-	validValues := make(map[string]bool)
-	for _, val := range validValueDestination {
-		validValues[val] = true
-	}
-	if _, ok := validValues[a.ValueDestination]; !ok {
+	if !foundItem(validValueDestination, a.ValueDestination) {
 		return fmt.Errorf("invalid ValueDestination: %s", a.ValueDestination)
 	}
+
 	switch a.ValueDestination {
 	case InputMapping:
 		if len(a.ValueMap) == 0 {
@@ -228,7 +225,7 @@ func (a *Action) Encode() *internal.InputDefinitionAction {
 	}
 }
 
-// convert pointer to uint64
+// convert pointer to uint64.
 func convert(x *uint64) uint64 {
 	if x != nil {
 		return *x
@@ -242,7 +239,7 @@ type InputFrame struct {
 	Options FrameOptions `json:"options,omitempty"`
 }
 
-// Validate the InputFrame data
+// Validate the InputFrame data.
 func (i *InputFrame) Validate() error {
 	if err := ValidateName(i.Name); err != nil {
 		return err
@@ -265,7 +262,7 @@ type InputDefinitionInfo struct {
 	Fields []InputDefinitionField `json:"fields"`
 }
 
-// Validate the InputDefinitionInfo data
+// Validate the InputDefinitionInfo data.
 func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 	numPrimaryKey := 0
 	accountRowID := make(map[string]uint64)

From 57a67f16ed6bbdc853278fe244bfbd0c90dd0b15 Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 11:12:06 -0500
Subject: [PATCH 08/12] error cleanup

---
 handler.go               |  2 +-
 handler_test.go          | 30 +++++++++++++++---------------
 input_definition_test.go | 18 +++++++++---------
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/handler.go b/handler.go
index 88f1e3fa6..4fabdf911 100644
--- a/handler.go
+++ b/handler.go
@@ -1655,7 +1655,7 @@ func (h *Handler) handlePostInput(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
-// InputJSONDataParser validate input json file and execute SetBit
+// InputJSONDataParser validates input json file and executes SetBit.
 func (h *Handler) InputJSONDataParser(req map[string]interface{}, index *Index, name string) (map[string][]*Bit, error) {
 	inputDef, err := index.InputDefinition(name)
 	if err != nil {
diff --git a/handler_test.go b/handler_test.go
index 33c537fa8..8cc734b23 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1121,7 +1121,7 @@ func TestHandler_CreateInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
-	// Test index not found
+	// Test index not found.
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/foo/input-definition/input2", bytes.NewBuffer(inputBody)))
 	if w.Code != http.StatusNotFound {
@@ -1200,7 +1200,7 @@ func TestHandler_DuplicatePrimaryKey(t *testing.T) {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
-	// Eusure throwing error if request body is invalid
+	// Eusure throwing error if request body is invalid.
 	jsonErrorBody := []byte(`
 			{
 			"frames":[{
@@ -1237,7 +1237,7 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
 	h.Holder = hldr.Holder
 	h.Cluster = NewCluster(1)
 
-	// Test index not found
+	// Test index not found.
 	w := httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("DELETE", "/index/i0/input-definition/test", strings.NewReader("")))
 	if w.Code != http.StatusNotFound {
@@ -1246,7 +1246,7 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
-	// Test input definition is deleted
+	// Test input definition is deleted.
 	index := hldr.MustCreateIndexIfNotExists("i0", pilosa.IndexOptions{})
 	frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
 	action := internal.InputDefinitionAction{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
@@ -1257,7 +1257,7 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Test definition not found
+	// Test definition not found.
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("DELETE", "/index/i0/input-definition/foo", strings.NewReader("")))
 	if w.Code != http.StatusNotFound {
@@ -1277,7 +1277,7 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
 	}
 }
 
-// Ensure handler can get existing input definition
+// Ensure handler can get existing input definition.
 func TestHandler_GetInputDefinition(t *testing.T) {
 	hldr := MustOpenHolder()
 	defer hldr.Close()
@@ -1290,7 +1290,7 @@ func TestHandler_GetInputDefinition(t *testing.T) {
 	fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, InputDefinitionActions: []*internal.InputDefinitionAction{&action}}
 	def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
 
-	// Return error if index does not exist
+	// Return error if index does not exist.
 	w := httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("GET", "/index/i0/input-definition/test", strings.NewReader("")))
 	if w.Code != http.StatusNotFound {
@@ -1299,7 +1299,7 @@ func TestHandler_GetInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected body: %s, expect: %s", body, pilosa.ErrIndexNotFound)
 	}
 
-	// Return existing input definition
+	// Return existing input definition.
 	index := hldr.MustCreateIndexIfNotExists("i0", pilosa.IndexOptions{})
 	inputDef, err := index.CreateInputDefinition(&def)
 	if err != nil {
@@ -1319,7 +1319,7 @@ func TestHandler_GetInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected body: %s, expect: %s", body, string(expect))
 	}
 
-	// Check non existant definition
+	// Check nonexistant definition.
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("GET", "/index/i0/input-definition/foo", strings.NewReader("")))
 	if w.Code != http.StatusNotFound {
@@ -1430,7 +1430,7 @@ func TestHandler_CreateInput(t *testing.T) {
 	h.Holder = hldr.Holder
 	h.Cluster = NewCluster(1)
 
-	// Return error if index does not exist
+	// Return error if index does not exist.
 	w := httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/foo/input/input1", bytes.NewBuffer(inputBody)))
 	if w.Code != http.StatusNotFound {
@@ -1439,7 +1439,7 @@ func TestHandler_CreateInput(t *testing.T) {
 		t.Fatalf("unexpected body: %s, expect: %s", body, pilosa.ErrIndexNotFound)
 	}
 
-	// Check non existant definition
+	// Check nonexistant definition.
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/i0/input/input2", bytes.NewBuffer(inputBody)))
 	if w.Code != http.StatusNotFound {
@@ -1454,13 +1454,13 @@ func TestHandler_CreateInput(t *testing.T) {
 		t.Fatalf("unexpected body: %s", body)
 	}
 
-	// Verify the bits set per frame
+	// Verify the bits set per frame.
 	// f := index.Frame("cab-type")
 	f0 := index.Frame("distance-miles")
 	v0 := f0.View(pilosa.ViewStandard)
 	fragment0 := v0.Fragment(0)
 
-	// Verify the distanceMiles Bit was set
+	// Verify the distanceMiles Bit was set.
 	if a := fragment0.Row(8).Bits(); !reflect.DeepEqual(a, []uint64{1}) {
 		t.Fatalf("unexpected bits: %+v", a)
 	}
@@ -1469,12 +1469,12 @@ func TestHandler_CreateInput(t *testing.T) {
 	v1 := f1.View(pilosa.ViewStandard)
 	fragment1 := v1.Fragment(0)
 
-	// Verify the add-ons frame does not have a distanceMiles Bit set
+	// Verify the add-ons frame does not have a distanceMiles Bit set.
 	// The Input process must respect the Action Frame assignments
 	if a := fragment1.Row(8).Bits(); !reflect.DeepEqual(a, []uint64{}) {
 		t.Fatalf("unexpected bits: %+v", a)
 	}
-	// Verify the withPet Bit was set
+	// Verify the withPet Bit was set.
 	if a := fragment1.Row(100).Bits(); !reflect.DeepEqual(a, []uint64{1}) {
 		t.Fatalf("unexpected bits: %+v", a)
 	}
diff --git a/input_definition_test.go b/input_definition_test.go
index 32219b68c..68ac11593 100644
--- a/input_definition_test.go
+++ b/input_definition_test.go
@@ -111,7 +111,7 @@ func TestActionValidation(t *testing.T) {
 	info := pilosa.InputDefinitionInfo{Fields: []pilosa.InputDefinitionField{field}}
 	err := info.Validate("id")
 	if err != pilosa.ErrInputDefinitionAttrsRequired {
-		t.Fatalf("Expected frame required error, actual error: %s", err)
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionAttrsRequired, err)
 	}
 
 	frame := pilosa.InputFrame{Name: "f", Options: pilosa.FrameOptions{RowLabel: "row"}}
@@ -126,8 +126,8 @@ func TestActionValidation(t *testing.T) {
 	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
 	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
 	err = info.Validate("id")
-	if !strings.Contains(err.Error(), "invalid index or frame's name") {
-		t.Fatalf("Expected iinvalid index or frame's name error, actual error: %s", err)
+	if err != pilosa.ErrName {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrName, err)
 	}
 
 	frame = pilosa.InputFrame{Name: "f", Options: pilosa.FrameOptions{RowLabel: "row"}}
@@ -135,24 +135,24 @@ func TestActionValidation(t *testing.T) {
 	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
 	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
 	err = info.Validate("id")
-	if !strings.Contains(err.Error(), "frame required") {
-		t.Fatalf("Expected frame required error, actual error: %s", err)
+	if err != pilosa.ErrFrameRequired {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrFrameRequired, err)
 	}
 
 	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
 	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
 	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
 	err = info.Validate("test")
-	if !strings.Contains(err.Error(), "PrimaryKey field name does not match columnLabel") {
-		t.Fatalf("Expected PrimaryKey field name does not match columnLabel error, actual error: %s", err)
+	if err != pilosa.ErrInputDefinitionColumnLabel {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionColumnLabel, err)
 	}
 
 	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
 	field = pilosa.InputDefinitionField{Name: "x", PrimaryKey: false, Actions: []pilosa.Action{action}}
 	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
 	err = info.Validate("id")
-	if !strings.Contains(err.Error(), "input-definition must contain one PrimaryKey") {
-		t.Fatalf("Expected input-definition must contain one PrimaryKey error, actual error: %s", err)
+	if err != pilosa.ErrInputDefinitionHasPrimaryKey {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionHasPrimaryKey, err)
 	}
 
 	action = pilosa.Action{Frame: "f", ValueDestination: "value-to-ROW", ValueMap: map[string]uint64{"Green": 1}}

From 7abab7b795106cea29d7d76fa5be1b35e4cbafcc Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 11:23:36 -0500
Subject: [PATCH 09/12] Error ErrInputDefinitionValueMap

---
 input_definition.go      | 3 +--
 input_definition_test.go | 4 ++--
 pilosa.go                | 1 +
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/input_definition.go b/input_definition.go
index 5f748f571..c0f468b55 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -15,7 +15,6 @@
 package pilosa
 
 import (
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -209,7 +208,7 @@ func (a *Action) Validate() error {
 	switch a.ValueDestination {
 	case InputMapping:
 		if len(a.ValueMap) == 0 {
-			return errors.New("valueMap required for map")
+			return ErrInputDefinitionValueMap
 		}
 	}
 	return nil
diff --git a/input_definition_test.go b/input_definition_test.go
index 68ac11593..c395b0630 100644
--- a/input_definition_test.go
+++ b/input_definition_test.go
@@ -167,8 +167,8 @@ func TestActionValidation(t *testing.T) {
 	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []pilosa.Action{action}}
 	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field}}
 	err = info.Validate("id")
-	if !strings.Contains(err.Error(), "valueMap required for map") {
-		t.Fatalf("Expected valueMap required for map error, actual error: %s", err)
+	if err != pilosa.ErrInputDefinitionValueMap {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionValueMap, err)
 	}
 
 	action = pilosa.Action{Frame: "f", ValueDestination: pilosa.InputSingleRowBool, RowID: &rowID}
diff --git a/pilosa.go b/pilosa.go
index cb650e02d..228a27fd2 100644
--- a/pilosa.go
+++ b/pilosa.go
@@ -42,6 +42,7 @@ var (
 	ErrInputDefinitionColumnLabel    = errors.New("PrimaryKey field name does not match columnLabel")
 	ErrInputDefinitionNameRequired   = errors.New("input-definition name required")
 	ErrInputDefinitionAttrsRequired  = errors.New("frames and fields are required")
+	ErrInputDefinitionValueMap       = errors.New("valueMap required for map")
 
 	ErrFieldNameRequired       = errors.New("field name required")
 	ErrInvalidFieldType        = errors.New("invalid field type")

From fb7e1091b5e2508a477e2d5356f3890602e88251 Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 13:16:05 -0500
Subject: [PATCH 10/12] field definitions require an action

---
 handler_test.go          |  4 ++--
 input_definition.go      | 16 ++++++++++------
 input_definition_test.go |  8 ++++++++
 pilosa.go                |  1 +
 4 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/handler_test.go b/handler_test.go
index 8cc734b23..853d05cd0 100644
--- a/handler_test.go
+++ b/handler_test.go
@@ -1319,7 +1319,7 @@ func TestHandler_GetInputDefinition(t *testing.T) {
 		t.Fatalf("unexpected body: %s, expect: %s", body, string(expect))
 	}
 
-	// Check nonexistant definition.
+	// Check nonexistent definition.
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("GET", "/index/i0/input-definition/foo", strings.NewReader("")))
 	if w.Code != http.StatusNotFound {
@@ -1439,7 +1439,7 @@ func TestHandler_CreateInput(t *testing.T) {
 		t.Fatalf("unexpected body: %s, expect: %s", body, pilosa.ErrIndexNotFound)
 	}
 
-	// Check nonexistant definition.
+	// Check nonexistent definition.
 	w = httptest.NewRecorder()
 	h.ServeHTTP(w, MustNewHTTPRequest("POST", "/index/i0/input/input2", bytes.NewBuffer(inputBody)))
 	if w.Code != http.StatusNotFound {
diff --git a/input_definition.go b/input_definition.go
index c0f468b55..0e5375507 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -278,13 +278,9 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 
 	// Validate columnLabel and duplicate primaryKey.
 	for _, field := range i.Fields {
-		if field.PrimaryKey {
-			numPrimaryKey++
-			if field.Name != columnLabel {
-				return ErrInputDefinitionColumnLabel
-			}
-		}
+		var actionCount int
 		for _, action := range field.Actions {
+			actionCount++
 			if err := action.Validate(); err != nil {
 				return err
 			}
@@ -299,6 +295,14 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 				accountRowID[action.Frame] = convert(action.RowID)
 			}
 		}
+		if field.PrimaryKey {
+			numPrimaryKey++
+			if field.Name != columnLabel {
+				return ErrInputDefinitionColumnLabel
+			}
+		} else if actionCount == 0 {
+			return ErrInputDefinitionActionRequired
+		}
 	}
 
 	if len(i.Fields) > 0 && numPrimaryKey == 0 {
diff --git a/input_definition_test.go b/input_definition_test.go
index c395b0630..9a7b6f706 100644
--- a/input_definition_test.go
+++ b/input_definition_test.go
@@ -180,6 +180,14 @@ func TestActionValidation(t *testing.T) {
 	if !strings.Contains(err.Error(), "duplicate rowID with other field") {
 		t.Fatalf("Expected duplicate rowID with other field error, actual error: %s", err)
 	}
+
+	field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true}
+	field1 = pilosa.InputDefinitionField{Name: "test", PrimaryKey: false}
+	info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field, field1}}
+	err = info.Validate("id")
+	if err != pilosa.ErrInputDefinitionActionRequired {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionActionRequired, err)
+	}
 }
 
 func TestHandleAction(t *testing.T) {
diff --git a/pilosa.go b/pilosa.go
index 228a27fd2..943b67a3a 100644
--- a/pilosa.go
+++ b/pilosa.go
@@ -43,6 +43,7 @@ var (
 	ErrInputDefinitionNameRequired   = errors.New("input-definition name required")
 	ErrInputDefinitionAttrsRequired  = errors.New("frames and fields are required")
 	ErrInputDefinitionValueMap       = errors.New("valueMap required for map")
+	ErrInputDefinitionActionRequired = errors.New("field definitions require an action")
 
 	ErrFieldNameRequired       = errors.New("field name required")
 	ErrInvalidFieldType        = errors.New("invalid field type")

From 64ed6f78153ae0d3eac0ccc8613af02ff18b536b Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 13:36:04 -0500
Subject: [PATCH 11/12] action length test

---
 input_definition.go | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/input_definition.go b/input_definition.go
index 0e5375507..d9cc543b5 100644
--- a/input_definition.go
+++ b/input_definition.go
@@ -278,9 +278,7 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 
 	// Validate columnLabel and duplicate primaryKey.
 	for _, field := range i.Fields {
-		var actionCount int
 		for _, action := range field.Actions {
-			actionCount++
 			if err := action.Validate(); err != nil {
 				return err
 			}
@@ -300,7 +298,7 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error {
 			if field.Name != columnLabel {
 				return ErrInputDefinitionColumnLabel
 			}
-		} else if actionCount == 0 {
+		} else if len(field.Actions) == 0 {
 			return ErrInputDefinitionActionRequired
 		}
 	}

From 122bff142ce9b972c1fdc96b8457d749209dec1a Mon Sep 17 00:00:00 2001
From: Michael Baird <mbaird@umbel.com>
Date: Thu, 29 Jun 2017 13:53:07 -0500
Subject: [PATCH 12/12] refactor validation comment.

---
 handler.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/handler.go b/handler.go
index 4fabdf911..09f920920 100644
--- a/handler.go
+++ b/handler.go
@@ -1520,7 +1520,7 @@ func (h *Handler) handlePostInputDefinition(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	// validation definition before/after encode?
+	// Validation the input definition with the curent index's ColumnLabel.
 	if err := req.Validate(index.ColumnLabel()); err != nil {
 		http.Error(w, err.Error(), http.StatusBadRequest)
 		return