Skip to content

Commit

Permalink
Improved: Refactor ‘ComponentConfig.ClasspathInfo’
Browse files Browse the repository at this point in the history 
(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.


git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1868030 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@mthl
mthl committed Oct 5, 2019
1 parent f628dec commit 13188ab
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 51 deletions.
69 changes: 62 additions & 7 deletions framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
View file
Expand Up @@ -287,7 +287,7 @@ private ComponentConfig(Builder b) {
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;
Expand Down Expand Up @@ -452,14 +452,26 @@ private ComponentConfig(String globalName, String rootLocation) throws Component
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));
}

public boolean enabled() {
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;
}
Expand Down Expand Up @@ -610,14 +622,57 @@ public boolean isFileResourceLoader(String resourceLoaderName) throws ComponentE
*
*/
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; '");
}
}
}
}

Expand Down
19 changes: 13 additions & 6 deletions framework/base/src/main/java/org/apache/ofbiz/base/container/Classpath.java
View file
Expand Up @@ -30,6 +30,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.apache.ofbiz.base.component.ComponentConfig.ClasspathInfo;

/**
* A class path object.
*
Expand All @@ -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;
}
}

Expand Down
36 changes: 8 additions & 28 deletions framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
View file
Expand Up @@ -325,35 +325,16 @@ private List<ComponentConfig.DependsOnInfo> checkDependencyForComponent(Componen
}

/**
* 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
Expand All @@ -364,5 +345,4 @@ public void stop() {
public String getName() {
return name;
}

}
14 changes: 4 additions & 10 deletions ...rk/webtools/src/main/java/org/apache/ofbiz/webtools/labelmanager/LabelManagerFactory.java
View file
Expand Up @@ -21,7 +21,6 @@
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;
Expand Down Expand Up @@ -84,16 +83,11 @@ protected static void loadLabelFiles() throws IOException {
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()));
}
}
}
Expand Down

0 comments on commit 13188ab

Please sign in to comment.