-
Notifications
You must be signed in to change notification settings - Fork 35
[volume-1] TDD 기반 1주차 과제 (회원, 포인트) 기능 구현 #41
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
base: leeminkyu-kr96
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package com.loopers.application.point; | ||
|
|
||
| import com.loopers.domain.point.PointModel; | ||
| import com.loopers.domain.point.PointService; | ||
| import com.loopers.domain.user.UserModel; | ||
| import com.loopers.domain.user.UserService; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Component | ||
| public class PointFacade { | ||
| private final PointService pointService; | ||
| private final UserService userService; | ||
|
|
||
| public PointInfo getPoint(String userId) { | ||
| UserModel user = userService.getUser(userId); | ||
| if (user == null) { | ||
| throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다."); | ||
| } | ||
| PointModel pointModel = new PointModel(user, 0); | ||
| PointModel point = pointService.findPoint(pointModel); | ||
|
|
||
| if (point == null) { | ||
| throw new CoreException(ErrorType.NOT_FOUND, "포인트 정보가 없습니다."); | ||
| } | ||
|
|
||
| return PointInfo.from(point); | ||
| } | ||
|
|
||
| public PointInfo chargePoint(String userId, int amount) { | ||
| UserModel user = userService.getUser(userId); | ||
| if (user == null) { | ||
| throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다."); | ||
| } | ||
| PointModel pointModel = new PointModel(user, amount); | ||
| pointService.charge(pointModel); | ||
|
|
||
| PointModel charged = pointService.findPoint(new PointModel(user, 0)); | ||
| return PointInfo.from(charged); | ||
| } | ||
|
Comment on lines
+33
to
+43
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Null 안전성 문제와 중복 코드를 개선해주세요. Line 42에서 또한 Lines 34-37의 사용자 검증 로직은 다음과 같이 개선을 권장합니다: + private UserModel validateAndGetUser(String userId) {
+ UserModel user = userService.getUser(userId);
+ if (user == null) {
+ throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다.");
+ }
+ return user;
+ }
+
public PointInfo chargePoint(String userId, int amount) {
- UserModel user = userService.getUser(userId);
- if (user == null) {
- throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다.");
- }
+ UserModel user = validateAndGetUser(userId);
PointModel pointModel = new PointModel(user, amount);
pointService.charge(pointModel);
PointModel charged = pointService.findPoint(new PointModel(user, 0));
+ if (charged == null) {
+ throw new CoreException(ErrorType.NOT_FOUND, "포인트 충전 후 조회에 실패했습니다.");
+ }
return PointInfo.from(charged);
}🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.loopers.application.point; | ||
|
|
||
| import com.loopers.domain.point.PointModel; | ||
| import com.loopers.domain.user.UserModel; | ||
|
|
||
| public record PointInfo(Long id, UserModel user, int point) { | ||
| public static PointInfo from(PointModel model) { | ||
| return new PointInfo(model.getId(), model.getUser(), model.getPoint()); | ||
| } | ||
| public int getPoint() { | ||
| return point; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| package com.loopers.application.user; | ||
|
|
||
| import com.loopers.domain.user.UserModel; | ||
| import com.loopers.domain.user.UserService; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Component | ||
| public class UserFacade { | ||
| private final UserService userService; | ||
|
|
||
| public UserInfo signup(String userId, String email, String birthDate) { | ||
| UserModel userModel = new UserModel(userId, email, birthDate); | ||
| UserModel savedUser = userService.signUp(userModel); | ||
| return UserInfo.from(savedUser); | ||
| } | ||
|
|
||
| public UserInfo getUser(String userId) { | ||
| UserModel user = userService.getUser(userId); | ||
| if (user == null) { | ||
| throw new CoreException(ErrorType.NOT_FOUND, "존재하지 않는 요청입니다."); | ||
| } | ||
| return UserInfo.from(user); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.loopers.application.user; | ||
|
|
||
| import com.loopers.domain.user.UserModel; | ||
|
|
||
| public record UserInfo(String userId, String email, String birthDate) { | ||
| public static UserInfo from(UserModel model) { | ||
| return new UserInfo( | ||
| model.getUserId(), | ||
| model.getEmail(), | ||
| model.getBirthDate() | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package com.loopers.domain.point; | ||
|
|
||
| import com.loopers.domain.BaseEntity; | ||
| import com.loopers.domain.user.UserModel; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.JoinColumn; | ||
| import jakarta.persistence.ManyToOne; | ||
| import jakarta.persistence.Table; | ||
|
|
||
| @Entity | ||
| @Table(name = "point") | ||
| public class PointModel extends BaseEntity { | ||
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "user_model_id") | ||
| private UserModel user; | ||
| private int point = 0; | ||
|
|
||
| public PointModel() {} | ||
|
|
||
| public PointModel(UserModel user, int point) { | ||
|
|
||
| if( point < 0 ){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다."); | ||
| } | ||
| this.user = user; | ||
| this.point = point; | ||
| } | ||
|
|
||
| public UserModel getUser() { | ||
| return user; | ||
| } | ||
|
|
||
| public int getPoint() { | ||
| return point; | ||
| } | ||
|
|
||
| public void charge(int amount) { | ||
| if (amount < 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다."); | ||
| } | ||
| this.point += amount; | ||
| } | ||
|
|
||
| public void use(int amount) { | ||
|
|
||
| if (amount < 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0보다 커야 합니다."); | ||
| } | ||
| if (point < 0) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다."); | ||
| } | ||
| this.point -= amount; | ||
| } | ||
|
Comment on lines
+47
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 잔액 부족 검증 로직이 잘못되었습니다. Line 52의 다음 diff를 적용하여 잔액 부족 검증을 수정하세요: public void use(int amount) {
if (amount < 0) {
throw new CoreException(ErrorType.BAD_REQUEST, "사용 금액은 0보다 커야 합니다.");
}
- if (point < 0) {
- throw new CoreException(ErrorType.BAD_REQUEST, "포인트는 0 이상이어야 합니다.");
+ if (point < amount) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "포인트 잔액이 부족합니다.");
}
this.point -= amount;
}🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package com.loopers.domain.point; | ||
|
|
||
| import com.loopers.domain.user.UserModel; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| public interface PointRepository { | ||
| Optional<PointModel> findPoint(UserModel user); | ||
| PointModel save(PointModel pointModel); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| package com.loopers.domain.point; | ||
|
|
||
| import com.loopers.domain.user.UserRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import com.loopers.domain.user.UserModel; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Component | ||
| public class PointService { | ||
|
|
||
| private final PointRepository pointRepository; | ||
| private final UserRepository userRepository; | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public PointModel findPoint(PointModel point) { | ||
| UserModel requestUser = point.getUser(); | ||
| var foundUser = userRepository.find(requestUser.getUserId()); | ||
| if (foundUser.isEmpty()) { | ||
| return null; | ||
| } | ||
| return pointRepository.findPoint(foundUser.get()).orElse(null); | ||
| } | ||
|
|
||
| @Transactional | ||
| public void charge(PointModel point) { | ||
| UserModel user = point.getUser(); | ||
| var foundUser = userRepository.find(user.getUserId()) | ||
| .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "유저가 존재하지 않습니다.")); | ||
|
|
||
| var existing = pointRepository.findPoint(foundUser); | ||
| if (existing.isPresent()) { | ||
| existing.get().charge(point.getPoint()); | ||
| pointRepository.save(existing.get()); | ||
| return; | ||
| } | ||
| pointRepository.save(new PointModel(foundUser, point.getPoint())); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| package com.loopers.domain.user; | ||
|
|
||
| import com.loopers.domain.BaseEntity; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.Table; | ||
|
|
||
| @Entity | ||
| @Table(name = "user") | ||
| public class UserModel extends BaseEntity { | ||
|
|
||
| private String userId; | ||
| private String email; | ||
| private String birthDate; | ||
|
|
||
| protected UserModel() {} | ||
|
|
||
| public UserModel(String userId, String email, String birthDate) { | ||
|
|
||
| if ( userId == null || userId.isBlank() ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "UserId는 비어있을 수 없습니다."); | ||
| } | ||
| if ( !userId.matches("^[a-zA-Z0-9_-]{1,10}$") ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "ID 가 `영문 및 숫자 10자 이내` 형식에 맞아야 합니다."); | ||
| } | ||
|
|
||
| if ( email == null || email.isBlank() ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이메일은 비어있을 수 없습니다."); | ||
| } | ||
| if ( !email.matches("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,6}$") ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이메일이 `xx@yy.zz` 형식에 맞아야 합니다."); | ||
| } | ||
|
|
||
| if ( birthDate == null || birthDate.isBlank() ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 비어있을 수 없습니다."); | ||
| } | ||
| if ( !birthDate.matches("^\\d{4}-\\d{2}-\\d{2}$") ) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "생년월일이 `yyyy-MM-dd` 형식에 맞아야 합니다."); | ||
| } | ||
|
|
||
| this.userId = userId; | ||
| this.email = email; | ||
| this.birthDate = birthDate; | ||
| } | ||
|
Comment on lines
+9
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 테이블 이름 충돌: -@Table(name = "user")
+@Table(name = "users")🤖 Prompt for AI Agents |
||
|
|
||
| public String getUserId() { | ||
| return userId; | ||
| } | ||
|
|
||
| public String getEmail() { | ||
| return email; | ||
| } | ||
|
|
||
| public String getBirthDate() { | ||
| return birthDate; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package com.loopers.domain.user; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| public interface UserRepository { | ||
| Optional<UserModel> find(String userId); | ||
| Optional<UserModel> findById(Long id); | ||
|
|
||
| UserModel save(UserModel userModel); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package com.loopers.domain.user; | ||
|
|
||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Component | ||
| public class UserService { | ||
|
|
||
| private final UserRepository userRepository; | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public UserModel getUser(String userId) { | ||
| return userRepository.find(userId).orElse(null); | ||
| } | ||
|
|
||
| @Transactional | ||
| public UserModel signUp(UserModel userModel) { | ||
| Optional<UserModel> user = userRepository.find(userModel.getUserId()); | ||
|
|
||
| if (user.isPresent()) { | ||
| throw new CoreException(ErrorType.CONFLICT, "[userId = " + userModel.getUserId() + "] 아이디가 중복되었습니다."); | ||
| } | ||
| return userRepository.save(userModel); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package com.loopers.infrastructure.point; | ||
|
|
||
| import com.loopers.domain.point.PointModel; | ||
| import com.loopers.domain.user.UserModel; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| public interface PointJpaRepository extends JpaRepository<PointModel, Long> { | ||
| Optional<PointModel> findByUser(UserModel user); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package com.loopers.infrastructure.point; | ||
|
|
||
| import com.loopers.domain.point.PointModel; | ||
| import com.loopers.domain.point.PointRepository; | ||
| import com.loopers.domain.user.UserModel; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Component | ||
| public class PointRepositoryImpl implements PointRepository { | ||
| private final PointJpaRepository pointJpaRepository; | ||
|
|
||
| @Override | ||
| public Optional<PointModel> findPoint(UserModel user) { | ||
| return pointJpaRepository.findByUser(user); | ||
| } | ||
|
|
||
| @Override | ||
| public PointModel save(PointModel pointModel) { | ||
| return pointJpaRepository.save(pointModel); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package com.loopers.infrastructure.user; | ||
|
|
||
| import com.loopers.domain.user.UserModel; | ||
| import org.springframework.data.jpa.repository.JpaRepository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| public interface UserJpaRepository extends JpaRepository<UserModel, Long> { | ||
| Optional<UserModel> findByUserId(String userId); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package com.loopers.infrastructure.user; | ||
|
|
||
| import com.loopers.domain.user.UserModel; | ||
| import com.loopers.domain.user.UserRepository; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @RequiredArgsConstructor | ||
| @Component | ||
| public class UserRepositoryImpl implements UserRepository { | ||
| private final UserJpaRepository userJpaRepository; | ||
|
|
||
| @Override | ||
| public Optional<UserModel> find(String userId) { | ||
| return userJpaRepository.findByUserId(userId); | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<UserModel> findById(Long id) { | ||
| return Optional.empty(); | ||
| } | ||
|
Comment on lines
+21
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 구현 누락: findById가 항상 빈 Optional 반환 @Override
public Optional<UserModel> findById(Long id) {
- return Optional.empty();
+ return userJpaRepository.findById(id);
}
🤖 Prompt for AI Agents |
||
|
|
||
| @Override | ||
| public UserModel save(UserModel userModel) { | ||
| return userJpaRepository.save(userModel); | ||
| } | ||
|
|
||
|
|
||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
포인트 조회 로직의 비즈니스 요구사항을 명확히 해주세요.
현재 구현에서는 포인트 정보가 없으면 예외를 던지는데, 신규 유저의 경우 포인트 레코드가 아직 생성되지 않았을 수 있습니다. 이 경우 0 포인트를 반환하는 것이 더 적절할 수 있습니다.
또한 Line 23에서
new PointModel(user, 0)로 생성하는 부분은 설계상 어색합니다.PointService.findPoint가 실제로는 user만 필요로 하는데 PointModel을 파라미터로 받고 있습니다.다음을 확인해주세요:
PointService.findPoint(UserModel user)시그니처로 리팩토링 검토Length of output: 168
🏁 Script executed:
Length of output: 7228
🏁 Script executed:
Length of output: 153
🏁 Script executed:
Length of output: 3467
🏁 Script executed:
Length of output: 3712
PointService.findPoint 메서드의 API 설계를 개선하세요.
코드 검증 결과, PointService.findPoint가 PointModel을 받지만 실제로는 user만 사용합니다(line 21:
point.getUser()). 반면 호출되는 PointRepository.findPoint는 UserModel을 직접 받습니다(line 35에서 이미 이렇게 사용됨).필수 개선사항:
비즈니스 로직 검토:
현재 getPoint에서 포인트 레코드가 없으면 예외 발생(line 26-27). charge 메서드는 신규 유저 포인트를 자동 생성하므로, getPoint도 0으로 반환하거나 일관된 동작으로 정의하세요.
🤖 Prompt for AI Agents