diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/history/HistoryService.java b/core/src/main/java/org/sterl/spring/persistent_tasks/history/HistoryService.java index b6350f8a..4d2ca0f7 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/history/HistoryService.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/history/HistoryService.java @@ -7,6 +7,7 @@ import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Pageable; +import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.sterl.spring.persistent_tasks.api.TriggerKey; import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryDetailEntity; @@ -23,18 +24,18 @@ @TransactionalService @RequiredArgsConstructor public class HistoryService { - private final TriggerHistoryDetailRepository triggerHistoryDetailRepository; private final TriggerHistoryLastStateRepository triggerHistoryLastStateRepository; + private final TriggerHistoryDetailRepository triggerHistoryDetailRepository; private final TriggerService triggerService; public Optional findStatus(long triggerId) { - return triggerHistoryDetailRepository.findById(triggerId); + return triggerHistoryLastStateRepository.findById(triggerId); } public Optional findLastKnownStatus(TriggerKey triggerKey) { - PageRequest page = PageRequest.of(0, 1).withSort(Direction.DESC, "e.data.createdTime", "id"); - var result = triggerHistoryDetailRepository.listKnownStatusFor(triggerKey, page); - return result.isEmpty() ? Optional.empty() : Optional.of(result.get(0)); + final var page = PageRequest.of(0, 1).withSort(Direction.DESC, "data.createdTime", "id"); + final var result = triggerHistoryLastStateRepository.listKnownStatusFor(triggerKey, page); + return result.isEmpty() ? Optional.empty() : Optional.of(result.getContent().get(0)); } public void deleteAll() { @@ -54,14 +55,22 @@ public long countTriggers(TriggerStatus status) { return triggerHistoryDetailRepository.countByStatus(status); } - public List findAllForInstance(long instanceId) { - return triggerHistoryLastStateRepository.findAllByInstanceId(instanceId); + public List findAllDetailsForInstance(long instanceId) { + return triggerHistoryDetailRepository.findAllByInstanceId(instanceId); + } + + public Page findAllDetailsForKey(TriggerKey key) { + return findAllDetailsForKey(key, PageRequest.of(0, 100)); + } + public Page findAllDetailsForKey(TriggerKey key, Pageable page) { + page = sortByIdIfNeeded(page); + return triggerHistoryDetailRepository.listKnownStatusFor(key, page); } public Optional reQueue(Long id, OffsetDateTime runAt) { - final var lastState = triggerHistoryDetailRepository.findById(id); + final var lastState = triggerHistoryLastStateRepository.findById(id); if (lastState.isEmpty()) return Optional.empty(); - + final var data = lastState.get().getData(); final var trigger = lastState.get().newTaskId().newTrigger() .state(data.getState()) @@ -74,19 +83,29 @@ public Optional reQueue(Long id, OffsetDateTime runAt) { } public long countTriggers(TriggerKey key) { - return triggerHistoryDetailRepository.countByKey(key); + return triggerHistoryLastStateRepository.countByKey(key); } public Page findTriggerState( TriggerKey key, Pageable page) { - if (key == null) return triggerHistoryDetailRepository.findAll(page); - if (key.getId() == null && key.getTaskName() == null) return triggerHistoryDetailRepository.findAll(page); + + page = sortByIdIfNeeded(page); + if (key == null) return triggerHistoryLastStateRepository.findAll(page); + if (key.getId() == null && key.getTaskName() == null) return triggerHistoryLastStateRepository.findAll(page); if (key.getId() == null && key.getTaskName() != null) { - return triggerHistoryDetailRepository.findAll(key.getTaskName(), page); + return triggerHistoryLastStateRepository.findAll(key.getTaskName(), page); } - return triggerHistoryDetailRepository.findAll( + return triggerHistoryLastStateRepository.findAll( key.getId(), key.getTaskName(), page); } + + private Pageable sortByIdIfNeeded(Pageable page) { + if (page.getSort() == Sort.unsorted()) { + return PageRequest.of(page.getPageNumber(), page.getPageSize(), + Sort.by(Direction.DESC, "id")); + } + return page; + } } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/history/api/TriggerHistoryResource.java b/core/src/main/java/org/sterl/spring/persistent_tasks/history/api/TriggerHistoryResource.java index c0955be0..662d4bef 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/history/api/TriggerHistoryResource.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/history/api/TriggerHistoryResource.java @@ -30,7 +30,7 @@ public class TriggerHistoryResource { @GetMapping("history/instance/{instanceId}") public List listInstances(@PathVariable("instanceId") long instanceId) { return FromTriggerStateDetailEntity.INSTANCE.convert( // - historyService.findAllForInstance(instanceId)); + historyService.findAllDetailsForInstance(instanceId)); } @GetMapping("history") diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/history/component/TriggerHistoryComponent.java b/core/src/main/java/org/sterl/spring/persistent_tasks/history/component/TriggerHistoryComponent.java index 9757c668..3d620692 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/history/component/TriggerHistoryComponent.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/history/component/TriggerHistoryComponent.java @@ -2,8 +2,8 @@ import java.time.OffsetDateTime; -import org.springframework.transaction.event.TransactionPhase; -import org.springframework.transaction.event.TransactionalEventListener; +import org.springframework.context.event.EventListener; +import org.springframework.transaction.annotation.Transactional; import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryDetailEntity; import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryLastStateEntity; import org.sterl.spring.persistent_tasks.history.repository.TriggerHistoryDetailRepository; @@ -18,14 +18,14 @@ @RequiredArgsConstructor public class TriggerHistoryComponent { - private final TriggerHistoryDetailRepository triggerHistoryDetailRepository; private final TriggerHistoryLastStateRepository triggerHistoryLastStateRepository; + private final TriggerHistoryDetailRepository triggerHistoryDetailRepository; public void write(TriggerEntity e) { var state = new TriggerHistoryLastStateEntity(); state.setId(e.getId()); state.setData(e.getData().toBuilder().build()); - triggerHistoryDetailRepository.save(state); + triggerHistoryLastStateRepository.save(state); var detail = new TriggerHistoryDetailEntity(); detail.setInstanceId(e.getId()); @@ -33,10 +33,11 @@ public void write(TriggerEntity e) { .state(null) .build()); detail.getData().setCreatedTime(OffsetDateTime.now()); - triggerHistoryLastStateRepository.save(detail); + triggerHistoryDetailRepository.save(detail); } - @TransactionalEventListener(phase = TransactionPhase.BEFORE_COMMIT) + @Transactional + @EventListener public void onPersistentTaskEvent(TriggerLifeCycleEvent triggerLifeCycleEvent) { write(triggerLifeCycleEvent.trigger()); } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/HistoryTriggerRepository.java b/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/HistoryTriggerRepository.java index 0e76d714..86e27fc1 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/HistoryTriggerRepository.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/HistoryTriggerRepository.java @@ -1,7 +1,6 @@ package org.sterl.spring.persistent_tasks.history.repository; -import java.util.List; - +import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.NoRepositoryBean; @@ -17,5 +16,5 @@ public interface HistoryTriggerRepository extends Trig SELECT e FROM #{#entityName} e WHERE e.data.key = :key """) - List listKnownStatusFor(@Param("key") TriggerKey key, Pageable page); + Page listKnownStatusFor(@Param("key") TriggerKey key, Pageable page); } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepository.java b/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepository.java index ac18547a..a01b72da 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepository.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepository.java @@ -1,7 +1,40 @@ package org.sterl.spring.persistent_tasks.history.repository; -import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryLastStateEntity; +import java.util.List; -public interface TriggerHistoryDetailRepository extends HistoryTriggerRepository { +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.sterl.spring.persistent_tasks.api.HistoryOverview; +import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryDetailEntity; +public interface TriggerHistoryDetailRepository extends HistoryTriggerRepository { + + @Query(""" + SELECT new org.sterl.spring.persistent_tasks.api.HistoryOverview( + e.instanceId, + e.data.key.taskName, + count(1) as entryCount, + MIN(e.data.start) as start, + MAX(e.data.end) as end, + MIN(e.data.createdTime) as createdTime, + MAX(e.data.executionCount) as executionCount, + AVG(e.data.runningDurationInMs) as runningDurationInMs + ) + FROM #{#entityName} e + GROUP BY + e.instanceId, + e.data.key.taskName + ORDER BY end DESC, createdTime DESC + """) + Page listHistoryOverview(Pageable page); + + @Query(""" + SELECT e + FROM #{#entityName} e + WHERE e.instanceId = :instanceId + ORDER BY e.id DESC + """) + List findAllByInstanceId(@Param("instanceId") long instanceId); } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepository.java b/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepository.java index 2752f107..4ae88949 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepository.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepository.java @@ -8,33 +8,9 @@ import org.springframework.data.repository.query.Param; import org.sterl.spring.persistent_tasks.api.HistoryOverview; import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryDetailEntity; +import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryLastStateEntity; -public interface TriggerHistoryLastStateRepository extends HistoryTriggerRepository { +public interface TriggerHistoryLastStateRepository extends HistoryTriggerRepository { - @Query(""" - SELECT new org.sterl.spring.persistent_tasks.api.HistoryOverview( - e.instanceId, - e.data.key.taskName, - count(1) as entryCount, - MIN(e.data.start) as start, - MAX(e.data.end) as end, - MIN(e.data.createdTime) as createdTime, - MAX(e.data.executionCount) as executionCount, - AVG(e.data.runningDurationInMs) as runningDurationInMs - ) - FROM #{#entityName} e - GROUP BY - e.instanceId, - e.data.key.taskName - ORDER BY end DESC, createdTime DESC - """) - Page listHistoryOverview(Pageable page); - - @Query(""" - SELECT e - FROM #{#entityName} e - WHERE e.instanceId = :instanceId - ORDER BY e.data.createdTime ASC - """) - List findAllByInstanceId(@Param("instanceId") long instanceId); + } diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/shared/repository/TriggerDataRepository.java b/core/src/main/java/org/sterl/spring/persistent_tasks/shared/repository/TriggerDataRepository.java index bb7e66c9..9aab4124 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/shared/repository/TriggerDataRepository.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/shared/repository/TriggerDataRepository.java @@ -43,14 +43,14 @@ SELECT COUNT(e.data.key) long countByKey(@Param("key") TriggerKey key); @Query(""" - SELECT COUNT(e.data.key) + SELECT COUNT(e.id) FROM #{#entityName} e WHERE e.data.status = :status """) long countByStatus(@Param("status") TriggerStatus status); @Query(""" - SELECT COUNT(e.data.key) + SELECT COUNT(e.id) FROM #{#entityName} e WHERE e.data.status IN ( :status ) """) diff --git a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/ReadTriggerComponent.java b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/ReadTriggerComponent.java index b4403ca2..413d1653 100644 --- a/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/ReadTriggerComponent.java +++ b/core/src/main/java/org/sterl/spring/persistent_tasks/trigger/component/ReadTriggerComponent.java @@ -22,7 +22,6 @@ @RequiredArgsConstructor public class ReadTriggerComponent { private final TriggerRepository triggerRepository; - public long countByTaskName(@NotNull String name) { return triggerRepository.countByTaskName(name); diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java index c9cda8f7..87d17424 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/AbstractSpringTest.java @@ -165,7 +165,7 @@ protected Optional runNextTrigger() { protected void awaitRunningTasks() throws TimeoutException, InterruptedException { final long start = System.currentTimeMillis(); - if (triggerService.countTriggers(TriggerStatus.RUNNING) > 0) { + while (triggerService.countTriggers(TriggerStatus.RUNNING) > 0) { if (System.currentTimeMillis() - start > 2000) { throw new TimeoutException("Still running after 2s"); } diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/history/HistoryServiceTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/history/HistoryServiceTest.java index d6880eed..d0083e43 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/history/HistoryServiceTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/history/HistoryServiceTest.java @@ -4,12 +4,15 @@ import java.time.OffsetDateTime; import java.util.Optional; +import java.util.concurrent.TimeoutException; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.domain.PageRequest; import org.sterl.spring.persistent_tasks.AbstractSpringTest; import org.sterl.spring.persistent_tasks.AbstractSpringTest.TaskConfig.Task3; import org.sterl.spring.persistent_tasks.api.AddTriggerRequest; +import org.sterl.spring.persistent_tasks.shared.model.TriggerStatus; import org.sterl.spring.persistent_tasks.trigger.model.TriggerEntity; class HistoryServiceTest extends AbstractSpringTest { @@ -35,4 +38,20 @@ void testReQueueTrigger() { // AND assertThat(subject.countTriggers(trigger.getKey())).isEqualTo(2); } + + @Test + void testTriggerHistory() throws TimeoutException, InterruptedException { + // GIVEN + final var trigger = Task3.ID.newUniqueTrigger("Hallo"); + triggerService.queue(trigger); + persistentTaskService.executeTriggersAndWait(); + // WHEN + var triggers = subject.findAllDetailsForKey(trigger.key(), PageRequest.of(0, 100)).getContent(); + + // AND + assertThat(triggers).hasSize(3); + assertThat(triggers.get(0).getData().getStatus()).isEqualTo(TriggerStatus.SUCCESS); + assertThat(triggers.get(1).getData().getStatus()).isEqualTo(TriggerStatus.RUNNING); + assertThat(triggers.get(2).getData().getStatus()).isEqualTo(TriggerStatus.WAITING); + } } diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepositoryTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepositoryTest.java similarity index 91% rename from core/src/test/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepositoryTest.java rename to core/src/test/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepositoryTest.java index 27538fa2..e82497ec 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryLastStateRepositoryTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/history/repository/TriggerHistoryDetailRepositoryTest.java @@ -11,9 +11,9 @@ import org.sterl.spring.persistent_tasks.history.model.TriggerHistoryDetailEntity; import org.sterl.spring.persistent_tasks.shared.model.TriggerStatus; -class TriggerHistoryLastStateRepositoryTest extends AbstractSpringTest { +class TriggerHistoryDetailRepositoryTest extends AbstractSpringTest { - @Autowired private TriggerHistoryLastStateRepository subject; + @Autowired private TriggerHistoryDetailRepository subject; @Test void testGrouping() { diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java index e133a7e3..7a92eb0d 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/scheduler/SchedulerServiceTransactionTest.java @@ -33,6 +33,12 @@ TransactionalTask savePersonInTrx(PersonRepository personRepository) { return new TransactionalTask() { @Override public void accept(String name) { + try { + Thread.sleep(50); + } catch (InterruptedException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + } personRepository.save(new PersonBE(name)); if (sendError.get()) { throw new RuntimeException("Error requested for " + name); @@ -85,7 +91,7 @@ void testSaveTransactions() throws Exception { // THEN // AND one the service, one the event and one more status update - hibernateAsserts.assertTrxCount(3); + hibernateAsserts.assertTrxCount(4); assertThat(personRepository.count()).isOne(); } @@ -152,8 +158,10 @@ void testRollbackAndRetry() throws Exception { // THEN awaitRunningTasks(); - assertThat(persistentTaskService.getLastTriggerData(key).get().getStatus()) - .isEqualTo(TriggerStatus.WAITING); + // AND the last status before we are back to running should be FAILED + assertThat(historyService.findAllDetailsForKey(key) + .getContent().get(0).getData().getStatus()) + .isEqualTo(TriggerStatus.FAILED); // WHEN sendError.set(false); diff --git a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java index 3fb987ce..b0d34d65 100644 --- a/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java +++ b/core/src/test/java/org/sterl/spring/persistent_tasks/task/TaskTransactionTest.java @@ -128,7 +128,7 @@ void testRequiresNewHasOwnTransaction() { triggerService.run(t).get(); // THEN - hibernateAsserts.assertTrxCount(3); + hibernateAsserts.assertTrxCount(4); assertThat(personRepository.count()).isEqualTo(1); }