refactor: 약속 단건 조회 API 구현

[eun-byeol]

🚩 연관 이슈

close #224

📝 작업 내용

#245 리뷰 반영했습니다.

• @ArraySchema
• @errorcode404
• LocalDate, LocalTime JsonFormat 적용

🏞️ 스크린샷 (선택)

🗣️ 리뷰 요구사항 (선택)

[github-actions]

Test Results

65 tests   65 ✅  3s ⏱️
18 suites   0 💤
18 files     0 ❌

Results for commit 0dbd363.

♻️ This comment has been updated with latest results.

[hyeon0208]

꼼꼼하게 테스트까지 작성해주셨네요 👍🏻
approve 드리고 갑니다 😄

[hyeon0208]

예외 상황까지 꼼꼼한 테스트 👍🏻

[hyeon0208]

👍🏻

[hyeon0208]

[질문]
pattern으로 시간 분만 응답으로 나가게 되었는데
타입을 String으로 변환한 이유가 있나요 ??

[mzeong]

[제안] 컨트롤러 단에서 @PathVariable Long meetingId 코드가 쓰여 Long이 사용된 것 같은데 null이 들어올 수 없을 것 같아요. long을 사용하는 것은 어떤가요?

[eun-byeol]

반영했습니다~👍

[mzeong]

[질문] 해당 메서드가 dto인 MeetingWithMatesResponse를 반환하게 하지 않고 List<Mate>를 반환하게 한 이유가 있나요?

[eun-byeol]

두 가지 이유가 있었어요.

1. 약속 1건 조회의 책임이 Mate보다는 Meeting에 있다고 생각했어요.
   물론, Mate는 Meeting을 알고 있어서 Mate에서 Response를 반환할 수도 있겠지만,
   MeetingService가 MateService를 호출만 하게 되는 구조가 어색한 것 같아요.

2. MateService에서 바로 반환하면, MateRepository의 재사용성이 매우 낮다고 생각했어요.
   MateService에서 Response를 반환하게 되면, 사실상 MeetingService는 불필요해요.
   MateService에서 모든 로직을 처리하고, MateRepository 한방쿼리로 해결할 수 있을 거예요. 당장은 Mate 인원수가 최대 8명이고, 성능이 중요한 케이스는 아니라고 생각해요. Repository 재사용성 측면을 더 고려했습니다!

[coli-geonwoo]

조조! 구현하느라 고생 많으셨어요! 몇가지 리뷰 사안 적었어요!

테스트 코드 상에서 반영이 꼭 필요한 부분 생겨 우선 RC로 남겨놓습니다.

[coli-geonwoo]

[제안-선택]

커스텀 직렬화 도구를 통해 LocalDate와 LocalTime 형식을 지정할 수 있어요. json으로 반환할 때 우리가 반환로직을 지정해줄 수 있습니다. 매번 LocalDate나 LocalTime마다 jsonformat을 붙이기 보다 하나의 클래스 선언으로 모든 LocalTime과 LocalDate의 형식을 지정해줄 수 있습니다.

ex) yyyy-MM-dd로 형식 맞추는 클래스
다만 조금 시간이 걸리는 리팩터링 사안이라 반영은 조조 의사에 맡길게요!!

@JsonComponent
public class CustomLocalDateSerializer extends JsonSerializer<LocalDate> {
    private static final DateTimeFormatter TIME_FORMATTER = new DateTimeFormatterBuilder()
            .appendPattern("yyyy-MM-dd")
            .toFormatter()
            .withZone(ZoneId.of("Asia/Seoul"));

    @Override
    public void serialize(
            LocalDate localDate,
            JsonGenerator jsonGenerator,
            SerializerProvider serializerProvider
    ) {
        try {
            jsonGenerator.writeString(TIME_FORMATTER.format(localDate));
        } catch (IOException exception) {
            throw new RuntimeException("날짜 변환 과정에서 문제가 발생했습니다.");
        }
    }
}

@JsonComponent
public class CustomLocalTimeSerializer extends JsonSerializer<LocalTime> {
    private static final DateTimeFormatter TIME_FORMATTER = new DateTimeFormatterBuilder()
            .appendPattern("HH:mm")
            .toFormatter()
            .withZone(ZoneId.of("Asia/Seoul"));

    @Override
    public void serialize(
            LocalTime time,
            JsonGenerator jsonGenerator,
            SerializerProvider serializerProvider
    ) {
        try {
            jsonGenerator.writeString(TIME_FORMATTER.format(time));
        } catch (IOException exception) {
            throw new RuntimeException("시간 변환 과정에서 문제가 발생했습니다.");
        }
    }
}

[eun-byeol]

커스텀 직렬화 도구 사용하는 것은 새롭게 이슈 파서 적용하는 게 좋을 것 같아요.
이미 @jsonformat 적용되어 있는 코드가 많아서, 한 번에 수정하면 좋을 것 같습니다~

[coli-geonwoo]

👍

[coli-geonwoo]

[제안- 필수]

오늘 카키와 발견한 문제인데 Fixture.Mate들은 쓰면 안될 것 같아요. 연관관계가 설정된 엔티티인데 static 객체라서 전체 테스트를 돌릴 때 id가 계속 변하게 되더라고요.

테스트 스트림이 병렬이라 어쩔 땐 mate id가 일치해서 통과하는데 다른 코드가 먼저 돌아가 autoincrement되어 테스트가 깨져요.

Mate는 새로운 객체를 만들어서 사용해야 안전할 것 같습니다. 관련 내용은 내일 더 이야기나눠보아요.

Suggested change

	Long mate1Id = mateRepository.save(Fixture.MATE1).getId();
	Mate mate = new Mate(.......) //mate는 fixtue 쓰지 말것!!
	Long mate1Id = mateRepository.save(mate).getId();

[eun-byeol]

반영 완료했습니다~👍

[coli-geonwoo]

[제안]

순서까지 검증해주는 containsExactly를 쓰면 더 정확할 것 같아요

[eun-byeol]

데이터를 정렬해서 주는 것이 아니라서,
오히려 순서를 보장해서 테스트하는 것이 맞지 않다고 생각했어요!

[eun-byeol]

변경 사항

1. meetingId primitive type으로 변경
2. JsonFormat 불필요한 옵션 제거
3. Fixture.MATE 제거

리뷰 반영한 코드는 여기에서 확인 가능합니다.

[coli-geonwoo]

직접 커밋 변경 범위도 설정해주고 친절한 조조.... 👍

리뷰 사안 최대한 많이 수용해줘서 감사합니다!! approve 합니다!