[seminar3] 과제 구현

[gahyuun]

🔥Pull requests

👷 작업한 내용

구현 내용

1. 회원가입, 로그인 기능

사용자로부터 요청 body를 전달받고 회원가입 ( DB에 사용자 정보 저장 ), 로그인 ( 사용자 정보 확인 후 토큰 역할을 할 수 있는 ID 전달)을 구현하였습니다.

이후 일기 작성, 수정 등의 기능에서 userId가 필요한데, header로부터 userId를 전달받고 id가 존재하지 않으면 401 Error를, id가 존재하지만 수정하려는 일기 주인의 user id랑 일치하지 않을 시 403 Error를 반환하도록 구현했습니다.

2. 일기 작성 기능 , 수정 기능 , 삭제 기능

작성 및 수정 또한 요청 body의 유효성 검사를 진행 후 DB에 저장하는 형식으로 구현했습니다

수정이나 삭제 시 params로 전달한 diary id가 존재하지 않는다면 404 Error를 반환했습니다.

3. 일기 상세 조회 기능

일기 상세 조회 시 header로 userId, params로 diaryId를 전달합니다. 만약 다른 사용자의 일기를 조회하는데 해당 일기가 private이라면, 403 Error를 반환하고 아니라면 일기의 상세 정보를 응답으로 반환했습니다. 이 부분에 대해서는, 비공개라면 해당 id에 일치하는 일기의 존재 여부도 알려주면 안되나? 라는 생각때문에 404를 반환할까 고민했습니다. 하지만 일기를 찾지 못했다보단, 비공개이므로 접근 권한이 없다는게 의미적으로 적합한 것 같아 403 Error를 반환했습니다.

4. 내 일기 목록 조회 기능

사용자로부터 userId를 전달받으면 findByUserId 메서드를 통해 userId 기반으로 diary를 조회했습니다. 이때 querystring으로 category와 sort를 넘겨줄 수 있는데, 다음과 같이 querystring을 처리했습니다.

• category가 잘못된 값이 들어온 경우 ( ex ?category=YAMM)
  • querystring은 필터링의 역할이기에 빈 배열을 반환해줬습니다.
• sort에 잘못된 값이 들어온 경우
  • 정렬 기준 없이 일기 목록을 반환했습니다
• category 또는 sort의 값이 지정되지 않은 경우
  • defatulValue 설정으로 인해 모든 카테고리, 최신순 기준으로 일기 목록을 반환했습니다

5. 전체 일기 목록 조회 기능

전체 일기 목록 조회에서는 본인의 일기인지 상관없이 공개된 일기의 목록을 반환했습니다. querystring 처리는 위와 같습니다.

---

고민 과정

1. 에러처리

GlobalExceptionHandler

지난 2주차 과제에서는 Error를 throw해주고 controller에서 try catch를 하나하나 작성해 응답값을 반환하도록 구현하였습니다. 이렇게 되면 Controller 코드도 많아지고 Custom Error를 반환하는게 아니다보니 예상치 못한 경우가 생길 수 있다고 느꼈습니다. 이후 코드 리뷰를 통해 GlobalExceptionHandler의 존재를 알게 되었고 CustomError를 구현해 필요시에 Error를 throw만 해주고, GlobalExceptionHandler에서 처리하도록 구현했습니다.

굳이 호출부에서 try catch를 하나씩 써도 되지 않아서 코드가 깔끔해지고 Error 처리를 어디서 진행하는지 한 번에 알 수 있어서 편리함을 느꼈습니다.

중복에 대한 에러처리

미미나 시간에 중복으로 인한 에러 처리는 동시성 문제로 인해 Repository 단에서 처리하는게 더 적합하다는 걸 알게되었습니다. 그래서 중복될 수 없는 필드에 대해서는 unique=true를 설정해서 중복된 값이 할당되었을 경우, Repository에서 알아서 Error를 발생시키고 해당 Error를 GlobalExceptionHandler에서 처리하도록 구현했습니다. 하지만 중복값으로 인해 발생한 SQLIntegrityConstraintViolationException 오류는 광범위한 오류이기 때문에 특정 메세지를 작성하기 모호해졌습니다. 예를 들어 제목의 unique를 true로 설정해서 중복된 제목이 있을 경우 SQLIntegrityConstraintViolationException 에러를 반환하긴 하지만, CustomError가 아니므로 “중복된 제목을 입력할 수 없습니다” 같은 메세지를 전달할 수는 없습니다. 그래서 “데이터 무결성에 위배됩니다. “라는 메세지를 남기긴 했지만, 클라이언트측에게 넘겨주기에 적합하다고 생각하지는 않습니다. 이 부분에 대해서 어떻게 처리하면 좋을까요? 좀 더 좋은 방법이 있을까요?! 다른 분들의 의견이 궁금합니다 🤔

// Entity
@Column(nullable = false, unique = true)

// GlobalExceptionHandler
  @ExceptionHandler(SQLIntegrityConstraintViolationException.class)
    protected ResponseEntity<String> handleSQLIntegrityConstraintViolationException(final SQLIntegrityConstraintViolationException e) {
        return ResponseEntity.status(400).body(ErrorMessages.INTEGRITY_CONSTRAINT);
    }

2. 유효성 검사

2주차 과제에서는 유효성 검사를 아래와 같이 Controller에서 분기문을 통해 비교해서 처리해주도록 구현했습니다.

   if (diaryRequest.content().length() > CONTENT_LENGTH) {
                return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(new ErrorResponse("30글자를 초과했습니다."));
            }

하지만 이렇게 될 경우 Controller의 코드가 너무 길어지고 유효성이 많아지게 된다면 복잡해질 수도 있다고 느꼈습니다. 코드 리뷰때 성준이 오빠가 Validator라는 Class를 분리하고 Validator 객체의 유효성 검사 함수를 호출했던 것을 확인했고 좋은 방법인 것 같아서 Validator Class를 구현했습니다.

public class Validator {
    private static final int TITLE_MAX = 10;

    private static final int CONTENT_MAX = 30;

    public static void validTitleLength(final String text) {
        if (text.isEmpty() || text.length() > TITLE_MAX) {
            throw new BadRequestException(ErrorMessages.INVALID_TITLE_LENGTH);
        }
    }

그러다 미미나 이후에 @Valid annotation을 알게 되었고 annotation을 사용하면 더 깔끔해지겠다고 생각해 수정을 진행했습니다.

@NotBlank(message = "일기 본문을 입력해주세요")
@Size(min = 1, max = 30, message = "본문은 1자~30자로 입력해주세요")
String content,

annotation을 통해 Validator class를 지우고 검사 과정이 훨씬 간단해진 것을 확인할 수 있었습니다. 또한 어떤 유효성을 검사하는지 Controller가 아닌 Dto에서 알 수 있다는 것이 큰 장점으로 느껴졌습니다.

유효성 검사를 리팩토링하는 과정을 통해 내 과제의 부족함과, 다른 팀원의 코드를 보고 배울 점을 찾는게 중요하다고 생각했습니다. 🤗

3.DiaryService 에서 final method ( 질문 )

DiaryService에서 getDiary 메서드에 final 키워드를 추가했을 때, 다음과 같은 에러가 발생했습니다

Cannot invoke "org.sopt.~~~" because "this.userRepository" is null

final 키워드를 추가했더니 의존성 주입이 제대로 이루어지지 않았고 결국 500 Error가 발생했습니다 ( DiaryService의 다른 메서드들은 정상적으로 작동했습니다 )

관련 개념을 찾아보았을땐, @Transactional과 같이 AOP 기능이 적용된 메서드들은 오버라이드하여 프록시 객체로 감싸는데 final 키워드를 붙이면 오버라이드가 불가능하므로 문제가 된다는 것을 알게 되었습니다.

하지만 getDiary @Transactional을 사용하지도 않았고 AOP 기능이 적용된 메서드는 아니라고 생각하는데 해당 오류가 왜 발생하는지 궁금합니다,, 🥹

[jjuny-won]

와 엄청 열심히 공부하고 구현하게 보인당... 진짜 세심하네! 배우고 갑니당..춍춍👍

[jjuny-won]

우와 구체적으로 Exception 을 처리했네.. 나도 공부해야겠당... 나는 CustomException 을 썼는데.. 그걸 쓰고 조금더 명시적으로 전달할수있는것 같긴행!

[jjuny-won]

나는 /diaries 랑 /diary 랑 분리해서 컨트롤러를 작성했는데, 그러면 조금 더 가독성 있어지는 것 같긴해!

[jjuny-won]

오 이런 방법이 있구나.. 에러 메시지랑 처리가 많아서 살짝 복잡해보이는 거 같은데... 에러메시지는 따로 처리는건 어떨까?? 이렇게 message를 담은 이유가있을까?

[jjuny-won]

오 소문자가 들어와도 그러면 처리될 수 있겠넹.... 좋다!

[jjuny-won]

@CreationTimestamp 는 어떤 역할이지??? 궁금해서 물어보는 것...

[jjuny-won]

이런거 따로 메소드를 분리할 수도 있지 않을까... 계속해서 사용되는데?!

[jjuny-won]

오 findAll 보다 훨씬 좋은 것 같아용

[jjuny-won]

이거랑 message 랑 무슨 차이지??