-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor/#166] Report 관련 Dto의 Enum class 직렬화 전략을 변경합니다. #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough리포트 관련 DTO와 도메인 열거형의 직렬화 전략을 문자열 기반에서 열거형 기반으로 전환하고, ReportStatus 상수명을 대문자 형식으로 변경 및 fromString/toString 헬퍼를 제거했습니다. Changes
Estimated code review effort🎯 3 (중간) | ⏱️ ~22분
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
data/src/main/java/com/threegap/bitnagil/data/report/model/request/ReportRequestDto.kt(2 hunks)data/src/main/java/com/threegap/bitnagil/data/report/model/response/ReportDetailDto.kt(2 hunks)data/src/main/java/com/threegap/bitnagil/data/report/model/response/ReportItemDto.kt(2 hunks)domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportCategory.kt(0 hunks)domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportStatus.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/reportdetail/model/ReportProcess.kt(1 hunks)presentation/src/main/java/com/threegap/bitnagil/presentation/reporthistory/model/ReportProcess.kt(1 hunks)
💤 Files with no reviewable changes (1)
- domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportCategory.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
data/src/main/java/com/threegap/bitnagil/data/report/model/request/ReportRequestDto.kt (1)
15-15: LGTM! Enum 타입 직접 사용으로 코드가 간결해졌습니다.수동 변환 로직이 제거되고 kotlinx.serialization이 자동으로 처리하도록 변경되어 코드가 더 명확하고 안전해졌습니다.
Also applies to: 30-30
presentation/src/main/java/com/threegap/bitnagil/presentation/reporthistory/model/ReportProcess.kt (1)
17-19: LGTM! 새로운 enum 상수명에 맞게 정확히 매핑되었습니다.Domain의 변경된 enum 상수명(PENDING, IN_PROGRESS, COMPLETED)을 올바르게 참조하고 있습니다.
presentation/src/main/java/com/threegap/bitnagil/presentation/reportdetail/model/ReportProcess.kt (1)
16-18: LGTM! 새로운 enum 상수명에 맞게 정확히 매핑되었습니다.Domain의 변경된 enum 상수명을 올바르게 참조하고 있습니다.
data/src/main/java/com/threegap/bitnagil/data/report/model/response/ReportItemDto.kt (2)
14-14: LGTM! Enum 타입 직접 사용으로 매핑 로직이 간결해졌습니다.String에서 enum으로 변경하여 수동 변환(
fromString) 로직이 제거되고, 타입 안전성이 향상되었습니다.Also applies to: 18-18, 28-28, 30-30
12-12: ReportItemDto의 reportId 타입은 도메인 모델의 ReportItem과 일관성 있게 Int로 정의되어 있으므로, 현재 코드에는 타입 불일치 문제가 없습니다. Swagger 명세에서 Long으로 정의되었다는 PR 코멘트 내용을 확인할 수 없으며, 이 Android 저장소에서는 관련 API 명세를 찾을 수 없습니다.Likely an incorrect or invalid review comment.
data/src/main/java/com/threegap/bitnagil/data/report/model/response/ReportDetailDto.kt (1)
15-15: LGTM! Enum 타입 직접 사용으로 매핑 로직이 간결해졌습니다.reportStatus와 reportCategory를 String에서 enum 타입으로 변경하여 수동 변환 로직이 제거되고, kotlinx.serialization이 자동으로 직렬화를 처리하도록 개선되었습니다.
Also applies to: 21-21, 32-32, 36-36
domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportStatus.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
domain/build.gradle.kts(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportCategory.kt(1 hunks)domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportStatus.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
domain/build.gradle.kts (1)
3-3: LGTM! Kotlin Serialization 설정이 올바르게 추가되었습니다.도메인 모듈에 kotlinx.serialization 플러그인과 의존성을 추가하여
@Serializable어노테이션을 사용할 수 있는 환경이 구성되었습니다. 이는 Report 관련 enum 클래스들의 자동 직렬화를 위해 필수적입니다.Also applies to: 7-7
domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportStatus.kt (1)
5-9: 서버 API 포맷이 대문자 언더스코어 형식이므로 현재 구현은 정확합니다.이전 코드의
companion object에서"IN_PROGRESS"문자열로 매핑되었던 점을 확인했습니다. 현재 enum 상수명IN_PROGRESS는 kotlinx.serialization의 기본 동작에 따라 그대로"IN_PROGRESS"문자열로 직렬화되므로, 서버 API와 일치합니다.따라서
@SerialName어노테이션 추가는 불필요합니다.
domain/src/main/java/com/threegap/bitnagil/domain/report/model/ReportCategory.kt
Show resolved
Hide resolved
l5x5l
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 👍
여담으로 보내주신 Link 2상에서는 서버측에서 보내주는 데이터와 enum간 이름이 동일한 경우 굳이 Serializable 어노테이션이 없어도 될 것 같다는 생각이 들어서 Gemini에 물어보니, 아래처럼 답변하네요!
- 기본적으로 Enum 이름 그래도 JSON으로 주고 받는 경우라면 굳이 붙일 필요가 없지만, 서버 데이터와 Enum 상수의 이름이 다른 경우가 흔하며 이 경우에는 반드시 Serializable 어노테이션이 필요
- 이름이 동일하더라도 "이 Enum은 단순히 내부 로직용이 아니라, API 통신이나 데이터 저장에 사용되는 객체(DTO의 일부)"라는 것을 명확하게 알리는 용도로 사용할 수 있기에 관습적으로 붙여두는 것이 유지보수 측에서 유리
어노테이션이 직렬화/코드 생성과 같은 기능적인 부분 뿐만 아니라 Dto/Domain 모델과 같이 어느 레이어에서 사용되는 클래스인지를 표시할 때도 유용하게 사용될 수 있다는 점을 덕분에 알게 되었습니다!
[ PR Content ]
kotlin serialization은 enum class에 대해서 내부적으로 최적화된 데이터 변환 코드를 제공해준다는 사실을 알게되어 프로젝트에 적용해봤습니다. (우선은 해당 수정으로 인해 변경영향이 가장 적은 Report에 대해서 먼저 적용을 했습니다.)
주요 포인트는 Enum class를 명시하는 경우 빌드 타임에 플러그인이
@Serializable이 붙은 Enum을 분석한 뒤 해당 Enum이 어떤 이름과 순서를 가지고 있는지와 매핑 로직을 작성해준다는 점입니다. 이로 인해 수동으로 매핑로직을 작성하지 않아도 되고 DTO단계부터 타입을 명시할 수 있다는 장점이 있습니다. -> Link 1변경 내용을 아래에 작성한 내용을 참고해주세요. 또한 관련 문서는 해당 링크를 통해 확인할 수 있습니다. -> Link 2
프로젝트 변경점
기존(as-is):
-> 서버로부터 받은 json 데이터를 Stirng 타입의 필드로 수신 후 수동 매퍼 로직을 통해 Enum class로 변환
변경(to-be):
-> DTO에서 Enum 클래스를 String으로 치환하지 않고 직접 타입으로 선언하여 사용하도록 변경.
Related issue
Screenshot 📸
Work Description
To Reviewers 📢
Summary by CodeRabbit
릴리스 노트
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.