Skip to content

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

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.

Sign up for GitHub

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

Open
oungsi2000 wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99d8ab2
feat(notification): 서버와 알림 구독 상태 동기화 기능 추가
oungsi2000 Jan 29, 2026
b8cb8df
test(Setting): FestivalNotificationRepository 테스트 작성
oungsi2000 Jan 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...va/com/daedan/festabook/data/datasource/remote/festival/FestivalNotificationDataSource.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.daedan.festabook.data.datasource.remote.festival

import com.daedan.festabook.data.datasource.remote.ApiResult
import com.daedan.festabook.data.model.response.festival.FestivalNotificationResponse
import com.daedan.festabook.data.model.response.festival.RegisteredFestivalNotificationResponse

interface FestivalNotificationDataSource {
suspend fun saveFestivalNotification(
Expand All @@ -10,4 +11,6 @@ interface FestivalNotificationDataSource {
): ApiResult<FestivalNotificationResponse>

suspend fun deleteFestivalNotification(festivalNotificationId: Long): ApiResult<Unit>

suspend fun getFestivalNotification(deviceId: Long): ApiResult<List<RegisteredFestivalNotificationResponse>>
}
6 changes: 6 additions & 0 deletions ...om/daedan/festabook/data/datasource/remote/festival/FestivalNotificationDataSourceImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package com.daedan.festabook.data.datasource.remote.festival
import com.daedan.festabook.data.datasource.remote.ApiResult
import com.daedan.festabook.data.model.request.FestivalNotificationRequest
import com.daedan.festabook.data.model.response.festival.FestivalNotificationResponse
import com.daedan.festabook.data.model.response.festival.RegisteredFestivalNotificationResponse
import com.daedan.festabook.data.service.FestivalNotificationService
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
Expand All @@ -28,4 +29,9 @@ class FestivalNotificationDataSourceImpl(
ApiResult.toApiResult {
festivalNotificationService.deleteFestivalNotification(festivalNotificationId)
}

override suspend fun getFestivalNotification(deviceId: Long): ApiResult<List<RegisteredFestivalNotificationResponse>> =
ApiResult.toApiResult {
festivalNotificationService.getFestivalNotification(deviceId)
}
}
16 changes: 16 additions & 0 deletions ...m/daedan/festabook/data/model/response/festival/RegisteredFestivalNotificationResponse.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.daedan.festabook.data.model.response.festival

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable

@Serializable
data class RegisteredFestivalNotificationResponse(
@SerialName("festivalNotificationId")
val festivalNotificationId: Long,
@SerialName("festivalId")
val festivalId: Long,
@SerialName("universityName")
val universityName: String,
@SerialName("festivalName")
val festivalName: String,
)
88 changes: 62 additions & 26 deletions app/src/main/java/com/daedan/festabook/data/repository/FestivalNotificationRepositoryImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import com.daedan.festabook.domain.repository.FestivalNotificationRepository
import dev.zacsweers.metro.AppScope
import dev.zacsweers.metro.ContributesBinding
import dev.zacsweers.metro.Inject
import timber.log.Timber

@ContributesBinding(AppScope::class)
@Inject
Expand All @@ -20,43 +19,73 @@ class FestivalNotificationRepositoryImpl(
private val festivalLocalDataSource: FestivalLocalDataSource,
) : FestivalNotificationRepository {
override suspend fun saveFestivalNotification(): Result<Unit> {
val deviceId = deviceLocalDataSource.getDeviceId()
if (deviceId == null) {
Timber.e("${::FestivalNotificationRepositoryImpl.name}: DeviceId가 없습니다.")
return Result.failure(IllegalStateException())
}
val festivalId = festivalLocalDataSource.getFestivalId()
val deviceId =
deviceLocalDataSource.getDeviceId() ?: return Result.failure(
IllegalArgumentException(NO_DEVICE_ID_EXCEPTION),
)
val festivalId =
festivalLocalDataSource.getFestivalId() ?: return Result.failure(
IllegalArgumentException(NO_FESTIVAL_ID_EXCEPTION),
)

val result =
festivalId?.let {
festivalNotificationDataSource
.saveFestivalNotification(
festivalId = it,
deviceId = deviceId,
).toResult()
}
?: throw IllegalArgumentException("${::FestivalNotificationRepositoryImpl.javaClass.simpleName}festivalId가 null 입니다.")
return result
.mapCatching {
return festivalNotificationDataSource
.saveFestivalNotification(
festivalId = festivalId,
deviceId = deviceId,
).toResult()
.mapCatching { response ->
festivalNotificationLocalDataSource.saveFestivalNotificationId(
festivalId,
it.festivalNotificationId,
response.festivalNotificationId,
)
}
}

override suspend fun deleteFestivalNotification(): Result<Unit> {
val festivalId =
festivalLocalDataSource.getFestivalId() ?: return Result.failure(
IllegalStateException(),
)
festivalLocalDataSource.getFestivalId()
?: return Result.failure(IllegalStateException(NO_FESTIVAL_ID_EXCEPTION))
val festivalNotificationId =
festivalNotificationLocalDataSource.getFestivalNotificationId(festivalId)
val response =
festivalNotificationDataSource.deleteFestivalNotification(festivalNotificationId)
festivalNotificationLocalDataSource.deleteFestivalNotificationId(festivalId)
return festivalNotificationDataSource
.deleteFestivalNotification(festivalNotificationId)
.toResult()
.mapCatching {
festivalNotificationLocalDataSource.deleteFestivalNotificationId(festivalId)
}
Comment on lines +51 to +55
Copy link
Contributor

@parkjiminnnn parkjiminnnn Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

}

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),
)
Comment on lines +58 to +66
Copy link
Contributor

@parkjiminnnn parkjiminnnn Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


return response.toResult()
return festivalNotificationDataSource
.getFestivalNotification(deviceId)
.toResult()
.mapCatching { response ->
val notificationId =
response.find { it.festivalId == festivalId }?.festivalNotificationId
val isSubscribed = notificationId != null
Copy link
Contributor

@parkjiminnnn parkjiminnnn Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

festivalNotificationLocalDataSource.saveFestivalNotificationIsAllowed(
festivalId,
isSubscribed,
)
if (isSubscribed) {
festivalNotificationLocalDataSource.saveFestivalNotificationId(
festivalId,
notificationId,
)
} else {
festivalNotificationLocalDataSource.deleteFestivalNotificationId(festivalId)
}
isSubscribed
}
}

override fun getFestivalNotificationIsAllow(): Boolean {
Expand All @@ -72,4 +101,11 @@ class FestivalNotificationRepositoryImpl(
)
}
}

companion object {
private val NO_FESTIVAL_ID_EXCEPTION =
"${::FestivalNotificationRepositoryImpl.name}: FestivalId가 없습니다."
private val NO_DEVICE_ID_EXCEPTION =
"${::FestivalNotificationRepositoryImpl.name}: DeviceId가 없습니다."
}
}
7 changes: 7 additions & 0 deletions app/src/main/java/com/daedan/festabook/data/service/FestivalNotificationService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package com.daedan.festabook.data.service

import com.daedan.festabook.data.model.request.FestivalNotificationRequest
import com.daedan.festabook.data.model.response.festival.FestivalNotificationResponse
import com.daedan.festabook.data.model.response.festival.RegisteredFestivalNotificationResponse
import retrofit2.Response
import retrofit2.http.Body
import retrofit2.http.DELETE
import retrofit2.http.GET
import retrofit2.http.POST
import retrofit2.http.Path

Expand All @@ -19,4 +21,9 @@ interface FestivalNotificationService {
suspend fun deleteFestivalNotification(
@Path("festivalNotificationId") id: Long,
): Response<Unit>

@GET("festivals/notifications/{deviceId}")
suspend fun getFestivalNotification(
@Path("deviceId") id: Long,
): Response<List<RegisteredFestivalNotificationResponse>>
}
2 changes: 2 additions & 0 deletions app/src/main/java/com/daedan/festabook/domain/repository/FestivalNotificationRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ interface FestivalNotificationRepository {

suspend fun deleteFestivalNotification(): Result<Unit>

suspend fun syncFestivalNotificationIsAllow(): Result<Boolean>

fun getFestivalNotificationIsAllow(): Boolean

fun setFestivalNotificationIsAllow(isAllowed: Boolean)
Expand Down
10 changes: 10 additions & 0 deletions app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ class SettingViewModel(

val success = _success.asSharedFlow()

init {
viewModelScope.launch {
festivalNotificationRepository
.syncFestivalNotificationIsAllow()
.onSuccess {
_isAllowed.emit(it)
Copy link
Contributor

@parkjiminnnn parkjiminnnn Feb 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

_isAllowed.value = it

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

}
}
}

fun notificationAllowClick() {
if (!_isAllowed.value) {
viewModelScope.launch {
Expand Down
Loading