Skip to content

[in_app_purchase] Switch to Kotlin Pigeon generator #185545

Description

@stuartmorgan-g

As part of #158287 I've been switching our plugins over to the Kotlin Pigeon generator, and then relying on Kotlin/Java interop to leave the rest of the plugin in Java. As identified in my comments there, the lack of Java-style builders is a pain point for some conversions, and since IAP does a lot of mirroring of data classes that start on the native side, it will hit this in a significant way.

I can see three paths for switching to the Kotlin generator for this plugin:

  1. Just use positional params. From a readability standpoint, this is actually fine because essentially all the object construction happens in Translator.java, with code that looks like this:

        return new PlatformPurchaseHistoryRecord.Builder()
          .setPurchaseTime(purchaseHistoryRecord.getPurchaseTime())
          .setPurchaseToken(purchaseHistoryRecord.getPurchaseToken())
          .setSignature(purchaseHistoryRecord.getSignature())
          .setProducts(purchaseHistoryRecord.getProducts())
          .setDeveloperPayload(purchaseHistoryRecord.getDeveloperPayload())
          .setOriginalJson(purchaseHistoryRecord.getOriginalJson())
          .setQuantity((long) purchaseHistoryRecord.getQuantity())
          .build();

    Replacing it with:

        return new PlatformPurchaseHistoryRecord(
          purchaseHistoryRecord.getPurchaseTime(),
          purchaseHistoryRecord.getPurchaseToken(),
          purchaseHistoryRecord.getSignature(),
          purchaseHistoryRecord.getProducts(),
          purchaseHistoryRecord.getDeveloperPayload(),
          purchaseHistoryRecord.getOriginalJson(),
          (long) purchaseHistoryRecord.getQuantity());

    is fine for readability because the getters provide the names. My concern with doing that in this case (as we've done in small cases in other plugins) is that there's a fair amount amount of churn in the the underlying APIs (roughly every year there seem to be non-trivial changes), and while the second version is readable, it's not safe. If someone accidentally swaps the order order of two params with the same type, it will compile, and not be caught via review. Unit tests might or might not catch it depending on how thorough we are.

  2. Block on adding the optional builder generation to the Pigeon Kotlin generator (which would be blocked on the in-progress ffigen/jnigen mega-PR, since we don't want to cause conflicts with that)

  3. Convert just Translator.java to Kotlin, so that we have named arguments, which gives us the same reviewable safety as the builder pattern.

I'm curious what the Android team's preference is. I think 3 would be very simple, as it's a very straightforward file that Android Studio auto-conversion should handle without problems, but it would be a change from our current policy of only using Java for non-generated code.

Metadata

Metadata

Labels

p: in_app_purchasePlugin for in-app purchasepackageflutter/packages repository. See also p: labels.platform-androidAndroid applications specificallyteam-androidOwned by Android platform team

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions