From 73676e18bb3fd3ce454990f17926fc8d9f4b5f54 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 17 Feb 2025 11:31:06 -0500 Subject: [PATCH 1/7] [JENKINS-31203] Sketch of HTML facet --- html/pom.xml | 81 ++++++++++++++++++ .../stapler/html/HtmlClassLoaderTearOff.java | 45 ++++++++++ .../stapler/html/HtmlClassTearOff.java | 25 ++++++ .../org/kohsuke/stapler/html/HtmlFacet.java | 83 +++++++++++++++++++ .../kohsuke/stapler/html/HtmlJellyScript.java | 65 +++++++++++++++ .../org/kohsuke/stapler/html/HtmlView.java | 17 ++++ .../stapler/html/HtmlViewRenderer.java | 18 ++++ .../org/kohsuke/stapler/html/SmokeTest.java | 40 +++++++++ .../stapler/html/SmokeTest/standalone.xhtml | 9 ++ pom.xml | 1 + 10 files changed, 384 insertions(+) create mode 100644 html/pom.xml create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlView.java create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java create mode 100644 html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java create mode 100644 html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml diff --git a/html/pom.xml b/html/pom.xml new file mode 100644 index 000000000..141afeeea --- /dev/null +++ b/html/pom.xml @@ -0,0 +1,81 @@ + + + 4.0.0 + + org.kohsuke.stapler + stapler-parent + ${changelist} + + stapler-html + + + ${project.groupId} + stapler-jelly + ${project.version} + + + org.kohsuke.metainf-services + metainf-services + true + + + jakarta.servlet + jakarta.servlet-api + 5.0.0 + provided + + + ${project.groupId} + stapler + ${project.version} + tests + test + + + org.eclipse.jetty + jetty-server + test + + + org.eclipse.jetty + jetty-util + test + + + org.eclipse.jetty.ee9 + jetty-ee9-servlet + test + + + org.eclipse.jetty.ee9 + jetty-ee9-webapp + test + + + org.hamcrest + hamcrest + test + + + org.jenkins-ci.main + jenkins-test-harness-htmlunit + test + + + org.junit.jupiter + junit-jupiter + test + + + org.junit.vintage + junit-vintage-engine + test + + + org.slf4j + slf4j-jdk14 + 2.0.16 + test + + + diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java new file mode 100644 index 000000000..3fa8be386 --- /dev/null +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java @@ -0,0 +1,45 @@ +package org.kohsuke.stapler.html; + +import java.net.URL; +import java.util.logging.Logger; +import org.dom4j.io.SAXReader; +import org.kohsuke.stapler.MetaClass; +import org.kohsuke.stapler.MetaClassLoader; + +public final class HtmlClassLoaderTearOff { + private static final Logger LOGGER = Logger.getLogger(HtmlClassLoaderTearOff.class.getName()); + private final MetaClassLoader owner; + private final SAXReader parser; + + public HtmlClassLoaderTearOff(MetaClassLoader owner) throws Exception { + this.owner = owner; + parser = new SAXReader(); + parser.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + parser.setFeature("http://xml.org/sax/features/external-general-entities", false); + parser.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + } + + public HtmlJellyScript parse(URL script, MetaClass owner) throws Exception { + LOGGER.info(() -> "TODO parsing " + script + " from " + owner); + // Some of this could be precomputed in HtmlClassTearOff, more efficiently for classes with many views: + Class c = owner.klass.toJavaClass(); + var base = c.getProtectionDomain().getCodeSource().getLocation() + + c.getName().replace('.', '/') + "/"; + for (var m : c.getMethods()) { + var ann = m.getAnnotation(HtmlView.class); + if (ann == null) { + continue; + } + if (m.getParameterCount() > 0) { + throw new Exception(m + " must not take arguments"); + } + if (m.getReturnType() != HtmlViewRenderer.class) { + throw new Exception(m + " must return " + HtmlViewRenderer.class); + } + if (script.toString().equals(base + ann.value() + ".xhtml")) { + return new HtmlJellyScript(m, parser.read(script).getRootElement()); + } + } + throw new Exception(c + " does not have a @HtmlView corresponding to " + script); + } +} diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java new file mode 100644 index 000000000..987015cba --- /dev/null +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java @@ -0,0 +1,25 @@ +package org.kohsuke.stapler.html; + +import java.net.URL; +import java.util.logging.Logger; +import org.kohsuke.stapler.AbstractTearOff; +import org.kohsuke.stapler.MetaClass; + +public final class HtmlClassTearOff extends AbstractTearOff { + private static final Logger LOGGER = Logger.getLogger(HtmlClassTearOff.class.getName()); + + public HtmlClassTearOff(MetaClass owner) { + super(owner, HtmlClassLoaderTearOff.class); + LOGGER.fine(() -> "initialized " + owner); + } + + @Override + protected String getDefaultScriptExtension() { + return ".xhtml"; + } + + @Override + public HtmlJellyScript parseScript(URL res) throws Exception { + return classLoader.parse(res, owner); + } +} diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java new file mode 100644 index 000000000..629cec77b --- /dev/null +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java @@ -0,0 +1,83 @@ +package org.kohsuke.stapler.html; + +import static org.kohsuke.stapler.Facet.LOGGER; + +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletException; +import java.io.IOException; +import java.util.Collection; +import java.util.List; +import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.apache.commons.jelly.JellyException; +import org.apache.commons.jelly.Script; +import org.kohsuke.MetaInfServices; +import org.kohsuke.stapler.AbstractTearOff; +import org.kohsuke.stapler.Dispatcher; +import org.kohsuke.stapler.Facet; +import org.kohsuke.stapler.MetaClass; +import org.kohsuke.stapler.RequestImpl; +import org.kohsuke.stapler.ResponseImpl; +import org.kohsuke.stapler.jelly.JellyClassTearOff; +import org.kohsuke.stapler.jelly.JellyCompatibleFacet; +import org.kohsuke.stapler.jelly.JellyFacet; +import org.kohsuke.stapler.lang.Klass; + +@MetaInfServices(Facet.class) +public final class HtmlFacet extends Facet implements JellyCompatibleFacet { + + private static final Logger LOGGER = Logger.getLogger(HtmlFacet.class.getName()); + + // TODO seems like there could be some default methods in JellyCompatibleFacet to avoid boilerplate + + @Override + public void buildViewDispatchers(MetaClass owner, List dispatchers) { + dispatchers.add(createValidatingDispatcher( + owner.loadTearOff(HtmlClassTearOff.class), owner.webApp.getFacet(JellyFacet.class).scriptInvoker)); + } + + @Override + public RequestDispatcher createRequestDispatcher(RequestImpl request, Klass type, Object it, String viewName) + throws IOException { + // TODO is this actually used? + return createRequestDispatcher( + request.getWebApp().getMetaClass(type).loadTearOff(HtmlClassTearOff.class), + request.getWebApp().getFacet(JellyFacet.class).scriptInvoker, + it, + viewName); + } + + @Override + public void buildIndexDispatchers(MetaClass owner, List dispatchers) { + try { + if (owner.loadTearOff(JellyClassTearOff.class).findScript("index") != null) { + super.buildIndexDispatchers(owner, dispatchers); + } + } catch (JellyException e) { + LOGGER.log(Level.WARNING, "Failed to parse index.xhtml for " + owner, e); + } + } + + @Override + public boolean handleIndexRequest(RequestImpl req, ResponseImpl rsp, Object node, MetaClass nodeMetaClass) + throws IOException, ServletException { + return handleIndexRequest( + nodeMetaClass.loadTearOff(HtmlClassTearOff.class), + req.getWebApp().getFacet(JellyFacet.class).scriptInvoker, + req, + rsp, + node); + } + + @Override + public Collection>> getClassTearOffTypes() { + return Set.of(HtmlClassTearOff.class); + } + + @Override + public Collection getScriptExtensions() { + // TODO allow *.html if it can be parsed without external deps + return Set.of(".xhtml"); + } +} diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java new file mode 100644 index 000000000..b6137ee43 --- /dev/null +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java @@ -0,0 +1,65 @@ +package org.kohsuke.stapler.html; + +import java.lang.reflect.Method; +import java.util.logging.Logger; +import org.apache.commons.jelly.JellyContext; +import org.apache.commons.jelly.JellyException; +import org.apache.commons.jelly.JellyTagException; +import org.apache.commons.jelly.Script; +import org.apache.commons.jelly.XMLOutput; +import org.dom4j.Element; +import org.dom4j.VisitorSupport; + +final class HtmlJellyScript implements Script { + + private static final Logger LOGGER = Logger.getLogger(HtmlJellyScript.class.getName()); + + private final Method method; + private final Element root; + + HtmlJellyScript(Method method, Element root) { + this.method = method; + this.root = root; + } + + @Override + public Script compile() throws JellyException { + return this; + } + + @Override + public void run(JellyContext context, XMLOutput output) throws JellyTagException { + var it = context.getVariable("it"); + if (it == null) { + throw new JellyTagException("No `it` bound"); + } + LOGGER.info(() -> "TODO running " + method + " on " + it); + try { + var renderer = (HtmlViewRenderer) method.invoke(it); + // Could find a more efficient way to render without allocation: + var rendered = (Element) root.clone(); + rendered.accept(new VisitorSupport() { + @Override + public void visit(Element node) { + var id = node.attributeValue("id"); + if (id != null) { + String text; + try { + text = renderer.supplyText(id); + } catch (RuntimeException x) { + throw x; + } catch (Exception x) { + throw new RuntimeException(x); + } + if (text != null) { + node.addText(text); + } + } + } + }); + rendered.write(output.asWriter()); + } catch (Exception x) { + throw new JellyTagException(x); + } + } +} diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java new file mode 100644 index 000000000..2cc6f7d56 --- /dev/null +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java @@ -0,0 +1,17 @@ +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; + +/** + * A method which renders an HTML view. + * The method must be public, take no arguments, and return {@link HtmlViewRenderer}. + */ +@Target(ElementType.METHOD) +@Retention(RetentionPolicy.RUNTIME) +public @interface HtmlView { + /** View name, which should be {@code *.xhtml} base name. */ + String value(); +} diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java new file mode 100644 index 000000000..ceedb49e9 --- /dev/null +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java @@ -0,0 +1,18 @@ +package org.kohsuke.stapler.html; + +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; + +/** + * A renderer for {@link HtmlView}. + */ +public interface HtmlViewRenderer { + + /** + * Dynamically injects text context into an element. + * @param id an HTML {@code id} attribute found in the content + * @return plain text to insert, or null to skip + */ + @CheckForNull + String supplyText(@NonNull String id) throws Exception; +} diff --git a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java b/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java new file mode 100644 index 000000000..9293c4d1e --- /dev/null +++ b/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java @@ -0,0 +1,40 @@ +package org.kohsuke.stapler.html; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; + +import org.htmlunit.html.HtmlPage; +import org.kohsuke.stapler.test.JettyTestCase; + +public final class SmokeTest extends JettyTestCase { + + String text; + + public void testStandalone() throws Exception { + text = "Some text & stuff."; + var wc = createWebClient(); + HtmlPage page = wc.getPage(url.toURI().resolve("standalone").toURL()); + assertThat( + page.getWebResponse().getContentAsString().replaceAll("\\s+", " "), + containsString("

Some text & stuff.

")); + text = "Altered text."; + page = wc.getPage(url.toURI().resolve("standalone").toURL()); + assertThat( + page.getWebResponse().getContentAsString().replaceAll("\\s+", " "), + containsString("

Altered text.

")); + } + + @HtmlView("standalone") + public HtmlViewRenderer renderStandalone() throws Exception { + return new HtmlViewRenderer() { + @Override + public String supplyText(String id) throws Exception { + if (id.equals("main")) { + return text; + } else { + throw new IllegalArgumentException(); + } + } + }; + } +} diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml new file mode 100644 index 000000000..11477221d --- /dev/null +++ b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml @@ -0,0 +1,9 @@ + + + + Standalone page + + +

+ + diff --git a/pom.xml b/pom.xml index af62c3130..5e24c689b 100644 --- a/pom.xml +++ b/pom.xml @@ -29,6 +29,7 @@ jsp jelly groovy + html From 45ab1dc088e9a40ccbc7629538467ce8654eedab Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 17 Feb 2025 17:30:02 -0500 Subject: [PATCH 2/7] Creating `impl` package --- .../stapler/html/{ => impl}/HtmlClassLoaderTearOff.java | 4 +++- .../org/kohsuke/stapler/html/{ => impl}/HtmlClassTearOff.java | 2 +- .../java/org/kohsuke/stapler/html/{ => impl}/HtmlFacet.java | 2 +- .../org/kohsuke/stapler/html/{ => impl}/HtmlJellyScript.java | 3 ++- 4 files changed, 7 insertions(+), 4 deletions(-) rename html/src/main/java/org/kohsuke/stapler/html/{ => impl}/HtmlClassLoaderTearOff.java (93%) rename html/src/main/java/org/kohsuke/stapler/html/{ => impl}/HtmlClassTearOff.java (94%) rename html/src/main/java/org/kohsuke/stapler/html/{ => impl}/HtmlFacet.java (98%) rename html/src/main/java/org/kohsuke/stapler/html/{ => impl}/HtmlJellyScript.java (96%) diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java similarity index 93% rename from html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java rename to html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java index 3fa8be386..028d00ee3 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlClassLoaderTearOff.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java @@ -1,10 +1,12 @@ -package org.kohsuke.stapler.html; +package org.kohsuke.stapler.html.impl; import java.net.URL; import java.util.logging.Logger; import org.dom4j.io.SAXReader; import org.kohsuke.stapler.MetaClass; import org.kohsuke.stapler.MetaClassLoader; +import org.kohsuke.stapler.html.HtmlView; +import org.kohsuke.stapler.html.HtmlViewRenderer; public final class HtmlClassLoaderTearOff { private static final Logger LOGGER = Logger.getLogger(HtmlClassLoaderTearOff.class.getName()); diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassTearOff.java similarity index 94% rename from html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java rename to html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassTearOff.java index 987015cba..7ca466ad9 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlClassTearOff.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassTearOff.java @@ -1,4 +1,4 @@ -package org.kohsuke.stapler.html; +package org.kohsuke.stapler.html.impl; import java.net.URL; import java.util.logging.Logger; diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java similarity index 98% rename from html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java rename to html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java index 629cec77b..2420348dc 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlFacet.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java @@ -1,4 +1,4 @@ -package org.kohsuke.stapler.html; +package org.kohsuke.stapler.html.impl; import static org.kohsuke.stapler.Facet.LOGGER; diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java similarity index 96% rename from html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java rename to html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java index b6137ee43..becd90e76 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlJellyScript.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java @@ -1,4 +1,4 @@ -package org.kohsuke.stapler.html; +package org.kohsuke.stapler.html.impl; import java.lang.reflect.Method; import java.util.logging.Logger; @@ -9,6 +9,7 @@ import org.apache.commons.jelly.XMLOutput; import org.dom4j.Element; import org.dom4j.VisitorSupport; +import org.kohsuke.stapler.html.HtmlViewRenderer; final class HtmlJellyScript implements Script { From c50dbd56854ae474cc9fae1011c677ed23025bb3 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 17 Feb 2025 19:07:00 -0500 Subject: [PATCH 3/7] Return `record`s to reflect structure of rendered text directly --- .../org/kohsuke/stapler/html/HtmlView.java | 11 ++- .../stapler/html/HtmlViewRenderer.java | 18 ---- .../html/impl/HtmlClassLoaderTearOff.java | 9 +- .../stapler/html/impl/HtmlJellyScript.java | 85 +++++++++++++------ .../org/kohsuke/stapler/html/SmokeTest.java | 70 ++++++++++----- .../stapler/html/SmokeTest/standalone.xhtml | 18 +++- 6 files changed, 140 insertions(+), 71 deletions(-) delete mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java index 2cc6f7d56..6d3c70770 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java @@ -7,7 +7,16 @@ /** * A method which renders an HTML view. - * The method must be public, take no arguments, and return {@link HtmlViewRenderer}. + * The method must be public, take no arguments, and return a {@link Record}. + * The fields of the record must align with the element {@code id}s + * with names starting with the {@code st.} prefix. + * The following field types are permitted: + *

    + *
  • {@link String}, to insert character data. + *
  • {@link boolean}, to conditionally include a static subtree. + *
  • Some other {@link Record} type, to include a nested structure, skipped if {@code null}. + *
  • {@link List} of some {@link Record} type, to repeat a nested structure zero or more times. + *
*/ @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java deleted file mode 100644 index ceedb49e9..000000000 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlViewRenderer.java +++ /dev/null @@ -1,18 +0,0 @@ -package org.kohsuke.stapler.html; - -import edu.umd.cs.findbugs.annotations.CheckForNull; -import edu.umd.cs.findbugs.annotations.NonNull; - -/** - * A renderer for {@link HtmlView}. - */ -public interface HtmlViewRenderer { - - /** - * Dynamically injects text context into an element. - * @param id an HTML {@code id} attribute found in the content - * @return plain text to insert, or null to skip - */ - @CheckForNull - String supplyText(@NonNull String id) throws Exception; -} diff --git a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java index 028d00ee3..41bcdce4d 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java @@ -6,7 +6,6 @@ import org.kohsuke.stapler.MetaClass; import org.kohsuke.stapler.MetaClassLoader; import org.kohsuke.stapler.html.HtmlView; -import org.kohsuke.stapler.html.HtmlViewRenderer; public final class HtmlClassLoaderTearOff { private static final Logger LOGGER = Logger.getLogger(HtmlClassLoaderTearOff.class.getName()); @@ -19,11 +18,11 @@ public HtmlClassLoaderTearOff(MetaClassLoader owner) throws Exception { parser.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); parser.setFeature("http://xml.org/sax/features/external-general-entities", false); parser.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + // TODO enable validation against XHTML 1.1 schema when in test mode (-ea) } public HtmlJellyScript parse(URL script, MetaClass owner) throws Exception { LOGGER.info(() -> "TODO parsing " + script + " from " + owner); - // Some of this could be precomputed in HtmlClassTearOff, more efficiently for classes with many views: Class c = owner.klass.toJavaClass(); var base = c.getProtectionDomain().getCodeSource().getLocation() + c.getName().replace('.', '/') + "/"; @@ -35,9 +34,11 @@ public HtmlJellyScript parse(URL script, MetaClass owner) throws Exception { if (m.getParameterCount() > 0) { throw new Exception(m + " must not take arguments"); } - if (m.getReturnType() != HtmlViewRenderer.class) { - throw new Exception(m + " must return " + HtmlViewRenderer.class); + if (!m.getReturnType().isRecord()) { + throw new Exception(m + " must return a record"); } + // TODO verify that the Record fields & nesting match st.* DOM structure + // (or do this all at compile time using an annotation processor) if (script.toString().equals(base + ann.value() + ".xhtml")) { return new HtmlJellyScript(m, parser.read(script).getRootElement()); } diff --git a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java index becd90e76..713edc1ec 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java @@ -1,6 +1,8 @@ package org.kohsuke.stapler.html.impl; +import edu.umd.cs.findbugs.annotations.CheckForNull; import java.lang.reflect.Method; +import java.util.List; import java.util.logging.Logger; import org.apache.commons.jelly.JellyContext; import org.apache.commons.jelly.JellyException; @@ -8,8 +10,6 @@ import org.apache.commons.jelly.Script; import org.apache.commons.jelly.XMLOutput; import org.dom4j.Element; -import org.dom4j.VisitorSupport; -import org.kohsuke.stapler.html.HtmlViewRenderer; final class HtmlJellyScript implements Script { @@ -34,33 +34,70 @@ public void run(JellyContext context, XMLOutput output) throws JellyTagException if (it == null) { throw new JellyTagException("No `it` bound"); } - LOGGER.info(() -> "TODO running " + method + " on " + it); try { - var renderer = (HtmlViewRenderer) method.invoke(it); - // Could find a more efficient way to render without allocation: + 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(); - rendered.accept(new VisitorSupport() { - @Override - public void visit(Element node) { - var id = node.attributeValue("id"); - if (id != null) { - String text; - try { - text = renderer.supplyText(id); - } catch (RuntimeException x) { - throw x; - } catch (Exception x) { - throw new RuntimeException(x); - } - if (text != null) { - node.addText(text); - } - } - } - }); + render(rendered, record); rendered.write(output.asWriter()); } catch (Exception x) { throw new JellyTagException(x); } } + + /** + * Render a view or subtree of a view. + * {@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 { + for (var field : record.getClass().getRecordComponents()) { + var id = "st." + field.getName(); + var elt = find(rendered, id); + if (elt == null) { + throw new IllegalArgumentException("did not find " + id + " in " + rendered); + } + elt.remove(elt.attribute("id")); + var value = field.getAccessor().invoke(record); + if (value instanceof String text) { + elt.setText(text); + } else if (value instanceof Boolean enabled) { + if (!enabled) { + elt.detach(); + } + } else if (value instanceof Record subrecord) { + render(elt, subrecord); + } else if (value instanceof List list) { + var parent = elt.getParent(); + elt.detach(); + for (var item : list) { + var elt2 = (Element) elt.clone(); + render(elt2, (Record) item); + parent.add(elt2); + } + } else if (value == null) { + elt.detach(); + } else { + throw new IllegalArgumentException("did not recognize " + value); + } + } + } + + /** + * Like {@link Element#elementByID} except using attribute {@code id} not {@code ID}. + */ + @CheckForNull + private Element find(Element elt, String id) { + if (id.equals(elt.attributeValue("id"))) { + return elt; + } + for (var child : elt.elements()) { + var found = find(child, id); + if (found != null) { + return found; + } + } + return null; + } } diff --git a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java b/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java index 9293c4d1e..788701f5c 100644 --- a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java +++ b/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java @@ -1,40 +1,66 @@ package org.kohsuke.stapler.html; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; -import org.htmlunit.html.HtmlPage; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import java.util.List; import org.kohsuke.stapler.test.JettyTestCase; public final class SmokeTest extends JettyTestCase { - String text; + Status status; public void testStandalone() throws Exception { - text = "Some text & stuff."; - var wc = createWebClient(); - HtmlPage page = wc.getPage(url.toURI().resolve("standalone").toURL()); + status = new Status( + "Something #123", + null, + false, + new Status.Items( + List.of(new Status.Items.Element("k1", "v1"), new Status.Items.Element("k2", "v2 &c.")))); assertThat( - page.getWebResponse().getContentAsString().replaceAll("\\s+", " "), - containsString("

Some text & stuff.

")); - text = "Altered text."; - page = wc.getPage(url.toURI().resolve("standalone").toURL()); + load("standalone"), + allOf( + containsString("Something #123"), + containsString( + "

All items:

k1 v1
k2 v2 &c.
"), + not(containsString("Error in")), + not(containsString("There are")))); + status = new Status("Something #456", new Status.Warning("rotor"), true, null); assertThat( - page.getWebResponse().getContentAsString().replaceAll("\\s+", " "), - containsString("

Altered text.

")); + load("standalone"), + allOf( + containsString("Something #456"), + not(containsString("All items:")), + containsString("

Error in rotor.

"), + containsString("

There are no items.

"))); + } + + private String load(String uri) throws Exception { + return createWebClient() + .getPage(url.toURI().resolve(uri).toURL()) + .getWebResponse() + .getContentAsString() + .replaceAll("", " ") + .replaceAll("\\s+", " ") + .replace('"', '\''); } @HtmlView("standalone") - public HtmlViewRenderer renderStandalone() throws Exception { - return new HtmlViewRenderer() { - @Override - public String supplyText(String id) throws Exception { - if (id.equals("main")) { - return text; - } else { - throw new IllegalArgumentException(); - } - } - }; + public Status getStandalone() throws Exception { + return status; + } + + public record Status( + String displayName, @CheckForNull Warning warning, boolean empty, @CheckForNull Items nonempty) { + + public record Warning(String component) {} + + public record Items(List elements) { + + public record Element(String name, String value) {} + } } } diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml index 11477221d..9fa893d61 100644 --- a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml +++ b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml @@ -1,9 +1,23 @@ - Standalone page + + XXX #123 -

+ +

Error in some component.

+ +

There are no items.

+
+

All items:

+ + + + + + +
namevalue
+
From ad61130ebd0cd50d0c3b4844462786712a346e40 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 17 Feb 2025 19:42:01 -0500 Subject: [PATCH 4/7] Demonstrating that it is possible to embed in Jelly --- .../kohsuke/stapler/html/impl/HtmlFacet.java | 1 + .../org/kohsuke/stapler/html/SmokeTest.java | 35 +++++++++++++------ .../stapler/html/SmokeTest/center.xhtml | 5 +++ .../kohsuke/stapler/html/SmokeTest/top.jelly | 7 ++++ 4 files changed, 38 insertions(+), 10 deletions(-) create mode 100644 html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/center.xhtml create mode 100644 html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/top.jelly diff --git a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java index 2420348dc..26cfc1a7d 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java +++ b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlFacet.java @@ -78,6 +78,7 @@ public boolean handleIndexRequest(RequestImpl req, ResponseImpl rsp, Object node @Override public Collection getScriptExtensions() { // TODO allow *.html if it can be parsed without external deps + // or use *.xml? return Set.of(".xhtml"); } } diff --git a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java b/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java index 788701f5c..0d2bea16b 100644 --- a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java +++ b/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java @@ -3,6 +3,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import edu.umd.cs.findbugs.annotations.CheckForNull; @@ -38,16 +39,6 @@ public void testStandalone() throws Exception { containsString("

There are no items.

"))); } - private String load(String uri) throws Exception { - return createWebClient() - .getPage(url.toURI().resolve(uri).toURL()) - .getWebResponse() - .getContentAsString() - .replaceAll("", " ") - .replaceAll("\\s+", " ") - .replace('"', '\''); - } - @HtmlView("standalone") public Status getStandalone() throws Exception { return status; @@ -63,4 +54,28 @@ public record Items(List elements) { public record Element(String name, String value) {} } } + + public void testCalledFromJelly() throws Exception { + assertThat( + load("top"), + is("A prologue.
Center text includes a special value.
An epilogue.")); + } + + @HtmlView("center") + public Center getCenter() { + return new Center("special value"); + } + + public record Center(String value) {} + + private String load(String uri) throws Exception { + return createWebClient() + .getPage(url.toURI().resolve(uri).toURL()) + .getWebResponse() + .getContentAsString() + .replaceAll("", " ") + .replaceAll("\\s+", " ") + .trim() + .replace('"', '\''); + } } diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/center.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/center.xhtml new file mode 100644 index 000000000..a6777fa92 --- /dev/null +++ b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/center.xhtml @@ -0,0 +1,5 @@ + + +
+ Center text includes a value. +
diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/top.jelly b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/top.jelly new file mode 100644 index 000000000..e7a589fb8 --- /dev/null +++ b/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/top.jelly @@ -0,0 +1,7 @@ + + + + A prologue. + + An epilogue. + From ab9442d0cd35a8e3292bc5841420908344a0e90c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Sun, 2 Mar 2025 07:48:13 -0500 Subject: [PATCH 5/7] Javadoc --- html/src/main/java/org/kohsuke/stapler/html/HtmlView.java | 1 + 1 file changed, 1 insertion(+) diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java index 6d3c70770..867d010c8 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java +++ b/html/src/main/java/org/kohsuke/stapler/html/HtmlView.java @@ -4,6 +4,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.List; /** * A method which renders an HTML view. From 35493ac40222920c0a664368555f3d95e0e9cac5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Sun, 2 Mar 2025 08:13:49 -0500 Subject: [PATCH 6/7] Restructured test cases --- .../kohsuke/stapler/html/HtmlTestCase.java | 17 +++++++++++ .../stapler/html/IncludedFromTest.java | 20 +++++++++++++ .../{SmokeTest.java => StructureTest.java} | 28 +------------------ .../center.xhtml | 0 .../{SmokeTest => IncludedFromTest}/top.jelly | 0 .../standalone.xhtml | 0 6 files changed, 38 insertions(+), 27 deletions(-) create mode 100644 html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java create mode 100644 html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java rename html/src/test/java/org/kohsuke/stapler/html/{SmokeTest.java => StructureTest.java} (69%) rename html/src/test/resources/org/kohsuke/stapler/html/{SmokeTest => IncludedFromTest}/center.xhtml (100%) rename html/src/test/resources/org/kohsuke/stapler/html/{SmokeTest => IncludedFromTest}/top.jelly (100%) rename html/src/test/resources/org/kohsuke/stapler/html/{SmokeTest => StructureTest}/standalone.xhtml (100%) diff --git a/html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java b/html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java new file mode 100644 index 000000000..bbcb31cf2 --- /dev/null +++ b/html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java @@ -0,0 +1,17 @@ +package org.kohsuke.stapler.html; + +import org.kohsuke.stapler.test.JettyTestCase; + +public abstract class HtmlTestCase extends JettyTestCase { + + protected final String load(String uri) throws Exception { + return createWebClient() + .getPage(url.toURI().resolve(uri).toURL()) + .getWebResponse() + .getContentAsString() + .replaceAll("", " ") + .replaceAll("\\s+", " ") + .trim() + .replace('"', '\''); + } +} diff --git a/html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java b/html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java new file mode 100644 index 000000000..56ff8d78e --- /dev/null +++ b/html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java @@ -0,0 +1,20 @@ +package org.kohsuke.stapler.html; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + +public final class IncludedFromTest extends HtmlTestCase { + + public void testIncludedFromJelly() throws Exception { + assertThat( + load("top"), + is("A prologue.
Center text includes a special value.
An epilogue.")); + } + + @HtmlView("center") + public Center getCenter() { + return new Center("special value"); + } + + public record Center(String value) {} +} diff --git a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java b/html/src/test/java/org/kohsuke/stapler/html/StructureTest.java similarity index 69% rename from html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java rename to html/src/test/java/org/kohsuke/stapler/html/StructureTest.java index 0d2bea16b..d0c4801f4 100644 --- a/html/src/test/java/org/kohsuke/stapler/html/SmokeTest.java +++ b/html/src/test/java/org/kohsuke/stapler/html/StructureTest.java @@ -3,14 +3,12 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import edu.umd.cs.findbugs.annotations.CheckForNull; import java.util.List; -import org.kohsuke.stapler.test.JettyTestCase; -public final class SmokeTest extends JettyTestCase { +public final class StructureTest extends HtmlTestCase { Status status; @@ -54,28 +52,4 @@ public record Items(List elements) { public record Element(String name, String value) {} } } - - public void testCalledFromJelly() throws Exception { - assertThat( - load("top"), - is("A prologue.
Center text includes a special value.
An epilogue.")); - } - - @HtmlView("center") - public Center getCenter() { - return new Center("special value"); - } - - public record Center(String value) {} - - private String load(String uri) throws Exception { - return createWebClient() - .getPage(url.toURI().resolve(uri).toURL()) - .getWebResponse() - .getContentAsString() - .replaceAll("", " ") - .replaceAll("\\s+", " ") - .trim() - .replace('"', '\''); - } } diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/center.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/IncludedFromTest/center.xhtml similarity index 100% rename from html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/center.xhtml rename to html/src/test/resources/org/kohsuke/stapler/html/IncludedFromTest/center.xhtml diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/top.jelly b/html/src/test/resources/org/kohsuke/stapler/html/IncludedFromTest/top.jelly similarity index 100% rename from html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/top.jelly rename to html/src/test/resources/org/kohsuke/stapler/html/IncludedFromTest/top.jelly diff --git a/html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/StructureTest/standalone.xhtml similarity index 100% rename from html/src/test/resources/org/kohsuke/stapler/html/SmokeTest/standalone.xhtml rename to html/src/test/resources/org/kohsuke/stapler/html/StructureTest/standalone.xhtml From 6a8257c9be7aaac169ca8f48937ce90da3d72ca5 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Sun, 2 Mar 2025 11:43:01 -0500 Subject: [PATCH 7/7] Basic implementation of view include working --- .../stapler/jelly/groovy/JellyBuilder.java | 1 + .../org/kohsuke/stapler/html/HtmlInclude.java | 20 ++++++ .../html/impl/HtmlClassLoaderTearOff.java | 2 +- .../stapler/html/impl/HtmlJellyScript.java | 68 ++++++++++++++++--- .../kohsuke/stapler/html/HtmlTestCase.java | 2 + .../stapler/html/IncludedFromTest.java | 5 +- .../kohsuke/stapler/html/IncludesTest.java | 31 +++++++++ .../kohsuke/stapler/html/StructureTest.java | 12 ++-- .../html/IncludesTest/Nested/center.xhtml | 4 ++ .../stapler/html/IncludesTest/top.xhtml | 6 ++ .../html/StructureTest/standalone.xhtml | 4 ++ 11 files changed, 137 insertions(+), 18 deletions(-) create mode 100644 html/src/main/java/org/kohsuke/stapler/html/HtmlInclude.java create mode 100644 html/src/test/java/org/kohsuke/stapler/html/IncludesTest.java create mode 100644 html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/Nested/center.xhtml create mode 100644 html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/top.xhtml diff --git a/groovy/src/main/java/org/kohsuke/stapler/jelly/groovy/JellyBuilder.java b/groovy/src/main/java/org/kohsuke/stapler/jelly/groovy/JellyBuilder.java index 1745fb964..9b99cb8e5 100644 --- a/groovy/src/main/java/org/kohsuke/stapler/jelly/groovy/JellyBuilder.java +++ b/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); diff --git a/html/src/main/java/org/kohsuke/stapler/html/HtmlInclude.java b/html/src/main/java/org/kohsuke/stapler/html/HtmlInclude.java new file mode 100644 index 000000000..cfd0b5a54 --- /dev/null +++ b/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(); +} diff --git a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java index 41bcdce4d..20f3b6514 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlClassLoaderTearOff.java +++ b/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) { diff --git a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java b/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java index 713edc1ec..b1dba3211 100644 --- a/html/src/main/java/org/kohsuke/stapler/html/impl/HtmlJellyScript.java +++ b/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

, 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("&", "&").replace("<", "<").replace(">", ">")); } 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(); 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 replacements) { + var parent = elt.getParent(); + var kids = new ArrayList(); + 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); + } } diff --git a/html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java b/html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java index bbcb31cf2..d89761323 100644 --- a/html/src/test/java/org/kohsuke/stapler/html/HtmlTestCase.java +++ b/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()) diff --git a/html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java b/html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java index 56ff8d78e..de8d5d79e 100644 --- a/html/src/test/java/org/kohsuke/stapler/html/IncludedFromTest.java +++ b/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.
Center text includes a special value.
An epilogue.")); + is( + "A prologue.
Center text includes a special <cool> value.
An epilogue.")); } @HtmlView("center") public Center getCenter() { - return new Center("special value"); + return new Center("special value"); } public record Center(String value) {} diff --git a/html/src/test/java/org/kohsuke/stapler/html/IncludesTest.java b/html/src/test/java/org/kohsuke/stapler/html/IncludesTest.java new file mode 100644 index 000000000..55eae3250 --- /dev/null +++ b/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( + "
A prologue. There are 23 items. An epilogue.
")); + } + + @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) {} + } +} diff --git a/html/src/test/java/org/kohsuke/stapler/html/StructureTest.java b/html/src/test/java/org/kohsuke/stapler/html/StructureTest.java index d0c4801f4..646653836 100644 --- a/html/src/test/java/org/kohsuke/stapler/html/StructureTest.java +++ b/html/src/test/java/org/kohsuke/stapler/html/StructureTest.java @@ -23,8 +23,12 @@ public void testStandalone() throws Exception { load("standalone"), allOf( containsString("Something #123"), - containsString( - "

All items:

k1 v1
k2 v2 &c.
"), + containsString("

All items: " + + " " + + " " + + " " + + " " + + "
Initial nameInitial value
k1 v1
k2 v2 &c.
Penultimate namePenultimate value
Final nameFinal value
"), 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("Something #456"), not(containsString("All items:")), - containsString("

Error in rotor.

"), - containsString("

There are no items.

"))); + containsString("

Error in rotor."), + containsString("

There are no items."))); } @HtmlView("standalone") diff --git a/html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/Nested/center.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/Nested/center.xhtml new file mode 100644 index 000000000..d5197bf44 --- /dev/null +++ b/html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/Nested/center.xhtml @@ -0,0 +1,4 @@ + + + There are 1234 items. + diff --git a/html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/top.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/top.xhtml new file mode 100644 index 000000000..0191811e2 --- /dev/null +++ b/html/src/test/resources/org/kohsuke/stapler/html/IncludesTest/top.xhtml @@ -0,0 +1,6 @@ + +

+ A prologue. + sample interior text + An epilogue. +
diff --git a/html/src/test/resources/org/kohsuke/stapler/html/StructureTest/standalone.xhtml b/html/src/test/resources/org/kohsuke/stapler/html/StructureTest/standalone.xhtml index 9fa893d61..d22d0e994 100644 --- a/html/src/test/resources/org/kohsuke/stapler/html/StructureTest/standalone.xhtml +++ b/html/src/test/resources/org/kohsuke/stapler/html/StructureTest/standalone.xhtml @@ -12,11 +12,15 @@

All items:

+ + + +
Initial nameInitial value
name value
Penultimate namePenultimate value
Final nameFinal value