Skip to content
Permalink
Browse files
Improved: Remove ‘RequestHandler#ControllerConfig’ wrapper
(OFBIZ-11313)

‘RequestHandler#ControllerConfig’ was a wrapper around
‘ConfigXMLReader#ControllerConfig’ used as a convenience to gather
error handling when accessing configuration properties. Since the
later is now only throwing exceptions when instantiating the object,
‘RequestHandler#ControllerConfig’ is not useful anymore.
  • Loading branch information
mthl committed Dec 30, 2019
1 parent 6c745f6 commit 7508917a0af0311694c37cf1ddd4231a0c8b3529
@@ -285,7 +285,7 @@ checkstyle {
// the sum of errors that were present before introducing the
// ‘checkstyle’ tool present in the framework and in the official
// plugins.
tasks.checkstyleMain.maxErrors = 37725
tasks.checkstyleMain.maxErrors = 37713
// Currently there are a lot of errors so we need to temporarily
// hide them to avoid polluting the terminal output.
showViolations = false
@@ -182,6 +182,9 @@ private static Element loadDocument(URL location) throws WebAppConfigurationExce
}

public static class ControllerConfig {
private static final String DEFAULT_REDIRECT_STATUS_CODE =
UtilProperties.getPropertyValue("requestHandler", "status-code", "302");

public URL url;
private String errorpage;
private String protectView;
@@ -299,8 +302,14 @@ public String getSecurityClass() {
return getIncludes(ccfg -> ccfg.securityClass);
}

/**
* Provides the status code that should be used when redirecting an HTTP client.
*
* @return an HTTP response status code.
*/
public String getStatusCode() {
return getIncludes(ccfg -> ccfg.statusCode);
String status = getIncludes(ccfg -> ccfg.statusCode);
return UtilValidate.isEmpty(status) ? DEFAULT_REDIRECT_STATUS_CODE : status;
}

public Map<String, String> getViewHandlerMap() {
@@ -55,13 +55,13 @@
import org.apache.ofbiz.base.util.UtilObject;
import org.apache.ofbiz.base.util.UtilProperties;
import org.apache.ofbiz.base.util.UtilValidate;
import org.apache.ofbiz.base.util.collections.MultivaluedMapContext;
import org.apache.ofbiz.entity.Delegator;
import org.apache.ofbiz.entity.GenericEntityException;
import org.apache.ofbiz.entity.GenericValue;
import org.apache.ofbiz.entity.util.EntityQuery;
import org.apache.ofbiz.entity.util.EntityUtilProperties;
import org.apache.ofbiz.webapp.OfbizUrlBuilder;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap;
import org.apache.ofbiz.webapp.event.EventFactory;
import org.apache.ofbiz.webapp.event.EventHandler;
@@ -80,75 +80,13 @@
public class RequestHandler {

public static final String module = RequestHandler.class.getName();
private final static String defaultStatusCodeString =
UtilProperties.getPropertyValue("requestHandler", "status-code", "302");
private final ViewFactory viewFactory;
private final EventFactory eventFactory;
private final URL controllerConfigURL;
private final boolean trackServerHit;
private final boolean trackVisit;
private ControllerConfig ccfg;

static class ControllerConfig {
private final MultivaluedMapContext<String, RequestMap> requestMapMap;
private final Map<String, ConfigXMLReader.ViewMap> viewMapMap;
private String statusCodeString;
private final String defaultRequest;
private final Map<String, ConfigXMLReader.Event> firstVisitEventList;
private final Map<String, ConfigXMLReader.Event> preprocessorEventList;
private final Map<String, ConfigXMLReader.Event> postprocessorEventList;
private final String protectView;

ControllerConfig(ConfigXMLReader.ControllerConfig ccfg) throws WebAppConfigurationException {
preprocessorEventList = ccfg.getPreprocessorEventList();
postprocessorEventList = ccfg.getPostprocessorEventList();
requestMapMap = ccfg.getRequestMapMultiMap();
viewMapMap = ccfg.getViewMapMap();
defaultRequest = ccfg.getDefaultRequest();
firstVisitEventList = ccfg.getFirstVisitEventList();
protectView = ccfg.getProtectView();

String status = ccfg.getStatusCode();
statusCodeString = UtilValidate.isEmpty(status) ? defaultStatusCodeString : status;
}

public MultivaluedMapContext<String, RequestMap> getRequestMapMap() {
return requestMapMap;
}

public Map<String, ConfigXMLReader.ViewMap> getViewMapMap() {
return viewMapMap;
}

public String getStatusCodeString() {
return statusCodeString;
}

public String getDefaultRequest() {
return defaultRequest;
}

public void setStatusCodeString(String statusCodeString) {
this.statusCodeString = statusCodeString;
}

public Map<String, ConfigXMLReader.Event> getFirstVisitEventList() {
return firstVisitEventList;
}

public Map<String, ConfigXMLReader.Event> getPreprocessorEventList() {
return preprocessorEventList;
}

public Map<String, ConfigXMLReader.Event> getPostprocessorEventList() {
return postprocessorEventList;
}

public String getProtectView() {
return protectView;
}
}

public static RequestHandler getRequestHandler(ServletContext servletContext) {
RequestHandler rh = (RequestHandler) servletContext.getAttribute("_REQUEST_HANDLER_");
if (rh == null) {
@@ -193,7 +131,7 @@ public ConfigXMLReader.ControllerConfig getControllerConfig() {
* @return a collection of request maps which might be empty
*/
static Collection<RequestMap> resolveURI(ControllerConfig ccfg, HttpServletRequest req) {
Map<String, List<RequestMap>> requestMapMap = ccfg.getRequestMapMap();
Map<String, List<RequestMap>> requestMapMap = ccfg.getRequestMapMultiMap();
Collection<RequestMap> rmaps = resolveTemplateURI(requestMapMap, req);
if (rmaps.isEmpty()) {
Map<String, ConfigXMLReader.ViewMap> viewMapMap = ccfg.getViewMapMap();
@@ -277,7 +215,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,

// Parse controller config.
try {
ccfg = new ControllerConfig(getControllerConfig());
ccfg = ConfigXMLReader.getControllerConfig(controllerConfigURL);
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
throw new RequestHandlerException(e);
@@ -342,7 +280,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
// Check for chained request.
if (chain != null) {
String chainRequestUri = RequestHandler.getRequestUri(chain);
requestMap = ccfg.getRequestMapMap().getFirst(chainRequestUri);
requestMap = ccfg.getRequestMapMap().get(chainRequestUri);
if (requestMap == null) {
throw new RequestHandlerException("Unknown chained request [" + chainRequestUri + "]; this request does not exist");
}
@@ -366,11 +304,11 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
// Check to make sure we are allowed to access this request directly. (Also checks if this request is defined.)
// If the request cannot be called, or is not defined, check and see if there is a default-request we can process
if (!requestMap.securityDirectRequest) {
if (ccfg.getDefaultRequest() == null || !ccfg.getRequestMapMap().getFirst(ccfg.getDefaultRequest()).securityDirectRequest) {
if (ccfg.getDefaultRequest() == null || !ccfg.getRequestMapMap().get(ccfg.getDefaultRequest()).securityDirectRequest) {
// use the same message as if it was missing for security reasons, ie so can't tell if it is missing or direct request is not allowed
throw new RequestHandlerException(requestMissingErrorMessage);
} else {
requestMap = ccfg.getRequestMapMap().getFirst(ccfg.getDefaultRequest());
requestMap = ccfg.getRequestMapMap().get(ccfg.getDefaultRequest());
}
}
// Check if we SHOULD be secure and are not.
@@ -412,7 +350,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
String newUrl = RequestHandler.makeUrl(request, response, urlBuf.toString());
if (newUrl.toUpperCase().startsWith("HTTPS")) {
// if we are supposed to be secure, redirect secure.
callRedirect(newUrl, response, request, ccfg.getStatusCodeString());
callRedirect(newUrl, response, request, ccfg.getStatusCode());
return;
}
}
@@ -497,7 +435,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
// Invoke the security handler
// catch exceptions and throw RequestHandlerException if failed.
if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler]: AuthRequired. Running security check. " + showSessionId(request), module);
ConfigXMLReader.Event checkLoginEvent = ccfg.getRequestMapMap().getFirst("checkLogin").event;
ConfigXMLReader.Event checkLoginEvent = ccfg.getRequestMapMap().get("checkLogin").event;
String checkLoginReturnString = null;

try {
@@ -510,9 +448,9 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
eventReturn = checkLoginReturnString;
// if the request is an ajax request we don't want to return the default login check
if (!"XMLHttpRequest".equals(request.getHeader("X-Requested-With"))) {
requestMap = ccfg.getRequestMapMap().getFirst("checkLogin");
requestMap = ccfg.getRequestMapMap().get("checkLogin");
} else {
requestMap = ccfg.getRequestMapMap().getFirst("ajaxCheckLogin");
requestMap = ccfg.getRequestMapMap().get("ajaxCheckLogin");
}
}
}
@@ -644,7 +582,7 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
redirectTarget += "?" + queryString;
}

callRedirect(makeLink(request, response, redirectTarget), response, request, ccfg.getStatusCodeString());
callRedirect(makeLink(request, response, redirectTarget), response, request, ccfg.getStatusCode());
return;
}
}
@@ -702,31 +640,32 @@ public void doRequest(HttpServletRequest request, HttpServletResponse response,
}
}

String responseStatusCode = nextRequestResponse.statusCode;
if(UtilValidate.isNotEmpty(responseStatusCode))
ccfg.setStatusCodeString(responseStatusCode);

// The status code used to redirect the HTTP client.
String redirectSC = UtilValidate.isNotEmpty(nextRequestResponse.statusCode)
? nextRequestResponse.statusCode
: ccfg.getStatusCode();

if ("url".equals(nextRequestResponse.type)) {
if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler.doRequest]: Response is a URL redirect." + showSessionId(request), module);
callRedirect(nextRequestResponse.value, response, request, ccfg.getStatusCodeString());
callRedirect(nextRequestResponse.value, response, request, redirectSC);
} else if ("url-redirect".equals(nextRequestResponse.type)) {
// check for a cross-application redirect
if (Debug.verboseOn())
Debug.logVerbose("[RequestHandler.doRequest]: Response is a URL redirect with redirect parameters."
+ showSessionId(request), module);
callRedirect(nextRequestResponse.value + this.makeQueryString(request, nextRequestResponse), response,
request, ccfg.getStatusCodeString());
request, redirectSC);
} else if ("cross-redirect".equals(nextRequestResponse.type)) {
// check for a cross-application redirect
if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler.doRequest]: Response is a Cross-Application redirect." + showSessionId(request), module);
String url = nextRequestResponse.value.startsWith("/") ? nextRequestResponse.value : "/" + nextRequestResponse.value;
callRedirect(url + this.makeQueryString(request, nextRequestResponse), response, request, ccfg.getStatusCodeString());
callRedirect(url + this.makeQueryString(request, nextRequestResponse), response, request, redirectSC);
} else if ("request-redirect".equals(nextRequestResponse.type)) {
if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler.doRequest]: Response is a Request redirect." + showSessionId(request), module);
callRedirect(makeLinkWithQueryString(request, response, "/" + nextRequestResponse.value, nextRequestResponse), response, request, ccfg.getStatusCodeString());
callRedirect(makeLinkWithQueryString(request, response, "/" + nextRequestResponse.value, nextRequestResponse), response, request, redirectSC);
} else if ("request-redirect-noparam".equals(nextRequestResponse.type)) {
if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler.doRequest]: Response is a Request redirect with no parameters." + showSessionId(request), module);
callRedirect(makeLink(request, response, nextRequestResponse.value), response, request, ccfg.getStatusCodeString());
callRedirect(makeLink(request, response, nextRequestResponse.value), response, request, redirectSC);
} else if ("view".equals(nextRequestResponse.type)) {
if (Debug.verboseOn()) Debug.logVerbose("[RequestHandler.doRequest]: Response is a view." + showSessionId(request), module);

@@ -1140,13 +1079,7 @@ public String makeLink(HttpServletRequest request, HttpServletResponse response,
String requestUri = RequestHandler.getRequestUri(url);
ConfigXMLReader.RequestMap requestMap = null;
if (requestUri != null) {
try {
requestMap = getControllerConfig().getRequestMapMap().get(requestUri);
} catch (WebAppConfigurationException e) {
// If we can't read the controller.xml file, then there is no point in continuing.
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
return null;
}
requestMap = getControllerConfig().getRequestMapMap().get(requestUri);
}
boolean didFullSecure = false;
boolean didFullStandard = false;
@@ -1302,20 +1235,15 @@ private boolean track(HttpServletRequest request, boolean globalToggle, Predicat
if (uriString == null) {
uriString= "";
}
try {
Map<String, RequestMap> rmaps = getControllerConfig().getRequestMapMap();
RequestMap requestMap = rmaps.get(uriString);
Map<String, RequestMap> rmaps = getControllerConfig().getRequestMapMap();
RequestMap requestMap = rmaps.get(uriString);
if (requestMap == null) {
requestMap = rmaps.get(getControllerConfig().getDefaultRequest());
if (requestMap == null) {
requestMap = rmaps.get(getControllerConfig().getDefaultRequest());
if (requestMap == null) {
return false;
}
return false;
}
return pred.test(requestMap);
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
return false;
}
return pred.test(requestMap);
}

/**
@@ -161,12 +161,7 @@ public String invoke(Event event, RequestMap requestMap, HttpServletRequest requ
} catch (WebAppConfigurationException e) {
throw new EventHandlerException(e);
}
boolean eventGlobalTransaction;
try {
eventGlobalTransaction = controllerConfig.getRequestMapMap().get(requestUri).event.globalTransaction;
} catch (WebAppConfigurationException e) {
throw new EventHandlerException(e);
}
boolean eventGlobalTransaction = controllerConfig.getRequestMapMap().get(requestUri).event.globalTransaction;

// big try/finally to make sure commit or rollback are run
boolean beganTrans = false;
@@ -40,6 +40,7 @@
import javax.servlet.http.HttpServletRequest;

import org.apache.ofbiz.base.util.collections.MultivaluedMapContext;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.ControllerConfig;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.RequestMap;
import org.apache.ofbiz.webapp.control.ConfigXMLReader.ViewMap;
import org.junit.Before;
@@ -52,15 +53,15 @@ public static class ResolveURITests {
private Map<String, ViewMap> viewMaps;
private HttpServletRequest req;
private Element dummyElement;
private RequestHandler.ControllerConfig ccfg;
private ControllerConfig ccfg;

@Before
public void setUp() {
ccfg = mock(RequestHandler.ControllerConfig.class);
ccfg = mock(ControllerConfig.class);
reqMaps = new MultivaluedMapContext<>();
viewMaps = new HashMap<>();
when(ccfg.getDefaultRequest()).thenReturn(null);
when(ccfg.getRequestMapMap()).thenReturn(reqMaps);
when(ccfg.getRequestMapMultiMap()).thenReturn(reqMaps);
when(ccfg.getViewMapMap()).thenReturn(viewMaps);
req = mock(HttpServletRequest.class);
dummyElement = mock(Element.class);
@@ -27,7 +27,6 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.apache.ofbiz.base.util.Debug;
import org.apache.ofbiz.base.util.UtilCodec;
import org.apache.ofbiz.base.util.UtilGenerics;
import org.apache.ofbiz.base.util.UtilHttp;
@@ -36,7 +35,6 @@
import org.apache.ofbiz.service.LocalDispatcher;
import org.apache.ofbiz.webapp.control.ConfigXMLReader;
import org.apache.ofbiz.webapp.control.RequestHandler;
import org.apache.ofbiz.webapp.control.WebAppConfigurationException;
import org.apache.ofbiz.webapp.taglib.ContentUrlTag;
import org.apache.ofbiz.widget.model.ModelForm;
import org.apache.ofbiz.widget.model.ModelFormField;
@@ -300,12 +298,7 @@ public static String determineAutoLinkType(String linkType, String target, Strin
String requestUri = (target.indexOf('?') > -1) ? target.substring(0, target.indexOf('?')) : target;
ServletContext servletContext = request.getSession().getServletContext();
RequestHandler rh = (RequestHandler) servletContext.getAttribute("_REQUEST_HANDLER_");
ConfigXMLReader.RequestMap requestMap = null;
try {
requestMap = rh.getControllerConfig().getRequestMapMap().get(requestUri);
} catch (WebAppConfigurationException e) {
Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
}
ConfigXMLReader.RequestMap requestMap = rh.getControllerConfig().getRequestMapMap().get(requestUri);
if (requestMap != null && requestMap.event != null) {
return "hidden-form";
}

0 comments on commit 7508917

Please sign in to comment.