[Feat] 데모 시연용 트래킹 사진 조회 API 구현#143
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 본 PR은 데모 시연을 원활하게 진행하기 위한 트래킹 사진 조회 기능을 구현합니다. 시스템에 미리 등록된 데모 사진들과 해당 세션에서 실제 촬영된 사진들을 조합하여 제공함으로써, 시연 환경에서 풍부한 데이터를 보여줄 수 있도록 설계되었습니다. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a demo endpoint /api/demo/tracking/sessions/{sessionId}/photos along with its configuration properties and Swagger documentation to retrieve a combined list of random demo photos and uploaded tracking photos. Feedback highlights the need to prevent potential NullPointerException in DemoProperties by handling null values, and recommends extracting the business logic from DemoController into a dedicated DemoService to ensure proper separation of concerns and transaction management.
| public record DemoProperties( | ||
| List<String> photoFilenames | ||
| ) {} |
There was a problem hiding this comment.
[P2] NullPointerException 방지를 위한 기본값 설정
demo.photo-filenames 설정이 누락되었거나 빈 값일 경우, demoProperties.photoFilenames()가 null을 반환하여 DemoController에서 NullPointerException이 발생할 위험이 있습니다.
Record의 콤팩트 생성자(Compact Constructor)를 사용하여 null인 경우 빈 리스트로 초기화하도록 방어 코드를 추가하는 것이 안전합니다.
| public record DemoProperties( | |
| List<String> photoFilenames | |
| ) {} | |
| public record DemoProperties( | |
| List<String> photoFilenames | |
| ) { | |
| public DemoProperties { | |
| photoFilenames = photoFilenames == null ? List.of() : photoFilenames; | |
| } | |
| } |
References
- Check soft delete behavior, nullable policies, enum expansion, and DB constraints for consistency. (link)
| private final DemoProperties demoProperties; | ||
| private final MinioClient minioClient; | ||
| private final MinioProperties minioProperties; | ||
| private final TrackingPhotoRepository trackingPhotoRepository; | ||
|
|
||
| @GetMapping("/tracking/sessions/{sessionId}/photos") | ||
| @Override | ||
| public ResponseEntity<ApiResponse<List<String>>> getDemoPhotos( | ||
| @PathVariable Long sessionId, | ||
| @RequestParam(defaultValue = "3") int count | ||
| ) { | ||
| List<String> shuffled = new ArrayList<>(demoProperties.photoFilenames()); | ||
| Collections.shuffle(shuffled); | ||
| List<String> randomUrls = shuffled.subList(0, Math.min(count, shuffled.size())) | ||
| .stream() | ||
| .map(this::presignedGetUrl) | ||
| .toList(); | ||
|
|
||
| List<String> uploadedUrls = trackingPhotoRepository | ||
| .findByTrackingSession_IdOrderByMilestoneIndexAsc(sessionId) | ||
| .stream() | ||
| .map(TrackingPhoto::getImageUrl) | ||
| .toList(); | ||
|
|
||
| List<String> combined = new ArrayList<>(randomUrls); | ||
| combined.addAll(uploadedUrls); | ||
|
|
||
| return ApiResponse.success(SuccessStatus.TRACKING_PHOTO_LIST_SUCCESS, combined); | ||
| } | ||
|
|
||
| private String presignedGetUrl(String objectKey) { | ||
| try { | ||
| String url = minioClient.getPresignedObjectUrl( | ||
| GetPresignedObjectUrlArgs.builder() | ||
| .method(Method.GET) | ||
| .bucket(BUCKET) | ||
| .object(objectKey) | ||
| .expiry(1, TimeUnit.HOURS) | ||
| .build() | ||
| ); | ||
| return url.replace(minioProperties.endpoint(), minioProperties.publicUrl()); | ||
| } catch (Exception e) { | ||
| log.warn("Failed to generate presigned URL for {}", objectKey, e); | ||
| return minioProperties.publicUrl() + "/" + BUCKET + "/" + objectKey; | ||
| } | ||
| } |
There was a problem hiding this comment.
[P2] 컨트롤러 내 비즈니스 로직 분리 및 트랜잭션 적용
현재 DemoController에서 직접 MinIO presigned URL을 생성하고, 사진 목록을 셔플하며, Repository를 호출하는 등 핵심 비즈니스 로직을 모두 처리하고 있습니다.
이는 컨트롤러와 서비스의 책임을 명확히 분리하라는 설계 원칙(Repository Style Guide 20, 21번)에 위배됩니다. 또한, 조회 전용 작업임에도 트랜잭션 경계가 설정되지 않아 성능 최적화(@Transactional(readOnly = true)) 혜택을 받지 못합니다.
개선 제안:
비즈니스 로직을 처리하는 DemoService를 추가하여 컨트롤러의 책임을 최소화하고, 서비스 메서드에 @Transactional(readOnly = true)를 적용해 주세요.
private final DemoService demoService;
@GetMapping("/tracking/sessions/{sessionId}/photos")
@Override
public ResponseEntity<ApiResponse<List<String>>> getDemoPhotos(
@PathVariable Long sessionId,
@RequestParam(defaultValue = "3") int count
) {
List<String> combined = demoService.getDemoPhotos(sessionId, count);
return ApiResponse.success(SuccessStatus.TRACKING_PHOTO_LIST_SUCCESS, combined);
}References
1 participant
🧾 요약
🔗 이슈
✨ 변경 내용
DemoProperties설정 프로퍼티 추가 (데모 사진 파일명 목록)GET /api/demo/tracking/sessions/{sessionId}/photosAPI 구현application.yaml,application-prod.yaml에 데모 사진 파일명 설정 추가✅ 확인