svn commit: r1868030 - in /ofbiz/ofbiz-framework/trunk/framework: base/src/main/java/org/apache/ofbiz/base/component/ base/src/main/java/org/apache/ofbiz/base/container/ webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/

[mthl]

Author: mthl
Date: Sat Oct  5 16:30:12 2019
New Revision: 1868030

URL: http://svn.apache.org/viewvc?rev=1868030&view=rev
Log:
Improved: Refactor ‘ComponentConfig.ClasspathInfo’
(OFBIZ-11192)(OFBIZ-11238)

The validity checks have been moved at construction time instead of
relying on client code to ensure that the class path information are
sound.

A new ‘ComponentConfig.ClasspathInfo.Type’ enum has been defined to
improve type safety.

The location of the class path information is now using a
‘java.nio.file.Path’ class instance.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
    ofbiz/ofbiz-framework/trunk/framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelManagerFactory.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java?rev=1868030&r1=1868029&r2=1868030&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java Sat Oct  5 16:30:12 2019
@@ -287,7 +287,7 @@ public final class ComponentConfig {
         this.componentName = b.componentName;
         this.enabled = b.enabled;
         this.resourceLoaderInfos = b.resourceLoaderInfos;
-        this.classpathInfos = b.classpathInfos;
+        this.classpathInfos = (b.classpathInfos == null) ? Collections.emptyList() : b.classpathInfos;
         this.dependsOnInfos = (b.dependsOnInfos == null) ? Collections.emptyList() : b.dependsOnInfos;
         this.entityResourceInfos = b.entityResourceInfos;
         this.serviceResourceInfos = b.serviceResourceInfos;
@@ -452,7 +452,14 @@ public final class ComponentConfig {
     private <T> List<T> collectElements(Element ofbizComponentElement, String elemName,
             BiFunction<ComponentConfig, Element, T> mapper) {
         return UtilXml.childElementList(ofbizComponentElement, elemName).stream()
-                .map(element -> mapper.apply(this, element))
+                .flatMap(element -> {
+                    try {
+                        return Stream.of(mapper.apply(this, element));
+                    } catch(IllegalArgumentException e) {
+                        Debug.log(e.getMessage());
+                        return Stream.empty();
+                    }
+                })
                 .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList));
     }
 
@@ -460,6 +467,11 @@ public final class ComponentConfig {
         return this.enabled;
     }
 
+    /**
+     * Provides the list of class path information.
+     *
+     * @return an immutable list containing the class path information.
+     */
     public List<ClasspathInfo> getClasspathInfos() {
         return this.classpathInfos;
     }
@@ -610,14 +622,57 @@ public final class ComponentConfig {
      *
      */
     public static final class ClasspathInfo {
-        public final ComponentConfig componentConfig;
-        public final String type;
-        public final String location;
+        private final ComponentConfig componentConfig;
+        private final Type type;
+        private final Path location;
 
         private ClasspathInfo(ComponentConfig componentConfig, Element element) {
+            String loc = element.getAttribute("location").replace('\\', '/');
+            Path location = Paths.get(loc.startsWith("/") ? loc.substring(1) : loc).normalize();
+            Path fullLocation = componentConfig.rootLocation().resolve(location);
+            Type type = Type.of(element.getAttribute("type"));
+            switch (type) {
+            case DIR:
+                if (!Files.isDirectory(fullLocation)) {
+                    String msg = String.format("Classpath location '%s' is not a valid directory", location);
+                    throw new IllegalArgumentException(msg);
+                }
+                break;
+            case JAR:
+                if (Files.notExists(fullLocation)) {
+                    String msg = String.format("Classpath location '%s' does not exist", location);
+                    throw new IllegalArgumentException(msg);
+                }
+                break;
+            }
+
             this.componentConfig = componentConfig;
-            this.type = element.getAttribute("type");
-            this.location = element.getAttribute("location");
+            this.type = type;
+            this.location = location;
+        }
+
+        public ComponentConfig componentConfig() {
+            return componentConfig;
+        }
+
+        public Type type() {
+            return type;
+        }
+
+        public Path location() {
+            return componentConfig.rootLocation().resolve(location);
+        }
+
+        public static enum Type {
+            DIR, JAR;
+
+            private static Type of(String type) {
+                if ("jar".equals(type) || "dir".equals(type)) {
+                    return valueOf(type.toUpperCase());
+                } else {
+                    throw new IllegalArgumentException("Classpath type '" + type + "' is not supported; '");
+                }
+            }
         }
     }
 

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java?rev=1868030&r1=1868029&r2=1868030&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java Sat Oct  5 16:30:12 2019
@@ -30,6 +30,8 @@ import java.util.List;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.apache.ofbiz.base.component.ComponentConfig.ClasspathInfo;
+
 /**
  * A class path object.
  *
@@ -48,19 +50,24 @@ final class Classpath {
      * In the directory case, all files ending with ".jar" or ".zip" inside this directory
      * are added to the class path.
      *
-     * @param file  the absolute normalized file name of a directory or a file that must exist
-     * @param type  either "dir" or "jar"
-     * @throws NullPointerException when {@code file} is {@code null}.
+     * @param cpi  a valid class path information
+     * @throws NullPointerException when {@code cpi} is {@code null}.
      */
-    void add(Path file, String type) {
-        elements.add(file);
-        if (Files.isDirectory(file) && "dir".equals(type)) {
+    void add(ClasspathInfo cpi) {
+        Path file = cpi.location();
+        switch (cpi.type()) {
+        case JAR:
+            elements.add(file);
+            break;
+        case DIR:
+            elements.add(file);
             try (Stream<Path> innerFiles = Files.list(file)) {
                 innerFiles.filter(JAR_ZIP_FILES::matches).forEach(elements::add);
             } catch (IOException e) {
                 String fmt = "Warning : Module classpath component '%s' is not valid and will be ignored...";
                 System.err.println(String.format(fmt, file));
             }
+            break;
         }
     }
 

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java?rev=1868030&r1=1868029&r2=1868030&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java Sat Oct  5 16:30:12 2019
@@ -325,35 +325,16 @@ public class ComponentContainer implemen
     }
 
     /**
-     * Construct a <code>Classpath</code> object for a certain component based
-     * on its configuration defined in <code>ComponentConfig</code>
+     * Constructs a {@code Classpath} object for a specific component definition.
      *
-     * @param config the component configuration
-     * @return the constructed classpath
-     * @throws IOException
+     * @param config  the component configuration
+     * @return the associated class path information
+     * @see ComponentConfig
      */
-    private static Classpath buildClasspathFromComponentConfig(ComponentConfig config) throws IOException {
-        Classpath classPath = new Classpath();
-        Path configRoot = config.rootLocation();
-        List<ComponentConfig.ClasspathInfo> classpathInfos = config.getClasspathInfos();
-
-        for (ComponentConfig.ClasspathInfo cp: classpathInfos) {
-            String location = cp.location.replace('\\', '/');
-            if (!"jar".equals(cp.type) && !"dir".equals(cp.type)) {
-                Debug.logError("Classpath type '" + cp.type + "' is not supported; '" + location + "' not loaded", module);
-                continue;
-            }
-
-            location = location.startsWith("/") ? location.substring(1) : location;
-            String dirLoc = location.endsWith("/*") ? location.substring(0, location.length() - 2) : location;
-            Path path = configRoot.resolve(dirLoc).normalize();
-            if (Files.exists(path)) {
-                classPath.add(path, cp.type);
-            } else {
-                Debug.logWarning("Location '" + path + "' does not exist", module);
-            }
-        }
-        return classPath;
+    private static Classpath buildClasspathFromComponentConfig(ComponentConfig config) {
+        Classpath res = new Classpath();
+        config.getClasspathInfos().forEach(res::add);
+        return res;
     }
 
     @Override
@@ -364,5 +345,4 @@ public class ComponentContainer implemen
     public String getName() {
         return name;
     }
-
 }

Modified: ofbiz/ofbiz-framework/trunk/framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelManagerFactory.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelManagerFactory.java?rev=1868030&r1=1868029&r2=1868030&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelManagerFactory.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelManagerFactory.java Sat Oct  5 16:30:12 2019
@@ -21,7 +21,6 @@ package org.apache.ofbiz.webtools.labelm
 import java.io.File;
 import java.io.IOException;
 import java.net.MalformedURLException;
-import java.nio.file.Path;
 import java.util.Collection;
 import java.util.LinkedList;
 import java.util.List;
@@ -84,16 +83,11 @@ public class LabelManagerFactory {
         filesFound = new TreeMap<>();
         List<ClasspathInfo> cpInfos = ComponentConfig.getAllClasspathInfos();
         for (ClasspathInfo cpi : cpInfos) {
-            if ("dir".equals(cpi.type)) {
-                Path configRoot = cpi.componentConfig.rootLocation();
-                String location = cpi.location.replace('\\', '/');
-                if (location.startsWith("/")) {
-                    location = location.substring(1);
-                }
-                Path fullLocation = configRoot.resolve(location);
-                List<File> resourceFiles = FileUtil.findXmlFiles(fullLocation.toString(), null, "resource", null);
+            if (ClasspathInfo.Type.DIR == cpi.type()) {
+                List<File> resourceFiles = FileUtil.findXmlFiles(cpi.location().toString(), null, "resource", null);
                 for (File resourceFile : resourceFiles) {
-                    filesFound.put(resourceFile.getName(), new LabelFile(resourceFile, cpi.componentConfig.getComponentName()));
+                    filesFound.put(resourceFile.getName(),
+                            new LabelFile(resourceFile, cpi.componentConfig().getComponentName()));
                 }
             }
         }