Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG]: Platform parameter syncing races against SplashActivity #5676

Open
BenHenning opened this issue Jan 29, 2025 · 0 comments
Open

[BUG]: Platform parameter syncing races against SplashActivity #5676

BenHenning opened this issue Jan 29, 2025 · 0 comments
Labels
bug End user-perceivable behaviors which are not desirable. Impact: High High perceived user impact (breaks a critical feature or blocks a release). triage needed

Comments

@BenHenning
Copy link
Member

Describe the bug

Currently, platform parameter syncing relies on a application-scope singleton to be fully initialized before its properties will be correctly retrieved from the local data store. See:

platformParameterSingleton.setPlatformParameterMap(platformParameterMap)

And

return platformParameterSingleton.getBooleanPlatformParameter(DOWNLOADS_SUPPORT)
?: PlatformParameterValue.createDefaultParameter(ENABLE_DOWNLOADS_SUPPORT_DEFAULT_VALUE)

Yet, the singleton gracefully fails:

if (platformParameterMap.isEmpty()) return null
val parameter = platformParameterMap[platformParameterName] ?: return null
if (!parameter.valueTypeCase.equals(PlatformParameter.ValueTypeCase.BOOLEAN)) return null

And SplashActivityPresenter relies on app-critical features during construction:

@EnableAppAndOsDeprecation
private val enableAppAndOsDeprecation: PlatformParameterValue<Boolean>,

I have not tested this, but I strongly suspect that if the quite failure logic were to be updated above to fail with an exception that splash activity would always fail (which means it will never have a synced state). This, in turn, means that OS and app deprecation will never work correctly in the current implementation.

Steps To Reproduce

I haven't devised actual steps to repro the issue.

Expected Behavior

State needs to be synced such that even splash activity can have platform parameters upon starting.

Screenshots/Videos

No response

What device/emulator are you using?

No response

Which Android version is your device/emulator running?

No response

Which version of the Oppia Android app are you using?

No response

Additional Context

I think that this could probably be solved by introducing a hard block on PlatformParameterController during startup. Normally, this would happen in SplashActivity but it seems that may not be an option since SplashActivity is doing a lot of other stuff.

This is actually rather tricky: we want the Dagger injections to be non-blocking (which means there does need to be some state syncing ahead of injection), but we need a good point in the app startup cycle to perform this logic. My current thinking is that we should:

  • Make invalid states throw an exception (so trying to inject unsynced parameters should always crash the app, including in splash).
  • Update splash to use lazy injection for platform parameters.
  • Introduce a proper syncing flow (such as adding a PlatformParameterController.syncAppState() function which force-refreshes the local state used by providers).

I'm not very keen on the mechanisms of the last option since it still leaves the possibility of having broken Dagger state, but short of introducing Hilt and carefully managed Dagger components, it's probably a reasonable approach.

@BenHenning BenHenning added bug End user-perceivable behaviors which are not desirable. triage needed Impact: High High perceived user impact (breaks a critical feature or blocks a release). labels Jan 29, 2025
@BenHenning BenHenning added this to the 1.0 Global availability milestone Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable. Impact: High High perceived user impact (breaks a critical feature or blocks a release). triage needed
Projects
None yet
Development

No branches or pull requests

1 participant