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 @@ -15,6 +15,7 @@
import org.springframework.stereotype.Repository;

import java.util.Optional;
import java.util.Set;

import static konkuk.thip.common.exception.code.ErrorCode.*;

Expand Down Expand Up @@ -86,4 +87,9 @@ public void saveSavedBook(Long userId, Long bookId) {
public void deleteSavedBook(Long userId, Long bookId) {
savedBookJpaRepository.deleteByUserIdAndBookId(userId, bookId);
}

@Override
public void deleteAllByIdInBatch(Set<Long> unusedBookIds) {
bookJpaRepository.deleteAllByIdInBatch(unusedBookIds);
Comment on lines +92 to +93
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

확인했습니다! delete 쿼리를 한번만 날리도록 deleteAllByIdInBatch 메서드를 사용하셨군요!

찾아보니 deleteAllByIdInBatch 메서드는 영속성 컨텍스트를 거치지 않고 바로 database 로 1번의 delete 쿼리를 날려서 영속성 컨텍스트와 db 사이의 동기화가 즉각적으로 되지는 않는다고 하네요! (삭제한 엔티티를 조회할 경우 영속성 컨텍스트에 남아있는 엔티티가 조회된다는 의미)

다만 현재 코드에서는 @async + @transactional 으로 인해 생기는 별도 쓰레드의 트랜잭션 내부에서 사용되지 않는 책을 물리 삭제 이후, 트랜잭션을 commit 함으로써 영속성 컨텍스트를 close 하므로 문제가 되지는 않을 것 같습니다!!

ref : https://wisdom-and-record.tistory.com/139

}
Comment on lines +91 to +94
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

빈 컬렉션 가드 및 대용량 배치 분할 삭제를 추가해 안정성 향상

일부 JPA 구현/DB에서는 IN () 빈 리스트가 SQL 오류를 유발할 수 있습니다. 또한 매우 큰 ID 집합은 파라미터/패킷 한계를 초과할 수 있습니다. 빈 컬렉션 가드와 분할(batch partition) 삭제를 권장합니다.

다음 변경을 메서드 본문(해당 라인 범위)에서 적용해 주세요:

-    public void deleteAllByIdInBatch(Set<Long> unusedBookIds) {
-        bookJpaRepository.deleteAllByIdInBatch(unusedBookIds);
-    }
+    public void deleteAllByIdInBatch(Set<Long> unusedBookIds) {
+        if (unusedBookIds == null || unusedBookIds.isEmpty()) {
+            return;
+        }
+        final int batchSize = 1000; // DB 파라미터 제한/네트워크 패킷 고려
+        if (unusedBookIds.size() <= batchSize) {
+            bookJpaRepository.deleteAllByIdInBatch(unusedBookIds);
+            return;
+        }
+        java.util.List<Long> ids = new java.util.ArrayList<>(unusedBookIds);
+        for (int i = 0; i < ids.size(); i += batchSize) {
+            int end = Math.min(i + batchSize, ids.size());
+            bookJpaRepository.deleteAllByIdInBatch(ids.subList(i, end));
+        }
+    }

메서드 외부(클래스 상단)에는 별도 import가 필요합니다:

import java.util.ArrayList;
import java.util.List;

추가 권장 사항:

  • 서비스 계층에서 트랜잭션 경계를 보장하고, 삭제 중 참조가 새로 생겨 FK 제약 위반이 발생할 수 있는 TOCTOU 상황을 대비해 DataIntegrityViolationException 로깅/스킵 전략을 갖추면 운영 안정성이 올라갑니다.
  • 삭제 대상 개수 및 소요 시간에 대한 로깅/메트릭을 남겨 관찰 가능성을 높여 주세요.
🤖 Prompt for AI Agents
In
src/main/java/konkuk/thip/book/adapter/out/persistence/BookCommandPersistenceAdapter.java
around lines 91-94, the deleteAllByIdInBatch method should guard against an
empty collection and split very large ID sets into smaller batches to avoid SQL
errors and parameter/packet limits; add imports java.util.ArrayList and
java.util.List at the top of the class, return early if unusedBookIds is null or
empty, partition the Set<Long> into fixed-size batches (e.g., 500–1000 ids), and
call bookJpaRepository.deleteAllByIdInBatch for each batch (converting each
batch to a List) inside a try/catch that logs and continues on
DataIntegrityViolationException so one failing batch doesn't abort the whole
operation.

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.springframework.stereotype.Repository;

import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

import static konkuk.thip.common.exception.code.ErrorCode.USER_NOT_FOUND;
Expand Down Expand Up @@ -55,4 +56,9 @@ public List<Book> findJoiningRoomsBooksByUserId(Long userId) {
.map(bookMapper::toDomainEntity)
.collect(Collectors.toList());
}

@Override
public Set<Long> findUnusedBookIds() {
return bookJpaRepository.findUnusedBookIds();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import java.util.List;
import java.util.Optional;
import java.util.Set;

public interface BookJpaRepository extends JpaRepository<BookJpaEntity, Long> {
Optional<BookJpaEntity> findByIsbn(String isbn);
Expand All @@ -28,4 +29,23 @@ public interface BookJpaRepository extends JpaRepository<BookJpaEntity, Long> {
List<BookJpaEntity> findJoiningRoomsBooksByUserId(Long userId);

boolean existsByIsbn(String isbn);

// Room, Feed, SavedBook에 모두 참조되지 않은 책 ID만 찾는 쿼리
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋네요

@Query(
"SELECT b.bookId " +
"FROM BookJpaEntity b " +
"WHERE NOT EXISTS ( " +
" SELECT 1 FROM RoomJpaEntity r " +
" WHERE r.bookJpaEntity = b " +
") " +
"AND NOT EXISTS ( " +
" SELECT 1 FROM FeedJpaEntity f " +
" WHERE f.bookJpaEntity = b " +
") " +
"AND NOT EXISTS ( " +
" SELECT 1 FROM SavedBookJpaEntity s " +
" WHERE s.bookJpaEntity = b " +
")"
)
Set<Long> findUnusedBookIds();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package konkuk.thip.book.application.port.in;

public interface BookCleanUpUseCase {
void deleteUnusedBooks();
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import konkuk.thip.common.exception.code.ErrorCode;

import java.util.Optional;
import java.util.Set;

public interface BookCommandPort {

Expand All @@ -27,4 +28,5 @@ default Book getByIsbnOrThrow(String isbn){
void saveSavedBook(Long userId, Long bookId);
void deleteSavedBook(Long userId, Long bookId);

void deleteAllByIdInBatch(Set<Long> unusedBookIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import konkuk.thip.book.domain.Book;

import java.util.List;
import java.util.Set;

public interface BookQueryPort {

Expand All @@ -13,4 +14,6 @@ public interface BookQueryPort {
List<Book> findSavedBooksByUserId(Long userId);

List<Book> findJoiningRoomsBooksByUserId(Long userId);

Set<Long> findUnusedBookIds();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package konkuk.thip.book.application.service;

import konkuk.thip.book.application.port.in.BookCleanUpUseCase;
import konkuk.thip.book.application.port.out.BookCommandPort;
import konkuk.thip.book.application.port.out.BookQueryPort;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.Set;

@Slf4j
@Service
@RequiredArgsConstructor
public class BookCleanUpService implements BookCleanUpUseCase {

private final BookCommandPort bookCommandPort;
private final BookQueryPort bookQueryPort;

@Async
@Override
@Transactional
public void deleteUnusedBooks() {
Set<Long> unusedBookIds = bookQueryPort.findUnusedBookIds();
log.info("삭제할 사용되지 않는 Book IDs: {}", unusedBookIds);
bookCommandPort.deleteAllByIdInBatch(unusedBookIds);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package konkuk.thip.common.scheduler;

import konkuk.thip.book.application.port.in.BookCleanUpUseCase;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;

@Slf4j
@Component
@RequiredArgsConstructor
public class BookDeleteScheduler {

private final BookCleanUpUseCase bookCleanUpUseCase;

// 매일 새벽 4시 실행
@Scheduled(cron = "0 0 4 * * *", zone = "Asia/Seoul")
public void cleanUpUnusedBooks() {
log.info("[스케줄러] 사용되지 않는 Book 데이터 삭제 시작");
bookCleanUpUseCase.deleteUnusedBooks();
log.info("[스케줄러] 사용되지 않는 Book 데이터 삭제 완료");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package konkuk.thip.common.scheduler;

import konkuk.thip.book.adapter.out.jpa.BookJpaEntity;
import konkuk.thip.book.adapter.out.persistence.repository.BookJpaRepository;
import konkuk.thip.book.adapter.out.persistence.repository.SavedBookJpaRepository;
import konkuk.thip.book.application.port.in.BookCleanUpUseCase;
import konkuk.thip.common.util.TestEntityFactory;
import konkuk.thip.feed.adapter.out.persistence.repository.FeedJpaRepository;
import konkuk.thip.room.adapter.out.persistence.repository.RoomJpaRepository;
import konkuk.thip.user.adapter.out.jpa.UserJpaEntity;
import konkuk.thip.user.adapter.out.persistence.repository.UserJpaRepository;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;

@SpringBootTest
@ActiveProfiles("test")
@Transactional
@DisplayName("[통합] Book 삭제 스케줄러 기능 테스트")
class BookDeleteSchedulerTest {

@Autowired
private BookJpaRepository bookJpaRepository;

@Autowired
private RoomJpaRepository roomJpaRepository;

@Autowired
private FeedJpaRepository feedJpaRepository;

@Autowired
private SavedBookJpaRepository savedBookJpaRepository;

@Autowired
private UserJpaRepository userJpaRepository;

@Autowired
private BookCleanUpUseCase bookCleanUpUseCase;

@Test
@DisplayName("Room, Feed, SavedBook 어디에도 연결되지 않은 Book은 삭제된다")
void deleteUnusedBooks_success() {
// given
// 사용되지 않는 Book
BookJpaEntity unusedBook = bookJpaRepository.save(TestEntityFactory.createBookWithBookTitle("고아책"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ㅋㅋㅋㅋㅋ

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? ㅋㅋ


// Room에 연결된 Book
BookJpaEntity roomBook = bookJpaRepository.save(TestEntityFactory.createBookWithBookTitle("방책"));
roomJpaRepository.save(TestEntityFactory.createRoom(roomBook, TestEntityFactory.createLiteratureCategory()));

// Feed에 연결된 Book
BookJpaEntity feedBook = bookJpaRepository.save(TestEntityFactory.createBookWithBookTitle("피드책"));
UserJpaEntity feedUser = userJpaRepository.save(TestEntityFactory.createUser(TestEntityFactory.createLiteratureAlias()));
feedJpaRepository.save(TestEntityFactory.createFeed(feedUser, feedBook, true));

// SavedBook에 연결된 Book
BookJpaEntity savedBook = bookJpaRepository.save(TestEntityFactory.createBookWithBookTitle("저장책"));
UserJpaEntity savedUser = userJpaRepository.save(TestEntityFactory.createUser(TestEntityFactory.createLiteratureAlias(), "저장유저"));
savedBookJpaRepository.save(TestEntityFactory.createSavedBook(savedUser, savedBook));

// when
bookCleanUpUseCase.deleteUnusedBooks();

// then
List<BookJpaEntity> remainingBooks = bookJpaRepository.findAll();

// 삭제되지 않은 책의 제목만 수집
List<String> remainingTitles = remainingBooks.stream().map(BookJpaEntity::getTitle).toList();

assertThat(remainingTitles).contains("방책", "피드책", "저장책");
assertThat(remainingTitles).doesNotContain("고아책");
}
}
18 changes: 18 additions & 0 deletions src/test/java/konkuk/thip/config/TestAsyncConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package konkuk.thip.config;

import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Profile;
import org.springframework.core.task.SyncTaskExecutor;

import java.util.concurrent.Executor;

@Configuration
@Profile("test")
public class TestAsyncConfig {
@Bean(name = "taskExecutor")
public Executor taskExecutor() {
return new SyncTaskExecutor(); // 동기 실행
}
}