API 직렬화 라이브러리를 Kotlin Serialization으로 통합#729
Open
boiledeggg wants to merge 19 commits into
Open
Conversation
- DefaultResponse를 적용
- DefaultResponse를 적용 - Gson에서 Kotlin Serialization으로 마이그레이션
- DefaultResponse를 적용 - Gson에서 Kotlin Serialization으로 마이그레이션
- DefaultResponse 타입을 Nothing에서 Unit으로 변경 - Gson에서 Kotlin Serialization으로 마이그레이션
- DefaultResponse 타입을 Nothing에서 Unit으로 변경
- DefaultResponse 타입을 Nothing에서 Unit으로 변경
- DefaultResponse를 적용 - Gson에서 Kotlin Serialization으로 마이그레이션
- DefaultResponse 타입을 Nothing에서 Unit으로 변경 - Gson에서 Kotlin Serialization으로 마이그레이션
- Gson에서 Kotlin Serialization으로 마이그레이션
- Gson에서 Kotlin Serialization으로 마이그레이션
Contributor
WalkthroughKotlinx.serialization로 Retrofit 변환기 통합, 여러 Retrofit 응답을 ChangesKotlinx Serialization 및 DefaultResponse 통합
Sequence DiagramsequenceDiagram
participant Client as UI/UseCase
participant Repo as Repository
participant Remote as Retrofit(Service)
participant DB as Database
Client->>Repo: fetchItems()
Repo->>Remote: service.fetchSomething() (DefaultResponse<T>)
Remote-->>Repo: DefaultResponse[data=list] / resultCode
alt isSuccessAndDataExists
Repo->>DB: map & insert(entities)
DB-->>Repo: insert result
Repo-->>Client: mapped domain list
else failure or data null
Repo-->>Client: throw IllegalStateException(resultMsg) or return empty list
end
Possibly related PRs
시
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.kt (1)
13-16:⚠️ Potential issue | 🔴 Critical@SerialName 어노테이션 누락으로 인한 역직렬화 실패
leftAskCount,maxAskCount필드에@SerialName어노테이션이 없습니다. 프로젝트의 모든 다른 응답 DTO(UserDataResponse, SearchStaffResponse, DefaultResponse 등)는 모든 필드에@SerialName을 적용하고 있으며, 코딩 가이드라인에서도 "모든 필드에@SerialName어노테이션을 사용하여 실제 API 필드명과 명시적으로 매핑"하도록 요구하고 있습니다. 이 어노테이션이 없으면 Kotlinx Serialization이 필드를 올바르게 역직렬화하지 못할 수 있습니다.실제 API 응답의 필드명 형식(snake_case 또는 camelCase)을 확인하여 적절한
@SerialName값을 추가해주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.kt` around lines 13 - 16, KuringBotQueryCountData is missing `@SerialName` annotations on its properties (leftAskCount, maxAskCount) which breaks kotlinx.serialization deserialization; update the data class KuringBotQueryCountData to add `@SerialName`("...") for each property using the exact API field names (e.g. "left_ask_count" or "leftAskCount" and "max_ask_count" or "maxAskCount" depending on the API), apply `@SerialName` to both leftAskCount and maxAskCount, and ensure imports for kotlinx.serialization.SerialName are present so the serializer maps JSON fields explicitly.
🧹 Nitpick comments (1)
data/remote/src/test/java/com/ku_stacks/ku_ring/remote/ApiAbstract.kt (1)
14-14: ⚡ Quick win테스트 Json 설정을 프로덕션 설정과 재사용하도록 맞추는 게 좋습니다.
Line 14의 로컬 Json 설정이 Line 45에서 직접 사용되어, 추후 프로덕션
Json옵션이 바뀔 때 테스트가 다른 규칙으로 파싱할 수 있습니다. 공통 팩토리/상수 재사용으로 설정 드리프트를 막는 것을 권장합니다.Also applies to: 45-45
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/ApiAbstract.kt` at line 14, The test's local Json instance (private val json) diverges from production and risks config drift; replace it by reusing the shared production Json factory/constant (e.g., JsonFactory.provideJson() or a common JsonConfig/COMMON_JSON instance) instead of declaring a local Json, and update all usages (including where json is directly used at call sites) to reference that shared provider; ensure the shared provider uses Json { ignoreUnknownKeys = true } (or the current production options) so tests and production parse with identical settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/academicevent/src/main/java/com/ku_stacks/ku_ring/academicevent/repository/AcademicEventRepositoryImpl.kt`:
- Around line 29-30: 현재 코드 in AcademicEventRepositoryImpl uses
response.data?.toEntity() without checking response.isSuccess, which can persist
invalid data when isSuccess == false; update the logic to verify
response.isSuccess is true and response.data is non-null before calling
toEntity(), otherwise throw IllegalStateException(response.resultMsg) (i.e., use
a conditional like if (response.isSuccess == true && response.data != null) {
val eventEntities = response.data.toEntity() } else { throw
IllegalStateException(response.resultMsg) }) so both success flag and payload
are validated.
In
`@data/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.kt`:
- Around line 142-143: In NoticeRepositoryImpl, don't mask remote failures by
converting a null response.data into emptyList; instead explicitly handle the
null case from noticeClient.fetchSubscribe: if fetchSubscribe returns null data,
throw or propagate a descriptive exception (e.g., RemoteFetchException or
IllegalStateException) so callers can distinguish actual empty subscriptions
from a failed fetch; keep the successful path mapping data.map { it.koreanName }
and only return emptyList when the response is a valid empty list, not when data
is null.
In
`@data/notice/src/main/java/com/ku_stacks/ku_ring/notice/source/DepartmentNoticeMediator.kt`:
- Around line 46-47: The exception message incorrectly refers to
"noticeResponse" when the null check is on noticeResponse.data; update the
thrown IllegalStateException messages in DepartmentNoticeMediator where you have
checks like noticeResponse.data ?: throw IllegalStateException("noticeResponse
is null") to accurately reference noticeResponse.data (e.g.,
"noticeResponse.data is null" or include both noticeResponse and data), so
errors point to the real failing variable; adjust the same change for the other
occurrence around the second null-check as well.
In
`@data/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/response/DepartmentResponse.kt`:
- Around line 8-10: The DepartmentResponse DTO is missing `@SerialName` on the
properties name and korName; update the data class in DepartmentResponse.kt to
annotate both 'name' and 'korName' with the exact API field names (matching the
API’s keys) just like 'shortName' is annotated with `@SerialName`("hostPrefix"),
and ensure kotlinx.serialization.SerialName is imported; verify and use the
API’s canonical key strings (e.g., "name" or snake_case "kor_name") when adding
the annotations.
In
`@data/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/DepartmentNoticeResponse.kt`:
- Around line 6-15: DepartmentNoticeResponse is missing `@SerialName` annotations
on its properties; add `@SerialName`("...") to each property (id, articleId,
postedDate, subject, url, category, important, commentCount) to exactly match
the API field names and ensure stable deserialization, and import
kotlinx.serialization.SerialName; keep the `@Serializable` annotation on the data
class and ensure the string values in `@SerialName` match the API contract (e.g.,
use the exact JSON keys for articleId, postedDate, etc.).
In
`@data/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/request/RegisterUserRequest.kt`:
- Around line 5-6: The RegisterUserRequest data class is missing a `@SerialName`
on the token property; update the RegisterUserRequest definition to annotate the
token property with `@SerialName` using the exact API field name (e.g.,
`@SerialName`("token")) and add the necessary import
(kotlinx.serialization.SerialName) so serialization contracts are explicit for
the RegisterUserRequest.token field.
In
`@data/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.kt`:
- Around line 6-10: The DTO KuringBotQueryCountResponse and its nested
KuringBotQueryCountData are missing `@SerialName` annotations on their properties
(code, message, data, leftAskCount, maxAskCount); update the data class
definitions for KuringBotQueryCountResponse and KuringBotQueryCountData to add
`@SerialName`("code"), `@SerialName`("message"), `@SerialName`("data") on the
top-level properties and `@SerialName`("leftAskCount"), `@SerialName`("maxAskCount")
on the nested properties so they match the project's serialization convention
and other DTOs like UserDataResponse.
In
`@data/user/src/main/java/com/ku_stacks/ku_ring/user/repository/UserRepositoryImpl.kt`:
- Around line 94-97: The current mapping in UserRepositoryImpl (inside the map {
response -> with(response) { ... } } block) treats responses with isSuccess ==
true but data == null as successful; change the condition to require data
non-null (e.g., if (isSuccess && data != null) { pref.accessToken =
data.accessToken } else { treat as failure: log an error with resultMsg and the
missing-data context and return/fail appropriately }). Ensure you reference the
response fields isSuccess, data, resultMsg and the side-effect pref.accessToken
so the repository consistently fails when the success flag is true but payload
is missing.
---
Outside diff comments:
In
`@data/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.kt`:
- Around line 13-16: KuringBotQueryCountData is missing `@SerialName` annotations
on its properties (leftAskCount, maxAskCount) which breaks kotlinx.serialization
deserialization; update the data class KuringBotQueryCountData to add
`@SerialName`("...") for each property using the exact API field names (e.g.
"left_ask_count" or "leftAskCount" and "max_ask_count" or "maxAskCount"
depending on the API), apply `@SerialName` to both leftAskCount and maxAskCount,
and ensure imports for kotlinx.serialization.SerialName are present so the
serializer maps JSON fields explicitly.
---
Nitpick comments:
In `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/ApiAbstract.kt`:
- Line 14: The test's local Json instance (private val json) diverges from
production and risks config drift; replace it by reusing the shared production
Json factory/constant (e.g., JsonFactory.provideJson() or a common
JsonConfig/COMMON_JSON instance) instead of declaring a local Json, and update
all usages (including where json is directly used at call sites) to reference
that shared provider; ensure the shared provider uses Json { ignoreUnknownKeys =
true } (or the current production options) so tests and production parse with
identical settings.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05de3451-c2f0-4a2e-8e1c-834d6edb6d1e
📒 Files selected for processing (76)
build-logic/src/main/kotlin/com/ku_stacks/ku_ring/buildlogic/primitive/RetrofitPlugin.ktdata/academicevent/src/main/java/com/ku_stacks/ku_ring/academicevent/repository/AcademicEventRepositoryImpl.ktdata/academicevent/src/test/java/com/ku_stacks/ku_ring/academicevent/AcademicEventTestUtil.ktdata/academicevent/src/test/java/com/ku_stacks/ku_ring/academicevent/AcademicRepositoryTest.ktdata/department/src/test/java/com/ku_stacks/ku_ring/department/repository/DepartmentRepositoryTest.ktdata/notice/src/main/java/com/ku_stacks/ku_ring/notice/mapper/ResponseToDomain.ktdata/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.ktdata/notice/src/main/java/com/ku_stacks/ku_ring/notice/source/CategoryNoticeMediator.ktdata/notice/src/main/java/com/ku_stacks/ku_ring/notice/source/DepartmentNoticeMediator.ktdata/notice/test/src/main/java/com/ku_stacks/ku_ring/notice/test/NoticeTestUtil.ktdata/remote/AGENTS.mddata/remote/src/main/java/com/ku_stacks/ku_ring/remote/academicevent/AcademicEventClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/academicevent/AcademicEventService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/academicevent/di/AcademicEventModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/academicevent/response/AcademicEventListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/club/di/ClubModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/DepartmentService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/di/DepartmentModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/request/DepartmentSubscribeRequest.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/response/DepartmentListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/response/DepartmentResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/di/LibraryModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibraryRoomBranchResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibraryRoomListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibraryRoomResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibraryRoomSeatResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibraryRoomTypeResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibrarySeatResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/NoticeClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/NoticeService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/di/NoticeModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/request/SubscribeRequest.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/CategoryResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/DepartmentNoticeListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/DepartmentNoticeResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/NoticeListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/NoticeResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SearchNoticeDataResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SearchNoticeListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SearchNoticeResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SubscribeListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/noticecomment/NoticeCommentClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/noticecomment/NoticeCommentService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/noticecomment/di/NoticeCommentModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/report/ReportClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/report/ReportService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/report/di/ReportModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/space/di/KuringSpaceModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/space/response/AppVersionResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/StaffClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/StaffService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/di/StaffModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/response/SearchStaffDataResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/response/SearchStaffListResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/response/SearchStaffResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/UserClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/UserService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/di/UserModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/request/AcademicEventNotificationRequest.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/request/FeedbackRequest.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/request/RegisterUserRequest.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/SignInResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/util/DefaultResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/util/NetworkModule.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/util/NetworkQualifier.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/verification/VerificationClient.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/verification/VerificationService.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/verification/di/VerficationModule.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/AcademicEventServiceTest.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/ApiAbstract.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/NoticeServiceTest.ktdata/remote/src/test/resources/api-response/GetNoticeCommentResponse.jsondata/staff/src/main/java/com/ku_stacks/ku_ring/staff/mapper/ResponseToDomain.ktdata/user/src/main/java/com/ku_stacks/ku_ring/user/repository/UserRepositoryImpl.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (10)
- build-logic/src/main/kotlin/com/ku_stacks/ku_ring/buildlogic/primitive/RetrofitPlugin.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/staff/response/SearchStaffListResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/response/DepartmentListResponse.kt
- gradle/libs.versions.toml
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/NoticeListResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/academicevent/response/AcademicEventListResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SubscribeListResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SearchNoticeListResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/util/DefaultResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/DepartmentNoticeListResponse.kt
l2hyunwoo
reviewed
May 1, 2026
l2hyunwoo
reviewed
May 1, 2026
Comment on lines
7
to
+9
| data class SignInResponse( | ||
| @SerialName("code") val code: Int, | ||
| @SerialName("message") val message: String, | ||
| @SerialName("data") val data: SignInToken, | ||
| ) { | ||
| @Serializable | ||
| data class SignInToken( | ||
| @SerialName("accessToken") val accessToken: String, | ||
| ) | ||
|
|
||
| val isSuccess: Boolean get() = (code == 200) | ||
| } | ||
| @SerialName("accessToken") val accessToken: String, | ||
| ) |
Member
Author
There was a problem hiding this comment.
직접 확인은 했었는데, 확실한 검증을 위해 테스트 코드 추가해뒀습니다!
l2hyunwoo
reviewed
May 1, 2026
l2hyunwoo
reviewed
May 1, 2026
Collaborator
l2hyunwoo
left a comment
l2hyunwoo
left a comment
There was a problem hiding this comment.
의존성에서 gson 날라간 것 확인
추가로 API 단 unit test 전체 통과한거 스샷찍어서 올려주심 감사용
ikseong00
reviewed
May 2, 2026
- 동아리 목록 및 동아리 정보 - 학과 목록 - 깃허브에 저장된 앱 최소 스펙 - 교수 목록 - 로그인 응답
Member
Author
|
@l2hyunwoo PR 코멘트에 스샷 추가해뒀습니다! (저거 말하신거 맞죠?) |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/notice/src/test/java/com/ku_stacks/ku_ring/notice/NoticeRepositoryTest.kt (1)
78-83:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win성공 케이스 테스트의 기대값은 non-null로 고정해 계약을 명확히 검증해 주세요.
Line 78의 nullable 기대값은 성공 경로 검증을 약하게 만듭니다.
data를 명시적으로 non-null로 두고assertEquals로 비교하는 편이 명확합니다.제안 수정안
- val expected = mockSubscribeList.data?.map { it.koreanName } + val expected = requireNotNull(mockSubscribeList.data).map { it.koreanName } @@ - assert(result == expected) + assertEquals(expected, result)🤖 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 `@data/notice/src/test/java/com/ku_stacks/ku_ring/notice/NoticeRepositoryTest.kt` around lines 78 - 83, The test uses a nullable expected value (mockSubscribeList.data?.map { it.koreanName }) which weakens the success assertion; change it to a non-null expectation and use an equality assertion method. Replace the nullable mapping with a non-null mapping (e.g., mockSubscribeList.data!!.map { it.koreanName } or construct a non-null expected list) and assert using assertEquals(expected, repository.fetchSubscriptionFromRemote(mockToken)) instead of assert(result == expected) so fetchSubscriptionFromRemote is validated against a guaranteed non-null expected value.
🧹 Nitpick comments (6)
data/remote/src/test/java/com/ku_stacks/ku_ring/remote/KuringSpaceServiceTest.kt (1)
32-35: ⚡ Quick win
takeRequest()결과도 검증해서 회귀 감지를 강화해 주세요.요청을 꺼내기만 하고 검증하지 않으면 잘못된 경로/메서드 호출이 숨어도 테스트가 통과할 수 있습니다. 최소한
path,method는 assert 하는 것이 좋습니다.예시 diff
// when val response = service.getMinimumAppVersion() - mockWebServer.takeRequest() + val request = mockWebServer.takeRequest() // then + assertEquals("GET", request.method) + assertEquals("/minimum-app-version", request.path) // 실제 엔드포인트에 맞게 조정 assertEquals("2.0.0", response.minimumAppVersion)🤖 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 `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/KuringSpaceServiceTest.kt` around lines 32 - 35, 테스트에서 mockWebServer.takeRequest()로 꺼낸 요청(Request)을 검증하지 않아 경로/메서드 변경이 누락될 수 있으니 KuringSpaceServiceTest 내 mockWebServer.takeRequest() 호출 결과를 변수(request)로 받아 request.path와 request.method를 assert하여 예상 엔드포인트와 HTTP 메서드와 일치하는지 확인하고 기존의 assertEquals("2.0.0", response.minimumAppVersion)도 유지하세요; 예를 들어 request.path가 기대값(예: "/expected/path")과 request.method가 기대값(예: "GET")인지 검증하도록 수정하세요.data/remote/src/test/java/com/ku_stacks/ku_ring/remote/UserServiceTest.kt (1)
60-79: ⚡ Quick winJUnit4 스타일이 JUnit5 컨벤션과 맞지 않습니다.
이 파일은
**/*Test.kt패턴에 해당하며, 코딩 가이드라인에 따라 JUnit5를 사용해야 합니다. 현재 코드는org.junit.Test와org.junit.Assert.assertEquals(JUnit4)를 사용하고 있으므로,org.junit.jupiter.api패키지의 JUnit5 애노테이션과 어설션으로 마이그레이션하는 것이 좋습니다.마이그레이션 예시
-import org.junit.Assert.assertEquals +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test🤖 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 `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/UserServiceTest.kt` around lines 60 - 79, The test uses JUnit4 imports/annotations; switch to JUnit5 by replacing org.junit.Test with org.junit.jupiter.api.Test and replace org.junit.Assert.assertEquals calls with org.junit.jupiter.api.Assertions.assertEquals (update any other JUnit4 imports similarly), keeping the test body (the `fun `sign in test`` method, `AuthorizeUserRequest`, `service.signIn(...)`, `mockWebServer.takeRequest()` and coroutine `runTest` usage) unchanged so only the import/annotation and assertion references are migrated to JUnit5 conventions.data/remote/src/test/java/com/ku_stacks/ku_ring/remote/StaffServiceTest.kt (1)
31-33: ⚡ Quick win요청 검증이 빠져 있어 회귀 탐지력이 약합니다.
Line 32에서
takeRequest()만 호출하고 경로/쿼리를 검증하지 않아content파라미터 직렬화/전달 오류를 놓칠 수 있습니다. 최소한 path와 query(content=김은이)를 assert 해주세요.예시 보강 diff
val response = service.fetchStaffs(content = "김은이") - mockWebServer.takeRequest() + val request = mockWebServer.takeRequest() + assertEquals("GET", request.method) + assertEquals("/staff?content=%EA%B9%80%EC%9D%80%EC%9D%B4", request.path)🤖 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 `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/StaffServiceTest.kt` around lines 31 - 33, 테스트에서 요청 검증이 빠져 있어 직렬화/전달 오류를 놓칠 수 있으니 mockWebServer.takeRequest() 호출의 반환값을 받아 request 객체로 저장하고 경로 및 쿼리 검증을 추가하세요: service.fetchStaffs(...) 호출 뒤 mockWebServer.takeRequest() 대신 val request = mockWebServer.takeRequest()를 사용하고 request.path 또는 request.requestUrl.queryParameter("content")를 이용해 expected path와 content=김은이 쿼리가 포함되는지(assertEquals/assertTrue) 확인하도록 수정하세요; 관련 식별자는 fetchStaffs 및 mockWebServer.takeRequest()입니다.data/remote/src/test/java/com/ku_stacks/ku_ring/remote/DepartmentServiceTest.kt (1)
34-34: ⚡ Quick win테스트 내
throw IllegalStateException대신 단언 API 사용 권장
?: throw IllegalStateException(...)패턴은 테스트 프레임워크의 단언 실패 메시지 대신 일반 예외 스택 트레이스만 출력합니다.assertNotNull이나 Kotlin 표준 라이브러리의requireNotNull을 사용하면 더 명확한 실패 메시지를 얻을 수 있습니다.♻️ 제안 수정
- val departments = response.data ?: throw IllegalStateException("Data should not be null") + assertNotNull("data should not be null", response.data) + val departments = requireNotNull(response.data)🤖 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 `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/DepartmentServiceTest.kt` at line 34, The test currently uses the Elvis throw pattern on response.data (val departments = response.data ?: throw IllegalStateException(...)), which yields poor failure output; replace it with a test assertion or Kotlin null-check helper—e.g., call assertNotNull(response.data, "response.data should not be null") from your test framework and then assign departments from the asserted value, or use requireNotNull(response.data) { "response.data should not be null" } to provide a clear failure message; update the line referencing response.data / departments in DepartmentServiceTest.kt accordingly.data/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.kt (2)
142-146: ⚡ Quick win실패 케이스에서 nullable 메시지를 그대로 예외에 넣지 않는 편이 안전합니다.
Line 145의
IllegalStateException(response.resultMsg)는resultMsg == null일 때 원인 파악이 어려워집니다. 기본 메시지를 보강하거나 도메인 예외로 변환해 실패 이유를 보존해 주세요.제안 수정안
- else -> throw IllegalStateException(response.resultMsg) + else -> throw IllegalStateException( + response.resultMsg ?: "Failed to fetch subscriptions from remote" + )Based on learnings: "Implement error handling in repository mappers rather than service definitions by catching specific exceptions (HttpRequestTimeoutException, ResponseException) and transforming them to domain-level exceptions".
🤖 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 `@data/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.kt` around lines 142 - 146, The repository currently throws IllegalStateException(response.resultMsg) in NoticeRepositoryImpl when fetchSubscribe fails; change this to produce a non-null, informative domain-level exception: when response.isSuccessAndDataExists is false, throw a NoticeFetchException (or at minimum IllegalStateException) with a composed message that includes a safe default for response.resultMsg (e.g. response.resultMsg ?: "no message"), plus any available identifiers like response.resultCode or the token, so the cause is preserved; also ensure repository-level mapping catches transport exceptions (e.g. HttpRequestTimeoutException, ResponseException) around noticeClient.fetchSubscribe and rethrows them as the same domain NoticeFetchException to centralize error handling.
167-173: ⚡ Quick win하드코딩된
Dispatchers.IO대신 주입된ioDispatcher로 통일해 주세요.Line 167은 테스트에서 주입한 디스패처를 우회하므로, 같은 저장소 내 디스패처 정책을 일관되게 맞추는 것이 좋습니다.
제안 수정안
- withContext(Dispatchers.IO) { + withContext(ioDispatcher) {🤖 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 `@data/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.kt` around lines 167 - 173, In NoticeRepositoryImpl replace the hardcoded withContext(Dispatchers.IO) usage with the injected ioDispatcher so tests and runtime use the same dispatcher policy: locate the coroutine block that calls noticeClient.fetchNoticeList(query) and change the withContext call to use the class-level or constructor-provided ioDispatcher (the injected CoroutineDispatcher variable named ioDispatcher) so fetching and subsequent logic (including noticeClient.fetchNoticeList and noticeDao.getSavedNoticeList) run on the injected dispatcher.
🤖 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
`@data/academicevent/src/main/java/com/ku_stacks/ku_ring/academicevent/repository/AcademicEventRepositoryImpl.kt`:
- Around line 29-31: The current branch swallows API failures by doing nothing
when response.isSuccessAndDataExists is false; update the block around response
and insertAcademicEventsIntoDB so that when isSuccessAndDataExists is false you
throw an appropriate exception (or call Result.failure) with the response
error/message to propagate the failure to suspendRunCatching callers; locate the
conditional that checks response.isSuccessAndDataExists in
AcademicEventRepositoryImpl (and any use of
response.data!!.toEntity()/insertAcademicEventsIntoDB) and add an else branch
that constructs and throws a descriptive exception (including response
error/details) so callers can distinguish success from API error.
In `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/ClubServiceTest.kt`:
- Around line 5-10: The test class is using JUnit4 annotations
(org.junit.Before, org.junit.After, org.junit.Test) and JUnit4 asserts; migrate
ClubServiceTest to JUnit5 by replacing `@Before` with `@BeforeEach`, `@After` with
`@AfterEach` and `@Test` with org.junit.jupiter.api.Test, update imports to
org.junit.jupiter.api.* and switch assertions to
org.junit.jupiter.api.Assertions (e.g., assertEquals/assertNotNull/assertNull
from Assertions), and remove any leftover org.junit.* imports so all test
lifecycle and assertion symbols come from JUnit5.
In
`@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/KuringSpaceServiceTest.kt`:
- Around line 5-9: Update the test to use JUnit5 imports and annotations:
replace org.junit.Before, org.junit.After, org.junit.Test imports with
org.junit.jupiter.api.BeforeEach, org.junit.jupiter.api.AfterEach,
org.junit.jupiter.api.Test and swap org.junit.Assert.assertEquals to
org.junit.jupiter.api.Assertions.assertEquals; then update the annotations on
the test methods in KuringSpaceServiceTest (replace `@Before` -> `@BeforeEach`,
`@After` -> `@AfterEach`, `@Test` stays but ensure it is the JUnit5 `@Test`) so methods
using those annotations compile against JUnit5.
In `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/StaffServiceTest.kt`:
- Around line 5-9: StaffServiceTest.kt is using JUnit4 imports (`@Before`, `@After`,
org.junit.Assert) but the repo requires JUnit5; update the test to use JUnit5
APIs by replacing org.junit.Before -> org.junit.jupiter.api.BeforeEach,
org.junit.After -> org.junit.jupiter.api.AfterEach, org.junit.Test ->
org.junit.jupiter.api.Test and switch assertions from
org.junit.Assert.assertEquals to org.junit.jupiter.api.Assertions.assertEquals
(and any other Assert methods to Assertions equivalents); ensure annotation
names on methods (e.g., methods currently annotated with `@Before/`@After) are
renamed to `@BeforeEach/`@AfterEach and adjust static imports if present.
---
Outside diff comments:
In
`@data/notice/src/test/java/com/ku_stacks/ku_ring/notice/NoticeRepositoryTest.kt`:
- Around line 78-83: The test uses a nullable expected value
(mockSubscribeList.data?.map { it.koreanName }) which weakens the success
assertion; change it to a non-null expectation and use an equality assertion
method. Replace the nullable mapping with a non-null mapping (e.g.,
mockSubscribeList.data!!.map { it.koreanName } or construct a non-null expected
list) and assert using assertEquals(expected,
repository.fetchSubscriptionFromRemote(mockToken)) instead of assert(result ==
expected) so fetchSubscriptionFromRemote is validated against a guaranteed
non-null expected value.
---
Nitpick comments:
In
`@data/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.kt`:
- Around line 142-146: The repository currently throws
IllegalStateException(response.resultMsg) in NoticeRepositoryImpl when
fetchSubscribe fails; change this to produce a non-null, informative
domain-level exception: when response.isSuccessAndDataExists is false, throw a
NoticeFetchException (or at minimum IllegalStateException) with a composed
message that includes a safe default for response.resultMsg (e.g.
response.resultMsg ?: "no message"), plus any available identifiers like
response.resultCode or the token, so the cause is preserved; also ensure
repository-level mapping catches transport exceptions (e.g.
HttpRequestTimeoutException, ResponseException) around
noticeClient.fetchSubscribe and rethrows them as the same domain
NoticeFetchException to centralize error handling.
- Around line 167-173: In NoticeRepositoryImpl replace the hardcoded
withContext(Dispatchers.IO) usage with the injected ioDispatcher so tests and
runtime use the same dispatcher policy: locate the coroutine block that calls
noticeClient.fetchNoticeList(query) and change the withContext call to use the
class-level or constructor-provided ioDispatcher (the injected
CoroutineDispatcher variable named ioDispatcher) so fetching and subsequent
logic (including noticeClient.fetchNoticeList and noticeDao.getSavedNoticeList)
run on the injected dispatcher.
In
`@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/DepartmentServiceTest.kt`:
- Line 34: The test currently uses the Elvis throw pattern on response.data (val
departments = response.data ?: throw IllegalStateException(...)), which yields
poor failure output; replace it with a test assertion or Kotlin null-check
helper—e.g., call assertNotNull(response.data, "response.data should not be
null") from your test framework and then assign departments from the asserted
value, or use requireNotNull(response.data) { "response.data should not be null"
} to provide a clear failure message; update the line referencing response.data
/ departments in DepartmentServiceTest.kt accordingly.
In
`@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/KuringSpaceServiceTest.kt`:
- Around line 32-35: 테스트에서 mockWebServer.takeRequest()로 꺼낸 요청(Request)을 검증하지 않아
경로/메서드 변경이 누락될 수 있으니 KuringSpaceServiceTest 내 mockWebServer.takeRequest() 호출 결과를
변수(request)로 받아 request.path와 request.method를 assert하여 예상 엔드포인트와 HTTP 메서드와 일치하는지
확인하고 기존의 assertEquals("2.0.0", response.minimumAppVersion)도 유지하세요; 예를 들어
request.path가 기대값(예: "/expected/path")과 request.method가 기대값(예: "GET")인지 검증하도록
수정하세요.
In `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/StaffServiceTest.kt`:
- Around line 31-33: 테스트에서 요청 검증이 빠져 있어 직렬화/전달 오류를 놓칠 수 있으니
mockWebServer.takeRequest() 호출의 반환값을 받아 request 객체로 저장하고 경로 및 쿼리 검증을 추가하세요:
service.fetchStaffs(...) 호출 뒤 mockWebServer.takeRequest() 대신 val request =
mockWebServer.takeRequest()를 사용하고 request.path 또는
request.requestUrl.queryParameter("content")를 이용해 expected path와 content=김은이 쿼리가
포함되는지(assertEquals/assertTrue) 확인하도록 수정하세요; 관련 식별자는 fetchStaffs 및
mockWebServer.takeRequest()입니다.
In `@data/remote/src/test/java/com/ku_stacks/ku_ring/remote/UserServiceTest.kt`:
- Around line 60-79: The test uses JUnit4 imports/annotations; switch to JUnit5
by replacing org.junit.Test with org.junit.jupiter.api.Test and replace
org.junit.Assert.assertEquals calls with
org.junit.jupiter.api.Assertions.assertEquals (update any other JUnit4 imports
similarly), keeping the test body (the `fun `sign in test`` method,
`AuthorizeUserRequest`, `service.signIn(...)`, `mockWebServer.takeRequest()` and
coroutine `runTest` usage) unchanged so only the import/annotation and assertion
references are migrated to JUnit5 conventions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 798c09b8-1ad9-473b-9168-25b7a4751f22
📒 Files selected for processing (25)
data/academicevent/src/main/java/com/ku_stacks/ku_ring/academicevent/repository/AcademicEventRepositoryImpl.ktdata/library/src/test/java/com/ku_stacks/ku_ring/library/LibraryTestUtil.ktdata/notice/src/main/java/com/ku_stacks/ku_ring/notice/repository/NoticeRepositoryImpl.ktdata/notice/src/main/java/com/ku_stacks/ku_ring/notice/source/DepartmentNoticeMediator.ktdata/notice/src/test/java/com/ku_stacks/ku_ring/notice/NoticeRepositoryTest.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/department/response/DepartmentResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/DepartmentNoticeResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SearchNoticeResponse.ktdata/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/ClubServiceTest.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/DepartmentServiceTest.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/KuringSpaceServiceTest.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/NoticeServiceTest.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/StaffServiceTest.ktdata/remote/src/test/java/com/ku_stacks/ku_ring/remote/UserServiceTest.ktdata/remote/src/test/resources/api-response/AppVersionResponse.jsondata/remote/src/test/resources/api-response/ClubDetailResponse.jsondata/remote/src/test/resources/api-response/ClubListResponse.jsondata/remote/src/test/resources/api-response/DepartmentListResponse.jsondata/remote/src/test/resources/api-response/DepartmentNoticeResponse.jsondata/remote/src/test/resources/api-response/NoticeResponse.jsondata/remote/src/test/resources/api-response/SearchNoticeResponse.jsondata/remote/src/test/resources/api-response/SignInResponse.jsondata/remote/src/test/resources/api-response/StaffSearchResponse.jsondata/user/src/main/java/com/ku_stacks/ku_ring/user/repository/UserRepositoryImpl.kt
💤 Files with no reviewable changes (1)
- data/library/src/test/java/com/ku_stacks/ku_ring/library/LibraryTestUtil.kt
✅ Files skipped from review due to trivial changes (6)
- data/remote/src/test/resources/api-response/StaffSearchResponse.json
- data/remote/src/test/resources/api-response/AppVersionResponse.json
- data/remote/src/test/resources/api-response/SignInResponse.json
- data/remote/src/test/resources/api-response/ClubListResponse.json
- data/remote/src/test/resources/api-response/DepartmentListResponse.json
- data/remote/src/test/resources/api-response/SearchNoticeResponse.json
🚧 Files skipped from review as they are similar to previous changes (4)
- data/notice/src/main/java/com/ku_stacks/ku_ring/notice/source/DepartmentNoticeMediator.kt
- data/user/src/main/java/com/ku_stacks/ku_ring/user/repository/UserRepositoryImpl.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/notice/response/SearchNoticeResponse.kt
- data/remote/src/main/java/com/ku_stacks/ku_ring/remote/user/response/KuringBotQueryCountResponse.kt
ikseong00
approved these changes
May 11, 2026
mwy3055
approved these changes
May 11, 2026
Member
mwy3055
left a comment
mwy3055
left a comment
There was a problem hiding this comment.
Nothing이 생각보다 큰 의미를 갖네요. 언젠가는 해야 할 작업이었는데 해주셔서 감사합니다.
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
4 participants
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.
요약
Nothing이 아닌Unit으로 설정@Named어노테이션을Qualifier로 대체설명
익성님이 제보해주신 에러를 확인해보니, 응답이
DefaultResponse<Nothing>으로 되어있으면Kotlin-Serialization이 직렬화를 하지 못해 발생하는 것이었습니다.Kotlin-Serialization의 경우:Nothing을 사용하면 해당 필드에 어떠한 값도 할당될 수 없도록 제한한다고 합니다.@Serialzable을 찾을 수 없다는 런타임 에러가 발생한 것으로 파악됩니다.Gson을 사용하는 경우:Nothing을 제너릭으로 넘기면 인스턴스화가 불가능하다고 파악해 그냥null을 할당한다고 합니다.DefaultResponse<Nothing>과Gson을 사용하는 케이스가 있었는데 여기선 에러가 발생하지 않았습니다.앞으로 위와 같은 런타임 오류를 방지하기 위해 직렬화 라이브러리를
Kotlin Serialization으로 통합했습니다.그리고 Nothing 사용이 불가해 응답이 없는 API에 대해선 DefaultResponse 타입으로 응답을 받도록 수정했습니다.
인지하시고 리뷰해주시면 감사하겠습니다.
테스트
Summary by CodeRabbit
릴리스 노트
Refactor
Chores