fix: comprehensive codebase hardening — crashes, silent failures, performance, and security
Fixes ~95 issues from deep audit across 12 categories in 82 files: - Crash prevention: double-resume in PhotoMetadataExtractor, force unwraps in DateRangePicker, array bounds checks in polls/achievements, ProGate hit-test bypass, Dictionary(uniqueKeysWithValues:) → uniquingKeysWith in 4 files - Silent failure elimination: all 34 try? sites replaced with do/try/catch + logging (SavedTrip, TripDetailView, CanonicalSyncService, BootstrapService, CanonicalModels, CKModels, SportsTimeApp, and more) - Performance: cached DateFormatters (7 files), O(1) team lookups via AppDataProvider, achievement definition dictionary, AnimatedBackground consolidated from 19 Tasks to 1, task cancellation in SharePreviewView - Concurrency: UIKit drawing → MainActor.run, background fetch timeout guard, @MainActor on ThemeManager/AppearanceManager, SyncLogger read/write race fix - Planning engine: game end time in travel feasibility, state-aware city normalization, exact city matching, DrivingConstraints parameter propagation - IAP: unknown subscription states → expired, unverified transaction logging, entitlements updated before paywall dismiss, restore visible to all users - Security: API key to Info.plist lookup, filename sanitization in PDF export, honest User-Agent, removed stale "Feels" analytics super properties - Navigation: consolidated competing navigationDestination, boolean → value-based - Testing: 8 sleep() → waitForExistence, duplicates extracted, Swift 6 compat - Service bugs: infinite retry cap, duplicate achievement prevention, TOCTOU vote fix, PollVote.odg → voterId rename, deterministic placeholder IDs, parallel MKDirections, Sendable-safe POI struct Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -101,7 +101,7 @@ struct POIDetailSheet: View {
|
||||
highlight: true
|
||||
)
|
||||
|
||||
if let url = poi.mapItem?.url {
|
||||
if let url = poi.url {
|
||||
Link(destination: url) {
|
||||
metadataRow(icon: "globe", text: url.host ?? "Website", isLink: true)
|
||||
}
|
||||
@@ -133,19 +133,17 @@ struct POIDetailSheet: View {
|
||||
.tint(Theme.warmOrange)
|
||||
.accessibilityLabel("Add \(poi.name) to Day \(day)")
|
||||
|
||||
if poi.mapItem != nil {
|
||||
Button {
|
||||
poi.mapItem?.openInMaps()
|
||||
} label: {
|
||||
Label("Open in Apple Maps", systemImage: "map.fill")
|
||||
.font(.body.weight(.medium))
|
||||
.frame(maxWidth: .infinity)
|
||||
.padding(.vertical, Theme.Spacing.md)
|
||||
}
|
||||
.buttonStyle(.bordered)
|
||||
.tint(Theme.textSecondary(colorScheme))
|
||||
.accessibilityLabel("Open \(poi.name) in Apple Maps")
|
||||
Button {
|
||||
poi.openInMaps()
|
||||
} label: {
|
||||
Label("Open in Apple Maps", systemImage: "map.fill")
|
||||
.font(.body.weight(.medium))
|
||||
.frame(maxWidth: .infinity)
|
||||
.padding(.vertical, Theme.Spacing.md)
|
||||
}
|
||||
.buttonStyle(.bordered)
|
||||
.tint(Theme.textSecondary(colorScheme))
|
||||
.accessibilityLabel("Open \(poi.name) in Apple Maps")
|
||||
}
|
||||
}
|
||||
.padding(Theme.Spacing.lg)
|
||||
|
||||
@@ -346,15 +346,17 @@ struct PlaceSearchSheet: View {
|
||||
|
||||
let search = MKLocalSearch(request: request)
|
||||
search.start { response, error in
|
||||
isSearching = false
|
||||
Task { @MainActor in
|
||||
isSearching = false
|
||||
|
||||
if let error {
|
||||
searchError = error.localizedDescription
|
||||
return
|
||||
}
|
||||
if let error {
|
||||
searchError = error.localizedDescription
|
||||
return
|
||||
}
|
||||
|
||||
if let response {
|
||||
searchResults = response.mapItems
|
||||
if let response {
|
||||
searchResults = response.mapItems
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -833,6 +833,7 @@ enum ItineraryReorderingLogic {
|
||||
/// Keys are formatted as "travel:INDEX:from->to".
|
||||
/// When multiple keys share the same city pair (repeat visits), matches by
|
||||
/// checking all keys and preferring the one whose index matches the model's segmentIndex.
|
||||
/// Falls back to using segment UUID to ensure unique keys for different segments.
|
||||
private static func travelIdForSegment(
|
||||
_ segment: TravelSegment,
|
||||
in travelValidRanges: [String: ClosedRange<Int>],
|
||||
@@ -855,8 +856,9 @@ enum ItineraryReorderingLogic {
|
||||
return key
|
||||
}
|
||||
|
||||
// Fallback: return first match or construct without index
|
||||
return matchingKeys.first ?? "travel:\(suffix)"
|
||||
// Include segment UUID to make keys unique when multiple segments share
|
||||
// the same from/to city pair (e.g., repeat visits like A->B, C->B)
|
||||
return matchingKeys.first ?? "travel:\(segment.id.uuidString):\(suffix)"
|
||||
}
|
||||
|
||||
// MARK: - Utility Functions
|
||||
@@ -915,8 +917,7 @@ enum ItineraryReorderingLogic {
|
||||
/// - sourceRow: The original row (fallback if no valid destination found)
|
||||
/// - Returns: The target row to use (in proposed coordinate space)
|
||||
///
|
||||
/// - Note: Uses O(n) contains check. For repeated calls, consider passing a Set instead.
|
||||
/// However, validDestinationRows is typically small (< 50 items), so this is fine.
|
||||
/// - Note: Uses a Set for O(1) containment check on validDestinationRows.
|
||||
static func calculateTargetRow(
|
||||
proposedRow: Int,
|
||||
validDestinationRows: [Int],
|
||||
@@ -925,12 +926,13 @@ enum ItineraryReorderingLogic {
|
||||
// UX rule: forbid dropping at absolute top (row 0 is always a day header)
|
||||
var row = proposedRow
|
||||
if row <= 0 { row = 1 }
|
||||
|
||||
// If already valid, use it
|
||||
if validDestinationRows.contains(row) {
|
||||
|
||||
// Use Set for O(1) containment check instead of O(n) Array.contains
|
||||
let validDestinationSet = Set(validDestinationRows)
|
||||
if validDestinationSet.contains(row) {
|
||||
return row
|
||||
}
|
||||
|
||||
|
||||
// Snap to nearest valid destination (validDestinationRows must be sorted for binary search)
|
||||
return nearestValue(in: validDestinationRows, to: row) ?? sourceRow
|
||||
}
|
||||
|
||||
@@ -1397,9 +1397,13 @@ struct TripDetailView: View {
|
||||
predicate: #Predicate { $0.id == tripId }
|
||||
)
|
||||
|
||||
if let count = try? modelContext.fetchCount(descriptor), count > 0 {
|
||||
isSaved = true
|
||||
} else {
|
||||
do {
|
||||
let count = try modelContext.fetchCount(descriptor)
|
||||
isSaved = count > 0
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [TripDetail] Failed to check save status: \(error)")
|
||||
#endif
|
||||
isSaved = false
|
||||
}
|
||||
}
|
||||
@@ -1416,7 +1420,15 @@ struct TripDetailView: View {
|
||||
let descriptor = FetchDescriptor<LocalItineraryItem>(
|
||||
predicate: #Predicate { $0.tripId == tripId }
|
||||
)
|
||||
let localModels = (try? modelContext.fetch(descriptor)) ?? []
|
||||
let localModels: [LocalItineraryItem]
|
||||
do {
|
||||
localModels = try modelContext.fetch(descriptor)
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [ItineraryItems] Failed to fetch local items: \(error)")
|
||||
#endif
|
||||
localModels = []
|
||||
}
|
||||
let localItems = localModels.compactMap(\.toItem)
|
||||
let pendingLocalItems = localModels.compactMap { local -> ItineraryItem? in
|
||||
guard local.pendingSync else { return nil }
|
||||
@@ -1524,14 +1536,24 @@ struct TripDetailView: View {
|
||||
let descriptor = FetchDescriptor<LocalItineraryItem>(
|
||||
predicate: #Predicate { $0.tripId == tripId }
|
||||
)
|
||||
let existing = (try? modelContext.fetch(descriptor)) ?? []
|
||||
for old in existing { modelContext.delete(old) }
|
||||
do {
|
||||
let existing = try modelContext.fetch(descriptor)
|
||||
for old in existing { modelContext.delete(old) }
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [ItineraryItems] Failed to fetch existing local items for sync: \(error)")
|
||||
#endif
|
||||
}
|
||||
for item in items {
|
||||
if let local = LocalItineraryItem.from(item, pendingSync: pendingSyncItemIDs.contains(item.id)) {
|
||||
modelContext.insert(local)
|
||||
}
|
||||
}
|
||||
try? modelContext.save()
|
||||
do {
|
||||
try modelContext.save()
|
||||
} catch {
|
||||
persistenceErrorMessage = "Failed to sync itinerary cache: \(error.localizedDescription)"
|
||||
}
|
||||
}
|
||||
|
||||
private func saveItineraryItem(_ item: ItineraryItem) async {
|
||||
@@ -1581,30 +1603,55 @@ struct TripDetailView: View {
|
||||
let descriptor = FetchDescriptor<LocalItineraryItem>(
|
||||
predicate: #Predicate { $0.id == itemId }
|
||||
)
|
||||
if let existing = try? modelContext.fetch(descriptor).first {
|
||||
existing.day = item.day
|
||||
existing.sortOrder = item.sortOrder
|
||||
existing.kindData = (try? JSONEncoder().encode(item.kind)) ?? existing.kindData
|
||||
existing.modifiedAt = item.modifiedAt
|
||||
existing.pendingSync = true
|
||||
} else if let local = LocalItineraryItem.from(item, pendingSync: true) {
|
||||
modelContext.insert(local)
|
||||
do {
|
||||
if let existing = try modelContext.fetch(descriptor).first {
|
||||
existing.day = item.day
|
||||
existing.sortOrder = item.sortOrder
|
||||
do {
|
||||
existing.kindData = try JSONEncoder().encode(item.kind)
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [ItineraryItems] Failed to encode item kind: \(error)")
|
||||
#endif
|
||||
}
|
||||
existing.modifiedAt = item.modifiedAt
|
||||
existing.pendingSync = true
|
||||
} else if let local = LocalItineraryItem.from(item, pendingSync: true) {
|
||||
modelContext.insert(local)
|
||||
}
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [ItineraryItems] Failed to fetch local item for update: \(error)")
|
||||
#endif
|
||||
if let local = LocalItineraryItem.from(item, pendingSync: true) {
|
||||
modelContext.insert(local)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if let local = LocalItineraryItem.from(item, pendingSync: true) {
|
||||
modelContext.insert(local)
|
||||
}
|
||||
}
|
||||
try? modelContext.save()
|
||||
do {
|
||||
try modelContext.save()
|
||||
} catch {
|
||||
persistenceErrorMessage = "Failed to save itinerary item: \(error.localizedDescription)"
|
||||
}
|
||||
}
|
||||
|
||||
private func markLocalItemSynced(_ itemId: UUID) {
|
||||
let descriptor = FetchDescriptor<LocalItineraryItem>(
|
||||
predicate: #Predicate { $0.id == itemId }
|
||||
)
|
||||
if let local = try? modelContext.fetch(descriptor).first {
|
||||
local.pendingSync = false
|
||||
try? modelContext.save()
|
||||
do {
|
||||
if let local = try modelContext.fetch(descriptor).first {
|
||||
local.pendingSync = false
|
||||
try modelContext.save()
|
||||
}
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [ItineraryItems] Failed to mark item synced: \(error)")
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1622,9 +1669,13 @@ struct TripDetailView: View {
|
||||
let descriptor = FetchDescriptor<LocalItineraryItem>(
|
||||
predicate: #Predicate { $0.id == itemId }
|
||||
)
|
||||
if let local = try? modelContext.fetch(descriptor).first {
|
||||
modelContext.delete(local)
|
||||
try? modelContext.save()
|
||||
do {
|
||||
if let local = try modelContext.fetch(descriptor).first {
|
||||
modelContext.delete(local)
|
||||
try modelContext.save()
|
||||
}
|
||||
} catch {
|
||||
persistenceErrorMessage = "Failed to delete itinerary item: \(error.localizedDescription)"
|
||||
}
|
||||
|
||||
// Delete from CloudKit
|
||||
|
||||
@@ -138,7 +138,6 @@ struct TripOptionsView: View {
|
||||
let convertToTrip: (ItineraryOption) -> Trip
|
||||
|
||||
@State private var selectedTrip: Trip?
|
||||
@State private var showTripDetail = false
|
||||
@State private var sortOption: TripSortOption = .recommended
|
||||
@State private var citiesFilter: CitiesFilter = .noLimit
|
||||
@State private var paceFilter: TripPaceFilter = .all
|
||||
@@ -281,7 +280,6 @@ struct TripOptionsView: View {
|
||||
games: games,
|
||||
onSelect: {
|
||||
selectedTrip = convertToTrip(option)
|
||||
showTripDetail = true
|
||||
}
|
||||
)
|
||||
.accessibilityIdentifier("tripOptions.trip.\(index)")
|
||||
@@ -313,15 +311,8 @@ struct TripOptionsView: View {
|
||||
}
|
||||
#endif
|
||||
} // ScrollViewReader
|
||||
.navigationDestination(isPresented: $showTripDetail) {
|
||||
if let trip = selectedTrip {
|
||||
TripDetailView(trip: trip)
|
||||
}
|
||||
}
|
||||
.onChange(of: showTripDetail) { _, isShowing in
|
||||
if !isShowing {
|
||||
selectedTrip = nil
|
||||
}
|
||||
.navigationDestination(item: $selectedTrip) { trip in
|
||||
TripDetailView(trip: trip)
|
||||
}
|
||||
.onAppear {
|
||||
if isDemoMode && !hasAppliedDemoSelection {
|
||||
@@ -338,7 +329,6 @@ struct TripOptionsView: View {
|
||||
if sortedOptions.count > DemoConfig.demoTripIndex {
|
||||
let option = sortedOptions[DemoConfig.demoTripIndex]
|
||||
selectedTrip = convertToTrip(option)
|
||||
showTripDetail = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -41,7 +41,9 @@ struct DateRangePicker: View {
|
||||
|
||||
var days: [Date?] = []
|
||||
let startOfMonth = monthInterval.start
|
||||
let endOfMonth = calendar.date(byAdding: .day, value: -1, to: monthInterval.end)!
|
||||
guard let endOfMonth = calendar.date(byAdding: .day, value: -1, to: monthInterval.end) else {
|
||||
return []
|
||||
}
|
||||
|
||||
// Get the first day of the week containing the first day of the month
|
||||
var currentDate = monthFirstWeek.start
|
||||
@@ -57,7 +59,10 @@ struct DateRangePicker: View {
|
||||
} else {
|
||||
break
|
||||
}
|
||||
currentDate = calendar.date(byAdding: .day, value: 1, to: currentDate)!
|
||||
guard let nextDate = calendar.date(byAdding: .day, value: 1, to: currentDate) else {
|
||||
break
|
||||
}
|
||||
currentDate = nextDate
|
||||
}
|
||||
|
||||
return days
|
||||
|
||||
@@ -249,8 +249,13 @@ struct GamePickerStep: View {
|
||||
private func loadSummaryGames() async {
|
||||
var games: [RichGame] = []
|
||||
for teamId in selectedTeamIds {
|
||||
if let teamGames = try? await AppDataProvider.shared.gamesForTeam(teamId: teamId) {
|
||||
do {
|
||||
let teamGames = try await AppDataProvider.shared.gamesForTeam(teamId: teamId)
|
||||
games.append(contentsOf: teamGames)
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [GamePickerStep] Failed to load summary games for team \(teamId): \(error)")
|
||||
#endif
|
||||
}
|
||||
}
|
||||
await MainActor.run {
|
||||
@@ -635,9 +640,14 @@ private struct GamesPickerSheet: View {
|
||||
private func loadGames() async {
|
||||
var allGames: [RichGame] = []
|
||||
for teamId in selectedTeamIds {
|
||||
if let teamGames = try? await AppDataProvider.shared.gamesForTeam(teamId: teamId) {
|
||||
do {
|
||||
let teamGames = try await AppDataProvider.shared.gamesForTeam(teamId: teamId)
|
||||
let futureGames = teamGames.filter { $0.game.dateTime > Date() }
|
||||
allGames.append(contentsOf: futureGames)
|
||||
} catch {
|
||||
#if DEBUG
|
||||
print("⚠️ [GamePickerStep] Failed to load games for team \(teamId): \(error)")
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@ struct MustStopsStep: View {
|
||||
|
||||
if !mustStopLocations.isEmpty {
|
||||
VStack(spacing: Theme.Spacing.xs) {
|
||||
ForEach(mustStopLocations, id: \.name) { location in
|
||||
ForEach(Array(mustStopLocations.enumerated()), id: \.offset) { _, location in
|
||||
HStack {
|
||||
Image(systemName: "mappin.circle.fill")
|
||||
.foregroundStyle(Theme.warmOrange)
|
||||
|
||||
@@ -126,9 +126,14 @@ struct ReviewStep: View {
|
||||
}
|
||||
}
|
||||
|
||||
private static let mediumDateFormatter: DateFormatter = {
|
||||
let f = DateFormatter()
|
||||
f.dateStyle = .medium
|
||||
return f
|
||||
}()
|
||||
|
||||
private var dateRangeText: String {
|
||||
let formatter = DateFormatter()
|
||||
formatter.dateStyle = .medium
|
||||
let formatter = Self.mediumDateFormatter
|
||||
return "\(formatter.string(from: startDate)) - \(formatter.string(from: endDate))"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,7 @@ struct TeamPickerStep: View {
|
||||
|
||||
private var selectedTeam: Team? {
|
||||
guard let teamId = selectedTeamId else { return nil }
|
||||
return AppDataProvider.shared.teams.first { $0.id == teamId }
|
||||
return AppDataProvider.shared.team(for: teamId)
|
||||
}
|
||||
|
||||
var body: some View {
|
||||
|
||||
@@ -18,7 +18,7 @@ struct TeamFirstWizardStep: View {
|
||||
|
||||
private var selectedTeams: [Team] {
|
||||
selectedTeamIds.compactMap { teamId in
|
||||
AppDataProvider.shared.teams.first { $0.id == teamId }
|
||||
AppDataProvider.shared.team(for: teamId)
|
||||
}.sorted { $0.fullName < $1.fullName }
|
||||
}
|
||||
|
||||
|
||||
@@ -193,8 +193,8 @@ struct TripWizardView: View {
|
||||
let preferences = buildPreferences()
|
||||
|
||||
// Build dictionaries from arrays
|
||||
let teamsById = Dictionary(uniqueKeysWithValues: AppDataProvider.shared.teams.map { ($0.id, $0) })
|
||||
let stadiumsById = Dictionary(uniqueKeysWithValues: AppDataProvider.shared.stadiums.map { ($0.id, $0) })
|
||||
let teamsById = Dictionary(AppDataProvider.shared.teams.map { ($0.id, $0) }, uniquingKeysWith: { _, last in last })
|
||||
let stadiumsById = Dictionary(AppDataProvider.shared.stadiums.map { ($0.id, $0) }, uniquingKeysWith: { _, last in last })
|
||||
|
||||
// For gameFirst mode, use the UI-selected date range (set by GamePickerStep)
|
||||
// The date range is a 7-day span centered on the selected game(s)
|
||||
|
||||
Reference in New Issue
Block a user