Merge pull request #696 from raskle/input-definition-validation

Validate incoming input definitions through the InputDefinitionInfo V…

handler.go

@@ -1520,34 +1520,16 @@ func (h *Handler) handlePostInputDefinition(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	// TODO: validation 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
+	}
 
 	// 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 {
@@ -1584,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 {
@@ -1608,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
 	}
 
@@ -1647,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
@@ -1668,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 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 {
+		return nil, err
 	}
 	// if field in input data is not in defined definition, return error
 	var columnLabel string

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.
@@ -1159,7 +1168,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)
 	}
 
@@ -1191,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":[{
@@ -1228,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 {
@@ -1237,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}}
@@ -1247,18 +1256,28 @@ 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 {
 		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)
 	}
 }
 
-// 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()
@@ -1271,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 {
@@ -1280,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 {
@@ -1299,6 +1318,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 nonexistent 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 = `
@@ -1363,6 +1389,16 @@ var defaultBody = `
 						   "frame":"distance-miles",
 						   "valueDestination":"value-to-row"
 
+						}
+					 ]
+				  },
+				  {
+					 "name":"noFrame",
+					 "actions":[
+						{
+						   "frame":"foo",
+						   "valueDestination":"value-to-row"
+
 						}
 					 ]
 				  }
@@ -1393,7 +1429,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 nonexistent 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)
@@ -1401,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)
 	}
@@ -1416,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)
 	}
@@ -1466,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

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

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)
 	}
 }
 

input_definition.go

@@ -20,8 +20,6 @@ import (
 	"os"
 	"path/filepath"
 
-	"errors"
-
 	"github.com/gogo/protobuf/proto"
 	"github.com/pilosa/pilosa/internal"
 )
@@ -101,20 +99,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,23 +196,35 @@ 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
+	}
+	if !foundItem(validValueDestination, a.ValueDestination) {
+		return fmt.Errorf("invalid ValueDestination: %s", a.ValueDestination)
+	}
+
+	switch a.ValueDestination {
+	case InputMapping:
+		if len(a.ValueMap) == 0 {
+			return ErrInputDefinitionValueMap
 		}
-	*/
+	}
+	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),
 	}
 }
 
-// convert pointer to uint64
+// convert pointer to uint64.
 func convert(x *uint64) uint64 {
 	if x != nil {
 		return *x
@@ -239,13 +238,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
 	}
+	// TODO frame option validation
+	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 +261,57 @@ 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 || len(i.Fields) == 0 {
+		return ErrInputDefinitionAttrsRequired
+	}
+
+	for _, frame := range i.Frames {
+		if err := frame.Validate(); err != nil {
+			return err
+		}
+	}
+
+	// Validate columnLabel and duplicate primaryKey.
+	for _, field := range i.Fields {
+		for _, action := range field.Actions {
+			if err := action.Validate(); err != nil {
+				return err
+			}
+			if action.ValueDestination == InputSingleRowBool {
+				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 field.PrimaryKey {
+			numPrimaryKey++
+			if field.Name != columnLabel {
+				return ErrInputDefinitionColumnLabel
+			}
+		} else if len(field.Actions) == 0 {
+			return ErrInputDefinitionActionRequired
+		}
+	}
+
+	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 +333,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

input_definition_test.go

@@ -102,64 +102,94 @@ 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 err != pilosa.ErrInputDefinitionAttrsRequired {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionAttrsRequired, 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 err != pilosa.ErrName {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrName, 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 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 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 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}}
+	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)
-	if !strings.Contains(err.Error(), "valueMap required for map") {
-		t.Fatalf("Expected valueMap required for map error, actual error: %s", err)
+	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 err != pilosa.ErrInputDefinitionValueMap {
+		t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionValueMap, 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)
+	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)
 	}
 }
 
-/*
-// 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)

pilosa.go

@@ -36,18 +36,21 @@ 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")
+	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")
-	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")