Validate incoming input definitions through the InputDefinitionInfo Validate rather than through Encode and piecemeal throughout the code.

This commit is contained in:
Michael Baird 2017-06-28 16:33:42 -05:00
parent 78d40694c6
commit 6028b50b06
5 changed files with 174 additions and 129 deletions

View file

@ -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 {

View file

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

View file

@ -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

View file

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

View file

@ -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")