Skip to content

Commit 426295a

Browse files
authored
[Collections] refreshCollection should have correct source config (#3314)
Motivation: `refreshCollection` relies on `CollectionSource` argument to provide the correct source config, but it should: 1. Check that the source exists in the config 2. Read latest source config from storage Modifications: Update `refreshCollection` to check that the given source exists in config file before actually doing a refresh. Also make sure that `Collection`'s `source` is current before returning it.
1 parent f73c0c7 commit 426295a

File tree

3 files changed

+52
-3
lines changed

3 files changed

+52
-3
lines changed

Sources/PackageCollections/Model/Collection.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ extension PackageCollectionsModel {
3131
public let identifier: Identifier
3232

3333
/// Where the collection and its contents are obtained
34-
public let source: Source
34+
public internal(set) var source: Source
3535

3636
/// The name of the collection
3737
public let name: String

Sources/PackageCollections/PackageCollections.swift

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,17 @@ public struct PackageCollections: PackageCollectionsProtocol {
158158
return callback(.failure(PackageCollectionError.unsupportedPlatform))
159159
}
160160

161-
self.refreshCollectionFromSource(source: source, trustConfirmationProvider: nil, callback: callback)
161+
self.storage.sources.list { result in
162+
switch result {
163+
case .failure(let error):
164+
callback(.failure(error))
165+
case .success(let sources):
166+
guard let savedSource = sources.first(where: { $0 == source }) else {
167+
return callback(.failure(NotFoundError("\(source)")))
168+
}
169+
self.refreshCollectionFromSource(source: savedSource, trustConfirmationProvider: nil, callback: callback)
170+
}
171+
}
162172
}
163173

164174
public func addCollection(_ source: PackageCollectionsModel.CollectionSource,
@@ -389,7 +399,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
389399
switch result {
390400
case .failure(let error):
391401
callback(.failure(error))
392-
case .success(let collection):
402+
case .success(var collection):
393403
// If collection is signed and signature is valid, save to storage. `provider.get`
394404
// would have failed if signature were invalid.
395405
if collection.isSigned {
@@ -426,6 +436,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
426436
callback(.failure(error))
427437
case .success:
428438
if userTrusted {
439+
collection.source = source
429440
self.storage.collections.put(collection: collection, callback: callback)
430441
} else {
431442
// Try to remove the untrusted collection (if previously saved) from storage before calling back

Tests/PackageCollectionsTests/PackageCollectionsTests.swift

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,44 @@ final class PackageCollectionsTests: XCTestCase {
977977
XCTAssertEqual(list.count, mockCollections.count, "list count should match")
978978
}
979979

980+
func testRefreshOneTrustedUnsigned() throws {
981+
try skipIfUnsupportedPlatform()
982+
983+
let configuration = PackageCollections.Configuration()
984+
let storage = makeMockStorage()
985+
defer { XCTAssertNoThrow(try storage.close()) }
986+
987+
let mockCollections = makeMockCollections(count: 1, signed: false)
988+
let collectionProviders = [PackageCollectionsModel.CollectionSourceType.json: MockCollectionsProvider(mockCollections)]
989+
let metadataProvider = MockMetadataProvider([:])
990+
let packageCollections = PackageCollections(configuration: configuration, storage: storage, collectionProviders: collectionProviders, metadataProvider: metadataProvider)
991+
992+
// User trusted
993+
let collection = try tsc_await { callback in packageCollections.addCollection(mockCollections[0].source, order: nil, trustConfirmationProvider: { _, cb in cb(true) }, callback: callback) }
994+
XCTAssertEqual(true, collection.source.isTrusted) // isTrusted is nil-able
995+
996+
// `isTrusted` should be true so refreshCollection should succeed
997+
XCTAssertNoThrow(try tsc_await { callback in packageCollections.refreshCollection(collection.source, callback: callback) })
998+
}
999+
1000+
func testRefreshOneNotFound() throws {
1001+
try skipIfUnsupportedPlatform()
1002+
1003+
let configuration = PackageCollections.Configuration()
1004+
let storage = makeMockStorage()
1005+
defer { XCTAssertNoThrow(try storage.close()) }
1006+
1007+
let mockCollections = makeMockCollections(count: 1, signed: false)
1008+
let collectionProviders = [PackageCollectionsModel.CollectionSourceType.json: MockCollectionsProvider(mockCollections)]
1009+
let metadataProvider = MockMetadataProvider([:])
1010+
let packageCollections = PackageCollections(configuration: configuration, storage: storage, collectionProviders: collectionProviders, metadataProvider: metadataProvider)
1011+
1012+
// Don't add collection so it's not found in the config
1013+
XCTAssertThrowsError(try tsc_await { callback in packageCollections.refreshCollection(mockCollections[0].source, callback: callback) }, "expected error") { error in
1014+
XCTAssert(error is NotFoundError)
1015+
}
1016+
}
1017+
9801018
func testListTargets() throws {
9811019
try skipIfUnsupportedPlatform()
9821020

0 commit comments

Comments
 (0)