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

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

Merged
merged 19 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
83d7daa
chore: 유효성 검증을 위한 build.gradle 수정
Mingyum-Kim Jul 24, 2024
635f6aa
feat: 엔티티와 데이터베이스 레벨에서 유효성 검증 추가
Mingyum-Kim Jul 24, 2024
656cb54
feat: 요청 DTO에 Bean Validation 추가
Mingyum-Kim Jul 24, 2024
6a23e49
feat: 모임 날짜를 현재 시점 이전인 경우 예외처리
Mingyum-Kim Jul 24, 2024
4b73b79
feat: 문자열 길이에 대한 검증 로직 추가
Mingyum-Kim Jul 24, 2024
26fc934
feat: Bean Validation 에러 핸들링 구현
Mingyum-Kim Jul 25, 2024
c930780
feat: 도메인에서 null, blank, 길이, 범위 검증을 하도록 구현
Mingyum-Kim Jul 25, 2024
7a17492
test: 모임 생성 및 실패 테스트
Mingyum-Kim Jul 25, 2024
de69c3b
feat: 회원 닉네임 유효성 검사 및 테스트
Mingyum-Kim Jul 25, 2024
f331193
fix: 실패하는 테스트 케이스 수정
Mingyum-Kim Jul 25, 2024
d9de4ab
chore: CI 워크플로우 중 테스트 포함하도록 수정
Mingyum-Kim Jul 25, 2024
2cc4edf
test: 테스트가 실패하도록 수정
Mingyum-Kim Jul 25, 2024
060203d
fix: 실패하는 테스트 케이스 수정
Mingyum-Kim Jul 25, 2024
6aec687
Merge branch 'feature/#104' of https://github.com/woowacourse-teams/2…
Mingyum-Kim Jul 25, 2024
d5ede70
test: 모임 생성 시 과거 시점으로 설정한 값 수정
Mingyum-Kim Jul 25, 2024
bdaf92e
test: 테스트 메서드 수정 및 컨벤션 통일
Mingyum-Kim Jul 31, 2024
a0fda8b
Merge branch 'develop-backend' into feature/#104
Mingyum-Kim Jul 31, 2024
8c2d30c
refactor: 사용하지 않는 필드와 메서드 제거
Mingyum-Kim Jul 31, 2024
948a561
test: 실패하는 테스트 케이스 수정
Mingyum-Kim Jul 31, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ build/
!**/src/main/**/build/
!**/src/test/**/build/
.idea

out
14 changes: 10 additions & 4 deletions backend/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,24 @@ repositories {
}

dependencies {
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
// web
implementation 'org.springframework.boot:spring-boot-starter-web'
implementation 'org.springframework.boot:spring-boot-starter-validation'
implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.2.0'

// lombok
compileOnly 'org.projectlombok:lombok'
annotationProcessor 'org.projectlombok:lombok'

// database
implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
runtimeOnly 'com.h2database:h2'
runtimeOnly 'com.mysql:mysql-connector-j'
annotationProcessor 'org.projectlombok:lombok'

// test
testImplementation 'io.rest-assured:rest-assured:5.3.1'
testImplementation 'org.springframework.boot:spring-boot-starter-test'
testRuntimeOnly 'org.junit.platform:junit-platform-launcher'

implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.2.0'
}

tasks.named('test') {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,26 @@
package mouda.backend.exception;

import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.context.request.WebRequest;
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;

@RestControllerAdvice
public class GlobalExceptionHandler extends ResponseEntityExceptionHandler {

@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));
}
Comment on lines +16 to +22
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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


@ExceptionHandler(MoudaException.class)
public ResponseEntity<ErrorResponse> handleMoudaException(MoudaException exception) {
return ResponseEntity.status(exception.getHttpStatus()).body(new ErrorResponse(exception.getMessage()));
Expand Down
16 changes: 16 additions & 0 deletions backend/src/main/java/mouda/backend/member/domain/Member.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package mouda.backend.member.domain;

import org.springframework.http.HttpStatus;

import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
Expand All @@ -10,12 +12,16 @@
import lombok.Getter;
import lombok.NoArgsConstructor;
import mouda.backend.moim.domain.Moim;
import mouda.backend.moim.exception.MoimErrorMessage;
import mouda.backend.moim.exception.MoimException;

@Entity
@Getter
@NoArgsConstructor
public class Member {

private static final int NICKNAME_MAX_LENGTH = 10;

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
Expand All @@ -27,9 +33,19 @@ public class Member {

@Builder
public Member(String nickname) {
validateNickname(nickname);
this.nickname = nickname;
}

private void validateNickname(String nickname) {
if (nickname.isBlank()) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.MEMBER_NICKNAME_NOT_EXISTS);
}
if (nickname.length() >= NICKNAME_MAX_LENGTH) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.MEMBER_NICKNAME_TOO_LONG);
}
}

public void joinMoim(Moim moim) {
this.moim = moim;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import mouda.backend.common.RestResponse;
import mouda.backend.moim.domain.Moim;
Expand All @@ -27,7 +28,7 @@ public class MoimController implements MoimSwagger {

@Override
@PostMapping
public ResponseEntity<RestResponse<Long>> createMoim(@RequestBody MoimCreateRequest moimCreateRequest) {
public ResponseEntity<RestResponse<Long>> createMoim(@Valid @RequestBody MoimCreateRequest moimCreateRequest) {
Moim moim = moimService.createMoim(moimCreateRequest);

return ResponseEntity.ok().body(new RestResponse<>(moim.getId()));
Expand All @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

MoimDetailsFindResponse moimDetailsFindResponse = moimService.findMoimDetails(moimId);

return ResponseEntity.ok().body(new RestResponse<>(moimDetailsFindResponse));
Expand Down
74 changes: 74 additions & 0 deletions backend/src/main/java/mouda/backend/moim/domain/Moim.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package mouda.backend.moim.domain;

import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;

import org.springframework.http.HttpStatus;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
Expand All @@ -22,18 +24,30 @@
@NoArgsConstructor
public class Moim {

private static final int TITLE_MAX_LENGTH = 30;
private static final int PLACE_MAX_LENGTH = 100;
private static final int MAX_PEOPLE_LOWER_BOUND = 1;
private static final int MAX_PEOPLE_UPPER_BOUND = 99;
private static final int AUTHOR_NICKNAME_MAX_LENGTH = 10;
private static final int DESCRIPTION_MAX_LENGTH = 1000;

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@Column(nullable = false)
private String title;

@Column(nullable = false)
private LocalDate date;

@Column(nullable = false)
private LocalTime time;

@Column(nullable = false)
private String place;

@Column(nullable = false)
private int maxPeople;

private String description;
Expand All @@ -52,6 +66,14 @@ public Moim(
int maxPeople,
String description
) {
validateTitle(title);
validateDate(date);
validateTime(time);
validateMoimIsFuture(date, time);
validatePlace(place);
validateMaxPeople(maxPeople);
validateDescription(description);

this.title = title;
this.date = date;
this.time = time;
Expand All @@ -60,6 +82,58 @@ public Moim(
this.description = description;
}

private void validateTitle(String title) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

if (title.isBlank()) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.TITLE_NOT_EXIST);
}
if (title.length() > TITLE_MAX_LENGTH) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.TITLE_TOO_LONG);
}
}

private void validateDate(LocalDate date) {
if (date == null) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.DATE_NOT_EXIST);
}
}

private void validateTime(LocalTime time) {
if (time == null) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.TIME_NOT_EXIST);
}
}

private void validateMoimIsFuture(LocalDate date, LocalTime time) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

LocalDateTime moimDateTime = LocalDateTime.of(date, time);
if (moimDateTime.isBefore(LocalDateTime.now())) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.PAST_DATE_TIME);
}
}

private void validatePlace(String place) {
if (place.isBlank()) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.PLACE_NOT_EXIST);
}
if (place.length() > PLACE_MAX_LENGTH) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.PLACE_TOO_LONG);
}
}

private void validateMaxPeople(int maxPeople) {
if (maxPeople < MAX_PEOPLE_LOWER_BOUND) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.MAX_PEOPLE_IS_POSITIVE);
}
if (maxPeople > MAX_PEOPLE_UPPER_BOUND) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.MAX_PEOPLE_TOO_MANY);
}
}

private void validateDescription(String description) {
if (description != null && description.length() > DESCRIPTION_MAX_LENGTH) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.DESCRIPTION_TOO_LONG);
}
}

public void validateAlreadyFullMoim(int currentPeople) {
if (currentPeople > maxPeople) {
throw new MoimException(HttpStatus.BAD_REQUEST, MoimErrorMessage.MAX_PEOPLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,29 @@
import java.time.LocalDate;
import java.time.LocalTime;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import mouda.backend.moim.domain.Moim;

public record MoimCreateRequest(
@NotBlank
String title,
Comment on lines 10 to 12
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@Mingyum-Kim Mingyum-Kim Jul 26, 2024

Choose a reason for hiding this comment

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

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


@NotNull
LocalDate date,

@NotNull
LocalTime time,

@NotBlank
String place,

@NotNull
Integer maxPeople,

@NotBlank
String authorNickname,

String description
) {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
package mouda.backend.moim.dto.request;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Positive;

public record MoimJoinRequest(
@NotNull
@Positive
Long moimId,

@NotBlank
String nickname
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,20 @@ public enum MoimErrorMessage {

NOT_FOUND("모임이 존재하지 않습니다."),
MAX_PEOPLE("모임 최대 인원 수를 초과합니다."),
;
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("모임 참여자 닉네임을 조금 더 짧게 입력해주세요.");
Comment on lines +12 to +25
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

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

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


private final String message;
}
Loading