Skip to content
This repository was archived by the owner on Jun 20, 2023. It is now read-only.

Improved app config handling (Identifier, local fallback) (DEV) #1553

Merged
merged 13 commits into from
Nov 10, 2020
Merged

Improved app config handling (Identifier, local fallback) (DEV) #1553

merged 13 commits into from
Nov 10, 2020

Conversation

@d4rken
Copy link
Member

@d4rken d4rken commented Nov 7, 2020

  • The app config now offers an identifier field that is unique, and represents the current app config. If the identifier changes, the app config should be considered changed (needed for EXPOSUREAPP-2718, risk calc reset after config change).
  • There is now a local default app config (@mlenkeit ❤️ ) that we can fall back to if all else fails which allows us to guarantee that the app config is never null, which makes our app behavior much more consistent.
  • The app config update call will now switch scopes, previously the callers scope was used, now the app scope is used. We want to prevent that a canceled caller's scope, will cancel the config retrieval, we'll want that new config in any case, it should never be aborted. I think this was the cause for the error @harambasicluka produced here. I can't reproduce it locally though, can you confirm whether this fixes it Luka?

Testing

  • Force the app into edge cases, fresh launch, no internet etc.

@d4rken d4rken added enhancement Improvement of an existing feature maintainers Tag pull requests created by maintainers labels Nov 7, 2020
@d4rken d4rken added this to the 1.7.0 milestone Nov 7, 2020
@d4rken d4rken requested a review from a team November 7, 2020 00:15
@harambasicluka harambasicluka mentioned this pull request Nov 9, 2020
Merged
3 tasks
@harambasicluka
Copy link
Contributor

harambasicluka commented Nov 9, 2020

Getting the same error as in #1550 , but the app isn't crashing any longer as promised by @d4rken ! <3

2020-11-12 19:10:03.846 10448-10686/de.rki.coronawarnapp.test V/AppConfigStorage: get() AppConfig
2020-11-12 19:10:03.849 10448-10686/de.rki.coronawarnapp.test E/AppConfigStorage: Couldn't load config.
    java.io.FileNotFoundException: /data/user/0/de.rki.coronawarnapp.test/files/appconfig_storage/appconfig.json (No such file or directory)
        at java.io.FileInputStream.open0(Native Method)
        at java.io.FileInputStream.open(FileInputStream.java:231)
        at java.io.FileInputStream.<init>(FileInputStream.java:165)
        at de.rki.coronawarnapp.appconfig.download.AppConfigStorage.getStoredConfig(AppConfigStorage.kt:98)
        at de.rki.coronawarnapp.appconfig.AppConfigSource$retrieveConfig$2.invokeSuspend(AppConfigSource.kt:53)
        at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
        at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
        at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:738)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
        at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)
2020-11-12 19:10:03.850 10448-10686/de.rki.coronawarnapp.test W/AppConfigRetriever: Current or fallback config was unavailable, using default.

harambasicluka
harambasicluka reviewed Nov 9, 2020
View reviewed changes
Copy link
Contributor

@harambasicluka harambasicluka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, also works on my device which crashed before.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 10, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

ralfgehrer
ralfgehrer approved these changes Nov 10, 2020
View reviewed changes
Copy link
Contributor

@ralfgehrer ralfgehrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm. Could almost be considered offline-mode ready ;)

@ralfgehrer ralfgehrer merged commit 22f3622 into release/1.7.x Nov 10, 2020
@ralfgehrer ralfgehrer deleted the feature/improved-app-config-handling branch November 10, 2020 16:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@mlenkeit mlenkeit mlenkeit left review comments

@ralfgehrer ralfgehrer ralfgehrer approved these changes

@harambasicluka harambasicluka harambasicluka approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement Improvement of an existing feature maintainers Tag pull requests created by maintainers

Projects

None yet

Milestone

1.7.0

Development

Successfully merging this pull request may close these issues.

5 participants

@d4rken @harambasicluka @ralfgehrer @mlenkeit