Conversation
WalkthroughThis update introduces a comprehensive reminder notification system. It adds Firebase Cloud Messaging (FCM) integration, Redis-based scheduling for notifications, new DTOs and services for push subscriptions, and modifies article creation and user signup flows to support notification tokens. Additional configuration and test updates support these features. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant AuthUsecase
participant PushSubscriptionSaveService
participant ArticleManagementUsecase
participant PushSubscriptionGetService
participant RedisNotificationService
participant Redis
participant NotificationExpirationListener
participant FcmService
participant Firebase
User->>API: SignUp (with token)
API->>AuthUsecase: signUp()
AuthUsecase->>PushSubscriptionSaveService: save(PushSubscription)
AuthUsecase-->>User: SignUpResponse
User->>API: Create Article (with reminder)
API->>ArticleManagementUsecase: createArticle()
ArticleManagementUsecase->>PushSubscriptionGetService: find(User)
ArticleManagementUsecase->>RedisNotificationService: scheduleArticleReminder(Article, User, token)
RedisNotificationService->>Redis: SET notification keys with TTL
Redis-->>NotificationExpirationListener: Key Expiration Event
NotificationExpirationListener->>FcmService: sendNotification(token, url)
FcmService->>Firebase: Send FCM
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 16
🔭 Outside diff range comments (1)
src/main/java/com/pinback/pinback_server/domain/notification/domain/entity/PushSubscription.java (1)
31-32: Remove unnecessary length constraint from ID columnThe
lengthattribute in@Columnannotation is meaningless for numeric types likeLong. It only applies to string-based columns.Apply this diff:
- @Column(name = "push_subscription_id", nullable = false, unique = true, length = 512) + @Column(name = "push_subscription_id", nullable = false, unique = true) private Long id;
🧹 Nitpick comments (12)
src/main/resources/application.yml (1)
32-32: Fix formatting issue: Add newline at end of file.The static analysis tool correctly identified that the file is missing a newline character at the end.
-fcm: ${FCM_JSON} +fcm: ${FCM_JSON} +src/test/resources/application-test.properties (2)
14-16: Use stronger JWT secret even for tests.The hardcoded JWT secret
"secret"is too weak and predictable, even for testing environments. Consider using a longer, more secure secret to ensure tests reflect realistic security conditions and prevent potential security oversights.-jwt.secret-key=secret +jwt.secret-key=test-secret-key-for-testing-purposes-only-do-not-use-in-production -jwt.issuer=secret +jwt.issuer=test-issuer
11-12: Consider the impact of show-sql in test environment.Enabling
show-sql=truewill create verbose SQL logging during tests. While useful for debugging, it might clutter test output. Consider whether this is necessary for all tests or if it should be conditionally enabled.src/test/java/com/pinback/pinback_server/domain/auth/application/AuthUsecaseTest.java (2)
39-39: Use more meaningful test data for the token parameter.The hardcoded
"token"string doesn't reflect realistic FCM token formats, which could miss edge cases in testing. Consider using a more realistic FCM token format or different token values for each test method.- SignUpCommand command = SignUpCommand.of("testEmail", LocalTime.of(10, 0, 0), "token"); + SignUpCommand command = SignUpCommand.of("testEmail", LocalTime.of(10, 0, 0), "fcm-test-token-" + System.currentTimeMillis());
55-55: Use different token values for distinct test scenarios.Both test methods use the same hardcoded token value. Using different tokens would better validate the system's ability to handle multiple unique subscriptions and improve test isolation.
- SignUpCommand command = SignUpCommand.of("testEmail", LocalTime.of(10, 0, 0), "token"); + SignUpCommand command = SignUpCommand.of("testEmail", LocalTime.of(10, 0, 0), "fcm-valid-token-test");src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/NotificationInfoCommand.java (1)
7-9: Consider more descriptive factory method name.The factory method name
fromis generic. Consider a more descriptive name likeoforcreateto match Java conventions, orfromEndpointAndKeyto be more explicit about the parameters.- public static NotificationInfoCommand from(String endpoint, KeyCommand command) { + public static NotificationInfoCommand of(String endpoint, KeyCommand key) { return new NotificationInfoCommand(endpoint, command); }src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionSaveService.java (2)
15-17: Consider returning the saved entity and adding validation.The service method doesn't return the saved entity, which callers might need (e.g., for the generated ID). Additionally, consider adding validation or business logic to justify the service layer, or evaluate if direct repository usage would be more appropriate.
- public void save(PushSubscription pushSubscription) { - repository.save(pushSubscription); + public PushSubscription save(PushSubscription pushSubscription) { + // Add validation if needed + if (pushSubscription == null) { + throw new IllegalArgumentException("PushSubscription cannot be null"); + } + return repository.save(pushSubscription); }
12-18: Evaluate the necessity of this service layer.This service currently provides no additional value over direct repository usage - it's a simple delegation. Consider whether business logic, validation, or transaction management will be added here, or if the service layer is needed at all for this simple operation.
src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/SignUpRequest.java (1)
21-22: Consider improving validation message clarity.The validation message "알림 정보는 비어있을 수 없습니다." (notification information cannot be empty) could be more specific about what type of token is expected for better API usability.
Consider updating the validation message to be more specific:
- @NotNull(message = "알림 정보는 비어있을 수 없습니다.") + @NotNull(message = "푸시 알림 토큰은 비어있을 수 없습니다.")Also consider adding a
@Schemaannotation to document the field:+ @Schema(description = "푸시 알림 토큰", example = "fcm_token_example") @NotNull(message = "푸시 알림 토큰은 비어있을 수 없습니다.") String tokensrc/main/java/com/pinback/pinback_server/infra/firebase/FcmService.java (1)
14-25: Enhance logging and consider internationalization.The service could benefit from improved logging and error message considerations.
Consider these improvements:
public void sendNotification(String token, String url) { + log.debug("Sending FCM notification to token: {} with URL: {}", token.substring(0, 10) + "...", url); try { Message.Builder messageBuilder = Message.builder() .setToken(token) .putData("url", url); FirebaseMessaging.getInstance().send(messageBuilder.build()); + log.info("FCM notification sent successfully"); } catch (FirebaseMessagingException e) { - throw new RuntimeException("익스텐션 알림 전송 실패", e); + log.error("Failed to send FCM notification: {}", e.getMessage(), e); + throw new RuntimeException("Failed to send extension notification", e); } }This adds debug logging for sends and uses English error messages for better international team collaboration.
src/main/java/com/pinback/pinback_server/domain/notification/domain/entity/PushSubscription.java (1)
38-39: Consider adding length constraint for token fieldThe length constraint was removed from the token field. Consider adding a reasonable limit to prevent storage of excessively long tokens.
If FCM tokens have a known maximum length, apply a constraint:
- @Column(name = "endpoint", nullable = false, unique = true) + @Column(name = "token", nullable = false, unique = true, length = 4096) private String token;src/main/java/com/pinback/pinback_server/global/config/redis/RedisConfig.java (1)
77-78: Consider limiting Redis keyspace event pattern to specific databaseThe current pattern
"__keyevent@*__:expired"listens to expiration events from all Redis databases. This might capture unintended events if multiple databases are used.If you're using a specific database (e.g., database 0), consider:
- new PatternTopic("__keyevent@*__:expired")); + new PatternTopic("__keyevent@0__:expired"));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
build.gradle(1 hunks)src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java(2 hunks)src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleSaveService.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/auth/application/AuthUsecase.java(2 hunks)src/main/java/com/pinback/pinback_server/domain/auth/application/command/SignUpCommand.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/KeyCommand.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/NotificationInfoCommand.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/SignUpRequest.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/domain/entity/PushSubscription.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/domain/repository/PushSubscriptionRepository.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionGetService.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionSaveService.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/KeyRequest.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/NotificationOnboardingRequest.java(1 hunks)src/main/java/com/pinback/pinback_server/global/common/util/TextUtil.java(0 hunks)src/main/java/com/pinback/pinback_server/global/config/firebase/FirebaseConfig.java(1 hunks)src/main/java/com/pinback/pinback_server/global/config/redis/RedisConfig.java(2 hunks)src/main/java/com/pinback/pinback_server/infra/firebase/FcmService.java(1 hunks)src/main/java/com/pinback/pinback_server/infra/redis/NotificationExpirationListener.java(1 hunks)src/main/java/com/pinback/pinback_server/infra/redis/dto/NotificationData.java(1 hunks)src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java(1 hunks)src/main/resources/application.yml(1 hunks)src/test/java/com/pinback/pinback_server/domain/auth/application/AuthUsecaseTest.java(2 hunks)src/test/resources/application-test.properties(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/pinback/pinback_server/global/common/util/TextUtil.java
🧰 Additional context used
🧠 Learnings (2)
src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleSaveService.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.748Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.748Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
🧬 Code Graph Analysis (2)
src/main/java/com/pinback/pinback_server/domain/auth/application/AuthUsecase.java (1)
src/main/java/com/pinback/pinback_server/domain/auth/exception/UserDuplicateException.java (1)
UserDuplicateException(6-10)
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java (3)
src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionGetService.java (1)
Service(11-19)src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleSaveService.java (1)
Service(11-20)src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java (1)
Service(17-88)
🪛 YAMLlint (1.37.1)
src/main/resources/application.yml
[error] 32-32: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (14)
src/main/resources/application.yml (1)
32-32: LGTM: Secure FCM configuration approach.Using environment variables for Firebase configuration is a security best practice that keeps sensitive credentials out of the codebase.
src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/KeyCommand.java (1)
3-7: LGTM: Correct WebPush subscription key structure.The record correctly represents the standard WebPush subscription key components (
p256dhandauth) as immutable data.src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleSaveService.java (1)
17-18: LGTM: Return saved entity enables notification workflow.Returning the saved
Articleis a good change that enables the notification scheduling functionality while following standard repository patterns.src/main/java/com/pinback/pinback_server/domain/auth/application/AuthUsecase.java (2)
27-27: Good practice: Adding transactional integrity.The
@Transactionalannotation ensures that both user creation and push subscription creation are handled atomically, which is important for data consistency.
35-40: Push subscription rollback is covered by the existing transactionThe
signUpmethod inAuthUsecaseis annotated with Spring’s@Transactional, andPushSubscriptionSaveService.save(...)simply delegates to the JPArepository.save(...)call without catching exceptions. Since Spring rolls back the transaction on any unchecked exception (and Spring Data repository methods throw runtime exceptions on failure), a failure inpushSubscriptionSaveService.save(...)will correctly trigger a rollback of the entiresignUptransaction (including user creation).No changes required.
src/main/java/com/pinback/pinback_server/domain/auth/application/command/SignUpCommand.java (1)
5-12: LGTM: Clean command object update.The addition of the
tokenfield and corresponding factory method update follows good practices and maintains the immutable nature of the command object.src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/NotificationOnboardingRequest.java (1)
6-9: KeyRequest record definition verifiedThe
KeyRequestrecord is properly defined and accessible in the codebase:
- src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/KeyRequest.java
No further action required.
src/main/java/com/pinback/pinback_server/infra/redis/dto/NotificationData.java (1)
10-21: LGTM! Well-structured DTO implementation.The DTO is properly designed with appropriate Lombok annotations and field types. The use of
LocalDateTimefor time fields and clear field naming follows good practices.src/main/java/com/pinback/pinback_server/infra/redis/NotificationExpirationListener.java (1)
19-21: Dual-Key TTL Strategy VerifiedThe
RedisNotificationServiceconfigures the data key to expire afterttlSeconds + 100_000, which is longer than the notification key’sttlSeconds, preventing race conditions.• In
src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java:
–objectRedisTemplate.opsForValue().set(notificationKey, …, Duration.ofSeconds(ttlSeconds));
–objectRedisTemplate.opsForValue().set(notificationDataKey, …, Duration.ofSeconds(ttlSeconds + 100_000)); // TODO: 명확한 TTL 정책 필요No further changes required for the dual-key expiration strategy.
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java (1)
65-65: Verified: ArticleSaveService.save now returns Article – no action neededThe
savemethod inArticleSaveServicehas been updated to return anArticle(see src/main/java/com/pinback/pinback_server/domain/article/domain/service/ArticleSaveService.java:17–19), and the usage inArticleManagementUsecasecorrectly captures this return value.src/main/java/com/pinback/pinback_server/domain/notification/domain/entity/PushSubscription.java (1)
41-46: Factory method implementation looks goodThe static factory method follows best practices and properly uses the builder pattern.
src/main/java/com/pinback/pinback_server/global/config/redis/RedisConfig.java (1)
45-67: ObjectMapper configuration for Redis template is appropriateThe configuration properly handles Java time objects and unknown properties, which is suitable for Redis serialization.
src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java (2)
70-79: NotificationData creation is well-structuredThe helper method properly encapsulates the DTO creation logic with all necessary fields.
54-56: Avoid blocking Redis with keys() in production
TheRedisTemplate#keys()call is a blocking operation and can lead to latency spikes on large datasets. Replace it with a non-blocking SCAN-based approach via aRedisCallback, for example:String pattern = NOTIFICATION_PREFIX + articleId + ":" + userId + ":*"; Set<String> keys = new HashSet<>(); objectRedisTemplate.execute((RedisCallback<Void>) connection -> { try (Cursor<byte[]> cursor = connection.scan( ScanOptions.scanOptions() .match(pattern) .count(100) .build())) { cursor.forEachRemaining(key -> keys.add(new String(key))); } return null; });• Ensure your Spring Data Redis version supports
RedisConnection.scan(...)(upgrade if necessary).
• Test in a staging environment to validate performance.
| //firebase | ||
| implementation 'com.google.firebase:firebase-admin:9.2.0' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify Firebase Admin SDK version and security status.
The Firebase Admin SDK version 9.2.0 should be checked to ensure it's current and free from known vulnerabilities.
🌐 Web query:
What is the latest version of Firebase Admin SDK for Java and are there any known security vulnerabilities in version 9.2.0?
💡 Result:
The latest version of the Firebase Admin SDK for Java is 9.5.0, released in May 2025[5][8].
Version 9.2.0 is not the most current version (it was released in June 2023), and there are known security vulnerabilities associated with it, specifically:
These vulnerabilities come from dependencies used by the SDK, not necessarily from the SDK's own codebase[2]. As such, using 9.2.0 exposes your application to publicly disclosed security issues.
To maintain the best security posture, it is strongly advised to upgrade to at least version 9.5.0[5][8].
Citations:
- 1: https://github.com/firebase/firebase-admin-java
- 2: https://mvnrepository.com/artifact/com.google.firebase/firebase-admin/9.2.0
- 3: https://firebase.press
- 4: https://security.snyk.io/package/npm/firebase/9.2.0-202192711727
- 5: https://firebase.google.com/support/releases
- 6: https://snyk.io/test/github/e2fyi/firebase-admin
- 7: https://central.sonatype.com/artifact/com.google.firebase/firebase-admin/9.0.0
- 8: https://mvnrepository.com/artifact/com.google.firebase/firebase-admin
- 9: https://firebase.google.com/docs/admin/setup
- 10: https://developers.google.com/workspace/admin/docs/release-notes
Upgrade Firebase Admin SDK to 9.5.0 to address security vulnerabilities
The currently pinned version 9.2.0 (lines 62–63 in build.gradle) is out of date and exposes your application to CVE-2025-24970 and CVE-2024-29025. The latest Firebase Admin SDK for Java is 9.5.0 (May 2025).
• File: build.gradle
Lines: 62–63
Current:
implementation 'com.google.firebase:firebase-admin:9.2.0'
Recommended:
implementation 'com.google.firebase:firebase-admin:9.5.0'
🤖 Prompt for AI Agents
In build.gradle at lines 62 to 63, update the Firebase Admin SDK dependency
version from 9.2.0 to 9.5.0 to fix known security vulnerabilities. Replace the
line "implementation 'com.google.firebase:firebase-admin:9.2.0'" with
"implementation 'com.google.firebase:firebase-admin:9.5.0'".
| public record KeyRequest( | ||
| String p256dh, | ||
| String auth | ||
| ) { | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate code duplication: Consolidate with KeyCommand.
This record is identical to KeyCommand in the auth domain, violating the DRY principle. Consider creating a shared data structure in a common package.
Create a shared record in a common package (e.g., com.pinback.pinback_server.global.dto) and remove the duplicate:
// In com.pinback.pinback_server.global.dto.WebPushKey
public record WebPushKey(
String p256dh,
String auth
) {
}Then update both domains to use the shared record instead of maintaining separate identical structures.
🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/KeyRequest.java
lines 3 to 7, the KeyRequest record duplicates the KeyCommand record from the
auth domain. To fix this, create a new shared record named WebPushKey in a
common package like com.pinback.pinback_server.global.dto with the same fields,
then replace both KeyRequest and KeyCommand usages with this shared WebPushKey
record to eliminate duplication and adhere to the DRY principle.
...pinback/pinback_server/domain/notification/domain/repository/PushSubscriptionRepository.java
Outdated
Show resolved
Hide resolved
| public record NotificationInfoCommand( | ||
| String endpoint, | ||
| KeyCommand key | ||
| ) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify consistency with simplified PushSubscription entity.
According to the AI summary, the PushSubscription entity was simplified to use only a token field, removing endpoint and cryptographic key fields. However, this command still maintains the endpoint and key structure. Please verify if this command is still needed in its current form or if it should be updated to align with the simplified entity model.
🏁 Script executed:
#!/bin/bash
# Check how NotificationInfoCommand is used and if it aligns with PushSubscription entity changes
ast-grep --pattern 'class PushSubscription {
$$$
}'
# Check usage of NotificationInfoCommand
rg -A 5 "NotificationInfoCommand"Length of output: 3100
🏁 Script executed:
#!/bin/bash
# Search for PushSubscription entity or record declaration across the repository
rg -n "PushSubscription"Length of output: 4412
🏁 Script executed:
#!/bin/bash
# Display PushSubscription entity fields and constructor/builder
rg -n "class PushSubscription" -A20 src/main/java/com/pinback/pinback_server/domain/notification/domain/entity/PushSubscription.javaLength of output: 753
Align DTO with simplified PushSubscription model
The PushSubscription entity now only stores a token (mapped from the endpoint column) and drops any cryptographic key fields. However, NotificationInfoCommand still carries a KeyCommand key that is never used when converting to the domain entity:
- NotificationInfoCommand.from(endpoint, key) → only passes
endpointintoPushSubscription.from(user, token) KeyCommandis parsed inNotificationOnboardingRequestbut then discarded
Action items:
- Remove the unused
KeyCommand keyfield and its factory parameter from:src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/NotificationInfoCommand.java
- Update
NotificationOnboardingRequest.toCommand()in:src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/NotificationOnboardingRequest.java
to only passendpoint(or rename totokenfor consistency)
- (Optional) If you intend to persist VAPID keys/auth secrets, extend the
PushSubscriptionentity to include those fields instead of dropping them
Example diff for NotificationInfoCommand.java:
-public record NotificationInfoCommand(
- String endpoint,
- KeyCommand key
-) {
- public static NotificationInfoCommand from(String endpoint, KeyCommand command) {
- return new NotificationInfoCommand(endpoint, command);
- }
-}
+public record NotificationInfoCommand(String endpoint) {
+ public static NotificationInfoCommand from(String endpoint) {
+ return new NotificationInfoCommand(endpoint);
+ }
+}And in NotificationOnboardingRequest.java:
-public NotificationInfoCommand toCommand() {
- return NotificationInfoCommand.from(
- endpoint,
- new KeyCommand(keyRequest().p256dh(), keyRequest().auth())
- );
-}
+public NotificationInfoCommand toCommand() {
+ return NotificationInfoCommand.from(endpoint);
+}📝 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 record NotificationInfoCommand( | |
| String endpoint, | |
| KeyCommand key | |
| ) { | |
| public record NotificationInfoCommand(String endpoint) { | |
| public static NotificationInfoCommand from(String endpoint) { | |
| return new NotificationInfoCommand(endpoint); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/domain/auth/presentation/dto/request/NotificationInfoCommand.java
lines 3 to 6, remove the unused KeyCommand key field and its constructor
parameter since the PushSubscription model no longer uses cryptographic keys.
Then, in
src/main/java/com/pinback/pinback_server/domain/notification/presentation/dto/request/NotificationOnboardingRequest.java,
update the toCommand() method to only pass the endpoint (or rename it to token)
without the key parameter to align with the simplified PushSubscription entity.
| public PushSubscription find(User user) { | ||
| return pushSubscriptionRepository.findPushSubscriptionByUser(user); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider handling null return values.
The find method delegates to the repository without null checking. Verify that the repository method findPushSubscriptionByUser either guarantees a non-null result or that callers are prepared to handle null returns.
🏁 Script executed:
#!/bin/bash
# Description: Check PushSubscriptionRepository method signature and return type
# Expected: Verify if method returns Optional<PushSubscription> or PushSubscription
ast-grep --pattern 'interface PushSubscriptionRepository {
$$$
findPushSubscriptionByUser($$$) {
$$$
}
$$$
}'
# Also check for any usage patterns
rg -A 3 "findPushSubscriptionByUser"Length of output: 843
Handle potential null returns from repository
The repository method findPushSubscriptionByUser(User user) returns a plain PushSubscription, which will be null if no record is found. Update the service to handle that case:
• File: src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionGetService.java
- Either change the return type to
Optional<PushSubscription>and wrap the call:- public PushSubscription find(User user) { - return pushSubscriptionRepository.findPushSubscriptionByUser(user); - } + public Optional<PushSubscription> find(User user) { + return Optional.ofNullable(pushSubscriptionRepository.findPushSubscriptionByUser(user)); + }
- Or throw a specific exception when the result is
null:- public PushSubscription find(User user) { - return pushSubscriptionRepository.findPushSubscriptionByUser(user); - } + public PushSubscription find(User user) { + return Optional.ofNullable(pushSubscriptionRepository.findPushSubscriptionByUser(user)) + .orElseThrow(() -> + new EntityNotFoundException("No PushSubscription found for user: " + user.getId())); + }
Ensure downstream callers are updated accordingly.
🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionGetService.java
around lines 16 to 18, the method find(User user) returns a PushSubscription
that can be null if no record is found. To fix this, either change the return
type to Optional<PushSubscription> and return
Optional.ofNullable(pushSubscriptionRepository.findPushSubscriptionByUser(user)),
or keep the return type and throw a specific exception if the repository returns
null. Also update any callers of this method to handle the new return type or
exception accordingly.
| @Column(name = "endpoint", nullable = false, unique = true) | ||
| private String token; |
There was a problem hiding this comment.
Fix column name to match field name
The column name is still "endpoint" while the field has been renamed to "token". This mismatch will cause issues with database persistence.
Apply this diff to fix the column name:
- @Column(name = "endpoint", nullable = false, unique = true)
+ @Column(name = "token", nullable = false, unique = true)
private String token;🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/domain/notification/domain/entity/PushSubscription.java
at lines 38-39, the @Column annotation specifies the column name as "endpoint"
while the field is named "token". To fix this mismatch, update the column name
in the @Column annotation to "token" so it matches the field name and ensures
correct database persistence.
| objectMapper.activateDefaultTyping( | ||
| objectMapper.getPolymorphicTypeValidator(), | ||
| ObjectMapper.DefaultTyping.NON_FINAL | ||
| ); |
There was a problem hiding this comment.
Security concern: Avoid using activateDefaultTyping
Using activateDefaultTyping with NON_FINAL can lead to deserialization vulnerabilities where malicious actors could execute arbitrary code by crafting specific JSON payloads.
Consider using explicit type information or @JsonTypeInfo annotations on your DTOs instead:
- objectMapper.activateDefaultTyping(
- objectMapper.getPolymorphicTypeValidator(),
- ObjectMapper.DefaultTyping.NON_FINAL
- );If polymorphic types are necessary, use a more restrictive approach with explicit type mappings.
📝 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.
| objectMapper.activateDefaultTyping( | |
| objectMapper.getPolymorphicTypeValidator(), | |
| ObjectMapper.DefaultTyping.NON_FINAL | |
| ); |
🤖 Prompt for AI Agents
In src/main/java/com/pinback/pinback_server/global/config/redis/RedisConfig.java
around lines 55 to 58, avoid using objectMapper.activateDefaultTyping with
NON_FINAL due to security risks of deserialization vulnerabilities. Instead,
remove this call and implement explicit type information by annotating your DTO
classes with @JsonTypeInfo or configure the ObjectMapper with a more restrictive
polymorphic type validator that only allows known safe types. This prevents
arbitrary code execution from crafted JSON payloads.
| objectRedisTemplate.opsForValue() | ||
| .set(notificationDataKey, notificationData, | ||
| Duration.ofSeconds(ttlSeconds + 100000)); //TODO: 명확한 TTL 정책 필요 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address TODO: Define clear TTL retention policy
The hardcoded 100,000 seconds (≈27.8 hours) for data retention needs a clear policy. Consider making this configurable.
Define a configuration property and inject it:
@Value("${notification.data.retention.seconds:172800}") // Default 48 hours
private long dataRetentionSeconds;Then use it:
- .set(notificationDataKey, notificationData,
- Duration.ofSeconds(ttlSeconds + 100000)); //TODO: 명확한 TTL 정책 필요
+ .set(notificationDataKey, notificationData,
+ Duration.ofSeconds(ttlSeconds + dataRetentionSeconds));🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java
around lines 38 to 40, replace the hardcoded additional TTL of 100000 seconds
with a configurable property. Define a new configuration property named
notification.data.retention.seconds with a default value (e.g., 172800 seconds
for 48 hours) using @Value annotation, inject it into the class, and use this
property to set the TTL duration instead of the fixed number. This will make the
TTL retention policy clear and adjustable.
| public void cancelArticleReminder(Long articleId, UUID userId) { | ||
| try { | ||
| String pattern = NOTIFICATION_PREFIX + articleId + ":" + userId + ":*"; | ||
| var keys = objectRedisTemplate.keys(pattern); | ||
|
|
||
| if (keys != null && !keys.isEmpty()) { | ||
| objectRedisTemplate.delete(keys); | ||
| log.info("Article 알림 취소 완료: articleId={}, userId={}, 삭제된 키 수={}", | ||
| articleId, userId, keys.size()); | ||
| } else { | ||
| log.debug("취소할 알림이 없습니다: articleId={}, userId={}", articleId, userId); | ||
| } | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("Article 알림 취소 실패: articleId={}, userId={}", articleId, userId, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider also deleting data keys when canceling reminders
The cancel method only deletes notification keys but not the corresponding data keys with notification:data: prefix.
Update the method to delete both key types:
public void cancelArticleReminder(Long articleId, UUID userId) {
try {
String pattern = NOTIFICATION_PREFIX + articleId + ":" + userId + ":*";
+ String dataPattern = NOTIFICATION_PREFIX_DATA + articleId + ":" + userId + ":*";
var keys = objectRedisTemplate.keys(pattern);
+ var dataKeys = objectRedisTemplate.keys(dataPattern);
- if (keys != null && !keys.isEmpty()) {
- objectRedisTemplate.delete(keys);
+ Set<String> allKeys = new HashSet<>();
+ if (keys != null) allKeys.addAll(keys);
+ if (dataKeys != null) allKeys.addAll(dataKeys);
+
+ if (!allKeys.isEmpty()) {
+ objectRedisTemplate.delete(allKeys);
log.info("Article 알림 취소 완료: articleId={}, userId={}, 삭제된 키 수={}",
- articleId, userId, keys.size());
+ articleId, userId, allKeys.size());
} else {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java
around lines 52 to 68, the cancelArticleReminder method deletes only
notification keys but not the related data keys with the "notification:data:"
prefix. Modify the method to also construct a pattern for data keys using the
same articleId and userId, retrieve those keys from Redis, and delete them
alongside the notification keys to ensure all related reminder data is removed.
| public void scheduleArticleReminder(Article article, User user, String token) { | ||
|
|
||
| try { | ||
| String notificationId = generateNotificationId(article.getId(), user.getId()); | ||
| String notificationKey = NOTIFICATION_PREFIX + notificationId; | ||
| String notificationDataKey = NOTIFICATION_PREFIX_DATA + notificationId; | ||
|
|
||
| NotificationData notificationData = createNotificationData(article, user, token, article.getRemindAt()); | ||
|
|
||
| long ttlSeconds = calculateTTLSeconds(article.getRemindAt()); | ||
|
|
||
| objectRedisTemplate.opsForValue().set(notificationKey, notificationData, Duration.ofSeconds(ttlSeconds)); | ||
| objectRedisTemplate.opsForValue() | ||
| .set(notificationDataKey, notificationData, | ||
| Duration.ofSeconds(ttlSeconds + 100000)); //TODO: 명확한 TTL 정책 필요 | ||
|
|
||
| log.info("Article 알림 예약 완료: articleId={}, userId={}, notificationId={}, remindTime={}, ttlSeconds={}", | ||
| article.getId(), user.getId(), notificationId, article.getRemindAt(), ttlSeconds); | ||
|
|
||
| } catch (Exception e) { | ||
| log.error("Article 알림 예약 실패: articleId={}, userId={}, remindTime={}", | ||
| article.getId(), user.getId(), article.getRemindAt(), e); | ||
|
|
||
| } | ||
| } |
There was a problem hiding this comment.
Validate reminder time before scheduling
The method doesn't validate if the reminder time is in the future. This could lead to immediate expiration or negative TTL values.
Add validation at the beginning of the method:
public void scheduleArticleReminder(Article article, User user, String token) {
+ if (article.getRemindAt() == null || article.getRemindAt().isBefore(LocalDateTime.now())) {
+ log.warn("Invalid reminder time for article: articleId={}, remindAt={}",
+ article.getId(), article.getRemindAt());
+ return;
+ }
try {📝 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 void scheduleArticleReminder(Article article, User user, String token) { | |
| try { | |
| String notificationId = generateNotificationId(article.getId(), user.getId()); | |
| String notificationKey = NOTIFICATION_PREFIX + notificationId; | |
| String notificationDataKey = NOTIFICATION_PREFIX_DATA + notificationId; | |
| NotificationData notificationData = createNotificationData(article, user, token, article.getRemindAt()); | |
| long ttlSeconds = calculateTTLSeconds(article.getRemindAt()); | |
| objectRedisTemplate.opsForValue().set(notificationKey, notificationData, Duration.ofSeconds(ttlSeconds)); | |
| objectRedisTemplate.opsForValue() | |
| .set(notificationDataKey, notificationData, | |
| Duration.ofSeconds(ttlSeconds + 100000)); //TODO: 명확한 TTL 정책 필요 | |
| log.info("Article 알림 예약 완료: articleId={}, userId={}, notificationId={}, remindTime={}, ttlSeconds={}", | |
| article.getId(), user.getId(), notificationId, article.getRemindAt(), ttlSeconds); | |
| } catch (Exception e) { | |
| log.error("Article 알림 예약 실패: articleId={}, userId={}, remindTime={}", | |
| article.getId(), user.getId(), article.getRemindAt(), e); | |
| } | |
| } | |
| public void scheduleArticleReminder(Article article, User user, String token) { | |
| if (article.getRemindAt() == null || article.getRemindAt().isBefore(LocalDateTime.now())) { | |
| log.warn("Invalid reminder time for article: articleId={}, remindAt={}", | |
| article.getId(), article.getRemindAt()); | |
| return; | |
| } | |
| try { | |
| String notificationId = generateNotificationId(article.getId(), user.getId()); | |
| String notificationKey = NOTIFICATION_PREFIX + notificationId; | |
| String notificationDataKey = NOTIFICATION_PREFIX_DATA + notificationId; | |
| NotificationData notificationData = createNotificationData(article, user, token, article.getRemindAt()); | |
| long ttlSeconds = calculateTTLSeconds(article.getRemindAt()); | |
| objectRedisTemplate.opsForValue().set(notificationKey, notificationData, Duration.ofSeconds(ttlSeconds)); | |
| objectRedisTemplate.opsForValue() | |
| .set(notificationDataKey, notificationData, | |
| Duration.ofSeconds(ttlSeconds + 100000)); //TODO: 명확한 TTL 정책 필요 | |
| log.info("Article 알림 예약 완료: articleId={}, userId={}, notificationId={}, remindTime={}, ttlSeconds={}", | |
| article.getId(), user.getId(), notificationId, article.getRemindAt(), ttlSeconds); | |
| } catch (Exception e) { | |
| log.error("Article 알림 예약 실패: articleId={}, userId={}, remindTime={}", | |
| article.getId(), user.getId(), article.getRemindAt(), e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/pinback/pinback_server/infra/redis/service/RedisNotificationService.java
around lines 26 to 50, the scheduleArticleReminder method lacks validation to
ensure the reminder time is in the future, which can cause negative or immediate
expiration TTL values. Add a check at the start of the method to verify that
article.getRemindAt() is after the current time, and if not, log a warning or
skip scheduling to prevent setting invalid TTLs.
ose0221
left a comment
ose0221
left a comment
There was a problem hiding this comment.
수고하셨습니다! 궁금한 점 댓글로 남겨두었습니다 !
| @Repository | ||
| public interface PushSubscriptionRepository extends JpaRepository<PushSubscription, Long> { | ||
|
|
||
| PushSubscription findPushSubscriptionByUser(User user); |
There was a problem hiding this comment.
여기서 PushSubscription가 null 이 될 가능성은 없나요?
There was a problem hiding this comment.
Optional로 변경했습니다~!!
| public PushSubscription find(User user) { | ||
| return pushSubscriptionRepository.findPushSubscriptionByUser(user); |
There was a problem hiding this comment.
위에서 이야기한 PushSubscription이 null 값을 반환한다면 여기서 예외처리 해주어야 할 것 같습니다 !
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/pinback/pinback_server/domain/notification/exception/SubscriptionNotFoundException.java (1)
6-10: Add JavaDoc documentation for better maintainability.Consider adding JavaDoc to explain when this exception should be thrown and its role in the notification system.
+/** + * Exception thrown when a push subscription is not found for a given user. + * This typically results in an HTTP 404 response when attempting to retrieve + * subscription data for notification scheduling. + */ public class SubscriptionNotFoundException extends ApplicationException { + /** + * Constructs a new SubscriptionNotFoundException with the predefined error code. + */ public SubscriptionNotFoundException() { super(ExceptionCode.SUBSCRIPTION_NOT_FOUND); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/pinback/pinback_server/domain/notification/domain/repository/PushSubscriptionRepository.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionGetService.java(1 hunks)src/main/java/com/pinback/pinback_server/domain/notification/exception/SubscriptionNotFoundException.java(1 hunks)src/main/java/com/pinback/pinback_server/global/exception/constant/ExceptionCode.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/pinback/pinback_server/global/exception/constant/ExceptionCode.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/pinback/pinback_server/domain/notification/domain/service/PushSubscriptionGetService.java
- src/main/java/com/pinback/pinback_server/domain/notification/domain/repository/PushSubscriptionRepository.java
🔇 Additional comments (1)
src/main/java/com/pinback/pinback_server/domain/notification/exception/SubscriptionNotFoundException.java (1)
1-10: Well-structured exception implementation.The exception class follows good Java practices by extending the application's base exception and using a specific error code. The implementation is clean and fits well within the notification domain architecture.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/resources/application.yml (2)
33-33: Add missing trailing newline to pass YAML lintingStatic analysis (
new-line-at-end-of-file) fails because the file ends immediately after thefcmproperty. Add a single newline to avoid tooling / CI noise.-fcm: ${FCM_JSON} +fcm: ${FCM_JSON} +
31-33: Reconsider placing barefcmat the root levelDropping the property at the top level deviates from the existing namespacing style (
spring.*,springdoc.*,jwt.*).
Two follow-ups worth weighing:
Namespace it to avoid future collisions:
notification:orfirebase:→credentials-json: ${FCM_JSON}Explicitly quote the value if it ever contains braces/colons (e.g. a JSON string or base64):
fcm: "${FCM_JSON}"Neither change is strictly required for runtime correctness today, but both harden the configuration against surprises.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java(3 hunks)src/main/resources/application.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java
🧰 Additional context used
🪛 YAMLlint (1.37.1)
src/main/resources/application.yml
[error] 33-33: no new line character at the end of file
(new-line-at-end-of-file)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java(1 hunks)src/test/resources/application-test.properties(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/resources/application-test.properties
🧰 Additional context used
🧠 Learnings (1)
src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java (1)
Learnt from: rootTiket
PR: Pinback-Team/pinback-server#18
File: src/main/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecase.java:38-41
Timestamp: 2025-07-07T15:33:02.748Z
Learning: In the Pinback server project, article detail retrieval (getArticleDetail method) does not require user authorization checks. Articles can be accessed by ID without ownership verification as per the project's design decision.
⏰ 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
| assertThat(article.getRemindAt()).isEqualTo(command.remindTime()); | ||
| assertThat(article.getIsRead()).isFalse(); | ||
| } | ||
| //TODO: 나중에 firebase mocking 처리해서 주석 해제 할 것 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add tracking issue for Firebase mocking implementation.
The TODO comment indicates this is a temporary solution, but there's no clear timeline or tracking mechanism for when this will be addressed.
Consider creating a GitHub issue to track the Firebase mocking implementation and link it in the TODO comment:
-//TODO: 나중에 firebase mocking 처리해서 주석 해제 할 것
+//TODO: Implement Firebase mocking and restore test coverage (Track: Issue #XX)This ensures the technical debt is properly tracked and prioritized.
📝 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.
| //TODO: 나중에 firebase mocking 처리해서 주석 해제 할 것 | |
| //TODO: Implement Firebase mocking and restore test coverage (Track: Issue #XX) |
🤖 Prompt for AI Agents
In
src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java
at line 43, the TODO comment about Firebase mocking lacks a tracking reference.
Create a GitHub issue to track the Firebase mocking implementation and update
the TODO comment to include a link or reference to this issue, ensuring the
technical debt is documented and prioritized.
| //TODO: 나중에 firebase mocking 처리해서 주석 해제 할 것 | ||
|
|
||
| // @DisplayName("사용자는 아티클을 생성할 수 있다.") | ||
| // @Test | ||
| // void articleSaveService() { | ||
| // User user = userRepository.save(user()); | ||
| // Category category = categoryRepository.save(category(user)); | ||
| // ArticleCreateCommand command = new ArticleCreateCommand("testUrl", category.getId() | ||
| // , "테스트메모", | ||
| // LocalDateTime.of(2025, 8, 6, 0, 0, 0)); | ||
| // //when | ||
| // articleManagementUsecase.createArticle(user, command); | ||
| // | ||
| // //then | ||
| // Article article = articleRepository.findById(1L).get(); | ||
| // assertThat(article.getUrl()).isEqualTo(command.url()); | ||
| // assertThat(article.getMemo()).isEqualTo(command.memo()); | ||
| // assertThat(article.getCategory()).isEqualTo(category); | ||
| // assertThat(article.getRemindAt()).isEqualTo(command.remindTime()); | ||
| // assertThat(article.getIsRead()).isFalse(); | ||
| // } |
There was a problem hiding this comment.
Critical: Restore test coverage with proper Firebase mocking.
Commenting out the core article creation test significantly reduces test coverage for a critical feature. This test was validating essential functionality including reminder time handling, which is central to the new notification feature.
The test should be restored immediately with proper Firebase mocking. Consider using:
@MockBeanfor Firebase services in the test context- Test containers or embedded Firebase emulator for integration testing
- Conditional mocking based on test profiles
+@MockBean
+private FirebaseMessaging firebaseMessaging;
+
+@MockBean
+private NotificationScheduleService notificationScheduleService;
-//TODO: 나중에 firebase mocking 처리해서 주석 해제 할 것
-
-// @DisplayName("사용자는 아티클을 생성할 수 있다.")
-// @Test
-// void articleSaveService() {
+@DisplayName("사용자는 아티클을 생성할 수 있다.")
+@Test
+void articleSaveService() {
+ // Given
+ when(notificationScheduleService.scheduleNotification(any(), any())).thenReturn(true);
+
User user = userRepository.save(user());
Category category = categoryRepository.save(category(user));
ArticleCreateCommand command = new ArticleCreateCommand("testUrl", category.getId()
, "테스트메모",
LocalDateTime.of(2025, 8, 6, 0, 0, 0));
-// //when
+ //when
articleManagementUsecase.createArticle(user, command);
-//
-// //then
+
+ //then
Article article = articleRepository.findById(1L).get();
assertThat(article.getUrl()).isEqualTo(command.url());
assertThat(article.getMemo()).isEqualTo(command.memo());
assertThat(article.getCategory()).isEqualTo(category);
assertThat(article.getRemindAt()).isEqualTo(command.remindTime());
assertThat(article.getIsRead()).isFalse();
-// }
+
+ // Verify notification scheduling was called
+ verify(notificationScheduleService).scheduleNotification(any(), any());
+}Would you like me to help create a comprehensive mocking strategy for the Firebase integration?
📝 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.
| //TODO: 나중에 firebase mocking 처리해서 주석 해제 할 것 | |
| // @DisplayName("사용자는 아티클을 생성할 수 있다.") | |
| // @Test | |
| // void articleSaveService() { | |
| // User user = userRepository.save(user()); | |
| // Category category = categoryRepository.save(category(user)); | |
| // ArticleCreateCommand command = new ArticleCreateCommand("testUrl", category.getId() | |
| // , "테스트메모", | |
| // LocalDateTime.of(2025, 8, 6, 0, 0, 0)); | |
| // //when | |
| // articleManagementUsecase.createArticle(user, command); | |
| // | |
| // //then | |
| // Article article = articleRepository.findById(1L).get(); | |
| // assertThat(article.getUrl()).isEqualTo(command.url()); | |
| // assertThat(article.getMemo()).isEqualTo(command.memo()); | |
| // assertThat(article.getCategory()).isEqualTo(category); | |
| // assertThat(article.getRemindAt()).isEqualTo(command.remindTime()); | |
| // assertThat(article.getIsRead()).isFalse(); | |
| // } | |
| // add mocks for Firebase integration | |
| @MockBean | |
| private FirebaseMessaging firebaseMessaging; | |
| @MockBean | |
| private NotificationScheduleService notificationScheduleService; | |
| @DisplayName("사용자는 아티클을 생성할 수 있다.") | |
| @Test | |
| void articleSaveService() { | |
| // Given | |
| when(notificationScheduleService.scheduleNotification(any(), any())).thenReturn(true); | |
| User user = userRepository.save(user()); | |
| Category category = categoryRepository.save(category(user)); | |
| ArticleCreateCommand command = new ArticleCreateCommand( | |
| "testUrl", | |
| category.getId(), | |
| "테스트메모", | |
| LocalDateTime.of(2025, 8, 6, 0, 0, 0) | |
| ); | |
| // when | |
| articleManagementUsecase.createArticle(user, command); | |
| // then | |
| Article article = articleRepository.findById(1L).get(); | |
| assertThat(article.getUrl()).isEqualTo(command.url()); | |
| assertThat(article.getMemo()).isEqualTo(command.memo()); | |
| assertThat(article.getCategory()).isEqualTo(category); | |
| assertThat(article.getRemindAt()).isEqualTo(command.remindTime()); | |
| assertThat(article.getIsRead()).isFalse(); | |
| // Verify notification scheduling was called | |
| verify(notificationScheduleService).scheduleNotification(any(), any()); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/pinback/pinback_server/domain/article/application/ArticleManagementUsecaseTest.java
between lines 43 and 63, the core article creation test is commented out due to
missing Firebase mocking, which reduces test coverage. To fix this, restore the
test by implementing proper Firebase mocking using @MockBean annotations for
Firebase services in the test context. Alternatively, set up a Firebase emulator
or test containers for integration tests. Ensure the Firebase dependencies are
mocked or emulated so the test can run successfully and validate the article
creation and reminder time handling.
2 participants
🚀 PR 요약
close #48 리마인드 알림기능을 구현합니다.
✨ PR 상세 내용
🚨 주의 사항
원래는
notification:키 만으로 구성하려 했으나 만료시 redis에서 사라져서 'notification:data:` 를 따로 저장하여 데이터를 가져오도록 구현했습니다. (이중키방식)✅ 체크 리스트
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores