Spring Framework
  1. Spring Framework
  2. SPR-10222

Allow @ControllerAdvice to be cofigured with a join point to target a subset of controller

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: 3.2 GA, 3.2.1
    • Fix Version/s: 4.0 RC1
    • Component/s: Web
    • Labels:
      None
    • Last commented by a User:
      true

      Description

      I have two types of controllers in my spring application.

      • View controllers that forward to views to generate HTML
      • API controllers that return JSON directly from the controllers

      Both the API and View controllers are part of the same spring dispatcher servlet.

      The documentation implies that @ControllerAdvice will be applied to every controller associated with a Dispatcher Servlet. With advice as part of the name I expected to be able to specify the pointcut and or join points that the advice applies to, but can't find out how that can be.

      For example in my scenario I want a @ControllerAdvice for my View Controllers and separate @ControllerAdvice for my API controllers.

      It would be great to provide a way to configure which controllers @ControllerAdvice will apply to.

        Activity

        Hide
        Adib Saikali added a comment -

        Hi Brian!

        I am not using the spring-plugin project. At the start my application was partitioned by stereotype of object. I had packages such as com.example.web,com.example.persistence,com.exmaple.service ... etc but as the application grew and some features were stable and other new features were being rapidly added the organization of the code would not scale.

        So I have since partitioned my application into 5 utility modules and 20 separate feature modules that produce jars, and 1 web module that produces a war. The web module only contains static resources, js files, and few traditional MVC controller rather than rest controllers.

        By doing the split into feature modules working on the application became a lot easier. It is much easier to find all the code for a feature, git history is easier to deal with to see the evolution of a specific feature, there are feature modules that I have not looked at in months because they just work and when refactoring they don't get touched.

        Building on top of component scanning, Spring Java Config, and flywaydb organizing into feature modules is a better solution and more practical solution, for integration testing you can easily start up subsets of the whole application just run the tests for those parts of the system. So if I am working on feature module #7 I can use the Spring MVC Test to just test my controller, services, repositories all the way down to the database simply from the the IDE. I have integration testng tests that launch an embedded tomcat and through the magic of component scanning end up launching a subset of the application which is great for isolating where problems and keep the application rest backend partitioned into services that can be split into into separate servers when the time is right.

        I don't know if my use case is the most common use case but I believe that it is the better programming model for working with spring based applications. I think that projects such as Spring Boot with its emphasis on an just having a main method to launch things from will make decomposition into feature modules more desirable and practical. So I hope that you create a design and implementation that can accommodate the feature module use case for Spring 4.0.

        Show
        Adib Saikali added a comment - Hi Brian! I am not using the spring-plugin project. At the start my application was partitioned by stereotype of object. I had packages such as com.example.web,com.example.persistence,com.exmaple.service ... etc but as the application grew and some features were stable and other new features were being rapidly added the organization of the code would not scale. So I have since partitioned my application into 5 utility modules and 20 separate feature modules that produce jars, and 1 web module that produces a war. The web module only contains static resources, js files, and few traditional MVC controller rather than rest controllers. By doing the split into feature modules working on the application became a lot easier. It is much easier to find all the code for a feature, git history is easier to deal with to see the evolution of a specific feature, there are feature modules that I have not looked at in months because they just work and when refactoring they don't get touched. Building on top of component scanning, Spring Java Config, and flywaydb organizing into feature modules is a better solution and more practical solution, for integration testing you can easily start up subsets of the whole application just run the tests for those parts of the system. So if I am working on feature module #7 I can use the Spring MVC Test to just test my controller, services, repositories all the way down to the database simply from the the IDE. I have integration testng tests that launch an embedded tomcat and through the magic of component scanning end up launching a subset of the application which is great for isolating where problems and keep the application rest backend partitioned into services that can be split into into separate servers when the time is right. I don't know if my use case is the most common use case but I believe that it is the better programming model for working with spring based applications. I think that projects such as Spring Boot with its emphasis on an just having a main method to launch things from will make decomposition into feature modules more desirable and practical. So I hope that you create a design and implementation that can accommodate the feature module use case for Spring 4.0.
        Hide
        Rossen Stoyanchev added a comment -

        Adib Saikali, just to be sure this is clear, you don't need to specify any packages at all if you want @ControllerAdvice to apply everywhere. You probably know that but I mention it because it wasn't clear to me from the example you gave what controllers need to be excluded. That's a fairly important question in order to understand how this applies to your use case.

        Show
        Rossen Stoyanchev added a comment - Adib Saikali , just to be sure this is clear, you don't need to specify any packages at all if you want @ControllerAdvice to apply everywhere. You probably know that but I mention it because it wasn't clear to me from the example you gave what controllers need to be excluded. That's a fairly important question in order to understand how this applies to your use case.
        Hide
        Adib Saikali added a comment -

        Rossen Stoyanchev, Thanks for asking me to clarify the usecase. Here is my ultimate use case in more detail.

        I have a utility module called rest-utils.jar in which has a bunch of code that allows to do global error handling for REST APIs by having a RestApiController base class and it has helper classes for working with SpringRestTemplates for example there is a nice login method to log in to the API, there are nice tracing options to print out the request json and response json so I can use these helpers to create example of how to use the API that the JavaScript developers can use. Much of the code in res-utils.jar is not specific to one application can be used in many different apps that use the same architecture as the application I am currently working on.

        What I want is way to put global error handling code in rest-utils.jar such that if I drop the rest-utils.jar into my classpath of an application then the error handling logic in the rest-utils.jar would apply to those controllers.

        Here is a subset of what is currently in my BaseApiController which is in rest-utils.jar

        @Controller
        public class BaseApiController
        {
        	private static Logger logger = LoggerFactory.getLogger(BaseApiController.class);
        	protected final static String JSON = "application/json";
         
        	protected ResponseEntity<RestApiError> createResponseEntity(RestApiError restApiError)
        	{
        		HttpHeaders headers = new HttpHeaders();
        		headers.setContentType(MediaType.APPLICATION_JSON);
         
        		HttpStatus responseStatus = HttpStatus.valueOf(restApiError.getHttpStatus());
        		ResponseEntity<RestApiError> result = new ResponseEntity<>(restApiError, headers, responseStatus);
        		return result;
        	}
         
        	@ExceptionHandler(RestApiException.class)
        	public ResponseEntity<RestApiError> handleRestApiException(RestApiException e)
        	{
        		logger.error("Api Error caused by exception", e);
        		return this.createResponseEntity(e.getRestApiError());
        	}
         
         
        	@ExceptionHandler(InvalidInputException.class)
        	public ResponseEntity<RestApiError> handleInvalidInputException(InvalidInputException e)
        	{
        		logger.error("Api Error caused by exception", e);
        		RestApiError restApiError = new RestApiError(RestApiHttpStatus.BAD_REQUEST);
        		restApiError.setApiCode(ApiErrorCodes.ERROR_CODE_TYPE_MISMATCH_EXCEPTION);
        		restApiError.setUserMessage("An Error has occured and the request could not be completed");
        		restApiError.setDeveloperMessage(e.getMessage());
         
        		return createResponseEntity(restApiError);
        	}
        }
         

        Here is what a feature controller would look like in my news.jar which also contains the database entities, services ... etc.

        @Controller
        public class NewsController extends BaseApiController
        {
        	@Autowired
        	private NewsService newsService;
         
        	@RequestMapping(value = Company.NEWS_POSTS, method = RequestMethod.POST)
        	@ResponseStatus(value = HttpStatus.OK)
        	@ResponseBody
        	public NewsPostJson createNewPost(@Valid @PathVariable("id") Integer id, @Valid @RequestBody NewsPostJson newsPostJson)
        	{
        		
        	}
        }

        What I want to be able to do is to convert the BaseApiController into some sort of advice such that the NewsController would not need to extend BaseApiController but when I drop rest-utils.jar into the application classpath I get the standard error handling mechanism applied to my rest controllers. I know this make previous suggestion to target the advice based on the class subtype wrong.

        If it was possible for me to define my own controller sterotype like so.

        @Target({ElementType.TYPE})
        @Retention(RetentionPolicy.RUNTIME)
        @Controller
        public @interface StandardController 
        {
        }

        Then the BaseApiController becomes

        @ControllerAdvice(steroype=StandardController.class)
        public class StandardApiErrorHandler
        {
        	private static Logger logger = LoggerFactory.getLogger(BaseApiController.class);
        	protected final static String JSON = "application/json";
         
        	protected ResponseEntity<RestApiError> createResponseEntity(RestApiError restApiError)
        	{
        		HttpHeaders headers = new HttpHeaders();
        		headers.setContentType(MediaType.APPLICATION_JSON);
         
        		HttpStatus responseStatus = HttpStatus.valueOf(restApiError.getHttpStatus());
        		ResponseEntity<RestApiError> result = new ResponseEntity<>(restApiError, headers, responseStatus);
        		return result;
        	}
         
        	@ExceptionHandler(RestApiException.class)
        	public ResponseEntity<RestApiError> handleRestApiException(RestApiException e)
        	{
        		logger.error("Api Error caused by exception", e);
        		return this.createResponseEntity(e.getRestApiError());
        	}
         
         
        	@ExceptionHandler(InvalidInputException.class)
        	public ResponseEntity<RestApiError> handleInvalidInputException(InvalidInputException e)
        	{
        		logger.error("Api Error caused by exception", e);
        		RestApiError restApiError = new RestApiError(RestApiHttpStatus.BAD_REQUEST);
        		restApiError.setApiCode(ApiErrorCodes.ERROR_CODE_TYPE_MISMATCH_EXCEPTION);
        		restApiError.setUserMessage("An Error has occured and the request could not be completed");
        		restApiError.setDeveloperMessage(e.getMessage());
         
        		return createResponseEntity(restApiError);
        	}
        }
         

        I hope this clarifies my use case.

        Show
        Adib Saikali added a comment - Rossen Stoyanchev, Thanks for asking me to clarify the usecase. Here is my ultimate use case in more detail. I have a utility module called rest-utils.jar in which has a bunch of code that allows to do global error handling for REST APIs by having a RestApiController base class and it has helper classes for working with SpringRestTemplates for example there is a nice login method to log in to the API, there are nice tracing options to print out the request json and response json so I can use these helpers to create example of how to use the API that the JavaScript developers can use. Much of the code in res-utils.jar is not specific to one application can be used in many different apps that use the same architecture as the application I am currently working on. What I want is way to put global error handling code in rest-utils.jar such that if I drop the rest-utils.jar into my classpath of an application then the error handling logic in the rest-utils.jar would apply to those controllers. Here is a subset of what is currently in my BaseApiController which is in rest-utils.jar @Controller public class BaseApiController { private static Logger logger = LoggerFactory.getLogger(BaseApiController.class); protected final static String JSON = "application/json";   protected ResponseEntity<RestApiError> createResponseEntity(RestApiError restApiError) { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON);   HttpStatus responseStatus = HttpStatus.valueOf(restApiError.getHttpStatus()); ResponseEntity<RestApiError> result = new ResponseEntity<>(restApiError, headers, responseStatus); return result; }   @ExceptionHandler(RestApiException.class) public ResponseEntity<RestApiError> handleRestApiException(RestApiException e) { logger.error("Api Error caused by exception", e); return this.createResponseEntity(e.getRestApiError()); }     @ExceptionHandler(InvalidInputException.class) public ResponseEntity<RestApiError> handleInvalidInputException(InvalidInputException e) { logger.error("Api Error caused by exception", e); RestApiError restApiError = new RestApiError(RestApiHttpStatus.BAD_REQUEST); restApiError.setApiCode(ApiErrorCodes.ERROR_CODE_TYPE_MISMATCH_EXCEPTION); restApiError.setUserMessage("An Error has occured and the request could not be completed"); restApiError.setDeveloperMessage(e.getMessage());   return createResponseEntity(restApiError); } }   Here is what a feature controller would look like in my news.jar which also contains the database entities, services ... etc. @Controller public class NewsController extends BaseApiController { @Autowired private NewsService newsService;   @RequestMapping(value = Company.NEWS_POSTS, method = RequestMethod.POST) @ResponseStatus(value = HttpStatus.OK) @ResponseBody public NewsPostJson createNewPost(@Valid @PathVariable("id") Integer id, @Valid @RequestBody NewsPostJson newsPostJson) { } } What I want to be able to do is to convert the BaseApiController into some sort of advice such that the NewsController would not need to extend BaseApiController but when I drop rest-utils.jar into the application classpath I get the standard error handling mechanism applied to my rest controllers. I know this make previous suggestion to target the advice based on the class subtype wrong. If it was possible for me to define my own controller sterotype like so. @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Controller public @interface StandardController { } Then the BaseApiController becomes @ControllerAdvice(steroype=StandardController.class) public class StandardApiErrorHandler { private static Logger logger = LoggerFactory.getLogger(BaseApiController.class); protected final static String JSON = "application/json";   protected ResponseEntity<RestApiError> createResponseEntity(RestApiError restApiError) { HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON);   HttpStatus responseStatus = HttpStatus.valueOf(restApiError.getHttpStatus()); ResponseEntity<RestApiError> result = new ResponseEntity<>(restApiError, headers, responseStatus); return result; }   @ExceptionHandler(RestApiException.class) public ResponseEntity<RestApiError> handleRestApiException(RestApiException e) { logger.error("Api Error caused by exception", e); return this.createResponseEntity(e.getRestApiError()); }     @ExceptionHandler(InvalidInputException.class) public ResponseEntity<RestApiError> handleInvalidInputException(InvalidInputException e) { logger.error("Api Error caused by exception", e); RestApiError restApiError = new RestApiError(RestApiHttpStatus.BAD_REQUEST); restApiError.setApiCode(ApiErrorCodes.ERROR_CODE_TYPE_MISMATCH_EXCEPTION); restApiError.setUserMessage("An Error has occured and the request could not be completed"); restApiError.setDeveloperMessage(e.getMessage());   return createResponseEntity(restApiError); } }   I hope this clarifies my use case.
        Hide
        Brian Clozel added a comment - - edited

        Adib Saikali, thank for this valuable feedback.
        I just updated the PR (still work in progress though) to give you a chance to take another look at it.
        You can check one of the test classes for examples and the new documentation for @ControllerAdvice.

        Thanks!

        Show
        Brian Clozel added a comment - - edited Adib Saikali , thank for this valuable feedback. I just updated the PR (still work in progress though) to give you a chance to take another look at it. You can check one of the test classes for examples and the new documentation for @ControllerAdvice . Thanks!
        Hide
        Adib Saikali added a comment -

        Brian this is looking really great, being able to select controller to apply the advice to based on subtypes, packages, or annotations is really cool, I think this will meet my needs. Thanks very much for being open to the feedback.

        Show
        Adib Saikali added a comment - Brian this is looking really great, being able to select controller to apply the advice to based on subtypes, packages, or annotations is really cool, I think this will meet my needs. Thanks very much for being open to the feedback.

          People

          • Assignee:
            Brian Clozel
            Reporter:
            Adib Saikali
            Last updater:
            Phil Webb
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              Days since last comment:
              1 year, 24 weeks, 3 days ago