Conversation
- contest 도메인에서 lol 도메인으로 LoL 관련 로직 분리 - LoL 세션 관리 (WebSocket Hub, Redis adapter) 추가 - LoL 커스텀 매치 도메인 및 DB 마이그레이션 추가 - notification 도메인 제거 (SSE → WebSocket으로 대체) - 팀 밸런싱 서비스 및 벤치마크 테스트 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new League of Legends (LoL) module, providing custom match session management and team balancing capabilities. It includes new domain models, services, and WebSocket-based session handling. My feedback focuses on improving system reliability and observability by recommending the inclusion of context.Context in repository interfaces to support proper timeout and cancellation propagation. Additionally, I suggested improving error handling in the WebSocket event builder and refactoring the authentication helper for better consistency across controllers.
| type LolCustomMatchRepositoryPort interface { | ||
| Save(match *domain.LolCustomMatch, players []*domain.LolCustomMatchPlayer) error | ||
| FindByID(matchID int64) (*domain.LolCustomMatch, error) | ||
| UpdateResult(match *domain.LolCustomMatch) error | ||
| } |
There was a problem hiding this comment.
리포지토리 인터페이스의 모든 메서드에 context.Context를 첫 번째 인자로 추가하는 것이 좋습니다.
요청 범위의 컨텍스트를 데이터베이스 작업까지 전달하면 타임아웃, 취소 전파, 트레이싱이 가능해져 시스템의 안정성을 높일 수 있습니다. GORM에서는 db.WithContext(ctx)를 사용하여 컨텍스트를 전달할 수 있습니다.
이 변경은 LolCustomMatchRepositoryPort 인터페이스, LolCustomMatchDatabaseAdapter 구현, 그리고 LolTeamBalanceService에 걸쳐 적용되어야 합니다.
| type LolCustomMatchRepositoryPort interface { | |
| Save(match *domain.LolCustomMatch, players []*domain.LolCustomMatchPlayer) error | |
| FindByID(matchID int64) (*domain.LolCustomMatch, error) | |
| UpdateResult(match *domain.LolCustomMatch) error | |
| } | |
| type LolCustomMatchRepositoryPort interface { | |
| Save(ctx context.Context, match *domain.LolCustomMatch, players []*domain.LolCustomMatchPlayer) error | |
| FindByID(ctx context.Context, matchID int64) (*domain.LolCustomMatch, error) | |
| UpdateResult(ctx context.Context, match *domain.LolCustomMatch) error | |
| } |
| type LolSessionRepositoryPort interface { | ||
| Save(session *domain.LolSession) error | ||
| FindByID(sessionID string) (*domain.LolSession, error) | ||
| Delete(sessionID string) error | ||
| } |
There was a problem hiding this comment.
리포지토리 인터페이스의 모든 메서드에 context.Context를 첫 번째 인자로 추가하는 것을 권장합니다.
context.Context를 전달하면 요청 타임아웃, 취소, 분산 추적과 같은 기능을 올바르게 전파할 수 있어 애플리케이션의 안정성과 관측 가능성이 향상됩니다. 현재 LolSessionRedisAdapter 구현에서는 context.Background()를 사용하고 있는데, 이는 요청 범위의 컨텍스트를 전달하는 모범 사례에 어긋납니다.
이 변경은 LolSessionRepositoryPort 인터페이스, LolSessionRedisAdapter 구현, 그리고 LolSessionService에 걸쳐 적용되어야 합니다.
| type LolSessionRepositoryPort interface { | |
| Save(session *domain.LolSession) error | |
| FindByID(sessionID string) (*domain.LolSession, error) | |
| Delete(sessionID string) error | |
| } | |
| type LolSessionRepositoryPort interface { | |
| Save(ctx context.Context, session *domain.LolSession) error | |
| FindByID(ctx context.Context, sessionID string) (*domain.LolSession, error) | |
| Delete(ctx context.Context, sessionID string) error | |
| } |
| func (c *LolSessionController) buildEvent(event string, data interface{}) []byte { | ||
| msg := dto.WsServerMessage{Event: event, Data: data} | ||
| b, _ := json.Marshal(msg) | ||
| return b | ||
| } |
There was a problem hiding this comment.
json.Marshal에서 발생하는 오류를 무시하고 있습니다. WsServerMessage에 포함되는 data의 타입이 제어되므로 실제 오류 발생 가능성은 낮지만, 만약 마샬링할 수 없는 데이터(예: 함수, 채널)가 포함될 경우 예기치 않은 동작을 유발할 수 있습니다.
오류를 로깅하고, 가능하면 클라이언트에게 내부 서버 오류를 알리는 것이 더 안정적인 구현입니다.
func (c *LolSessionController) buildEvent(event string, data interface{}) []byte {
msg := dto.WsServerMessage{Event: event, Data: data}
b, err := json.Marshal(msg)
if err != nil {
log.Printf("[LolSessionController] error marshaling event %s: %v", event, err)
// Optionally, create a marshaled error message to send to the client
errorEvent := dto.WsServerMessage{Event: dto.WsEventError, Data: gin.H{"message": "Internal server error preparing event."}}
b, _ = json.Marshal(errorEvent)
}
return b
}| func getAuthenticatedUserID(ctx *gin.Context) (int64, bool) { | ||
| userID, ok := middleware.GetUserIdFromContext(ctx) | ||
| if !ok { | ||
| response.JSON(ctx, response.Error(401, "user not authenticated")) | ||
| return 0, false | ||
| } | ||
| return userID, true | ||
| } |
There was a problem hiding this comment.
이 헬퍼 함수의 이름은 getAuthenticatedUserID이지만, 실제로는 사용자 ID를 가져오는 것 외에 인증 실패 시 HTTP 응답을 직접 전송하는 부수 효과(side effect)를 가지고 있습니다. 이는 함수 이름이 암시하는 것 이상의 동작이며, 코드의 명확성을 해칠 수 있습니다.
lol_temporal_controller.go에서 사용된 것처럼 middleware.GetUserIdFromContext를 직접 호출하고 컨트롤러 핸들러 내에서 오류를 처리하는 방식이 더 일관성 있고 명확합니다. 이 컨트롤러에서도 동일한 패턴을 적용하여 이 헬퍼 함수를 제거하는 것을 고려해 보세요.
1 participant
Summary
contest도메인에 혼재되어 있던 LoL 관련 로직을 독립적인lol도메인으로 분리notification도메인 제거 (WebSocket으로 대체)Changes
New:
internal/lol/도메인domain/:LolSession,LolCustomMatch엔티티application/: 팀 밸런싱 서비스, 세션 서비스, port 인터페이스infra/: MySQL 어댑터, Redis 어댑터, WebSocket Hubpresentation/: LoL Temporal 컨트롤러, 세션 컨트롤러provider.go: DI 와이어링Removed:
internal/notification/도메인Modified
cmd/server.go: lol 도메인 DI 등록internal/contest/: LoL 관련 로직 제거 및 정리internal/game/application/team_service.go: 팀 밸런싱 로직 lol 도메인으로 이전DB Migration
000026_create_lol_custom_matches_table: LoL 커스텀 매치 테이블 생성Tests
test/lol/application/: 팀 밸런싱 서비스 단위 테스트 및 벤치마크test/lol/presentation/: LoL Temporal 컨트롤러 단위 테스트Test plan
go test ./test/lol/...— LoL 도메인 단위 테스트 통과go test ./...— 전체 테스트 통과make migrate-up— 마이그레이션 000026 정상 적용🤖 Generated with Claude Code