이노베이션 5주차 3 - 권한 부여, 도메인과 비즈니스 로직 분리

@Controller
@RequestMapping("/api/auth")
@RequiredArgsConstructor
public class UserController {

    private final UserService userService;

    @ResponseBody
    @GetMapping
    public String home() {
        return "home";
    }

    @PostMapping("/signup")
    public ResponseEntity<String> signup(@RequestBody @Valid SignupRequestDto signupRequestDto) {
        String message = userService.signup(signupRequestDto);
        return new ResponseEntity<>(message, HttpStatus.OK);
    }

    @PostMapping("/signup/secureadmin")
    public ResponseEntity<String> signupAdmin(@RequestBody @Valid SignupRequestDto signupRequestDto) {
        signupRequestDto.setAdmin(true);
        String message = userService.signup(signupRequestDto);
        return new ResponseEntity<>(message, HttpStatus.OK);
    }
}

@Service
@Slf4j
@RequiredArgsConstructor
public class UserService {

    private final UserRepository userRepository;
    private final PasswordEncoder passwordEncoder;
    private static final String ADMIN_TOKEN = "ASDdsfkjfigdajdaejifalkdjfDFDAK";

    public String signup(SignupRequestDto signupRequestDto) {
        String username = signupRequestDto.getUsername();
        String password = passwordEncoder.encode(signupRequestDto.getPassword());

        checkDuplicatedUsername(username);
        UserRoleEnum role = getUserRoleEnum(signupRequestDto);

        User user = new User(username, password, role);
        userRepository.save(user);

        return "회원가입 성공";
    }

    // 회원 중복 확인
    private void checkDuplicatedUsername(String username) {
        Optional<User> found = userRepository.findByUsername(username);
        if (found.isPresent()) {
            log.error("중복사용자 존재");
            throw new IllegalArgumentException("중복된 사용자가 존재합니다.");
        }
    }

    // 사용자 ROLE 확인 (관리자인 경우, ADMIN / 사옹자인 경우, USER 부여)
    private UserRoleEnum getUserRoleEnum(SignupRequestDto signupRequestDto) {
        UserRoleEnum role = UserRoleEnum.USER;
        if (signupRequestDto.isAdmin()) {
            if (!signupRequestDto.getAdminToken().equals(ADMIN_TOKEN)) {
                throw new IllegalArgumentException("관리자 암호가 틀려 등록이 불가능합니다.");
            }
            role = UserRoleEnum.ADMIN;
        }
        return role;
    }
}

관리자 권한은 관리자용 회원가입 페이지를 따로 만들어 입력한 관리자 토큰이 일치하는지 확인하고 해당 토큰이 일치한다면 권한을 부여해주는 방식으로 진행했다. 해당 부분은 이 정도로만 진행하고 핵심 비즈니스 로직에 대해서 고민이 생겨 관련 부분들에 대해서 좀 더 고민해보는 것이 좋을 것 같다고 생각했다.

---

V1

    public PostResponseDto updatePost(Long postId, PostRequestDto postRequestDto, User user) {
        Post post = findPost(postId);
        if (user.getId() == (post.getUser().getId())) {
            post.update(postRequestDto);
            return new PostResponseDto(post);
        } else throw new IllegalArgumentException("해당 게시글 작성자만 수정할 수 있습니다");
    }

    public String deletePost(Long postID, User user) {
        Post post = findPost(postID);
        if (user.getId() == (post.getUser().getId())) {
            postRepository.delete(post);
            return "삭제 완료";
        } else throw new IllegalArgumentException("해당 게시글 작성자만 삭제할 수 있습니다");
    }

원래 기존에 작성한 PostService의 유저 아이디와 해당 포스트를 작성한 유저의 아이디를 비교하여 일치하면 수정, 삭제 작업을하는 비즈니스 로직이었다. 다만 중복되는 코드가 많고 해당 코드가 도메인 내부로 들어가면 서비스 계층이 좀 더 간결한 코드가 되지 않을까 싶어 해당 비즈니스 로직들을 도메인에서 처리하도록 수정해보았다.

---

v2

    private void update(PostRequestDto postRequestDto) {
        this.title = postRequestDto.getTitle();
        this.description = postRequestDto.getDescription();
    }

    public Post changePost(PostRequestDto postRequestDto, User user) {
        if (user.getId() != this.getUser().getId() && user.getRole().getAuthority() == "ROLE_USER") throw new IllegalArgumentException("해당 게시글 작성자 혹은 관리자만 수정할 수 있습니다");
        this.update(postRequestDto);
        return this;
    }

    public Post checkDeleteablePost(User user) {
        if (user.getId() != this.getUser().getId() && user.getRole().getAuthority() == "ROLE_USER") throw new IllegalArgumentException("해당 게시글 작성자 혹은 관리자만 삭제할 수 있습니다");
        return this;
    }

Post Entity

 

    public PostResponseDto updatePost(Long postId, PostRequestDto postRequestDto, User user) {
        Post post = findPost(postId).changePost(postRequestDto, user);
        return new PostResponseDto(post);
    }

    public String deletePost(Long postID, User user) {
        Post post = findPost(postID).checkDeleteablePost(user);
        postRepository.delete(post);
        return "삭제완료";
    }

새롭게 바꾼 PostService

 

확실히 도메인 내에서 비즈니스 로직을 처리한 후 Service계층의 코드 복잡도는 이전보다 나아졌음을 느낄 수 있었다. 이러한 부분을 고려했을 때 만족스러운 결과물이라고 생각했고 한동안 해당 부분에 대해서는 더 이상 고민하지 않았다. 다만 오늘 작성한 코드들을 훑어보니 마음에 들지 않는 점이 생겼다. 어쨌든 해당 도메인 내에서도 유저 일치 여부와 권한여부를 체크하는 if문이 반복되고 있었다. 그리고 같은 캠프내 프로그래밍을 잘하신다고 느꼈던 분에게 리뷰를 부탁드렸는데 뭔가 entity내부에서 예외를 처리하는 게 어색하게 느껴진다는 피드백을 받을 수 있었다. 해당 도메인의 메서드를 호출한 곳에서 엔티티의 예외를 처리하면 되지 않을까라는 생각을 했지만 예외는 좀 더 Service 계층에서 던지는 게 합리적일 수도 있겠다는 생각이 들어, if 문의 반복, 예외처리를 Service 계층에서 던지게만들기 두 가지를 중점적으로 다시 리팩토링을 진행햅고자했다.

---

v3

    public boolean hasRole(User user) {
        if (user.getId() == this.getUser().getId() || user.getRole().getAuthority() == "ROLE_ADMIN") return true;
        return false;
    }

단순히 Post 엔티티에 boolean 타입을 반환하는 메서드를 만들었다. 이때부터 뭔가 좀 헷갈렸다. 내가 의도하고자 하는 방향은 특정 조건이 만족되면 해당 객체의 상태를 객체 스스로가 판단하여 상태를 바꾸는 것이었는데, 조건의 반복을 없애기 위해서 boolean타입으로 반환타입을 설정한다면 도메인은 상태변화가 아닌 비즈니스 로직의 가능여부만 판단하는 메서드를 가지게 되는 것이고 이러한 판단 여부 메서드가 굳이 도메인 안에 존재해야하는지에 대해서 의문이 들었다.

 

    public PostResponseDto updatePost(Long postId, PostRequestDto postRequestDto, User user) {
        Post post = findPost(postId);
        if (!post.hasRole(user)) throw new IllegalArgumentException("해당 게시글 작성자 혹은 관리자만 수정할 수 있습니다");
        post.update(postRequestDto);
        return new PostResponseDto(post);
    }

    public String deletePost(Long postID, User user) {
        Post post = findPost(postID);
        if (!post.hasRole(user)) throw new IllegalArgumentException("해당 게시글 작성자 혹은 관리자만 수정할 수 있습니다");
        postRepository.delete(post);
        return "삭제완료";
    }

해당 boolean을 이용하여 작성한 Service계층. if 문 안의 글자만 줄었지 이전과는 크게 달라진 모습이 없었다. 결국 같은 코드가 반복되고 의도했던 것과는 좀 다른 모양의 코드가 됐던 것 같다. 고민을 하다가 결국 과감하게 도메인에서 비즈니스 로직 부분들을 전부 제외하기로 결정했다. 해당 부분은 Service 계층에서 하나의 메서드로 뽑아내는 게 낫지 않을까라는 생각이 들었다.

 

    public PostResponseDto updatePost(Long postId, PostRequestDto postRequestDto, User user) {
        Post post = userConfirmPost(postId, user);
        post.update(postRequestDto);
        return new PostResponseDto(post);
    }

    public String deletePost(Long postId, User user) {
        Post post = userConfirmPost(postId, user);
        postRepository.delete(post);
        return "삭제완료";
    }
    
    private Post userConfirmPost(Long postId, User user) {
        Post post = findPost(postId);
        if (!(user.getId() == post.getUser().getId() || user.getRole().getAuthority() == "ROLE_ADMIN"))
            throw new IllegalArgumentException("해당 게시글 작성자 혹은 관리자만 수정,삭제할 수 있습니다");
        return post;
    }

원래 처음 작성했던 도메인으로 바꾸고, Service계층에서 위의 코드로 작성하니 좀 더 깔끔하다는 느낌이 들었다. userConfirmPost가 어떤 로직인지도 바로 확인이 가능하고 기존에 비즈니스 로직을 도메인에 넣었던 것보다 깔끔해보였다. 뭔가 이번 리팩토링과정에서는 자신감있게 도메인에 비즈니스 로직을 넣어보는 시도를 했는데, 결과적으로 실패했던 것 같다. 반복되는 조건이 있는 비즈니스 로직은 해당 조건들을 각각 도메인 내부에 넣을 게 아니라 Service 계층에서 하나의 메서드로 뽑는 게 더 효율적인 것 같다. 아무래도 추가적으로 도메인에 비즈니스 로직은 어떨 때 넣으면 좋은지 공부해야겠다는 생각이 든다. 

---

뭔가 리팩토링과정을 길게 가져가면서 결국 원점으로 돌아온 것 같다. 물론 기존의 코드보다는 좀 더 직관적이고 깔끔해지긴했지만 시도해보고 싶었던 도메인에 비즈니스 로직을 넣는 것은 객관적으로 평가했을 때 실패했다. 좀 아쉽긴 하지만, 그런 아쉬움보다도 해당 개념에 대해서 부족한 내 지식때문에 좀 더 알고 싶다는 생각이든다.

 

그래도 리팩토링 과정에서 뭔가 느낄 수 있었던 점은 

한 서비스 계층에서 반복되는 로직은 도메인에 넘기는 게 아닌 해당 서비스 계층에서 메서드를 뽑는 게 낫고

여러 서비스 계층에서 사용될 수 있는 공통적인 로직은 도메인 내부에서 작성하는 게 나을 것 같다는 점이다.

단순히 서비스 계층의 로직만을 가지고 도메인에 모두 넣는 것은 그렇게 바람직하지 않은 것 같다.