address feedback in PR

This commit is contained in:
Travis Turner 2018-07-01 17:59:18 -05:00
parent 3adc3f5978
commit 9633e34300
No known key found for this signature in database
GPG key ID: 7F08008DFD9314C9
5 changed files with 66 additions and 61 deletions

16
api.go
View file

@ -89,7 +89,7 @@ func (api *API) validate(f apiMethod) error {
if _, ok := validAPIMethods[state][f]; ok {
return nil
}
return ApiMethodNotAllowedError{errors.Errorf("api method %s not allowed in state %s", f, state)}
return NewApiMethodNotAllowedError(errors.Errorf("api method %s not allowed in state %s", f, state))
}
// Query parses a PQL query out of the request and executes it.
@ -205,7 +205,7 @@ func (api *API) Index(ctx context.Context, indexName string) (*Index, error) {
index := api.Holder.Index(indexName)
if index == nil {
return nil, NotFoundError{ErrIndexNotFound}
return nil, NewNotFoundError(ErrIndexNotFound)
}
return index, nil
}
@ -253,7 +253,7 @@ func (api *API) CreateField(ctx context.Context, indexName string, fieldName str
// Find index.
index := api.Holder.Index(indexName)
if index == nil {
return nil, NotFoundError{ErrIndexNotFound}
return nil, NewNotFoundError(ErrIndexNotFound)
}
// Create field.
@ -288,7 +288,7 @@ func (api *API) DeleteField(ctx context.Context, indexName string, fieldName str
// Find index.
index := api.Holder.Index(indexName)
if index == nil {
return NotFoundError{ErrIndexNotFound}
return NewNotFoundError(ErrIndexNotFound)
}
// Delete field from the index.
@ -416,11 +416,11 @@ func (api *API) FragmentBlockData(ctx context.Context, body io.Reader) ([]byte,
reqBytes, err := ioutil.ReadAll(body)
if err != nil {
return nil, BadRequestError{errors.Wrap(err, "read body error")}
return nil, NewBadRequestError(errors.Wrap(err, "read body error"))
}
var req internal.BlockDataRequest
if err := proto.Unmarshal(reqBytes, &req); err != nil {
return nil, BadRequestError{errors.Wrap(err, "unmarshal body error")}
return nil, NewBadRequestError(errors.Wrap(err, "unmarshal body error"))
}
// Retrieve fragment from holder.
@ -575,7 +575,7 @@ func (api *API) IndexAttrDiff(ctx context.Context, indexName string, blocks []At
// Retrieve index from holder.
index := api.Holder.Index(indexName)
if index == nil {
return nil, NotFoundError{ErrIndexNotFound}
return nil, NewNotFoundError(ErrIndexNotFound)
}
// Retrieve local blocks.
@ -717,7 +717,7 @@ func (api *API) indexField(indexName string, fieldName string, shard uint64) (*I
index := api.Holder.Index(indexName)
if index == nil {
api.server.logger.Printf("fragment error: index=%s, field=%s, shard=%d, err=%s", indexName, fieldName, shard, ErrIndexNotFound.Error())
return nil, nil, NotFoundError{ErrIndexNotFound}
return nil, nil, NewNotFoundError(ErrIndexNotFound)
}
// Retrieve field.

View file

@ -14,8 +14,6 @@
package http
import "bytes"
// Error defines a standard application error.
type Error struct {
// Machine-readable error code.
@ -27,7 +25,5 @@ type Error struct {
// Error returns the string representation of the error message.
func (e *Error) Error() string {
var buf bytes.Buffer
buf.WriteString(e.Message)
return buf.String()
return e.Message
}

View file

@ -578,19 +578,14 @@ func (h *Handler) handlePostIndex(w http.ResponseWriter, r *http.Request) {
resp := successResponse{}
err := func() error {
// Decode request.
var req postIndexRequest
err := json.NewDecoder(r.Body).Decode(&req)
if err == io.EOF {
// If no data was provided (EOF), we still create the index
// with default values.
} else if err != nil {
return err
}
_, err = h.API.CreateIndex(r.Context(), indexName, req.Options)
return err
}()
// Decode request.
var req postIndexRequest
err := json.NewDecoder(r.Body).Decode(&req)
if err != nil && err != io.EOF {
resp.write(w, err)
return
}
_, err = h.API.CreateIndex(r.Context(), indexName, req.Options)
resp.write(w, err)
}
@ -647,42 +642,34 @@ func (h *Handler) handlePostField(w http.ResponseWriter, r *http.Request) {
resp := successResponse{}
err := func() error {
// Decode request.
var req postFieldRequest
dec := json.NewDecoder(r.Body)
dec.DisallowUnknownFields()
err := dec.Decode(&req)
if err == io.EOF {
// If no data was provided (EOF), we still create the field
// with default values.
} else if err != nil {
return err
}
// Decode request.
var req postFieldRequest
dec := json.NewDecoder(r.Body)
dec.DisallowUnknownFields()
err := dec.Decode(&req)
if err != nil && err != io.EOF {
resp.write(w, err)
return
}
// Validate field options.
if err := req.Options.validate(); err != nil {
return err
}
// Validate field options.
if err := req.Options.validate(); err != nil {
resp.write(w, err)
return
}
// Convert json options into functional options.
var fos pilosa.FieldOption
switch req.Options.Type {
case pilosa.FieldTypeSet:
fos = pilosa.OptFieldTypeSet(*req.Options.CacheType, *req.Options.CacheSize)
case pilosa.FieldTypeInt:
fos = pilosa.OptFieldTypeInt(*req.Options.Min, *req.Options.Max)
case pilosa.FieldTypeTime:
fos = pilosa.OptFieldTypeTime(*req.Options.TimeQuantum)
}
_, err = h.API.CreateField(r.Context(), indexName, fieldName, fos)
if err != nil {
return err
}
return nil
}()
// Convert json options into functional options.
var fos pilosa.FieldOption
switch req.Options.Type {
case pilosa.FieldTypeSet:
fos = pilosa.OptFieldTypeSet(*req.Options.CacheType, *req.Options.CacheSize)
case pilosa.FieldTypeInt:
fos = pilosa.OptFieldTypeInt(*req.Options.Min, *req.Options.Max)
case pilosa.FieldTypeTime:
fos = pilosa.OptFieldTypeTime(*req.Options.TimeQuantum)
}
_, err = h.API.CreateField(r.Context(), indexName, fieldName, fos)
resp.write(w, err)
}

View file

@ -21,6 +21,7 @@ import (
"github.com/pilosa/pilosa"
"github.com/pilosa/pilosa/test"
"github.com/pkg/errors"
)
// ShardWidth is a helper reference to use when testing.
@ -196,8 +197,8 @@ func TestIndex_DeleteField(t *testing.T) {
// Delete again to make sure it errors.
err := index.DeleteField("f")
if err == nil || err.Error() != pilosa.ErrFieldNotFound.Error() {
t.Fatal(err)
if !isNotFoundError(err) {
t.Fatalf("expected 'field not found' error, got: %#v", err)
}
}
@ -215,3 +216,9 @@ func TestIndex_InvalidName(t *testing.T) {
t.Fatalf("unexpected index name %v", index)
}
}
func isNotFoundError(err error) bool {
root := errors.Cause(err)
_, ok := root.(pilosa.NotFoundError)
return ok
}

View file

@ -71,6 +71,11 @@ type ApiMethodNotAllowedError struct {
error
}
// NewApiMethodNotAllowedError returns err wrapped in an ApiMethodNotAllowedError.
func NewApiMethodNotAllowedError(err error) ApiMethodNotAllowedError {
return ApiMethodNotAllowedError{err}
}
// BadRequestError wraps an error value to signify that a request could not be
// read, decoded, or parsed such that in an HTTP scenario, http.StatusBadRequest
// would be returned.
@ -90,12 +95,22 @@ type ConflictError struct {
error
}
// NewConflictError returns err wrapped in a ConflictError.
func NewConflictError(err error) ConflictError {
return ConflictError{err}
}
// NotFoundError wraps an error value to signify that a resource was not found
// such that in an HTTP scenario, http.StatusNotFound would be returned.
type NotFoundError struct {
error
}
// NewNotFoundError returns err wrapped in a NotFoundError.
func NewNotFoundError(err error) NotFoundError {
return NotFoundError{err}
}
// Regular expression to validate index and field names.
var nameRegexp = regexp.MustCompile(`^[a-z][a-z0-9_-]{0,63}$`)