Merge pull request #12 from nierajsingh/async_validations

Separate delayed constraints into slow and fast collections
This commit is contained in:
nierajsingh
2017-08-02 12:46:31 -07:00
committed by GitHub
9 changed files with 114 additions and 31 deletions

View File

@@ -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
@@ -16,6 +16,22 @@ public interface IProblemCollector {
void endCollecting();
void accept(ReconcileProblem problem);
/**
* Optional for both implementors and callers.
* <p/>
* 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.
* <p/>
* 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.
*/
default void checkPointCollecting() {
}
/**
* Problem collector that simply ignores/discards anything passed to it.
*/

View File

@@ -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 {

View File

@@ -26,35 +26,39 @@ import com.google.common.collect.ImmutableSet;
public class EnumValueParser implements ValueParser {
private String typeName;
private Provider<PartialCollection<String>> values;
private final boolean longRunning;
public EnumValueParser(String typeName, String... values) {
this(typeName, ImmutableSet.copyOf(values));
}
public EnumValueParser(String typeName, Collection<String> values) {
this(typeName, provider(values));
this(typeName, false /* not long running by default */, provider(values));
}
private static <T> Provider<PartialCollection<T>> provider(Collection<T> values) {
return () -> PartialCollection.compute(() -> values);
}
private static <T> Provider<PartialCollection<T>> provider(Callable<Collection<T>> values) {
return () -> PartialCollection.compute(() -> values.call());
}
public EnumValueParser(String typeName, Callable<Collection<String>> values) {
this(typeName, provider(values));
public EnumValueParser(String typeName, boolean longRunning, Callable<Collection<String>> values) {
this(typeName, longRunning, provider(values));
}
public EnumValueParser(String typeName, Provider<PartialCollection<String>> values) {
public EnumValueParser(String typeName, boolean longRunning, Provider<PartialCollection<String>> values) {
this.typeName = typeName;
this.values = values;
this.longRunning = longRunning;
}
public EnumValueParser(String name, PartialCollection<String> values) {
this(name, () -> values);
this(name, false /* not long running by default */, () -> values);
}
@Override
@@ -91,6 +95,8 @@ public class EnumValueParser implements ValueParser {
protected Exception errorOnBlank(String message) {
return new ValueParseException(message);
}
public boolean longRunning() {
return this.longRunning ;
}
}

View File

@@ -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;

View File

@@ -65,13 +65,15 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
private final YTypeUtil typeUtil;
private final ITypeCollector typeCollector;
private final YamlQuickfixes quickfixes;
private List<Runnable> delayedConstraints = new ArrayList<>();
private List<Runnable> 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<Runnable> slowDelayedConstraints = new ArrayList<>();
public SchemaBasedYamlASTReconciler(IProblemCollector problems, YamlSchema schema, ITypeCollector typeCollector, YamlQuickfixes quickfixes) {
this.problems = problems;
this.schema = schema;
@@ -84,6 +86,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
public void reconcile(YamlFileAST ast) {
if (typeCollector!=null) typeCollector.beginCollecting(ast);
delayedConstraints.clear();
slowDelayedConstraints.clear();
try {
List<Node> nodes = ast.getNodes();
IntegerRange expectedDocs = schema.expectedNumberOfDocuments();
@@ -193,20 +196,16 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
if (typeUtil.isAtomic(type)) {
SchemaContextAware<ValueParser> 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.longRunning()) {
slowDelayedConstraints.add(() -> {
parse(ast, node, type, parser);
});
} else {
delayedConstraints.add(() -> {
parse(ast, node, type, parser);
});
}
});
}
} else {
@@ -219,6 +218,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 +318,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) {

View File

@@ -20,7 +20,7 @@ import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems;
public class CFServicesValueParser extends EnumValueParser {
public CFServicesValueParser(String typeName, Callable<Collection<String>> values) {
super(typeName, values);
super(typeName, true /*CF value parsers are potentially long running*/, values);
}
@Override

View File

@@ -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");

View File

@@ -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<Collection<YValueHint>> hintProvider, YAtomicType type, ProblemType problemType) {
return new EnumValueParser(type.toString(), YTypeFactory.valuesFromHintProvider(hintProvider)) {
public static EnumValueParser fromCFValueHints(Callable<Collection<YValueHint>> 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);

View File

@@ -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<CFDomain> 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);