-
Notifications
You must be signed in to change notification settings - Fork 55
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
[Spring Data JPA] 신혜빈 미션 제출합니다. #108
base: shin378378
Are you sure you want to change the base?
Conversation
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.
안녕하세요 혜빈님 ! 저도 원래 계시던 메인테이너와 리뷰어분들이 혜빈님 짱 멋있다는 이야기를 많이 해주셨어요. :D
과제도 바쁘셨을텐데 어려운 미션 하시느라 고생 많으셨어요!
클래스 위치에 대해
저도 클래스의 위치를 정할 때 Interceptor, Argument Resolver처럼
특정 도메인에 속하지 않고 전역적으로 적용되는 클래스들의 위치를 정하는 게 항상 어렵습니다.
정해진 정답도 없는 것 같구요!
이번 미션에서는 도메인 단위로 패키지가 분리되어 있으니
인증/인가와 관련된 클래스들을 auth 패키지에 둔 지금 구조가 좋은 것 같아요. :D
그런데 CookieUtil은 현재 인증에 필요한 token을 추출하는 메서드만 있어 auth 패키지 외에 다른 패키지에서 쓸 일이 없고, 같은 이유로 다른 패키지에서 사용해서도 안 되기 때문에
util 패키지에 분리되어 있는 것보다 auth 패키지에 소속되어 있는 것이 더 자연스럽지 않을까라는 생각이 들어요. 혜빈님의 의견은 어떠신지 궁금해요!
AdminInterceptor
클래스는 member라는 파일에 들어가야하는 클래스일까, 아님 util이라는 파일에 들어가야 하는 클래스일까
'유틸성 클래스' 라는 키워드를 통해서 먼저 유틸의 정의를 알아보셨음 좋겠어요! 그리고 AdminInterceptor는 유틸성 클래스의 조건을 만족하는 지 고민해 보신다면, 위 의문은 자연스럽게 해결되리라 생각해요.
어노테이션
어떤 점이 부족한지 먼저 알려주셔서 감사해요 혜빈님! :D
스프링, 그 중에서도 JPA는 다양한 기능을 제공하는 데에 어노테이션을 정말 적극적으로 활용하는 프레임워크에요. 그래서 이번 리뷰는 JPA에서 제공하는 다양한 어노테이션들을 알아보고 사용해보실 수 있는 방향으로 남겨보았어요.
Repository 의존성
현재 혜빈님의 도메인 구조는 Reservation, Theme, Time, Member, Waiting이라는 도메인이
모두 동일한 레벨의 독립된 도메인으로 취급되고 있습니다.
객체들이 필요에 따라 서로 연관관계를 맺기는 하지만, Time이 Reservation에 포함된다라는 식의 포함관계를 맺고있지는 않다는 뜻인데요,
이럴 경우에는 각 도메인 객체를 조회하기 위해서 각각의 Repository를 사용해야 하니,
혜빈님의 서비스가 Repository를 많이 들게 되는 것은 조금 못생겼으나 최선인 것으로 보입니다.
만약 Repository의 수를 줄이고 싶으시다면
엔티티들 사이의 관계를 잘 분석한 후, 연관관계보다 포함관계가 더 어울리는 엔티티들을 합치는 방법이 있습니다.
예를 들어 Time이 Reservation과 독립된 개념이 아니라 Reservation에 포함된 아이라는 판단을 하셨다면, Time 객체를 별도의 테이블로 분리하지 않고 Reservation의 컬럼으로 병합하는 것도 그 예시 중 하나가 될 수 있죠!
하지만 혜빈님의 앞선 미션에서 은우님과 나누셨던 대화처럼, 서로 다른 도메인을 별도 테이블(엔티티)로 분리하는 것과 병합하는 것 둘은 서로 상반된 장단점을 가지고 있습니다. 그러니 본인만의 기준에 맞게 고민해보시면 좋을 것 같습니다.
(1, '어드민', '2024-03-01', 3, 3); | ||
|
||
INSERT INTO reservation (name, date, time_id, theme_id) | ||
VALUES ('브라운', '2024-03-01', 1, 2); |
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.
이 SQL문을 보시면 브라운의 예약정보를 저장할 때 member_id가 없습니다.
예약 데이터인데 누가 예약을 했는지에 대한 정보가 없다면 불완전한 데이터라고 할 수 있어요.
하지만 어떤 이유로든 이런 식의 잘못된 데이터를 저장할 가능성이 있고,
잘못된 데이터가 저장되는 것을 예방하려면 테이블을 생성할 때 적절한 제약사항을 명시해야 합니다.
JPA를 사용하는 환경에서는 엔티티 클래스를 바탕으로 DDL문을 만들어주는데,
현재 reservation 테이블을 생성하는 DDL문은 아래와 같습니다.
(애플리케이션을 실행할 때 콘솔에 출력되는 로그를 통해 볼 수 있습니다 !)
create table reservation (
id bigint generated by default as identity,
member_id bigint,
theme_id bigint,
time_id bigint,
date varchar(255),
name varchar(255),
primary key (id)
)
데이터를 저장할 때 필요한 정보가 누락되지 않게 하려면
컬럼을 선언할 때 not null 제약사항을 명시해주면 돼요. 이렇게요 !
create table reservation (
id bigint generated by default as identity,
member_id bigint not null,
theme_id bigint not null,
time_id bigint not null,
date varchar(255) not null,
name varchar(255) not null,
primary key (id)
)
JPA에서 테이블을 생성할 때 이 not null 제약사항을 포함하게 하려면
Reservation 엔티티를 어떻게 수정해야 할까요?
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.
JPA테이블에 null값이 포함되지 않게 하기 위해서는
-
@NotNull
-
@Column(nullable = false)
(수정 중)
private Long id; | ||
|
||
private String name; | ||
private String email; | ||
private String password; | ||
private String role; |
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.
Member의 role에 들어가는 값으로는 'ADMIN', 'USER' 이렇게 두 가지가 있습니다.
이 두 값은 비즈니스에 뭔가 대대적인 개편이 있지 않으면 잘 변하지 않는, 상수와 같은 아이들이에요!
자바에서는 이러한 상수들의 집합을 enum 객체를 사용하여 편하게 관리할 수 있는데요,
이 role 필드를 enum으로 관리할 수 있다면 훨씬 관리하기 편할 것 같습니다.
JPA의 엔티티에서 필드의 타입으로 enum을 사용하려면 어떻게 해야 하고, 또 그 과정에서 어떤 것을 고려해야 할까요?
private Long id; | ||
|
||
@Column(name = "time_value") | ||
private String value; |
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.
자바에서는 '시간'을 관리하는 객체로 LocalDateTime, LocalDate, LocalTime 등의 클래스를 제공합니다.
문자열 대신 위 객체를 사용하면 시간과 관련해서 편리한 기능을 많이 사용할 수 있어 훨씬 좋은데요! :)
아래 두 유의사항에 신경쓰면서 ,
각 엔티티에서 시간을 나타내는 필드의 타입을 시간 관리 객체로 바꾸어주세요.
- http requestBody 시간 표시방법("2024-03-01", "10:00")이 변경되지 않게 주의해주세요!
- 데이터베이스에 저장되는 시간 표시방법("2024-03-01", "10:00")이 변경되지 않게 주의해주세요!
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
private Long id; | ||
|
||
private String date; |
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.
ditto, 시간 관리 객체로 바꾸어보세요! ;)
final List<Reservation> reservations = reservationRepository.findByMemberId(member.getId()); | ||
final List<WaitingWithRank> waitingWithRanks = waitingRepository.findWaitingsWithRankByMemberId(member.getId()); | ||
return MyReservationResponse.of(reservations, waitingWithRanks); | ||
} |
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.
reservationRepository.findByDateAndThemeIdAndTimeId(reservationRequest.getDate(), theme.getId(), time.getId()) | ||
.ifPresent(it -> { | ||
throw new IllegalArgumentException("이미 예약된 시간입니다."); | ||
}); | ||
} |
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.
예약 날짜, 테마, 예약 시간 이 세 정보가 동시에 일치하면 안된다는 제약조건을 만들어 주셨군요 ! :D
이와 동일한 제약조건을 데이터베이스 레벨에서도 걸어줄 수 있습니다.
date, time_id, theme_id를 하나로 묶어 유니크 제약을 만들어주는 거에요.
JPA의 엔티티에서 유니크 제약을 만들려면 어떻게 할 수 있을까요?
안녕하세요:) 여우 리뷰어님!! 처음 만나뵙게 되어 반갑습니다!!
워낙 실력이 뛰어나시다고 전해들어서 많이 기대가 되네요 : ) 잘 부탁드립니다☘️
<구현과정>
4단계 - Jpa전환
5단계 - 내 예약 목록 조회
6단계 - 예약 대기 기능
<궁금한 점>
auth파일과 util파일에 있는 클래스들(5개)의 위치가 적당한 지 한 번만 봐주실 수 있나요?
제가 처음 Interceptor와 Argument Resolver를 사용해 봤는데요. 특히 이들이 어디에 위치해 있어야 하는 지 너무 헷갈립니다.
어노테이션 활용 부족
스프링에 익숙해 지려고 노력하고 있지만 개인적으로 아직 어노테이션 활용능력이 아쉽다고 느껴집니다.
제가 작성한 코드에서 어노테이션을 적절히 활용할 수 있었던 부분이 있다면 피드백 부탁드립니다.
그리고 자주 사용하는 어노테이션들을 간단하게 언급주시면 제가 공부해보겠습니다!!
ReservationService, WaitingService의 레포지토리 의존성
현재 제 코드에서 ReservationService, WaitingService가 아래와 같이 많은 레포지토리를 가지고 있는데요.
현재 제 지식 수준에서는 저 레포지토리들을 다 들고와서 데이터를 꺼내쓰는 방법 밖에 생각이 안 납니다 ㅜㅠ
뭔가 제 생각엔 이게 비효율적인 방법 같은 데 막상 다른 대안이 잘 생각나지 않습니다.
제 코드에서 레포지토리 의존성을 줄일 수 있는 방법이 있나요? 제 방법이 최선이라면 그 이유는 무엇인 지 궁금합니다!!