From f7350119911940a6ee02020e8bf6719967407c6a Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 20 Mar 2018 12:15:39 -0700 Subject: [PATCH 1/5] Experimental execute client-side command protocol extension Signed-off-by: Kris De Volder --- .../core/internal/JavaClientConnection.java | 11 ++++++++- .../lsp/ExecuteCommandProposedClient.java | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java index 51d76c023e..5495aab3fa 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java @@ -13,9 +13,11 @@ import java.util.List; import org.eclipse.jdt.ls.core.internal.handlers.LogHandler; +import org.eclipse.jdt.ls.core.internal.lsp.ExecuteCommandProposedClient; import org.eclipse.lsp4j.ApplyWorkspaceEditParams; import org.eclipse.lsp4j.ApplyWorkspaceEditResponse; import org.eclipse.lsp4j.Command; +import org.eclipse.lsp4j.ExecuteCommandParams; import org.eclipse.lsp4j.MessageActionItem; import org.eclipse.lsp4j.MessageParams; import org.eclipse.lsp4j.MessageType; @@ -27,9 +29,12 @@ import org.eclipse.lsp4j.jsonrpc.services.JsonNotification; import org.eclipse.lsp4j.services.LanguageClient; +import com.google.common.collect.ImmutableList; + public class JavaClientConnection { - public interface JavaLanguageClient extends LanguageClient { + public interface JavaLanguageClient extends LanguageClient, ExecuteCommandProposedClient { + /** * The show message notification is sent from a server to a client to ask * the client to display a particular message in the user interface. @@ -56,6 +61,10 @@ public JavaClientConnection(JavaLanguageClient client) { logHandler.install(this); } + public Object executeCommand(String id, Object... params) { + return this.client.executeCommand(new ExecuteCommandParams(id, ImmutableList.copyOf(params))).join(); + } + /** * Sends the logMessage message back to the client as a notification * @param msg The message to send back to the client diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java new file mode 100644 index 0000000000..222b8ec225 --- /dev/null +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java @@ -0,0 +1,23 @@ +/******************************************************************************* + * Copyright (c) 2018 Red Hat Inc. and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat Inc. - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.ls.core.internal.lsp; + +import java.util.concurrent.CompletableFuture; + +import org.eclipse.lsp4j.ExecuteCommandParams; +import org.eclipse.lsp4j.jsonrpc.services.JsonRequest; + +public interface ExecuteCommandProposedClient { + + @JsonRequest("workspace/executeCommand") + CompletableFuture executeCommand(ExecuteCommandParams params); + +} From 3f74781d1320f3542af8972344125cc49b830dfe Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 20 Mar 2018 13:32:46 -0700 Subject: [PATCH 2/5] Rename `workspace/executeCommand` Change it to `workspace/executeClientCommand` to distinguish it from existing `workspace/executeCommand` on the server side. --- .../jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java index 222b8ec225..c2efdf41ab 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java @@ -17,7 +17,7 @@ public interface ExecuteCommandProposedClient { - @JsonRequest("workspace/executeCommand") + @JsonRequest("workspace/executeClientCommand") CompletableFuture executeCommand(ExecuteCommandParams params); } From 66aa25d44cea30d2fdf90b30058508772b990b48 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 20 Mar 2018 15:07:03 -0700 Subject: [PATCH 3/5] Also rename Java methods to executeClientCommand Follow through on renaming protocol message "workspace/executeCommand". Java methods involved in handling this message now also renamed. --- .../eclipse/jdt/ls/core/internal/JavaClientConnection.java | 4 ++-- .../ls/core/internal/lsp/ExecuteCommandProposedClient.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java index 5495aab3fa..70f7999e7b 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java @@ -61,8 +61,8 @@ public JavaClientConnection(JavaLanguageClient client) { logHandler.install(this); } - public Object executeCommand(String id, Object... params) { - return this.client.executeCommand(new ExecuteCommandParams(id, ImmutableList.copyOf(params))).join(); + public Object executeClientCommand(String id, Object... params) { + return this.client.executeClientCommand(new ExecuteCommandParams(id, ImmutableList.copyOf(params))).join(); } /** diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java index c2efdf41ab..4a3f7802a5 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java @@ -18,6 +18,6 @@ public interface ExecuteCommandProposedClient { @JsonRequest("workspace/executeClientCommand") - CompletableFuture executeCommand(ExecuteCommandParams params); + CompletableFuture executeClientCommand(ExecuteCommandParams params); } From 666012babbc63f91882803bf7e5c734cb37701c1 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 23 Mar 2018 12:40:31 -0700 Subject: [PATCH 4/5] Add tests for executeClientCommand Signed-off-by: Kris De Volder --- org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF | 3 +- .../core/internal/JavaClientConnection.java | 8 + .../internal/ExecuteClientCommandTest.java | 152 ++++++++++++++++++ 3 files changed, 162 insertions(+), 1 deletion(-) create mode 100644 org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java diff --git a/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF b/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF index 20f2bf2159..e7d0fa364a 100644 --- a/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF +++ b/org.eclipse.jdt.ls.core/META-INF/MANIFEST.MF @@ -42,7 +42,8 @@ Export-Package: org.eclipse.jdt.ls.core.internal;x-friends:="org.eclipse.jdt.ls. org.eclipse.jdt.ls.core.internal.managers;x-friends:="org.eclipse.jdt.ls.tests", org.eclipse.jdt.ls.core.internal.preferences;x-friends:="org.eclipse.jdt.ls.tests", org.eclipse.jdt.ls.core.internal.corrections;x-internal:=true, - org.eclipse.jdt.ls.core.internal.corrections.proposals;x-internal:=true + org.eclipse.jdt.ls.core.internal.corrections.proposals;x-internal:=true, + org.eclipse.jdt.ls.core.internal.lsp;x-friends:="org.eclipse.jdt.ls.tests" Bundle-ClassPath: lib/jsoup-1.9.2.jar, lib/remark-1.0.0.jar, . diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java index 70f7999e7b..8e32ffed6f 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java @@ -10,7 +10,11 @@ *******************************************************************************/ package org.eclipse.jdt.ls.core.internal; +import java.time.Duration; import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.eclipse.jdt.ls.core.internal.handlers.LogHandler; import org.eclipse.jdt.ls.core.internal.lsp.ExecuteCommandProposedClient; @@ -61,6 +65,10 @@ public JavaClientConnection(JavaLanguageClient client) { logHandler.install(this); } + public Object executeClientCommand(Duration timeout, String id, Object... params) throws InterruptedException, ExecutionException, TimeoutException { + return this.client.executeClientCommand(new ExecuteCommandParams(id, ImmutableList.copyOf(params))).get(timeout.toNanos(), TimeUnit.NANOSECONDS); + } + public Object executeClientCommand(String id, Object... params) { return this.client.executeClientCommand(new ExecuteCommandParams(id, ImmutableList.copyOf(params))).join(); } diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java new file mode 100644 index 0000000000..c923c5baff --- /dev/null +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java @@ -0,0 +1,152 @@ +/******************************************************************************* + * Copyright (c) 2018 Red Hat Inc. and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Red Hat Inc. - initial API and implementation + *******************************************************************************/ +package org.eclipse.jdt.ls.core.internal; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.time.Duration; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeoutException; + +import org.eclipse.jdt.ls.core.internal.JavaClientConnection.JavaLanguageClient; +import org.eclipse.lsp4j.ExecuteCommandParams; +import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; + +import com.google.common.collect.ImmutableList; + +public class ExecuteClientCommandTest { + + private JavaLanguageClient client = mock(JavaLanguageClient.class); + private JavaClientConnection javaClient = new JavaClientConnection(client); + + @Test + public void testExecuteClientCommandNoArgs() throws Exception { + when(client.executeClientCommand(any())).thenAnswer(handler("send.it.back", params -> params.getArguments())); + Object response = javaClient.executeClientCommand("send.it.back"); + assertEquals(ImmutableList.of(), response); + } + + @Test + public void testExecuteClientCommandNoArgsAndLongEnoughTimeout() throws Exception { + when(client.executeClientCommand(any())).thenAnswer(handler("send.it.back", params -> params.getArguments())); + Object response = javaClient.executeClientCommand(Duration.ofDays(1), "send.it.back"); + assertEquals(ImmutableList.of(), response); + } + + @Test + public void testExecuteClientCommandThrows() throws Exception { + when(client.executeClientCommand(any())).thenAnswer(handler(params -> { + throw new IllegalArgumentException("BOOM!"); + })); + try { + javaClient.executeClientCommand("whatever"); + fail("Should have thrown"); + } catch (Throwable e) { + e = getDeepestCause(e); + assertEquals("BOOM!", e.getMessage()); + } + } + + @Test + public void testExecuteClientCommandThrowsAndLongEnoughTimeout() throws Exception { + when(client.executeClientCommand(any())).thenAnswer(handler(params -> { + throw new IllegalArgumentException("BOOM!"); + })); + try { + javaClient.executeClientCommand(Duration.ofDays(1), "whatever"); + fail("Should have thrown"); + } catch (Throwable e) { + e = getDeepestCause(e); + assertEquals("BOOM!", e.getMessage()); + } + } + + @Test + public void testExecuteClientCommandSomeArgs() throws Exception { + when(client.executeClientCommand(any())).thenAnswer(handler("send.it.back", params -> params.getArguments())); + Object[] params = { "one", 2, ImmutableList.of(3) }; + Object response = javaClient.executeClientCommand("send.it.back", params); + assertEquals(ImmutableList.copyOf(params), response); + } + + @Test + public void testExecuteClientCommandSomeArgsAndLongEnoughTimeout() throws Exception { + when(client.executeClientCommand(any())).thenAnswer(handler("send.it.back", params -> params.getArguments())); + Object[] params = { "one", 2, ImmutableList.of(3) }; + Object response = javaClient.executeClientCommand(Duration.ofDays(1), "send.it.back", params); + assertEquals(ImmutableList.copyOf(params), response); + } + + @Test + public void testExecuteClientCommandTimesOut() throws Exception { + when(client.executeClientCommand(any())).thenReturn(new CompletableFuture<>()); //Future never resolves + try { + javaClient.executeClientCommand(Duration.ofMillis(10), "whatever"); + fail("Should have thrown"); + } catch (Exception e) { + assertEquals(TimeoutException.class, e.getClass()); + } + } + + ///////////////////// Harness, helper, setup etc. below ///////////////////////////////////// + + interface SyncHandler { + Object executeClientCommand(ExecuteCommandParams params); + } + + private static Answer handler(String command, SyncHandler h) { + return handler((ExecuteCommandParams params) -> { + if (params.getCommand().equals(command)) { + return h.executeClientCommand(params); + } + throw new IllegalArgumentException("Unknown command: " + params.getCommand()); + }); + } + + /** + * Convenience method to wrap a 'nice', type-checked SyncHandler into a mockito + * {@link Answer}. + */ + private static Answer handler(SyncHandler h) { + return new Answer() { + @SuppressWarnings("unchecked") + @Override + public T answer(InvocationOnMock invocation) throws Throwable { + try { + Object[] args = invocation.getArguments(); + assertEquals(1, args.length); + ExecuteCommandParams params = (ExecuteCommandParams) args[0]; + return (T) CompletableFuture.completedFuture(h.executeClientCommand(params)); + } catch (Throwable e) { + CompletableFuture fail = new CompletableFuture<>(); + fail.completeExceptionally(e); + return (T) fail; + } + } + }; + } + + private static Throwable getDeepestCause(Throwable e) { + Throwable cause = e; + Throwable parent = e.getCause(); + while (parent != null && parent != e) { + cause = parent; + parent = cause.getCause(); + } + return cause; + } +} \ No newline at end of file From 3909f038d18b5f21fc461f9a6c850ba4658959e7 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 23 Mar 2018 15:26:02 -0700 Subject: [PATCH 5/5] Fix copyright headers and small test tweak --- .../eclipse/jdt/ls/core/internal/JavaClientConnection.java | 3 ++- .../ls/core/internal/lsp/ExecuteCommandProposedClient.java | 4 ++-- .../jdt/ls/core/internal/ExecuteClientCommandTest.java | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java index 8e32ffed6f..17772e23be 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/JavaClientConnection.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2016-2017 Red Hat Inc. and others. + * Copyright (c) 2016-2018 Red Hat Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,7 @@ * * Contributors: * Red Hat Inc. - initial API and implementation + * Pivotal Inc. - added executeClientCommand API. *******************************************************************************/ package org.eclipse.jdt.ls.core.internal; diff --git a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java index 4a3f7802a5..4f210e3097 100644 --- a/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java +++ b/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/lsp/ExecuteCommandProposedClient.java @@ -1,12 +1,12 @@ /******************************************************************************* - * Copyright (c) 2018 Red Hat Inc. and others. + * Copyright (c) 2018 Pivotal Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Red Hat Inc. - initial API and implementation + * Pivotal Inc. - initial API and implementation *******************************************************************************/ package org.eclipse.jdt.ls.core.internal.lsp; diff --git a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java index c923c5baff..9d329ae8f8 100644 --- a/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java +++ b/org.eclipse.jdt.ls.tests/src/org/eclipse/jdt/ls/core/internal/ExecuteClientCommandTest.java @@ -1,12 +1,12 @@ /******************************************************************************* - * Copyright (c) 2018 Red Hat Inc. and others. + * Copyright (c) 2018 Pivotal Inc. and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: - * Red Hat Inc. - initial API and implementation + * Pivotal Inc. - initial API and implementation *******************************************************************************/ package org.eclipse.jdt.ls.core.internal; @@ -97,7 +97,7 @@ public void testExecuteClientCommandTimesOut() throws Exception { try { javaClient.executeClientCommand(Duration.ofMillis(10), "whatever"); fail("Should have thrown"); - } catch (Exception e) { + } catch (Throwable e) { assertEquals(TimeoutException.class, e.getClass()); } }