Fix for field validation on nested sub-resources

[pakali]

After @Dragomir-Ivanov added nested sub-resources I found that is some issue when you have multiple sub-resources and schema references, for example:

user = schema.Schema{
  Fields: schema.Fields{
    "id": schema.IDField,
  },
}

ticket = schema.Schema{
  Fields: schema.Fields{
    "id": schema.IDField,
    "user": {
      Required:   true,
      Filterable: true,
      Validator: &schema.Reference{
        Path: "users",
      },
    },
  },
}

message = schema.Schema{
    Fields: schema.Fields{
        "id": schema.IDField,
        "ticket": {
            Required:   true,
            Filterable: true,
            Validator: &schema.Reference{
                Path: "users.tickets",
            },
        },
    },
}

http POST :8080/api/users/[user-id]/tickets/[ticker-id]/messages

Give you error:

{
    "code": 422,
    "issues": {
        "user": [
            "invalid field"
        ]
    },
    "message": "Document contains error(s)"
}

Because base is filled by user, and ticket id, in message you dont have user field so error occured. In this case we will check base field exists in resource fields.

[Dragomir-Ivanov]

Hi @pakali , it seems that there are some failing tests. Can you come online at gophers.slack.com, channel #rest-layer so we can discuss in a timely manner. You should paste also how you are building your resource graph. Did your case works with some more old rest-layer HEAD. Say: ca3afb7adc9e28f4c000023c1706c46b87822970

[pakali]

Hi @Dragomir-Ivanov, tests fails because this commit change logic of application, application after this commit ignore fields that doesnt exists in Schema (so if you pass in tests additional non-exist field as base field then the error doesn't occured and data will be processed by application). This need to be discussed.

You cannot reproduce this problem on older version of rest-layer (like ca3afb7adc9e28f4c000023c1706c46b87822970) because there is no nested sub-resources there.

I will join to #slack soon and we will discuss this problem.

[Dragomir-Ivanov]

@pakali There were nested sub-resources before, however they are linked with schema.Connection instead of Reference. Can you paste you are builidng your resource graph here. Note my PR message on #230, on example similar resource graph .

[pakali]

users := index.Bind("users", user, mongo.NewHandler(session, db, "users"), resource.Conf{
  AllowedModes: resource.ReadWrite,
})

users.Bind("tickets", "user", ticket, mongo.NewHandler(session, db, "tickets"), resource.Conf{
  AllowedModes: resource.ReadWrite,
}).Bind("messages", "ticket", message, mongo.NewHandler(session, db, "messages"), resource.Conf{
  AllowedModes: resource.ReadWrite,
})

There is problem only with saving data on endpoint like: http POST :8080/api/users/[user-id]/tickets/[ticker-id]/messages Because messages get user id, ticket id, message id and body of message - In message schema we dont have user reference thats why we get this validation error.

[Dragomir-Ivanov]

Okay, I can't say for certain, but can you remove your schema.Reference fields from definitions. Rest-layer will put the second parameter of schema.Bind as a filed of schema.Connection type.

user = schema.Schema{
  Fields: schema.Fields{
    "id": schema.IDField,
  },
}

ticket = schema.Schema{
  Fields: schema.Fields{
    "id": schema.IDField,
  },
}

message = schema.Schema{
    Fields: schema.Fields{
        "id": schema.IDField,
    },
}

[pakali]

When I removed references then I get error: Cannot bindtickets' as sub-resource: field user' does not exist in the sub-resource'

I will read about schema.Connection and give you answer about result of this case.

[Dragomir-Ivanov]

Hmm, schema.Connection is implicit, you are NOT supposed to add it in your defintions. Look for the schema.Bind() method, it puts it in the source schema. I will definitely look more deeply for your problems, but will be later this evening.

[pakali]

I will check this problem with some old version of rest-layer and schema.Connection approach and give you results but tommorow. Im almost sure this will fail too because I suppose every sub-resource give ID to main body of last sub-resource. So if you have just two sub-resources there is no problem with that Ticket get User ID and that how it should works, but if you get 3 nested sub-resources then Message get Ticket and User ID but only Ticket field is on body of Message. If this validation wasnt update last time then schema.Connection should give same error like sub-resources based on references.

[Dragomir-Ivanov]

Can you put your Go source somewhere, along with the curl commands you are testing, so I can reproduce locally. Connection and Resource are like Has-{One,Many} and BelogsTo relations. They are different beasts used for different things. Will explain later.

[pakali]

Sure:

// +build go1.7

var (
    // Define a user resource schema
    user = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
        },
    }

    // Define a post resource schema
    ticket = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
            "user": {
                Required:   true,
                Filterable: true,
                Validator: &schema.Reference{
                    Path: "users",
                },
            },
        },
    }

    message = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
            "ticket": {
                Required:   true,
                Filterable: true,
                Validator: &schema.Reference{
                    Path: "users.tickets",
                },
            },
        },
    }
)

func main() {
    session, err := mgo.Dial("127.0.0.1:27017")
    db := "test"

    if err != nil {
        panic(err)
    }

    index := resource.NewIndex()

    index.Bind("users", user, mongo.NewHandler(session, db, "users"), resource.Conf{
        AllowedModes: resource.ReadWrite,
    }).Bind("tickets", "user", ticket, mongo.NewHandler(session, db, "tickets"), resource.Conf{
        AllowedModes: resource.ReadWrite,
    }).Bind("messages", "ticket", message, mongo.NewHandler(session, db, "messages"), resource.Conf{
        AllowedModes: resource.ReadWrite,
    })

    api, err := rest.NewHandler(index)
    if err != nil {
        log.Fatal().Msgf("Invalid API configuration: %s", err)
    }

    c := alice.New()

    http.Handle("/api/", c.Then(http.StripPrefix("/api/", api)))

    if err := http.ListenAndServe(":8080", nil); err != nil {
        log.Fatal().Err(err).Msg("")
    }
}

After that:

$ http POST :8080/api/users id=test
HTTP/1.1 201 Created
Content-Length: 13
Content-Location: users/test
Content-Type: application/json
Date: Sat, 19 Jan 2019 16:37:32 GMT
Etag: W/"38c5a156b60082ce31bcd0549f49b6cb"
Last-Modified: Sat, 19 Jan 2019 16:37:32 GMT

{
    "id": "test"
}

$ http POST :8080/api/users/test/tickets id=test
HTTP/1.1 201 Created
Content-Length: 27
Content-Location: users/test/tickets/test
Content-Type: application/json
Date: Sat, 19 Jan 2019 16:37:38 GMT
Etag: W/"82acdab886e7441899b4219166acd897"
Last-Modified: Sat, 19 Jan 2019 16:37:38 GMT

{
    "id": "test",
    "user": "test"
}

$ http POST :8080/api/users/test/tickets/test/messages id=test
HTTP/1.1 422 Unprocessable Entity
Content-Length: 87
Content-Type: application/json
Date: Sat, 19 Jan 2019 16:37:46 GMT

{
    "code": 422,
    "issues": {
        "user": [
            "invalid field"
        ]
    },
    "message": "Document contains error(s)"
}

[Dragomir-Ivanov]

Hi @pakali It seems that your code doesn't work even for the commit ID I gave you, before my work was merged. I will try your fix and think about if it is reasonable, or there is some more deeper work to be done. Good catch!

[Dragomir-Ivanov]

I got your code working @pakali . The patched I used over (https://github.com/rs/rest-layer/commit/ca3afb7adc9e28f4c000023c1706c46b87822970) is this: resource_path.go

// Values returns all the key=value pairs defined by the resource path.
func (p ResourcePath) Values() map[string]interface{} {
    // v := map[string]interface{}{}
    // for _, rp := range p {
    //  if _, found := v[rp.Field]; !found && rp.Value != nil {
    //      v[rp.Field] = rp.Value
    //  }
    // }
    // return v

    // Return only the last key=value pair
    v := map[string]interface{}{}
    var lastField string
    var lastValue interface{}
    for _, rp := range p {
        if _, found := v[rp.Field]; !found && rp.Value != nil {
                        // id needs to be there
            if rp.Field == "id" {
                v[rp.Field] = rp.Value
            } else {
                lastField = rp.Field
                lastValue = rp.Value
            }
        }
    }
    if lastValue != nil {
        v[lastField] = lastValue
    }
    return v
}

I don't claim that this is the right way to fix it, however it is a starting point to work on. @smyrman you input is always welcomed.

@pakali Here is the code:

package main

import (
    "net/http"

    "github.com/justinas/alice"
    "github.com/rs/rest-layer-mem"
    "github.com/rs/rest-layer/resource"
    "github.com/rs/rest-layer/rest"
    "github.com/rs/rest-layer/schema"
    "github.com/rs/zerolog/log"
)

var (
    // Define a user resource schema
    user = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
        },
    }

    // Define a post resource schema
    ticket = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
            "user": {
                Validator: &schema.Reference{
                    Path: "users",
                },
            },
        },
    }

    message = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
            "ticket": {
                Validator: &schema.Reference{
                    Path: "users.tickets",
                },
            },
        },
    }
)

func main() {
    index := resource.NewIndex()

    index.Bind("users", user, mem.NewHandler(), resource.Conf{
        AllowedModes: resource.ReadWrite,
    }).Bind("tickets", "user", ticket, mem.NewHandler(), resource.Conf{
        AllowedModes: resource.ReadWrite,
    }).Bind("messages", "ticket", message, mem.NewHandler(), resource.Conf{
        AllowedModes: resource.ReadWrite,
    })

    api, err := rest.NewHandler(index)
    if err != nil {
        log.Fatal().Msgf("Invalid API configuration: %s", err)
    }

    c := alice.New()

    http.Handle("/api/", c.Then(http.StripPrefix("/api/", api)))

    if err := http.ListenAndServe(":8080", nil); err != nil {
        log.Fatal().Err(err).Msg("")
    }
}

POST http://localhost:8080/api/users

{
  "id":"test"
}

###
POST http://localhost:8080/api/users/test/tickets

{
  "id":"test"
}

###
POST http://localhost:8080/api/users/test/tickets/test/messages

{
  "id":"test"
}

###

If you checkout my `rest-layer` repo, you will be able to use this query
GET  http://localhost:8080/api/users?fields=tickets{messages}

[pakali]

Thanks for this, both fixes yours and mine working fine, probably your fix is more proper one.

[Dragomir-Ivanov]

Yes @pakali , I think the same. But lets wait for some input from the owners of the project, since I don't have much insight in this part of the code.

[smyrman]

For triple-nested resources, it was always necessary to add all of the outer keys to the inner resource. I.e.:

message = schema.Schema{
        Fields: schema.Fields{
            "id": {
                Required:   true,
                Filterable: true,
                Validator:  &schema.String{},
            },
            "users": {
                Validator: &schema.Reference{
                    Path: "users",
                },
            },
            "ticket": {
                Validator: &schema.Reference{
                    Path: "users.tickets",
                },
            },
        },
    }

By setting the fields ReadOnly, you would avoid items being movable via PATCH or PUT. If we are to allow not having the outer resource in the inner resource (e.g. skipping users), it would be nice if it still worked to do so. Thus the solution should presumably add all route values that are in the resource schema (ignoring other keys). I.e. just updating the last field in the path, is not something I would recommend. This may involve updating some unit-test schemas so that the tests pass.

[smyrman]

Sorry for a very late reply on this one.

[Dragomir-Ivanov]

Sorry for a very late reply on this one.

Thanks Sindre and don't worry.

Thus the solution should presumably add all route values that are in the resource schema

you mean the target resource, i.e. the last one in the url path?

[smyrman]

you mean the target resource, i.e. the last one in the url path?

I mean if we do POST /users/:id/tickets/:id/messages,

• "ticket" (or which ever field name was used in tickets.Bind(...)) gets set in the message item base -- will always be in the message schema, or the Bind would not have worked.
• "user" (or which ever field name was used in users.Bind(...)) gets set in then message item base if present in the message schema.

The last point probably isn't super-important, but would keep current schemas working. What do you think @Dragomir-Ivanov?

[Dragomir-Ivanov]

@smyrman Yes, I think I understand. Will make a PR later today. Thanks, and please take a look at #232 when have some free time. I think that it is a bite-sized one.