Conversation
joonfluence
left a comment
There was a problem hiding this comment.
전체 요약
Spring Boot 기반 Discodeit 백엔드를 RESTful API로 재설계한 PR입니다. Controller-Service-Repository 계층 분리, DTO 도입, Swagger 문서화, 전역 예외 처리 등 전반적으로 잘 구성되어 있습니다.
특히 Controller-Api 인터페이스 분리 패턴, @RestControllerAdvice 전역 예외 처리, @ConditionalOnProperty를 활용한 Repository 전략 패턴 등이 인상적입니다.
아래 코멘트들을 참고해주세요.
- P2: 2건 (적극적으로 고려해주세요)
- P3: 4건 (웬만하면 반영해 주세요)
- P4: 2건 (반영해도 좋고 넘어가도 좋습니다)
| User user = userRepository.findByUsername(request.getUsername()) | ||
| .orElseThrow(() -> new NoSuchElementException( | ||
| "User with username " + request.getUsername() + " not found")); | ||
| if (!user.getPassword().equals(request.getPassword())) { |
There was a problem hiding this comment.
[P2] 비밀번호를 평문(plain text)으로 저장하고 equals()로 직접 비교하고 있습니다.
학습 프로젝트라 하더라도 BCryptPasswordEncoder 같은 해시 기반 비교를 사용하는 습관을 들이는 것이 좋습니다. 평문 비밀번호가 직렬화된 파일이나 로그에 노출될 위험이 있습니다.
| if (!user.getPassword().equals(request.getPassword())) { | |
| if (!passwordEncoder.matches(request.getPassword(), user.getPassword())) { |
(PasswordEncoder를 주입받고, 회원가입 시 passwordEncoder.encode()로 저장하는 로직도 함께 필요합니다)
| public class ChannelController implements ChannelApi { | ||
|
|
||
| private final ChannelService channelService; | ||
| private final MessageRepository messageRepository; |
There was a problem hiding this comment.
[P2] Controller가 MessageRepository를 직접 주입받아 마지막 메시지 시간을 조회하고 있습니다.
이는 계층 구조(Controller → Service → Repository)를 위반합니다. ChannelService 또는 MessageService에 마지막 메시지 시간 조회 로직을 위임하고, Controller에서는 Service만 의존하도록 수정하는 것이 바람직합니다.
| private final MessageRepository messageRepository; | |
| // private final MessageRepository messageRepository; // Repository 직접 의존 제거 |
| .orElse(ResponseEntity.notFound().build()); | ||
| } | ||
|
|
||
| private BinaryContentResponse convertToResponse(BinaryContent content) { |
There was a problem hiding this comment.
[P3] BinaryContentResponse에 byte[]가 포함되어 있어 JSON 응답 시 Base64로 직렬화됩니다.
파일 크기가 커지면 메모리 사용량과 응답 크기가 급증합니다. 메타데이터 조회 API에서는 바이트 데이터를 제외하고, 파일 다운로드는 별도의 스트리밍 엔드포인트(StreamingResponseBody 또는 Resource)로 제공하는 것을 권장합니다.
|
|
||
| @ExceptionHandler(Exception.class) | ||
| public ResponseEntity<String> handleAll(Exception e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
[P3] e.printStackTrace()는 stdout으로 출력되어 로그 수집 도구에서 누락될 수 있습니다. SLF4J 로거를 사용해주세요.
| e.printStackTrace(); | |
| log.error("서버 내부 오류 발생", e); |
(클래스에 @Slf4j 어노테이션 또는 private static final Logger log = LoggerFactory.getLogger(GlobalExceptionHandler.class); 추가 필요)
| private final UserStatusService userStatusService; | ||
|
|
||
| @Override | ||
| public ResponseEntity<UserDto> login(@Valid @RequestBody LoginRequest request, |
There was a problem hiding this comment.
[P3] AuthController와 UserController에 동일한 convertToDto(User) 메서드가 중복되어 있습니다.
변환 로직이 변경되면 양쪽 모두 수정해야 하므로, 별도의 UserMapper 클래스로 추출하거나 서비스 계층에서 DTO 변환을 담당하도록 하면 유지보수성이 높아집니다.
| this.authorId = authorId; | ||
| this.channelId = channelId; | ||
| if (attachmentIds != null) { | ||
| this.attachmentIds = attachmentIds; |
There was a problem hiding this comment.
[P3] Message.update() 내부에서 recordUpdate()를 호출하지 않고 있습니다.
현재는 BasicMessageService.update()에서 별도로 recordUpdate()를 호출하고 있지만, Channel.update()는 엔티티 내부에서 recordUpdate()를 호출하는 등 패턴이 일관되지 않습니다.
엔티티의 상태 변경 메서드 내부에서 타임스탬프 갱신을 통일하면 실수를 줄일 수 있습니다.
| this.attachmentIds = attachmentIds; | |
| public void update(String content) { | |
| if (content == null || content.isBlank()) { | |
| throw new IllegalArgumentException("메시지 내용은 비어있을 수 없습니다."); | |
| } | |
| this.content = content; | |
| recordUpdate(); | |
| } |
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; | ||
| import org.springframework.web.multipart.MultipartFile; | ||
| import jakarta.servlet.http.HttpSession; |
There was a problem hiding this comment.
[P4] jakarta.servlet.http.HttpSession import가 7번째 줄과 중복됩니다. 하나를 제거해주세요.
| @AllArgsConstructor | ||
| public class UserCreateRequest { | ||
|
|
||
| private String username; |
There was a problem hiding this comment.
[P4] PublicChannelCreateRequest에는 @NotBlank가 적용되어 있지만, UserCreateRequest에는 username, email, password 등 필수 필드에 대한 validation 어노테이션이 없습니다.
회원가입 시 빈 값이 들어올 수 있으므로 @NotBlank, @Email 등을 추가하면 좋겠습니다.
Discodeit Backend Project - Mission 5
1. 요구사항
기본 항목
심화 항목
정적 리소스 서빙 및 프론트엔드 통합: 백엔드 서버에서 정적 리소스(HTML/JS/CSS)를 서빙하도록 설정하여 프론트엔드-백엔드 간의 통합 연동을 완수했습니다.
Railway.app 클라우드 배포: GitHub 레포지토리와 Railway.app을 연동하여 CI/CD 환경을 구축하고, 자동 생성된 도메인을 통해 실서비스 환경에서 동작을 검증했습니다.
서버 URL: https://9-sprint-mission-production-e541.up.railway.app
멀티파트 파일 핸들링:
MultipartFile인터페이스를 활용하여 메시지 전송 시 파일 첨부 및 서버 로컬 저장 기능을 구현했습니다.채널 타입별 비즈니스 로직 분리: PUBLIC/PRIVATE 채널 성격에 따른 생성 및 조회 권한 로직을 분리하여 구현했습니다.
2. 주요 변경사항
@RestControllerAdvice를 통해 중앙 집약적으로 관리하도록 변경했습니다.@RequestMapping을 활용하여 HTTP 메서드와 경로를 명시적으로 매핑함으로써 API의 직관성을 높였습니다.binaryContentId)를 통해 서버 내 정적 리소스에 접근할 수 있는 인터페이스를 마련했습니다.UserDto와 같은 전용 데이터 객체를 도입하여 비즈니스 엔티티가 직접 외부로 노출되는 것을 방지했습니다.MultipartFile을 동시에 수용하는 구조를 구축했습니다. UUID 식별자를 포인터처럼 활용하여 작성자, 채널, 메시지 간의 논리적 연결 고리를 생성하고 데이터의 무결성을 유지했습니다.3. API 테스트 가이드 (Postman)
본 프로젝트의 모든 API 기능 및 테스트 시나리오는 제공된 Postman 컬렉션 파일을 통해 확인할 수 있습니다.
discodeit/postman/mission5.postman_collection.json4. 멘토님께 드리는 질문