Basic implementation of view include working

Author: jglick

groovy/src/main/java/org/kohsuke/stapler/jelly/groovy/JellyBuilder.java

@@ -469,6 +469,7 @@ public Object with(XMLOutput out, Closure c) {
      * @return null
      *      if nothing was generated.
      */
+    // TODO apparently unused
     public Element redirectToDom(Closure c) {
         SAXContentHandler sc = new SAXContentHandler();
         with(new XMLOutput(sc), c);

html/src/main/java/org/kohsuke/stapler/html/HtmlInclude.java

@@ -0,0 +1,20 @@
+package org.kohsuke.stapler.html;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Denotes that a record field should include another view.
+ * The field type should be Stapler-dispatchable.
+ */
+@Target(ElementType.RECORD_COMPONENT)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface HtmlInclude {
+    /**
+     * The name of the view to include.
+     * No file extension should be used, so for example use {@code index} or {@code config}.
+     */
+    String value();
+}

html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java

@@ -25,7 +25,7 @@ public HtmlJellyScript parse(URL script, MetaClass owner) throws Exception {
         LOGGER.info(() -> "TODO parsing " + script + " from " + owner);
         Class<?> c = owner.klass.toJavaClass();
         var base = c.getProtectionDomain().getCodeSource().getLocation()
-                + c.getName().replace('.', '/') + "/";
+                + c.getName().replace('.', '/').replace('$', '/') + "/";
         for (var m : c.getMethods()) {
             var ann = m.getAnnotation(HtmlView.class);
             if (ann == null) {

html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java

@@ -2,6 +2,7 @@
 
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.logging.Logger;
 import org.apache.commons.jelly.JellyContext;
@@ -10,6 +11,12 @@
 import org.apache.commons.jelly.Script;
 import org.apache.commons.jelly.XMLOutput;
 import org.dom4j.Element;
+import org.dom4j.Node;
+import org.dom4j.io.SAXContentHandler;
+import org.dom4j.io.SAXWriter;
+import org.kohsuke.stapler.WebApp;
+import org.kohsuke.stapler.html.HtmlInclude;
+import org.kohsuke.stapler.jelly.JellyClassTearOff;
 
 final class HtmlJellyScript implements Script {
 
@@ -34,13 +41,14 @@ public void run(JellyContext context, XMLOutput output) throws JellyTagException
         if (it == null) {
             throw new JellyTagException("No `it` bound");
         }
+        // TODO set thread name like JellyViewScript does
         try {
             var record = (Record) method.invoke(it);
             LOGGER.info(() -> "TODO " + method + " on " + it + " ⇒ " + record);
             // TODO find a more efficient way to render without allocation:
             var rendered = (Element) root.clone();
-            render(rendered, record);
-            rendered.write(output.asWriter());
+            render(context, rendered, record);
+            new SAXWriter(output, output).write(rendered);
         } catch (Exception x) {
             throw new JellyTagException(x);
         }
@@ -51,7 +59,7 @@ var record = (Record) method.invoke(it);
      * {@link Visitor} cannot be used easily here because it lacks any way to prune a tree
      * or record entry and exit from an element.
      */
-    private void render(Element rendered, Record record) throws Exception {
+    private void render(JellyContext context, Element rendered, Record record) throws Exception {
         for (var field : record.getClass().getRecordComponents()) {
             var id = "st." + field.getName();
             var elt = find(rendered, id);
@@ -60,22 +68,47 @@ private void render(Element rendered, Record record) throws Exception {
             }
             elt.remove(elt.attribute("id"));
             var value = field.getAccessor().invoke(record);
-            if (value instanceof String text) {
-                elt.setText(text);
+            var include = field.getAnnotation(HtmlInclude.class);
+            if (include != null) {
+                var metaClass = WebApp.getCurrent().getMetaClass(value.getClass());
+                var script = metaClass.loadTearOff(JellyClassTearOff.class).findScript(include.value());
+                var subcontext = new JellyContext(context);
+                subcontext.setExportLibraries(false); // defaults to true, weirdly
+                subcontext.setVariable("from", value);
+                subcontext.setVariable("it", value);
+                var handler = new SAXContentHandler();
+                var oldCCL = Thread.currentThread().getContextClassLoader();
+                Thread.currentThread().setContextClassLoader(metaClass.classLoader.loader);
+                try {
+                    script.run(subcontext, new XMLOutput(handler));
+                } finally {
+                    Thread.currentThread().setContextClassLoader(oldCCL);
+                }
+                // TODO honor JellyFacet.TRACE somehow
+                replace(elt, List.of(handler.getDocument().getRootElement()));
+            } else if (value instanceof String text) {
+                // TODO why do we need to replace entities like this?
+                // It is not necessary if run calls rendered.write(output.asWriter())
+                // but that does not seem right because it would be just sending text content, not XML.
+                // (And then includes do not work at all: the SAXContentHandler is left empty.)
+                // Also using SAXWriter causes DefaultScriptInvoker.createXMLOutput to use HTMLWriterOutput
+                // and thus dropping </p>, which does not match what actual Jenkins text/html output is like.
+                // Retest in the context of Jenkins which might wrap things differently.
+                elt.setText(text.replace("&", "&amp;").replace("<", "&lt;").replace(">", "&gt;"));
             } else if (value instanceof Boolean enabled) {
                 if (!enabled) {
                     elt.detach();
                 }
             } else if (value instanceof Record subrecord) {
-                render(elt, subrecord);
+                render(context, elt, subrecord);
             } else if (value instanceof List<?> list) {
-                var parent = elt.getParent();
-                elt.detach();
+                var replacements = new ArrayList<Element>();
                 for (var item : list) {
                     var elt2 = (Element) elt.clone();
-                    render(elt2, (Record) item);
-                    parent.add(elt2);
+                    render(context, elt2, (Record) item);
+                    replacements.add(elt2);
                 }
+                replace(elt, replacements);
             } else if (value == null) {
                 elt.detach();
             } else {
@@ -88,7 +121,7 @@ private void render(Element rendered, Record record) throws Exception {
      * Like {@link Element#elementByID} except using attribute {@code id} not {@code ID}.
      */
     @CheckForNull
-    private Element find(Element elt, String id) {
+    private static Element find(Element elt, String id) {
         if (id.equals(elt.attributeValue("id"))) {
             return elt;
         }
@@ -100,4 +133,17 @@ private Element find(Element elt, String id) {
         }
         return null;
     }
+
+    // TODO dom4j does not seem to define an insertAt or replace
+    // also Element.node(int) does not work as documented
+    private static void replace(Element elt, List<Element> replacements) {
+        var parent = elt.getParent();
+        var kids = new ArrayList<Node>();
+        parent.nodeIterator().forEachRemaining(kids::add);
+        kids.stream().forEach(Node::detach);
+        int index = kids.indexOf(elt);
+        kids.remove(index);
+        kids.addAll(index, replacements);
+        kids.stream().forEach(parent::add);
+    }
 }

html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java

@@ -4,6 +4,8 @@
 
 public abstract class HtmlTestCase extends JettyTestCase {
 
+    // TODO enable fine logging in subpackages
+
     protected final String load(String uri) throws Exception {
         return createWebClient()
                 .getPage(url.toURI().resolve(uri).toURL())

html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java

@@ -8,12 +8,13 @@ public final class IncludedFromTest extends HtmlTestCase {
     public void testIncludedFromJelly() throws Exception {
         assertThat(
                 load("top"),
-                is("A prologue. <div> Center text includes a <span>special value</span>. </div> An epilogue."));
+                is(
+                        "A prologue. <div> Center text includes a <span>special &lt;cool&gt; value</span>. </div> An epilogue."));
     }
 
     @HtmlView("center")
     public Center getCenter() {
-        return new Center("special value");
+        return new Center("special <cool> value");
     }
 
     public record Center(String value) {}

html/src/test/java/org/kohsuke/stapler/html/IncludesTest.java

@@ -0,0 +1,31 @@
+package org.kohsuke.stapler.html;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+public final class IncludesTest extends HtmlTestCase {
+
+    public void testIncludes() throws Exception {
+        assertThat(
+                load("top"),
+                is(
+                        "<div> A prologue. <span style='color: red'> There are <span>23</span> items. </span> An epilogue. </div>"));
+    }
+
+    @HtmlView("top")
+    public Top getTop() {
+        return new Top(new Nested());
+    }
+
+    public record Top(@HtmlInclude("center") Nested nested) {}
+
+    public static final class Nested {
+
+        @HtmlView("center")
+        public Center getCenter() {
+            return new Center("23");
+        }
+
+        public record Center(String count) {}
+    }
+}

html/src/test/java/org/kohsuke/stapler/html/StructureTest.java

@@ -23,8 +23,12 @@ public void testStandalone() throws Exception {
                 load("standalone"),
                 allOf(
                         containsString("<title>Something #123</title>"),
-                        containsString(
-                                "<p>All items:</p> <table style='border: 1px solid black'> <tr> <td>k1</td> <td>v1</td> </tr><tr> <td>k2</td> <td>v2 &amp;c.</td> </tr></table>"),
+                        containsString("<p>All items: <table style='border: 1px solid black'> "
+                                + "<tr><td>Initial name</td><td>Initial value</td></tr> "
+                                + "<tr> <td>k1</td> <td>v1</td> </tr><tr> <td>k2</td> <td>v2 &amp;c.</td> </tr> "
+                                + "<tr><td>Penultimate name</td><td>Penultimate value</td></tr> "
+                                + "<tr><td>Final name</td><td>Final value</td></tr> "
+                                + "</table>"),
                         not(containsString("Error in")),
                         not(containsString("There are"))));
         status = new Status("Something #456", new Status.Warning("rotor"), true, null);
@@ -33,8 +37,8 @@ public void testStandalone() throws Exception {
                 allOf(
                         containsString("<title>Something #456</title>"),
                         not(containsString("All items:")),
-                        containsString("<p style='color: red'>Error in <code>rotor</code>.</p>"),
-                        containsString("<p>There are <em>no</em> items.</p>")));
+                        containsString("<p style='color: red'>Error in <code>rotor</code>."),
+                        containsString("<p>There are <em>no</em> items.")));
     }
 
     @HtmlView("standalone")

html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/Nested/center.xhtml

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<span style="color: red">
+    There are <span id="st.count">1234</span> items.
+</span>

html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/top.xhtml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<div>
+    A prologue.
+    <span id="st.nested">sample interior text</span>
+    An epilogue.
+</div>