Merge pull request #55 from SwiftPackageIndex/cleanup

Cleanup
SwiftPackageIndex · Jan 5, 2024 · 593b371 · 593b371
2 parents bdc08ad + e64250f
commit 593b371
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 81 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -24,7 +24,21 @@ env:
   SPI_API_TOKEN: ${{ secrets.SPI_API_TOKEN }}
 
 jobs:
-  test-validator-Linux:
+  swift-test-Linux:
+    runs-on: ubuntu-latest
+    container:
+      image: swift:5.9-jammy
+    steps:
+      - name: Get swift version
+        run: swift --version
+
+      - name: Checkout
+        uses: actions/checkout@v3
+
+      - name: Run tests
+        run: swift test
+
+  end-to-end-test-Linux:
     runs-on: ubuntu-latest
     container:
       image: swift:5.9-jammy
@@ -36,8 +50,7 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Build validator
-        run: |
-          swift build
+        run: swift build
 
       - name: Install curl
         run: apt-get update && apt-get install -y curl

diff --git a/Package.resolved b/Package.resolved
@@ -5,17 +5,17 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/swift-server/async-http-client.git",
       "state" : {
-        "revision" : "16f7e62c08c6969899ce6cc277041e868364e5cf",
-        "version" : "1.19.0"
+        "revision" : "5ccda442f103792d67680aefc8d0a87392fbd66c",
+        "version" : "1.20.0"
       }
     },
     {
       "identity" : "canonicalpackageurl",
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/SwiftPackageIndex/CanonicalPackageURL.git",
       "state" : {
-        "revision" : "1588003a7b745ae899d9e9c9869187e6883bf129",
-        "version" : "0.0.7"
+        "revision" : "0652109907f980a4020e158f67d154424e3caa30",
+        "version" : "0.0.8"
       }
     },
     {
@@ -68,8 +68,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/apple/swift-collections.git",
       "state" : {
-        "revision" : "a902f1823a7ff3c9ab2fba0f992396b948eda307",
-        "version" : "1.0.5"
+        "revision" : "d029d9d39c87bed85b1c50adee7c41795261a192",
+        "version" : "1.0.6"
       }
     },
     {

diff --git a/Sources/ValidatorCore/Commands/CheckDependencies.swift b/Sources/ValidatorCore/Commands/CheckDependencies.swift
@@ -69,7 +69,7 @@ public struct CheckDependencies: AsyncParsableCommand {
 
             // resolve redirects
             print("Processing:", dep.packageURL, "...")
-            guard let resolved = await Current.resolvePackageRedirectsAsync(dep.packageURL).url else {
+            guard let resolved = await Current.resolvePackageRedirects(dep.packageURL).url else {
                 // TODO: consider adding retry for some errors
                 print("  ... ⛔ redirect resolution returned nil")
                 continue
@@ -85,7 +85,7 @@ public struct CheckDependencies: AsyncParsableCommand {
             }
 
             do {  // run package dump to validate
-                let repo = try await Current.fetchRepositoryAsync(client, resolved)
+                let repo = try await Current.fetchRepository(client, resolved)
                 let manifest = try await Package.getManifestURL(client: client, repository: repo)
                 _ = try Current.decodeManifest(manifest)
             } catch {

diff --git a/Sources/ValidatorCore/Environment.swift b/Sources/ValidatorCore/Environment.swift
@@ -23,13 +23,9 @@ struct Environment {
     var fileManager: FileManager
     var fetch: (_ client: HTTPClient, _ url: URL) -> EventLoopFuture<ByteBuffer>
     var fetchDependencies: (_ api: SwiftPackageIndexAPI) async throws -> [SwiftPackageIndexAPI.PackageRecord]
-    var fetchRepository: (_ client: HTTPClient,
-                          _ owner: String,
-                          _ repository: String) -> EventLoopFuture<Github.Repository>
-    var fetchRepositoryAsync: (_ client: HTTPClient, _ url: PackageURL) async throws -> Github.Repository
+    var fetchRepository: (_ client: HTTPClient, _ url: PackageURL) async throws -> Github.Repository
     var githubToken: () -> String?
-    var resolvePackageRedirects: (EventLoop, PackageURL) -> EventLoopFuture<Redirect>
-    var resolvePackageRedirectsAsync: (PackageURL) async -> Redirect
+    var resolvePackageRedirects: (PackageURL) async -> Redirect
     var shell: Shell
 }
 
@@ -40,11 +36,9 @@ extension Environment {
         fileManager: .live,
         fetch: Github.fetch(client:url:),
         fetchDependencies: { try await $0.fetchDependencies() },
-        fetchRepository: Github.fetchRepository(client:owner:repository:),
-        fetchRepositoryAsync: Github.fetchRepository(client:url:),
+        fetchRepository: Github.fetchRepository(client:url:),
         githubToken: { ProcessInfo.processInfo.environment["GITHUB_TOKEN"] },
-        resolvePackageRedirects: resolveRedirects(eventLoop:for:),
-        resolvePackageRedirectsAsync: resolveRedirects(for:),
+        resolvePackageRedirects: resolveRedirects(for:),
         shell: .live
     )
 
@@ -53,16 +47,9 @@ extension Environment {
         fileManager: .mock,
         fetch: { client, _ in client.eventLoopGroup.next().makeFailedFuture(AppError.runtimeError("unimplemented")) },
         fetchDependencies: { _ in [] },
-        fetchRepository: { client, _, _ in
-            client.eventLoopGroup.next()
-                .makeSucceededFuture( Github.Repository(defaultBranch: "main", owner: "foo", name: "bar") )
-        },
-        fetchRepositoryAsync: { _, _ in .init(defaultBranch: "main", owner: "foo", name: "bar") },
+        fetchRepository: { _, _ in .init(defaultBranch: "main", owner: "foo", name: "bar") },
         githubToken: { nil },
-        resolvePackageRedirects: { eventLoop, url in
-            eventLoop.makeSucceededFuture(.initial(url))
-        },
-        resolvePackageRedirectsAsync: { .initial($0) },
+        resolvePackageRedirects: { .initial($0) },
         shell: .mock
     )
 }

diff --git a/Sources/ValidatorCore/Github.swift b/Sources/ValidatorCore/Github.swift
@@ -134,32 +134,6 @@ extension Github {
     static var repositoryCache = Cache<Repository>()
 
 
-    static func fetchRepository(client: HTTPClient, owner: String, repository: String) -> EventLoopFuture<Repository> {
-        let url = URL(string: "https://api.github.com/repos/\(owner)/\(repository)")!
-        if let cached = repositoryCache[Cache.Key(string: url.absoluteString)] {
-            return client.eventLoopGroup.next().makeSucceededFuture(cached)
-        }
-        let promise = client.eventLoopGroup.next().makePromise(of: Repository.self)
-        promise.completeWithTask {
-            try await fetch(Repository.self, client: client, url: url)
-        }
-        return promise.futureResult
-            .flatMapError { error in
-                let eventLoop = client.eventLoopGroup.next()
-                if case AppError.requestFailed(_, 404) = error {
-                    return eventLoop.makeFailedFuture(
-                        AppError.repositoryNotFound(owner: owner, name: repository)
-                    )
-                }
-                return eventLoop.makeFailedFuture(error)
-            }
-            .map { repo in
-                repositoryCache[Cache.Key(string: url.absoluteString)] = repo
-                return repo
-            }
-    }
-
-
     static func fetchRepository(client: HTTPClient, url: PackageURL) async throws-> Repository {
         try await fetchRepository(client: client, url: url, attempt: 0)
     }
@@ -184,27 +158,6 @@ extension Github {
     }
 
 
-    /// Fetch repositories for a collection of package urls. Repository is `nil` for package urls that are not found (404).
-    /// - Parameters:
-    ///   - client: http client
-    ///   - urls: list of package urls
-    /// - Returns: list of `(PackageURL, Repository?)` pairs
-    static func fetchRepositories(client: HTTPClient, urls: [PackageURL]) -> EventLoopFuture<[(PackageURL, Repository?)]> {
-        let req: [EventLoopFuture<(PackageURL, Repository?)>] = urls.map { url -> EventLoopFuture<(PackageURL, Repository?)> in
-            Current.fetchRepository(client, url.owner, url.repository)
-                .map { (url, $0) }
-                .flatMapError { error -> EventLoopFuture<(PackageURL, Repository?)> in
-                    // convert 'repository not found' into nil value
-                    if case AppError.repositoryNotFound = error {
-                        return client.eventLoopGroup.next().makeSucceededFuture((url, nil))
-                    }
-                    return client.eventLoopGroup.next().makeFailedFuture(error)
-                }
-        }
-        return EventLoopFuture.whenAllSucceed(req, on: client.eventLoopGroup.next())
-    }
-
-
     static func listRepositoryFilePaths(client: HTTPClient, repository: Repository) async throws -> [String] {
         let apiURL = URL( string: "https://api.github.com/repos/\(repository.path)/git/trees/\(repository.defaultBranch)" )!
         struct Response: Decodable {

diff --git a/...idatorTests/CheckDependencies2Tests.swift → ...lidatorTests/CheckDependenciesTests.swift b/...idatorTests/CheckDependencies2Tests.swift → ...lidatorTests/CheckDependenciesTests.swift
@@ -20,14 +20,15 @@ import CanonicalPackageURL
 import NIO
 
 
-final class CheckDependencies2Tests: XCTestCase {
+final class CheckDependenciesTests: XCTestCase {
     var check = CheckDependencies()
 
     override func setUp() {
         super.setUp()
         check.apiBaseURL = "unused"
         check.input = nil
         check.limit = .max
+        check.maxCheck = .max
         check.spiApiToken = "unused"
         check.output = "unused"
     }
@@ -56,7 +57,7 @@ final class CheckDependencies2Tests: XCTestCase {
             saved = list
             return true
         }
-        Current.fetchRepositoryAsync = { _, url in
+        Current.fetchRepository = { _, url in
             if url == PackageURL.p3 {
                 return .init(defaultBranch: "main", owner: "org", name: "3")
             } else {
@@ -114,7 +115,7 @@ final class CheckDependencies2Tests: XCTestCase {
             saved = list
             return true
         }
-        Current.fetchRepositoryAsync = { _, url in
+        Current.fetchRepository = { _, url in
             if url == PackageURL.p3 {
                 return .init(defaultBranch: "main", owner: "org", name: "3")
             } else {
@@ -162,7 +163,7 @@ final class CheckDependencies2Tests: XCTestCase {
             saved = list
             return true
         }
-        Current.fetchRepositoryAsync = { _, url in
+        Current.fetchRepository = { _, url in
             if url == PackageURL.p3 {
                 return .init(defaultBranch: "main", owner: "org", name: "3")
             } else {

diff --git a/Tests/ValidatorTests/GithubTests.swift b/Tests/ValidatorTests/GithubTests.swift
@@ -22,6 +22,10 @@ import NIO
 
 final class GithubTests: XCTestCase {
 
+    override func setUp() async throws {
+        Github.repositoryCache = .init()
+    }
+
     func test_fetchRepository_retry() async throws {
         // setup
         let client = HTTPClient(eventLoopGroupProvider: .singleton)