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

When scanning super interfaces in OgnlRuntime#getMethods(Class, bool… #40

Merged
merged 2 commits into from
Nov 16, 2017

Conversation

harawata
Copy link
Contributor

@harawata harawata commented Aug 26, 2017

…ean), only default methods should be collected instead of all declared methods. This should fix #35 .

Also, it now searches super interfaces recursively to handle deeper interface hierarchy as follows.

interface InterfaceWithDefaults {
  default public void defaultMethod() { }
}

interface SubInterfaceWithDefaults extends InterfaceWithDefaults {
}

class ClassWithDefaults implements SubInterfaceWithDefaults {
}
@Test
public void testDefaultMethodOnClass() {
  List result = OgnlRuntime.getMethods(
    ClassWithDefaults.class, "defaultMethod", false);
  assertNotNull(defaultMethod);
}

To achieve the recursive search, I extracted some methods. Was it OK ?
I ask because the original code seemed to avoid it intentionally for some reason.

A few not-directly-related things I noticed:

If this PR is acceptable, I will send another PR to backport the change to 3.1 (and 3.0 if it's appropriate).

…ean)`, only default methods should be collected instead of all declared methods.

Also, super interfaces should be searched recursively so that it can handle deeper interface hierarchy.
@lukaszlenart
Copy link
Collaborator

Looks good to me ... give me some time to take another shot (just returned from vacations ;-)

@harawata
Copy link
Contributor Author

harawata commented Sep 4, 2017

Hope you recharged your batteries! =)
Please take your time. I'm not surprised if I missed something.

@lukaszlenart
Copy link
Collaborator

@harawata could you resolve that conflict? Or you can target 3.1.x

harawata added a commit to harawata/ognl that referenced this pull request Nov 15, 2017
When scanning super interfaces in `OgnlRuntime#getMethods(Class, boolean)`, only default methods should be collected instead of all declared methods.
Also, super interfaces should be searched recursively so that it can handle deeper interface hierarchy.
@harawata
Copy link
Contributor Author

@lukaszlenart Done!
I'll open another PR for 3.1.x.

Thanks!

lukaszlenart added a commit that referenced this pull request Nov 16, 2017
Backporting #40 to ognl-3-1-x branch.
@lukaszlenart
Copy link
Collaborator

LGTM 💯 & thank you again :)

@lukaszlenart lukaszlenart merged commit eb83c4c into orphan-oss:master Nov 16, 2017
@harawata harawata deleted the issue35 branch October 1, 2018 14:41
harawata added a commit to harawata/ognl that referenced this pull request Jan 8, 2022
The test passes, but there is a message in System.err: Two methods with same method signature but not providing classes assignable?
This was originally reported as orphan-oss#35 and temporarily resolved by orphan-oss#40 , but as you can see, this is not an error case in the first place.
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.

Output in System.err 'Two methods with same method
2 participants