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 <mathieu.lir...@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 + "]"); }