[Feat] 동아리, 출퇴근 API 구현 #13
Conversation
Walkthrough새로운 동아리/출퇴근 REST API를 추가하고, 이를 지원하는 서비스·리포지토리·DTO를 도입했습니다. 클럽번호 조회, 내 동아리 목록/추가, 출근/퇴근, 재실자 조회 엔드포인트가 포함됩니다. Member의 setExist 시그니처가 변경되었고, ErrorCode에 동아리/출결 관련 코드가 추가되었습니다. 로컬 DB의 DDL 전략이 update로 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller
participant Service
participant UserRepo as UserRepository
participant ClubRepo as ClubRepository
participant MemberRepo as MemberRepository
rect rgb(240,248,255)
note right of Client: 출근 (POST /api/clubs/{clubId}/check-in)
Client->>Controller: checkIn(clubId)
Controller->>Service: checkIn(clubId)
Service->>UserRepo: current user lookup
UserRepo-->>Service: User
Service->>ClubRepo: findById(clubId)
ClubRepo-->>Service: Club
Service->>MemberRepo: findByUserAndClub(user, club)
MemberRepo-->>Service: Member (존재 여부/상태 확인)
alt already checked in
Service-->>Controller: throw ALREADY_CHECKED_IN
Controller-->>Client: BaseResponse error
else proceed
Service->>MemberRepo: save(member.setExist(true))
Service-->>Controller: success (void)
Controller-->>Client: BaseResponse.ok(null)
end
end
sequenceDiagram
autonumber
participant Client
participant Controller
participant Service
participant ClubRepo as ClubRepository
participant MemberRepo as MemberRepository
rect rgb(245,255,250)
note right of Client: 재실자 조회 (GET /api/clubs/{clubId}/presences)
Client->>Controller: getClubPresence(clubId)
Controller->>Service: getClubPresence(clubId)
Service->>ClubRepo: findById(clubId)
ClubRepo-->>Service: Club
Service->>MemberRepo: findByUserAndClub(currentUser, club) — membership 검증
MemberRepo-->>Service: Member
Service->>MemberRepo: findAllByClubAndIsExistTrue(club)
MemberRepo-->>Service: List<Member> (현재 출석자)
Service-->>Controller: ClubPresenceResponse(clubName, presentMembers)
Controller-->>Client: BaseResponse.ok(data)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/model/Member.java (1)
38-40: Lombok의@Setter와 커스텀 setter가 공존하고 있어요.클래스 레벨에
@Setter애노테이션이 있어서 Lombok이setIsExist(Boolean isExist)메서드를 자동 생성하는데, 여기서setExist(boolean isExist)라는 이름이 살짝 다른 커스텀 setter도 만들고 계세요. 이렇게 되면:
setIsExist()(Lombok 생성) vssetExist()(커스텀) 두 메서드가 같이 존재- 필드 타입은
Boolean인데 파라미터는boolean프리미티브 타입 사용혼란을 줄이려면 다음 중 하나를 선택하시면 좋을 것 같아요:
- 커스텀 setter를 유지하려면 클래스 레벨
@Setter를 제거하고 필요한 필드에만 개별적으로@Setter추가- 또는 Lombok 생성 메서드를 사용하고 이 커스텀 메서드는 제거
옵션 1: 클래스 레벨
@Setter제거하고 필요한 필드에만 적용@Entity @Table(name = "members") @NoArgsConstructor(access = AccessLevel.PROTECTED) -@Setter @Getter +@Getter public class Member extends BaseEntity { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name = "member_id", nullable = false) + @Setter private Long id; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "user_id", nullable = false) + @Setter private User user; @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "club_id", nullable = false) + @Setter private Club club; @Column(name = "is_exist") private Boolean isExist;옵션 2: 커스텀 setter 제거하고 Lombok 생성 메서드 활용
호출하는 쪽에서
setIsExist(boolean)사용:- public void setExist(boolean isExist){ - this.isExist = isExist; - }src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/model/Club.java (1)
10-10: 엔티티에 @Setter 사용 재검토 권장JPA 엔티티에 클래스 레벨
@Setter를 사용하면 모든 필드가 외부에서 직접 변경 가능해져서 도메인 로직 우회나 예상치 못한 상태 변경이 발생할 수 있습니다. 특히id나clubNumber같은 식별자 필드는 생성 후 변경되면 안 되는데, setter가 열려 있으면 실수로 변경될 위험이 있어요.필요한 필드에만 개별적으로 setter를 제공하거나, 비즈니스 의미를 담은 메서드(예:
updateName(String newName))로 대체하는 것을 고려해보세요.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/controller/ClubController.java (3)
20-30: 출퇴근 HTTP 메서드 선택 재검토 제안출근(
check-in)은 POST로, 퇴근(check-out)은 DELETE로 매핑되어 있는데, REST 관점에서 보면 둘 다 멤버 재실 상태를 업데이트하는 동작이므로 DELETE보다는 POST나 PATCH가 더 의미론적으로 적합합니다. DELETE는 보통 리소스 삭제를 의미하거든요.예를 들어:
POST /{clubId}/check-in(현재 그대로)POST /{clubId}/check-out(DELETE 대신 POST)또는 더 RESTful하게:
PATCH /{clubId}/attendancewith body{"present": true/false}현재 구조가 작동은 하지만, API 일관성 측면에서 고려해보시면 좋을 것 같아요.
38-42: clubNumber 쿼리 파라미터 검증 추가 권장
clubNumber파라미터에 대한 검증이 없어서 빈 문자열이나 공백 등이 그대로 서비스 레이어로 전달될 수 있습니다.
@RequestParam에@NotBlank같은 검증 애노테이션을 추가하고, 컨트롤러 클래스에@Validated를 붙이면 입력 검증을 자동화할 수 있어요.예시:
+import jakarta.validation.constraints.NotBlank; +import org.springframework.validation.annotation.Validated; +@Validated @RestController public class ClubController { @GetMapping - public BaseResponse<ClubResponse> getClubByClubNumber(@RequestParam String clubNumber) { + public BaseResponse<ClubResponse> getClubByClubNumber( + @RequestParam @NotBlank(message = "동아리 번호는 필수입니다") String clubNumber) {
32-36: 동아리 가입 엔드포인트 경로 명확화 제안
POST /{clubId}경로는 동아리 가입용으로 사용되고 있는데, RESTful 관점에서POST /clubs/{clubId}는 보통 해당 클럽 자체를 생성하는 것으로 오해될 수 있습니다. 향후 클럽 하위 리소스를 추가할 때 경로 충돌 가능성도 있고요.더 명확한 경로를 고려해보세요:
POST /{clubId}/members(멤버십 리소스 생성)POST /{clubId}/join(액션 기반 명명)현재 구조로도 동작하지만, API 확장성과 가독성 측면에서 개선 여지가 있습니다.
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/service/ClubService.java (4)
48-63: 퇴근 로직도 잘 구현되었습니다.checkIn과 대칭적인 구조로 깔끔하게 작성하셨네요. 동일하게 동시성 이슈는 checkIn과 같은 상황입니다.
선택사항: checkIn/checkOut에서 사용자·동아리·멤버십 검증 로직이 중복되는데, 별도 헬퍼 메서드로 추출하면 코드 중복을 줄일 수 있습니다. 다만 현재 메서드들이 충분히 짧아서 굳이 지금 리팩토링하지 않아도 괜찮습니다.
65-69: 현재 사용자 조회 로직을 확인해주세요.SecurityContext에서 인증 정보를 가져오는 방식은 적절합니다.
다만
getAuthentication().getName()이 null을 반환할 가능성이나, 인증이 없는 상태에서 호출될 경우를 고려해보세요. Spring Security 필터 체인이 올바르게 설정되어 있다면 문제없겠지만, 방어 코드가 있으면 더 안전합니다.또한 매 서비스 메서드 호출마다 DB 조회가 발생하는데, 요청 스코프 캐싱이나 SecurityContext에 User 객체를 직접 저장하는 방식도 고려해볼만 합니다.
99-115: 내 동아리 목록 조회 로직을 확인해주세요.전반적으로 깔끔하게 작성되었습니다.
다만 Line 107-108에서
member.getClub().getId()와.getName()을 호출할 때, Club이 지연 로딩되어 있다면 N+1 쿼리 문제가 발생할 수 있습니다. Member 엔티티의 Club 연관관계가 EAGER 페치이거나, MemberRepository에서join fetch쿼리를 사용하고 있는지 확인해주세요.멤버십이 많지 않다면 당장 문제되지 않겠지만, 추후 성능 개선 포인트로 고려해보세요.
#!/bin/bash # Description: Check Member entity fetch strategy for club relationship # Expected: Find @ManyToOne annotation with fetch type ast-grep --pattern $'@ManyToOne$$$ private Club club' rg -B 2 -A 1 "private Club club" --type java
117-137: 재실자 조회 로직 잘 작성하셨습니다.현재 사용자가 멤버인지 검증한 후에만 재실 정보를 반환하는 인가 로직이 좋습니다.
isMe플래그도 클라이언트에서 유용하게 쓰일 것 같네요.마찬가지로 Line 131-132에서
member.getUser()접근 시 User가 지연 로딩이면 N+1 쿼리가 발생할 수 있습니다.findAllByClubAndIsExistTrue에서join fetch user를 사용하거나 Member의 User 관계를 EAGER로 설정하는 것을 고려해보세요.#!/bin/bash # Description: Check MemberRepository for join fetch in findAllByClubAndIsExistTrue # Expected: @Query with join fetch or @EntityGraph rg "findAllByClubAndIsExistTrue" --type java -A 3 -B 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/controller/ClubController.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubPresenceResponse.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubResponse.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/MyClubsResponse.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/PresenceResponse.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/model/Club.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/repository/ClubRepository.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/service/ClubService.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/model/Member.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/repository/UserRepository.java(1 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/global/common/response/ErrorCode.java(1 hunks)src/main/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/PresenceResponse.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubPresenceResponse.java (1)
Getter(8-13)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/MyClubsResponse.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubResponse.java (1)
Getter(7-13)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubResponse.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/MyClubsResponse.java (1)
Getter(9-14)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubPresenceResponse.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/PresenceResponse.java (1)
Getter(7-14)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/service/ClubService.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/controller/ClubController.java (1)
Slf4j(12-55)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/controller/ClubController.java (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/service/ClubService.java (1)
Slf4j(23-138)
🔇 Additional comments (14)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/MyClubsResponse.java (1)
9-14: 깔끔한 DTO 구조네요!Lombok을 활용해서 보일러플레이트를 잘 줄였고, 불변 객체 패턴을 따르고 있어서 좋습니다. 코드가 간결하고 명확해요.
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/user/repository/UserRepository.java (1)
12-12: 닉네임 조회 메서드 추가 잘하셨어요!기존의
findByEmail패턴과 일관성 있게 구현되었고, Spring Data JPA 네이밍 컨벤션을 잘 따르고 있네요.Optional반환 타입도 적절합니다.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubResponse.java (1)
7-13: 기본적인 DTO 구조가 잘 잡혔습니다.동아리 정보를 나타내는 DTO로서 필요한 필드들을 포함하고 있고, Lombok을 통해 깔끔하게 구현되었어요.
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/repository/ClubRepository.java (1)
8-10: 리포지토리 구현 깔끔합니다!
findByClubNumber메서드가 Spring Data JPA 컨벤션을 잘 따르고 있고,Optional반환으로 null 안정성도 확보되었네요. Club 엔티티의 clubNumber 유니크 제약조건과도 잘 맞아떨어집니다.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/model/Club.java (1)
21-21: unique 제약 조건 추가 확인 완료
clubNumber필드에 unique 제약 조건을 추가한 것은ClubRepository.findByClubNumber()조회 로직과 잘 맞아떨어집니다. DB 레벨에서 중복을 방지하니 안전하네요.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/ClubPresenceResponse.java (1)
8-13: DTO 구조 깔끔합니다응답 DTO로서 구조가 명확하고, Lombok을 활용해 보일러플레이트도 최소화되어 있네요.
ClubService.getClubPresence()에서 스트림으로 리스트를 생성하므로 null 걱정도 없습니다.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java (1)
11-15: 리포지토리 메서드 선언 잘 되어 있습니다Spring Data JPA 네이밍 컨벤션을 정확히 따르고 있고,
ClubService에서 사용하는 패턴과도 잘 맞아떨어지네요. 특히findAllByClubAndIsExistTrue는 재실 여부를 필터링하는 목적이 명확합니다.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/dto/response/PresenceResponse.java (1)
7-14: DTO 구조와 JSON 매핑 적절합니다
@JsonProperty("isMe")로 boolean 필드를 명확한 JSON 키로 매핑한 점이 좋네요. API 응답에서isMe로 표현되어 클라이언트 입장에서도 이해하기 쉽습니다.src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/controller/ClubController.java (1)
12-18: 컨트롤러 구조와 의존성 주입 적절합니다Lombok
@RequiredArgsConstructor로 깔끔하게 의존성 주입이 되어 있고, 컨트롤러가 서비스 레이어에만 의존하는 구조가 좋네요. 로깅도 준비되어 있고요.src/main/java/com/WhoIsRoom/WhoIs_Server/global/common/response/ErrorCode.java (2)
23-23: 오타 수정 잘 하셨습니다."시용자" → "사용자" 수정 확인했습니다.
27-27: 내부 에러 코드 번호와 HTTP 상태 코드 간 의미 불일치를 확인해주세요.새로 추가된 에러 코드들의 첫 번째 파라미터(내부 코드 번호)가 HTTP 상태 코드 범위와 혼동될 수 있습니다:
MEMBER_NOT_FOUND(400, ...)- 400은 일반적으로 Bad Request 범위인데 FORBIDDEN(403)을 사용ALREADY_MEMBER(401, ...)- 401은 Unauthorized를 연상시키는데 BAD_REQUEST(400)를 사용ATTENDANCE_NOT_FOUND(500, ...)- 500은 서버 에러를 연상시키는데 BAD_REQUEST(400)를 사용ALREADY_CHECKED_IN(501, ...)- 501은 Not Implemented를 연상시키는데 BAD_REQUEST(400)를 사용또한
MEMBER_NOT_FOUND가NOT_FOUND대신FORBIDDEN을 사용하는 것이 의도된 것인지 확인이 필요합니다. 보안상 동아리 회원 여부를 숨기려는 의도라면 타당하지만, 메시지는 명시적으로 "회원이 아닙니다"라고 표현하고 있어서 일관성 검토가 필요합니다.Also applies to: 30-31, 34-35
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/service/ClubService.java (2)
31-46: 출근 로직은 잘 구현되었으나, 동시성 이슈를 검토해주세요.전반적인 로직 흐름은 좋습니다. null-safe 체크(
Boolean.TRUE.equals())도 적절하게 사용하셨네요.다만
isExist상태를 확인한 후 변경하는 사이에 동시 요청이 발생하면 중복 체크인이 가능할 수 있습니다. Member 엔티티에 낙관적 락이나 DB 레벨 제약이 없다면 동시성 문제가 발생할 수 있으니 확인해주세요.트래픽이 낮다면 당장 문제되지 않을 수 있지만, 추후 고려해보시면 좋을 것 같습니다.
91-97: 동아리 조회 로직 깔끔합니다.clubNumber로 조회하는 간단한 로직인데 명확하게 작성하셨네요.
src/main/resources/application.yml (1)
22-22:ddl-auto를update로 전환할 때 스키마 드리프트 위험을 점검해 주세요로컬 프로필에서
update를 사용하면 기존 데이터를 유지할 수 있는 대신, 컬럼/테이블 삭제·이름 변경·NOT NULL 추가처럼 파괴적인 변경은 자동으로 반영되지 않아, 실제 스키마와 엔티티 간 불일치가 남을 수 있습니다. 이는 로컬 환경에서 숨은 컬럼, 제약조건 미적용, 캐시 불일치와 같은 문제를 일으킬 수 있으므로, 개발자들이 주기적으로 DB를 초기화하거나 Flyway·Liquibase 등으로 마이그레이션을 관리하는 절차가 마련되어 있는지 확인해 주세요.(neon.tech)
| @Transactional | ||
| public void joinClub(Long clubId) { | ||
| User user = getCurrentUser(); | ||
|
|
||
| Club club = clubRepository.findById(clubId) | ||
| .orElseThrow(() -> new BusinessException(ErrorCode.CLUB_NOT_FOUND)); | ||
|
|
||
| memberRepository.findByUserAndClub(user, club).ifPresent(member -> { | ||
| throw new BusinessException(ErrorCode.ALREADY_MEMBER); | ||
| }); | ||
|
|
||
| Member member = Member.builder() | ||
| .user(user) | ||
| .club(club) | ||
| .isExist(false) | ||
| .build(); | ||
|
|
||
| memberRepository.save(member); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
동아리 가입 로직은 잘 구현되었으나, 중복 가입 방지를 확인해주세요.
ifPresent를 활용한 중복 체크는 깔끔합니다. 새 멤버의 isExist를 false로 초기화하는 것도 합리적이네요.
다만 멤버십 존재 여부를 확인(line 78)한 후 저장(line 88)하기 전에 동시 요청이 들어오면 중복 멤버십이 생성될 수 있습니다. Member 테이블에 (user_id, club_id) 유니크 제약이 있는지 확인해주세요. DB 레벨 제약이 있으면 예외가 발생할 테니 처리 로직 추가도 고려해보세요.
🏁 Script executed:
#!/bin/bash
# Description: Check if Member entity has unique constraint on user and club
# Expected: @Table annotation with uniqueConstraints or @UniqueConstraint on user+club
ast-grep --pattern $'@Entity
$$$
class Member {
$$$
}'
rg -A 10 "class Member" --type java | rg -C 5 "unique|Unique|UNIQUE"Length of output: 0
Member 엔티티의 유니크 제약 여부 확인을 위해 아래 스크립트를 실행해주세요.
🏁 Script executed:
#!/bin/bash
set -eux
# 1. Member.java 파일 경로 찾기
FILE=$(fd Member.java)
echo "Found Member entity at: $FILE"
# 2. 상단 어노테이션 확인 (200줄까지 출력)
sed -n '1,200p' "$FILE" | grep -C3 "@Table"Length of output: 566
Member 엔티티에 user_id+club_id 복합 유니크 제약 추가
현재 @Table(name = "members")에 (user_id, club_id) 복합 유니크 제약이 없어 동시 요청 시 중복 가입이 발생할 수 있습니다. JPA의 @Table(uniqueConstraints = …) 또는 DB 마이그레이션을 통해 해당 제약을 추가해주세요.
🤖 Prompt for AI Agents
In src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/service/ClubService.java
around lines 71-89: the Member entity/table currently lacks a (user_id, club_id)
unique constraint which allows duplicate memberships on concurrent requests; add
the composite unique constraint to the Member entity (e.g., annotate the Member
@Table with uniqueConstraints = @UniqueConstraint(columnNames =
{"user_id","club_id"})) and create a DB migration to apply the same unique index
at the database level; additionally, update transactional error handling so a
unique constraint violation
(DataIntegrityViolationException/ConstraintViolationException) is caught and
translated to BusinessException(ErrorCode.ALREADY_MEMBER) to maintain consistent
behavior.
…into feat/#11-club-api
…into feat/#11-club-api # Conflicts: # src/main/java/com/WhoIsRoom/WhoIs_Server/domain/club/repository/ClubRepository.java # src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java # src/main/java/com/WhoIsRoom/WhoIs_Server/global/common/response/ErrorCode.java
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java (1)
25-25: findByUserAndClub 메서드 검토 완료 — 성능을 위해 인덱스 추가 고려
메서드 시그니처와 Optional 반환은 적절합니다. Member 엔티티의 user_id, club_id 컬럼에 복합 인덱스가 정의되어 있지 않으므로, 자주 사용되는 쿼리라면 아래와 같이 @table에 복합 인덱스를 추가하세요.@Table( name = "members", indexes = @Index(name = "idx_member_user_club", columnList = "user_id,club_id") )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java(2 hunks)src/main/java/com/WhoIsRoom/WhoIs_Server/global/common/response/ErrorCode.java(1 hunks)src/main/resources/application.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/application.yml
🔇 Additional comments (5)
src/main/java/com/WhoIsRoom/WhoIs_Server/global/common/response/ErrorCode.java (2)
23-23: 오타 수정 잘 하셨습니다!"시용자" → "사용자" 오타 수정이 정확합니다.
27-27: 메시지 개선 좋습니다!더 구체적이고 명확한 메시지로 변경되었습니다.
src/main/java/com/WhoIsRoom/WhoIs_Server/domain/member/repository/MemberRepository.java (3)
3-3: 필요한 import가 올바르게 추가되었습니다.새로 추가된 메서드의 파라미터와 반환 타입에 필요한 임포트들이 적절히 추가되었습니다.
Also applies to: 5-5, 13-13
26-26: User의 모든 Member 관계를 조회하는 메서드가 추가되었습니다.메서드 시그니처는 올바르며 Spring Data JPA 규칙을 잘 따르고 있습니다. 다만 이 메서드로 조회한 Member 리스트에서 Club이나 다른 연관 엔티티에 접근할 경우 N+1 쿼리 문제가 발생할 수 있으니, 실제 사용 패턴을 보고 필요하다면
@EntityGraph나JOIN FETCH를 고려해보세요.
27-27: 재실 중인 멤버를 조회하는 메서드가 적절히 구현되었습니다.
IsExistTrue조건을 통해 출근 상태의 멤버만 필터링하는 로직이 잘 반영되어 있습니다. 앞의 메서드와 마찬가지로 User 등의 연관 엔티티 접근 시 N+1 쿼리 이슈가 있을 수 있으니 주의하시고, 재실자 조회가 빈번하다면 (club_id, is_exist) 복합 인덱스를 고려해보세요.
| // Member | ||
| MEMBER_NOT_FOUND(400, HttpStatus.FORBIDDEN.value(), "해당 동아리의 회원이 아닙니다."), | ||
| ALREADY_MEMBER(401, HttpStatus.BAD_REQUEST.value(), "이미 동아리에 가입된 사용자입니다."), |
There was a problem hiding this comment.
HTTP 상태 코드 불일치 및 시맨틱 문제가 있습니다.
다음 이슈들을 확인해주세요:
-
MEMBER_NOT_FOUND (Line 30): 내부 code는 400이지만 HTTP 상태는 FORBIDDEN(403)을 사용하고 있습니다. 동아리 회원이 아닌 경우는 권한 문제(403)보다는 NOT_FOUND(404) 또는 BAD_REQUEST(400)가 더 적절해 보입니다.
-
ALREADY_MEMBER (Line 31): 내부 code는 401이지만 HTTP 상태는 BAD_REQUEST(400)를 사용하고 있습니다. 일반적으로 401은 인증(authentication) 관련 상태 코드로 예약되어 있습니다.
일관성을 위해 다음과 같이 수정하는 것을 권장합니다:
// Member
-MEMBER_NOT_FOUND(400, HttpStatus.FORBIDDEN.value(), "해당 동아리의 회원이 아닙니다."),
-ALREADY_MEMBER(401, HttpStatus.BAD_REQUEST.value(), "이미 동아리에 가입된 사용자입니다."),
+MEMBER_NOT_FOUND(400, HttpStatus.NOT_FOUND.value(), "해당 동아리의 회원이 아닙니다."),
+ALREADY_MEMBER(401, HttpStatus.CONFLICT.value(), "이미 동아리에 가입된 사용자입니다."),또는 BAD_REQUEST로 통일:
// Member
-MEMBER_NOT_FOUND(400, HttpStatus.FORBIDDEN.value(), "해당 동아리의 회원이 아닙니다."),
+MEMBER_NOT_FOUND(400, HttpStatus.BAD_REQUEST.value(), "해당 동아리의 회원이 아닙니다."),🤖 Prompt for AI Agents
In
src/main/java/com/WhoIsRoom/WhoIs_Server/global/common/response/ErrorCode.java
around lines 29-31, the numeric internal codes and HttpStatus values are
inconsistent and semantically incorrect: MEMBER_NOT_FOUND uses internal code 400
but HttpStatus.FORBIDDEN (403) — change both to a matching NOT_FOUND/404 (or at
least make internal code 404 and HttpStatus.NOT_FOUND) so the error represents a
missing member; ALREADY_MEMBER uses internal code 401 while
HttpStatus.BAD_REQUEST (400) — change both to BAD_REQUEST/400 (or at minimum set
internal code to 400 and keep HttpStatus.BAD_REQUEST) because 401 is reserved
for authentication; update the enum entries so internal code and
HttpStatus.value() match and reflect the recommended semantics.
| // Attendance | ||
| ATTENDANCE_NOT_FOUND(500, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."), | ||
| ALREADY_CHECKED_IN(501, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."), |
There was a problem hiding this comment.
내부 에러 코드 범위 선택에 주의가 필요합니다.
Attendance 관련 에러 코드로 500번대를 사용하고 있는데, 이는 일반적으로 서버 에러를 의미하는 HTTP 5xx 상태 코드와 혼동될 수 있습니다.
ATTENDANCE_NOT_FOUND(500, ...): 출근 기록이 없는 것은 클라이언트 측 문제(BAD_REQUEST)인데 내부 코드가 500입니다.ALREADY_CHECKED_IN(501, ...): 이미 출근 중인 것도 클라이언트 측 문제인데 내부 코드가 501입니다.
모니터링 시스템이나 로그 분석 시 5xx 코드는 서버 에러로 간주되어 혼란을 줄 수 있습니다. 클라이언트 에러는 4xx 범위의 내부 코드를 사용하는 것을 권장합니다:
// Attendance
-ATTENDANCE_NOT_FOUND(500, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."),
-ALREADY_CHECKED_IN(501, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."),
+ATTENDANCE_NOT_FOUND(500, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."),
+ALREADY_CHECKED_IN(501, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."),또는 더 명확하게 4xx 범위로 변경:
// Attendance
-ATTENDANCE_NOT_FOUND(500, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."),
-ALREADY_CHECKED_IN(501, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."),
+ATTENDANCE_NOT_FOUND(450, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."),
+ALREADY_CHECKED_IN(451, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Attendance | |
| ATTENDANCE_NOT_FOUND(500, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."), | |
| ALREADY_CHECKED_IN(501, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."), | |
| // Attendance | |
| - ATTENDANCE_NOT_FOUND(500, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."), | |
| ATTENDANCE_NOT_FOUND(450, HttpStatus.BAD_REQUEST.value(), "출근 기록이 없습니다."), | |
| ALREADY_CHECKED_IN(451, HttpStatus.BAD_REQUEST.value(), "이미 출근 중입니다."), |
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit