diff --git a/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java b/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java index 520b1efd..d2ca11c8 100644 --- a/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java +++ b/commafeed-server/src/main/java/com/commafeed/CommaFeedConfiguration.java @@ -312,6 +312,12 @@ public interface CommaFeedConfiguration { @WithDefault("100") int batchSize(); + /** + * Whether to keep starred entries when cleaning up old entries. + */ + @WithDefault("true") + boolean keepStarredEntries(); + default Instant statusesInstantThreshold() { return statusesMaxAge().toMillis() > 0 ? Instant.now().minus(statusesMaxAge()) : null; } diff --git a/commafeed-server/src/main/java/com/commafeed/backend/dao/FeedEntryDAO.java b/commafeed-server/src/main/java/com/commafeed/backend/dao/FeedEntryDAO.java index 5b17c079..61b33121 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/dao/FeedEntryDAO.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/dao/FeedEntryDAO.java @@ -11,6 +11,7 @@ import com.commafeed.backend.model.FeedEntry; import com.commafeed.backend.model.QFeedEntry; import com.querydsl.core.Tuple; import com.querydsl.core.types.dsl.NumberExpression; +import com.querydsl.jpa.impl.JPAQuery; @Singleton public class FeedEntryDAO extends GenericDAO { @@ -25,15 +26,21 @@ public class FeedEntryDAO extends GenericDAO { return query().select(ENTRY).from(ENTRY).where(ENTRY.guidHash.eq(guidHash), ENTRY.feed.eq(feed)).limit(1).fetchOne(); } - public List findFeedsExceedingCapacity(long maxCapacity, long max) { + public List findFeedsExceedingCapacity(long maxCapacity, long max, boolean keepStarredEntries) { NumberExpression count = ENTRY.id.count(); - List tuples = query().select(ENTRY.feed.id, count) - .from(ENTRY) - .groupBy(ENTRY.feed) + JPAQuery query = query().select(ENTRY.feed.id, count).from(ENTRY); + + if (keepStarredEntries) { + query.where(Predicates.isNotStarred(ENTRY)); + } + + return query.groupBy(ENTRY.feed) .having(count.gt(maxCapacity)) .limit(max) - .fetch(); - return tuples.stream().map(t -> new FeedCapacity(t.get(ENTRY.feed.id), t.get(count))).toList(); + .fetch() + .stream() + .map(t -> new FeedCapacity(t.get(ENTRY.feed.id), t.get(count))) + .toList(); } public int delete(Long feedId, long max) { @@ -44,21 +51,30 @@ public class FeedEntryDAO extends GenericDAO { /** * Delete entries older than a certain date */ - public int deleteEntriesOlderThan(Instant olderThan, long max) { - List list = query().selectFrom(ENTRY) + public int deleteEntriesOlderThan(Instant olderThan, long max, boolean keepStarredEntries) { + JPAQuery query = query().selectFrom(ENTRY) .where(ENTRY.published.lt(olderThan)) .orderBy(ENTRY.published.asc()) - .limit(max) - .fetch(); - return delete(list); + .limit(max); + + if (keepStarredEntries) { + query.where(Predicates.isNotStarred(ENTRY)); + } + + return delete(query.fetch()); } /** * Delete the oldest entries of a feed */ - public int deleteOldEntries(Long feedId, long max) { - List list = query().selectFrom(ENTRY).where(ENTRY.feed.id.eq(feedId)).orderBy(ENTRY.published.asc()).limit(max).fetch(); - return delete(list); + public int deleteOldEntries(Long feedId, long max, boolean keepStarredEntries) { + JPAQuery query = query().selectFrom(ENTRY).where(ENTRY.feed.id.eq(feedId)).orderBy(ENTRY.published.asc()).limit(max); + + if (keepStarredEntries) { + query.where(Predicates.isNotStarred(ENTRY)); + } + + return delete(query.fetch()); } public record FeedCapacity(Long id, Long capacity) { diff --git a/commafeed-server/src/main/java/com/commafeed/backend/dao/Predicates.java b/commafeed-server/src/main/java/com/commafeed/backend/dao/Predicates.java new file mode 100644 index 00000000..817dd3de --- /dev/null +++ b/commafeed-server/src/main/java/com/commafeed/backend/dao/Predicates.java @@ -0,0 +1,18 @@ +package com.commafeed.backend.dao; + +import com.commafeed.backend.model.QFeedEntry; +import com.commafeed.backend.model.QFeedEntryStatus; +import com.querydsl.core.types.dsl.BooleanExpression; +import com.querydsl.jpa.JPAExpressions; + +import lombok.experimental.UtilityClass; + +@UtilityClass +public class Predicates { + + private static final QFeedEntryStatus STATUS = QFeedEntryStatus.feedEntryStatus; + + public static BooleanExpression isNotStarred(QFeedEntry entry) { + return JPAExpressions.selectOne().from(STATUS).where(STATUS.entry.eq(entry).and(STATUS.starred.isTrue())).notExists(); + } +} diff --git a/commafeed-server/src/main/java/com/commafeed/backend/service/db/DatabaseCleaningService.java b/commafeed-server/src/main/java/com/commafeed/backend/service/db/DatabaseCleaningService.java index 564d1d96..a4e21891 100644 --- a/commafeed-server/src/main/java/com/commafeed/backend/service/db/DatabaseCleaningService.java +++ b/commafeed-server/src/main/java/com/commafeed/backend/service/db/DatabaseCleaningService.java @@ -27,13 +27,13 @@ import lombok.extern.slf4j.Slf4j; @Singleton public class DatabaseCleaningService { - private final int batchSize; - private final UnitOfWork unitOfWork; private final FeedDAO feedDAO; private final FeedEntryDAO feedEntryDAO; private final FeedEntryContentDAO feedEntryContentDAO; private final FeedEntryStatusDAO feedEntryStatusDAO; + private final int batchSize; + private final boolean keepStarredEntries; private final Meter entriesDeletedMeter; public DatabaseCleaningService(CommaFeedConfiguration config, UnitOfWork unitOfWork, FeedDAO feedDAO, FeedEntryDAO feedEntryDAO, @@ -44,6 +44,7 @@ public class DatabaseCleaningService { this.feedEntryContentDAO = feedEntryContentDAO; this.feedEntryStatusDAO = feedEntryStatusDAO; this.batchSize = config.database().cleanup().batchSize(); + this.keepStarredEntries = config.database().cleanup().keepStarredEntries(); this.entriesDeletedMeter = metrics.meter(MetricRegistry.name(getClass(), "entriesDeleted")); } @@ -86,21 +87,23 @@ public class DatabaseCleaningService { log.info("cleaning entries exceeding feed capacity"); long total = 0; while (true) { - List feeds = unitOfWork.call(() -> feedEntryDAO.findFeedsExceedingCapacity(maxFeedCapacity, batchSize)); + List feeds = unitOfWork + .call(() -> feedEntryDAO.findFeedsExceedingCapacity(maxFeedCapacity, batchSize, keepStarredEntries)); if (feeds.isEmpty()) { break; } for (final FeedCapacity feed : feeds) { long remaining = feed.capacity() - maxFeedCapacity; + int deleted; do { final long rem = remaining; - int deleted = unitOfWork.call(() -> feedEntryDAO.deleteOldEntries(feed.id(), Math.min(batchSize, rem))); + deleted = unitOfWork.call(() -> feedEntryDAO.deleteOldEntries(feed.id(), Math.min(batchSize, rem), keepStarredEntries)); entriesDeletedMeter.mark(deleted); total += deleted; remaining -= deleted; log.debug("removed {} entries for feeds exceeding capacity", total); - } while (remaining > 0); + } while (deleted > 0 && remaining > 0); } } log.info("cleanup done: {} entries for feeds exceeding capacity deleted", total); @@ -111,7 +114,7 @@ public class DatabaseCleaningService { long total = 0; long deleted; do { - deleted = unitOfWork.call(() -> feedEntryDAO.deleteEntriesOlderThan(olderThan, batchSize)); + deleted = unitOfWork.call(() -> feedEntryDAO.deleteEntriesOlderThan(olderThan, batchSize, keepStarredEntries)); entriesDeletedMeter.mark(deleted); total += deleted; log.debug("removed {} old entries", total); diff --git a/commafeed-server/src/test/java/com/commafeed/backend/service/db/DatabaseCleaningServiceTest.java b/commafeed-server/src/test/java/com/commafeed/backend/service/db/DatabaseCleaningServiceTest.java index 91f31441..a1a69fa9 100644 --- a/commafeed-server/src/test/java/com/commafeed/backend/service/db/DatabaseCleaningServiceTest.java +++ b/commafeed-server/src/test/java/com/commafeed/backend/service/db/DatabaseCleaningServiceTest.java @@ -115,13 +115,13 @@ class DatabaseCleaningServiceTest { Mockito.when(feed2.id()).thenReturn(2L); Mockito.when(feed2.capacity()).thenReturn(120L); - Mockito.when(feedEntryDAO.findFeedsExceedingCapacity(50, BATCH_SIZE)) + Mockito.when(feedEntryDAO.findFeedsExceedingCapacity(50, BATCH_SIZE, false)) .thenReturn(Arrays.asList(feed1, feed2)) .thenReturn(Collections.emptyList()); - Mockito.when(feedEntryDAO.deleteOldEntries(1L, 100)).thenReturn(80); - Mockito.when(feedEntryDAO.deleteOldEntries(1L, 50)).thenReturn(50); - Mockito.when(feedEntryDAO.deleteOldEntries(2L, 70)).thenReturn(70); + Mockito.when(feedEntryDAO.deleteOldEntries(1L, 100, false)).thenReturn(80); + Mockito.when(feedEntryDAO.deleteOldEntries(1L, 50, false)).thenReturn(50); + Mockito.when(feedEntryDAO.deleteOldEntries(2L, 70, false)).thenReturn(70); service.cleanEntriesForFeedsExceedingCapacity(50); @@ -132,11 +132,11 @@ class DatabaseCleaningServiceTest { void cleanEntriesOlderThanDeletesOldEntries() { Instant cutoff = LocalDate.now().minusDays(30).atStartOfDay().toInstant(ZoneOffset.UTC); - Mockito.when(feedEntryDAO.deleteEntriesOlderThan(cutoff, BATCH_SIZE)).thenReturn(100, 50, 0); + Mockito.when(feedEntryDAO.deleteEntriesOlderThan(cutoff, BATCH_SIZE, false)).thenReturn(100, 50, 0); service.cleanEntriesOlderThan(cutoff); - Mockito.verify(feedEntryDAO, Mockito.times(3)).deleteEntriesOlderThan(cutoff, BATCH_SIZE); + Mockito.verify(feedEntryDAO, Mockito.times(3)).deleteEntriesOlderThan(cutoff, BATCH_SIZE, false); Mockito.verify(entriesDeletedMeter, Mockito.times(3)).mark(Mockito.anyLong()); } diff --git a/commafeed-server/src/test/java/com/commafeed/integration/cleanup/DatabaseCleaningIT.java b/commafeed-server/src/test/java/com/commafeed/integration/cleanup/DatabaseCleaningIT.java new file mode 100644 index 00000000..9ab7db17 --- /dev/null +++ b/commafeed-server/src/test/java/com/commafeed/integration/cleanup/DatabaseCleaningIT.java @@ -0,0 +1,185 @@ +package com.commafeed.integration.cleanup; + +import java.time.Duration; +import java.time.Instant; + +import jakarta.inject.Inject; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import com.commafeed.TestConstants; +import com.commafeed.backend.service.db.DatabaseCleaningService; +import com.commafeed.frontend.model.Entries; +import com.commafeed.frontend.model.Entry; +import com.commafeed.frontend.model.request.StarRequest; +import com.commafeed.frontend.resource.CategoryREST; +import com.commafeed.integration.BaseIT; + +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.RestAssured; +import io.restassured.http.ContentType; + +@QuarkusTest +class DatabaseCleaningIT extends BaseIT { + + @Inject + DatabaseCleaningService databaseCleaningService; + + @BeforeEach + void setup() { + initialSetup(TestConstants.ADMIN_USERNAME, TestConstants.ADMIN_PASSWORD); + RestAssured.authentication = RestAssured.preemptive().basic(TestConstants.ADMIN_USERNAME, TestConstants.ADMIN_PASSWORD); + } + + @AfterEach + void cleanup() { + RestAssured.reset(); + } + + private void starEntry(String entryId, Long subscriptionId) { + StarRequest starRequest = new StarRequest(); + starRequest.setId(entryId); + starRequest.setFeedId(subscriptionId); + starRequest.setStarred(true); + RestAssured.given().body(starRequest).contentType(ContentType.JSON).post("rest/entry/star").then().statusCode(200); + } + + private void unstarEntry(String entryId, Long subscriptionId) { + StarRequest starRequest = new StarRequest(); + starRequest.setId(entryId); + starRequest.setFeedId(subscriptionId); + starRequest.setStarred(false); + RestAssured.given().body(starRequest).contentType(ContentType.JSON).post("rest/entry/star").then().statusCode(200); + } + + @Nested + class KeepStarredEntries { + + @Test + void starredEntriesAreKeptWhenCleaningFeedsExceedingCapacity() { + // Subscribe to feed and wait for entries + Long subscriptionId = subscribeAndWaitForEntries(getFeedUrl()); + + // Verify we have 2 entries + Entries entriesBefore = getFeedEntries(subscriptionId); + Assertions.assertEquals(2, entriesBefore.getEntries().size()); + + // Star the first entry + Entry entryToStar = entriesBefore.getEntries().getFirst(); + starEntry(entryToStar.getId(), subscriptionId); + + // Verify the entry is starred + Entries starredEntries = getCategoryEntries(CategoryREST.STARRED); + Assertions.assertEquals(1, starredEntries.getEntries().size()); + Assertions.assertEquals(entryToStar.getId(), starredEntries.getEntries().getFirst().getId()); + + // Run cleanup with capacity of 0 (should delete all non-starred entries) + // With keepStarredEntries=true (default), only non-starred entries are counted for capacity. + // We have 2 entries, 1 starred and 1 non-starred. With capacity=0, the 1 non-starred entry exceeds capacity. + databaseCleaningService.cleanEntriesForFeedsExceedingCapacity(0); + + // Verify starred entry is still present + Entries starredEntriesAfter = getCategoryEntries(CategoryREST.STARRED); + Assertions.assertEquals(1, starredEntriesAfter.getEntries().size()); + Assertions.assertEquals(entryToStar.getId(), starredEntriesAfter.getEntries().getFirst().getId()); + + // Verify the non-starred entry was deleted (only starred entry should remain) + Entries entriesAfter = getFeedEntries(subscriptionId); + Assertions.assertEquals(1, entriesAfter.getEntries().size()); + Assertions.assertEquals(entryToStar.getId(), entriesAfter.getEntries().getFirst().getId()); + } + + @Test + void starredEntriesAreKeptWhenCleaningOldEntries() { + // Subscribe to feed and wait for entries + Long subscriptionId = subscribeAndWaitForEntries(getFeedUrl()); + + // Verify we have 2 entries + Entries entriesBefore = getFeedEntries(subscriptionId); + Assertions.assertEquals(2, entriesBefore.getEntries().size()); + + // Star the first entry (oldest one based on published date in rss.xml) + Entry entryToStar = entriesBefore.getEntries().getFirst(); + starEntry(entryToStar.getId(), subscriptionId); + + // Verify the entry is starred + Entries starredEntries = getCategoryEntries(CategoryREST.STARRED); + Assertions.assertEquals(1, starredEntries.getEntries().size()); + + // Run cleanup for entries older than now (should try to delete all entries) + // With keepStarredEntries=true (default), the starred entry should be preserved + Instant olderThan = Instant.now().plus(Duration.ofDays(1)); + databaseCleaningService.cleanEntriesOlderThan(olderThan); + + // Verify starred entry is still present + Entries starredEntriesAfter = getCategoryEntries(CategoryREST.STARRED); + Assertions.assertEquals(1, starredEntriesAfter.getEntries().size()); + Assertions.assertEquals(entryToStar.getId(), starredEntriesAfter.getEntries().getFirst().getId()); + + // Verify the non-starred entry was deleted + Entries entriesAfter = getFeedEntries(subscriptionId); + Assertions.assertEquals(1, entriesAfter.getEntries().size()); + Assertions.assertEquals(entryToStar.getId(), entriesAfter.getEntries().getFirst().getId()); + } + + @Test + void multipleStarredEntriesAreAllKept() { + // Subscribe to feed and wait for entries + Long subscriptionId = subscribeAndWaitForEntries(getFeedUrl()); + + // Verify we have 2 entries + Entries entriesBefore = getFeedEntries(subscriptionId); + Assertions.assertEquals(2, entriesBefore.getEntries().size()); + + // Star both entries + entriesBefore.getEntries().forEach(entry -> starEntry(entry.getId(), subscriptionId)); + + // Verify both entries are starred + Entries starredEntries = getCategoryEntries(CategoryREST.STARRED); + Assertions.assertEquals(2, starredEntries.getEntries().size()); + + // Run cleanup with capacity of 0 (should delete all non-starred entries) + databaseCleaningService.cleanEntriesForFeedsExceedingCapacity(0); + + // Verify both starred entries are still present + Entries starredEntriesAfter = getCategoryEntries(CategoryREST.STARRED); + Assertions.assertEquals(2, starredEntriesAfter.getEntries().size()); + + // Verify all entries are preserved (since all are starred) + Entries entriesAfter = getFeedEntries(subscriptionId); + Assertions.assertEquals(2, entriesAfter.getEntries().size()); + } + + @Test + void unstarringEntryMakesItEligibleForCleanup() { + // Subscribe to feed and wait for entries + Long subscriptionId = subscribeAndWaitForEntries(getFeedUrl()); + + // Star the first entry + Entries entriesBefore = getFeedEntries(subscriptionId); + Entry entry = entriesBefore.getEntries().getFirst(); + starEntry(entry.getId(), subscriptionId); + + // Verify entry is starred + Assertions.assertEquals(1, getCategoryEntries(CategoryREST.STARRED).getEntries().size()); + + // Unstar the entry + unstarEntry(entry.getId(), subscriptionId); + + // Verify entry is no longer starred + Assertions.assertEquals(0, getCategoryEntries(CategoryREST.STARRED).getEntries().size()); + + // Run cleanup for entries older than now + Instant olderThan = Instant.now().plus(Duration.ofDays(1)); + databaseCleaningService.cleanEntriesOlderThan(olderThan); + + // Verify both entries were deleted (neither is starred) + Entries entriesAfter = getFeedEntries(subscriptionId); + Assertions.assertEquals(0, entriesAfter.getEntries().size()); + } + } +}