Repository navigation
feat: 휴지통 템플릿 복구 API (#67) - #69
Conversation
chjpooh2002
left a comment
chjpooh2002
left a comment
There was a problem hiding this comment.
리뷰 남겨두었으니 한 번 확인 부탁드립니다!
| Shortcut original = template.getShortcut(); | ||
| template.restore(templateCommandService.assignShortcut(command.userId(), original)); | ||
|
|
||
| return TemplateShortcutAssignResult.of(templateCommandService.save(template), original); |
There was a problem hiding this comment.
수정 API는 회원을 잠그지 않아서, assignShortcut으로 번호를 고른 뒤 저장 전에 다른 탭에서 같은 단축어로 수정하면 여기서 유니크 위반이 나고 save()가 TEMPLATE409_1로 바꿔 내보낼 것 같아요!
그런데 명세 16번 에러 표에는 409_1이 없고, 복구는 사용자가 단축어를 고르는 게 아니라서 "이미 사용 중인 단축어예요." 문구도 안 맞아요. 갤러리 저장(21번)도 같은 경로라 같은 상황이고요.
거의 안 일어나는 경우라 코드를 바꾸기보다는, 명세 16번·21번 에러 표에 409_1을 "동시 수정과 겹친 경우, 다시 시도하면 됨"으로 추가하고 프론트에 공유하는 정도면 충분할 것 같은데 어떻게 생각하시나요?!
There was a problem hiding this comment.
좋습니다 해당 부분은 명세 16, 21번에 반영하구 프론트에 공유 드리겠습니다 👏
| /* | ||
| * 아래 패키지 범위 메서드 3개는 휴지통 복구(TrashCommandService)가 같이 쓴다. 생성·갤러리 저장과 규칙이 같아야 하기 때문이다. | ||
| * 셋 다 트랜잭션을 따로 열지 않으므로, 호출자가 lockUser를 첫 조회로 부른 트랜잭션 안에서 불러야 한다. | ||
| * save는 엔티티를 받고 돌려주고, 순서를 지키지 않으면 한도를 넘을 수 있어 template 도메인 안에서만 부른다 | ||
| * (패키지 범위라 service 패키지 밖에서는 부를 수 없다). | ||
| */ | ||
|
|
||
| /** 활성 템플릿(휴지통 제외)이 한도 이상이면 TEMPLATE403_1 */ | ||
| void validateTemplateLimit(Long userId) { | ||
| if (templateRepository.countByUserIdAndDeletedAtIsNull(userId) >= UsageLimits.TEMPLATE_LIMIT) { | ||
| throw new BusinessException(TemplateErrorCode.TEMPLATE_LIMIT_REACHED); | ||
| } | ||
| } | ||
|
|
||
| /** 활성 템플릿과 겹치지 않으면 원본, 겹치면 자동 번호를 붙인 단축어. 2~99가 모두 차면 TEMPLATE409_2 */ | ||
| Shortcut assignShortcut(Long userId, Shortcut original) { | ||
| Set<String> usedKeys = templateRepository.findActiveShortcutKeys(userId, original.candidateKeys()); | ||
| return original.findAvailable(usedKeys) | ||
| .orElseThrow(() -> new BusinessException(TemplateErrorCode.SHORTCUT_NUMBER_EXHAUSTED)); | ||
| } |
There was a problem hiding this comment.
(선택, 후속 이슈로 넘겨도 돼요) 한도 검사·자동 번호·저장을 이제 세 곳(생성·갤러리 저장·복구)이 같은 규칙으로 쓰게 됐는데, 지금은 TrashCommandService가 TemplateCommandService의 패키지 범위 메서드를 직접 부르는 구조라 서비스끼리 의존하게 됐어요.
이 세 메서드를 패키지 범위 컴포넌트(예: TemplateSaveSupport)로 따로 빼면, 두 서비스가 같은 도우미에 기대는 형태가 되고 "회원 잠금을 먼저 잡은 트랜잭션 안에서 불러야 한다"는 규칙도 한 클래스 주석에 모을 수 있을 것 같아요.
동작은 지금도 문제없어서 이번 PR에서 꼭 바꿀 필요는 없다고 생각합니다!
There was a problem hiding this comment.
의견 감사합니당 🙂
확인해 보니 이 세 메서드를 쓰는 곳은 생성, 갤러리 저장, 복구 세 곳으로 끝이고 지금은 같은 패키지 안에서만 부르고
있다보니 괜찮을것 같습니다 !
쓰는 곳이 더 생기거나 TemplateCommandService가 커지면 그때 분리하는것도 좋을것 같아요 !
해당 부분은 남은 API 추가 후 리팩토링 지점으로 두겠습니다
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| @Transactional(readOnly = true) |
There was a problem hiding this comment.
(사소) 명령 서비스인데 클래스에 readOnly = true가 붙어 있어서, 나중에 쓰기 메서드를 추가하면서 @Transactional을 빠뜨리면 읽기 전용 트랜잭션에서 쓰기가 실행될 수 있을 것 같아요.
TemplateCommandService처럼 클래스 수준 어노테이션은 빼고 메서드에만 두면 두 명령 서비스 방식도 맞춰질 것 같습니다!
There was a problem hiding this comment.
좋습니다 해당 부분은 CLAUDE.md 파일에 규칙이 있는것 같아서 같이 제거 해서 반영 하겠습니다 !
관련 이슈
작업 내용
명세 16번
POST /api/v1/trash/templates/{templateId}/restore를 구현했습니다. 한도 검사, 자동 번호, 저장 규칙은 생성·갤러리 저장과 같은 코드를 씁니다.커밋별로 보기
Template.restore(Shortcut). 휴지통이 아닌 템플릿에 부르면IllegalStateException(코드 실수)TemplateCommandService의 한도 검사·자동 번호·저장을 패키지 범위로 열고, 자동 번호는assignShortcut으로 뽑음 (동작 변경 없음)TrashCommandService.restore,TrashRestoreCommand,TRASH404_1TrashController에 추가,TRASH200_2,TrashRestoreResponseConcurrentRunner로 뽑아 기존 생성 동시성 테스트와 같이 씀FOR UPDATE로 잠가 읽음 (TemplateRepository.findTrashedByIdAndUserIdForUpdate추가)templateId→COMMON400_1ConcurrentRunner를support에서 template 서비스 테스트 패키지로 옮기고 패키지 범위로 좁힘 (템플릿 동시성 테스트만 씀)TEMPLATE409_1추가TrashCommandService의 클래스 수준@Transactional(readOnly = true)제거 (리뷰 반영,TemplateCommandService와 같은 방식)readOnly, 명령 서비스는 쓰기 메서드에만@Transactional"로 변경처리 순서 (한 트랜잭션)
lockUser). 트랜잭션의 첫 조회FOR UPDATE)TRASH404_1TEMPLATE403_1TEMPLATE409_2deleted_atNULL, 번호가 붙었으면shortcut·shortcut_key도 변경save()로 변환TEMPLATE409_1edited_at·last_used_at·created_at은 삭제 전 값 그대로,updated_at만 감사 기능이 갱신합니다.TRASH404_1이 됩니다.TRASH404_1입니다. 잠금 조회는 스냅샷을 만들지 않아 이어지는 개수 검사에는 영향이 없습니다.명세와 달라진 점
TEMPLATE409_1을 추가했습니다(리뷰 반영, docs 커밋). 잠금을 잡지 않는 수정 요청이 조회와 저장 사이에 같은 단축어로 바꿀 때만 나며, 화면은 입력창 에러가 아니라 "다시 시도" 안내로 처리합니다. 다시 요청하면 자동 번호가 붙어 성공합니다.TEMPLATE409_2는 지금 한도로는 실제 행으로 닿을 수 없습니다. 번호 후보는 최소 91개인데 한도가 50개라 활성 49개에서도 번호가 남습니다. 테스트는 개수 조회만 바꿔 확인했습니다.테스트
TrashRestoreControllerTest(테스트컨테이너 MySQL, 고정 시계)no-store, DB의deleted_atNULL,edited_at·last_used_at·created_at유지,updated_at갱신shortcutChanged: false), 겹치면/견적→/견적2, 대소문자만 다른 경우, 자모 분리형 입력, 10자 자르기TEMPLATE403_1(휴지통에 그대로), 한도와 번호 소진이 겹치면403_1이 먼저TRASH404_1, 토큰 없음COMMON401_1, 숫자가 아닌templateIdCOMMON400_1TrashRestoreConflictTest: 번호 소진TEMPLATE409_2, 저장 시 유니크 위반TEMPLATE409_1(500으로 새지 않고 휴지통에 그대로)TrashRestoreConcurrencyTest(실제 커밋)403_1, 최종 50개403_1, 최종 50개TRASH404_1/견적·/견적2TRASH404_1(고치기 전에는ObjectOptimisticLockingFailureException으로 500)lockUser를 템플릿 조회 뒤로 옮기면 네 건이 모두 실패하는 것을 확인했습니다TrashTemplateRepositoryTest: 복구용 잠금 조회는 본인 휴지통만 찾음 (활성·다른 회원 제외)TrashCommandServiceTest(모키토): 호출 순서(잠금 → 조회 → 한도 → 번호 → 저장), 주요 분기, 실패하면 휴지통에 그대로TemplateTest:restore시각 컬럼 유지, 번호 단축어, 휴지통이 아닐 때IllegalStateException, 단축어 없음IllegalArgumentExceptionTrashErrorCodeTest:TRASH404_1코드와 문구확인 체크리스트
./gradlew build통과global공통 코드 변경 없음리뷰어에게
save()를 복사하지 않고TemplateCommandService의 패키지 범위 메서드로 열었습니다. #50에서 합의한TemplateQueryService.getActiveTemplate과 같은 모양이고, 다른 도메인에서는 부를 수 없습니다. 별도 구성 요소로 뽑는 방법도 있지만, 그러면 기존TemplateCommandServiceTest의 준비 코드를 바꿔야 해서 변경을 줄이는 쪽을 골랐습니다TrashTemplateRow라 영향이 없습니다. 17·18번과 정리 배치는 엔티티를 읽지 않고 지울 예정입니다TemplateRepository는 메서드 추가만 했습니다: 복구용 잠금 조회 하나입니다. #68이 같은 파일을 고치므로 병합할 때 겹칠 수 있지만 둘 다 추가라 정리가 쉽습니다TrashControllerTest는 아직 네이티브 SQL로 복구를 흉내 냅니다. 이번 PR 범위를 넘으므로 그대로 뒀습니다🤖 Generated with Claude Code