You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by ta...@apache.org on 2016/12/06 12:31:01 UTC

svn commit: r1772879 - in /ofbiz/trunk: ./ framework/base/src/main/java/org/apache/ofbiz/base/container/ framework/start/src/main/java/org/apache/ofbiz/base/start/

Author: taher
Date: Tue Dec  6 12:31:00 2016
New Revision: 1772879

URL: http://svn.apache.org/viewvc?rev=1772879&view=rev
Log:
Improved: Refactor and simplify the startup sequence in OFBiz
(OFBIZ-8337)

This is another major commit to the refactoring of the startup sequence in
OFBiz with the following highlighted changes:

- Delete the NativeLibClassLoader.java
- Delete the native classpath logic in Classpath.java
- Refactor the ComponentContainer and StartupControlPanel to operate without
  the NativeLibClassLoader. This substantially simplifies the code
- Declare a URLClassLoader in ComponentContainer that is instantiated upon
  building the classpath for all components. This makes the classloader start
  in one shot
- Simplify the Config file and remove fields that are not used. Also
  refactor some messy logic for loading the props file and other code
  improvements
- Refactor all the .properties files for startup to have a consistent
  structure that clearly documents all available properties and the
  default value for each property. This change is also related to the
  changes applied on Config.java
- Remove the declaration of the StartupLoader implementation class
  from all startup .properties files because only one implementation exists
  and it should be the default always.
- Refactor the loaders retrieval code (main, rmi, test, load-data) defined
  in the startup .properties files
- Refactor some switch statements to comply with java coding standards
- Add the DTDs defined in base through Gradle because we removed the
  NativeLibClassLoader and the classpath buildup logic in StartupControlPanel

Thanks: Jacopo Cappellato for reviewing the work

Removed:
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/NativeLibClassLoader.java
Modified:
    ofbiz/trunk/build.gradle
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
    ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Classpath.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties
    ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties

Modified: ofbiz/trunk/build.gradle
URL: http://svn.apache.org/viewvc/ofbiz/trunk/build.gradle?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/build.gradle (original)
+++ ofbiz/trunk/build.gradle Tue Dec  6 12:31:00 2016
@@ -204,19 +204,19 @@ sourceSets {
         resources {
             srcDirs = getDirectoryInActiveComponentsIfExists('src/main/java')
             srcDirs += getDirectoryInActiveComponentsIfExists('config')
+            srcDirs += "${rootDir}/framework/base/dtd"
             exclude excludedJavaSources
             exclude excludedConfigFiles
             // Below are necessary for unit tests run by Gradle and integration tests
+            exclude { FileTreeElement elem -> elem.getName().contains('.Labels.xml') }
             exclude { FileTreeElement elem -> elem.getName().contains('.properties') && 
                 !elem.getName().contains('start.properties') && 
                 !elem.getName().contains('load-data.properties') && 
                 !elem.getName().contains('debug.properties') &&
                 !elem.getName().contains('cache.properties') &&
                 !elem.getName().contains('test.properties') &&
-                !elem.getName().contains('rmi.properties')}
-            exclude { FileTreeElement elem -> elem.getName().contains('.xml') && 
-                !elem.getName().contains('entityengine.xml')
-                }
+                !elem.getName().contains('rmi.properties')
+            }
         }
     }
 

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java Tue Dec  6 12:31:00 2016
@@ -22,6 +22,8 @@ import java.io.File;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -30,7 +32,6 @@ import org.apache.ofbiz.base.component.C
 import org.apache.ofbiz.base.component.ComponentException;
 import org.apache.ofbiz.base.component.ComponentLoaderConfig;
 import org.apache.ofbiz.base.start.Classpath;
-import org.apache.ofbiz.base.start.NativeLibClassLoader;
 import org.apache.ofbiz.base.start.Start;
 import org.apache.ofbiz.base.start.StartupCommand;
 import org.apache.ofbiz.base.util.Debug;
@@ -43,7 +44,7 @@ import org.apache.ofbiz.base.util.FileUt
  * defined in OFBiz. This container must run before any other containers to
  * allow components to access any necessary resources. Furthermore, the
  * ComponentContainer also builds up the <code>ComponentConfigCache</code>
- * to keep track of all loaded components
+ * defined in <code>ComponentConfig</code> to keep track of loaded components
  *
  */
 public class ComponentContainer implements Container {
@@ -52,6 +53,7 @@ public class ComponentContainer implemen
 
     private String name;
     private final AtomicBoolean loaded = new AtomicBoolean(false);
+    private final List<Classpath> componentsClassPath = new ArrayList<Classpath>();
 
     @Override
     public void init(List<StartupCommand> ofbizCommands, String name, String configFile) throws ContainerException {
@@ -68,6 +70,7 @@ public class ComponentContainer implemen
         } catch (IOException | ComponentException e) {
             throw new ContainerException(e);
         }
+        loadClassPathForAllComponents(componentsClassPath);
         Debug.logInfo("All components loaded", module);
     }
 
@@ -79,6 +82,28 @@ public class ComponentContainer implemen
     }
 
     /**
+     * Iterate over all the components and load their classpath URLs into the classloader
+     * and set the classloader as the context classloader
+     *
+     * @param componentsClassPath: a list of classpaths for all components
+     * @throws ContainerException
+     */
+    private void loadClassPathForAllComponents(List<Classpath> componentsClassPath) throws ContainerException {
+        List<URL> allComponentUrls = new ArrayList<URL>();
+        for(Classpath classPath : componentsClassPath) {
+            try {
+                allComponentUrls.addAll(Arrays.asList(classPath.getUrls()));
+            } catch (MalformedURLException e) {
+                Debug.logError("Unable to load component classpath" + classPath.toString(), module);
+                Debug.logError(e.getMessage(), module);
+            }
+        }
+        URL[] componentURLs = allComponentUrls.toArray(new URL[allComponentUrls.size()]);
+        URLClassLoader classLoader = new URLClassLoader(componentURLs, Thread.currentThread().getContextClassLoader());
+        Thread.currentThread().setContextClassLoader(classLoader);
+    }
+
+    /**
      * Checks if <code>ComponentDef.type</code> is a directory or a single component.
      * If it is a directory, load the directory, otherwise load a single component
      *
@@ -197,28 +222,19 @@ public class ComponentContainer implemen
 
     /**
      * Load a single component by adding all its classpath entries to
-     * the classloader
+     * the list of classpaths to be loaded
      *
      * @param config: the component configuration
      * @throws IOException
      */
     private void loadComponent(ComponentConfig config) throws IOException {
-        if (!config.enabled()) {
+        if (config.enabled()) {
+            Classpath classpath = buildClasspathFromComponentConfig(config);
+            componentsClassPath.add(classpath);
+            Debug.logInfo("Added class path for component : [" + config.getComponentName() + "]", module);
+        } else {
             Debug.logInfo("Not loading component [" + config.getComponentName() + "] because it is disabled", module);
-            return;
-        }
-
-        NativeLibClassLoader classloader = (NativeLibClassLoader) Thread.currentThread().getContextClassLoader();
-        Classpath classPath = buildClasspathFromComponentConfig(config);
-
-        for (URL url : classPath.getUrls()) {
-            classloader.addURL(url);
-        }
-        for (File folder : classPath.getNativeFolders()) {
-            classloader.addNativeClassPath(folder);
         }
-
-        Debug.logInfo("Loaded component : [" + config.getComponentName() + "]", module);
     }
 
     /**

Modified: ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java (original)
+++ ofbiz/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ContainerLoader.java Tue Dec  6 12:31:00 2016
@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
-import java.util.Map;
 import java.util.stream.Collectors;
 
 import org.apache.ofbiz.base.component.ComponentConfig;
@@ -31,7 +30,6 @@ import org.apache.ofbiz.base.start.Start
 import org.apache.ofbiz.base.start.StartupException;
 import org.apache.ofbiz.base.start.StartupLoader;
 import org.apache.ofbiz.base.util.Debug;
-import org.apache.ofbiz.base.util.StringUtil;
 import org.apache.ofbiz.base.util.UtilValidate;
 
 import edu.emory.mathcs.backport.java.util.Collections;
@@ -57,12 +55,7 @@ public class ContainerLoader implements
     public synchronized void load(Config config, List<StartupCommand> ofbizCommands) throws StartupException {
 
         // loaders defined in startup (e.g. main, test, load-data, etc ...)
-        List<String> loaders = null;
-        for (Map<String,String> loaderMap: config.loaders) {
-            if (module.equals(loaderMap.get("class"))) {
-                loaders = StringUtil.split((String)loaderMap.get("profiles"), ",");
-            }
-        }
+        List<String> loaders = config.loaders;
 
         // load containers defined in ofbiz-containers.xml
         Debug.logInfo("[Startup] Loading containers...", module);

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Classpath.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Classpath.java?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Classpath.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Classpath.java Tue Dec  6 12:31:00 2016
@@ -35,9 +35,7 @@ import java.util.List;
  */
 public final class Classpath {
 
-    private static final String nativeLibExt = System.mapLibraryName("someLib").replace("someLib", "").toLowerCase();
     private List<File> elements = new ArrayList<File>();
-    private final List<File> nativeFolders = new ArrayList<File>();
 
     /**
      * Default constructor.
@@ -103,7 +101,6 @@ public final class Classpath {
         }
         if (path.isDirectory() && path.exists()) {
             // load all .jar, .zip files and native libs in this directory
-            boolean containsNativeLibs = false;
             for (File file : path.listFiles()) {
                 String fileName = file.getName().toLowerCase();
                 if (fileName.endsWith(".jar") || fileName.endsWith(".zip")) {
@@ -113,16 +110,6 @@ public final class Classpath {
                             elements.add(key);
                         }
                     }
-                } else if (fileName.endsWith(nativeLibExt)) {
-                    containsNativeLibs = true;
-                }
-            }
-            if (containsNativeLibs) {
-                File key = path.getCanonicalFile();
-                synchronized (nativeFolders) {
-                    if (!nativeFolders.contains(key)) {
-                        nativeFolders.add(key);
-                    }
                 }
             }
         } else {
@@ -140,17 +127,6 @@ public final class Classpath {
         }
     }
 
-    /**
-     * Returns a list of folders containing native libraries.
-     * 
-     * @return A list of folders containing native libraries
-     */
-    public List<File> getNativeFolders() {
-        synchronized (nativeFolders) {
-            return new ArrayList<File>(nativeFolders);
-        }
-    }
-
     /**
      * Returns a list of class path component URLs.
      * 

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Config.java Tue Dec  6 12:31:00 2016
@@ -20,14 +20,11 @@ package org.apache.ofbiz.base.start;
 
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -35,6 +32,8 @@ import java.util.Optional;
 import java.util.Properties;
 import java.util.TimeZone;
 
+import org.apache.ofbiz.base.util.StringUtil;
+
 /**
  * OFBiz server parameters needed on system startup and retrieved from
  * one of the properties files in the start component
@@ -42,33 +41,25 @@ import java.util.TimeZone;
 public final class Config {
 
     public final String ofbizHome;
-    public final String awtHeadless;
     public final InetAddress adminAddress;
     public final String adminKey;
     public final int portOffset;
     public final int adminPort;
     public final String containerConfig;
-    public final List<Map<String, String>> loaders;
+    public final List<String> loaders;
     public final String logDir;
     public final boolean shutdownAfterLoad;
     public final boolean useShutdownHook;
-    public final String classpathAddComponent;
 
     Config(List<StartupCommand> ofbizCommands) throws StartupException {
 
         // fetch OFBiz Properties object
-        Properties props;
-        try {
-            props = getPropertiesFile(ofbizCommands);
-        } catch (IOException e) {
-            throw new StartupException(e);
-        }
+        Properties props = getPropertiesFile(ofbizCommands);;
 
         // set this class fields
         ofbizHome = getOfbizHome(props);
-        awtHeadless = getProperty(props, "java.awt.headless", "false");
         adminAddress = getAdminAddress(props);
-        adminKey = getProperty(props, "ofbiz.admin.key", "NA");
+        adminKey = getProperty(props, "ofbiz.admin.key", "so3du5kasd5dn");
         portOffset = getPortOffsetValue(ofbizCommands);
         adminPort = getAdminPort(props, portOffset);
         containerConfig = getAbsolutePath(props, "ofbiz.container.config", "framework/base/config/ofbiz-containers.xml", ofbizHome);
@@ -76,13 +67,12 @@ public final class Config {
         logDir = getAbsolutePath(props, "ofbiz.log.dir", "runtime/logs", ofbizHome);
         shutdownAfterLoad = isShutdownAfterLoad(props);
         useShutdownHook = isUseShutdownHook(props);
-        classpathAddComponent = props.getProperty("ofbiz.start.classpath.addComponent");
 
         System.out.println("Set OFBIZ_HOME to - " + ofbizHome);
 
         // set system properties
         System.setProperty("ofbiz.home", ofbizHome);
-        System.setProperty("java.awt.headless", awtHeadless);
+        System.setProperty("java.awt.headless", getProperty(props, "java.awt.headless", "true"));
         System.setProperty("derby.system.home", getProperty(props, "derby.system.home", "runtime/data/derby"));
 
         // set the default locale
@@ -111,46 +101,30 @@ public final class Config {
         }
     }
 
-    private Properties getPropertiesFile(List<StartupCommand> ofbizCommands) throws IOException {
+    private Properties getPropertiesFile(List<StartupCommand> ofbizCommands) throws StartupException {
         String fileName = determineOfbizPropertiesFileName(ofbizCommands);
-        String fullyQualifiedFileName = "org/apache/ofbiz/base/start/" + fileName + ".properties";
-        InputStream propsStream = null;
+        String fullyQualifiedFileName = "org/apache/ofbiz/base/start/" + fileName;
         Properties props = new Properties();
-        try {
+
+        try (InputStream propsStream = getClass().getClassLoader().getResourceAsStream(fullyQualifiedFileName)) {
             // first try classpath
-            propsStream = getClass().getClassLoader().getResourceAsStream(fullyQualifiedFileName);
             if (propsStream != null) {
                 props.load(propsStream);
             } else {
-                throw new IOException();
-            }
-        } catch (IOException e) {
             // next try file location
-            File propsFile = new File(fullyQualifiedFileName);
-            if (propsFile != null) {
-                FileInputStream fis = null;
-                try {
-                    fis = new FileInputStream(propsFile);
+                try(FileInputStream fis = new FileInputStream(new File(fullyQualifiedFileName))) {
                     if (fis != null) {
                         props.load(fis);
                     }
-                } catch (FileNotFoundException e2) {
-                    // do nothing; we will see empty props below
-                } finally {
-                    if (fis != null) {
-                        fis.close();
-                    }
                 }
             }
-        } finally {
-            if (propsStream != null) {
-                propsStream.close();
-            }
+        } catch (IOException e) {
+            throw new StartupException(e);
         }
 
         // check for empty properties
         if (props.isEmpty()) {
-            throw new IOException("Cannot load configuration properties : " + fullyQualifiedFileName);
+            throw new StartupException("Cannot load configuration properties : " + fullyQualifiedFileName);
         }
         System.out.println("Start.java using configuration file " + fullyQualifiedFileName);
         return props;
@@ -159,37 +133,45 @@ public final class Config {
     private String determineOfbizPropertiesFileName(List<StartupCommand> ofbizCommands) {
         String fileName = null;
         if (ofbizCommands.stream().anyMatch(command ->
-        command.getName() == StartupCommandUtil.StartupOption.START.getName()
-        || command.getName() == StartupCommandUtil.StartupOption.SHUTDOWN.getName()
-        || command.getName() == StartupCommandUtil.StartupOption.STATUS.getName() )
+        command.getName().equals(StartupCommandUtil.StartupOption.START.getName())
+        || command.getName().equals(StartupCommandUtil.StartupOption.SHUTDOWN.getName())
+        || command.getName().equals(StartupCommandUtil.StartupOption.STATUS.getName()))
         || ofbizCommands.isEmpty()
-        || ofbizCommands.stream().allMatch(command ->
-            command.getName() == StartupCommandUtil.StartupOption.PORTOFFSET.getName()) 
-                ){
-            fileName = "start";
+        || ofbizCommands.stream().allMatch(command -> 
+                command.getName().equals(StartupCommandUtil.StartupOption.PORTOFFSET.getName()))) {
+            fileName = "start.properties";
         } else if(ofbizCommands.stream().anyMatch(
-                option -> option.getName() == StartupCommandUtil.StartupOption.LOAD_DATA.getName())) {
-            fileName = "load-data";
+                option -> option.getName().equals(StartupCommandUtil.StartupOption.LOAD_DATA.getName()))) {
+            fileName = "load-data.properties";
         } else if(ofbizCommands.stream().anyMatch(
-                option -> option.getName() == StartupCommandUtil.StartupOption.TEST.getName())) {
-            fileName = "test";
+                option -> option.getName().equals(StartupCommandUtil.StartupOption.TEST.getName()))) {
+            fileName = "test.properties";
         }
         return fileName;
     }
 
+    private List<String> getLoaders(Properties props) {
+        String loadersProp = getProperty(props, "ofbiz.start.loaders", "");
+        List<String> loaders = new ArrayList<String>();
+        for(String loader : StringUtil.split(loadersProp, ",")) {
+            loaders.add(loader);
+        }
+        return loaders;
+    }
+
     private void setDefaultLocale(Properties props) {
         String localeString = props.getProperty("ofbiz.locale.default");
         if (localeString != null && localeString.length() > 0) {
             String locales[] = localeString.split("_");
             switch (locales.length) {
-                case 1:
-                    Locale.setDefault(new Locale(locales[0]));
-                    break;
-                case 2:
-                    Locale.setDefault(new Locale(locales[0], locales[1]));
-                    break;
-                case 3:
-                    Locale.setDefault(new Locale(locales[0], locales[1], locales[2]));
+            case 1:
+                Locale.setDefault(new Locale(locales[0]));
+                break;
+            case 2:
+                Locale.setDefault(new Locale(locales[0], locales[1]));
+                break;
+            case 3:
+                Locale.setDefault(new Locale(locales[0], locales[1], locales[2]));
             }
             System.setProperty("user.language", localeString);
         }
@@ -234,27 +216,6 @@ public final class Config {
         return calculatedAdminPort;
     }
 
-    private List<Map<String, String>> getLoaders(Properties props) {
-        ArrayList<Map<String, String>> loadersTmp = new ArrayList<Map<String, String>>();
-        int currentPosition = 1;
-        Map<String, String> loader = null;
-        while (true) {
-            loader = new HashMap<String, String>();
-            String loaderClass = props.getProperty("ofbiz.start.loader" + currentPosition);
-
-            if (loaderClass == null || loaderClass.length() == 0) {
-                break;
-            } else {
-                loader.put("class", loaderClass);
-                loader.put("profiles", props.getProperty("ofbiz.start.loader" + currentPosition + ".loaders"));
-                loadersTmp.add(Collections.unmodifiableMap(loader));
-                currentPosition++;
-            }
-        }
-        loadersTmp.trimToSize();
-        return Collections.unmodifiableList(loadersTmp);
-    }
-
     private String getOfbizHome(Properties props) {
         String extractedOfbizHome = props.getProperty("ofbiz.home", ".");
         if (extractedOfbizHome.equals(".")) {

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/Start.java Tue Dec  6 12:31:00 2016
@@ -70,22 +70,22 @@ public final class Start {
             instance.config = StartupControlPanel.init(ofbizCommands);
         }
         switch (commandType) {
-            case HELP:
-                StartupCommandUtil.printOfbizStartupHelp(System.out);
-                break;
-            case STATUS:
-                System.out.println("Current Status : " + AdminClient.requestStatus(instance.config));
-                break;
-            case SHUTDOWN:
-                System.out.println("Shutting down server : " + AdminClient.requestShutdown(instance.config));
-                break;
-            case START:
-                try {
-                    StartupControlPanel.start(instance.config, instance.serverState, ofbizCommands);
-                } catch (StartupException e) {
-                    StartupControlPanel.fullyTerminateSystem(e);
-                }
-                break;
+        case HELP:
+            StartupCommandUtil.printOfbizStartupHelp(System.out);
+            break;
+        case STATUS:
+            System.out.println("Current Status : " + AdminClient.requestStatus(instance.config));
+            break;
+        case SHUTDOWN:
+            System.out.println("Shutting down server : " + AdminClient.requestShutdown(instance.config));
+            break;
+        case START:
+            try {
+                StartupControlPanel.start(instance.config, instance.serverState, ofbizCommands);
+            } catch (StartupException e) {
+                StartupControlPanel.fullyTerminateSystem(e);
+            }
+            break;
         }
     }
 

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/StartupControlPanel.java Tue Dec  6 12:31:00 2016
@@ -23,7 +23,6 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.ofbiz.base.start.Start.ServerState;
@@ -61,14 +60,13 @@ final class StartupControlPanel {
             AtomicReference<ServerState> serverState,
             List<StartupCommand> ofbizCommands) throws StartupException {
 
+        //TODO loaders should be converted to a single loader
         List<StartupLoader> loaders = new ArrayList<StartupLoader>();
         Thread adminServer = createAdminServer(config, serverState, loaders);
-        Classpath classPath = createClassPath(config);
-        NativeLibClassLoader classLoader = createAndSetContextClassLoader(config, classPath);
 
         createLogDirectoryIfMissing(config);
         createRuntimeShutdownHook(config, loaders, serverState);
-        loadStartupLoaders(config, loaders, ofbizCommands, serverState, classLoader);
+        loadStartupLoaders(config, loaders, ofbizCommands, serverState);
         executeShutdownAfterLoadIfConfigured(config, loaders, serverState, adminServer);
     }
 
@@ -159,42 +157,6 @@ final class StartupControlPanel {
         return adminServer;
     }
 
-    private static Classpath createClassPath(Config config) throws StartupException {
-        Classpath classPath = new Classpath();
-        try {
-            classPath.addComponent(config.ofbizHome);
-            String ofbizHomeTmp = config.ofbizHome;
-            if (!ofbizHomeTmp.isEmpty() && !ofbizHomeTmp.endsWith("/")) {
-                ofbizHomeTmp = ofbizHomeTmp.concat("/");
-            }
-            if (config.classpathAddComponent != null) {
-                String[] components = config.classpathAddComponent.split(",");
-                for (String component : components) {
-                    classPath.addComponent(ofbizHomeTmp.concat(component.trim()));
-                }
-            }
-        } catch (IOException e) {
-            throw new StartupException("Cannot create classpath", e);
-        }
-        return classPath;
-    }
-
-    private static NativeLibClassLoader createAndSetContextClassLoader(Config config, Classpath classPath) throws StartupException {
-        ClassLoader parent = Thread.currentThread().getContextClassLoader();
-        NativeLibClassLoader classloader = null;
-        try {
-            classloader = new NativeLibClassLoader(classPath.getUrls(), parent);
-            classloader.addNativeClassPath(System.getProperty("java.library.path"));
-            for (File folder : classPath.getNativeFolders()) {
-                classloader.addNativeClassPath(folder);
-            }
-        } catch (IOException e) {
-            throw new StartupException("Couldn't create NativeLibClassLoader", e);
-        }
-        Thread.currentThread().setContextClassLoader(classloader);
-        return classloader;
-    }
-
     private static void createLogDirectoryIfMissing(Config config) {
         File logDir = new File(config.logDir);
         if (!logDir.exists()) {
@@ -224,33 +186,24 @@ final class StartupControlPanel {
     private static void loadStartupLoaders(Config config, 
             List<StartupLoader> loaders,
             List<StartupCommand> ofbizCommands,
-            AtomicReference<ServerState> serverState,
-            NativeLibClassLoader classloader) throws StartupException {
+            AtomicReference<ServerState> serverState) throws StartupException {
+
+        String startupLoaderName = "org.apache.ofbiz.base.container.ContainerLoader";
+        ClassLoader classloader = Thread.currentThread().getContextClassLoader();
 
         synchronized (loaders) {
-            for (Map<String, String> loaderMap : config.loaders) {
-                if (serverState.get() == ServerState.STOPPING) {
-                    return;
-                }
-                try {
-                    String loaderClassName = loaderMap.get("class");
-                    Class<?> loaderClass = classloader.loadClass(loaderClassName);
-                    StartupLoader loader = (StartupLoader) loaderClass.newInstance();
-                    loaders.add(loader); // add before loading, so unload can occur if error during loading
-                    loader.load(config, ofbizCommands);
-                } catch (ReflectiveOperationException e) {
-                    throw new StartupException(e.getMessage(), e);
-                }
+            if (serverState.get() == ServerState.STOPPING) {
+                return;
             }
-        }
-        StringBuilder sb = new StringBuilder();
-        for (String path : classloader.getNativeLibPaths()) {
-            if (sb.length() > 0) {
-                sb.append(File.pathSeparator);
+            try {
+                Class<?> loaderClass = classloader.loadClass(startupLoaderName);
+                StartupLoader loader = (StartupLoader) loaderClass.newInstance();
+                loaders.add(loader); // add before loading, so unload can occur if error during loading
+                loader.load(config, ofbizCommands);
+            } catch (ReflectiveOperationException e) {
+                throw new StartupException(e);
             }
-            sb.append(path);
         }
-        System.setProperty("java.library.path", sb.toString());
     }
 
     private static void executeShutdownAfterLoadIfConfigured(

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/load-data.properties Tue Dec  6 12:31:00 2016
@@ -16,35 +16,47 @@
 # specific language governing permissions and limitations
 # under the License.
 ###############################################################################
-####
+
+####################################
 # OFBiz Startup Application Settings
-####
+####################################
 
-# --- Class paths needed to get StartupLoaders to work (see Start.createClassLoader()).
-ofbiz.start.classpath.addComponent=framework/base/config,framework/base/dtd
+# --- OFBiz startup loaders (comma separated)
+ofbiz.start.loaders=load-data
 
-# --- Default logs directory (relative to ofbiz.home)
-#ofbiz.log.dir=runtime/logs
+# --- OFBiz home directory. Default is current directory
+#ofbiz.home=
 
-# --- Location (relative to ofbiz.home) for (normal) container configuration
-#ofbiz.container.config=framework/base/config/ofbiz-containers.xml
+# --- logs directory relative to ofbiz.home. Default is runtime/logs
+#ofbiz.log.dir=
 
-# --- StartupLoader implementations to load (in order)
-ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
-ofbiz.start.loader1.loaders=load-data
+# --- Derby directory relative to ofbiz.home. Default is runtime/data/derby
+#derby.system.home=
 
-# -- Enable the shutdown hook
-#ofbiz.enable.hook=false
+# --- Container config file relative to ofbiz.home.
+#     Default is framework/base/config/ofbiz-containers.xml
+#ofbiz.container.config=
 
-# -- Auto-Shutdown after load
-ofbiz.auto.shutdown=true
+# --- Network host, port and key used by the AdminClient to communicate
+#     with AdminServer for shutting down OFBiz or inquiring on status
+#     Default ofbiz.admin.host 127.0.0.1
+#     Default ofbiz.admin.port 10523
+#     Default ofbiz.admin.key so3du5kasd5dn
+#ofbiz.admin.host=
+#ofbiz.admin.port=
+#ofbiz.admin.key=
 
-# --- Default Derby system home directory
-#derby.system.home=runtime/data/derby
+# -- Enable the JVM shutdown hook. Default is true
+ofbiz.enable.hook=false
+
+# -- Auto-Shutdown after load. Default is false
+ofbiz.auto.shutdown=true
 
-# --- Tells AWT (i.e. JasperReports/FOP/etc) to not require a head (X11)
-java.awt.headless=true
+# --- Tells AWT not not require a head (X11). Default is true
+#java.awt.headless=false
 
-# -- The default locale for this OFBiz instance.
+# -- The locale for this OFBiz instance. Default depends on JVM environment
 ofbiz.locale.default=en
 
+# -- The time zone for this OFBiz instance. Default depends on JVM environment
+#ofbiz.timeZone.default=GMT

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/rmi.properties Tue Dec  6 12:31:00 2016
@@ -16,31 +16,47 @@
 # specific language governing permissions and limitations
 # under the License.
 ###############################################################################
-####
+
+####################################
 # OFBiz Startup Application Settings
-####
+####################################
+
+# --- OFBiz startup loaders (comma separated)
+ofbiz.start.loaders=rmi
+
+# --- OFBiz home directory. Default is current directory
+#ofbiz.home=
+
+# --- logs directory relative to ofbiz.home. Default is runtime/logs
+#ofbiz.log.dir=
 
-# --- Class paths needed to get StartupLoaders to work (see Start.createClassLoader()).
-ofbiz.start.classpath.addComponent=framework/base/config,framework/base/dtd
+# --- Derby directory relative to ofbiz.home. Default is runtime/data/derby
+#derby.system.home=
 
-# --- Default logs directory (relative to ofbiz.home)
-#ofbiz.log.dir=runtime/logs
+# --- Container config file relative to ofbiz.home.
+#     Default is framework/base/config/ofbiz-containers.xml
+#ofbiz.container.config=
 
-# --- Location (relative to ofbiz.home) for (normal) container configuration
-#ofbiz.container.config=framework/base/config/ofbiz-containers.xml
+# --- Network host, port and key used by the AdminClient to communicate
+#     with AdminServer for shutting down OFBiz or inquiring on status
+#     Default ofbiz.admin.host 127.0.0.1
+#     Default ofbiz.admin.port 10523
+#     Default ofbiz.admin.key so3du5kasd5dn
+#ofbiz.admin.host=
+#ofbiz.admin.port=
+#ofbiz.admin.key=
 
-# --- StartupLoader implementations to load (in order)
-ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
-ofbiz.start.loader1.loaders=rmi
+# -- Enable the JVM shutdown hook. Default is true
+#ofbiz.enable.hook=false
 
-# -- Enable the shutdown hook
-#ofbiz.enable.hook=true
+# -- Auto-Shutdown after load. Default is false
+#ofbiz.auto.shutdown=true
 
-# -- Auto-Shutdown after load
-#ofbiz.auto.shutdown=false
+# --- Tells AWT not not require a head (X11). Default is true
+#java.awt.headless=false
 
-# --- Default Derby system home directory
-#derby.system.home=runtime/data/derby
+# -- The locale for this OFBiz instance. Default depends on JVM environment
+#ofbiz.locale.default=en
 
-# --- Tells AWT (i.e. JasperReports/FOP/etc) to not require a head (X11)
-java.awt.headless=true
+# -- The time zone for this OFBiz instance. Default depends on JVM environment
+#ofbiz.timeZone.default=GMT

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/start.properties Tue Dec  6 12:31:00 2016
@@ -16,48 +16,47 @@
 # specific language governing permissions and limitations
 # under the License.
 ###############################################################################
-####
+
+####################################
 # OFBiz Startup Application Settings
-####
+####################################
+
+# --- OFBiz startup loaders (comma separated)
+ofbiz.start.loaders=main
 
-# --- By default we will use the current directory
+# --- OFBiz home directory. Default is current directory
 #ofbiz.home=
 
-# --- Class paths needed to get StartupLoaders to work (see Start.createClassLoader()).
-ofbiz.start.classpath.addComponent=framework/base/config,framework/base/dtd
+# --- logs directory relative to ofbiz.home. Default is runtime/logs
+#ofbiz.log.dir=
+
+# --- Derby directory relative to ofbiz.home. Default is runtime/data/derby
+#derby.system.home=
 
-# --- Set these for shutting down when running as background process
-ofbiz.admin.host=127.0.0.1
-ofbiz.admin.port=10523
-ofbiz.admin.key=so3du5kasd5dn
-
-# --- Default logs directory (relative to ofbiz.home)
-#ofbiz.log.dir=runtime/logs
-
-# --- Location (relative to ofbiz.home) for (normal) container configuration
-#ofbiz.container.config=framework/base/config/ofbiz-containers.xml
-
-# --- StartupLoader implementations to load (in order)
-ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
-# Because of the danger of Java deserialization when using RMI, the RMI component has been disabled in the default configuration of OFBiz.
-# If you need RMI you just need to uncomment those places - See OFBIZ-6942 for details -->
-#ofbiz.start.loader1.loaders=main,rmi
-ofbiz.start.loader1.loaders=main
- 
-# -- Enable the shutdown hook
-#ofbiz.enable.hook=true
-
-# -- Auto-Shutdown after load
-#ofbiz.auto.shutdown=false
+# --- Container config file relative to ofbiz.home.
+#     Default is framework/base/config/ofbiz-containers.xml
+#ofbiz.container.config=
+
+# --- Network host, port and key used by the AdminClient to communicate
+#     with AdminServer for shutting down OFBiz or inquiring on status
+#     Default ofbiz.admin.host 127.0.0.1
+#     Default ofbiz.admin.port 10523
+#     Default ofbiz.admin.key so3du5kasd5dn
+#ofbiz.admin.host=
+#ofbiz.admin.port=
+#ofbiz.admin.key=
+
+# -- Enable the JVM shutdown hook. Default is true
+#ofbiz.enable.hook=false
 
-# --- Default Derby system home directory
-#derby.system.home=runtime/data/derby
+# -- Auto-Shutdown after load. Default is false
+#ofbiz.auto.shutdown=true
 
-# --- Tells AWT (i.e. JasperReports/FOP/etc) to not require a head (X11)
-java.awt.headless=true
+# --- Tells AWT not not require a head (X11). Default is true
+#java.awt.headless=false
 
-# -- The default locale for this OFBiz instance.
+# -- The locale for this OFBiz instance. Default depends on JVM environment
 ofbiz.locale.default=en
 
-# -- The default time zone for this OFBiz instance.
+# -- The time zone for this OFBiz instance. Default depends on JVM environment
 #ofbiz.timeZone.default=GMT

Modified: ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties
URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties?rev=1772879&r1=1772878&r2=1772879&view=diff
==============================================================================
--- ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties (original)
+++ ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/test.properties Tue Dec  6 12:31:00 2016
@@ -16,34 +16,47 @@
 # specific language governing permissions and limitations
 # under the License.
 ###############################################################################
-####
+
+####################################
 # OFBiz Startup Application Settings
-####
+####################################
+
+# --- OFBiz startup loaders (comma separated)
+ofbiz.start.loaders=test
+
+# --- OFBiz home directory. Default is current directory
+#ofbiz.home=
 
-# --- Class paths needed to get StartupLoaders to work (see Start.createClassLoader()).
-ofbiz.start.classpath.addComponent=framework/base/config,framework/base/dtd
+# --- logs directory relative to ofbiz.home. Default is runtime/logs
+#ofbiz.log.dir=
 
-# --- Default logs directory (relative to ofbiz.home)
-#ofbiz.log.dir=runtime/logs
+# --- Derby directory relative to ofbiz.home. Default is runtime/data/derby
+#derby.system.home=
 
-# --- Location (relative to ofbiz.home) for (normal) container configuration
-#ofbiz.container.config=framework/base/config/ofbiz-containers.xml
+# --- Container config file relative to ofbiz.home.
+#     Default is framework/base/config/ofbiz-containers.xml
+#ofbiz.container.config=
 
-# --- StartupLoader implementations to load (in order)
-ofbiz.start.loader1=org.apache.ofbiz.base.container.ContainerLoader
-ofbiz.start.loader1.loaders=test
+# --- Network host, port and key used by the AdminClient to communicate
+#     with AdminServer for shutting down OFBiz or inquiring on status
+#     Default ofbiz.admin.host 127.0.0.1
+#     Default ofbiz.admin.port 10523
+#     Default ofbiz.admin.key so3du5kasd5dn
+#ofbiz.admin.host=
+#ofbiz.admin.port=
+#ofbiz.admin.key=
 
-# -- Enable the shutdown hook
-#ofbiz.enable.hook=true
+# -- Enable the JVM shutdown hook. Default is true
+ofbiz.enable.hook=false
 
-# -- Auto-Shutdown after load
+# -- Auto-Shutdown after load. Default is false
 ofbiz.auto.shutdown=true
 
-# --- Default Derby system home directory
-#derby.system.home=runtime/data/derby
+# --- Tells AWT not not require a head (X11). Default is true
+#java.awt.headless=false
 
-# -- The default locale for this OFBiz instance.
+# -- The locale for this OFBiz instance. Default depends on JVM environment
 ofbiz.locale.default=en
 
-# --- Tells AWT (i.e. JasperReports/FOP/etc) to not require a head (X11)
-java.awt.headless=true
+# -- The time zone for this OFBiz instance. Default depends on JVM environment
+#ofbiz.timeZone.default=GMT




Re: svn commit: r1772879 - in /ofbiz/trunk: ./ framework/base/src/main/java/org/apache/ofbiz/base/container/ framework/start/src/main/java/org/apache/ofbiz/base/start/

Posted by Pierre Smits <pi...@gmail.com>.
I suggest to start new issues in our JIRA.

Best regards

Pierre Smits

ORRTIZ.COM <http://www.orrtiz.com>
OFBiz based solutions & services

OFBiz Extensions Marketplace
http://oem.ofbizci.net/oci-2/

On Thu, Dec 8, 2016 at 11:54 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:

> Thanks for the clarification and the fix Taher
>
> I was unsure all the labels files followed the "*Labels.xml" pattern.
> Obviously it does, good!
>
> There are few files which should maybe not be in the ofbiz.jar file, like
>
> ImageProperties.xml
> SeoConfig.xml
>
> But I don't think it's a concern, and if we eventually find it does it
> will be easy to fix that
>
> Jacques
>
>
>
> Le 08/12/2016 à 10:14, Taher Alkhateeb a écrit :
>
>> Hi Jacques, actually I entered a type, that "." before Labels.xml should
>> not be there. So I already excluded all the label files, I'll fix this
>> shortly.
>>
>> As to why include other XML files, it's because the classpath logic
>> changed
>> and we need to define things in Gradle (no longer loaded from OFBiz, which
>> is the correct thing to do. So your old code would not work (you can give
>> it a try). I think we're going overboard with trying to exclude as many
>> things as we can from the jar file given that it is a dynamic thing that
>> changes constantly.
>>
>> On Thu, Dec 8, 2016 at 11:57 AM, Jacques Le Roux <
>> jacques.le.roux@les7arts.com> wrote:
>>
>> Le 06/12/2016 à 13:31, taher@apache.org a écrit :
>>>
>>> Author: taher
>>>> Date: Tue Dec  6 12:31:00 2016
>>>> New Revision: 1772879
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1772879&view=rev
>>>> Log:
>>>> Improved: Refactor and simplify the startup sequence in OFBiz
>>>> (OFBIZ-8337)
>>>>
>>>> This is another major commit to the refactoring of the startup sequence
>>>> in
>>>> OFBiz with the following highlighted changes:
>>>>
>>>> [...]
>>>> - Add the DTDs defined in base through Gradle because we removed the
>>>>     NativeLibClassLoader and the classpath buildup logic in
>>>> StartupControlPanel
>>>>
>>>> Thanks: Jacopo Cappellato for reviewing the work
>>>>
>>>> Removed:
>>>>       ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/
>>>> base/start/NativeLibClassLoader.java
>>>> Modified:
>>>>       ofbiz/trunk/build.gradle
>>>>
>>>> [...]
>>>
>>> Modified: ofbiz/trunk/build.gradle
>>>
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/build.gradle?rev=17
>>>> 72879&r1=1772878&r2=1772879&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- ofbiz/trunk/build.gradle (original)
>>>> +++ ofbiz/trunk/build.gradle Tue Dec  6 12:31:00 2016
>>>> @@ -204,19 +204,19 @@ sourceSets {
>>>>            resources {
>>>>                srcDirs = getDirectoryInActiveComponents
>>>> IfExists('src/main/java')
>>>>                srcDirs += getDirectoryInActiveComponents
>>>> IfExists('config')
>>>> +            srcDirs += "${rootDir}/framework/base/dtd"
>>>>                exclude excludedJavaSources
>>>>                exclude excludedConfigFiles
>>>>                // Below are necessary for unit tests run by Gradle and
>>>> integration tests
>>>> +            exclude { FileTreeElement elem ->
>>>> elem.getName().contains('.Labels.xml') }
>>>>                exclude { FileTreeElement elem ->
>>>> elem.getName().contains('.properties') &&
>>>>                    !elem.getName().contains('start.properties') &&
>>>>                    !elem.getName().contains('load-data.properties') &&
>>>>                    !elem.getName().contains('debug.properties') &&
>>>>                    !elem.getName().contains('cache.properties') &&
>>>>                    !elem.getName().contains('test.properties') &&
>>>> -                !elem.getName().contains('rmi.properties')}
>>>> -            exclude { FileTreeElement elem ->
>>>> elem.getName().contains('.xml') &&
>>>> -                !elem.getName().contains('entityengine.xml')
>>>> -                }
>>>> +                !elem.getName().contains('rmi.properties')
>>>> +            }
>>>>            }
>>>>        }
>>>>
>>>>
>>>> Hi Taher,
>>>
>>> I'm just begining to review and stumbled upon this. What is the plan of
>>> having all XML files embedded in the ofbiz.jar file?
>>>
>>> Notably the labels files are bad because being static in the ofbiz.jar
>>> file they no longer can be modified.
>>> It seems you took this into account after https://issues.apache.org/jira
>>> /browse/OFBIZ-8321 but wrongly wrote the related line (you would need a
>>> regexp)
>>>
>>> +            exclude { FileTreeElement elem ->
>>> elem.getName().contains('.Labels.xml') }
>>>
>>> Now I see no reasons to put other XML files in the ofbiz.jar file but
>>> entityengine.xml which should not be changed when OFBiz runs.
>>> So the only resources which should be in ofbiz.jars are
>>>      cache.properties
>>>      debug.properties
>>>      entityengine.xml
>>> Else please explain why.
>>>
>>> Thanks
>>>
>>> Jacques
>>>
>>>
>>>
>

Re: svn commit: r1772879 - in /ofbiz/trunk: ./ framework/base/src/main/java/org/apache/ofbiz/base/container/ framework/start/src/main/java/org/apache/ofbiz/base/start/

Posted by Jacques Le Roux <ja...@les7arts.com>.
Thanks for the clarification and the fix Taher

I was unsure all the labels files followed the "*Labels.xml" pattern. Obviously it does, good!

There are few files which should maybe not be in the ofbiz.jar file, like

ImageProperties.xml
SeoConfig.xml

But I don't think it's a concern, and if we eventually find it does it will be easy to fix that

Jacques


Le 08/12/2016 � 10:14, Taher Alkhateeb a �crit :
> Hi Jacques, actually I entered a type, that "." before Labels.xml should
> not be there. So I already excluded all the label files, I'll fix this
> shortly.
>
> As to why include other XML files, it's because the classpath logic changed
> and we need to define things in Gradle (no longer loaded from OFBiz, which
> is the correct thing to do. So your old code would not work (you can give
> it a try). I think we're going overboard with trying to exclude as many
> things as we can from the jar file given that it is a dynamic thing that
> changes constantly.
>
> On Thu, Dec 8, 2016 at 11:57 AM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> Le 06/12/2016 � 13:31, taher@apache.org a �crit :
>>
>>> Author: taher
>>> Date: Tue Dec  6 12:31:00 2016
>>> New Revision: 1772879
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1772879&view=rev
>>> Log:
>>> Improved: Refactor and simplify the startup sequence in OFBiz
>>> (OFBIZ-8337)
>>>
>>> This is another major commit to the refactoring of the startup sequence in
>>> OFBiz with the following highlighted changes:
>>>
>>> [...]
>>> - Add the DTDs defined in base through Gradle because we removed the
>>>     NativeLibClassLoader and the classpath buildup logic in
>>> StartupControlPanel
>>>
>>> Thanks: Jacopo Cappellato for reviewing the work
>>>
>>> Removed:
>>>       ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/
>>> base/start/NativeLibClassLoader.java
>>> Modified:
>>>       ofbiz/trunk/build.gradle
>>>
>> [...]
>>
>> Modified: ofbiz/trunk/build.gradle
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/build.gradle?rev=17
>>> 72879&r1=1772878&r2=1772879&view=diff
>>> ============================================================
>>> ==================
>>> --- ofbiz/trunk/build.gradle (original)
>>> +++ ofbiz/trunk/build.gradle Tue Dec  6 12:31:00 2016
>>> @@ -204,19 +204,19 @@ sourceSets {
>>>            resources {
>>>                srcDirs = getDirectoryInActiveComponents
>>> IfExists('src/main/java')
>>>                srcDirs += getDirectoryInActiveComponentsIfExists('config')
>>> +            srcDirs += "${rootDir}/framework/base/dtd"
>>>                exclude excludedJavaSources
>>>                exclude excludedConfigFiles
>>>                // Below are necessary for unit tests run by Gradle and
>>> integration tests
>>> +            exclude { FileTreeElement elem ->
>>> elem.getName().contains('.Labels.xml') }
>>>                exclude { FileTreeElement elem ->
>>> elem.getName().contains('.properties') &&
>>>                    !elem.getName().contains('start.properties') &&
>>>                    !elem.getName().contains('load-data.properties') &&
>>>                    !elem.getName().contains('debug.properties') &&
>>>                    !elem.getName().contains('cache.properties') &&
>>>                    !elem.getName().contains('test.properties') &&
>>> -                !elem.getName().contains('rmi.properties')}
>>> -            exclude { FileTreeElement elem ->
>>> elem.getName().contains('.xml') &&
>>> -                !elem.getName().contains('entityengine.xml')
>>> -                }
>>> +                !elem.getName().contains('rmi.properties')
>>> +            }
>>>            }
>>>        }
>>>
>>>
>> Hi Taher,
>>
>> I'm just begining to review and stumbled upon this. What is the plan of
>> having all XML files embedded in the ofbiz.jar file?
>>
>> Notably the labels files are bad because being static in the ofbiz.jar
>> file they no longer can be modified.
>> It seems you took this into account after https://issues.apache.org/jira
>> /browse/OFBIZ-8321 but wrongly wrote the related line (you would need a
>> regexp)
>>
>> +            exclude { FileTreeElement elem ->
>> elem.getName().contains('.Labels.xml') }
>>
>> Now I see no reasons to put other XML files in the ofbiz.jar file but
>> entityengine.xml which should not be changed when OFBiz runs.
>> So the only resources which should be in ofbiz.jars are
>>      cache.properties
>>      debug.properties
>>      entityengine.xml
>> Else please explain why.
>>
>> Thanks
>>
>> Jacques
>>
>>


Re: svn commit: r1772879 - in /ofbiz/trunk: ./ framework/base/src/main/java/org/apache/ofbiz/base/container/ framework/start/src/main/java/org/apache/ofbiz/base/start/

Posted by Taher Alkhateeb <sl...@gmail.com>.
Hi Jacques, actually I entered a type, that "." before Labels.xml should
not be there. So I already excluded all the label files, I'll fix this
shortly.

As to why include other XML files, it's because the classpath logic changed
and we need to define things in Gradle (no longer loaded from OFBiz, which
is the correct thing to do. So your old code would not work (you can give
it a try). I think we're going overboard with trying to exclude as many
things as we can from the jar file given that it is a dynamic thing that
changes constantly.

On Thu, Dec 8, 2016 at 11:57 AM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:

> Le 06/12/2016 à 13:31, taher@apache.org a écrit :
>
>> Author: taher
>> Date: Tue Dec  6 12:31:00 2016
>> New Revision: 1772879
>>
>> URL: http://svn.apache.org/viewvc?rev=1772879&view=rev
>> Log:
>> Improved: Refactor and simplify the startup sequence in OFBiz
>> (OFBIZ-8337)
>>
>> This is another major commit to the refactoring of the startup sequence in
>> OFBiz with the following highlighted changes:
>>
>> [...]
>
>> - Add the DTDs defined in base through Gradle because we removed the
>>    NativeLibClassLoader and the classpath buildup logic in
>> StartupControlPanel
>>
>> Thanks: Jacopo Cappellato for reviewing the work
>>
>> Removed:
>>      ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/
>> base/start/NativeLibClassLoader.java
>> Modified:
>>      ofbiz/trunk/build.gradle
>>
> [...]
>
> Modified: ofbiz/trunk/build.gradle
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/build.gradle?rev=17
>> 72879&r1=1772878&r2=1772879&view=diff
>> ============================================================
>> ==================
>> --- ofbiz/trunk/build.gradle (original)
>> +++ ofbiz/trunk/build.gradle Tue Dec  6 12:31:00 2016
>> @@ -204,19 +204,19 @@ sourceSets {
>>           resources {
>>               srcDirs = getDirectoryInActiveComponents
>> IfExists('src/main/java')
>>               srcDirs += getDirectoryInActiveComponentsIfExists('config')
>> +            srcDirs += "${rootDir}/framework/base/dtd"
>>               exclude excludedJavaSources
>>               exclude excludedConfigFiles
>>               // Below are necessary for unit tests run by Gradle and
>> integration tests
>> +            exclude { FileTreeElement elem ->
>> elem.getName().contains('.Labels.xml') }
>>               exclude { FileTreeElement elem ->
>> elem.getName().contains('.properties') &&
>>                   !elem.getName().contains('start.properties') &&
>>                   !elem.getName().contains('load-data.properties') &&
>>                   !elem.getName().contains('debug.properties') &&
>>                   !elem.getName().contains('cache.properties') &&
>>                   !elem.getName().contains('test.properties') &&
>> -                !elem.getName().contains('rmi.properties')}
>> -            exclude { FileTreeElement elem ->
>> elem.getName().contains('.xml') &&
>> -                !elem.getName().contains('entityengine.xml')
>> -                }
>> +                !elem.getName().contains('rmi.properties')
>> +            }
>>           }
>>       }
>>
>>
> Hi Taher,
>
> I'm just begining to review and stumbled upon this. What is the plan of
> having all XML files embedded in the ofbiz.jar file?
>
> Notably the labels files are bad because being static in the ofbiz.jar
> file they no longer can be modified.
> It seems you took this into account after https://issues.apache.org/jira
> /browse/OFBIZ-8321 but wrongly wrote the related line (you would need a
> regexp)
>
> +            exclude { FileTreeElement elem ->
> elem.getName().contains('.Labels.xml') }
>
> Now I see no reasons to put other XML files in the ofbiz.jar file but
> entityengine.xml which should not be changed when OFBiz runs.
> So the only resources which should be in ofbiz.jars are
>     cache.properties
>     debug.properties
>     entityengine.xml
> Else please explain why.
>
> Thanks
>
> Jacques
>
>

Re: svn commit: r1772879 - in /ofbiz/trunk: ./ framework/base/src/main/java/org/apache/ofbiz/base/container/ framework/start/src/main/java/org/apache/ofbiz/base/start/

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 06/12/2016 � 13:31, taher@apache.org a �crit :
> Author: taher
> Date: Tue Dec  6 12:31:00 2016
> New Revision: 1772879
>
> URL: http://svn.apache.org/viewvc?rev=1772879&view=rev
> Log:
> Improved: Refactor and simplify the startup sequence in OFBiz
> (OFBIZ-8337)
>
> This is another major commit to the refactoring of the startup sequence in
> OFBiz with the following highlighted changes:
>
[...]
> - Add the DTDs defined in base through Gradle because we removed the
>    NativeLibClassLoader and the classpath buildup logic in StartupControlPanel
>
> Thanks: Jacopo Cappellato for reviewing the work
>
> Removed:
>      ofbiz/trunk/framework/start/src/main/java/org/apache/ofbiz/base/start/NativeLibClassLoader.java
> Modified:
>      ofbiz/trunk/build.gradle
[...]
> Modified: ofbiz/trunk/build.gradle
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/build.gradle?rev=1772879&r1=1772878&r2=1772879&view=diff
> ==============================================================================
> --- ofbiz/trunk/build.gradle (original)
> +++ ofbiz/trunk/build.gradle Tue Dec  6 12:31:00 2016
> @@ -204,19 +204,19 @@ sourceSets {
>           resources {
>               srcDirs = getDirectoryInActiveComponentsIfExists('src/main/java')
>               srcDirs += getDirectoryInActiveComponentsIfExists('config')
> +            srcDirs += "${rootDir}/framework/base/dtd"
>               exclude excludedJavaSources
>               exclude excludedConfigFiles
>               // Below are necessary for unit tests run by Gradle and integration tests
> +            exclude { FileTreeElement elem -> elem.getName().contains('.Labels.xml') }
>               exclude { FileTreeElement elem -> elem.getName().contains('.properties') &&
>                   !elem.getName().contains('start.properties') &&
>                   !elem.getName().contains('load-data.properties') &&
>                   !elem.getName().contains('debug.properties') &&
>                   !elem.getName().contains('cache.properties') &&
>                   !elem.getName().contains('test.properties') &&
> -                !elem.getName().contains('rmi.properties')}
> -            exclude { FileTreeElement elem -> elem.getName().contains('.xml') &&
> -                !elem.getName().contains('entityengine.xml')
> -                }
> +                !elem.getName().contains('rmi.properties')
> +            }
>           }
>       }
>   
Hi Taher,

I'm just begining to review and stumbled upon this. What is the plan of having all XML files embedded in the ofbiz.jar file?

Notably the labels files are bad because being static in the ofbiz.jar file they no longer can be modified.
It seems you took this into account after https://issues.apache.org/jira/browse/OFBIZ-8321 but wrongly wrote the related line (you would need a regexp)

+            exclude { FileTreeElement elem -> elem.getName().contains('.Labels.xml') }

Now I see no reasons to put other XML files in the ofbiz.jar file but entityengine.xml which should not be changed when OFBiz runs.
So the only resources which should be in ofbiz.jars are
     cache.properties
     debug.properties
     entityengine.xml
Else please explain why.

Thanks

Jacques