[Fix] Notification 잘못된 요청 응답 시, 방어적 로직 작성 #49

oungsi2000 · 2026-01-29T14:05:15Z

#️⃣ 이슈 번호

#46

🛠️ 작업 내용

//FestivalNotificationRepository
override suspend fun deleteFestivalNotification(): Result<Unit> {
    val festivalId =
        festivalLocalDataSource.getFestivalId() ?: return Result.failure(
            IllegalStateException(),
        )
    val festivalNotificationId =
        festivalNotificationLocalDataSource.getFestivalNotificationId(festivalId)
    val response =
        festivalNotificationDataSource.deleteFestivalNotification(festivalNotificationId)
    festivalNotificationLocalDataSource.deleteFestivalNotificationId(festivalId)

    return response.toResult()
}

해당 코드에서 festivalNotificationDataSource.deleteFestivalNotification(festivalNotificationId)가 삭제에 실패하더라도 로컬 데이터소스에 삭제를 요청합니다.

이를 mapCatching으로 감싸서 서버에서 삭제를 실패했을 때만 로컬 데이터소스에 삭제를 요청합니다.

다만 로컬 데이터 저장, 삭제에 실패했을 시에는 별도의 복구 로직을 실행하지 않습니다.
처음 SettingViewModel이 생성되었을 때, 서버와 동기화하여 isAllowed, festivalNotificationId를 갱신합니다. (서버 통신 전에는 LocalDatasource의 값을 사용하고 추후 업데이트 합니다)

아직 스낵바 PR이 머지 전이라 스낵바는 뜨지 않습니다.

🙇🏻 중점 리뷰 요청

특히 확인이 필요한 부분, 고민했던 부분 등을 적어주세요.

📸 이미지 첨부 (Optional)

앱 실행 시 서버로부터 사용자의 축제 알림 구독 목록을 가져와 로컬 상태와 동기화하는 기능을 추가했습니다. 이를 통해 다른 기기에서 알림 설정을 변경했거나, 앱을 재설치했을 때도 정확한 알림 수신 동의 상태가 반영되도록 개선했습니다. - **`FestivalNotificationService.kt` 수정:** - `GET /festivals/notifications/{deviceId}` API를 호출하는 `getFestivalNotification` 함수를 추가하여 특정 기기에 등록된 모든 축제 알림 목록을 조회하는 기능을 구현했습니다. - **`FestivalNotificationDataSource.kt` & `FestivalNotificationDataSourceImpl.kt` 수정:** - `FestivalNotificationService`의 변경 사항을 반영하여 `getFestivalNotification` 함수를 인터페이스와 구현체에 추가했습니다. - **`FestivalNotificationRepository.kt` & `FestivalNotificationRepositoryImpl.kt` 수정:** - `syncFestivalNotificationIsAllow` 함수를 새로 추가했습니다. - 이 함수는 서버에서 현재 기기의 알림 구독 목록을 가져온 뒤, 현재 선택된 축제에 대한 구독 여부를 확인하여 로컬 데이터(SharedPreferences)를 갱신합니다. - 구독 중인 경우 `festivalNotificationId`를 로컬에 저장하고, 아닌 경우 삭제합니다. - `saveFestivalNotification`, `deleteFestivalNotification` 등 기존 로직의 예외 처리와 가독성을 개선했습니다. - **`SettingViewModel.kt` 수정:** - `init` 블록에서 `festivalNotificationRepository.syncFestivalNotificationIsAllow()`를 호출하여, 설정 화면 진입 시 알림 동의 상태를 서버와 동기화하도록 로직을 추가했습니다.

기기 변경 또는 앱 재설치 시에도 축제 알림 설정 상태가 유지되도록, 앱 시작 시 서버와 알림 설정 상태를 동기화하는 기능을 추가했습니다. - **`SettingViewModel.kt` 수정:** - ViewModel이 생성될 때(`init` 블록) `festivalNotificationRepository.syncFestivalNotificationIsAllow()`를 호출하여 서버의 알림 등록 상태를 가져와 로컬 데이터와 동기화합니다. - 동기화된 상태를 바탕으로 알림 허용 여부 UI(`uiState`)를 갱신합니다. - **`FestivalNotificationRepositoryTest.kt` 추가:** - `FestivalNotificationRepository`의 동기화 로직에 대한 단위 테스트를 작성했습니다. - **주요 테스트 시나리오:** - 서버에 알림이 등록되어 있을 때, 로컬 DB에 알림 ID와 허용 상태(`true`)를 저장하는지 확인합니다. - 서버에 알림이 등록되어 있지 않을 때, 로컬 DB의 알림 ID를 삭제하고 허용 상태(`false`)를 저장하는지 확인합니다. - 알림 ID 저장 및 삭제 시, 서버 API 호출 성공 여부에 따라 로컬 데이터가 올바르게 처리되는지 검증합니다. - **`RegisteredFestivalNotificationResponse.kt` 추가:** - 서버에서 등록된 축제 알림 목록을 받아오기 위한 새로운 데이터 모델 클래스를 정의했습니다.

coderabbitai · 2026-01-29T14:05:27Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🔍 Trigger a full review

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/46

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

✨ 136.113.208.247/32 (new)
34.170.211.100/32
35.222.179.152/32

Failure to add the new IP will result in interrupted reviews.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

etama123

👍👍👍

parkjiminnnn

테스트코드까지 작성해주셨군요!
고생 하셨습니다 밀러~ 궁금한 점 코멘트 남겨보았습니다.

parkjiminnnn · 2026-02-01T13:18:01Z

app/src/main/java/com/daedan/festabook/data/repository/FestivalNotificationRepositoryImpl.kt

+            .deleteFestivalNotification(festivalNotificationId)
+            .toResult()
+            .mapCatching {
+                festivalNotificationLocalDataSource.deleteFestivalNotificationId(festivalId)
+            }


이를 mapCatching으로 감싸서 서버에서 삭제를 실패했을 때만 로컬 데이터소스에 삭제를 요청합니다.

저 이해가 안간 부분이 있습니다.
PR 본문의 내용과 관련된 코드가 이 부분 같은데 이 코드는 서버에서 삭제를 '실패'했을 때가 아니라 성공 했을 때 로컬 데이터 소스에 삭제를 요청하는게 아닌가용?

아...제가 말을 잘못적었네요.. 제이가 이해하신게 맞습니다 !!
뭔가 생각하면서 적다가 한 번 꼬인 것 같습니다 ㅠㅠㅠㅠ

parkjiminnnn · 2026-02-01T13:20:47Z

app/src/main/java/com/daedan/festabook/data/repository/FestivalNotificationRepositoryImpl.kt

+    override suspend fun syncFestivalNotificationIsAllow(): Result<Boolean> {
+        val deviceId =
+            deviceLocalDataSource.getDeviceId() ?: return Result.failure(
+                IllegalArgumentException(NO_DEVICE_ID_EXCEPTION),
+            )
+        val festivalId =
+            festivalLocalDataSource.getFestivalId() ?: return Result.failure(
+                IllegalArgumentException(NO_FESTIVAL_ID_EXCEPTION),
+            )


이 부분도 로그 찍어놓으면 좋을 것 같아서 현재 KMP 코드에서 제가 run으로 해놨는데 어떻게 생각하시나용?
근데 갑자기 생각해보니까 밀러가 작업하신 이 PR 내용이 아마 KMP에는 하나도 안되어 있는데 이거 두 번 작업하는 방법 밖에 없나..🥲

run으로 하는거 좋은 것 같습니다!
투트랙 전략의 가장 큰 단점인 것 같아요 ㅜㅜ 어쩔 수 없는 것 같습니다

parkjiminnnn · 2026-02-01T13:23:24Z

app/src/main/java/com/daedan/festabook/data/repository/FestivalNotificationRepositoryImpl.kt

+            .mapCatching { response ->
+                val notificationId =
+                    response.find { it.festivalId == festivalId }?.festivalNotificationId
+                val isSubscribed = notificationId != null


isAllowed로 통일 시켜주면 읽을 때 덜 헷갈릴 거 같습니당.

반영했습니다!

parkjiminnnn · 2026-02-01T13:31:04Z

app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.kt

+            festivalNotificationRepository
+                .syncFestivalNotificationIsAllow()
+                .onSuccess {
+                    _isAllowed.emit(it)


_isAllowed.value = it

emit 말고 이렇게 즉시 setter해주는건 어떤가용?
emit(suspend)로 하신 이유가 있으신가 궁금합니다! 제가 놓친 부분이 있을까요?

StateFlow에서는 둘이 똑같이 동작합니다.
하지만 개인적으로 tryEmit보다 emit을 쓰는걸 선호하는데요.
tryemit은 collector 구독하기 이전에 호출되면 즉시 false를 반환하며 이벤트 송신에 실패하지만 emit은 대기하기 떄문입니다.
(StateFlow는 hot flow라서 사실상 둘이 동일하지만요)

parkjiminnnn · 2026-02-01T13:38:16Z

app/src/test/java/com/daedan/festabook/setting/FestivalNotificationRepositoryTest.kt

+        coEvery {
+            deviceLocalDataSource.getDeviceId()
+        } returns 1
+        coEvery {
+            festivalLocalDataSource.getFestivalId()
+        } returns 1


coEvery에 대한 coVerify 검증 코드가 없는 것 같아요!
귀찮으시겠지만 assertAll로 추가해주시면 감사하겠습니다.🙇

코드를 보니 setup이네요. 모킹을 설정해주는 용도인데 이것도 검증해줘야 하나요??

`FestivalNotificationRepositoryImpl`에서 알림 구독 상태를 나타내는 변수명을 `isSubscribed`에서 `isAllowed`로 변경하여 의미를 더 명확하게 했습니다. - **`FestivalNotificationRepositoryImpl.kt` 수정:** - `isSubscribed` 변수명을 `isAllowed`로 변경하여, 서버 응답에서 얻은 알림 허용 여부를 나타내도록 수정했습니다. - 로컬 데이터 소스에 알림 허용 상태를 저장하고, `isAllowed` 값에 따라 알림 ID를 저장하거나 삭제하는 로직을 일관성 있게 변경했습니다.

oungsi2000 added 2 commits January 29, 2026 22:58

oungsi2000 requested review from etama123 and parkjiminnnn January 29, 2026 14:05

etama123 added the Fix label Jan 29, 2026

etama123 assigned oungsi2000 Jan 29, 2026

etama123 approved these changes Jan 30, 2026

View reviewed changes

parkjiminnnn approved these changes Feb 1, 2026

View reviewed changes

oungsi2000 merged commit 0aadfee into develop Feb 4, 2026
3 checks passed

[Fix] Notification 잘못된 요청 응답 시, 방어적 로직 작성 #49

[Fix] Notification 잘못된 요청 응답 시, 방어적 로직 작성 #49

Uh oh!

Conversation

oungsi2000 commented Jan 29, 2026

#️⃣ 이슈 번호

🛠️ 작업 내용

🙇🏻 중점 리뷰 요청

📸 이미지 첨부 (Optional)

Uh oh!

coderabbitai bot commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

etama123 left a comment

Choose a reason for hiding this comment

Uh oh!

parkjiminnnn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Jan 29, 2026 •

edited

Loading