diff --git a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java index d74986b2d..cfd07fa67 100644 --- a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java +++ b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java @@ -68,6 +68,17 @@ public class ExceptionUtil { return "An error occurred: " + getSimpleError(e); } } + + public static String getMessageNoAppendedInformation(Throwable e) { + Throwable deepestCause = ExceptionUtil.getDeepestCause(e); + String msg = deepestCause != null ? deepestCause.getMessage() : null; + + if (StringUtil.hasText(msg)) { + return msg; + } else { + return "An error occurred: " + getSimpleError(e); + } + } public static String getSimpleError(Throwable e) { return e.getClass().getSimpleName(); diff --git a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java index 2d32b0283..aa35d098a 100644 --- a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java +++ b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java @@ -186,12 +186,7 @@ public class YTypeAssistContext extends AbstractYamlAssistContext { // If value parse exception, do not append any additional information if (e instanceof ValueParseException) { - String msg = e.getMessage(); - if (StringUtil.hasText(msg)) { - return msg; - } else { - return "An error occurred: " + getSimpleError(e); - } + return ExceptionUtil.getMessageNoAppendedInformation(e); } else { return ExceptionUtil.getMessage(e); } diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java index 5261d2cc2..c67d74d2a 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java @@ -11,7 +11,6 @@ package org.springframework.ide.vscode.manifest.yaml; import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.concurrent.Callable; @@ -40,22 +39,30 @@ public abstract class AbstractCFHintsProvider implements Callable call() throws Exception { - Collection hints = new ArrayList<>(); + try { List targets = targetCache.getOrCreate(); - Collection resolvedHints = getHints(targets); - if (resolvedHints != null) { - hints.addAll(resolvedHints); - } + + // Do NOT wrap the results in another list. Allow null values to return + // as the reconcile framework expects null if hints failed to be resolved + return getHints(targets); } catch (Throwable e) { - // Convert any non "no-target" errors to something readable. The - // "no-target" errors - // are generated by the target provider so they should be propagated - // as is without further - // transformation - if (ExceptionUtil.getThrowable(e, NoTargetsException.class) == null) { + // Convert any error into something readable to the user as it may + // appear in the content assist + // UI. Do NOT wrap the original exception as the framework may look + // for the deepest cause when + // resolving the error message. Instead, log the full error, and + // only throw a + // new exception with a "nicer" message + Throwable noTargetsError = ExceptionUtil.getThrowable(e, NoTargetsException.class); + if (noTargetsError != null) { + // Do not log the no-targets exception as it may be encountered + // frequently + // if a user does not have a CF client installed + throw new ValueParseException(ExceptionUtil.getMessageNoAppendedInformation(noTargetsError)); + } else { // Log any other error - logger.log(Level.SEVERE, e.getMessage(), e); + logger.log(Level.SEVERE, ExceptionUtil.getMessage(e), e); if (ExceptionUtil.getThrowable(e, IOException.class) != null) { throw new ValueParseException( @@ -65,11 +72,8 @@ public abstract class AbstractCFHintsProvider implements Callable getHints(List targets) throws Exception { + + List hints = new ArrayList<>(); + for (CFTarget cfTarget : targets) { List buildpacks = cfTarget.getBuildpacks(); if (buildpacks != null && !buildpacks.isEmpty()) { - List hints = new ArrayList<>(); for (CFBuildpack buildpack : buildpacks) { String name = buildpack.getName(); @@ -45,9 +47,12 @@ public class ManifestYamlCFBuildpacksProvider extends AbstractCFHintsProvider { return hints; } } - // Return null if no hints an be resolved rather than empty list (seems to be - // what is expected for parsing for reconciler) - return null; + // Contract for the reconciler: return null if values cannot be + // resolved. Otherwise + // return non-empty list of buildpacks. For CF targets, a non-empty list + // of buildpacks is + // typically expected. + return !hints.isEmpty() ? hints : null; } protected String getBuildpackLabel(CFTarget target, CFBuildpack buildpack) { diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java index bf2939fe6..852e553c2 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java @@ -29,10 +29,16 @@ public class ManifestYamlCFServicesProvider extends AbstractCFHintsProvider { @Override public Collection getHints(List targets) throws Exception { + // NOTE: empty list of services is a VALID result. A CF target may have + // no service instances + // created, so if empty list is returned from the client, then RETURN empty list. don't + // return null + // for empty services cases + List hints = new ArrayList<>(); + for (CFTarget cfTarget : targets) { List services = cfTarget.getServices(); if (services != null && !services.isEmpty()) { - List hints = new ArrayList<>(); for (CFServiceInstance service : services) { String name = service.getName(); @@ -45,9 +51,8 @@ public class ManifestYamlCFServicesProvider extends AbstractCFHintsProvider { return hints; } } - // Return null if no hints an be resolved rather than empty list (seems to be - // what is expected for parsing for reconciler) - return null; + + return hints; } private String getServiceLabel(CFTarget cfClientTarget, CFServiceInstance service) { diff --git a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java index 8fc8b6ceb..78b9169f1 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java +++ b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java @@ -790,13 +790,31 @@ public class ManifestYamlEditorTest { editor.assertProblems("bogus|Unknown property"); } + @Ignore + @Test + public void reconcileCFService() throws Exception { + ClientRequests cfClient = cfClientFactory.client; + CFServiceInstance service = Mockito.mock(CFServiceInstance.class); + when(service.getName()).thenReturn("myservice"); + when(cfClient.getServices()).thenReturn(ImmutableList.of(service)); + Editor editor = harness.newEditor( + "applications:\n" + + "- name: foo\n" + + " services:\n" + + " - myservice\n" + + ); + // Should have no problems + editor.assertProblems(/*none*/); + } + @Ignore @Test public void reconcileShowsWarningOnUnknownService() throws Exception { ClientRequests cfClient = cfClientFactory.client; CFServiceInstance service = Mockito.mock(CFServiceInstance.class); when(service.getName()).thenReturn("myservice"); - when(cfClient.getServices()).thenReturn(ImmutableList.of()); + when(cfClient.getServices()).thenReturn(ImmutableList.of(service)); Editor editor = harness.newEditor( "applications:\n" + "- name: foo\n" + @@ -834,4 +852,5 @@ public class ManifestYamlEditorTest { Editor editor = harness.newEditor(textBefore); editor.assertCompletions(textAfter); } + }