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.

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

Fixing dispatching of indexed parameter req #323

Merged
merged 2 commits into from
Aug 30, 2024
Merged

Conversation

notthetup
Copy link
Collaborator

@notthetup notthetup commented Aug 29, 2024

This is a fix for ParameterMessageBehavior when setting indexed parameters. When an Agent has a defined setter method setMyParam(int ndx, Object value), the ParameterMessageBehavior can't find the method.

I traced this down to Apache Commons Lang. ParameterMessageBehavior calls invokeCompatibleSetter which then calls MethodUtils.invokeMethod(agent, name, ndx, value), where name is the method name (setMyParam in our example), ndx is the parameter index and value is the value to be set.

While doing the reflection MethodUtils does a ClassUtils.toClass(args) on the list of all args. That converts the int ndx into a boxed Integer. Because of this, looks ups of indexed setters can fail since they're commonly defined with the index being an int.

This fix tries to detect that and tries to use the standard Java reflection API (Class.getMethod) to find an appropriate method to dispatch to.

@notthetup notthetup requested a review from mchitre August 29, 2024 07:06
@notthetup notthetup self-assigned this Aug 29, 2024
@notthetup notthetup marked this pull request as draft August 29, 2024 07:56
@notthetup notthetup closed this Aug 29, 2024
@notthetup notthetup reopened this Aug 29, 2024
@notthetup notthetup marked this pull request as ready for review August 29, 2024 08:44
@notthetup notthetup merged commit 5d106c5 into master Aug 30, 2024
1 of 2 checks passed
@notthetup notthetup deleted the ndx-parameter-fix branch August 30, 2024 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants