Skip to content
Permalink
Browse files
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.
  • Loading branch information
mthl committed Dec 30, 2019
1 parent 28b658e commit 6c745f6ea56db746e3a2d3c96c9c6b2cca1496bf
@@ -189,7 +189,7 @@ public static class ControllerConfig {
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,97 +218,96 @@ public ControllerConfig(URL url) throws WebAppConfigurationException {
}
}

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;
}
}
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 @@ private void loadHandlerMap(Element rootElement) {
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);
}
@@ -827,7 +827,7 @@ public String getDefaultErrorPage(HttpServletRequest request) {
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 String getDefaultErrorPage(HttpServletRequest request) {

/** 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 @@ private void renderView(String view, boolean allowExtView, HttpServletRequest re
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 + "]");
}

0 comments on commit 6c745f6

Please sign in to comment.