From 1a355e992117945bf29844f289791f525f96259c Mon Sep 17 00:00:00 2001 From: Cody Garvin Date: Fri, 2 Jul 2021 16:26:21 -0700 Subject: [PATCH 1/4] Added file size storage limits --- .../DestinationsExample/AppDelegate.swift | 4 +- .../Plugins/Platforms/iOS/iOSDelegation.swift | 6 +-- .../Platforms/watchOS/watchOSDelegation.swift | 6 +-- .../Segment/Plugins/SegmentDestination.swift | 45 ++++--------------- Sources/Segment/Utilities/Storage.swift | 43 +++++++++++++----- 5 files changed, 47 insertions(+), 57 deletions(-) diff --git a/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift b/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift index a390c360..8b056139 100644 --- a/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift +++ b/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift @@ -17,7 +17,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool { // Override point for customization after application launch. - let configuration = Configuration(writeKey: "WRITE_KEY") + let configuration = Configuration(writeKey: "1234") .trackApplicationLifecycleEvents(true) .flushInterval(1) @@ -86,7 +86,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { // This functionality is needed to forward deep link attribution data with AppsFlyer // Report Push Notification attribution data for re-engagements - func application(_ application: UIApplication, didReceiveRemoteNotification userInfo: [String : Any], fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) { + func application(_ application: UIApplication, didReceiveRemoteNotification userInfo: [AnyHashable : Any], fetchCompletionHandler completionHandler: @escaping (UIBackgroundFetchResult) -> Void) { // this enables remote notifications for various destinations (appsflyer) analytics?.receivedRemoteNotification(userInfo: userInfo) } diff --git a/Sources/Segment/Plugins/Platforms/iOS/iOSDelegation.swift b/Sources/Segment/Plugins/Platforms/iOS/iOSDelegation.swift index 28df4f38..33e55caf 100644 --- a/Sources/Segment/Plugins/Platforms/iOS/iOSDelegation.swift +++ b/Sources/Segment/Plugins/Platforms/iOS/iOSDelegation.swift @@ -16,14 +16,14 @@ import UIKit public protocol RemoteNotifications: Plugin { func registeredForRemoteNotifications(deviceToken: Data) func failedToRegisterForRemoteNotification(error: Error?) - func receivedRemoteNotification(userInfo: [String: Any]) + func receivedRemoteNotification(userInfo: [AnyHashable: Any]) func handleAction(identifier: String, userInfo: [String: Any]) } extension RemoteNotifications { public func registeredForRemoteNotifications(deviceToken: Data) {} public func failedToRegisterForRemoteNotification(error: Error?) {} - public func receivedRemoteNotification(userInfo: [String: Any]) {} + public func receivedRemoteNotification(userInfo: [AnyHashable: Any]) {} public func handleAction(identifier: String, userInfo: [String: Any]) {} } @@ -46,7 +46,7 @@ extension Analytics { } } - public func receivedRemoteNotification(userInfo: [String: Any]) { + public func receivedRemoteNotification(userInfo: [AnyHashable: Any]) { apply { plugin in if let p = plugin as? RemoteNotifications { p.receivedRemoteNotification(userInfo: userInfo) diff --git a/Sources/Segment/Plugins/Platforms/watchOS/watchOSDelegation.swift b/Sources/Segment/Plugins/Platforms/watchOS/watchOSDelegation.swift index 895a5ee6..46618446 100644 --- a/Sources/Segment/Plugins/Platforms/watchOS/watchOSDelegation.swift +++ b/Sources/Segment/Plugins/Platforms/watchOS/watchOSDelegation.swift @@ -15,13 +15,13 @@ import WatchKit public protocol RemoteNotifications: Plugin { func registeredForRemoteNotifications(deviceToken: Data) func failedToRegisterForRemoteNotification(error: Error?) - func receivedRemoteNotification(userInfo: [String: Any]) + func receivedRemoteNotification(userInfo: [AnyHashable: Any]) } extension RemoteNotifications { public func registeredForRemoteNotifications(deviceToken: Data) {} public func failedToRegisterForRemoteNotification(error: Error?) {} - public func receivedRemoteNotification(userInfo: [String: Any]) {} + public func receivedRemoteNotification(userInfo: [AnyHashable: Any]) {} } extension Analytics { @@ -43,7 +43,7 @@ extension Analytics { } } - public func receivedRemoteNotification(userInfo: [String: Any]) { + public func receivedRemoteNotification(userInfo: [AnyHashable: Any]) { apply { plugin in if let p = plugin as? RemoteNotifications { p.receivedRemoteNotification(userInfo: userInfo) diff --git a/Sources/Segment/Plugins/SegmentDestination.swift b/Sources/Segment/Plugins/SegmentDestination.swift index 6fd79e70..dac75181 100644 --- a/Sources/Segment/Plugins/SegmentDestination.swift +++ b/Sources/Segment/Plugins/SegmentDestination.swift @@ -37,7 +37,6 @@ public class SegmentDestination: DestinationPlugin { private var httpClient: HTTPClient? private var uploads = [UploadTaskInfo]() private var storage: Storage? - private var maxPayloadSize = 500000 // Max 500kb private var apiKey: String? = nil private var apiHost: String? = nil @@ -139,21 +138,19 @@ public class SegmentDestination: DestinationPlugin { for url in data { analytics.log(message: "Processing Batch:\n\(url.lastPathComponent)") - if isPayloadSizeAcceptable(url: url) { - let uploadTask = httpClient.startBatchUpload(writeKey: analytics.configuration.values.writeKey, batch: url) { (result) in - switch result { + let uploadTask = httpClient.startBatchUpload(writeKey: analytics.configuration.values.writeKey, batch: url) { (result) in + switch result { case .success(_): storage.remove(file: url) default: analytics.logFlush() - } - - analytics.log(message: "Processed: \(url.lastPathComponent)") - } - // we have a legit upload in progress now, so add it to our list. - if let upload = uploadTask { - add(uploadTask: UploadTaskInfo(url: url, task: upload)) } + + analytics.log(message: "Processed: \(url.lastPathComponent)") + } + // we have a legit upload in progress now, so add it to our list. + if let upload = uploadTask { + add(uploadTask: UploadTaskInfo(url: url, task: upload)) } } } else { @@ -190,30 +187,4 @@ extension SegmentDestination { internal func add(uploadTask: UploadTaskInfo) { uploads.append(uploadTask) } - - internal func isPayloadSizeAcceptable(url: URL) -> Bool { - var result = true - var fileSizeTotal: Int64 = 0 - - // Make sure we're under the max payload size. - do { - let attributes = try FileManager.default.attributesOfItem(atPath: url.path) - guard let fileSize = attributes[FileAttributeKey.size] as? Int64 else { - analytics?.log(message: "File size could not be read") - // none of the logic beyond here will work if we can't get the - // filesize so assume everything is good and hope for the best. - return true - } - fileSizeTotal += fileSize - } catch { - analytics?.log(message: "Could not read file attributes") - } - - if fileSizeTotal >= maxPayloadSize { - analytics?.log(message: "Batch file is too large to be sent") - result = false - } - return result - } - } diff --git a/Sources/Segment/Utilities/Storage.swift b/Sources/Segment/Utilities/Storage.swift index af7f4601..45f62125 100644 --- a/Sources/Segment/Utilities/Storage.swift +++ b/Sources/Segment/Utilities/Storage.swift @@ -13,6 +13,7 @@ internal class Storage: Subscriber { let writeKey: String let syncQueue = DispatchQueue(label: "storage.segment.com") let userDefaults: UserDefaults? + static let MAXFILESIZE = 475000 // Server accepts max 500k per batch init(store: Store, writeKey: String) { self.store = store @@ -44,14 +45,8 @@ extension Storage { switch key { case .events: if let event = value as? RawEvent { - // this is synchronized against finish(file:) down below. - var currentFile = 0 - syncQueue.sync { - let index: Int = userDefaults?.integer(forKey: key.rawValue) ?? 0 - userDefaults?.set(index, forKey: key.rawValue) - currentFile = index - } - self.storeEvent(toFile: self.eventsFile(index: currentFile), event: event) + let eventStoreFile = currentFile(key) + self.storeEvent(toFile: eventStoreFile, event: event) } break default: @@ -137,6 +132,16 @@ extension Storage { } return result } + + func currentFile(_ key: Storage.Constants) -> URL { + var currentFile = 0 + syncQueue.sync { + let index: Int = userDefaults?.integer(forKey: key.rawValue) ?? 0 + userDefaults?.set(index, forKey: key.rawValue) + currentFile = index + } + return self.eventsFile(index: currentFile) + } } // MARK: - State Subscriptions @@ -213,10 +218,24 @@ extension Storage { extension Storage { func storeEvent(toFile file: URL, event: RawEvent) { + + var storeFile = file + let fm = FileManager.default var newFile = false - if fm.fileExists(atPath: file.path) == false { - start(file: file) + if fm.fileExists(atPath: storeFile.path) == false { + start(file: storeFile) + newFile = true + } + + // Verify file size isn't too large + if let attributes = try? fm.attributesOfItem(atPath: storeFile.path), + let fileSize = attributes[FileAttributeKey.size] as? UInt64, + fileSize >= MAXFILESIZE { + finish(file: storeFile) + // Set the new file path + storeFile = currentFile(.events) + start(file: storeFile) newFile = true } @@ -224,7 +243,7 @@ extension Storage { do { let jsonString = event.toString() if let jsonData = jsonString.data(using: .utf8) { - let handle = try FileHandle(forWritingTo: file) + let handle = try FileHandle(forWritingTo: storeFile) handle.seekToEndOfFile() // prepare for the next entry if newFile == false { @@ -237,7 +256,7 @@ extension Storage { assert(false, "Storage: Unable to convert event to json!") } } catch { - assert(false, "Storage: failed to write event to \(file), error: \(error)") + assert(false, "Storage: failed to write event to \(storeFile), error: \(error)") } } } From fddabcef960204877142b8852b37996f59c94aa6 Mon Sep 17 00:00:00 2001 From: Cody Garvin Date: Fri, 2 Jul 2021 16:28:36 -0700 Subject: [PATCH 2/4] Change writekey --- .../DestinationsExample/DestinationsExample/AppDelegate.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift b/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift index 8b056139..035136b4 100644 --- a/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift +++ b/Examples/apps/DestinationsExample/DestinationsExample/AppDelegate.swift @@ -17,7 +17,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate { func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool { // Override point for customization after application launch. - let configuration = Configuration(writeKey: "1234") + let configuration = Configuration(writeKey: "WRITE_KEY") .trackApplicationLifecycleEvents(true) .flushInterval(1) From 0b1f17707ea6db1b9fd8b950e0060657dc6c37ce Mon Sep 17 00:00:00 2001 From: Cody Garvin Date: Fri, 2 Jul 2021 16:34:16 -0700 Subject: [PATCH 3/4] Added clarity --- Sources/Segment/Utilities/Storage.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Segment/Utilities/Storage.swift b/Sources/Segment/Utilities/Storage.swift index 45f62125..588cc082 100644 --- a/Sources/Segment/Utilities/Storage.swift +++ b/Sources/Segment/Utilities/Storage.swift @@ -217,6 +217,7 @@ extension Storage { // MARK: - Event Storage extension Storage { + func storeEvent(toFile file: URL, event: RawEvent) { var storeFile = file From a026ddf7956c5eb2b157f5cb2437680cceedface Mon Sep 17 00:00:00 2001 From: Cody Garvin Date: Fri, 2 Jul 2021 16:37:40 -0700 Subject: [PATCH 4/4] Added test fix --- Sources/Segment/Utilities/Storage.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Segment/Utilities/Storage.swift b/Sources/Segment/Utilities/Storage.swift index 588cc082..97f1a45b 100644 --- a/Sources/Segment/Utilities/Storage.swift +++ b/Sources/Segment/Utilities/Storage.swift @@ -232,7 +232,7 @@ extension Storage { // Verify file size isn't too large if let attributes = try? fm.attributesOfItem(atPath: storeFile.path), let fileSize = attributes[FileAttributeKey.size] as? UInt64, - fileSize >= MAXFILESIZE { + fileSize >= Storage.MAXFILESIZE { finish(file: storeFile) // Set the new file path storeFile = currentFile(.events)