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/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 e6bb5e972..10179ca31 100644 --- a/input_definition.go +++ b/input_definition.go @@ -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 +} diff --git a/input_definition_test.go b/input_definition_test.go index 037a97c28..160064e90 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) { @@ -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) + } +}