From 1693f708717980542330a4e01fb39251c6b89af1 Mon Sep 17 00:00:00 2001 From: nsingh Date: Tue, 24 Jan 2017 16:48:17 -0800 Subject: [PATCH] Changes to CF hints provider to return empty list of services --- .../manifest/yaml/AbstractCFHintsProvider.java | 12 +++++------- .../yaml/ManifestYamlCFBuildpacksProvider.java | 13 +++++++++---- .../yaml/ManifestYamlCFServicesProvider.java | 13 +++++++++---- .../manifest/yaml/ManifestYamlEditorTest.java | 2 -- 4 files changed, 23 insertions(+), 17 deletions(-) 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 b71ce46f0..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,13 +39,13 @@ 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 error into something readable to the user as it may // appear in the content assist @@ -75,7 +74,6 @@ 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..62000240d 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,7 +790,6 @@ public class ManifestYamlEditorTest { editor.assertProblems("bogus|Unknown property"); } - @Ignore @Test public void reconcileShowsWarningOnUnknownService() throws Exception { ClientRequests cfClient = cfClientFactory.client; @@ -811,7 +810,6 @@ public class ManifestYamlEditorTest { assertEquals(DiagnosticSeverity.Warning, problem.getSeverity()); } - @Ignore @Test public void reconcileShowsWarningOnNoService() throws Exception { ClientRequests cfClient = cfClientFactory.client;