Merge pull request #12 from nierajsingh/async_validations

Separate delayed constraints into slow and fast collections

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
@@ -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.
 	 */

headless-services/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java

@@ -345,6 +345,12 @@ public void beginCollecting() {
 					diagnostics.clear();
 				}
 
+				@Override
+				public void checkPointCollecting() {
+					// publish what has been collected so far
+					documents.publishDiagnostics(docId, diagnostics);
+				}
+
 				@Override
 				public void accept(ReconcileProblem problem) {
 					try {

headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java

@@ -26,35 +26,39 @@
 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 @@ protected Exception errorOnParse(String message) {
 	protected Exception errorOnBlank(String message) {
 		return new ValueParseException(message);
 	}
-
-
+
+	public boolean longRunning() {
+		return this.longRunning ;
+	}
 }

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;

headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java

@@ -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 SchemaBasedYamlASTReconciler(IProblemCollector problems, YamlSchema schem
 	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 @@ private void reconcile(YamlFileAST ast, YamlPath path, Node parent, Node node, Y
 				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 @@ private void reconcile(YamlFileAST ast, YamlPath path, Node parent, Node node, Y
 		}
 	}
 
+	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 @@ private void verifyDelayedConstraints() {
 		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) {

headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/CFServicesValueParser.java

@@ -20,7 +20,7 @@
 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

headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java

@@ -129,13 +129,13 @@ public ManifestYmlSchema(ManifestYmlHintProviders providers) {
 		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");

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 Object parse(String str) throws Exception {
 		};
 	}
 
-	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);

headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java

@@ -1120,6 +1120,35 @@ public void reconcileShowsWarningOnNoService() throws Exception {
 		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);