From 0a530d9e3206a105cb54b81a62ce69072b7296f7 Mon Sep 17 00:00:00 2001 From: nsingh Date: Tue, 1 Aug 2017 13:48:27 -0700 Subject: [PATCH 1/4] Separate delayed constraints into slow and fast collections The slow part is for CF-related constraints like long-running value parsers. Also, the "fast" constraints and other problems are reported first via a new checkpoint call to the problems collector. The complete list of problems are then reported afterwards at the end of the collection, like before. --- .../reconcile/IProblemCollector.java | 5 +- .../util/SimpleLanguageServer.java | 6 ++ .../vscode/commons/util/EnumValueParser.java | 15 +++-- .../SchemaBasedYamlASTReconciler.java | 55 +++++++++++++------ .../manifest/yaml/CFServicesValueParser.java | 2 +- .../manifest/yaml/ManifestYmlSchema.java | 4 +- .../yaml/ManifestYmlValueParsers.java | 6 +- .../manifest/yaml/ManifestYamlEditorTest.java | 29 ++++++++++ 8 files changed, 94 insertions(+), 28 deletions(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java index 6ac511745..7ee9dfe3d 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2014-2016 Pivotal, Inc. + * Copyright (c) 2014-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 @@ -15,6 +15,7 @@ public interface IProblemCollector { void beginCollecting(); void endCollecting(); void accept(ReconcileProblem problem); + void checkPointCollecting(); /** * Problem collector that simply ignores/discards anything passed to it. @@ -26,5 +27,7 @@ public interface IProblemCollector { } public void accept(ReconcileProblem problem) { } + public void checkPointCollecting() { + } }; } \ No newline at end of file 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 5b44fc190..91ea2cad2 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 @@ -345,6 +345,12 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl diagnostics.clear(); } + @Override + public void checkPointCollecting() { + // publish what has been collected so far + documents.publishDiagnostics(docId, diagnostics); + } + @Override public void accept(ReconcileProblem problem) { try { 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 0edd2a742..e1ee565ee 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 @@ -27,22 +27,24 @@ public class EnumValueParser implements ValueParser { private String typeName; private Provider> values; + private final boolean longRunning; public EnumValueParser(String typeName, String... values) { this(typeName, ImmutableSet.copyOf(values)); } public EnumValueParser(String typeName, Collection values) { - this(typeName, provider(values)); + this(typeName, false /* not long running by default */, provider(values)); } - public EnumValueParser(String typeName, Callable> values) { - this(typeName, provider(values)); + public EnumValueParser(String typeName, boolean longRunning, Callable> values) { + this(typeName, longRunning, provider(values)); } - public EnumValueParser(String typeName, Provider> values) { + public EnumValueParser(String typeName, boolean longRunning, Provider> values) { this.typeName = typeName; this.values = values; + this.longRunning = longRunning; } @Override @@ -94,5 +96,8 @@ public class EnumValueParser implements ValueParser { } }; } - + + public boolean longRunning() { + return this.longRunning ; + } } diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java index 73bff9672..b175c7a66 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java @@ -31,6 +31,7 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.Reconcile import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblemImpl; import org.springframework.ide.vscode.commons.languageserver.reconcile.ReplacementQuickfix; import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion; +import org.springframework.ide.vscode.commons.util.EnumValueParser; import org.springframework.ide.vscode.commons.util.ExceptionUtil; import org.springframework.ide.vscode.commons.util.IntegerRange; import org.springframework.ide.vscode.commons.util.Log; @@ -65,13 +66,15 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { private final YTypeUtil typeUtil; private final ITypeCollector typeCollector; private final YamlQuickfixes quickfixes; - - private List delayedConstraints = new ArrayList<>(); + + private List delayedConstraints = new ArrayList<>(); // keeps track of dynamic constraints discovered during reconciler walk // the constraints are validated at the end of the walk rather than during the walk. // This facilitates constraints that depend on, for example, the contents of the ast type cache being // populated prior to checking. + private List slowDelayedConstraints = new ArrayList<>(); + public SchemaBasedYamlASTReconciler(IProblemCollector problems, YamlSchema schema, ITypeCollector typeCollector, YamlQuickfixes quickfixes) { this.problems = problems; this.schema = schema; @@ -84,6 +87,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { public void reconcile(YamlFileAST ast) { if (typeCollector!=null) typeCollector.beginCollecting(ast); delayedConstraints.clear(); + slowDelayedConstraints.clear(); try { List nodes = ast.getNodes(); IntegerRange expectedDocs = schema.expectedNumberOfDocuments(); @@ -193,20 +197,16 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { if (typeUtil.isAtomic(type)) { SchemaContextAware parserProvider = typeUtil.getValueParser(type); if (parserProvider!=null) { - delayedConstraints.add(() -> { - parserProvider.safeWithContext(schemaContext).ifPresent(parser -> { - try { - String value = NodeUtil.asScalar(node); - if (value!=null) { - parser.parse(value); - } - } catch (Exception e) { - ProblemType problemType = getProblemType(e); - DocumentRegion region = getRegion(e, ast.getDocument(), node); - String msg = getMessage(e); - valueParseError(type, region, msg, problemType, getValueReplacement(e)); - } - }); + parserProvider.safeWithContext(schemaContext).ifPresent(parser -> { + if (parser instanceof EnumValueParser && ((EnumValueParser) parser).longRunning()) { + slowDelayedConstraints.add(() -> { + parse(ast, node, type, parser); + }); + } else { + delayedConstraints.add(() -> { + parse(ast, node, type, parser); + }); + } }); } } else { @@ -219,6 +219,20 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { } } + private void parse(YamlFileAST ast, Node node, YType type, ValueParser parser) { + try { + String value = NodeUtil.asScalar(node); + if (value!=null) { + parser.parse(value); + } + } catch (Exception e) { + ProblemType problemType = getProblemType(e); + DocumentRegion region = getRegion(e, ast.getDocument(), node); + String msg = getMessage(e); + valueParseError(type, region, msg, problemType, getValueReplacement(e)); + } + } + protected ReplacementQuickfix getValueReplacement(Exception _e) { if (_e instanceof ReconcileException) { ReconcileException e = (ReconcileException) _e; @@ -305,7 +319,16 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { for (Runnable runnable : delayedConstraints) { runnable.run(); } + + // First report the "faster" delayed constraints + problems.checkPointCollecting(); + delayedConstraints.clear(); + + for (Runnable runnable : slowDelayedConstraints) { + runnable.run(); + } + slowDelayedConstraints.clear(); } protected NodeId getNodeId(Node node) { diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/CFServicesValueParser.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/CFServicesValueParser.java index 1a2966693..3f1a11b8f 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/CFServicesValueParser.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/CFServicesValueParser.java @@ -20,7 +20,7 @@ import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems; public class CFServicesValueParser extends EnumValueParser { public CFServicesValueParser(String typeName, Callable> values) { - super(typeName, values); + super(typeName, true /*CF value parsers are potentially long running*/, values); } @Override 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 6f81233c8..5fa678fd3 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 @@ -129,13 +129,13 @@ public final class ManifestYmlSchema implements YamlSchema { YAtomicType t_stack = f.yatomic("Stack"); if (stacksProvider!=null) { t_stack.setHintProvider(stacksProvider); - t_stack.parseWith(ManifestYmlValueParsers.fromValueHints(stacksProvider, t_stack, ManifestYamlSchemaProblemsTypes.UNKNOWN_STACK_PROBLEM)); + t_stack.parseWith(ManifestYmlValueParsers.fromCFValueHints(stacksProvider, t_stack, ManifestYamlSchemaProblemsTypes.UNKNOWN_STACK_PROBLEM)); } YAtomicType t_domain = f.yatomic("Domain"); if (domainsProvider != null) { t_domain.setHintProvider(domainsProvider); - t_domain.parseWith(ManifestYmlValueParsers.fromValueHints(domainsProvider, t_domain, ManifestYamlSchemaProblemsTypes.UNKNOWN_DOMAIN_PROBLEM)); + t_domain.parseWith(ManifestYmlValueParsers.fromCFValueHints(domainsProvider, t_domain, ManifestYamlSchemaProblemsTypes.UNKNOWN_DOMAIN_PROBLEM)); } YAtomicType t_service = f.yatomic("Service"); diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlValueParsers.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlValueParsers.java index 6849f02df..dfbe04635 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlValueParsers.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlValueParsers.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2016 Pivotal, Inc. + * Copyright (c) 2016, 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 @@ -95,8 +95,8 @@ public class ManifestYmlValueParsers { }; } - public static EnumValueParser fromValueHints(Callable> hintProvider, YAtomicType type, ProblemType problemType) { - return new EnumValueParser(type.toString(), YTypeFactory.valuesFromHintProvider(hintProvider)) { + public static EnumValueParser fromCFValueHints(Callable> hintProvider, YAtomicType type, ProblemType problemType) { + return new EnumValueParser(type.toString(), true /*CF value parsers are potentially long running*/, YTypeFactory.valuesFromHintProvider(hintProvider)) { @Override protected Exception errorOnParse(String message) { return new ReconcileException(message, problemType); 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 07c9159b5..7d0e201f1 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 @@ -1120,6 +1120,35 @@ public class ManifestYamlEditorTest { assertEquals(DiagnosticSeverity.Warning, problem.getSeverity()); } + @Test + public void delayedConstraints() throws Exception { + // This tests the two different types of delayed constraints: + // Slow delayed constraints that require CF connection (services + // and "faster" delayed constraints that check that 'routes' property + // cannot exist with 'domain' and 'host' + ClientRequests cfClient = cloudfoundry.client; + when(cfClient.getServices()).thenReturn(ImmutableList.of()); + + List domains = ImmutableList.of(mockDomain("test.cfapps.io")); + when(cloudfoundry.client.getDomains()).thenReturn(domains); + Editor editor = harness.newEditor( + "applications:\n" + + "- name: foo\n" + + " host: foosite\n" + + " domain: test.cfapps.io\n" + + " routes:\n" + + " - route: test.cfapps.io/path\n" + + " services:\n" + + " - bad-service\n"); + editor.assertProblems( + // These are the "fast" delayed constraints + "host|Property cannot co-exist with property 'routes'", + "domain|Property cannot co-exist with property 'routes'", + "routes|Property cannot co-exist with properties [domain, host]", + // This is the "slow" delayed constraint + "bad-service|There is no service instance called"); + } + @Test public void servicesContentAssistShowErrorMessageWhenNotLoggedIn() throws Exception { reset(cloudfoundry.defaultParamsProvider); From ef3981b162a8fd386ac85a40e6d18efb2f589f98 Mon Sep 17 00:00:00 2001 From: nsingh Date: Wed, 2 Aug 2017 10:51:00 -0700 Subject: [PATCH 2/4] Adding docs and some code cleanup --- .../reconcile/IProblemCollector.java | 18 +++++++++++++++--- .../ide/vscode/commons/util/ValueParser.java | 4 ++++ .../SchemaBasedYamlASTReconciler.java | 3 +-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java index 7ee9dfe3d..eec0df9c1 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java @@ -15,7 +15,21 @@ public interface IProblemCollector { void beginCollecting(); void endCollecting(); void accept(ReconcileProblem problem); - void checkPointCollecting(); + + /** + * Allows the problem collector to process problems that has been collected so + * far, BEFORE the end collecting. It is to handle cases where a subset of + * problems need to be processed or published in an "intermediate" phase during + * a collecting session, but prior to the final end collecting. For example, if + * a collection session in a reconcile engine wants to publish fast problems + * first before handling slow problems , this method allows the reconcile engine + * to notify the problem collector to process the fast problems first before + * starting with the slow ones. The reconcile engine, or whoever is calling the + * collector, is responsible for deciding when to call this checkpoint. + */ + default void checkPointCollecting() { + + } /** * Problem collector that simply ignores/discards anything passed to it. @@ -27,7 +41,5 @@ public interface IProblemCollector { } public void accept(ReconcileProblem problem) { } - public void checkPointCollecting() { - } }; } \ No newline at end of file diff --git a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ValueParser.java b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ValueParser.java index 35b218cd9..bd5685e89 100644 --- a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ValueParser.java +++ b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ValueParser.java @@ -23,6 +23,10 @@ public interface ValueParser { * that the String is not the format this parser expects. */ Object parse(String str) throws Exception; + + default boolean longRunning() { + return false; + } static ValueParser of(ValueParser x) { return x; diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java index b175c7a66..d68fea323 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java @@ -31,7 +31,6 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.Reconcile import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblemImpl; import org.springframework.ide.vscode.commons.languageserver.reconcile.ReplacementQuickfix; import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion; -import org.springframework.ide.vscode.commons.util.EnumValueParser; import org.springframework.ide.vscode.commons.util.ExceptionUtil; import org.springframework.ide.vscode.commons.util.IntegerRange; import org.springframework.ide.vscode.commons.util.Log; @@ -198,7 +197,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { SchemaContextAware parserProvider = typeUtil.getValueParser(type); if (parserProvider!=null) { parserProvider.safeWithContext(schemaContext).ifPresent(parser -> { - if (parser instanceof EnumValueParser && ((EnumValueParser) parser).longRunning()) { + if (parser.longRunning()) { slowDelayedConstraints.add(() -> { parse(ast, node, type, parser); }); From edc94daa4f815b3394792d220860d9bb84fc4dad Mon Sep 17 00:00:00 2001 From: nsingh Date: Wed, 2 Aug 2017 12:43:52 -0700 Subject: [PATCH 3/4] Changed doc to be clearer on the contract for checkpoint method --- .../reconcile/IProblemCollector.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java index eec0df9c1..31d4cbe1e 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java @@ -17,15 +17,16 @@ public interface IProblemCollector { void accept(ReconcileProblem problem); /** - * Allows the problem collector to process problems that has been collected so - * far, BEFORE the end collecting. It is to handle cases where a subset of - * problems need to be processed or published in an "intermediate" phase during - * a collecting session, but prior to the final end collecting. For example, if - * a collection session in a reconcile engine wants to publish fast problems - * first before handling slow problems , this method allows the reconcile engine - * to notify the problem collector to process the fast problems first before - * starting with the slow ones. The reconcile engine, or whoever is calling the - * collector, is responsible for deciding when to call this checkpoint. + * Optional for both implementors and callers. + *

+ * This method optionally allows callers to do partial collection between the + * start and end collecting, and can be called numerous times. The caller is + * responsible to decide when and how often these checkpoints are invoked during + * a collecting session. + *

+ * For implementors, this support cases where problems need to be processed in + * intermediate phases between the start and end collecting stages, and if + * implemented, should support multiple checkpoint invocations. */ default void checkPointCollecting() { From 464e36ad2df176d9c59f2dae6886b9ddae22afe5 Mon Sep 17 00:00:00 2001 From: nsingh Date: Wed, 2 Aug 2017 12:44:30 -0700 Subject: [PATCH 4/4] Further doc change --- .../commons/languageserver/reconcile/IProblemCollector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java index 31d4cbe1e..9f6d8018a 100644 --- a/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java +++ b/headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/IProblemCollector.java @@ -24,7 +24,7 @@ public interface IProblemCollector { * responsible to decide when and how often these checkpoints are invoked during * a collecting session. *

- * For implementors, this support cases where problems need to be processed in + * For implementors, this optional support handles cases where problems need to be processed in * intermediate phases between the start and end collecting stages, and if * implemented, should support multiple checkpoint invocations. */