[FEAT] 홈 화면 서재/포커스 통계 API 구현 및 캐싱#224
Merged
JiwonLee42 merged 16 commits intodevelop-demofrom Feb 26, 2026
Merged
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughRedis 기반 캐싱 도입과 서재 월별 포커스 통계 API(컨트롤러·서비스·리포지토리·DTO) 추가, 라이브러리 커스텀 리포지토리 제거, 캐시 무효화 이벤트·리스너 및 관련 테스트·문서·빌드 설정이 추가/수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as "LibraryStatsController"
participant Service as "LibraryStatsService"
participant Repo as "FocusRepository"
participant Cache as "RedisCacheManager"
participant EventPub as "ApplicationEventPublisher"
participant Listener as "LibraryCacheInvalidationListener"
Client->>Controller: GET /api/library/stats/monthly?yearMonth=YYYY-MM
Controller->>Service: viewMonthly(userId, yearMonth)
Service->>Cache: lookup key (userId:yearMonth)
alt cache hit
Cache-->>Service: cached DTO
Service-->>Controller: DTO
else cache miss
Service->>Repo: findMonthlyFocusStats(userId, start, end)
Repo-->>Service: projection rows
Service->>Service: aggregate -> DTO
Service->>Cache: put key = DTO
Service-->>Controller: DTO
end
Note over Service,EventPub: on save/delete/update
Service->>EventPub: publish LibraryCacheInvalidateEvent(userId, affectedYearMonths, evictStatus)
EventPub-->>Listener: deliver afterCommit
Listener->>Cache: evict keys (libraryMonthlyCurrent, focusMonthlyCurrent, libraryStatusFirstPage)
Cache-->>Listener: evicted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/app/nook/library/domain/Library.java (1)
60-76:⚠️ Potential issue | 🔴 Critical
focusMin이null인 기존 DB 레코드에서 NPE가 발생합니다.
focusMin필드는Long(래퍼 타입)으로 선언되어 있고= 0L초기화는 새 인스턴스에만 적용됩니다. 기존 DB 레코드에NULL이 저장되어 있으면, JPA로 로딩 후recordFocus()호출 시this.focusMin += addedMinutes가null + long연산을 시도해NullPointerException이 발생합니다.🐛 NPE 방어 처리 제안
public void recordFocus(long addedMinutes) { - this.focusMin += addedMinutes; + if (this.focusMin == null) { + this.focusMin = 0L; + } + this.focusMin += addedMinutes; }또는 DB 컬럼에
NOT NULL DEFAULT 0제약을 추가하고 기존 데이터를 backfill하는 방법도 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/domain/Library.java` around lines 60 - 76, The field focusMin (Long) can be null for existing DB rows causing NPE in recordFocus; update recordFocus in class Library to defensively handle null by initializing focusMin to 0L when null (e.g., if (this.focusMin == null) this.focusMin = 0L;) before performing the addition, or alternatively change focusMin to the primitive long or annotate the field with non-null constraints and backfill DB—ensure the fix references the focusMin field and the recordFocus(long addedMinutes) method.
🧹 Nitpick comments (11)
src/main/java/app/nook/NookApplication.java (1)
9-11:@EnableCaching을CacheConfig로 이동하는 것을 고려하세요.기능적으로는 문제없으나, 이미
CacheConfig클래스가 존재하므로 캐싱 관련 설정을 해당 클래스에서 일원화하면 관심사 분리가 더 명확해집니다.♻️ 리팩터 제안
NookApplication.java에서 제거:-import org.springframework.cache.annotation.EnableCaching; `@SpringBootApplication` `@EnableJpaAuditing` -@EnableCaching public class NookApplication { ... }
CacheConfig.java에 추가:+import org.springframework.cache.annotation.EnableCaching; `@Configuration` +@EnableCaching public class CacheConfig { ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/NookApplication.java` around lines 9 - 11, Remove the `@EnableCaching` annotation from the NookApplication class and move it into the existing CacheConfig class so caching configuration is centralized; specifically, ensure CacheConfig is annotated with `@Configuration` (if not already) and add `@EnableCaching` to CacheConfig, and then delete `@EnableCaching` from NookApplication to keep caching concerns inside CacheConfig.build.gradle (1)
3-3: SNAPSHOT 버전 대신 GA 릴리즈 버전을 사용하세요.Spring Boot 3.5.10 GA가 2026년 1월 22일에 Maven Central에 릴리즈되었습니다. SNAPSHOT 버전은 빌드 재현성을 보장하지 않으며, 원격 스냅샷 리포지토리 없이는 CI/CD 환경에서 resolve에 실패할 수 있습니다.
🔧 버전 교체 제안
- id 'org.springframework.boot' version '3.5.10-SNAPSHOT' + id 'org.springframework.boot' version '3.5.10'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle` at line 3, Replace the SNAPSHOT Spring Boot version with the GA release by changing the version string used in the Gradle plugin declaration (the line containing "id 'org.springframework.boot' version '3.5.10-SNAPSHOT'") to the stable release "3.5.10"; also search the repository for other occurrences of "3.5.10-SNAPSHOT" (build.gradle, settings.gradle, pluginManagement blocks, or CI config) and update them to "3.5.10" so builds resolve deterministically in CI/CD.src/main/java/app/nook/library/domain/Library.java (1)
63-71: 생성자 내LocalDateTime.now()호출은 테스트 고정 날짜 지정을 어렵게 합니다.
startedAt을 생성자 내부에서now()로 고정하면 단위 테스트에서 날짜를 제어하기 어렵습니다.Clock을 주입하거나, 상위 레이어(서비스)에서 날짜를 전달받는 방식을 고려할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/domain/Library.java` around lines 63 - 71, The Library constructor currently sets startedAt via LocalDateTime.now() which makes tests non-deterministic; change the constructor to accept either a LocalDate (startedAt) or a java.time.Clock and assign startedAt with LocalDate.now(clock) (or directly from the passed LocalDate), update the `@Builder` usage so callers/tests can supply a deterministic value (e.g., a fixed Clock or a test LocalDate), and modify call sites/tests to pass the clock/startedAt instead of relying on the constructor to call now().src/main/java/app/nook/focus/domain/Focus.java (1)
17-20: 현재 구성에서는 인덱스 변경이 자동으로 적용되지만, 명시적 마이그레이션 스크립트 추가를 권장합니다.
application.yml에서ddl-auto의 기본값이update로 설정되어 있으므로 Hibernate가@Index어노테이션 기반으로 자동으로 인덱스를 생성합니다. 따라서 현재 구성에서는 인덱스 변경(idx_focus_library_started, 컬럼:library_id, started_at)이 자동 적용됩니다.다만 프로덕션 환경에서
JPA_DDL_AUTO환경변수를validate또는none으로 설정하는 경우를 대비하여, Flyway나 Liquibase를 통한 명시적 마이그레이션 스크립트를 추가하는 것이 권장되는 모범 사례입니다. 현재 마이그레이션 스크립트가 없으므로, 다음과 같은 마이그레이션을 추가하는 것을 검토해주세요:예시 마이그레이션 스크립트
DROP INDEX IF EXISTS idx_focus_library; CREATE INDEX idx_focus_library_started ON focuses (library_id, started_at);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/focus/domain/Focus.java` around lines 17 - 20, The entity Focus was annotated with a new index named idx_focus_library_started (columns library_id, started_at) but no explicit DB migration was added; add a Flyway/Liquibase migration that ensures the index exists in production (e.g., DROP INDEX IF EXISTS idx_focus_library; CREATE INDEX idx_focus_library_started ON focuses (library_id, started_at)) so environments with JPA_DDL_AUTO=validate/none get the change; implement the migration using your project’s migration tool and include it alongside other migrations so the Focus index is created reliably in production.src/main/java/app/nook/library/service/LibraryFocusUtil.java (1)
5-24: 로직이 깔끔합니다. 유틸리티 클래스에 private 생성자 추가를 고려하세요.정적 유틸리티 클래스이므로 인스턴스화를 방지하기 위해 private 생성자를 추가하는 것이 관례입니다.
♻️ private 생성자 추가
public class LibraryFocusUtil { + + private LibraryFocusUtil() {} // 하루에 포커스 시간 계산, 한달 중 가장 많이 포커스한 날짜 기준으로 나머지를 계산 public static FocusTimeSlot toFocusTimeSlot(long daySec, long maxDaySec) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryFocusUtil.java` around lines 5 - 24, Add a private no-arg constructor to the LibraryFocusUtil class to prevent instantiation of this static utility class: create a private LibraryFocusUtil() { throw new AssertionError("no instances"); } (or an empty private constructor) alongside the existing toFocusTimeSlot method so the class cannot be instantiated by callers or reflection attempts.src/test/java/app/nook/library/service/LibraryStatsServiceTest.java (1)
22-28:viewFocusTimeStats메서드에 대한 테스트도 추가를 고려하세요현재
viewMonthly만 테스트되고 있습니다. AI 요약에 따르면LibraryStatsService에는viewFocusTimeStats메서드도 있으므로, 해당 메서드의 테스트 케이스도 함께 추가하면 커버리지가 향상됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/library/service/LibraryStatsServiceTest.java` around lines 22 - 28, Add a unit test for LibraryStatsService.viewFocusTimeStats in LibraryStatsServiceTest: mock FocusRepository to return controlled focus entries, call libraryStatsService.viewFocusTimeStats(...) with appropriate inputs, and assert the returned DTO/values match expected aggregates; use the existing `@Mock` focusRepository and `@InjectMocks` libraryStatsService to set up the scenario and verify edge cases (empty data, single entry) to improve coverage.src/main/java/app/nook/library/dto/FocusRankDto.java (1)
49-55:MonthlyFocusRow는 내부 매핑 전용 DTO — 가시성 제한 고려
MonthlyFocusRow와FocusTimeRow는 서비스 레이어에서 projection → DTO 매핑 시 사용되는 중간 객체입니다. API 응답에 노출될 필요가 없으므로, 향후 public record가 아닌 package-private 또는 서비스 내부 정의로 이동하는 것을 고려해보세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/dto/FocusRankDto.java` around lines 49 - 55, MonthlyFocusRow (and similarly FocusTimeRow) are internal projection-to-DTO intermediates and should not be public; remove the public modifier from the record declarations (or relocate them as package-private types inside the service/class that performs the mapping) so they are package-private or defined as private static nested records within the service class that builds FocusRankDto; update references in the mapping code to the new location/visibility to ensure compile-time references still resolve.src/main/java/app/nook/global/config/CacheConfig.java (1)
25-37: 캐시 이름을 상수로 관리하세요캐시 이름 문자열(
"libraryMonthlyCurrent","libraryMonthlyPast"등)이 여기와 서비스의@Cacheable어노테이션에서 각각 하드코딩됩니다. 오타로 인한 캐시 미적용을 방지하기 위해 상수 클래스로 추출하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/global/config/CacheConfig.java` around lines 25 - 37, The cache name strings in CacheConfig (cacheConfigs map entries for "libraryMonthlyCurrent", "libraryMonthlyPast", "focusMonthlyCurrent", "libraryStatusFirstPage") are hard-coded and must be centralized: extract these names into a single constant holder (e.g., a CacheNames class or enum) and replace the literal strings in CacheConfig (and all usages annotated with `@Cacheable`) with those constants; update references in methods that rely on these keys so both the cache registration (cacheConfigs in CacheConfig) and `@Cacheable` annotations use the shared constants to prevent typos and mismatches.src/main/java/app/nook/focus/repository/FocusRepository.java (1)
31-49:FUNCTION('DATE', f.startedAt)대신 Hibernate의 표준CAST()함수 사용 권장현재 코드는 PostgreSQL에서 정상 작동합니다. PostgreSQL은
date(timestamp)변환을 지원하므로 Hibernate의FUNCTION('DATE', ...)호출은 호환됩니다.다만 Hibernate 모범 사례에 따르면, 데이터베이스 간 이식성을 위해
CAST(f.startedAt AS date)를 사용하는 것이 권장됩니다. 현재 코드가FUNCTION('DATE')와YEAR()/MONTH()를 혼용하고 있으므로, 일관성 있게CAST(f.startedAt AS date)로 통일하면 더 명확하고 표준화된 JPQL 코드가 될 것입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/focus/repository/FocusRepository.java` around lines 31 - 49, The query in findMonthlyFocusStats uses the nonstandard FUNCTION('DATE', f.startedAt); replace that with the standard JPQL cast syntax CAST(f.startedAt AS date) everywhere it appears (select and group by) to make the JPQL portable across databases; update the select alias (dateValue) and the GROUP BY expression to use CAST(f.startedAt AS date) while keeping f.library.book.id, f.library.book.coverImageUrl, sum(f.durationSec) and the ORDER BY unchanged so MonthlyFocusStatsProjection still maps correctly.src/main/java/app/nook/library/service/LibraryStatsService.java (1)
33-36: 월 경계 계산 로직은 헬퍼로 추출하면 중복을 줄일 수 있습니다.같은 월 시작/종료 계산이 두 메서드에 반복되어 있어, 공용 메서드로 빼두면 이후 수정 포인트를 줄일 수 있습니다.
Also applies to: 98-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryStatsService.java` around lines 33 - 36, Extract the repeated month-boundary calculation in LibraryStatsService into a single private helper (e.g., private Pair<LocalDateTime,LocalDateTime> computeMonthBounds(YearMonth yearMonth) or a small MonthRange record) and replace the duplicated logic in both methods that currently build start/end (the blocks using yearMonth.atDay(1).atStartOfDay() and yearMonth.plusMonths(1).atDay(1).atStartOfDay()). The helper should return the inclusive start and exclusive end LocalDateTime values and be used by the two callers to reduce duplication and centralize future changes.src/main/java/app/nook/library/service/LibraryService.java (1)
126-130: 캐시 키 규칙을 한 곳으로 모으면 추후 무효화 누락 위험을 줄일 수 있습니다.현재 키 포맷이
@CacheableSpEL과 수동evict양쪽에 분산되어 있어, 포맷 변경 시 불일치가 생기기 쉽습니다. 키 생성 로직을 공용 메서드/상수로 모으는 것을 권장합니다.Also applies to: 190-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryService.java` around lines 126 - 130, Extract the cache name/key logic into shared symbols and reference them from both the `@Cacheable` and any evict calls: add a public static final String FIRST_PAGE_CACHE = "libraryStatusFirstPage" and a public static helper method public static String makeFirstPageCacheKey(Long userId, String status) in LibraryService, update the annotation to use value = FIRST_PAGE_CACHE and key = "T(app.nook.library.service.LibraryService).makeFirstPageCacheKey(`#user.id`, `#status`)" with the same condition, and replace any manual evict key constructions (the evict calls currently paired with the cacheable usage) to call LibraryService.makeFirstPageCacheKey(...) so the key format is defined in one place and reused for both caching and eviction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle`:
- Around line 62-66: The build.gradle currently declares the same dependency
'org.springframework.boot:spring-boot-starter-data-redis' twice; remove the
duplicate declaration so the dependency appears only once (keep whichever single
occurrence fits your intended grouping—e.g., alongside
'spring-boot-starter-cache' under the "cache" comment) to improve readability
and avoid future version drift.
In `@src/main/java/app/nook/focus/repository/FocusRepository.java`:
- Around line 51-62: Replace the Hibernate-specific YEAR() and MONTH() calls in
the query used by findDistinctFocusYearMonthsByLibraryAndUser with standard JPQL
EXTRACT syntax: use EXTRACT(YEAR FROM f.startedAt) AS yearValue and
EXTRACT(MONTH FROM f.startedAt) AS monthValue so the query selects distinct
EXTRACT results for Focus f (startedAt) and still maps to
FocusYearMonthProjection; update the `@Query` string accordingly to maintain the
same aliases and parameter usage for libraryId and userId.
In `@src/main/java/app/nook/global/config/CacheConfig.java`:
- Around line 18-43: The cache configuration in cacheManager uses the default
JDK serializer which will cause SerializationException for non-Serializable
record DTOs; update cacheManager to configure JSON serialization by creating a
RedisCacheConfiguration that sets serializeValuesWith to a
GenericJackson2JsonRedisSerializer (and ensure the underlying ObjectMapper
registers JavaTimeModule and any needed modules for YearMonth/LocalDate) and use
that configuration as default and when building per-cache entries (replace
usages of defaultConfig.entryTtl(...) with copies based on the JSON-configured
RedisCacheConfiguration). Ensure the symbol names to change are cacheManager and
RedisCacheConfiguration/defaultConfig and make sure the ObjectMapper used by
GenericJackson2JsonRedisSerializer registers jackson-datatype-jsr310.
In `@src/main/java/app/nook/library/domain/Library.java`:
- Around line 79-86: The updateStatus method leaves endedAt set when switching
away from FINISHED; modify Library.updateStatus(ReadingStatus) so that when
readingStatus equals ReadingStatus.FINISHED it sets endedAt = LocalDate.now(),
and otherwise clears it (endedAt = null) to handle transitions like FINISHED →
READING; update the method logic that currently checks
readingStatus.equals(ReadingStatus.FINISHED) to branch and set or reset the
endedAt field accordingly.
- Around line 29-39: In Library.updateStatus(ReadingStatus) ensure endedAt is
cleared when the new status is not FINISHED: set this.readingStatus =
readingStatus, if readingStatus == ReadingStatus.FINISHED assign this.endedAt =
LocalDateTime.now().toLocalDate(), else set this.endedAt = null so stale dates
are removed; update the method in the Library class (updateStatus) accordingly.
In `@src/main/java/app/nook/library/dto/FocusRankDto.java`:
- Around line 36-39: The record BookCalendarInfo declares a component named
CoverUrl which violates Java naming conventions and generates an accessor
CoverUrl(); rename the component to coverUrl (i.e., change "String CoverUrl" to
"String coverUrl") so the accessor becomes coverUrl(), update all usages (tests
and call sites like day1.topBook().CoverUrl() → day1.topBook().coverUrl()), and
run/adjust JSON serialization or mapping expectations if any serializers rely on
the field name so API responses remain camelCase-consistent.
In `@src/main/java/app/nook/library/repository/LibraryRepository.java`:
- Line 14: Remove the unused import
org.springframework.security.core.parameters.P from LibraryRepository (the class
using Spring Data `@Param`), since the file references
org.springframework.data.repository.query.Param but not Spring Security's `@P`;
update the imports in
src/main/java/app/nook/library/repository/LibraryRepository.java to delete that
unused import to fix the unused-import warning.
In `@src/main/java/app/nook/library/service/LibraryService.java`:
- Around line 73-74: 현재 `@Transactional` 메서드들에서 즉시 cache.evict()를 호출(예:
evictStatusBookFirstPageCaches 및 다른 cache.evict 호출)하여 트랜잭션 커밋 전에 캐시가 무효화되는 문제가
있습니다; 각 해당 트랜잭션 내부에서 즉시 evict 하지 말고
TransactionSynchronization.registerSynchronization(...)를 사용해 afterCommit 콜백에서
cache.evict를 수행하도록 변경하여 커밋 완료 후에 캐시를 무효화하도록 구현하세요 (대상 메서드명:
evictStatusBookFirstPageCaches 및 다른 즉시 cache.evict 호출들을 찾아 after-commit으로 이동).
In `@src/main/resources/application.yml`:
- Line 22: 현재 메인 application.yml의 generate_statistics: true는 프로덕션에 불필요한 오버헤드를
유발하므로 기본값을 false로 변경하고 개발용 프로파일(application-local.yml 또는 application-dev.yml
등)에만 true로 설정하도록 옮기세요; 구체적으로 메인 application.yml의 generate_statistics 값을 false로
바꾸고, 개발 전용 설정 파일에서 generate_statistics: true를 추가하거나 Spring 프로파일 별로 설정되도록 구성하여
Hibernate 통계 수집(hibernate.generate_statistics)이 개발환경에서만 활성화되게 하세요.
---
Outside diff comments:
In `@src/main/java/app/nook/library/domain/Library.java`:
- Around line 60-76: The field focusMin (Long) can be null for existing DB rows
causing NPE in recordFocus; update recordFocus in class Library to defensively
handle null by initializing focusMin to 0L when null (e.g., if (this.focusMin ==
null) this.focusMin = 0L;) before performing the addition, or alternatively
change focusMin to the primitive long or annotate the field with non-null
constraints and backfill DB—ensure the fix references the focusMin field and the
recordFocus(long addedMinutes) method.
---
Nitpick comments:
In `@build.gradle`:
- Line 3: Replace the SNAPSHOT Spring Boot version with the GA release by
changing the version string used in the Gradle plugin declaration (the line
containing "id 'org.springframework.boot' version '3.5.10-SNAPSHOT'") to the
stable release "3.5.10"; also search the repository for other occurrences of
"3.5.10-SNAPSHOT" (build.gradle, settings.gradle, pluginManagement blocks, or CI
config) and update them to "3.5.10" so builds resolve deterministically in
CI/CD.
In `@src/main/java/app/nook/focus/domain/Focus.java`:
- Around line 17-20: The entity Focus was annotated with a new index named
idx_focus_library_started (columns library_id, started_at) but no explicit DB
migration was added; add a Flyway/Liquibase migration that ensures the index
exists in production (e.g., DROP INDEX IF EXISTS idx_focus_library; CREATE INDEX
idx_focus_library_started ON focuses (library_id, started_at)) so environments
with JPA_DDL_AUTO=validate/none get the change; implement the migration using
your project’s migration tool and include it alongside other migrations so the
Focus index is created reliably in production.
In `@src/main/java/app/nook/focus/repository/FocusRepository.java`:
- Around line 31-49: The query in findMonthlyFocusStats uses the nonstandard
FUNCTION('DATE', f.startedAt); replace that with the standard JPQL cast syntax
CAST(f.startedAt AS date) everywhere it appears (select and group by) to make
the JPQL portable across databases; update the select alias (dateValue) and the
GROUP BY expression to use CAST(f.startedAt AS date) while keeping
f.library.book.id, f.library.book.coverImageUrl, sum(f.durationSec) and the
ORDER BY unchanged so MonthlyFocusStatsProjection still maps correctly.
In `@src/main/java/app/nook/global/config/CacheConfig.java`:
- Around line 25-37: The cache name strings in CacheConfig (cacheConfigs map
entries for "libraryMonthlyCurrent", "libraryMonthlyPast",
"focusMonthlyCurrent", "libraryStatusFirstPage") are hard-coded and must be
centralized: extract these names into a single constant holder (e.g., a
CacheNames class or enum) and replace the literal strings in CacheConfig (and
all usages annotated with `@Cacheable`) with those constants; update references in
methods that rely on these keys so both the cache registration (cacheConfigs in
CacheConfig) and `@Cacheable` annotations use the shared constants to prevent
typos and mismatches.
In `@src/main/java/app/nook/library/domain/Library.java`:
- Around line 63-71: The Library constructor currently sets startedAt via
LocalDateTime.now() which makes tests non-deterministic; change the constructor
to accept either a LocalDate (startedAt) or a java.time.Clock and assign
startedAt with LocalDate.now(clock) (or directly from the passed LocalDate),
update the `@Builder` usage so callers/tests can supply a deterministic value
(e.g., a fixed Clock or a test LocalDate), and modify call sites/tests to pass
the clock/startedAt instead of relying on the constructor to call now().
In `@src/main/java/app/nook/library/dto/FocusRankDto.java`:
- Around line 49-55: MonthlyFocusRow (and similarly FocusTimeRow) are internal
projection-to-DTO intermediates and should not be public; remove the public
modifier from the record declarations (or relocate them as package-private types
inside the service/class that performs the mapping) so they are package-private
or defined as private static nested records within the service class that builds
FocusRankDto; update references in the mapping code to the new
location/visibility to ensure compile-time references still resolve.
In `@src/main/java/app/nook/library/service/LibraryFocusUtil.java`:
- Around line 5-24: Add a private no-arg constructor to the LibraryFocusUtil
class to prevent instantiation of this static utility class: create a private
LibraryFocusUtil() { throw new AssertionError("no instances"); } (or an empty
private constructor) alongside the existing toFocusTimeSlot method so the class
cannot be instantiated by callers or reflection attempts.
In `@src/main/java/app/nook/library/service/LibraryService.java`:
- Around line 126-130: Extract the cache name/key logic into shared symbols and
reference them from both the `@Cacheable` and any evict calls: add a public static
final String FIRST_PAGE_CACHE = "libraryStatusFirstPage" and a public static
helper method public static String makeFirstPageCacheKey(Long userId, String
status) in LibraryService, update the annotation to use value = FIRST_PAGE_CACHE
and key =
"T(app.nook.library.service.LibraryService).makeFirstPageCacheKey(`#user.id`,
`#status`)" with the same condition, and replace any manual evict key
constructions (the evict calls currently paired with the cacheable usage) to
call LibraryService.makeFirstPageCacheKey(...) so the key format is defined in
one place and reused for both caching and eviction.
In `@src/main/java/app/nook/library/service/LibraryStatsService.java`:
- Around line 33-36: Extract the repeated month-boundary calculation in
LibraryStatsService into a single private helper (e.g., private
Pair<LocalDateTime,LocalDateTime> computeMonthBounds(YearMonth yearMonth) or a
small MonthRange record) and replace the duplicated logic in both methods that
currently build start/end (the blocks using yearMonth.atDay(1).atStartOfDay()
and yearMonth.plusMonths(1).atDay(1).atStartOfDay()). The helper should return
the inclusive start and exclusive end LocalDateTime values and be used by the
two callers to reduce duplication and centralize future changes.
In `@src/main/java/app/nook/NookApplication.java`:
- Around line 9-11: Remove the `@EnableCaching` annotation from the
NookApplication class and move it into the existing CacheConfig class so caching
configuration is centralized; specifically, ensure CacheConfig is annotated with
`@Configuration` (if not already) and add `@EnableCaching` to CacheConfig, and then
delete `@EnableCaching` from NookApplication to keep caching concerns inside
CacheConfig.
In `@src/test/java/app/nook/library/service/LibraryStatsServiceTest.java`:
- Around line 22-28: Add a unit test for LibraryStatsService.viewFocusTimeStats
in LibraryStatsServiceTest: mock FocusRepository to return controlled focus
entries, call libraryStatsService.viewFocusTimeStats(...) with appropriate
inputs, and assert the returned DTO/values match expected aggregates; use the
existing `@Mock` focusRepository and `@InjectMocks` libraryStatsService to set up
the scenario and verify edge cases (empty data, single entry) to improve
coverage.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.gitignorebuild.gradlesrc/docs/asciidoc/library.adocsrc/main/java/app/nook/NookApplication.javasrc/main/java/app/nook/focus/domain/Focus.javasrc/main/java/app/nook/focus/repository/FocusRepository.javasrc/main/java/app/nook/global/config/CacheConfig.javasrc/main/java/app/nook/library/controller/LibraryStatsController.javasrc/main/java/app/nook/library/domain/Library.javasrc/main/java/app/nook/library/dto/FocusRankDto.javasrc/main/java/app/nook/library/dto/LibraryViewDto.javasrc/main/java/app/nook/library/repository/LibraryRepository.javasrc/main/java/app/nook/library/repository/LibraryRepositoryCustom.javasrc/main/java/app/nook/library/repository/LibraryRepositoryImpl.javasrc/main/java/app/nook/library/service/LibraryFocusUtil.javasrc/main/java/app/nook/library/service/LibraryService.javasrc/main/java/app/nook/library/service/LibraryStatsService.javasrc/main/java/app/nook/timeline/converter/TimeLineConverter.javasrc/main/java/app/nook/timeline/domain/BookTimeLine.javasrc/main/resources/application.ymlsrc/test/java/app/nook/controller/library/LibraryStatsControllerTest.javasrc/test/java/app/nook/focus/service/ThemeServiceTest.javasrc/test/java/app/nook/library/service/LibraryCachingIntegrationTest.javasrc/test/java/app/nook/library/service/LibraryServiceTest.javasrc/test/java/app/nook/library/service/LibraryStatsServiceTest.java
💤 Files with no reviewable changes (3)
- src/main/java/app/nook/library/repository/LibraryRepositoryCustom.java
- src/main/java/app/nook/library/dto/LibraryViewDto.java
- src/main/java/app/nook/library/repository/LibraryRepositoryImpl.java
Comment on lines
29
to
+39
| @Index( | ||
| name = "idx_library_user", | ||
| columnList = "user_id" | ||
| name = "idx_library_user_status_id", | ||
| columnList = "user_id, reading_status, library_id DESC" | ||
| ), | ||
| @Index( | ||
| name = "idx_library_book", | ||
| columnList = "book_id" | ||
| ) | ||
| } | ||
| ) | ||
| public class Library extends BaseEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| @Column(name = "bookcase_id") | ||
| @Column(name = "library_id") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate Library.java and examine the full content
find . -name "Library.java" -path "*/domain/*" | head -5Repository: UMC-NOOK/Server
Length of output: 111
🏁 Script executed:
# Search for migration directories
find . -type d \( -name "migration" -o -name "flyway" -o -name "liquibase" -o -name "db" \) 2>/dev/null | head -10Repository: UMC-NOOK/Server
Length of output: 41
🏁 Script executed:
# Search for any SQL files that might contain schema changes
fd -e sql 2>/dev/null | head -20Repository: UMC-NOOK/Server
Length of output: 41
🏁 Script executed:
# Search for bookcase_id references in the codebase
rg "bookcase_id" --type-list | head -5Repository: UMC-NOOK/Server
Length of output: 448
🏁 Script executed:
# Search for focusMin field usage
rg "focusMin" -A 2 -B 2Repository: UMC-NOOK/Server
Length of output: 2070
🏁 Script executed:
# Search for idx_library indexes
rg "idx_library" -rRepository: UMC-NOOK/Server
Length of output: 472
🏁 Script executed:
# Read the full Library.java file
cat -n ./src/main/java/app/nook/library/domain/Library.javaRepository: UMC-NOOK/Server
Length of output: 2974
🏁 Script executed:
# Search for bookcase_id across all file types (without --type-list)
rg "bookcase_id"Repository: UMC-NOOK/Server
Length of output: 41
🏁 Script executed:
# Search for `@Index` annotations and index definitions
rg "@Index|@Table" -A 3Repository: UMC-NOOK/Server
Length of output: 3944
🏁 Script executed:
# Search for updateStatus method
rg "updateStatus|endedAt" -B 2 -A 5Repository: UMC-NOOK/Server
Length of output: 6857
🏁 Script executed:
# Final verification: check git history for any reference to bookcase_id
git log --all --source --full-history -S "bookcase_id" -- "src/main/java/app/nook/library/domain/Library.java" 2>/dev/null | head -20Repository: UMC-NOOK/Server
Length of output: 268
🏁 Script executed:
# Check if there are any migration files at all in the repo
find . -type f \( -name "*.sql" -o -name "V*.java" -o -name "*.xml" \) | grep -E "(migration|flyway|liquibase)" 2>/dev/nullRepository: UMC-NOOK/Server
Length of output: 41
updateStatus 메서드에서 endedAt 미초기화 문제를 해결하세요.
updateStatus 메서드(lines 81-86)는 읽기 상태를 FINISHED로 변경할 때만 endedAt을 설정합니다. FINISHED 상태에서 다른 상태(예: READING)로 변경할 때 기존 endedAt 값이 제거되지 않아 오래된 데이터가 남아있게 됩니다.
public void updateStatus(ReadingStatus readingStatus) {
this.readingStatus = readingStatus;
if(readingStatus.equals(ReadingStatus.FINISHED)){
this.endedAt = LocalDateTime.now().toLocalDate();
} else {
this.endedAt = null; // 추가 필요
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/app/nook/library/domain/Library.java` around lines 29 - 39, In
Library.updateStatus(ReadingStatus) ensure endedAt is cleared when the new
status is not FINISHED: set this.readingStatus = readingStatus, if readingStatus
== ReadingStatus.FINISHED assign this.endedAt =
LocalDateTime.now().toLocalDate(), else set this.endedAt = null so stale dates
are removed; update the method in the Library class (updateStatus) accordingly.
Comment on lines
79
to
86
| // 완독 -> 독서 중 | ||
| // 독서 중 -> 완독 : endTime 업데이트 | ||
| public void updateStatus(ReadingStatus readingStatus) { | ||
| this.readingStatus = readingStatus; | ||
| if(readingStatus.equals(ReadingStatus.FINISHED)){ | ||
| this.endedAt = LocalDateTime.now().toLocalDate(); | ||
| } | ||
| } |
There was a problem hiding this comment.
FINISHED에서 다른 상태로 변경 시 endedAt이 초기화되지 않습니다.
현재 updateStatus()는 FINISHED로 변경될 때 endedAt을 설정하지만, 반대로 FINISHED → READING 등으로 변경될 때 endedAt을 null로 되돌리지 않습니다. 주석에도 "완독 → 독서 중" 전환이 언급되어 있으므로, endedAt이 stale 값으로 남아 통계 집계에 오류를 유발할 수 있습니다.
🐛 수정 제안
public void updateStatus(ReadingStatus readingStatus) {
this.readingStatus = readingStatus;
if (readingStatus.equals(ReadingStatus.FINISHED)) {
this.endedAt = LocalDateTime.now().toLocalDate();
+ } else {
+ this.endedAt = null;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // 완독 -> 독서 중 | |
| // 독서 중 -> 완독 : endTime 업데이트 | |
| public void updateStatus(ReadingStatus readingStatus) { | |
| this.readingStatus = readingStatus; | |
| if(readingStatus.equals(ReadingStatus.FINISHED)){ | |
| this.endedAt = LocalDateTime.now().toLocalDate(); | |
| } | |
| } | |
| public void updateStatus(ReadingStatus readingStatus) { | |
| this.readingStatus = readingStatus; | |
| if(readingStatus.equals(ReadingStatus.FINISHED)){ | |
| this.endedAt = LocalDateTime.now().toLocalDate(); | |
| } else { | |
| this.endedAt = null; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/app/nook/library/domain/Library.java` around lines 79 - 86, The
updateStatus method leaves endedAt set when switching away from FINISHED; modify
Library.updateStatus(ReadingStatus) so that when readingStatus equals
ReadingStatus.FINISHED it sets endedAt = LocalDate.now(), and otherwise clears
it (endedAt = null) to handle transitions like FINISHED → READING; update the
method logic that currently checks readingStatus.equals(ReadingStatus.FINISHED)
to branch and set or reset the endedAt field accordingly.
src/main/java/app/nook/library/repository/LibraryRepository.java
Outdated
Show resolved
Hide resolved
src/main/resources/application.yml
Outdated
| format_sql: true | ||
| show_sql: false | ||
| dialect: org.hibernate.dialect.PostgreSQLDialect | ||
| generate_statistics: true |
There was a problem hiding this comment.
generate_statistics: true는 프로덕션 환경에서 성능 오버헤드를 유발합니다
Hibernate 통계 수집은 모든 쿼리에 추가 비용이 발생합니다. 디버깅/개발 목적이라면 application-local.yml 등 개발 프로파일에서만 활성화하는 것이 좋습니다. 메인 application.yml에 설정하면 모든 환경에 적용됩니다.
🔧 수정 제안: 기본값을 false로 변경
hibernate:
format_sql: true
show_sql: false
dialect: org.hibernate.dialect.PostgreSQLDialect
- generate_statistics: true
+ generate_statistics: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| generate_statistics: true | |
| generate_statistics: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/resources/application.yml` at line 22, 현재 메인 application.yml의
generate_statistics: true는 프로덕션에 불필요한 오버헤드를 유발하므로 기본값을 false로 변경하고 개발용
프로파일(application-local.yml 또는 application-dev.yml 등)에만 true로 설정하도록 옮기세요; 구체적으로
메인 application.yml의 generate_statistics 값을 false로 바꾸고, 개발 전용 설정 파일에서
generate_statistics: true를 추가하거나 Spring 프로파일 별로 설정되도록 구성하여 Hibernate 통계
수집(hibernate.generate_statistics)이 개발환경에서만 활성화되게 하세요.
kimdanha
reviewed
Feb 26, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/app/nook/global/config/CacheConfig.java (1)
24-29:⚠️ Potential issue | 🔴 Critical
GenericJackson2JsonRedisSerializer기본 생성자 사용으로 Java Time 직렬화 예외가 발생할 수 있습니다.Line 28 구성은
YearMonth/LocalDate가 포함된 값 캐싱 시 런타임SerializationException위험이 있습니다. 캐시용 serializer에 애플리케이션ObjectMapper(JavaTime 모듈 반영)를 주입해 주세요.🔧 수정 제안
+import com.fasterxml.jackson.databind.ObjectMapper; ... - public RedisCacheManager cacheManager(RedisConnectionFactory connectionFactory) { + public RedisCacheManager cacheManager(RedisConnectionFactory connectionFactory, + ObjectMapper objectMapper) { + ObjectMapper cacheObjectMapper = objectMapper.copy(); RedisCacheConfiguration defaultConfig = RedisCacheConfiguration.defaultCacheConfig() .entryTtl(Duration.ofMinutes(5)) .serializeKeysWith(RedisSerializationContext.SerializationPair.fromSerializer(new StringRedisSerializer())) - .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(new GenericJackson2JsonRedisSerializer())); + .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(new GenericJackson2JsonRedisSerializer(cacheObjectMapper)));#!/bin/bash # CacheConfig 직렬화 설정과 DTO의 Java time 필드 사용 여부 확인 rg -n "GenericJackson2JsonRedisSerializer|ObjectMapper|JavaTimeModule" src/main/java/app/nook/global/config/CacheConfig.java rg -n "YearMonth|LocalDate" src/main/java/app/nook/library/dto/FocusRankDto.java🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/global/config/CacheConfig.java` around lines 24 - 29, CacheConfig currently builds defaultConfig using new GenericJackson2JsonRedisSerializer() which can throw SerializationException for Java Time types; update CacheConfig to inject the application's ObjectMapper bean into the serializer: autowire or accept ObjectMapper in the CacheConfig constructor, ensure that ObjectMapper has JavaTimeModule registered and WRITE_DATES_AS_TIMESTAMPS disabled, and construct GenericJackson2JsonRedisSerializer with that ObjectMapper when setting serializeValuesWith on defaultConfig (refer to class CacheConfig, variable defaultConfig, and GenericJackson2JsonRedisSerializer).src/main/java/app/nook/library/domain/Library.java (1)
81-87:⚠️ Potential issue | 🟠 Major상태 변경 시
endedAt초기화 누락으로 stale 값이 남습니다.Line 85-86 분기에서는
FINISHED가 아닐 때endedAt을 비우지 않아,FINISHED -> READING전환 후에도 완료일이 유지됩니다.🔧 수정 제안
public void updateStatus(ReadingStatus readingStatus) { this.readingStatus = readingStatus; - if(readingStatus.equals(ReadingStatus.FINISHED)){ - this.endedAt = LocalDateTime.now().toLocalDate(); - } else if(readingStatus.equals(ReadingStatus.READING)){ - this.startedAt = LocalDateTime.now().toLocalDate(); + if (readingStatus.equals(ReadingStatus.FINISHED)) { + this.endedAt = LocalDate.now(); + } else if (readingStatus.equals(ReadingStatus.READING)) { + this.startedAt = LocalDate.now(); + this.endedAt = null; + } else { + this.endedAt = null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/domain/Library.java` around lines 81 - 87, The updateStatus method leaves stale endedAt when switching out of FINISHED: in updateStatus(ReadingStatus) ensure you clear endedAt (set to null) whenever readingStatus != ReadingStatus.FINISHED so a previous finish date isn't retained after FINISHED -> READING transitions; likewise, consider clearing startedAt when readingStatus != ReadingStatus.READING to avoid stale start dates—modify the updateStatus method (symbols: updateStatus, readingStatus, endedAt, startedAt, ReadingStatus.FINISHED, ReadingStatus.READING) to set the appropriate fields to null in the non-matching branches.
🧹 Nitpick comments (5)
src/main/java/app/nook/timeline/repository/BookTimeLineRepository.java (1)
10-10: Derived delete 쿼리의 성능 이슈를 고려하세요.Spring Data JPA의
deleteByLibrary는 먼저 SELECT로 매칭되는 모든 엔티티를 조회한 후 각각 개별적으로 DELETE합니다. Library에 연관된 BookTimeLine 레코드가 많으면 N+1 쿼리 문제가 발생합니다.성능 최적화를 위해
@Modifying과@Query를 사용한 벌크 삭제로 변경하길 권장합니다. 다만 이 방식은@PreRemove등 엔티티 라이프사이클 콜백을 실행하지 않는다는 점을 고려하세요.♻️ 벌크 삭제 쿼리로 개선
package app.nook.timeline.repository; import app.nook.library.domain.Library; import app.nook.timeline.domain.BookTimeLine; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; import java.util.List; public interface BookTimeLineRepository extends JpaRepository<BookTimeLine, Long> { - void deleteByLibrary(Library library); + `@Modifying` + `@Query`("DELETE FROM BookTimeLine b WHERE b.library = :library") + void deleteByLibrary(`@Param`("library") Library library); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/timeline/repository/BookTimeLineRepository.java` at line 10, 현재 BookTimeLineRepository의 deleteByLibrary(Library library)는 파생 삭제로 인해 많은 레코드에서 N+1 문제가 발생할 수 있으니, deleteByLibrary 대신 BookTimeLineRepository에 `@Modifying와` `@Query를` 사용한 벌크 삭제 메서드(예: void bulkDeleteByLibrary(Library library) 또는 같은 이름으로 쿼리 어노테이션 적용)를 추가하여 JPQL/네이티브로 "delete from BookTimeLine b where b.library = :library" 형태의 벌크 삭제를 수행하도록 변경하세요; 이 변경은 `@Modifying와` `@Transactional을` 선언하고 메서드 파라미터로 `@Param`("library")를 사용해 바인딩하며, `@PreRemove` 같은 엔티티 라이프사이클 콜백이 실행되지 않는 점을 주석으로 명시해 주세요.src/test/java/app/nook/library/service/LibraryServiceTest.java (1)
55-63: 새로 추가된ApplicationEventPublisher의존성에 대한 행위 검증을 보강해 주세요.
save/deleteById/changeStatus성공 케이스에서publishEvent(...)호출 여부를 검증하면 캐시 무효화 회귀를 더 잘 방지할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/library/service/LibraryServiceTest.java` around lines 55 - 63, 테스트의 save / deleteById / changeStatus 성공 케이스에 ApplicationEventPublisher의 publishEvent(...) 호출 검증을 추가하세요: LibraryServiceTest에서 ApplicationEventPublisher mock인 eventPublisher가 존재하므로 각 성공 시나리오 테스트 끝에 Mockito.verify(eventPublisher).publishEvent(Mockito.any())(또는 구체적 이벤트 타입으로 검증) 호출을 추가해 캐시 무효화 관련 회귀를 방지하도록 하세요.src/test/java/app/nook/library/service/LibraryStatsServiceTest.java (1)
30-170:viewFocusTimeStats에 대한 단위 테스트도 추가하는 편이 좋겠습니다.현재는
viewMonthly만 검증되어viewFocusTimeStats의 일별 정렬/슬롯 매핑/총합 계산 회귀를 잡기 어렵습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/library/service/LibraryStatsServiceTest.java` around lines 30 - 170, Add unit tests for viewFocusTimeStats in LibraryStatsServiceTest to cover daily sorting, slot mapping, and total calculation: create test cases that mock the repository method used by viewFocusTimeStats to return controlled projections (similar to the toProjection helper and row helper already present), then call libraryStatsService.viewFocusTimeStats(...) and assert the returned days are correctly ordered, each day's slot-mapped values are correct, and the total focus time aggregates as expected; include edge cases (multiple entries same day to verify ordering by total, and empty result) and reuse/extend existing helper methods to build projection objects for the repository mock.src/main/java/app/nook/library/event/LibraryCacheInvalidateEvent.java (1)
22-23: 이벤트 payload 리스트는 방어적 복사가 안전합니다.
affectedYearMonths를 그대로 보관하면 호출 측 변경이 이벤트 처리 결과에 영향을 줄 수 있습니다.🔧 수정 제안
public static LibraryCacheInvalidateEvent statusAndMonthly(Long userId, List<YearMonth> affectedYearMonths) { - return new LibraryCacheInvalidateEvent(userId, affectedYearMonths, true); + return new LibraryCacheInvalidateEvent( + userId, + affectedYearMonths == null ? List.of() : List.copyOf(affectedYearMonths), + true + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/event/LibraryCacheInvalidateEvent.java` around lines 22 - 23, The factory method LibraryCacheInvalidateEvent.statusAndMonthly currently stores the caller-provided affectedYearMonths list directly; make a defensive copy (e.g., new ArrayList<>(affectedYearMonths) or Collections.unmodifiableList(new ArrayList<>(...))) before passing/storing it so external mutations can't affect the event; update the LibraryCacheInvalidateEvent constructor or the statusAndMonthly method to accept and retain the copied list (refer to LibraryCacheInvalidateEvent(Long, List<YearMonth>, boolean) and the affectedYearMonths field) and consider handling null inputs appropriately.src/main/java/app/nook/library/service/LibraryStatsService.java (1)
26-29:viewMonthly는 과거 월 캐시 TTL 정책을 현재 반영하지 못합니다.Line 27이 고정
libraryMonthlyCurrent라서libraryMonthlyPast(24h) 설정이 실제로 사용되지 않습니다.yearMonth기준으로 캐시를 분기하는CacheResolver적용을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryStatsService.java` around lines 26 - 29, The `@Cacheable` on viewMonthly in LibraryStatsService currently hardcodes value="libraryMonthlyCurrent" so past-month TTL is never used; implement a CacheResolver that inspects the yearMonth argument and returns either the "libraryMonthlyCurrent" or "libraryMonthlyPast" cache based on whether the supplied YearMonth is the current month, register that CacheResolver as a Spring bean, and replace the annotation to use cacheResolver="<beanName>" while leaving the key (key = "#userId + ':' + `#yearMonth`") intact so caching routing (and respective TTLs) is chosen dynamically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/app/nook/library/domain/Library.java`:
- Around line 75-77: The recordFocus method must reject negative inputs to
prevent decreasing focusMin; update Library.recordFocus to validate addedMinutes
(e.g., if (addedMinutes < 0) throw new IllegalArgumentException("addedMinutes
must be non-negative")) before adding to this.focusMin so negative values are
disallowed and data integrity is preserved.
---
Duplicate comments:
In `@src/main/java/app/nook/global/config/CacheConfig.java`:
- Around line 24-29: CacheConfig currently builds defaultConfig using new
GenericJackson2JsonRedisSerializer() which can throw SerializationException for
Java Time types; update CacheConfig to inject the application's ObjectMapper
bean into the serializer: autowire or accept ObjectMapper in the CacheConfig
constructor, ensure that ObjectMapper has JavaTimeModule registered and
WRITE_DATES_AS_TIMESTAMPS disabled, and construct
GenericJackson2JsonRedisSerializer with that ObjectMapper when setting
serializeValuesWith on defaultConfig (refer to class CacheConfig, variable
defaultConfig, and GenericJackson2JsonRedisSerializer).
In `@src/main/java/app/nook/library/domain/Library.java`:
- Around line 81-87: The updateStatus method leaves stale endedAt when switching
out of FINISHED: in updateStatus(ReadingStatus) ensure you clear endedAt (set to
null) whenever readingStatus != ReadingStatus.FINISHED so a previous finish date
isn't retained after FINISHED -> READING transitions; likewise, consider
clearing startedAt when readingStatus != ReadingStatus.READING to avoid stale
start dates—modify the updateStatus method (symbols: updateStatus,
readingStatus, endedAt, startedAt, ReadingStatus.FINISHED,
ReadingStatus.READING) to set the appropriate fields to null in the non-matching
branches.
---
Nitpick comments:
In `@src/main/java/app/nook/library/event/LibraryCacheInvalidateEvent.java`:
- Around line 22-23: The factory method
LibraryCacheInvalidateEvent.statusAndMonthly currently stores the
caller-provided affectedYearMonths list directly; make a defensive copy (e.g.,
new ArrayList<>(affectedYearMonths) or Collections.unmodifiableList(new
ArrayList<>(...))) before passing/storing it so external mutations can't affect
the event; update the LibraryCacheInvalidateEvent constructor or the
statusAndMonthly method to accept and retain the copied list (refer to
LibraryCacheInvalidateEvent(Long, List<YearMonth>, boolean) and the
affectedYearMonths field) and consider handling null inputs appropriately.
In `@src/main/java/app/nook/library/service/LibraryStatsService.java`:
- Around line 26-29: The `@Cacheable` on viewMonthly in LibraryStatsService
currently hardcodes value="libraryMonthlyCurrent" so past-month TTL is never
used; implement a CacheResolver that inspects the yearMonth argument and returns
either the "libraryMonthlyCurrent" or "libraryMonthlyPast" cache based on
whether the supplied YearMonth is the current month, register that CacheResolver
as a Spring bean, and replace the annotation to use cacheResolver="<beanName>"
while leaving the key (key = "#userId + ':' + `#yearMonth`") intact so caching
routing (and respective TTLs) is chosen dynamically.
In `@src/main/java/app/nook/timeline/repository/BookTimeLineRepository.java`:
- Line 10: 현재 BookTimeLineRepository의 deleteByLibrary(Library library)는 파생 삭제로
인해 많은 레코드에서 N+1 문제가 발생할 수 있으니, deleteByLibrary 대신 BookTimeLineRepository에
`@Modifying와` `@Query를` 사용한 벌크 삭제 메서드(예: void bulkDeleteByLibrary(Library library)
또는 같은 이름으로 쿼리 어노테이션 적용)를 추가하여 JPQL/네이티브로 "delete from BookTimeLine b where
b.library = :library" 형태의 벌크 삭제를 수행하도록 변경하세요; 이 변경은 `@Modifying와` `@Transactional을`
선언하고 메서드 파라미터로 `@Param`("library")를 사용해 바인딩하며, `@PreRemove` 같은 엔티티 라이프사이클 콜백이 실행되지
않는 점을 주석으로 명시해 주세요.
In `@src/test/java/app/nook/library/service/LibraryServiceTest.java`:
- Around line 55-63: 테스트의 save / deleteById / changeStatus 성공 케이스에
ApplicationEventPublisher의 publishEvent(...) 호출 검증을 추가하세요: LibraryServiceTest에서
ApplicationEventPublisher mock인 eventPublisher가 존재하므로 각 성공 시나리오 테스트 끝에
Mockito.verify(eventPublisher).publishEvent(Mockito.any())(또는 구체적 이벤트 타입으로 검증)
호출을 추가해 캐시 무효화 관련 회귀를 방지하도록 하세요.
In `@src/test/java/app/nook/library/service/LibraryStatsServiceTest.java`:
- Around line 30-170: Add unit tests for viewFocusTimeStats in
LibraryStatsServiceTest to cover daily sorting, slot mapping, and total
calculation: create test cases that mock the repository method used by
viewFocusTimeStats to return controlled projections (similar to the toProjection
helper and row helper already present), then call
libraryStatsService.viewFocusTimeStats(...) and assert the returned days are
correctly ordered, each day's slot-mapped values are correct, and the total
focus time aggregates as expected; include edge cases (multiple entries same day
to verify ordering by total, and empty result) and reuse/extend existing helper
methods to build projection objects for the repository mock.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
build.gradlesrc/main/java/app/nook/focus/repository/FocusRepository.javasrc/main/java/app/nook/global/config/CacheConfig.javasrc/main/java/app/nook/library/domain/Library.javasrc/main/java/app/nook/library/dto/FocusRankDto.javasrc/main/java/app/nook/library/event/LibraryCacheInvalidateEvent.javasrc/main/java/app/nook/library/event/LibraryCacheInvalidationListener.javasrc/main/java/app/nook/library/repository/LibraryRepository.javasrc/main/java/app/nook/library/service/LibraryService.javasrc/main/java/app/nook/library/service/LibraryStatsService.javasrc/main/java/app/nook/timeline/repository/BookTimeLineRepository.javasrc/main/resources/application.ymlsrc/test/java/app/nook/controller/library/LibraryStatsControllerTest.javasrc/test/java/app/nook/library/service/LibraryCachingIntegrationTest.javasrc/test/java/app/nook/library/service/LibraryServiceTest.javasrc/test/java/app/nook/library/service/LibraryStatsServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/app/nook/library/dto/FocusRankDto.java
- src/main/java/app/nook/library/repository/LibraryRepository.java
- src/main/resources/application.yml
- src/test/java/app/nook/controller/library/LibraryStatsControllerTest.java
Comment on lines
75
to
77
| public void recordFocus(long addedMinutes) { | ||
| this.focusMin += addedMinutes; | ||
| } |
There was a problem hiding this comment.
recordFocus는 음수 입력을 방어해야 합니다.
Line 76에서 음수 addedMinutes가 들어오면 누적 시간이 감소해 데이터 정합성이 깨집니다.
🔧 수정 제안
public void recordFocus(long addedMinutes) {
+ if (addedMinutes <= 0) {
+ throw new IllegalArgumentException("addedMinutes must be positive");
+ }
this.focusMin += addedMinutes;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/app/nook/library/domain/Library.java` around lines 75 - 77, The
recordFocus method must reject negative inputs to prevent decreasing focusMin;
update Library.recordFocus to validate addedMinutes (e.g., if (addedMinutes < 0)
throw new IllegalArgumentException("addedMinutes must be non-negative")) before
adding to this.focusMin so negative values are disallowed and data integrity is
preserved.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/app/nook/library/domain/Library.java (1)
66-74:⚠️ Potential issue | 🟡 Minor생성자에서
startedAt초기화가 부적절할 수 있습니다.새
Library객체 생성 시readingStatus기본값은BEFORE이지만, 생성자에서startedAt을 현재 날짜로 설정하고 있습니다. 읽기 시작 전(BEFORE) 상태에서는startedAt이null이어야 데이터 의미가 일관됩니다.🐛 수정 제안
`@Builder` public Library( User user, Book book ){ this.user = user; this.book = book; - this.startedAt = LocalDateTime.now().toLocalDate(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/domain/Library.java` around lines 66 - 74, The Library constructor currently sets startedAt = LocalDateTime.now().toLocalDate(), which makes startedAt non-null even when readingStatus is the default BEFORE; change the constructor in class Library (the `@Builder` public Library(User user, Book book) method) to not initialize startedAt unconditionally — either remove the startedAt assignment so it remains null by default, or set startedAt conditionally (e.g., only assign now() when readingStatus != BEFORE). Ensure the fix references the Library constructor and the startedAt and readingStatus fields so the semantics remain consistent.
🧹 Nitpick comments (5)
src/test/java/app/nook/global/common/AbstractWebMvcRestDocsTests.java (1)
39-79:AbstractRestDocsTests와의 코드 중복을 고려해 주세요.
AbstractRestDocsTests와AbstractWebMvcRestDocsTests간에AUTH_HEADER,AUTH_TOKEN,setUp(),documentWithAuth()등 상당 부분이 중복됩니다. 공통 로직을 별도의 유틸리티 클래스나 인터페이스로 추출하면 유지보수가 용이해집니다.현재 구조로도 동작에 문제는 없으므로, 추후 리팩토링 시 고려해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/global/common/AbstractWebMvcRestDocsTests.java` around lines 39 - 79, This class duplicates constants and methods from AbstractRestDocsTests (AUTH_HEADER, AUTH_TOKEN, setUp(), documentWithAuth()), causing maintenance overhead; refactor by extracting the shared constants and behaviors into a common base or utility (e.g., a new AbstractRestDocsBase or RestDocsUtil) and have AbstractWebMvcRestDocsTests and AbstractRestDocsTests delegate to or extend that shared component — move AUTH_HEADER/AUTH_TOKEN, the MockMvc setup in setUp(), and the documentWithAuth(Snippet...) merging logic (including use of authorizationHeaderSnippet) into that shared location and replace the duplicated implementations with calls to the shared methods/fields.src/main/java/app/nook/library/service/LibraryStatsService.java (4)
103-108: 스트림 중복 순회 최적화 가능
rows스트림을 두 번 순회하여totalSec합계와maxDaySec를 각각 계산하고 있습니다. 단일 순회로 최적화할 수 있습니다.♻️ 단일 순회 리팩토링
- long totalSec = rows.stream().mapToLong(FocusRankDto.FocusTimeRow::totalSec).sum(); - int totalFocusMin = (int) (totalSec / 60); - long maxDaySec = rows.stream() - .mapToLong(FocusRankDto.FocusTimeRow::totalSec) - .max() - .orElse(0L); + LongSummaryStatistics stats = rows.stream() + .mapToLong(FocusRankDto.FocusTimeRow::totalSec) + .summaryStatistics(); + int totalFocusMin = (int) (stats.getSum() / 60); + long maxDaySec = stats.getMax() == Long.MIN_VALUE ? 0L : stats.getMax();
java.util.LongSummaryStatisticsimport가 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryStatsService.java` around lines 103 - 108, The code currently iterates rows twice to compute totalSec/totalFocusMin and maxDaySec; change to a single-pass aggregation using LongSummaryStatistics by calling rows.stream().mapToLong(FocusRankDto.FocusTimeRow::totalSec).summaryStatistics(), then derive totalSec from stats.getSum(), totalFocusMin from (int)(totalSec/60), and maxDaySec from stats.getMax(); add import java.util.LongSummaryStatistics to the file.
86-90: viewMonthly와 동일하게 과거/현재 월 캐시 분리 검토
viewMonthly메서드와 마찬가지로, 이 메서드도 과거 월 데이터에 대해서는 더 긴 TTL을 적용하는 것이 효율적일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryStatsService.java` around lines 86 - 90, The Cacheable on the focusMonthlyCurrent method should be split or adjusted like viewMonthly so past-month data gets a longer TTL than current-month data; modify LibraryStatsService.focusMonthlyCurrent to detect whether the provided yearMonth is before the current month and use a different cache name/key (e.g., "focusMonthlyPast" vs "focusMonthlyCurrent") or otherwise route to a cache resolver that applies different TTLs, and update your cache configuration to set the longer TTL for the past-month cache while keeping a short TTL for the current-month cache.
66-79: Optional 활용으로 null 처리 간소화 가능현재
orElse(null)후 null 체크 패턴은 동작하지만,Optional을 활용하면 더 깔끔하게 처리할 수 있습니다.♻️ 리팩토링 제안
- // 가장 오래 읽은 책 - FocusRankDto.MonthlyFocusRow top = dayRows.stream() - .max(Comparator.comparingLong(FocusRankDto.MonthlyFocusRow::totalSec)) - .orElse(null); - - FocusRankDto.BookCalendarInfo topBook = null; - - // 오래 읽은 책이 있으면 매핑 - if (top!=null) { - topBook = new FocusRankDto.BookCalendarInfo( - top.bookId(), - top.coverImageUrl() - ); - } + // 가장 오래 읽은 책 + FocusRankDto.BookCalendarInfo topBook = dayRows.stream() + .max(Comparator.comparingLong(FocusRankDto.MonthlyFocusRow::totalSec)) + .map(top -> new FocusRankDto.BookCalendarInfo( + top.bookId(), + top.coverImageUrl() + )) + .orElse(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryStatsService.java` around lines 66 - 79, The code uses dayRows.stream().max(...).orElse(null) and a subsequent null check to build topBook; replace this pattern with Optional mapping to avoid nulls: capture the Optional<FocusRankDto.MonthlyFocusRow> from dayRows.stream().max(Comparator.comparingLong(FocusRankDto.MonthlyFocusRow::totalSec)) and use its map(...) to construct a FocusRankDto.BookCalendarInfo (using top.bookId() and top.coverImageUrl()) and orElse(null) only on the final Optional if needed, then pass that result into new FocusRankDto.DailyBookItem(date, bookCount, topBook) so the intermediate null check for the variable top is removed.
26-29: 캐싱 전략: 과거 월과 현재 월 구분 필요 검토현재
libraryMonthlyCurrent캐시만 사용되고 있습니다. AI 요약에 따르면libraryMonthlyPast캐시도 설정되어 있는 것으로 보이는데, 과거 월 데이터는 변경되지 않으므로 더 긴 TTL을 가진 별도 캐시를 사용하면 효율적일 수 있습니다.현재 월인지 과거 월인지에 따라 조건부로 캐시를 분리하는 것을 고려해보세요:
`@Cacheable`( value = "libraryMonthlyCurrent", key = "#userId + ':' + `#yearMonth`", condition = "#yearMonth.equals(java.time.YearMonth.now())" ) `@Cacheable`( value = "libraryMonthlyPast", key = "#userId + ':' + `#yearMonth`", condition = "!#yearMonth.equals(java.time.YearMonth.now())" )또는
@Caching어노테이션을 활용할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/app/nook/library/service/LibraryStatsService.java` around lines 26 - 29, The current `@Cacheable` on the method in LibraryStatsService always uses "libraryMonthlyCurrent"; change the caching so past months use a separate longer‑TTL cache ("libraryMonthlyPast") and current month uses "libraryMonthlyCurrent" by adding conditional cache rules: apply two `@Cacheable` annotations (or wrap them in `@Caching`) on the same method with condition = "#yearMonth.equals(java.time.YearMonth.now())" for current and the inverse condition for past, keeping the key as "#userId + ':' + `#yearMonth`" so the method (the annotated method in LibraryStatsService) stores current and historical monthly results in different caches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/app/nook/library/service/LibraryService.java`:
- Around line 102-103: The deletion order is incorrect and Focus cleanup is
missing: before calling libraryRepository.delete(library) you must first remove
dependent Focus and BookTimeLine rows to avoid FK violations. Add a
deleteByLibrary(Library library) method to FocusRepository, then change the
removal sequence in the service to call focusRepository.deleteByLibrary(library)
followed by bookTimeLineRepository.deleteByLibrary(library) and only after those
succeed call libraryRepository.delete(library); ensure you reference the
existing bookTimeLineRepository.deleteByLibrary(...) and
libraryRepository.delete(...) calls when updating the code.
---
Outside diff comments:
In `@src/main/java/app/nook/library/domain/Library.java`:
- Around line 66-74: The Library constructor currently sets startedAt =
LocalDateTime.now().toLocalDate(), which makes startedAt non-null even when
readingStatus is the default BEFORE; change the constructor in class Library
(the `@Builder` public Library(User user, Book book) method) to not initialize
startedAt unconditionally — either remove the startedAt assignment so it remains
null by default, or set startedAt conditionally (e.g., only assign now() when
readingStatus != BEFORE). Ensure the fix references the Library constructor and
the startedAt and readingStatus fields so the semantics remain consistent.
---
Nitpick comments:
In `@src/main/java/app/nook/library/service/LibraryStatsService.java`:
- Around line 103-108: The code currently iterates rows twice to compute
totalSec/totalFocusMin and maxDaySec; change to a single-pass aggregation using
LongSummaryStatistics by calling
rows.stream().mapToLong(FocusRankDto.FocusTimeRow::totalSec).summaryStatistics(),
then derive totalSec from stats.getSum(), totalFocusMin from (int)(totalSec/60),
and maxDaySec from stats.getMax(); add import java.util.LongSummaryStatistics to
the file.
- Around line 86-90: The Cacheable on the focusMonthlyCurrent method should be
split or adjusted like viewMonthly so past-month data gets a longer TTL than
current-month data; modify LibraryStatsService.focusMonthlyCurrent to detect
whether the provided yearMonth is before the current month and use a different
cache name/key (e.g., "focusMonthlyPast" vs "focusMonthlyCurrent") or otherwise
route to a cache resolver that applies different TTLs, and update your cache
configuration to set the longer TTL for the past-month cache while keeping a
short TTL for the current-month cache.
- Around line 66-79: The code uses dayRows.stream().max(...).orElse(null) and a
subsequent null check to build topBook; replace this pattern with Optional
mapping to avoid nulls: capture the Optional<FocusRankDto.MonthlyFocusRow> from
dayRows.stream().max(Comparator.comparingLong(FocusRankDto.MonthlyFocusRow::totalSec))
and use its map(...) to construct a FocusRankDto.BookCalendarInfo (using
top.bookId() and top.coverImageUrl()) and orElse(null) only on the final
Optional if needed, then pass that result into new
FocusRankDto.DailyBookItem(date, bookCount, topBook) so the intermediate null
check for the variable top is removed.
- Around line 26-29: The current `@Cacheable` on the method in LibraryStatsService
always uses "libraryMonthlyCurrent"; change the caching so past months use a
separate longer‑TTL cache ("libraryMonthlyPast") and current month uses
"libraryMonthlyCurrent" by adding conditional cache rules: apply two `@Cacheable`
annotations (or wrap them in `@Caching`) on the same method with condition =
"#yearMonth.equals(java.time.YearMonth.now())" for current and the inverse
condition for past, keeping the key as "#userId + ':' + `#yearMonth`" so the
method (the annotated method in LibraryStatsService) stores current and
historical monthly results in different caches.
In `@src/test/java/app/nook/global/common/AbstractWebMvcRestDocsTests.java`:
- Around line 39-79: This class duplicates constants and methods from
AbstractRestDocsTests (AUTH_HEADER, AUTH_TOKEN, setUp(), documentWithAuth()),
causing maintenance overhead; refactor by extracting the shared constants and
behaviors into a common base or utility (e.g., a new AbstractRestDocsBase or
RestDocsUtil) and have AbstractWebMvcRestDocsTests and AbstractRestDocsTests
delegate to or extend that shared component — move AUTH_HEADER/AUTH_TOKEN, the
MockMvc setup in setUp(), and the documentWithAuth(Snippet...) merging logic
(including use of authorizationHeaderSnippet) into that shared location and
replace the duplicated implementations with calls to the shared methods/fields.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/generated/app/nook/library/domain/QLibrary.javais excluded by!**/generated/**
📒 Files selected for processing (21)
build.gradlesrc/main/java/app/nook/focus/repository/FocusRepository.javasrc/main/java/app/nook/library/controller/LibraryController.javasrc/main/java/app/nook/library/domain/Library.javasrc/main/java/app/nook/library/dto/FocusRankDto.javasrc/main/java/app/nook/library/dto/LibraryViewDto.javasrc/main/java/app/nook/library/repository/LibraryRepository.javasrc/main/java/app/nook/library/service/LibraryService.javasrc/main/java/app/nook/library/service/LibraryStatsService.javasrc/main/java/app/nook/timeline/domain/BookTimeLine.javasrc/test/java/app/nook/controller/book/BookControllerTest.javasrc/test/java/app/nook/controller/book/BookSearchControllerTest.javasrc/test/java/app/nook/controller/library/LibraryControllerTest.javasrc/test/java/app/nook/controller/library/LibraryStatsControllerTest.javasrc/test/java/app/nook/controller/user/AuthControllerTest.javasrc/test/java/app/nook/global/common/AbstractRestDocsTests.javasrc/test/java/app/nook/global/common/AbstractWebMvcRestDocsTests.javasrc/test/java/app/nook/global/common/TestSecurityConfig.javasrc/test/java/app/nook/global/common/security/WithCustomUser.javasrc/test/java/app/nook/global/common/security/WithCustomUserSecurityContextFactory.javasrc/test/java/app/nook/library/service/LibraryStatsServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/app/nook/library/repository/LibraryRepository.java
- src/main/java/app/nook/timeline/domain/BookTimeLine.java
Comment on lines
102
to
103
| libraryRepository.delete(library); | ||
| bookTimeLineRepository.deleteByLibrary(library); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Finding relevant files =="
fd 'Library.java|BookTimeLine.java|BookTimeLineRepository.java' src/main/java
echo ""
echo "== Checking entity relationships and cascade settings =="
rg -n "class Library|class BookTimeLine|@ManyToOne|@OneToMany|cascade|orphanRemoval|@OnDelete" src/main/java -C 3Repository: UMC-NOOK/Server
Length of output: 11049
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full Library.java inspection =="
cat -n src/main/java/app/nook/library/domain/Library.java
echo ""
echo "== Check BookTimeLineRepository for deleteByLibrary method =="
cat -n src/main/java/app/nook/timeline/repository/BookTimeLineRepository.javaRepository: UMC-NOOK/Server
Length of output: 3830
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check LibraryService deleteById method (full context) =="
rg -n "deleteById" src/main/java/app/nook/library/service/LibraryService.java -A 20
echo ""
echo "== Check Focus entity and its repository =="
cat -n src/main/java/app/nook/focus/domain/Focus.java
echo ""
echo "== Check if FocusRepository is used in deleteById =="
rg -n "focusRepository|FocusRepository" src/main/java/app/nook/library/service/LibraryService.javaRepository: UMC-NOOK/Server
Length of output: 3519
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full LibraryService.deleteById method =="
sed -n '84,120p' src/main/java/app/nook/library/service/LibraryService.java
echo ""
echo "== Find all entities with FK to Library =="
rg -n "@JoinColumn.*library" src/main/java -C 5 | head -100
echo ""
echo "== Check if Focus records are deleted anywhere =="
rg -n "focusRepository.*delete" src/main/java/app/nook/library/service/LibraryService.javaRepository: UMC-NOOK/Server
Length of output: 3767
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check database schema: library_id constraints in timeline and focus tables =="
rg -n "FOREIGN KEY|CONSTRAINT|ON DELETE" . --type sql | head -50
echo ""
echo "== Check if there's a FocusRepository.deleteByLibrary or similar =="
cat -n src/main/java/app/nook/focus/repository/FocusRepository.java | head -50
echo ""
echo "== Verify complete deleteById method including event publishing =="
sed -n '84,113p' src/main/java/app/nook/library/service/LibraryService.javaRepository: UMC-NOOK/Server
Length of output: 139
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full FocusRepository inspection =="
cat -n src/main/java/app/nook/focus/repository/FocusRepository.java
echo ""
echo "== Verify: Are Focus records deleted in deleteById? =="
sed -n '84,115p' src/main/java/app/nook/library/service/LibraryService.java | cat -nRepository: UMC-NOOK/Server
Length of output: 7028
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Critical: Check if Focus records are deleted anywhere in deleteById =="
sed -n '84,113p' src/main/java/app/nook/library/service/LibraryService.java | grep -i "focus.*delete"
echo ""
echo "== Check BookTimeLine entity for cascade/orphanRemoval =="
cat -n src/main/java/app/nook/timeline/domain/BookTimeLine.java | head -60
echo ""
echo "== Summary: Who has FKs to Library? =="
rg -n "class.*extends|@ManyToOne.*Library|@JoinColumn.*library_id" src/main/java -C 2 | grep -E "(class|@ManyToOne|@JoinColumn)" | head -30Repository: UMC-NOOK/Server
Length of output: 132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Complete deleteById method and surrounding context =="
sed -n '82,120p' src/main/java/app/nook/library/service/LibraryService.java | cat -n
echo ""
echo "== Check if focusRepository has deleteByLibrary or any delete methods =="
grep -n "delete" src/main/java/app/nook/focus/repository/FocusRepository.java
echo ""
echo "== List all methods in FocusRepository =="
rg -n "^\s*(void|List|Slice|int)\s+" src/main/java/app/nook/focus/repository/FocusRepository.javaRepository: UMC-NOOK/Server
Length of output: 2087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check all entities with `@JoinColumn`(name = \"library_id\") =="
rg -n "@JoinColumn(name = \"library_id\")" src/main/java -B 5 | head -60
echo ""
echo "== List all repository methods that might delete by Library =="
fd "Repository.java" src/main/java -x grep -l "deleteBy" {}
echo ""
echo "== Explicitly check for all deleteBy* methods in repositories =="
fd "Repository.java" src/main/java -x grep -n "delete" {}Repository: UMC-NOOK/Server
Length of output: 121
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find all *Repository.java files =="
find src/main/java -name "*Repository.java" -type f
echo ""
echo "== Check BookTimeLineRepository.java for deleteByLibrary =="
cat src/main/java/app/nook/timeline/repository/BookTimeLineRepository.java
echo ""
echo "== Check FocusRepository.java for any delete methods =="
cat src/main/java/app/nook/focus/repository/FocusRepository.java | grep -i "delete"
echo ""
echo "== Check all entities that have library_id FK =="
find src/main/java -name "*.java" -path "*/domain/*" -type f | while read f; do
if grep -q "library_id" "$f" 2>/dev/null; then
echo "=== File: $f ==="
grep -n "library_id\|class.*extends\|@Entity" "$f" | head -5
fi
doneRepository: UMC-NOOK/Server
Length of output: 1122
삭제 순서 오류 및 누락된 엔티티 삭제
Lines 102-103에서 부모(Library)를 먼저 삭제한 후 자식(BookTimeLine)을 삭제하고 있으며, 더 심각하게는 Focus 엔티티 삭제가 완전히 누락되어 있습니다.
Database schema에서 BookTimeLine.library_id와 Focus.library_id 모두 Library에 대한 FK입니다. Library를 먼저 삭제하면 FK 제약 위반이 발생할 수 있으며, 특히 Focus 레코드가 전혀 삭제되지 않아 고아 레코드(orphaned records)가 남게 됩니다.
올바른 삭제 순서:
- Focus 레코드 삭제
- BookTimeLine 레코드 삭제
- Library 레코드 삭제
🔧 제안 수정안
// 캐시 무효화에 필요한 데이터 조회
List<YearMonth> affectedYearMonths =
focusRepository.findDistinctFocusYearMonthsByLibraryAndUser(
library.getId(),
user.getId()
).stream()
.map(ym -> YearMonth.of(ym.getYearValue(), ym.getMonthValue()))
.collect(Collectors.toList());
+ focusRepository.deleteByLibrary(library);
bookTimeLineRepository.deleteByLibrary(library);
libraryRepository.delete(library);FocusRepository에 deleteByLibrary 메서드 추가 필요:
void deleteByLibrary(Library library);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/app/nook/library/service/LibraryService.java` around lines 102
- 103, The deletion order is incorrect and Focus cleanup is missing: before
calling libraryRepository.delete(library) you must first remove dependent Focus
and BookTimeLine rows to avoid FK violations. Add a deleteByLibrary(Library
library) method to FocusRepository, then change the removal sequence in the
service to call focusRepository.deleteByLibrary(library) followed by
bookTimeLineRepository.deleteByLibrary(library) and only after those succeed
call libraryRepository.delete(library); ensure you reference the existing
bookTimeLineRepository.deleteByLibrary(...) and libraryRepository.delete(...)
calls when updating the code.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/resources/application.yml (1)
23-28: Redis 포트도 환경변수로 외부화해 운영 유연성을 높여주세요.Line 24에서 Redis 캐시를 활성화한 만큼, Redis 연결값도 동일한 수준으로 외부화하는 편이 안전합니다. 현재 Line 28의 고정 포트(
6379)는 환경별 설정 전환 시 제약이 됩니다.🔧 제안 수정안
data: redis: host: ${REDIS_HOST:localhost} - port: 6379 + port: ${REDIS_PORT:6379}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/application.yml` around lines 23 - 28, Replace the hard-coded Redis port with an environment-variable-based value so the application uses data.redis.port from an env var; update the YAML entry for data.redis.port to reference something like ${REDIS_PORT:6379} (leave 6379 as the default) and ensure related settings cache.type and data.redis.host remain unchanged so configuration is consistent and externally overrideable.src/test/java/app/nook/controller/book/BookControllerTest.java (1)
324-327: 가독성 향상을 위해 주석 추가를 고려해 주세요.
multipart()가 기본적으로 POST 메서드를 사용하기 때문에 PATCH로 변경하는 workaround입니다. 향후 코드 리뷰어나 유지보수 담당자를 위해 간단한 설명 주석을 추가하면 좋겠습니다.💡 주석 추가 제안
.param("isbn13", "9788936439743") + // MockMvc의 multipart()는 기본적으로 POST를 사용하므로 PATCH로 변경 .with(request -> { request.setMethod("PATCH"); return request; })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/app/nook/controller/book/BookControllerTest.java` around lines 324 - 327, Add a brief inline comment above the multipart request workaround in BookControllerTest that explains multipart() defaults to POST so we explicitly change the method to PATCH using the lambda (the with(request -> { request.setMethod("PATCH"); return request; }) block); place the comment near the multipart(...) call to clarify intent for future reviewers and maintainers, referencing the specific lambda used to set the HTTP method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/resources/application.yml`:
- Around line 23-28: Replace the hard-coded Redis port with an
environment-variable-based value so the application uses data.redis.port from an
env var; update the YAML entry for data.redis.port to reference something like
${REDIS_PORT:6379} (leave 6379 as the default) and ensure related settings
cache.type and data.redis.host remain unchanged so configuration is consistent
and externally overrideable.
In `@src/test/java/app/nook/controller/book/BookControllerTest.java`:
- Around line 324-327: Add a brief inline comment above the multipart request
workaround in BookControllerTest that explains multipart() defaults to POST so
we explicitly change the method to PATCH using the lambda (the with(request -> {
request.setMethod("PATCH"); return request; }) block); place the comment near
the multipart(...) call to clarify intent for future reviewers and maintainers,
referencing the specific lambda used to set the HTTP method.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/resources/application.ymlsrc/test/java/app/nook/controller/book/BookControllerTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📄 작업 내용 요약
📎 Issue 번호
✅ 작업 목록
📝 기타 참고사항
캐싱을 도입했는데 다른 API들에도 적용하면 좋을 것 같습니다~!
그리고 포커스 생성 시에 캐시 무효화, 서재에도 업데이트 추후 구현 예정입니다.
Summary by CodeRabbit
새로운 기능
문서
테스트
Chores