Thierry Messer opened SPR-13195 and commented

If I extend a @Controller with the intention to replace the extended with the sub-classed one there is no possibility to disable a request mapping for a method of the super controller. Example:

@Controller
@RequestMapping("/XY")
public class ParentController {
    @RequestMapping("/")
    public ModelAndView index() {
        return new ModelAndView("my-index-view");
    }

    @RequestMapping("/list")
    public ModelAndView list() {
        return new ModelAndView("my-list-view");
    }
}

The sub-classed controller:

@Controller
@RequestMapping("/XY")
public class SubController extends ParentController {
    @Override
    @RequestMapping("/should-not-be-accessible")
    public ModelAndView list() {
        return null;
    }

    @Override
    @RequestMapping("/list")
    public ModelAndView list(@RequestParam(value = "selection", required = false) String selection) {
        ModelAndView mav = ModelAndView("my--specialized-list-view");
        mav.addObject("selection", selection);
        return mav;
    }
}

It would be great to support disabling request mapping definition of the parent controller by overriding methods using the following approach:

@Controller
@RequestMapping("/XY")
public class SubController extends ParentController {
    @Override
    @RequestMapping(ValueConstants.DEFAULT_NONE)
    public ModelAndView list() {
        return null;
    }

    @Override
    @RequestMapping("/list")
    public ModelAndView list(@RequestParam(value = "selection", required = false) String selection) {
        ModelAndView mav = ModelAndView("my--specialized-list-view");
        mav.addObject("selection", selection);
        return mav;
    }
}

I think it should be fairly easy to add an appropriate check to the method "getMappingForMethod" in the class "RequestMappingHandlerMapping".

Affects: 4.1.7

5 votes, 7 watchers

Comment From: spring-projects-issues

Rossen Stoyanchev commented

I see what you are requesting and it would be easy enough to do something like what you've suggested. From a design perspective however this doesn't strike me as particularly elegant and bulletproof since it requires using @RequestMapping in a specific way, e.g. add one pattern only, don't add other attributes, etc.

What I'm really interested to hear more about your use case, i.e. why do you need this in the first place? Typically with inheritance if a base class provides functionality that's useful to some sub-classes but not all, a new sub-class may be introduced. Or is this a case where you don't control the source code from the base class?

Comment From: spring-projects-issues

Thierry Messer commented

The motivation for this feature is the ability to extend a base-controller provided in a separate package (something like a base framework for similar web-applications that provide the same essential features) which is maintained by another department/party than the application built on top of it. So, instead of copying the whole controller implementation (moving the maintenance of the whole controller-logic to the application owner) it would be easier to only maintain the additional feature of the "overridden" method.

Example: Adding a further component (which is controlled using a request-parameter/path-variable) to the default implementation provided by the base framework:

@Controller
@RequestMapping("/XY")
public class SubController extends ParentController {
    @Override
    @RequestMapping(ValueConstants.DEFAULT_NONE)
    public ModelAndView list() {
        return list(null);
    }

    @Override
    @RequestMapping("/list")
    public ModelAndView list(@RequestParam(value = "selection", required = false) String selection) {
        ModelAndView mav = super.list();
        mav.setViewName("my-specialized-list-view");
        mav.addObject("selection", selection);
        return mav;
    }
}

The above example code demonstrates the implementation in case we only want to add a parameter "selection" for the additional component. If the parent controller calls the overridden method the call is forwarded to the new implementation which provides the data for the additional component. IMHO a simple and elegant solution for extending the functionality of an existing controller.

Comment From: spring-projects-issues

Rossen Stoyanchev commented

How about using delegation instead of extension?

@Controller
@RequestMapping("/XY")
public class SubController {

    private final ParentController delegate;

    // constructor accepting ParentController instance or instantiating it directly ...

    @Override
    @RequestMapping("/list")
    public ModelAndView list(@RequestParam(value = "selection", required = false) String selection) {
        ModelAndView mav = this.delegate.list();
        mav.setViewName("my-specialized-list-view");
        mav.addObject("selection", selection);
        return mav;
    }
}

Comment From: spring-projects-issues

Yaser Eftekhari commented

I have a use case for this issue, and I will be delighted to hear if spring team resolve it!

In one project, I have numerous controllers that would connect to management layer and they will provide basic functions like load, search, delete, ... on different data-services. In general, these functions behave in the same fashion, however, we have some cases where we need to exclude one function from the set of functions. As we want to prevent redundant code and go for readability, currently we just throw some error in the body of excluded routines in the sub-class.

It would be best if Spring provided an additional annotation or attribute in @RequestMapping (like disabled=true) so that we would be able to disable mapping of url rest request.

Comment From: spring-projects-issues

Rossen Stoyanchev commented

You could raise a dedicated exception, e.g. RequestMappingNotSupportedException, and handle it as a 404 in a global @ExceptionHandler method, i.e. declared in an @AdviceController, possibly even returning a more helpful message in the body to explain the deviation from the general pattern of the URL hierarchy.

Comment From: spring-projects-issues

M. Justin commented

I have a similar situation currently. We have a parent controller that defines standard CRUD operations for a RESTful resource, so that all our endpoints have similar functionality, regardless of the type of data it's storing. We have one controller that we're deprecating, where we want the GET methods to remain accessible for now so that existing clients continue to work, but we do not want to have them available to to be modified through PUT/POST/DELETE.

The easiest solution for us, if it were possible, would be to mark the PUT/POST/DELETE methods of the subclass as not accessible. As that's not possible, we've changed it to manually return a 405 response. The downside of this is that we lose all the stuff that knows what the "Allow" header should be allowing, since Spring MVC doesn't know that PUT/POST/DELETE aren't really allowed at that URL any more.

Comment From: spring-projects-issues

Rossen Stoyanchev commented

AbstractHandlerMethodMapping has methods to register mappings, as well as to unregister, which can be invoked at any time. You can use that on startup to unregister a list of mappings that would even be obtained from the method:

Method method = ... ;

RequestMapping annot = AnnotatedElementUtils.findMergedAnnotation(method, RequestMapping.class);

RequestMappingInfo info = RequestMappingInfo
    .paths(annot.path())
    .methods(annot.method())
    .params(annot.params())
    .headers(annot.headers())
    .consumes(annot.consumes())
    .produces(annot.produces())
    .build();

handlerMapping.unregister(info);

This could even be automated, for example by using some annotation such as @IgnoreRequestMapping which could be detected by some component on startup that then invokes the above code.

Comment From: spring-projects-issues

Alexey Melnikov commented

This could even be automated, for example by using some annotation such as @IgnoreRequestMapping which could be detected by some component on startup that then invokes the above code.

Am I wrong or this means, all my own components should be initialised after this "some component"?

Comment From: foal

Not implemented yet?

Comment From: PengShangwen

I also need this feature.

Comment From: ExxaD

Need this too, would be great feature