Merged
Conversation
- CommentResponse, CommentArticleResponse, Reply에 @builder 패턴 적용 - Reply 생성자를 정적 팩토리 메서드 from()으로 변경 - 코드 포맷팅 (들여쓰기 2 spaces 통일) - CommentRepositoryCustom 인터페이스 제거
- NotificationResponse.from()에서 new 대신 @builder 패턴 사용 - 전체 코드 포맷팅 (들여쓰기 2공백 통일) - 불필요한 공백 라인 제거
- Article 도메인 엔티티, DTO, Repository 코드 포맷팅 - 테스트 코드 JUnit assertions에서 AssertJ로 마이그레이션 - ArticleCreateResponse DTO 추가 - 테스트 코드 실제 동작에 맞게 수정
- AuthService: 회원가입, 로그인, 토큰 재발급, 로그아웃 테스트 - TokenService: 토큰 생성, 재발급, 추출 테스트
- CategoryReader 테스트: getCategories, getById, getByMemberIdAndCategoryName, getCategory - CategoryWriter 테스트: createCategory, updateCategory, deleteCategory - BDD 스타일 Mock 검증 및 AssertJ assertions 적용
- CommentReaderTest: getById, getMyComments 테스트 - CommentWriterTest: createComment, updateComment, deleteComment 테스트
- MemberReader: getById, getByNickname, getByEmail, existsByProviderId, existsByEmail 테스트 - MemberWriter: update, deleteById, saveOrUpdate 테스트 - NotificationSettingReader: isDisabled, getNotificationSettings 테스트 - NotificationSettingWriter: createNotificationSetting, toggleNotification 테스트 - MemberService: getMember, updateMember, delete 테스트
|
Caution Review failedThe pull request is closed. Note
|
| Cohort / File(s) | Summary |
|---|---|
설정 및 빌드 build.gradle, gradle.properties, settings.gradle |
dev 도구 제거, springdoc 및 spotless 버전 변경, Java 홈 경로 추가로 인한 멀티모듈 구조 단순화 |
API 문서 제거 .claudedocs/api/*, .claudedocs/screen/*, .claudedocs/db/schema.sql, .claudedocs/tech/tech.md |
OpenAPI 사양 및 설계 문서 일괄 제거 |
아키텍처 참고 문서 추가 .serena/memories/article-domain/structure.md |
아티클 도메인 구조 및 CQRS 패턴 설명 문서 |
보안 및 응답 처리 강화 src/main/java/com/mylog/common/response/ErrorResponse.java, src/main/java/com/mylog/common/response/SuccessResponse.java, src/main/java/com/mylog/common/security/JwtProvider.java, src/main/java/com/mylog/common/security/SecurityConfig.java |
ErrorResponse factory 메서드 추가, SuccessResponse에 HTTP 상태 기반 메서드 추가, JWT refresh 토큰 멤버ID 추출 로직 변경, UserDetailsService 의존성 제거 |
쿼리 설정 src/main/java/com/mylog/config/QuerydslConfig.java |
QueryDSL JPAQueryFactory 빈 등록 및 생성자 기반 의존성 주입 추가 |
아티클 도메인 src/main/java/com/mylog/domain/article/* |
ArticleCreateResponse 신규 추가, Repository QueryDSL 기반 재구현, ArticleReader/Writer 응답 타입 변경, 태그 조회 메서드 제거 |
댓글 도메인 src/main/java/com/mylog/domain/comment/* |
CommentResponse/Reply에 @Builder 추가, 생성자 기반에서 팩토리 메서드로 변경, CommentRepositoryCustom 제거 |
회원 도메인 src/main/java/com/mylog/domain/member/* |
UpdateMemberRequest 검증 어노테이션 재배치, NotificationSetting 저장 메서드 정리 |
알림 도메인 src/main/java/com/mylog/domain/notification/* |
NotificationResponse에 @Builder 추가, 읽음 상태 알림만 조회하는 쿼리 메서드 추가, NotificationRepositoryCustom 제거 |
테스트 src/test/java/com/mylog/domain/*/service/*Test.java |
ArticleReader, ArticleService, ArticleWriter, TagReader, TagWriter, AuthService, TokenService, CategoryReader, CategoryWriter, CommentReader, CommentWriter, MemberService, MemberReader, MemberWriter, NotificationSetting*Reader/Writer, NotificationReader/Writer 단위 테스트 신규 추가 (15개 이상의 테스트 클래스) |
Sequence Diagram(s)
sequenceDiagram
participant Client as Client
participant ArticleController as ArticleController
participant ArticleService as ArticleService
participant ArticleWriter as ArticleWriter
participant ArticleRepository as ArticleRepository (QueryDSL)
participant S3Service as S3Service
Client->>ArticleController: POST /api/articles (multipart form)
ArticleController->>ArticleService: createArticle(request, memberId, file)
ArticleService->>ArticleWriter: create(request, memberId, imageUrl)
ArticleWriter->>ArticleRepository: save(article)
ArticleRepository-->>ArticleWriter: Article (with id)
ArticleWriter->>ArticleRepository: saveTag (async)
ArticleWriter-->>ArticleService: Article
ArticleService-->>ArticleController: ArticleCreateResponse(articleId)
ArticleController-->>Client: 201 Created with response body
sequenceDiagram
participant Client as Client
participant ArticleController as ArticleController
participant ArticleService as ArticleService
participant ArticleReader as ArticleReader
participant ArticleRepositoryImpl as ArticleRepositoryImpl (QueryDSL)
participant Database as Database
Client->>ArticleController: GET /api/articles/all?page=0&size=10
ArticleController->>ArticleService: getArticles(pageable)
ArticleService->>ArticleReader: getArticles(pageable)
ArticleReader->>ArticleRepositoryImpl: findAllCustom(pageable)
ArticleRepositoryImpl->>ArticleRepositoryImpl: Build QueryDSL query with fetch joins
ArticleRepositoryImpl->>Database: Execute: SELECT article JOIN member JOIN category
Database-->>ArticleRepositoryImpl: Results
ArticleRepositoryImpl-->>ArticleReader: Page<Article>
ArticleReader-->>ArticleService: Page<ArticleResponse>
ArticleService-->>ArticleController: Page<ArticleResponse>
ArticleController-->>Client: 200 OK with paginated articles
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
- Recover: 머지 충돌 해결 및 불필요 코드 정리 #51: ArticleController의 createArticle 메서드 서명 변경 및
@MemberId파라미터 처리 통일 - 50: JwtProvider, SecurityConfig 등 보안 계층 변경사항 오버랩
- 56: ArticleRepositoryImpl의 QueryDSL 기반 리팩토링, ArticleService/Reader/Writer 변경사항 오버랩
Poem
쿼리는 DSL로, 응답은 빌더로 🏗️
커스텀 리포지토리 작별 인사,
테스트는 군데군데 촘촘하게,
문서는 사라지고 코드만 남아
리팩토링의 봄이 왔네요 🌱
🔍 코드 리뷰 주요 지적사항
🚨 주의 깊게 검토가 필요한 영역
1. TagWriter.saveTag() 메서드 미구현 (HIGH RISK)
// src/main/java/com/mylog/domain/article/service/TagWriter.java
`@Async`
public void saveTag(List<String> tags, Article article) {
// 메서드가 완전히 비어있음
}문제점:
- 태그 저장 로직이 구현되지 않음
- ArticleWriter.create()가 태그 저장을 호출하지만, 실제 저장이 일어나지 않음
- 비동기 실행이므로 race condition 위험성 존재
- 데이터 일관성 문제: 아티클은 저장되지만 태그는 저장 안 됨
개선 권장사항:
`@Async`
public void saveTag(List<String> tags, Article article) {
if (tags == null || tags.isEmpty()) {
return;
}
// TODO: 태그 저장 로직 구현
// 1. 기존 태그 조회/생성
// 2. ArticleTag 저장
// 3. 저장 실패 시 로깅 및 폴백 처리
}2. NotificationReader.receiveNotification() 필터링 의도 모호 (MEDIUM RISK)
// src/main/java/com/mylog/domain/notification/service/NotificationReader.java
public Page<NotificationResponse> receiveNotification(Long memberId, Pageable pageable) {
Member member = memberReader.getById(memberId);
return repository.findByMemberAndReadTrue(member, pageable)
.map(NotificationResponse::from);
}문제점:
- 읽지 않은 알림(read=false)은 조회하지 않음
- API 응답으로 읽음 상태 알림만 반환
- 읽지 않은 알림은 어디서 조회하나? 별도 메서드 필요한 상태
개선 권장사항:
- 메서드명 명확히:
receiveReadNotifications()또는getReadNotifications() - 문서화: JavaDoc에 "읽음 상태의 알림만 조회" 명시
3. JwtProvider.getRefreshMemberId() 타입 변환 안전성 (MEDIUM RISK)
public long getRefreshMemberId(String token) {
Claims claims = getRefreshClaims(token);
return claims.get(MEMBER_ID, Long.class); // Long 타입으로 클레임 추출
}문제점:
- 클레임에 MEMBER_ID가 없으면 null 반환
- null을 primitive long으로 언박싱하면 NullPointerException 발생 가능
- 에러 핸들링 부재
개선 권장사항:
public long getRefreshMemberId(String token) {
Claims claims = getRefreshClaims(token);
Object memberId = claims.get(MEMBER_ID);
if (memberId == null) {
throw new BusinessException(ErrorCode.TOKEN_INVALID);
}
return ((Number) memberId).longValue();
}4. ArticleWriter.create() 반환값 변경 및 태그 저장 순서 (MEDIUM RISK)
public Article create(ArticleCreateRequest request, Long memberId, String imageUrl) {
// ... 아티클 생성 로직 ...
Article article = articleRepository.save(article); // DB에 저장
if (!request.tagNames().isEmpty()) {
tagWriter.saveTag(request.tagNames(), article); // 비동기 호출
}
return article; // 저장된 아티클 반환 (tag는 비동기로 나중에 저장)
}문제점:
- 아티클은 DB에 저장되지만, 태그는 비동기로 저장됨
- 클라이언트가 즉시 아티클 ID를 받고 조회할 때 태그가 없을 수 있음
- 비동기 저장 실패 시 처리 메커니즘 없음
- 응답 구성 단계와 저장 단계의 불일치
개선 권장사항:
public Article create(ArticleCreateRequest request, Long memberId, String imageUrl) {
// ... 동기적으로 아티클+태그 저장 완료 후 반환
Article article = articleRepository.save(article);
if (!request.tagNames().isEmpty()) {
tagWriter.saveTag(request.tagNames(), article);
// 비동기를 원한다면 CompletableFuture 활용 및 로깅/재시도 정책 추가
}
return article;
}5. SecurityConfig에서 UserDetailsService 제거 (LOW-MEDIUM RISK)
// Before:
public SecurityFilterChain filterChain(HttpSecurity http, UserDetailsService userDetailsService) throws Exception
// After:
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception확인 사항:
- UserDetailsService가 다른 곳에서 주입되는가?
- CustomUserDetailsService가 별도로 빈으로 등록되어 있는가?
- Spring Security의 기본 UserDetailsService 빈이 생성되는가?
- 테스트에서 UserDetailsService 모킹이 필요한 경우 영향이 있을 수 있음
6. S3 파일 삭제 권한 검증 누락 (MEDIUM RISK)
// src/main/java/com/mylog/external/s3/S3Service.java
public void deleteImage(String imageUrl) {
// 이미지 URL의 소유권 검증 없음
// 아무 URL이나 삭제 가능할 수도 있음
}개선 권장사항:
- 삭제 전 이미지 URL이 현재 사용자의 자산인지 검증
- ArticleService 레벨에서 권한 체크 후 호출하는지 확인
✅ 좋은 개선사항
- ErrorResponse Factory 메서드: 다양한 에러 타입 처리 표준화
- SuccessResponse HTTP 상태 메서드: REST 응답 규칙 준수 강화
- QueryDSL 도입: 타입 안전 쿼리, N+1 문제 해결, 유지보수성 향상
- 광범위한 단위 테스트: 도메인 로직 검증 강화
- @builder 패턴: 불변성 강화 및 복잡한 객체 생성 간소화
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
dev
Merged
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
1 participant
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.
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항