From 8a0609f1fd24bf22f648b0ff13625e4ea5f8b6e9 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Thu, 22 Jun 2017 11:11:36 -0500 Subject: [PATCH] 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") {