Skip to content
This repository has been archived by the owner on Sep 13, 2024. It is now read-only.

if a bean have multiple methods with same name corresponding to a configuration field, the methods can be returned randomly by the jvm and the search was not taken into account the implementation atttribute of a field to match the corresponding set method #52

Merged

Conversation

olamy
Copy link
Contributor

@olamy olamy commented May 2, 2024

Signed-off-by: Olivier Lamy olamy@apache.org

…figuration field, the methods can be returned randomly by the jvm and the search was not taken into account the implementation atttribute of a field to match the corresponding set method

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
Copy link
Contributor Author

olamy commented May 2, 2024

issue found via Jetty issue jetty/jetty.project#11732

…lures depending on random return value of a jdk method

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
Copy link
Contributor Author

olamy commented May 3, 2024

FTR Apache Maven issue https://issues.apache.org/jira/browse/MNG-8116

@olamy
Copy link
Contributor Author

olamy commented May 4, 2024

@mcculls we might need som release thanks

Copy link
Contributor

@kwin kwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

paramTypeHolder[0] = paramTypes[0];
return m;
}
}
paramTypeHolder[0] = paramTypes[0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this assignment should be moved inside the if(candidate == null) block below, otherwise the returned param type won't be the type captured for the candidate but would be the type from the last method in the loop...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcculls Thanks for the review! change made with 7e42714

Copy link
Contributor

@mcculls mcculls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 once the paramType assignment for candidate is fixed

Also can you run mvn spotless:apply to fix up the formatting?

olamy added 2 commits May 5, 2024 10:16
Signed-off-by: Olivier Lamy <olamy@apache.org>
Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy
Copy link
Contributor Author

olamy commented May 5, 2024

+1 once the paramType assignment for candidate is fixed

Also can you run mvn spotless:apply to fix up the formatting?

spotless:apply done

@mcculls mcculls merged commit 132f6f3 into eclipse-sisu:master May 5, 2024
12 checks passed
@olamy olamy deleted the fix-class-same-name-settter-different-type branch May 5, 2024 01:54
@olamy
Copy link
Contributor Author

olamy commented May 5, 2024

@mcculls thanks for merging! Would it be possible to have a release for integrating the fix with next Maven release?

@@ -264,21 +264,35 @@ private Object convertProperty( final Class<?> beanType, final Class<?> rawPrope
listener );
}

private static Method findMethod( final Class<?> beanType, final Type[] paramTypeHolder, final String methodName )
private Method findMethod( final Class<?> beanType, final Type[] paramTypeHolder, final String methodName,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this was converted from class method to instance method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not really intentional. but could it be a problem for a private method?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's undo it, created #64

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants