I refactored many parts of the framework. I might have done more
refactoring than anyone recently.
However, I never just pushed a commit. I always ask for feedback, I make a
patch and I consistently ask for people's feedback, some of that feedback
came directly from you Jacques.
So for you to call it just refactoring as if this makes it a benign this is
surprising to me.
I would join Michael in recommending that you revert and start a proper
discussion.
On Jun 29, 2018 1:29 PM, "Jacques Le Roux" <[email protected]>
wrote:
Michael
It's not a change just a refactorization. I thought it was simple enough to
be committed. If we go this way for simple and small changes like here
things will be quite slow
I also answered in the Jira since you also asked the same there
Jacques
Le 29/06/2018 à 12:21, Michael Brohl a écrit :
Jacques,
didn't we just agreed upon a slower process and review from more
committers when changing these core aspects of the framework?
Especially when you change the patch there is no chance for anyone to
review before it gets committed {#emotions_dlg.sad}
Michael
Am 29.06.18 um 12:03 schrieb [email protected]:
Author: jleroux
Date: Fri Jun 29 10:03:22 2018
New Revision: 1834662
URL: http://svn.apache.org/viewvc?rev=1834662&view=rev
Log:
Improved: Factorize code logic from ‘ConfigXMLReader’
(OFBIZ-10453)
There is a lot of repetitive code in ConfigXMLReader.
Using a functional interface as a parameter of a generic algorithm
avoids those
repetitions.
Thanks: Mathieu Lirzin
Modified:
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
Modified:
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
URL:
http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java?rev=1834662&r1=1834661&r2=1834662&view=diff
==============================================================================
---
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
(original)
+++
ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
Fri Jun 29 10:03:22 2018
@@ -32,6 +32,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
+import java.util.function.Function;
import java.util.stream.Collectors;
import javax.servlet.ServletContext;
@@ -218,120 +219,67 @@ public class ConfigXMLReader {
}
}
- public Map<String, Event> getAfterLoginEventList() throws
WebAppConfigurationException {
- MapContext<String, Event> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getAfterLoginEventList());
+ private <K, V> Map<K, V>
pushIncludes(Function<ControllerConfig, Map<K, V>> f) throws
WebAppConfigurationException {
+ MapContext<K, V> res = MapContext.getMapContext();
+ for (URL include : includes) {
+ res.push(getControllerConfig(include).pushIncludes(f));
+ }
+ res.push(f.apply(this));
+ return res;
+ }
+
+ private String getIncludes(Function<ControllerConfig, String>
f) throws WebAppConfigurationException {
+ String val = f.apply(this);
+ if (val != null) {
+ return val;
+ }
+ for (URL include : includes) {
+ String inc =
getControllerConfig(include).getIncludes(f);
+ if (inc != null) {
+ return inc;
+ }
}
- result.push(afterLoginEventList);
- return result;
+ return null;
+ }
+
+ public Map<String, Event> getAfterLoginEventList() throws
WebAppConfigurationException {
+ return pushIncludes(ccfg -> ccfg.afterLoginEventList);
}
public Map<String, Event> getBeforeLogoutEventList() throws
WebAppConfigurationException {
- MapContext<String, Event> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getBeforeLogoutEventList());
- }
- result.push(beforeLogoutEventList);
- return result;
+ return pushIncludes(ccfg -> ccfg.beforeLogoutEventList);
}
public String getDefaultRequest() throws
WebAppConfigurationException {
- if (defaultRequest != null) {
- return defaultRequest;
- }
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- String defaultRequest =
controllerConfig.getDefaultRequest();
- if (defaultRequest != null) {
- return defaultRequest;
- }
- }
- return null;
+ return getIncludes(ccfg -> ccfg.defaultRequest);
}
public String getErrorpage() throws
WebAppConfigurationException {
- if (errorpage != null) {
- return errorpage;
- }
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- String errorpage = controllerConfig.getErrorpage();
- if (errorpage != null) {
- return errorpage;
- }
- }
- return null;
+ return getIncludes(ccfg -> ccfg.errorpage);
}
public Map<String, String> getEventHandlerMap() throws
WebAppConfigurationException {
- MapContext<String, String> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getEventHandlerMap());
- }
- result.push(eventHandlerMap);
- return result;
+ return pushIncludes(ccfg -> ccfg.eventHandlerMap);
}
public Map<String, Event> getFirstVisitEventList() throws
WebAppConfigurationException {
- MapContext<String, Event> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getFirstVisitEventList());
- }
- result.push(firstVisitEventList);
- return result;
+ return pushIncludes(ccfg -> ccfg.firstVisitEventList);
}
public String getOwner() throws WebAppConfigurationException
{
- if (owner != null) {
- return owner;
- }
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- String owner = controllerConfig.getOwner();
- if (owner != null) {
- return owner;
- }
- }
- return null;
+ return getIncludes(ccfg -> ccfg.owner);
}
public Map<String, Event> getPostprocessorEventList() throws
WebAppConfigurationException {
- MapContext<String, Event> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getPostprocessorEventList());
- }
- result.push(postprocessorEventList);
- return result;
+ return pushIncludes(ccfg -> ccfg.postprocessorEventList);
}
public Map<String, Event> getPreprocessorEventList() throws
WebAppConfigurationException {
- MapContext<String, Event> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getPreprocessorEventList());
- }
- result.push(preprocessorEventList);
- return result;
+ return pushIncludes(ccfg -> ccfg.preprocessorEventList);
}
public String getProtectView() throws
WebAppConfigurationException {
- if (protectView != null) {
- return protectView;
- }
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- String protectView = controllerConfig.getProtectView();
- if (protectView != null) {
- return protectView;
- }
- }
- return null;
+ return getIncludes(ccfg -> ccfg.protectView);
}
// XXX: Temporary wrapper to handle conversion from Map to
MultiMap.
@@ -391,51 +339,19 @@ public class ConfigXMLReader {
}
public String getSecurityClass() throws
WebAppConfigurationException {
- if (securityClass != null) {
- return securityClass;
- }
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- String securityClass =
controllerConfig.getSecurityClass();
- if (securityClass != null) {
- return securityClass;
- }
- }
- return null;
+ return getIncludes(ccfg -> ccfg.securityClass);
}
public String getStatusCode() throws
WebAppConfigurationException {
- if (statusCode != null) {
- return statusCode;
- }
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- String statusCode = controllerConfig.getStatusCode();
- if (statusCode != null) {
- return statusCode;
- }
- }
- return null;
+ return getIncludes(ccfg -> ccfg.statusCode);
}
public Map<String, String> getViewHandlerMap() throws
WebAppConfigurationException {
- MapContext<String, String> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getViewHandlerMap());
- }
- result.push(viewHandlerMap);
- return result;
+ return pushIncludes(ccfg -> ccfg.viewHandlerMap);
}
public Map<String, ViewMap> getViewMapMap() throws
WebAppConfigurationException {
- MapContext<String, ViewMap> result =
MapContext.getMapContext();
- for (URL includeLocation : includes) {
- ControllerConfig controllerConfig =
getControllerConfig(includeLocation);
- result.push(controllerConfig.getViewMapMap());
- }
- result.push(viewMapMap);
- return result;
+ return pushIncludes(ccfg -> ccfg.viewMapMap);
}
private void loadGeneralConfig(Element rootElement) {
