Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -61,7 +61,7 @@ public CreateApplicationResponse apply(Long userId, ApplicationRequest request)

@Transactional
public CreateApplicationResponse applyByGuest(ApplicationGuestRequest request) {
Long userId = userService.saveUser(request.toUserJpaEntity());
Long userId = userService.saveOrGetUser(request.toUserJpaEntity());
CreateApplicationResponse response = handleApplication(
userId,
List.of(request.examApplication().examId()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,16 @@ public void syncUserInfoFromProfile(UserJpaEntity user, ProfileJpaEntity profile
}
}

public Long saveUser(UserJpaEntity user) {
Long userId = userJpaRepository.save(user).getId();
if (userId == null) {
throw new CustomRuntimeException(ErrorCode.USER_SAVE_FAILED);
}
return userId;
public Long saveOrGetUser(UserJpaEntity user) {
return userJpaRepository.findByPhoneNumber(user.getPhoneNumber())
.map(UserJpaEntity::getId)
.orElseGet(() -> {
Long userId = userJpaRepository.save(user).getId();
if (userId == null) {
throw new CustomRuntimeException(ErrorCode.USER_SAVE_FAILED);
}
return userId;
});
}
Comment on lines +40 to 50

Choose a reason for hiding this comment

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

high

The current implementation of saveOrGetUser is vulnerable to a race condition. If two concurrent requests attempt to create a user with the same phone number, both might find that the user doesn't exist and then both attempt to save a new user. This will lead to one of the transactions failing with a unique constraint violation (e.g., DataIntegrityViolationException).

To handle this, you can catch the DataIntegrityViolationException and re-query for the user, which should exist at that point. This is a common pattern for handling get-or-create scenarios under concurrency.

Additionally, I recommend annotating the method with @Transactional to ensure it always runs within a transaction, providing predictable behavior. You'll need to import org.springframework.transaction.annotation.Transactional and org.springframework.dao.DataIntegrityViolationException.

Suggested change
public Long saveOrGetUser(UserJpaEntity user) {
return userJpaRepository.findByPhoneNumber(user.getPhoneNumber())
.map(UserJpaEntity::getId)
.orElseGet(() -> {
Long userId = userJpaRepository.save(user).getId();
if (userId == null) {
throw new CustomRuntimeException(ErrorCode.USER_SAVE_FAILED);
}
return userId;
});
}
@Transactional
public Long saveOrGetUser(UserJpaEntity user) {
return userJpaRepository.findByPhoneNumber(user.getPhoneNumber())
.map(UserJpaEntity::getId)
.orElseGet(() -> {
try {
Long userId = userJpaRepository.save(user).getId();
if (userId == null) {
throw new CustomRuntimeException(ErrorCode.USER_SAVE_FAILED);
}
return userId;
} catch (org.springframework.dao.DataIntegrityViolationException e) {
// Race condition: another transaction might have created the user. Re-fetch.
return userJpaRepository.findByPhoneNumber(user.getPhoneNumber())
.map(UserJpaEntity::getId)
.orElseThrow(() -> new CustomRuntimeException(ErrorCode.USER_SAVE_FAILED));
}
});
}


public UserInfoResponse getUserInfo(Long userId) {
Expand Down