Skip to content

fix: JoinRoom 비밀번호 검증 조건 및 응답 형식 개선#31

Merged
Sean-mn merged 7 commits intorefactor/claudefrom
refactor/auth-workflow
Apr 1, 2026
Merged

fix: JoinRoom 비밀번호 검증 조건 및 응답 형식 개선#31
Sean-mn merged 7 commits intorefactor/claudefrom
refactor/auth-workflow

Conversation

@Sean-mn
Copy link
Copy Markdown
Contributor

@Sean-mn Sean-mn commented Apr 1, 2026

📚작업 내용

  • JoinRoomService 비밀번호 검증 조건을 request.Password != null에서 room.IsPrivate 기준으로 수정 (공개 방에 비밀번호 전달 시 잘못된 검증 실행되던 버그 수정)
  • Auth 컨트롤러 Login, Logout 응답 형식을 CommonApiResponse로 통일
  • Room 컨트롤러 GetRoom, GetAllRoom, JoinRoom 응답 형식을 CommonApiResponse로 통일
  • RoomSummary 레코드 제거 — GetRoomResponse로 대체하여 중복 타입 제거
  • UserRepository.CreateAsync를 Raw SQL에서 EF Core AddAsync로 교체
  • Room 엔터티 최대 플레이어 수를 DefaultMaxPlayers 상수로 추출
  • 비공개 방에 비밀번호 없이 참여 시 예외를 검증하는 테스트 케이스 추가

◀️참고 사항

비밀번호 검증 버그: 기존 로직은 요청에 비밀번호가 포함된 경우에만 검증을 수행해 공개 방에도 비밀번호를 전달하면 검증이 실행되는 문제가 있었습니다. room.IsPrivate 기준으로 변경하여 비공개 방에서만 검증이 동작하도록 수정했습니다.

✅체크리스트

  • 현재 의도하고자 하는 기능이 정상적으로 작동하나요?
  • 변경한 기능이 다른 기능을 깨뜨리지 않나요?

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request standardizes API responses using CommonApiResponse, refactors room DTOs by replacing RoomSummary with GetRoomResponse, and adds unit tests for private room access. Key issues identified include a regression in UserRepository.CreateAsync where the removal of the SQL Upsert logic will lead to primary key violations, and a weakening of password validation in JoinRoomService which should continue to check for whitespace or empty strings.

@Sean-mn Sean-mn merged commit be87e83 into refactor/claude Apr 1, 2026
1 check passed
@Sean-mn Sean-mn deleted the refactor/auth-workflow branch April 1, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant