feat: 도서관 남은 좌석 API 구현 #387

boiledEgg-s · 2024-11-19T05:50:51Z

https://kuring.atlassian.net/browse/KURING-224?atlOrigin=eyJpIjoiNWVhZjQ3N2Y3YWVmNGMyZTg2Mjk0MTY0M2M5NDc3OGEiLCJwIjoiaiJ9

요약

도서관 남은 좌석을 가져오는 API을 프로젝트에서 사용하기 위해 Service, Client, Repository를 구현했습니다.
화면에서 테스트 해보기 어려워 테스트 코드도 구현했습니다! 근데 테스트 코드도 "300줄 미만 PR" 컨벤션에 포함이 되는건가요?
Response를 다 쪼개서 구현을 해놨는데 이름 지어주는게 너무 어렵네요..

추가 수정 사항

리뷰 사항을 반영해봤습니다!
추후 도메인 및 dto 수정을 대비한 trailing comma 반영
난독화 문제에 대비한 @SerializedName 추가
runCatching을 사용한 예외처리
예외처리 변경으로 인한 테스트 코드 수정

mwy3055

Retrofit response 클래스를 proguard-rules.pro의 30라인 밑에 하나도 빠짐없이 선언해 주세요. 난독화 때문에 release에서 제대로 파싱하지 못하는 경우가 많았습니다.

그 외에는 코멘트 가볍게 읽어보시면 되겠습니다.

data/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/LibraryService.kt

data/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/request/LibrarySeatRequest.kt

mwy3055 · 2024-11-20T10:52:25Z

data/library/build.gradle.kts

+plugins {
+    kuring("feature")
+    kuringPrimitive("retrofit")
+    kuringPrimitive("test")
+}


이거 이해하기 어렵지 않았나요?

사실 아직 제대로 이해하진 못했어요,,
저 플러그인들에 필요한 라이브러리들이 있겠거니 해서 일단 넣었습니다😅

ㅋㅋㅋㅋㅋ이것도 강의 해줄게요

data/domain/src/main/java/com/ku_stacks/ku_ring/domain/LibraryRoom.kt

mwy3055 · 2024-11-20T10:54:51Z

data/library/src/main/java/com/ku_stacks/ku_ring/library/repository/LibraryRepositoryImpl.kt

+    override suspend fun getRemainingSeats(): List<LibraryRoom> {
+        return try {
+            libraryClient.fetchRoomSeatStatus().toLibraryAreaList()
+        } catch (e: HttpException) {
+            emptyList()
+        }
+    }


예외처리 굿

그런데 여기서 예외처리하는게 맞을까요? UI쪽에서 에러 캐치를 해야 특정 Exception에 대응하는 화면을 만들 수 있는 등의 처리를 할 수 있을 것 같아서요.

음.. 생각해 보니 그렇네요. 여기서는 exception을 그대로 던지거나, exception을 잡고 LibraryException 같은 커스텀 예외를 반환하는 게 나을수도 있겠네요

그게 아니어도 그냥 runCatching 정도로 잡아도 될듯합니다

data/library/build.gradle.kts

l2hyunwoo · 2024-11-21T02:11:35Z

data/library/src/test/java/com/ku_stacks/ku_ring/library/LibraryRepositoryTest.kt

+    private val client: LibraryClient = Mockito.mock(LibraryClient::class.java)
+
+    @Before
+    fun setup() {
+        libraryRepository = LibraryRepositoryImpl(client)
+    }
+
+    @Test
+    fun `get Library Seat Status From Remote Test`() = runTest {
+        val mockLibraryStatus = LibraryTestUtil.mockLibrarySeatResponse()
+
+        Mockito.`when`(client.fetchRoomSeatStatus()).thenReturn(mockLibraryStatus)


굳이 Mock을 사용할 필요가 있을까요? 인터페이스 다형성을 활용해보면 좋을 것 같아요

제가 테스트 코드 작성이 아직 미흡해서 기존 테스트 코드를 좀 차용했습니다..
추후에 좀 더 괜찮은 방법으로 테스트 코드를 작성해보도록 할게요!

넵 좋습니다.

l2hyunwoo · 2024-11-21T02:14:50Z

...ote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibraryRoomBranchResponse.kt

+data class LibraryRoomBranchResponse (
+    val id: Int,
+    @SerializedName("name") val roomBranchName: String,
+    val alias: String,
+    val libraryCode: String,
+    val sortOrder: Int
+)


이거 난독화 걸리면 name 빼고 다른 프로퍼티는 난독화 안전하지 않습니다. 혹시 모르니 수정해주시는걸 권장합니다.

cf: 제 리뷰 코멘트와 같은 맥락입니다

l2hyunwoo · 2024-11-21T02:15:21Z

data/remote/src/main/java/com/ku_stacks/ku_ring/remote/util/NetworkModule.kt

+        return Retrofit.Builder()
+            .client(okHttpClient)
+            .baseUrl("https://library.konkuk.ac.kr/pyxis-api/")
+            .addConverterFactory(GsonConverterFactory.create())


@mwy3055 null 안정성 생각하면 kotlinx.serialization 적용하는것도...?

사실 null 때문에 티켓도 만들긴 했습니다. 오래전부터 인지하고 있던 문제였는데요

kotlinx.serialization 좀 찾아보니까 서버에서 값이 안 오면 변환할 때 exception이 나는 거 같은데, 예측할 수 없는 NPE보다는 이게 나은 거 같네요. 라이브러리 바꿀 게 많네요 ㅋㅋ

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

이것도 차근차근 해봅시다(그리고 하루컷 나고 그러는거 아닌가)

l2hyunwoo · 2024-11-21T02:15:37Z

data/remote/src/test/java/com/ku_stacks/ku_ring/remote/LibraryServiceTest.kt

이 테스트는 작성한 의도가 있을까요?

이 테스트 코드도 기존 코드를 차용한 것 입니다..
제가 이해한 테스트 코드 진행 순서인데, 이게 맞을까요?

initService에서 createService 함수를 통해 테스트용 서비스 변수를 초기화

@test 함수에서 enqueueResponse로 service의 함수를 호출했을 때 반환할 응답으로 json 파일을 지정

mockWebServer의 takeRequest()를 통해 service가 의도대로 동작했는지 확인

assert로 반환된 값이랑 예상 값이랑 비교

mockWebServer에서 response를 테스트 하는것은 저희가 직접 정해주는 값을 넣고 그 값이 객체로 잘 역직렬화되는지를 확인하는 것과 같은데, 이런 테스트를 작성해서 얻을 수 있는 효험이 어떤 것이 있을지 생각해보면 좋을 것 같아요. 추가로 리스폰스가 변경이 되면 json 파일도 변경이 되어야 하는 등 유지보수를 해야하는데 그걸 감안하더라도 의미가 있는지까지 같이!

mwy3055

이정도면 괜찮은 것 같습니다 👍

mwy3055 · 2024-11-23T08:12:49Z

data/remote/src/main/java/com/ku_stacks/ku_ring/remote/library/response/LibrarySeatResponse.kt

 data class LibrarySeatResponse(
+    @SerializedName("success")
    val success: Boolean,
+    @SerializedName("code")
    val code: String,
+    @SerializedName("message")
    val message: String,
+    @SerializedName("data")
    val data: LibraryRoomListResponse
 )


@SerializedName을 자동으로 붙여주는 gradle 플러그인을 만들어 보면..? 그냥 아이디어입니다 ㅎㅎ

boiledEgg-s added 13 commits November 9, 2024 16:13

[추가] LibraryArea 도메인 엔티티 추가

875f775

[추가] 도서관용 레트로핏 모듈 추가

38e4866

[추가] 응답 dto 추가

3eb841d

[추가] LibraryService 및 힐트 모듈 추가

c0a455f

[추가] LibraryClient 및 모듈 추가

ab4ada9

[수정] LibraryService 미사용 라이브러리 제거

6e5ab88

[추가] data:library 모듈 추가

421b6a3

[수정] 도메인 엔티티 이름 수정

435b015

[추가] Library Repository 구현

6b2dad6

[추가] LibraryRepository DI 모듈 구현

f0f9525

[추가] 레포지토리 테스트 코드 작성

7a0e0b3

[추가] develop 풀

8257079

[추가] 서비스 테스트 코드 구현

bd1beba

boiledEgg-s self-assigned this Nov 19, 2024

auto-assign bot requested review from l2hyunwoo and mwy3055 November 19, 2024 05:50

mwy3055 requested changes Nov 20, 2024

View reviewed changes

l2hyunwoo reviewed Nov 21, 2024

View reviewed changes

data/library/build.gradle.kts Show resolved Hide resolved

l2hyunwoo reviewed Nov 21, 2024

View reviewed changes

boiledEgg-s added 5 commits November 23, 2024 12:30

[추가] DTO SerializedName 추가

ff2768c

[수정] Repository 예외처리 수정

b6eecb8

[추가] dto trailing comma 추가

cdbb761

[추가] 엔티티 trailing comma 추가

4db7c8b

[수정] 레포지토리 테스트 수정

80eec3b

boiledEgg-s requested review from l2hyunwoo and mwy3055 November 23, 2024 04:37

mwy3055 approved these changes Nov 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: 도서관 남은 좌석 API 구현 #387

feat: 도서관 남은 좌석 API 구현 #387

boiledEgg-s commented Nov 19, 2024 •

edited

Loading

mwy3055 left a comment

mwy3055 Nov 20, 2024

boiledEgg-s Nov 20, 2024

l2hyunwoo Nov 21, 2024

mwy3055 Nov 20, 2024

l2hyunwoo Nov 21, 2024

mwy3055 Nov 21, 2024

l2hyunwoo Nov 21, 2024

l2hyunwoo Nov 21, 2024

boiledEgg-s Nov 21, 2024

l2hyunwoo Nov 21, 2024

l2hyunwoo Nov 21, 2024

mwy3055 Nov 21, 2024

l2hyunwoo Nov 21, 2024

mwy3055 Nov 21, 2024

l2hyunwoo Nov 21, 2024

l2hyunwoo Nov 21, 2024

boiledEgg-s Nov 21, 2024

l2hyunwoo Nov 21, 2024

mwy3055 left a comment

mwy3055 Nov 23, 2024

feat: 도서관 남은 좌석 API 구현 #387

Are you sure you want to change the base?

feat: 도서관 남은 좌석 API 구현 #387

Conversation

boiledEgg-s commented Nov 19, 2024 • edited Loading

요약

추가 수정 사항

mwy3055 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwy3055 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boiledEgg-s commented Nov 19, 2024 •

edited

Loading