-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat & refactor: 회원 검색 및 조회 기능을 구현한다 #48
Conversation
- /api/members/hobbies - /api/members/styles
- HobbyRepository - HobbyJpaRepository - HobbyQueryRepository - HobbyRepositoryImpl
- HobbyNameAlreadyExistedException - HobbyCodeAlreadyExistedException - HobbyNotFoundException - InvalidHobbyException
- StyleRepository - StyleJpaRepository - StyleQueryRepository - StyleRepositoryImpl
- StyleNameAlreadyExistedException - StyleCodeAlreadyExistedException - StyleNotFoundException - MemberStyleDuplicateException - MemberStyleDuplicateException - InvalidStyleException
- 요구사항: 페이징 처리시 전체 데이터 수도 반환하도록 변경
- 기존의 초기화와 업데이트 요청을 하나의 메서드 로직으로 처리하던 방식을 분리 및 프로필 상태를 나타내는 필드 추가
- 내부에서 사용할 dto의 네이밍 변경으로 인한 코드 수정
- MemberHobbyDuplicateException - MemberHobbySizeException
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.
많은 작업 하신 게 느껴지셔서 정말 수고하셨습니다! 코멘트 확인해주시면 될 것 같습니다.
그리고 Hobby와 Style 단의 도메인 패키지를 hobby, style 이렇게 세분화해서 관리하는 건 어떨지 태인님 의견을 여쭤보고 싶습니다.
@@ -15,7 +15,7 @@ public class CorsCustomFilter extends OncePerRequestFilter { | |||
protected void doFilterInternal(final HttpServletRequest request, | |||
final HttpServletResponse response, | |||
final FilterChain filterChain) throws ServletException, IOException { | |||
response.setHeader("Access-Control-Allow-Origin", "*"); | |||
response.setHeader("Access-Control-Allow-Origin", "https://admin.atwoz.kr"); |
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.
구입한 도메인 주소는 private static final
로 설정해두는 것이 어떨까요?
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 Member 회원_생성() { | ||
return memberRepository.save(일반_유저_생성()); | ||
return memberRepository.save(MemberFixture.회원_생성_취미목록_스타일목록(hobbies, styles)); |
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.
fixture는 기존처럼 static import하시면 좋을 것 같습니다!
private List<Style> styles; | ||
|
||
@BeforeEach | ||
void init() { |
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.
IntegrationHelper을 상속받았으니 매번 실행 전에 init이 AcceptanceFixture에서도 돌아가겠군요..! 향후 다른 기능들에도 유용하게 쓰일 패턴인 것 같습니다 :)
|
||
protected Member 회원_생성() { | ||
return memberRepository.save(일반_유저_생성()); | ||
return memberRepository.save(MemberFixture.회원_생성_취미목록_스타일목록(hobbies, styles)); |
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.
static import하시면 좋을 것 같습니다!
protected Member 회원_생성() { | ||
return memberRepository.save(일반_유저_생성()); | ||
return memberRepository.save(MemberFixture.회원_생성_취미목록_스타일목록(hobbies, styles)); |
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.
여기도 static import 부탁드립니다 :)
return new InternalStylesInitializeRequest(convertToMemberStyles(styles)); | ||
} | ||
|
||
private static List<MemberStyle> convertToMemberStyles(final List<Style> hobbies) { |
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.
List<Style>이 hobbies로 되어 있습니다 👀
@@ -16,29 +15,27 @@ | |||
|
|||
@Getter | |||
@Builder | |||
//@EqualsAndHashCode(of = "hobby") |
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.
EqualsAndHashCode를 아예 삭제하셔도 될 것 같습니다!
memberQueryService = new MemberQueryService(memberRepository, hobbyRepository); | ||
취미_목록_생성(hobbyRepository); | ||
스타일_목록_생성(new StyleFakeRepository()); | ||
memberLikeRepository = new MemberLikeFakeRepository(); |
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.
setup을 작성할 때 객체 생성 후 별도의 로직 (ex: 취미 목록 생성)을 그 다음에 써두는 식으로 통일하면 흐름 이해 상 좋아질 것 같은데 어떠실까요?
} | ||
} | ||
|
||
private 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.
Member를 클래스 안에서 만들어주셨는데, 어떤 상황에서 Fixture를 쓰고 어떤 상황에서 클래스 안에서 만드는 방식으로 사용하시게 되었는지 설정하신 기준을 여쭤봐도 될까요? 저도 테스트 코드를 작성하면서 이런 경우가 종종 있을 것 같은데 태인님께서 사용하셨던 기준이 궁금합니다!
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.
이 기준은 테스트 메서드의 가독성을 높이고 싶어서 다음과 같이 작성했습니다! 회원도 골드, 다이아 회원을 나눈 이유도 한글 이름으로 메서드를 만들었을 때 이성_회원_생성_등급() 이렇게 제공하는 것보다 각각 나눠서 하는게 좀 더 가독성이 높다고 판단하였고,
회원_생성_등급() 이런식으로 하면 차라리 MemberFixture를 사용하는 것이 더 좋다고 생각했습니다.
그리고 MemberFixture를 사용하지 않은 이유는 MemberFixture로는 어떤 값이 사용되는지를 파악하여 쉽게 회원을 생성하는 것을목적으로 만든것이지만 복잡한 테스트에서는 정확히 어떤 객체가 생성되는지를 간결하면서도 정확하게 표현하는 것이 중요하다고 생각했습니다.
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.
메서드 가독성을 기준으로 판단하셨군요! 공유 감사합니다 :)
|
||
@RequiredArgsConstructor | ||
@Repository | ||
public class MemberJdbcRepository { |
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.
findTodayProfiles 메서드를 제외하고는 JPA, Querydsl을 이용하셨는데 JDBC를 쓰시게 된 계기가 있을까요?
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.
findTodayProfiles 메서드는 골드 회원 2명과 다이아 회원 1명을 조회해야합니다. Querydsl로는 한계가 있어 해당 메서드만 JDBC를 이용하게 되었습니다. 나머지 기능들은 한명만 조회하면 되기 때문에 Querydsl을 이용하였습니다!
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.
아하 여러 회원을 조회해야 해서 그렇군요..! JDBC 쿼리를 보니 엄청 고생하신 게 보였습니다 🥲 고생하셨습니다!
- getPhysicalProfileInitialRequest -> getPhysicalProfileUpdateRequest
|
- Hobby - Style
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.
수고하셨습니다!
📄 Summary
회원 검색 및 조회 기능을 구현했습니다. 구현 중에 취미와 스타일을 VO로 관리하지 않고 API를 호출하여 관리하는 방식으로 변경해달라는 요청이 있었습니다. 그래서 Member 도메인의 구조를 변경하게 되어 작업이 늦어졌습니다.
추가적으로 조회 API에 총 조회된 데이터 수도 넘겨달라는 요구사항을 반영하여 취미와 스타일 API를 개발 완료했습니다.
이번에 수정 및 개발한 내용을 정리해보면
close #47