Android SDK: All tests passing (9/9 unit) - #2
Conversation
Rebased from fix/android-sdk-tests-passing onto current main (original branch shared no git history with main after a history rewrite; net diff reapplied as a single commit). - Fixed Android SDK to achieve 9/9 unit tests passing with exit code 0 - Updated build configuration: AGP 8.2.2, Gradle 8.5, compileSdk/targetSdk 35, Java 8 - Fixed test assertions to use public fields instead of getter/setter methods (matching Java SDK field access patterns) - Added robolectric.properties for SDK 35 compatibility - Improved SqliteAndroidLocalCache with proper error handling, transactions, cursor closing, and synchronized methods - Replaced heavy appcompat/material dependencies with lightweight androidx.annotation - Added consumer proguard rules - Removed local.properties from version control
WalkthroughThe Android SDK now provides an Estimated code review effort: 4 (Complex) | ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
android-sdk/consumer-proguard-rules.pro (1)
7-12: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winScope the Jackson consumer rules consistently.
The library rule and installation documentation both retain all Jackson classes and members. Narrow the actual consumer rule to the SDK’s reflective surface, then mirror that narrower rule in the README so consuming applications do not lose R8 shrinking unnecessarily.
android-sdk/consumer-proguard-rules.pro#L7-L12: remove the blanketcom.fasterxml.jackson.**keep rule and preserve only required model/annotation members.README.md#L42-L49: document the narrowed consumer rules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-sdk/consumer-proguard-rules.pro` around lines 7 - 12, Remove the blanket com.fasterxml.jackson.** keep rule from android-sdk/consumer-proguard-rules.pro, retaining only the required reflective model and annotation members in the existing keepclassmembers rule. Update README.md to document the same narrowed consumer rules instead of retaining all Jackson classes and members.android-sdk/build.gradle (1)
31-34: 📐 Maintainability & Code Quality | 🔵 TrivialPin JDK 17 for Gradle builds
sourceCompatibility/targetCompatibilityonly affect bytecode; they do not change the JDK Gradle runs on. With AGP 8.2.2, CI and local dev need JDK 17, or the build can fail before this block is reached.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-sdk/build.gradle` around lines 31 - 34, Configure the Gradle build to run with JDK 17 rather than relying on sourceCompatibility and targetCompatibility in compileOptions. Update the project-level build configuration or Gradle toolchain settings near the existing Java configuration, while preserving the Java 8 bytecode compatibility if required.android-sdk/src/main/java/com/absmartly/android/sdk/cache/SqliteAndroidLocalCache.java (1)
94-111: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant double-logging on serialization failure.
serialize()already logs the failure before throwingCacheSerializationException; thecatch (CacheSerializationException e)block here logs the same failure again before rethrowing. Same pattern repeats inwriteContextData(Lines 161-164). Low-value duplicate log noise, not a correctness issue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-sdk/src/main/java/com/absmartly/android/sdk/cache/SqliteAndroidLocalCache.java` around lines 94 - 111, Remove the duplicate logging from the CacheSerializationException catch blocks in writePublishEvent and writeContextData, while preserving their rethrow behavior. Keep the SQLiteException logging and CacheSerializationException propagation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android-sdk/build.gradle`:
- Around line 49-62: The android-sdk dependency declarations must expose all
types used by its public API and README. In the dependencies block of
android-sdk/build.gradle, change core-api.jar from compileOnly to a published
implementation dependency and promote streamsupport-minifuture from
testImplementation to implementation, while preserving the existing test
coverage dependencies.
In `@build.gradle`:
- Around line 3-4: Align the Android Gradle Plugin declarations in build.gradle
with the API 35 configuration by upgrading both com.android.application and
com.android.library from 8.2.2 to 8.6 or newer; android-sdk/build.gradle lines
13-17 require no direct change because they already target API 35 and are
corrected by the plugin upgrade.
In `@README.md`:
- Around line 536-540: Update the fragment helper getDeviceId() to reuse the
persisted UUID creation logic from getOrCreateDeviceId() instead of returning an
empty string when no device ID exists. Preserve retrieval of the stored ID while
ensuring first launch creates and persists a valid unique device identifier.
- Line 19: Polish the README prose by adding the requested commas after
“application” and “API key,” and replace “when the application is first started”
with “when the application starts,” without changing the surrounding content.
- Around line 241-243: Update the README examples using Map.of, including the
snippets around context.setUnits and the other referenced examples, to use Java
8-compatible map construction. Keep the examples’ existing behavior and values
unchanged, and do not rely on undocumented core-library desugaring.
- Around line 118-131: Update the manual wiring example around
SqliteAndroidLocalCache, ClientConfig, and ABsmartlyConfig to register the
created cache with the SDK/client using the actual persistence registration
calls. Also show the corresponding cache close operation during SDK teardown so
the example demonstrates the complete cache lifecycle.
---
Nitpick comments:
In `@android-sdk/build.gradle`:
- Around line 31-34: Configure the Gradle build to run with JDK 17 rather than
relying on sourceCompatibility and targetCompatibility in compileOptions. Update
the project-level build configuration or Gradle toolchain settings near the
existing Java configuration, while preserving the Java 8 bytecode compatibility
if required.
In `@android-sdk/consumer-proguard-rules.pro`:
- Around line 7-12: Remove the blanket com.fasterxml.jackson.** keep rule from
android-sdk/consumer-proguard-rules.pro, retaining only the required reflective
model and annotation members in the existing keepclassmembers rule. Update
README.md to document the same narrowed consumer rules instead of retaining all
Jackson classes and members.
In
`@android-sdk/src/main/java/com/absmartly/android/sdk/cache/SqliteAndroidLocalCache.java`:
- Around line 94-111: Remove the duplicate logging from the
CacheSerializationException catch blocks in writePublishEvent and
writeContextData, while preserving their rethrow behavior. Keep the
SQLiteException logging and CacheSerializationException propagation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12240d91-fb8f-40a4-a65e-a35c2de5ffe0
📒 Files selected for processing (13)
.gitignoreREADME.mdandroid-sdk/build.gradleandroid-sdk/consumer-proguard-rules.proandroid-sdk/src/main/java/com/absmartly/android/sdk/ABSmartlyAndroid.javaandroid-sdk/src/main/java/com/absmartly/android/sdk/cache/SqliteAndroidLocalCache.javaandroid-sdk/src/test/java/com/absmartly/android/sdk/ABSmartlyAndroidTest.javaandroid-sdk/src/test/java/com/absmartly/android/sdk/SqliteAndroidLocalCacheTest.javaandroid-sdk/src/test/java/com/absmartly/android/sdk/UnitTest.javaandroid-sdk/src/test/resources/robolectric.propertiesbuild.gradlegradle/wrapper/gradle-wrapper.propertieslocal.properties
💤 Files with no reviewable changes (2)
- local.properties
- android-sdk/src/test/java/com/absmartly/android/sdk/UnitTest.java
| implementation 'androidx.annotation:annotation:1.7.1' | ||
|
|
||
| implementation 'androidx.appcompat:appcompat:1.6.1' | ||
| implementation 'com.google.android.material:material:1.8.0' | ||
| testImplementation 'junit:junit:4.13.2' | ||
| testImplementation 'org.mockito:mockito-core:4.11.0' | ||
| testImplementation 'org.mockito:mockito-inline:4.11.0' | ||
| testImplementation 'org.robolectric:robolectric:4.10.3' | ||
|
|
||
| androidTestImplementation 'androidx.test.ext:junit:1.1.5' | ||
| androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1' | ||
|
|
||
| implementation 'com.absmartly.sdk:core-api:1.6.0' | ||
| implementation 'com.fasterxml.jackson.core:jackson-databind:2.13.4.2' | ||
| compileOnly files('../../java-sdk/core-api/build/libs/core-api.jar') | ||
| testImplementation files('../../java-sdk/core-api/build/libs/core-api.jar') | ||
| testImplementation 'net.sourceforge.streamsupport:streamsupport-minifuture:1.7.4' | ||
| implementation 'com.fasterxml.jackson.core:jackson-databind:2.17.2' |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n "compileOnly|testImplementation|api|implementation|core-api|streamsupport|minifuture" \
android-sdk/build.gradle README.md
rg -n "com\.absmartly\.sdk|java8\.util\.concurrent\.CompletableFuture" \
android-sdk README.mdRepository: absmartly/android-sdk
Length of output: 4340
Publish the core API and Java 8 backport as public dependencies android-sdk/build.gradle still declares core-api.jar as compileOnly and streamsupport-minifuture as testImplementation, but the Android SDK’s public surface uses com.absmartly.sdk.* types and the README imports java8.util.concurrent.CompletableFuture. Consumers who depend only on com.absmartly.sdk:android-sdk will be missing those classes. Make the dependency graph transitive, or remove those types from the published API/docs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@android-sdk/build.gradle` around lines 49 - 62, The android-sdk dependency
declarations must expose all types used by its public API and README. In the
dependencies block of android-sdk/build.gradle, change core-api.jar from
compileOnly to a published implementation dependency and promote
streamsupport-minifuture from testImplementation to implementation, while
preserving the existing test coverage dependencies.
| id 'com.android.application' version '8.2.2' apply false | ||
| id 'com.android.library' version '8.2.2' apply false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== build.gradle ==\n'
cat -n build.gradle
printf '\n== android-sdk/build.gradle ==\n'
cat -n android-sdk/build.gradleRepository: absmartly/android-sdk
Length of output: 6335
🌐 Web query:
AGP 8.2.2 compileSdk 35 targetSdk 35 supported API level release notes
💡 Result:
Android Gradle Plugin (AGP) 8.2.2 does not provide official support for compileSdk 35 or targetSdk 35 [1]. According to official Android documentation, the minimum AGP version required to support API level 35 is AGP 8.6.0 [1]. Attempting to use compileSdk 35 with older versions of AGP, such as 8.2.2, can lead to build errors or unexpected behavior because these versions were designed and tested for earlier API levels (AGP 8.2 specifically supports up to API level 34) [2][3][1]. For projects requiring API level 35 support, you should upgrade to at least AGP 8.6.0 or higher [1].
Citations:
- 1: https://developer.android.com/build/releases/about-agp
- 2: https://developer.android.com/build/releases/agp-8-2-0-release-notes
- 3: https://developer.android.com/build/releases/past-releases/agp-8-2-0-release-notes
🌐 Web query:
site:developer.android.com AGP 8.2 API 35 compileSdk warning max supported API 34
💡 Result:
The warning you are seeing occurs because Android Gradle Plugin (AGP) version 8.2 is explicitly designed to support a maximum compileSdk of API level 34 [1][2]. When you set your compileSdk to 35, the build tools detect a mismatch between the requested SDK version and the validation logic included in that specific version of AGP, triggering the warning [3][4]. To resolve this issue, you should upgrade your Android Gradle Plugin to a version that officially supports API level 35. As AGP versions are tightly coupled with the Android Studio version and the underlying Gradle version, it is recommended to update both Android Studio and AGP to the latest stable releases to ensure full compatibility with newer Android platform APIs [3][4]. In summary: - The warning is expected behavior because AGP 8.2 has a hard-coded maximum supported API level of 34 [1][2]. - Updating to a newer version of AGP (which supports API 35) will remove the warning and allow you to properly compile against the newer platform SDK. - Always check the official Android Gradle Plugin release notes for the specific version you are upgrading to, as it will specify the minimum required Gradle version and Android Studio version [1][3][5].
Citations:
- 1: https://developer.android.com/build/releases/past-releases/agp-8-2-0-release-notes
- 2: https://developer.android.com/build/releases/agp-8-2-0-release-notes
- 3: https://developer.android.com/build/releases/agp-8-3-0-release-notes
- 4: https://developer.android.com/build/releases/past-releases/agp-8-3-0-release-notes
- 5: https://developer.android.com/build/releases/agp-8-4-0-release-notes
Align AGP with API 35 support. build.gradle pins AGP 8.2.2, but android-sdk/build.gradle sets compileSdk/targetSdk to 35. Upgrade AGP to 8.6+ or lower the SDK levels so the module stays within the plugin’s supported range.
📍 Affects 2 files
build.gradle#L3-L4(this comment)android-sdk/build.gradle#L13-L17
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` around lines 3 - 4, Align the Android Gradle Plugin
declarations in build.gradle with the API 35 configuration by upgrading both
com.android.application and com.android.library from 8.2.2 to 8.6 or newer;
android-sdk/build.gradle lines 13-17 require no direct change because they
already target API 35 and are corrected by the plugin upgrade.
| It uses [A/B Smartly - Java SDK.](https://github.com/absmartly/java-sdk) | ||
| The A/B Smartly Android SDK is compatible with Android 5.0 and later (API level 21+). | ||
|
|
||
| The `android.permission.INTERNET` permission is required. To add this permission to your application ensure the following line is present in the `AndroidManifest.xml` file: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Polish the flagged prose.
Add a comma after “application” at Line 19 and after “API key” at Line 166. At Line 220, replace “when the application is first started” with “when the application starts”.
Also applies to: 166-166, 220-220
🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: Possible missing comma found.
Context: ...equired. To add this permission to your application ensure the following line is present in...
(AI_HYDRA_LEO_MISSING_COMMA)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 19, Polish the README prose by adding the requested commas
after “application” and “API key,” and replace “when the application is first
started” with “when the application starts,” without changing the surrounding
content.
Source: Linters/SAST tools
| SqliteAndroidLocalCache cache = new SqliteAndroidLocalCache(getApplicationContext()); | ||
|
|
||
| final ClientConfig clientConfig = ClientConfig.create() | ||
| .setEndpoint("https://your-company.absmartly.io/v1") | ||
| .setAPIKey("YOUR-API-KEY") | ||
| .setApplication("android-app") | ||
| .setEnvironment("production"); | ||
|
|
||
| final Client absmartlyClient = Client.create(clientConfig); | ||
|
|
||
| final ABsmartlyConfig sdkConfig = ABsmartlyConfig.create() | ||
| .setClient(absmartlyClient); | ||
|
|
||
| final ABsmartly sdk = ABsmartly.create(sdkConfig); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n "ContextDataProvider|ContextEventHandler|setContextDataProvider|setContextEventHandler|SqliteAndroidLocalCache" \
android-sdk java-sdk README.mdRepository: absmartly/android-sdk
Length of output: 3814
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '\n## README excerpt\n'
sed -n '108,145p' README.md
printf '\n## ABsmartlyConfig API\n'
rg -n "class ABsmartlyConfig|setContextDataProvider|setContextEventHandler|close\\(" android-sdk/src/main/java -g '*.java'
printf '\n## Related wiring in ABSmartlyAndroid\n'
sed -n '1,140p' android-sdk/src/main/java/com/absmartly/android/sdk/ABSmartlyAndroid.java
printf '\n## SqliteAndroidLocalCache lifecycle methods\n'
sed -n '1,220p' android-sdk/src/main/java/com/absmartly/android/sdk/cache/SqliteAndroidLocalCache.javaRepository: absmartly/android-sdk
Length of output: 13685
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '\n## Relevant source files\n'
git ls-files 'android-sdk/src/main/java/**/*.java' | sed -n '1,120p'
printf '\n## Find ABsmartlyConfig and related APIs\n'
fd -a 'ABsmartlyConfig.java|ABsmartly.java|ClientConfig.java|ContextDataProvider.java|ContextEventHandler.java' android-sdk/src/main/java
printf '\n## README lines around the manual wiring section\n'
sed -n '110,140p' README.mdRepository: absmartly/android-sdk
Length of output: 1439
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '\n## Search repository for context provider/event handler wiring\n'
rg -n "setContextDataProvider|setContextEventHandler|ContextDataProvider|ContextEventHandler|implements .*ContextDataProvider|implements .*ContextEventHandler" .
printf '\n## Search for ABsmartlyConfig class definition\n'
rg -n "class ABsmartlyConfig|interface ContextDataProvider|interface ContextEventHandler" android-sdk/src/main/java .Repository: absmartly/android-sdk
Length of output: 835
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '156,180p' README.mdRepository: absmartly/android-sdk
Length of output: 3775
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '586,640p' README.mdRepository: absmartly/android-sdk
Length of output: 2491
Attach the cache in the manual wiring example. cache is created but never used, so this snippet doesn’t show how persistence is enabled. Show the actual registration calls and close the cache when the SDK is torn down.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 118 - 131, Update the manual wiring example around
SqliteAndroidLocalCache, ClientConfig, and ABsmartlyConfig to register the
created cache with the SDK/client using the actual persistence registration
calls. Also show the corresponding cache close operation during SDK teardown so
the example demonstrates the complete cache lifecycle.
| context.setUnits(Map.of( | ||
| "db_user_id", "1000013" | ||
| )); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n '\bMap\.of\(' README.md
rg -n 'coreLibraryDesugaring|desugar_jdk_libs' --glob '*.gradle*' .Repository: absmartly/android-sdk
Length of output: 292
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the relevant README sections
sed -n '1,120p' README.md
printf '\n---\n'
sed -n '230,330p' README.md
# Search for Java 8 / Android compatibility and desugaring references
rg -n 'Java 8|API 21|desugar|desugaring|core library|coreLibraryDesugaring|desugar_jdk_libs' README.md . --glob '!**/build/**'Repository: absmartly/android-sdk
Length of output: 7320
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Read the README sections that usually state supported Java/Android versions.
rg -n 'Java 8|Android API 21|Android 5\.0|minSdk|compatib|support' README.md
sed -n '1,80p' README.mdRepository: absmartly/android-sdk
Length of output: 3487
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n 'coreLibraryDesugaring|desugar_jdk_libs|isCoreLibraryDesugaringEnabled|sourceCompatibility|targetCompatibility|JavaVersion\.VERSION_1_8|compileOptions' . --glob '*.gradle*' --glob '*.gradle.kts' --glob '!**/build/**'Repository: absmartly/android-sdk
Length of output: 374
Replace Map.of in these README examples
android-sdk/build.gradle only sets Java 8 sourceCompatibility/targetCompatibility, and the README documents API 21+ without any core-library desugaring setup. Map.of is Java 9+, so these snippets won’t compile as written on the documented baseline. Use Java 8-compatible map construction or document and require desugaring. Also applies to README.md:289-292, README.md:304-306, and README.md:314-317.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 241 - 243, Update the README examples using Map.of,
including the snippets around context.setUnits and the other referenced
examples, to use Java 8-compatible map construction. Keep the examples’ existing
behavior and values unchanged, and do not rely on undocumented core-library
desugaring.
| private String getDeviceId() { | ||
| return requireActivity() | ||
| .getSharedPreferences("absmartly", android.content.Context.MODE_PRIVATE) | ||
| .getString("device_id", ""); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not default a new fragment to an empty device ID.
Unlike the Activity example, this helper returns "" when no ID has been stored. On first launch, that can create an invalid or shared identity. Reuse the persisted UUID creation logic from getOrCreateDeviceId().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 536 - 540, Update the fragment helper getDeviceId()
to reuse the persisted UUID creation logic from getOrCreateDeviceId() instead of
returning an empty string when no device ID exists. Preserve retrieval of the
stored ID while ensuring first launch creates and persists a valid unique device
identifier.
1 participant
Summary
Rebase/re-open of #1 (closed without merge). The original branch's git history no longer shares a common ancestor with
main(GitHub rejected reopening: "no history in common"), so this PR reapplies the same net changes as a fresh commit on top of currentmain.robolectric.propertiesfor SDK 35 compatibilitySqliteAndroidLocalCachewith proper error handling, transactions, cursor closing, and synchronized methodsandroidx.annotationlocal.propertiesfrom version controlTest plan
./gradlew :android-sdk:test), verified locally after rebaseCloses #1 (superseded).
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Maintenance