[refactor] #185 - schedule, staff, cast의 add, delete, update DTO를 하나의 DTO로 변경 및 scheduleNumber를 서버측에서 부여하도록 변경

[hoonyworld]

Related issue 🛠

• closes [refactor] schedule, staff, cast의 add, delete,update DTO를 하나의 DTO로 합치고 서비스 로직에서 add, delete, update를 판단하여 처리하도록 변경 #185

Work Description ✏️

• add, delete, update Request DTO를 하나의 Request DTO로 합치고 서비스 레이어에서 add, delete, update를 판단하여 주어진 로직을 수행도록 변경하였습니다.
• for문을 돌려서 각 서비스 로직에 맞게끔 회차를 추가, 삭제, 업데이트를 하도록 하였습니다.
• scheduleNumber를 클라이언트 측에서 보내주는게 아닌 서버측에서 회차별 날짜로 재정렬하여 회차번호를 부여하도록 하였습니다.
• 해당 공연에 속하지 않은 회차, 등장인물, 스태프를 수정하려고 할 때 403 에러를 발생시키도록 하였습니다.

Trouble Shooting ⚽️

• 회차 요청을 보낼 때 총 만들어지는 회차의 수는 3개가 맞지만 3개를 초과했다는 400에러 발생하였습니다.
• 회차 삭제 작업이 먼저 완료되지 않아서 새로운 회차를 추가할 때 회차 제한을 초과하게 되는 상황이였습니다.
• 따라서 회차 삭제 작업을 먼저 완료하도록 코드를 수정하여 문제를 해결했습니다.

Related ScreenShot 📷

수정 시 공연 정보 GET 했을 때 현재 상태

• scheduleId 55, 56인 회차가 존재합니다.
• castId 33, 34인 등장인물이 존재합니다.
• staffId 34, 35인 스태프가 존재합니다.

회차, 등장인물, 스태프 수정 테스트1

Do

• scheduleId가 55, 56이며 회차 하나를 추가하고, 55번 회차를 수정하고, 56번 회차를 삭제
• castId가 33, 34이며 등장인물 두개 다 모두 삭제
• staffId가 34, 35이며 34, 35번 스태프를 삭제하고, 스태프를 두개 더 추가

Assume

• 회차는 scheduleId 55, 57가 남게되고, PerformanceDate가 scheduleId 57이 더 작기 때문에 scheduleNumber가 FIRST로 바뀌고, scheduleId 55는 scheduleNumber가 FIRST에서 SECOND로 바뀌어 응답이 올 것이다.
• 등장인물은 모두 삭제되어 빈 리스트로 응답이 올 것이다.
• 스태프는 staffId 36, 37으로 새로운 스태프가 등록되어 응답이 올 것이다.

테스트 1 결과

• 수정 요청 시 받은 응답과, 새롭게 수정 시 공연 정보 GET 했을 때를 둘 다 비교해 본 결과, 가정한 결과와 동일한 것을 볼 수 있습니다.

회차, 등장인물, 스태프 수정 테스트2

• performanceId가 16인 새로운 공연을 만들었고, 이를 활용해 테스트를 하였습니다.

Do

• scheduleId가 66, 67이며, 66, 67번 회차를 삭제하고 새로운 회차를 3개를 추가
• castId가 35, 36이며 35를 삭제하고 새로운 등장인물 2개 추가하고 36을 업데이트
• staffId가 42, 43이며 스태프는 둘 다 정보를 업데이트

Assume

• 회차는 scheduleId 새로운 회차 id가 autoIncrement로 등록되어 응답이 올 것이다. 공연 날짜 순서대로 2023-12-29T19:00:00가 FIRST, 2023-12-29T19:30:00가 SECOND, 2023-12-31T19:00:00가 THIRD로 등록이 되어 응답이 올 것이다.
• 등장인물은 castId 36의 정보가 업데이트 되고 37, 38로 새로운 등장인물이 등록되어 응답이 올 것이다.
• 스태프는 staffId 42, 43의 정보가 새롭게 업데이트 되어 응답이 올 것이다.

테스트 2 결과

• 수정 요청 시 받은 응답과, 새롭게 수정 시 공연 정보 GET 했을 때를 둘 다 비교해 본 결과, 가정한 결과와 동일한 것을 볼 수 있습니다.

회차 요청 시 만들어지는 회차가 3개 초과 시 에러처리

• 이전 코드에서 처리했지만, 로직이 변경되었기 때문에 다시 한 번 에러처리 테스트를 해보았습니다.
• 회차 4개를 만들어야 하는 요청을 보냈고, 잘 에러처리가 되어있음을 확인하였습니다.

존재하지 않는 회차, 등장인물, 스태프 에러처리

• 이전 코드에서 처리했지만, 로직이 변경되었기 때문에 다시 한 번 에러처리 테스트를 해보았습니다.
• 존재하지 않는 scheduleId, castId, staffId로 요청을 보냈을 때 에러가 발생함을 확인하였습니다.

해당 공연의 메이커가 아닐 경우 에러처리

• 역시 이전 코드에서 처리했지만, 로직이 변경되었기 때문에 다시 한 번 에러처리 테스트를 해보았습니다.
• 일치하지 않으면 에러를 발생시키도록 했고, 잘 에러처리가 되어있음을 확인하였습니다.

예매자가 존재하지 않는데 가격을 동일하게 해서 요청

• 예매자가 존재하지 않는 경우 가격 수정이 가능합니다.
• 기존 가격과 동일하게 설정해서 요청을 하였기에, 가격 업데이트 메서드 호출되어 가격 업데이트는 되지만, 가격은 그대로이기 때문에 가격 변동은 없습니다.

예매자가 존재하지 않는데 가격을 동일하게 해서 요청한 후 공연 정보 GET

• 가격이 그대로인 것을 확인할 수 있습니다.

예매자가 존재하지 않는데 가격을 다르게 해서 요청

• 예매자가 존재하지 않는 경우 가격 수정이 가능합니다.
• 기존 가격과 다르게 10만원을 설정해서 요청을 하였고 티켓 가격 업데이트 메서드가 호출되어, 티켓 가격이 10만원으로 업데이트 되어야 합니다.
• response에서는 티켓 가격이 10만원으로 잘 오는 것을 확인할 수 있습니다.

예매자가 존재하지 않는데 가격을 다르게 해서 요청한 후 공연 정보 GET

• 가격이 10만원으로 변동된 것을 확인할 수 있습니다.

예매 진행 후 isBookerExist true 확인

• 비회원 예매를 진행 후 공연 정보를 GET 해보았고 isBookerExist가 true인 것을 확인했습니다.

예매자가 존재하고 가격을 다르게 해서 요청

• 예매자가 존재하는 경우 가격 수정이 불가능합니다.
• 기존 10만원에서 14만원으로 수정하려 했으나 에러가 발생하는 것을 볼 수 있습니다.

예매자가 존재하며, 가격을 다르게 해서 요청한 후 공연 정보 GET

• 10만원으로 티켓 가격이 변동 없는 것을 확인할 수 있습니다.

예매자가 존재하고 가격을 동일하게 해서 요청

• 예매자가 존재하는 경우 가격 수정이 불가능합니다.
• 기존 가격과 동일하게 10만원을 설정해서 요청을 하였고 티켓 가격 업데이트 메서드가 호출되지 않아, 기존 가격을 유지해야 합니다.
• response에서는 티켓 가격이 기존 가격인 10만원으로 잘 오는 것을 확인할 수 있습니다.

예매자가 존재하며, 가격을 동일하게 해서 요청한 후 공연 정보 GET

• 기존 가격 10만원이 변동 없이 잘 저장되어 있는것을 확인할 수 있습니다.

음수로 티켓 가격 업데이트시 예외처리

• 티켓 가격을 음수로 업데이트 할 시 예외가 잘 발생함을 확인할 수 있습니다.

해당 공연에 속하지 않은 회차, 등장인물, 스태프를 수정하려고 할 때 에러처리

• 순서대로 해당 공연에 속하지 않은 회차, 등장인물, 스태프를 수정하려고 할 때 에러처리가 잘 작동하는 것을 확인할 수 있습니다.

Uncompleted Tasks 😅

To Reviewers 📢

[hyerinhwang-sailin]

기존 다른 코드에서는 멤버 및 공연 관련 권한 확인을 아래처럼 따로 메소드로 분리하지 않고 필요한 메소드 내에서 작성하셨었는데 이렇게 분리할 경우의 이점이 뭔가요?

Member member = memberRepository.findById(memberId)
                .orElseThrow(() -> new NotFoundException(MemberErrorCode.MEMBER_NOT_FOUND));

        Long userId = member.getUser().getId();

        Performance performance = performanceRepository.findById(ticketDeleteRequest.performanceId())
                .orElseThrow(() -> new NotFoundException(BookingErrorCode.NO_PERFORMANCE_FOUND));

        if (!performance.getUsers().getId().equals(userId)) {
            throw new ForbiddenException(PerformanceErrorCode.NOT_PERFORMANCE_OWNER);
        }
     

그런데 이렇게 분리하게 되면 한 서비스 내에서 검증이 반복되는 경우의 코드를 줄일 수 있을 것 같네요! 검증 과정이 반복되는 다른 서비스에 적용하면 좋을 것 같습니다!

[hoonyworld]

네 맞습니다!! 저렇게 분리하게 되면 좀 더 책임 분리가 확실해지고 가독성도 높아집니다!

하지만 저희가 지금 설계한 Service가 재사용성이 부족해서 기능 구현 후 2차 스프린트 때, 전반적으로 코드 리팩토링을 하고 싶은데 어떻게 생각하시나요? (DTO 부분에 대해서도 네이밍 부분을 조금 손을 봐야할 것 같습니다)

[hyerinhwang-sailin]

저도 전반적인 코드 리팩토링 필요하다고 생각합니다! 같이 잘 논의해봐요~

[hyerinhwang-sailin]

현재 한 공연당 스케쥴을 최대 3개 등록할 수 있어 addSchedule 메소드에서 스케쥴을 추가하기 전에 추가했을 때 3개가 초과하지 않는지 확인하는 로직을 하단에 작성하셨는데, 만약 기존 스케쥴 수가 3개였고 하나를 삭제, 하나를 추가할 경우 추가로직이 먼저 실행되어 정상적으로 반영되지 못할 것 같습니다.
추가 전에 삭제를 먼저 하는 것이 안전할 것 같은데, 아래와 같은 방식은 어떤가요?

    // 요청에 포함된 스케줄 ID를 사용하여 기존 스케줄 목록에서 삭제할 스케줄을 제외합니다.
    scheduleRequests.forEach(request -> {
        if (request.scheduleId() != null) {
            existingScheduleIds.remove(request.scheduleId());
        }
    });

    // 요청에 포함되지 않은 기존 스케줄은 삭제합니다.
    deleteRemainingSchedules(existingScheduleIds);

    // 스케줄 요청에 따라 추가 또는 업데이트된 스케줄 객체를 생성합니다.
    List<Schedule> schedules = scheduleRequests.stream()
            .map(request -> {
                if (request.scheduleId() == null) {
                    return addSchedule(request, performance);
                } else {
                    return updateSchedule(request, performance);
                }
            })
            .collect(Collectors.toList());

[hoonyworld]

맞습니다 삭제를 먼저해야 위와 같은 불상사를 예방할 수 있습니다!

공연에 현재 존재하는 스케줄 ID 가져오기 -> 요청된 스케줄 ID 추출 -> 삭제할 스케줄 ID 결정(클라이언트 요청에 포함되지 않은 스케줄) -> 삭제할 스케줄 처리

의 순서로 회차를 삭제한 후 추가 및 업데이트 하도록 리팩토링 해두었습니다~

[hyerinhwang-sailin]

존재하는 id리스트를 existingScheduleIds에 저장한 후 비교 작업하는 아이디어 너무 좋네요!
schedule을 직접적으로 비교하지 않고 이렇게 id만 리스트화시켜서 비교할 때의 장점이 어떻게 되나요?

[hoonyworld]

schedule 객체를 조회하는 쿼리는 전체 데이터를 메모리로 가져와서 전송해야 하므로 무거울 수 있지만, ID만 조회하는 쿼리는 필요한 데이터 양이 적어 가볍기 때문에 성능이 더 효율적일 것 같습니다!

[hyerinhwang-sailin]

ID만 조회하는 게 확실히 더 효율적이겠네요!! 답변 감사합니다~

[hyerinhwang-sailin]

볼륨이 큰 리팩토링 작업이었을텐데 정말 고생 많으셨습니다!!
꼼꼼한 log 처리, 테스트까지 최고입니다~