-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/287 봉사 기록 엔티티와 이벤트 기반 생성 방식 구현 #293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
m-a-king
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~
|
|
||
| return switch (DomainEventSubType.from(eventType)) { | ||
| case CREATE_RECRUIT_BOARD -> parseCreateRecruitBoardEvent(message); | ||
| case VOLUNTEER_HOURS_SETTLE -> null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조치해주셔서 감사합니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 null인 이유는 추후 구현인건가요?
로직을 몰라서 여쭤봅니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 구조 자체가 조금 잘못돼서 추후에 수정해야하는 부분입니다.
| apply.changeAttended(true); | ||
| updateVolunteerUseCase.updateVolunteerStats(apply.getVolunteerId(), hours); | ||
| publishVolunteerReviewRequestEvent(apply, recruitBoard); | ||
| volunteerRecordEventPublisher.publishVolunteerRecordCreateEvent(apply, recruitBoard); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serverEventPublisher를 활용해서 추상화하는 것이 좋을 것 같습니다.
혹시 봉사 기록 이벤트만을 위한 퍼블리셔를 만들고 의존하신 특별한 이유가 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹은 봉사 기록 이벤트 내부에서 정적 팩토리 메서드를 활용할 수 있을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
처음에 이벤트에 대한 개념이 확실히 잡히지 않아서
봉사 시간 정산시 봉사 기록 생성 이벤트를 발행한다 라는 의미로 생각하고 메서드를 만들었습니다
현재는 봉사 시간 정산 이벤트가 발생하고 이를 구독하는 클래스에서 자신들이 필요한 로직을 수행하도록
이벤트 발행과 구독을 완전히 분리하는 방향을 생각하고 있는데요
봉사 시간 정산 이벤트가 발생하면 RedisSettleVolunteerHoursSubscriber 에서 이를 구독하여
그 안에서 메세지를 핸들링 하는것을 구상중인데 혹시 이 부분에 대해서는 어떻게 생각하시나요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
publishVolunteerReviewRequestEvent(apply, recruitBoard);
volunteerRecordEventPublisher.publishVolunteerRecordCreateEvent(apply, recruitBoard);
이거 방식이 달라서 상의해서 한가지로 통합하는게 좋아보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@7zrv 좋습니다. 봉사 시간 정산 이벤트가 발생하면 그것을 봉사 시간 정산 이벤트 구독자가 받고 그제야 봉사 시간 정산 이벤트로부터 발생해야 하는 리뷰 요청 알림 이벤트와 봉사 기록 생성 이벤트로 나누어져서 두 번의 이벤트를 발생시키면 좋을 것 같습니다.
두 번의 이벤트가 발생하는 시점에서 범수 님이 말해주신 것 중에서 골라야 하는데, 제 생각에는 이벤트를 발생시킨다는 개념은 serverEventPublisher에게 맡기고 이벤트는 이벤트 클래스의 정적 메서드를 활용해서 생성하는 게 좋을 것 같다고 생각했습니다.
@leebs0521
-> publishVolunteerReviewRequestEvent(apply, recruitBoard) 메서드를 사용하자. 하지만 빌더 패턴을 드러내지 말자 (이벤트 클래스의 정적 팩토리 메서드를 활용하자)
-> 별도의 클래스(volunteerRecordEventPublisher)에서 이벤트를 생성하고 그것을 의존하는 현재 구조는 과하지 않을까?
라고 생각했습니다!
| @Component | ||
| public class VolunteerRecordMessageConverter { | ||
|
|
||
| private static final String SUB_TYPE = "subType"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋네요~
| private VolunteerRecord buildVolunteerRecordCreate(String message) throws JsonProcessingException { | ||
|
|
||
| VolunteerRecordCreateEvent volunteerRecordCreateEvent = objectMapper.readValue(message, VolunteerRecordCreateEvent.class); | ||
|
|
||
| return VolunteerRecord.create( | ||
| volunteerRecordCreateEvent.getVolunteerId(), | ||
| volunteerRecordCreateEvent.getTitle(), | ||
| volunteerRecordCreateEvent.getVolunteerDate(), | ||
| volunteerRecordCreateEvent.getVolunteerHours() | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빌더를 드러내지 않고 create 하는 것 확인했습니다~
메서드 명도 create~~~로 바꿔도 될 것 같은데 create~~~create라서 어색해서 build라고 하셨나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 문맥상 어색해보여서 크리에이트를 빌드한다 라는 문장으로 적은건데
이제 보니 CreateVolunteerRecord라고 해도될 거 같네요 어떻게 생각하시나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 재중님 의견이 괜찮아보이네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 좋습니다~
|
|
||
| settleVolunteerHoursHandler.handle(volunteerRecord); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개행이 필요하대요!
ayoung-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
leebs0521
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨씁니다.
| class VolunteerRecordTest { | ||
|
|
||
| @Test | ||
| void createVolunteerRecord_success() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@display가 있으면 좋을것 같습니다.
| @Service | ||
| @Transactional | ||
| public class VolunteerRecordCreateService implements VolunteerRecordCreateUseCase { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 서비스 클래스들은 CreateDomainService 이런식으로 지었는데 여기만 DomainCreateService로 지은 이유가 있으신가요?
| private VolunteerRecord buildVolunteerRecordCreate(String message) throws JsonProcessingException { | ||
|
|
||
| VolunteerRecordCreateEvent volunteerRecordCreateEvent = objectMapper.readValue(message, VolunteerRecordCreateEvent.class); | ||
|
|
||
| return VolunteerRecord.create( | ||
| volunteerRecordCreateEvent.getVolunteerId(), | ||
| volunteerRecordCreateEvent.getTitle(), | ||
| volunteerRecordCreateEvent.getVolunteerDate(), | ||
| volunteerRecordCreateEvent.getVolunteerHours() | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 재중님 의견이 괜찮아보이네요
|
|
||
| return switch (DomainEventSubType.from(eventType)) { | ||
| case CREATE_RECRUIT_BOARD -> parseCreateRecruitBoardEvent(message); | ||
| case VOLUNTEER_HOURS_SETTLE -> null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기 null인 이유는 추후 구현인건가요?
로직을 몰라서 여쭤봅니다
| apply.changeAttended(true); | ||
| updateVolunteerUseCase.updateVolunteerStats(apply.getVolunteerId(), hours); | ||
| publishVolunteerReviewRequestEvent(apply, recruitBoard); | ||
| volunteerRecordEventPublisher.publishVolunteerRecordCreateEvent(apply, recruitBoard); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
publishVolunteerReviewRequestEvent(apply, recruitBoard);
volunteerRecordEventPublisher.publishVolunteerRecordCreateEvent(apply, recruitBoard);
이거 방식이 달라서 상의해서 한가지로 통합하는게 좋아보입니다.
- 봉사 기록 엔티티 추가 - 엔티티 생성 메서드 구현
- 레포지토리 인터페이스 생성 - Jpa 레포지토리 생성 - 레포지토리 구현체 생성
- 엔티티 생성 메서드 테스트및 검증 완료
- 봉사 기록 등록 유스케이스 생성 - 봉사 기록 등록 구현체 생성 - 테스트 코드 작성및 검증 완
- 도메인 이벤트 타입에 봉사 시간 정산 이벤트 등록 - 발행될 봉사 시간 정산 이벤트 클래스 생성 - 봉사 시간 정산 이벤트 퍼블리셔 생성 - 구독을 위한 봉사 시간 정산 이벤트 RedisSubscriber 생성 - RedisListenerRegistrar애 구독 클래스 추가 - 봉사 시간 정산 이벤트 메세지 컨버터 생성 - 봉사 기록 생성 이벤트 핸들러 인터페이스 생성 - 봉사 기록 생성 이벤트 핸들러 구현체 생성 - 봉사 시간 정산 메서드에 봉사 기록 생성 이벤트 의존성 주입 - 테스트 코드 작성및 검증 완료
- 추가된 도메인 이벤트 타입으로 인해 발생한 에러 해결을 위해 case추가
- 이벤트 발행 테스트의 분리
- RedisSettleVolunteerHoursSubscriber 클래스에 마지막 라인 개행추가 - VolunteerRecordMessageConverter의 봉사 기록 객체 생성 메서드 이름 변경
468c35b to
7ff6c7e
Compare
|



📌 과제 설명
봉사 기록 테이블이 추가됨에 따라 해당 데이터에 대한 엔티티와
봉사 시간 정산시 발행하는 이벤트 기반 생성 기능의 구현입니다.
👩💻 요구 사항과 구현 내용
✅ PR 포인트 & 궁금한 점
이벤트 기반 방식이 처음이라 어색한 부분이 있을수도 있을거 같습니다
제가 작성한 코드가 이벤트 발행 ~ 봉사 기록 생성까지의 논리가 정확한지 봐주시면 감사하겠습니다.