Do checks inside the job and compute the classpath entries

Currently some checks are performed outside the update job, for example
the model is fetched and the javanature. These values can change between
submit the data to the job and it actually pick it up.

This now moves these check into the job itself and uses a concurrent
queue to prevent the need for synchronized. Also results are collected
inside a Map so if new update request are received in the meanwhile we
discard the previous computed values.

Beside that, entries are now eagerly computed as they are required
anyways when we set the classpath and we can catch problems that occur
here instead of just swallow them.

Also the job now react to cancel request much better than before.

Author: laeubi

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java

@@ -23,6 +23,8 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Queue;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -35,6 +37,7 @@
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.MultiStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.jdt.core.IClasspathAttribute;
@@ -502,30 +505,17 @@ public static void requestClasspathUpdate(Collection<IProject> updateProjects) {
 		if (updateProjects == null || updateProjects.isEmpty()) {
 			return;
 		}
-		boolean added = false;
-		for (IProject project : updateProjects) {
-			if (project.exists() && project.isOpen()) {
-				IPluginModelBase model = PluginModelManager.getInstance().findModel(project);
-				if (model != null && PluginProject.isJavaProject(project)) {
-					fUpdateJob.add(JavaCore.create(project), new RequiredPluginsClasspathContainer(model, project));
-					added = true;
-				}
-			}
-		}
-		if (added) {
-			fUpdateJob.schedule();
-		}
+		fUpdateJob.addAll(updateProjects);
 	}
 
 	/**
 	 * Job to update class path containers asynchronously. Avoids blocking the
-	 * UI thread while saving the manifest editor. The job is given a workspace
-	 * lock so other jobs can't run on a stale classpath.
+	 * UI thread. The job is given a workspace lock so other jobs can't run on a
+	 * stale classpath.
 	 */
 	private static final class UpdateClasspathsJob extends Job {
 
-		private final List<IJavaProject> fProjects = new ArrayList<>();
-		private final List<IClasspathContainer> fContainers = new ArrayList<>();
+		private final Queue<IProject> workQueue = new ConcurrentLinkedQueue<>();
 
 		/**
 		 * Constructs a new job.
@@ -539,43 +529,77 @@ public UpdateClasspathsJob() {
 
 		@Override
 		public boolean belongsTo(Object family) {
-			return family == PluginModelManager.class;
+			return family == PluginModelManager.class || family == ClasspathComputer.class;
 		}
 
 		@Override
 		protected IStatus run(IProgressMonitor monitor) {
-			try {
-				boolean more = false;
-				do {
-					IJavaProject[] projects = null;
-					IClasspathContainer[] containers = null;
-					synchronized (fProjects) {
-						projects = fProjects.toArray(new IJavaProject[fProjects.size()]);
-						containers = fContainers.toArray(new IClasspathContainer[fContainers.size()]);
-						fProjects.clear();
-						fContainers.clear();
+			PluginModelManager modelManager = PluginModelManager.getInstance();
+			Map<IJavaProject, IClasspathContainer> updateProjects = new LinkedHashMap<>();
+			Map<IProject, IStatus> errorsPerProject = new LinkedHashMap<>();
+			IProject project;
+			while (!monitor.isCanceled() && (project = workQueue.poll()) != null) {
+				if (project.exists() && project.isOpen()) {
+					IPluginModelBase model = modelManager.findModel(project);
+					if (model != null && PluginProject.isJavaProject(project)) {
+						IJavaProject javaProject = JavaCore.create(project);
+						RequiredPluginsClasspathContainer classpathContainer = new RequiredPluginsClasspathContainer(
+								model, project);
+						try {
+							// eager compute the entries as they will be
+							// needed soon, if any error occurs here we do
+							// not update the entry, all errors will be
+							// reported at the end of the update!
+							classpathContainer.computeEntries();
+							updateProjects.put(javaProject, classpathContainer);
+							errorsPerProject.remove(project);
+						} catch (CoreException e) {
+							errorsPerProject.put(project, e.getStatus());
+						}
 					}
-					JavaCore.setClasspathContainer(PDECore.REQUIRED_PLUGINS_CONTAINER_PATH, projects, containers,
-							monitor);
-					synchronized (fProjects) {
-						more = !fProjects.isEmpty();
+				}
+			}
+			if (monitor.isCanceled()) {
+				return Status.CANCEL_STATUS;
+			}
+			if (!updateProjects.isEmpty()) {
+				try {
+					int i = 0;
+					int n = updateProjects.size();
+					IJavaProject[] javaProjects = new IJavaProject[n];
+					IClasspathContainer[] container = new IClasspathContainer[n];
+					for (Entry<IJavaProject, IClasspathContainer> entry : updateProjects.entrySet()) {
+						javaProjects[i] = entry.getKey();
+						container[i] = entry.getValue();
+						i++;
 					}
-				} while (more);
-
-			} catch (JavaModelException e) {
-				return e.getStatus();
+					JavaCore.setClasspathContainer(PDECore.REQUIRED_PLUGINS_CONTAINER_PATH, javaProjects, container,
+							monitor);
+				} catch (JavaModelException e) {
+					return e.getStatus();
+				}
+			}
+			IStatus[] errors = errorsPerProject.values().toArray(IStatus[]::new);
+			if (errors.length == 0) {
+				return Status.OK_STATUS;
 			}
-			return Status.OK_STATUS;
+			if (errors.length == 1) {
+				return errors[0];
+			}
+			MultiStatus overallStatus = new MultiStatus(ClasspathComputer.class, 0,
+					PDECoreMessages.ClasspathComputer_failed);
+			for (IStatus status : errors) {
+				overallStatus.add(status);
+			}
+			return overallStatus;
 		}
 
 		/**
 		 * Queues more projects/containers.
 		 */
-		void add(IJavaProject project, IClasspathContainer container) {
-			synchronized (fProjects) {
-				fProjects.add(project);
-				fContainers.add(container);
-			}
+		void addAll(Collection<IProject> tocheck) {
+			workQueue.addAll(tocheck);
+			schedule();
 		}
 
 	}

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/PDECoreMessages.java

@@ -281,6 +281,8 @@ public class PDECoreMessages extends NLS {
 
 	public static String BuildErrorReporter_SourceCompatibility;
 
+	public static String ClasspathComputer_failed;
+
 	public static String ClasspathHelper_BadFileLocation;
 
 	public static String ConvertSchemaToHTML_CannotFindIncludedSchema;

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/RequiredPluginsClasspathContainer.java

@@ -36,7 +36,6 @@
 import org.eclipse.core.resources.IFile;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IResource;
-import org.eclipse.core.resources.IWorkspaceRoot;
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IConfigurationElement;
 import org.eclipse.core.runtime.IExtensionRegistry;
@@ -131,6 +130,14 @@ public String getDescription() {
 
 	@Override
 	public IClasspathEntry[] getClasspathEntries() {
+		try {
+			return computeEntries();
+		} catch (CoreException e) {
+			return new IClasspathEntry[0];
+		}
+	}
+
+	IClasspathEntry[] computeEntries() throws CoreException {
 		if (fEntries == null) {
 			if (fModel == null) {
 				fEntries = computePluginEntriesByProject();
@@ -215,9 +222,8 @@ protected void debugProblems(Project bnd) {
 		}
 	}
 
-	private List<IClasspathEntry> computePluginEntriesByModel() {
+	private List<IClasspathEntry> computePluginEntriesByModel() throws CoreException {
 		List<IClasspathEntry> entries = new ArrayList<>();
-		try {
 			BundleDescription desc = fModel.getBundleDescription();
 			if (desc == null) {
 				return List.of();
@@ -278,8 +284,6 @@ private List<IClasspathEntry> computePluginEntriesByModel() {
 			addJunit5RuntimeDependencies(added, entries);
 			addImplicitDependencies(desc, added, entries);
 
-		} catch (CoreException e) {
-		}
 		return entries;
 	}
 
@@ -728,24 +732,4 @@ private void addExtraLibrary(IPath path, IPluginModelBase model, List<IClasspath
 		}
 	}
 
-	/**
-	 * Tries to compute a full set of bundle dependencies, including not
-	 * exported bundle dependencies and bundles contributing packages possibly
-	 * imported by any of bundles in the dependency graph.
-	 *
-	 * @return never null, but possibly empty project list which all projects in
-	 *         the workspace this container depends on, directly or indirectly.
-	 */
-	public List<IProject> getAllProjectDependencies() {
-		IWorkspaceRoot root = PDECore.getWorkspace().getRoot();
-		try {
-			addImportedPackages = true;
-			return computePluginEntriesByModel().stream()
-					.filter(cpe -> cpe.getEntryKind() == IClasspathEntry.CPE_PROJECT)
-					.map(cpe -> cpe.getPath().lastSegment()).map(root::getProject) //
-					.filter(IProject::exists).toList();
-		} finally {
-			addImportedPackages = false;
-		}
-	}
 }

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/pderesources.properties

@@ -222,6 +222,7 @@ FeatureExportOperation_runningPackagerScript=Running packager script
 FeatureExportOperation_workspaceBuildErrorsFoundDuringExport=Export completed successfully, but build problems were detected in the following required projects: {0}
 FeatureModelManager_initializingFeatureTargetPlatform=Initializing feature from target platform
 BaseExportTask_pdeExport=PDE Export
+ClasspathComputer_failed=Classpath Update failed
 ClasspathHelper_BadFileLocation=Could not determine absolute location of file: {0}
 ConvertSchemaToHTML_CannotFindIncludedSchema=Cannot find included schema ''{0}'' required by parent schema ''{1}''
 ConvertSchemaToHTML_InvalidAdditionalSearchPath=Invalid path found in additional search paths: {0}