Merge pull request #825 from apache/WW-5381-method-accessor

WW-5381 Introduce extension point for MethodAccessor
apache · Jan 6, 2024 · 3b76678 · 3b76678
2 parents 3b07899 + 503c8da
commit 3b76678
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 12 deletions.
diff --git a/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java b/core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java
@@ -90,6 +90,7 @@
 import com.opensymphony.xwork2.ognl.SecurityMemberAccess;
 import com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor;
 import com.opensymphony.xwork2.ognl.accessor.RootAccessor;
+import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor;
 import com.opensymphony.xwork2.util.OgnlTextParser;
 import com.opensymphony.xwork2.util.PatternMatcher;
 import com.opensymphony.xwork2.util.StrutsLocalizedTextProvider;
@@ -100,6 +101,7 @@
 import com.opensymphony.xwork2.util.fs.DefaultFileManagerFactory;
 import com.opensymphony.xwork2.util.location.LocatableProperties;
 import com.opensymphony.xwork2.util.reflection.ReflectionProvider;
+import ognl.MethodAccessor;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -385,6 +387,7 @@ public static ContainerBuilder bootstrapFactories(ContainerBuilder builder) {
 
                 .factory(ObjectTypeDeterminer.class, DefaultObjectTypeDeterminer.class, Scope.SINGLETON)
                 .factory(RootAccessor.class, CompoundRootAccessor.class, Scope.SINGLETON)
+                .factory(MethodAccessor.class, XWorkMethodAccessor.class, Scope.SINGLETON)
 
                 .factory(ExpressionCacheFactory.class, DefaultOgnlExpressionCacheFactory.class, Scope.SINGLETON)
                 .factory(BeanInfoCacheFactory.class, DefaultOgnlBeanInfoCacheFactory.class, Scope.SINGLETON)

diff --git a/...ain/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java b/...ain/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java
@@ -47,7 +47,6 @@
 import com.opensymphony.xwork2.ognl.accessor.XWorkIteratorPropertyAccessor;
 import com.opensymphony.xwork2.ognl.accessor.XWorkListPropertyAccessor;
 import com.opensymphony.xwork2.ognl.accessor.XWorkMapPropertyAccessor;
-import com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor;
 import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
 import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker;
 import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker;
@@ -66,7 +65,6 @@
 import com.opensymphony.xwork2.validator.DefaultValidatorFileParser;
 import com.opensymphony.xwork2.validator.ValidatorFactory;
 import com.opensymphony.xwork2.validator.ValidatorFileParser;
-import ognl.MethodAccessor;
 import ognl.PropertyAccessor;
 import org.apache.struts2.dispatcher.HttpParameters;
 import org.apache.struts2.dispatcher.Parameter;
@@ -142,8 +140,6 @@ public void register(ContainerBuilder builder, LocatableProperties props) throws
                 .factory(PropertyAccessor.class, HttpParameters.class.getName(), HttpParametersPropertyAccessor.class, Scope.SINGLETON)
                 .factory(PropertyAccessor.class, Parameter.class.getName(), ParameterPropertyAccessor.class, Scope.SINGLETON)
 
-                .factory(MethodAccessor.class, Object.class.getName(), XWorkMethodAccessor.class, Scope.SINGLETON)
-
                 .factory(NullHandler.class, Object.class.getName(), InstantiatingNullHandler.class, Scope.SINGLETON)
                 .factory(ActionValidatorManager.class, AnnotationActionValidatorManager.class, Scope.SINGLETON)
                 .factory(ActionValidatorManager.class, "no-annotations", DefaultActionValidatorManager.class, Scope.SINGLETON)

diff --git a/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java b/core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java
@@ -31,6 +31,8 @@
 import ognl.OgnlRuntime;
 import ognl.PropertyAccessor;
 import org.apache.commons.lang3.BooleanUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.apache.struts2.StrutsConstants;
 
 import java.util.Set;
@@ -40,6 +42,8 @@
  */
 public class OgnlValueStackFactory implements ValueStackFactory {
 
+    private static final Logger LOG = LogManager.getLogger(OgnlValueStackFactory.class);
+
     protected XWorkConverter xworkConverter;
     protected RootAccessor compoundRootAccessor;
     protected TextProvider textProvider;
@@ -57,6 +61,11 @@ protected void setCompoundRootAccessor(RootAccessor compoundRootAccessor) {
         OgnlRuntime.setMethodAccessor(CompoundRoot.class, compoundRootAccessor);
     }
 
+    @Inject
+    protected void setMethodAccessor(MethodAccessor methodAccessor) {
+        OgnlRuntime.setMethodAccessor(Object.class, methodAccessor);
+    }
+
     @Inject("system")
     protected void setTextProvider(TextProvider textProvider) {
         this.textProvider = textProvider;
@@ -79,26 +88,76 @@ protected ValueStack createValueStack(ValueStack stack, boolean useTextProvider)
         return newStack.getActionContext().withContainer(container).withValueStack(newStack).getValueStack();
     }
 
+    /**
+     * {@link PropertyAccessor}'s, {@link MethodAccessor}'s and {@link NullHandler}'s are registered on a per-class
+     * basis by defining a bean adhering to the corresponding interface with a name corresponding to the class it is
+     * intended to handle.
+     * <p>
+     * The only exception is the {@link MethodAccessor} for the {@link Object} type which has its own extension point.
+     *
+     * @see #setMethodAccessor(MethodAccessor)
+     * @see #registerAdditionalMethodAccessors()
+     */
     @Inject
     protected void setContainer(Container container) throws ClassNotFoundException {
-        Set<String> names = container.getInstanceNames(PropertyAccessor.class);
+        this.container = container;
+        registerPropertyAccessors();
+        registerNullHandlers();
+        registerAdditionalMethodAccessors();
+    }
+
+    /**
+     * Note that the default {@link MethodAccessor} for handling {@link Object} methods is registered in
+     * {@link #setMethodAccessor} and can be configured using the extension point
+     * {@link StrutsConstants#STRUTS_METHOD_ACCESSOR}.
+     */
+    protected void registerAdditionalMethodAccessors() {
+        Set<String> names = container.getInstanceNames(MethodAccessor.class);
         for (String name : names) {
-            Class<?> cls = Class.forName(name);
-            OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
+            Class<?> cls;
+            try {
+                cls = Class.forName(name);
+                if (cls.equals(Object.class)) {
+                    // The Object method accessor can only be configured using the struts.methodAccessor extension point
+                    continue;
+                }
+                if (cls.equals(CompoundRoot.class)) {
+                    // TODO: This bean is deprecated, please remove this if statement when removing the struts-beans.xml entry
+                    continue;
+                }
+            } catch (ClassNotFoundException e) {
+                // Since this interface is also used as an extension point for the Object MethodAccessor, we expect
+                // there to be beans with names that don't correspond to classes. We can safely ignore these.
+                continue;
+            }
+            MethodAccessor methodAccessor = container.getInstance(MethodAccessor.class, name);
+            OgnlRuntime.setMethodAccessor(cls, methodAccessor);
+            LOG.info("Registered custom OGNL MethodAccessor [{}] for class [{}]", methodAccessor.getClass().getName(), cls.getName());
         }
+    }
 
-        names = container.getInstanceNames(MethodAccessor.class);
+    protected void registerNullHandlers() throws ClassNotFoundException {
+        Set<String> names = container.getInstanceNames(NullHandler.class);
         for (String name : names) {
             Class<?> cls = Class.forName(name);
-            OgnlRuntime.setMethodAccessor(cls, container.getInstance(MethodAccessor.class, name));
+            NullHandler nullHandler = container.getInstance(NullHandler.class, name);
+            OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(nullHandler));
+            LOG.info("Registered custom OGNL NullHandler [{}] for class [{}]", nullHandler.getClass().getName(), cls.getName());
         }
+    }
 
-        names = container.getInstanceNames(NullHandler.class);
+    protected void registerPropertyAccessors() throws ClassNotFoundException {
+        Set<String> names = container.getInstanceNames(PropertyAccessor.class);
         for (String name : names) {
             Class<?> cls = Class.forName(name);
-            OgnlRuntime.setNullHandler(cls, new OgnlNullHandlerWrapper(container.getInstance(NullHandler.class, name)));
+            if (cls.equals(CompoundRoot.class)) {
+                // TODO: This bean is deprecated, please remove this if statement when removing the struts-beans.xml entry
+                continue;
+            }
+            PropertyAccessor propertyAccessor = container.getInstance(PropertyAccessor.class, name);
+            OgnlRuntime.setPropertyAccessor(cls, propertyAccessor);
+            LOG.info("Registered custom OGNL PropertyAccessor [{}] for class [{}]", propertyAccessor.getClass().getName(), cls.getName());
         }
-        this.container = container;
     }
 
     /**

diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -223,6 +223,9 @@ public final class StrutsConstants {
     /** Extension point for the Struts CompoundRootAccessor */
     public static final String STRUTS_COMPOUND_ROOT_ACCESSOR = "struts.compoundRootAccessor";
 
+    /** Extension point for the Struts MethodAccessor */
+    public static final String STRUTS_METHOD_ACCESSOR = "struts.methodAccessor";
+
     /** The name of the xwork converter implementation */
     public static final String STRUTS_XWORKCONVERTER = "struts.xworkConverter";
 

diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java
@@ -61,6 +61,7 @@
 import com.opensymphony.xwork2.util.reflection.ReflectionContextFactory;
 import com.opensymphony.xwork2.util.reflection.ReflectionProvider;
 import com.opensymphony.xwork2.validator.ActionValidatorManager;
+import ognl.MethodAccessor;
 import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.components.UrlRenderer;
 import org.apache.struts2.components.date.DateFormatter;
@@ -389,6 +390,7 @@ public void register(ContainerBuilder builder, LocatableProperties props) {
         alias(FileManagerFactory.class, StrutsConstants.STRUTS_FILE_MANAGER_FACTORY, builder, props, Scope.SINGLETON);
 
         alias(RootAccessor.class, StrutsConstants.STRUTS_COMPOUND_ROOT_ACCESSOR, builder, props);
+        alias(MethodAccessor.class, StrutsConstants.STRUTS_METHOD_ACCESSOR, builder, props);
 
         alias(XWorkConverter.class, StrutsConstants.STRUTS_XWORKCONVERTER, builder, props);
         alias(CollectionConverter.class, StrutsConstants.STRUTS_CONVERTER_COLLECTION, builder, props);

diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml
@@ -208,6 +208,11 @@
     <bean type="ognl.PropertyAccessor" name="org.apache.struts2.dispatcher.Parameter"
           class="com.opensymphony.xwork2.ognl.accessor.ParameterPropertyAccessor"/>
 
+    <bean type="ognl.MethodAccessor" name="struts"
+          class="com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor"/>
+
+    <!-- DEPRECATED since 6.4.0 - Retained for backwards compatibility with custom beans
+            that may expect it to be present. Inject the DEFAULT bean of 'ognl.MethodAccessor' above instead.  -->
     <bean type="ognl.MethodAccessor" name="java.lang.Object"
           class="com.opensymphony.xwork2.ognl.accessor.XWorkMethodAccessor"/>