Wrong warning logged for multiple @RequestMapping annotations

[membersound]

The following creates a false log warning message:

@ControllerDefaults
@RestController("/rest")
public class ExampleController {
   @GetMapping(params = "version=1")
   public void v1() {
       // ...
   }
   
   @GetMapping(params = "version=2")
   public void v2() {
        // ...
   }
}

@Target({ElementType.TYPE, ElementType.METHOD})
@Retention(RetentionPolicy.RUNTIME)
@RestController
@RequestMapping(value = "/rest",
       produces = {APPLICATION_JSON_VALUE, TEXT_PLAIN_VALUE},
       consumes = {APPLICATION_JSON_VALUE, APPLICATION_XML_VALUE})
public @interface ControllerDefaults {
}

Result on startup:

2024-08-14 09:18:18,068 [][] WARN [main] o.s.w.s.m.m.a.RequestMappingHandlerMapping: Multiple @RequestMapping annotations found on class ExampleController, but only the first will be used: [@org.springframework.web.bind.annotation.RequestMapping(consumes={"application/json"}, headers={}, method={}, name="", params={}, path={"/rest"}, produces={}, value={"/rest"}), @ExampleController(value={"/rest"})]
2024-08-14 09:18:18,069 [][] WARN [main] o.s.w.s.m.m.a.RequestMappingHandlerMapping: Multiple @RequestMapping annotations found on class ExampleController, but only the first will be used: [@org.springframework.web.bind.annotation.RequestMapping(consumes={"application/json"}, headers={}, method={}, name="", params={}, path={"/rest"}, produces={}, value={"/rest"}), @dExampleController(value={"/rest"})]

Which is wrong, because both endpoints are working correctly and can be accessed using:
localhost:8080/rest?version=1 or localhost:8080/rest?version=2.

This the change that introduced the problem: #31962

Probably the implementation simply forgot to evaluate if different params are set on the mappings.

[simonbasle]

I was not able to reproduce this, and the logs don't seem entirely consistent with the snippet. Please provide a fuller reproducer (additionally specifying the exact version that you are using).

[membersound]

I see what the logging caused now (updated comment above and also attached sample project):

I have a "common" annotation that defines many defaults for all implementing @RestController classes. So they can use the ControllerDefaults annotation instead of repeating all the stuff.

But in case a controller has to override some of those definitions, then the class has @ControllerDefaults plus @RestController annotated, which seems to cause the warning.

But still the warning is wrong in this special case, as both my endpoints are reachable.

[sbrannen]

Hi @membersound,

Thanks for the feedback.

I see what the logging caused now (updated comment above and also attached sample project):

I edited your updated comment, but due to an issue with caching in the browser, the link to your attached sample project got deleted, and I don't know how to reintroduce the link to that attachment. So, I apologize for that.

I have a "common" annotation that defines many defaults for all implementing @RestController classes. So they can use the ControllerDefaults annotation instead of repeating all the stuff.

But in case a controller has to override some of those definitions, then the class has @ControllerDefaults plus @RestController annotated, which seems to cause the warning.

I think I understand what you are trying to convey now; however, @RestController("/rest") will not result in the warning.

In addition, @RestController("/rest") does not make much sense. The string there is the name of the component, not the path to the controller.

In any case, annotating ExampleController with @ControllerDefaults and @RequestMapping("/rest") will result in a warning since @ControllerDefaults is meta-annotated with @RequestMapping.

Is that perhaps the scenario you encountered?

But still the warning is wrong in this special case, as both my endpoints are reachable.

Assuming you meant a setup similar to the following, yes, I agree that the warning can be potentially confusing.

package example;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

import org.junit.jupiter.api.Test;

import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.http.MediaType;
import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.context.WebApplicationContext;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup;

@SpringJUnitWebConfig
class ControllerTests {

	@Test
	void test(WebApplicationContext wac) throws Exception {
		MockMvc mockMvc = webAppContextSetup(wac).build();

		mockMvc.perform(get("/rest").param("version", "1"))
			.andExpectAll(
				status().isOk(),
				content().string("version1"));

		mockMvc.perform(get("/rest").param("version", "2"))
			.andExpectAll(
				status().isOk(),
				content().string("version2"));
	}


	@Configuration
	@EnableWebMvc
	@Import(ExampleController.class)
	static class Config {
	}

	@ControllerDefaults
	@RequestMapping("/rest")
	public static class ExampleController {

		@GetMapping(params = "version=1")
		public String v1() {
			return "version1";
		}

		@GetMapping(params = "version=2")
		public String v2() {
			return "version2";
		}
	}

	@Target({ ElementType.TYPE, ElementType.METHOD })
	@Retention(RetentionPolicy.RUNTIME)
	@RestController
	@RequestMapping(path = "/default",
		produces = { MediaType.APPLICATION_JSON_VALUE, MediaType.TEXT_PLAIN_VALUE },
		consumes = { MediaType.APPLICATION_JSON_VALUE, MediaType.APPLICATION_XML_VALUE })
	public @interface ControllerDefaults {
	}

}

When I run that test class, I see the following log output.

17:04:21.879 [main] WARN  o.s.w.s.m.m.a.RequestMappingHandlerMapping - Multiple @RequestMapping annotations found on class example.ControllerTests$ExampleController, but only the first will be used: [@org.springframework.web.bind.annotation.RequestMapping(consumes={}, headers={}, method={}, name="", params={}, path={"/rest"}, produces={}, value={"/rest"}), @example.ControllerTests$ControllerDefaults()]

However, as I stated in #31962 (comment):

In addition, please note that a locally declared @RequestMapping annotation will always override/hide @GetMapping, @PostMapping, etc.

Thus, I can see how one might expect that the locally declared @RequestMapping annotation on ExampleController (which overrides @ControllerDefaults) would not result in a warning for multiple @RequestMapping annotations on ExampleController.

We'll see if we can improve this.

[sbrannen]

But still the warning is wrong in this special case, as both my endpoints are reachable.

If your actual scenario is analogous to the example I provided above, the warning is technically correct in that the local @RequestMapping annotation (the first annotation) is used.

In any case, can you please confirm that your use case is analogous to the ControllerTests example I provided above?

Thanks

[membersound]

You perfectly reproduced my issue. Thanks a lot.

So you are saying, the warning is correct because the local @RequestMapping is overriding the inherited one?

So far I thought the warning message was about the endpoints that are accessible under the same path. But it seems the complain is more about the mapping annotations then.

[sbrannen]

You perfectly reproduced my issue.

Glad we were able to sort that out.

Thanks a lot.

You're welcome.

So you are saying, the warning is correct because the local @RequestMapping is overriding the inherited one?

Yes, the warning is technically correct.

The directly present @RequestMapping annotation takes precedence over the @RequestMapping annotation that is meta-present on the @ControllerDefaults annotation.

So far I thought the warning message was about the endpoints that are accessible under the same path. But it seems the complain is more about the mapping annotations then.

The warning is about potentially competing/conflicting @RequestMapping annotations at the class level in your particular scenario. This is made evident by the statement "Multiple @RequestMapping annotations found on class ..." in the log message. Thus, the method-level @RequestMapping annotations are not the issue.

In any case, as I mentioned above, although the warning is technically correct, I can see how it can be misleading since a directly present @RequestMapping annotation always takes precedence over any @RequestMapping annotation that is meta-present.

Thus, we may choose to avoid logging a warning in such cases, and we'll discuss this within the team.

[sbrannen]

After discussing this within the team, we decided to leave the warning as-is based on the following rationale.

1. The warning is technically correct.
2. The warning is the only way to be informed that competing @RequestMapping declarations are not supported (i.e., they are not merged and only the first one will be used).
3. The correct way to provide default mapping configuration is via @AliasFor in a composed annotation that allows the @RequestMapping attributes to be overridden.

For example, @ControllerDefaults could be redesigned (and renamed for improved clarity) as follows.

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Documented
@RestController
@RequestMapping
public @interface DefaultsRestController {

	@AliasFor(annotation = RequestMapping.class)
	String[] path() default "/default";

	@AliasFor(annotation = RequestMapping.class)
	String[] consumes() default { MediaType.APPLICATION_JSON_VALUE, MediaType.APPLICATION_XML_VALUE };

	@AliasFor(annotation = RequestMapping.class)
	String[] produces() default { MediaType.APPLICATION_JSON_VALUE, MediaType.TEXT_PLAIN_VALUE };

	@AliasFor(annotation = RequestMapping.class)
	String[] params() default {};

	@AliasFor(annotation = RequestMapping.class)
	String[] headers() default {};

}

The ExampleController can then be declared as follows, with only the @DefaultsRestController annotation and no additional, competing @RequestMapping annotation.

@DefaultsRestController(path = "/rest", consumes = MediaType.TEXT_PLAIN_VALUE)
public class ExampleController {
  // ...

And the test can be revised as follows, specifying the content type.

@Test
void test(WebApplicationContext wac) throws Exception {
	MockMvc mockMvc = webAppContextSetup(wac).build();

	mockMvc.perform(get("/rest")
			.contentType(MediaType.TEXT_PLAIN)
			.param("version", "1"))
		.andExpectAll(
			status().isOk(),
			content().string("version1"));

	mockMvc.perform(get("/rest")
			.contentType(MediaType.TEXT_PLAIN)
			.param("version", "2"))
		.andExpectAll(
			status().isOk(),
			content().string("version2"));
}

The combination of the above achieves what we believe were the original goals of the @ControllerDefaults annotation, and no warnings are issued for competing @RequestMapping annotations.

In light of that, I am closing this issue.

[membersound]

Thanks for the detailed feedback. I can fully relate to the explaination and will follow your suggestions to rewrite our code. Thanks for your time!