From 7e53e6b36611b537ea15804d53e984e04d9bcced Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 14 Jul 2017 16:52:57 -0700 Subject: [PATCH] Some tweaks to the contraints checking for releases. --- .../ide/vscode/bosh/BoshConstraints.java | 43 +++++++++++++++++++ .../bosh/BoshDeploymentManifestSchema.java | 12 +++--- .../ide/vscode/concourse/BoshEditorTest.java | 17 +++++++- .../ide/vscode/commons/yaml/ast/NodeUtil.java | 12 ++++++ .../yaml/schema/constraints/Constraints.java | 14 +++++- 5 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshConstraints.java diff --git a/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshConstraints.java b/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshConstraints.java new file mode 100644 index 000000000..5c80ef16d --- /dev/null +++ b/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshConstraints.java @@ -0,0 +1,43 @@ +/******************************************************************************* + * Copyright (c) 2017 Pivotal, Inc. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.bosh; + +import static org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems.MISSING_PROPERTY; +import static org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems.problem; + +import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector; +import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil; +import org.springframework.ide.vscode.commons.yaml.schema.DynamicSchemaContext; +import org.springframework.ide.vscode.commons.yaml.schema.YType; +import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint; +import org.yaml.snakeyaml.nodes.Node; +import org.yaml.snakeyaml.nodes.NodeTuple; + +public class BoshConstraints { + + public static final Constraint SHA1_REQUIRED_FOR_HTTP_URL = new Constraint() { + + @Override + public void verify(DynamicSchemaContext dc, Node parent, Node node, YType type, IProblemCollector problems) { + NodeTuple urlProp = NodeUtil.getPropertyTuple(node, "url"); + if (urlProp!=null) { + String url = NodeUtil.asScalar(urlProp.getValueNode()); + if (url!=null && url.startsWith("http")) { + Node sha1 = NodeUtil.getProperty(node, "sha1"); + if (sha1==null) { + problems.accept(problem(MISSING_PROPERTY, "'sha1' is required when the 'url' is http(s)", urlProp.getKeyNode())); + } + } + } + } + }; + +} diff --git a/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshDeploymentManifestSchema.java b/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshDeploymentManifestSchema.java index 79d53c016..6b4189514 100644 --- a/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshDeploymentManifestSchema.java +++ b/headless-services/bosh-language-server/src/main/java/org/springframework/ide/vscode/bosh/BoshDeploymentManifestSchema.java @@ -142,15 +142,15 @@ public class BoshDeploymentManifestSchema implements YamlSchema { YBeanType t_release = f.ybean("Release"); addProp(t_release, "name", t_ne_string).isPrimary(true); - addProp(t_release, "version", t_version).isRequired(true); //TODO: Is it really required? Maybe not id we use url + sha - //See: https://github.com/cloudfoundry/docs-bosh/issues/330 + addProp(t_release, "version", t_version); + //TODO: the checking here is just 'my best guess'. Unclarity remains: + // See: https://github.com/cloudfoundry/docs-bosh/issues/330 addProp(t_release, "url", t_url); addProp(t_release, "sha1", t_ne_string); addProp(v2Schema, "releases", f.yseq(t_release)).isRequired(true); - //TODO: these extra checks disabled because we aren't sure they are valid - // See: https://github.com/cloudfoundry/docs-bosh/issues/330 - // t_release.require(Constraints.requireOneOf("url", "version")); - // t_release.require(Constraints.mutuallyExclusive("version", "sha1")); + t_release.require(Constraints.requireAtLeastOneOf("url", "version")); //allthough docs seem to imply you shouldn't define both url and version.. + //... it seems bosh tolerates it. + t_release.require(BoshConstraints.SHA1_REQUIRED_FOR_HTTP_URL); YBeanType t_stemcell = f.ybean("Stemcell"); addProp(t_stemcell, "alias", t_ne_string).isRequired(true); diff --git a/headless-services/bosh-language-server/src/test/java/org/springframework/ide/vscode/concourse/BoshEditorTest.java b/headless-services/bosh-language-server/src/test/java/org/springframework/ide/vscode/concourse/BoshEditorTest.java index f9e86bccf..a968591b1 100644 --- a/headless-services/bosh-language-server/src/test/java/org/springframework/ide/vscode/concourse/BoshEditorTest.java +++ b/headless-services/bosh-language-server/src/test/java/org/springframework/ide/vscode/concourse/BoshEditorTest.java @@ -232,7 +232,22 @@ public class BoshEditorTest { " version: <*>" ); } - + + @Test public void releasesBlockSha1RequiredForHttpUrl() throws Exception { + Editor editor = harness.newEditor( + "releases:\n" + + "- name: some-release\n" + + " url: https://my.releases.com/funky.tar.gz\n" + + "- name: other-relase\n" + + " url: file:///root/releases/a-nice-file.tar.gz\n" + + "#x" + ); + editor.assertProblems( + "url|'sha1' is required when the 'url' is http(s)", + "x|are required" + ); + } + @Test public void releasesBlockReconcileAndHovers() throws Exception { Editor editor = harness.newEditor( "releases:\n" + diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/ast/NodeUtil.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/ast/NodeUtil.java index e6aa0d844..43d12b476 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/ast/NodeUtil.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/ast/NodeUtil.java @@ -116,6 +116,18 @@ public class NodeUtil { return null; } + public static NodeTuple getPropertyTuple(Node node, String propName) { + if (node instanceof MappingNode) { + for (NodeTuple entry : ((MappingNode)node).getValue()) { + String key = NodeUtil.asScalar(entry.getKeyNode()); + if (propName.equals(key)) { + return entry; + } + } + } + return null; + } + public static Node getProperty(Node node, String propName) { if (node instanceof MappingNode) { for (NodeTuple entry : ((MappingNode)node).getValue()) { diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java index 71350b6e9..c26f580b9 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java @@ -49,17 +49,28 @@ public class Constraints { public static Constraint requireAtMostOneOf(String... properties) { return new RequireOneOf(properties).allowFewer(true); } + + public static Constraint requireAtLeastOneOf(String... properties) { + return new RequireOneOf(properties).allowMultiple(true); + } + static private class RequireOneOf implements Constraint { private final String[] _requiredProps; private boolean allowFewer = false; + private boolean allowMultiple = false; public RequireOneOf(String[] properties) { Assert.isLegal(properties.length>1); this._requiredProps = properties; } + public Constraint allowMultiple(boolean b) { + this.allowMultiple = b; + return this; + } + public Constraint allowFewer(boolean b) { this.allowFewer = b; return this; @@ -80,7 +91,7 @@ public class Constraints { problems.accept(missingProperty( "One of "+requiredProps+" is required for '"+type+"'", doc, parent, map)); } - } else if (foundPropsCount>1) { + } else if (foundPropsCount>1 && !allowMultiple) { //Mark each of the found keys as a violation: for (NodeTuple entry : map.getValue()) { String key = NodeUtil.asScalar(entry.getKeyNode()); @@ -144,4 +155,5 @@ public class Constraints { } }; } + } \ No newline at end of file