From 505363d8ed6854872fc54a8dde511b16309e0254 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 26 Sep 2017 10:57:22 -0500 Subject: [PATCH 1/4] get fields --- frame.go | 22 ++++++++++++++++++++-- handler.go | 23 +++++++++++++++++++++++ handler_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ index.go | 3 +++ 4 files changed, 94 insertions(+), 2 deletions(-) diff --git a/frame.go b/frame.go index f9580cc6b..88a0686b2 100644 --- a/frame.go +++ b/frame.go @@ -425,7 +425,7 @@ func (f *Frame) CreateField(field *Field) error { defer f.mu.Unlock() // Ensure frame supports fields. - if f.rangeEnabled { + if !f.rangeEnabled { return ErrFrameFieldsNotAllowed } @@ -435,10 +435,28 @@ func (f *Frame) CreateField(field *Field) error { return err } f.schema = schema - + f.saveSchema() return nil } +// GetFields list all the fields. +func (f *Frame) GetFields() (*FrameSchema, error) { + //f.mu.Lock() + //defer f.mu.Unlock() + + // Ensure frame supports fields. + if !f.RangeEnabled() { + return nil, nil + } + err := f.loadSchema() + if err != nil { + return nil, err + } + fmt.Println("HERE") + fmt.Printf("%+v\n", f.schema) + return f.schema, nil +} + // DeleteField deletes an existing field on the schema. func (f *Frame) DeleteField(name string) error { f.mu.Lock() diff --git a/handler.go b/handler.go index fef22dfff..4ddc6d62a 100644 --- a/handler.go +++ b/handler.go @@ -118,6 +118,7 @@ func NewRouter(handler *Handler) *mux.Router { router.HandleFunc("/index/{index}/frame/{frame}/restore", handler.handlePostFrameRestore).Methods("POST") router.HandleFunc("/index/{index}/frame/{frame}/time-quantum", handler.handlePatchFrameTimeQuantum).Methods("PATCH") router.HandleFunc("/index/{index}/frame/{frame}/field/{field}", handler.handlePostFrameField).Methods("POST") + router.HandleFunc("/index/{index}/frame/{frame}/fields", handler.handleGetFrameField).Methods("GET") router.HandleFunc("/index/{index}/frame/{frame}/field/{field}", handler.handleDeleteFrameField).Methods("DELETE") router.HandleFunc("/index/{index}/frame/{frame}/views", handler.handleGetFrameViews).Methods("GET") router.HandleFunc("/index/{index}/frame/{frame}/view/{view}", handler.handleDeleteView).Methods("DELETE") @@ -841,6 +842,28 @@ func (h *Handler) handleDeleteFrameField(w http.ResponseWriter, r *http.Request) } } +func (h *Handler) handleGetFrameField(w http.ResponseWriter, r *http.Request) { + indexName := mux.Vars(r)["index"] + frameName := mux.Vars(r)["frame"] + + index := h.Holder.index(indexName) + frame := index.frame(frameName) + schema, err := frame.GetFields() + fmt.Printf("%+v\n", schema) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + // Encode response. + if err := json.NewEncoder(w).Encode(getFrameFieldsResponse{Fields: schema.Fields}); err != nil { + h.logger().Printf("response encoding error: %s", err) + } +} + +type getFrameFieldsResponse struct { + Fields []*Field `json:"fields,omitempty"` +} + type deleteFrameFieldRequest struct{} type deleteFrameFieldResponse struct{} diff --git a/handler_test.go b/handler_test.go index 8159cf586..8071a43b0 100644 --- a/handler_test.go +++ b/handler_test.go @@ -32,6 +32,7 @@ import ( "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/test" + "fmt" ) func TestHandlerPanics(t *testing.T) { @@ -976,6 +977,7 @@ func TestHandler_Frame_DeleteField(t *testing.T) { t.Run("OK", func(t *testing.T) { idx := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) f, err := idx.CreateFrameIfNotExists("f", pilosa.FrameOptions{RangeEnabled: true}) + fmt.Println(f.RangeEnabled()) if err != nil { t.Fatal(err) } else if err := f.CreateField(&pilosa.Field{Name: "x", Type: pilosa.FieldTypeInt, Min: 0, Max: 100}); err != nil { @@ -1030,6 +1032,52 @@ func TestHandler_Frame_DeleteField(t *testing.T) { }) } +func TestHandler_Frame_GetFields(t *testing.T) { + hldr := test.MustOpenHolder() + defer hldr.Close() + + s := test.NewServer() + s.Handler.Holder = hldr.Holder + defer s.Close() + + + t.Run("OK", func(t *testing.T) { + idx := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) + f, err := idx.CreateFrameIfNotExists("f", pilosa.FrameOptions{RangeEnabled: true}) + fmt.Println(f.RangeEnabled()) + if err != nil { + t.Fatal(err) + } else if err := f.CreateField(&pilosa.Field{Name: "x", Type: pilosa.FieldTypeInt, Min: 1, Max: 100}); err != nil { + t.Fatal(err) + } + fmt.Println(f.RangeEnabled()) + resp, err := http.Get(s.URL+"/index/i/frame/f/fields") + if err != nil { + t.Fatal(err) + } + body, err := ioutil.ReadAll(resp.Body) + fmt.Println(string(body)) + if err != nil { + t.Fatal(err) + } else if resp.StatusCode != http.StatusOK { + t.Fatalf("unexpected status code: %d", resp.StatusCode) + } + + //var fields []pilosa.Field + //if err = json.NewDecoder(resp.Body).Decode(&fields); err != nil { + // t.Fatal(err) + //} + //if fields[0].Name != "x" { + // t.Fatalf("expected field's name: x, actuall name: %v", fields[0].Name) + //} + // + // + //if field := f.Field("x"); field != nil { + // t.Fatalf("expected nil field, got: %#v", field) + //} + + }) +} // Ensure the handler can backup a fragment and then restore it. func TestHandler_Fragment_BackupRestore(t *testing.T) { hldr := test.MustOpenHolder() diff --git a/index.go b/index.go index 345215690..a4f3876e0 100644 --- a/index.go +++ b/index.go @@ -476,6 +476,9 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { return nil, err } + f.rangeEnabled = opt.RangeEnabled + fmt.Println("CREATE FRAME", opt.RangeEnabled) + // Set schema & save. f.schema = &FrameSchema{ Fields: opt.Fields, From a1a6c7d71739acd42d2df7378b54c22ecc79aa30 Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Tue, 26 Sep 2017 13:46:00 -0500 Subject: [PATCH 2/4] update test --- frame.go | 8 +++----- handler.go | 3 +-- handler_test.go | 39 +++++++++++++++++++++++---------------- index.go | 1 - input_definition_test.go | 12 ++++++------ 5 files changed, 33 insertions(+), 30 deletions(-) diff --git a/frame.go b/frame.go index 88a0686b2..cd864e97b 100644 --- a/frame.go +++ b/frame.go @@ -441,8 +441,8 @@ func (f *Frame) CreateField(field *Field) error { // GetFields list all the fields. func (f *Frame) GetFields() (*FrameSchema, error) { - //f.mu.Lock() - //defer f.mu.Unlock() + f.mu.Lock() + defer f.mu.Unlock() // Ensure frame supports fields. if !f.RangeEnabled() { @@ -452,8 +452,6 @@ func (f *Frame) GetFields() (*FrameSchema, error) { if err != nil { return nil, err } - fmt.Println("HERE") - fmt.Printf("%+v\n", f.schema) return f.schema, nil } @@ -463,7 +461,7 @@ func (f *Frame) DeleteField(name string) error { defer f.mu.Unlock() // Ensure frame supports fields. - if f.rangeEnabled { + if !f.rangeEnabled { return ErrFrameFieldsNotAllowed } diff --git a/handler.go b/handler.go index 4ddc6d62a..03707b26d 100644 --- a/handler.go +++ b/handler.go @@ -842,14 +842,13 @@ func (h *Handler) handleDeleteFrameField(w http.ResponseWriter, r *http.Request) } } -func (h *Handler) handleGetFrameField(w http.ResponseWriter, r *http.Request) { +func (h *Handler) handleGetFrameField(w http.ResponseWriter, r *http.Request) { indexName := mux.Vars(r)["index"] frameName := mux.Vars(r)["frame"] index := h.Holder.index(indexName) frame := index.frame(frameName) schema, err := frame.GetFields() - fmt.Printf("%+v\n", schema) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/handler_test.go b/handler_test.go index 8071a43b0..62361b060 100644 --- a/handler_test.go +++ b/handler_test.go @@ -32,7 +32,6 @@ import ( "github.com/pilosa/pilosa/internal" "github.com/pilosa/pilosa/pql" "github.com/pilosa/pilosa/test" - "fmt" ) func TestHandlerPanics(t *testing.T) { @@ -977,7 +976,6 @@ func TestHandler_Frame_DeleteField(t *testing.T) { t.Run("OK", func(t *testing.T) { idx := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) f, err := idx.CreateFrameIfNotExists("f", pilosa.FrameOptions{RangeEnabled: true}) - fmt.Println(f.RangeEnabled()) if err != nil { t.Fatal(err) } else if err := f.CreateField(&pilosa.Field{Name: "x", Type: pilosa.FieldTypeInt, Min: 0, Max: 100}); err != nil { @@ -1040,36 +1038,40 @@ func TestHandler_Frame_GetFields(t *testing.T) { s.Handler.Holder = hldr.Holder defer s.Close() - t.Run("OK", func(t *testing.T) { idx := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) f, err := idx.CreateFrameIfNotExists("f", pilosa.FrameOptions{RangeEnabled: true}) - fmt.Println(f.RangeEnabled()) if err != nil { t.Fatal(err) } else if err := f.CreateField(&pilosa.Field{Name: "x", Type: pilosa.FieldTypeInt, Min: 1, Max: 100}); err != nil { t.Fatal(err) } - fmt.Println(f.RangeEnabled()) - resp, err := http.Get(s.URL+"/index/i/frame/f/fields") + resp, err := http.Get(s.URL + "/index/i/frame/f/fields") if err != nil { t.Fatal(err) } - body, err := ioutil.ReadAll(resp.Body) - fmt.Println(string(body)) if err != nil { t.Fatal(err) - } else if resp.StatusCode != http.StatusOK { + } else if resp.StatusCode != http.StatusOK { t.Fatalf("unexpected status code: %d", resp.StatusCode) } - //var fields []pilosa.Field - //if err = json.NewDecoder(resp.Body).Decode(&fields); err != nil { - // t.Fatal(err) - //} - //if fields[0].Name != "x" { - // t.Fatalf("expected field's name: x, actuall name: %v", fields[0].Name) - //} + var fields FrameFields + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if err = json.Unmarshal([]byte(body), &fields); err != nil { + t.Fatal(err) + } + field := fields.Fields[0] + if field.Name != "x" { + t.Fatalf("expected field's name: x, actuall name: %v", field.Name) + } else if field.Min != 1 { + t.Fatalf("expected field's min: x, actuall min: %v", field.Min) + } else if field.Max != 100 { + t.Fatalf("expected field's max: x, actuall max: %v", field.Max) + } // // //if field := f.Field("x"); field != nil { @@ -1078,6 +1080,11 @@ func TestHandler_Frame_GetFields(t *testing.T) { }) } + +type FrameFields struct { + Fields []pilosa.Field +} + // Ensure the handler can backup a fragment and then restore it. func TestHandler_Fragment_BackupRestore(t *testing.T) { hldr := test.MustOpenHolder() diff --git a/index.go b/index.go index a4f3876e0..f165df141 100644 --- a/index.go +++ b/index.go @@ -477,7 +477,6 @@ func (i *Index) createFrame(name string, opt FrameOptions) (*Frame, error) { } f.rangeEnabled = opt.RangeEnabled - fmt.Println("CREATE FRAME", opt.RangeEnabled) // Set schema & save. f.schema = &FrameSchema{ diff --git a/input_definition_test.go b/input_definition_test.go index 6531ca69d..5bb767203 100644 --- a/input_definition_test.go +++ b/input_definition_test.go @@ -203,14 +203,14 @@ func TestHandleAction(t *testing.T) { name string value interface{} expected uint64 - err string + err string }{ {name: "integer single-row-bool", action: pilosa.InputSingleRowBool, value: 1, err: "single-row-boolean value"}, - {name: "string single-row-bool", action: pilosa.InputSingleRowBool,value: "1", err: "single-row-boolean value 1 must equate to a Bool"}, - {name: "string value-to-row", action: pilosa.InputValueToRow,value: "25", err: "value-to-row value must equate to an integer"}, - {name: "string mapping", action: pilosa.InputMapping,value: "test", err: "Value test does not exist in definition map"}, - {name: "int mapping", action: pilosa.InputMapping,value: 25, err: "Mapping value must be a string"}, - {name: "invalid action", action: "test",value: true, err: "Unrecognized Value Destination"}, + {name: "string single-row-bool", action: pilosa.InputSingleRowBool, value: "1", err: "single-row-boolean value 1 must equate to a Bool"}, + {name: "string value-to-row", action: pilosa.InputValueToRow, value: "25", err: "value-to-row value must equate to an integer"}, + {name: "string mapping", action: pilosa.InputMapping, value: "test", err: "Value test does not exist in definition map"}, + {name: "int mapping", action: pilosa.InputMapping, value: 25, err: "Mapping value must be a string"}, + {name: "invalid action", action: "test", value: true, err: "Unrecognized Value Destination"}, } for _, r := range tests { t.Run(r.name, func(t *testing.T) { From 7e53a4ed1dd579fb5a4710a00d8c29063277860e Mon Sep 17 00:00:00 2001 From: Linh Vo Date: Wed, 27 Sep 2017 10:10:43 -0500 Subject: [PATCH 3/4] more test --- frame.go | 2 +- handler.go | 5 ++++- handler_test.go | 25 ++++++++++++++++++++----- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/frame.go b/frame.go index cd864e97b..5b8b06f63 100644 --- a/frame.go +++ b/frame.go @@ -446,7 +446,7 @@ func (f *Frame) GetFields() (*FrameSchema, error) { // Ensure frame supports fields. if !f.RangeEnabled() { - return nil, nil + return nil, ErrFrameFieldsNotAllowed } err := f.loadSchema() if err != nil { diff --git a/handler.go b/handler.go index 03707b26d..674f1baa4 100644 --- a/handler.go +++ b/handler.go @@ -849,7 +849,10 @@ func (h *Handler) handleGetFrameField(w http.ResponseWriter, r *http.Request) { index := h.Holder.index(indexName) frame := index.frame(frameName) schema, err := frame.GetFields() - if err != nil { + if err == ErrFrameFieldsNotAllowed { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } else if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } diff --git a/handler_test.go b/handler_test.go index 62361b060..6e8ebab88 100644 --- a/handler_test.go +++ b/handler_test.go @@ -1072,13 +1072,28 @@ func TestHandler_Frame_GetFields(t *testing.T) { } else if field.Max != 100 { t.Fatalf("expected field's max: x, actuall max: %v", field.Max) } - // - // - //if field := f.Field("x"); field != nil { - // t.Fatalf("expected nil field, got: %#v", field) - //} }) + + t.Run("ErrFrameFieldNotAllowed", func(t *testing.T) { + idx := hldr.MustCreateIndexIfNotExists("i", pilosa.IndexOptions{}) + _, err := idx.CreateFrameIfNotExists("f1", pilosa.FrameOptions{RangeEnabled: false}) + + resp, err := http.Get(s.URL + "/index/i/frame/f1/fields") + if err != nil { + t.Fatal(err) + } + if err != nil { + t.Fatal(err) + } else if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("unexpected status code: %d", resp.StatusCode) + } else if body, err := ioutil.ReadAll(resp.Body); err != nil { + t.Fatal(err) + } else if strings.TrimSpace(string(body)) != `frame fields not allowed` { + t.Fatalf("unexpected body: %q", body) + } + }) + } type FrameFields struct { From 06f387842b55461b9fafa4cebcdfe2e5fe83083e Mon Sep 17 00:00:00 2001 From: Travis Date: Wed, 27 Sep 2017 15:51:16 -0500 Subject: [PATCH 4/4] Adjust some comments and handle error conditions in Handler. --- frame.go | 10 ++++++---- handler.go | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/frame.go b/frame.go index 7c48fc2f5..e7bfd4a40 100644 --- a/frame.go +++ b/frame.go @@ -439,19 +439,21 @@ func (f *Frame) CreateField(field *Field) error { return nil } -// GetFields list all the fields. +// GetFields returns a list of all the fields in the frame. func (f *Frame) GetFields() (*FrameSchema, error) { - f.mu.Lock() - defer f.mu.Unlock() + f.mu.RLock() + defer f.mu.RUnlock() - // Ensure frame supports fields. + // Ensure the frame supports fields. if !f.RangeEnabled() { return nil, ErrFrameFieldsNotAllowed } + err := f.loadSchema() if err != nil { return nil, err } + return f.schema, nil } diff --git a/handler.go b/handler.go index e4c16cb9a..d8463862a 100644 --- a/handler.go +++ b/handler.go @@ -119,7 +119,7 @@ func NewRouter(handler *Handler) *mux.Router { router.HandleFunc("/index/{index}/frame/{frame}/restore", handler.handlePostFrameRestore).Methods("POST") router.HandleFunc("/index/{index}/frame/{frame}/time-quantum", handler.handlePatchFrameTimeQuantum).Methods("PATCH") router.HandleFunc("/index/{index}/frame/{frame}/field/{field}", handler.handlePostFrameField).Methods("POST") - router.HandleFunc("/index/{index}/frame/{frame}/fields", handler.handleGetFrameField).Methods("GET") + router.HandleFunc("/index/{index}/frame/{frame}/fields", handler.handleGetFrameFields).Methods("GET") router.HandleFunc("/index/{index}/frame/{frame}/field/{field}", handler.handleDeleteFrameField).Methods("DELETE") router.HandleFunc("/index/{index}/frame/{frame}/views", handler.handleGetFrameViews).Methods("GET") router.HandleFunc("/index/{index}/frame/{frame}/view/{view}", handler.handleDeleteView).Methods("DELETE") @@ -843,12 +843,22 @@ func (h *Handler) handleDeleteFrameField(w http.ResponseWriter, r *http.Request) } } -func (h *Handler) handleGetFrameField(w http.ResponseWriter, r *http.Request) { +func (h *Handler) handleGetFrameFields(w http.ResponseWriter, r *http.Request) { indexName := mux.Vars(r)["index"] frameName := mux.Vars(r)["frame"] index := h.Holder.index(indexName) + if index == nil { + http.Error(w, ErrIndexNotFound.Error(), http.StatusNotFound) + return + } + frame := index.frame(frameName) + if frame == nil { + http.Error(w, ErrFrameNotFound.Error(), http.StatusNotFound) + return + } + schema, err := frame.GetFields() if err == ErrFrameFieldsNotAllowed { http.Error(w, err.Error(), http.StatusBadRequest) @@ -857,6 +867,7 @@ func (h *Handler) handleGetFrameField(w http.ResponseWriter, r *http.Request) { http.Error(w, err.Error(), http.StatusInternalServerError) return } + // Encode response. if err := json.NewEncoder(w).Encode(getFrameFieldsResponse{Fields: schema.Fields}); err != nil { h.logger().Printf("response encoding error: %s", err)