Issue #4628 - Non-Required Module Dependency Support

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
This commit is contained in:
Joakim Erdfelt 2020-03-02 10:13:29 -06:00
parent 8aabb52557
commit 1ee8c8ad3a
No known key found for this signature in database
GPG Key ID: 2D0E1FB8FE4B68B4
8 changed files with 211 additions and 20 deletions

View File

@ -20,7 +20,6 @@ package org.eclipse.jetty.start;
import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.charset.StandardCharsets;
@ -154,12 +153,12 @@ public class Module implements Comparable<Module>
private final List<String> _license = new ArrayList<>();
/**
* Dependencies
* Dependencies from {@code [depends]} section
*/
private final List<String> _depends = new ArrayList<>();
/**
* Optional
* Optional dependencies from {@code [optional]} section are structural in nature.
*/
private final Set<String> _optional = new HashSet<>();
@ -196,6 +195,18 @@ public class Module implements Comparable<Module>
process(basehome);
}
public static boolean isRequiredDependency(String depends)
{
return (depends != null) && (depends.charAt(0) != '?');
}
public static String normalizeModuleName(String name)
{
if (!isRequiredDependency(name))
return name.substring(1);
return name;
}
public String getName()
{
return _name;

View File

@ -247,6 +247,7 @@ public class ModuleGraphWriter
{
for (String depends : module.getDepends())
{
depends = Module.normalizeModuleName(depends);
out.printf(" \"%s\" -> \"%s\";%n", module.getName(), depends);
}
for (String optional : module.getOptional())

View File

@ -20,6 +20,7 @@ package org.eclipse.jetty.start;
import java.io.FileInputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
@ -132,7 +133,12 @@ public class Modules implements Iterable<Module>
label = " Depend: %s";
for (String parent : module.getDepends())
{
parent = Module.normalizeModuleName(parent);
System.out.printf(label, parent);
if (!Module.isRequiredDependency(parent))
{
System.out.print(" [not-required]");
}
label = ", %s";
}
System.out.println();
@ -352,45 +358,59 @@ public class Modules implements Iterable<Module>
// Process module dependencies (always processed as may be dynamic)
StartLog.debug("Enabled module %s depends on %s", module.getName(), module.getDepends());
for (String dependsOn : module.getDepends())
for (String dependsOnRaw : module.getDepends())
{
// Look for modules that provide that dependency
Set<Module> providers = getAvailableProviders(dependsOn);
boolean isRequired = Module.isRequiredDependency(dependsOnRaw);
// Final to allow lambda's below to use name
final String dependentModule = Module.normalizeModuleName(dependsOnRaw);
StartLog.debug("Module %s depends on %s provided by %s", module, dependsOn, providers);
// Look for modules that provide that dependency
Set<Module> providers = getAvailableProviders(dependentModule);
StartLog.debug("Module %s depends on %s provided by %s", module, dependentModule, providers);
// If there are no known providers of the module
if (providers.isEmpty())
{
// look for a dynamic module
if (dependsOn.contains("/"))
if (dependentModule.contains("/"))
{
Path file = _baseHome.getPath("modules/" + dependsOn + ".mod");
registerModule(file).expandDependencies(_args.getProperties());
providers = _provided.get(dependsOn);
if (providers == null || providers.isEmpty())
throw new UsageException("Module %s does not provide %s", _baseHome.toShortForm(file), dependsOn);
Path file = _baseHome.getPath("modules/" + dependentModule + ".mod");
if (isRequired || Files.exists(file))
{
registerModule(file).expandDependencies(_args.getProperties());
providers = _provided.get(dependentModule);
if (providers == null || providers.isEmpty())
throw new UsageException("Module %s does not provide %s", _baseHome.toShortForm(file), dependentModule);
enable(newlyEnabled, providers.stream().findFirst().get(), "dynamic dependency of " + module.getName(), true);
enable(newlyEnabled, providers.stream().findFirst().get(), "dynamic dependency of " + module.getName(), true);
continue;
}
}
// is this a non-required module
if (!isRequired)
{
StartLog.debug("Skipping non-required module [%s]: doesn't exist", dependentModule);
continue;
}
throw new UsageException("No module found to provide %s for %s", dependsOn, module);
// throw an exception (not a dynamic module and a required dependency)
throw new UsageException("No module found to provide %s for %s", dependentModule, module);
}
// If a provider is already enabled, then add a transitive enable
if (providers.stream().filter(Module::isEnabled).count() > 0)
providers.stream().filter(m -> m.isEnabled() && !m.equals(module)).forEach(m -> enable(newlyEnabled, m, "transitive provider of " + dependsOn + " for " + module.getName(), true));
if (providers.stream().anyMatch(Module::isEnabled))
providers.stream().filter(m -> m.isEnabled() && !m.equals(module)).forEach(m -> enable(newlyEnabled, m, "transitive provider of " + dependentModule + " for " + module.getName(), true));
else
{
// Is there an obvious default?
Optional<Module> dftProvider = (providers.size() == 1)
? providers.stream().findFirst()
: providers.stream().filter(m -> m.getName().equals(dependsOn)).findFirst();
: providers.stream().filter(m -> m.getName().equals(dependentModule)).findFirst();
if (dftProvider.isPresent())
enable(newlyEnabled, dftProvider.get(), "transitive provider of " + dependsOn + " for " + module.getName(), true);
enable(newlyEnabled, dftProvider.get(), "transitive provider of " + dependentModule + " for " + module.getName(), true);
else if (StartLog.isDebugEnabled())
StartLog.debug("Module %s requires a %s implementation from one of %s", module, dependsOn, providers);
StartLog.debug("Module %s requires a %s implementation from one of %s", module, dependentModule, providers);
}
}
}

View File

@ -36,6 +36,8 @@ import org.junit.jupiter.api.extension.ExtendWith;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
@ExtendWith(WorkDirExtension.class)
public class ModulesTest
@ -202,6 +204,152 @@ public class ModulesTest
assertThat("Resolved XMLs: " + actualXmls, actualXmls, contains(expectedXmls.toArray()));
}
@Test
public void testResolveNotRequiredModuleNotFound() throws IOException
{
// Test Env
File homeDir = MavenTestingUtils.getTestResourceDir("non-required-deps");
File baseDir = testdir.getEmptyPathDir().toFile();
String[] cmdLine = new String[]{"bar.type=cannot-find-me"};
// Configuration
CommandLineConfigSource cmdLineSource = new CommandLineConfigSource(cmdLine);
ConfigSources config = new ConfigSources();
config.add(cmdLineSource);
config.add(new JettyHomeConfigSource(homeDir.toPath()));
config.add(new JettyBaseConfigSource(baseDir.toPath()));
// Initialize
BaseHome basehome = new BaseHome(config);
StartArgs args = new StartArgs(basehome);
args.parse(config);
// Test Modules
Modules modules = new Modules(basehome, args);
modules.registerAll();
// Enable module
modules.enable("bar", TEST_SOURCE);
// Collect active module list
List<Module> active = modules.getEnabled();
// Assert names are correct, and in the right order
List<String> expectedNames = new ArrayList<>();
expectedNames.add("foo");
expectedNames.add("bar");
List<String> actualNames = new ArrayList<>();
for (Module actual : active)
{
actualNames.add(actual.getName());
}
assertThat("Resolved Names: " + actualNames, actualNames, contains(expectedNames.toArray()));
Props props = args.getProperties();
assertThat(props.getString("bar.name"), is(nullValue()));
}
@Test
public void testResolveNotRequiredModuleFound() throws IOException
{
// Test Env
File homeDir = MavenTestingUtils.getTestResourceDir("non-required-deps");
File baseDir = testdir.getEmptyPathDir().toFile();
String[] cmdLine = new String[]{"bar.type=dive"};
// Configuration
CommandLineConfigSource cmdLineSource = new CommandLineConfigSource(cmdLine);
ConfigSources config = new ConfigSources();
config.add(cmdLineSource);
config.add(new JettyHomeConfigSource(homeDir.toPath()));
config.add(new JettyBaseConfigSource(baseDir.toPath()));
// Initialize
BaseHome basehome = new BaseHome(config);
StartArgs args = new StartArgs(basehome);
args.parse(config);
// Test Modules
Modules modules = new Modules(basehome, args);
modules.registerAll();
// Enable module
modules.enable("bar", TEST_SOURCE);
// Collect active module list
List<Module> active = modules.getEnabled();
// Assert names are correct, and in the right order
List<String> expectedNames = new ArrayList<>();
expectedNames.add("foo");
expectedNames.add("bar");
expectedNames.add("bar-dive");
List<String> actualNames = new ArrayList<>();
for (Module actual : active)
{
actualNames.add(actual.getName());
}
assertThat("Resolved Names: " + actualNames, actualNames, contains(expectedNames.toArray()));
Props props = args.getProperties();
assertThat(props.getString("bar.name"), is("dive"));
}
@Test
public void testResolveNotRequiredModuleFoundDynamic() throws IOException
{
// Test Env
File homeDir = MavenTestingUtils.getTestResourceDir("non-required-deps");
File baseDir = testdir.getEmptyPathDir().toFile();
String[] cmdLine = new String[]{"bar.type=dynamic"};
// Configuration
CommandLineConfigSource cmdLineSource = new CommandLineConfigSource(cmdLine);
ConfigSources config = new ConfigSources();
config.add(cmdLineSource);
config.add(new JettyHomeConfigSource(homeDir.toPath()));
config.add(new JettyBaseConfigSource(baseDir.toPath()));
// Initialize
BaseHome basehome = new BaseHome(config);
StartArgs args = new StartArgs(basehome);
args.parse(config);
// Test Modules
Modules modules = new Modules(basehome, args);
modules.registerAll();
// Enable module
modules.enable("bar", TEST_SOURCE);
// Collect active module list
List<Module> active = modules.getEnabled();
// Assert names are correct, and in the right order
List<String> expectedNames = new ArrayList<>();
expectedNames.add("foo");
expectedNames.add("bar");
expectedNames.add("impls/bar-dynamic");
List<String> actualNames = new ArrayList<>();
for (Module actual : active)
{
actualNames.add(actual.getName());
}
assertThat("Resolved Names: " + actualNames, actualNames, contains(expectedNames.toArray()));
Props props = args.getProperties();
assertThat(props.getString("bar.name"), is("dynamic"));
}
private List<String> normalizeLibs(List<Module> active)
{
List<String> libs = new ArrayList<>();

View File

@ -0,0 +1,2 @@
[ini]
bar.name=dive

View File

@ -0,0 +1,6 @@
# Top level mod
[depends]
foo
?bar-${bar.type}
?impls/bar-${bar.type}

View File

@ -0,0 +1 @@
# nothing here

View File

@ -0,0 +1,2 @@
[ini]
bar.name=dynamic