Skip to content

Commit 54dbfbd

Browse files
authored
[Collections] Trigger collection refresh if deserialization of SQLite data fails (#3305)
Motivation: When package collection model/format changes, deserializing data stored in SQLite might fail. Most of the time `SQLitePackageCollectionsStorage` just silently ignores deserialization errors, and no one would know that something had gone wrong. This could lead to collection APIs not returning any data, which would be confusing for users. What we should do with this kind of deserialization error is to trigger collection refresh so that data in SQLite gets updated. `listCollections` can do this quite easily by calling refresh whenever the results count does not equal sources count. Modifications: In `listCollections` check that sources count equals results count, otherwise trigger refresh on the "missing" collections.
1 parent 61a369b commit 54dbfbd

File tree

3 files changed

+30
-3
lines changed

3 files changed

+30
-3
lines changed

Sources/PackageCollections/Model/Collection.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ extension PackageCollectionsModel {
122122
public static func == (lhs: CollectionSource, rhs: CollectionSource) -> Bool {
123123
lhs.type == rhs.type && lhs.url == rhs.url
124124
}
125+
126+
public func hash(into hasher: inout Hasher) {
127+
hasher.combine(self.type)
128+
hasher.combine(self.url)
129+
}
125130
}
126131

127132
/// Represents the source type of a `Collection`

Sources/PackageCollections/PackageCollections.swift

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,30 @@ public struct PackageCollections: PackageCollectionsProtocol {
8484
callback(.failure(error))
8585
case .success(var collections):
8686
// re-order by profile order which reflects the user's election
87-
collections.sort(by: { lhs, rhs in collectionOrder[lhs.identifier] ?? 0 < collectionOrder[rhs.identifier] ?? 0 })
88-
callback(.success(collections))
87+
let sort = { (lhs: PackageCollectionsModel.Collection, rhs: PackageCollectionsModel.Collection) -> Bool in
88+
collectionOrder[lhs.identifier] ?? 0 < collectionOrder[rhs.identifier] ?? 0
89+
}
90+
91+
// We've fetched all the configured collections and we're done
92+
if collections.count == sources.count {
93+
collections.sort(by: sort)
94+
return callback(.success(collections))
95+
}
96+
97+
// Some of the results are missing. This happens when deserialization of stored collections fail,
98+
// so we will try refreshing the missing collections to update data in storage.
99+
let missingSources = Set(sources).subtracting(Set(collections.map { $0.source }))
100+
let refreshResults = ThreadSafeArrayStore<Result<Model.Collection, Error>>()
101+
missingSources.forEach { source in
102+
self.refreshCollectionFromSource(source: source, trustConfirmationProvider: nil) { refreshResult in
103+
let count = refreshResults.append(refreshResult)
104+
if count == missingSources.count {
105+
collections += refreshResults.compactMap { $0.success } // best-effort; not returning errors
106+
collections.sort(by: sort)
107+
callback(.success(collections))
108+
}
109+
}
110+
}
89111
}
90112
}
91113
}

Tests/PackageCollectionsTests/PackageCollectionsTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ final class PackageCollectionsTests: XCTestCase {
9898
}
9999
// User preference unknown
100100
XCTAssertThrowsError(
101-
try tsc_await { callback in packageCollections.addCollection(mockCollections[1].source, order: nil, trustConfirmationProvider: nil, callback: callback) }) { error in
101+
try tsc_await { callback in packageCollections.addCollection(mockCollections[2].source, order: nil, trustConfirmationProvider: nil, callback: callback) }) { error in
102102
guard case PackageCollectionError.trustConfirmationRequired = error else {
103103
return XCTFail("Expected PackageCollectionError.trustConfirmationRequired")
104104
}

0 commit comments

Comments
 (0)