From 7f4967d0a6e6f471166712755b4542df8e9a359b Mon Sep 17 00:00:00 2001 From: nsingh Date: Mon, 7 May 2018 13:58:55 -0700 Subject: [PATCH] PT 156688501 - Re-enable live hover optimisation Live hovers are not published by the server if there are no running apps in two consecutive update operations in the live hover watchdog. Also fixed test cases that test no running apps. --- .../languageserver/testharness/Editor.java | 6 ++-- .../testharness/LanguageServerHarness.java | 28 ++++++++++++++++++- .../java/utils/SpringLiveHoverWatchdog.java | 14 +++++----- .../test/AutowiredHoverProviderTest.java | 2 +- .../ComponentInjectionsHoverProviderTest.java | 2 +- 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java index 6e30ae565..a73fcb8e3 100644 --- a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java +++ b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java @@ -209,8 +209,9 @@ public class Editor { public List assertHighlights(String... expectedHighlights) throws Exception { - HighlightParams highlights = harness.getHighlights(doc); - List ranges = new ArrayList<>(highlights.getRanges()); + HighlightParams highlights = expectedHighlights == null || expectedHighlights.length == 0 ? harness.getHighlights(false, doc) + : harness.getHighlights(doc); + List ranges = highlights != null ? new ArrayList<>(highlights.getRanges()) : ImmutableList.of(); Collections.sort(ranges, RANGE_COMPARATOR); List actualHighlights = ranges.stream() .map(this::getText) @@ -219,7 +220,6 @@ public class Editor { return ranges; } - /** * Get the editor text, with cursor markers inserted (for easy textual comparison * after applying a proposal) diff --git a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java index b3baadfe0..6e9f5327e 100644 --- a/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java +++ b/headless-services/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/LanguageServerHarness.java @@ -38,6 +38,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import org.assertj.core.api.Condition; @@ -434,7 +435,32 @@ public class LanguageServerHarness { } public HighlightParams getHighlights(TextDocumentInfo doc) throws Exception { - return getHighlightsFuture(doc).get(HIGHLIGHTS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + return getHighlights(true, doc); + } + + /** + * Set expectServerHighlights to false if NO highlights are expected from the server (for example, test cases + * that test that no highlights are received from the server because there are no running apps). + * @param expectServerHighlights false if NOT expecting any highlights from the server + * @param doc + * @return highlights, if they are expected, or null if they are not expected. + * @throws Exception + */ + public HighlightParams getHighlights(boolean expectServerHighlights, TextDocumentInfo doc) throws Exception { + try { + return getHighlightsFuture(doc).get(HIGHLIGHTS_TIMEOUT.toMillis(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + // highlight requestor will timeout if the server does not send any highlights. This + // is not always an error. For example, if there are no initial running apps, the server will + // NOT send highlights (see PT 156688501), so in this case we expect to time out as part of + // the expected behaviour + if (!expectServerHighlights) { + return null; + } + else { + throw e; + } + } } public static Condition isDiagnosticWithSeverity(DiagnosticSeverity severity) { diff --git a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/SpringLiveHoverWatchdog.java b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/SpringLiveHoverWatchdog.java index 50d42f51b..d9c001ceb 100644 --- a/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/SpringLiveHoverWatchdog.java +++ b/headless-services/spring-boot-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/SpringLiveHoverWatchdog.java @@ -49,7 +49,7 @@ public class SpringLiveHoverWatchdog { private RunningAppProvider runningAppProvider; private boolean highlightsEnabled = true; -// private boolean hadPreviousRunningBootApps = false; + private boolean hadPreviousRunningBootApps = false; private Timer timer; @@ -157,17 +157,17 @@ public class SpringLiveHoverWatchdog { publishLiveHints(docURI, ranges); } } - else -// if (this.hadPreviousRunningBootApps) - { + else if (this.hadPreviousRunningBootApps) { // PT 156688501: - // Only clean up live hovers if there were running boot apps in the previous update, but not + // Only clean up live hovers if there were running boot apps in the previous + // update, but not // in the current one. - // This is to avoid unnecessary publishing of live hovers when there have been no running apps + // This is to avoid unnecessary publishing of live hovers when there have been + // no running apps // at all between consecutive updates. cleanupLiveHints(docURI); } -// this.hadPreviousRunningBootApps = hasCurrentRunningBootApps; + this.hadPreviousRunningBootApps = hasCurrentRunningBootApps; } catch (Exception e) { logger.error("", e); } diff --git a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java index e7df5c677..2b1060325 100644 --- a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java +++ b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/autowired/test/AutowiredHoverProviderTest.java @@ -156,7 +156,7 @@ public class AutowiredHoverProviderTest { "}\n" ); - editor.assertHighlights(/*MONE*/); + editor.assertHighlights(/*NONE*/); editor.assertNoHover("@Autowired"); } diff --git a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/livehover/test/ComponentInjectionsHoverProviderTest.java b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/livehover/test/ComponentInjectionsHoverProviderTest.java index 3106def93..60d8bd601 100644 --- a/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/livehover/test/ComponentInjectionsHoverProviderTest.java +++ b/headless-services/spring-boot-language-server/src/test/java/org/springframework/ide/vscode/boot/java/livehover/test/ComponentInjectionsHoverProviderTest.java @@ -471,7 +471,7 @@ public class ComponentInjectionsHoverProviderTest { " }\n" + "}\n" ); - editor.assertHighlights(/*MONE*/); + editor.assertHighlights(/*NONE*/); editor.assertNoHover("@Component"); }