Skip to content

[Spring]Added support for composed annotations (@GetMapping,@PostMapping,@PutMapping,@DeleteMapping). #21118

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dRbAndrade
Copy link

@dRbAndrade dRbAndrade commented Apr 22, 2025

I have noticed that when using the Spring generator, the API interface file would contain @RequestMapping(method = RequestMethod.) annotation. This would isn't exactly an issue but would be a code smell under sonar coding rule java:s4488 https://sonarsource.github.io/rspec/#/rspec/S4488. I tried to search for an existing way to change it but could not find it. So I went ahead and added the support myself.

This is a change for Java Spring generator so I'm tagging the committee:
@cachescrubber @welshm @MelleD @atextor @manedev79 @javisst @borsch @banlevente @Zomzog @martin-mfg

PR checklist

  • Read the contribution guidelines.
  • 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.
  • Run the following to build the project and update samples:
    ./mvnw clean package || exit
    ./bin/generate-samples.sh ./bin/configs/*.yaml || exit
    ./bin/utils/export_docs_generators.sh || exit
    
    (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.
  • File the PR against the correct branch: master (upcoming 7.x.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

Copy link
Contributor

@martin-mfg martin-mfg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dRbAndrade, thanks for the PR!

Could you please take care of the review comments below and also add your new lambda to this section in the docs:

## Mustache Lambdas

@@ -226,8 +226,15 @@ public interface {{classname}} {
})
{{/swagger1AnnotationLibrary}}
{{/implicitHeadersParams.0}}
{{#useApiComposedAnnotation}}
{{#lambda.composedannotationlambda}}{{httpMethod}}{{/lambda.composedannotationlambda}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename it to lambda.composedannotation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS: This should also include updating the JavaDoc in ComposedRequestMappingAnnotationLambda.java.

@@ -98,6 +98,8 @@ public class SpringCodegen extends AbstractJavaCodegen
public static final String USE_SEALED = "useSealed";
public static final String OPTIONAL_ACCEPT_NULLABLE = "optionalAcceptNullable";
public static final String USE_SPRING_BUILT_IN_VALIDATION = "useSpringBuiltInValidation";
public static final String USE_API_COMPOSED_ANNOTATION = "useApiComposedAnnotation";
public static final String API_COMPOSED_ANNOTATION_LAMBDA = "apiComposedAnnotationLambda";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used anywhere, please remove it.

@@ -224,6 +228,9 @@ public SpringCodegen() {
cliOptions.add(CliOption.newBoolean(USE_SPRING_BUILT_IN_VALIDATION,
"Disable `@Validated` at the class level when using built-in validation.",
useSpringBuiltInValidation));
cliOptions.add(CliOption.newBoolean(USE_API_COMPOSED_ANNOTATION,
"Use @<method>Mapping instead of @RequestMapping(method = RequestMethod.<method>) when generating the API interfaces.",
useApiComposedAnnotation));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even make it true by default. What do you think? @dRbAndrade @wing328

@Override
public void execute(Template.Fragment fragment, Writer writer) throws IOException {
String text = fragment.execute();
writer.write(String.format("@%sMapping(",text.substring(0,1).toUpperCase()+text.substring(1).toLowerCase()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fails if the input String is empty. Would be nice if this case could be handled gracefully.

Comment on lines +234 to +235
{{/useApiComposedAnnotation}}
{{^useApiComposedAnnotation}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like you can remove these 2 lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants