-
-
Notifications
You must be signed in to change notification settings - Fork 317
Add Android TV variant #2682
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.
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
base: main
Are you sure you want to change the base?
Add Android TV variant #2682
Changes from 34 commits
046427b
ea2cd8d
8c5e6b1
43b5b9a
33e3893
c61312f
c992906
8e56a7e
3f5fb4e
8a6d7ad
ecf2e00
40aebba
4a57b25
60cb86e
82fdfde
4074269
7f5c60e
3fab82c
79590e0
fbab751
c3fd2d4
3aff103
d7c916c
e3dba87
612170a
60f5640
4980f75
13f8914
7df8a8a
342d4e6
212b033
301c354
59ef6e6
387e9cc
59d2c9c
0f9eb43
a8db922
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||||||||||||||
| name: 📺 Android TV CI | ||||||||||||||||||
|
|
||||||||||||||||||
| # rethink-tv fork: builds the `tv` Gradle flavor on every push and PR. | ||||||||||||||||||
| # This workflow lives alongside upstream's `android.yml` (which builds | ||||||||||||||||||
| # the phone variant) and does not modify it, keeping the upstream-sync | ||||||||||||||||||
| # diff minimal. | ||||||||||||||||||
|
|
||||||||||||||||||
| on: | ||||||||||||||||||
| push: | ||||||||||||||||||
| branches: [ "main" ] | ||||||||||||||||||
| pull_request: | ||||||||||||||||||
| branches: [ "main" ] | ||||||||||||||||||
| # Allow ad-hoc validation of feature branches (e.g. `for-upstream`, | ||||||||||||||||||
| # `upstream-sync`) without changing the regular trigger surface. | ||||||||||||||||||
| workflow_dispatch: | ||||||||||||||||||
|
|
||||||||||||||||||
| jobs: | ||||||||||||||||||
| build-tv: | ||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||
|
Comment on lines
+17
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [security · medium] Suggestion:
Suggested change
|
||||||||||||||||||
| env: | ||||||||||||||||||
|
Comment on lines
+19
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [other · low] Suggestion:
Suggested change
|
||||||||||||||||||
| VARIANT: assembleFdroidTvDebug | ||||||||||||||||||
| steps: | ||||||||||||||||||
| - name: 🥏 Checkout | ||||||||||||||||||
| uses: actions/checkout@v6 | ||||||||||||||||||
|
Comment on lines
+23
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [other · low] Suggestion:
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| - name: ☕️ JDKv17 | ||||||||||||||||||
| uses: actions/setup-java@v5 | ||||||||||||||||||
| with: | ||||||||||||||||||
| java-version: '17' | ||||||||||||||||||
| distribution: 'temurin' | ||||||||||||||||||
| cache: gradle | ||||||||||||||||||
|
|
||||||||||||||||||
| - name: ⚡️ +x gradlew | ||||||||||||||||||
| run: chmod +x gradlew | ||||||||||||||||||
|
|
||||||||||||||||||
| - name: 🚂 Assemble TV variant | ||||||||||||||||||
| run: | | ||||||||||||||||||
| ./gradlew \ | ||||||||||||||||||
| ${VARIANT} \ | ||||||||||||||||||
| --info \ | ||||||||||||||||||
| --stacktrace \ | ||||||||||||||||||
| --warning-mode all | ||||||||||||||||||
| env: | ||||||||||||||||||
| VARIANT: ${{ env.VARIANT }} | ||||||||||||||||||
|
|
||||||||||||||||||
| - name: 📦 Upload TV debug APK | ||||||||||||||||||
| if: success() | ||||||||||||||||||
| uses: actions/upload-artifact@v4 | ||||||||||||||||||
| with: | ||||||||||||||||||
| name: rethink-tv-fdroid-debug | ||||||||||||||||||
| path: app/build/outputs/apk/fdroidTv/debug/*.apk | ||||||||||||||||||
| if-no-files-found: warn | ||||||||||||||||||
| retention-days: 14 | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,10 @@ plugins { | |||||
| id 'com.android.application' | ||||||
| id 'com.google.devtools.ksp' | ||||||
| id 'kotlin-android' | ||||||
| // rethink-tv fork: Compose Compiler plugin for the `tv` flavor's | ||||||
| // Compose-for-TV UI. Safe to apply project-wide — phone variants | ||||||
| // contain no @Composable and the plugin then no-ops. | ||||||
| id 'org.jetbrains.kotlin.plugin.compose' | ||||||
| } | ||||||
|
|
||||||
| // apply Google Services and Firebase Crashlytics plugins conditionally | ||||||
|
|
@@ -219,6 +223,11 @@ android { | |||||
| buildFeatures { | ||||||
| viewBinding true | ||||||
| buildConfig true | ||||||
| // rethink-tv fork: Compose UI for the `tv` flavor. Project-wide so | ||||||
| // the Compose Compiler picks up @Composable declarations under | ||||||
| // `app/src/tv/`. Phone variants have no @Composable code so this | ||||||
| // costs nothing at compile time. | ||||||
| compose true | ||||||
| } | ||||||
|
|
||||||
| compileOptions { | ||||||
|
|
@@ -258,10 +267,84 @@ android { | |||||
| versionName = gitVersion | ||||||
| vectorDrawables.useSupportLibrary = true | ||||||
| } | ||||||
| // Android TV flavor (rethink-tv fork addition). | ||||||
| // Peer of `full` in the releaseType dimension. Inherits the entire | ||||||
| // upstream `app/src/full/` source set (java + res + manifest) so the | ||||||
| // engine and shared `app/src/main/` code compile unchanged. Phase 2 | ||||||
| // intentionally reuses upstream's phone UI to validate the build / | ||||||
| // distribution pipeline; the dedicated Compose-for-TV navigation host | ||||||
| // is introduced in a subsequent phase (`tv-ux-dashboard`). The TV | ||||||
| // override surface lives in `app/src/tv/` and is layered on top of | ||||||
| // `full/` via the `sourceSets` block below. See README for the fork | ||||||
| // notice and `docs/` for the upstream-sync workflow. | ||||||
| tv { | ||||||
| dimension "releaseType" | ||||||
| applicationIdSuffix ".tv" | ||||||
| versionCode = getVersionCode() | ||||||
| versionName = gitVersion | ||||||
| vectorDrawables.useSupportLibrary = true | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // The `tv` flavor inherits the entire `full` source set so that engine / | ||||||
| // service / view-model classes upstream keeps in `app/src/full/` (which | ||||||
| // shared `app/src/main/` code references via constants like `ID_WG_BASE`) | ||||||
| // are visible to the `tv` variants. The `app/src/tv/` source set then | ||||||
| // layers TV-specific resources (e.g. `strings.xml` to rename the app to | ||||||
| // "Rethink TV") and a TV launcher Activity on top via the standard | ||||||
| // Android resource / manifest overlay rules. `src/tv/AndroidManifest.xml` | ||||||
| // is the flavor's primary manifest — see that file for the rationale | ||||||
| // (it currently mirrors full's manifest and adds the TV launcher). | ||||||
| sourceSets { | ||||||
| tv { | ||||||
| java.srcDirs += ['src/full/java'] | ||||||
| res.srcDirs += ['src/full/res'] | ||||||
| } | ||||||
|
Comment on lines
+382
to
+385
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [bug · medium] |
||||||
| } | ||||||
| lint { | ||||||
| abortOnError true | ||||||
| } | ||||||
|
|
||||||
| // ------------------------------------------------------------------ | ||||||
| // rethink-tv fork: env-var-based signing for the `release` build type | ||||||
| // when used to ship TV builds via GitHub Actions. | ||||||
| // | ||||||
| // When the CI workflow `.github/workflows/android-tv-release.yml` | ||||||
| // injects the `TV_RELEASE_KS_*` secrets, we: | ||||||
| // 1. create a `tvRelease` signing config that reads those env vars | ||||||
| // (same shape as upstream's `alpha` signing config — see line 101 | ||||||
| // of this file) | ||||||
| // 2. attach it to the `release` build type so that | ||||||
| // `assembleFdroidTvRelease` produces a signed APK suitable for | ||||||
| // attaching to a GitHub Release. | ||||||
| // | ||||||
| // When the env vars are NOT set (local builds, `🫣 Android CI`, the | ||||||
| // `📺 Android TV CI` debug job, manual unsigned-release dry runs): | ||||||
| // - upstream's existing `keystore.properties`-based `config` signing | ||||||
| // config remains the only signing config that touches `release`, | ||||||
| // preserving 100% upstream behaviour for phone builds. | ||||||
| // | ||||||
| // This block intentionally does not modify upstream's existing signing | ||||||
| // configs or build types — it only conditionally appends. | ||||||
| // ------------------------------------------------------------------ | ||||||
| def tvKsAlias = System.getenv("TV_RELEASE_KS_ALIAS") | ||||||
| def tvKsPassphrase = System.getenv("TV_RELEASE_KS_PASSPHRASE") | ||||||
| def tvKsFile = System.getenv("TV_RELEASE_KS_FILE") | ||||||
| def tvKsStorePassphrase = System.getenv("TV_RELEASE_KS_STORE_PASSPHRASE") | ||||||
| if (tvKsAlias && tvKsPassphrase && tvKsFile && tvKsStorePassphrase) { | ||||||
| signingConfigs { | ||||||
| tvRelease { | ||||||
| keyAlias tvKsAlias | ||||||
| keyPassword tvKsPassphrase | ||||||
| storeFile file(tvKsFile) | ||||||
| storePassword tvKsStorePassphrase | ||||||
| } | ||||||
| } | ||||||
| buildTypes.release.signingConfig = signingConfigs.tvRelease | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [bug · medium] |
||||||
| println("rethink-tv: TV_RELEASE_KS_* env vars detected; 'release' build type will be signed with signingConfigs.tvRelease") | ||||||
| } else { | ||||||
| println("rethink-tv: TV_RELEASE_KS_* env vars NOT set; 'release' builds will be unsigned (upstream default)") | ||||||
| } | ||||||
|
Comment on lines
+413
to
+430
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Locate AGP version and any existing per-variant signing assignments for reference.
rg -n "com.android.tools.build:gradle" build.gradle
rg -n "signingConfig" --glob '*.gradle' -C2Repository: celzero/rethink-app Length of output: 218 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- app/build.gradle signing/flavor context ---'
sed -n '360,450p' app/build.gradle
printf '%s\n' '--- Android plugin and variant API references ---'
rg -n -C3 'com\.android\.tools\.build:gradle|androidComponents|applicationVariants|variant\.|flavorDimensions|productFlavors|signingConfig' \
build.gradle settings.gradle gradle.properties app --glob '*.gradle' --glob '*.gradle.kts' || true
printf '%s\n' '--- relevant flavor declarations ---'
rg -n -C8 'fdroidTv|fdroidFull|playFull|websiteFull|productFlavors|flavorDimensions' app/build.gradleRepository: celzero/rethink-app Length of output: 11190 🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- complete flavor and build-type declarations ---'
sed -n '220,370p' app/build.gradle
printf '%s\n' '--- workflow variant and signing environment usage ---'
rg -n -C5 'assemble|bundle|TV_RELEASE_KS_|fdroidTv|playFull|websiteFull|fdroidFull' .github app README.md docs --glob '*.yml' --glob '*.yaml' --glob '*.gradle' --glob '*.md' 2>/dev/null || true
printf '%s\n' '--- Gradle wrapper and local AGP-related sources ---'
cat gradle/wrapper/gradle-wrapper.properties 2>/dev/null || true
find . -path '*/node_modules' -prune -o -path '*/build' -prune -o -type f \\( -iname '*variant*' -o -iname '*signing*' \\) -print 2>/dev/null | head -100Repository: celzero/rethink-app Length of output: 16927 🌐 Web query:
💡 Result: In the Android Gradle Plugin (AGP) 8.13, the Variant API allows you to access and configure signing properties for specific variants [1][2]. However, the SigningConfig object provided by the Variant API is primarily intended for inspecting or modifying specific signing properties (such as enabling/disabling V1, V2, V3, or V4 signatures) rather than assigning an entirely new signing configuration object created in the DSL [3][1]. To apply a different signing configuration to a specific variant in your build.gradle (Groovy) file, you should typically use the android {... } block to define your signing configs and then assign them to the desired build types or product flavors within the Android DSL [4]. If you need to programmatically modify the signing configuration of a variant at configuration time using the Variant API, you access it through the androidComponents extension [1][5]: androidComponents { onVariants(selector.all, { variant -> // Access the signingConfig property of the variant def signing = variant.signingConfig // Example: Programmatically enable or disable signature schemes signing.enableV4Signing.set(true) }) } It is important to note that the signingConfig property on the ApplicationVariant object returns a SigningConfig instance that is initialized by the corresponding DSL element [1][6]. If you intend to change the actual store file or passwords for a specific variant, the standard approach remains defining multiple signingConfigs in the android { signingConfigs {... } } block and assigning the appropriate one to the relevant buildType or flavor [4]. The Variant API's setConfig function, marked as Citations:
🌐 Web query:
💡 Result: In Android development, signingConfig is a Domain Specific Language (DSL) object used within Gradle build scripts to configure the signing settings for your application, such as APKs and bundles [1][2]. It allows you to specify the credentials and configurations required to sign your application, which is a mandatory step for publishing to the Google Play Store and ensuring the integrity of your app [2]. Key properties configurable through a signingConfig include: - storeFile: The location of the keystore file [1][2]. - storePassword: The password for the keystore [1][2]. - keyAlias: The alias of the specific key used for signing [1][2]. - keyPassword: The password for the specific key [1][2]. - storeType: The type of the keystore (e.g., JKS or PKCS12) [1][2]. - Signing Schemes: You can explicitly enable or disable different APK signature schemes, including v1 (JAR signing), v2, v3, and v4 signing [1][3][4]. In your build.gradle (or build.gradle.kts) file, you define these configurations within the android block. Typically, you create a signingConfigs block to define one or more named configurations, which can then be assigned to specific build types or product flavors [2][5]. Example configuration structure: android { signingConfigs { release { storeFile file("my-release-key.jks") storePassword "password" keyAlias "my-alias" keyPassword "password" } } buildTypes { release { signingConfig signingConfigs.release } } } Citations:
🌐 Web query:
💡 Result: The Android Gradle Plugin uses a signingConfigs block within the android extension to define signing configurations for an application. While the provided documentation for the Variant API does not directly detail a setConfig method for signing, standard configuration is typically handled through the build.gradle (Groovy) or build.gradle.kts (Kotlin) DSL. To configure signing in your build file, define the configurations within the android block: android { signingConfigs { release { storeFile file('my-release-key.jks') storePassword 'password' keyAlias 'my-alias' keyPassword 'password' } } buildTypes { release { signingConfig signingConfigs.release } } } In this configuration, the signingConfigs block creates named signing definitions, which are then assigned to specific build types using the signingConfig property. If you are attempting to programmatically set or modify configurations via the Variant API, note that the API focuses on providing access to variant properties, and direct mutation of signing configurations via a setConfig method is not standard usage for the Android Gradle Plugin. Citations:
🏁 Script executed: #!/bin/bash
set -u
printf '%s\n' '--- exact product flavor declarations ---'
sed -n '327,365p' app/build.gradle
printf '%s\n' '--- exact existing signing declarations ---'
sed -n '135,175p' app/build.gradle
sed -n '225,262p' app/build.gradle
printf '%s\n' '--- workflow files mentioning the release workflow ---'
fd -t f -i 'android-tv-release' .github 2>/dev/null | xargs -r -n1 sh -c 'echo "--- $0"; cat -n "$0"'Repository: celzero/rethink-app Length of output: 5614 Scope
🤖 Prompt for AI Agents |
||||||
| } | ||||||
|
|
||||||
| configurations { | ||||||
|
|
@@ -405,6 +488,105 @@ dependencies { | |||||
|
|
||||||
| fullImplementation 'androidx.biometric:biometric:1.1.0' | ||||||
|
|
||||||
| // ------------------------------------------------------------------ | ||||||
| // rethink-tv fork: Android TV flavor (`tv`) dependencies. | ||||||
| // The `tv` flavor's source set inherits `app/src/full/` (configured in | ||||||
| // the `android.sourceSets` block above), so it needs the same UI / | ||||||
| // runtime dependencies that `fullImplementation` provides. This block | ||||||
| // mirrors them as `tvImplementation` declarations. It is intentionally | ||||||
| // append-only and does not modify upstream's existing declarations, | ||||||
| // keeping the upstream-sync diff minimal. | ||||||
| // | ||||||
| // Note: `firestackDependency()` is NOT mirrored here because firestack | ||||||
| // is scoped by `releaseChannel` (play / fdroid / website) above, and | ||||||
| // the `tv` flavor lives in the orthogonal `releaseType` dimension — | ||||||
| // each combined variant (e.g. `fdroidTv`) already picks up firestack | ||||||
| // via `fdroidImplementation`. Mirroring here would double-add it. | ||||||
| // ------------------------------------------------------------------ | ||||||
| tvImplementation 'org.jetbrains.kotlin:kotlin-stdlib-jdk8:2.1.20' | ||||||
| tvImplementation 'androidx.appcompat:appcompat:1.7.1' | ||||||
| tvImplementation 'androidx.core:core-ktx:1.17.0' | ||||||
| tvImplementation 'androidx.constraintlayout:constraintlayout:2.2.1' | ||||||
| tvImplementation 'androidx.swiperefreshlayout:swiperefreshlayout:1.1.0' | ||||||
| tvImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2' | ||||||
| tvImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-android:1.10.2' | ||||||
| tvImplementation 'androidx.lifecycle:lifecycle-viewmodel-ktx:2.9.4' | ||||||
| tvImplementation 'androidx.lifecycle:lifecycle-runtime-ktx:2.9.4' | ||||||
| tvImplementation 'androidx.fragment:fragment-ktx:1.8.9' | ||||||
| tvImplementation 'androidx.viewpager2:viewpager2:1.1.0' | ||||||
| tvImplementation 'com.squareup.okhttp3:okhttp:5.3.2' | ||||||
| tvImplementation 'com.squareup.okhttp3:okhttp-dnsoverhttps:5.3.2' | ||||||
| tvImplementation 'com.squareup.okhttp3:logging-interceptor:5.3.2' | ||||||
| tvImplementation 'com.squareup.retrofit2:retrofit:3.0.0' | ||||||
| tvImplementation 'com.squareup.retrofit2:converter-gson:3.0.0' | ||||||
| tvImplementation('com.github.bumptech.glide:glide:5.0.5') { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [bug · high] Suggestion:
Suggested change
|
||||||
| exclude group: 'glide-parent' | ||||||
| } | ||||||
| tvImplementation('com.github.bumptech.glide:okhttp3-integration:5.0.5') { | ||||||
| exclude group: 'glide-parent' | ||||||
| } | ||||||
| kspTv 'com.github.bumptech.glide:compiler:5.0.5' | ||||||
| tvImplementation 'com.facebook.shimmer:shimmer:0.5.0' | ||||||
| tvImplementation 'com.github.kirich1409:viewbindingpropertydelegate:1.5.9' | ||||||
| tvImplementation 'com.github.kirich1409:viewbindingpropertydelegate-noreflection:1.5.9' | ||||||
| tvImplementation 'androidx.navigation:navigation-fragment-ktx:2.9.6' | ||||||
| tvImplementation 'androidx.navigation:navigation-ui-ktx:2.9.6' | ||||||
| tvImplementation 'androidx.biometric:biometric:1.1.0' | ||||||
| tvImplementation 'com.journeyapps:zxing-android-embedded:4.3.0' | ||||||
| tvImplementation 'com.simplecityapps:recyclerview-fastscroll:2.0.1' | ||||||
| tvImplementation 'nl.dionsegijn:konfetti-xml:2.0.5' | ||||||
|
|
||||||
| // ------------------------------------------------------------------ | ||||||
| // rethink-tv fork: Compose-for-TV stack for the `tv` flavor. | ||||||
| // The Compose Compiler plugin (applied project-wide so a single | ||||||
| // `id 'org.jetbrains.kotlin.plugin.compose'` covers every variant) | ||||||
| // performs a classpath check at compile time on every applied variant, | ||||||
| // including phone variants that contain zero @Composable code. To | ||||||
| // satisfy that check without pulling the full UI stack into phone | ||||||
| // builds, we expose only the BOM and the runtime artifact to all | ||||||
| // variants; the rest (ui, foundation, material3, tv-material, | ||||||
| // navigation-compose, etc.) stays scoped to `tvImplementation`. | ||||||
| // Cost to phone variants: ~250 KB of unused Compose runtime classes, | ||||||
| // which never get loaded because phone code never invokes @Composable. | ||||||
| // ------------------------------------------------------------------ | ||||||
| implementation platform('androidx.compose:compose-bom:2024.12.01') | ||||||
| implementation 'androidx.compose.runtime:runtime' | ||||||
|
|
||||||
| tvImplementation platform('androidx.compose:compose-bom:2024.12.01') | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [maintainability · low] |
||||||
| tvImplementation 'androidx.compose.ui:ui' | ||||||
| tvImplementation 'androidx.compose.ui:ui-tooling-preview' | ||||||
| tvImplementation 'androidx.compose.foundation:foundation' | ||||||
| tvImplementation 'androidx.compose.runtime:runtime-livedata' | ||||||
| tvImplementation 'androidx.compose.material3:material3' | ||||||
| // TV-styled Material 3 components (focusable Surface / Button / | ||||||
| // Card with the focus borders Android TV users expect). | ||||||
| tvImplementation 'androidx.tv:tv-material:1.0.0' | ||||||
| tvImplementation 'androidx.activity:activity-compose:1.9.3' | ||||||
| tvImplementation 'androidx.lifecycle:lifecycle-runtime-compose:2.8.7' | ||||||
| tvImplementation 'androidx.lifecycle:lifecycle-viewmodel-compose:2.8.7' | ||||||
| // Koin's Compose extensions (`koinInject` / `koinViewModel`) so the TV | ||||||
| // UI can pull `PersistentState` and other injected singletons without | ||||||
| // hand-wiring KoinComponent into every Composable. | ||||||
| tvImplementation 'io.insert-koin:koin-androidx-compose:4.1.1' | ||||||
| // Compose Navigation drives the TV NavHost. Used in tandem with the | ||||||
| // tv-material NavigationDrawer to host every TV destination | ||||||
| // (Home / DNS / Firewall / Apps / Proxy / Logs / Stats / Settings) | ||||||
| // inside a single Activity, avoiding new <activity> declarations in | ||||||
| // the TV-flavor manifest. | ||||||
| tvImplementation 'androidx.navigation:navigation-compose:2.8.5' | ||||||
| // Material Icons Extended supplies the leanback iconography | ||||||
| // (Shield, Dns, Apps, Hub, Article, Insights, Settings, Home) that | ||||||
| // the TV nav rail needs. Scoped to `tvImplementation` so phone | ||||||
| // variants never pull the ~3 MB icon font. | ||||||
| tvImplementation 'androidx.compose.material:material-icons-extended' | ||||||
| // Paging-Compose provides `collectAsLazyPagingItems()` and the | ||||||
| // `items()` overload that consume PagingSource directly — the Logs | ||||||
| // destination feeds it ConnectionTrackerDAO.getConnectionTrackerByName() | ||||||
| // verbatim, so we don't have to recreate the upstream paging | ||||||
| // pipeline. Scoped to tvImplementation; phone variants use their | ||||||
| // existing Fragment + Paging-Runtime adapter. | ||||||
| tvImplementation 'androidx.paging:paging-compose:3.3.5' | ||||||
|
|
||||||
| playImplementation 'com.google.android.play:app-update:2.1.0' | ||||||
| playImplementation 'com.google.android.play:app-update-ktx:2.1.0' | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,7 +71,30 @@ abstract class AppDatabase : RoomDatabase() { | |
| // Otherwise, WRITE_AHEAD_LOGGING will be used. | ||
| // Ref: | ||
| // https://developer.android.com/reference/android/arch/persistence/room/RoomDatabase.JournalMode#automatic | ||
| fun buildDatabase(context: Context) = | ||
| fun buildDatabase(context: Context): AppDatabase { | ||
| return try { | ||
| val db = newBuilder(context).build() | ||
| // Force the DB to open now so that any | ||
| // `Room cannot verify the data integrity` failure (e.g. an older | ||
| // install whose schema, after running the registered migration | ||
| // chain, no longer matches the current entity hashes) surfaces | ||
| // here instead of at the first VPN-service DAO access, which | ||
| // would crash BraveVPNService on every cold start. | ||
| db.openHelper.writableDatabase | ||
| db | ||
| } catch (e: IllegalStateException) { | ||
| val msg = e.message.orEmpty() | ||
| if ("Room cannot verify" in msg || "data integrity" in msg) { | ||
| Logger.w(LOG_TAG_APP_DB, "schema mismatch; wiping db and recreating: $msg") | ||
| context.deleteDatabase(DATABASE_NAME) | ||
| newBuilder(context).build() | ||
| } else { | ||
| throw e | ||
| } | ||
| } | ||
|
Comment on lines
+121
to
+131
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -e
file="app/src/main/java/com/celzero/bravedns/database/AppDatabase.kt"
printf '%s\n' '--- file outline ---'
ast-grep outline "$file" --lang kotlin || true
printf '%s\n' '--- target lines ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- related database and corruption checks ---'
rg -n -i "isValidSQLiteFile|SQLiteException|SQLiteDatabaseCorruptException|Room cannot verify|data integrity|deleteDatabase|writableDatabase|Schema mismatch" app/src test* 2>/dev/null || true
printf '%s\n' '--- candidate tests ---'
git ls-files | rg -i '(AppDatabase|database|sqlite|room).*(test|spec)|((test|spec).*(AppDatabase|database|sqlite|room))' || trueRepository: celzero/rethink-app Length of output: 12862 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- LogDatabase exception handling ---'
sed -n '120,180p' app/src/main/java/com/celzero/bravedns/database/LogDatabase.kt
printf '%s\n' '--- Android SQLite imports and exception usage ---'
rg -n -C 4 "android\.database\.sqlite\.(SQLiteException|SQLiteDatabaseCorruptException)|catch \(.*SQLite" app/src/main/java app/src/test app/src/androidTest 2>/dev/null || true
printf '%s\n' '--- dependency declarations ---'
rg -n -i "room|sqlite|androidx\.sqlite|compileSdk|minSdk|targetSdk" --glob '!*build*' --glob '*.gradle' --glob '*.gradle.kts' .Repository: celzero/rethink-app Length of output: 9874 🌐 Web query:
💡 Result: The SQLiteDatabaseCorruptException is a specialized exception in the Android framework that indicates the underlying SQLite database file is corrupt [1][2][3]. Class Hierarchy The class hierarchy for SQLiteDatabaseCorruptException is as follows [2][4][3][5]: java.lang.Object ↳ java.lang.Throwable ↳ java.lang.Exception ↳ java.lang.RuntimeException ↳ android.database.SQLException ↳ android.database.sqlite.SQLiteException ↳ android.database.sqlite.SQLiteDatabaseCorruptException As shown above, SQLiteDatabaseCorruptException extends SQLiteException, which is a RuntimeException [3][5]. It is one of many specific subclasses of SQLiteException that provides granular error reporting for database operations [3][5]. Handling Corruption When SQLite reports database corruption, the Android framework allows developers to define custom handling logic via the DatabaseErrorHandler interface [6][7]. An implementation of this interface can be provided when opening a database; the onCorruption method is then invoked by the system if corruption is detected [6][7]. Regarding "malformed database header" While SQLiteDatabaseCorruptException is the general exception thrown for corruption, the specific error message "malformed database header" is a standard diagnostic string returned by the SQLite engine itself when it cannot validate the file's header, indicating the file is not a valid SQLite database or is severely damaged. This string is typically passed as the detail message within the SQLiteDatabaseCorruptException object [1][8]. Citations:
🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- AppDatabase callers ---'
rg -n -C 5 "buildDatabase\(" app/src/main app/src/test app/src/androidTest 2>/dev/null || true
printf '%s\n' '--- database test infrastructure and test rules ---'
rg -n -i -C 3 "Room\.inMemoryDatabaseBuilder|Room\.databaseBuilder|AppDatabase|SupportSQLiteOpenHelper|SQLiteDatabaseCorruptException|createFromAsset" app/src/test app/src/androidTest 2>/dev/null || true
printf '%s\n' '--- relevant framework source references ---'
for url in \
"https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/core/java/android/database/sqlite/SQLiteDatabase.java?format=TEXT" \
"https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/core/java/android/database/sqlite/SQLiteOpenHelper.java?format=TEXT" \
"https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/core/java/android/database/sqlite/SQLiteConnection.java?format=TEXT"; do
echo "--- $url ---"
curl -L --fail --silent "$url" | base64 -d 2>/dev/null | rg -n -C 5 "SQLiteDatabaseCorruptException|onCorruption|open failed|malformed|SQLiteException" || true
doneRepository: celzero/rethink-app Length of output: 34676 🌐 Web query:
💡 Result: The SQLiteDatabaseCorruptException with the message database disk image is malformed indicates that the SQLite database engine has detected inconsistency in the database file, often triggered by a malformed header or internal page corruption [1][2][3]. When this occurs during an attempt to open or use a database in Android, it typically triggers the DatabaseErrorHandler [4][5]. Default Behavior and Handling: By default, Android uses the DefaultDatabaseErrorHandler [6]. When it detects corruption, this handler attempts to close the connection and delete the affected database file (and its associated journal/WAL/SHM files) [7][5][6]. This ensures that subsequent attempts to open the database start from a clean state, though it results in data loss for that database [7][5]. Common Causes: 1. Improper File Management: When using a pre-populated database (e.g., from assets), failing to delete old WAL (-wal) and SHM (-shm) files before copying a new version often leads to corruption errors, as SQLite attempts to associate these orphaned files with the new database [2]. Using context.deleteDatabase("name") is the recommended way to ensure all associated files are removed [2]. 2. Improper File Copying: Corruptions frequently occur during the initial copy of a database file if the I/O stream is not managed correctly (e.g., incomplete writes) [8][3]. 3. Environmental Factors: External factors such as unexpected app termination, device power loss, or bugs in the underlying SQLite engine can lead to physical file corruption [7][9]. 4. Multi-threading/Connection Issues: Accessing the same database file from multiple connections or threads without proper synchronization can cause state inconsistency [7][9]. Preventative and Recovery Measures: - Use Correct Deletion: Always use Context.deleteDatabase(String) instead of manually deleting just the main database file to ensure all journal/WAL files are purged [2]. - Implement Custom Error Handling: You can override the default behavior by implementing the DatabaseErrorHandler interface and passing it to the SQLiteOpenHelper constructor [4][5]. This allows you to perform custom actions, such as logging the error, attempting a repair (e.g., using sqlite3 command-line tools if accessible), or restoring from a backup, instead of simply allowing the file to be deleted [5][10][6]. - Maintain Backups: Because SQLite corruption can be irreversible, maintaining periodic backups of critical data is a standard best practice [9]. Citations:
🌐 Web query:
💡 Result: In AndroidX Room, the call path implied by your query is: RoomDatabase -> SupportSQLiteOpenHelper -> (default on Android) FrameworkSQLiteOpenHelper -> FrameworkSQLiteOpenHelper.OpenHelper extends SQLiteOpenHelper -> SQLiteOpenHelper.getWritableDatabase -> SQLiteDatabaseCorruptException can be thrown; AndroidX Room wraps it through its SQLite wrapper and corruption callback logic. 1) Where FrameworkSQLiteOpenHelper delegates to Android’s SQLiteOpenHelper (i.e., where writableDatabase is obtained) - FrameworkSQLiteOpenHelper exposes a writable database by delegating to its internal delegate/open helper. In the Room support code, getWritableDatabase returns mDelegate.getWritableSupportDatabase. [1][2] - The underlying OpenHelper class (FrameworkSQLiteOpenHelper.OpenHelper) extends android.database.sqlite.SQLiteOpenHelper and overrides getWritableSupportDatabase to call super.getWritableDatabase. [1] 2) How corruption is handled inside FrameworkSQLiteOpenHelper (what “wrapping source” looks like) - FrameworkSQLiteOpenHelper.OpenHelper constructs SQLiteOpenHelper with a DatabaseErrorHandler implementation. That handler overrides onCorruption(SQLiteDatabase dbObj) and forwards corruption to Room’s callback via callback.onCorruption(getWrappedDb(dbRef, dbObj)). [1] - So when Android/SQLite reports corruption, Android’s DatabaseErrorHandler flow supplies the SQLiteDatabase instance (dbObj) to FrameworkSQLiteOpenHelper, which then wraps it as a FrameworkSQLiteDatabase (via getWrappedDb) before invoking Room’s SupportSQLiteOpenHelper.Callback.onCorruption. [1] 3) What a “SQLiteDatabaseCorruptException” means in Android - android.database.sqlite.SQLiteDatabaseCorruptException is explicitly documented as indicating that the SQLite database file is corrupt. [3] 4) What Android does by default when corruption is detected (corresponds to the exception being caught/handled at open time) - Android’s DefaultDatabaseErrorHandler.onCorruption(...) logs the corruption and wipes/deletes the detected database files (and attached databases if applicable), which is part of the mechanism that prevents repeated open attempts from using a corrupted file. [4] - FrameworkSQLiteOpenHelper, however, supplies its own DatabaseErrorHandler to route corruption into Room’s onCorruption callback instead of relying solely on the default handler’s deletion behavior. [1][4] Practical implication / typical outcome in Room - If corruption is detected while opening for write (writableDatabase), SQLiteOpenHelper will invoke the DatabaseErrorHandler.onCorruption flow. FrameworkSQLiteOpenHelper routes that to SupportSQLiteOpenHelper.Callback.onCorruption with a wrapped SupportSQLiteDatabase created from the underlying SQLiteDatabase object. [1] If you want, paste the exact stack trace lines around FrameworkSQLiteOpenHelper and the first occurrence of SQLiteDatabaseCorruptException, and I can map each frame precisely to the relevant Room/FrameworkSQLiteOpenHelper code paths (including whether it’s thrown from open-time or from a later Room operation). Citations:
Handle A malformed database can pass 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| private fun newBuilder(context: Context) = | ||
| Room.databaseBuilder(context.applicationContext, AppDatabase::class.java, DATABASE_NAME) | ||
| .createFromAsset(DATABASE_PATH) | ||
| .addCallback(roomCallback) | ||
|
|
@@ -105,7 +128,9 @@ abstract class AppDatabase : RoomDatabase() { | |
| .addMigrations(MIGRATION_27_28) | ||
| .addMigrations(MIGRATION_28_29) | ||
| .addMigrations(MIGRATION_29_30) | ||
| .build() | ||
| // Belt and suspenders: if a migration is *missing* (not just | ||
| // mismatched), let Room destructively rebuild from the asset. | ||
| .fallbackToDestructiveMigration(dropAllTables = true) | ||
|
|
||
| private val roomCallback: Callback = | ||
| object : Callback() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,19 +79,35 @@ interface ConnectionTrackerDAO { | |
| @Query("select * from ConnectionTracker order by id desc LIMIT $MAX_LOGS") | ||
| fun getConnectionTrackerByName(): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query("select * from ConnectionTracker where uid = :uid order by id desc LIMIT $MAX_LOGS") | ||
| fun getConnectionTrackerByName(uid: Int): PagingSource<Int, ConnectionTracker> | ||
|
Comment on lines
+84
to
+85
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [performance · medium] |
||
|
|
||
| @Query( | ||
| "select * from ConnectionTracker where (appName like :query or ipAddress like :query or dnsQuery like :query or flag like :query or proxyDetails like :query or connId like :query) order by id desc LIMIT $MAX_LOGS" | ||
| ) | ||
| fun getConnectionTrackerByName(query: String): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query( | ||
| "select * from ConnectionTracker where uid = :uid and (appName like :query or ipAddress like :query or dnsQuery like :query or flag like :query or proxyDetails like :query or connId like :query) order by id desc LIMIT $MAX_LOGS" | ||
| ) | ||
| fun getConnectionTrackerByName(query: String, uid: Int): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query("select * from ConnectionTracker where isBlocked = 1 order by id desc LIMIT $MAX_LOGS") | ||
| fun getBlockedConnections(): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query("select * from ConnectionTracker where uid = :uid and isBlocked = 1 order by id desc LIMIT $MAX_LOGS") | ||
| fun getBlockedConnections(uid: Int): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query( | ||
| "select * from ConnectionTracker where (appName like :query or ipAddress like :query or dnsQuery like :query or flag like :query or proxyDetails like :query or connId like :query) and isBlocked = 1 order by id desc LIMIT $MAX_LOGS" | ||
| ) | ||
| fun getBlockedConnections(query: String): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query( | ||
| "select * from ConnectionTracker where uid = :uid and (appName like :query or ipAddress like :query or dnsQuery like :query or flag like :query or proxyDetails like :query or connId like :query) and isBlocked = 1 order by id desc LIMIT $MAX_LOGS" | ||
| ) | ||
| fun getBlockedConnections(query: String, uid: Int): PagingSource<Int, ConnectionTracker> | ||
|
|
||
| @Query( | ||
| "SELECT uid, ipAddress, port, COUNT(ipAddress) as count, flag as flag, 0 as blocked, GROUP_CONCAT(DISTINCT dnsQuery) as appOrDnsName, SUM(downloadBytes) as downloadBytes, SUM(uploadBytes) as uploadBytes, SUM(downloadBytes + uploadBytes) as totalBytes FROM ConnectionTracker WHERE uid = :uid and timeStamp > :to GROUP BY uid, ipAddress, port ORDER BY count DESC" | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -38,11 +38,19 @@ interface DnsLogDAO { | |||||
| @Query("select * from DNSLogs order by id desc LIMIT $MAX_LOGS") | ||||||
| fun getAllDnsLogs(): PagingSource<Int, DnsLog> | ||||||
|
|
||||||
| @Query("select * from DNSLogs where uid = :uid order by id desc LIMIT $MAX_LOGS") | ||||||
| fun getAllDnsLogs(uid: Int): PagingSource<Int, DnsLog> | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [performance · low] Suggestion:
Suggested change
|
||||||
|
|
||||||
| @Query( | ||||||
| "select * from DNSLogs where (queryStr like :searchString or responseIps like :searchString or appName like :searchString) order by id desc LIMIT $MAX_LOGS" | ||||||
| ) | ||||||
| fun getDnsLogsByName(searchString: String): PagingSource<Int, DnsLog> | ||||||
|
|
||||||
| @Query( | ||||||
| "select * from DNSLogs where uid = :uid and (queryStr like :searchString or responseIps like :searchString or appName like :searchString) order by id desc LIMIT $MAX_LOGS" | ||||||
| ) | ||||||
| fun getDnsLogsByName(searchString: String, uid: Int): PagingSource<Int, DnsLog> | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [test · low] |
||||||
|
|
||||||
| @Query("select * from DNSLogs where proxyId like :wgDnsId order by id desc LIMIT $MAX_LOGS") | ||||||
| fun getDnsLogsForWireGuard(wgDnsId: String): PagingSource<Int, DnsLog> | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[other · low]
The workflow triggers on both
pushandpull_requesttomainbut has noconcurrencygroup. Rapid successive commits to the same branch/PR can launch redundant, simultaneous builds that waste CI minutes. Add aconcurrencygroup keyed to the workflow and ref withcancel-in-progress: trueso in-progress runs of the same branch are cancelled.Suggestion: