Conversation
개요공간 정보(Geospatial) 기능을 지원하기 위한 의존성 추가 및 근처 스포츠 시설 검색 기능을 구현하는 새로운 REST 엔드포인트, 서비스, 저장소, 엔터티, DTO, 매퍼를 도입합니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant Controller as SportsFacilityController
participant Service as SportsFacilityService
participant Repository as SportsFacilityRepository
participant DB as PostgreSQL<br/>(PostGIS)
Client->>Controller: GET /api/facilities/near?lat=37.5&lng=127.0&radiusM=3000&limit=50&type=BASKETBALL
activate Controller
Controller->>Service: getNear(37.5, 127.0, 3000, 50, "BASKETBALL")
deactivate Controller
activate Service
Service->>Service: 정규화(반경: 1k-20k, 제한: 1-200)
Service->>Repository: findNear(37.5, 127.0, 3000, 50, "BASKETBALL")
deactivate Service
activate Repository
Repository->>DB: ST_DWithin(), ST_Distance()<br/>네이티브 SQL 쿼리 실행
deactivate Repository
activate DB
DB-->>Repository: FacilityNearProjection 목록 반환
deactivate DB
activate Repository
Repository-->>Service: List<FacilityNearProjection>
deactivate Repository
activate Service
Service->>Service: FacilityMapper.toNearResponse() 사용<br/>FacilityNearResponse로 매핑
Service-->>Controller: List<FacilityNearResponse>
deactivate Service
activate Controller
Controller-->>Client: HTTP 200 OK + JSON 응답
deactivate Controller
예상 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~25분 시
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/main/java/com/be/sportizebe/domain/facility/entity/SportsFacility.java`:
- Around line 43-45: The changeLocation(Point location) method assigns a value
to the non-nullable field location without validation; add a null check at the
start of changeLocation (or use Objects.requireNonNull) and throw a clear
exception (e.g., IllegalArgumentException or NullPointerException) if location
is null so callers receive an immediate, descriptive error instead of a DB
constraint failure when JPA flushes.
In `@src/main/java/com/be/sportizebe/domain/facility/mapper/FacilityMapper.java`:
- Around line 8-17: Handle potential null from
FacilityNearProjection.getDistanceM() in toNearResponse: replace the direct
Math.round(p.getDistanceM()) call with a null-safe conditional so that
FacilityNearResponse.builder().distanceM(...) receives null when getDistanceM()
is null, otherwise cast the rounded double to int; update the toNearResponse
method to use p.getDistanceM() == null ? null : (int)
Math.round(p.getDistanceM()) (or equivalent) to avoid NPE.
In
`@src/main/java/com/be/sportizebe/domain/facility/repository/SportsFacilityRepository.java`:
- Line 29: The query in SportsFacilityRepository uses facility_type = :type
which is case-sensitive; ensure the incoming type parameter is normalized to
uppercase before reaching the repository (e.g., call type = type.toUpperCase()
in the controller method that accepts the request or in the service method that
delegates to SportsFacilityRepository), or validate/convert the incoming enum
string and reject invalid values; update the controller/service method that
calls the repository (the request handler that forwards the type param) to
perform the uppercase conversion so the repository comparison matches the
EnumType.STRING values like SOCCER/BASKETBALL.
- Around line 12-32: The SportsFacility entity is missing a spatial GIST index
on the location column, causing ST_DWithin queries (used in
SportsFacilityRepository) to do full table scans; add a table-level index
annotation to the SportsFacility entity such as declaring `@Table`(... indexes = {
`@Index`(name = "idx_sports_facilities_location", columnList = "location",
columnDefinition = "USING GIST") }) so the location column gets a PostGIS GIST
index, and also ensure the corresponding DB migration (CREATE INDEX CONCURRENTLY
... USING GIST (location)) is applied in your migration scripts if you use
schema migrations.
🧹 Nitpick comments (3)
src/main/java/com/be/sportizebe/domain/facility/entity/FacilityType.java (1)
3-10: 기존 스포츠 타입 enum들과의 관계 검토 필요코드베이스에 유사한 목적의 enum들이 있습니다:
SportType: SOCCER, BASKETBALLPostProperty: SOCCER, BASKETBALL, FREEFacilityType: BASKETBALL, SOCCER, BADMINTON, TENNIS, BOWLING, ETC의도적인 설계라면 괜찮지만, 향후 유지보수를 위해 이들 enum 간의 관계를 문서화하거나 통합을 고려해 보세요.
src/main/java/com/be/sportizebe/domain/facility/controller/SportsFacilityController.java (1)
17-27: 입력 값 검증 추가 권장
lat과lng파라미터에 대한 범위 검증이 없습니다. 유효하지 않은 좌표값이 DB 쿼리로 전달될 수 있습니다.또한,
type파라미터를String대신FacilityTypeenum으로 받으면 유효하지 않은 타입 입력을 방지할 수 있습니다.♻️ 검증 추가 제안
+import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import com.be.sportizebe.domain.facility.entity.FacilityType; +import org.springframework.validation.annotation.Validated; `@RestController` `@RequiredArgsConstructor` `@RequestMapping`("/api/facilities") +@Validated public class SportsFacilityController { private final SportsFacilityService sportsFacilityService; `@GetMapping`("/near") public List<FacilityNearResponse> near( - `@RequestParam` double lat, - `@RequestParam` double lng, + `@RequestParam` `@Min`(-90) `@Max`(90) double lat, + `@RequestParam` `@Min`(-180) `@Max`(180) double lng, `@RequestParam`(defaultValue = "3000") int radiusM, `@RequestParam`(defaultValue = "50") int limit, - `@RequestParam`(required = false) String type + `@RequestParam`(required = false) FacilityType type ) { - return sportsFacilityService.getNear(lat, lng, radiusM, limit, type); + return sportsFacilityService.getNear(lat, lng, radiusM, limit, type != null ? type.name() : null); } }src/main/java/com/be/sportizebe/domain/facility/service/SportsFacilityService.java (1)
20-30: 좌표 유효성 검증 누락
radiusM과limit에 대한 경계값 검증은 잘 구현되어 있습니다. 하지만lat과lng파라미터에 대한 유효성 검증이 없습니다. 유효하지 않은 좌표값(예: lat > 90, lng > 180)이 전달되면 PostGIS 쿼리에서 예상치 못한 결과나 오류가 발생할 수 있습니다.♻️ 좌표 검증 추가 제안
public List<FacilityNearResponse> getNear( double lat, double lng, long radiusM, int limit, String type ) { + if (lat < -90 || lat > 90 || lng < -180 || lng > 180) { + throw new IllegalArgumentException("Invalid coordinates: lat must be between -90 and 90, lng must be between -180 and 180"); + } if (radiusM <= 0) radiusM = 1000; if (radiusM > 20000) radiusM = 20000; if (limit <= 0) limit = 50; if (limit > 200) limit = 200;
| public void changeLocation(Point location) { | ||
| this.location = location; | ||
| } |
There was a problem hiding this comment.
changeLocation 메서드에 null 검증 누락
location 컬럼은 nullable = false로 정의되어 있지만(line 32), changeLocation 메서드는 null 체크 없이 값을 할당합니다. null이 전달되면 JPA flush 시점에 DB 제약 조건 위반 예외가 발생합니다.
🐛 null 검증 추가 제안
public void changeLocation(Point location) {
+ if (location == null) {
+ throw new IllegalArgumentException("Location cannot be null");
+ }
this.location = location;
}📝 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.
| public void changeLocation(Point location) { | |
| this.location = location; | |
| } | |
| public void changeLocation(Point location) { | |
| if (location == null) { | |
| throw new IllegalArgumentException("Location cannot be null"); | |
| } | |
| this.location = location; | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/be/sportizebe/domain/facility/entity/SportsFacility.java`
around lines 43 - 45, The changeLocation(Point location) method assigns a value
to the non-nullable field location without validation; add a null check at the
start of changeLocation (or use Objects.requireNonNull) and throw a clear
exception (e.g., IllegalArgumentException or NullPointerException) if location
is null so callers receive an immediate, descriptive error instead of a DB
constraint failure when JPA flushes.
| public static FacilityNearResponse toNearResponse(FacilityNearProjection p){ | ||
| return FacilityNearResponse.builder() | ||
| .id(p.getId()) | ||
| .facilityName(p.getFacilityName()) | ||
| .introduce(p.getIntroduce()) | ||
| .thumbnailUrl(p.getThumbnailUrl()) | ||
| .facilityType(p.getFacilityType()) | ||
| .distanceM((int) Math.round(p.getDistanceM())) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
distanceM가 null일 경우 NPE 발생 가능성
FacilityNearProjection.getDistanceM()이 Double 타입을 반환하므로 null이 될 수 있습니다. Math.round(null)은 NPE를 발생시킵니다.
🐛 null 처리 추가 제안
public static FacilityNearResponse toNearResponse(FacilityNearProjection p){
return FacilityNearResponse.builder()
.id(p.getId())
.facilityName(p.getFacilityName())
.introduce(p.getIntroduce())
.thumbnailUrl(p.getThumbnailUrl())
.facilityType(p.getFacilityType())
- .distanceM((int) Math.round(p.getDistanceM()))
+ .distanceM(p.getDistanceM() != null ? (int) Math.round(p.getDistanceM()) : 0)
.build();
}📝 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.
| public static FacilityNearResponse toNearResponse(FacilityNearProjection p){ | |
| return FacilityNearResponse.builder() | |
| .id(p.getId()) | |
| .facilityName(p.getFacilityName()) | |
| .introduce(p.getIntroduce()) | |
| .thumbnailUrl(p.getThumbnailUrl()) | |
| .facilityType(p.getFacilityType()) | |
| .distanceM((int) Math.round(p.getDistanceM())) | |
| .build(); | |
| } | |
| public static FacilityNearResponse toNearResponse(FacilityNearProjection p){ | |
| return FacilityNearResponse.builder() | |
| .id(p.getId()) | |
| .facilityName(p.getFacilityName()) | |
| .introduce(p.getIntroduce()) | |
| .thumbnailUrl(p.getThumbnailUrl()) | |
| .facilityType(p.getFacilityType()) | |
| .distanceM(p.getDistanceM() != null ? (int) Math.round(p.getDistanceM()) : 0) | |
| .build(); | |
| } |
🤖 Prompt for AI Agents
In `@src/main/java/com/be/sportizebe/domain/facility/mapper/FacilityMapper.java`
around lines 8 - 17, Handle potential null from
FacilityNearProjection.getDistanceM() in toNearResponse: replace the direct
Math.round(p.getDistanceM()) call with a null-safe conditional so that
FacilityNearResponse.builder().distanceM(...) receives null when getDistanceM()
is null, otherwise cast the rounded double to int; update the toNearResponse
method to use p.getDistanceM() == null ? null : (int)
Math.round(p.getDistanceM()) (or equivalent) to avoid NPE.
| @Query(value = """ | ||
| SELECT | ||
| sf.id AS id, | ||
| sf.facility_name AS facilityName, | ||
| sf.introduce AS introduce, | ||
| sf.thumbnail_url AS thumbnailUrl, | ||
| sf.facility_type AS facilityType, | ||
| ST_Distance( | ||
| sf.location, | ||
| ST_SetSRID(ST_MakePoint(:lng, :lat), 4326)::geography | ||
| ) AS distanceM | ||
| FROM sports_facilities sf | ||
| WHERE ST_DWithin( | ||
| sf.location, | ||
| ST_SetSRID(ST_MakePoint(:lng, :lat), 4326)::geography, | ||
| :radiusM | ||
| ) | ||
| AND (:type IS NULL OR sf.facility_type = :type) | ||
| ORDER BY distanceM | ||
| LIMIT :limit | ||
| """, nativeQuery = true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for spatial index on sports_facilities table location column
echo "=== Searching for GIST index creation ==="
fd -e sql | xargs grep -i "create.*index.*gist" 2>/dev/null | head -20
echo ""
echo "=== Searching for index on location column ==="
fd -e sql | xargs grep -i "location" 2>/dev/null | grep -i "index" | head -20
echo ""
echo "=== Searching for sports_facilities table migration ==="
fd -e sql | xargs grep -i "sports_facilities" 2>/dev/null
echo ""
echo "=== Checking Java migration files ==="
fd -e java | xargs grep -l -i "sports_facilities\|location.*index" 2>/dev/nullRepository: Sportize/be
Length of output: 404
🏁 Script executed:
#!/bin/bash
# Check for different migration approaches
echo "=== Looking for migration directories ==="
find . -type d \( -name migration -o -name migrations -o -name flyway -o -name db \) 2>/dev/null
echo ""
echo "=== Checking for Flyway V*.sql files ==="
find . -name "V*.sql" -o -name "V*.sql" 2>/dev/null
echo ""
echo "=== Checking for Liquibase files ==="
find . -name "*changelog*" -o -name "*.xml" -path "*/db/*" 2>/dev/null | head -20
echo ""
echo "=== Checking SportsFacility entity for index annotations ==="
cat src/main/java/com/be/sportizebe/domain/facility/entity/SportsFacility.java | head -100
echo ""
echo "=== Checking application properties for migration ==="
find . -name "application*.properties" -o -name "application*.yml" 2>/dev/null | xargs grep -i "flyway\|liquibase" 2>/dev/nullRepository: Sportize/be
Length of output: 1743
🏁 Script executed:
#!/bin/bash
# Check for index creation and schema initialization
echo "=== Searching for `@Index` annotations ==="
rg "@Index" --type java
echo ""
echo "=== Searching for schema.sql or database.sql ==="
find . -name "schema.sql" -o -name "database.sql" -o -name "init.sql" 2>/dev/null
echo ""
echo "=== Checking for Hibernate index creation in entity files ==="
rg "indexes\s*=|@Index" --type java -A 3
echo ""
echo "=== Checking all Java entity files for index configuration ==="
find . -path "*/entity/*.java" -type f | xargs grep -l "Index\|index" 2>/dev/nullRepository: Sportize/be
Length of output: 1022
🏁 Script executed:
#!/bin/bash
# Final verification - check SportsFacility entity for any index definitions we might have missed
echo "=== Full SportsFacility entity check ==="
cat src/main/java/com/be/sportizebe/domain/facility/entity/SportsFacility.java | grep -A 2 -B 2 "Table\|Index\|location"Repository: Sportize/be
Length of output: 525
공간 인덱스 생성 필수입니다.
PostGIS 쿼리 구현은 정확합니다. ST_MakePoint(:lng, :lat) 순서와 geography 타입 캐스팅이 올바르게 적용되어 있습니다.
그러나 location 컬럼에 공간 인덱스(GIST)가 존재하지 않습니다. ST_DWithin 쿼리는 인덱스 없이 전체 행을 스캔하게 되어 데이터가 증가하면 심각한 성능 저하를 초래합니다. 프로젝트에서 다른 엔티티(ChatMessage)에서 @Index 애너테이션을 사용하고 있으므로, SportsFacility 엔티티의 location 컬럼에도 공간 인덱스를 추가해야 합니다.
`@Table`(name = "sports_facilities", indexes = {
`@Index`(name = "idx_sports_facilities_location", columnList = "location", columnDefinition = "USING GIST")
})
🤖 Prompt for AI Agents
In
`@src/main/java/com/be/sportizebe/domain/facility/repository/SportsFacilityRepository.java`
around lines 12 - 32, The SportsFacility entity is missing a spatial GIST index
on the location column, causing ST_DWithin queries (used in
SportsFacilityRepository) to do full table scans; add a table-level index
annotation to the SportsFacility entity such as declaring `@Table`(... indexes = {
`@Index`(name = "idx_sports_facilities_location", columnList = "location",
columnDefinition = "USING GIST") }) so the location column gets a PostGIS GIST
index, and also ensure the corresponding DB migration (CREATE INDEX CONCURRENTLY
... USING GIST (location)) is applied in your migration scripts if you use
schema migrations.
| ST_SetSRID(ST_MakePoint(:lng, :lat), 4326)::geography, | ||
| :radiusM | ||
| ) | ||
| AND (:type IS NULL OR sf.facility_type = :type) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find FacilityType enum definition
echo "=== Finding FacilityType enum ==="
fd "FacilityType.java" --exec cat {}
echo -e "\n=== Checking facility type enum storage ==="
find . -type f -name "*.java" | xargs grep -l "FacilityType" | head -5
echo -e "\n=== Checking repository query handling ==="
fd "SportsFacilityRepository.java" --exec cat {}Repository: Sportize/be
Length of output: 2017
🏁 Script executed:
#!/bin/bash
# Search for calls to findNear method
echo "=== Searching for findNear method calls ==="
rg "findNear" --type java -B 2 -A 5
# Check SportsFacility entity and facility_type field
echo -e "\n=== Checking SportsFacility entity ==="
fd "SportsFacility.java" --exec cat {}
# Check for controller handling of type parameter
echo -e "\n=== Searching for facility type parameter in controllers ==="
rg "@RequestParam|@Param|type.*String" --type java --glob "*Controller.java" -B 3 -A 3Repository: Sportize/be
Length of output: 16947
클라이언트에서 전달하는 type 값은 반드시 대문자(예: SOCCER)로 변환해야 함
현재 컨트롤러에서 type 파라미터를 그대로 리포지토리에 전달하는데, 데이터베이스는 @Enumerated(EnumType.STRING) 설정으로 인해 대문자 enum 값(SOCCER, BASKETBALL 등)만 저장되어 있습니다. SQL의 facility_type = :type 비교는 대소문자를 구분하므로, 클라이언트가 소문자나 혼합 케이스로 입력하면 쿼리 결과가 없을 것입니다. 서비스 또는 컨트롤러에서 type.toUpperCase() 처리를 추가하거나, API 문서에서 대문자만 허용되는 것을 명시하세요.
🤖 Prompt for AI Agents
In
`@src/main/java/com/be/sportizebe/domain/facility/repository/SportsFacilityRepository.java`
at line 29, The query in SportsFacilityRepository uses facility_type = :type
which is case-sensitive; ensure the incoming type parameter is normalized to
uppercase before reaching the repository (e.g., call type = type.toUpperCase()
in the controller method that accepts the request or in the service method that
delegates to SportsFacilityRepository), or validate/convert the incoming enum
string and reject invalid values; update the controller/service method that
calls the repository (the request handler that forwards the type param) to
perform the uppercase conversion so the repository comparison matches the
EnumType.STRING values like SOCCER/BASKETBALL.
#️⃣ Issue Number
📝 요약(Summary)
🛠️ PR 유형
어떤 변경 사항이 있나요?
📸스크린샷 (선택)
💬 공유사항 to 리뷰어
✅ PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.