diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java index 449198e1b..1200e52ce 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFBuildpacksProvider.java @@ -30,9 +30,12 @@ public class ManifestYamlCFBuildpacksProvider extends AbstractCFHintsProvider { @Override public Collection getHints(List targets) throws Exception { - + if (targets==null || targets.isEmpty()) { + //no targets... means we don't know anything. Indicate this by returning null... + // this "don't know" value will suppress bogus warnings in the reconciler. + return null; + } List hints = new ArrayList<>(); - for (CFTarget cfTarget : targets) { List buildpacks = cfTarget.getBuildpacks(); @@ -51,12 +54,7 @@ public class ManifestYamlCFBuildpacksProvider extends AbstractCFHintsProvider { } } } - // 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; + return hints; } protected String getBuildpackLabel(CFTarget target, CFBuildpack buildpack) { diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFDomainsProvider.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFDomainsProvider.java index e588e81f3..d3496e32a 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFDomainsProvider.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFDomainsProvider.java @@ -29,9 +29,12 @@ public class ManifestYamlCFDomainsProvider extends AbstractCFHintsProvider { @Override public Collection getHints(List targets) throws Exception { - + if (targets==null || targets.isEmpty()) { + //no targets... means we don't know anything. Indicate this by returning null... + // this "don't know" value will suppress bogus warnings in the reconciler. + return null; + } List hints = new ArrayList<>(); - for (CFTarget cfTarget : targets) { List domains = cfTarget.getDomains(); @@ -48,10 +51,7 @@ public class ManifestYamlCFDomainsProvider extends AbstractCFHintsProvider { } } } - // Contract for the reconciler: return null if values cannot be - // resolved. Otherwise - // return non-empty list - return !hints.isEmpty() ? hints : null; + return hints; } protected String getLabel(CFTarget target, CFDomain domain) { diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java index c2f773738..e2d329039 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlCFServicesProvider.java @@ -30,14 +30,12 @@ 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 + if (targets==null || targets.isEmpty()) { + //no targets... means we don't know anything. Indicate this by returning null... + // this "don't know" value will suppress bogus warnings in the reconciler. + return null; + } List hints = new ArrayList<>(); - for (CFTarget cfTarget : targets) { List services = cfTarget.getServices(); Renderable targetLabel = Renderables.text(cfTarget.getLabel()); @@ -54,7 +52,6 @@ public class ManifestYamlCFServicesProvider extends AbstractCFHintsProvider { } } } - return hints; } diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlStacksProvider.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlStacksProvider.java index 542069a71..3a2179850 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlStacksProvider.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlStacksProvider.java @@ -35,13 +35,12 @@ public class ManifestYamlStacksProvider extends AbstractCFHintsProvider { @Override protected 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 + if (targets==null || targets.isEmpty()) { + //no targets... means we don't know anything. Indicate this by returning null... + // this "don't know" value will suppress bogus warnings in the reconciler. + return null; + } List hints = new ArrayList<>(); - for (CFTarget cfTarget : targets) { List stacks = cfTarget.getStacks(); Renderable targetLabel = Renderables.text(cfTarget.getLabel()); @@ -57,7 +56,6 @@ public class ManifestYamlStacksProvider extends AbstractCFHintsProvider { } } } - return hints; } diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java index a662de7fc..2dca078bd 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java @@ -42,7 +42,7 @@ import com.google.common.collect.ImmutableSet; /** * @author Kris De Volder */ -public class ManifestYmlSchema implements YamlSchema { +public final class ManifestYmlSchema implements YamlSchema { private static final String HEALTH_CHECK_HTTP_ENDPOINT_PROP = "health-check-http-endpoint"; private static final String HEALTH_CHECK_TYPE_PROP = "health-check-type"; diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/RouteValueParser.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/RouteValueParser.java index 440957a57..0f037aa26 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/RouteValueParser.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/RouteValueParser.java @@ -13,28 +13,32 @@ import org.springframework.ide.vscode.commons.util.RegexpParser; import org.springframework.ide.vscode.commons.util.ValueParseException; public class RouteValueParser extends RegexpParser { - + private static final String ROUTE_REGEX = "^([\\da-z\\.-]+)(:\\d{1,5})?((\\/[\\dA-Za-z\\.-]+)*\\/?)?$"; private static final String ROUTE_TYPE_NAME = "Route"; private static final String ROUTE_DESCRIPTION = "HTTP or TCP application root route"; private static final int MAX_PORT_NUMBER = 65535; - + private Callable> domains; public RouteValueParser(Callable> domains) { super(ROUTE_REGEX, ROUTE_TYPE_NAME, ROUTE_DESCRIPTION); this.domains = domains; } - + private Matcher staticValidation(String str) throws Exception { return (Matcher) super.parse(str); } - + private Object dynamicValidation(String str, Matcher matcher) throws Exception { + if (domains==null) { + // If domains is unknown we can't do the dynamic checks, so bail out. + return str; + } try { Collection cloudDomains = Collections.emptyList(); try { - cloudDomains = domains == null ? Collections.emptyList() : domains.call(); + cloudDomains = domains.call(); } catch (ValueParseException e) { /* * If domains hint provider throws exception it is @@ -43,9 +47,9 @@ public class RouteValueParser extends RegexpParser { */ return matcher; } - // Ensure cloud domains is empty list instead of null if (cloudDomains == null) { - cloudDomains = Collections.emptyList(); + // If domains is unknown we can't do the dynamic checks, so bail out. + return str; } CFRoute route = CFRoute.builder().from(str, cloudDomains).build(); if (route.getDomain() == null || route.getDomain().isEmpty()) { @@ -65,13 +69,13 @@ public class RouteValueParser extends RegexpParser { String hostDomain = matcher.group(1); throw new ReconcileException("Unknown 'Domain'. Valid domains are: "+cloudDomains, ManifestYamlSchemaProblemsTypes.UNKNOWN_DOMAIN_PROBLEM, hostDomain.lastIndexOf(route.getDomain()), hostDomain.length()); } - return route; + return str; } catch (ConnectionException | NoTargetsException e) { // No connection to CF? Abort dynamic validation - return matcher; + return str; } } - + @Override public Object parse(String str) throws Exception { Matcher matcher = staticValidation(str); @@ -80,5 +84,5 @@ public class RouteValueParser extends RegexpParser { } return null; } - + } diff --git a/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java b/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java index ac341148b..383825933 100644 --- a/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java +++ b/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java @@ -24,6 +24,7 @@ import org.eclipse.lsp4j.CompletionItem; import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.DiagnosticSeverity; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.mockito.Mockito; import org.springframework.ide.vscode.commons.cloudfoundry.client.CFBuildpack; @@ -997,6 +998,33 @@ public class ManifestYamlEditorTest { ); } + @Test public void noReconcileErrorsWhenNoTargets() throws Exception { + Editor editor; + cloudfoundry.reset(); + when(cloudfoundry.defaultParamsProvider.getParams()).thenReturn(ImmutableList.of()); + + editor = harness.newEditor( + "applications:\n" + + "- name: foo\n" + + " buildpack: bad-buildpack\n" + + " stack: blah\n" + + " domain: something-domain.com\n" + + " services:\n" + + " - bad-service\n" + + " bogus: bad" //a token error to make sure reconciler is actually running! + ); + editor.assertProblems("bogus|Unknown property"); + + editor = harness.newEditor( + "applications:\n" + + "- name: foo-foo\n" + + " buildpack: java_buildpack\n" + + " routes:\n" + + " - route: foo.blah/fooo\n" + ); + editor.assertProblems(/*NONE*/); + } + @Test public void noReconcileErrorsWhenCFFactoryThrows() throws Exception { reset(cloudfoundry.factory); @@ -1010,6 +1038,15 @@ public class ManifestYamlEditorTest { " bogus: bad" //a token error to make sure reconciler is actually running! ); editor.assertProblems("bogus|Unknown property"); + + editor = harness.newEditor( + "applications:\n" + + "- name: foo-foo\n" + + " buildpack: java_buildpack\n" + + " routes:\n" + + " - route: foo.blah/fooo\n" + ); + editor.assertProblems(/*NONE*/); } @Test