-
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: 호감 기능을 구현한다 #33
Conversation
- 좋아요 레벨을 나타내는 LikeType 등록 - LikeType 조회 실패 예외 등록
- MySQL 예약어에 충돌되지 않도록 이름 수정
- MemberLike 기본 도메인 엔티티 설계
- 좋아요 레벨 이름 변경
- 좋아요 아이콘 타입 (기본, 메시지, 하트) 추가
- 좋아요 아이콘, 최근 여부 필드 추가
- 회원 id가 존재하는지 검증하는 이벤트 및 핸들러 작성
- 좋아요 (호감) 기능 구현 - 좋아요 실행 시 타겟 회원 id 존재 유무 이벤트 검증 진행
- 호감 응답이 30일 이상 없을 경우 삭제되도록 스케줄 적용
- 호감 재전송 예외 검증 추가
- named query 컨벤션 수정
- 문서화 오류 수정
- 48시간 기준 최근 여부 변경 탐지 메서드 작성
- 회원 호감 내역 조회 기능 개발 (보낸 호감, 받은 호감)
- 회원 호감 내역 조회 리스트로 수정 - 회원 호감 컨트롤러 생성 - 회원 호감 내역 조회 전용 서비스 생성
- 자기 자신에게 호감을 보낼 수 없도록 검증 추가
- 호감 도메인 테스트 작성
- LikeType을 LikeLevel로 이름 수정
- JdbcTemplate SQL 오류 수정
- MemberLikeJdbcRepository 테스트 작성 - 관련된 fixture 등록 - MemberLike SuperBuilder 추가
- MemberLikeJpaRepositoryTest 작성
- MemberLikeQueryService가 MemberLikeRepository를 의존하도록 수정
- MemberLikeQueryRepositoryTest 테스트 작성
- MemberLikeServiceTest 작성 - 필요한 Fixture 생성
- LikeIcon 이름 반환하도록 수정
- MemberLikeQueryRepository 조회 방식 변경 - MemberLikeQueryRepository 테스트 시 limit 적용
- MemberLikeQueryServiceTest 테스트 작성 - MemberLikeFakeRepository 작성
- MemberLikeController RestController 등록 - 통합 테스트 작성
- MemberLikeControllerAcceptanceTest 테스트 작성
- 호감 문서화 적용
- 호감 전송 테스트 시 회원 저장 실패 예외 수정
- MemberLikeServiceTest 이벤트에 mockito 적용하여 이벤트 발생하지 않도록 수정
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.
고생하셨습니다. 몇가지만 수정해주세요~
@@ -0,0 +1,6 @@ | |||
package com.atwoz.member.application.member.event; | |||
|
|||
public record ValidatedMemberExistEvent( |
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.
exist는 동사인데 클래스 이름으로는 명사인 existence가 더 적합해보입니다 :)
public Page<MemberLikeSimpleResponse> findReceivedLikesWithPaging(final Long receiverId, final Pageable pageable) { | ||
List<Member> senders = findSenders(receiverId).stream() | ||
.map(this::findMember) | ||
.toList(); | ||
List<MemberLikeSimpleResponse> responses = senders.stream() | ||
.map(receiver -> collectSenderResponse(receiverId, receiver)) | ||
.toList(); | ||
|
||
int total = responses.size(); | ||
int start = (int) pageable.getOffset(); | ||
int end = Math.min((start + pageable.getPageSize()), total); | ||
|
||
List<MemberLikeSimpleResponse> result = responses.subList(start, end); | ||
|
||
return new PageImpl<>(result, pageable, total); | ||
} |
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.
쿼리 dsl로 조회할 때 offset, limit로 페이징해서 조회하는게 좋아보이는데 이렇게 구현하신 이유가 있나요?
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.
톡으로 전달한 내용을 PR에서도 작성하면 좋을 것 같아 재작성 하겠습니다!
MemberLikeQueryRepository에서 현재 구현할 때는 쿼리 매핑으로 한 번에 응답 타입으로 조회하기보다는, Member와 MemberLike들을 객체들로 가져온 다음 페이지 갯수에 맞게 필터링 해주는 방식으로 했습니다.
다시 보니 private 메서드들에서 pageable들도 함께 넘기면 쿼리 단에서 바로 적용할 수 있었을텐데 지금 방식은 모두 조회하게 한 다음에 자르는 형식이라 메모리 낭비가 더 심할 것이라고 느꼈고, pageable을 내부 메서드로 전달시켜 QueryResults를 통한 전체 사이즈를 이용하도록 수정 완료하였습니다 :)
private final MemberLikeService memberLikeService; | ||
private final MemberLikeQueryService memberLikeQueryService; | ||
|
||
@GetMapping("/send") |
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.
restful convention에 맞게 sending으로 바꾸면 좋을 것 같습니다 :)
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.
Restful 원칙에 대해 다시 생각해 본 결과, send와 receive 모두 동사 형태로 현재 적합하지 않은 형태라는 것을 느꼈습니다.
그러나 말씀해주신 sending 또한 동명사 형태긴 하지만 현재 진행중인 느낌이 강한 것 같아 대신 likes/sent
, likes/received
등 수동태 느낌이 나도록 수정 완료하였습니다! 짚어주셔서 감사합니다 :)
@Test | ||
void 호감_정상_생성() { | ||
// given | ||
Long senderId = 1L; | ||
Long receiverId = 2L; | ||
String likeLevel = "관심있어요"; | ||
|
||
// when & then | ||
assertDoesNotThrow(() -> MemberLike.createWith(senderId, receiverId, likeLevel)); | ||
} | ||
|
||
@Test | ||
void 호감_값_정상() { | ||
// given | ||
Long senderId = 1L; | ||
Long receiverId = 2L; | ||
String likeLevel = "관심있어요"; | ||
LikeLevel expectedLevel = LikeLevel.DEFAULT_LIKE; | ||
LikeIcon expectedIcon = LikeIcon.NONE; | ||
|
||
// when | ||
MemberLike memberLike = MemberLike.createWith(senderId, receiverId, likeLevel); | ||
|
||
// then | ||
assertSoftly(softly -> { | ||
softly.assertThat(memberLike.getSenderId()).isEqualTo(senderId); | ||
softly.assertThat(memberLike.getReceiverId()).isEqualTo(receiverId); | ||
softly.assertThat(memberLike.getLikeLevel()).isEqualTo(expectedLevel); | ||
softly.assertThat(memberLike.getLikeIcon()).isEqualTo(expectedIcon); | ||
softly.assertThat(memberLike.getIsRecent()).isEqualTo(true); | ||
}); | ||
} |
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.
이 테스트 메서드들도 @nested안에 포함시켜도 되지 않을까요?
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.
거의 항상 예외 단에서만 nested를 적용했는데 정상인 경우에도 가짓수가 늘어난다면 nested를 적용하는 게 가독성 측면에서 좋은 걸 느꼈습니다 :) 적용 완료했습니다!
private MemberLikeSimpleResponse convertToResponse(Long id, MemberLike memberLike) { | ||
return new MemberLikeSimpleResponse( | ||
id, | ||
"회원 " + id, | ||
"서울시", | ||
"강남구", | ||
20, | ||
memberLike.getLikeIcon().getName(), | ||
memberLike.getIsRecent() | ||
); | ||
} |
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.
final 키워드 붙여주세요~
@Test | ||
void 호감을_보낸_지_30일이_지나도_응답이_없는_호감은_삭제된다() { | ||
// given | ||
LocalDateTime pastTime = LocalDateTime.now().minusDays(MINUS_DAY_FOR_DELETE_NOT_ANSWERED_LIKE); |
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개네요 줄바꿈 부탁드려요~
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<Long> expected = memberLikes.stream() | ||
.sorted(Comparator.comparing(MemberLike::getCreatedAt).reversed()) | ||
.map(MemberLike::getReceiverId) | ||
.limit(9) | ||
.toList(); | ||
List<Long> foundMembers = found.getContent().stream() | ||
.map(MemberLikeSimpleResponse::memberId) | ||
.limit(9) | ||
.toList(); |
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.
테스트 코드에서는 메서드 분리를 도입하지 않았었는데 테스트 코드에서도 반복된다면 메서드 분리를 도입하는 게 가독성이 좋아진다는 걸 느꼈습니다 :) 메서드 추출 후 적용하였습니다
private MemberLike(final Long senderId, final Long receiverId, final LikeLevel likeLevel) { | ||
this.senderId = senderId; | ||
this.receiverId = receiverId; | ||
this.likeLevel = likeLevel; | ||
this.likeIcon = LikeIcon.NONE; | ||
this.isRecent = true; | ||
} |
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.
LikeLevel에 대한 검증을 진행해야하지 않을까요?
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.
LikeLevel에 대한 검증은 정적 팩터리 메서드에서 생성할 때 LikeLevel.findByName
을 통해 검증하고 있는데, 혹시 다른 검증을 의미하신 것일까요?
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.
정적 팩토리 메서드에서 findByName으로 검증을 진행하셨군요! 제가 이 부분은 확인을 못했던 것 같습니다 ㅎㅎ
public void updateLikeIcon(final LikeIcon likeIcon) { | ||
this.likeIcon = likeIcon; | ||
} |
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.
updateLikeIcon
은 상대가 메시지를 보내거나, 서로 매칭이 성사될 때 이벤트를 통해 해당 호감 엔티티의 상태를 바꾸도록 만들 계획입니다. 그런 점에서 이벤트를 제외하고는 해당 메서드를 쓸 일이 없다고 판단하여 String 타입으로 받기보다는 직접적인 LikeIcon.HEART
등으로 이벤트 코드에서 작성할 생각이었는데 어떻게 생각하시는지 궁금합니다!
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.
매칭이 성사되면 이벤트로 LikeIcon의 상태를 바꾸는 건 좋은 방식인거 같습니다!
- 문법에 맞게 Exist를 Existence로 수정
- LikeLevel에 맞게 예외 이름 수정
- MemberLikeTest Nested class 적용
- 메서드에서 적용하지 못했던 final 키워드 적용
- LocalDateTime 메서드 체이닝 컨벤션 적용
- MemberLikeQueryRepositoryTest 반복 코드 메서드 추출
- MemberLikeQueryRepositoryTest 이용 메서드 수정
- total, start, end 변수를 직접 구하지 않고 QueryResults를 이용하여 주입하도록 수정
- RESTful 원칙에 맞게 경로 수정
- MemberLikeQueryRepositoryTest에서 발생한 정렬 관련 오류 수정
- MemberLikeFixture 날짜 주입 픽스처 추가 - Member 생성 시 initializeWith 사용하도록 수정
- 호감 조회 테스트 예외 수정되도록 변경
- MemberLikeQueryRepositoryTest의 시간 정렬 문제를 AuditingHandler 주입으로 해결
- 호감 정렬 기준에 id 추가함으로써 AuditingHandler 의존 제거
📄 Summary
호감 전송, 조회 기능을 완료했습니다.
🙋🏻 More
호감은 보낸 회원의 id, 받은 회원의 id, 호감 정도 (LikeLevel), 호감 아이콘 (LikeIcon), 최근 여부 (isRecent)로 구성되어 있습니다.
호감 전송
ValidatedMemberExistEvent
를 멤버 도메인에 전달하여, 받는 회원이 존재하는 회원인지도 검증합니다.호감 조회
기타
close #22