swaggest / rest

Web services with OpenAPI and JSON Schema done quick in Go
https://pkg.go.dev/github.com/swaggest/rest
MIT License
335 stars 17 forks source link

Unable to set default values for custom typed strings #194

Closed jpalawaga closed 4 months ago

jpalawaga commented 4 months ago

Describe the bug I'm trying to build an enum type that can be used for input. As a part of this, I'd like to be able to set a default value for the enum field as well. I'd like to be able to define this in code (for reuse) rather than as a struct tag (i.e make our actual inputs/outputs match the documentation).

However, when I try to use a custom type with the default struct tags, it seems as though the parser expects a json string/object rather than a string value. You can use a json string to get past this, but then default does something weird. Concrete example below.

To Reproduce

package main_test

import (
    "bytes"
    "context"
    "net/http"
    "net/http/httptest"
    "testing"

    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/require"
    "github.com/swaggest/rest/web"
    "github.com/swaggest/usecase"
)

type Discover string

const (
    DiscoverAll  Discover = "all"
    DiscoverNone Discover = "none"
)

func (d *Discover) Enum() []any {
    return []any{DiscoverAll, DiscoverNone}
}

func TestFoo(t *testing.T) {
    type NewThing struct {
        DiscoverMode *Discover `json:"discover,omitempty" default:"all"`
    }

    s := web.DefaultService()

    s.Post("/foo", usecase.NewInteractor(func(ctx context.Context, input NewThing, output *string) error {
        disc := string(*input.DiscoverMode)
        *output = disc

        return nil
    }))

    req, err := http.NewRequest(http.MethodPost, "/foo", bytes.NewReader([]byte(`{}`)))
    require.NoError(t, err)
    req.Header.Set("Content-Type", "application/json")
    rw := httptest.NewRecorder()

    s.ServeHTTP(rw, req)
    assert.Equal(t, `"all" `, rw.Body.String())
}

Observed behavior

=== RUN   TestFoo
--- FAIL: TestFoo (0.00s)
panic: DiscoverMode: parsing default as JSON: invalid character 'a' looking for beginning of value [recovered]
        panic: DiscoverMode: parsing default as JSON: invalid character 'a' looking for beginning of value

Expected behavior The API returns all.

Side note: you can change this so it reads default:"\"all\"" -- then the input validation passes, but your default ends up as the quoted json string "all", not all (i.e. it is not possible to produce the valid enum option).

--

I'm happy to make a contribution here if you can point me in the right direction. I did step through the code a bit. I tried Implementing a TextUn/Marshaller so that maybe the type would properly get set as a string in jsonschema-go/reflect.go, but that didn't seem to make anything better on the version I was using. I believe it's because string is added as "a" type, rather than "the" SimpleType, which causes the checkInlineValue to fall into the default rather than String case (where it does a json.Unmarshal)

vearutop commented 4 months ago

Thank you for raising this issue, please try latest version v0.2.64.

I'd like to be able to define this in code (for reuse) rather than as a struct tag (i.e make our actual inputs/outputs match the documentation).

With a new version you should be able to define default on a type using Preparer, Exposer or RawExposer. https://github.com/swaggest/jsonschema-go?tab=readme-ov-file#implementing-interfaces-on-a-type

jpalawaga commented 4 months ago

Hi @vearutop ! thanks for the quick fix.

In my project, I'm still having a bit of issues with this. The problem lies in the jsonschema-go. Specifically, it seems like there is an issue when the custom type appears as a part of more than one variable definition in the request payload.

What I'm seeing (I apologies I snippet for this yet, I'll try and construct one, but for now I'll describe):

I think the solution is to change the code reference above to pull the cached object from rc.definitions instead of rc.definitionRefss

jpalawaga commented 4 months ago

^ PR open with updated test and code change