Repository navigation
refactor(android SDK): convert SessionService and MessageService to Kotlin - #983
Conversation
Kotlin migration progressThis pull request converts 502 lines of Java to Kotlin.
Conversion progress: 9.2% (15,026 of 16,550 baseline Java LOC left to convert). 112 Java files and 56 Kotlin files in scope. |
Binary compatibility
Compares this branch with the last version published to Maven Central, both built without R8, over the public classes and members that release keeps under their own names. It also checks that this branch's release AAR still keeps them (see the script docstring). Full japicmp HTML/XML reports are attached as the |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It rewrites two persistence classes, about 500 lines, and changes the API dump, so a human look is still worthwhile, mainly to decide whether this is class L or class M under the playbook.
What was reviewed:
- The SQL in
SessionService.ktandMessageService.ktagainst the deleted Java: selection clauses, bound args and theNOT INtext all match, and cursors are closed infinallyas before. - Nullability: parameters are now
T?and use!!where the Java first dereferenced them, so a null still throws NPE at the same step.readyMessagesis still a public mutable static field (@ JvmField var). - The API dump changes (new
Companionentries, static methods nowfinal) are additive for Java callers. Whether that meets the playbook's "zero public-surface diff" rule for class L is a process decision, not a code defect.
Extended reasoning...
The change converts MessageService and SessionService in android-core's database layer from Java to Kotlin, regenerates android-core.api and adds 42 Kotlin unit tests. It touches persistence code: local SQLite queries, deletes and updates. No auth or crypto code is involved, and the one string-built SQL clause (the deleteSessions set overload) is carried over unchanged from the Java. I deferred because the conversion is large and carries behaviour, and because a CODEOWNERS wildcard assigns every file to the SDK team. The PR also says it is risk class L while the playbook lists persistence as class M, so a maintainer should decide which merge gate applies.
📦 SDK Size Impact ReportWhat the SDK adds to a minified release APK. mParticle Core SDKMeasured against an empty baseline app. Unlike the Rokt kit, android-core ships no Compose and no resources, so there is nothing here that a host app would already provide.
mParticle Core + Rokt kitMeasured against a Compose + Material3 reference app, so these are the costs on top of an app that already ships Compose. The reference app's dependencies are a documented convention, not a measured average: see
Rokt SDK+ umbrella (adds the payment extension)
➡️ SDK size impact change is minimal. Raw measurementscore, Target branch: {"baseline_dex_bytes": 0, "baseline_download_bytes": 2513, "baseline_install_bytes": 7530, "core_dex_bytes": 221948, "core_download_bytes": 124034, "core_install_bytes": 131286}core, This PR: {"baseline_dex_bytes": 0, "baseline_download_bytes": 2512, "baseline_install_bytes": 7529, "core_dex_bytes": 221760, "core_download_bytes": 123908, "core_install_bytes": 131161}rokt, Target branch: {"baseline_dex_bytes": 1829108, "baseline_download_bytes": 1240062, "baseline_install_bytes": 1300934, "kit_dex_bytes": 4171332, "kit_download_bytes": 2548131, "kit_install_bytes": 2623275, "sdkplus_dex_bytes": 9293096, "sdkplus_download_bytes": 7985735, "sdkplus_install_bytes": 8165359}rokt, This PR: {"baseline_dex_bytes": 1829108, "baseline_download_bytes": 1240062, "baseline_install_bytes": 1300934, "kit_dex_bytes": 4171100, "kit_download_bytes": 2548065, "kit_install_bytes": 2623275, "sdkplus_dex_bytes": 9292860, "sdkplus_download_bytes": 7985775, "sdkplus_install_bytes": 8165359} |
Why
Risk class L.
This pull request converts row A4 of the Kotlin migration tracker from Java to Kotlin:
SessionServiceandMessageService. Their two test classes are instrumented tests, so it first adds unit tests that pin what each class does. Apps and kits do not see it: the classes are internal, R8 renames them in the published AAR, and no call site in the repository needed an edit.Programme
The Java-to-Kotlin migration of android-core, TRACKER.md row A4 (Stack A, persistence), converted under PLAYBOOK.md. Row A1 is #971 and #970.
What changes
The two classes become Kotlin in the same directory and package. Every public member has its JVM name and descriptor from before. Each class gains a Companion class, and its static methods become final. The
MessageService$ReadyMessageentry in the API dump does not move.Characterization tests, added first, in
android-core/src/test/kotlin/com/mparticle/internal/database/services/:SessionServiceTest, 21 tests: the deletes by session id, end time and an excepted set, the stored columns of a new session, the updates of end time, attributes, status and install referrer, the open-session reads, app and device info set on each session's batches, a session whose info is not JSON skipped, the batches grouped by session, and a null database.MessageServiceTest, 21 tests: the distinct session ids, the session history and the messages for upload with every column read, one MPID selected, a missing column thrown with the cursor closed, the deletes, the upload mark, a message stored as ready or as batch ready, the session id removed from a message with no session, an oversize message logged and not stored, and a null database.Decisions made by hand:
open classwith its public no-argument constructor, becauseMParticleDBManagercreates each one to callupdateMpId.@JvmStaticfunctions in companion objects, and three@Throws(JSONException::class)ongetSessionForSessionEndMessage,insertSessionandinsertMessage.SessionService.readyMessagesis a@JvmField varon the companion, so it is a public, mutable static field as before.getSessionHistory, the three-argumentgetMessagesForUploadandflattenBySessionIdbecomeinternalcompanion functions with no@JvmStatic. No Java code calls them.ReadyMessage. Anopen classwith sixopen vals and the getters it had. Its private constructor becomesinternal, because Kotlin's outer class cannot call a nested class's private constructor.BaseColumns._IDfor"_id", andString.formatfor the session history clause, as the Java did. The unit tests compare each clause with the Java's literal text.@Synchronizedor@Volatile, no default arguments and no top-level declarations.Left alone:
deleteSessionswith a set still quotes each id into the SQL text without escaping it. Themobsf-ignore: sqlite_injectioncomment stays on therawQueryline. Dropped: the unused privategetDeletableMessagesQueryand unused imports.Nullability. The playbook's default is a nullable type for every unannotated parameter and return. The return choices depart from it:
!!at the step where the Java first used them, so a null throwsNullPointerExceptionat that step.getSessionsandgetSessionForSessionEndMessagereturn a non-nullCursor, becauseMPDatabase.querydeclares one.MParticleDBManageradds to the setgetSessionIdsreturns.getSessionIdsholdsString?, so a Kotlin caller can add a nullable id.getOrphanSessionIdsholdsString, becausesession_idisNOT NULL.readyMessagesisArray<String>?, because Java code can assign null to it.ReadyMessage.mpidisLongandmessageIdisInt, primitive returns as before.sessionId,messageanddataplanIdareString?, anddataplanVersionisInt?, which compiles to anIntegerreturn.The API dump diff, as
scripts/check_api_dump.pyreports it against main. A@JvmStaticfunction in a companion object adds a Companion field and a Companion class, and Kotlin marks the static method final. The entries are written by hand in the form of the dump'sUploadSettings$Companion. No ProGuard rule namescom.mparticle.internal.database.services, so R8 renames these classes in the published AAR, and apps and kits see no change. TheapiChecktask in Unit Tests is the check:The other ten static methods of
SessionServiceand the eleven ofMessageServicechange the same way, each class with a$Companionclass added.public static field readyMessages [Ljava/lang/String;does not change.The two new Companion classes may shift R8's short names in the release jar, so Binary Compatibility may report
ConfigManager.setLastUploadSettings, whoseUploadSettingsparameter R8 renames. Row A2 meets the same finding and changesscripts/api_compat_report.pyto match such a rename. This pull request leaves that script alone.Linked work
Row A1: #971 and #970. Rows A2 and A3 are separate pull requests. They share no source file with this one, and each edits the API dump near these entries. Whichever merges second rebases its dump and runs
apiCheckagain.Rollout
This merges with no release, and it reaches apps in the next SDK release. No flag applies, because a library conversion has no runtime switch. To undo it, revert the squash commit.
The playbook checklist. Done before opening:
scripts/check_api_dump.pyagainst main: no frozen class, and the reviewable diff is above.Waiting for this pull request's jobs:
./gradlew apiCheck: Unit Tests../gradlew test ktlintCheck lint, with 42 more unit tests than main: Unit Tests, Lint Checks and Kotlin Lint Checks.SessionServiceTest,MessageServiceTest,MParticleDBManagerTestandUpgradeVersionTest, which drive the two classes on a real database: the Instrumented Tests jobs.trunk check: Trunk code check.Risks
apiDumpwrites. TheapiChecktask in Unit Tests shows it, and the fix is to runapiDumpand commit its output.Who
An AI coding agent wrote the tests and the conversion, as the next row of the tracker. The SDK team reviews it on this pull request. What ran and what waits for this pull request's jobs is under Rollout.
Size
Cost: not yet read.
2 Kotlin files, 505 lines, replacing 2 Java files of 502 lines. 2 new unit test files, 570 lines. The API dump: 56 lines added, 24 removed. Each class gains a small Companion class. Expect a few hundred bytes in SDK Size Impact.