search

swaggest / jsonschema-go

JSON Schema mapping for Go
https://pkg.go.dev/github.com/swaggest/jsonschema-go
MIT License
102 stars 13 forks source link

fix type reflection for repeated custom types #113

Open jpalawaga opened 4 months ago

jpalawaga commented 4 months ago

See discussion in #194

jpalawaga commented 3 months ago

@vearutop ping! if you're busy, np, I can just work off of the fork.

vearutop commented 3 months ago

TL;DR this does not seem like a fix to anything.

I think you've highlighted a bug in the current test.

For JSON schema up to draft-07 (OpenAPI 3 is based on draft-04 behavior to my knowledge) any keywords adjacent to $ref should be ignored. See this issue for relevant discussion.

OpenAPI 3.1 is based on a recent JSON schema, so probably adjacent keywords should not be a problem.

This library is draft-07 compliant, so I think we should try to avoid $ref adjacent keywords or workaround them with allOf (at least by default).

One way to "solve" the original issue is to get rid of the $ref altogether by inlining the definition with SchemaInliner.

jpalawaga commented 3 months ago

Ah, yeah, I see that this causes the rest of the machinery to pull the full schema instead of just the reference.

From the link you provide, it does seem like the library is currently doing the right thing by not placing adjacent properties on a ref value.

Though my follow-on thought is not to inline the schema definition. Recursive document exploration (with no short-circuiting) can get expensive depending on the size of the schema.

There is a keepType parameter on reflect, though I'm not sure it's intended purpose. I tried to move the typeKeep-ing into a defer statement that could decorate from the cached schema if it needed the type, though this seems to still break tests.

The root issue seems to be that walkProperty expects to have a full schema type returned by reflect but sometimes it just returns a ref.