Refactor / simplify how Yaml schema Constraints work:

- simplify access to DynamicSchemaContext.
 - allow constraints on non-map type nodes.

These changes pave the way to checking inertaction constraint on job-name
used in 'passed' attribute.
This commit is contained in:
Kris De Volder
2017-04-21 12:16:37 -07:00
parent 2181130e78
commit da66be7aec
9 changed files with 188 additions and 52 deletions

View File

@@ -1,3 +0,0 @@
#!/bin/bash
branch=`git rev-parse --abbrev-ref HEAD`
fly -t tools set-pipeline --var "branch=${branch}" --load-vars-from ${HOME}/.sts4-concourse-credentials.yml -p sts4-${branch} -c pipeline.yml

View File

@@ -252,10 +252,9 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
} }
//Check for other constraints attached to the type //Check for other constraints attached to the type
for (SchemaContextAware<Constraint> _constraint : typeUtil.getConstraints(type)) { for (Constraint constraint : typeUtil.getConstraints(type)) {
Constraint constraint = _constraint.withContext(dc);
if (constraint!=null) { if (constraint!=null) {
constraint.verify(dc.getDocument(), parent, map, type, foundProps, problems); constraint.verify(dc, parent, map, type, problems);
} }
} }
} }

View File

@@ -200,7 +200,7 @@ public class YTypeFactory {
} }
@Override @Override
public List<SchemaContextAware<Constraint>> getConstraints(YType type) { public List<Constraint> getConstraints(YType type) {
return ((AbstractType)type).getConstraints(); return ((AbstractType)type).getConstraints();
} }
}; };
@@ -218,7 +218,7 @@ public class YTypeFactory {
private Map<String, YTypedProperty> cachedPropertyMap; private Map<String, YTypedProperty> cachedPropertyMap;
private SchemaContextAware<Callable<Collection<YValueHint>>> hintProvider; private SchemaContextAware<Callable<Collection<YValueHint>>> hintProvider;
private List<SchemaContextAware<Constraint>> constraints = new ArrayList<>(2); private List<Constraint> constraints = new ArrayList<>(2);
public boolean isSequenceable() { public boolean isSequenceable() {
return false; return false;
@@ -282,7 +282,7 @@ public class YTypeFactory {
return ImmutableList.of(); return ImmutableList.of();
} }
public List<SchemaContextAware<Constraint>> getConstraints() { public List<Constraint> getConstraints() {
return ImmutableList.copyOf(constraints); return ImmutableList.copyOf(constraints);
} }
@@ -357,12 +357,12 @@ public class YTypeFactory {
return parser == null ? null : parser.withContext(dc); return parser == null ? null : parser.withContext(dc);
} }
public void require(SchemaContextAware<Constraint> dynamicConstraint) { public void require(Constraint dynamicConstraint) {
this.constraints.add(dynamicConstraint); this.constraints.add(dynamicConstraint);
} }
public void requireOneOf(String... properties) { public void requireOneOf(String... properties) {
this.constraints.add(SchemaContextAware.just(Constraints.requireOneOf(properties))); this.constraints.add(Constraints.requireOneOf(properties));
} }
public String[] getPropertyNames() { public String[] getPropertyNames() {

View File

@@ -46,5 +46,5 @@ public interface YTypeUtil {
* should be returned. * should be returned.
*/ */
YType inferMoreSpecificType(YType type, DynamicSchemaContext dc); YType inferMoreSpecificType(YType type, DynamicSchemaContext dc);
List<SchemaContextAware<Constraint>> getConstraints(YType type); List<Constraint> getConstraints(YType type);
} }

View File

@@ -14,6 +14,7 @@ import java.util.Set;
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector; import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
import org.springframework.ide.vscode.commons.util.text.IDocument; import org.springframework.ide.vscode.commons.util.text.IDocument;
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.YType;
import org.yaml.snakeyaml.nodes.MappingNode; import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node; import org.yaml.snakeyaml.nodes.Node;
@@ -32,12 +33,11 @@ public interface Constraint {
* constraint is satisfied. When the constrain is not satisfied they should report any * constraint is satisfied. When the constrain is not satisfied they should report any
* violations by adding problems to the provide {@link IProblemCollector}. * violations by adding problems to the provide {@link IProblemCollector}.
* *
* @param map The node being validated * @param node The node being validated
* @param type The inferred type of the node. * @param type The inferred type of the node.
* @param foundProps The properties this node defines. * @param foundProps The properties this node defines.
* @param problems Problem collector where to which the constraint should add the validation problems it finds. * @param problems Problem collector where to which the constraint should add the validation problems it finds.
*/ */
void verify(IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, void verify(DynamicSchemaContext dc, Node parent, Node node, YType type, IProblemCollector problems);
IProblemCollector problems);
} }

View File

@@ -24,6 +24,8 @@ import org.springframework.ide.vscode.commons.util.Assert;
import org.springframework.ide.vscode.commons.util.text.IDocument; import org.springframework.ide.vscode.commons.util.text.IDocument;
import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil; import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil;
import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems; import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems;
import org.springframework.ide.vscode.commons.yaml.schema.DynamicSchemaContext;
import org.springframework.ide.vscode.commons.yaml.schema.SchemaContextAware;
import org.springframework.ide.vscode.commons.yaml.schema.YType; import org.springframework.ide.vscode.commons.yaml.schema.YType;
import org.yaml.snakeyaml.nodes.MappingNode; import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node; import org.yaml.snakeyaml.nodes.Node;
@@ -62,23 +64,28 @@ public class Constraints {
} }
@Override @Override
public void verify(IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) { public void verify(DynamicSchemaContext dc, Node parent, Node _map, YType type, IProblemCollector problems) {
List<String> requiredProps = Arrays.asList(_requiredProps); IDocument doc = dc.getDocument();
long foundPropsCount = requiredProps.stream() Set<String> foundProps = dc.getDefinedProperties();
.filter(foundProps::contains) if (_map instanceof MappingNode) {
.count(); MappingNode map = (MappingNode) _map;
if (foundPropsCount==0) { List<String> requiredProps = Arrays.asList(_requiredProps);
if (!allowFewer) { long foundPropsCount = requiredProps.stream()
problems.accept(missingProperty( .filter(foundProps::contains)
"One of "+requiredProps+" is required for '"+type+"'", doc, parent, map)); .count();
} if (foundPropsCount==0) {
} else if (foundPropsCount>1) { if (!allowFewer) {
//Mark each of the found keys as a violation: problems.accept(missingProperty(
for (NodeTuple entry : map.getValue()) { "One of "+requiredProps+" is required for '"+type+"'", doc, parent, map));
String key = NodeUtil.asScalar(entry.getKeyNode()); }
if (key!=null && requiredProps.contains(key)) { } else if (foundPropsCount>1) {
problems.accept(problem(EXTRA_PROPERTY, //Mark each of the found keys as a violation:
"Only one of "+requiredProps+" should be defined for '"+type+"'", entry.getKeyNode())); for (NodeTuple entry : map.getValue()) {
String key = NodeUtil.asScalar(entry.getKeyNode());
if (key!=null && requiredProps.contains(key)) {
problems.accept(problem(EXTRA_PROPERTY,
"Only one of "+requiredProps+" should be defined for '"+type+"'", entry.getKeyNode()));
}
} }
} }
} }
@@ -87,14 +94,32 @@ public class Constraints {
public static Constraint deprecated(Function<String, String> messageFormatter, String... _deprecatedNames) { public static Constraint deprecated(Function<String, String> messageFormatter, String... _deprecatedNames) {
Set<String> deprecatedNames = ImmutableSet.copyOf(_deprecatedNames); Set<String> deprecatedNames = ImmutableSet.copyOf(_deprecatedNames);
return (IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) -> { return (DynamicSchemaContext dc, Node parent, Node _map, YType type, IProblemCollector problems) -> {
for (NodeTuple prop : map.getValue()) { if (_map instanceof MappingNode) {
Node keyNode = prop.getKeyNode(); MappingNode map = (MappingNode) _map;
String name = NodeUtil.asScalar(keyNode); for (NodeTuple prop : map.getValue()) {
if (deprecatedNames.contains(name)) { Node keyNode = prop.getKeyNode();
problems.accept(YamlSchemaProblems.deprecatedProperty(messageFormatter.apply(name), keyNode)); String name = NodeUtil.asScalar(keyNode);
if (deprecatedNames.contains(name)) {
problems.accept(YamlSchemaProblems.deprecatedProperty(messageFormatter.apply(name), keyNode));
}
} }
} }
}; };
} }
/**
* Deprecated because you shouldn't need to use this method to create a {@link SchemaContextAware} Constraint.
* A Constraint itself is already implicitly aware of the {@link DynamicSchemaContext} (i.e. it already receives
* the {@link DynamicSchemaContext} as a parameter to its verify method.
* <p>
* So instead of using this method to get a hold of the {@link DynamicSchemaContext} simply use the context
* passed to your constraint instead.
*/
@Deprecated
public static Constraint schemaContextAware(SchemaContextAware<Constraint> dispatcher) {
return (DynamicSchemaContext dc, Node parent, Node node, YType type, IProblemCollector problems) -> {
dispatcher.withContext(dc).verify(dc, parent, node, type, problems);
};
}
} }

View File

@@ -20,8 +20,8 @@ import java.util.Set;
import java.util.function.Function; import java.util.function.Function;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
import org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer; import org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer;
import org.springframework.ide.vscode.commons.languageserver.util.SimpleTextDocumentService;
import org.springframework.ide.vscode.commons.languageserver.util.SnippetBuilder; import org.springframework.ide.vscode.commons.languageserver.util.SnippetBuilder;
import org.springframework.ide.vscode.commons.util.Log; import org.springframework.ide.vscode.commons.util.Log;
import org.springframework.ide.vscode.commons.util.text.IDocument; import org.springframework.ide.vscode.commons.util.text.IDocument;
@@ -35,10 +35,12 @@ 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.YamlPathSegment;
import org.springframework.ide.vscode.commons.yaml.schema.BasicYValueHint; import org.springframework.ide.vscode.commons.yaml.schema.BasicYValueHint;
import org.springframework.ide.vscode.commons.yaml.schema.DynamicSchemaContext; 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.YTypeFactory; 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.AbstractType;
import org.springframework.ide.vscode.commons.yaml.schema.YTypedProperty; import org.springframework.ide.vscode.commons.yaml.schema.YTypedProperty;
import org.springframework.ide.vscode.commons.yaml.schema.YValueHint; import org.springframework.ide.vscode.commons.yaml.schema.YValueHint;
import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint;
import org.springframework.ide.vscode.concourse.util.CollectorUtil; import org.springframework.ide.vscode.concourse.util.CollectorUtil;
import org.springframework.ide.vscode.concourse.util.StaleFallbackCache; import org.springframework.ide.vscode.concourse.util.StaleFallbackCache;
import org.yaml.snakeyaml.Yaml; import org.yaml.snakeyaml.Yaml;
@@ -58,6 +60,14 @@ import com.google.common.collect.Multiset;
*/ */
public class ConcourseModel { public class ConcourseModel {
// /**
// * Verification of contraint: a job used in the 'passed' attribute of a step
// * must interact with the resource in question.
// */
// public Constraint jobHasInteractionWithResource() {
// return (IDocument doc, Node parent, Node node, YType type, Set<String> foundProps, IProblemCollector problems) -> {
// }
/** /**
* Wraps around a Node in the AST that represents a 'step' and * Wraps around a Node in the AST that represents a 'step' and
* provides methods for accessing information from the node. * provides methods for accessing information from the node.

View File

@@ -240,7 +240,7 @@ public class PipelineYmlSchema implements YamlSchema {
addProp(task, "outputs", f.yseq(t_output)); addProp(task, "outputs", f.yseq(t_output));
addProp(task, "run", t_command).isRequired(true); addProp(task, "run", t_command).isRequired(true);
addProp(task, "params", t_string_params); addProp(task, "params", t_string_params);
task.require((dc) -> { task.require(Constraints.schemaContextAware((DynamicSchemaContext dc) -> {
LanguageId languageId = dc.getDocument().getLanguageId(); LanguageId languageId = dc.getDocument().getLanguageId();
if (LanguageId.CONCOURSE_PIPELINE.equals(languageId)) { if (LanguageId.CONCOURSE_PIPELINE.equals(languageId)) {
Node parentImageDef = models.getParentPropertyNode("image", dc); Node parentImageDef = models.getParentPropertyNode("image", dc);
@@ -256,7 +256,7 @@ public class PipelineYmlSchema implements YamlSchema {
} else { } else {
return Constraints.requireAtMostOneOf("image_resource", "image"); return Constraints.requireAtMostOneOf("image_resource", "image");
} }
}); }));
AbstractType t_put_get_name = f.contextAware("Name", (dc) -> { AbstractType t_put_get_name = f.contextAware("Name", (dc) -> {
if (models.getParentPropertyNode("resource", dc)!=null) { if (models.getParentPropertyNode("resource", dc)!=null) {
@@ -287,17 +287,20 @@ public class PipelineYmlSchema implements YamlSchema {
addProp(putStep, "get_params", f.contextAware("GetParams", (dc) -> addProp(putStep, "get_params", f.contextAware("GetParams", (dc) ->
resourceTypes.getInParamsType(getResourceType("put", models, dc)) resourceTypes.getInParamsType(getResourceType("put", models, dc))
)); ));
putStep.require((dc) -> (IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) -> { putStep.require((DynamicSchemaContext dc, Node parent, Node _map, YType type, IProblemCollector problems) -> {
StepModel step = models.newStep("put", map); if (_map instanceof MappingNode) {
String resourceName = step.getResourceName(); MappingNode map = (MappingNode) _map;
if (resourceName!=null) { StepModel step = models.newStep("put", map);
ResourceModel resource = models.getResource(doc, resourceName); String resourceName = step.getResourceName();
if (resource!=null) { if (resourceName!=null) {
if ("git".equals(resource.getType()) && !resource.hasSourceProperty("branch")) { ResourceModel resource = models.getResource(dc.getDocument(), resourceName);
problems.accept(YamlSchemaProblems.schemaProblem( if (resource!=null) {
"Resource of type 'git' is used in a 'put' step, so it should define 'branch' attribute in its 'source', but it doesn't.", if ("git".equals(resource.getType()) && !resource.hasSourceProperty("branch")) {
step.getResourceNameNode() problems.accept(YamlSchemaProblems.schemaProblem(
)); "Resource of type 'git' is used in a 'put' step, so it should define 'branch' attribute in its 'source', but it doesn't.",
step.getResourceNameNode()
));
}
} }
} }
} }

View File

@@ -3116,6 +3116,108 @@ public class ConcourseEditorTest {
); );
} }
@Ignore @Test public void reconcilerJobFromPassedAttributeMustInteractWithResource() throws Exception {
Editor editor;
editor = harness.newEditor(
"resources:\n" +
"- name: version\n" +
" type: semver\n" +
"- name: source-repo\n" +
" type: git\n" +
"jobs:\n" +
"- name: build-it\n" +
" plan:\n" +
" - aggregate:\n" +
" # - put: version\n" +
" - get: source-repo\n" +
"- name: test-it\n" +
" plan:\n" +
" - get: source-repo\n" +
" passed:\n" +
" - build-it # <- good\n" +
" - get: version\n" +
" passed:\n" +
" - build-it # <- bad\n"
);
editor.assertProblems(
"build-it^ # <- bad|Job 'build-it' doesn't interact with the resource 'version'"
);
editor = harness.newEditor(
"resources:\n" +
"- name: version\n" +
" type: semver\n" +
"- name: source-repo\n" +
" type: git\n" +
"jobs:\n" +
"- name: build-it\n" +
" plan:\n" +
" - aggregate:\n" +
" - put: version\n" +
" # - get: source-repo\n" +
"- name: test-it\n" +
" plan:\n" +
" - get: source-repo\n" +
" passed:\n" +
" - build-it # <- bad\n" +
" - get: version\n" +
" passed:\n" +
" - build-it # <- good\n"
);
editor.assertProblems(
"build-it^ # <- bad|Job 'build-it' doesn't interact with the resource 'source-repo'"
);
//Check that we find interactions in steps that are at the top-level of the plan:
editor = harness.newEditor(
"resources:\n" +
"- name: version\n" +
" type: semver\n" +
"- name: source-repo\n" +
" type: git\n" +
"jobs:\n" +
"- name: build-it\n" +
" plan:\n" +
" - aggregate:\n" +
" - put: version\n" +
" - get: source-repo\n" +
"- name: test-it\n" +
" plan:\n" +
" - get: source-repo\n" +
" passed:\n" +
" - build-it\n" +
" - get: version\n" +
" passed:\n" +
" - build-it\n"
);
editor.assertProblems(/*NONE*/);
//Check that we find interactions in steps that are nested in other steps
editor = harness.newEditor(
"resources:\n" +
"- name: version\n" +
" type: semver\n" +
"- name: source-repo\n" +
" type: git\n" +
"jobs:\n" +
"- name: build-it\n" +
" plan:\n" +
" - put: version\n" +
" - get: source-repo\n" +
"- name: test-it\n" +
" plan:\n" +
" - get: source-repo\n" +
" passed:\n" +
" - build-it\n" +
" - get: version\n" +
" passed:\n" +
" - build-it\n"
);
editor.assertProblems(/*NONE*/);
}
////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////////////
private void assertContextualCompletions(String conText, String textBefore, String... textAfter) throws Exception { private void assertContextualCompletions(String conText, String textBefore, String... textAfter) throws Exception {