feature/#174 Carousel 관리자용 API 및 리스트 조회 API 구현 #175
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 풀 리퀘스트는 캐러셀(Carousel) 관련 기능을 위한 새로운 API와 서비스를 구현합니다. 관리자용 API와 일반 사용자용 조회 API를 포함하며, 캐러셀 생성, 삭제, 조회 기능을 제공합니다. 도메인, API, 관리자 모듈에 걸쳐 새로운 클래스와 인터페이스를 추가하여 캐러셀 관리 기능을 종합적으로 구현했습니다. Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Copilot reviewed 5 out of 19 changed files in this pull request and generated no comments.
Files not reviewed (14)
- aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileRepositoryImpl.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/file/domain/FileRepository.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/carousel/domain/CarouselRepository.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java: Evaluated as low risk
- aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/request/CarouselRequest.java: Evaluated as low risk
- aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselControllerImpl.java: Evaluated as low risk
- aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/response/CarouselPersistResponse.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/JpaCarouselRepository.java: Evaluated as low risk
- aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java: Evaluated as low risk
- aics-admin/src/main/java/kgu/developers/admin/carousel/application/CarouselAdminFacade.java: Evaluated as low risk
- aics-api/src/main/java/kgu/developers/api/carousel/application/CarouselFacade.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/CarouselRepositoryImpl.java: Evaluated as low risk
- aics-domain/src/main/java/kgu/developers/domain/carousel/application/query/CarouselQueryService.java: Evaluated as low risk
Comments suppressed due to low confidence (1)
aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java:33
- Add a null check for the
requestparameter to prevent potential NullPointerException. Suggested change:if (request == null) { return ResponseEntity.badRequest().build(); }
CarouselPersistResponse response = carouselAdminFacade.createCarousel(fileId, request);
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java (1)
15-17: 로깅 추가를 고려해보세요.파일을 찾지 못했을 때의 예외 상황과 파일 조회 성공 시의 상황을 로깅하면 운영 시 문제 해결에 도움이 될 것 같습니다.
다음과 같이 로깅을 추가하는 것을 제안드립니다:
+import lombok.extern.slf4j.Slf4j; @Service @RequiredArgsConstructor +@Slf4j public class FileQueryService { private final FileRepository fileRepository; public FileEntity getFileById(Long id) { - return fileRepository.findById(id).orElseThrow(FileNotFoundException::new); + return fileRepository.findById(id) + .map(file -> { + log.info("파일을 성공적으로 조회했습니다. fileId={}", id); + return file; + }) + .orElseThrow(() -> { + log.warn("파일을 찾을 수 없습니다. fileId={}", id); + return new FileNotFoundException(); + }); } }aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java (1)
40-46: 정적 팩토리 메서드 구현이 깔끔합니다.
create메서드를 통해 객체 생성 로직을 캡슐화한 것이 좋습니다. 하지만 몇 가지 개선사항을 제안드립니다:public static Carousel create(String text, String link, FileEntity file) { + validateInput(text, link, file); return Carousel.builder() .text(text) .link(link) .file(file) .build(); } + +private static void validateInput(String text, String link, FileEntity file) { + if (text == null || text.trim().isEmpty()) { + throw new IllegalArgumentException("텍스트는 필수 입력값입니다."); + } + if (link == null || link.trim().isEmpty()) { + throw new IllegalArgumentException("링크는 필수 입력값입니다."); + } + if (file == null) { + throw new IllegalArgumentException("파일은 필수 입력값입니다."); + } +}입력값 검증 로직을 추가하여 더 안정적인 객체 생성을 보장하는 것이 좋겠습니다.
aics-domain/src/main/java/kgu/developers/domain/carousel/domain/CarouselRepository.java (1)
5-11: 메서드 문서화 추가 권장각 메서드의 동작과 매개변수에 대한 명확한 이해를 위해 JavaDoc 문서화를 추가하는 것이 좋습니다.
예시:
public interface CarouselRepository { + /** + * 캐러셀 엔티티를 저장합니다. + * @param carousel 저장할 캐러셀 엔티티 + * @return 저장된 캐러셀 엔티티 + */ Carousel save(Carousel carousel); + /** + * 지정된 ID의 캐러셀을 삭제합니다. + * @param id 삭제할 캐러셀의 ID + */ void deleteById(Long id); + /** + * 파일이 존재하는 모든 캐러셀을 생성일 기준 내림차순으로 조회합니다. + * @return 정렬된 캐러셀 목록 + */ List<Carousel> findAllByFileIsNotNullOrderByCreatedAtDesc(); }aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/JpaCarouselRepository.java (1)
9-11: 성능 최적화 제안
createdAt필드에 대한 인덱스 추가를 고려해보세요. 캐러셀 목록이 많아질 경우 정렬 성능이 저하될 수 있습니다.+@Index(name = "idx_carousel_created_at", columnList = "createdAt") public interface JpaCarouselRepository extends JpaRepository<Carousel, Long> { List<Carousel> findAllByFileIsNotNullOrderByCreatedAtDesc(); }aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/CarouselRepositoryImpl.java (2)
16-19: @transactional 어노테이션 추가를 고려해주세요.데이터 일관성을 보장하기 위해 저장 작업에
@Transactional어노테이션 추가를 권장드립니다.
26-29: 메서드 이름 리팩토링을 고려해주세요.현재 메서드 이름이 매우 길어 가독성이 떨어집니다.
findAllActiveOrderByCreatedAtDesc()와 같이 더 간단한 이름을 고려해보시는 것은 어떨까요?aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselController.java (1)
15-22: 페이지네이션 지원 추가를 고려해주세요.리스트 조회 API의 경우, 데이터가 많아질 것을 대비하여 페이지네이션 지원을 추가하는 것이 좋습니다. 이는 서버 리소스 관리와 클라이언트 성능에 도움이 될 것입니다.
예시 변경사항:
- ResponseEntity<CarouselListResponse> getCarousels(); + ResponseEntity<CarouselListResponse> getCarousels( + @Parameter(description = "페이지 번호 (0부터 시작)") + @RequestParam(defaultValue = "0") int page, + @Parameter(description = "페이지 크기") + @RequestParam(defaultValue = "10") int size + );aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java (1)
29-35: 성공 응답 코드 201로 변경 권장리소스 생성 엔드포인트의 경우 HTTP 201 Created를 반환하는 것이 REST 규약에 더 부합합니다.
다음과 같이 수정하는 것을 권장드립니다:
- return ResponseEntity.ok(response); + return ResponseEntity.created(URI.create("/api/v1/carousels/" + response.id())).body(response);aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java (1)
22-25: API 문서 개선 제안API 문서에 다음 정보들을 추가하면 좋을 것 같습니다:
- 입력값 제약조건
- 에러 응답 케이스
- 권한 요구사항
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
aics-admin/src/main/java/kgu/developers/admin/carousel/application/CarouselAdminFacade.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/request/CarouselRequest.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/response/CarouselPersistResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/application/CarouselFacade.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselController.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselControllerImpl.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselListResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselResponse.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/application/query/CarouselQueryService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/domain/CarouselRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/CarouselRepositoryImpl.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/JpaCarouselRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/domain/FileRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileRepositoryImpl.java(2 hunks)
🔇 Additional comments (15)
aics-domain/src/main/java/kgu/developers/domain/file/domain/FileRepository.java (1)
3-7: 구현이 깔끔하고 적절합니다!Optional을 사용하여 null 처리를 안전하게 구현한 점이 좋습니다. Spring Data JPA의 관행을 잘 따르고 있습니다.
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileRepositoryImpl.java (1)
20-23: 구현이 적절합니다!JpaFileRepository의 findById 메소드를 잘 활용하여 깔끔하게 구현되었습니다.
aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java (2)
1-3: 패키지 구조가 적절합니다!도메인 중심의 패키지 구조를 잘 따르고 있으며, 필요한 CascadeType.REMOVE import를 명시적으로 추가한 것이 좋습니다.
36-38: 캐스케이드 설정 변경이 적절합니다.
@OneToOne(cascade = REMOVE)설정을 통해 Carousel 엔티티가 삭제될 때 연관된 FileEntity도 함께 삭제되도록 처리한 것이 리소스 관리 측면에서 매우 적절합니다.하지만 다음 사항들을 확인해 주시기 바랍니다:
- FileEntity가 다른 엔티티에서도 참조되고 있지는 않은지
- 삭제 시 실제 물리적 파일도 함께 정리되는지
aics-api/src/main/java/kgu/developers/api/carousel/application/CarouselFacade.java (1)
17-20: 구현이 깔끔합니다!Facade 패턴을 잘 활용하여 캐러셀 조회 로직을 단순하고 명확하게 구현하였습니다.
aics-admin/src/main/java/kgu/developers/admin/carousel/application/CarouselAdminFacade.java (1)
15-18: 🛠️ Refactor suggestion파일 ID 유효성 검증이 필요합니다.
fileId가 유효하지 않은 경우에 대한 처리가 없습니다. 존재하지 않는 파일 ID가 전달될 경우 적절한 예외 처리가 필요합니다.다음과 같은 검증 로직 추가를 제안드립니다:
public CarouselPersistResponse createCarousel(Long fileId, CarouselRequest request) { + if (fileId == null) { + throw new IllegalArgumentException("파일 ID는 필수 값입니다."); + } Long id = carouselCommandService.createCarousel(fileId, request.text(), request.link()); return CarouselPersistResponse.of(id); }aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselController.java (1)
15-18: API 문서화가 잘 되어있습니다!OpenAPI 명세를 활용한 API 문서화가 명확하고 상세하게 작성되어 있습니다.
aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselControllerImpl.java (1)
12-24: 구현이 깔끔하고 적절합니다!컨트롤러의 구현이 Spring 프레임워크의 모범 사례를 잘 따르고 있으며, 책임 분리가 명확합니다.
aics-domain/src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java (1)
17-21:⚠️ Potential issue예외 처리 로직이 필요합니다
fileQueryService.getFileById()가 실패할 경우에 대한 예외 처리가 없습니다. 파일이 존재하지 않을 경우 적절한 예외 처리가 필요합니다.다음과 같이 수정하는 것을 제안드립니다:
public Long createCarousel(Long fileId, String text, String link) { - FileEntity file = fileQueryService.getFileById(fileId); + try { + FileEntity file = fileQueryService.getFileById(fileId); + Carousel carousel = Carousel.create(text, link, file); + return carouselRepository.save(carousel).getId(); + } catch (EntityNotFoundException e) { + throw new BusinessException(ErrorCode.FILE_NOT_FOUND); + } - Carousel carousel = Carousel.create(text, link, file); - return carouselRepository.save(carousel).getId(); }aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselListResponse.java (1)
11-35: 응답 구조와 문서화가 잘 되어있습니다!Swagger 문서화가 상세하게 되어있고, 예시도 실제 사용 사례에 맞게 잘 작성되어 있습니다.
aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselResponse.java (1)
28-35: null 체크 로직이 적절합니다!파일이 null인 경우에 대한 처리가 잘 되어 있습니다. 이는 PR 목표에서 언급된 "빈 이미지가 홈페이지에 표시되는 것을 방지"하는 요구사항을 잘 충족시키고 있습니다.
aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java (1)
31-32: request 매개변수의 required 속성 검토 필요
CarouselRequest가required = false로 설정되어 있는데, 이는 클라이언트가 빈 요청을 보낼 수 있다는 것을 의미합니다. 이것이 의도된 동작인지 확인이 필요합니다.빈 요청 처리에 대한 유닛 테스트를 작성하는 것을 도와드릴까요?
aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java (1)
19-52: LGTM! API 설계가 잘 되어있습니다.
- REST 규약을 잘 준수하고 있습니다
- OpenAPI 문서화가 잘 되어있습니다
- 매개변수 검증이 적절히 구현되어 있습니다
aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/response/CarouselPersistResponse.java (2)
8-12: 응답 객체 설계가 깔끔합니다.Record와 Builder 패턴을 적절히 활용하여 불변성을 보장하고 있습니다.
13-17: 정적 팩토리 메서드 구현이 좋습니다.
of메서드를 통한 객체 생성 방식이 가독성을 높여줍니다.
...-admin/src/main/java/kgu/developers/admin/carousel/presentation/request/CarouselRequest.java
Outdated
Show resolved
Hide resolved
...ain/src/main/java/kgu/developers/domain/carousel/application/query/CarouselQueryService.java
Show resolved
Hide resolved
...src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java
Show resolved
Hide resolved
aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselResponse.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java (1)
15-17: 예외 처리 개선을 고려해보세요.현재 구현은 기본적인 예외 처리만 되어있습니다. 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
- 예외 메시지에 파일 ID를 포함
- 로깅 추가
public FileEntity getFileById(Long id) { - return fileRepository.findById(id).orElseThrow(FileNotFoundException::new); + return fileRepository.findById(id) + .orElseThrow(() -> { + log.error("파일을 찾을 수 없습니다. ID: {}", id); + return new FileNotFoundException("파일을 찾을 수 없습니다. (ID: " + id + ")"); + }); }aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselListResponse.java (1)
11-27: 응답 구조가 잘 설계되었습니다!스키마 문서화가 잘 되어있고 불변성을 보장하는 record를 사용한 점이 좋습니다.
예시 JSON에서 들여쓰기가 일관적이지 않습니다. 아래와 같이 수정하는 것을 추천드립니다:
[{ "id": 1, "text": "경기대학교 AI컴퓨터공학부 메인 이미지", - "link": "https://www.kgu.ac.kr/", - "file": { - "id": 1, - "physicalPath": "/cloud/carousel/3/2025-curriculum" - } + "link": "https://www.kgu.ac.kr/", + "file": { + "id": 1, + "physicalPath": "/cloud/carousel/3/2025-curriculum" + } }]aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/JpaCarouselRepository.java (1)
9-11: 메소드 문서화 추가 제안
findAllByFileIsNotNullOrderByCreatedAtDesc()메소드에 JavaDoc 주석을 추가하여 메소드의 목적과 반환값을 명확히 설명하면 좋을 것 같습니다.+/** + * file이 null이 아닌 모든 Carousel을 생성일자 내림차순으로 조회합니다. + * + * @return file이 존재하는 Carousel 목록 (최신순) + */ List<Carousel> findAllByFileIsNotNullOrderByCreatedAtDesc();aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/response/CarouselPersistResponse.java (1)
8-18: 단일 필드 레코드의 Builder 패턴 사용 최적화단일 필드를 가진 레코드의 경우 Builder 패턴이 불필요한 복잡성을 추가할 수 있습니다. 다음과 같이 단순화하는 것을 고려해보세요:
-@Builder public record CarouselPersistResponse( @Schema(description = "캐러셀 ID", example = "1", requiredMode = REQUIRED) Long id ) { public static CarouselPersistResponse of(Long id) { - return CarouselPersistResponse.builder() - .id(id) - .build(); + return new CarouselPersistResponse(id); } }aics-domain/src/main/java/kgu/developers/domain/carousel/application/query/CarouselQueryService.java (1)
16-18: 예외 처리 및 페이지네이션 구현 필요
- Repository 작업 시 발생할 수 있는 예외 처리가 누락되어 있습니다.
- 캐러셀 데이터가 많아질 경우를 대비하여 페이지네이션 도입을 고려해주세요.
다음과 같이 개선하는 것을 제안드립니다:
- public List<Carousel> getAllCarousels() { - return carouselRepository.findAllByFileIsNotNullOrderByCreatedAtDesc(); + public List<Carousel> getAllCarousels() { + try { + return carouselRepository.findAllByFileIsNotNullOrderByCreatedAtDesc(); + } catch (Exception e) { + log.error("캐러셀 조회 중 오류 발생", e); + throw new CarouselQueryException("캐러셀 조회 중 오류가 발생했습니다.", e); + } + } + + public Page<Carousel> getAllCarouselsWithPagination(Pageable pageable) { + try { + return carouselRepository.findAllByFileIsNotNullOrderByCreatedAtDesc(pageable); + } catch (Exception e) { + log.error("캐러셀 페이지 조회 중 오류 발생", e); + throw new CarouselQueryException("캐러셀 조회 중 오류가 발생했습니다.", e); + } + }aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/CarouselRepositoryImpl.java (2)
21-24: 삭제 정책 개선 필요물리적 삭제 대신 소프트 삭제 도입을 고려해주세요. 이는 데이터 복구와 감사 추적에 도움이 됩니다.
다음과 같이 개선하는 것을 제안드립니다:
+ @Transactional @Override public void deleteById(Long id) { - jpaCarouselRepository.deleteById(id); + Carousel carousel = jpaCarouselRepository.findById(id) + .orElseThrow(() -> new CarouselNotFoundException("캐러셀을 찾을 수 없습니다.")); + carousel.softDelete(); + jpaCarouselRepository.save(carousel); }
26-29: 성능 최적화 고려 필요파일이 없는 캐러셀을 필터링하는 로직을 인덱스를 활용하도록 개선하면 성능을 향상시킬 수 있습니다.
+ @Transactional(readOnly = true) @Override public List<Carousel> findAllByFileIsNotNullOrderByCreatedAtDesc() { return jpaCarouselRepository.findAllByFileIsNotNullOrderByCreatedAtDesc(); }aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselController.java (1)
15-18: API 문서화 개선이 필요합니다API 문서화에 다음 내용을 추가하면 좋을 것 같습니다:
- 응답 데이터의 구조와 각 필드에 대한 설명
- 발생 가능한 에러 케이스와 해당 응답 코드
- 페이지네이션 관련 정보 (필요한 경우)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
aics-admin/src/main/java/kgu/developers/admin/carousel/application/CarouselAdminFacade.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/request/CarouselRequest.java(1 hunks)aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/response/CarouselPersistResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/application/CarouselFacade.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselController.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/CarouselControllerImpl.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselListResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselResponse.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/application/query/CarouselQueryService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java(2 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/domain/CarouselRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/CarouselRepositoryImpl.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/carousel/infrastructure/JpaCarouselRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/domain/FileRepository.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileRepositoryImpl.java(2 hunks)
🔇 Additional comments (12)
aics-domain/src/main/java/kgu/developers/domain/file/domain/FileRepository.java (1)
7-7: 구현이 적절합니다!표준 Spring Data JPA 패턴을 따르는 메소드 시그니처이며, Optional을 사용하여 null 처리를 안전하게 구현했습니다.
aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java (1)
10-14: 아키텍처 구현이 잘 되었습니다!생성자 주입을 통한 의존성 관리와 계층 분리가 잘 되어있습니다.
aics-domain/src/main/java/kgu/developers/domain/file/infrastructure/FileRepositoryImpl.java (1)
20-23: 구현이 깔끔합니다!JPA 리포지토리로의 위임이 명확하게 구현되어 있습니다.
aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselListResponse.java (1)
28-34: 도메인 객체 변환 로직이 깔끔합니다!스트림 API를 활용하여 간결하고 함수형 스타일로 구현한 점이 좋습니다.
aics-api/src/main/java/kgu/developers/api/carousel/presentation/response/CarouselResponse.java (2)
22-26: 파일 경로 노출에 대한 보안 검토가 필요합니다.스키마 예시에 물리적 파일 경로가 노출되어 있습니다. 이는 잠재적인 보안 위험이 될 수 있습니다.
보안을 위해 다음 사항들을 확인해주세요:
- 물리적 파일 경로가 외부에 노출되어도 안전한지
- 파일 접근 권한이 적절히 설정되어 있는지
파일 경로를 해시화하거나 별도의 접근 URL을 사용하는 것을 고려해보세요.
28-35: Carousel과 FileEntity 간의 관계 검증이 필요합니다.PR 목표에 따르면 Carousel이 삭제될 때 FileEntity도 함께 삭제되도록
@OneToOne(cascade = REMOVE)설정이 추가되었습니다. 이 변경사항이 올바르게 적용되었는지 확인이 필요합니다.다음 스크립트를 실행하여 도메인 엔티티의 관계 설정을 확인해주세요:
✅ Verification successful
Carousel과 FileEntity 간의 cascade 관계가 올바르게 설정되었습니다.
검증 결과:
@OneToOne(cascade = REMOVE)설정이 정확하게 구현되어 있습니다.- Carousel 엔티티가 삭제될 때 연관된 FileEntity도 자동으로 삭제됩니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Carousel 엔티티에서 FileEntity와의 관계 설정 확인 ast-grep --pattern 'class Carousel { $$$ @OneToOne(cascade = $_) $_ $$$ }'Length of output: 84
Script:
#!/bin/bash # Find Carousel entity file fd Carousel.java # Once found, examine its content echo "=== Carousel Entity Content ===" rg -A 5 "@OneToOne" --type javaLength of output: 2055
aics-domain/src/main/java/kgu/developers/domain/carousel/domain/CarouselRepository.java (1)
5-11: 구현이 잘 되어있습니다!Spring Data JPA 네이밍 컨벤션을 잘 따르고 있으며, 메소드들이 명확하게 정의되어 있습니다.
aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java (1)
36-38: @OnetoOne 관계 설정이 적절합니다cascade = REMOVE 설정을 통해 캐러셀 삭제 시 연관된 파일도 함께 삭제되도록 한 것이 좋습니다.
aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java (2)
20-24: 보안 설정과 기본 경로가 적절히 구성되었습니다!관리자 전용 API에 대한 보안 설정(
@PreAuthorize)과 REST 기반의 경로 설정이 잘 되어있습니다.
37-44: 삭제 API가 REST 표준을 잘 준수하고 있습니다!올바른 응답 코드(204)를 사용하고 있으며, 경로 변수에 대한 유효성 검사도 적절히 구현되어 있습니다.
aics-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java (2)
19-20: API 문서화가 잘 되어있습니다!OpenAPI 어노테이션을 사용한 문서화가 명확하고 상세하게 작성되어 있습니다.
40-51: 삭제 API의 명세가 명확하게 작성되어 있습니다!파라미터 설명과 응답 코드가 적절하게 문서화되어 있습니다.
...-admin/src/main/java/kgu/developers/admin/carousel/presentation/request/CarouselRequest.java
Outdated
Show resolved
Hide resolved
aics-api/src/main/java/kgu/developers/api/carousel/application/CarouselFacade.java
Show resolved
Hide resolved
aics-admin/src/main/java/kgu/developers/admin/carousel/application/CarouselAdminFacade.java
Show resolved
Hide resolved
aics-admin/src/main/java/kgu/developers/admin/carousel/application/CarouselAdminFacade.java
Show resolved
Hide resolved
...main/src/main/java/kgu/developers/domain/carousel/infrastructure/CarouselRepositoryImpl.java
Show resolved
Hide resolved
...src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java
Show resolved
Hide resolved
...src/main/java/kgu/developers/domain/carousel/application/command/CarouselCommandService.java
Show resolved
Hide resolved
aics-domain/src/main/java/kgu/developers/domain/carousel/domain/Carousel.java
Show resolved
Hide resolved
...in/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminControllerImpl.java
Outdated
Show resolved
Hide resolved
...-admin/src/main/java/kgu/developers/admin/carousel/presentation/CarouselAdminController.java
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Test Coverage Report
Files
|
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## develop #175 +/- ##
=============================================
- Coverage 94.07% 89.94% -4.13%
Complexity 129 129
=============================================
Files 44 49 +5
Lines 371 388 +17
Branches 4 4
=============================================
Hits 349 349
- Misses 17 34 +17
Partials 5 5
Continue to review full report in Codecov by Sentry.
|
Summary
Carousel 관리자용 API 및 리스트 조회 API 구현
Tasks
To Reviewer
Carousel FileEntity간 연관관계 매핑 변경
기존 Carousel, FileEntity간 연관관계는 OneToOne으로 설정 되어 있었습니다. 이때 Carousel 객체가 삭제할 경우, 여전히 캐러셀 이미지인 FileEntity 객체가 서버에 존재하여 서버 리소스를 낭비할 우려가 있었습니다. 따라서 이를 방지하고자
@OneToOne(cascade = REMOVE)를 적용하여 Carousel 객체가 삭제되면 Carousel과의 관계가 끊어진 FileEntity를 자동으로 삭제하도록 변경하였습니다.Carousel 조회 규칙
AICS 커뮤니티 홈에서 Carousel을 조회할 때 순서를 지정하자는 의견도 존재하였으나, 이는 비즈니스 로직을 복잡하게 만들 우려가 있어요. 따라서 가장 최근에 업로드한 이미지를 맨 앞에 배치하도록 하였습니다. 또한 첨부 이미지가 존재하지 않을 때 빈 이미지가 홈화면에 뜨는 상황을 방지하기 위해 file의 null 여부 체크를 추가하였습니다.