This is an automated email from the ASF dual-hosted git repository.
mthl pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/trunk by this push:
new a521de9 Reverted: "Fixed: Remove dependency management from
‘ComponentContainer’" (OFBIZ-11275)
a521de9 is described below
commit a521de9ad8a0b5a0f9ceadab348c46d7961ff89a
Author: Mathieu Lirzin <[email protected]>
AuthorDate: Sat Nov 9 00:50:56 2019 +0100
Reverted: "Fixed: Remove dependency management from ‘ComponentContainer’"
(OFBIZ-11275)
This reverts commit 3d3533cf5e04b303e5d7a4b0f178c54f10ddf620.
‘./gradlew cleanAll loadAll’ do not work anymore.
---
build.gradle | 2 +-
.../base/component/AlreadyLoadedException.java | 44 ++++++++
.../ofbiz/base/component/ComponentConfig.java | 61 +----------
.../ofbiz/base/container/ComponentContainer.java | 120 +++++++++++++++++++--
.../base/container/ComponentContainerTest.java | 2 +
5 files changed, 160 insertions(+), 69 deletions(-)
diff --git a/build.gradle b/build.gradle
index 0395b8c..6c8ac6c 100644
--- a/build.gradle
+++ b/build.gradle
@@ -309,7 +309,7 @@ checkstyle {
// the sum of errors that were present before introducing the
// ‘checkstyle’ tool present in the framework and in the official
// plugins.
- tasks.checkstyleMain.maxErrors = 37776
+ tasks.checkstyleMain.maxErrors = 37779
// Currently there are a lot of errors so we need to temporarily
// hide them to avoid polluting the terminal output.
showViolations = false
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/component/AlreadyLoadedException.java
b/framework/base/src/main/java/org/apache/ofbiz/base/component/AlreadyLoadedException.java
new file mode 100644
index 0000000..4485ccf
--- /dev/null
+++
b/framework/base/src/main/java/org/apache/ofbiz/base/component/AlreadyLoadedException.java
@@ -0,0 +1,44 @@
+/*******************************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+
*******************************************************************************/
+package org.apache.ofbiz.base.component;
+
+/**
+ * Component Already Loaded Exception
+ *
+ */
+@SuppressWarnings("serial")
+public class AlreadyLoadedException extends ComponentException {
+
+ public AlreadyLoadedException() {
+ super();
+ }
+
+ public AlreadyLoadedException(String str) {
+ super(str);
+ }
+
+ public AlreadyLoadedException(Throwable nested) {
+ super(nested);
+ }
+
+ public AlreadyLoadedException(String str, Throwable nested) {
+ super(str, nested);
+ }
+}
+
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
b/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
index c1a1ada..4a39885 100644
---
a/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
+++
b/framework/base/src/main/java/org/apache/ofbiz/base/component/ComponentConfig.java
@@ -18,9 +18,6 @@
*******************************************************************************/
package org.apache.ofbiz.base.component;
-import static java.util.function.Function.identity;
-import static java.util.stream.Collectors.toMap;
-
import java.io.InputStream;
import java.net.URL;
import java.nio.file.Files;
@@ -43,7 +40,6 @@ import org.apache.ofbiz.base.container.ContainerConfig;
import org.apache.ofbiz.base.location.FlexibleLocation;
import org.apache.ofbiz.base.util.Assert;
import org.apache.ofbiz.base.util.Debug;
-import org.apache.ofbiz.base.util.Digraph;
import org.apache.ofbiz.base.util.KeyStoreUtil;
import org.apache.ofbiz.base.util.UtilURL;
import org.apache.ofbiz.base.util.UtilValidate;
@@ -95,16 +91,6 @@ public final class ComponentConfig {
return componentConfigCache.values();
}
- /**
- * Sorts the cached component configurations.
- *
- * @throws ComponentException when a component configuration contains an
invalid dependency
- * or if the dependency graph contains a cycle.
- */
- public static void sortDependencies() throws ComponentException {
- componentConfigCache.sort();
- }
-
public static Stream<ComponentConfig> components() {
return componentConfigCache.values().stream();
}
@@ -695,8 +681,6 @@ public final class ComponentConfig {
private final Map<String, ComponentConfig> componentConfigs = new
LinkedHashMap<>();
// Root location mapped to global name.
private final Map<String, String> componentLocations = new HashMap<>();
- /** Sorted component configurations (based on depends-on declaration
if ofbiz-component.xml). */
- private List<ComponentConfig> componentConfigsSorted;
private synchronized ComponentConfig fromGlobalName(String globalName)
{
return componentConfigs.get(globalName);
@@ -717,49 +701,8 @@ public final class ComponentConfig {
return componentConfigs.put(globalName, config);
}
- /**
- * Provides a sequence of cached component configurations.
- *
- * <p>the order of the sequence is arbitrary unless the {@link
#sort())} has been invoked beforehand.
- *
- * @return the list of cached component configurations
- */
- synchronized List<ComponentConfig> values() {
- // When the configurations have not been sorted use arbitrary
order.
- List<ComponentConfig> res = (componentConfigsSorted == null)
- ? new ArrayList<>(componentConfigs.values())
- : componentConfigsSorted;
- return Collections.unmodifiableList(res);
- }
-
- /**
- * Provides the stored dependencies as component configurations.
- *
- * <p>Handle invalid dependencies by creating dummy component
configurations.
- *
- * @param the component configuration containing dependencies
- * @return a collection of component configurations dependencies.
- */
- private Collection<ComponentConfig>
componentConfigDeps(ComponentConfig cc) {
- return cc.getDependsOn().stream()
- .map(dep -> {
- ComponentConfig storedCc = componentConfigs.get(dep);
- return (storedCc != null) ? storedCc : new
Builder().globalName(dep).create();
- })
- .collect(Collectors.toList());
- }
-
- /**
- * Sorts the component configurations based on their “depends-on” XML
elements.
- */
- synchronized void sort() throws ComponentException {
- Map<ComponentConfig, Collection<ComponentConfig>> graphSpec =
componentConfigs.values().stream()
- .collect(toMap(identity(), this::componentConfigDeps));
- try {
- componentConfigsSorted = new Digraph<>(graphSpec).sort();
- } catch (IllegalArgumentException | IllegalStateException e) {
- throw new ComponentException("failed to initialize
components", e);
- }
+ synchronized Collection<ComponentConfig> values() {
+ return Collections.unmodifiableList(new
ArrayList<>(componentConfigs.values()));
}
}
diff --git
a/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
b/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
index 31f6e60..3f14707 100644
---
a/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
+++
b/framework/base/src/main/java/org/apache/ofbiz/base/container/ComponentContainer.java
@@ -26,7 +26,13 @@ import java.net.URLClassLoader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
import java.util.stream.Stream;
@@ -38,6 +44,7 @@ import
org.apache.ofbiz.base.component.ComponentLoaderConfig.ComponentDef;
import org.apache.ofbiz.base.start.Start;
import org.apache.ofbiz.base.start.StartupCommand;
import org.apache.ofbiz.base.util.Debug;
+import org.apache.ofbiz.base.util.UtilValidate;
/**
* ComponentContainer - StartupContainer implementation for Components
@@ -55,6 +62,9 @@ public class ComponentContainer implements Container {
private String name;
private final AtomicBoolean loaded = new AtomicBoolean(false);
+ /** The set of ready components in their inverse dependency order. */
+ private final LinkedHashSet<ComponentConfig> readyComponents = new
LinkedHashSet<>();
+ private static Map<String, List<String>> toBeLoadedComponents = new
ConcurrentHashMap<>();
@Override
public void init(List<StartupCommand> ofbizCommands, String name, String
configFile) throws ContainerException {
@@ -79,12 +89,11 @@ public class ComponentContainer implements Container {
for (ComponentDef def: ComponentLoaderConfig.getRootComponents()) {
loadComponent(ofbizHome, def);
}
- ComponentConfig.sortDependencies();
} catch (IOException | ComponentException e) {
throw new ContainerException(e);
}
String fmt = "Added class path for component : [%s]";
- List<Classpath> componentsClassPath = ComponentConfig.components()
+ List<Classpath> componentsClassPath = readyComponents.stream()
.peek(cmpnt -> Debug.logInfo(String.format(fmt,
cmpnt.getComponentName()), module))
.map(cmpnt -> buildClasspathFromComponentConfig(cmpnt))
.collect(Collectors.toList());
@@ -125,15 +134,19 @@ public class ComponentContainer implements Container {
* @param dir the location where the component should be loaded
* @param component a single component or a component directory definition
* @throws IOException when component directory loading fails.
+ * @throws ComponentException when retrieving component configuration
files fails.
*/
- private void loadComponent(Path dir, ComponentDef component) throws
IOException {
+ private void loadComponent(Path dir, ComponentDef component) throws
IOException, ComponentException {
Path location = component.location.isAbsolute() ? component.location :
dir.resolve(component.location);
switch (component.type) {
case COMPONENT_DIRECTORY:
loadComponentDirectory(location);
break;
case SINGLE_COMPONENT:
- retrieveComponentConfig(null, location);
+ ComponentConfig config = retrieveComponentConfig(null, location);
+ if (config != null) {
+ loadSingleComponent(config);
+ }
break;
}
}
@@ -144,8 +157,9 @@ public class ComponentContainer implements Container {
*
* @param directoryName the name of component directory to load
* @throws IOException
+ * @throws ComponentException
*/
- private void loadComponentDirectory(Path directoryName) throws IOException
{
+ private void loadComponentDirectory(Path directoryName) throws
IOException, ComponentException {
Debug.logInfo("Auto-Loading component directory : [" + directoryName +
"]", module);
if (Files.exists(directoryName) && Files.isDirectory(directoryName)) {
Path componentLoad =
directoryName.resolve(ComponentLoaderConfig.COMPONENT_LOAD_XML_FILENAME);
@@ -191,15 +205,56 @@ public class ComponentContainer implements Container {
* for loading purposes
*
* @param directoryPath a valid absolute path of a component directory
- * @throws IOException if an I/O error occurs when opening the directory
+ * @throws IOException
+ * @throws ComponentException
*/
- private static void loadComponentsInDirectory(Path directoryPath) throws
IOException {
+ private void loadComponentsInDirectory(Path directoryPath) throws
IOException, ComponentException {
try (Stream<Path> paths = Files.list(directoryPath)) {
- paths.sorted()
+ List<ComponentConfig> componentConfigs = paths.sorted()
.map(cmpnt ->
directoryPath.resolve(cmpnt).toAbsolutePath().normalize())
.filter(Files::isDirectory)
.filter(dir ->
Files.exists(dir.resolve(ComponentConfig.OFBIZ_COMPONENT_XML_FILENAME)))
- .forEach(componentDir -> retrieveComponentConfig(null,
componentDir));
+ .map(componentDir -> retrieveComponentConfig(null,
componentDir))
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+
+ for (ComponentConfig cmpnt : componentConfigs) {
+ loadSingleComponent(cmpnt);
+ }
+ }
+ loadComponentWithDependency();
+ }
+
+ /**
+ * Checks dependency for unloaded components and add them into
+ * componentsClassPath
+ *
+ * @throws IOException
+ * @throws ComponentException
+ */
+ private void loadComponentWithDependency() throws IOException,
ComponentException {
+ while (true) {
+ if (UtilValidate.isEmpty(toBeLoadedComponents)) {
+ return;
+ } else {
+ for (Map.Entry<String, List<String>> entries :
toBeLoadedComponents.entrySet()) {
+ ComponentConfig config =
retrieveComponentConfig(entries.getKey(), null);
+ if (config.enabled()) {
+ List<String> dependencyList =
checkDependencyForComponent(config);
+ if (UtilValidate.isNotEmpty(dependencyList)) {
+
toBeLoadedComponents.replace(config.getComponentName(), dependencyList);
+ String msg = "Not loading component [" +
config.getComponentName() + "] because it's dependent Component is not loaded [
" + dependencyList + "]";
+ Debug.logInfo(msg, module);
+ }
+ if (UtilValidate.isEmpty(dependencyList)) {
+ readyComponents.add(config);
+
toBeLoadedComponents.replace(config.getComponentName(), dependencyList);
+ }
+ } else {
+ Debug.logInfo("Not loading component [" +
config.getComponentName() + "] because it's disabled", module);
+ }
+ }
+ }
}
}
@@ -224,6 +279,53 @@ public class ComponentContainer implements Container {
}
/**
+ * Load a single component by adding all its classpath entries to
+ * the list of classpaths to be loaded
+ *
+ * @param config the component configuration
+ * @throws ComponentException
+ */
+ private void loadSingleComponent(ComponentConfig config) throws
ComponentException {
+ if (config.enabled()) {
+ List<String> dependencyList = checkDependencyForComponent(config);
+ if (UtilValidate.isEmpty(dependencyList)) {
+ readyComponents.add(config);
+ }
+ } else {
+ Debug.logInfo("Not loading component [" +
config.getComponentName() + "] because it's disabled", module);
+ }
+ }
+
+ /**
+ * Check for components loaded and Removes loaded components dependency
+ * from list of unloaded components
+ *
+ * @param config the component configuration
+ * @throws ComponentException
+ */
+ private List<String> checkDependencyForComponent(ComponentConfig config)
throws ComponentException {
+ List<String> dependencyList = new ArrayList<>(config.getDependsOn());
+ if (UtilValidate.isNotEmpty(dependencyList)) {
+ Set<String> resolvedDependencyList = new HashSet<>();
+ for (String dependency : dependencyList) {
+ Debug.logInfo("Component : " + config.getComponentName() + "
is Dependent on " + dependency, module);
+ ComponentConfig componentConfig =
ComponentConfig.getComponentConfig(String.valueOf(dependency));
+ if (readyComponents.contains(componentConfig)) {
+ resolvedDependencyList.add(dependency);
+ }
+ }
+ resolvedDependencyList.forEach(resolvedDependency ->
Debug.logInfo("Resolved : " + resolvedDependency + " Dependency for Component "
+ config.getComponentName(), module));
+ dependencyList.removeAll(resolvedDependencyList);
+ if (UtilValidate.isEmpty(dependencyList)) {
+ toBeLoadedComponents.remove(config.getComponentName());
+ } else {
+ toBeLoadedComponents.put(config.getComponentName(),
dependencyList);
+ }
+ }
+ return dependencyList;
+ }
+
+ /**
* Constructs a {@code Classpath} object for a specific component
definition.
*
* @param config the component configuration
diff --git
a/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
b/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
index 57be05a..358f0bc 100644
---
a/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
+++
b/framework/base/src/test/java/org/apache/ofbiz/base/container/ComponentContainerTest.java
@@ -35,6 +35,7 @@ import org.apache.ofbiz.base.component.ComponentConfig;
import org.apache.ofbiz.base.start.StartupException;
import org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
@@ -60,6 +61,7 @@ public class ComponentContainerTest {
}
}
+ @Ignore("Dependency declarations do not impact the components order")
@Test
public void testCheckDependencyForComponent() throws ContainerException,
StartupException, MalformedURLException {
ComponentContainer containerObj = new ComponentContainer();
