refactor : 게시글 리스트 조회시 발생하던 오류 제거하는 코드 리펙토링 #85
Conversation
WalkthroughThis PR tightens post list validation and refactors getPostList to normalize inputs and guard pagination, adds Spring Validation dependency, adjusts Firebase config to use a Resource and provide FirebaseMessaging bean, updates .gitignore to ignore all Changes
Sequence Diagram(s)sequenceDiagram
participant C as PostController
participant S as PostQueryServiceImpl
participant R as PostRepository
participant M as Converter
C->>S: getPostList(boardType, cursor, size)
S->>S: normalizeCursor(cursor), normalizeSize(size)
S->>R: findByBoardTypeBeforeCursor(boardType, normalizedCursor, Pageable(size))
R-->>S: Slice<Post>
S->>M: map slice.stream() -> PostPreviewDTOs
S->>S: compute nextCursor (hasNext && not empty ? lastId : null)
S-->>C: PostPreviewListDTO(previews, nextCursor)
C-->>C: wrap as CustomResponse OK
sequenceDiagram
participant Ctx as Spring Context
participant F as FireBaseConfig
participant GA as FirebaseApp
participant GM as FirebaseMessaging
Ctx->>F: request firebaseApp()
F->>F: serviceAccountResource.getInputStream()
F->>GA: initialize FirebaseApp if none exists
GA-->>F: FirebaseApp instance
Ctx->>F: request firebaseMessaging(FirebaseApp)
F->>GM: FirebaseMessaging.getInstance(FirebaseApp)
GM-->>Ctx: FirebaseMessaging bean
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
.gitignore (1)
39-39: Broadened .env ignore looks good; consider keeping an example trackedIgnoring all .env files reduces the risk of committing secrets. To keep an example file in VCS, consider explicitly unignoring a sample:
*.env +.env.* +!.env.examplesrc/main/java/naughty/tuzamate/global/config/FireBaseConfig.java (1)
45-49: Reasonable error handling; minor alternativeThrowing a RuntimeException is fine to fail fast. Alternatively, IllegalStateException better reflects initialization failure semantics, but this is optional.
src/main/java/naughty/tuzamate/domain/post/service/query/PostQueryServiceImpl.java (3)
46-51: nextCursor guard is correct; optional: derive from entities to decouple from DTO shapeThis works as-is. If you prefer to decouple nextCursor computation from DTO structure, compute it from the slice content:
- List<PostResDTO.PostPreviewDTO> previews = slice.stream() - .map(PostConverter::toPostPreviewDTO) - .toList(); - - Long nextCursor = (slice.hasNext() && !previews.isEmpty()) ? previews.get(previews.size() - 1).id() : null; + List<Post> content = slice.getContent(); + List<PostResDTO.PostPreviewDTO> previews = content.stream() + .map(PostConverter::toPostPreviewDTO) + .toList(); + + Long nextCursor = (slice.hasNext() && !content.isEmpty()) + ? content.get(content.size() - 1).getId() + : null;
63-66: Extract page size constants for clarity and reuseAvoid magic numbers; define defaults/max once and reuse:
- private int normalizeSize(int size) { - return (size < 1 || size > 10) ? 10 : size; - } + private int normalizeSize(int size) { + return (size < 1 || size > MAX_PAGE_SIZE) ? DEFAULT_PAGE_SIZE : size; + }Add these near the top of the class:
private static final int DEFAULT_PAGE_SIZE = 10; private static final int MAX_PAGE_SIZE = 10;
37-57: Add tests for getPostList to prevent regressionsGiven this method previously caused a runtime error, adding unit/integration tests for the cursor semantics (null, 0, valid id) and size bounds would lock in the fix.
I can draft a test matrix (repository slice stub or @DataJpaTest) covering: {cursor: null|0|valid}, size: {-1|0|1|10|11}, and verify ordering, hasNext, nextCursor.
src/main/java/naughty/tuzamate/domain/post/controller/PostController.java (2)
52-52: Enforce @max(10) by enabling method parameter validation on this controllerConstraints on simple @RequestParam like @max are not applied unless method validation is enabled (e.g., class-level @validated or a global config). Add @validated on this controller to ensure the size constraint is actually enforced.
You can apply this outside the changed line:
// import import org.springframework.validation.annotation.Validated; // annotate the controller @Validated @RestController @RequiredArgsConstructor @Tag(name = "게시판 컨트롤러", description = "게시판 관련 API") public class PostController { ... }
52-52: Add a lower bound (@min(1)) to align with service-side clamping [1..10]Currently only the upper bound is validated. Adding @min(1) prevents size=0 or negatives at the edge, matching the service normalization.
Apply this diff on the method signature:
- @RequestParam(defaultValue = "10") @Max(10) int size) { + @RequestParam(defaultValue = "10") @Min(1) @Max(10) int size) {Also add the import outside the changed line:
import jakarta.validation.constraints.Min;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.gitignore(1 hunks)build.gradle(1 hunks)src/main/java/naughty/tuzamate/domain/post/controller/PostController.java(1 hunks)src/main/java/naughty/tuzamate/domain/post/converter/PostConverter.java(1 hunks)src/main/java/naughty/tuzamate/domain/post/service/query/PostQueryServiceImpl.java(2 hunks)src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/naughty/tuzamate/domain/post/service/query/PostQueryServiceImpl.java (6)
src/main/java/naughty/tuzamate/domain/post/dto/PostResDTO.java (1)
PostResDTO(8-53)src/main/java/naughty/tuzamate/domain/post/service/query/PostQueryService.java (1)
getPostList(9-9)src/main/java/naughty/tuzamate/domain/comment/service/query/CommentQueryService.java (1)
getCommentList(7-7)src/main/java/naughty/tuzamate/domain/post/repository/PostRepository.java (2)
PostRepository(11-27)findPostListByUserIdLessThanOrderByIdDesc(25-26)src/main/java/naughty/tuzamate/domain/profile/service/query/ProfileQueryService.java (1)
getPostList(61-73)src/main/java/naughty/tuzamate/domain/comment/service/query/CommentQueryServiceImpl.java (1)
getCommentList(35-60)
src/main/java/naughty/tuzamate/domain/post/controller/PostController.java (2)
src/main/java/naughty/tuzamate/domain/post/service/query/PostQueryService.java (2)
getPostList(9-9)PostQueryService(6-10)src/main/java/naughty/tuzamate/domain/post/repository/PostRepository.java (2)
findByBoardTypeAndCursor(12-18)PostRepository(11-27)
src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java (3)
src/main/java/naughty/tuzamate/domain/comment/service/FCMService.java (3)
FCMService(10-42)send(26-33)sendNotification(17-24)src/main/java/naughty/tuzamate/domain/user/service/UserService.java (2)
updateFcmToken(12-12)UserService(6-13)src/main/java/naughty/tuzamate/domain/user/entity/User.java (1)
updateFcmToken(90-92)
⏰ 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: deploy
🔇 Additional comments (7)
src/main/java/naughty/tuzamate/domain/post/converter/PostConverter.java (1)
12-12: LGTM: harmless import additionThe added Optional import does not affect runtime. If it remains unused after this PR, consider removing in a follow-up cleanup.
build.gradle (1)
33-33: Validation starter addition aligns with controller constraintsAdding spring-boot-starter-validation is appropriate for @Max/@min and other Jakarta Bean Validation annotations used in controllers/services.
src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java (2)
23-26: Ensure property resolves to a Resource with the correct schemeWhen injecting a Resource from a property, the value should include a scheme like classpath: or file:. Please verify firebase.service-account.path is configured accordingly (for example, classpath:firebase/serviceAccount.json).
37-43: Idempotent initialization is correctThe guard against duplicate FirebaseApp initialization is good and prevents IllegalStateException on repeated initializations.
src/main/java/naughty/tuzamate/domain/post/service/query/PostQueryServiceImpl.java (1)
40-45: Good normalization of cursor and size; likely addresses the runtime issueNormalizing null/zero cursor to a sentinel and clamping page size aligns with repository semantics and avoids edge-case failures. The Pageable usage is clear.
src/main/java/naughty/tuzamate/domain/post/controller/PostController.java (2)
52-52: LGTM: tighter request cap matches service-level normalizationCapping to 10 at the edge aligns with the refactored getPostList normalization and prevents oversized page requests.
52-52: Mind the breaking change: reducing max size from 30 to 10 can reject existing clientsI ran the repo checks you provided — no matches found for size > 10, no other
@Max(30)occurrences, and no global validation advice classes in-repo. Absence of matches isn't proof; check OpenAPI/docs/clients and external consumers.
- Location to review:
- src/main/java/naughty/tuzamate/domain/post/controller/PostController.java (around line 52)
@RequestParam(defaultValue = "10") @Max(10) int size)Recommended next steps (pick one):
- If this change is intentional: update API docs, notify clients, and consider a version/migration note.
- If you need backward compatibility: soft-clamp server-side (accept >10 but cap to 10) or keep validation looser and enforce limits elsewhere.
Please verify generated OpenAPI/Swagger, documentation (README/docs/*), frontend/client repos, and integration tests for references to size > 10.
| try { | ||
| // 🚨 수정 포인트 2: 주입받은 Resource에서 바로 InputStream을 얻습니다. | ||
| InputStream serviceAccount = serviceAccountResource.getInputStream(); | ||
|
|
||
| FirebaseOptions options = FirebaseOptions.builder() | ||
| .setCredentials(GoogleCredentials.fromStream(serviceAccount)) | ||
| .build(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Close the service account InputStream via try-with-resources
The InputStream should be closed to avoid resource leaks. Wrap it in a try-with-resources.
- try {
- // 🚨 수정 포인트 2: 주입받은 Resource에서 바로 InputStream을 얻습니다.
- InputStream serviceAccount = serviceAccountResource.getInputStream();
-
- FirebaseOptions options = FirebaseOptions.builder()
- .setCredentials(GoogleCredentials.fromStream(serviceAccount))
- .build();
+ try {
+ // 🚨 수정 포인트 2: 주입받은 Resource에서 바로 InputStream을 얻습니다.
+ try (InputStream serviceAccount = serviceAccountResource.getInputStream()) {
+ FirebaseOptions options = FirebaseOptions.builder()
+ .setCredentials(GoogleCredentials.fromStream(serviceAccount))
+ .build();And close the additional brace at the end of the init block:
- if (FirebaseApp.getApps().isEmpty()) {
+ if (FirebaseApp.getApps().isEmpty()) {
log.info("Successfully initialized firebase app");
- return FirebaseApp.initializeApp(options);
- } else {
- return FirebaseApp.getInstance();
- }
+ return FirebaseApp.initializeApp(options);
+ } else {
+ log.info("Reusing existing firebase app instance");
+ return FirebaseApp.getInstance();
+ }
+ }📝 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.
| try { | |
| // 🚨 수정 포인트 2: 주입받은 Resource에서 바로 InputStream을 얻습니다. | |
| InputStream serviceAccount = serviceAccountResource.getInputStream(); | |
| FirebaseOptions options = FirebaseOptions.builder() | |
| .setCredentials(GoogleCredentials.fromStream(serviceAccount)) | |
| .build(); | |
| try { | |
| // 🚨 수정 포인트 2: 주입받은 Resource에서 바로 InputStream을 얻습니다. | |
| try (InputStream serviceAccount = serviceAccountResource.getInputStream()) { | |
| FirebaseOptions options = FirebaseOptions.builder() | |
| .setCredentials(GoogleCredentials.fromStream(serviceAccount)) | |
| .build(); | |
| if (FirebaseApp.getApps().isEmpty()) { | |
| log.info("Successfully initialized firebase app"); | |
| return FirebaseApp.initializeApp(options); | |
| } else { | |
| log.info("Reusing existing firebase app instance"); | |
| return FirebaseApp.getInstance(); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java around lines
29 to 35, the InputStream obtained from serviceAccountResource is not closed and
the init block is missing a closing brace; wrap obtaining and using
serviceAccountResource.getInputStream() in a try-with-resources to automatically
close the InputStream (e.g., try (InputStream serviceAccount =
serviceAccountResource.getInputStream()) { ... }) and ensure you add the missing
closing brace at the end of the init block so the method/block compiles
correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java (1)
27-50: Fix resource leak and avoid unnecessary InputStream when app already exists
serviceAccountResource.getInputStream()is opened unconditionally and never closed if the app is already initialized, causing a resource leak. Wrap the stream in try-with-resources and only open it when initialization is required. This also addresses the prior review’s suggestion.Apply this diff:
@Bean public FirebaseApp firebaseApp() { - try { - // 주입받은 Resource에서 바로 InputStream을 얻음 - InputStream serviceAccount = serviceAccountResource.getInputStream(); - - FirebaseOptions options = FirebaseOptions.builder() - .setCredentials(GoogleCredentials.fromStream(serviceAccount)) - .build(); - - // 앱이 이미 초기화되었는지 확인 (중복 초기화 방지) - if (FirebaseApp.getApps().isEmpty()) { - log.info("Successfully initialized firebase app"); - return FirebaseApp.initializeApp(options); - } else { - return FirebaseApp.getInstance(); - } - - } catch (IOException exception) { - log.error("Fail to initialize firebase app: {}", exception.getMessage(), exception); - // 초기화 실패 시 null 대신 예외를 던져서 애플리케이션이 문제를 인지하게 하는 것이 더 좋습니다. - throw new RuntimeException("Failed to initialize Firebase app.", exception); - } + // 이미 초기화되어 있다면 리소스를 열지 않고 즉시 반환 + if (!FirebaseApp.getApps().isEmpty()) { + log.info("Reusing existing firebase app instance"); + return FirebaseApp.getInstance(); + } + try (InputStream serviceAccount = serviceAccountResource.getInputStream()) { + FirebaseOptions options = FirebaseOptions.builder() + .setCredentials(GoogleCredentials.fromStream(serviceAccount)) + .build(); + FirebaseApp app = FirebaseApp.initializeApp(options); + log.info("Successfully initialized firebase app"); + return app; + } catch (IOException exception) { + log.error("Fail to initialize firebase app: {}", exception.getMessage(), exception); + // 초기화 실패 시 null 대신 예외를 던져서 애플리케이션이 문제를 인지하게 하는 것이 더 좋습니다. + throw new RuntimeException("Failed to initialize Firebase app.", exception); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java(1 hunks)
⏰ 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: deploy
🔇 Additional comments (1)
src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java (1)
52-55: FirebaseMessaging bean wiring looks goodUsing the provided FirebaseApp to obtain FirebaseMessaging is correct and keeps initialization centralized.
| // String 대신 Resource 타입으로 주입받음 | ||
| @Value("${firebase.service-account.path}") | ||
| private String SERVICE_ACCOUNT_PATH; | ||
| private Resource serviceAccountResource; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure property resolves to a valid Resource (prevent NPE at startup)
If ${firebase.service-account.path} is missing or invalid, serviceAccountResource may be null and cause a NullPointerException before the IOException catch. Add a null check to fail fast with a clear message.
Optionally add this at the top of firebaseApp():
+ Objects.requireNonNull(
+ serviceAccountResource,
+ "Property 'firebase.service-account.path' must resolve to a valid Resource (e.g., classpath:/..., file:/...)"
+ );And add the import:
import java.util.Objects;To verify configuration is present, run:
🏁 Script executed:
#!/bin/bash
# Verify the firebase service account property is defined in configuration files
rg -n -C2 --glob '!**/build/**' --glob '!**/out/**' 'firebase\.service-account\.path'Length of output: 557
🏁 Script executed:
#!/bin/bash
set -e
sed -n '1,240p' src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java || trueLength of output: 2044
Fail-fast if firebase.service-account.path doesn't resolve (prevent NPE at startup)
serviceAccountResource is injected and used immediately via getInputStream(); if the property is missing/invalid the field can be null and cause an NPE. Add a null (and optionally existence/readability) check before use.
- File: src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java — in firebaseApp(), before calling serviceAccountResource.getInputStream()
Suggested change (minimal):
+import java.util.Objects;
...
public FirebaseApp firebaseApp() {
+ Objects.requireNonNull(
+ serviceAccountResource,
+ "Property 'firebase.service-account.path' must resolve to a valid Resource (e.g., classpath:/..., file:/...)"
+ );
try {
// 주입받은 Resource에서 바로 InputStream을 얻음
InputStream serviceAccount = serviceAccountResource.getInputStream();Optionally also check serviceAccountResource.exists() / isReadable() and throw a clear IllegalStateException if false.
📝 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.
| // String 대신 Resource 타입으로 주입받음 | |
| @Value("${firebase.service-account.path}") | |
| private String SERVICE_ACCOUNT_PATH; | |
| private Resource serviceAccountResource; | |
| // (add near other imports) | |
| import java.util.Objects; | |
| // String 대신 Resource 타입으로 주입받음 | |
| @Value("${firebase.service-account.path}") | |
| private Resource serviceAccountResource; | |
| @Bean | |
| public FirebaseApp firebaseApp() { | |
| Objects.requireNonNull( | |
| serviceAccountResource, | |
| "Property 'firebase.service-account.path' must resolve to a valid Resource (e.g., classpath:/..., file:/...)" | |
| ); | |
| try { | |
| // 주입받은 Resource에서 바로 InputStream을 얻음 | |
| InputStream serviceAccount = serviceAccountResource.getInputStream(); | |
| FirebaseOptions options = FirebaseOptions.builder() | |
| .setCredentials(GoogleCredentials.fromStream(serviceAccount)) | |
| .build(); | |
| if (FirebaseApp.getApps().isEmpty()) { | |
| log.info("Successfully initialized firebase app"); | |
| return FirebaseApp.initializeApp(options); | |
| } else { | |
| return FirebaseApp.getInstance(); | |
| } | |
| } catch (IOException ex) { | |
| throw new RuntimeException(ex); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/naughty/tuzamate/global/config/FireBaseConfig.java around lines
23 to 26, the injected serviceAccountResource may be null or unreadable which
will cause a NullPointerException when calling
serviceAccountResource.getInputStream(); before invoking getInputStream() add a
fail-fast guard that checks serviceAccountResource != null and optionally
serviceAccountResource.exists() and serviceAccountResource.isReadable(), and if
any check fails throw a clear IllegalStateException (including the property key
and resolved value/path) so startup fails with a helpful message rather than an
NPE.
📍 PR 타입 (하나 이상 선택)
🏷️ 관련 이슈
Close #81
📌 개요
🔁 변경 사항
📸 스크린샷
✅ 체크리스트
💡 추가 사항 (리뷰어가 참고해야 할 것)
Summary by CodeRabbit
Bug Fixes
Refactor
Chores