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

#4 세션, 코드리뷰 후기

1. AWS 사용

AWS는 사실 키 유출되서 요금폭탄 맞은 사례가 많아서 멀리했었는데, 이번 기회에 사용해 볼 수 있었네요!
그래서 사용하면서 보안쪽이 굉장히 중요하다고 생각이 되서 Jasypt를 사용해서 계정 AccessKey, SecretKey를 암호화 했습니다.
그래도 혹시 몰라서 이걸로 충분한지 싶어서, application.yml 파일자체를 git에서 빼버려야 되는건지 싶어서 질문을 드렸는데,
파일 자체를 빼버리는 건 좋은 방법은 아니라고, 암호화 하는게 좋은 방법이라고 답변해주셨습니다. 😄

• 네트워크의 경우, 예상치 못한 오류가 발생하는 경우가 많기 때문에 분리는 하지 않고 일괄 warn으로 먼저 하는것이 좋을듯?
• controller 단에서 예외처리의 2가지 방법이 있는데,
  • 1. 따로 업로드 하는 uploadProfileImage 함수에서 try catch를 해서 로깅 처리하는 방법
  • 2. upplyAsync() 의 체이닝 함수인 exceptionally() 함수를 통해서 로깅 처리 하는 방법!
    • 단 이 경우는 uploadProfileImage에서 Optional 반환할 필요가 없다!!!

2. 비동기 등록

CompletableFuture를 이용해서 비동기를 사용해볼 수 있었고, 코드 리뷰 세션을 통해서도 많이 배울 수 있었습니다.

• 비동기 예외 처리는?
  • 비동기 코드의 예외는 MVC의 쓰레드에서 예외가 발생한것이 아니다!
  • 그래서 @ControllerAdvice 에서는 처리할 수 없다.
• 네트워크 I/O, 디스크 I/O
  • ex) 디스크 작업의 경우, Blocking이 오래 걸릴 수도 있다?
  • 비지니스 로직에 연관되지 않고, 비동기 식으로 분리할 수 있다면 분리하는것이 좋다!
• 비동기 처리에서 동시성 이슈?
  • update 시, 같은 row가 아니면 상관 없을 듯!
  • 트랜잭션 격리수준에 따른 동시성 이슈 → ✨ 공부 필요!
• @Async vs CompletableFuture ?
  • 효과는 똑같은데, 실행되는 쓰레드풀이 다르다!
  • @Async는 CompletableFuture에서 지원되는 등의 체이닝 이벤트(thenAccept, exceptionally)와는 거리가 멀다.
  • @Async의 경우에는 별도의 쓰레드풀을 지정하는 반면, CompletableFuture는 JVM이 실행될때 기본 쓰레드풀에서 실행된다.
  • 쓰레드 풀을 직접 설정해서 쓰레드를 제한도 가능!

그 외

나름 고심하면서 코딩 했던것 같은데, 코드 리뷰를 받고나서 코드가 참 지저분했구나... 를 매우 많이 느낄 수 있었습니다.

코드 리뷰 결과

• profileImageUrl 필드 추가
  • 체크 구문 자체도 실수가 있었고, checkArgument를 제대로 활용하지도 못했다는 것을 알게 되었습니다.. 😱
• 이미지 업로드
  • S3Client - upload 구현
    • metadata 설정에서 불필요한 for문을 사용했음! 😫
  • 예외처리
    • AmazonServiceException, SdkClientException, 그 외의 예외로 나누어 처리할 필요는 없음!
  • 업로드 관련
    • 함수 한 곳에서만 쓰이기 때문에, 굳이 상수로 선언할 필요가 없다!
    • getBaseUrl 함수도 따로 필요 없음!
• Comment 관련 비지니스 로직 구현
  • 변수로 받은 comment의 postId, userId와 매개변수로 받은 postId, userId의 유효성 체크부분을 놓침...
  • ⭐️ 반환되는 값이 없다고 NotFoundException 처리를 해주는것보다 orElse를 이용해서 emptyList를 반환하는 것도 좋은 방법!

코드 리뷰 적용

• profileImageUrl 필드 추가

  • 변경 전

    public User(...) {
      // 💩 → 지저분!
      if (profileImageUrl != null)
        checkArgument(profileImageUrl.length() > 250, "profileImageUrl length must be less than 255 characters");
    
      this.profileImageUrl = profileImageUrl;
    }
    ...
    
    public void updateProfileImage(String profileImageUrl) {
      // 💩 → 체크 부분 빠짐!
      this.profileImageUrl = profileImageUrl;
    }

  • 변경 후

    public User(...) {
      // ✨✨
      checkArgument(profileImageUrl == null || profileImageUrl.length() <= 255, "profileImageUrl length must be less than 255 characters.");
    
      this.profileImageUrl = profileImageUrl;
    }
    
    public void updateProfileImage(String profileImageUrl) {
      // ✨✨
      checkArgument(profileImageUrl == null || profileImageUrl.length() <= 255, "profileImageUrl length must be less than 255 characters.");
    
      this.profileImageUrl = profileImageUrl;
    }

• 이미지 업로드

  • S3Client - upload 구현

    • 변경 전

      if (metadata != null && metadata.size() > 0)
        metadata.forEach(objectMetadata::addUserMetadata); // 💩 → 불필요한 for문 사용!

    • 변경 후

      if (metadata != null && metadata.size() > 0)
        objectMetadata.setUserMetadata(metadata); // ✨✨

  • 예외처리

    • 변경 전

      toAttachedFile(file).ifPresent(attachedFile ->
        supplyAsync(...
        ).thenAccept(...
        ).exceptionally(e -> { // 💩 → exception을 나누어 처리할 필요가 없다!!!
          if (e instanceof AmazonServiceException)
            log.warn("AmazonServiceException occurred: {}", e.getMessage(), e);
          else if (e instanceof SdkClientException)
            log.warn("SdkClientException occurred: {}", e.getMessage(), e);
          else
            log.error("Unexpected aws-s3 error occurred: {}", e.getMessage(), e);
          return null;
        })
      );

    • 변경 후

      toAttachedFile(file).ifPresent(attachedFile ->
        supplyAsync(...
          ).thenAccept(...
          ).exceptionally(e -> { // ✨✨
            log.warn("aws s3 - upload error occurred: {}", e.getMessage(), e);
            return null;
          })
        );
      }

  • 업로드 관련

    • 변경 전

      private static final String BASE_URL = "user/";            // 💩 불필요한 상수 선언!
      private static final String DEFAULT_EXTENSION = "png";     // 💩 불필요한 상수 선언!
      
      // 💩 불필요한 함수 정의!
      // 경로명 가져오기
      // User 폴더 → UserId → FileName 으로 저장 될 수 있도록!
      public String getBaseUrl(Long userId) {
        return userId == null ? "" : BASE_URL + userId.toString();
      }
      
      // 💩 → CompletableFuture - supplyAsync 함수에서 예외 처리를 했다면, 여기서는 Optional로 리턴할 필요가 없음
      // 💩 → 이유는 예외 발생을 한다면 거기서 throw exception을 하고 끝이기 때문!
      public Optional<String> uploadProfileImage(AttachedFile profileFile, String baseUrl) {
        Map<String, String> map = new HashMap<>();
        String key = profileFile.randomName(baseUrl, DEFAULT_EXTENSION);
        return Optional.of(
          s3Client.upload(profileFile.inputStream(), profileFile.length(), key, profileFile.getContentType(), map)
        );
      }

    • 변경 후

      // ✨✨
      public String uploadProfileImage(AttachedFile profileFile) {
        String key = profileFile.randomName("profile", "png");
        return s3Client.upload(profileFile.inputStream(), profileFile.length(), key, profileFile.getContentType(), null);
      }

• Comment 관련 비지니스 로직 구현

  • 변경 전

    // 💩 → comment.postId, comment.userId vs postId, userId 체크 구문이 빠졌음!
    @Transactional
    public Comment write(...) {
      return findPost(...).map(post -> {
        postRepository.updateCommentsUp(postId);
        return commentRepository.insert(comment);
      }).orElseThrow(() -> new NotFoundException(...));
    }
    
    // 👍
    @Transactional(readOnly = true)
    public List<Comment> findAll(...) {
      return findPost(...)
        .map(post -> commentRepository.findAll(postId))
        .orElse(emptyList());
    }

  • 변경 후

    @Transactional
    public Comment write(...) {
      // ✨✨
      checkArgument(comment != null, "comment must be provided");
      checkArgument(comment.getPostId().equals(postId), "comment.postId must equals postId");
      checkArgument(comment.getUserId().equals(userId), "comment.userId must equals userId");
    
      return findPost(...).map(post -> {
        postRepository.updateComments(postId, postWriterId);
        return insert(comment);
      }).orElseThrow(() -> new NotFoundException(...));
    }