Skip to content

Commit

Permalink
[OING-289] refactor: Member 모듈의 안티패턴과 짜잘한 부분 리팩토링 (#223)
Browse files Browse the repository at this point in the history
* refactor: Remove unused api parameter got by parsing token and reformat parameter sequence from MemberController. MemberApi

* refactor: Rename and reformat the methods in MemberController

* refactor: Rename and reformat the methods in MemberSerivce

* refactor: Fix the anti-pattern of MemberController by moving transactional code to MemberService

* refactor: Refactor minor code dirty

* refactor: Refactor legacy MemberControllerTest by removing unnessary tests and spliting to MemberServiceTest due to anti-pattern refactoring

* refactor: Resolve code smell issue, distinguishing AuthenticationFailedException by ErrorCode

* feat: Add MemberServiceTests for projections methods
  • Loading branch information
Kwon770 authored Apr 8, 2024
1 parent b45164b commit be800db
Show file tree
Hide file tree
Showing 19 changed files with 474 additions and 363 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package com.oing.exception;

public class AuthorizationFailedException extends DomainException {
public AuthorizationFailedException(ErrorCode errorCode) {
super(errorCode);
}

public AuthorizationFailedException() {
super(ErrorCode.AUTHORIZATION_FAILED);
}
Expand Down
3 changes: 2 additions & 1 deletion common/src/main/java/com/oing/exception/ErrorCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,14 @@ public enum ErrorCode {
AUTHORIZATION_FAILED("AU0003", "권한이 없습니다."),
REFRESH_TOKEN_INVALID("AU0004", "유효하지 않은 리프레쉬 토큰입니다."),
TOKEN_EXPIRED("AU0005", "토큰이 만료됐습니다."),
UNAUTHORIZED_MEMBER_USED("AU0006", "인증되지 않은/권한이 없는 회원에 접근하였습니다."),

/**
* Member Related Errors
*/
MEMBER_NOT_FOUND("MB0001", "찾을 수 없는 회원입니다."),
MEMBER_ALREADY_EXISTS("MB0002", "이미 존재하는 회원입니다."),

INVALID_MEMBER_NAME_LENGTH("MB0003", "회원 이름의 길이가 유효하지 않습니다."),
/**
* Post Related Errors
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private List<CalendarResponse> convertToCalendarResponse(List<Post> daysLatestPo

// 탈퇴한 회원을 제외하고 allFamilyMembersUploaded 기본값이 true이므로, 탈퇴한 회원이 allFamilyMembersUploaded 계산에 영향을 미치지 않음
// edge case: 글을 업로드하지 않은 회원이 탈퇴하면, 과거 날짜들의 allFamilyMembersUploaded이 true로 변함 -> 핸들링할 수 없는 케이스
List<String> familyMembersIds = memberService.findFamilyMembersIdsByFamilyJoinAtBefore(familyId, postDate.plusDays(1));
List<String> familyMembersIds = memberService.getFamilyMembersIdsByFamilyIdAndJoinAtBefore(familyId, postDate.plusDays(1));
boolean allFamilyMembersUploaded = true;
for (String memberId : familyMembersIds) {
if (!postService.existsByMemberIdAndFamilyIdAndCreatedAt(memberId, familyId, postDate)) {
Expand Down Expand Up @@ -95,7 +95,7 @@ public BannerResponse getBanner(String yearMonth, String familyId) {
boolean allFamilyMembersUploaded = true;

if (postService.existsByFamilyIdAndCreatedAt(familyId, startDate)) {
List<String> familyMembersIds = memberService.findFamilyMembersIdsByFamilyJoinAtBefore(familyId, startDate.plusDays(1));
List<String> familyMembersIds = memberService.getFamilyMembersIdsByFamilyIdAndJoinAtBefore(familyId, startDate.plusDays(1));
for (String memberId : familyMembersIds) {
if (!postService.existsByMemberIdAndFamilyIdAndCreatedAt(memberId, familyId, startDate)) {
allFamilyMembersUploaded = false;
Expand Down Expand Up @@ -167,7 +167,7 @@ public FamilyMonthlyStatisticsResponse getSummary(String yearMonth, String login
int year = Integer.parseInt(yearMonthArray[0]);
int month = Integer.parseInt(yearMonthArray[1]);

String familyId = memberService.findFamilyIdByMemberId(loginMemberId);
String familyId = memberService.getFamilyIdByMemberId(loginMemberId);
long monthlyPostCount = postService.countMonthlyPostByFamilyId(year, month, familyId);
return new FamilyMonthlyStatisticsResponse((int) monthlyPostCount);
}
Expand Down
8 changes: 4 additions & 4 deletions gateway/src/main/java/com/oing/controller/MeController.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class MeController implements MeApi {

@Override
public MemberResponse getMe(String loginMemberId) {
Member member = memberService.findMemberById(loginMemberId);
Member member = memberService.getMemberByMemberId(loginMemberId);
return MemberResponse.of(member);
}

Expand Down Expand Up @@ -68,7 +68,7 @@ public FamilyResponse joinFamily(JoinFamilyRequest request, String loginMemberId
FamilyInviteLink link = familyInviteLinkService.retrieveDeepLinkDetails(request.inviteCode());
Family targetFamily = familyService.getFamilyById(link.getFamilyId());

Member member = memberService.findMemberById(loginMemberId);
Member member = memberService.getMemberByMemberId(loginMemberId);
// TODO: iOS 업데이트 이슈로 온보딩 플로우에 갖힌 유저를 위해 일시적으로 예외 핸들링 주석 처리 !!!
// if (member.hasFamily()) throw new AlreadyInFamilyException();
member.setFamilyId(targetFamily.getId());
Expand All @@ -81,7 +81,7 @@ public FamilyResponse joinFamily(JoinFamilyRequest request, String loginMemberId
@Override
public FamilyResponse createFamilyAndJoin(String loginMemberId) {
log.info("Member {} is trying to create a family", loginMemberId);
Member member = memberService.findMemberById(loginMemberId);
Member member = memberService.getMemberByMemberId(loginMemberId);
// TODO: iOS 업데이트 이슈로 온보딩 플로우에 갖힌 유저를 위해 일시적으로 예외 핸들링 주석 처리 !!!
// if (member.hasFamily()) throw new AlreadyInFamilyException();

Expand All @@ -102,7 +102,7 @@ public AppVersionResponse getCurrentAppVersion(UUID appKey) {
@Override
public DefaultResponse quitFamily(String loginMemberId) {
log.info("Member {} is trying to quit from family", loginMemberId);
Member member = memberService.findMemberById(loginMemberId);
Member member = memberService.getMemberByMemberId(loginMemberId);
if (!member.hasFamily()) {
log.warn("Member {} is not in any family", loginMemberId);
throw new FamilyNotFoundException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public ResponseEntity<SingleRecentPostWidgetResponse> getSingleRecentFamilyPostW
}

Post latestPost = latestPosts.get(0);
Member author = memberService.findMemberById(latestPost.getMemberId());
Member author = memberService.getMemberByMemberId(latestPost.getMemberId());
return ResponseEntity.ok(new SingleRecentPostWidgetResponse(
author.getName(),
optimizedImageUrlGenerator.getKBImageUrlGenerator(author.getProfileImgUrl()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ public class FamilyNotificationEventListener {
@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
public void onPostCreatedEvent(PostCreatedEvent postCreatedEvent) {
if (postCreatedEvent.getSource() instanceof Post post) {
Member author = memberService.findMemberById(post.getMemberId());
Member author = memberService.getMemberByMemberId(post.getMemberId());
HashSet<String> targetFcmTokens = new HashSet<>();
String familyId = post.getFamilyId();
List<String> familyMemberIds = memberService.findFamilyMembersIdsByFamilyId(familyId);
List<String> familyMemberIds = memberService.getFamilyMembersIdsByFamilyId(familyId);
for (String familyMemberId : familyMemberIds) {
if(post.getMemberId().equals(familyMemberId)) continue; //게시물 작성자는 발송 X
targetFcmTokens.addAll(memberDeviceService.getFcmTokensByMemberId(familyMemberId));
Expand Down Expand Up @@ -62,7 +62,7 @@ public void onPostCommentCreatedEvent(CommentCreatedEvent commentCreatedEvent) {
if (commentCreatedEvent.getSource() instanceof Comment comment) {
Post sourcePost = comment.getPost();
String postAuthorId = sourcePost.getMemberId(); //게시물 작성자 ID
Member author = memberService.findMemberById(comment.getMemberId()); //댓글 작성자
Member author = memberService.getMemberByMemberId(comment.getMemberId()); //댓글 작성자

if (!postAuthorId.equals(comment.getMemberId())) { //내가 내 게시물에 단 댓글이 아니라면
List<String> targetFcmTokens = memberDeviceService.getFcmTokensByMemberId(postAuthorId);
Expand Down
4 changes: 2 additions & 2 deletions gateway/src/main/java/com/oing/job/DailyNotificationJob.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public void sendDailyUploadNotification() {

try {
HashSet<String> targetFcmTokens = new HashSet<>();
List<Member> members = memberService.findAllMember();
List<Member> members = memberService.findAllActiveMembers();
for (Member member : members) {
targetFcmTokens.addAll(memberDeviceService.getFcmTokensByMemberId(member.getId()));
}
Expand Down Expand Up @@ -91,7 +91,7 @@ public void sendDailyRemainingNotification() {

try {
LocalDate today = LocalDate.now();
List<Member> allMembers = memberService.findAllMember();
List<Member> allMembers = memberService.findAllActiveMembers();
HashSet<String> targetFcmTokens = new HashSet<>();
HashSet<String> postedMemberIds = new HashSet<>(postService.findMemberIdsPostedToday(today));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class WidgetControllerTest {
String familyId = testMember1.getFamilyId();

when(postService.findLatestPostOfEveryday(LocalDate.parse(date), LocalDate.parse(date).plusDays(1), familyId)).thenReturn(List.of(testPost1));
when(memberService.findMemberById(testPost1.getMemberId())).thenReturn(testMember1);
when(memberService.getMemberByMemberId(testPost1.getMemberId())).thenReturn(testMember1);
when(optimizedImageUrlGenerator.getKBImageUrlGenerator(testMember1.getProfileImgUrl())).thenReturn(testMember1.getProfileImgUrl());
when(optimizedImageUrlGenerator.getKBImageUrlGenerator(testPost1.getPostImgUrl())).thenReturn(testPost1.getPostImgUrl());

Expand Down Expand Up @@ -105,7 +105,7 @@ class WidgetControllerTest {
String familyId = testMember1.getFamilyId();

when(postService.findLatestPostOfEveryday(LocalDate.now(), LocalDate.now().plusDays(1), familyId)).thenReturn(List.of(testPost1));
when(memberService.findMemberById(testPost1.getMemberId())).thenReturn(testMember1);
when(memberService.getMemberByMemberId(testPost1.getMemberId())).thenReturn(testMember1);
when(optimizedImageUrlGenerator.getKBImageUrlGenerator(testMember1.getProfileImgUrl())).thenReturn(testMember1.getProfileImgUrl());
when(optimizedImageUrlGenerator.getKBImageUrlGenerator(testPost1.getPostImgUrl())).thenReturn(testPost1.getPostImgUrl());

Expand Down
120 changes: 112 additions & 8 deletions gateway/src/test/java/com/oing/restapi/MemberApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
@Transactional
@ActiveProfiles("test")
@AutoConfigureMockMvc
public class MemberApiTest {
class MemberApiTest {
@Autowired
private MockMvc mockMvc;

Expand All @@ -42,10 +42,14 @@ public class MemberApiTest {
@Autowired
private MemberRepository memberRepository;

private String TEST_MEMBER1_ID = "01HGW2N7EHJVJ4CJ999RRS2E97";
private String TEST_MEMBER2_ID = "01HGW2N7EHJVJ4CJ99IIFIFE94";
private String TEST_FAMILY_ID = "01HGW2N7EHJVJ4CJ999RRS2E44";
private String TEST_MEMBER1_ID = "id1";
private String TEST_MEMBER1_TOKEN;
private String TEST_MEMBER2_ID = "id2";
private String TEST_MEMBER2_TOKEN;
private String TEST_FAMILY_ID = "family1";
private String TEST_MEMBER3_ID = "id3";
private String TEST_MEMBER3_TOKEN;



@BeforeEach
Expand All @@ -62,6 +66,7 @@ void setUp() {
TEST_MEMBER1_TOKEN = tokenGenerator
.generateTokenPair(TEST_MEMBER1_ID)
.accessToken();

memberRepository.save(
new Member(
TEST_MEMBER2_ID,
Expand All @@ -71,10 +76,26 @@ void setUp() {
LocalDateTime.now()
)
);
TEST_MEMBER2_TOKEN = tokenGenerator
.generateTokenPair(TEST_MEMBER2_ID)
.accessToken();

memberRepository.save(
new Member(
TEST_MEMBER3_ID,
"different-family",
LocalDate.now(),
"타인", "http://test.com/test-profile.jpg", "/test-profile.jpg",
LocalDateTime.now()
)
);
TEST_MEMBER3_TOKEN = tokenGenerator
.generateTokenPair(TEST_MEMBER3_ID)
.accessToken();
}

@Test
void 가족_멤버_프로필_조회_테스트() throws Exception {
void 가족_회원_프로필_조회_테스트() throws Exception {
// given

// when
Expand All @@ -93,7 +114,7 @@ void setUp() {
}

@Test
void 멤버_프로필_조회_테스트() throws Exception {
void 회원_프로필_조회_테스트() throws Exception {
// given

// when
Expand All @@ -111,7 +132,22 @@ void setUp() {
}

@Test
void 멤버_프로필이미지_수정_테스트() throws Exception {
void 다른_가족의_회원_프로필_조회_예외_검증() throws Exception {
// when
ResultActions resultActions = mockMvc.perform(
get("/v1/members/{memberId}", TEST_MEMBER1_ID)
.header("X-AUTH-TOKEN", TEST_MEMBER3_TOKEN)
.contentType(MediaType.APPLICATION_JSON)
);

// then
resultActions
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("AU0006"));
}

@Test
void 회원_프로필이미지_수정_테스트() throws Exception {
// given
UpdateMemberProfileImageUrlRequest request = new UpdateMemberProfileImageUrlRequest("https://bucket.com/new-image.jpg");

Expand All @@ -130,6 +166,25 @@ void setUp() {
.andExpect(jsonPath("$.imageUrl").value(request.profileImageUrl()));
}

@Test
void 타인의_프로필이미지_수정_예외_검증() throws Exception {
// given
UpdateMemberProfileImageUrlRequest request = new UpdateMemberProfileImageUrlRequest("https://bucket.com/new-image.jpg");

// when
ResultActions resultActions = mockMvc.perform(
put("/v1/members/profile-image-url/{memberId}", TEST_MEMBER1_ID)
.header("X-AUTH-TOKEN", TEST_MEMBER2_TOKEN)
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request))
);

// then
resultActions
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("AU0006"));
}

@Test
void 회원_프로필이미지_삭제_테스트() throws Exception {
// given
Expand All @@ -147,7 +202,22 @@ void setUp() {
}

@Test
void 멤버_닉네임_수정_테스트() throws Exception {
void 타인의_프로필이미지_삭제_예외_검증() throws Exception {
// when
ResultActions resultActions = mockMvc.perform(
delete("/v1/members/profile-image-url/{memberId}", TEST_MEMBER1_ID)
.header("X-AUTH-TOKEN", TEST_MEMBER2_TOKEN)
.contentType(MediaType.APPLICATION_JSON)
);

// then
resultActions
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("AU0006"));
}

@Test
void 회원_닉네임_수정_테스트() throws Exception {
// given
UpdateMemberNameRequest request = new UpdateMemberNameRequest("newName");

Expand All @@ -166,6 +236,25 @@ void setUp() {
.andExpect(jsonPath("$.name").value(request.name()));
}

@Test
void 타인의_닉네임_수정_예외_검증() throws Exception {
// given
UpdateMemberNameRequest request = new UpdateMemberNameRequest("newName");

// when
ResultActions resultActions = mockMvc.perform(
put("/v1/members/name/{memberId}", TEST_MEMBER1_ID)
.header("X-AUTH-TOKEN", TEST_MEMBER3_TOKEN)
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request))
);

// then
resultActions
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("AU0006"));
}

@Test
void 회원탈퇴_이유없이_테스트() throws Exception {
// given
Expand Down Expand Up @@ -221,4 +310,19 @@ void setUp() {
.andExpect(status().isOk())
.andExpect(jsonPath("$.success").value(true));
}

@Test
void 타인의_탈퇴_예외_검증() throws Exception {
// when
ResultActions resultActions = mockMvc.perform(
delete("/v1/members/{memberId}", TEST_MEMBER1_ID)
.header("X-AUTH-TOKEN", TEST_MEMBER2_TOKEN)
.contentType(MediaType.APPLICATION_JSON)
);

// then
resultActions
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.code").value("AU0006"));
}
}
4 changes: 2 additions & 2 deletions member/src/main/java/com/oing/controller/AuthController.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public AuthResultResponse socialLogin(String provider, NativeSocialLoginRequest

// 위 결과에서 나온 identifier로 이미 있는 사용자인지 확인
Optional<Member> member = memberService
.findMemberBySocialMemberKey(socialLoginProvider, socialLoginResult.identifier());
.getMemberBySocialMemberKey(socialLoginProvider, socialLoginResult.identifier());
if (member.isEmpty()) {
//회원가입이 안된 경우 임시 토큰 발행
TokenPair temporaryTokenPair = tokenGenerator
Expand Down Expand Up @@ -75,7 +75,7 @@ public AuthResultResponse register(Authentication authentication, CreateNewMembe

// identifier로 이미 있는 사용자인지 확인
Optional<Member> preExistsMember = memberService
.findMemberBySocialMemberKey(provider, token.userId());
.getMemberBySocialMemberKey(provider, token.userId());
if (preExistsMember.isPresent()) {
throw new MemberAlreadyExistsException();
}
Expand Down
Loading

0 comments on commit be800db

Please sign in to comment.