Skip to content

Commit

Permalink
Merge pull request #539 from woowacourse-teams/hotfix/#537,#538
Browse files Browse the repository at this point in the history
[BE] Hotfix/#537,#538 ํšŒ์› ์ •๋ณด ๊ด€๋ จ ์ด์Šˆ ํ•ด๊ฒฐ
  • Loading branch information
yoondgu authored Oct 2, 2023
2 parents 0c0d6b5 + 9fbaefd commit bbed204
Show file tree
Hide file tree
Showing 14 changed files with 52 additions and 60 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.mapbefine.mapbefine.auth.domain;

import com.mapbefine.mapbefine.member.domain.Role;
import com.mapbefine.mapbefine.topic.domain.Topic;
import java.util.List;
import java.util.Objects;

public abstract class AuthMember {

Expand All @@ -27,11 +27,13 @@ protected AuthMember(
public abstract boolean canTopicUpdate(Topic topic);

public abstract boolean canPinCreateOrUpdate(Topic topic);

public abstract boolean isRole(Role role);


public Long getMemberId() {
return memberId;
}

public boolean isSameMember(Long memberId) {
return Objects.equals(memberId, this.memberId);
}

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.mapbefine.mapbefine.auth.domain.member;

import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.member.domain.Role;
import com.mapbefine.mapbefine.topic.domain.Topic;
import java.util.Collections;

Expand Down Expand Up @@ -35,9 +34,4 @@ public boolean canPinCreateOrUpdate(Topic topic) {
return true;
}

@Override
public boolean isRole(Role role) {
return Role.ADMIN == role;
}

}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.mapbefine.mapbefine.auth.domain.member;

import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.member.domain.Role;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicStatus;
import java.util.Collections;
Expand Down Expand Up @@ -37,8 +36,4 @@ public boolean canPinCreateOrUpdate(Topic topic) {
return false;
}

@Override
public boolean isRole(Role role) {
return Role.GUEST == role;
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.mapbefine.mapbefine.auth.domain.member;

import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.member.domain.Role;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicStatus;
import java.util.List;
Expand Down Expand Up @@ -55,9 +54,4 @@ private boolean hasPermission(Long topicId) {
return createdTopic.contains(topicId) || topicsWithPermission.contains(topicId);
}

@Override
public boolean isRole(Role role) {
return Role.USER == role;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public boolean preHandle(
if (!(handler instanceof HandlerMethod handlerMethod)) {
return true;
}
if (isAuthMemberNotRequired(handlerMethod)) {
if (isAuthMemberNotRequired(handlerMethod) && isLoginNotRequired(handlerMethod)) {
return true;
}

Expand All @@ -60,6 +60,10 @@ private boolean isAuthMemberNotRequired(HandlerMethod handlerMethod) {
.noneMatch(parameter -> parameter.getParameterType().equals(AuthMember.class));
}

private boolean isLoginNotRequired(HandlerMethod handlerMethod) {
return !isLoginRequired(handlerMethod);
}

private boolean isLoginRequired(HandlerMethod handlerMethod) {
LoginRequired loginRequired = handlerMethod.getMethodAnnotation(LoginRequired.class);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,16 @@
import com.mapbefine.mapbefine.member.dto.response.MemberDetailResponse;
import com.mapbefine.mapbefine.member.dto.response.MemberResponse;
import com.mapbefine.mapbefine.member.exception.MemberErrorCode;
import com.mapbefine.mapbefine.member.exception.MemberException.MemberForbiddenException;
import com.mapbefine.mapbefine.member.exception.MemberException.MemberNotFoundException;
import com.mapbefine.mapbefine.pin.dto.response.PinResponse;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import com.mapbefine.mapbefine.topic.dto.response.TopicResponse;
import java.util.List;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.List;

@Service
@Transactional(readOnly = true)
public class MemberQueryService {
Expand All @@ -41,29 +41,33 @@ public MemberQueryService(
this.topicRepository = topicRepository;
}

public MemberDetailResponse findById(Long id) {
Member member = findMemberById(id);

return MemberDetailResponse.from(member);
public MemberDetailResponse findById(AuthMember authMember, Long id) {
if (authMember.isSameMember(id)) {
Member member = findMemberById(id);
return MemberDetailResponse.from(member);
}
throw new MemberForbiddenException(MemberErrorCode.FORBIDDEN_ACCESS, id);
}

private Member findMemberById(Long id) {
return memberRepository.findById(id)
.filter(Member::isNormalStatus)
.orElseThrow(() -> new MemberNotFoundException(MemberErrorCode.MEMBER_NOT_FOUND, id));
}

// TODO: 2023/09/13 ์ฐจ๋‹จ๋œ or ํƒˆํ‡ดํ•œ ์‚ฌ์šฉ์ž ํ•„ํ„ฐ๋ง ํ•„์š”
public List<MemberResponse> findAll() {
return memberRepository.findAll()
.stream()
.filter(Member::isNormalStatus)
.map(MemberResponse::from)
.toList();
}

public List<TopicResponse> findAllTopicsInBookmark(AuthMember authMember) {
Member member = findMemberById(authMember.getMemberId());

List<Topic> bookMarkedTopics = topicRepository.findTopicsByBookmarksMemberIdAndIsDeletedFalse(authMember.getMemberId());
List<Topic> bookMarkedTopics = topicRepository.findTopicsByBookmarksMemberIdAndIsDeletedFalse(
authMember.getMemberId());
return bookMarkedTopics.stream()
.map(topic -> TopicResponse.from(
topic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,17 @@
import com.mapbefine.mapbefine.member.domain.Member;
import com.mapbefine.mapbefine.member.domain.MemberInfo;

public record MemberResponse (
public record MemberResponse(
Long id,
String nickName,
String email
String nickName
) {

public static MemberResponse from(Member member) {
MemberInfo memberInfo = member.getMemberInfo();

return new MemberResponse(
member.getId(),
memberInfo.getNickName(),
memberInfo.getEmail()
memberInfo.getNickName()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public enum MemberErrorCode {
ILLEGAL_EMAIL_NULL("05002", "์ด๋ฉ”์ผ์€ ํ•„์ˆ˜๋กœ ์ž…๋ ฅํ•ด์•ผํ•ฉ๋‹ˆ๋‹ค."),
ILLEGAL_EMAIL_PATTERN("05003", "์˜ฌ๋ฐ”๋ฅด์ง€ ์•Š์€ ์ด๋ฉ”์ผ ํ˜•์‹์ž…๋‹ˆ๋‹ค."),
FORBIDDEN_MEMBER_STATUS("05100", "ํƒˆํ‡ด ํ˜น์€ ์ฐจ๋‹จ๋œ ํšŒ์›์ž…๋‹ˆ๋‹ค."),
FORBIDDEN_ACCESS("05101", "ํ•ด๋‹น ํšŒ์› ์ •๋ณด์— ๋Œ€ํ•œ ์ ‘๊ทผ ๊ถŒํ•œ์ด ์—†์Šต๋‹ˆ๋‹ค."),
MEMBER_NOT_FOUND("05400", "์กด์žฌํ•˜์ง€ ์•Š๋Š” ํšŒ์›์ž…๋‹ˆ๋‹ค."),
ILLEGAL_NICKNAME_ALREADY_EXISTS("05900", "์ด๋ฏธ ์กด์žฌํ•˜๋Š” ๋‹‰๋„ค์ž„์ž…๋‹ˆ๋‹ค."),
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public ResponseEntity<List<MemberResponse>> findAllMember() {

@LoginRequired
@GetMapping("/{memberId}")
public ResponseEntity<MemberDetailResponse> findMemberById(@PathVariable Long memberId) {
MemberDetailResponse response = memberQueryService.findById(memberId);
public ResponseEntity<MemberDetailResponse> findMemberById(AuthMember authMember, @PathVariable Long memberId) {
MemberDetailResponse response = memberQueryService.findById(authMember, memberId);

return ResponseEntity.ok(response);
}
Expand Down
2 changes: 1 addition & 1 deletion backend/src/main/resources/data-default.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ VALUES ('dummyMember', 'dummy@gmail.com', 'https://map-befine-official.github.io
1L, 'KAKAO',
now(), now());

INSERT INTO topic (name, image_url, description,๊ฐ€
INSERT INTO topic (name, image_url, description,
permission_type, publicity,
member_id,
created_at, updated_at, last_pin_updated_at)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.mapbefine.mapbefine.member.application;

import static com.mapbefine.mapbefine.member.domain.Role.USER;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

Expand All @@ -19,7 +20,7 @@
import com.mapbefine.mapbefine.member.domain.Role;
import com.mapbefine.mapbefine.member.dto.response.MemberDetailResponse;
import com.mapbefine.mapbefine.member.dto.response.MemberResponse;
import com.mapbefine.mapbefine.member.exception.MemberException.MemberNotFoundException;
import com.mapbefine.mapbefine.member.exception.MemberException.MemberForbiddenException;
import com.mapbefine.mapbefine.pin.PinFixture;
import com.mapbefine.mapbefine.pin.domain.Pin;
import com.mapbefine.mapbefine.pin.domain.PinRepository;
Expand Down Expand Up @@ -81,10 +82,10 @@ private void createTopics(Member member) {
void findAllMember() {
// given
Member member2 = memberRepository.save(
MemberFixture.create("member2", "member2@member.com", Role.USER)
MemberFixture.create("member2", "member2@member.com", USER)
);
Member member3 = memberRepository.save(
MemberFixture.create("member3", "member3@member.com", Role.USER)
MemberFixture.create("member3", "member3@member.com", USER)
);

// when
Expand All @@ -103,26 +104,29 @@ void findAllMember() {
@DisplayName("ํšŒ์›์„ ๋‹จ์ผ ์กฐํšŒํ•œ๋‹ค.")
void findMemberById() {
// given
Member member = memberRepository.save(
MemberFixture.create("member", "member@naver.com", Role.USER)
);

// when
MemberDetailResponse response = memberQueryService.findById(member.getId());
MemberDetailResponse response = memberQueryService.findById(authMember, member.getId());

// then
assertThat(response).usingRecursiveComparison()
.isEqualTo(MemberDetailResponse.from(member));
}

@Test
@DisplayName("์กฐํšŒํ•˜๋ ค๋Š” ํšŒ์›์ด ์—†๋Š” ๊ฒฝ์šฐ ์˜ˆ์™ธ๋ฅผ ๋ฐ˜ํ™˜ํ•œ๋‹ค.")
@DisplayName("์กฐํšŒํ•˜๋ ค๋Š” ํšŒ์›์ด ์—†๋Š” ๊ฒฝ์šฐ ๋ณธ์ธ์ด ์•„๋‹ˆ๋ฏ€๋กœ ์˜ˆ์™ธ๋ฅผ ๋ฐ˜ํ™˜ํ•œ๋‹ค.")
void findMemberById_whenNoneExists_thenFail() {
// given when then
assertThatThrownBy(() -> memberQueryService.findById(Long.MAX_VALUE))
.isInstanceOf(MemberNotFoundException.class);
assertThatThrownBy(() -> memberQueryService.findById(authMember, Long.MAX_VALUE))
.isInstanceOf(MemberForbiddenException.class);
}

@Test
@DisplayName("์กฐํšŒํ•˜๋ ค๋Š” ํšŒ์›์ด ๋ณธ์ธ์ด ์•„๋‹Œ ๊ฒฝ์šฐ ์˜ˆ์™ธ๋ฅผ ๋ฐ˜ํ™˜ํ•œ๋‹ค.")
void findMemberById_whenNotSameMember_thenFail() {
// given when then
assertThatThrownBy(() -> memberQueryService.findById(authMember, Long.MAX_VALUE))
.isInstanceOf(MemberForbiddenException.class);
}

@Test
@DisplayName("์ฆ๊ฒจ์ฐพ๊ธฐ ๋ชฉ๋ก์— ์ถ”๊ฐ€ ๋œ ํ† ํ”ฝ์„ ์กฐํšŒํ•  ์ˆ˜ ์žˆ๋‹ค")
Expand Down Expand Up @@ -158,7 +162,7 @@ void findAtlasByMember_Success() {
}

@Test
@DisplayName("")
@DisplayName("๋กœ๊ทธ์ธํ•œ ํšŒ์›์˜ ๋ชจ๋“  ์ง€๋„๋ฅผ ๊ฐ€์ ธ์˜ฌ ์ˆ˜ ์žˆ๋‹ค.")
void findMyAllTopics_Success() {
//when
List<TopicResponse> myAllTopics = memberQueryService.findMyAllTopics(authMember);
Expand Down Expand Up @@ -198,5 +202,5 @@ void findMyAllPins_Success() {
.isEqualTo(pinIds);

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ void createMemberResponse_success() {
// then
assertThat(memberResponse.nickName())
.isEqualTo(member.getMemberInfo().getNickName());
assertThat(memberResponse.email())
.isEqualTo(member.getMemberInfo().getEmail());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,11 @@ void findAllMember() throws Exception {
List<MemberResponse> memberResponses = List.of(
new MemberResponse(
1L,
"member1",
"member1@member.com"
"member1"
),
new MemberResponse(
2L,
"member2",
"member2@member.com"
"member2"
)
);

Expand All @@ -60,7 +58,7 @@ void findMemberById() throws Exception {
LocalDateTime.now()
);

given(memberQueryService.findById(any())).willReturn(memberDetailResponse);
given(memberQueryService.findById(any(), any())).willReturn(memberDetailResponse);

mockMvc.perform(
MockMvcRequestBuilders.get("/members/1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ void deletePermission() throws Exception {
@DisplayName("ํŠน์ • ํ† ํ”ฝ ์ ‘๊ทผ ์ •๋ณด ์กฐํšŒ(๊ถŒํ•œ ํšŒ์› ๋ชฉ๋ก, ๊ณต๊ฐœ ์—ฌ๋ถ€)")
void findTopicAccessDetailByTopicId() throws Exception {
List<PermissionedMemberResponse> permissionedMembers = List.of(
new PermissionedMemberResponse(1L, new MemberResponse(1L, "member", "member@naver.com")),
new PermissionedMemberResponse(1L, new MemberResponse(2L, "memberr", "memberr@naver.com"))
new PermissionedMemberResponse(1L, new MemberResponse(1L, "member")),
new PermissionedMemberResponse(1L, new MemberResponse(2L, "memberr"))
);
TopicAccessDetailResponse response = new TopicAccessDetailResponse(Publicity.PUBLIC, permissionedMembers);

Expand Down

0 comments on commit bbed204

Please sign in to comment.