Skip to content
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

feat: ShowtimeResponse 객체와 직렬화 Config 추가 #6

Merged

Conversation

gominnam
Copy link
Collaborator

📌 변경 사항

  • 직렬화 하기위한 JacksonConfig 추가
  • ShowtimeResponse DTO 추가

🤔 고민한 점

  • 서로 연관된 테이블끼리 Response를 할때 내부에 어떻게 해야 좋을지(EventResponse에서 List 필드)
  • GET /events/{id} 를 요청하였을 때 ShowtimeResponse 내부에 연관 엔티티 객체로 인해 문제 발생(프록시 객체를 직렬화하면서)을 JacksonConfig를 추가하여 해결했지만 옳바른 방법인지 아니면 더 좋은방법이 없는지 아직도 고민중입니다.

- 직렬화 하기위한 JacksonConfig 추가
- ShowtimeResponse DTO 추가
@gominnam gominnam requested a review from f-lab-troy February 13, 2025 10:48
import org.springframework.context.annotation.Configuration;

@Configuration
public class JacksonConfig {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 class없이도 동작하도록 수정

@@ -29,6 +33,7 @@ public static EventResponse of(Event event) {
.description(event.getDescription())
.durationMinutes(event.getDurationMinutes())
.status(event.getStatus().getStatus())
.showtimes(event.getShowtimes() == null ? null : ShowtimeResponse.ofList(event.getShowtimes()))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null check 없애는 방법 고려

@AllArgsConstructor
@JsonIgnoreProperties({"hibernateLazyInitializer", "handler"})
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public class ShowtimeResponse {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Response vs DTO 고려

@gominnam gominnam merged commit fd0fa2c into feature/3-ticket-api-event Feb 14, 2025
1 check passed
@gominnam gominnam deleted the review/3-ticket-api-serialization branch February 14, 2025 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants