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 configuring custom redirect prefix in HandlerMethodReturnValueHandler's [SPR-12054] #16670

Closed
spring-issuemaster opened this issue Aug 1, 2014 · 7 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

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

Tobias Mattsson opened SPR-12054 and commented

In multiple projects I've implemented custom ViewResolvers that issue redirects based on view names. Similar to how redirect: and forward: works. They've interpreted these view names and built the redirects based on context and various factors such as locale of the logged in user. For instance "my-project:checkout-page" or "project:2156".

However flash attributes don't work for the redirects these ViewResolvers issue.

The reason is that the ModelAndViewContainer needs to have redirectModelScenario set in order for them to work. The only place I've found where I can set it is in a HandlerMethodReturnValueHandler. Unfortunately the ViewNameMethodReturnValueHandler eats all returned Strings so I have to extend it and replace the default one with my own. The same goes for ModelAndViewMethodReturnValueHandler. Replacing any of them means explicitly configuring all of the default ones since the custom ones that I could add to RequestMappingHandlerAdapter are executed after the default ones and so can't operate on String or ModelAndViewContainer.

I've come up with a solution but it's not very elegant so I'm wondering if you know of a better solution. Ideally ViewResolvers should be able to output redirects with flash attributes.

My solution extends RequestMappingHandlerAdapter and detects whether the returned view name is in a format that will be processed by a custom ViewResolver later. I'm not happy with how the logic for this gets duplicated.

Attached is maven projects with two DispatcherServlets that illustrate the problem, one where it works and one where it doesn't. It's runnable from command line with mvn jetty:run.

Here's my custom RequestMappingHandlerAdapter:

public class CustomRequestMappingHandlerAdapter extends RequestMappingHandlerAdapter {

    @Override
    public void afterPropertiesSet() {
        super.afterPropertiesSet();

        ArrayList<HandlerMethodReturnValueHandler> handlers = new ArrayList<HandlerMethodReturnValueHandler>(super.getReturnValueHandlers());
        for (int i = 0; i < handlers.size(); i++) {
            HandlerMethodReturnValueHandler handler = handlers.get(i);
            if (handler instanceof ViewNameMethodReturnValueHandler) {
                handlers.set(i, new ViewNameMethodReturnValueHandler() {

                    @Override
                    protected boolean isRedirectViewName(String viewName) {
                        return super.isRedirectViewName(viewName) || CustomRequestMappingHandlerAdapter.this.isRedirectViewName(viewName);
                    }
                });
            }
            if (handler instanceof ModelAndViewMethodReturnValueHandler) {
                handlers.set(i, new ModelAndViewMethodReturnValueHandler() {

                    @Override
                    public void handleReturnValue(Object returnValue, MethodParameter returnType, ModelAndViewContainer mavContainer, NativeWebRequest webRequest) throws Exception {
                        if (returnValue != null) {
                            ModelAndView mav = (ModelAndView) returnValue;
                            if (mav.isReference()) {
                                String viewName = mav.getViewName();
                                if (viewName != null && CustomRequestMappingHandlerAdapter.this.isRedirectViewName(viewName)) {
                                    mavContainer.setRedirectModelScenario(true);
                                }
                            }
                        }
                        super.handleReturnValue(returnValue, returnType, mavContainer, webRequest);
                    }
                });
            }
        }
        super.setReturnValueHandlers(handlers);
    }

    protected boolean isRedirectViewName(String viewName) {
        return viewName.startsWith(CustomRedirectViewResolver.CUSTOM_REDIRECT_PREFIX);
    }
}

Affects: 4.0.6

Attachments:

Referenced from: commits 8f715a8

@spring-issuemaster
Copy link
Collaborator Author

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

Rossen Stoyanchev commented

In the sample you have a redirect string that is based on a constant. From the comments I gather the determination may be dynamic and also incorporate request-specific information. Can you clarify which one is what you need? The answer is relevant for a potential solution.

@spring-issuemaster
Copy link
Collaborator Author

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

Tobias Mattsson commented

My current need is to support two constants, "magnolia-redirect:main-content" and "magnolia-redirect:current-content" and a prefix based convention "website:<uuid>" and "dms:<uuid>". These prefixes are configurable and maps to JCR workspaces. The UUID is used to find a node in the workspace and the returned redirect is a URI that maps to the node. Most of this is done in the View class though.

The source for this is online:
UuidRedirectViewResolver.java http://pastie.org/9437210
UuidRedirectView.java http://pastie.org/9437240

In other projects I've used similar approaches, always with a prefix and then a key which is handled differently based on the value in a ThreadLocal.

@spring-issuemaster
Copy link
Collaborator Author

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

Rossen Stoyanchev commented

Thanks for clarifying. I can't think of a better way to do it currently.

One possible improvement is to allow configuring a list of redirect prefixes on ViewNameMethodReturnValueHandler and ModelAndViewMethodReturnValueHandler respectively. That way you won't have to replace them but simply find and configure them. This is quite easily achievable for 4.1 still.

@spring-issuemaster
Copy link
Collaborator Author

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

Tobias Mattsson commented

Thanks for your feedback. I'll go ahead with my custom RequestMappingHandlerAdapter.

Expanding on your idea I think it would be better to have a list of patterns instead and match them using PatternMatchUtils.simpleMatch(). Identical to how UrlBasedViewResolver.canHandle() works. It would work both with prefixes and exact matches. I suppose it should be done in ViewNameMethodReturnValueHandler.isRedirectViewName() and ModelAndViewMethodReturnValueHandler should then also have a method isRedirectViewName() that does the same thing.

@spring-issuemaster
Copy link
Collaborator Author

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

Rossen Stoyanchev commented

Alright, I've added a "redirectPatterns" property to ViewNameMethodReturnValueHandler and ModelAndViewMethodReturnValueHandler. The isRedirectViewName protected method uses it to check for redirect view names in addition to the customary "redirect:" prefix.

@spring-issuemaster
Copy link
Collaborator Author

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

Rossen Stoyanchev commented

Modified title (was "Flash attributes not working with ViewResolvers doing redirects based on view names").

@spring-issuemaster
Copy link
Collaborator Author

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

Tobias Mattsson commented

Looks good, thank you!

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.