[LNK-71] domain/notification 코틀린 마이그레이션#93
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough이 PR은 알림 시스템을 Java 기반 직접 서비스 호출에서 Kotlin 이벤트 기반 아키텍처로 마이그레이션합니다. 새로운 MongoDB 엔티티, 도메인 이벤트, 정책 기반 핸들링, 그리고 비동기 알림 처리를 도입합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant FeedUsecase
participant ApplicationEventPublisher
participant FeedNotificationEventListener
participant NotificationPolicy
participant NotificationPort
participant NotificationService
participant NotificationSaveService
participant NotificationPublisher
FeedUsecase->>ApplicationEventPublisher: publishEvent(FeedDomainEvent.created)
ApplicationEventPublisher->>FeedNotificationEventListener: on(FeedDomainEvent)
FeedNotificationEventListener->>NotificationPolicy: shouldNotify(userId, NEW_FEED)
NotificationPolicy-->>FeedNotificationEventListener: true/false
FeedNotificationEventListener->>NotificationPort: send(NotificationRequest)
NotificationPort->>NotificationService: send(request)
NotificationService->>NotificationSaveService: save(NotificationEntity)
NotificationSaveService-->>NotificationService: saved
NotificationService->>NotificationPublisher: publish(userId, notification)
NotificationPublisher-->>NotificationService: published
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/leets/leenk/domain/notification/domain/entity/Notification.java (1)
16-41:⚠️ Potential issue | 🟡 Minor코딩 가이드라인에 따른
deletedAt필드가 누락되었습니다.도메인 엔티티에 soft delete를 위한
deletedAt필드가 존재하지 않습니다. 이미 존재하는 PR이 아닌 경우, 추가를 검토해 주세요.As per coding guidelines,
**/domain/**/*.{kt,java}: Implement soft deletes viadeletedAtfield in entities.src/main/java/leets/leenk/domain/notification/presentation/NotificationController.java (1)
26-26:⚠️ Potential issue | 🟡 Minor
feedNotificationUsecase필드가 사용되지 않습니다.
markAsRead가notificationUseCase로 이관되면서, 이 컨트롤러 내에서feedNotificationUsecase를 참조하는 곳이 없습니다. 미사용 의존성과 import(Line 9)를 제거해 주세요.🧹 미사용 코드 제거 제안
-import leets.leenk.domain.notification.application.usecase.FeedNotificationUsecase;- private final FeedNotificationUsecase feedNotificationUsecase;src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java (1)
399-407:⚠️ Potential issue | 🟡 Minor
notifyIfReachedReactionMilestone메서드는 더 이상 사용되지 않으므로 제거해 주세요.리액션 플로우가 이벤트 기반으로 전환되면서 이 메서드(399-407줄)는 호출되지 않는 데드 코드가 되었습니다.
참고:
feedNotificationUsecase필드는NotificationController에서 별도로 사용되고 있으므로 제거하지 않아야 합니다.
🤖 Fix all issues with AI agents
In `@gradle.properties`:
- Line 5: Remove the hardcoded org.gradle.java.home entry from gradle.properties
(the line setting /Users/jj/...), rely on the existing java.toolchain
configuration in build.gradle.kts instead, and if per-developer overrides are
needed move the setting into a local, gitignored file such as
gradle-local.properties referenced in .gitignore; ensure no other committed
files contain developer-specific Java paths.
In `@src/main/kotlin/leets/leenk/domain/feed/domain/event/FeedDomainEvent.kt`:
- Around line 57-67: The current FeedDomainEvent exposes unsafe accessors (e.g.,
feedId, authorId, authorName, taggedUserIds, feedAuthorId, reactorId,
reactorName, previousReactionCount, totalReactionCount) that cast entries from
data and will throw NPEs when callers access fields not present for a given
event type; refactor FeedDomainEvent into a sealed class with explicit
subclasses (e.g., FeedDomainEvent.Created and FeedDomainEvent.Reacted) that
declare only the fields relevant to each event, move any factory/serialization
helpers (companion factory methods) to produce the correct subclass, and update
call sites to pattern-match (is checks or when) on the sealed types instead of
using the untyped accessors.
In
`@src/main/kotlin/leets/leenk/domain/notification/application/adapter/FeedNotificationAdapter.kt`:
- Around line 147-154: The duplicate regex parsing for milestones in
FeedNotificationAdapter (within the achievedMilestones.forEach block computing
alreadyExists) repeats the same (\d+)개 extraction already implemented in
NotificationEntityGetService.checkReactionCountDuplicated; instead, update the
notification detail structure to include a parsed "milestone" field when
notifications are created and then replace the regex logic in
FeedNotificationAdapter with a direct integer comparison against
detail["milestone"]; specifically, remove the body regex extraction inside the
alreadyExists check and compare milestone == (detail["milestone"] as? Int) (or
null-safe equivalent), and ensure the producer side that creates notification
details populates that milestone field so both FeedNotificationAdapter and
NotificationEntityGetService can use the structured value.
- Around line 19-25: The on(event: FeedDomainEvent) handler executes
after-commit and dispatches async work that writes to the DB; ensure those
writes run inside their own transaction by annotating the NotificationService
methods (send, sendBatch, sendOrUpdate) or the NotificationSaveService methods
with `@Transactional`(propagation = Propagation.REQUIRES_NEW) so each async
coroutine obtains a new transactional boundary; update the NotificationService
(or NotificationSaveService) method signatures to include the
`@Transactional`(requires_new) propagation and keep the async scope.launch calls
in FeedNotificationAdapter.on as-is.
In
`@src/main/kotlin/leets/leenk/domain/notification/domain/entity/enums/NotificationType.kt`:
- Around line 95-114: The formatContent function incorrectly replaces both
{name} and {count} with every param because forEachIndexed ignores index; change
the loop in formatContent to map params by index to their intended placeholders
(e.g., index 0 -> "{name}", index 1 -> "{count}" or use a placeholders list and
replace only the matching placeholder for each param) and avoid blanket
replacements that overwrite previous values; also stop swallowing the exception
in the catch block — either rethrow a more specific exception or log the caught
exception (include e.message/stack) before returning the fallback formatted
string so failures in String.format are observable (refer to formatContent,
content variable and the catch (e: Exception) block).
In
`@src/main/kotlin/leets/leenk/domain/notification/domain/entity/NotificationEntity.kt`:
- Around line 10-23: Add a nullable deletedAt field to the NotificationEntity
data class to implement soft deletes: declare a var deletedAt: LocalDateTime? =
null (use the same LocalDateTime import as createDate/updateDate) and ensure it
is initialized to null so existing records are treated as not-deleted; update
any equals/hashCode/constructors if needed and adjust repositories/queries to
respect deletedAt where soft-delete behavior is required.
In
`@src/main/kotlin/leets/leenk/domain/notification/domain/repository/NotificationEntityRepository.kt`:
- Around line 8-31: Add a nullable deletedAt field to NotificationEntity (or
ensure it inherits it once you switch to the Mongo BaseEntity as noted) and
update the repository queries to filter out soft-deleted records (deletedAt ==
null): modify the `@Query` in findNotificationByUserAndTypeAndTarget and
findByFeedIdAndUserIdInFirstReactions to include "'deletedAt': null" and change
findAllByUserId to only return entities where deletedAt is null (e.g., add a
method or query variant like findAllByUserIdAndDeletedAtIsNull or adjust
existing query), keeping the existing parameter names and repository method
signatures (findNotificationByUserAndTypeAndTarget, findAllByUserId,
findByFeedIdAndUserIdInFirstReactions) so behavior excludes soft-deleted
notifications.
In
`@src/main/kotlin/leets/leenk/domain/notification/domain/service/NotificationEntityGetService.kt`:
- Around line 55-64: The duplicate milestone check in
NotificationEntityGetService currently parses the milestone from detail["body"]
using a regex, which is fragile and duplicated in FeedNotificationAdapter;
update NotificationEntityGetService to first look for a structured milestone
field (e.g., detail["milestone"] as? Int or as? String then toIntOrNull) and
compare that to the incoming milestone, only falling back to parsing the body if
the structured field is absent. Extract the body-parsing regex logic into a
shared utility function (e.g.,
NotificationUtils.extractMilestoneFromDetail(detail: Map<String, Any>): Int?)
and use that utility from both NotificationEntityGetService and
FeedNotificationAdapter to remove duplication and hard-coded regex usage. Ensure
the details extraction still filters to Map<String, Any> as before and that the
comparison uses the parsed/structured Int? value.
In
`@src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationPublisher.kt`:
- Around line 17-37: In publishIfEligible, wrap the call to
userGetService.findById(userId) in a try-catch (or null-safe handling if the
service can return null) so exceptions thrown during user lookup do not bubble
out of the event listener; on failure catch the exception, log a warning/error
with context (include userId and exception) and return early before constructing
the SqsMessageEvent and calling eventPublisher.publishEvent; ensure the rest of
the method (fcm token check, SqsMessageEvent creation,
notification.notificationType.path and user.id usage) only runs when a valid
user is obtained.
- Around line 39-45: Replace the reflection-based FCM token lookup in
User.getFcmTokenReflection with a direct, type-safe access to the token (call
the User.getFcmToken() or expose a fcmToken property) or introduce an
interface/DTO that exposes getFcmToken for cross-module boundaries; remove use
of this.javaClass.getMethod and method.invoke, and if you cannot change User,
add a safe adapter that calls the real accessor. Also stop swallowing all
exceptions: catch only expected exceptions and log failures (use the existing
logger in NotificationPublisher) including the exception message and context so
token lookup failures are visible.
In
`@src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationService.kt`:
- Around line 74-76: Replace e.printStackTrace() in the catch blocks inside the
NotificationService class with structured SLF4J logging: add a private logger
(LoggerFactory.getLogger(NotificationService::class.java)) to the class and
change the catch handlers to call logger.error("Descriptive message about the
failure in [operation/name of method]", e) (or logger.warn where appropriate)
for the catch blocks shown (including the catch at lines ~74–76 and the one at
~88–90) so exceptions are logged with message and stacktrace via SLF4J instead
of printing to stdout.
🧹 Nitpick comments (9)
src/main/java/leets/leenk/domain/notification/domain/entity/Notification.java (1)
37-39: Lombok@Getter와 중복되는 명시적 getter 메서드입니다.클래스에 이미
@Getter어노테이션(Line 13)이 적용되어 있어getContent()메서드가 자동 생성됩니다. 명시적으로 동일한 getter를 추가할 필요가 없으며, 불필요한 보일러플레이트 코드가 됩니다.♻️ 중복 getter 제거 제안
- public NotificationContent getContent() { - return this.content; - } -src/main/kotlin/leets/leenk/domain/notification/application/port/NotificationPort.kt (1)
5-21: 코루틴 기반 구현에서 인터페이스 메서드 설계 검토 권장
NotificationService구현체에서CoroutineScope와launch,async를 사용한 코루틴 기반 구현을 확인했습니다. 다만 현재 인터페이스 메서드(send,sendBatch,sendOrUpdate)가 일반 함수로 정의되어 있으면서 구현체에서scope.launch로 감싸는 패턴을 사용 중입니다.내부적으로
suspend fun을 활용하고 있으므로, 모든 호출부가 코루틴 환경을 지원한다면 포트 인터페이스 메서드도suspend fun으로 선언하는 것이 더 명확한 의도 표현과 structured concurrency 원칙에 부합합니다. 다만 non-coroutine 호출부를 지원해야 한다면 현재 설계가 적절합니다.src/main/kotlin/leets/leenk/domain/notification/domain/entity/NotificationEntity.kt (1)
24-32:markRead()/markUnread()는 in-place 변경이지만updateContent()는 새 인스턴스를 반환합니다 — 일관성 부족.
data class에서var필드를 직접 변경하는 패턴과 새 인스턴스를 반환하는 패턴이 혼재되어 있습니다. 하나의 방식으로 통일하는 것을 권장합니다. 특히data class는copy()를 활용한 불변 패턴이 더 자연스럽습니다.불변 패턴으로 통일하는 제안
- fun markRead() { - this.isRead = true - this.updateDate = LocalDateTime.now() - } - - fun markUnread() { - this.isRead = false - this.updateDate = LocalDateTime.now() - } + fun markRead(): NotificationEntity = + copy(isRead = true, updateDate = LocalDateTime.now()) + + fun markUnread(): NotificationEntity = + copy(isRead = false, updateDate = LocalDateTime.now())src/main/java/leets/leenk/domain/feed/application/usecase/FeedUsecase.java (1)
203-214: Companion 객체 접근 방식이 일관적이지 않습니다.Line 205에서는
FeedDomainEvent.created()로 호출하고, Line 251에서는FeedDomainEvent.Companion.reacted()로 호출합니다. Java에서 Kotlin companion 메서드 호출 시@JvmStatic이 선언되어 있으므로 둘 다 동작하지만, 일관성을 위해 하나의 방식으로 통일하는 것이 좋겠습니다.통일 제안
- eventPublisher.publishEvent( - FeedDomainEvent.Companion.reacted( + eventPublisher.publishEvent( + FeedDomainEvent.reacted(Also applies to: 250-259
src/main/kotlin/leets/leenk/domain/notification/application/policy/NotificationPolicy.kt (1)
6-14: 스텁 구현은 괜찮지만, TODO 추적을 확인해 주세요.현재 모든 알림을 허용하는 상태이므로,
NotificationSetting연동 전까지는 의도대로 동작합니다. 이슈 트래커에 해당 TODO가 등록되어 있는지 확인해 주세요.이 TODO 항목에 대해 이슈를 생성해 드릴까요?
src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationService.kt (1)
26-47: fire-and-forget 패턴에서 알림 실패 시 재시도/보상 메커니즘이 없습니다.
send,sendBatch,sendOrUpdate모두scope.launch로 비동기 실행되어 호출자에게 결과가 전파되지 않습니다. 내부 catch에서 예외를 로깅만 하므로, 알림 저장이나 발행 실패 시 데이터가 유실될 수 있습니다. 현 단계에서는 허용 가능하나, 향후 재시도 또는 실패 큐 도입을 고려해 주세요.src/main/kotlin/leets/leenk/domain/notification/application/adapter/FeedNotificationAdapter.kt (3)
17-17: FQCN 대신 import 사용을 권장합니다.
leets.leenk.domain.user.domain.service.usersetting.UserSettingGetService를 FQCN으로 사용하고 있습니다. 파일 상단에 import 문을 추가하면 가독성이 향상됩니다. Line 74의java.time.LocalDateTime.now()도 동일합니다.
73-198:handleFeedReacted메서드가 과도하게 복잡합니다 — 분리를 권장합니다.이 메서드가 약 130줄에 걸쳐 first-reaction 처리와 milestone 처리 두 가지 독립된 로직을 수행하고 있습니다. 각각을 별도 private 메서드로 추출하면 가독성과 유지보수성이 크게 개선됩니다.
167-197:send와sendOrUpdate분기의NotificationRequest생성 로직이 동일합니다.
currentNotification의 존재 여부에 따라 호출 메서드만 다르고 요청 객체는 동일합니다. 중복을 제거할 수 있습니다.♻️ 중복 제거 제안
val lastMilestone = achievedMilestones.last() - if (currentNotification != null) { - notificationPort.sendOrUpdate( - NotificationRequest( - userId = event.feedAuthorId, - type = NotificationType.FEED_REACTION_COUNT, - targetId = event.feedId, - dynamicParams = listOf(lastMilestone), - metadata = - mapOf( - "reactionCount" to lastMilestone, - "details" to allDetails, - ), - ), - ) - } else { - notificationPort.send( - NotificationRequest( - userId = event.feedAuthorId, - type = NotificationType.FEED_REACTION_COUNT, - targetId = event.feedId, - dynamicParams = listOf(lastMilestone), - metadata = - mapOf( - "reactionCount" to lastMilestone, - "details" to allDetails, - ), - ), - ) - } + val request = NotificationRequest( + userId = event.feedAuthorId, + type = NotificationType.FEED_REACTION_COUNT, + targetId = event.feedId, + dynamicParams = listOf(lastMilestone), + metadata = mapOf( + "reactionCount" to lastMilestone, + "details" to allDetails, + ), + ) + + if (currentNotification != null) { + notificationPort.sendOrUpdate(request) + } else { + notificationPort.send(request) + }
src/main/kotlin/leets/leenk/domain/feed/domain/event/FeedDomainEvent.kt
Outdated
Show resolved
Hide resolved
.../kotlin/leets/leenk/domain/notification/application/adapter/FeedNotificationEventListener.kt
Show resolved
Hide resolved
.../kotlin/leets/leenk/domain/notification/application/adapter/FeedNotificationEventListener.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/domain/entity/enums/NotificationType.kt
Outdated
Show resolved
Hide resolved
...ain/kotlin/leets/leenk/domain/notification/domain/repository/NotificationEntityRepository.kt
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/domain/service/NotificationEntityGetService.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationPublisher.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationPublisher.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationService.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/kotlin/leets/leenk/domain/feed/application/usecase/FeedUsecase.kt`:
- Around line 273-286: The code double-counts reactions because
feedUpdateService.updateTotalReaction(...) calls
feed.increaseTotalReactionCount(request.reactionCount) and then you compute
totalReactionCount as feed.totalReactionCount + request.reactionCount; fix by
using the already-updated value (set val totalReactionCount =
feed.totalReactionCount) before publishing the FeedDomainEvent.reacted, or
alternatively capture previousReactionCount = feed.totalReactionCount before
calling feedUpdateService.updateTotalReaction(...) and compute
totalReactionCount = previousReactionCount + request.reactionCount; update the
logic around feedUpdateService.updateTotalReaction,
feed.increaseTotalReactionCount, and the totalReactionCount assignment
accordingly.
🧹 Nitpick comments (2)
src/main/kotlin/leets/leenk/domain/feed/application/usecase/FeedUsecase.kt (1)
74-74: 알림 패턴 혼재: 이벤트 기반 + 직접 호출.
FeedDomainEvent를 통한 이벤트 기반 알림(Lines 220, 277)과feedNotificationUsecase를 직접 호출하는 마일스톤 알림(Line 470)이 혼재되어 있습니다. 점진적 마이그레이션이라면 괜찮지만, 최종적으로 모든 알림을 이벤트 기반으로 통합하는 것이 일관성 면에서 바람직합니다.Also applies to: 277-286, 461-473
build.gradle.kts (1)
35-37:kotlinx-coroutines를 최신 버전(1.10.2)으로 업데이트하고 BOM으로 관리하세요.현재 1.8.0은 2024년 초 릴리스로 오래되었습니다. 최신 버전은 1.10.2(2025년 4월 릴리스)이며, Kotlin 2.3.0과 호환됩니다.
core와reactor버전을 개별 하드코딩하는 대신kotlinx-coroutines-bom을 사용하면 버전 불일치를 방지하고 의존성 관리를 단순화할 수 있습니다.♻️ BOM 사용 제안
// Kotlin Coroutines - implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core:1.8.0") - implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor:1.8.0") + implementation(platform("org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.10.2")) + implementation("org.jetbrains.kotlinx:kotlinx-coroutines-core") + implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor")
src/main/kotlin/leets/leenk/domain/feed/application/usecase/FeedUsecase.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/test/kotlin/leets/leenk/domain/feed/application/usecase/FeedUsecaseTest.kt`:
- Around line 96-97: Tests are out of sync: remove or update verifications for
feedNotificationUsecase.saveNewFeedNotification() and saveTagNotification() in
FeedUsecaseTest and instead assert that eventPublisher.publishEvent(...) is
called with the correct FeedDomainEvent; specifically, for
FeedUsecase.uploadFeed() and reactToFeed tests, change the mocks so
eventPublisher (mockk<ApplicationEventPublisher>) is verified (e.g., verify {
eventPublisher.publishEvent(match { it is FeedDomainEvent && /* check created()
payload fields */ }) }) and delete any verify calls referencing
saveNewFeedNotification or saveTagNotification; ensure you match the
FeedDomainEvent.created() payload fields used by the use case when asserting
publishEvent.
src/test/kotlin/leets/leenk/domain/feed/application/usecase/FeedUsecaseTest.kt
Show resolved
Hide resolved
hyxklee
left a comment
hyxklee
left a comment
There was a problem hiding this comment.
고생하셨습니다! 우선 아키텍처가 일부 애매한 부분이 있어서 그 부분에 집중해서 리뷰를 달았습니당
NotificationService 자체의 동작에 문제가 없었다면 괜찮을 것 같은데. 한 번 E2E 테스트는 진행이 되어야 머지가 가능할 것 같긴 하네욥
내일 재심사를 넣을 예정인데, 심사가 완료되면 수정할 부분 수정하고, 모든 도메인에 적용해서 머지하는 것을 목표로 해봅시다!
| var notificationType: NotificationType, | ||
| var content: NotificationPayload, | ||
| var isRead: Boolean = false, | ||
| // TODO: 마이그레이션 이후 몽고 BaseEntity 상속하도록 수정 |
There was a problem hiding this comment.
현재 몽고 BaseEntity가 kotlin으로 구현되지 않아 있습니다
먼저 kotlin으로 수정한 후에 기존 엔티티들(자바)도 kotlin 몽고 BaseEntity를 상속하도록 수정했었지만,
문제가 많이 발생하여 롤백하였고 다음 PR에서 작성하려고합니다!
|
|
||
| import leets.leenk.domain.notification.application.dto.NotificationRequest | ||
|
|
||
| interface NotificationPort { |
There was a problem hiding this comment.
현재 Port의 정의가 애매하게 느껴지는 것 같습니다!
Port와 Adapter의 구조를 봤을 때 해당 인터페이스를 NotificationService가 구현하고 있는데, 이 구조가
NotificationPublishPort(인터페이스) <- SqsNotificationPublisher(구현체) 이 방향으로 바뀌어야 외부 의존성의 교체가 자유로워질 것 같아욤
There was a problem hiding this comment.
NotificationPublishPort 인터페이스를 새로 생성하고, NotificationPublisher를 SqsNotificationPublisher로 이름을 변경하여 해당 포트를 구현하도록 수정했습니다
NotificationService에서도 구체 클래스 대신 NotificationPublishPort에 의존하도록 변경했습니다
| private val notificationEntityGetService: NotificationEntityGetService, | ||
| private val notificationPublisher: NotificationPublisher, | ||
| private val notificationPolicy: NotificationPolicy, | ||
| ) : NotificationPort { |
There was a problem hiding this comment.
해당 클래스를 인바운드 포트의 구현체로 둔 것이라면 이 인터페이스 관계를 제거하고, SendNotificationUsecase("Service)로 이전하는게 어떨까 싶어요
해당 클래스는 실질적인 "전송"의 서비스 로직을 구현하고 있는데, NotificationPort를 구현하는 서비스라는게 잘 와닿지 않는 것 같아용
There was a problem hiding this comment.
NotificationPort의 리뷰내용과 함께 반영해서 해당 클래스를 Application으로 올리고, Port 들을 의존하면서 Infra를 사용하는게 어떨까요??
There was a problem hiding this comment.
해당 클래스를 application.usecase를 올릴까요 domain.service로 올릴까요? 기능 별인지 실질적인 이름대로 위치를 해야할지 고민입니다
아 그리고 다른분들 피드백 완료되면 E2E 진행 하고, 심사가 완료되면 머지할 예정입니다
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component | ||
| class NotificationPublisher( |
There was a problem hiding this comment.
SqsNofiticaitionPublisher로 이름을 수정하고 NotificationPublishPort를 구현하면 아웃바운드 포트의 역할이 분명해질 것 같아염
There was a problem hiding this comment.
넵 맞습니다 NotificationPublisher -> SqsNotificationPublisher로 수정하고 NotificationPublishPort를 구현하도록 수정했습니다
| val user = userGetService.findById(userId) | ||
| val userSetting = userSettingGetService.findByUser(user) | ||
|
|
||
| return when (type) { |
1winhyun
left a comment
1winhyun
left a comment
There was a problem hiding this comment.
수고하셨습니다!! 리뷰가 늦어서 죄송합니다... 확인한번 부탁드려요!!
src/main/kotlin/leets/leenk/domain/notification/infrastructure/NotificationService.kt
Show resolved
Hide resolved
.../kotlin/leets/leenk/domain/notification/application/adapter/FeedNotificationEventListener.kt
Show resolved
Hide resolved
.../kotlin/leets/leenk/domain/notification/application/adapter/FeedNotificationEventListener.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/domain/service/NotificationEntityGetService.kt
Show resolved
Hide resolved
src/main/kotlin/leets/leenk/domain/notification/domain/service/NotificationSaveService.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/leets/leenk/domain/notification/infrastructure/NotificationServiceTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/leets/leenk/domain/notification/infrastructure/NotificationServiceTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/leets/leenk/domain/notification/infrastructure/NotificationServiceTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/leets/leenk/domain/notification/infrastructure/NotificationServiceTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/leets/leenk/domain/notification/infrastructure/NotificationServiceTest.kt
Outdated
Show resolved
Hide resolved
soo0711
left a comment
soo0711
left a comment
There was a problem hiding this comment.
리뷰를 너무 늦게 달아서 죄송합니다 ㅜ.ㅜ
수고하셨습니다!! 아래 내용들 확인 부탁드립니당 👍
| } | ||
|
|
||
| @Transactional | ||
| public void markNotificationAsRead(Long userId, String notificationId) { |
There was a problem hiding this comment.
클래스명에서 이미 Notification이라는 문맥을 알 수 있으니 메서드명에서는 중복을 제거하고 markAsRead로 간결하게 줄이는 게 어떨까요?
| FeedDomainEvent.Created( | ||
| feedId = feed.id!!, | ||
| authorId = author.id!!, | ||
| authorName = author.name, | ||
| taggedUserIds = taggedUserIds, | ||
| ), |
There was a problem hiding this comment.
저도 피드백 받았는데 코틀린에서 단언은 지양한다고 합니당
feedId = feed.id ?: throw IllegalStateException("영속화되지 않은 Feed입니다."),로 예외를 던진다로 변경하는게 어떨까요?
There was a problem hiding this comment.
넵! 좋습니다 형식은 수현님이 해두신 형식 따라가겠습니다
| FeedDomainEvent.Reacted( | ||
| feedId = feed.id!!, | ||
| feedAuthorId = feed.user.id!!, | ||
| reactorId = user.id!!, |
| val targetId: Long, | ||
| val name: String? = null, | ||
| val title: String? = null, | ||
| val count: Any? = null, |
There was a problem hiding this comment.
현재 count가 주로 좋아요 개수나 마일스톤 같은 숫자형 데이터를 담는 용도로 보이는데 그렇다면 Any?보다는 Int?로 타입을 변환하는 게 어떨까욤??!
4 participants
작업 내용 💻
기존 알림과 구조가 달라져서 자바에서 구조를 바꾸면 전체 코드를 바꿔야하는 상황이라, 일단 알림 코틀린 버전으로 추가했습니다
조회는 추후 추가하겠습니다
알림 시스템 아키텍처 전면 개편
기존 구조의 문제점
강한 결합: 각 도메인 Usecase가 알림 Usecase를 직접 의존하여 도메인 순수성 훼손
중복 코드: 16개 알림 타입마다 FCM 토큰 확인, UserSetting 확인, 예외 처리 로직 반복
복잡한 다형성: 16개의 NotificationContent 서브클래스와 instanceof 체크 난립
동기 처리: 대량 알림 생성 시 성능 병목 발생
낮은 확장성: 새 알림 타입 추가 시 5-6개 파일 수정 필요
도메인은 알림 시스템을 모르고 순수 도메인 이벤트만 발행
도메인 이벤트 → 알림 변환은 어댑터가 담당
도메인 간 결합도 제거 (이벤트 기반 비동기 처리)
도메인 이벤트: 11개 클래스 → 3개로 통합 (도메인별 1개 + Map 활용)
NotificationContent: 16개 서브클래스 → 1개 데이터 클래스 (metadata로 유연하게 확장)
알림 발행 정책: NotificationPolicy로 중앙화 (FCM, UserSetting 확인)
Thread Pool 대신 경량 코루틴 사용 (메모리 효율 향상)
sendBatch()로 대량 알림 병렬 처리
SupervisorJob으로 개별 실패가 전체에 영향 없도록 격리
FEED_FIRST_REACTION, FEED_REACTION_COUNT는 metadata의 details 배열에 집계
각 detail에 createDate 추가하여 개별 반응 시간 추적
마일스톤은 이전/현재 카운트 사이의 모든 마일스톤 한 번에 처리
스크린샷 📷
[ { "_id": { "$oid": "698d68c0a87a558af0421aef" }, "_class": "leets.leenk.domain.notification.domain.entity.NotificationEntity", "userId": 1, "notificationType": "FEED_REACTION_COUNT", "isRead": false, "content": { "title": "Leenk", "body": "내가 쓴 피드에 좋아요를 50개 받았어", "path": "/feeds/2", "targetId": 2, "metadata": { "details": [ { "milestone": 5, "title": "Leenk", "body": "내가 쓴 피드에 좋아요를 5개 받았어", "createDate": { "$date": "2026-02-12T05:44:32.611Z" } }, { "milestone": 10, "title": "Leenk", "body": "내가 쓴 피드에 좋아요를 10개 받았어", "createDate": { "$date": "2026-02-12T05:44:32.611Z" } }, { "milestone": 25, "title": "Leenk", "body": "내가 쓴 피드에 좋아요를 25개 받았어", "createDate": { "$date": "2026-02-12T05:44:32.611Z" } }, { "milestone": 50, "title": "Leenk", "body": "내가 쓴 피드에 좋아요를 50개 받았어", "createDate": { "$date": "2026-02-12T05:44:32.611Z" } } ] } }, "createDate": { "$date": "2026-02-12T05:44:32.686Z" }, "updateDate": { "$date": "2026-02-12T05:44:32.686Z" } } ]{ "title": "Leenk", "body": "[user1]이 나를 함께한 사람에 추가했어", "path": "/feeds/2", "targetId": 2, "metadata": {} }같이 얘기해보고 싶은 내용이 있다면 작성 📢
Summary by CodeRabbit
릴리즈 노트
새로운 기능
리팩토링
테스트