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.

Sign up for GitHub

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

Jump to bottom

[GO] allow multiple types in jsonschema #1221

Closed
wants to merge 4 commits into from
+89 −29
Closed

[GO] allow multiple types in jsonschema #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa31c3d
Feat: add support for multiple types
alonsopec89 Nov 8, 2024
a19ce93
Merge branch 'next' into 610-allow-multiple-types-in-jsonschema
alonsopec89 Nov 11, 2024
185c5d2
run the skipped tests
alonsopec89 Nov 12, 2024
cfa850f
attempt to fix tests fixing the Schema unmarshall json
alonsopec89 Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions go/genkit/servers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,14 @@ func TestDevServer(t *testing.T) {
"/custom/devServer/inc": {
Key: "/custom/devServer/inc",
Name: "devServer/inc",
InputSchema: &jsonschema.Schema{Type: "integer"},
OutputSchema: &jsonschema.Schema{Type: "integer"},
InputSchema: &jsonschema.Schema{Type: []string{"integer"}},
OutputSchema: &jsonschema.Schema{Type: []string{"integer"}},
Metadata: map[string]any{"foo": "bar"},
},
"/custom/devServer/dec": {
Key: "/custom/devServer/dec",
InputSchema: &jsonschema.Schema{Type: "integer"},
OutputSchema: &jsonschema.Schema{Type: "integer"},
InputSchema: &jsonschema.Schema{Type: []string{"integer"}},
OutputSchema: &jsonschema.Schema{Type: []string{"integer"}},
Name: "devServer/dec",
Metadata: map[string]any{"bar": "baz"},
},
Expand Down
3 changes: 3 additions & 0 deletions go/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ require (
github.com/MicahParks/keyfunc v1.9.0 // indirect
github.com/PuerkitoBio/purell v1.1.1 // indirect
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 // indirect
github.com/alonsopf/jsonschema v0.0.0-20241112032241-b0be673d3d4c // indirect
github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 // indirect
github.com/bahlo/generic-list-go v0.2.0 // indirect
github.com/buger/jsonparser v1.1.1 // indirect
Expand Down Expand Up @@ -95,3 +96,5 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20240708141625-4ad9e859172b // indirect
google.golang.org/grpc v1.65.0 // indirect
)

replace github.com/invopop/jsonschema => github.com/alonsopf/jsonschema v0.0.0-20241112034953-8a224be9076f
Copy link
Collaborator

@apascal07 apascal07 Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert? Or are you actually relying on changes in your fork?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually relying on changes in my fork to support multiple types

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's no good. We don't want to rely on a fork of this, it's not sustainable for us to maintain it. Is there any way to roll those changes into the Genkit library? Otherwise we need to explore a different solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened a PR in the jsonschema repo, also I'm reviewing anothers options if doesn't get merged

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I believe the maintainer of the package has stated that they do not intend to support multiple types because Go is strongly typed and this would make it very complicated. We need a different solution, either a different package or something creative within our own codebases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good, I tried to do in this PR using our own codebase #1400

6 changes: 4 additions & 2 deletions go/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ github.com/PuerkitoBio/purell v1.1.1 h1:WEQqlqaGbrPkxLJWfBwQmfEAE1Z7ONdDLqrN38tN
github.com/PuerkitoBio/purell v1.1.1/go.mod h1:c11w/QuzBsJSee3cPx9rAFu61PvFxuPbtSwDGJws/X0=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578 h1:d+Bc7a5rLufV/sSk/8dngufqelfh6jnri85riMAaF/M=
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
github.com/alonsopf/jsonschema v0.0.0-20241112032241-b0be673d3d4c h1:eharL8A3PtLM6cFVEJmWUBjpXyAhb/Dfj+5RzrnRcOk=
github.com/alonsopf/jsonschema v0.0.0-20241112032241-b0be673d3d4c/go.mod h1:xAW0fRKWYyjgf/vtEjMlCWc1qprM9DnMtlM8kaZO08E=
github.com/alonsopf/jsonschema v0.0.0-20241112034953-8a224be9076f h1:koSFHSM3XlrN8TqKUxzNz0epkpTaLHEzKWo2QBZyY/Q=
github.com/alonsopf/jsonschema v0.0.0-20241112034953-8a224be9076f/go.mod h1:ffZ5Km5SWWRAIN6wbDXItl95euhFz2uON45H2qjYt+0=
github.com/ankane/disco-go v0.1.0 h1:nkz+y4O+UFKnEGH8FkJ8wcVwX5boZvaRzJN6EMK7NVw=
github.com/ankane/disco-go v0.1.0/go.mod h1:nkR7DLW+KkXeRRAsWk6poMTpTOWp9/4iKYGDwg8dSS0=
github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
Expand Down Expand Up @@ -175,8 +179,6 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.2/go.mod h1:VLSiSSBs/ksP
github.com/googleapis/gax-go/v2 v2.12.5 h1:8gw9KZK8TiVKB6q3zHY3SBzLnrGp6HQjyfYBYGmXdxA=
github.com/googleapis/gax-go/v2 v2.12.5/go.mod h1:BUDKcWo+RaKq5SC9vVYL0wLADa3VcfswbOMMRmB9H3E=
github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=
github.com/invopop/jsonschema v0.12.0 h1:6ovsNSuvn9wEQVOyc72aycBMVQFKz7cPdMJn10CvzRI=
github.com/invopop/jsonschema v0.12.0/go.mod h1:ffZ5Km5SWWRAIN6wbDXItl95euhFz2uON45H2qjYt+0=
github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM=
github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg=
github.com/jackc/pgservicefile v0.0.0-20221227161230-091c0ba34f0a h1:bbPeKD0xmW/Y25WS6cokEszi5g+S0QxI/d45PkRi7Nk=
Expand Down
9 changes: 4 additions & 5 deletions go/internal/base/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func TestSchemaAsMap(t *testing.T) {
}
type Foo struct {
BarField Bar
Str string
Str interface{} `json:"str" jsonschema:"type=number;string"`
}

want := map[string]any{
Expand All @@ -84,12 +84,11 @@ func TestSchemaAsMap(t *testing.T) {
"properties": map[string]any{
"Bar": map[string]any{"type": string("string")},
},
"required": []any{string("Bar")},
"type": string("object"),
"type": string("object"),
},
"Str": map[string]any{"type": string("string")},
"str": map[string]any{"type": []any{string("number"), string("string")}},
},
"required": []any{string("BarField"), string("Str")},
"required": []any{string("BarField"), string("str")},
"type": string("object"),
}

Expand Down
84 changes: 74 additions & 10 deletions go/plugins/dotprompt/picoschema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ func picoschemaToJSONSchema(val any) (*jsonschema.Schema, error) {
if err != nil {
return nil, err
}
s.Type = "object"

s.Type = []string{"object"}
return s, nil
}
}
Expand All @@ -64,15 +65,15 @@ func parsePico(val any) (*jsonschema.Schema, error) {
case string:
typ, desc, found := strings.Cut(val, ",")
switch typ {
case "string", "boolean", "null", "number", "integer", "any":
case "string", "boolean", "null", "number", "integer": // Valid types
case "any":
typ = ""
default:
return nil, fmt.Errorf("picoschema: unsupported scalar type %q", typ)
}
if typ == "any" {
typ = ""
}
ret := &jsonschema.Schema{
Type: typ,
ret := &jsonschema.Schema{}
if typ != "" {
ret.Type = []string{typ}
}
if found {
ret.Description = strings.TrimSpace(desc)
Expand All @@ -84,7 +85,7 @@ func parsePico(val any) (*jsonschema.Schema, error) {

case map[string]any:
ret := &jsonschema.Schema{
Type: "object",
Type: []string{"object"},
Properties: orderedmap.New[string, *jsonschema.Schema](),
AdditionalProperties: jsonschema.FalseSchema,
}
Expand All @@ -99,7 +100,15 @@ func parsePico(val any) (*jsonschema.Schema, error) {
if err != nil {
return nil, err
}

// Only add "null" to the "type" array if Type is already set and not empty
if isOptional {
if len(property.Type) > 0 {
if !contains(property.Type, "null") {
property.Type = append(property.Type, "null")
}
}
// Do not set property.Type to ["null"] if Type is empty or nil
}
if !found {
ret.Properties.Set(propertyName, property)
continue
Expand All @@ -110,7 +119,7 @@ func parsePico(val any) (*jsonschema.Schema, error) {
switch typ {
case "array":
property = &jsonschema.Schema{
Type: "array",
Type: []string{"array"},
Items: property,
}
case "object":
Expand Down Expand Up @@ -142,6 +151,15 @@ func parsePico(val any) (*jsonschema.Schema, error) {
}
}

func contains(slice []string, item string) bool {
for _, v := range slice {
if v == item {
return true
}
}
return false
}

// mapToJSONSchema converts a YAML value to a JSONSchema.
func mapToJSONSchema(m map[string]any) (*jsonschema.Schema, error) {
var ret jsonschema.Schema
Expand All @@ -166,6 +184,52 @@ func mapToJSONSchema(m map[string]any) (*jsonschema.Schema, error) {
}

switch rf.Type() {
case reflect.TypeOf([]string(nil)):
switch v := v.(type) {
case string:
if v != "" {
rf.Set(reflect.ValueOf([]string{v}))
}
case []any:
sstrs := make([]string, 0, len(v))
for i, astr := range v {
s, ok := astr.(string)
if !ok {
return nil, fmt.Errorf("picoschema: found type %T for field element %d of %q, want string", astr, i, k)
}
if s != "" {
sstrs = append(sstrs, s)
}
}
if len(sstrs) > 0 {
rf.Set(reflect.ValueOf(sstrs))
}
default:
return nil, fmt.Errorf("picoschema: found type %T for field %q, want string or array of strings", v, k)
}

case reflect.TypeOf(""):
str, ok := v.(string)
if !ok {
return nil, fmt.Errorf("picoschema: found type %T for field %q, want string", v, k)
}
rf.SetString(str)

case reflect.TypeOf((*uint64)(nil)):
rf.Set(reflect.New(reflect.TypeOf(uint64(0))))
switch v := v.(type) {
case uint64, uint32, uint16, uint8, int, int8, int16, int32, int64:
rf.Elem().SetUint(reflect.ValueOf(v).Uint())
default:
return nil, fmt.Errorf("picoschema: found type %T for field %q, want an integer type", v, k)
}

case reflect.TypeOf(true):
b, ok := v.(bool)
if !ok {
return nil, fmt.Errorf("picoschema: found type %T for field %q, want bool", v, k)
}
rf.SetBool(b)
case reflect.TypeFor[any]():
rf.Set(reflect.ValueOf(v))

Expand Down
8 changes: 0 additions & 8 deletions go/plugins/dotprompt/picoschema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,8 @@ func TestPicoschema(t *testing.T) {
t.Fatal(err)
}

skip := map[string]bool{
"required field": true,
"nested object in array and out": true,
}

for _, test := range tests {
t.Run(test.Description, func(t *testing.T) {
if skip[test.Description] {
t.Skip("no support for type as an array")
}
var val any
if err := yaml.Unmarshal([]byte(test.YAML), &val); err != nil {
t.Fatalf("YAML unmarshal failure: %v", err)
Expand Down
Loading