Skip to content

Conversation

@blu3fishez
Copy link
Collaborator

@blu3fishez blu3fishez commented Nov 14, 2024

관련 이슈 번호

  • 작성자: 김찬우, 송수민
  • 작성 날짜: 2024.11.14

✅ 체크리스트

  • 코드가 정상적으로 작동하는지 확인했습니다.
  • 주요 변경사항에 대한 설명을 작성했습니다.
  • 코드 스타일 가이드에 따라 코드를 작성했습니다.

🧩 작업 내용

  • Redis 를 활용한 스터디 세션 기능 구현
  • Redis 에 정의된 스터디 세션 정보를 접근하기 위한 레포지토리 계층, 서비스 계층 구현
  • 스터디 세션과 관련된 웹소켓 이벤트 핸들러 구현 (room.gateway.ts)

📝 작업 상세 내역

Redis 를 이용한 세션 관리

우선, 레디스를 어떻게 구성하는지에 대해서 어떤 문제를 겪었는지 말씀드려야할 것 같습니다.

레디스는 간단한 키와 값으로 저장되어있는 데이터들의 모음입니다.

1. 검색에 관한 문제

스터디 세션에 접속 중인 소켓들의 연결정보를 확인할 수 있어야하고,
특정 멤버(소켓)이 어떤 방으로 연결되어있는지 확인해야합니다.

이를 위해서 연결 정보를 결국 서브키를 두 depth 로 나누어 join:${roomId}:${socketId} 로 정의하여 키 자체가 존재하면 연결중임을 알려주도록 표현했습니다.

image

이렇게 할 경우, wildcard 서치를 하면 편리하게 검색이 가능하므로 코드 구현이 쉬워집니다.

보다 구체적인 데이터 구조는 해당 명세서에서 확인 가능합니다.

Room 모듈 구현

저희 PR이 늦어진 이유부터 알려드리려고 합니다. 우선 기본적으로 PR 요청을 올리는 기준, 즉 의미단위죠. 의미 단위로 PR을 올리기 위해서는 적어도 모든 작동이 다 잘 되어야하는 상태로 올려야합니다.

저희 같은 경우는 일단은 잘 돌아가는 만큼 구현을 하려면 모든 계층을 설계해야합니다. 그래서 모든 계층을 설계하느라 늦었습니다.

설계같은 부분은 이곳에서 확인 가능합니다.

반대로 미리 모든 계층에 대한 설계를 해둘 수 있다면 해두는게 좋다고 판단하였고, 이번에 저를 포함한 저와 같이 작업한 @twalla26 (수민)님 또한 다음 스프린트부터는 미리 작성한 본 스터디 세션 모듈을 통해 설계를 하는데 있어서 속도가 더 빠르게 나올 것이라 판단합니다.

모듈 구조

Spring 에서는 각 모듈이 정확하게 나뉘어서 작동하나, nest.js 의 경우 비교적 자유롭게 파일 수정이 가능합니다.

이부분에서 개발자에게 주어진 책임으로 인해 어떻게 설계할지 많이 고민을 하였습니다. 특히, Service 계층과 Repository 계층은 어떻게보면 같은 코드를 구현하는 양상이 되고 있는 것도 확인하실 수 있을 겁니다.

그렇기에 우선 각자의 모듈의 역할에 책임 분리를 아래와 같이 지었습니다.

  • *.gateway.ts , *.controller.ts
    • 서비스로부터 예외 처리를 받아서 어떤 통신정보를 리턴해야하는지 정의
    • 비즈니스 로직이 들어갈 수도 있으나 그 과정에서 통신정보가 포함되어야 한다.
    • 통신 정보와 가까운 클라이언트와 직접 맞닿은 게이트웨이
  • *.service.ts
    • 비즈니스 로직 처리, 데이터 구조, 통신정보 에 관해서는 신경쓰지 않도록 하는 것이 원칙
    • 예외 처리 로직이 해당 부분에 들어가야함
  • *.repository.ts
    • 말 그대로 저장소와 관련된 부분
    • 데이터베이스 접근 로직, 레디스 접근 로직이 주요 부분으로 들어가야한다.
    • 데이터를 가져오는 로직을 제외한 로직은 포함되어선 안된다.

웹소켓에서 들어오는 데이터는 string인가?

일반적으로 모든 소켓 연결에서 string으로 데이터를 받는데, 웹소켓 또한 마찬가지인 줄 알았습니다. 보통 그래서 @MessageBody 라는 nestjs 에서 제공하는 기능이 있는데 이걸 활용하여 문제 해결을 하고자 했습니다.

하지만, 알고보니 Postman 측에서 Text 형식으로 보내는 것을 알 수 있었고, 클라이언트 측에서 보내는 데이터 형식을 정할 수 있다는 것도 깨달았습니다.

그러다보니 이 부분에 관해서는 그냥 any 타입으로 받고 급하게 구현하게 되었는데, 사실은 Nest.js 에서는 Pipe라는 아름다운 기능이 있는데 추후 이것을 validator로써 사용할 예정입니다.

현재로써는 데이터 검증 로직과 예외 처리 로직이 부족한 것은 사실입니다. 하지만 리팩토링 기간이 남아있기도 하고, 일단은 돌아가게 만들고 완성을 위한 구현을 한번 더 하자 라는 원칙을 지키기 위해서 시간이 부족하기도 하고 일단 한번 더 구현하는 식으로 문제를 해결하였습니다.

@WebSocketGateway()
export class ChatGateway {
    @SubscribeMessage('message')
    handleMessage(@MessageBody(new ValidationPipe()) createDto: CreateMessageDto) {
        // DTO를 통한 유효성 검사가 적용됩니다
    }
}

WAS 초기 화 시 redis 초기화 문제

현재, 레디스는 서버 전역적으로 설치되어 있습니다. 다만, 이때 레디스 에 삽입할 인덱스도 초기화가 되는 문제가 있습니다.

다음에 새롭게 생성하는 방의 인덱스가 3인데, WAS 가 초기화 될경우 해당 WAS가 생성하는 새로운 방의 인덱스가 1이 되도록 만드는 문제가 있습니다.

당연히 현재로써는 WAS를 하나만 사용 중이고, 여러 WAS를 사용할 수 있도록 확장설계를 하기 위해선 Redis Pubsub 과 함께 해당 인덱스 정보를 Redis 에서 저장하도록 만들어서 추후에 이를 이슈화하여 해결할 예정입니다.

퇴장 로직 설정 중 생긴 문제

handleLeavRoom 이벤트 핸들러를 작성 시 생긴 문제입니다.

유저가 퇴장 될 때 어떤 상태가 되는지 생각해봐야했습니다.

사실 이런 부분에 관해서는 상태를 두고 코드를 짜는 것이 얼마나 번거로운지 알게되었고, 함수를 순수하게 짜는 것 외에 어떤 점이 코드를 짤 때 편하게 짤 수 있는지 고민을 하게 된 것 같네요.

본 코드를 자세히 살펴보면 각각의 프로시저가 수행되는 순서에 따라 출력되는 값이 천차만별이 됩니다. 그렇기에 상태를 건드리는 작업을 하게 된다면 조심하셔야할 것 같습니다.

@SubscribeMessage(EVENT_NAME.LEAVE_ROOM)
async handleLeaveRoom(client: Socket) {
    const roomId = await this.roomService.getRoomId(client.id);

    const isHost = await this.roomService.checkHost(client.id);
    const leftCount = await this.roomService.leaveRoom(client.id);

    if (!leftCount) {
        await this.roomService.deleteRoom(roomId);
        return;
    }

    if (isHost) {
        const hostConnection = await this.roomService.delegateHost(roomId);
        this.server.to(roomId).emit(EVENT_NAME.MASTER_CHANGED, {
            masterSocketId: hostConnection.socketId,
            masterNickname: hostConnection.nickname,
        });
    }

    this.server
        .to(roomId)
        .emit(EVENT_NAME.USER_EXIT, { socketId: client.id });
}

특히 이부분과 관련해서 피드백 받을 수 있는 좋은 부분은 이런 상태에 영향을 주고 상태에 따라 값이 달라지는 코드 를 작성할 때에는 어떻게 생각하시는지 궁금하기도 해서 데모데이 피드백으로 올려볼까 합니다.

문제 제안 : 만료 기한을 갱신해야하는가?

실제 태스크를 나누다가 생각하지 못한 문제를 또 구현에서 찾게 되었는데, 만료 기한 갱신 여부를 또 기획적으로 생각하게 되었습니다.

이전에 태스크를 나눌 때에는 단순히 스터디 룸 세션은 6시간 이후에 만료된다. 라는 문장 하나로 이해하고 구현에 시작했습니다.

하지만, 실제로 구현을 하다보니 스터디 룸 세션 정보를 수정할 일이 많은데, 그때마다 기존에 처음 설정했던 TTL(Time to Live), 레디스에서 관리하는 유효기한 대로 계속 갱신을 하다보니 의문점이 들게 되었습니다.

음.. 이 과정에서 다들 경계하시겠지만 클린코드 에서 나온 문장이 조금 생각나긴 합니다. 코드를 작성하는 것은 요구사항을 컴퓨터도 이해할 수 있을만큼 상세하게 적는 과정이라고 주장합니다.

물론 이에 관해서 다들 어떻게 생각하시는지는 잘 모르겠지만, 이 과정에서 정말 동의하게 되었는데, 이렇게 직접 코드를 짜보면서 요구사항을 확실히 이해할 수 있는 상황이 되게 많았습니다.

📌 테스트 및 검증 결과

Postman 으로 테스트 하였습니다.

3가지 Use-case 를 테스트 하였고, 보다 더 엄밀한 검증을 위해 현재로써는 단위 테스트까지 필요한 상황입니다.

하지만 현재로써는 한계점이 존재하는데, 단위 테스트를 수행할만큼 코드의 역할분리가 아직은 잘 되어있지 않습니다.

그래서 현재 단위 테스트를 통해 정확한 커버리지는 구하지 못하는 것이 현 실정입니다. 시간이 부족한 것이 원인입니다. 사실 오늘도 시간을 정하고 리팩토링을 그만두고 테스트 코드 작성을 진행했어야했는데, 시간이 없어서 테스트 코드 작성을 미루고 있는 상황입니다.

이를 피드백으로 올려, 다른 팀의 현재 상황을 파악하고, 어떻게 코드를 짜는지 확인해보고 싶긴 합니다.

reaction 테스트 예시

현재 브랜치를 수정하다가 reaction 기능이 누락됨을 확인하였고, 새롭게 기능을 다시 복구하여 추가하였습니다.

이 과정에서 테스트를 아래와 같이 하였습니다.

image image

Postman 자체에서 웹소켓(소켓IO 라이브러리까지 지원) 을 지원하므로 손쉽게 테스팅을 할 수 있었습니다.

💬 다음 작업 또는 논의 사항

  • 프론트엔드 개발환경 설정 - Docker 설치
  • 통합 테스트 환경 구축 방법 논의
  • 단위 테스트 고민

🐥 리뷰 받고 싶은 포인트

  • 비즈니스 로직의 경계선에 대해서 조금 궁금하긴 합니다.
  • 실제로 Repository 의 경우에서도 사실 비즈니스 로직이 조금 들어가 있어서 조금 애매한 상황입니다.
  • 그리고 Gateway 부분도 서비스 로직이 포함되니 이것을 단위 테스트 코드를 넣게 될 경우 웹소켓 연결을 굳이 같이 테스트 해야하는 코드가 있는지 의문인 부분도 있어서 이에 대한 해결책도 궁금하네요.
  • 무엇보다, 프론트엔드에서는 PR을 쪼개서 여러번 올리시던데, 저희 같은 경우는 일단은 잘 돌아가는 만큼 구현을 하려면 모든 계층을 설계해야합니다.
    • 현재 같은 경우도 룸 모듈을 전체 구현을 해야 모든 작업이 확인 가능한 상황입니다.

twalla26 and others added 24 commits November 12, 2024 22:21
- RedisService : 레디스 클라이언트가 작업할 일련의 공통 처리 작업 모음
- MemberConnection : 각 방에 참여하는 유저의 연결 상세 정보
- Room : 방의 상세정보(메타데이터)
- create-room.dto.ts : 방 생성 시 필요한 객체 정보 정의
- 스터디 세션 생성과 관련된 비즈니스 로직
- `Redis` 등의 저장소 정보와 관심분리 목적
- `Redis subkey` 등 서브키들을 구축하는데 있어서 해당 모듈 사용
- `Redis` 자체 클라이언트와 관심 분리, 동시에 비즈니스 로직과 연관된 작업들 명시
- 방 생성에 대한 이벤트 핸들러 추가
- 방 참가에 대한 로직 변경 중..
- primitive 타입인지 object 타입인지는 개발자에게 검사하기로 넘김
- 로그문 삭제
- Postman 측에서 생긴 에러였으므로 로그문 삭제
- 추후 `DTO`와 `Pipe` 이용하여 validation 로직 추가 예정
- members 항목은 사용하지 않으므로 삭제
- 사용하지 않는 주석, 로그문 제거
- 여러 서비스의 작업으로 이루어지도록 수정
- 서비스의 작업이 간단해지도록 수정
- 방을 나가기 / 방장을 변경하기 작업을 나눔
- null 일 경우 false 를 반환하도록 `checkHost` 수정
- getRoomById 를 통해 받아오는 데이터를 단언하여 `Room` 으로 타입화
  - 타입 가드를 사용할 예정
- host를 설정해서 받아오는 로직 수정
  - 이는 추후 서비스로 올릴 예정
@yiseungyun
Copy link
Member

프론트엔드에서는 서로 사용해야하는 공통 컴포넌트나 이런 것들을 사용해야해서 하나 작업이 끝나면 PR을 올리고 있습니다. 아무래도 다른 작업과 함께 PR을 올리게 되면 기다리는 시간이 길어지다보니 이렇게 하고 있어요!

Copy link
Member

@yiseungyun yiseungyun left a comment

Choose a reason for hiding this comment

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

제가 좀 더 시간을 가지고 코드를 읽어보면 좋은데 급하게 읽느라 제대로 된 리뷰를 못 남긴거 같은 느낌이 드네요! ㅜㅜ

정말 수고 많으셨습니다. 👍☃️👍

Comment on lines +64 to +86
async getNewHost(roomId: string) {
const memberKeys = await this.redisService.getKeys(`join:${roomId}:*`);
const members = [];
for (const key of memberKeys) {
const socketId = key.split(":")[2];
const value = await this.redisService.get(
`join:${roomId}:${socketId}`
);
const result = JSON.parse(value);
members.push({
socketId,
joinTime: result.joinTime,
nickname: result.nickname,
});
}

const sortedMembers = members.sort((a, b) => a.joinTime - b.joinTime);
return sortedMembers[0] as {
joinTime: number;
nickname: string;
socketId: string;
};
}
Copy link
Member

Choose a reason for hiding this comment

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

이 코드가 새로운 방장을 위임하는 코드인가요? 해당 코드 내에서 현재 가장 오래 남아있던 사용자를 찾아서 호스트로 지정하는데, 레포지토리에서는 roomId에 접속한 사용자를 반환해주는 로직만 남기고 서비스 계층에서 sortedMembers 코드를 작성해서 호스트를 찾는 로직으로 변경하면 조금 분리되지 않을까하는 생각이 듭니다.

저도 계층에 대해 자세히 아는건 아니지만, 비즈니스 로직을 서비스 계층으로 옮기고 레포지토리에서는 데이터 접근만 하게 하면 좋을거 같습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

헉,, 말씀해주신 덕분에 지금보니 레포지토리에 비지니스 로직이 구현되어있다는 걸 알게되었네요...!! 좋은 피드백 감사합니다. 리팩토링 기간에 아무래도 확실하게 분리해야게어요!!

Comment on lines 70 to 91
@SubscribeMessage(EVENT_NAME.JOIN_ROOM)
async handleJoinRoom(client: Socket, data: any) {
const { roomId, nickname } = data;

if (!(await this.roomService.checkAvailable(roomId))) {
// client joins full room
client.emit(EVENT_NAME.ROOM_FULL);
return;
}

await this.roomService.joinRoom(client.id, roomId, nickname);

client.join(roomId);

console.log(`[${data.roomId}]: ${client.id} enter`);

const usersInThisRoom = (
await this.roomService.getMemberSocket(roomId)
).filter((user) => user !== client.id);

client.emit(EVENT_NAME.ALL_USERS, usersInThisRoom);
}
Copy link
Member

Choose a reason for hiding this comment

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

이 코드는 join_room 이벤트 처리하는 코드인가요? 룸 아이디랑 사용자 닉네임을 받고, 방이 꽉차면 room_full을 보내고, 아니라면 join_room을 바로 하는거 같은데, 이 부분은 비즈니스 로직과 웹소켓 통신을 분리해도 괜찮지 않을까 생각했습니다!

바로 emit, join하는게 아니라 return { status: '상태' } 이런식으로 반환(비즈니스 로직)해서 이걸 받아서, 웹소켓 통신 처리 코드에서 상태 코드를 통해 웹소켓 통신을 처리할 수도 있을거 같아요! 이렇게 분리하면 테스트 할 때 비즈니스 로직에서는 소켓 없이도 테스트 가능해지지 않을까하는 생각이 듭니다.

Copy link
Collaborator Author

@blu3fishez blu3fishez Nov 14, 2024

Choose a reason for hiding this comment

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

반환하는 상대에 따라서 저는 emit 말고 ACK 로 설정하는 것도 좋다고 생각합니다.

테스트 관련 언급해주셔서 감사합니다. 결국엔 테스트 가능하게 코드를 짜야 검증이 쉽기 때문에 중요한 요소라고 생각이 되네요. 참고하겠습니다.

- 머지 과정에서 사라진 코드 복구
@blu3fishez
Copy link
Collaborator Author

PR 메세지 새로 작성했습니다.

다 읽어봐주세요~ @twalla26 님께서는 저희 백엔드 이번주 구현 사항이 모두 설명되었는지도 확인해주세요. 저희 메인 PR이 해당 PR이다보니 이번주 발표 자료로 써볼 예정입니다.

다 읽어봐주시고 해당 메세지에 아무 반응 하나만 해주세요. 마지막으로 반응해주시는 분께서 머지해주시면 됩니다.

- `GET /api/rooms` 시, status != PRIVATE 인 모든 방 Record 반환
@blu3fishez blu3fishez force-pushed the feature/study-session branch from 00cab19 to 9edeb22 Compare November 14, 2024 09:53
- 단순 소켓 배열에서 방장 정보를 확인할 수 있도록 변경했습니다.
- `all_users` 에 방 정보 추가
- `room_created` 에 방 정보 추가
@ShipFriend0516 ShipFriend0516 merged commit 63c022e into dev Nov 14, 2024
@blu3fishez blu3fishez deleted the feature/study-session branch November 20, 2024 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants