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

Fix ReflectionUtils#getDeclaredMethods to provide consistent result #34110

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

facewise
Copy link

@facewise facewise commented Dec 18, 2024

Hi. This PR is inspired by gh-34082.

As @welovelain pointed in it, the test below sometimes passes but sometimes doesn't.

import jakarta.servlet.http.Cookie;
import org.junit.jupiter.api.Test;
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
import org.springframework.util.ReflectionUtils;
import java.lang.reflect.Method;

import static org.assertj.core.api.Assertions.assertThat;

class ReflectionTests {
    @Test
    void test() {
        Method cookie = ReflectionUtils.findMethod(MockHttpServletRequestBuilder.class, "cookie", Cookie[].class);
        assertThat(cookie.isVarArgs()).isTrue();
    }
}

I've tested with IntelliJ IDEA 2024.3 and on several JDKs such as Temurin 17.0.13, Temurin 21.0.3, Corretto 21.0.5, etc., but the problem occurred upon all these JDKs.

You can check it out with this reproducer.

sample.zip

The problem is, how Class#getDeclaredMethods() returns a Method[] in a different order on each run. However ReflectionUtils#getDeclaredMethods() just iterates through this array and returns the first matched method.

Even ReflectionUtilsTests#findMethod() sometimes fails if you remove the sorting line introduced in this changes.

FWIW ReflectionUtils in junit5 (heavily contributed by @sbrannen) also sorts methods of Class#getDeclaredMethods() and I believe this is worth it.

My sorter would not be optimized but I intended that the methods declared in actual source come first.

@facewise facewise changed the title Fix ReflectionUtils#getDeclaredMethods to provide consistent result. Fix ReflectionUtils#getDeclaredMethods to provide consistent result Dec 18, 2024
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 18, 2024
@sbrannen sbrannen self-assigned this Dec 18, 2024
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: waiting-for-triage An issue we've not yet triaged or decided on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants