[12기] 웹 백엔드 시스템 구현 스터디(SpringBoot)

세션 #3, 코드리뷰 후기

이번 미션은 좋아요기능 구현, 페이징처리와 예외처리, Swagger 였습니다.
미션이 참 쉬워보였는데, 해보니까 정말 생각이 많아지더라구요.

1. 좋아요 기능

좋아요 기능 구현은 post를 조회를 할 때, userId를 모두 넣어야 되나? 라는 고민이 있었습니다.
하지만 함수 이름은 findById 인데, 메서드 이름만 본다면 post pk 만 넣어야 되지 않나라는 고민이 있었지만!
리뷰 통해서 결론적으로는 넣는것이 좋다는 답변을 받을 수 있었습니다. 👍

⭐️ 좋아요를 update 하는 부분의 꿀팁도 들을 수 있었습니다.

• update를 post를 조회한 후에, +1 해서 해당 Post를 일괄 수정을 했었는데,
• update post set like_count += 1 where postId = ? 이렇게 따로 함수도 분리해서 사용해야 된다!
• 여러명의 사람들이 일괄적으로 좋아요를 눌렀을때, 조회하는 타이밍에 따라서 +1 아니게 될수도 있기 때문에!

2. 페이징 처리

페이징 처리은 참 어려웠지만, 그래도 HandlerMethodArgumentResolver를 이용해서 구현을 하였습니다.
하지만 기본값 세팅하는 부분이 부족했다 라는것을 코드리뷰를 통해서 배울 수 있었습니다.

3. 에러 처리

세 번째인 에러에 대한 로그 처리는 정말 아무 생각하지 않고 error를 사용해서 로그를 남겼는데, 정말 큰 패착이었습니다.
사실 일괄 로그 처리하는 error로 통일하다보니 함수가 중복되서,
메서드를 분리하고 stack-trace도 남겨서 아래와 같이 구현을 하고 '오케이! 됐어!' 했었습니다만... 😱

코드 리뷰와 4주차 세션에서 설명해주시는 것을 듣고 많은걸 깨달을 수 있었습니다.
먼저 예외를 일괄 error 처리 해버리면 로그가 남발해서, 유지보수 하는 입장에서는 보는것도 일이 되버리는 문제점이 있기 때문에!
예외 종류에 따라서 로깅 레벨을 분리해서 유연하게 운영 해야되는 점이었습니다.

결과적으로는 개발자가 상정한 오류는 warn 으로 하고, 생각치 못한 오류를 error로 빼야되는 것이었습니다.
그리고 stack-trace의 경우에는 마지막에 exception 객체면 넘겨줘도 남길 수 있는것도 알게되었습니다.

4. Swagger

마지막 네 번째인 Swagger의 경우에는 미션 수행하면서 찾아보니,
Model 정보를 등록하는 메서드도 있어서 Entity(Table 정보)도 등록을 해야 되나? 싶었는데,
Swagger 어노테이션은 DTO 클래스에 대해서만 추가해주면되구요, DB 스키마까지 고려할 필요는 없다는 답변을 해주셨습니다. 👍

그 외

⭐️⭐️ 그리고 정답인 구현된 코드를 리뷰해주시면서 fluent하게 작성해야 한다는건 어떻게 하는건지를 알려주셨는데,
코드가 이렇게 깔끔할수가!? 해서 참 많이 배울수 있었던 3주차였습니다.

코드 리뷰 결과

• Post - findById 에 작성자(userId)도 매개변수로 넣어서 좀더 정확하게 사용합시다!
  • userId를 넣게 되면 일부 validation 로직은 필요없어짐!
• Swagger는 DTO 클래스만 추가해주면 되고, Schema(DB - Table)는 필요 없음!
• Pageable 부분!
  • isAssignableFrom 메서드 사용은 good! 👍
  • 생성자에서 예외가 throw 되지 않도록 offset, limit 값이 적절하지 않다면 기본값으로 대체해주는 처리가 있으면 좋을것 같습니다!
  • PageRequest - 디버깅에 유용하도록 toString() 를 오버라이드 해두는것도 좋을것 같습니다!
• Exception Handler 부분!
  • log.error만 사용하는건 좋지 않다! → 상황에 따라 로깅 레벨을 달리하는게 좋습니다!
  • 로깅에 stack trace를 포함하는건 좋은 방법! → ExceptionUtils.getStackTrace(throwable) 호출하지 않아도 객체만 넘겨줘도 됩니다!
  • ServiceRuntimeException 대한 처리를 세분화해서, 메서드 안에서 한번에 처리하기!
• PostService
  • like & findById 관련 → fluent 하게 작성하기!
  • userId, requesterId 에 대해 확인 하는 부분은 꼭 필요하진 않을것 같아요

코드 리뷰 적용

• Post, PostService 부분

  • 변경 후 - 작성자를 추가하고 fluent 하게 작성!

    ...
    public class PostService {
      ...
      @Transactional
      public Optional<Post> like(Id<Post, Long> postId, Id<User, Long> writerId, Id<User, Long> userId) {
        // ✨✨ → return 다 감쌀 수 있고, 코드 리딩도 쉽다!
        return findById(postId, writerId, userId).map(post -> {
          if (!post.isLikesOfMe()) {
            post.incrementAndGetLikes();
            postLikeRepository.insert(userId, postId);
            update(post);
          }
          return post;
        });
      }
    }

• Pageable 부분!

  • 변경 전

    @Override
    public boolean supportsParameter(MethodParameter parameter) {
      return Pageable.class.isAssignableFrom(parameter.getParameterType()); // 👍 isAssignableFrom를 사용 해야됨 👍
    }
    
    @Override
    public Pageable resolveArgument(...) {
      long offset = toLong(webRequest.getParameter("offset"), OFFSET_DEFAULT_VALUE);
      int limit = toInt(webRequest.getParameter("limit"), LIMIT_DEFAULT_VALUE);
      return PageRequest.of(offset, limit);
    }

  • 변경 후 → 기본값이 무조건 세팅 되도록!

    @Override
    public Object resolveArgument(...)) {
      String offsetString = webRequest.getParameter(offsetParam);
      String limitString = webRequest.getParameter(limitParam);
    
      long offset = toLong(offsetString, OFFSET_DEFAULT_VALUE);
      int limit = toInt(limitString, LIMIT_DEFAULT_VALUE);
    
      if (offset < 0) {
        offset = OFFSET_DEFAULT_VALUE;
      }
      if (limit < 1 || limit > 5) {
        limit = LIMIT_DEFAULT_VALUE;
      }
    
      return new SimpleOffsetPageRequest(offset, limit);
    }

• Exception Handler 부분!

  • 변경 전

    @ExceptionHandler(Exception.class)
    public final ResponseEntity<ApiResult<?>> handleAllException(Throwable throwable) {
      HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR;
      logError(status, throwable);
      return new ResponseEntity<>(ERROR(throwable, status), status); // 💩 → 이걸! 함수로 따로 뺐어야됨!
    }
    
    @ExceptionHandler({...})
    public final ResponseEntity<ApiResult<?>> handleBadRequestException(Throwable throwable) {
      HttpStatus status = HttpStatus.BAD_REQUEST;
      logError(status, throwable);
      return new ResponseEntity<>(ERROR(throwable, status), status);
    }
    
    // 💩 → 로깅 레벨별로 관리를 해줘야되므로 공통 될수가 없음!
    public void logError(HttpStatus status, Throwable throwable) {
      log.error("{} ERROR - message : {}, details : {}", status, throwable.getMessage(), ExceptionUtils.getStackTrace(throwable));
    }

  • 변경 후 → 로깅의 단계별 처리!

    // ✨✨ 상정된 오류는 이 클래스를 상속받도록 따로 ServiceRuntimeException 클래스를 생성하고,
    @ExceptionHandler(ServiceRuntimeException.class)
    public ResponseEntity<?> handleServiceRuntimeException(ServiceRuntimeException e) {
      // ✨✨ 이렇게 하나의 함수에서 일괄 처리할 수 있도록!!!!! → 불필요하게 함수를 늘리지 않아도 된다!
      if (e instanceof A_Exception)
        return newResponse(e, HttpStatus.NOT_FOUND);
      if (e instanceof B_Exception)
        return newResponse(e, HttpStatus.UNAUTHORIZED);
    
      log.warn("Unexpected service exception occurred: {}", e.getMessage(), e);
      return newResponse(e, HttpStatus.INTERNAL_SERVER_ERROR);
    }