Refactor UpdateClassPathJob to provide a more general interface

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ProjectUtils.java

@@ -314,26 +314,21 @@ private static IClasspathEntry removeFilters(IClasspathEntry entry, IPath path)
 		}
 	}
 
-	public static void updateBinaries(IJavaProject javaProject, IPath libFolderPath, IProgressMonitor monitor) throws CoreException {
-		updateBinaries(javaProject, Collections.singleton(libFolderPath), monitor);
-	}
-
-	public static void updateBinaries(IJavaProject javaProject, Set<IPath> libFolderPaths, IProgressMonitor monitor) throws CoreException {
-		Set<Path> binaries = collectBinaries(libFolderPaths, monitor);
+	public static void updateBinaries(IJavaProject javaProject, Map<String, IPath> libraries, IProgressMonitor monitor) throws CoreException {
 		if (monitor.isCanceled()) {
 			return;
 		}
 		IClasspathEntry[] rawClasspath = javaProject.getRawClasspath();
 		List<IClasspathEntry> newEntries = Arrays.stream(rawClasspath).filter(cpe -> cpe.getEntryKind() != IClasspathEntry.CPE_LIBRARY).collect(Collectors.toCollection(ArrayList::new));
 
-		for (Path file : binaries) {
+		for (Map.Entry<String, IPath> library : libraries.entrySet()) {
 			if (monitor.isCanceled()) {
 				return;
 			}
-			IPath newLibPath = new org.eclipse.core.runtime.Path(file.toString());
-			IPath sourcePath = detectSources(file);
-			IClasspathEntry newEntry = JavaCore.newLibraryEntry(newLibPath, sourcePath, null);
-			JavaLanguageServerPlugin.logInfo("Adding " + newLibPath + " to the classpath");
+			IPath binary = new org.eclipse.core.runtime.Path(library.getKey());
+			IPath source = library.getValue();
+			IClasspathEntry newEntry = JavaCore.newLibraryEntry(binary, source, null);
+			JavaLanguageServerPlugin.logInfo("Adding " + binary + " to the classpath");
 			newEntries.add(newEntry);
 		}
 		IClasspathEntry[] newClasspath = newEntries.toArray(new IClasspathEntry[newEntries.size()]);
@@ -342,7 +337,7 @@ public static void updateBinaries(IJavaProject javaProject, Set<IPath> libFolder
 		}
 	}
 
-	private static Set<Path> collectBinaries(Set<IPath> libFolderPaths, IProgressMonitor monitor) throws CoreException {
+	public static Set<Path> collectBinaries(Set<IPath> libFolderPaths, IProgressMonitor monitor) throws CoreException {
 		Set<Path> binaries = new LinkedHashSet<>();
 		FileVisitor<? super Path> jarDetector = new SimpleFileVisitor<Path>() {
 			@Override
@@ -372,6 +367,14 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IO
 		return binaries;
 	}
 
+	public static IPath detectSources(Path file) {
+		String filename = file.getFileName().toString();
+		//better approach would be to (also) resolve sources using Maven central, or anything smarter really
+		String sourceName = filename.substring(0, filename.lastIndexOf(JAR_SUFFIX)) + SOURCE_JAR_SUFFIX;
+		Path sourcePath = file.getParent().resolve(sourceName);
+		return Files.isRegularFile(sourcePath) ? new org.eclipse.core.runtime.Path(sourcePath.toString()) : null;
+	}
+
 	private static boolean isBinary(Path file) {
 		String fileName = file.getFileName().toString();
 		return (fileName.endsWith(JAR_SUFFIX)
@@ -380,14 +383,6 @@ private static boolean isBinary(Path file) {
 				&& !fileName.endsWith(SOURCE_JAR_SUFFIX));
 	}
 
-	private static IPath detectSources(Path file) {
-		String filename = file.getFileName().toString();
-		//better approach would be to (also) resolve sources using Maven central, or anything smarter really
-		String sourceName = filename.substring(0, filename.lastIndexOf(JAR_SUFFIX)) + SOURCE_JAR_SUFFIX;
-		Path sourcePath = file.getParent().resolve(sourceName);
-		return Files.isRegularFile(sourcePath) ? new org.eclipse.core.runtime.Path(sourcePath.toString()) : null;
-	}
-
 	public static void removeJavaNatureAndBuilder(IProject project, IProgressMonitor monitor) throws CoreException {
 		if (project != null && project.isAccessible() && ProjectUtils.isJavaProject(project)) {
 			IProjectDescription description = project.getDescription();

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectBuildSupport.java

@@ -46,7 +46,7 @@ public boolean fileChanged(IResource resource, CHANGE_TYPE changeType, IProgress
 		if (realFolderPath != null) {
 			IPath libFolderPath = realFolderPath.append(LIB_FOLDER);
 			if (libFolderPath.isPrefixOf(resource.getLocation())) {
-				UpdateClasspathJob.getInstance().updateClasspath(JavaCore.create(invisibleProject), libFolderPath);
+				UpdateClasspathJob.getInstance().updateClasspath(JavaCore.create(invisibleProject), libFolderPath, monitor);
 			}
 		}
 		return false;

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectImporter.java

@@ -109,7 +109,7 @@ public void importToWorkspace(IProgressMonitor monitor) throws OperationCanceled
 			}
 		}
 		IJavaProject javaProject = JavaCore.create(invisibleProject);
-		ProjectUtils.updateBinaries(javaProject, rootPath.append(libFolder), monitor);
+		UpdateClasspathJob.getInstance().updateClasspath(javaProject, rootPath.append(libFolder), monitor);
 	}
 
 	@Override

org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/managers/UpdateClasspathJob.java

@@ -10,12 +10,19 @@
  *******************************************************************************/
 package org.eclipse.jdt.ls.core.internal.managers;
 
+import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import org.eclipse.core.resources.WorkspaceJob;
@@ -49,13 +56,25 @@ public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException {
 			requests = new ArrayList<>(this.queue);
 			this.queue.clear();
 		}
-		Map<IJavaProject, Set<IPath>> reqPerProject = requests.stream().collect(Collectors.groupingBy(UpdateClasspathRequest::getProject, Collectors.mapping(UpdateClasspathRequest::getLibPath, Collectors.toSet())));
-
-		for (Map.Entry<IJavaProject, Set<IPath>> reqs : reqPerProject.entrySet()) {
+		Map<IJavaProject, Optional<UpdateClasspathRequest>> mergedRequestPerProject = requests.stream().collect(
+			Collectors.groupingBy(UpdateClasspathRequest::getProject,
+				Collectors.reducing((mergedRequest, request) -> {
+					mergedRequest.getInclude().addAll(request.getInclude());
+					mergedRequest.getExclude().addAll(request.getExclude());
+					mergedRequest.getSources().putAll(request.getSources());
+					return mergedRequest;
+				})
+			)
+		);
+		for (Map.Entry<IJavaProject, Optional<UpdateClasspathRequest>> entry : mergedRequestPerProject.entrySet()) {
 			if (monitor.isCanceled()) {
 				return Status.CANCEL_STATUS;
 			}
-			updateClasspath(reqs.getKey(), reqs.getValue(), monitor);
+			if (entry.getValue() != null) {
+				final IJavaProject project = entry.getKey();
+				final UpdateClasspathRequest request = entry.getValue().get();
+				updateClasspath(project, request.include, request.exclude, request.sources, monitor);
+			}
 		}
 		synchronized (queue) {
 			if (!queue.isEmpty()) {
@@ -65,18 +84,43 @@ public IStatus runInWorkspace(IProgressMonitor monitor) throws CoreException {
 		return Status.OK_STATUS;
 	}
 
-	private void updateClasspath(IJavaProject project, Set<IPath> libFolders, IProgressMonitor monitor) throws CoreException {
-		ProjectUtils.updateBinaries(project, libFolders, monitor);
+	private void updateClasspath(IJavaProject project, Set<String> include, Set<String> exclude, Map<String, IPath> sources, IProgressMonitor monitor) throws CoreException {
+		final Map<String, IPath> libraries = new HashMap<>();
+		for (final String binary: include) {
+			if (exclude.contains(binary)) {
+				continue;
+			}
+			libraries.put(binary, sources.get(binary));
+		}
+		ProjectUtils.updateBinaries(project, libraries, monitor);
 	}
 
-	public void updateClasspath(IJavaProject project, IPath libPath) {
-		if (project == null || libPath == null) {
+	public void updateClasspath(IJavaProject project, Collection<String> include, Collection<String> exclude, Map<String, IPath> sources) {
+		if (project == null || include == null) {
 			return;
 		}
-		queue(new UpdateClasspathRequest(project, libPath));
+		if (exclude == null) {
+			exclude = new ArrayList<>();
+		}
+		if (sources == null) {
+			sources = new HashMap<>();
+		}
+		queue(new UpdateClasspathRequest(project, new HashSet<>(include), new HashSet<>(exclude), sources));
 		schedule(SCHEDULE_DELAY);
 	}
 
+	public void updateClasspath(IJavaProject project, IPath libFolderPath, IProgressMonitor monitor) throws CoreException {
+		final Set<Path> binaries = ProjectUtils.collectBinaries(Collections.singleton(libFolderPath), monitor);
+		final Set<String> include = new HashSet<>();
+		final Map<String, IPath> sources = new HashMap<>();
+		for (final Path binary: binaries) {
+			final IPath source = ProjectUtils.detectSources(binary);
+			include.add(binary.toString());
+			sources.put(binary.toString(), source);
+		}
+		updateClasspath(project, include, null, sources);
+	}
+
 	void update(UpdateClasspathRequest updateRequest) {
 		queue(updateRequest);
 		schedule(SCHEDULE_DELAY);
@@ -90,20 +134,36 @@ void queue(UpdateClasspathRequest updateRequest) {
 
 	static class UpdateClasspathRequest {
 		private IJavaProject project;
-		private IPath libPath;
+		private Set<String> include;
+		private Set<String> exclude;
+		private Map<String, IPath> sources;
 
-		UpdateClasspathRequest(IJavaProject project, IPath libPath) {
+		UpdateClasspathRequest(IJavaProject project, Set<String> include, Set<String> exclude, Map<String, IPath> sources) {
 			this.project = project;
-			this.libPath = libPath;
+			this.include = include;
+			this.exclude = exclude;
+			this.sources = sources;
 		}
 
-		IPath getLibPath() {
-			return libPath;
+		IJavaProject getProject() {
+			return project;
+		}
+
+		Set<String> getInclude() {
+			return include;
+		}
+
+		Set<String> getExclude() {
+			return exclude;
+		}
+
+		Map<String, IPath> getSources() {
+			return sources;
 		}
 
 		@Override
 		public int hashCode() {
-			return Objects.hash(libPath, project);
+			return Objects.hash(include, exclude, sources, project);
 		}
 
 		@Override
@@ -118,11 +178,10 @@ public boolean equals(Object obj) {
 				return false;
 			}
 			UpdateClasspathRequest other = (UpdateClasspathRequest) obj;
-			return Objects.equals(libPath, other.libPath) && Objects.equals(project, other.project);
-		}
-
-		IJavaProject getProject() {
-			return project;
+			return Objects.equals(project, other.project)
+				&& Objects.equals(include, other.include)
+				&& Objects.equals(exclude, other.exclude)
+				&& Objects.equals(sources, other.sources);
 		}
 
 	}

org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/managers/InvisibleProjectBuildSupportTest.java

@@ -16,10 +16,16 @@
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
+import org.apache.commons.io.FileUtils;
 import org.eclipse.core.resources.IMarker;
 import org.eclipse.core.resources.IProject;
+import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.jobs.IJobChangeEvent;
 import org.eclipse.core.runtime.jobs.IJobChangeListener;
 import org.eclipse.core.runtime.jobs.Job;
@@ -139,4 +145,78 @@ public void scheduled(IJobChangeEvent event) {
 
 	}
 
+	@Test
+	public void testReferencedBinariesUpdate() throws Exception {
+		File projectFolder = createSourceFolderWithMissingLibs("dynamicLibDetection");
+		IProject project = importRootFolder(projectFolder, "Test.java");
+		List<IMarker> errors = ResourceUtils.getErrorMarkers(project);
+		assertEquals("Unexpected errors " + ResourceUtils.toString(errors), 2, errors.size());
+
+		File originBinary = new File(getSourceProjectDirectory(), "eclipse/source-attachment/foo.jar");
+		File originSource = new File(getSourceProjectDirectory(), "eclipse/source-attachment/foo-sources.jar");
+
+		Set<String> include = new HashSet<>();
+		Set<String> exclude = new HashSet<>();
+		Map<String, IPath> sources = new HashMap<>();
+
+		// Include following jars (by lib/** detection)
+		// - /lib/foo.jar
+		// - /lib/foo-sources.jar
+		File libFolder = Files.createDirectories(projectFolder.toPath().resolve(InvisibleProjectBuildSupport.LIB_FOLDER)).toFile();
+		File fooBinary = new File(libFolder, "foo.jar");
+		File fooSource = new File(libFolder, "foo-source.jar");
+		FileUtils.copyFile(originBinary, fooBinary);
+		FileUtils.copyFile(originSource, fooSource);
+
+		// Include following jars (by manually add include)
+		// - /bar.jar
+		// - /library/bar-src.jar
+		File libraryFolder = Files.createDirectories(projectFolder.toPath().resolve("library")).toFile();
+		File barBinary = new File(projectFolder, "bar.jar");
+		File barSource = new File(libraryFolder, "bar-src.jar");
+		FileUtils.copyFile(originBinary, barBinary);
+		FileUtils.copyFile(originSource, barSource);
+		include.add(barBinary.toString());
+		sources.put(barBinary.toString(), new org.eclipse.core.runtime.Path(barSource.toString()));
+
+		// Exclude following jars (by manually add exclude)
+		// - /lib/foo.jar
+		exclude.add(fooBinary.toString());
+
+		// Before sending requests
+		IJavaProject javaProject = JavaCore.create(project);
+		int[] jobInvocations = new int[1];
+		IJobChangeListener listener = new JobChangeAdapter() {
+			@Override
+			public void scheduled(IJobChangeEvent event) {
+				if (event.getJob() instanceof UpdateClasspathJob) {
+					jobInvocations[0] = jobInvocations[0] + 1;
+				}
+			}
+		};
+
+		try { // Send two update request concurrently
+			Job.getJobManager().addJobChangeListener(listener);
+			projectsManager.fileChanged(fooBinary.toString(), CHANGE_TYPE.CREATED); // Request sent by jdt.ls's lib detection
+			UpdateClasspathJob.getInstance().updateClasspath(javaProject, include, exclude, sources); // Request sent by third-party client
+			waitForBackgroundJobs();
+			assertEquals("Update classpath job should have been invoked once", 1, jobInvocations[0]);
+		} finally {
+			Job.getJobManager().removeJobChangeListener(listener);
+		}
+
+		{
+			// The requests sent by `jdt'ls lib detection` and `third-party client` is merged in queue,
+			// So client's `exclude: lib/foo.jar` comes into effect to block jdt.ls's `include: lib/foo.jar`
+			// This is the way client uses to neutralize the effect of jdt.ls's lib detection
+			IClasspathEntry[] classpath = javaProject.getRawClasspath();
+			// Check only one jar file is added to classpath (foo.jar is excluded)
+			assertEquals("Unexpected classpath:\n" + JavaProjectHelper.toString(classpath), 3, classpath.length);
+			// Check the only added jar is bar.jar
+			assertEquals("bar.jar", classpath[2].getPath().lastSegment());
+			assertEquals("bar-src.jar", classpath[2].getSourceAttachmentPath().lastSegment());
+			// Check the source of bar.jar is in /library folder
+			assertEquals("library", classpath[2].getSourceAttachmentPath().removeLastSegments(1).lastSegment());
+		}
+	}
 }