Skip to content

refactor(android SDK): convert SessionService and MessageService to Kotlin - #983

Merged
nickolas-dimitrakas merged 1 commit into
mainfrom
refactor/convert-sessionservice-and-messageservice-to
Nov 3, 2026
Merged

nickolas-dimitrakas merged 1 commit into
mainfrom
refactor/convert-sessionservice-and-messageservice-to

Conversation

@nickolas-dimitrakas

Copy link
Copy Markdown
Contributor

Why

Risk class L.

This pull request converts row A4 of the Kotlin migration tracker from Java to Kotlin: SessionService and MessageService. 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$ReadyMessage entry 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:

  • The two classes. Each stays an open class with its public no-argument constructor, because MParticleDBManager creates each one to call updateMpId.
  • Static methods. 24 @JvmStatic functions in companion objects, and three @Throws(JSONException::class) on getSessionForSessionEndMessage, insertSession and insertMessage.
  • Static field. SessionService.readyMessages is a @JvmField var on the companion, so it is a public, mutable static field as before.
  • Package-private members. The four-argument getSessionHistory, the three-argument getMessagesForUpload and flattenBySessionId become internal companion functions with no @JvmStatic. No Java code calls them.
  • ReadyMessage. An open class with six open vals and the getters it had. Its private constructor becomes internal, because Kotlin's outer class cannot call a nested class's private constructor.
  • SQL text. String templates over the column constants, BaseColumns._ID for "_id", and String.format for the session history clause, as the Java did. The unit tests compare each clause with the Java's literal text.
  • Modifiers. No @Synchronized or @Volatile, no default arguments and no top-level declarations.
  • Reflection strings and ProGuard files. No ProGuard rule and no string in the repository names these classes, and this pull request edits neither.

Left alone: deleteSessions with a set still quotes each id into the SQL text without escaping it. The mobsf-ignore: sqlite_injection comment stays on the rawQuery line. Dropped: the unused private getDeletableMessagesQuery and unused imports.

Nullability. The playbook's default is a nullable type for every unannotated parameter and return. The return choices depart from it:

  • Parameters are nullable and read with !! at the step where the Java first used them, so a null throws NullPointerException at that step.
  • getSessions and getSessionForSessionEndMessage return a non-null Cursor, because MPDatabase.query declares one.
  • Returned lists, sets and maps are non-null and mutable, because every path returns a new collection and MParticleDBManager adds to the set getSessionIds returns. getSessionIds holds String?, so a Kotlin caller can add a nullable id. getOrphanSessionIds holds String, because session_id is NOT NULL.
  • readyMessages is Array<String>?, because Java code can assign null to it.
  • ReadyMessage.mpid is Long and messageId is Int, primitive returns as before. sessionId, message and dataplanId are String?, and dataplanVersion is Int?, which compiles to an Integer return.

The API dump diff, as scripts/check_api_dump.py reports it against main. A @JvmStatic function 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's UploadSettings$Companion. No ProGuard rule names com.mparticle.internal.database.services, so R8 renames these classes in the published AAR, and apps and kits see no change. The apiCheck task in Unit Tests is the check:

com.mparticle.internal.database.services.SessionService
  - public static fun deleteSessions (L.../MPDatabase;Ljava/lang/String;)I
  - public static fun deleteSessions (L.../MPDatabase;Ljava/util/Set;)I
  - public static fun processSessions (L.../MPDatabase;Ljava/util/HashMap;)Ljava/util/List;
  + public static final field Companion L.../services/SessionService$Companion;
  + public static final fun deleteSessions (L.../MPDatabase;Ljava/lang/String;)I
  + public static final fun deleteSessions (L.../MPDatabase;Ljava/util/Set;)I
  + public static final fun processSessions (L.../MPDatabase;Ljava/util/HashMap;)Ljava/util/List;
com.mparticle.internal.database.services.SessionService$Companion
  + class added

The other ten static methods of SessionService and the eleven of MessageService change the same way, each class with a $Companion class 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, whose UploadSettings parameter R8 renames. Row A2 meets the same finding and changes scripts/api_compat_report.py to 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 apiCheck again.

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.py against main: no frozen class, and the reviewable diff is above.
  • Every hand-made nullability and modifier decision is listed above, and reflection strings and ProGuard files are not edited.
  • The risk class is the first line.

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.
  • Instrumented tests on an API 28 emulator, among them SessionServiceTest, MessageServiceTest, MParticleDBManagerTest and UpgradeVersionTest, which drive the two classes on a real database: the Instrumented Tests jobs.
  • Binary Compatibility: its job.
  • Kit compatibility against Maven Local: Kit Compatibility Test and build-kits.
  • trunk check: Trunk code check.
  • SDK Size Impact: its comment.

Risks

  • A unit test fails on its first run. The tests were written where no Android SDK was available, so Unit Tests is their first run. The fix goes on this pull request.
  • A SQL clause changes. Each is built from the column constants the Java used, and the new unit tests compare every clause.
  • The hand-written API dump differs from what apiDump writes. The apiCheck task in Unit Tests shows it, and the fix is to run apiDump and commit its output.
  • A minified app or kit breaks. No ProGuard rule keeps these classes. Binary Compatibility and SDK Size Impact show it.

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.

@nickolas-dimitrakas
nickolas-dimitrakas requested a review from a team as a code owner November 2, 2026 19:10
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Kotlin migration progress

This pull request converts 502 lines of Java to Kotlin.

Module Kotlin LOC Java LOC Java staying (facade) Java left to convert Kotlin share
android-core 4,986 20,988 9,716 11,272 19.2%
android-kit-base 601 5,405 1,651 3,754 10.0%
Total 5,587 26,393 11,367 15,026 17.5%

Conversion progress: 9.2% (15,026 of 16,550 baseline Java LOC left to convert). 112 Java files and 56 Kotlin files in scope.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Binary compatibility

  • ✅ android-core is compatible with 6.1.9 (112 classes compared, 0 with additions only).
  • ✅ android-kit-base is compatible with 6.1.9 (37 classes compared, 0 with additions only).

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 api-compat-reports workflow artifact. This check is not in the required-check set yet, so a red result here does not block merging.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.kt and MessageService.kt against the deleted Java: selection clauses, bound args and the NOT IN text all match, and cursors are closed in finally as before.
  • Nullability: parameters are now T? and use !! where the Java first dereferenced them, so a null still throws NPE at the same step. readyMessages is still a public mutable static field (@ JvmField var).
  • The API dump changes (new Companion entries, static methods now final) 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.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

📦 SDK Size Impact Report

What the SDK adds to a minified release APK.

mParticle Core SDK

Measured 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.

Metric Target branch This PR Change
APK size 120.86 KB 120.73 KB -124 bytes
Download size 118.67 KB 118.55 KB -125 bytes
Dex bytes 216.75 KB 216.56 KB -188 bytes

mParticle Core + Rokt kit

Measured 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 size-report-rokt/README.md.

Metric Target branch This PR Change
APK size 1.26 MB 1.26 MB 0 bytes
Download size 1.25 MB 1.25 MB -66 bytes
Dex bytes 2.23 MB 2.23 MB -232 bytes

Rokt SDK+ umbrella (adds the payment extension)

Metric Target branch This PR Change On top of mParticle Core + Rokt kit
APK size 6.55 MB 6.55 MB 0 bytes +5.29 MB
Download size 6.43 MB 6.43 MB +40 bytes +5.19 MB
Dex bytes 7.12 MB 7.12 MB -236 bytes +4.88 MB

➡️ SDK size impact change is minimal.

Raw measurements

core, 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}

Measured 48a3396 merged into d247e12

@nickolas-dimitrakas
nickolas-dimitrakas merged commit 860f6ca into main Nov 3, 2026
47 checks passed
@nickolas-dimitrakas
nickolas-dimitrakas deleted the refactor/convert-sessionservice-and-messageservice-to branch November 3, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants