ControllerAdvice order is not preserved in AnnotatedControllerExceptionResolver

[yodono]

Hello,

When you have many @ControllerAdvice classes, the @Order given to each bean is not respected.

I built a sample project demonstrating this behavior: https://github.com/yodono/spring-graphql-advice-order. Despite one of my classes having an @Order(Ordered.HIGHEST_PRECEDENCE) annotation, it does not run first.

It seems that AnnotatedControllerExceptionResolver is using a ConcurrentHashMap when registering beans in registerControllerAdvice. Later, when resolveException is called, the entries of this Map are iterated over but keeping no reference to the original order.

As a suggestion, there could be an extra call to OrderComparator before iterating over the controllerAdviceCache.

List<Map.Entry<ControllerAdviceBean, MethodResolver>>
    entryList = new ArrayList<>(this.controllerAdviceCache.entrySet());

entryList.sort(Map.Entry.comparingByKey(OrderComparator.INSTANCE));

for (Map.Entry<ControllerAdviceBean, MethodResolver> entry : entryList) {
    ControllerAdviceBean advice = entry.getKey();
    ...
}

[bclozel]

Thanks @yodono for this report! The fix will be available in the 1.2.4-SNAPSHOT version in a few minutes.

[notrealgandalf]

Hello @bclozel. Sorry for reopening but after this change I can't create multiple @ControlledAdvice classes without adding explicit @Order. Wwhen I'm trying to use ConcurrentHashMap again, it works.

Example from tests with additional advice (handler moved from controller)

        @SuppressWarnings("unused")
	@ControllerAdvice
	private static class TestControllerAdvice {
		@GraphQlExceptionHandler
		GraphQLError handle(IllegalArgumentException ex) {
			return GraphQLError.newError().message("handle: " + ex.getMessage()).build();
		}

	}

	@ControllerAdvice
	private static class AnotherAdvice {
		@GraphQlExceptionHandler
		GraphQLError handleToObject(ClassCastException ex) {
			return GraphQLError.newError().message("handle: " + ex.getMessage()).build();
		}

	}


	@SuppressWarnings("unused")
	@ControllerAdvice
	private static class OrderedTestControllerAdvice {
		@GraphQlExceptionHandler
		GraphQLError handle(IllegalArgumentException ex) {
			return GraphQLError.newError().message("ordered handle: " + ex.getMessage()).build();
		}

	}

[bclozel]

I don't understand. Can you create a new issue and share a sample application that demonstrates the problem?