openapi-generator

openapi-generator copied to clipboard

Related fix for #17485 which was a regression in 7.2.0 after #17165. That change did not add the necessary imports to the generated API when bean validation was enabled and the schema contained an array that had item level validation.

#17857 only fixed the issue for java native client, this is a similar fix for java resttemplate client.

PR checklist

• [x] Read the contribution guidelines.
• [x] Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• [x] Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh ./bin/configs/*.yaml
  ./bin/utils/export_docs_generators.sh
  

  (For Windows users, please run the script in Git BASH) Commit all changed files. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
• [x] File the PR against the correct branch: master (upcoming 7.1.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
• [x] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@nbruno

@cachescrubber @welshm @MelleD @atextor @manedev79 @javisst @borsch @banlevente @Zomzog @martin-mfg

thanks for the PR. can you please follow step 3 to update the samples so that the CI can verify the change?

@wing328

apologies, issue with newline

fixed now, samples should match

Hi @horaceli @wing328

For webclient it also fails. It would be nice to use this PR to solve both cases. What do you think?

Thank you very much

not sure what is missing, just rebased and reran step 3 and diff is empty

not sure what is missing, just rebased and reran step 3 and diff is empty

I tried re-generating samples and can confirm that it doesn't generate any changes.

@martin-mfg

I tested the changes against the yaml from #17485 and got the below:

package org.openapitools.client.api;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.web.client.RestClientException;

import java.time.LocalDate;
import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
 * API tests for UserApi
 */
@Disabled
class UserApiTest {

    private final UserApi api = new UserApi();

    
    /**
     * 
     *
     * 
     *
     * @throws RestClientException
     *          if the Api call fails
     */
    @Test
    void userGetTest() {
        List<String> username = null;

        api.userGet(username);

        // TODO: test validations
    }
    
}

The API tests don't seem to need jakarta validation imports.

Also, addressed the javax/jakarta issue. The javaxPackage placeholder is only relevant for jakarta.annotation-api - jakarta.validation-api will always use jakarta.

Hi @horaceli,

using the java generator on the input from my previous comment, with library set to "resttemplate" and useBeanValidation set to true, using your latest commit d1633af23ba28506dec6457bb94631ef7df20723, I get this output for UserApiTest.java:

[...]
package org.openapitools.client.api;

import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.web.client.RestClientException;

import java.time.LocalDate;
import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
 * API tests for UserApi
 */
@Disabled
class UserApiTest {

    private final UserApi api = new UserApi();

    
    /**
     * 
     *
     * 
     *
     * @throws RestClientException
     *          if the Api call fails
     */
    @Test
    void userGetTest() {
        List<@Pattern(regexp = "^[a-zA-Z0-9]$")String> username = null;

        api.userGet(username);

        // TODO: test validations
    }
    
}

This uses @Pattern without importing it. I didn't mention in my previous comment that I used useBeanValidation=true. Sorry for that! Could you please try again with this setting?

---

The javaxPackage placeholder is only relevant for jakarta.annotation-api - jakarta.validation-api will always use jakarta.

Why? javax.validation.validation-api also exists, and I think if users set useJakartaEe=false, the generated output should use javax instead of the jakarta equivalent. I agree that eventually we might want to migrate away from javax completely, but I think this should be a separate PR, and maybe it should even happen in a major version of OpenAPI Generator, not in a minor version (more info).

Thanks for the review.

Re the jakarta.validation-api issue, I should have clarified it only applies to generators under modules/openapi-generator/src/main/resources/Java/libraries/. If you look at any pom generated under that directory, e.g. modules/openapi-generator/src/main/resources/Java/libraries/rest-assured/pom.mustache the useJakartaEe flag makes no difference to the validation API, it only affects the jakarta-annotation API. Everywhere in the Java/libraries/* generators, we use jakarta.validation-api:3.0.2 (which uses the jakarta namespace)

This is the same point as you made earlier in https://github.com/OpenAPITools/openapi-generator/pull/18332#discussion_r1563979991, but I was just applying to all the other generators under Java/libraries

Re the generated API tests, I'll check again and get back to you.

Oh, I wasn't aware the javax problem affects so many different libraries. Good to have it fixed everywhere!

Re the generated API tests, I'll check again and get back to you.

Great. Just for your information - I won't be available to review any changes at least next week.

@martin-mfg

Managed to reproduce the issue for API tests, maybe it was a bad cache, apologies.

I've made the changes now for all the generators I've touched, and added a sample/test with the swagger we've been testing with. Let me know your thoughts.

Thanks for the PR, which has been merged.

CI failure fixed via 8860537e2b9