refactor: Board controller, service 계층 리팩토링/테스트 코드 추가 #106
Conversation
Walkthrough이 변경사항은 게시물(보드) 존재 여부에 대한 상태 코드 추가, BoardService의 리팩토링 및 메서드 분리, BoardController의 일부 메서드 위치 변경, 그리고 BoardController와 BoardService에 대한 단위 테스트 신규 추가로 구성되어 있습니다. 주요 로직 변화 없이 구조 개선과 테스트 커버리지가 강화되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BoardController
participant BoardService
participant BoardRepository
User->>BoardController: isBoardSaved 요청
BoardController->>BoardService: isBoardSaved(user, problemNumberRequest)
BoardService->>BoardRepository: findByUserAndProblem(user, problem)
BoardRepository-->>BoardService: Board or null
BoardService->>BoardService: boardSavedStatus(board)
BoardService-->>BoardController: ResultCode, BoardDetailResponseDTO
BoardController-->>User: ResultResponse(ResultCode, Board)
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
src/main/java/com/Alchive/backend/config/result/ResultCode.java (1)
35-35: 오타가 있습니다: "EXSIST"
BOARD_ALREADY_EXSIST에서 "EXSIST"는 오타입니다. 정확한 영어 철자는 "EXIST"입니다.- BOARD_ALREADY_EXSIST("B007", "존재하는 게시물"), + BOARD_ALREADY_EXIST("B007", "존재하는 게시물"),다만, 이미 다른 코드에서 이 상수를 사용 중이라면 수정 시 연관된 모든 코드도 함께 수정해야 합니다.
src/test/java/com/Alchive/backend/service/BoardServiceTest.java (2)
80-86: 주석 처리된 테스트 메서드 정리 필요주석 처리된
toBoardSolutionDtoSuccess테스트 메서드가 있습니다. 이 테스트를 완성하여 사용할 계획이 있다면 TODO 주석을 추가하고, 그렇지 않다면 제거하는 것이 좋습니다.-// @DisplayName("게시물, 문제, 풀이 묶어서 반환 - 성공") -// @Test -// public void toBoardSolutionDtoSuccess() { -// when(problemRepository.findById(problemId)).thenReturn(Optional.ofNullable(problem)); -// when(algorithmProblemRepository.findAlgorithmNamesByProblemId(problemId)).thenReturn(algorithms); -// when(solutionRepository.findAllByBoard_Id(boardId)).thenReturn(new ArrayList<>()); -// }또는 향후 구현을 위해 유지하려면:
+// TODO: 이 테스트 메서드 구현 완료 필요 // @DisplayName("게시물, 문제, 풀이 묶어서 반환 - 성공") // @Test // public void toBoardSolutionDtoSuccess() { // when(problemRepository.findById(problemId)).thenReturn(Optional.ofNullable(problem)); // when(algorithmProblemRepository.findAlgorithmNamesByProblemId(problemId)).thenReturn(algorithms); // when(solutionRepository.findAllByBoard_Id(boardId)).thenReturn(new ArrayList<>()); // }
162-163: 불필요한 빈 줄연속된 빈 줄이 있습니다. 코드 가독성을 위해 하나의 빈 줄만 남기는 것이 좋습니다.
- - +src/main/java/com/Alchive/backend/service/BoardService.java (3)
64-68: 조회 전용 트랜잭션(readOnly) 설정 권장
getBoardDetail은 단순 조회 로직이므로@Transactional(readOnly = true)를 붙이면
- 불필요한 쓰기 잠금 방지
- 스프링 JPA 최적화(더티 체킹 생략)
효과를 기대할 수 있습니다.
70-77:createBoard내부 다중 쿼리로 인한 경쟁 조건 가능성
createProblem(존재 여부 확인 후 저장) → 이후findByNumberAndPlatform순서로 두 번 조회가 실행됩니다. 동시성 상황에서 다음 시나리오가 가능합니다.
- 스레드 A :
createProblem존재 검사 통과 직전- 스레드 B : 동일 문제를 먼저 저장
- 스레드 A : 검사 통과 후
save, 중복 키 예외
Problem의(number, platform)컬럼에 UNIQUE 제약을 두고,save시 예외를 캐치하여 재조회하거나,findOrCreate패턴으로 한 번의 쿼리로 처리하는 방식이 안전합니다.
138-143: 알고리즘-문제 연결 시 불필요한 반복 쿼리
for루프마다
existsByNamefindByName
등 최소 2회 DB 접근이 발생합니다. 대량 알고리즘 입력 시 성능 저하가 큽니다.배치 방식으로 일괄 조회 후 메모리에서 필터링하거나 JPA
saveAll을 활용하는 리팩터를 고려해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/Alchive/backend/config/result/ResultCode.java(1 hunks)src/main/java/com/Alchive/backend/controller/BoardController.java(2 hunks)src/main/java/com/Alchive/backend/service/BoardService.java(3 hunks)src/test/java/com/Alchive/backend/controller/BoardControllerTest.java(1 hunks)src/test/java/com/Alchive/backend/service/BoardServiceTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (9)
src/main/java/com/Alchive/backend/controller/BoardController.java (2)
39-39: 코드 단순화 - 좋은 개선사항입니다!null 체크를 명시적으로 수행하는 대신 서비스 계층에서 제공하는 상태 코드를 직접 사용하도록 변경하여 코드가 더 간결해졌습니다. 이는 컨트롤러와 서비스 계층 간의 책임 분리를 잘 보여주는 예입니다.
77-83: 메서드 위치 변경메서드의 위치만 변경되었고 구현은 동일합니다. 메서드의 순서가 중요한 특별한 이유가 있나요? 아니면 단순히 코드 구조 개선을 위한 변경인가요?
src/test/java/com/Alchive/backend/controller/BoardControllerTest.java (2)
67-77: 기존 게시물 테스트 구현이 잘 되었습니다게시물이 이미 존재하는 경우에 대한 테스트가 잘 구현되었습니다. Mockito를 사용하여 서비스 계층의 응답을 적절히 모킹하고, 컨트롤러의 응답 메시지를 검증하는 방식이 좋습니다.
테스트를 더욱 견고하게 만들기 위해 메시지뿐만 아니라 상태 코드와 응답 본문의 다른 속성들도 검증하는 것을 고려해보세요.
79-88: 존재하지 않는 게시물 테스트 구현이 잘 되었습니다존재하지 않는 게시물에 대한 테스트가 잘 구현되었습니다. nullable 파라미터를 처리하는 방식과 BOARD_NOT_EXIST 상태 코드 검증이 적절합니다.
메시지 확인 외에도 HTTP 상태 코드가 200 OK인지 확인하는 테스트를 추가하는 것도 좋을 것 같습니다.
src/test/java/com/Alchive/backend/service/BoardServiceTest.java (5)
88-103: 게시물 존재 여부 테스트가 잘 구현되었습니다
isBoardSavedTrue테스트 메서드가 잘 구현되었습니다. Board, Problem, Algorithm, Solution 등 여러 엔티티 간의 관계를 적절히 모킹하고 검증하는 방식이 좋습니다.추가로 결과값의 다른 속성들(예: problem 정보, board 상태 등)도 검증하면 더 견고한 테스트가 될 것 같습니다.
105-114: 게시물이 없는 경우 테스트가 잘 구현되었습니다
isBoardSavedFalse테스트 메서드가 간결하고 명확하게 구현되었습니다. Optional.empty()를 반환하도록 설정하고 결과가 null인지 확인하는 방식이 적절합니다.
116-130: 기존 문제에 대한 게시물 생성 테스트가 잘 구현되었습니다이미 존재하는 문제에 대해 게시물을 생성하는 시나리오를 잘 테스트하고 있습니다. 다양한 속성을 검증하는 부분이 좋습니다.
테스트를 더 견고하게 만들기 위해 problem 정보도 검증하는 것을 고려해보세요.
164-173: 작성자 메모 수정 테스트가 잘 구현되었습니다작성자가 메모를 수정하는 시나리오를 잘 테스트하고 있습니다. doNothing()을 사용하여 userService.validateUser() 메서드의 동작을 모킹하는 방식이 적절합니다.
다만, 메모가 실제로 업데이트되었는지 검증하는 부분이 없습니다. updateBoardMemo 메서드 호출 후 board 객체의 memo 값이 변경되었는지 확인하는 테스트를 추가하면 좋을 것 같습니다.
175-184: 비작성자의 메모 수정 시도 테스트가 잘 구현되었습니다작성자가 아닌 사용자가 메모를 수정하려고 할 때 예외가 발생하는지 테스트하는 부분이 잘 구현되었습니다. doCallRealMethod()를 사용하여 실제 validateUser() 메서드의 동작을 호출하도록 한 부분이 좋습니다.
예외 검증 시 예외 메시지까지 확인하면 더 정확한 테스트가 될 것 같습니다.
| public Page<List<BoardDetailResponseDTO>> getBoardList(int offset, int limit) { | ||
| Pageable pageable = PageRequest.of(offset, limit); | ||
| Page<Board> boardPage = boardRepository.findAll(pageable); | ||
|
|
||
| // Board를 BoardDetailResponseDTO로 변환 | ||
| List<BoardDetailResponseDTO> boardList = boardPage.getContent().stream() | ||
| .map(this::toBoardDetailResponseDTO) | ||
| .toList(); | ||
|
|
||
| // 변환된 리스트를 새로운 Page 객체로 감싸서 반환 | ||
| return new PageImpl<>(List.of(boardList), pageable, boardPage.getTotalElements()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Page<List<BoardDetailResponseDTO>> → 중첩 리스트 반환 구조 문제
현재 구현은
① boardList : List<BoardDetailResponseDTO>
② new PageImpl<>(List.of(boardList), …) 로 한 번 더 래핑하여 List<List<BoardDetailResponseDTO>> 형태로 페이지네이션됩니다.
컨트롤러/프론트단이 예상하는 스키마와 달라질 위험이 크며 직렬화 시 JSON 구조도 두 단계로 중첩됩니다.
아래와 같이 제너릭 타입을 단일 DTO 로 변경하고 불필요한 List.of 래핑을 제거해 주세요.
- public Page<List<BoardDetailResponseDTO>> getBoardList(int offset, int limit) {
+ public Page<BoardDetailResponseDTO> getBoardList(int offset, int limit) {
...
- return new PageImpl<>(List.of(boardList), pageable, boardPage.getTotalElements());
+ return new PageImpl<>(boardList, pageable, boardPage.getTotalElements());
}📝 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.
| public Page<List<BoardDetailResponseDTO>> getBoardList(int offset, int limit) { | |
| Pageable pageable = PageRequest.of(offset, limit); | |
| Page<Board> boardPage = boardRepository.findAll(pageable); | |
| // Board를 BoardDetailResponseDTO로 변환 | |
| List<BoardDetailResponseDTO> boardList = boardPage.getContent().stream() | |
| .map(this::toBoardDetailResponseDTO) | |
| .toList(); | |
| // 변환된 리스트를 새로운 Page 객체로 감싸서 반환 | |
| return new PageImpl<>(List.of(boardList), pageable, boardPage.getTotalElements()); | |
| } | |
| public Page<BoardDetailResponseDTO> getBoardList(int offset, int limit) { | |
| Pageable pageable = PageRequest.of(offset, limit); | |
| Page<Board> boardPage = boardRepository.findAll(pageable); | |
| List<BoardDetailResponseDTO> boardList = boardPage.getContent().stream() | |
| .map(this::toBoardDetailResponseDTO) | |
| .toList(); | |
| return new PageImpl<>(boardList, pageable, boardPage.getTotalElements()); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/Alchive/backend/service/BoardService.java around lines 53
to 62, the method getBoardList currently returns a Page of List of
BoardDetailResponseDTO, causing an unintended nested list structure. To fix
this, change the return type to Page<BoardDetailResponseDTO> and remove the
List.of wrapping around boardList when creating the PageImpl, so that the
pagination directly wraps the list of DTOs without extra nesting.
| public void createAlgorithm(String algorithmName) { | ||
| if (algorithmRepository.existsByName(algorithmName)) { | ||
| return; | ||
| } | ||
| Algorithm newAlgorithm = Algorithm.of(algorithmName); | ||
| algorithmRepository.save(newAlgorithm); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
createAlgorithm 경합 조건 및 중복 삽입 위험
existsByName 검사 후 save 로직은 트랜잭션 격리 수준이 높지 않으면 동시 요청에서 동일 이름이 중복 삽입될 수 있습니다.
- DB 레벨 UNIQUE 제약을 우선 추가하고
existsByName대신 예외 캐치 후 무시 패턴으로 단순화하면 I/O를 줄일 수 있습니다.
- if (algorithmRepository.existsByName(algorithmName)) {
- return;
- }
- Algorithm newAlgorithm = Algorithm.of(algorithmName);
- algorithmRepository.save(newAlgorithm);
+ try {
+ algorithmRepository.save(Algorithm.of(algorithmName));
+ } catch (DataIntegrityViolationException e) {
+ // UNIQUE 제약 위반 → 이미 존재하므로 무시
+ }📝 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.
| public void createAlgorithm(String algorithmName) { | |
| if (algorithmRepository.existsByName(algorithmName)) { | |
| return; | |
| } | |
| Algorithm newAlgorithm = Algorithm.of(algorithmName); | |
| algorithmRepository.save(newAlgorithm); | |
| public void createAlgorithm(String algorithmName) { | |
| try { | |
| algorithmRepository.save(Algorithm.of(algorithmName)); | |
| } catch (DataIntegrityViolationException e) { | |
| // UNIQUE 제약 위반 → 이미 존재하므로 무시 | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/Alchive/backend/service/BoardService.java around lines 146
to 151, the createAlgorithm method risks duplicate inserts due to a race
condition between existsByName check and save. To fix this, add a UNIQUE
constraint on the algorithm name column at the database level, then remove the
existsByName check and instead attempt to save directly. Catch any exceptions
caused by unique constraint violations and ignore them to prevent duplicates and
reduce I/O operations.
| public ResultCode boardSavedStatus(BoardDetailResponseDTO board) { | ||
| if (board != null) { | ||
| return ResultCode.BOARD_ALREADY_EXSIST; | ||
| } | ||
| return ResultCode.BOARD_NOT_EXIST; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
boardSavedStatus 역할 중복 및 오타 가능성
- 이미
isBoardSaved➡null/BoardDetailResponseDTO로 존재 여부를 판단하고 있는데,boardSavedStatus가 추가로 한 번 더 판단하도록 설계돼 중복 로직이 발생했습니다. 두 메서드 중 하나만으로도 충분하므로 응답 DTO 또는ResultCode둘 중 하나로 통일하는 편이 깔끔합니다. ResultCode.BOARD_ALREADY_EXSIST→ “EXIST” 철자 오타 가능성이 있습니다. 실제 enum 정의를 한 번 더 확인해 주세요(테스트 코드에서 찾기 어려움).
- return ResultCode.BOARD_ALREADY_EXSIST;
+ return ResultCode.BOARD_ALREADY_EXIST; // enum 정의에 맞춰 수정다음 스크립트를 실행하여 isBoardSaved 메서드 로직 중복 여부와 ResultCode enum 상수 철자(BOARD_ALREADY_EXSIST vs BOARD_ALREADY_EXIST)를 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 1) isBoardSaved 메서드 위치 및 구현 확인
rg -n -C 3 "isBoardSaved" src/main/java/com/Alchive/backend/service/BoardService.java
# 2) ResultCode enum 정의 파일 검색
rg -n "enum ResultCode" .
# 3) BOARD_ALREADY_* 상수 목록 출력
rg -n "BOARD_ALREADY" .Length of output: 1291
boardSavedStatus 메서드 로직 중복 제거 및 상수 철자 수정 필요
isBoardSaved()와 boardSavedStatus()가 게시물 존재 여부를 각각 판단하면서 중복 로직이 발생하고, ResultCode enum에 BOARD_ALREADY_EXSIST 오타가 있어 아래와 같이 리팩토링을 권장드립니다.
대상 파일 및 수정 포인트:
- src/main/java/com/Alchive/backend/config/result/ResultCode.java
•BOARD_ALREADY_EXSIST→BOARD_ALREADY_EXIST로 상수명 수정 - src/main/java/com/Alchive/backend/service/BoardService.java
•isBoardSaved+boardSavedStatus통합 → 게시물 조회 후 곧바로ResultCode반환 - src/test/java/com/Alchive/backend/controller/BoardControllerTest.java
• 테스트 내BOARD_ALREADY_EXSIST→BOARD_ALREADY_EXIST로 수정
예시 diff:
--- a/src/main/java/com/Alchive/backend/config/result/ResultCode.java
@@ -35,7 +35,7 @@ public enum ResultCode {
- BOARD_ALREADY_EXSIST("B007", "존재하는 게시물"),
+ BOARD_ALREADY_EXIST ("B007", "존재하는 게시물"),
--- a/src/main/java/com/Alchive/backend/service/BoardService.java
@@ -38,20 +38,19 @@ public class BoardService {
- public BoardDetailResponseDTO isBoardSaved(User user, ProblemNumberRequest request) {
- Optional<Board> board = boardRepository.findByProblem_PlatformAndProblem_NumberAndUser_Id(
- request.getPlatform(), request.getProblemNumber(), user.getId());
- return board.map(this::toBoardDetailResponseDTO).orElse(null);
- }
-
- public ResultCode boardSavedStatus(BoardDetailResponseDTO board) {
- if (board != null) {
- return ResultCode.BOARD_ALREADY_EXSIST;
- }
- return ResultCode.BOARD_NOT_EXIST;
- }
+ public ResultCode getBoardStatus(User user, ProblemNumberRequest request) {
+ return boardRepository
+ .findByProblem_PlatformAndProblem_NumberAndUser_Id(
+ request.getPlatform(), request.getProblemNumber(), user.getId()
+ )
+ .map(b -> ResultCode.BOARD_ALREADY_EXIST)
+ .orElse(ResultCode.BOARD_NOT_EXIST);
+ }컨트롤러 호출부 예시:
- BoardDetailResponseDTO response = boardService.isBoardSaved(user, request);
- ResultCode status = boardService.boardSavedStatus(response);
+ ResultCode status = boardService.getBoardStatus(user, request);테스트에서도 동일하게 상수명 변경을 적용해 주세요.
📝 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.
| public ResultCode boardSavedStatus(BoardDetailResponseDTO board) { | |
| if (board != null) { | |
| return ResultCode.BOARD_ALREADY_EXSIST; | |
| } | |
| return ResultCode.BOARD_NOT_EXIST; | |
| } | |
| // Remove the old isBoardSaved() and boardSavedStatus() methods, and replace with: | |
| public ResultCode getBoardStatus(User user, ProblemNumberRequest request) { | |
| return boardRepository | |
| .findByProblem_PlatformAndProblem_NumberAndUser_Id( | |
| request.getPlatform(), | |
| request.getProblemNumber(), | |
| user.getId() | |
| ) | |
| .map(b -> ResultCode.BOARD_ALREADY_EXIST) | |
| .orElse(ResultCode.BOARD_NOT_EXIST); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/Alchive/backend/service/BoardService.java lines 46-51,
remove the redundant boardSavedStatus method since isBoardSaved already checks
board existence; unify the logic to return the appropriate ResultCode directly
after board lookup. Also, in
src/main/java/com/Alchive/backend/config/result/ResultCode.java, correct the
typo BOARD_ALREADY_EXSIST to BOARD_ALREADY_EXIST. Finally, update all references
in tests such as
src/test/java/com/Alchive/backend/controller/BoardControllerTest.java to use the
corrected enum constant name BOARD_ALREADY_EXIST.
Summary
Description
리팩토링
테스트 코드
Screenshot
Test Checklist
Summary by CodeRabbit
버그 수정
리팩터링
테스트