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

Support default URI prefix for web service @RequestMapping [SPR-13882] #18455

Closed
spring-projects-issues opened this issue Jan 22, 2016 · 15 comments
Closed

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Jan 22, 2016

David Cole opened SPR-13882 and commented

I had opened a ticket in Spring-Boot on this topic and it was suggested I create one on the Spring JIRA I have added the original ticket as the Reference URL.

There doesn't seem to be anyway, short of explicitly prefixing each individual
@RequestMapping
with a default URI like

@RequestMapping(path="*/api/*controller1")

at the top of the @RestController class, where /api is the default. I'm trying to have a clear separation in URL paths between normal application requests and web service requests, allowing me to provide additional security specific to web services.

I have an ongoing StackOverflow post, with no meaningful solution to-date and would like to suggest an enhancement to allow a default URI prefix to be supported somewhere in the Spring Boot application.properties configuration.

http://stackoverflow.com/questions/34801351/how-to-configure-a-default-restcontroller-uri-prefix-for-all-controllers

The result would be, using the example above, that my request mapping would be defined as:

@RequestMapping(path="controller1") 

The resulting URL would resolve to:

/<root_context>/api/controller1

I was hoping for more of a 'building block' approach, like:

@RequestMapping(path="/api")
class BaseController{
    ....
}

@RequestMapping(path="sub-class1")
class SubClassedController1 extends BaseController{
   @RequestMapping(path="get-something") 
   public void getSomething(){
       ...
    }
}

@RequestMapping(path="sub-class2")
class SubClassedController2 extends BaseController{
   @RequestMapping(path="get-something") 
   public void getSomething(){
       ...
    }
}

Resulting in

/api/sub-class1/get-something

and

/api/sub-class2/get-something

Affects: 4.2.3, 5.0.1

Reference URL: spring-projects/spring-boot#4994

Issue Links:

  • #20883 Ability to provide an external base path for controllers ("is superseded by")

6 votes, 12 watchers

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 28, 2016

Rossen Stoyanchev commented

Not sure this is a good idea. It makes it harder to understand URL structure, and in a hierarchy of even 2 (but especially more) levels, it leads to situations where you might want to extend vs override base class mappings. Besides that a change like this would break many applications that rely on current semantics of overriding base class mappings.

One thing you could so is use a constant or a property placeholder inside the mapping. It's not ideal but I think simple trumps the alternative in this case.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 1, 2016

Brian Clozel commented

In my opinion, the current arrangement already spreads the routing/mapping information quite a bit.
FWIW, I've seen many times that using too much inheritance at the Controller level usually leads to complex controllers and hard to track issues.

I guess that means I'm not really in favor of this enhancement.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 1, 2016

David Cole commented

I definitely understand the concern for backwards compatibility and the concern over introducing additional confusion. After reading Rossen's comment, I was inclined to suggest that you could add an attribute to the @RequestMapping like @RequestMapping(inherit=true), that would default to false to preserve backwards compatibility. Then the concern over possible confusion is left up to the team making the decision to explicitly set inherit=true. For us, we only intend to have one base Controller and I don't see this to be a major concern.

However, taking into account both Rossen's and Brian's comments, made me re-evaluate the request. I wasn't actually intending to drive the implementation, only suggest a possible solution. Remember, my main requirement was to find a way to have all @RestController's endpoints respond to a set prefix like '/api'. Possibly a better solution would be to have a configuration setting (My intial ticket was opened on the SpringBoot side), to set the default prefix. So, maybe a configuration setting would be less confusing and make more sense.

Thoughts?

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 2, 2016

Rossen Stoyanchev commented

A solution more focused on the problem makes sense. I wonder how it would be configured. You have to specify a common prefix and then associate it with a subset of controllers perhaps referring to packages or something like that. It doesn't seem to like a clear win to me.

I'm wondering if your main concerns is typing the prefix everywhere or the ability to change the prefix externally? The latter is possible today with a property placeholder:

@RequestMapping(path="${prefix}/foo")

Despite the replication there is and argument to be made that seeing this in mappings is helpful.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 3, 2016

David Cole commented

Actually my primary concern is neither the need to manually place the prefix in the @RequestMapping on every Controller, nor the need to change it in a single place. Although, both are reasonable concerns. Generally, the property placeholder does solve the issue of making the change in a single place and having the prefix visible in the mapping does add some level of clarity. On the clarity part, an externally configured prefix is really no different to me than having property to set the root application context like using: server.contextPath=/MyWebApp and doesn't feel like it adds any confusion.

The main concern that caused me to look for a solution is to have a guaranteed starting point like /api in the URL that I can use to my advantage to specifically secure web service calls, which wouldn't apply to other URI's like /images /js /*.html, etc.

We do use IntelliJ with FileTemplates and LiveTemplates, which allows me to generate the initial Controller class with an initial RequestMapping configured with the prefix, so that's not a problem. I was mainly looking for a more guaranteed way to be sure that a developer could not accidentally or unknowingly cut/past or remove the prefix and create a 'hole', which circumvents any effort to secure the web service endpoints.

This is not an absolute must for our project, I'm just trying to come up with the best implementation scenario I can. If you guys don't feel a change would add value, I'm OK with that and will work with what is available. In considering it, I just wanted to make sure it was clear on what I was trying to achieve and why to see if that demonstrates enough value for making some type of change that someone else could also benefit from (including myself :) ).

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 4, 2016

Rossen Stoyanchev commented

There is a difference here, unlike setting a contextPath which is global to the entire application, if I understand correctly you're talking about sub-dividing controllers into at least two groups. That means you have to specify both a prefix as well as some way of indicating which controllers it should apply to, similar to the options available on @ControllerAdvice. That in turn still leaves room for errors.

For the problem at hand have you considered writing a simple unit test that instantiates your Web configuration, obtains the RequestMappingHandlerMapping, and then iterates and validates all request mappings (accessible via getHandlerMethods)? You can assert that each starts with one of the expected prefixes. You can also check which controller a mapping is mapped to and assert whether the prefix is expected for the given controller based on its package, type, annotation, etc.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 4, 2016

David Cole commented

Thanks for the feedback and comments guys. At this point I think this is my best option:

  1. Use the property placeholder to prefix each Controller level @RequestMapping like:
@RequestMapping(path="${prefix}/foo")

Where prefix will be extracted into a property in my SpringBoot application.properties

  1. Create a unit test that validates and asserts each Controller has the ${prefix} present in the @RequestMapping to insure all Controllers are compliant.

I really appreciate the time and effort each of you spent to consider and advise on best-case solution.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 4, 2016

Rossen Stoyanchev commented

Sounds pretty good. Thanks!

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2016

Olivier Liechti commented

I have used this successfully. But I am now trying to use Spring HATEOAS and when I generate the link with the linkTo method, it breaks. The problem is that down the line, the annotation is not interpreted as a Spring EL expression. Instead, the curly braces are interpreted as URI variables.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 5, 2016

Olivier Liechti commented

The proposed solution does not work when using Spring HATEOAS.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 7, 2016

Rossen Stoyanchev commented

The ${prefix} is not a Spring EL expression actually, it's a property placeholder and it should be pretty easy to update Spring HATEOAS to handle those as we do when we read the request mappings.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 7, 2016

Caleb Cushing commented

I think #19231 could be part of the solution, but I'm also thinking that maybe there should be some sort of "PrefixConfigurer" that can assign a path to several classes.

/v0/public/files/pdfs/id

context.path/configurer/package/class/method

oh and here's my current workaround

public final class RouteConstants {
    private static final int VERSION = 0;
    public static final String MOUNT = "/v" + VERSION;
    public static final String PUBLIC = MOUNT + "/public";
    public static final String PRIVATE = MOUNT + "/private";

    public static final String FILES = PRIVATE + "/files";
    private static final String REGISTRATION_PART = "/registration";
    public static final String REGISTRATION = PUBLIC + REGISTRATION_PART;
    private static final String AUTHENTICATION = "/authentication";
    private static final String PASSWORD = "/password";
    public static final String AUTH_PASS = PUBLIC + AUTHENTICATION + PASSWORD;


    private RouteConstants() {
    }
}

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 8, 2016

Caleb Cushing commented

it occurs to me that one might be able to accomplish this with composing annotations in some way if supported, would be better than an Abstract class, imho.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Nov 2, 2017

Rossen Stoyanchev commented

Assigning fix version to 5.x along with #20691.

Loading

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Jan 2, 2018

Rossen Stoyanchev commented

Resolving this as superseded by #20883.

Loading

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