[Refactor] Remove flowerSpotId from FlowerSpotCafe and support query BloomingStatus by FlowerSpotCafeId#135
Merged
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…feId Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThis PR refactors the blooming system to directly associate blooming records with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
LeeHanEum
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
pida-core/core-domain/src/test/kotlin/com/pida/notification/bloomed/BloomedFirstVoteNotificationEventListenerTest.kt (1)
22-65:⚠️ Potential issue | 🟡 MinorMissing test for the new
NewBlooming.FlowerSpotCafebranch.The test now injects
FlowerSpotCafeRepository(Lines 25 and 32), but only verifies theNewBlooming.FlowerEventflow.
Please add a case that sendsNewBlooming.FlowerSpotCafe, stubsflowerSpotCafeRepository.findBy(...), and verifies region-based notification behavior for that branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/test/kotlin/com/pida/notification/bloomed/BloomedFirstVoteNotificationEventListenerTest.kt` around lines 22 - 65, Add a parallel test case that exercises the NewBlooming.FlowerSpotCafe branch: mock/stub flowerSpotCafeRepository.findBy(...) to return a FlowerSpotCafe instance (set region = Region.SEOUL), ensure firstVoteChecker.isFirstBloomedVoteOfYear(Region.SEOUL) returns true and bloomedNotificationService.sendBloomedNotificationForRegion(Region.SEOUL) returns a value, call listener.handleBloomingAddedEvent(...) with NewBlooming.FlowerSpotCafe (userId and flowerSpotCafeId), then verify flowerSpotCafeRepository.findBy(...) was called and bloomedNotificationService.sendBloomedNotificationForRegion(Region.SEOUL) was invoked; reference: FlowerSpotCafeRepository, FlowerSpotCafe, NewBlooming.FlowerSpotCafe, listener.handleBloomingAddedEvent, flowerSpotCafeRepository.findBy, firstVoteChecker.isFirstBloomedVoteOfYear, bloomedNotificationService.sendBloomedNotificationForRegion.pida-core/core-domain/src/main/kotlin/com/pida/category/strategy/detail/CafeCategoryItemDetailReadStrategy.kt (1)
35-58:⚠️ Potential issue | 🟠 MajorCafe detail now drops
flowerSpotId, but API response still exposes it.
MapCategoryItemResponse(pida-core/core-api/src/main/kotlin/com/pida/presentation/v2/category/response/item/MapCategoryItemResponse.kt, Lines 86-120) still serializesflowerSpotId = mapCategoryItem.flowerSpotId. With this strategy returning aMapCategoryItemwithoutflowerSpotId, clients can receive unexpectednullfor cafe items.
Either map a transitional value here or remove/update that API contract in the same change set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/main/kotlin/com/pida/category/strategy/detail/CafeCategoryItemDetailReadStrategy.kt` around lines 35 - 58, MapCategoryItem constructed in CafeCategoryItemDetailReadStrategy (returning MapCategoryItem in MapCategoryItemDetail) does not set flowerSpotId while MapCategoryItemResponse still serializes mapCategoryItem.flowerSpotId, causing nulls for cafe items; fix by either adding a flowerSpotId field when instantiating MapCategoryItem in CafeCategoryItemDetailReadStrategy (use cafe.flowerSpotId or a defined transitional fallback) or update MapCategoryItemResponse to stop serializing flowerSpotId (remove or mark deprecated) so the API contract and MapCategoryItem align.pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingEntity.kt (1)
14-25:⚠️ Potential issue | 🟠 MajorAdd an index for
flower_spot_cafe_idbefore shipping this.This PR introduces cafe-scoped blooming reads, but
t_bloomingstill only indexesflower_spot_id. On a grown table the new cafe lookups will fall back to scans and undercut the latency goal of this change. At minimum, mirror the existing spot indexes forflower_spot_cafe_id.Suggested index additions
`@Table`( name = "t_blooming", indexes = [ Index(name = "idx_blooming_flower_spot_id_created_at", columnList = "flower_spot_id,created_at"), Index(name = "idx_blooming_flower_spot_id_status_created_at", columnList = "flower_spot_id,status,created_at"), + Index(name = "idx_blooming_flower_spot_cafe_id_created_at", columnList = "flower_spot_cafe_id,created_at"), + Index(name = "idx_blooming_flower_spot_cafe_id_status_created_at", columnList = "flower_spot_cafe_id,status,created_at"), ], )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingEntity.kt` around lines 14 - 25, Add matching indexes for flower_spot_cafe_id to the t_blooming table so cafe-scoped queries use indexes; inside the `@Table`(indexes = [...]) on BloomingEntity, add two new Index entries mirroring the existing idx_blooming_flower_spot_id_created_at and idx_blooming_flower_spot_id_status_created_at but for flower_spot_cafe_id (suggested names: idx_blooming_flower_spot_cafe_id_created_at and idx_blooming_flower_spot_cafe_id_status_created_at) with columnList "flower_spot_cafe_id,created_at" and "flower_spot_cafe_id,status,created_at" respectively.pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingService.kt (1)
88-106:⚠️ Potential issue | 🟡 MinorUpdate
bloomingVerifyByTargetendpoint to supportflowerSpotCafeIdparameter.The
verifyTodayBloomingservice method accepts and handlesflowerSpotCafeId(with explicit logic at line 100-101), but the API endpoint/blooming/verify/todaydoes not expose this parameter. Either addflowerSpotCafeIdas an optional@RequestParamto the controller endpoint and pass it to the service call, or remove the parameter from the service signature if cafe-scoped verification is not needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingService.kt` around lines 88 - 106, The controller endpoint handling bloomingVerifyByTarget (the /blooming/verify/today mapping) is missing the optional flowerSpotCafeId parameter while the service method verifyTodayBlooming(userId, flowerSpotId, flowerEventId, flowerSpotCafeId) expects it; either add an optional `@RequestParam`("flowerSpotCafeId") Long? flowerSpotCafeId to the controller method signature and pass it through to bloomingService.verifyTodayBlooming(...), or remove the flowerSpotCafeId parameter from BloomingService.verifyTodayBlooming and all related calls if cafe-scoped verification is not required.
🧹 Nitpick comments (4)
pida-core/core-domain/src/main/kotlin/com/pida/blooming/Blooming.kt (1)
15-16: Add a diagnostic message to the constructor invariant.Line 16 throws a bare
IllegalArgumentException; include field context to speed up debugging when invalid combinations are created.Suggested patch
- require(nonNullCount == 1) + require(nonNullCount == 1) { + "Exactly one of flowerSpotId, flowerEventId, flowerSpotCafeId must be non-null " + + "(flowerSpotId=$flowerSpotId, flowerEventId=$flowerEventId, flowerSpotCafeId=$flowerSpotCafeId)" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/main/kotlin/com/pida/blooming/Blooming.kt` around lines 15 - 16, The constructor currently enforces exactly one of flowerSpotId, flowerEventId, flowerSpotCafeId is non-null using require(nonNullCount == 1) but throws a bare IllegalArgumentException; update the invariant in Blooming (around the val nonNullCount calculation and its require) to include a diagnostic message that lists which fields are non-null and their values (or at least their presence) so the exception shows context (e.g., build a small message referencing flowerSpotId, flowerEventId, flowerSpotCafeId and pass it as the lambda message to require).pida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingServiceTest.kt (1)
47-83: Add validation tests forflowerSpotCafeIdrequest combinations.The suite currently covers only spot/event invalid combinations. With cafe-target support added, add cases for:
- cafe-only valid path
- cafe+spot invalid
- cafe+event invalid
- all-three invalid
This will guard the new input matrix from regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingServiceTest.kt` around lines 47 - 83, Add tests in BloomingServiceTest for the new flowerSpotCafeId combinations by extending the existing verifyTodayBlooming cases: add one test where only flowerSpotCafeId is provided (should not throw and should follow the valid path), and three tests where flowerSpotCafeId is combined with flowerSpotId, combined with flowerEventId, and where all three are provided (each should throw ErrorException with ErrorType.INVALID_REQUEST). Use the same setup pattern and assertions as the existing tests (mock BloomingAppender, BloomingValidator, BloomingFinder, CacheRepository and call BloomingService.verifyTodayBlooming) and name tests to mirror the current style to cover cafe-only valid, cafe+spot invalid, cafe+event invalid, and all-three invalid scenarios.pida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingFacadeTest.kt (1)
45-45: Add one facade test for the new cafe branch.Now that
Bloominghas a third target key, this file only updates the legacy spot/event fixtures. A dedicatedreadBloomingDetails(flowerSpotCafeId = …)oruploadBloomingStatus(NewBlooming.FlowerSpotCafe)case here would catch regressions in the new facade path.Also applies to: 54-54, 63-63, 118-118, 169-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingFacadeTest.kt` at line 45, Add a new unit test in BloomingFacadeTest that exercises the new cafe branch by calling the facade path that handles FlowerSpotCafe (e.g., invoke readBloomingDetails(flowerSpotCafeId = <someId>) or uploadBloomingStatus(NewBlooming.FlowerSpotCafe)) instead of passing null; update or add assertions to verify the facade returns/updates the expected Blooming details for the cafe key (use the same fixture setup used for legacy spot/event tests so the new test mirrors those cases), ensuring the new target key path in the Blooming facade is covered and will catch regressions.pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFacade.kt (1)
86-100: Consider simplifying the async pattern - no parallel work is being performed.The
coroutineScopeandasyncwrapper add overhead without benefit here. UnlikereadBloomingDetailsBySpotIdwhich parallelizes fetching bloomings and recent reporter, this method fetches bloomings first, then deriveslatestBlooming, then fetches the user profile sequentially.♻️ Simplified version without unnecessary coroutineScope
- private suspend fun readBloomingDetailsByCafeId(flowerSpotCafeId: Long): BloomingDetails = - coroutineScope { - val bloomings = bloomingService.recentlyBloomingByCafeId(flowerSpotCafeId) - val latestBlooming = bloomings.maxByOrNull { it.createdAt } - val userProfileDeferred = - async { - latestBlooming?.userId?.let { userService.getProfile(it) } - } - - return@coroutineScope buildBloomingDetails( - bloomings = bloomings, - nickname = userProfileDeferred.await()?.nickname, - updatedAt = latestBlooming?.createdAt, - ) - } + private suspend fun readBloomingDetailsByCafeId(flowerSpotCafeId: Long): BloomingDetails { + val bloomings = bloomingService.recentlyBloomingByCafeId(flowerSpotCafeId) + val latestBlooming = bloomings.maxByOrNull { it.createdAt } + val nickname = latestBlooming?.userId?.let { userService.getProfile(it) }?.nickname + + return buildBloomingDetails( + bloomings = bloomings, + nickname = nickname, + updatedAt = latestBlooming?.createdAt, + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFacade.kt` around lines 86 - 100, The readBloomingDetailsByCafeId function uses coroutineScope and async unnecessarily since work is sequential; remove coroutineScope and the async block, call bloomingService.recentlyBloomingByCafeId to get bloomings, compute latestBlooming, then directly call userService.getProfile(latestBlooming?.userId) (suspend) and pass its nickname into buildBloomingDetails (use buildBloomingDetails(bloomings = bloomings, nickname = profile?.nickname, updatedAt = latestBlooming?.createdAt)). Keep existing function name readBloomingDetailsByCafeId and other identifiers (bloomingService.recentlyBloomingByCafeId, userService.getProfile, buildBloomingDetails) so behavior and types remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingCustomRepository.kt`:
- Around line 42-56: The BloomingEntity is missing an index on flowerSpotCafeId,
causing queries like recentlyByCafeId, recentlyByCafeIds, and
findTodayCafeBloomingByUserId to trigger full table scans; add a database index
(ideally a composite index on flowerSpotCafeId and createdAt) to the
BloomingEntity JPA mapping (e.g., via `@Table`(indexes = [...]) or equivalent) so
the database can use the index for WHERE clauses filtering by flowerSpotCafeId
and createdAt and improve query performance.
---
Outside diff comments:
In `@pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingService.kt`:
- Around line 88-106: The controller endpoint handling bloomingVerifyByTarget
(the /blooming/verify/today mapping) is missing the optional flowerSpotCafeId
parameter while the service method verifyTodayBlooming(userId, flowerSpotId,
flowerEventId, flowerSpotCafeId) expects it; either add an optional
`@RequestParam`("flowerSpotCafeId") Long? flowerSpotCafeId to the controller
method signature and pass it through to
bloomingService.verifyTodayBlooming(...), or remove the flowerSpotCafeId
parameter from BloomingService.verifyTodayBlooming and all related calls if
cafe-scoped verification is not required.
In
`@pida-core/core-domain/src/main/kotlin/com/pida/category/strategy/detail/CafeCategoryItemDetailReadStrategy.kt`:
- Around line 35-58: MapCategoryItem constructed in
CafeCategoryItemDetailReadStrategy (returning MapCategoryItem in
MapCategoryItemDetail) does not set flowerSpotId while MapCategoryItemResponse
still serializes mapCategoryItem.flowerSpotId, causing nulls for cafe items; fix
by either adding a flowerSpotId field when instantiating MapCategoryItem in
CafeCategoryItemDetailReadStrategy (use cafe.flowerSpotId or a defined
transitional fallback) or update MapCategoryItemResponse to stop serializing
flowerSpotId (remove or mark deprecated) so the API contract and MapCategoryItem
align.
In
`@pida-core/core-domain/src/test/kotlin/com/pida/notification/bloomed/BloomedFirstVoteNotificationEventListenerTest.kt`:
- Around line 22-65: Add a parallel test case that exercises the
NewBlooming.FlowerSpotCafe branch: mock/stub
flowerSpotCafeRepository.findBy(...) to return a FlowerSpotCafe instance (set
region = Region.SEOUL), ensure
firstVoteChecker.isFirstBloomedVoteOfYear(Region.SEOUL) returns true and
bloomedNotificationService.sendBloomedNotificationForRegion(Region.SEOUL)
returns a value, call listener.handleBloomingAddedEvent(...) with
NewBlooming.FlowerSpotCafe (userId and flowerSpotCafeId), then verify
flowerSpotCafeRepository.findBy(...) was called and
bloomedNotificationService.sendBloomedNotificationForRegion(Region.SEOUL) was
invoked; reference: FlowerSpotCafeRepository, FlowerSpotCafe,
NewBlooming.FlowerSpotCafe, listener.handleBloomingAddedEvent,
flowerSpotCafeRepository.findBy, firstVoteChecker.isFirstBloomedVoteOfYear,
bloomedNotificationService.sendBloomedNotificationForRegion.
In
`@pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingEntity.kt`:
- Around line 14-25: Add matching indexes for flower_spot_cafe_id to the
t_blooming table so cafe-scoped queries use indexes; inside the `@Table`(indexes =
[...]) on BloomingEntity, add two new Index entries mirroring the existing
idx_blooming_flower_spot_id_created_at and
idx_blooming_flower_spot_id_status_created_at but for flower_spot_cafe_id
(suggested names: idx_blooming_flower_spot_cafe_id_created_at and
idx_blooming_flower_spot_cafe_id_status_created_at) with columnList
"flower_spot_cafe_id,created_at" and "flower_spot_cafe_id,status,created_at"
respectively.
---
Nitpick comments:
In `@pida-core/core-domain/src/main/kotlin/com/pida/blooming/Blooming.kt`:
- Around line 15-16: The constructor currently enforces exactly one of
flowerSpotId, flowerEventId, flowerSpotCafeId is non-null using
require(nonNullCount == 1) but throws a bare IllegalArgumentException; update
the invariant in Blooming (around the val nonNullCount calculation and its
require) to include a diagnostic message that lists which fields are non-null
and their values (or at least their presence) so the exception shows context
(e.g., build a small message referencing flowerSpotId, flowerEventId,
flowerSpotCafeId and pass it as the lambda message to require).
In `@pida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFacade.kt`:
- Around line 86-100: The readBloomingDetailsByCafeId function uses
coroutineScope and async unnecessarily since work is sequential; remove
coroutineScope and the async block, call
bloomingService.recentlyBloomingByCafeId to get bloomings, compute
latestBlooming, then directly call
userService.getProfile(latestBlooming?.userId) (suspend) and pass its nickname
into buildBloomingDetails (use buildBloomingDetails(bloomings = bloomings,
nickname = profile?.nickname, updatedAt = latestBlooming?.createdAt)). Keep
existing function name readBloomingDetailsByCafeId and other identifiers
(bloomingService.recentlyBloomingByCafeId, userService.getProfile,
buildBloomingDetails) so behavior and types remain unchanged.
In
`@pida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingFacadeTest.kt`:
- Line 45: Add a new unit test in BloomingFacadeTest that exercises the new cafe
branch by calling the facade path that handles FlowerSpotCafe (e.g., invoke
readBloomingDetails(flowerSpotCafeId = <someId>) or
uploadBloomingStatus(NewBlooming.FlowerSpotCafe)) instead of passing null;
update or add assertions to verify the facade returns/updates the expected
Blooming details for the cafe key (use the same fixture setup used for legacy
spot/event tests so the new test mirrors those cases), ensuring the new target
key path in the Blooming facade is covered and will catch regressions.
In
`@pida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingServiceTest.kt`:
- Around line 47-83: Add tests in BloomingServiceTest for the new
flowerSpotCafeId combinations by extending the existing verifyTodayBlooming
cases: add one test where only flowerSpotCafeId is provided (should not throw
and should follow the valid path), and three tests where flowerSpotCafeId is
combined with flowerSpotId, combined with flowerEventId, and where all three are
provided (each should throw ErrorException with ErrorType.INVALID_REQUEST). Use
the same setup pattern and assertions as the existing tests (mock
BloomingAppender, BloomingValidator, BloomingFinder, CacheRepository and call
BloomingService.verifyTodayBlooming) and name tests to mirror the current style
to cover cafe-only valid, cafe+spot invalid, cafe+event invalid, and all-three
invalid scenarios.
🪄 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: ac1796ed-e885-4554-a2dc-4441b4d172ee
📒 Files selected for processing (33)
pida-core/core-api/src/main/kotlin/com/pida/presentation/v2/category/response/detail/CafeCategoryItemDetailPayloadResponse.ktpida-core/core-api/src/main/kotlin/com/pida/presentation/v2/category/response/detail/MapCategoryItemDetailResponse.ktpida-core/core-api/src/main/kotlin/com/pida/presentation/v2/category/response/item/MapCategoryItemResponse.ktpida-core/core-api/src/main/kotlin/com/pida/presentation/v2/flowerspotcafe/request/FlowerSpotCafeCreateRequest.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/Blooming.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFacade.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingFinder.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingRepository.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/BloomingService.ktpida-core/core-domain/src/main/kotlin/com/pida/blooming/NewBlooming.ktpida-core/core-domain/src/main/kotlin/com/pida/category/strategy/detail/CafeCategoryItemDetailReadStrategy.ktpida-core/core-domain/src/main/kotlin/com/pida/category/strategy/read/CafeCategoryItemReadStrategy.ktpida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotCafe.ktpida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotCafeFinder.ktpida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotCafeRepository.ktpida-core/core-domain/src/main/kotlin/com/pida/flowerspot/NewFlowerSpotCafe.ktpida-core/core-domain/src/main/kotlin/com/pida/notification/bloomed/BloomedFirstVoteNotificationEventListener.ktpida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingFacadeTest.ktpida-core/core-domain/src/test/kotlin/com/pida/blooming/BloomingServiceTest.ktpida-core/core-domain/src/test/kotlin/com/pida/category/CafeCategoryItemDetailReadStrategyTest.ktpida-core/core-domain/src/test/kotlin/com/pida/category/CafeCategoryItemReadStrategyTest.ktpida-core/core-domain/src/test/kotlin/com/pida/category/EventCategoryItemReadStrategyTest.ktpida-core/core-domain/src/test/kotlin/com/pida/category/FlowerSpotCategoryItemReadStrategyTest.ktpida-core/core-domain/src/test/kotlin/com/pida/flowerspot/FlowerSpotFacadeTest.ktpida-core/core-domain/src/test/kotlin/com/pida/flowerspot/perf/FlowerSpotPerformanceBenchmarkRunner.ktpida-core/core-domain/src/test/kotlin/com/pida/notification/bloomed/BloomedFirstVoteNotificationEventListenerTest.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingCoreRepository.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingCustomRepository.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingEntity.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingJpaRepository.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/flowerspot/FlowerSpotCafeCoreRepository.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/flowerspot/FlowerSpotCafeEntity.ktpida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/flowerspot/FlowerSpotCafeJpaRepository.kt
💤 Files with no reviewable changes (9)
- pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotCafe.kt
- pida-core/core-api/src/main/kotlin/com/pida/presentation/v2/category/response/detail/MapCategoryItemDetailResponse.kt
- pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotCafeRepository.kt
- pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/flowerspot/FlowerSpotCafeJpaRepository.kt
- pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/flowerspot/FlowerSpotCafeCoreRepository.kt
- pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/FlowerSpotCafeFinder.kt
- pida-core/core-api/src/main/kotlin/com/pida/presentation/v2/category/response/detail/CafeCategoryItemDetailPayloadResponse.kt
- pida-core/core-api/src/main/kotlin/com/pida/presentation/v2/flowerspotcafe/request/FlowerSpotCafeCreateRequest.kt
- pida-core/core-domain/src/main/kotlin/com/pida/flowerspot/NewFlowerSpotCafe.kt
Comment on lines
+42
to
+56
| fun recentlyByCafeId(cafeId: Long): List<BloomingEntity> { | ||
| val threshold = LocalDateTime.now().minusDays(DATE_THRESHOLD) | ||
|
|
||
| val query = | ||
| jpql { | ||
| select(entity(BloomingEntity::class)) | ||
| .from(entity(BloomingEntity::class)) | ||
| .whereAnd( | ||
| path(BloomingEntity::flowerSpotCafeId).eq(cafeId), | ||
| path(BloomingEntity::createdAt).gt(threshold), | ||
| ) | ||
| } | ||
|
|
||
| return entityManager.createQuery(query, jdslRenderContext).resultList | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's an existing index on flowerSpotCafeId in migrations or entity annotations
rg -n "flowerSpotCafeId" --type=kt -C3 | rg -i "index|@Column"
# Check for Flyway/Liquibase migrations related to blooming table
fd -e sql -e xml | xargs rg -l -i "blooming" 2>/dev/null | head -5 | xargs cat 2>/dev/null | head -100Repository: Team-PIDA/Pida-Server
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for flowerSpotCafeId references with proper ripgrep syntax
rg "flowerSpotCafeId" -C3 | head -80
# Find BloomingEntity definition
fd -name "BloomingEntity*" -type f
# Find migration files related to blooming
fd -name "*.sql" -o -name "*.xml" | xargs rg -l -i "blooming" 2>/dev/nullRepository: Team-PIDA/Pida-Server
Length of output: 16174
🏁 Script executed:
#!/bin/bash
# Read BloomingEntity to check for index annotations
echo "=== BloomingEntity.kt ==="
cat -n pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingEntity.kt
# Find migration files related to blooming
echo -e "\n=== Looking for migration files ==="
find . -type f \( -name "*.sql" -o -name "*.xml" \) | head -20
# Search for blooming table migrations
echo -e "\n=== Migration files mentioning 'blooming' ==="
find . -type f \( -name "*.sql" -o -name "*.xml" \) -exec grep -l "blooming" {} \; 2>/dev/nullRepository: Team-PIDA/Pida-Server
Length of output: 1931
🏁 Script executed:
#!/bin/bash
# Search for usages of recentlyByCafeId
echo "=== Usages of recentlyByCafeId ==="
rg "recentlyByCafeId" -C3
# Check docker/init.sql for blooming table definition
echo -e "\n=== docker/init.sql content ==="
cat docker/init.sql | grep -A 20 -i "blooming" || echo "No blooming table in docker/init.sql"
# Check if there are Flyway migration directories
echo -e "\n=== Flyway migration directories ==="
find . -type d -name "db" -o -type d -name "migration*" | head -10Repository: Team-PIDA/Pida-Server
Length of output: 4053
🏁 Script executed:
#!/bin/bash
# Read the full BloomingCustomRepository to see all query methods
echo "=== BloomingCustomRepository.kt ==="
cat -n pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingCustomRepository.ktRepository: Team-PIDA/Pida-Server
Length of output: 16220
Add a database index on flowerSpotCafeId to improve query performance.
The recentlyByCafeId method (lines 42–56) queries the blooming table filtering on flowerSpotCafeId and createdAt, but the entity currently has no index on flowerSpotCafeId. Additional methods like recentlyByCafeIds and findTodayCafeBloomingByUserId filter on the same column. Without an index, these queries will result in full table scans on large datasets.
Consider adding a composite index on (flowerSpotCafeId, createdAt) to align with the existing indexing strategy for flowerSpotId.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@pida-storage/db-core/src/main/kotlin/com/pida/storage/db/core/blooming/BloomingCustomRepository.kt`
around lines 42 - 56, The BloomingEntity is missing an index on
flowerSpotCafeId, causing queries like recentlyByCafeId, recentlyByCafeIds, and
findTodayCafeBloomingByUserId to trigger full table scans; add a database index
(ideally a composite index on flowerSpotCafeId and createdAt) to the
BloomingEntity JPA mapping (e.g., via `@Table`(indexes = [...]) or equivalent) so
the database can use the index for WHERE clauses filtering by flowerSpotCafeId
and createdAt and improve query performance.
char-yb
approved these changes
Mar 27, 2026
char-yb
left a comment
char-yb
left a comment
Member
There was a problem hiding this comment.
Blooming에 cafeId 확인했고, Cache도 잘 적용했네용
LGTM
2 tasks
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
2 participants
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.
🌱 관련 이슈
📌 작업 내용 및 특이 사항
📝 참고
📌 체크 리스트
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactoring