Skip to content

[FEAT] 계약서 : 대기 / 정보조회 / 금액조정#84

Merged
MeongW merged 7 commits intodevelopfrom
feat/contract_fix
Aug 13, 2025
Merged

[FEAT] 계약서 : 대기 / 정보조회 / 금액조정#84
MeongW merged 7 commits intodevelopfrom
feat/contract_fix

Conversation

@minnieming
Copy link
Contributor

@minnieming minnieming commented Aug 13, 2025

🚀 관련 이슈

🔑 주요 변경사항

✔️ 체크 리스트

  • Merge 하려는 브랜치가 올바른가? (main branch에 실수로 PR 생성 금지)
  • Merge 하려는 PR 및 Commit들을 로컬에서 실행했을 때 에러가 발생하지 않았는가?
  • 라벨을 등록했는가?
  • 리뷰어를 지정했는가?

📢 To Reviewers

📸 스크린샷 or 실행영상

↗️ 개선 사항

Summary by CodeRabbit

  • 신기능
    • 계약 조회 시 인사와 단계별 안내 메시지 및 다음 단계 알림이 추가되어 흐름이 명확해졌습니다.
    • 금액 협의 진입부 메시지와 협상 유도 메시지가 도입되었습니다.
    • 일부 단계가 자동/조건부로 전환되어 진행이 매끄러워졌습니다.
  • 문서화
    • API 설명 라벨을 단계 중심으로 명확화했습니다.
  • 리팩터링
    • 초기 대기용 엔드포인트가 제거되어 단계 중심 흐름으로 단순화되었습니다.

@minnieming minnieming requested a review from MeongW August 13, 2025 08:18
@minnieming minnieming self-assigned this Aug 13, 2025
@minnieming minnieming added the 🐞 bug 버그 리포트 label Aug 13, 2025
@minnieming minnieming linked an issue Aug 13, 2025 that may be closed by this pull request
4 tasks
@coderabbitai
Copy link

coderabbitai bot commented Aug 13, 2025

Walkthrough

standByContract 엔드포인트 및 서비스 흐름 삭제, getContractNext/nextSteps(Redis 기반) 등 단계 진행 로직 추가, 사용자 검증 확장, ContractMapper에 getOwnerId/getBuyerId 추가 및 Mapper SQL 수정, PreContract 관련 타입명이 TenantPreContract로 변경되었습니다.

Changes

Cohort / File(s) Summary
Contract Controller
src/main/java/.../contract/controller/ContractController.java, src/main/java/.../contract/controller/ContractControllerImpl.java
standByContract 엔드포인트 삭제; 다수 @ApiOperation 설명문(라벨링) 변경. 기타 엔드포인트 시그니처는 유지.
Contract Service
src/main/java/.../contract/service/ContractService.java, src/main/java/.../contract/service/ContractServiceImpl.java
인터페이스에서 standByContract 제거. Impl에서 기존 stand-by 흐름 삭제, getContractNext·nextSteps(Redis 상태 관리) 추가, nextStep 리팩토링, 사용자 검증 확장, 메시징 지연 삽입, 보증금 관련 단계 전파 변경.
Contract Mapper & SQL
src/main/java/.../contract/mapper/ContractMapper.java, src/main/resources/.../contract/mapper/ContractMapper.xml
getOwnerId, getBuyerId 메서드·쿼리 추가. getContract 매핑에서 본인인증 테이블 조인 및 소유자/임차인 이름 소스 변경(IV 테이블 사용), 기존 user 테이블 조인 제거.
Pre-contract 타입명 변경
src/main/java/.../precontract/service/TenantPreContractService.java, src/main/java/.../precontract/service/TenantPreContractServiceImpl.java, src/main/java/.../precontract/controller/TenantPreContractControllerImpl.java
PreContractServiceTenantPreContractService (인터페이스·구현·컨트롤러 의존성명 변경). 동작 로직 자체 변경 없음.
Owner Pre-contract status update
src/main/java/.../precontract/service/OwnerPreContractServiceImpl.java
saveMongoDB 처리 후 contractChatMapper.updateStatus(contractChatId, ContractChat.ContractStatus.STEP0) 호출 추가.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant Controller as ContractController
  participant Service as ContractServiceImpl
  participant Redis as Redis(steps)
  participant Mapper as ContractMapper
  participant Msg as AiMessage

  User->>Controller: GET /api/contract/{id}
  Controller->>Service: getContract(id, userId)
  Service->>Service: validateUserId(id, userId)
  Service->>Mapper: getContract(id)
  Mapper-->>Service: ContractDTO
  Service-->>Controller: ContractDTO
  Controller-->>User: 200 OK

  User->>Controller: POST /api/contract/{id}/next
  Controller->>Service: nextStep(id, userId, dto)
  Service->>Service: validateUserId
  Service->>Redis: load/update step state (nextSteps)
  alt both confirmed
    Service->>Msg: 안내 메시지 (딜레이 후 다음 단계)
  end
  Service-->>Controller: boolean (완료 여부)
  Controller-->>User: 200 OK
Loading
sequenceDiagram
  actor User
  participant Controller as ContractController
  participant Service as ContractServiceImpl
  participant Mapper as ContractMapper
  participant Msg as AiMessage

  User->>Controller: GET /api/contract/{id}/deposit
  Controller->>Service: getDepositPrice(id, userId)
  Service->>Service: validateUserId
  Service->>Mapper: getOwnerId/getBuyerId
  Service->>Msg: 단계 안내 + 협상 유도 메시지 (지연)
  Service-->>Controller: Deposit DTO/정보
  Controller-->>User: 200 OK

  User->>Controller: PUT /api/contract/{id}/deposit
  Controller->>Service: updateDepositPrice(...)
  Service->>Service: persist + 상태 STEP2 업데이트
  Service->>Msg: 다음 단계 안내 메시지
  Service-->>Controller: 200 OK
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~35 minutes

Assessment against linked issues

Objective Addressed Explanation
계약 전 채팅 로직 구현, 계약 전 채팅 api 구현 (#1) 계약 단계별 메시징/엔드포인트 일부 존재하나 WebSocket/STOMP 기반 채팅 구현 여부 불명확.
SSE 활용한 채팅 알림 구현 (#1) SSE 관련 엔드포인트나 Emitter 구현 코드 없음.
채팅 리스트 API 구현 (#1) 채팅방 리스트 조회 관련 컨트롤러/서비스/쿼리 없음.

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
PreContractService → TenantPreContractService 네이밍 변경 (src/main/java/.../precontract/service/TenantPreContractService.java) 채팅 모듈 요구사항(실시간 통신, SSE 등)과 직접 연관되지 않는 인터페이스/네이밍 변경입니다.
ContractMapper에 getOwnerId/getBuyerId 추가 (src/main/java/.../contract/mapper/ContractMapper.java, src/main/resources/.../contract/mapper/ContractMapper.xml) 채팅 기능의 핵심 요구사항(실시간 전송, 리스트, SSE)과 직접적으로 연결되지 않는 데이터 조회 확장입니다.
OwnerPreContractServiceImpl에서 STEP0 상태 업데이트 추가 (src/main/java/.../precontract/service/OwnerPreContractServiceImpl.java) 계약 채팅 상태 관리 확장은 채팅 인프라 구현 목표와 별개로 보입니다.

Possibly related PRs

Suggested labels

✨ feature

Suggested reviewers

  • MeongW
  • Whatdoyumin

Poem

당근 한 움큼 들고 와 인사할게요 🥕
스탠바이는 접고, 다음 단계로 깡충깡충
레디스에 발자국 찍고 이름 부르면
보증금 얘긴 살짝, 다음은 특약 춤
토끼가 전하는 작은 배포의 축하 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/contract_fix

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (10)
src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java (6)

427-447: 계약 상태 전이(SET STEP0) 타이밍/원자성, NPE 가능성, 링크 메시지 발신자/수신자 뒤바뀜 문제

  • contractChat/chatRoom null 검증 없이 필드를 접근하고 있어 NPE 위험이 큽니다.
  • 메시지 발송(채팅/AI) → Mongo 저장 → 상태 전이 순으로 진행되어, 중간 실패 시 메시지는 발송됐지만 데이터 저장/상태전이는 실패한 “부분 성공” 상태가 됩니다. 특히 Mongo(R2DBC가 아님)와 RDBMS가 서로 다른 트랜잭션 경계를 가져 일관성 이슈가 큽니다.
  • 링크 메시지의 senderId/receiverId가 현재 구현 의도(“임대인 입장”)와 반대로 설정되어 있습니다. 계약 링크는 임대인이 발신하고 세입자가 수신하는 것이 자연스럽습니다.
  • 상태 전이 실패 시 반환값(업데이트 카운트) 검증이 없어 조용히 실패할 수 있습니다.
  • 불필요해 보이는 getContractChatStatus 호출(결과 미사용)이 있습니다.

아래와 같이 순서를 “저장 → 상태전이 → 메시지 발송”으로 변경하고, null 체크 및 업데이트 결과 검증을 추가하는 것을 권장합니다. 또한 발신자/수신자를 의도에 맞게 교정합니다.

   ContractChat contractChat = contractChatMapper.findByContractChatId(contractChatId);
   ChatRoom chatRoom =
           chatRoomMapper.findByUserAndHome(
                   contractChat.getOwnerId(),
                   contractChat.getBuyerId(),
                   contractChat.getHomeId());
-  contractChatService.getContractChatStatus(contractChat.getStatus());
-  ChatMessageRequestDto linkMessages =
-          ChatMessageRequestDto.builder()
-                  .chatRoomId(chatRoom.getChatRoomId())
-                  .senderId(contractChat.getBuyerId())
-                  .receiverId(contractChat.getOwnerId())
-                  .content("계약 채팅방 URL")
-                  .type("URLLINK")
-                  .build();
-  chatService.handleChatMessage(linkMessages);
-  contractChatService.AiMessage(
-          contractChatId, "\uD83D\uDC4B 임대인께서 입장하셨습니다!  \n" + "지금부터 계약을 진행하겠습니다.");
-  contractService.saveContractMongo(contractChatId, userId);
-  contractChatMapper.updateStatus(contractChatId, ContractChat.ContractStatus.STEP0);
+  if (contractChat == null) {
+      throw new BusinessException(OwnerPreContractErrorCode.OWNER_SELECT);
+  }
+  if (chatRoom == null) {
+      throw new BusinessException(OwnerPreContractErrorCode.OWNER_SELECT);
+  }
+
+  // 1) Mongo 저장 (외부 저장소이므로 예외 전파)
+  contractService.saveContractMongo(contractChatId, userId);
+
+  // 2) 상태 전이 (업데이트 카운트 검증)
+  int updated = contractChatMapper.updateStatus(contractChatId, ContractChat.ContractStatus.STEP0);
+  if (updated != 1) {
+      throw new BusinessException(OwnerPreContractErrorCode.OWNER_UPDATE, "계약 채팅 상태 전이에 실패했습니다.");
+  }
+
+  // 3) 메시지 발송 (사이드이펙트는 마지막에)
+  ChatMessageRequestDto linkMessage =
+          ChatMessageRequestDto.builder()
+                  .chatRoomId(chatRoom.getChatRoomId())
+                  .senderId(contractChat.getOwnerId())
+                  .receiverId(contractChat.getBuyerId())
+                  .content("계약 채팅방 URL")
+                  .type("URLLINK")
+                  .build();
+  chatService.handleChatMessage(linkMessage);
+  contractChatService.AiMessage(
+          contractChatId, "\uD83D\uDC4B 임대인께서 입장하셨습니다!  \n" + "지금부터 계약을 진행하겠습니다.");
   return null;

운영 측면에서, 메시지 발송과 DB 변경의 최종적 일관성을 보장하려면 Outbox 패턴/이벤트 기반 처리를 적극 고려하세요. 현재와 같은 직접 호출은 일시적 오류에 취약합니다.


505-511: PII/민감정보 로그 유출 위험: 전체 Document 직렬 출력 금지

OwnerMongoDocument 전체를 info 레벨로 직렬화해 로그에 남기면 계좌번호 등 민감정보가 그대로 노출될 수 있습니다. 메타 정보만 남기거나 debug 레벨로 축소하세요.

-    OwnerMongoDocument document = OwnerMongoDocument.from(dto);
-    log.info("변환된 document: {}", document);
+    OwnerMongoDocument document = OwnerMongoDocument.from(dto);
+    // 민감정보 보호: 메타만 로그로 남김 (필요 시 debug로 강등)
+    log.debug("Mongo 변환 메타 - contractChatId: {}, userId: {}",
+            dto.getContractChatId(), dto.getUserId());

577-581: AI 요청 페이로드 전체 로그 금지(민감정보 포함 가능) — 메타만 로깅

AI 요청 전체 JSON을 info로 남기면 이름/계좌/주민번호 일부 등 민감정보가 유출될 수 있습니다. 계약/임대차 타입 등 최소 메타만 로그로 남기세요.

-    log.info("AI 요청 데이터: {}", objectMapper.writeValueAsString(aiRequest));
+    Long chatId = aiRequest.getOwnerData() != null ? aiRequest.getOwnerData().getContractChatId() : null;
+    String rentType = (aiRequest.getOwnerData() != null && aiRequest.getOwnerData().getRentType() != null)
+            ? aiRequest.getOwnerData().getRentType() : null;
+    int specialTermsCount =
+            (aiRequest.getOcrData() != null && aiRequest.getOcrData().getSpecialTerms() != null)
+                    ? aiRequest.getOcrData().getSpecialTerms().size() : 0;
+    log.info("AI 요청 메타 - contractChatId: {}, rentType: {}, specialTermsCount: {}",
+            chatId, rentType, specialTermsCount);

288-299: Enum 파싱 안전성(2): 동일 이슈 재발 지점

여기도 동일하게 valueOf 예외 전환이 필요합니다. 위와 같은 try-catch로 래핑해 주세요.


53-54: 서비스 인터페이스 명명 규칙 수정 필요: OwnerPreContractServiceInterface.java로 리네임
가이드라인(*ServiceInterface.java / *ServiceImpl.java)에 따라 현재 OwnerPreContractService.java 인터페이스 파일명을 다음과 같이 변경해야 합니다.

• 파일명 변경

  • src/main/java/org/scoula/domain/precontract/service/OwnerPreContractService.java
    src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceInterface.java

• 인터페이스 선언부 수정

- public interface OwnerPreContractService {
+ public interface OwnerPreContractServiceInterface {

• 구현 클래스 수정

- public class OwnerPreContractServiceImpl implements OwnerPreContractService {
+ public class OwnerPreContractServiceImpl implements OwnerPreContractServiceInterface {

• 프로젝트 전반의 참조(import, 의존성 주입 등)도 함께 업데이트 바랍니다.


250-265: Enum 파싱 안전성 강화: RentType.valueOf 예외 처리 추가

RentType.valueOf(rentTypeStr)는 DB에서 조회된 문자열이 enum 상수와 불일치할 경우 IllegalArgumentException을 던져 500 오류로 이어집니다. 아래와 같이 try-catch로 감싸고, 적절한 BusinessException으로 변환해 주세요.

  • 적용 위치:
    src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java
    250–265행
-    RentType rentType = RentType.valueOf(rentTypeStr);
+    RentType rentType;
+    try {
+        rentType = RentType.valueOf(rentTypeStr);
+    } catch (IllegalArgumentException ex) {
+        throw new BusinessException(OwnerPreContractErrorCode.RENT_TYPE_MISSING, ex);
+    }
src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java (2)

48-64: 사용자 검증 로직 중복 개선 필요

모든 메서드에서 동일한 사용자 검증 로직이 반복되고 있습니다. 공통 메서드로 추출하여 중복을 제거하고 유지보수성을 향상시키는 것이 좋겠습니다.

공통 검증 메서드를 추가하고 활용하는 것을 제안합니다:

+    private void validateUser(Long contractChatId, Long userId) {
+        Long buyerId = tenantMapper
+                .selectContractBuyerId(contractChatId)
+                .orElseThrow(() -> new BusinessException(PreContractErrorCode.TENANT_USER));
+
+        if (!userId.equals(buyerId)) {
+            throw new BusinessException(PreContractErrorCode.TENANT_USER);
+        }
+    }
+
     @Override
     public Boolean getCheckRisk(Long contractChatId, Long userId) {
-        // 0. UserId 검증하기
-        Long buyerId =
-                tenantMapper
-                        .selectContractBuyerId(contractChatId)
-                        .orElseThrow(() -> new BusinessException(PreContractErrorCode.TENANT_USER));
-
-        if (!userId.equals(buyerId)) {
-            throw new BusinessException(PreContractErrorCode.TENANT_USER);
-        }
+        validateUser(contractChatId, userId);
         
         // 1. risk_check에 riskId가 있는지 확인하기
         Optional<Long> riskId = tenantMapper.selectRiskId(contractChatId, userId);
         
         // 2. iF : 있다면 -> true 값 보내기 / 없다면 -> false 값 보내기
         return riskId.isPresent(); // 사기 위험도 값 받기 (프론트에서 다른 api 호출)
     }

233-245: 검증 로직 순서 개선 필요

주차장 및 반려동물 관련 검증 로직이 데이터베이스 업데이트 후에 수행되고 있습니다. 검증을 먼저 수행한 후 업데이트하는 것이 더 안전합니다.

         TenantPreContractCheckVO vo = TenantPreContractCheckVO.toStep2VO(step2DTO);
+        
+        // 반려동물 관련 검증
+        if (Boolean.TRUE.equals(vo.getHasPet())) {
+            if (step2DTO.getHasPet() == null
+                    || step2DTO.getPetInfo() == null
+                    || step2DTO.getPetCount() < 1) {
+                throw new BusinessException(PreContractErrorCode.TENANT_NULL);
+            }
+        }
+        
+        // 주차장 관련 검증
+        if (Boolean.TRUE.equals(vo.getHasParking())) {
+            if (step2DTO.getHasParking() == null || step2DTO.getParkingCount() < 1) {
+                throw new BusinessException(PreContractErrorCode.TENANT_NULL);
+            }
+        }

         int result = tenantMapper.updateStep2(vo, userId, contractChatId);
         if (result != 1) throw new BusinessException(PreContractErrorCode.TENANT_UPDATE);

-        if (Boolean.TRUE.equals(vo.getHasParking())) {
-            if (step2DTO.getHasParking() == null || step2DTO.getParkingCount() < 1) {
-                throw new BusinessException(PreContractErrorCode.TENANT_NULL);
-            }
-        }
-        // 반려동물 관련 추가 처리
-        if (Boolean.TRUE.equals(vo.getHasPet())) {
-            if (step2DTO.getHasPet() == null
-                    || step2DTO.getPetInfo() == null
-                    || step2DTO.getPetCount() < 1) {
-                throw new BusinessException(PreContractErrorCode.TENANT_NULL);
-            }
-        }
src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xml (2)

64-73: 하드코딩된 contractChatId 값 수정 필요

insertSignatureInit 쿼리의 WHERE 절에 하드코딩된 값 4가 있습니다. 이는 파라미터로 전달된 #{contractChatId}를 사용해야 합니다.

-        WHERE cc.contract_chat_id = 4;
+        WHERE cc.contract_chat_id = #{contractChatId};

116-120: 잘못된 테이블 별칭 참조

119번 라인의 JOIN 절에서 fc 별칭이 정의되지 않은 상태로 사용되고 있습니다. 이로 인해 SQL 실행 시 오류가 발생할 수 있습니다.

-            LEFT JOIN electronic_signature es ON cc.home_id = fc.home_id AND cc.owner_id = fc.owner_id
+            LEFT JOIN electronic_signature es ON es.contract_id = (
+                SELECT fc.contract_id 
+                FROM final_contract fc 
+                WHERE fc.home_id = cc.home_id AND fc.owner_id = cc.owner_id
+            )
🧹 Nitpick comments (14)
src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java (3)

207-215: 복구 범위 upsert의 N+1 쿼리 가능성 — 배치 처리 권장

카테고리 개수만큼 upsert 호출 시 DB round-trip이 커집니다. Mapper 수준에서 bulk upsert(예: INSERT ... ON DUPLICATE KEY UPDATE 다중 VALUES, 또는 임시테이블/배치)로 최소화하는 것을 권장합니다.


585-597: AI 특약 라운드(round) 고정 값(1L) — 누적/버전 관리 고려

라운드를 1L로 고정하면 재요청 시 이전 데이터가 덮어씌워질 수 있습니다. 동일 contractChatId에 대해 최신 round+1을 계산해 저장하거나, 동일 라운드 재생성 시 upsert 기준을 명확히 하세요. 필요 시 인덱스(contractChatId, round) 추가 권장.


433-433: 결과 미사용 호출 제거 또는 의미 부여 필요

contractChatService.getContractChatStatus(contractChat.getStatus());의 반환값을 사용하지 않습니다. 부수효과가 없다면 제거하거나, 검증 목적이라면 실패 시 예외를 던지도록 보장하세요.

src/main/java/org/scoula/domain/precontract/service/TenantPreContractService.java (1)

14-14: 주석의 오타 수정 필요

"사기위헙도"는 "사기위험도"로 수정되어야 합니다.

-       * @return 사기위헙도 조사 여부를 boolean 값으로 보내기
+       * @return 사기위험도 조사 여부를 boolean 값으로 보내기
src/main/java/org/scoula/domain/precontract/controller/TenantPreContractControllerImpl.java (1)

43-45: 주석 처리된 코드 정리 필요

주석 처리된 중복 코드를 제거하는 것이 좋겠습니다.

          return ResponseEntity.ok(
                  ApiResponse.success(
                          service.saveTenantInfo(contractChatId, userDetails.getUserId())));
-          //            return
-          // ResponseEntity.ok(ApiResponse.success(service.saveTenantInfo(contractChatId,
-          // userDetails.getUserId())))
src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java (4)

177-177: 주석의 오타 수정 필요

"받아오 값들을"은 "받아온 값들을"로 수정되어야 합니다.

-          // 3. dto로 받아오 값들을 update로 저장한다.
+          // 3. dto로 받아온 값들을 update로 저장한다.

187-187: 주석의 오타 수정 필요

"애완동뭄"은 "애완동물"로 수정되어야 합니다.

-          // 5. 다음스텝을 위해서 애완동뭄 여부를 response 값으로 보내기!
+          // 5. 다음스텝을 위해서 애완동물 여부를 response 값으로 보내기!

306-306: 주석의 문구 수정 필요

"보증된 값으로"는 "검증된 값으로"가 더 적절한 표현입니다.

-          // 1. 보증된 값으로 조회하기
+          // 1. 검증된 값으로 조회하기

373-375: 메서드명 일관성 개선 필요

AiMessageAiMessageBtn 메서드명이 Java 컨벤션에 맞지 않습니다. camelCase로 수정하는 것이 좋겠습니다.

기존 서비스의 메서드명이 PascalCase로 되어 있다면 해당 서비스에서 메서드명을 수정하는 것을 제안합니다:

  • AiMessageaiMessage
  • AiMessageBtnaiMessageBtn
src/main/java/org/scoula/domain/contract/controller/ContractController.java (1)

18-21: Swagger 어노테이션 명명 일관성 개선 필요

[계약전_임차인] 태그가 다른 엔드포인트들의 [채팅 _ ...] 또는 [계약서 _ ...] 패턴과 일치하지 않습니다. 일관된 명명 규칙을 사용하는 것이 좋습니다.

-      @ApiOperation(value = "[계약전_임차인] 계약서를 몽고DB에 저장", notes = "계약서에 필요한 항목들을 가져와서 몽고 DB에 계약서 만들기")
+      @ApiOperation(value = "[계약서 _ 초기화] 계약서를 몽고DB에 저장", notes = "계약서에 필요한 항목들을 가져와서 몽고 DB에 계약서 만들기")
src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java (4)

135-139: Thread.sleep() 사용에 대한 개선 필요

동기식 Thread.sleep()은 스레드를 블로킹하여 성능 문제를 일으킬 수 있습니다. 비동기 처리나 스케줄링 방식을 고려해보세요.

CompletableFuture나 ScheduledExecutorService를 사용한 비동기 처리를 권장합니다:

// 예시: CompletableFuture 사용
CompletableFuture.delayedExecutor(2, TimeUnit.SECONDS)
    .execute(() -> {
        // 다음 메시지 전송
    });

453-488: nextSteps 메서드의 가독성 개선 필요

현재 메서드가 너무 길고 복잡합니다. Redis 작업과 비즈니스 로직을 분리하여 가독성을 높이는 것이 좋습니다.

Redis 작업을 별도 메서드로 분리하여 재사용성과 가독성을 개선하세요:

private NextStepDTO loadStepState(String redisKey, ObjectMapper objectMapper) throws JsonProcessingException {
    String currentJson = stringRedisTemplate.opsForValue().get(redisKey);
    return (currentJson != null) 
        ? objectMapper.readValue(currentJson, NextStepDTO.class)
        : new NextStepDTO();
}

private void saveStepState(String redisKey, NextStepDTO state, ObjectMapper objectMapper) throws JsonProcessingException {
    String updatedJson = objectMapper.writeValueAsString(state);
    stringRedisTemplate.opsForValue().set(redisKey, updatedJson);
}

164-169: 조건문 단순화 가능

!deposit 조건을 명시적으로 체크할 필요가 없습니다. else 블록으로 충분합니다.

            if (deposit) {
                contractChatService.AiMessage(contractChatId, "다음 단계는 '금액 조율' 단계입니다");
-            } else if (!deposit) {
+            } else {
                contractChatService.AiMessageBtn(contractChatId, """

423-442: 사용자 검증 로직의 중복 제거 가능

Owner와 Buyer ID를 먼저 조회한 후 검증하는 로직이 중복되어 있습니다. 코드를 간소화할 수 있습니다.

      public void validateUserId(Long contractChatId, Long userId) {
          if (userId == null) {
              throw new BusinessException(PreContractErrorCode.TENANT_USER);
          }

          Long ownerContractId = contractMapper.getOwnerId(contractChatId);
          Long buyerContractId = contractMapper.getBuyerId(contractChatId);

-          if (userId.equals(ownerContractId)) {
-              validateIsOwner(contractChatId, userId);
-              return;
-          }
-
-          if (userId.equals(buyerContractId)) {
-              Long buyerId = tenantMapper
-                      .selectContractBuyerId(contractChatId)
-                      .orElseThrow(() -> new BusinessException(PreContractErrorCode.TENANT_USER));
-
-              if (!userId.equals(buyerId)) {
-                  throw new BusinessException(PreContractErrorCode.TENANT_USER);
-              }
-              return;
-          }
-
-          throw new BusinessException(PreContractErrorCode.TENANT_USER);
+          boolean isOwner = userId.equals(ownerContractId);
+          boolean isBuyer = userId.equals(buyerContractId);
+          
+          if (!isOwner && !isBuyer) {
+              throw new BusinessException(PreContractErrorCode.TENANT_USER);
+          }
+          
+          // 추가 검증이 필요한 경우
+          if (isOwner) {
+              validateIsOwner(contractChatId, userId);
+          } else if (isBuyer) {
+              // Buyer 추가 검증 로직
+              Long buyerId = tenantMapper.selectContractBuyerId(contractChatId)
+                      .orElseThrow(() -> new BusinessException(PreContractErrorCode.TENANT_USER));
+              if (!userId.equals(buyerId)) {
+                  throw new BusinessException(PreContractErrorCode.TENANT_USER);
+              }
+          }
      }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d124fd1 and c1a2bf8.

📒 Files selected for processing (10)
  • src/main/java/org/scoula/domain/contract/controller/ContractController.java (1 hunks)
  • src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java (0 hunks)
  • src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java (1 hunks)
  • src/main/java/org/scoula/domain/contract/service/ContractService.java (0 hunks)
  • src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java (7 hunks)
  • src/main/java/org/scoula/domain/precontract/controller/TenantPreContractControllerImpl.java (2 hunks)
  • src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java (1 hunks)
  • src/main/java/org/scoula/domain/precontract/service/TenantPreContractService.java (1 hunks)
  • src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java (1 hunks)
  • src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xml (1 hunks)
💤 Files with no reviewable changes (2)
  • src/main/java/org/scoula/domain/contract/service/ContractService.java
  • src/main/java/org/scoula/domain/contract/controller/ContractControllerImpl.java
🧰 Additional context used
📓 Path-based instructions (5)
src/main/java/org/scoula/domain/*/mapper/*Mapper.java

📄 CodeRabbit Inference Engine (CLAUDE.md)

src/main/java/org/scoula/domain/*/mapper/*Mapper.java: Define MyBatis mapper interfaces under src/main/java/org/scoula/domain/*/mapper/*Mapper.java
Annotate MyBatis mapper interfaces with @Mapper

Files:

  • src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java
src/main/java/org/scoula/**/service/*Service{Interface,Impl}.java

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use interface-implementation pattern for services: *ServiceInterface.java (interface) and *ServiceImpl.java (implementation)

Files:

  • src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java
  • src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java
  • src/main/java/org/scoula/domain/contract/service/ContractServiceImpl.java
src/main/java/org/scoula/**/controller/*Controller{,Impl}.java

📄 CodeRabbit Inference Engine (CLAUDE.md)

Use interface-implementation pattern for controllers: *Controller.java (interface) and *ControllerImpl.java (implementation)

Files:

  • src/main/java/org/scoula/domain/precontract/controller/TenantPreContractControllerImpl.java
  • src/main/java/org/scoula/domain/contract/controller/ContractController.java
src/main/java/org/scoula/**/controller/**/*.java

📄 CodeRabbit Inference Engine (CLAUDE.md)

src/main/java/org/scoula/**/controller/**/*.java: All controller endpoints should return a consistent ApiResponse<T> wrapper
Add Swagger annotations to controller endpoints for API documentation

Files:

  • src/main/java/org/scoula/domain/precontract/controller/TenantPreContractControllerImpl.java
  • src/main/java/org/scoula/domain/contract/controller/ContractController.java
src/main/resources/org/scoula/domain/*/mapper/*Mapper.xml

📄 CodeRabbit Inference Engine (CLAUDE.md)

src/main/resources/org/scoula/domain/*/mapper/*Mapper.xml: Place MyBatis XML mappers at src/main/resources/org/scoula/domain/*/mapper/*Mapper.xml
Name MyBatis XML mappers *Mapper.xml to match their corresponding *Mapper.java interfaces

Files:

  • src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xml
🧠 Learnings (2)
📚 Learning: 2025-08-12T03:59:30.512Z
Learnt from: CR
PR: ITZEEP/backend#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-12T03:59:30.512Z
Learning: Applies to src/main/java/org/scoula/domain/*/mapper/*Mapper.java : Define MyBatis mapper interfaces under `src/main/java/org/scoula/domain/*/mapper/*Mapper.java`

Applied to files:

  • src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xml
📚 Learning: 2025-08-04T00:51:44.587Z
Learnt from: Whatdoyumin
PR: ITZEEP/backend#40
File: src/main/resources/org/scoula/domain/precontract/mapper/OwnerPreContractMapper.xml:188-192
Timestamp: 2025-08-04T00:51:44.587Z
Learning: In the ITZEEP backend codebase, the owner_precontract_check table uses contract_chat_id as both primary key (PK) and foreign key (FK), so selectOwnerPrecheckId correctly returns contract_chat_id for use in upsertRestoreScope operations. This is confirmed by the join pattern "LEFT JOIN owner_restore_scope ors ON opc.contract_chat_id = ors.owner_precheck_id" in the mapper files.

Applied to files:

  • src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xml
🪛 GitHub Actions: CI
src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java

[error] 1-1: Spotless formatting violations detected in ContractMapper.java: Adjust getBuyerId formatting to conform to style rules. Run './gradlew :spotlessApply' to fix.

src/main/java/org/scoula/domain/contract/controller/ContractController.java

[error] 32-40: Spotless format violations detected by 'gradlew spotlessCheck' in ContractController.java. Run './gradlew spotlessApply' to fix formatting.

⏰ 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: CodeQL Security Analysis (java)
🔇 Additional comments (4)
src/main/java/org/scoula/domain/precontract/service/TenantPreContractService.java (1)

5-5: 인터페이스명 변경이 적절함

PreContractService에서 TenantPreContractService로 변경하여 임차인 관련 서비스임을 명확히 하였습니다. 향후 임대인용 서비스와 구분하기에 좋은 명명입니다.

src/main/java/org/scoula/domain/precontract/controller/TenantPreContractControllerImpl.java (2)

4-4: 서비스 의존성 업데이트 완료

PreContractService에서 TenantPreContractService로 import를 정상적으로 업데이트하였습니다.


20-20: 서비스 필드 타입 업데이트 완료

서비스 필드 타입이 TenantPreContractService로 정상적으로 변경되었습니다.

src/main/java/org/scoula/domain/precontract/service/TenantPreContractServiceImpl.java (1)

30-30: 클래스명 변경이 적절함

PreContractServiceImpl에서 TenantPreContractServiceImpl로 변경하여 인터페이스와 일관성을 맞춘 것이 좋습니다.

contractChatService.AiMessage(
contractChatId, "\uD83D\uDC4B 임대인께서 입장하셨습니다! \n" + "지금부터 계약을 진행하겠습니다.");
contractService.saveContractMongo(contractChatId, userId);
contractChatMapper.updateStatus(contractChatId, ContractChat.ContractStatus.STEP0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

STEP0 전이의 유효성(상태 정의/전이 규칙) 확인 필요

새로 추가된 STEP0로의 전이가 유효한지(ContractStatus에 정의되어 있고, 현재 상태에서 STEP0로의 전이가 비즈니스 규칙상 허용되는지) 확인이 필요합니다. 상태머신/전이 검증이 서비스 계층에서 이루어지지 않으면 불법 전이가 발생할 수 있습니다.

다음 스크립트로 상태 Enum과 STEP0 정의 및 사용처를 확인해 주세요.


🏁 Script executed:

#!/bin/bash
# ContractStatus Enum과 STEP0 정의/사용처 점검
rg -n --line-number "enum\s+ContractStatus"
rg -n --line-number "STEP0"
rg -n --line-number "updateStatus\(.*STEP0"

Length of output: 1802


🏁 Script executed:

#!/bin/bash
# ChatServiceImpl 및 ContractChatServiceImpl에서 updateStatus 정의 및 호출 위치 점검
rg -n "updateStatus(" src/main/java/org/scoula/domain/chat/service

Length of output: 145


🏁 Script executed:

#!/bin/bash
# ContractChatServiceImpl에서 STEP0 전이 검증 로직 확인
rg -n -A2 -B2 "case STEP0" src/main/java/org/scoula/domain/chat/service/ContractChatServiceImpl.java

Length of output: 535


🏁 Script executed:

#!/bin/bash
# OwnerPreContractServiceImpl에서 updateStatus 호출 전후 로직 확인
rg -n -C5 "updateStatus" src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java

Length of output: 673


STEP0 전이 검증 로직 누락
OwnerPreContractServiceImpl에서 직접 contractChatMapper.updateStatus(contractChatId, ContractChat.ContractStatus.STEP0)를 호출하며, 이 과정에서 “현재 상태 → STEP0” 전이가 비즈니스 규칙상 허용되는지 검증하는 로직이 전혀 없습니다. 이대로 두면 불법 전이가 발생할 수 있습니다.

다음 사항을 반영해 수정해 주세요.

  • OwnerPreContractServiceImpl (src/main/java/org/scoula/domain/precontract/service/OwnerPreContractServiceImpl.java) 446행: 직접 mapper 호출 대신 서비스 계층(ContractChatServiceImpl.updateStatus)을 사용하거나, 호출 전후에 전이 유효성 검증 로직 추가
  • ContractChatServiceImplupdateStatus 메서드에 정의된 전이 검증을 활용하도록 변경
  • ContractStatus enum(또는 별도 상태 머신/Validator) 내에 “STEP0으로의 전이가 허용되는 이전 상태 목록”이 올바르게 정의됐는지 재검토하고, 부족하다면 추가

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java (1)

12-12: getBuyerId 시그니처 포맷팅 이슈 해결됨

이전 파이프라인이 지적했던 괄호 앞 공백 문제가 사라졌습니다. 현재 서명은 스타일 규칙을 준수합니다.

🧹 Nitpick comments (1)
src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java (1)

10-12: 두 번의 DB 왕복을 한 번으로 병합하는 선택사항

owner_id와 buyer_id를 별도 메서드로 조회하면 동일한 조건으로 DB를 두 번 왕복합니다. 성능·지연 최소화를 위해 두 값을 한 번에 가져오는 메서드를 추가하는 것을 고려해 보세요. 하위 호환을 위해 기존 메서드는 유지하고, 서비스 계층에서 새 메서드를 우선 사용하도록 전환하는 전략을 권장합니다.

예시(인터페이스):

// 추가 제안
Map<String, Long> selectParticipantIds(@Param("contractChatId") Long contractChatId);

예시(XML 매핑):

<select id="selectParticipantIds" resultType="map">
  SELECT
    owner_id AS ownerId,
    buyer_id AS buyerId
  FROM contract_chat
  WHERE contract_chat_id = #{contractChatId}
</select>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1a2bf8 and afe7ff2.

📒 Files selected for processing (2)
  • src/main/java/org/scoula/domain/contract/controller/ContractController.java (1 hunks)
  • src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/org/scoula/domain/contract/controller/ContractController.java
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/org/scoula/domain/*/mapper/*Mapper.java

📄 CodeRabbit Inference Engine (CLAUDE.md)

src/main/java/org/scoula/domain/*/mapper/*Mapper.java: Define MyBatis mapper interfaces under src/main/java/org/scoula/domain/*/mapper/*Mapper.java
Annotate MyBatis mapper interfaces with @Mapper

Files:

  • src/main/java/org/scoula/domain/contract/mapper/ContractMapper.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). (2)
  • GitHub Check: Test Coverage
  • GitHub Check: CodeQL Security Analysis (java)
🔇 Additional comments (2)
src/main/java/org/scoula/domain/contract/mapper/ContractMapper.java (2)

10-10: 신규 메서드 getOwnerId 추가 적절

  • @param 키와 SQL 파라미터명이 일치하고, 결과가 없을 때 null을 표현할 수 있도록 Long(래퍼) 타입을 사용한 점이 적절합니다. 기존 네이밍 컨벤션(getContract 등)과도 일관됩니다.

10-12: 매퍼 매핑 및 null 처리 확인 완료

  • src/main/resources/org/scoula/domain/contract/mapper/ContractMapper.xml<select id="getOwnerId">/<select id="getBuyerId"> 매핑이 존재하며, #{contractChatId} 파라미터명도 인터페이스(@Param("contractChatId"))와 완전히 일치합니다.
  • 서비스 로직(ContractServiceImpl)에서
    Long ownerContractId = contractMapper.getOwnerId(contractChatId);
    Long buyerContractId = contractMapper.getBuyerId(contractChatId);
    if (userId.equals(ownerContractId)) { … }
    와 같이 userId.equals(...)를 사용하고 있어, 매퍼가 null을 반환하더라도 NPE 없이 false로 처리되어 이후 비즈니스 예외 처리 흐름으로 진입합니다.
    → 별도의 Optional 래핑이나 추가 null 체크 없이도 안전합니다.

@MeongW MeongW merged commit 4552dbf into develop Aug 13, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐞 bug 버그 리포트

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feat] 임차인 계약전 프론트엔드 연동 [Feat] 이슈 제목

2 participants