[
https://issues.apache.org/jira/browse/WW-5326?focusedWorklogId=987868&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-987868
]
ASF GitHub Bot logged work on WW-5326:
--------------------------------------
Author: ASF GitHub Bot
Created on: 18/Oct/25 10:59
Start Date: 18/Oct/25 10:59
Worklog Time Spent: 10m
Work Description: Copilot commented on code in PR #1249:
URL: https://github.com/apache/struts/pull/1249#discussion_r2442158045
##########
pom.xml:
##########
@@ -120,7 +120,7 @@
<log4j2.version>2.24.3</log4j2.version>
<maven-surefire-plugin.version>3.5.2</maven-surefire-plugin.version>
<mockito.version>5.15.2</mockito.version>
- <ognl.version>3.3.5</ognl.version>
+ <ognl.version>3.5.0-SNAPSHOT</ognl.version>
Review Comment:
Snapshot dependency (3.5.0-SNAPSHOT) with an activeByDefault snapshot
repository profile will leak into all builds. Remove or disable the ognl
profile and pin a released OGNL version before merging to avoid unintended
snapshot consumption.
```suggestion
<ognl.version>3.4.2</ognl.version>
```
##########
core/src/main/java/org/apache/struts2/util/StrutsTypeConverter.java:
##########
@@ -89,5 +91,5 @@ protected Object performFallbackConversion(Map context,
Object o, Class toClass)
* @param o the object to be converted
* @return the converted String
*/
- public abstract String convertToString(Map context, Object o);
+ public abstract String convertToString(StrutsContext context, Object o);
Review Comment:
Javadoc above still references Map-based conversion semantics but method
signatures now use StrutsContext. Update documentation to reflect new parameter
type and any behavioral differences to prevent confusion in custom converter
implementations.
##########
core/src/main/java/org/apache/struts2/ActionContext.java:
##########
@@ -232,11 +242,17 @@ public Map<String, Object> getApplication() {
* Gets the context map.
*
* @return the context map.
+ * @deprecated since 7.1.0, use {@link #getStrutsContext()} instead
*/
+ @Deprecated(since = "7.1.0", forRemoval = true)
public Map<String, Object> getContextMap() {
return context;
}
+ public StrutsContext getStrutsContext() {
+ return context;
Review Comment:
Deprecation notice recommends getStrutsContext(), but getContextMap() still
returns the StrutsContext instance directly (which is no longer a plain Map in
user intent). Clarify in javadoc that returned object implements Map only for
legacy compatibility and encourage migration steps.
##########
core/src/main/java/org/apache/struts2/conversion/impl/XWorkBasicConverter.java:
##########
@@ -185,40 +184,40 @@ private Class doConvertToClass(Object value) {
return clazz;
}
- private Object doConvertToCollection(Map<String, Object> context, Object
o, Member member, String prop, Object value, Class toType) {
- TypeConverter converter =
container.getInstance(CollectionConverter.class);
+ private Object doConvertToCollection(StrutsContext context, Object o,
Member member, String prop, Object value, Class<?> toType) {
+ TypeConverter<StrutsContext> converter =
container.getInstance(CollectionConverter.class);
if (converter == null) {
throw new TypeConversionException("TypeConverter with name [#0]
must be registered first! Converter: "+
StrutsConstants.STRUTS_CONVERTER_COLLECTION);
}
return converter.convertValue(context, o, member, prop, value, toType);
}
- private Object doConvertToArray(Map<String, Object> context, Object o,
Member member, String prop, Object value, Class toType) {
- TypeConverter converter = container.getInstance(ArrayConverter.class);
+ private Object doConvertToArray(StrutsContext context, Object o, Member
member, String prop, Object value, Class<?> toType) {
+ TypeConverter<StrutsContext> converter =
container.getInstance(ArrayConverter.class);
if (converter == null) {
throw new TypeConversionException("TypeConverter with name [#0]
must be registered first! Converter: "+ StrutsConstants.STRUTS_CONVERTER_ARRAY);
}
return converter.convertValue(context, o, member, prop, value, toType);
}
- private Object doConvertToDate(Map<String, Object> context, Object value,
Class toType) {
- TypeConverter converter = container.getInstance(DateConverter.class);
+ private Object doConvertToDate(StrutsContext context, Object value,
Class<?> toType) {
+ TypeConverter<StrutsContext> converter =
container.getInstance(DateConverter.class);
if (converter == null) {
throw new TypeConversionException("TypeConverter with name [#0]
must be registered first! Converter: "+ StrutsConstants.STRUTS_CONVERTER_DATE);
}
return converter.convertValue(context, null, null, null, value,
toType);
}
- private Object doConvertToNumber(Map<String, Object> context, Object
value, Class toType) {
- TypeConverter converter = container.getInstance(NumberConverter.class);
+ private Object doConvertToNumber(StrutsContext context, Object value,
Class<?> toType) {
+ TypeConverter<StrutsContext> converter =
container.getInstance(NumberConverter.class);
if (converter == null) {
throw new TypeConversionException("TypeConverter with name [#0]
must be registered first! Converter: "+
StrutsConstants.STRUTS_CONVERTER_NUMBER);
}
return converter.convertValue(context, null, null, null, value,
toType);
}
- private Object doConvertToString(Map<String, Object> context, Object
value) {
- TypeConverter converter = container.getInstance(StringConverter.class);
+ private Object doConvertToString(StrutsContext context, Object value) {
Review Comment:
Internal helper methods all accept StrutsContext but some ignore it (e.g.,
doConvertToString). If context is not used, consider removing the parameter to
simplify signatures or document future planned use to justify retention.
##########
core/src/main/java/org/apache/struts2/conversion/impl/InstantiatingNullHandler.java:
##########
@@ -145,7 +148,7 @@ public Object nullPropertyValue(Map<String, Object>
context, Object target, Obje
return null;
}
- private Object createObject(Class clazz, Object target, String property,
Map<String, Object> context) throws Exception {
+ private Object createObject(Class clazz, Object target, String property,
StrutsContext context) throws Exception {
Review Comment:
Javadoc above still refers to ObjectFactory#buildBean(Class, java.util.Map);
update the reference to ObjectFactory#buildBean(Class, StrutsContext) to
prevent outdated guidance.
##########
core/src/main/java/org/apache/struts2/util/StrutsTypeConverter.java:
##########
@@ -67,7 +69,7 @@ public Object convertValue(Map context, Object o, Class
toClass) {
* @param toClass the class to convert to
* @return The fallback conversion
*/
- protected Object performFallbackConversion(Map context, Object o, Class
toClass) {
+ protected Object performFallbackConversion(StrutsContext context, Object
o, Class toClass) {
Review Comment:
Javadoc above still references Map-based conversion semantics but method
signatures now use StrutsContext. Update documentation to reflect new parameter
type and any behavioral differences to prevent confusion in custom converter
implementations.
##########
pom.xml:
##########
@@ -209,6 +209,23 @@
</plugins>
</build>
</profile>
+ <!
Issue Time Tracking
-------------------
Worklog Id: (was: 987868)
Time Spent: 20m (was: 10m)
> Upgrade OGNL to version 3.5.x
> -----------------------------
>
> Key: WW-5326
> URL: https://issues.apache.org/jira/browse/WW-5326
> Project: Struts 2
> Issue Type: Dependency
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.2.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> OGNL 3.5.x introduces a few major changes - the upgrade depends on this
> change in OGNL https://github.com/orphan-oss/ognl/pull/376
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
