Merge pull request #668 from linhvo/validate-input-def

add validation for input definition fields
This commit is contained in:
Michael Baird 2017-06-22 14:29:32 -05:00 committed by GitHub
commit ba0eff4ffe
6 changed files with 154 additions and 21 deletions

View file

@ -1518,7 +1518,11 @@ func (h *Handler) handlePostInputDefinition(w http.ResponseWriter, r *http.Reque
return
}
def := req.Encode()
def, err := req.Encode()
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
def.Name = inputDefName
// Create InputDefinition.

View file

@ -1121,7 +1121,7 @@ func TestHandler_DeleteInputDefinition(t *testing.T) {
index := hldr.MustCreateIndexIfNotExists("i0", pilosa.IndexOptions{})
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
_, err := index.CreateInputDefinition(&def)
@ -1150,7 +1150,7 @@ func TestHandler_GetInputDefinition(t *testing.T) {
index := hldr.MustCreateIndexIfNotExists("i0", pilosa.IndexOptions{})
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
inputDef, err := index.CreateInputDefinition(&def)

View file

@ -650,7 +650,9 @@ func (i *Index) createInputDefinition(pb *internal.InputDefinition) (*InputDefin
return nil, err
}
inputDef.LoadDefinition(pb)
if err = inputDef.LoadDefinition(pb); err != nil {
return nil, err
}
if err = inputDef.saveMeta(); err != nil {
return nil, err
}

View file

@ -247,7 +247,7 @@ func TestIndex_CreateInputDefinition(t *testing.T) {
// Create Input Definition.
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
inputDef, err := index.CreateInputDefinition(&def)
@ -266,7 +266,7 @@ func TestIndex_CreateExistingInputDefinition(t *testing.T) {
// Create Input Definition.
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
_, err := index.CreateInputDefinition(&def)
@ -297,7 +297,7 @@ func TestIndex_DeleteInputDefinition(t *testing.T) {
// Create Input Definition.
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
_, err := index.CreateInputDefinition(&def)
@ -321,7 +321,7 @@ func TestIndex_CreateFrameWhenOpenInputDefinition(t *testing.T) {
// Create Input Definition.
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
input, err := index.CreateInputDefinition(&def)

View file

@ -19,10 +19,20 @@ import (
"os"
"path/filepath"
"errors"
"fmt"
"github.com/gogo/protobuf/proto"
"github.com/pilosa/pilosa/internal"
)
const (
Mapping = "mapping"
ValueToRow = "value-to-row"
SingleRowBool = "single-row-boolean"
)
var ValidValueDestination = []string{Mapping, ValueToRow, SingleRowBool}
// InputDefinition represents a container for the data input definition.
type InputDefinition struct {
name string
@ -89,16 +99,36 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error {
i.frames = append(i.frames, inputFrame)
}
numPrimaryKey := 0
countRowID := make(map[string]uint64)
for _, field := range pb.Fields {
var actions []Action
for _, action := range field.Actions {
if err := i.ValidateAction(action); err != nil {
return err
}
if action.ValueDestination == SingleRowBool && action.Frame != "" {
val, ok := countRowID[action.Frame]
if ok && val == action.RowID {
return fmt.Errorf("duplicate rowID with other field: %v", action.RowID)
} else {
countRowID[action.Frame] = action.RowID
}
}
actions = append(actions, Action{
Frame: action.Frame,
ValueDestination: action.ValueDestination,
ValueMap: action.ValueMap,
RowID: action.RowID,
RowID: &action.RowID,
})
}
if field.PrimaryKey {
numPrimaryKey += 1
}
if numPrimaryKey > 1 {
return errors.New("duplicate primaryKey with other field")
}
inputField := Field{
Name: field.Name,
@ -151,7 +181,7 @@ func (i *InputDefinition) saveMeta() error {
Frame: action.Frame,
ValueDestination: action.ValueDestination,
ValueMap: action.ValueMap,
RowID: action.RowID,
RowID: convert(action.RowID),
}
actions = append(actions, actionMeta)
}
@ -188,12 +218,17 @@ type Field struct {
}
// Encode converts Field into its internal representation.
func (o *Field) Encode() *internal.InputDefinitionField {
func (o *Field) Encode() (*internal.InputDefinitionField, error) {
field := internal.InputDefinitionField{Name: o.Name, PrimaryKey: o.PrimaryKey}
for _, action := range o.Actions {
field.Actions = append(field.Actions, action.Encode())
actionEncode, err := action.Encode()
if err != nil {
return nil, err
}
field.Actions = append(field.Actions, actionEncode)
}
return &field
return &field, nil
}
// Action descripes the mapping method for the field in the InputDefinition.
@ -201,17 +236,27 @@ type Action struct {
Frame string `json:"frame,omitempty"`
ValueDestination string `json:"valueDestination,omitempty"`
ValueMap map[string]uint64 `json:"valueMap,omitempty"`
RowID uint64 `json:"rowID,omitempty"`
RowID *uint64 `json:"rowID,omitempty"`
}
// Encode converts Action into its internal representation.
func (o *Action) Encode() *internal.Action {
func (o *Action) Encode() (*internal.Action, error) {
if o.RowID == nil && o.ValueDestination == "single-row-boolean" {
return nil, errors.New("rowID required for single-row-boolean")
}
return &internal.Action{
Frame: o.Frame,
ValueDestination: o.ValueDestination,
ValueMap: o.ValueMap,
RowID: o.RowID,
RowID: convert(o.RowID),
}, nil
}
func convert(x *uint64) uint64 {
if x != nil {
return *x
}
return 0
}
// InputFrame defines the frame used in the input definition.
@ -227,16 +272,20 @@ type InputDefinitionInfo struct {
}
// Encode converts InputDefinitionInfo into its internal representation.
func (i *InputDefinitionInfo) Encode() *internal.InputDefinition {
func (i *InputDefinitionInfo) Encode() (*internal.InputDefinition, error) {
var def internal.InputDefinition
for _, f := range i.Frames {
def.Frames = append(def.Frames, &internal.Frame{Name: f.Name, Meta: f.Options.Encode()})
}
for _, f := range i.Fields {
def.Fields = append(def.Fields, f.Encode())
fEncode, err := f.Encode()
if err != nil {
return nil, err
}
def.Fields = append(def.Fields, fEncode)
}
return &def
return &def, nil
}
func (i *InputDefinition) AddFrame(frame InputFrame) error {
@ -246,3 +295,23 @@ func (i *InputDefinition) AddFrame(frame InputFrame) error {
}
return nil
}
func (i *InputDefinition) ValidateAction(action *internal.Action) 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 Mapping:
if len(action.ValueMap) == 0 {
return errors.New("valueMap required for map")
}
}
return nil
}

View file

@ -20,6 +20,7 @@ import (
"github.com/pilosa/pilosa"
"github.com/pilosa/pilosa/internal"
"strings"
)
func TestInputDefinition_Open(t *testing.T) {
@ -28,7 +29,7 @@ func TestInputDefinition_Open(t *testing.T) {
// Create Input Definition.
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "map", ValueMap: map[string]uint64{"Green": 1}}
action := internal.Action{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}}
fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}}
inputDef, err := index.CreateInputDefinition(&def)
@ -79,7 +80,10 @@ func TestInputDefinition_Encoding(t *testing.T) {
t.Fatal(err)
}
internalDef := def.Encode()
internalDef, err := def.Encode()
if err != nil {
t.Fatal(err)
}
if internalDef.Frames[0].Name != "event-time" {
t.Fatalf("unexpected frame: %v", internalDef)
@ -93,3 +97,57 @@ func TestInputDefinition_Encoding(t *testing.T) {
t.Fatalf("unexpected ValueDestination: %v", internalDef.Fields[1].Actions[0])
}
}
func TestInputDefinition_LoadDefinition(t *testing.T) {
index := MustOpenIndex()
defer index.Close()
// Create Input Definition.
input := pilosa.InputDefinition{}
frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}}
action := internal.Action{Frame: "f", ValueDestination: "value-to-ROW", ValueMap: map[string]uint64{"Green": 1}}
field := internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action}}
def := &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}}
err := input.LoadDefinition(def)
if !strings.Contains(err.Error(), "invalid ValueDestination") {
t.Fatalf("Expected invalid ValueDestination error, actual error: %s", err)
}
act := pilosa.Action{Frame: "f", ValueDestination: pilosa.SingleRowBool, ValueMap: map[string]uint64{"Green": 1}}
_, err = act.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)
}
action = internal.Action{Frame: "f", ValueDestination: pilosa.Mapping, RowID: 100}
field = internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&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 = internal.Action{Frame: "f", ValueDestination: pilosa.SingleRowBool, RowID: 100}
action1 := internal.Action{Frame: "f", ValueDestination: pilosa.SingleRowBool, RowID: 0}
field1 := internal.InputDefinitionField{Name: "newID", PrimaryKey: true, Actions: []*internal.Action{&action1}}
def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field, &field1}}
err = input.LoadDefinition(def)
if !strings.Contains(err.Error(), "duplicate primaryKey with other field") {
t.Fatalf("Expected duplicate primaryKey error, actual error: %s", err)
}
action1 = internal.Action{Frame: "f", ValueDestination: pilosa.SingleRowBool, RowID: 100}
field1 = internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{&action1}}
def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field, &field1}}
err = input.LoadDefinition(def)
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.Action{ValueDestination: pilosa.SingleRowBool, 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)
}
}