Conversation
- Test.java 파일들 제거 (articleview, common/config) - 불필요한 테스트 코드 정리
- AWS SDK BOM 2.29.32 버전 추가 - S3 클라이언트 라이브러리 추가 - 기사 백업 및 로그 백업을 위한 S3 연동 준비
- AWS S3 리전 및 버킷 설정 추가 (기사 백업용, 로그 백업용) - AWS 자격 증명 환경 변수 설정 - 로그 파일 경로 및 보관 기간 설정 추가 - Spring Batch job 자동 실행 비활성화 (수동 트리거 활성화) - 배치 트리거 설정 재배치
- 일별 로그 파일 롤링 정책 추가 - 로그 파일 경로 및 이름 동적 설정 (application.yml 연동) - 최대 7일간 로그 파일 보관 설정 - 파일 로깅과 콘솔 로깅 동시 사용 - S3 로그 백업 배치를 위한 로그 파일 생성
- S3Client 빈 생성 및 설정 - AWS 리전 및 자격 증명 설정 - 기사 백업 및 로그 백업을 위한 S3 연동 기반 구성
- RequestIdFilter 구현 (UUID 기반 요청 ID 생성) - MDC를 통한 로그 컨텍스트에 요청 정보 저장 - 요청 ID, HTTP 메서드, URI 추적 기능 - 배치 작업 및 API 요청 로그 추적 개선
- LoggingUtils 클래스 추가 (MDC 헬퍼 메서드) - 요청 컨텍스트 정보 설정/조회/정리 기능 - 배치 작업 및 비동기 작업 로깅 지원
- ArticleBackupService 인터페이스 및 S3 구현체 추가 - LogBackupService 인터페이스 및 S3 구현체 추가 - S3 업로드, 다운로드, GZIP 압축/해제 기능 - 기사 데이터 JSON 백업 및 복원 기능 - 로그 파일 백업 및 로컬 정리 기능 - S3ServiceException 커스텀 예외 추가
- ArticleBackupBatchConfig 구현 - 전체 기사 데이터를 S3에 백업하는 배치 작업 - Chunk 기반 처리 (100개씩) - ArticleBackupProcessor 및 ArticleBackupWriter 연동 - 배치 실행 로그 한글 메시지 적용
- LogBackupBatchConfig 구현 - 전날 로그 파일을 S3에 백업하는 Tasklet 배치 - GZIP 압축 후 S3 업로드 - 업로드 성공 시 로컬 파일 자동 삭제 - 배치 실행 로그 한글 메시지 적용
- ArticleBackupProcessor: 삭제된 기사를 S3 백업용 JSON으로 변환 - ArticleBackupWriter: 배치 처리된 기사 백업을 S3에 업로드 - ArticleBackupScheduler: 매일 새벽 2시 삭제된 기사 자동 백업 실행 - LogBackupScheduler: 매일 새벽 1시 로그 파일 자동 백업 실행
- deletedAt 필드 추가로 소프트 삭제 지원 - @where 애너테이션으로 삭제되지 않은 기사만 기본 조회 - @SQLDelete 애너테이션으로 DELETE 쿼리를 UPDATE로 자동 변환
- findAllByDeletedAtIsNotNull: 삭제된 기사 조회 (백업용) - hardDeleteById: 물리적 삭제 쿼리 - restoreById: 소프트 삭제 복원 쿼리
- softDeleteArticle: 소프트 삭제 - hardDeleteArticle: 물리적 삭제 - restoreArticle: 삭제된 기사 복원
- softDeleteArticle: deletedAt 설정 후 댓글/조회수 cascade 삭제 - hardDeleteArticle: DB에서 완전 삭제 (복원 불가) - restoreArticle: deletedAt 초기화로 복원 - 각 작업마다 AuditLogger로 감사 로그 기록
- DELETE /api/articles/{id}: 소프트 삭제
- DELETE /api/articles/{id}/hard: 물리적 삭제
- POST /api/articles/{id}/restore: 삭제 복원
- Swagger 문서화 포함
- deleteByArticleId: 기사 삭제 시 연관된 댓글 자동 삭제 - @Modifying @transactional로 벌크 삭제 지원
- deleteByArticleId: 기사 삭제 시 연관된 조회수 자동 삭제 - @Modifying @transactional로 벌크 삭제 지원
- POST /api/batch/trigger/log-backup: 로그 백업 수동 실행 - Swagger 문서화 포함
- triggerLogBackupJob: 로그 백업 배치 작업 수동 실행 - 기존 기사 백업 Job과 동일한 패턴으로 구현
- /api/batch/trigger/** 경로 인증 없이 허용 - 개발/테스트 환경에서 배치 작업 수동 실행 지원
- BackupException: 500 Internal Server Error 응답 - 백업 실패 시 명확한 에러 메시지 반환
Walkthrough기사·로그의 S3 기반 백업/복구 및 관련 배치(Job/Step/Processor/Writer)/스케줄러/트리거와 소프트/하드 삭제·복구 로직, 요청 ID 필터·감사 로깅·예외 처리, S3 클라이언트 구성, 로그 파일 롤링·스키마·엔티티 soft-delete가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Ctrl as ArticleController
participant Svc as ArticleServiceImpl
participant Repo as ArticleRepository
participant CRepo as CommentRepository
participant VRepo as ArticleViewRepository
participant S3 as S3BackupService
participant Audit as AuditLogger
rect rgb(230,248,255)
Client->>Ctrl: DELETE /api/articles/{id}
Ctrl->>Svc: softDeleteArticle(articleId, user)
Svc->>Repo: mark soft-deleted (update is_deleted)
Svc->>Audit: logSoftDelete(...)
Ctrl-->>Client: 204
end
rect rgb(255,244,230)
Client->>Ctrl: DELETE /api/articles/{id}/hard
Ctrl->>Svc: hardDeleteArticle(articleId, user)
Svc->>CRepo: deleteCommentLikesByArticleId(articleId)
Svc->>CRepo: deleteByArticleId(articleId)
Svc->>VRepo: deleteByArticleId(articleId)
Svc->>Repo: hardDelete(articleId) (physical DELETE)
Svc->>Audit: logHardDelete(...)
Ctrl-->>Client: 204
end
rect rgb(240,255,240)
Client->>Ctrl: GET /api/articles/restore?from=..&to=..
Ctrl->>Svc: restoreArticlesFromBackup(from,to,user)
loop per date
Svc->>S3: backupExists(date)
alt exists
Svc->>S3: downloadBackup(date)
Svc->>Repo: findAllSourceUrls()/existsIncludingDeleted(...)
alt missing
Svc->>Svc: restoreArticle(dto) -> Repo.save(...)
end
else not found
Svc->>Audit: logRestoreFailure(...)
end
end
Svc->>Audit: logRestore(...)
Svc-->>Ctrl: ArticleRestoreResultDto
Ctrl-->>Client: 200
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 주의 집중 영역:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ 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: 13
🧹 Nitpick comments (5)
monew/src/main/java/com/spring/monew/common/config/AwsS3Config.java (1)
25-29: region 유효성 검증과 DefaultCredentialsProvider를 명시적으로 사용하는 것을 권장합니다.
Region.of(region)호출 시 유효하지 않은 region 문자열이 입력되면IllegalArgumentException이 발생할 수 있습니다. 애플리케이션 시작 시 명확한 에러 메시지를 제공하기 위해 region 유효성을 검증하는 것이 좋습니다.credentials가 없을 때 암묵적으로
DefaultCredentialsProvider가 사용되지만, 명시적으로 설정하면 코드 의도가 더 명확해집니다.다음과 같이 개선할 수 있습니다:
+import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider; + @Bean public S3Client s3Client() { + // region 유효성 검증 + Region awsRegion; + try { + awsRegion = Region.of(region); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("유효하지 않은 AWS region: " + region, e); + } + if (accessKey == null || accessKey.isEmpty() || secretKey == null || secretKey.isEmpty()) { + // 명시적으로 DefaultCredentialsProvider 사용 (IAM Role, 환경 변수 등) return S3Client.builder() - .region(Region.of(region)) + .region(awsRegion) + .credentialsProvider(DefaultCredentialsProvider.create()) .build(); } AwsBasicCredentials awsCredentials = AwsBasicCredentials.create(accessKey, secretKey); return S3Client.builder() - .region(Region.of(region)) + .region(awsRegion) .credentialsProvider(StaticCredentialsProvider.create(awsCredentials)) .build(); }monew/src/main/java/com/spring/monew/common/filter/RequestIdFilter.java (1)
41-43: 분산 추적을 위한 기존 Request ID 수용 고려현재는 항상 새로운 Request ID를 생성하지만, 클라이언트나 API Gateway에서 이미
X-Request-ID헤더를 전달한 경우 이를 재사용하면 분산 시스템 전체에서 요청을 추적하기 더 용이합니다.다음과 같이 개선할 수 있습니다:
- private String generateRequestId() { - return REQUEST_ID_PREFIX + UUID.randomUUID().toString(); + private String generateRequestId(HttpServletRequest request) { + String existingRequestId = request.getHeader(REQUEST_ID_HEADER); + if (existingRequestId != null && !existingRequestId.isBlank()) { + return existingRequestId; + } + return REQUEST_ID_PREFIX + UUID.randomUUID().toString(); }그리고 호출부를 수정:
- String requestId = generateRequestId(); + String requestId = generateRequestId(request);monew/src/main/java/com/spring/monew/batch/processor/ArticleBackupProcessor.java (1)
18-27: 배치 프로세서 구현이 적절합니다.
Article을ArticleBackupDto로 변환하는 로직이 명확하며, 예외 처리도 Spring Batch의 재시도/스킵 메커니즘과 잘 통합됩니다.대량 백업 시 디버그 로그가 과도할 수 있습니다. 프로덕션 환경에서 로그 레벨을 조정하거나, 청크 단위로 한 번만 로깅하는 것을 고려하세요:
- log.debug("기사 {} 백업 준비 완료", article.getId()); + // 필요시 청크 레벨에서 로깅 (Writer에서 처리)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java (1)
34-36: 배치 실패 모니터링 강화를 고려하세요.현재 예외를 로그로만 기록하고 있습니다. 배치 작업이 반복적으로 실패할 경우를 대비하여 알림 메커니즘(예: 메트릭 발행, 알림 전송)을 추가하는 것을 권장합니다.
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (1)
24-32: 청크 처리 로직을 단순화할 수 있습니다.Line 26의 명시적 캐스팅과 Line 27의 null 필터는 불필요합니다.
ArticleBackupProcessor가 null을 반환하지 않으므로 방어 코드가 과도합니다.다음과 같이 단순화할 수 있습니다:
@Override public void write(Chunk<? extends ArticleBackupDto> chunk) { - List<ArticleBackupDto> articles = chunk.getItems().stream() - .map(dto -> (ArticleBackupDto) dto) - .filter(dto -> dto != null) - .toList(); + List<ArticleBackupDto> articles = chunk.getItems(); if (articles.isEmpty()) { return; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
monew/build.gradle(1 hunks)monew/src/main/java/com/spring/monew/article/controller/ArticleController.java(3 hunks)monew/src/main/java/com/spring/monew/article/domain/Article.java(1 hunks)monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java(1 hunks)monew/src/main/java/com/spring/monew/article/service/ArticleService.java(2 hunks)monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java(3 hunks)monew/src/main/java/com/spring/monew/articleview/controller/Test.java(0 hunks)monew/src/main/java/com/spring/monew/articleview/repository/ArticleViewRepository.java(2 hunks)monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java(1 hunks)monew/src/main/java/com/spring/monew/backup/dto/ArticleBackupDto.java(1 hunks)monew/src/main/java/com/spring/monew/backup/exception/BackupNotFoundException.java(1 hunks)monew/src/main/java/com/spring/monew/backup/exception/S3ServiceException.java(1 hunks)monew/src/main/java/com/spring/monew/backup/service/LogBackupService.java(1 hunks)monew/src/main/java/com/spring/monew/backup/service/S3BackupService.java(1 hunks)monew/src/main/java/com/spring/monew/backup/service/impl/S3BackupServiceImpl.java(1 hunks)monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java(1 hunks)monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java(1 hunks)monew/src/main/java/com/spring/monew/batch/config/LogBackupBatchConfig.java(1 hunks)monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java(1 hunks)monew/src/main/java/com/spring/monew/batch/processor/ArticleBackupProcessor.java(1 hunks)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java(1 hunks)monew/src/main/java/com/spring/monew/batch/scheduler/LogBackupScheduler.java(1 hunks)monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java(2 hunks)monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java(1 hunks)monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java(1 hunks)monew/src/main/java/com/spring/monew/common/config/AwsS3Config.java(1 hunks)monew/src/main/java/com/spring/monew/common/config/Test.java(0 hunks)monew/src/main/java/com/spring/monew/common/exception/GlobalExceptionHandler.java(2 hunks)monew/src/main/java/com/spring/monew/common/filter/RequestIdFilter.java(1 hunks)monew/src/main/java/com/spring/monew/common/logging/AuditLogger.java(1 hunks)monew/src/main/resources/application.yml(3 hunks)monew/src/main/resources/logback-spring.xml(1 hunks)
💤 Files with no reviewable changes (2)
- monew/src/main/java/com/spring/monew/common/config/Test.java
- monew/src/main/java/com/spring/monew/articleview/controller/Test.java
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-10-30T04:47:15.305Z
Learnt from: Namsoo315
Repo: codeit-monew/sb05-monew-team7 PR: 59
File: monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java:29-30
Timestamp: 2025-10-30T04:47:15.305Z
Learning: In the file monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java, the method `findIncludingDeleted(Param("commentId") UUID commentId)` is intentionally named this way even though the query `SELECT * FROM comments c WHERE c.id = :commentId AND c.is_deleted = true` only returns deleted records (is_deleted = true), not all records including deleted ones. This naming is by design and should not be flagged.
Applied to files:
monew/src/main/java/com/spring/monew/articleview/repository/ArticleViewRepository.javamonew/src/main/java/com/spring/monew/article/repository/ArticleRepository.javamonew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java
📚 Learning: 2025-10-31T05:05:50.992Z
Learnt from: yuhandemian
Repo: codeit-monew/sb05-monew-team7 PR: 69
File: monew/src/main/java/com/spring/monew/articleview/service/impl/ArticleViewServiceImpl.java:62-68
Timestamp: 2025-10-31T05:05:50.992Z
Learning: In ArticleViewServiceImpl.trackView(), the implementation intentionally distinguishes between view count (Article.viewCount) and view records (ArticleView in PostgreSQL, ActivityArticleViewDoc in MongoDB). View count increments at most once per user per 24 hours (Redis-controlled deduplication), while view records track all view events including duplicates within 24 hours for user activity history. When Redis indicates a duplicate but no recent DB record exists (edge case), the code correctly creates a new view record WITHOUT incrementing the count, respecting Redis as the source of truth for deduplication.
Applied to files:
monew/src/main/java/com/spring/monew/articleview/repository/ArticleViewRepository.javamonew/src/main/java/com/spring/monew/article/domain/Article.java
📚 Learning: 2025-10-29T09:36:40.948Z
Learnt from: Namsoo315
Repo: codeit-monew/sb05-monew-team7 PR: 57
File: monew/src/test/java/com/spring/monew/comment/repository/CommentRepositoryTest.java:175-197
Timestamp: 2025-10-29T09:36:40.948Z
Learning: In the file monew/src/test/java/com/spring/monew/comment/repository/CommentRepositoryTest.java, the content strings like "like0", "like5", "like3" in the sortByLikeCount test are just comment content labels, not indicators of the actual like counts. The actual like counts are controlled by incrementLikeCount() calls.
Applied to files:
monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java
📚 Learning: 2025-10-31T04:05:41.435Z
Learnt from: yuhandemian
Repo: codeit-monew/sb05-monew-team7 PR: 69
File: monew/src/main/java/com/spring/monew/activity/domain/ActivityCommentDoc.java:20-28
Timestamp: 2025-10-31T04:05:41.435Z
Learning: In the file monew/src/main/java/com/spring/monew/activity/domain/ActivityCommentDoc.java, the ActivityCommentDoc class intentionally does not include a commentId field because it is a denormalized MongoDB document designed to store snapshot data for user activity history. It is used to fetch comments, likes, and recently viewed articles in a single query without joins, so linking back to the original comment entity is not required.
Applied to files:
monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java
📚 Learning: 2025-10-28T02:10:38.287Z
Learnt from: yuhandemian
Repo: codeit-monew/sb05-monew-team7 PR: 37
File: monew/src/main/java/com/spring/monew/comment/controller/CommentController.java:57-65
Timestamp: 2025-10-28T02:10:38.287Z
Learning: The monew project currently does not implement Spring Security authentication - SecurityConfig exists but uses .anyRequest().permitAll(). All controllers use RequestHeader(name = "Monew-Request-User-ID") UUID userId to receive user identification, not AuthenticationPrincipal.
Applied to files:
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java
🧬 Code graph analysis (10)
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (1)
monew/src/main/java/com/spring/monew/batch/processor/ArticleBackupProcessor.java (1)
Component(11-28)
monew/src/main/java/com/spring/monew/batch/config/LogBackupBatchConfig.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-56)
monew/src/main/java/com/spring/monew/batch/processor/ArticleBackupProcessor.java (1)
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (1)
Component(15-46)
monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java (2)
monew/src/main/java/com/spring/monew/backup/exception/S3ServiceException.java (1)
S3ServiceException(3-11)monew/src/main/java/com/spring/monew/backup/service/impl/S3BackupServiceImpl.java (1)
Slf4j(24-152)
monew/src/main/java/com/spring/monew/common/exception/GlobalExceptionHandler.java (2)
monew/src/main/java/com/spring/monew/backup/exception/BackupNotFoundException.java (1)
BackupNotFoundException(5-21)monew/src/main/java/com/spring/monew/backup/exception/S3ServiceException.java (1)
S3ServiceException(3-11)
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (2)
monew/src/main/java/com/spring/monew/article/exception/ArticleNotFoundException.java (1)
ArticleNotFoundException(5-9)monew/src/main/java/com/spring/monew/common/exception/GlobalExceptionHandler.java (1)
Slf4j(17-107)
monew/src/main/java/com/spring/monew/batch/scheduler/LogBackupScheduler.java (1)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java (1)
Component(14-38)
monew/src/main/java/com/spring/monew/backup/service/impl/S3BackupServiceImpl.java (3)
monew/src/main/java/com/spring/monew/backup/exception/BackupNotFoundException.java (1)
BackupNotFoundException(5-21)monew/src/main/java/com/spring/monew/backup/exception/S3ServiceException.java (1)
S3ServiceException(3-11)monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java (1)
Slf4j(21-147)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java (2)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-56)monew/src/main/java/com/spring/monew/batch/scheduler/LogBackupScheduler.java (1)
Component(14-38)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (22)
monew/src/main/resources/logback-spring.xml (3)
12-13: ✅ LOG_PATTERN 속성이 파일 로깅용으로 적절하게 구성되었습니다.콘솔 로깅의
LOG_PATTERN_COLOR와 달리 색상 정보 없이 순수 로그 정보만 포함되어 파일 저장에 적합합니다. MDC 정보(requestId, requestMethod, requestUri)도 올바르게 포함되어 있습니다.
15-17: 📋 로그 파일 경로 설정 확인 필요합니다.기본값
./logs는 개발 환경에서는 적절하지만, ECS/컨테이너 환경에서 로그가 정상 저장되는지 확인이 필요합니다. 특히 S3 백업 배치(logback-spring.xml파일명 패턴monew-application-YYYY-MM-DD.log.gz)와 연동되는 부분이므로, 배치 작업이 올바른 경로에서 로그 파일을 감지하는지 검증해주세요.
26-37: ✅ RollingFileAppender 기본 구조는 적절합니다.일별 롤링 및 로그 파일 분리가 잘 설계되어 있으나, 위에서 지적한
maxHistory와totalSizeCap설정이 요구사항과 일치하는지 확인이 필요합니다.monew/src/main/resources/application.yml (2)
37-42: 배치 작업 활성화 방식 확인 필요.
spring.batch.job.enabled: false로 설정하면 Spring Batch Job이 자동 시작되지 않습니다. PR 요약에서 "기사 백업 배치(매일 02:00), 로그 백업 배치(매일 01:00)"에 자동 실행이 필요하다고 명시되어 있는데, 이것이@Scheduled애너테이션으로 구현되었는지, 아니면 다른 스케줄링 메커니즘으로 구현되었는지 확인이 필요합니다.
- 만약 Spring Batch Job으로 배치를 구현했다면, 본 설정은 자동 실행을 방해할 것입니다.
- 만약
@Scheduled방식으로 구현했다면, 이 설정은 문제없습니다.
108-114: AWS 자격증명 설정 확인 및 문서화 필요.AWS
access-key와secret-key의 기본값이 빈 문자열("")로 설정되어 있습니다. 로컬 개발 환경에서는 문제가 없을 수 있으나, 스테이징/프로덕션 환경에서는AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY환경변수를 반드시 설정해야 합니다.이러한 필수 환경변수 설정이 배포 문서나 환경 설정 가이드에 명확하게 문서화되어 있는지 확인이 필요합니다. 또한, 환경에 따라 다른 기본값을 사용하는 프로필 설정(예:
application-prod.yml) 사용을 고려해보시기 바랍니다.monew/src/main/java/com/spring/monew/common/filter/RequestIdFilter.java (1)
19-21: 상수 정의가 적절합니다.헤더명, MDC 키, 프리픽스 모두 명확하고 일관성 있게 정의되었습니다.
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (2)
15-15: RequestIdFilter 의존성 주입이 올바르게 구성되었습니다.생성자 주입 방식으로 필터를 적절히 주입하고 있습니다.
Also applies to: 23-23
45-46: 필터 체인 순서가 적절하게 구성되었습니다.
RequestIdFilter가HeaderAuthFilter보다 먼저 실행되도록 올바르게 등록되어, 모든 후속 필터와 컨트롤러에서 MDC를 통해 Request ID에 접근할 수 있습니다. 이는 감사 로깅과 요청 추적에 필수적입니다.monew/src/main/java/com/spring/monew/batch/scheduler/LogBackupScheduler.java (2)
23-37: 배치 작업 실행 로직이 적절합니다.스케줄러가 JobParameters를 구성하고 배치 작업을 실행하는 방식이 ArticleBackupScheduler와 일관성 있게 구현되어 있습니다. 예외 처리도 스케줄 작업에 적합합니다.
22-22: PR 설명 내용 확인 필요코드 검증 결과:
- LogBackupScheduler.java 라인 22의 cron 표현식은
"0 0 3 * * *"로 확인됨 (매일 03:00 실행)- 코드베이스에서 "01:00" 관련 참조 없음
다만 PR 설명에서 실제로 "01:00"이라고 명시하는지 확인할 수 없으므로, 다음을 수동으로 확인해야 합니다:
- PR 설명(PR description)에 기재된 실제 예정 시간
- 의도한 스케줄이 01:00인지 03:00인지 확인
의도가 01:00이면 cron 수정이 필요하고, 03:00이 의도라면 PR 설명을 수정해야 합니다.
monew/src/main/java/com/spring/monew/backup/exception/BackupNotFoundException.java (1)
5-21: 예외 클래스 구현이 깔끔합니다.백업 날짜 정보를 포함하는 예외 클래스 설계가 적절하며, 디버깅에 필요한 컨텍스트를 제공합니다.
monew/src/main/java/com/spring/monew/articleview/repository/ArticleViewRepository.java (2)
26-28: 하드 삭제 메서드가 적절히 구현되었습니다.
deleteByArticleId가 기사 하드 삭제 시 연관된 조회 기록을 정리하는 역할을 수행합니다.@Modifying(clearAutomatically = true)사용으로 영속성 컨텍스트 동기화도 보장됩니다.
30-31: 카운트 조회 메서드 추가를 확인했습니다.특정 기사의 조회 수를 집계하는 메서드가 추가되었습니다. 통계나 검증 용도로 활용될 수 있습니다.
monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java (2)
36-41: 기사별 댓글 삭제 및 카운트 메서드가 추가되었습니다.하드 삭제 지원을 위한
deleteByArticleId와 통계 수집을 위한countByArticleId가 적절히 구현되었습니다.
32-34: FK 제약 조건 확인 완료 - 삭제 순서가 올바르게 구현되었습니다.
ArticleServiceImpl.hardDeleteArticle()메서드에서 다음과 같이 올바른 순서로 삭제가 수행됩니다:
commentRepository.deleteCommentLikesByArticleId(articleId)- 댓글 좋아요 삭제commentRepository.deleteByArticleId(articleId)- 댓글 삭제articleViewRepository.deleteByArticleId(articleId)- 조회 기록 삭제articleRepository.hardDelete(articleId)- 기사 삭제
@Transactional(isolation = Isolation.SERIALIZABLE)어노테이션으로 트랜잭션 안정성이 보장되며, FK 제약 조건으로 인한 오류는 발생하지 않습니다. 코드 변경 사항이 올바르게 구현되었습니다.monew/src/main/java/com/spring/monew/backup/exception/S3ServiceException.java (1)
3-11: S3 예외 클래스가 적절히 구현되었습니다.S3 작업 실패를 래핑하는 표준 예외 패턴이 올바르게 적용되었습니다. GlobalExceptionHandler와의 통합도 적절합니다.
monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java (1)
47-55: 로그 백업 트리거 엔드포인트가 추가되었습니다.로그 백업 수동 실행 기능이 명확히 구현되었습니다.
기사 백업 엔드포인트와 동일하게, 관리자 권한 검증이 필요한지 확인하세요.
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java (1)
22-33: 배치 작업 스케줄링 구현이 올바릅니다.일일 02:00에 실행되는 크론 표현식과 작업 파라미터 구성이 요구사항과 일치합니다. LogBackupScheduler와 일관된 패턴을 사용하고 있어 유지보수성이 좋습니다.
monew/src/main/java/com/spring/monew/backup/service/S3BackupService.java (1)
9-15: 인터페이스 설계가 명확하고 간결합니다.백업 업로드, 다운로드, 존재 확인의 세 가지 핵심 기능을 잘 정의하고 있습니다.
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
33-48: 배치 작업 구성이 올바릅니다.Job과 Step 정의가 Spring Batch 모범 사례를 따르고 있으며, 청크 크기 100은 적절합니다.
monew/src/main/java/com/spring/monew/article/service/ArticleService.java (1)
4-4: 인터페이스 확장이 깔끔합니다.소프트 삭제, 하드 삭제, 백업 복원 메서드가 명확하게 정의되어 있으며, 요구사항을 잘 반영하고 있습니다.
Also applies to: 29-33
monew/src/main/java/com/spring/monew/backup/service/LogBackupService.java (1)
5-11: 로그 백업 인터페이스가 명확합니다.로그 업로드, 로컬 정리, 존재 확인의 세 가지 기능이 잘 정의되어 있습니다.
| @GetMapping("/restore") | ||
| @Operation( | ||
| summary = "백업에서 기사 복원", | ||
| description = "S3 백업에서 지정된 날짜 범위의 누락된 기사를 복원합니다. 최대 31일 범위까지 가능합니다." | ||
| ) | ||
| public ResponseEntity<ArticleRestoreResultDto> restoreArticles( | ||
| @Parameter(description = "시작 날짜", required = true) | ||
| @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant from, | ||
|
|
||
| @Parameter(description = "종료 날짜", required = true) | ||
| @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant to, | ||
|
|
||
| Principal principal | ||
| ) { | ||
| UUID userId = userExtractor.extractUserId(principal); | ||
| ArticleRestoreResultDto result = articleService.restoreArticlesFromBackup(from, to, userId); | ||
| return ResponseEntity.ok(result); | ||
| } |
There was a problem hiding this comment.
HTTP 메서드 선택 오류 – 상태 변경은 GET으로 노출하면 안 됩니다.
restoreArticles는 백업 데이터를 실제로 복원하여 시스템 상태를 변경하지만, 현재 @GetMapping으로 노출되어 있습니다. GET 요청은 캐시·프리페치·검색봇 등에 의해 의도치 않게 호출될 수 있고, HTTP 명세상 부작용이 없어야 하기 때문에 안전하지 않습니다. 복원처럼 상태를 바꾸는 작업은 POST(또는 PUT/PATCH) 등 안전하지 않은 메서드로 전환해 주세요.
- @GetMapping("/restore")
+ @PostMapping("/restore")📝 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.
| @GetMapping("/restore") | |
| @Operation( | |
| summary = "백업에서 기사 복원", | |
| description = "S3 백업에서 지정된 날짜 범위의 누락된 기사를 복원합니다. 최대 31일 범위까지 가능합니다." | |
| ) | |
| public ResponseEntity<ArticleRestoreResultDto> restoreArticles( | |
| @Parameter(description = "시작 날짜", required = true) | |
| @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant from, | |
| @Parameter(description = "종료 날짜", required = true) | |
| @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant to, | |
| Principal principal | |
| ) { | |
| UUID userId = userExtractor.extractUserId(principal); | |
| ArticleRestoreResultDto result = articleService.restoreArticlesFromBackup(from, to, userId); | |
| return ResponseEntity.ok(result); | |
| } | |
| @PostMapping("/restore") | |
| @Operation( | |
| summary = "백업에서 기사 복원", | |
| description = "S3 백업에서 지정된 날짜 범위의 누락된 기사를 복원합니다. 최대 31일 범위까지 가능합니다." | |
| ) | |
| public ResponseEntity<ArticleRestoreResultDto> restoreArticles( | |
| @Parameter(description = "시작 날짜", required = true) | |
| @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant from, | |
| @Parameter(description = "종료 날짜", required = true) | |
| @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant to, | |
| Principal principal | |
| ) { | |
| UUID userId = userExtractor.extractUserId(principal); | |
| ArticleRestoreResultDto result = articleService.restoreArticlesFromBackup(from, to, userId); | |
| return ResponseEntity.ok(result); | |
| } |
🤖 Prompt for AI Agents
In
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java
around lines 138 to 155, the restoreArticles endpoint is currently exposed with
@GetMapping which is inappropriate for a state-changing operation; change the
mapping to a non-safe HTTP verb (e.g., @PostMapping("/restore")) and update any
relevant method-level annotations/docs if needed (OpenAPI summary/description
can remain), ensure any callers/tests that invoke this endpoint use POST, and if
your API expects complex input in the body consider switching request parameters
to a request body DTO; keep authentication/parameter extraction (userId)
unchanged.
monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java
Show resolved
Hide resolved
| @Operation( | ||
| summary = "기사 백업 배치 작업 수동 트리거", | ||
| description = "articleBackupJob을 수동으로 실행하여 모든 기사를 S3에 백업합니다." | ||
| ) | ||
| @PostMapping("/trigger/article-backup") | ||
| public ResponseEntity<BatchJobTriggerResponse> triggerArticleBackupJob() { | ||
| BatchJobTriggerResponse response = batchJobService.triggerArticleBackupJob(); | ||
| return ResponseEntity.ok(response); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
기사 백업 트리거 엔드포인트가 추가되었습니다.
수동 백업 트리거 기능이 기존 패턴과 일관되게 구현되었으며, Swagger 문서화도 적절합니다.
배치 작업 트리거는 민감한 작업이므로, 관리자 권한이 필요한지 확인하세요. 전역 보안 설정이나 메서드 레벨 보안이 적용되어 있는지 검증하세요:
🏁 Script executed:
#!/bin/bash
# 보안 설정 확인 - SecurityConfig나 전역 필터 검색
rg -n "@PreAuthorize|@Secured|hasRole|permitAll" --type java -g "*Security*" -A 3
# 배치 컨트롤러 경로에 대한 보안 설정 확인
rg -n "/api/batch" --type java -g "*Security*" -C 3Length of output: 2000
배치 작업 엔드포인트의 보안 설정이 미흡합니다 - 즉시 수정 필요
확인 결과, SecurityConfig.java 라인 42에서 /api/batch/** 경로가 .permitAll()로 설정되어 있습니다. 이는 인증 없이 누구나 기사 백업 배치 작업을 트리거할 수 있다는 의미입니다.
모든 기사를 S3에 백업하는 민감한 작업이므로 관리자 권한만 실행 가능하도록 수정이 필수입니다:
SecurityConfig.java라인 42:/api/batch/**를hasRole('ADMIN')또는 유사한 권한 검증으로 변경- 또는
@PreAuthorize애노테이션을BatchJobController의 메서드에 추가하여 메서드 레벨 보안 적용
🤖 Prompt for AI Agents
In monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java
lines 37-45 the triggerArticleBackupJob endpoint is publicly exposed; update
security so only admins can call it by either: (A) change
monew/src/main/java/com/spring/monew/config/SecurityConfig.java at/around line
42 to replace .permitAll() for "/api/batch/**" with an authority check such as
.hasRole("ADMIN") (ensure role prefix usage matches your config), or (B) add
method-level security: annotate triggerArticleBackupJob with
@PreAuthorize("hasRole('ADMIN')") and enable pre/post annotations in your
security config (e.g., @EnableGlobalMethodSecurity(prePostEnabled = true));
ensure necessary imports and that roles used match existing user authorities.
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java
Outdated
Show resolved
Hide resolved
| @Value("${aws.s3.access-key}") | ||
| private String accessKey; | ||
|
|
||
| @Value("${aws.s3.secret-key}") | ||
| private String secretKey; |
There was a problem hiding this comment.
프로덕션 환경에서 credentials를 평문으로 저장하는 것은 보안 위험입니다.
평문 프로퍼티로 AWS credentials를 주입하면 소스 코드, 설정 파일, 환경 변수 등에 노출될 위험이 있습니다. 프로덕션 환경에서는 다음 방안을 권장합니다:
- IAM Role 기반 인증(EC2, ECS, EKS 등에서 instance profile 사용)
- AWS Secrets Manager 또는 Systems Manager Parameter Store를 통한 암호화된 credentials 관리
- Spring Cloud Config의 암호화 기능 활용
또한 lines 31-34에서 StaticCredentialsProvider를 사용하는 것은 개발 환경에는 적합하지만, 프로덕션에서는 DefaultCredentialsProvider 체인을 사용하여 AWS SDK가 자동으로 credential source를 탐지하도록 하는 것이 best practice입니다.
프로덕션 배포 전에 다음과 같이 수정하는 것을 고려하세요:
+ // 프로덕션: IAM Role 또는 DefaultCredentialsProvider 사용 권장
@Bean
public S3Client s3Client() {
- if (accessKey == null || accessKey.isEmpty() || secretKey == null || secretKey.isEmpty()) {
- return S3Client.builder()
- .region(Region.of(region))
- .build();
- }
-
- AwsBasicCredentials awsCredentials = AwsBasicCredentials.create(accessKey, secretKey);
- return S3Client.builder()
- .region(Region.of(region))
- .credentialsProvider(StaticCredentialsProvider.create(awsCredentials))
- .build();
+ // 프로덕션: credentials 제거하고 IAM Role 활용
+ return S3Client.builder()
+ .region(Region.of(region))
+ .build(); // DefaultCredentialsProvider 체인 자동 사용
}개발 환경에서만 명시적 credentials가 필요한 경우, profile 또는 환경별 분기 처리를 추가하세요.
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (1)
152-169: 상태 변경 작업은 GET 메서드로 노출하면 안 됩니다. (과거 리뷰 지적사항 미반영)이 엔드포인트는 과거 리뷰에서 이미 지적된 문제가 해결되지 않았습니다.
restoreArticles는 S3 백업에서 기사를 복원하여 시스템 상태를 변경하는 작업이지만@GetMapping으로 노출되어 있습니다. GET 요청은 HTTP 명세상 안전한(safe) 메서드로, 부작용이 없어야 하며 캐시·프리페치·검색봇 등에 의해 의도치 않게 호출될 수 있습니다. 복원처럼 상태를 변경하는 작업은 반드시POST메서드로 변경해야 합니다.다음 diff를 적용하세요:
- @GetMapping("/restore") + @PostMapping("/restore") @Operation( summary = "백업에서 기사 복원", description = "S3 백업에서 지정된 날짜 범위의 누락된 기사를 복원합니다. 최대 31일 범위까지 가능합니다." ) + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "복원 성공"), + @ApiResponse(responseCode = "400", description = "잘못된 날짜 범위"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") + }) public ResponseEntity<ArticleRestoreResultDto> restoreArticles(
🧹 Nitpick comments (3)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (3)
128-138: ApiResponses 문서화 추가를 권장합니다.소프트 삭제 엔드포인트의 REST 설계는 적절하나(DELETE 메서드, 204 응답), 다른 엔드포인트와의 일관성을 위해 OpenAPI 문서화를 추가하는 것이 좋습니다.
다음 diff를 적용하여 문서화를 추가하세요:
@DeleteMapping("/{articleId}") @ResponseStatus(HttpStatus.NO_CONTENT) @Operation(summary = "뉴스 기사 삭제", description = "특정 기사를 소프트 삭제합니다 (논리 삭제)") + @ApiResponses({ + @ApiResponse(responseCode = "204", description = "삭제 성공"), + @ApiResponse(responseCode = "404", description = "기사를 찾을 수 없음"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") + }) public void softDeleteArticle(
140-150: ApiResponses 문서화 추가를 권장합니다.하드 삭제 엔드포인트의 REST 설계는 적절하나, 다른 엔드포인트와의 일관성을 위해 OpenAPI 문서화를 추가하는 것이 좋습니다.
다음 diff를 적용하여 문서화를 추가하세요:
@DeleteMapping("/{articleId}/hard") @ResponseStatus(HttpStatus.NO_CONTENT) @Operation(summary = "뉴스 기사 영구 삭제", description = "특정 기사 및 관련된 모든 데이터(댓글, 좋아요, 조회수)를 데이터베이스에서 완전히 삭제합니다") + @ApiResponses({ + @ApiResponse(responseCode = "204", description = "삭제 성공"), + @ApiResponse(responseCode = "404", description = "기사를 찾을 수 없음"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") + }) public void hardDeleteArticle(
152-169: 날짜 범위 검증 추가를 권장합니다.설명에 "최대 31일 범위까지 가능합니다"라고 명시되어 있으나, 컨트롤러 레벨에서 날짜 범위에 대한 검증이 없습니다. 잘못된 요청을 조기에 차단하기 위해
from < to검증 및 최대 31일 범위 제한을 추가하는 것이 좋습니다.컨트롤러 메서드 시작 부분에 검증 로직을 추가하거나, 서비스 레이어에서 검증을 수행하고 적절한 예외를 발생시키도록 구현하세요:
public ResponseEntity<ArticleRestoreResultDto> restoreArticles( @Parameter(description = "시작 날짜", required = true) @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant from, @Parameter(description = "종료 날짜", required = true) @RequestParam @DateTimeFormat(iso = DateTimeFormat.ISO.DATE_TIME) Instant to, Principal principal ) { // 날짜 범위 검증 if (from.isAfter(to)) { throw new IllegalArgumentException("시작 날짜는 종료 날짜보다 이전이어야 합니다"); } long daysDiff = Duration.between(from, to).toDays(); if (daysDiff > 31) { throw new IllegalArgumentException("날짜 범위는 최대 31일까지 가능합니다"); } UUID userId = userExtractor.extractUserId(principal); ArticleRestoreResultDto result = articleService.restoreArticlesFromBackup(from, to, userId); return ResponseEntity.ok(result); }또는 서비스 레이어에서 검증을 수행하는 방식을 선택할 수 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-60)
🔇 Additional comments (2)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (2)
1-34: 임포트 및 컨트롤러 태그 업데이트 적절합니다.새로운 삭제/복원 기능에 필요한 임포트와 API 태그 설명이 정확하게 추가되었습니다. 특히 태그를 "뉴스 기사 관리"로 변경한 것은 확장된 기능 범위를 잘 반영하고 있습니다.
40-126: OpenAPI 문서화 개선이 좋습니다.기존 엔드포인트에 적절한 응답 코드와 설명이 추가되어 API 문서 품질이 향상되었습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java (2)
43-51: 이전 리뷰에서 지적된 보안 문제가 해결되지 않았습니다이전 리뷰에서 지적된 바와 같이,
/api/batch/**경로가SecurityConfig에서.permitAll()로 설정되어 있어 인증 없이 누구나 이 엔드포인트를 호출할 수 있습니다. 모든 기사를 S3에 백업하는 민감한 작업이므로 관리자 권한으로 제한해야 합니다.추가 개선 사항:
일관성을 위해
@ApiResponses어노테이션을 추가하는 것을 고려하세요 (37-41라인의triggerNewsCollectionJob과 동일한 패턴):@Operation( summary = "기사 백업 배치 작업 수동 트리거", description = "articleBackupJob을 수동으로 실행하여 모든 기사를 S3에 백업합니다." ) +@ApiResponses({ + @ApiResponse(responseCode = "200", description = "배치 작업 성공"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") +}) @PostMapping("/trigger/article-backup") public ResponseEntity<BatchJobTriggerResponse> triggerArticleBackupJob() {
53-61: 동일한 보안 문제가 적용됩니다이 엔드포인트도 43-51라인의 기사 백업 엔드포인트와 동일한 보안 문제가 있습니다. 로그 파일을 S3에 백업하는 작업은 민감한 정보(애플리케이션 로그, 클라이언트 IP 등)를 다루므로 관리자 권한이 필요합니다.
추가 권장 사항:
- 일관성을 위해
@ApiResponses어노테이션 추가- 반복적인 배치 작업 트리거를 방지하기 위한 rate limiting 고려 (예: 동일 작업을 5분 내 재실행 방지)
@Operation( summary = "로그 백업 배치 작업 수동 트리거", description = "logBackupJob을 수동으로 실행하여 로그 파일을 S3에 백업합니다." ) +@ApiResponses({ + @ApiResponse(responseCode = "200", description = "배치 작업 성공"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") +}) @PostMapping("/trigger/log-backup") public ResponseEntity<BatchJobTriggerResponse> triggerLogBackupJob() {monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (1)
155-172: [중요] GET 메서드 사용 오류 – 이전 리뷰에서 지적된 문제가 수정되지 않았습니다.복원 작업은 시스템 상태를 변경하는 작업이므로 GET 메서드를 사용하면 안 됩니다. GET 요청은 다음과 같은 위험이 있습니다:
- 브라우저 프리페치나 검색 엔진 크롤러에 의해 의도치 않게 실행될 수 있음
- 브라우저 캐시로 인한 예상치 못한 동작
- 북마크나 링크 공유로 인한 우발적 실행
이전 리뷰에서도 동일한 문제가 지적되었으나 수정되지 않았습니다. 반드시 POST 메서드로 변경해야 합니다.
- @GetMapping("/restore") + @PostMapping("/restore") @Operation( summary = "백업에서 기사 복원", description = "S3 백업에서 지정된 날짜 범위의 누락된 기사를 복원합니다. 최대 31일 범위까지 가능합니다." ) + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "복원 성공"), + @ApiResponse(responseCode = "400", description = "잘못된 날짜 범위"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") + }) public ResponseEntity<ArticleRestoreResultDto> restoreArticles(추가로, 날짜 범위 검증(from < to, 범위 <= 31일)이 서비스 레이어에 구현되어 있는지 확인하시기 바랍니다.
🧹 Nitpick comments (2)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (2)
131-141: 소프트 삭제 엔드포인트 구현이 적절합니다.DELETE 메서드 사용과 204 응답 코드가 올바르게 설정되었습니다. 다만 일관성을 위해 다른 엔드포인트들처럼
@ApiResponses애노테이션을 추가하는 것을 권장합니다.@DeleteMapping("/{articleId}") @ResponseStatus(HttpStatus.NO_CONTENT) @Operation(summary = "뉴스 기사 삭제", description = "특정 기사를 소프트 삭제합니다 (논리 삭제)") + @ApiResponses({ + @ApiResponse(responseCode = "204", description = "삭제 성공"), + @ApiResponse(responseCode = "404", description = "기사를 찾을 수 없음"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") + }) public void softDeleteArticle(
143-153: 하드 삭제 엔드포인트 구현이 적절합니다.영구 삭제 동작에 대한 명확한 설명과 올바른 HTTP 메서드를 사용하고 있습니다. 다만 일관성을 위해
@ApiResponses애노테이션을 추가하는 것을 권장합니다.@DeleteMapping("/{articleId}/hard") @ResponseStatus(HttpStatus.NO_CONTENT) @Operation(summary = "뉴스 기사 영구 삭제", description = "특정 기사 및 관련된 모든 데이터(댓글, 좋아요, 조회수)를 데이터베이스에서 완전히 삭제합니다") + @ApiResponses({ + @ApiResponse(responseCode = "204", description = "삭제 성공"), + @ApiResponse(responseCode = "404", description = "기사를 찾을 수 없음"), + @ApiResponse(responseCode = "500", description = "서버 내부 오류") + }) public void hardDeleteArticle(
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java(5 hunks)monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (1)
monew/src/main/java/com/spring/monew/article/controller/ArticleController.java (1)
4-29: 새로운 엔드포인트를 위한 import 추가가 적절합니다.복원 및 삭제 기능을 위한 필요한 클래스들이 올바르게 import되었습니다.
CommentRepository 패턴과 일관성을 위해 is_deleted = true 조건 추가
업로드 성공 후 로컬 파일 삭제를 별도 try-catch로 분리하여 정확한 실패 지점 보고
UTC 자정 대신 Asia/Seoul 타임존 기준으로 날짜 범위 계산하여 백업 누락 방지
메서드 레벨 트랜잭션 제거하고 ArticleService로 위임하여 개별 트랜잭션 처리 장시간 트랜잭션으로 인한 잠금/타임아웃 문제 해결
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
59-64: 타임존 처리는 개선되었으나, 설정 가능하도록 변경을 권장합니다.
ZoneId.of("Asia/Seoul")을 사용하여 이전 리뷰의 UTC 타임존 문제를 해결했습니다. 하지만 타임존이 하드코딩되어 있어 배포 환경이나 요구사항 변경 시 유연성이 떨어집니다.다음과 같이 설정 가능하도록 변경하는 것을 권장합니다:
+ @Value("${backup.timezone:Asia/Seoul}") + private String backupTimezone; + @Bean @StepScope public JpaPagingItemReader<Article> articleBackupReader() { - ZoneId zoneId = ZoneId.of("Asia/Seoul"); + ZoneId zoneId = ZoneId.of(backupTimezone); LocalDate yesterday = LocalDate.now(zoneId).minusDays(1);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java(1 hunks)monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java(1 hunks)monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java(1 hunks)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java
- monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java
🧰 Additional context used
🧬 Code graph analysis (2)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-60)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-60)
🔇 Additional comments (4)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (2)
23-24: 이전 트랜잭션 문제가 해결되었습니다.과거 리뷰에서 제기된 메서드 레벨
@Transactional어노테이션이 제거되어, 장시간 트랜잭션으로 인한 성능 및 안정성 문제가 해결되었습니다. 현재는 서비스 계층에 삭제 로직을 위임하는 권장 패턴을 따르고 있습니다.
23-23: 스케줄 실행 시간이 다른 정리 작업들과 불일치합니다.ArticleCleanupScheduler가 15:00(오후 3시)에 실행되도록 설정되어 있는데, 프로젝트의 다른 정리 작업들은 모두 야간 시간대(00:00~03:00)에 실행됩니다:
- CommentCleanupScheduler: 00:00
- UserCleanupScheduler: 00:00
- NotificationCleanupScheduler: 03:00
이것이 의도된 설정인지 확인해 주시기 바랍니다. 필요시 다른 정리 작업들과 일관성 있게 조정을 검토해 주세요.
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (2)
38-54: 배치 작업 구조가 올바르게 구성되어 있습니다.Job과 Step 구성이 적절하며, chunk 크기(100)와 reader의 page 크기가 일치합니다. Writer를 listener로도 등록한 것은 lifecycle 훅을 위한 적절한 패턴입니다.
73-73: 파일 이름 패턴을 잘못 파악했습니다. 검토 코멘트는 부정확합니다.실제 코드를 확인한 결과, S3BackupServiceImpl.java의
generateS3Key()메서드는 파일명을articles/YYYY-MM-DD.json.gz형식으로 생성합니다. 리뷰 코멘트에서 가정한deleted-articles-YYYY-MM-DD.json패턴이 아닙니다.코드는 일관성 있게 동작합니다:
- 파일명 패턴(
articles/YYYY-MM-DD.json.gz) → 일반 기사 백업- 쿼리(
isDeleted = false) → 삭제되지 않은 기사만 선택- 날짜 필터(
createdAt) → 어제 생성된 기사 대상이 배치는 어제 생성된 활성(삭제되지 않은) 기사를 백업하려는 명확한 의도를 가지고 있으며, 현재 구현이 그 의도를 올바르게 반영하고 있습니다.
Likely an incorrect or invalid review comment.
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java
Outdated
Show resolved
Hide resolved
배치 작업에서 시스템 사용자 ID(00000000-0000-0000-0000-000000000000) 사용 MDC에 배치 requestId 설정하여 감사 로그 추적성 향상
monew-application-2025-11-03.log → monew-application.2025-11-03.log logback-spring.xml의 fileNamePattern과 동일한 형식 사용
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java (1)
127-129: 로그 파일 네이밍 규칙 불일치로 인해 기능이 작동하지 않습니다.Line 128의
generateLogFilePath가logFileName + "." + logDate로 경로를 생성하여monew-application.2025-11-02.log형태의 파일명을 만들지만,extractDateFromFileName(lines 145-150)은 마지막 하이픈(-) 이후 문자열로 날짜를 파싱합니다. PR 설명과 Logback 롤링 패턴은monew-application-YYYY-MM-DD.log형태(하이픈 구분)를 명시하고 있습니다.이 불일치로 인해:
uploadLogFile이 실제 로그 파일을 찾지 못함 (항상 line 51-54에서 조기 리턴)cleanupLocalLogs의 날짜 파싱 실패로 오래된 로그 삭제 불가logFileExists가 항상 false 반환이전 리뷰에서 동일한 이슈가 지적되고 해결된 것으로 표시되었으나, 코드에는 여전히 점(
.) 연결 방식이 남아있습니다.다음 diff를 적용하여 하이픈 기반 패턴으로 수정하세요:
private String generateLogFilePath(LocalDate logDate) { - return Paths.get(logFilePath, logFileName + "." + logDate.format(DATE_FORMATTER) + ".log").toString(); + return Paths.get(logFilePath, logFileName + "-" + logDate.format(DATE_FORMATTER) + ".log").toString(); }
🧹 Nitpick comments (4)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (4)
21-21: 시스템 사용자 ID에 대한 문서화를 추가하는 것이 좋습니다.all-zeros UUID를 시스템 사용자 ID로 사용하는 것은 합리적이지만, 이것이 실제 데이터베이스의 사용자 테이블에 존재하는 특수 사용자인지, 아니면 단순히 감사 로그의 마커로만 사용되는지 명확하지 않습니다. Javadoc 주석을 추가하여 이 UUID의 의미와 사용 목적을 문서화하면 유지보수성이 향상됩니다.
예시:
+ /** + * 시스템에 의해 실행되는 배치 작업의 사용자 ID. + * 감사 로그에서 사용자 작업과 배치 작업을 구분하기 위해 사용됩니다. + */ private static final UUID SYSTEM_USER_ID = UUID.fromString("00000000-0000-0000-0000-000000000000");
27-27: 클린업 작업 실행 시간을 오프피크 시간대로 조정하는 것을 권장합니다.현재 스케줄러는 매일 15:00 (오후 3시)에 실행되도록 설정되어 있습니다. 대용량 삭제 작업은 데이터베이스 리소스를 많이 사용하므로, 사용자 트래픽이 적은 새벽 시간대(예: 03:00-04:00)에 실행하는 것이 시스템 성능과 사용자 경험에 더 유리합니다.
예시:
- @Scheduled(cron = "0 0 15 * * *") + @Scheduled(cron = "0 0 3 * * *") // 새벽 3시 실행 public void deleteSoftDeletedArticles() {
36-36: 클린업 임계값을 설정 가능하게 외부화하는 것을 권장합니다.30일 임계값이 하드코딩되어 있어 운영 환경에서 유연하게 조정할 수 없습니다. 비즈니스 요구사항이나 스토리지 정책 변경 시 코드 수정과 재배포가 필요합니다.
application.yml에 설정 추가:batch: cleanup: retention-days: 30코드 수정:
+ @Value("${batch.cleanup.retention-days:30}") + private int retentionDays; + @Scheduled(cron = "0 0 15 * * *") public void deleteSoftDeletedArticles() { // ... - Instant threshold = Instant.now().minus(30, ChronoUnit.DAYS); + Instant threshold = Instant.now().minus(retentionDays, ChronoUnit.DAYS);
49-57: 대량 기사 처리 시 안전장치 추가를 고려하세요.현재 구현은 조회된 모든 기사를 순차적으로 처리합니다. 일반적으로는 문제가 없지만, 예외적으로 수천 개의 기사가 대상이 될 경우 배치 실행 시간이 매우 길어지고 중간에 실패 시 재처리가 어려워질 수 있습니다.
최대 배치 크기 제한을 추가하는 것을 고려하세요:
+ @Value("${batch.cleanup.max-batch-size:1000}") + private int maxBatchSize; + // ... List<UUID> articleIds = articleRepository.findSoftDeletedBefore(threshold); if (articleIds.isEmpty()) { log.info("[Batch] 정리할 삭제된 게시글이 없습니다."); return; } + if (articleIds.size() > maxBatchSize) { + log.warn("[Batch] 대상 게시글 {}개가 최대 배치 크기 {}를 초과하여 일부만 처리합니다.", + articleIds.size(), maxBatchSize); + articleIds = articleIds.subList(0, maxBatchSize); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java(1 hunks)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-60)
monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java (2)
monew/src/main/java/com/spring/monew/backup/exception/S3ServiceException.java (1)
S3ServiceException(3-11)monew/src/main/java/com/spring/monew/backup/service/impl/S3BackupServiceImpl.java (1)
Slf4j(24-152)
🔇 Additional comments (6)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (2)
29-34: 배치 컨텍스트 초기화가 잘 구현되었습니다.배치 요청 ID를 생성하고 MDC에 설정하여 로그 추적성을 보장하는 구현이 적절합니다. 시작 시간 기록과 로깅도 명확합니다.
59-64: 배치 완료 처리 및 리소스 정리가 올바르게 구현되었습니다.실행 시간 측정, 성공/실패 통계 로깅, 그리고 finally 블록에서의 MDC 정리가 모두 적절하게 구현되었습니다. 예외 발생 시에도 MDC가 확실히 정리되어 메모리 누수를 방지합니다.
monew/src/main/java/com/spring/monew/backup/service/impl/S3LogBackupServiceImpl.java (4)
82-87: 로컬 파일 삭제 오류 처리가 올바르게 분리되었습니다.S3 업로드 성공 후 로컬 파일 삭제를 별도의 try-catch 블록으로 분리하여, 삭제 실패 시에도 예외를 던지지 않고 로그만 남기도록 구현되었습니다. 업로드 성공과 삭제 실패를 명확히 구분하여 운영 혼란을 방지합니다.
131-133: LGTM!S3 키 생성 로직이 올바르게 구현되었습니다.
logs/YYYY-MM-DD.log.gz형태로 일관된 키를 생성합니다.
135-141: LGTM!GZIP 압축 로직이 표준 패턴을 따르며, try-with-resources로 리소스 관리가 적절하게 구현되었습니다. 초기 용량 설정을 통한 최적화도 적용되어 있습니다.
143-155: 날짜 파싱 로직은 올바르게 구현되었으나 generateLogFilePath 수정에 의존합니다.
extractDateFromFileName은 하이픈 기반 파일명에서 날짜를 추출하는 로직이 올바르게 구현되어 있습니다:
- 마지막 하이픈 위치를 찾아 날짜 부분을 추출
- 적절한 경계 검사로 IndexOutOfBoundsException 방지
- 파싱 실패 시 null 반환
다만,
generateLogFilePath(line 127-129)가 점(.) 기반 파일명을 생성하므로, 해당 메서드가 수정되어야 이 파싱 로직이 정상 작동합니다.
yuhandemian
force-pushed
the
feature/#84
branch
from
cc680eb to
7e357cc
Compare
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 (2)
monew/src/main/resources/application.yml (1)
16-27: 운영 환경에서 위험한 설정 - 즉시 수정 필요현재 설정은 매우 위험합니다:
ddl-auto: none+init.mode: always: 애플리케이션 시작 시마다schema.sql이 실행됩니다.schema.sql은DROP TABLE명령을 포함하고 있어 모든 데이터가 삭제됩니다.권장 조치:
프로파일별 설정 분리:
# application-local.yml spring: jpa: hibernate: ddl-auto: create-drop sql: init: mode: always # application-prod.yml spring: jpa: hibernate: ddl-auto: validate sql: init: mode: never또는 현재 파일의 주석대로 설정:
ddl-auto: validate # not 'none' mode: never # not 'always'다음 명령으로 schema.sql의 DROP 명령을 확인하세요:
#!/bin/bash # schema.sql의 DROP TABLE 명령 확인 echo "=== DROP TABLE commands in schema.sql ===" rg -n "DROP TABLE" monew/src/main/resources/schema.sqlmonew/src/main/resources/schema.sql (1)
16-23: DROP TABLE 명령과 application.yml 설정 조합이 위험함이 스키마 파일은 모든 테이블을 삭제하는
DROP TABLE명령을 포함하고 있습니다.application.yml의spring.sql.init.mode: always설정과 결합되면 애플리케이션 시작 시마다 모든 데이터가 삭제됩니다.이는
application.yml파일에서 이미 지적한 critical issue와 연관된 문제입니다. 해당 설정을 즉시 수정하세요.
🧹 Nitpick comments (1)
monew/src/main/java/com/spring/monew/interest/service/impl/InterestServiceImpl.java (1)
92-98: 배치 소프트 삭제로 N+1 쿼리 문제 개선 필요
findAllByInterestId와deleteAll조합은 Spring Data JPA의 기본 반복 삭제 패턴으로, 대량 데이터 시 성능 문제를 야기합니다. 현재 구현은 1번의 SELECT + N번의 UPDATE 쿼리를 발생시킵니다(예: 10,000건 = 10,001회 쿼리).권장 개선사항:
ArticleRepository에 배치 소프트 삭제 메서드 추가:
@Modifying(clearAutomatically = true) @Query("UPDATE Article a SET a.isDeleted = true, a.deletedAt = CURRENT_TIMESTAMP WHERE a.interest.id = :interestId") void softDeleteByInterestId(@Param("interestId") UUID interestId);InterestServiceImpl의 92-98줄을 다음으로 교체:
int deletedCount = articleRepository.softDeleteByInterestId(interestId); if (deletedCount > 0) { log.info("관심사 삭제로 인한 연관 게시글 소프트 삭제: interestId={}, articleCount={}", interestId, deletedCount); }이 방식은 단일 UPDATE 쿼리로 트랜잭션 시간을 단축하고 메모리 효율을 개선합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java(1 hunks)monew/src/main/java/com/spring/monew/interest/domain/Interest.java(2 hunks)monew/src/main/java/com/spring/monew/interest/service/impl/InterestServiceImpl.java(3 hunks)monew/src/main/resources/application.yml(5 hunks)monew/src/main/resources/schema-h2.sql(1 hunks)monew/src/main/resources/schema.sql(3 hunks)monew/src/test/java/com/spring/monew/interest/domain/InterestServiceTest.java(3 hunks)monew/src/test/java/com/spring/monew/interest/service/InterestIntegrationTest.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-29T09:36:40.959Z
Learnt from: Namsoo315
Repo: codeit-monew/sb05-monew-team7 PR: 57
File: monew/src/test/java/com/spring/monew/comment/repository/CommentRepositoryTest.java:175-197
Timestamp: 2025-10-29T09:36:40.959Z
Learning: In the file monew/src/test/java/com/spring/monew/comment/repository/CommentRepositoryTest.java, the content strings like "like0", "like5", "like3" in the sortByLikeCount test are just comment content labels, not indicators of the actual like counts. The actual like counts are controlled by incrementLikeCount() calls.
Applied to files:
monew/src/test/java/com/spring/monew/interest/domain/InterestServiceTest.javamonew/src/test/java/com/spring/monew/interest/service/InterestIntegrationTest.java
🧬 Code graph analysis (2)
monew/src/main/java/com/spring/monew/interest/service/impl/InterestServiceImpl.java (2)
monew/src/main/java/com/spring/monew/subscription/service/impl/SubscriptionServiceImpl.java (1)
Service(16-57)monew/src/main/java/com/spring/monew/comment/service/impl/CommentServiceImpl.java (1)
Service(21-89)
monew/src/main/java/com/spring/monew/interest/domain/Interest.java (3)
monew/src/main/java/com/spring/monew/article/domain/Article.java (1)
Entity(15-143)monew/src/main/java/com/spring/monew/comment/domain/Comment.java (1)
Entity(27-105)monew/src/main/java/com/spring/monew/user/domain/User.java (1)
Entity(20-88)
🔇 Additional comments (14)
monew/src/test/java/com/spring/monew/interest/domain/InterestServiceTest.java (4)
12-14: LGTM!관심사 삭제 시 연관 게시글 처리를 테스트하기 위해 필요한 의존성들이 적절히 추가되었습니다.
41-42: LGTM!ArticleRepository 목 객체가 적절히 추가되어 관심사 삭제 시 연관 게시글 처리를 테스트할 수 있습니다.
141-151: LGTM!관련 게시글이 없는 경우의 관심사 삭제 시나리오가 명확하게 테스트되고 있습니다. 적절한 검증이 수행되고 있습니다.
154-171: 검토 의견은 부정확함 - 코드는 올바르게 구현됨검토 의견이 잘못된 가정에 기반하고 있습니다.
@SQLDelete어노테이션이 Article 엔티티에 설정되어 있으면, Spring Data JPA의deleteAll()메서드는 자동으로 이 커스텀 SQL을 존중하여 소프트 삭제를 수행합니다.검증 결과:
- Article 엔티티 (line 19):
@SQLDelete(sql = "UPDATE articles SET is_deleted = true, deleted_at = CURRENT_TIMESTAMP WHERE id = ?")설정됨- ArticleRepository:
deleteAll()오버라이드 없음 → Spring Data의 기본 동작이 @SQLDelete 적용- InterestServiceImpl.removeInterest() (line 97):
articleRepository.deleteAll(relatedArticles)호출- 로그 메시지 (line 95): "연관 게시글 소프트 삭제"로 명시적 확인
deleteAll()은@SQLDelete로 인해UPDATE articles SET is_deleted = true, deleted_at = CURRENT_TIMESTAMP WHERE id = ?쿼리를 실행하므로, 테스트 이름 "관련 게시글도 소프트 삭제"는 정확합니다.monew/src/main/java/com/spring/monew/interest/service/impl/InterestServiceImpl.java (1)
38-40: 유사도 체크 강화 확인됨
similarNames가 비어있지 않을 때 예외를 발생시키도록 로직이 강화되었습니다. 이는 중복/유사 관심사 생성을 방지하는 좋은 개선입니다.monew/src/test/java/com/spring/monew/interest/service/InterestIntegrationTest.java (1)
129-175: 소프트 삭제 검증 테스트가 포괄적으로 작성됨관심사 삭제 시 연관 기사들도 소프트 삭제되는지 확인하는 테스트가 잘 작성되었습니다.
장점:
- Native query로 DB의 실제
is_deleted플래그 확인- Repository 레벨에서
@SQLRestriction필터링 동작 확인flush()/clear()로 영속성 컨텍스트 관리- 관심사와 2개 기사 모두 검증
테스트 커버리지가 우수합니다.
monew/src/main/java/com/spring/monew/interest/domain/Interest.java (2)
28-29: 소프트 삭제 구현이 올바르게 적용됨Hibernate의
@SQLDelete와@SQLRestriction어노테이션을 사용한 소프트 삭제 구현이 다른 엔티티(Article, Comment, User)와 일관되게 적용되었습니다.
@SQLDelete:repository.delete()호출 시 UPDATE 쿼리 실행@SQLRestriction: 조회 쿼리에 자동으로is_deleted = false조건 추가
50-63: 감사 및 소프트 삭제 필드 추가 확인새로 추가된 필드들이 적절하게 구성되었습니다:
updatedAt:@UpdateTimestamp로 자동 갱신isDeleted: 기본값 false로 설정deletedAt: nullable로 삭제 시점 기록스키마 정의(schema.sql, schema-h2.sql)와 일치합니다.
monew/src/main/resources/application.yml (3)
63-66: 로깅 파일 설정 적절함로그 파일 경로, 이름, 최대 보관 기간(7일)이 명확하게 설정되었습니다. 환경 변수로 오버라이드 가능한 구조도 좋습니다.
108-114: AWS S3 설정 적절함S3 리전, 버킷, 인증 정보가 환경 변수에서 주입되도록 구성되어 보안상 안전합니다. 기사 백업용 버킷과 로그 백업용 버킷을 분리한 것도 좋은 접근입니다.
38-42: 배치 트리거 설정 검증 완료설정이 올바르게 구성되어 있습니다. YAML 파일의
spring.batch.trigger.enabled: true는 Java 코드의@ConditionalOnProperty(name = "spring.batch.trigger.enabled", havingValue = "true")와 정확히 일치하며, BatchJobController가 이 속성으로 조건부 활성화됩니다.spring.batch.job.enabled: false와 함께 수동 트리거 API 구현이라는 PR 목표와 완벽하게 부합합니다.monew/src/main/resources/schema.sql (3)
67-70: interests 테이블 소프트 삭제 컬럼 추가 확인
updated_at,is_deleted,deleted_at컬럼이 Interest 엔티티 정의와 일치하게 추가되었습니다.TIMESTAMP WITH TIME ZONE타입 사용도 적절합니다.
111-114: ON DELETE CASCADE 제거가 소프트 삭제 패턴에 적합함
fk_article_interest에서ON DELETE CASCADE를 제거한 것은 소프트 삭제 구현에 필수적인 변경입니다.이유:
- 소프트 삭제는
UPDATE쿼리로 동작하므로 DB의ON DELETE CASCADE가 트리거되지 않습니다.- 대신
InterestServiceImpl.removeInterest()에서 애플리케이션 레벨로 연관 기사를 삭제합니다.ON UPDATE CASCADE는 유지되어 interest ID 변경 시 전파됩니다.주의사항:
- 이제 관심사 삭제 로직이 애플리케이션 코드에 의존합니다.
- 직접 SQL로 관심사를 삭제하면 기사가 고아(orphan) 상태로 남을 수 있습니다.
283-284: 알림 조회 성능 개선을 위한 인덱스 추가
idx_notif_user_confirm_created_desc인덱스는 사용자별 알림 조회 쿼리(확인 여부와 생성 시간 기준 정렬)의 성능을 개선합니다. 복합 인덱스 컬럼 순서도 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java (1)
44-76: 배치 엔드포인트 보안 설정 누락 - 즉시 수정 필요이전 리뷰에서 지적된 보안 이슈가 여전히 해결되지 않았습니다.
SecurityConfig.java42번 라인의.permitAll()설정으로 인해 인증 없이 누구나 다음의 민감한 배치 작업을 실행할 수 있습니다:
- 기사 백업 (모든 기사를 S3에 백업)
- 로그 백업 (애플리케이션 로그를 S3에 업로드)
- 기사 정리 (30일 이상 지난 삭제된 기사 물리 삭제)
SecurityConfig에서
/api/batch/**경로를hasRole('ADMIN')으로 변경하거나, 각 메서드에@PreAuthorize("hasRole('ADMIN')")애노테이션을 추가해야 합니다.
🧹 Nitpick comments (3)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
63-63: 타임존 설정을 외부화하는 것을 권장합니다.
Asia/Seoul타임존이 하드코딩되어 있어 배포 환경이 변경될 경우 코드 수정이 필요합니다.다음과 같이 설정 파일에서 주입받도록 개선할 수 있습니다:
+ @Value("${application.timezone:Asia/Seoul}") + private String timezone; + @Bean @StepScope public JpaPagingItemReader<Article> articleBackupReader( @Value("#{jobParameters['triggerType']}") String triggerType ) { - ZoneId zoneId = ZoneId.of("Asia/Seoul"); + ZoneId zoneId = ZoneId.of(timezone);monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java (2)
54-90: 코드 중복을 줄이는 리팩토링을 고려해보세요.
triggerArticleBackupJob()와triggerLogBackupJob()메서드가triggerNewsCollectionJob()와 거의 동일한 구조를 가지고 있습니다.공통 로직을 추출하여 중복을 줄일 수 있습니다:
private BatchJobTriggerResponse triggerJob(Job job, String jobName) { log.info("{} 수동 실행 요청", jobName); JobParameters jobParameters = new JobParametersBuilder() .addString("triggerType", "MANUAL") .addLong("triggerTime", System.currentTimeMillis()) .toJobParameters(); try { JobExecution jobExecution = jobLauncher.run(job, jobParameters); log.info("{} 실행 성공 - executionId: {}, status: {}", jobName, jobExecution.getId(), jobExecution.getStatus()); return BatchJobTriggerResponse.from(jobExecution); } catch (Exception e) { log.error("{} 실행 실패", jobName, e); throw new RuntimeException("배치 작업 실행 실패", e); } } public BatchJobTriggerResponse triggerArticleBackupJob() { return triggerJob(articleBackupJob, "articleBackupJob"); }
92-103: 정리 작업 트리거 패턴이 다른 배치 작업과 일관성이 없습니다.
triggerArticleCleanupJob()이 다른 배치 트리거 메서드와 다른 패턴을 사용합니다:
- 다른 메서드:
JobLauncher.run(job, params)사용 → Spring Batch 인프라 활용, JobExecution 추적 가능- 이 메서드:
articleCleanupScheduler.deleteSoftDeletedArticles()직접 호출 → 추적 불가또한 Line 98의
CleanupTriggerResponse.success("articleCleanupJob", 0, 0)에서 성공/실패 카운트가 하드코딩되어 있어 실제 처리 결과를 반영하지 못합니다.설계 의도를 확인하고, 가능하다면 ArticleCleanup도 Spring Batch Job으로 전환하거나,
deleteSoftDeletedArticles()가 실제 처리 건수를 반환하도록 개선하는 것을 권장합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java(1 hunks)monew/src/main/java/com/spring/monew/batch/controller/BatchJobController.java(2 hunks)monew/src/main/java/com/spring/monew/batch/dto/response/CleanupTriggerResponse.java(1 hunks)monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java(3 hunks)monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java(1 hunks)monew/src/main/resources/application.yml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- monew/src/main/resources/application.yml
🧰 Additional context used
🧬 Code graph analysis (3)
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (3)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
Slf4j(29-95)monew/src/main/java/com/spring/monew/batch/processor/ArticleBackupProcessor.java (1)
Component(11-28)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java (1)
Component(14-38)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-60)
monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java (1)
monew/src/main/java/com/spring/monew/batch/exception/BatchJobExecutionNotFoundException.java (1)
BatchJobExecutionNotFoundException(3-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (3)
monew/src/main/java/com/spring/monew/batch/dto/response/CleanupTriggerResponse.java (1)
5-33: LGTM! 깔끔한 DTO 구현입니다.Record 패턴을 사용한 불변 DTO로 잘 설계되었으며, 정적 팩토리 메서드로 성공/실패 응답을 명확하게 구분하고 있습니다.
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
71-81: MANUAL 트리거의 날짜 범위 검증이 필요합니다.수동 트리거 시
tomorrow까지 포함하는 로직이 의도된 동작인지 확인이 필요합니다. PR 요약에는 "어제+오늘 기사를 백업"이라고 명시되어 있으나, 코드는 내일(미래)까지의 범위를 포함합니다.
- 의도: 수동 실행 중 생성되는 오늘의 기사를 모두 포함하기 위함
- 의문:
tomorrow범위는 아직 생성되지 않은 미래 시점이므로 불필요할 수 있음스케줄 트리거(어제
오늘)와의 차이가 명확한지, 혹은 수동 트리거도 어제오늘 범위로 통일해야 하는지 검토해주세요.monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (1)
26-26: 동시 실행 시 데이터 혼합 가능성을 검증해주세요.
aggregatedArticles가 싱글톤 컴포넌트의 인스턴스 필드로 선언되어 있어, 여러 배치 작업이 동시에 실행될 경우 서로 다른 작업의 데이터가 혼합될 수 있습니다.Spring Batch가 동일 Writer 인스턴스를 여러 Job 실행에서 재사용하는지, 그리고 동시 실행을 방지하는 설정이 있는지 확인해주세요. 만약 동시 실행 가능성이 있다면
ThreadLocal<List<ArticleBackupDto>>또는 Step별 격리된 저장소 사용을 권장합니다.
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java (2)
30-31: 대량 데이터 환경에서의 메모리 사용을 고려하세요.페이지네이션 없이 모든
sourceUrl을 조회하고 있습니다. 기사 수가 많아질 경우 메모리 부담이 발생할 수 있습니다. 현재 백업/배치 용도로 사용된다면 허용 가능하지만, 향후 데이터 증가 시 페이지네이션이나 스트리밍 방식을 고려해보세요.
33-48: 시간 범위 쿼리가 방어적으로 잘 구현되었습니다.
deleted_at IS NOT NULL체크로 데이터 무결성을 보장하고,Instant타입으로 타임존 일관성도 확보되었습니다. 포괄적 범위(>=,<=) 사용도 적절합니다.다만
deleted_at컬럼에 인덱스가 없다면, 테이블 크기 증가 시 이러한 시간 범위 쿼리의 성능이 저하될 수 있습니다. 프로덕션 환경에서deleted_at컬럼에 인덱스 추가를 고려해보세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java(1 hunks)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java(1 hunks)monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T04:47:15.335Z
Learnt from: Namsoo315
Repo: codeit-monew/sb05-monew-team7 PR: 59
File: monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java:29-30
Timestamp: 2025-10-30T04:47:15.335Z
Learning: In the file monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java, the method `findIncludingDeleted(Param("commentId") UUID commentId)` is intentionally named this way even though the query `SELECT * FROM comments c WHERE c.id = :commentId AND c.is_deleted = true` only returns deleted records (is_deleted = true), not all records including deleted ones. This naming is by design and should not be flagged.
Applied to files:
monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java
🧬 Code graph analysis (1)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (2)
monew/src/main/java/com/spring/monew/auth/config/SecurityConfig.java (1)
RequiredArgsConstructor(17-60)monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java (1)
Slf4j(22-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (6)
monew/src/main/java/com/spring/monew/article/repository/ArticleRepository.java (5)
4-11: 필요한 import가 모두 적절하게 추가되었습니다.새로운 메서드들의 타입과 어노테이션에 필요한 import들이 정확하게 추가되었습니다.
20-22: 하드 삭제 메서드가 올바르게 구현되었습니다.
@Modifying(clearAutomatically = true)설정으로 persistence context가 자동으로 클리어되어 안전합니다. 트랜잭션 관리는 서비스 레이어에서 처리되고 있습니다.
24-25: 효율적인 존재 여부 확인 구현입니다.
EXISTS서브쿼리를 사용하여 soft-delete 상태와 관계없이 효율적으로 존재 여부를 확인합니다.
27-28: 과거 리뷰 이슈가 정확히 해결되었습니다.
is_deleted = true필터가 추가되어 이제CommentRepository.findIncludingDeleted와 일관된 패턴으로 삭제된 레코드만 조회합니다.
50-50: Spring Data JPA 규칙에 따라 적절하게 정의되었습니다.메서드 네이밍 규칙을 따르고 있으며,
Article엔티티의@SQLRestriction에 의해 자동으로 soft-delete된 레코드가 필터링됩니다.monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (1)
27-64: MDC 컨텍스트 관리와 예외 격리 흐름 좋습니다한 번 생성한 requestId를 finally에서 정리하고, 기사 단위 try/catch로 실패를 격리해 두신 점이 안정적으로 보입니다.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
monew/src/main/java/com/spring/monew/interest/repository/InterestRepository.java (1)
18-19: 성능 및 메서드 네이밍 개선을 고려하세요.두 가지 개선 사항을 제안합니다:
성능: 삭제된 관심사가 많을 경우
is_deleted = true조건으로 모든 레코드를 스캔하고 정렬하는 것은 비효율적일 수 있습니다.(is_deleted, deleted_at)복합 인덱스 추가를 권장합니다.네이밍:
findFirstDeleted()는 모호합니다("first"가 최초 삽입? 최초 삭제? 최근 삭제?). 쿼리 동작(ORDER BY deleted_at DESC LIMIT 1)을 고려하면findMostRecentlyDeleted()또는findLatestDeleted()가 더 명확합니다.마이그레이션 스크립트에 인덱스 추가 예시:
CREATE INDEX idx_interests_deleted ON interests(is_deleted, deleted_at DESC);메서드명 변경:
- @Query(value = "SELECT * FROM interests WHERE is_deleted = true ORDER BY deleted_at DESC LIMIT 1", nativeQuery = true) - Optional<Interest> findFirstDeleted(); + @Query(value = "SELECT * FROM interests WHERE is_deleted = true ORDER BY deleted_at DESC LIMIT 1", nativeQuery = true) + Optional<Interest> findMostRecentlyDeleted();monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (4)
226-242: DTO를 Map으로 변환하는 불필요한 오버헤드 제거Line 232에서
ArticleBackupDto를Map<String, Object>로 변환하는데, 이는 타입 안전성을 잃고 성능 오버헤드를 추가합니다.restoreArticle메서드에서 DTO를 직접 사용하면 더 효율적이고 안전합니다.다음과 같이 리팩토링하세요:
- private List<Map<String, Object>> findBackupsForDate(LocalDate date) { + private List<ArticleBackupDto> findBackupsForDate(LocalDate date) { try { List<ArticleBackupDto> backupDtos = s3BackupService.downloadBackup(date); - - List<Map<String, Object>> backups = new ArrayList<>(); - for (ArticleBackupDto dto : backupDtos) { - Map<String, Object> backupMap = objectMapper.convertValue(dto, new TypeReference<Map<String, Object>>() {}); - backups.add(backupMap); - } - - log.info("[복원] {} 날짜의 백업 파일에서 {}개 기사 발견", date, backups.size()); - return backups; + log.info("[복원] {} 날짜의 백업 파일에서 {}개 기사 발견", date, backupDtos.size()); + return backupDtos; } catch (Exception e) { log.warn("백업 검색 실패 for date {}: {}", date, e.getMessage()); return new ArrayList<>(); } }그리고
findMissingArticles와restoreArticle메서드도ArticleBackupDto를 사용하도록 수정해주세요.
320-323: 구체적인 예외 타입 사용 권장Line 322에서 일반
RuntimeException으로 래핑하는 대신, 더 구체적인 예외 타입을 사용하면 예외 처리 및 디버깅이 용이합니다.커스텀 예외를 생성하거나 기존 예외를 사용하세요:
} catch (Exception e) { log.error("기사 복원 실패: {}", backupData, e); - throw new RuntimeException("기사 복원 실패", e); + throw new ArticleRestoreException("기사 복원 실패: " + backupData.get("id"), e); }
ArticleRestoreException을 새로 생성하거나, 기존의 적절한 예외를 사용할 수 있습니다.
326-337: 타임스탬프 파싱 실패 시 명시적 처리 고려
parseInstant메서드는 null이나 잘못된 값에 대해Instant.now()를 반환합니다. 이는 백업 데이터의 원본 타임스탬프 정보를 조용히 손실시켜 데이터 무결성 문제를 일으킬 수 있습니다. 복원 작업의 특성상 원본 시간 정보 보존이 중요합니다.null이나 잘못된 형식에 대해 예외를 발생시키거나, 최소한 경고 로그를 남기는 것을 고려하세요:
private Instant parseInstant(Object value) { if (value == null) { + log.warn("타임스탬프 값이 null입니다. 현재 시간을 사용합니다."); return Instant.now(); } if (value instanceof String) { - return Instant.parse((String) value); + try { + return Instant.parse((String) value); + } catch (Exception e) { + log.warn("타임스탬프 파싱 실패: {}. 현재 시간을 사용합니다.", value); + return Instant.now(); + } } if (value instanceof Number) { return Instant.ofEpochMilli(((Number) value).longValue()); } + log.warn("지원하지 않는 타임스탬프 형식: {}. 현재 시간을 사용합니다.", value.getClass()); return Instant.now(); }
339-358: 기본 Interest 생성 전략 재검토Line 355에서 활성 Interest가 전혀 없을 때 "복원됨"이라는 이름의 기본 Interest를 생성합니다. 이는 임시 방편으로 보이며, 실제 운영 환경에서는 시스템이 일관성 없는 상태에 있음을 나타낼 수 있습니다.
다음 중 하나를 고려하세요:
- 예외 발생: Interest가 없는 것은 시스템 설정 오류이므로 복원을 실패시키는 것이 더 명확할 수 있습니다.
throw new IllegalStateException("복원에 필요한 Interest가 시스템에 존재하지 않습니다.");
- 관리자 알림: 기본 Interest를 생성하되, 관리자에게 알림을 보내 수동 개입을 요청합니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java(3 hunks)monew/src/main/java/com/spring/monew/interest/domain/Interest.java(3 hunks)monew/src/main/java/com/spring/monew/interest/repository/InterestRepository.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- monew/src/main/java/com/spring/monew/interest/domain/Interest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-30T04:47:15.335Z
Learnt from: Namsoo315
Repo: codeit-monew/sb05-monew-team7 PR: 59
File: monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java:29-30
Timestamp: 2025-10-30T04:47:15.335Z
Learning: In the file monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java, the method `findIncludingDeleted(Param("commentId") UUID commentId)` is intentionally named this way even though the query `SELECT * FROM comments c WHERE c.id = :commentId AND c.is_deleted = true` only returns deleted records (is_deleted = true), not all records including deleted ones. This naming is by design and should not be flagged.
Applied to files:
monew/src/main/java/com/spring/monew/interest/repository/InterestRepository.java
🧬 Code graph analysis (1)
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (2)
monew/src/main/java/com/spring/monew/article/exception/ArticleNotFoundException.java (1)
ArticleNotFoundException(5-9)monew/src/main/java/com/spring/monew/common/exception/GlobalExceptionHandler.java (1)
Slf4j(17-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (2)
monew/src/main/java/com/spring/monew/interest/repository/InterestRepository.java (1)
15-16:findIncludingDeleted메서드의 쿼리가 다른 저장소와 불일치합니다.InterestRepository의
findIncludingDeleted메서드는SELECT * FROM interests WHERE id = :interestId쿼리를 사용하여 삭제 상태를 필터링하지 않으므로, 삭제된 레코드와 활성 레코드를 구별하지 않습니다. 반면 ArticleRepository와 CommentRepository의 동일 메서드는 모두AND is_deleted = true조건을 포함합니다.조건을 추가하여 일관성을 맞추세요:
@Query(value = "SELECT * FROM interests WHERE id = :interestId AND is_deleted = true", nativeQuery = true) Optional<Interest> findIncludingDeleted(@Param("interestId") UUID interestId);⛔ Skipped due to learnings
Learnt from: Namsoo315 Repo: codeit-monew/sb05-monew-team7 PR: 59 File: monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java:29-30 Timestamp: 2025-10-30T04:47:15.335Z Learning: In the file monew/src/main/java/com/spring/monew/comment/repository/CommentRepository.java, the method `findIncludingDeleted(Param("commentId") UUID commentId)` is intentionally named this way even though the query `SELECT * FROM comments c WHERE c.id = :commentId AND c.is_deleted = true` only returns deleted records (is_deleted = true), not all records including deleted ones. This naming is by design and should not be flagged.monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (1)
146-159: 검증 완료 - 우려사항 없음Article 엔티티의
@SQLRestriction("is_deleted = false")와@SQLDelete어노테이션을 확인했습니다.
findById()는 자동으로 소프트 삭제된 기사를 제외합니다.delete()호출 시@SQLDelete어노테이션이 is_deleted=true로 업데이트를 수행합니다.- 이미 삭제된 기사를 재삭제하려고 하면
ArticleNotFoundException이 발생하여 404가 반환됩니다.코드가 PR 요구사항과 일치하게 올바르게 구현되었습니다.
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java
Outdated
Show resolved
Hide resolved
| try { | ||
| List<Map<String, Object>> backupDataList = findBackupsForDate(currentDate); | ||
| List<Map<String, Object>> missingArticles = findMissingArticles(backupDataList); | ||
|
|
||
| for (Map<String, Object> backupData : missingArticles) { | ||
| Article restored = restoreArticle(backupData); | ||
| restoredArticleIds.add(restored.getId()); | ||
| } | ||
| } catch (Exception e) { | ||
| log.warn("날짜 {}의 백업 복원 중 오류 발생: {}", currentDate, e.getMessage()); | ||
| } | ||
|
|
||
| currentDate = currentDate.plusDays(1); | ||
| } |
There was a problem hiding this comment.
복원 실패 정보를 결과에 포함하세요
현재 구현은 특정 날짜의 백업 복원 실패 시 경고 로그만 남기고 계속 진행합니다. 사용자는 어느 날짜가 실패했는지 알 수 없고, ArticleRestoreResultDto에도 실패 정보가 포함되지 않습니다. 부분 성공 시나리오에서 명확한 피드백이 필요합니다.
실패한 날짜를 추적하고 결과 DTO에 포함하는 것을 고려해주세요:
List<UUID> restoredArticleIds = new ArrayList<>();
+ List<LocalDate> failedDates = new ArrayList<>();
LocalDate currentDate = fromDate.atZone(ZoneOffset.UTC).toLocalDate();
LocalDate endDate = toDate.atZone(ZoneOffset.UTC).toLocalDate();
while (!currentDate.isAfter(endDate)) {
try {
List<Map<String, Object>> backupDataList = findBackupsForDate(currentDate);
List<Map<String, Object>> missingArticles = findMissingArticles(backupDataList);
for (Map<String, Object> backupData : missingArticles) {
Article restored = restoreArticle(backupData);
restoredArticleIds.add(restored.getId());
}
} catch (Exception e) {
log.warn("날짜 {}의 백업 복원 중 오류 발생: {}", currentDate, e.getMessage());
+ failedDates.add(currentDate);
}
currentDate = currentDate.plusDays(1);
}그리고 ArticleRestoreResultDto에 failedDates 필드를 추가해주세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java
around lines 206-219, the catch block only logs backup restore errors so callers
and the returned ArticleRestoreResultDto don't know which dates failed; update
the method to collect failedDates: initialize a List<LocalDate> failedDates
before the loop, in the catch block add currentDate (and optionally error
message) to failedDates instead of only logging, and after the loop populate and
return an ArticleRestoreResultDto that includes this failedDates field (also add
the failedDates property to ArticleRestoreResultDto with appropriate
getter/setter/constructor and update any builders/serializers as needed).
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
monew/src/main/resources/schema-h2.sql (1)
26-26: 타임존 처리 불일치: H2 schema의 interests.deleted_at 컬럼 타입 수정 필요검증 결과, H2 schema의
deleted_at컬럼이 PostgreSQL schema와 일치하지 않습니다. PostgreSQL에서는TIMESTAMP WITH TIME ZONE을 사용하지만, H2에서는TIMESTAMP만 사용하고 있으며, 이는 같은 테이블의created_at,updated_at컬럼과도 불일치합니다.현재 상태:
- PostgreSQL:
deleted_at TIMESTAMP WITH TIME ZONE✓- H2 (line 26):
deleted_at TIMESTAMP✗다음 수정을 적용하세요:
- deleted_at TIMESTAMP + deleted_at TIMESTAMP WITH TIME ZONE
🧹 Nitpick comments (6)
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (2)
31-34: 불필요한 캐스팅 및 null 필터 제거 가능
Chunk<? extends ArticleBackupDto>의 아이템은 이미ArticleBackupDto타입이므로 명시적 캐스팅이 불필요하며,ArticleBackupProcessor는 예외 발생 시 실패 처리하므로 null을 반환하지 않습니다.- List<ArticleBackupDto> articles = chunk.getItems().stream() - .map(dto -> (ArticleBackupDto) dto) - .filter(dto -> dto != null) - .toList(); + List<ArticleBackupDto> articles = new ArrayList<>(chunk.getItems());또는 null 체크를 유지하려면:
- List<ArticleBackupDto> articles = chunk.getItems().stream() - .map(dto -> (ArticleBackupDto) dto) - .filter(dto -> dto != null) - .toList(); + List<ArticleBackupDto> articles = chunk.getItems().stream() + .filter(dto -> dto != null) + .toList();
58-58: 타임존 하드코딩으로 인한 유지보수 위험
"Asia/Seoul"타임존이 Writer와 Reader(ArticleBackupBatchConfigline 59)에 중복 하드코딩되어 있습니다. 향후 타임존 변경 시 여러 곳을 수정해야 하며, 불일치 시 백업 날짜 계산 오류가 발생할 수 있습니다.권장 사항:
타임존을 설정 파일(application.yml)에서 주입하거나 상수로 중앙 관리하세요.공통 상수 정의:
// BatchConstants.java (새 파일) public class BatchConstants { public static final ZoneId BACKUP_TIMEZONE = ZoneId.of("Asia/Seoul"); }Writer에서 사용:
+import static com.spring.monew.batch.BatchConstants.BACKUP_TIMEZONE; + - LocalDate backupDate = LocalDate.now(ZoneId.of("Asia/Seoul")).minusDays(1); + LocalDate backupDate = LocalDate.now(BACKUP_TIMEZONE).minusDays(1);또는 설정 주입 방식:
@Value("${batch.backup.timezone:Asia/Seoul}") private String backupTimezone; // afterStep()에서 LocalDate backupDate = LocalDate.now(ZoneId.of(backupTimezone)).minusDays(1);monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (3)
232-248: 타입 안전성을 위해 Map 대신 DTO를 직접 사용하세요.Line 238에서
ArticleBackupDto를Map<String, Object>로 변환하고 있습니다. 이후 메서드들(findMissingArticles,restoreArticle)도 Map으로 작업합니다. 이는 타입 안전성을 잃고 런타임 오류 가능성을 높입니다.
ArticleBackupDto를 직접 사용하도록 리팩토링하는 것을 권장합니다:- private List<Map<String, Object>> findBackupsForDate(LocalDate date) { + private List<ArticleBackupDto> findBackupsForDate(LocalDate date) { try { List<ArticleBackupDto> backupDtos = s3BackupService.downloadBackup(date); - - List<Map<String, Object>> backups = new ArrayList<>(); - for (ArticleBackupDto dto : backupDtos) { - Map<String, Object> backupMap = objectMapper.convertValue(dto, new TypeReference<Map<String, Object>>() {}); - backups.add(backupMap); - } - - log.info("[복원] {} 날짜의 백업 파일에서 {}개 기사 발견", date, backups.size()); - return backups; + log.info("[복원] {} 날짜의 백업 파일에서 {}개 기사 발견", date, backupDtos.size()); + return backupDtos; } catch (Exception e) { log.warn("백업 검색 실패 for date {}: {}", date, e.getMessage()); return new ArrayList<>(); } }그리고
findMissingArticles,restoreArticle메서드도 이에 맞춰 수정하세요.
282-334: 복원 로직이 포괄적으로 구현되었으나 사이드 이펙트를 문서화하세요.Interest 처리 로직(lines 300-311)이 잘 설계되었습니다. 활성 Interest를 찾고, 없으면 삭제된 Interest를 복원하며, 그것도 없으면 기본 Interest를 생성하는 다단계 폴백 전략이 적절합니다.
참고: Line 305에서 기사 복원 중 삭제된 Interest를 자동으로 복원하는 것은 중요한 사이드 이펙트입니다. 이는 의도된 동작으로 보이지만, 메서드나 클래스 수준의 JavaDoc에 이 동작을 명시하는 것을 권장합니다.
메서드에 JavaDoc을 추가하여 사이드 이펙트를 문서화하세요:
+ /** + * 백업 데이터로부터 단일 기사를 복원합니다. + * + * @param backupData 백업된 기사 데이터 + * @return 복원된 기사 엔티티 + * @throws RuntimeException 복원 중 오류 발생 시 + * + * 참고: 연관된 Interest가 삭제된 경우 자동으로 복원합니다. + */ private Article restoreArticle(Map<String, Object> backupData) {
349-368: 기본 Interest 생성 로직이 적절하나 효율성 개선이 가능합니다.Line 350에서
findAll().stream().findFirst()를 사용하는 것은 모든 Interest를 로드하므로 비효율적입니다.InterestRepository에 전용 메서드를 추가하여 효율성을 개선하세요:
// InterestRepository에 추가 Optional<Interest> findFirstByOrderByCreatedAtAsc();그리고 이를 사용:
private Interest getOrCreateDefaultInterest() { - return interestRepository.findAll().stream() - .findFirst() + return interestRepository.findFirstByOrderByCreatedAtAsc() .orElseGet(() -> { // ... 나머지 로직 }); }참고: "복원됨" 이름과 ["복원"] 키워드는 엣지 케이스용 하드코딩된 값이지만, 이런 상황(활성 및 삭제된 Interest가 모두 없음)은 극히 드물므로 수용 가능합니다.
monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java (1)
95-99: 성공/실패 건수를 로그에 함께 남겨주세요.
수동 정리 결과를 counts 배열로 돌려받지만, 현재 로그는 건수 정보를 포함하지 않아 모니터링 시 직전 실행 성과를 로그만으로 파악하기 어렵습니다. 스케줄러 로그 포맷과 맞춰 성공/실패 건수를 함께 출력해 두면 추적성이 좋아집니다.다음과 같이 개선을 제안합니다:
- log.info("articleCleanupJob 실행 완료"); + log.info("articleCleanupJob 실행 완료 - 성공: {}, 실패: {}", counts[0], counts[1]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java(3 hunks)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java(1 hunks)monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java(3 hunks)monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java(1 hunks)monew/src/main/resources/schema-h2.sql(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (1)
monew/src/main/java/com/spring/monew/article/exception/ArticleNotFoundException.java (1)
ArticleNotFoundException(5-9)
monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java (3)
monew/src/main/java/com/spring/monew/batch/config/ArticleBackupBatchConfig.java (1)
Slf4j(29-95)monew/src/main/java/com/spring/monew/batch/processor/ArticleBackupProcessor.java (1)
Component(11-28)monew/src/main/java/com/spring/monew/batch/scheduler/ArticleBackupScheduler.java (1)
Component(14-38)
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java (1)
monew/src/main/java/com/spring/monew/batch/service/BatchJobService.java (1)
Slf4j(22-142)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-test
🔇 Additional comments (4)
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java (4)
3-50: 의존성 및 필드 선언이 적절합니다.백업/복구 및 감사 로깅 기능에 필요한 의존성들이 올바르게 추가되었습니다.
@RequiredArgsConstructor를 통한 의존성 주입 패턴도 일관성 있게 유지되고 있습니다.
161-182: 하드 삭제 로직이 적절하게 구현되었습니다.READ_COMMITTED 격리 수준으로 변경된 점이 좋습니다. 삭제 순서(댓글 좋아요 → 댓글 → 조회수 → 기사)가 외래 키 제약 조건을 올바르게 고려하고 있으며, 감사 로그에 삭제된 데이터 개수를 포함하는 것도 추적성 측면에서 유용합니다.
참고: Line 169-170의 카운트와 실제 삭제 사이에 이론적으로 TOCTOU 경합이 있을 수 있지만, 감사 로그 목적상 근사치로 충분하며 READ_COMMITTED는 성능과 정확성의 합리적인 균형점입니다.
250-279: 중복 제거 로직이 효율적으로 구현되었습니다.Line 260에서
findExistingSourceUrls(backupUrls)를 사용하여 백업 내 URL만 조회하는 방식으로 메모리 효율성 문제가 해결되었습니다. 이전 리뷰에서 제기된 전체 URL 로딩 문제가 적절히 개선되었습니다.
146-159: 검증 완료: 소프트 삭제가 올바르게 구현되었습니다.
Article엔티티에서 Hibernate의@SQLDelete어노테이션으로 소프트 삭제가 올바르게 구현되어 있습니다.articleRepository.delete(article)호출 시is_deleted = true와deleted_at = CURRENT_TIMESTAMP를 설정하는 UPDATE 쿼리가 실행되므로 물리적 삭제가 아닌 논리적 삭제가 수행됩니다.@SQLRestriction("is_deleted = false")으로 인해 삭제된 데이터는 조회에서 자동으로 제외됩니다.
| @Override | ||
| @Transactional | ||
| public ArticleRestoreResultDto restoreArticlesFromBackup(Instant fromDate, Instant toDate, UUID userId) { | ||
| String requestId = MDC.get("requestId"); | ||
| if (fromDate == null || toDate == null) { | ||
| throw new IllegalArgumentException("시작 날짜와 종료 날짜는 필수입니다"); | ||
| } | ||
|
|
||
| if (fromDate.isAfter(toDate)) { | ||
| throw new IllegalArgumentException("시작 날짜는 종료 날짜보다 이전이어야 합니다"); | ||
| } | ||
|
|
||
| Duration duration = Duration.between(fromDate, toDate); | ||
| if (duration.toDays() > 31) { | ||
| throw new IllegalArgumentException("날짜 범위는 최대 31일까지만 가능합니다"); | ||
| } | ||
|
|
||
| List<UUID> restoredArticleIds = new ArrayList<>(); | ||
| List<String> failedDates = new ArrayList<>(); | ||
| LocalDate currentDate = fromDate.atZone(ZoneOffset.UTC).toLocalDate(); | ||
| LocalDate endDate = toDate.atZone(ZoneOffset.UTC).toLocalDate(); | ||
|
|
||
| while (!currentDate.isAfter(endDate)) { | ||
| try { | ||
| List<Map<String, Object>> backupDataList = findBackupsForDate(currentDate); | ||
| List<Map<String, Object>> missingArticles = findMissingArticles(backupDataList); | ||
|
|
||
| for (Map<String, Object> backupData : missingArticles) { | ||
| Article restored = restoreArticle(backupData); | ||
| restoredArticleIds.add(restored.getId()); | ||
| } | ||
| } catch (Exception e) { | ||
| log.warn("날짜 {}의 백업 복원 중 오류 발생: {}", currentDate, e.getMessage()); | ||
| failedDates.add(currentDate.toString()); | ||
| } | ||
|
|
||
| currentDate = currentDate.plusDays(1); | ||
| } | ||
|
|
||
| if (!failedDates.isEmpty()) { | ||
| log.warn("복원 실패한 날짜: {}", String.join(", ", failedDates)); | ||
| } | ||
|
|
||
| log.info("복원 완료: {} 개의 기사 복원됨", restoredArticleIds.size()); | ||
| auditLogger.logRestore(fromDate, toDate, restoredArticleIds.size(), userId, requestId); | ||
| return new ArticleRestoreResultDto(Instant.now(), restoredArticleIds, restoredArticleIds.size()); | ||
| } |
There was a problem hiding this comment.
장기 실행 트랜잭션으로 인한 잠금 및 타임아웃 위험이 있습니다.
이 메서드는 최대 31일간의 백업을 단일 트랜잭션으로 복원합니다. 수천 개의 기사를 복원할 경우 트랜잭션이 매우 오래 실행되어 데이터베이스 잠금 문제, 타임아웃, 그리고 롤백 시 많은 작업 손실이 발생할 수 있습니다.
다음 중 하나의 접근 방식을 고려하세요:
옵션 1: 날짜별 독립 트랜잭션 (권장)
@Override
- @Transactional
public ArticleRestoreResultDto restoreArticlesFromBackup(Instant fromDate, Instant toDate, UUID userId) {
// ... validation ...
while (!currentDate.isAfter(endDate)) {
try {
- List<Map<String, Object>> backupDataList = findBackupsForDate(currentDate);
- List<Map<String, Object>> missingArticles = findMissingArticles(backupDataList);
-
- for (Map<String, Object> backupData : missingArticles) {
- Article restored = restoreArticle(backupData);
- restoredArticleIds.add(restored.getId());
- }
+ List<UUID> restored = restoreDateInTransaction(currentDate);
+ restoredArticleIds.addAll(restored);
} catch (Exception e) {
// ...
}
}
}
+ @Transactional
+ private List<UUID> restoreDateInTransaction(LocalDate date) {
+ List<Map<String, Object>> backupDataList = findBackupsForDate(date);
+ List<Map<String, Object>> missingArticles = findMissingArticles(backupDataList);
+
+ List<UUID> restored = new ArrayList<>();
+ for (Map<String, Object> backupData : missingArticles) {
+ Article article = restoreArticle(backupData);
+ restored.add(article.getId());
+ }
+ return restored;
+ }옵션 2: 배치 크기 제한
복원 대상이 1000개를 초과하면 경고를 표시하거나 거부합니다.
🤖 Prompt for AI Agents
In
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java
around lines 184-230, the restoreArticlesFromBackup method is currently
annotated with @Transactional and runs a loop over up to 31 days which can
create a long-running transaction (locks/timeouts/huge rollback). Fix by
removing the method-level transaction and instead run each day's restore inside
its own short-lived transaction (or a bounded batch transaction): extract the
inner loop body that processes a single date into a private method annotated
with @Transactional (or programmatically start/commit a transaction per date),
limit the number of articles processed per transaction (e.g., batch size), and
add a hard cap/guard that rejects or warns when total restore candidate count
exceeds a configured threshold (e.g., 1000) before starting restores. Ensure
audit logging and result aggregation remain outside per-date transactions so
they are not rolled back with individual failures.
| private Instant parseInstant(Object value) { | ||
| if (value == null) { | ||
| return Instant.now(); | ||
| } | ||
| if (value instanceof String) { | ||
| return Instant.parse((String) value); | ||
| } | ||
| if (value instanceof Number) { | ||
| return Instant.ofEpochMilli(((Number) value).longValue()); | ||
| } | ||
| return Instant.now(); | ||
| } |
There was a problem hiding this comment.
타임스탬프 파싱 실패 시 Instant.now()를 사용하면 데이터가 손상됩니다.
Line 338과 346에서 값이 null이거나 파싱할 수 없을 때 Instant.now()를 반환합니다. 이는 복원된 기사의 publishDate와 createdAt가 실제 값 대신 현재 시간으로 설정되어 데이터 무결성이 손상됩니다.
백업/복구 시나리오에서는 원본 타임스탬프 보존이 중요하므로, 파싱 실패 시 예외를 발생시켜야 합니다:
private Instant parseInstant(Object value) {
if (value == null) {
- return Instant.now();
+ throw new IllegalArgumentException("Timestamp value cannot be null");
}
if (value instanceof String) {
- return Instant.parse((String) value);
+ try {
+ return Instant.parse((String) value);
+ } catch (Exception e) {
+ throw new IllegalArgumentException("Invalid timestamp format: " + value, e);
+ }
}
if (value instanceof Number) {
return Instant.ofEpochMilli(((Number) value).longValue());
}
- return Instant.now();
+ throw new IllegalArgumentException("Unsupported timestamp type: " + value.getClass());
}🤖 Prompt for AI Agents
In
monew/src/main/java/com/spring/monew/article/service/impl/ArticleServiceImpl.java
around lines 336 to 347, do not return Instant.now() on null or failed parses
because that corrupts restored timestamps; instead validate input and throw an
unchecked exception so callers can fail the restore. Change the method to: if
value is null throw new IllegalArgumentException("timestamp value is null for
field X"); if value is a String attempt Instant.parse(...) and catch
DateTimeParseException to rethrow as IllegalArgumentException with the original
value in the message; if value is a Number convert with Instant.ofEpochMilli;
for any other type throw IllegalArgumentException describing the unsupported
type and including the actual value.
| String batchRequestId = "MANUAL-CLEANUP-" + UUID.randomUUID(); | ||
| MDC.put(REQUEST_ID_KEY, batchRequestId); | ||
|
|
||
| try { | ||
| Instant startTime = Instant.now(); | ||
| log.info("[Manual] 삭제된 게시글 정리 작업 시작 (최근 30일)"); | ||
|
|
||
| Instant thirtyDaysAgo = Instant.now().minus(30, ChronoUnit.DAYS); | ||
| List<UUID> articleIds = articleRepository.findSoftDeletedBetween(thirtyDaysAgo, Instant.now()); | ||
|
|
||
| if (articleIds.isEmpty()) { | ||
| log.info("[Manual] 정리할 삭제된 게시글이 없습니다."); | ||
| return new int[]{0, 0}; | ||
| } | ||
|
|
||
| log.info("[Manual] 정리 대상 게시글 {}개 발견", articleIds.size()); | ||
|
|
||
| int successCount = 0; | ||
| int failureCount = 0; | ||
|
|
||
| for (UUID articleId : articleIds) { | ||
| try { | ||
| articleService.hardDeleteArticle(articleId, SYSTEM_USER_ID); | ||
| successCount++; | ||
| } catch (Exception e) { | ||
| failureCount++; | ||
| log.error("[Manual] 게시글 {} 정리 실패: {}", articleId, e.getMessage(), e); | ||
| } | ||
| } | ||
|
|
||
| Duration executionTime = Duration.between(startTime, Instant.now()); | ||
| log.info("[Manual] 삭제된 게시글 정리 작업 완료 - 성공: {}개, 실패: {}개, 실행 시간: {}초", | ||
| successCount, failureCount, executionTime.getSeconds()); | ||
| return new int[]{successCount, failureCount}; | ||
| } finally { | ||
| MDC.remove(REQUEST_ID_KEY); | ||
| } |
There was a problem hiding this comment.
MDC requestId 복구 누락으로 요청 추적이 끊어집니다.
수동 정리 메서드는 기존 MDC requestId 값을 덮어쓴 뒤 finally 블록에서 무조건 제거하고 있습니다. 이 메서드가 HTTP 요청 컨텍스트에서 호출될 때는 필터가 심어 둔 원래 requestId가 사라져 이후 로그(예: BatchJobService의 후속 로그, 응답 처리)에서 요청 ID가 빠집니다. 감사·추적 요구사항을 충족하려면 이전 값을 저장해 두었다가 복원하고, 값이 없던 경우에만 remove 해야 합니다.
다음과 같이 수정해 주세요:
- String batchRequestId = "MANUAL-CLEANUP-" + UUID.randomUUID();
- MDC.put(REQUEST_ID_KEY, batchRequestId);
+ String previousRequestId = MDC.get(REQUEST_ID_KEY);
+ String batchRequestId = "MANUAL-CLEANUP-" + UUID.randomUUID();
+ MDC.put(REQUEST_ID_KEY, batchRequestId);
@@
- } finally {
- MDC.remove(REQUEST_ID_KEY);
- }
+ } finally {
+ if (previousRequestId != null) {
+ MDC.put(REQUEST_ID_KEY, previousRequestId);
+ } else {
+ MDC.remove(REQUEST_ID_KEY);
+ }
+ }🤖 Prompt for AI Agents
In
monew/src/main/java/com/spring/monew/batch/scheduler/ArticleCleanupScheduler.java
around lines 67 to 103, the method overwrites MDC requestId without preserving
the previous value and unconditionally removes it in finally, breaking tracing
for callers; fix by capturing the existing MDC.get(REQUEST_ID_KEY) into a local
variable before MDC.put(...), and in the finally block restore the previous
value (if previousValue == null then MDC.remove(REQUEST_ID_KEY) else
MDC.put(REQUEST_ID_KEY, previousValue)) so the original requestId is preserved
when present.
| @Component | ||
| @RequiredArgsConstructor | ||
| @Slf4j | ||
| public class ArticleBackupWriter implements ItemWriter<ArticleBackupDto>, StepExecutionListener { | ||
|
|
||
| private final S3BackupService s3BackupService; | ||
| private static final String BACKUP_COUNT_KEY = "backupCount"; | ||
| private final List<ArticleBackupDto> aggregatedArticles = new ArrayList<>(); | ||
|
|
There was a problem hiding this comment.
싱글톤 Writer의 상태 관리 문제로 인한 동시성 위험
@Component로 등록된 Writer는 Spring 싱글톤이지만, aggregatedArticles 인스턴스 필드는 가변 상태입니다. 수동 트리거 API와 스케줄러가 동시에 Job을 실행하거나, Job이 실패 후 재시작될 경우 다음 문제가 발생합니다:
- 동시 실행 시 여러 Job 인스턴스가 동일한
aggregatedArticles를 동시에 수정 → 데이터 손실/오염 - 실패 후 재시작 시 이전 실행의 잔여 데이터가 남아있어 중복 백업 가능
해결 방안:
StepExecution의 ExecutionContext에 데이터를 저장하거나, @StepScope를 사용하여 Step 실행마다 Writer 인스턴스를 새로 생성하세요.
-@Component
+@Component
+@StepScope
@RequiredArgsConstructor
@Slf4j
public class ArticleBackupWriter implements ItemWriter<ArticleBackupDto>, StepExecutionListener {
private final S3BackupService s3BackupService;
private static final String BACKUP_COUNT_KEY = "backupCount";
- private final List<ArticleBackupDto> aggregatedArticles = new ArrayList<>();
+ private List<ArticleBackupDto> aggregatedArticles = new ArrayList<>();또는 ExecutionContext를 사용하는 방식:
// write() 메서드에서
var ec = stepExecution.getExecutionContext();
List<ArticleBackupDto> aggregated = (List<ArticleBackupDto>) ec.get("aggregatedArticles");
if (aggregated == null) {
aggregated = new ArrayList<>();
}
aggregated.addAll(articles);
ec.put("aggregatedArticles", aggregated);🤖 Prompt for AI Agents
In monew/src/main/java/com/spring/monew/batch/writer/ArticleBackupWriter.java
around lines 20 to 28, the writer holds mutable instance state
(aggregatedArticles) on a singleton bean which causes concurrency issues and
stale data on restarts; fix by removing the shared field and either (A) annotate
the writer with @StepScope so a new writer instance is created per step
execution and move aggregatedArticles to an instance field there, or (B) persist
per-run state in the StepExecution.getExecutionContext(): on write() read the
list from the ExecutionContext, create if null, append current items, and put it
back into the ExecutionContext; ensure you also remove/clear the key on step
completion or job restart to avoid residual data.
3 participants
Issues
✔️ Check-list
🗒️ Work Description
뉴스 기사 삭제 및 복구 기능을 구현. 소프트 삭제(논리적 삭제), 하드 삭제(물리적 삭제), AWS S3 백업, 복원 기능, 요청 추적 및 로그 관리 기능
1️⃣ 소프트 삭제 (Soft Delete)
DELETE /api/articles/{articleId}2️⃣ 하드 삭제 (Hard Delete)
DELETE /api/articles/{articleId}/hard3️⃣ 기사 복원 (Restore)
GET /api/articles/{articleId}/restore4️⃣ S3 자동 백업 시스템
백업 배치 작업 (매일 새벽 2시 실행):
deleted-articles-YYYY-MM-DD.jsonmonew-article-backups로그 백업 배치 작업 (매일 새벽 1시 실행):
monew-application-YYYY-MM-DD.log.gzmonew-bucket-s3-logback수동 백업 트리거 API:
POST /api/batch/trigger/article-backupPOST /api/batch/trigger/log-backup5️⃣ 요청 추적 및 감사 로깅
Request ID 필터:
X-Request-ID포함감사 로그 (AuditLogger):
Logback 파일 로깅:
monew-application-YYYY-MM-DD.log)📷 Screenshot
관심사 삭제 시 관련 기사 논리 삭제
관심사도 논리 삭제 됩니다.
로그 S3 백업 (로그 백업 수동 트리거 사용)
S3에서 로그가 잘 저장 되었습니다.
뉴스 기사 백업 (수동트리거를 통해 테스트함)
수동트리거는 어제부터 오늘까지 기사 백업함( 기존 배치 스케줄러는 어제 기사만)
S3에서 뉴스 기사가 잘 백업 되었습니다.
복구
복구를 하기 위해 논리삭제 자동 물리 삭제 트리거를 만들었습니다.
뉴스 기사 배치 자동 삭제 트리거 - 30일 전부터 오늘까지 ( 원래 30일 지나야 자동 삭제 됨)
논리적 삭제 기사들 자동 삭제
http://localhost:8080/api/batch/trigger/article-cleanup
기존 DB 논리삭제됨
삭제 트리거
DB에서 없어짐
기사 복구
복구 완료
📚 Reference
Summary by CodeRabbit