Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
import io.jsonwebtoken.JwtException;
import lombok.extern.slf4j.Slf4j;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.MethodArgumentNotValidException;
Expand All @@ -13,6 +14,7 @@
import java.util.ArrayList;
import java.util.List;

import static com.example.solidconnection.common.exception.ErrorCode.DATA_INTEGRITY_VIOLATION;
import static com.example.solidconnection.common.exception.ErrorCode.INVALID_INPUT;
import static com.example.solidconnection.common.exception.ErrorCode.JSON_PARSING_FAILED;
import static com.example.solidconnection.common.exception.ErrorCode.JWT_EXCEPTION;
Expand Down Expand Up @@ -56,6 +58,15 @@ public ResponseEntity<ErrorResponse> handleValidationExceptions(MethodArgumentNo
.body(errorResponse);
}

@ExceptionHandler(DataIntegrityViolationException.class)
public ResponseEntity<Object> handleDataIntegrityViolationException(DataIntegrityViolationException ex) {
log.error("데이터 무결성 제약조건 위반 예외 발생 : {}", ex.getMessage());
ErrorResponse errorResponse = new ErrorResponse(DATA_INTEGRITY_VIOLATION, "데이터 무결성 제약조건 위반 예외 발생");
return ResponseEntity
.status(DATA_INTEGRITY_VIOLATION.getCode())
.body(errorResponse);
}

@ExceptionHandler(JwtException.class)
public ResponseEntity<Object> handleJwtException(JwtException ex) {
String errorMessage = ex.getMessage();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ public enum ErrorCode {
USER_DO_NOT_HAVE_GPA(HttpStatus.BAD_REQUEST.value(), "해당 유저의 학점을 찾을 수 없음"),
REJECTED_REASON_REQUIRED(HttpStatus.BAD_REQUEST.value(), "거절 사유가 필요합니다."),

// database
DATA_INTEGRITY_VIOLATION(HttpStatus.CONFLICT.value(), "데이터베이스 무결성 제약조건 위반이 발생했습니다."),

// general
JSON_PARSING_FAILED(HttpStatus.BAD_REQUEST.value(), "JSON 파싱을 할 수 없습니다."),
JWT_EXCEPTION(HttpStatus.BAD_REQUEST.value(), "JWT 토큰을 처리할 수 없습니다."),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.example.solidconnection.siteuser.controller;


import com.example.solidconnection.common.resolver.AuthorizedUser;
import com.example.solidconnection.siteuser.domain.SiteUser;
import com.example.solidconnection.siteuser.dto.MyPageResponse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@
@UniqueConstraint(
name = "uk_site_user_email_auth_type",
columnNames = {"email", "auth_type"}
),
@UniqueConstraint(
name = "uk_site_user_nickname",
columnNames = {"nickname"}
)
})
public class SiteUser {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.example.solidconnection.siteuser.repository;


import com.example.solidconnection.siteuser.domain.AuthType;
import com.example.solidconnection.siteuser.domain.SiteUser;
import org.springframework.data.jpa.repository.JpaRepository;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import static com.example.solidconnection.common.exception.ErrorCode.CAN_NOT_CHANGE_NICKNAME_YET;
import static com.example.solidconnection.common.exception.ErrorCode.NICKNAME_ALREADY_EXISTED;
import static com.example.solidconnection.common.exception.ErrorCode.USER_NOT_FOUND;

@RequiredArgsConstructor
@Service
Expand All @@ -47,27 +48,23 @@ public MyPageResponse getMyPageInfo(SiteUser siteUser) {
* */
@Transactional
public void updateMyPageInfo(SiteUser siteUser, MultipartFile imageFile, String nickname) {
SiteUser user = siteUserRepository.findById(siteUser.getId())
Copy link
Member Author

Choose a reason for hiding this comment

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

메서드 파라미터로 SiteUser 객체 전달 관련 이슈: #299

.orElseThrow(() -> new CustomException(USER_NOT_FOUND));

if (nickname != null) {
validateNicknameNotChangedRecently(user.getNicknameModifiedAt());
validateNicknameUnique(nickname);
validateNicknameNotChangedRecently(siteUser.getNicknameModifiedAt());
siteUser.setNickname(nickname);
siteUser.setNicknameModifiedAt(LocalDateTime.now());
user.setNickname(nickname);
user.setNicknameModifiedAt(LocalDateTime.now());
}

if (imageFile != null && !imageFile.isEmpty()) {
UploadedFileUrlResponse uploadedFile = s3Service.uploadFile(imageFile, ImgType.PROFILE);
if (!isDefaultProfileImage(siteUser.getProfileImageUrl())) {
s3Service.deleteExProfile(siteUser);
if (!isDefaultProfileImage(user.getProfileImageUrl())) {
s3Service.deleteExProfile(user);
}
String profileImageUrl = uploadedFile.fileUrl();
siteUser.setProfileImageUrl(profileImageUrl);
}
siteUserRepository.save(siteUser);
}

private void validateNicknameUnique(String nickname) {
if (siteUserRepository.existsByNickname(nickname)) {
throw new CustomException(NICKNAME_ALREADY_EXISTED);
user.setProfileImageUrl(profileImageUrl);
}
}

Expand All @@ -82,6 +79,12 @@ private void validateNicknameNotChangedRecently(LocalDateTime lastModifiedAt) {
}
}

private void validateNicknameUnique(String nickname) {
if (siteUserRepository.existsByNickname(nickname)) {
throw new CustomException(NICKNAME_ALREADY_EXISTED);
}
}

private boolean isDefaultProfileImage(String profileImageUrl) {
String prefix = "profile/";
return profileImageUrl == null || !profileImageUrl.startsWith(prefix);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE site_user
ADD CONSTRAINT uk_site_user_nickname
UNIQUE (nickname);
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

import static com.example.solidconnection.e2e.DynamicFixture.createSiteUserByEmail;
import static com.example.solidconnection.e2e.DynamicFixture.createSiteUserByEmailAndNickname;
import static org.junit.jupiter.api.Assertions.assertEquals;

@TestContainerSpringBootTest
Expand Down Expand Up @@ -92,7 +92,8 @@ private Post createPost(Board board, SiteUser siteUser) {

for (int i = 0; i < THREAD_NUMS; i++) {
String email = "email" + i;
SiteUser tmpSiteUser = siteUserRepository.save(createSiteUserByEmail(email));
String nickname = "nickname" + i;
SiteUser tmpSiteUser = siteUserRepository.save(createSiteUserByEmailAndNickname(email, nickname));
executorService.submit(() -> {
try {
postLikeService.likePost(tmpSiteUser, post.getId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@

public class DynamicFixture { // todo: test fixture 개선 작업 이후, 이 클래스의 사용이 대체되면 삭제 필요

public static SiteUser createSiteUserByEmail(String email) {
public static SiteUser createSiteUserByEmailAndNickname(String email, String nickname) {
return new SiteUser(
email,
"nickname",
nickname,
"profileImage",
PreparationStatus.CONSIDERING,
Role.MENTEE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class 이메일과_인증_유형이_동일한_사용자는_저장할_수_없다
@Test
void 이메일과_인증_유형이_동일한_사용자를_저장하면_예외_응답을_반환한다() {
// given
SiteUser user1 = createSiteUser("email", AuthType.KAKAO);
SiteUser user2 = createSiteUser("email", AuthType.KAKAO);
SiteUser user1 = createSiteUser("email", "nickname1", AuthType.KAKAO);
SiteUser user2 = createSiteUser("email", "nickname2", AuthType.KAKAO);
siteUserRepository.save(user1);

// when, then
Expand All @@ -36,8 +36,8 @@ class 이메일과_인증_유형이_동일한_사용자는_저장할_수_없다
@Test
void 이메일이_같더라도_인증_유형이_다른_사용자는_정상_저장한다() {
// given
SiteUser user1 = createSiteUser("email", AuthType.KAKAO);
SiteUser user2 = createSiteUser("email", AuthType.APPLE);
SiteUser user1 = createSiteUser("email", "nickname1", AuthType.KAKAO);
SiteUser user2 = createSiteUser("email", "nickname2", AuthType.APPLE);
siteUserRepository.save(user1);

// when, then
Expand All @@ -46,10 +46,42 @@ class 이메일과_인증_유형이_동일한_사용자는_저장할_수_없다
}
}

private SiteUser createSiteUser(String email, AuthType authType) {
@Nested
class 닉네임은_중복될_수_없다 {

@Test
void 중복된_닉네임으로_사용자를_저장하면_예외_응답을_반환한다() {
// given
SiteUser user1 = createSiteUser("email1", "nickname", AuthType.KAKAO);
SiteUser user2 = createSiteUser("email2", "nickname", AuthType.KAKAO);
siteUserRepository.save(user1);

// when, then
assertThatCode(() -> siteUserRepository.saveAndFlush(user2))
.isInstanceOf(DataIntegrityViolationException.class);
Comment on lines +60 to +61
Copy link
Collaborator

@nayonsoso nayonsoso Jun 17, 2025

Choose a reason for hiding this comment

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

save에서 saveAnsFlush()로 바꾸신 이유가 궁금해요!

Copy link
Member Author

Choose a reason for hiding this comment

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

DB 레벨 테스트는 save 대신 saveAndFlush를 사용하였습니다. 아무래도 DB에 반영시킨 후의 결과를 봐야 하니까요. 물론 중복된_닉네임으로_사용자를_저장하면_예외_응답을_반환한다 테스트는 save 를 사용해도 통과가 되지만, DB 레벨 테스트라 saveAndFlush를 사용하였습니다. 아무래도 성능 측면에서는 save가 더 좋긴 할텐데, 어떤 기준으로 save, saveAndFlush를 선택해야 할지 고민이 되네요 ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

아무래도 DB에 반영시킨 후의 결과를 봐야 하니까요.

아무래도 성능 측면에서는 save가 더 좋긴 할텐데,

이 부분에 대해서 제가 아는 내용으로는, (아니라면 지적해주세요!)
우리 테스트 코드에서는 Transactional 어노테이션을 사용하지 않고 있기 때문에,
assert문 내부에 save와 saveAndFlush 의 동작은 실질적으로 차이가 없다고 알고 있습니다!

물론 Transactional 어노테이션이 있는 테스트 코드라면 반드시 saveAndFlush를 써야 assert문에 걸리겠지만요 ㅎㅎ


어떤 기준으로 save, saveAndFlush를 선택해야 할지 고민이 되네요 ...

저도 고민이 되네요.. 🤔 우리 컨벤션대로라면 save를 써도 될텐데,.
미래에 Transactional 어노테이션을 쓸 상황을 대비해서 saveAndFlush로 방어적인 코드를 작성해야할지...?
개인적으로 saveAndFlush를 쓰는게 더 좋겠다는 생각은 듭니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

그렇군요 .. @Transaction 이 붙은 테스트 코드 작성 시에는 주의해야겠네요.

저는 방어적으로 작성한다고 손해보지는 않는다고 생각합니다 ㅎㅎ

}

@Test
void 중복된_닉네임으로_변경하면_예외_응답을_반환한다() {
// given
SiteUser user1 = createSiteUser("email1", "nickname1", AuthType.KAKAO);
SiteUser user2 = createSiteUser("email2", "nickname2", AuthType.KAKAO);
siteUserRepository.save(user1);
siteUserRepository.save(user2);

// when
user2.setNickname("nickname1");

// then
assertThatCode(() -> siteUserRepository.saveAndFlush(user2))
.isInstanceOf(DataIntegrityViolationException.class);
}
}

private SiteUser createSiteUser(String email, String nickname, AuthType authType) {
return new SiteUser(
email,
"nickname",
nickname,
"profileImageUrl",
PreparationStatus.CONSIDERING,
Role.MENTEE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@
import java.util.List;

import static com.example.solidconnection.common.exception.ErrorCode.CAN_NOT_CHANGE_NICKNAME_YET;
import static com.example.solidconnection.common.exception.ErrorCode.NICKNAME_ALREADY_EXISTED;
import static com.example.solidconnection.siteuser.service.MyPageService.MIN_DAYS_BETWEEN_NICKNAME_CHANGES;
import static com.example.solidconnection.siteuser.service.MyPageService.NICKNAME_LAST_CHANGE_DATE_FORMAT;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.BDDMockito.any;
import static org.mockito.BDDMockito.eq;
import static org.mockito.BDDMockito.given;
Expand Down Expand Up @@ -118,7 +118,8 @@ class 프로필_이미지_수정_테스트 {
myPageService.updateMyPageInfo(user, imageFile, "newNickname");

// then
assertThat(user.getProfileImageUrl()).isEqualTo(expectedUrl);
SiteUser updatedUser = siteUserRepository.findById(user.getId()).get();
assertThat(updatedUser.getProfileImageUrl()).isEqualTo(expectedUrl);
}

@Test
Expand Down Expand Up @@ -147,7 +148,8 @@ class 프로필_이미지_수정_테스트 {
myPageService.updateMyPageInfo(커스텀_프로필_사용자, imageFile, "newNickname");

// then
then(s3Service).should().deleteExProfile(커스텀_프로필_사용자);
then(s3Service).should().deleteExProfile(argThat(user ->
user.getId().equals(커스텀_프로필_사용자.getId())));
}
}

Expand Down Expand Up @@ -175,23 +177,13 @@ void setUp() {
assertThat(updatedUser.getNickname()).isEqualTo(newNickname);
}

@Test
void 중복된_닉네임으로_변경하면_예외_응답을_반환한다() {
// given
SiteUser existingUser = siteUserFixture.사용자(1, "existing nickname");

// when & then
assertThatCode(() -> myPageService.updateMyPageInfo(user, null, existingUser.getNickname()))
.isInstanceOf(CustomException.class)
.hasMessage(NICKNAME_ALREADY_EXISTED.getMessage());
}

@Test
void 최소_대기기간이_지나지_않은_상태에서_변경하면_예외_응답을_반환한다() {
// given
MockMultipartFile imageFile = createValidImageFile();
LocalDateTime modifiedAt = LocalDateTime.now().minusDays(MIN_DAYS_BETWEEN_NICKNAME_CHANGES - 1);
user.setNicknameModifiedAt(modifiedAt);
siteUserRepository.save(user);

// when & then
assertThatCode(() -> myPageService.updateMyPageInfo(user, imageFile, "nickname12"))
Expand Down
Loading