You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@ofbiz.apache.org by Michael Brohl <mi...@ecomify.de> on 2018/06/29 10:21:14 UTC

Re: svn commit: r1834662 - /ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java

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 jleroux@apache.org:
> 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) {
>
>