Fix PT-140783423 'branch' only required if used in 'put' step.

This commit is contained in:
Kris De Volder
2017-04-06 14:14:22 -07:00
parent e4c16348f2
commit b42c5d793a
5 changed files with 165 additions and 26 deletions

View File

@@ -113,4 +113,16 @@ public class NodeUtil {
return null;
}
public static Node getProperty(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.getValueNode();
}
}
}
return null;
}
}

View File

@@ -28,12 +28,14 @@ import org.springframework.ide.vscode.commons.yaml.ast.YamlParser;
import org.springframework.ide.vscode.commons.yaml.path.ASTRootCursor;
import org.springframework.ide.vscode.commons.yaml.path.NodeCursor;
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.schema.YTypeFactory;
import org.springframework.ide.vscode.commons.yaml.schema.YValueHint;
import org.springframework.ide.vscode.concourse.util.CollectorUtil;
import org.springframework.ide.vscode.concourse.util.StaleFallbackCache;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.error.YAMLException;
import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
import com.google.common.collect.ImmutableMultiset;
@@ -46,6 +48,50 @@ import com.google.common.collect.Multiset;
* and completion engine).
*/
public class ConcourseModel {
/**
* Wraps around a Node in the AST that represents a 'step' and
* provides methods for accessing information from the node.
*/
public static class StepModel {
private final String stepType;
private final MappingNode step;
public StepModel(String stepType, MappingNode step) {
this.stepType = stepType;
this.step = step;
}
public Node getResourceNameNode() {
Node node = NodeUtil.getProperty(step, "resource");
return node!=null ? node : NodeUtil.getProperty(step, stepType);
}
public String getResourceName() {
return NodeUtil.asScalar(getResourceNameNode());
}
}
public static class ResourceModel {
private final Node resource;
public ResourceModel(Node resource) {
this.resource = resource;
}
public String getType() {
return NodeUtil.getScalarProperty(resource, "type");
}
public boolean hasSourceProperty(String propName) {
YamlPath path = new YamlPath(YamlPathSegment.valueAt("source"), YamlPathSegment.keyAt(propName));
return path.traverseAmbiguously(resource).findFirst().isPresent();
}
}
public static final YamlPath JOB_NAMES_PATH = new YamlPath(
anyChild(),
valueAt("jobs"),
@@ -113,16 +159,25 @@ public class ConcourseModel {
* was never successfully parsed.
*/
public String getResourceType(IDocument doc, String resourceName) {
ResourceModel resource = getResource(doc, resourceName);
if (resource!=null) {
return resource.getType();
}
return null;
}
public ResourceModel getResource(IDocument doc, String resourceName) {
return getFromAst(doc, (ast) -> {
Node resource = RESOURCES_PATH.traverseAmbiguously(new ASTRootCursor(ast))
.map((cursor) -> ((NodeCursor)cursor).getNode())
.filter((resourceNode) -> resourceName.equals(NodeUtil.getScalarProperty(resourceNode, "name")))
.findFirst().orElse(null);
if (resource!=null) {
return NodeUtil.getScalarProperty(resource, "type");
return new ResourceModel(resource);
}
return null;
});
}
/**
@@ -220,4 +275,8 @@ public class ConcourseModel {
return astTypes;
}
public StepModel newStep(String stepType, MappingNode stepNode) {
return new StepModel(stepType, stepNode);
}
}

View File

@@ -15,16 +15,19 @@ import java.util.Set;
import java.util.stream.Collectors;
import org.springframework.ide.vscode.commons.languageserver.LanguageIds;
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
import org.springframework.ide.vscode.commons.util.MimeTypes;
import org.springframework.ide.vscode.commons.util.Renderable;
import org.springframework.ide.vscode.commons.util.Renderables;
import org.springframework.ide.vscode.commons.util.ValueParseException;
import org.springframework.ide.vscode.commons.util.ValueParser;
import org.springframework.ide.vscode.commons.util.ValueParsers;
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.YamlFileAST;
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.reconcile.YamlSchemaProblems;
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.YType;
@@ -39,6 +42,9 @@ 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.YamlSchema;
import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraints;
import org.springframework.ide.vscode.concourse.ConcourseModel.ResourceModel;
import org.springframework.ide.vscode.concourse.ConcourseModel.StepModel;
import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
import reactor.core.publisher.Flux;
@@ -200,14 +206,14 @@ public class PipelineYmlSchema implements YamlSchema {
resourceTypes.getSourceType(getResourceTypeTag(models, dc))
);
AbstractType resource = f.ybean("Resource");
addProp(resource, "name", resourceNameDef).isRequired(true);
addProp(resource, "type", t_resource_type_name).isRequired(true);
addProp(resource, "source", resourceSource);
addProp(resource, "check_every", t_duration);
AbstractType t_resource = f.ybean("Resource");
addProp(t_resource, "name", resourceNameDef).isRequired(true);
addProp(t_resource, "type", t_resource_type_name).isRequired(true);
addProp(t_resource, "source", resourceSource);
addProp(t_resource, "check_every", t_duration);
AbstractType t_image_resource = f.ybean("ImageResource");
for (YTypedProperty p : resource.getProperties()) {
for (YTypedProperty p : t_resource.getProperties()) {
if (!"name".equals(p.getName())) {
t_image_resource.addProperty(p);
}
@@ -285,6 +291,21 @@ public class PipelineYmlSchema implements YamlSchema {
addProp(putStep, "get_params", f.contextAware("GetParams", (dc) ->
resourceTypes.getInParamsType(getResourceType("put", models, dc))
));
putStep.require((dc) -> (IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) -> {
StepModel step = models.newStep("put", map);
String resourceName = step.getResourceName();
if (resourceName!=null) {
ResourceModel resource = models.getResource(doc, resourceName);
if (resource!=null) {
if ("git".equals(resource.getType()) && !resource.hasSourceProperty("branch")) {
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()
));
}
}
}
});
YBeanType taskStep = f.ybean("TaskStep");
addProp(taskStep, "task", t_ne_string);
addProp(taskStep, "file", t_string);
@@ -343,7 +364,7 @@ public class PipelineYmlSchema implements YamlSchema {
addProp(group, "resources", f.yseq(t_resource_name));
addProp(group, "jobs", f.yseq(t_job_name));
addProp(TOPLEVEL_TYPE, "resources", f.yseq(resource));
addProp(TOPLEVEL_TYPE, "resources", f.yseq(t_resource));
addProp(TOPLEVEL_TYPE, "jobs", f.yseq(job));
addProp(TOPLEVEL_TYPE, "resource_types", f.yseq(resourceType));
addProp(TOPLEVEL_TYPE, "groups", f.yseq(group));
@@ -360,7 +381,7 @@ public class PipelineYmlSchema implements YamlSchema {
{
AbstractType source = f.ybean("GitSource");
addProp(source, "uri", t_string).isRequired(true);
addProp(source, "branch", t_string).isRequired(true);
addProp(source, "branch", t_string); //It's more complicated than that! Its only required in 'put' step. So we'll check this as a contrain in put steps!
addProp(source, "private_key", t_ne_string);
addProp(source, "username", t_ne_string);
addProp(source, "password", t_string);
@@ -581,11 +602,21 @@ public class PipelineYmlSchema implements YamlSchema {
return driver!=null ? driver : "s3";
}
private String getResourceType(String resourceNameProp, ConcourseModel models, DynamicSchemaContext dc) {
String resourceName = getParentPropertyValue("resource", models, dc);
private Node getResourceNameNode(String resourceNameProp, DynamicSchemaContext dc) {
Node resourceName = getParentPropertyNode("resource", models, dc);
if (resourceName==null) {
resourceName = getParentPropertyValue(resourceNameProp, models, dc);
resourceName = getParentPropertyNode(resourceNameProp, models, dc);
}
return resourceName;
}
private String getResourceName(String resourceNameProp, DynamicSchemaContext dc) {
Node resourceName = getResourceNameNode(resourceNameProp, dc);
return NodeUtil.asScalar(resourceName);
}
private String getResourceType(String resourceNameProp, ConcourseModel models, DynamicSchemaContext dc) {
String resourceName = getResourceName(resourceNameProp, dc);
if (resourceName!=null) {
return models.getResourceType(dc.getDocument(), resourceName);
}

View File

@@ -1 +1,3 @@
*Required.* The branch to track.
The branch to track. This is *optional* if the resource is
only used in `get` steps (default value in this case is `master`).
However, it is *required* when used in a `put` step.

View File

@@ -21,7 +21,6 @@ import java.util.stream.Collectors;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.springframework.ide.vscode.commons.languageserver.LanguageIds;
import org.springframework.ide.vscode.commons.util.IOUtil;
@@ -709,7 +708,10 @@ public class ConcourseEditorTest {
" - build\n"
);
editor.assertProblems("not-a-job|does not exist");
editor.assertProblems(
"build-artefact|should define 'branch'",
"not-a-job|does not exist"
);
}
@Test
@@ -726,7 +728,7 @@ public class ConcourseEditorTest {
" - get: git-repo\n" +
" - task: run-build\n" +
" file: tasks/some-task.yml\n" +
" - put: build-artefact\n" +
" - put: build-artefact # <- bad\n" +
"- name: test\n" +
" plan:\n" +
" - get: git-repo\n" +
@@ -737,6 +739,7 @@ public class ConcourseEditorTest {
);
editor.assertProblems(
"build-artefact^ # <- bad|should define 'branch'",
"bogus-job|does not exist",
"not-a-resource|does not exist"
);
@@ -1117,6 +1120,9 @@ public class ConcourseEditorTest {
"resources:\n" +
"- name: my-git\n" +
" type: git\n" +
" source:\n" +
" uri: some-uri\n" +
" branch: master\n" +
"jobs:\n" +
"- name: do-stuff\n" +
" plan:\n" +
@@ -1129,6 +1135,9 @@ public class ConcourseEditorTest {
"resources:\n" +
"- name: my-git\n" +
" type: git\n" +
" source:\n" +
" uri: some-uri\n" +
" branch: master\n" +
"jobs:\n" +
"- name: do-stuff\n" +
" plan:\n" +
@@ -1313,16 +1322,6 @@ public class ConcourseEditorTest {
);
editor.assertProblems("source|'uri' is required");
//addProp(gitSource, "branch", t_string).isRequired(true);
editor = harness.newEditor(
"resources:\n" +
"- name: foo\n" +
" type: git\n" +
" source:\n" +
" uri: https://yada"
);
editor.assertProblems("source|'branch' is required");
//addProp(group, "name", t_ne_string).isRequired(true);
editor = harness.newEditor(
"groups:\n" +
@@ -1331,6 +1330,42 @@ public class ConcourseEditorTest {
editor.assertProblems("-^ jobs: []|'name' is required");
}
@Test public void gitBranchRequiredInPutStep() throws Exception {
Editor editor;
editor = harness.newEditor(
"resources:\n" +
"- name: repo\n" +
" type: git\n" +
" source:\n" +
" uri: git@github.com/johny-coder/test-repo\n" +
"jobs:\n" +
"- name: do-stuff\n" +
" plan:\n" +
" - get: repo\n" +
" - put: repo # <- bad\n"
);
editor.assertProblems(
"repo^ # <- bad|should define 'branch'"
);
editor = harness.newEditor(
"resources:\n" +
"- name: repo\n" +
" type: git\n" +
" source:\n" +
" uri: git@github.com/johny-coder/test-repo\n" +
"jobs:\n" +
"- name: do-stuff\n" +
" plan:\n" +
" - get: repo\n" +
" - put: blah\n" +
" resource: repo # <- bad\n"
);
editor.assertProblems(
"repo^ # <- bad|should define 'branch'"
);
}
@Test public void dockerImageResourceSourceReconcileAndHovers() throws Exception {
Editor editor;