diff --git a/api.go b/api.go index 2b96d023c..bdaa35b03 100644 --- a/api.go +++ b/api.go @@ -213,7 +213,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, fieldName str for _, opt := range opts { err := opt(&fo) if err != nil { - return nil, errors.Wrap(err, "applying option") + return nil, NewBadRequestError(errors.Wrap(err, "applying option")) } } diff --git a/field.go b/field.go index 73c09f14c..3e71af589 100644 --- a/field.go +++ b/field.go @@ -135,6 +135,9 @@ func OptFieldTypeInt(min, max int64) FieldOption { if fo.Type != "" { return errors.Errorf("field type is already set to: %s", fo.Type) } + if min > max { + return errors.New("int field min cannot be greater than max") + } fo.Type = FieldTypeInt fo.Min = min fo.Max = max diff --git a/http/handler.go b/http/handler.go index 0ce3ee6cd..8e932271a 100644 --- a/http/handler.go +++ b/http/handler.go @@ -759,7 +759,15 @@ func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) { case pilosa.FieldTypeSet: fos = append(fos, pilosa.OptFieldTypeSet(*req.Options.CacheType, *req.Options.CacheSize)) case pilosa.FieldTypeInt: - fos = append(fos, pilosa.OptFieldTypeInt(math.MinInt64, math.MaxInt64)) + if req.Options.Min == nil { + min := int64(math.MinInt64) + req.Options.Min = &min + } + if req.Options.Max == nil { + max := int64(math.MaxInt64) + req.Options.Max = &max + } + fos = append(fos, pilosa.OptFieldTypeInt(*req.Options.Min, *req.Options.Max)) case pilosa.FieldTypeTime: fos = append(fos, pilosa.OptFieldTypeTime(*req.Options.TimeQuantum, req.Options.NoStandardView)) case pilosa.FieldTypeMutex: @@ -774,6 +782,10 @@ func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) { } _, err = h.api.CreateField(r.Context(), indexName, fieldName, fos...) + if _, ok := err.(pilosa.BadRequestError); ok { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } resp.write(w, err) } diff --git a/server/handler_test.go b/server/handler_test.go index fc832269a..c4e98d7f3 100644 --- a/server/handler_test.go +++ b/server/handler_test.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "io/ioutil" + "math" gohttp "net/http" "net/http/httptest" "reflect" @@ -542,6 +543,104 @@ func TestHandler_Endpoints(t *testing.T) { } }) + t.Run("Query int field unbounded", func(t *testing.T) { + w := httptest.NewRecorder() + fieldName := "f-int-ubound" + h.ServeHTTP(w, test.MustNewHTTPRequest("POST", fmt.Sprintf("/index/i0/field/%s", fieldName), + strings.NewReader(`{"options":{"type":"int"}}`))) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + w = httptest.NewRecorder() + h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema", strings.NewReader(""))) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + rsp := getSchemaResponse{} + if err := json.Unmarshal(w.Body.Bytes(), &rsp); err != nil { + t.Fatalf("json decode: %s", err) + } + field := rsp.findField("i0", fieldName) + if field == nil { + t.Fatalf("field not found: %s", fieldName) + } + if math.MinInt64 != field.Options.Min { + t.Fatalf("field min %d != %d", int64(math.MinInt64), field.Options.Min) + } + if math.MaxInt64 != field.Options.Max { + t.Fatalf("field max %d != %d", int64(math.MaxInt64), field.Options.Max) + } + }) + + t.Run("Query int field unbounded min", func(t *testing.T) { + w := httptest.NewRecorder() + fieldName := "f-int-ubound-min" + h.ServeHTTP(w, test.MustNewHTTPRequest("POST", fmt.Sprintf("/index/i0/field/%s", fieldName), + strings.NewReader(`{"options":{"type":"int", "max": 10}}`))) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + w = httptest.NewRecorder() + h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema", strings.NewReader(""))) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + rsp := getSchemaResponse{} + if err := json.Unmarshal(w.Body.Bytes(), &rsp); err != nil { + t.Fatalf("json decode: %s", err) + } + field := rsp.findField("i0", fieldName) + if field == nil { + t.Fatalf("field not found: %s", fieldName) + } + if math.MinInt64 != field.Options.Min { + t.Fatalf("field min %d != %d", int64(math.MinInt64), field.Options.Min) + } + if 10 != field.Options.Max { + t.Fatalf("field max %d != %d", 10, field.Options.Max) + } + }) + + t.Run("Query int field unbounded max", func(t *testing.T) { + w := httptest.NewRecorder() + fieldName := "f-int-ubound-max" + h.ServeHTTP(w, test.MustNewHTTPRequest("POST", fmt.Sprintf("/index/i0/field/%s", fieldName), + strings.NewReader(`{"options":{"type":"int", "min": -10}}`))) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + w = httptest.NewRecorder() + h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/schema", strings.NewReader(""))) + if w.Code != gohttp.StatusOK { + t.Fatalf("unexpected status code: %d", w.Code) + } + rsp := getSchemaResponse{} + if err := json.Unmarshal(w.Body.Bytes(), &rsp); err != nil { + t.Fatalf("json decode: %s", err) + } + field := rsp.findField("i0", fieldName) + if field == nil { + t.Fatalf("field not found: %s", fieldName) + } + if -10 != field.Options.Min { + t.Fatalf("field min %d != %d", 10, field.Options.Min) + } + if math.MaxInt64 != field.Options.Max { + t.Fatalf("field max %d != %d", int64(math.MaxInt64), field.Options.Max) + } + }) + + t.Run("Query int field min > max return 400", func(t *testing.T) { + w := httptest.NewRecorder() + fieldName := "f-int-ubound-err" + h.ServeHTTP(w, test.MustNewHTTPRequest("POST", fmt.Sprintf("/index/i0/field/%s", fieldName), + strings.NewReader(`{"options":{"type":"int", "min": 10, "max": -10}}`))) + fmt.Println("body", w.Body.String()) + if w.Code != gohttp.StatusBadRequest { + t.Fatalf("unexpected status code: %d", w.Code) + } + }) + t.Run("Method not allowed", func(t *testing.T) { w := httptest.NewRecorder() h.ServeHTTP(w, test.MustNewHTTPRequest("GET", "/index/i0/query", nil)) @@ -999,3 +1098,20 @@ func mustJSONDecodeSlice(t *testing.T, r io.Reader) (ret []interface{}) { } return ret } + +type getSchemaResponse struct { + Indexes []*pilosa.IndexInfo `json:"indexes"` +} + +func (r getSchemaResponse) findField(indexName, fieldName string) *pilosa.FieldInfo { + for _, index := range r.Indexes { + if index.Name == indexName { + for _, field := range index.Fields { + if field.Name == fieldName { + return field + } + } + } + } + return nil +}