입력값과 엔티티 필드에 대한 유효성 검증 구현 #120

Mingyum-Kim · 2024-07-25T04:44:49Z

PR의 목적이 무엇인가요?

입력값과 엔티티 필드 등에 대한 유효성 검증 구현
Bean Validation 에러 발생 시 에러 핸들링 구현

이슈 ID는 무엇인가요?

Closed 입력값과 엔티티 필드에 대한 유효성 검증 구현 #104

설명

엔티티와 DTO에서의 유효성 검증을 다르게 가져갔습니다.
엔티티는 모임 생성 API 뿐만 아니라 의도치 않은 객체 생성을 고려해서 모든 유효성 검증을 처리하였습니다. 반면 DTO는 세밀한 유효성 검증이 필요하지 않고, NPE 등을 방지하기 위해 NULL과 BLANK 체크만 이루어지면 된다고 생각했어요.

정리하자면 유효성 검증 목록은 아래와 같습니다 😄

엔티티 레벨에서의 유효성 검증

NULL 체크
BLANK 체크
문자열 길이 검증
숫자 범위 검증
모임 날짜와 시간이 과거인지 검증

DTO 레벨에서의 유효성 검증

NULL 체크
BLANK 체크

추가로, 회의 결과를 반영하여 Bean Validation 오류 시 던져지는 MethodArgumentNotValidException을 핸들링하여 제일 처음의 오류 메시지만 반환하도록 구현하였습니다.

질문 혹은 공유 사항 (Optional)

유효성 검증에 의해 엔티티가 길어지는 현상
MIN, MAX 등의 상수와 validate 함수로 인해 엔티티가 너무 길어지는 문제가 있었습니다.
VO 사용 등을 통해 해당 문제와 중복 코드를 해결할 수 있을 것 같습니다.
DTO와 엔티티의 유효성 검증 범위
앞서 설명 드린 유효성 검증에 대한 의견이 궁금합니다 😄
엔티티와 DTO에 대해 유효성 검증 역할이 적절히 분배된 게 맞나요?

의존성의 종류 별로 build.gradle를 정리한다.

현재 시점보다 이전인 모임 날짜를 수정 파라미터 미입력으로 인한 오류 수정

PR 올린 시점에서 CI가 잘 동작하는 지 확인하기 위해 실패하는 테스트 케이스 생성

…024-mouda into feature/#104

ay-eonii

고생했어요 안나 👍

코멘트 확인 부탁드려용~!!

ay-eonii · 2024-07-25T05:42:10Z

backend/src/main/java/mouda/backend/exception/GlobalExceptionHandler.java

+	@Override
+	protected ResponseEntity<Object> handleMethodArgumentNotValid(MethodArgumentNotValidException exception,
+		HttpHeaders headers, HttpStatusCode status, WebRequest request) {
+		String error = exception.getBindingResult().getFieldErrors().get(0).getDefaultMessage();
+
+		return ResponseEntity.badRequest().body(new ErrorResponse(error));
+	}


개인적인 취향인데 오버라이딩한 메서드를 아래에 두는 건 어떤가용?

조금 더 이유를 붙여서 설명해주면 좋을 것 같아요 -!

논리적인 이유는 없고 ..
오버라이딩한 메서드보다 GlobalExceptionHandler 만이 가진 메서드(handleMoudaException, handleException)가 클래스를 더 잘 나타낸다는 느낌이 있어서 위에 말한 방식을 선호하는 것 같아요 ㅎ_ㅎ

논리적인 이유는 없고 ..
오버라이딩한 메서드보다 GlobalExceptionHandler 만이 가진 메서드(handleMoudaException, handleException)가 클래스를 더 잘 나타낸다는 느낌이 있어서 위에 말한 방식을 선호하는 것 같아요 ㅎ_ㅎ

ay-eonii · 2024-07-25T05:57:13Z

backend/src/main/java/mouda/backend/moim/exception/MoimErrorMessage.java

+	PAST_DATE_TIME("모임 날짜를 현재 시점 이후로 입력해주세요."),
+	TITLE_NOT_EXIST("모임 제목을 입력해주세요."),
+	TITLE_TOO_LONG("모임 제목을 조금 더 짧게 입력해주세요."),
+	DATE_NOT_EXIST("모임 날짜를 입력해주세요."),
+	TIME_NOT_EXIST("모임 시간을 입력해주세요."),
+	PLACE_NOT_EXIST("모임 장소를 입력해주세요."),
+	PLACE_TOO_LONG("모임 장소를 조금 더 짧게 입력해주세요."),
+	MAX_PEOPLE_IS_POSITIVE("모임 최대 인원은 양수여야 합니다."),
+	MAX_PEOPLE_TOO_MANY("모임 최대 인원을 조금 더 적게 입력해주세요."),
+	AUTHOR_NICKNAME_NOT_EXIST("모임 생성자 닉네임을 입력해주세요."),
+	AUTHOR_NICKNAME_TOO_LONG("모임 생성자 이름을 조금 더 짧게 입력해주세요."),
+	DESCRIPTION_TOO_LONG("모임 설명을 조금 더 짧게 입력해주세요."),
+	MEMBER_NICKNAME_NOT_EXISTS("모임 참여자 닉네임을 입력해주세요."),
+	MEMBER_NICKNAME_TOO_LONG("모임 참여자 닉네임을 조금 더 짧게 입력해주세요.");


저는 백엔드의 예외메시지는 개발자가 예외를 빨리 파악하기 위한 용도라고 생각해요. 현재 메시지는 사용자에게 좋지만, 상태를 표현하는 예외메시지는 개발자에게 좋을 것 같아요. 예를 들어, "모임 참여자 닉네임 길이를 초과했습니다." 처럼요. 이 예외가 발생하는 게 사용자의 입력에 의해 발생하는 건 맞지만 Moim 객체는 어디서 파라미터가 들어오는지 모르니까요!

물론 취향 문제라고도 생각해서 안나 의견 존중합니다~!

저는 백엔드의 예외메시지는 개발자가 예외를 빨리 파악하기 위한 용도라고 생각해요. 현재 메시지는 사용자에게 좋지만, 상태를 표현하는 예외메시지는 개발자에게 좋을 것 같아요. 예를 들어, "모임 참여자 닉네임 길이를 초과했습니다." 처럼요. 이 예외가 발생하는 게 사용자의 입력에 의해 발생하는 건 맞지만 Moim 객체는 어디서 파라미터가 들어오는지 모르니까요!

물론 취향 문제라고도 생각해서 안나 의견 존중합니다~!

나중에 로깅이 추가될때 다시 짚어보면 좋을듯요!

ay-eonii · 2024-07-25T06:02:33Z

backend/src/test/java/mouda/backend/moim/domain/MoimTest.java

+			.date(LocalDate.now())
+			.time(LocalTime.now().minusHours(1))


Suggested change

.date(LocalDate.now())

.time(LocalTime.now().minusHours(1))

.date(DATE)

.time(TIME)

과거 시간이어서 예외가 발생할 수 있을 것 같아요~!

과거 시간을 테스트하는 게 맞아요 😀

ay-eonii · 2024-07-25T06:04:50Z

backend/src/test/java/mouda/backend/moim/domain/MoimTest.java

+	@DisplayName("설명의 길이가 길면 모임 객체 생성에 성공한다.")
+	@Test
+	void createMoimWhenDescriptionIsTooLong() {


Suggested change

@DisplayName("설명의 길이가 길면 모임 객체 생성에 성공한다.")

@Test

void createMoimWhenDescriptionIsTooLong() {

@DisplayName("설명의 길이가 길면 모임 객체 생성에 실패한다.")

@Test

void failToCreateMoimWhenDescriptionIsTooLong() {

ksk0605

고생하셨어요! 몇 가지 코멘트들 확인 부탁드릴게요.
테스트 픽스쳐 같은 경우는 이번 이슈에서 적용안해도 될거같아요 참고만 부탁드려요 ㅎㅎ

ksk0605 · 2024-07-25T08:23:58Z

backend/src/main/java/mouda/backend/moim/dto/request/MoimCreateRequest.java

 public record MoimCreateRequest(
+	@NotBlank
 	String title,


클래스 명 이후 한 줄 띄어쓰는 것이 컨벤션 맞죠..?!

Record의 경우에는 이 부분이 파라미터에 해당해서 안 띄우는 게 맞다고 생각했어요 👍

ksk0605 · 2024-07-25T08:27:44Z

backend/src/test/java/mouda/backend/member/repository/MemberRepositoryTest.java

+			.title("모임 제목")
+			.date(LocalDate.now().plusDays(1))
+			.time(LocalTime.now().minusHours(1))
+			.place("서울시 강북구 중앙로 2길 25")
+			.maxPeople(10)
+			.authorNickname("안나")
+			.description("모임 설명입니다.")


반복적인 모임 생성이 슬슬 보이기 시작하군요..!
테스트 픽스쳐 적용도 고려해보시죠!

ksk0605 · 2024-07-25T08:31:58Z

backend/src/test/java/mouda/backend/moim/service/MoimServiceTest.java

@@ -107,7 +107,7 @@ void joinMoim() {
 	@Test
 	void deleteMoim() {
 		MoimCreateRequest moimCreateRequest = new MoimCreateRequest(
-			"title", LocalDate.now(), LocalTime.now(), "place",
+			"title", LocalDate.now().plusDays(1), LocalTime.now(), "place",
 			10, "안나", "설명"
 		);


요청 객체도 마찬가지구요! (Test Fixture)

hoyeonyy

꼼꼼하게 검증 처리해준 안나에게 발박수 드립니다,,

테바가 제안해준 test fixture가 슬슬 필요할 시기라고도 느껴지네요!

고생하셨습니다! approve 남길께요!

hoyeonyy · 2024-07-27T07:09:25Z

backend/src/main/java/mouda/backend/moim/controller/MoimController.java

@@ -43,7 +44,7 @@ public ResponseEntity<RestResponse<MoimFindAllResponses>> findAllMoim() {

 	@Override
 	@GetMapping("/{moimId}")
-	public ResponseEntity<RestResponse<MoimDetailsFindResponse>> findMoimDetails(@PathVariable Long moimId) {
+	public ResponseEntity<RestResponse<MoimDetailsFindResponse>> findMoimDetails(@PathVariable("moimId") Long moimId) {


이 코드는 조금 더 명시적인 측면에서 작성하신 건가요?!

이 코드는 조금 더 명시적인 측면에서 작성하신 건가요?!

moimId를 명시하지 않았을때 안나 테스트가 터지더라고~! 머지 되면 각자 컴퓨터에서 다시 돌려서 확인해봐용~~

hoyeonyy · 2024-07-27T07:10:40Z

backend/src/main/java/mouda/backend/moim/domain/Moim.java

@@ -57,6 +76,67 @@ public Moim(
 		this.description = description;
 	}

+	private void validateTitle(String title) {


안나의 꼼꼼함에 발박수 드립니다.

hoyeonyy

꼼꼼하게 검증처리를 해준 안나에게 발 박수 드립니다,,

테바가 제안해준 Test Fixture를 도입해야 될 때가 된것 같긴하네요..!

고생하셨습니다~! approve 할께요!

pricelees

안나 고생많았어~!👍

MIN, MAX 등의 상수와 validate 함수로 인해 엔티티가 너무 길어지는 문제가 있었습니다.
VO 사용 등을 통해 해당 문제와 중복 코드를 해결할 수 있을 것 같습니다.

아직은 괜찮은 것 같아요!

앞서 설명 드린 유효성 검증에 대한 의견이 궁금합니다 😄
엔티티와 DTO에 대해 유효성 검증 역할이 적절히 분배된 게 맞나요?

저는 좋아요~! 역할에 따라 꼼꼼히 잘 해주신 것 같습니다!

pricelees · 2024-07-27T09:42:19Z

backend/src/main/java/mouda/backend/moim/domain/Moim.java

+		String title, LocalDate date, LocalTime time,
+		String place, int maxPeople, String authorNickname, String description


여기서 파라미터 3개,4개로 나누는건 합의가 된 부분인가용?? 나는 동의하지만 반대 의견도 있었어서~!

pricelees · 2024-07-27T09:44:03Z

backend/src/main/java/mouda/backend/moim/domain/Moim.java

+		}
+	}
+
+	private void validateMoimIsFuture(LocalDate date, LocalTime time) {


(개취) 메서드명은 validateIsFutureMoim이 조금 더 자연스러운것 같음~! 아니면 다른 validation 메서드처럼 validateDate 형식으로 가도될듯요!

pricelees · 2024-07-27T09:46:04Z

backend/src/main/java/mouda/backend/moim/domain/Moim.java

+	}
+
+	private void validateDescription(String description) {
+		if (description != null && description.length() > DESCRIPTION_MAX_LENGTH) {


같은 string인데 authorNickname처럼 isBlank가 아니라 null 체크를 한 이유가 있나용?

description은 NULL이 가능하기 때문에 위와 같이 처리하였습니다 👍

pricelees · 2024-07-27T09:54:08Z

backend/src/test/java/mouda/backend/config/JacksonConfigTest.java

-				"date", "2024-07-19",
+				"date", date.format(formatter),


테스트 수정까지 꼼꼼하게 했네요👍 formatter 사용도 좋습니다!

(참고)
LocalDate의 toString()은 yyyy-MM-dd가 기본값이라 formatter 없이도 정상적으로 동작할거에요!

pricelees · 2024-07-27T09:55:00Z

backend/src/test/java/mouda/backend/member/domain/MemberTest.java

+	@DisplayName("회원을 정상적으로 생성한다.")
+	@Test
+	void createMember() {
+		Assertions.assertDoesNotThrow(() -> Member.builder()


전체적으로 Assertions의 static import를 통일해보면 좋겠네요~!

pricelees · 2024-07-27T09:57:32Z

backend/src/main/java/mouda/backend/moim/exception/MoimErrorMessage.java

+	PAST_DATE_TIME("모임 날짜를 현재 시점 이후로 입력해주세요."),
+	TITLE_NOT_EXIST("모임 제목을 입력해주세요."),
+	TITLE_TOO_LONG("모임 제목을 조금 더 짧게 입력해주세요."),
+	DATE_NOT_EXIST("모임 날짜를 입력해주세요."),
+	TIME_NOT_EXIST("모임 시간을 입력해주세요."),
+	PLACE_NOT_EXIST("모임 장소를 입력해주세요."),
+	PLACE_TOO_LONG("모임 장소를 조금 더 짧게 입력해주세요."),
+	MAX_PEOPLE_IS_POSITIVE("모임 최대 인원은 양수여야 합니다."),
+	MAX_PEOPLE_TOO_MANY("모임 최대 인원을 조금 더 적게 입력해주세요."),
+	AUTHOR_NICKNAME_NOT_EXIST("모임 생성자 닉네임을 입력해주세요."),
+	AUTHOR_NICKNAME_TOO_LONG("모임 생성자 이름을 조금 더 짧게 입력해주세요."),
+	DESCRIPTION_TOO_LONG("모임 설명을 조금 더 짧게 입력해주세요."),
+	MEMBER_NICKNAME_NOT_EXISTS("모임 참여자 닉네임을 입력해주세요."),
+	MEMBER_NICKNAME_TOO_LONG("모임 참여자 닉네임을 조금 더 짧게 입력해주세요.");


저는 백엔드의 예외메시지는 개발자가 예외를 빨리 파악하기 위한 용도라고 생각해요. 현재 메시지는 사용자에게 좋지만, 상태를 표현하는 예외메시지는 개발자에게 좋을 것 같아요. 예를 들어, "모임 참여자 닉네임 길이를 초과했습니다." 처럼요. 이 예외가 발생하는 게 사용자의 입력에 의해 발생하는 건 맞지만 Moim 객체는 어디서 파라미터가 들어오는지 모르니까요!

물론 취향 문제라고도 생각해서 안나 의견 존중합니다~!

나중에 로깅이 추가될때 다시 짚어보면 좋을듯요!

pricelees

굿~~!!

ksk0605

고생했어요 안나~

Mingyum-Kim added 12 commits July 24, 2024 13:13

chore: 유효성 검증을 위한 build.gradle 수정

83d7daa

의존성의 종류 별로 build.gradle를 정리한다.

feat: 엔티티와 데이터베이스 레벨에서 유효성 검증 추가

635f6aa

feat: 요청 DTO에 Bean Validation 추가

656cb54

feat: 모임 날짜를 현재 시점 이전인 경우 예외처리

6a23e49

feat: 문자열 길이에 대한 검증 로직 추가

4b73b79

feat: Bean Validation 에러 핸들링 구현

26fc934

feat: 도메인에서 null, blank, 길이, 범위 검증을 하도록 구현

c930780

test: 모임 생성 및 실패 테스트

7a17492

feat: 회원 닉네임 유효성 검사 및 테스트

de69c3b

fix: 실패하는 테스트 케이스 수정

f331193

현재 시점보다 이전인 모임 날짜를 수정 파라미터 미입력으로 인한 오류 수정

chore: CI 워크플로우 중 테스트 포함하도록 수정

d9de4ab

test: 테스트가 실패하도록 수정

2cc4edf

PR 올린 시점에서 CI가 잘 동작하는 지 확인하기 위해 실패하는 테스트 케이스 생성

Mingyum-Kim added BE 백엔드 관련 이슈입니다. 🌱 기능추가 feature (새로운 기능 구현) labels Jul 25, 2024

Mingyum-Kim requested review from ksk0605, ay-eonii, hoyeonyy and pricelees July 25, 2024 04:44

Mingyum-Kim self-assigned this Jul 25, 2024

Mingyum-Kim added 3 commits July 25, 2024 13:52

fix: 실패하는 테스트 케이스 수정

060203d

Merge branch 'feature/#104' of https://github.com/woowacourse-teams/2…

6aec687

…024-mouda into feature/#104

test: 모임 생성 시 과거 시점으로 설정한 값 수정

d5ede70

ay-eonii reviewed Jul 25, 2024

View reviewed changes

ksk0605 requested changes Jul 25, 2024

View reviewed changes

hoyeonyy approved these changes Jul 27, 2024

View reviewed changes

pricelees requested changes Jul 27, 2024

View reviewed changes

ss0526100 deleted the branch develop-backend July 27, 2024 13:01

ss0526100 closed this Jul 27, 2024

ss0526100 reopened this Jul 27, 2024

Mingyum-Kim added 3 commits July 31, 2024 11:16

test: 테스트 메서드 수정 및 컨벤션 통일

bdaf92e

Merge branch 'develop-backend' into feature/#104

a0fda8b

refactor: 사용하지 않는 필드와 메서드 제거

8c2d30c

pricelees approved these changes Jul 31, 2024

View reviewed changes

ksk0605 approved these changes Jul 31, 2024

View reviewed changes

test: 실패하는 테스트 케이스 수정

948a561

Mingyum-Kim merged commit c85a227 into develop-backend Jul 31, 2024
1 check passed

Mingyum-Kim deleted the feature/#104 branch July 31, 2024 02:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

입력값과 엔티티 필드에 대한 유효성 검증 구현 #120

입력값과 엔티티 필드에 대한 유효성 검증 구현 #120

Mingyum-Kim commented Jul 25, 2024 •

edited

Loading

ay-eonii left a comment

ay-eonii Jul 25, 2024

Mingyum-Kim Jul 26, 2024

ay-eonii Jul 27, 2024

ay-eonii Jul 27, 2024

ay-eonii Jul 25, 2024

pricelees Jul 27, 2024

ay-eonii Jul 25, 2024

Mingyum-Kim Jul 31, 2024

ay-eonii Jul 25, 2024

ksk0605 left a comment

ksk0605 Jul 25, 2024

Mingyum-Kim Jul 26, 2024 •

edited

Loading

ksk0605 Jul 25, 2024

ksk0605 Jul 25, 2024

hoyeonyy left a comment

hoyeonyy Jul 27, 2024

pricelees Jul 27, 2024

hoyeonyy Jul 27, 2024

hoyeonyy left a comment

pricelees left a comment •

edited

Loading

pricelees Jul 27, 2024

pricelees Jul 27, 2024

pricelees Jul 27, 2024

Mingyum-Kim Jul 31, 2024

pricelees Jul 27, 2024 •

edited

Loading

pricelees Jul 27, 2024

pricelees Jul 27, 2024

pricelees left a comment

ksk0605 left a comment

		String title, LocalDate date, LocalTime time,
		String place, int maxPeople, String authorNickname, String description

입력값과 엔티티 필드에 대한 유효성 검증 구현 #120

입력값과 엔티티 필드에 대한 유효성 검증 구현 #120

Conversation

Mingyum-Kim commented Jul 25, 2024 • edited Loading

PR의 목적이 무엇인가요?

이슈 ID는 무엇인가요?

설명

질문 혹은 공유 사항 (Optional)

ay-eonii left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksk0605 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mingyum-Kim Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoyeonyy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoyeonyy left a comment

Choose a reason for hiding this comment

pricelees left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pricelees Jul 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pricelees left a comment

Choose a reason for hiding this comment

ksk0605 left a comment

Choose a reason for hiding this comment

Mingyum-Kim commented Jul 25, 2024 •

edited

Loading

Mingyum-Kim Jul 26, 2024 •

edited

Loading

pricelees left a comment •

edited

Loading

pricelees Jul 27, 2024 •

edited

Loading