Fix 7 data mutation layer risks identified in audit
- save()/saveAndRunDataListeners() now return @discardableResult Bool; listeners only fire on successful save - MoodLogger.updateMood() now recalculates streak, updates Live Activity, and notifies watch (was missing these side effects) - CSV import uses new addBatch()/importMoods() for O(1) side effects instead of O(n) per-row widget reloads and streak calcs - Foreground task ordering: fillInMissingDates() now runs before removeDuplicates() so backfill-created duplicates are caught same cycle - WidgetMoodSaver deletes ALL entries for date (was fetchLimit=1, leaving CloudKit sync duplicates behind) - cleanupPhotoIfNeeded logs warning on failed photo deletion instead of silently orphaning files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -52,19 +52,32 @@ final class DataController: ObservableObject {
|
||||
editedDataClosure.append(closure)
|
||||
}
|
||||
|
||||
func saveAndRunDataListeners() {
|
||||
save()
|
||||
for closure in editedDataClosure {
|
||||
closure()
|
||||
@discardableResult
|
||||
func saveAndRunDataListeners() -> Bool {
|
||||
let success = save()
|
||||
if success {
|
||||
for closure in editedDataClosure {
|
||||
closure()
|
||||
}
|
||||
}
|
||||
return success
|
||||
}
|
||||
|
||||
func save() {
|
||||
guard modelContext.hasChanges else { return }
|
||||
@discardableResult
|
||||
func save() -> Bool {
|
||||
guard modelContext.hasChanges else { return true }
|
||||
do {
|
||||
try modelContext.save()
|
||||
return true
|
||||
} catch {
|
||||
Self.logger.error("Failed to save context: \(error.localizedDescription)")
|
||||
Self.logger.error("Failed to save context, retrying: \(error.localizedDescription)")
|
||||
do {
|
||||
try modelContext.save()
|
||||
return true
|
||||
} catch {
|
||||
Self.logger.critical("Failed to save context after retry: \(error.localizedDescription)")
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,16 +7,24 @@
|
||||
|
||||
import SwiftData
|
||||
import Foundation
|
||||
import os.log
|
||||
|
||||
private let logger = Logger(subsystem: Bundle.main.bundleIdentifier ?? "com.tt.feels", category: "DataControllerADD")
|
||||
|
||||
extension DataController {
|
||||
func add(mood: Mood, forDate date: Date, entryType: EntryType) {
|
||||
// Delete ALL existing entries for this date (handles duplicates)
|
||||
let existing = getAllEntries(byDate: date)
|
||||
for entry in existing {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
modelContext.delete(entry)
|
||||
}
|
||||
if !existing.isEmpty {
|
||||
try? modelContext.save()
|
||||
do {
|
||||
try modelContext.save()
|
||||
} catch {
|
||||
logger.error("Failed to save after deleting existing entries: \(error.localizedDescription)")
|
||||
}
|
||||
}
|
||||
|
||||
let entry = MoodEntryModel(
|
||||
@@ -34,21 +42,32 @@ extension DataController {
|
||||
let currentOnboarding = UserDefaultsStore.getOnboarding()
|
||||
var endDate = ShowBasedOnVoteLogics.getCurrentVotingDate(onboardingData: currentOnboarding)
|
||||
// Since it's for views, take away the last date so vote is enabled
|
||||
endDate = Calendar.current.date(byAdding: .day, value: -1, to: endDate)!
|
||||
guard let adjustedEndDate = Calendar.current.date(byAdding: .day, value: -1, to: endDate) else {
|
||||
logger.error("Failed to calculate adjusted end date")
|
||||
return
|
||||
}
|
||||
endDate = adjustedEndDate
|
||||
|
||||
let descriptor = FetchDescriptor<MoodEntryModel>(
|
||||
sortBy: [SortDescriptor(\.forDate, order: .reverse)]
|
||||
)
|
||||
|
||||
guard let entries = try? modelContext.fetch(descriptor),
|
||||
let firstEntry = entries.last else { return }
|
||||
|
||||
let allDates: [Date] = Date.dates(from: firstEntry.forDate, toDate: endDate, includingToDate: true).map {
|
||||
Calendar.current.date(bySettingHour: 0, minute: 0, second: 0, of: $0)!
|
||||
let entries: [MoodEntryModel]
|
||||
do {
|
||||
entries = try modelContext.fetch(descriptor)
|
||||
} catch {
|
||||
logger.error("Failed to fetch entries for fill-in: \(error.localizedDescription)")
|
||||
return
|
||||
}
|
||||
|
||||
let existingDates: Set<Date> = Set(entries.map {
|
||||
Calendar.current.date(bySettingHour: 0, minute: 0, second: 0, of: $0.forDate)!
|
||||
guard let firstEntry = entries.last else { return }
|
||||
|
||||
let allDates: [Date] = Date.dates(from: firstEntry.forDate, toDate: endDate, includingToDate: true).compactMap {
|
||||
Calendar.current.date(bySettingHour: 0, minute: 0, second: 0, of: $0)
|
||||
}
|
||||
|
||||
let existingDates: Set<Date> = Set(entries.compactMap {
|
||||
Calendar.current.date(bySettingHour: 0, minute: 0, second: 0, of: $0.forDate)
|
||||
})
|
||||
|
||||
let missing = Array(Set(allDates).subtracting(existingDates)).sorted(by: >)
|
||||
@@ -58,7 +77,10 @@ extension DataController {
|
||||
// Batch insert all missing dates without triggering listeners
|
||||
for date in missing {
|
||||
// Add 12 hours to avoid UTC offset issues
|
||||
let adjustedDate = Calendar.current.date(byAdding: .hour, value: 12, to: date)!
|
||||
guard let adjustedDate = Calendar.current.date(byAdding: .hour, value: 12, to: date) else {
|
||||
logger.error("Failed to calculate adjusted date for \(date)")
|
||||
continue
|
||||
}
|
||||
let entry = MoodEntryModel(
|
||||
forDate: adjustedDate,
|
||||
mood: .missing,
|
||||
@@ -77,11 +99,26 @@ extension DataController {
|
||||
for entry in data {
|
||||
entry.weekDay = Calendar.current.component(.weekday, from: entry.forDate)
|
||||
}
|
||||
save()
|
||||
saveAndRunDataListeners()
|
||||
}
|
||||
|
||||
func removeNoForDates() {
|
||||
// Note: With SwiftData's non-optional forDate, this is essentially a no-op
|
||||
// Keeping for API compatibility
|
||||
}
|
||||
|
||||
/// Batch insert mood entries without per-entry analytics or listener notifications.
|
||||
/// Used for CSV import where side effects should fire once at the end.
|
||||
func addBatch(entries: [(mood: Mood, date: Date, entryType: EntryType)]) {
|
||||
for (mood, date, entryType) in entries {
|
||||
let existing = getAllEntries(byDate: date)
|
||||
for entry in existing {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
modelContext.delete(entry)
|
||||
}
|
||||
let entry = MoodEntryModel(forDate: date, mood: mood, entryType: entryType)
|
||||
modelContext.insert(entry)
|
||||
}
|
||||
saveAndRunDataListeners()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,10 +10,32 @@ import Foundation
|
||||
import os.log
|
||||
|
||||
extension DataController {
|
||||
|
||||
// MARK: - Photo Cleanup
|
||||
|
||||
func cleanupPhotoIfNeeded(for entry: MoodEntryModel) {
|
||||
if let photoID = entry.photoID {
|
||||
if !PhotoManager.shared.deletePhoto(id: photoID) {
|
||||
AppLogger.general.warning("Failed to delete orphaned photo \(photoID.uuidString) for entry on \(entry.forDate)")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Delete Operations
|
||||
|
||||
func clearDB() {
|
||||
do {
|
||||
// Clean up photo files before batch delete
|
||||
let descriptor = FetchDescriptor<MoodEntryModel>()
|
||||
if let allEntries = try? modelContext.fetch(descriptor) {
|
||||
for entry in allEntries {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
}
|
||||
}
|
||||
|
||||
try modelContext.delete(model: MoodEntryModel.self)
|
||||
saveAndRunDataListeners()
|
||||
AnalyticsManager.shared.track(.allDataCleared)
|
||||
} catch {
|
||||
AppLogger.general.error("Failed to clear database: \(error.localizedDescription)")
|
||||
}
|
||||
@@ -24,9 +46,10 @@ extension DataController {
|
||||
let entries = getData(startDate: startDate, endDate: Date(), includedDays: [])
|
||||
|
||||
for entry in entries {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
modelContext.delete(entry)
|
||||
}
|
||||
save()
|
||||
saveAndRunDataListeners()
|
||||
}
|
||||
|
||||
func deleteRandomFromLast(numberOfEntries: Int) {
|
||||
@@ -34,9 +57,10 @@ extension DataController {
|
||||
let entries = getData(startDate: startDate, endDate: Date(), includedDays: [])
|
||||
|
||||
for entry in entries where Bool.random() {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
modelContext.delete(entry)
|
||||
}
|
||||
save()
|
||||
saveAndRunDataListeners()
|
||||
}
|
||||
|
||||
/// Get ALL entries for a specific date (not just the first one)
|
||||
@@ -46,7 +70,7 @@ extension DataController {
|
||||
|
||||
let descriptor = FetchDescriptor<MoodEntryModel>(
|
||||
predicate: #Predicate { entry in
|
||||
entry.forDate >= startDate && entry.forDate <= endDate
|
||||
entry.forDate >= startDate && entry.forDate < endDate
|
||||
},
|
||||
sortBy: [SortDescriptor(\.forDate, order: .forward)]
|
||||
)
|
||||
@@ -58,9 +82,10 @@ extension DataController {
|
||||
func deleteAllEntries(forDate date: Date) {
|
||||
let entries = getAllEntries(byDate: date)
|
||||
for entry in entries {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
modelContext.delete(entry)
|
||||
}
|
||||
save()
|
||||
saveAndRunDataListeners()
|
||||
}
|
||||
|
||||
/// Find and remove duplicate entries, keeping only the most recent for each date
|
||||
@@ -92,6 +117,7 @@ extension DataController {
|
||||
|
||||
// Keep the first (best) entry, delete the rest
|
||||
for entry in sorted.dropFirst() {
|
||||
cleanupPhotoIfNeeded(for: entry)
|
||||
modelContext.delete(entry)
|
||||
duplicatesRemoved += 1
|
||||
}
|
||||
@@ -100,6 +126,7 @@ extension DataController {
|
||||
if duplicatesRemoved > 0 {
|
||||
saveAndRunDataListeners()
|
||||
AppLogger.general.info("Removed \(duplicatesRemoved) duplicate entries")
|
||||
AnalyticsManager.shared.track(.duplicatesRemoved(count: duplicatesRemoved))
|
||||
}
|
||||
|
||||
return duplicatesRemoved
|
||||
|
||||
@@ -70,10 +70,12 @@ protocol MoodDataDeleting {
|
||||
@MainActor
|
||||
protocol MoodDataPersisting {
|
||||
/// Save pending changes
|
||||
func save()
|
||||
@discardableResult
|
||||
func save() -> Bool
|
||||
|
||||
/// Save and notify listeners
|
||||
func saveAndRunDataListeners()
|
||||
@discardableResult
|
||||
func saveAndRunDataListeners() -> Bool
|
||||
|
||||
/// Add a listener for data changes
|
||||
func addNewDataListener(closure: @escaping (() -> Void))
|
||||
|
||||
Reference in New Issue
Block a user