-
Notifications
You must be signed in to change notification settings - Fork 16
[강은혁] Sprint12 #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 강은혁
Are you sure you want to change the base?
[강은혁] Sprint12 #175
Conversation
# Conflicts: # .gitignore # build.gradle # gradle/wrapper/gradle-wrapper.jar # gradle/wrapper/gradle-wrapper.properties # gradlew # gradlew.bat # settings.gradle # src/main/java/com/sprint/mission/discodeit/entity/Channel.java # src/main/java/com/sprint/mission/discodeit/entity/Message.java # src/main/java/com/sprint/mission/discodeit/entity/User.java # src/main/java/com/sprint/mission/discodeit/repository/ChannelRepository.java # src/main/java/com/sprint/mission/discodeit/repository/MessageRepository.java # src/main/java/com/sprint/mission/discodeit/repository/UserRepository.java # src/main/java/com/sprint/mission/discodeit/service/ChannelService.java # src/main/java/com/sprint/mission/discodeit/service/MessageService.java # src/main/java/com/sprint/mission/discodeit/service/UserService.java
# Conflicts: # build.gradle # src/main/java/com/sprint/mission/discodeit/controller/AuthController.java # src/main/java/com/sprint/mission/discodeit/controller/BinaryContentController.java # src/main/java/com/sprint/mission/discodeit/controller/ChannelController.java # src/main/java/com/sprint/mission/discodeit/controller/MessageController.java # src/main/java/com/sprint/mission/discodeit/controller/ReadStatusController.java # src/main/java/com/sprint/mission/discodeit/controller/UserController.java # src/main/java/com/sprint/mission/discodeit/dto/request/BinaryContentCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/LoginRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/MessageCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/MessageUpdateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/PrivateChannelCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/PublicChannelCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/PublicChannelUpdateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/ReadStatusCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/ReadStatusUpdateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/UserCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/UserStatusCreateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/UserStatusUpdateRequest.java # src/main/java/com/sprint/mission/discodeit/dto/request/UserUpdateRequest.java # src/main/java/com/sprint/mission/discodeit/entity/User.java # src/main/java/com/sprint/mission/discodeit/entity/base/BaseEntity.java # src/main/java/com/sprint/mission/discodeit/exception/DiscodeitException.java # src/main/java/com/sprint/mission/discodeit/exception/ErrorCode.java # src/main/java/com/sprint/mission/discodeit/exception/ErrorResponse.java # src/main/java/com/sprint/mission/discodeit/exception/GlobalExceptionHandler.java # src/main/java/com/sprint/mission/discodeit/exception/channel/ChannelException.java # src/main/java/com/sprint/mission/discodeit/exception/channel/ChannelNotFoundException.java # src/main/java/com/sprint/mission/discodeit/exception/channel/PrivateChannelUpdateException.java # src/main/java/com/sprint/mission/discodeit/exception/message/MessageException.java # src/main/java/com/sprint/mission/discodeit/exception/message/MessageNotFoundException.java # src/main/java/com/sprint/mission/discodeit/exception/user/UserException.java # src/main/java/com/sprint/mission/discodeit/exception/user/UserNotFoundException.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicAuthService.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicBinaryContentService.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicChannelService.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicMessageService.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicReadStatusService.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicUserService.java # src/main/java/com/sprint/mission/discodeit/service/basic/BasicUserStatusService.java # src/main/resources/application-dev.yaml # src/main/resources/application-prod.yaml # src/main/resources/application.yaml # src/main/resources/logback-spring.xml
|
웹소켓 구현부분이 아예 누락되어 있습니다. 이부분 확인이 필요 합니다. |
|
sprint 12 에 대한 구현이 아닌듯합니다. 이부분에 대해 다시 확인해주시기 바랍니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements real-time communication features using WebSocket and SSE (Server-Sent Events), along with comprehensive test coverage across the application. The implementation includes authentication/authorization for WebSocket connections, SSE message broadcasting, and infrastructure for distributed deployment using Docker Compose, Nginx, Redis, and Kafka.
Key Changes:
- Added WebSocket and SSE infrastructure for real-time messaging
- Implemented comprehensive test suite covering repositories, services, controllers, and integrations
- Added S3 storage support as an alternative to local storage
- Refactored services to use DTOs and proper exception handling
Reviewed Changes
Copilot reviewed 216 out of 232 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Test files (multiple) | Added unit and integration tests for all major components |
| S3BinaryContentStorage.java | Implements S3-based binary content storage with retry logic |
| BasicUserService.java | Refactored user service with proper validation and caching |
| BasicNotificationService.java | New service for handling notifications |
| application*.yaml | Configuration files for different environments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public S3BinaryContentStorage( | ||
| @Value("${discodeit.storage.s3.access-key}") String accessKey, | ||
| @Value("${discodeit.storage.s3.secret-key}") String secretKey, | ||
| @Value("${discodeit.storage.s3.region}") String region, | ||
| @Value("${discodeit.storage.s3.bucket}") String bucket, | ||
| BasicNotificationService basicNotificationService | ||
| ) { |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constructor is missing JavaDoc documentation explaining the purpose of each parameter, especially the notification service dependency which may not be obvious for S3 storage operations.
| private S3Client getS3Client() { | ||
| return S3Client.builder() | ||
| .region(Region.of(region)) | ||
| .credentialsProvider( | ||
| StaticCredentialsProvider.create( | ||
| AwsBasicCredentials.create(accessKey, secretKey) | ||
| ) | ||
| ) | ||
| .build(); | ||
| } |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new S3Client instance on every invocation is inefficient. The S3Client should be created once during construction and reused, as it's thread-safe and intended to be a singleton.
| private S3Presigner getS3Presigner() { | ||
| return S3Presigner.builder() | ||
| .region(Region.of(region)) | ||
| .credentialsProvider( | ||
| StaticCredentialsProvider.create( | ||
| AwsBasicCredentials.create(accessKey, secretKey) | ||
| ) | ||
| ) | ||
| .build(); | ||
| } |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to getS3Client(), creating a new S3Presigner on every call is inefficient. S3Presigner should be instantiated once and reused across calls.
| try { | ||
| Thread.sleep(3000); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException("Thread interrupted while simulating delay", e); | ||
| } |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded 3-second sleep appears to be debug/test code that should not be in production. This should be removed or made configurable via a feature flag for testing purposes only.
| } | ||
|
|
||
| @Transactional | ||
| @CacheEvict(value = "userNotifications", key = "#notificationId") |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache key uses #notificationId but the cache is keyed by #receiverId in the @Cacheable annotation. This mismatch means cache eviction won't work correctly. The key should be #userDetails.userDto.id.
| @CacheEvict(value = "userNotifications", key = "#notificationId") | |
| @CacheEvict(value = "userNotifications", key = "#userDetails.userDto.id") |
| UserDto userDto = new UserDto( | ||
| UUID.randomUUID(), | ||
| "eunhyeok", | ||
| "[email protected]", | ||
| null, | ||
| true | ||
| ); |
Copilot
AI
Nov 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UserDto constructor call is missing the Role parameter based on the constructor shown in other test files. This will cause a compilation error.
프로젝트 마일스톤
웹소켓 구현하기
클라이언트는 웹소켓으로 /pub/messages 엔드포인트에 메시지를 전송할 수 있어야 합니다.
메시지 전송 요청의 페이로드 타입은 MessageCreateRequest 를 그대로 활용합니다.
메시지 수신
MessageCreatedEvent를 통해 새로운 메시지 생성 이벤트를 확인하세요.
SimpMessagingTemplate를 통해 적절한 엔드포인트로 메시지를 전송하세요.
SSE 구현하기
SSE 환경을 구성하세요.
사용자별 SseEmitter 객체를 생성하고 메시지를 전송하는 컴포넌트를 구현하세요.
connect: SseEmitter 객체를 생성합니다.
send, broadcast: SseEmitter 객체를 통해 이벤트를 전송합니다.
cleanUp: 주기적으로 ping을 보내서 만료된 SseEmitter 객체를 삭제합니다.
ping: 최초 연결 또는 만료 여부를 확인하기 위한 용도로 더미 이벤트를 보냅니다.
ConcurrentMap: 스레드 세이프한 자료구조를 사용합니다.
List: 사용자 당 N개의 연결을 허용할 수 있도록 합니다. (예: 다중 탭)
이벤트 유실 복원을 위해 SSE 메시지를 저장하는 컴포넌트를 구현하세요.
각 메시지 별로 고유한 ID를 부여합니다.
클라이언트에서 LastEventId를 전송해 이벤트 유실 복원이 가능하도록 해야 합니다.
기존에 클라이언트에서 폴링 방식으로 주기적으로 요청하던 데이터를 SSE를 이용해 서버에서 실시간으로 전달하는 방식으로 리팩토링하세요.
idnamenotifications.createddataNotificationDtoidnamebinaryContents.updateddataBinaryContentDtoidnamechannels.created/channels.updated/channels.deleteddataChannelDtoidnameusers.created/users.updated/users.deleteddataUserDto배포 아키텍처 구성하기
Reverse Proxy
외부에서 접근 가능한 유일한 컨테이너이며, 3000번 포트를 통해 접근할 수 있어야 합니다.
Backend
DB, Memory DB, Message Broker
각 컨테이너는 Docker Compose 네트워크를 통해 백엔드에서 통신할 수 있어야 합니다.
외부 네트워크와 단절되어야 합니다.
웹소켓 인증/인가 처리하기
참고 문서: Spring 공식 문서
인가 처리
AuthorizationChannelInterceptor를 사용해 메시지 권한 검사를 수행합니다.
AuthorizationChannelInterceptor를 활용하기 위해의존성을 추가하세요.
분산 환경 배포 아키텍처 구성하기
Backend-*
Reverse Proxy
$upstream_addr 변수를 활용해 실제 요청을 처리하는 서버의 IP를 헤더에 추가하고 브라우저 개발자 도구를 활용해 비교해보세요.
멘토에게