Skip to content

Allow schema.Reference to validate as normal field #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 10, 2017
Merged

Allow schema.Reference to validate as normal field #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b54368e
Fix incorrect behaviour for AnyOf validator
smyrman Jul 7, 2017
4e1b0c9
Fix data race in DefalutResponseFormatter
smyrman Jul 7, 2017
07df663
schema.Reference now validated as normal field.
smyrman Mar 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions graphql/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/graphql-go/graphql"
"github.com/rs/rest-layer/resource"
"github.com/rs/rest-layer/schema"
)

// Handler is a net/http compatible handler used to serve the configured GraphQL
Expand All @@ -21,7 +20,7 @@ type Handler struct {
// NewHandler creates an new GraphQL API HTTP handler with the specified
// resource index.
func NewHandler(i resource.Index) (*Handler, error) {
if c, ok := i.(schema.Compiler); ok {
if c, ok := i.(resource.Compiler); ok {
if err := c.Compile(); err != nil {
return nil, err
}
Expand Down
3 changes: 1 addition & 2 deletions graphql/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (

"github.com/graphql-go/graphql"
"github.com/rs/rest-layer/resource"
"github.com/rs/rest-layer/schema"
)

func newRootQuery(idx resource.Index) *graphql.Object {
t := types{}
if c, ok := idx.(schema.Compiler); ok {
if c, ok := idx.(resource.Compiler); ok {
if err := c.Compile(); err != nil {
log.Fatal(err)
}
Expand Down
91 changes: 64 additions & 27 deletions resource/index.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,25 @@ import (
type Index interface {
// Bind a new resource at the "name" endpoint
Bind(name string, s schema.Schema, h Storer, c Conf) *Resource
// GetResource retrives a given resource by it's path.
// For instance if a resource user has a sub-resource posts,
// a users.posts path can be use to retrieve the posts resource.
// GetResource retrieves a given resource by it's path. For instance if a
// resource "user" has a sub-resource "posts", a "users.posts" path can be
// use to retrieve the posts resource.
//
// If a parent is given and the path starts with a dot, the lookup is started at the
// parent's location instead of root's.
// If a parent is given and the path starts with a dot, the lookup is
// started at the parent's location instead of root's.
GetResource(path string, parent *Resource) (*Resource, bool)
// GetResources returns first level resources
// GetResources returns first level resources.
GetResources() []*Resource
}

// Compiler is an optional interface for Index that's task is to prepare the
// index for usage. When the method exists, it's automatically called by
// rest.NewHandler(). When the resource package is used without the rest
// package, it's the user's responsibilty to call this method.
type Compiler interface {
Compile() error
}

// index is the root of the resource graph.
type index struct {
resources subResources
Expand All @@ -38,26 +46,34 @@ func NewIndex() Index {
}

// Bind a resource at the specified endpoint name.
func (r *index) Bind(name string, s schema.Schema, h Storer, c Conf) *Resource {
assertNotBound(name, r.resources, nil)
func (i *index) Bind(name string, s schema.Schema, h Storer, c Conf) *Resource {
assertNotBound(name, i.resources, nil)
sr := new(name, s, h, c)
r.resources.add(sr)
i.resources.add(sr)
return sr
}

// Compile the resource graph and report any error.
func (r *index) Compile() error {
return compileResourceGraph(r.resources)
func (i *index) Compile() error {
for _, r := range i.resources {
if err := r.Compile(refChecker{i}); err != nil {
sep := "."
if err.Error()[0] == ':' {
sep = ""
}
return fmt.Errorf("%s%s%s", r.name, sep, err)
}
}
return nil
}

// GetResource retrives a given resource by it's path.
// For instance if a resource user has a sub-resource posts,
// a users.posts path can be use to retrieve the posts resource.
// GetResource retrieves a given resource by it's path. For instance if a resource "user" has a sub-resource "posts", a
// "users.posts" path can be use to retrieve the posts resource.
//
// If a parent is given and the path starts with a dot, the lookup is started at the
// parent's location instead of root's.
func (r *index) GetResource(path string, parent *Resource) (*Resource, bool) {
resources := r.resources
func (i *index) GetResource(path string, parent *Resource) (*Resource, bool) {
resources := i.resources
if len(path) > 0 && path[0] == '.' {
if parent == nil {
// If field starts with a dot and no parent is given, fail the lookup.
Expand All @@ -83,21 +99,42 @@ func (r *index) GetResource(path string, parent *Resource) (*Resource, bool) {
}

// GetResources returns first level resources.
func (r *index) GetResources() []*Resource {
return r.resources
func (i *index) GetResources() []*Resource {
return i.resources
}

func compileResourceGraph(resources subResources) error {
for _, r := range resources {
if err := r.Compile(); err != nil {
sep := "."
if err.Error()[0] == ':' {
sep = ""
// resourceLookup provides a wrapper for Index that implements the schema.ReferenceChecker interface.
type refChecker struct {
index Index
}

// ReferenceChecker implements the schema.ReferenceChecker interface.
func (rc refChecker) ReferenceChecker(path string) schema.FieldValidator {
rsc, exists := rc.index.GetResource(path, nil)
if !exists {
return nil
}
validator := rsc.Schema().Fields["id"].Validator

return schema.FieldValidatorFunc(func(value interface{}) (interface{}, error) {
var id interface{}
var err error

if validator != nil {
id, err = validator.Validate(value)
if err != nil {
return nil, err
}
return fmt.Errorf("%s%s%s", r.name, sep, err)
} else {
id = value
}
}
return nil

_, err = rsc.Get(context.TODO(), id)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why TODO over Background?

Copy link
Collaborator Author

@smyrman smyrman Jul 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the docs on context.TODO():

Code should use context.TODO when it's unclear which Context to use or it is not yet available (because the surrounding function has not yet been extended to accept a Context parameter).

From the docs on context.Background():

It is typically used by the main function, initialization, and tests, and as the top-level Context for incoming requests.

I feel this case is closer to the description offered for context.TODO() than what's offered by context.Background(). I think if we where to use the right context here, we should get it passed in from somewhere so that when using the rest package, we use the context created for a new request, and not a new context created later.

if err != nil {
return nil, err
}
return id, nil
})
}

// assertNotBound asserts a given resource name is not already bound.
Expand Down
65 changes: 47 additions & 18 deletions resource/index_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,35 +30,64 @@ func TestIndexBind(t *testing.T) {

func TestIndexCompile(t *testing.T) {
r, ok := NewIndex().(*index)
if assert.True(t, ok) {
s := schema.Schema{Fields: schema.Fields{"f": {}}}
r.Bind("foo", s, nil, DefaultConf)
assert.NoError(t, r.Compile())
if !assert.True(t, ok) {
return
}
s := schema.Schema{Fields: schema.Fields{"f": {}}}
r.Bind("foo", s, nil, DefaultConf)
assert.NoError(t, r.Compile())
}

func TestIndexCompileError(t *testing.T) {
r, ok := NewIndex().(*index)
if assert.True(t, ok) {
s := schema.Schema{
Fields: schema.Fields{
"f": {Validator: schema.String{Regexp: "["}},
},
}
r.Bind("foo", s, nil, DefaultConf)
assert.Error(t, r.Compile())
if !assert.True(t, ok) {
return
}
s := schema.Schema{
Fields: schema.Fields{
"f": {Validator: schema.String{Regexp: "["}},
},
}
r.Bind("foo", s, nil, DefaultConf)
assert.Error(t, r.Compile())
}

func TestIndexCompileSubError(t *testing.T) {
r, ok := NewIndex().(*index)
if assert.True(t, ok) {
foo := r.Bind("foo", schema.Schema{Fields: schema.Fields{"f": {}}}, nil, DefaultConf)
bar := foo.Bind("bar", "f", schema.Schema{Fields: schema.Fields{"f": {}}}, nil, DefaultConf)
s := schema.Schema{Fields: schema.Fields{"f": {Validator: &schema.String{Regexp: "["}}}}
bar.Bind("baz", "f", s, nil, DefaultConf)
assert.EqualError(t, r.Compile(), "foo.bar.baz: schema compilation error: f: invalid regexp: error parsing regexp: missing closing ]: `[`")
if !assert.True(t, ok) {
return
}
foo := r.Bind("foo", schema.Schema{Fields: schema.Fields{"f": {}}}, nil, DefaultConf)
bar := foo.Bind("bar", "f", schema.Schema{Fields: schema.Fields{"f": {}}}, nil, DefaultConf)
s := schema.Schema{Fields: schema.Fields{"f": {Validator: &schema.String{Regexp: "["}}}}
bar.Bind("baz", "f", s, nil, DefaultConf)
assert.EqualError(t, r.Compile(), "foo.bar.baz: schema compilation error: f: invalid regexp: error parsing regexp: missing closing ]: `[`")
}

func TestIndexCompileReferenceChecker(t *testing.T) {
i, ok := NewIndex().(*index)
if !assert.True(t, ok) {
return
}

i.Bind("b", schema.Schema{Fields: schema.Fields{"id": {}}}, nil, DefaultConf)
i.Bind("a", schema.Schema{Fields: schema.Fields{"ref": {
Validator: &schema.Reference{Path: "b"},
}}}, nil, DefaultConf)
assert.NoError(t, i.Compile())
}

func TestIndexCompileReferenceCheckerError(t *testing.T) {
i, ok := NewIndex().(*index)
if !assert.True(t, ok) {
return
}

i.Bind("b", schema.Schema{Fields: schema.Fields{"id": {}}}, nil, DefaultConf)
i.Bind("a", schema.Schema{Fields: schema.Fields{"ref": {
Validator: &schema.Reference{Path: "c"},
}}}, nil, DefaultConf)
assert.Error(t, i.Compile())
}

func TestIndexGetResource(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions resource/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ func (r *Resource) ParentField() string {
}

// Compile the resource graph and report any error.
func (r *Resource) Compile() error {
func (r *Resource) Compile(rc schema.ReferenceChecker) error {
// Compile schema and panic on any compilation error.
if c, ok := r.validator.Validator.(schema.Compiler); ok {
if err := c.Compile(); err != nil {
if err := c.Compile(rc); err != nil {
return fmt.Errorf(": schema compilation error: %s", err)
}
}
for _, r := range r.resources {
if err := r.Compile(); err != nil {
if err := r.Compile(rc); err != nil {
if err.Error()[0] == ':' {
// Check if I'm the direct ancestor of the raised sub-error.
return fmt.Errorf("%s%s", r.name, err)
Expand Down
3 changes: 1 addition & 2 deletions rest/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"net/http"

"github.com/rs/rest-layer/resource"
"github.com/rs/rest-layer/schema"
)

// Handler is a net/http compatible handler used to serve the configured REST
Expand All @@ -27,7 +26,7 @@ type methodHandler func(ctx context.Context, r *http.Request, route *RouteMatch)
// NewHandler creates an new REST API HTTP handler with the specified resource
// index.
func NewHandler(i resource.Index) (*Handler, error) {
if c, ok := i.(schema.Compiler); ok {
if c, ok := i.(resource.Compiler); ok {
if err := c.Compile(); err != nil {
return nil, err
}
Expand Down
5 changes: 0 additions & 5 deletions rest/method_item_patch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,6 @@ func itemPatch(ctx context.Context, r *http.Request, route *RouteMatch) (status
if len(errs) > 0 {
return 422, nil, &Error{422, "Document contains error(s)", errs}
}
// Check that fields with the Reference validator reference an existing
// object.
if e = checkReferences(ctx, doc, rsrc.Validator()); e != nil {
return e.Code, nil, e
}
if id, found := doc["id"]; found && id != original.ID {
return 422, nil, &Error{422, "Cannot change document ID", nil}
}
Expand Down
4 changes: 0 additions & 4 deletions rest/method_item_put.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ func itemPut(ctx context.Context, r *http.Request, route *RouteMatch) (status in
if len(errs) > 0 {
return 422, nil, &Error{422, "Document contains error(s)", errs}
}
// Check that fields with the Reference validator reference an existing object.
if err := checkReferences(ctx, doc, rsrc.Validator()); err != nil {
return err.Code, nil, err
}
if original != nil {
if id, found := doc["id"]; found && id != original.ID {
return 422, nil, &Error{422, "Cannot change document ID", nil}
Expand Down
6 changes: 0 additions & 6 deletions rest/method_post.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ func listPost(ctx context.Context, r *http.Request, route *RouteMatch) (status i
if len(errs) > 0 {
return 422, nil, &Error{422, "Document contains error(s)", errs}
}
// Check that fields with the Reference validator reference an existing
// object.
if err := checkReferences(ctx, doc, rsrc.Validator()); err != nil {
e = NewError(err)
return e.Code, nil, e
}
item, err := resource.NewItem(doc)
if err != nil {
e = NewError(err)
Expand Down
Loading