diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/META-INF/MANIFEST.MF b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/META-INF/MANIFEST.MF index 67e0bd272..c9a9596f6 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/META-INF/MANIFEST.MF +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/META-INF/MANIFEST.MF @@ -12,6 +12,5 @@ Require-Bundle: org.springframework.tooling.jdt.ls.commons, org.apache.commons.lang3, org.junit, org.apache.commons.io, - org.eclipse.swt, com.google.guava;bundle-version="21.0.0" diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/test/ClasspathListenerHandlerTest.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/test/ClasspathListenerHandlerTest.java index 6eb7de143..d947b31dd 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/test/ClasspathListenerHandlerTest.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/test/ClasspathListenerHandlerTest.java @@ -46,6 +46,7 @@ import org.junit.rules.TemporaryFolder; import org.springframework.tooling.jdt.ls.commons.Logger.TestLogger; import org.springframework.tooling.jdt.ls.commons.classpath.Classpath; import org.springframework.tooling.jdt.ls.commons.classpath.Classpath.CPE; +import org.springframework.tooling.jdt.ls.commons.test.ClasspathListenerHandlerTest.MockClientCommandExecutor; import org.springframework.tooling.jdt.ls.commons.classpath.ClientCommandExecutor; import org.springframework.tooling.jdt.ls.commons.classpath.ReusableClasspathListenerHandler; import org.springsource.ide.eclipse.commons.frameworks.test.util.ACondition; @@ -53,13 +54,15 @@ import org.springsource.ide.eclipse.commons.frameworks.test.util.Asserter; import com.google.common.collect.ImmutableList; +import junit.framework.Assert; import junit.framework.AssertionFailedError; public class ClasspathListenerHandlerTest { private TestLogger logger = new TestLogger(); + private MockClientCommandExecutor commandExecutor = new MockClientCommandExecutor(); private MockClasspathCache classpaths = new MockClasspathCache(); - private ReusableClasspathListenerHandler service = new ReusableClasspathListenerHandler(logger, classpaths); + private ReusableClasspathListenerHandler service = new ReusableClasspathListenerHandler(logger, commandExecutor); @Test public void classpathIsSentForExistingProject() throws Exception { String projectName = "classpath-test-simple-java-project"; @@ -75,7 +78,7 @@ public class ClasspathListenerHandlerTest { } - @Ignore //TODO: fails randomly for unknown reason. + //@Ignore //TODO: fails randomly for unknown reason. @Test public void classpathIsSentForNewProject_and_removedForDeletedProject() throws Exception { service.addClasspathListener(classpaths.commandId); @@ -95,7 +98,39 @@ public class ClasspathListenerHandlerTest { }); } - @Ignore //TODO: fails randomly for unknown reason. + @Test public void classpathIsRemovedWhenProjectDeletedFromFileSystem_twoSubscribers() throws Exception { + MockClasspathCache secondListener = new MockClasspathCache(); + try { + String projectName = "classpath-test-simple-java-project"; + IProject project = createTestProject(projectName); + File loc = project.getLocation().toFile(); + + service.addClasspathListener(classpaths.commandId); + ACondition.waitFor("Project with classpath to appear", Duration.ofSeconds(5), () -> { + Classpath cp = classpaths.getFor(loc).classpath; + assertTrue(cp.getEntries().stream().filter(cpe -> Classpath.isSource(cpe)).count()==1); //has 1 source entry + assertClasspath(cp, cp.getEntries().stream().filter(cpe -> Classpath.isBinary(cpe) && cpe.isSystem()).count()>=1); //has some system libraries + }); + + FileUtils.deleteQuietly(loc); + safe(() -> project.refreshLocal(IResource.DEPTH_INFINITE, null)); + + ACondition.waitFor("Project to disapear", Duration.ofSeconds(5), () -> { + { + Info cp = classpaths.getFor(loc); + assertNull(cp); + } + { + Info cp = secondListener.getFor(loc); + assertNull(cp); + } + }); + } finally { + secondListener.dispose(); + } + } + + //@Ignore //TODO: fails randomly for unknown reason. @Test public void classpathIsRemovedWhenProjectDeletedFromFileSystem() throws Exception { String projectName = "classpath-test-simple-java-project"; IProject project = createTestProject(projectName); @@ -117,7 +152,7 @@ public class ClasspathListenerHandlerTest { }); } - @Ignore //TODO: why is this failing in CI builds but passing locally? + //@Ignore //TODO: why is this failing in CI builds but passing locally? @Test public void sourceJar() throws Exception { String projectName = "maven-with-jar-dependency"; IProject project = createTestProject(projectName); @@ -151,12 +186,39 @@ public class ClasspathListenerHandlerTest { } } + public class MockClientCommandExecutor implements ClientCommandExecutor { + + private Map handlers = new HashMap<>(); + + @Override + public synchronized Object executeClientCommand(String id, Object... params) throws Exception { + ClientCommandExecutor handler = handlers.get(id); + if (handler!=null) { + return handler.executeClientCommand(id, params); + } + throw new IllegalStateException("No handler for "+id); + } + + public void addHandler(String id, ClientCommandExecutor handler) { + Assert.assertFalse(handlers.containsKey(id)); + handlers.put(id, handler); + } + + public void removeHandler(String commandId) { + handlers.remove(commandId); + } + } + public class MockClasspathCache implements ClientCommandExecutor { String commandId = RandomStringUtils.randomAlphabetic(8); Map classpaths = new HashMap<>(); + { + commandExecutor.addHandler(commandId, this); + } + @Override public synchronized Object executeClientCommand(String id, Object... _params) throws Exception { if (id.equals(commandId)) { @@ -183,6 +245,7 @@ public class ClasspathListenerHandlerTest { public void dispose() throws Exception { service.removeClasspathListener(commandId); + commandExecutor.removeHandler(commandId); } } diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springsource/ide/eclipse/commons/frameworks/test/util/ACondition.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springsource/ide/eclipse/commons/frameworks/test/util/ACondition.java index a7da3ccb3..ef7a40b36 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springsource/ide/eclipse/commons/frameworks/test/util/ACondition.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springsource/ide/eclipse/commons/frameworks/test/util/ACondition.java @@ -15,7 +15,6 @@ import java.util.Map; import org.eclipse.core.runtime.jobs.IJobManager; import org.eclipse.core.runtime.jobs.Job; -import org.eclipse.swt.widgets.Display; import junit.framework.AssertionFailedError; @@ -223,29 +222,29 @@ public abstract class ACondition { * but such calls have no effect. */ public static void waitForDisplay() { - if (inUIThread()) { - try { - while (Display.getDefault().readAndDispatch()) { - // do nothing - } - } catch (Throwable e) { - // in e 44 this is throwing exceptions... a lot. Log them in case they contain - // some valuable hints... but move along. These errors happen because some component - // probably unrelated to our tests misbehaved. I suspect GTK3 may be causing - // NPEs and other errors in the Eclipse UI code, for example. These errors seem - // to propagate out of the 'readAndDispatch' call on e44. - e.printStackTrace(); - } - } +// if (inUIThread()) { +// try { +// while (Display.getDefault().readAndDispatch()) { +// // do nothing +// } +// } catch (Throwable e) { +// // in e 44 this is throwing exceptions... a lot. Log them in case they contain +// // some valuable hints... but move along. These errors happen because some component +// // probably unrelated to our tests misbehaved. I suspect GTK3 may be causing +// // NPEs and other errors in the Eclipse UI code, for example. These errors seem +// // to propagate out of the 'readAndDispatch' call on e44. +// e.printStackTrace(); +// } +// } } public static boolean inUIThread() { - try { - return Display.getDefault().getThread() == Thread.currentThread(); - } catch (Exception e) { - e.printStackTrace(); +// try { +// return Display.getDefault().getThread() == Thread.currentThread(); +// } catch (Exception e) { +// e.printStackTrace(); return false; - } +// } } public static StringBuffer getStackDumps() { diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ClasspathListenerManager.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ClasspathListenerManager.java index e8ec4c53b..957ec2775 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ClasspathListenerManager.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ClasspathListenerManager.java @@ -87,44 +87,11 @@ public class ClasspathListenerManager { private MyListener myListener; private final Logger logger; - /** - * @param initialEvent If true, events are fired immediately on all existing java - * projects, treating the connection of the listener itself as a change event. - * This allows clients to become aware of all classpaths from the start and - * continually monitor them for changes from that point onward. - */ - public ClasspathListenerManager(Logger logger, ClasspathListener listener, boolean initialEvent, Supplier> projectSorterFactory) { + public ClasspathListenerManager(Logger logger, ClasspathListener listener) { this.logger = logger; logger.log("Setting up ClasspathListenerManager"); this.listener = listener; JavaCore.addElementChangedListener(myListener=new MyListener(), ElementChangedEvent.POST_CHANGE); - if (initialEvent) { - logger.log("Sending initial event for all projects ..."); - - IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects(); - if (projectSorterFactory != null) { - Arrays.sort(projects, projectSorterFactory.get()); - } - - for (IProject p : projects) { - logger.log("project "+p.getName() +" ..." ); - try { - if (p.isAccessible() && p.hasNature(JavaCore.NATURE_ID)) { - IJavaProject jp = JavaCore.create(p); - listener.classpathChanged(jp); - } else { - logger.log("project "+p.getName() +" SKIPPED" ); - } - } catch (CoreException e) { - logger.log(e); - } - } - logger.log("Sending initial event for all projects DONE"); - } - } - - public ClasspathListenerManager(Logger logger, ClasspathListener listener) { - this(logger, listener, false, null); } public void dispose() { diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java index 571afe253..d2935bc66 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/ReusableClasspathListenerHandler.java @@ -12,17 +12,25 @@ package org.springframework.tooling.jdt.ls.commons.classpath; import java.io.File; import java.net.URI; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; import org.eclipse.core.resources.IProject; +import org.eclipse.core.resources.ResourcesPlugin; +import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.jdt.core.JavaCore; import org.springframework.tooling.jdt.ls.commons.Logger; import org.springframework.tooling.jdt.ls.commons.classpath.ClasspathListenerManager.ClasspathListener; @@ -74,7 +82,7 @@ public class ReusableClasspathListenerHandler { URI loc = getProjectLocation(jp); if (loc!=null) { File f = new File(loc); - return f.isDirectory(); + return f.isDirectory() && jp.getProject().hasNature(JavaCore.NATURE_ID); } } catch (Exception e) { //Something bogus about this project... so just pretend it doesn't exist. @@ -86,23 +94,51 @@ public class ReusableClasspathListenerHandler { class Subscriptions { - private Map subscribers = null; + private Set subscribers = null; + private ClasspathListenerManager classpathListener = null; public synchronized void subscribe(String callbackCommandId) { logger.log("subscribing to classpath changes: " + callbackCommandId); if (subscribers==null) { - subscribers = new HashMap<>(1); + //First subscriber + subscribers = new HashSet<>(); + classpathListener = new ClasspathListenerManager(logger, new ClasspathListener() { + @Override + public void classpathChanged(IJavaProject jp) { + sendNotification(jp, subscribers); + } + }); } - subscribers.computeIfAbsent(callbackCommandId, (cid) -> new ClasspathListenerManager(logger, new ClasspathListener() { - @Override - public void classpathChanged(IJavaProject jp) { - sendNotification(callbackCommandId, jp); + subscribers.add(callbackCommandId); + logger.log("subsribers = " + subscribers); + sendInitialEvents(callbackCommandId); + } + + private void sendInitialEvents(String callbackCommandId) { + logger.log("Sending initial event for all projects ..."); + + IProject[] projects = ResourcesPlugin.getWorkspace().getRoot().getProjects(); + if (projectSorterFactory != null) { + Arrays.sort(projects, projectSorterFactory.get()); + } + + for (IProject p : projects) { + logger.log("project "+p.getName() +" ..." ); + try { + if (p.isAccessible() && p.hasNature(JavaCore.NATURE_ID)) { + IJavaProject jp = JavaCore.create(p); + sendNotification(jp, Collections.singleton(callbackCommandId)); + } else { + logger.log("project "+p.getName() +" SKIPPED" ); + } + } catch (CoreException e) { + logger.log(e); } - }, true, projectSorterFactory)); - logger.log("subsribers = " + subscribers.keySet()); + } + logger.log("Sending initial event for all projects DONE"); } - private void sendNotification(String callbackCommandId, IJavaProject jp) { + private void sendNotification(IJavaProject jp, Collection callbackIds) { //TODO: make one Job to accumulate all requested notification and work more efficiently by batching // and avoiding multiple executions of duplicated requests. Job job = new Job("SendClasspath notification") { @@ -116,7 +152,7 @@ public class ReusableClasspathListenerHandler { logger.log("Could not send event for project because no project location: "+jp.getElementName()); } else { boolean exsits = projectExists(jp); - boolean open = true; // WARNING: calling is jp.isOpen is unreliable and subject to race condition. After a POST_CHAGE project open event + boolean open = true; // WARNING: calling jp.isOpen is unreliable and subject to race condition. After a POST_CHAGE project open event // this should be true but it typically is not unless you wait for some time. No idea how you would know // how long you should wait (200ms is not enough, and that seems pretty long). Isn't it kind of the point // for a 'POST_CHANGE' event to come **after** model has already changed? I guess not in Eclipse. @@ -128,7 +164,7 @@ public class ReusableClasspathListenerHandler { Classpath classpath = Classpath.EMPTY; if (deleted) { - projectLocations.remove(projectName); + // projectLocations.remove(projectName); } else { projectLocations.put(projectName, projectLoc); try { @@ -137,13 +173,15 @@ public class ReusableClasspathListenerHandler { logger.log(e); } } - try { - logger.log("executing callback "+callbackCommandId+" "+projectName+" "+deleted+" "+ classpath.getEntries().size()); - Object r = conn.executeClientCommand(callbackCommandId, projectLoc.toString(), projectName, deleted, classpath); - logger.log("executing callback "+callbackCommandId+" SUCCESS ["+r+"]"); - } catch (Exception e) { - logger.log("executing callback "+callbackCommandId+" FAILED"); - logger.log(e); + for (String callbackCommandId : callbackIds) { + try { + logger.log("executing callback "+callbackCommandId+" "+projectName+" "+deleted+" "+ classpath.getEntries().size()); + Object r = conn.executeClientCommand(callbackCommandId, projectLoc.toString(), projectName, deleted, classpath); + logger.log("executing callback "+callbackCommandId+" SUCCESS ["+r+"]"); + } catch (Exception e) { + logger.log("executing callback "+callbackCommandId+" FAILED"); + logger.log(e); + } } } } catch (Exception e) { @@ -159,15 +197,16 @@ public class ReusableClasspathListenerHandler { public synchronized void unsubscribe(String callbackCommandId) { logger.log("unsubscribing from classpath changes: " + callbackCommandId); if (subscribers != null) { - ClasspathListenerManager mgr = subscribers.remove(callbackCommandId); - if (mgr!=null) { - mgr.dispose(); - } + subscribers.remove(callbackCommandId); if (subscribers.isEmpty()) { subscribers = null; + if (classpathListener!=null) { + classpathListener.dispose(); + classpathListener = null; + } } } - logger.log("subsribers = " + (subscribers == null ? "null" : subscribers.keySet())); + logger.log("subsribers = " + subscribers); } public boolean isEmpty() { diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/target-platform.target b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/target-platform.target deleted file mode 100644 index 6379d953d..000000000 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.extension/target-platform.target +++ /dev/null @@ -1,55 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -