From 8d570e748ea7baeaa9e85a8724ad93be04733eb6 Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 30 Jun 2017 10:15:44 -0700 Subject: [PATCH] Fix handling of 'unknown domains' case in routes validation --- .../manifest/yaml/ManifestYmlSchema.java | 2 +- .../manifest/yaml/RouteValueParser.java | 26 +++++++++++-------- .../manifest/yaml/ManifestYamlEditorTest.java | 22 +++++++++++++++- 3 files changed, 37 insertions(+), 13 deletions(-) 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 b2896892f..4ec4ddbf6 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 @@ -999,9 +999,11 @@ public class ManifestYamlEditorTest { } @Test public void noReconcileErrorsWhenNoTargets() throws Exception { + Editor editor; cloudfoundry.reset(); when(cloudfoundry.defaultParamsProvider.getParams()).thenReturn(ImmutableList.of()); - Editor editor = harness.newEditor( + + editor = harness.newEditor( "applications:\n" + "- name: foo\n" + " buildpack: bad-buildpack\n" + @@ -1012,6 +1014,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 @@ -1027,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