[Review] 스프링 카페 5단계 - 게시글 권한부여 (2022/03/17)

 

[Shine] 스프링 카페 5단계 - 게시글 권한부여 by zbqmgldjfh · Pull Request #94 · codesquad-members-2022/java-sprin

1. 코드 리뷰

1. 반환값에 대하여 생각해보기

우선 Long을 사용한 가능 큰 이유는 "정상적으로 삭제 되었는지 확인하고 싶다" 였다.

조금 다른 이야기 이기인데.. 그 학교 시스템프로그래밍 시간에 System call 들 공부할때 느낀점이 API 호출시 실패할 경우 -1을 반환해주는 API 가 상당하게 많다 생각했었다.

어쩌다 보니 그뒤로 save, delete와 같은 유형의 메서드들에게 반환값으로 정수를 반환하도록 하고 있었던것 같습니다.

추가로, 혹? 정수를 반환하면 좋지 못할만한 이유가 있을까요?
단순하게 리뷰어님의 의견에 순응하기 보다는, 로치님의 의견 또한 꼭 들어보고 싶습니다!

 

2. 중복 문장열 상수로 뽑기

다음과 같이 interface로 상수를 하나 만들었습니다.

public interface SessionConst {
    public static final String SESSIONED_USER = "SESSIONED_USER";
}

interface로 만든 이유는 해당 class는 생성할 객체가 아니기 때문에 interface로 해두었습니다!

 

3. Session에 대한 검사

음? 우선 약간 햇갈리게 이름을 만들어 죄송합니다. (저도 이거 항상 햇갈리는 거 같아요 단어....)

리뷰어님의 의도가 2가지 가능한 것 같습니다??

 

1)Authorization을 잘못 보신 경우
메서드의 이름이 잘 보면 Authentication 이 아니라, Authorization 입니다.
즉, 해당 메서드는 세션에서 로그인 한 사용자인지를 확인하는 메서드가 아니라!
해당 article을 삭제할 권한이 있는지 => 작성자 user_id와 삭제하려는 user_id 가 같은지? 를 판단하는 메서드 입니다.

 

따라서 isUserHasAuthorization 와 같이 "당신은 지울수 있는 권한이 있습니까?" 라는 메시지를 전달하는 함수 입니다!

 

또한 이는 web 계층이 아니라, 서비스 계층에서 로그인 사용자의 id와 비교하여 다를경우 예외를 던지게 됩니다.

따라서 Authorization(인가) 해당 게시물을 삭제할 권한이 있는지 알아내는 용도 입니다.

원래 의도하신 것 처럼 session의 로그인 체크는 LoginInterceptor로 따로 구현하여 처리되고 있습니다!!

 

2) Authorization 을 잘 봤다
isUserHasAuthorization 즉, 자신의 article인지 확인하는 메서드를 controller로 뺴라