Feat/reading status/1#130
Conversation
📝 WalkthroughWalkthroughThis PR adds AI-powered reading insights by introducing GPT integration services, multiple new business services, REST controllers, data repositories, and DTOs to generate monthly reading summaries and mood-based insights. It also refactors reading log validation logic to enforce absolute page tracking and updates CORS configuration. Changes
Sequence DiagramsequenceDiagram
participant User
participant Controller
participant Service
participant Repository
participant GPT["GPT Service"]
participant OpenAI["OpenAI API"]
User->>Controller: GET /reading-status<br/>(YearMonth)
activate Controller
Controller->>Service: getMonthlyReadingStatus<br/>(userId, month)
activate Service
Service->>Repository: findMonthlyStatusCounts<br/>(userId, start, end)
activate Repository
Repository-->>Service: MonthlyStatusCountProjection
deactivate Repository
Service->>Repository: findTopMoodTags<br/>(userId, start, end, 3)
activate Repository
Repository-->>Service: List<String> moodTags
deactivate Repository
Service->>GPT: generateCalendarMonthlySummary<br/>(month, progress, day, tags)
activate GPT
GPT->>OpenAI: POST /v1/chat/completions<br/>(system+user prompt)
activate OpenAI
OpenAI-->>GPT: completion response
deactivate OpenAI
GPT->>GPT: extractContent & postProcess
GPT-->>Service: aiSummary (truncated)
deactivate GPT
Service-->>Controller: ReadingStatusResponse
deactivate Service
Controller-->>User: ApiResponse<ReadingStatusResponse>
deactivate Controller
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
booklog/src/main/java/com/example/booklog/domain/library/shelves/controller/ReadingLogsController.java (1)
30-35:⚠️ Potential issue | 🟡 MinorStale Swagger description still references
pagesRead.Line 33 says
"입력: readDate(날짜), pagesRead(그날 읽은 페이지 수)"but the field is nowcurrentPagewith different semantics (absolute page number, not pages read that day). Line 53's summary "57페이지 읽음" (read 57 pages) is also misleading — it should convey "currently on page 57."Proposed fix
description = """ 특정 저장 도서(userBookId)에 대해 독서 기록을 추가(append)합니다. - 인증: Access Token(Bearer) - - 입력: readDate(날짜), pagesRead(그날 읽은 페이지 수) + - 입력: readDate(날짜), currentPage(현재까지 읽은 페이지) - 처리: 누적 currentPage는 서버 계산, user_books 최신상태 함께 갱신 """examples = `@ExampleObject`( name = "기본 예시", - summary = "특정 날짜에 57페이지 읽음", + summary = "특정 날짜에 57페이지까지 읽음",
🤖 Fix all issues with AI agents
In `@booklog/src/main/java/com/example/booklog/domain/ai/service/GptService.java`:
- Around line 307-330: generateMonthlyReadingStatusSummary currently can return
null and lacks robust post-processing; update it to run the raw gptResponse
through the same normalization used by GptUxCopyService.postProcess (trim,
truncate to 120 chars, strip surrounding quotes, collapse/normalize newlines and
whitespace) and treat empty or null post-processed results as missing by
returning defaultMonthlySummary(progressPercent, topMoodTags); ensure
callGptApiForSimpleText’s return is checked and post-processed before any return
so the method never returns null.
In
`@booklog/src/main/java/com/example/booklog/domain/ai/service/GptUxCopyService.java`:
- Around line 68-98: The three duplicated methods (callChatCompletion,
extractContent, postProcess) found in GptUxCopyService,
GptKeyScreenInsightService and GptService.callGptApiForSimpleText should be
refactored into a single shared component (e.g., a new GptApiClient class or
utility). Move the HTTP request/response logic (headers, body construction for
model/messages, restTemplate.exchange against OPENAI_API_URL), the JSON parsing
done by extractContent, and common postProcess behavior into GptApiClient,
expose a simple method like callChatCompletion(systemMessage, userPrompt,
temperature, maxTokens) or callApiAndExtract(...), and have GptUxCopyService,
GptKeyScreenInsightService and GptService delegate to that client while keeping
only their prompt-building and fallback logic locally.
- Around line 87-92: GptConfig's RestTemplate bean is created without timeouts
which can block threads on slow OpenAI calls used by GptUxCopyService,
GptService, and GptKeyScreenInsightService; update the RestTemplate bean in
GptConfig to use a client factory (e.g., SimpleClientHttpRequestFactory or
HttpComponentsClientHttpRequestFactory) and set explicit connect and read
timeouts (for example connect ~5s, read ~30s) so that all uses of
restTemplate.exchange(...) will fail fast on network stalls.
In
`@booklog/src/main/java/com/example/booklog/domain/library/shelves/service/ReadingLogsService.java`:
- Around line 32-35: In ReadingLogsService, make the total page validation
consistent by changing the guard in create() (the block using Integer total =
ub.getPageCountSnapshot()) to reject zero pages as well — use total == null ||
total <= 0 and throw GeneralException(ErrorStatus.TOTAL_PAGE_REQUIRED); this
aligns create() with update() and recalcLogsAndUserBook() and prevents create()
from calling recalcLogsAndUserBook() with an invalid 0 page count.
In
`@booklog/src/main/java/com/example/booklog/domain/users/controller/MeReadingStatusController.java`:
- Around line 26-29: Two controllers (MeReadingStatusController.getReadingStatus
and MeCalendarReadingStatusController.getReadingStatus) both map GET
/api/v1/me/reading-status causing ambiguous handler mapping; rename one route to
be unique. Update the `@GetMapping` on either
MeReadingStatusController#getReadingStatus or
MeCalendarReadingStatusController#getReadingStatus to a distinct path
(suggestion: "/reading-status/calendar" or "/reading-status/ai-comment"), then
update any callers/tests/OpenAPI annotations that reference the old path so they
match the new mapping and run tests to verify startup and routing.
In
`@booklog/src/main/java/com/example/booklog/domain/users/service/ReadingStatusService.java`:
- Around line 15-53: The method getMonthlyReadingStatus currently runs under the
class-level `@Transactional`(readOnly = true), which keeps the DB connection open
during the external call to gptUxCopyService.generateCalendarMonthlySummary;
refactor by moving all DB interactions (calls to
readingStatusQueryRepository.findMonthlyStatusCounts and findTopMoodTags and any
computations that only depend on DB values like completed, reading,
progressPercent, currentDay, lastDay) into a new private `@Transactional`(readOnly
= true) method (e.g., fetchMonthlyStatusData or loadMonthlyReadingData) that
returns a plain DTO/holder with the values needed for the summary, then call
gptUxCopyService.generateCalendarMonthlySummary outside that transactional
method inside getMonthlyReadingStatus so the external HTTP call does not hold
the DB transaction/connection open. Ensure getMonthlyReadingStatus uses the DTO
from the transactional helper to build the ReadingStatusResponse.
In
`@booklog/src/main/java/com/example/booklog/global/common/apiPayload/code/status/ErrorStatus.java`:
- Line 72: The enum constant TOTAL_PAGE_REQUIRED in ErrorStatus currently uses
HttpStatus.CONFLICT (409) which is misleading for a missing field; update
TOTAL_PAGE_REQUIRED to use a more appropriate status such as
HttpStatus.BAD_REQUEST (400) or HttpStatus.UNPROCESSABLE_ENTITY (422) in the
ErrorStatus enum so the API communicates a client-side validation/missing-field
error (locate the TOTAL_PAGE_REQUIRED entry in ErrorStatus to make this change).
In
`@booklog/src/main/java/com/example/booklog/global/common/apiPayload/handler/GlobalExceptionHandler.java`:
- Around line 108-116: The handler handleTypeMismatch currently returns
ApiResponse<?> directly which causes Spring to send HTTP 200; change its
signature to return ResponseEntity<ApiResponse<?>> and wrap the failure payloads
in ResponseEntity with an appropriate HTTP status (e.g. HttpStatus.BAD_REQUEST)
so both branches return
ResponseEntity.status(...).body(ApiResponse.onFailure(...)) — preserve the
existing checks for "month" (ErrorStatus.UNSUPPORTED_CALENDAR_FORMAT) and the
default (GeneralErrorCode.BAD_REQUEST) but wrap each ApiResponse.onFailure(...)
in a ResponseEntity with the correct HTTP status.
🧹 Nitpick comments (15)
booklog/src/test/java/com/example/booklog/BooklogApplicationTests.java (1)
8-14: Avoid permanently disabling the Spring Boot context load test.The
contextLoads()test is a valuable smoke test that catches wiring and configuration errors early. Blanket-disabling it at the class level hides potential issues introduced by the new services (e.g., missing beans, misconfigured properties).If the test fails in CI because of environment-specific configuration (e.g., an OpenAI API key), consider one of:
@SpringBootTestwith@MockBeanfor the external-service client, so the context loads without real credentials.@EnabledIfEnvironmentVariable(JUnit 5) to conditionally skip only when the key is absent, instead of unconditionally disabling.- A dedicated test profile (
application-test.yml) that stubs or disables the OpenAI integration.Any of these keeps the smoke test active in environments where it can run.
booklog/src/main/java/com/example/booklog/domain/users/converter/StringToYearMonthConverter.java (2)
11-15: Consider handlingDateTimeParseExceptionfor a cleaner error response.
YearMonth.parse(source)will throw a rawDateTimeParseExceptionon malformed input (e.g.,"2026-1","abc"). Spring converts this into a 400 response, but the default error body may expose internal details like the Java exception class name and stack trace. Wrapping with a domain-appropriate message improves the API experience.♻️ Proposed fix
+import java.time.format.DateTimeParseException; + `@Component` public class StringToYearMonthConverter implements Converter<String, YearMonth> { `@Override` public YearMonth convert(String source) { - // "2026-01" 같은 형식만 허용 (YearMonth.parse) - return YearMonth.parse(source); + try { + return YearMonth.parse(source); + } catch (DateTimeParseException e) { + throw new IllegalArgumentException( + "Invalid YearMonth format: '" + source + "'. Expected format: yyyy-MM", e); + } } }
1-1: Package placement: this converter is not user-specific.The converter is used by multiple controllers across different domains (calendar, key screen, reading status). Placing it under
domain.users.convertercouples a cross-cutting infrastructure concern to a specific domain. A package likecommon.converterorconfig.converterwould be a better fit.booklog/src/main/java/com/example/booklog/global/common/apiPayload/handler/GlobalExceptionHandler.java (1)
111-111: Hardcoded parameter name"month"is fragile.If the
@RequestParamname changes in the controller, this check silently stops working and falls through to the genericBAD_REQUESTbranch. Consider either extracting this as a constant shared with the controller or matching on the required type (YearMonth.class) instead, which is less prone to drift.booklog/src/main/java/com/example/booklog/domain/library/shelves/service/ReadingLogsService.java (1)
42-58: ThepagesReadcomputed at Line 46 is immediately overwritten byrecalcLogsAndUserBook.The delta at Line 46 is saved into the entity, but Line 58's recalc recomputes every log's
pagesRead. Consider passing0(or documenting) to make it obvious the initial value is a placeholder.booklog/src/main/java/com/example/booklog/domain/library/shelves/controller/ReadingLogsController.java (1)
14-14: Unused wildcard importio.swagger.v3.oas.annotations.Parameter.*.This import doesn't appear to be used anywhere in the file. The
@RequestBodyannotation on Line 46 comes fromio.swagger.v3.oas.annotations.parameters.RequestBody, which is referenced by its fully-qualified name inline.booklog/src/main/java/com/example/booklog/domain/users/dto/CalendarReadingStatusResponse.java (1)
1-1: Remove stray// dtocomment.This comment above the
packagedeclaration serves no purpose and adds noise.Proposed fix
-// dto package com.example.booklog.domain.users.dto;booklog/src/main/java/com/example/booklog/domain/ai/service/GptKeyScreenInsightService.java (1)
16-18: Missing@Slf4j— failure logging inconsistency with sibling GPT services.
GptUxCopyServiceandGptServiceboth use@Slf4jand log warnings/errors on failure. This service silently swallows exceptions in the catch block (line 49) with no logging, making production debugging harder. Consider adding@Slf4jand alog.warn(...)in the catch block for consistency.Proposed fix
+@Slf4j `@Service` `@RequiredArgsConstructor` public class GptKeyScreenInsightService {And in the catch block (line 49):
} catch (Exception e) { - // 기본 문구 + log.warn("키스크린 인사이트 생성 실패: {}", e.getMessage()); String tag = (topMoodTags != null && !topMoodTags.isEmpty()) ? topMoodTags.get(0) : null;booklog/src/main/java/com/example/booklog/domain/ai/service/GptService.java (1)
327-329: Exception silently swallowed — no logging.The existing methods in this class (
callGptApi,parseBookKeywordsResponse, etc.) uselog.error(...)on failure. This catch block silently drops the exception, making production issues invisible.Proposed fix
} catch (Exception e) { + log.warn("월간 독서 현황 요약 생성 실패: {}", e.getMessage()); return defaultMonthlySummary(progressPercent, topMoodTags); }booklog/src/main/java/com/example/booklog/domain/users/service/CalendarReadingStatusService.java (2)
1-1: Remove stray comment.
// serviceon line 1 appears to be a leftover artifact.
18-31: Near-duplicate ofKeyScreenInsightFacadeService.This service has virtually identical logic to
KeyScreenInsightFacadeService: same date-range calculation, samefindTopMoodTagscall, and samegenerateTasteInsightinvocation. The only difference is the response DTO shape. Consider extracting the shared retrieval + GPT logic into a common helper or merging the two services to reduce duplication and avoid redundant GPT calls if both endpoints are hit for the same month.booklog/src/main/java/com/example/booklog/domain/users/service/KeyScreenInsightFacadeService.java (1)
22-40: Synchronous GPT call on a GET request path may cause latency spikes.
generateTasteInsightmakes a synchronous HTTP call to OpenAI. If the API is slow or times out, the user's request will hang. Consider adding a timeout on theRestTemplateused byGptKeyScreenInsightService, or caching the insight per user+month to avoid repeated calls. The fallback inGptKeyScreenInsightServiceonly catches exceptions—it won't help with a slow response.booklog/src/main/java/com/example/booklog/domain/users/service/ReadingStatusService.java (1)
55-58:LocalDate.now()makes this method difficult to unit test.Consider accepting a
LocalDate todayparameter (or injecting aClock) so tests can control the current date deterministically.booklog/src/main/java/com/example/booklog/domain/users/repository/ReadingStatusQueryRepository.java (1)
44-62:LIMIT :limit— verify DB dialect compatibility.
LIMIT :limitworks with MySQL/PostgreSQL but not with all databases (e.g., Oracle, SQL Server). If this project is committed to a single dialect, this is fine; otherwise consider using pagination or dialect-agnostic approaches.booklog/src/main/java/com/example/booklog/domain/users/controller/MeCalendarReadingStatusController.java (1)
1-1: Remove stray// controllercomment.This looks like a leftover note. The
@RestControllerannotation and the class name already make the role clear.Proposed fix
-// controller package com.example.booklog.domain.users.controller;
| public String generateMonthlyReadingStatusSummary(YearMonth month, int progressPercent, List<String> topMoodTags) { | ||
| //월 독서현황 회색 문구를 실제로 만들어서 문자열로 리턴하는 “메인 함수" | ||
| try { | ||
| String prompt = buildMonthlyReadingStatusPrompt(month, progressPercent, topMoodTags); | ||
|
|
||
| // system prompt는 추천전문가보다 "짧은 문구 작성자"가 맞음 | ||
| String gptResponse = callGptApiForSimpleText( | ||
| "너는 독서 기록 서비스의 요약 카피를 쓰는 UX 라이터야.\n" + | ||
| "사용자 독서 취향 데이터를 기반으로 관찰자 시점에서 따뜻하지만 과장 없이 2문장 이내의 독서 현황 요약 문장을 작성해줘.", | ||
| prompt | ||
| ); | ||
|
|
||
| String text = (gptResponse == null) ? null : gptResponse.trim(); | ||
|
|
||
| // 너무 길면 UI 터질 수 있으니 안전장치(원하면 길이 조정) | ||
| if (text != null && text.length() > 120) { | ||
| text = text.substring(0, 120).trim(); | ||
| } | ||
| return text; | ||
|
|
||
| } catch (Exception e) { | ||
| return defaultMonthlySummary(progressPercent, topMoodTags); | ||
| } | ||
| } |
There was a problem hiding this comment.
generateMonthlyReadingStatusSummary can return null and lacks post-processing.
Two issues:
-
Null return path: If
callGptApiForSimpleTextreturnsnull, the method returnsnull(line 319→325). The sibling methods inGptUxCopyServiceandGptKeyScreenInsightServicenever returnnull— they either return processed text or a deterministic fallback. AnullaiSummaryinReadingStatusResponsewill serialize as JSONnull, which the frontend may not handle. -
Missing post-processing: Unlike
GptUxCopyService.postProcess, this method only trims and truncates — it doesn't strip surrounding quotes or normalize newlines. GPT frequently wraps output in quotes, so this text may reach the UI with stray quotation marks.
Proposed fix
- String text = (gptResponse == null) ? null : gptResponse.trim();
-
- // 너무 길면 UI 터질 수 있으니 안전장치(원하면 길이 조정)
- if (text != null && text.length() > 120) {
- text = text.substring(0, 120).trim();
- }
- return text;
+ if (gptResponse == null || gptResponse.isBlank()) {
+ return defaultMonthlySummary(progressPercent, topMoodTags);
+ }
+ String text = gptResponse.trim()
+ .replaceAll("^[\"'""'']+", "")
+ .replaceAll("[\"'""'']+$", "")
+ .replaceAll("\\s*\\R\\s*", " ")
+ .replaceAll("\\s{2,}", " ")
+ .trim();
+ if (text.length() > 120) {
+ text = text.substring(0, 120).trim();
+ }
+ return text;📝 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 String generateMonthlyReadingStatusSummary(YearMonth month, int progressPercent, List<String> topMoodTags) { | |
| //월 독서현황 회색 문구를 실제로 만들어서 문자열로 리턴하는 “메인 함수" | |
| try { | |
| String prompt = buildMonthlyReadingStatusPrompt(month, progressPercent, topMoodTags); | |
| // system prompt는 추천전문가보다 "짧은 문구 작성자"가 맞음 | |
| String gptResponse = callGptApiForSimpleText( | |
| "너는 독서 기록 서비스의 요약 카피를 쓰는 UX 라이터야.\n" + | |
| "사용자 독서 취향 데이터를 기반으로 관찰자 시점에서 따뜻하지만 과장 없이 2문장 이내의 독서 현황 요약 문장을 작성해줘.", | |
| prompt | |
| ); | |
| String text = (gptResponse == null) ? null : gptResponse.trim(); | |
| // 너무 길면 UI 터질 수 있으니 안전장치(원하면 길이 조정) | |
| if (text != null && text.length() > 120) { | |
| text = text.substring(0, 120).trim(); | |
| } | |
| return text; | |
| } catch (Exception e) { | |
| return defaultMonthlySummary(progressPercent, topMoodTags); | |
| } | |
| } | |
| public String generateMonthlyReadingStatusSummary(YearMonth month, int progressPercent, List<String> topMoodTags) { | |
| //월 독서현황 회색 문구를 실제로 만들어서 문자열로 리턴하는 "메인 함수" | |
| try { | |
| String prompt = buildMonthlyReadingStatusPrompt(month, progressPercent, topMoodTags); | |
| // system prompt는 추천전문가보다 "짧은 문구 작성자"가 맞음 | |
| String gptResponse = callGptApiForSimpleText( | |
| "너는 독서 기록 서비스의 요약 카피를 쓰는 UX 라이터야.\n" + | |
| "사용자 독서 취향 데이터를 기반으로 관찰자 시점에서 따뜻하지만 과장 없이 2문장 이내의 독서 현황 요약 문장을 작성해줘.", | |
| prompt | |
| ); | |
| if (gptResponse == null || gptResponse.isBlank()) { | |
| return defaultMonthlySummary(progressPercent, topMoodTags); | |
| } | |
| String text = gptResponse.trim() | |
| .replaceAll("^[\"'""'']+", "") | |
| .replaceAll("[\"'""'']+$", "") | |
| .replaceAll("\\s*\\R\\s*", " ") | |
| .replaceAll("\\s{2,}", " ") | |
| .trim(); | |
| if (text.length() > 120) { | |
| text = text.substring(0, 120).trim(); | |
| } | |
| return text; | |
| } catch (Exception e) { | |
| return defaultMonthlySummary(progressPercent, topMoodTags); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@booklog/src/main/java/com/example/booklog/domain/ai/service/GptService.java`
around lines 307 - 330, generateMonthlyReadingStatusSummary currently can return
null and lacks robust post-processing; update it to run the raw gptResponse
through the same normalization used by GptUxCopyService.postProcess (trim,
truncate to 120 chars, strip surrounding quotes, collapse/normalize newlines and
whitespace) and treat empty or null post-processed results as missing by
returning defaultMonthlySummary(progressPercent, topMoodTags); ensure
callGptApiForSimpleText’s return is checked and post-processed before any return
so the method never returns null.
| private String callChatCompletion(String systemMessage, String userPrompt, double temperature, int maxTokens) { | ||
| String apiKey = gptConfig.getSecretKey(); | ||
| if (apiKey == null || apiKey.isEmpty() || apiKey.equals("dummy-key-for-development")) { | ||
| throw new RuntimeException("GPT API 키 미설정"); | ||
| } | ||
|
|
||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
| headers.setBearerAuth(apiKey); | ||
|
|
||
| Map<String, Object> body = new HashMap<>(); | ||
| body.put("model", gptConfig.getModel()); | ||
| body.put("messages", List.of( | ||
| Map.of("role", "system", "content", systemMessage), | ||
| Map.of("role", "user", "content", userPrompt) | ||
| )); | ||
| body.put("temperature", temperature); | ||
| body.put("max_tokens", maxTokens); | ||
|
|
||
| ResponseEntity<String> response = restTemplate.exchange( | ||
| OPENAI_API_URL, | ||
| HttpMethod.POST, | ||
| new HttpEntity<>(body, headers), | ||
| String.class | ||
| ); | ||
|
|
||
| if (response.getStatusCode() == HttpStatus.OK && response.getBody() != null) { | ||
| return extractContent(response.getBody()); | ||
| } | ||
| throw new RuntimeException("GPT API 호출 실패: " + response.getStatusCode()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication across GPT service classes.
callChatCompletion, extractContent, and postProcess are nearly identical in GptUxCopyService, GptKeyScreenInsightService, and GptService.callGptApiForSimpleText. This triplication makes maintenance error-prone — a bug fix or API change must be applied in three places.
Extract the shared HTTP call, response parsing, and post-processing logic into a single base class or a shared utility (e.g., GptApiClient), and let each service delegate to it with only the prompt-building and fallback logic remaining service-specific.
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/ai/service/GptUxCopyService.java`
around lines 68 - 98, The three duplicated methods (callChatCompletion,
extractContent, postProcess) found in GptUxCopyService,
GptKeyScreenInsightService and GptService.callGptApiForSimpleText should be
refactored into a single shared component (e.g., a new GptApiClient class or
utility). Move the HTTP request/response logic (headers, body construction for
model/messages, restTemplate.exchange against OPENAI_API_URL), the JSON parsing
done by extractContent, and common postProcess behavior into GptApiClient,
expose a simple method like callChatCompletion(systemMessage, userPrompt,
temperature, maxTokens) or callApiAndExtract(...), and have GptUxCopyService,
GptKeyScreenInsightService and GptService delegate to that client while keeping
only their prompt-building and fallback logic locally.
| ResponseEntity<String> response = restTemplate.exchange( | ||
| OPENAI_API_URL, | ||
| HttpMethod.POST, | ||
| new HttpEntity<>(body, headers), | ||
| String.class | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's understand the directory structure
find . -type f -name "*.java" | grep -E "(config|Config|rest)" | head -20Repository: Project-BookLog/BookLog-BackEnd
Length of output: 1031
🏁 Script executed:
# Search for RestTemplate usage and configuration
rg -n 'RestTemplate' --type=java -g '!**/test/**' | head -50Repository: Project-BookLog/BookLog-BackEnd
Length of output: 1231
🏁 Script executed:
# Look at the specific file mentioned in the review
fd 'GptUxCopyService.java' --type fRepository: Project-BookLog/BookLog-BackEnd
Length of output: 156
🏁 Script executed:
# Read the GptConfig.java file
cat -n booklog/src/main/java/com/example/booklog/global/config/GptConfig.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 954
🏁 Script executed:
# Read the GptUxCopyService.java file, focusing on the RestTemplate injection and usage
cat -n booklog/src/main/java/com/example/booklog/domain/ai/service/GptUxCopyService.javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 5971
Configure timeouts on the RestTemplate bean in GptConfig.
The RestTemplate bean (line 19 in GptConfig.java) is created without timeout configuration. This RestTemplate is used across multiple services (GptUxCopyService, GptService, GptKeyScreenInsightService) for OpenAI API calls. Without connect/read timeouts, a slow or unresponsive OpenAI endpoint can block the calling thread indefinitely, causing thread pool exhaustion and service degradation.
Update the bean to configure explicit timeouts:
Suggested fix
`@Bean`
public RestTemplate restTemplate() {
SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
factory.setConnectTimeout(Duration.ofSeconds(5));
factory.setReadTimeout(Duration.ofSeconds(30));
return new RestTemplate(factory);
}🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/ai/service/GptUxCopyService.java`
around lines 87 - 92, GptConfig's RestTemplate bean is created without timeouts
which can block threads on slow OpenAI calls used by GptUxCopyService,
GptService, and GptKeyScreenInsightService; update the RestTemplate bean in
GptConfig to use a client factory (e.g., SimpleClientHttpRequestFactory or
HttpComponentsClientHttpRequestFactory) and set explicit connect and read
timeouts (for example connect ~5s, read ~30s) so that all uses of
restTemplate.exchange(...) will fail fast on network stalls.
| Integer total = ub.getPageCountSnapshot(); | ||
| if (total == null || total < 0) { | ||
| throw new GeneralException(ErrorStatus.TOTAL_PAGE_REQUIRED); // 409 | ||
| } |
There was a problem hiding this comment.
Inconsistent total validation: create() uses < 0 but update() and recalcLogsAndUserBook() use <= 0.
Line 33 checks total < 0 (allowing total == 0), while Line 79 and Line 120 check total <= 0 (rejecting total == 0). Since create() calls recalcLogsAndUserBook() at Line 58, a book with pageCountSnapshot == 0 would pass the create-check but immediately fail in recalc. Pick one consistent guard — likely <= 0 everywhere, since a 0-page book doesn't make sense.
Proposed fix
Integer total = ub.getPageCountSnapshot();
- if (total == null || total < 0) {
+ if (total == null || total <= 0) {
throw new GeneralException(ErrorStatus.TOTAL_PAGE_REQUIRED); // 409
}📝 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.
| Integer total = ub.getPageCountSnapshot(); | |
| if (total == null || total < 0) { | |
| throw new GeneralException(ErrorStatus.TOTAL_PAGE_REQUIRED); // 409 | |
| } | |
| Integer total = ub.getPageCountSnapshot(); | |
| if (total == null || total <= 0) { | |
| throw new GeneralException(ErrorStatus.TOTAL_PAGE_REQUIRED); // 409 | |
| } |
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/library/shelves/service/ReadingLogsService.java`
around lines 32 - 35, In ReadingLogsService, make the total page validation
consistent by changing the guard in create() (the block using Integer total =
ub.getPageCountSnapshot()) to reject zero pages as well — use total == null ||
total <= 0 and throw GeneralException(ErrorStatus.TOTAL_PAGE_REQUIRED); this
aligns create() with update() and recalcLogsAndUserBook() and prevents create()
from calling recalcLogsAndUserBook() with an invalid 0 page count.
| @GetMapping("/reading-status") | ||
| public ApiResponse<ReadingStatusResponse> getReadingStatus( | ||
| @AuthenticationPrincipal CustomUserDetails userDetails, | ||
| @RequestParam @DateTimeFormat(pattern = "yyyy-MM") YearMonth month |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for reading-status mappings with context
rg -n 'reading-status' --type=java -B3 -A3Repository: Project-BookLog/BookLog-BackEnd
Length of output: 2324
🏁 Script executed:
#!/bin/bash
# Find all Java controllers with `@RequestMapping` or `@RestController`
fd -e java . | xargs rg -l 'Controller' | head -20Repository: Project-BookLog/BookLog-BackEnd
Length of output: 1958
🏁 Script executed:
#!/bin/bash
# Look specifically for both controllers mentioned
rg -n 'class.*ReadingStatusController|class.*MeCalendarReadingStatusController' --type=java -A10Repository: Project-BookLog/BookLog-BackEnd
Length of output: 3344
🏁 Script executed:
#!/bin/bash
# Check `@RequestMapping` for both controllers
rg -B5 'class MeReadingStatusController' --type=java
rg -B5 'class MeCalendarReadingStatusController' --type=javaRepository: Project-BookLog/BookLog-BackEnd
Length of output: 1670
Route conflict: GET /api/v1/me/reading-status is mapped by two controllers.
Both MeReadingStatusController and MeCalendarReadingStatusController use @RequestMapping("/api/v1/me") with @GetMapping("/reading-status"), resulting in the same endpoint. Spring will fail at startup with an ambiguous handler mapping exception.
These serve distinct purposes:
MeReadingStatusController: Monthly reading status (for calendar/status card)MeCalendarReadingStatusController: Calendar page AI comment (monthly)
Rename one endpoint to be distinct (e.g., /reading-status/calendar or /ai-comment).
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/users/controller/MeReadingStatusController.java`
around lines 26 - 29, Two controllers
(MeReadingStatusController.getReadingStatus and
MeCalendarReadingStatusController.getReadingStatus) both map GET
/api/v1/me/reading-status causing ambiguous handler mapping; rename one route to
be unique. Update the `@GetMapping` on either
MeReadingStatusController#getReadingStatus or
MeCalendarReadingStatusController#getReadingStatus to a distinct path
(suggestion: "/reading-status/calendar" or "/reading-status/ai-comment"), then
update any callers/tests/OpenAPI annotations that reference the old path so they
match the new mapping and run tests to verify startup and routing.
| @Service | ||
| @RequiredArgsConstructor | ||
| @Transactional(readOnly = true) | ||
| public class ReadingStatusService { | ||
|
|
||
| private final ReadingStatusQueryRepository readingStatusQueryRepository; | ||
| private final GptUxCopyService gptUxCopyService; | ||
|
|
||
| public ReadingStatusResponse getMonthlyReadingStatus(Long userId, YearMonth month) { | ||
|
|
||
| LocalDate start = month.atDay(1); | ||
| LocalDate end = month.plusMonths(1).atDay(1); | ||
|
|
||
| MonthlyStatusCountProjection p = | ||
| readingStatusQueryRepository.findMonthlyStatusCounts(userId, start, end); | ||
|
|
||
| long completed = safeLong(p == null ? null : p.getCompletedCnt()); | ||
| long reading = safeLong(p == null ? null : p.getReadingCnt()); | ||
|
|
||
| int progressPercent = calcPercentFloor(completed, reading); | ||
|
|
||
| List<String> topMoodTags = | ||
| readingStatusQueryRepository.findTopMoodTags(userId, start, end, 3); | ||
|
|
||
| int lastDay = month.lengthOfMonth(); | ||
| int currentDay = resolveCurrentDay(month, lastDay); | ||
|
|
||
| String aiSummary = gptUxCopyService.generateCalendarMonthlySummary( | ||
| month, progressPercent, currentDay, lastDay, topMoodTags | ||
| ); | ||
|
|
||
| return new ReadingStatusResponse( | ||
| month.toString(), | ||
| progressPercent, | ||
| new ReadingStatusResponse.DayProgress(currentDay, lastDay), | ||
| topMoodTags, | ||
| aiSummary | ||
| ); | ||
| } |
There was a problem hiding this comment.
DB connection held open during external GPT call.
The class-level @Transactional(readOnly = true) means the database connection is held for the entire method execution, including the synchronous HTTP call to OpenAI (gptUxCopyService.generateCalendarMonthlySummary). Under load this can exhaust the connection pool while threads wait on the external API.
Consider splitting: run the DB queries in a transactional method, then call the GPT service outside the transaction boundary.
This same concern applies to CalendarReadingStatusService and KeyScreenInsightFacadeService.
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/domain/users/service/ReadingStatusService.java`
around lines 15 - 53, The method getMonthlyReadingStatus currently runs under
the class-level `@Transactional`(readOnly = true), which keeps the DB connection
open during the external call to
gptUxCopyService.generateCalendarMonthlySummary; refactor by moving all DB
interactions (calls to readingStatusQueryRepository.findMonthlyStatusCounts and
findTopMoodTags and any computations that only depend on DB values like
completed, reading, progressPercent, currentDay, lastDay) into a new private
`@Transactional`(readOnly = true) method (e.g., fetchMonthlyStatusData or
loadMonthlyReadingData) that returns a plain DTO/holder with the values needed
for the summary, then call gptUxCopyService.generateCalendarMonthlySummary
outside that transactional method inside getMonthlyReadingStatus so the external
HTTP call does not hold the DB transaction/connection open. Ensure
getMonthlyReadingStatus uses the DTO from the transactional helper to build the
ReadingStatusResponse.
| TOTAL_PAGE_INVALID(HttpStatus.BAD_REQUEST, "LB003", "총 페이지는 1 이상이어야 합니다."), | ||
| INVALID_TOTAL_PAGE(HttpStatus.BAD_REQUEST, "LB004", "총 페이지를 입력해야합니다"), | ||
| TOTAL_PAGE_INVALID(HttpStatus.BAD_REQUEST, "LB003", "총 페이지는 양수이어야 합니다."), | ||
| TOTAL_PAGE_REQUIRED(HttpStatus.CONFLICT, "LB004", "총 페이지 정보가 없어 독서 기록을 저장할 수 없습니다."), |
There was a problem hiding this comment.
HttpStatus.CONFLICT (409) is semantically misleading for missing data.
409 Conflict signals that the request conflicts with the current state of the target resource (e.g., duplicate creation, concurrent edit). Here the error represents a missing prerequisite field — totalPage not being set. HttpStatus.BAD_REQUEST (400) or HttpStatus.UNPROCESSABLE_ENTITY (422) would better communicate the actual problem to API consumers.
Suggested fix
- TOTAL_PAGE_REQUIRED(HttpStatus.CONFLICT, "LB004", "총 페이지 정보가 없어 독서 기록을 저장할 수 없습니다."),
+ TOTAL_PAGE_REQUIRED(HttpStatus.BAD_REQUEST, "LB004", "총 페이지 정보가 없어 독서 기록을 저장할 수 없습니다."),📝 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.
| TOTAL_PAGE_REQUIRED(HttpStatus.CONFLICT, "LB004", "총 페이지 정보가 없어 독서 기록을 저장할 수 없습니다."), | |
| TOTAL_PAGE_REQUIRED(HttpStatus.BAD_REQUEST, "LB004", "총 페이지 정보가 없어 독서 기록을 저장할 수 없습니다."), |
🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/global/common/apiPayload/code/status/ErrorStatus.java`
at line 72, The enum constant TOTAL_PAGE_REQUIRED in ErrorStatus currently uses
HttpStatus.CONFLICT (409) which is misleading for a missing field; update
TOTAL_PAGE_REQUIRED to use a more appropriate status such as
HttpStatus.BAD_REQUEST (400) or HttpStatus.UNPROCESSABLE_ENTITY (422) in the
ErrorStatus enum so the API communicates a client-side validation/missing-field
error (locate the TOTAL_PAGE_REQUIRED entry in ErrorStatus to make this change).
| @ExceptionHandler(MethodArgumentTypeMismatchException.class) | ||
| public ApiResponse<?> handleTypeMismatch(MethodArgumentTypeMismatchException e) { | ||
| // @RequestParam YearMonth month 변환 실패 케이스 | ||
| if ("month".equals(e.getName())) { | ||
| return ApiResponse.onFailure(ErrorStatus.UNSUPPORTED_CALENDAR_FORMAT); | ||
| } | ||
| // 나머지는 프로젝트 정책대로 (원하면 더 세분화 가능) | ||
| return ApiResponse.onFailure(GeneralErrorCode.BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
Bug: This handler returns HTTP 200 for error responses.
Every other handler in this class wraps the response in ResponseEntity with an explicit HTTP status code. This method returns ApiResponse<?> directly, so Spring will send HTTP 200 OK even though it's reporting UNSUPPORTED_CALENDAR_FORMAT or BAD_REQUEST. Clients relying on HTTP status codes will not detect the error.
🐛 Proposed fix: wrap in ResponseEntity like the other handlers
`@ExceptionHandler`(MethodArgumentTypeMismatchException.class)
- public ApiResponse<?> handleTypeMismatch(MethodArgumentTypeMismatchException e) {
+ public ResponseEntity<ApiResponse<Void>> handleTypeMismatch(MethodArgumentTypeMismatchException e) {
+ log.error("TypeMismatchException: {}", e.getMessage(), e);
// `@RequestParam` YearMonth month 변환 실패 케이스
if ("month".equals(e.getName())) {
- return ApiResponse.onFailure(ErrorStatus.UNSUPPORTED_CALENDAR_FORMAT);
+ return ResponseEntity.status(HttpStatus.BAD_REQUEST)
+ .body(ApiResponse.onFailure(ErrorStatus.UNSUPPORTED_CALENDAR_FORMAT));
}
- // 나머지는 프로젝트 정책대로 (원하면 더 세분화 가능)
- return ApiResponse.onFailure(GeneralErrorCode.BAD_REQUEST);
+ return ResponseEntity.status(HttpStatus.BAD_REQUEST)
+ .body(ApiResponse.onFailure(GeneralErrorCode.BAD_REQUEST));
}🤖 Prompt for AI Agents
In
`@booklog/src/main/java/com/example/booklog/global/common/apiPayload/handler/GlobalExceptionHandler.java`
around lines 108 - 116, The handler handleTypeMismatch currently returns
ApiResponse<?> directly which causes Spring to send HTTP 200; change its
signature to return ResponseEntity<ApiResponse<?>> and wrap the failure payloads
in ResponseEntity with an appropriate HTTP status (e.g. HttpStatus.BAD_REQUEST)
so both branches return
ResponseEntity.status(...).body(ApiResponse.onFailure(...)) — preserve the
existing checks for "month" (ErrorStatus.UNSUPPORTED_CALENDAR_FORMAT) and the
default (GeneralErrorCode.BAD_REQUEST) but wrap each ApiResponse.onFailure(...)
in a ResponseEntity with the correct HTTP status.
#129
#128
#127
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores