Remove old sub-resource syntax

[tgirod]

I want a resource with an array of a sub-resource, and everything properly checked for validity.

As a preamble, I found two ways of declaring a sub-resource and I fail to see the difference between them. First approach:

Fields: schema.Fields{
    "sub": {
        Schema: &subresource
    },
}

Second approach:

Fields: schema.Fields{
    "sub": {
        Validator: &schema.Object{
            Schema: &subresource,
        },
    },
}

With that in mind, I found that using the first approach works when the subresource is a regular field, but not when declared inside an array. OTOH the second approach works in both cases.

var inner = schema.Schema{
    Fields: schema.Fields{
        "foo": {
            Required:  true,
            Validator: &schema.Integer{},
        },
    },
}

// approach using Validator:Object
var legit = schema.Schema{
    Fields: schema.Fields{
        "id": schema.IDField,
        "single": {
            Validator: &schema.Object{
                Schema: &inner,
            },
        },
        "array": {
            Validator: &schema.Array{
                Values: schema.Field{
                    Validator: &schema.Object{
                        Schema: &inner,
                    },
                },
            },
        },
    },
}

// approach using Field.Schema
var bug = schema.Schema{
    Fields: schema.Fields{
        "id": schema.IDField,
        "single": {
            Schema: &inner,
        },
        "array": {
            Validator: &schema.Array{
                Values: schema.Field{
                    Schema: &inner,
                },
            },
        },
    },
}

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

    index.Bind("bug", bug, mem.NewHandler(), resource.DefaultConf)
    index.Bind("legit", legit, mem.NewHandler(), resource.DefaultConf)

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

    // Bind the API under /api/ path
    http.Handle("/api/", http.StripPrefix("/api/", api))

    // Serve it
    log.Print("Serving API on http://localhost:8080")
    if err := http.ListenAndServe(":8080", nil); err != nil {
        log.Fatal(err)
    }
}

Running that code and POSTing some data, here is what I see:

# using the validator Object

$ echo '{"single":{"foo":42}}' | http POST :8080/api/legit -p b
{
    "id": "bjdrmssfhc2qebh96omg", 
    "single": {
        "foo": 42
    }
}
# OK. POSTing bogus data properly fails

$ echo '{"array":[{"foo":42}]}' | http POST :8080/api/legit -p b
{
    "array": [
        {
            "foo": 42
        }
    ], 
    "id": "bjdrn6cfhc2qebh96on0"
}
# OK. POSTing bogus data properly fails

# using the Schema parameter instead

$ echo '{"single":{"foo":42}}' | http POST :8080/api/bug -p b
{
    "id": "bjdrndcfhc2qebh96ong", 
    "single": {
        "foo": 42
    }
}
# OK. POSTing bogus data properly fails

$ echo '{"array":[{"foo":42}]}' | http POST :8080/api/bug -p b
{
    "code": 422, 
    "issues": {
        "single": [
            {
                "foo": [
                    "required"
                ]
            }
        ]
    }, 
    "message": "Document contains error(s)"
}
# BUG?

[smyrman]

This is related to #77.

Long story short, the second approach (the newest syntax) is recommended, and should work for more cases. The first approach (the earliest implementation of sub-schemas) should be deprecated, but some code still exists that only considers the earlier syntax.

My plan for solving this and other issues, is to do a proto-type for a new schema package design. However my progress have been (and is going to be) extremely slow.

[smyrman]

Since the scope of #77 has grown, and my progress is expected to be very slow btw. It might be good to keep this issue open, if anyone wants to fix it against the current design.

I would recommend to remove the Schema property in the schema.Field type, and updates the documentation to recommend using the Object validator approach for sub-schemas. When doing this, it's a good opportunity to spot any code that relies on the removed field, and rewrite it to work with an Object validator (preferably through use of interfaces, so that AllOff/AnyOff/Dict etc. will also work).

I am happy to accept/review any PR with such changes, but am afraid I won't have time to write the code.

[tgirod]

@smyrman thanks for the explanation. I might have a swing at that next month.

[smyrman]

Renamed to reflect the work that is to be done.