Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WW-5381 Introduce extension point for MethodAccessor #825

Merged
merged 5 commits into from
Jan 6, 2024
Merged

WW-5381 Introduce extension point for MethodAccessor #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c322aba
WW-5381 Introduce extension point for MethodAccessor
kusalk Dec 29, 2023
573dbbc
WW-5381 Revert bean removal for backwards compatibility
kusalk Jan 3, 2024
d4a0b3b
WW-5381 Reimplement ability to register additional MethodAccessors
kusalk Jan 5, 2024
78e97a4
Merge branch 'master' into WW-5381-method-accessor
kusalk Jan 5, 2024
503c8da
WW-5381 Remove unnecessary/confusing parameters
kusalk Jan 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/src/main/java/com/opensymphony/xwork2/config/impl/DefaultConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -389,6 +391,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)
Expand Down
4 changes: 0 additions & 4 deletions ...ain/java/com/opensymphony/xwork2/config/providers/StrutsDefaultConfigurationProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
75 changes: 67 additions & 8 deletions core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now there is one drawback here - applications lose the ability to specify custom method accessors for different types. They will now instead need to use the extension point and do the delegation within this "global" method accessor. I'm not sure there's a way to retain this capability and make it compatible with the current extension point system.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... so maybe just mark this functionality as @deprecated or remove - once someone starts complain we can re-think how to put back such functionality.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm actually just had another think and we should be able to retain backwards compatibility, the code will be a little ugly though - let me give it a shot

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are good now :) Hopefully the code makes sense, added some JavaDoc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

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;
}

/**
Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/struts2/StrutsConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions core/src/main/resources/struts-beans.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>

Expand Down