Skip to content

Conversation

@ptrdom
Copy link
Contributor

@ptrdom ptrdom commented Oct 25, 2023

If spec would contain paths that use said schemas, then models would be generated. #1854 (comment)

@ptrdom ptrdom marked this pull request as draft October 25, 2023 14:48
@ptrdom
Copy link
Contributor Author

ptrdom commented Oct 25, 2023

Also, code works fine with OpenAPI v3.0.0. even though paths only became optional in v3.1.0.

@blast-hardcheese
Copy link
Member

So I poked around a bit and discovered that (hilariously?) JsonSchema extends Schema<Object>, but is functionally distinct from ObjectSchema extends Schema<Object>.

All the matching we do on ObjectSchema is invalid in the JsonSchema case, which is why you get RandomType instead of something coherent.

@ptrdom ptrdom changed the title Model of schemas are not generated for OpenAPI v3.1.0 spec without paths Codegen does not work for OpenAPI v3.1.0 Oct 26, 2023
@ptrdom
Copy link
Contributor Author

ptrdom commented Oct 26, 2023

Seems like this is not related to schemas being not referenced by paths, but a v3.1.0 issue in general. Adjusted the PR title to reflect that.

@blast-hardcheese
Copy link
Member

OK. A little more coffee helped clear up what likely needs to happen in the interim, and some initial poking around to see if I could convince the parser to emit anything other than JsonSchema suggests to me that we're dealing with some future-proofing.

Considering that, we can fall back to the supertype and just match on the various implementation branches and be done with things for now.

@blast-hardcheese
Copy link
Member

I'm offering this here as what I intend to merge, though the test still needs to be reflowed into src/test/scala/core/issues/Issue1854.scala with the original test restored.

@github-actions github-actions bot added core Pertains to guardrail-core java-support Pertains to guardrail-java-support scala-support Pertains to guardrail-scala-support labels Oct 26, 2023
@blast-hardcheese blast-hardcheese changed the title Codegen does not work for OpenAPI v3.1.0 OpenAPI v3.1.0 spec files do not generate valid data classes Oct 27, 2023
@ptrdom
Copy link
Contributor Author

ptrdom commented Oct 27, 2023

Nice! So is this solution okay to merge? Will you create the test case in regression tests for this or should I do it?

ptrdom and others added 3 commits October 27, 2023 22:27
@blast-hardcheese blast-hardcheese marked this pull request as ready for review October 28, 2023 05:33
@blast-hardcheese blast-hardcheese merged commit f2a7584 into guardrail-dev:master Oct 28, 2023
@blast-hardcheese
Copy link
Member

Thanks again for the report!

@ptrdom
Copy link
Contributor Author

ptrdom commented Oct 28, 2023

Ah, too bad there are no snapshots for guardrail, hoping for a release soon!

@blast-hardcheese
Copy link
Member

Yeah, I've not had time to figure out how to get snapshots working with the release. If you know something about that, PRs are welcome! This will come in the 1.0 release coming soon.

@ptrdom
Copy link
Contributor Author

ptrdom commented Oct 29, 2023

I have some thoughts about snapshot releases, will create an issue.

@blast-hardcheese blast-hardcheese added the bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behaviour for functionality that either was intended to work, or has worked in the past core Pertains to guardrail-core java-support Pertains to guardrail-java-support scala-support Pertains to guardrail-scala-support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants