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

[BUG][RUST-AXUM] rust-axum generator does not handle uuid::Uuid in headerParams #18554

Closed
myz-dev opened this issue May 2, 2024 · 0 comments
Closed

Comments

@myz-dev
Copy link
Contributor

myz-dev commented May 2, 2024

Version used

openapi-generator version: 7.6.0-SNAPSHOT

Description

The rust-axum generator has the following issue:
It is smart enough to detect the format: uuid specifier in openAPI types and map those parameters to the Rust type uuid::Uuid.
Problem: When the openAPI type is within a header, the generated Rust code fails to convert the header value string into a uuid::Uuid because of a missing impl TryFrom<HeaderValue> for IntoHeaderValue<uuid::Uuid> implementation.

This makes generated code like this invalid:

// in src/server/mod.rs
Some(v) => match header::IntoHeaderValue::<uuid::Uuid>::try_from((*v).clone()) {

Specification

This is my minimal openAPI spec that reproduces the issue:

openapi: 3.0.0
info:
  title: Sample API
  version: 0.1.9
paths:
  /users:
    post:
      summary: Create a user.
      description: Adds a user to the users database table.
      parameters: 
        - $ref: "#/components/parameters/CorrelatonIdHeaderParam"
      responses:
        "201": # status code
          description: Added row to table!
          content:
            application/json:
              schema:
                type: number 
components:
  schemas: 
      CorrelationId:
        type: string
        format: uuid
        example: a9f5a638-728c-479d-af9b-016eb8049ab6
  parameters: 
    CorrelatonIdHeaderParam:
      name: some_uid
      in: header
      required: true
      description: lets say we need this.
      schema:
        $ref: "#/components/schemas/CorrelationId"

Generation details:

I just call the CLI with these options:
java -jar openapi-generator-cli.jar generate -i minimal.yaml -g rust-axum -o out

Suggestion:

A very easy fix would be to add this to the end of the modules/openapi-generator/src/main/resources/rust-axum/header.mustache file unconditionally:

//  uuid::Uuid

impl TryFrom<HeaderValue> for IntoHeaderValue<uuid::Uuid> {
    type Error = String;

    fn try_from(hdr_value: HeaderValue) -> Result<Self, Self::Error> {
        match hdr_value.to_str() {
            Ok(hdr_value) => match uuid::Uuid::from_str(hdr_value) {
                Ok(uuid) => Ok(IntoHeaderValue(uuid)),
                Err(e) => Err(format!("Unable to parse: {} as uuid - {}", hdr_value, e)),
            },
            Err(e) => Err(format!(
                "Unable to convert header {:?} to string {}",
                hdr_value, e
            )),
        }
    }
}

impl TryFrom<IntoHeaderValue<uuid::Uuid>> for HeaderValue {
    type Error = String;

    fn try_from(hdr_value: IntoHeaderValue<uuid::Uuid>) -> Result<Self, Self::Error> {
        match HeaderValue::from_bytes(hdr_value.0.as_bytes()) {
            Ok(hdr_value) => Ok(hdr_value),
            Err(e) => Err(format!(
                "Unable to convert {:?} to a header: {}",
                hdr_value, e
            )),
        }
    }
}

I wanted to add this code in conditionally, when there is at least one header parameter, with the format set to uuid.
I tried to implement this idea but I neither have any Java experience nor do I understand the flow of the generator program at the moment. What I have tried so far:
Within RustAxumServerCodegen.java in the public CodegenOperation fromOperation(String path, String httpMethod, Operation operation, List<Server> servers) { function I have collected all the parameters in a list:

private List<String> typesWithinHeader = new ArrayList<>();
// later within public CodegenOperation fromOperation(String path, String httpMethod, Operation operation, List<Server> servers) 
typesWithinHeader = op.headerParams.stream().map(CodegenParameter::getDataType).collect(Collectors.toList()); 

Now I would like to add the suggested fix above to headers.mustache only when this array is not empty. In a further step the contents of this array could be used to also add conversion implementations for other types than uuid::Uuid that might break the generated code in the future (I cannot think of any right now).

Also just trying to loop over all header parameters in the header.mustache file does not produce any output at all. I wonder if the template is populated without passing any parameters to it:

{{#headerParams}}
This is not included in the final `header.rs` even though there are `headerParams` in the resulting `CodegenOperation` structure
{{/headerParams}}
myz-dev pushed a commit to myz-dev/openapi-generator that referenced this issue May 4, 2024
Running `./bin/generate-samples.sh ./bin/configs/manual/*.yaml`
updated all examples. Most of them contain changes to their previously
checked in version that are not related to the fix for OpenAPITools#18554.
`mvn integration-test -f samples/server/petstore/rust-axum/pom.xml`
passes.
myz-dev added a commit to myz-dev/openapi-generator that referenced this issue May 5, 2024
Running `./bin/generate-samples.sh ./bin/configs/manual/*.yaml`
updated all examples. Most of them contain changes to their previously
checked in version that are not related to the fix for OpenAPITools#18554.
`mvn integration-test -f samples/server/petstore/rust-axum/pom.xml`
passes.
myz-dev added a commit to myz-dev/openapi-generator that referenced this issue May 5, 2024
This commit adds an integration test that tests the bug fix for OpenAPITools#18554.
A header parameter of `format: uuid` is included in one route.
This makes the example create a route handler that tries to extract a
Rust `uuid::Uuid` type from the header. The integration test will check
that the generated code compiles.
wing328 pushed a commit that referenced this issue May 7, 2024
)

* fix: Fix uuid in header params causing errors

Routes with header parameters with a `format` of `uuid` in the openAPI
specification used to cause a compilation error in the resulting
Rust Axum code.
This commit fixes the issue by including the correct conversion trait
implementation on the condition that at least one header parameter of
`format` `uuid` is included in the specification.

* refactor: Add final to boolean

* fix: Bring str::FromStr optionally into scope

The trait needs to be in scope for the TryFrom implementation:
`TryFrom<HeaderValue> for IntoHeaderValue<uuid::Uuid> `
It will only be brought into scope when the implementation is rendered.

* test: Add integration test and its specification

This commit adds an integration test that tests the bug fix for #18554.
A header parameter of `format: uuid` is included in one route.
This makes the example create a route handler that tries to extract a
Rust `uuid::Uuid` type from the header. The integration test will check
that the generated code compiles.

* test: Update examples and run integration test

The generated samples are updated with:
`./bin/generate-samples.sh ./bin/configs/manual/*.yaml`
Most example projects have their version numbers bumped. Some changes
show, that there are some other unrelated changes to the files, which
indicates that some prior commit did not update the samples accordingly.
The relevant integration test
`mvn integration-test -f samples/server/petstore/rust-axum/pom.xml`
passes.
@wing328 wing328 closed this as completed May 8, 2024
renatomameli pushed a commit to renatomameli/openapi-generator that referenced this issue May 17, 2024
…nAPITools#18563)

* fix: Fix uuid in header params causing errors

Routes with header parameters with a `format` of `uuid` in the openAPI
specification used to cause a compilation error in the resulting
Rust Axum code.
This commit fixes the issue by including the correct conversion trait
implementation on the condition that at least one header parameter of
`format` `uuid` is included in the specification.

* refactor: Add final to boolean

* fix: Bring str::FromStr optionally into scope

The trait needs to be in scope for the TryFrom implementation:
`TryFrom<HeaderValue> for IntoHeaderValue<uuid::Uuid> `
It will only be brought into scope when the implementation is rendered.

* test: Add integration test and its specification

This commit adds an integration test that tests the bug fix for OpenAPITools#18554.
A header parameter of `format: uuid` is included in one route.
This makes the example create a route handler that tries to extract a
Rust `uuid::Uuid` type from the header. The integration test will check
that the generated code compiles.

* test: Update examples and run integration test

The generated samples are updated with:
`./bin/generate-samples.sh ./bin/configs/manual/*.yaml`
Most example projects have their version numbers bumped. Some changes
show, that there are some other unrelated changes to the files, which
indicates that some prior commit did not update the samples accordingly.
The relevant integration test
`mvn integration-test -f samples/server/petstore/rust-axum/pom.xml`
passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants
@wing328 @myz-dev and others