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

Allow customizing "base" path in MvcUriComponentsBuilder (e.g. path prefix for locale) [SPR-12617] #17218

Closed
spring-issuemaster opened this issue Jan 11, 2015 · 20 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jan 11, 2015

Christian Rudolph opened SPR-12617 and commented

Google recommends to use country specific (toplevel) domains, subdomains or subdirectories to indicate the locale of a sites content.

Spring provides out of the box support for url parameters to change the locale of a site (explicitly not recommended by google) when using a LocaleChangeInterceptor. It should be relatively easy to create an interceptor that extracts the locale from the TLD or a subdomain, but creating "directories" that indicate the locale is not possible as of spring 4.1.4 in a noninvasive way.

The urls should look like this:
http://www.example.com/en/foo

It is easy to write an interceptor/localeResolver to extract the locale part from the url. But request mapping becomes a pain, because every controller must be aware of the locale that is present in the url. It becomes more difficult, if one would like
http://www.example.com/foo
to point to an automatically evaluated locale (AcceptHeaderLocale, IP lookup, default locale).

I tried to solve the problem by extending UrlPathHelper to strip the locale part from the url. Then the mapping becomes easy again.

The problem is, that the MvcUriComponentsBuilder is not aware of the fact that there is some stripped part of the url. At least when using the spring url tag library to construct urls from controller methods (the tag utilizes MvcUriComponentsBuilder#fromMappingName), the stripped part is lost. I didn't test it, but all factory methods of MvcUriComponentsBuilder should lose the locale part.

It should be possible to to configure a strategy to manipulate the retrieved url path, like it is possible for controller method arguments using UriComponentsContributor s. With such a strategy, it should be possible to restore the stripped part of the url.


Affects: 4.1.4

Attachments:

Issue Links:

  • #17397 Allow use of MvcUriComponentsBuilder independent of Servlet request lifecycle

Referenced from: commits 2b528bb, 1cd0f43, 0ddcbce

1 votes, 4 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 13, 2015

Rossen Stoyanchev commented

At least when using the spring url tag library to construct urls from controller methods ... the stripped part is lost.

This could be easily misinterpreted. I think you mean that the stripped "re-appears" since MvcUriComponentsBuilder uses information from the HttpServletRequest which has the locale in the path. And yes the UrlPathHelper mainly affects determination of the "look path" that is used for request mapping purposes.

So is your goal to strip the path entirely? If yes have you considered a filter to wrap the request to strip the path and maybe expose the locale as a request parameter instead?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 13, 2015

Christian Rudolph commented

This could be easily misinterpreted. I think you mean that the stripped "re-appears" since MvcUriComponentsBuilder uses information from the HttpServletRequest which has the locale in the path.
I mean it exactly as you first thought I mean it. The wanted behaviour is that the stripped part "re-appears" in the generated urls.
The MvcUriComponentsBuilder uses the ServletUriComponentsBuilder to construct the major part of the url. The interesting method is MvcUriComponentsBuilder#fromMethod, especially the line ServletUriComponentsBuilder.fromCurrentServletMapping().path(path). path was constructed from the RequestMapping annotations on the controller and method. It is prepended with the scheme, hostname, port and servlet mapping from the current request context.
Because the stripped part with the locale is after the servlet mapping, it doesn't get reconstructed by ServletUriComponentsBuilder.

I attach an example project showing this behaviour. I suggest stepping through MvcUriComponentsBuilder#fromMethod when loading http://localhost:8080/SPR-12617/en-us/foo/bar

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2015

Rossen Stoyanchev commented

Okay I see now. Indeed while "/en" is in the request path, in MvcUriComponentsBuilder we only use the context and servlet mapping portions of the request path while "/en" is logically part of the controller mappings but in your case not actually present in any @RequestMapping.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2015

Christian Rudolph commented

Exactly. That's why I think there is a need for something like a counterpart for UrlPathHelper that can be configured as simple like a UriComponentsContributor.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 14, 2015

Rossen Stoyanchev commented

All methods in MvcUriComponentsBuilder result in either fromController or fromMethod both of which in turn call ServletUriComponentsBuilder.fromCurrentServletMapping() and then append some more to the path and/or the query. So any customization mechanism would make sense to be applied from ServletUriComponentsBuilder, which would give us more universally a solution for all cases (code in controller methods or in views).

In fact ServletUriComponentsBuilder already does something similar where it detects "X-Forwarded-Prefix" added by proxies that removed a prefix. Only in this case it is a prefix that applies to controller mappings and so it needs to be appended to the paths returned from ServletUriComponentsBuilder (not prepended). So that's where we need to focus, the question is what exactly do we provide? It's not clear yet.

The simplest option might be to support a special ServletUriComponentsBuilder-specific request attribute containing the prefix. Beyond that somehow this scenario implies strongly there is a custom UrlPathHelper at work that removes some prefix from the lookup path. In MvcUriComponentsBuilder we actually can get access to the custom UrlPathHelper in use (currently we lookup the RequestMappingHandlerMapping for mapping names) but that's not entirely fail-safe (e.g. there can be multiple instances of a RequestMappingHandlerMapping with other frameworks in use like Spring Social). Let's say we we did have the custom UrlPathHelper somehow, then there is the question of how to make use of it short of comparing the result of calling it vs calling a stock UrlPathHelper. Worth thinking some more since re-using the custom UrlPathHelper somehow seems logical (similar to how a UriComponentsContributor is commonly also a HandlerMethodArgumentResolver).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 16, 2015

Christian Rudolph commented

I also think that ServletUriComponentsBuilder would be the right place for such a feature.
Using a special request attribute seems a bit inflexible (what if the stripped or changed part is in the middle of the path?) and not really expressive (from a developers point of view), because "some magic attribute" is set and then something special happens somewhere else. A custom interface for this piece of functionality seems much cleaner to me.
Just comparing the url of UrlPathHelper and an extended UrlPathHelper has the disadvantage, that one can't disable it for e. g. resource paths, where localization is not necessary.
Combining UrlPathHelper with the "reverse" functionality seems logical to me. But when doing so, a separate interface seems still appropriate, even if it might be combined with the extended UrlPathHelper.

My knowledge in spring isn't deep enough for qualified thoughts about a good way to inject the object into ServletUriComponentsBuilder.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 16, 2015

Rossen Stoyanchev commented

I find it hard to disagree on any point but a new interface requires more clarity and use cases than I feel we have. Your case is clear and very specific, i.e. strip and re-constitute a path prefix (very similar to "X-Forwarded-Prefix"). I hesitate to go from there to being able to customize any part of the path, at least not without a specific case.

I do agree a request attribute is too error prone. With "X-Forwarded-Prefix" the request is changed for everyone while here the change is limited to a specific UrlPathHelper instance and it should remain limited to wherever that instance applies, not anywhere where ServletUriComponentsBuilder may be used. So a request attribute could lead to unintended side effects.

I think a new method in UrlPathHelper to check for a lookup path prefix (by default "") seems fitting. Based on your example, something like:

@Override
public String getLookupPathForRequest(HttpServletRequest request) {
    return stripPath(super.getLookupPathForRequest(request));
}
/** 
 * Return any prefix that may be stripped with calls to getLookupPathForRequest.
 */
@Override
public String getLookupPathPrefix(HttpServletRequest request) {
    return extractPrefix(super.getLookupPathForRequest(request));
}

As for access to the UrlPathHelper from ServletUriComponentsBuilder, there is really no option for injecting it with its static methods. It can only be passed in. Fortunately we can do that transparently when MvcUriComponentsBuilder is used and we can also do the same for @RequestMapping methods that declare a ServletUriComponentsBuilder as an argument.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 16, 2015

Christian Rudolph commented

Sounds great for at least my use case which I believe is not so uncommon. The close relationship to the stripped prefix becomes clear.
With this solution it is not possible to create URLs to static resources that don't have any prefix. But it should not be a problem, because there shouldn't be a need for generating a static resource's path dynamically.
I understand what you write about the injection, although I don't know how UriComponentsBuilderMethodArgumentResolver (which I believe is responsible for resolving UriComponentsBuilder arguments of request mappings) could retrieve the correct UrlPathHelper.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 19, 2015

Rossen Stoyanchev commented

Actually as of 4.1 we do in fact generate resource paths dynamically (e.g. URLs with a content hash version). I don't expect we'd even try to automatically prepend a lookup path prefix in that case. Some resources might require it (e.g. locale-specific images) but most probably wouldn't.

As for your question about UriComponentsBuilderMethodArgumentResolver, you're right there is currently no access there to the correct UrlPathHelper. We could change that but now I think it makes sense to keep the change limited to MvcUriComponentsBuilder where it's clear what kinds of links we are building and where there is no other way to prepend the prefix. By comparison when a ServletUriComponentsBuilder is injected you can add any path to it, and it's also not so clear that you will be building a link to another @RequestMapping method.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 19, 2015

Rossen Stoyanchev commented

Taking the last comment a step further, perhaps a better alternative to the current proposal with the change in UrlPathHelper would be to look for a request attribute specific to MvcUriComponentsBuilder that exposes a UriComponentsBuilder to use for the base path, which by default is obtained with ServletUriComponentsBuilder.fromCurrentServletMapping(). It's not super elegant but it's dead simple, it will work for all methods of MvcUriComponentsBuilder, it can be applied selectively since the attribute can be added and removed where needed, and it makes fewer assumptions, e.g. as compared to assuming that a custom UrlPathHelper was used to remove a locale-specific prefix.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 20, 2015

Christian Rudolph commented

I thought about when path modification due to localization is necessary. When linking to locale specific resources, I think it is ok that the template is aware of the fact that there might be localized variations. Spring's designated mechanism to implement this is through themes.
As you mentionend above, path modification for localization is only necessary when building urls to controller methods. Hence it is plausible that the modification should only take place in MvcUriComponentsBuilder where the controller related path building logic is encapsulated.
I can't estimate the side effects of publishing a customized (request-) global (Mvc)UriComponentsBuilder due to missing knowledge, but technically it should work. When someone extends UrlPathHelper and overrides e. g. getLookupPathForRequest, it is easily possible to inject or replace a request attribute.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 20, 2015

Rossen Stoyanchev commented

Modified title (was: "Noninvasive support for locales in URLs").

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 23, 2015

Rossen Stoyanchev commented

Here is where I ended up after all.

First #17397 introduced overloaded fromXxx(UriComponentsBuilder,...) methods in MvcUriComponentsBuilder to provide the baseUrl as a method argument. The use case there is different, it's creating URLs outside the context of a request (e.g. batch process), but the change is related.

Then commit 1cd0f4 built on top of that by adding instance based methods to MvcUriComponentsBuilder so you can do something like this:

UriComponentsBuilder baseUrl = UriComponentsBuilder.fromUriString("http://example.org:9090/base");
MvcUriComponentsBuilder mvcBuilder = MvcUriComponentsBuilder.relativeTo(baseUrl);

UriComponentsBuilder builder1 = mvcBuilder.withMethodCall(...);
UriComponentsBuilder builder2 = mvcBuilder.withMappingName(...) ;
etc...

This should let you prepare and use a specific MvcUriComponents instance with a fixed base URL. You can even create a custom HandlerMethodArgumentResolver to inject an MvcUriComponentsBuilder instance into @RequestMapping methods.

I think this is a good balance between giving you what you need and not using something (like a request attribute) that could cause unwanted side effects.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2015

Christian Rudolph commented

This solution gives me the flexibility to do what I need to. Although it doesn't address the problem given in the description completely, because it is not possible to use the url tag library with a modified MvcUriComponentsBuilder. This isn't a problem for me, because I use thymeleaf and extended it to make it configurable. But it might be a good idea to provide this functionality out of the box for the users of spring tag library.
Many thanks for the fix!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2015

Rossen Stoyanchev commented

Thanks for the comment. The Spring url tag library declares a function for invoking MvcUriComponentsBuilder statically. It's a very thin wrapper that is meant to be a simple easy option. It's trivial to replace with your own function declaration or custom tag file.

I don't see many options other than looking for a UriComponentsBuilder (stored by the application) in a request attribute and while that would give out-of-the-box experience it wouldn't make it any more obvious how to solve the use case unless you find it in the docs. So I think there would be more value in documenting what is possible with the MvcUriComponentsBuilder in the reference, not just in the javadoc. That would enable anyone, using any view technology, to imagine how to make use of it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 7, 2015

Rossen Stoyanchev commented

Docs updated.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 9, 2015

Christian Rudolph commented

I just read the updated docs: I think this makes the point clear. It is really just a very little step to achieve the wanted effect. My solution for thymeleaf is just two little classes small.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 11, 2015

Rossen Stoyanchev commented

Can you provide a little more detail? What would one possible solution that you have in mind given what we already have?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 11, 2015

Christian Rudolph commented

I'm not sure whether I understand your second question correctly.
My last comment was just my acknowledgement, that from my point of view the documentation is good now and this issue is now fully resolved.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 11, 2015

Rossen Stoyanchev commented

Gotcha, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.