OFBiz › OFBiz - Commits

[ofbiz-framework] branch trunk updated (28b658e -> 7508917)

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

3 messages Options

mthl

[ofbiz-framework] branch trunk updated (28b658e -> 7508917)

This is an automated email from the ASF dual-hosted git repository.

mthl pushed a change to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git.

from 28b658e Improved: Equivalent of svn:auto-props properties on the server (OFBIZ-OFBIZ-11279)
new 6c745f6 Improved: Retrieve the included controller files eagerly (OFBIZ-11313)
new 7508917 Improved: Remove ‘RequestHandler#ControllerConfig’ wrapper (OFBIZ-11313)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

Summary of changes:
build.gradle | 2 +-
.../ofbiz/webapp/control/ConfigXMLReader.java | 67 ++++++----
.../ofbiz/webapp/control/RequestHandler.java | 148 +++++----------------
.../webapp/event/ServiceMultiEventHandler.java | 7 +-
.../ofbiz/webapp/control/RequestHandlerTests.java | 7 +-
.../java/org/apache/ofbiz/widget/WidgetWorker.java | 9 +-
6 files changed, 77 insertions(+), 163 deletions(-)

mthl

[ofbiz-framework] 01/02: Improved: Retrieve the included controller files eagerly (OFBIZ-11313)

mthl

[ofbiz-framework] 02/02: Improved: Remove ‘RequestHandler#ControllerConfig’ wrapper (OFBIZ-11313)

In reply to this post by mthl

This is an automated email from the ASF dual-hosted git repository.

mthl pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git

commit 7508917a0af0311694c37cf1ddd4231a0c8b3529
Author: Mathieu Lirzin <[hidden email]>
AuthorDate: Sun Dec 29 16:29:01 2019 +0100

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.
---
build.gradle | 2 +-
.../ofbiz/webapp/control/ConfigXMLReader.java | 11 +-
.../ofbiz/webapp/control/RequestHandler.java | 128 +++++----------------
.../webapp/event/ServiceMultiEventHandler.java | 7 +-
.../ofbiz/webapp/control/RequestHandlerTests.java | 7 +-
.../java/org/apache/ofbiz/widget/WidgetWorker.java | 9 +-
6 files changed, 45 insertions(+), 119 deletions(-)

diff --git a/build.gradle b/build.gradle
index 34bd3be..09ead78 100644
--- a/build.gradle
+++ b/build.gradle
@@ -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
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
index fddb48b..0802582 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
@@ -182,6 +182,9 @@ public class ConfigXMLReader {
}

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 class ConfigXMLReader {
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() {
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
index f7d8d08..965d0d1 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/RequestHandler.java
@@ -55,13 +55,13 @@ import org.apache.ofbiz.base.util.UtilMisc;
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,8 +80,6 @@ import org.apache.ofbiz.widget.model.ThemeFactory;
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;
@@ -89,66 +87,6 @@ public class RequestHandler {
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 class RequestHandler {
* @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 class RequestHandler {

// 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 class RequestHandler {
// 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 class RequestHandler {
// 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 class RequestHandler {
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 class RequestHandler {
// 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 class RequestHandler {
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 class RequestHandler {
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 class RequestHandler {
}
}

- 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 class RequestHandler {
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 @@ public class RequestHandler {
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);
}

/**
diff --git a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/ServiceMultiEventHandler.java b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/ServiceMultiEventHandler.java
index 55c8929..e768553 100644
--- a/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/ServiceMultiEventHandler.java
+++ b/framework/webapp/src/main/java/org/apache/ofbiz/webapp/event/ServiceMultiEventHandler.java
@@ -161,12 +161,7 @@ public class ServiceMultiEventHandler implements EventHandler {
} 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;
diff --git a/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java b/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java
index 0469d76..53d760d 100644
--- a/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java
+++ b/framework/webapp/src/test/java/org/apache/ofbiz/webapp/control/RequestHandlerTests.java
@@ -40,6 +40,7 @@ import java.util.Map;
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 class RequestHandlerTests {
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);
diff --git a/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java b/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
index 3a1646d..2f043bb 100644
--- a/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
+++ b/framework/widget/src/main/java/org/apache/ofbiz/widget/WidgetWorker.java
@@ -27,7 +27,6 @@ import javax.servlet.ServletContext;
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.entity.Delegator;
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 final class WidgetWorker {
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";
}