From 6d12478e6bc9409eabb32c34151a03872407824e Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 20 Jun 2017 12:12:49 -0500 Subject: [PATCH 1/6] add validation for input definition fields --- input_definition.go | 50 ++++++++++++++++++++++++++++++++++++++++ input_definition_test.go | 48 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/input_definition.go b/input_definition.go index e6bb5e972..cb15732ca 100644 --- a/input_definition.go +++ b/input_definition.go @@ -19,10 +19,14 @@ import ( "os" "path/filepath" + "errors" + "fmt" "github.com/gogo/protobuf/proto" "github.com/pilosa/pilosa/internal" ) +var ValidValueDestination = []string{"map", "valueToRow", "stringToBool"} + // InputDefinition represents a container for the data input definition. type InputDefinition struct { name string @@ -89,9 +93,22 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { i.frames = append(i.frames, inputFrame) } + numPrimaryKey := 0 + countRowID := make(map[uint64]bool) for _, field := range pb.Fields { var actions []Action for _, action := range field.Actions { + if err := i.ValidateAction(action); err != nil { + return err + } + if action.RowID != 0 { + _, ok := countRowID[action.RowID] + if !ok { + countRowID[action.RowID] = true + } else { + return fmt.Errorf("duplicate rowID with other field: %s", action.RowID) + } + } actions = append(actions, Action{ Frame: action.Frame, ValueDestination: action.ValueDestination, @@ -100,6 +117,14 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { }) } + if field.PrimaryKey { + numPrimaryKey += 1 + } + + if numPrimaryKey > 1 { + return errors.New("duplicate primaryKey with other field") + } + inputField := Field{ Name: field.Name, PrimaryKey: field.PrimaryKey, @@ -246,3 +271,28 @@ func (i *InputDefinition) AddFrame(frame InputFrame) error { } return nil } + +func (i *InputDefinition) ValidateAction(action *internal.Action) error { + 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 "map": + if len(action.ValueMap) == 0 { + return errors.New("valueMap required for map") + } + case "stringToBool": + if action.RowID == 0 { + return errors.New("rowID required for stringToBool") + } + default: + return nil + } + + return nil +} diff --git a/input_definition_test.go b/input_definition_test.go index 037a97c28..178b30ea7 100644 --- a/input_definition_test.go +++ b/input_definition_test.go @@ -20,6 +20,7 @@ import ( "github.com/pilosa/pilosa" "github.com/pilosa/pilosa/internal" + "strings" ) func TestInputDefinition_Open(t *testing.T) { @@ -93,3 +94,50 @@ 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: "ValueToRow", 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) + } + + action = internal.Action{Frame: "f", ValueDestination: "stringToBool", ValueMap: map[string]uint64{"Green": 1}} + def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}} + err = input.LoadDefinition(def) + if !strings.Contains(err.Error(), "rowID required for stringToBool") { + t.Fatalf("Expected rowID required for stringToBool error, actual error: %s", err) + } + + action = internal.Action{Frame: "f", ValueDestination: "map", RowID: 100} + 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: "stringToBool", RowID: 100} + action1 := internal.Action{Frame: "f", ValueDestination: "stringToBool", RowID: 101} + 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: "stringToBool", 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) + } +} From e0bf37da40e7f08921a7cd61e814f44020cfbc86 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 20 Jun 2017 21:37:27 -0500 Subject: [PATCH 2/6] check same rowID for same frame in actions --- input_definition.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/input_definition.go b/input_definition.go index cb15732ca..f05ccb64a 100644 --- a/input_definition.go +++ b/input_definition.go @@ -94,19 +94,19 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { } numPrimaryKey := 0 - countRowID := make(map[uint64]bool) + 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.RowID != 0 { - _, ok := countRowID[action.RowID] - if !ok { - countRowID[action.RowID] = true - } else { + if action.RowID != 0 && action.Frame != ""{ + val, ok := countRowID[action.Frame] + if ok && val == action.RowID { return fmt.Errorf("duplicate rowID with other field: %s", action.RowID) + } else { + countRowID[action.Frame] = action.RowID } } actions = append(actions, Action{ From 9fd1de8f2966f3ffad42416f5b35ac2dfda1439a Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Wed, 21 Jun 2017 14:38:04 -0500 Subject: [PATCH 3/6] handle nil rowID --- handler_test.go | 4 ++-- index.go | 4 +++- index_test.go | 8 ++++---- input_definition.go | 34 ++++++++++++++++++++-------------- input_definition_test.go | 21 ++++++++++++--------- 5 files changed, 41 insertions(+), 30 deletions(-) diff --git a/handler_test.go b/handler_test.go index 45ca9f021..1b8c914ad 100644 --- a/handler_test.go +++ b/handler_test.go @@ -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) diff --git a/index.go b/index.go index c0877f0d7..c70c9eec4 100644 --- a/index.go +++ b/index.go @@ -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 } diff --git a/index_test.go b/index_test.go index f948310ca..f4cb534af 100644 --- a/index_test.go +++ b/index_test.go @@ -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) diff --git a/input_definition.go b/input_definition.go index f05ccb64a..10cfafd68 100644 --- a/input_definition.go +++ b/input_definition.go @@ -25,7 +25,7 @@ import ( "github.com/pilosa/pilosa/internal" ) -var ValidValueDestination = []string{"map", "valueToRow", "stringToBool"} +var ValidValueDestination = []string{"mapping", "value-to-row", "single-row-boolean"} // InputDefinition represents a container for the data input definition. type InputDefinition struct { @@ -101,10 +101,10 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { if err := i.ValidateAction(action); err != nil { return err } - if action.RowID != 0 && action.Frame != ""{ + if action.RowID != 0 && action.Frame != "" { val, ok := countRowID[action.Frame] if ok && val == action.RowID { - return fmt.Errorf("duplicate rowID with other field: %s", action.RowID) + return fmt.Errorf("duplicate rowID with other field: %v", action.RowID) } else { countRowID[action.Frame] = action.RowID } @@ -113,7 +113,7 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { Frame: action.Frame, ValueDestination: action.ValueDestination, ValueMap: action.ValueMap, - RowID: action.RowID, + RowID: &action.RowID, }) } @@ -176,7 +176,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) } @@ -226,7 +226,7 @@ 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. @@ -235,10 +235,19 @@ func (o *Action) Encode() *internal.Action { Frame: o.Frame, ValueDestination: o.ValueDestination, ValueMap: o.ValueMap, - RowID: o.RowID, + RowID: convert(o.RowID), } } +func convert(x *uint64) uint64 { + if x != nil { + return *x + } + var v int64 = -1 + var v2 uint64 = uint64(v) + return v2 +} + // InputFrame defines the frame used in the input definition. type InputFrame struct { Name string `json:"name,omitempty"` @@ -282,17 +291,14 @@ func (i *InputDefinition) ValidateAction(action *internal.Action) error { } switch action.ValueDestination { - case "map": + case "mapping": if len(action.ValueMap) == 0 { return errors.New("valueMap required for map") } - case "stringToBool": - if action.RowID == 0 { - return errors.New("rowID required for stringToBool") + case "single-row-boolean": + if int64(action.RowID) == -1 { + return errors.New("rowID required for single-row-boolean") } - default: - return nil } - return nil } diff --git a/input_definition_test.go b/input_definition_test.go index 178b30ea7..d8b95d5ba 100644 --- a/input_definition_test.go +++ b/input_definition_test.go @@ -29,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) @@ -102,7 +102,7 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { // Create Input Definition. input := pilosa.InputDefinition{} frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}} - action := internal.Action{Frame: "f", ValueDestination: "ValueToRow", ValueMap: map[string]uint64{"Green": 1}} + 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) @@ -110,22 +110,25 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { t.Fatalf("Expected invalid ValueDestination error, actual error: %s", err) } - action = internal.Action{Frame: "f", ValueDestination: "stringToBool", ValueMap: map[string]uint64{"Green": 1}} + act := pilosa.Action{Frame: "f", ValueDestination: "single-row-boolean", ValueMap: map[string]uint64{"Green": 1}} + encodeAction := act.Encode() + field = internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{encodeAction}} def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}} err = input.LoadDefinition(def) - if !strings.Contains(err.Error(), "rowID required for stringToBool") { - t.Fatalf("Expected rowID required for stringToBool error, actual error: %s", err) + 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: "map", RowID: 100} + action = internal.Action{Frame: "f", ValueDestination: "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: "stringToBool", RowID: 100} - action1 := internal.Action{Frame: "f", ValueDestination: "stringToBool", RowID: 101} + action = internal.Action{Frame: "f", ValueDestination: "single-row-boolean", RowID: 100} + action1 := internal.Action{Frame: "f", ValueDestination: "single-row-boolean", 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) @@ -133,7 +136,7 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { t.Fatalf("Expected duplicate primaryKey error, actual error: %s", err) } - action1 = internal.Action{Frame: "f", ValueDestination: "stringToBool", RowID: 100} + action1 = internal.Action{Frame: "f", ValueDestination: "single-row-boolean", 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) From 56c16e923ac3e0c7cb242fc5dd4e53c44edd726a Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Wed, 21 Jun 2017 14:53:01 -0500 Subject: [PATCH 4/6] add frame required in action --- input_definition.go | 3 +++ input_definition_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/input_definition.go b/input_definition.go index 10cfafd68..742139a4a 100644 --- a/input_definition.go +++ b/input_definition.go @@ -282,6 +282,9 @@ func (i *InputDefinition) AddFrame(frame InputFrame) error { } 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 diff --git a/input_definition_test.go b/input_definition_test.go index d8b95d5ba..9a1c4a5a4 100644 --- a/input_definition_test.go +++ b/input_definition_test.go @@ -143,4 +143,11 @@ func TestInputDefinition_LoadDefinition(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) } + + action = internal.Action{ValueDestination: "single-row-boolean", 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) + } } From 8a0609f1fd24bf22f648b0ff13625e4ea5f8b6e9 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 22 Jun 2017 11:11:36 -0500 Subject: [PATCH 5/6] handle nil rowID --- handler.go | 6 ++++- input_definition.go | 48 ++++++++++++++++++++++++---------------- input_definition_test.go | 22 +++++++++--------- 3 files changed, 45 insertions(+), 31 deletions(-) diff --git a/handler.go b/handler.go index cefb60ec1..9a9617994 100644 --- a/handler.go +++ b/handler.go @@ -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. diff --git a/input_definition.go b/input_definition.go index 742139a4a..a24163ca6 100644 --- a/input_definition.go +++ b/input_definition.go @@ -25,7 +25,13 @@ import ( "github.com/pilosa/pilosa/internal" ) -var ValidValueDestination = []string{"mapping", "value-to-row", "single-row-boolean"} +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 { @@ -116,7 +122,6 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { RowID: &action.RowID, }) } - if field.PrimaryKey { numPrimaryKey += 1 } @@ -213,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. @@ -230,22 +240,23 @@ type Action struct { } // 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: convert(o.RowID), - } + }, nil } func convert(x *uint64) uint64 { if x != nil { return *x } - var v int64 = -1 - var v2 uint64 = uint64(v) - return v2 + return 0 } // InputFrame defines the frame used in the input definition. @@ -261,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 { @@ -292,16 +307,11 @@ func (i *InputDefinition) ValidateAction(action *internal.Action) error { if _, ok := validValues[action.ValueDestination]; !ok { return fmt.Errorf("invalid ValueDestination: %s", action.ValueDestination) } - switch action.ValueDestination { - case "mapping": + case Mapping: if len(action.ValueMap) == 0 { return errors.New("valueMap required for map") } - case "single-row-boolean": - if int64(action.RowID) == -1 { - return errors.New("rowID required for single-row-boolean") - } } return nil } diff --git a/input_definition_test.go b/input_definition_test.go index 9a1c4a5a4..160064e90 100644 --- a/input_definition_test.go +++ b/input_definition_test.go @@ -80,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) @@ -110,16 +113,13 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { t.Fatalf("Expected invalid ValueDestination error, actual error: %s", err) } - act := pilosa.Action{Frame: "f", ValueDestination: "single-row-boolean", ValueMap: map[string]uint64{"Green": 1}} - encodeAction := act.Encode() - field = internal.InputDefinitionField{Name: "id", PrimaryKey: true, Actions: []*internal.Action{encodeAction}} - def = &internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}} - err = input.LoadDefinition(def) + 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: "mapping", RowID: 100} + 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) @@ -127,8 +127,8 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { t.Fatalf("Expected valueMap required for map error, actual error: %s", err) } - action = internal.Action{Frame: "f", ValueDestination: "single-row-boolean", RowID: 100} - action1 := internal.Action{Frame: "f", ValueDestination: "single-row-boolean", RowID: 0} + 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) @@ -136,7 +136,7 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { t.Fatalf("Expected duplicate primaryKey error, actual error: %s", err) } - action1 = internal.Action{Frame: "f", ValueDestination: "single-row-boolean", RowID: 100} + 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) @@ -144,7 +144,7 @@ func TestInputDefinition_LoadDefinition(t *testing.T) { t.Fatalf("Expected duplicate rowID with other field error, actual error: %s", err) } - action = internal.Action{ValueDestination: "single-row-boolean", RowID: 100} + 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") { From 13b32b0b109e66bea051d3d368871375fcf68874 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 22 Jun 2017 14:12:16 -0500 Subject: [PATCH 6/6] check SingleRowBool for duplicate rowID --- input_definition.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/input_definition.go b/input_definition.go index a24163ca6..10179ca31 100644 --- a/input_definition.go +++ b/input_definition.go @@ -107,7 +107,7 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { if err := i.ValidateAction(action); err != nil { return err } - if action.RowID != 0 && action.Frame != "" { + 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)