Conversation
joonfluence
left a comment
joonfluence
left a comment
There was a problem hiding this comment.
전체 요약
Spring Data JPA 마이그레이션, DTO/Mapper 패턴 도입, BinaryContentStorage 추상화, 커서 기반 페이지네이션 구현이 포함된 Sprint 6 과제입니다. 전반적으로 잘 구현되었으나, application.yaml 설정 오류, N+1 문제, 엔티티 직접 노출 방지 미흡 등 일부 개선이 필요합니다.
라인 코멘트로 달 수 없는 항목
[p4] .discodeit/*.ser 바이너리 파일과 storage/ 디렉토리의 바이너리 데이터가 PR에 포함되어 있습니다. 로컬 테스트 데이터이므로 .gitignore에 추가하여 VCS에서 제외하는 것이 좋습니다.
[p5] ReadStatusMapper, UserStatusMapper에 MapStruct를 적절히 도입한 점 좋습니다! 다른 Mapper(MessageMapper, ChannelMapper, UserMapper)도 MapStruct로 통일하면 보일러플레이트를 더 줄일 수 있습니다.
| datasource: | ||
| url: jdbc:postgresql://localhost:5432/discodeit | ||
| username: discodeit_user | ||
| password: discodeit1234 |
There was a problem hiding this comment.
[p1] 꼭 반영해주세요
sql.init.mode와 jpa 설정이 spring: 하위가 아닌 루트 레벨에 위치해 있어, Spring Boot가 이 설정을 인식하지 못합니다.
현재 구조:
spring:
datasource: ...
sql: # ← spring 밖
init:
mode: always
jpa: # ← spring 밖
hibernate:
ddl-auto: nonespring: 블록 안으로 옮겨야 DDL 초기화와 JPA 설정(ddl-auto, show-sql, format_sql 등)이 정상 동작합니다.
| password: discodeit1234 | |
| datasource: | |
| url: jdbc:postgresql://localhost:5432/discodeit | |
| username: discodeit_user | |
| password: discodeit1234 | |
| driver-class-name: org.postgresql.Driver | |
| sql: | |
| init: | |
| mode: always |
| .findLastMessageTimesByChannelIds(channelIds) | ||
| .stream() | ||
| .collect(Collectors.toMap( | ||
| row -> (UUID) row[0], |
There was a problem hiding this comment.
[p2] 적극적으로 고려해주세요
findAllByChannelIdIn으로 ReadStatus를 가져온 뒤 rs.getUser()를 호출하면 N+1 문제가 발생합니다. ReadStatus마다 User를 개별 SELECT하게 됩니다.
Repository에 JOIN FETCH 쿼리를 추가하여 ReadStatus + User를 한 번에 가져오도록 개선해주세요:
@Query("SELECT rs FROM ReadStatus rs JOIN FETCH rs.user WHERE rs.channel.id IN :channelIds")
List<ReadStatus> findAllByChannelIdInWithUser(@Param("channelIds") List<UUID> channelIds);| return new MessageDto( | ||
| message.getId(), | ||
| message.getCreatedAt(), | ||
| message.getUpdatedAt(), |
There was a problem hiding this comment.
[p2] 적극적으로 고려해주세요
message.getAuthor()와 message.getAttachments()에 접근하는데, OSIV가 비활성화된 상태에서 이들이 LAZY 로딩이면 LazyInitializationException이 발생할 수 있습니다.
findByChannelIdWithCursor 쿼리에서 JOIN FETCH로 author와 attachments를 함께 로딩하거나, @EntityGraph를 사용해주세요:
@Query("""
SELECT m FROM Message m
JOIN FETCH m.author
LEFT JOIN FETCH m.attachments
WHERE m.channel.id = :channelId
AND (CAST(:cursor AS java.time.Instant) IS NULL OR m.createdAt < :cursor)
ORDER BY m.createdAt DESC
""")
Slice<Message> findByChannelIdWithCursor(...);| encodedPassword, | ||
| profile | ||
| ); | ||
| UserStatus userStatus = new UserStatus(user); |
There was a problem hiding this comment.
[p2] 적극적으로 고려해주세요
update 시 username/email 중복 검사에서 자기 자신의 기존 값도 중복으로 판정됩니다. 예를 들어 email만 바꾸고 username을 그대로 보내면 "이미 존재하는 유저네임" 에러가 발생합니다.
자기 자신을 제외하는 조건을 추가해주세요:
if (jpaUserRepository.existsByUsernameAndIdNot(request.newUsername(), userId)) {
throw new IllegalArgumentException("이미 존재하는 유저네임입니다.");
}| private final BinaryContentMapper binaryContentMapper; | ||
|
|
||
| public UserDto toDto(User user) { | ||
| Boolean online = user.getUserStatus() != null |
There was a problem hiding this comment.
[p3] 웬만하면 반영해 주세요
User → UserStatus는 @OneToOne(mappedBy = "user")인데, findAll()에서 모든 유저를 조회하면 유저 수만큼 UserStatus SELECT가 추가 실행되어 N+1이 발생합니다.
Repository에 @EntityGraph나 JOIN FETCH를 적용하여 UserStatus를 함께 로딩하는 것을 고려해주세요.
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "channel_id", nullable = false) | ||
| private Channel channel; |
There was a problem hiding this comment.
[p3] 웬만하면 반영해 주세요
컬럼명에 대문자 A가 포함되어 있습니다 (last_read_At). schema.sql의 last_read_at과 일관성을 위해 소문자로 수정하는 것이 좋습니다.
| private Channel channel; | |
| @Column(name = "last_read_at", nullable = false) |
| operationId = "create_2") | ||
| @ApiResponses(value = { | ||
| @ApiResponse(responseCode = "201", | ||
| description = "Message가 성공적으로 생성됨"), |
There was a problem hiding this comment.
[p3] 웬만하면 반영해 주세요
파일 읽기 실패 시 RuntimeException으로 감싸면 클라이언트에게 500 에러만 전달됩니다. 적절한 메시지를 포함하면 디버깅에 도움이 됩니다.
| description = "Message가 성공적으로 생성됨"), | |
| } catch (IOException e) { | |
| throw new RuntimeException("첨부파일 읽기 실패: " + file.getOriginalFilename(), e); |
| private final PasswordEncoder passwordEncoder; | ||
| private final UserMapper userMapper; | ||
|
|
||
| public UserDto login(UserLoginRequest request) { |
There was a problem hiding this comment.
[p3] 웬만하면 반영해 주세요
login 메서드에 @Transactional(readOnly = true)가 없습니다. OSIV가 비활성화된 상태에서 userMapper.toDto(user) 시 UserStatus 등 LAZY 필드 접근 시 LazyInitializationException이 발생할 수 있습니다.
| public UserDto login(UserLoginRequest request) { | |
| @Transactional(readOnly = true) | |
| public UserDto login(UserLoginRequest request) { |
2 participants
기본 요구사항
API 명세
이번 미션은 아래의 API 스펙과 비교하며 구현해보세요.
API 스펙 v1.1
API 스펙을 준수한다면, 아래의 프론트엔드 코드와 호환됩니다.
정적 리소스 v1.1.4
소스 코드(참고용) v1.1.4
데이터베이스
작성한 DDL 파일은 /src/main/resources/schema.sql 경로에 포함하세요.
Spring Data JPA 적용하기
엔티티 정의하기
이때 Serializable 인터페이스는 제외합니다.
@CreatedDate, @LastModifiedDate
자식: UserStatus
자식: ReadStatus
자식: Message
자식: ReadStatus
자식: Message
자식: BinaryContent
자식: BinaryContent
@entity, @table
@column, @Enumerated
@onetomany, @OnetoOne, @manytoone
@joincolumn, @jointable
레포지토리와 서비스에 JPA 도입하기
DTO 적극 도입하기
Entity를 Controller 까지 그대로 노출했을 때 발생할 수 있는 문제점에 대해 정리해보세요. DTO를 적극 도입했을 때 보일러플레이트 코드가 많아지지만, 그럼에도 불구하고 어떤 이점이 있는지 알 수 있을거에요.
힌트
다음의 클래스 다이어그램을 참고하여 DTO를 정의하세요.
BinaryContent 저장 로직 고도화
데이터베이스에 이미지와 같은 파일을 저장하면 성능 상 불리한 점이 많습니다. 따라서 실제 바이너리 데이터는 별도의 공간에 저장하고, 데이터베이스에는 바이너리 데이터에 대한 메타 정보(파일명, 크기, 유형 등)만 저장하는 것이 좋습니다.
BinaryContent 엔티티는 파일의 메타 정보(fileName, size, contentType)만 표현하도록 bytes 속성을 제거하세요.
BinaryContent의 byte[] 데이터 저장을 담당하는 인터페이스를 설계하세요.
저장 매체의 확장성(로컬 저장소, 원격 저장소)을 고려해 인터페이스부터 설계합니다.
패키지명: com.sprint.mission.discodeit.storage
BinaryContentStorage
바이너리 데이터의 저장/로드를 담당하는 컴포넌트입니다.
UUID put(UUID, byte[])
InputStream get(UUID)
ResponseEntity<?> download(BinaryContentDto)
서비스 레이어에서 기존에 BinaryContent를 저장하던 로직을 BinaryContentStorage를 활용하도록 리팩토링하세요.
BinaryContentController에 파일을 다운로드하는 API를 추가하고, BinaryContentStorage에 로직을 위임하세요.
페이징과 정렬
패키지명: com.sprint.mission.discodeit.dto.response
심화 요구사항
N+1 문제
읽기전용 트랜잭션 활용
페이지네이션 최적화
오프셋 페이지네이션과 커서 페이지네이션 방식의 차이에 대해 정리해보세요.
기존에 구현한 오프셋 페이지네이션을 커서 페이지네이션으로 리팩토링하세요.
스크린샷
멘토에게