Skip to content
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

@HandlerInterceptor [SPR-4770] #9447

Closed
spring-issuemaster opened this issue May 1, 2008 · 7 comments
Closed

@HandlerInterceptor [SPR-4770] #9447

spring-issuemaster opened this issue May 1, 2008 · 7 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented May 1, 2008

David Pedowitz opened SPR-4770 and commented

Introduce @HandlerInterceptor to create a more symmetrical relationship between @Controller, @ModelAttribute and @RequestParam.

This came out of some questions and brainstorming in a Spring training session in LA, 5/1/08 with Chris Beams, Tchavdar Ivanov (FIM) and myself.

In Spring 2.5 the implementation of HandlerInterceptor has not changed leaving you to either implement the interface or extend HandlerInterceptorAdapter as before. If you use @Controller and component scan you still must define a DefaultAnnotationHandlerMapping to configure a single list of HandlerInterceptors for all Controllers.

It'd be nice if you could define a class as a @HandlerInterceptor and annotate methods as @AfterCompletion, @PreHandle and @PostHandle. The default behavior could automagically intercept all @Controllers scanned and fire according to the same contract as the original HandlerInterceptor. Also interaction could be similar to @Controller with the methods allowing use of @ModelAttribute and @RequestParam.

Example provided by Chris Beams:

// @HandlerInterceptors are @Components, thus eligible for component scanning.
// If no class args are supplied, the interceptor applies globally to all 
// registered @Controllers.  Naturally, specifying one or more classes narrows 
// its scope.

@HandlerInterceptor({AccountController.class, CustomerController.class}) public class ExceptionLoggingHandlerInterceptor {
	@AfterCompletion
	public void logException(Object handler, Throwable ex) {
		logger.error(...);
	}

	// Or to be more explicit, do it @AspectJ-style, using
	// attributes to bind by name

	@AfterCompletion(handler="handler", exception="ex")
	public void logException(Object handler, Throwable ex) {
		logger.error(...);
	}

	// In keeping with @Controller methods, the user could
	// request the request, response, model, etc.

	@AfterCompletion(handler="handler", exception="ex")
	public void logException(HttpServletRequest req, Object handler, Throwable ex) {
		logger.error(...);
	}

	// Could even have handler methods return @ModelAttributes!

	@PreHandle
	public @ModelAttribute("currentDate") Date currentDate(/* no params needed, none requested */) {
		return new Date();
	}

}

Obviously there's a lot to be worked out. Some questions we have are:

  • How do you express the order of the interceptors, something that comes naturally in the <list> configuratin. @Order?
  • Is it more natural to express the Controller classes the HandlerInterceptor applies to in the @HandlerInterceptor (above) or would something like @Controller(interceptors=(@HandlerInterceptors{class, class, ...})) be more expressive? which is more flexible?
    • The later does express the list naturally but you would have to repeat the list in multiple controllers to achieve the same behavior.
    • Maybe an XML annotation hybrid? <handlerInterceptorList id="authInterceptors">...</handlerInterceptorList> and @Controller(interceptor="authInterceptors")
  • Could tooling help visualize how an @Controller relates to it @HandlerInterceptors (add bonus for sure!)

Issue Links:

  • #17959 Apply Filter or Interceptor to Controller class via annotations ("is duplicated by")
  • #8695 Ability to restrict HandlerInterceptors to specific controller paths

14 votes, 16 watchers

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 24, 2008

Dave Syer commented

Some comments:

  • I don't like the "@AspectJ" style. It doesn't seem necessary and it is more in keeping with @Controller to use a parameter annotation to denote the handler instance
  • HandlerInterceptors are not traditionally applied per controller, so I'm not sure if that should change (although if the @Handler method parameter was strongly typed, that would select the controller)
  • Controllers should not know about their interceptors (interceptors have a role outside the controller's responsibilities, before and after rendering)
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 24, 2008

Dave Syer commented

The RESTful style parameter extractions (@UriParam) would be useful as would the @RequestParam method parameters.

I'm a litle suspicious of @ModelAttribute in a HandlerInterceptor (e.g. see #6721), but open to suggestion if anyone wants to provide some use cases.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Feb 11, 2010

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Feb 21, 2011

christophe blin commented

Just to say my 2 cents on "How do you express the order of the interceptors" : I'd like the @Ordered annotation to be used

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 4, 2011

Rafael Naufal commented

I think it's good to be able to use @ModelAttribute in a HandlerInterceptor. As an example, at our project here some interceptors need to obtain the logged user after the authentication completes. I think it would be nice to have some way to get the user from the model, as the code below suggests:

@PreHandle
public void beforeExecution(@ModelAttribute("loggedUser") User user) { 

}
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 21, 2014

Rossen Stoyanchev commented

Rafael Naufal, note that the model is not available in preHandle. According to the HandlerAdapter contract, a ModelAndView is returned as a result of the invocation of the controller. This is pretty much reflected the method signatures of HandlerInterceptor.

What it sounds like you could use is an @ControllerAdvice with one ore more @ModelAttribute methods. Those would be applicable to a subset or to all controllers. It is invoked before the execution of the controller's @RequestMapping method so it's very much a "preHandler" style invocation.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Aug 21, 2014

Rossen Stoyanchev commented

In contrast to controllers, HandlerInterceptor's are few and well suited to a simple interface-based contract.

As mentioned in my last comment an @ControllerAdvice may fulfill some needs and can be applied selectively to all or a specific subset of controllers.

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.