From 6945fe18fc0f8d767add6a25838dcee30521840d Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Tue, 18 May 2021 15:07:52 -0700 Subject: [PATCH] Fix https://github.com/spring-projects/sts4/issues/639 Basic support for using glob pattern in concourse group declarations. --- .../vscode/commons/util/EnumValueParser.java | 6 +- .../ide/vscode/commons/util/SimpleGlob.java | 118 ++++++++++++++++++ .../commons/yaml/schema/YTypeFactory.java | 43 +++++++ .../vscode/commons/util/SimpleGlobTests.java | 64 ++++++++++ .../ide/vscode/concourse/ConcourseModel.java | 13 +- .../vscode/concourse/PipelineYmlSchema.java | 10 +- .../vscode/concourse/ConcourseEditorTest.java | 78 +++++++++++- 7 files changed, 328 insertions(+), 4 deletions(-) create mode 100644 headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/util/SimpleGlob.java create mode 100644 headless-services/commons/commons-yaml/src/test/java/org/springframework/ide/vscode/commons/util/SimpleGlobTests.java diff --git a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java index 251eb04bf..e6a6157f8 100644 --- a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java +++ b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java @@ -73,13 +73,17 @@ public class EnumValueParser implements ValueParser { PartialCollection values = this.values.get(); // If values is not fully known then just assume the str is acceptable. - if (values == null || !values.isComplete() || values.getElements().contains(str)) { + if (values == null || !values.isComplete() || hasMatchingValue(str, values.getElements())) { return str; } else { throw errorOnParse(createErrorMessage(str, values.getElements())); } } + protected boolean hasMatchingValue(String str, Collection values) { + return values.contains(str); + } + protected String createBlankTextErrorMessage() { return "'" + typeName + "'" + " cannot be blank."; } diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/util/SimpleGlob.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/util/SimpleGlob.java new file mode 100644 index 000000000..4fbcfc400 --- /dev/null +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/util/SimpleGlob.java @@ -0,0 +1,118 @@ +package org.springframework.ide.vscode.commons.util; + +import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; +import java.util.regex.Pattern; + +import org.springframework.util.Assert; + + +/** + * Glob matcher to use for https://github.com/spring-projects/sts4/issues/639 + *

+ * The matcher only handles a simple subset of the full glob syntax. It is conservative and detects + * where a pattern looks 'too complex to handle'. + * + */ +public abstract class SimpleGlob { + + /** + * Matches patterns containing one or more '*'. + */ + private static class MultiStarGlob extends SimpleGlob { + + String prefix; + String[] middle; + String postfix; + + public MultiStarGlob(String pattern) { + super(pattern); + Assert.isTrue(pattern.contains("*"), "Pattern must contain at least 1 '*'"); + List parts = new ArrayList<>(); + int scan = 0; + int star = pattern.indexOf('*', scan); + while (star>=0) { + parts.add(pattern.substring(scan, star)); + scan = star+1; + star = pattern.indexOf('*', scan); + } + postfix = pattern.substring(scan); + middle = parts.subList(1, parts.size()).toArray(String[]::new); + prefix = parts.get(0); + } + + @Override + public Match match(String value) { + if (!value.startsWith(prefix)) { + return Match.FAIL; + } + int scan = prefix.length(); + for (String part : middle) { + scan = value.indexOf(part, scan); + if (scan<0) { + return Match.FAIL; + } else { + //consume matched part so we cannot use that in matching next parts again. + scan += part.length(); + } + } + if (postfix.length() > value.length()-scan) { + // there's not enough data left to be able to find the postfix + return Match.FAIL; + } + if (!value.endsWith(postfix)) { + return Match.FAIL; + } + return Match.SUCCESS; + } + + } + + static final Pattern IS_COMPLEX = Pattern.compile(".*[\\[\\]\\{\\}?!].*"); + private String pattern; + + public static enum Match { + SUCCESS, + FAIL, + UNKNOWN + } + + /** + * Private, use 'create' method instead. + */ + private SimpleGlob(String pattern) { + this.pattern = pattern; + } + + @Override + public String toString() { + return "SimpleGlob("+pattern+")"; + } + + public abstract Match match(String value); + + public static SimpleGlob create(String pattern) { + if (IS_COMPLEX.matcher(pattern).matches()) { + return fakeMatcher(pattern); + } + if (pattern.contains("*")) { + return new MultiStarGlob(pattern); + } else { + //Not a real pattern, just a simple string to match with 'equals' + return new SimpleGlob(pattern) { + @Override public Match match(String value) { + return pattern.equals(value) ? Match.SUCCESS : Match.FAIL; + } + }; + } + } + + private static SimpleGlob fakeMatcher(String pattern) { + return new SimpleGlob(pattern) { + @Override public Match match(String value) { + return Match.UNKNOWN; + } + }; + } +} diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java index f0ccd1998..32985dc82 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java @@ -35,6 +35,8 @@ import org.springframework.ide.vscode.commons.util.EnumValueParser; import org.springframework.ide.vscode.commons.util.PartialCollection; import org.springframework.ide.vscode.commons.util.Renderable; import org.springframework.ide.vscode.commons.util.Renderables; +import org.springframework.ide.vscode.commons.util.SimpleGlob; +import org.springframework.ide.vscode.commons.util.SimpleGlob.Match; import org.springframework.ide.vscode.commons.util.ValueParser; import org.springframework.ide.vscode.commons.yaml.ast.YamlFileAST; import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems; @@ -1091,6 +1093,47 @@ public class YTypeFactory { return yenum(name, (dc) -> errorMessageFormatter, values); } + /** + * Like an enum, checks a given reference matches one of a list of known values. Except that the meaning of 'matching' is based on glob pattern matching + * instead of simple string equality. + *

+ * See: https://github.com/spring-projects/sts4/issues/639 + */ + public AbstractType yGlobEnum(String name, BiFunction, String> errorMessageFormatter , SchemaContextAware> values) { + return yGlobEnum(name, (dc) -> errorMessageFormatter, values); + } + + public YAtomicType yGlobEnum(String name, SchemaContextAware, String>> errorMessageFormatter, SchemaContextAware> values) { + YAtomicType t = yatomic(name); + t.setHintProvider((dc) -> { + return PartialCollection.compute(() -> values.withContext(dc)) + .map(BasicYValueHint::new); + }); + t.parseWith((DynamicSchemaContext dc) -> { + EnumValueParser enumParser = new EnumValueParser(name, values.withContext(dc)) { + @Override + protected String createErrorMessage(String parseString, Collection values) { + try { + return errorMessageFormatter.withContext(dc).apply(parseString, values); + } catch (Exception e) { + return super.createErrorMessage(parseString, values); + } + } + + protected boolean hasMatchingValue(String stringOrGlob, Collection values) { + SimpleGlob glob = SimpleGlob.create(stringOrGlob); + //Note we accept 'Match.UNKNOWN' as if it is a proper match. This is the conservative thing to do + // in this situation to avoid false positives (i.e. we only report an error if we are *sure* that no + // value matches. + return values.stream().anyMatch(s -> glob.match(s)!=Match.FAIL); + } + }; + return enumParser; + }); + return t; + + } + public static Collection values(Collection hints) { return hints == null ? null : hints.stream().map(YValueHint::getValue).collect(Collectors.toList()); } diff --git a/headless-services/commons/commons-yaml/src/test/java/org/springframework/ide/vscode/commons/util/SimpleGlobTests.java b/headless-services/commons/commons-yaml/src/test/java/org/springframework/ide/vscode/commons/util/SimpleGlobTests.java new file mode 100644 index 000000000..1c3cee8e8 --- /dev/null +++ b/headless-services/commons/commons-yaml/src/test/java/org/springframework/ide/vscode/commons/util/SimpleGlobTests.java @@ -0,0 +1,64 @@ +package org.springframework.ide.vscode.commons.util; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.springframework.ide.vscode.commons.util.SimpleGlob.Match; + +public class SimpleGlobTests { + + @Test + public void testing() throws Exception { + expect("*", "anything", Match.SUCCESS); + expect("*", "", Match.SUCCESS); + expect("abc**aaa", "abcXXaaa", Match.SUCCESS); + + expect("start-*", "start-", Match.SUCCESS); + expect("start-*", "start-andmore", Match.SUCCESS); + expect("start-*", "start", Match.FAIL); + + expect("*-end", "-end", Match.SUCCESS); + expect("*-end", "andmore-end", Match.SUCCESS); + expect("*-end", "end", Match.FAIL); + + expect("start-*-end", "start--end", Match.SUCCESS); + expect("start-*-end", "start-middle-end", Match.SUCCESS); + expect("start-*-end", "start-end", Match.FAIL); + expect("start-*-end", "start-middle", Match.FAIL); + expect("start-*-end", "-middle-end", Match.FAIL); + expect("start-*-end", "astart-middle-end", Match.FAIL); + expect("start-*-end", "start-middle-enda", Match.FAIL); + } + + @Test + public void multiStar() throws Exception { + expect("start-*part*art", "start-part", Match.FAIL); + expect("start-*-part-*-end", "start-XXX-part-YYY-end", Match.SUCCESS); + expect("start-*-part-*-end", "start--part--end", Match.SUCCESS); + expect("start-*part*art", "start-part-part", Match.SUCCESS); + } + + @Test + public void complexCases() throws Exception { + expectUnkown( + "?at", + "[abc]at", + "[!abc]at", + "[a-c]at", + "[!a-c]at", + "{cat,bat,[fr]at}" + ); + } + + private void expectUnkown(String... patterns) { + for (String p : patterns) { + expect(p, "whatever", Match.UNKNOWN); + } + } + + private void expect(String pattern, String data, Match expected) { + Match actual = SimpleGlob.create(pattern).match(data); + assertEquals(expected, actual); + } + +} diff --git a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java index c3c187b1f..dba522aff 100644 --- a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java +++ b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/ConcourseModel.java @@ -16,7 +16,9 @@ import static org.springframework.ide.vscode.commons.yaml.path.YamlPathSegment.v import java.util.Arrays; import java.util.Collection; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Map.Entry; import java.util.Optional; import java.util.Set; @@ -30,6 +32,8 @@ import org.springframework.ide.vscode.commons.languageserver.util.SnippetBuilder import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.util.CollectorUtil; import org.springframework.ide.vscode.commons.util.Log; +import org.springframework.ide.vscode.commons.util.SimpleGlob; +import org.springframework.ide.vscode.commons.util.SimpleGlob.Match; import org.springframework.ide.vscode.commons.util.Streams; import org.springframework.ide.vscode.commons.util.StringUtil; import org.springframework.ide.vscode.commons.util.text.IDocument; @@ -136,10 +140,17 @@ public class ConcourseModel { public final void jobAssignmentIsComplete(DynamicSchemaContext dc, Node parent, Node node, YType type, IProblemCollector problems) { Multiset assignedJobs = getStringsFromAst(dc.getDocument(), JOBS_ASSIGNED_TO_GROUPS); if (assignedJobs!=null && !assignedJobs.isEmpty()) { + Map assignedJobMatchers = new HashMap<>(); + for (String jobPattern : assignedJobs) { + if (!assignedJobMatchers.containsKey(jobPattern)) { + assignedJobMatchers.put(jobPattern, SimpleGlob.create(jobPattern)); + } + } + getJobNameNodes(dc).forEach(jobDefName -> { String name = NodeUtil.asScalar(jobDefName); if (StringUtil.hasText(name)) { //'not assigned to a group' errors for empty names are a bit silly, so avoid that - if (!assignedJobs.contains(name)) { + if (!assignedJobMatchers.values().stream().anyMatch(jobPat -> jobPat.match(name)!=Match.FAIL)) { problems.accept(YamlSchemaProblems.schemaProblem("'"+name+"' belongs to no group", jobDefName)); } } diff --git a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/PipelineYmlSchema.java b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/PipelineYmlSchema.java index 7afb08926..7c448404a 100644 --- a/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/PipelineYmlSchema.java +++ b/headless-services/concourse-language-server/src/main/java/org/springframework/ide/vscode/concourse/PipelineYmlSchema.java @@ -217,6 +217,14 @@ public class PipelineYmlSchema implements YamlSchema { return models.getJobNames(dc); } ).require(models::passedJobHasInteractionWithResource); + AbstractType t_job_name_pattern = f.yGlobEnum("Job Name Pattern", + (parseString, validValues) -> { + return "The '"+parseString+"' Job does not match any existing job: "+validValues; + }, + (DynamicSchemaContext dc) -> { + return models.getJobNames(dc); + } + ); YAtomicType t_resource_name_def = f.yatomic("Resource Name"); t_resource_name_def.parseWith(ConcourseValueParsers.resourceNameDef(models)); @@ -462,7 +470,7 @@ public class PipelineYmlSchema implements YamlSchema { AbstractType group = f.ybean("Group"); addProp(group, "name", t_group_name_def).isPrimary(true); addProp(group, "resources", f.yseq(t_resource_name)); - addProp(group, "jobs", f.yseq(t_job_name)); + addProp(group, "jobs", f.yseq(t_job_name_pattern)); YType t_background_image_def = f.yatomic("Background Image") .parseWith(ValueParsers.NE_STRING); diff --git a/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java b/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java index 8af1f35f1..cc2baacf1 100644 --- a/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java +++ b/headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java @@ -74,6 +74,82 @@ public class ConcourseEditorTest { serverInitializer.setMaxCompletions(100); } + @Test public void GH_639_globStar() throws Exception { + Editor editor = harness.newEditor( + "groups:\n" + + "- name: build\n" + + " jobs:\n" + + " - \"ci-*\"\n" + + "\n" + + "jobs:\n" + + "- name: ci-project-one\n" + + " plan:\n" + + " - task: gradle-build\n" + + " file: gradle-build-dcind.yml" + ); + editor.assertProblems(/*NONE*/); + + editor = harness.newEditor( + "groups:\n" + + "- name: build\n" + + " jobs:\n" + + " - one-*\n" + + " - two-*\n" + + "- name: scan\n" + + " jobs:\n" + + " - one-*\n" + + " - three-*\n" + + "\n" + + "jobs:\n" + + "- name: one-project\n" + + " plan: []\n" + + "- name: two-project\n" + + " plan: []\n" + + "- name: three-project\n" + + " plan: []\n" + + "- name: four-project\n" + + " plan: []\n" + ); + editor.assertProblems("four-project|no group"); + + editor = harness.newEditor( + "groups:\n" + + "- name: build\n" + + " jobs:\n" + + " - ci-*\n" + + "\n" + + "jobs:\n" + + "- name: xx-project-one\n" + + " plan:\n" + + " - task: gradle-build\n" + + " file: gradle-build-dcind.yml" + ); + editor.assertProblems( + "ci-*|does not match any existing job", + "xx-project-one|belongs to no group" + ); + + // No errors if patterns are too complex for our SimpleGlob to understand + editor = harness.newEditor( + "groups:\n" + + "- name: build\n" + + " jobs:\n" + + " - \"{one,two,three,four}-project*\"\n" + + "- name: scan\n" + + "\n" + + "jobs:\n" + + "- name: one-project\n" + + " plan: []\n" + + "- name: two-project\n" + + " plan: []\n" + + "- name: three-project\n" + + " plan: []\n" + + "- name: four-project\n" + + " plan: []\n" + ); + editor.assertProblems(/*NONE*/); + } + @Test public void addSingleRequiredPropertiesQuickfix() throws Exception { Editor editor = harness.newEditor( "resources:\n" + @@ -1327,7 +1403,7 @@ public class ConcourseEditorTest { editor.assertProblems( "build-artefact^ # <- bad|should define 'branch'", - "bogus-job|does not exist", + "bogus-job|does not match", "not-a-resource|does not exist" ); }