[
https://issues.apache.org/jira/browse/WW-5352?focusedWorklogId=897935&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-897935
]
ASF GitHub Bot logged work on WW-5352:
--------------------------------------
Author: ASF GitHub Bot
Created on: 04/Jan/24 00:55
Start Date: 04/Jan/24 00:55
Worklog Time Spent: 10m
Work Description: kusalk commented on code in PR #831:
URL: https://github.com/apache/struts/pull/831#discussion_r1441161370
##########
core/src/main/java/org/apache/struts2/dispatcher/Parameter.java:
##########
@@ -58,7 +58,7 @@ public String getName() {
@Override
public String getValue() {
String[] values = toStringArray();
- return (values != null && values.length > 0) ? values[0] : null;
Review Comment:
`#toStringArray` never returns a null value
##########
core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java:
##########
@@ -132,138 +133,149 @@ static private int countOGNLCharacters(String s) {
@Override
public String doIntercept(ActionInvocation invocation) throws Exception {
Review Comment:
Used early returns to reduce nesting and improve readability
##########
core/src/main/java/com/opensymphony/xwork2/security/AcceptedPatternsChecker.java:
##########
@@ -32,37 +32,37 @@ public interface AcceptedPatternsChecker {
* @param value to check
* @return object containing result of matched pattern and pattern itself
*/
- public IsAccepted isAccepted(String value);
Review Comment:
Redundant in interfaces
##########
core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java:
##########
@@ -132,138 +133,149 @@ static private int countOGNLCharacters(String s) {
@Override
public String doIntercept(ActionInvocation invocation) throws Exception {
Object action = invocation.getAction();
- if (!(action instanceof NoParameters)) {
- ActionContext ac = invocation.getInvocationContext();
- HttpParameters parameters = retrieveParameters(ac);
+ if (action instanceof NoParameters) {
+ return invocation.invoke();
+ }
- if (LOG.isDebugEnabled()) {
- LOG.debug("Setting params {}",
normalizeSpace(getParameterLogMap(parameters)));
- }
+ ActionContext actionContext = invocation.getInvocationContext();
+ HttpParameters parameters = retrieveParameters(actionContext);
- if (parameters != null) {
- Map<String, Object> contextMap = ac.getContextMap();
- try {
- ReflectionContextState.setCreatingNullObjects(contextMap,
true);
- ReflectionContextState.setDenyMethodExecution(contextMap,
true);
-
ReflectionContextState.setReportingConversionErrors(contextMap, true);
-
- ValueStack stack = ac.getValueStack();
- setParameters(action, stack, parameters);
- } finally {
- ReflectionContextState.setCreatingNullObjects(contextMap,
false);
- ReflectionContextState.setDenyMethodExecution(contextMap,
false);
-
ReflectionContextState.setReportingConversionErrors(contextMap, false);
- }
- }
+ if (parameters == null) {
+ return invocation.invoke();
+ }
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Setting params {}",
normalizeSpace(getParameterLogMap(parameters)));
+ }
+
+ Map<String, Object> contextMap = actionContext.getContextMap();
+ batchSetReflectionContextState(contextMap, true);
+ try {
+ setParameters(action, actionContext.getValueStack(), parameters);
+ } finally {
+ batchSetReflectionContextState(contextMap, false);
}
+
return invocation.invoke();
}
/**
* Gets the parameter map to apply from wherever appropriate
*
- * @param ac The action context
+ * @param actionContext The action context
* @return The parameter map to apply
*/
- protected HttpParameters retrieveParameters(ActionContext ac) {
- return ac.getParameters();
+ protected HttpParameters retrieveParameters(ActionContext actionContext) {
+ return actionContext.getParameters();
}
/**
* Adds the parameters into context's ParameterMap
+ * <p>
+ * In this class this is a no-op, since the parameters were fetched from
the same location. In subclasses both this
+ * and {@link #retrieveParameters} should be overridden.
*
* @param ac The action context
* @param newParams The parameter map to apply
- * <p>
- * In this class this is a no-op, since the parameters
were fetched from the same location.
- * In subclasses both retrieveParameters() and
addParametersToContext() should be overridden.
- * </p>
*/
protected void addParametersToContext(ActionContext ac, Map<String, ?>
newParams) {
}
protected void setParameters(final Object action, ValueStack stack,
HttpParameters parameters) {
Review Comment:
Extracted a bunch of helper methods to make this method much more readable
##########
core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java:
##########
@@ -338,18 +344,17 @@ protected boolean acceptableName(String name) {
return false;
}
boolean accepted = isWithinLengthLimit(name) && !isExcluded(name) &&
isAccepted(name);
- if (devMode && accepted) { // notify only when in devMode
Review Comment:
Cleaned out comments for behaviours which are fairly obvious from code
##########
core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java:
##########
@@ -287,13 +302,11 @@ protected boolean isAcceptableParameter(String name,
Object action) {
* @return true if parameter is accepted
*/
protected boolean isAcceptableParameterValue(Parameter param, Object
action) {
- ParameterValueAware parameterValueAware = (action instanceof
ParameterValueAware) ? (ParameterValueAware) action : null;
- boolean acceptableParamValue = (parameterValueAware == null ||
parameterValueAware.acceptableParameterValue(param.getValue()));
- if (hasParamValuesToExclude() || hasParamValuesToAccept()) {
- // Additional validations to process
- acceptableParamValue &= acceptableValue(param.getName(),
param.getValue());
Review Comment:
`&=` doesn't short-circuit
Issue Time Tracking
-------------------
Worklog Id: (was: 897935)
Time Spent: 3.5h (was: 3h 20m)
> Implement annotation mechanism for injectable fields via parameters
> -------------------------------------------------------------------
>
> Key: WW-5352
> URL: https://issues.apache.org/jira/browse/WW-5352
> Project: Struts 2
> Issue Type: Improvement
> Components: Core, Core Interceptors
> Reporter: Kusal Kithul-Godage
> Priority: Minor
> Fix For: 6.4.0
>
> Time Spent: 3.5h
> Remaining Estimate: 0h
>
> struts.parameters.requireAnnotations
>
> Require an explicit annotation '@StrutsParameter' on one of:
> Getter/Setter/Field/ReturnType for injecting parameters.
>
> This mechanism is intended to be a more usable replacement for
> 'ParameterNameAware'
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
