Output context when schema validation fails

2 files changed, 86 insertions, 37 deletions

diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/SchemaValidator.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/SchemaValidator.java
index 16469bb13ae..1f6822a770b 100644
--- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/SchemaValidator.java
+++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/SchemaValidator.java
@@ -16,15 +16,20 @@ import org.xml.sax.SAXParseException;
 
 import java.io.File;
 import java.io.IOException;
+import java.io.LineNumberReader;
 import java.io.Reader;
+import java.io.StringReader;
 import java.util.logging.Level;
 
 /**
  * Validates xml files against a schema.
+ * Note: Tested by SchemaValidatorTest in config-model module, since
+ * needed schema files are in that module
  *
  * @author Tony Vaagenes
  */
 public class SchemaValidator {
+    private static final int linesOfContextForErrors = 3;
 
     private final CustomErrorHandler errorHandler = new CustomErrorHandler();
     private final ValidationDriver driver;
@@ -59,7 +64,8 @@ public class SchemaValidator {
     }
 
     public void validate(InputSource inputSource, String fileName)  throws IOException {
-        errorHandler.fileName = (fileName == null ? " input" : fileName);
+        errorHandler.fileName = (fileName == null ? "input" : fileName);
+        errorHandler.reader = inputSource.getCharacterStream();
         try {
             if ( ! driver.validate(inputSource)) {
                 // Shouldn't happen, error handler should have thrown
@@ -68,8 +74,7 @@ public class SchemaValidator {
         } catch (SAXException e) {
             // This should never happen, as it is handled by the ErrorHandler
             // installed for the driver.
-            throw new IllegalArgumentException(
-                    "XML error in " + (fileName == null ? " input" : fileName) + ": " + Exceptions.toMessageString(e));
+            throw new IllegalArgumentException("XML error in " + errorHandler.fileName + ": " + Exceptions.toMessageString(e));
         }
     }
 
@@ -81,6 +86,7 @@ public class SchemaValidator {
 
     private class CustomErrorHandler implements ErrorHandler {
         volatile String fileName;
+        volatile Reader reader;
 
         public void warning(SAXParseException e) {
             deployLogger.log(Level.WARNING, message(e));
@@ -97,8 +103,33 @@ public class SchemaValidator {
         private String message(SAXParseException e) {
             return "XML error in " + fileName + ": " +
                     Exceptions.toMessageString(e)
-                    + " [" + e.getLineNumber() + ":" + e.getColumnNumber() + "]";
+                    + " [" + e.getLineNumber() + ":" + e.getColumnNumber() + "]" +
+                    ", input:\n" + getErrorContext(e.getLineNumber());
         }
+
+        private String getErrorContext(int lineNumberWithError) {
+            if (!(reader instanceof StringReader)) return "";
+
+            int fromLine = Math.max(0, lineNumberWithError - linesOfContextForErrors);
+            int toLine = lineNumberWithError + linesOfContextForErrors;
+
+            LineNumberReader r = new LineNumberReader(reader);
+            StringBuilder sb = new StringBuilder();
+            String line;
+            try {
+                reader.reset();
+                while ((line = r.readLine()) != null) {
+                    int lineNumber = r.getLineNumber();
+                    if (lineNumber >= fromLine && lineNumber <= toLine)
+                        sb.append(lineNumber).append(":").append(line).append("\n");
+                }
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+
+            return sb.toString();
+        }
+
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/config/model/application/provider/SchemaValidatorTest.java b/config-model/src/test/java/com/yahoo/config/model/application/provider/SchemaValidatorTest.java
index 5c4b51ca3fa..a89378cb7ba 100644
--- a/config-model/src/test/java/com/yahoo/config/model/application/provider/SchemaValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/application/provider/SchemaValidatorTest.java
@@ -3,69 +3,87 @@ package com.yahoo.config.model.application.provider;
 
 import com.yahoo.component.Version;
 import com.yahoo.vespa.config.VespaVersion;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.xml.sax.InputSource;
-import org.xml.sax.SAXException;
 
 import java.io.IOException;
 import java.io.StringReader;
 
 /**
  * @author hmusum
- * @since 5.1.9
  */
 public class SchemaValidatorTest {
 
-    private static final String okServices = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>" +
-            "<services>" +
-            "  <config name=\"standard\">" +
-            "    <basicStruct>" +
-            "      <stringVal>default</stringVal>" +
-            "    </basicStruct>" +
-            "  </config> " +
-            "  <admin version=\"2.0\">" +
-            "    <adminserver hostalias=\"node1\" />" +
-            "  </admin>" +
-            "</services>";
+    private static final String okServices = "<?xml version='1.0' encoding='utf-8' ?>\n" +
+            "<services>\n" +
+            "  <config name='standard'>\n" +
+            "    <basicStruct>\n" +
+            "      <stringVal>default</stringVal>\n" +
+            "    </basicStruct>\n" +
+            "  </config>\n" +
+            "  <admin version='2.0'>\n" +
+            "    <adminserver hostalias='node1' />\n" +
+            "  </admin>\n" +
+            "</services>\n";
 
-    private static final String badServices = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>" +
-            "<services>" +
-            "  <config name=\"standard\">" +
-            "    <basicStruct>" +
-            "      <stringVal>default</stringVal>" +
-            "    </basicStruct>" +
-            "  </config> " +
-            "  <admin version=\"2.0\">" +
-            "    <adminserver hostalias=\"node1\"" +
-            "  </admin>" +
-            "</services>";
+    // Typo in closing end tag for <config> (<confih>)
+    private static final String invalidServices = "<?xml version='1.0' encoding='utf-8' ?>\n" +
+            "<services>\n" +
+            "  <config name='standard'>\n" +
+            "    <basicStruct>\n" +
+            "      <stringVal>default</stringVal>\n" +
+            "    </basicStruct>\n" +
+            "  </confih>\n" +
+            "  <admin version='2.0'>\n" +
+            "    <adminserver hostalias='node1'>\n" +
+            "  </admin>\n" +
+            "</services>\n";
 
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
 
     @Test
-    public void testXMLParse() throws SAXException, IOException {
+    public void testXMLParse() throws IOException {
         SchemaValidator validator = createValidator();
         validator.validate(new InputSource(new StringReader(okServices)), "services.xml");
     }
 
-    @Test(expected = RuntimeException.class)
-    public void testXMLParseError() throws SAXException, IOException {
+    @Test
+    public void testXMLParseError() throws IOException {
         SchemaValidator validator = createValidator();
-        validator.validate(new InputSource(new StringReader(badServices)), "services.xml");
+        expectedException.expect(RuntimeException.class);
+        expectedException.expectMessage(expectedErrorMessage("services.xml"));
+        validator.validate(new InputSource(new StringReader(invalidServices)), "services.xml");
     }
 
     @Test
-    public void testXMLParseWithReader() throws SAXException, IOException {
+    public void testXMLParseWithReader() throws IOException {
         SchemaValidator validator = createValidator();
         validator.validate(new StringReader(okServices));
     }
 
-    @Test(expected = RuntimeException.class)
-    public void testXMLParseErrorWithReader() throws SAXException, IOException {
+    @Test
+    public void testXMLParseErrorWithReader() throws IOException {
         SchemaValidator validator = createValidator();
-        validator.validate(new StringReader(badServices));
+        expectedException.expect(RuntimeException.class);
+        expectedException.expectMessage(expectedErrorMessage("input"));
+        validator.validate(new StringReader(invalidServices));
     }
 
-    private SchemaValidator createValidator() throws IOException {
+    private SchemaValidator createValidator() {
         return new SchemaValidators(new Version(VespaVersion.major)).servicesXmlValidator();
     }
+
+    private String expectedErrorMessage(String input) {
+        return "XML error in " + input + ": The element type \"config\" must be terminated by the matching end-tag \"</config>\". [7:5], input:\n" +
+                "4:    <basicStruct>\n" +
+                "5:      <stringVal>default</stringVal>\n" +
+                "6:    </basicStruct>\n" +
+                "7:  </confih>\n" +
+                "8:  <admin version='2.0'>\n" +
+                "9:    <adminserver hostalias='node1'>\n" +
+                "10:  </admin>\n";
+    }
 }