[5.5][Collections] populateTargetTrie changes (#3637)

* [Collections] Disable target trie in some of the tests

Motivation:
Some of the package collection tests don't require search and finish quickly, so when background thread tries to `populateTargetTrie` it results in signal 4.

For [example](#3623 (comment)):

```
10:23:08 Test Suite 'PackageCollectionsTests' started at 2021-07-20 10:21:53.935
10:23:08 Test Case '-[PackageCollectionsTests.PackageCollectionsTests testCollectionPendingTrustConfirmIsKeptOnAdd]' started.
10:23:08 Test Case '-[PackageCollectionsTests.PackageCollectionsTests testCollectionPendingTrustConfirmIsKeptOnAdd]' passed (0.105 seconds).
10:23:08 Exited with signal code 4
```

Modifications:
Set `initializeTargetTrie` to `false` for these tests such that `populateTargetTrie` is not called.

* [Collections] Don't queue populateTargetTrie if there is no data

Motivation:
`populateTargetTrie`, potentially because it's done in a background queue, causes a lot of random crashes in tests (not just in SwiftPM but other projects as well). The exception happens inside SQLite C library and it's not yet clear what the real cause is. However, since most tests do not use the package collections feature at all, `populateTargetTrie` is basically a no-op and therefore doesn't need to be queued.

Modification:
Queue `populateTargetTrie` only if there is package collection data.

This code change doesn't actually fix anything but may perhaps reduce the chance of crashing.

rdar://80840989

Author: yim-lee

Sources/PackageCollections/Storage/SQLitePackageCollectionsStorage.swift

@@ -801,6 +801,27 @@ final class SQLitePackageCollectionsStorage: PackageCollectionsStorage, Closable
     }
 
     internal func populateTargetTrie(callback: @escaping (Result<Void, Error>) -> Void = { _ in }) {
+        // Check to see if there is any data before submitting task to queue because otherwise it's no-op anyway
+        do {
+            let numberOfCollections: Int = try self.executeStatement("SELECT COUNT(*) FROM \(Self.packageCollectionsTableName);") { statement in
+                let row = try statement.step()
+                guard let count = row?.int(at: 0) else {
+                    throw StringError("Failed to get count of \(Self.packageCollectionsTableName) table")
+                }
+                return count
+            }
+            // No collections means no data, so no need to populate target trie
+            guard numberOfCollections > 0 else {
+                self.populateTargetTrieLock.withLock {
+                    self.targetTrieReady = true
+                }
+                return callback(.success(()))
+            }
+        } catch {
+            self.diagnosticsEngine?.emit(warning: "Failed to determine if database is empty or not: \(error)")
+            // Try again in background
+        }
+
         DispatchQueue.sharedConcurrent.async(group: nil, qos: .background, flags: .assignCurrentContext) {
             do {
                 try self.populateTargetTrieLock.withLock { // Prevent race to populate targetTrie

Tests/PackageCollectionsTests/PackageCollectionsTests.swift

@@ -23,7 +23,7 @@ final class PackageCollectionsTests: XCTestCase {
         let authTokens = ThreadSafeKeyValueStore<AuthTokenType, String>()
         let configuration = PackageCollections.Configuration(authTokens: { authTokens.get() })
 
-        // This test doesn't use storage at all and finishes quickly so disable target trie to prevent race
+        // This test doesn't use search at all and finishes quickly so disable target trie to prevent race
         let storageConfig = SQLitePackageCollectionsStorage.Configuration(initializeTargetTrie: false)
         let storage = makeMockStorage(storageConfig)
         defer { XCTAssertNoThrow(try storage.close()) }
@@ -156,7 +156,9 @@ final class PackageCollectionsTests: XCTestCase {
         try skipIfUnsupportedPlatform()
 
         let configuration = PackageCollections.Configuration()
-        let storage = makeMockStorage()
+        // This test doesn't use search at all and finishes quickly so disable target trie to prevent race
+        let storageConfig = SQLitePackageCollectionsStorage.Configuration(initializeTargetTrie: false)
+        let storage = makeMockStorage(storageConfig)
         defer { XCTAssertNoThrow(try storage.close()) }
 
         let mockCollection = makeMockCollections(count: 1).first!
@@ -174,7 +176,8 @@ final class PackageCollectionsTests: XCTestCase {
         }
 
         // add fails because collection is not found
-        guard case .failure = tsc_await({ callback in packageCollections.addCollection(mockCollection.source, order: nil, callback: callback) }) else {
+        guard case .failure(let error) = tsc_await({ callback in packageCollections.addCollection(mockCollection.source, order: nil, callback: callback) }),
+            error is NotFoundError else {
             return XCTFail("expected error")
         }
 
@@ -191,7 +194,9 @@ final class PackageCollectionsTests: XCTestCase {
         try skipIfUnsupportedPlatform()
 
         let configuration = PackageCollections.Configuration()
-        let storage = makeMockStorage()
+        // This test doesn't use search at all and finishes quickly so disable target trie to prevent race
+        let storageConfig = SQLitePackageCollectionsStorage.Configuration(initializeTargetTrie: false)
+        let storage = makeMockStorage(storageConfig)
         defer { XCTAssertNoThrow(try storage.close()) }
 
         let mockCollection = makeMockCollections(count: 1, signed: false).first!
@@ -208,7 +213,9 @@ final class PackageCollectionsTests: XCTestCase {
             XCTAssertEqual(sources.count, 0, "sources should be empty")
         }
 
-        guard case .failure = tsc_await({ callback in packageCollections.addCollection(mockCollection.source, order: nil, callback: callback) }) else {
+        // add fails because collection requires trust confirmation
+        guard case .failure(let error) = tsc_await({ callback in packageCollections.addCollection(mockCollection.source, order: nil, callback: callback) }),
+            case PackageCollectionError.trustConfirmationRequired = error else {
             return XCTFail("expected error")
         }
 
@@ -225,7 +232,9 @@ final class PackageCollectionsTests: XCTestCase {
         try skipIfUnsupportedPlatform()
 
         let configuration = PackageCollections.Configuration()
-        let storage = makeMockStorage()
+        // This test doesn't use search at all and finishes quickly so disable target trie to prevent race
+        let storageConfig = SQLitePackageCollectionsStorage.Configuration(initializeTargetTrie: false)
+        let storage = makeMockStorage(storageConfig)
         defer { XCTAssertNoThrow(try storage.close()) }
 
         let mockCollection = makeMockCollections(count: 1).first!
@@ -244,7 +253,7 @@ final class PackageCollectionsTests: XCTestCase {
 
         // add fails because collection's signature is invalid
         guard case .failure(let error) = tsc_await({ callback in packageCollections.addCollection(mockCollection.source, order: nil, callback: callback) }),
-            PackageCollectionError.invalidSignature == error as? PackageCollectionError else {
+            case PackageCollectionError.invalidSignature = error else {
             return XCTFail("expected PackageCollectionError.invalidSignature")
         }