Fix cast exception when running peon tasks(#16271) #16281
Conversation
| @@ -909,24 +909,9 @@ public CommandListBuilder add(String arg) | |||
| return this; | |||
| } | |||
|
|
|||
| public CommandListBuilder addSystemProperty(String property, int value) | |||
| public CommandListBuilder addSystemProperty(String property, Object value) | |||
There was a problem hiding this comment.
Using an Object as the value of a system property is not appropriate. It must always be a primitive or a String. Hence, the different addSystemProperty() methods.
As a workaround to the original problem, you can simply pass the integer value quoted as a String. (#16271 (comment))
There was a problem hiding this comment.
if (context != null) {
for (String propName : context.keySet()) {
if (propName.startsWith(CHILD_PROPERTY_PREFIX)) {
command.addSystemProperty(
propName.substring(CHILD_PROPERTY_PREFIX.length()),
task.getContextValue(propName).toString()
);
}
}
}
How about calling the toString() method to make the public CommandListBuilder addSystemProperty (String property, String value) called
There was a problem hiding this comment.
Yeah, that would be nice 👍🏻 . The value returned by task.getContextValue() would most likely not be null but best add a null check too before invoking the .toString().
| @@ -316,7 +317,7 @@ public TaskStatus call() | |||
| if (propName.startsWith(CHILD_PROPERTY_PREFIX)) { | |||
| command.addSystemProperty( | |||
| propName.substring(CHILD_PROPERTY_PREFIX.length()), | |||
| task.getContextValue(propName) | |||
| Preconditions.checkNotNull(task.getContextValue(propName), propName).toString() | |||
There was a problem hiding this comment.
I don't think we should throw an exception. Maybe a property was set to null on purpose. We should just use the value of null.
Maybe something like this:
if (propName.startsWith(CHILD_PROPERTY_PREFIX)) {
final String contextValue = task.getContextValue(propName);
command.addSystemProperty(
propName.substring(CHILD_PROPERTY_PREFIX.length()),
contextValue == null ? null : contextValue.toString()
);
} 
|
@soullkk , the build is failing due to low coverage. Do you think you could add a test in |
ok， no problem. |
| command.addSystemProperty( | ||
| propName.substring(CHILD_PROPERTY_PREFIX.length()), | ||
| task.getContextValue(propName) | ||
| contextValue == null ? null : contextValue.toString() |
There was a problem hiding this comment.
if the contextValue is null, does it make sense to call the addSystemProperty method?
There was a problem hiding this comment.
Yeah, that's true. We can just skip the addSystemProperty if the value is null.
Fixes #16271.
Description
When calling method CommandListBuilder.addSystemProperty, if the parameter passed in is a generic type, then the actual method being called is CommandListBuilder addSystemProperty(String property, String value). If the actual type of the generic type is Integer, a cast exception will occur. So overloaded methods with parameters such as int, long, boolean, and String need to be removed, and used Object instead to solve runtime exceptions caused by generic type.
Fixed the bug with cast exception when running peon tasks #16271.
Release note
Fix the bug of cast exception when running peon task.
Key changed/added classes in this PR
This PR has: