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
4 changes: 4 additions & 0 deletions src/xcode/ENA/ENA.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@
ABC6B7ED255AEF77000A1AC0 /* RiskProviderAndNewKeyPackagesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = ABC6B7E8255AEF74000A1AC0 /* RiskProviderAndNewKeyPackagesTests.swift */; };
ABD2F634254C533200DC1958 /* KeyPackageDownload.swift in Sources */ = {isa = PBXBuildFile; fileRef = ABD2F633254C533200DC1958 /* KeyPackageDownload.swift */; };
ABDA2792251CE308006BAE84 /* DMServerEnvironmentViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = ABDA2791251CE308006BAE84 /* DMServerEnvironmentViewController.swift */; };
ABFCE98A255C32EF0075FF13 /* AppConfigMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = ABFCE989255C32EE0075FF13 /* AppConfigMetadata.swift */; };
B103193224E18A0A00DD02EF /* DMMenuItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = B103193124E18A0A00DD02EF /* DMMenuItem.swift */; };
B10F9B8B249961BC00C418F4 /* DynamicTypeLabelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B10F9B89249961B500C418F4 /* DynamicTypeLabelTests.swift */; };
B10F9B8C249961CE00C418F4 /* UIFont+DynamicTypeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B163D11424993F64001A322C /* UIFont+DynamicTypeTests.swift */; };
Expand Down Expand Up @@ -979,6 +980,7 @@
ABC6B7E8255AEF74000A1AC0 /* RiskProviderAndNewKeyPackagesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RiskProviderAndNewKeyPackagesTests.swift; sourceTree = "<group>"; };
ABD2F633254C533200DC1958 /* KeyPackageDownload.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = KeyPackageDownload.swift; sourceTree = "<group>"; };
ABDA2791251CE308006BAE84 /* DMServerEnvironmentViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DMServerEnvironmentViewController.swift; sourceTree = "<group>"; };
ABFCE989255C32EE0075FF13 /* AppConfigMetadata.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppConfigMetadata.swift; sourceTree = "<group>"; };
B102BDC22460410600CD55A2 /* README.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = README.md; sourceTree = "<group>"; };
B103193124E18A0A00DD02EF /* DMMenuItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DMMenuItem.swift; sourceTree = "<group>"; };
B10F9B89249961B500C418F4 /* DynamicTypeLabelTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DynamicTypeLabelTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1448,6 +1450,7 @@
A16714BA248D18D20031B111 /* SummaryMetadata.swift */,
354E305824EFF26E00526C9F /* Country.swift */,
941ADDB12518C3FB00E421D9 /* ENSettingEuTracingViewModel.swift */,
ABFCE989255C32EE0075FF13 /* AppConfigMetadata.swift */,
94B255A52551B7C800649B4C /* WarnOthersReminder.swift */,
94427A4F25502B8900C36BE6 /* WarnOthersNotificationsTimeInterval.swift */,
948AFE662553DC5B0019579A /* WarnOthersRemindable.swift */,
Expand Down Expand Up @@ -3435,6 +3438,7 @@
A3816086250633D7002286E9 /* RequiresDismissConfirmation.swift in Sources */,
51D420B424583ABB00AD70CA /* AppStoryboard.swift in Sources */,
4026C2DC24852B7600926FB4 /* AppInformationViewController+LegalModel.swift in Sources */,
ABFCE98A255C32EF0075FF13 /* AppConfigMetadata.swift in Sources */,
01C6ABF42527273E0052814D /* String+Insertion.swift in Sources */,
EE278B30245F2C8A008B06F9 /* FriendsInviteController.swift in Sources */,
710ABB27247533FA00948792 /* DynamicTableViewController.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ final class CachingHTTPClientMock: CachingHTTPClient {
return config
}()

static let staticAppConfigMetadata: AppConfigMetadata = {
let bundle = Bundle(for: CachingHTTPClientMock.self)
let configMetadata = AppConfigMetadata(lastAppConfigETag: "\"SomeETag\"", lastAppConfigFetch: .distantPast, appConfig: CachingHTTPClientMock.staticAppConfig)
return configMetadata
}()

// MARK: AppConfigurationFetching

var onFetchAppConfiguration: ((String?, @escaping CachingHTTPClient.AppConfigResultHandler) -> Void)?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ final class DMStoreViewController: UITableViewController {
store.previousRiskLevel?.description ?? ""
},
DMStoreItem(attribute: "lastAppConfigETag") { store in
store.lastAppConfigETag?.description ?? "<nil>"
store.appConfigMetadata?.lastAppConfigETag.description ?? "<nil>"
},
DMStoreItem(attribute: "lastAppConfigFetch") { store in
store.lastAppConfigFetch?.description ?? "<nil>"
store.appConfigMetadata?.lastAppConfigFetch.description ?? "<nil>"
},
DMStoreItem(attribute: "appConfig") { store in
store.appConfig?.debugDescription ?? "<nil>"
store.appConfigMetadata?.appConfig.debugDescription ?? "<nil>"
}
]
}()
Expand Down
73 changes: 73 additions & 0 deletions src/xcode/ENA/ENA/Source/Models/Exposure/AppConfigMetadata.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//
// Corona-Warn-App
//
// SAP SE and all other contributors
// copyright owners license this file to you under the Apache
// License, Version 2.0 (the "License"); you may not use this
// file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
//

import Foundation

struct AppConfigMetadata: Codable, Equatable {

// MARK: - Init

init(
lastAppConfigETag: String,
lastAppConfigFetch: Date,
appConfig: SAP_Internal_ApplicationConfiguration
) {
self.lastAppConfigETag = lastAppConfigETag
self.lastAppConfigFetch = lastAppConfigFetch
self.appConfig = appConfig
}

// MARK: - Protocol Codable

enum CodingKeys: String, CodingKey {
case lastAppConfigETag
case lastAppConfigFetch
case appConfig
}

init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)

lastAppConfigETag = try container.decode(String.self, forKey: .lastAppConfigETag)
lastAppConfigFetch = try container.decode(Date.self, forKey: .lastAppConfigFetch)

let appConfigData = try container.decode(Data.self, forKey: .appConfig)
appConfig = try SAP_Internal_ApplicationConfiguration(serializedData: appConfigData)
}

func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)

try container.encode(lastAppConfigETag, forKey: .lastAppConfigETag)
try container.encode(lastAppConfigFetch, forKey: .lastAppConfigFetch)

let appConfigData = try appConfig.serializedData()
try container.encode(appConfigData, forKey: .appConfig)
}

// MARK: - Internal

var lastAppConfigETag: String
var lastAppConfigFetch: Date
var appConfig: SAP_Internal_ApplicationConfiguration

mutating func refeshLastAppConfigFetchDate() {
lastAppConfigFetch = Date()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,7 @@ final class ExposureDetectionExecutor: ExposureDetectionDelegate {
downloadedPackagesStore.open()

// Clear the app config
store.appConfig = nil
store.lastAppConfigETag = nil
store.lastAppConfigFetch = nil
store.appConfigMetadata = nil
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ import XCTest

final class ExposureDetectionExecutorTests: XCTestCase {

private var dummyAppConfigMetadata: AppConfigMetadata {
AppConfigMetadata(
lastAppConfigETag: "ETag",
lastAppConfigFetch: Date(),
appConfig: SAP_Internal_ApplicationConfiguration()
)
}

// MARK: - Write Downloaded Package Tests

func testWriteDownloadedPackage() throws {
Expand Down Expand Up @@ -139,7 +147,7 @@ final class ExposureDetectionExecutorTests: XCTestCase {
try packageStore.set(country: "DE", day: "SomeDay", etag: nil, package: package)

let store = MockTestStore()
store.appConfig = SAP_Internal_ApplicationConfiguration()
store.appConfigMetadata = dummyAppConfigMetadata

let sut = ExposureDetectionExecutor.makeWith(
packageStore: packageStore,
Expand All @@ -153,7 +161,7 @@ final class ExposureDetectionExecutorTests: XCTestCase {
)

XCTAssertNotEqual(packageStore.allDays(country: "DE").count, 0)
XCTAssertNotNil(store.appConfig)
XCTAssertNotNil(store.appConfigMetadata)

_ = sut.exposureDetection(
exposureDetection,
Expand All @@ -162,9 +170,7 @@ final class ExposureDetectionExecutorTests: XCTestCase {
completion: { _ in

XCTAssertEqual(packageStore.allDays(country: "DE").count, 0)
XCTAssertNil(store.appConfig)
XCTAssertNil(store.lastAppConfigETag)
XCTAssertNil(store.lastAppConfigFetch)
XCTAssertNil(store.appConfigMetadata)

completionExpectation.fulfill()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,63 +32,75 @@ final class CachedAppConfiguration {
deviceTimeCheck: DeviceTimeCheckProtocol? = nil,
configurationDidChange: (() -> Void)? = nil
) {
Log.debug("CachedAppConfiguration init called", log: .appConfig)

self.client = client
self.store = store
self.configurationDidChange = configurationDidChange

self.deviceTimeCheck = deviceTimeCheck ?? DeviceTimeCheck(store: store)

guard shouldFetch() else { return }

// edge case: if no app config is cached, omit a potentially existing ETag to force fetch a new configuration
let etag = store.appConfig == nil ? nil : store.lastAppConfigETag
guard shouldFetch() else { return }

// check for updated or fetch initial app configuration
fetchConfig(with: etag)
fetchConfig(with: store.appConfigMetadata?.lastAppConfigETag)
}

private func fetchConfig(with etag: String?, completion: Completion? = nil) {
Log.debug("fetchConfig called with etag:\(etag ?? "nil")", log: .appConfig)

client.fetchAppConfiguration(etag: etag) { [weak self] result in
guard let self = self else { return }

switch result.0 /* fyi, `result.1` would be the server time */{
case .success(let response):
self.store.lastAppConfigETag = response.eTag
self.store.appConfig = response.config
self.completeOnMain(completion: completion, result: .success(response.config))
let configMetadata = AppConfigMetadata(
lastAppConfigETag: response.eTag ?? "\"ReloadMe\"",
lastAppConfigFetch: Date(),
appConfig: response.config
)

// Skip processing of config if it didn't change.
guard self.store.appConfigMetadata?.lastAppConfigETag != configMetadata.lastAppConfigETag else {
Log.debug("Skip processing app config, because it didn't change", log: .appConfig)
self.completeOnMain(completion: completion, result: .success(response.config))
return
}

// keep track of last successful fetch
self.store.lastAppConfigFetch = Date()
Log.debug("Persist new app configuration", log: .appConfig)
self.store.appConfigMetadata = configMetadata

// update revokation list
let revokationList = self.store.appConfig?.revokationEtags ?? []
let revokationList = self.store.appConfigMetadata?.appConfig.revokationEtags ?? []
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("Error while removing invalidated key packages.", log: .appConfig, error: error)
// no further action - yet
}

self.completeOnMain(completion: completion, result: .success(response.config))

self.configurationDidChange?()
case .failure(let error):
switch error {
case CachedAppConfiguration.CacheError.notModified where self.store.appConfig != nil:
Log.error("config not modified", log: .api)
case CachedAppConfiguration.CacheError.notModified where self.store.appConfigMetadata != nil:
Log.error("Config not modified", log: .appConfig)
// server is not modified and we have a cached config
guard let config = self.store.appConfig else {
guard var appConfigMetadata = self.store.appConfigMetadata else {
fatalError("App configuration cache broken!") // in `where` we trust
}
self.completeOnMain(completion: completion, result: .success(config))

// server response HTTP 304 is considered a 'successful fetch'
self.store.lastAppConfigFetch = Date()
default:
// ensure reset
self.store.lastAppConfigETag = nil
self.store.lastAppConfigFetch = nil
Log.debug("Update lastAppConfigFetchDate of persisted app configuration", log: .appConfig)
appConfigMetadata.refeshLastAppConfigFetchDate()
self.store.appConfigMetadata = appConfigMetadata

self.completeOnMain(completion: completion, result: .success(appConfigMetadata.appConfig))
default:
self.completeOnMain(completion: completion, result: .failure(error))
}
}
Expand Down Expand Up @@ -117,16 +129,18 @@ extension CachedAppConfiguration: AppConfigurationProviding {
fileprivate static let timestampKey = "LastAppConfigFetch"

func appConfiguration(forceFetch: Bool = false, completion: @escaping Completion) {
Log.debug("Request app configuration forceFetch: \(forceFetch)", log: .appConfig)

let force = shouldFetch() || forceFetch

if let cachedVersion = store.appConfig, !force {
Log.debug("[App Config] fetching cached app configuration", log: .localData)
if let cachedVersion = store.appConfigMetadata, !force {
Log.debug("fetching cached app configuration", log: .appConfig)
// use the cached version
completeOnMain(completion: completion, result: .success(cachedVersion))
completeOnMain(completion: completion, result: .success(cachedVersion.appConfig))
} else {
Log.debug("[App Config] fetching fresh app configuration", log: .localData)
Log.debug("fetching fresh app configuration. forceFetch: \(forceFetch), force: \(force)", log: .appConfig)
// fetch a new one
fetchConfig(with: store.lastAppConfigETag, completion: completion)
fetchConfig(with: store.appConfigMetadata?.lastAppConfigETag, completion: completion)
}
}

Expand All @@ -139,14 +153,19 @@ extension CachedAppConfiguration: AppConfigurationProviding {
/// which does not easily return response headers. This requires further refactoring of `URLSession+Convenience.swift`.
/// - Returns: `true` is a network call should be done; `false` if cache should be used
private func shouldFetch() -> Bool {
if store.appConfig == nil { return true }
Log.debug("shouldFetch called", log: .appConfig)

if store.appConfigMetadata == nil {
Log.debug("store.appConfigMetadata is nil", log: .appConfig)
return true
}

// naïve cache control
guard let lastFetch = store.lastAppConfigFetch else {
Log.debug("[Cache-Control] no last config fetch timestamp stored", log: .localData)
guard let lastFetch = store.appConfigMetadata?.lastAppConfigFetch else {
Log.debug("no last config fetch timestamp stored", log: .appConfig)
return true
}
Log.debug("[Cache-Control] timestamp >= 300s? \(abs(lastFetch.distance(to: Date())) >= 300)", log: .localData)
Log.debug("timestamp >= 300s? \(abs(lastFetch.distance(to: Date())) >= 300)", log: .appConfig)
return abs(lastFetch.distance(to: Date())) >= 300
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ final class DeviceTimeCheck: DeviceTimeCheckProtocol {
deviceTime: deviceTime
),
isDeviceTimeCheckKillSwitchActive: self.isDeviceTimeCheckKillSwitchActive(
config: self.store.appConfig
config: self.store.appConfigMetadata?.appConfig
)
)
}
Expand Down
Loading