refactor(google): unify horizon and play flavors with independent deps#19
Conversation
hyochan
changed the title
|
Warning Rate limit exceeded@hyochan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughRemoves the "auto" Android flavor and related Horizon stub wiring, consolidates Horizon/Play code into shared namespaces, relocates and renames modules/helpers, adds reflection-based module loading and managed coroutine scope, updates VSCode workspace configs, modifies Gradle/manifests, updates docs and CI, and adds a Play ViewModel and shared utilities. (41 words) Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application/UI
participant Ext as Flavor Extensions
participant Module as OpenIapModule (flavor impl)
participant Store as OpenIapStore
App->>Ext: OpenIapStore(context, alternativeBillingMode?, listener?)
Ext->>Module: construct Module (AlternativeBillingMode)
Module->>Module: lazy-read appId from AndroidManifest (if present)
Ext->>Store: wrap Module as OpenIapProtocol
Store->>Store: init storeScope (SupervisorJob)
Store->>Store: background refresh via storeScope.launch (replace GlobalScope)
sequenceDiagram
participant Old as Old Gradle (auto default)
participant New as New Gradle (explicit flavors)
participant Play as Play variant
participant Horizon as Horizon variant
Old->>Old: auto flavor selected by default (injected OCULUS_APP_ID)
Note over New: explicit build-time flavor selection replaces auto
New->>Play: build playDebug/playRelease (default)
New->>Horizon: build horizonDebug/horizonRelease
Horizon->>Module: Module reads appId from manifest at runtime (if present)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
packages/google/Example/build.gradle.kts (1)
56-59: Inconsistency in appId resolution between defaultConfig and horizon flavor is confirmed.The review comment's analysis is accurate. The actual file shows:
- defaultConfig (lines 30-34): Checks
EXAMPLE_HORIZON_APP_ID→EXAMPLE_OPENIAP_APP_ID→ empty string across both local and project properties- horizon flavor (lines 56-59): Only checks
EXAMPLE_HORIZON_APP_ID→ empty string, missing theEXAMPLE_OPENIAP_APP_IDfallbackThe horizon flavor omits the fallback to
EXAMPLE_OPENIAP_APP_IDthat exists in defaultConfig, creating an inconsistency. IfEXAMPLE_HORIZON_APP_IDis not set, the horizon flavor silently defaults to an empty string instead of attempting the secondary fallback. The suggested fix correctly mirrors defaultConfig's chain and should be applied.packages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/BillingConverters.kt (1)
60-71: Fix potential NPE when pricingPhases is null.Horizon logs use safe calls for offer.pricingPhases, so this direct access can crash. Switch to a safe fallback.
- pricingPhases = PricingPhasesAndroid( - pricingPhaseList = offer.pricingPhases.pricingPhaseList.map { phase -> + pricingPhases = PricingPhasesAndroid( + pricingPhaseList = (offer.pricingPhases?.pricingPhaseList.orEmpty()).map { phase ->packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt (1)
100-121: Remove unusedproductTypeparameter and fallbacktransactionIdtopurchaseToken.Two call sites require updates:
packages/google/openiap/src/play/java/dev/hyo/openiap/helpers/Helpers.kt:66– removeproductTypeargumentpackages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt:855– removeproductTypeargument- fun BillingPurchase.toPurchase(productType: String): PurchaseAndroid { + fun BillingPurchase.toPurchase(): PurchaseAndroid { val state = PurchaseState.fromBillingState(purchaseState) return PurchaseAndroid( ... - transactionId = orderId, + transactionId = orderId ?: purchaseToken, transactionDate = purchaseTime.toDouble() ) }This aligns the Google Play implementation with the existing Horizon pattern, which already uses parameterless
toPurchase().packages/google/openiap/build.gradle.kts (1)
16-16: Update namespace in build.gradle.kts AND reflect this change in OpenIapStore.kt Class.forName() call.Changing the namespace in build.gradle.kts will generate BuildConfig in the new namespace. However, the reflection at OpenIapStore.kt:535 still targets the old namespace, causing a ClassNotFoundException at runtime.
Both locations must be updated:
- build.gradle.kts:16 →
namespace = "dev.hyo.openiap"- OpenIapStore.kt:535 →
Class.forName("dev.hyo.openiap.BuildConfig")packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt (3)
267-307: UI method must run on main thread (dialog).showAlternativeBillingInformationDialog executes a UI API under Dispatchers.IO. This can crash or hang on some devices.
-override suspend fun showAlternativeBillingInformationDialog(activity: Activity): Boolean = withContext(Dispatchers.IO) { +override suspend fun showAlternativeBillingInformationDialog(activity: Activity): Boolean = withContext(Dispatchers.Main) {
555-611: launchBillingFlow must be invoked on main thread.Billing docs require calling launchBillingFlow from the UI thread. It's currently triggered within an IO context.
- val result = client.launchBillingFlow(activity, flowBuilder.build()) + val result = withContext(Dispatchers.Main) { + client.launchBillingFlow(activity, flowBuilder.build()) + }
79-83: Constructor listener is never registered; only logged.userChoiceBillingListener (single) is not added to userChoiceBillingListeners (set), so no callbacks are delivered.
class OpenIapModule( private val context: Context, private var alternativeBillingMode: AlternativeBillingMode = AlternativeBillingMode.NONE, - private var userChoiceBillingListener: dev.hyo.openiap.listener.UserChoiceBillingListener? = null + private var userChoiceBillingListener: dev.hyo.openiap.listener.UserChoiceBillingListener? = null ) : OpenIapProtocol, PurchasesUpdatedListener { @@ override fun addUserChoiceBillingListener(listener: OpenIapUserChoiceBillingListener) { userChoiceBillingListeners.add(listener) } @@ override fun setUserChoiceBillingListener(listener: dev.hyo.openiap.listener.UserChoiceBillingListener?) { userChoiceBillingListener = listener + userChoiceBillingListeners.clear() + listener?.let { l -> + // Bridge single-listener API into the multi-listener set + userChoiceBillingListeners.add( + OpenIapUserChoiceBillingListener { details -> l.onUserChoiceBilling(details) } + ) + } }Also applies to: 835-841, 1047-1049
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
546-571: Fix manifest key mismatch and remove unused appId parameter.Both issues are confirmed:
Manifest key mismatch (line 553): OpenIapStore reads
"com.meta.horizon.platform.ovr.OCULUS_APP_ID"but OpenIapModule (the Horizon module at line 81) reads"com.oculus.vr.APP_ID". These keys must match for the app ID to be found.Unused appId parameter (line 605, 623): The
appIdparameter is declared inloadHorizonModule(context: Context, appId: String)but never passed to the constructor on line 623:constructor.newInstance(context, noneMode, null). Remove the unused parameter or use it.Change line 553 to use the key expected by the Horizon module, and either drop or use the
appIdparameter.packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (1)
413-421: Replace magic number 5 with named constant for subscription replacement mode.Line 417: Use
BillingFlowParams.SubscriptionUpdateParams.ReplacementMode.CHARGE_FULL_PRICEinstead of the hardcoded integer 5. The Horizon billing compatibility library exposes the same ReplacementMode constants as Google Play Billing, making the named constant directly available and improving code clarity.val replacementMode = androidArgs.replacementModeAndroid ?: BillingFlowParams.SubscriptionUpdateParams.ReplacementMode.CHARGE_FULL_PRICE
🧹 Nitpick comments (26)
packages/google/Example/build.gradle.kts (1)
36-48: Clarify manifest placeholder override behavior across flavors.The comment on line 36 states "Ensure placeholder exists for all variants (play included)", but the horizon flavor (line 59) overrides
manifestPlaceholders["OCULUS_APP_ID"]after the defaultConfig value is set.If the intent is for play and horizon flavors to use different appId values (as the code suggests), the comment is misleading. If both flavors should use the same value from defaultConfig, the override on line 59 should be removed.
For clarity and consistency, consider:
- Explicitly documenting why horizon overrides the default placeholder while play does not
- Or, if only horizon needs override, refactor so the play flavor explicitly sets its value as well for clarity
packages/apple/.vscode/launch.json (1)
1-54: Good Apple development workflow coverage.The seven launch configurations cover common workflows (Xcode, build, test, package management). However, note that the iOS simulator OS version is hardcoded to 18.5 (line 15); consider parameterizing or documenting when this needs updates as new iOS versions are released.
.vscode/tasks.json (1)
70-88: Android "Build & Run" task has implicit dependencies.Line 73's task chains
gradlew installDebugwithadb shell am startand assumes both:
- An Android device/emulator is connected and ready
- App package name is
dev.hyo.martiewith.MainActivityas entry pointIf these assumptions change or a device is unavailable, the task will fail silently. Consider documenting this dependency or adding a guard.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/BillingConverters.kt (1)
134-141: Deduplicate toActiveSubscription() by moving it to main/utils.This extension is identical to the Play flavor. Centralize it in ProductExtensions.kt to avoid divergence. As per coding guidelines (place reusable helpers in main/utils).
Apply:
- fun PurchaseAndroid.toActiveSubscription(): ActiveSubscription = ActiveSubscription( - autoRenewingAndroid = autoRenewingAndroid, - isActive = true, - productId = productId, - purchaseToken = purchaseToken.orEmpty(), - transactionDate = transactionDate, - transactionId = transactionId.orEmpty() - )And add to packages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.kt:
+import dev.hyo.openiap.ActiveSubscription +import dev.hyo.openiap.PurchaseAndroid + +fun PurchaseAndroid.toActiveSubscription(): ActiveSubscription = ActiveSubscription( + autoRenewingAndroid = autoRenewingAndroid, + isActive = true, + productId = productId, + purchaseToken = purchaseToken.orEmpty(), + transactionDate = transactionDate, + transactionId = transactionId.orEmpty() +)packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt (1)
133-140: Move toActiveSubscription() to main/utils to avoid duplication.Same implementation exists in Horizon. Centralize it in ProductExtensions.kt. As per coding guidelines (reusable helpers → main/utils).
- fun PurchaseAndroid.toActiveSubscription(): ActiveSubscription = ActiveSubscription( - autoRenewingAndroid = autoRenewingAndroid, - isActive = true, - productId = productId, - purchaseToken = purchaseToken, - transactionDate = transactionDate, - transactionId = id - )(See addition in ProductExtensions.kt in related comment.)
packages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.kt (1)
1-33: Co-locate shared extension toActiveSubscription() here.Add the shared extension in this file and remove flavor copies. As per coding guidelines (helpers → main/utils).
package dev.hyo.openiap.utils import dev.hyo.openiap.Product import dev.hyo.openiap.ProductAndroid import dev.hyo.openiap.ProductSubscriptionAndroid import dev.hyo.openiap.Purchase import dev.hyo.openiap.PurchaseInput +import dev.hyo.openiap.ActiveSubscription +import dev.hyo.openiap.PurchaseAndroid @@ fun Purchase.toPurchaseInput(): PurchaseInput = this + +fun PurchaseAndroid.toActiveSubscription(): ActiveSubscription = ActiveSubscription( + autoRenewingAndroid = autoRenewingAndroid, + isActive = true, + productId = productId, + purchaseToken = purchaseToken.orEmpty(), + transactionDate = transactionDate, + transactionId = transactionId.orEmpty() +)packages/google/openiap/build.gradle.kts (1)
74-87: Tighten flavor-scoped dependencies; avoid duplicate compileOnly and unnecessary api.
- api re-exports types to consumers; use implementation unless public API exposes these types.
- compileOnly duplicates api’s compile classpath; drop compileOnly to reduce noise.
- add("playCompileOnly", "com.android.billingclient:billing-ktx:8.0.0") - add("playApi", "com.android.billingclient:billing-ktx:8.0.0") + add("playImplementation", "com.android.billingclient:billing-ktx:8.0.0") @@ - add("horizonCompileOnly", "com.meta.horizon.platform.ovr:android-platform-sdk:72") - add("horizonApi", "com.meta.horizon.platform.ovr:android-platform-sdk:72") - add("horizonCompileOnly", "com.meta.horizon.billingclient.api:horizon-billing-compatibility:1.1.1") - add("horizonApi", "com.meta.horizon.billingclient.api:horizon-billing-compatibility:1.1.1") + add("horizonImplementation", "com.meta.horizon.platform.ovr:android-platform-sdk:72") + add("horizonImplementation", "com.meta.horizon.billingclient.api:horizon-billing-compatibility:1.1.1")If you intentionally re-export these to consumers, keep api, but still remove compileOnly duplicates.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/ProductManager.kt (1)
56-65: ChangeproductType: Stringto useBillingClient.ProductTypetyped constants for consistency and type safety.The codebase already uses
BillingClient.ProductType.INAPPandBillingClient.ProductType.SUBSconstants in other helper functions (e.g.,queryPurchasesHorizonin Helpers.kt:29, 34). Update theproductTypeparameter in bothProductManager.getOrQuery()andqueryProductDetailsHorizon()to use these typed constants instead of rawStringto prevent typos and align with the pattern used elsewhere in the codebase.packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapError.kt (3)
297-313: fromBillingResponseCode ignores debugMessage and returns Unknown for unmapped codes.When unknown, preserve context for troubleshooting.
fun fromBillingResponseCode(responseCode: Int, debugMessage: String? = null): OpenIapError { return when (responseCode) { BillingClient.BillingResponseCode.USER_CANCELED -> UserCancelled ... - else -> UnknownError + else -> object : OpenIapError() { + override val code: String = UnknownError.code + override val message: String = + if (!debugMessage.isNullOrBlank()) "Unknown billing error: $debugMessage" else UnknownError.message + } } }
12-16: JSON payload lacks flavor/store context.platform="android" is ambiguous across Play and Horizon. Add a “store” or “flavor” field for analytics and debugging.
fun toJSON(): Map<String, Any?> = mapOf( "code" to toCode(this), "message" to (this.message ?: ""), "platform" to "android", + "store" to "horizon" )
18-27: Two “not found” variants (ProductNotFound and SkuNotFound).They represent the same condition but diverge in naming, multiplying handling paths.
- Keep a single variant (SkuNotFound) and remove ProductNotFound, or make ProductNotFound a thin wrapper around SkuNotFound for source compatibility.
- Update defaultMessages accordingly.
Also applies to: 146-155
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt (2)
852-856: Brittle product type inference in onPurchasesUpdated.Checking if productId contains “subs” is unreliable. Prefer purchase.isAutoRenewing or ProductManager lookup.
- val productType = if (purchase.products.any { it.contains("subs") }) BillingClient.ProductType.SUBS else BillingClient.ProductType.INAPP + val productType = + if (purchase.isAutoRenewing) BillingClient.ProductType.SUBS + else BillingClient.ProductType.INAPPOptionally, fall back to ProductManager when available for absolute correctness.
708-741: Android-suffixed function names in Android-only package.Per coding guidelines, functions shouldn’t use the Android suffix in Android-only modules (types may). Consider renaming acknowledgePurchaseAndroid/consumePurchaseAndroid to acknowledgePurchase/consumePurchase in the public API surface when feasible.
As per coding guidelines.
Also applies to: 725-740
packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/Helpers.kt (2)
51-59: Duplicate logging backends; prefer a single logger.Mixing android.util.Log and OpenIapLog doubles noise and complicates filtering.
- Remove android.util.Log calls and keep OpenIapLog only.
- If needed, make OpenIapLog delegate to android.util.Log internally. Based on learnings.
Also applies to: 69-93
98-106: Type the productType parameter to the SDK enum for safety.Using String is error-prone. If Horizon’s SDK exposes a ProductType type, accept that instead and map internally.
-internal suspend fun queryProductDetailsHorizon( - client: BillingClient?, - productManager: ProductManager, - skus: List<String>, - productType: String -): List<HorizonProductDetails> { +internal suspend fun queryProductDetailsHorizon( + client: BillingClient?, + productManager: ProductManager, + skus: List<String>, + productType: String // TODO: prefer BillingClient.ProductType when available +): List<HorizonProductDetails> {If the SDK only exposes strings, add @StringDef-like checks or constants.
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapViewModel.kt (4)
21-25: Don’t swallow failures in runCatching.Add onFailure logging (or post to status) so issues aren’t silent.
- viewModelScope.launch { runCatching { store.initConnection(config) } } + viewModelScope.launch { + runCatching { store.initConnection(config) } + .onFailure { dev.hyo.openiap.OpenIapLog.e("initConnection failed: ${it.message}", it, "OpenIapViewModel") } + }
26-33: Apply consistent error handling for fetchProducts.Mirror the onFailure pattern to aid diagnostics.
- runCatching { + runCatching { val request = ProductRequest(skus = skus, type = type) store.fetchProducts(request) - } + }.onFailure { dev.hyo.openiap.OpenIapLog.e("fetchProducts failed: ${it.message}", it, "OpenIapViewModel") }
35-41: Apply consistent error handling for restorePurchases.Add onFailure to avoid silent errors.
- runCatching { + runCatching { store.getAvailablePurchases(null) - } + }.onFailure { dev.hyo.openiap.OpenIapLog.e("restorePurchases failed: ${it.message}", it, "OpenIapViewModel") }
43-83: Deduplicate purchase props construction.Extract props building into a small helper to keep ViewModel focused on orchestration and reduce error surface.
+private fun buildProps(skus: List<String>, type: ProductQueryType): RequestPurchaseProps = when (type) { /*...existing when body...*/ } ... - val props = when (type) { ... } + val props = buildProps(skus, type)packages/google/openiap/src/play/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt (1)
21-26: Factory naming is non-idiomatic Kotlin.PascalCase top-level function compiles but is unusual. Consider a lowerCamelCase factory (e.g., openIapStore(...)) or an extension (Context.openIapStore(...)) for clarity.
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (3)
111-139: Post-purchase refresh: add bounded retry/backoff.Fixed 500ms delay + single fetch may miss eventual consistency. Consider a short bounded retry (e.g., 3 attempts with 300/600/1200ms) before giving up when connected.
- kotlinx.coroutines.delay(500) - val result = module.getAvailablePurchases(null) + var attempts = 0 + var result: List<Purchase> = emptyList() + val delays = longArrayOf(300, 600, 1200) + while (attempts < delays.size) { + kotlinx.coroutines.delay(delays[attempts]) + result = module.getAvailablePurchases(null) + if (result.isNotEmpty()) break + attempts++ + }
601-629: Horizon loader ignores appId and duplicates Play loader logic.Since appId is no longer needed, remove the parameter from loadHorizonModule and its call, or wire it if you reintroduce an appId‑aware constructor. Also consider factoring common reflection into one helper to DRY.
-private fun loadHorizonModule(context: Context, appId: String): OpenIapProtocol { +private fun loadHorizonModule(context: Context): OpenIapProtocol { ... - loadHorizonModule(context, resolvedAppId) + loadHorizonModule(context)
634-656: Reflection factory duplication.Play/Horizon loaders are identical except log messages. Extract a single constructModule(context) to reduce maintenance.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (3)
93-96: Background scope not canceled; potential leak.scope = SupervisorJob() + Dispatchers.IO is never canceled. Cancel it in endConnection() (and/or expose clear()) to avoid dangling jobs.
override val endConnection: MutationEndConnectionHandler = { withContext(Dispatchers.IO) { runCatching { billingClient?.endConnection() billingClient = null + // Cancel any pending background tasks + scope.coroutineContext.cancelChildren() true }.getOrElse { false } } }
707-787: onPurchasesUpdated: logging is thorough; consider lowering severity.Log.wtf may be too aggressive for routine callbacks. Prefer Log.i/w to avoid noisy crash reports on some OEMs.
309-542: Purchase flow orchestration: solid; consider throttling error emissions.Code verification confirms ProductManager correctly caches both INAPP and SUBS types via composite keys (productId + type), so cache collisions are prevented and your type-aware calls to
get(sku, desiredType)andputAll(list, desiredType)work as intended.The flow logic is well-structured: prefetch + fallback query, offer token resolution, and proactive post-launch purchase query are all sound. Activity and client null-checks are appropriate.
Optional: Consider throttling repeated error emissions per request—multiple SKU mismatches or offer validation failures currently fire listeners for each occurrence, which could create listener chatter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
.gitignore(0 hunks).vscode/extensions.json(1 hunks).vscode/launch.json(1 hunks).vscode/settings.json(1 hunks).vscode/tasks.json(1 hunks)gql/.vscode/settings.json(1 hunks)packages/apple/.vscode/launch.json(1 hunks)packages/apple/.vscode/settings.json(1 hunks)packages/google/Example/build.gradle.kts(1 hunks)packages/google/Example/src/horizon/AndroidManifest.xml(0 hunks)packages/google/openiap/build.gradle.kts(3 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapError.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt(6 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/Helpers.kt(3 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/ProductManager.kt(2 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/SharedHelpers.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/BillingConverters.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/AlternativeBillingMode.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/helpers/MissingCurrentActivityException.kt(0 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(6 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.kt(1 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt(1 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapViewModel.kt(1 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(0 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt(1 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.kt(1 hunks)
💤 Files with no reviewable changes (4)
- packages/google/openiap/src/play/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
- .gitignore
- packages/google/Example/src/horizon/AndroidManifest.xml
- packages/google/openiap/src/main/java/dev/hyo/openiap/helpers/MissingCurrentActivityException.kt
🧰 Additional context used
📓 Path-based instructions (2)
packages/google/openiap/src/main/java/dev/hyo/openiap/utils/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable Kotlin helpers in openiap/src/main/java/dev/hyo/openiap/utils/
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.kt
packages/google/openiap/src/main/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
packages/google/openiap/src/main/**/*.kt: In Android-only package, do not add Android suffix to function names (e.g., acknowledgePurchase, not acknowledgePurchaseAndroid)
Only use Android suffix for types that are part of a cross-platform API (e.g., ProductAndroid, PurchaseAndroid)
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/AlternativeBillingMode.kt
🧠 Learnings (4)
📚 Learning: 2025-10-18T05:54:54.790Z
Learnt from: CR
PR: hyodotdev/openiap#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T05:54:54.790Z
Learning: Applies to packages/google/openiap/src/main/**/*.kt : In Android-only package, do not add Android suffix to function names (e.g., acknowledgePurchase, not acknowledgePurchaseAndroid)
Applied to files:
packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/SharedHelpers.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/BillingConverters.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/play/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt
📚 Learning: 2025-10-18T05:46:51.571Z
Learnt from: hyochan
PR: hyodotdev/openiap#17
File: packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt:620-629
Timestamp: 2025-10-18T05:46:51.571Z
Learning: In packages/google/openiap/src/main/java/**/*.kt (Android-only package): DO NOT add Android suffix to function names, even for Android-specific APIs. Exception: Only use Android suffix for cross-platform API types (e.g., ProductAndroid, PurchaseAndroid) that contrast with iOS types. Examples of correct naming: isHorizonEnvironment(context: Context), buildModule(context: Context), acknowledgePurchase(), consumePurchase().
Applied to files:
packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/store/OpenIapStoreExtensions.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/SharedHelpers.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/BillingConverters.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/play/java/dev/hyo/openiap/store/OpenIapStoreExtensions.ktpackages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapViewModel.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt
📚 Learning: 2025-10-18T05:54:54.790Z
Learnt from: CR
PR: hyodotdev/openiap#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T05:54:54.790Z
Learning: Applies to packages/google/openiap/src/main/**/*.kt : Only use Android suffix for types that are part of a cross-platform API (e.g., ProductAndroid, PurchaseAndroid)
Applied to files:
packages/google/openiap/src/play/java/dev/hyo/openiap/utils/BillingConverters.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.kt
📚 Learning: 2025-10-18T05:54:54.790Z
Learnt from: CR
PR: hyodotdev/openiap#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-18T05:54:54.790Z
Learning: Applies to packages/google/openiap/src/main/java/dev/hyo/openiap/utils/**/*.kt : Place reusable Kotlin helpers in openiap/src/main/java/dev/hyo/openiap/utils/
Applied to files:
packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/Helpers.kt
🧬 Code graph analysis (2)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapError.kt (1)
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapError.kt (1)
toCode(292-292)
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapViewModel.kt (1)
packages/google/openiap/src/play/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt (1)
OpenIapStore(21-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Android
🔇 Additional comments (23)
packages/google/Example/build.gradle.kts (1)
43-48: Verify play flavor is the appropriate default for this example app.The play flavor is now marked as default (line 47) following the removal of the auto flavor. Confirm this aligns with your testing and distribution strategy for the Example app.
gql/.vscode/settings.json (1)
1-11: Configuration looks good.Spell-check dictionary appropriately includes project-specific terminology to prevent false positives. The terms align with GraphQL codegen, billing/store terms, and monorepo references.
packages/apple/.vscode/settings.json (1)
1-16: Comprehensive editor configuration for Apple package.The spell-check dictionary, file associations, and language-specific settings are well-suited for Swift/iOS development. Excludes appropriately cover build artifacts and dependencies.
.vscode/extensions.json (1)
1-10: Well-curated extension recommendations.The six extensions comprehensively cover the monorepo's multi-language toolchain (Java, Swift, Bun, ESLint, Prettier) and align with development workflows defined in tasks and launch configs.
.vscode/launch.json (2)
1-10: Launch configurations look appropriate.The remaining three configurations (Xcode, GQL, Docs) follow correct path patterns with properly set working directories.
Also applies to: 23-36
11-22: Android Studio launch script verified—no issues found.The script
packages/google/scripts/open-android-studio.shexists and is executable. The launch configuration is valid..vscode/settings.json (1)
1-44: Well-structured workspace editor configuration.The settings provide sensible defaults across the monorepo while delegating language-specific spell-check to package settings. Excludes, formatters, and language tab settings are consistent and appropriate.
.vscode/tasks.json (1)
1-247: Comprehensive and well-organized task suite.All 17 tasks are properly scoped with working directories, presentation settings, and groups. Commands are standard and consistent. The tasks provide good developer ergonomics for multi-package workflows across Apple, Google, GQL, and Docs.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/ProductManager.kt (1)
35-49: Overall solid async + caching flow.Cache-first, order-preserving, and cancellation-safe. LGTM.
packages/google/openiap/src/main/java/dev/hyo/openiap/utils/ProductExtensions.kt (2)
13-27: LGTM — shared toProduct() centralization reduces duplication.
33-33: PurchaseInput is confirmed as a typealias for Purchase—no issues found.The verification confirms that
PurchaseInputis defined aspublic typealias PurchaseInput = Purchasein the Types.kt file. Since it's a typealias, the identity conversion in the extension function is type-safe and valid. No changes are needed.packages/google/openiap/build.gradle.kts (1)
116-121: Confirm publishing only Play variant is intended.You now publish playRelease. If Horizon needs distribution too, consider adding a separate publication.
packages/google/openiap/src/main/java/dev/hyo/openiap/AlternativeBillingMode.kt (1)
7-14: Shared enum looks good and documents intent clearly.Centralizing the mode in main avoids flavor drift and simplifies API surface.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/Helpers.kt (1)
61-68: Good readiness guard before querying purchases.Prevents spurious calls and flakes when the client isn’t ready.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt (1)
24-28: Constructor extension looks clean and aligns flavors.Uses shared AlternativeBillingMode and keeps API concise. Note about manifest App ID is helpful.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/helpers/SharedHelpers.kt (3)
18-30: Listener await helper: lifecycle looks correct.Temporary listener + suspendCancellableCoroutine with cleanup on cancel/success is solid.
35-47: Error listener await helper: LGTM.Mirrors the success path with safe resume and cleanup.
109-117: Error mapping fallback is robust.Graceful ErrorCode parsing with Unknown fallback and targeted productId extraction is good.
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
68-70: Good move to a scoped CoroutineScope.Main + SupervisorJob with cancellation in clear() prevents leaks vs GlobalScope.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (3)
74-89: Manifest key aligns with Horizon module; ensure it’s the single source.This uses "com.oculus.vr.APP_ID". Please align the main store’s manifest read to the same key (or remove the other read) to avoid confusion. See related comment in OpenIapStore.kt.
815-833: Billing client builder: appId hook is good.Conditional setAppId(id) from manifest is correct; nice.
214-253: Restore + cache merge is a solid resiliency improvement.Merging query results with shared cache avoids gaps when callbacks are missed.
packages/google/openiap/src/play/java/dev/hyo/openiap/store/OpenIapStoreExtensions.kt (1)
33-39: No internal callers of the deprecated function—safe to proceed.The codebase contains zero calls to the deprecated
OpenIapStore(Context, Boolean)overload. All instantiations useOpenIapStore(context)with a single argument. The ReplaceWith guidance is clear and correct for external library users.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapErrorExtensions.kt (1)
9-9: Remove or utilize the unuseddebugMessageparameter.The
debugMessageparameter is declared but never used in the function body. Either remove it to clean up the API surface, or pass it to the error ifOpenIapErrorsupports storing debug information.If removing, apply this diff:
-fun OpenIapError.Companion.fromBillingResponseCode(responseCode: Int, debugMessage: String? = null): OpenIapError { +fun OpenIapError.Companion.fromBillingResponseCode(responseCode: Int): OpenIapError {packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapErrorExtensions.kt (1)
9-9: Remove or utilize the unuseddebugMessageparameter.The
debugMessageparameter is declared but never used. This is the same issue present in the Horizon flavor extension. Both files should be updated consistently.If removing, apply this diff:
-fun OpenIapError.Companion.fromBillingResponseCode(responseCode: Int, debugMessage: String? = null): OpenIapError { +fun OpenIapError.Companion.fromBillingResponseCode(responseCode: Int): OpenIapError {packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
610-635: Consider extracting common reflection logic to reduce duplication.The
loadPlayModuleandloadHorizonModulefunctions contain nearly identical reflection code. The only differences are the unusedappIdparameter inloadHorizonModuleand the error messages.Consider extracting the common reflection logic into a private helper function to improve maintainability:
private fun loadModuleViaReflection(context: Context, flavorName: String): OpenIapProtocol { return try { val clazz = Class.forName("dev.hyo.openiap.OpenIapModule") val alternativeBillingModeClass = Class.forName("dev.hyo.openiap.AlternativeBillingMode") val userChoiceBillingListenerClass = Class.forName("dev.hyo.openiap.listener.UserChoiceBillingListener") val constructor = clazz.getConstructor( Context::class.java, alternativeBillingModeClass, userChoiceBillingListenerClass ) val noneMode = alternativeBillingModeClass.enumConstants?.first { (it as Enum<*>).name == "NONE" } val instance = constructor.newInstance(context, noneMode, null) as OpenIapProtocol OpenIapLog.d("Successfully loaded OpenIapModule ($flavorName flavor)", "OpenIapStore") instance } catch (e: Throwable) { throw IllegalStateException("Failed to load OpenIapModule ($flavorName flavor). Make sure you're using the $flavorName flavor.", e) } } private fun loadHorizonModule(context: Context): OpenIapProtocol { return loadModuleViaReflection(context, "Horizon") } private fun loadPlayModule(context: Context): OpenIapProtocol { return loadModuleViaReflection(context, "Play") }Also update line 570 to not pass the unused
appIdparameter:- loadHorizonModule(context, resolvedAppId) + loadHorizonModule(context)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/docs/src/pages/docs/horizon-setup.tsx(6 hunks)packages/google/Example/build.gradle.kts(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapErrorExtensions.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapError.kt(0 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(6 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapErrorExtensions.kt(1 hunks)
💤 Files with no reviewable changes (1)
- packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapError.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/google/Example/build.gradle.kts
🧰 Additional context used
📓 Path-based instructions (3)
packages/google/openiap/src/main/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
packages/google/openiap/src/main/**/*.kt: In Android-only package, do not add Android suffix to function names (e.g., acknowledgePurchase, not acknowledgePurchaseAndroid)
Only use Android suffix for types that are part of a cross-platform API (e.g., ProductAndroid, PurchaseAndroid)
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
packages/docs/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/docs/src/**/*.{ts,tsx}: Platform suffixes in function names for documentation site examples: iOS-only functions end with IOS; Android-only functions end with Android; cross-platform functions have no suffix
Use kebab-case for search modal IDs in docs (e.g., id: 'request-products')
Trigger opening modals from pages/components by calling openAuthModal from the signals module
Components must not overflow parent boundaries: ensure children fit; use overflow-hidden on parents as needed; apply break-words for long text; use whitespace-nowrap for nav items
Delete unused components, functions, imports; avoid commented-out code; remove unused variables/parameters
Wrap any Promise-returning function used in a void context (e.g., React event handlers) with the void operator
Files:
packages/docs/src/pages/docs/horizon-setup.tsx
packages/docs/src/**/*.{md,mdx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Respect deprecations in documentation: replace buy-promoted-product-ios with requestPurchaseOnPromotedProductIOS; requestProducts with fetchProducts; get-storefront-ios with getStorefront
Files:
packages/docs/src/pages/docs/horizon-setup.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-18T05:46:51.571Z
Learnt from: hyochan
PR: hyodotdev/openiap#17
File: packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt:620-629
Timestamp: 2025-10-18T05:46:51.571Z
Learning: In packages/google/openiap/src/main/java/**/*.kt (Android-only package): DO NOT add Android suffix to function names, even for Android-specific APIs. Exception: Only use Android suffix for cross-platform API types (e.g., ProductAndroid, PurchaseAndroid) that contrast with iOS types. Examples of correct naming: isHorizonEnvironment(context: Context), buildModule(context: Context), acknowledgePurchase(), consumePurchase().
Applied to files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
🔇 Additional comments (13)
packages/docs/src/pages/docs/horizon-setup.tsx (5)
60-63: LGTM! Clear explanation of build flavor architecture.The note accurately describes how build flavors determine which billing SDK is compiled into the APK, and clarifies the default behavior when no flavor is specified.
195-213: Excellent refactoring of the setup instructions.The section has been appropriately renamed from "Automatic Platform Detection" to "Default Constructor" and the code example now clearly demonstrates the simplified approach where build flavors determine the compiled SDK. The inline comments effectively explain the behavior for each flavor variant.
241-242: LGTM! Usage example updated correctly.The simplified constructor usage is consistent with the build-flavor-based approach described earlier in the documentation.
403-412: Well-structured guidance for Android Studio users.The build variant instructions have been properly updated to reflect only the horizon and play flavors, and the added paragraph clearly explains the purpose of building separate APKs for each platform.
439-447: Command line examples are clear and complete.The Gradle commands correctly demonstrate building and installing for both flavor variants, with helpful comments indicating the target device types.
packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapErrorExtensions.kt (1)
1-24: Good separation of flavor-specific concerns.The identical structure between Horizon and Play flavor extensions ensures API consistency while keeping implementations separate. Be mindful that changes to error mapping logic will need to be applied to both files.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapErrorExtensions.kt (1)
8-8: @Suppress("DEPRECATION") is justified and appropriate.The BillingResponseCode enum itself is not deprecated, but some individual response-code constants like SERVICE_TIMEOUT have been marked deprecated in the Google Play Billing API. The code in
OpenIapErrorExtensions.ktusesBillingClient.BillingResponseCode.SERVICE_TIMEOUT(line 21), which is deprecated. The@Suppress("DEPRECATION")annotation is necessary and correctly applied to suppress the deprecation warning for this usage. No changes required.packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (6)
38-38: LGTM! Good documentation and appropriate imports.The comment clearly explains the reflection-based loading approach, and the new imports support the scoped coroutine pattern introduced later in the file.
Also applies to: 45-46
68-69: LGTM! Proper lifecycle management with scoped coroutines.Replacing GlobalScope with a scoped CoroutineScope using SupervisorJob is excellent for memory safety and lifecycle management. The SupervisorJob ensures that a failure in one coroutine doesn't cancel the entire scope.
199-199: LGTM! Proper scope cleanup.Canceling the storeScope in the
clear()method prevents memory leaks and ensures that background operations don't outlive the store instance.
569-578: LGTM! Clear routing logic for flavor selection.The routing between Horizon and Play module loaders is straightforward and includes appropriate logging for debugging.
547-562: Remove unused appId resolution logic from buildModule.The
resolvedAppIdcomputed at lines 547–562 is passed toloadHorizonModule(line 570), but the function never uses it. The parameter is accepted but not referenced in the implementation (lines 585–609).The Horizon
OpenIapModuleconstructor does not accept an appId parameter; instead, it reads the Oculus App ID internally from the manifest using a lazy property. This dead code should be removed:
- Delete the
resolvedAppIdresolution block (lines 547–562)- Update the
loadHorizonModulecall to remove the second argument:loadHorizonModule(context)- Update the
loadHorizonModulefunction signature to remove theappIdparameter
111-138: Threading verification complete—module.getAvailablePurchases properly handles I/O dispatching.The
getAvailablePurchases()function is a suspend function that internally useswithContext(Dispatchers.IO)in both the Horizon and Play billing implementations. Calling this suspend function fromstoreScope.launch(which uses Dispatchers.Main) is safe and correct—the function manages its own thread switching appropriately. No additional wrapping is required.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
532-548: BuildConfig package mismatch will force default to "play".Hard-coded io.github.hyochan.openiap.BuildConfig contradicts unified package (dev.hyo.openiap). This likely throws and silently defaults to "play". Fix with a robust fallback.
- val defaultStore = try { - val buildConfig = Class.forName("io.github.hyochan.openiap.BuildConfig") - val storeValue = buildConfig.getField("OPENIAP_STORE").get(null) as? String ?: "play" - android.util.Log.i("OpenIapStore", "BuildConfig.OPENIAP_STORE = $storeValue") - storeValue - } catch (e: Throwable) { - android.util.Log.w("OpenIapStore", "Failed to read BuildConfig.OPENIAP_STORE: ${e.message}") - "play" - } + val defaultStore = run { + val candidates = listOf( + "dev.hyo.openiap.BuildConfig", + "${context.applicationContext.packageName}.BuildConfig" + ).distinct() + var value: String? = null + for (cn in candidates) { + try { + val bc = Class.forName(cn) + value = (bc.getField("OPENIAP_STORE").get(null) as? String) + if (!value.isNullOrBlank()) { + OpenIapLog.i("BuildConfig.OPENIAP_STORE($cn) = $value", "OpenIapStore") + break + } + } catch (_: Throwable) { /* try next */ } + } + value ?: "play" + } @@ - android.util.Log.i("OpenIapStore", "buildModule: selected=$selected, defaultStore=$defaultStore") - OpenIapLog.d("buildModule: selected=$selected, defaultStore=$defaultStore", "OpenIapStore") + OpenIapLog.d("buildModule: selected=$selected, defaultStore=$defaultStore", "OpenIapStore")
🧹 Nitpick comments (4)
.github/workflows/ci.yml (1)
25-32: Refactor: Extract retry logic to reduce duplication.The same retry loop is duplicated identically across 4 jobs. Consider extracting to a reusable composite action or shared shell script to improve maintainability.
Also applies to: 66-73, 105-112, 139-146
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (3)
45-47: Scoped coroutines replace GlobalScope — good; consider Main.immediate.Great move to a SupervisorJob-backed scope and cancel in clear(). For snappier UI resume, prefer Dispatchers.Main.immediate.
-import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel - private val storeScope = CoroutineScope(Dispatchers.Main + SupervisorJob()) + private val storeScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)Also applies to: 68-70, 199-200
111-139: Post-purchase refresh: offload potential I/O and unify logging.module.getAvailablePurchases(null) may block; run it on IO and keep logs consistent via OpenIapLog.
+import kotlinx.coroutines.withContext @@ - storeScope.launch { + storeScope.launch { try { - android.util.Log.i("OpenIapStore", "Purchase update received, refreshing available purchases") + OpenIapLog.i("Purchase update received, refreshing available purchases", "OpenIapStore") @@ - kotlinx.coroutines.delay(500) + kotlinx.coroutines.delay(500) @@ - if (!isConnected.value) { - android.util.Log.w("OpenIapStore", "Not connected, skipping purchase refresh (connection will be restored on next app start)") + if (!isConnected.value) { + OpenIapLog.w("Not connected, skipping purchase refresh (will refresh on next app start)", "OpenIapStore") return@launch } @@ - android.util.Log.i("OpenIapStore", "About to call module.getAvailablePurchases(null)") - val result = module.getAvailablePurchases(null) - android.util.Log.i("OpenIapStore", "module.getAvailablePurchases returned: ${result.size} purchases") + OpenIapLog.d("About to call module.getAvailablePurchases(null)", "OpenIapStore") + val result = withContext(Dispatchers.IO) { module.getAvailablePurchases(null) } + OpenIapLog.i("module.getAvailablePurchases returned: ${result.size} purchases", "OpenIapStore") @@ - android.util.Log.i("OpenIapStore", "Available purchases updated: ${result.size} purchases") + OpenIapLog.i("Available purchases updated: ${result.size} purchases", "OpenIapStore") } catch (e: Exception) { - android.util.Log.e("OpenIapStore", "Failed to refresh purchases after update", e) - e.printStackTrace() + OpenIapLog.e("Failed to refresh purchases after update", e, "OpenIapStore") } }
- If module.getAvailablePurchases is guaranteed non-blocking on Main, ignore the IO offload; please confirm.
549-559: Duplicate reflection loaders; consolidate into one to reduce drift.Bodies are identical except messages. Collapse to a single loader with a flavor label.
@@ - return when (selected) { - "horizon", "meta", "quest" -> { - OpenIapLog.d("Loading OpenIapModule (Horizon flavor)", "OpenIapStore") - loadHorizonModule(context) - } - else -> { - // Default to Play Store (includes "play", "google", "gplay", "googleplay", "gms") - OpenIapLog.d("Loading OpenIapModule (Play flavor)", "OpenIapStore") - loadPlayModule(context) - } - } + return when (selected) { + "horizon", "meta", "quest" -> + loadModule(context, "Horizon") + else -> + loadModule(context, "Play") // includes "play", "google", "gplay", "googleplay", "gms" + } } -/** - * Load OpenIapModule (Horizon flavor) via reflection - * Note: Horizon flavor now uses the same package and class name as Play flavor - * App ID is read from AndroidManifest.xml by the Horizon module - */ -private fun loadHorizonModule(context: Context): OpenIapProtocol { - return try { - // Both Play and Horizon flavors now use the same class name: dev.hyo.openiap.OpenIapModule - val clazz = Class.forName("dev.hyo.openiap.OpenIapModule") - val alternativeBillingModeClass = Class.forName("dev.hyo.openiap.AlternativeBillingMode") - val userChoiceBillingListenerClass = Class.forName("dev.hyo.openiap.listener.UserChoiceBillingListener") - - val constructor = clazz.getConstructor( - Context::class.java, - alternativeBillingModeClass, - userChoiceBillingListenerClass - ) - - // Get NONE enum value - val noneMode = alternativeBillingModeClass.enumConstants?.first { - (it as Enum<*>).name == "NONE" - } - - val instance = constructor.newInstance(context, noneMode, null) as OpenIapProtocol - OpenIapLog.d("Successfully loaded OpenIapModule (Horizon flavor)", "OpenIapStore") - instance - } catch (e: Throwable) { - throw IllegalStateException("Failed to load OpenIapModule (Horizon flavor). Make sure you're using the Horizon flavor.", e) - } -} - -/** - * Load OpenIapModule (Play flavor) via reflection - */ -private fun loadPlayModule(context: Context): OpenIapProtocol { - return try { - // Try to load OpenIapModule with default parameters (Context, NONE mode, null listener) - val clazz = Class.forName("dev.hyo.openiap.OpenIapModule") - val alternativeBillingModeClass = Class.forName("dev.hyo.openiap.AlternativeBillingMode") - val userChoiceBillingListenerClass = Class.forName("dev.hyo.openiap.listener.UserChoiceBillingListener") - - val constructor = clazz.getConstructor( - Context::class.java, - alternativeBillingModeClass, - userChoiceBillingListenerClass - ) - - // Get NONE enum value - val noneMode = alternativeBillingModeClass.enumConstants?.first { - (it as Enum<*>).name == "NONE" - } - - constructor.newInstance(context, noneMode, null) as OpenIapProtocol - } catch (e: Throwable) { - throw IllegalStateException("Failed to load OpenIapModule. Make sure you're using the Play flavor.", e) - } -} +private fun loadModule(context: Context, flavorLabel: String): OpenIapProtocol { + return try { + val clazz = Class.forName("dev.hyo.openiap.OpenIapModule") + val altMode = Class.forName("dev.hyo.openiap.AlternativeBillingMode") + val ucbListener = Class.forName("dev.hyo.openiap.listener.UserChoiceBillingListener") + val ctor = clazz.getConstructor( + Context::class.java, + altMode, + ucbListener + ) + val noneMode = altMode.enumConstants + ?.firstOrNull { (it as Enum<*>).name == "NONE" } + ?: altMode.enumConstants?.firstOrNull() + ?: error("AlternativeBillingMode enum has no constants") + val instance = ctor.newInstance(context, noneMode, null) as OpenIapProtocol + OpenIapLog.d("Successfully loaded OpenIapModule ($flavorLabel flavor)", "OpenIapStore") + instance + } catch (e: Throwable) { + throw IllegalStateException("Failed to load OpenIapModule ($flavorLabel flavor). Ensure the correct flavor is selected.", e) + } +}Also applies to: 562-617
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/ci.yml(4 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt(7 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapError.kt(3 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(6 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt(2 hunks)packages/google/openiap/src/test/java/dev/hyo/openiap/OpenIapErrorTest.kt(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/google/openiap/src/main/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
packages/google/openiap/src/main/**/*.kt: In Android-only package, do not add Android suffix to function names (e.g., acknowledgePurchase, not acknowledgePurchaseAndroid)
Only use Android suffix for types that are part of a cross-platform API (e.g., ProductAndroid, PurchaseAndroid)
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapError.kt
🧠 Learnings (1)
📚 Learning: 2025-10-18T05:46:51.571Z
Learnt from: hyochan
PR: hyodotdev/openiap#17
File: packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt:620-629
Timestamp: 2025-10-18T05:46:51.571Z
Learning: In packages/google/openiap/src/main/java/**/*.kt (Android-only package): DO NOT add Android suffix to function names, even for Android-specific APIs. Exception: Only use Android suffix for cross-platform API types (e.g., ProductAndroid, PurchaseAndroid) that contrast with iOS types. Examples of correct naming: isHorizonEnvironment(context: Context), buildModule(context: Context), acknowledgePurchase(), consumePurchase().
Applied to files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Android
🔇 Additional comments (14)
packages/google/openiap/src/test/java/dev/hyo/openiap/OpenIapErrorTest.kt (2)
201-205: LGTM! Test correctly validates updated ServiceTimeout error.The test now validates the string-based error code and generic billing service message, aligning with the cross-flavor error handling refactor.
340-340: LGTM! Test correctly validates ServiceTimeout code mapping.The test correctly verifies that
toCode(ServiceTimeout)returns the string"service-timeout".packages/google/openiap/src/horizon/java/dev/hyo/openiap/OpenIapModule.kt (4)
75-88: LGTM! Well-implemented manifest reading with proper error handling.The lazy property correctly reads the Oculus App ID from AndroidManifest meta-data with appropriate error handling and logging.
824-830: LGTM! Correct conditional appId usage.The code properly handles the nullable appId and only sets it on the BillingClient when a valid value exists.
631-631: LGTM! Correct error type for unsupported feature.The change from
NotSupportedtoFeatureNotSupportedaligns with the error handling refactor and correctly indicates that receipt validation is not supported on Horizon.
666-685: LGTM! Appropriate visibility change for storefront access.Making
getStorefront()public provides external access to country code information, aligning with the cross-platform API pattern.packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapError.kt (3)
189-189: LGTM! Generic message appropriate for cross-flavor support.The change from "Google Play service" to "Billing service" makes the error message platform-agnostic, supporting both Play and Horizon flavors.
233-237: LGTM! ServiceTimeout uses string-based code for flexibility.The change to a string constant code
"service-timeout"and generic billing service message aligns with the cross-flavor refactor. Note that this differs from most other errors that useErrorCode.X.rawValue.
251-282: LGTM! Consistent refactor to CODE-based map keys.The refactor from
ErrorCode.X.rawValuetoX.CODEfor all map keys is consistent and makes the code more direct. All error messages are preserved correctly.packages/google/openiap/src/play/java/dev/hyo/openiap/OpenIapModule.kt (2)
69-69: LGTM! Helpful comment documenting the refactor.The comment clearly indicates that
AlternativeBillingModeis now shared in the main source set, supporting the cross-flavor unification.
760-760: LGTM! Consistent error type across flavors.The change to
FeatureNotSupportedmatches the Horizon flavor and correctly indicates that receipt validation is not supported on Android.packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (3)
38-38: Naming/logging note — OKReflection comment is clear. Function names avoid Android suffixes; only types carry the suffix. Good alignment with our guideline. Based on learnings
66-66: NoteAck on moving Play-specific alternative billing constructors to extensions. Keeps core API clean.
329-329: Review comment is incorrect—FeatureNotSupported is not replacing NotSupported.The codebase has no
OpenIapError.NotSupportederror type (never did).FeatureNotSupportedis an established error already defined in the sealed class with message "Requested feature is not supported by Play Store" and already used consistently throughout tests, modules, and error handling. The code at line 329 simply uses this existing error type; no behavior change or replacement has occurred.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/google/openiap/src/test/java/dev/hyo/openiap/OpenIapErrorTest.kt (1)
196-196: Consider replacing hardcoded strings with constants.The hardcoded strings
"service-disconnected"and"service-timeout"are inconsistent with other tests that useErrorCode.*.rawValue. If these codes don't exist in theErrorCodeenum, consider adding them for consistency and maintainability.fun `ServiceDisconnected has correct code and message`() { val error = OpenIapError.ServiceDisconnected - assertEquals("service-disconnected", error.code) + assertEquals(ErrorCode.ServiceDisconnected.rawValue, error.code) assertEquals("Play Store service is not connected", error.message) }fun `ServiceTimeout has correct code and message`() { val error = OpenIapError.ServiceTimeout - assertEquals("service-timeout", error.code) + assertEquals(ErrorCode.ServiceTimeout.rawValue, error.code) assertEquals("The request has reached the maximum timeout before billing service responds", error.message) }Also applies to: 203-203
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/google/openiap/src/test/java/dev/hyo/openiap/OpenIapErrorTest.kt(4 hunks)
🔇 Additional comments (2)
packages/google/openiap/src/test/java/dev/hyo/openiap/OpenIapErrorTest.kt (2)
208-246: LGTM: Comprehensive test coverage.The test suite thoroughly covers all error types with clear, well-structured tests for individual error properties, JSON serialization, billing response code mapping, error code retrieval, and default messages.
Also applies to: 248-267, 270-310, 313-348, 350-355
275-302: All CODE properties verified—no issues found.The OpenIapError implementation properly defines CODE properties for all 28 error types referenced in the test. ProductNotFound and SkuNotFound, despite being classes with constructor parameters, correctly define companion objects with CODE properties that are accessible via the
OpenIapError.ErrorType.CODEpattern.
1 participant
Completely separated Horizon and Play flavors by moving all flavor-specific code to independent source sets (src/horizon and src/play), unified both flavors to use identical package names (dev.hyo.openiap) and class names (OpenIapModule), standardized constructor signatures to remove appId parameter (Horizon reads from AndroidManifest.xml instead), and replaced GlobalScope with scoped CoroutineScope for proper lifecycle management and memory safety.
Summary by CodeRabbit
New Features
Documentation
Refactor
Bug Fixes