Feat/#14 URI Parser및 Matcher 구현 #15
Conversation
jm991014
added
the
FEAT✨
angryPodo
left a comment
angryPodo
left a comment
There was a problem hiding this comment.
고생하셨습니다~! 구조적인 고민도 느껴져서 감탄하면서 리뷰했어요😊
단위 테스트까지 꼼꼼하게 작성해주셔서 프로젝트의 안정성도 견고해졌다고 생각해요! 리뷰라기 보단 좀더 거시적인 관점에서의 논의를 남겼어요! 확인해주시면 감사하겠습니다
| class Wisp( | ||
| private val registry: WispRegistrySpec, | ||
| private val parser: WispUriParser = DefaultWispUriParser() | ||
| ) { |
There was a problem hiding this comment.
클래스로 구현해주셨네요! 제가 생각할때는 object로도 구현이 가능해보이는데 어떻게 생각하시나요?
현재 구현대로면 사용자가 직접 객체를 생성하고 생명주기를 관리해야 하는 방식이네요. 테스트 용이성이나 parser의 교체가능성을 생각하면 class도 좋다고 생각해요! 저는 편의성에서 object로 구현하면 어떨까 생각이 드네요🤔🤔🤔
There was a problem hiding this comment.
저도 처음에 object로 구현했었는데요, 구현하다 보니 WispRegistry가 KSP로 생성되는 코드라서 런타임에 이를 유연하게 주입받으려면 생성자 주입 방식이 더 적합하다고 판단했습니다.
또한, 말씀하신 대로 사용자가 커스텀 Parser를 사용하고 싶을 때, object라면 전역 상태를 변경해야 하는 위험이 있지만, class라면 필요한 설정에 따라 인스턴스를 분리할 수 있어 확장성 면에서 이점이 더 크다고 생각했습니다.
실제로 적용했을 때 말씀하신 것처럼 생명주기를 관리해야하는 부분에서 불편함이 느껴진다면 object로 변경하는 방향은 어떨까요?ㅎㅎ
There was a problem hiding this comment.
좋습니다! 지금 방향이 확장성 면에서는 더 좋다고 생각해요!
| for (pattern in allPatterns) { | ||
| val params = WispUriMatcher.match(inputUri, pattern) | ||
|
|
||
| if (params != null) { | ||
| val factory = registry.getRouteFactory(pattern) | ||
| ?: throw WispError.UnknownPath(pattern) | ||
|
|
||
| return factory.create(params) | ||
| } | ||
| } |
There was a problem hiding this comment.
지금은 등록된 모든 라우트 패턴을 순회하고 있는데, 만약에 저희가 만드는 라이브러리가 수백개의 딥링크를 가진 앱에 적용된다면 성능에 병목이 생길 것 같아요.🤔
초기 구현으로는 사실 충분히 실용적이라 추후의 이슈로 넘겨도 좋다고 생각합니다!
There was a problem hiding this comment.
미처 고려하지 못한 상황이네요!
해당 부분은 새로운 이슈를 열어서 적용해보도록 하겠습니다 👍
| private fun parseQueryString(query: String, params: MutableMap<String, String>) { | ||
| query.split('&').forEach { pair -> | ||
| val parts = pair.split('=', limit = 2) | ||
|
|
||
| if (parts.size == 2) params[parts[0]] = parts[1] | ||
| } | ||
| } |
There was a problem hiding this comment.
이번 프리코스를 하면서 체감했던 생각이 "바퀴를 재발명 하지 마라" 인데요!
현재 쿼리 파라미터를 직접 파싱하다보면 값 자체에 인코딩된 특수문자나 복잡한 URI를 만났을때 쉽게 깨질 수 있을 것 같습니다.
그래서 저는 안드로이드 프레임워크의 getQueryParameter()와 같은 기능을 사용하는건 어떨까요?
There was a problem hiding this comment.
해당 방식으로 수정해보았더니 Uri 클래스에 대한 Mocking이 필요하더라구요.
Uri의 내부 로직을 일일이 Mocking 하는 것보다, Robolectric을 도입하여 해결해보려 했지만, JUnit4에서만 사용 가능하여 적용은 조금 더 미뤄야할 것 같습니다ㅠㅠ
Summary
런타임에서 딥링크 URI를 수신하여 파싱하고,
WispRegistry에 등록된 라우트 패턴과 매칭하여 실제 라우트 객체로 변환하는 핵심 매칭 로직을 구현했습니다.Related Issue
Closes #14
ScreenShot(Option)
Details
이번 PR은
Wisp라이브러리의 런타임 진입점을 완성합니다. 서버로부터 받은 문자열 URI를 실제 객체(List)로 변환하는 로직을 포함합니다.WispUriMatcher구현Wisp클래스 구현object대신class로 설계하여WispRegistrySpec과WispUriParser를 생성자로 주입받도록 하였습니다.WispUriParser및 기본 구현체DefaultWispUriParser: ?stack= 쿼리 파라미터와 | 구분자를 사용하여 딥링크를 파싱하는 기본 로직을 구현했습니다.WispRegistry기능 확장에러 핸들링 및 테스트
ParsingFailed,UnknownPath등 런타임 예외 클래스를 추가했습니다.WispUriMatcherTest를 통해 경로 매칭 케이스와 예외 상황을 검증했습니다.