refactor/#261 Facade에서 FileEntity 대신 File의physicalPath를 사용하도록 수정#262
Merged
refactor/#261 Facade에서 FileEntity 대신 File의physicalPath를 사용하도록 수정#262
Conversation
Contributor
Walkthrough클럽/랩 목록 조회 흐름에서 파일 매핑 타입을 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Coverage Report
Files
|
1 task
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## develop #262 +/- ##
==========================================
Coverage 92.98% 92.98%
Complexity 160 160
==========================================
Files 49 49
Lines 456 456
Branches 6 6
==========================================
Hits 424 424
Misses 25 25
Partials 7 7
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
aics-api/src/main/java/kgu/developers/api/club/presentation/response/ClubDetailResponse.java (1)
26-28: Swagger 예시(JSON)와 실제 응답 스키마를 일치시켜 주세요현재 예시는
physicalPath만 담고 있으나, 실제FilePathResponse는id와physicalPath를 함께 포함합니다. 문서-실제 응답 불일치를 해소해 주세요.- @Schema(description = "첨부 파일 정보", - example = "{\"physicalPath\": \"/cloud/file/3/2025-curriculum\"}", - requiredMode = NOT_REQUIRED) + @Schema(description = "첨부 파일 정보", + example = "{\"id\": 3, \"physicalPath\": \"/cloud/file/3/2025-curriculum\"}", + requiredMode = NOT_REQUIRED)
🧹 Nitpick comments (8)
aics-api/src/main/java/kgu/developers/api/lab/application/LabFacade.java (1)
31-31: 중복 ID 제거 및 가독성 개선 제안fileIds에 distinct()를 적용해 불필요한 조회/매핑을 줄이고, 콤마 뒤 공백을 추가해 컨벤션을 맞추면 좋겠습니다.
아래처럼 변경을 제안합니다.
- List<Long> fileIds = labs.stream() - .map(Lab::getFileId) - .filter(Objects::nonNull) - .toList(); + List<Long> fileIds = labs.stream() + .map(Lab::getFileId) + .filter(Objects::nonNull) + .distinct() + .toList(); - return LabListResponse.from(labs,fileQueryService.findPhysicalPathMapByIds(fileIds)); + return LabListResponse.from(labs, fileQueryService.findPhysicalPathMapByIds(fileIds));aics-api/src/main/java/kgu/developers/api/club/application/ClubFacade.java (1)
30-30: 중복 ID 제거 및 가독성 개선 제안동일한 개선을 클럽에도 적용해주세요. distinct()로 조회 안정성을 높이고, 콤마 뒤 공백을 추가하는 것이 좋습니다.
- List<Long> fileIds = clubs.stream() - .map(Club::getFileId) - .filter(Objects::nonNull) - .toList(); + List<Long> fileIds = clubs.stream() + .map(Club::getFileId) + .filter(Objects::nonNull) + .distinct() + .toList(); - return ClubListResponse.from(clubs,fileQueryService.findPhysicalPathMapByIds(fileIds)); + return ClubListResponse.from(clubs, fileQueryService.findPhysicalPathMapByIds(fileIds));aics-api/src/main/java/kgu/developers/api/club/presentation/response/ClubListResponse.java (2)
8-8: 불필요한 import 제거FileEntity를 더 이상 사용하지 않으므로 import를 제거하세요.
-import kgu.developers.domain.file.domain.FileEntity;
25-31: 의미가 드러나는 파라미터명과 null 안전 처리 제안fileMap보다는 physicalPathMap이 더 명확합니다. 또한 매핑 누락 시 null 전달이 의도인지 확인 바랍니다. 필요하면 placeholder로 대체하는 예시는 아래와 같습니다.
- public static ClubListResponse from(List<Club> clubs, Map<Long,String> fileMap) { + public static ClubListResponse from(List<Club> clubs, Map<Long, String> physicalPathMap) { return ClubListResponse.builder() .contents(clubs.stream() - .map(club -> ClubDetailResponse.from(club, - fileMap.get(club.getFileId()))) + .map(club -> { + String path = physicalPathMap.get(club.getFileId()); + return ClubDetailResponse.from(club, path /* null 허용/대체 여부 확인 */); + }) .toList()) .build(); }aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java (1)
21-25: 빈 입력 최적화(미세 최적화)ids가 비어있으면 조기 반환하여 불필요한 DB 호출을 피할 수 있습니다.
- public Map<Long,String> findPhysicalPathMapByIds(List<Long> ids) { + public Map<Long, String> findPhysicalPathMapByIds(List<Long> ids) { + if (ids == null || ids.isEmpty()) { + return Map.of(); + } return fileRepository.findAllByIds(ids) .stream() .collect(Collectors.toMap(FileEntity::getId, FileEntity::getPhysicalPath)); }aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabListResponse.java (1)
26-31: 의미가 드러나는 파라미터명과 null 처리 확인Club과 동일하게 physicalPathMap으로 이름을 명확히 하고, 매핑 누락 시 처리 방식을 확인해주세요.
- public static LabListResponse from(List<Lab> labs, Map<Long, String> fileMap) { + public static LabListResponse from(List<Lab> labs, Map<Long, String> physicalPathMap) { return LabListResponse.builder() .contents(labs.stream() - .map(lab -> LabDetailResponse.from(lab,fileMap.get(lab.getFileId()))) + .map(lab -> LabDetailResponse.from(lab, physicalPathMap.get(lab.getFileId()))) .toList()) .build(); }aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabDetailResponse.java (1)
35-44: 물리 경로가 null/공백일 때 불완전한 응답 생성을 방지하세요
fileId는 있지만physicalPath가null/공백일 수 있는 경우를 가드하지 않으면, 불완전한FilePathResponse가 내려갈 수 있습니다. 안전하게 가드해 주세요.다음과 같이 생성 로직을 한 단계 풀어 가드를 추가하는 것을 제안합니다.
- .advisor(lab.getAdvisor()) - .img(lab.getFileId() != null ? FilePathResponse.of(lab.getFileId(), physicalPath) : null) + .advisor(lab.getAdvisor()) + .img(buildImg(lab, physicalPath)) .build(); } + + private static FilePathResponse buildImg(Lab lab, String physicalPath) { + if (lab.getFileId() == null) return null; + if (physicalPath == null || physicalPath.isBlank()) return null; + return FilePathResponse.of(lab.getFileId(), physicalPath); + }aics-api/src/main/java/kgu/developers/api/club/presentation/response/ClubDetailResponse.java (1)
31-38: physicalPath null/공백 가드로 응답 일관성 확보
club.getFileId()만 체크하면physicalPath가 누락된 상태로FilePathResponse를 만들 가능성이 있습니다. Lab과 동일하게 가드를 추가해 주세요.- .site(club.getSite()) - .file(club.getFileId() != null ? FilePathResponse.of(club.getFileId(), physicalPath) : null) + .site(club.getSite()) + .file(buildFile(club, physicalPath)) .build(); } + + private static FilePathResponse buildFile(Club club, String physicalPath) { + if (club.getFileId() == null) return null; + if (physicalPath == null || physicalPath.isBlank()) return null; + return FilePathResponse.of(club.getFileId(), physicalPath); + }
📜 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 (7)
aics-api/src/main/java/kgu/developers/api/club/application/ClubFacade.java(1 hunks)aics-api/src/main/java/kgu/developers/api/club/presentation/response/ClubDetailResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/club/presentation/response/ClubListResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/lab/application/LabFacade.java(1 hunks)aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabDetailResponse.java(1 hunks)aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabListResponse.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java(1 hunks)
🔇 Additional comments (5)
aics-api/src/main/java/kgu/developers/api/lab/application/LabFacade.java (1)
31-31: physicalPath 누락 시 처리 확인 필요fileMap.get(lab.getFileId())가 null이 될 수 있습니다. LabDetailResponse.from이 null physicalPath를 안전하게 처리(placeholder 등)하는지 확인 부탁드립니다.
aics-api/src/main/java/kgu/developers/api/club/application/ClubFacade.java (1)
30-30: physicalPath 누락 시 처리 확인 필요fileMap에서 매핑을 찾지 못하면 null이 전달됩니다. ClubDetailResponse.from이 null을 허용하는지(또는 기본 이미지로 대체하는지) 확인 바랍니다.
aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabListResponse.java (1)
21-23: public 노출 경로 점검예시의 "physicalPath"가 서버 내부 물리 경로가 아니라 클라이언트에 노출 가능한 공개 경로(또는 프록시/CDN 경로)인지 확인 바랍니다. 내부 경로 노출은 보안/운영 리스크가 있습니다.
aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabDetailResponse.java (1)
42-42: 의존 역전 정리 좋습니다Facade에서 엔티티를 숨기고 DTO가 경로 문자열만 받도록 한 점이 깔끔합니다. 호출자-응답 간 결합도가 줄었습니다.
aics-api/src/main/java/kgu/developers/api/club/presentation/response/ClubDetailResponse.java (1)
31-38: 엔티티 의존성 제거 방향 LGTMFacade → DTO 경계에서 엔티티 노출을 제거하고 경로 문자열로 치환한 방향은 타 계층과의 결합도를 줄여 유지보수성에 유리합니다.
| FilePathResponse img | ||
| ) { | ||
| public static LabDetailResponse from(Lab lab, FileEntity file) { | ||
| public static LabDetailResponse from(Lab lab, String physicalPath) { |
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
시그니처 변경(physicalPath) 반영 범위 확인 부탁
from(Lab, String)로 바뀐 만큼, 호출부에서 FileEntity를 넘기던 코드가 남아있지 않은지 확인 필요합니다. 또한 FilePathResponse.from(...) 사용이 모두 of(Long, String)으로 교체되었는지 점검해 주세요.
아래 스크립트로 잔여 사용처를 빠르게 확인할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# 남아있는 구 시그니처/팩토리 사용 탐지
rg -nP 'FilePathResponse\.from\s*\(' -C2
rg -nP 'LabDetailResponse\.from\s*\(' -C2
rg -nP 'new\s+FileEntity\(|\bFileEntity\b' -g '!**/build/**' -C2Length of output: 34599
FilePathResponse.from 호출부 교체 필요
- aics-domain/src/main/java/kgu/developers/domain/post/application/response/PostDetailResponse.java:79
FilePathResponse.from(post.getFile())→FilePathResponse.of(post.getFile().getId(), post.getFile().getPhysicalPath()) - aics-admin/src/main/java/kgu/developers/admin/file/application/FileAdminFacade.java:22
FilePathResponse.from(savedFile)→FilePathResponse.of(savedFile.getId(), storedPath)
LabDetailResponse.from 호출부는 모두 String physicalPath 형태로 변경되어 문제 없습니다.
🤖 Prompt for AI Agents
In
aics-api/src/main/java/kgu/developers/api/lab/presentation/response/LabDetailResponse.java
around line 35, the factory should use the new FilePathResponse.of(id,
physicalPath) signature instead of FilePathResponse.from(...); update the
LabDetailResponse.from(Lab lab, String physicalPath) implementation to call
FilePathResponse.of(lab.getFile().getId(), physicalPath) (and adjust/remove any
now-incorrect FilePathResponse.from(...) calls), and add/update imports if
necessary.
Comment on lines
+21
to
25
| public Map<Long,String> findPhysicalPathMapByIds(List<Long> ids) { | ||
| return fileRepository.findAllByIds(ids) | ||
| .stream() | ||
| .collect(Collectors.toMap(FileEntity::getId, Function.identity())); | ||
| .collect(Collectors.toMap(FileEntity::getId, FileEntity::getPhysicalPath)); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
toMap 중복 키 병합 함수 추가로 안전성 확보
중복 ID가 반환될 경우 Collectors.toMap이 IllegalStateException을 던질 수 있습니다. 병합 함수를 명시해 런타임 예외를 방지하세요. (첫 값 유지)
- return fileRepository.findAllByIds(ids)
- .stream()
- .collect(Collectors.toMap(FileEntity::getId, FileEntity::getPhysicalPath));
+ return fileRepository.findAllByIds(ids)
+ .stream()
+ .collect(Collectors.toMap(
+ FileEntity::getId,
+ FileEntity::getPhysicalPath,
+ (prev, curr) -> prev
+ ));📝 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.
Suggested change
| public Map<Long,String> findPhysicalPathMapByIds(List<Long> ids) { | |
| return fileRepository.findAllByIds(ids) | |
| .stream() | |
| .collect(Collectors.toMap(FileEntity::getId, Function.identity())); | |
| .collect(Collectors.toMap(FileEntity::getId, FileEntity::getPhysicalPath)); | |
| } | |
| public Map<Long, String> findPhysicalPathMapByIds(List<Long> ids) { | |
| return fileRepository.findAllByIds(ids) | |
| .stream() | |
| .collect(Collectors.toMap( | |
| FileEntity::getId, | |
| FileEntity::getPhysicalPath, | |
| (prev, curr) -> prev | |
| )); | |
| } |
🤖 Prompt for AI Agents
In
aics-domain/src/main/java/kgu/developers/domain/file/application/query/FileQueryService.java
around lines 21 to 25, the Collectors.toMap call can throw IllegalStateException
if duplicate IDs appear; update the collector to supply a merge function that
keeps the first value (e.g., (existing, replacement) -> existing) so duplicates
are merged safely and the method returns a stable Map.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
해당 PR에 대한 요약을 작성해주세요.
현재
clubFacde,labFacade에서 FileEntity를 사용하는 문제가 있습니다.File Entity관리를FileQueryService계층에서 진행하도록 수정합니다Tasks