From dfe8cd1e8dd0a8046e13d6582d52528552e914a6 Mon Sep 17 00:00:00 2001 From: nsingh Date: Mon, 23 Jan 2017 21:26:28 -0800 Subject: [PATCH] Allow services parsing to show as warning instead of error --- .../reconcile/ProblemTypeProvider.java | 16 +++++++ .../reconcile/ReconcileException.java | 36 ++++++++++++++++ .../vscode/commons/util/EnumValueParser.java | 9 ++-- .../vscode/commons/util/ExceptionUtil.java | 16 +------ .../SchemaBasedYamlASTReconciler.java | 42 +++++++++++++++--- .../yaml/reconcile/YamlSchemaProblems.java | 21 ++++++--- .../yaml/AbstractCFHintsProvider.java | 8 ++-- .../manifest/yaml/CFServicesValueParser.java | 9 +++- .../yaml/ManifestYamlSchemaProblemsTypes.java | 27 ++++++++++++ .../manifest/yaml/ManifestYamlEditorTest.java | 43 +++++++++++++++++++ 10 files changed, 191 insertions(+), 36 deletions(-) create mode 100644 vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ProblemTypeProvider.java create mode 100644 vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ReconcileException.java create mode 100644 vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlSchemaProblemsTypes.java diff --git a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ProblemTypeProvider.java b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ProblemTypeProvider.java new file mode 100644 index 000000000..d0e615e4a --- /dev/null +++ b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ProblemTypeProvider.java @@ -0,0 +1,16 @@ +/******************************************************************************* + * Copyright (c) 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.commons.languageserver.reconcile; + +public interface ProblemTypeProvider { + + ProblemType getProblemType(); +} diff --git a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ReconcileException.java b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ReconcileException.java new file mode 100644 index 000000000..147b2facf --- /dev/null +++ b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/reconcile/ReconcileException.java @@ -0,0 +1,36 @@ +/******************************************************************************* + * Copyright (c) 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.commons.languageserver.reconcile; + +/** + * Exception if there is a failure when parsing a value. It does not wrap + * other exceptions such that when thrown, the parse exception is the "deepest" + * error. + * + */ +public class ReconcileException extends Exception implements ProblemTypeProvider { + + /** + * + */ + private static final long serialVersionUID = 1L; + private final ProblemType problemType; + + public ReconcileException(String message, ProblemType problemType) { + super(message); + this.problemType = problemType; + } + + @Override + public ProblemType getProblemType() { + return problemType; + } +} diff --git a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java index fcb8c41e2..bf668c693 100644 --- a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java +++ b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/EnumValueParser.java @@ -48,7 +48,7 @@ public class EnumValueParser implements ValueParser { // IMPORTANT: check the text FIRST before fetching values // from the hints provider, as the hints provider may be expensive when resolving values if (!StringUtil.hasText(str)) { - throw toValueParseError(createBlankTextErrorMessage()); + throw createException(createBlankTextErrorMessage()); } Collection values = this.values.get(); @@ -57,12 +57,13 @@ public class EnumValueParser implements ValueParser { if (values==null || values.contains(str)) { return str; } else { - throw toValueParseError(createErrorMessage(str, values)); + throw createException(createErrorMessage(str, values)); } } - protected Exception toValueParseError(String message) throws Exception{ - return ExceptionUtil.asValueParseException(message); + + protected Exception createException(String message) { + return new IllegalArgumentException(message); } protected String createBlankTextErrorMessage() { diff --git a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java index 9b6f7c723..d74986b2d 100644 --- a/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java +++ b/vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/ExceptionUtil.java @@ -61,15 +61,7 @@ public class ExceptionUtil { // The message of nested exception is usually more interesting than the // one on top. Throwable cause = getDeepestCause(e); - // If value parse exception, do not append any additional information - if (cause instanceof ValueParseException) { - String msg = cause.getMessage(); - if (StringUtil.hasText(msg)) { - return msg; - } else { - return "An error occurred: " + getSimpleError(cause); - } - } else if (cause != null) { + if (cause != null) { String msg = getSimpleError(cause) + ": " + cause.getMessage(); return msg; } else { @@ -77,7 +69,7 @@ public class ExceptionUtil { } } - private static String getSimpleError(Throwable e) { + public static String getSimpleError(Throwable e) { return e.getClass().getSimpleName(); } @@ -135,8 +127,4 @@ public class ExceptionUtil { return exception(error); } } - - public static Exception asValueParseException(String message) { - return new ValueParseException(message); - } } diff --git a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java index 581baef8f..a26ac1f6e 100644 --- a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java +++ b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java @@ -11,6 +11,7 @@ package org.springframework.ide.vscode.commons.yaml.reconcile; +import static org.springframework.ide.vscode.commons.util.ExceptionUtil.getSimpleError; import static org.springframework.ide.vscode.commons.yaml.ast.NodeUtil.asScalar; import java.util.ArrayList; @@ -21,11 +22,12 @@ import java.util.Set; import java.util.TreeSet; import java.util.regex.Pattern; import java.util.stream.Collectors; -import java.util.stream.Stream; import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemType; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemTypeProvider; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileException; import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion; -import org.springframework.ide.vscode.commons.util.CollectionUtil; import org.springframework.ide.vscode.commons.util.ExceptionUtil; import org.springframework.ide.vscode.commons.util.IntegerRange; import org.springframework.ide.vscode.commons.util.Log; @@ -160,8 +162,9 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { parser.parse(value); } } catch (Exception e) { - String msg = ExceptionUtil.getMessage(e); - valueParseError(type, node, msg); + ProblemType problemType = getProblemType(e); + String msg = getMessage(e); + valueParseError(type, node, msg, problemType); } } } else { @@ -174,6 +177,27 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { } } + private String getMessage(Exception _e) { + Throwable e = ExceptionUtil.getDeepestCause(_e); + + // If value parse exception, do not append any additional information + if (e instanceof ReconcileException) { + String msg = e.getMessage(); + if (StringUtil.hasText(msg)) { + return msg; + } else { + return "An error occurred: " + getSimpleError(e); + } + } else { + return ExceptionUtil.getMessage(e); + } + } + + protected ProblemType getProblemType(Exception _e) { + Throwable e = ExceptionUtil.getDeepestCause(_e); + return e instanceof ProblemTypeProvider ? ((ProblemTypeProvider) e).getProblemType() : YamlSchemaProblems.SCHEMA_PROBLEM; + } + private void checkRequiredProperties(MappingNode map, YType type, Map beanProperties) { Set foundProps = NodeUtil.getScalarKeys(map); boolean allPropertiesKnown = beanProperties.keySet().containsAll(foundProps); @@ -271,12 +295,12 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { } } } - - private void valueParseError(YType type, Node node, String parseErrorMsg) { + + private void valueParseError(YType type, Node node, String parseErrorMsg, ProblemType problemType) { if (!StringUtil.hasText(parseErrorMsg)) { parseErrorMsg= "Couldn't parse as '"+describe(type)+"'"; } - problem(node, parseErrorMsg); + problem(node, parseErrorMsg, problemType); } private void unknownBeanProperty(Node keyNode, YType type, String name) { @@ -331,6 +355,10 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { private void problem(Node node, String msg) { problems.accept(YamlSchemaProblems.schemaProblem(msg, node)); } + + private void problem(Node node, String msg, ProblemType problemType) { + problems.accept(YamlSchemaProblems.problem(msg, node, problemType)); + } private void problem(DocumentRegion region, String msg) { problems.accept(YamlSchemaProblems.schemaProblem(msg, region)); diff --git a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java index a5db4b2f7..772ec8bb5 100644 --- a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java +++ b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java @@ -16,7 +16,6 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemTy import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem; import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblemImpl; import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion; -import org.springframework.ide.vscode.commons.yaml.ast.YamlFileAST; import org.yaml.snakeyaml.nodes.Node; /** @@ -26,10 +25,11 @@ import org.yaml.snakeyaml.nodes.Node; */ public class YamlSchemaProblems { - private static final ProblemType SCHEMA_PROBLEM = problemType("YamlSchemaProblem"); - private static final ProblemType SYNTAX_PROBLEM = problemType("YamlSyntaxProblem"); + public static final ProblemType SCHEMA_PROBLEM = problemType("YamlSchemaProblem"); + public static final ProblemType SYNTAX_PROBLEM = problemType("YamlSyntaxProblem"); - private static ProblemType problemType(final String typeName) { + public static ProblemType problemType(final String typeName, ProblemSeverity defaultSeverity) { + return new ProblemType() { @Override public String toString() { @@ -37,7 +37,7 @@ public class YamlSchemaProblems { } @Override public ProblemSeverity getDefaultSeverity() { - return ProblemSeverity.ERROR; + return defaultSeverity; } @Override public String getCode() { @@ -45,6 +45,10 @@ public class YamlSchemaProblems { } }; } + + public static ProblemType problemType(final String typeName) { + return problemType(typeName, ProblemSeverity.ERROR); + } public static ReconcileProblem syntaxProblem(String msg, int offset, int len) { return new ReconcileProblemImpl(SYNTAX_PROBLEM, msg, offset, len); @@ -59,5 +63,10 @@ public class YamlSchemaProblems { public static ReconcileProblem schemaProblem(String msg, DocumentRegion node) { return new ReconcileProblemImpl(SCHEMA_PROBLEM, msg, node.getStart(), node.getLength()); } - + + public static ReconcileProblem problem(String msg, Node node, ProblemType problemType) { + int start = node.getStartMark().getIndex(); + int end = node.getEndMark().getIndex(); + return new ReconcileProblemImpl(problemType, msg, start, end-start); + } } diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java index 734d76ed6..5261d2cc2 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/AbstractCFHintsProvider.java @@ -23,6 +23,7 @@ import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CFTar import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.NoTargetsException; import org.springframework.ide.vscode.commons.util.Assert; import org.springframework.ide.vscode.commons.util.ExceptionUtil; +import org.springframework.ide.vscode.commons.util.ValueParseException; import org.springframework.ide.vscode.commons.yaml.schema.YValueHint; public abstract class AbstractCFHintsProvider implements Callable> { @@ -57,15 +58,14 @@ public abstract class AbstractCFHintsProvider implements Callable values) { return "There is no service instance called '" + parseString + "'. Available service instances are: " + values; } - + + @Override protected String createBlankTextErrorMessage() { return "At least one service instance name must be specified"; } + @Override + protected Exception createException(String message) { + return new ReconcileException(message, ManifestYamlSchemaProblemsTypes.UNKNOWN_SERVICES_PROBLEM); + } } diff --git a/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlSchemaProblemsTypes.java b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlSchemaProblemsTypes.java new file mode 100644 index 000000000..dec057b68 --- /dev/null +++ b/vscode-extensions/vscode-manifest-yaml/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlSchemaProblemsTypes.java @@ -0,0 +1,27 @@ +/******************************************************************************* + * Copyright (c) 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 + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Pivotal, Inc. - initial API and implementation + *******************************************************************************/ +package org.springframework.ide.vscode.manifest.yaml; + +import static org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems.problemType; + +import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemSeverity; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemType; + +/** + * + */ +public class ManifestYamlSchemaProblemsTypes { + + public static final ProblemType UNKNOWN_SERVICES_PROBLEM = problemType("UnknownServicesProblem", + ProblemSeverity.WARNING); + + +} diff --git a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java index 21f1f81e1..5e0fa7676 100644 --- a/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java +++ b/vscode-extensions/vscode-manifest-yaml/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java @@ -10,20 +10,27 @@ *******************************************************************************/ package org.springframework.ide.vscode.manifest.yaml; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; import java.io.IOException; import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.DiagnosticSeverity; import org.junit.Before; import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.ide.vscode.commons.cloudfoundry.client.CFServiceInstance; import org.springframework.ide.vscode.commons.cloudfoundry.client.ClientRequests; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.CfCliParamsProvider; import org.springframework.ide.vscode.commons.cloudfoundry.client.cftarget.ClientParamsProvider; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemSeverity; import org.springframework.ide.vscode.languageserver.testharness.Editor; import org.springframework.ide.vscode.languageserver.testharness.LanguageServerHarness; +import com.google.common.collect.ImmutableList; + public class ManifestYamlEditorTest { LanguageServerHarness harness; @@ -782,6 +789,42 @@ public class ManifestYamlEditorTest { ); editor.assertProblems("bogus|Unknown property"); } + + @Test + public void reconcileShowsWarningOnUnknownService() throws Exception { + ClientRequests cfClient = cfClientFactory.client; + CFServiceInstance service = Mockito.mock(CFServiceInstance.class); + when(service.getName()).thenReturn("myservice"); + when(cfClient.getServices()).thenReturn(ImmutableList.of()); + Editor editor = harness.newEditor( + "applications:\n" + + "- name: foo\n" + + " services:\n" + + " - bad-service\n" + + ); + editor.assertProblems("bad-service|There is no service instance called"); + + Diagnostic problem = editor.assertProblem("bad-service"); + + assertEquals(DiagnosticSeverity.Warning, problem.getSeverity()); + } + + @Test + public void reconcileShowsWarningOnNoService() throws Exception { + ClientRequests cfClient = cfClientFactory.client; + when(cfClient.getServices()).thenReturn(ImmutableList.of()); + Editor editor = harness.newEditor( + "applications:\n" + + "- name: foo\n" + + " services:\n" + + " - bad-service\n"); + editor.assertProblems("bad-service|There is no service instance called"); + + Diagnostic problem = editor.assertProblem("bad-service"); + + assertEquals(DiagnosticSeverity.Warning, problem.getSeverity()); + } //////////////////////////////////////////////////////////////////////////////