Feature/board service: Suspended item 조회

[shin-jingyu]

Pull Request

Issues

Resolves #71
Resolves #45

Description

• Board 목록 조회 시, SUSPENDED 상태의 아이템은 제목을 제거했습니다.
• Board 단건 조회 시, SUSPENDED 상태의 아이템에 대해 403 Forbidden 응답과 적절한 안내 메시지를 반환하도록 처리했습니다.
• 테스트 코드에서 boardDetail.status가 SUSPENDED일 때 예외가 발생하는 케이스를 추가했습니다.
• 테스트 코드에서 BoardStatus로 목록을 조회하는 경우, status가 SUSPENDED이고 title이 null인 객체를 추가하여 검증했습니다.

How Has This Been Tested?

• 테스트 환경: OpenJDK 21(Amazon Corretto 21), kotest (mockk 라이브러리) with FreeSpec.

[merge-simpson]

👍 전체적으로 이해하기 쉽고 깔끔하게 잘 작성된 것 같습니다.

코드를 보기 굉장히 편했습니다.
몇 가지 질문 사항이나 생각거리가 남아 있어서 그 부분만 같이 검토해 주시면 좋을 것 같습니다.

가벼운 작업이니 다른 분들도 너무 거창한 리뷰보다는 가볍게 보이는 부분만 말씀해 주시면 좋을 것 같습니다.

🚀	🏅 Approve!

[merge-simpson]

👍 훌륭하고 깔끔한 작업입니다.

이 항목들이 특히 보기 좋습니다.
팀 내 작업 방식에 대한 리뷰를 지금까지 꼼꼼하게 확인해서 반영해 주신 것 같습니다.

• 👍 Page 객체에서 아이템 매핑에 적절한 map 함수 사용
• 👍 Enum 열거상수 비교에 == 연산자 허용
• 👍 적절한 삼항연산자 사용

이 부분은 조금 더 생각해 볼 수 있을 것 같습니다.

• 💬 기술적인 이야기는 아닐 수 있지만, title을 null로 할지 기타 기본값을 내릴지 나중에 정할 수 있어 보입니다. 이번 단계 온보딩 프로젝트에서는 다국어 등을 고려하지 않으므로 나중에 생각해 볼 수 있을 것 같습니다.
• 💊 record 인스턴스 생성 시, 생성자보다 빌더나 mapstruct 함수, record 내부 메서드 등을 권장하고 싶습니다.
  • 생성자를 사용하는 방식은 데이터 스펙이 바뀔 때마다 많은 곳을 함께 수정하거나, 생성자를 매번 추가적으로 작성해야 하기 때문입니다.

    // 기존 레코드에 age 필드가 추가되었을 때 예시
    public record ExampleRecord(String name, Integer age) {
        // 기존 스펙을 생성자로 작성합니다. (단, 스펙이 바뀔 때마다 이런 생성자가 추가되어야 할 수 있습니다.)
        public ExampleRecord(String name) {
            // 모든 인자를 전체인자생성자로 넘기고, 나머지는 기본값으로 처리할 수 있습니다.
            this(name, null);
        }
    }

[shin-jingyu]

@merge-simpson 좋은 의견 감사합니다.
온보딩에서는 다중 사용이 필요 없다고 판단해 생성자를 사용했지만,
향후 다양한 활용 가능성을 고려하면 record 내부 메서드를 사용하는 것이 더 적절해 보입니다.

[merge-simpson]

❓ beforeTest에서 clearMocks(...)를 사용한 이유가 무엇인가요?

다른 분들께서도 이 사용 목적과 대안이 있는지 찾아보면 좋을 것 같습니다!
멀티모듈이 시작되면 짬이 안 나겠지만, 적절한 시기를 봐서
따로 테스트 코드와 관련해서 토픽을 정하고 공동 리서치 기간이 필요한가 싶기도 하네요.

리서치와 공유 빈도를 다시 높이면 좋을 것 같다는 생각이 듭니다!

[github-insu]

@merge-simpson 님 코드 리뷰 감사합니다.

beforeTest에서 clearMocks(...)를 사용한 이유

• mock 객체(boardQueryPort)를 사용하는 테스트 코드 작성을 하고 있습니다.
• mock 객체는 호출 기록을 저장합니다.
• veryfy(exactly = 1)... 코드는 호출 횟수를 검증합니다.
  -> 다른 테스트 코드에서 mock 객체를 이용하여 검증하고자 하는 메서드를 호출한 적이 있다면 예상한 결과와 다를 수 있습니다. 따라서 테스트 케이스마다 호출 기록을 초기화할 필요가 있다고 생각했습니다.

대안

• 1] 호출 기록을 초기화합니다.
• 2] 각 테스트를 시작하기 전에, mock 객체 상태를 재설정합니다.

리서치 내용

• 1] clearMocks(mock): 호출 기록을 초기화합니다.
  • clearMocks(mock, answers = false)와 같은 코드입니다.
  • answers가 true일 때, 호출 기록과 다른 설정들을 초기화합니다.
• 2] clearInvocations(mock): 호출 기록을 초기화합니다.
• 3] reset(mock): 호출 기록과 다른 설정들을 초기화합니다.
  -> 호출 기록만을 초기화함으로써 불필요한 추가 작업을 피할 수 있다고 생각합니다.

[merge-simpson]

좋은 리서치 공유 감사합니다.
혹시 각 테스트 종료 후가 아니라 beforeTest에서 작성한 데에도 이유가 있나요?