Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ android {

dependencies {
ktlintRuleset(libs.ktlint)
implementation(libs.kotlinx.datetime)
implementation(libs.map.sdk)
implementation(libs.androidx.navigation.compose)
implementation(libs.play.services.location)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.daedan.festabook.data.model.response.notice

import com.daedan.festabook.domain.model.Notice
import kotlinx.datetime.LocalDateTime
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import java.time.LocalDateTime

@Serializable
data class NoticeResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import com.daedan.festabook.domain.model.Notice
import com.daedan.festabook.domain.model.Place
import com.daedan.festabook.domain.model.PlaceDetail
import com.daedan.festabook.domain.model.PlaceDetailImage
import kotlinx.datetime.LocalDateTime
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find PlaceDetailResponse.kt file
fd PlaceDetailResponse.kt

Repository: festabook/android

Length of output: 178


🏁 Script executed:

# Search for createdAt references in the codebase
rg "createdAt" -t kt -t json -B 2 -A 2

Repository: festabook/android

Length of output: 87


🏁 Script executed:

# Look for test fixtures or mock data files
fd -e json | head -20

Repository: festabook/android

Length of output: 133


🏁 Script executed:

# Search for LocalDateTime parsing and serialization configuration
rg "LocalDateTime" -t kt -B 3 -A 3

Repository: festabook/android

Length of output: 87


🏁 Script executed:

# Find PlaceDetailResponse.kt with full path
find . -name "PlaceDetailResponse.kt" -type f

Repository: festabook/android

Length of output: 150


🏁 Script executed:

# Search for createdAt in files using correct rg syntax
rg "createdAt" -g "*.kt" -g "*.json" -B 2 -A 2

Repository: festabook/android

Length of output: 15496


🏁 Script executed:

# Look for test fixtures or sample responses
find . -name "*fixture*" -o -name "*mock*" -o -name "*sample*" | grep -E "\.(json|kt)$"

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Search for LocalDateTime usage and serialization configuration
rg "LocalDateTime" -g "*.kt" -B 3 -A 3

Repository: festabook/android

Length of output: 27862


🏁 Script executed:

# View PlaceDetailResponse.kt file
cat -n "./app/src/main/java/com/daedan/festabook/data/model/response/place/PlaceDetailResponse.kt"

Repository: festabook/android

Length of output: 3832


🏁 Script executed:

# Search for PlaceDetailResponse fixtures or test data
rg -n "PlaceDetailResponse\|PlaceDetail" -g "*.kt" --max-count=20

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Search for JSON files containing place or createdAt examples
find . -name "*.json" -type f -exec grep -l "createdAt" {} \;

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Search for custom deserializers for LocalDateTime
rg -n "KSerializer\|JsonDeserializer\|Deserializer" -g "*.kt" | grep -i "localdatetime\|serializ"

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Check for serialization module configuration
fd -e gradle -e kt | xargs grep -l "serialization\|json" 2>/dev/null | head -10

Repository: festabook/android

Length of output: 938


🏁 Script executed:

# Search for any JSON configuration or serialization setup
rg -n "@Serializable|Json|Serialization" -g "*.kt" | grep -B 2 -A 2 "LocalDateTime" | head -20

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Check for Http client configuration and JSON serialization setup
rg -n "provideHttpClient\|@Provides\|@Singleton" -g "*.kt" -A 10 | grep -E "@|HttpClient|json|Json" | head -30

Repository: festabook/android

Length of output: 43


🏁 Script executed:

# Look for serialization module or JSON configuration
find . -name "*.kt" -path "*/di/*" -o -name "*Module.kt" -o -name "*Configuration.kt" | xargs grep -l "serialization\|json" 2>/dev/null

Repository: festabook/android

Length of output: 123


🏁 Script executed:

# Check build.gradle for serialization settings
find . -name "build.gradle" -o -name "build.gradle.kts" | xargs cat 2>/dev/null | grep -A 5 -B 5 "serialization"

Repository: festabook/android

Length of output: 934


🏁 Script executed:

# View NetworkBindings.kt to check HTTP client and serialization configuration
cat -n "./app/src/main/java/com/daedan/festabook/di/NetworkBindings.kt"

Repository: festabook/android

Length of output: 3759


🏁 Script executed:

# Search for any custom serializers or format configuration
rg -n "Format\|format\|ISO\|format.*LocalDateTime" -g "*.kt" | head -20

Repository: festabook/android

Length of output: 43


LocalDateTime 파싱 시 예외 처리 누락

Line 91에서 LocalDateTime.parse(it.createdAt)를 직접 호출하고 있으나 예외 처리가 없습니다. 서버에서 ISO-8601 형식이 아닌 createdAt을 반환하면 런타임 예외가 발생합니다. LostItemResponsetoLocalDateTime() 확장 함수처럼 try-catch로 보호하거나, 서버 응답 형식이 ISO-8601임을 명시적으로 문서화하시기 바랍니다.

참고: LostItemResponse의 에러 처리 패턴
fun String.toLocalDateTime(): LocalDateTime =
    runCatching {
        LocalDateTime.parse(this)
    }.onFailure {
        Timber.e(it, "LostItem: 날짜 파싱 실패:${it.message}")
    }.getOrElse {
        LocalDateTime.MIN
    }
🤖 Prompt for AI Agents
In
`@app/src/main/java/com/daedan/festabook/data/model/response/place/PlaceDetailResponse.kt`
at line 8, The code calls LocalDateTime.parse(it.createdAt) without handling
parse failures in PlaceDetailResponse mapping; wrap the parse in a safe
converter (like LostItemResponse.toLocalDateTime()) or use runCatching around
LocalDateTime.parse to log the exception (Timber.e) and return a safe fallback
(e.g., LocalDateTime.MIN) so malformed createdAt values don't crash; update the
place mapping code that invokes LocalDateTime.parse to use this safe helper or
inline try-catch/runCatching and ensure the log includes the exception and
context.

import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import java.time.LocalDateTime
import java.time.LocalTime
import java.time.format.DateTimeFormatter
Comment on lines -10 to 12
Copy link
Contributor

Choose a reason for hiding this comment

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

kmp 프로젝트는 kotlinx.LocalDate로 마이그레이션 해놓긴 했는데 나중에 각자 파트 작업할 때 확인 한 번 해주세요!!
틀린 부분 많을거에요 아마두


Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.daedan.festabook.domain.model

import java.time.LocalDateTime
import kotlinx.datetime.LocalDateTime

data class Notice(
val id: Long,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import android.os.Parcelable
import android.util.TypedValue
import android.view.View
import android.view.ViewGroup
import androidx.compose.ui.platform.ComposeView
import androidx.core.view.WindowInsetsCompat
import androidx.fragment.app.Fragment
import com.daedan.festabook.R
import com.daedan.festabook.data.util.ApiResultException
import com.google.android.material.floatingactionbutton.FloatingActionButton
import com.google.android.material.snackbar.Snackbar
import java.io.Serializable

Expand Down Expand Up @@ -63,7 +63,7 @@ fun Activity.showErrorSnackBar(msg: String) {
Snackbar.LENGTH_SHORT,
)
snackBar.setAnchorView(
findViewById<FloatingActionButton>(R.id.bab_menu),
findViewById<ComposeView>(R.id.cv_main),
)
snackBar
.setAction(
Expand Down Expand Up @@ -116,7 +116,7 @@ fun Activity.showSnackBar(msg: String) {
Snackbar.LENGTH_SHORT,
)
snackBar.setAnchorView(
findViewById<FloatingActionButton>(R.id.bab_menu),
findViewById<ComposeView>(R.id.cv_main),
)
snackBar
.setAction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fun showNotificationDeniedSnackbar(
view,
text,
Snackbar.LENGTH_LONG,
).setAnchorView(view.rootView.findViewById(R.id.bab_menu))
).setAnchorView(view.rootView.findViewById(R.id.cv_main))
.setAction(context.getString(R.string.move_to_setting_text)) {
val intent =
Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
package com.daedan.festabook.presentation.home.navigation

import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.foundation.layout.padding
import androidx.compose.ui.Modifier
import androidx.compose.runtime.getValue
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.NavGraphBuilder
import androidx.navigation.compose.composable
import com.daedan.festabook.presentation.home.HomeViewModel
import com.daedan.festabook.presentation.home.component.HomeScreen
import com.daedan.festabook.presentation.main.MainTabRoute
import com.daedan.festabook.presentation.main.MainViewModel
import com.daedan.festabook.presentation.main.component.FirstVisitDialog

fun NavGraphBuilder.homeNavGraph(
padding: PaddingValues,
viewModel: HomeViewModel,
mainViewModel: MainViewModel,
onSubscriptionConfirm: () -> Unit,
onNavigateToExplore: () -> Unit,
) {
composable<MainTabRoute.Home> {
val isFirstVisit by mainViewModel.isFirstVisit.collectAsStateWithLifecycle()
if (isFirstVisit) {
FirstVisitDialog(
onConfirm = { onSubscriptionConfirm() },
onDecline = { mainViewModel.declineAlert() },
)
}
HomeScreen(
modifier = Modifier.padding(padding),
viewModel = viewModel,
onNavigateToExplore = onNavigateToExplore,
)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package com.daedan.festabook.presentation.main

import androidx.compose.runtime.Composable
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.remember
import androidx.navigation.NavDestination.Companion.hasRoute
import androidx.navigation.NavGraph.Companion.findStartDestination
import androidx.navigation.NavHostController
import androidx.navigation.NavOptions
import androidx.navigation.compose.currentBackStackEntryAsState
import androidx.navigation.compose.rememberNavController
import androidx.navigation.navOptions

Expand All @@ -17,19 +17,11 @@ class FestabookNavigator(
@Composable
get() =
navController
.currentBackStackEntryAsState()
.visibleEntries
.collectAsState()
.value
.lastOrNull { it.destination.route != null }
?.destination

private val defaultNavOptions =
navOptions {
popUpTo(navController.graph.findStartDestination().id) {
saveState = true
}
launchSingleTop = true
restoreState = true
}

val currentTab
@Composable
get() =
Expand All @@ -46,10 +38,16 @@ class FestabookNavigator(

val startRoute = MainTabRoute.Home // TODO: Splash와 Explore 연동 시 변경

fun navigateToMainTab(route: FestabookRoute) {
fun navigateToMainTab(tab: FestabookMainTab) {
navController.navigate(
route,
defaultNavOptions,
tab.route,
navOptions {
popUpTo(navController.graph.findStartDestination().id) {
saveState = true
}
launchSingleTop = true
restoreState = true
},
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
package com.daedan.festabook.presentation.main

import com.daedan.festabook.presentation.placeDetail.model.PlaceDetailUiModel
import com.daedan.festabook.presentation.placeMap.model.PlaceUiModel
import kotlinx.serialization.Serializable

@Serializable
sealed interface FestabookRoute {
@Serializable
data object Splash : FestabookRoute

// TODO: PlaceUiModel, PlaceDetailUiModel 생성자에 추가 후 UiModel에 @Serializable 어노테이션 필요
@Serializable
data object PlaceDetail : FestabookRoute
data class PlaceDetail(
val placeUiModel: PlaceUiModel? = null,
val placeDetailUiModel: PlaceDetailUiModel? = null,
) : FestabookRoute
Comment on lines +3 to +16
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find PlaceDetailUiModel and PlaceUiModel files
fd 'PlaceDetailUiModel|PlaceUiModel' -t f --extension kt

Repository: festabook/android

Length of output: 178


🏁 Script executed:

# Try searching using web for information about Jetpack Compose navigation serialization constraints
# and common issues with navigation route payloads
curl -s "https://developer.android.com/" 2>/dev/null | head -20 || echo "Web access limited"

Repository: festabook/android

Length of output: 808


🌐 Web query:

Jetpack Compose Navigation kotlinx.serialization route payload bundle size limits constraints

💡 Result:

In Navigation Compose, “type-safe routes” that use kotlinx.serialization still end up as navigation arguments associated with a NavBackStackEntry (and commonly accessed via toRoute() / SavedStateHandle). (developer.android.com) Those arguments are ultimately stored/transported using Android’s state/Bundles (e.g., SavedStateHandle / SavedStateBundle). (developer.android.com)

Practical size limits / constraints (what will bite you)

  • Binder transaction limit (~1MB, process-wide shared buffer): Any time these arguments/state are sent through Binder (e.g., saving/restoring state, activity/fragment lifecycle handoff, etc.), you can hit TransactionTooLargeException. Android’s docs state the Binder transaction buffer is currently 1MB and shared across transactions in the process. (developer.android.com)
  • Saved-state guidance is far smaller than 1MB: In practice you should keep saved state tiny. A commonly cited guideline is < ~50KB for saved instance state, because the system may need to retain it for a long time for back navigation/process death restore. (stackoverflow.com)
  • Type constraint (Bundle-compatible) still applies: When using SavedStateHandle with navigation, values must be types that can go in a Bundle (or you need another mechanism like a ViewModel/repository). (developer.android.com)

What this means for “serialized route payloads”

  • Serializing a whole object (especially to a string/JSON) and passing it as a nav arg is still constrained by Bundle/Binder limits, and can blow up when state is saved/restored (not necessarily at the moment you call navigate()).
  • Best practice is to pass small identifiers (IDs, keys) in the route, and load the heavy object from a repository/DB/shared ViewModel instead of bundling it.

If you share a concrete example of your route payload (types + approximate size), I can suggest a safer pattern and where it should live (route vs SavedStateHandle vs repository).

Citations:


🌐 Web query:

Android Compose navigation @serializable route performance best practices data classes

💡 Result:

Best practices (performance + reliability) for @Serializable route data classes in Navigation-Compose (type-safe routes)

  1. Keep route arguments small and primitive-ish

    • Prefer passing IDs, enums, small strings, etc., then load the real data from a repository/ViewModel.
    • Type-safe routes are turned into navigation arguments (and can also participate in deep-link URL parameter generation), so large/nested objects (big lists, JSON blobs) increase allocations, encoding/decoding work, and risk hitting size limits. (medium.com)
  2. Parse (toRoute()) once, not on every recomposition

    • If you do backStackEntry.toRoute<MyRoute>() inside a composable body, don’t let it re-run on every recomposition—cache it:
    composable<Product> { entry ->
      val route = remember(entry) { entry.toRoute<Product>() }
      ProductScreen(route.id)
    }

    This follows general Compose guidance to avoid repeated “work” during recomposition. (developer.android.com)

  3. Better: decode in the ViewModel via SavedStateHandle.toRoute()

    • This makes the decode happen once per VM instance and keeps the composable “dumb”:
    class ProductViewModel(savedStateHandle: SavedStateHandle) : ViewModel() {
      val route: Product = savedStateHandle.toRoute()
    }

    Note: SavedStateHandle.toRoute() currently depends on Android Bundle (relevant for tests and also hints it’s not “free”). (medium.com)

  4. Use the right route shape

    • @Serializable object for no-arg destinations, @Serializable data class for destinations with args. (developer.android.com)
  5. If you need custom/complex argument types, use the supported customization points (don’t stringify yourself)

    • Navigation added a typeMap parameter for toRoute() to support custom argument types. (developer.android.com)
  6. Baseline profile / macro-level perf

    • If navigation + serialization is on a critical path (cold start / first screen), also lean on Compose perf tooling and Baseline Profiles. (developer.android.com)

Citations:


라우트에서 전체 모델 객체 전달 방식 개선 필수.

PlaceUiModel과 PlaceDetailUiModel 전체 객체를 라우트 페이로드로 전달하는 것은 안드로이드 네비게이션 모범 사례에 위배됩니다. Navigation Compose의 @Serializable 라우트는 Bundle 인자로 처리되며, Bundle은 Binder 트랜잭션 한계(~1MB 공유 버퍼)와 백 스택 저장 시 크기 제한(권장: <50KB)에 제약을 받습니다. 상태 저장/복원 중 TransactionTooLargeException이 발생할 수 있습니다.

개선 방안: PlaceDetail 라우트에는 ID나 최소 필드만 전달하고, 실제 데이터는 Repository나 ViewModel에서 로드하세요.

🤖 Prompt for AI Agents
In `@app/src/main/java/com/daedan/festabook/presentation/main/FestabookRoute.kt`
around lines 3 - 16, The FestabookRoute.PlaceDetail currently serializes and
passes full PlaceUiModel and PlaceDetailUiModel (which risks
TransactionTooLargeException); change the sealed class to carry only a small
identifier (e.g., placeId: String or Long or minimal primitive key) instead of
PlaceUiModel/PlaceDetailUiModel, remove those model fields from
FestabookRoute.PlaceDetail, and update navigation callers to pass the id; then
fetch the full PlaceUiModel/PlaceDetailUiModel inside the destination via the
Repository or ViewModel (e.g., use the placeId in your ViewModel to load data)
and update any code referencing FestabookRoute.PlaceDetail to use the id-based
payload.


@Serializable
data object Explore : FestabookRoute
Expand Down
Loading