Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.support.TransactionTemplate;

import java.time.Duration;
import java.time.Instant;
Expand Down Expand Up @@ -37,12 +37,15 @@ public class SpotCleanupService {
private static final int CLEANUP_BATCH_SIZE = 100_000;

private final SpotRepository spotRepository;
private final TransactionTemplate transactionTemplate;
private final Duration ttl;

public SpotCleanupService(
SpotRepository spotRepository,
TransactionTemplate transactionTemplate,
@Value("${nextskip.spots.persistence.ttl:24h}") Duration ttl) {
this.spotRepository = spotRepository;
this.transactionTemplate = transactionTemplate;
this.ttl = ttl;
}

Expand Down Expand Up @@ -81,12 +84,17 @@ public int executeCleanup() {
/**
* Deletes a single batch of expired spots within its own transaction.
*
* <p>Uses programmatic transaction management via {@link TransactionTemplate}
* to ensure each batch commits independently, even when called from within
* the same class (avoiding Spring's proxy-based AOP limitation).
*
* @param cutoff delete spots created before this time
* @return number of spots deleted in this batch
*/
@Transactional
int deleteBatch(Instant cutoff) {
return spotRepository.deleteExpiredSpotsBatch(cutoff, CLEANUP_BATCH_SIZE);
Integer deleted = transactionTemplate.execute(
status -> spotRepository.deleteExpiredSpotsBatch(cutoff, CLEANUP_BATCH_SIZE));
return deleted != null ? deleted : 0;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.springframework.transaction.support.TransactionCallback;
import org.springframework.transaction.support.TransactionTemplate;

import java.time.Duration;
import java.time.Instant;
Expand All @@ -16,6 +18,8 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -34,11 +38,20 @@ class SpotCleanupServiceTest {
@Mock
private SpotRepository spotRepository;

@Mock
private TransactionTemplate transactionTemplate;

private SpotCleanupService cleanupService;

@BeforeEach
void setUp() {
cleanupService = new SpotCleanupService(spotRepository, DEFAULT_TTL);
// Configure TransactionTemplate mock to execute the callback directly
lenient().when(transactionTemplate.execute(any()))
.thenAnswer(invocation -> {
TransactionCallback<?> callback = invocation.getArgument(0);
return callback.doInTransaction(null);
});
cleanupService = new SpotCleanupService(spotRepository, transactionTemplate, DEFAULT_TTL);
}

// ===========================================
Expand Down Expand Up @@ -105,7 +118,7 @@ void testExecuteCleanup_UsesTtlForCutoff() {
@Test
void testExecuteCleanup_CustomTtl_UsesConfiguredValue() {
Duration customTtl = Duration.ofHours(12);
SpotCleanupService customService = new SpotCleanupService(spotRepository, customTtl);
SpotCleanupService customService = new SpotCleanupService(spotRepository, transactionTemplate, customTtl);
Instant beforeCall = Instant.now();
when(spotRepository.deleteExpiredSpotsBatch(any(Instant.class), anyInt()))
.thenReturn(0);
Expand Down Expand Up @@ -137,6 +150,18 @@ void testExecuteCleanup_ExactBatchSize_ContinuesUntilPartialBatch() {
verify(spotRepository, times(2)).deleteExpiredSpotsBatch(any(Instant.class), eq(BATCH_SIZE));
}

@Test
void testExecuteCleanup_TransactionReturnsNull_TreatedAsZero() {
// When TransactionTemplate.execute() returns null (e.g., rollback), treat as 0 deleted
// Reset the mock to clear the lenient stub and configure new behavior
reset(transactionTemplate);
when(transactionTemplate.execute(any())).thenReturn(null);

int deleted = cleanupService.executeCleanup();

assertThat(deleted).isZero();
}

// ===========================================
// getTtl tests
// ===========================================
Expand All @@ -149,7 +174,7 @@ void testGetTtl_DefaultTtl_Returns24Hours() {
@Test
void testGetTtl_CustomTtl_ReturnsConfiguredValue() {
Duration customTtl = Duration.ofHours(48);
SpotCleanupService customService = new SpotCleanupService(spotRepository, customTtl);
SpotCleanupService customService = new SpotCleanupService(spotRepository, transactionTemplate, customTtl);

assertThat(customService.getTtl()).isEqualTo(customTtl);
}
Expand All @@ -161,15 +186,15 @@ void testGetTtl_CustomTtl_ReturnsConfiguredValue() {
@Test
void testConstructor_NullTtl_DefaultsTo24Hours() {
// The Spring @Value annotation provides the default, but we test the service works
SpotCleanupService service = new SpotCleanupService(spotRepository, Duration.ofHours(24));
SpotCleanupService service = new SpotCleanupService(spotRepository, transactionTemplate, Duration.ofHours(24));

assertThat(service.getTtl()).isEqualTo(Duration.ofHours(24));
}

@Test
void testConstructor_ZeroTtl_Accepted() {
// Edge case: immediate cleanup
SpotCleanupService service = new SpotCleanupService(spotRepository, Duration.ZERO);
SpotCleanupService service = new SpotCleanupService(spotRepository, transactionTemplate, Duration.ZERO);

assertThat(service.getTtl()).isEqualTo(Duration.ZERO);
}
Expand All @@ -178,7 +203,7 @@ void testConstructor_ZeroTtl_Accepted() {
void testConstructor_NegativeTtl_Accepted() {
// Edge case: would delete future spots (unlikely but valid Duration)
Duration negativeTtl = Duration.ofHours(-1);
SpotCleanupService service = new SpotCleanupService(spotRepository, negativeTtl);
SpotCleanupService service = new SpotCleanupService(spotRepository, transactionTemplate, negativeTtl);

assertThat(service.getTtl()).isEqualTo(negativeTtl);
}
Expand Down