From 17289d289c3df481ce61c92c3709f71a444e5870 Mon Sep 17 00:00:00 2001 From: Arun Date: Sat, 31 Oct 2026 13:09:03 +0000 Subject: [PATCH 1/5] WW-5827 check the preparer type before instantiating it and do not cache failed preparer lookups ClassUtil.getClass() resolved a class with initialize=true, so the named class ran its static initialiser on lookup, and instantiate(String, boolean) passed Object.class as the base type, which left the asSubclass check unable to fail. BasicPreparerFactory therefore cast to ViewPreparer only after the named class had been loaded, initialised and constructed. The class is now resolved without being initialised, and a typed instantiate(String, Class, boolean) applies the caller's base type before the constructor runs, so a class that does not denote a ViewPreparer is rejected without anything being constructed. A name that resolves but does not conform is reported as CannotInstantiateObjectException rather than returned as null, which keeps the failure as visible as the ClassCastException it replaces; returnNull continues to mean "class not found" only. Resolution and construction are caught separately, so an exception thrown by the constructor itself is never reported as a type mismatch. BasicPreparerFactory.getPreparer() stored the result of createPreparer() unconditionally, so a null was retained and thereafter reported as resolved by containsKey(). That made a single failure permanent for the lifetime of the factory even when its cause was transient. Failed lookups are no longer recorded, and the map is a ConcurrentHashMap, since it is read and written by request threads. TilesDispatchServlet is the only other caller of the untyped helper and is migrated to the typed one. --- .../prepare/factory/BasicPreparerFactory.java | 25 ++-- .../tiles/request/reflect/ClassUtil.java | 62 +++++++++- .../tiles/web/util/TilesDispatchServlet.java | 2 +- .../BasicPreparerFactoryTypeCheckTest.java | 117 ++++++++++++++++++ .../tiles/request/reflect/ClassUtilTest.java | 49 ++++++++ 5 files changed, 246 insertions(+), 9 deletions(-) create mode 100644 plugins/tiles/src/test/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactoryTypeCheckTest.java diff --git a/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java b/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java index b28151da20..94458d95d4 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java @@ -24,8 +24,8 @@ import org.apache.tiles.request.Request; import org.apache.tiles.request.reflect.ClassUtil; -import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Default implementation of the {@link org.apache.tiles.core.prepare.factory.PreparerFactory}. @@ -50,7 +50,7 @@ public class BasicPreparerFactory implements PreparerFactory { * Constructor. */ public BasicPreparerFactory() { - this.preparers = new HashMap<>(); + this.preparers = new ConcurrentHashMap<>(); } @@ -63,11 +63,23 @@ public BasicPreparerFactory() { * @return ViewPreparer instance */ public ViewPreparer getPreparer(String name, Request context) { - if (!preparers.containsKey(name)) { - preparers.put(name, createPreparer(name)); + ViewPreparer preparer = preparers.get(name); + if (preparer != null) { + return preparer; } - return preparers.get(name); + preparer = createPreparer(name); + if (preparer == null) { + // A lookup that did not produce a preparer is not recorded. Caching it would make a + // single failure permanent for the lifetime of the factory, even when the cause was + // transient, and a cached null was previously reported as resolved by containsKey(). + return null; + } + + // Not computeIfAbsent: createPreparer may load classes and call back into the container, + // which must not run while a bin of this map is locked. + ViewPreparer existing = preparers.putIfAbsent(name, preparer); + return existing != null ? existing : preparer; } /** @@ -78,7 +90,6 @@ public ViewPreparer getPreparer(String name, Request context) { */ protected ViewPreparer createPreparer(String name) { LOG.debug("Creating ViewPreparer '{}'", name); - Object instance = ClassUtil.instantiate(name, true); - return (ViewPreparer) instance; + return ClassUtil.instantiate(name, ViewPreparer.class, true); } } diff --git a/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java b/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java index 8211b3ac62..d4f6bed2b3 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java @@ -24,6 +24,7 @@ import java.beans.BeanInfo; import java.beans.Introspector; import java.beans.PropertyDescriptor; +import java.lang.reflect.InvocationTargetException; import java.util.Map; /** @@ -49,18 +50,26 @@ private ClassUtil() { * @param baseClass The base class to subclass to. * @return The loaded class. * @throws ClassNotFoundException If the class has not been found. + * @throws ClassCastException If the class is not assignable to baseClass. */ public static Class getClass(String className, Class baseClass) throws ClassNotFoundException { ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); if (classLoader == null) { classLoader = ClassUtil.class.getClassLoader(); } - return Class.forName(className, true, classLoader).asSubclass(baseClass); + // The class is resolved without being initialised, so that the asSubclass check below runs + // before any of its code does. Initialisation still happens on first real use, e.g. when a + // constructor is invoked. + return Class.forName(className, false, classLoader).asSubclass(baseClass); } /** * Returns an instance of the given class name, by calling the default * constructor. + *

+ * The returned object is not constrained to any type, so a caller that requires one has to + * cast after the object has already been constructed. Prefer + * {@link #instantiate(String, Class, boolean)}, which applies the required type first. * * @param className The class name to load and to instantiate. * @return The new instance of the class name. @@ -71,9 +80,60 @@ public static Object instantiate(String className) { return instantiate(className, false); } + /** + * Returns an instance of the given class name, constrained to the given base class, by + * calling the default constructor. + *

+ * The base class is applied by {@link #getClass(String, Class)} before the constructor runs, + * so a class that does not conform to it is rejected without being initialised or constructed. + * + * @param The type the named class has to conform to. + * @param className The class name to load and to instantiate. + * @param baseClass The base class the named class has to be assignable to. + * @param returnNull If true, if the class is not found it returns + * null, otherwise it throws a + * CannotInstantiateObjectException. A class that is found but + * does not conform to baseClass is always reported as an + * exception, because it denotes a misconfigured name rather than a missing + * one. + * @return The new instance of the class name. + * @throws CannotInstantiateObjectException If something goes wrong during instantiation. + */ + public static T instantiate(String className, Class baseClass, boolean returnNull) { + Class namedClass; + try { + namedClass = getClass(className, baseClass); + } catch (ClassNotFoundException e) { + if (returnNull) { + return null; + } + throw new CannotInstantiateObjectException("Unable to resolve factory class: '" + className + "'", e); + } catch (ClassCastException e) { + throw new CannotInstantiateObjectException("Factory class: '" + className + "' is not a '" + + baseClass.getName() + "'", e); + } + // Resolution and construction are caught separately, so that an exception thrown by the + // constructor itself is never reported as a type mismatch. + try { + return namedClass.getDeclaredConstructor().newInstance(); + } catch (NoSuchMethodException | InstantiationException e) { + throw new CannotInstantiateObjectException("Unable to instantiate factory class: '" + className + + "'. Make sure that this class has a default constructor", e); + } catch (IllegalAccessException e) { + throw new CannotInstantiateObjectException("Unable to access factory class: '" + className + "'", e); + } catch (InvocationTargetException e) { + throw new CannotInstantiateObjectException("Constructor of factory class: '" + className + + "' has thrown an exception", e); + } + } + /** * Returns an instance of the given class name, by calling the default * constructor. + *

+ * The returned object is not constrained to any type, so a caller that requires one has to + * cast after the object has already been constructed. Prefer + * {@link #instantiate(String, Class, boolean)}, which applies the required type first. * * @param className The class name to load and to instantiate. * @param returnNull If true, if the class is not found it diff --git a/plugins/tiles/src/main/java/org/apache/tiles/web/util/TilesDispatchServlet.java b/plugins/tiles/src/main/java/org/apache/tiles/web/util/TilesDispatchServlet.java index decdd607bf..4235e2fd7a 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/web/util/TilesDispatchServlet.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/web/util/TilesDispatchServlet.java @@ -78,7 +78,7 @@ public void init() throws ServletException { String temp = getInitParameter("mutator"); if (temp != null) { try { - mutator = (org.apache.tiles.web.util.AttributeContextMutator) ClassUtil.instantiate(temp); + mutator = ClassUtil.instantiate(temp, AttributeContextMutator.class, false); } catch (Exception e) { throw new ServletException("Unable to instantiate specified context mutator.", e); } diff --git a/plugins/tiles/src/test/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactoryTypeCheckTest.java b/plugins/tiles/src/test/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactoryTypeCheckTest.java new file mode 100644 index 0000000000..23aa5df90d --- /dev/null +++ b/plugins/tiles/src/test/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactoryTypeCheckTest.java @@ -0,0 +1,117 @@ +/* + * 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. + */ +package org.apache.tiles.core.prepare.factory; + +import org.apache.tiles.api.AttributeContext; +import org.apache.tiles.api.preparer.ViewPreparer; +import org.apache.tiles.request.Request; +import org.apache.tiles.request.reflect.CannotInstantiateObjectException; +import org.junit.Test; + +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.assertSame; +import static org.junit.Assert.fail; + +/** + * The factory applies the ViewPreparer type before the named class is instantiated, so a class that + * does not conform is rejected without being initialised or constructed, and a lookup that did not + * produce a preparer is not retained. + */ +public class BasicPreparerFactoryTypeCheckTest { + + /** + * The evidence lives in its own class. Reading a static member of {@link NotAPreparer} would + * itself initialise that class and so invalidate the assertions below. + */ + public static final class Evidence { + public static volatile boolean classInitialised; + public static volatile boolean constructed; + } + + public static class NotAPreparer { + static { + Evidence.classInitialised = true; + } + + public NotAPreparer() { + Evidence.constructed = true; + } + } + + public static class LegitimatePreparer implements ViewPreparer { + @Override + public void execute(Request tilesContext, AttributeContext attributeContext) { + } + } + + @Test + public void nonPreparerIsRejectedBeforeInitialisationAndConstruction() { + CountingPreparerFactory factory = new CountingPreparerFactory(); + + // A class literal does not initialise the class it denotes, so naming the class this way + // does not disturb the assertions below. + String name = NotAPreparer.class.getName(); + try { + factory.getPreparer(name, null); + fail("a class that is not a ViewPreparer must be reported"); + } catch (CannotInstantiateObjectException expected) { + assertNotNull(expected.getMessage()); + } + + assertFalse("the static initialiser must not run for a class that is not a ViewPreparer", + Evidence.classInitialised); + assertFalse("the constructor must not run for a class that is not a ViewPreparer", + Evidence.constructed); + assertEquals("a rejected name must not be retained", 0, factory.size()); + } + + @Test + public void unresolvableNameIsNotRetained() { + CountingPreparerFactory factory = new CountingPreparerFactory(); + + for (int i = 0; i < 100; i++) { + assertNull(factory.getPreparer("org.doesnotexist.Preparer" + i, null)); + } + + assertEquals("a name that cannot be resolved must not be retained", 0, factory.size()); + } + + @Test + public void resolvedPreparerIsStillCachedAndReused() { + CountingPreparerFactory factory = new CountingPreparerFactory(); + String name = LegitimatePreparer.class.getName(); + + ViewPreparer first = factory.getPreparer(name, null); + assertNotNull("a genuine ViewPreparer must still be created", first); + assertSame("a resolved preparer must be cached and reused", first, factory.getPreparer(name, null)); + assertEquals(1, factory.size()); + } + + /** + * Exposes the size of the inherited cache. + */ + private static final class CountingPreparerFactory extends BasicPreparerFactory { + int size() { + return preparers.size(); + } + } +} diff --git a/plugins/tiles/src/test/java/org/apache/tiles/request/reflect/ClassUtilTest.java b/plugins/tiles/src/test/java/org/apache/tiles/request/reflect/ClassUtilTest.java index db01a2414f..3d74235aec 100644 --- a/plugins/tiles/src/test/java/org/apache/tiles/request/reflect/ClassUtilTest.java +++ b/plugins/tiles/src/test/java/org/apache/tiles/request/reflect/ClassUtilTest.java @@ -28,6 +28,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * Tests {@link ClassUtil}. @@ -92,6 +93,42 @@ public void testInstantiate() { assertNull(ClassUtil.instantiate("this.class.does.not.Exist", true)); } + /** + * Test method for {@link ClassUtil#instantiate(String, Class, boolean)}. + */ + @Test + public void testInstantiateWithBaseClass() { + TestBase instance = ClassUtil.instantiate(TestSubClass.class.getName(), TestBase.class, true); + assertNotNull(instance); + assertTrue(instance instanceof TestSubClass); + } + + /** + * Test method for {@link ClassUtil#instantiate(String, Class, boolean)}. + */ + @Test + public void testInstantiateWithBaseClassNotFound() { + assertNull(ClassUtil.instantiate("this.class.does.not.Exist", TestBase.class, true)); + } + + /** + * Test method for {@link ClassUtil#instantiate(String, Class, boolean)}. + */ + @Test(expected = CannotInstantiateObjectException.class) + public void testInstantiateWithBaseClassNotFoundException() { + ClassUtil.instantiate("this.class.does.not.Exist", TestBase.class, false); + } + + /** + * Test method for {@link ClassUtil#instantiate(String, Class, boolean)}. A class that is found + * but does not conform to the base class denotes a misconfigured name rather than a missing + * one, so it is reported even when missing classes are tolerated. + */ + @Test(expected = CannotInstantiateObjectException.class) + public void testInstantiateWithBaseClassMismatch() { + ClassUtil.instantiate(TestClass.class.getName(), TestBase.class, true); + } + /** * Test method for {@link ClassUtil#instantiate(String, boolean)}. */ @@ -167,6 +204,18 @@ public interface TestInterface { public static class TestClass { } + /** + * A base class to constrain a lookup to. + */ + public static class TestBase { + } + + /** + * A class that conforms to {@link TestBase}. + */ + public static class TestSubClass extends TestBase { + } + /** * A test static private class. */ From d93bd110c82cf88c52f1136a867994bf83f19c85 Mon Sep 17 00:00:00 2001 From: Arun Date: Sun, 1 Nov 2026 23:58:59 +0530 Subject: [PATCH 2/5] WW-5827 drop the inline comments and the untyped javadoc notes --- .../tiles/core/prepare/factory/BasicPreparerFactory.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java b/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java index 94458d95d4..4600282598 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java @@ -69,15 +69,10 @@ public ViewPreparer getPreparer(String name, Request context) { } preparer = createPreparer(name); - if (preparer == null) { - // A lookup that did not produce a preparer is not recorded. Caching it would make a - // single failure permanent for the lifetime of the factory, even when the cause was - // transient, and a cached null was previously reported as resolved by containsKey(). + if (preparer == null) { return null; } - - // Not computeIfAbsent: createPreparer may load classes and call back into the container, - // which must not run while a bin of this map is locked. + ViewPreparer existing = preparers.putIfAbsent(name, preparer); return existing != null ? existing : preparer; } From ead24f85ec9f106bc943c5b1ed6321c643825ed4 Mon Sep 17 00:00:00 2001 From: Arun Date: Mon, 2 Nov 2026 00:04:30 +0530 Subject: [PATCH 3/5] WW-5827 drop the inline comments and the untyped javadoc notes --- .../org/apache/tiles/request/reflect/ClassUtil.java | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java b/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java index d4f6bed2b3..5433daf6de 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/request/reflect/ClassUtil.java @@ -57,19 +57,12 @@ public static Class getClass(String className, Class baseCla if (classLoader == null) { classLoader = ClassUtil.class.getClassLoader(); } - // The class is resolved without being initialised, so that the asSubclass check below runs - // before any of its code does. Initialisation still happens on first real use, e.g. when a - // constructor is invoked. return Class.forName(className, false, classLoader).asSubclass(baseClass); } /** * Returns an instance of the given class name, by calling the default * constructor. - *

- * The returned object is not constrained to any type, so a caller that requires one has to - * cast after the object has already been constructed. Prefer - * {@link #instantiate(String, Class, boolean)}, which applies the required type first. * * @param className The class name to load and to instantiate. * @return The new instance of the class name. @@ -112,8 +105,6 @@ public static T instantiate(String className, Class baseClass, boolean re throw new CannotInstantiateObjectException("Factory class: '" + className + "' is not a '" + baseClass.getName() + "'", e); } - // Resolution and construction are caught separately, so that an exception thrown by the - // constructor itself is never reported as a type mismatch. try { return namedClass.getDeclaredConstructor().newInstance(); } catch (NoSuchMethodException | InstantiationException e) { @@ -130,10 +121,6 @@ public static T instantiate(String className, Class baseClass, boolean re /** * Returns an instance of the given class name, by calling the default * constructor. - *

- * The returned object is not constrained to any type, so a caller that requires one has to - * cast after the object has already been constructed. Prefer - * {@link #instantiate(String, Class, boolean)}, which applies the required type first. * * @param className The class name to load and to instantiate. * @param returnNull If true, if the class is not found it From 75c21cba50a3c9630c6cc4c82b3341889ed6ff2d Mon Sep 17 00:00:00 2001 From: Arun Date: Wed, 4 Nov 2026 12:10:35 +0000 Subject: [PATCH 4/5] WW-5827 drop the trailing whitespace left by the previous push --- .../tiles/core/prepare/factory/BasicPreparerFactory.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java b/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java index 4600282598..b97288eb0f 100644 --- a/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java +++ b/plugins/tiles/src/main/java/org/apache/tiles/core/prepare/factory/BasicPreparerFactory.java @@ -69,10 +69,10 @@ public ViewPreparer getPreparer(String name, Request context) { } preparer = createPreparer(name); - if (preparer == null) { + if (preparer == null) { return null; } - + ViewPreparer existing = preparers.putIfAbsent(name, preparer); return existing != null ? existing : preparer; } From 39d93e9e07303ac351f261c1b72e306c4a7c2107 Mon Sep 17 00:00:00 2001 From: Arun Date: Wed, 4 Nov 2026 12:10:35 +0000 Subject: [PATCH 5/5] WW-5827 check the preparer type in StrutsPreparerFactory before building the bean --- .../struts2/tiles/StrutsPreparerFactory.java | 28 +++- .../StrutsPreparerFactoryTypeCheckTest.java | 143 ++++++++++++++++++ 2 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsPreparerFactoryTypeCheckTest.java diff --git a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsPreparerFactory.java b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsPreparerFactory.java index db37af2788..dcefcb312b 100644 --- a/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsPreparerFactory.java +++ b/plugins/tiles/src/main/java/org/apache/struts2/tiles/StrutsPreparerFactory.java @@ -22,9 +22,10 @@ import org.apache.struts2.ObjectFactory; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.tiles.api.preparer.ViewPreparer; import org.apache.tiles.core.prepare.factory.BasicPreparerFactory; +import org.apache.tiles.request.reflect.CannotInstantiateObjectException; +import org.apache.tiles.request.reflect.ClassUtil; /** * This is a basic ViewPreparer factory that uses {@link ObjectFactory} to create the ViewPreparer @@ -41,12 +42,33 @@ protected ViewPreparer createPreparer(String name) { return super.createPreparer(name); } + checkType(name); + + Object preparer; try { ObjectFactory factory = actionContext.getContainer().getInstance(ObjectFactory.class); - return (ViewPreparer) factory.buildBean(name, ActionContext.getContext().getContextMap()); + preparer = factory.buildBean(name, actionContext.getContextMap()); } catch (Exception e) { - LOG.error(new ParameterizedMessage("Cannot create a ViewPreparer [{}], fallback to super.createPreparer!", name), e); + LOG.warn("Cannot create a ViewPreparer [{}], fallback to super.createPreparer: {}", name, e.toString()); + LOG.debug("Cannot create a ViewPreparer [{}]", name, e); return super.createPreparer(name); } + + if (preparer instanceof ViewPreparer viewPreparer) { + return viewPreparer; + } + throw new CannotInstantiateObjectException("Preparer: '" + name + "' is not a '" + + ViewPreparer.class.getName() + "'", null); + } + + private void checkType(String name) { + try { + ClassUtil.getClass(name, ViewPreparer.class); + } catch (ClassNotFoundException e) { + LOG.debug("Preparer [{}] does not name a class, leaving it to the object factory", name); + } catch (ClassCastException e) { + throw new CannotInstantiateObjectException("Preparer: '" + name + "' is not a '" + + ViewPreparer.class.getName() + "'", e); + } } } diff --git a/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsPreparerFactoryTypeCheckTest.java b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsPreparerFactoryTypeCheckTest.java new file mode 100644 index 0000000000..71454b4c4c --- /dev/null +++ b/plugins/tiles/src/test/java/org/apache/struts2/tiles/StrutsPreparerFactoryTypeCheckTest.java @@ -0,0 +1,143 @@ +/* + * 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. + */ +package org.apache.struts2.tiles; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.ObjectFactory; +import org.apache.struts2.inject.Container; +import org.apache.tiles.api.AttributeContext; +import org.apache.tiles.api.preparer.ViewPreparer; +import org.apache.tiles.request.Request; +import org.apache.tiles.request.reflect.CannotInstantiateObjectException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +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.assertSame; +import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * The factory applies the ViewPreparer type before the bean is built, so a configured name that does + * not denote a preparer is rejected without anything being constructed, and a name that cannot be + * built is not constructed a second time through the superclass. + */ +public class StrutsPreparerFactoryTypeCheckTest { + + /** + * The evidence lives in its own class so that reading it does not initialise {@link NotAPreparer}. + */ + public static final class Evidence { + public static volatile boolean constructed; + public static volatile int preparerConstructions; + } + + public static class NotAPreparer { + public NotAPreparer() { + Evidence.constructed = true; + } + } + + public static class LegitimatePreparer implements ViewPreparer { + public LegitimatePreparer() { + Evidence.preparerConstructions++; + } + + @Override + public void execute(Request tilesContext, AttributeContext attributeContext) { + } + } + + private ObjectFactory objectFactory; + private StrutsPreparerFactory factory; + + @Before + public void setUp() { + Evidence.constructed = false; + Evidence.preparerConstructions = 0; + + objectFactory = mock(ObjectFactory.class); + Container container = mock(Container.class); + when(container.getInstance(ObjectFactory.class)).thenReturn(objectFactory); + + ActionContext.of().withContainer(container).bind(); + + factory = new StrutsPreparerFactory(); + } + + @After + public void tearDown() { + ActionContext.clear(); + Evidence.constructed = false; + } + + @Test + public void nonPreparerIsRejectedBeforeTheBeanIsBuilt() throws Exception { + // A class literal does not initialise the class it denotes. + String name = NotAPreparer.class.getName(); + + CannotInstantiateObjectException e = assertThrows(CannotInstantiateObjectException.class, + () -> factory.getPreparer(name, null)); + assertNotNull(e.getMessage()); + + verify(objectFactory, never()).buildBean(anyString(), anyMap()); + assertFalse("a class that is not a ViewPreparer must not be constructed", Evidence.constructed); + } + + @Test + public void beanThatIsNotAPreparerIsReportedAndNotBuiltTwice() throws Exception { + String name = "someContainerManagedName"; + when(objectFactory.buildBean(anyString(), anyMap())).thenReturn("not a preparer"); + + assertThrows(CannotInstantiateObjectException.class, () -> factory.getPreparer(name, null)); + + verify(objectFactory).buildBean(anyString(), anyMap()); + assertFalse(Evidence.constructed); + } + + @Test + public void buildFailureStillFallsBackToTheSuperclassAndConstructsOnce() throws Exception { + String name = LegitimatePreparer.class.getName(); + when(objectFactory.buildBean(anyString(), anyMap())).thenThrow(new IllegalStateException("boom")); + + assertNotNull("a build failure must still fall back to the superclass", factory.getPreparer(name, null)); + assertEquals("the preparer must be constructed exactly once", 1, Evidence.preparerConstructions); + } + + @Test + public void genuinePreparerIsBuiltThroughTheObjectFactory() throws Exception { + String name = LegitimatePreparer.class.getName(); + LegitimatePreparer expected = new LegitimatePreparer(); + when(objectFactory.buildBean(anyString(), anyMap())).thenReturn(expected); + + ViewPreparer preparer = factory.getPreparer(name, null); + + assertSame(expected, preparer); + assertSame("a resolved preparer must still be cached and reused", preparer, factory.getPreparer(name, null)); + } +}