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

Add support for 404, 204, and 3xx responses to the MVC config [SPR-11543] #16168

Closed
spring-issuemaster opened this issue Mar 12, 2014 · 13 comments
Closed
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Mar 12, 2014

cemo koc opened SPR-11543 and commented

#16166 encouraged me to open this issue :)

Currently one can easily register views to Controllers by WebMvcConfigurer as this:

/**
 * Add view controllers to create a direct mapping between a URL path and
 * view name without the need for a controller in between.
 */
void addViewControllers(ViewControllerRegistry registry);

What I would like to see is easy ways to provide mapping between common http status's such as 404, 301, 302 and 204. It would be great to have redirections, not founds and no contents controllers without the need for a controller by new registry types.

void addRedirectControllers(RedirectControllerRegistry registry);
void addNoContentControllers(NoContentControllerRegistry registry);
void addNotFoundControllers(NotFoundControllerRegistry registry);

This will greatly reduce unnecessary repeated codes.


Affects: 3.2.8, 4.0.2

Referenced from: commits 1ad22b9, 0bbb770, 0e2c5ee

0 votes, 5 watchers

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 13, 2014

Rossen Stoyanchev commented

Can you provide a little more detail? A ViewControllerRegistry is for mapping incoming URL paths to view names without the need for controller logic. What would a NotFoundControllerRegistry for example be for?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 13, 2014

cemo koc commented

We are using Spring as a complete stack. Since the beginning of Spring 3.x we have moved our code base almost completely to Spring. We migrated to our legacy architecture based on Tuckey Filters etc... to Spring as well.

Let me to summarize our use cases:

NotFound:

  • Some browser malware extensions are requesting some unknown urls 1 to our application. You can imagine how ugly and annoying looks these stupid urls at our logs.
  • Some browsers are sending illegal favicon requests despite of specifying clearly its location. Also users who bookmarked our site can request at wrong favicon url.

Implementing a new url is easy but repeating these steps is annoying and polluting code base.

Redirections:

  • This is very important in terms of SEO. SEO guides clearly states that never remove a indexed page. Instead of removing, redirect them to correct one.

Forbidden pages:

  • This is not actually very much used for us but I thought that It can be complementary to other common http statuses. We are heavily using redirections to not having a SEO penalty.
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 13, 2014

Rossen Stoyanchev commented

Even if we did provide what you're requesting, the requests would have to be looked at by the DispatcherSerlvet, an appropriate handler found and invoked, and only then the response is completed. So unwanted logging and processing is not really avoided.

It seems to me that UrlRewriteFilter is actually well suited for this sort of thing. You can deal with unwanted URLs as early as possible, never even reach the DispatcherSerlvet. Plus it offers a variety of ways to match URLs as well.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 13, 2014

cemo koc commented

I agree with you, these sort of things must be handled as early as possible. Tuckey is nice but not designed to work with programmatic configurations. We are using embedded environment, as spring-boot, and dealing with these things is not easy.

There are other reasons as well. Spring's WebMvcConfigurer is great to build modular web modules. We are developing separately our modules and each of them is responsible for their MvcConfig. This approach well suits with redirection as well. We need a way to decouple redirection and other configurations. For example a payment module can be responsible "/payment/xxx" redirections. This is not to be done by Tuckey.

Also WebMvcConfigurer's are Spring Beans. It means It can provide all of the easiness of Spring Framework. Redirections can be even dynamically configured by Database.

As I said I love Tuckey but I need more modular and native Spring solution.

By the way I have internally implemented a similar solution. Currently we are using it but I would be glad if it will be supported out of the box by Spring.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 13, 2014

Rossen Stoyanchev commented

Okay you've made some very good points. Certainly the embedded container scenario adds an interesting perspective to consider about the role formerly played by filters.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Mar 13, 2014

cemo koc commented

That is why I love Spring so much guys :) Thanks.

@spring-issuemaster
Copy link
Collaborator Author

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

Rossen Stoyanchev commented

How about a single registry to avoid adding too many methods on WebMvcConfigurer and since they're closely related:

void addStatusControllers(StatusControllerRegistry registry) {
  registry.redirectPermanent301("fromUrl", "toUrl");
  registry.redirectTemp302("fromUrl", "toUrl");
  registry.notFound404("url");
  registry.noContent204("url");
}

Are there are other options?

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 2, 2014

cemo koc commented

Totally fine for me.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 2, 2014

cemo koc commented

I came across two minor issue and would be better to share.

  1. For redirect cases query params also be redirected to new one.
  2. Mappings should support patters as well such as /xx/*
@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 5, 2014

Rossen Stoyanchev commented

  1. For redirect cases query params also be redirected to new one.

Seems a little random to automatically propagate all query params from the source to the target URL, or did you have something else in mind? I would prefer more explicit control, for example a lit of query parameter names to be propagated, or even a pattern of query parameter names.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 5, 2014

cemo koc commented

It would be definitely better to have explicit control but so far I have used for SEO purposes and It has been just redirection to new URL with same parameters. However I might be useful for someone else to control explicitly.

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 12, 2014

@spring-issuemaster
Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 17, 2014

Rossen Stoyanchev commented

cemo koc, the existing view controller config has been enhanced to support setting status codes. Initially the intent was simply clarify how view controllers relate to this new capability being added. After seeing the result I think it's pretty good for doing the kinds of things you've described. Take a look at the latest, start for example with the Javadoc for WebMvcConfigurer.addViewControllers. This is the actual commit 1ad22b.

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.