Skip to content
Open
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
@@ -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);
}
Comment on lines +18 to +31
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

포인트 조회 로직의 비즈니스 요구사항을 명확히 해주세요.

현재 구현에서는 포인트 정보가 없으면 예외를 던지는데, 신규 유저의 경우 포인트 레코드가 아직 생성되지 않았을 수 있습니다. 이 경우 0 포인트를 반환하는 것이 더 적절할 수 있습니다.

또한 Line 23에서 new PointModel(user, 0)로 생성하는 부분은 설계상 어색합니다. PointService.findPoint가 실제로는 user만 필요로 하는데 PointModel을 파라미터로 받고 있습니다.

다음을 확인해주세요:

  1. 포인트 레코드가 없는 신규 유저의 경우 어떻게 처리해야 하는지 비즈니스 요구사항 확인
  2. PointService.findPoint(UserModel user) 시그니처로 리팩토링 검토


---

🏁 Script executed:

```shell
#!/bin/bash
# Find PointService class and findPoint method
find . -type f -name "*.java" | xargs grep -l "class PointService"

Length of output: 168


🏁 Script executed:

#!/bin/bash
# Search for findPoint method definition
rg -n "findPoint\s*\(" --type=java -A 3 -B 3

Length of output: 7228


🏁 Script executed:

#!/bin/bash
# Find PointModel class definition
find . -type f -name "*.java" | xargs grep -l "class PointModel"

Length of output: 153


🏁 Script executed:

#!/bin/bash
# Find all usages of findPoint
rg -n "\.findPoint\s*\(" --type=java -B 2 -A 2

Length of output: 3467


🏁 Script executed:

#!/bin/bash
# Check PointModel constructor
rg -n "PointModel\s*\(" --type=java -A 2

Length of output: 3712


PointService.findPoint 메서드의 API 설계를 개선하세요.

코드 검증 결과, PointService.findPoint가 PointModel을 받지만 실제로는 user만 사용합니다(line 21: point.getUser()). 반면 호출되는 PointRepository.findPoint는 UserModel을 직접 받습니다(line 35에서 이미 이렇게 사용됨).

필수 개선사항:

  • PointService.findPoint(PointModel point) → findPoint(UserModel user)로 리팩토링 필요
  • PointFacade 24번, 41번 줄에서 불필요한 new PointModel(user, 0) 제거
  • PointRepository와 일관된 인터페이스로 정렬

비즈니스 로직 검토:
현재 getPoint에서 포인트 레코드가 없으면 예외 발생(line 26-27). charge 메서드는 신규 유저 포인트를 자동 생성하므로, getPoint도 0으로 반환하거나 일관된 동작으로 정의하세요.

🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java
around lines 18–31, refactor to call pointService.findPoint with a UserModel
(not a PointModel) and remove the unnecessary new PointModel(user, 0) at lines
24 and 41; update PointService signature from findPoint(PointModel) to
findPoint(UserModel) and align PointRepository to accept UserModel as well, then
change getPoint so that when the service returns null it returns a PointInfo
representing zero balance for the user (e.g., create/return a PointInfo for user
with 0) instead of throwing NOT_FOUND, ensuring the service/repository
interfaces remain consistent.


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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Null 안전성 문제와 중복 코드를 개선해주세요.

Line 42에서 charged가 null인지 확인하지 않고 PointInfo.from(charged)를 호출하면 NPE가 발생할 수 있습니다. pointService.charge는 void를 반환하고 내부에서 저장을 처리하지만, 이후 조회 시 이론적으로 null이 반환될 수 있습니다.

또한 Lines 34-37의 사용자 검증 로직은 getPoint 메서드(Lines 19-22)와 중복됩니다.

다음과 같이 개선을 권장합니다:

+    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
In
apps/commerce-api/src/main/java/com/loopers/application/point/PointFacade.java
around lines 33 to 43, avoid duplicating user validation and prevent a possible
NPE: reuse the existing getPoint/getUser validation method instead of repeating
userService.getUser(...) so the same not-found handling is used, and after
charging, check that the returned charged PointModel from
pointService.findPoint(...) is not null; if it is null throw an appropriate
CoreException (e.g., ErrorType.NOT_FOUND with a clear message) before calling
PointInfo.from(charged).

}
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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

잔액 부족 검증 로직이 잘못되었습니다.

Line 52의 point < 0 검증은 차감 후 잔액이 음수가 되는지를 확인하지 못합니다. 현재 로직으로는 차감 전 잔액이 양수이기만 하면 차감 후 음수가 되어도 통과합니다.

다음 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
In apps/commerce-api/src/main/java/com/loopers/domain/point/PointModel.java
around lines 47 to 56, the insufficient-balance check currently tests point < 0
before deduction which misses cases where amount > point; change the validation
to either verify amount is not greater than current point (e.g., if (amount >
point) throw ...) or compute the resulting balance and ensure it is non-negative
(e.g., if (point - amount < 0) throw ...), keeping the negative-amount guard
as-is and then perform the subtraction only after the correct
insufficient-balance check.

}
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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

테이블 이름 충돌: user는 표준 SQL 예약어
Hibernate가 스키마를 생성할 때 user를 그대로 사용하면 H2·MySQL·PostgreSQL 등 주요 DB에서 예약어 충돌로 DDL 실행이 실패합니다. 애플리케이션 부팅/테스트가 바로 깨지므로 안전한 이름(예: users)으로 변경해 주세요.

-@Table(name = "user")
+@Table(name = "users")
🤖 Prompt for AI Agents
In apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java around
lines 9 to 45, the @Table(name = "user") uses the SQL reserved word "user" which
causes DDL failures on many DBs; change the table name to a safe identifier (for
example "users") by updating the @Table annotation, and then update any JPQL/SQL
queries, repository definitions, tests, and migration scripts that reference
"user" to the new name so schema generation and runtime queries remain
consistent.


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
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

구현 누락: findById가 항상 빈 Optional 반환
UserRepository.findById 계약을 어기고 있어 어느 계층에서 호출해도 무조건 미존재로 처리됩니다. 이 상태로는 ID 기반 조회 로직과 연쇄 검증이 전부 실패하므로 즉시 수정이 필요합니다. 아래처럼 JPA 리포지토리에 위임해 주세요.

     @Override
     public Optional<UserModel> findById(Long id) {
-        return Optional.empty();
+        return userJpaRepository.findById(id);
     }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java
around lines 21-23, the findById method currently always returns
Optional.empty(); replace this with a delegation to the JPA repository (e.g.,
call userJpaRepository.findById(id)) and map the returned User entity to the
UserModel before returning (wrap mapping result in Optional), ensuring null-safe
handling and preserving the contract of UserRepository.findById.


@Override
public UserModel save(UserModel userModel) {
return userJpaRepository.save(userModel);
}



}
Loading