Skip to content

Commit d0fd238

Browse files
HannesWellHannesWell
committed
[FIXUP?] Suggestions for enhancements
- Move code a bit around, trying to simplify it a bit - Implement version handling when looking up dependencies in host - Avoid unnecessary code - Add TODOs
1 parent 6634744 commit d0fd238

File tree

3 files changed

+83
-71
lines changed

3 files changed

+83
-71
lines changed

‎ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/PDECore.java‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@
1616
import java.lang.reflect.InvocationTargetException;
1717
import java.net.URI;
1818
import java.util.Arrays;
19+
import java.util.Comparator;
1920
import java.util.HashMap;
2021
import java.util.Hashtable;
22+
import java.util.List;
2123
import java.util.Map;
24+
import java.util.stream.Collectors;
2225
import java.util.stream.Stream;
2326

2427
import org.eclipse.core.resources.IResourceChangeEvent;
@@ -56,6 +59,8 @@
5659
import org.osgi.framework.FrameworkUtil;
5760
import org.osgi.framework.ServiceReference;
5861
import org.osgi.framework.ServiceRegistration;
62+
import org.osgi.framework.Version;
63+
import org.osgi.framework.VersionRange;
5964
import org.osgi.util.tracker.ServiceTracker;
6065

6166
import aQute.bnd.build.Workspace;
@@ -109,7 +114,7 @@ public class PDECore extends Plugin implements DebugOptionsListener {
109114
*/
110115
private static PDEPreferencesManager fPreferenceManager;
111116

112-
private Map<String, IPluginModelBase> fHostPlugins;
117+
private Map<String, List<IPluginModelBase>> fHostPlugins;
113118

114119
public static PDECore getDefault() {
115120
return inst;
@@ -221,7 +226,28 @@ public PDECore() {
221226
inst = this;
222227
}
223228

224-
public synchronized IPluginModelBase findPluginInHost(String id) {
229+
public synchronized IPluginModelBase findPluginInHost(String id, VersionRange version) {
230+
Map<String, List<IPluginModelBase>> hostPlugins = getHostPlugins();
231+
if (hostPlugins == null) {
232+
return null;
233+
}
234+
Stream<IPluginModelBase> plugins = hostPlugins.getOrDefault(id, List.of()).stream();
235+
if (version != null) {
236+
plugins = plugins.filter(p -> version.includes(getVersion(p)));
237+
}
238+
return plugins.max(VERSION).orElse(null);
239+
}
240+
241+
private static final Comparator<IPluginModelBase> VERSION = Comparator.comparing(PDECore::getVersion);
242+
243+
private static Version getVersion(IPluginModelBase p) {
244+
// TODO: Check the following also works for plugins from host. If yes,
245+
// use it.
246+
Version version2 = p.getBundleDescription().getVersion();
247+
return Version.parseVersion(p.getPluginBase().getVersion());
248+
}
249+
250+
private synchronized Map<String, List<IPluginModelBase>> getHostPlugins() {
225251
if (fHostPlugins == null) {
226252
fHostPlugins = new HashMap<>();
227253

@@ -239,12 +265,10 @@ public synchronized IPluginModelBase findPluginInHost(String id) {
239265
PDEState state = new PDEState(pluginPaths, true, false, new NullProgressMonitor());
240266
state.resolveState(false);
241267

242-
for (IPluginModelBase plugin : state.getTargetModels()) {
243-
fHostPlugins.put(plugin.getPluginBase().getId(), plugin);
244-
}
268+
fHostPlugins = Arrays.stream(state.getTargetModels())
269+
.collect(Collectors.groupingBy(p -> p.getPluginBase().getId()));
245270
}
246-
247-
return fHostPlugins.get(id);
271+
return fHostPlugins;
248272
}
249273

250274
public PluginModelManager getModelManager() {

‎ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/JUnitLaunchRequirements.java‎

Lines changed: 51 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@
1616

1717
import java.util.ArrayList;
1818
import java.util.Collection;
19-
import java.util.Collections;
19+
import java.util.LinkedHashMap;
2020
import java.util.LinkedHashSet;
2121
import java.util.List;
2222
import java.util.Map;
23+
import java.util.Map.Entry;
24+
import java.util.Optional;
2325
import java.util.Set;
2426

2527
import org.eclipse.core.runtime.CoreException;
26-
import org.eclipse.core.runtime.IStatus;
2728
import org.eclipse.core.runtime.Status;
2829
import org.eclipse.debug.core.ILaunchConfiguration;
2930
import org.eclipse.osgi.service.resolver.BundleDescription;
@@ -33,127 +34,114 @@
3334
import org.eclipse.pde.internal.core.DependencyManager;
3435
import org.eclipse.pde.internal.core.PDECore;
3536
import org.eclipse.pde.internal.launching.launcher.BundleLauncherHelper;
37+
import org.osgi.framework.Version;
3638
import org.osgi.framework.VersionRange;
3739
import org.osgi.framework.wiring.BundleRevision;
3840

3941
public class JUnitLaunchRequirements {
4042

41-
public static final String JUNIT4_RUNTIME_PLUGIN = "org.eclipse.jdt.junit4.runtime"; //$NON-NLS-1$
42-
public static final String JUNIT5_RUNTIME_PLUGIN = "org.eclipse.jdt.junit5.runtime"; //$NON-NLS-1$
43+
public static final String JUNIT4_JDT_RUNTIME_PLUGIN = "org.eclipse.jdt.junit4.runtime"; //$NON-NLS-1$
44+
public static final String JUNIT5_JDT_RUNTIME_PLUGIN = "org.eclipse.jdt.junit5.runtime"; //$NON-NLS-1$
4345

4446
private static final VersionRange JUNIT5_VERSIONS = new VersionRange("[1, 5)"); //$NON-NLS-1$
4547

4648
// we add launcher and jupiter.engine to support @RunWith(JUnitPlatform.class)
47-
private static final String[] JUNIT5_RUN_WITH_PLUGINS = {"junit-platform-launcher", //$NON-NLS-1$
48-
"junit-jupiter-engine", //$NON-NLS-1$
49-
};
49+
private static final Map<String, List<String>> JUNIT5_RUN_WITH_BUNDLES = new LinkedHashMap<>();
50+
static { // consider JUnit bundle names from old Eclipse-Orbit times. Assume either only new or only old names are used.
51+
JUNIT5_RUN_WITH_BUNDLES.put("junit-platform-runner", List.of("junit-platform-launcher", "junit-jupiter-engine")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
52+
JUNIT5_RUN_WITH_BUNDLES.put("org.junit.platform.runner", List.of("org.junit.platform.launcher", "org.junit.jupiter.engine")); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
53+
}
5054

5155
public static void addRequiredJunitRuntimePlugins(ILaunchConfiguration configuration, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
52-
Collection<String> plugins = getRequiredJunitRuntimePlugins(configuration);
53-
addPlugins(plugins, allBundles, allModels);
54-
if (plugins.contains(JUNIT5_RUNTIME_PLUGIN) && (allBundles.containsKey("junit-platform-runner") || allBundles.containsKey("org.junit.platform.runner"))) { //$NON-NLS-1$ //$NON-NLS-2$
55-
Set<BundleDescription> descriptions = JUnitLaunchRequirements.junit5PlatformRequirements();
56-
Set<BundleDescription> junitRquirements = DependencyManager.findRequirementsClosure(descriptions);
57-
addAbsentRequirements(junitRquirements, allBundles, allModels);
56+
Collection<String> runtimePlugins = getRequiredJunitRuntimeEclipsePlugins(configuration);
57+
58+
Set<BundleDescription> addedRuntimeBundles = addAbsentRequirements(runtimePlugins, allBundles, allModels);
59+
Set<BundleDescription> runtimeRequirements = DependencyManager.findRequirementsClosure(addedRuntimeBundles);
60+
addAbsentRequirements(runtimeRequirements, allBundles, allModels);
61+
62+
if (runtimePlugins.contains(JUNIT5_JDT_RUNTIME_PLUGIN)) {
63+
Optional<List<String>> runWithBundles = JUNIT5_RUN_WITH_BUNDLES.entrySet().stream().filter(e -> allBundles.containsKey(e.getKey())).findFirst().map(Entry::getValue);
64+
if (runWithBundles.isPresent()) {
65+
Set<BundleDescription> descriptions = findBundlesInTargetOrHost(runWithBundles.get(), JUNIT5_VERSIONS);
66+
Set<BundleDescription> junitRquirements = DependencyManager.findRequirementsClosure(descriptions);
67+
addAbsentRequirements(junitRquirements, allBundles, allModels);
68+
}
5869
}
5970
}
6071

6172
@SuppressWarnings("restriction")
62-
public static Collection<String> getRequiredJunitRuntimePlugins(ILaunchConfiguration configuration) {
73+
public static Collection<String> getRequiredJunitRuntimeEclipsePlugins(ILaunchConfiguration configuration) {
6374
org.eclipse.jdt.internal.junit.launcher.ITestKind testKind = org.eclipse.jdt.internal.junit.launcher.JUnitLaunchConfigurationConstants.getTestRunnerKind(configuration);
6475
if (testKind.isNull()) {
65-
return Collections.emptyList();
76+
return List.of();
6677
}
6778
List<String> plugins = new ArrayList<>();
6879
plugins.add("org.eclipse.pde.junit.runtime"); //$NON-NLS-1$
69-
70-
if (org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT4_TEST_KIND_ID.equals(testKind.getId())) {
71-
plugins.add(JUNIT4_RUNTIME_PLUGIN);
72-
} else if (org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT5_TEST_KIND_ID.equals(testKind.getId())) {
73-
plugins.add(JUNIT5_RUNTIME_PLUGIN);
80+
switch (testKind.getId()) {
81+
case org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT3_TEST_KIND_ID -> {
82+
} // Nothing to add for JUnit-3
83+
case org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT4_TEST_KIND_ID -> plugins.add(JUNIT4_JDT_RUNTIME_PLUGIN);
84+
case org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT5_TEST_KIND_ID -> plugins.add(JUNIT4_JDT_RUNTIME_PLUGIN);
85+
default -> throw new IllegalArgumentException("Unsupported junit test kind: " + testKind.getId()); //$NON-NLS-1$
7486
}
7587
return plugins;
7688
}
7789

78-
private static void addPlugins(Collection<String> plugins, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
79-
Set<String> requiredPlugins = new LinkedHashSet<>(plugins);
80-
90+
private static Set<BundleDescription> addAbsentRequirements(Collection<String> requirements, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
8191
Set<BundleDescription> addedRequirements = new LinkedHashSet<>();
82-
addAbsentRequirements(requiredPlugins, addedRequirements, allBundles, allModels);
83-
84-
Set<BundleDescription> requirementsOfRequirements = DependencyManager.findRequirementsClosure(addedRequirements);
85-
addAbsentRequirements(requirementsOfRequirements, allBundles, allModels);
86-
}
87-
88-
private static void addAbsentRequirements(Collection<String> requirements, Set<BundleDescription> addedRequirements, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
8992
for (String id : requirements) {
9093
List<IPluginModelBase> models = allBundles.computeIfAbsent(id, k -> new ArrayList<>());
91-
if (models.stream().noneMatch(m -> m.getBundleDescription().isResolved())) {
92-
IPluginModelBase model = JUnitLaunchRequirements.findRequiredPluginInTargetOrHost(id);
94+
if (models.stream().noneMatch(p -> p.getBundleDescription().isResolved())) {
95+
IPluginModelBase model = findRequiredPluginInTargetOrHost(id, null);
9396
models.add(model);
9497
BundleLauncherHelper.addDefaultStartingBundle(allModels, model);
95-
if (addedRequirements != null) {
96-
addedRequirements.add(model.getBundleDescription());
97-
}
98+
addedRequirements.add(model.getBundleDescription());
9899
}
99100
}
101+
return addedRequirements;
100102
}
101103

102-
private static void addAbsentRequirements(Set<BundleDescription> toAdd, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
103-
for (BundleDescription requirement : toAdd) {
104+
private static void addAbsentRequirements(Set<BundleDescription> requirements, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
105+
for (BundleRevision requirement : requirements) {
104106
String id = requirement.getSymbolicName();
107+
Version version = requirement.getVersion();
105108
List<IPluginModelBase> models = allBundles.computeIfAbsent(id, k -> new ArrayList<>());
106-
boolean replace = !models.isEmpty() && models.stream().noneMatch(m -> m.getBundleDescription().getVersion().equals(requirement.getVersion()));
109+
boolean replace = !models.isEmpty() && models.stream().noneMatch(m -> m.getBundleDescription().getVersion().equals(version));
107110
if (replace || models.stream().noneMatch(m -> m.getBundleDescription().isResolved())) {
108-
IPluginModelBase model = JUnitLaunchRequirements.findRequiredPluginInTargetOrHost(requirement);
111+
IPluginModelBase model = findRequiredPluginInTargetOrHost(requirement.getSymbolicName(), new VersionRange(VersionRange.LEFT_OPEN, version, version, VersionRange.RIGHT_OPEN));
109112
if (replace) {
110113
String startLevel = null;
111114
for (IPluginModelBase m : models) {
112115
startLevel = allModels.remove(m);
113116
}
114117
models.clear();
115-
allModels.put(model, startLevel);
118+
allModels.put(model, startLevel); //TODO: isn't this overwritten? Should it be retained or just ignored?
116119
}
117120
models.add(model);
118121
BundleLauncherHelper.addDefaultStartingBundle(allModels, model);
119122
}
120123
}
121124
}
122125

123-
public static Set<BundleDescription> junit5PlatformRequirements() throws CoreException {
126+
private static Set<BundleDescription> findBundlesInTargetOrHost(List<String> bundleIDs, VersionRange version) throws CoreException {
124127
Set<BundleDescription> descriptions = new LinkedHashSet<>();
125-
for (String id : JUNIT5_RUN_WITH_PLUGINS) {
126-
IPluginModelBase model = findRequiredPluginInTargetOrHost(id, JUNIT5_VERSIONS);
128+
for (String id : bundleIDs) {
129+
IPluginModelBase model = findRequiredPluginInTargetOrHost(id, version);
127130
if (model != null) {
128-
BundleDescription description = model.getBundleDescription();
129-
descriptions.add(description);
131+
descriptions.add(model.getBundleDescription());
130132
}
131133
}
132134
return descriptions;
133135
}
134136

135-
public static IPluginModelBase findRequiredPluginInTargetOrHost(String id) throws CoreException {
136-
return findRequiredPluginInTargetOrHost(id, PluginRegistry.findModel(id));
137-
}
138-
139-
private static IPluginModelBase findRequiredPluginInTargetOrHost(String id, VersionRange versionRange) throws CoreException {
140-
return findRequiredPluginInTargetOrHost(id, PluginRegistry.findModel(id, versionRange));
141-
}
142-
143-
public static IPluginModelBase findRequiredPluginInTargetOrHost(BundleRevision bundleRevision) throws CoreException {
144-
String id = bundleRevision.getSymbolicName();
145-
return findRequiredPluginInTargetOrHost(id, PluginRegistry.findModel(bundleRevision));
146-
}
147-
148-
private static IPluginModelBase findRequiredPluginInTargetOrHost(String id, IPluginModelBase model) throws CoreException {
137+
private static IPluginModelBase findRequiredPluginInTargetOrHost(String id, VersionRange version) throws CoreException {
138+
IPluginModelBase model = version != null ? PluginRegistry.findModel(id, version) : PluginRegistry.findModel(id);
149139
if (model == null || !model.getBundleDescription().isResolved()) {
150140
// prefer bundle from host over unresolved bundle from target
151-
model = PDECore.getDefault().findPluginInHost(id);
141+
model = PDECore.getDefault().findPluginInHost(id, version);
152142
}
153143
if (model == null) {
154-
String message = NLS.bind(PDEMessages.JUnitLaunchConfiguration_error_missingPlugin, id);
155-
Status error = new Status(IStatus.ERROR, IPDEConstants.PLUGIN_ID, IStatus.OK, message, null);
156-
throw new CoreException(error);
144+
throw new CoreException(Status.error(NLS.bind(PDEMessages.JUnitLaunchConfiguration_error_missingPlugin, id)));
157145
}
158146
return model;
159147
}

‎ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/PluginBlock.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ protected void addRequiredPlugins() {
165165
// Check that the application or product we are launching has its requirements included
166166
try {
167167
List<String> requiredIds = RequirementHelper.getApplicationLaunchRequirements(fLaunchConfig);
168-
Collection<String> requiredPlugins = JUnitLaunchRequirements.getRequiredJunitRuntimePlugins(fLaunchConfig);
168+
Collection<String> requiredPlugins = JUnitLaunchRequirements.getRequiredJunitRuntimeEclipsePlugins(fLaunchConfig);
169169
Stream.concat(requiredPlugins.stream(), requiredIds.stream()).forEach(requiredId -> {
170170
// see if launcher plugin is already included
171171
IPluginModelBase base = findPlugin(requiredId);

0 commit comments

Comments
 (0)