From 6a5a023557dda7141734761fdbec7afc0920e9c9 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 4 May 2018 14:19:20 -0700 Subject: [PATCH] Handle project create and delete in jdt.ls classpath service --- .../ClasspathListenerHandlerTest.java | 103 +++++++++--- .../classpath/ClasspathListenerManager.java | 154 ++++++++++-------- .../ReusableClasspathListenerHandler.java | 86 +++++++--- 3 files changed, 226 insertions(+), 117 deletions(-) rename headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/{ => test}/ClasspathListenerHandlerTest.java (59%) diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/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 similarity index 59% rename from headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/ClasspathListenerHandlerTest.java rename to headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/test/ClasspathListenerHandlerTest.java index 68e886551..5910ec0b3 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons.test/src/org/springframework/tooling/jdt/ls/commons/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 @@ -8,8 +8,9 @@ * Contributors: * Pivotal, Inc. - initial API and implementation *******************************************************************************/ -package org.springframework.tooling.jdt.ls.commons; +package org.springframework.tooling.jdt.ls.commons.test; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.io.File; @@ -18,17 +19,27 @@ import java.time.Duration; import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.CompletableFuture; import org.apache.commons.lang3.RandomStringUtils; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IProjectDescription; +import org.eclipse.core.resources.IResource; +import org.eclipse.core.resources.IncrementalProjectBuilder; import org.eclipse.core.resources.ResourcesPlugin; +import org.eclipse.core.resources.WorkspaceJob; +import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.FileLocator; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.NullProgressMonitor; import org.eclipse.core.runtime.Path; import org.eclipse.core.runtime.Platform; +import org.eclipse.core.runtime.Status; import org.eclipse.jdt.core.JavaCore; import org.junit.After; import org.junit.Test; +import org.springframework.tooling.jdt.ls.commons.Logger; 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.classpath.ClientCommandExecutor; @@ -38,7 +49,46 @@ import org.springsource.ide.eclipse.commons.frameworks.test.util.ACondition; import junit.framework.AssertionFailedError; public class ClasspathListenerHandlerTest { + + private MockClasspathCache classpaths = new MockClasspathCache(); + private ReusableClasspathListenerHandler service = new ReusableClasspathListenerHandler(classpaths); + + @Test public void classpathIsSentForExistingProject() throws Exception { + 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(50), () -> { + 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 + }); + } + + @Test public void classpathIsSentForNewProject_and_removedForDeletedProject() throws Exception { + service.addClasspathListener(classpaths.commandId); + + String projectName = "classpath-test-simple-java-project"; + IProject project = createTestProject(projectName); + File loc = project.getLocation().toFile(); + 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 + }); + Logger.log("=== Deleteing project"); + project.delete(false, true, null); + ACondition.waitFor("Project classpath to disapear", Duration.ofSeconds(5), () -> { + Info cp = classpaths.getFor(loc); + assertNull(cp); + }); + + } + + ///////////// harness stuff below /////////////////////////////////////////////// + static class Info { public final String name; public final Classpath classpath; @@ -52,7 +102,6 @@ public class ClasspathListenerHandlerTest { public class MockClasspathCache implements ClientCommandExecutor { - String commandId = RandomStringUtils.randomAlphabetic(8); Map classpaths = new HashMap<>(); @@ -65,47 +114,34 @@ public class ClasspathListenerHandlerTest { String name = (String) params[1]; boolean deleted = (boolean) params[2]; if (deleted) { + System.out.println("DELETING "+name); classpaths.remove(projectLoc); } else { Classpath cp = (Classpath) params[3]; + System.out.println("PUT "+name+" "+cp.getEntries().size()+" entries"); classpaths.put(projectLoc, new Info(name, cp)); } } return "whatever"; } - public synchronized Info getFor(IProject project) { - File location = project.getLocation().toFile(); + public synchronized Info getFor(File location) { return classpaths.get(location); } - public void dispose() { + public void dispose() throws Exception { service.removeClasspathListener(commandId); + deleteAllProjects(); } } - private MockClasspathCache classpaths = new MockClasspathCache(); - private ReusableClasspathListenerHandler service = new ReusableClasspathListenerHandler(classpaths); @After - public void tearDown() { + public void tearDown() throws Exception { classpaths.dispose(); assertTrue(service.hasNoActiveSubscriptions()); } - @Test - public void classpathIsSentForExistingProject() throws Exception { - String projectName = "classpath-test-simple-java-project"; - IProject project = createTestProject(projectName); - - service.addClasspathListener(classpaths.commandId); - ACondition.waitFor("Project with classpath to appear", Duration.ofSeconds(5), () -> { - Classpath cp = classpaths.getFor(project).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 - }); - } - private static void assertClasspath(Classpath cp, boolean b) { if (!b) { StringBuilder buf = new StringBuilder(); @@ -130,5 +166,28 @@ public class ClasspathListenerHandlerTest { return project; } - + + public static void deleteAllProjects() throws Exception { + CompletableFuture done = new CompletableFuture(); + WorkspaceJob job = new WorkspaceJob("Delete projects") { + @Override public IStatus runInWorkspace(IProgressMonitor arg0) throws CoreException { + try { + ResourcesPlugin.getWorkspace().getRuleFactory().buildRule(); + IProject[] allProjects = ResourcesPlugin.getWorkspace().getRoot().getProjects(); + for (IProject project : allProjects) { + project.refreshLocal(IResource.DEPTH_INFINITE, null); + project.close(null); + project.delete(false, true, new NullProgressMonitor()); + } + done.complete(null); + } catch (Throwable e) { + done.completeExceptionally(e); + } + return Status.OK_STATUS; + } + }; + job.setRule(ResourcesPlugin.getWorkspace().getRuleFactory().buildRule()); + job.schedule(); + done.get(); + } } 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 e510b22e3..afe16b1f3 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 @@ -32,86 +32,96 @@ import static org.springframework.tooling.jdt.ls.commons.Logger.*; */ public class ClasspathListenerManager { - public interface ClasspathListener { - public abstract void classpathChanged(IJavaProject jp); - } + public interface ClasspathListener { + public abstract void classpathChanged(IJavaProject jp); + } - private class MyListener implements IElementChangedListener { + private class MyListener implements IElementChangedListener { - @Override - public void elementChanged(ElementChangedEvent event) { - visit(event.getDelta()); - } + @Override + public void elementChanged(ElementChangedEvent event) { + Logger.log("changeEvent = "+event); + visit(event.getDelta()); + } - private void visit(IJavaElementDelta delta) { - IJavaElement el = delta.getElement(); - switch (el.getElementType()) { - case IJavaElement.JAVA_MODEL: - visitChildren(delta); - break; - case IJavaElement.JAVA_PROJECT: - if (isClasspathChanged(delta.getFlags())) { - listener.classpathChanged((IJavaProject)el); - } - break; - default: - break; - } - } + private void visit(IJavaElementDelta delta) { + IJavaElement el = delta.getElement(); + switch (el.getElementType()) { + case IJavaElement.JAVA_MODEL: + visitChildren(delta); + break; + case IJavaElement.JAVA_PROJECT: + if (isCreatedOrDeleted(delta) || isClasspathChanged(delta.getFlags())) { + listener.classpathChanged((IJavaProject)el); + } + break; + default: + break; + } + } - private boolean isClasspathChanged(int flags) { - return 0!= (flags & ( - IJavaElementDelta.F_CLASSPATH_CHANGED | - IJavaElementDelta.F_RESOLVED_CLASSPATH_CHANGED - )); - } + private boolean isCreatedOrDeleted(IJavaElementDelta delta) { + int kind = delta.getKind(); + return kind == IJavaElementDelta.ADDED || kind==IJavaElementDelta.REMOVED; + } - public void visitChildren(IJavaElementDelta delta) { - for (IJavaElementDelta c : delta.getAffectedChildren()) { - visit(c); - } - } - } + private boolean isClasspathChanged(int flags) { + return 0!= (flags & ( + IJavaElementDelta.F_CLASSPATH_CHANGED | + IJavaElementDelta.F_RESOLVED_CLASSPATH_CHANGED | + IJavaElementDelta.F_CLOSED | + IJavaElementDelta.F_OPENED + )); + } - private ClasspathListener listener; - private MyListener myListener; + public void visitChildren(IJavaElementDelta delta) { + for (IJavaElementDelta c : delta.getAffectedChildren()) { + visit(c); + } + } + } - /** - * @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(ClasspathListener listener, boolean initialEvent) { - log("Setting up ClasspathListenerManager"); - this.listener = listener; - if (initialEvent) { - log("Sending initial event for all projects ..."); - for (IProject p : ResourcesPlugin.getWorkspace().getRoot().getProjects()) { - log("project = "+p); - try { - if (p.isAccessible() && p.hasNature(JavaCore.NATURE_ID)) { - IJavaProject jp = JavaCore.create(p); - listener.classpathChanged(jp); - } - } catch (CoreException e) { - Logger.log(e); - } - } - log("Sending initial event for all projects DONE"); - } - JavaCore.addElementChangedListener(myListener=new MyListener(), ElementChangedEvent.POST_CHANGE); - } + private ClasspathListener listener; + private MyListener myListener; - public ClasspathListenerManager(ClasspathListener listener) { - this(listener, false); - } + /** + * @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(ClasspathListener listener, boolean initialEvent) { + log("Setting up ClasspathListenerManager"); + this.listener = listener; + JavaCore.addElementChangedListener(myListener=new MyListener(), ElementChangedEvent.POST_CHANGE); + if (initialEvent) { + log("Sending initial event for all projects ..."); + for (IProject p : ResourcesPlugin.getWorkspace().getRoot().getProjects()) { + log("project "+p.getName() +" ..." ); + try { + if (p.isAccessible() && p.hasNature(JavaCore.NATURE_ID)) { + IJavaProject jp = JavaCore.create(p); + listener.classpathChanged(jp); + } else { + log("project "+p.getName() +" SKIPPED" ); + } + } catch (CoreException e) { + Logger.log(e); + } + } + log("Sending initial event for all projects DONE"); + } + } - public void dispose() { - if (myListener!=null) { - JavaCore.removeElementChangedListener(myListener); - myListener = null; - } - } + public ClasspathListenerManager(ClasspathListener listener) { + this(listener, false); + } + + public void dispose() { + if (myListener!=null) { + JavaCore.removeElementChangedListener(myListener); + myListener = null; + } + } } \ No newline at end of file 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 72e2674b4..6f180baef 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,6 +12,8 @@ package org.springframework.tooling.jdt.ls.commons.classpath; import static org.springframework.tooling.jdt.ls.commons.Logger.log; +import java.io.File; +import java.net.URI; import java.util.HashMap; import java.util.Map; @@ -34,6 +36,24 @@ public class ReusableClasspathListenerHandler { this.conn = conn; log("Instantiating ReusableClasspathListenerHandler"); } + + /** + * To keep track of project locations. Without this we can't properly handle deleting events because + * deleted projects (no longer) have a location. So we can only send a proper 'project with this location' + * was deleted' events if we keep track of project locations ourselves. + */ + private Map projectLocations = new HashMap<>(); + + private URI getProjectLocation(IJavaProject jp) { + URI loc = jp.getProject().getLocationURI(); + if (loc!=null) { + return loc; + } else { + //fallback on what we stored ourselves. + return projectLocations.get(jp.getElementName()); + } + } + class Subscribptions { @@ -56,35 +76,55 @@ public class ReusableClasspathListenerHandler { private void sendNotification(String callbackCommandId, IJavaProject jp) { //TODO: make one Job to accumulate all requested notification and work more efficiently by batching // and avoiding multiple executions of duplicated requests. - new Job("SendClasspath notification") { + Job job = new Job("SendClasspath notification") { @Override protected IStatus run(IProgressMonitor monitor) { - log("Classpath changed " + jp.getElementName()); - String project = jp.getProject().getLocationURI().toString(); - boolean deleted = !jp.exists(); -// JavaClientConnection conn = JavaLanguageServerPlugin.getInstance().getClientConnection(); - String projectName = jp.getElementName(); - - Classpath classpath = null; - if (!deleted) { + synchronized (projectLocations) { //Could use some Eclipse job rule. But its really a bit of a PITA to create the right one. try { - classpath = ClasspathUtil.resolve(jp); + log("Preparing classpath changed notification " + jp.getElementName()); + URI projectLoc = getProjectLocation(jp); + if (projectLoc==null) { + Logger.log("Could not send event for project because no project location: "+jp.getElementName()); + } else { + boolean exsits = jp.exists(); + boolean open = true; // WARNING: calling is 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. + // So we will just pretend / assume project is always open. If resolving classpath fails because it is not + // open... so be it (there will be no classpath... this is expected for closed project, so that is fine). + boolean deleted = !(exsits && open); + Logger.log("exists = "+exsits +" open = "+open +" => deleted = "+deleted); + String projectName = jp.getElementName(); + + Classpath classpath = null; + if (deleted) { + projectLocations.remove(projectName); + } else { + projectLocations.put(projectName, projectLoc); + try { + classpath = ClasspathUtil.resolve(jp); + } catch (Exception e) { + Logger.log(e); + } + } + try { + Logger.log("executing callback "+callbackCommandId+" "+projectName+" "+deleted+" "+(classpath==null ? "" : classpath.getEntries().size())); + conn.executeClientCommand(callbackCommandId, projectLoc.toString(), projectName, deleted, classpath); + Logger.log("executing callback "+callbackCommandId+" SUCCESS"); + } catch (Exception e) { + Logger.log("executing callback "+callbackCommandId+" FAILED"); + Logger.log(e); + } + } } catch (Exception e) { Logger.log(e); } + return Status.OK_STATUS; } - try { - Logger.log("executing callback "+callbackCommandId+" "+projectName+" "+deleted+" "+(classpath==null ? "" : classpath.getEntries().size())); - conn.executeClientCommand(callbackCommandId, project, projectName, deleted, classpath); - Logger.log("executing callback "+callbackCommandId+" SUCCESS"); - } catch (Exception e) { - Logger.log("executing callback "+callbackCommandId+" FAILED"); - Logger.log(e); - } - return Status.OK_STATUS; } - } - .schedule(); + }; + job.schedule(); } public synchronized void unsubscribe(String callbackCommandId) { @@ -109,9 +149,9 @@ public class ReusableClasspathListenerHandler { private Subscribptions subscribptions = new Subscribptions(); public Object removeClasspathListener(String callbackCommandId) { - log("ClasspathListenerHandler addClasspathListener " + callbackCommandId); + log("ClasspathListenerHandler removeClasspathListener " + callbackCommandId); subscribptions.unsubscribe(callbackCommandId); - log("ClasspathListenerHandler addClasspathListener " + callbackCommandId + " => OK"); + log("ClasspathListenerHandler removeClasspathListener " + callbackCommandId + " => OK"); return "ok"; }