feat: openapiv3 cli-ng codegen

[tiagolobocastro]

This adds a template based code generator for openapi v3 using mustache.

Why a new generator? It didn't seem entirely straightforward changing the current v2 generator to work with the v3 api. Also I like the idea of using templates and putting much of the generator on the templates. Templates would allow users to add their own and control code generation to match their needs and opinions :)

Why mustache? I have a generator on a fork of openapi-generator: https://github.com/openebs/openapi-generator/blob/rust_mayastor/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/RustMayastorCodegen.java which is based on another generator from the same project. This would allow me to use it as a drop-in replacement.

In many ways it's not as nice as the the current v2 generator but... we can change that easily (I hope) simply by writing some mustache!

[zeerooth]

Hey, I just wanted so say that I'm super excited by this change, especially considering the templates!

I wanted to test it out, but alas I see that you use a custom version of the ramhorns crate, so I can't install from this branch:

error: no matching package named `ramhorns` found
location searched: https://github.com/paperclip-rs/paperclip.git?branch=draft-poc#b8310db6
required by package `paperclip v0.8.1 (https://github.com/paperclip-rs/paperclip.git?branch=draft-poc#b8310db6)`

[tiagolobocastro]

Great, glad someone else took an interest!

Ah sorry yes I had to add a few features/fixes on both ramhorns and openapiv3.. I'm super busy with work right now, I'll try to make this branch usable for anyone to try when a get a little slack and ping you.

[zeerooth]

Great, glad someone else took an interest!

Ah sorry yes I had to add a few features/fixes on both ramhorns and openapiv3.. I'm super busy with work right now, I'll try to make this branch usable for anyone to try when a get a little slack and ping you.

No problem, thanks!

If you also need some help with implementing some features/testing or whatever that's necessary to get it merged I can help get it done. Just need to familiarize myself a bit with the paperclip codebase :D

[tiagolobocastro]

Great, glad someone else took an interest! Ah sorry yes I had to add a few features/fixes on both ramhorns and openapiv3.. I'm super busy with work right now, I'll try to make this branch usable for anyone to try when a get a little slack and ping you.

No problem, thanks!

If you also need some help with implementing some features/testing or whatever that's necessary to get it merged I can help get it done. Just need to familiarize myself a bit with the paperclip codebase :D

Sounds good! What I'm adding is kinda novel and doesn't share most of the existing paperclip logic. Paperclip works in two ways, generate openapi from code and generate code from openapi. The existing "code from openapi" seems tightly coupled with openapi v2. I was a bit lazy to find ways of integrating v2 with v3 so instead I opted to make use of the existing openapiv3 crate and break away from the existing code.

[tiagolobocastro]

@Zeerooth alright, you should be able to run it now, example:

RUST_LOG=info cargo run --bin paperclip --features "cli v3" -- --spec ../mayastor/controller/control-plane/rest/openapi-specs/v0_api_spec.yaml --api v3 -o ./testgen/src/autogen --models

[Kavan72]

@tiagolobocastro, I'm also happy to see these changes. Can't wait to test this!

[zeerooth]

Hey @tiagolobocastro thanks for the work on that!

i've tested it for a bit and unfortunately sometimes I'll get

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

I've uploaded the example schema that the stack overflow happens with to https://gist.github.com/Zeerooth/45d8c6d0beaa6a4f5e47918e5b0c41f8

[zeerooth]

BTW, you're using the ramhorns dependency here, so because it is licensed under GPL v3 then if this was to get merged you'd need to change the license of paperclip to GPL v3 too. It's something to keep in mind.

[tiagolobocastro]

Hey @tiagolobocastro thanks for the work on that!

i've tested it for a bit and unfortunately sometimes I'll get

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

I've uploaded the example schema that the stack overflow happens with to https://gist.github.com/Zeerooth/45d8c6d0beaa6a4f5e47918e5b0c41f8

Thanks I'll test with this api when I get a chance :+1:

BTW, you're using the ramhorns dependency here, so because it is licensed under GPL v3 then if this was to get merged you'd need to change the license of paperclip to GPL v3 too. It's something to keep in mind.

Thanks a lot I hadn't spotted that, seems I may need to move away from ramhorns as I wouldn't want to change paperclip's license :(

[zeerooth]

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Aborted (core dumped)

I've uploaded the example schema that the stack overflow happens with to https://gist.github.com/Zeerooth/45d8c6d0beaa6a4f5e47918e5b0c41f8

Ok, looking at the traceback it becomes clear that the problem lies with circular references. In the schema that I referenced above there is a Status component with reblog field that contains a nullable Status and paperclip tries to resolve these references indefinitely, eventually leading to a stack overflow. I presume it should be supported though. Some caching of already resolved Schemas would probably be the best.

[zeerooth]

Besides that there is also this:

[DEBUG paperclip::v3] Response: Item(Response { description: "Get response following the Nodeinfo 2.1 schema", headers: {}, content: {"application/json": MediaType { schema: Some(Reference { reference: "#/components/schemas/TwoOne" }), example: None, examples: {}, encoding: {}, extensions: {} }}, links: {}, extensions: {} })
[DEBUG paperclip::v3::operation] Operation::Some("post") => post::/oauth/token::["oauth::token"]
thread 'main' panicked at 'not yet implemented', src/v3/operation.rs:193:21

Edit: seems like this is an issue with my schema as per openAPI spec, every endpoint should list at least one response, but this one does't. Still, there should be some info about that displayed.

[tiagolobocastro]

Sorry haven't had time lately to catch up on this, circular references should work indeed, I'll try to take a quick look tonight.

[tiagolobocastro]

Yeah so for the responses we're only currently expecting 200 and 204 as valid responses, probably we need to also consider the other 201,202,203 codes and consider if we want to support multiple possible success response codes.

As for the circular reference yeah there's nothing to handle that at the moment, I'm trying to figure out how best to do it since the children properties will basically have to point to a parent (or grandparent, etc) which isn't fully constructed yet. I think solvable with some indirection. A bonus of handling this will be that we'll probably avoid parsing properties more than once, if we just take it from the cache.

[zeerooth]

Yeah so for the responses we're only currently expecting 200 and 204 as valid responses, probably we need to also consider the other 201,202,203 codes and consider if we want to support multiple possible success response codes.

That'd definitely be awesome, but that might mean that we need to have multiple different models per each operation, which probably wouldn't be possible to model easily in rust, right?

As for the circular reference yeah there's nothing to handle that at the moment, I'm trying to figure out how best to do it since the children properties will basically have to point to a parent (or grandparent, etc) which isn't fully constructed yet. I think solvable with some indirection. A bonus of handling this will be that we'll probably avoid parsing properties more than once, if we just take it from the cache.

I agree, that's what I figured out too, I tried to make a global HashMap of props mapped to each schema in the OpenApiV3 but that'd require making it mutable and passing around a lot on which I mostly gave up.

Another thing I considered is that we could have a method that recursively asks each parent of the prop if it has discovered a model mapped to the certain schema and if it has, then use a reference to it instead of constructing it again. This is probably still quite inefficient as we might need to compute a model multiple times (it it has different roots) but wouldn't require as many changes in the code at this point.

[tiagolobocastro]

So I think I was actually looking at this wrong... When we're resolving a member variable of an object schema we don't really need to re-resolve the "parent" again, we just need simple information, so we can reference it, example:

struct Parent {
 a: Option<model::Parent>
}

So we need the variable name, the Parent type name and not much more, so we don't need to even try to resolve Parent again.. I've pushed a change that should address this.

[zeerooth]

OMG thanks, it works splendidly!

I was able to generate rust models now, but it seems like paperclip doesn't handle reserved rust keywords, so if I have a type field in my schema it's going to get put raw into model and it generates compile errors. It should probably get renamed so sth like _type or paperclip should make use of raw identifiers which would make it into r#type and no serde rename would be needed in that case.

[Kavan72]

@tiagolobocastro any news 😊

[tiagolobocastro]

@tiagolobocastro any news 😊

Hey, haven't really had the time to look at this any further. There are some issues with crate I've added which is GPLv3... so before merging this to paperclip I'd have to switch to another crate for the mustache parsing. Although in theory that may even be ok as paperclip itself would not be part of the final binary, it'd only be used to generate the code? Erring on the side of caution might be better to replace it.

[tiagolobocastro]

@zeerooth thank you for pinging ram horns about the license, you're awesome! Also my PR there got merged, so I'll resume work in this PR to tidy it up. I'm at Kubecon this week so won't have time or energy but maybe on the next one I'll start!

[tiagolobocastro]

Started tidying up this and making it available as experimental feature. Currently it's very opinionated, basically same template which I was previously using. Once merged I'll add optional builder pattern and cli generation using the builder, similar to v2 codegen today.