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

Fix/3711 refactor cached app config #1506

Merged
merged 11 commits into from
Nov 13, 2020

Conversation

ArturFriesen
Copy link
Contributor

Description

This PR prevents the race condition which is leading to trigger risk detection more then the timeframe (24h) allows.
The meta data and the cached app config are persisted at once, preventing data collisions when the cache called multiple times.

Link to Jira

links follows. Cant access jira atm.

@ArturFriesen ArturFriesen added the bug Something isn't working label Nov 13, 2020
@ArturFriesen ArturFriesen added this to the v1.7.0 milestone Nov 13, 2020
@ArturFriesen ArturFriesen requested a review from a team November 13, 2020 07:44
let bundle = Bundle(for: CachingHTTPClientMock.self)
// there is a test for this (`testStaticAppConfiguration`), let's keep it short.
guard
let fixtureUrl = bundle.url(forResource: "de-config-int-2020-09-25", withExtension: nil),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just FYI, the app configuration format will change in the next two sprints which will likely break the deserialization. However, with the default app configuration on the horizon we might have this covered!

Unless we have a need for a bespoke config, we could use the static default configuration, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Made use if CachingHTTPClientMock.staticAppConfig

self.packageStore?.revokationList = revokationList // for future package-operations
// validate currently stored key packages
do {
try self.packageStore?.validateCachedKeyPackages(revokationList: revokationList)
} catch {
Log.error("Error while removing invalidated key packages.", log: .localData, error: error)
Log.error("[App Config] Error while removing invalidated key packages.", log: .localData, error: error)
Copy link
Contributor

Choose a reason for hiding this comment

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

We could create custom OSLog keys for .cache and .appConfig!? This would allow nicer filtering for the different components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added .appConfig and used it.

@marcussc marcussc merged commit ccb7819 into release/1.7.x Nov 13, 2020
@marcussc marcussc deleted the fix/3711-Refactor_Cached_AppConfig branch November 13, 2020 11:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants