Oudwins / zog

Go with simple schema validation
Other
81 stars 1 forks source link

Validate slices of structs #8

Open alexluong opened 4 weeks ago

alexluong commented 4 weeks ago

Hi, it seems this use case isn't supported yet. I ran into an error when trying this out. Sharing the recreation below.

On a similar note, does the zog/zhttp package support nested objects & arrays as well?


package main

import (
    "fmt"

    z "github.com/Oudwins/zog"
)

type User struct {
    Name string
}

type Team struct {
    Users []User
}

var userSchema = z.Struct(z.Schema{
    "name": z.String().Required(),
})

var teamSchema = z.Struct(z.Schema{
    "users": z.Slice(userSchema),
})

var data = map[string]interface{}{
    "users": []interface{}{
        map[string]interface{}{
            "name": "Jane",
        },
        map[string]interface{}{
            "name": "John",
        },
    },
}

func main() {
    var team Team

    errsMap := teamSchema.Parse(z.NewMapDataProvider(data), &team)
    if errsMap != nil {
        fmt.Println("Errors:")
        for key, value := range errsMap {
            fmt.Println(key, value)
        }
    }

    fmt.Println("\nTeam:")
    fmt.Println(team)
}
$ go version
go version go1.22.6 darwin/arm64
$ go run main.go
Errors:
$first [failed to validate field]
users.0 [failed to validate field]
users.1 [failed to validate field]

Team:
{[{} {}]}
Oudwins commented 4 weeks ago

Let me look into this. Its tecnically not supported yet but I did think it was working. @alexluong Thanks again :P

alexluong commented 4 weeks ago

No worries!

I took a look and noticed that for slice, when you process each item, the item passed to the struct is not a pointer but a map instead. That's what I've found but don't know how to make it work just yet.

Oudwins commented 4 weeks ago

@alexluong Actually I was wrong. This was not implemented. I have just implemented it & all tests are passing. If you need it I can merge it now.

alexluong commented 4 weeks ago

@Oudwins that would be great, but I can also pull your code locally and run my project against that branch if you prefer more time to polish.

Do you know if the zog/zhttp package can support it as well?

Oudwins commented 4 weeks ago

@alexluong The zhttp package is literally 10 lines of code. It will support it yes.

Here is the PR, it would probably be fine to merge. But I am thinking some refactoring is in order to better support this use case: https://github.com/Oudwins/zog/pull/10

Do note that the zhttp package only has a provider for form data & query params currently. A provider for json is actually already done. I just haven't had the time to properly test it.

This would be the code if you want to use it:

func NewJsonDataProvider(r *http.Request) (p.DataProvider, error) {
    var data map[string]any
    decod := json.NewDecoder(r.Body)
    err := decod.Decode(&data)
    if err != nil {
        return nil, err
    }
    return z.NewMapDataProvider(data), nil
}
alexluong commented 4 weeks ago

@Oudwins thanks, I'll give it a try and let you know how it goes.

Also I'm testing this for nested structs within slices for FormData, which seems to be a bit tricky so that's why I'm a bit concerned with the zhttp package. Will let you know how it goes. If you have an example HTML setup or something to test that would be great too.

Oudwins commented 4 weeks ago

@alexluong, Damm. Actually, I don't think it will work. Could you give me an example of what you are trying to do?

I think it won't work because there is no nested structure in form data

alexluong commented 4 weeks ago

Yeah for sure. So imagine you build a form that can accept an array of objects. Say you want to invite multiple users at once.

The expected payload should be

type Payload struct {
  Users []struct {
    Name string
    Email string
  }
}

How would you approach this? I think this isn't an anti-pattern or anything, is it?

To be honest, I'm more of a SPA / REST person so this is usually pretty simple. I'm trying to embrace the web standard with FormData, so trying a simple Go x HTMX project and this is what I'm struggling with atm 😅

Oudwins commented 4 weeks ago

Why are you not doing this:

type Payload struct {
  UserEmails: []string
}

I am struggling to imagine what you expect the form data to look like for this:

type Payload struct {
  Users []struct {
    Name string
    Email string
  }
}

If not using a struct doesn't work. We can think about if there is a clean way of implementing this. Otherwise you can take a look at the zhttp data provider and make your own.

It shouldn't be very difficult

alexluong commented 4 weeks ago

@Oudwins that's because this is a simplified example. In the real use case, the struct may go a bit deeper, with the possibility of having more nested struct of its own. I try not to go too deep but I've come across this pattern a few times. Just wondering how other folks solve this problem.

Maybe I'm more used to JS / Rails having custom helpers to parse it. I should be able to figure something out tho, thanks!

Oudwins commented 4 weeks ago

@alexluong I haven't ever come across something like this for form data. With json yes of course all the time. If you show me an example of what the form data looks like before it is parsed by one of the js /rails libraries. I can look into it. Otherwise you just have to make a function that will convert the form data into something like this

{
 Users: []Map[string]any
}
alexluong commented 4 weeks ago

This is one good discussion from the JS side on how to solve this problem: https://github.com/remix-run/remix/discussions/1541

For Ruby, Rack server has a built-in helper to parse nested objects like this: https://github.com/rack/rack/blob/main/lib/rack/query_parser.rb

Hope that helps. I'm trying to figure out what Go folks do for now and probably write a custom data provider here.

alexluong commented 4 weeks ago

I think the FormData looks something like this

{
  "users[][name]": ["John", "Jane"],
  "users[][email]": ["john@example.com", "jane@example.com"]
}
Oudwins commented 4 weeks ago

Okay, yea @alexluong, that makes sense. Its what I was thinking. Does remix support this out of the box?

I think this might make sense for a framework but I'm hesitant to add something that forces your form data to conform to a specific naming scheme in a library. I would need to think about this. If you want you can write a custom provider and we can go from there.

edit: ignore all this below ->

i think the syntax that would make most sense is something like

<!-- User 1 -->
<input name="users[].name" />
<input name="users[].email" />
<!-- User 2 -->
<input name="users[].name" />
<input name="users[].email" />

because go already parses form data into a map[string][]string

This is assuming that form data comes with a guaranteed order, if it doesn't that syntax wouldn't work

Oudwins commented 4 weeks ago

Actually. Reading more about it, this is something that they do support under the remix-validated-form

as per the docs:

      <MyInput name="firstName" label="First Name" />
      <MyInput name="lastName" label="Last Name" />
      <MyInput name="address.street" label="Street" />
      <MyInput name="address.city" label="City" />
      <MyInput name="phones[0].type" label="Phone 1 Type" />
      <MyInput name="phones[0].number" label="Phone 1 Number" />
      <MyInput name="phones[1].type" label="Phone 2 Type" />
      <MyInput name="phones[1].number" label="Phone 2 Number" />

I would be willing to add this to the zhttp package if you want to make a PR. Ignore my previous idea though, I think it would make sense to keep the same API.

alexluong commented 4 weeks ago

Yup, I've used remix-validated-form in the past for this use case.

Yeah, I can look more into it. I haven't messed around with struct tag and reflection much, so probably will need your help finalizing it but happy to spend some time on this.

Oudwins commented 4 weeks ago

@alexluong, I happy to help, but you actually don't need any struct tag reflection, that is handled inside zog automatically for you.

Just make a function that builds a map[string]any and pass that into the NewMapDataProvider() function.

So:

  1. Iterate over all keys in url.Values (the type in r.Form)
  2. For each key based on some heuristics add values to a map recursively

So for example, if you get something like this

map[string][]string{
"todo[0].name": []string{"alex"}
}

You want to make a slice of any & place this map at the 0s position:

map[string]any{
"name": "alex",
}
Oudwins commented 4 weeks ago

Does that make sense? We are creating a lot of garbage. But I don't think there is another reasonable way of getting the dx you want.

Oudwins commented 4 weeks ago

I have been thinking about this. I think there will probably be a way of supporting this use case in a much simpler way without having to create the data structure I was suggesting. But I need to think about the architecture quite a bit more, it will probably take same weeks to explore the idea

alexluong commented 4 weeks ago

That's alright, in that case I'll write a helper to parse form data into map and use NewMapDataProvider to make it work for now.

Oudwins commented 4 weeks ago

Sure, let me show you what we will do in the future though in case it helps inform the interface. Basically, Zog is already calculating the path of the current value for returning usable errors. Which means if we can give the data provider access to the current path it can use that to resolve the query.

Here is an example of the error paths generated for the code example from before: image

alexluong commented 4 weeks ago

@Oudwins gotcha, so you'll come up with some sort of logic to deduce the path for this case and the data provider will use that logic to construct the map accordingly?

alexluong commented 4 weeks ago

Sooo I haven't quite got this to work just yet, but I'm gonna call it for right now. I'm not 100% sure if I'll continue exploring this, given you're going to come up with an alternative approach anyway. For the time being, I'll handle this from the client side and make a JSON request to the server and use z.NewMapDataProvider() for it.

Leaving my WIP here in case anyone want to take it and run with it. I got some support from Mr.ChatGPT but I think there may be a fundamental error in the approach so I may have to rewrite quite a bit 😅. But anyway, this is the test file, I got deep struct to work, just need to support slices via TestParseSlices.

Code ```golang package parse import ( "fmt" "regexp" "strconv" "strings" "testing" "github.com/stretchr/testify/assert" ) func TestParseStruct(t *testing.T) { var formData = map[string]string{ "user.email": "john@example.com", "user.name": "John", } fmt.Println(formData) data := Parse(formData) user, ok := data["user"].(map[string]interface{}) if !ok { t.Errorf("Expected user to be a map, got %v", user) } assert.Equal(t, "john@example.com", user["email"]) assert.Equal(t, "John", user["name"]) fmt.Println(data) fmt.Print("\n\n\n") // deep struct map formData = map[string]string{ "user.email": "john@example.com", "user.name.first": "John", "user.name.last": "Doe", } fmt.Println(formData) data = Parse(formData) user, ok = data["user"].(map[string]interface{}) if !ok { t.Errorf("Expected user to be a map, got %v", user) } name, ok := user["name"].(map[string]interface{}) if !ok { t.Errorf("Expected name to be a map, got %v", user) } assert.Equal(t, user["email"], "john@example.com") assert.Equal(t, name["first"], "John") assert.Equal(t, name["last"], "Doe") fmt.Println(data) fmt.Print("\n\n\n") // 3 level deep formData = map[string]string{ "user.email": "john@example.com", "user.name.first.legal": "John", "user.name.first.nickname": "Jonny", "user.name.last": "Doe", } fmt.Println(formData) data = Parse(formData) fmt.Println(data) user, ok = data["user"].(map[string]interface{}) if !ok { t.Errorf("Expected user to be a map, got %v", user) } name, ok = user["name"].(map[string]interface{}) if !ok { t.Errorf("Expected name to be a map, got %v", user) } first, ok := name["first"].(map[string]interface{}) if !ok { t.Errorf("Expected first to be a map, got %v", name) } assert.Equal(t, "john@example.com", user["email"]) assert.Equal(t, "John", first["legal"]) assert.Equal(t, "Jonny", first["nickname"]) assert.Equal(t, "Doee", name["last"]) } func TestParseSlice(t *testing.T) { var formData = map[string]string{ "users[0]": "John", "users[1]": "Jane", } data := Parse(formData) users, ok := data["users"].([]string) fmt.Println(data["users"]) fmt.Println(users) if !ok { t.Errorf("Expected user to be a slice of string, got %v", users) } assert.Equal(t, users[0], "John") assert.Equal(t, users[1], "Jane") } func TestParseKeys(t *testing.T) { tests := []struct { input string expected []string }{ {"users[0].name", []string{"users", "[0]", "name"}}, {"users[1].address.street", []string{"users", "[1]", "address", "street"}}, {"items[0].details[1].info", []string{"items", "[0]", "details", "[1]", "info"}}, {"simpleKey", []string{"simpleKey"}}, {"simpleKey.simpleProperty", []string{"simpleKey", "simpleProperty"}}, } for _, test := range tests { result := parseKeys(test.input) assert.Equal(t, test.expected, result, "parseKeys(%q) should return %v", test.input, test.expected) } } func TestIsArrayKey(t *testing.T) { tests := []struct { input string expected bool }{ {"[0]", true}, {"[123]", true}, {"users[0]", false}, {"[0].name", false}, {"simpleKey", false}, } for _, test := range tests { result := isArrayKey(test.input) assert.Equal(t, test.expected, result, "isArrayKey(%q) should return %v", test.input, test.expected) } } func TestParseArrayIndex(t *testing.T) { tests := []struct { input string expected int }{ {"[0]", 0}, {"[123]", 123}, {"users[0]", -1}, {"[0].name", -1}, {"simpleKey", -1}, } for _, test := range tests { result := parseArrayIndex(test.input) assert.Equal(t, test.expected, result, "parseArrayIndex(%q) should return %v", test.input, test.expected) } } // Parse function to convert map[string]string to map[string]interface{} func Parse(formData map[string]string) map[string]interface{} { result := make(map[string]interface{}) for key, value := range formData { keys := parseKeys(key) nestedMap(result, keys, value) } return result } // Helper function to parse keys and handle array indices func parseKeys(key string) []string { re := regexp.MustCompile(`(\[\d+\])|(\.)`) parts := re.Split(key, -1) matches := re.FindAllString(key, -1) var keys []string for i, part := range parts { if part != "" { keys = append(keys, part) } if i < len(matches) && matches[i] != "." { keys = append(keys, matches[i]) } } return keys } // Helper function to check if a key is an array key func isArrayKey(key string) bool { return strings.HasPrefix(key, "[") && strings.HasSuffix(key, "]") } // Helper function to parse array index from a key func parseArrayIndex(key string) int { if !isArrayKey(key) { return -1 } re := regexp.MustCompile(`\[(\d+)\]`) matches := re.FindStringSubmatch(key) if len(matches) > 1 { index, _ := strconv.Atoi(matches[1]) return index } return -1 } // Helper function to handle nested maps and arrays func nestedMap(currentMap map[string]interface{}, keys []string, value interface{}) { fmt.Println(currentMap, keys, value) if len(keys) == 0 { return } key := keys[0] if isArrayKey(key) { parentKey := key[:len(key)-3] // Remove the "[0]" part index := parseArrayIndex(key) if index == -1 { fmt.Printf("Invalid array index for key: %s\n", key) return } var slice []interface{} if existing, ok := currentMap[parentKey]; ok { slice, ok = existing.([]interface{}) if !ok { fmt.Printf("Existing value for key %s is not a slice: %v\n", parentKey, existing) return } } else { slice = make([]interface{}, 0) } for len(slice) <= index { slice = append(slice, nil) } if len(keys) == 1 { slice[index] = value } else { if slice[index] == nil { slice[index] = make(map[string]interface{}) } nestedMap(slice[index].(map[string]interface{}), keys[1:], value) } currentMap[parentKey] = slice } else { if len(keys) == 1 { currentMap[key] = value } else { if _, ok := currentMap[key]; !ok { currentMap[key] = make(map[string]interface{}) } nestedMap(currentMap[key].(map[string]interface{}), keys[1:], value) } } } ```
Oudwins commented 4 weeks ago

@alexluong Yes something like that. The issue is that currently zog builds the path based on the input data. Meaning it might actually be impossible (or require a large rewrite) to use that path to index into the form data map. I'll need to think more about this. It won't be a priority for now but I would like to support it if possible.

Thanks for sharing your WIP. Probably the most complex part of this approach is handling slices so good place to put it down hahaha.

If you are not already I would suggest you use v0.6.2 since I merged some breaking changes for the error paths related to what we spoke about. Also, if you are using templ & tailwind you might find another package I maintain useful (tailwind-merge-go)

Other than that, I'll try to ping you here once I find a solution for this use case