Conversation
📝 WalkthroughWalkthrough컨트롤러의 클래스 레벨 API 버전핑 제거 및 엔드포인트별 버전 명시, raw product용 v2 찜(toggle/조회) 기능 추가(서비스·레포·페이사드·DTO·테스트 포함). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Client
participant Controller as JjymController\n(v2 endpoints)
participant Facade as JjymOptimisticLockFacade
participant Service as JjymServiceImpl
participant Repo as Repository Layer
participant DB as Database
User->>Client: 찜 토글 요청 (rawProduct)
Client->>Controller: POST /api/v2/curation-raw-products/{id}/jjym
Controller->>Facade: toggleRawProduct(user, rawProductId)
loop retry with backoff
Facade->>Service: rawProductJjymToggle(userId, rawProductId)
Service->>Repo: find User, find CurationRawProduct
Repo->>DB: select by id
DB-->>Repo: entities
Repo-->>Service: entities
Service->>Repo: find RecommendFurniture by (RAW, productId)
Repo->>DB: select recommend_furniture
DB-->>Repo: entity or empty
alt recommend missing
Service->>Repo: save RecommendFurniture
Repo->>DB: insert
DB-->>Repo: saved entity
end
Service->>Repo: find/delete or save Jjym
Repo->>DB: transaction
DB-->>Repo: success or optimistic lock error
end
Service-->>Facade: true/false
Facade-->>Controller: result
Controller-->>Client: ApiResponse<JjymToggleResponse>
User->>Client: 찜 목록 조회 요청
Client->>Controller: GET /api/v2/jjyms
Controller->>Service: getMyRawProductJjyms(userId)
Service->>Repo: find Jjyms with RecommendFurniture
Repo->>DB: join query
DB-->>Repo: jjym+rf rows
Repo-->>Service: results
Service->>Repo: findAllByProductIdIn(productIds)
Repo->>DB: bulk select raw products (latest per productId)
DB-->>Repo: raw product rows
Repo-->>Service: raw products
Service->>Repo: find colors by rawProductIds
Repo->>DB: select colors
DB-->>Repo: color rows
Repo-->>Service: colors
Service->>Repo: count jjym by recommendFurnitureIds
Repo->>DB: group count
DB-->>Repo: counts
Repo-->>Service: counts
Service-->>Controller: JjymV2ListResponse
Controller-->>Client: ApiResponse<JjymV2ListResponse>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Test Results282 tests 282 ✅ 16s ⏱️ Results for commit 2cd4d47. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/main/java/or/sopt/houme/domain/furniture/service/facade/JjymOptimisticLockFacade.java (2)
40-58:DataIntegrityViolationException대신 도메인 예외 사용 권장코딩 가이드라인에 따르면 서비스/파사드 레이어에서는
DataIntegrityViolationException같은 Spring 예외 대신FurnitureException같은 도메인 예외로 변환해서 던지는 게 좋아요. 기존toggle()메서드도 동일한 패턴인데, 둘 다 리팩토링하면 좋을 것 같아요.♻️ 도메인 예외로 변환하는 예시
} catch (InterruptedException ie) { Thread.currentThread().interrupt(); - throw new DataIntegrityViolationException("찜 토글 처리 중 인터럽트가 발생했습니다", ie); + throw new FurnitureException(ErrorCode.JJYM_TOGGLE_INTERRUPTED); } retryCount++; } - throw new DataIntegrityViolationException("찜 시도가 정해진 횟수를 초과하였습니다"); + throw new FurnitureException(ErrorCode.JJYM_TOGGLE_MAX_RETRIES_EXCEEDED);As per coding guidelines: "Never throw
IllegalArgumentExceptiondirectly from services or entities. Input/domain validation failures must be converted to domain exceptions."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/or/sopt/houme/domain/furniture/service/facade/JjymOptimisticLockFacade.java` around lines 40 - 58, Replace thrown Spring exceptions in toggleRawProduct with the domain FurnitureException: catch blocks should still handle OptimisticLockException and DataIntegrityViolationException from jjymService.rawProductJjymToggle, but when InterruptedException occurs inside Thread.sleep wrap and rethrow a new FurnitureException (include the InterruptedException as cause and use a clear message like "찜 토글 처리 중 인터럽트가 발생했습니다"); likewise, when retries exceed MAX_RETRIES replace the final throw of DataIntegrityViolationException with a FurnitureException describing that retry attempts were exhausted. Mirror the same pattern used by the existing toggle() method for consistent domain-exception handling.
19-58: 중복된 retry 로직 추출 고려
toggle()과toggleRawProduct()메서드의 retry 로직이 거의 동일해요. 나중에 추가 기능이 생기면 공통 유틸리티 메서드로 추출하는 것도 좋을 것 같아요.♻️ 공통 retry 유틸리티 예시
private <T> T executeWithRetry(Supplier<T> action) { int retryCount = 0; while (retryCount < MAX_RETRIES) { try { return action.get(); } catch (OptimisticLockException | DataIntegrityViolationException e) { long backoffTime = (long) Math.pow(2, retryCount) * RETRY_DELAY_MS; try { Thread.sleep(backoffTime); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); throw new FurnitureException(ErrorCode.JJYM_TOGGLE_INTERRUPTED); } retryCount++; } } throw new FurnitureException(ErrorCode.JJYM_TOGGLE_MAX_RETRIES_EXCEEDED); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/or/sopt/houme/domain/furniture/service/facade/JjymOptimisticLockFacade.java` around lines 19 - 58, Both toggle(User, Long) and toggleRawProduct(User, Long) contain duplicated retry/backoff logic; extract that into a reusable generic helper (e.g., executeWithRetry or executeWithRetrySupplier) and call it from both methods. Implement a private generic method that accepts a Supplier<Boolean> (or a ThrowingSupplier) and encapsulates the retry loop, backoff calculation, Thread.sleep handling (preserving interrupt behavior by Thread.currentThread().interrupt()), and consistent exception translation (use the same exception type/messages as currently thrown in toggle/toggleRawProduct). Replace the bodies of toggle and toggleRawProduct to simply invoke this helper with jjymService.jjymToggle(user.getId(), recommendFurnitureId) and jjymService.rawProductJjymToggle(user.getId(), rawProductId) respectively so behavior remains identical but logic is centralized.src/test/java/or/sopt/houme/domain/furniture/service/JjymServiceImplTest.java (2)
67-72: 예외 케이스 테스트도 고려해보면 좋겠어요 (optional)현재 happy path 위주로 테스트가 작성되어 있어요. 시간 여유 있을 때 아래 케이스들 추가하면 더 견고해져요:
- 존재하지 않는
userId로 호출 시 예외 처리- 존재하지 않는
rawProductId로 호출 시 예외 처리- 찜 목록이 비어있는 경우
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/or/sopt/houme/domain/furniture/service/JjymServiceImplTest.java` around lines 67 - 72, The test currently covers only the happy path; add negative tests in JjymServiceImplTest that mock userRepository.findById, curationRawProductRepository.findById, and jjymRepository.findByUserIdAndRecommendFurnitureId to return Optional.empty() to assert the service method under test (e.g., createJjym / the method being exercised in this test) throws the expected exceptions for: missing userId, missing rawProductId, and an empty jjym list scenario; use given(...).willReturn(Optional.empty()) for each repository and assertThrows (or the project's equivalent) to verify the correct exception type/message.
46-79: 찜 해제(toggle-off) 케이스 테스트 추가하면 좋겠어요현재 테스트는 찜이 없을 때 새로 생성하는 케이스만 커버하고 있어요. Toggle 기능 특성상 이미 찜이 존재할 때 삭제하는 시나리오도 테스트해두면 안전해요.
💡 추가 테스트 케이스 예시
`@Test` `@DisplayName`("raw product 기준 찜 토글 시 이미 찜이 존재하면 삭제한다") void rawProductJjymToggle_deletesExistingJjym() { User user = User.builder().id(1L).build(); CurationRawProduct rawProduct = CurationRawProduct.builder() .id(10L) .productId(1000L) // ... 필요한 필드들 .build(); RecommendFurniture recommendFurniture = RecommendFurniture.builder() .id(20L) .furnitureProductId(1000L) .source(CurationSource.RAW) .build(); Jjym existingJjym = Jjym.builder() .id(30L) .user(user) .recommendFurniture(recommendFurniture) .build(); given(userRepository.findById(1L)).willReturn(Optional.of(user)); given(curationRawProductRepository.findById(10L)).willReturn(Optional.of(rawProduct)); given(recommendFurnitureRepository.findBySourceAndFurnitureProductId(CurationSource.RAW, 1000L)) .willReturn(Optional.of(recommendFurniture)); given(jjymRepository.findByUserIdAndRecommendFurnitureId(1L, 20L)) .willReturn(Optional.of(existingJjym)); boolean result = jjymService.rawProductJjymToggle(1L, 10L); assertThat(result).isFalse(); then(jjymRepository).should().delete(existingJjym); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/or/sopt/houme/domain/furniture/service/JjymServiceImplTest.java` around lines 46 - 79, Add a complementary "toggle-off" unit test in JjymServiceImplTest that exercises rawProductJjymToggle when a Jjym already exists: set up User, CurationRawProduct, RecommendFurniture and an existing Jjym, mock userRepository.findById(...), curationRawProductRepository.findById(...), recommendFurnitureRepository.findBySourceAndFurnitureProductId(...) to return the recommendFurniture, and jjymRepository.findByUserIdAndRecommendFurnitureId(...) to return Optional.of(existingJjym); call jjymService.rawProductJjymToggle(...) and assert the result is false and that jjymRepository.delete(existingJjym) was called (and no new recommendFurniture or jjym save occurred).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/or/sopt/houme/domain/furniture/service/facade/JjymOptimisticLockFacade.java`:
- Around line 40-58: Replace thrown Spring exceptions in toggleRawProduct with
the domain FurnitureException: catch blocks should still handle
OptimisticLockException and DataIntegrityViolationException from
jjymService.rawProductJjymToggle, but when InterruptedException occurs inside
Thread.sleep wrap and rethrow a new FurnitureException (include the
InterruptedException as cause and use a clear message like "찜 토글 처리 중 인터럽트가
발생했습니다"); likewise, when retries exceed MAX_RETRIES replace the final throw of
DataIntegrityViolationException with a FurnitureException describing that retry
attempts were exhausted. Mirror the same pattern used by the existing toggle()
method for consistent domain-exception handling.
- Around line 19-58: Both toggle(User, Long) and toggleRawProduct(User, Long)
contain duplicated retry/backoff logic; extract that into a reusable generic
helper (e.g., executeWithRetry or executeWithRetrySupplier) and call it from
both methods. Implement a private generic method that accepts a
Supplier<Boolean> (or a ThrowingSupplier) and encapsulates the retry loop,
backoff calculation, Thread.sleep handling (preserving interrupt behavior by
Thread.currentThread().interrupt()), and consistent exception translation (use
the same exception type/messages as currently thrown in
toggle/toggleRawProduct). Replace the bodies of toggle and toggleRawProduct to
simply invoke this helper with jjymService.jjymToggle(user.getId(),
recommendFurnitureId) and jjymService.rawProductJjymToggle(user.getId(),
rawProductId) respectively so behavior remains identical but logic is
centralized.
In
`@src/test/java/or/sopt/houme/domain/furniture/service/JjymServiceImplTest.java`:
- Around line 67-72: The test currently covers only the happy path; add negative
tests in JjymServiceImplTest that mock userRepository.findById,
curationRawProductRepository.findById, and
jjymRepository.findByUserIdAndRecommendFurnitureId to return Optional.empty() to
assert the service method under test (e.g., createJjym / the method being
exercised in this test) throws the expected exceptions for: missing userId,
missing rawProductId, and an empty jjym list scenario; use
given(...).willReturn(Optional.empty()) for each repository and assertThrows (or
the project's equivalent) to verify the correct exception type/message.
- Around line 46-79: Add a complementary "toggle-off" unit test in
JjymServiceImplTest that exercises rawProductJjymToggle when a Jjym already
exists: set up User, CurationRawProduct, RecommendFurniture and an existing
Jjym, mock userRepository.findById(...),
curationRawProductRepository.findById(...),
recommendFurnitureRepository.findBySourceAndFurnitureProductId(...) to return
the recommendFurniture, and
jjymRepository.findByUserIdAndRecommendFurnitureId(...) to return
Optional.of(existingJjym); call jjymService.rawProductJjymToggle(...) and assert
the result is false and that jjymRepository.delete(existingJjym) was called (and
no new recommendFurniture or jjym save occurred).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e0cac057-1df9-4ff2-a5e4-ceaa413ae208
📒 Files selected for processing (9)
src/main/java/or/sopt/houme/domain/furniture/presentation/controller/JjymController.javasrc/main/java/or/sopt/houme/domain/furniture/presentation/dto/response/JjymV2ItemResponse.javasrc/main/java/or/sopt/houme/domain/furniture/presentation/dto/response/JjymV2ListResponse.javasrc/main/java/or/sopt/houme/domain/furniture/repository/CurationRawProductRepository.javasrc/main/java/or/sopt/houme/domain/furniture/service/JjymService.javasrc/main/java/or/sopt/houme/domain/furniture/service/JjymServiceImpl.javasrc/main/java/or/sopt/houme/domain/furniture/service/facade/JjymOptimisticLockFacade.javasrc/test/java/or/sopt/houme/domain/furniture/presentation/controller/JjymV2ControllerTest.javasrc/test/java/or/sopt/houme/domain/furniture/service/JjymServiceImplTest.java
Collaborator
📝 Code Coverage
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/test/java/or/sopt/houme/domain/furniture/presentation/controller/JjymV2ControllerTest.java`:
- Around line 83-118: The test getMyRawProductJjyms_success in
JjymV2ControllerTest currently instantiates JjymController and calls
controller.getMyRawProductJjyms(...) directly which bypasses Spring MVC routing,
`@GetMapping` and `@AuthenticationPrincipal` binding; change the test to perform an
HTTP GET against the real endpoint using mockMvc: call
mockMvc.perform(get("/api/v2/jjyms").with(authentication(authToken()))) (or set
up MockMvc.standaloneSetup(new JjymController(...)) if needed), configure the
jjymService mock as before, and replace direct ResponseEntity assertions with
mockMvc.andExpect assertions for status and response JSON (e.g.,
status().isOk(), jsonPath(...) checks for items[0].rawProductId,
items[0].isJjym, brandName, jjymCount) so the HTTP layer, routing and security
binding are exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 393f94a1-585d-41cd-b790-d0817db305a1
📒 Files selected for processing (1)
src/test/java/or/sopt/houme/domain/furniture/presentation/controller/JjymV2ControllerTest.java
Comment on lines
+83
to
+118
| @Test | ||
| @DisplayName("GET /api/v2/jjyms 요청 시 원천 상품 찜 목록을 반환한다") | ||
| void getMyRawProductJjyms_success() { | ||
| given(jjymService.getMyRawProductJjyms(1L)).willReturn( | ||
| JjymV2ListResponse.of(List.of( | ||
| JjymV2ItemResponse.of( | ||
| 10L, | ||
| true, | ||
| "https://image", | ||
| "https://site", | ||
| List.of("화이트"), | ||
| "브랜드A", | ||
| "소파", | ||
| 100000L, | ||
| 20, | ||
| 80000L, | ||
| 5L | ||
| ) | ||
| )) | ||
| ); | ||
|
|
||
| JjymController controller = new JjymController(jjymService, jjymOptimisticLockFacade); | ||
|
|
||
| ResponseEntity<or.sopt.houme.global.api.ApiResponse<JjymV2ListResponse>> response = | ||
| controller.getMyRawProductJjyms((CustomUserDetails) authToken().getPrincipal()); | ||
|
|
||
| assertThat(response.getStatusCode().value()).isEqualTo(200); | ||
| assertThat(response.getBody()).isNotNull(); | ||
| assertThat(response.getBody().code()).isEqualTo(200); | ||
| assertThat(response.getBody().data()).isNotNull(); | ||
| assertThat(response.getBody().data().items()).hasSize(1); | ||
| assertThat(response.getBody().data().items().getFirst().rawProductId()).isEqualTo(10L); | ||
| assertThat(response.getBody().data().items().getFirst().isJjym()).isTrue(); | ||
| assertThat(response.getBody().data().items().getFirst().brandName()).isEqualTo("브랜드A"); | ||
| assertThat(response.getBody().data().items().getFirst().jjymCount()).isEqualTo(5L); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
# `@WebMvcTest` 내에서 직접 컨트롤러 생성/호출 패턴이 있는지 확인
rg -nP --type=java '@WebMvcTest|new\s+JjymController\s*\(|mockMvc\.perform\s*\(\s*get\("/api/v2/jjyms"'Repository: TEAM-HOUME/HOUME-SERVER
Length of output: 1053
🏁 Script executed:
#!/bin/bash
# Check the full test class setup for JjymV2ControllerTest.java
head -n 120 src/test/java/or/sopt/houme/domain/furniture/presentation/controller/JjymV2ControllerTest.javaRepository: TEAM-HOUME/HOUME-SERVER
Length of output: 5543
🏁 Script executed:
#!/bin/bash
# Check if mockMvc is declared in the test class
rg -A 5 -B 5 'mockMvc' src/test/java/or/sopt/houme/domain/furniture/presentation/controller/JjymV2ControllerTest.javaRepository: TEAM-HOUME/HOUME-SERVER
Length of output: 841
🏁 Script executed:
#!/bin/bash
# Look at other `@WebMvcTest` examples to see the correct pattern
grep -l '@WebMvcTest' src/test/java/or/sopt/houme/domain/*/presentation/controller/*Test.java | head -2 | xargs -I {} sh -c 'echo "=== {} ===" && head -n 120 {}'Repository: TEAM-HOUME/HOUME-SERVER
Length of output: 8587
mockMvc.perform()으로 실제 엔드포인트 테스트하기
Line 104에서 new JjymController(...)로 직접 호출하면 @GetMapping 라우팅, @AuthenticationPrincipal 바인딩, JSON 직렬화를 검증하지 못해요. 같은 파일의 POST 테스트(line 76)는 mockMvc.perform(post(...))으로 제대로 하는데, GET도 일관성 있게 맞춰주세요.
mockMvc.perform(get("/api/v2/jjyms").with(authentication(authToken()))).andExpect(...) 패턴으로 바꾸면 HTTP 레이어도 함께 검증되니까 라우팅 이슈나 매핑 문제를 놓치지 않을 수 있어요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/or/sopt/houme/domain/furniture/presentation/controller/JjymV2ControllerTest.java`
around lines 83 - 118, The test getMyRawProductJjyms_success in
JjymV2ControllerTest currently instantiates JjymController and calls
controller.getMyRawProductJjyms(...) directly which bypasses Spring MVC routing,
`@GetMapping` and `@AuthenticationPrincipal` binding; change the test to perform an
HTTP GET against the real endpoint using mockMvc: call
mockMvc.perform(get("/api/v2/jjyms").with(authentication(authToken()))) (or set
up MockMvc.standaloneSetup(new JjymController(...)) if needed), configure the
jjymService mock as before, and replace direct ResponseEntity assertions with
mockMvc.andExpect assertions for status and response JSON (e.g.,
status().isOk(), jsonPath(...) checks for items[0].rawProductId,
items[0].isJjym, brandName, jjymCount) so the HTTP layer, routing and security
binding are exercised.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📣 Related Issue
📝 Summary
찜 조회 및 등록 v2 API 구현
🙏 Question & PR point
대부분의 로직은 동일합니다. 참조 대상이 RecommnedFurniture에서 Curation_raw_furniture로 변경되었습니다.
또한 낙관적 락 로직에 대해서 재검토를 진행해볼까 합니다.
이게 현재의 낙관적 락 로직 및 fallback 로직인데, 비관적 락과 기능을 좀 더 비교해보는 시간을 가져봐야 할 것 같습니다.
또한 찜한 가구가 많을때의 조회 로직 최적화도 고민을 해보고 성능 최적화를 해보겠습니다.
📬 Postman
찜 가구 토글 API
찜 가구 조회 API
Summary by CodeRabbit