-
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] 안금서 미션 제출합니다. #110
base: goldm0ng
Are you sure you want to change the base?
Conversation
먼저 답변부터 진행해볼게요! 나머지는 너무 졸려서 내일 작성할 수도 있고, 아니면 적는 곳까지는 적을 수도 있을 것 같아요 쉬운 순서대로 짧은 순서대로 진행해볼게요 4번 1번 일반적으로는 service레이어에서 이렇게 진행해왔던 것 같아요
딱 제가 짠다고 해도 이렇게 짤 것 같아요! 2번, 3번
답변오늘 글을 읽다보니 저보다 금서님이 더 jpa 에 대해서 더 잘 알고 계실 수도 있을 것 같다는 생각이 들지만 뉴비의 시선에서 말씀드린다고 생각해주세요 :) 대부분의 경우에 jpa 의 특정한 기능을 사용하기 보다는 순수 저장 순서들을 조절하고, 삭제 순서들을 조절하는 방식으로 삭제를 해왔던 것 같아요 실무에서 가장 중요하게 보는 것은 프로젝트를 누군가가 이어서 작업하는 것이 가능해야한다는 전제가 가장 중요한 프로젝트의 핵심인 것 같아요 이 전제를 기준으로 봤을 때 jpa 는 러닝커브가 정말 깊은 끝없는 그런 기술입니다 그렇다보니 cascade, onDelete 옵션같은 것들을 많이 쓰지 않는 것 같아요 그렇다면 반대로 jpa 를 몰라도 되냐? 라고 했을 때는 jpa 에서 다루고 있는 개념들은 db 에 핵심적인 개념일 때가 많습니다 그렇기에 배워두면 무조건 좋죠 일단 꼰대인 제가 생각하는 jpa 에 대한 생각을 먼저 말씀드렸으니 다음은 다시 답변으로 돌아가볼게요
저는 프로젝트로 간단하게 oneToMany, manyToOne 정도까지를 그냥 간단한 프로젝트를 하면서 배웠던 것 같아요 오히려 저는 db 쪽을 더 중요시 하는 것 같은데요 |
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.
정말 너무 잘 해주셔서 뭐라 달 말이 많이 없네요
천천히 보시고 잠결에 쓰다보니 뭔소리를 하는지 모르겠다 하시면 언제든 질문해주시면 됩니다!
일단 언제든 머지가 가능하게 approve 를 드리지만, 추가적으로 요청을 주시거나 dm 으로 요청을 주시면 언제든 확인해드릴게요!
고생 많으셨습니다
@ExceptionHandler(MemberNotFoundException.class) | ||
public ResponseEntity<String> handleMemberNotFound(MemberNotFoundException e) { | ||
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | ||
} | ||
|
||
@ExceptionHandler(JwtValidationException.class) | ||
public ResponseEntity<String> handleJwtValidationException(JwtValidationException e) { | ||
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | ||
} | ||
|
||
@ExceptionHandler(JwtProviderException.class) | ||
public ResponseEntity<String> handleJwtProviderException(JwtProviderException e) { | ||
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | ||
} |
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.
이렇게 로직이 같은 것들은
@ExceptionHandler(MemberNotFoundException.class) | |
public ResponseEntity<String> handleMemberNotFound(MemberNotFoundException e) { | |
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | |
} | |
@ExceptionHandler(JwtValidationException.class) | |
public ResponseEntity<String> handleJwtValidationException(JwtValidationException e) { | |
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | |
} | |
@ExceptionHandler(JwtProviderException.class) | |
public ResponseEntity<String> handleJwtProviderException(JwtProviderException e) { | |
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | |
} | |
@ExceptionHandler(MemberNotFoundException.class, JwtValidationException.class, JwtProviderException.class) | |
public ResponseEntity<String> handleUnauthorized(Exception e) { | |
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(e.getMessage()); | |
} | |
같은 느낌으로 통일해보면 어떨까요?
중복을 줄일 수 있을 것 같아요
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.
네 좋습니다! 훨씬 깔끔하고 좋은 방식이네요.
잦은 코드 중복에 유의해야겠어요!
|
||
@ExceptionHandler(Exception.class) | ||
public ResponseEntity<String> handleGeneralException(Exception e) { | ||
e.printStackTrace(); |
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.
e.printStackTrace 함수는 성능상으로 되게 안좋은 함수인데요
https://dev-jwblog.tistory.com/167 요것을 참고해서 로깅을 진행해보면 어떨까요?
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.
코드를 한줄 한줄 짤 때, 구현에 초점을 맞춰서 짜기 바빴어서 성능의 측면에서 생각해 보지는 못 했던 것 같네요!
실제 프로덕트를 운영하는 프로젝트를 할 때는 이런 사소한 것들이 발목을 잡는 경우가 많을 것 같아요.
앞으로 학습하고 코드를 짤 때는 성능도 함께 고려해야겠습니다!
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.
언급해주신 참고 자료 잘 읽어보았습니다.
e.printStactTrace를 지양해야 하는 이유를 적어놓겠습니다!
<e.printStackTrace() 사용을 지양해야 하는 이유>
- System.err 로 쓰여지게 되면서 제어하기 어렵고, 리소스 비용이 비싼편이다.
- System.err는 표준 오류 스트림이고, 기본적으로 콘솔에 출력되는데, 이 작업은 비교적 느리다. 특히, 많은 예외가 발생하거나 고빈도로 호출될 경우, 콘솔 출력이 성능 병목을 일으킬 수 있다.
- Java의 Reflection을 사용해 예외를 추적하는 것이라 많은 오버헤드가 발생한다.
- 서버에서 메소드 스택정보를 취합하기 때문에 서버 부하의 원인이 된다.
- e.printStackTrace()는 모든 스택 트레이스를 출력한다.
하지만, 모든 정보가 필요한 것은 아니며, 필요한 정보만 선별적으로 로그에 기록해야 한다. 긴 스택 트레이스를 콘솔에 출력하면 가독성이 떨어지고, 로그 파일 크기가 불필요하게 커질 수 있다.
- e.printStackTrace()는 모든 스택 트레이스를 출력한다.
- 출력이 어디로 향하는지 파악하기 어렵다.
- 관리가 어렵다.
- 로그 프레임워크(Logback, Log4j 등)를 사용하면 파일, 원격 서버, 콘솔 등 다양한 대상으로 로그를 출력할 수 있지만, printStackTrace()는 항상 표준 오류 스트림으로 출력한다. 이러한 제한은 로깅 설정을 통해 로그를 관리하고 최적화하려는 시도와 상충된다.
- 로그 프레임워크는 TRACE, DEBUG, INFO, WARN, ERROR 등의 수준(Level)을 제공하여 로그의 중요도를 구분할 수 있는데, e,printStackTrace는 이러한 수준을 제공하지 않아 로그 관리가 어렵다.
- 운영 환경에서의 문제가 발생할 수 있다.
- 운영 환경에서 e.printStackTrace()로 인해 민감한 정보가 콘솔에 출력될 경우 보안 문제를 유발할 가능성이 있다.
- 표준 오류 스트림이 파일로 리다이렉션 되지 않으면, 로그가 제대로 저장되지 않거나 손실될 가능성이 있다.
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.
e.printStackTrace 말고 로깅을 하는 다른 방법은 여러 가지가 있는데,
e.getStackTrace 와 e.getMessage, e.ToString 등을 로그 프레임워크와 함께 필요에 따라 적절히 조합해서 사용하는 게 좋은 방법이겠네요!
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.
@ExceptionHandler(Exception.class) | ||
public ResponseEntity<String> handleGeneralException(Exception e) { | ||
e.printStackTrace(); | ||
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(e.getMessage()); |
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.
보통
ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(e.getMessage());
와 같은 진짜 뭔지 모르는 곳에서 터지는 에러의 경우에는 e.getMessage() 가 서버의 중요한 부분을 담고 있을 수도 있어서 이런 케이스에서는 그냥 body 에 "잠깐 문제가 생겼어요. 다음에 다시 시도해주세요" 같은 문구를 내려주는 편입니다!
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.
아하 그렇군요!
Q1. 예를 들면 어떤 중요한 부분 말씀이신가요? 감이 잘 잡히지 않네요!
Q2. 그리고 누누님은 예외 응답을 내려줄 때 중대한 문제를 일으킨 경험이 있으신가요? 있다면 어떤 문제였고, 어떻게 해결하셨는지도 궁금합니다! (꼭 예외 응답을 내려줄 때가 아니어도 예외처리 관련한 경험이 있으시다면 말씀해주세요!)
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.
Q1 > 서버에 어떤 라이브러리를 사용하고 있는지와 같은 정보들이 전달되는 것 자체가 문제가 될 수 있어요!
Q2 > 저는 예외 응답을 만들다가 예외가 터졌던 기억이 나네요
근데, 예외 응답을 만드는 곳도(catch 문) 다시 한번 감싸져서 기본 응답을 주도록 되어있어서 문제 없이 동작했던 경험이 있어요!
@Slf4j | ||
@ControllerAdvice(assignableTypes = PageController.class) | ||
public class PageExceptionHandler { | ||
@ExceptionHandler(Exception.class) | ||
public String handleException(Exception e) { | ||
log.error("error: " + e.getMessage()); | ||
return "error/500"; //view 렌더링 페이지는 만들지 않음 | ||
} | ||
} |
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.
저도 이건 처음보는데 열심히 찾아보셨군요! 👍
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.
우선, GeneralExceptionHandler와 PageExceptionHandler를 나눈 이유는 예외처리를 할 때 상황에 따라 응답을 다르게 해주기 위해서입니다.
현재 구현되어 있는 컨트롤러들은 역할에 따라 크게 두가지로 나눌 수 있을 것 같아요.
- HTML 페이지를 렌더링 하는 역할을 하는 PageController
- API 요청을 처리하는 그 외 나머지 Controller( ex: MemberController, ReservationController, WaitingController, TimeController, ThemeController 등)
만약, 이 두 컨트롤러에서 500 에러와 같은 예상치 못한 예외가 발생한다면?
같은 예외라 하더라도,
PageController는 오류 페이지를 랜더링해서 보여주는 방식으로 응답을 내릴 수 있고,
그 외 다른 API 컨트롤러는 응답 바디에 상태 코드를 넣고 오류 메세지를 전달하는 방식으로 내릴 수 있습니다.
1번의 컨트롤러의 경우, 뷰 렌더링 과정에서 문제가 생기면 원래 받아야할 HTML 응답과 비슷한 형태의 예외 응답을 주는 것이 적절할 것이라고 판단하였습니다! 위 코드처럼 예외 페이지를 응답하거나 리다이렉트를 하는 방향으로요!
2번의 컨트롤러의 경우 상태 코드나, 오류 메세지 등을 전달해주는 방식으로 예외 응답을 줘서 프론트 측에서 예외 응답에 따라 적절한 조치를 취할 수 있도록 하는 것이죠.
Q. 누누님은 이런 방식에 대해 어떻게 생각하시나요?
그리고 실제로 이런식으로 행해지는 것인지도 궁금하네요!
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.
솔직히 말씀드려서 말씀해주신 2가지가 같이 케이스는 없었던 것 같아요
일반적으로는 html 을 렌더링하는 controller 가 없으니까요! (프론트가 있다보니...)
1번은 정확하게 모르겠어요 저도 실무에서는 전혀 볼 일이 없는 코드다보니...?
비슷하게 응답하는 방법처럼 진행해주신 방법은 좋은 것 같습니다! 일반 응답과 많이 달라지면 그 처리를 클라이언트에서 하지 못해서 에러가 많이 발생하는 것 같아요
2번의 경우에는 실제로 많이 쓰는데요
상태 코드 + 오류 메시지 + 서버의 오류 타입을 추가해서 이렇게 총 3가지를 내려주는 것 같아요
상태코드 400 에도 다양한 에러 메시지가 있을텐데, 이거를 프론트에서 분기하려면 오류 메시지를 의존하는 것 보다는 오류 타입에 의존하는 것이 좋은 것 같아요
{
"message":"요청에 시간이 없습니다",
"type":"invalid_time"
}
같은 형태이려나요?
public Member() { | ||
} |
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.
@NoArgsConstructor(access = AccessLevel.PROTECTED)
저는 개인적으로 클래스에 이 어노테이션을 더 선호했던 것 같아요!
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.
접근지정자를 protected로 설정해서 외부에서 기본 생성자를 호출하지 못하게 하도록 하는 방법을 어노테이션으로 지정할 수 있군요!
Q.
protected Member() {
}
말고
@NoArgsConstructor(access = AccessLevel.PROTECTED)
어노테이션을 쓰는 이유가
- 편리성
- 가독성
때문인지, 아니면 다른 이유가 더 있는지 궁금합니다!
그리고 이 리뷰에서 새로 알게 된 내용이 있어요!
우선, JPA는 내부적으로 리플렉션을 사용해 객체를 생성하므로 기본 생성자가 반드시 필요합니다. JPA가 객체를 생성 시, 접근지정자가 public 이어야만 접근이 가능한 줄 알았는데, protected로 해도 잘 생성이 되네요.
protected 접근지정자 사용으로 JPA나 상속 구조에서만 기본 생성자를 사용하도록 의도할 수 있겠어요!
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.
가독성때문입니다!
저거를 모든 코드에 다 넣고 있게 되면 중복 코드도 많아지고, ide 에서는 안쓰는 생성자라고 판단해서 unused 코드를 띄워줄 수 있고, 이를 따라 삭제했을 때 에러가 나는 어지러운 상황이 나올 수도 있어서 그랬던 것 같아요
JwtResponse jwtResponse = jwtUtils.extractTokenFromCookie(request.getCookies()); | ||
MemberAuthInfo memberAuthInfo = jwtUtils.extractMemberAuthInfoFromToken(jwtResponse.accessToken()); | ||
LoginCheckResponse loginCheckResponse = loginService.checkLogin(memberAuthInfo); |
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.
개인 취향이긴 하지만 보통 저는 이 로직을 담은 서비스를 선호하기는 합니다!
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.
제가 누누님이 하신 말씀이 정확히 이해했는지는 잘 모르겠는데,
누누님은 보통 저 세줄의 로직을 보통 서비스의 한 메서드 로직으로 쓰신다는 말씀이신가요~?
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.
넵 맞아요
한 컨트롤러의 한 api 에서는 한 service 의 메소드만을 호출하는 것이 가장 익숙했던 것 같아요
@Query("SELECT COUNT(r) > 0 FROM Reservation r WHERE r.date = :date AND r.time.id = :timeId AND r.theme.id = :themeId") | ||
boolean existsByDateAndTimeIdAndThemeId(@Param("date") 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.
jpa 자체에 exists 라는 named 쿼리가 있을거에요!
https://hungseong.tistory.com/73
추가로 exists 라는 쿼리 메소드도 있으니 참고해도 좋을 것 같아요!
참고로 저희 팀에서 사용하는 순서는 이렇게 됩니다
- named 쿼리 <-- 최대한 이쪽으로 풀어보려고 함
- jpql
- native query <--- 거의 사용하지 않기 위해 노력함
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.
감사합니다! 쿼리 메서드 사용해서 수정하였습니다~
추가적으로, 말씀해주신 3가지에 대해서도 더 학습을 해봐야겠네요.
Q. named 쿼리로 최대한 많이 풀어보려고 한다고 하셨는데, 팀에서 jpql보다 named 쿼리를 더 우선순위로 두는 이유가 뭔가요?
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.
JPQL 의 경우에는 문법이 그냥 sql 과 달라서 익숙하지 않은 것이 가장 크구요
잘못된 필드가 나왔을 때 에러 메시지가 가장 직관적이었던 것 같아요
roomescape.auth.jwt.secret=Yn2kjibddFAWtnPJ2AFlL8WXmohJMCvigQggaEypa5Eaadfasdfsadfsafdsa2df= | ||
server.connection-timeout=60s |
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.
오 혹시 이 부분은 무슨 의도로 추가하게 되었을까요?
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.
1.secret
이 값은 WT 토큰을 생성하고 검증할 때 사용하는 키이고, 보안상의 이유로 키를 코드에 직접 적어놓지 않고 설정 파일이나 환경 변수에서 관리하기 위해 추가했습니다!
2. timeout
이걸 추가한 이유는 사실 MVC(인증)미션 요구사항 중
Response
HTTP/1.1 200 OK
Connection: keep-alive
Content-Type: application/json
Date: Sun, 03 Mar 2024 19:16:56 GMT
Keep-Alive: timeout=60
Transfer-Encoding: chunked
{
"name": "어드민"
}
Keep-Alive: timeout=60 의 내용이 있어, 추가하게 되었습니다! 근데 디폴트값이 60인가보네요?
코드를 제거해도 똑같은 응답이 옵니다.
그리고 timeout 설정에 대해 말씀드리자면, 이는 서버의 연결 제한 시간을 설정하는 프로퍼티입니다.
위 코드의 경우, 클라이언트가 서버에 요청을 보낸 후 60초 내에 연결이 이루어지지 않으면 타임아웃됩니다.
60초라는 제한을 둠으로써, 서버가 장시간 대기 상태로 남아 있는 것을 방지하고 리소스를 효율적으로 관리하려는 목적으로 쓰인다고 합니다!
근데 정확히 timeout이 발생했을 때, 서버 측에서 어떤 식으로 그걸 캐치해서 응답을 내려주는지에 대해 그림이 그려지지 않아서 이 부분에 대해서는 한 번 찾아봐야할 것 같습니다 🤔
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.
아마 keep-alive 속성쪽과는 전혀 무관할거에요
gpt 피셜
Keep-Alive
헤더와 Spring Boot의server.tomcat.connection-timeout
설정은 둘 다 HTTP 연결 시간과 관련이 있지만, 서로 다른 수준에서 작동하며 목적도 다릅니다.1. Keep-Alive 헤더
- 역할: HTTP/1.1에서 기본적으로 활성화되어 있는 기능으로, 하나의 TCP 연결을 재사용할 수 있도록 지원합니다. 클라이언트와 서버 간의 연결을 유지하여 다수의 요청/응답을 처리할 수 있도록 합니다.
- 주요 요소:
- 클라이언트는
Connection: Keep-Alive
헤더를 요청에 포함하여 연결 유지 의사를 전달합니다.- 서버는 Keep-Alive를 지원하면, 일정 시간 동안 연결을 유지합니다.
- 관련 헤더:
Keep-Alive: timeout=5, max=100
timeout
: 연결을 유지할 최대 시간(초).max
: 연결을 통해 처리할 최대 요청 수.- 사용 목적: 네트워크 비용을 줄이고, 요청 처리 속도를 높임.
2. server.tomcat.connection-timeout
- 역할: Spring Boot에서 내장된 Tomcat 서버의 클라이언트와의 연결 타임아웃 시간을 설정합니다.
- 기본값: 20초(
20000ms
)로 설정.- 의미:
- 클라이언트가 연결을 시도한 뒤 요청 데이터를 보내는 데 걸리는 최대 시간을 설정.
- 클라이언트 요청이 이 시간을 초과하면 서버는 연결을 끊습니다.
- 예시 설정:
properties코드 복사server.tomcat.connection-timeout=30000 # 30초
차이점
항목 | Keep-Alive 헤더 | server.tomcat.connection-timeout |
---|---|---|
적용 대상 | HTTP 프로토콜 수준 | 서버(Tomcat) 수준 |
목적 | 연결 재사용 및 성능 향상 | 클라이언트 요청의 최대 대기 시간 제한 |
작동 방식 | 요청-응답 연결을 유지 | 연결 시도 후 요청 데이터를 받을 때까지 기다림 |
설정 위치 | 클라이언트/서버 HTTP 헤더 | Spring Boot 애플리케이션 설정 파일 |
결론
Keep-Alive
는 연결 유지를 목적으로 하며, 네트워크 효율성을 높이는 데 사용됩니다.server.tomcat.connection-timeout
은 연결이 초과 대기하는 상황을 방지하여 자원 낭비를 줄이기 위한 타임아웃 설정입니다.
서로 관련이 있을 수는 있지만, 정확한 역할과 설정 대상이 다르기 때문에 혼동하지 않도록 주의하세요!
connection timeout 은 특정 시간 이상 걸리는 요청의 경우에는 대기 없이 바로 응답을 줄거에요!
https://alden-kang.tistory.com/20
아마 말씀해주신 부분은 read-timeout 쪽과 조금 더 가까울 것 같네요!
if (memberAuthInfo == null || | ||
waitingRequest.date() == null || | ||
waitingRequest.theme() == null || | ||
waitingRequest.time() == null) { | ||
return ResponseEntity.badRequest().build(); | ||
} |
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.
저는 개인 취향이지만 요것도 service 에 두는 것을 선호합니다!
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.
개인적인 의견이더라도, 선호하시는 이유 여쭤봐도 될까요?
저도 저런 검증로직을 service 와 controller 중 어디에 둘지 고민을 많이 했어서 누누님의 의견이 궁금합니다!
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.
컨트롤러에는 그냥 service 1줄 호출하는 것이 가장 현재 코드에서 많이 보다보니 그런 것 같아요!
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.
고생하셨습니다!
이미 너무 잘 해주셔서 수정할 부분이 거의 없는 것 같네요!
참고용으로 간단한 내용을 남겨드릴게요!
너무 잘 해주셔서 별다른 내용을 달아드릴만한 것이 없다보니
이번 미션 내용은 아니지만 보안을 지키는 코딩에 관한 내용을 조금 남겨두려고 합니다! 🙇
실제 다른 리뷰에서 진행했던 내용인데요
#107 (comment)
토큰에 있는 role 을 믿어서는 안되는 이유인데요
secure coding 관점에서는 진짜 사용자의 어떠한 인풋도 신뢰하면 안되는... 그런 문제가 항상 있는 것 같아요
보안과 권한 관리에서 어디까지 타협하는 것이 좋은지는 경험에 많이 의존하게 되는 것 같은데요
이런 부분도 고민을 해보시면 많이 배울 수 있을 것 같아요
#107 (comment)
관련해서 다양한 보안 정책이 나오고, 이런 것들은 언젠가는 한번쯤 고민해보셔야 될 것이다보니 미리 슬쩍 던져봅니다 🙇
.orElseThrow(() -> new MemberNotFoundException("가입된 회원이 아닙니다.")); | ||
} | ||
|
||
validateDuplicateReservation(reservationRequest); |
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.
서버 코드로 검증을 하시고 계시는 것은 아주 좋은 것 같아요 👍
하지만 저희는 멀티 스레드 환경이다보니 테이블에 실제 insert 가 잘못되는 것은 막을 수 없는데요
다음 미션때 date, time, theme 에 해당하는 unique index 를 추가해보면 좋을 것 같아요!
date, time, theme 에 unique index 를 추가하면 같은 예약이 2개 생기는 것을 db 레벨에서도 막을 수 있을 것 같아요
안녕하세요 누누 리뷰어님!
첫 리뷰어 매칭이네요 반갑습니다:)
끈질긴 감기녀석 때문에 이번 미션 안 그래도 어려운데 🥶 배로 어렵게 느껴졌네요,,,
요즘 독감 유행이라던데 건강 잘 챙기세요 누누님.
Spring Data JPA 미션은 유독 에러도 많고 시행착오도 정말 많았는데,
이번 미션도 지난 미션과 마찬가지로 진행하면서 겪었던 생겼던 시행착오에 대해 말씀드리고 조언을 구하고 싶습니다!
<겪었던 시행착오 및 해결방안 혹은 고민사항>
'Repository에서의 Optional 예외처리, 어디서 어떻게 처리하면 좋을까?' 에 대한 고민사항이 있습니다.
데이터 영속성 문제에 대한 시행착오
이 문제는 특정 객체를 생성하고 저장할 때 발생했습니다.
이 코드에서는
라는 예외가 발생했는데요.
이는 JPA/Hibernate에서 영속되지 않은 엔티티를 참조하려고 시도할 때 발생한다고 합니다.
Reservation 엔티티의 theme 필드와 time 필드가 아직 데이터베이스에 저장되지 않은 Theme 객체, Time 객체를 참조하고 있다는 의미입니다. 즉, Reservation 객체를 저장하려고 할 때, Reservation 엔티티에 매핑된 Theme와 Time 객체가 영속 상태가 아니기 때문에 Hibernate가 이 관계를 처리할 수 없으므로 예외가 발생하는 것으로 보입니다.
해결 방안으로 총 두가지를 찾아봤습니다.
첫번째로, Theme 객체 및 Time 객체 (Reservation에 매핑된 객체) 들을 먼저 데이터베이스에 저장 후, Reservation을 저장
`
ex)
`
두번째로, Cascade 설정 추가
Reservation 엔티티의 theme과 time 필드에 CascadeType.PERSIST 또는 CascadeType.ALL을 추가하면 Reservation이 저장될 때 Theme도 자동으로 저장됩니다.
`
ex)
`
이 방법을 사용하면 Reservation을 저장할 때, Hibernate가 자동으로 Theme 객체를 먼저 저장한다고 합니다!
이는 앞서 말했던 데이터 영속성 문제와 비슷한 맥락으로 시행착오를 겪었는데요.
(정보: 관리자는 관리자 페이지로 접속해 예약 생성 및 삭제, 테마 추가 및 삭제, 시간 추가 및 삭제를 할 수 있습니다.)
관리자로 접속하여 예약 추가와 테마 및 시간 추가/삭제는 되는데 예약 삭제 시, 다음과 같은 예외가 발생했습니다.
해당 예외는 데이터베이스에서 참조 무결성 제약 조건이 위반되어서 발생하는 예외라고 합니다.
즉, Time/Theme 테이블에서 삭제하려는 id가 Reservation 테이블의 외래 키(time_id/theme_id)로 참조되고 있기 때문에 삭제가 불가능하다는 것입니다. 이 문제 또한 위와 비슷한 방식으로 문제를 해결할 수 있습니다!
첫번째로, 참조된 데이터를 먼저 삭제하기
`
ex) Reservation 테이블에서 time_id = 2인 데이터를 먼저 삭제한 후 Time 테이블의 행 삭제
`
두번째로는, 외래 키 설정에 ON DELETE CASCADE 추가하기
데이터베이스 외래 키를 ON DELETE CASCADE로 설정하면, TIME 테이블의 행이 삭제될 때 참조된 RESERVATION 데이터도 자동으로 삭제된다고 합니다.
`
ex)
`
세번째로, @onDelete 활용하기
@onDelete는 외래 키가 참조하는 부모 엔티티가 삭제될 때의 동작을 정의하는 어노테이션이며,
부모 엔티티가 삭제되었을 때, 관련된 자식 엔티티를 자동으로 삭제하도록 데이터베이스에 위임한다고 합니다.
`
ex)
`
이러한 데이터 영속성 및 무결성 위반에 대한 시행착오를 겪고 ... JPA에 대해 더 깊이 공부해야겠다는 생각이 드네요.
혹시 누누님은 처음 JPA 학습하실 때, 어떤식으로 하셨나요? 아니면 같이 알면 좋을 법한 내용이나, 키워드, 공부 방향성 같은 것들을 알려주셔도 좋아요 🤩
(이건,, 계속 붙잡고 있었는데 도저히 뭐가 문제인지 모르겠어서 도움 요청 드립니다! 저도 계속 보긴 할 거지만! 혹시나 코드 보다가 원인을 찾으셨다면 공유 부탁드려요 🥺)
우선 상황 설명 드리겠습니다!
<문제 상황>
예약 대기 취소 버튼을 누르면 예상치 못한 500 error 가 발생합니다.
예외 내용은 다음과 같아요.
문제 발생은 WaitingController 중,
`
`
이 부분에서, id가 매핑이 안 되는 문제 같아보입니다.
요청이 "DELETE /waitings/undefined" 로 오더라고요..
그렇다고 해서 예약 대기 응답 객체에 id가 안 담기는 것도 아닙니다 😭
일단, 계속 원인을 찾아보겠습니다! + 코드 리팩토링까지도요!
추가적으로 궁금한 사항이 생기면 더 적어놓겠습니다!
날카로운 리뷰 부탁드립니다 누누님!
화이팅!!