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

[1 - 3단계 방탈출 사용자 예약] 재즈(함석명) 미션 제출합니다. #80

Merged
merged 79 commits into from
May 6, 2024

Conversation

seokmyungham
Copy link
Member

@seokmyungham seokmyungham commented May 2, 2024

러너덕 안녕하세요 🤗
이번 미션 리뷰이 재즈입니다~
첫 미션 자동차 경주 리뷰어가 러너덕이었는데 몇 달 만에 또 뵙게 되었네요 ㅎㅎ 🙇‍♀️ (제 인생 첫 리뷰어..)
당시 러너덕께 배운 내용들이 중요하고도 많았는데, 덕분에 그 동안 많이 성장할 수 있었던 것 같아 정말 감사드립니다 😄

DM으로 말씀드린대로 Pull Request 시 conflict가 발생했었는데, 해결한 것 같습니다.. 😓

페어와 일정을 조율하면서 부득이하게도 수, 목 2일 안에 모두 구현 해야 했기에 시간이 부족했습니다. 😅
때문에 전체적인 네이밍, 테스트가 개인적으로 너무 아쉬운데 리뷰를 반영하며 리팩토링 해보겠습니다.

이전 미션 코드 migration 커밋을 제외한 변경사항은 다음과 같습니다. 😀
변경 사항

스프링 경험

강의를 찾아 들으며 스프링을 2년 정도 학습하고 프로젝트 배포까지 한 경험이 있습니다.
하지만 부족한 점이 너무 많다고 느끼고, 러너덕께 이번에도 많이 배우고 싶습니다.

궁금한 사항

1. 도메인과 ResponseDto 스펙 일치로 인한 보안 우려

현재 예약 관련 응답을 모두 ReservationResponseDto를 사용해서 반환하고 있습니다. (도메인 스펙과 일치하는 superDto)
때문에 응답시 화면에 보이는 데이터만 반환하는 것이 아니고, Reservation 도메인 스펙을 그대로 외부에 노출하고 있는 것 같아 상당히 위험하다고 느끼고 있습니다. 또 불필요한 네트워크 소모로 인한 낭비도 있을 것 같습니다.

필요한 응답 데이터에 FIT 하게 맞추어 상황에 맞는 Dto를 작성해야 할 것 같은데 그러면 모든 응답 상황에 맞는 dto를 만들어야 할까요?

유명한 서비스 기업.. (인스타, 구글 등)의 응답 json을 찾아봤눈데 막 그렇지만도 않은 것 같습니다..
러너덕은 어떻게 생각하시는지 궁금합니다.

2. RequestDto에서 검증 진행

각 요청 상황마다 원하는 검증이 다를 수 있고, 데이터가 도메인과 일치하지 않기때문에
RequestDto에서 원하는 검증을 진행하는 게 자연스러울 것 같다고 생각했습니다.

그러다 보니 현재 도메인에서 아무런 검증을 하고 있지 않은데
도메인에서 책임질 검증과 사용자 요청 검증을 나누어 작성해야 할 지 궁금합니다.

3. Service, Controller 테스트를 이렇게 작성하는 것이 맞는지..

이전 미션을 진행하는 동안, 개인적으로 @DirtiesContext 를 지양해야 하는 이유를 학습했습니다.
과정에서 레이어 단위 슬라이스 테스트가 중요하다고 느껴 학습하고 미션에 적용해보았습니다.
(시간이 부족해 구현에 급급하여 아직 테스트가 많이 부족합니다)

특히 서비스 테스트의 행위 검증, 컨트롤러 WebMvcTest를 활용한 테스트에 있어
제가 올바른 테스트를 작성한 게 맞는지, 혹여나 의미없이 당연한 걸 테스트하고 있는 것인지 배우고 싶습니다.
(테스트를 잘 작성하고 싶습니다!!.)

코드와 긴 글 읽어주셔서 감사합니다. 😄
이번에도 잘 부탁드립니다🙇‍♀️

@seokmyungham seokmyungham changed the base branch from main to seokmyungham May 2, 2024 08:59
@seokmyungham seokmyungham changed the title delete(RoomescapeApplicationTest): 테스트 삭제 [1 - 3단계 방탈출 사용자 예약] 재즈(함석명) 미션 제출합니다. May 2, 2024
seokmyungham and others added 27 commits May 2, 2024 18:07
- 시작 시간이 입력되지 않은 경우
- 시작 시간의 형식이 잘못된 경우

Co-authored-by: hyxrxn <[email protected]>
- 이름이 입력되지 않은 경우
- 날짜가 입력되지 않은 경우
- 날짜의 형식이 잘못된 경우
- 시간 아이디가 입력되지 않은 경우
- 시간 아이디가 자연수가 아닌 경우

Co-authored-by: hyxrxn <[email protected]>
- 이름이 입력되지 않은 경우
- 설명이 입력되지 않은 경우
- 썸네일이 입력되지 않은 경우

Co-authored-by: hyxrxn <[email protected]>
@seokmyungham
Copy link
Member Author

seokmyungham commented May 6, 2024

안녕하세요 러너덕!
우선 정성스런 리뷰 감사드립니다.🙇‍♀️
단 한 번의 리뷰 요청이었는데 궁금했던 점들이 많이 해소되어 너무 재미있네요. 👍👍

그리고 도메인 스펙을 노출하는 것이 왜 위험한가요?

에러 stack trace 를 통해 서버 정보가 노출되는 것 처럼
우리 도메인 이렇게 생겼다~ 라고 외부에 노출하는 것 같아 위험하다는 생각이 들었습니다.
아직 경험해보지 못해서 감으로만 예상했던 부분인데 제 생각이 틀렸을 수도 있을 것 같아요.

전체적인 네이밍 및 리팩토링에 신경쓰고, 리뷰해주신 사항에 집중적으로 테스트를 작성해보았습니다.

긴 글과 코드 읽어주셔서 감사드리고
연휴 즐겁게 마무리 하시길 바랍니다 😊

Copy link

@Deocksoo Deocksoo left a comment

Choose a reason for hiding this comment

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

코드와 테스트 모두 생각하시는 방향대로 잘 반영되었네요 👍
다음 미션 계속 진행해주세요~
재즈도 남은 연휴 즐겁게 보내셔요 :)

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