Skip to content

[1주차] 멀티 모듈 설계 및 요구사항 구현#19

Open
somin-jeong wants to merge 61 commits intohanghae-skillup:mainfrom
somin-jeong:main
Open

[1주차] 멀티 모듈 설계 및 요구사항 구현#19
somin-jeong wants to merge 61 commits intohanghae-skillup:mainfrom
somin-jeong:main

Conversation

@somin-jeong
Copy link

제목(title)

[1주차] 멀티 모듈 설계 및 상영 중인 영화 조회 API 구현


작업 내용

이번 PR에서 진행된 주요 변경 사항을 기술해 주세요.
코드 구조, 핵심 로직 등에 대해 설명해 주시면 좋습니다. (이미지 첨부 가능)
ex: ConcurrentOrderService에 동시 주문 요청 처리 기능 추가

  • 레이어드 아키텍처 적용하여 멀티 모듈 설계
  • 데이터베이스 ERD 설계
  • 테이블과 엔티티 관계 매핑
  • 상영 중인 영화 조회 API 구현

발생했던 문제와 해결 과정을 남겨 주세요.

ex) 문제 1 - 다수의 사용자가 동시에 같은 리소스를 업데이트할 때 재고 수량이 음수로 내려가는 데이터 불일치 문제 발생
해결 방법 1 - Redis SET 명령어에 NX(Not Exists)와 PX(Expire Time) 옵션을 활용해 락을 설정했습니다. 이유는 ~

  • 문제1 - 스프링부트와 데이터베이스 연결이 안되는 문제 발생
  • 해결 방법1 - application.yml 파일을 루트 모듈에서 api 모듈로 옮겼습니다. 스프링부트를 실행하는 모듈에 application.yml 파일이 있어야 하기 때문입니다.
  • 해결 방법2 - 데이터베이스 URL 설정할 때 도커 컨테이너 IP로 설정했던 것을 localhost로 변경했습니다.

이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.

과제를 해결하며 특히 어려웠던 점이나 고민되었던 지점이 있다면 남겨주세요.

  • 몇 개의 모듈로 나눠야 할 지, 어떻게하면 각 모듈들의 역할과 책임이 명확하도록 나눌 수 있을지 고민되었습니다.

리뷰 포인트

리뷰어가 특히 의견을 주었으면 하는 부분이 있다면 작성해 주세요.

ex) Redis 락 설정 부분의 타임아웃 값이 적절한지 의견을 여쭙고 싶습니다.

  • 저는 api, domain, infra, core로 나눴는데 이러한 방식으로 모듈을 나누는 것이 맞는지 궁금합니다. 그리고 실무에서는 어떤 방식으로 모듈을 나누는지도 궁금합니다

기타 질문

추가로 질문하고 싶은 내용이 있다면 남겨주세요.

ex) 테스트 환경에서 동시성 테스트를 수행하였고, 모든 케이스를 통과했습니다. 추가할 테스트 시나리오가 있을까요?

@pyuseon
Copy link
Contributor

pyuseon commented Jan 11, 2025

안녕하세요, 소민님! 이번 주차 리뷰를 맡게 된 박유선입니다. 😊
1주차 구현 열심히 하신게 느껴집니다. 💪
소민님께서 궁금해하신 부분을 중심으로 꼼꼼히 리뷰해드리도록 하겠습니다.

README.md Outdated
domain 모듈에서 관리하는 인터페이스를 구현한 클래스를 관리합니다.

### module-core
다른 모듈들에서 공통으로 사용되는 클래스들을 모아두기 위한 모듈입니다.
Copy link
Contributor

Choose a reason for hiding this comment

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

이 모듈의 이름을 common으로 변경하는 것도 고려해보시면 좋을 것 같습니다.
현재 이름인 core는 비즈니스 로직 중심의 역할로 오해될 가능성이 있으니, 공통 모듈임을 더 명확히 나타내는 common이라는 이름이 적합해 보입니다.

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
@Column(name = "screen_schedule_id", nullable = false)
private Long id;
Copy link
Contributor

@pyuseon pyuseon Jan 11, 2025

Choose a reason for hiding this comment

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

현재 ddl-auto: create로 설정되어 있어,
데이터베이스에서 생성된 PK 필드 설정을 직접 확인하기 어려운 상황이네요.
다만, PK 필드는 음수가 필요 없으므로, 데이터베이스에서 INT UNSIGNED나 BIGINT UNSIGNED로 설정되었는지 확인해 보시는 것이 좋을 것 같습니다. ID에 실수로 음수값이 들어가는 상황을 막을 수 있어 더 안전한 설계가 될 것 같습니다. 😄

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "screen_room_id")
private ScreenRoom screenRoom;
}
Copy link
Contributor

@pyuseon pyuseon Jan 11, 2025

Choose a reason for hiding this comment

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

현재 @manytoone@joincolumn을 통해 외래키(FK)가 설정된 상태로 보입니다.
실무에서는 FK로 인한 성능 저하, 스키마 변경의 어려움, 데이터 이관 및 클렌징의 복잡성 등을 고려하여 FK를 설정하지 않는 경우가 많습니다.
특히, 현재 진행중인 프로젝트는 데이터의 무결성이 비즈니스적으로 크게 중요하지 않은 서비스이므로 FK를 생략하는 것이 유연한 설계와 유지보수에 더 유리 할 것 같습니다. ⭐
FK 설정을 제거하고 애플리케이션 레벨에서 데이터의 무결성을 관리하는 방식은 어떨까요? 🤔

public class ScreenScheduleDto {
private LocalDateTime startTime;
private LocalDateTime endTime;
}
Copy link
Contributor

@pyuseon pyuseon Jan 11, 2025

Choose a reason for hiding this comment

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

현재 DTO가 도메인 모듈에 위치하고 있네요. API 모듈로 이동시키는 것이 더 적합할 것 같습니다.
DTO는 주로 클라이언트와 데이터 교환을 위해 사용되며, 도메인 모델과 분리하는 것이 유지보수와 확장성 측면에서 유리합니다.
만약 도메인 모듈에서 DTO를 직접 사용하게 되면, API 요구사항 변경 시 도메인 모듈에도 영향을 미칠 수 있습니다.
예를 들어, DTO에 필드가 하나 추가될 경우, API 모듈에 있다면 해당 모듈만 수정하면 되지만, 도메인 모듈에 위치한다면 두 모듈을 모두 수정해야 하는 번거로움이 생길 수 있습니다. 😢
따라서, DTO는 API 모듈에 위치시켜 모듈 간 책임을 분리하는 것이 더 적절해 보입니다! 😊

Copy link
Contributor

@pyuseon pyuseon Jan 11, 2025

Choose a reason for hiding this comment

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

현재 인프라 모듈과 도메인 모듈을 분리하신 것으로 보이는데요, 인프라 모듈에 추가적인 구현이 필요해 보입니다. 🕵️‍♀️
보통 인프라 모듈은 외부 시스템과의 통신을 처리하는 역할을 담당하며, 예를 들면 JPA 엔티티나 레포지토리 구현체 등이 포함될 수 있습니다.

또한, 인프라 모듈을 별도로 분리하면 데이터 접근 기술(MyBatis → JPA 등) 을 변경해야 할 때, 도메인 모듈에 영향을 주지 않고 구현체만 교체할 수 있어 더 유연한 설계가 가능합니다.
현재 도메인 모듈에 레포지토리가 포함된 것으로 보이는데요, 이 중 JPA와 관련된 부분(Entity, JPA Repository 구현체 등)을 인프라 모듈로 이동하시면 더 적합할 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

만약 인프라 모듈과 도메인 모듈을 분리하여 인프라 모듈에 JPA 엔티티와 JPA 레포지토리를 구현한다면 도메인 모듈에는 어떤 객체가 들어가야 하는지 궁금합니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

위처럼 피드백 드린 이유는 현재 인프라 모듈이 비어 있어서 이 부분을 어떤 방식으로 채워 나갈지에 대한 방안 이었습니다. 그 중에 하나가 jpa 레파지토리는 infra에 레포지토리 인터페이스는 domain에 넣는 방법이 있을 것 같습니다. 엔티티와 도메인을 분리해서 엔티티는 infra에 도메인은 도메인으로 분리해서 설계하는 방법도 있을 것 같네요. 🔍
이렇게 하면 domain 모듈은 비즈니스 로직에 집중하고, infra 모듈은 데이터베이스와의 상호작용을 전담하게 됩니다.
물론 도메인 모듈에서 DB와의 상호작용까지 처리할 수도 있지만, 그렇게 되면 infra 모듈의 존재 의의가 약해질 수 있습니다
현재 인프라 모듈의 역할이 희미하다고 생각해서 피드백을 드리게 되었는데요. 인프라 모듈에 어떤 역할과 책임을 주어야 할지 생각해 보는것도 좋을 것 같습니다. ⭐
다만, 이 방식이 정답은 아닙니다. 멀티 모듈에서 모듈을 나누는 방식은 프로젝트의 요구사항에 따라 다양한 선택지가 있으며, 각 방식에는 트레이드오프가 존재합니다.

구현하다 보니까 상영관 테이블이 없어도 될 것 같다는 생각이 들었습니다.
현재 상영관에 관한 요구사항이 없기 때문에 상영관 테이블에는 이름 속성 하나만 있습니다.
하나의 속성만 갖고, 상영시간표 테이블과 비슷한 목적으로 만들어진 테이블을 하나 더 만드는 것이 성능 측면에서 안좋을 것이라고 생각하였습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

요구사항에 대해 많이 고민하신 부분이 보이네요. 고생 많으셨습니다. 👍
상영관 테이블은 추후 좌석 정보와 예매를 고려했을 때 유지하시는 것이 좋을 것 같습니다. 또한, 좌석 정보와 관련된 테이블을 추가로 설계하시면 상영관과 연결지어서 사용하실 수 있을 것 같습니다.

private final MovieService movieService;

@GetMapping("/movies/playing")
public ResponseEntity<List<PlayingMoviesResponseDto>> getPlayingMovies() {
Copy link
Contributor

Choose a reason for hiding this comment

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

API 구성이 상영 중인 항목만 조회하도록 깔끔하게 설계되었네요. 👍
개인적으로는 API에 버전을 추가하는 것도 추천 드려요!
버전을 명시적으로 포함하면 외부 시스템과의 연동이나 시스템 업데이트 시 변경 사항을 관리하기가 훨씬 수월해지며, 유지보수 측면에서도 도움이 됩니다. 실제로 제가 실무에서 사용하는 방식이기도 합니다. 🔍
또한, 현재 /movies/playing이라는 URL은 자원과 동작을 잘 표현하고 있지만, RESTful 설계 관점에서는 상태나 동작을 쿼리 파라미터로 표현하는 방식(/movies?status=playing)도 고려해 볼 만 한 것 같습니다.
이 방식은 확장성에 도움이 될 수 있으니 필요에 따라 검토해 보셔도 좋을 것 같습니다. 😊

@pyuseon
Copy link
Contributor

pyuseon commented Jan 11, 2025

추가 코멘트 사항입니다! 📝

  1. Git 브랜치 관리

    • 현재 메인 브랜치에서 작업하신 것으로 보이는데요, 주차별로 별도의 브랜치를 생성하여 관리하시면 더 효율적으로 버전 관리를 하실 수 있을 것 같습니다.
      예를 들어, 전 주차의 브랜치를 머지하거나 필요한 내용을 가져오는 방식으로 작업하시면, 변경 이력을 명확히 추적하고 관리하기 쉬울 것 같아요. 😊
  2. 모듈 구조

    • 현재 네 개의 모듈로 나누신 구조도 좋지만, infra 모듈을 제거하고, ReadMe에 설명에따라 domain 모듈에서 비즈니스 로직과 데이터 조회를 함께 다루는 방식도 고려해 보시면 좋을 것 같습니다. infra모듈을 살리시고자 한다면 코드에 리뷰 드린 방식으로 jpa와 관련된 부분을 infra로 옮기시는 것이 좋을 것 같습니다.

@pyuseon
Copy link
Contributor

pyuseon commented Jan 12, 2025

좋았던점

  • ReadMe를 깔끔하게 작성해 주셔서 설계 내용을 한눈에 파악하기 좋았습니다.
  • 요구사항에 맞춰 API를 설계해 주셨고, 응답 값도 누락 없이 코드에 잘 반영해 주신 점이 인상적이었습니다.

아쉬운점

  • 현재 ddl: create 옵션을 사용하고 계신 것으로 보이는데, DB 컬럼 구조를 따로 설정해 두신 건지, 혹은 코드에서 처리하고 계신 건지 명확히 파악하기 어려웠습니다. 테이블 생성과 관련된 SQL문을 따로 작성해 주시면 리뷰 시 큰 도움이 될 것 같습니다.

추가 리뷰 포인트

  • 설계는 잘 이루어진 것으로 보이지만, 일부 모듈(infra)은 아직 구현 중인 것으로 보입니다. 해당 부분까지 구현해 주시면 모듈의 역할과 책임을 더욱 명확히 파악할 수 있을 것 같고, 이에 따라 보다 구체적인 리뷰를 드릴 수 있을 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants