fix: 핀 조회 파라미터 복구 및 필터 파싱 재적용#107
Merged
Merged
Conversation
📝 WalkthroughWalkthrough필터 파라미터( Changes필터 enum 타입화 및 컨버터 도입
추정 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~20 minutes 관련 가능성 있는 PR
제안 리뷰어
시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/test/java/com/zimdugo/locker/application/pin/LockerPinQueryServiceTest.java (1)
201-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
never()검증이 너무 구체적합니다.여기는
SUBWAY_INDOOR_SMALL_FILTER로 호출된 경우만 막습니다. 키워드 핀 경로가 잘못되어findWithinBounds(...)를 다른 필터로 호출해도 테스트가 통과할 수 있으니, 아예 reader 무호출을 검증하는 편이 안전합니다.변경 예시
- verify(nearbyLockerPlaceReader, never()).findWithinBounds( - 37.54, - 126.92, - 37.56, - 126.94, - SUBWAY_INDOOR_SMALL_FILTER - ); + then(nearbyLockerPlaceReader).shouldHaveNoInteractions();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/zimdugo/locker/application/pin/LockerPinQueryServiceTest.java` around lines 201 - 207, The current `never()` verification on `nearbyLockerPlaceReader.findWithinBounds(...)` is too specific because it only blocks calls using `SUBWAY_INDOOR_SMALL_FILTER`. Update `LockerPinQueryServiceTest` to assert that `nearbyLockerPlaceReader` is not called at all in this keyword-pin path, using the reader mock itself rather than matching a particular filter. Keep the verification aligned with the relevant service flow in `LockerPinQueryServiceTest` so accidental calls with other filters are also caught.src/main/java/com/zimdugo/locker/application/filter/LockerFacilityFilterType.java (1)
15-17: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win여기도 문자열 기반 enum 브리징은 피하는 편이 안전합니다.
지금 구현은
LockerFacilityFilterType과LockerType의 상수명이 완전히 같다는 가정에 묶여 있습니다. 한쪽 enum만 수정돼도 배포 전에는 안 보이고, 실제 요청 처리 중에만 실패하니 명시 매핑으로 바꿔 두는 편이 낫습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/zimdugo/locker/application/filter/LockerFacilityFilterType.java` around lines 15 - 17, The LockerFacilityFilterType.toDomain() bridge currently depends on matching enum constant names via valueOf(name()), which is fragile if LockerType or LockerFacilityFilterType changes. Replace this string-based conversion with an explicit mapping inside LockerFacilityFilterType.toDomain() (or equivalent dedicated mapper) so each enum constant maps intentionally to the correct LockerType value without relying on name equality.src/main/java/com/zimdugo/locker/application/filter/LockerSizeFilterType.java (1)
10-12: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
valueOf(name())대신 명시 매핑으로 고정해 주세요.
LockerSearchFilterFactory.create()가 이 메서드를 바로 호출하므로, application/domain enum 이름이 한쪽만 바뀌어도 컴파일은 통과하고 여기서 런타임 예외가 납니다.switch로 매핑하면 enum 변경 누락을 컴파일 단계에서 잡을 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/zimdugo/locker/application/filter/LockerSizeFilterType.java` around lines 10 - 12, `LockerSizeFilterType.toDomain()` currently relies on `LockerSizeType.valueOf(name())`, which makes the mapping depend on matching enum names at runtime. Replace this with an explicit `switch`-based mapping inside `toDomain()` so each `LockerSizeFilterType` value maps directly to the corresponding `LockerSizeType`, and review `LockerSearchFilterFactory.create()` callers to ensure they still use this conversion path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/zimdugo/locker/application/filter/LockerSearchFilterFactory.java`:
- Around line 18-32: The filter conversion in LockerSearchFilterFactory is
mapping request enums directly to domain values without removing null entries
first, which can cause a failure when LockerSearchRequest receives empty filter
tokens from the request converters. Update the set-building logic in the factory
to filter out null elements before calling LockerSizeFilterType.toDomain,
IndoorOutdoorFilterType.toDomain, and LockerFacilityFilterType.toDomain, so
empty inputs are treated as empty sets rather than causing an error.
In
`@src/test/java/com/zimdugo/locker/entrypoint/converter/LockerFilterRequestConverterTest.java`:
- Around line 37-42: The invalid locker type test only asserts
BusinessException, so it can pass even if the converter throws the wrong error
code. Update invalidLockerTypeThrowsException in
LockerFilterRequestConverterTest to also verify the BusinessException errorCode
is INVALID_PARAMETER_FORMAT, using the same
lockerTypeRequestConverter.convert("unknown") path so the API contract is pinned
down.
---
Nitpick comments:
In
`@src/main/java/com/zimdugo/locker/application/filter/LockerFacilityFilterType.java`:
- Around line 15-17: The LockerFacilityFilterType.toDomain() bridge currently
depends on matching enum constant names via valueOf(name()), which is fragile if
LockerType or LockerFacilityFilterType changes. Replace this string-based
conversion with an explicit mapping inside LockerFacilityFilterType.toDomain()
(or equivalent dedicated mapper) so each enum constant maps intentionally to the
correct LockerType value without relying on name equality.
In
`@src/main/java/com/zimdugo/locker/application/filter/LockerSizeFilterType.java`:
- Around line 10-12: `LockerSizeFilterType.toDomain()` currently relies on
`LockerSizeType.valueOf(name())`, which makes the mapping depend on matching
enum names at runtime. Replace this with an explicit `switch`-based mapping
inside `toDomain()` so each `LockerSizeFilterType` value maps directly to the
corresponding `LockerSizeType`, and review `LockerSearchFilterFactory.create()`
callers to ensure they still use this conversion path.
In
`@src/test/java/com/zimdugo/locker/application/pin/LockerPinQueryServiceTest.java`:
- Around line 201-207: The current `never()` verification on
`nearbyLockerPlaceReader.findWithinBounds(...)` is too specific because it only
blocks calls using `SUBWAY_INDOOR_SMALL_FILTER`. Update
`LockerPinQueryServiceTest` to assert that `nearbyLockerPlaceReader` is not
called at all in this keyword-pin path, using the reader mock itself rather than
matching a particular filter. Keep the verification aligned with the relevant
service flow in `LockerPinQueryServiceTest` so accidental calls with other
filters are also caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9f91c175-aef5-416b-82c2-31cd9304855a
📒 Files selected for processing (25)
src/main/java/com/zimdugo/locker/application/filter/IndoorOutdoorFilterType.javasrc/main/java/com/zimdugo/locker/application/filter/LockerFacilityFilterType.javasrc/main/java/com/zimdugo/locker/application/filter/LockerSearchFilterFactory.javasrc/main/java/com/zimdugo/locker/application/filter/LockerSizeFilterType.javasrc/main/java/com/zimdugo/locker/application/pin/LockerPinQuery.javasrc/main/java/com/zimdugo/locker/application/pin/LockerPinQueryService.javasrc/main/java/com/zimdugo/locker/application/place/PlaceLockerQueryCommand.javasrc/main/java/com/zimdugo/locker/application/place/PlaceLockerQueryService.javasrc/main/java/com/zimdugo/locker/application/search/LockerSearchCommand.javasrc/main/java/com/zimdugo/locker/application/search/LockerSearchResultQueryService.javasrc/main/java/com/zimdugo/locker/domain/search/LockerSearchFilter.javasrc/main/java/com/zimdugo/locker/entrypoint/config/LockerRequestWebMvcConfig.javasrc/main/java/com/zimdugo/locker/entrypoint/converter/IndoorOutdoorTypeRequestConverter.javasrc/main/java/com/zimdugo/locker/entrypoint/converter/LockerFilterRequestValueNormalizer.javasrc/main/java/com/zimdugo/locker/entrypoint/converter/LockerSizeTypeRequestConverter.javasrc/main/java/com/zimdugo/locker/entrypoint/converter/LockerTypeRequestConverter.javasrc/main/java/com/zimdugo/locker/entrypoint/dto/request/pin/LockerPinRequest.javasrc/main/java/com/zimdugo/locker/entrypoint/dto/request/place/PlaceLockerRequest.javasrc/main/java/com/zimdugo/locker/entrypoint/dto/request/search/LockerSearchRequest.javasrc/test/java/com/zimdugo/locker/application/pin/LockerPinQueryServiceTest.javasrc/test/java/com/zimdugo/locker/application/place/PlaceLockerQueryServiceTest.javasrc/test/java/com/zimdugo/locker/application/search/LockerSearchResultQueryServiceTest.javasrc/test/java/com/zimdugo/locker/domain/search/LockerSearchFilterTest.javasrc/test/java/com/zimdugo/locker/entrypoint/converter/LockerFilterRequestConverterTest.javasrc/test/java/com/zimdugo/locker/entrypoint/dto/request/search/LockerSearchRequestTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/zimdugo/locker/domain/search/LockerSearchFilter.java
Comment on lines
+37
to
+42
| @Test | ||
| @DisplayName("보관함 유형 컨버터는 유효하지 않은 입력을 예외로 처리한다") | ||
| void invalidLockerTypeThrowsException() { | ||
| assertThatThrownBy(() -> lockerTypeRequestConverter.convert("unknown")) | ||
| .isInstanceOf(BusinessException.class); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
오류 코드까지 검증해 주세요.
지금은 BusinessException 타입만 확인해서, 컨버터가 잘못된 errorCode를 던져도 테스트가 통과합니다. 이 경로는 API 계약이라 INVALID_PARAMETER_FORMAT까지 고정해 두는 편이 좋습니다.
변경 예시
+import com.zimdugo.core.exception.ErrorCode;
+
`@Test`
`@DisplayName`("보관함 유형 컨버터는 유효하지 않은 입력을 예외로 처리한다")
void invalidLockerTypeThrowsException() {
assertThatThrownBy(() -> lockerTypeRequestConverter.convert("unknown"))
- .isInstanceOf(BusinessException.class);
+ .isInstanceOf(BusinessException.class)
+ .extracting("errorCode")
+ .isEqualTo(ErrorCode.INVALID_PARAMETER_FORMAT);
}📝 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
| @Test | |
| @DisplayName("보관함 유형 컨버터는 유효하지 않은 입력을 예외로 처리한다") | |
| void invalidLockerTypeThrowsException() { | |
| assertThatThrownBy(() -> lockerTypeRequestConverter.convert("unknown")) | |
| .isInstanceOf(BusinessException.class); | |
| } | |
| import com.zimdugo.core.exception.ErrorCode; | |
| `@Test` | |
| `@DisplayName`("보관함 유형 컨버터는 유효하지 않은 입력을 예외로 처리한다") | |
| void invalidLockerTypeThrowsException() { | |
| assertThatThrownBy(() -> lockerTypeRequestConverter.convert("unknown")) | |
| .isInstanceOf(BusinessException.class) | |
| .extracting("errorCode") | |
| .isEqualTo(ErrorCode.INVALID_PARAMETER_FORMAT); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/zimdugo/locker/entrypoint/converter/LockerFilterRequestConverterTest.java`
around lines 37 - 42, The invalid locker type test only asserts
BusinessException, so it can pass even if the converter throws the wrong error
code. Update invalidLockerTypeThrowsException in
LockerFilterRequestConverterTest to also verify the BusinessException errorCode
is INVALID_PARAMETER_FORMAT, using the same
lockerTypeRequestConverter.convert("unknown") path so the API contract is pinned
down.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
✨ 작업 내용 한 줄 요약
/lockers/pins요청은 클라이언트 계약에 맞게 검증 없이 유지🛠️ 작업 내용
userLat,userLng로 분리하고 공통 필터 생성 로직 복구/lockers/pins요청 DTO의 사용자 위치 검증을 제거해 idle map 호출과 keyword pin 호출이 같은 엔드포인트를 계속 사용하도록 정리🧠 기술적 의사결정
sizeTypes,indoorOutdoorTypes,lockerTypes문자열 파싱 책임을 request DTO와 도메인 필터에서 빼고 web converter로 이동해 서비스 계층이 이미 해석된 값만 받도록 변경Summary by CodeRabbit
New Features
Bug Fixes