[FEAT] 지역 조회 / 검색 API 구현#23
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough지역 조회/검색 API를 신설하고 Region 도메인 모델의 패키지 변경 및 Lombok 생성자/게터를 적용했습니다. 레포지토리, 쿼리/검색 서비스, 컨트롤러, 요청/응답 DTO, 교차필드 검증기, 오류 코드 및 Swagger 응답 설명이 추가/수정되었고, FoodTruckServiceArea의 Region import 경로가 갱신되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as RegionController
participant Facade as RegionService
participant Query as RegionQueryService
participant Repo as RegionRepository
participant DB as Database
Client->>Controller: GET /regions?depth=&parentCode=
Controller->>Facade: getRegions(request)
Facade->>Query: getRegions(request)
alt request.depth >= 2
Query->>Repo: existsByRegionCodeAndDepth(parentCode, depth-1)
Repo-->>Query: false
Query-->>Facade: throw EntityNotFound(PARENT_REGION_NOT_FOUND)
Facade-->>Controller: 예외 전파
Controller-->>Client: 404 Error
else parent exists or depth<2
Query->>Repo: findRegions(depth, parentCode)
Repo->>DB: JPQL 조회
DB-->>Repo: List<Region>
Repo-->>Query: List<Region>
Query-->>Facade: List<RegionResponse>
Facade-->>Controller: List<RegionResponse>
Controller-->>Client: 200 BaseResponse(List)
end
sequenceDiagram
autonumber
actor Client
participant Controller as RegionController
participant Facade as RegionService
participant Search as RegionSearchService
participant Repo as RegionRepository
participant DB as Database
Client->>Controller: GET /regions/search?keyword=
Controller->>Facade: searchRegions(request)
Facade->>Search: searchRegions(request)
Search->>Repo: searchSubRegionsByFullName(trim(keyword))
Repo->>DB: JPQL LIKE 조회 (fullName 부분일치 + depth 필터)
DB-->>Repo: List<Region>
Repo-->>Search: List<Region>
Search-->>Facade: List<RegionResponse>
Facade-->>Controller: List<RegionResponse>
Controller-->>Client: 200 BaseResponse(List)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (17)
src/main/java/konkuk/chacall/domain/region/presentation/dto/response/RegionResponse.java (2)
5-11: 응답 의미 혼동 방지: name ↔ fullName 구분조회 API는
name, 검색 API는fullName을 같은 필드(name)로 내려 혼동 소지가 큽니다. DTO 분리(예:RegionListResponse,RegionSearchResponse) 또는fullName필드를 추가해 명시적으로 구분하는 방안을 권장합니다.
9-10: 스키마 문구 보정“행정동 코드”는 depth=1/2에도 쓰이므로 “지역 코드”로 표현하는 것이 정확합니다.
적용 diff:
- @Schema(description = "지역 행정동 코드", example = "11") + @Schema(description = "지역 코드", example = "11")src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java (2)
3-3: 불필요한 import 제거
jakarta.xml.bind.annotation.XmlType미사용입니다. 삭제하세요.적용 diff:
-import jakarta.xml.bind.annotation.XmlType;
112-119: GET_REGIONS 응답 코드 보완서비스에서 사용한다면
REGION_NOT_FOUND도 포함하는 편이 명세 일관성에 좋습니다.적용 diff:
- GET_REGIONS(new LinkedHashSet<>(Set.of( - PARENT_REGION_NOT_FOUND - ))), + GET_REGIONS(new LinkedHashSet<>(Set.of( + PARENT_REGION_NOT_FOUND, + REGION_NOT_FOUND + ))),src/main/java/konkuk/chacall/domain/region/domain/model/Region.java (2)
31-32: 정수 컬럼에 length 속성 불필요
Integer depth에@Column(length = 1)은 의미 없습니다(문자열 전용). 제거 권장.적용 diff:
- @Column(nullable = false, length = 1) + @Column(nullable = false) private Integer depth; // 1: 시/도, 2: 구/군, 3: 동/읍/면
12-12: 조회 패턴 대비 인덱스 추가 제안
parentCode기준 리스트 조회가 빈번하다면 인덱스가 유효합니다. (데이터가 작더라도 미래 대비)적용 diff:
-@Table(name = "regions") +@Table( + name = "regions", + indexes = { + @Index(name = "idx_regions_parent_code", columnList = "parentCode"), + @Index(name = "idx_regions_parent_depth", columnList = "parentCode,depth") + } +)src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionSearchRequest.java (1)
6-10: 입력 길이 제한 추가리소스 보호를 위해 키워드 길이 상한을 두세요(예: 50).
적용 diff:
import io.swagger.v3.oas.annotations.media.Schema; import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; public record RegionSearchRequest( @Schema(description = "지역명 검색 키워드(부분일치, fullName 기준)", example = "광", requiredMode = Schema.RequiredMode.REQUIRED) @NotBlank(message = "keyword 는 필수입니다.") + @Size(max = 50, message = "keyword 는 최대 50자까지 허용됩니다.") String keyword ) { }src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionQueryRequest.java (1)
16-17: parentCode 값 제약 보강유효 시 양수 제약을 추가해 방어적 프로그래밍을 강화하세요.
적용 diff:
import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Positive; import konkuk.chacall.domain.region.presentation.validator.ValidRegionQuery; ... @Schema(description = "부모 행정동 코드 (depth=2/3일 때 필수)", example = "11", requiredMode = Schema.RequiredMode.NOT_REQUIRED) - Long parentCode + @Positive(message = "parentCode 는 양수여야 합니다.") + Long parentCodesrc/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java (3)
4-6: @query 파라미터 바인딩을 @Param으로 명시하세요.컴파일 옵션에 따라 파라미터 이름 추론이 실패하면 런타임 오류가 날 수 있습니다. 안전하게
@Param을 붙이는 것을 권장합니다.import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; ... - List<Region> findRegions(Integer depth, Long parentCode); + List<Region> findRegions(@Param("depth") Integer depth, @Param("parentCode") Long parentCode);Also applies to: 18-18
22-22: 검색을 대소문자 무시로 확장하는 것을 제안합니다.키워드 검색은 보통 대소문자 구분이 필요 없습니다. 메서드를
...ContainingIgnoreCase...로 바꾸면 실사용성이 좋아집니다. (서비스 호출부도 함께 변경 필요)- List<Region> findByFullNameContainingOrderByRegionIdAsc(String keyword); + List<Region> findByFullNameContainingIgnoreCaseOrderByRegionIdAsc(String keyword);
11-18: 인덱스 권고: (depth, parent_code), LOWER(full_name).조회/검색 경로가 잦다면 DB에 다음 인덱스를 고려해 주세요.
- B-Tree: (depth, parent_code)
- 함수 인덱스: LOWER(full_name) (IgnoreCase 검색 채택 시)
Also applies to: 20-22
src/main/java/konkuk/chacall/domain/region/presentation/validator/ValidRegionQueryValidator.java (3)
10-18: NPE 방지: null 입력 시 조기 반환.Bean Validation 규약상 값이 null일 수 있으므로 NPE를 막기 위해 가드 추가를 권장합니다.
@Override public boolean isValid(RegionQueryRequest value, ConstraintValidatorContext ctx) { + if (value == null) { + return true; // 다른 단일 필드 제약(@NotNull 등)에 맡깁니다. + } Integer depth = value.depth(); Long parentCode = value.parentCode();
5-5: 미사용 import 제거.
RegionRepositoryimport가 사용되지 않습니다.-import konkuk.chacall.domain.region.domain.repository.RegionRepository;
21-25: depth 범위 검증도 여기서 처리할지 확정해주세요.현재 다중 필드 관계만 검증합니다. depth 허용 범위(예: 1~3) 단일 필드 제약이 DTO에 없다면 본 밸리데이터에서 함께 막는 것도 방법입니다.
필요 시 제가 DTO에
@Min(1) @Max(3)패치 제안 드리겠습니다.src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java (1)
20-23: 서비스 레이어에서도 parentCode null 방어를 얇게 두세요.프레젠테이션 레이어 검증이 우회될 수 있으니,
depth >= 2일 때parentCode가 null이면 바로 실패하도록 얇은 가드를 둬서 오동작을 막는 것을 권장합니다.public List<RegionResponse> getRegions(RegionQueryRequest request) { - if (request.depth() >= 2 && !regionRepository.existsByRegionCodeAndDepth(request.parentCode(), request.depth() - 1)) { + if (request.depth() >= 2 && request.parentCode() == null) { + throw new IllegalArgumentException("parentCode is required when depth >= 2"); + } + if (request.depth() >= 2 && !regionRepository.existsByRegionCodeAndDepth(request.parentCode(), request.depth() - 1)) { throw new EntityNotFoundException(ErrorCode.PARENT_REGION_NOT_FOUND); }src/main/java/konkuk/chacall/domain/region/presentation/RegionController.java (1)
14-14: 미사용 로깅 제거로 깔끔하게.
@Slf4j와 해당 import가 사용되지 않습니다.import lombok.RequiredArgsConstructor; -import lombok.extern.slf4j.Slf4j; ... -@Slf4j public class RegionController {Also applies to: 24-24
src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java (1)
18-23: 유니코드 공백 처리 및 대소문자 무시 검색으로 개선 제안.
trim()대신strip()사용(유니코드 공백), Repository는 IgnoreCase 파생 메서드로 맞추면 UX가 좋아집니다.- List<Region> regions = regionRepository.findByFullNameContainingOrderByRegionIdAsc(request.keyword().trim()); + List<Region> regions = regionRepository + .findByFullNameContainingIgnoreCaseOrderByRegionIdAsc(request.keyword().strip());Repository 시그니처 변경은
RegionRepository코멘트의 패치와 함께 적용해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruckServiceArea.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/RegionService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/domain/model/Region.java(1 hunks)src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/RegionController.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionQueryRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionSearchRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/response/RegionResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/validator/ValidRegionQuery.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/validator/ValidRegionQueryValidator.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java (3)
src/main/java/konkuk/chacall/global/common/exception/EntityNotFoundException.java (1)
EntityNotFoundException(6-11)src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java (1)
RequiredArgsConstructor(12-26)src/main/java/konkuk/chacall/domain/region/application/RegionService.java (1)
Service(14-29)
src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java (2)
src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java (1)
RequiredArgsConstructor(14-31)src/main/java/konkuk/chacall/domain/region/application/RegionService.java (1)
Service(14-29)
src/main/java/konkuk/chacall/domain/region/domain/model/Region.java (1)
src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruckServiceArea.java (1)
Entity(10-28)
src/main/java/konkuk/chacall/domain/region/presentation/RegionController.java (2)
src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java (1)
RequiredArgsConstructor(14-31)src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java (1)
RequiredArgsConstructor(12-26)
src/main/java/konkuk/chacall/domain/region/application/RegionService.java (2)
src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java (1)
RequiredArgsConstructor(14-31)src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java (1)
RequiredArgsConstructor(12-26)
🔇 Additional comments (7)
src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java (1)
80-85: Region 에러 코드 추가 적절 + 사용처/스웨거 반영 확인두 상수 추가는 타 도메인과 코드 체계가 일관적입니다. 다만
REGION_NOT_FOUND의 실제 사용처가 본 PR에 없다면 제거하거나, 사용 예정이라면 Swagger(GET_REGIONS)에도 포함해주세요.src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruckServiceArea.java (1)
4-4: Region import 경로 변경 LGTM모델 패키지 이동과 일치합니다.
src/main/java/konkuk/chacall/domain/region/presentation/validator/ValidRegionQuery.java (1)
8-19: DTO 교차 필드 검증 접근 적절프레젠테이션 레이어의 요청 DTO에 타입 수준 제약을 둔 선택이 합리적입니다.
컨트롤러에서 해당 DTO 파라미터에
@Valid(또는 클래스 레벨@Validated) 적용돼 있는지 확인해주세요.src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java (1)
27-29: 응답 name vs fullName 사용 차이 의도 확인 요청.조회(get)에서는
name, 검색(search)에서는fullName을 매핑합니다. API 스펙 의도라면 OK, 아니라면 일관성 재검토 부탁드립니다.src/main/java/konkuk/chacall/domain/region/presentation/RegionController.java (1)
29-39: 컨트롤러 구성 깔끔합니다.요청 DTO에 @Valid, ParameterObject 바인딩, BaseResponse 래핑까지 일관성 좋습니다.
Also applies to: 41-51
src/main/java/konkuk/chacall/domain/region/application/RegionService.java (1)
14-28: 파사드 구성 좋아요.읽기 전용 트랜잭션에 두 서비스 위임으로 역할이 명확합니다.
src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java (1)
18-23: 검증 완료 — RegionSearchRequest.keyword에 @notblank 적용됨
src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionSearchRequest.java에서 keyword에 @notblank(message = "keyword 는 필수입니다.")가 선언되어 있어 null/공백 검증이 적용됩니다.
There was a problem hiding this comment.
처음 보는 클래스라 조금 찾아보고 리뷰하느라 시간이 조금 걸렸네요! validator를 presentation 패키지 하위에 두는 것 좋은 것 같습니다. 다만, dto 유효성 검증을 담당하는 클래스이니 dto 패키지 하위에 request와 response 패키지와 수평적으로 두는 것 어떤가요?? 정해진 것은 없으니 상균님 판단에 지금이 더 보기 편하신 것 같으면 유지해도 좋습니다!
엄청 복잡한 작업일 줄 알았는데 생각보다 엄청 빨리 올리셔서 놀랐네요.. 하핳 전반적으로 모든 코드가 깔끔해서 리뷰할 맛이 나네욥,, 수고하셨습니당~
| public class ValidRegionQueryValidator implements ConstraintValidator<ValidRegionQuery, RegionQueryRequest> { | ||
|
|
||
| @Override | ||
| public boolean isValid(RegionQueryRequest value, ConstraintValidatorContext ctx) { | ||
| Integer depth = value.depth(); | ||
| Long parentCode = value.parentCode(); | ||
|
|
||
| // depth == 1 인데 parentCode 를 보냈으면 거절 | ||
| if (depth == 1 && parentCode != null) { | ||
| addViolation(ctx, "depth = 1 인 경우 parentCode 를 보내면 안됩니다.", "parentCode"); | ||
| return false; | ||
| } | ||
|
|
||
| // depth >= 2 인데 parentCode 가 없으면 거절 | ||
| if (depth >= 2 && parentCode == null) { | ||
| addViolation(ctx, "depth 가 2 또는 3일 때 parentCode 는 필수입니다.", "parentCode"); | ||
| return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
이렇게 dto 파싱시에 유효성 검증이 가능하군요! 또 하나 알아갑니다~
p2: postman으로 직접 날려보니 depth와 parentCode가 둘다 null일 경우에 Internal Server Error가 발생하는 것 같아요!
원인을 찾아보니 현재 isValid() 메서드 내부에서 depth를 Integer로 정의하고 있는데 이를 if 조건문에서 1과 비교하는 과정에서 intValue()가 호출되어 NPE가 발생하는 것 같습니다!
RegionQueryRequest에 달려있는 NotNull 제약 조건보다 위 Validator가 먼저 동작해서 생기는 문제인 것 같네요,,
일단 지금 상황에선 맨 앞쪽에 request DTO 자체가 null인지 여부를 판단하는 조건문만 붙여주면 해결될 것 같습니다!
if (value == null) return true;
There was a problem hiding this comment.
어 위에서 제가 제시한 해결책으로 직접 실행시켜보았는데 해결이 안되네요,, 모든 필드가 null이여도 dto 객체는 만들어주나봅니다.. 그렇다면 그냥 depth가 null일 때의 조건문을 추가해주면 될 것 같습니다.
Integer depth = value.depth();
Long parentCode = value.parentCode();
if (depth == null) {
return true;
}
(뒤의 기존 조건분기)There was a problem hiding this comment.
오호 이 부분은 제가 생각했던 거랑 다른 방식으로 동작하나보네요..
제가 생각하기로는 Bean Validation -> ContraintValidator 순서로 검증이 들어갈 거라고 생각해서, Bean Validation 을 통해 detph 에 대해 @NotNull 제약을 걸어줬기 때문에 depth == null 인 케이스는 Validator 가 동작하기 이전에 먼저 검증되어서 Validator 내부에서는 depth == null 인 케이스를 고려해주지 않아도 된다고 생각했는데, 찾아보니 이 둘 간에는 기본적으로 우선순위가 존재하지 않는다는 것 같네용
현준님이 말씀하신 것처럼 Validator 내부에서도 NPE 가 발생하지 않도록 가드 로직을 추가해야할 것 같네요. 찾아주셔서 감사합니다!!
|
|
||
| boolean existsByRegionCodeAndDepth(Long regionCode, Integer depth); | ||
|
|
||
| List<Region> findByFullNameContainingOrderByRegionIdAsc(String keyword); |
There was a problem hiding this comment.
지역 검색은 추후에 정렬 순서를 조금 고려하면 좋을 것 같네요! 예를 들어, 사용자가 현재 입력한 키워드가 '당'이라면, '서울시 영등포구 당산동'이 '경기 성남시 분당동'보다 앞에 정렬되는 것을 보장한다던지..
나중에 리팩 포인트로 남겨두면 좋을 것 같습니다~ (듣기론 검색엔진에서 full text search를 사용한다고도 들었습니다)
There was a problem hiding this comment.
좋습니당
원래는 데이터 자체를 서울 -> 경기 -> 인천 순으로 삽입해서 서울이 경기보다는 앞에 위치하도록 할 생각이었는데, 이것도 가능은 하겠지만 좀 더 정렬 조건을 고려해보면 좋을 것 같네요
| if (request.depth() >= 2 && !regionRepository.existsByRegionCodeAndDepth(request.parentCode(), request.depth() - 1)) { | ||
| throw new EntityNotFoundException(ErrorCode.PARENT_REGION_NOT_FOUND); | ||
| } |
| public record RegionResponse( | ||
| @Schema(description = "지역 이름", example = "서울") | ||
| String name, | ||
|
|
||
| @Schema(description = "지역 행정동 코드", example = "11") | ||
| Long code | ||
| ) { | ||
| public static RegionResponse of(String name, Long regionCode) { | ||
| return new RegionResponse( | ||
| name, | ||
| regionCode); | ||
| } | ||
| } |
There was a problem hiding this comment.
p2: 그 지역 반환시에 parent와 같이 있는 지역 (예를 들어, '서울 전체') 같은 것은 아마 '전체'라는 키워드를 넣기로 했었던 것 같은데 깜빡하신 것 같습니다!
그 제 기억으론 DB에 중복된 지역을 넣는 것과 조회시에 '전체'를 붙이는 것 중에 후자로 결정했었던 것 같은데 맞죠..?
{
"regions": [
{ "name": "서울 전체", "code": 11 },
{ "name": "강남구", "code": 11680 },
{ "name": "강동구", "code": 11740 }
]
}There was a problem hiding this comment.
p3: 엇 그리고 추가적으로 노션 명세서를 보면 전체 데이터를 regions라는 리스트로 감싸진 형식인데 현재 구현은 그냥 List 자체를 반환하고 있어서 약간 명세가 다른 것 같아요! 확인 부탁드립니당~
There was a problem hiding this comment.
p2: 그 지역 반환시에 parent와 같이 있는 지역 (예를 들어, '서울 전체') 같은 것은 아마 '전체'라는 키워드를 넣기로 했었던 것 같은데 깜빡하신 것 같습니다!
그 제 기억으론 DB에 중복된 지역을 넣는 것과 조회시에 '전체'를 붙이는 것 중에 후자로 결정했었던 것 같은데 맞죠..?
{ "regions": [ { "name": "서울 전체", "code": 11 }, { "name": "강남구", "code": 11680 }, { "name": "강동구", "code": 11740 } ] }
원래는 '전체' 를 붙이는 것으로 결정하긴 했었는데, 그 방식은 지역 검색에 문제를 일으킬 수 있겠다는 판단이 들어서 DB에 중복된 지역을 넣는 것이 좀 더 나을 것 같다는 판단하에 이렇게 구현하였습니다!
왜냐면 지역 검색에서 서울 을 검색하면 서울 전체 가 검색이 되어야하는데, 서울 전체 가 DB 상에 존재하지 않는다면 검색 로직을 좀 복잡하게 가져가야할 것 같더라구요.
그래서 서울(depth = 1), 서울 전체(depth = 2) 모두를 DB에 저장하는 방식으로 정하고 현재 제가 구현한 방식을 그대로 가져가는 것이 좋을 것 같다고 생각했습니다!
There was a problem hiding this comment.
p3: 엇 그리고 추가적으로 노션 명세서를 보면 전체 데이터를 regions라는 리스트로 감싸진 형식인데 현재 구현은 그냥 List 자체를 반환하고 있어서 약간 명세가 다른 것 같아요! 확인 부탁드립니당~
현준님이 작성해주신 전체 지역 조회 API 명세 페이지 상에는 regions 로 감싸져있는 구조가 맞습니다!
다만 어차피 regions 내부에 List 만 존재할 뿐 추가적인 객체, 혹은 값이 존재하지 않는다면 프론트 입장에서는 불필요하게 변수 접근 depth 가 한 단계 늘어날 뿐일 것 같더라구요.
그래서 Chacall API 명세서 상에는 제가 아래와 같이 수정해서 명시해뒀습니당
There was a problem hiding this comment.
p2: 그 지역 반환시에 parent와 같이 있는 지역 (예를 들어, '서울 전체') 같은 것은 아마 '전체'라는 키워드를 넣기로 했었던 것 같은데 깜빡하신 것 같습니다!
그 제 기억으론 DB에 중복된 지역을 넣는 것과 조회시에 '전체'를 붙이는 것 중에 후자로 결정했었던 것 같은데 맞죠..?
{ "regions": [ { "name": "서울 전체", "code": 11 }, { "name": "강남구", "code": 11680 }, { "name": "강동구", "code": 11740 } ] }원래는 '전체' 를 붙이는 것으로 결정하긴 했었는데, 그 방식은 지역 검색에 문제를 일으킬 수 있겠다는 판단이 들어서 DB에 중복된 지역을 넣는 것이 좀 더 나을 것 같다는 판단하에 이렇게 구현하였습니다!
왜냐면 지역 조회에서 서울 을 검색하면
서울 전체가 검색이 되어야하는데,서울 전체가 DB 상에 존재하지 않는다면 검색 로직을 좀 복잡하게 가져가야할 것 같더라구요. 그래서 서울(depth = 1), 서울 전체(depth = 2) 모두를 DB에 저장하는 방식으로 정하고 현재 제가 구현한 방식을 그대로 가져가는 것이 좋을 것 같다고 생각했습니다!
넵 좋습니다!! 그게 더 구현 난이도가 쉬울 것 같네요!
There was a problem hiding this comment.
p3: 엇 그리고 추가적으로 노션 명세서를 보면 전체 데이터를 regions라는 리스트로 감싸진 형식인데 현재 구현은 그냥 List 자체를 반환하고 있어서 약간 명세가 다른 것 같아요! 확인 부탁드립니당~
현준님이 작성해주신 전체 지역 조회 API 명세 페이지 상에는 regions 로 감싸져있는 구조가 맞습니다!
다만 어차피 regions 내부에 List 만 존재할 뿐 추가적인 객체, 혹은 값이 존재하지 않는다면 프론트 입장에서는 불필요하게 변수 접근 depth 가 한 단계 늘어날 뿐일 것 같더라구요. 그래서 Chacall API 명세서 상에는 제가 아래와 같이 수정해서 명시해뒀습니당
아하 넵 그러면 커서가 들어가 있지 않은 리스트 response 반환은 그냥 감싸지 않고 return하는 것으로 구현 해보겠습니다~
request / response 와 수평적인 위치 좋습니다! 그 형태로 바꿔볼게용 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQueryValidator.java (2)
35-40: 여러 위반을 한 번에 보고하도록 누적 수집 고려현재는 최초 위반에서 즉시 return false 합니다. 클라이언트 UX 관점에서 모든 위반을 한 번에 알려주는 편이 낫습니다. boolean valid = true로 시작해 위반마다 addViolation만 하고 마지막에 return valid 하는 방식으로 바꿀 수 있습니다.
37-39: 메시지 i18n 키로 외부화하드코딩된 한글 메시지 대신 ValidationMessages.properties의 메시지 키를 사용하면 다국어 및 유지보수성이 좋아집니다. 예: "{region.query.depth.required}", "{region.query.parent.required}" 등.
src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQuery.java (2)
14-18: 기본 메시지는 키로 외부화 권장하드코딩 대신 메시지 키로 전환하면 일관성↑. 예: message = "{region.query.invalid}".
- String message() default "잘못된 지역 조회 요청입니다."; + String message() default "{region.query.invalid}";
9-11: 재사용/합성 고려 시 ANNOTATION_TYPE 대상 추가(Optional)향후 이 제약을 다른 합성 제약에 포함시킬 계획이 있다면 @target에 ANNOTATION_TYPE을 추가해 둬도 안전합니다.
-@Target(ElementType.TYPE) +@Target({ElementType.TYPE, ElementType.ANNOTATION_TYPE})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/konkuk/chacall/domain/region/application/RegionService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionQueryRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQuery.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQueryValidator.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/konkuk/chacall/domain/region/application/RegionService.java
- src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java
- src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionQueryRequest.java
- src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java
🔇 Additional comments (2)
src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQuery.java (1)
8-12: 어노테이션 메타 구성 적절TYPE 수준 제약과 Validator 바인딩 모두 적절합니다.
src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQueryValidator.java (1)
26-30: 서비스에서 parentCode의 depth-소속 검증을 이미 수행하므로 validator 변경 불필요RegionQueryService#getRegions에서 depth >= 2일 때 regionRepository.existsByRegionCodeAndDepth(request.parentCode(), request.depth() - 1)로 parentCode의 존재·소속을 검증하고 있습니다 (src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java:21-22). ValidRegionQueryValidator는 현재 depth>=2 시 parentCode null 체크만 하므로 서비스 레이어에서 책임지고 있어 레포지토리 주입은 필요하지 않습니다. 입력 시점 빠른 피드백이 필요하면 validator에 depth-1 기준 existsByRegionCodeAndDepth(parentCode, depth-1) 검사를 선택적으로 추가하세요.
buzz0331
left a comment
buzz0331
left a comment
There was a problem hiding this comment.
수고하셨습니다~ 한가지 질문 남겼는데 확인 부탁드릴게욥!
| @Query(""" | ||
| select r | ||
| from Region r | ||
| where r.depth >= 2 | ||
| and r.fullName like concat('%', :keyword, '%') | ||
| order by r.regionId asc | ||
| """) | ||
| List<Region> searchSubRegionsByFullName(String keyword); |
There was a problem hiding this comment.
엇 where문이 추가된 이유는 서울, 인천, 경기 같은 depth = 1의 지역은 검색이 되지 않도록 하기 위함인가요??
추가적으로 저희는 이제 '서울 광진구'와 '서울 광진구 전체'가 DB에 중복되게 존재하는데 이것도 검색시에 모두 조회되도록 할 의도 맞을까요??
There was a problem hiding this comment.
네 맞습니다! 지금 알바몬의 로직을 그대로 따라가보고 있는데, 알바몬은 depth = 1 인 지역은 검색되지 않도록 해뒀더라구요.
근데 또 확인해보니까, depth = 2 인 애들은 서울 전체, 경기 전체, 인천 전체 만 조회되도록 하고, 그 외의 서울 광진구 같은 것은 검색이 안되도록 막아뒀네요..?
이렇게 하려면 (depth == 2 and regionCode == parentCode) or depth == 3 인 지역들을 반환하면 될 것 같은데 이 부분 어떻게 생각하시나용
알바몬 따라갈까요?
There was a problem hiding this comment.
오호 좋은 것 같습니다! regionCode == parentCode 이 조건문을 사용하면 전체와 아닌 것을 구분할 수 있겠네요! 👍🏻
- 지역 조회 시 요청 값 (depth, parentCode) 검증을 validator 에서 맡도록 구현
- depth = 2 -> 서울 전체 / 경기 전체 / 인천 전체 만 조회 - depth = 3 -> 전부 조회
ksg1227
force-pushed
the
feat/#22-region-get-search
branch
from
2793920 to
e04e90f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java (1)
26-29: 검색 범위(where) 조건 합의사항 반영 확인depth=2의 ‘전체’((regionCode=parentCode))와 depth=3만 검색 대상으로 제한한 점, 이전 논의와 일치합니다. LGTM.
🧹 Nitpick comments (7)
src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java (7)
3-7: @Param 임포트 추가 권장@query 메서드 파라미터 이름 바인딩에 안전하게 대응하려면 @Param 사용을 권장합니다. 임포트가 없으니 아래 추가 부탁드립니다.
import konkuk.chacall.domain.region.domain.model.Region; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param;
11-17: 정렬 대상 필드(regionId) 존재 여부 확인 필요order by r.regionId asc는 엔티티 필드명을 직접 참조합니다. Region에 regionId 프로퍼티가 없다면 런타임 에러가 발생합니다. 존재 여부를 확인하고, 없다면 아래처럼 대체를 검토해 주세요.
- order by r.regionId asc + order by r.regionCode asc또는 PK 필드명이 id라면 r.id 사용을 고려하세요.
20-20: 파생 쿼리 네이밍 적절 — 성능 인덱스만 고려existsByRegionCodeAndDepth 네이밍과 용도 모두 적절합니다. 빈번히 호출된다면 (region_code, depth) 복합 인덱스를 권장합니다.
22-32: @Param 명시 + 검색 정렬 품질 향상(옵션)
- 안전한 바인딩을 위해 @Param 추가를 권장합니다.
- 사용자 체감 품질을 위해 접두어 매치 우선/키워드 위치 기반 정렬을 제안합니다.
- List<Region> searchSubRegionsByFullName(String keyword); + List<Region> searchSubRegionsByFullName(@Param("keyword") String keyword);선택) 접두어 매치 우선 정렬:
- order by r.regionId asc + order by + case when lower(r.fullName) like lower(concat(:keyword, '%')) then 0 else 1 end, + locate(lower(:keyword), lower(r.fullName)), + r.regionId asc주의: lower/locate 사용 시 인덱스 활용도는 떨어집니다. 데이터셋이 작다는 전제 하에 UX 개선용으로만 권장합니다.
22-32: 키워드 입력 검증(빈 문자열/NULL) 확인 요청keyword가 NULL/blank면 like concat('%', :keyword, '%')가 비의도적으로 전체/빈 결과가 될 수 있습니다. 컨트롤러/서비스 레벨에서 @notblank 검증 또는 trim 후 길이 체크를 보장하는지 확인 부탁드립니다.
11-32: 조회/검색 쿼리 용도별 메서드 분리 제안(옵션)null 센티널 패턴((:parentCode is null or ...))은 가독성과 최적화에 불리합니다. 아래처럼 두 메서드로 분리하면 명확성과 인덱스 활용이 좋아집니다.
- @Query(""" - select r - from Region r - where r.depth = :depth - and (:parentCode is null or r.parentCode = :parentCode) - order by r.regionId asc - """) - List<Region> findRegions(@Param("depth") Integer depth, - @Param("parentCode") Long parentCode); + List<Region> findByDepthOrderByRegionCodeAsc(Integer depth); + List<Region> findByDepthAndParentCodeOrderByRegionCodeAsc(Integer depth, Long parentCode);서비스에서 depth==1이면 전자, 그 외에는 후자를 호출하도록 분기하세요.
11-32: DB 인덱스/운영 팁
- find(depth, parentCode): (depth, parent_code) 복합 인덱스
- exists(regionCode, depth): (region_code, depth) 복합 인덱스
- search(fullName like '%…%'): 선행 와일드카드로 B-Tree 인덱스 효과가 약합니다. 데이터 증가 시엔 ngram/tri-gram 인덱스나 전문검색(FULLTEXT/PG trigram, Elastic 등) 고려를 추천합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruckServiceArea.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/RegionService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java(1 hunks)src/main/java/konkuk/chacall/domain/region/domain/model/Region.java(1 hunks)src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/RegionController.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionQueryRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionSearchRequest.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/response/RegionResponse.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQuery.java(1 hunks)src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQueryValidator.java(1 hunks)src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java(1 hunks)src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQuery.java
- src/main/java/konkuk/chacall/global/common/exception/code/ErrorCode.java
- src/main/java/konkuk/chacall/domain/region/application/search/RegionSearchService.java
- src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionSearchRequest.java
- src/main/java/konkuk/chacall/domain/region/application/query/RegionQueryService.java
- src/main/java/konkuk/chacall/domain/region/presentation/dto/response/RegionResponse.java
- src/main/java/konkuk/chacall/domain/foodtruck/domain/FoodTruckServiceArea.java
- src/main/java/konkuk/chacall/domain/region/application/RegionService.java
- src/main/java/konkuk/chacall/domain/region/presentation/dto/request/RegionQueryRequest.java
- src/main/java/konkuk/chacall/domain/region/presentation/dto/validator/ValidRegionQueryValidator.java
- src/main/java/konkuk/chacall/domain/region/domain/model/Region.java
- src/main/java/konkuk/chacall/domain/region/presentation/RegionController.java
- src/main/java/konkuk/chacall/global/common/swagger/SwaggerResponseDescription.java
🔇 Additional comments (1)
src/main/java/konkuk/chacall/domain/region/domain/repository/RegionRepository.java (1)
11-18: @query 파라미터는 @Param으로 명시 바인딩하세요메서드 매개변수가 @Param 없이 -parameters 컴파일 옵션에 의존합니다. 빌드/배포 환경에서 -parameters가 비활성화되면 런타임 오류가 발생할 수 있으므로 명시 바인딩을 권장합니다.
- List<Region> findRegions(Integer depth, Long parentCode); + List<Region> findRegions(@Param("depth") Integer depth, + @Param("parentCode") Long parentCode);빌드 설정에 -parameters 적용 여부를 확인해 주세요. 레포 루트에서 실행할 확인 스크립트 예:
#!/bin/bash rg -n --hidden --glob '!**/build/**' -e '\b-parameters\b' || true rg -n --hidden --glob '!**/build/**' -e 'compilerArgs' || true rg -n --hidden --glob '!**/build/**' -e '<parameters>\s*true' || true
2 participants
#️⃣연관된 이슈
📝작업 내용
지역 조회 / 검색 기능을 구현하였습니다.
지역 조회
기존에 귀찮았던 설계와 달리 실제로 구현시에는 큰 이슈가 없었습니다.
그저 프론트로부터 depth, parentCode 를 전달받은 뒤, 해당 값들을 기반으로 DB 에서 Region 들을 조회하여 반환할 수 있도록 하였습니다.
여기서 고려해야했던 부분은 다음과 같습니다.
였습니다.
이러한 검증이 필요했는데, 이러한 검증을 서비스 단보다는 컨트롤러 단에서 처리한다면, 서비스 로직은 순수하게 비즈니스 로직만 구현해주면 되기 때문에 더욱 편리할 것이라 생각했습니다.
다만 Bean Validation 만을 활용하는 경우 단일 필드만을 검증할 수 있을 뿐 여러 필드에 걸쳐진 조건을 검증할 수는 없기 때문에 Validator 를 적용하였습니다.
다만 기존에 OwnerValidator, MemberValidator 는 요청 dto 에서부터 검증이 들어가는 것이 아니라, 서비스 단에서 검증이 들어가기 때문에 application 패키지 하위에 validator 라는 패키지를 만들고 구현했었는데, 이번에는 요청값을 검증하는 validator 이다보니 presentation 패키지 하위에 validator 패키지를 만들었습니다.
이 부분이 괜찮을까 약간 의문이 들어서 이부분에 대한 현준님의 의견이 궁금합니다!!
지역 검색
이 기능은 아주 간단했습니다.
그저 프론트로부터 키워드를 전달받고, 그 키워드를 부분문자열로 갖는 Region 의 fullName 을 반환해주면 끝이었습니다.
추가적으로, 이 기능은 List 형식으로의 반환이 필요하긴 했으나 별도로 무한 스크롤을 적용하지는 않았습니다.
애초에 지역들은 한정되어있고, 사용자가 동적으로 지역 데이터를 추가하거나 할 수 없기 때문에, 굳이 페이징 로직을 적용하지 않아도 상관 없을 것 같다는 판단이었습니다. (결정적으로 알바몬도 페이징 없더라구요 ㅋㅎ)
스크린샷 (선택)
💬리뷰 요구사항(선택)
Validator 관련해서는 저도 제대로 사용해본 적이 없어서 미흡할 수 있습니다! 감안해주십셔 ㅎ
Summary by CodeRabbit
신기능
오류 처리
문서