Skip to content

Commit

Permalink
Empty string constraint fixes (flipt-io#193)
Browse files Browse the repository at this point in the history
* Fix issue where != would evaluate to true if constraint value was not present
* Require value for constraints unless operator is one of [empty, not empty, present, not present]
* Move validation into middleware
* Move errors into own package
  • Loading branch information
markphelps authored Nov 28, 2019
1 parent df73704 commit 434a58b
Show file tree
Hide file tree
Showing 30 changed files with 2,029 additions and 1,719 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ cover: test ## Run all the tests and opens the coverage report
.PHONY: fmt
fmt: ## Run gofmt and goimports on all go files
@echo ">> running gofmt"
@find . -name '*.go' -not -wholename './rpc/*' -not -wholename './ui/*' -not -wholename './swagger/*' | while read -r file; do gofmt -w -s "$$file"; goimports -w "$$file"; done
@find . -name '*.go' | while read -r file; do gofmt -w -s "$$file"; goimports -w "$$file"; done

.PHONY: lint
lint: ## Run all the linters
Expand Down
3 changes: 2 additions & 1 deletion cmd/flipt/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,12 @@ func execute() error {
srv = server.New(logger, builder, db, serverOpts...)

grpcOpts = append(grpcOpts, grpc_middleware.WithUnaryServerChain(
grpc_recovery.UnaryServerInterceptor(),
grpc_ctxtags.UnaryServerInterceptor(),
grpc_logrus.UnaryServerInterceptor(logger),
grpc_prometheus.UnaryServerInterceptor,
srv.ErrorUnaryInterceptor,
grpc_recovery.UnaryServerInterceptor(),
srv.ValidationUnaryInterceptor,
))

if cfg.Server.Protocol == config.HTTPS {
Expand Down
2 changes: 2 additions & 0 deletions codecov.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ignore:
- "rpc/flipt.pb.*"
55 changes: 55 additions & 0 deletions errors/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package errors

import (
"errors"
"fmt"
)

// New creates a new error with errors.New
func New(s string) error {
return errors.New(s)
}

// ErrNotFound represents a not found error
type ErrNotFound string

// ErrNotFoundf creates an ErrNotFound using a custom format
func ErrNotFoundf(format string, args ...interface{}) error {
return ErrNotFound(fmt.Sprintf(format, args...))
}

func (e ErrNotFound) Error() string {
return fmt.Sprintf("%s not found", string(e))
}

// ErrInvalid represents an invalid error
type ErrInvalid string

// ErrInvalidf creates an ErrInvalid using a custom format
func ErrInvalidf(format string, args ...interface{}) error {
return ErrInvalid(fmt.Sprintf(format, args...))
}

func (e ErrInvalid) Error() string {
return string(e)
}

// ErrValidation is a validation error for a specific field and reason
type ErrValidation struct {
field string
reason string
}

func (e ErrValidation) Error() string {
return fmt.Sprintf("invalid field %s: %s", e.field, e.reason)
}

// InvalidFieldError creates an ErrInvalidField for a specific field and reason
func InvalidFieldError(field, reason string) error {
return ErrValidation{field, reason}
}

// EmptyFieldError creates an ErrInvalidField for an empty field
func EmptyFieldError(field string) error {
return InvalidFieldError(field, "must not be empty")
}
3 changes: 2 additions & 1 deletion rpc/flipt.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

67 changes: 67 additions & 0 deletions rpc/operators.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package flipt

const (
OpEQ = "eq"
OpNEQ = "neq"
OpLT = "lt"
OpLTE = "lte"
OpGT = "gt"
OpGTE = "gte"
OpEmpty = "empty"
OpNotEmpty = "notempty"
OpTrue = "true"
OpFalse = "false"
OpPresent = "present"
OpNotPresent = "notpresent"
OpPrefix = "prefix"
OpSuffix = "suffix"
)

var (
ValidOperators = map[string]struct{}{
OpEQ: {},
OpNEQ: {},
OpLT: {},
OpLTE: {},
OpGT: {},
OpGTE: {},
OpEmpty: {},
OpNotEmpty: {},
OpTrue: {},
OpFalse: {},
OpPresent: {},
OpNotPresent: {},
OpPrefix: {},
OpSuffix: {},
}
NoValueOperators = map[string]struct{}{
OpEmpty: {},
OpNotEmpty: {},
OpPresent: {},
OpNotPresent: {},
}
StringOperators = map[string]struct{}{
OpEQ: {},
OpNEQ: {},
OpEmpty: {},
OpNotEmpty: {},
OpPrefix: {},
OpSuffix: {},
}
NumberOperators = map[string]struct{}{
OpEQ: {},
OpNEQ: {},
OpLT: {},
OpLTE: {},
OpGT: {},
OpGTE: {},
OpPresent: {},
OpNotPresent: {},
}
BooleanOperators = map[string]struct{}{
OpTrue: {},
OpFalse: {},
OpPresent: {},
OpNotPresent: {},
}
)
Loading

0 comments on commit 434a58b

Please sign in to comment.