🐛 [Fix] Flow 생성 시 participants, totalPlayTime NPE 방지 및 생성/업데이트 중복 로직 제거

[88dldl]

📍 PR 타입 (하나 이상 선택)

• 기능 추가
• 버그 수정
• 의존성, 환경 변수, 빌드 관련 코드 업데이트
• 기타 사소한 수정

❗️ 관련 이슈 링크

Close #211

📌 개요

• 예외 로그 확인해보니 다음과 같았고, 플로우 추천 과정이 아닌 플로우 생성과정에서 participants 값을 지정해주지 않으면 null로 저장되어 추후 조회 로직에서 에러가 발생하는 것이었습니다.

[예외 로그]

{
  "isSuccess": false,
  "code": "COMMON500",
  "message": "서버 에러, 관리자에게 문의 바랍니다.",
  "result": "Cannot invoke \"java.lang.Integer.intValue()\" because the return value of \"com.avab.avab.domain.Flow.getParticipants()\" is null"
}

🔥[6/19 수정]🔥

• participants는 현재 nullable하게 설계되어 있기 때문에 Entity 자체는 수정하지 않았습니다.
  대신, 실제 NPE가 발생했던 플로우 추천 로직 내에서 participants가 null일 경우를 예외 처리해주었습니다.
  (+ totalPlayTime 역시 같은 문제가 발생할 수 있어 함께 null 체크를 추가했습니다.)

[이전 수정]
❗f41224dd57dd1f87e24cbfa5c1c5789c45c76de7 원래는 DTO에서 직접 기본값을 초기화했었는데, 찾아보니 자바에서는 보통 DTO는 단순히 값을 전달하는 용도로만 사용하고, 디폴트값 처리는 서비스 계층에서 수행하는 게 일반적인 패턴이라고 하더라구요.

• 그래서 디폴트값 처리를 서비스 레이어로 옮기고, 추후 다른 값들도 초기화가 필요할 수 있을 것 같아 normalizeRequest() 메서드를 만들고 그 안에서 participants 값을 초기화하는 normalizeParticipants() 메서드를 호출하도록 구성했습니다.
• 플로우 생성 메서드 내부에서 PostFlowDTO 객체를 매개변수로 사용하고 있어서, participants 값만 초기화된 새로운 PostFlowDTO 객체가 필요했습니다. 일반적으로 setter 사용은 지양하는 편이지만, DTO는 데이터 전달과 가공을 허용하는 객체이고, 해당 필드(participants)만 조건적으로 수정이 필요한 상황에서 전체 객체를 다시 생성하는 것은 오히려 가독성과 유지보수에 좋지 않다고 판단했습니다!

[리팩토링]

• 플로우 생성(postFlow()) 을 살펴보니 내부 로직이 길어 가독성이 떨어져, updateFlow()와 중복되는 부분을 별도의 private 메서드로 모듈화하였습니다. ed65950

• FlowConverter의 toFlow()와 toUpdateFlow()가 거의 동일한 로직을 포함하고 있어 중복 코드를 buildFlow()메서드로 분리하였습니다. 이를 통해 생성과 업데이트 시 ID 유무에 따라 구분하여 사용할 수 있도록 개선했습니다. ac610b2

🔁 변경 사항

📸 스크린샷

👀 기타 더 이야기해볼 점

✅ 체크 리스트

• PR 템플릿에 맞추어 작성했어요.
• 변경 내용에 대한 테스트를 진행했어요.
• 프로그램이 정상적으로 동작해요.
• PR에 적절한 라벨을 선택했어요.
• 불필요한 코드는 삭제했어요.

[eomgerm]

DTO에 기본값을 설정하는 건 그닥 좋지 않아보이는데 플로우 생성 시 인원 수를 어떻게 할 건지 논의해봐야할 듯 하네요

[88dldl]

DTO에 기본값을 설정하는 건 그닥 좋지 않아보이는데 플로우 생성 시 인원 수를 어떻게 할 건지 논의해봐야할 듯 하네요

값이 null로 들어왔을 때 초기화하는 로직을 추가하기보다 이 방법이 깔끔하다고 생각했는데
다른 방법 있으시면 공유부탁드려요~ 저도 고민해보겠습니다!

[88dldl]

DTO에 기본값을 설정하는 건 그닥 좋지 않아보이는데 플로우 생성 시 인원 수를 어떻게 할 건지 논의해봐야할 듯 하네요

값이 null로 들어왔을 때 초기화하는 로직을 추가하기보다 이 방법이 깔끔하다고 생각했는데
다른 방법 있으시면 공유부탁드려요~ 저도 고민해보겠습니다!

관련해서 조금 찾아보니까 자바에서는 보통 DTO에서는 값만 받고 서비스 레이어에서 디폴트 값을 적용하는 방식이 일반적이더라고요!

제가 요즘 코틀린으로 개발하고 있어서 이런 부분을 깊게 생각하지 못했네요😅
피드백 감사드리고, 해당 내용 반영해서 수정하겠습니다!

[eomgerm]

늦어서 죄송합니다...! 조별 인원은 옵션으로 받을 예정이라 DB단에서부터 수정들어가야할 것 같습니다.

[88dldl]

늦어서 죄송합니다...! 조별 인원은 옵션으로 받을 예정이라 DB단에서부터 수정들어가야할 것 같습니다.

현재 buildFlow 메서드에서 아래와 같이 Flow.builder()를 통해 객체를 생성하고 있습니다.

Flow.builder()
    .participants(request.getParticipants())
    .totalPlayTime(request.getTotalPlayTime())
    .title(request.getTitle())
    .imageUrl(imageUrl)
    .author(user);

=> Builder에서 participants() 값을 명시적으로 설정하고 있기 때문에 엔티티에 디폴트값을 설정하더라도 request.getParticipants()값이 우선시됩니다.
request.getParticipants()가 null인 경우 그대로 null이 들어가기 때문에, DB 단에서 디폴트값을 넣어 처리하는 것은 어려울 것 같은데, 혹시 제가 다른 방향으로 이해했거나, 다른 좋은 방법이 있다면 알려주세요!

=> 제 생각에는 FlowConverter에서 빌더 패턴으로 객체를 만드는 메서드가 buildFlow 메서드로 통일되어 있으니,
이 부분에서 아래처럼 수정하는 게 깔끔할 것 같습니다

  FlowBuilder builder =
          Flow.builder()
                  .participants(Optional.ofNullable(request.getParticipants()).orElse(0))
                  .totalPlayTime(request.getTotalPlayTime())
                  .title(request.getTitle())
                  .imageUrl(imageUrl)
                  .author(user);

지금은 서비스 단에서 일반화하고 있는데, DTO → Entity 변환 책임은 converter단에서 처리하는게 더 적절한거 같다고 생각합니다