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

Adding spring-boot-starter-data-jpa dependency stops @ApplicationModuleListener receiving events #477

Open
timothysparg opened this issue Jan 21, 2024 · 6 comments
Assignees
Labels
in: event publication registry Event publication registry meta: waiting for feedback Waiting for feedback of the original reporter

Comments

@timothysparg
Copy link

the following snippet works until I add the spring-boot-starter-data-jpa dependency.

@EnableScheduling
@SpringBootApplication
public class DemoApplication {

  private ApplicationEventPublisher publisher;

    public DemoApplication(ApplicationEventPublisher publisher) {
        this.publisher = publisher;
    }

    public static void main(String[] args) {
		SpringApplication.run(DemoApplication.class, args);
	}

	@ApplicationModuleListener
	public void onCustomEvent(CustomEvent event) {
		System.out.println("Event received!");
	}

	@Scheduled(timeUnit = TimeUnit.SECONDS, fixedRate = 5)
	public void fireEvents() {
		System.out.println("firing event");
		publisher.publishEvent(new CustomEvent());
	}

	record CustomEvent() {}
}

Is there something that I am missing ?

@timothysparg
Copy link
Author

when I add @Transactional then it works.

	@Transactional
	@Scheduled(timeUnit = TimeUnit.SECONDS, fixedRate = 5)
	public void fireEvents() {
		System.out.println("firing event");
		publisher.publishEvent(new CustomEvent());
	}

In hindsight when I look at the code snippets in the events reference page I can see the implication that all events need to be transacted, but maybe it should be more explict?

@odrotbohm
Copy link
Member

I am inclined your original code won't work without the JPA starter either as @ApplicationModuleListener is a @TransactionalEventListener in standard Spring terms. Thus, without the scheduled method producing the event within some transaction, it won't actually get invoked anyway.

@odrotbohm odrotbohm self-assigned this Jan 29, 2024
@odrotbohm odrotbohm added the meta: waiting for feedback Waiting for feedback of the original reporter label Jan 29, 2024
@timothysparg
Copy link
Author

Hi Oliver,

Apologies for the delayed response, I was on holiday 🙂

I've created the a repo where I replicated the above. https://github.com/timothysparg/sm-477

If you run that app without any modifications it should fire and receive events.
If you uncomment the spring-boot-starter-data-jpa dependency in the pom then it will never receive any events.

@Arondc
Copy link

Arondc commented Feb 11, 2024

@timothysparg I ran into the exact same issue. The issue is, that the method that produces publishes your events needs to be @Transactional. You've already mentioned that in your past comment.

As far as I understood it, as long as you don't add a database starter to your Modulith project, Modulith won't try to persist your events and thus no @Transactional is needed, because the event handling won't (can't?) be backed by an event publication registry.

As soon as a database starter is part of your project it will automatically connect to your datasource to keep your events in a table.

This may be an oversight in autoconfiguration and/or documentation.

@timothysparg
Copy link
Author

As far as I understood it, as long as you don't add a database starter to your Modulith project, Modulith won't try to persist your events and thus no @transactional is needed, because the event handling won't (can't?) be backed by an event publication registry.

I think you need to add an explicit modulith db starter for it to persist events to the database. Or at least that is the behaviour that I have seen.

This may be an oversight in autoconfiguration and/or documentation.

Broadly this is what I am trying to understand - if it is the expected behaviour I feel like it should be more explicit and if possible shouldn't just silently fail. There will be other people trying to get started who run into the same thing.

@odrotbohm
Copy link
Member

Thanks, everyone, for chiming in here. I dug into the example and here are my findings.

  • Fundamentally, an @ApplicationModuleListener is a @TransactionalEventListener that only gets triggered on transaction commit. In other words, without a transaction running, those should and usually will not be invoked.
  • The reason you see the listener invoked without a transaction is that, in its original state, the reproducer project, does not contain any database-related dependencies which would implicitly cause @EnableTransactionManagement to be activated. Without that in place, the transactional event listener is detected and registered as an ordinary event listener and directly invoked for the event publication.
  • This also explains why adding the Spring Modulith JPA starter dependency turns the project back to behaving as intended (the event not reaching the listener at all). The presence of the database-related dependencies implicitly activates the proper handling of the listener.

I've filed spring-projects/spring-framework#32319 to fix the transactional event listener handling in case no transactional infrastructure is enabled. I am inclined to close this as working as designed.

@odrotbohm odrotbohm changed the title Adding spring-boot-starter-data-jpa dependency stops ApplicationModuleListener receiving events Adding spring-boot-starter-data-jpa dependency stops @ApplicationModuleListener receiving events Feb 23, 2024
@odrotbohm odrotbohm added the in: event publication registry Event publication registry label Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: event publication registry Event publication registry meta: waiting for feedback Waiting for feedback of the original reporter
Projects
None yet
Development

No branches or pull requests

3 participants