♻️ Refactor : feature/52 코드리뷰 적용

Team-Shaka · Oct 10, 2023 · 38d37c9 · 38d37c9
1 parent 088c02d
commit 38d37c9
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 25 deletions.
diff --git a/src/main/java/briefing/exception/ErrorCode.java b/src/main/java/briefing/exception/ErrorCode.java
@@ -48,6 +48,7 @@ public enum ErrorCode {
     // member 관련 에러
 
     MEMBER_NOT_FOUND(BAD_REQUEST, "MEMBER_400_1", "사용자가 없습니다"),
+    MEMBER_NOT_SAME(BAD_REQUEST, "MEMBER_002", "로그인 된 사용자와 대상 사용자가 일치하지 않습니다."),
 
     // member 에러
 
@@ -101,8 +102,8 @@ public static ErrorCode valueOf(HttpStatus httpStatus) {
                 });
     }
 
-    @Override
-    public String toString() {
-        return String.format("%s (%d)", this.name(), this.getCode());
-    }
+//    @Override
+//    public String toString() {
+//        return String.format("%s (%d)", this.name(), this.getCode());
+//    }
 }
diff --git a/src/main/java/briefing/member/api/MemberApi.java b/src/main/java/briefing/member/api/MemberApi.java
@@ -11,6 +11,7 @@
 import briefing.redis.service.RedisService;
 import briefing.security.handler.annotation.AuthMember;
 import briefing.security.provider.TokenProvider;
+import briefing.validation.annotation.CheckSameMember;
 import io.swagger.v3.oas.annotations.Operation;
 import io.swagger.v3.oas.annotations.Parameter;
 import io.swagger.v3.oas.annotations.Parameters;
@@ -70,12 +71,13 @@ public CommonResponse<MemberResponse.ReIssueTokenDTO> reissueToken(@Valid @Reque
     }
 
 
-    @DeleteMapping("/")
+    @DeleteMapping("/{memberId}")
     @Parameters({
-            @Parameter(name = "member", hidden = true)
+            @Parameter(name = "member", hidden = true),
+            @Parameter(name = "memberId", description = "삭제 대상 멤버아이디")
     })
-    public CommonResponse<MemberResponse.QuitDTO> quitMember(@AuthMember Member member){
-        memberCommandService.deleteMember(member);
+    public CommonResponse<MemberResponse.QuitDTO> quitMember(@AuthMember Member member, @CheckSameMember @PathVariable Long memberId){
+        memberCommandService.deleteMember(memberId);
         return CommonResponse.onSuccess(MemberConverter.toQuitDTO());
     }
 }
diff --git a/src/main/java/briefing/member/api/MemberConverter.java b/src/main/java/briefing/member/api/MemberConverter.java
@@ -19,14 +19,6 @@
 @Component
 public class MemberConverter {
 
-    private final MemberRepository memberRepository;
-
-    private static MemberRepository staticMemberRepository;
-    @PostConstruct
-    public void init() {
-        this.staticMemberRepository = this.memberRepository;
-    }
-
     public static MemberResponse.LoginDTO toLoginDTO(Member member, String accessToken, String refreshToken) {
         return MemberResponse.LoginDTO.builder()
                 .memberId(member.getId())
@@ -57,10 +49,6 @@ public static Member toMember(String appleSocialId) {
                 .build();
     }
 
-    public static Member toMember(Long memberId){
-        return staticMemberRepository.findById(memberId).orElseThrow(()->new MemberException(ErrorCode.MEMBER_NOT_FOUND));
-    }
-
     public static MemberResponse.ReIssueTokenDTO toReIssueTokenDTO(String accessToken, String refreshToken){
         return MemberResponse.ReIssueTokenDTO.builder()
                 .accessToken(accessToken)

diff --git a/src/main/java/briefing/member/application/MemberCommandService.java b/src/main/java/briefing/member/application/MemberCommandService.java
@@ -134,7 +134,7 @@ public Member parseRefreshToken(RefreshToken refreshToken){
         return memberRepository.findById(refreshToken.getMemberId()).orElseThrow(() -> new MemberException(ErrorCode.MEMBER_NOT_FOUND));
     }
 
-    public void deleteMember(Member member){
-        memberRepository.delete(member);
+    public void deleteMember(Long memberId){
+        memberRepository.delete(memberRepository.findById(memberId).get());
     }
 }
diff --git a/src/main/java/briefing/member/application/MemberQueryService.java b/src/main/java/briefing/member/application/MemberQueryService.java
@@ -1,5 +1,7 @@
 package briefing.member.application;
 
+import briefing.exception.ErrorCode;
+import briefing.exception.handler.MemberException;
 import briefing.member.domain.Member;
 import briefing.member.domain.MemberRole;
 import briefing.member.domain.SocialType;
@@ -8,14 +10,16 @@
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
+import java.util.Optional;
+
 @Service
 @Transactional(readOnly = true)
 @RequiredArgsConstructor
 public class MemberQueryService {
     private final MemberRepository memberRepository;
 
     public Member findMember(Long memberId){
-        return memberRepository.findById(memberId).get();
+        return memberRepository.findById(memberId).orElseThrow(()->new MemberException(ErrorCode.MEMBER_NOT_FOUND));
     }
 
     @Transactional

diff --git a/src/main/java/briefing/scrap/domain/Scrap.java b/src/main/java/briefing/scrap/domain/Scrap.java
@@ -26,7 +26,7 @@ public class Scrap extends BaseDateTimeEntity {
 
     public void setMember(Member member){
         if (this.member != null)
-            member.getScrapList().remove(this);
+            this.member.getScrapList().remove(this);
         this.member = member;
         member.getScrapList().add(this);
     }

diff --git a/src/main/java/briefing/security/handler/annotation/AuthUserArgumentResolver.java b/src/main/java/briefing/security/handler/annotation/AuthUserArgumentResolver.java
@@ -3,6 +3,7 @@
 import briefing.exception.ErrorCode;
 import briefing.exception.handler.MemberException;
 import briefing.member.api.MemberConverter;
+import briefing.member.application.MemberQueryService;
 import briefing.member.domain.Member;
 import lombok.RequiredArgsConstructor;
 import org.springframework.core.MethodParameter;
@@ -20,6 +21,8 @@
 public class AuthUserArgumentResolver implements HandlerMethodArgumentResolver {
 
 
+    private final MemberQueryService memberQueryService;
+
     @Override
     public boolean supportsParameter(MethodParameter parameter) {
         AuthMember authUser = parameter.getParameterAnnotation(AuthMember.class);
@@ -43,7 +46,7 @@ public Object resolveArgument(MethodParameter parameter, ModelAndViewContainer m
         }
 
         UsernamePasswordAuthenticationToken authenticationToken = (UsernamePasswordAuthenticationToken) authentication;
-        Member member = MemberConverter.toMember(Long.valueOf(authenticationToken.getName()));
+        Member member = memberQueryService.findMember(Long.valueOf(authenticationToken.getName()));
         return member;
     }
 }
diff --git a/src/main/java/briefing/validation/annotation/CheckSameMember.java b/src/main/java/briefing/validation/annotation/CheckSameMember.java
@@ -0,0 +1,17 @@
+package briefing.validation.annotation;
+
+import briefing.validation.validator.CheckSameMemberValidator;
+import jakarta.validation.Constraint;
+import jakarta.validation.Payload;
+
+import java.lang.annotation.*;
+
+@Documented
+@Constraint(validatedBy = CheckSameMemberValidator.class)
+@Target( { ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER })
+@Retention(RetentionPolicy.RUNTIME)
+public @interface CheckSameMember {
+    String message() default "로그인 한 사용자와 대상 사용자가 동일하지 않습니다.";
+    Class<?>[] groups() default {};
+    Class<? extends Payload>[] payload() default {};
+}
diff --git a/src/main/java/briefing/validation/validator/CheckSameMemberValidator.java b/src/main/java/briefing/validation/validator/CheckSameMemberValidator.java
@@ -0,0 +1,51 @@
+package briefing.validation.validator;
+
+import briefing.exception.ErrorCode;
+import briefing.exception.handler.MemberException;
+import briefing.member.domain.Member;
+import briefing.validation.annotation.CheckSameMember;
+import jakarta.validation.ConstraintValidator;
+import jakarta.validation.ConstraintValidatorContext;
+import lombok.RequiredArgsConstructor;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.stereotype.Component;
+
+import java.util.concurrent.RecursiveTask;
+
+@Component
+@RequiredArgsConstructor
+public class CheckSameMemberValidator implements ConstraintValidator<CheckSameMember, Long>{
+
+
+    @Override
+    public void initialize(CheckSameMember constraintAnnotation) {
+        ConstraintValidator.super.initialize(constraintAnnotation);
+    }
+
+    @Override
+    public boolean isValid(Long value, ConstraintValidatorContext context) {
+        Object principal = null;
+        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+
+        if (authentication != null) {
+            principal = authentication.getPrincipal();
+        }
+        if (principal == null || principal.getClass() == String.class) {
+            context.disableDefaultConstraintViolation();
+            context.buildConstraintViolationWithTemplate(ErrorCode.MEMBER_NOT_FOUND.toString()).addConstraintViolation();
+            return false;
+        }
+
+        UsernamePasswordAuthenticationToken authenticationToken = (UsernamePasswordAuthenticationToken) authentication;
+
+        // 로그인 한 사용자가 어드민인지 나중에 추가
+        if(!value.equals(Long.valueOf(authenticationToken.getName()))){
+            context.disableDefaultConstraintViolation();
+            context.buildConstraintViolationWithTemplate(ErrorCode.MEMBER_NOT_SAME.toString()).addConstraintViolation();
+            return false;
+        }
+        return true;
+    }
+}