Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package busanVibe.busan.domain.place.controller

import busanVibe.busan.domain.place.dto.PlaceMapResponseDTO
import busanVibe.busan.domain.place.enums.PlaceType
import busanVibe.busan.domain.place.service.PlaceCongestionQueryService
import busanVibe.busan.global.apiPayload.exception.ApiResponse
import io.swagger.v3.oas.annotations.Operation
import org.springframework.web.bind.annotation.GetMapping
Expand All @@ -12,30 +13,35 @@ import org.springframework.web.bind.annotation.RestController

@RestController
@RequestMapping("/api/congestion")
class PlaceCongestionController {
class PlaceCongestionController (
private val placeCongestionQueryService: PlaceCongestionQueryService,
){

@GetMapping
@Operation(summary = "지도 조회")
fun map(
@RequestParam("type", required = false) type: PlaceType,
@RequestParam("type", required = false, defaultValue = "ALL") type: PlaceType,
@RequestParam("latitude")latitude: Double,
@RequestParam("longtitude")longtitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>?{
@RequestParam("longitude")longtitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>{
Comment on lines +25 to +27
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix parameter name typo

There's a typo in the parameter name: longtitude should be longitude.

-        @RequestParam("longitude")longtitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>{
+        @RequestParam("longitude")longitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>{

Also update the service call:

-        val places = placeCongestionQueryService.getMap(type, latitude, longtitude)
+        val places = placeCongestionQueryService.getMap(type, latitude, longitude)
📝 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
@RequestParam("type", required = false, defaultValue = "ALL") type: PlaceType,
@RequestParam("latitude")latitude: Double,
@RequestParam("longtitude")longtitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>?{
@RequestParam("longitude")longtitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>{
@RequestParam("type", required = false, defaultValue = "ALL") type: PlaceType,
@RequestParam("latitude")latitude: Double,
@RequestParam("longitude")longitude: Double): ApiResponse<PlaceMapResponseDTO.MapListDto>{
val places = placeCongestionQueryService.getMap(type, latitude, longitude)
//
}
🤖 Prompt for AI Agents
In
src/main/kotlin/busanVibe/busan/domain/place/controller/PlaceCongestionController.kt
around lines 23 to 25, fix the typo in the parameter name by renaming
`longtitude` to `longitude`. Also update any corresponding service calls or
references to use the corrected parameter name `longitude` to ensure consistency
and avoid errors.


return null;
val places = placeCongestionQueryService.getMap(type, latitude, longtitude)
return ApiResponse.onSuccess(places);
}

@GetMapping("/place/{placeId}")
@Operation(summary = "명소 기본 정보 조회")
fun placeDefaultInfo(@PathVariable("placeId") placeId: Long): ApiResponse<PlaceMapResponseDTO.PlaceDefaultInfoDto>?{
return null;
fun placeDefaultInfo(@PathVariable("placeId") placeId: Long): ApiResponse<PlaceMapResponseDTO.PlaceDefaultInfoDto>{

val place = placeCongestionQueryService.getPlaceDefault(placeId)
return ApiResponse.onSuccess(place)
}

@GetMapping("/place/{placeId}/read-time")
@GetMapping("/place/{placeId}/real-time")
@Operation(summary = "명소 실시간 혼잡도 조회")
fun placeRealTimeCongestion(
@PathVariable("placeId") placeId: Long,
@RequestParam("standard-time") standardTime: Integer): ApiResponse<PlaceMapResponseDTO.PlaceCongestionDto>?{
return null;
@PathVariable("placeId") placeId: Long): ApiResponse<PlaceMapResponseDTO.PlaceCongestionDto>{
val congestion = placeCongestionQueryService.getCongestion(placeId)
return ApiResponse.onSuccess(congestion)
}

@GetMapping("/place/{placeId}/distribution")
Expand Down
5 changes: 3 additions & 2 deletions src/main/kotlin/busanVibe/busan/domain/place/domain/Place.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package busanVibe.busan.domain.place.domain

import busanVibe.busan.domain.common.BaseEntity
import busanVibe.busan.domain.place.enums.PlaceType
import busanVibe.busan.domain.review.domain.Review
import jakarta.persistence.Column
import jakarta.persistence.Entity
import jakarta.persistence.EnumType
Expand Down Expand Up @@ -46,8 +47,8 @@ class Place(
// @JoinColumn(name = "region_id", nullable = false)
// val region: Region,
//
// @OneToMany(mappedBy = "place", fetch = FetchType.LAZY)
// val reviews: List<Review> = emptyList(),
@OneToMany(mappedBy = "place", fetch = FetchType.LAZY)
val reviews: List<Review>,

@OneToMany(mappedBy = "place", fetch = FetchType.LAZY)
val placeLikes: Set<PlaceLike>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package busanVibe.busan.domain.place.dto
import com.fasterxml.jackson.annotation.JsonProperty
import com.fasterxml.jackson.databind.PropertyNamingStrategies
import com.fasterxml.jackson.databind.annotation.JsonNaming
import java.math.BigDecimal


class PlaceMapResponseDTO {
Expand All @@ -14,23 +15,23 @@ class PlaceMapResponseDTO {

@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy::class)
data class PlaceMapInfoDto(
val placeId: Long,
val placeId: Long?,
val name: String,
val type: String,
val congestionLevel: Integer,
val latitude: Double,
val longitude: Double
val congestionLevel: Int,
val latitude: BigDecimal,
val longitude: BigDecimal
)

@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy::class)
data class PlaceDefaultInfoDto(
val id: Long,
val id: Long?,
val name: String,
val congestionLevel: Integer,
val congestionLevel: Int,
val grade: Float,
val reviewAmount: Integer,
val latitude: Double,
val longitude: Double,
val reviewAmount: Int,
val latitude: BigDecimal,
val longitude: BigDecimal,
val address: String,
@get:JsonProperty("is_open")
val isOpen: Boolean,
Expand All @@ -39,8 +40,8 @@ class PlaceMapResponseDTO {

@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy::class)
data class PlaceCongestionDto(
val standardTime: String,
val realTimeCongestionLevel: Integer,
val standardTime: Int,
val realTimeCongestionLevel: Int,
val byTimePercent: List<Float>
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import busanVibe.busan.domain.place.enums.PlaceType
import org.springframework.data.jpa.repository.JpaRepository
import org.springframework.data.jpa.repository.Query
import org.springframework.data.repository.query.Param
import java.math.BigDecimal

interface PlaceRepository: JpaRepository<Place, Long> {

Expand All @@ -28,4 +29,32 @@ interface PlaceRepository: JpaRepository<Place, Long> {
)
fun findByIdWithLIkeAndImage(@Param("placeId") placeId: Long): Place?

// ALL 이면 검사 안 함
@Query(
"""
SELECT p FROM Place p
LEFT JOIN FETCH p.openTime
WHERE p.latitude BETWEEN :minLat AND :maxLat
AND p.longitude BETWEEN :minLng AND :maxLng
AND (:#{#type.name() == 'ALL'} = true OR p.type = :type)
"""
)
fun findPlacesByLocationAndType(
@Param("minLat") minLat: BigDecimal,
@Param("maxLat") maxLat: BigDecimal,
@Param("minLng") minLng: BigDecimal,
@Param("maxLng") maxLng: BigDecimal,
@Param("type") type: PlaceType?
): List<Place>

@Query(
"""
SELECT p FROM Place p
LEFT JOIN FETCH p.openTime
LEFT JOIN FETCH p.placeImages
WHERE p.id = :placeId
"""
)
fun findByIdWithReviewAndImage(@Param("placeId") placeId: Long): Place?

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package busanVibe.busan.domain.place.service

import busanVibe.busan.domain.place.domain.Place
import busanVibe.busan.domain.place.domain.PlaceImage
import busanVibe.busan.domain.place.dto.PlaceMapResponseDTO
import busanVibe.busan.domain.place.enums.PlaceType
import busanVibe.busan.domain.place.repository.PlaceRepository
import busanVibe.busan.domain.review.domain.Review
import busanVibe.busan.domain.review.domain.repository.ReviewRepository
import busanVibe.busan.global.apiPayload.code.status.ErrorStatus
import busanVibe.busan.global.apiPayload.exception.handler.ExceptionHandler
import org.slf4j.LoggerFactory
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional
import java.math.BigDecimal
import java.math.RoundingMode
import java.time.LocalDateTime

@Service
class PlaceCongestionQueryService(
private val placeRepository: PlaceRepository,
private val placeRedisUtil: PlaceRedisUtil,
private val reviewRepository: ReviewRepository,
) {

private val latitudeRange: Double = 0.05
private val longitudeRange: Double = 0.05

private val log = LoggerFactory.getLogger(PlaceCongestionQueryService::class.java)

@Transactional(readOnly = true)
fun getMap(type: PlaceType?, latitude: Double, longitude: Double): PlaceMapResponseDTO.MapListDto{

// Place -> name, type, latitude, longitude
// Redis -> congestion level

// Place 목록 조회
val placeList = placeRepository.findPlacesByLocationAndType(
BigDecimal(latitude - latitudeRange).setScale(6, RoundingMode.HALF_UP),
BigDecimal(latitude + latitudeRange).setScale(6, RoundingMode.HALF_UP),
BigDecimal(longitude - longitudeRange).setScale(6, RoundingMode.HALF_UP),
BigDecimal(longitude + longitudeRange).setScale(6, RoundingMode.HALF_UP),
type
)

// DTO 변환
val placeDtoList :List<PlaceMapResponseDTO.PlaceMapInfoDto> = placeList.map {
PlaceMapResponseDTO.PlaceMapInfoDto(
placeId = it.id,
name = it.name,
type = it.type.capitalEnglish,
latitude = it.latitude,
longitude = it.longitude,
congestionLevel = placeRedisUtil.getRedisCongestion(it.id)
)
}
Comment on lines +47 to +56
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Potential N+1 performance issue with Redis calls

The current implementation makes individual Redis calls for each place when fetching congestion levels. For large result sets, this could impact performance.

Consider implementing a batch fetch method in PlaceRedisUtil to retrieve congestion data for multiple places in a single operation:

// In PlaceRedisUtil
fun getRedisCongestionBatch(placeIds: List<Long>): Map<Long, Int> {
    val keys = placeIds.map { "place:congestion:$it" }
    val values = redisTemplate.opsForValue().multiGet(keys)
    return placeIds.zip(values ?: emptyList()).associate { (id, value) ->
        id to (value?.toIntOrNull() ?: generateAndStoreCongestion(id))
    }
}

Then update the mapping:

+        val congestionMap = placeRedisUtil.getRedisCongestionBatch(placeList.map { it.id })
         val placeDtoList :List<PlaceMapResponseDTO.PlaceMapInfoDto> = placeList.map {
             PlaceMapResponseDTO.PlaceMapInfoDto(
                 placeId = it.id,
                 name = it.name,
                 type = it.type.capitalEnglish,
                 latitude = it.latitude,
                 longitude = it.longitude,
-                congestionLevel = placeRedisUtil.getRedisCongestion(it.id)
+                congestionLevel = congestionMap[it.id] ?: 0
             )
         }
🤖 Prompt for AI Agents
In
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt
between lines 47 and 56, the code currently fetches congestion levels from Redis
one-by-one for each place, causing a potential N+1 performance issue. To fix
this, implement a batch fetch method in PlaceRedisUtil that takes a list of
place IDs and retrieves all congestion data in a single Redis multiGet call.
Then, update the placeDtoList mapping to first collect all place IDs, call this
batch method once, and map the congestion levels from the returned map instead
of calling Redis individually for each place.


return PlaceMapResponseDTO.MapListDto(placeDtoList)
}

@Transactional(readOnly = true)
fun getPlaceDefault(placeId: Long): PlaceMapResponseDTO.PlaceDefaultInfoDto{

// Place -> name, imageList, address, openTime
// Review -> grade, count
// Image -> list
// Redis -> congestion

// 명소 조회
val place: Place? = placeRepository.findByIdWithReviewAndImage(placeId)
place?: throw ExceptionHandler(ErrorStatus.PLACE_NOT_FOUND)

// 이미지 조회
val placeImageSet: Set<PlaceImage> = place.placeImages
val placeImageList = placeImageSet.toList()
.sortedBy { it.createdAt }
.map { it.imgUrl }

// 리뷰 조회
val reviewSet: Set<Review> = reviewRepository.findByPlace(place).toSet()
val grade = reviewSet.map { it.score }.average().toFloat()


return PlaceMapResponseDTO.PlaceDefaultInfoDto(
id = place.id,
name = place.name,
congestionLevel = placeRedisUtil.getRedisCongestion(place.id),
grade = grade,
reviewAmount = reviewSet.size,
latitude = place.latitude,
longitude = place.longitude,
address = place.address,
isOpen = true,
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Hardcoded isOpen value doesn't reflect actual status

The isOpen field is hardcoded to true, which doesn't represent the actual open/closed status of the place. The comment mentions "openTime" but it's not being used.

Consider implementing actual open/closed logic based on the place's open times:

isOpen = determineIfOpen(place.openTimes, LocalDateTime.now())
🤖 Prompt for AI Agents
In
src/main/kotlin/busanVibe/busan/domain/place/service/PlaceCongestionQueryService.kt
at line 93, the isOpen field is currently hardcoded to true, which does not
reflect the actual open or closed status of the place. To fix this, replace the
hardcoded true with a call to a function that determines if the place is open
based on its openTimes and the current time, such as isOpen =
determineIfOpen(place.openTimes, LocalDateTime.now()). Ensure the
determineIfOpen function correctly evaluates the openTimes to return the
accurate open status.

imgList = placeImageList
)
}

@Transactional(readOnly = true)
fun getCongestion(placeId: Long): PlaceMapResponseDTO.PlaceCongestionDto {

val current = LocalDateTime.now()
log.info("현재 시간: ${current}")

val roundedBase = (current.hour / 3) * 3

// 최근 6개 3시간 단위 시간 생성 (기준시간 포함 총 7개)
val hours = (6 downTo 0).map { i -> (roundedBase - i * 3 + 24) % 24 }

log.info("배열 담기 시작")

val byTimePercent: List<Float> = hours.map { hour ->
val adjustedDateTime = current.withHour(hour)
.withMinute(0).withSecond(0).withNano(0)
.let {
if (hour > current.hour) it.minusDays(1) else it
}
placeRedisUtil.getTimeCongestion(placeId, adjustedDateTime)
}

log.info("배열 담기 끝")

return PlaceMapResponseDTO.PlaceCongestionDto(
standardTime = roundedBase,
realTimeCongestionLevel = placeRedisUtil.getTimeCongestion(placeId, current).toInt(),
byTimePercent = byTimePercent
)
}



}
Original file line number Diff line number Diff line change
@@ -1,21 +1,67 @@
package busanVibe.busan.domain.place.service

import org.slf4j.LoggerFactory
import org.springframework.data.redis.core.StringRedisTemplate
import org.springframework.stereotype.Component
import java.time.Duration
import java.time.LocalDateTime


@Component
class PlaceRedisUtil(
private val redisTemplate: StringRedisTemplate
) {

private val log = LoggerFactory.getLogger("busanVibe.busan.domain.place")

// 임의로 혼잡도 생성하여 반환. 레디스 키 값으로 저장함.
fun getRedisCongestion(placeId: Long?): Int{

val key = "place:congestion:$placeId"
val randomCongestion: Int = (Math.random() * 5 + 1).toInt()
val randomCongestion = getRandomCongestion().toInt().toString()

redisTemplate.opsForValue()
.set(key, randomCongestion.toString())
.set(key, randomCongestion)

return Integer.parseInt(randomCongestion)
}
Comment on lines 18 to +27
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Method always overwrites existing congestion values

The current implementation always generates a new random value and overwrites any existing congestion data. This doesn't preserve historical data and makes the congestion values inconsistent across calls.

Consider checking if a value exists before overwriting:

     fun getRedisCongestion(placeId: Long?): Int{
         val key = "place:congestion:$placeId"
-        val randomCongestion = getRandomCongestion().toInt().toString()
-
-        redisTemplate.opsForValue()
-            .set(key, randomCongestion)
-
-        return Integer.parseInt(randomCongestion)
+        
+        val existingValue = redisTemplate.opsForValue().get(key)
+        if (existingValue != null) {
+            return existingValue.toIntOrNull() ?: 0
+        }
+        
+        val randomCongestion = getRandomCongestion().toInt()
+        redisTemplate.opsForValue().set(key, randomCongestion.toString(), Duration.ofHours(1))
+        return randomCongestion
     }
🤖 Prompt for AI Agents
In src/main/kotlin/busanVibe/busan/domain/place/service/PlaceRedisUtil.kt
between lines 18 and 27, the getRedisCongestion method always overwrites
existing congestion values with a new random value, losing any previously stored
data. Modify the method to first check if a congestion value already exists in
Redis for the given placeId; if it does, return that existing value instead of
generating and setting a new one. Only generate and set a new random congestion
value if no existing value is found.


// 시간 혼잡도 설정
fun setPlaceTimeCongestion(placeId: Long, dateTime: LocalDateTime) {
val roundedHour = (dateTime.hour / 3) * 3
val key = "place:congestion:$placeId-${dateTime.year}-${dateTime.monthValue}-${dateTime.dayOfMonth}-$roundedHour"
val congestion = getRandomCongestion().toString()
val success = redisTemplate.opsForValue().setIfAbsent(key, congestion, Duration.ofHours(24))

if (success == true) {
log.info("혼잡도 기록 저장 완료: $key, 저장된 혼잡도: $congestion")
} else {
val existing = redisTemplate.opsForValue().get(key)
log.info("이미 존재하는 혼잡도 기록: $key, 기존 혼잡도: $existing")
}
}

// 지정 시간 혼잡도 조회
fun getTimeCongestion(placeId: Long, dateTime: LocalDateTime): Float {
val roundedHour = (dateTime.hour / 3) * 3
val key = "place:congestion:$placeId-${dateTime.year}-${dateTime.monthValue}-${dateTime.dayOfMonth}-$roundedHour"

val value = redisTemplate.opsForValue().get(key)

return randomCongestion
return if (value != null) {
log.info("이미 존재하는 혼잡도 기록: $key, 기존 혼잡도: $value")
value.toFloatOrNull() ?: 0f
} else {
setPlaceTimeCongestion(placeId, dateTime.withHour(roundedHour))
val newValue = redisTemplate.opsForValue().get(key)
newValue?.toFloatOrNull() ?: 0f
}
}

// 혼잡도 생성 (1.0 ~ 5.0 사이의 Float)
private fun getRandomCongestion(): Float {
return (Math.random() * 4 + 1).toFloat()
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package busanVibe.busan.domain.review.domain.repository

import busanVibe.busan.domain.place.domain.Place
import busanVibe.busan.domain.review.domain.Review
import org.springframework.data.jpa.repository.EntityGraph
import org.springframework.data.jpa.repository.JpaRepository
import org.springframework.data.jpa.repository.Query
import org.springframework.data.repository.query.Param
Expand All @@ -17,4 +18,7 @@ interface ReviewRepository: JpaRepository<Review, Long> {
)
fun findForDetails(@Param("place")place: Place): List<Review>

@EntityGraph(attributePaths = ["user"])
fun findByPlace(place: Place): List<Review>

}