From 6112276867359728802f87a847a3ceb1fa404ae1 Mon Sep 17 00:00:00 2001 From: fabrizzio-dotCMS Date: Fri, 7 Aug 2026 14:48:15 -0600 Subject: [PATCH] feat(velocity): resolve record components from VTL (#34154) A record's canonical accessor is named after the component itself (foo(), not getFoo()), so none of the strategies in UberspectImpl.getPropertyGet could reach it: getFoo() -> getfoo() -> Map.get -> get("foo") -> isFoo(). An unresolved Velocity reference is not an error, it renders as literal template text, so a template reading a record printed "$rec.id" into the page, silently. That is why SearchHit's components are named getId/getIndex/getSourceAsMap instead of id/index/sourceAsMap: the record was deformed to satisfy the template engine. This removes the need for that workaround on new records. RecordComponentExecutor is deliberately narrow: - It resolves only when the target is a record AND the identifier names one of its declared components, never an arbitrary no-argument method. Widening resolution to any foo() would silently change the meaning of existing templates across the product. - It is tried last in the chain, after every strategy that could already resolve the reference. So it can only add a resolution where there was none: no reference that resolves today changes meaning, without exception. Records whose components are bean-named (SearchHit) keep resolving via PropertyExecutor. - The accessor is looked up through Introspector.getMethod rather than RecordComponent.getAccessor(), so the method cache and the checks of the configured introspector (SecureIntrospectorImpl) both still apply. SecureUberspector inherits getPropertyGet unchanged, so the fix covers the uberspect dotCMS actually configures. Resolution cost is amortized: ASTIdentifier caches the VelPropertyGet per AST-node/class in the introspection cache, so getPropertyGet runs once per pair. Also pinned by test, and unrelated to this fix: a non-public record is invisible to VTL. ClassMap checks Modifier.isPublic on the class before collecting its methods, so a package-private or method-local record resolves to nothing, with the same silent literal-text outcome. A record read from a template must be public, or nested in a public type. Testing: 111 green. - RecordComponentExecutorTest (14 unit) - split between what the change adds and what it must not touch, including the guardrail that a non-record exposing a no-arg id() still does not resolve. - RecordComponentRenderingTest (7 integration) - the same claims end-to-end through the real engine via VelocityUtil.eval, asserting rendered output rather than introspection results. Registered in MainSuite1b. References are written non-quiet on purpose; quiet notation would let a broken accessor pass. - Regression over 10 existing VTL families (90 tests): ContentToolTest, NavToolTest, StoryBlockMapTest, ContentSearchToolTest, ContentMapTest, VelocityUtilTest, ASTMethodTest, DotParseTest, VelocityMacroCacheTest, SimpleNodeTest. Co-Authored-By: Claude Opus 5 (1M context) --- .../parser/node/RecordComponentExecutor.java | 180 +++++++++++ .../util/introspection/UberspectImpl.java | 21 +- .../node/RecordComponentExecutorTest.java | 290 ++++++++++++++++++ .../src/test/java/com/dotcms/MainSuite1b.java | 1 + .../RecordComponentRenderingTest.java | 176 +++++++++++ 5 files changed, 666 insertions(+), 2 deletions(-) create mode 100644 dotCMS/src/main/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutor.java create mode 100644 dotCMS/src/test/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutorTest.java create mode 100644 dotcms-integration/src/test/java/com/dotcms/rendering/velocity/RecordComponentRenderingTest.java diff --git a/dotCMS/src/main/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutor.java b/dotCMS/src/main/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutor.java new file mode 100644 index 000000000000..6b0e93d63588 --- /dev/null +++ b/dotCMS/src/main/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutor.java @@ -0,0 +1,180 @@ +package org.apache.velocity.runtime.parser.node; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import com.dotmarketing.util.Logger; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.RecordComponent; +import org.apache.commons.lang.StringUtils; +import org.apache.velocity.exception.VelocityException; +import org.apache.velocity.util.introspection.Introspector; + +/** + * Resolves {@code $reference.component} against the accessor of a Java {@code record} component. + * + *

{@link PropertyExecutor} only looks for JavaBean-shaped getters ({@code getFoo()} / + * {@code getfoo()}), and {@link BooleanPropertyExecutor} only for {@code isFoo()}. A record's + * canonical accessor is named after the component itself ({@code foo()}), so none of the existing + * executors can reach it and the reference silently renders as literal template text. This executor + * closes that gap.

+ * + *

It is deliberately narrow. Resolution is attempted only when the target class + * is a record and the identifier names one of its declared components. It never resolves an + * arbitrary no-argument method, so classes that are not records behave exactly as before.

+ * + *

It is also tried last in + * {@link org.apache.velocity.util.introspection.UberspectImpl#getPropertyGet}, after the bean getter, + * the {@code Map} key lookup, {@code get("foo")} and {@code isFoo()}. Every strategy that could + * already resolve the reference is given its chance first, so this executor can only add a resolution + * where there was none — no reference that resolves today changes meaning, without exception. Records + * whose components are bean-named (as {@code SearchHit} still is) keep resolving through + * {@link PropertyExecutor}.

+ * + *

The accessor is looked up through the {@link Introspector} rather than through + * {@link RecordComponent#getAccessor()} so that the method cache and the security checks of the + * configured introspector (see {@code SecureIntrospectorImpl}) both still apply.

+ * + * @see PropertyExecutor + * @see org.apache.velocity.util.introspection.UberspectImpl#getPropertyGet + */ +public class RecordComponentExecutor extends AbstractExecutor +{ + private final Introspector introspector; + + /** + * @param introspector the introspector used to resolve (and cache) the accessor + * @param clazz the class of the object the reference is being resolved against + * @param property the identifier written in the template + */ + public RecordComponentExecutor(final Introspector introspector, + final Class clazz, final String property) + { + this.introspector = introspector; + + // Mirrors PropertyExecutor: an empty identifier would only confuse the introspector. + if (clazz != null && clazz.isRecord() && StringUtils.isNotEmpty(property)) + { + discover(clazz, property); + } + } + + /** + * @return The current introspector. + */ + protected Introspector getIntrospector() + { + return this.introspector; + } + + /** + * Resolves the accessor, but only if {@code property} names a declared component of the record. + * + * @param clazz the record class + * @param property the identifier written in the template + */ + protected void discover(final Class clazz, final String property) + { + try + { + final String component = componentNamed(clazz, property); + + if (component != null) + { + final Object[] params = {}; + setMethod(introspector.getMethod(clazz, component, params)); + } + } + /* + * pass through application level runtime exceptions + */ + catch (RuntimeException e) + { + throw e; + } + catch (Exception e) + { + final String msg = "Exception while looking for record component accessor for '" + + property + "'"; + Logger.error(this, msg, e); + throw new VelocityException(msg, e); + } + } + + /** + * Returns the declared component name matching {@code property}, or {@code null} when the record + * has no such component. + * + *

An exact match is preferred. Failing that, the first character is case-flipped, which is the + * same convenience {@link PropertyExecutor} offers for bean getters so that {@code $rec.foo} and + * {@code $rec.Foo} behave alike.

+ */ + private String componentNamed(final Class clazz, final String property) + { + final RecordComponent[] components = clazz.getRecordComponents(); + + if (components == null) + { + return null; + } + + for (final RecordComponent candidate : components) + { + if (candidate.getName().equals(property)) + { + return candidate.getName(); + } + } + + final String flipped = flipFirstCharacter(property); + + for (final RecordComponent candidate : components) + { + if (candidate.getName().equals(flipped)) + { + return candidate.getName(); + } + } + + return null; + } + + /** + * Flips the case of the first character, e.g. {@code Title} to {@code title}. + */ + private String flipFirstCharacter(final String property) + { + final char first = property.charAt(0); + final char flipped = Character.isLowerCase(first) + ? Character.toUpperCase(first) + : Character.toLowerCase(first); + + return flipped + property.substring(1); + } + + /** + * @see AbstractExecutor#execute(java.lang.Object) + */ + @Override + public Object execute(Object o) + throws IllegalAccessException, InvocationTargetException + { + return isAlive() ? getMethod().invoke(o, ((Object[]) null)) : null; + } +} diff --git a/dotCMS/src/main/java/org/apache/velocity/util/introspection/UberspectImpl.java b/dotCMS/src/main/java/org/apache/velocity/util/introspection/UberspectImpl.java index 8ffe34c7c6e1..a1c23cafb512 100644 --- a/dotCMS/src/main/java/org/apache/velocity/util/introspection/UberspectImpl.java +++ b/dotCMS/src/main/java/org/apache/velocity/util/introspection/UberspectImpl.java @@ -34,6 +34,7 @@ import org.apache.velocity.runtime.parser.node.MapSetExecutor; import org.apache.velocity.runtime.parser.node.PropertyExecutor; import org.apache.velocity.runtime.parser.node.PutExecutor; +import org.apache.velocity.runtime.parser.node.RecordComponentExecutor; import org.apache.velocity.runtime.parser.node.SetExecutor; import org.apache.velocity.runtime.parser.node.SetPropertyExecutor; import org.apache.velocity.util.ArrayIterator; @@ -232,7 +233,7 @@ public VelPropertyGet getPropertyGet(Object obj, String identifier, Info i) /* * Let's see if we are a map... */ - if (!executor.isAlive()) + if (!executor.isAlive()) { executor = new MapGetExecutor(claz, identifier); } @@ -247,7 +248,7 @@ public VelPropertyGet getPropertyGet(Object obj, String identifier, Info i) } /* - * finally, look for boolean isFoo() + * then look for boolean isFoo() */ if (!executor.isAlive()) @@ -256,6 +257,22 @@ public VelPropertyGet getPropertyGet(Object obj, String identifier, Info i) identifier); } + /* + * finally, if the target is a record, look for the component accessor foo(). A record's + * canonical accessor carries the component's own name, so none of the bean-shaped lookups + * above can reach it and the reference would render as literal text. + * + * Deliberately last in the chain: every strategy that could already resolve the reference + * has been given its chance first, so this can only add a resolution where there was none. + * It is also restricted to actual record components (see RecordComponentExecutor) rather + * than to any no-argument method, which would silently change existing templates. + */ + + if (!executor.isAlive()) + { + executor = new RecordComponentExecutor(introspector, claz, identifier); + } + return (executor.isAlive()) ? new VelGetterImpl(executor) : null; } diff --git a/dotCMS/src/test/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutorTest.java b/dotCMS/src/test/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutorTest.java new file mode 100644 index 000000000000..175cfc41332b --- /dev/null +++ b/dotCMS/src/test/java/org/apache/velocity/runtime/parser/node/RecordComponentExecutorTest.java @@ -0,0 +1,290 @@ +package org.apache.velocity.runtime.parser.node; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Map; +import org.apache.commons.collections.ExtendedProperties; +import org.apache.velocity.runtime.RuntimeServices; +import org.apache.velocity.util.introspection.ClassMap; +import org.apache.velocity.util.introspection.Introspector; +import org.apache.velocity.util.introspection.SecureUberspector; +import org.apache.velocity.util.introspection.UberspectImpl; +import org.apache.velocity.util.introspection.VelPropertyGet; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit tests for {@link RecordComponentExecutor} and its wiring into + * {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)}. + * + *

The suite is deliberately split in two halves:

+ * + * + * + * @author Fabrizio Araya + */ +public class RecordComponentExecutorTest { + + /** A record with idiomatic (canonical) accessors: {@code id()}, {@code title()}. */ + public record CanonicalRecord(String id, String title, int hits, boolean draft) {} + + /** A record whose components are bean-named, the shape {@code SearchHit} uses today. */ + public record BeanNamedRecord(String getId, String getTitle) {} + + /** Not a record, but exposes a no-arg method named like a record component would be. */ + public static final class NotARecord { + + public String id() { + return "should-not-resolve"; + } + + public String getTitle() { + return "bean-getter"; + } + + public boolean isPublished() { + return true; + } + } + + /** Exercises the {@code get("key")} branch of the chain. */ + public static final class HasGenericGet { + + public String get(final String key) { + return "generic:" + key; + } + } + + /** A public record carrying a collection component, walked the way a template would. */ + public record PageRecord(String title, List tags) {} + + /** Package-private on purpose: Velocity cannot introspect a non-public class. */ + record PackagePrivateRecord(String id) {} + + private UberspectImpl uberspect; + + @Before + public void setUp() { + uberspect = new UberspectImpl(); + uberspect.init(); + } + + private Object resolve(final Object target, final String identifier) throws Exception { + final VelPropertyGet getter = uberspect.getPropertyGet(target, identifier, null); + return getter == null ? null : getter.invoke(target); + } + + // --------------------------------------------------------------------- + // What the change adds + // --------------------------------------------------------------------- + + /** + * Method to test: {@link RecordComponentExecutor#discover(Class, String)} + * Given scenario: a record with canonical accessors is referenced as {@code $rec.component}. + * Expected result: every component resolves to its accessor's value. Before this change the + * getter was {@code null} and Velocity rendered the reference as literal text. + */ + @Test + public void test_canonicalRecordComponents_resolve() throws Exception { + final CanonicalRecord record = new CanonicalRecord("abc-123", "Hello", 42, true); + + assertEquals("abc-123", resolve(record, "id")); + assertEquals("Hello", resolve(record, "title")); + assertEquals(42, resolve(record, "hits")); + assertEquals(true, resolve(record, "draft")); + } + + /** + * Method to test: {@link RecordComponentExecutor#discover(Class, String)} + * Given scenario: the identifier does not name any component of the record. + * Expected result: nothing resolves, so Velocity keeps its existing behaviour for unknown + * references instead of failing. + */ + @Test + public void test_unknownComponent_doesNotResolve() throws Exception { + final CanonicalRecord record = new CanonicalRecord("abc-123", "Hello", 42, true); + + assertNull(uberspect.getPropertyGet(record, "nope", null)); + } + + /** + * Method to test: {@link RecordComponentExecutor#discover(Class, String)} + * Given scenario: the reference capitalises the first character ({@code $rec.Title}). + * Expected result: it resolves, matching the case-flip convenience {@link PropertyExecutor} + * already offers for bean getters. + */ + @Test + public void test_firstCharacterCaseFlip_resolves() throws Exception { + final CanonicalRecord record = new CanonicalRecord("abc-123", "Hello", 42, true); + + assertEquals("Hello", resolve(record, "Title")); + } + + /** + * Method to test: {@link RecordComponentExecutor#discover(Class, String)} + * Given scenario: a record component whose accessor legitimately returns {@code null}. + * Expected result: the getter resolves (it is alive) and yields {@code null}, which is different + * from the reference not resolving at all. + */ + @Test + public void test_componentReturningNull_stillResolves() throws Exception { + final CanonicalRecord record = new CanonicalRecord(null, "Hello", 0, false); + + assertNotNull("the getter itself must resolve", uberspect.getPropertyGet(record, "id", null)); + assertNull(resolve(record, "id")); + } + + /** + * Method to test: {@link RecordComponentExecutor#discover(Class, String)} + * Given scenario: resolution goes through {@link SecureUberspector}, which is the uberspect + * dotCMS actually configures (see {@code system.properties}). + * Expected result: it resolves there too, because {@code SecureUberspector} inherits + * {@code getPropertyGet} from {@link UberspectImpl}. + */ + @Test + public void test_secureUberspector_resolvesRecordComponents() throws Exception { + final RuntimeServices runtimeServices = mock(RuntimeServices.class); + when(runtimeServices.getConfiguration()).thenReturn(new ExtendedProperties()); + + final SecureUberspector secure = new SecureUberspector(); + secure.setRuntimeServices(runtimeServices); + secure.init(); + + final CanonicalRecord record = new CanonicalRecord("abc-123", "Hello", 42, true); + final VelPropertyGet getter = secure.getPropertyGet(record, "id", null); + + assertNotNull(getter); + assertEquals("abc-123", getter.invoke(record)); + } + + // --------------------------------------------------------------------- + // What the change must not touch + // --------------------------------------------------------------------- + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a plain class (not a record) exposing a no-argument method {@code id()}. + * Expected result: {@code $obj.id} still does NOT resolve. This is the guardrail of the whole + * change — resolving arbitrary no-argument methods would silently alter existing templates. + */ + @Test + public void test_nonRecordNoArgMethod_stillDoesNotResolve() throws Exception { + assertNull(uberspect.getPropertyGet(new NotARecord(), "id", null)); + } + + /** + * Method to test: {@link RecordComponentExecutor#RecordComponentExecutor(Introspector, Class, String)} + * Given scenario: the executor is handed a class that is not a record. + * Expected result: it never becomes alive, so the chain falls through to the next strategy. + */ + @Test + public void test_executorIsInertForNonRecords() { + final Introspector introspector = new Introspector(); + + assertFalse(new RecordComponentExecutor(introspector, NotARecord.class, "id").isAlive()); + assertTrue(new RecordComponentExecutor(introspector, CanonicalRecord.class, "id").isAlive()); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a bean getter on a plain class. + * Expected result: unchanged — still resolved by {@link PropertyExecutor}. + */ + @Test + public void test_beanGetter_unchanged() throws Exception { + assertEquals("bean-getter", resolve(new NotARecord(), "title")); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a boolean {@code isFoo()} accessor. + * Expected result: unchanged — still resolved by {@link BooleanPropertyExecutor}. + */ + @Test + public void test_booleanIsGetter_unchanged() throws Exception { + assertEquals(true, resolve(new NotARecord(), "published")); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a {@code Map}, which is how most dotCMS content reaches templates. + * Expected result: unchanged — still resolved by {@link MapGetExecutor} as a key lookup. + */ + @Test + public void test_mapKeyLookup_unchanged() throws Exception { + assertEquals("mapped", resolve(Map.of("title", "mapped"), "title")); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a class exposing {@code get(String)}. + * Expected result: unchanged — still resolved by {@link GetExecutor}. + */ + @Test + public void test_genericGet_unchanged() throws Exception { + assertEquals("generic:title", resolve(new HasGenericGet(), "title")); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a record whose components are bean-named, the shape {@code SearchHit} uses today. + * Expected result: {@code $rec.id} keeps resolving through {@link PropertyExecutor}, because the + * record executor is tried only after it. The already-shipped records are therefore untouched. + */ + @Test + public void test_beanNamedRecord_stillResolvesThroughPropertyExecutor() throws Exception { + final BeanNamedRecord record = new BeanNamedRecord("abc-123", "Hello"); + + assertEquals("abc-123", resolve(record, "id")); + assertEquals("Hello", resolve(record, "title")); + + // The component name itself also resolves now, which is additive: it used to render literally. + assertEquals("abc-123", resolve(record, "getId")); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: a record component holding a collection, walked the way a template would. + * Expected result: the component resolves and the collection is usable downstream. + */ + @Test + public void test_recordComponentHoldingCollection_resolves() throws Exception { + final Object resolved = resolve(new PageRecord("Home", List.of("a", "b")), "tags"); + + assertEquals(List.of("a", "b"), resolved); + } + + /** + * Method to test: {@link UberspectImpl#getPropertyGet(Object, String, org.apache.velocity.util.introspection.Info)} + * Given scenario: the record is not {@code public} (package-private, or declared local to a + * method). + * Expected result: it still does not resolve, and that is not a shortcoming of this change — + * {@link ClassMap} only reflects over publicly accessible classes (it checks + * {@code Modifier.isPublic} on the class before collecting its methods), so no Velocity + * resolution strategy has ever reached a non-public type. + * + *

Pinned as a test because it is the trap of using records from templates: a small record is + * naturally declared package-private or local next to its use, and doing so makes it invisible to + * VTL with no error — only literal text in the rendered page. A record that must be readable + * from a template has to be {@code public}, or nested inside a public type.

+ */ + @Test + public void test_nonPublicRecord_doesNotResolve() throws Exception { + assertNull(uberspect.getPropertyGet(new PackagePrivateRecord("abc-123"), "id", null)); + } +} diff --git a/dotcms-integration/src/test/java/com/dotcms/MainSuite1b.java b/dotcms-integration/src/test/java/com/dotcms/MainSuite1b.java index 0f5cc787de6a..ac1ddaf68f2e 100644 --- a/dotcms-integration/src/test/java/com/dotcms/MainSuite1b.java +++ b/dotcms-integration/src/test/java/com/dotcms/MainSuite1b.java @@ -47,6 +47,7 @@ com.dotcms.rendering.velocity.ASTMethodTest.class, com.dotcms.rendering.velocity.VelocityMacroCacheTest.class, com.dotcms.rendering.velocity.VelocityUtilTest.class, + com.dotcms.rendering.velocity.RecordComponentRenderingTest.class, com.dotcms.rendering.velocity.viewtools.navigation.NavToolTest.class, com.dotcms.rendering.velocity.viewtools.navigation.NavToolCacheTest.class, com.dotcms.rendering.velocity.viewtools.content.ContentMapTest.class, diff --git a/dotcms-integration/src/test/java/com/dotcms/rendering/velocity/RecordComponentRenderingTest.java b/dotcms-integration/src/test/java/com/dotcms/rendering/velocity/RecordComponentRenderingTest.java new file mode 100644 index 000000000000..eaca6adebf52 --- /dev/null +++ b/dotcms-integration/src/test/java/com/dotcms/rendering/velocity/RecordComponentRenderingTest.java @@ -0,0 +1,176 @@ +package com.dotcms.rendering.velocity; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import com.dotcms.rendering.velocity.util.VelocityUtil; +import com.dotcms.util.IntegrationTestInitService; +import java.util.List; +import java.util.Map; +import org.apache.velocity.VelocityContext; +import org.apache.velocity.context.Context; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * End-to-end coverage for reading Java {@code record} components from VTL through the real dotCMS + * Velocity engine. + * + *

A record's canonical accessor is named after the component ({@code id()}), while Velocity's + * property resolution only ever looked for {@code getId()} / {@code getid()} / {@code Map.get} / + * {@code get("id")} / {@code isId()}. A reference that resolves to nothing is not an error in + * Velocity — it renders as literal template text — so before + * {@link org.apache.velocity.runtime.parser.node.RecordComponentExecutor} a template reading a + * record printed {@code $rec.id} into the page, silently. That is why the assertions here check + * rendered output rather than the introspection result: the unit test + * {@code RecordComponentExecutorTest} covers the resolution chain, this one covers what a page + * actually shows.

+ * + *

References are written non-quiet ({@code $rec.id}, never {@code $!{rec.id}}) on + * purpose. Quiet notation renders an unresolved reference as the empty string, which would let a + * broken accessor pass an assertion that only checks for absence.

+ * + * @author Fabrizio Araya + */ +public class RecordComponentRenderingTest { + + /** Idiomatic record: components are read as {@code $rec.id} / {@code $rec.title}. */ + public record Article(String id, String title, int views, List tags) {} + + /** A record nested inside another, to walk {@code $rec.author.name}. */ + public record Author(String name) {} + + /** Composite record, for the nested-walk case. */ + public record Post(String title, Author author) {} + + /** The bean-named shape already shipped in the neutral search layer ({@code SearchHit}). */ + public record BeanNamedHit(String getId, String getIndex) {} + + /** Not a record, but exposes a no-argument {@code id()} method. */ + public static final class LooksLikeARecord { + + public String id() { + return "must-not-resolve"; + } + } + + @BeforeClass + public static void prepare() throws Exception { + IntegrationTestInitService.getInstance().init(); + } + + private Context context(final String name, final Object value) { + final Context ctx = new VelocityContext(); + ctx.put(name, value); + return ctx; + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} over a record reference. + * Given scenario: a template reads every component of a record with canonical accessors. + * Expected result: the values are rendered. Before the fix the output was the literal text + * {@code $article.id | $article.title | $article.views}. + */ + @Test + public void test_canonicalRecordComponents_render() throws Exception { + final Article article = new Article("abc-123", "Modern Java", 42, List.of("java", "records")); + + final String output = VelocityUtil.eval( + "$article.id | $article.title | $article.views", + context("article", article)); + + assertEquals("abc-123 | Modern Java | 42", output.trim()); + assertFalse("no reference may survive as literal text", output.contains("$article")); + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} over a collection component. + * Given scenario: a {@code #foreach} walks a {@code List} held by a record component. + * Expected result: the loop runs and emits every element. + */ + @Test + public void test_foreachOverRecordCollectionComponent_renders() throws Exception { + final Article article = new Article("abc-123", "Modern Java", 42, List.of("java", "records")); + + final String output = VelocityUtil.eval( + "#foreach($tag in $article.tags)[$tag]#end", + context("article", article)); + + assertEquals("[java][records]", output.trim()); + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} over nested records. + * Given scenario: a record component is itself a record, walked as {@code $post.author.name}. + * Expected result: the walk resolves at both levels. + */ + @Test + public void test_nestedRecordWalk_renders() throws Exception { + final Post post = new Post("Hello", new Author("Fabrizio")); + + final String output = VelocityUtil.eval("$post.author.name", context("post", post)); + + assertEquals("Fabrizio", output.trim()); + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} over a non-record. + * Given scenario: a plain class exposing a no-argument {@code id()} method. + * Expected result: the reference still does NOT resolve and renders as literal text. This is the + * guardrail of the change, asserted end-to-end: resolution was widened for records only, not for + * every no-argument method, so no existing template changes meaning. + */ + @Test + public void test_nonRecordNoArgMethod_stillRendersAsLiteralText() throws Exception { + final String output = VelocityUtil.eval("$obj.id", context("obj", new LooksLikeARecord())); + + assertEquals("$obj.id", output.trim()); + assertFalse(output.contains("must-not-resolve")); + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} over a bean-named record. + * Given scenario: a record whose components are named {@code getId} / {@code getIndex}, the + * workaround the neutral search layer adopted so its hits could be read from VTL. + * Expected result: {@code $hit.id} keeps rendering exactly as before, through the bean-getter + * path. Already-shipped records are untouched by this change. + */ + @Test + public void test_beanNamedRecord_rendersUnchanged() throws Exception { + final String output = VelocityUtil.eval( + "$hit.id | $hit.index", + context("hit", new BeanNamedHit("abc-123", "live_index"))); + + assertEquals("abc-123 | live_index", output.trim()); + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} over a {@code Map}. + * Given scenario: a map reference, which is how most dotCMS content reaches templates. + * Expected result: key lookup renders unchanged. + */ + @Test + public void test_mapKeyLookup_rendersUnchanged() throws Exception { + final String output = VelocityUtil.eval( + "$content.title", + context("content", Map.of("title", "From a map"))); + + assertEquals("From a map", output.trim()); + } + + /** + * Method to test: {@link VelocityUtil#eval(String, Context)} for an unknown component. + * Given scenario: the template reads a component the record does not declare. + * Expected result: Velocity's existing behaviour for unresolved references is preserved — literal + * text, not an exception. + */ + @Test + public void test_unknownComponent_rendersAsLiteralText() throws Exception { + final Article article = new Article("abc-123", "Modern Java", 42, List.of()); + + final String output = VelocityUtil.eval("$article.nope", context("article", article)); + + assertTrue(output.trim().contains("$article.nope")); + } +}