[FEAT] 회원 부서 관리 기능을 추가합니다. #279
Conversation
- 사용자의 ID 및 Role로 AuthorityModel을 가져온다.
- 인자로 받은 role 이 존재하는지 검증
- 전달받은 role이 존재하는지 검증하는 로직 추가
- ROLE_USER , ROLE_DPT_NONE
- Department로 이전
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java (1)
92-109: PUT /api/members/{memberId}/department 보안 매칭 수정 필요
SecurityFilterChainConfig.java에서
requests.requestMatchers(HttpMethod.PUT, "/api/members/department").hasAnyRole(ADMIN);으로 설정되어 있으나, 실제 컨트롤러 매핑은 MemberController.java 76행의
@PutMapping("/{memberId}/department")이므로 ADMIN 권한 검사가 전혀 적용되지 않습니다.
아래와 같이 보안 필터를 실제 경로와 일치하도록 수정하세요.
- requests.requestMatchers(HttpMethod.PUT, "/api/members/department").hasAnyRole(ADMIN); + requests + .requestMatchers(HttpMethod.PUT, "/api/members/{memberId}/department") + .hasAnyRole(ADMIN);부가 검토
• 코드 상에 부서 리스트 조회용 GET 매핑(@GetMapping.*department)이 존재하지 않으므로, 만약 별도 조회 엔드포인트가 있을 경우 동일하게 권한 제한이 필요한지 확인 바랍니다.
•hasAnyRole("ADMIN")는 내부적으로ROLE_접두사를 붙여 검사하므로Role.ROLE_ADMIN.getRole()설계와 호환됩니다.
🧹 Nitpick comments (21)
eeos/src/main/java/com/blackcompany/eeos/member/application/dto/QueryMemberResponse.java (1)
17-17: DTO에 department 필드 추가는 타당. 응답 스키마/표준화(코드 vs 한글명) 의도 확인 필요현재 String 한글명으로만 노출되는 것으로 보입니다. 클라이언트가 안정적으로 필터링/정렬할 수 있도록 departmentCode(예: "PLATFORM") 같은 기계가독 코드와 표시용 koName을 함께 내려줄지 결정해주세요. 한글명이 바뀌어도 API 계약이 안정적입니다.
필요 시, Converter에서
source.getDepartment().name()를 code로,getKoName()을 label로 내려주는 형태로 확장 제안 가능합니다.eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java (3)
12-18: 필드/생성자 설계 간결. 단,role이름이 enum 상수와 혼동 가능
role필드는 의미상 코드 또는 키 값입니다. 혼동 방지를 위해code나nameKey같은 이름을 고려해도 좋습니다(선택).
31-36: findById: 성능은 충분하나, 예외 메시지와 캐시 고려역할 수가 적어도, 반복 조회 시 매번
values()스트림은 비용이 누적될 수 있습니다. 맵 캐시를 두거나 최소한 예외 메시지를 보강하세요.@@ - public static Role findById(Long id) { - return Arrays.stream(Role.values()) - .filter(obj -> obj.getId().equals(id)) - .findFirst() - .orElseThrow(IllegalArgumentException::new); - } + public static Role findById(Long id) { + return Arrays.stream(Role.values()) + .filter(obj -> obj.getId().equals(id)) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("Unknown role id: " + id)); + }추가로, 필요 시 정적 맵 캐시(
Map<Long, Role>)를 도입하면 O(1) 조회가 가능합니다.
38-40: getAllRoles: 불변 리스트로 방어적 반환 권장현재
Arrays.asList(values())는 크기 변경만 금지됩니다. 외부에서 안전하게 다루도록 불변화가 낫습니다.@@ - public static List<Role> getAllRoles() { - return Arrays.asList(Role.values()); - } + public static List<Role> getAllRoles() { + return List.of(Role.values()); + }eeos/src/main/java/com/blackcompany/eeos/auth/application/repository/AuthorityRepository.java (1)
11-12: 메서드 시그니처의 의미와 예외 정책을 인터페이스에서 명확히
findByIdAndRole(Long memberId, Role role)이 반환 타입이AuthorityModel(non-Optional)인 반면, 미존재 시 구현체에서 예외를 던지는 계약입니다. "find" 접두사는 보통 Optional을 연상시켜 혼동 여지가 있습니다.다음 중 하나로 일관성 정비를 권장합니다:
- 반환 타입을
Optional<AuthorityModel>로 변경하고 호출부에서 처리.- 메서드명을
getByMemberIdAndRole또는findByIdAndRoleOrThrow처럼 명시적으로.
또는 Javadoc 주석으로 미존재 시 예외 발생을 계약에 명시하세요.eeos/src/main/java/com/blackcompany/eeos/auth/application/service/AuthService.java (1)
72-74: 기본 권한 부여 로직 분리 👍 — 중복 생성 방지/제약 보강 고려헬퍼로 분리한 것은 가독성과 재사용 측면에서 좋습니다. 추가로:
- 동일 멤버에 ROLE_USER가 이미 존재하는 경우의 중복 처리(고유 제약 위반) 방어.
- 예외 시 롤백이 전체 회원가입 트랜잭션을 일관되게 되돌리는지 확인.
중복 방지를 위해 저장 전 존재 여부 확인 또는 DB Unique(Index)를 권장합니다(예: member_id + role 고유 제약).
eeos/src/main/java/com/blackcompany/eeos/member/application/dto/converter/QueryMemberResponseConverter.java (1)
17-17: NPE 가능성 및 표현 일관성: department가 null이면 NPE 발생 가능현재
source.getDepartment().getKoName()호출은department가 null일 경우 NPE를 유발할 수 있습니다. 모델/엔티티에서 기본값을NONE으로 두셨더라도 이행(마이그레이션) 전 데이터나 외부 주입 경로에서 null이 들어올 수 있어 방어 코드를 권장합니다. 또한 API 응답에서 사람이 읽는 값(koName)만 내려줄지, 코드(enName)까지 함께 내려줄지 명확히 정리 필요합니다. 응답 스키마가 확정되지 않았다면, 최소한의 방어 로직만 추가하는 쪽으로 제안드립니다.다음과 같이 null-safe 하게 매핑하는 것을 제안드립니다.
- .department(source.getDepartment().getKoName()) + .department( + (source.getDepartment() != null ? source.getDepartment() + : Department.NONE).getKoName() + )해당 변경에는
Departmentimport가 필요합니다(선택적):// 파일 상단 import 구역에 추가 import com.blackcompany.eeos.member.application.model.Department;응답 스키마가
koName단일 필드로 확정된 것이 맞는지 한 번 더 확인 부탁드립니다. 만약 클라이언트에서 코드값도 필요하다면departmentCode(enName)와departmentName(koName) 이원화가 안전합니다.eeos/src/main/java/com/blackcompany/eeos/member/application/usecase/DepartmentUsecase.java (1)
8-10: Usecase 시그니처 의미 명확화(문자열 대신 타입, 파라미터명 구체화)
changeDepartment(Long memberId, String to)는 문자열 파싱 책임을 애플리케이션 계층으로 끌어올려 타입 안정성이 떨어집니다. 유스케이스에는 도메인 타입을 받도록 하거나(권장), 최소한 파라미터명을targetDepartment로 명확히 해 주세요.다음 두 가지 중 하나를 권장합니다(1안 권장):
- 도메인 타입 사용(권장)
- void changeDepartment(Long memberId, String to); + void changeDepartment(Long memberId, Department targetDepartment);
- 파라미터명 명확화(문자열 유지 시)
- void changeDepartment(Long memberId, String to); + void changeDepartment(Long memberId, String targetDepartment);컨트롤러/서비스 구현부와 테스트에서 호출부 시그니처도 함께 업데이트되어야 합니다. 예외(
NotFoundDepartmentException) 발생 조건도 javadoc에 명시하면 사용성이 좋아집니다.eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberEntity.java (1)
61-64: DB 마이그레이션 확인 필요: NOT NULL enum 컬럼 추가 시 기존 데이터 안전성
@Column(nullable = false)인member_department를 새로 추가하셨습니다. JPA의@Builder.Default는 자바 객체 생성 시에만 적용되며 DB 기본값이 아닙니다. 운영 DB에 기존 레코드가 있다면, 마이그레이션에서 기본값(NONE)으로 백필 후NOT NULL제약을 거는 순서를 반드시 보장해야 합니다. 그렇지 않으면 스키마 적용/애플리케이션 구동 시점에 실패할 수 있습니다.
- Flyway/Liquibase 스크립트에 다음 절차가 포함되어 있는지 확인 부탁드립니다.
- 컬럼 추가(초기엔 NULL 허용)
- 기존 행을
NONE으로 백필- 컬럼에
NOT NULL제약 추가- 필요 시 인덱싱(부서 기반 조회가 빈번하다면
member_department인덱스 고려)원하시면 마이그레이션 스크립트 초안까지 드리겠습니다.
eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityJpaRepository.java (1)
15-17: 메서드명 오해 소지:findByIdAndRole→findByMemberIdAndRole로 변경 권장현재 JPQL은
a.memberId를 기준으로 검색하지만 메서드명은Id가 엔티티 PK로 읽힐 여지가 있습니다. Spring Data 파생 메서드 네이밍으로 명확성과 일관성을 확보하는 편이 좋습니다. 커스텀@Query도 불필요합니다.다음과 같이 정리하면 가독성과 유지보수성이 좋아집니다.
- @Query("SELECT a FROM AuthorityEntity a WHERE a.memberId=:memberId AND a.role=:role") - Optional<AuthorityEntity> findByIdAndRole( - @Param("memberId") Long memberId, @Param("role") Role role); + Optional<AuthorityEntity> findByMemberIdAndRole(Long memberId, Role role);eeos/src/main/java/com/blackcompany/eeos/auth/application/exception/NotFoundAuthorityException.java (1)
14-17: 메시지 하드코딩 제거 및 i18n 전략 일원화 필요여러
BusinessException하위 클래스에서getMessage()를 오버라이드하여 한글 메시지를 직접 반환하고 있는 것을 확인했습니다.
이 방식을 유지하면
- 예외 메시지가 코드에 분산되어 관리가 어렵고
- 다국어 지원(i18n)이나 메시지 변경 시 일괄 반영이 불가능합니다.
따라서 각 예외 클래스에서는
FAIL_CODE(예:"4013")와HttpStatus만 전달하고, 글로벌 핸들러(GlobalExceptionHandler)에서 메시지 소스(예:MessageSource)를 통해MessageCode또는 코드-메시지 매핑을 일원화해 주세요.– 적용 대상 예시
•eeos/src/main/java/com/blackcompany/eeos/auth/application/exception/NotFoundAuthorityException.java
•eeos/src/main/java/com/blackcompany/eeos/teamBuilding/application/exception/NotFoundResultException.java
•eeos/src/main/java/com/blackcompany/eeos/team/application/exception/NotFoundTeamException.java
•eeos/src/main/java/com/blackcompany/eeos/program/application/exception/NotFoundProgramException.java
… 외 다수 (extends BusinessException+getMessage()오버라이드 패턴)– 작업 제안
- 각 예외 클래스에서
getMessage()오버라이드 제거- 메시지 리소스(
.properties등)에 에러 코드별 메시지 등록- 글로벌 핸들러에서
e.getCode()기준으로 메시지 조회- 기존 하드코딩 메시지는 리소스에 반영 후 삭제
이렇게 하면 예외 메시지 관리가 중앙화되고, 추후 다국어 또는 메시지 수정 시 유지보수가 훨씬 용이해집니다.
eeos/src/main/java/com/blackcompany/eeos/member/application/dto/DepartmentResponse.java (1)
6-12: DepartmentResponse null 안전성 및 필드 명명 확인
Null 안전성
•DepartmentResponse.from(Department department)에null이 입력되면department.getId()호출 시 NPE가 바로 발생합니다.
• 프로젝트 전반의 팩토리 메소드(다른*Response.from(...))에서는 명시적 null 검사를 도입하고 있지 않으므로, 필요에 따라 아래처럼Objects.requireNonNull추가를 검토하세요.필드 명명 컨벤션
• 다른 Response DTO들( 예:AttendPenaltyResponse,AttendSummaryInfoResponse등)에서는 ID 필드를memberId,programId처럼EntityName + Id형태로 사용하고 있습니다.
•DepartmentResponse의departmentId는 이와 일치하며,enName/koName도 도메인 모델(getEnName()/getKoName())과 동일한 명명입니다.
• 별도의 로컬라이제이션 필드가 없는 타 DTO와 차이가 있지만, 도메인 요구 사항에 부합하므로 현재 명명으로도 무리가 없어 보입니다.Jackson 네이밍 전략
• 전역snake_case등 Jackson 설정을 사용 중이라면,en_name·ko_name매핑 여부를application.properties또는application.yml의spring.jackson.property-naming-strategy설정에서 재확인해 주세요.검토 대상:
- eeos/src/main/java/com/blackcompany/eeos/member/application/dto/DepartmentResponse.java
선택적 코드 예시(diff):
+ import java.util.Objects; ... public static DepartmentResponse from(Department department) { + Objects.requireNonNull(department, "department must not be null"); return new DepartmentResponse( department.getId(), department.getEnName(), department.getKoName() ); }eeos/src/main/java/com/blackcompany/eeos/auth/application/model/AuthorityModel.java (3)
21-23:updateRole에 null 방지 가드 추가 권장
null입력을 그대로 반영하면 이후 로직에서 NPE 가능성이 큽니다. 간단한 입력 검증을 추가해 주세요.- public void updateRole(Role role) { - this.role = role; - } + public void updateRole(Role role) { + if (role == null) { + throw new IllegalArgumentException("role must not be null"); + } + this.role = role; + }
15-19: 명시 생성자와 @builder의 이중 경로 관리 비용클래스 레벨
@Builder와 공개 전역 생성자를 동시에 두면 필드 추가 시 두 경로를 모두 유지해야 합니다. 생성자 접근 제어자를protected로 낮추고 빌더/정적 팩토리 중심으로 생성 경로를 일원화하는 방안을 제안합니다.
13-23: AuthorityModel의 불변성 유지 및 역할 변경 처리 방식 검토
- AuthorityModel은 equals/hashCode를 기본 Object 구현을 사용하므로, 현재 Set 사용 시 컬렉션 해시·동치성 문제는 발생하지 않습니다.
- 다만 mutable
role필드(updateRole메서드)로 인해
- 동시성 환경에서 예기치 못한 상태 변경이 발생할 수 있고,
- 향후
equals/hashCode를 오버라이드할 경우 컬렉션 동치성 이슈가 생길 여지가 있습니다.- 가급적 도메인 모델은 불변(immutable)으로 유지하고, “역할 변경”이 필요할 땐 새로운 인스턴스를 생성하거나 도메인 서비스를 통해 처리하는 패턴을 고려해 주세요.
eeos/src/main/java/com/blackcompany/eeos/member/presentation/docs/MemberApi.java (1)
49-50: 부서 리스트 응답 타입 도입 LGTM
List<DepartmentResponse>로 문서화된 점 좋습니다. 목록 정렬 기준(예: id 오름차순)을 스펙에 명시하면 클라이언트 UX가 더 안정적입니다.eeos/src/main/java/com/blackcompany/eeos/member/application/model/MemberModel.java (1)
34-37: null 방지 가드 추가 제안(updateDepartment)
updateDepartment가null을 허용하면 상태가 불안정해질 수 있습니다. 호출 실수 방지를 위해 널 가드를 권장합니다.+import java.util.Objects; ... public MemberModel updateDepartment(Department department) { - this.department = department; + this.department = Objects.requireNonNull(department, "department must not be null"); return this; }eeos/src/main/java/com/blackcompany/eeos/member/presentation/controller/MemberController.java (2)
83-89: 부서 목록 정렬 안정화현재 enum 선언 순서대로 반환됩니다. 응답 순서를 안정적으로 보장하려면 정렬을 명시하세요(예: id 오름차순).
+import java.util.Comparator; ... @GetMapping("/departments") public ApiResponse<SuccessBody<List<DepartmentResponse>>> getDepartments() { - List<Department> departments = departmentUsecase.getAllDepartments(); + List<Department> departments = departmentUsecase.getAllDepartments().stream() + .sorted(Comparator.comparing(Department::getId)) + .toList(); List<DepartmentResponse> responses = departments.stream().map(DepartmentResponse::from).toList(); return ApiResponseGenerator.success(responses, HttpStatus.OK, MessageCode.GET); }
76-81: 엔드포인트 파라미터 정교화 및 예외 매핑 확인
관리자 전용 API 일관성 및 가독성 향상을 위해 아래와 같이 선택적 리팩터링을 제안합니다.
@Member Long adminMemberId추가로 호출자(관리자) 식별 및 감사/로그 일관성 확보@RequestParam("to")→@RequestParam("department")파라미터명 명확화@PutMapping("/{memberId}/department") public ApiResponse<SuccessBody<Void>> changeDepartment( - @PathVariable("memberId") Long memberId, - @RequestParam("to") String to) { - departmentUsecase.changeDepartment(memberId, to); + @Member Long adminMemberId, + @PathVariable("memberId") Long memberId, + @RequestParam("department") String department) { + departmentUsecase.changeDepartment(memberId, department); return ApiResponseGenerator.success(HttpStatus.OK, MessageCode.UPDATE); }추가로,
NotFoundDepartmentException은BusinessException을 상속하며 생성자에서HttpStatus.NOT_FOUND를 설정하도록 되어 있어,GlobalExceptionHandler.handleConflict에서 HTTP 404 상태로 매핑됩니다.eeos/src/main/java/com/blackcompany/eeos/member/application/service/DepartmentService.java (1)
25-35: 패키지 프라이빗으로 가시성 낮추기 제안실행하신 검색 결과,
getDepartment(String)및getDepartmentById(Long)메서드는 선언부 외에 코드베이스 내에 호출 처리가 전혀 없습니다. 불필요한 공개 API를 축소하면 결합도를 낮추고 유지 보수성을 높일 수 있습니다.
- DepartmentService.java
public Department getDepartment(String name)→Department getDepartment(String name)public Department getDepartmentById(Long id)→Department getDepartmentById(Long id)컨트롤러나 다른 서비스에서 실제 사용되는 건
getAllDepartments()뿐이므로, 사용처가 없으면 위 두 메서드의 가시성을package-private으로 낮추거나 제거를 검토해주세요.eeos/src/main/java/com/blackcompany/eeos/member/application/model/Department.java (1)
49-51: 반환 리스트 불변성 및 정렬 고려
Arrays.asList(values())는 크기 고정이나 요소 변경은 가능하여, 외부에서 실수로 요소 값을 바꿀 위험(참조형이면)이 있습니다. 불변 리스트로 반환하거나, 정렬 기준을 명시해주면 더 안전/안정적입니다.-public static List<Department> getAllDepartments() { - return Arrays.asList(Department.values()); -} +public static List<Department> getAllDepartments() { + // Java 9+: 불변 리스트 + return List.of(Department.values()); + // 또는 Java 8 호환: + // return Collections.unmodifiableList(Arrays.asList(Department.values())); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (20)
eeos/src/main/java/com/blackcompany/eeos/auth/application/exception/NotFoundAuthorityException.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/application/model/AuthorityModel.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/application/repository/AuthorityRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/application/service/AuthService.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityJpaRepository.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityRepositoryImpl.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/dto/DepartmentResponse.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/dto/QueryMemberResponse.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/dto/converter/QueryMemberResponseConverter.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/exception/NotFoundDepartmentException.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/model/Department.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/model/MemberModel.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/model/converter/MemberEntityConverter.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/service/DepartmentService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/usecase/DepartmentUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberEntity.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/member/presentation/controller/MemberController.java(3 hunks)eeos/src/main/java/com/blackcompany/eeos/member/presentation/docs/MemberApi.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-20T14:09:51.877Z
Learnt from: Daae-Kim
PR: JNU-econovation/EEOS-BE#256
File: eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java:93-106
Timestamp: 2025-07-20T14:09:51.877Z
Learning: EEOS-BE 프로젝트에서 Spring Security 도입 시 JWT 토큰의 역할 정보는 DB에 "ADMIN", "USER" 형태로 저장되지만, Spring Security의 hasAnyRole() 메서드는 "ROLE_" 접두사가 붙은 권한을 기대하므로, JWT 생성 시점에 SimpleGrantedAuthority("ROLE_" + role) 형태로 접두사를 추가해야 권한 매칭이 올바르게 작동합니다.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java
🔇 Additional comments (8)
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java (1)
9-11: 역할 상수 정의 OK. 식별자 부여로 이관/마이그레이션 용이
id를 부여해두어 DB 매핑과 외부 연동 시 안정적입니다. 다만 고정 ID는 문서화가 필수입니다(운영 DB 시드와의 정합성).운영/스테이징 DB에서 Role 테이블(또는 시드)의 ID가 1=ADMIN, 2=USER로 합의돼 있는지 확인 부탁드립니다.
eeos/src/main/java/com/blackcompany/eeos/member/application/exception/NotFoundDepartmentException.java (1)
6-18: 예외 설계 적절 및 코드값 중복 없음 확인됨 🎉
NotFoundDepartmentException클래스에서만FAIL_CODE("3003")가 정의되어 있어 중복 충돌 우려가 없습니다.- 패턴은 명확하며 404 처리도 잘 연결되어 있습니다.
- 다만, 한글 메시지와 코드 관리 방식(하드코딩 vs. 공통 메시지 소스/코드 테이블) 은 팀 컨벤션에 맞춰 일관되게 결정해 주세요.
eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityRepositoryImpl.java (1)
38-47: 매핑 로직은 명확합니다
toEntity/toModel빌더 매핑이 간결하고 읽기 좋습니다. 수정 없이 유지해도 좋겠습니다.eeos/src/main/java/com/blackcompany/eeos/auth/application/exception/NotFoundAuthorityException.java (1)
8-12: 에러 코드(4013) 중복 없음 확인
eeos/src/main/java 경로에서"4013"를 검색한 결과, 해당 코드가 NotFoundAuthorityException.java에만 정의되어 있어 다른 예외와 충돌하지 않음을 확인했습니다. 추가 검토나 수정은 필요하지 않습니다.eeos/src/main/java/com/blackcompany/eeos/member/application/dto/DepartmentResponse.java (1)
9-12: DTO 매핑은 깔끔합니다
Department→DepartmentResponse매핑이 단순·명확합니다. 컨트롤러에서map(DepartmentResponse::from)로 쓰기 좋습니다.eeos/src/main/java/com/blackcompany/eeos/member/application/model/converter/MemberEntityConverter.java (2)
21-22: 엔티티↔모델 간department양방향 매핑 추가 좋습니다추가된
department필드의 상호 변환이 대칭적입니다. 다른 필드들과 일관되고, 컨버터 책임이 명확합니다.Also applies to: 33-34
37-39: 확인 완료:department필드에 기본값이 설정되어 있습니다
MemberEntity클래스의department필드는 Lombok의@Builder.Default애노테이션을 통해 기본값인Department.NONE으로 초기화되어 있습니다.
따라서toEntity(String name, OauthServerType oauthServerType)메서드에서department를 명시적으로 설정하지 않더라도, 빌더 호출 시 자동으로Department.NONE이 할당되어 DB의nullable = false제약을 충족합니다.별도 조치가 필요하지 않습니다.
eeos/src/main/java/com/blackcompany/eeos/member/application/model/MemberModel.java (1)
25-25: 기본값 NONE 도입 적절부서 필드를 추가하면서 기본값을
Department.NONE으로 둔 선택은 NPE를 줄이고 도메인 의도를 드러냅니다. 좋습니다.
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/auth/application/service/AuthService.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityJpaRepository.java
Show resolved
Hide resolved
.../src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityRepositoryImpl.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/member/application/model/Department.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/member/application/service/DepartmentService.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/member/presentation/docs/MemberApi.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java (2)
25-33: findRole: 입력 정규화 및 예외 메시지 보강 필요현재 "ROLE_ADMIN" 입력이 실패하며, 최종 orElseThrow가 빈 IllegalArgumentException을 던져 디버깅이 어렵습니다. 이전 리뷰 코멘트와 동일한 개선 제안입니다.
다음과 같이 수정하세요.
- public static Role findRole(String role) { - if (role == null || role.trim().isEmpty()) { - throw new IllegalArgumentException("해당 role 을 찾을 수 없습니다."); - } - return Arrays.stream(Role.values()) - .filter(obj -> obj.getRole().equalsIgnoreCase(role)) - .findFirst() - .orElseThrow(IllegalArgumentException::new); - } + public static Role findRole(String role) { + String normalized = normalizeRole(role); + if (normalized == null || normalized.isEmpty()) { + throw new IllegalArgumentException("해당 role 을 찾을 수 없습니다. 입력값이 비어있습니다."); + } + return Arrays.stream(Role.values()) + .filter(obj -> obj.getRole().equalsIgnoreCase(normalized)) + .findFirst() + .orElseThrow(() -> new IllegalArgumentException("알 수 없는 role: " + role)); + }
20-23: 'ROLE_' 접두사/대소문자/공백 정규화 누락으로 "ROLE_ADMIN" 입력 매칭 실패 가능외부 입력이 "ROLE_ADMIN", "role_user" 등으로 들어올 수 있는데 현재는 equalsIgnoreCase(role)만 수행하여 접두사 제거를 하지 않습니다. 보안 경계(권한 확인)에서 오탐·미탐을 유발할 수 있습니다. 이전 리뷰에서도 동일 지적이 있었습니다.
아래처럼 normalizeRole을 도입하고 이를 사용해 주세요.
- public static boolean isExist(String role) { - if (role == null || role.trim().isEmpty()) return false; - return Arrays.stream(Role.values()).anyMatch(obj -> obj.getRole().equalsIgnoreCase(role)); - } + public static boolean isExist(String role) { + String normalized = normalizeRole(role); + if (normalized == null || normalized.isEmpty()) return false; + return Arrays.stream(Role.values()) + .anyMatch(obj -> obj.getRole().equalsIgnoreCase(normalized)); + }정규화 유틸(아래)은 enum 내부에 추가해 주세요:
private static String normalizeRole(String role) { if (role == null) return null; String r = role.trim(); if (r.regionMatches(true, 0, "ROLE_", 0, 5)) { r = r.substring(5); } return r; }
🧹 Nitpick comments (2)
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java (2)
9-18: Spring Security 연동 보조 API 추가 제안(asGrantedAuthority)권한 부여 시 Spring Security는 "ROLE_" 접두사가 포함된 권한명을 기대합니다. 현재 getRole()은 "ADMIN"/"USER"를 반환하므로, GrantedAuthority 이름으로는 enum name()을 사용하는 헬퍼를 제공하면 실수를 줄일 수 있습니다. (이 제안은 과거 러닝을 반영했습니다: JWT 생성 시 "ROLE_" 접두사 필요)
러닝 참고: SecurityFilterChain에서 hasAnyRole 사용 시 접두사 필요.
추가 메서드 예시:
public String asGrantedAuthority() { return this.name(); // "ROLE_ADMIN", "ROLE_USER" }토큰 생성/파싱부가 위 메서드를 사용하거나 "ROLE_" 접두사를 부여하는지 확인해 주세요.
42-44: getAllRoles 불변 리스트 반환 제안외부에서 set을 통해 요소 교체가 가능한 Arrays.asList 대신 불변 리스트를 권장합니다. 영향도 낮은 미세 개선입니다.
- public static List<Role> getAllRoles() { - return Arrays.asList(Role.values()); - } + public static List<Role> getAllRoles() { + return List.copyOf(Arrays.asList(Role.values())); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/application/repository/AuthorityRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/application/service/SignUpService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityEntity.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityJpaRepository.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityRepositoryImpl.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/dto/converter/QueryMemberResponseConverter.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/model/Department.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/service/DepartmentService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityJpaRepository.java
- eeos/src/main/java/com/blackcompany/eeos/member/application/service/DepartmentService.java
- eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java
- eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityRepositoryImpl.java
- eeos/src/main/java/com/blackcompany/eeos/member/application/dto/converter/QueryMemberResponseConverter.java
- eeos/src/main/java/com/blackcompany/eeos/member/application/model/Department.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-20T14:09:51.877Z
Learnt from: Daae-Kim
PR: JNU-econovation/EEOS-BE#256
File: eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java:93-106
Timestamp: 2025-07-20T14:09:51.877Z
Learning: EEOS-BE 프로젝트에서 Spring Security 도입 시 JWT 토큰의 역할 정보는 DB에 "ADMIN", "USER" 형태로 저장되지만, Spring Security의 hasAnyRole() 메서드는 "ROLE_" 접두사가 붙은 권한을 기대하므로, JWT 생성 시점에 SimpleGrantedAuthority("ROLE_" + role) 형태로 접두사를 추가해야 권한 매칭이 올바르게 작동합니다.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java
🧬 Code graph analysis (1)
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java (2)
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/AuthorityModel.java (1)
Getter(7-28)eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberEntity.java (1)
Getter(24-65)
🔇 Additional comments (2)
eeos/src/main/java/com/blackcompany/eeos/auth/application/service/SignUpService.java (1)
49-49: 토큰 생성 인자(memberId) 일관성 확인 — 승인 (LGTM)AuthenticationTokenGenerator가 memberId를 인자로 받으며 토큰 클레임도 "memberId"를 사용하므로 현재 변경은 일관적입니다.
확인한 위치:
- eeos/src/main/java/com/blackcompany/eeos/auth/application/support/AuthenticationTokenGenerator.java — execute(Long memberId) / execute(Long memberId, Set authorities)
- eeos/src/main/java/com/blackcompany/eeos/auth/application/domain/token/TokenResolver.java — MEMBER_ID_CLAIM_KEY = "memberId"
- eeos/src/main/java/com/blackcompany/eeos/auth/application/service/SignUpService.java — tokenGenerator.execute(savedMember.getMemberId())
- eeos/src/main/java/com/blackcompany/eeos/auth/application/service/ReissueService.java — authenticationTokenGenerator.execute(memberId)
- eeos/src/main/java/com/blackcompany/eeos/auth/application/service/AuthFacadeService.java — authenticationTokenGenerator.execute(memberId, authorities)
- eeos/src/main/java/com/blackcompany/eeos/config/security/AccessTokenFilter.java — parseToken(...) 반환값을 memberId로 사용
- eeos/src/main/java/com/blackcompany/eeos/config/security/JwtAuthentication.java — principal/getName이 memberId
eeos/src/main/java/com/blackcompany/eeos/auth/application/repository/AuthorityRepository.java (1)
11-12: 제안: 인터페이스에 예외 동작 문서화 또는 Optional 반환 고려
AuthorityRepository인터페이스의findByMemberIdAndRole(Long memberId, Role role)메서드는 구현체(AuthorityRepositoryImpl)에서 조회 결과 없을 시NotFoundAuthorityException을 던지고 있으나, 인터페이스에는 해당 동작이 명시되어 있지 않습니다.대안으로 반환 타입을
Optional<AuthorityModel>로 변경하여 예외 대신 빈값을 표현하는 것도 고려해 보세요.인터페이스에 Javadoc을 추가하여 동작을 명확히 문서화하십시오:
- AuthorityModel findByMemberIdAndRole(Long memberId, Role role); + /** + * 지정된 회원 ID와 역할에 해당하는 권한을 조회합니다. + * @param memberId 조회할 회원의 ID + * @param role 조회할 역할 + * @return 조회된 AuthorityModel + * @throws com.blackcompany.eeos.auth.application.exception.NotFoundAuthorityException + * 조회 결과가 없을 때 발생 + */ + AuthorityModel findByMemberIdAndRole(Long memberId, Role role);구현체에서
orElseThrow(NotFoundAuthorityException::new)로 예외를 던지므로,
- 서비스 및 컨트롤러 계층에서 이 예외를 의도한 대로 처리하고 있는지
- 글로벌 예외 처리기(
@ControllerAdvice또는@ExceptionHandler)에 매핑되어 있는지
점검이 필요합니다.사용처에서 예외를 기대하고 있는지 확인한 뒤, 문서화를 적용하거나 반환 방식을 조정하십시오.
eeos/src/main/java/com/blackcompany/eeos/auth/application/model/Role.java
Show resolved
Hide resolved
| indexes = {@Index(name = "idx_member_id", columnList = AuthorityEntity.NAME + "_member_id")}, | ||
| uniqueConstraints = | ||
| @UniqueConstraint( | ||
| name = "uk_authority_member_role", | ||
| columnNames = {AuthorityEntity.NAME + "_member_id", AuthorityEntity.NAME + "_role"})) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
(member_id, role) 유니크 제약 추가: 스키마 정합성/마이그레이션 확인 필요
JPA 레벨에서 중복 방지에 유효합니다. 다만 운영 DB에 중복 데이터가 존재하면 마이그레이션 시 실패합니다. 배포 전 중복 정리 및 DDL 적용 스크립트 존재 여부를 확인해 주세요.
레포 내 마이그레이션에 본 제약이 포함되어 있는지 검색:
운영 DB 점검용 SQL 예시(참고):
-- 중복 후보
SELECT authority_member_id, authority_role, COUNT(*)
FROM authority
GROUP BY authority_member_id, authority_role
HAVING COUNT(*) > 1;🏁 Script executed:
#!/bin/bash
# 마이그레이션 파일 내 제약 이름 검색
rg -n "uk_authority_member_role" -S
# authority 테이블 관련 마이그레이션 후보도 같이 탐색
rg -nP -C2 '\bauthority\b.*(unique|constraint|index)' -SLength of output: 227
마이그레이션에 uk_authority_member_role 제약 미포함 확인됨: DDL 스크립트 추가 필요
검색 결과 AuthorityEntity.java(29–33줄)에만 JPA 어노테이션으로 UNIQUE 제약이 선언되어 있고, 마이그레이션 스크립트(예: Flyway/Liquibase) 내에는 해당 제약을 적용하는 구문을 찾을 수 없습니다.
운영 DB에 중복 데이터가 남아 있으면 배포 시 마이그레이션이 실패할 수 있으므로, 아래 작업을 반드시 수행해 주세요:
- migration 스크립트에 다음과 같은 구문 추가
ALTER TABLE authority ADD CONSTRAINT uk_authority_member_role UNIQUE (authority_member_id, authority_role);
- 배포 전 운영 DB에서 중복 레코드 조회 및 정리
SELECT authority_member_id, authority_role, COUNT(*) AS cnt FROM authority GROUP BY authority_member_id, authority_role HAVING COUNT(*) > 1;
작업 완료 후, 마이그레이션 파일이 정상 적용되는지 재확인 부탁드립니다.
🤖 Prompt for AI Agents
eeos/src/main/java/com/blackcompany/eeos/auth/persistence/authority/AuthorityEntity.java
(lines 29-33): the JPA annotation declares a UNIQUE constraint
uk_authority_member_role but there is no corresponding migration; add a new
migration (Flyway/Liquibase) that issues an ALTER TABLE to add a UNIQUE
constraint named uk_authority_member_role on columns authority_member_id and
authority_role, before applying it run a query on the production DB to find and
remove any duplicate (member_id, role) rows so the ALTER TABLE will succeed, and
verify the migration applies cleanly in a pre-prod environment before
merging/deploying.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
eeos/src/main/java/com/blackcompany/eeos/program/application/model/SemesterPeriodModel.java (1)
22-36: 1–2월 케이스 누락으로 학기 기간이 잘못 계산됩니다.현재 else 블록이 1–2월에도 적용되어, 1–2월 실행 시 3/1–8/31로 설정됩니다. 1–2월은 전년도 9/1–당해 2월 말이어야 합니다.
다음과 같이 분기 추가를 제안합니다.
- // 9월 이후이면 9월 1일부터 다음 해 2월 말까지 + // 9월 이후이면 9월 1일부터 다음 해 2월 말까지 if (now.getMonthValue() >= Month.SEPTEMBER.getValue()) { startDate = Timestamp.valueOf(LocalDate.of(now.getYear(), Month.SEPTEMBER, 1).atStartOfDay()); endDate = Timestamp.valueOf( LocalDate.of( now.getYear() + 1, Month.FEBRUARY, Month.FEBRUARY.length(Year.isLeap(now.getYear() + 1))) .atTime(23, 59, 59)); - } else { - // 3월 이후이면 3월 1일부터 8월 말까지 + } else if (now.getMonthValue() >= Month.MARCH.getValue()) { + // 3월~8월이면 3월 1일부터 8월 말까지 startDate = Timestamp.valueOf(LocalDate.of(now.getYear(), Month.MARCH, 1).atStartOfDay()); endDate = Timestamp.valueOf(LocalDate.of(now.getYear(), Month.AUGUST, 31).atTime(23, 59, 59)); + } else { + // 1~2월이면 전년도 9월 1일부터 당해 2월 말까지 + startDate = Timestamp.valueOf(LocalDate.of(now.getYear() - 1, Month.SEPTEMBER, 1).atStartOfDay()); + endDate = + Timestamp.valueOf( + LocalDate.of( + now.getYear(), + Month.FEBRUARY, + Month.FEBRUARY.length(Year.isLeap(now.getYear()))) + .atTime(23, 59, 59)); }eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java (1)
15-21: 타임존 의존 버그: 시스템 기본 타임존에 따라 달라질 수 있습니다 (KST 고정 필요)
Timestamp.valueOf(localDate.atStartOfDay())와timestamp.toLocalDateTime()은 JVM 기본 타임존에 의존합니다. KST 일관성을 보장하려면 KST로 명시 변환해야 합니다.아래 패치를 적용해 주세요.
- public static Timestamp toEpochSecond(LocalDate localDate) { + public static Timestamp toEpochSecond(LocalDate localDate) { if (localDate == null) { return null; } - - return Timestamp.valueOf(localDate.atStartOfDay()); + return Timestamp.from(localDate.atStartOfDay(ZoneId.of(KST)).toInstant()); } @@ - private static LocalDate toLocalDate(Timestamp epochSecond) { + private static LocalDate toLocalDate(Timestamp epochSecond) { if (epochSecond == null) { return null; } - - return epochSecond.toLocalDateTime().toLocalDate(); + return epochSecond.toInstant().atZone(ZoneId.of(KST)).toLocalDate(); }Also applies to: 32-38
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (2)
211-245: N+1 문제: 프로그램 조회를 루프에서 개별 호출.
programRepository.findById를 스트림 내에서 반복 호출합니다.attendModel들의programId를 수집해findAllById로 일괄 조회 후 맵으로 조인하세요.- myAttend.stream() - .map(attendEntityConverter::from) - .map(attendModel -> { - ProgramModel program = programRepository.findById(attendModel.getProgramId()) - .map(programEntityConverter::from) - .orElse(null); - if (program == null) return null; - return attendInfoWithProgramConverter.from(attendModel, program); - }) + var attendModels = myAttend.stream().map(attendEntityConverter::from).toList(); + var programIds = attendModels.stream().map(AttendModel::getProgramId).distinct().toList(); + var programMap = programRepository.findAllById(programIds).stream() + .map(programEntityConverter::from) + .collect(Collectors.toMap(ProgramModel::getId, p -> p)); + attendModels.stream() + .map(am -> { + ProgramModel program = programMap.get(am.getProgramId()); + return (program == null) ? null : attendInfoWithProgramConverter.from(am, program); + })
297-328: 집계 재조회로 인한 N+1 쿼리.페이지 결과(
pages)로 이미 합계가 계산되었다면 각 멤버별 합계를 다시 조회하지 말고, 페이지 결과를 맵으로 변환해 사용하세요. 랭킹 계산도 윈도우 함수(가능 시)나 정렬 후 인덱스 계산으로 줄일 수 있습니다.대략적인 개선 스케치:
// pages: each row => [memberId, totalScore] Map<Long, Long> memberIdToPenaltyPoint = pages.stream().collect(Collectors.toMap(o -> (Long) o[0], o -> (Long) o[1])); // 랭킹: DB 함수가 어렵다면 정렬 기반으로 1회 계산 List<Map.Entry<Long,Long>> sorted = memberIdToPenaltyPoint.entrySet().stream() .sorted(Map.Entry.<Long,Long>comparingByValue().reversed()) .toList(); Map<Long,Integer> rankByMember = new HashMap<>(); int rank = 1; for (var e : sorted) rankByMember.put(e.getKey(), rank++);
🧹 Nitpick comments (33)
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateUtil.java (4)
6-6: 클래스 주석과 실제 제공 기능이 불일치(윤년 판별 언급만 있고 미구현)문서와 구현을 맞추기 위해 주석 보정 또는 기능 추가 중 하나를 택하세요. 빠른 보정안(diff):
-/** DateUtil => 날짜의 유틸 기능과 관련된 클래스 ex) 한 날짜의 마지막 날 구하기, 윤년인지 아닌지 판별하기 */ +/** 날짜 유틸 기능 모음: 월의 마지막 날 계산 등 */
10-16: Javadoc 보강: 파라미터/반환/예외 명세 명확화현재 @param/@return이 비어 있습니다. 호출자에게 기대치와 실패 케이스를 명확히 전달하세요. 제안 diff:
/** - * 년, 월을 받아 해당 년도의 해당 월이 며칠까지 존재하는지 반환 - * - * @param year - * @param month - * @return + * 주어진 연(year)과 월(month)에 대해 해당 월의 마지막 일(day-of-month)을 반환합니다. + * + * @param year 연(예: 2025) + * @param month 월(1~12) + * @return 마지막 일(28~31) + * @throws java.time.DateTimeException month가 1~12가 아니거나 year가 범위를 벗어난 경우 */
17-19: 월 입력 경계값(1~12) 명시적 검증은 선택 사항현재도 YearMonth.of가 잘못된 값에 DateTimeException을 던지므로 기능상 문제는 없습니다. 다만, API 일관성(IllegalArgumentException 선호 등)을 위해 명시적 검증을 추가하고 싶은 경우 다음과 같이 변경 가능합니다. 이 경우 위 Javadoc의 @throws도 IllegalArgumentException으로 맞춰주세요.
public static int getLastDay(int year, int month) { - return YearMonth.of(year, month).lengthOfMonth(); + if (month < 1 || month > 12) { + throw new IllegalArgumentException("month must be 1..12: " + month); + } + return YearMonth.of(year, month).lengthOfMonth(); }또한 간단한 단위 테스트(예: 2024-02→29, 2025-02→28, 2025-04→30, 2025-12→31)를 추가하면 회귀를 방지할 수 있습니다. 필요하시면 테스트 스케치 드리겠습니다.
17-20: 편의 API 추가 제안: YearMonth 오버로드와 윤년 판별호출부가 YearMonth를 이미 보유한 경우, 오버로드가 깔끔합니다. 클래스에 아래 메서드를 추가하는 것을 고려해 주세요(추가 import 불필요).
public static int getLastDay(YearMonth yearMonth) { return yearMonth.lengthOfMonth(); } public static boolean isLeapYear(int year) { return java.time.Year.isLeap(year); }eeos/src/main/java/com/blackcompany/eeos/program/application/model/SemesterPeriodModel.java (1)
25-31: 끝 시각(23:59:59) 대신 ‘다음날 00:00 exclusive’로 다루는 방식을 고려해 주세요.쿼리/비교 로직 단순화를 위해 endDate를 “배타적 종료”로 설정하는 편이 안전합니다. 예:
LocalDate.of(...).plusDays(1).atStartOfDay()를 저장하거나, 현재 방식 유지 시minusNanos(1)로 하루 끝을 명시하세요.배타/포함 정책이 서비스 전반에 어떻게 정의되어 있는지 확인 부탁드립니다.
Also applies to: 35-35
eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarQuery.java (1)
3-3: 필드 null 허용과 범위(월/일/기간) 검증 명시 필요Integer 사용은 null 허용 의도를 시사하므로 필수/선택 구분을 명확히 해주세요(필수면
int, 선택이면 컨트롤러 단 유효성 검사). month(1–12), date(1–31), duration의 단위/범위도 명시 바랍니다.eeos/src/main/java/com/blackcompany/eeos/member/application/repository/MemberRepository.java (1)
30-31: 예외 계약(Javadoc) 추가 제안
findNameById도 미존재 시NotFoundMemberException을 던지므로findById와 동일하게 Javadoc에 명시해 주세요.void deleteById(Long memberId); - String findNameById(Long memberId); + /** + * @throws NotFoundMemberException + */ + String findNameById(Long memberId);eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarType.java (1)
11-11: 불필요한 기본 생성자입니다.enum에서 명시적인 기본 생성자는 불필요합니다. Java에서 enum은 자동으로 private 생성자를 가집니다.
다음과 같이 제거할 수 있습니다:
- CalendarType() {}eeos/src/main/java/com/blackcompany/eeos/program/application/repository/SemesterPeriodRepository.java (1)
6-10: Optional 반환에는 find 접두어 권장 + 업데이트 동시성 계약 명확화 필요
- Optional을 반환하는 조회 메서드는 관례상
find...가 가독성에 유리합니다(getSemesterPeriod→findSemesterPeriod).updateSemesterPeriod의 동시성/정합성 계약(업서트 여부, 낙관/비관 잠금, 단일 행 보장 등)을 Javadoc으로 명시해 주세요. 서비스/퍼시스턴스 구현 간 기대치 불일치 방지 목적입니다.eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java (1)
56-58: Null 입력 시 NPE 가능 — 다른 메서드와 정책 일관화다른 변환 메서드는
null을 허용하고null을 반환합니다.toMillis도 동일하게 처리하거나, 파라미터에@NotNull을 명시해 주세요.- public static Long toMillis(LocalDateTime localDateTime) { - return localDateTime.atZone(ZoneId.of(KST)).toInstant().toEpochMilli(); - } + public static Long toMillis(LocalDateTime localDateTime) { + if (localDateTime == null) { + return null; + } + return localDateTime.atZone(ZoneId.of(KST)).toInstant().toEpochMilli(); + }eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarCreateCommand.java (1)
5-6: 타입 강제와 검증 강화를 고려해 주세요
type을String대신CalendarType(enum)으로 받으면 불필요한 문자열 검증/매핑을 제거할 수 있습니다.startAt,endAt은Long(epoch ms) 대신Instant/ZonedDateTime사용도 고려 대상입니다(이미 공용 변환 유틸이 존재).- null 허용 필요 없으면 원시형(
long)으로 강제해 NPE 리스크를 제거하세요.컨트롤러/서비스 레이어에서 title/url/type에 대한 NotBlank·패턴 검증이 이미 수행되는지 확인 부탁드립니다.
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/DeniedCalendarTypeException.java (1)
17-20: 사용자 메시지 오탈자 및 표시명 개선 제안
- "칼렌더" → "캘린더"로 수정 권장.
department.name()은 ENUM 상수(영문/대문자)를 그대로 노출합니다. 사용자 친화적 표시명(국문 라벨 등)으로 교체하는 헬퍼/프로퍼티를 사용하는 편이 좋습니다.- return String.format("%s 부서는 해당 타입의 칼렌더를 생성할 수 없습니다.", department.name()); + return String.format("%s 부서는 해당 타입의 캘린더를 생성할 수 없습니다.", department.name());eeos/src/main/java/com/blackcompany/eeos/program/application/support/SemesterPeriodProvider.java (1)
13-15: 디폴트 객체 반환의 의미 명확화 및 관찰성 보강 제안조회 실패 시
new SemesterPeriodModel()을 반환하면 “값이 존재하지 않음”과 “정상 기간(기본값)”이 구분되지 않을 수 있습니다.
- 의미 있는
empty()정적 팩토리 또는Optional유지가 더 안전합니다.- 최소한 로그를 남겨 운영 관측성을 확보하는 것을 권장합니다.
예:
@Slf4j추가 후 fallback 시log.info("SemesterPeriod not found. Using default.").기본 생성된
SemesterPeriodModel의 필드들이 이후 로직(NPE/검증)에 안전한지 확인 부탁드립니다.eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/UpdateCalendarUsecase.java (1)
5-8: 업데이트 반환 타입(Long)의 의미를 명확히 해주세요.update가 무엇을 반환하는지(수정된 엔티티의 id인지, 이전/이후 버전의 id인지, 혹은 영향 행 수) 명확하지 않습니다. 컨트롤러·서비스 호출부에서 이 반환값을 어떻게 소비하는지 확인·문서화하거나, 불분명하다면
void또는 응답 DTO 반환으로 일관성을 주는 것을 권장합니다.eeos/src/main/java/com/blackcompany/eeos/program/application/dto/request/SemesterPeriodApplicationCommand.java (1)
7-10: 기간 유효성(시작 ≤ 종료) 검증과 java.time 사용을 고려해주세요.
- 현재
@NotNull만 있어 시작/종료 역전값이 유입될 수 있습니다. 레코드의 컴팩트 생성자 또는@AssertTrue로 교차 필드 검증을 추가하세요.java.sql.Timestamp대신java.time.Instant/LocalDateTime사용을 권장합니다.적용 예시:
+import jakarta.validation.constraints.AssertTrue; +import java.time.Instant; - public record SemesterPeriodApplicationCommand( - @NotNull(message = "시작 날짜는 필수입니다") Timestamp startDate, - @NotNull(message = "종료 날짜는 필수입니다") Timestamp endDate) + public record SemesterPeriodApplicationCommand( + @NotNull(message = "시작 날짜는 필수입니다") Instant startDate, + @NotNull(message = "종료 날짜는 필수입니다") Instant endDate) implements AbstractApplicationDto { + @AssertTrue(message = "시작 날짜는 종료 날짜보다 이후일 수 없습니다") + public boolean isValidRange() { + return startDate == null || endDate == null || !startDate.isAfter(endDate); + } + }eeos/src/main/java/com/blackcompany/eeos/calendar/application/repository/CalendarRepository.java (1)
7-18: 리포지토리 API 설계 정교화(명명·옵셔널·페이지네이션).
findById는 존재하지 않을 수 있으므로Optional<CalendarModel>반환을 권장합니다.findByBetweenDate명명은 애매합니다. 의미상 "기간 겹침"이라면findOverlapping(LocalDateTime start, LocalDateTime end)처럼 바꾸는 것을 제안합니다.- 기간 조회 결과가 많을 수 있으므로 페이지네이션 지원 메서드도 고려해주세요.
- CalendarModel findById(Long id); + Optional<CalendarModel> findById(Long id); - List<CalendarModel> findByBetweenDate(LocalDateTime startAt, LocalDateTime endAt); + List<CalendarModel> findOverlapping(LocalDateTime startAt, LocalDateTime endAt); + Page<CalendarModel> findOverlapping(LocalDateTime startAt, LocalDateTime endAt, Pageable pageable);eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (3)
151-154: 상태 비교에 하드코딩된 문자열 사용 지양.
"attend"리터럴 대신 Enum 기반 값을 사용해 오타/변경에 안전하게 하세요.- if (changedModel.getStatus().equals("attend")) { + if (changedModel.getStatus().equals(AttendStatus.ATTEND.getStatus())) { Long rank = getNextRank(programId); changedModel.setRank(rank); }
256-260: 중복 호출 최소화 및 널 처리 간결화.
semesterPeriodProvider.getSemesterPeriod()호출을 1회로 줄이고,Objects.requireNonNullElse패턴을 적용하면 가독성이 좋아집니다.- if (startDate == null) - startDate = semesterPeriodProvider.getSemesterPeriod().getStartDate().getTime(); - if (endDate == null) - endDate = semesterPeriodProvider.getSemesterPeriod().getEndDate().getTime(); + var period = semesterPeriodProvider.getSemesterPeriod(); + if (startDate == null) startDate = period.getStartDate().getTime(); + if (endDate == null) endDate = period.getEndDate().getTime();
280-286:sortTypeNPE 가능성 및 비교 일관성.
sortType가 null이면 NPE가 납니다. 상수 우선 비교와 대소문자 무시 비교를 권장합니다.- if (sortType.equals("asc")) { + if ("asc".equalsIgnoreCase(sortType)) {eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarJpaRepository.java (2)
11-21: 주석 처리된 과거 JPQL 제거 제안.주석 블록은 혼란만 유발합니다. 히스토리는 Git이 보존하므로 삭제를 권장합니다.
28-34:findNotStarted의 의미 명확화 및 경계 포함 확인.
BETWEEN :from AND :to는 양 끝 포함입니다. 요구사항이 배타 경계라면 조건을 조정하세요. 메서드명도findStartingBetween등으로 의미를 명확히 하면 좋습니다.eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarEntity.java (2)
51-61: 칼럼 제약(길이/인덱스) 보강 제안.
title,url의 길이 제약을 명시하세요(예:title100,url2048).- 조회 패턴에 따라
writer인덱스도 고려할 수 있습니다.- @Column(name = ENTITY_PREFIX + "_title", nullable = false) + @Column(name = ENTITY_PREFIX + "_title", nullable = false, length = 100) private String title; - @Column(name = ENTITY_PREFIX + "_url", nullable = false) + @Column(name = ENTITY_PREFIX + "_url", nullable = false, length = 2048) private String url;
30-34: 인덱스 구성 검토(복합 인덱스 고려).기간 겹침 조회가 많다면
(start_at, end_at)복합 인덱스 또는(start_at)단독 인덱스 외에 쿼리 패턴에 맞춘 보조 인덱스를 검토하세요.eeos/src/main/java/com/blackcompany/eeos/calendar/application/validator/CalendarValidator.java (2)
18-34: AVAILABLE 자료구조를 EnumMap/불변 맵으로 고정해 동시성·오용 리스크를 줄여주세요.현재 HashMap 인스턴스 필드를 생성자에서 채우는 방식입니다. Enum 키에는 EnumMap이 더 적합하고, 불변으로 감싸 오용을 방지하는 편이 안전합니다. 또한 상수형 네이밍(대문자)인데 인스턴스 필드라 혼동을 줍니다(관례상 static final 상수에 사용).
- private final Map<CalendarType, Set<Department>> AVAILABLE = new HashMap<>(); - - public CalendarValidator() { - final Set<Department> EVENT_AVAILABLE = Set.of(Department.EVENT, Department.PRESIDENT); - final Set<Department> PRESENTATION_AVAILABLE = Set.of(Department.PRESIDENT); - final Set<Department> ETC_AVAILABLE = - Set.of(Department.PRESIDENT, Department.EVENT, Department.MANAGEMENT, Department.MARKETING, Department.NONE); - - AVAILABLE.put(CalendarType.ETC, ETC_AVAILABLE); - AVAILABLE.put(CalendarType.PRESENTATION, PRESENTATION_AVAILABLE); - AVAILABLE.put(CalendarType.EVENT, EVENT_AVAILABLE); - } + private final Map<CalendarType, Set<Department>> availableByType; + + public CalendarValidator() { + Map<CalendarType, Set<Department>> m = new EnumMap<>(CalendarType.class); + m.put(CalendarType.EVENT, Set.of(Department.EVENT, Department.PRESIDENT)); + m.put(CalendarType.PRESENTATION, Set.of(Department.PRESIDENT)); + m.put( + CalendarType.ETC, + Set.of(Department.PRESIDENT, Department.EVENT, Department.MANAGEMENT, Department.MARKETING, Department.NONE)); + this.availableByType = Map.copyOf(m); // 불변화 + }
56-60: URL 검증 TODO는 지금 마무리하는 것이 좋습니다.간단히 http/https만 허용하고 공백 미포함 정도를 검증해도 1차 방어가 됩니다. 전용 예외가 없다면 일단 IllegalArgumentException으로 리턴하고, 후속으로 BusinessException 파생 예외 도입을 권장합니다.
- // TODO: 정규표현식으로 url 검증 + if (url == null || url.isBlank()) return; + try { + var uri = java.net.URI.create(url); + String scheme = uri.getScheme(); + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + throw new IllegalArgumentException("Only http/https URLs are allowed"); + } + if (uri.getHost() == null) { + throw new IllegalArgumentException("URL must contain host"); + } + } catch (IllegalArgumentException e) { + throw e; + }eeos/src/main/java/com/blackcompany/eeos/program/persistence/SemesterPeriodRepositoryImpl.java (2)
19-24: "update"가 매번 신규 행을 생성하는 동작입니다. 의도 확인 필요.toEntity에서 id 없이 save하여 이력 누적이 됩니다. 이게 요구사항(이력 보존)이라면 OK, 아니라면 기존 레코드 업데이트/대체가 되어야 합니다. 메서드명도 create/replace 등으로 명확화 권장.
이력이 불필요하다면 id를 보존하도록 toEntity에 id 매핑 추가 또는 JPA update 쿼리 사용을 제안합니다.
26-38: 모델/엔티티 간 매핑 필드 최소화로 확장성 제한.id/createdDate 등 핵심 메타가 모델에서 손실됩니다. 이후 API에서 “마지막 갱신일” 표출, 정합성 검증 등에 필요할 수 있습니다.
필요 시 SemesterPeriodModel/Entity에 id/createdDate 동기 매핑을 고려해주세요.
eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarRepositoryImpl.java (1)
24-28: 엔티티 미존재 시 도메인 예외로 변환하는 것이 좋습니다.현재 orElseThrow()는 NoSuchElementException을 던질 수 있습니다. MemberRepositoryImpl은 NotFoundMemberException으로 변환하고 있어 일관성 차원에서 Calendar도 전용 예외(NotFoundCalendarException 등)로 매핑을 권장합니다.
- Optional<CalendarEntity> entity = jpaRepository.findById(id); - return entity.map(CalendarEntity::toModel).orElseThrow(); + return jpaRepository + .findById(id) + .map(CalendarEntity::toModel) + .orElseThrow(NotFoundCalendarException::new);전용 예외 클래스 존재 여부를 확인 부탁드립니다.
eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/SemesterPeriodController.java (1)
24-31: 업데이트에 201/CREATE 메시지를 반환합니다. 200/UPDATE가 더 자연스럽습니다.PUT은 멱등 업데이트 의미가 강하므로 상태코드 200 OK, MessageCode.UPDATE 사용을 권장합니다(신규 리소스 생성 시에만 201).
- return ApiResponseGenerator.success(response, HttpStatus.CREATED, MessageCode.CREATE); + return ApiResponseGenerator.success(response, HttpStatus.OK, MessageCode.UPDATE);eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java (1)
60-67: 작성자 이름 조회 N+1 가능성.일정 N개마다 memberRepository.findNameById를 호출합니다. 대량 조회 시 성능 저하가 큽니다. 작성자 ID 집합을 추출해 배치 조회 후 Map으로 조인하세요(또는 JPA 페치 조인/프로젝션).
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarCommandService.java (2)
53-56: save 반환값 사용 일관성 정리 제안.
create()에선repository.save(...)반환값을 그대로 리턴하지만,update()에선 무시하고model.getId()를 리턴합니다. 저장 구현이 동일한 시그니처를 가진다면 반환값 처리 방식을 통일하는 편이 명확합니다.가능하면 아래처럼 정리:
- repository.save(model); - - return model.getId(); + return repository.save(model);
16-17: 미사용 로거 제거(선택).
@Slf4j를 사용하지 않습니다. 유지할 계획이 없다면 제거해 경고/잡음을 줄이세요.-@Slf4j public class CalendarCommandService및 관련 import 제거.
Also applies to: 23-23
eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarModel.java (1)
24-26: 잠재적 NPE:isWriter에서writer또는updater가 null일 경우.새로 생성된 모델 등에서
writer가 null일 가능성을 배제하지 못합니다.Objects.equals로 안전 비교하세요.- return this.writer.equals(updater); + return java.util.Objects.equals(this.writer, updater);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarCreateCommand.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarQuery.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarResponse.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarResponses.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarUpdateCommand.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/DeniedCalendarTypeException.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/DeniedCalendarUpdateException.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/InvalidDateException.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/NotFoundCalendarTypeException.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarModel.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarType.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/repository/CalendarRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarCommandService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/CreateCalendarUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/DeleteCalendarUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/GetCalendarUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/UpdateCalendarUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/application/validator/CalendarValidator.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarEntity.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarJpaRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarRepositoryImpl.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/common/utils/DateUtil.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/member/application/repository/MemberRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/member/persistence/JpaMemberRepository.java(2 hunks)eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberRepositoryImpl.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/dto/request/SemesterPeriodApplicationCommand.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/dto/response/SemesterPeriodApplicationQuery.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/model/SemesterPeriodModel.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/repository/CalendarRepository.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/repository/SemesterPeriodRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/service/CalendarService.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/service/SemesterPeriodService.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/support/CalendarProvider.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/support/SemesterPeriodProvider.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/GetCalendarUsecase.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/GetSemesterPeriodUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/UpdateCalendarUsecase.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/UpdateSemesterPeriodUsecase.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/persistence/CalendarRepositoryImpl.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/persistence/JpaCalendarRepository.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/persistence/JpaSemesterPeriodRepository.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/persistence/SemesterPeriodEntity.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/persistence/SemesterPeriodRepositoryImpl.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/CalendarController.java(0 hunks)eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/SemesterPeriodController.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/program/presentation/dto/UpdateSemesterPeriodRequest.java(1 hunks)eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java(5 hunks)
💤 Files with no reviewable changes (8)
- eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/UpdateCalendarUsecase.java
- eeos/src/main/java/com/blackcompany/eeos/program/persistence/JpaCalendarRepository.java
- eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/GetCalendarUsecase.java
- eeos/src/main/java/com/blackcompany/eeos/program/application/repository/CalendarRepository.java
- eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/CalendarController.java
- eeos/src/main/java/com/blackcompany/eeos/program/application/support/CalendarProvider.java
- eeos/src/main/java/com/blackcompany/eeos/program/persistence/CalendarRepositoryImpl.java
- eeos/src/main/java/com/blackcompany/eeos/program/application/service/CalendarService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- eeos/src/main/java/com/blackcompany/eeos/config/security/SecurityFilterChainConfig.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-30T10:51:03.804Z
Learnt from: rlajm1203
PR: JNU-econovation/EEOS-BE#250
File: eeos/src/main/java/com/blackcompany/eeos/program/application/service/ProgramQuitService.java:47-55
Timestamp: 2025-03-30T10:51:03.804Z
Learning: DateConverter.toEpochSecond(LocalDate) 메소드는 long 값이 아닌 Timestamp 객체를 반환하므로, epoch 시간을 밀리초 단위의 long 값으로 변환하려면 .getTime()을 호출해야 합니다.
Applied to files:
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java
🧬 Code graph analysis (13)
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateUtil.java (1)
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java (1)
UtilityClass(11-59)
eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarEntity.java (1)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarModel.java (1)
Getter(10-88)
eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/SemesterPeriodController.java (2)
eeos/src/main/java/com/blackcompany/eeos/common/presentation/response/ApiResponseBody.java (1)
ApiResponseBody(7-24)eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java (1)
RequestMapping(30-79)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/validator/CalendarValidator.java (3)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/DeniedCalendarTypeException.java (1)
DeniedCalendarTypeException(7-21)eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/DeniedCalendarUpdateException.java (1)
DeniedCalendarUpdateException(6-18)eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/InvalidDateException.java (1)
InvalidDateException(6-18)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarCommandService.java (2)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java (1)
Service(19-71)eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (1)
Service(64-454)
eeos/src/main/java/com/blackcompany/eeos/program/application/service/SemesterPeriodService.java (1)
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (1)
Service(64-454)
eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarRepositoryImpl.java (2)
eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberRepositoryImpl.java (1)
Repository(12-75)eeos/src/main/java/com/blackcompany/eeos/program/persistence/SemesterPeriodRepositoryImpl.java (1)
Repository(9-39)
eeos/src/main/java/com/blackcompany/eeos/program/persistence/SemesterPeriodRepositoryImpl.java (2)
eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarRepositoryImpl.java (1)
Repository(11-46)eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberRepositoryImpl.java (1)
Repository(12-75)
eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java (2)
eeos/src/main/java/com/blackcompany/eeos/common/presentation/response/ApiResponseBody.java (1)
ApiResponseBody(7-24)eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/SemesterPeriodController.java (1)
RestController(17-39)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/NotFoundCalendarTypeException.java (2)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarModel.java (1)
Getter(10-88)eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarEntity.java (1)
Getter(23-78)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java (1)
eeos/src/main/java/com/blackcompany/eeos/member/application/exception/NotFoundMemberException.java (1)
NotFoundMemberException(7-18)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarModel.java (2)
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/NotFoundCalendarTypeException.java (1)
Getter(7-20)eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarEntity.java (1)
Getter(23-78)
eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberRepositoryImpl.java (1)
eeos/src/main/java/com/blackcompany/eeos/member/application/exception/NotFoundMemberException.java (1)
NotFoundMemberException(7-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (24)
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateUtil.java (1)
7-8: @UtilityClass 적용 적절정적 유틸 의도를 명확히 드러내며, 인스턴스화 차단이 보장됩니다. 현재 공개 API 범위도 간결합니다.
eeos/src/main/java/com/blackcompany/eeos/program/application/dto/response/SemesterPeriodApplicationQuery.java (1)
12-15: DTO 리네임 및 필드 구성 적합이전 Calendar* 네이밍과의 일관성 이슈 해소되고, 필드도 그대로 유지되어 호환성 문제 없어 보입니다.
eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/GetSemesterPeriodUsecase.java (1)
5-7: Usecase 정의 명확—문제 없음조회 전용 트랜잭션 경계(
@Transactional(readOnly = true))가 구현체에 적용되어 있는지만 확인해 주세요.eeos/src/main/java/com/blackcompany/eeos/member/persistence/MemberRepositoryImpl.java (1)
71-74: 승인 — JpaMemberRepository#findNameById 시그니처 및 soft-delete 필터링 확인됨
JpaMemberRepository에 findNameById 메서드가 존재하며 @query에 m.isDeleted=false가 포함되어 있어 soft-delete 필터링이 적용됩니다.eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarResponses.java (1)
6-6: LGTM! 깔끔한 응답 래퍼 DTO 구현입니다.
CalendarResponse리스트를 감싸는 간단하고 명확한 래퍼 DTO입니다.AbstractResponseDto를 구현하여 일관성 있는 응답 구조를 유지하고 있습니다.eeos/src/main/java/com/blackcompany/eeos/program/persistence/JpaSemesterPeriodRepository.java (1)
7-7: 최신 레코드 조회 메서드가 올바르게 구현되었습니다.Spring Data JPA에서
findTop은findFirst와 동일한 기능을 제공하며, 숫자가 생략될 경우 기본적으로 1개의 결과를 반환합니다.CreatedDate내림차순으로 정렬하여 가장 최신 레코드를 조회하는 의도가 명확하게 드러나며,Optional반환 타입으로 안전한 null 처리가 가능합니다.eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/CreateCalendarUsecase.java (1)
5-8: LGTM! 명확한 캘린더 생성 유스케이스 인터페이스입니다.인터페이스의 시그니처가 명확하며,
CalendarCreateCommand를 받아 생성된 캘린더의 ID를 반환하는 구조가 직관적입니다. 단일 책임 원칙을 잘 따르고 있습니다.eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/DeleteCalendarUsecase.java (1)
3-6: LGTM! 간결하고 명확한 삭제 유스케이스 인터페이스입니다.캘린더 ID를 받아 삭제하는 간단하고 명확한 인터페이스입니다.
void반환 타입이 삭제 작업에 적합합니다.eeos/src/main/java/com/blackcompany/eeos/calendar/application/model/CalendarType.java (1)
13-18: LGTM! 타입 안전한 조회 메서드입니다.대소문자를 구분하지 않는 조회 로직과 적절한 예외 처리가 구현되어 있습니다. Stream API를 사용한 구현도 깔끔합니다.
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/NotFoundCalendarTypeException.java (2)
7-20: LGTM! 표준 예외 패턴을 따르는 구현입니다.
BusinessException을 상속하고 일관성 있는 구조를 가지고 있습니다. 한국어 메시지도 명확하게 작성되었습니다.
10-10: 승인 — '10000' 에러 코드 중복 없음 확인프로젝트 예외 클래스 검색 결과 '10000'은 NotFoundCalendarTypeException에만 정의되어 있으며(같은 캘린더 도메인 예외들은 10001·10002·10003 등 사용), gradle wrapper의 networkTimeout=10000은 예외 코드와 무관합니다.
eeos/src/main/java/com/blackcompany/eeos/member/persistence/JpaMemberRepository.java (1)
26-27: LGTM! 효율적인 이름 조회 쿼리입니다.전체
MemberEntity를 조회하는 대신 필요한name필드만 선택하여 성능상 이점이 있습니다.isDeleted=false조건도 적절하게 포함되어 있고,Optional반환 타입으로 안전한 null 처리가 가능합니다.eeos/src/main/java/com/blackcompany/eeos/calendar/application/usecase/GetCalendarUsecase.java (1)
7-12: LGTM! 잘 설계된 조회 유스케이스 인터페이스입니다.두 가지 조회 방식(쿼리 기반, D-Day 기반)을 제공하는 것이 실용적입니다. 메서드명이 명확하고 반환 타입도 일관성 있게 구성되어 있습니다.
eeos/src/main/java/com/blackcompany/eeos/program/application/usecase/UpdateSemesterPeriodUsecase.java (1)
6-8: 인터페이스 설계 적절합니다입력/출력 DTO가 명확하고, 사용처에서 구현을 교체하기 용이합니다. 별도 이슈 없습니다.
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/InvalidDateException.java (1)
10-17: 예외 설계 적절합니다고정 실패 코드와 400 매핑이 명확합니다. 메시지도 검증 조건(시작 > 종료 불가)과 일치합니다.
eeos/src/main/java/com/blackcompany/eeos/program/presentation/dto/UpdateSemesterPeriodRequest.java (1)
7-15: API 날짜 표현 통일 필요 — Timestamp → epoch ms(Long) 또는 LocalDate 전환 검토
- 문제: 해당 DTO가 java.sql.Timestamp를 사용합니다. 프로젝트의 다른 캘린더 엔드포인트가 epoch ms(Long)나 LocalDate/ISO-8601을 사용 중이면 클라이언트가 서로 다른 포맷을 처리해야 하므로 외부 API 표면을 통일할 것을 권장합니다.
- 작업(검토 항목): UpdateSemesterPeriodRequest를 Long startAtMillis/Long endAtMillis 또는 LocalDate startDate/LocalDate endDate로 전환 가능한지 검토(컨트롤러 바인딩, 직렬화/역직렬화, 기존 소비자 영향 포함). 검증 로직(isEndDateAfterStartDate)은 millis 비교 또는 LocalDate.isAfter로 대체 필요.
파일: eeos/src/main/java/com/blackcompany/eeos/program/presentation/dto/UpdateSemesterPeriodRequest.java (lines 7–15)
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java (1)
15-31: 메서드명 혼동 — toEpochSecond는 Timestamp를 반환합니다toEpochSecond(...)는 java.sql.Timestamp를 반환(초/long 아님). 네이밍(toStartOfDayTimestamp 등) 또는 Javadoc으로 반환 타입·단위를 명확히 하고, 호출부에서 epoch(long)이 필요할 때 .getTime() 호출 누락이 없는지 전수 점검하세요.
파일: eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java (15–31)검사용 스크립트(리포지토리 루트에서 실행):
#!/bin/bash set -euo pipefail # 1) 모든 호출(테스트 제외) 출력 if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then git grep -n --no-color "DateConverter.toEpochSecond" | grep -v '/test/' || true else rg -n --hidden -uu "DateConverter.toEpochSecond" -g '!**/test/**' -C2 || true fi # 2) 같은 표현에서 .getTime()으로 체이닝되는지 판별 (체이닝이 없으면 POTENTIAL로 표시) if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then git grep -n --no-color "DateConverter.toEpochSecond" | grep -v '/test/' | while IFS=: read -r file lineno _; do line=$(sed -n "${lineno}p" "$file") if printf '%s\n' "$line" | grep -q '\.getTime'; then printf "OK %s:%s\n" "$file" "$lineno" else printf "POT %s:%s\n" "$file" "$lineno" sed -n "$((lineno-2)),$((lineno+2))p" "$file" echo "----" fi done fi참고: 위 스크립트는 동일 표현 내 체이닝 여부만 자동으로 판별합니다. Timestamp를 변수에 할당한 뒤 별도 라인에서 .getTime()을 호출하는 경우는 수동 확인이 필요합니다.
eeos/src/main/java/com/blackcompany/eeos/target/application/service/AttendService.java (1)
119-129: 동시성 안전한 순위 증가 로직: 트랜잭션 범위 확인.
findByProgramIdForUpdate를 사용하는 점은 좋습니다.getNextRank에@Transactional이 선언되어 있어READ_ONLY=false가 보장되는지, 상위 호출(changeStatus)이 같은 트랜잭션 안에서 실행되는지 확인해 주세요.eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarJpaRepository.java (1)
21-27: 기간 겹침(Overlap) 조건식 적절.
(c.startAt <= :endAt AND c.endAt >= :startAt)형태의 겹침 검출이 정확합니다. 인덱스(startAt)가 있어 성능도 적절할 것으로 보입니다.엔티티에 소프트 삭제 컬럼이 있다면(where 절) 포함 여부를 확인해 주세요.
eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarResponse.java (1)
11-18: 응답 계약 확인: type 직렬화 규칙과 시간 단위.
type을enum.name()으로 내보내는데, FE가 소문자/코드값을 기대하지 않는지 확인 바랍니다.startAt/endAt은 millis입니다. API 스펙(예: ms vs s)을 명확히 문서화해주세요.eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarEntity.java (1)
63-77: 엔티티-모델 변환은 적절합니다.양방향 변환이 명확하고 누락된 필드 없습니다. 생성 전략(ID)과
toEntity에서의 id 전달도 일관적입니다.eeos/src/main/java/com/blackcompany/eeos/calendar/persistence/CalendarRepositoryImpl.java (1)
31-40: 확인: 리포지토리 쿼리는 양쪽 포함(inclusive)으로 구현되어 있습니다.findBetween은
c.startAt <= :endAt and c.endAt >= :startAt로, findNotStarted는c.startAt BETWEEN :from AND :to로 각각 양측 포함 처리됩니다 (CalendarJpaRepository.java). 따라서 리포지토리 수준에서는 누락이 발생하지 않습니다. 서비스 레이어에서 endAt/from을 자정(00:00)으로만 설정하면 마지막 날 일정이 빠질 수 있으니, 서비스에서 endAt을 end-of-day로 변환하거나 범위를 [start, nextDay) 방식으로 일관되게 처리하세요.eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java (1)
39-45: 경계 시간(끝점) 처리 확인 필요 — 마지막 날 일정 누락 가능end를 00:00으로 설정하면 월/일 조회에서 마지막 날의 일정이 빠질 수 있으니 repository 쿼리의 경계(포함/배제) 정책 확인이 필요합니다.
- 위치: eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java (약 라인 41–44: end 계산, 라인 56: getDefault 호출)
- 원인: end가 "마지막날 00:00" 또는 startDate.plusDays(duration).atTime(00:00)으로 설정되어 해당 날짜의 시간대가 범위 밖으로 떨어질 수 있음.
- 조치(옵션):
- 반개구간 [start, end) 정책이면: end를 마지막날.plusDays(1).atStartOfDay() (또는 duration 쪽에 +1일)로 바꾸고 쿼리를 "< end"로 맞추기.
- 포함구간(예: SQL BETWEEN) 정책이면: end를 마지막날.atTime(LocalTime.MAX)로 설정(23:59:59.999999999).
- 검증 요청: repository.findByBetweenDate 및 repository.findNotStarted 구현(@query, JPQL/SQL, 메서드명 규약)을 확인해 현재 경계 정책을 회신하거나 정책에 맞춰 end 계산을 일관되게 수정해 주세요.
eeos/src/main/java/com/blackcompany/eeos/program/application/service/SemesterPeriodService.java (1)
30-36: 시맨틱 검증 누락 — 시작일이 종료일 이후인 경우 차단 필요updateSemesterPeriod에서 시작일이 종료일 이후인지 검증하지 않습니다. 잘못된 학사기간 저장 시 전역 계산(출결/벌점 등)에 영향이 큽니다. 간단한 선검증 추가 권장.
public SemesterPeriodApplicationQuery updateSemesterPeriod( SemesterPeriodApplicationCommand command) { + if (command.startDate().after(command.endDate())) { + throw new IllegalArgumentException("학기 시작일은 종료일보다 앞서야 합니다."); + } SemesterPeriodModel model = semesterPeriodRepository.updateSemesterPeriod( new SemesterPeriodModel(command.startDate(), command.endDate())); return new SemesterPeriodApplicationQuery(model.getStartDate(), model.getEndDate()); }해당 검증이 Controller/Repository/Model 등 다른 계층에서 이미 보장된다면 위치를 명확히 알려주세요.
| public record CalendarUpdateCommand(String title, String url, String type, Long startAt, Long endAt) | ||
| implements AbstractRequestDto {} |
There was a problem hiding this comment.
입력 유효성 보강(필드 제약·교차 검증)과 타입 명확화가 필요합니다.
- 제목/타입은 공란 불가, URL 길이/형식 제한, 시간 필수 및
startAt ≤ endAt보장이 필요합니다. - 가능하면
type은 String 대신 Enum(예: CalendarType)으로 받고, 시간은Instant/LocalDateTime로 받는 것을 권장합니다.
예시:
+import jakarta.validation.constraints.*;
+import org.hibernate.validator.constraints.URL;
-public record CalendarUpdateCommand(String title, String url, String type, Long startAt, Long endAt)
- implements AbstractRequestDto {}
+public record CalendarUpdateCommand(
+ @NotBlank @Size(max = 100) String title,
+ @Size(max = 2048) @URL(regexp = "^(https?://).*$", message = "유효한 URL이어야 합니다") String url,
+ @NotBlank String type,
+ @NotNull Long startAt,
+ @NotNull Long endAt
+) implements AbstractRequestDto {
+ public CalendarUpdateCommand {
+ if (startAt != null && endAt != null && startAt > endAt) {
+ throw new IllegalArgumentException("startAt은 endAt보다 이후일 수 없습니다");
+ }
+ }
+}📝 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.
| public record CalendarUpdateCommand(String title, String url, String type, Long startAt, Long endAt) | |
| implements AbstractRequestDto {} | |
| import jakarta.validation.constraints.*; | |
| import org.hibernate.validator.constraints.URL; | |
| public record CalendarUpdateCommand( | |
| @NotBlank @Size(max = 100) String title, | |
| @Size(max = 2048) @URL(regexp = "^(https?://).*$", message = "유효한 URL이어야 합니다") String url, | |
| @NotBlank String type, | |
| @NotNull Long startAt, | |
| @NotNull Long endAt | |
| ) implements AbstractRequestDto { | |
| public CalendarUpdateCommand { | |
| if (startAt != null && endAt != null && startAt > endAt) { | |
| throw new IllegalArgumentException("startAt은 endAt보다 이후일 수 없습니다"); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto/CalendarUpdateCommand.java
around lines 5-6, the DTO currently exposes raw Strings and Longs without
validation; change the API to use a CalendarType enum for the type and a
temporal type (Instant or LocalDateTime) for start/end, add field-level
constraints (e.g., @NotBlank for title and type, @Size and URL/regex constraint
for url, @NotNull for times) and implement cross-field validation to enforce
startAt ≤ endAt (either an @AssertTrue method or a custom class-level
ConstraintValidator). Ensure the enum CalendarType is added to the model,
convert/deserialize incoming values appropriately, and surface validation errors
via standard Bean Validation so invalid inputs are rejected before business
logic.
| public DeniedCalendarUpdateException() { | ||
| super(FAIL_CODE, HttpStatus.UNAUTHORIZED); | ||
| } |
There was a problem hiding this comment.
401(UNAUTHORIZED) 대신 403(FORBIDDEN) 사용 권장
인증은 되었으나 권한이 없는 상황이므로 403이 더 적합합니다. 현재 401은 인증 실패 의미로 오해될 수 있습니다.
- super(FAIL_CODE, HttpStatus.UNAUTHORIZED);
+ super(FAIL_CODE, HttpStatus.FORBIDDEN);📝 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.
| public DeniedCalendarUpdateException() { | |
| super(FAIL_CODE, HttpStatus.UNAUTHORIZED); | |
| } | |
| public DeniedCalendarUpdateException() { | |
| super(FAIL_CODE, HttpStatus.FORBIDDEN); | |
| } |
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/application/exception/DeniedCalendarUpdateException.java
around lines 10 to 12, the exception currently returns HttpStatus.UNAUTHORIZED
but this scenario represents an authenticated user lacking permission; change
the status to HttpStatus.FORBIDDEN by replacing HttpStatus.UNAUTHORIZED with
HttpStatus.FORBIDDEN in the constructor call (ensure imports remain valid and
adjust any tests or callers expecting 401 to expect 403).
| private void updateCalendar(Long memberId, CalendarUpdateCommand command, CalendarModel model) { | ||
| MemberModel member = memberRepository.findById(memberId); | ||
|
|
||
| validator.updateValidate(model, memberId); | ||
| validator.typeValidate(model, member.getDepartment()); | ||
| validator.durationValidate(model); | ||
|
|
||
| LocalDateTime startAt = DateConverter.toLocalDateTime(command.startAt()); | ||
| LocalDateTime endAt = DateConverter.toLocalDateTime(command.endAt()); | ||
|
|
||
| model | ||
| .updateEndAt(endAt) | ||
| .updateStartAt(startAt) | ||
| .updateTitle(command.title()) | ||
| .updateUrl(command.url()) | ||
| .updateType(command.type()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
업데이트 검증 순서가 잘못되어 새 값이 검증되지 않습니다 (실제 유효성 누락 가능).
현재 updateCalendar는 기존 model 상태로 typeValidate, durationValidate를 수행한 뒤에야 command 값으로 필드를 갱신합니다. 이러면 잘못된 날짜 범위·타입이 그대로 저장될 수 있습니다.
아래처럼 "새 상태(candidate)"를 만들어 그 상태로 검증한 뒤 실제 모델에 적용하도록 바꿔주세요.
private void updateCalendar(Long memberId, CalendarUpdateCommand command, CalendarModel model) {
MemberModel member = memberRepository.findById(memberId);
validator.updateValidate(model, memberId);
- validator.typeValidate(model, member.getDepartment());
- validator.durationValidate(model);
-
- LocalDateTime startAt = DateConverter.toLocalDateTime(command.startAt());
- LocalDateTime endAt = DateConverter.toLocalDateTime(command.endAt());
-
- model
- .updateEndAt(endAt)
- .updateStartAt(startAt)
- .updateTitle(command.title())
- .updateUrl(command.url())
- .updateType(command.type());
+ // 1) 새 값 구성
+ LocalDateTime startAt = DateConverter.toLocalDateTime(command.startAt());
+ LocalDateTime endAt = DateConverter.toLocalDateTime(command.endAt());
+ CalendarModel candidate =
+ CalendarModel.create(command.title(), startAt, endAt, command.type(), command.url(), model.getWriter());
+
+ // 2) 새 상태로 검증
+ validator.typeValidate(candidate, member.getDepartment());
+ validator.durationValidate(candidate);
+
+ // 3) 실제 반영
+ model
+ .updateTitle(candidate.getTitle())
+ .updateUrl(candidate.getUrl())
+ .updateStartAt(candidate.getStartAt())
+ .updateEndAt(candidate.getEndAt())
+ .updateType(command.type());
}또한 CalendarUpdateCommand의 필드가 부분 업데이트를 허용(Null 가능)하는지에 따라 DateConverter.toLocalDateTime(...)의 NPE 위험이 있습니다. DTO의 nullability를 확인해 주세요.
검증 스크립트:
🏁 Script executed:
#!/bin/bash
# CalendarUpdateCommand/CalendarCreateCommand 정의와 필드 제약 확인
rg -nP -C3 'record\s+Calendar(Update|Create)Command|class\s+Calendar(Update|Create)Command' eeos | sed -n '1,200p'
rg -nP -C2 '@NotNull|@NotBlank|@Positive|@PositiveOrZero|@Min|@Max' eeos/src/main/java/com/blackcompany/eeos/calendar/application/dto || trueLength of output: 1364
유효성 검사 순서 및 NPE 위험 수정 필요 (CalendarCommandService.java:70–86)
- updateCalendar 메소드: 기존 model 상태로 typeValidate/durationValidate 수행 후 command 값 반영 → 새 값으로 검증되지 않아 유효성 누락 가능. command 기반으로 생성한 candidate 모델에서 typeValidate, durationValidate 수행 후 실제 model에 적용하도록 순서 변경 필요.
- CalendarUpdateCommand(record)의 startAt, endAt(Long) 필드에 @NotNull 등 제약 미적용 → null 입력 시 DateConverter.toLocalDateTime 호출하면서 NPE 발생 위험. DTO에 nullability 명시(@NotNull) 또는 메소드 호출 전 null 체크·디폴트 처리 로직 추가 요망.
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarCommandService.java
around lines 70 to 86, change the validation/apply order and add null-safety:
build a candidate CalendarModel (or clone the existing model) populated with
command values (title, url, type, startAt, endAt) and run
validator.updateValidate/ typeValidate/ durationValidate against that candidate;
only if validations pass, apply those values to the real model. Also guard
DateConverter.toLocalDateTime(command.startAt()) and
toLocalDateTime(command.endAt()) against null (either add @NotNull to the
CalendarUpdateCommand DTO fields or add explicit null checks and throw a clear
validation exception) before conversion to prevent NPEs.
| boolean dateIsNull = Objects.isNull(date); | ||
|
|
||
| LocalDate startDate = LocalDate.of(year, month, dateIsNull ? 1 : date); | ||
|
|
||
| LocalDateTime start = LocalDateTime.of(startDate, LocalTime.MIDNIGHT); | ||
| LocalDateTime end = | ||
| dateIsNull | ||
| ? LocalDateTime.of( | ||
| LocalDate.of(year, month, DateUtil.getLastDay(year, month)), LocalTime.MIDNIGHT) | ||
| : startDate.plusDays(duration).atTime(LocalTime.MIDNIGHT); | ||
|
|
There was a problem hiding this comment.
duration null 시 NPE 발생 가능.
Controller에서 duration이 optional(required=false)입니다. date가 있고 duration이 null이면 plusDays(duration)에서 NPE가 납니다. 기본값 0을 적용하거나 명시적 검증이 필요합니다(위 경계 수정 diff에 반영).
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java
around lines 33 to 43, duration can be null (controller marks it optional) which
causes a NPE at plusDays(duration); ensure duration is null-safe by mapping null
to a default (e.g., 0) or validating and throwing a clear exception; replace
direct use of duration with a non-null local value (duration == null ? 0 :
duration) and use that for plusDays/end-date calculation or explicitly check and
reject null with a descriptive IllegalArgumentException.
| public List<CalendarResponse> getCalendarForDDay(int DDay) { | ||
|
|
||
| LocalDateTime now = LocalDate.now().atTime(LocalTime.MIDNIGHT); | ||
| LocalDateTime end = now.plusDays(DDay); | ||
|
|
||
| return repository.findNotStarted(now, end).stream().map(this::createResponse).toList(); | ||
| } |
There was a problem hiding this comment.
D-Day 조회도 동일한 경계 이슈 및 DDay=0 케이스 처리.
현재 now.plusDays(DDay)는 “자정”으로 끝나므로 당일이 제외될 수 있습니다. end를 해당 날짜의 끝으로 올려주세요.
- LocalDateTime end = now.plusDays(DDay);
+ LocalDateTime end = now.toLocalDate().plusDays(DDay).atTime(LocalTime.MAX);📝 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.
| public List<CalendarResponse> getCalendarForDDay(int DDay) { | |
| LocalDateTime now = LocalDate.now().atTime(LocalTime.MIDNIGHT); | |
| LocalDateTime end = now.plusDays(DDay); | |
| return repository.findNotStarted(now, end).stream().map(this::createResponse).toList(); | |
| } | |
| public List<CalendarResponse> getCalendarForDDay(int DDay) { | |
| LocalDateTime now = LocalDate.now().atTime(LocalTime.MIDNIGHT); | |
| LocalDateTime end = now.toLocalDate().plusDays(DDay).atTime(LocalTime.MAX); | |
| return repository.findNotStarted(now, end).stream().map(this::createResponse).toList(); | |
| } |
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/application/service/CalendarQueryService.java
around lines 48 to 54, the end boundary uses now.plusDays(DDay) set to midnight
which can exclude the target day (and fails for DDay=0); change the range to
start at the day's start and end at the target day's end — e.g. compute start =
LocalDate.now().atStartOfDay() and end =
LocalDate.now().plusDays(DDay).atTime(LocalTime.MAX) (or equivalent) so the
repository query includes the full target day.
| @PutMapping("/{calendarId}") | ||
| public ApiResponse<SuccessBody<Long>> updateCalendar( | ||
| @PathVariable("calendarId") Long calendarId, @RequestBody CalendarUpdateCommand command) { | ||
| Long id = updateUsecase.update(calendarId, command); | ||
| return ApiResponseGenerator.success(id, HttpStatus.OK, MessageCode.UPDATE); | ||
| } |
There was a problem hiding this comment.
업데이트 요청 본문에도 @Valid 적용.
- public ApiResponse<SuccessBody<Long>> updateCalendar(
- @PathVariable("calendarId") Long calendarId, @RequestBody CalendarUpdateCommand command) {
+ public ApiResponse<SuccessBody<Long>> updateCalendar(
+ @PathVariable("calendarId") Long calendarId, @RequestBody @jakarta.validation.Valid CalendarUpdateCommand command) {📝 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.
| @PutMapping("/{calendarId}") | |
| public ApiResponse<SuccessBody<Long>> updateCalendar( | |
| @PathVariable("calendarId") Long calendarId, @RequestBody CalendarUpdateCommand command) { | |
| Long id = updateUsecase.update(calendarId, command); | |
| return ApiResponseGenerator.success(id, HttpStatus.OK, MessageCode.UPDATE); | |
| } | |
| @PutMapping("/{calendarId}") | |
| public ApiResponse<SuccessBody<Long>> updateCalendar( | |
| @PathVariable("calendarId") Long calendarId, @RequestBody @jakarta.validation.Valid CalendarUpdateCommand command) { | |
| Long id = updateUsecase.update(calendarId, command); | |
| return ApiResponseGenerator.success(id, HttpStatus.OK, MessageCode.UPDATE); | |
| } |
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java
around lines 58 to 63, the update endpoint's request body is not validated;
annotate the CalendarUpdateCommand parameter with @Valid (add the
javax.validation.Valid import if missing) so Spring performs bean validation on
the incoming payload, and ensure the CalendarUpdateCommand has appropriate
validation annotations on its fields; optionally handle
MethodArgumentNotValidException globally or add a BindingResult parameter if you
need per-controller error handling.
| @GetMapping("/d-day") | ||
| public ApiResponse<SuccessBody<CalendarResponses>> getCalendarsForDDay( | ||
| @RequestParam("measure") @PositiveOrZero int measure) { | ||
| List<CalendarResponse> calendars = getUsecase.getCalendarForDDay(measure); | ||
| CalendarResponses responses = new CalendarResponses(calendars); | ||
| return ApiResponseGenerator.success(responses, HttpStatus.OK, MessageCode.GET); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
DDay 파라미터 검증이 실제 동작하는지 확인.
@PositiveOrZero는 위에서 제안한 @Validated 추가 시에만 보장됩니다. 반영 후 수동/통합 테스트로 400 응답이 나는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# 컨트롤러 클래스에 @Validated, 메서드에 @Valid 적용 여부 확인
rg -n 'class CalendarController' -n -C2 eeos/src/main/java
rg -n '@Validated' eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java
rg -n '@Valid' eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.javaLength of output: 632
@validated 누락 — D-Day 파라미터 검증이 동작하지 않습니다
CalendarController 클래스에 @validated가 선언되어 있지 않습니다. 클래스 선언부 위에 @validated를 추가하고, measure에 음수 값을 전송했을 때 400 응답이 반환되는지 수동/통합 테스트로 확인하세요.
위치: eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/calendar/presentation/controller/CalendarController.java
around lines 72 to 78, the controller is missing @Validated on the class so the
@PositiveOrZero validation on the measure request param won't run; add the
org.springframework.validation.annotation.Validated annotation to the class
declaration (and import it), then re-run or add a manual/integration test that
sends a negative measure and asserts a 400 Bad Request is returned to verify
validation is active.
eeos/src/main/java/com/blackcompany/eeos/common/utils/DateConverter.java
Show resolved
Hide resolved
eeos/src/main/java/com/blackcompany/eeos/program/persistence/SemesterPeriodEntity.java
Show resolved
Hide resolved
| @PutMapping("/admin/semester-periods") // TODO : 인터셉터 단에서 어드민 검증 | ||
| public ApiResponse<ApiResponseBody.SuccessBody<SemesterPeriodApplicationQuery>> | ||
| updateSemesterPeriod(@RequestBody @Valid UpdateSemesterPeriodRequest request) { | ||
| SemesterPeriodApplicationQuery response = | ||
| updateSemesterPeriodUsecase.updateSemesterPeriod( | ||
| new SemesterPeriodApplicationCommand(request.startDate(), request.endDate())); | ||
| return ApiResponseGenerator.success(response, HttpStatus.CREATED, MessageCode.CREATE); | ||
| } |
There was a problem hiding this comment.
관리자 검증 TODO는 노출 API 특성상 선반영을 권장.
관리자만 접근 가능한 엔드포인트입니다. 인터셉터/어노테이션 기반 권한 검증을 반드시 연결하고, 누락 시 릴리스 차단을 권장합니다.
🤖 Prompt for AI Agents
In
eeos/src/main/java/com/blackcompany/eeos/program/presentation/controller/SemesterPeriodController.java
around lines 24 to 31, the endpoint is admin-only but currently lacks any
authorization enforcement; add a concrete admin permission check (for example
annotate the controller or method with your project's security annotation such
as @PreAuthorize("hasRole('ADMIN')") or apply the existing interceptor that
enforces admin role) so only admins can call PUT /admin/semester-periods, wire
the same check into the security configuration if needed, and add/adjust
unit/integration tests to cover allowed and denied access cases before merging.
📌 관련 이슈
#277
✒️ 작업 내용
Department Enum 클래스 추가
회원이 속한 부서 정보 변경 API 엔드 포인트 추가
부서 리스트 API 엔드포인트 추가
사용자 리스트 조회 API 엔드포인트 수정
스크린샷 🏞️ (선택)
💬 REVIEWER에게 요구사항 💬
Summary by CodeRabbit
신규 기능
보안
오류 처리