Skip to content
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.

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

Unable to set default values for custom typed strings #194

Closed
jpalawaga opened this issue Mar 5, 2024 · 3 comments · Fixed by #195
Closed

Unable to set default values for custom typed strings #194

jpalawaga opened this issue Mar 5, 2024 · 3 comments · Fixed by #195

Comments

@jpalawaga
Copy link

jpalawaga commented Mar 5, 2024

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
Copy link
Member

vearutop commented Mar 6, 2024

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
Copy link
Author

jpalawaga commented Mar 6, 2024

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):

  • on the first instance of the custom-typed paramters, we correctly detect its type in reflect().
  • then, we do reflectDefer, and cache the reference. keepType is true, so the type is promoted
  • then, on the second occurence of the parameter type, we see that we have a cached version so we short circuit. You can see that here: https://github.com/swaggest/jsonschema-go/blob/9575eb9/reflect.go#L497
  • but we only take the ref! Which is a mostly empty schema--no associated type!
  • we end up paniccing similar to the original issue, since the type isn't selected correctly.

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

@jpalawaga
Copy link
Author

^ PR open with updated test and code change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants