Skip to content
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.

Sign up for GitHub

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

[feat/CK-242] 로드맵 관련 부분 의존성 리팩토링을 한다 #206

Merged
merged 23 commits into from
Mar 27, 2024
Merged

[feat/CK-242] 로드맵 관련 부분 의존성 리팩토링을 한다 #206

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d72b469
refactor: roadmap 관련 코드 패키지 분리
Ohjintaek Dec 19, 2023
2ea6239
refactor: RoadmapCreateService에서 goalRoom 의존성 제거
Ohjintaek Dec 19, 2023
91f128b
refactor: RoadmapReadService에서 goalRoom 의존성 제거
Ohjintaek Dec 19, 2023
7085619
refactor: RoadmapScheduler에서 goalRoom 의존성 제거 및 테스트코드 패키지 분리
Ohjintaek Dec 19, 2023
3c0ba17
refactor: Roadmap에서 발생한 예외는 RoaadmapException을 던지도록 수정
Ohjintaek Dec 22, 2023
354f9b6
refactor: RoadmapSaveArgumentResolver 의존성 역전
Ohjintaek Dec 22, 2023
c8fc8f4
test: 테스트 코드 수정
Ohjintaek Dec 22, 2023
a307525
chore: build.gradle 수정 (jacoco 테스트 커버리지)
Ohjintaek Dec 22, 2023
7ce0184
refactor: roadmap과 roadmapReview에서 member 직접 참조를 삭제
Ohjintaek Jan 12, 2024
b63f1b7
chore: ScrollResponseMapper common으로 이관
Ohjintaek Jan 13, 2024
a0f588b
refactor: 로드맵 삭제 시 하위 골룸 삭제 방식 변경 (OnDelete 옵션 사용)
Ohjintaek Jan 13, 2024
3d6cd17
chore: 패키지 경로 수정
Ohjintaek Jan 13, 2024
df672b0
refactor: dto 패키지 분리
Ohjintaek Jan 13, 2024
7a1b1f3
refactor: Event 객체에서 도메인 직접 전달 제거
Ohjintaek Jan 14, 2024
3c5fc04
refactor: RoadmapNode에서 RoadmapContent 의존 제거
Ohjintaek Jan 14, 2024
9cf3c2a
refactor: Roadmap과 RoadmapContent, RoadmapReview간 의존 관계 제거
Ohjintaek Jan 14, 2024
bdccbbc
refactor: Roadmap 생성 시 태그, RoadmapContent 생성 시 노드들을 필수로 넣도록 수정
Ohjintaek Jan 21, 2024
5cf8113
test: RoadmapNodes 테스트코드 추가
Ohjintaek Jan 21, 2024
a0472bc
refactor: 불필요한 클래스 삭제
Ohjintaek Feb 15, 2024
732673f
fix: 데이터베이스 무결성 오류 수정
Ohjintaek Feb 15, 2024
fc761d5
refactor: 로드맵 삭제 시 연관된 리뷰 삭제를 이벤트로 발행
Ohjintaek Feb 16, 2024
ac71f6e
chore: merge conflict 해결
Ohjintaek Mar 27, 2024
3a7e610
refactor: 불필요한 이벤트 호출 제거
Ohjintaek Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion backend/kirikiri/src/main/java/co/kirikiri/roadmap/service/RoadmapCreateService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import co.kirikiri.roadmap.service.dto.request.RoadmapReviewSaveRequest;
import co.kirikiri.roadmap.service.dto.request.RoadmapSaveRequest;
import co.kirikiri.roadmap.service.event.RoadmapCreateEvent;
import co.kirikiri.roadmap.service.event.RoadmapDeleteEvent;
import co.kirikiri.roadmap.service.mapper.RoadmapMapper;
import co.kirikiri.service.aop.ExceptionConvert;
import co.kirikiri.service.exception.AuthenticationException;
Expand Down Expand Up @@ -145,8 +146,8 @@ public void deleteRoadmap(final String identifier, final Long roadmapId) {
final Roadmap roadmap = findRoadmapById(roadmapId);
validateRoadmapCreator(roadmapId, identifier);
if (!roadmapGoalRoomService.hasGoalRooms(roadmapId)) {
applicationEventPublisher.publishEvent(new RoadmapDeleteEvent(roadmap.getId(), "Review"));
roadmapContentRepository.deleteAllByRoadmapId(roadmapId);
roadmapReviewRepository.deleteAllByRoadmapId(roadmapId);
roadmapRepository.delete(roadmap);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 삭제하는 부분을 트랜잭션에서 제거하기 위해서 @EventListener를 쓰신거죠?!
리뷰가 로드맵을 외래키로 갖고 있어서 만약 이벤트 리스너에서 리뷰를 삭제하는 로직을 수행하지 못하고 에러가 났을때, 트랜잭션으로 묶여있지 않아서 로드맵 삭제하는 코드를 실행할 때 에러가 날 수도 있을 것 같아요.
썬샷이 말씀해주신 외래키 때문에 트랜잭션을 분리할 수 없다는게 맞군요..!
외래키 제약조건을 없애거나 soft delete를 하는 방식이 더 좋을수도 있겠네요! (이렇게 하자는건 아닙니다)

우선은 이대로도 좋고 썬샷이 조금 더 옳다고 판단되는 방식으로 해주시면 좋을 것 같아요 🙂

return;
}
Expand Down
26 changes: 26 additions & 0 deletions .../kirikiri/src/main/java/co/kirikiri/roadmap/service/RoadmapReviewDeleteEventListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package co.kirikiri.roadmap.service;

import co.kirikiri.roadmap.persistence.RoadmapReviewRepository;
import co.kirikiri.roadmap.service.event.RoadmapDeleteEvent;
import lombok.RequiredArgsConstructor;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Service;

@Service
@RequiredArgsConstructor
public class RoadmapReviewDeleteEventListener {

private final RoadmapReviewRepository roadmapReviewRepository;
private final ApplicationEventPublisher applicationEventPublisher;

@EventListener(condition = "#roadmapDeleteEvent.target == 'Review'")
public void handleRoadmapReviewDelete(final RoadmapDeleteEvent roadmapDeleteEvent) {
deleteRoadmapReviews(roadmapDeleteEvent.roadmapId());
}

private void deleteRoadmapReviews(final Long roadmapId) {
roadmapReviewRepository.deleteAllByRoadmapId(roadmapId);
applicationEventPublisher.publishEvent(new RoadmapDeleteEvent(roadmapId, "Roadmap"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 코드는 왜 있는건가요?!
이벤트 리스너 내에 로드맵 삭제를 처리하는 메서드가 없는 것 같은데요..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 원래 로드맵 리뷰 제거와 로드맵 제거 간의 트랜잭션을 분리해주려고 하려고 의도했어서 저렇게 했었습니다.
근데 외래키 제약조건 때문에 반드시 로드맵 리뷰가 모두 삭제된 뒤에 로드맵이 제거되어야 해서 의미가 없어진 부분이네요
삭제해주었습니다 😅

}
}
7 changes: 7 additions & 0 deletions backend/kirikiri/src/main/java/co/kirikiri/roadmap/service/event/RoadmapDeleteEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package co.kirikiri.roadmap.service.event;

public record RoadmapDeleteEvent(
Long roadmapId,
String target
) {
}
7 changes: 7 additions & 0 deletions backend/kirikiri/src/main/java/co/kirikiri/roadmap/service/scheduler/RoadmapScheduler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

import co.kirikiri.roadmap.domain.Roadmap;
import co.kirikiri.roadmap.domain.RoadmapStatus;
import co.kirikiri.roadmap.persistence.RoadmapContentRepository;
import co.kirikiri.roadmap.persistence.RoadmapRepository;
import co.kirikiri.roadmap.service.RoadmapGoalRoomService;
import co.kirikiri.roadmap.service.event.RoadmapDeleteEvent;
import co.kirikiri.service.aop.ExceptionConvert;
import lombok.RequiredArgsConstructor;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -19,7 +22,9 @@
public class RoadmapScheduler {

private final RoadmapRepository roadmapRepository;
private final RoadmapContentRepository roadmapContentRepository;
private final RoadmapGoalRoomService roadmapGoalRoomService;
private final ApplicationEventPublisher applicationEventPublisher;

@Scheduled(cron = "0 0 4 * * *")
public void deleteRoadmaps() {
Expand All @@ -34,6 +39,8 @@ private void delete(final Roadmap roadmap) {
final boolean canDelete = roadmapGoalRoomService.canDeleteGoalRoomsInRoadmap(roadmap.getId());
// TODO : GoalRoom 내부의 Roadmap 직접 의존 제거 시 로드맵에 포함된 GoalRoom 따로 제거해주기 (이벤트 활용)
if (canDelete) {
applicationEventPublisher.publishEvent(new RoadmapDeleteEvent(roadmap.getId(), "Review"));
roadmapContentRepository.deleteAllByRoadmapId(roadmap.getId());
roadmapRepository.delete(roadmap);
}
}
Expand Down