Test bad templates

9 files changed, 86 insertions, 33 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/IfSection.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/IfSection.java
index 4a00115cee4..fdb14189656 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/IfSection.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/IfSection.java
@@ -18,7 +18,7 @@ class IfSection extends Section {
 
     IfSection(CursorRange range, boolean negated, String name, Cursor nameOffset,
               SectionList ifSections, Optional<SectionList> elseSections) {
-        super(range);
+        super("if", range);
         this.negated = negated;
         this.name = name;
         this.nameOffset = nameOffset;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListSection.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListSection.java
index 831dc3fe5e8..a137a27f3bb 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListSection.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/ListSection.java
@@ -17,7 +17,7 @@ class ListSection extends Section {
     private final List<Template> elements = new ArrayList<>();
 
     ListSection(CursorRange range, String name, Cursor nameOffset, Template body) {
-        super(range);
+        super("list", range);
         this.name = name;
         this.nameOffset = new Cursor(nameOffset);
         this.body = body;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/LiteralSection.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/LiteralSection.java
index c03653253af..667c515dcec 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/LiteralSection.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/LiteralSection.java
@@ -11,7 +11,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.text.CursorRange;
  */
 class LiteralSection extends Section {
     LiteralSection(CursorRange range) {
-        super(range);
+        super("literal", range);
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/NameAlreadyExistsTemplateException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/NameAlreadyExistsTemplateException.java
index dd92af14609..0869b7f181c 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/NameAlreadyExistsTemplateException.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/NameAlreadyExistsTemplateException.java
@@ -1,22 +1,14 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.task.util.template;
 
-import com.yahoo.vespa.hosted.node.admin.task.util.text.Cursor;
-import com.yahoo.vespa.hosted.node.admin.task.util.text.CursorRange;
-
 /**
  * @author hakonhall
  */
 public class NameAlreadyExistsTemplateException extends TemplateException {
-    public NameAlreadyExistsTemplateException(String name, CursorRange range) {
-        super("Name '" + name + "' already exists in the " + describeSection(range));
-    }
-
-    public NameAlreadyExistsTemplateException(String name, Cursor firstNameLocation,
-                                              Cursor secondNameLocation) {
-        super("Section named '" + name + "' at " +
-              firstNameLocation.calculateLocation().lineAndColumnText() +
-              " conflicts with earlier section with the same name at " +
-              secondNameLocation.calculateLocation().lineAndColumnText());
+    public NameAlreadyExistsTemplateException(String name, Section first, Section second) {
+        super("The name '" + name + "' of the " + second.type() + " section at " +
+              second.range().start().calculateLocation().lineAndColumnText() +
+              " is in conflict with the identically named " + first.type() + " section at " +
+              first.range().start().calculateLocation().lineAndColumnText());
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Section.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Section.java
index 2c52fd5c34e..42927cd2c04 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Section.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/Section.java
@@ -12,10 +12,12 @@ import java.util.Objects;
  * @author hakonhall
  */
 abstract class Section {
+    private final String type;
     private final CursorRange range;
     private Template template;
 
-    protected Section(CursorRange range) {
+    protected Section(String type, CursorRange range) {
+        this.type = type;
         this.range = range;
     }
 
@@ -24,6 +26,7 @@ abstract class Section {
     /** Guaranteed to return non-null after TemplateBuilder::build() returns. */
     protected Template template() { return Objects.requireNonNull(template); }
 
+    protected String type() { return type; }
     protected CursorRange range() { return range; }
 
     abstract void appendTo(StringBuilder buffer);
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateBuilder.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateBuilder.java
index 8041a17fe74..2827a9eb005 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateBuilder.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateBuilder.java
@@ -35,8 +35,7 @@ class TemplateBuilder {
 
         ListSection existing = lists.get(section.name());
         if (existing != null)
-            throw new NameAlreadyExistsTemplateException(section.name(), existing.nameOffset(),
-                                                         section.nameOffset());
+            throw new NameAlreadyExistsTemplateException(section.name(), existing, section);
 
         sampleVariables.put(section.name(), section);
         allSections.add(section);
@@ -48,8 +47,7 @@ class TemplateBuilder {
 
         ListSection list = lists.get(section.name());
         if (list != null)
-            throw new NameAlreadyExistsTemplateException(section.name(), list.nameOffset(),
-                                                         section.nameOffset());
+            throw new NameAlreadyExistsTemplateException(section.name(), list, section);
 
         sampleIfSections.put(section.name(), section);
         allSections.add(section);
@@ -58,18 +56,15 @@ class TemplateBuilder {
     void addListSection(ListSection section) {
         VariableSection variableSection = sampleVariables.get(section.name());
         if (variableSection != null)
-            throw new NameAlreadyExistsTemplateException(section.name(), variableSection.nameOffset(),
-                                                         section.nameOffset());
+            throw new NameAlreadyExistsTemplateException(section.name(), variableSection, section);
 
         IfSection ifSection = sampleIfSections.get(section.name());
         if (ifSection != null)
-            throw new NameAlreadyExistsTemplateException(section.name(), ifSection.nameOffset(),
-                                                         section.nameOffset());
+            throw new NameAlreadyExistsTemplateException(section.name(), ifSection, section);
 
         ListSection previous = lists.put(section.name(), section);
         if (previous != null)
-            throw new NameAlreadyExistsTemplateException(section.name(), previous.nameOffset(),
-                                                         section.nameOffset());
+            throw new NameAlreadyExistsTemplateException(section.name(), previous, section);
         allSections.add(section);
     }
 
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateParser.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateParser.java
index c2202dea4a0..41fd716a3e6 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateParser.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateParser.java
@@ -47,9 +47,7 @@ class TemplateParser {
 
             if (current.eot()) {
                 if (!sentinels.contains(Sentinel.EOT)) {
-                    throw new BadTemplateException(current,
-                                                   "Missing end directive for section started at " +
-                                                   start.calculateLocation().lineAndColumnText());
+                    throw new BadTemplateException(start, "Missing end directive for section started");
                 }
                 return Sentinel.EOT;
             }
@@ -71,12 +69,12 @@ class TemplateParser {
             switch (type) {
                 case "else":
                     if (!sentinels.contains(Sentinel.ELSE))
-                        throw new BadTemplateException(startOfType, "Extraneous 'else'");
+                        throw new BadTemplateException(startOfType, "Stray 'else'");
                     parseEndDirective();
                     return Optional.of(Sentinel.ELSE);
                 case "end":
                     if (!sentinels.contains(Sentinel.END))
-                        throw new BadTemplateException(startOfType, "Extraneous 'end'");
+                        throw new BadTemplateException(startOfType, "Stray 'end'");
                     parseEndDirective();
                     return Optional.of(Sentinel.END);
                 case "if":
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/VariableSection.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/VariableSection.java
index 6a7bec2e485..17fedce55fa 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/VariableSection.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/template/VariableSection.java
@@ -15,7 +15,7 @@ class VariableSection extends Section {
     private final Cursor nameOffset;
 
     VariableSection(CursorRange range, String name, Cursor nameOffset) {
-        super(range);
+        super("variable", range);
         this.name = name;
         this.nameOffset = nameOffset;
     }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java
index e010b9780c6..b0260af1582 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/template/TemplateTest.java
@@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test;
 import java.nio.file.Path;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * @author hakonhall
@@ -147,6 +148,70 @@ public class TemplateTest {
                      template.render());
     }
 
+    @Test
+    void badTemplates() {
+        assertException(BadTemplateException.class, "Unknown section 'zoo' at line 2 and column 6",
+                        () -> Template.from("foo\nbar%{zoo}"));
+
+        assertException(BadTemplateException.class, "Expected identifier at line 1 and column 4",
+                        () -> Template.from("%{="));
+
+        assertException(BadTemplateException.class, "Expected identifier at line 1 and column 4",
+                        () -> Template.from("%{=&notatoken}"));
+
+        assertException(BadTemplateException.class, "Expected identifier at line 1 and column 8",
+                        () -> Template.from("%{list &notatoken}"));
+
+        assertException(BadTemplateException.class, "Missing end directive for section started at line 1 and column 12",
+                        () -> Template.from("%{list foo}missing end"));
+
+        assertException(BadTemplateException.class, "Stray 'end' at line 1 and column 3",
+                        () -> Template.from("%{end}stray end"));
+
+        assertException(TemplateNameNotSetException.class, "Variable at line 1 and column 4 has not been set: notset",
+                        () -> Template.from("%{=notset}").render());
+
+        assertException(TemplateNameNotSetException.class, "Variable at line 1 and column 6 has not been set: cond",
+                        () -> Template.from("%{if cond}%{end}").render());
+
+        assertException(NotBooleanValueTemplateException.class, "cond was set to a non-boolean value: must be true or false",
+                        () -> Template.from("%{if cond}%{end}").set("cond", 1).render());
+
+        assertException(NoSuchNameTemplateException.class, "No such element 'listname' in the template section starting at " +
+                                                           "line 1 and column 1, and ending at line 1 and column 4",
+                        () -> Template.from("foo").add("listname"));
+
+        assertException(NameAlreadyExistsTemplateException.class,
+                        "The name 'a' of the list section at line 1 and column 16 is in conflict with the identically " +
+                        "named list section at line 1 and column 1",
+                        () -> Template.from("%{list a}%{end}%{list a}%{end}"));
+
+        assertException(NameAlreadyExistsTemplateException.class,
+                        "The name 'a' of the list section at line 1 and column 6 is in conflict with the identically " +
+                        "named variable section at line 1 and column 1",
+                        () -> Template.from("%{=a}%{list a}%{end}"));
+
+        assertException(NameAlreadyExistsTemplateException.class,
+                        "The name 'a' of the variable section at line 1 and column 16 is in conflict with the identically " +
+                        "named list section at line 1 and column 1",
+                        () -> Template.from("%{list a}%{end}%{=a}"));
+
+        assertException(NameAlreadyExistsTemplateException.class,
+                        "The name 'a' of the list section at line 1 and column 14 is in conflict with the identically " +
+                        "named if section at line 1 and column 1",
+                        () -> Template.from("%{if a}%{end}%{list a}%{end}"));
+
+        assertException(NameAlreadyExistsTemplateException.class,
+                        "The name 'a' of the if section at line 1 and column 16 is in conflict with the identically " +
+                        "named list section at line 1 and column 1",
+                        () -> Template.from("%{list a}%{end}%{if a}%{end}"));
+    }
+
+    private <T extends Throwable> void assertException(Class<T> class_, String message, Runnable runnable) {
+        T exception = assertThrows(class_, runnable::run);
+        assertEquals(message, exception.getMessage());
+    }
+
     private Template getTemplate(String filename) {
         return Template.at(Path.of("src/test/resources/" + filename));
     }