Conversation
- 불필요한 @mapping 제거 - ChannelMapper participants, lastMessageAt 관련 문제 수정
raonPsm
added
미완성🛠️
|
안녕하세요! 미션 진행하시느라 고생 많으셨습니다. 현재 sprint5, sprint 6 PR 모두 conflict 해결이 필요해 보입니다. 이전 미션에서 conflict로 인해 머지를 완료하지 못한 상태로 계속 진행하시다 보니, 이번 미션에서 순수하게 작업하신 변경 사항이 명확하게 드러나지 않는 것 같습니다. conflict를 해결해 주시고, PR을 한 번 정리해 주시면 감사하겠습니다. |
byungwook-min
left a comment
byungwook-min
left a comment
There was a problem hiding this comment.
전체적으로 미션을 잘 완수하신 것 같습니다. Spring Data JPA의 핵심 개념들(영속성 컨텍스트, 변경 감지, 영속성 전이, 지연 로딩)을 왜 그렇게 동작하는지 스스로 정리하며 학습한 흔적이 곳곳에 보입니다. 이 부분이 가장 인상적이었습니다.
DDL과 엔티티 매핑의 완성도가 높습니다. schema.sql과 JPA 엔티티의 매핑이 ERD/클래스 다이어그램과 정확히 일치하고, cascade와 orphanRemoval을 적재적소에 활용하고 있습니다. 특히 BaseEntity → BaseUpdatableEntity 상속 구조와 @MappedSuperclass + @EntityListeners(AuditingEntityListener.class) 조합이 교과서적으로 잘 구현되어 있습니다.
DTO와 MapStruct 활용이 뛰어납니다. Entity를 절대 Controller 밖으로 노출하지 않고, Record 기반 DTO로 깔끔하게 분리했습니다. MapStruct의 uses, expression, abstract class 패턴 등 다양한 기법을 적절히 활용하고 있어, 보일러플레이트 코드 없이 매핑 로직이 간결합니다.
N+1 문제 식별과 대응이 적절합니다. LEFT JOIN FETCH와 @BatchSize를 상황에 맞게 사용하고 있고, OSIV=false 환경에서 트랜잭션 범위 내 DTO 변환을 수행하여 LazyInitializationException을 방지하고 있습니다.
눈에 띄는 개선 포인트는 ChannelMapper에서의 N+1 문제입니다. 현재 ChannelMapper.toDto()에서 채널마다 messageRepository와 readStatusRepository를 호출하고 있는데, 이것은 Mapper 계층이 가져야 할 책임을 넘어서는 것이기도 하고, 채널 목록 조회 시 채널 수만큼 추가 쿼리가 발생하는 성능 문제이기도 합니다. 이 로직을 Service 계층으로 이동시키고, batch 조회 후 매핑하는 방식으로 리팩토링하면 좋겠습니다.
전반적으로 요구사항을 충실히 이행하면서도, 원리를 이해하려는 노력이 돋보이는 것 같습니다. 고생하셨습니다!
| -- 기존 테이블 삭제 (초기화용) | ||
| -- 1. 가장 하위 자식 테이블 (다른 테이블들을 참조함) | ||
| DROP TABLE IF EXISTS message_attachments; | ||
| DROP TABLE IF EXISTS read_statuses; | ||
|
|
||
| -- 2. 중간 계층 테이블 (상위 부모를 참조함) | ||
| DROP TABLE IF EXISTS messages; | ||
| DROP TABLE IF EXISTS user_statuses; | ||
|
|
||
| -- 3. 최상위 부모 테이블 (참조를 당하는 입장이므로 가장 마지막에 삭제) | ||
| DROP TABLE IF EXISTS users; | ||
| DROP TABLE IF EXISTS channels; | ||
| DROP TABLE IF EXISTS binary_contents; | ||
|
|
||
| -- |
There was a problem hiding this comment.
schema.sql에서 매번 모든 테이블을 DROP하고 다시 CREATE하고 있는데요, application.yaml에서 sql.init.mode: always로 설정되어 있어서 애플리케이션을 재시작할 때마다 기존 데이터가 전부 날아갑니다. 개발 단계에서는 편할 수 있지만, 이 설정이 그대로 적용되면 문제가 될 수 있습니다.
개발 환경에서는 CREATE TABLE IF NOT EXISTS를 사용하거나, ddl-auto: validate와 함께 sql.init.mode: embedded로 변경하는 것을 고려해보세요. 아니면 프로파일을 분리해서 application-dev.yaml에서만 always를 사용하는 방법도 있습니다.
There was a problem hiding this comment.
테이블 삭제 순서를 외래키 의존 관계에 따라 하위 → 상위로 정리한 것, CHECK 제약조건으로 ChannelType을 검증하는 것, ON DELETE CASCADE와 ON DELETE SET NULL을 ERD에 맞게 적용한 것 등 DDL 작성이 꼼꼼합니다.
| datasource: | ||
| url: jdbc:postgresql://localhost:5432/discodeit # PostgreSQL DB에 접속하기 위한 JDBC URL | ||
| # jdbc:postgresql: -> PostgreSQL JDBC 드라이버 사용 | ||
| # localhost -> DB 서버 주소 | ||
| # 5432 -> PostgreSQL의 기본 포트 번호 | ||
| # discodeit -> 접속할 데이터베이스 이름 | ||
| username: discodeit_user # DB 접속 계정명 | ||
| password: discodeit1234 # 비밀번호 FIXME: ${DB_PASSWORD} + .env 파일로 관리 |
There was a problem hiding this comment.
환경변수(${DB_PASSWORD})나 .env 파일로 분리하면 좋을 것 같습니다.
There was a problem hiding this comment.
@MappedSuperclass + @EntityListeners(AuditingEntityListener.class) + @CreatedDate 조합이 정확합니다. @GeneratedValue(strategy = GenerationType.UUID)로 UUID 자동 생성하는 것도 적절하고, @Column(updatable = false, nullable = false)로 id와 createdAt이 변경되지 않도록 보호한 것도 좋습니다. @NoArgsConstructor(access = AccessLevel.PROTECTED)로 직접 인스턴스화를 방지한 것도 좋은 것 같습니다.
| // 현재 '온라인' 상태인지 확인 (5분 이내 활동 시 true) | ||
| // TODO: (Later) 온라인 / 자리 비움 / 방해 금지 / 오프라인 표시 / 오프라인 -> Enum 으로 상태 구현 | ||
| public Boolean isOnline() { | ||
| Instant instantFiveMinutesAgo = Instant.now().minus(Duration.ofMinutes(5)); | ||
| return lastActiveAt.isAfter(instantFiveMinutesAgo); | ||
| } |
There was a problem hiding this comment.
엔티티 안에 비즈니스 로직을 두는 것은 도메인 주도 설계 관점에서 좋은 선택인 것 같습니다. 다만 Boolean(wrapper)이 아닌 boolean(primitive)을 반환하는 것이 더 적절합니다.
There was a problem hiding this comment.
schema.sql의 CONSTRAINT uk_user_channel UNIQUE (user_id, channel_id)와 일치하도록 JPA 엔티티에도 복합 유니크 제약을 명시한 것이 좋습니다. name까지 지정하여 DDL과 동일하게 맞춘 것도 좋은 것 같습니다.
| @Autowired protected MessageRepository messageRepository; | ||
| @Autowired protected ReadStatusRepository readStatusRepository; | ||
| @Autowired protected UserMapper userMapper; |
There was a problem hiding this comment.
Mapper는 단순히 객체 간 변환을 담당하는 계층인데, 여기서 Repository를 직접 주입받아 DB 쿼리를 실행하고 있습니다.
이는 레이어 아키텍처 위반하여 Mapper가 데이터 접근 계층에 직접 의존하고 toDtoList() 호출 시 채널 수만큼 findTopByChannel_Id와 findAllByChannel_Id 쿼리가 추가 발생하는 N+1 문제가 발생 가능합니다.
또한, Mapper에서 발생하는 DB 조회가 Service의 트랜잭션 범위 밖에서 실행될 수 있고 Mapper를 단위 테스트하려면 Repository를 mock해야 하므로 지양하는 것이 좋습니다.
BasicChannelService에서 처리하고, Mapper는 이미 조회된 데이터만 변환하는 역할로 제한하는 것이 좋습니다.
| if (user.getUserStatus() == null) { | ||
| throw new IllegalStateException("유저 상태 데이터가 누락되었습니다. (userId: " + userId + ")"); | ||
| } | ||
|
|
There was a problem hiding this comment.
IllegalStateException을 사용한 것은 좋은데, GlobalExceptionHandler에 이 예외에 대한 핸들러가 없습니다. 현재는 handleGeneralException(Exception e)에서 500으로 처리되겠지만, 명시적으로 IllegalStateException 핸들러를 추가하거나, 이 검증이 꼭 필요한지 재고해보면 좋을 것 같습니다. User 생성 시 항상 UserStatus를 함께 만들고 있으므로, 정상적인 흐름에서는 null이 될 수 없습니다.
| // 유효성 검증 - 유저 존재 여부 검증 | ||
| for (UUID userId : requestedUserIds) { | ||
| if (!userRepository.existsById(userId)) { | ||
| throw new NoSuchElementException("유저가 존재하지 않습니다. id: " + userId); | ||
| } | ||
| } |
There was a problem hiding this comment.
참여자 수만큼 existsById 쿼리가 개별적으로 실행됩니다. 대신 findAllById(requestedUserIds)로 한 번에 조회한 후, 반환된 리스트의 크기와 요청된 ID 수를 비교하는 방식이 더 효율적입니다. 이미userRepository.findAllById(requestedUserIds)를 호출하고 있으므로, 검증과 조회를 합칠 수 있습니다.
| messageRepository.deleteAllByChannel_Id(channelId); | ||
| readStatusRepository.deleteAllByChannel_Id(channelId); |
There was a problem hiding this comment.
schema.sql에서 이미 ON DELETE CASCADE가 설정되어 있으므로, DB 레벨에서 채널 삭제 시 관련 messages와 read_statuses가 자동 삭제됩니다.
JPA 레벨에서도 명시적으로 삭제하면 불필요한 쿼리가 추가로 발생할 수 있습니다. 다만 JPA의 영속성 컨텍스트와 DB 간 일관성을 보장하기 위해 명시적으로 삭제하는 접근도 유효하므로, 의도적인 선택이라면 괜찮습니다.
2 participants
[SB] 스프린트 미션 6
🏔️ 프로젝트 마일스톤
📝 요구사항
✏️ 기본 요구사항
API 명세
이번 미션은 아래의 API 스펙과 비교하며 구현해보세요.
API 스펙을 준수한다면, 아래의 프론트엔드 코드와 호환됩니다.
프론트엔드 소스 코드는 참고용으로만 활용하세요. 수정하여 활용하는 경우 이어지는 요구사항 또는 미션을 수행하는 데 어려움이 있을 수 있습니다.
데이터베이스
discodeitdiscodeit_userdiscodeit1234작성한 DDL 파일은 /src/main/resources/schema.sql 경로에 포함하세요.
PK: Primary KeyUK: Unique KeyNN: Not NullFK: Foreign KeyON DELETE CASCADE: 연관 엔티티 삭제 시 같이 삭제ON DELETE SET NULL: 연관 엔티티 삭제 시 NULL로 변경Spring Data JPA 적용하기
application.yaml파일에 작성하세요.application.yaml파일에 작성하세요.엔티티 정의하기
클래스 다이어그램을 참고해 도메인 모델의 공통 속성을 추상 클래스로 정의하고 상속 관계를 구현하세요.
이때 Serializable 인터페이스는 제외합니다.
패키지명:
com.sprint.mission.discodeit.entity.base클래스 다이어그램
JPA의 어노테이션을 활용해
createdAt,updatedAt속성이 자동으로 설정되도록 구현하세요.@CreatedDate,@LastModifiedDate클래스 다이어그램을 참고해 클래스 참조 관계를 수정하세요. 필요한 경우 생성자, update 메소드를 수정할 수 있습니다. 단, 아직 JPA Entity와 관련된 어노테이션은 작성하지 마세요.
클래스 다이어그램
화살표의 방향과 화살표 유무에 유의하세요.
ERD와 클래스 다이어그램을 토대로 연관관계 매핑 정보를 표로 정리해보세요.(이 내용은 PR에 첨부해주세요.)
@Entity,@Table@Column,@Enumerated@OneToMany,@OneToOne,@ManyToOne@JoinColumn,@JoinTablecascade,orphanRemoval레포지토리와 서비스에 JPA 도입하기
트랜잭션,영속성 전이,변경 감지,지연로딩DTO 적극 도입하기
다음의 클래스 다이어그램을 참고하여 DTO를 정의하세요.
Entity를 DTO로 매핑하는 로직을 책임지는 Mapper 컴포넌트를 정의해 반복되는 코드를 줄여보세요.
com.sprint.mission.discodeit.mapperBinaryContent 저장 로직 고도화
데이터베이스에 이미지와 같은 파일을 저장하면 성능 상 불리한 점이 많습니다. 따라서 실제 바이너리 데이터는 별도의 공간에 저장하고, 데이터베이스에는 바이너리 데이터에 대한 메타 정보(파일명, 크기, 유형 등)만 저장하는 것이 좋습니다.
BinaryContent 엔티티는 파일의 메타 정보(
fileName,size,contentType)만 표현하도록bytes속성을 제거하세요.BinaryContent의
byte[]데이터 저장을 담당하는 인터페이스를 설계하세요.저장 매체의 확장성(로컬 저장소, 원격 저장소)을 고려해 인터페이스부터 설계합니다.
패키지명:
com.sprint.mission.discodeit.storage클래스 다이어그램
BinaryContentStorage
UUID put(UUID, byte[])byte[]데이터를 저장합니다.InputStream get(UUID)byte[]데이터를 읽어 InputStream 타입으로 반환합니다.ResponseEntity<?> download(BinaryContentDto)서비스 레이어에서 기존에 BinaryContent를 저장하던 로직을 BinaryContentStorage를 활용하도록 리팩토링하세요.
BinaryContentController에 파일을 다운로드하는 API를 추가하고, BinaryContentStorage에 로직을 위임하세요.
엔드포인트:
GET /api/binaryContents/{binaryContentId}/download요청
응답:
ResponseEntity<?>클래스 다이어그램
로컬 디스크 저장 방식으로 BinaryContentStorage 구현체를 구현하세요.
클래스 다이어그램
discodeit.storage.type값이local인 경우에만 Bean으로 등록되어야 합니다.Path rootdiscodeit.storage.local.root-path설정값을 정의하고, 이 값을 통해 주입합니다.void init()Path resolvePath(UUID){root}/{UUID}put,get메소드에서 호출해 일관된 파일 경로 규칙을 유지합니다.ResponseEntity<Resource> donwload(BinaryContentDto)get메소드를 통해 파일의 바이너리 데이터를 조회합니다.ResponseEntity<Resource>응답을 생성 후 반환합니다.페이징과 정렬
메시지 목록을 조회할 때 다음의 조건에 따라 페이지네이션 처리를 해보세요.
일관된 페이지네이션 응답을 위해 제네릭을 활용해 DTO로 구현하세요.
패키지명:
com.sprint.mission.discodeit.dto.response클래스 다이어그램
content: 실제 데이터입니다.number: 페이지 번호입니다.size: 페이지의 크기입니다.totalElements: T 데이터의 총 갯수를 의미하며, null일 수 있습니다.Slice 또는 Page 객체로부터 DTO를 생성하는 Mapper를 구현하세요.
패키지명:
com.sprint.mission.discodeit.mapper확장성을 위해 제네릭 메소드로 구현하세요.
✏️ 심화 요구사항
N+1 문제
읽기전용 트랜잭션 활용
OSIV 비활성화하기
spring: jpa: open-in-view: false페이지네이션 최적화
이 내용은 PR에 첨부해주세요.
PageResponse는 다음과 같이 변경하세요.
다음의 API 명세를 준수하세요.
API 스펙을 준수한다면, 아래의 프론트엔드 코드와 호환됩니다.
프론트엔드 소스 코드는 참고용으로만 활용하세요. 수정하여 활용하는 경우 이어지는 요구사항 또는 미션을 수행하는 데 어려움이 있을 수 있습니다.
MapStruct 적용
🔄 주요 변경사항
📸 스크린샷
🙇🏽♂️ 멘토에게
⚖️ License & Copyright