You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by mt...@apache.org on 2020/01/01 13:41:18 UTC

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

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 6c745f6ea56db746e3a2d3c96c9c6b2cca1496bf
Author: Mathieu Lirzin <ma...@nereide.fr>
AuthorDate: Sun Dec 29 16:46:47 2019 +0100

    Improved: Retrieve the included controller files eagerly
    (OFBIZ-11313)
    
    The included controller files resolution was previously delayed when
    accessing the configuration properties. The issue is that it imposes
    callers to handle exceptions when accessing those
    properties. Additionally this has the drawback of detecting inclusion
    issues late.
    
    We are now retrieving the included files when instantiating the
    controller configuration object. This provides more guarantee on the
    integrity of instantiated controllers and relax the error handling
    requirements on configuration properties callers.
---
 .../ofbiz/webapp/control/ConfigXMLReader.java      | 56 +++++++++++-----------
 .../ofbiz/webapp/control/RequestHandler.java       | 20 ++------
 2 files changed, 32 insertions(+), 44 deletions(-)

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 ff585a2..fddb48b 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
@@ -189,7 +189,7 @@ public class ConfigXMLReader {
         private String securityClass;
         private String defaultRequest;
         private String statusCode;
-        private List<URL> includes = new ArrayList<>();
+        private List<ControllerConfig> includes = new ArrayList<>();
         private final Map<String, Event> firstVisitEventList = new LinkedHashMap<>();
         private final Map<String, Event> preprocessorEventList = new LinkedHashMap<>();
         private final Map<String, Event> postprocessorEventList = new LinkedHashMap<>();
@@ -218,22 +218,22 @@ public class ConfigXMLReader {
             }
         }
 
-        private <K, V> Map<K, V> pushIncludes(Function<ControllerConfig, Map<K, V>> f) throws WebAppConfigurationException {
+        private <K, V> Map<K, V> pushIncludes(Function<ControllerConfig, Map<K, V>> f) {
             MapContext<K, V> res = new MapContext<>();
-            for (URL include : includes) {
-                res.push(getControllerConfig(include).pushIncludes(f));
+            for (ControllerConfig include : includes) {
+                res.push(include.pushIncludes(f));
             }
             res.push(f.apply(this));
             return res;
         }
 
-        private String getIncludes(Function<ControllerConfig, String> f) throws WebAppConfigurationException {
+        private String getIncludes(Function<ControllerConfig, String> f) {
             String val = f.apply(this);
             if (val != null) {
                 return val;
             }
-            for (URL include : includes) {
-                String inc = getControllerConfig(include).getIncludes(f);
+            for (ControllerConfig include : includes) {
+                String inc = include.getIncludes(f);
                 if (inc != null) {
                     return inc;
                 }
@@ -241,74 +241,73 @@ public class ConfigXMLReader {
             return null;
         }
 
-        public Map<String, Event> getAfterLoginEventList() throws WebAppConfigurationException {
+        public Map<String, Event> getAfterLoginEventList() {
             return pushIncludes(ccfg -> ccfg.afterLoginEventList);
         }
 
-        public Map<String, Event> getBeforeLogoutEventList() throws WebAppConfigurationException {
+        public Map<String, Event> getBeforeLogoutEventList() {
             return pushIncludes(ccfg -> ccfg.beforeLogoutEventList);
         }
 
-        public String getDefaultRequest() throws WebAppConfigurationException {
+        public String getDefaultRequest() {
             return getIncludes(ccfg -> ccfg.defaultRequest);
         }
 
-        public String getErrorpage() throws WebAppConfigurationException {
+        public String getErrorpage() {
             return getIncludes(ccfg -> ccfg.errorpage);
         }
 
-        public Map<String, String> getEventHandlerMap() throws WebAppConfigurationException {
+        public Map<String, String> getEventHandlerMap() {
             return pushIncludes(ccfg -> ccfg.eventHandlerMap);
         }
 
-        public Map<String, Event> getFirstVisitEventList() throws WebAppConfigurationException {
+        public Map<String, Event> getFirstVisitEventList() {
             return pushIncludes(ccfg -> ccfg.firstVisitEventList);
         }
 
-        public String getOwner() throws WebAppConfigurationException {
+        public String getOwner() {
             return getIncludes(ccfg -> ccfg.owner);
         }
 
-        public Map<String, Event> getPostprocessorEventList() throws WebAppConfigurationException {
+        public Map<String, Event> getPostprocessorEventList() {
             return pushIncludes(ccfg -> ccfg.postprocessorEventList);
         }
 
-        public Map<String, Event> getPreprocessorEventList() throws WebAppConfigurationException {
+        public Map<String, Event> getPreprocessorEventList() {
             return pushIncludes(ccfg -> ccfg.preprocessorEventList);
         }
 
-        public String getProtectView() throws WebAppConfigurationException {
+        public String getProtectView() {
             return getIncludes(ccfg -> ccfg.protectView);
         }
 
         // XXX: Keep it for backward compatibility until moving everything to 鈥榞etRequestMapMultiMap鈥�.
-        public Map<String, RequestMap> getRequestMapMap() throws WebAppConfigurationException {
+        public Map<String, RequestMap> getRequestMapMap() {
             return new MultivaluedMapContextAdapter<>(getRequestMapMultiMap());
         }
 
-        public MultivaluedMapContext<String, RequestMap> getRequestMapMultiMap() throws WebAppConfigurationException {
+        public MultivaluedMapContext<String, RequestMap> getRequestMapMultiMap() {
             MultivaluedMapContext<String, RequestMap> result = new MultivaluedMapContext<>();
-            for (URL includeLocation : includes) {
-                ControllerConfig controllerConfig = getControllerConfig(includeLocation);
-                result.push(controllerConfig.getRequestMapMultiMap());
+            for (ControllerConfig include : includes) {
+                result.push(include.getRequestMapMultiMap());
             }
             result.push(requestMapMap);
             return result;
         }
 
-        public String getSecurityClass() throws WebAppConfigurationException {
+        public String getSecurityClass() {
             return getIncludes(ccfg -> ccfg.securityClass);
         }
 
-        public String getStatusCode() throws WebAppConfigurationException {
+        public String getStatusCode() {
             return getIncludes(ccfg -> ccfg.statusCode);
         }
 
-        public Map<String, String> getViewHandlerMap() throws WebAppConfigurationException {
+        public Map<String, String> getViewHandlerMap() {
             return pushIncludes(ccfg -> ccfg.viewHandlerMap);
         }
 
-        public Map<String, ViewMap> getViewMapMap() throws WebAppConfigurationException {
+        public Map<String, ViewMap> getViewMapMap() {
             return pushIncludes(ccfg -> ccfg.viewMapMap);
         }
 
@@ -369,13 +368,14 @@ public class ConfigXMLReader {
             eventHandlerMap.putAll(handlers.get(false));
         }
 
-        protected void loadIncludes(Element rootElement) {
+        protected void loadIncludes(Element rootElement) throws WebAppConfigurationException {
             for (Element includeElement : UtilXml.childElementList(rootElement, "include")) {
                 String includeLocation = includeElement.getAttribute("location");
                 if (!includeLocation.isEmpty()) {
                     try {
                         URL urlLocation = FlexibleLocation.resolveLocation(includeLocation);
-                        includes.add(urlLocation);
+                        ControllerConfig includedController = getControllerConfig(urlLocation);
+                        includes.add(includedController);
                     } catch (MalformedURLException mue) {
                         Debug.logError(mue, "Error processing include at [" + includeLocation + "]:" + mue.toString(), module);
                     }
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 1a23451..f7d8d08 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
@@ -827,7 +827,7 @@ public class RequestHandler {
         try {
             String errorPageLocation = getControllerConfig().getErrorpage();
             errorPage = FlexibleLocation.resolveLocation(errorPageLocation);
-        } catch (WebAppConfigurationException | MalformedURLException e) {
+        } catch (MalformedURLException e) {
             Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
         }
         if (errorPage == null) {
@@ -838,14 +838,8 @@ public class RequestHandler {
 
     /** Returns the default status-code for this request. */
     public String getStatusCode(HttpServletRequest request) {
-        String statusCode = null;
-        try {
-            statusCode = getControllerConfig().getStatusCode();
-        } catch (WebAppConfigurationException e) {
-            Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
-        }
-        if (UtilValidate.isNotEmpty(statusCode)) return statusCode;
-        return null;
+        String statusCode = getControllerConfig().getStatusCode();
+        return UtilValidate.isNotEmpty(statusCode) ? statusCode : null;
     }
 
     /** Returns the ViewFactory Object. */
@@ -987,13 +981,7 @@ public class RequestHandler {
             req.getSession().removeAttribute("_SAVED_VIEW_PARAMS_");
         }
 
-        ConfigXMLReader.ViewMap viewMap = null;
-        try {
-            viewMap = (view == null ? null : getControllerConfig().getViewMapMap().get(view));
-        } catch (WebAppConfigurationException e) {
-            Debug.logError(e, "Exception thrown while parsing controller.xml file: ", module);
-            throw new RequestHandlerException(e);
-        }
+        ConfigXMLReader.ViewMap viewMap = (view == null) ? null : getControllerConfig().getViewMapMap().get(view);
         if (viewMap == null) {
             throw new RequestHandlerException("No definition found for view with name [" + view + "]");
         }