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

ApplicationListener no longer invoked for generic ApplicationEvent with 6.2.0 #33982

Closed
denAbramoff opened this issue Nov 28, 2024 · 10 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@denAbramoff
Copy link

denAbramoff commented Nov 28, 2024

Hello!

I have simple interface like this:

public interface ITest
{
}

and the simple event:

public class TestEvent<T extends ITest> extends ApplicationEvent
{
    public TestEvent(Object source)
    {
        super(source);
    }
}

and two listeners:

@Component
public class MethodListener
{
    @EventListener
    public void handleTestEvent(TestEvent<ITest> iTest)
    {
        System.out.println(this.getClass() + " DONE!");
    }
}
@Component
public class SimpleTestListener implements ApplicationListener<TestEvent<ITest>>
{
    @Override
    public void onApplicationEvent(TestEvent<ITest> event)
    {
        System.out.println(this.getClass() + " DONE!");
    }
}

and when i trying to send an event via the spring context like this:

public class MySpringApplication
{
    public static void main(String[] args)
    {
        ApplicationContext context = new AnnotationConfigApplicationContext(AppConfig.class);
        context.publishEvent(new TestEvent<>(""));
    }
}

i have different result in depends on the version Spring. When i use Spring 6.2 i can see in the log this:

class ... MethodListener DONE!

when i use Spring 6.1.15 i see both results:

class ...SimpleTestListener DONE!
class ...MethodListener DONE!

I guess it's a bug.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 28, 2024
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 28, 2024
@sbrannen
Copy link
Member

If you make the parameterized type for TestEvent concrete as follows, you get the expected result.

context.publishEvent(new TestEvent<ITest>("") {});

Note the use of {} which makes it a subclass of TestEvent bound to ITest instead of T.

So this might have been a bug fix instead of a bug/regression.

@jhoeller, thoughts?

@sbrannen
Copy link
Member

sbrannen commented Nov 28, 2024

The following all-in-one test class demonstrates this.

@SpringJUnitConfig
class GenericApplicationEventListenerTests {

	@Test
	void test(ApplicationContext context) {
		context.publishEvent(new TestEvent<ITest>("") {
		});
	}


	@Configuration
	@Import({MethodListener.class, SimpleTestListener.class})
	static class Config {
	}

	interface ITest {
	}

	static class TestEvent<T extends ITest> extends ApplicationEvent {

		TestEvent(Object source) {
			super(source);
		}
	}

	static class MethodListener {

		@EventListener
		public void handleTestEvent(TestEvent<ITest> iTest) {
			System.err.println(getClass().getSimpleName() + " DONE!");
		}
	}

	static class SimpleTestListener implements ApplicationListener<TestEvent<ITest>> {

		@Override
		public void onApplicationEvent(TestEvent<ITest> event) {
			System.err.println(getClass().getSimpleName() + " DONE!");
		}
	}

}

Output:

SimpleTestListener DONE!
MethodListener DONE!

@sbrannen sbrannen changed the title Broken ApplicationListener after 6.2.0 ApplicationListener no longer invoked for generic ApplicationEvent with 6.2.0 Nov 28, 2024
@anaconda875
Copy link

anaconda875 commented Nov 28, 2024

Hi @sbrannen, I'm a Java developer who want to contribute to the framework. How can I get this issue/task assigned (to me)? Thanks

@denAbramoff
Copy link
Author

@sbrannen thanks for your reaction! Yep, this version works:

context.publishEvent(new TestEvent<ITest>("") {});

But it's not convenient and there are a lot of places where old version works.

And there is also one weird case, if i add "? extends" my listenerer will work:

@Configuration
public class WildcardSimpleTestListener implements ApplicationListener<TestEvent<? extends ITest>>
{
    @Override
    public void onApplicationEvent(TestEvent<? extends ITest> event)
    {
        System.out.println(this.getClass() + " DONE!");
    }
}

It works on Spring 6.2!

class ...WildcardSimpleTestListener$$SpringCGLIB$$0 DONE!

@theigl
Copy link

theigl commented Nov 29, 2024

Probably related to #33968.

@sbrannen
Copy link
Member

Thanks for the feedback, @denAbramoff.

In light of the above, I believe this is a regression, and we'll look into it.

@jhoeller, I have tentatively assigned this to you due to your work on related issues.

Potentially Related Issues

@sbrannen sbrannen added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 30, 2024
@sbrannen
Copy link
Member

Hi @sbrannen, I'm a Java developer who want to contribute to the framework. How can I get this issue/task assigned (to me)? Thanks

Hi @anaconda875,

Thanks for the offer!

As you can see above, I've assigned to this @jhoeller, since he has done the most recent work in this area. However, if you feel confident in fixing this regression, please post your ideas here first before submitting a PR.

@sbrannen sbrannen added this to the 6.2.1 milestone Nov 30, 2024
@anaconda875
Copy link

anaconda875 commented Dec 1, 2024

Hi @sbrannen ,
In Spring Core , ResolvableType.isAssignableFrom line 336-337:

WildcardBounds ourBounds = WildcardBounds.get(this);    //ourBounds IS NULL SINCE this PASSED TO WildcardBounds.get() IS A ResolvableType OF ITest
WildcardBounds typeBounds = WildcardBounds.get(other);  //typeBounds IS NOT NULL SINCE other PASSED TO WildcardBounds.get() IS ResolvableType OF <T extends ITest>

My proposal:
image

Inside red circle is newly added.
Plz take a look!! Thanks

@jhoeller
Copy link
Contributor

jhoeller commented Dec 5, 2024

This turned out to be rather involved, a consequence of the partial generics matching that we do as of 6.2 now where we also take the bounds of an unresolvable type variable into account. The solution that I went with is a rather minimal tweaking of the existing strict flag when we go into nested generics. I hope that this covers the reported case here. @denAbramoff please give an upcoming 6.2.1 snapshot an early try...

@anaconda875 I ended up with a different approach but thanks for the PR in any case!

@sbrannen
Copy link
Member

sbrannen commented Dec 6, 2024

I hope that this covers the reported case here.

I tested with context.publishEvent(new TestEvent<>("")); on main, and that now works as expected.

Thanks, Juergen!

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) type: regression A bug that is also a regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants