Draft
Conversation
Collaborator
Generated by 🚫 Danger |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds persistent cellular (and overall) data usage tracking across streaming, downloads, uploads, and sync so the iOS app can display more detailed data-usage breakdowns.
Changes:
- Introduces a new
CellularDataUsagedatabase table +CellularDataUsageManagerfor recording and aggregating usage by operation/connection/session. - Hooks tracking into streaming (direct AVPlayer + cached streaming), downloads, uploads, and background sync via
URLSessionTaskMetrics/ access logs. - Adds periodic retention cleanup for usage records after sync.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| podcasts/StreamingCellularTracker.swift | New NWPathMonitor/access-log-based tracker for direct AVPlayer streaming |
| podcasts/ServerSyncManager.swift | Adds periodic cleanup of old CellularDataUsage records |
| podcasts/MediaExporterResourceLoaderDelegate.swift | Captures per-task streamed bytes via URLSession metrics (cached streaming path) |
| podcasts/DownloadManager.swift | Passes episode/podcast context into streaming resource loader delegate |
| podcasts/DownloadManager+URLSessionDelegate.swift | Records bytes for episode downloads/stream-downloads using URLSession metrics |
| podcasts/DefaultPlayer.swift | Starts/stops StreamingCellularTracker for remote (non-file) AVPlayer assets |
| podcasts/Constants.swift | Adds UserDefaults key for cellular data cleanup scheduling |
| Modules/Server/.../UploadManager+URLSessionDelegate.swift | Records background upload bytes using URLSession metrics |
| Modules/Server/.../BackgroundSyncManager+URLSession.swift | Records background sync bytes (download + upload) using URLSession metrics |
| Modules/DataModel/.../DataManager.swift | Exposes cellularDataUsageManager off the shared DataManager |
| Modules/DataModel/.../CellularDataUsageRecord.swift | Adds GRDB record model for CellularDataUsage table |
| Modules/DataModel/.../CellularDataUsageManager.swift | Adds DB insert/query/weekly aggregation + retention delete API |
| Modules/DataModel/.../DatabaseHelper.swift | Adds schema migration (v72) to create CellularDataUsage table + indexes |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+14
to
+18
| let isCellular = metrics.transactionMetrics.last?.isCellular ?? false | ||
|
|
||
| if bytesSent > 0 { | ||
| let connectionType: CellularDataUsageManager.ConnectionType = isCellular ? .cellular : .wifi | ||
|
|
Comment on lines
+887
to
+905
| try db.executeUpdate(""" | ||
| CREATE TABLE IF NOT EXISTS CellularDataUsage ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| timestamp REAL NOT NULL, | ||
| episode_uuid TEXT, | ||
| podcast_uuid TEXT, | ||
| bytes_downloaded INTEGER NOT NULL DEFAULT 0, | ||
| bytes_streamed INTEGER NOT NULL DEFAULT 0, | ||
| bytes_uploaded INTEGER NOT NULL DEFAULT 0, | ||
| operation_type TEXT NOT NULL, | ||
| connection_type INTEGER NOT NULL, | ||
| session_type TEXT | ||
| ); | ||
| """, values: nil) | ||
|
|
||
| try db.executeUpdate("CREATE INDEX IF NOT EXISTS cellular_data_timestamp ON CellularDataUsage (timestamp);", values: nil) | ||
| try db.executeUpdate("CREATE INDEX IF NOT EXISTS cellular_data_episode ON CellularDataUsage (episode_uuid);", values: nil) | ||
| try db.executeUpdate("CREATE INDEX IF NOT EXISTS cellular_data_connection ON CellularDataUsage (connection_type);", values: nil) | ||
|
|
Comment on lines
+115
to
+119
| let bytesThreshold: Int64 = 100 * 1024 | ||
| if bytesOnConnection - lastReportedBytes >= bytesThreshold { | ||
| let bytesToReport = bytesOnConnection - lastReportedBytes | ||
| reportConnectionBytes(bytesToReport) | ||
| lastReportedBytes = bytesOnConnection |
Comment on lines
+185
to
+188
| let isCellular = metrics.transactionMetrics.last?.isCellular ?? false | ||
|
|
||
| guard bytesReceived > 0 else { return } | ||
|
|
Comment on lines
+9
to
+13
| let isCellular = metrics.transactionMetrics.last?.isCellular ?? false | ||
|
|
||
| if bytesReceived > 0 || bytesSent > 0 { | ||
| let connectionType: CellularDataUsageManager.ConnectionType = isCellular ? .cellular : .wifi | ||
|
|
Comment on lines
+214
to
+216
| let bytesReceived = task.countOfBytesReceived | ||
| let isCellular = metrics.transactionMetrics.last?.isCellular ?? false | ||
|
|
Comment on lines
+61
to
+65
| func stopTracking() { | ||
| reportCurrentConnectionUsageIfNeeded() | ||
|
|
||
| monitor.cancel() | ||
|
|
| accessLogObserver = NotificationCenter.default.addObserver( | ||
| forName: .AVPlayerItemNewAccessLogEntry, | ||
| object: playerItem, | ||
| queue: nil |
Comment on lines
+152
to
+156
| guard let accessLog = playerItem?.accessLog(), | ||
| let lastEvent = accessLog.events.last else { | ||
| return 0 | ||
| } | ||
| return lastEvent.numberOfBytesTransferred |
Comment on lines
+65
to
+69
| public func totalDataUsage(since date: Date, connectionType: ConnectionType? = nil) -> Int64 { | ||
| let since = date.timeIntervalSince1970 | ||
| var total: Int64 = 0 | ||
|
|
||
| dbQueue.read { db in |
Adds database infrastructure for tracking data usage: - CellularDataUsageRecord GRDB model with operation type, connection type, and bytes - CellularDataUsageManager with record/query methods - Weekly aggregation queries for reporting
Records bytes transferred during episode downloads, distinguishing between user-initiated and automatic downloads.
Records bytes transferred during custom episode uploads.
Records bytes transferred during background sync operations.
Adds StreamingCellularTracker to monitor bytes received during streaming. Integrates with DefaultPlayer and MediaExporter for tracking streamed audio data.
Removes records older than 60 days during sync to prevent unbounded database growth.
3023bb9 to
6e30a5f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of PCIOS-562
Summary
Changes
Database Schema
CellularDataUsageRecord: GRDB model storing operation type, connection type, bytes, and timestampCellularDataUsageManager: Records usage and provides query methods including weekly aggregationsTracking Integration
DownloadManager: Tracks bytes for user-initiated and automatic episode downloadsUploadManager: Tracks bytes for custom episode uploadsBackgroundSyncManager: Tracks bytes during background sync operationsStreamingCellularTracker: Monitors bytes received during streaming playback viaDefaultPlayerandMediaExporterResourceLoaderDelegateTo test
Checklist
CHANGELOG.mdif necessary.