-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/#54 제휴 장소 관련 조회 기능 성능 최적화 #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…한번에 조회하는 로직으로 수정, 조회된 게시글이 없을 시 빈 리스트를 반환하여 리소스를 아끼도록 수정, Collectors.toSet()을 통해 중복 제거해 IN 쿼리 낭비가 없도록 수정
…tCouncilPost와 Place를 JOIN하여 한번에 조회하도록 수정 (N+1 문제 해결)
Walkthrough파트너십 기능을 place 도메인으로 통합하고 PartnershipService를 PartnershipPlaceService로 이동/리팩토링했으며, 배치 이미지 조회 메서드 추가, 리포지토리 JOIN/ FETCH 변경, 컨트롤러/서비스 시그니처 및 호출지점 조정이 반영되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PlaceController
participant PartnershipPlaceService
participant PostImageRepository
participant LikedPlacesRepository
participant DB
Client->>PlaceController: GET /places/partnership?lat,lng,cursor,size
PlaceController->>PartnershipPlaceService: getPartnershipPlaces(params, userId)
PartnershipPlaceService->>DB: 조회: 파트너십 게시글 페이징 쿼리
PartnershipPlaceService->>PostImageRepository: findAllByPostIn(posts)
PostImageRepository->>DB: select images where post IN (...)
PartnershipPlaceService->>LikedPlacesRepository: findLikedPlaceIds(userId, placeIds)
LikedPlacesRepository->>DB: select liked place ids
PartnershipPlaceService->>PlaceController: DTO 생성(이미지 목록 및 isLiked 포함)
PlaceController->>Client: 200 OK (partnership list)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (2)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (2)
src/main/java/com/campus/campus/domain/councilpost/domain/repository/PostImageRepository.java (1)
20-29: 배치 조회 메서드의 정렬 순서 보장 필요
findAllByPostIn(...)은 데이터베이스 수준에서 반환 순서를 보장하지 않습니다. 현재 PartnershipPlaceService에서는 grouping 이후 전체 이미지 리스트를 반환하지만, 데이터 일관성과 예측 가능성을 위해 기존findAllByPostOrderByIdAsc(post)메서드처럼 명시적 정렬을 추가하는 것을 권장합니다. 향후 첫 번째 이미지를 썸네일로 사용하는 경우를 대비해서도 필요합니다.권장 구현
public interface PostImageRepository extends JpaRepository<PostImage, Long> { - List<PostImage> findAllByPostIn(List<StudentCouncilPost> posts); + @Query(""" + select pi + from PostImage pi + where pi.post in :posts + order by pi.post.id asc, pi.id asc + """) + List<PostImage> findAllByPostIn(@Param("posts") List<StudentCouncilPost> posts); }src/main/java/com/campus/campus/domain/councilpost/domain/repository/StudentCouncilPostRepository.java (1)
215-247: INNER JOIN이 PARTNERSHIP 카테고리 특화 최적화인지 확인 필요. 메서드명이 일반적이라 향후 재사용 시 주의.현재 코드에서
findByUserScopeWithCursor는 오직PartnershipPlaceService.getPartnershipPlaces()에서만PostCategory.PARTNERSHIP으로 호출되며, PARTNERSHIP 게시글 생성 시placeService.findOrCreatePlace(dto)로 항상 place가 생성되므로 INNER JOIN 필터링이 현재로는 의도된 것으로 보입니다. 또한 유사 메서드findPinsInBounds도 동일하게 INNER JOIN을 사용합니다.그러나 메서드명
findByUserScopeWithCursor는 카테고리별 제약을 명시하지 않으므로, 향후 EVENT 등 다른 카테고리로 재사용될 경우 예상치 못한 필터링으로 기능 누락이 발생할 수 있습니다. 의도가 PARTNERSHIP 전용이면 메서드명 변경이나 주석을 통해 명시하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In
@src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java:
- Around line 121-129: The call to migrateImagesToOci can throw an NPE if
imageUrls is null because likePlace currently calls migrateImagesToOci without
checking; update likePlace (the call at ~line 159) to mirror findOrCreatePlace
and only call migrateImagesToOci when placeInfo.imgUrls() != null &&
!placeInfo.imgUrls().isEmpty(), and/or add a defensive guard at the start of
migrateImagesToOci (method migrateImagesToOci) that returns immediately if
imageUrls is null or empty to prevent the for-each NPE.
In
@src/main/java/com/campus/campus/domain/place/presentation/PlaceResponseCode.java:
- Around line 15-17: Remove the unused enum constant
CHECK_PARTNERSHIP_PLACE_SUCCESS from the PlaceResponseCode enum: delete the
CHECK_PARTNERSHIP_PLACE_SUCCESS declaration and adjust the remaining enum
entries (CHECK_PARTNERSHIP_PLACES_SUCCESS and
CHECK_ONE_PARTNERSHIP_PLACE_SUCCESS) so the enum syntax remains valid
(commas/semicolon placement). Verify no other code references
CHECK_PARTNERSHIP_PLACE_SUCCESS and run tests/compile to ensure no usages
remain.
🧹 Nitpick comments (5)
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (1)
35-37: 타임아웃 설정 일관성 개선을 고려하세요.연결 타임아웃(10초)이 일부 요청 타임아웃(3초, lines 112, 145)보다 길게 설정되어 있습니다. 요청 타임아웃이 전체 작업 시간을 제어하므로, 연결 타임아웃은 그보다 짧거나 같아야 합니다. 또한 HttpClient 레벨에서 읽기 타임아웃을 명시적으로 설정하면 더 일관된 타임아웃 관리가 가능합니다.
♻️ 타임아웃 설정 개선 제안
HttpClient httpClient = HttpClient.create() - .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 10000) + .option(ChannelOption.CONNECT_TIMEOUT_MILLIS, 3000) + .responseTimeout(Duration.ofSeconds(10)) .followRedirect(true);이렇게 하면 연결 타임아웃을 3초로 낮추고, 읽기 타임아웃을 10초로 설정하여 메서드별 타임아웃과 충돌하지 않으면서도 기본 보호 장치를 제공합니다.
src/main/java/com/campus/campus/domain/councilpost/domain/repository/StudentCouncilPostRepository.java (1)
249-283:JOIN FETCH p.place로 핀 조회 N+1 제거 방향은 좋습니다(좌표 인덱스는 운영 관점에서 확인 권장).
Line 253의JOIN FETCH p.place pl+ 좌표 범위 조건으로 place 접근 시 N+1을 줄일 수 있어 보입니다. 트래픽이 크면pl.coordinate.latitude/longitude컬럼 인덱스(또는 공간 인덱스) 여부만 운영 관점에서 점검하면 더 안전합니다.src/main/java/com/campus/campus/domain/place/presentation/PlaceController.java (1)
127-162: 지도 bounds 기반 제휴 장소 조회 엔드포인트.좌표 범위를 이용한 장소 조회 API가 적절히 구현되었습니다.
Doublewrapper 타입 사용으로 null 체크가 가능하지만, 필수 파라미터이므로doubleprimitive 타입으로 통일하는 것도 고려해볼 수 있습니다.src/main/java/com/campus/campus/domain/place/application/service/PartnershipPlaceService.java (2)
132-133: 읽기 전용 메서드에 readOnly 옵션 누락.
findPartnerInBounds는 데이터를 조회만 하므로@Transactional(readOnly = true)를 사용하는 것이 좋습니다. Hibernate 최적화 및 DB 레플리카 활용에 유리합니다.♻️ 수정 제안
- @Transactional + @Transactional(readOnly = true) public List<PartnershipPinResponse> findPartnerInBounds(Long userId, Double minLat, Double maxLat, Double minLng, Double maxLng) {
158-159: 읽기 전용 메서드에 readOnly 옵션 누락.
getPartnershipDetail도 조회 전용 메서드이므로@Transactional(readOnly = true)를 적용해야 합니다.♻️ 수정 제안
- @Transactional + @Transactional(readOnly = true) public PartnershipResponse getPartnershipDetail(Long postId, Long userId, double userLat, double userLng) {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/campus/campus/domain/councilpost/domain/repository/PostImageRepository.javasrc/main/java/com/campus/campus/domain/councilpost/domain/repository/StudentCouncilPostRepository.javasrc/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostForUserController.javasrc/main/java/com/campus/campus/domain/partnership/presentation/PartnershipController.javasrc/main/java/com/campus/campus/domain/partnership/presentation/PartnershipResponseCode.javasrc/main/java/com/campus/campus/domain/place/application/dto/response/PartnershipPinResponse.javasrc/main/java/com/campus/campus/domain/place/application/mapper/PlaceMapper.javasrc/main/java/com/campus/campus/domain/place/application/service/PartnershipPlaceService.javasrc/main/java/com/campus/campus/domain/place/application/service/PlaceService.javasrc/main/java/com/campus/campus/domain/place/domain/repository/LikedPlacesRepository.javasrc/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.javasrc/main/java/com/campus/campus/domain/place/presentation/PlaceController.javasrc/main/java/com/campus/campus/domain/place/presentation/PlaceResponseCode.java
💤 Files with no reviewable changes (2)
- src/main/java/com/campus/campus/domain/partnership/presentation/PartnershipController.java
- src/main/java/com/campus/campus/domain/partnership/presentation/PartnershipResponseCode.java
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (3)
src/main/java/com/campus/campus/domain/place/domain/entity/PlaceImages.java (1)
Entity(18-40)src/main/java/com/campus/campus/domain/place/domain/entity/Place.java (1)
Entity(17-57)src/main/java/com/campus/campus/domain/place/domain/repository/PlaceImagesRepository.java (1)
PlaceImagesRepository(10-15)
src/main/java/com/campus/campus/domain/place/presentation/PlaceResponseCode.java (1)
src/main/java/com/campus/campus/domain/partnership/presentation/PartnershipController.java (1)
GetMapping(28-67)
src/main/java/com/campus/campus/domain/place/application/dto/response/PartnershipPinResponse.java (4)
src/main/java/com/campus/campus/domain/partnership/application/dto/response/PartnershipPinResponse.java (1)
PartnershipPinResponse(5-22)src/main/java/com/campus/campus/domain/place/application/dto/response/partnership/PartnershipResponse.java (1)
PartnershipResponse(6-24)src/main/java/com/campus/campus/domain/partnership/application/service/PartnershipService.java (2)
post(128-128)post(127-127)src/main/java/com/campus/campus/domain/councilpost/application/dto/response/GetActivePartnershipListForUserResponse.java (1)
GetActivePartnershipListForUserResponse(5-18)
src/main/java/com/campus/campus/domain/councilpost/domain/repository/PostImageRepository.java (3)
src/main/java/com/campus/campus/domain/partnership/application/service/PartnershipService.java (1)
getImgUrls(160-162)src/main/java/com/campus/campus/domain/councilnotice/domain/repository/NoticeImageRepository.java (1)
NoticeImageRepository(10-17)src/main/java/com/campus/campus/domain/place/domain/repository/PlaceImagesRepository.java (1)
PlaceImagesRepository(10-15)
src/main/java/com/campus/campus/domain/place/application/service/PartnershipPlaceService.java (1)
src/main/java/com/campus/campus/global/util/geocoder/GeoUtil.java (1)
GeoUtil(3-32)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (15)
src/main/java/com/campus/campus/domain/councilpost/presentation/StudentCouncilPostForUserController.java (2)
37-37: LGTM!필드 선언이 올바르며, Lombok의
@RequiredArgsConstructor를 통한 의존성 주입 패턴이 적절하게 적용되었습니다.
81-86: LGTM!메서드 호출부를 여러 줄로 분할한 포맷팅 변경입니다. 가독성을 개선하면서 로직에는 영향을 주지 않습니다.
Also applies to: 114-115, 145-146
src/main/java/com/campus/campus/domain/place/infrastructure/google/GooglePlaceClient.java (2)
39-41: WebClient 구성이 적절합니다.ExchangeStrategies를 통해 최대 인메모리 크기를 10 MB로 설정하고, 커스텀 HTTP 클라이언트를 ReactorClientHttpConnector로 통합한 구성이 올바릅니다. 이미지 다운로드 용도로 10 MB 버퍼는 충분합니다.
Also applies to: 46-47
165-165: 쿼리 파라미터 이름 변경이 올바릅니다.
maxWidth에서maxwidth로 변경된 것이 맞습니다. Google Places Photo API의 레거시 버전에서는maxwidth(소문자)가 정확한 파라미터 이름이며, API는 대소문자를 구분합니다. 이 수정이 "장소 검색 불가" 버그의 원인이었을 가능성이 높습니다.src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (1)
121-129: 트랜잭션 범위 내에서 이미지 마이그레이션 실패 시 동작을 확인해주세요.현재 구현에서는 Place 엔티티를 먼저 저장(122번 라인)한 후 이미지 마이그레이션(125번 라인)을 수행합니다.
migrateImagesToOci메서드는 개별 이미지 업로드 실패 시 경고 로그만 남기고 예외를 전파하지 않습니다(246-248번 라인).이는 이미지 마이그레이션이 실패하더라도 Place는 DB에 저장되며, 해당 장소에 대한 이미지가 없는 상태로 남을 수 있음을 의미합니다. 이것이 의도된 동작인지 확인이 필요합니다.
만약 이미지 마이그레이션이 필수적이라면, 실패 시 롤백하거나 재시도 로직을 고려해야 합니다. 반면 "best effort" 방식이 의도된 것이라면, 이미지가 없는 장소에 대한 후처리 로직(예: 배치 작업으로 재시도)이 있는지 확인해주세요.
src/main/java/com/campus/campus/domain/place/application/dto/response/PartnershipPinResponse.java (1)
1-1: 패키지 이동 정리 LGTM (참조 import만 일괄 갱신 확인).
PartnershipPinResponse패키지 이동 자체는 좋고, PR 방향(파트너십→place 도메인 통합)과도 일치합니다. 남은 컴파일 이슈가 없도록 전 사용처 import만 전부 갱신됐는지 확인만 해주세요.src/main/java/com/campus/campus/domain/place/application/mapper/PlaceMapper.java (1)
7-14: Import 갱신만 반영된 점 깔끔합니다.
매핑 로직 변경 없이 DTO 이동만 반영되어 영향 범위가 최소화되어 좋습니다.src/main/java/com/campus/campus/domain/place/domain/repository/LikedPlacesRepository.java (1)
18-27:Set<Long> placeIds파라미터로 IN 조건 최적화는 올바른 방향입니다.빈 Set 관련 우려사항은 이미 해결되었습니다.
PartnershipPlaceService(라인 105-108)에서placeIds.isEmpty()체크 후 빈 경우Collections.emptySet()을 반환하므로, 빈 Set이 repository 메서드로 전달되지 않습니다.src/main/java/com/campus/campus/domain/place/presentation/PlaceController.java (3)
86-125: 무한 스크롤 제휴 장소 목록 조회 엔드포인트 구현 확인.Swagger 문서화가 잘 되어 있고, cursor 기반 페이지네이션 파라미터가 적절히 정의되었습니다.
@CurrentUserId를 통한 사용자 인증 처리도 올바릅니다.
164-181: 제휴 장소 상세 조회 엔드포인트.단건 조회 API가 적절히 구현되었습니다. 파라미터 순서가 lat, lng, postId, userId 순서로 일관성 있게 정의되었습니다.
13-14: 제휴 장소 서비스 의존성 및 import 추가가 올바르게 구현됨.PartnershipPlaceService가 제대로 주입되었고, PartnershipPinResponse, PartnershipResponse 등 관련 DTO가 place 패키지 구조에 맞게 import되었습니다. 세 개의 partnership 엔드포인트(/partnership, /partnership/map, /partnership/detail)가 PlaceController에 통합되어 있으며, 별도의 PartnershipController는 존재하지 않으므로 중복 엔드포인트의 우려는 없습니다.
src/main/java/com/campus/campus/domain/place/application/service/PartnershipPlaceService.java (4)
49-50: 읽기 전용 트랜잭션 추가 - 좋은 최적화입니다.
@Transactional(readOnly = true)를 적용하여 Hibernate flush 모드 최적화 및 DB 레플리카 라우팅이 가능해졌습니다.
71-83: 거리 정렬 후 limit 적용 로직 검토 필요.DB에서
PageRequest.of(0, size)로size개의 게시글을 가져온 후, 다시 거리순 정렬 후.limit(size)를 적용하고 있습니다. 이미size개만 가져왔으므로 limit은 실질적으로 중복됩니다.cursor 기반 페이지네이션과 거리순 정렬을 함께 사용할 경우, 다음 페이지에서 더 가까운 장소가 누락될 수 있습니다. 의도된 동작인지 확인이 필요합니다.
85-109: N+1 문제 해결을 위한 배치 조회 - 좋은 최적화입니다.이미지와 좋아요 여부를 각각 단일 쿼리로 조회하여 N+1 문제를 해결했습니다.
placeIds가 빈 경우에 대한 방어 로직도 적절합니다.
180-186: 단건 조회용 헬퍼 메서드 유지.
getPartnershipDetail에서 단건 조회 시 사용되는isLiked와getImgUrls메서드가 남아있습니다. 배치 조회 최적화가 적용된getPartnershipPlaces와 달리 단건 조회에서는 이 방식이 적절합니다.
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java
Show resolved
Hide resolved
| CHECK_PARTNERSHIP_PLACE_SUCCESS(200, HttpStatus.OK, "제휴 장소 조회가 완료되었습니다."), | ||
| CHECK_PARTNERSHIP_PLACES_SUCCESS(200, HttpStatus.OK, "제휴 장소 리스트 조회가 완료되었습니다."), | ||
| CHECK_ONE_PARTNERSHIP_PLACE_SUCCESS(200, HttpStatus.OK, "제휴 장소 단건 조회가 완료되었습니다."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check usage of the response codes in question
echo "=== CHECK_PARTNERSHIP_PLACE_SUCCESS 사용처 ==="
rg -n "CHECK_PARTNERSHIP_PLACE_SUCCESS" --type java
echo ""
echo "=== CHECK_ONE_PARTNERSHIP_PLACE_SUCCESS 사용처 ==="
rg -n "CHECK_ONE_PARTNERSHIP_PLACE_SUCCESS" --type java
echo ""
echo "=== CHECK_PARTNERSHIP_PLACES_SUCCESS 사용처 ==="
rg -n "CHECK_PARTNERSHIP_PLACES_SUCCESS" --type javaRepository: our-campUS/our-campUS-BE
Length of output: 1155
🏁 Script executed:
#!/bin/bash
# Check the context of usage in PlaceController
echo "=== PlaceController 179번째 줄 근처 ==="
sed -n '175,185p' src/main/java/com/campus/campus/domain/place/presentation/PlaceController.java
echo ""
echo "=== PlaceController 124번째 줄 근처 ==="
sed -n '120,130p' src/main/java/com/campus/campus/domain/place/presentation/PlaceController.java
echo ""
echo "=== PlaceController 159번째 줄 근처 ==="
sed -n '155,165p' src/main/java/com/campus/campus/domain/place/presentation/PlaceController.javaRepository: our-campUS/our-campUS-BE
Length of output: 1069
CHECK_PARTNERSHIP_PLACE_SUCCESS 상수는 코드에서 사용되지 않으므로 제거해주세요.
검색 결과, CHECK_PARTNERSHIP_PLACE_SUCCESS는 enum 정의에만 존재하고 실제로 사용되는 곳이 없습니다. 반면 CHECK_ONE_PARTNERSHIP_PLACE_SUCCESS는 PlaceController의 단건 조회 메서드에서 사용되고 있으므로 유지가 필요합니다.
🤖 Prompt for AI Agents
In
@src/main/java/com/campus/campus/domain/place/presentation/PlaceResponseCode.java
around lines 15 - 17, Remove the unused enum constant
CHECK_PARTNERSHIP_PLACE_SUCCESS from the PlaceResponseCode enum: delete the
CHECK_PARTNERSHIP_PLACE_SUCCESS declaration and adjust the remaining enum
entries (CHECK_PARTNERSHIP_PLACES_SUCCESS and
CHECK_ONE_PARTNERSHIP_PLACE_SUCCESS) so the enum syntax remains valid
(commas/semicolon placement). Verify no other code references
CHECK_PARTNERSHIP_PLACE_SUCCESS and run tests/compile to ensure no usages
remain.
1224kang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java:
- Around line 72-73: The create() method calls
placeService.findOrCreatePlace(dto.place()) without a null check, which is
inconsistent with the defensive null check in update(); modify create() so it
guards dto.place() the same way as update() (e.g., if (dto.place() != null &&
... ) then call placeService.findOrCreatePlace(dto.place()) and handle the null
case appropriately) to match PostRequest validation behavior and maintain
consistency between create() and update().
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.javasrc/main/java/com/campus/campus/domain/place/application/service/PlaceService.java
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (1)
224-227: LGTM!
imageUrls에 대한 null/empty 체크를 추가하여 불필요한 처리를 방지하고 NPE를 예방하는 좋은 방어적 코딩입니다.src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java (1)
216-218: LGTM!
dto.place()null 체크가 적절히 되어 있고, 장소가 변경된 경우에만findOrCreatePlace를 호출하는 로직이 올바르게 구현되어 있습니다.
| //Place 객체 생성 | ||
| Place place = placeService.findOrCreatePlace(dto); | ||
| Place place = placeService.findOrCreatePlace(dto.place()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# PostRequest 파일 찾기
fd -t f "PostRequest.java" --exec cat {}Repository: our-campUS/our-campUS-BE
Length of output: 1752
🏁 Script executed:
# StudentCouncilPostService.java의 create 메서드와 update 메서드 주변 코드 확인
fd -t f "StudentCouncilPostService.java" --exec cat -n {} \;Repository: our-campUS/our-campUS-BE
Length of output: 12179
🏁 Script executed:
# PlaceService의 findOrCreatePlace 메서드 확인
fd -t f "PlaceService.java" --exec cat -n {} \;Repository: our-campUS/our-campUS-BE
Length of output: 19757
create() 메서드의 null 체크 부재로 인한 일관성 문제
PostRequest의 place 필드에는 @NotNull 검증이 적용되어 있지만, update() 메서드(line 216)에서는 if (dto.place() != null && ...) 체크가 있는 반면, create() 메서드(line 73)에서는 dto.place()를 null 체크 없이 바로 전달합니다. 검증 레이어에서 @NotNull로 보호되더라도, update() 메서드의 방어적 프로그래밍 접근과 일관성 있게 create() 메서드에도 동일한 null 체크를 추가하는 것이 좋습니다.
🤖 Prompt for AI Agents
In
@src/main/java/com/campus/campus/domain/councilpost/application/service/StudentCouncilPostService.java
around lines 72 - 73, The create() method calls
placeService.findOrCreatePlace(dto.place()) without a null check, which is
inconsistent with the defensive null check in update(); modify create() so it
guards dto.place() the same way as update() (e.g., if (dto.place() != null &&
... ) then call placeService.findOrCreatePlace(dto.place()) and handle the null
case appropriately) to match PostRequest validation behavior and maintain
consistency between create() and update().
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (2)
153-169:@Transactional내부에서DataIntegrityViolationException처리 시 주의 필요.
likePlace메서드는@Transactional이 적용되어 있습니다.DataIntegrityViolationException이 발생하면 Spring은 해당 트랜잭션을 rollback-only로 마킹합니다. 따라서 catch 블록 내의findByPlaceKey조회가 동일한 실패 트랜잭션 내에서 실행되어 예상대로 동작하지 않을 수 있습니다.
findOrCreatePlace처럼 Place 생성 로직을 별도의 비-트랜잭션 메서드로 분리하거나,Propagation.REQUIRES_NEW를 사용하는 것을 고려해 주세요.♻️ 개선 제안: Place 생성 로직을 findOrCreatePlace로 위임
@Transactional public LikeResponse likePlace(SavedPlaceInfo placeInfo, Long userId) { User user = userRepository.findById(userId) .orElseThrow(UserNotFoundException::new); String placeKey = placeInfo.placeKey(); //이미 좋아요가 존재하는지 확인 Optional<LikedPlace> likedPlace = likedPlacesRepository.findByUserIdAndPlace_PlaceKey(userId, placeKey); if (likedPlace.isPresent()) { //이미 좋아요 상태->좋아요 취소 likedPlacesRepository.delete(likedPlace.get()); return new LikeResponse(null, false); } - //Place 엔티티 생성 - Place place; - try { - //조회 - place = placeRepository.findByPlaceKey(placeKey) - .orElseGet(() -> { - //없으면 생성 - String placeName = stripHtml(placeInfo.placeName()); - Place newPlace = placeRepository.save(placeMapper.createPlace(placeInfo)); - //신규 생성된 경우에만 이미지 저장 - migrateImagesToOci(newPlace.getPlaceKey(), placeInfo.imgUrls()); - - return newPlace; - }); - } catch (DataIntegrityViolationException e) { - //동시 생성으로 unique 제약 위반 시 다시 조회 - place = placeRepository.findByPlaceKey(placeKey) - .orElseThrow(PlaceCreationException::new); - } + Place place = findOrCreatePlace(placeInfo); + //likedPlace 저장 LikedPlace savedLikedPlace = placeMapper.createLikedPlace(user, place); likedPlacesRepository.save(savedLikedPlace); return placeMapper.toLikeResponse(place); }
158-158: 사용되지 않는 변수.
placeName변수가 선언되었지만 사용되지 않습니다.🔧 제거 제안
place = placeRepository.findByPlaceKey(placeKey) .orElseGet(() -> { //없으면 생성 - String placeName = stripHtml(placeInfo.placeName()); Place newPlace = placeRepository.save(placeMapper.createPlace(placeInfo)); //신규 생성된 경우에만 이미지 저장 migrateImagesToOci(newPlace.getPlaceKey(), placeInfo.imgUrls());
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (2)
src/main/java/com/campus/campus/domain/place/application/exception/PlaceCreationException.java (1)
PlaceCreationException(5-9)src/main/java/com/campus/campus/domain/place/application/mapper/PlaceMapper.java (1)
createPlace(82-92)
🔇 Additional comments (2)
src/main/java/com/campus/campus/domain/place/application/service/PlaceService.java (2)
115-131: 동시성 처리 로직이 적절합니다.
@Transactional을 제거하고DataIntegrityViolationException을 catch하여 재조회하는 방식은 올바른 접근입니다. 트랜잭션이 없으므로save()실패 시 별도의 트랜잭션에서 재조회가 가능합니다.다만, 동시 요청이 발생했을 때 두 번째
findByPlaceKey호출 시점에 첫 번째 요청의 트랜잭션이 아직 커밋되지 않았다면 조회가 실패할 수 있습니다. 현재 구현에서는PlaceCreationException을 던지므로 안전하게 처리됩니다.
230-232: LGTM!null 및 빈 리스트에 대한 방어적 검사가 추가되어 불필요한 처리를 방지합니다.
🔀 변경 내용
✅ 작업 항목
기존1: 이미지 조회 및 좋아요 여부 조회에서 N+1 문제가 발생.
수정1: 단 한번만 조회되도록 하여 DB 왕복 횟수를 줄이고 DB 부하를 감소시켜 성능이 향상되도록 했습니다.
기존2: 일단 DB에서 조회한 후 JAVA 단에서 장소가 없는 글을 필터링하는 방식 사용 (이는 페이지네이션에서의 문제를 일으켜 무한 스크롤에 문제가 생길 수 있음)
수정2: 메모리 필터링을 기존 JAVA 단에서 DB 쿼리로 옮겨 쓰지 않는 데이터를 DB에서 가져오는 낭비를 줄였습니다.
기존: 게시글을 우선 전부 가져오고, 필요할 때마다 장소를 하나씩 조회하도록 구현. (N+1 문제 발생)
수정: 쿼리문에 JOIN FETCH를 사용해 게시글을 가져올 때 장소 데이터도 함께 가져오도록 해 네트워크 왕복이 딱 한번 발생하도록 했습니다.
추가 항목
📸 스크린샷 (선택)
장소 검색 기능이 동작하고 이미지가 반환되었을 때 해당 url로 접속 시 이미지가 제대로 나타납니다.
장소 좋아요 기능 동작 시 버킷에 저장된 이미지가 정상적으로 저장되어 조회되는 것을 확인했습니다.
📎 참고 이슈
관련 이슈 번호 #54
Summary by CodeRabbit
새로운 기능
개선 사항
버그 수정
✏️ Tip: You can customize this high-level summary in your review settings.