openapi-generator icon indicating copy to clipboard operation
openapi-generator copied to clipboard

[BUG] @RequestMapping annotation not allowed on @FeignClient interfaces

Open cTAbdIsi opened this issue 3 years ago • 11 comments

Bug Report Checklist

  • [x] Have you provided a full/minimal spec to reproduce the issue?
  • [x] Have you validated the input using an OpenAPI validator (example)?
  • [ ] Have you tested with the latest master to confirm the issue still exists?
  • [x] Have you searched for related issues/PRs?
  • [x] What's the actual output vs expected output?
  • [ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

Code generation with plugin failed with error @RequestMapping annotation not allowed on @FeignClient interfaces after update to 6.1.0

openapi-generator version

6.1.0

Generation Details

This is our configuration for the maven plugin:

<plugin>
    <groupId>org.openapitools</groupId>
    <artifactId>openapi-generator-maven-plugin</artifactId>
    <version>${openapi-generator-maven-plugin.version}</version>
    <configuration>
        <configOptions>
            <dateLibrary>java8</dateLibrary>
            <disableHtmlEscaping>true</disableHtmlEscaping>
            <hateoas>false</hateoas>
            <hideGenerationTimestamp>true</hideGenerationTimestamp>
            <interfaceOnly>true</interfaceOnly>
            <sourceFolder>models</sourceFolder>
            <useTags>true</useTags>
        </configOptions>
        <generateModels>true</generateModels>
        <generatorName>spring</generatorName>
        <library>spring-cloud</library>
        <skipIfSpecIsUnchanged>true</skipIfSpecIsUnchanged>
    </configuration>
    <executions>
        <execution>
            <id>generate-api</id>
            <goals>
                <goal>generate</goal>
            </goals>
            <configuration>
                <apiPackage>${project.groupId}.${project.artifactId}.api.v1</apiPackage>
                <inputSpec>${project.basedir}/docs/openapi.yaml</inputSpec>
                <modelPackage>${project.groupId}.${project.artifactId}.api.v1.model</modelPackage>
                <modelNameSuffix>ApiV1</modelNameSuffix>
            </configuration>
        </execution>
    </executions>
</plugin>

In openapi-generator version < 6.1.0, the plugin generate following code:

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen")
@Validated
@Tag(name = "Test")
public interface TestApi {
...
}

This is the result after updating to 6.1.0+.

@Generated(value = "org.openapitools.codegen.languages.SpringCodegen")
@Validated
@Tag(name = "Test")
@RequestMapping("${openapi.localServices.base-path:/test/v1}")
public interface TestApi {

We use this interface to setup the feign clients like this:

@FeignClient(name = "test",
             url = "${sdu.test.url:}",
             configuration = {TestApiConfig.class, ApiClientConfig.class})
public interface TestApiClient extends TestApi {
}
Steps to reproduce
Related issues/PRs
Suggest a fix

cTAbdIsi avatar Sep 21 '22 14:09 cTAbdIsi

The related change is probably https://github.com/OpenAPITools/openapi-generator/pull/10573 where @RequestMapping was moved from the controller to the interface.

dersvenhesse avatar Sep 22 '22 08:09 dersvenhesse

@RomainPruvostMHH and @welshm could you please habe look and revert this change?

MelleD avatar Sep 27 '22 15:09 MelleD

@RomainPruvostMHH and @welshm could you please habe look and revert this change?

I can try to take a look at this today or tomorrow

welshm avatar Sep 28 '22 11:09 welshm

@welshm i commented the issue in der merged PR modules/openapi-generator/src/main/resources/JavaSpring/api.mustache

Not sure if we have a condition, that we know that we use Feign on this time and can add @RequestMapping just if we don't use Feign.

MelleD avatar Sep 28 '22 13:09 MelleD

@welshm i commented the issue in der merged PR modules/openapi-generator/src/main/resources/JavaSpring/api.mustache

Not sure if we have a condition, that we know that we use Feign on this time and can add @RequestMapping just if we don't use Feign.

I think we have a condition to check (because it exists in other .mustache) but I will verify and post back here

welshm avatar Sep 28 '22 13:09 welshm

@welshm i commented the issue in der merged PR modules/openapi-generator/src/main/resources/JavaSpring/api.mustache Not sure if we have a condition, that we know that we use Feign on this time and can add @RequestMapping just if we don't use Feign.

I think we have a condition to check (because it exists in other .mustache) but I will verify and post back here

Ah nice, then i think it's an easy fix. Maybe i can look into it tomorrow

MelleD avatar Sep 28 '22 13:09 MelleD

@welshm found no condition. Thought {{^useSpringCloudClient}} could work, but did not find where this condition is set.

Tested it with

   @Test
   public void testNoRequestMappingAnnotation() throws IOException {
      final SpringCodegen codegen = new SpringCodegen();
      codegen.setLibrary( "spring-cloud" );

      final Map<String, File> files = generateFiles( codegen, "src/test/resources/2_0/petstore.yaml" );

      // Check that the @RequestMapping annotation is not generated in the Api file
      final File petApiFile = files.get( "PetApi.java" );
      JavaFileAssert.assertThat( petApiFile ).assertTypeAnnotations().hasSize( 3 ).containsWithName( "Validated" )
            .containsWithName( "Generated" ).containsWithName( "Tag" );

   }

MelleD avatar Sep 28 '22 19:09 MelleD

@welshm created a new boolean for the feign client: https://github.com/OpenAPITools/openapi-generator/pull/13546

If this is okay, i would adapt the other test(s)

MelleD avatar Sep 28 '22 19:09 MelleD

Please, fix this bug, because Spring Framework expects annotation @RequestMapping on type-level and start to register request mapping for such class. Such bug make impossible to use Feign client due to ambiguous mapping.

Here is code sample from Spring source code :

public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMapping
		implements MatchableHandlerMapping, EmbeddedValueResolverAware {

	/**
	 * {@inheritDoc}
	 * <p>Expects a handler to have either a type-level @{@link Controller}
	 * annotation or a type-level @{@link RequestMapping} annotation.
	 */
	@Override
	protected boolean isHandler(Class<?> beanType) {
		return (AnnotatedElementUtils.hasAnnotation(beanType, Controller.class) ||
				AnnotatedElementUtils.hasAnnotation(beanType, RequestMapping.class));
	}
}

DarrMirr avatar Oct 06 '22 12:10 DarrMirr

Please, fix this bug, because Spring Framework expects annotation @RequestMapping on type-level and start to register request mapping for such class. Such bug make impossible to use Feign client due to ambiguous mapping.

Here is code sample from Spring source code :

public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMapping
		implements MatchableHandlerMapping, EmbeddedValueResolverAware {

	/**
	 * {@inheritDoc}
	 * <p>Expects a handler to have either a type-level @{@link Controller}
	 * annotation or a type-level @{@link RequestMapping} annotation.
	 */
	@Override
	protected boolean isHandler(Class<?> beanType) {
		return (AnnotatedElementUtils.hasAnnotation(beanType, Controller.class) ||
				AnnotatedElementUtils.hasAnnotation(beanType, RequestMapping.class));
	}
}

Are you suggesting that the annotation should be on the controller implementation as opposed to the interface?

Can you take a look at the output from https://github.com/OpenAPITools/openapi-generator/pull/13546 and see if that is what you'd expect? That will remove the annotation from the interface, but does not re-add it to the controller.

welshm avatar Oct 06 '22 13:10 welshm

Hello @welshm. Thank you for response and link to #13546. I think it solve this issue.

DarrMirr avatar Oct 10 '22 13:10 DarrMirr

Hello,

May I know when this fix would be released?

Thank you!

vvinod1310 avatar Oct 31 '22 15:10 vvinod1310

@vvinod1310 this is part of 6.2.1

borsch avatar Jan 13 '23 12:01 borsch

Is anyone else encountering this bug when using the Kotlin-Spring generator with library spring-cloud?

We're using openapi-generator version: 6.6.0

Our generated interface:

@Validated
@RequestMapping("\${api.base-path:/api/test/v1}")
interface TestApi {

The generated FeignClient:

@FeignClient(
    name="Test",
    url="\${test.url:http://localhost/api/test/v1}", 
    configuration = [ClientConfiguration::class]
)
interface TestApiClient : TestApi

Then when running our application we encounter the same exception: @RequestMapping annotation not allowed on @FeignClient interfaces

Think the fix made in this pull request should also be implemented in the Kotlin-Spring generator right? Or am I missing something?

ctbarnev avatar Jun 22 '23 14:06 ctbarnev

I created a MR to fix this issue also for the Kotlin-Spring generator, hopefully someone can have a look?

ctbarnev avatar Jun 23 '23 06:06 ctbarnev