feat(horizon): add horizon support with flavor#17
Conversation
WalkthroughAdds Horizon (Meta Quest) billing support and an OpenIapProtocol abstraction, implements Horizon and Play modules, introduces product-flavor build configuration and runtime store selection, adds Horizon-specific helpers/converters and product-flavor build/publish changes, and updates example app UI, manifests, and Gradle settings for multi-store behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant App as MainActivity
participant Store as OpenIapStore
participant Builder as buildModule()
participant Detect as isHorizonEnvironment()
participant HorizonMod as OpenIapHorizonModule
participant PlayMod as OpenIapModule
participant Protocol as OpenIapProtocol
App->>Store: instantiate OpenIapStore(context)
Store->>Builder: buildModule(context, store?, appId?)
Builder->>Detect: isHorizonEnvironment(context)
alt Horizon detected
Detect-->>Builder: true
Builder->>HorizonMod: instantiate (appId)
HorizonMod-->>Builder: returns OpenIapProtocol
else Play fallback
Detect-->>Builder: false
Builder->>PlayMod: instantiate
PlayMod-->>Builder: returns OpenIapProtocol
end
Builder-->>Store: module (OpenIapProtocol)
App->>Protocol: initConnection()
Protocol-->>App: connected
sequenceDiagram
participant Screen as SubscriptionScreen
participant Store as OpenIapStore
participant Protocol as OpenIapProtocol
participant Module as PlatformModule
participant Billing as BillingClient
Screen->>Store: remember iapStore
Screen->>Protocol: initConnection()
Protocol->>Module: establish connection
Module->>Billing: connect()
Billing-->>Module: ready
Module-->>Protocol: connected
rect rgba(200,240,220,0.12)
note over Screen,Module: Fetch products & select offer
Screen->>Module: fetchProducts(skus)
Module->>Billing: queryProductDetails()
Billing-->>Module: product details / offers
Module-->>Screen: product list
end
rect rgba(255,230,230,0.10)
note over Screen,Module: Purchase flow
Screen->>Module: requestPurchase(productId, offer, activity)
Module->>Billing: launchBillingFlow()
Billing-->>Module: purchase result
Module-->>Screen: purchase success/failure
end
Screen->>Protocol: endConnection()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/google/openiap/build.gradle.kts (1)
11-14: versions file path is brittle; add fallback to property and existence check.Current reference breaks when the build runs at repo root. Guard reads and allow overriding via -POPENIAP_GOOGLE_VERSION.
-// Read version from monorepo root openiap-versions.json -val versionsFile = File(rootDir.parentFile.parentFile, "openiap-versions.json") -val versionsJson = JsonSlurper().parseText(versionsFile.readText()) as Map<*, *> -val openIapVersion: String = versionsJson["google"]?.toString() ?: "1.0.0" +// Read version from monorepo root openiap-versions.json, with safe fallbacks +val openIapVersion: String = (project.findProperty("OPENIAP_GOOGLE_VERSION") as String?) + ?: run { + val candidates = listOfNotNull( + File(rootDir, "openiap-versions.json"), + rootDir.parentFile?.let { File(it, "openiap-versions.json") }, + rootDir.parentFile?.parentFile?.let { File(it, "openiap-versions.json") } + ) + val vf = candidates.firstOrNull { it.exists() } + if (vf != null) { + val json = JsonSlurper().parseText(vf.readText()) as Map<*, *> + json["google"]?.toString() ?: "1.0.0" + } else { + "1.0.0" + } + }packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (4)
618-621: launchBillingFlow must run on the main thread.Calling launchBillingFlow() from Dispatchers.IO risks crashes/ANRs; move to Dispatchers.Main.
Apply:
- val result = client.launchBillingFlow(activity, flowBuilder.build()) + val result = withContext(Dispatchers.Main) { + client.launchBillingFlow(activity, flowBuilder.build()) + }
620-631: Clear currentPurchaseCallback on immediate launch failure.Prevent stale callback retention on non-OK result.
purchaseErrorListeners.forEach { listener -> runCatching { listener.onPurchaseError(err) } } currentPurchaseCallback?.invoke(Result.success(emptyList())) + currentPurchaseCallback = null
855-861: Redact sensitive data in logs (purchaseToken, orderId).Purchase tokens/order IDs are sensitive; avoid full logging.
- Log.d( - TAG, - "[Purchase $index] token=${purchase.purchaseToken} orderId=${purchase.orderId} state=${purchase.purchaseState} autoRenew=${purchase.isAutoRenewing} acknowledged=${purchase.isAcknowledged} products=${purchase.products}" - ) + val redactedToken = purchase.purchaseToken.take(8) + "…" + val redactedOrder = purchase.orderId?.take(8)?.plus("…") + Log.d( + TAG, + "[Purchase $index] token=$redactedToken orderId=$redactedOrder state=${purchase.purchaseState} " + + "autoRenew=${purchase.isAutoRenewing} acknowledged=${purchase.isAcknowledged} products=${purchase.products}" + )
242-269: Guard reflection for Alternative Billing availability.On older Billing libraries, getMethod(...) can throw; fail closed and log instead of crashing.
- val checkAvailabilityMethod = client.javaClass.getMethod( - "isAlternativeBillingOnlyAvailableAsync", - com.android.billingclient.api.AlternativeBillingOnlyAvailabilityListener::class.java - ) + val checkAvailabilityMethod = runCatching { + client.javaClass.getMethod( + "isAlternativeBillingOnlyAvailableAsync", + Class.forName("com.android.billingclient.api.AlternativeBillingOnlyAvailabilityListener") + ) + }.getOrElse { + OpenIapLog.w("Alternative billing availability API not found: ${it.message}", TAG) + return@withContext false + }
🧹 Nitpick comments (12)
packages/google/Example/build.gradle.kts (1)
21-23: Remove duplicated BuildConfig fields from defaultConfig; keep them per-flavor.They’re redefined in each flavor; defaultConfig copies are redundant and risk drift.
- buildConfigField("String", "OPENIAP_STORE", "\"play\"") - buildConfigField("String", "HORIZON_APP_ID", "\"\"") @@ productFlavors { create("play") { dimension = "store" buildConfigField("String", "OPENIAP_STORE", "\"play\"") buildConfigField("String", "HORIZON_APP_ID", "\"\"") } create("horizon") { dimension = "store" buildConfigField("String", "OPENIAP_STORE", "\"horizon\"") buildConfigField("String", "HORIZON_APP_ID", "\"\"") } } @@ buildFeatures { compose = true buildConfig = true }Also applies to: 27-37, 65-65
packages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/HorizonBillingConverters.kt (2)
98-123: Remove unused parameter from toPurchase.productType is never used; drop it to avoid confusion.
- fun HorizonPurchase.toPurchase(productType: String): PurchaseAndroid { + fun HorizonPurchase.toPurchase(): PurchaseAndroid { val token = purchaseToken val productsList = products ?: emptyList() val purchaseState = PurchaseState.PurchasedAdditionally, ensure call sites pass no arg (horizon-only sources).
125-133: Consider deriving isActive from renewal/expiry when available.Hardcoding isActive = true may mislead UIs post-expiry. If Horizon exposes expiry/auto-renew, set isActive accordingly.
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (4)
312-313: Idempotency guard is fine; consider logging when finishTransaction handler is missing.Optional: log a warning to aid debugging if handler is null.
325-332: Return-type clarity for deepLinkToSubscriptions.Expose an explicit return type (e.g., Unit or Boolean?) to stabilize API surface.
-suspend fun deepLinkToSubscriptions(options: DeepLinkOptions) = module.mutationHandlers.deepLinkToSubscriptions?.invoke(options) +suspend fun deepLinkToSubscriptions(options: DeepLinkOptions): Unit = + module.mutationHandlers.deepLinkToSubscriptions?.invoke(options) ?: UnitOptionally, rename to deepLinkToSubscriptionsAndroid to match guidelines for Android-specific APIs. As per coding guidelines.
511-513: Locale‑safe lowercasing.Use Locale.ROOT to avoid edge cases (e.g., Turkish i).
- val selected = (store ?: defaultStore).lowercase() + val selected = (store ?: defaultStore).lowercase(java.util.Locale.ROOT)
546-555: Use non-deprecated getPackageInfo on API 33+.Current call is deprecated; add SDK check and use PackageInfoFlags.
-private fun isHorizonEnvironment(context: Context): Boolean { +private fun isHorizonEnvironment(context: Context): Boolean { val manufacturer = android.os.Build.MANUFACTURER.lowercase() if (manufacturer.contains("meta") || manufacturer.contains("oculus")) return true return try { - context.packageManager.getPackageInfo("com.oculus.vrshell", 0) + val pm = context.packageManager + if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.TIRAMISU) { + pm.getPackageInfo("com.oculus.vrshell", android.content.pm.PackageManager.PackageInfoFlags.of(0)) + } else { + @Suppress("DEPRECATION") + pm.getPackageInfo("com.oculus.vrshell", 0) + } true } catch (_: Throwable) { false } }packages/google/openiap/build.gradle.kts (1)
69-73: Scope Play Billing Library to the play flavor to reduce unnecessary dependencies in horizon builds.The suggestion to use
add("playApi", ...)is valid and recommended. The Horizon artifactcom.meta.horizon.billingclient.api:horizon-billing-compatibility:1.1.1is the latest stable version, and the build correctly defines both flavors ("play" and "horizon"). Changing fromapi()toplayApi()will scope the Google Play Billing Library exclusively to the play flavor, preventing it from being included in horizon artifact builds—aligning with the established pattern ofhorizonImplementationfor the Horizon Billing Compatibility Library.-// Google Play Billing Library (align with app/lib v8) -api("com.android.billingclient:billing-ktx:8.0.0") +// Google Play Billing Library (align with app/lib v8) - only for play flavor +add("playApi", "com.android.billingclient:billing-ktx:8.0.0") // Horizon Billing Compatibility Library (for horizon flavor only) add("horizonImplementation", "com.meta.horizon.billingclient.api:horizon-billing-compatibility:1.1.1")packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapProtocol.kt (1)
33-47: Android-specific API names should end with Android (guideline).In main/java, Android-specific functions must carry the Android suffix. Consider renaming:
- setActivity → setActivityAndroid
- checkAlternativeBillingAvailability → checkAlternativeBillingAvailabilityAndroid
- showAlternativeBillingInformationDialog → showAlternativeBillingInformationDialogAndroid
- createAlternativeBillingReportingToken → createAlternativeBillingReportingTokenAndroid
- set/add/removeUserChoiceBillingListener → set/add/removeUserChoiceBillingListenerAndroid
As per coding guidelines
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (2)
282-304: Also guard reflection for dialog/token APIs.Mirror the above runCatching pattern for showAlternativeBillingOnlyInformationDialog and createAlternativeBillingOnlyReportingDetailsAsync to avoid runtime NoSuchMethodException.
Also applies to: 330-363
458-466: Remove commented-out code per repo guidelines.Keep guidance in docs; drop commented callback block.
As per coding guidelines
- // TODO: In production, emit this token via callback for payment processing - // alternativeBillingCallback?.onTokenCreated( - // token = tokenResult, - // productId = props.skus.first(), - // onPaymentComplete = { transactionId -> - // // App reports to backend after payment success - // } - // )packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (1)
325-326: Consider signaling unsupported deepLinkToSubscriptions.No-op hides behavior; either log a notice or throw OpenIapError.NotSupported for consistency.
- override val deepLinkToSubscriptions: MutationDeepLinkToSubscriptionsHandler = { _ -> } + override val deepLinkToSubscriptions: MutationDeepLinkToSubscriptionsHandler = { + OpenIapLog.i("Horizon does not support subscription deep links", TAG) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/google/.github/workflows/ci-horizon.yml(1 hunks)packages/google/Example/build.gradle.kts(2 hunks)packages/google/Example/src/main/java/dev/hyo/martie/screens/PurchaseFlowScreen.kt(1 hunks)packages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt(1 hunks)packages/google/gradle.properties(1 hunks)packages/google/openiap/build.gradle.kts(4 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/HorizonBillingConverters.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt(19 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapProtocol.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(8 hunks)packages/google/openiap/src/play/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/Example/src/main/java/dev/hyo/martie/screens/PurchaseFlowScreen.ktpackages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapProtocol.ktpackages/google/openiap/src/play/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/HorizonBillingConverters.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
packages/google/openiap/src/main/java/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Android-specific functions must end with Android; cross-platform functions have no suffix
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapProtocol.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
🧬 Code graph analysis (2)
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (2)
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
getActiveSubscriptions(325-326)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (1)
getStorefront(369-388)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (2)
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (2)
buildBillingClient(895-1014)getStorefront(809-827)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
getActiveSubscriptions(325-326)
⏰ 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 (8)
packages/google/gradle.properties (1)
2-2: Resource tuning appropriate for expanded scope.Doubling heap memory (2GB→4GB) and explicitly setting metaspace (1GB) are reasonable given the addition of Horizon billing support, build flavors, and generated types. This prevents out-of-memory errors during multi-variant compilations and is aligned with build best practices.
packages/google/openiap/build.gradle.kts (1)
100-106: Publishing only playRelease is OK; confirm if horizon artifacts are intentionally not published.If horizon should remain internal, this is fine. Otherwise, add a second AndroidSingleVariantLibrary for horizonRelease.
packages/google/Example/src/main/java/dev/hyo/martie/screens/PurchaseFlowScreen.kt (1)
57-59: LGTM: remember OpenIapStore with appContext avoids leaking Activity.packages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt (1)
87-89: LGTM: remember OpenIapStore with appContext avoids leaking Activity.packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (2)
52-60: Constructors look good; verify auto/store selection across flavors.Ensure BuildConfig.OPENIAP_STORE exists in both play and horizon variants and that appId is wired from Example’s HORIZON_APP_ID when needed. Add an instrumented init smoke test?
298-300: Good: explicit NotSupported when requestPurchase handler is absent.This surfaces capability mismatches cleanly.
packages/google/openiap/src/play/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (1)
31-38: Delegation looks good; stub is minimal and consistent.The Play-flavor stub cleanly delegates to OpenIapModule and suppresses the unused appId as intended. No blockers.
Also applies to: 91-131
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (1)
780-786: Naming mismatch: getStorefrontIOS set from Android code.QueryHandlers.getStorefrontIOS is wired to Android getStorefront(); verify this is intentional, or rename to getStorefrontAndroid for clarity.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
36-36: Remove unused import.
BillingClientis not used in this file.-import com.android.billingclient.api.BillingClientAs per coding guidelines (remove unused/commented code).
🧹 Nitpick comments (2)
packages/google/Example/build.gradle.kts (1)
30-35: Asymmetric app ID resolution between defaultConfig and horizon flavor.The app ID fallback chain differs between defaultConfig and the horizon flavor:
- defaultConfig (lines 30-34): tries
EXAMPLE_HORIZON_APP_ID→EXAMPLE_OPENIAP_APP_ID→ project props →""- horizon flavor (lines 54-56): tries
EXAMPLE_HORIZON_APP_ID→ project props →""(skipsEXAMPLE_OPENIAP_APP_ID)This asymmetry means that if a user sets
EXAMPLE_OPENIAP_APP_IDin local.properties, it will be used as a fallback in defaultConfig but not in the horizon flavor. Clarify the intent: should the horizon flavor also checkEXAMPLE_OPENIAP_APP_ID, or is excluding it deliberate?Additionally, consider extracting this resolution logic into a helper function to reduce duplication and make the precedence explicit:
fun resolveAppId(horizonKey: String, fallbackKey: String?): String { return localProperties.getProperty(horizonKey) ?: (fallbackKey?.let { localProperties.getProperty(it) } ?: "") ?: (project.findProperty(horizonKey) as String?) ?: (fallbackKey?.let { project.findProperty(it) as String? } ?: "") ?: "" }Then use it consistently:
// In defaultConfig buildConfigField("String", "HORIZON_APP_ID", "\"${resolveAppId("EXAMPLE_HORIZON_APP_ID", "EXAMPLE_OPENIAP_APP_ID")}\"") // In horizon flavor manifestPlaceholders["OCULUS_APP_ID"] = resolveAppId("EXAMPLE_HORIZON_APP_ID", null)Also applies to: 54-56
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
503-551: Improve exception logging for easier debugging.The reflection-based module loading has appropriate fallback behavior, but exception details are not always logged, making troubleshooting difficult.
Apply this diff to improve logging consistency:
val defaultStore = try { val buildConfig = Class.forName("io.github.hyochan.openiap.BuildConfig") buildConfig.getField("OPENIAP_STORE").get(null) as? String ?: "play" } catch (e: Throwable) { + OpenIapLog.d("Failed to load BuildConfig, using default 'play': ${e.message}", "OpenIapStore") "play" }} catch (e: Throwable) { - OpenIapModule(context) as OpenIapProtocol + OpenIapLog.e("Failed to load OpenIapHorizonModule in auto mode, falling back to Play", e, "OpenIapStore") + OpenIapModule(context) as OpenIapProtocol }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/google/CONTRIBUTING.md(1 hunks)packages/google/Example/build.gradle.kts(3 hunks)packages/google/Example/src/horizon/AndroidManifest.xml(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(8 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/google/CONTRIBUTING.md
- packages/google/Example/src/horizon/AndroidManifest.xml
🧰 Additional context used
📓 Path-based instructions (2)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
packages/google/openiap/src/main/java/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Android-specific functions must end with Android; cross-platform functions have no suffix
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
🪛 detekt (1.23.8)
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
[warning] 508-508: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 539-539: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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 (7)
packages/google/Example/build.gradle.kts (3)
41-51: Verify storeOverride pattern doesn't allow both flavors to be set to the same store.The
storeOverridepattern (line 41) allowsEXAMPLE_OPENIAP_STOREto override both flavors. If set to"play", both theplayandhorizonflavors would haveOPENIAP_STORE="play", defeating the flavor distinction.Is this intentional? If
storeOverrideis meant to force a specific store globally (for testing), document this clearly. Otherwise, consider flavored overrides likeEXAMPLE_OPENIAP_STORE_PLAYandEXAMPLE_OPENIAP_STORE_HORIZONto preserve flavor semantics.
86-86: Verify necessity of buildConfig feature flag.Line 86 enables
buildConfig = true. Verify whether this is required given that the file already declaresbuildConfigFieldcalls (lines 35, 46, 51). Modern Android Gradle plugin versions may enable this by default; confirm it's necessary or remove it if redundant.
48-57: Manifest verification confirms proper OCULUS_APP_ID placeholder usage.The horizon-specific manifest exists and correctly references the
${OCULUS_APP_ID}placeholder on line 9, which is properly injected by the build.gradle.kts flavor configuration. No issues detected.packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (4)
57-85: Well-designed protocol abstraction.The refactoring to use
OpenIapProtocolprovides good flexibility for supporting multiple billing implementations (Play Store and Horizon) while maintaining backward compatibility through multiple constructor overloads.
326-332: Appropriate fallback behavior for optional handlers.The nullable handler invocations with safe default values (empty list, false, Unit) are appropriate for optional subscription features.
553-562: Horizon environment detection logic is sound.The implementation correctly detects Meta Horizon environments by checking both the device manufacturer and the presence of Oculus system packages.
308-320: Architectural inconsistency in null-handler pattern: finishTransaction silently fails while requestPurchase throws.All current
OpenIapProtocolimplementations (Play Store and Horizon) do provide afinishTransactionhandler. However, the design creates an inconsistency: at line 313,finishTransactionuses safe invoke with silent failure, whilerequestPurchase(line 299-300) throwsNotSupportedif null. Additionally, theMutationHandlersdata class definesfinishTransactionas nullable with default null, which could allow silent failures in future implementations.Consider either making
finishTransactionnon-nullable inMutationHandlers(matching the protocol interface requirement), or throwingOpenIapError.NotSupportedwhen null for consistency withrequestPurchase.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
530-542: Add logging for auto-detection fallback.The auto-detection mode (lines 537-538) falls back silently to Play Store when Horizon module loading fails, unlike the explicit "horizon" mode which logs the fallback (line 526). This makes debugging harder when auto-detection doesn't work as expected.
Apply this diff:
} else { try { val clazz = Class.forName("dev.hyo.openiap.horizon.OpenIapHorizonModule") val constructor = clazz.getConstructor(Context::class.java, String::class.java) constructor.newInstance(context, resolvedAppId) as OpenIapProtocol } catch (e: Throwable) { + OpenIapLog.e("Auto-detect: Failed to load Horizon module, falling back to Play", e, "OpenIapStore") OpenIapModule(context) as OpenIapProtocol }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/google/.github/workflows/ci-horizon.yml(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/HorizonBillingConverters.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
🧰 Additional context used
📓 Path-based instructions (2)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/HorizonBillingConverters.kt
packages/google/openiap/src/main/java/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Android-specific functions must end with Android; cross-platform functions have no suffix
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.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 (9)
packages/google/.github/workflows/ci-horizon.yml (1)
44-54: ✅ Past issues successfully addressed.The developer has resolved both previous review concerns:
- Line 46: License acceptance (
yes | sdkmanager --licenses) now executes before the SDK install step, preventing potential hangs on non-interactive runners.- Line 53:
working-directory: packages/googleis properly specified, ensuring Gradle tasks find the wrapper and build configuration.The workflow is well-structured and should execute correctly.
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (3)
37-54: LGTM! Protocol abstraction documented clearly.The imports are appropriate and the updated class documentation accurately reflects the refactored design that wraps an OpenIapProtocol implementation.
55-83: LGTM! Constructor refactoring supports protocol abstraction well.The new constructor overloads provide flexible initialization paths while maintaining backward compatibility through proper delegation to the buildModule factory.
284-330: Document or standardize null-handler behavior in purchase flow operations.The inconsistent null-handling across handlers is real and warrants verification:
requestPurchase(line 297-298): throwsOpenIapError.NotSupportedwhen handler is nullfinishTransaction(line 311): silently succeeds when handler is null- Query methods (lines 325, 328, 330): return safe defaults when handlers are null
This pattern correlates with return types—data-returning operations throw, side-effect operations degrade gracefully—but is undocumented. The risk: silent failure in
finishTransactioncould mask missing implementations and cause purchase processing issues (token leaks, duplication).Consider either:
- Adding KDoc explaining why mutation vs. query handlers have different null-handling strategies, or
- Making
finishTransactionthrow or log a warning when its handler is null, aligning it withrequestPurchasebehavior since both are critical transaction operationspackages/google/openiap/src/horizon/java/dev/hyo/openiap/utils/HorizonBillingConverters.kt (5)
1-16: LGTM!Clean imports with helpful type aliases to distinguish Horizon SDK types from domain models.
19-46: LGTM!The mapping logic correctly converts Horizon in-app product details with appropriate null-safe defaults.
83-89: Verify inclusion of oneTimePurchaseOfferDetails for subscription products.The mapping includes
oneTimePurchaseOfferDetailsfor subscription products, which is unusual. Please verify whether the Horizon SDK supports one-time purchase details for subscriptions or if this should be set tonullfor subscription-only products.
125-132: Verify hardcoded isActive flag.Line 127 hardcodes
isActive = true. Verify that this function is only called for active subscriptions, or if the HorizonPurchaseobject has an active/expired state that should be checked instead.
134-141: LGTM!The mapping correctly converts
PurchaseAndroidtoActiveSubscription. The hardcodedisActive = truealigns with the function name, suggesting it's only called for active subscriptions.
...es/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt
Outdated
Show resolved
Hide resolved
...es/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
Show resolved
Hide resolved
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (1)
7-9: Remove unused icon importsIcons and ArrowBack aren’t used here.
As per coding guidelines
-import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ArrowBackpackages/google/Example/src/main/java/dev/hyo/martie/screens/AlternativeBillingScreen.kt (2)
67-82: Do not log full externalTransactionToken/products.Tokens/ids are sensitive. Mask tokens and avoid logging full product lists in production logs.
- android.util.Log.d("UserChoiceEvent", "External Token: ${details.externalTransactionToken}") - android.util.Log.d("UserChoiceEvent", "Products: ${details.products}") + android.util.Log.d("UserChoiceEvent", "External Token: ${details.externalTransactionToken.takeLast(6).padStart(12, '*')}") + android.util.Log.d("UserChoiceEvent", "Products count: ${details.products.size}") @@ - message = "User selected alternative billing\nToken: ${details.externalTransactionToken.take(20)}...\nProducts: ${details.products.joinToString()}", + message = "User selected alternative billing\nToken: ****${details.externalTransactionToken.takeLast(6)}\nProducts: ${details.products.size}",Also applies to: 74-79
537-545: Potential NPE: activity!!findActivity() may return null in some contexts. Avoid non-null assertion.
- val dialogAccepted = iapStore.showAlternativeBillingInformationDialog(activity!!) + val act = activity ?: run { + iapStore.postStatusMessage("No Activity available", PurchaseResultStatus.Error) + return@launch + } + val dialogAccepted = iapStore.showAlternativeBillingInformationDialog(act)packages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt (2)
682-683: Token logging: keep masked.Even truncated tokens are sensitive. Prefer last 4–6 chars and avoid printing full state dumps in release.
- println("SubscriptionFlow: Current purchase details - productId: ${subscription.productId}, token: ${subscription.purchaseToken?.take(10)}") + println("SubscriptionFlow: Current purchase details - productId: ${subscription.productId}, token: ****${subscription.purchaseToken?.takeLast(6)}")Also applies to: 662-674
927-1059: UseandroidSubscriptionsinstead ofandroidProductsin "Available Subscriptions" section (lines 927–932).
androidProductsincludes bothProductType.SubsandProductType.InApp, but this section is labeled "Available Subscriptions" and should display subscriptions only. Use the dedicatedandroidSubscriptionsstream instead.Update required:
- Line 932: Change
items(androidProducts)toitems(androidSubscriptions)- Update
ProductCardsignature inpackages/google/Example/src/main/java/dev/hyo/martie/screens/uis/ProductCard.ktto acceptProductSubscriptionAndroidinstead ofProductAndroid(both types implementProductCommonand have identical properties)
♻️ Duplicate comments (4)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt (1)
79-86: Remove unused parameter from toPurchase call.HorizonBillingConverters.toPurchase(productType) doesn’t use productType. Drop the argument and update the converter signature. (Duplicate of prior review.)
- it.toPurchase(productType) + it.toPurchase()packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt (1)
98-123: Map real purchase state; don’t hardcode Purchased.Hardcoding PurchaseState.Purchased can misreport pending/failed states. Read HorizonPurchase state and map to internal enum. Also remove the unused productType parameter.
- fun HorizonPurchase.toPurchase(productType: String): PurchaseAndroid { + fun HorizonPurchase.toPurchase(): PurchaseAndroid { - val token = purchaseToken - val productsList = products ?: emptyList() - val purchaseState = PurchaseState.Purchased + val token = purchaseToken + val productsList = products ?: emptyList() + val purchaseState = when (this.purchaseState) { // Kotlin maps getPurchaseState() + 1 -> PurchaseState.Purchased + 2 -> PurchaseState.Pending + else -> PurchaseState.Unspecified + } @@ return PurchaseAndroid( autoRenewingAndroid = isAutoRenewing(), dataAndroid = originalJson, developerPayloadAndroid = developerPayload, id = orderId ?: token, ids = productsList, isAcknowledgedAndroid = isAcknowledged(), isAutoRenewing = isAutoRenewing(), obfuscatedAccountIdAndroid = null, obfuscatedProfileIdAndroid = null, packageNameAndroid = packageName, platform = IapPlatform.Android, productId = productsList.firstOrNull().orEmpty(), purchaseState = purchaseState, purchaseToken = token, quantity = quantity ?: 1, signatureAndroid = signature, transactionDate = (purchaseTime ?: 0L).toDouble(), transactionId = orderId ?: token ) }And update call sites (HorizonHelpers.queryPurchasesHorizon):
- it.toPurchase(productType) + it.toPurchase()packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (2)
60-63: Rename Android-specific helper per guidelines.buildModule uses Android APIs; suffix with Android and update constructor call sites.
Apply:
- constructor(context: Context) : this(buildModule(context, null, null)) - constructor(context: Context, store: String?) : this(buildModule(context, store, null)) - constructor(context: Context, store: String?, appId: String?) : this(buildModule(context, store, appId)) + constructor(context: Context) : this(buildModuleAndroid(context, null, null)) + constructor(context: Context, store: String?) : this(buildModuleAndroid(context, store, null)) + constructor(context: Context, store: String?, appId: String?) : this(buildModuleAndroid(context, store, appId))-private fun buildModule(context: Context, store: String?, appId: String?): OpenIapProtocol { +private fun buildModuleAndroid(context: Context, store: String?, appId: String?): OpenIapProtocol {As per coding guidelines.
Also applies to: 549-549
600-606: Suffix environment detector with Android and modernize PackageManager use.Function and call site should be Android-suffixed; also use API 33+ flags.
Apply:
- if (isHorizonEnvironment(context)) { + if (isHorizonEnvironmentAndroid(context)) {-private fun isHorizonEnvironment(context: Context): Boolean { +private fun isHorizonEnvironmentAndroid(context: Context): Boolean { val manufacturer = android.os.Build.MANUFACTURER.lowercase() if (manufacturer.contains("meta") || manufacturer.contains("oculus")) return true return try { - context.packageManager.getPackageInfo("com.oculus.vrshell", 0) + val pm = context.packageManager + if (android.os.Build.VERSION.SDK_INT >= 33) { + pm.getPackageInfo("com.oculus.vrshell", android.content.pm.PackageManager.PackageInfoFlags.of(0)) + } else { + @Suppress("DEPRECATION") + pm.getPackageInfo("com.oculus.vrshell", 0) + } true } catch (_: Throwable) { false } }As per coding guidelines.
Also applies to: 620-629
🧹 Nitpick comments (19)
packages/google/openiap/build.gradle.kts (1)
71-77: Clarify the source set configuration comment.The comment at line 72 states "Auto flavor includes horizon implementation only," which is potentially misleading. If the auto flavor is designed to support both Play and Horizon platforms at runtime, the comment should clarify that:
src/main/javacontains shared code (likely Play-centric baseline)src/horizon/javacontains Horizon-specific implementations that are added to auto- Why play flavor doesn't require a separate
src/play/javaconfigurationVerify this architecture and update the comment for clarity.
packages/google/Example/src/main/java/dev/hyo/martie/util/SubscriptionOfferUtils.kt (1)
79-96: Harden period extraction and add tests
- Consider normalizing (uppercase) and validating the period string to avoid downstream surprises.
- Add unit tests with sample Horizon receipts covering:
- { billingPeriod }, { subscriptionPeriod }, and lineItems[0].billingPeriod.
- Missing/empty fields and malformed JSON.
I can add a small test matrix with canned receipts if helpful.
packages/google/Example/build.gradle.kts (2)
60-70: DRY: reuse the same horizonAppId computed in defaultConfigAvoid recomputing; reuse the already computed value or a shared helper.
Apply this diff once you move placeholder to defaultConfig:
- val appId = localProperties.getProperty("EXAMPLE_HORIZON_APP_ID") - ?: (project.findProperty("EXAMPLE_HORIZON_APP_ID") as String?) - ?: "" - manifestPlaceholders["OCULUS_APP_ID"] = appId + // Placeholder already set in defaultConfig; keep if you need to override per-flavor. + // manifestPlaceholders["OCULUS_APP_ID"] = android.defaultConfig.manifestPlaceholders["OCULUS_APP_ID"]!!
41-53: Extract duplicate appId resolution logic into a single variableThe OCULUS_APP_ID appId resolution is duplicated across flavors at lines 48-50 and lines 67-69 in
packages/google/Example/build.gradle.kts. Extract this into a singlevalvariable outside the flavors block (e.g., in defaultConfig or at module scope) and reuse it.packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt (1)
33-52: Update getOrQuery to use typed cache keys and preserve orderMinimal changes to align with the new key.
Apply this diff:
- val missing = productIds.filter { cache[it] == null }.distinct() + val missing = productIds.filter { cache[CacheKey(it, productType)] == null }.distinct() ... - if (missing.isEmpty()) { - val cached = productIds.mapNotNull { cache[it] } + if (missing.isEmpty()) { + val cached = productIds.mapNotNull { cache[CacheKey(it, productType)] } OpenIapLog.d("getOrQuery: Returning ${cached.size} cached products", TAG) return cached }packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (1)
55-58: Nit: avoid ‘!!’ by using takeIfSmall readability tweak.
Apply this diff:
- val startRoute = remember { - val route = (context as? android.app.Activity)?.intent?.getStringExtra("openiap_route") - if (route in setOf("home", "purchase_flow", "subscription_flow", "available_purchases", "offer_code", "alternative_billing")) route!! else "home" - } + val startRoute = remember { + val allowed = setOf("home","purchase_flow","subscription_flow","available_purchases","offer_code","alternative_billing") + (context as? android.app.Activity) + ?.intent?.getStringExtra("openiap_route") + ?.takeIf { it in allowed } ?: "home" + }packages/google/Example/src/main/java/dev/hyo/martie/Constants.kt (2)
36-42: Avoid println in Android code; use Log with a TAG.Swap println for Log.d and add a TAG to keep logs consistent and filterable.
+ private const val TAG = "IapConstants" @@ - println("IapConstants: getSubscriptionSkus() - isHorizon=$isHorizon, skus=$skus") + android.util.Log.d(TAG, "getSubscriptionSkus() - isHorizon=$isHorizon, skus=$skus")
48-50: Constant name/comment mismatch across platforms.PREMIUM_YEARLY_PRODUCT_ID_PLAY is also used in Horizon flows; name “_PLAY” can mislead.
Option A (preferred): rename to PREMIUM_YEARLY_PRODUCT_ID and update usages.
Option B: add alias and deprecate the old constant.+ @Deprecated("Use PREMIUM_YEARLY_PRODUCT_ID") const val PREMIUM_YEARLY_PRODUCT_ID_PLAY = "dev.hyo.martie.premium_year" // Play only + const val PREMIUM_YEARLY_PRODUCT_ID = PREMIUM_YEARLY_PRODUCT_ID_PLAYpackages/google/Example/src/main/java/dev/hyo/martie/screens/AlternativeBillingScreen.kt (1)
119-123: Fixed delay after endConnection risks flakiness.Waiting a hardcoded 500 ms may race on slow devices. Poll for disconnect or await store state instead.
Example:
iapStore.endConnection() withTimeoutOrNull(2000) { while (iapStore.isConnected.value) delay(100) }packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt (1)
28-38: Optional: parallelize INAPP and SUBS queries.Restore can be faster by querying both types concurrently.
val (inapp, subs) = coroutineScope { val a = async { queryPurchasesHorizon(client, BillingClient.ProductType.INAPP) } val b = async { queryPurchasesHorizon(client, BillingClient.ProductType.SUBS) } a.await() to b.await() }packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt (1)
125-142: Active subscription should reflect real state.Currently always sets isActive = true. Consider deriving from mapped purchaseState to avoid false positives.
- fun HorizonPurchase.toActiveSubscription(): ActiveSubscription = ActiveSubscription( - autoRenewingAndroid = isAutoRenewing(), - isActive = true, + fun HorizonPurchase.toActiveSubscription(): ActiveSubscription = ActiveSubscription( + autoRenewingAndroid = isAutoRenewing(), + isActive = (this.purchaseState == 1), // PURCHASEDpackages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt (2)
25-25: Remove unused import or use it to gate logs.BuildConfig is imported but unused. Either remove it or wrap debug prints with BuildConfig.DEBUG.
As per coding guidelines
- import dev.hyo.martie.BuildConfig + // import dev.hyo.martie.BuildConfig // Uncomment if used to gate debug logs
389-406: Naming mismatch can confuse: YEARLY_PRODUCT_ID_PLAY used for Horizon.You treat PREMIUM_YEARLY_PRODUCT_ID_PLAY as premium on Horizon too. Align constant naming (see Constants.kt) to reduce confusion.
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
127-156: Avoid GlobalScope; use a class scope and stop printing stack traces.GlobalScope leaks work across lifecycles; prefer a structured scope tied to this store and cancel it in clear(). Also remove printStackTrace.
Apply:
- kotlinx.coroutines.GlobalScope.launch { + scope.launch { @@ - } catch (e: Exception) { - android.util.Log.e("OpenIapStore", "Failed to refresh purchases after update", e) - e.printStackTrace() - } + } catch (e: Exception) { + android.util.Log.e("OpenIapStore", "Failed to refresh purchases after update", e) + } }fun clear() { module.removePurchaseUpdateListener(purchaseUpdateListener) module.removePurchaseErrorListener(purchaseErrorListener) processedPurchaseTokens.clear() pendingRequestProductId = null + // cancel in-flight work started by this store + scope.coroutineContext.cancelChildren() }Add near the top of the class:
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main)Imports:
import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.cancelChildrenAlso applies to: 213-217
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (5)
217-256: Redact transactionId in logs.Avoid logging full purchase identifiers.
Apply:
- allPurchases.forEachIndexed { index, purchase -> + allPurchases.forEachIndexed { index, purchase -> val txnId = when (purchase) { is dev.hyo.openiap.PurchaseAndroid -> purchase.transactionId else -> "N/A" } - android.util.Log.i("HORIZON_QUERY", "Purchase[$index] productId=${purchase.productId} txnId=$txnId") + val redactedTxn = txnId.take(8).plus("…") + android.util.Log.i("HORIZON_QUERY", "Purchase[$index] productId=${purchase.productId} txnId=$redactedTxn") OpenIapLog.i( " [$index] productId=${purchase.productId} " + - "transactionId=$txnId " + + "transactionId=$redactedTxn " + "platform=${purchase.platform}", TAG ) }
88-92: Make shared cache concurrent (and consider bounding).mutableMapOf isn’t thread-safe; multiple coroutines access it. Use ConcurrentHashMap and optionally cap size/TTL.
Apply:
- private val sharedPurchaseCache = mutableMapOf<String, Purchase>() // productId -> Purchase + private val sharedPurchaseCache: MutableMap<String, Purchase> = + java.util.concurrent.ConcurrentHashMap() // productId -> PurchaseOptionally add simple eviction if size grows unexpectedly.
162-168: Guard queries with isReady().If the BillingClient isn’t ready, fail fast to avoid inconsistent results.
Apply:
- val client = billingClient ?: throw OpenIapError.NotPrepared + val client = billingClient ?: throw OpenIapError.NotPrepared + if (!client.isReady()) throw OpenIapError.NotPrepared
679-683: Avoid Log.wtf for normal callbacks.wtf is too severe and can crash under strict handlers. Use i/w.
Apply:
- android.util.Log.wtf("HORIZON_CALLBACK", "onPurchasesUpdated START - responseCode=${result.responseCode}, count=${purchases?.size ?: 0}") + android.util.Log.i("HORIZON_CALLBACK", "onPurchasesUpdated START - responseCode=${result.responseCode}, count=${purchases?.size ?: 0}")
803-831: Handle exceptions explicitly (detekt).Catches currently swallow exceptions. Ensure they’re logged and mapped (e.g., to NotSupported) consistently so callers can branch reliably.
Minimal approach:
- Log with OpenIapLog.e including message and exception.
- For unsupported features (NoSuchMethodError), return false/null and/or throw OpenIapError.NotSupported to make semantics explicit.
Also applies to: 833-866, 868-898
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/google/Example/build.gradle.kts(3 hunks)packages/google/Example/src/main/AndroidManifest.xml(1 hunks)packages/google/Example/src/main/java/dev/hyo/martie/Constants.kt(2 hunks)packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt(2 hunks)packages/google/Example/src/main/java/dev/hyo/martie/screens/AlternativeBillingScreen.kt(7 hunks)packages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt(13 hunks)packages/google/Example/src/main/java/dev/hyo/martie/util/SubscriptionOfferUtils.kt(3 hunks)packages/google/openiap/build.gradle.kts(3 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt(1 hunks)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt(13 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/Example/src/main/java/dev/hyo/martie/util/SubscriptionOfferUtils.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.ktpackages/google/Example/src/main/java/dev/hyo/martie/screens/AlternativeBillingScreen.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.ktpackages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.ktpackages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.ktpackages/google/Example/src/main/java/dev/hyo/martie/Constants.ktpackages/google/Example/src/main/java/dev/hyo/martie/MainActivity.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt
packages/google/openiap/src/main/java/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Android-specific functions must end with Android; cross-platform functions have no suffix
Files:
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt
🧬 Code graph analysis (3)
packages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt (1)
packages/google/Example/src/main/java/dev/hyo/martie/util/SubscriptionOfferUtils.kt (1)
resolvePremiumOfferInfo(27-57)
packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapProtocol.kt (1)
initConnection(12-47)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (2)
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (2)
buildBillingClient(895-1014)getStorefront(809-827)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt (3)
queryProductDetailsHorizon(98-106)restorePurchasesHorizon(19-42)queryPurchasesHorizon(47-93)
🪛 detekt (1.23.8)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
[warning] 813-813: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 849-849: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 877-877: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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 (12)
packages/google/openiap/build.gradle.kts (2)
131-136: Publishing strategy is sound.Publishing the
autoReleasevariant as the default provides consumers with cross-platform support out of the box. This is a good choice for a library that aims to support both Google Play and Horizon OS.
96-100: Remove this review comment — it mischaracterizes the dependency architecture.The Meta Horizon Billing Compatibility SDK is designed to facilitate the porting of Android applications that utilize Google's Play Service Billing Library for In-App Purchasing and Subscriptions to the Quest ecosystem. The Horizon library is a compatibility wrapper with SDK compatible with Google Play Billing Library version 7.0, not a duplicate. Gradle automatically creates build variants based on your build types and product flavors, ensuring that Gradle will automatically pull the dependencies for the selected productFlavors only. This means the
autoApiconfiguration is flavor-scoped: the final APK includes only the runtime dependencies for its selected flavor (eitherplayApiorhorizonApi), never both. The libraries use different package namespaces and there is no ClassLoader conflict. The current comment is architecturally correct.packages/google/Example/src/main/java/dev/hyo/martie/util/SubscriptionOfferUtils.kt (3)
14-16: LGTM: adding billingPeriod to SubscriptionOfferInfoOptional field with sensible default; backward compatible.
43-45: Good: derive billingPeriod from receiptStraightforward extraction; resilient to missing data.
If Horizon expects ISO‑8601 durations (e.g., P1M/P1Y) strictly, consider normalizing to uppercase and validating with a light regex before storing.
52-56: LGTM: propagating billingPeriod in resolved infoKeeps monthly/yearly display while carrying Horizon period when present.
packages/google/Example/build.gradle.kts (2)
9-14: Nice: local.properties loading is safe and guardedNo issues.
98-98: LGTM: buildConfig enabledRequired for BuildConfig fields used by runtime selection.
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (2)
23-25: LGTM: hoisting OpenIapStore to Activity scopeEnsures a single shared connection across screens.
60-65: LGTM: providing store via IapContextCorrectly scopes the store to the composition subtree.
packages/google/Example/src/main/java/dev/hyo/martie/screens/AlternativeBillingScreen.kt (1)
54-63: Constructor usage looks good; minor suggestion.Init via default OpenIapStore(appContext) and passing mode via config is aligned. Consider disabling OpenIapLog in release builds.
If you want, I can gate logs by BuildConfig.DEBUG across the Example module.
packages/google/Example/src/main/java/dev/hyo/martie/screens/SubscriptionFlowScreen.kt (2)
88-90: OpenIapStore init looks correct.Constructor change is aligned with protocol-based init; no issues spotted.
120-125: Good: platform-aware SKU selection.Using IapConstants.getSubscriptionSkus() centralizes platform logic.
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt
Outdated
Show resolved
Hide resolved
...ges/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt
Show resolved
Hide resolved
...ges/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt
Outdated
Show resolved
Hide resolved
...ges/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (1)
435-461: Critical: launchBillingFlow must run on the main thread.The
launchBillingFlowcall at line 440 is a UI operation that must execute on the main thread, but it's currently invoked withinwithContext(Dispatchers.IO). This can cause crashes or undefined behavior.val billingFlowParams = flowBuilder.build() android.util.Log.i(TAG, "=== LAUNCHING BILLING FLOW ===") android.util.Log.i(TAG, " - Is subscription? ${androidArgs.type == ProductQueryType.Subs}") android.util.Log.i(TAG, " - Has purchaseToken? ${!androidArgs.purchaseTokenAndroid.isNullOrBlank()}") - val result = client.launchBillingFlow(activity, billingFlowParams) + val result = withContext(Dispatchers.Main) { + client.launchBillingFlow(activity, billingFlowParams) + } android.util.Log.i(TAG, "=== BILLING FLOW LAUNCHED ===")
🧹 Nitpick comments (2)
packages/google/CONVENTION.md (1)
5-26: Clarify convention change relative to prior learning and fix markdown formatting.The new Android-Specific Functions convention states "DO NOT add
Androidsuffix" for function names, which contradicts the retrieved learning indicating "Android-specific functions must end with Android". Confirm this is an intentional convention shift for the Horizon integration.Additionally, the static analysis tool flagged markdown formatting: code blocks at lines 10–16 and 19–23 should be surrounded by blank lines per MD031.
Apply this diff to fix the markdown formatting:
**✅ Correct**: + ```kotlin fun acknowledgePurchase() fun consumePurchase() fun getPackageName() fun buildModule(context: Context) fun isHorizonEnvironment(context: Context)
❌ Incorrect:
+fun acknowledgePurchaseAndroid() // Don't add Android suffix fun consumePurchaseAndroid() // Don't add Android suffix fun buildModuleAndroid() // Don't add Android suffix
</blockquote></details> <details> <summary>packages/google/Example/src/main/java/dev/hyo/martie/Constants.kt (1)</summary><blockquote> `29-34`: **Add safe-call operators for consistency and defensive programming.** While `Build.MANUFACTURER` and `Build.BRAND` are marked non-null in the SDK, line 33 already uses a safe-call for `Build.MODEL`. For consistency and to guard against rare device edge cases, apply safe-calls to all Build fields. ```diff fun isHorizonOS(): Boolean { - return Build.MANUFACTURER.equals("Meta", ignoreCase = true) || - Build.BRAND.equals("Meta", ignoreCase = true) || + return (Build.MANUFACTURER?.equals("Meta", ignoreCase = true) == true) || + (Build.BRAND?.equals("Meta", ignoreCase = true) == true) || (Build.MODEL?.contains("Quest", ignoreCase = true) == true) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CLAUDE.md(3 hunks)packages/google/CONVENTION.md(1 hunks)packages/google/Example/build.gradle.kts(3 hunks)packages/google/Example/src/main/java/dev/hyo/martie/Constants.kt(2 hunks)packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt(2 hunks)packages/google/openiap/build.gradle.kts(4 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt(1 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt
🧰 Additional context used
📓 Path-based instructions (1)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.ktpackages/google/Example/src/main/java/dev/hyo/martie/Constants.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt
🧠 Learnings (1)
📚 Learning: 2025-10-17T07:09:45.156Z
Learnt from: CR
PR: hyodotdev/openiap#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-17T07:09:45.156Z
Learning: Applies to packages/google/openiap/src/main/java/**/*.kt : Android-specific functions must end with Android; cross-platform functions have no suffix
Applied to files:
packages/google/CONVENTION.md
🧬 Code graph analysis (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (3)
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (1)
buildBillingClient(895-1014)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt (3)
queryProductDetailsHorizon(98-106)restorePurchasesHorizon(19-42)queryPurchasesHorizon(47-93)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
getActiveSubscriptions(372-373)
🪛 markdownlint-cli2 (0.18.1)
packages/google/CONVENTION.md
10-10: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
19-19: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
⏰ 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 (17)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (2)
22-23: LGTM: Activity-level store with lazy initialization.The lazy initialization at Activity level ensures the store persists across navigation and avoids memory leaks by using
applicationContext.
61-62: LGTM: Provider pattern for store sharing.The
IapContext.OpenIapProviderwrapper cleanly shares the store instance across all navigation destinations, following Compose best practices for dependency injection.packages/google/Example/build.gradle.kts (2)
30-38: LGTM! Past review concern addressed.The OCULUS_APP_ID placeholder is now set in defaultConfig, ensuring all flavors (including play) inherit it. This resolves the previous critical issue where play builds would fail due to an unresolved manifest placeholder.
42-73: LGTM! Flavor configuration is correct.The product flavors are properly configured:
- Auto flavor (default) includes both Play and Horizon support with appropriate OCULUS_APP_ID
- Play flavor correctly inherits OCULUS_APP_ID from defaultConfig
- Horizon flavor overrides with Horizon-specific app ID
packages/google/openiap/build.gradle.kts (2)
36-77: LGTM! Flavor and source set configuration is correct.The configuration properly sets up three platform flavors with appropriate build settings. The auto flavor sourcing from
src/horizon/javais intentional, enabling runtime platform detection while maintaining horizon implementation.
84-103: LGTM! Well-documented dependency strategy.The billing dependency configuration is correct and clearly documented. Using
compileOnlyfor main source and per-flavor runtime dependencies ensures proper compilation while minimizing APK size for single-platform builds.packages/google/Example/src/main/java/dev/hyo/martie/Constants.kt (1)
36-54: LGTM! Runtime SKU selection is well-implemented.The platform-aware SKU selection with clear constants and delegation pattern properly supports both Horizon and Play Store environments.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/utils/HorizonBillingConverters.kt (4)
19-46: LGTM! In-app product converter is correct.The converter properly handles Horizon product details with appropriate null-safety and maps to the internal ProductAndroid model.
48-96: LGTM! Subscription product converter handles complex nested structure correctly.The converter properly maps Horizon's subscription offer details including nested pricing phases to the internal ProductSubscriptionAndroid model.
98-123: LGTM! Purchase converter now correctly maps state.The past critical issue regarding hardcoded purchase state has been resolved. The converter now properly calls
PurchaseState.fromHorizonState()to map the actual purchase state.
149-154: LGTM! State mapping is comprehensive and correct.The companion function properly maps all Horizon purchase states to internal enum values with appropriate fallback to Unknown.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonProductManager.kt (2)
16-27: LGTM! Past thread-safety concerns fully addressed.The implementation now uses
ConcurrentHashMapwith a compositeCacheKey(productId, productType)to ensure thread-safe operations and prevent collisions between product types.
35-123: LGTM! Coroutine cancellation properly handled.The
getOrQueryimplementation correctly guards continuation resume withisActivechecks and includes cancellation callback. The logic preserves input order and provides comprehensive logging.packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (4)
83-110: LGTM! Module initialization is correct.The class properly implements
OpenIapProtocolandPurchasesUpdatedListener, initializes billing client, and sets up listener management.
582-605: LGTM! restorePurchases now properly emits updates.The past issue where
restorePurchaseswas a no-op has been resolved. The implementation now queries all purchases and notifies registered listeners correctly.
679-767: LGTM! Sensitive data properly redacted in logs.The past security concern has been addressed. Purchase tokens and order IDs are now redacted at lines 690-691, showing only the first 8 characters.
837-872: LGTM! Alternative billing dialog correctly shown on main thread.The past issue has been resolved. The dialog invocation at lines 849-854 is now properly wrapped in
runOnUiThreadto ensure UI operations occur on the main thread.
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
🧬 Code graph analysis (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (3)
packages/google/openiap/src/main/java/dev/hyo/openiap/OpenIapModule.kt (2)
buildBillingClient(895-1014)buildAndLaunch(518-632)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt (3)
queryProductDetailsHorizon(98-106)restorePurchasesHorizon(19-42)queryPurchasesHorizon(47-93)packages/google/openiap/src/main/java/dev/hyo/openiap/store/OpenIapStore.kt (1)
getActiveSubscriptions(372-373)
🪛 detekt (1.23.8)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
[warning] 849-849: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 887-887: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 915-915: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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 (3)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (3)
801-836: LGTM!The helper methods for querying product details and building the billing client are well-implemented with proper error handling and clean logic.
838-904: LGTM! Alternative billing implementation is correct.The alternative billing methods properly:
- Call UI operations on the main thread via
runOnUiThread- Log exceptions before rethrowing
- Handle unsupported methods gracefully
The static analysis warnings about swallowed exceptions (lines 849, 887, 915) are false positives—exceptions are logged via OpenIapLog.w before being wrapped and rethrown.
938-948: User choice billing no-ops are appropriate.User choice billing is correctly marked as unsupported for Horizon. The no-op implementations prevent errors when protocol methods are called.
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Outdated
Show resolved
Hide resolved
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (1)
7-8: Remove unused icon imports.The
IconsandIcons.filled.ArrowBackimports are not used in this file.As per coding guidelines
Apply this diff:
-import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.filled.ArrowBack
♻️ Duplicate comments (3)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (1)
39-47: Potential ANR risk withrunBlockingon main thread.This issue was already flagged in a previous review. Using
runBlockingon the main thread duringonDestroy()to callendConnection()(which performs I/O operations) can cause an Application Not Responding (ANR) error.As previously recommended, consider moving the cleanup to
onStop()wherelifecycleScopeis still active.packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (2)
740-762: Fragile product type detection fallback remains.The fallback heuristic that checks if product ID contains "subs" (lines 749-755) is unreliable and was previously flagged. Product IDs are arbitrary strings and may not follow naming conventions.
Consider implementing the previously suggested alternatives:
- Query both product types (SUBS and INAPP) when cache misses and use whichever returns a valid result
- Ensure
fetchProductsexplicitly stores and persistsproductTypefor all IDs- Replace the string-matching fallback with a deterministic lookup that queries
productManagerfor both types
113-113: Remove excessive android.util.Log statements per coding guidelines.The file contains extensive
android.util.Logcalls that duplicateOpenIapLogfunctionality throughout. These debug artifacts should be removed per the coding guidelines for Kotlin sources.Based on coding guidelines.
Remove the redundant
android.util.Logstatements and retain onlyOpenIapLogcalls. For example:- android.util.Log.i(TAG, "=== OpenIapHorizonModule INIT (Modified version with fix) ===")Apply similar removals for the
android.util.Logcalls at lines 124-127, 136-152, 227-229, 233-243, 265-271, 274-276, 279-280, 283-285, 288-290, 293-294, 297-299, 307-308, 387-388, 441-443, 448-450, 467, 479-480, 488-489, 717, 724, 728-729, 733-734, 737-738, 752-753, 756-757, 760-761, 764-765, 766-768, 770-772, 776-778, 781-782, 787-788, 791, 794-795, 798-799, and 801.Also applies to: 124-127, 136-152, 227-229, 233-243, 265-271, 274-276, 279-280, 283-285, 288-290, 293-294, 297-299, 307-308, 387-388, 441-443, 448-450, 467-467, 479-480, 488-489, 717-717, 724-724, 728-729, 733-734, 737-738, 752-753, 756-757, 760-761, 764-765, 766-768, 770-772, 776-778, 781-782, 787-788, 791-791, 794-795, 798-799, 801-801
🧹 Nitpick comments (1)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (1)
42-42: Consider importingrunBlockingfor consistency.Using the fully qualified name
kotlinx.coroutines.runBlockingis unusual. For consistency with typical Kotlin conventions, consider importing it at the top of the file.Apply this diff:
Add the import:
import dev.hyo.openiap.IapContext import dev.hyo.openiap.store.OpenIapStore +import kotlinx.coroutines.runBlockingThen update the usage:
runCatching { - kotlinx.coroutines.runBlocking { + runBlocking { iapStore.endConnection() } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt(2 hunks)packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
packages/google/**/*.kt
📄 CodeRabbit inference engine (CLAUDE.md)
Remove unused code and commented-out code in Kotlin sources
Files:
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.ktpackages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.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/Example/src/main/java/dev/hyo/martie/MainActivity.kt
🧬 Code graph analysis (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt (1)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/helpers/HorizonHelpers.kt (3)
queryProductDetailsHorizon(98-106)restorePurchasesHorizon(19-42)queryPurchasesHorizon(47-93)
🪛 detekt (1.23.8)
packages/google/openiap/src/horizon/java/dev/hyo/openiap/horizon/OpenIapHorizonModule.kt
[warning] 853-853: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 891-891: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 919-919: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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 (2)
packages/google/Example/src/main/java/dev/hyo/martie/MainActivity.kt (2)
21-22: LGTM!The lazy initialization of
OpenIapStoreat the Activity level withapplicationContextis correct and ensures the store persists across navigation while avoiding context leaks.
50-95: LGTM!The updated
AppNavigationsignature and theIapContext.OpenIapProviderwrapping correctly enable the sharedOpenIapStoreto be accessible across all navigation destinations. The implementation follows the provider pattern appropriately.
1 participant
Summary
Follow-up from hyodotdev/openiap-google#10
Summary by CodeRabbit
New Features
Chores
Documentation