-
Notifications
You must be signed in to change notification settings - Fork 2
feat: 행사 타임테이블 상세 조회 responsedto 필드 변경 - #129 #130
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
Conversation
📝 WalkthroughSummary by CodeRabbit
Summary by CodeRabbit
WalkthroughTimetableBlock 및 TimetableBlockEntity에서 imageUrl을 제거하고, 블록 미디어용 엔티티/도메인/레포지토리/리트리버를 새로 추가했다. TimetableDetailResponse는 imageUrl을 대체해 blockInfoUrl + media(List)로 변경되며, TimetableService는 블록 미디어를 조회·정렬해 응답에 매핑하도록 수정됐다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as 클라이언트
participant S as TimetableService
participant M as TimetableBlockMediaRetriever
participant R as TimetableBlockMediaRepository
C->>S: getEventTimetableDetail(eventId, blockId, userId)
Note over S: 기존 블록/영역/카테고리 조회 유지
S->>M: getAllTimetableBlockMediaByBlockId(blockId)
M->>R: findAllByTimetableBlockId(blockId)
R-->>M: List<TimetableBlockMediaEntity>
M-->>S: List<TimetableBlockMedia> (엔티티→도메인 매핑)
S->>S: sequence 오름차순 정렬 및 MediaInfo 매핑
S->>S: getUserLiked(userId, blockId)
S-->>C: TimetableDetailResponse(blockInfoUrl, media[List], ...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/entity/TimetableBlockEntity.java (1)
61-72: NPE 가능성: startAt/endAt, blockName, information에 대한 null 방어가 필요합니다현재 validateDateTime 호출 전에 startAt/endAt가 null이면 NPE가 발생합니다. DB 제약(nullable=false)만으로는 엔티티 생성 시점을 보호하지 못하므로 방어 로직을 권장합니다.
다음과 같이 안전한 로컬 변수로 null을 점검하고 검증/할당 순서를 정리해 주세요:
- validateDateTime(startAt, endAt); - - this.timetableId = timetableId; - this.timetableCategoryId = timetableCategoryId; - this.timetableAreaId = timetableAreaId; - this.startAt = startAt; - this.endAt = endAt; - this.blockName = blockName; - this.artist = artist; - this.information = information; - this.blockInfoRedirectUrl = blockInfoRedirectUrl; + final LocalDateTime safeStartAt = java.util.Objects.requireNonNull(startAt, "startAt must not be null"); + final LocalDateTime safeEndAt = java.util.Objects.requireNonNull(endAt, "endAt must not be null"); + validateDateTime(safeStartAt, safeEndAt); + + this.timetableId = timetableId; + this.timetableCategoryId = timetableCategoryId; + this.timetableAreaId = timetableAreaId; + this.startAt = safeStartAt; + this.endAt = safeEndAt; + this.blockName = java.util.Objects.requireNonNull(blockName, "blockName must not be null"); + this.artist = artist; + this.information = java.util.Objects.requireNonNull(information, "information must not be null"); + this.blockInfoRedirectUrl = blockInfoRedirectUrl;
🧹 Nitpick comments (19)
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/entity/TimetableBlockEntity.java (2)
86-90: 동시간 시작·종료 허용 여부 확인 필요현재는 startAt > endAt만 금지되어 startAt == endAt은 허용됩니다. “양의 길이”가 요구된다면 아래처럼 변경이 필요합니다.
- if (startAt.isAfter(endAt)) { + if (!startAt.isBefore(endAt)) { throw new TimeTableIllegalArgumentException(); }
50-60: 긴 매개변수 목록 개선 제안(가독성/누락 방지)생성자/팩토리에 9개 인자가 연속 전달됩니다. 빌더 또는 CreateCommand(레코드/DTO)로 캡슐화하면 가독성과 안전성이 좋아집니다.
예시:
public record TimetableBlockCreateCommand( long timetableId, long timetableCategoryId, long timetableAreaId, LocalDateTime startAt, LocalDateTime endAt, String blockName, String artist, String information, String blockInfoRedirectUrl ) {} public static TimetableBlockEntity create(TimetableBlockCreateCommand c) { return new TimetableBlockEntity( c.timetableId(), c.timetableCategoryId(), c.timetableAreaId(), c.startAt(), c.endAt(), c.blockName(), c.artist(), c.information(), c.blockInfoRedirectUrl() ); }src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/dto/TimetableDetailResponse.java (3)
15-16: blockInfoUrl ↔ blockInfoRedirectUrl 네이밍 불일치 정리 필요DTO 필드명은 blockInfoUrl인데, 팩토리 메서드 파라미터/서비스 쪽은 blockInfoRedirectUrl을 사용합니다. 의미적·계약적 혼동을 막기 위해 한 가지로 통일하세요. 외부 공개 DTO이므로 변경 시 API 문서/클라이언트 영향도 함께 확인 필요.
권장 변경안(필드명을 blockInfoRedirectUrl로 통일):
- String blockInfoUrl, + String blockInfoRedirectUrl, ... - blockInfoRedirectUrl, + blockInfoRedirectUrl,이 변경은 응답 JSON 키도 변경합니다. 클라이언트가 기존 blockInfoUrl을 사용한다면 @JsonProperty로 하위호환을 유지하세요(필요 시 제안 diff 제공 가능).
Also applies to: 29-33, 40-41
19-21: media null 방지 및 불변 리스트로 방어적 복사media가 null로 전달되면 직렬화 시 null 출력/클라이언트 NPE 유발 여지가 있습니다. 빈 리스트로 정규화하고 불변화하세요.
적용 diff:
- final List<MediaInfo> mediaInfo) { - return new TimetableDetailResponse( + final List<MediaInfo> mediaInfo) { + final List<MediaInfo> media = (mediaInfo == null) ? List.of() : List.copyOf(mediaInfo); + return new TimetableDetailResponse( ... - mediaInfo + mediaAlso applies to: 31-33, 42-44
47-53: MediaInfo 확장성 고려(타입/순번 등 메타 포함 가능성)향후 동영상/썸네일 등 미디어 타입 구분이 필요할 수 있습니다. 최소한 type(ENUM) 필드 도입을 옵션으로 검토해 주세요. 현재 요구만 충족한다면 현 상태 유지 가능.
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/repository/TimetableBlockMediaRepository.java (1)
12-13: 정렬을 DB로 위임해 I/O 줄이기시퀀스 정렬이 필요하다면 저장소 단계에서 정렬해 전송량/정렬 비용을 줄이세요.
적용 diff:
- List<TimetableBlockMediaEntity> findAllByTimetableBlockId(final long timetableBlockId); + List<TimetableBlockMediaEntity> findAllByTimetableBlockIdOrderBySequenceAsc(final long timetableBlockId);인덱스 권장: (timetable_block_id, sequence).
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/TimetableBlockMedia.java (1)
16-23: fromEntity null 방어(특히 mediaUrl)DB 제약이 느슨할 경우 null이 전파될 수 있습니다. 최소 mediaUrl은 requireNonNull로 방어하거나, 도메인에서 빈 문자열로 정규화하세요.
예시:
- return new TimetableBlockMedia( + return new TimetableBlockMedia( timetableBlockMediaEntity.getTimetableBlockMediaId(), timetableBlockMediaEntity.getTimetableBlockId(), timetableBlockMediaEntity.getSequence(), - timetableBlockMediaEntity.getMediaUrl() + java.util.Objects.requireNonNull(timetableBlockMediaEntity.getMediaUrl(), "mediaUrl") );추가로, 해당 도메인은 변경 불가능 데이터 홀더 성격이 강하므로 Java record로의 전환도 고려 가능합니다.
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/component/TimetableBlockMediaRetriever.java (1)
15-17: 읽기 전용 트랜잭션 + DB 정렬 메서드 사용데이터 조회 메서드는 readOnly 트랜잭션이 적합합니다. 또한 저장소에서 정렬하도록 바꾸면 서비스 단 정렬이 불필요합니다.
적용 diff:
+import org.springframework.transaction.annotation.Transactional; ... - public List<TimetableBlockMedia> getAllTimetableBlockMediaByBlockId(final long blockId) { - return mediaRepository.findAllByTimetableBlockId(blockId).stream().map(TimetableBlockMedia::fromEntity).toList(); - } + @Transactional(readOnly = true) + public List<TimetableBlockMedia> getAllTimetableBlockMediaByBlockId(final long blockId) { + return mediaRepository + .findAllByTimetableBlockIdOrderBySequenceAsc(blockId) + .stream() + .map(TimetableBlockMedia::fromEntity) + .toList(); + }src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/service/TimetableService.java (3)
114-116: 좋아요 여부 조회 분기 단순화 가능가독성을 위해 early return만 남겨 간결화 가능. 성능 영향은 경미.
적용 diff:
- if (userId == null) { - return false; - } else { - return timetableUserLikeRetriever.isExistUserLikeByUserIdAndBlockId(userId, timetableBlockId); - } + if (userId == null) return false; + return timetableUserLikeRetriever.isExistUserLikeByUserIdAndBlockId(userId, timetableBlockId);
117-128: DTO 필드 네이밍 불일치 재확인여기서는 blockInfoRedirectUrl을 전달하지만, DTO 필드명은 blockInfoUrl입니다. 위 DTO 코멘트와 함께 통일하거나, @JsonProperty로 API 키를 유지하세요. 클라이언트 합의 필요.
139-144: 정렬 중복 제거 고려저장소에서 sequence ASC로 가져오면 서비스 단 정렬은 불필요합니다. 다만 안전을 위해 유지할 수도 있습니다. 정렬 키 충돌 시 보조 키(id)로 안정성 강화도 고려.
예시:
- return timetableBlockMediaList.stream() - .sorted(Comparator.comparingInt(TimetableBlockMedia::getSequence)) + return timetableBlockMediaList.stream() + .sorted(Comparator.comparingInt(TimetableBlockMedia::getSequence) + .thenComparing(TimetableBlockMedia::getTimetableBlockMediaId, Comparator.nullsLast(Comparator.naturalOrder()))) .map(media -> TimetableDetailResponse.MediaInfo.of(media.getMediaUrl())) .toList();src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/entity/TimetableBlockMediaEntity.java (8)
9-9: 조회 패턴 대비 인덱스 및 (block_id, sequence) 유니크 제약 추가 제안블록 ID로 자주 조회하고 시퀀스로 정렬한다면 성능/데이터 무결성에 이점이 큽니다.
마이그레이션 동반 필요.
-@Table(name = "event_timetable_block_media") +@Table( + name = "event_timetable_block_media", + indexes = { + @Index(name = "idx_event_timetable_block_media_block_id", columnList = "timetable_block_id") + }, + uniqueConstraints = { + @UniqueConstraint( + name = "uk_event_timetable_block_media_block_seq", + columnNames = {"timetable_block_id", "sequence"} + ) + } +)
25-27: 컬럼명 케이스 일관성(mediaUrl → media_url) 및 URL 길이/공백 제약스네이크 케이스로 통일하고 2048자로 늘려 실운영 URL(특히 CDN/쿼리스트링) 손실을 방지하세요.
DB 컬럼 리네임/길이 변경 마이그레이션 필요.
- @Column(name = "mediaUrl", nullable = false) - private String mediaUrl; + @Size(max = 2048) + @NotBlank + @Column(name = "media_url", nullable = false, length = 2048) + private String mediaUrl;
22-23: sequence 음수 방지 제약 추가도메인상 0 이상이 자연스러우면 명시적으로 막아두는 편이 안전합니다.
- @Column(name = "sequence", nullable = false) - private int sequence; + @Min(0) + @Column(name = "sequence", nullable = false) + private int sequence;
19-21: FK 변경 불가 시 표현 강화(updatable = false)생성 후 블록 참조를 변경하지 않는다면 업데이트 금지를 명시하세요.
- @Column(name = "timetable_block_id", nullable = false) + @Column(name = "timetable_block_id", nullable = false, updatable = false) private long timetableBlockId;
28-33: 생성 편의 제공(정적 팩토리)현재 생성자가 private이라 애플리케이션 코드에서 신규 영속화를 할 수 없습니다. 읽기 전용 의도면 괜찮지만, 생성 필요 시 정적 팩토리를 노출하세요.
private TimetableBlockMediaEntity(long timetableBlockId, int sequence, String mediaUrl) { this.timetableBlockId = timetableBlockId; this.sequence = sequence; this.mediaUrl = mediaUrl; } + + public static TimetableBlockMediaEntity create(long timetableBlockId, int sequence, String mediaUrl) { + return new TimetableBlockMediaEntity(timetableBlockId, sequence, mediaUrl); + } }
19-21: ID 필드 유지 vs. 연관 매핑 검토성능/단순성을 위해 FK ID만 두는 설계는 합리적입니다. 다만 도메인 탐색(블록→미디어 역참조)이 잦다면
@ManyToOne(fetch = LAZY)매핑으로 일관성을 갖추는 방안도 고려하세요. 현재 요구범위를 벗어나므로 선택 사항입니다.
14-18: equals/hashCode 정책 검토엔티티 컬렉션 사용 시 식별자 기반 equals/hashCode가 없으면 미묘한 버그가 날 수 있습니다. ID 생성 전 비교가 필요 없다면 ID 기반 구현을 추가하는 것을 권장합니다.
8-13: 읽기 전용이라면 @immutable 고려쓰기 경로가 전혀 없다면 Hibernate의
@Immutable로 의도를 명확히 할 수 있습니다. 도입 시 변경 불가 보장을 얻습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/TimetableBlock.java(0 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/entity/TimetableBlockEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/component/TimetableBlockMediaRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/TimetableBlockMedia.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/entity/TimetableBlockMediaEntity.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/repository/TimetableBlockMediaRepository.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/dto/TimetableDetailResponse.java(2 hunks)src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/service/TimetableService.java(5 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/TimetableBlock.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/entity/TimetableBlockMediaEntity.java (1)
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/TimetableBlockMedia.java (1)
Getter(8-24)
🔇 Additional comments (5)
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/entity/TimetableBlockEntity.java (1)
74-84: 시그니처 변경 반영 확인 — LGTM (단, 추가 검증 필요)
- TimetableBlockEntity의 팩토리/생성자에서 imageUrl 제거가 반영됨. (파일: src/main/java/com/permitseoul/permitserver/domain/eventtimetable/block/core/domain/entity/TimetableBlockEntity.java — create(...) → new TimetableBlockEntity(..., blockInfoRedirectUrl)).
- 레거시 imageUrl 잔존 확인: 이벤트 이미지 도메인에서 imageUrl / image_url 사용 다수 발견(예: src/main/java/com/permitseoul/permitserver/domain/eventimage/core/domain/EventImage.java, src/main/java/com/permitseoul/permitserver/domain/eventimage/core/domain/entity/EventImageEntity.java, 관련 서비스 AdminEventService.java, EventService.java). 이벤트 전용 이미지 테이블로 분리된 의도인지 확인할 것.
- 네이밍 혼재 발견: TimetableDetailResponse에
blockInfoUrl선언과blockInfoRedirectUrl사용이 혼재함(파일: src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/dto/TimetableDetailResponse.java). DTO 필드명 통일 또는 매핑 보장 필요.src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/dto/TimetableDetailResponse.java (1)
16-19: 날짜 포맷/타임존 계약 확인@jsonformat("yyyy.MM.dd HH:mm")는 타임존 정보가 없습니다. 클라이언트가 KST 고정 가정인지, 또는 서버 타임존 의존인지 확인 바랍니다. ISO-8601(예: yyyy-MM-dd'T'HH:mm:ssXXX)로의 전환 여부도 검토해 주세요.
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/timetable/api/service/TimetableService.java (1)
100-104: 미디어 조회 타이밍 적절함(LGTM)Block 식별 후 미디어 일괄 조회 흐름은 자연스럽습니다.
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/entity/TimetableBlockMediaEntity.java (2)
8-13: 엔티티 기본 구성은 적절합니다JPA 필드 접근, 보호 수준의 기본 생성자, IDENTITY 전략 사용 모두 무난합니다.
1-33: 정렬 보장 및 마이그레이션 존재 여부 확인 필요
- 정렬 미보장: Repository 메서드가 ORDER BY를 명시하지 않습니다 — src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/repository/TimetableBlockMediaRepository.java (findAllByTimetableBlockId) 및 호출부 src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/component/TimetableBlockMediaRetriever.java:16. 권장: findAllByTimetableBlockIdOrderBySequenceAsc(...) 사용 또는 Sort/JPQL로 ORDER BY 적용.
- 마이그레이션 없음: 레포에서 .sql 또는 db/migration 파일이 발견되지 않았습니다. 엔티티(src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/entity/TimetableBlockMediaEntity.java, @column(name="mediaUrl"))와 실제 DB 스키마(event_timetable_block_media 테이블, 컬럼명(mediaUrl vs media_url)/인덱스/유니크) 일치 여부를 확인하고 필요한 DDL 마이그레이션을 추가하세요.
| import jakarta.persistence.*; | ||
| import lombok.AccessLevel; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
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.
🛠️ Refactor suggestion
Bean Validation 도입(import)으로 제약 명시
URL 길이·공백, 시퀀스 음수 방지를 애노테이션으로 명시합시다.
import jakarta.persistence.*;
+import jakarta.validation.constraints.Min;
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Size;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import jakarta.persistence.*; | |
| import lombok.AccessLevel; | |
| import lombok.Getter; | |
| import lombok.NoArgsConstructor; | |
| import jakarta.persistence.*; | |
| import jakarta.validation.constraints.Min; | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.Size; | |
| import lombok.AccessLevel; | |
| import lombok.Getter; | |
| import lombok.NoArgsConstructor; |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/eventtimetable/blockmedia/domain/entity/TimetableBlockMediaEntity.java
around lines 3-7, add the Jakarta Bean Validation imports and annotate the
entity fields: import jakarta.validation.constraints.NotBlank, Size, NotNull,
and PositiveOrZero (or Min(0)); annotate the URL/string field with @NotBlank and
@Size(max=2048) (and keep @Column(length=2048) if present), annotate any
sequence/index numeric field with @NotNull and @PositiveOrZero (or @Min(0)), and
ensure these imports are added at the top of the file so the constraints are
enforced.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항