[#11] 요청 빌드 로직을 구현한다#19
Conversation
|
/gemini review |
There was a problem hiding this comment.
Code Review
이번 PR은 NXRequestBuilder에 preparedURLRequest 메서드를 추가하고, URLRequest 조립을 담당하는 NXRequestAssembler를 구현하였습니다. 경로 병합, 헤더 및 쿼리 아이템 처리 로직이 포함되었으며 이에 대한 단위 테스트도 추가되었습니다. 리뷰에서는 RequestAssembler의 명명 규칙을 NX 접두어를 사용하는 방식으로 통일할 것과, mergedPath 구현 시 의도적인 후행 슬래시(trailing slash)가 삭제되지 않도록 로직을 개선할 것을 제안하였습니다.
| let trimmedBasePath = basePath.trimmingCharacters(in: CharacterSet(charactersIn: "/")) | ||
| let trimmedRequestPath = requestPath.trimmingCharacters(in: CharacterSet(charactersIn: "/")) | ||
|
|
||
| if trimmedBasePath.isEmpty { | ||
| return "/\(trimmedRequestPath)" | ||
| } | ||
|
|
||
| if trimmedRequestPath.isEmpty { | ||
| return "/\(trimmedBasePath)" | ||
| } | ||
|
|
||
| return "/\(trimmedBasePath)/\(trimmedRequestPath)" |
There was a problem hiding this comment.
현재 mergedPath 구현은 trimmingCharacters를 사용하여 모든 선행/후행 슬래시를 제거합니다. 이로 인해 requestPath에 포함된 의도적인 후행 슬래시(trailing slash)가 소실되는 문제가 발생합니다. 일부 API는 후행 슬래시 유무를 엄격하게 구분하므로, 이를 보존하도록 수정해야 합니다. 또한 CharacterSet을 매번 생성하는 대신 더 효율적인 방식으로 경로를 병합할 수 있습니다.
let trimmedBase = basePath.hasSuffix("/") ? String(basePath.dropLast()) : basePath
let trimmedRequest = requestPath.hasPrefix("/") ? String(requestPath.dropFirst()) : requestPath
if trimmedRequest.isEmpty {
return basePath.hasPrefix("/") ? basePath : "/\(basePath)"
}
let joined = "\(trimmedBase)/\(trimmedRequest)"
return joined.hasPrefix("/") ? joined : "/\(joined)"There was a problem hiding this comment.
Code Review
이번 PR은 NXRequestBuilder에 preparedURLRequest() 메서드를 추가하고, 실제 URLRequest를 조립하는 RequestAssembler 로직을 구현하였습니다. 또한 상대 경로 및 절대 경로 처리, 경로 병합 로직에 대한 단위 테스트가 포함되었습니다. 리뷰에서는 명명 규칙에 따른 NX 접두어 추가, allHTTPHeaderFields를 활용한 헤더 설정 최적화, CharacterSet 재사용을 통한 성능 개선, 그리고 딕셔너리 병합 로직 간소화에 대한 피드백이 제공되었습니다.
🔗 연관된 이슈
📝 작업 내용
📌 요약
🔍 상세
NXRequestAssembler추가NXClientConfiguration + RequestSpec를URLRequest로 조립NXRequestBuilder.preparedURLRequest()추가RequestAssembler.compile(...)연결NXRequestBuildLogicTests추가테스트
swift test통과📸 영상 / 이미지 (Optional)