Skip to content

Conversation

@ggamnunq
Copy link
Contributor

@ggamnunq ggamnunq commented Sep 9, 2025

명소 혼잡도 요구사항 변경에 따른 수정

변경내용

[기존] 현재시간 기준 시간대별(7개) 혼잡도 제공
⬇️
[변경] 현재 기준으로 요일별 혼잡도 & 해당 요일의 시간대별 혼잡도 정보 제공


수정사항

  • 응답값 변경 - 기준 요일, 요일별 혼잡도, 시간대별 혼잡도 추가
  • 혼잡도 endpoint 변경
  • redis에 저장할 key 변경 - 요일,시간으로 표시하도록
  • 요일, 시간으로 혼잡도 구하는 메서드 추가
  • redis key 구하는 메서드 extract

Summary by CodeRabbit

  • New Features

    • Added richer congestion data: per-day overview and a full 7×24 hourly grid, plus explicit standard day/time and real-time congestion level.
    • Updated endpoint path to /place/{placeId}/congestions for retrieving congestion data.
  • Documentation

    • Expanded API description to explain weekly/hourly congestion fields, standard day/time semantics, and real-time congestion level.
  • Breaking Changes

    • Congestion response schema changed (removed prior by-time slice; added standardDay, congestionsByDay, congestionsByTime).

  - 응답값 변경 - 기준 요일, 요일별 혼잡도, 시간대별 혼잡도 추가
  - 혼잡도 endpoint 변경
  - redis에 저장할 key 변경 - 요일,시간으로 표시하도록
  - 요일, 시간으로 혼잡도 구하는 메서드 추가
  - redis key 구하는 메서드 extract
@ggamnunq ggamnunq self-assigned this Sep 9, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 9, 2025

Walkthrough

Endpoint path changed to /place/{placeId}/congestions; OpenAPI docs expanded. PlaceCongestionDto restructured (added standardDay, congestionsByDay, congestionsByTime; removed byTimePercent). Service now builds a 7×24 congestion grid and per-day slices. Redis util adds time-based getters and centralized day/hour key generation.

Changes

Cohort / File(s) Summary
API Controller
src/main/kotlin/busanVibe/busan/domain/place/controller/PlaceCongestionController.kt
Route changed from /place/{placeId}/real-time to /place/{placeId}/congestions; OpenAPI summary/description expanded; controller method signature unchanged.
DTO Restructure
src/main/kotlin/busanVibe/busan/domain/place/dto/PlaceMapResponseDTO.kt
PlaceCongestionDto signature changed: added standardDay: Int, congestionsByDay: List<Float>, congestionsByTime: List<List<Float>>; removed byTimePercent. JSON response shape changed.
Service Logic Expansion
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt
getCongestion now computes congestionsByTime (7×24 grid), congestionsByDay (current-hour per day), standardDay/standardTime, and uses PlaceRedisUtil.getTimeCongestion overloads; removed prior 3-hour window logic and related logging.
Redis Time-Keyed API
src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt
Added getTimeCongestion(placeId: Long?) and getTimeCongestion(placeId: Long?, day: Int, hour: Int); introduced getCongestionRedisKey (day/hour-based keys); refactored get/set congestion methods and removed previous 3-hour bucketing/random generation.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant Ctrl as PlaceCongestionController
  participant Svc as PlaceCongestionQueryService
  participant RU as PlaceRedisUtil (Redis)

  Note over C,Ctrl: HTTP GET /place/{placeId}/congestions
  C->>Ctrl: Request congestion snapshot
  Ctrl->>Svc: getCongestion(placeId)
  Note over Svc: Determine standardDay/standardTime (now)

  rect rgba(200,220,255,0.18)
    Note right of Svc: Build 7×24 grid
    loop day = 0..6
      loop hour = 0..23
        Svc->>RU: getTimeCongestion(placeId, day, hour)
        RU-->>Svc: Float congestion
      end
    end
  end

  rect rgba(220,255,220,0.18)
    Note right of Svc: Per-day current-hour slice + real-time
    loop day = 0..6
      Svc->>RU: getTimeCongestion(placeId, day, currentHour)
      RU-->>Svc: Float congestion
    end
    Svc->>RU: getTimeCongestion(placeId)  <!-- real-time -->
    RU-->>Svc: Float
  end

  Svc-->>Ctrl: PlaceCongestionDto {standardDay, standardTime, realTimeCongestionLevel, congestionsByDay, congestionsByTime}
  Ctrl-->>C: 200 OK + JSON
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely states that the feature implements the ability to query place congestion information by weekday and time, which directly reflects the main change in the changeset of updating the endpoint and response to include weekday and time-based congestion data. It is specific, avoids extraneous details, and aligns with the core developer intent.
Description Check ✅ Passed The description succinctly outlines the shift from providing a seven-slot current-time congestion to delivering weekday and corresponding time-slot congestion, lists the updated response payload, endpoint change, Redis key format, and new methods, all of which map directly to the code modifications. It stays on topic and describes the actual changes without being off-topic.

Poem

I mapped the week in tidy rows,
Seven days, each hour glows.
From Redis tides the levels stream,
A grid of crowds, a humming beam.
Hopping happy — data flows. 🐇📊

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (8)
src/main/kotlin/busanVibe/busan/domain/place/dto/PlaceMapResponseDTO.kt (1)

41-47: Clarify index conventions and shapes in the DTO.

Please document in-code (KDoc or @Schema) that:

  • standardDay: 0..6 (0=Mon, 6=Sun)
  • standardTime: 0..23
  • congestionsByDay size = 7
  • congestionsByTime size = 7 and each inner list size = 24

This prevents producer/consumer mismatches.

src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (2)

60-60: TTL may not align with weekly keys.

Keys are partitioned by day-of-week and hour; a 24h TTL can expire mid-week and recreate values unpredictably. Consider 7 days to align with weekly cadence (or no TTL + periodic refresh).

-        val success = redisTemplate.opsForValue().setIfAbsent(key, congestion, Duration.ofHours(24))
+        val success = redisTemplate.opsForValue().setIfAbsent(key, congestion, Duration.ofDays(7))

75-76: Key format consistency with docs.

Docs use 0..6 (Mon..Sun). Keys currently store dayOfWeek names (e.g., MONDAY). To avoid locale/case drift and simplify ops, prefer numeric day index.

-    private fun getCongestionRedisKey(placeId: Long?, dateTime: LocalDateTime): String
-        = "place:congestion:${placeId}-${dateTime.dayOfWeek}-${dateTime.hour}"
+    private fun getCongestionRedisKey(placeId: Long?, dateTime: LocalDateTime): String =
+        "place:congestion:$placeId:${dateTime.dayOfWeek.value - 1}:${dateTime.hour}"

If you have existing data, confirm migration impact before changing key format.

src/main/kotlin/busanVibe/busan/domain/place/controller/PlaceCongestionController.kt (1)

53-69: Ensure day index semantics in docs match service/util.

Docs state 0:월..6:일. After fixing PlaceRedisUtil, confirm PlaceCongestionQueryService builds arrays with the same 0-based mapping.

Also consider renaming the handler to reflect aggregated data (not only “real-time”):

-    fun placeRealTimeCongestion(
+    fun placeCongestions(
         @PathVariable("placeId") placeId: Long): ApiResponse<PlaceMapResponseDTO.PlaceCongestionDto>{
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt (4)

126-136: Pre-size structures; assign by index (less GC, clearer mapping).

Avoid repeated list growth and boxing; index 0..6 -> Mon..Sun explicitly.

-        val congestionsByDay: MutableList<Float> = mutableListOf() // 요일별
-        val congestionsByTime: MutableList<List<Float>> = mutableListOf() // 시간대별
+        val congestionsByDay = MutableList(7) { 0f } // 0:월 ~ 6:일
+        val congestionsByTime = MutableList(7) { MutableList(24) { 0f } } // [dayIdx][hour]
-        for( day in 1..7){
-            val congestions = mutableListOf<Float>() // 각 요일의 혼잡도 정보 담을 List
-            for( hour in 0..23){
-                congestions.add(placeRedisUtil.getTimeCongestion(placeId, day, hour))
-            }
-            congestionsByTime.add(congestions)
-        }
+        for (day in 1..7) {
+            for (hour in 0..23) {
+                congestionsByTime[day - 1][hour] = placeRedisUtil.getTimeCongestion(placeId, day, hour)
+            }
+        }

130-136: Batch Redis reads (pipelining/MGET) to cut ~176 round trips.

Building the 7×24 grid with per-key GETs is chatty. Add a util that pipelines or MGETs keys.

-        for( day in 1..7){
-            val congestions = mutableListOf<Float>() // 각 요일의 혼잡도 정보 담을 List
-            for( hour in 0..23){
-                congestions.add(placeRedisUtil.getTimeCongestion(placeId, day, hour))
-            }
-            congestionsByTime.add(congestions)
-        }
+        congestionsByTime.addAll(placeRedisUtil.getTimeCongestions(placeId)) // returns List<List<Float>> (7×24) via pipeline

If you prefer keeping logic here, consider RedisTemplate.executePipelined to fetch the 168 keys at once.


139-141: Also batch the “current-hour per day” fetch.

Seven sequential GETs; mirror the above with a day-at-hour pipeline.

-        for( day in 1..7){
-            congestionsByDay.add(placeRedisUtil.getTimeCongestion(placeId, day, currentDateTime.hour))
-        }
+        congestionsByDay.addAll(
+            placeRedisUtil.getDayCongestionsAtHour(placeId, currentDateTime.hour) // 7 values via pipeline
+        )

147-147: Harden real-time value parsing and bounds.

If getTimeCongestion(placeId) isn’t guaranteed Int, toInt() can throw. Prefer a typed return or safe parsing with clamping.

-            realTimeCongestionLevel = placeRedisUtil.getTimeCongestion(placeId).toInt(),
+            realTimeCongestionLevel = when (val v = placeRedisUtil.getTimeCongestion(placeId)) {
+                is Int -> v
+                is Float -> v.toInt().coerceAtLeast(0)
+                is String -> v.toIntOrNull()?.coerceAtLeast(0) ?: 0
+                else -> 0
+            },

Confirm the expected scale (e.g., 0–3 or 0–100) and apply coerceIn accordingly.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7200d25 and a77c0aa.

📒 Files selected for processing (4)
  • src/main/kotlin/busanVibe/busan/domain/place/controller/PlaceCongestionController.kt (1 hunks)
  • src/main/kotlin/busanVibe/busan/domain/place/dto/PlaceMapResponseDTO.kt (1 hunks)
  • src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt (1 hunks)
  • src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (2)
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceQueryService.kt (1)
  • placeRedisUtil (72-72)
src/main/kotlin/busanVibe/busan/domain/place/converter/PlaceDetailsConverter.kt (1)
  • redisUtil (11-42)

Comment on lines +123 to +124
val currentDateTime = LocalDateTime.now()

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix timezone source: use KST or inject Clock.

LocalDateTime.now() depends on server default TZ; day/hour can drift if infra runs in UTC. Use Asia/Seoul or inject Clock for testability.

-        val currentDateTime = LocalDateTime.now()
+        val currentDateTime = LocalDateTime.now(java.time.ZoneId.of("Asia/Seoul"))

Alternative (preferred for tests):

// ctor
class PlaceCongestionQueryService(..., private val clock: java.time.Clock) { ... }

// usage
val currentDateTime = LocalDateTime.now(clock)
🤖 Prompt for AI Agents
In
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt
around lines 123-124, replace the direct call to LocalDateTime.now() (which
relies on server default TZ) with a testable, timezone-correct source: inject a
java.time.Clock into the service constructor and call LocalDateTime.now(clock);
register a Clock bean (e.g., Clock.system(ZoneId.of("Asia/Seoul"))) in your
Spring/Koin config for production and use Clock.fixed(...) in tests so
timestamps are consistently in KST and deterministic for testing.

Comment on lines +145 to +149
standardDay = currentDateTime.dayOfWeek.value-1, // 원래 1:월~7:일인데, 0:월~6일로 변경해서 반환
standardTime = currentDateTime.hour, // 0~23
realTimeCongestionLevel = placeRedisUtil.getTimeCongestion(placeId).toInt(),
congestionsByDay = congestionsByDay,
congestionsByTime = congestionsByTime
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Clarify day indexing contract and align all arrays.

standardDay is 0=월..6=일. Ensure congestionsByDay[standardDay] and congestionsByTime[standardDay][standardTime] follow the same zero-based mapping; verify PlaceRedisUtil(day, hour) expects 1..7 or 0..6 to avoid off-by-one.

Run to confirm signatures and usages:


🏁 Script executed:

#!/bin/bash
# Verify PlaceRedisUtil day/hour semantics and DTO usage
rg -nP -C2 'getTimeCongestion\s*\(.*day\s*:\s*Int.*hour\s*:\s*Int' --type=kt
rg -nP -C2 'getTimeCongestion\s*\(.*placeId' --type=kt
rg -nP -C2 'PlaceCongestionDto' --type=kt

Length of output: 80


🏁 Script executed:

#!/bin/bash
# Locate PlaceRedisUtil implementation and inspect getTimeCongestion signature
rg -l 'class PlaceRedisUtil' -g '*.kt'
rg -n 'class PlaceRedisUtil' -g '*.kt' -C3
rg -n 'fun getTimeCongestion' -g '*.kt' -C3

# Locate PlaceCongestionDto and its fields
rg -l 'data class PlaceCongestionDto' -g '*.kt'
rg -n 'PlaceCongestionDto' -g '*.kt' -C3

Length of output: 6165


🏁 Script executed:

#!/bin/bash
sed -n '1,80p' src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt

Length of output: 2445


🏁 Script executed:

#!/bin/bash
rg -n 'congestionsByDay' -g '*.kt'
rg -n 'congestionsByTime' -g '*.kt'

Length of output: 1174


Refactor getTimeCongestion(day,hour) to use true weekday semantics
The getTimeCongestion(placeId, day, hour) overload treats its day: Int (1=Monday…7=Sunday) as a day-of-month when calling LocalDateTime.of(year, month, day, hour, …), so the resulting dateTime.dayOfWeek may not match the requested weekday. Replace this with logic that sets the date to the desired weekday (for example, using LocalDate.now().with(TemporalAdjusters.nextOrSame(DayOfWeek.of(day))).atTime(hour, dateTime.minute)) so that both congestionsByDay[standardDay] and congestionsByTime[standardDay][standardTime] pull from the correct Redis key.
PlaceRedisUtil.kt:24–32

Comment on lines 22 to 36
// [요일, 시간] 으로 혼잡도 구하는 메서드
// day: 1~7
fun getTimeCongestion(placeId: Long?, day: Int, hour: Int): Float{
val now = LocalDateTime.now()
return getTimeCongestion(
placeId,
LocalDateTime.of(
now.year,
now.month,
day,
hour,
now.minute
)
)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Bug: day parameter is treated as day-of-month, not day-of-week.

LocalDateTime.of(now.year, now.month, day, ...) interprets day as day-of-month and will not map 0..6 (Mon..Sun). It can also throw for invalid days. Use day-of-week indexing (0=Mon..6=Sun) and validate ranges.

Apply:

-    // [요일, 시간] 으로 혼잡도 구하는 메서드
-    // day: 1~7
-    fun getTimeCongestion(placeId: Long?, day: Int, hour: Int): Float{
-        val now = LocalDateTime.now()
-        return getTimeCongestion(
-            placeId,
-            LocalDateTime.of(
-                now.year,
-                now.month,
-                day,
-                hour,
-                now.minute
-            )
-        )
-    }
+    // [요일(0=월..6=일), 시간] 으로 혼잡도 구하는 메서드
+    fun getTimeCongestion(placeId: Long?, day: Int, hour: Int): Float {
+        require(day in 0..6) { "day must be 0..6 (0=Mon, 6=Sun)" }
+        require(hour in 0..23) { "hour must be 0..23" }
+        val now = LocalDateTime.now()
+        val target = now
+            .with(java.time.temporal.ChronoField.DAY_OF_WEEK, (day + 1).toLong()) // ISO: Mon=1..Sun=7
+            .withHour(hour).withMinute(0).withSecond(0).withNano(0)
+        return getTimeCongestion(placeId, target)
+    }

Run a quick grep to confirm all call sites pass day in 0..6:


🏁 Script executed:

#!/bin/bash
rg -nP -C2 'getTimeCongestion\(\s*[^,]+,\s*[^,]+,\s*[^)]+\)' --type=kotlin

Length of output: 1854


Fix getTimeCongestion to use day‐of‐week (1=Mon..7=Sun) instead of day‐of‐month
Calls currently pass day in 1..7 (day-of-week), but LocalDateTime.of(…, day, …) treats it as day-of-month. Replace with ISO day-of-week logic:

-    // [요일, 시간] 으로 혼잡도 구하는 메서드
-    // day: 1~7
-    fun getTimeCongestion(placeId: Long?, day: Int, hour: Int): Float {
-        val now = LocalDateTime.now()
-        return getTimeCongestion(
-            placeId,
-            LocalDateTime.of(
-                now.year,
-                now.month,
-                day,
-                hour,
-                now.minute
-            )
-        )
-    }
+    /** 
+     * [요일(1=Mon..7=Sun), 시간]으로 혼잡도 구하는 메서드
+     * @param placeId 조회 대상 장소 ID
+     * @param dayOfWeek 1..7 (1=월요일, 7=일요일)
+     * @param hour 0..23
+     */
+    fun getTimeCongestion(placeId: Long?, dayOfWeek: Int, hour: Int): Float {
+        require(dayOfWeek in 1..7) { "dayOfWeek must be 1..7 (1=Mon, 7=Sun)" }
+        require(hour in 0..23)    { "hour must be 0..23" }
+        val now = LocalDateTime.now()
+        val target = now
+            .with(ChronoField.DAY_OF_WEEK, dayOfWeek.toLong())
+            .withHour(hour)
+            .withMinute(0)
+            .withSecond(0)
+            .withNano(0)
+        return getTimeCongestion(placeId, target)
+    }

— Update call sites remain passing 1..7 and need no changes.

📝 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.

Suggested change
// [요일, 시간] 으로 혼잡도 구하는 메서드
// day: 1~7
fun getTimeCongestion(placeId: Long?, day: Int, hour: Int): Float{
val now = LocalDateTime.now()
return getTimeCongestion(
placeId,
LocalDateTime.of(
now.year,
now.month,
day,
hour,
now.minute
)
)
}
/**
* [요일(1=Mon..7=Sun), 시간]으로 혼잡도 구하는 메서드
* @param placeId 조회 대상 장소 ID
* @param dayOfWeek 1..7 (1=월요일, 7=일요일)
* @param hour 0..23
*/
fun getTimeCongestion(placeId: Long?, dayOfWeek: Int, hour: Int): Float {
require(dayOfWeek in 1..7) { "dayOfWeek must be 1..7 (1=Mon, 7=Sun)" }
require(hour in 0..23) { "hour must be 0..23" }
val now = LocalDateTime.now()
val target = now
.with(ChronoField.DAY_OF_WEEK, dayOfWeek.toLong())
.withHour(hour)
.withMinute(0)
.withSecond(0)
.withNano(0)
return getTimeCongestion(placeId, target)
}

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (1)

22-40: Bug: day is treated as day-of-month, not ISO day-of-week.

This returns wrong dates and can silently mis-key Redis. Use ChronoField.DAY_OF_WEEK with validation; also zero minutes/seconds to match hour-keying.

+import java.time.temporal.ChronoField
@@
-    * @param dayOfWeek 1..7 (1=월요일, 7=일요일)
+    * @param dayOfWeek 1..7 (1=월요일, 7=일요일)
@@
-    fun getTimeCongestion(placeId: Long?, day: Int, hour: Int): Float{
-        val now = LocalDateTime.now()
-        return getTimeCongestion(
-            placeId,
-            LocalDateTime.of(
-                now.year,
-                now.month,
-                day,
-                hour,
-                now.minute
-            )
-        )
-    }
+    fun getTimeCongestion(placeId: Long?, dayOfWeek: Int, hour: Int): Float {
+        require(dayOfWeek in 1..7) { "dayOfWeek must be 1..7 (1=Mon, 7=Sun)" }
+        require(hour in 0..23)     { "hour must be 0..23" }
+        val now = LocalDateTime.now()
+        val target = now
+            .with(ChronoField.DAY_OF_WEEK, dayOfWeek.toLong())
+            .withHour(hour).withMinute(0).withSecond(0).withNano(0)
+        return getTimeCongestion(placeId, target)
+    }
🧹 Nitpick comments (5)
src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (5)

16-21: Avoid server timezone drift: inject Clock/ZoneId instead of using system time directly.

Using LocalDateTime.now() binds behavior to the server’s default timezone. Expose a Clock (or ZoneId) with a sensible default (e.g., Asia/Seoul) for deterministic behavior across environments.

+import java.time.Clock
@@
-    fun getTimeCongestion(placeId: Long?):Float{
-        return getTimeCongestion(placeId, LocalDateTime.now())
-    }
+    fun getTimeCongestion(placeId: Long?, clock: Clock = Clock.systemDefaultZone()): Float {
+        return getTimeCongestion(placeId, LocalDateTime.now(clock))
+    }

45-59: Minor: avoid parameter shadowing; fix typos in comments.

The local val dateTime shadows the parameter and the comments have small typos. Also, nice job removing the unsafe Int→Float cast; the null-safe parsing looks good.

-        // DateTime
-        val dateTime = dateTime ?: LocalDateTime.now()
+        // DateTime
+        val effectiveDateTime = dateTime ?: LocalDateTime.now()
@@
-        val key = getCongestionRedisKey(placeId, dateTime)
+        val key = getCongestionRedisKey(placeId, effectiveDateTime)
@@
-        val parsed = value?.toFloatOrNull()
+        val parsed = value?.toFloatOrNull()
-        // 값이 존재하교 유효하다면
-        if (parsed != null)return parsed // 그대로 반환
+        // 값이 존재하고 유효하면
+        if (parsed != null) return parsed // 그대로 반환
-        // 값이 없다면
-        setPlaceTimeCongestion(placeId, dateTime) // 새로 만들어서 할당
+        // 값이 없거나 파싱에 실패했다면 생성 후 재조회
+        setPlaceTimeCongestion(placeId, effectiveDateTime) // 새로 만들어서 할당
@@
-        return newValue ?: 0f // 반환, 값이 이상하다면 0 반환
+        return newValue ?: 0f // 값이 없으면 0 반환

66-66: TTL likely too short for weekly keys; prefer 7 days.

Keys encode day-of-week and hour, so a 24h TTL causes daily churn without reuse on next week’s same slot. Consider 7 days for cache efficiency.

-        val success = redisTemplate.opsForValue().setIfAbsent(key, congestion, Duration.ofHours(24))
+        val success = redisTemplate.opsForValue().setIfAbsent(key, congestion, Duration.ofDays(7))

76-78: Prefer kotlin.random for testability and consistency.

Switch to kotlin.random.Random with DI-friendly parameter for deterministic tests.

-    private fun getRandomCongestion(): Float = (Math.random() * 4 + 1).toFloat()
+    private fun getRandomCongestion(random: kotlin.random.Random = kotlin.random.Random.Default): Float =
+        random.nextFloat() * 4f + 1f

80-83: Key normalization: use numeric ISO day-of-week, guard null placeId, and use colon separators.

Aligns with docs (1..7), avoids “MONDAY” strings, and prevents accidental "null" keys.

-    private fun getCongestionRedisKey(placeId: Long?, dateTime: LocalDateTime): String
-        = "place:congestion:${placeId}-${dateTime.dayOfWeek}-${dateTime.hour}"
+    private fun getCongestionRedisKey(placeId: Long?, dateTime: LocalDateTime): String {
+        requireNotNull(placeId) { "placeId must not be null" }
+        return "place:congestion:$placeId:${dateTime.dayOfWeek.value}:${dateTime.hour}"
+    }

If you adopt this, please confirm no consumers rely on the old “MONDAY/TUESDAY” key format and plan a one-time cache invalidation to avoid misses.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a77c0aa and e31b1ed.

📒 Files selected for processing (1)
  • src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/kotlin/busanVibe/busan/domain/place/util/PlaceRedisUtil.kt (3)
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt (3)
  • readOnly (119-144)
  • placeRepository (23-176)
  • PlaceMapResponseDTO (66-75)
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceQueryService.kt (1)
  • placeRedisUtil (72-72)
src/main/kotlin/busanVibe/busan/domain/place/converter/PlaceDetailsConverter.kt (1)
  • redisUtil (11-42)

@ggamnunq ggamnunq merged commit 03fbc1f into main Sep 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants