diff --git a/docs/input-definition.md b/docs/input-definition.md index da59ee391..494f4757e 100644 --- a/docs/input-definition.md +++ b/docs/input-definition.md @@ -46,7 +46,7 @@ curl localhost:10101/index/repository/input-definition/stargazer \ { "name": "repo_id", "primaryKey": true - }, + }, { "actions": [ { @@ -105,8 +105,8 @@ curl localhost:10101/index/repository/input/stargazer \ -X POST \ -d '[ { - "language_id": "Go", "repo_id": 91720568, + "language_id": "Go", "stargazer_id": 513114, "time_value": "2017-05-18T20:40" }, diff --git a/handler.go b/handler.go index 0f224c31d..f3d9fe9d3 100644 --- a/handler.go +++ b/handler.go @@ -1760,8 +1760,7 @@ func (h *Handler) handlePostInputDefinition(w http.ResponseWriter, r *http.Reque return } - // Validation the input definition with the curent index's ColumnLabel. - if err := req.Validate(index.ColumnLabel()); err != nil { + if err := req.Validate(); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) return } @@ -1908,10 +1907,9 @@ func (h *Handler) InputJSONDataParser(req map[string]interface{}, index *Index, for _, field := range inputDef.Fields() { validFields[field.Name] = true if field.PrimaryKey { - columnLabel := field.Name - value, ok := req[columnLabel] + value, ok := req[field.Name] if !ok { - return nil, fmt.Errorf("columnLabel required") + return nil, fmt.Errorf("primary key does not exist") } rawValue, ok := value.(float64) // The default JSON marshalling will interpret this as a float if !ok { diff --git a/handler_test.go b/handler_test.go index fcae35a28..796782402 100644 --- a/handler_test.go +++ b/handler_test.go @@ -1309,7 +1309,7 @@ func TestHandler_DuplicatePrimaryKey(t *testing.T) { t.Fatalf("unexpected body: %s", body) } - // Eusure throwing error if primary field's name doesn't match columnLabel + // Ensure throwing error if there's no primary key hldr.MustCreateIndexIfNotExists("i1", pilosa.IndexOptions{ColumnLabel: "id"}) unmatchColumnBody := []byte(` { @@ -1323,8 +1323,17 @@ func TestHandler_DuplicatePrimaryKey(t *testing.T) { }], "fields": [ { - "name": "columnID", - "primaryKey": true + "name": "foo", + "actions": [ + { + "frame": "cab-type", + "valueDestination": "mapping", + "valueMap": { + "Green": 1, + "Yellow": 2 + } + } + ] } ] }`) @@ -1333,7 +1342,7 @@ func TestHandler_DuplicatePrimaryKey(t *testing.T) { h.ServeHTTP(w, test.MustNewHTTPRequest("POST", "/index/i1/input-definition/input1", bytes.NewBuffer(unmatchColumnBody))) if w.Code != http.StatusBadRequest { t.Fatalf("unexpected status code: %d", w.Code) - } else if body := w.Body.String(); body != pilosa.ErrInputDefinitionColumnLabel.Error()+"\n" { + } else if body := w.Body.String(); body != pilosa.ErrInputDefinitionHasPrimaryKey.Error()+"\n" { t.Fatalf("unexpected body: %s", body) } @@ -1677,7 +1686,7 @@ func TestInput_JSON(t *testing.T) { "distanceMiles": 8, "withPet": true }]`, - err: "columnLabel required"}, + err: "primary key does not exist"}, {json: `[{ "id": 1, "cabType": "yellow", diff --git a/index.go b/index.go index 2031ec5fe..bdf2832b0 100644 --- a/index.go +++ b/index.go @@ -687,7 +687,8 @@ func (i *Index) createInputDefinition(pb *internal.InputDefinition) (*InputDefin for _, fr := range pb.Frames { opt := FrameOptions{ - RowLabel: fr.Meta.RowLabel, + // Deprecating row labels per #810. So, setting the default row label here. + RowLabel: DefaultRowLabel, InverseEnabled: fr.Meta.InverseEnabled, CacheType: fr.Meta.CacheType, CacheSize: fr.Meta.CacheSize, diff --git a/index_test.go b/index_test.go index a7cff68f7..5e44a53a4 100644 --- a/index_test.go +++ b/index_test.go @@ -309,14 +309,14 @@ func TestIndex_CreateInputDefinition(t *testing.T) { // Create Input Definition. frames := internal.Frame{Name: "f", Meta: &internal.FrameMeta{RowLabel: "row"}} action := internal.InputDefinitionAction{Frame: "f", ValueDestination: "mapping", ValueMap: map[string]uint64{"Green": 1}} - fields := internal.InputDefinitionField{Name: "id", PrimaryKey: true, InputDefinitionActions: []*internal.InputDefinitionAction{&action}} - def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&fields}} + field := internal.InputDefinitionField{Name: "id", PrimaryKey: true, InputDefinitionActions: []*internal.InputDefinitionAction{&action}} + def := internal.InputDefinition{Name: "test", Frames: []*internal.Frame{&frames}, Fields: []*internal.InputDefinitionField{&field}} inputDef, err := index.CreateInputDefinition(&def) if err != nil { t.Fatal(err) } else if inputDef.Frames()[0].Name != frames.Name { t.Fatalf("unexpected input definition frames %v", inputDef.Frames()) - } else if inputDef.Fields()[0].Name != fields.Name { + } else if inputDef.Fields()[0].Name != field.Name { t.Fatalf("unexpected input definition actions %v", inputDef.Fields()) } } diff --git a/input_definition.go b/input_definition.go index 13b27bd17..84d22d4ac 100644 --- a/input_definition.go +++ b/input_definition.go @@ -90,7 +90,8 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { inputFrame := InputFrame{ Name: fr.Name, Options: FrameOptions{ - RowLabel: frameMeta.RowLabel, + // Deprecating row labels per #810. So, setting the default row label here. + RowLabel: DefaultRowLabel, InverseEnabled: frameMeta.InverseEnabled, CacheSize: frameMeta.CacheSize, CacheType: frameMeta.CacheType, @@ -100,6 +101,8 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { i.frames = append(i.frames, inputFrame) } + primaryKeyGiven := false + for _, field := range pb.Fields { var actions []Action for _, action := range field.InputDefinitionActions { @@ -111,6 +114,10 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { }) } + if field.PrimaryKey { + primaryKeyGiven = true + } + inputField := InputDefinitionField{ Name: field.Name, PrimaryKey: field.PrimaryKey, @@ -119,6 +126,10 @@ func (i *InputDefinition) LoadDefinition(pb *internal.InputDefinition) error { i.fields = append(i.fields, inputField) } + if len(pb.Fields) > 0 && !primaryKeyGiven { + return ErrInputDefinitionHasPrimaryKey + } + return nil } @@ -265,7 +276,7 @@ type InputDefinitionInfo struct { } // Validate the InputDefinitionInfo data. -func (i *InputDefinitionInfo) Validate(columnLabel string) error { +func (i *InputDefinitionInfo) Validate() error { numPrimaryKey := 0 accountRowID := make(map[string]uint64) @@ -281,6 +292,9 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error { // Validate columnLabel and duplicate primaryKey. for _, field := range i.Fields { + if field.Name == "" { + return ErrInputDefinitionNameRequired + } for _, action := range field.Actions { if err := action.Validate(); err != nil { return err @@ -298,9 +312,6 @@ func (i *InputDefinitionInfo) Validate(columnLabel string) error { } if field.PrimaryKey { numPrimaryKey++ - if field.Name != columnLabel { - return ErrInputDefinitionColumnLabel - } } else if len(field.Actions) == 0 { return ErrInputDefinitionActionRequired } diff --git a/input_definition_test.go b/input_definition_test.go index f2daade2c..93d0e393d 100644 --- a/input_definition_test.go +++ b/input_definition_test.go @@ -110,14 +110,14 @@ func TestActionValidation(t *testing.T) { 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") + err := info.Validate() if err != pilosa.ErrInputDefinitionAttrsRequired { t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionAttrsRequired, 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") + err = info.Validate() 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) } @@ -126,7 +126,7 @@ func TestActionValidation(t *testing.T) { 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") + err = info.Validate() if err != pilosa.ErrName { t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrName, err) } @@ -135,23 +135,15 @@ func TestActionValidation(t *testing.T) { 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") + err = info.Validate() if err != pilosa.ErrFrameRequired { t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrFrameRequired, 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 err != pilosa.ErrInputDefinitionColumnLabel { - t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionColumnLabel, 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") + err = info.Validate() if err != pilosa.ErrInputDefinitionHasPrimaryKey { t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionHasPrimaryKey, err) } @@ -159,7 +151,7 @@ func TestActionValidation(t *testing.T) { 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") + err = info.Validate() if !strings.Contains(err.Error(), "invalid ValueDestination") { t.Fatalf("Expected invalid ValueDestination error, actual error: %s", err) } @@ -167,7 +159,7 @@ func TestActionValidation(t *testing.T) { 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") + err = info.Validate() if err != pilosa.ErrInputDefinitionValueMap { t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionValueMap, err) } @@ -177,7 +169,7 @@ func TestActionValidation(t *testing.T) { 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") + err = info.Validate() if !strings.Contains(err.Error(), "duplicate rowID with other field") { t.Fatalf("Expected duplicate rowID with other field error, actual error: %s", err) } @@ -185,7 +177,7 @@ func TestActionValidation(t *testing.T) { field = pilosa.InputDefinitionField{Name: "id", PrimaryKey: true} field1 = pilosa.InputDefinitionField{Name: "test", PrimaryKey: false} info = pilosa.InputDefinitionInfo{Frames: []pilosa.InputFrame{frame}, Fields: []pilosa.InputDefinitionField{field, field1}} - err = info.Validate("id") + err = info.Validate() if err != pilosa.ErrInputDefinitionActionRequired { t.Fatalf("Expect error: %s, actual err: %s", pilosa.ErrInputDefinitionActionRequired, err) }