즐겨찾기_만들기_기능_추가 #103
즐겨찾기_만들기_기능_추가 #103
Conversation
|
""" Walkthrough이번 변경 사항은 파일 업로드 경로 설정, 정적 리소스 제공, 북마크(Bookmark) 관리 REST API 및 서비스, 장소(Place) 페이징 조회 API 추가, 보안 설정 개선, 리포지토리 어노테이션 및 엔티티 필드 변경 등을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as BookmarkController
participant Service as BookmarkService
participant Storage as FileStorageService
participant Repo as BookmarkRepository
Client->>Controller: POST /api/bookmark/create (multipart/form-data)
Controller->>Service: create(CustomUserDetails, BookmarkRequest)
Service->>Storage: storeImage(MultipartFile)
Storage-->>Service: "/uploads/{filename}"
Service->>Repo: save(Bookmark)
Repo-->>Service: Bookmark
Service-->>Controller: BookmarkResponse
Controller-->>Client: 201 Created + BookmarkResponse
sequenceDiagram
participant Client
participant Controller as BookmarkController
participant Service as BookmarkService
participant Repo as BookmarkRepository
Client->>Controller: GET /api/bookmark/mylist
Controller->>Service: listByMember(CustomUserDetails)
Service->>Repo: findAllByMember(Member)
Repo-->>Service: List<Bookmark>
Service-->>Controller: List<BookmarkResponse>
Controller-->>Client: 200 OK + List<BookmarkResponse>
Possibly related PRs
Poem
""" ✨ Finishing Touches
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. 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
Documentation and Community
|
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
1 similar comment
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
src/main/java/jombi/freemates/service/PlaceService.java (1)
78-89: 🛠️ Refactor suggestion루프 내 개별
save호출로 인한 퍼포먼스 저하 가능성
placeRepository.save(place)를 장소 개수만큼 반복 호출하면 DB round-trip 이 반복되어 성능이 떨어집니다. 모든Place객체의 필드만 수정한 뒤 한꺼번에saveAll또는flush를 사용하는 방식이 효율적입니다. 예시 diff:- for (Place place : places) { + List<Place> updated = new ArrayList<>(); + for (Place place : places) { ... - placeRepository.save(place); + updated.add(place); ... } + placeRepository.saveAll(updated);또한, 대용량일 경우 배치 크기를 지정하거나 chunking 을 고려해 주세요.
♻️ Duplicate comments (1)
src/main/java/jombi/freemates/model/constant/PinColor.java (1)
6-17: Enum 직렬화 방식 통일 필요
Visibilityenum 과 마찬가지로, 클라이언트가 “빨강/노랑 …” 한글 값을 직접 받을 필요가 있을 경우@JsonValue메서드를 추가해 직렬화 방식을 맞춰주세요.+import com.fasterxml.jackson.annotation.JsonValue; @AllArgsConstructor @Getter public enum PinColor { RED("빨강"), YELLOW("노랑"), GREEN("초록"), BLUE("파랑"), PURPLE("보라"), PINK("핑크"); private final String description; + @JsonValue + public String jsonValue() { + return description; + } }
🧹 Nitpick comments (22)
src/main/java/jombi/freemates/model/constant/Visibility.java (1)
6-14: 🎨 Enum 직렬화용 @jsonvalue 메서드 추가 권장현재 Enum 이름(PUBLIC, PRIVATE)이 그대로 JSON 으로 노출될 가능성이 높습니다. 클라이언트가 “공개/비공개” 한글 값을 직접 받기를 원한다면
@JsonValue또는 커스텀@JsonSerialize를 지정해주는 편이 안전합니다.+import com.fasterxml.jackson.annotation.JsonValue; @AllArgsConstructor @Getter public enum Visibility { PUBLIC("공개"), PRIVATE("비공개"); private final String description; + @JsonValue + public String jsonValue() { + return description; + } }src/main/java/jombi/freemates/service/PlaceFetchService.java (1)
120-121: 빈 리스트 생성 시 Collections.emptyList() 사용 고려
new ArrayList<>()는 불필요한 가변 리스트 객체를 매번 생성합니다.
해당 필드는 초기 상태에서 수정되지 않는다면Collections.emptyList()로 불변 객체를 제공해 GC 부하를 줄일 수 있습니다.- .tags(new ArrayList<>()) + .tags(Collections.emptyList())단, 이후 JPA 영속 상태에서
tags를 수정할 필요가 있다면 그대로 두셔도 무방합니다.src/main/java/jombi/freemates/repository/BookmarkRepository.java (1)
11-13: 페이징 메서드 추가 고려
findAllByMember(Member member)는 사용자가 북마크를 많이 보유할 경우 한 번에 모든 데이터를 불러오게 됩니다.
장기적으로는Page<Bookmark>를 반환하도록 오버로드하거나Slice기반 쿼리를 추가하여 페이징을 지원하는 것이 성능‧트래픽 측면에서 안전합니다.+ Page<Bookmark> findAllByMember(Member member, Pageable pageable);src/main/java/jombi/freemates/model/postgres/Place.java (1)
3-15: 사용되지 않는 import 정리 필요
org.checkerframework.common.aliasing.qual.Unique가 현재 클래스에서 사용되지 않습니다.
IDE-level 검사에서는 통과할 수 있으나, CI 단계에서 unused-import 규칙이 걸려 있다면 빌드가 실패할 수 있습니다.src/main/java/jombi/freemates/config/StaticResourceConfig.java (1)
8-19: 필드 주입 대신 생성자 주입으로 전환 권장
@Value필드 주입은 테스트 작성과 불변성 확보에 불리합니다. Lombok@RequiredArgsConstructor(또는 명시적 생성자)와final키워드를 사용해 생성자 주입으로 전환하면 가독성과 테스트 용이성이 향상됩니다.- @Configuration -public class StaticResourceConfig implements WebMvcConfigurer { - @Value("${file.upload-dir}") // application.yml 에 정의된 경로 - private String uploadDir; +@Configuration +@RequiredArgsConstructor +public class StaticResourceConfig implements WebMvcConfigurer { + @Value("${file.upload-dir}") // application.yml 에 정의된 경로 + private final String uploadDir;src/main/java/jombi/freemates/model/dto/BookmarkRequest.java (2)
9-10: 불필요한 import 제거 권장
@RequiredArgsConstructor는 현재 클래스에서 사용되지 않고 있습니다. IDE 경고를 줄이고 가독성을 높이기 위해 사용하지 않는 import 문을 삭제해 주세요.
14-19:@ToString에MultipartFile노출은 로그 과다·보안 위험
@ToString이 모든 필드를 문자열로 출력하므로, 업로드된 파일 메타데이터가 로그에 그대로 남을 수 있습니다. 파일 크기에 따라 성능 저하도 발생할 수 있습니다.-@ToString +@ToString(exclude = "file")또는 필드 수준에
@ToString.Exclude를 추가해 주세요.src/main/java/jombi/freemates/model/dto/BookmarkResponse.java (1)
13-18: 응답 DTO 불변(immutable) 설계 고려API 응답 객체는 주로 읽기 전용으로 사용되므로,
@Setter를 제거하고@Builder·@AllArgsConstructor만으로 값을 주입하면 불변성이 확보되어 추후 사이드이펙트 위험을 줄일 수 있습니다.src/main/java/jombi/freemates/model/dto/KakaoPlaceCrawlDetail.java (1)
21-22: 가변 리스트 노출로 인한 캡슐화 깨짐
tags필드가ArrayList로 그대로 노출돼 외부 코드에서 자유롭게 수정할 수 있습니다. DTO의 무결성을 보장하려면 불변 리스트로 감싸거나 defensive copy를 반환하도록 변경해 주세요.- @Builder.Default - private List<String> tags = new ArrayList<>(); + @Builder.Default + private List<String> tags = new ArrayList<>(); + + public List<String> getTags() { + return List.copyOf(tags); // Java 10+ + } + + public void setTags(List<String> tags) { + this.tags = new ArrayList<>(tags); + }또는
@Singular를 사용해 빌더 단계에서만 추가하도록 하는 방법도 있습니다.src/main/java/jombi/freemates/service/PlaceService.java (2)
89-89: 로그 메시지 형식 개선 제안현재
"이미지 저장된 거 {} 카카오에서 크롤링한거{}"는 중간 공백이 없어 가독성이 떨어집니다.-log.debug("이미지 저장된 거 {} 카카오에서 크롤링한거{}", ...); +log.debug("저장된 이미지: {}, 크롤링 이미지: {}", ...);소소하지만 디버깅 시 가독성이 크게 향상됩니다.
96-102:getAllPlaces()는 페이징/캐싱 고려 필요전체 레코드를 그대로 반환하면 데이터가 많을 때 메모리 사용량과 응답 시간이 급격히 증가합니다.
- Spring Data
Pageable파라미터를 받아 페이징 처리- 혹은 Redis/캐시 계층 도입
을 검토해 주세요.src/main/java/jombi/freemates/service/crawler/KakaoCrawler.java (2)
149-157:tags추출 로직 간결화 가능Stream API 를 활용하면 가독성과 성능(내부 루프 최적화) 모두 개선됩니다.
-List<String> tags = new ArrayList<>(); -... - JsonArray tagsArray = addInfo.getAsJsonArray("tags"); - for (JsonElement el : tagsArray) { - tags.add(el.getAsString()); - } +List<String> tags = StreamSupport.stream(addInfo.getAsJsonArray("tags").spliterator(), false) + .map(JsonElement::getAsString) + .collect(Collectors.toList());또한,
tags리스트를 그대로 DTO 에 넘기면 외부 수정 가능성이 있으므로Collections.unmodifiableList(tags)를 전달해 불변성을 확보하는 것도 고려해 주세요.
110-117: 파라미터명 변경과 메서드명 불일치메서드명은
crawlByPlaceId인데 파라미터는kakaoPlaceId로 변경되었습니다.
혼동을 피하려면 메서드명도crawlByKakaoPlaceId로 맞추거나 Javadoc 에 명확히 기재하는 것이 좋습니다.src/main/java/jombi/freemates/config/FileStorageProperties.java (1)
8-16: 프로퍼티 값 검증 추가 제안
uploadDir가 설정되지 않았을 때 애플리케이션이 부팅 직후 NPE 로 종료될 수 있습니다.
@Validated와@NotBlank를 사용해 필수 프로퍼티임을 명시하면 조기 검증이 가능합니다.@Configuration @ConfigurationProperties(prefix = "file") +@Validated ... -public class FileStorageProperties { - private String uploadDir; +public class FileStorageProperties { + @NotBlank + private String uploadDir;이를 위해
org.springframework.validation.annotation.Validated와jakarta.validation.constraints.NotBlank를 추가해 주세요.src/main/java/jombi/freemates/config/FileStorageConfig.java (1)
13-23: 프로퍼티 검증 및 예외 유형 명확화 제안
props.getUploadDir()가null이거나 공백일 경우 프로젝트 루트가 생성될 수 있습니다.
또한 현재RuntimeException은 모호하므로 서비스 로직에서 구분하기 어렵습니다.+ String uploadDir = props.getUploadDir(); + Assert.hasText(uploadDir, "'file.upload-dir' 프로퍼티가 비어있습니다"); + - Path path = Paths - .get(props.getUploadDir()) + Path path = Paths + .get(uploadDir) .toAbsolutePath() .normalize(); @@ - } catch (IOException e) { - throw new RuntimeException("업로드 디렉터리 생성 실패", e); + } catch (IOException e) { + throw new FileSystemException( + "업로드 디렉터리 생성 실패: " + path, null, e.getMessage()); }
org.springframework.util.Assert와java.nio.file.FileSystemException사용을 고려해 보세요.src/main/java/jombi/freemates/service/FileStorageService.java (1)
16-17:uploadDir디렉터리 존재 보장 확인
FileStorageConfig에서 디렉터리를 미리 생성하겠지만, 안전을 위해storeImage내부에서Files.createDirectories(uploadDir)로 한 번 더 보장하는 편이 좋습니다. 컨테이너 재배포로 디렉터리가 유실될 경우를 대비할 수 있습니다.src/main/java/jombi/freemates/model/postgres/Bookmark.java (1)
25-28: 엔티티 PK 명칭 혼동 우려필드명이
favoriteId인데 엔티티명은Bookmark입니다. 팀 내 다른 도메인(예:Favorite)과 혼동될 여지가 있으니bookmarkId등으로 통일하는 것을 권장합니다.src/main/java/jombi/freemates/service/BookmarkService.java (2)
52-66: 중복 매핑 로직 추출 제안
create,listByMember두 곳에서Bookmark → BookmarkResponse변환 코드가 중복됩니다. 전용 메서드 또는ModelMapper/MapStruct를 활용해 중복을 제거하면 유지보수성이 높아집니다.
52-54:@Transactional읽기 전용 옵션 고려
listByMember는 조회만 수행하므로@Transactional(readOnly = true)를 사용하면 플러시 검사 비용을 줄일 수 있습니다.
src/main/java/jombi/freemates/controller/BookmarkController.java (3)
15-20: 불필요한 import 정리 필요
ResponseEntity와@Controller는 본 파일에서 사용되지 않습니다. 사용하지 않는 import를 그대로 두면 IDE / 빌드 도구에서 경고가 발생하고 가독성이 떨어집니다.-import org.springframework.http.ResponseEntity; -import org.springframework.stereotype.Controller;
45-66: API 문서 주석과 실제 코드 사이의 괴리
@Operation설명 블록의 에러코드 섹션이 비어 있습니다. 스웨거 문서에 빈 섹션이 노출되면 사용자 혼란을 야기할 수 있으므로 실제 예외 케이스(예: 400 Bad Request, 401 Unauthorized 등)를 기입하거나 섹션을 제거해 주세요.
또한 summary 가 “즐겨찾기 생성”이지만, REST 에서는 자원 생성 자체가 이미 POST/api/bookmark로 암시됩니다. path 에/create를 붙이는 것이 과연 필요한지 함께 고려해 주세요.
100-104: RESTful URL·HTTP 메서드 설계 개선 제안
POST /api/bookmark/create,GET /api/bookmark/list처럼 동사형 하위 경로를 두는 것은 REST 원칙(자원=명사)을 벗어납니다. 일반적으로는
- 생성:
POST /api/bookmarks- 사용자별 조회:
GET /api/bookmarks(AuthenticationPrincipal 로 필터링)과 같이 명사형 경로로 충분합니다. URL 을 변경하면 클라이언트·서버 모두 직관성이 높아집니다. 배포 전 API 호환성 영향 여부를 검토해 주세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/main/java/jombi/freemates/config/FileStorageConfig.java(1 hunks)src/main/java/jombi/freemates/config/FileStorageProperties.java(1 hunks)src/main/java/jombi/freemates/config/StaticResourceConfig.java(1 hunks)src/main/java/jombi/freemates/config/WebSecurityConfig.java(2 hunks)src/main/java/jombi/freemates/controller/BookmarkController.java(1 hunks)src/main/java/jombi/freemates/controller/PlaceController.java(2 hunks)src/main/java/jombi/freemates/model/constant/PinColor.java(1 hunks)src/main/java/jombi/freemates/model/constant/Visibility.java(1 hunks)src/main/java/jombi/freemates/model/dto/BookmarkRequest.java(1 hunks)src/main/java/jombi/freemates/model/dto/BookmarkResponse.java(1 hunks)src/main/java/jombi/freemates/model/dto/KakaoPlaceCrawlDetail.java(2 hunks)src/main/java/jombi/freemates/model/postgres/Bookmark.java(1 hunks)src/main/java/jombi/freemates/model/postgres/Place.java(2 hunks)src/main/java/jombi/freemates/repository/BookmarkRepository.java(1 hunks)src/main/java/jombi/freemates/service/BookmarkService.java(1 hunks)src/main/java/jombi/freemates/service/FileStorageService.java(1 hunks)src/main/java/jombi/freemates/service/PlaceFetchService.java(2 hunks)src/main/java/jombi/freemates/service/PlaceService.java(1 hunks)src/main/java/jombi/freemates/service/crawler/KakaoCrawler.java(3 hunks)
🔇 Additional comments (3)
src/main/java/jombi/freemates/config/StaticResourceConfig.java (1)
15-19: 리소스 핸들러 경로 주의
addResourceLocations("file:" + uploadDir + "/")에서
uploadDir값이 슬래시로 끝나는 경우//가 중복될 수 있습니다.- OS 마다 경로 구분자가 달라 발생할 수 있는 문제를
Paths.get(uploadDir).toUri().toString()으로 예방할 수 있습니다.추가 검증을 권합니다.
src/main/java/jombi/freemates/service/PlaceService.java (1)
86-86:setTags호출 시 Null 보호 필요 여부 확인크롤링 결과가
null이거나 빈 리스트일 수도 있으므로,place.setTags()전에tags가null인지 확인하거나Collections.emptyList()로 치환하는 방어 로직을 추가하면 NPE 를 예방할 수 있습니다.src/main/java/jombi/freemates/config/WebSecurityConfig.java (1)
34-35:Details
❌ Incorrect review comment
CORS 기본값만으로는 프론트엔드 Preflight 실패 가능
cors(withDefaults())는allowedOrigins("*")가 아니며, 표준 GET/HEAD 만 허용합니다.
프론트엔드에서Authorization헤더를 보내거나 PUT/DELETE 요청을 사용할 경우 403 이 발생할 수 있으니, 별도CorsConfigurationSource빈을 정의해 정확한 도메인·헤더·메서드를 등록하는 편이 안전합니다.
CORS 기본 설정 확인을 위해 다음 정보를 조회합니다:
🌐 Web query:
What are the default values for CorsConfiguration (allowedOrigins, allowedMethods, allowedHeaders) when using http.cors(withDefaults()) in Spring Security?💡 Result:
In Spring Security, when you use
http.cors(withDefaults()), the defaultCorsConfigurationapplied has the following default values:
- allowedOrigins:
*(allows all origins)- allowedMethods: GET, HEAD, POST (only these HTTP methods are allowed by default)
- allowedHeaders:
*(allows all headers)To summarize:
- allowedOrigins: all origins allowed by default (
*)- allowedMethods: GET, HEAD, POST
- allowedHeaders: all headers allowed by default (
*)This means that by default, the CORS support in Spring Security permits requests from any origin, allows GET, HEAD, and POST methods, and permits any headers. If you want to customize these values, you'll need to provide your own
CorsConfigurationSource.
CORS 기본 설정 재확인 필요
http.cors(withDefaults())의 기본CorsConfiguration은 다음과 같습니다:
- allowedOrigins: “*” (모든 출처 허용)
- allowedMethods: GET, HEAD, POST
- allowedHeaders: “*” (모든 헤더 허용)
따라서 PUT/DELETE 등 기타 HTTP 메서드를 사용하거나, 사전 요청(Preflight) 시점에 CORS 설정이 필요한 요청을 보낼 경우 403 에러가 발생할 수 있습니다.
별도CorsConfigurationSource빈을 정의하여 허용할 도메인, 메서드, 헤더(그리고 자격증명 사용 시allowCredentials(true))를 명시적으로 등록하세요.
- 파일: src/main/java/jombi/freemates/config/WebSecurityConfig.java
- 라인: 34–35
Likely an incorrect or invalid review comment.
| private String title; | ||
| private String description; | ||
| private PinColor pinColor; | ||
| private Visibility visibility; | ||
| private MultipartFile file; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
요청 DTO에 Bean Validation 추가 필요
title, description, pinColor, visibility 값이 null/빈 문자열로 전달되는 경우 서비스 계층에서 NPE 또는 비즈니스 오류가 발생할 가능성이 있습니다. DTO 단에서 검증을 거치는 편이 안전합니다.
+import javax.validation.constraints.NotBlank;
+import javax.validation.constraints.NotNull;
private String title;
private String description;
- private PinColor pinColor;
- private Visibility visibility;
+ @NotNull
+ private PinColor pinColor;
+ @NotNull
+ private Visibility visibility;
+
+ @NotBlank
+ private String title;
+ private String description;컨트롤러 메서드에 @Valid를 붙이면 자동으로 검증됩니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/model/dto/BookmarkRequest.java around lines 21
to 25, the DTO fields lack Bean Validation annotations, which can lead to null
or empty values causing NPE or business errors downstream. Add appropriate
validation annotations such as @NotBlank or @NotNull to each field to enforce
non-null and non-empty constraints. Ensure that the controller method handling
this DTO has the @Valid annotation to trigger validation.
There was a problem hiding this comment.
@Cassiiopeia 네, 알겠습니다. 해당 제안은 넘어가겠습니다.
/| /|
( . .)
)___(
/ \
( o o )
\ - /
\_____/
| @GetMapping("/all") | ||
| public ResponseEntity<List<Place>> getAllPlaces() { | ||
| List<Place> places = placeService.getAllPlaces(); | ||
| return ResponseEntity.ok(places); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
엔티티 직접 반환·페이징 미구현으로 인한 성능·보안 위험
List<Place> 엔티티를 그대로 반환하면
- LAZY 필드의 순환 직렬화로 인한 StackOverflow/쿼리 난수화를 유발할 수 있고,
- DB 컬럼 전부(내부 식별자·FK 등) 가 노출됩니다.
또한 전체 데이터를 한 번에 내려주면 레코드 수가 늘어날수록 응답 지연·메모리 폭증이 발생합니다.
- @GetMapping("/all")
- public ResponseEntity<List<Place>> getAllPlaces() {
- List<Place> places = placeService.getAllPlaces();
- return ResponseEntity.ok(places);
- }
+ @GetMapping
+ public ResponseEntity<Page<PlaceDto>> getPlaces(
+ @PageableDefault(size = 20) Pageable pageable) {
+ Page<PlaceDto> page = placeService.getPlaces(pageable)
+ .map(PlaceDto::from);
+ return ResponseEntity.ok(page);
+ }- DTO 로 치환하여 노출 필드를 제한하고,
Pageable로 페이징 처리해 주세요.
추가로,@PreAuthorize("hasRole('ROLE_USER')")등 권한을 부여하지 않으면 인증 없이 모든 사용자가 장소 전체 목록을 열람할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/controller/PlaceController.java around lines 32
to 36, returning the entity list directly without DTO transformation and
pagination poses performance and security risks. Refactor the method to convert
Place entities into a DTO that exposes only necessary fields, and implement
pagination using Pageable to limit data volume. Also, add appropriate
authorization annotations like @PreAuthorize to restrict access to authenticated
users with specific roles.
There was a problem hiding this comment.
@daye200 저도 이건 동의
페이지네이션 쓰세요 파라미터는 저렇게 받지말고
pageSize
pageCount
이거두개였나?
페이지 번호, 페이지크기
이거 파라미터 두개받고
Page<> 객체 반환받아주세요
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| public String storeImage(MultipartFile file) { | ||
| String ext = StringUtils.getFilenameExtension(file.getOriginalFilename()); | ||
| String filename = UUID.randomUUID() + "." + ext; | ||
| Path target = uploadDir.resolve(filename); | ||
| try { | ||
| file.transferTo(target); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("파일 저장 실패", e); | ||
| } | ||
| // 예: "/uploads/xxxxx.jpg" 같은 URL 경로 리턴 | ||
| return "/uploads/" + filename; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
파일 검증·예외 처리 보강 필요
file.getOriginalFilename()가null일 경우 NPE가 발생하고, 확장자(ext)도null이 되어filename이"uuid.null"형식으로 저장됩니다.- 업로드 용량·MIME 타입·빈 파일 여부를 체크하지 않아 악성/빈 파일이 저장될 수 있습니다.
catch (Exception e)로 모든 예외를 삼켜RuntimeException으로 래핑하면, 상위 계층에서 원인 분석이 어려워집니다.
@@
- public String storeImage(MultipartFile file) {
- String ext = StringUtils.getFilenameExtension(file.getOriginalFilename());
- String filename = UUID.randomUUID() + "." + ext;
+ public String storeImage(MultipartFile file) {
+ if (file == null || file.isEmpty()) {
+ throw new IllegalArgumentException("업로드할 파일이 없습니다.");
+ }
+ String originalName = StringUtils.cleanPath(file.getOriginalFilename());
+ String ext = StringUtils.getFilenameExtension(originalName);
+ if (ext == null) {
+ throw new IllegalArgumentException("지원하지 않는 확장자입니다: " + originalName);
+ }
+ String filename = UUID.randomUUID() + "." + ext.toLowerCase();
@@
- } catch (Exception e) {
- throw new RuntimeException("파일 저장 실패", e);
+ } catch (IOException | IllegalStateException e) {
+ throw new UncheckedIOException("파일 저장 실패", e);
}이렇게 하면 입력 검증·예외 원인 파악이 용이해집니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/service/FileStorageService.java around lines 18
to 29, the method storeImage does not handle the case where
getOriginalFilename() returns null, leading to potential NullPointerException
and filenames like "uuid.null". Additionally, it lacks validation for file size,
MIME type, and empty files, and catches all exceptions broadly, which hampers
debugging. To fix this, add null checks for getOriginalFilename() and validate
the filename and extension, implement checks for file size, MIME type, and empty
files before saving, and catch specific exceptions instead of all, providing
clearer error messages for easier troubleshooting.
There was a problem hiding this comment.
@Cassiiopeia 네, 맞습니다! 파일 업로드 관련 코드는 특히 예외 처리가 중요하죠.
현재 코드에서 개선이 필요한 부분들:
null파일명 처리- 파일 크기/타입 검증
- 구체적인 예외 타입별 처리
이 부분들을 개선한 코드를 작성해드릴까요? 아니면 나중에 별도 PR로 처리하실 건가요?
✏️ Learnings added
Learnt from: Cassiiopeia
PR: freeMates/FreeMates_Backend#103
File: src/main/java/jombi/freemates/service/FileStorageService.java:18-29
Timestamp: 2025-05-27T01:22:10.869Z
Learning: 파일 업로드 서비스에서는 null 파일명, 파일 크기/타입 검증, 구체적인 예외 처리가 필요하다고 사용자(Cassiiopeia)가 동의함. 한국어 환경에서 파일 업로드 관련 예외 처리의 중요성을 인정받음.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) { | ||
| // 파일 저장 → URL | ||
| String imageUrl = storage.storeImage(req.getFile()); | ||
| Member member = customUserDetails.getMember(); | ||
|
|
||
| // 엔티티 생성 및 저장 | ||
| Bookmark b = Bookmark.builder() | ||
| .member(member) | ||
| .title(req.getTitle()) | ||
| .description(req.getDescription()) | ||
| .pinColor(req.getPinColor()) | ||
| .visibility(req.getVisibility()) | ||
| .imageUrl(imageUrl) | ||
| .build(); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
요청 데이터·파일 검증 누락
req.getFile() 이 null 이거나 빈 파일이면 storeImage 내부에서 예외가 발생합니다. 비즈니스 계층에서 먼저 검증하여 사용자 친화적인 오류 메시지를 반환하세요.
또한, 같은 로직을 테스트 코드 없이 신뢰하기 어렵습니다. 단위 테스트에서
- 정상 생성
- 빈 파일 업로드
- 확장자 누락
케이스를 검증해 주시기 바랍니다.
storeImage 메서드가 null 또는 빈 파일을 어떻게 처리하는지 확인하기 위해 아래 스크립트를 실행해주세요.
🏁 Script executed:
#!/bin/bash
# storeImage 호출 위치 및 구현 정의 검색
rg "storeImage" -n srcLength of output: 255
storeImage 메서드가 null 또는 빈 파일을 어떻게 처리하는지 확인하기 위해 아래 스크립트를 실행해주세요.
🏁 Script executed:
#!/bin/bash
rg -n -A50 "public String storeImage" src/main/java/jombi/freemates/service/FileStorageService.javaLength of output: 581
파일 업로드 입력 검증 로직 추가 필요
현재 BookmarkService#create 메서드에서 req.getFile()이 null 또는 빈 파일인지 검증하지 않고 바로
storage.storeImage를 호출하고, 실제 FileStorageService#storeImage에도 null·empty 파일과 확장자 누락 처리가 없어
NPE 또는 “파일 저장 실패” 런타임 예외가 발생할 수 있습니다. 사용자 친화적인 오류 메시지와 안정적인 동작을 위해 아래와 같이 수정을 권장합니다.
-
src/main/java/jombi/freemates/service/BookmarkService.java
•create메서드 시작부에file == null || file.isEmpty()검증 추가 (라인 24–27) -
src/main/java/jombi/freemates/service/FileStorageService.java
•storeImage진입부에 null·empty 파일 검증 및ext == null(확장자 누락) 처리 로직 추가 (라인 18–21)
예시 변경(diff):
public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) {
- String imageUrl = storage.storeImage(req.getFile());
+ MultipartFile file = req.getFile();
+ if (file == null || file.isEmpty()) {
+ throw new InvalidRequestException("업로드할 이미지를 선택해주세요.");
+ }
+ String imageUrl = storage.storeImage(file); public String storeImage(MultipartFile file) {
- String ext = StringUtils.getFilenameExtension(file.getOriginalFilename());
+ if (file == null || file.isEmpty()) {
+ throw new InvalidRequestException("유효한 이미지를 업로드해주세요.");
+ }
+ String ext = StringUtils.getFilenameExtension(file.getOriginalFilename());
+ if (ext == null || ext.isBlank()) {
+ throw new InvalidRequestException("파일 확장자를 확인해주세요.");
+ }단위 테스트에도 아래 케이스를 반드시 추가해주세요:
- 정상 업로드 (유효 이미지)
file == null- 빈 파일 업로드
- 확장자 누락
📝 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 BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) { | |
| // 파일 저장 → URL | |
| String imageUrl = storage.storeImage(req.getFile()); | |
| Member member = customUserDetails.getMember(); | |
| // 엔티티 생성 및 저장 | |
| Bookmark b = Bookmark.builder() | |
| .member(member) | |
| .title(req.getTitle()) | |
| .description(req.getDescription()) | |
| .pinColor(req.getPinColor()) | |
| .visibility(req.getVisibility()) | |
| .imageUrl(imageUrl) | |
| .build(); | |
| public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) { | |
| // 파일 저장 → URL | |
| MultipartFile file = req.getFile(); | |
| if (file == null || file.isEmpty()) { | |
| throw new InvalidRequestException("업로드할 이미지를 선택해주세요."); | |
| } | |
| String imageUrl = storage.storeImage(file); | |
| Member member = customUserDetails.getMember(); | |
| // 엔티티 생성 및 저장 | |
| Bookmark b = Bookmark.builder() | |
| .member(member) | |
| .title(req.getTitle()) | |
| .description(req.getDescription()) | |
| .pinColor(req.getPinColor()) | |
| .visibility(req.getVisibility()) | |
| .imageUrl(imageUrl) | |
| .build(); |
| public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) { | |
| // 파일 저장 → URL | |
| String imageUrl = storage.storeImage(req.getFile()); | |
| Member member = customUserDetails.getMember(); | |
| // 엔티티 생성 및 저장 | |
| Bookmark b = Bookmark.builder() | |
| .member(member) | |
| .title(req.getTitle()) | |
| .description(req.getDescription()) | |
| .pinColor(req.getPinColor()) | |
| .visibility(req.getVisibility()) | |
| .imageUrl(imageUrl) | |
| .build(); | |
| public String storeImage(MultipartFile file) { | |
| if (file == null || file.isEmpty()) { | |
| throw new InvalidRequestException("유효한 이미지를 업로드해주세요."); | |
| } | |
| String ext = StringUtils.getFilenameExtension(file.getOriginalFilename()); | |
| if (ext == null || ext.isBlank()) { | |
| throw new InvalidRequestException("파일 확장자를 확인해주세요."); | |
| } | |
| // ... the rest of the existing implementation ... |
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/service/BookmarkService.java lines 24 to 37,
the create method calls storage.storeImage(req.getFile()) without validating if
the file is null or empty, which can cause runtime exceptions. Add a check at
the start of the method to verify if req.getFile() is null or empty and throw a
user-friendly exception if so. Also, review the storeImage method in
FileStorageService.java around lines 18 to 21 to add null, empty file, and
extension validation, throwing clear exceptions for invalid input. Additionally,
implement unit tests for normal upload, null file, empty file, and missing
extension cases to ensure robustness.
There was a problem hiding this comment.
ㅋㅋ 근데 이런건 기본적으로 해줘야해
파일유효성 체크는 좀 필수긴함.. 로깅도 제대로
There was a problem hiding this comment.
@Cassiiopeia 맞습니다! 파일 검증과 로깅은 정말 기본이어야 하는 부분이네요.
현재 코드를 보면 @Slf4j는 있지만 실제로 로깅이 거의 없고, 파일 검증도 전혀 없는 상태입니다. 아래와 같이 종합적으로 개선하는 것을 제안합니다:
BookmarkService.java 개선:
@Transactional
public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) {
log.info("북마크 생성 요청 - 사용자: {}, 제목: {}",
customUserDetails.getUsername(), req.getTitle());
// 파일 검증
MultipartFile file = req.getFile();
if (file == null || file.isEmpty()) {
log.warn("파일 업로드 실패 - 빈 파일: 사용자 {}", customUserDetails.getUsername());
throw new InvalidRequestException("업로드할 이미지를 선택해주세요.");
}
try {
String imageUrl = storage.storeImage(file);
log.info("이미지 저장 완료 - URL: {}", imageUrl);
Member member = customUserDetails.getMember();
Bookmark b = Bookmark.builder()
.member(member)
.title(req.getTitle())
.description(req.getDescription())
.pinColor(req.getPinColor())
.visibility(req.getVisibility())
.imageUrl(imageUrl)
.build();
bookmarkRepository.save(b);
log.info("북마크 생성 완료 - ID: {}, 사용자: {}", b.getFavoriteId(), member.getNickname());
return BookmarkResponse.builder()
.memberId(member.getMemberId())
.nickname(member.getNickname())
.imageUrl(imageUrl)
.title(b.getTitle())
.description(b.getDescription())
.pinColor(b.getPinColor())
.visibility(b.getVisibility())
.build();
} catch (Exception e) {
log.error("북마크 생성 실패 - 사용자: {}, 오류: {}",
customUserDetails.getUsername(), e.getMessage(), e);
throw e;
}
}FileStorageService.java 개선:
public String storeImage(MultipartFile file) {
log.debug("이미지 저장 시작 - 파일명: {}, 크기: {}",
file.getOriginalFilename(), file.getSize());
if (file == null || file.isEmpty()) {
log.warn("이미지 저장 실패 - 유효하지 않은 파일");
throw new InvalidRequestException("유효한 이미지를 업로드해주세요.");
}
String ext = StringUtils.getFilenameExtension(file.getOriginalFilename());
if (ext == null || ext.isBlank()) {
log.warn("이미지 저장 실패 - 확장자 없음: {}", file.getOriginalFilename());
throw new InvalidRequestException("파일 확장자를 확인해주세요.");
}
// 이미지 파일 타입 검증 추가
if (!isValidImageExtension(ext)) {
log.warn("이미지 저장 실패 - 지원하지 않는 확장자: {}", ext);
throw new InvalidRequestException("지원하지 않는 이미지 형식입니다.");
}
try {
String filename = UUID.randomUUID() + "." + ext;
Path targetLocation = uploadDir.resolve(filename);
file.transferTo(targetLocation);
String imageUrl = "/uploads/" + filename;
log.info("이미지 저장 완료 - URL: {}", imageUrl);
return imageUrl;
} catch (Exception e) {
log.error("이미지 저장 실패 - 파일: {}, 오류: {}",
file.getOriginalFilename(), e.getMessage(), e);
throw new RuntimeException("파일 저장에 실패했습니다.", e);
}
}이렇게 하면 파일 검증 + 상세한 로깅이 모두 포함되어 운영 시 문제 추적이 훨씬 쉬워질 것 같습니다!
✏️ Learnings added
Learnt from: Cassiiopeia
PR: freeMates/FreeMates_Backend#103
File: src/main/java/jombi/freemates/service/BookmarkService.java:24-37
Timestamp: 2025-05-27T01:22:25.997Z
Learning: This codebase requires comprehensive file validation and proper logging as essential development standards. When implementing file upload functionality, both input validation (null, empty, file extension checks) and detailed logging (info, warn, error levels with contextual information) must be included.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| @ModelAttribute @Valid BookmarkRequest request | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
@Valid 사용 효과가 없음
BookmarkRequest 필드에는 @NotBlank, @NotNull, @Size 등의 제약 어노테이션이 없으므로 @Valid 가 실제로 유효성 검사를 수행하지 않습니다. 필드 제약을 명시해 두면 컨트롤러 진입 전에 400 에러로 방어할 수 있어 안정성이 높아집니다.
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/controller/BookmarkController.java around lines
70 to 71, the @Valid annotation on the BookmarkRequest parameter is ineffective
because the BookmarkRequest class lacks validation annotations like @NotBlank,
@NotNull, or @Size on its fields. To fix this, add appropriate validation
annotations to the fields within the BookmarkRequest class to enable meaningful
validation before controller logic executes.
src/main/java/jombi/freemates/controller/BookmarkController.java
Outdated
Show resolved
Hide resolved
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/jombi/freemates/controller/BookmarkController.java (1)
70-71:@Valid사용 효과가 없음
BookmarkRequest필드에는@NotBlank,@NotNull,@Size등의 제약 어노테이션이 없으므로@Valid가 실제로 유효성 검사를 수행하지 않습니다. 필드 제약을 명시해 두면 컨트롤러 진입 전에 400 에러로 방어할 수 있어 안정성이 높아집니다.
🧹 Nitpick comments (2)
src/main/resources/application.yml (1)
31-33: [제안] 파일 저장 디렉토리 설정 네이밍 및 환경변수 활용 개선
현재file.upload-dir키는 다소 일반적이며, 운영환경별로 변경 시 설정 파일을 직접 수정해야 합니다.
- 환경변수 오버라이드를 위해 기본값과 함께
${...:...}문법 사용을 권장합니다.- 설정 그룹명(
file)이 다른 설정과 충돌할 수 있으므로,file-storage등 보다 구체적인 prefix를 고려해보세요.예시:
file: - upload-dir: /tmp/uploads + upload-dir: ${UPLOAD_DIR:/tmp/uploads}또는
-file: - upload-dir: ${UPLOAD_DIR:/tmp/uploads} +# file-storage: +file-storage: + upload-dir: ${UPLOAD_DIR:/tmp/uploads}src/main/java/jombi/freemates/controller/BookmarkController.java (1)
84-84: 요약 문구를 더 명확하게 개선할 수 있습니다.현재 "즐겨찾기 가져오기"보다는 "즐겨찾기 목록 조회"가 GET /list 엔드포인트의 기능을 더 정확하게 표현합니다.
- summary = "즐겨찾기 가져오기", + summary = "즐겨찾기 목록 조회",
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/jombi/freemates/controller/BookmarkController.java(1 hunks)src/main/resources/application.yml(1 hunks)
🔇 Additional comments (3)
src/main/java/jombi/freemates/controller/BookmarkController.java (3)
1-26: 패키지 선언 및 임포트 구조가 적절합니다.필요한 라이브러리들이 정확히 임포트되어 있고, 패키지 구조도 적절합니다.
27-35: 컨트롤러 클래스 구조가 잘 설계되었습니다.Swagger 문서화, REST 컨트롤러 어노테이션, 의존성 주입이 올바르게 구성되어 있습니다.
101-105: 인증된 사용자의 즐겨찾기만 조회하는 적절한 구현입니다.JWT 인증을 통해 현재 사용자의 즐겨찾기만 조회하도록 구현되어 있어 보안이 적절히 고려되었습니다.
| @PostMapping(value = "/create", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| @ResponseStatus(HttpStatus.CREATED) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
파일 업로드 엔드포인트의 적절한 구현을 확인합니다.
multipart/form-data를 사용한 파일 업로드와 HTTP 201 Created 상태 코드 사용이 적절합니다. 파일 크기 제한이나 허용 파일 형식에 대한 검증이 서비스 레이어에서 처리되고 있는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# Description: BookmarkService에서 파일 업로드 관련 검증 로직 확인
# Expected: 파일 크기, 형식 등의 검증 로직 존재
# BookmarkService의 create 메서드에서 파일 검증 로직 확인
ast-grep --pattern 'class BookmarkService {
$$$
create($_, $_) {
$$$
}
$$$
}'
# 파일 업로드 관련 검증 로직 검색
rg -A 10 "file.*valid|upload.*valid|size.*check|type.*check" --type javaLength of output: 166
추가 검증: BookmarkService 파일 경로 확인 및 create 메서드 내 검증 로직 유무 확인
🏁 Script executed:
#!/bin/bash
# 1. BookmarkService 클래스 파일 위치 찾기
service_file=$(rg -l "class BookmarkService" --type java)
echo "🔍 BookmarkService 파일 경로: $service_file"
# 2. 클래스 상단부(1~200라인) 출력
echo "===== 클래스 상단부 (1~200라인) ====="
sed -n '1,200p' "$service_file"
# 3. create 메서드 정의 및 주변 코드 확인
echo "===== create 메서드 및 주변(20라인) ====="
rg -n "create\s*\(" -A 20 "$service_file"
# 4. MultipartFile 관련 검증 로직 검색
echo "===== MultipartFile 검증 로직 검색 ====="
rg -n "MultipartFile" -A 10 "$service_file"Length of output: 3902
파일 업로드 검증 로직 누락 – 서비스 레이어에 파일 크기 및 형식 체크 추가 필요
현재 BookmarkService#create 메서드에는 파일 크기 제한이나 허용 형식에 대한 검증 로직이 없습니다. FileStorageService.storeImage 내부에 검증이 구현되어 있지 않다면, 다음 위치에 검증 로직을 추가해 주세요.
- 대상 파일:
src/main/java/jombi/freemates/service/BookmarkService.java - 메서드:
public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req)
검증 예시
// 파일 크기 제한 (예: 최대 5MB)
if (req.getFile().getSize() > 5 * 1024 * 1024) {
throw new FileValidationException("파일 크기가 5MB를 초과합니다.");
}
// 허용 형식 검사 (예: PNG, JPG)
String contentType = req.getFile().getContentType();
if (!List.of("image/png", "image/jpeg").contains(contentType)) {
throw new FileValidationException("허용되지 않는 파일 형식입니다.");
}위와 같은 검증을 서비스 레이어에 구현하거나, 컨트롤러 단에서 @Validated 및 커스텀 어노테이션을 활용해 처리해 주세요.
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/controller/BookmarkController.java at lines
66-67, the file upload endpoint correctly uses multipart/form-data and returns
HTTP 201 Created, but there is no validation for uploaded file size or type. Add
validation logic in the BookmarkService#create method to check that the uploaded
file does not exceed a size limit (e.g., 5MB) and is of an allowed content type
(e.g., image/png or image/jpeg). Implement these checks before processing the
file to ensure invalid files are rejected early.
| @GetMapping("/all") | ||
| public ResponseEntity<List<Place>> getAllPlaces() { | ||
| List<Place> places = placeService.getAllPlaces(); | ||
| return ResponseEntity.ok(places); | ||
| } |
There was a problem hiding this comment.
@daye200 저도 이건 동의
페이지네이션 쓰세요 파라미터는 저렇게 받지말고
pageSize
pageCount
이거두개였나?
페이지 번호, 페이지크기
이거 파라미터 두개받고
Page<> 객체 반환받아주세요
| private String title; | ||
| private String description; | ||
| private PinColor pinColor; | ||
| private Visibility visibility; | ||
| private MultipartFile file; |
| private String description; | ||
| private PinColor pinColor; | ||
| private Visibility visibility; | ||
| // --- 아래 필드 추가 --- |
| public BookmarkResponse create(CustomUserDetails customUserDetails, BookmarkRequest req) { | ||
| // 파일 저장 → URL | ||
| String imageUrl = storage.storeImage(req.getFile()); | ||
| Member member = customUserDetails.getMember(); | ||
|
|
||
| // 엔티티 생성 및 저장 | ||
| Bookmark b = Bookmark.builder() | ||
| .member(member) | ||
| .title(req.getTitle()) | ||
| .description(req.getDescription()) | ||
| .pinColor(req.getPinColor()) | ||
| .visibility(req.getVisibility()) | ||
| .imageUrl(imageUrl) | ||
| .build(); |
There was a problem hiding this comment.
ㅋㅋ 근데 이런건 기본적으로 해줘야해
파일유효성 체크는 좀 필수긴함.. 로깅도 제대로
| .build(); | ||
| } | ||
|
|
||
| @Transactional(Transactional.TxType.SUPPORTS) |
| public String storeImage(MultipartFile file) { | ||
| String ext = StringUtils.getFilenameExtension(file.getOriginalFilename()); | ||
| String filename = UUID.randomUUID() + "." + ext; | ||
| Path target = uploadDir.resolve(filename); | ||
| try { | ||
| file.transferTo(target); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("파일 저장 실패", e); | ||
| } | ||
| // 예: "/uploads/xxxxx.jpg" 같은 URL 경로 리턴 | ||
| return "/uploads/" + filename; | ||
| } |
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
PR 빌드 검증 결과: ❌ 빌드 실패빌드 검증이 완료되었습니다. |
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/jombi/freemates/model/postgres/Bookmark.java (1)
37-38: 과거 피드백 반영 확인됨과거 리뷰에서 요청된
@ManyToOne(fetch = FetchType.LAZY)가 적절히 적용되었습니다. 이는 N+1 문제를 방지하는데 도움이 됩니다.src/main/java/jombi/freemates/service/BookmarkService.java (1)
31-71: 파일 검증 및 로깅 개선 필요기본적인 null/empty 체크는 추가되었지만, 여전히 파일 크기 제한과 허용 형식 검증이 누락되어 있습니다. 또한 실패 시 로깅이 부족합니다.
누락된 검증 항목:
- 파일 크기 제한 (예: 최대 5MB)
- 허용 파일 형식 검증 (예: image/png, image/jpeg)
- 실패 케이스 로깅
다음과 같이 개선을 권장합니다:
@Transactional public BookmarkResponse create( CustomUserDetails customUserDetails, BookmarkRequest req, MultipartFile image ) { + log.info("북마크 생성 요청 - 사용자: {}, 제목: {}", + customUserDetails.getUsername(), req.getTitle()); String imageUrl = null; if (image != null && !image.isEmpty()) { + // 파일 크기 검증 (5MB 제한) + if (image.getSize() > 5 * 1024 * 1024) { + log.warn("파일 크기 초과 - 사용자: {}, 크기: {}", + customUserDetails.getUsername(), image.getSize()); + throw new InvalidRequestException("파일 크기가 5MB를 초과합니다."); + } + + // 파일 형식 검증 + String contentType = image.getContentType(); + if (!List.of("image/png", "image/jpeg", "image/jpg").contains(contentType)) { + log.warn("지원하지 않는 파일 형식 - 사용자: {}, 타입: {}", + customUserDetails.getUsername(), contentType); + throw new InvalidRequestException("지원하지 않는 이미지 형식입니다."); + } + imageUrl = storage.storeImage(image); + log.info("이미지 저장 완료 - URL: {}", imageUrl); }
🧹 Nitpick comments (2)
src/main/java/jombi/freemates/model/dto/BookmarkRequest.java (1)
10-10: 사용되지 않는 import 제거 필요
RequiredArgsConstructor와MultipartFileimport가 사용되지 않고 있습니다. 코드 정리를 위해 제거해주세요.-import lombok.RequiredArgsConstructor; -import org.springframework.web.multipart.MultipartFile;Also applies to: 13-13
src/main/java/jombi/freemates/service/BookmarkService.java (1)
73-87: 성능 모니터링을 위한 로깅 추가 권장메서드 로직은 올바르나 사용자별 북마크 조회 성능 모니터링을 위한 로깅이 있으면 운영에 도움이 될 것 같습니다.
@Transactional(Transactional.TxType.SUPPORTS) public List<BookmarkResponse> listByMember(CustomUserDetails customUserDetails) { Member member = customUserDetails.getMember(); + log.debug("북마크 목록 조회 - 사용자: {}", member.getNickname()); return bookmarkRepository.findAllByMember(member).stream() .map(b -> BookmarkResponse.builder() .memberId(member.getMemberId()) .nickname(member.getNickname()) .imageUrl(b.getImageUrl()) .title(b.getTitle()) .description(b.getDescription()) .pinColor(b.getPinColor()) .visibility(b.getVisibility()) .build()) .collect(Collectors.toList()); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/jombi/freemates/controller/BookmarkController.java(1 hunks)src/main/java/jombi/freemates/controller/PlaceController.java(2 hunks)src/main/java/jombi/freemates/model/dto/BookmarkRequest.java(1 hunks)src/main/java/jombi/freemates/model/dto/BookmarkResponse.java(1 hunks)src/main/java/jombi/freemates/model/postgres/Bookmark.java(3 hunks)src/main/java/jombi/freemates/model/postgres/Place.java(3 hunks)src/main/java/jombi/freemates/repository/BookmarkRepository.java(1 hunks)src/main/java/jombi/freemates/service/BookmarkService.java(1 hunks)src/main/java/jombi/freemates/service/FileStorageService.java(1 hunks)src/main/java/jombi/freemates/service/PlaceService.java(2 hunks)src/main/java/jombi/freemates/util/exception/ErrorCode.java(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/java/jombi/freemates/service/PlaceService.java
- src/main/java/jombi/freemates/model/postgres/Place.java
- src/main/java/jombi/freemates/util/exception/ErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/jombi/freemates/controller/PlaceController.java
- src/main/java/jombi/freemates/service/FileStorageService.java
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/jombi/freemates/model/dto/BookmarkRequest.java (1)
src/main/java/jombi/freemates/model/dto/BookmarkResponse.java (1)
Builder(13-30)
src/main/java/jombi/freemates/model/dto/BookmarkResponse.java (1)
src/main/java/jombi/freemates/model/dto/BookmarkRequest.java (1)
ToString(15-31)
src/main/java/jombi/freemates/model/postgres/Bookmark.java (1)
src/main/java/jombi/freemates/model/postgres/Place.java (1)
Entity(29-88)
src/main/java/jombi/freemates/controller/BookmarkController.java (1)
src/main/java/jombi/freemates/service/BookmarkService.java (1)
Slf4j(23-119)
🔇 Additional comments (8)
src/main/java/jombi/freemates/model/dto/BookmarkResponse.java (1)
1-31: LGTM! 표준적인 응답 DTO 구조입니다.북마크 응답 데이터를 담기 위한 DTO 클래스가 잘 구성되어 있습니다. Lombok 어노테이션을 적절히 사용하여 boilerplate 코드를 줄였고, 필드들도 북마크 기능에 필요한 정보들을 포함하고 있습니다.
src/main/java/jombi/freemates/model/dto/BookmarkRequest.java (1)
21-31: LGTM! 북마크 요청 DTO가 잘 구성되어 있습니다.필요한 필드들이 적절히 정의되어 있고 Swagger 문서화를 위한 Schema 어노테이션도 포함되어 있습니다.
src/main/java/jombi/freemates/repository/BookmarkRepository.java (1)
1-17: LGTM! 잘 설계된 Repository 인터페이스입니다.Spring Data JPA 규약을 잘 따라 구현되었고, 메서드명이 명확하여 가독성이 좋습니다.
findAllByMember와findByBookmarkIdAndMember메서드는 북마크 기능에 필요한 쿼리들을 적절히 표현하고 있습니다.src/main/java/jombi/freemates/model/postgres/Bookmark.java (3)
3-3: 적절한 import 추가새로운 JPA 어노테이션들과 컬렉션 관련 import들이 필요에 따라 잘 추가되었습니다.
Also applies to: 7-7, 11-16
34-35: ID 필드 개선 사항 승인
favoriteId에서bookmarkId로 변경된 것이 의미상 더 명확하고,@Column어노테이션으로updatable = false, nullable = false속성을 명시한 것도 좋습니다. 이는 ID 필드의 불변성과 필수성을 명확히 표현합니다.
52-58: Many-to-Many 관계 설정이 적절합니다북마크와 장소 간의 다대다 관계가 적절히 설정되었습니다.
@JoinTable을 사용하여 중간 테이블bookmark_place의 구조를 명확히 정의했고,ArrayList로 초기화하여 NPE를 방지했습니다.src/main/java/jombi/freemates/service/BookmarkService.java (1)
89-118: 잘 구현된 메서드입니다권한 검증, 엔티티 존재 확인, 중복 방지, 적절한 로깅이 모두 포함되어 있어 안전하고 신뢰할 수 있는 구현입니다.
src/main/java/jombi/freemates/controller/BookmarkController.java (1)
123-128: 깔끔한 구현입니다서비스로의 단순 위임과 인증 처리가 적절하게 구현되어 있습니다.
PR 빌드 검증 결과: ✅ 빌드 성공빌드 검증이 완료되었습니다. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/jombi/freemates/controller/BookmarkController.java (1)
88-93: 수동 DTO 매핑보다는 검증 어노테이션 활용 고려현재 개별
@RequestParam으로 받아서 수동으로BookmarkRequest객체를 구성하고 있습니다. 요청 데이터 검증을 위해BookmarkRequest클래스에@NotBlank,@NotNull등의 Bean Validation 어노테이션을 추가하고@Valid를 사용하는 것을 고려해보세요.// BookmarkRequest에 검증 어노테이션 추가 후 @PostMapping(value = "/create", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) @ResponseStatus(HttpStatus.CREATED) public ResponseEntity<BookmarkResponse> create( @Valid @ModelAttribute BookmarkRequest req, @RequestParam(value = "image", required = false) MultipartFile image, @AuthenticationPrincipal CustomUserDetails user ) { BookmarkResponse response = bookmarkService.create(user, req, image); return ResponseEntity.status(HttpStatus.CREATED).body(response); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/jombi/freemates/controller/BookmarkController.java(1 hunks)
🔇 Additional comments (3)
src/main/java/jombi/freemates/controller/BookmarkController.java (3)
1-42: 클래스 구조와 의존성 주입이 올바르게 구현됨컨트롤러 클래스의 기본 구조가 Spring Boot 베스트 프랙티스를 잘 따르고 있습니다. Swagger 문서화, 로깅, 의존성 주입이 적절히 설정되어 있습니다.
122-127: 단순하고 명확한 목록 조회 구현인증된 사용자의 즐겨찾기 목록을 조회하는 간단한 엔드포인트로, 구현이 명확하고 적절합니다.
162-171: API 문서와 구현이 일치함을 확인과거 리뷰에서 지적된 Request Body vs RequestParam 불일치 문제가 해결되었습니다. 현재 문서(149-150라인)와 구현(166라인) 모두 RequestParam을 사용하도록 일치되어 있습니다.
| @PostMapping(value = "/create", | ||
| consumes = MediaType.MULTIPART_FORM_DATA_VALUE) | ||
| @ResponseStatus(HttpStatus.CREATED) | ||
| public ResponseEntity<BookmarkResponse> create( | ||
| @RequestParam String title, | ||
| @RequestParam String description, | ||
| @RequestParam PinColor pinColor, | ||
| @RequestParam Visibility visibility, | ||
| @RequestParam(value = "image", required = false) | ||
| @Parameter(description = "이미지 파일", content = @Content(mediaType = MediaType.MULTIPART_FORM_DATA_VALUE)) | ||
| MultipartFile image, | ||
| @AuthenticationPrincipal CustomUserDetails user | ||
| ) { | ||
| BookmarkRequest req = new BookmarkRequest(); | ||
| req.setTitle(title); | ||
| req.setDescription(description); | ||
| req.setPinColor(pinColor); | ||
| req.setVisibility(visibility); | ||
| BookmarkResponse response = bookmarkService.create(user, req, image); | ||
| return ResponseEntity.status(HttpStatus.CREATED).body(response); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
파일 업로드 검증 로직 확인 필요
멀티파트 파일 업로드 엔드포인트가 올바르게 구현되어 있으나, 파일 크기 제한이나 허용 형식에 대한 검증이 컨트롤러나 서비스 레이어에서 처리되고 있는지 확인이 필요합니다.
다음 스크립트로 BookmarkService에서 파일 검증 로직이 구현되어 있는지 확인하겠습니다:
🏁 Script executed:
#!/bin/bash
# BookmarkService의 create 메서드에서 파일 검증 로직 확인
echo "=== BookmarkService 파일 위치 확인 ==="
fd -t f "BookmarkService.java"
echo "=== create 메서드 내용 확인 ==="
rg -A 20 "public.*create.*CustomUserDetails.*BookmarkRequest.*MultipartFile" --type java
echo "=== 파일 검증 관련 코드 검색 ==="
rg -B 5 -A 10 "getSize\(\)|getContentType\(\)|file.*valid|upload.*valid" --type javaLength of output: 456
파일 업로드 검증 로직 추가 필요
서비스 레이어(create 메서드)에서 MultipartFile에 대한 크기·형식 검증이 전혀 구현되어 있지 않습니다. 악성 파일 업로드나 서버 과부하를 방지하기 위해 아래 위치에 검증 로직을 추가해주세요.
• src/main/java/jombi/freemates/service/BookmarkService.java
– public BookmarkResponse create(CustomUserDetails user, BookmarkRequest req, MultipartFile image) 내 파일 유효성 검사
• src/main/java/jombi/freemates/controller/BookmarkController.java
– 컨트롤러 단에서도 요청 파라미터 유효성(필수 여부, 최대 크기 등) 재검증
추가 검증 예시:
- image.getSize()로
maxFileSize비교 - image.getContentType()으로 허용된 MIME 타입(
image/png,image/jpeg등) 확인 - 위반 시
BadRequestException또는MultipartException발생 처리
🤖 Prompt for AI Agents
In src/main/java/jombi/freemates/controller/BookmarkController.java lines 75 to
95 and in src/main/java/jombi/freemates/service/BookmarkService.java within the
create method, add validation logic for the MultipartFile image parameter. In
the controller, re-validate request parameters including checking if the image
size exceeds a defined maxFileSize and if the content type is among allowed MIME
types like image/png or image/jpeg. In the service layer's create method,
implement similar checks on image.getSize() and image.getContentType(), and if
validation fails, throw appropriate exceptions such as BadRequestException or
MultipartException to prevent malicious or oversized file uploads.
#96
Summary by CodeRabbit
신규 기능
/uploads/**경로를 통해 접근할 수 있습니다.설정 및 보안
문서화