Skip to content

Conversation

devjun10
Copy link
Contributor

📝 구현 내용

회원정보 수정을 Application/Json 방식으로 처리했습니다.

  • 회원정보 수정을 Application/Json 방식으로 변경
  • 인터셉터 처리 추가






✨ 인터셉터

인증/인가가 필요한 API의 공통 로직을 인터셉터로 옮겨, 중복 로직을 제거했습니다.

@Slf4j
public class SessionCheckHandlerInterceptor implements HandlerInterceptor {

    ......

    @Override
    public boolean preHandle(
        HttpServletRequest request,
        HttpServletResponse response,
        Object handler
    ) {
        Long sessionId = getSessionId(request.getCookies());
        validator.validateSessionId(sessionId, response);

        Session findSession = loginUseCase.findSessionById(sessionId);
        log.info("Session:{}", findSession);
        validator.validateSession(findSession, response);

        LoginUser loginUser = new LoginUser(findSession);
        request.setAttribute("loginUser", loginUser);
        return false;
    }

    ......

}






이는 HandlerExecutionChain 내부에 존재하며, Handler를 호출하기 전, 실행됩니다.

public class HandlerExecutionChain {

    private List<HandlerInterceptor> interceptors;
    private final Object handler;

    ......

}

@devjun10 devjun10 added ⚙️ refactor 의사결정이 있는 코드 수정 🐝 test 테스트코드 작성 🚄 feat 기능 개발, 모듈 구현, 기능 개발을 위한 코드 추가 labels Mar 26, 2024
@devjun10 devjun10 requested a review from kmularise March 26, 2024 07:33
@devjun10 devjun10 self-assigned this Mar 26, 2024
@devjun10 devjun10 requested review from J-MU and kmularise and removed request for kmularise and J-MU March 26, 2024 07:36
Copy link

@kmularise kmularise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다!

String username = request.getAttribute("username");
String password = request.getAttribute("password");
log.info("username: {}, password: {}", username, password);
String username = (String) request.getAttribute("username");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 리팩토링 하신 이유가 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인터셉터를 별도로 생성해, 중복되는 로직을 모두 삭제했습니다.


@Override
@GetMapping(path = "/users/{userId}")
@GetMapping(path = "/my-info")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path가 바뀐 이유가 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은. html과 같게 하려고 경로를 수정했습니다. 처음 설계할 때, 조금 더 신경 써야 했던 것 같습니다.

console.error('Error:', error);
localStorage.clear();
document.cookie = 'sessionId=; Path=/; Expires=Thu, 01 Jan 1970 00:00:01 GMT;';
window.location.href = "http://localhost:8080/";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

html 관련 내용이긴 하지만, 쿠키 관련 설정이 추가된 이유가 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존 로직에는 서버를 재 실행 했을 때도 쿠키가 남아있었습니다. 따라서 에러가 발생하면 로그아웃 처리를 하기 위해 해당 설정을 추가했습니다.

Copy link

@kmularise kmularise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리팩토링으로 코드가 더 깔끔해진 거 같아요! 고생하셨습니다!

addInterceptors(getBean(InterceptorRegistry.class));
}

@Override

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인터셉터를 직접 구현하셨군요! 인터셉터를 구현하신 이유가 있나요? 인터셉터의 역할은 무엇인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

각 컨트롤러에서 권한 체크를 하니, 로직의 중복이 발생했는데요, 이를 제거하기 위해 인터셉터를 만들었습니다. 즉, 권한 체크가 인터셉터의 역할입니다.

@Override
public void delete(User user) {
String sql = update(User.class, "deleted");
String sql = JdbcHelper.update(User.class, "deleted");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JdbcHelper에서 update메소드를 호출할 때 두번째 메소드 파라미터가 유효하지 않은 경우면 어떻게 되나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다시 보니, validator가 파라미터 검증을 안 하고 있네요. 이 부분 수정하겠습니다.

@Slf4j
@RestController
public class UserInfoUpdateController implements Handler {

    private final UserValidator validator;
    private final UserLoginUseCase loginUseCase;
    private final UserUpdateUseCase userUpdateUseCase;

    ......

    @Override
    @PutMapping(path = "/api/users")
    public ModelAndView process(
        HttpServletRequest request,
        HttpServletResponse response
    ) {
        LoginUser loginUser = (LoginUser) request.getAttribute("loginUser");
        userUpdateUseCase.update(loginUser, (String) request.getAttribute("password"));

        response.setStatus(OK);
        return null;
    }
}

@@ -0,0 +1,4 @@
package project.server.mvc;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인터페이스가 아닌 추상클래스로 만드신 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 스프링을 참조했는데, 상태를 가질 필요가 있어서 추상 클래스를 사용했습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractJsonHttpMessageConverter.java이 코드 상에서는 상태가 보이지 않는 거 같은데 더 자세히 말씀해주실 수 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 JsonConverter를 구현하려고 했는데, 굳이 구현할 필요가 있을까 싶어 DispatcherServlet에서 간략하게 처리했습니다. 아직 구현되지 않은 부분인데, 이 부분은 그냥 넘어갈까 합니다.

protected void doPost(HttpServletRequest request, HttpServletResponse response) throws Exception {
}

protected void doPut(HttpServletRequest request, HttpServletResponse response) throws Exception {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PUT 요청과 POST 요청의 차이점에 대해서 설명해주세요!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post는 자원의 생성, Put은 수정에 사용됩니다. 이미 아실 것 같아서 기본적인 내용은 제외하면, URI 결정, 멱등성 정도의 차이가 존재할 것 같은데, Post는 URI를 서버가 결정하며, Put은 URI를 클라이언트가 상세하게 지정합니다. 즉, Post는 자원의 위치를 모르기 때문에 서버가 이를 관리하며, Put은 어떤 위치에 있는 자원을 수정할 것인지 클라이언트가 알고 있는 것입니다.

멱등성은 같은 행위를 계속해서 반복하더라도 똑같은 결과가 나오는 것을 말하는데요, Put의 경우 같은 행위를 반복하더라도 최종 값으로 덮어 씌워지게 됩니다.

import java.util.Arrays;
import java.util.function.Predicate;

public enum ContentType {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content Type 이넘으로 만드신 점 좋은 거 같아요 👍

if (contentTypeHeader == null || contentTypeHeader.isEmpty()) {
return false;
}
Optional<HttpHeader> findApplicationJson = contentTypeHeader.stream()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스트림은 for문에 비해 어떤 장점을 가지나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream은 for 문에 비해 가독성, 체이닝, 백 프레셔, 함수형 프로그래밍 지원 등의 이점이 있습니다. 이중 가장 큰 이점은 함수형 프로그래밍 지원이라고 생각하는데요, 함수형을 사용해서 부수 효과를 제거하고, 어떤 것(What)을 할 지에 집중할 수 있기 때문입니다.

가독성은 상황에 따라 다를 수 있기 때문에 사람마다 조금씩 생각의 차이가 있을 것 같네요.


Map<String, Object> attribute = new HashMap<>();

String[] bodyArray = parsedBody.split(",");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"," 사소하지만 파싱 기준이 되는 문자열을 static 상수로 두는 것은 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

괜찮은 것 같아요. 반영하겠습니다.

import project.server.mvc.springframework.annotation.Configuration;
import project.server.mvc.springframework.web.servlet.DispatcherServlet;

@Configuration

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Configuration@Component의 차이점에 대해서 설명해주세요. 그리고 @Configuration은 어떠한 상황에서 쓰는지 설명해주세요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@component는 사용자가 만든 클래스를 빈으로 등록할 때 사용합니다. 반면 @configuration은 라이브러리나 다른 외부 컴포넌트, 예를 들어, RestTemplate 같은 이미 제공해주는 것들을 빈으로 등록할 때 사용합니다. 둘을 혼합해서 사용해도 동작은 하는데요, 결국 @configuration도 내부에 @component가 존재하기 때문입니다.

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Documented
@Component
public @interface Configuration {
    ......
}


public interface HandlerInterceptor {

default boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default 메소드를 사용하신 이유가 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분은 스프링을 참조했습니다. 오전에 같이 이야기 나눴는데, 추상 메서드로 처리해도 되고, 추상 클래스에 공통 메서드를 둬도 되고, 지금 같이 default 메서드로 처리해도 됩니다. 이 중 스프링이 채택하고 있는 default 메서드로 이를 구현했습니다.

ModelAndView modelAndView
) throws Exception {
if (modelAndView == null) {
if (request.isContentType(APPLICATION_JSON)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getter 메소드와는 어떤 차이점이 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반환 타입이 boolean 이어서 isXXX를 사용했습니다. 해당 포스팅을 한 번 읽어보면 도움될 것 같습니다.

@@ -0,0 +1,4 @@
package project.server.mvc;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractJsonHttpMessageConverter.java이 코드 상에서는 상태가 보이지 않는 거 같은데 더 자세히 말씀해주실 수 있나요?

@devjun10 devjun10 requested a review from kmularise April 5, 2024 21:28
Copy link

@J-MU J-MU left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~!

HttpServletResponse response,
Object handler
) {
Long sessionId = getSessionId(request.getCookies());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getSessionId를 HeaderUtils에 선언하셨던데 HeaderUtils에 선언하기로 결정한 이유에 대해서 들을 수 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Header에서 여러 유형의 값들을 파싱할 수 있다고 판단해서 별도의 파싱 클래스를 생성했습니다. Session 이외에도 여러 값들을 파싱할 수 있기 때문입니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그럼 그건 HttpHeaders가 가질 수 있는 책임 아니였을까요??
HeaderUtils의 디렉토리 위치를 보면 common필드 내부에 존재하는데 HeaderParser와 같은 클래스를 정의해서 HeaderUtils의 필드로 넣거나, 혹은 HeaderUtils의 내부 메서드로 정의하는게 더 이상적인 방식이라고 느껴서요.

HttpHeaders 클래스에 존재하는

public int getContentLength() {
        List<HttpHeader> headers = this.headers.getOrDefault(
            CONTENT_LENGTH, List.of(new HttpHeader(CONTENT_LENGTH, "0"))
        );
        return Integer.parseInt(headers.get(0).getValue());
    }

위 메서드는 headers중에 contentLength를 찾아서 반환하는데 이것이 session과 관련된 메서드를 찾아서 가져오는 것과 어떤 차이가 있나요? 비슷한 성질의 역할을 하는 메서드인데 contentLength를 찾아오는 메서드는 HttpHeaders에 존재하고 sessionId를 찾아오는 메서드는 HeadersUtils에 존재하는 것이 납득되지 않습니다.

또한 common 패키지 밑에 HeadersUtils가 들어가는 것도 이해가 잘 안됩니다.
HeadersUtils는 util클래스라 명명되어 있지만, HttpHeaders라는 객체에 관련된 일만 할 것이 명백한 상황입니다.
그렇다면 적어도 HttpHeaders와 같은 디렉토리인 http 패키지 아래로 들어가는 것이 더 이상적인 구조지 않았을까요?

HttpServletResponse response,
Object handler
) {
Long sessionId = getSessionId(request.getCookies());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sessionId로 UUID가 아니라 Long을 사용하신 이유가 궁금합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 userId 같은데 제 실수 같네요. 수정하겠습니다!

Object handler
) {
Long sessionId = getSessionId(request.getCookies());
validator.validateSessionId(sessionId, response);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateSessionId를 보면 파라미터로 userId가 선언되어있는데..
잘못 된거 아닌가요?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다. 위에 변수명 실수 같아요.

Long sessionId = getSessionId(request.getCookies());
validator.validateSessionId(sessionId, response);

Session findSession = loginUseCase.findSessionById(sessionId);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

findSession에서 find의 수동태는 found라 foundSession이 맞지않나요?

저는 그냥 session이라 선언해도 의미 전달에 전혀 문제가 없다 느껴져요. 굳이 변수명이 길어지는 것 보다는 session으로 두는 것이 더 낫다고 생각하는데 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그것도 괜찮죠. 다만 찾아온 값들의 접두사로 find{$NAME} 형태로 사용하다보니 전체를 이렇게 통일했습니다.


Session findSession = loginUseCase.findSessionById(sessionId);
log.info("Session:{}", findSession);
validator.validateSession(findSession, response);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
invalid일 경우 응답을 내릴 때, max-age=0;으로 응답을 내리던데 max-age를 왜 0으로 설정하는시 설명부탁드립니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쿠키 만료 시간을 0으로 해야 브라우저에 저장된 값을 삭제할 수 있기 때문입니다.


@Configuration
public class ServletConfiguration {
public class DispatcherServletAutoConfiguration {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위 클래스가 하는 역할이 뭔가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DispatcherServlet을 스프링 빈으로 등록합니다.

}
}

private void registerMapping(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET,POST,PUT,PATCH가 아닌 해당 메서드는 어떤 용도로 만든걸까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 몰라서 두었는데, 이번 미션에선 딱히 사용할 일이 없습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배포도 염두해 두고 있어, CORS 신경 쓴다고 뒀던 것 같은데, 현재는 사용하진 않습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 CORS와 어떤 연관성이 있는 건지 설명해주실 수 있을까요?

) {
String username = request.getAttribute("username");
String password = request.getAttribute("password");
String username = (String) request.getAttribute("username");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getAttribute의 반환값이 Object로 되어있는데 String이 아닌 경우는 어떤 경우죠?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attribute가 꼭 String이 아니더라도, Integer이나 BigDecimal 같은 다른 값이 저장되는 경우에요. 예를 들어, 나이를 파라미터로 받으면 Integer이 들어가겠네요.

}

tasks.named("downloadYml") {
dependsOn downloadYml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dependsOn downloadYml이라고 검색해보도 잘 안나와서 그런데,
어떤 작업을 하신건지 설명해주실 수 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

외부 저장소에 저장된 yml 값을 받아오는 역할을 합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이전 글 한 번 참조해보면 좋을 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚄 feat 기능 개발, 모듈 구현, 기능 개발을 위한 코드 추가 ⚙️ refactor 의사결정이 있는 코드 수정 🐝 test 테스트코드 작성
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants