-
Notifications
You must be signed in to change notification settings - Fork 4
LOOP-1201 Export of Critical Event Logs #242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1294,6 +1294,86 @@ extension CarbStore { | |
| } | ||
| } | ||
|
|
||
| // MARK: - Critical Event Log Export | ||
|
|
||
| extension CarbStore: CriticalEventLog { | ||
| private var exportProgressUnitCountPerObject: Int64 { 1 } | ||
| private var exportFetchLimit: Int { Int(criticalEventLogExportProgressUnitCountPerFetch / exportProgressUnitCountPerObject) } | ||
|
|
||
| public var exportName: String { "Carbs.json" } | ||
|
|
||
| public func exportProgressTotalUnitCount(startDate: Date, endDate: Date? = nil) -> Result<Int64, Error> { | ||
| var result: Result<Int64, Error>? | ||
|
|
||
| self.cacheStore.managedObjectContext.performAndWait { | ||
| do { | ||
| let request: NSFetchRequest<CachedCarbObject> = CachedCarbObject.fetchRequest() | ||
| request.predicate = self.exportDatePredicate(startDate: startDate, endDate: endDate) | ||
|
|
||
| let objectCount = try self.cacheStore.managedObjectContext.count(for: request) | ||
| result = .success(Int64(objectCount) * exportProgressUnitCountPerObject) | ||
| } catch let error { | ||
| result = .failure(error) | ||
| } | ||
| } | ||
|
|
||
| return result! | ||
| } | ||
|
|
||
| public func export(startDate: Date, endDate: Date, to stream: OutputStream, progress: Progress) -> Error? { | ||
| let encoder = JSONStreamEncoder(stream: stream) | ||
| var anchorKey: Int64 = 0 | ||
| var fetching = true | ||
| var error: Error? | ||
|
|
||
| while fetching && error == nil { | ||
| self.cacheStore.managedObjectContext.performAndWait { | ||
| do { | ||
| guard !progress.isCancelled else { | ||
| throw CriticalEventLogError.cancelled | ||
| } | ||
|
|
||
| let request: NSFetchRequest<CachedCarbObject> = CachedCarbObject.fetchRequest() | ||
| request.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [NSPredicate(format: "anchorKey > %d", anchorKey), | ||
| self.exportDatePredicate(startDate: startDate, endDate: endDate)]) | ||
| request.sortDescriptors = [NSSortDescriptor(key: "anchorKey", ascending: true)] | ||
| request.fetchLimit = self.exportFetchLimit | ||
|
|
||
| let objects = try self.cacheStore.managedObjectContext.fetch(request) | ||
| if objects.isEmpty { | ||
| fetching = false | ||
| return | ||
| } | ||
|
|
||
| try encoder.encode(objects) | ||
|
|
||
| anchorKey = objects.last!.anchorKey | ||
|
|
||
| progress.completedUnitCount += Int64(objects.count) * exportProgressUnitCountPerObject | ||
| } catch let fetchError { | ||
| error = fetchError | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let closeError = encoder.close(), error == nil { | ||
| error = closeError | ||
| } | ||
|
|
||
| return error | ||
| } | ||
|
|
||
| private func exportDatePredicate(startDate: Date, endDate: Date? = nil) -> NSPredicate { | ||
| var addedDatePredicate = NSPredicate(format: "addedDate >= %@", startDate as NSDate) | ||
| var supercededDatePredicate = NSPredicate(format: "supercededDate >= %@", startDate as NSDate) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a roughly similar question about this that I had with Similarly for
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since historical exports are captured on a daily basis it is necessary to cover the use case of the carb object added during one daily period and the carb object superseded in a subsequent daily period. If
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ |
||
| if let endDate = endDate { | ||
| addedDatePredicate = NSCompoundPredicate(andPredicateWithSubpredicates: [addedDatePredicate, NSPredicate(format: "addedDate < %@", endDate as NSDate)]) | ||
| supercededDatePredicate = NSCompoundPredicate(andPredicateWithSubpredicates: [supercededDatePredicate, NSPredicate(format: "supercededDate < %@", endDate as NSDate)]) | ||
| } | ||
| return NSCompoundPredicate(orPredicateWithSubpredicates: [addedDatePredicate, supercededDatePredicate]) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Core Data (Bulk) - TEST ONLY | ||
|
|
||
| extension CarbStore { | ||
|
|
@@ -1306,16 +1386,14 @@ extension CarbStore { | |
| queue.async { | ||
| var error: Error? | ||
|
|
||
| let date = Date() | ||
|
|
||
| self.cacheStore.managedObjectContext.performAndWait { | ||
| do { | ||
| for entry in entries { | ||
| let syncIdentifier = try self.cacheStore.managedObjectContext.generateUniqueSyncIdentifier() | ||
|
|
||
| let object = CachedCarbObject(context: self.cacheStore.managedObjectContext) | ||
| object.create(from: entry, | ||
| on: date, | ||
| on: entry.date, | ||
| provenanceIdentifier: self.provenanceIdentifier, | ||
| syncIdentifier: syncIdentifier) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| // | ||
| // CriticalEventLog.swift | ||
| // LoopKit | ||
| // | ||
| // Created by Darin Krauss on 7/15/20. | ||
| // Copyright © 2020 LoopKit Authors. All rights reserved. | ||
| // | ||
|
|
||
| import Foundation | ||
|
|
||
| public protocol CriticalEventLog { | ||
|
|
||
| /// The name for the critical event log export. | ||
| var exportName: String { get } | ||
|
|
||
| /// Calculate the progress total unit count for the critical event log export for the specified date range. | ||
| /// | ||
| /// - Parameters: | ||
| /// - startDate: The start date for the critical events to export. | ||
| /// - endDate: The end date for the critical events to export. Optional. If not specified, default to now. | ||
| /// - Returns: An progress total unit count, or an error. | ||
| func exportProgressTotalUnitCount(startDate: Date, endDate: Date?) -> Result<Int64, Error> | ||
|
|
||
| /// Export the critical event log for the specified date range. | ||
| /// | ||
| /// - Parameters: | ||
| /// - startDate: The start date for the critical events to export. | ||
| /// - endDate: The end date for the critical events to export. | ||
| /// - stream: The output stream to write the critical event log to. Typically writes JSON UTF-8 text. | ||
| /// - progressor: The estimated duration progress to use to check if cancelled and report progress. | ||
| /// - Returns: Any error that occurs during the export, or nil if successful. | ||
| func export(startDate: Date, endDate: Date, to stream: OutputStream, progress: Progress) -> Error? | ||
| } | ||
|
|
||
| public enum CriticalEventLogError: Error { | ||
|
|
||
| /// The export was cancelled either by the user or the OS. | ||
| case cancelled | ||
| } | ||
|
|
||
| public let criticalEventLogExportProgressUnitCountPerFetch: Int64 = 250 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,17 @@ | ||
| <?xml version="1.0" encoding="UTF-8" standalone="yes"?> | ||
| <model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="15400" systemVersion="19C57" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier=""> | ||
| <model type="com.apple.IDECoreDataModeler.DataModel" documentVersion="1.0" lastSavedToolsVersion="16119" systemVersion="19G2021" minimumToolsVersion="Automatic" sourceLanguage="Swift" userDefinedModelVersionIdentifier=""> | ||
| <entity name="Entry" representedClassName=".DeviceLogEntry" syncable="YES"> | ||
| <attribute name="deviceIdentifier" optional="YES" attributeType="String"/> | ||
| <attribute name="managerIdentifier" attributeType="String"/> | ||
| <attribute name="message" attributeType="String"/> | ||
| <attribute name="modificationCounter" attributeType="Integer 64" defaultValueString="0" usesScalarValueType="YES"/> | ||
| <attribute name="timestamp" attributeType="Date" usesScalarValueType="NO"/> | ||
| <attribute name="type" attributeType="String"/> | ||
| <fetchIndex name="byTimestampIndex"> | ||
| <fetchIndexElement property="timestamp" type="Binary" order="ascending"/> | ||
| </fetchIndex> | ||
| </entity> | ||
| <elements> | ||
| <element name="Entry" positionX="-36" positionY="9" width="128" height="118"/> | ||
| <element name="Entry" positionX="-36" positionY="9" width="128" height="133"/> | ||
| </elements> | ||
| </model> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,6 +117,84 @@ public class PersistentDeviceLog { | |
| } | ||
| } | ||
|
|
||
| // MARK: - Critical Event Log Export | ||
|
|
||
| extension PersistentDeviceLog: CriticalEventLog { | ||
| private var exportProgressUnitCountPerObject: Int64 { 1 } | ||
| private var exportFetchLimit: Int { Int(criticalEventLogExportProgressUnitCountPerFetch / exportProgressUnitCountPerObject) } | ||
|
|
||
| public var exportName: String { "DeviceLog.json" } | ||
|
|
||
| public func exportProgressTotalUnitCount(startDate: Date, endDate: Date? = nil) -> Result<Int64, Error> { | ||
| var result: Result<Int64, Error>? | ||
|
|
||
| self.managedObjectContext.performAndWait { | ||
| do { | ||
| let request: NSFetchRequest<DeviceLogEntry> = DeviceLogEntry.fetchRequest() | ||
| request.predicate = self.exportDatePredicate(startDate: startDate, endDate: endDate) | ||
|
|
||
| let objectCount = try self.managedObjectContext.count(for: request) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. count(*) operations often require a full table scan (and thus a lot of io). I'm not sure that's the case here or how heavily you're relying on this being performant, but thought I'd mention it. Count can be faster than an actual fetch because you save on hydration; but since IO is usually the slowest part of the query, the count operation might be nearly as slow an actual fetch of all the objects involved.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need progress (as discussed above) and progress highly depends upon the amount of data, thus the need for the count. The count has only a little effect on overall duration. (It was necessary to add indexes on dosing decisions and device comms table for exporting in general, not specific to progress, so this solves most of the issue).
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ |
||
| result = .success(Int64(objectCount) * exportProgressUnitCountPerObject) | ||
| } catch let error { | ||
| result = .failure(error) | ||
| } | ||
| } | ||
|
|
||
| return result! | ||
| } | ||
|
|
||
| public func export(startDate: Date, endDate: Date, to stream: OutputStream, progress: Progress) -> Error? { | ||
| let encoder = JSONStreamEncoder(stream: stream) | ||
| var modificationCounter: Int64 = 0 | ||
| var fetching = true | ||
| var error: Error? | ||
|
|
||
| while fetching && error == nil { | ||
| self.managedObjectContext.performAndWait { | ||
| do { | ||
| guard !progress.isCancelled else { | ||
| throw CriticalEventLogError.cancelled | ||
| } | ||
|
|
||
| let request: NSFetchRequest<DeviceLogEntry> = DeviceLogEntry.fetchRequest() | ||
| request.predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [NSPredicate(format: "modificationCounter > %d", modificationCounter), | ||
| self.exportDatePredicate(startDate: startDate, endDate: endDate)]) | ||
| request.sortDescriptors = [NSSortDescriptor(key: "modificationCounter", ascending: true)] | ||
| request.fetchLimit = self.exportFetchLimit | ||
|
|
||
| let objects = try self.managedObjectContext.fetch(request) | ||
| if objects.isEmpty { | ||
| fetching = false | ||
| return | ||
| } | ||
|
|
||
| try encoder.encode(objects) | ||
|
|
||
| modificationCounter = objects.last!.modificationCounter | ||
|
|
||
| progress.completedUnitCount += Int64(objects.count) * exportProgressUnitCountPerObject | ||
| } catch let fetchError { | ||
| error = fetchError | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if let closeError = encoder.close(), error == nil { | ||
| error = closeError | ||
| } | ||
|
|
||
| return error | ||
| } | ||
|
|
||
| private func exportDatePredicate(startDate: Date, endDate: Date? = nil) -> NSPredicate { | ||
| var predicate = NSPredicate(format: "timestamp >= %@", startDate as NSDate) | ||
| if let endDate = endDate { | ||
| predicate = NSCompoundPredicate(andPredicateWithSubpredicates: [predicate, NSPredicate(format: "timestamp < %@", endDate as NSDate)]) | ||
| } | ||
| return predicate | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Core Data (Bulk) - TEST ONLY | ||
|
|
||
| extension PersistentDeviceLog { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with Rick; if this work is really isolated to an extension, putting it into a separate file might make this a little more navigable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I can do this, but if we are going this way, we should make this a general, well-known policy. We haven't been doing this before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel that strongly, though, FWIW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's not a big issue for me either; just seems a little more navigable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be fine if we don't do this here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code in this extension uses the managed object context, which is a private property. To move this extension out of the main file would require changing the property access from private to internal. I believe keeping the property private is more important than moving the extension to a separate file. Going to leave as-is unless anyone objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅