diff --git a/java/org/apache/catalina/storeconfig/LocalStrings.properties b/java/org/apache/catalina/storeconfig/LocalStrings.properties index 2facbe157c..b257859ea3 100644 --- a/java/org/apache/catalina/storeconfig/LocalStrings.properties +++ b/java/org/apache/catalina/storeconfig/LocalStrings.properties @@ -25,8 +25,11 @@ factory.storeTag=store tag [{0}] ( Object: [{1}] ) globalNamingResourcesSF.noFactory=Cannot find NamingResources store factory globalNamingResourcesSF.wrongElement=Wrong element [{0}] +registry.interfacesLoaded=Loaded [{0}] interface classes for registry registry.loadClassFailed=Failed to load class [{0}] registry.noDescriptor=Can't find descriptor for key [{0}] +registry.optionalClassLoaded=Loaded optional class [{0}] +registry.optionalClassNotFound=Optional class [{0}] not found, skipping standardContextSF.cannotWriteFile=Cannot write file at [{0}] standardContextSF.canonicalPathError=Failed to obtain the canonical path of the configuration file [{0}] diff --git a/java/org/apache/catalina/storeconfig/StandardEngineSF.java b/java/org/apache/catalina/storeconfig/StandardEngineSF.java index 8963cc76af..bec6f6a789 100644 --- a/java/org/apache/catalina/storeconfig/StandardEngineSF.java +++ b/java/org/apache/catalina/storeconfig/StandardEngineSF.java @@ -26,13 +26,23 @@ import org.apache.catalina.LifecycleListener; import org.apache.catalina.Realm; import org.apache.catalina.Valve; import org.apache.catalina.core.StandardEngine; -import org.apache.catalina.ha.ClusterValve; /** * Store server.xml Element Engine */ public class StandardEngineSF extends StoreFactoryBase { + private static final Class clusterValveClass; + static { + Class clazz = null; + try { + clazz = Class.forName("org.apache.catalina.ha.ClusterValve"); + } catch (ClassNotFoundException e) { + // Expected when clustering JARs are not present + } + clusterValveClass = clazz; + } + /** * Store the specified Engine properties. * @@ -72,7 +82,7 @@ public class StandardEngineSF extends StoreFactoryBase { if(valves != null && valves.length > 0 ) { List engineValves = new ArrayList<>() ; for (Valve valve : valves) { - if (!(valve instanceof ClusterValve)) { + if (clusterValveClass == null || !clusterValveClass.isInstance(valve)) { engineValves.add(valve); } } diff --git a/java/org/apache/catalina/storeconfig/StandardHostSF.java b/java/org/apache/catalina/storeconfig/StandardHostSF.java index 54d2aeed42..5c511a5256 100644 --- a/java/org/apache/catalina/storeconfig/StandardHostSF.java +++ b/java/org/apache/catalina/storeconfig/StandardHostSF.java @@ -26,13 +26,23 @@ import org.apache.catalina.LifecycleListener; import org.apache.catalina.Realm; import org.apache.catalina.Valve; import org.apache.catalina.core.StandardHost; -import org.apache.catalina.ha.ClusterValve; /** * Store server.xml Element Host */ public class StandardHostSF extends StoreFactoryBase { + private static final Class clusterValveClass; + static { + Class clazz = null; + try { + clazz = Class.forName("org.apache.catalina.ha.ClusterValve"); + } catch (ClassNotFoundException e) { + // Expected when clustering JARs are not present + } + clusterValveClass = clazz; + } + /** * Store the specified Host properties and children * (Listener,Alias,Realm,Valve,Cluster, Context) @@ -78,7 +88,7 @@ public class StandardHostSF extends StoreFactoryBase { if(valves != null && valves.length > 0 ) { List hostValves = new ArrayList<>() ; for (Valve valve : valves) { - if (!(valve instanceof ClusterValve)) { + if (clusterValveClass == null || !clusterValveClass.isInstance(valve)) { hostValves.add(valve); } } diff --git a/java/org/apache/catalina/storeconfig/StoreRegistry.java b/java/org/apache/catalina/storeconfig/StoreRegistry.java index 4865171d14..1c1b21112e 100644 --- a/java/org/apache/catalina/storeconfig/StoreRegistry.java +++ b/java/org/apache/catalina/storeconfig/StoreRegistry.java @@ -16,7 +16,9 @@ */ package org.apache.catalina.storeconfig; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.naming.directory.DirContext; @@ -28,17 +30,6 @@ import org.apache.catalina.Realm; import org.apache.catalina.Valve; import org.apache.catalina.WebResourceRoot; import org.apache.catalina.WebResourceSet; -import org.apache.catalina.ha.CatalinaCluster; -import org.apache.catalina.ha.ClusterDeployer; -import org.apache.catalina.ha.ClusterListener; -import org.apache.catalina.tribes.Channel; -import org.apache.catalina.tribes.ChannelInterceptor; -import org.apache.catalina.tribes.ChannelReceiver; -import org.apache.catalina.tribes.ChannelSender; -import org.apache.catalina.tribes.Member; -import org.apache.catalina.tribes.MembershipService; -import org.apache.catalina.tribes.MessageListener; -import org.apache.catalina.tribes.transport.DataSender; import org.apache.coyote.UpgradeProtocol; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; @@ -49,10 +40,16 @@ import org.apache.tomcat.util.res.StringManager; * Central StoreRegistry for all server.xml elements */ public class StoreRegistry { - private static Log log = LogFactory.getLog(StoreRegistry.class); - private static StringManager sm = StringManager.getManager(StoreRegistry.class); + /** + * Constructs a new StoreRegistry with default settings. + */ + public StoreRegistry() { + } - private Map descriptors = new HashMap<>(); + private static final Log log = LogFactory.getLog(StoreRegistry.class); + private static final StringManager sm = StringManager.getManager(StoreRegistry.class); + + private final Map descriptors = new HashMap<>(); private String encoding = "UTF-8"; @@ -61,49 +58,116 @@ public class StoreRegistry { private String version; // Access Information - private static Class interfaces[] = { CatalinaCluster.class, - ChannelSender.class, ChannelReceiver.class, Channel.class, - MembershipService.class, ClusterDeployer.class, Realm.class, - Manager.class, DirContext.class, LifecycleListener.class, - Valve.class, ClusterListener.class, MessageListener.class, - DataSender.class, ChannelInterceptor.class, Member.class, - WebResourceRoot.class, WebResourceSet.class, - CredentialHandler.class, UpgradeProtocol.class, - CookieProcessor.class }; + // Lazily initialized to gracefully handle optional features like clustering + private static volatile Class[] interfaces = null; + + /** + * Initialize the interfaces array with all available classes. + * Uses dynamic loading for optional classes (e.g., clustering) to avoid + * ClassNotFoundException when those JARs are not present. This approach + * is consistent with how Catalina.addClusterRuleSet() handles clustering. + */ + private static Class[] getInterfaces() { + if (interfaces == null) { + synchronized (StoreRegistry.class) { + if (interfaces == null) { + // Required interfaces - always present + List> list = new ArrayList<>(); + list.add(Realm.class); + list.add(Manager.class); + list.add(DirContext.class); + list.add(LifecycleListener.class); + list.add(Valve.class); + list.add(WebResourceRoot.class); + list.add(WebResourceSet.class); + list.add(CredentialHandler.class); + list.add(UpgradeProtocol.class); + list.add(CookieProcessor.class); + + // Optional clustering interfaces - load dynamically to support + // deployments where clustering JARs may not be present + tryAddClass(list, "org.apache.catalina.ha.CatalinaCluster"); + tryAddClass(list, "org.apache.catalina.tribes.ChannelSender"); + tryAddClass(list, "org.apache.catalina.tribes.ChannelReceiver"); + tryAddClass(list, "org.apache.catalina.tribes.Channel"); + tryAddClass(list, "org.apache.catalina.tribes.MembershipService"); + tryAddClass(list, "org.apache.catalina.ha.ClusterDeployer"); + tryAddClass(list, "org.apache.catalina.ha.ClusterListener"); + tryAddClass(list, "org.apache.catalina.tribes.MessageListener"); + tryAddClass(list, "org.apache.catalina.tribes.transport.DataSender"); + tryAddClass(list, "org.apache.catalina.tribes.ChannelInterceptor"); + tryAddClass(list, "org.apache.catalina.tribes.Member"); + + interfaces = list.toArray(new Class[0]); + + if (log.isDebugEnabled()) { + log.debug(sm.getString("registry.interfacesLoaded", Integer.valueOf(interfaces.length))); + } + } + } + } + return interfaces; + } + + /** + * Try to load a class by name and add it to the list if successful. + * Logs at TRACE level if the class is not available. + */ + private static void tryAddClass(List> list, String className) { + try { + Class clazz = Class.forName(className, false, StoreRegistry.class.getClassLoader()); + list.add(clazz); + if (log.isTraceEnabled()) { + log.trace(sm.getString("registry.optionalClassLoaded", className)); + } + } catch (ClassNotFoundException | NoClassDefFoundError e) { + if (log.isTraceEnabled()) { + log.trace(sm.getString("registry.optionalClassNotFound", className)); + } + } + } /** - * @return the name + * Returns the name of this registry. + * + * @return the registry name */ public String getName() { return name; } /** - * @param name The name to set. + * Sets the name of this registry. + * + * @param name the registry name */ public void setName(String name) { this.name = name; } /** - * @return the version + * Returns the version of this registry. + * + * @return the registry version */ public String getVersion() { return version; } /** - * @param version The version to set + * Sets the version of this registry. + * + * @param version the registry version */ public void setVersion(String version) { this.version = version; } /** - * Find a description for id. Handle interface search when no direct match - * found. + * Find a description for id. Handle interface search when no direct match found. * * @param id The class name + * * @return the description */ public StoreDescription findDescription(String id) { @@ -120,17 +184,17 @@ public class StoreRegistry { } if (aClass != null) { desc = descriptors.get(aClass.getName()); - for (int i = 0; desc == null && i < interfaces.length; i++) { - if (interfaces[i].isAssignableFrom(aClass)) { - desc = descriptors.get(interfaces[i].getName()); + Class[] availableInterfaces = getInterfaces(); + for (int i = 0; desc == null && i < availableInterfaces.length; i++) { + if (availableInterfaces[i].isAssignableFrom(aClass)) { + desc = descriptors.get(availableInterfaces[i].getName()); } } } } if (log.isDebugEnabled()) { if (desc != null) { - log.trace("find descriptor " + id + "#" + desc.getTag() + "#" - + desc.getStoreFactoryClass()); + log.trace("find descriptor " + id + "#" + desc.getTag() + "#" + desc.getStoreFactoryClass()); } else { log.debug(sm.getString("registry.noDescriptor", id)); } @@ -142,6 +206,7 @@ public class StoreRegistry { * Find Description by class. * * @param aClass The class + * * @return the description */ public StoreDescription findDescription(Class aClass) { @@ -152,6 +217,7 @@ public class StoreRegistry { * Find factory from class name. * * @param aClassName The class name + * * @return the factory */ public IStoreFactory findStoreFactory(String aClassName) { @@ -168,6 +234,7 @@ public class StoreRegistry { * Find factory from class. * * @param aClass The class + * * @return the factory */ public IStoreFactory findStoreFactory(Class aClass) { @@ -186,8 +253,7 @@ public class StoreRegistry { } descriptors.put(key, desc); if (log.isTraceEnabled()) { - log.trace("register store descriptor " + key + "#" + desc.getTag() - + "#" + desc.getTagClass()); + log.trace("register store descriptor " + key + "#" + desc.getTag() + "#" + desc.getTagClass()); } } @@ -195,11 +261,12 @@ public class StoreRegistry { * Unregister a description. * * @param desc The description + * * @return the description, or null if it was not registered */ public StoreDescription unregisterDescription(StoreDescription desc) { String key = desc.getId(); - if (key == null || "".equals(key)) { + if (key == null || key.isEmpty()) { key = desc.getTagClass(); } return descriptors.remove(key); @@ -208,7 +275,9 @@ public class StoreRegistry { // Attributes /** - * @return the encoding + * Returns the character encoding used when writing configuration files. + * + * @return the character encoding */ public String getEncoding() { return encoding; @@ -216,6 +285,7 @@ public class StoreRegistry { /** * Set the encoding to use when writing the configuration files. + * * @param string The encoding */ public void setEncoding(String string) { diff --git a/test/org/apache/catalina/storeconfig/TestStoreRegistry.java b/test/org/apache/catalina/storeconfig/TestStoreRegistry.java new file mode 100644 index 0000000000..e6869d3642 --- /dev/null +++ b/test/org/apache/catalina/storeconfig/TestStoreRegistry.java @@ -0,0 +1,104 @@ +/* + * 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.catalina.storeconfig; + +import java.lang.reflect.Method; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.Manager; +import org.apache.catalina.Realm; +import org.apache.catalina.Valve; + +/** + * Test StoreRegistry behavior, particularly dynamic loading of optional classes like clustering. + * + * Verifies StoreRegistry uses the same dynamic loading pattern. + */ +public class TestStoreRegistry { + + /** + * Test that clustering classes are dynamically loaded like other Tomcat components. + * + * StoreRegistry should initialize successfully whether clustering is available or not. + * This matches the pattern used in Catalina.addClusterRuleSet(). + */ + @Test + public void testClusteringClassesOptional() throws Exception { + // Verify StoreRegistry initializes successfully with dynamic class loading + StoreRegistry registry = new StoreRegistry(); + Assert.assertNotNull("Registry should initialize with dynamic loading", registry); + + // Trigger lazy loading of interfaces array + Method getInterfacesMethod = StoreRegistry.class.getDeclaredMethod("getInterfaces"); + getInterfacesMethod.setAccessible(true); + + Class[] interfaces = (Class[]) getInterfacesMethod.invoke(null); + Assert.assertNotNull("Interfaces should load dynamically", interfaces); + + // Test passes if we get here without ClassNotFoundException. + // The actual number of interfaces loaded depends on whether clustering is available, + // but we should always have at least the core 10 interfaces. + Assert.assertTrue("Should have at least 10 core interfaces", + interfaces.length >= 10); + + // Verify required core interfaces are always present + boolean hasRealm = false; + boolean hasManager = false; + boolean hasValve = false; + + for (Class iface : interfaces) { + if (iface.equals(Realm.class)) { + hasRealm = true; + } + if (iface.equals(Manager.class)) { + hasManager = true; + } + if (iface.equals(Valve.class)) { + hasValve = true; + } + } + + Assert.assertTrue("Should contain Realm interface", hasRealm); + Assert.assertTrue("Should contain Manager interface", hasManager); + Assert.assertTrue("Should contain Valve interface", hasValve); + } + + /** + * Test that findDescription works with interface inheritance and + * dynamically loaded interfaces. + */ + @Test + public void testFindDescriptionWithDynamicInterfaces() throws Exception { + StoreRegistry registry = new StoreRegistry(); + + // Register a description for the Valve interface + StoreDescription valveDesc = new StoreDescription(); + valveDesc.setId(Valve.class.getName()); + valveDesc.setTag("Valve"); + valveDesc.setTagClass(Valve.class.getName()); + registry.registerDescription(valveDesc); + + // AccessLogValve implements Valve interface - should find via dynamic interface matching + String accessLogValveClass = "org.apache.catalina.valves.AccessLogValve"; + StoreDescription foundDesc = registry.findDescription(accessLogValveClass); + + Assert.assertNotNull("Should find description via interface matching", foundDesc); + Assert.assertEquals("Should match Valve descriptor", "Valve", foundDesc.getTag()); + } +} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 012b607f18..17f639d669 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -138,6 +138,13 @@ The rewrite valve should not do a rewrite if the output is identical to the input. (remm) + + Update StoreRegistry to dynamically load optional clustering + classes rather than statically referencing them. This matches the pattern + used in Catalina.addClusterRuleSet() and prevents + NoClassDefFoundError when StoreConfigLifecycleListener + is configured but clustering classes are not available. (csutherl) + Add a new valveSkip (or VS) rule flag to the rewrite valve to allow skipping over the next valve in the Catalina