Skip to content

refactor(android SDK): convert four B1 networking classes to Kotlin - #985

Merged
nickolas-dimitrakas merged 1 commit into
mainfrom
refactor/convert-four-b1-networking-classes-to-kotlin
Nov 3, 2026
Merged

nickolas-dimitrakas merged 1 commit into
mainfrom
refactor/convert-four-b1-networking-classes-to-kotlin

Conversation

@nickolas-dimitrakas

Copy link
Copy Markdown
Contributor

Why

Risk class L.

This pull request converts four of the five classes in row B1 of the Kotlin migration tracker from Java to Kotlin: MParticleBaseClient, MPUrlImpl, MPConnectionImpl and MParticleApiClient. None of them had a unit test, 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. NetworkOptionsManager waits for an API review, for the reason below.

Programme

The Java-to-Kotlin migration of android-core, TRACKER.md row B1 (Stack B, networking), converted under PLAYBOOK.md.

What changes

The four classes become Kotlin, each in its Java file's directory and package. scripts/check_api_dump.py against main finds no difference in either API dump.

Characterization tests, added first:

  • networking/MPUrlImplTest, 8 tests: the file, authority and path each constructor reads, toString as the whole URL, a malformed or null string, equality against an MPUrlImpl, a URL and other objects, and openConnection for HTTPS, HTTP and a file: URL.
  • networking/MPConnectionImplTest, 10 tests: each setter and getter reaching the HttpURLConnection, null boxed values, the response code read on every call, IOException, the SSL socket factory on HTTPS and HTTP connections, and a null connection.
  • internal/MParticleApiClientTest, 6 tests: the AliasNetworkResponse constructors, getters and setter, its JVM name and modifiers, each interface method's checked exceptions, and the requestHandler accessors.

Decisions made by hand:

  • MPUrlImpl and MPConnectionImpl. Package-private, so internal class. Both become final. Nothing extends or mocks either one.
  • Checked exceptions. 11 @Throws: 4 on MParticleApiClient, 2 on MPUrlImpl and 5 on MPConnectionImpl.
  • AliasNetworkResponse. An open class with open getResponseCode and getErrorMessage. Its two-argument constructor is internal.
  • setErrorMessage. An internal function with @JvmName("setErrorMessage"), because UploadHandler, in Java, calls it by that name, and Kotlin would otherwise add a module suffix.
  • Getters stay functions. getCookies and the AliasNetworkResponse getters stay functions. No Kotlin code calls them with property syntax.
  • androidx annotations. @NonNull stays on sendAliasRequest and @Nullable on getErrorMessage, because proguard.pro keeps members that carry them.
  • Modifiers. No @JvmStatic, @JvmField, @Synchronized or @Volatile, no default arguments, no companion objects 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.

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

  • MParticleBaseClient.requestHandler is a non-null var. PinningTestHelper and MParticleApiClientImplTest read it with property syntax and no null check.
  • The message and uploadSettings parameters of sendMessageBatch and sendAliasRequest are nullable, where the Java said @NonNull. UploadHandlerTest stubs sendAliasRequest through the interface with Mockito.any(), which returns null, and a non-null Kotlin parameter would make those calls throw IllegalStateException. MParticleApiClientImpl still carries @NonNull on its own parameters.
  • sendAliasRequest returns a non-null AliasNetworkResponse, as the Java's @NonNull says.
  • Every parameter of MPUrlImpl and MPConnectionImpl is nullable, read with !! at the step where the Java first used it. Their returns are nullable, except toString, isHttps and getResponseCode.

What a strict client could notice:

  • A null still throws NullPointerException at the step where the Java threw, but the exception from !! has no message.
  • Where a connection and a boxed value are both null, the connection's check now fails first.
  • MPUrlImpl, MPConnectionImpl, the internal constructor and setErrorMessage are public in bytecode, where the Java was package-private. R8 renames them in the published AAR.
  • Kotlin code can now pass null to the message and uploadSettings parameters through the interface.

NetworkOptionsManager stays Java. scripts/check_api_dump.py treats every class outside com.mparticle.internal as frozen. A Kotlin form adds a public Companion field, and @JvmStatic marks validateAndResolve final, so the check fails without the api-change-approved label. No ProGuard rule keeps the class, so R8 renames it in the published AAR. Its conversion belongs in a pull request of its own, with that label.

Linked work

Rows A2 and A3 are separate pull requests. This pull request shares no file with either.

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: the dumps are unchanged.
  • 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 24 more unit tests than main: Unit Tests, Lint Checks and Kotlin Lint Checks.
  • Instrumented tests on an API 28 emulator, among them PinningTest, UploadMessageTest and MParticleBaseClientImplTest, which drive these classes on a device: 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 Kotlin caller stops compiling on a nullability choice. Each choice was checked against every Kotlin caller in the repository.
  • A null argument throws at a different step. Each !! sits where the Java first used the value.
  • 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.
4 Kotlin files, 147 lines, replacing 4 Java files of 229 lines. 3 new unit test files, 299 lines. The API dump does not change. 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:19
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown

Kotlin migration progress

This pull request converts 228 lines of Java to Kotlin.

Module Kotlin LOC Java LOC Java staying (facade) Java left to convert Kotlin share
android-core 4,628 21,262 9,716 11,546 17.9%
android-kit-base 601 5,405 1,651 3,754 10.0%
Total 5,229 26,667 11,367 15,300 16.4%

Conversion progress: 7.6% (15,300 of 16,550 baseline Java LOC left to convert). 110 Java files and 58 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 converts classes on the upload and certificate-pinning network path, and it changes the declared shape of the public MParticleBaseClient and MParticleApiClient interfaces, so a look from the SDK team is still worthwhile.

What was reviewed:

  • MPUrlImpl and MPConnectionImpl against the deleted Java. Each !! dereferences at the point where the Java first used the value, and the SSL socket factory cast still throws for non-HTTPS connections, as it did before.
  • MParticleApiClient / AliasNetworkResponse: the constructor and setErrorMessage keep their JVM names, and @ NonNull/@ Nullable are kept on sendAliasRequest and getErrorMessage.
  • requestHandler as a non-null var: Kotlin callers (PinningTestHelper, UserAudiencesRetriever, UploadHandlerTest) are compatible. No Kotlin class implements either interface with non-null overrides that the change would break.
Extended reasoning...

The change converts four android-core networking types from Java to Kotlin in place (MParticleApiClient, MParticleBaseClient, MPUrlImpl, MPConnectionImpl) and adds about 300 lines of new unit tests. It touches the HTTP connection wrapper, including where the SSL socket factory is set for certificate pinning, but it adds no new logic there. Two public interfaces change their nullability and accessor declarations: requestHandler becomes a non-null property, and the message and uploadSettings parameters, which were @ NonNull, become nullable. The PR author says these are deliberate choices. CODEOWNERS assigns every path to the SDK team, and these API-surface decisions belong to them, so this is deferred rather than approved.

@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.90 KB +45 bytes
Download size 118.67 KB 118.72 KB +45 bytes
Dex bytes 216.75 KB 216.97 KB +232 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 +14 bytes
Dex bytes 2.23 MB 2.23 MB +220 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 +22 bytes +5.19 MB
Dex bytes 7.12 MB 7.12 MB +100 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": 2510, "baseline_install_bytes": 7526, "core_dex_bytes": 222180, "core_download_bytes": 124076, "core_install_bytes": 131327}

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": 4171552, "kit_download_bytes": 2548145, "kit_install_bytes": 2623275, "sdkplus_dex_bytes": 9293196, "sdkplus_download_bytes": 7985757, "sdkplus_install_bytes": 8165359}

Measured e4d332b merged into d247e12

@nickolas-dimitrakas
nickolas-dimitrakas merged commit d8dd619 into main Nov 3, 2026
47 checks passed
@nickolas-dimitrakas
nickolas-dimitrakas deleted the refactor/convert-four-b1-networking-classes-to-kotlin branch November 3, 2026 16:08
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