Skip to content

Commit

Permalink
Revert "Grails would sometimes attempt to render the wrong view, part…
Browse files Browse the repository at this point in the history 
…icularly"

This reverts commit 10eea0e.
  • Loading branch information
Jeff Brown committed Aug 17, 2010
1 parent 69767ab commit 440ed77
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 77 deletions.
2 changes: 1 addition & 1 deletion src/java/org/codehaus/groovy/grails/web/servlet/GrailsDispatcherServlet.java
View file
Expand Up @@ -301,7 +301,7 @@ public Locale getLocale() {

mv = super.processHandlerException(processedRequest, response, mappedHandler, e);
handlerException = e;
if (mv != null) render(mv, processedRequest, response);
render(mv, processedRequest, response);
}
else {
request.removeAttribute(GrailsApplicationAttributes.RENDERING_ERROR_ATTRIBUTE);
Expand Down
6 changes: 3 additions & 3 deletions src/java/org/codehaus/groovy/grails/web/servlet/mvc/SimpleGrailsControllerHelper.java
View file
Expand Up @@ -388,13 +388,13 @@ public Object handleAction(GroovyObject controller,Closure action, HttpServletRe
public ModelAndView handleActionResponse( GroovyObject controller,Object returnValue,String closurePropertyName, String viewName) {
boolean viewNameBlank = (viewName == null || viewName.length() == 0);
// reset the metaclass
ModelAndView explicitModelAndView = (ModelAndView)controller.getProperty(ControllerDynamicMethods.MODEL_AND_VIEW_PROPERTY);
ModelAndView explicityModelAndView = (ModelAndView)controller.getProperty(ControllerDynamicMethods.MODEL_AND_VIEW_PROPERTY);

if(!webRequest.isRenderView()) {
return null;
}
else if(explicitModelAndView != null) {
return explicitModelAndView;
else if(explicityModelAndView != null) {
return explicityModelAndView;
}
else if (returnValue == null) {
if (viewNameBlank) {
Expand Down
100 changes: 27 additions & 73 deletions src/java/org/codehaus/groovy/grails/web/util/WebUtils.java
View file
Expand Up @@ -297,12 +297,6 @@ public static String forwardRequestForUrlMappingInfo(HttpServletRequest request,
//populateParamsForMapping(info);
RequestDispatcher dispatcher = request.getRequestDispatcher(forwardUrl);

// Clear the request attributes that affect view rendering. Otherwise
// whatever we forward to may render the wrong thing! Note that we
// don't care about the return value because we're delegating
// responsibility for rendering the response.
saveAndResetWebRequest(request, info);

exposeForwardRequestAttributes(request);
exposeRequestAttributes(request, model);
dispatcher.forward(request, response);
Expand All @@ -324,82 +318,42 @@ public static IncludedContent includeForUrlMappingInfo(HttpServletRequest reques
String includeUrl = buildDispatchUrlForMapping(info, true);

final GrailsWebRequest webRequest = GrailsWebRequest.lookup(request);
InternalSavedRequest savedRequest = null;


String currentController = null;
String currentAction = null;
String currentId = null;
ModelAndView currentMv = null;
Map currentParams = null;
if (webRequest!=null) {
currentController = webRequest.getControllerName();
currentAction = webRequest.getActionName();
currentId = webRequest.getId();
currentParams = new HashMap();
currentParams.putAll(webRequest.getParameterMap());
currentMv = (ModelAndView)webRequest.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0);
}
try {
savedRequest = saveAndResetWebRequest(request, info);
if (webRequest!=null) {
webRequest.getParameterMap().clear();
info.configure(webRequest);
webRequest.getParameterMap().putAll(info.getParameters());
webRequest.removeAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0);
}
return includeForUrl(includeUrl, request, response, model);
}
finally {
if (savedRequest != null) {
if (webRequest!=null) {
webRequest.getParameterMap().clear();
webRequest.getParameterMap().putAll(savedRequest.getParameterMap());
webRequest.setId(savedRequest.getId());
webRequest.setControllerName(savedRequest.getController());
webRequest.setActionName(savedRequest.getAction());
if (savedRequest.getModelAndView() != null) {
webRequest.setAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, savedRequest.getModelAndView(), 0);
webRequest.getParameterMap().putAll(currentParams);
webRequest.setId(currentId);
webRequest.setControllerName(currentController);
webRequest.setActionName(currentAction);
if(currentMv != null) {
webRequest.setAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, currentMv, 0);
}
}
}
}

/**
* Saves the details of the current web request in an {@link InternalSavedRequest}
* instance, clears information related to view rendering from the request
* attributes, and returns the saved request.
* @param request The underlying HTTP request to process.
* @param info The URL mapping that should be applied to the request after
* the attributes have been cleared.
* @return The saved web request details.
*/
public static InternalSavedRequest saveAndResetWebRequest(HttpServletRequest request, UrlMappingInfo info) {
final GrailsWebRequest webRequest = GrailsWebRequest.lookup(request);
InternalSavedRequest savedRequest = null;
if (webRequest != null) {
savedRequest = new InternalSavedRequest(
webRequest.getControllerName(),
webRequest.getActionName(),
webRequest.getId(),
webRequest.getParameterMap(),
(ModelAndView) webRequest.getAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0));
webRequest.getParameterMap().clear();
info.configure(webRequest);
webRequest.getParameterMap().putAll(info.getParameters());
webRequest.removeAttribute(GrailsApplicationAttributes.MODEL_AND_VIEW, 0);
}

return savedRequest;
}

/**
* Simple class that stores a subset of information about a request, including
* the target controller and action, and the request parameters. Also stores
* information about any current ModelAndView that will be used to render the
* response.
*/
private static class InternalSavedRequest {
private String controller;
private String action;
private String id;
private HashMap parameters;
private ModelAndView modelAndView;

public InternalSavedRequest(String controllerName, String actionName, String id, Map parameterMap, ModelAndView mv) {
this.controller = controllerName;
this.action = actionName;
this.id = id;
this.parameters = new HashMap(parameterMap);
this.modelAndView = mv;
}

public String getController() { return controller; }
public String getAction() { return action; }
public String getId() { return id; }
public Map getParameterMap() { return parameters; }
public ModelAndView getModelAndView() { return modelAndView; }
}


/**
* Includes the given URL returning the resulting content as a String
Expand Down

2 comments on commit 440ed77

@pledbrook
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeff, the GrailsDispatcherServlet change should go back in. I couldn't work out how to test for it because it mostly manifests as an exception on the server without an obvious impact on the page rendering. But without it, logs can quickly fill up with the related exception.

The SimpleGrailsControllerHelper change is a simple spelling fix.

@grails
Copy link
Collaborator

@grails grails commented on 440ed77 Aug 18, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Peter. Agreed. As we discussed, I wanted to do a proper "git revert" for tracking purposes and then put back in any specific things that need to go back. Feel free to commit whatever change you would like.

Please sign in to comment.