Conversation
Summary of ChangesHello @seamooll, 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! 이 Pull Request는 적금 추천 시스템의 아키텍처를 재구성하여 사용자 경험과 시스템 안정성을 향상시키는 데 중점을 둡니다. 기존의 복잡한 비동기 추천 생성 과정을 MBTI 테스트 완료 시점에 동기적으로 처리하도록 변경함으로써, 추천 결과를 더 빠르고 예측 가능하게 제공합니다. 또한, API 응답 DTO를 간소화하고 에러 처리 로직을 명확히 하여 개발 및 유지보수 효율성을 높였습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory 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. Changelog
Activity
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. 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 PR refactors the recommendation logic from asynchronous to synchronous, improving API response structure and error handling, aiming to reduce code complexity and enhance user experience. However, these changes, especially the synchronous processing and removal of idempotency checks, introduce a high risk of Denial of Service (DoS) due to thread exhaustion when calling the external Gemini API, and a medium risk of resource exhaustion from redundant recommendation generation. It is strongly recommended to re-implement asynchronous processing and restore idempotency checks to ensure system stability and cost-efficiency. Further improvements could include fast-fail for empty candidate sets, logging enhancements, and consistent API error codes.
| // 제미나이 호출 | ||
| log.info("[RecommendAsync] Gemini request. memberId={}, promptChars={}", memberId, prompt.length()); | ||
| log.info("[Recommend] Gemini request. memberId={}, promptChars={}", memberId, prompt.length()); | ||
| String raw = geminiClient.generateText(prompt); |
There was a problem hiding this comment.
The generateAndSaveRecommendations method performs a synchronous call to the Gemini API via geminiClient.generateText(prompt). This method is called directly from the RecommendationController and FinanceMbtiService, blocking the HTTP worker thread. The GeminiClient uses a small fixed thread pool (size 4) and has a long overall deadline (240s). If multiple concurrent requests are made, the HTTP worker threads will be exhausted while waiting for the limited Gemini threads or the API response, leading to a complete denial of service (DoS) for the entire application. It is highly recommended to revert to an asynchronous processing model or implement a more robust concurrency strategy with shorter timeouts and a larger, dedicated thread pool.
|
|
||
| @Transactional | ||
| public void generateAndSaveRecommendations(Long memberId) { | ||
| public SavingsResponseDTO.SavingsListResponse generateAndSaveRecommendations(Long memberId) { |
There was a problem hiding this comment.
The removal of idempotency checks for recommendation generation introduces a medium risk of resource exhaustion. Without this check, the system can generate duplicate recommendations and make unnecessary, expensive calls to the Gemini API, potentially leading to quota exhaustion, database bloat, and increased server load if the /api/savings/recommendations endpoint is spammed. It is crucial to restore the idempotency check to prevent redundant processing. Consider adding logic to check if recommendations already exist for a given memberMbtiTest before generating new ones, returning existing data if found.
// 멱등성 체크: 이미 해당 MBTI 테스트에 대한 추천이 존재하면, 새로 생성하지 않고 기존 데이터를 반환합니다.
// 참고: recommendationRepository에 existsByMemberMbtiTestId(Long memberMbtiTestId) 메소드 추가가 필요할 수 있습니다.
if (recommendationRepository.existsByMemberMbtiTestId(memberMbtiTest.getId())) {
log.info("[Recommend] 이미 추천 데이터가 존재하여 기존 데이터를 반환합니다. memberId={}, mbtiTestId={}", memberId, memberMbtiTest.getId());
return getRecommendation(memberId, null); // rsrvType=null로 전체 조회
}| log.info("[Recommend] MBTI 검사 후 자동 추천 생성 성공. memberId={}", memberId); | ||
| } catch (Exception e) { | ||
| // 제미나이 호출이 실패해도 MBTI 저장은 유지되도록 예외를 삼키고 로그만 남김 | ||
| log.error("[Recommend] MBTI 저장에는 성공했으나, 자동 추천 생성 중 오류 발생. memberId={}: {}", memberId, e.getMessage()); |
There was a problem hiding this comment.
예외를 로깅할 때 e.getMessage()만 사용하면 원인 파악에 필요한 정보가 부족할 수 있습니다. 특히 getMessage()가 null을 반환하는 경우 디버깅이 어려워집니다. 전체 스택 트레이스를 포함하도록 로거를 사용하는 것이 좋습니다.
| log.error("[Recommend] MBTI 저장에는 성공했으나, 자동 추천 생성 중 오류 발생. memberId={}: {}", memberId, e.getMessage()); | |
| log.error("[Recommend] MBTI 저장에는 성공했으나, 자동 추천 생성 중 오류 발생. memberId={}", memberId, e); |
| Pageable candidatePage = PageRequest.of(0, CANDIDATE_LIMIT); | ||
| List<SavingsOption> candidates = savingsOptionRepository.findCandidates(candidatePage); |
There was a problem hiding this comment.
추천 상품 후보군(candidates)이 비어있는 경우에 대한 처리가 없습니다. 이 경우 불필요하게 Gemini API를 호출하게 되어 비효율적이며, 결국 RECOMMENDATION_FAILED 예외가 발생합니다.
후보군 조회 직후 리스트가 비어있는지 확인하고, 비어있다면 API 호출 없이 바로 예외를 발생시키는 '빠른 실패(fail-fast)' 로직을 추가하는 것이 좋습니다. candidates 조회 후에 아래 코드를 추가하는 것을 제안합니다.
if (candidates.isEmpty()) {
log.warn("[Recommend] 추천 상품 후보가 없습니다. memberId={}", memberId);
throw new SavingsException(SavingsErrorCode.RECOMMENDATION_FAILED, "추천할 상품 후보가 없습니다.");
}
src/main/java/org/umc/valuedi/domain/savings/service/RecommendationService.java
Show resolved
Hide resolved
| options | ||
| ); | ||
| SavingsOption representativeOption = savings.getSavingsOptionList().stream() | ||
| .filter(o -> Integer.valueOf(12).equals(o.getSaveTrm())) |
There was a problem hiding this comment.
대표 옵션을 찾을 때 12라는 매직 넘버(magic number)가 사용되었습니다. 코드의 가독성과 유지보수성을 높이기 위해 클래스 레벨에 private static final Integer REPRESENTATIVE_TERM_MONTHS = 12; 와 같이 상수로 정의하여 사용하는 것을 권장합니다.
| .filter(o -> Integer.valueOf(12).equals(o.getSaveTrm())) | |
| .filter(o -> REPRESENTATIVE_TERM_MONTHS.equals(o.getSaveTrm())) |
| SAVINGS_NOT_FOUND(HttpStatus.NOT_FOUND, "SAVINGS404_1", "해당 적금 상품을 찾을 수 없습니다.") | ||
| SAVINGS_NOT_FOUND(HttpStatus.NOT_FOUND, "SAVINGS404_1", "해당 적금 상품을 찾을 수 없습니다."), | ||
| RECOMMENDATION_NOT_FOUND(HttpStatus.NOT_FOUND, "SAVINGS404_2", "아직 추천받은 내역이 없습니다. 먼저 상품 추천을 진행해 주세요."), | ||
| FILTERED_RECOMMENDATION_NOT_FOUND(HttpStatus.NOT_FOUND, "SAVINGS400_1", "해당 필터 조건에 맞는 추천 상품이 없습니다."), |
There was a problem hiding this comment.
FILTERED_RECOMMENDATION_NOT_FOUND 에러 코드의 HTTP 상태는 NOT_FOUND(404)이지만, 코드명은 SAVINGS400_1로 불일치합니다. API 응답의 일관성을 위해 HTTP 상태와 코드명을 맞추는 것이 좋습니다. 필터 조건에 맞는 결과가 없는 것은 클라이언트의 요청(필터)에 따른 결과로 볼 수 있으므로, HttpStatus.BAD_REQUEST(400)로 변경하는 것을 제안합니다.
| FILTERED_RECOMMENDATION_NOT_FOUND(HttpStatus.NOT_FOUND, "SAVINGS400_1", "해당 필터 조건에 맞는 추천 상품이 없습니다."), | |
| FILTERED_RECOMMENDATION_NOT_FOUND(HttpStatus.BAD_REQUEST, "SAVINGS400_1", "해당 필터 조건에 맞는 추천 상품이 없습니다."), |
1 participant
🔗 Related Issue
📝 Summary
🔄 Changes
💬 Questions & Review Points
📸 API Test Results (Swagger)
✅ Checklist