refactor : 유저 문제 풀이 기록 에러 반환 수정 #188
Merged
Conversation
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
Walkthrough관리자용 문제 ID 조회 기능이 추가되었다. 컨트롤러에 GET Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin
participant Controller as ProblemAdminController
participant AppService as ProblemService
participant Domain as ProblemDomainService
participant Repo as ProblemRepository
participant JPA as ProblemJpaRepository/DB
Admin->>Controller: GET /api/admin/problems/list
Note over Controller: @PreAuthorize(ROLE_ADMIN)
Controller->>AppService: getProblemIdList()
AppService->>Domain: getProblemIdList()
Domain->>Repo: getProblemIdList()
Repo->>JPA: findAll()
JPA-->>Repo: List<Problem>
Repo-->>Domain: List<Long> (IDs)
Domain-->>AppService: List<Long>
AppService-->>Controller: List<Long>
Controller-->>Admin: 200 OK + List<Long>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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: 5
🧹 Nitpick comments (2)
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java (1)
204-206: 읽기 전용 트랜잭션으로 최적화ID 리스트 조회는 읽기 전용입니다. 아래와 같이 지정하세요.
+ @Transactional(readOnly = true) public List<Long> getProblemIdList() { return problemDomainService.getProblemIdList(); }src/main/java/org/ezcode/codetest/domain/problem/service/ProblemDomainService.java (1)
135-137: 애플리케이션 서비스에 읽기 전용 트랜잭션 적용
- 파일: src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java
메서드: public List getProblemIdList() — @transactional(readOnly = true) 추가하여 불필요한 플러시/락 최소화.- 권장(선택): ProblemRepositoryImpl.getProblemIdList()가 problemJpaRepository.findAll()로 엔티티 전체를 로드하므로, 성능·메모리 개선을 위해 「select p.id from Problem p」와 같이 id만 조회하는 전용 쿼리로 변경 고려.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java(2 hunks)src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java(0 hunks)src/main/java/org/ezcode/codetest/domain/problem/repository/ProblemRepository.java(2 hunks)src/main/java/org/ezcode/codetest/domain/problem/service/ProblemDomainService.java(2 hunks)src/main/java/org/ezcode/codetest/infrastructure/persistence/config/QuerydslConfig.java(2 hunks)src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/problem/ProblemRepositoryImpl.java(2 hunks)src/main/java/org/ezcode/codetest/presentation/problemmanagement/problem/ProblemAdminController.java(3 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-14T14:33:58.372Z
Learnt from: Kimminu7
PR: ezcode-my/backend#63
File: src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/problem/ProblemQueryRepositoryImpl.java:24-40
Timestamp: 2025-06-14T14:33:58.372Z
Learning: ProblemController에서 ProblemSearchCondition 객체는 항상 new ProblemSearchCondition(category, difficulty)로 유효한 인스턴스를 생성해서 전달하므로, ProblemQueryRepositoryImpl의 searchByCondition 메서드에서 searchCondition 파라미터 자체에 대한 null 체크는 불필요하다. category와 difficulty 필드만 각각 null일 수 있다.
Applied to files:
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/problem/ProblemRepositoryImpl.java
📚 Learning: 2025-06-02T06:57:37.929Z
Learnt from: NCookies
PR: ezcode-my/backend#17
File: src/main/java/org/ezcode/codetest/presentation/problemmanagement/ProblemAdminController.java:25-25
Timestamp: 2025-06-02T06:57:37.929Z
Learning: 이 프로젝트에서는 아직 Spring Security가 적용되지 않아서 PreAuthorize 어노테이션이 주석 처리되어 있음. Spring Security 도입 전까지는 권한 검증 어노테이션들이 비활성화 상태임.
Applied to files:
src/main/java/org/ezcode/codetest/presentation/problemmanagement/problem/ProblemAdminController.java
⏰ 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
🔇 Additional comments (2)
src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/problem/ProblemRepositoryImpl.java (1)
57-62: 수정 필요 — Soft delete 반영 및 findAll() 전량 로드 개선 (검증 필요)파일: src/main/java/org/ezcode/codetest/infrastructure/persistence/repository/problem/ProblemRepositoryImpl.java (57–62)
- 문제: soft delete 미반영(삭제된 항목 포함) 및 ID만 필요한데 findAll()으로 전체 엔티티 전량 로드.
- 빠른 수정(즉시 적용 가능):
@Override public List<Long> getProblemIdList() { - return problemJpaRepository.findAll().stream() - .map(Problem::getId) - .toList(); + return problemJpaRepository.findAll().stream() + .filter(p -> !p.isDeleted()) + .map(Problem::getId) + .toList(); }
- 권장 수정(효율적, DB에서 ID만 조회):
추가:@Query("select p.id from Problem p where p.isDeleted = false") List<Long> findAllActiveIds();교체:
- return problemJpaRepository.findAll().stream() - .filter(p -> !p.isDeleted()) - .map(Problem::getId) - .toList(); + return problemJpaRepository.findAllActiveIds();검증 필요: 제공된 스크립트 실행 결과 "No files were searched"로 파일 검색이 실패했습니다. 리포지토리 루트에서 아래 명령을 실행해 확인하세요:
# .toList 사용 검사 git grep -n '\.toList(' || rg -n --hidden '\.toList\(\)' # 빌드 설정(프로젝트 Java 버전) 확인 git grep -n 'sourceCompatibility\|maven.compiler.source\|java.version' || true # 런타임 자바 버전 확인 java -versionsrc/main/java/org/ezcode/codetest/infrastructure/persistence/config/QuerydslConfig.java (1)
6-16: 정정: JPAQueryFactory(JPQLTemplates.DEFAULT, EntityManager) 생성자 사용은 유효 — 변경 불필요JPAQueryFactory는 JPQLTemplates와 EntityManager를 받는 생성자를 제공합니다(예: Querydsl 4.x/5.x). (querydsl.com)
제안된 new Configuration(...) 대체는 잘못된 타입/네임스페이스 사용으로 오류를 유발하므로 적용하지 마십시오.Likely an incorrect or invalid review comment.
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.PageImpl; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.http.ResponseEntity; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
애플리케이션 서비스에 불필요한 ResponseEntity import
컨트롤러 전용 타입이므로 import 를 제거하세요.
-import org.springframework.http.ResponseEntity;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import org.springframework.http.ResponseEntity; |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/problem/service/ProblemService.java
around line 29, remove the unnecessary import of
org.springframework.http.ResponseEntity (it's a controller-only type); delete
that import line and run import organization (or IDE optimize imports) to ensure
no unused imports remain and the file compiles cleanly.
| import org.ezcode.codetest.domain.problem.model.entity.Problem; | ||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.http.ResponseEntity; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
도메인 계층에 web 의존 침투(ResponseEntity import 제거 필요)
Repository 인터페이스가 org.springframework.http.ResponseEntity를 import 하고 있습니다. 계층 경계 위반입니다. 제거하세요.
적용 diff:
-import org.springframework.http.ResponseEntity;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import org.springframework.http.ResponseEntity; |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/domain/problem/repository/ProblemRepository.java
around line 10, the repository interface improperly imports
org.springframework.http.ResponseEntity (web layer type) which violates domain
layer boundaries; remove that import and change any method signatures or return
types that reference ResponseEntity to appropriate domain-layer types (e.g., the
entity type, Optional<Entity>, List<Entity>, boolean, or a custom domain DTO)
and update callers in the application/service layer to wrap results in
ResponseEntity there instead.
|
|
||
| void problemCountAdjustment(Long problemId, int correctInc); | ||
|
|
||
| List<Long> getProblemIdList(); |
There was a problem hiding this comment.
메서드 계약 모호/부적절: 소프트 삭제 데이터 포함 가능성
프로젝트 전반이 soft delete(isDeleted=false)를 전제로 동작합니다. getProblemIdList()는 삭제된 문제를 배제해야 합니다. 구현/자바독에 명확히 하거나 메서드명을 getActiveProblemIdList로 조정하세요.
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/domain/problem/repository/ProblemRepository.java
around line 29, the method signature List<Long> getProblemIdList() is ambiguous
about soft-deleted rows; update the contract to exclude deleted problems by
either (a) renaming the method to getActiveProblemIdList() and updating its
Javadoc to state it returns only problems with isDeleted = false, or (b) keep
the name but add an explicit implementation/query that filters WHERE is_deleted
= false and add Javadoc clarifying the behavior; ensure callers/tests are
updated to use the new name/semantics.
| import org.ezcode.codetest.domain.problem.repository.ProblemRepository; | ||
| import org.springframework.data.domain.Page; | ||
| import org.springframework.data.domain.Pageable; | ||
| import org.springframework.http.ResponseEntity; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
도메인 서비스에 web 의존 침투(ResponseEntity import 제거 필요)
ResponseEntity import는 불필요하며 계층 침범입니다. 제거하세요.
-import org.springframework.http.ResponseEntity;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import org.springframework.http.ResponseEntity; |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/domain/problem/service/ProblemDomainService.java
around line 21, remove the unnecessary web-layer import
"org.springframework.http.ResponseEntity" and eliminate any usage of
ResponseEntity in this domain service; replace method signatures and return
types with domain-specific types or DTOs (or plain domain objects) and convert
any ResponseEntity-based returns to return those types (or throw domain
exceptions) so the domain layer has no dependency on Spring Web classes.
.../java/org/ezcode/codetest/presentation/problemmanagement/problem/ProblemAdminController.java
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
작업 내용
변경 사항
UserService.createUser()메서드 추가@Email유효성 검증 적용트러블 슈팅
@Transactional이 적용되지 않음this.→AopProxyUtils.사용)해결해야 할 문제
UserController에서 비즈니스 로직 일부 처리 → 서비스로 이전 고려 필요참고 사항
코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
신기능
기타 작업