-
Notifications
You must be signed in to change notification settings - Fork 8
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
카테고리 순서 변경 #988
base: dev/be
Are you sure you want to change the base?
카테고리 순서 변경 #988
Changes from 26 commits
2e7fd59
fc4681a
2d61a2c
8e1037d
2b966cd
c4f283f
d90802b
339286a
52810d3
f8dfd92
ddcb834
64096db
bc199d8
0102194
fa04a1d
d61ac47
25e0e47
11ab595
9eea751
5a2e765
a760937
d795cbe
f94b833
ff27d42
ff3acb9
92794bd
29be344
1f93dc0
85e87cb
f96d279
39d0e0c
26a0fab
c4a6ffc
ec7a539
74fc71c
9757b17
771d391
f79c1c3
601ff62
bb06d03
fa429ec
6d27648
347eb85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,7 +4,7 @@ | |||||
import org.springframework.http.ResponseEntity; | ||||||
|
||||||
import codezap.category.dto.request.CreateCategoryRequest; | ||||||
import codezap.category.dto.request.UpdateCategoryRequest; | ||||||
import codezap.category.dto.request.UpdateAllCategoriesRequest; | ||||||
import codezap.category.dto.response.CreateCategoryResponse; | ||||||
import codezap.category.dto.response.FindAllCategoriesResponse; | ||||||
import codezap.global.swagger.error.ApiErrorResponse; | ||||||
|
@@ -49,34 +49,19 @@ ResponseEntity<CreateCategoryResponse> createCategory( | |||||
@SecurityRequirement(name = "쿠키 인증 토큰") | ||||||
@Operation(summary = "카테고리 수정", description = "해당하는 식별자의 카테고리를 수정합니다.") | ||||||
@ApiResponse(responseCode = "200", description = "카테고리 수정 성공") | ||||||
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST, instance = "/categories/1", errorCases = { | ||||||
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST, instance = "/categories", errorCases = { | ||||||
@ErrorCase(description = "기본 카테고리를 수정한 경우", exampleMessage = "기본 카테고리는 수정 및 삭제할 수 없습니다."), | ||||||
@ErrorCase(description = "카테고리 이름이 15자를 초과한 경우", exampleMessage = "카테고리 이름은 최대 15자까지 입력 가능합니다."), | ||||||
@ErrorCase(description = "카테고리의 순서가 잘못된 경우", exampleMessage = "템플릿 순서가 중복됩니다.") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ErrorCase(description = "모든 필드 중 null인 값이 있는 경우", exampleMessage = "카테고리 이름이 null 입니다."),
@ErrorCase(description = "카테고리 이름이 15자를 초과한 경우", exampleMessage = "카테고리 이름은 최대 15자까지 입력 가능합니다."),
@ErrorCase(description = "삭제하려는 카테고리에 템플릿이 존재하는 경우", exampleMessage = "템플릿이 존재하는 카테고리는 삭제할 수 없습니다."), 해당 에러 케이스도 추가해주세요~! 그리고 마지막 에러 케이스에도 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 애플리케이션 예외 메시지와 동일하게
Suggested change
로 변경해주세요~! |
||||||
}) | ||||||
@ApiErrorResponse(status = HttpStatus.NOT_FOUND, instance = "/categories/1", errorCases = { | ||||||
@ApiErrorResponse(status = HttpStatus.NOT_FOUND, instance = "/categories", errorCases = { | ||||||
@ErrorCase(description = "해당하는 id 값인 카테고리가 없는 경우", exampleMessage = "식별자 1에 해당하는 카테고리가 존재하지 않습니다."), | ||||||
}) | ||||||
@ApiErrorResponse(status = HttpStatus.CONFLICT, instance = "/categories", errorCases = { | ||||||
@ErrorCase(description = "동일한 이름의 카테고리가 존재하는 경우", exampleMessage = "이름이 Spring 인 카테고리가 이미 존재합니다."), | ||||||
}) | ||||||
@ApiErrorResponse(status = HttpStatus.FORBIDDEN, instance = "/categories/1", errorCases = { | ||||||
@ApiErrorResponse(status = HttpStatus.FORBIDDEN, instance = "/categories", errorCases = { | ||||||
@ErrorCase(description = "카테고리를 수정할 권한이 없는 경우", exampleMessage = "해당 카테고리를 수정 또는 삭제할 권한이 없는 유저입니다.") | ||||||
}) | ||||||
ResponseEntity<Void> updateCategory(Member member, Long id, UpdateCategoryRequest updateCategoryRequest); | ||||||
|
||||||
@SecurityRequirement(name = "쿠키 인증 토큰") | ||||||
@Operation(summary = "카테고리 삭제", description = "해당하는 식별자의 카테고리를 삭제합니다.") | ||||||
@ApiResponse(responseCode = "204", description = "카테고리 삭제 성공") | ||||||
@ApiErrorResponse(status = HttpStatus.BAD_REQUEST, instance = "/categories/1", errorCases = { | ||||||
@ErrorCase(description = "삭제하려는 카테고리에 템플릿이 존재하는 경우", | ||||||
exampleMessage = "템플릿이 존재하는 카테고리는 삭제할 수 없습니다."), | ||||||
}) | ||||||
@ApiErrorResponse(status = HttpStatus.NOT_FOUND, instance = "/categories/1", errorCases = { | ||||||
@ErrorCase(description = "존재하지 않는 카테고리인 경우", | ||||||
exampleMessage = "식별자 1에 해당하는 카테고리가 존재하지 않습니다."), | ||||||
}) | ||||||
@ApiErrorResponse(status = HttpStatus.FORBIDDEN, instance = "/categories/1", errorCases = { | ||||||
@ErrorCase(description = "카테고리를 수정할 권한이 없는 경우", | ||||||
exampleMessage = "해당 카테고리를 수정 또는 삭제할 권한이 없는 유저입니다.") | ||||||
}) | ||||||
ResponseEntity<Void> deleteCategory(Member member, Long id); | ||||||
ResponseEntity<Void> updateCategory(Member member, UpdateAllCategoriesRequest request); | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -36,6 +36,7 @@ | |||||||||||||||||||
public class Category extends BaseTimeEntity { | ||||||||||||||||||||
|
||||||||||||||||||||
private static final String DEFAULT_CATEGORY_NAME = "카테고리 없음"; | ||||||||||||||||||||
private static final long DEFAULT_CATEGORY_ORDINAL = 1L; | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 넵 전달되었습니다!! |
||||||||||||||||||||
|
||||||||||||||||||||
@Id | ||||||||||||||||||||
@GeneratedValue(strategy = GenerationType.IDENTITY) | ||||||||||||||||||||
|
@@ -50,18 +51,23 @@ public class Category extends BaseTimeEntity { | |||||||||||||||||||
@Column(nullable = false) | ||||||||||||||||||||
private Boolean isDefault; | ||||||||||||||||||||
|
||||||||||||||||||||
public Category(String name, Member member) { | ||||||||||||||||||||
@Column(nullable = false) | ||||||||||||||||||||
private long ordinal; | ||||||||||||||||||||
|
||||||||||||||||||||
public Category(String name, Member member, long ordinal) { | ||||||||||||||||||||
this.name = name; | ||||||||||||||||||||
this.member = member; | ||||||||||||||||||||
this.isDefault = false; | ||||||||||||||||||||
this.ordinal = ordinal; | ||||||||||||||||||||
} | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||
|
||||||||||||||||||||
public static Category createDefaultCategory(Member member) { | ||||||||||||||||||||
return new Category(null, member, DEFAULT_CATEGORY_NAME, true); | ||||||||||||||||||||
return new Category(null, member, DEFAULT_CATEGORY_NAME, true, DEFAULT_CATEGORY_ORDINAL); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
public void updateName(String name) { | ||||||||||||||||||||
public void update(String name, long ordinal) { | ||||||||||||||||||||
this.name = name; | ||||||||||||||||||||
this.ordinal = ordinal; | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
public void validateAuthorization(Member member) { | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
package codezap.category.dto.request; | ||
|
||
import jakarta.validation.constraints.NotBlank; | ||
import jakarta.validation.constraints.NotNull; | ||
import jakarta.validation.constraints.Size; | ||
|
||
import codezap.global.validation.ValidationGroups.NotNullGroup; | ||
|
@@ -11,6 +12,10 @@ public record CreateCategoryRequest( | |
@Schema(description = "카테고리 이름", example = "Spring") | ||
@NotBlank(message = "카테고리 이름이 null 입니다.", groups = NotNullGroup.class) | ||
@Size(max = 15, message = "카테고리 이름은 최대 15자까지 입력 가능합니다.", groups = SizeCheckGroup.class) | ||
String name | ||
String name, | ||
|
||
@Schema(description = "카테고리 순서", example = "1") | ||
@NotNull(message = "카테고리 순서가 null 입니다.", groups = NotNullGroup.class) | ||
Long ordinal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. null 뿐만 아니라 최소 값 check도 하면 좋을 것 같아요 @Min(value = 1, message = "1 이상이어야 합니다.") |
||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package codezap.category.dto.request; | ||
|
||
import java.util.List; | ||
|
||
import jakarta.validation.Valid; | ||
import jakarta.validation.constraints.NotNull; | ||
|
||
import codezap.global.validation.ValidationGroups.NotNullGroup; | ||
import io.swagger.v3.oas.annotations.media.Schema; | ||
|
||
public record UpdateAllCategoriesRequest( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (⚡어필의 차원에서 zap을 사용합니다 ㅎㅎ ) 해당 dto에서 fail fast를 위해 소스 코드 request처럼 SourceCodesOrdinalValidator를 이용해서 dto에서도 순서 검증하는 건 어떨까요? 현재 해당 로직이 템플릿 수정 로직과 매우 유사하다고 생각합니다. 그래서 현재 SourceCodesOrdinalValidator의 이름을 OrdinalValidator로 변경하여 소스코드에서만 사용하지 않고 이렇게 순서 검증이 필요한 곳에서 재사용하면 좋을 것 같아요 ~! 이렇게 한다면 ValidatedSourceCodesOrdinalRequest의 이에 대해 어떻게 생각하시나요?
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 너무 좋습니다!! |
||
@Schema(description = "생성할 카테고리 목록") | ||
@Valid | ||
List<CreateCategoryRequest> createCategories, | ||
|
||
@Schema(description = "수정할 카테고리 목록") | ||
@Valid | ||
List<UpdateCategoryRequest> updateCategories, | ||
|
||
@Schema(description = "삭제할 카테고리 목록") | ||
@NotNull(message = "삭제하는 카테고리 ID 목록이 null 입니다.", groups = NotNullGroup.class) | ||
List<Long> deleteCategoryIds | ||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,25 @@ | ||
package codezap.category.dto.request; | ||
|
||
import jakarta.validation.constraints.NotBlank; | ||
import jakarta.validation.constraints.NotNull; | ||
import jakarta.validation.constraints.Size; | ||
|
||
import codezap.global.validation.ValidationGroups.NotNullGroup; | ||
import codezap.global.validation.ValidationGroups.SizeCheckGroup; | ||
import io.swagger.v3.oas.annotations.media.Schema; | ||
|
||
public record UpdateCategoryRequest( | ||
@Schema(description = "카테고리 ID", example = "1") | ||
@NotNull(message = "카테고리 ID가 null 입니다.", groups = NotNullGroup.class) | ||
Long id, | ||
|
||
@Schema(description = "카테고리 이름", example = "Spring") | ||
@NotBlank(message = "카테고리 이름이 null 입니다.", groups = NotNullGroup.class) | ||
@Size(max = 15, message = "카테고리 이름은 최대 15자까지 입력 가능합니다.", groups = SizeCheckGroup.class) | ||
String name | ||
String name, | ||
|
||
@Schema(description = "카테고리 순서", example = "1") | ||
@NotNull(message = "카테고리 순서가 null 입니다.", groups = NotNullGroup.class) | ||
Long ordinal | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기도 null 뿐만 아니라 최소 값 check도 하면 좋을 것 같아요 @Min(value = 1, message = "1 이상이어야 합니다.") |
||
) { | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 코드가 너무 길어서 검증 로직을 CategoryValidationService로 분리하면 어떨까 제안해요. @Service
@RequiredArgsConstructor
public class CategoryService {
private final CategoryRepository categoryRepository;
private final CategoryValidationService validationService;
@Transactional
public CreateCategoryResponse create(Member member, CreateCategoryRequest request) {
validationService.validateDuplicatedCategory(request.name(), member);
validationService.validateOrdinal(member, request);
Category category = categoryRepository.save(createCategory(member, request));
return CreateCategoryResponse.from(category);
}
@Transactional(readOnly = true)
public FindAllCategoriesResponse findAllByMemberId(Long memberId) {
return FindAllCategoriesResponse.from(categoryRepository.findAllByMemberIdOrderById(memberId));
}
@Transactional
public void updateCategories(Member member, UpdateAllCategoriesRequest request) {
validationService.validateDuplicateNameRequest(request);
validationService.validateOrdinals(request);
validationService.validateIds(request);
createCategories(member, request);
updateCategories(request.updateCategories(), member);
deleteCategories(request.deleteCategoryIds(), member);
validationService.validateCategoriesCount(member, request);
}
private void createCategories(Member member, UpdateAllCategoriesRequest request) {
categoryRepository.saveAll(request.createCategories().stream()
.map(createRequest -> new Category(createRequest.name(), member, createRequest.ordinal()))
.toList());
}
private void updateCategories(List<UpdateCategoryRequest> updates, Member member) {
updates.forEach(update -> {
Category category = categoryRepository.fetchById(update.id());
category.validateAuthorization(member);
validationService.validateDefaultCategory(category);
category.update(update.name(), update.ordinal());
});
}
private void deleteCategories(List<Long> ids, Member member) {
ids.forEach(id -> {
Category category = categoryRepository.fetchById(id);
category.validateAuthorization(member);
validationService.validateDefaultCategory(category);
validationService.validateHasTemplate(id);
categoryRepository.deleteById(id);
});
}
} package codezap.category.service;
import java.util.HashSet;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.springframework.stereotype.Service;
import codezap.category.domain.Category;
import codezap.category.dto.request.CreateCategoryRequest;
import codezap.category.dto.request.UpdateAllCategoriesRequest;
import codezap.category.dto.request.UpdateCategoryRequest;
import codezap.category.repository.CategoryRepository;
import codezap.global.exception.CodeZapException;
import codezap.global.exception.ErrorCode;
import codezap.member.domain.Member;
import codezap.template.repository.TemplateRepository;
import lombok.RequiredArgsConstructor;
@Service
@RequiredArgsConstructor
public class CategoryValidationService {
private final CategoryRepository categoryRepository;
private final TemplateRepository templateRepository;
public void validateDuplicatedCategory(String categoryName, Member member) {
if (categoryRepository.existsByNameAndMember(categoryName, member)) {
throw new CodeZapException(ErrorCode.DUPLICATE_CATEGORY, "이름이 " + categoryName + "인 카테고리가 이미 존재합니다.");
}
}
public void validateDefaultCategory(Category category) {
if (category.isDefault()) {
throw new CodeZapException(ErrorCode.DEFAULT_CATEGORY, "기본 카테고리는 수정 및 삭제할 수 없습니다.");
}
}
public void validateHasTemplate(Long id) {
if (templateRepository.existsByCategoryId(id)) {
throw new CodeZapException(ErrorCode.CATEGORY_HAS_TEMPLATES, "템플릿이 존재하는 카테고리는 삭제할 수 없습니다.");
}
}
public void validateDuplicateNameRequest(UpdateAllCategoriesRequest request) {
List<String> allNames = Stream.concat(
request.createCategories().stream().map(CreateCategoryRequest::name),
request.updateCategories().stream().map(UpdateCategoryRequest::name)
).toList();
if (hasDuplicates(allNames)) {
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "요청에 중복된 카테고리 이름이 존재합니다.");
}
}
public void validateOrdinal(Member member, CreateCategoryRequest request) {
long ordinal = request.ordinal();
long count = categoryRepository.countByMember(member);
if (ordinal != count) {
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "카테고리 순서가 잘못되었습니다.");
}
}
public void validateOrdinals(UpdateAllCategoriesRequest request) {
List<Long> allOrdinals = Stream.concat(
request.createCategories().stream().map(CreateCategoryRequest::ordinal),
request.updateCategories().stream().map(UpdateCategoryRequest::ordinal)
).sorted().toList();
if (!isSequential(allOrdinals)) {
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "카테고리 순서가 연속적이지 않습니다.");
}
}
public void validateIds(UpdateAllCategoriesRequest request) {
List<Long> allIds = Stream.concat(
request.updateCategories().stream().map(UpdateCategoryRequest::id),
request.deleteCategoryIds().stream()
).toList();
if (hasDuplicates(allIds)) {
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "요청에 중복된 id가 존재합니다.");
}
}
public void validateCategoriesCount(Member member, UpdateAllCategoriesRequest request) {
if (request.updateCategories().size() + request.createCategories().size()
!= categoryRepository.countByMember(member) - 1) {
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "카테고리의 개수가 일치하지 않습니다.");
}
}
private boolean hasDuplicates(List<?> items) {
return items.size() != new HashSet<>(items).size();
}
private boolean isSequential(List<Long> ordinals) {
return ordinals.stream()
.distinct()
.sorted()
.collect(Collectors.toList())
.equals(ordinals);
}
} 다른 분들 의견도 궁금하네용 동의한다면 따봉을, 다른 의견이 있다면 코멘트 부탁해요~! There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 음 validateDuplicateNameRequest, validateOrdinal, validateOrdinals, validateIds, validateCategoriesCount 등 레포지토리를 호출하지 않는 검증들은 일급 컬렉션을 활용해서 도메인의 책임으로 주는 건 어떨까 싶은데............. 어떻게 분리하면 좋을지 고민되네요.......... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 저도 몰리 의견대로 도메인의 책임으로 주는 것이 좋다고 생각했습니다. 더 좋은 의견 있으면 공유해주세요!! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,16 @@ | ||
package codezap.category.service; | ||
|
||
import java.util.HashSet; | ||
import java.util.List; | ||
import java.util.stream.IntStream; | ||
import java.util.stream.Stream; | ||
|
||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
import codezap.category.domain.Category; | ||
import codezap.category.dto.request.CreateCategoryRequest; | ||
import codezap.category.dto.request.UpdateAllCategoriesRequest; | ||
import codezap.category.dto.request.UpdateCategoryRequest; | ||
import codezap.category.dto.response.CreateCategoryResponse; | ||
import codezap.category.dto.response.FindAllCategoriesResponse; | ||
|
@@ -24,13 +30,17 @@ public class CategoryService { | |
private final TemplateRepository templateRepository; | ||
|
||
@Transactional | ||
public CreateCategoryResponse create(Member member, CreateCategoryRequest createCategoryRequest) { | ||
String categoryName = createCategoryRequest.name(); | ||
validateDuplicatedCategory(categoryName, member); | ||
Category category = categoryRepository.save(new Category(categoryName, member)); | ||
public CreateCategoryResponse create(Member member, CreateCategoryRequest request) { | ||
validateDuplicatedCategory(request.name(), member); | ||
validateOrdinal(member, request); | ||
Category category = categoryRepository.save(createCategory(member, request)); | ||
return CreateCategoryResponse.from(category); | ||
} | ||
|
||
private Category createCategory(Member member, CreateCategoryRequest request) { | ||
return new Category(request.name(), member, request.ordinal()); | ||
} | ||
|
||
public FindAllCategoriesResponse findAllByMemberId(Long memberId) { | ||
return FindAllCategoriesResponse.from(categoryRepository.findAllByMemberIdOrderById(memberId)); | ||
} | ||
|
@@ -44,11 +54,43 @@ public Category fetchById(Long id) { | |
} | ||
|
||
@Transactional | ||
public void update(Member member, Long id, UpdateCategoryRequest updateCategoryRequest) { | ||
validateDuplicatedCategory(updateCategoryRequest.name(), member); | ||
Category category = categoryRepository.fetchById(id); | ||
public void updateCategories(Member member, UpdateAllCategoriesRequest request) { | ||
validateDuplicateNameRequest(request); | ||
validateOrdinals(request); | ||
validateIds(request); | ||
|
||
createCategories(member, request); | ||
for (UpdateCategoryRequest updateCategory : request.updateCategories()) { | ||
update(member, updateCategory); | ||
} | ||
for (Long deleteCategoryId : request.deleteCategoryIds()) { | ||
delete(member, deleteCategoryId); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. �List의 forEach 쓰면 어떨까요~? 한줄로 가독성이 좋아질 것 같아요! |
||
validateCategoriesCount(member, request); | ||
} | ||
|
||
private void createCategories(Member member, UpdateAllCategoriesRequest request) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 전달 인자를 UpdateAllCategoriesRequest가 아닌 List 로 넘기면 더 좋을 것 같아요. 사용하는 데이터는 CreateCategoryRequest 뿐이니까요 |
||
categoryRepository.saveAll( | ||
request.createCategories().stream() | ||
.map(createRequest -> createCategory(member, createRequest)) | ||
.toList() | ||
); | ||
} | ||
|
||
private void update(Member member, UpdateCategoryRequest request) { | ||
Category category = categoryRepository.fetchById(request.id()); | ||
category.validateAuthorization(member); | ||
category.updateName(updateCategoryRequest.name()); | ||
validateDefaultCategory(category); | ||
category.update(request.name(), request.ordinal()); | ||
} | ||
|
||
private void delete(Member member, Long categoryId) { | ||
Category category = categoryRepository.fetchById(categoryId); | ||
category.validateAuthorization(member); | ||
|
||
validateDefaultCategory(category); | ||
validateHasTemplate(categoryId); | ||
categoryRepository.deleteById(categoryId); | ||
} | ||
|
||
private void validateDuplicatedCategory(String categoryName, Member member) { | ||
|
@@ -57,17 +99,66 @@ private void validateDuplicatedCategory(String categoryName, Member member) { | |
} | ||
} | ||
|
||
@Transactional | ||
public void deleteById(Member member, Long id) { | ||
Category category = categoryRepository.fetchById(id); | ||
category.validateAuthorization(member); | ||
private void validateDefaultCategory(Category category) { | ||
if (category.isDefault()) { | ||
throw new CodeZapException(ErrorCode.DEFAULT_CATEGORY, "기본 카테고리는 수정 및 삭제할 수 없습니다."); | ||
} | ||
} | ||
|
||
private void validateHasTemplate(Long id) { | ||
if (templateRepository.existsByCategoryId(id)) { | ||
throw new CodeZapException(ErrorCode.CATEGORY_HAS_TEMPLATES, "템플릿이 존재하는 카테고리는 삭제할 수 없습니다."); | ||
} | ||
if (category.isDefault()) { | ||
throw new CodeZapException(ErrorCode.DEFAULT_CATEGORY, "기본 카테고리는 삭제할 수 없습니다."); | ||
} | ||
|
||
private void validateDuplicateNameRequest(UpdateAllCategoriesRequest request) { | ||
List<String> allNames = Stream.concat( | ||
request.createCategories().stream().map(CreateCategoryRequest::name), | ||
request.updateCategories().stream().map(UpdateCategoryRequest::name) | ||
).toList(); | ||
|
||
if (allNames.size() != new HashSet<>(allNames).size()) { | ||
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "요청에 중복된 카테고리 이름이 존재합니다."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. swagger에는 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이게 좀 고민이 많은데 둘은 다른 에러입니다. 첫 번째 에러는 카테고리 생성 시에 발생하는 에러인데, 이 경우는 1개의 카테고리에 대해서만 생성 요청을 보내므로 DB에서 이름 중복 검증을 진행합니다. 하지만 두 번째 에러는 카테고리 편집(생성, 수정, 삭제) 시에 발생하는 에러인데, 이 경우는 DB에서 검증하면 안되고 요청 안에서 중복된 이름이 있는지 확인해야 합니다. 왜냐면 통째로 덮어 씌우는 로직이기 때문입니다. 똑같이 가져가는게 나을까요? 다른 분들의 의견도 궁금해요~ 추가로 문서에 이 경우에 대한 에러도 추가해주었습니다. |
||
} | ||
} | ||
|
||
private void validateOrdinal(Member member, CreateCategoryRequest request) { | ||
long ordinal = request.ordinal(); | ||
long count = categoryRepository.countByMember(member); | ||
if (ordinal != count) { | ||
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "카테고리 순서가 잘못되었습니다."); | ||
} | ||
} | ||
Comment on lines
+96
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 치명적이네요. |
||
|
||
private void validateOrdinals(UpdateAllCategoriesRequest request) { | ||
List<Long> allOrdinals = Stream.concat( | ||
request.createCategories().stream().map(CreateCategoryRequest::ordinal), | ||
request.updateCategories().stream().map(UpdateCategoryRequest::ordinal) | ||
).sorted().toList(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 위에 말한대로 ValidatedSourceCodesOrdinalRequest을 사용한다면 해당 코드를 request에 extractSourceCodesOrdinal() 메서드로 두고 호출해서 사용할 수 있을 것 같아요~ 그래서 dto에서 1차 검증으로 fast fail의 장점을 잡고, 내부에서는 안정성을 위해 한 번 더 체크하는 장점을 잡을 때 하나의 메서드를 사용하면 추후 리펙터링 시에도 좋을 것 같아요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
이 부분이 잘 이해가 되지 않는데, 다시 설명해줄 수 있나요? 제가 이해한 바로는 위 코드를 dto에 두고 service에서도 해당 코드를 불러서 재사용하자는 것 같은데 맞나요?? |
||
|
||
boolean isSequential = IntStream.range(0, allOrdinals.size()) | ||
.allMatch(i -> allOrdinals.get(i) == i + 1); | ||
|
||
if (!isSequential) { | ||
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "카테고리 순서가 연속적이지 않습니다."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 켬미가 얘기한 |
||
} | ||
} | ||
|
||
private void validateIds(UpdateAllCategoriesRequest request) { | ||
List<Long> allIds = Stream.concat( | ||
request.updateCategories().stream().map(UpdateCategoryRequest::id), | ||
request.deleteCategoryIds().stream() | ||
).toList(); | ||
|
||
if (allIds.size() != new HashSet<>(allIds).size()) { | ||
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "요청에 중복된 id가 존재합니다."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 예외도 swagger에 추가해주세요~ |
||
} | ||
} | ||
|
||
private void validateCategoriesCount(Member member, UpdateAllCategoriesRequest request) { | ||
if (request.updateCategories().size() + request.createCategories().size() | ||
!= categoryRepository.countByMember(member) - 1) { | ||
throw new CodeZapException(ErrorCode.INVALID_REQUEST, "카테고리의 개수가 일치하지 않습니다."); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 예외도 swagger에 추가해주세요~ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +) 카테고리 전체 (수정되지 않는 카테고리)도 보내주어야 한다는 게 전달되면 좋을 것 같네요! |
||
} | ||
categoryRepository.deleteById(id); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1 @@ | ||||||||
ALTER TABLE category ADD COLUMN ordinal BIGINT NOT NULL; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ (member_id, ordinal) 합성 필드에 unique 조건 걸어야 하지 않을까요?!
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가를 했었는데 오류가 발생해서 삭제했습니다. 추가로 기존에 걸려있던 (member_id, name)에 대한 unique 조건도 삭제하였습니다. 발생한 오류는 다음과 같습니다. 현재 저희는 생성, 수정, 삭제를 한 트랜잭션 안에서 동시에 수행합니다.
이 문제를 어떻게 해겷해야 할 지 잘 모르겠네요. flush를 해서라도 제약 조거는 거는게 좋을까요? |
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.
설명을 조금 수정하면 좋을 것 같아요 !
설명만 보면 업데이트 기능만 존재하는 것 같으나, 실제로는 생성, 삭제도 진행되고 있으니까요 !
카테고리를 생성, 수정, 삭제할 수 있습니다.
혹은사용자의 카테고리들을 생성, 수정, 삭제할 수 있습니다.
어떤가요?