Skip to content
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
package com.example.solidconnection.application.dto;

import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;

public record ApplyRequest(

@NotNull(message = "gpa score id를 입력해주세요.")
Long gpaScoreId,

@NotNull(message = "language test score id를 입력해주세요.")
Long languageTestScoreId,

@Valid
UniversityChoiceRequest universityChoiceRequest
) {
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package com.example.solidconnection.application.dto;

import jakarta.validation.constraints.NotNull;
import com.example.solidconnection.custom.validation.annotation.ValidUniversityChoice;

@ValidUniversityChoice
public record UniversityChoiceRequest(
@NotNull(message = "1지망 대학교를 입력해주세요.")
Long firstChoiceUniversityId,

Long secondChoiceUniversityId,
Long thirdChoiceUniversityId) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;

Expand Down Expand Up @@ -49,7 +51,6 @@ public class ApplicationSubmissionService {
@Transactional
public boolean apply(SiteUser siteUser, ApplyRequest applyRequest) {
UniversityChoiceRequest universityChoiceRequest = applyRequest.universityChoiceRequest();
validateUniversityChoices(universityChoiceRequest);

Long gpaScoreId = applyRequest.gpaScoreId();
Long languageTestScoreId = applyRequest.languageTestScoreId();
Expand Down Expand Up @@ -117,23 +118,4 @@ private void validateUpdateLimitNotExceed(Application application) {
throw new CustomException(APPLY_UPDATE_LIMIT_EXCEED);
}
}

// 입력값 유효성 검증
private void validateUniversityChoices(UniversityChoiceRequest universityChoiceRequest) {
Set<Long> uniqueUniversityIds = new HashSet<>();
uniqueUniversityIds.add(universityChoiceRequest.firstChoiceUniversityId());
if (universityChoiceRequest.secondChoiceUniversityId() != null) {
addUniversityChoice(uniqueUniversityIds, universityChoiceRequest.secondChoiceUniversityId());
}
if (universityChoiceRequest.thirdChoiceUniversityId() != null) {
addUniversityChoice(uniqueUniversityIds, universityChoiceRequest.thirdChoiceUniversityId());
}
}

private void addUniversityChoice(Set<Long> uniqueUniversityIds, Long universityId) {
boolean notAdded = !uniqueUniversityIds.add(universityId);
if (notAdded) {
throw new CustomException(CANT_APPLY_FOR_SAME_UNIVERSITY);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ public enum ErrorCode {
CANT_APPLY_FOR_SAME_UNIVERSITY(HttpStatus.BAD_REQUEST.value(), "1, 2, 3지망에 동일한 대학교를 입력할 수 없습니다."),
CAN_NOT_CHANGE_NICKNAME_YET(HttpStatus.BAD_REQUEST.value(), "마지막 닉네임 변경으로부터 " + MIN_DAYS_BETWEEN_NICKNAME_CHANGES + "일이 지나지 않았습니다."),
PROFILE_IMAGE_NEEDED(HttpStatus.BAD_REQUEST.value(), "프로필 이미지가 필요합니다."),
FIRST_CHOICE_REQUIRED(HttpStatus.BAD_REQUEST.value(), "1지망 대학교를 입력해주세요."),
THIRD_CHOICE_REQUIRES_SECOND(HttpStatus.BAD_REQUEST.value(), "2지망 없이 3지망을 선택할 수 없습니다."),
DUPLICATE_UNIVERSITY_CHOICE(HttpStatus.BAD_REQUEST.value(), "지망 선택이 중복되었습니다."),

// community
INVALID_POST_CATEGORY(HttpStatus.BAD_REQUEST.value(), "잘못된 카테고리명입니다."),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.example.solidconnection.custom.validation.annotation;

import com.example.solidconnection.custom.validation.validator.ValidUniversityChoiceValidator;
import jakarta.validation.Constraint;
import jakarta.validation.Payload;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Constraint(validatedBy = ValidUniversityChoiceValidator.class)
public @interface ValidUniversityChoice {

String message() default "유효하지 않은 지망 대학 선택입니다.";
Class<?>[] groups() default {};
Class<? extends Payload>[] payload() default {};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package com.example.solidconnection.custom.validation.validator;

import com.example.solidconnection.application.dto.UniversityChoiceRequest;
import com.example.solidconnection.custom.validation.annotation.ValidUniversityChoice;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;

import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;

import static com.example.solidconnection.custom.exception.ErrorCode.DUPLICATE_UNIVERSITY_CHOICE;
import static com.example.solidconnection.custom.exception.ErrorCode.FIRST_CHOICE_REQUIRED;
import static com.example.solidconnection.custom.exception.ErrorCode.THIRD_CHOICE_REQUIRES_SECOND;

public class ValidUniversityChoiceValidator implements ConstraintValidator<ValidUniversityChoice, UniversityChoiceRequest> {

@Override
public boolean isValid(UniversityChoiceRequest request, ConstraintValidatorContext context) {
context.disableDefaultConstraintViolation();

if (isFirstChoiceNotSelected(request)) {
context.buildConstraintViolationWithTemplate(FIRST_CHOICE_REQUIRED.getMessage())
.addConstraintViolation();
return false;
}

if (isThirdChoiceWithoutSecond(request)) {
context.buildConstraintViolationWithTemplate(THIRD_CHOICE_REQUIRES_SECOND.getMessage())
.addConstraintViolation();
return false;
}

if (isDuplicate(request)) {
context.buildConstraintViolationWithTemplate(DUPLICATE_UNIVERSITY_CHOICE.getMessage())
.addConstraintViolation();
return false;
}

return true;
}

private boolean isFirstChoiceNotSelected(UniversityChoiceRequest request) {
return request.firstChoiceUniversityId() == null;
}

private boolean isThirdChoiceWithoutSecond(UniversityChoiceRequest request) {
return request.thirdChoiceUniversityId() != null && request.secondChoiceUniversityId() == null;
}

private boolean isDuplicate(UniversityChoiceRequest request) {
Set<Long> uniqueIds = new HashSet<>();
return Stream.of(
request.firstChoiceUniversityId(),
request.secondChoiceUniversityId(),
request.thirdChoiceUniversityId()
)
.filter(Objects::nonNull)
.anyMatch(id -> !uniqueIds.add(id));
}
}
Copy link
Member

@wibaek wibaek Feb 8, 2025

Choose a reason for hiding this comment

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

isValidChoice로 분리하신 점이 좋네요👍 addPropertyNode로 문제 필드 알려주는 것도 좋을 것 같습니다.

  1. 선택 중 중복 대학을 검증하는 메서드
  2. n번째 없이 n+1 요소만 선택된 경우를 검증하는 메서드

처럼 메서드를 나누어 검증하는 것도 좋을 것 같네요.

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번째 대학이 있는 경우는 없나 생각했는데, 1번은

        @NotNull(message = "1지망 대학교를 입력해주세요.")

로 검증되고 있었네요.
첫번째 대학도 필드 레벨 검증이 아닌 ValidUniversityChoiceValidator에서 같이 검증하도록 하는 것에 대해서는 어떻게 생각하시나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

제 생각에 addPropertyNode 를 사용하는건 구현이 복잡해질 것 같고, 지금 메세지만으로도 충분히 디버깅할 수 있다 생각해요.
이 부분은 규혁님이 판단하셔서 선택해주세요~

Copy link
Collaborator

Choose a reason for hiding this comment

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

첫번째 대학도 필드 레벨 검증이 아닌 ValidUniversityChoiceValidator에서 같이 검증하도록 하는 것에 대해서는 어떻게 생각하시나요?

저도 같은 생각입니다😊
"유효한 대학 선택인지"에 대한 검증이 한 군데에 모여있어야 더 응집도 있을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

그렇네요 기존코드를 유지한 채 한다는 생각을 자꾸 가지다보니 오히려 더 헷갈리게 만들었네요.. 😂 반영했습니다!

Original file line number Diff line number Diff line change
Expand Up @@ -118,26 +118,6 @@ class ApplicationSubmissionServiceTest extends BaseIntegrationTest {
.hasMessage(INVALID_LANGUAGE_TEST_SCORE_STATUS.getMessage());
}

@Test
void 동일한_대학을_중복_선택하면_예외_응답을_반환한다() {
// given
GpaScore gpaScore = createApprovedGpaScore(테스트유저_1);
LanguageTestScore languageTestScore = createUnapprovedLanguageTestScore(테스트유저_1);
UniversityChoiceRequest universityChoiceRequest = new UniversityChoiceRequest(
괌대학_A_지원_정보.getId(),
괌대학_A_지원_정보.getId(),
메모리얼대학_세인트존스_A_지원_정보.getId()
);
ApplyRequest request = new ApplyRequest(gpaScore.getId(), languageTestScore.getId(), universityChoiceRequest);

// when & then
assertThatCode(() ->
applicationSubmissionService.apply(테스트유저_1, request)
)
.isInstanceOf(CustomException.class)
.hasMessage(CANT_APPLY_FOR_SAME_UNIVERSITY.getMessage());
}

@Test
void 지원서_수정_횟수를_초과하면_예외_응답을_반환한다() {
// given
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
package com.example.solidconnection.custom.validation.validator;

import com.example.solidconnection.application.dto.UniversityChoiceRequest;
import jakarta.validation.ConstraintViolation;
import jakarta.validation.Validation;
import jakarta.validation.Validator;
import jakarta.validation.ValidatorFactory;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.util.Set;

import static com.example.solidconnection.custom.exception.ErrorCode.DUPLICATE_UNIVERSITY_CHOICE;
import static com.example.solidconnection.custom.exception.ErrorCode.FIRST_CHOICE_REQUIRED;
import static com.example.solidconnection.custom.exception.ErrorCode.THIRD_CHOICE_REQUIRES_SECOND;
import static org.assertj.core.api.Assertions.assertThat;

@DisplayName("대학 선택 유효성 검사 테스트")
class ValidUniversityChoiceValidatorTest {

private static final String MESSAGE = "message";

private Validator validator;

@BeforeEach
void setUp() {
ValidatorFactory factory = Validation.buildDefaultValidatorFactory();
validator = factory.getValidator();
}

@Test
void 정상적인_지망_선택은_유효하다() {
// given
UniversityChoiceRequest request = new UniversityChoiceRequest(1L, 2L, 3L);

// when
Set<ConstraintViolation<UniversityChoiceRequest>> violations = validator.validate(request);

// then
assertThat(violations).isEmpty();
}

@Test
void 첫_번째_지망만_선택하는_것은_유효하다() {
// given
UniversityChoiceRequest request = new UniversityChoiceRequest(1L, null, null);

// when
Set<ConstraintViolation<UniversityChoiceRequest>> violations = validator.validate(request);

// then
assertThat(violations).isEmpty();
}

@Test
void 두_번째_지망_없이_세_번째_지망을_선택하면_예외_응답을_반환한다() {
// given
UniversityChoiceRequest request = new UniversityChoiceRequest(1L, null, 3L);

// when
Set<ConstraintViolation<UniversityChoiceRequest>> violations = validator.validate(request);

// then
assertThat(violations)
.extracting(MESSAGE)
.contains(THIRD_CHOICE_REQUIRES_SECOND.getMessage());
}

@Test
void 첫_번째_지망을_선택하지_않으면_예외_응답을_반환한다() {
// given
UniversityChoiceRequest request = new UniversityChoiceRequest(null, 2L, 3L);

// when
Set<ConstraintViolation<UniversityChoiceRequest>> violations = validator.validate(request);

// then
assertThat(violations)
.isNotEmpty()
.extracting(MESSAGE)
.contains(FIRST_CHOICE_REQUIRED.getMessage());
}

@Test
void 대학을_중복_선택하면_예외_응답을_반환한다() {
// given
UniversityChoiceRequest request = new UniversityChoiceRequest(1L, 1L, 2L);

// when
Set<ConstraintViolation<UniversityChoiceRequest>> violations = validator.validate(request);

// then
assertThat(violations)
.isNotEmpty()
.extracting(MESSAGE)
.contains(DUPLICATE_UNIVERSITY_CHOICE.getMessage());
}
}