refactor: 키워드 알림 EventListener 책임 분리#2237
Open
Soundbar91 wants to merge 26 commits into
Open
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughRefactors keyword notification: replaces per-keyword event with per-article and lost-item events, simplifies keyword extraction to title matching, introduces ArticleKeywordUserMatcher, adds dedicated notification services and listener delegation, updates repository queries, and removes related unit tests. ChangesKeyword Notification System Refactoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (9)
src/main/java/in/koreatech/koin/domain/community/keyword/service/ArticleKeywordUserMatcher.java (2)
24-49: 💤 Low valueAdd an empty-input short-circuit.
When
matchedKeywordsis empty, the repository query still executes (... IN ()), incurring a needless roundtrip and, depending on JPA provider settings, potentially generating an awkward SQL. Returning an empty map immediately is cheaper and explicit.♻️ Suggested guard
public Map<String, List<Integer>> findUserIdsByMatchedKeyword( KeywordCategory category, List<String> matchedKeywords ) { + if (matchedKeywords == null || matchedKeywords.isEmpty()) { + return Map.of(); + } Map<Integer, ArticleKeyword> keywordByUserId = new LinkedHashMap<>();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/community/keyword/service/ArticleKeywordUserMatcher.java` around lines 24 - 49, Add a short-circuit at the start of findUserIdsByMatchedKeyword: if the matchedKeywords list is null or empty, return an empty LinkedHashMap immediately to avoid calling articleKeywordUserMapRepository.findAllByArticleKeywordCategoryAndArticleKeywordKeywordIn with an empty collection; ensure you check the matchedKeywords parameter (and treat null as empty) before any repository call so the method returns quickly when there's nothing to query.
29-49: ⚖️ Poor tradeoffConsider letting
KeywordExtractorreturnArticleKeywordentities to avoid double-fetching.
KeywordExtractor.matchKeywordspaginates and loadsArticleKeywordentities, then returns only theirkeywordtext. This service then re-fetches the sameArticleKeywordrows via the newIN :keywordsquery (joined toArticleKeywordUserMap). For categories with many keywords or high publish frequency, this is a redundant DB hit per article.If you keep the current shape, this is fine; otherwise consider returning
List<ArticleKeyword>(or IDs) from the extractor and querying the user-map byarticleKeyword.id IN :ids, which is also a more selective index path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/community/keyword/service/ArticleKeywordUserMatcher.java` around lines 29 - 49, KeywordExtractor.matchKeywords currently returns List<String> causing ArticleKeyword rows to be refetched via articleKeywordUserMapRepository.findAllByArticleKeywordCategoryAndArticleKeywordKeywordIn; change the extractor to return List<ArticleKeyword> (or List<Long> ids) so the service can use those directly and avoid the redundant lookup, then update the repository call to use articleKeyword IDs (e.g., findAllByArticleKeywordCategoryAndArticleKeywordIdIn) or compare ArticleKeyword entities directly (findAllByArticleKeywordCategoryAndArticleKeywordIn) and adapt the loop that builds keywordByUserId to use ArticleKeyword objects instead of re-querying by keyword text.src/main/java/in/koreatech/koin/domain/notification/model/NotificationFactory.java (1)
235-240: ⚡ Quick winURL-encode
keywordwhen building the scheme URI.
keywordis interpolated raw into the deep-link query string. If a user's keyword contains&,=,?,#, whitespace, or non-ASCII characters, the resulting URI is malformed and the downstreamLIKE :schemeUriPatterndedupe inNotificationRepository.findUserIdsBySchemeUriLikeAndUserIdInmay also under-/over-match. The same concern applies to the existinggenerateKeywordSchemeUrion line 232.♻️ Suggested fix
+ import java.net.URLEncoder; + import java.nio.charset.StandardCharsets; ... private String generateLostItemKeywordSchemeUri(MobileAppPath path, Integer eventId, String keyword) { if (keyword == null) { return generateSchemeUri(path, eventId); } - return String.format("%s?id=%d&keyword=%s", path.getPath(), eventId, keyword); + String encoded = URLEncoder.encode(keyword, StandardCharsets.UTF_8); + return String.format("%s?id=%d&keyword=%s", path.getPath(), eventId, encoded); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/notification/model/NotificationFactory.java` around lines 235 - 240, The methods generateLostItemKeywordSchemeUri and generateKeywordSchemeUri interpolate raw keyword into the deep-link query and must URL-encode keyword before building the scheme URI; update both functions to return generateSchemeUri(path,eventId) when keyword is null, otherwise encode the keyword using UTF-8 (e.g., via URLEncoder.encode(keyword, StandardCharsets.UTF_8.name()) or equivalent) and use the encoded value in the String.format call so reserved characters and non-ASCII text produce a valid query string.src/main/java/in/koreatech/koin/common/event/LostItemKeywordEvent.java (1)
12-16: 💤 Low valueConsider sharing
MatchedKeywordUserswithKoreatechArticleKeywordEventif the shape is identical.Per the PR description, both events carry a
Map<String, List<Integer>> userIdsByKeyword. IfKoreatechArticleKeywordEventdefines its own nestedMatchedKeywordUsers, the duplication will drift over time. Hoisting the value object toin.koreatech.koin.common.event(or a sibling shared type) keeps both events aligned and lets cross-cutting concerns (validation, defensive copy) live in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/common/event/LostItemKeywordEvent.java` around lines 12 - 16, The nested record MatchedKeywordUsers in LostItemKeywordEvent duplicates the same shape used by KoreatechArticleKeywordEvent; hoist MatchedKeywordUsers out of the nested class into a shared top-level type in package in.koreatech.koin.common.event (or a sibling shared package), remove the nested declarations from LostItemKeywordEvent and KoreatechArticleKeywordEvent, update both classes to import and reference the new shared MatchedKeywordUsers type, and consolidate any validation or defensive-copy logic into that shared record so both events reuse the single canonical implementation.src/main/java/in/koreatech/koin/domain/notification/repository/NotificationSubscribeRepository.java (1)
31-43: ⚡ Quick winMethod name embeds "ArticleKeyword" but the query is reused for lost-item subscribes.
The query is fully parameterized over
subscribeType, so it serves bothARTICLE_KEYWORDandLOST_ITEM_KEYWORDflows. Naming itfindArticleKeywordSubscribesByUserIdInwill mislead future readers and grep-based audits when the lost-item service calls the same method.♻️ Suggested rename
- List<NotificationSubscribe> findArticleKeywordSubscribesByUserIdIn( + List<NotificationSubscribe> findActiveSubscribesByTypeAndUserIdIn( `@Param`("subscribeType") NotificationSubscribeType subscribeType, `@Param`("userIds") List<Integer> userIds );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/notification/repository/NotificationSubscribeRepository.java` around lines 31 - 43, The repository method NotificationSubscribeRepository.findArticleKeywordSubscribesByUserIdIn is misnamed because the JPQL is generic over subscribeType; rename the method to a neutral, accurate name like findKeywordSubscribesByUserIdIn or findBySubscribeTypeAndUserIds (keeping the same parameters `@Param`("subscribeType") NotificationSubscribeType subscribeType and `@Param`("userIds") List<Integer> userIds) and update all call sites that reference findArticleKeywordSubscribesByUserIdIn (e.g., services handling ARTICLE_KEYWORD and LOST_ITEM_KEYWORD flows) to the new method name so behavior and query remain unchanged but the name reflects its generic use.src/main/java/in/koreatech/koin/domain/community/util/KeywordExtractor.java (2)
29-42: 💤 Low valuePagination index can be simplified.
PageRequest.of(offset / KEYWORD_BATCH_SIZE, KEYWORD_BATCH_SIZE)re-derives a page number from a manual offset counter. A directint page = 0; ... page++;is clearer and avoids the implicit invariant thatoffsetmust always be a multiple ofKEYWORD_BATCH_SIZE.♻️ Suggested simplification
- int offset = 0; + int page = 0; while (true) { - Pageable pageable = PageRequest.of(offset / KEYWORD_BATCH_SIZE, KEYWORD_BATCH_SIZE); + Pageable pageable = PageRequest.of(page, KEYWORD_BATCH_SIZE); List<ArticleKeyword> keywords = articleKeywordRepository.findAllByCategory(category, pageable); if (keywords.isEmpty()) { break; } for (ArticleKeyword keyword : keywords) { if (title.contains(keyword.getKeyword())) { matchedKeywords.add(keyword.getKeyword()); } } - offset += KEYWORD_BATCH_SIZE; + page++; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/community/util/KeywordExtractor.java` around lines 29 - 42, The pagination loop in KeywordExtractor uses an offset integer and computes the page as PageRequest.of(offset / KEYWORD_BATCH_SIZE, KEYWORD_BATCH_SIZE), which is confusing and fragile; change the loop to use an explicit page counter (e.g., int page = 0) and create the pageable with PageRequest.of(page, KEYWORD_BATCH_SIZE), incrementing page++ each iteration and removing the offset arithmetic; keep the same stop condition on keywords.isEmpty() and leave KEYWORD_BATCH_SIZE and articleKeywordRepository.findAllByCategory intact.
25-45: 💤 Low valueGuard against null
title.
title.contains(...)will NPE if a caller ever passes a null title. Adding a defensive early-return keeps this utility robust against upstream changes (e.g., a new article publisher path) and avoids paginating the keyword table just to throw.🛡️ Suggested guard
public List<String> matchKeywords(String title, KeywordCategory category) { + if (title == null || title.isBlank()) { + return List.of(); + } List<String> matchedKeywords = new ArrayList<>(); int offset = 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/community/util/KeywordExtractor.java` around lines 25 - 45, In matchKeywords (KeywordExtractor) add a defensive null-check for the title parameter and return an empty List<String> immediately if title is null (to avoid NPE from title.contains(...)); locate the matchKeywords method and put the guard before any pagination or repository calls (before PageRequest.of(...) / articleKeywordRepository.findAllByCategory(...)) so the method exits early when title is null and avoids unnecessary DB paging using the existing matchedKeywords empty list as the return.src/main/java/in/koreatech/koin/domain/notification/service/ArticleKeywordNotificationService.java (1)
67-69: ⚡ Quick winBuild the dedupe pattern from
KEYWORD, not a string literal.
createNotification()derives the target URI fromKEYWORD, but this lookup hardcodes"keyword". If those ever diverge, duplicate suppression breaks and users can receive repeated notifications for the same article.Suggested change
private Set<Integer> getAlreadyNotifiedUserIds(Integer articleId, List<Integer> userIds) { return new HashSet<>(notificationRepository - .findUserIdsBySchemeUriLikeAndUserIdIn("keyword?id=%d&%%".formatted(articleId), userIds)); + .findUserIdsBySchemeUriLikeAndUserIdIn( + "%s?id=%d&%%".formatted(KEYWORD.getPath(), articleId), + userIds + )); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/domain/notification/service/ArticleKeywordNotificationService.java` around lines 67 - 69, getAlreadyNotifiedUserIds currently hardcodes "keyword" when building the scheme-uri LIKE pattern which can diverge from the KEYWORD used in createNotification; change the pattern construction to use the KEYWORD constant (e.g., build "%s?id=%d&%%" with KEYWORD and articleId) so the call to notificationRepository.findUserIdsBySchemeUriLikeAndUserIdIn uses the same source of truth as createNotification, preventing duplicate notifications.src/main/java/in/koreatech/koin/common/event/KoreatechArticleKeywordEvent.java (1)
24-28: ⚡ Quick winSnapshot the matched users before publishing the async event.
MatchedKeywordUserscurrently wraps the original mutableMap<String, List<Integer>>by reference. Because the listener runsAFTER_COMMITand@Async, later mutations can leak into delivery or trigger concurrent modification issues. Copy the map and inner lists when constructing the event.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/in/koreatech/koin/common/event/KoreatechArticleKeywordEvent.java` around lines 24 - 28, The event is passing the original mutable Map into MatchedKeywordUsers by reference, risking concurrent mutation during async AFTER_COMMIT delivery; update the construction so MatchedKeywordUsers receives a defensive deep copy (copy the outer Map and for each entry copy the inner List) instead of the original reference—either modify where new MatchedKeywordUsers(userIdsByKeyword) is called in KoreatechArticleKeywordEvent or add copying logic inside MatchedKeywordUsers' constructor to produce an immutable/independent Map<String,List<Integer>> snapshot.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/in/koreatech/koin/common/event/KoreatechArticleKeywordEvent.java`:
- Around line 6-10: KoreatechArticleKeywordEvent is missing authorId so
ArticleKeywordNotificationService cannot exclude notifying the post author; add
an Integer authorId field to the KoreatechArticleKeywordEvent record (same
pattern as LostItemKeywordEvent), update all places that construct the
KoreatechArticleKeywordEvent to pass the article's authorId, and update any
consumers (e.g., ArticleKeywordNotificationService) to read event.authorId() to
preserve the "exclude self" logic; ensure compilation by adjusting imports and
tests that construct or pattern-match the record.
In
`@src/main/java/in/koreatech/koin/domain/notification/eventlistener/LostItemKeywordEventListener.java`:
- Around line 13-15: The LostItemKeywordEventListener class is always active and
must be disabled for the test profile; add the same profile restriction used on
ArticleKeywordEventListener by annotating the LostItemKeywordEventListener class
with `@Profile`("!test") (and import
org.springframework.context.annotation.Profile) so the async AFTER_COMMIT
listener is not active during tests.
---
Nitpick comments:
In
`@src/main/java/in/koreatech/koin/common/event/KoreatechArticleKeywordEvent.java`:
- Around line 24-28: The event is passing the original mutable Map into
MatchedKeywordUsers by reference, risking concurrent mutation during async
AFTER_COMMIT delivery; update the construction so MatchedKeywordUsers receives a
defensive deep copy (copy the outer Map and for each entry copy the inner List)
instead of the original reference—either modify where new
MatchedKeywordUsers(userIdsByKeyword) is called in KoreatechArticleKeywordEvent
or add copying logic inside MatchedKeywordUsers' constructor to produce an
immutable/independent Map<String,List<Integer>> snapshot.
In `@src/main/java/in/koreatech/koin/common/event/LostItemKeywordEvent.java`:
- Around line 12-16: The nested record MatchedKeywordUsers in
LostItemKeywordEvent duplicates the same shape used by
KoreatechArticleKeywordEvent; hoist MatchedKeywordUsers out of the nested class
into a shared top-level type in package in.koreatech.koin.common.event (or a
sibling shared package), remove the nested declarations from
LostItemKeywordEvent and KoreatechArticleKeywordEvent, update both classes to
import and reference the new shared MatchedKeywordUsers type, and consolidate
any validation or defensive-copy logic into that shared record so both events
reuse the single canonical implementation.
In
`@src/main/java/in/koreatech/koin/domain/community/keyword/service/ArticleKeywordUserMatcher.java`:
- Around line 24-49: Add a short-circuit at the start of
findUserIdsByMatchedKeyword: if the matchedKeywords list is null or empty,
return an empty LinkedHashMap immediately to avoid calling
articleKeywordUserMapRepository.findAllByArticleKeywordCategoryAndArticleKeywordKeywordIn
with an empty collection; ensure you check the matchedKeywords parameter (and
treat null as empty) before any repository call so the method returns quickly
when there's nothing to query.
- Around line 29-49: KeywordExtractor.matchKeywords currently returns
List<String> causing ArticleKeyword rows to be refetched via
articleKeywordUserMapRepository.findAllByArticleKeywordCategoryAndArticleKeywordKeywordIn;
change the extractor to return List<ArticleKeyword> (or List<Long> ids) so the
service can use those directly and avoid the redundant lookup, then update the
repository call to use articleKeyword IDs (e.g.,
findAllByArticleKeywordCategoryAndArticleKeywordIdIn) or compare ArticleKeyword
entities directly (findAllByArticleKeywordCategoryAndArticleKeywordIn) and adapt
the loop that builds keywordByUserId to use ArticleKeyword objects instead of
re-querying by keyword text.
In `@src/main/java/in/koreatech/koin/domain/community/util/KeywordExtractor.java`:
- Around line 29-42: The pagination loop in KeywordExtractor uses an offset
integer and computes the page as PageRequest.of(offset / KEYWORD_BATCH_SIZE,
KEYWORD_BATCH_SIZE), which is confusing and fragile; change the loop to use an
explicit page counter (e.g., int page = 0) and create the pageable with
PageRequest.of(page, KEYWORD_BATCH_SIZE), incrementing page++ each iteration and
removing the offset arithmetic; keep the same stop condition on
keywords.isEmpty() and leave KEYWORD_BATCH_SIZE and
articleKeywordRepository.findAllByCategory intact.
- Around line 25-45: In matchKeywords (KeywordExtractor) add a defensive
null-check for the title parameter and return an empty List<String> immediately
if title is null (to avoid NPE from title.contains(...)); locate the
matchKeywords method and put the guard before any pagination or repository calls
(before PageRequest.of(...) / articleKeywordRepository.findAllByCategory(...))
so the method exits early when title is null and avoids unnecessary DB paging
using the existing matchedKeywords empty list as the return.
In
`@src/main/java/in/koreatech/koin/domain/notification/model/NotificationFactory.java`:
- Around line 235-240: The methods generateLostItemKeywordSchemeUri and
generateKeywordSchemeUri interpolate raw keyword into the deep-link query and
must URL-encode keyword before building the scheme URI; update both functions to
return generateSchemeUri(path,eventId) when keyword is null, otherwise encode
the keyword using UTF-8 (e.g., via URLEncoder.encode(keyword,
StandardCharsets.UTF_8.name()) or equivalent) and use the encoded value in the
String.format call so reserved characters and non-ASCII text produce a valid
query string.
In
`@src/main/java/in/koreatech/koin/domain/notification/repository/NotificationSubscribeRepository.java`:
- Around line 31-43: The repository method
NotificationSubscribeRepository.findArticleKeywordSubscribesByUserIdIn is
misnamed because the JPQL is generic over subscribeType; rename the method to a
neutral, accurate name like findKeywordSubscribesByUserIdIn or
findBySubscribeTypeAndUserIds (keeping the same parameters
`@Param`("subscribeType") NotificationSubscribeType subscribeType and
`@Param`("userIds") List<Integer> userIds) and update all call sites that
reference findArticleKeywordSubscribesByUserIdIn (e.g., services handling
ARTICLE_KEYWORD and LOST_ITEM_KEYWORD flows) to the new method name so behavior
and query remain unchanged but the name reflects its generic use.
In
`@src/main/java/in/koreatech/koin/domain/notification/service/ArticleKeywordNotificationService.java`:
- Around line 67-69: getAlreadyNotifiedUserIds currently hardcodes "keyword"
when building the scheme-uri LIKE pattern which can diverge from the KEYWORD
used in createNotification; change the pattern construction to use the KEYWORD
constant (e.g., build "%s?id=%d&%%" with KEYWORD and articleId) so the call to
notificationRepository.findUserIdsBySchemeUriLikeAndUserIdIn uses the same
source of truth as createNotification, preventing duplicate notifications.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 441d18df-9053-4a73-95db-3417d872b94e
📒 Files selected for processing (22)
src/main/java/in/koreatech/koin/common/event/ArticleKeywordEvent.javasrc/main/java/in/koreatech/koin/common/event/KoreatechArticleKeywordEvent.javasrc/main/java/in/koreatech/koin/common/event/LostItemKeywordEvent.javasrc/main/java/in/koreatech/koin/domain/community/article/repository/ArticleRepository.javasrc/main/java/in/koreatech/koin/domain/community/article/service/LostItemArticleService.javasrc/main/java/in/koreatech/koin/domain/community/keyword/dto/KeywordNotificationRequest.javasrc/main/java/in/koreatech/koin/domain/community/keyword/model/ArticleKeyword.javasrc/main/java/in/koreatech/koin/domain/community/keyword/repository/ArticleKeywordRepository.javasrc/main/java/in/koreatech/koin/domain/community/keyword/repository/ArticleKeywordUserMapRepository.javasrc/main/java/in/koreatech/koin/domain/community/keyword/service/ArticleKeywordUserMatcher.javasrc/main/java/in/koreatech/koin/domain/community/keyword/service/KeywordService.javasrc/main/java/in/koreatech/koin/domain/community/util/KeywordExtractor.javasrc/main/java/in/koreatech/koin/domain/notification/eventlistener/ArticleKeywordEventListener.javasrc/main/java/in/koreatech/koin/domain/notification/eventlistener/LostItemKeywordEventListener.javasrc/main/java/in/koreatech/koin/domain/notification/model/NotificationFactory.javasrc/main/java/in/koreatech/koin/domain/notification/repository/NotificationRepository.javasrc/main/java/in/koreatech/koin/domain/notification/repository/NotificationSubscribeRepository.javasrc/main/java/in/koreatech/koin/domain/notification/service/ArticleKeywordNotificationService.javasrc/main/java/in/koreatech/koin/domain/notification/service/LostItemKeywordNotificationService.javasrc/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.javasrc/test/java/in/koreatech/koin/unit/domain/community/util/KeywordExtractorTest.javasrc/test/java/in/koreatech/koin/unit/domain/notification/eventlistener/ArticleKeywordEventListenerTest.java
💤 Files with no reviewable changes (5)
- src/main/java/in/koreatech/koin/common/event/ArticleKeywordEvent.java
- src/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.java
- src/main/java/in/koreatech/koin/domain/community/keyword/repository/ArticleKeywordRepository.java
- src/test/java/in/koreatech/koin/unit/domain/community/util/KeywordExtractorTest.java
- src/test/java/in/koreatech/koin/unit/domain/notification/eventlistener/ArticleKeywordEventListenerTest.java
There was a problem hiding this comment.
Pull request overview
키워드 알림 처리에서 EventListener에 섞여 있던 비즈니스 로직(구독 조회/중복 제거/기발송 제외/알림 생성)을 역할별 서비스로 분리하고, 공지(KOREATECH)와 분실물(LOST_ITEM) 키워드 알림 흐름을 이벤트/서비스 단위로 분리한 PR입니다.
Changes:
- 게시글(공지) 키워드 알림:
KoreatechArticleKeywordEvent+ArticleKeywordNotificationService로 책임 분리 - 분실물 키워드 알림:
LostItemKeywordEvent+LostItemKeywordNotificationService및 전용 리스너 추가 - 키워드 매칭/구독 유저 매핑 로직 재구성(
KeywordExtractor단순화,ArticleKeywordUserMatcher도입) 및 관련 Repository 쿼리 추가
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/in/koreatech/koin/unit/domain/notification/eventlistener/ArticleKeywordEventListenerTest.java | 기존 리스너 중심 단위 테스트 삭제 |
| src/test/java/in/koreatech/koin/unit/domain/community/util/KeywordExtractorTest.java | 기존 KeywordExtractor 이벤트 생성 테스트 삭제 |
| src/test/java/in/koreatech/koin/unit/domain/community/keyword/service/KeywordServiceTest.java | 기존 KeywordService 이벤트 발행/이력 저장 테스트 삭제 |
| src/main/java/in/koreatech/koin/domain/notification/service/LostItemKeywordNotificationService.java | 분실물 키워드 알림 비즈니스 로직 서비스로 분리 |
| src/main/java/in/koreatech/koin/domain/notification/service/ArticleKeywordNotificationService.java | 공지(게시글) 키워드 알림 비즈니스 로직 서비스로 분리 |
| src/main/java/in/koreatech/koin/domain/notification/repository/NotificationSubscribeRepository.java | 매칭 유저 ID 기반 구독 조회 쿼리 추가 |
| src/main/java/in/koreatech/koin/domain/notification/repository/NotificationRepository.java | schemeUri LIKE 기반 기발송 사용자 조회 쿼리 추가 |
| src/main/java/in/koreatech/koin/domain/notification/model/NotificationFactory.java | 키워드 알림 생성 API 변경 및 분실물 전용 생성 메서드 추가 |
| src/main/java/in/koreatech/koin/domain/notification/eventlistener/LostItemKeywordEventListener.java | 분실물 키워드 이벤트 리스너 신규 추가 |
| src/main/java/in/koreatech/koin/domain/notification/eventlistener/ArticleKeywordEventListener.java | 리스너를 단순 위임 구조로 리팩터링 및 이벤트 타입 분리 |
| src/main/java/in/koreatech/koin/domain/community/util/KeywordExtractor.java | 이벤트 생성 책임 제거, 제목 기반 매칭 키워드 리스트 반환으로 단순화 |
| src/main/java/in/koreatech/koin/domain/community/keyword/service/KeywordService.java | 공지 키워드 알림 이벤트 발행 흐름을 신규 구성 요소에 맞게 변경 |
| src/main/java/in/koreatech/koin/domain/community/keyword/service/ArticleKeywordUserMatcher.java | 매칭 키워드에 대한 구독 유저 매핑/우선순위(더 긴 키워드) 선정 로직 분리 |
| src/main/java/in/koreatech/koin/domain/community/keyword/repository/ArticleKeywordUserMapRepository.java | 카테고리/키워드 IN 기반 유저 매핑 조회 쿼리 추가 및 미사용 메서드 제거 |
| src/main/java/in/koreatech/koin/domain/community/keyword/repository/ArticleKeywordRepository.java | 미사용 deleteById 제거 |
| src/main/java/in/koreatech/koin/domain/community/keyword/model/ArticleKeyword.java | “더 긴 키워드 우선” 비교 유틸 메서드 추가 |
| src/main/java/in/koreatech/koin/domain/community/keyword/dto/KeywordNotificationRequest.java | updateNotification 타입을 List → Set으로 변경 |
| src/main/java/in/koreatech/koin/domain/community/article/service/LostItemArticleService.java | 분실물 키워드 이벤트 발행 흐름을 신규 구성 요소에 맞게 변경 |
| src/main/java/in/koreatech/koin/domain/community/article/repository/ArticleRepository.java | findAllByIdIn 파라미터를 Collection으로 확장 |
| src/main/java/in/koreatech/koin/common/event/LostItemKeywordEvent.java | 분실물 키워드 이벤트 신규 추가 |
| src/main/java/in/koreatech/koin/common/event/KoreatechArticleKeywordEvent.java | 공지(게시글) 키워드 이벤트 신규 추가 |
| src/main/java/in/koreatech/koin/common/event/ArticleKeywordEvent.java | 기존 통합 이벤트 삭제 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+18
to
+30
| public static KoreatechArticleKeywordEvent of( | ||
| Integer articleId, | ||
| Integer boardId, | ||
| String articleTitle, | ||
| Map<String, List<Integer>> userIdsByKeyword | ||
| ) { | ||
| return new KoreatechArticleKeywordEvent( | ||
| articleId, | ||
| boardId, | ||
| articleTitle, | ||
| new MatchedKeywordUsers(userIdsByKeyword) | ||
| ); | ||
| } |
Comment on lines
+18
to
+30
| public static LostItemKeywordEvent of( | ||
| Integer articleId, | ||
| String articleTitle, | ||
| Integer authorId, | ||
| Map<String, List<Integer>> userIdsByKeyword | ||
| ) { | ||
| return new LostItemKeywordEvent( | ||
| articleId, | ||
| articleTitle, | ||
| authorId, | ||
| new MatchedKeywordUsers(userIdsByKeyword) | ||
| ); | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 개요
🚀 주요 변경 내용
게시글/분실물 키워드 알림 처리 흐름을 역할별 서비스로 분리했습니다.
게시글 키워드 알림 API 처리 흐름
KeywordService가 알림 대상 게시글 제목에서KOREATECH카테고리 키워드를 추출합니다.ArticleKeywordUserMatcher가 매칭된 키워드별 구독 유저 ID를 조회합니다.KoreatechArticleKeywordEvent를 발행합니다.ArticleKeywordEventListener가ArticleKeywordNotificationService에 처리를 위임합니다.ArticleKeywordNotificationService처리 흐름userIdsByKeyword에서 중복을 제거한 전체 매칭 유저 ID 목록을 만듭니다.ARTICLE_KEYWORD타입으로 알림 수신 가능한 구독 정보를 조회합니다.분실물 키워드 알림 API 처리 흐름
LostItemArticleService가 제목에서LOST_ITEM카테고리 키워드를 추출합니다.ArticleKeywordUserMatcher가 매칭된 키워드별 구독 유저 ID를 조회합니다.LostItemKeywordEvent를 발행합니다.LostItemKeywordEventListener가LostItemKeywordNotificationService에 처리를 위임합니다.LostItemKeywordNotificationService처리 흐름userIdsByKeyword에서 중복을 제거한 전체 매칭 유저 ID 목록을 만듭니다.LOST_ITEM_KEYWORD타입으로 알림 수신 가능한 구독 정보를 조회합니다.기타 정리
💬 참고 사항
✅ Checklist (완료 조건)
Summary by CodeRabbit
Refactor
Bug Fixes
Style