@DomainEvents method not being called on saveAll with Window<>

[jiaheng]

I have an entity which will registerEvent when I update the field, and MyClassModifyEventHandler.handle() is expected to be executed upon repository.save() or repository.saveAll()

@DynamicInsert
@DynamicUpdate
@Entity
public class MyClass extends AbstractAggregateRoot<MyClass> {

    @Id private UUID id;

    @Column(name = "field")
    private String field;

    @CreationTimestamp Instant createdAt;
    @UpdateTimestamp Instant updatedAt;
    @Version Integer version;

    public void setField(String field) {
        this.field = field;
        registerEvent(new MyClassModifyEvent(this));
    }
}

@Component
public class MyClassModifyEventHandler {
    @EventListener
    public void handle(final MyClassModifyEvent persistenceEvent) {
        // do something
    }
}

I have a method which will query a list of MyClass and update their respective fields.

    @Transactional
    public void eventHandlerNotBeingCalled() {
        // ..
        Window<MyClass> myClasses =
                myClassRepo.findFirst20(ScrollPosition.keyset());

        for (MyClass myClass : myClasses) {
            myClass.setField("newField");
        }
        myClassRepo.findFirst20.saveAll(myClasses);
        // working solution
        // myClassRepo.findFirst20.saveAll(myClasses.toList());
    }

So with this, the @EventListener is not working even though the event is registered before saveAll(). If I calling saveAll on List<MyClass> instead (ie myClassRepo.saveAll(myClasses.toList())), the MyClassModifyEventHandler.handle will be called. Is this expected behaviour?

[quaff]

Could you provide a minimal reproducer project?

[jiaheng]

Could you provide a minimal reproducer project?

@quaff here is the demo project https://github.com/jiaheng/spring-data-jpa-domain-event, and these are the steps to reproduce:

1. Insert record(s) via POST /my-classes endpoint

curl -X 'POST' \
  'http://localhost:8080/my-classes' \
  -H 'accept: */*' \
  -d ''

2. Trigger batch update via POST /my-classes/created/update endpoint

curl -X 'POST' \
  'http://localhost:8080/my-classes/created/update' \
  -H 'accept: */*' \
  -d ''

Based on the demo code EventHandler.handleMyClassUpdateEvent is not being called upon myClassRepository.saveAll(myClasses).
By changing line here to myClassRepository.saveAll(myClasses.toList());, EventHandler.handleMyClassUpdateEvent will be called using same steps above.

[quaff]

It's not minimal, and doesn't contains any test case.

[jiaheng]

It's not minimal, and doesn't contains any test case.

@quaff I updated spring-data-jpa-domain-event repo to strip away some unnecessary code and component. Also I added 2 test cases to compare the behaviour of using List<> and Window<>. Let me know if this works for you.

[quaff]

It can be minified to this:

package com.example.demo.service;

import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.event.EventListener;
import org.springframework.data.domain.AbstractAggregateRoot;
import org.springframework.data.domain.KeysetScrollPosition;
import org.springframework.data.domain.ScrollPosition;
import org.springframework.data.domain.Window;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.stereotype.Repository;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.then;

@EnableJpaRepositories(considerNestedRepositories = true)
@SpringBootTest
class DomainEventHandlerTest {

	@Autowired
	MyClassRepository myClassRepository;

	@Autowired
	MyClassService myClassService;

	@SpyBean
	EventHandler eventHandler;

	@Test
	void test() {
        myClassRepository.save(new MyClass());
		myClassService.update();
        then(eventHandler).should().handleMyClassUpdateEvent(any());
	}

	@TestConfiguration
	static class Config {
		@Bean
		MyClassService myClassService(MyClassRepository myClassRepository) {
			return new MyClassService(myClassRepository);
		}

		@Bean
		EventHandler eventHandler() {
			return new EventHandler();
		}
	}

	@Entity
	static class MyClass extends AbstractAggregateRoot<MyClass> {
		@Id
		@GeneratedValue
		private Integer id;

		private String field;

		public void setField(String field) {
			this.field = field;
			registerEvent(new MyClassUpdateEvent());
		}
	}

	@Repository
	interface MyClassRepository extends JpaRepository<MyClass, Integer> {
		Window<MyClass> findFirst1By(KeysetScrollPosition position);
	}

	static class MyClassUpdateEvent {
	}

	static class MyClassService {
		private final MyClassRepository myClassRepository;

		public MyClassService(MyClassRepository myClassRepository) {
			this.myClassRepository = myClassRepository;
		}

		public void update() {
			Window<MyClass> myClasses = myClassRepository.findFirst1By(ScrollPosition.keyset());
			for (MyClass myClass : myClasses) {
				myClass.setField("updated");
			}
			myClassRepository.saveAll(myClasses); // change myClasses to myClasses.toList() will pass
		}
	}

	static class EventHandler {
		@EventListener
		public void handleMyClassUpdateEvent(MyClassUpdateEvent event) {
		}
	}

}

[quaff]

I believe it's a bug of spring-data-commons, I'm going to fix it.

[jiaheng]

It can be minified to this:

package com.example.demo.service;

import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.TestConfiguration;
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.context.annotation.Bean;
import org.springframework.context.event.EventListener;
import org.springframework.data.domain.AbstractAggregateRoot;
import org.springframework.data.domain.KeysetScrollPosition;
import org.springframework.data.domain.ScrollPosition;
import org.springframework.data.domain.Window;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.stereotype.Repository;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.then;

@EnableJpaRepositories(considerNestedRepositories = true)
@SpringBootTest
class DomainEventHandlerTest {

	@Autowired
	MyClassRepository myClassRepository;

	@Autowired
	MyClassService myClassService;

	@SpyBean
	EventHandler eventHandler;

	@Test
	void test() {
        myClassRepository.save(new MyClass());
		myClassService.update();
        then(eventHandler).should().handleMyClassUpdateEvent(any());
	}

	@TestConfiguration
	static class Config {
		@Bean
		MyClassService myClassService(MyClassRepository myClassRepository) {
			return new MyClassService(myClassRepository);
		}

		@Bean
		EventHandler eventHandler() {
			return new EventHandler();
		}
	}

	@Entity
	static class MyClass extends AbstractAggregateRoot<MyClass> {
		@Id
		@GeneratedValue
		private Integer id;

		private String field;

		public void setField(String field) {
			this.field = field;
			registerEvent(new MyClassUpdateEvent());
		}
	}

	@Repository
	interface MyClassRepository extends JpaRepository<MyClass, Integer> {
		Window<MyClass> findFirst1By(KeysetScrollPosition position);
	}

	static class MyClassUpdateEvent {
	}

	static class MyClassService {
		private final MyClassRepository myClassRepository;

		public MyClassService(MyClassRepository myClassRepository) {
			this.myClassRepository = myClassRepository;
		}

		public void update() {
			Window<MyClass> myClasses = myClassRepository.findFirst1By(ScrollPosition.keyset());
			for (MyClass myClass : myClasses) {
				myClass.setField("updated");
			}
			myClassRepository.saveAll(myClasses); // change myClasses to myClasses.toList() will pass
		}
	}

	static class EventHandler {
		@EventListener
		public void handleMyClassUpdateEvent(MyClassUpdateEvent event) {
		}
	}

}

Got it. Thanks for the suggestion.

[odrotbohm]

This has been fixed for the 2.7, 3.0, 3.1 bugfix releases and the 3.2 RC1 coming. I've turned this into a bug as we already had test cases in place that described the feature as being expected to be available but ultimately didn't actually test it was.

This is now in place by handling calls to saveAll(…) explicitly and considering the Iterable handed to it as is.