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.

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

[refactor] : Result와 Stadium의 DDD구조 변경 및 Result 어그리게이트에 관한 간접참조와, 객체탐색 리팩토링을 진행한다. #84

Merged
merged 39 commits into from
Nov 25, 2024

Conversation

juuuunny
Copy link
Contributor

@juuuunny juuuunny commented Nov 23, 2024

✅ PR 유형

어떤 변경 사항이 있었나요?

  • 새로운 기능 추가
  • 버그 수정
  • 코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
  • 코드 리팩토링
  • 주석 추가 및 수정
  • 문서 수정
  • 빌드 부분 혹은 패키지 매니저 수정
  • 파일 혹은 폴더명 수정
  • 파일 혹은 폴더 삭제

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • Result 어그리게이트에 관한 간접참조와, 객체탐색 리팩토링을 진행한다.

✏️ 관련 이슈

본인이 작업한 내용이 어떤 Issue Number와 관련이 있는지만 작성해주세요

DDD 스타일로 작성하셨다고 하셨는데,
User entity는 user 도메인에, Stadium entity는 stadium 도메인에, Result entity는 result domain에 있는것으로 보입니다.
이때, 각각의 도메인의 어그리거트루트는 아래와 같은 이유 때문에 서로 간접참조를 합니다.

유지 보수성
트랜잭션 관리
DB 분리
MSA로 가면서 각 도메인별로 DB가 나눠지는 상황을 배제하고서라도. (DB가 다르면 당연히 직접참조를 할 수 없겠죠?)
Result 도메인이 User와 Stadium을 직접 참조 하고 있다면, Result 도메인에 User의 정보와 Stadium의 정보를 언제든지 변경할 수 있다는 뜻이 됩니다. 이렇게 되면 어디서 User의 정보를 변경할 수 있는지 제어가 되지 않기 때문에, 코드가 늘어났을때 관리하기 매우 힘들어져요.
Result 도메인의 어그리거트 루트는 누구인가요?
DDD에서는 어그리거트 루트에 대한 Repository 하나만 만들고, 어그리거트 루트의 하위 Entity들에 대해서는 객체탐색을 통해 조회해서 사용하게 됩니다.

만약, Result가 어그리거트 루트라면 Profile이나 Zone은 Result를 조회하고, Result를 이용해 객체탐색을 해서 가져와서 사용해야하는것이죠.

이렇게 하는 이유는 여러가지가 있겠지만, 다음과 같은 이유가 크다고 생각해요.

도메인의 생명주기 관리가 용이함.
모든 도메인 로직이 도메인에 들어있기 때문에, 어그리거트 루트를 통해 호출이 시작되면 예외가 없이 적용됨.

기존 코드

@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Result extends BaseTimeEntity {

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "result_id", nullable = false)
    private Long id;

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "stadium_id", nullable = false)
    private Stadium stadium;

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "user_id")
    private User user;

    @Column(nullable = false)
    private String preference;

    ---생략----

수정 코드

@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Table(name = "result")
public class ResultEntity extends BaseTimeEntity {

    @Id
    @GeneratedValue(strategy = GenerationType.IDENTITY)
    @Column(name = "result_id", nullable = false)
    private Long id;

    @Column(name = "user_id", nullable = false)
    private Long userId;

    @Column(name = "stadium_id", nullable = false)
    private Long stadiumId;

    @Column(nullable = false)
    private String preference;

    @OneToOne(mappedBy = "resultEntity", cascade = CascadeType.ALL, orphanRemoval = true)
    private ProfileEntity profileEntity;

    @OneToMany(mappedBy = "resultEntity", cascade = CascadeType.ALL, orphanRemoval = true)
    private List<ZoneEntity> zoneEntities = new ArrayList<>();

    public ResultEntity(
            Long id,
            Long userId,
            Long stadiumId,
            String preference
    ) {
        this.id = id;
        this.userId = userId;
        this.stadiumId = stadiumId;
        this.preference = preference;
    }

    public static ResultEntity toEntity(
            Long id,
            Long userId,
            Long stadiumId,
            String preference
    ) {
        return new ResultEntity(id, userId, stadiumId, preference);
    }

    public void addProfileEntity(ProfileEntity profileEntity) {
        this.profileEntity = profileEntity;
        profileEntity.assignToResultEntity(this);
    }

    public void addZoneEntity(ZoneEntity zoneEntity) {
        this.zoneEntities.add(zoneEntity);
        zoneEntity.assignToResultEntity(this);
    }
}

기존에는 어그리거트 구분을 하지 않고 도메인으로 나누어 도메인 간에는 위와 같이 직접 참조를 통해 서로의 엔티티에 접근할 수 있도록 하였습니다.
하지만, DDD 구조에서 어그리게이트는 자신의 경계 내에서만 일관성을 유지하고 관리하는 단위입니다.
따라서, Result 어그리게이트는 User와 Stadium 어그리게이트와 느슨한 결합을 유지해야 하며, 직접 참조 대신 간접 참조(ID 참조 등)를 사용하는 것이 더 적합하다고 판단하여 수정하였습니다.

또한, 같은 어그리거트 내에 있는 엔티티들일 경우, 어그리거트 루트를 통해서만 하위 엔티티들을 관리 할 수 있습니다.
Result가 어그리거트 루트, Zone과 Profile이 하위 도메인이라고 하면 도메인의 책임과 일관성을 유지하기 위해 어그리게이트 루트인 Result가 중심이 되어 생명주기를 관리하게 하여, Profile이나 Zone이 Result와 분리되지 않고 자연스럽게 연결된 상태를 유지할 수 있도록 리팩토링 하였습니다.

도메인 중심 설계를 위해 DDD구조로 수정하며
presentation, application, domain, infra구조로 변경하였습니다.

기본 규칙

repository는 applicationService에서만 호출한다. (domain에서는 repository에 접근을 하지 않아야 함) 참고
infra repository에서는 인수를 전달받아 로직을 통해 DB에서 엔티티를 추출하고 Mapper를 통해 도메인 객체로 변환하여 반환한다.
domain과 dto사이의 변환은 service에서 변환한다.

application의 메서드에서 해당 도메인 내의 기능만으로 설계가 가능할 경우엔 application Service만 사용하고
메서드에서 repository를 통한 해당 도메인의 DB나 타 어그리거트의 서비스에 접근해야 할 시엔 domain service로 분리한다.
(레포지토리나 타 어그리거트 서비스는 애플리케이션 서비스에서 호출해서 사용한다.)

주의점

도메인의 모델과 인프라의 엔티티에서는 빌더패턴이 아닌 new로 생성자 호출을 해야합니다.
(빌더패턴으로 만들 시 스택오버플로우가 발생합니다. 이유는 추후 찾아볼 예정!)

인프라 계층의 jpa에서
repository를 resultJpaRepository라 하였을 시, repositoryImpl은 resultJpaRepositoryImpl 빼곤 모두 가능하다. (resultJpaRepositoryImpl) 라고 클래스명을 둘 경우 순환 참조가 계속해서 일어나는데 추후 공부하여 이유는 찾아볼 예정!


🎸 기타 사항 or 추가 코멘트

@juuuunny
Copy link
Contributor Author

어그리게이트 루트를 통해 어그리게이트를 구분합니다.

Result 도메인이 User와 Stadium을 직접 참조 하고 있다면, Result 도메인에 User의 정보와 Stadium의 정보를 언제든지 변경할 수 있다는 뜻이 됩니다. 이렇게 되면 어디서 User의 정보를 변경할 수 있는지 제어가 되지 않기 때문에, 코드가 늘어났을때 관리하기 매우 힘들어져요.

-> 위와 같은 이유로 타 어그리게이트일 경우에는 간접참조를 통해서만 정보를 가져옵니다!

@juuuunny
Copy link
Contributor Author

동일 어그리게이트일 경우에는 어그리게이트 루트를 통해서만 하위 엔티티를 생성, 수정, 삭제하며 관리합니다.
이때는 간접참조가 아닌 직접 참조를 할 수 있고, 객체탐색을 통하여 접근합니다.

@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class Result extends BaseTimeEntity {

    ~~생략~~


    @OneToOne(mappedBy = "result", cascade = CascadeType.ALL, orphanRemoval = true)
    private Profile profile;

    @OneToMany(mappedBy = "result", cascade = CascadeType.ALL, orphanRemoval = true)
    private List<Zone> zones = new ArrayList<>();


    public void addProfile(Profile profile) {
        this.profile = profile;
        profile.assignToResult(this);
    }

    public void addZone(Zone zone){
        this.zones.add(zone);
        zone.assignToResult(this);
    }
}

위와 같은 경우에서도 result.addProfile(profile)을 통하여 profile을 생성합니다!

@juuuunny juuuunny self-assigned this Nov 23, 2024
@juuuunny juuuunny added 🔄 refactor 코드 리팩토링 😆 JUNHYEONG 준형 Issue or PR labels Nov 23, 2024
Copy link
Member

@bbbang105 bbbang105 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!! 덕분에 이해가 조금 더 된 것 같아요
LGTM 👍🏻

@juuuunny juuuunny changed the title [refactor] : Result 어그리게이트에 관한 간접참조와, 객체탐색 리팩토링을 진행한다. [refactor] : DDD구조 변경 및 Result 어그리게이트에 관한 간접참조와, 객체탐색 리팩토링을 진행한다. Nov 25, 2024
build.gradle Outdated
Copy link
Contributor Author

@juuuunny juuuunny Nov 25, 2024

Choose a reason for hiding this comment

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

queryDsl 사용을 위한 Q도메인은 infra 계층에서만 사용할 수 있도로 아래와 같이 설정하였습니다.

private final ResultDomainService resultDomainService;
private final ResultRepository resultRepository;
private final JWTUtil jwtUtil;

Copy link
Contributor Author

@juuuunny juuuunny Nov 25, 2024

Choose a reason for hiding this comment

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

ResultRepository 또는 타 어그리거트의 ApplicationService는
ApplicationService에서만 주입받고 사용해야 합니다!

public void assignToResult(Result result) {
this.result = result;
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

빌더패턴이 아닌 new를 통해 생성자 호출을 해야합니다. (빌더패턴 사용 시 스택오버플로우 발생)
profile에 result 할당을 하는 메서드를 추가하였습니다.

Copy link
Member

Choose a reason for hiding this comment

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

왜 스택오버플로우가 날려나요..궁금하네 🤔
다음에 한 번 테스트 해보면 좋을 것 같습니다!

this.zones.add(zone);
zone.assignToResult(this);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

result 어그리거트 내부의 zone과 profile은 어그리거트 루트인 result를 통해서만 생성, 수정, 삭제가 가능하도록 하기 위해 add 메서드를 만들었습니다.

@Repository
public interface ResultRepository {
Long saveResult(Result result);
Result findResultById(Long id);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

도메인 레포지토리로 ApplicationService에서 이 레포지토리의 메서드를 사용합니다.

}



Copy link
Contributor Author

Choose a reason for hiding this comment

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

도메인의 resultRepository 구현체입니다. DB에서 관련 엔티티를 찾을 후 도메인 객체로 변환하여 반환합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

QueryDsl로 작성한 StadiumCustomRepository 구현체 부분입니다.
해당 조건에 맞는 FoodEntity를 반환합니다.
동적 조건을 위해 QueryDsl로 작성했습니다.

Copy link
Member

Choose a reason for hiding this comment

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

좋습니다 👍🏻

@juuuunny juuuunny changed the base branch from develop to refactor-v1 November 25, 2024 07:52
Copy link
Contributor Author

Choose a reason for hiding this comment

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

stadium 어그리거트에서는 쿼리dsl을 사용하여 구조가 특이한데

main
    - domain
        - repository
            - stadiumRepository 
    - infra
        - jpa 
            - repository
                - custom
                    - stadiumCustomRepository
                stadiumJpaRepository
            - repositoryImpl
                - custom
                    - stadiumCustomRepositoryImpl
                stadiumRepositoryImpl

위와 같은 구조로 되어 있고
stadiumCustomRepository 인터페이스는 stadiumCustomRepositoryImpl에서 QueryDsl로 구현.
domain의 stadiumRepository를 구현한 stadiumRepositoryImpl은 stadiumJpaRepository 인터페이스와 stadiumCustomRepository 인터페이스를 사용하여 구현.

ApplicationService에서는 domain의 stadiumRepository를 사용합니다.

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
Member

@bbbang105 bbbang105 left a comment

Choose a reason for hiding this comment

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

정말 고생 많으셨습니다!! 👍🏻👍🏻

Comment on lines 31 to 32
private final RecommendUserProfileService recommendUserProfileService;
private final RecommendTopRankedZonesService recommendTopRankedZonesService;
Copy link
Member

Choose a reason for hiding this comment

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

요 두개 서비스 말고 RecommendUserProfileManager와 같이 바꾸는 건 어떨까요??
이제 도메인 & 애플리케이션 서비스로 나뉘어서 네이밍이 서비스랑 분리되어도 좋을 것 같아서요!

Copy link
Contributor Author

@juuuunny juuuunny Nov 25, 2024

Choose a reason for hiding this comment

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

두개 다 추천 받은 것 저장하는 거라 그렇게 바꿔도 좋을 것 같아요
그러면 이거는 @component로 붙이는 게 나을까요?

Copy link
Member

Choose a reason for hiding this comment

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

넵넵! 👍🏻

public void assignToResult(Result result) {
this.result = result;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

왜 스택오버플로우가 날려나요..궁금하네 🤔
다음에 한 번 테스트 해보면 좋을 것 같습니다!

public interface ResultRepository extends JpaRepository<Result, Long> {
@Repository
public interface ResultRepository {
Long saveResult(Result result);
Copy link
Member

Choose a reason for hiding this comment

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

요거 void로 하고 저장 후에 Result 도메인에서 getId()로 가져오는 게 낫지 않을까요??
제가 이해하는 걸로는 도메인과 엔터티의 필드 값은 동일해서..
네이밍이 save~인데 ID를 반환하는 건 조금 적절하지 않은 것 같습니다!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아하 그렇네요 그럼 여기서 도메인인 Result를 반환하고 꺼내서 id는 꺼내서 사용하는 식으로 변경하겠습니다!

Comment on lines 12 to 13
@Service
public class RecommendTopRankedZonesService {
Copy link
Member

Choose a reason for hiding this comment

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

이 쪽 위에서 언급한 것처럼 네이밍 변경하면 좋을 것 같구,
현재 코드가 너무 길고 복잡해서 단계별로 분리할 수 있으면 좋을 것 같아요!!

Comment on lines 14 to 15
@Service
public class RecommendUserProfileService {
Copy link
Member

Choose a reason for hiding this comment

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

이것도 위와 동일한 의견입니다!

  1. 네이밍 변경 2) 메서드 분리

Comment on lines 23 to +25
public class StadiumController {

private final StadiumService stadiumService;
private final StadiumApplicationService stadiumApplicationService;
Copy link
Member

@bbbang105 bbbang105 Nov 25, 2024

Choose a reason for hiding this comment

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

혹시 stadium 도메인이라면 StadiumController 외에는 다른 컨트롤러나 서비스가 들어갈 수 없나요??
그렇지 않다면 Food~, Entertain~ 컨트롤러가 있으면 좋을 것 같아서요!

Copy link
Contributor Author

@juuuunny juuuunny Nov 25, 2024

Choose a reason for hiding this comment

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

분리할지 말지 결정 기준

도메인 규칙의 의존성:
Food와 Entertainment 로직이 Stadium의 도메인 규칙에 강하게 의존한다면, Application Service를 분리하기 어렵습니다. 이런 경우 StadiumApplicationService에서 관리하는 것이 자연스럽습니다.

독립적 도메인 규칙:
Food와 Entertainment가 독립적인 도메인 로직과 기능을 가지는 경우, 별도의 Application Service를 만드는 것이 더 적합합니다.
예: FoodApplicationService와 EntertainmentApplicationService를 생성하고, StadiumApplicationService는 FoodApplicationService와 EntertainmentApplicationService를 호출해 조율하는 역할로 축소.

또한, 단순 조회일 경우엔 꼭 StadiumApplicationService에서 타 애플리케이션 서비스를 호출해서 사용하는 것이 아닌FoodApplicationService에서 독립적으로 사용해도 된다고 합니다!!
그리고 결과를 저장하는 것과 같이 CUD는 어그리거트 루트와 관련된 것이니 타 애플리케이션 서비스를 호출해서 사용하는 식으로 하고,
단순 조회일 경우에는 FoodController, FoodApplicationService에서 독립적으로 실행해도 될 것 같습니다.

사실 어그리거트 루트의 컨트롤러, 서비스에서만 관리하면 양이 너무 비대해질 가능성도 있어서 분리하는 것도 좋을 것 같아요!
Stadium 어그리거트 다 이렇게 수정하고 Result는 zone과 profile이 결과에 너무 밀접한 부분이기도 하고 ApplicationService를 나누기에는 양이 너무 적기도 하고 그대로 두었습니다!

Copy link
Member

Choose a reason for hiding this comment

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

넵넵 Result 쪽은 다 api/v1/results~로 통일되니까 하나로 두어도 될 것 같아요!!
반영 좋습니다 👍🏻

Comment on lines 53 to 59
/**
* 해당 구장의 boundary에 해당하는 즐길거리의 리스트를 반환한다.
* @param stadiumName 스타디움명
* @param boundary 매장 위치 (내부 or 외부)
* @return 즐길거리 리스트 (이미지 Url, boundary, 이름, 설명 리스트, 팁 리스트)
*/
@GetMapping("/culture/entertainments")
Copy link
Member

Choose a reason for hiding this comment

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

저는 개인적으로 StadiumController라면,
@RequestMapping("/api/v1/stadium") 으로 고정해두고 그 하위 엔드포인트를 작성합니다!!

여기서 culture 라는 자원까지 접근하는 게 적절할까..?🤔 라는 고민이 듭니다!

Comment on lines 15 to 16
_BAD_REQUEST_BOUNDARY(HttpStatus.BAD_REQUEST, "FOOD-001", "잘못된 영역입니다. 내부 또는 외부로 입력해주세요."),
_BAD_REQUEST_COURSE(HttpStatus.BAD_REQUEST, "FOOD-002", "잘못된 값입니다. 식사 또는 후식으로 입력해주세요.");
Copy link
Member

Choose a reason for hiding this comment

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

여기두 StadiumErrorStatus인데 Food와 관련한 에러 코드가 있는 것이 조금 적절하지 않다고 느껴져요!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사실 Stadium 어그리거트 아래에 있는 엔티티들이라 StadiumErrorStatus에 넣어두었는데 따로 FoodErrorStatus로 파일을 분리하는 게 낫다고 생각하시나요.?!

Copy link
Member

Choose a reason for hiding this comment

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

흐으음 이 부분은 취향 차이일 것 같기는 합니다... 뭔가 저라면 분리했을 것 같기는 해요!!

log.info("유저 정보 조회 성공");
}


Copy link
Member

Choose a reason for hiding this comment

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

공백 있습니당

Comment on lines 29 to 34
@Transactional(readOnly = true)
public void isExistUserById(Long userId){
userRepository.findById(userId)
.orElseThrow(() -> new CustomException(UserErrorStatus._NOT_FOUND_USER));
log.info("유저 정보 조회 성공");
}
Copy link
Member

Choose a reason for hiding this comment

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

is~로 시작하는 메서드는 boolean 타입을 반환해야 합니다!!

그래서 아래 두 방식으로 변경할 수 있을 것 같은데요.

1. boolean 값을 반환

@Transactional(readOnly = true)
public boolean isExistUserById(Long userId) {
    boolean exists = userRepository.existsById(userId);
    if (exists) {
        log.info("유저 정보가 존재합니다: userId = {}", userId);
    } else {
        log.warn("유저 정보가 존재하지 않습니다: userId = {}", userId);
    }
    return exists;
}

다만 이는 서비스 단에서 userRepository.existsById(userId)를 호출하는 것과 다름 없는 메서드이기 때문에 불필요하다고 판단됩니다!

2. validate~로 네이밍 변경

@Transactional(readOnly = true)
public void validateUserExistsById(Long userId) {
    userRepository.findById(userId)
            .orElseThrow(() -> new CustomException(UserErrorStatus._NOT_FOUND_USER));
    log.info("유저 정보 조회 성공: userId = {}", userId);
}

user가 없는 경우에 에러 처리까지 하고 싶다면 2번 방식도 괜찮을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 정보 감사합니다!
아직 user와 auth 부분은 리팩토링을 진행하지 않아 참고하도록 하겠습니다!!

@juuuunny juuuunny changed the title [refactor] : DDD구조 변경 및 Result 어그리게이트에 관한 간접참조와, 객체탐색 리팩토링을 진행한다. [refactor] : Result와 Stadium의 DDD구조 변경 및 Result 어그리게이트에 관한 간접참조와, 객체탐색 리팩토링을 진행한다. Nov 25, 2024
@juuuunny
Copy link
Contributor Author

src/main/resources/static/docs/open-api-3.0.1.json 이게 또 올라가버렸네요 ㅎ..

@juuuunny juuuunny merged commit 6e282c7 into refactor-v1 Nov 25, 2024
@juuuunny juuuunny deleted the feature/#78/result-refactor branch November 26, 2024 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
😆 JUNHYEONG 준형 Issue or PR 🔄 refactor 코드 리팩토링
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants