From fb0ddc0469b26eff2990a8318f00f4e7303a3025 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 28 May 2019 10:43:41 -0700 Subject: [PATCH] Refactor classpath notification job Change it so there is one job per subscriber to handle communication with that particular subscriber. This will allow the job to behave differently depending on whether that subscriber supports batched update events. --- .../java/ClasspathListenerParams.java | 27 +++++++++++-- .../tooling/jdt/ls/commons/Logger.java | 2 +- .../ReusableClasspathListenerHandler.java | 31 ++++++++------- .../SendClasspathNotificationsJob.java | 38 +++++++------------ 4 files changed, 57 insertions(+), 41 deletions(-) diff --git a/headless-services/commons/commons-lsp-extensions/src/main/java/org/springframework/ide/vscode/commons/protocol/java/ClasspathListenerParams.java b/headless-services/commons/commons-lsp-extensions/src/main/java/org/springframework/ide/vscode/commons/protocol/java/ClasspathListenerParams.java index 70035115d..e90043401 100644 --- a/headless-services/commons/commons-lsp-extensions/src/main/java/org/springframework/ide/vscode/commons/protocol/java/ClasspathListenerParams.java +++ b/headless-services/commons/commons-lsp-extensions/src/main/java/org/springframework/ide/vscode/commons/protocol/java/ClasspathListenerParams.java @@ -13,9 +13,22 @@ package org.springframework.ide.vscode.commons.protocol.java; public class ClasspathListenerParams { private String callbackCommandId; + + /** + * The requestor can set this to true to request 'batched' events + * (if the client supports this, it will send multiple events in a single + * callback. + */ + private boolean batched = false; + + public ClasspathListenerParams() {} + + public ClasspathListenerParams(String callbackCommandId, boolean isBatched) { + this.callbackCommandId = callbackCommandId; + this.batched = isBatched; + } public ClasspathListenerParams(String callbackCommandId) { - super(); this.callbackCommandId = callbackCommandId; } @@ -26,10 +39,18 @@ public class ClasspathListenerParams { public void setCallbackCommandId(String callbackCommandId) { this.callbackCommandId = callbackCommandId; } - + @Override public String toString() { - return "ClasspathListenerParams [callbackCommandId=" + callbackCommandId + "]"; + return "ClasspathListenerParams [callbackCommandId=" + callbackCommandId + ", batched=" + batched + "]"; + } + + public boolean isBatched() { + return batched; + } + + public void setBatched(boolean batched) { + this.batched = batched; } } diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/Logger.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/Logger.java index 4b469ccc3..c74c5eae3 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/Logger.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/Logger.java @@ -23,7 +23,7 @@ import org.eclipse.core.runtime.Status; /** - * Poor man's logger with a defauly implementation writes log output for jdt.ls extension into a predictable location. + * Poor man's logger with a default implementation writes log output for jdt.ls extension into a predictable location. */ public interface Logger { 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 ebeccc797..95be46750 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 @@ -18,6 +18,8 @@ import java.util.HashMap; import java.util.Map; import java.util.function.Supplier; +import javax.management.Notification; + import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; @@ -25,7 +27,6 @@ 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; -import org.springframework.tooling.jdt.ls.commons.classpath.SendClasspathNotificationsJob.Notification; /** * {@link ReusableClasspathListenerHandler} is an 'abstracted' version of the jdtls ClasspathListenerHandler. @@ -33,8 +34,8 @@ import org.springframework.tooling.jdt.ls.commons.classpath.SendClasspathNotific public class ReusableClasspathListenerHandler { private final Logger logger; + private final ClientCommandExecutor conn; private final Supplier> projectSorterFactory; - private final SendClasspathNotificationsJob sendNotificationJob; public ReusableClasspathListenerHandler(Logger logger, ClientCommandExecutor conn) { this(logger, conn, null); @@ -43,30 +44,32 @@ public class ReusableClasspathListenerHandler { public ReusableClasspathListenerHandler(Logger logger, ClientCommandExecutor conn, Supplier> projectSorterFactory) { this.logger = logger; this.projectSorterFactory = projectSorterFactory; - this.sendNotificationJob = new SendClasspathNotificationsJob(logger, conn); + this.conn = conn; logger.log("Instantiating ReusableClasspathListenerHandler"); } class Subscriptions { - private Map subscribers = null; + private Map subscribers = null; private ClasspathListenerManager classpathListener = null; public synchronized void subscribe(String callbackCommandId, boolean isBatched) { - logger.log("subscribing to classpath changes: " + callbackCommandId +" isBatched = "+isBatched); if (subscribers==null) { //First subscriber subscribers = new HashMap<>(); + } + if (!subscribers.containsKey(callbackCommandId)) { + logger.log("subscribing to classpath changes: " + callbackCommandId +" isBatched = "+isBatched); classpathListener = new ClasspathListenerManager(logger, new ClasspathListener() { @Override public void classpathChanged(IJavaProject jp) { sendNotification(jp, subscribers.keySet()); } }); + subscribers.put(callbackCommandId, new SendClasspathNotificationsJob(logger, conn, callbackCommandId, isBatched)); + logger.log("subsribers = " + subscribers); + sendInitialEvents(callbackCommandId); } - subscribers.put(callbackCommandId, isBatched); - logger.log("subsribers = " + subscribers); - sendInitialEvents(callbackCommandId); } private void sendInitialEvents(String callbackCommandId) { @@ -93,12 +96,14 @@ public class ReusableClasspathListenerHandler { logger.log("Sending initial event for all projects DONE"); } - - private void sendNotification(IJavaProject jp, Collection callbackIds) { - for (String callbackId : callbackIds) { - sendNotificationJob.queue.add(new Notification(jp, callbackId)); + private synchronized void sendNotification(IJavaProject jp, Collection callbackIds) { + if (subscribers!=null) { + for (String callbackId : callbackIds) { + SendClasspathNotificationsJob sendNotificationJob = subscribers.get(callbackId); + sendNotificationJob.queue.add(jp); + sendNotificationJob.schedule(); + } } - sendNotificationJob.schedule(); } public synchronized void unsubscribe(String callbackCommandId) { diff --git a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java index c8554162c..94c10af73 100644 --- a/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java +++ b/headless-services/jdt-ls-extension/org.springframework.tooling.jdt.ls.commons/src/org/springframework/tooling/jdt/ls/commons/classpath/SendClasspathNotificationsJob.java @@ -30,21 +30,15 @@ public class SendClasspathNotificationsJob extends Job { private final ClientCommandExecutor conn; private final Logger logger; + private String callbackCommandId; + private boolean isBatched; - public SendClasspathNotificationsJob(Logger logger, ClientCommandExecutor conn) { + public SendClasspathNotificationsJob(Logger logger, ClientCommandExecutor conn, String callbackId, boolean isBatched) { super("Send Classpath Notifications"); this.logger = logger; this.conn = conn; - } - - public static class Notification { - IJavaProject jp; - String callbackId; - - public Notification(IJavaProject jp, String callbackId) { - this.jp = jp; - this.callbackId = callbackId; - } + this.callbackCommandId = callbackId; + this.isBatched = isBatched; } /** @@ -66,7 +60,7 @@ public class SendClasspathNotificationsJob extends Job { } } - public final Queue queue = new ConcurrentLinkedQueue<>(); + public final Queue queue = new ConcurrentLinkedQueue<>(); private boolean projectExists(IJavaProject jp) { //We can't really deal with projects that don't exist in disk. So using this more strict 'exists' check @@ -93,8 +87,7 @@ public class SendClasspathNotificationsJob extends Job { protected IStatus run(IProgressMonitor monitor) { synchronized (projectLocations) { //Could use some Eclipse job rule. But its really a bit of a PITA to create the right one. try { - for (Notification notification = queue.poll(); notification!=null; notification = queue.poll()) { - IJavaProject jp = notification.jp; + for (IJavaProject jp = queue.poll(); jp!=null; jp = queue.poll()) { logger.log("Preparing classpath changed notification " + jp.getElementName()); URI projectLoc = getProjectLocation(jp); if (projectLoc==null) { @@ -121,16 +114,13 @@ public class SendClasspathNotificationsJob extends Job { logger.log(e); } } - String callbackCommandId = notification.callbackId; - { - 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); - } + 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); } } }