Add freemium model with 30-day trial, Google Play IAP, and write-access gating#630
Open
tobihagemann wants to merge 46 commits intodevelopfrom
Open
Add freemium model with 30-day trial, Google Play IAP, and write-access gating#630tobihagemann wants to merge 46 commits intodevelopfrom
tobihagemann wants to merge 46 commits intodevelopfrom
Conversation
…ial info, fix subscription state
…itles, rename section to Premium
…eContentViewBinder
…ove double enforcement
…haredFiles write gate, add security tests
…b write-access checks, clean up flavor indirection
WalkthroughAdds a freemium IAP flavor and billing dependency plus two IAP service implementations (Google Play and a non-playstore no-op). Introduces purchase models, PurchaseManager, billing binder/clients, product querying, restore flows, and UI layouts/components for license/purchase and onboarding (WelcomeActivity + fragments). Implements license/trial/subscription persistence and listeners in SharedPreferencesHandler, a DB migration (Upgrade13To14) that removes licenseToken and marks welcome flow complete, FlavorConfig runtime flags, LicenseEnforcer and orchestrator, gating of write operations across presenters/activities/fragments, hub-vault license flags, many resource/string updates, build-flavor changes, and extensive unit/instrumented tests. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt (1)
540-559:⚠️ Potential issue | 🔴 CriticalRe-check write access before uploading edits from external apps.
Write-access enforcement happens only when opening the file (line 555), not when uploading modifications. In
openFileFinished(), when a file change is detected,uploadChangedFile()is called (line 621) without verifying write access again. If circumstances change between file open and close, a locked vault can still upload edits.One way to close the gap
+ private var openedFileIsWritable = false + private fun viewExternalFile(cloudFile: CloudFileModel) { val viewFileIntent = Intent(Intent.ACTION_VIEW) var openFileType = OpenFileType.DEFAULT + openedFileIsWritable = licenseEnforcer.hasWriteAccessForVault(view?.folder?.vault()) uriToOpenedFile = if (useMicrosoftWorkaround(cloudFile)) { openFileType = OpenFileType.MICROSOFT_WORKAROUND Uri.fromFile(fileUtil.getLegacyFileForMicrosoftWorkaround(cloudFile)) @@ - if (licenseEnforcer.hasWriteAccessForVault(view?.folder?.vault())) { + if (openedFileIsWritable) { permissionFlags = permissionFlags or Intent.FLAG_GRANT_WRITE_URI_PERMISSION } @@ override fun onSuccess(hashAfterEdit: ByteArray) { + if (!openedFileIsWritable) { + deleteFileIfMicrosoftWorkaround(openFileType, uriToOpenedFile) + return + } openedCloudFileMd5?.let { hashBeforeEdit ->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt` around lines 540 - 559, The presenter currently checks write access only when setting up the external edit (using licenseEnforcer.hasWriteAccessForVault(...) in the open flow) but does not re-verify before uploading edits; update openFileFinished() (or inside uploadChangedFile()) to call licenseEnforcer.hasWriteAccessForVault(view?.folder?.vault()) again and abort (log/show error and skip calling uploadChangedFile(uploadedUri, openedCloudFile, openedCloudFileMd5) or return early from uploadChangedFile) if the check fails, ensuring you reference openedCloudFile/openedCloudFileMd5 and view?.folder?.vault() so the upload is gated by the fresh permission check.presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt (1)
84-84:⚠️ Potential issue | 🟡 MinorRemove the unused
licenseCheckUseCasedependency.
licenseCheckUseCaseis injected but never used in this presenter. It's not invoked inprepareView()or any other method, and is only registered in theunsubscribeOnDestroycleanup list. Remove the parameter from the constructor (line 84) and from the unsubscribeOnDestroy call (line 636-637).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt` at line 84, The injected dependency licenseCheckUseCase is unused in VaultListPresenter; remove it from the class constructor parameter list and any corresponding property/field, and delete its entry from the unsubscribeOnDestroy(...) call so it is no longer registered for cleanup; update the VaultListPresenter constructor signature and remove any import or field reference to DoLicenseCheckUseCase to keep the class consistent.
🧹 Nitpick comments (16)
presentation/src/main/res/drawable/ic_checkmark.xml (1)
7-9: Consider using a theme attribute or app color resource for consistency.The day-mode drawable uses
@android:color/holo_green_darkwhile the night-mode variant uses a hardcoded#ffffff. For better maintainability and theme consistency, consider defining an app color resource (e.g.,@color/checkmark_color) with day/night variants, or use a theme attribute.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/res/drawable/ic_checkmark.xml` around lines 7 - 9, The drawable ic_checkmark.xml currently hardcodes android:fillColor="@android:color/holo_green_dark" (day) and "#ffffff" (night variant); replace these with a reusable color resource or theme attribute by creating a color resource like `@color/checkmark_color` with day/night values or referencing a theme attribute (e.g., ?attr/checkmarkColor) and update the android:fillColor on the <path> element to use that resource/attribute so both day and night drawables use the same named color.buildsystem/dependencies.gradle (1)
108-109: Consider upgrading to version 8.3.0 of Google Play Billing Library.Version 8.2.1 is a stable bug-fix release (released 2025-12-15) that addresses issues in the billing program APIs introduced in 8.2.0. However, the latest version is 8.3.0 (released 2025-12-23). Evaluate whether upgrading to the latest version is feasible for your use case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@buildsystem/dependencies.gradle` around lines 108 - 109, Update the billingclientVersion constant to use the newer Google Play Billing Library version by changing the value of billingclientVersion from '8.2.1' to '8.3.0' in buildsystem/dependencies.gradle; verify compatibility by running the build and any billing-related tests after updating and, if needed, adjust any references to APIs introduced or changed between versions in modules that depend on billingclientVersion.util/src/main/java/org/cryptomator/util/FlavorConfig.kt (1)
21-23:excludesGoogleDriveis narrower than its name.
litealso excludes Google Drive, so callers still need extra special cases for that flavor. Either foldliteinto this flag or rename the property to the underlying reason; otherwise this abstraction is easy to misuse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/FlavorConfig.kt` around lines 21 - 23, The excludesGoogleDrive property currently checks BuildConfig.FLAVOR == "fdroid" || "accrescent" but omits the "lite" flavor; update the logic in the excludesGoogleDrive getter (or rename the property to reflect the real cause, e.g., excludesGooglePlayServices) so it either includes "lite" in the flavor checks (BuildConfig.FLAVOR == "fdroid" || "accrescent" || "lite") or rename the property to a more accurate name and adjust callers, ensuring all flavors that lack Google Play Services are covered consistently by the single property (reference: excludesGoogleDrive property and BuildConfig.FLAVOR string literals "fdroid", "accrescent", "lite").util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt (3)
169-176: Naming inconsistency: method names use plural "Listeners" but operate on a single listener.Consider renaming to
addLicenseChangedListenerandremoveLicenseChangedListener(singular) for consistency with the existingaddLockTimeoutChangedListenerpattern on line 55.Suggested rename
- fun addLicenseChangedListeners(listener: Consumer<String>) { + fun addLicenseChangedListener(listener: Consumer<String>) { licenseChangedListeners[listener] = null listener.accept(licenseToken()) } - fun removeLicenseChangedListeners(listener: Consumer<String>) { + fun removeLicenseChangedListener(listener: Consumer<String>) { licenseChangedListeners.remove(listener) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt` around lines 169 - 176, Rename the two methods addLicenseChangedListeners and removeLicenseChangedListeners to the singular forms addLicenseChangedListener and removeLicenseChangedListener to match the existing addLockTimeoutChangedListener pattern; update their declarations and all call sites to the new names and keep their behavior unchanged (they should still add/remove the Consumer<String> to the licenseChangedListeners map and call listener.accept(licenseToken()) on add); also update any documentation or references to licenseChangedListeners if they refer to the plural method names.
178-192: Indentation is inconsistent inonSharedPreferenceChanged.The method body has mixed indentation levels that reduce readability. The
whenblock and its branches should align consistently.Suggested formatting fix
-override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences, key: String?) { - when (key) { - LOCK_TIMEOUT -> { + override fun onSharedPreferenceChanged(sharedPreferences: SharedPreferences, key: String?) { + when (key) { + LOCK_TIMEOUT -> { val lockTimeout = lockTimeout lockTimeoutChangedListeners.keys.forEach { listener -> - listener.accept(lockTimeout) - } - } - LICENSE_TOKEN, TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION -> { - licenseChangedListeners.keys.forEach { listener -> - listener.accept(licenseToken()) + listener.accept(lockTimeout) + } + } + LICENSE_TOKEN, TRIAL_EXPIRATION_DATE, HAS_RUNNING_SUBSCRIPTION -> { + licenseChangedListeners.keys.forEach { listener -> + listener.accept(licenseToken()) + } + } } } -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt` around lines 178 - 192, The onSharedPreferenceChanged method has inconsistent indentation around the when branches (especially the LOCK_TIMEOUT branch and its nested listener loop) — fix by aligning the when block and each branch consistently, ensure the LOCK_TIMEOUT branch’s opening and closing braces line up with the when entries, and indent the inner loop and listener.accept calls uniformly; update the block around LOCK_TIMEOUT, lockTimeoutChangedListeners, LICENSE_TOKEN/TRIAL_EXPIRATION_DATE/HAS_RUNNING_SUBSCRIPTION, licenseChangedListeners and licenseToken() so all braces and nested statements follow the same indentation style as the rest of the class.
24-25: Thread-safety consideration forWeakHashMap-based listeners.
WeakHashMapis not thread-safe, andonSharedPreferenceChangedcan be invoked from any thread. If listeners are added/removed from a different thread than the callback, this could causeConcurrentModificationException. The existinglockTimeoutChangedListenershas the same pattern, so this may be an accepted risk in this codebase, but worth noting for the new license listeners.Consider using
Collections.synchronizedMap(WeakHashMap())or ensuring all listener operations happen on the same thread.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt` around lines 24 - 25, lockTimeoutChangedListeners and the new licenseChangedListeners are backed by java.util.WeakHashMap which is not thread-safe and onSharedPreferenceChanged may be called from any thread; replace these maps with thread-safe variants (e.g., wrap the WeakHashMap using Collections.synchronizedMap(...) or use a ConcurrentHashMap-based weak-key implementation) or enforce that all listener add/remove and notification in onSharedPreferenceChanged run on the same single thread/handler; update the fields lockTimeoutChangedListeners and licenseChangedListeners and adjust add/remove/iteration logic in onSharedPreferenceChanged accordingly to avoid ConcurrentModificationException.presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.kt (1)
31-34: Minor inconsistency: in-app clearing checks existing token, subscription clearing does not.
handleInAppPurchasescheckslicenseToken().isNotEmpty()before clearing (line 31), buthandleSubscriptionPurchasesunconditionally setsfalse(line 56). Consider aligning the behavior for consistency, though this is a minor nit.Optional: align clearing logic
if (clearIfNotFound) { + if (sharedPreferencesHandler.hasRunningSubscription()) { sharedPreferencesHandler.setHasRunningSubscription(false) + } }Also applies to: 55-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.kt` around lines 31 - 34, handleSubscriptionPurchases unconditionally clears the subscription flag while handleInAppPurchases only clears the license when a token exists; make them consistent by checking the current state before clearing. In handleSubscriptionPurchases call sharedPreferencesHandler.subscriptionActive() (or the existing getter) and only call sharedPreferencesHandler.setSubscriptionActive(false) and log the removal if the subscription was true, mirroring the licenseToken().isNotEmpty() guard used in handleInAppPurchases so both flows behave the same.presentation/src/main/res/layout/fragment_welcome_intro.xml (1)
12-21: Consider adding an accessibility content description for the image.Setting
contentDescription="@null"means screen readers will skip this image. If the image is purely decorative, this is acceptable. However, for a welcome screen mascot, providing a brief description like"Cryptomator logo"could improve the experience for users with screen readers.Optional: add content description
<androidx.appcompat.widget.AppCompatImageView android:id="@+id/ivCryptobot" android:layout_width="280dp" android:layout_height="280dp" android:layout_gravity="center_horizontal" android:layout_marginTop="24dp" android:adjustViewBounds="true" - android:contentDescription="@null" + android:contentDescription="@string/app_name" android:scaleType="fitCenter" app:srcCompat="@drawable/ic_launcher_foreground" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/res/layout/fragment_welcome_intro.xml` around lines 12 - 21, The ImageView with id ivCryptobot currently uses contentDescription="@null"; update it to provide an accessibility label (e.g., contentDescription="@string/cryptobot_description" or a literal like "Cryptobot mascot") unless the image is intentionally decorative, in which case keep `@null`; add the corresponding string resource (cryptobot_description) to strings.xml and reference that resource from the AppCompatImageView to ensure screen readers receive a descriptive label.presentation/src/main/res/layout/activity_welcome.xml (1)
55-63: Remove unusedlayout_anchorGravityattribute.The
app:layout_anchorGravityattribute onbtnNexthas no effect since the button is a child ofLinearLayout, not a direct child ofCoordinatorLayout. This attribute is only meaningful for views that use anchor-based positioning within aCoordinatorLayout.Suggested fix
<Button android:id="@+id/btnNext" android:layout_width="0dp" android:layout_height="wrap_content" android:layout_weight="1" - android:text="@string/next" - android:layout_gravity="bottom|end" - app:layout_anchorGravity="bottom|right|end" /> + android:text="@string/next" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/res/layout/activity_welcome.xml` around lines 55 - 63, The attribute app:layout_anchorGravity on the Button with id btnNext is unused because btnNext is inside a LinearLayout (not a CoordinatorLayout); remove the app:layout_anchorGravity attribute from the btnNext element to clean up the layout XML and rely on valid LinearLayout attributes such as android:layout_gravity if you need positioning.data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt (1)
993-996: Cover the premium-flavor path explicitly.The current assertion only checks token migration for non-premium builds. If premium builds start copying the DB token into preferences, this test would still pass.
✅ Cover the "do not migrate" branch too
Assert.assertThat(sharedPreferencesHandler.hasCompletedWelcomeFlow(), CoreMatchers.`is`(true)) if (!FlavorConfig.isPremiumFlavor) { Assert.assertThat(sharedPreferencesHandler.licenseToken(), CoreMatchers.`is`(licenseToken)) + } else { + Assert.assertThat(sharedPreferencesHandler.licenseToken(), CoreMatchers.`is`("")) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt` around lines 993 - 996, The test in UpgradeDatabaseTest currently only asserts migrated token for non-premium builds; update the assertions around sharedPreferencesHandler.licenseToken() to explicitly cover both branches: if FlavorConfig.isPremiumFlavor assert that sharedPreferencesHandler.licenseToken() indicates no migration (e.g., is null or empty), otherwise assert it equals the expected licenseToken value as currently done; keep the existing check for hasCompletedWelcomeFlow() and change the if/else to explicitly assert the "do not migrate" behavior for the premium path so the test fails if premium builds start copying the DB token into preferences.data/src/main/java/org/cryptomator/data/db/Upgrade13To14.kt (1)
27-29: Consider renaming for clarity.
nonLicenseKeyVariant()with!nonLicenseKeyVariant()creates a double-negative that's harder to parse. A name likeshouldMigrateLicenseToken()or inverting the logic would improve readability.♻️ Suggested rename
- if (!nonLicenseKeyVariant()) { + if (shouldMigrateLicenseToken()) { val licenseToken = getExistingLicenseToken(db) ... } ... -private fun nonLicenseKeyVariant(): Boolean { - return FlavorConfig.isPremiumFlavor -} +private fun shouldMigrateLicenseToken(): Boolean { + return !FlavorConfig.isPremiumFlavor +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/java/org/cryptomator/data/db/Upgrade13To14.kt` around lines 27 - 29, The method nonLicenseKeyVariant() is misnamed and causes double-negatives when used as !nonLicenseKeyVariant(); rename it to a clearer positive form (e.g., shouldMigrateLicenseToken() or isLicenseKeyVariant()) and invert its boolean return logic accordingly so callers don't need a negation; update the implementation that currently returns FlavorConfig.isPremiumFlavor and adjust all call sites to use the new name (or inverted logic) to keep behavior unchanged while improving readability.presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt (1)
167-176: Potential deadlock if callbacks re-enterqueryProductDetails.The callbacks in
drainPendingProductDetailsCallbacksare invoked while potentially still holding implicit state (though the synchronized block exits before binder callback fires). However, ifbinder.queryProductDetailsinvokes the callback synchronously on the same thread, and that callback callsqueryProductDetailsagain, a deadlock could occur since the outer call may still be in progress.Consider copying and clearing the list before invoking the binder, ensuring callbacks execute outside any synchronized context:
♻️ Safer callback draining
private fun drainPendingProductDetailsCallbacks() { + val callbacks: List<(List<ProductInfo>) -> Unit> synchronized(pendingProductDetailsCallbacks) { if (pendingProductDetailsCallbacks.isEmpty()) return - val callbacks = ArrayList(pendingProductDetailsCallbacks) + callbacks = ArrayList(pendingProductDetailsCallbacks) pendingProductDetailsCallbacks.clear() - iapBillingServiceBinder?.queryProductDetails { products -> - callbacks.forEach { it(products) } - } } + iapBillingServiceBinder?.queryProductDetails { products -> + callbacks.forEach { it(products) } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt` around lines 167 - 176, The method drainPendingProductDetailsCallbacks holds a synchronized lock on pendingProductDetailsCallbacks while calling iapBillingServiceBinder?.queryProductDetails, which can deadlock if queryProductDetails invokes the provided callback synchronously and that callback re-enters queryProductDetails; fix by copying pendingProductDetailsCallbacks to a new local list and clearing the shared list while inside the synchronized block, then release the lock and call iapBillingServiceBinder?.queryProductDetails using the copied list (i.e., move the queryProductDetails invocation outside the synchronized block) so callbacks run without holding the lock; reference drainPendingProductDetailsCallbacks, pendingProductDetailsCallbacks and iapBillingServiceBinder.queryProductDetails.presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseStateOrchestrator.kt (1)
23-29: Consider guarding against duplicate listener registration.If
onResume()is called multiple times without an interveningonPause()(e.g., due to lifecycle edge cases), the same listener could be registered multiple times, causingupdateState()to be called redundantly on preference changes.💡 Suggested improvement
+ private var isListenerRegistered = false + fun onResume() { + if (!isListenerRegistered) { sharedPreferencesHandler.addLicenseChangedListeners(licenseChangeListener) + isListenerRegistered = true + } updateState() if (FlavorConfig.isFreemiumFlavor) { priceLoader?.invoke() } } fun onPause() { sharedPreferencesHandler.removeLicenseChangedListeners(licenseChangeListener) + isListenerRegistered = false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseStateOrchestrator.kt` around lines 23 - 29, The onResume() method currently calls sharedPreferencesHandler.addLicenseChangedListeners(licenseChangeListener) unconditionally, which can register the same listener multiple times; modify the LicenseStateOrchestrator to track listener registration (e.g., a private boolean like isLicenseListenerRegistered) and only call addLicenseChangedListeners(licenseChangeListener) when not already registered, and ensure the corresponding onPause() removes the listener and clears the flag (using sharedPreferencesHandler.removeLicenseChangedListeners or the existing removal method) so updateState() isn't invoked redundantly.presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt (2)
130-149: Consider simplifying the parallel query completion tracking.The manual
queriesCompletedcounter withsynchronized(lock)works but is fragile. If a third query is added later,totalQueriesmust be updated manually.💡 Alternative using CountDownLatch
fun queryProductDetails(callback: (List<ProductInfo>) -> Unit) { if (!billingClient.isReady) { synchronized(pendingProductDetailsCallbacks) { pendingProductDetailsCallbacks.add(callback) } return } val results = java.util.concurrent.CopyOnWriteArrayList<ProductInfo>() val latch = java.util.concurrent.CountDownLatch(2) // ... async queries that call latch.countDown() on completion // On a background thread: Thread { latch.await() callback(results.toList()) }.start() }This is optional — the current implementation is correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt` around lines 130 - 149, The completion tracking in queryProductDetails currently uses a fragile manual counter (queriesCompleted) and a hard-coded totalQueries guarded by lock inside onQueryComplete; replace this with a thread-safe synchronisation primitive (e.g., java.util.concurrent.CountDownLatch or an AtomicInteger with known expected count) so adding/removing async queries won't require changing a separate totalQueries constant — update the code in queryProductDetails (references: queriesCompleted, totalQueries, lock, onQueryComplete, results) to initialise a CountDownLatch with the actual number of launched queries, call countDown() from each async completion, and await the latch before invoking callback(results.toList()) (or use a CopyOnWriteArrayList for results to avoid additional locking).
36-37: Thread-safety concern:pendingProductDetailsCallbacksshould be thread-safe.
pendingProductDetailsCallbacksis amutableListOfaccessed from multiple threads (billing callbacks run on background threads). While you synchronize in some places, consider using a thread-safe collection to prevent subtle concurrency bugs.💡 Suggested fix
- private val pendingProductDetailsCallbacks = mutableListOf<(List<ProductInfo>) -> Unit>() + private val pendingProductDetailsCallbacks = java.util.Collections.synchronizedList(mutableListOf<(List<ProductInfo>) -> Unit>())Note: You'll still need
synchronizedblocks when iterating, but this prevents issues if a callback is added while the list is being modified elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt` around lines 36 - 37, The mutable list pendingProductDetailsCallbacks is not thread-safe; replace it with a thread-safe collection such as a java.util.concurrent.CopyOnWriteArrayList or ConcurrentLinkedQueue to hold callbacks (while keeping productDetailsMap as-is), update references to pendingProductDetailsCallbacks accordingly, and retain synchronized blocks when iterating or performing multi-step operations to avoid races; locate usages of pendingProductDetailsCallbacks in IapBillingService.kt to change construction and any mutation/iteration sites.presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt (1)
33-36: Consider logging or warning when the hosting Activity doesn't implement Listener.The
as?safe-cast silently setslistenertonullif the context doesn't implementListener. This could lead to silent failures where button clicks do nothing. Consider adding a warning log or throwing anIllegalStateExceptionduring development.💡 Suggested improvement
override fun onAttach(context: Context) { super.onAttach(context) - listener = context as? Listener + listener = context as? Listener + if (listener == null) { + Timber.tag("WelcomeLicenseFragment").w("Host activity does not implement Listener interface") + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt` around lines 33 - 36, The onAttach method currently uses a safe-cast (context as? Listener) which silently leaves listener null if the hosting Activity doesn't implement Listener; update onAttach to detect this case and either log a warning (using the fragment's logger) or throw an IllegalStateException so failures are fail-fast during development—modify the onAttach override (and the listener field) to check the result of the cast and call processLogger/warn or throw IllegalStateException with a clear message referencing Listener and the fragment class name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt`:
- Line 991: The test calls Upgrade13To14(sharedPreferencesHandler).applyTo(db,
12) while the DB has already been advanced to schema version 13 by
Upgrade12To13(...).applyTo(db, 12); update the test to pass schema version 13
into Upgrade13To14 so it exercises the 13→14 migration contract (i.e., call
Upgrade13To14(...).applyTo(db, 13) after running Upgrade12To13(...).applyTo(db,
12)), and make the same change for the other occurrence referenced.
In `@data/src/main/java/org/cryptomator/data/db/Upgrade13To14.kt`:
- Around line 58-67: In getExistingLicenseToken, guard against
getColumnIndex("LICENSE_TOKEN") returning -1 before calling getString: fetch the
index into a variable, check if index >= 0 and only then call
it.getString(index); if the index is -1 return null (or log an error) so the SQL
query "UPDATE_CHECK_ENTITY" path doesn't call getString(-1) and crash on
unexpected schemas.
In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt`:
- Around line 263-266: The onActivityResumed handler currently calls
restorePurchases() unconditionally; add debouncing by tracking the last restore
timestamp (e.g., a private AtomicLong lastRestoreMillis or similar in the same
class) and only call restorePurchases() when System.currentTimeMillis() -
lastRestoreMillis >= 30_000 (30 seconds), updating lastRestoreMillis immediately
before/after invoking restorePurchases(); ensure thread-safety by using
AtomicLong.compareAndSet or synchronizing the check/update so multiple rapid
resumes don't bypass the interval, and keep the call to
updateService(resumedActivities.incrementAndGet()) unchanged.
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt`:
- Around line 83-92: The trial logic in startTrial() and hasActiveTrial() relies
solely on System.currentTimeMillis() which allows bypass by rolling the device
clock; modify the logic to detect and prevent backward time jumps or use a
trusted time source: when starting a trial in startTrial(), record both the
wall-clock expiration (trialExpiration) and a monotonic baseline /
lastSeenRealtime (e.g., SystemClock.elapsedRealtime() or similar) in
sharedPreferencesHandler along with a lastSeenWallClock timestamp; in
hasActiveTrial(), compute expiration using the stored monotonic baseline plus
TRIAL_DURATION_MS (or compare current monotonic time against stored monotonic
expiration) and also reject trials if current wall-clock < lastSeenWallClock
(indicating rollback) so stored trial state is locked until verified with a
trusted timestamp or remote check; update methods that read/write
trialExpirationDate() to store/validate these additional fields to prevent clock
rollback bypass.
In
`@presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.kt`:
- Around line 390-401: In onVaultSelected, when a locked vault selection is
rejected you null selectedVault but must also clear the stored destination to
prevent prepareSavingFiles from using the previous location; set location = null
alongside selectedVault and clear the displayed destination in the view (e.g.
call the view's method to clear destination such as view?.setDestination(null)
or view?.clearDestination() near where view?.setUploadEnabled(false) is invoked)
so the UI/state remain consistent.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt`:
- Around line 280-286: The auto-advance runnable in autoAdvanceToNextPage()
reads the pager position when it fires, so if the user navigates before the
delay it can skip a page; to fix, capture the current position into a local
expectedPos when scheduling (inside autoAdvanceToNextPage) and in the Runnable
only advance if binding.welcomePager.currentItem == expectedPos, otherwise do
nothing; additionally ensure any manual navigation handlers (e.g., the Next
button click handler and page change listeners) call
binding.welcomePager.removeCallbacks(<theRunnable>) or otherwise cancel pending
runnables so scheduled advances don't run after manual navigation.
- Around line 111-114: validate(intent) calls showOrUpdateLicenseEntry() before
the licenseFragment exists on cold-start deep links, causing the license value
to be dropped; modify WelcomeActivity (and the pager/adapter used by
setupPages()/setupPager()) to cache a pending license value when
showOrUpdateLicenseEntry() is invoked and licenseFragment is null, expose a
method in the adapter (e.g., setPendingLicense(String)) or store the pending
value on the activity, and ensure the adapter/WelcomeActivity applies that
cached value to the actual licenseFragment as soon as the fragment is
created/attached (invoke the application in the fragment creation path where
licenseFragment is set). Ensure the same cached-then-apply logic is used for the
other occurrence mentioned (lines 225-227).
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeNotificationsFragment.kt`:
- Around line 32-36: The updatePermissionState function currently only checks
isAdded before touching the view binding, which can still be non-null after
onDestroyView when using ViewPager2; modify updatePermissionState to also guard
against a destroyed view by returning early if view == null (or if the
view-binding is null/cleared) before accessing binding, e.g., check both isAdded
and that the fragment's view (or binding) is non-null; updatePermissionState,
binding, isAdded and onDestroyView are the relevant symbols to change.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeScreenLockFragment.kt`:
- Around line 32-39: The updateScreenLockState function currently checks only
isAdded before touching the fragment view binding which can still be null after
onDestroyView; change the guard to ensure the view/binding exists (e.g., return
early if view == null or binding is null) before accessing
binding.btnSetScreenLock, binding.cbSetScreenLock, binding.tvScreenLockStatus,
and make sure any references to binding occur only while the fragment's view is
alive (i.e., after onCreateView and before onDestroyView).
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.kt`:
- Around line 97-105: The issue is that bindTrialState(...) can re-enable trial
UI after bindPurchaseState(...) has hidden it for paid users; update the logic
so trial UI stays suppressed when hasPaidLicense is true: modify bindTrialState
to accept a hasPaidLicense boolean (or check a shared paid flag set by
bindPurchaseState) and early-return or skip setting visibility for
binding.tvInfoText, binding.tvTrialStatusBadge, binding.tvTrialExpiration,
binding.tvTrialExpiration and any trial CTA when hasPaidLicense is true;
alternatively move all trial visibility control into bindPurchaseState and
ensure bindTrialState does nothing if the paid flag is set.
In `@presentation/src/main/res/layout/dialog_trial_expired.xml`:
- Around line 6-9: The RelativeLayout used as the direct child of the
NestedScrollView currently has android:layout_height="match_parent" which
prevents proper scrolling for long dialog content; update the RelativeLayout
(the tag named "RelativeLayout" in this diff) to use
android:layout_height="wrap_content" so the child can grow and allow
NestedScrollView to scroll when content exceeds the viewport.
In `@presentation/src/main/res/layout/fragment_welcome_screen_lock.xml`:
- Around line 41-48: The TextView with id tvScreenLockStatus is toggled visible
after returning from the system lock-settings flow but isn't announced to
TalkBack users; update the fragment_welcome_screen_lock layout to make this view
accessible by adding properties so screen readers announce its text when
visibility changes — specifically set android:accessibilityLiveRegion="polite"
(or "assertive" if immediate interruption is required) and
android:importantForAccessibility="yes" on the TextView (optionally ensure it
has a contentDescription or its text is sufficient) so the change from
gone→visible is announced to accessibility services.
In `@presentation/src/main/res/layout/view_license_check_content.xml`:
- Around line 39-42: The decorative checkmark ImageView elements (the
<ImageView> instances with android:src="@drawable/ic_checkmark") are missing
accessibility attributes; update each of these ImageView tags (the three
occurrences around lines 39-42, 59-62, 79-82) to mark them as decorative by
adding either android:contentDescription="@null" or
android:importantForAccessibility="no" so TalkBack will ignore them while
leaving the adjacent TextView labels for screen readers.
In
`@presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/presenter/VaultListPresenterFreemiumTest.kt`:
- Line 94: The test currently verifies vaultListView.showDialog(anyNonNull()),
which can match any dialog; update the assertions in
VaultListPresenterFreemiumTest so they explicitly target the TrialExpiredDialog
type: replace anyNonNull() with a type matcher for TrialExpiredDialog (e.g.,
isA(TrialExpiredDialog::class.java) or an argThat that checks instance-of
TrialExpiredDialog) for the positive checks, and for the negative checks use
verify(vaultListView, never()).showDialog(...) with the same TrialExpiredDialog
matcher to assert that TrialExpiredDialog specifically was not shown when
resumed() is exercised.
---
Outside diff comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt`:
- Around line 540-559: The presenter currently checks write access only when
setting up the external edit (using licenseEnforcer.hasWriteAccessForVault(...)
in the open flow) but does not re-verify before uploading edits; update
openFileFinished() (or inside uploadChangedFile()) to call
licenseEnforcer.hasWriteAccessForVault(view?.folder?.vault()) again and abort
(log/show error and skip calling uploadChangedFile(uploadedUri, openedCloudFile,
openedCloudFileMd5) or return early from uploadChangedFile) if the check fails,
ensuring you reference openedCloudFile/openedCloudFileMd5 and
view?.folder?.vault() so the upload is gated by the fresh permission check.
In
`@presentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.kt`:
- Line 84: The injected dependency licenseCheckUseCase is unused in
VaultListPresenter; remove it from the class constructor parameter list and any
corresponding property/field, and delete its entry from the
unsubscribeOnDestroy(...) call so it is no longer registered for cleanup; update
the VaultListPresenter constructor signature and remove any import or field
reference to DoLicenseCheckUseCase to keep the class consistent.
---
Nitpick comments:
In `@buildsystem/dependencies.gradle`:
- Around line 108-109: Update the billingclientVersion constant to use the newer
Google Play Billing Library version by changing the value of
billingclientVersion from '8.2.1' to '8.3.0' in buildsystem/dependencies.gradle;
verify compatibility by running the build and any billing-related tests after
updating and, if needed, adjust any references to APIs introduced or changed
between versions in modules that depend on billingclientVersion.
In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt`:
- Around line 993-996: The test in UpgradeDatabaseTest currently only asserts
migrated token for non-premium builds; update the assertions around
sharedPreferencesHandler.licenseToken() to explicitly cover both branches: if
FlavorConfig.isPremiumFlavor assert that sharedPreferencesHandler.licenseToken()
indicates no migration (e.g., is null or empty), otherwise assert it equals the
expected licenseToken value as currently done; keep the existing check for
hasCompletedWelcomeFlow() and change the if/else to explicitly assert the "do
not migrate" behavior for the premium path so the test fails if premium builds
start copying the DB token into preferences.
In `@data/src/main/java/org/cryptomator/data/db/Upgrade13To14.kt`:
- Around line 27-29: The method nonLicenseKeyVariant() is misnamed and causes
double-negatives when used as !nonLicenseKeyVariant(); rename it to a clearer
positive form (e.g., shouldMigrateLicenseToken() or isLicenseKeyVariant()) and
invert its boolean return logic accordingly so callers don't need a negation;
update the implementation that currently returns FlavorConfig.isPremiumFlavor
and adjust all call sites to use the new name (or inverted logic) to keep
behavior unchanged while improving readability.
In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt`:
- Around line 167-176: The method drainPendingProductDetailsCallbacks holds a
synchronized lock on pendingProductDetailsCallbacks while calling
iapBillingServiceBinder?.queryProductDetails, which can deadlock if
queryProductDetails invokes the provided callback synchronously and that
callback re-enters queryProductDetails; fix by copying
pendingProductDetailsCallbacks to a new local list and clearing the shared list
while inside the synchronized block, then release the lock and call
iapBillingServiceBinder?.queryProductDetails using the copied list (i.e., move
the queryProductDetails invocation outside the synchronized block) so callbacks
run without holding the lock; reference drainPendingProductDetailsCallbacks,
pendingProductDetailsCallbacks and iapBillingServiceBinder.queryProductDetails.
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseStateOrchestrator.kt`:
- Around line 23-29: The onResume() method currently calls
sharedPreferencesHandler.addLicenseChangedListeners(licenseChangeListener)
unconditionally, which can register the same listener multiple times; modify the
LicenseStateOrchestrator to track listener registration (e.g., a private boolean
like isLicenseListenerRegistered) and only call
addLicenseChangedListeners(licenseChangeListener) when not already registered,
and ensure the corresponding onPause() removes the listener and clears the flag
(using sharedPreferencesHandler.removeLicenseChangedListeners or the existing
removal method) so updateState() isn't invoked redundantly.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`:
- Around line 33-36: The onAttach method currently uses a safe-cast (context as?
Listener) which silently leaves listener null if the hosting Activity doesn't
implement Listener; update onAttach to detect this case and either log a warning
(using the fragment's logger) or throw an IllegalStateException so failures are
fail-fast during development—modify the onAttach override (and the listener
field) to check the result of the cast and call processLogger/warn or throw
IllegalStateException with a clear message referencing Listener and the fragment
class name.
In `@presentation/src/main/res/drawable/ic_checkmark.xml`:
- Around line 7-9: The drawable ic_checkmark.xml currently hardcodes
android:fillColor="@android:color/holo_green_dark" (day) and "#ffffff" (night
variant); replace these with a reusable color resource or theme attribute by
creating a color resource like `@color/checkmark_color` with day/night values or
referencing a theme attribute (e.g., ?attr/checkmarkColor) and update the
android:fillColor on the <path> element to use that resource/attribute so both
day and night drawables use the same named color.
In `@presentation/src/main/res/layout/activity_welcome.xml`:
- Around line 55-63: The attribute app:layout_anchorGravity on the Button with
id btnNext is unused because btnNext is inside a LinearLayout (not a
CoordinatorLayout); remove the app:layout_anchorGravity attribute from the
btnNext element to clean up the layout XML and rely on valid LinearLayout
attributes such as android:layout_gravity if you need positioning.
In `@presentation/src/main/res/layout/fragment_welcome_intro.xml`:
- Around line 12-21: The ImageView with id ivCryptobot currently uses
contentDescription="@null"; update it to provide an accessibility label (e.g.,
contentDescription="@string/cryptobot_description" or a literal like "Cryptobot
mascot") unless the image is intentionally decorative, in which case keep `@null`;
add the corresponding string resource (cryptobot_description) to strings.xml and
reference that resource from the AppCompatImageView to ensure screen readers
receive a descriptive label.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.kt`:
- Around line 130-149: The completion tracking in queryProductDetails currently
uses a fragile manual counter (queriesCompleted) and a hard-coded totalQueries
guarded by lock inside onQueryComplete; replace this with a thread-safe
synchronisation primitive (e.g., java.util.concurrent.CountDownLatch or an
AtomicInteger with known expected count) so adding/removing async queries won't
require changing a separate totalQueries constant — update the code in
queryProductDetails (references: queriesCompleted, totalQueries, lock,
onQueryComplete, results) to initialise a CountDownLatch with the actual number
of launched queries, call countDown() from each async completion, and await the
latch before invoking callback(results.toList()) (or use a CopyOnWriteArrayList
for results to avoid additional locking).
- Around line 36-37: The mutable list pendingProductDetailsCallbacks is not
thread-safe; replace it with a thread-safe collection such as a
java.util.concurrent.CopyOnWriteArrayList or ConcurrentLinkedQueue to hold
callbacks (while keeping productDetailsMap as-is), update references to
pendingProductDetailsCallbacks accordingly, and retain synchronized blocks when
iterating or performing multi-step operations to avoid races; locate usages of
pendingProductDetailsCallbacks in IapBillingService.kt to change construction
and any mutation/iteration sites.
In
`@presentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.kt`:
- Around line 31-34: handleSubscriptionPurchases unconditionally clears the
subscription flag while handleInAppPurchases only clears the license when a
token exists; make them consistent by checking the current state before
clearing. In handleSubscriptionPurchases call
sharedPreferencesHandler.subscriptionActive() (or the existing getter) and only
call sharedPreferencesHandler.setSubscriptionActive(false) and log the removal
if the subscription was true, mirroring the licenseToken().isNotEmpty() guard
used in handleInAppPurchases so both flows behave the same.
In `@util/src/main/java/org/cryptomator/util/FlavorConfig.kt`:
- Around line 21-23: The excludesGoogleDrive property currently checks
BuildConfig.FLAVOR == "fdroid" || "accrescent" but omits the "lite" flavor;
update the logic in the excludesGoogleDrive getter (or rename the property to
reflect the real cause, e.g., excludesGooglePlayServices) so it either includes
"lite" in the flavor checks (BuildConfig.FLAVOR == "fdroid" || "accrescent" ||
"lite") or rename the property to a more accurate name and adjust callers,
ensuring all flavors that lack Google Play Services are covered consistently by
the single property (reference: excludesGoogleDrive property and
BuildConfig.FLAVOR string literals "fdroid", "accrescent", "lite").
In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt`:
- Around line 169-176: Rename the two methods addLicenseChangedListeners and
removeLicenseChangedListeners to the singular forms addLicenseChangedListener
and removeLicenseChangedListener to match the existing
addLockTimeoutChangedListener pattern; update their declarations and all call
sites to the new names and keep their behavior unchanged (they should still
add/remove the Consumer<String> to the licenseChangedListeners map and call
listener.accept(licenseToken()) on add); also update any documentation or
references to licenseChangedListeners if they refer to the plural method names.
- Around line 178-192: The onSharedPreferenceChanged method has inconsistent
indentation around the when branches (especially the LOCK_TIMEOUT branch and its
nested listener loop) — fix by aligning the when block and each branch
consistently, ensure the LOCK_TIMEOUT branch’s opening and closing braces line
up with the when entries, and indent the inner loop and listener.accept calls
uniformly; update the block around LOCK_TIMEOUT, lockTimeoutChangedListeners,
LICENSE_TOKEN/TRIAL_EXPIRATION_DATE/HAS_RUNNING_SUBSCRIPTION,
licenseChangedListeners and licenseToken() so all braces and nested statements
follow the same indentation style as the rest of the class.
- Around line 24-25: lockTimeoutChangedListeners and the new
licenseChangedListeners are backed by java.util.WeakHashMap which is not
thread-safe and onSharedPreferenceChanged may be called from any thread; replace
these maps with thread-safe variants (e.g., wrap the WeakHashMap using
Collections.synchronizedMap(...) or use a ConcurrentHashMap-based weak-key
implementation) or enforce that all listener add/remove and notification in
onSharedPreferenceChanged run on the same single thread/handler; update the
fields lockTimeoutChangedListeners and licenseChangedListeners and adjust
add/remove/iteration logic in onSharedPreferenceChanged accordingly to avoid
ConcurrentModificationException.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aea10a5d-05d6-47cb-b3e8-859ed106de8c
📒 Files selected for processing (115)
buildsystem/dependencies.gradledata/build.gradledata/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.ktdata/src/main/java/org/cryptomator/data/db/DatabaseUpgrades.javadata/src/main/java/org/cryptomator/data/db/Upgrade13To14.ktdata/src/main/java/org/cryptomator/data/db/entities/UpdateCheckEntity.javadata/src/main/java/org/cryptomator/data/repository/HubRepositoryImpl.javadata/src/main/java/org/cryptomator/data/repository/UpdateCheckRepositoryImpl.javadomain/build.gradledomain/src/main/java/org/cryptomator/domain/Vault.javadomain/src/main/java/org/cryptomator/domain/repository/HubRepository.ktdomain/src/main/java/org/cryptomator/domain/repository/UpdateCheckRepository.javadomain/src/main/java/org/cryptomator/domain/usecases/DoLicenseCheck.javadomain/src/main/java/org/cryptomator/domain/usecases/vault/UnlockHubVault.javadomain/src/test/java/org/cryptomator/domain/usecases/DoLicenseCheckTest.javadomain/src/test/java/org/cryptomator/domain/usecases/vault/UnlockHubVaultTest.javapresentation/build.gradlepresentation/src/main/AndroidManifest.xmlpresentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.ktpresentation/src/main/java/org/cryptomator/presentation/di/component/ActivityComponent.javapresentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.ktpresentation/src/main/java/org/cryptomator/presentation/licensing/LicenseStateOrchestrator.ktpresentation/src/main/java/org/cryptomator/presentation/model/VaultModel.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/BaseLicensePresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/ChooseCloudServicePresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/CloudSettingsPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/SettingsPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/WelcomePresenter.ktpresentation/src/main/java/org/cryptomator/presentation/service/ProductInfo.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/BrowseFilesActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/SharedFilesActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/VaultListActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/view/SharedFilesView.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/view/UpdateLicenseView.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/view/WelcomeView.ktpresentation/src/main/java/org/cryptomator/presentation/ui/dialog/TrialExpiredDialog.ktpresentation/src/main/java/org/cryptomator/presentation/ui/dialog/UpdateLicenseDialog.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/SharedFilesFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/TextEditorFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeIntroFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeNotificationsFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeScreenLockFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.ktpresentation/src/main/res/drawable-night/ic_checkmark.xmlpresentation/src/main/res/drawable-night/ic_cryptomator_bot.xmlpresentation/src/main/res/drawable-night/ic_info_circle.xmlpresentation/src/main/res/drawable/ic_checkmark.xmlpresentation/src/main/res/drawable/ic_cryptomator_bot.xmlpresentation/src/main/res/drawable/ic_info_circle.xmlpresentation/src/main/res/layout/activity_license_check.xmlpresentation/src/main/res/layout/activity_welcome.xmlpresentation/src/main/res/layout/dialog_trial_expired.xmlpresentation/src/main/res/layout/fragment_welcome_intro.xmlpresentation/src/main/res/layout/fragment_welcome_license.xmlpresentation/src/main/res/layout/fragment_welcome_notifications.xmlpresentation/src/main/res/layout/fragment_welcome_screen_lock.xmlpresentation/src/main/res/layout/view_license_check_content.xmlpresentation/src/main/res/values-ar-rSA/strings.xmlpresentation/src/main/res/values-ba-rRU/strings.xmlpresentation/src/main/res/values-ca-rES/strings.xmlpresentation/src/main/res/values-cs-rCZ/strings.xmlpresentation/src/main/res/values-da-rDK/strings.xmlpresentation/src/main/res/values-de-rDE/strings.xmlpresentation/src/main/res/values-el-rGR/strings.xmlpresentation/src/main/res/values-es-rES/strings.xmlpresentation/src/main/res/values-fil-rPH/strings.xmlpresentation/src/main/res/values-fr-rFR/strings.xmlpresentation/src/main/res/values-hr-rHR/strings.xmlpresentation/src/main/res/values-hu-rHU/strings.xmlpresentation/src/main/res/values-in-rID/strings.xmlpresentation/src/main/res/values-it-rIT/strings.xmlpresentation/src/main/res/values-iw-rIL/strings.xmlpresentation/src/main/res/values-ja-rJP/strings.xmlpresentation/src/main/res/values-ko-rKR/strings.xmlpresentation/src/main/res/values-lv-rLV/strings.xmlpresentation/src/main/res/values-nb-rNO/strings.xmlpresentation/src/main/res/values-nl-rNL/strings.xmlpresentation/src/main/res/values-pa-rIN/strings.xmlpresentation/src/main/res/values-pl-rPL/strings.xmlpresentation/src/main/res/values-pt-rBR/strings.xmlpresentation/src/main/res/values-pt-rPT/strings.xmlpresentation/src/main/res/values-ro-rRO/strings.xmlpresentation/src/main/res/values-ru-rRU/strings.xmlpresentation/src/main/res/values-sk-rSK/strings.xmlpresentation/src/main/res/values-sv-rSE/strings.xmlpresentation/src/main/res/values-sw-rTZ/strings.xmlpresentation/src/main/res/values-te-rIN/strings.xmlpresentation/src/main/res/values-th-rTH/strings.xmlpresentation/src/main/res/values-tr-rTR/strings.xmlpresentation/src/main/res/values-uk-rUA/strings.xmlpresentation/src/main/res/values-vi-rVN/strings.xmlpresentation/src/main/res/values-zh-rCN/strings.xmlpresentation/src/main/res/values-zh-rHK/strings.xmlpresentation/src/main/res/values-zh-rTW/strings.xmlpresentation/src/main/res/values/strings.xmlpresentation/src/nonplaystoreiap/java/org/cryptomator/presentation/service/IapBillingService.ktpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/IapBillingService.ktpresentation/src/playstoreiap/java/org/cryptomator/presentation/service/PurchaseManager.ktpresentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.ktpresentation/src/test/java/org/cryptomator/presentation/presenter/VaultListPresenterTest.javapresentation/src/test/java/org/cryptomator/presentation/service/ProductInfoTest.ktpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/presenter/VaultListPresenterFreemiumTest.ktpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/service/PurchaseManagerTest.ktutil/build.gradleutil/src/main/java/org/cryptomator/util/FlavorConfig.ktutil/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt
💤 Files with no reviewable changes (40)
- presentation/src/main/res/values-te-rIN/strings.xml
- presentation/src/main/res/values-in-rID/strings.xml
- presentation/src/main/res/values-es-rES/strings.xml
- presentation/src/main/res/values-ru-rRU/strings.xml
- presentation/src/main/res/values-fil-rPH/strings.xml
- data/src/main/java/org/cryptomator/data/repository/UpdateCheckRepositoryImpl.java
- presentation/src/main/res/values-iw-rIL/strings.xml
- presentation/src/main/res/values-ar-rSA/strings.xml
- presentation/src/main/res/values-ca-rES/strings.xml
- presentation/src/main/res/values-ja-rJP/strings.xml
- presentation/src/main/res/values-nl-rNL/strings.xml
- domain/src/main/java/org/cryptomator/domain/repository/UpdateCheckRepository.java
- presentation/src/main/res/values-sw-rTZ/strings.xml
- presentation/src/main/res/values-pl-rPL/strings.xml
- presentation/src/main/res/values-pa-rIN/strings.xml
- presentation/src/main/res/values-cs-rCZ/strings.xml
- presentation/src/main/res/values-pt-rPT/strings.xml
- presentation/src/main/res/values-th-rTH/strings.xml
- presentation/src/main/res/values-ro-rRO/strings.xml
- presentation/src/main/res/values-sv-rSE/strings.xml
- presentation/src/main/res/values-zh-rTW/strings.xml
- presentation/src/main/res/values-hu-rHU/strings.xml
- presentation/src/main/res/values-el-rGR/strings.xml
- presentation/src/main/res/values-nb-rNO/strings.xml
- presentation/src/main/res/values-vi-rVN/strings.xml
- presentation/src/main/res/values-fr-rFR/strings.xml
- presentation/src/main/res/values-it-rIT/strings.xml
- presentation/src/main/res/values-hr-rHR/strings.xml
- presentation/src/main/res/values-ba-rRU/strings.xml
- presentation/src/main/res/values-da-rDK/strings.xml
- presentation/src/main/res/values-lv-rLV/strings.xml
- presentation/src/main/res/values-zh-rCN/strings.xml
- presentation/src/main/res/values-zh-rHK/strings.xml
- presentation/src/main/res/values-de-rDE/strings.xml
- presentation/src/main/res/values-uk-rUA/strings.xml
- presentation/src/main/res/values-ko-rKR/strings.xml
- presentation/src/main/res/values-pt-rBR/strings.xml
- presentation/src/main/res/values-tr-rTR/strings.xml
- presentation/src/main/res/values-sk-rSK/strings.xml
- presentation/src/main/java/org/cryptomator/presentation/ui/dialog/UpdateLicenseDialog.kt
data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.kt
Show resolved
Hide resolved
...stPlaystoreiap/java/org/cryptomator/presentation/presenter/VaultListPresenterFreemiumTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.kt (1)
309-314: Consider capturingselectedVaultin a local variable for null-safety.Since
selectedVaultis a mutable@JvmFieldproperty, Kotlin cannot smart-cast it to non-null after the null check. While the single-threaded UI context makes actual mutation between lines 309 and 311 unlikely, capturing the value ensures compile-time safety and clearer intent.🔧 Optional refactor
fun onSaveButtonPressed(filesForUpload: List<SharedFileModel>) { - if (selectedVault != null && !licenseEnforcer.hasWriteAccessForVault(selectedVault)) { + val vault = selectedVault + if (vault != null && !licenseEnforcer.hasWriteAccessForVault(vault)) { view?.let { v -> - licenseEnforcer.ensureWriteAccessForVault(v.activity(), selectedVault, LicenseEnforcer.LockedAction.UPLOAD_FILES) + licenseEnforcer.ensureWriteAccessForVault(v.activity(), vault, LicenseEnforcer.LockedAction.UPLOAD_FILES) } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.kt` around lines 309 - 314, Capture the current selectedVault into a local val (e.g., val vault = selectedVault) before the null-check to allow safe non-null usage; then use vault in the condition and in calls to licenseEnforcer.hasWriteAccessForVault(vault) and licenseEnforcer.ensureWriteAccessForVault(..., vault, LicenseEnforcer.LockedAction.UPLOAD_FILES) and inside view?.let to avoid relying on Kotlin smart-cast of the mutable `@JvmField` selectedVault.util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt (1)
206-228: Atomicity of entitlement state updates is a design concern but not currently manifested in the codebase.Each setter method individually calls
apply()and triggers the shared preferences listener. The listener publishes to alllicenseChangedListenerson any key change. However, current call sites never chain multiple entitlement setters—each location (PurchaseManager,LicenseEnforcer, migrations, license checks) updates only one field per operation. If future code chains these setters back-to-back, observers could briefly see inconsistent entitlement state. Consider adding asetLicenseState(token: String, trialDate: Long, hasSubscription: Boolean)method to batch related updates atomically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt` around lines 206 - 228, Add a new atomic updater that writes the three entitlement values in one SharedPreferences transaction to avoid intermediate listener notifications: create a method setLicenseState(token: String, trialDate: Long, hasSubscription: Boolean) that uses defaultSharedPreferences.edit() (or the existing editor helper) to put LICENSE_TOKEN, TRIAL_EXPIRATION_DATE and HAS_RUNNING_SUBSCRIPTION in the same Editor and call apply() (or commit()) once; update callers that need to change multiple fields to use this new setLicenseState instead of calling setLicenseToken, setTrialExpirationDate, and setHasRunningSubscription separately so listeners only see the final consistent state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt`:
- Around line 169-192: The onSharedPreferenceChanged listener only triggers on
writes and always emits licenseToken(), so UIs can remain "trial active" after
trialExpirationDate() passes; update SharedPreferencesHandler to schedule a
time-based invalidation when adding listeners (in addLicenseChangedListeners) or
when handling TRIAL_EXPIRATION_DATE in onSharedPreferenceChanged: compute
trialExpirationDate(), if in the future schedule a one-shot task to call the
same notification path at that timestamp (or immediately notify with a full
license snapshot instead of only licenseToken()) so listeners get an update at
expiry; ensure you reference licenseToken(), trialExpirationDate(),
addLicenseChangedListeners, onSharedPreferenceChanged and TRIAL_EXPIRATION_DATE
when implementing the scheduled refresh or switching to emitting a full license
snapshot.
---
Nitpick comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.kt`:
- Around line 309-314: Capture the current selectedVault into a local val (e.g.,
val vault = selectedVault) before the null-check to allow safe non-null usage;
then use vault in the condition and in calls to
licenseEnforcer.hasWriteAccessForVault(vault) and
licenseEnforcer.ensureWriteAccessForVault(..., vault,
LicenseEnforcer.LockedAction.UPLOAD_FILES) and inside view?.let to avoid relying
on Kotlin smart-cast of the mutable `@JvmField` selectedVault.
In `@util/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt`:
- Around line 206-228: Add a new atomic updater that writes the three
entitlement values in one SharedPreferences transaction to avoid intermediate
listener notifications: create a method setLicenseState(token: String,
trialDate: Long, hasSubscription: Boolean) that uses
defaultSharedPreferences.edit() (or the existing editor helper) to put
LICENSE_TOKEN, TRIAL_EXPIRATION_DATE and HAS_RUNNING_SUBSCRIPTION in the same
Editor and call apply() (or commit()) once; update callers that need to change
multiple fields to use this new setLicenseState instead of calling
setLicenseToken, setTrialExpirationDate, and setHasRunningSubscription
separately so listeners only see the final consistent state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdcbd420-8a24-42a4-84e3-6e14f0b4aa70
📒 Files selected for processing (4)
presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.ktpresentation/src/main/res/values/strings.xmlutil/src/main/java/org/cryptomator/util/SharedPreferencesHandler.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- presentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.kt
- presentation/src/main/res/values/strings.xml
SailReal
requested changes
Mar 31, 2026
Member
SailReal
left a comment
SailReal
left a comment
There was a problem hiding this comment.
I've got as far as the VaultListActivity class. I'll pick up where I left off later or tomorrow.
presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| companion object { | ||
| const val EXTRA_LOCKED_ACTION = "lockedAction" |
Member
There was a problem hiding this comment.
We should create an LicenseCheckIntent class like the TextEditorIntent which provides us the extra.
Member
There was a problem hiding this comment.
No, as you see here, this is still in the code -.-
Member
Author
There was a problem hiding this comment.
Fixed in 897194e. LicenseEnforcer uses Intents.licenseCheckIntent().withLockedAction(), and onNewIntent now refreshes the reader via setIntent + Activities.setIntent so the companion constant is gone.
|
|
||
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| lockedAction = LicenseEnforcer.LockedAction.fromName(intent.getStringExtra(EXTRA_LOCKED_ACTION)) |
Member
There was a problem hiding this comment.
Member
There was a problem hiding this comment.
No, as you see here, this is still in the code -.-
Member
Author
There was a problem hiding this comment.
Fixed in 22d1854, same as above. (Don't be confused, I had to do another commit above so the commit reference is different. 😄)
Comment on lines
+75
to
+76
| finish() | ||
| return |
Member
Author
There was a problem hiding this comment.
Without finish(), pressing Back from WelcomeActivity would return to an empty VaultListActivity. The welcome flow re-launches VaultListActivity after completion, so finishing here prevents the stale instance from sitting in the back stack.
Member
There was a problem hiding this comment.
Wait what? Why do we not use (as every where else) the start activity with the callback-annotation pattern like e.g. in the CloudSettingsPresenter?
private fun startAuthentication(cloud: Cloud) {
requestActivityResult( //
ActivityResultCallbacks.onCloudAuthenticated(), //
Intents.authenticateCloudIntent() //
.withCloud(cloudModelMapper.toModel(cloud))
)
}
@Callback
fun onCloudAuthenticated(result: ActivityResult) {
view?.update(result.getSingleResult(CloudModel::class.java))
}
Then we do not need such hacks to finish the current activity while starting another one.
Member
Author
There was a problem hiding this comment.
Fixed in 22d1854. Moved the welcome flow check to VaultListPresenter.prepareView() using requestActivityResult + @Callback welcomeFlowCompleted. WelcomeActivity.openVaultList() now does setResult(RESULT_OK); finish().
Comment on lines
+34
to
+35
| private val isFreemiumFlavor: Boolean | ||
| get() = FlavorConfig.isFreemiumFlavor |
Member
There was a problem hiding this comment.
What is the benefit here? (We do the same in other parts of this PR)
Member
There was a problem hiding this comment.
No, this is not fixed, see e.g.
Member
Author
There was a problem hiding this comment.
Fixed in 22d1854. Inlined shouldShowLicenseSection and isFreemiumFlavor to use FlavorConfig directly.
| } | ||
|
|
||
| fun startTrial() { | ||
| if (sharedPreferencesHandler.trialExpirationDate() > 0) return |
Member
There was a problem hiding this comment.
In general I do not like ifs without brackets.
Member
There was a problem hiding this comment.
No, still a ton of ifs without brackets in the diff.
Member
Author
There was a problem hiding this comment.
Fixed in 22d1854. Added braces to all single-line if blocks in the diff.
Member
Author
| } | ||
|
|
||
| companion object { | ||
| private const val TRIAL_DURATION_MS = 30L * 24 * 60 * 60 * 1000 |
Member
There was a problem hiding this comment.
You need to "calculate" how long the trial actually lasts; perhaps we should rename the variable so we can see how long the trial actually lasts without having to "calculate" it.
Member
Author
There was a problem hiding this comment.
Fixed in b6d6867. Extracted TRIAL_DURATION_DAYS = 30L so the duration is immediately readable.
Member
There was a problem hiding this comment.
WTF, why we now have two variables and when I read TRIAL_DURATION_MS I still have no idea how long the trial lasts, why the heck do I need to calculate this now even with two variables
private const val TRIAL_DURATION_DAYS = 30L
private const val TRIAL_DURATION_MS = TRIAL_DURATION_DAYS * 24 * 60 * 60 * 1000
and in the code where val trialExpiration = System.currentTimeMillis() + TRIAL_DURATION_MS is used I need to jump to the declaration? oO are you serious? Why can we not just have one variable which the name says what it is doing?
Member
Author
There was a problem hiding this comment.
Fixed in 8728e06. Inlined TimeUnit.DAYS.toMillis(30) directly at the usage site, removed the constant entirely.
…ace, add accessibility attributes
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt (1)
83-104:⚠️ Potential issue | 🟠 MajorTrial expiry is still bypassable via device clock rollback.
These methods store only a wall-clock expiration and compare it against
System.currentTimeMillis(). After the trial expires, setting the device time backward makeshasActiveTrial()true again and restores write access.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt` around lines 83 - 104, The current trial logic (startTrial, hasActiveTrial, evaluateTrialState) trusts System.currentTimeMillis() and is vulnerable to device clock rollback; fix by recording a monotonic boot-relative timestamp when the trial starts and using it to evaluate expiration: in startTrial store both the wall-clock expiration (or start time) and SystemClock.elapsedRealtime() (e.g., add sharedPreferencesHandler.setTrialStartElapsed(elapsed) or setTrialStartTime + setTrialStartElapsed), then update hasActiveTrial and evaluateTrialState to compute a monotonic “now” as trialStartWallClock + (SystemClock.elapsedRealtime() - trialStartElapsed) (or compute remaining based on elapsed delta) instead of directly comparing System.currentTimeMillis(); update sharedPreferencesHandler to persist the new elapsed field and use that in TrialState formatting logic.
🧹 Nitpick comments (2)
data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt (2)
994-996: Assert both flavor branches in license-token migration test.The premium-flavor branch is currently unverified. Add an
elseassertion to ensure premium builds do not persist the DB license token.✅ Suggested assertion hardening
Assert.assertThat(sharedPreferencesHandler.hasCompletedWelcomeFlow(), CoreMatchers.`is`(true)) if (!FlavorConfig.isPremiumFlavor) { Assert.assertThat(sharedPreferencesHandler.licenseToken(), CoreMatchers.`is`(licenseToken)) + } else { + Assert.assertThat(sharedPreferencesHandler.licenseToken(), CoreMatchers.`is`("")) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt` around lines 994 - 996, Update the test in UpgradeDatabaseTest.kt to assert both flavor branches: keep the existing assertion when !FlavorConfig.isPremiumFlavor to expect sharedPreferencesHandler.licenseToken() equals licenseToken, and add an else branch for FlavorConfig.isPremiumFlavor that asserts the license token was not persisted (e.g., Assert.assertThat(sharedPreferencesHandler.licenseToken(), CoreMatchers.nullValue()) or the appropriate "empty" matcher used in this codebase). Ensure you reference FlavorConfig.isPremiumFlavor and sharedPreferencesHandler.licenseToken() so both branches are explicitly checked.
998-1005: Explicitly verifyLICENSE_TOKENcolumn removal.Both new tests verify preserved fields but do not assert the schema contract (column dropped). Add a direct check that
LICENSE_TOKENno longer exists.✅ Suggested schema assertion
Sql.query("UPDATE_CHECK_ENTITY").executeOn(db).use { it.moveToFirst() + Assert.assertThat(it.getColumnIndex("LICENSE_TOKEN"), CoreMatchers.`is`(-1)) Assert.assertThat(it.getString(it.getColumnIndex("RELEASE_NOTE")), CoreMatchers.`is`(releaseNote)) Assert.assertThat(it.getString(it.getColumnIndex("VERSION")), CoreMatchers.`is`(version))Also applies to: 1044-1050
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt` around lines 998 - 1005, The test that queries Sql.query("UPDATE_CHECK_ENTITY") currently asserts preserved columns but misses verifying the schema change; update the cursor assertions in UpgradeDatabaseTest.kt (the Sql.query("UPDATE_CHECK_ENTITY") blocks) to explicitly assert that cursor.getColumnIndex("LICENSE_TOKEN") returns -1 (or otherwise assert the LICENSE_TOKEN column is absent) after moveToFirst(), and add the same assertion in the other similar block (lines around 1044-1050) so the test fails if the LICENSE_TOKEN column still exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt`:
- Around line 959-1052: The tests only cover success paths; add a regression
test that forces the DB migration step in Upgrade13To14.internalApplyTo
(specifically the removeLicenseFromDb DB update) to fail and then asserts
SharedPreferences were not mutated. Implement this by creating a test similar to
upgrade13To14ExistingLicense/upgrade13To14NoLicense but inject or mock the DB to
throw (or make Sql.update/executeOn fail) when Upgrade13To14 calls
removeLicenseFromDb, run Upgrade13To14(sharedPreferencesHandler).applyTo(db,
13), and then assert sharedPreferencesHandler.hasCompletedWelcomeFlow() and
licenseToken() remain in their original states; if using test doubles,
reset/verify the DB failure occurred and clean up any state.
In
`@presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt`:
- Around line 554-558: The code currently strips write URI permission for
read-only opens but still unconditionally triggers the
OpenWritableFileNotification / suspendLock() flow; update the logic so that
after computing permissionFlags (using permissionFlags and
licenseEnforcer.hasWriteAccessForVault(view?.folder?.vault())), you only call
viewFileIntent.addFlags(...) and execute the OpenWritableFileNotification and
suspendLock() block when the licenseEnforcer indicates write access (i.e., when
you actually set FLAG_GRANT_WRITE_URI_PERMISSION); for read-only opens skip
invoking OpenWritableFileNotification/suspendLock() so the app remains locked
and no editing affordance is shown.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt`:
- Around line 88-91: The locked-action banner persists because onNewIntent() can
leave lockedAction set when the new Intent lacks the extra and setupUpsellView()
only shows tvInfoText for a non-null lockedAction; fix by explicitly clearing or
resetting lockedAction when the incoming Intent does not contain the
locked-action extra in onNewIntent(), then call setupUpsellView(); additionally,
update setupUpsellView() to hide binding.licenseContent.tvInfoText (set
visibility GONE/INVISIBLE and clear text) when lockedAction is null so the
banner is removed on plain launches (references: onNewIntent(), lockedAction,
setupUpsellView(), binding.licenseContent.tvInfoText).
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt`:
- Around line 112-113: The current visibility gate using
menu.findItem(R.id.action_save_changes).isVisible = hasWriteAccess() (in
TextEditorActivity) runs only during menu preparation and on initial display, so
changes to license/trial state can leave the editor editable and allow
saveChanges() to run; fix by performing an entitlement check at action time and
keeping UI in sync: (1) add an explicit hasWriteAccess() check inside the save
action handler (the method calling saveChanges()) and early-return/refuse if it
fails, and (2) observe license/trial state changes (via the existing
license/entitlement observer or lifecycle-aware listener) and call
invalidateOptionsMenu() or update
menu.findItem(R.id.action_save_changes).isVisible accordingly so the menu
visibility reflects runtime changes. Ensure references to TextEditorActivity,
saveChanges(), hasWriteAccess(), and menu.findItem(R.id.action_save_changes) are
updated accordingly.
- Around line 34-36: Change hasWriteAccess() so the intent-provided hub flag is
authoritative: if textEditorIntent.hubWriteAllowed() is non-null, return that
value; otherwise fall back to licenseEnforcer.hasWriteAccess(). Update the logic
in the hasWriteAccess() method (referencing licenseEnforcer.hasWriteAccess() and
textEditorIntent.hubWriteAllowed()) to avoid OR-ing both values.
---
Duplicate comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt`:
- Around line 83-104: The current trial logic (startTrial, hasActiveTrial,
evaluateTrialState) trusts System.currentTimeMillis() and is vulnerable to
device clock rollback; fix by recording a monotonic boot-relative timestamp when
the trial starts and using it to evaluate expiration: in startTrial store both
the wall-clock expiration (or start time) and SystemClock.elapsedRealtime()
(e.g., add sharedPreferencesHandler.setTrialStartElapsed(elapsed) or
setTrialStartTime + setTrialStartElapsed), then update hasActiveTrial and
evaluateTrialState to compute a monotonic “now” as trialStartWallClock +
(SystemClock.elapsedRealtime() - trialStartElapsed) (or compute remaining based
on elapsed delta) instead of directly comparing System.currentTimeMillis();
update sharedPreferencesHandler to persist the new elapsed field and use that in
TrialState formatting logic.
---
Nitpick comments:
In `@data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt`:
- Around line 994-996: Update the test in UpgradeDatabaseTest.kt to assert both
flavor branches: keep the existing assertion when !FlavorConfig.isPremiumFlavor
to expect sharedPreferencesHandler.licenseToken() equals licenseToken, and add
an else branch for FlavorConfig.isPremiumFlavor that asserts the license token
was not persisted (e.g.,
Assert.assertThat(sharedPreferencesHandler.licenseToken(),
CoreMatchers.nullValue()) or the appropriate "empty" matcher used in this
codebase). Ensure you reference FlavorConfig.isPremiumFlavor and
sharedPreferencesHandler.licenseToken() so both branches are explicitly checked.
- Around line 998-1005: The test that queries Sql.query("UPDATE_CHECK_ENTITY")
currently asserts preserved columns but misses verifying the schema change;
update the cursor assertions in UpgradeDatabaseTest.kt (the
Sql.query("UPDATE_CHECK_ENTITY") blocks) to explicitly assert that
cursor.getColumnIndex("LICENSE_TOKEN") returns -1 (or otherwise assert the
LICENSE_TOKEN column is absent) after moveToFirst(), and add the same assertion
in the other similar block (lines around 1044-1050) so the test fails if the
LICENSE_TOKEN column still exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 91cd5e3d-82d4-49c0-bd80-5982b4934cf5
📒 Files selected for processing (12)
data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.ktpresentation/src/main/java/org/cryptomator/presentation/intent/LicenseCheckIntent.javapresentation/src/main/java/org/cryptomator/presentation/intent/TextEditorIntent.javapresentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.ktpresentation/src/main/res/layout/dialog_trial_expired.xmlpresentation/src/main/res/layout/fragment_welcome_screen_lock.xmlpresentation/src/main/res/layout/view_license_check_content.xmlpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/presenter/VaultListPresenterFreemiumTest.kt
✅ Files skipped from review due to trivial changes (2)
- presentation/src/main/res/layout/dialog_trial_expired.xml
- presentation/src/main/res/layout/fragment_welcome_screen_lock.xml
🚧 Files skipped from review as they are similar to previous changes (3)
- presentation/src/testPlaystoreiap/java/org/cryptomator/presentation/presenter/VaultListPresenterFreemiumTest.kt
- presentation/src/main/res/layout/view_license_check_content.xml
- presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt
data/src/androidTest/java/org/cryptomator/data/db/UpgradeDatabaseTest.kt
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/presenter/BrowseFilesPresenter.kt
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt
Show resolved
Hide resolved
presentation/src/main/java/org/cryptomator/presentation/ui/activity/TextEditorActivity.kt
Show resolved
Hide resolved
…n-nullable Kotlin params
…low callback, Hub write access
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
presentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.kt (1)
339-368:⚠️ Potential issue | 🟠 MajorRe-evaluate the license section on resume.
This fragment only refreshes the freemium UI through
licenseChangeListenerwhile it is already visible. If the Play purchase sheet returns, or the trial/license state changes while Settings is paused, the rows stay stale until another pref write happens. CallsetupLicense()inonResume()as well.💡 Suggested fix
override fun onResume() { super.onResume() + setupLicense() (findPreference(SEND_ERROR_REPORT_ITEM_KEY) as Preference?)?.onPreferenceClickListener = sendErrorReportClickListener (findPreference(LRU_CACHE_CLEAR_ITEM_KEY) as Preference?)?.onPreferenceClickListener = clearCacheClickListener (findPreference(CLEAR_TRUSTED_HUB_HOSTS) as Preference?)?.onPreferenceClickListener = clearTrustedHubHostsClickListener @@ (findPreference(SharedPreferencesHandler.PHOTO_UPLOAD_VAULT) as Preference?)?.intent = Intent(context, AutoUploadChooseVaultActivity::class.java) (findPreference(SharedPreferencesHandler.LICENSES_ACTIVITY) as Preference?)?.intent = Intent(context, LicensesActivity::class.java) sharedPreferencesHandler.addLicenseChangedListeners(licenseChangeListener) }Based on learnings, license/trial invalidation in this branch intentionally combines an
onResume()re-evaluation with live preference listeners; this fragment currently only implements the latter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.kt` around lines 339 - 368, The fragment doesn't re-evaluate license/trial state on resume, causing stale UI after an external purchase; in onResume() call setupLicense() (the method that updates freemium UI) before/after adding the licenseChangeListener so the license rows are refreshed when the fragment returns to foreground, and keep the existing sharedPreferencesHandler.addLicenseChangedListeners(licenseChangeListener) intact to preserve live updates.
🧹 Nitpick comments (1)
presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt (1)
243-245: Consider guarding the implicit intent againstActivityNotFoundException.If no app can handle
ACTION_VIEWfor the URL (rare but possible on restricted devices),startActivity()will throw. A try-catch orresolveActivity()check would add robustness.🛡️ Optional defensive wrapper
override fun onOpenLicenseLink() { - startActivity(Intent(Intent.ACTION_VIEW, Uri.parse("https://cryptomator.org/android/"))) + try { + startActivity(Intent(Intent.ACTION_VIEW, Uri.parse("https://cryptomator.org/android/"))) + } catch (e: android.content.ActivityNotFoundException) { + // No browser available – silently ignore or show a toast + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt` around lines 243 - 245, The onOpenLicenseLink method in WelcomeActivity currently calls startActivity with an implicit ACTION_VIEW intent without checking for a handler; wrap this call in a guard to avoid ActivityNotFoundException by either (a) checking packageManager.resolveActivity(intent, 0) and only calling startActivity when non-null or (b) catching ActivityNotFoundException around startActivity and handling it (e.g., show a toast or log). Update the method name onOpenLicenseLink in WelcomeActivity accordingly to perform one of these defensive checks before launching the intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt`:
- Around line 146-150: Currently launchPurchaseFlow(ActivityWeakReference,
productId) silently returns when iapBillingServiceBinder is null; change it to
buffer the request and/or expose readiness: add a nullable pendingPurchase:
Pair<WeakReference<Activity>, String>? (or a small queue) and in
launchPurchaseFlow store the activity/productId into pendingPurchase when
iapBillingServiceBinder == null, then attempt to start the purchase immediately
if binder is present. Also add an isBillingReady(): Boolean (or a LiveData/Flow)
that returns iapBillingServiceBinder != null so callers can disable the CTA
until ready, and ensure you consume pendingPurchase from the code path that
sets/updates iapBillingServiceBinder (e.g., in the binder connect callback or
setter) to call startPurchaseFlow and clear the pending field.
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt`:
- Around line 159-167: The current ensureWriteAccessForVault(vault, action)
returns after showing the hub-inactive toast and never calls ensureWriteAccess,
preventing upsell paths; change the logic so that if vault?.isHubVault == true
and hasWriteAccessForVault(vault) is false you still invoke
ensureWriteAccess(activity, action) (and only show the
read_only_reason_hub_inactive Toast if ensureWriteAccess returns false),
otherwise return true when hasWriteAccessForVault is true—this preserves local
license/trial unlocking hub vaults and still shows the hub-specific toast when
write access remains denied.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt`:
- Around line 129-133: In onNewIntent(Intent) the incoming intent must replace
the Activity's current intent to avoid stale data on configuration changes; add
a call to setIntent(intent) at the start of onNewIntent so the Activity (and
injected values like licenseCheckIntent created via Activities.setIntent())
reflect the new Intent before you update lockedAction, call setupUpsellView(),
and validate(intent).
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`:
- Around line 72-79: In afterTextChanged in WelcomeLicenseFragment, when the
input is cleared you currently remove the debounceRunnable but do not reset host
state; update the method so that when text is null or blank you also call
listener?.onLicenseTextChanged(null) (after cancelling any pending debounce on
debounceHandler/debounceRunnable) to ensure state derived from the previous
value is cleared; keep the existing debounce behavior (postDelayed with
DEBOUNCE_DELAY_MS and Runnable invoking listener?.onLicenseTextChanged(text))
for non-blank input.
---
Outside diff comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.kt`:
- Around line 339-368: The fragment doesn't re-evaluate license/trial state on
resume, causing stale UI after an external purchase; in onResume() call
setupLicense() (the method that updates freemium UI) before/after adding the
licenseChangeListener so the license rows are refreshed when the fragment
returns to foreground, and keep the existing
sharedPreferencesHandler.addLicenseChangedListeners(licenseChangeListener)
intact to preserve live updates.
---
Nitpick comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt`:
- Around line 243-245: The onOpenLicenseLink method in WelcomeActivity currently
calls startActivity with an implicit ACTION_VIEW intent without checking for a
handler; wrap this call in a guard to avoid ActivityNotFoundException by either
(a) checking packageManager.resolveActivity(intent, 0) and only calling
startActivity when non-null or (b) catching ActivityNotFoundException around
startActivity and handling it (e.g., show a toast or log). Update the method
name onOpenLicenseLink in WelcomeActivity accordingly to perform one of these
defensive checks before launching the intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c83c6f00-7188-42d4-9166-10b9994c055a
📒 Files selected for processing (12)
presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.ktpresentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.ktpresentation/src/main/java/org/cryptomator/presentation/presenter/VaultListPresenter.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/VaultListActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/SettingsFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeNotificationsFragment.ktpresentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeScreenLockFragment.ktpresentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.ktpresentation/src/testPlaystoreiap/java/org/cryptomator/presentation/presenter/VaultListPresenterFreemiumTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- presentation/src/main/java/org/cryptomator/presentation/ui/activity/VaultListActivity.kt
- presentation/src/test/java/org/cryptomator/presentation/licensing/LicenseEnforcerTest.kt
Comment on lines
+146
to
+150
| fun launchPurchaseFlow(activity: WeakReference<Activity>, productId: String) { | ||
| if (FlavorConfig.isFreemiumFlavor) { | ||
| iapBillingServiceBinder?.startPurchaseFlow(activity, productId) | ||
| } | ||
| } |
There was a problem hiding this comment.
Don't silently drop purchase taps before billing connects.
launchPurchaseFlow() is a no-op while iapBillingServiceBinder is null. SettingsFragment now exposes an immediately clickable upgrade row, so a tap during cold start or after a binder reconnect can do nothing with no retry or feedback. Buffer the request, or return a readiness signal so callers can keep the CTA disabled until billing is ready.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@presentation/src/main/java/org/cryptomator/presentation/CryptomatorApp.kt`
around lines 146 - 150, Currently launchPurchaseFlow(ActivityWeakReference,
productId) silently returns when iapBillingServiceBinder is null; change it to
buffer the request and/or expose readiness: add a nullable pendingPurchase:
Pair<WeakReference<Activity>, String>? (or a small queue) and in
launchPurchaseFlow store the activity/productId into pendingPurchase when
iapBillingServiceBinder == null, then attempt to start the purchase immediately
if binder is present. Also add an isBillingReady(): Boolean (or a LiveData/Flow)
that returns iapBillingServiceBinder != null so callers can disable the CTA
until ready, and ensure you consume pendingPurchase from the code path that
sets/updates iapBillingServiceBinder (e.g., in the binder connect callback or
setter) to call startPurchaseFlow and clear the pending field.
Comment on lines
+159
to
+167
| fun ensureWriteAccessForVault(activity: Activity, vault: VaultModel?, action: LockedAction): Boolean { | ||
| if (vault?.isHubVault == true) { | ||
| if (hasWriteAccessForVault(vault)) { | ||
| return true | ||
| } | ||
| Toast.makeText(activity, R.string.read_only_reason_hub_inactive, Toast.LENGTH_LONG).show() | ||
| return false | ||
| } | ||
| return ensureWriteAccess(activity, action) |
There was a problem hiding this comment.
Hub write locks currently dead-end instead of upselling.
When a Hub vault lacks both entitlements, this branch returns after the toast and never reaches ensureWriteAccess(...). Upload/create/rename/delete actions from Hub vaults then have no path to the purchase/trial screen even though local write access is supposed to unlock Hub vaults too.
Based on learnings, a local Cryptomator license or active trial grants write access to Hub vaults too; the Hub subscription path is additive, not exclusive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt`
around lines 159 - 167, The current ensureWriteAccessForVault(vault, action)
returns after showing the hub-inactive toast and never calls ensureWriteAccess,
preventing upsell paths; change the logic so that if vault?.isHubVault == true
and hasWriteAccessForVault(vault) is false you still invoke
ensureWriteAccess(activity, action) (and only show the
read_only_reason_hub_inactive Toast if ensureWriteAccess returns false),
otherwise return true when hasWriteAccessForVault is true—this preserves local
license/trial unlocking hub vaults and still shows the hub-specific toast when
write access remains denied.
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt
Show resolved
Hide resolved
Comment on lines
+72
to
+79
| override fun afterTextChanged(s: Editable?) { | ||
| debounceRunnable?.let { debounceHandler.removeCallbacks(it) } | ||
| val text = s?.toString() | ||
| if (!text.isNullOrBlank()) { | ||
| val runnable = Runnable { listener?.onLicenseTextChanged(text) } | ||
| debounceRunnable = runnable | ||
| debounceHandler.postDelayed(runnable, DEBOUNCE_DELAY_MS) | ||
| } |
There was a problem hiding this comment.
Reset host state when the field is cleared.
Blank input cancels the debounce but never calls onLicenseTextChanged(null), so state derived from the previous value can stay latched. The nullable listener already provides a reset path.
💡 Suggested fix
override fun afterTextChanged(s: Editable?) {
debounceRunnable?.let { debounceHandler.removeCallbacks(it) }
+ debounceRunnable = null
val text = s?.toString()
if (!text.isNullOrBlank()) {
val runnable = Runnable { listener?.onLicenseTextChanged(text) }
debounceRunnable = runnable
debounceHandler.postDelayed(runnable, DEBOUNCE_DELAY_MS)
+ } else {
+ listener?.onLicenseTextChanged(null)
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/fragment/WelcomeLicenseFragment.kt`
around lines 72 - 79, In afterTextChanged in WelcomeLicenseFragment, when the
input is cleared you currently remove the debounceRunnable but do not reset host
state; update the method so that when text is null or blank you also call
listener?.onLicenseTextChanged(null) (after cancelling any pending debounce on
debounceHandler/debounceRunnable) to ensure state derived from the previous
value is cleared; keep the existing debounce behavior (postDelayed with
DEBOUNCE_DELAY_MS and Runnable invoking listener?.onLicenseTextChanged(text))
for non-blank input.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt`:
- Around line 144-148: The unsafe cast arises in
LicenseEnforcer.ensureWriteAccess where you call
Intents.licenseCheckIntent().build(activity as ContextHolder); change the
ensureWriteAccess parameter from Activity to ContextHolder so the build() call
receives a real ContextHolder (update all callers such as VaultListActivity and
BrowseFilesActivity to pass their ContextHolder-compatible instance), and remove
the cast; alternatively, if you prefer a broader change, make BaseActivity
implement ContextHolder so existing callers remain valid—pick one approach and
update the function signature or BaseActivity accordingly and adjust call sites.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19501e59-0f3f-45ac-a4ac-c8acb983ffe5
📒 Files selected for processing (1)
presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt
Comment on lines
+144
to
+148
| val intent = Intents.licenseCheckIntent() | ||
| .withLockedAction(action.name) | ||
| .build(activity as ContextHolder) | ||
| intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_ACTIVITY_CLEAR_TOP | ||
| activity.startActivity(intent) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all activities calling ensureWriteAccess implement ContextHolder
# Find all usages of ensureWriteAccess
echo "=== Usages of ensureWriteAccess ==="
rg -n 'ensureWriteAccess\(' --type=kotlin -C2
echo ""
echo "=== Check if BaseActivity implements ContextHolder ==="
ast-grep --pattern 'class BaseActivity $$$: $$$ContextHolder$$$'
echo ""
echo "=== Find ContextHolder implementations ==="
rg -n 'implements ContextHolder|: ContextHolder|, ContextHolder' --type=kotlin --type=javaRepository: cryptomator/android
Length of output: 2072
🏁 Script executed:
#!/bin/bash
# Check VaultListActivity inheritance
echo "=== VaultListActivity class definition ==="
rg -n 'class VaultListActivity' --type=kotlin -A5
echo ""
echo "=== Check the actual lines 144-148 in LicenseEnforcer.kt ==="
sed -n '144,148p' presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt
echo ""
echo "=== Look for all callers of ensureWriteAccess comprehensively ==="
rg -n 'ensureWriteAccess' --type=kotlin --type=java
echo ""
echo "=== Check if there's a BaseActivity or common Activity base class ==="
fd -e kt -e java | xargs grep -l 'class.*Activity.*extends\|open class.*Activity' | head -5
echo ""
echo "=== Check ContextHolder interface definition ==="
rg -n 'interface ContextHolder' --type=kotlin -A3Repository: cryptomator/android
Length of output: 7086
🏁 Script executed:
#!/bin/bash
# Find BaseActivity definition
echo "=== BaseActivity class definition ==="
rg -n 'open class BaseActivity|class BaseActivity' --type=kotlin -A10 | head -30
echo ""
echo "=== ActivityHolder interface definition ==="
rg -n 'interface ActivityHolder' --type=kotlin -A5
echo ""
echo "=== Check if BaseActivity is in a specific file ==="
fd -e kt -e java | xargs grep -l 'class BaseActivity' 2>/dev/null
echo ""
echo "=== SharedFilesPresenter - what is v.activity()? ==="
sed -n '305,315p' presentation/src/main/java/org/cryptomator/presentation/presenter/SharedFilesPresenter.ktRepository: cryptomator/android
Length of output: 2683
Fix the unsafe cast on line 146.
The cast activity as ContextHolder will throw ClassCastException at runtime. VaultListActivity and BrowseFilesActivity (both callers of ensureWriteAccess()) extend BaseActivity, which does not implement ContextHolder.
Change the parameter type from Activity to ContextHolder, or require BaseActivity to implement ContextHolder so all activities can safely satisfy this requirement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseEnforcer.kt`
around lines 144 - 148, The unsafe cast arises in
LicenseEnforcer.ensureWriteAccess where you call
Intents.licenseCheckIntent().build(activity as ContextHolder); change the
ensureWriteAccess parameter from Activity to ContextHolder so the build() call
receives a real ContextHolder (update all callers such as VaultListActivity and
BrowseFilesActivity to pass their ContextHolder-compatible instance), and remove
the cast; alternatively, if you prefer a broader change, make BaseActivity
implement ContextHolder so existing callers remain valid—pick one approach and
update the function signature or BaseActivity accordingly and adjust call sites.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt`:
- Around line 189-197: The method updateNotificationPermissionState currently
returns early when needsNotificationPermission() is false, so pre-Android T
devices never update the notifications fragment UI; change it to treat pre-T as
already granted by calling
pagerAdapter.notificationsFragment?.updatePermissionState(true) (or use the
grantedOverride/hasNotificationPermission logic) before returning. In other
words, inside updateNotificationPermissionState (and keeping the
this::pagerAdapter.isInitialized guard), replace the early return on
!needsNotificationPermission() with a call to updatePermissionState(true) so the
fragment reflects granted permission on pre-T devices.
- Around line 149-155: The page-change callback only refreshes the License page;
to avoid stale UI on Notifications and ScreenLock pages, update the
onPageSelected in WelcomeActivity (the
binding.welcomePager.registerOnPageChangeCallback override) to also call the
appropriate refresh methods when those pages are selected: invoke
orchestrator.updatePermissionState() when pages[position] is
FragmentPage.Notifications (or equivalent enum) and invoke
orchestrator.updateScreenLockState() when pages[position] is
FragmentPage.ScreenLock, while keeping the existing
updateNavigationButtons(position) and the existing orchestrator.updateState()
for the License page.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a700a8be-5218-4c14-94c0-1a2abb8817d1
📒 Files selected for processing (1)
presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt
Comment on lines
+149
to
+155
| binding.welcomePager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() { | ||
| override fun onPageSelected(position: Int) { | ||
| updateNavigationButtons(position) | ||
| if (FlavorConfig.isFreemiumFlavor && pages[position] is FragmentPage.License) { | ||
| orchestrator.updateState() | ||
| } | ||
| } |
There was a problem hiding this comment.
Refresh deferred page state when the user lands on it.
Only the license page is refreshed from onPageSelected(). The notifications and screen-lock fragments depend on updatePermissionState() / updateScreenLockState() callbacks, so a device with notifications already granted or a secure lock screen can show stale controls the first time those pages open.
🛠️ Possible fix
binding.welcomePager.registerOnPageChangeCallback(object : ViewPager2.OnPageChangeCallback() {
override fun onPageSelected(position: Int) {
updateNavigationButtons(position)
- if (FlavorConfig.isFreemiumFlavor && pages[position] is FragmentPage.License) {
- orchestrator.updateState()
- }
+ when (pages[position]) {
+ is FragmentPage.License -> {
+ if (FlavorConfig.isFreemiumFlavor) {
+ orchestrator.updateState()
+ }
+ }
+ is FragmentPage.Notifications -> updateNotificationPermissionState()
+ is FragmentPage.ScreenLock -> updateScreenLockState()
+ is FragmentPage.Intro -> Unit
+ }
}
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt`
around lines 149 - 155, The page-change callback only refreshes the License
page; to avoid stale UI on Notifications and ScreenLock pages, update the
onPageSelected in WelcomeActivity (the
binding.welcomePager.registerOnPageChangeCallback override) to also call the
appropriate refresh methods when those pages are selected: invoke
orchestrator.updatePermissionState() when pages[position] is
FragmentPage.Notifications (or equivalent enum) and invoke
orchestrator.updateScreenLockState() when pages[position] is
FragmentPage.ScreenLock, while keeping the existing
updateNavigationButtons(position) and the existing orchestrator.updateState()
for the License page.
Comment on lines
+189
to
+197
| private fun updateNotificationPermissionState(grantedOverride: Boolean? = null) { | ||
| if (!this::pagerAdapter.isInitialized) { | ||
| return | ||
| } | ||
| if (!needsNotificationPermission()) { | ||
| return | ||
| } | ||
| val granted = grantedOverride ?: hasNotificationPermission() | ||
| pagerAdapter.notificationsFragment?.updatePermissionState(granted) |
There was a problem hiding this comment.
Treat pre-T devices as already granted.
Line 193 returns before the fragment is updated, so on Android 12L and below the notifications page never reflects success—even when WelcomePresenter.requestNotificationPermission() calls back with true. The button stays enabled forever on those devices.
🛠️ Possible fix
private fun updateNotificationPermissionState(grantedOverride: Boolean? = null) {
if (!this::pagerAdapter.isInitialized) {
return
}
- if (!needsNotificationPermission()) {
- return
- }
- val granted = grantedOverride ?: hasNotificationPermission()
+ val granted = grantedOverride ?: if (needsNotificationPermission()) {
+ hasNotificationPermission()
+ } else {
+ true
+ }
pagerAdapter.notificationsFragment?.updatePermissionState(granted)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/WelcomeActivity.kt`
around lines 189 - 197, The method updateNotificationPermissionState currently
returns early when needsNotificationPermission() is false, so pre-Android T
devices never update the notifications fragment UI; change it to treat pre-T as
already granted by calling
pagerAdapter.notificationsFragment?.updatePermissionState(true) (or use the
grantedOverride/hasNotificationPermission logic) before returning. In other
words, inside updateNotificationPermissionState (and keeping the
this::pagerAdapter.isInitialized guard), replace the early return on
!needsNotificationPermission() with a call to updatePermissionState(true) so the
fragment reflects granted permission on pre-T devices.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt`:
- Around line 104-123: The license entry UI is being revealed for IAP builds
even though the submit button is only wired in the non-freemium flow; update
BaseLicensePresenter.validate() to guard the call to showOrUpdateLicenseEntry()
so it only runs for non-freemium builds (or when a BuildConfig flag indicates
license entry is supported), or alternately change LicenseCheckActivity to only
call setupLicenseEntryView()/make btnPurchase visible when not in the
playstoreiap/freemium flavor; reference BaseLicensePresenter.validate(),
showOrUpdateLicenseEntry(), LicenseCheckActivity.setupIapView(),
LicenseCheckActivity.setupLicenseEntryView(), btnPurchase and onLicenseSubmit()
when making the conditional check.
- Around line 97-116: onNewIntent() currently calls setupUpsellView() which
rebuilds the initial IAP/license layout but does not reapply the current
license/trial state; immediately after the call to setupUpsellView() (and
likewise after any call that rebuilds the UI such as setupIapView() or
setupLicenseEntryView()), invoke orchestrator.updateState() so the orchestrator
reapplies the current state to the rebuilt views; update onNewIntent() to call
orchestrator.updateState() right after
setupUpsellView()/setupIapView()/setupLicenseEntryView() to ensure UI reflects
current license/trial state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4067f5b-eaee-4369-b044-c9b08cce4237
📒 Files selected for processing (1)
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt
Comment on lines
+97
to
116
| if (FlavorConfig.isFreemiumFlavor) { | ||
| setupIapView() | ||
| } else { | ||
| setupLicenseEntryView() | ||
| } | ||
| } | ||
|
|
||
| fun validate(intent: Intent) { | ||
| val data: Uri? = intent.data | ||
| licenseCheckPresenter.validate(data) | ||
| private fun setupIapView() { | ||
| supportActionBar?.title = getString(R.string.screen_license_check_title_full_version) | ||
| licenseContentViewBinder.bindInitialIapLayout() | ||
| licenseContentViewBinder.bindLegalLinks() | ||
| licenseContentViewBinder.bindPurchaseButtons( | ||
| activity = this, | ||
| app = application as CryptomatorApp, | ||
| onTrialClicked = { | ||
| licenseEnforcer.startTrial() | ||
| orchestrator.updateState() | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the initial IAP bind resets stateful UI and whether `updateState()` is the immediate rebind path.
rg -n -C3 'fun bindInitialIapLayout|fun bindPurchaseState|fun bindTrialState' \
presentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.kt
rg -n -C3 'fun onResume|fun onPause|fun updateState' \
presentation/src/main/java/org/cryptomator/presentation/licensing/LicenseStateOrchestrator.ktRepository: cryptomator/android
Length of output: 1711
🏁 Script executed:
# Fetch the onNewIntent method and surrounding context
rg -n -A 20 'fun onNewIntent' \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktRepository: cryptomator/android
Length of output: 808
🏁 Script executed:
# Fetch setupUpsellView() implementation to see what paths it handles
rg -n -A 15 'fun setupUpsellView' \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktRepository: cryptomator/android
Length of output: 735
🏁 Script executed:
# Get the complete setupUpsellView() method
sed -n '82,116p' presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktRepository: cryptomator/android
Length of output: 1169
🏁 Script executed:
# Get setupLicenseEntryView() to see if non-freemium path has the same issue
rg -n -A 15 'fun setupLicenseEntryView' \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktRepository: cryptomator/android
Length of output: 906
🏁 Script executed:
# Check what bindInitialLicenseEntryLayout() does
rg -n -A 10 'fun bindInitialLicenseEntryLayout' \
presentation/src/main/java/org/cryptomator/presentation/ui/layout/LicenseContentViewBinder.ktRepository: cryptomator/android
Length of output: 565
🏁 Script executed:
# Check if onCreate calls orchestrator.updateState() or if there's another mechanism
rg -n -B5 -A 15 'fun onCreate' \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt | head -40Repository: cryptomator/android
Length of output: 776
🏁 Script executed:
# Check what validate() and licenseCheckPresenter.validate() do
rg -n -A 5 'fun validate' \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt
# Also check the presenter's validate method
find . -name '*LicenseCheckPresenter*' -type fRepository: cryptomator/android
Length of output: 351
🏁 Script executed:
# Check the LicenseCheckPresenter.validate() method
rg -n -A 20 'fun validate' \
presentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.ktRepository: cryptomator/android
Length of output: 45
🏁 Script executed:
# Check what's in the LicenseCheckPresenter file
cat -n presentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.kt | head -100Repository: cryptomator/android
Length of output: 786
🏁 Script executed:
# Check BaseLicensePresenter for validate method
find . -name 'BaseLicensePresenter*' -type fRepository: cryptomator/android
Length of output: 154
🏁 Script executed:
# Check BaseLicensePresenter for validate method
rg -n -A 15 'fun validate' \
presentation/src/main/java/org/cryptomator/presentation/presenter/BaseLicensePresenter.ktRepository: cryptomator/android
Length of output: 933
Add state reapplication after rebuilding the initial IAP layout in onNewIntent().
When onNewIntent() calls setupUpsellView() (line 134), the initial IAP layout is reset to its default visibility state. Since the activity is already resumed, onResume() is not called again, which means orchestrator.updateState() does not run to reapply the current license/trial state. The UI will remain in the initial state until a preference change or the next pause/resume cycle. Add orchestrator.updateState() immediately after setupUpsellView() at line 134 to reapply the current state.
Also applies to: 129-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt`
around lines 97 - 116, onNewIntent() currently calls setupUpsellView() which
rebuilds the initial IAP/license layout but does not reapply the current
license/trial state; immediately after the call to setupUpsellView() (and
likewise after any call that rebuilds the UI such as setupIapView() or
setupLicenseEntryView()), invoke orchestrator.updateState() so the orchestrator
reapplies the current state to the rebuilt views; update onNewIntent() to call
orchestrator.updateState() right after
setupUpsellView()/setupIapView()/setupLicenseEntryView() to ensure UI reflects
current license/trial state.
Comment on lines
+104
to
+123
| private fun setupIapView() { | ||
| supportActionBar?.title = getString(R.string.screen_license_check_title_full_version) | ||
| licenseContentViewBinder.bindInitialIapLayout() | ||
| licenseContentViewBinder.bindLegalLinks() | ||
| licenseContentViewBinder.bindPurchaseButtons( | ||
| activity = this, | ||
| app = application as CryptomatorApp, | ||
| onTrialClicked = { | ||
| licenseEnforcer.startTrial() | ||
| orchestrator.updateState() | ||
| } | ||
| ) | ||
| } | ||
|
|
||
| override fun showOrUpdateLicenseDialog(license: String) { | ||
| showDialog(UpdateLicenseDialog.newInstance(license)) | ||
| private fun setupLicenseEntryView() { | ||
| supportActionBar?.title = getString(R.string.screen_license_check_title) | ||
| licenseContentViewBinder.bindInitialLicenseEntryLayout() | ||
| binding.licenseContent.btnPurchase.visibility = View.VISIBLE | ||
| binding.licenseContent.btnPurchase.text = getString(R.string.dialog_enter_license_ok_button) | ||
| binding.licenseContent.btnPurchase.setOnClickListener { onLicenseSubmit() } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `validate(data)` can reach `showOrUpdateLicenseEntry()` without a freemium guard.
rg -n -C4 'fun validate\(|showOrUpdateLicenseEntry\(' \
presentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.kt \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/view/UpdateLicenseView.kt
rg -n -C4 'FlavorConfig\.isFreemiumFlavor|btnPurchase\.setOnClickListener|onLicenseSubmit' \
presentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.kt \
presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.ktRepository: cryptomator/android
Length of output: 7477
🏁 Script executed:
#!/bin/bash
# Find and inspect LicenseCheckPresenter.validate() method
rg -n -A 20 'fun validate\(data: Uri' \
presentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.kt
# Also check if there are any flavor guards around showOrUpdateLicenseEntry calls
rg -n -B 3 -A 3 'showOrUpdateLicenseEntry' \
presentation/src/main/java/org/cryptomator/presentation/presenter/LicenseCheckPresenter.ktRepository: cryptomator/android
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Find the LicenseCheckPresenter file
fd -type f -name '*LicenseCheckPresenter*'
# Also search for mentions of showOrUpdateLicenseEntry across the codebase
rg -n 'showOrUpdateLicenseEntry' --type kotlinRepository: cryptomator/android
Length of output: 832
🏁 Script executed:
#!/bin/bash
# Find LicenseCheckPresenter and BaseLicensePresenter files
fd -type f -name '*.kt' | grep -i 'licensecheckpresenter\|baselicensepresenter'
# Read BaseLicensePresenter context around line 25
cat -n presentation/src/main/java/org/cryptomator/presentation/presenter/BaseLicensePresenter.kt | sed -n '1,40p'Repository: cryptomator/android
Length of output: 1871
Prevent deep-linked licenses from showing in the IAP flavor without a working submit action.
In BaseLicensePresenter.validate() (line 25), showOrUpdateLicenseEntry() is called unconditionally, but btnPurchase only wires to onLicenseSubmit() in the non-freemium branch (setupLicenseEntryView(), line 123). If a deep-linked license reaches the activity in the playstoreiap flavor, the license entry field becomes visible with the button still bound to the trial flow from setupIapView(), leaving no way to submit the license key.
Add a flavor guard to BaseLicensePresenter.validate() or conditionally reveal the license entry field only in non-freemium builds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@presentation/src/main/java/org/cryptomator/presentation/ui/activity/LicenseCheckActivity.kt`
around lines 104 - 123, The license entry UI is being revealed for IAP builds
even though the submit button is only wired in the non-freemium flow; update
BaseLicensePresenter.validate() to guard the call to showOrUpdateLicenseEntry()
so it only runs for non-freemium builds (or when a BuildConfig flag indicates
license entry is supported), or alternately change LicenseCheckActivity to only
call setupLicenseEntryView()/make btnPurchase visible when not in the
playstoreiap/freemium flavor; reference BaseLicensePresenter.validate(),
showOrUpdateLicenseEntry(), LicenseCheckActivity.setupIapView(),
LicenseCheckActivity.setupLicenseEntryView(), btnPurchase and onLicenseSubmit()
when making the conditional check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces a freemium distribution model via a new
playstoreiapbuild flavor. Free users get read-only vault access with a 30-day full-featured trial on first install. Write operations (create vault, upload, create folder/text file, rename, move, delete) are gated behindLicenseEnforcerand redirect to a purchase screen when the trial expires.Key changes:
playstoreiapflavor across all modules, with a realIapBillingServicebacked by Google Play Billing Library 8.2.1. Other flavors get a no-op stub.LicenseEnforcerevaluates write access from three sources: paid license token, active subscription, or active trial. Hub vaults use their own subscription status.PurchaseManagerhandles purchase verification, token persistence, and acknowledgement with TOCTOU-safe state transitions.LicenseStateOrchestratordrives reactive UI updates viaSharedPreferenceschange listeners, used by bothWelcomeActivityandLicenseCheckActivity.LicenseContentViewBinderextracts shared purchase/trial UI into a reusable binder for the onboarding welcome flow and the standalone IAP screen.WelcomeActivitywith a ViewPager2-based onboarding flow (intro, license, notifications, screen lock) that replaces the old dialog-based first-launch experience for IAP builds.BrowseFilesPresenter,SharedFilesPresenter,VaultListPresenter, andTextEditorActivity, plus read-only banner and per-action toast messages.Vaultmodel extended withhubSubscriptionState.LicenseEnforcerTestcovering trial lifecycle, paid license, subscription, Hub vaults, and flavor-based bypass. Additional tests forPurchaseManager,ProductInfo,DoLicenseCheck,UnlockHubVault, and vault list freemium behavior.LicenseCheckIntent,TextEditorIntent) for type-safe intent extras, replacing raw companion constants.wrap_contentfix for trial-expired dialog scroll.State Machine
stateDiagram-v2 [*] --> ReadOnly: fresh install (IAP flavor) [*] --> FullAccess: premium flavor ReadOnly --> Trial: startTrial() Trial --> FullAccess: purchase / subscribe Trial --> ReadOnly: trial expires ReadOnly --> FullAccess: purchase / subscribe FullAccess --> ReadOnly: subscription lapses (no lifetime license)Test Plan
:domain:testDebugUnitTest,:presentation:testPlaystoreiapDebugUnitTestassemblePlaystoreDebug,assemblePlaystoreiapDebug,assembleApkstoreDebug,assembleFdroidDebug,assembleLiteDebugplaystoreiap: onboarding flow shows license step, trial starts, write ops work