Make sure we are able to output context for validation errors

4 files changed, 58 insertions, 11 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 b0b1209aa90..f3da3f1aafc 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
@@ -7,6 +7,7 @@ import com.thaiopensource.validate.ValidateProperty;
 import com.thaiopensource.validate.ValidationDriver;
 import com.thaiopensource.validate.rng.CompactSchemaReader;
 import com.yahoo.config.application.api.DeployLogger;
+import com.yahoo.io.IOUtils;
 import com.yahoo.io.reader.NamedReader;
 import com.yahoo.yolean.Exceptions;
 import org.xml.sax.ErrorHandler;
@@ -52,18 +53,20 @@ public class SchemaValidator {
     }
 
     public void validate(File file, String fileName) throws IOException {
-        validate(ValidationDriver.fileInputSource(file), fileName);
+        validate(IOUtils.createReader(file.getAbsolutePath()), fileName);
     }
 
     public void validate(Reader reader) throws IOException {
-        validate(new InputSource(reader), null);
+        validate(reader, null);
     }
 
     public void validate(NamedReader reader) throws IOException {
-        validate(new InputSource(reader), reader.getName());
+        validate(reader, reader.getName());
     }
 
-    public void validate(InputSource inputSource, String fileName)  throws IOException {
+    @Deprecated
+    /*  @deprecated Will not give proper context from errors, use another validate method instead */
+    public void validate(InputSource inputSource, String fileName) throws IOException {
         errorHandler.fileName = (fileName == null ? "input" : fileName);
         errorHandler.reader = inputSource.getCharacterStream();
         try {
@@ -72,8 +75,23 @@ public class SchemaValidator {
                 throw new RuntimeException("Aborting due to earlier XML errors.");
             }
         } catch (SAXException e) {
-            // This should never happen, as it is handled by the ErrorHandler
-            // installed for the driver.
+            // Shouldn't happen, error handler should have thrown
+            throw new IllegalArgumentException("XML error in " + errorHandler.fileName + ": " + Exceptions.toMessageString(e));
+        }
+    }
+
+    private void validate(Reader reader, String fileName) throws IOException {
+        errorHandler.fileName = (fileName == null ? "input" : fileName);
+        // We need to read from a reader in error handler, so need to read all content into a new one
+        Reader newReader = new StringReader(IOUtils.readAll(reader));
+        errorHandler.reader = newReader;
+        try {
+            if ( ! driver.validate(new InputSource(newReader))) {
+                // Shouldn't happen, error handler should have thrown
+                throw new RuntimeException("Aborting due to earlier XML errors.");
+            }
+        } catch (SAXException e) {
+            // Shouldn't happen, error handler should have thrown
             throw new IllegalArgumentException("XML error in " + errorHandler.fileName + ": " + Exceptions.toMessageString(e));
         }
     }
diff --git a/config-model/src/test/cfg/application/invalid-services-syntax/services.xml b/config-model/src/test/cfg/application/invalid-services-syntax/services.xml
new file mode 100644
index 00000000000..77b6fc0e33a
--- /dev/null
+++ b/config-model/src/test/cfg/application/invalid-services-syntax/services.xml
@@ -0,0 +1,11 @@
+<?xml version='1.0' encoding='utf-8' ?>
+<services>
+  <config name='standard'>
+    <basicStruct>
+      <stringVal>default</stringVal>
+    </basicStruct>
+  </confih>
+  <admin version='2.0'>
+    <adminserver hostalias='node1'>
+  </admin>
+</services>
diff --git a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java
index 9780e9b503a..75cb41be13f 100644
--- a/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/ApplicationDeployTest.java
@@ -1,4 +1,4 @@
-// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.model;
 
 import com.google.common.io.Files;
@@ -197,13 +197,21 @@ public class ApplicationDeployTest {
 
     @Test
     public void testThatAppWithInvalidParallelDeploymentFails() throws IOException {
+        String expectedMessage = "4:  <staging/>\n" +
+        "5:  <prod global-service-id=\"query\">\n" +
+        "6:    <parallel>\n" +
+        "7:      <instance id=\"hello\" />\n" +
+        "8:    </parallel>\n" +
+        "9:  </prod>\n" +
+        "10:</deployment>\n";
         File tmpDir = tmpFolder.getRoot();
         IOUtils.copyDirectory(new File(TESTDIR, "invalid_parallel_deployment_xml"), tmpDir);
         try {
             ApplicationPackageTester.create(tmpDir.getAbsolutePath());
             fail("Expected exception");
         } catch (IllegalArgumentException e) {
-            assertEquals("Invalid XML according to XML schema, error in deployment.xml: element \"instance\" not allowed here; expected the element end-tag or element \"delay\", \"region\", \"steps\" or \"test\" [7:30], input:\n", e.getMessage());
+            assertEquals("Invalid XML according to XML schema, error in deployment.xml: element \"instance\" not allowed here; expected the element end-tag or element \"delay\", \"region\", \"steps\" or \"test\" [7:30], input:\n" + expectedMessage,
+                         e.getMessage());
         }
     }
 
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 c2938746443..3849e9e03fd 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
@@ -2,12 +2,14 @@
 package com.yahoo.config.model.application.provider;
 
 import com.yahoo.component.Version;
+import com.yahoo.io.IOUtils;
 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 java.io.File;
 import java.io.IOException;
 import java.io.StringReader;
 
@@ -47,15 +49,15 @@ public class SchemaValidatorTest {
     @Test
     public void testXMLParse() throws IOException {
         SchemaValidator validator = createValidator();
-        validator.validate(new InputSource(new StringReader(okServices)), "services.xml");
+        validator.validate(new StringReader(okServices));
     }
 
     @Test
     public void testXMLParseError() throws IOException {
         SchemaValidator validator = createValidator();
         expectedException.expect(RuntimeException.class);
-        expectedException.expectMessage(expectedErrorMessage("services.xml"));
-        validator.validate(new InputSource(new StringReader(invalidServices)), "services.xml");
+        expectedException.expectMessage(expectedErrorMessage("input"));
+        validator.validate(new StringReader(invalidServices));
     }
 
     @Test
@@ -72,6 +74,14 @@ public class SchemaValidatorTest {
         validator.validate(new StringReader(invalidServices));
     }
 
+    @Test
+    public void testXMLParseErrorFromFile() throws IOException {
+        SchemaValidator validator = createValidator();
+        expectedException.expect(IllegalArgumentException.class);
+        expectedException.expectMessage(expectedErrorMessage("services.xml"));
+        validator.validate(new File("src/test/cfg/application/invalid-services-syntax/services.xml"));
+    }
+
     private SchemaValidator createValidator() {
         return new SchemaValidators(new Version(VespaVersion.major)).servicesXmlValidator();
     }