From d7eda61c99f82170506543f74b5cbc670993b21e Mon Sep 17 00:00:00 2001 From: BoykoAlex Date: Wed, 3 May 2017 17:47:15 -0400 Subject: [PATCH] Rework mutually exclusive constraint --- .../util/SimpleLanguageServer.java | 3 +- .../util/SimpleTextDocumentService.java | 4 +- .../manifest/yaml/ManifestConstraints.java | 67 ++++++++++-------- .../manifest/yaml/ManifestYmlSchema.java | 29 +++----- .../manifest/yaml/ManifestYamlEditorTest.java | 68 ++++++++++++++----- 5 files changed, 101 insertions(+), 70 deletions(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java index ec037a7cb..6ad04769c 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java @@ -13,6 +13,7 @@ package org.springframework.ide.vscode.commons.languageserver.util; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; @@ -311,7 +312,7 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl } IProblemCollector problems = new IProblemCollector() { - private List diagnostics = new ArrayList<>(); + private LinkedHashSet diagnostics = new LinkedHashSet<>(); private List quickfixes = new ArrayList<>(); @Override diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java index 98cb8e463..cb76ed2ab 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleTextDocumentService.java @@ -326,12 +326,12 @@ public class SimpleTextDocumentService implements TextDocumentService { public void didSave(DidSaveTextDocumentParams params) { } - public void publishDiagnostics(TextDocumentIdentifier docId, List diagnostics) { + public void publishDiagnostics(TextDocumentIdentifier docId, Collection diagnostics) { LanguageClient client = server.getClient(); if (client!=null && diagnostics!=null) { PublishDiagnosticsParams params = new PublishDiagnosticsParams(); params.setUri(docId.getUri()); - params.setDiagnostics(diagnostics); + params.setDiagnostics(ImmutableList.copyOf(diagnostics)); client.publishDiagnostics(params); } } diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestConstraints.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestConstraints.java index 325057ad0..ab64d18c2 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestConstraints.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestConstraints.java @@ -10,16 +10,20 @@ *******************************************************************************/ package org.springframework.ide.vscode.manifest.yaml; -import java.util.Arrays; -import java.util.HashSet; +import java.util.List; import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Collectors; -import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblemImpl; +import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil; +import org.springframework.ide.vscode.commons.yaml.path.YamlPath; +import org.springframework.ide.vscode.commons.yaml.path.YamlPathSegment; +import org.springframework.ide.vscode.commons.yaml.path.YamlTraversal; +import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems; import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint; -import org.yaml.snakeyaml.nodes.MappingNode; import org.yaml.snakeyaml.nodes.Node; -import org.yaml.snakeyaml.nodes.SequenceNode; + /** * Constraints for Manifest YAML structure @@ -29,35 +33,38 @@ import org.yaml.snakeyaml.nodes.SequenceNode; */ public class ManifestConstraints { - public static Constraint exclusiveWith(String... propertyIds) { + public static Constraint mutuallyExclusive(String target, String... propertyIds) { return (dc, parent, node, type, problems) -> { - Set keys = new HashSet<>(); - Node root = dc.getAST().getNodes().get(0); - // First add keys from the root node - keys.addAll(NodeUtil.getScalarKeys(root)); - if (root == parent) { - // Add keys from all applications - SequenceNode apps = NodeUtil.asSequence(NodeUtil.getProperty(root, "applications")); - if (apps != null) { - apps.getValue().forEach(n -> keys.addAll(NodeUtil.getScalarKeys(n))); + Node targetNode = YamlPathSegment.keyAt(target).traverseNode(node); + if (targetNode != null) { + YamlTraversal conflictingTraversal = getConflictingNodesTraversal(dc.getPath(), propertyIds); + List conflictingNodes = conflictingTraversal.traverseAmbiguously(dc.getAST()).collect(Collectors.toList()); + if (!conflictingNodes.isEmpty()) { + Set conflicts = conflictingNodes.stream().map(NodeUtil::asScalar).collect(Collectors.toCollection(TreeSet::new)); + problems.accept(YamlSchemaProblems.problem( + ManifestYamlSchemaProblemsTypes.MUTUALLY_EXCLUSIVE_PROPERTY_PROBLEM, + "Property cannot co-exist with properties " + conflicts, targetNode)); + for (Node cn : conflictingNodes) { + problems.accept(YamlSchemaProblems.problem( + ManifestYamlSchemaProblemsTypes.MUTUALLY_EXCLUSIVE_PROPERTY_PROBLEM, + "Property cannot co-exist with property '" + target + "'", cn)); + } } - } else { - // Now add keys from application node, thus application node keys would replace root node keys if they present in both nodes - keys.addAll(NodeUtil.getScalarKeys(parent)); } - Arrays.stream(propertyIds).filter(id -> keys.contains(id)).findFirst().ifPresent(propertyId -> { - // Find key node, because the node parameter is the value node - MappingNode mapNode = (MappingNode) parent; - mapNode.getValue().stream().filter(t -> t.getValueNode() == node).findFirst().ifPresent(t -> { - Node keyNode = t.getKeyNode(); - int start = keyNode.getStartMark().getIndex(); - int end = keyNode.getEndMark().getIndex(); - problems.accept( - new ReconcileProblemImpl(ManifestYamlSchemaProblemsTypes.MUTUALLY_EXCLUSIVE_PROPERTY_PROBLEM, - "Property cannot co-exist with property '" + propertyId + "'", start, end - start)); - }); - }); }; } + private static YamlTraversal getConflictingNodesTraversal(YamlPath path, String[] propertyIds) { + Assert.isLegal(propertyIds.length > 0); + YamlTraversal properties = null; + for (String id : propertyIds) { + properties = properties == null ? YamlPathSegment.keyAt(id) : properties.or(YamlPathSegment.keyAt(id)); + } + YamlTraversal traversal = path.then(properties); + if (path.size() > 2) { + traversal = traversal.or(path.dropLast(2).then(properties)); + } + return traversal; + } + } 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 4f70d51cb..241c42215 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 @@ -30,7 +30,6 @@ import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory; import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.AbstractType; import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YAtomicType; import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YBeanType; -import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YSeqType; import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YTypedPropertyImpl; import org.springframework.ide.vscode.commons.yaml.schema.YTypeUtil; import org.springframework.ide.vscode.commons.yaml.schema.YValueHint; @@ -112,6 +111,8 @@ public class ManifestYmlSchema implements YamlSchema { AbstractType application = f.ybean("Application"); application.require(this::verify_heatth_check_http_end_point_constraint); + application.require( + ManifestConstraints.mutuallyExclusive("routes", "domain", "domains", "host", "hosts", "no-hostname")); YAtomicType t_path = f.yatomic("Path"); YAtomicType t_buildpack = f.yatomic("Buildpack"); @@ -127,7 +128,6 @@ public class ManifestYmlSchema implements YamlSchema { } YAtomicType t_domain = f.yatomic("Domain"); - t_domain.require(ManifestConstraints.exclusiveWith("routes")); if (domainsProvider != null) { t_domain.addHintProvider(domainsProvider); t_domain.parseWith(ManifestYmlValueParsers.fromValueHints(domainsProvider, t_domain, ManifestYamlSchemaProblemsTypes.UNKNOWN_DOMAIN_PROBLEM)); @@ -172,38 +172,25 @@ public class ManifestYmlSchema implements YamlSchema { TOPLEVEL_TYPE.addProperty(f.yprop("applications", f.yseq(application))); TOPLEVEL_TYPE.addProperty("inherit", t_string, descriptionFor("inherit")); - YSeqType routesType = f.yseq(route); - routesType.require(ManifestConstraints.exclusiveWith("domain", "domains", "host", "hosts", "no-hostname")); - - YSeqType domainsType = f.yseq(t_domain); - domainsType.require(ManifestConstraints.exclusiveWith("routes")); - - YSeqType hostsType = f.yseq(t_string); - hostsType.require(ManifestConstraints.exclusiveWith("routes")); - - YAtomicType hostType = f.yatomic("String"); - hostType.require(ManifestConstraints.exclusiveWith("routes")); - - YAtomicType noHostType = f.yenum("boolean", "true", "false"); - noHostType.require(ManifestConstraints.exclusiveWith("routes")); + AbstractType t_host = f.yatomic("Host").parseWith(ValueParsers.NE_STRING); YTypedPropertyImpl[] props = { f.yprop("buildpack", t_buildpack), f.yprop("command", t_string), f.yprop("disk_quota", t_memory), f.yprop("domain", t_domain), - f.yprop("domains", domainsType), + f.yprop("domains", f.yseq(t_domain)), f.yprop("env", t_env), - f.yprop("host", hostType), - f.yprop("hosts", hostsType), + f.yprop("host", t_ne_string), + f.yprop("hosts", f.yseq(t_host)), f.yprop("instances", t_strictly_pos_integer), f.yprop("memory", t_memory), f.yprop("name", t_ne_string).isRequired(true), - f.yprop("no-hostname", noHostType), + f.yprop("no-hostname", t_boolean), f.yprop("no-route", t_boolean), f.yprop("path", t_path), f.yprop("random-route", t_boolean), - f.yprop("routes", routesType), + f.yprop("routes", f.yseq(route)), f.yprop("services", f.yseq(t_service)), f.yprop("stack", t_stack), f.yprop("timeout", t_pos_integer), 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 f56e5d805..79abdc17d 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 @@ -535,7 +535,7 @@ public class ManifestYamlEditorTest { editor.assertProblems( "no-hostname|Property cannot co-exist with property 'routes'", - "routes|Property cannot co-exist with property 'no-hostname'" + "routes|Property cannot co-exist with properties [no-hostname]" ); editor = harness.newEditor( @@ -549,23 +549,59 @@ public class ManifestYamlEditorTest { editor.assertProblems( "no-hostname|Property cannot co-exist with property 'routes'", - "routes|Property cannot co-exist with property 'no-hostname'" + "routes|Property cannot co-exist with properties [no-hostname]" ); -// editor = harness.newEditor( -// "no-hostname: true\n" + -// "routes:\n" + -// "- route: myapp.org" + -// "applications:\n" + -// "- name: my-app\n" -// ); -// editor.ignoreProblem("UnknownDomainProblem"); -// -// editor.assertProblems( -// "no-hostname|Property cannot co-exist with property 'routes'", -// "routes|Property cannot co-exist with property 'no-hostname'" -// ); - } + editor = harness.newEditor( + "no-hostname: true\n" + + "applications:\n" + + "- name: my-app\n" + + " no-hostname: true\n" + + " routes:\n" + + " - route: myapp.org" + ); + editor.ignoreProblem("UnknownDomainProblem"); + + editor.assertProblems( + "no-hostname|Property cannot co-exist with property 'routes'", + "no-hostname|Property cannot co-exist with property 'routes'", + "routes|Property cannot co-exist with properties [no-hostname]" + ); + + editor = harness.newEditor( + "no-hostname: true\n" + + "applications:\n" + + "- name: my-app\n" + + " host: some-app\n" + + " routes:\n" + + " - route: myapp.org" + ); + editor.ignoreProblem("UnknownDomainProblem"); + + editor.assertProblems( + "no-hostname|Property cannot co-exist with property 'routes'", + "host|Property cannot co-exist with property 'routes'", + "routes|Property cannot co-exist with properties [host, no-hostname]" + ); + + editor = harness.newEditor( + "no-hostname: true\n" + + "applications:\n" + + "- name: my-app\n" + + " routes:\n" + + " - route: myapp.org\n" + + "- name: app2\n" + + " routes:\n" + + " - route: my-route.org" + ); + editor.ignoreProblem("UnknownDomainProblem"); + + editor.assertProblems( + "no-hostname|Property cannot co-exist with property 'routes'", + "routes|Property cannot co-exist with properties [no-hostname]", + "routes|Property cannot co-exist with properties [no-hostname]" + ); +} @Test public void deprecatedHealthCheckTypeQuickfix() throws Exception { Editor editor = harness.newEditor(