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 2 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
11 changes: 5 additions & 6 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 @@ -57,6 +57,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 Down Expand Up @@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException {
OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
}

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!

for (String name : names) {
Class<?> cls = Class.forName(name);
OgnlRuntime.setMethodAccessor(cls, container.getInstance(MethodAccessor.class, name));
}

names = container.getInstanceNames(NullHandler.class);
for (String name : names) {
Class<?> cls = Class.forName(name);
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 @@ -198,6 +198,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