[1주차] 멀티 모듈 설계 및 요구사항 구현 by sina-log · Pull Request #20 · hanghae-skillup/redis_1st

sina-log · 2025-01-11T07:13:44Z

제목(title)

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

작업 내용

Doamin

Movie
- Long id PK
- String title
- String grade
- String genre
- Integer runningTime
- Date releaseDate
- String thumbnailUrl
Theater
- Long id PK
- String name
Schedule
- Long id PK
- DateTime startTime
- DateTime endTime

멀티모듈 구성

api : 외부 통신 레이어
application : 서비스 레이어
domain : 도메인 레이어
infra : 인프라 레이어

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

멀티모듈 구성에 의존성 오류에서 좀 애를 먹었습니다.

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

개인적인 시간이 없는 이슈가 가능 문제 였습니다. 다음주 부턴 시간확보를 더 많이 해보겠습니다.

리뷰 포인트

도메인 분리가 잘 되었는지 궁금 합니다.
도메인들이 JPA 연관관계를 잘 맺었는지랑 꼭 맺어야 하는지 궁금합니다

기타 질문

JPA 연관관계 없이 그냥 필요할 때마다 조회를 따로하는건 안되는지 궁금합니다.

BAEKJungHo · 2025-01-11T08:52:40Z

@reversalSpring 안녕하세요 준형님 ~ 열심히 참여 해주셔서 감사합니다!

1주차 리뷰 진행하도록 하겠습니다 :)

BAEKJungHo · 2025-01-11T08:54:29Z

movie-api/src/main/java/com/movie/api/ApiApplication.java

+    "com.movie.api",
+    "com.movie.application",
+    "com.movie.infra",
+    "com.movie.domain"


이런 부분들은 asterisk(*) 로 한줄로 처리가 가능할 것 같아요 :)

BAEKJungHo · 2025-01-11T08:54:55Z

movie-api/src/main/java/com/movie/api/HelloController.java

+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+public class HelloController {


불필요한 컨트롤러는 제거 부탁드립니다 :)

BAEKJungHo · 2025-01-11T08:56:33Z

movie-api/src/main/java/com/movie/api/controller/MovieController.java

+
+    private final MovieService movieService;
+
+    public MovieController(MovieService movieService) {


MovieService 필드가 final 로 되어있어서, @requiredargsconstructor 를 사용하면 생성자를 제거할 수 있을 것 같습니다 :)

BAEKJungHo · 2025-01-11T08:57:28Z

movie-application/build.gradle

+    implementation project(":movie-infra")
+
+    implementation 'org.springframework.boot:spring-boot-starter:3.1.0'
+    implementation 'org.springframework:spring-tx:5.3.10'


버전(e.g 3.1.0) 을 의존성 마다 직접 명시하는 방법대신, 의존성 버전들을 한곳에서 관리하는 방법도 있습니다 !

BAEKJungHo · 2025-01-11T08:58:59Z

movie-application/src/main/java/com/movie/application/dto/MovieResponseDto.java

+
+@Getter
+@Setter
+public class MovieResponseDto {


데이터 전송 객체에 자바의 record 를 사용해보는 것을 어떨까요 ?

BAEKJungHo · 2025-01-11T09:06:01Z

movie-domain/src/main/java/com/movie/domain/entity/Movie.java

+@Getter
+@Setter
+@Table(name = "movie")
+public class Movie extends BaseEntity {


@Setter 를 사용하게 되면 객체지향적인프로그래밍에서 벗어나 절차지향으로 퇴화된다는 특징이 있습니다.

캡슐화의 특성상, 내부 데이터는 접근 권한 제어를 통해 숨겨져야 하며, 외부에서는 클래스에서 제공하는 제한된 인터페이스를 통해서만 내부 데이터에 접근할 수 있어야 합니다 !

BAEKJungHo · 2025-01-11T09:07:07Z

movie-domain/src/main/java/com/movie/domain/entity/Schedule.java

+    private Theater theater;
+
+    private LocalDateTime startTime;
+    private LocalDateTime endTime;


타입이 LocalDateTime 인 경우 변수명도 startDateTime 등으로 해주는 것이, 네이밍과 타입간의 불일치를 해소해줄 것 같습니다 ! 저는 개인적으로 at 이라는 네이밍을 선호합니다.

BAEKJungHo · 2025-01-11T09:08:24Z

movie-domain/src/main/java/com/movie/domain/entity/Movie.java

+    private String createdBy;
+    private java.time.LocalDateTime createdAt;
+    private String updatedBy;
+    private java.time.LocalDateTime updatedAt;


BaseEntity 를 사용 중이셔서, 하위 클래스에 system properties 를 중복 정의할 필요는 없을 것 같아요 !

BAEKJungHo · 2025-01-11T09:10:31Z

movie-infra/src/main/java/com/movie/infra/repository/MovieJpaRepository.java

+import org.springframework.stereotype.Repository;
+
+@Repository
+public interface MovieJpaRepository extends JpaRepository<Movie, Long>, MovieRepository {


MovieRepository 인터페이스를 통해서 application 에서 infra 를 직접 참조하지 않도록 DIP 를 적용해주신 부분 훌륭합니다 👍🏿

BAEKJungHo · 2025-01-11T09:15:34Z

movie-infra/src/main/java/com/movie/infra/repository/ScheduleJpaRepository.java

+
+    @Override
+    @EntityGraph(attributePaths = {"movie", "theater"})
+    List<Schedule> findAllFetchMovieTheater();


EntityGraph 를 사용하는 경우 left outer join 이 발생하는 것으로 알고 있는데, left outer join 이 맞을지 고민해주시면 좋을 것 같아요 !

BAEKJungHo · 2025-01-11T09:16:27Z

settings.gradle

+include 'movie-domain'
+include 'movie-infra'
+include 'movie-application'
+include 'movie-api'


movie prefix 는 생략 가능해보입니다 ! 있어도 무방하지만, 저는 생략 하더라도 의미전달이 충분히 된다면
생략하는 것을 선호합니다 :)

BAEKJungHo · 2025-01-11T09:20:48Z

movie-application/src/main/java/com/movie/application/service/MovieService.java

+
+        return movieList.stream()
+            .map(movie -> {
+                List<ScheduleInfo> scheduleInfos = scheduleList.stream()


2개의 쿼리를 통해서 Nested Stream 을 사용 중인데, scheduleList 를 통해서도 Movie 를 가져올 수 있을 것 같습니다. movieList 를 얻기 위한 로직이 꼭 필요한지 고민해주시면 좋을 것 같아요 :)

BAEKJungHo · 2025-01-11T09:27:40Z

movie-application/src/main/java/com/movie/application/service/MovieService.java

+
+@Service
+@Transactional(readOnly = true)
+public class MovieService {


MovieService 가 application 모듈에 있는 것을 보아, 비지니스로직을 오케스트레이션 하는 역할의 애플리케이션 서비스 로 설계를 해주신 것으로 생각됩니다.

만약, 현재 구조에서 도메인 로직을 담당하는 도메인 서비스가 필요한 경우에는 어떤 네이밍을 사용할지, 현재 작성된 서비스의 로직이 애플리케이션 서비스에 있는게 맞을지 등에 대한 추가적인 고민이 있으면 좋을 것 같아요 !

사실 현재 1주차 요구사항만 봤을때는 비지니스 로직을 오케이스트레이션 할 만한 요구 사항이 없어서 애플리케이션 서비스가 필요 없을 것 같습니다!

MovieService 가 도메인 서비스였다면 Domain 모듈로 이동을 시키실 것 같아요. 이때 MovieResponseDto 도 같이 Domain 모듈로 들어가게 될텐데 이 경우 domain <-> application 간의 순환 참조가 발생할 수 있습니다.

이 부분을 어떻게 해결할 수 있을지 고민해주시면 좋을 것 같습니다 !

BAEKJungHo · 2025-01-11T09:30:03Z

movie-application/src/main/java/com/movie/application/service/MovieService.java

+                dto.setRunningTime(movie.getRunningTime() != null ? movie.getRunningTime() : 0);
+                dto.setGenre(movie.getGenre());
+                dto.setSchedules(scheduleInfos);
+                return dto;


getNowShowingMovies 메서드는 되게 심플한 메서드인데, DTO 로 변환하는 로직 때문에 코드가 길어져서 가독성이 떨어진다는 단점이 있습니다 ! 가독성 개선을 위한 추가적인 고민을 해주시면 좋을 것 같습니다 :)

BAEKJungHo · 2025-01-11T09:33:34Z

README.md

+  - api : 외부 통신 레이어
+  - application : 서비스 레이어
+  - domain : 도메인 레이어
+  - infra : 인프라 레이어


Layered 형식으로 모듈을 잘 구성 해주셨네요 ! 다만, application 과 api 모듈이 별도로 구분되어야 하는지는 추가적인 고민이 필요해 보입니다.

api 모듈에는 Controller 가 존재하고, application 모듈에는 dto 가 존재하는데, 사실 이 둘은 같은 Layer 에 포함되어야 하는 성격이라고 생각됩니다. 즉, 계층을 많이 추가하는 경우 응집성 이 떨어질 수 있다는 단점이 있습니다.

BAEKJungHo · 2025-01-11T09:34:26Z

README.md

+  - Schedule
+    - Long id PK 
+    - DateTime startTime 
+    - DateTime endTime


좌석 테이블이 미반영된 것 같습니다.

BAEKJungHo · 2025-01-11T09:35:17Z

movie-api/src/main/java/com/movie/api/controller/MovieController.java

+        this.movieService = movieService;
+    }
+
+    @GetMapping("/api/v1/movies/now-showing")


/api/v1 실무에서 자주 사용되는 패턴을 잘 적용 해주셨네요 👍🏿

BAEKJungHo · 2025-01-11T09:36:00Z

movie-application/src/main/java/com/movie/application/service/MovieService.java

+    }
+
+    public List<MovieResponseDto> getNowShowingMovies() {
+


불필요한 줄바꿈이 있습니다.

BAEKJungHo · 2025-01-11T09:39:04Z

movie-api/build.gradle

+    implementation 'org.springframework.boot:spring-boot-starter'
+    implementation 'org.springframework.boot:spring-boot-starter-web'
+    implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
+    implementation 'com.h2database:h2'


docker-compose 를 누락하신 것 같아요 ! 2주차에서는 redis 도 사용해야하니
2주차에서 docker-compose 설정을 반영해주시면 좋을 것 같습니다 :)

BAEKJungHo · 2025-01-11T09:54:12Z

@reversalSpring

좋았던 점

JPA Repository 를 infra 에 정의하고, application 모듈에서 infra 를 참조하지 않도록 인터페이스 설계 사상 을 잘 적용해주셨습니다.
Layered 형태로 모듈을 잘 구성해주셨습니다.

아쉬웠던 점

확보하신 시간이 부족하셔서 몇몇 요구사항이 누락, 로직 작성에 대한 고민 할 시간이 부족한 모습이 느껴졌습니다.

리뷰 포인트 및 추가 질문에 대한 답변

도메인 분리가 잘 되었는지 궁금 합니다.

MovieService 의 위치가 application 모듈이 맞을지 고민이 필요합니다.
Movie, Theater, Schedule 엔티티에 대한 설계는 필요한 컬럼들을 잘 정의해주셨습니다.
Domain 모듈내에서 패키지를 도메인 단위가 아닌 기능(entity, repository) 단위로 구분해주셨는데, 도메인 단위 패키지 형식이 추후에 코드 구조를 전반적으로 파악하는데 도움이 됩니다.

도메인들이 JPA 연관관계를 잘 맺었는지랑 꼭 맺어야 하는지 궁금합니다

Schedule 에 ManyToOne 을 잘 사용해주셨습니다. 다만 N+1 문제를 해결하기 위해서 default_batch_fetch_size 옵션도 고려되면 좋을 것 같습니다.
꼭 맺지 않아도 됩니다. 간접 참조 방식(e.g 객체가 아닌 Id 를 참조하는 방식)을 사용해도 됩니다. 다만 간접 참조 방식을 사용하는 경우 JPA 의 dirty-checking, entity graph 탐색 등의 기능을 포기하는게 되기 때문에 JPA 대신 다른 기술을 고려할 수 있습니다.

꼭 필요한 연관관계를 맺는 방법 도 좋은 습관이라고 생각됩니다.

JPA 연관관계 없이 그냥 필요할 때마다 조회를 따로하는건 안되는지 궁금합니다.

필요할 때마다 조회하셔도 됩니다. 대신 필요할 때마다 조회를 잘 하기 위해서는 쿼리를 직접 작성해야할 것이며 @Query, QueryDsl 등의 도구를 충분히 활용해주셔야합니다. (또한 내 코드가 너무 절차지향적으로 가고 있는 것은 아닌지 중간중간 확인도 필요합니다.)

…epositories

…tion

…이동, RateLimitInterceptor를 API 모듈로 이동, 불필요한 테스트 파일 제거 및 테스트 코드 개선

LIFIC-JAMES added 3 commits January 11, 2025 15:01

Init module

156c443

Implementing movie search code

ebc22ed

Add readme

80eb082

github-actions bot added the 1주차 label Jan 11, 2025

BAEKJungHo reviewed Jan 11, 2025

View reviewed changes

phyeran changed the base branch from main to reversalSpring January 17, 2025 02:06

spartacontents deleted the branch hanghae-skillup:main January 17, 2025 03:20

spartacontents closed this Jan 17, 2025

spartacontents reopened this Jan 17, 2025

refactor: update project structure and docker configuration

63da73e

sina-log added 30 commits January 27, 2025 14:53

feat: 예외 처리 개선

177aa5e

feat: API 응답 형식 표준화 및 Swagger 문서화

abe1a48

feat: 성능 최적화 (N+1 문제 해결 및 캐시 적용)

d671748

feat: 로깅 및 모니터링 구현

1621a84

docs: README 업데이트 - 3주차 요구사항 추가

740799f

feat: Implement IP-based RateLimit for Movie API

4d48bb8

feat: Add time-based RateLimit for Reservation API

8b307a9

feat: Add unified API response format with error codes

bba63d4

test: Add unit tests for RateLimitService

cf8e431

test: Add unit tests for ReservationRateLimitService

5823b43

test: Add integration tests for Movie and Reservation API RateLimit

5d1dcc3

test: Add test coverage for RateLimit and ApiResponse

b443b2c

fix: Movie 엔티티 컬럼 매핑 수정 및 테스트 설정 개선

148fec2

feat: Configure Docker environment with MySQL, Redis and fix schema.sql

97bfd0f

fix: Change Seat entity rowNumber field type to String

efe440c

feat: Add standardized API response format and exception handling

34ee5ce

feat: Implement single-server rate limiting using Google Guava

a9dd27d

feat: Implement distributed rate limiting using Redis and Redisson

9915138

chore: Configure test environment with JaCoCo and test fixtures

a43c1ba

test: Add comprehensive test coverage for controllers, services and r…

5dec7cc

…epositories

refactor: Improve rate limiting implementation with interface abstrac…

62916d2

…tion

refactor: Improve test code structure with base integration test class

a809cf3

refactor: Improve exception handling with business exception hierarchy

f2953fd

test: Add unit tests for rate limiting components

d03b26f

refactor: Improve module structure with common module

c8a58d7

refactor: Move rate limiting tests to common module

db929af

test: Temporarily disable ReservationRepositoryTest

5762cec

feat: Add RateLimit implementation and consistent response format

6da3799

refactor: RateLimit 관련 모듈 구조 개선 - RateLimitService 인터페이스를 common 모듈로 …

397e132

…이동, RateLimitInterceptor를 API 모듈로 이동, 불필요한 테스트 파일 제거 및 테스트 코드 개선

docs: Rate Limit 구현 내용과 JaCoCo 테스트 커버리지 리포트 추가

233eaa1


		private final MovieService movieService;

		public MovieController(MovieService movieService) {

Conversation

sina-log commented Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

제목(title)

작업 내용

Doamin

멀티모듈 구성

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

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

리뷰 포인트

기타 질문

Uh oh!

BAEKJungHo commented Jan 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BAEKJungHo Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BAEKJungHo Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BAEKJungHo commented Jan 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

좋았던 점

아쉬웠던 점

리뷰 포인트 및 추가 질문에 대한 답변

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sina-log commented Jan 11, 2025 •

edited

Loading

BAEKJungHo Jan 11, 2025 •

edited

Loading

BAEKJungHo Jan 11, 2025 •

edited

Loading

BAEKJungHo commented Jan 11, 2025 •

edited

Loading