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

fetch & cache statistical data #1807

Merged
merged 18 commits into from Jan 18, 2021

Conversation

whiskey
Copy link
Contributor

@whiskey whiskey commented Jan 15, 2021

Description

Loads and caches statistical data. Live server fetching and testing is tbd!
Although the spec does not require a 'secure' caching of the public stats data I added them to our Store to be aligned with other data providers such as AddConfig. Implementing native caching for just this call would clutter the code even more.

Link to Jira

Screenshots

n/a

@whiskey whiskey added the enhancement Improvement of an existing feature label Jan 17, 2021
@whiskey whiskey added this to the v1.11.0 milestone Jan 17, 2021
@whiskey whiskey marked this pull request as ready for review January 17, 2021 21:48
@whiskey whiskey requested a review from a team January 17, 2021 21:48

import Foundation

struct StatisticsMetadata: Codable {
Copy link
Contributor

Choose a reason for hiding this comment

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

project MARKS missing

self.store = store
}

func statistics() -> AnyPublisher<SAP_Internal_Stats_Statistics, Error> {
Copy link
Contributor

Choose a reason for hiding this comment

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

also some project MARKs missing

/// - Parameters:
/// - client: The client to fetch the stats
/// - store: Used for caching
init(client: StatisticsFetching, store: Store) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
init(client: StatisticsFetching, store: Store) {
init(client: StatisticsFetching, store: StatisticsCaching) {

guard let lastFetch = store.statistics?.lastStatisticsFetch else {
return true
}
Log.debug("timestamp >= 300s? \(abs(Date().timeIntervalSince(lastFetch))) >= 300)", log: .appConfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Log.debug("timestamp >= 300s? \(abs(Date().timeIntervalSince(lastFetch))) >= 300)", log: .appConfig)
Log.debug("timestamp >= 300s? \(abs(Date().timeIntervalSince(lastFetch))) >= 300)", log: .statistics)

@whiskey whiskey merged commit 7a67d87 into release/1.11.x Jan 18, 2021
@whiskey whiskey deleted the feature/4611-fetch-statistical-data branch January 18, 2021 12:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants