카테고리 순서 변경 #988
카테고리 순서 변경 #988
Conversation
HoeSeong123
force-pushed
the
feat/966-change-category-ordinal
branch
from
15f20ef to
1242840
Compare
HoeSeong123
force-pushed
the
feat/966-change-category-ordinal
branch
from
316e4ef to
5a2e765
Compare
HoeSeong123
force-pushed
the
feat/966-change-category-ordinal
branch
from
c0558b1 to
d795cbe
Compare
backend/src/main/java/codezap/category/service/CategoryService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/codezap/category/service/CategoryService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/codezap/category/service/CategoryValidationService.java
Outdated
Show resolved
Hide resolved
| public void validateDuplicatedCategory(String categoryName, Member member) { | ||
| if (categoryRepository.existsByNameAndMember(categoryName, member)) { | ||
| throw new CodeZapException(ErrorCode.DUPLICATE_CATEGORY, "카테고리명이 중복되었습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
카테고리명 중복 에러 메세지를 다음과 같이 통합하였습니다.
| @Modifying(flushAutomatically = true, clearAutomatically = true) | ||
| @Query("DELETE FROM Category c WHERE c.id = :id") | ||
| void deleteByIdWithFlush(@Param("id") Long id); | ||
|
|
||
| @Modifying(flushAutomatically = true, clearAutomatically = true) | ||
| @Query("UPDATE Category c SET c.name = :name, c.ordinal = :ordinal WHERE c.id = :id") | ||
| void updateCategoryWithFlush(@Param("id") Long id, @Param("name") String name, @Param("ordinal") int ordinal); | ||
|
|
There was a problem hiding this comment.
@Modifying을 사용하여 삭제, 수정 쿼리에 flush를 하도록 수정하였습니다.
이는 데이터베이스에 변경 사항을 전송할 뿐, 트랜잭션이 커밋되지 않으면 해당 변경 사항은 최종적으로 반영되지 않으므로 같은 트랜잭션 내에서 실패할 경우 자동으로 롤백이 됩니다.
backend/src/main/java/codezap/category/service/CategoryValidationService.java
Outdated
Show resolved
Hide resolved
| public void validateIds() { | ||
| if (hasDuplicates()) { | ||
| throw new IllegalArgumentException("id가 중복되었습니다."); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
생성 시 즉시 검증되지 않고 public 메서드로 외부에서 호출하게 한 이유가 궁금해요!
생성자에서 처리하지 않는다면 외부에서 따로 검증 메서드를 호출해야 하는데, 휴먼 에러로 이 검증을 호출하지 않아버릴 수 있을 것 같아요 🤔 또 도메인 규칙을 지키지 않는 도메인 객체가 생성되어 남게 될 수도 있겠네요!
이것에 대해 어떻게 생각하는지 궁금해요!
There was a problem hiding this comment.
public void validateOrdinals(UpdateAllCategoriesRequest request) {
try {
Ordinals ordinals = new Ordinals(request.extractOrdinal());
} catch (IllegalArgumentException e) {
throw new CodeZapException(ErrorCode.INVALID_REQUEST, e.getMessage());
}
}
public void validateIds(UpdateAllCategoriesRequest request) {
try {
Ids ids = new Ids(request.extractIds());
} catch (IllegalArgumentException e) {
throw new CodeZapException(ErrorCode.DUPLICATE_ID, e.getMessage());
}
}
public void validateNames(UpdateAllCategoriesRequest request) {
try {
Names names = new Names(request.extractNames());
} catch (IllegalArgumentException e) {
throw new CodeZapException(ErrorCode.DUPLICATE_CATEGORY, e.getMessage());
}
}코드가 이런 느낌이 돼서 뭔가뭔가 어색하더라고요..ㅋㅋ
근데 몰리 말대로 도메인 규칙을 지키지 않는 객체가 생성될 수 있는 문제가 생기긴 하겠네요.
다른 분들 의견만 더 들어보고 수정해보겠습니다!
There was a problem hiding this comment.
제가 이해한 몰리의 "생성 시 즉시 검증"은 다음과 같이 코드를 작성하는 것을 의미하는 것 같아요.
public class Ids {
private final List<Long> ids;
public Ids(List<Long> ids) {
this.ids = validateDuplicates(ids);
}
private List<Long> validateDuplicates(List<Long> ids) {
if (ids.size() != new HashSet<>(ids).size()) {
throw new IllegalArgumentException("id가 중복되었습니다.");
}
return ids;
}
}저도 이런 경우의 도메인 검증 로직은 외부에서 호출되는 게 아니라 생성자에서 바로 이루어져야 한다고 생각해요.
There was a problem hiding this comment.
그런데... 코드 리뷰를 하며 곰곰이 생각해보니 저는 ids라는 클래스를 도메인으로 바라보는 게 맞지 않는 것 같아요.
우리가 검증을 하는 이유는 DB에 저장될 데이터의 생성, 수정, 삭제라는 서비스 로직이 실행되기 위한 입력이 정상적인지 확인하기 위해서에요.
우리가 "요청을 받는 방식"을 정책적으로 결정함에 따라 생겨난 요구사항이에요.
그 과정에서 사용되는 ID라는 속성은 RDBMS와 JPA에 의존하기 때문에 생겨났을 뿐이고, 카테고리라는 도메인과 무관해요.
본 클래스는 컬렉션에서의 중복 검증 로직일 뿐이에요.
로직이 도메인의 특성에서 비롯되지 않았어요.
카테고리가 아닌 다른 도메인에서도 컬렉션의 중복 검증 로직은 얼마든지 이루어질 수 있어요.
실제로 SourceCodeService에서도 동일한 검증 로직을 수행하고 있죠.
그런 의미에서 본 ID 중복 검증 클래스가 하는 일은 JPA Entity의 ID의 컬렉션을 이용한 서비스 로직으로 간주해야 한다고 생각해요.
SourceCodeService에서 같은 로직을 사용하고 있으니, global 패키지의 util 로 분리해도 되겠군요.
Ordinals, Names 클래스도 같은 의견이에요.
두 클래스는 서비스 레이어에서 DTO를 사용하는 방식 때문에 만들어졌을 뿐이고, 다른 도메인 객체와 상호작용하지 않아요.
Ordinals, Names가 도메인 영역에서 의미를 갖고 다른 도메인 객체와 상호작용이 있다면 도메인으로 볼 수 있겠지만, 그렇지 않으므로 categoryValidationService로 옮겨져야 한다고 생각해요.
저는 몰리가 일급컬렉션을 이야기했을때, Categories를 만드는 것을 생각했어요.
그렇게되면 Categories 객체가 생성될 때 각 원소의 ordinal과 name 필드의 중복을 검사할 수 있죠.
다만 중복 검증 로직은 여전히 util로 분리할 여지가 있다고 생각하고, Categories 클래스를 만든다면 생성자에서 util 클래스의 정적 메서드를 호출해서 검증하는 게 좋을 것 같아요.
핵심은 검증이라고 해서 다 같은 검증이 아니라는 거에요. 도메인의 검증과, 서비스의 검증을 구분하자는 이야기를 하고 싶었어요. 예를 들어 Default 카테고리가 있다는 정책은 도메인 영역으로 봐도 좋다고 생각해요. 카테고리를 수정할 수 있다는 것도 도메인 영역이에요. 그래서 카테고리를 수정할 때, Default 카테고리를 수정하려 할 경우 도메인 객체에서 예외를 발생시켜도 된다고 생각해요. 그래서 위 Category 클래스에 같은 의도로 의견을 남겼답니다.
너무 길게 써서 죄송합니다... 😭
그러나 제 생각을 가능한 그대로 전달하는 게 서로에게 더 도움이 된다고 생각했어요.
혹시 제가 모르는 게 있거나, 다른 의견이 있다면 얼마든지 부탁해요!!
| public void validateIds() { | ||
| if (hasDuplicates()) { | ||
| throw new IllegalArgumentException("id가 중복되었습니다."); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
제가 이해한 몰리의 "생성 시 즉시 검증"은 다음과 같이 코드를 작성하는 것을 의미하는 것 같아요.
public class Ids {
private final List<Long> ids;
public Ids(List<Long> ids) {
this.ids = validateDuplicates(ids);
}
private List<Long> validateDuplicates(List<Long> ids) {
if (ids.size() != new HashSet<>(ids).size()) {
throw new IllegalArgumentException("id가 중복되었습니다.");
}
return ids;
}
}저도 이런 경우의 도메인 검증 로직은 외부에서 호출되는 게 아니라 생성자에서 바로 이루어져야 한다고 생각해요.
| public void validateIds() { | ||
| if (hasDuplicates()) { | ||
| throw new IllegalArgumentException("id가 중복되었습니다."); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
그런데... 코드 리뷰를 하며 곰곰이 생각해보니 저는 ids라는 클래스를 도메인으로 바라보는 게 맞지 않는 것 같아요.
우리가 검증을 하는 이유는 DB에 저장될 데이터의 생성, 수정, 삭제라는 서비스 로직이 실행되기 위한 입력이 정상적인지 확인하기 위해서에요.
우리가 "요청을 받는 방식"을 정책적으로 결정함에 따라 생겨난 요구사항이에요.
그 과정에서 사용되는 ID라는 속성은 RDBMS와 JPA에 의존하기 때문에 생겨났을 뿐이고, 카테고리라는 도메인과 무관해요.
본 클래스는 컬렉션에서의 중복 검증 로직일 뿐이에요.
로직이 도메인의 특성에서 비롯되지 않았어요.
카테고리가 아닌 다른 도메인에서도 컬렉션의 중복 검증 로직은 얼마든지 이루어질 수 있어요.
실제로 SourceCodeService에서도 동일한 검증 로직을 수행하고 있죠.
그런 의미에서 본 ID 중복 검증 클래스가 하는 일은 JPA Entity의 ID의 컬렉션을 이용한 서비스 로직으로 간주해야 한다고 생각해요.
SourceCodeService에서 같은 로직을 사용하고 있으니, global 패키지의 util 로 분리해도 되겠군요.
Ordinals, Names 클래스도 같은 의견이에요.
두 클래스는 서비스 레이어에서 DTO를 사용하는 방식 때문에 만들어졌을 뿐이고, 다른 도메인 객체와 상호작용하지 않아요.
Ordinals, Names가 도메인 영역에서 의미를 갖고 다른 도메인 객체와 상호작용이 있다면 도메인으로 볼 수 있겠지만, 그렇지 않으므로 categoryValidationService로 옮겨져야 한다고 생각해요.
저는 몰리가 일급컬렉션을 이야기했을때, Categories를 만드는 것을 생각했어요.
그렇게되면 Categories 객체가 생성될 때 각 원소의 ordinal과 name 필드의 중복을 검사할 수 있죠.
다만 중복 검증 로직은 여전히 util로 분리할 여지가 있다고 생각하고, Categories 클래스를 만든다면 생성자에서 util 클래스의 정적 메서드를 호출해서 검증하는 게 좋을 것 같아요.
핵심은 검증이라고 해서 다 같은 검증이 아니라는 거에요. 도메인의 검증과, 서비스의 검증을 구분하자는 이야기를 하고 싶었어요. 예를 들어 Default 카테고리가 있다는 정책은 도메인 영역으로 봐도 좋다고 생각해요. 카테고리를 수정할 수 있다는 것도 도메인 영역이에요. 그래서 카테고리를 수정할 때, Default 카테고리를 수정하려 할 경우 도메인 객체에서 예외를 발생시켜도 된다고 생각해요. 그래서 위 Category 클래스에 같은 의도로 의견을 남겼답니다.
너무 길게 써서 죄송합니다... 😭
그러나 제 생각을 가능한 그대로 전달하는 게 서로에게 더 도움이 된다고 생각했어요.
혹시 제가 모르는 게 있거나, 다른 의견이 있다면 얼마든지 부탁해요!!
⚡️ 관련 이슈
close #966
📍주요 변경 사항
검증 로직
카테고리 생성시 (템플릿 생성 화면에서 생성)
카테고리 편집시 (모달창에서 생성, 수정, 삭제를 한 번에 하는 것)
혹시 더 추가로 검증할만한 내용이 있는지 확인해주시면 감사드리겠습니다.
🎸기타
🍗 PR 첫 리뷰 마감 기한
12/26 23:59