Merge pull request #3625 from SwiftPackageIndex/issue-3469-dependency-transition-19

finestructure · web-flow · commit 17184b7e2f35 · 2025-01-21T09:52:38.000+01:00
Issue 3469 dependency transition 19
diff --git a/Sources/App/Commands/Ingestion.swift b/Sources/App/Commands/Ingestion.swift
@@ -190,7 +190,7 @@ enum Ingestion {
 
             let s3Readme: S3Readme?
             do throws(S3Readme.Error) {
-                s3Readme = try await storeS3Readme(client: client, repository: repo, metadata: metadata, readme: readme)
+                s3Readme = try await storeS3Readme(repository: repo, metadata: metadata, readme: readme)
             } catch {
                 // We don't want to fail ingestion in case storing the readme fails - warn and continue.
                 Current.logger().warning("storeS3Readme failed: \(error)")
@@ -239,15 +239,16 @@ enum Ingestion {
     }
 
 
-    static func storeS3Readme(client: Client, repository: Repository, metadata: Github.Metadata, readme: Github.Readme?) async throws(S3Readme.Error) -> S3Readme? {
+    static func storeS3Readme(repository: Repository, metadata: Github.Metadata, readme: Github.Readme?) async throws(S3Readme.Error) -> S3Readme? {
+        @Dependency(\.s3) var s3
         if let upstreamEtag = readme?.etag,
            repository.s3Readme?.needsUpdate(upstreamEtag: upstreamEtag) ?? true,
            let owner = metadata.repositoryOwner,
            let repository = metadata.repositoryName,
            let html = readme?.html {
-            let objectUrl = try await Current.storeS3Readme(owner, repository, html)
+            let objectUrl = try await s3.storeReadme(owner, repository, html)
             if let imagesToCache = readme?.imagesToCache, imagesToCache.isEmpty == false {
-                try await Current.storeS3ReadmeImages(client, imagesToCache)
+                try await s3.storeReadmeImages(imagesToCache)
             }
             return .cached(s3ObjectUrl: objectUrl, githubEtag: upstreamEtag)
         } else {
diff --git a/Sources/App/Controllers/PackageController+routes.swift b/Sources/App/Controllers/PackageController+routes.swift
@@ -306,7 +306,8 @@ enum PackageController {
         }
 
         do {
-            let readme = try await Current.fetchS3Readme(req.client, owner, repository)
+            @Dependency(\.s3) var s3
+            let readme = try await s3.fetchReadme(owner, repository)
             guard let branch = pkg.repository?.defaultBranch else {
                 return PackageReadme.View(model: .cacheLookupFailed(url: readmeHtmlUrl)).document()
             }
diff --git a/Sources/App/Core/AppEnvironment.swift b/Sources/App/Core/AppEnvironment.swift
@@ -23,7 +23,6 @@ import FoundationNetworking
 
 
 struct AppEnvironment: Sendable {
-    var fetchS3Readme: @Sendable (_ client: Client, _ owner: String, _ repository: String) async throws -> String
     var fileManager: FileManager
     var getStatusCount: @Sendable (_ client: Client, _ status: Gitlab.Builder.Status) async throws -> Int
     var git: Git
@@ -33,11 +32,6 @@ struct AppEnvironment: Sendable {
     var logger: @Sendable () -> Logger
     var setLogger: @Sendable (Logger) -> Void
     var shell: Shell
-    var storeS3Readme: @Sendable (_ owner: String,
-                                  _ repository: String,
-                                  _ readme: String) async throws(S3Readme.Error) -> String
-    var storeS3ReadmeImages: @Sendable (_ client: Client,
-                                        _ imagesToCache: [Github.Readme.ImageToCache]) async throws(S3Readme.Error) -> Void
     var triggerBuild: @Sendable (_ client: Client,
                                  _ buildId: Build.Id,
                                  _ cloneURL: String,
@@ -53,7 +47,6 @@ extension AppEnvironment {
     nonisolated(unsafe) static var logger: Logger!
 
     static let live = AppEnvironment(
-        fetchS3Readme: { client, owner, repo in try await S3Readme.fetchReadme(client:client, owner: owner, repository: repo) },
         fileManager: .live,
         getStatusCount: { client, status in
             try await Gitlab.Builder.getStatusCount(client: client,
@@ -72,12 +65,6 @@ extension AppEnvironment {
         logger: { logger },
         setLogger: { logger in Self.logger = logger },
         shell: .live,
-        storeS3Readme: { owner, repo, readme throws(S3Readme.Error) in
-            try await S3Readme.storeReadme(owner: owner, repository: repo, readme: readme)
-        },
-        storeS3ReadmeImages: { client, images throws(S3Readme.Error) in
-            try await S3Readme.storeReadmeImages(client: client, imagesToCache: images)
-        },
         triggerBuild: { client, buildId, cloneURL, isDocBuild, platform, ref, swiftVersion, versionID in
             try await Gitlab.Builder.triggerBuild(client: client,
                                                   buildId: buildId,
diff --git a/Sources/App/Core/Dependencies/HTTPClient.swift b/Sources/App/Core/Dependencies/HTTPClient.swift
@@ -66,6 +66,8 @@ extension HTTPClient: DependencyKey {
             }
         )
     }
+
+    func get(url: String) async throws -> Response { try await get(url: url, headers: .init()) }
 }
 
 
diff --git a/Sources/App/Core/Dependencies/S3Client.swift b/Sources/App/Core/Dependencies/S3Client.swift
@@ -0,0 +1,66 @@
+// Copyright Dave Verwer, Sven A. Schmidt, and other contributors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import Dependencies
+import IssueReporting
+
+
+// We currently cannot use @DependencyClient here due to
+// https://github.com/pointfreeco/swift-dependencies/discussions/324
+//@DependencyClient
+struct S3Client {
+    var fetchReadme: @Sendable (_ owner: String, _ repository: String) async throws -> String
+    var storeReadme: @Sendable (_ owner: String, _ repository: String, _ readme: String) async throws(S3Readme.Error) -> String = { _, _, _ in
+        reportIssue("storeS3Readme"); return ""
+    }
+    var storeReadmeImages: @Sendable (_ imagesToCache: [Github.Readme.ImageToCache]) async throws(S3Readme.Error) -> Void
+}
+
+
+extension S3Client: DependencyKey {
+    static var liveValue: Self {
+        .init(
+            fetchReadme: { owner, repo in
+                try await S3Readme.fetchReadme(owner: owner, repository: repo)
+            },
+            storeReadme: { owner, repo, readme throws(S3Readme.Error) in
+                try await S3Readme.storeReadme(owner: owner, repository: repo, readme: readme)
+            },
+            storeReadmeImages: { images throws(S3Readme.Error) in
+                try await S3Readme.storeReadmeImages(imagesToCache: images)
+            }
+        )
+    }
+}
+
+
+extension S3Client: TestDependencyKey {
+    static var testValue: Self {
+        .init(
+            fetchReadme: { _, _ in unimplemented(); return "" },
+            storeReadme: { _, _, _ in unimplemented("storeS3Readme"); return "" },
+            storeReadmeImages: { _ throws(S3Readme.Error) in unimplemented("storeS3ReadmeImages") }
+        )
+    }
+}
+
+
+extension DependencyValues {
+    var s3: S3Client {
+        get { self[S3Client.self] }
+        set { self[S3Client.self] = newValue }
+    }
+}
+
+
diff --git a/Sources/App/Core/Extensions/S3Readme+ext.swift b/Sources/App/Core/Extensions/S3Readme+ext.swift
@@ -27,11 +27,12 @@ extension S3Readme {
         case storeImagesFailed
     }
 
-    static func fetchReadme(client: Client, owner: String, repository: String) async throws(S3Readme.Error) -> String {
+    static func fetchReadme(owner: String, repository: String) async throws(S3Readme.Error) -> String {
         let key = try S3Store.Key.readme(owner: owner, repository: repository)
-        let response: ClientResponse
+        @Dependency(\.httpClient) var httpClient
+        let response: HTTPClient.Response
         do {
-            response = try await client.get(URI(string: key.objectUrl))
+            response = try await httpClient.get(url: key.objectUrl)
         } catch {
             throw .requestFailed(key: key, error: error)
         }
@@ -56,16 +57,17 @@ extension S3Readme {
         return key.objectUrl
     }
 
-    static func storeReadmeImages(client: Client, imagesToCache: [Github.Readme.ImageToCache]) async throws(S3Readme.Error) {
+    static func storeReadmeImages(imagesToCache: [Github.Readme.ImageToCache]) async throws(S3Readme.Error) {
         @Dependency(\.environment) var environment
+        @Dependency(\.httpClient) var httpClient
         guard let accessKeyId = environment.awsAccessKeyId() else { throw .envVariableNotSet("AWS_ACCESS_KEY_ID") }
         guard let secretAccessKey = environment.awsSecretAccessKey() else { throw .envVariableNotSet("AWS_SECRET_ACCESS_KEY")}
 
         let store = S3Store(credentials: .init(keyId: accessKeyId, secret: secretAccessKey))
         for imageToCache in imagesToCache {
             Current.logger().debug("Copying readme image to \(imageToCache.s3Key.s3Uri) ...")
             do {
-                let response = try await client.get(URI(stringLiteral: imageToCache.originalUrl))
+                let response = try await httpClient.get(url: imageToCache.originalUrl)
                 if var body = response.body, let imageData = body.readData(length: body.readableBytes) {
                     try await store.save(payload: imageData, to: imageToCache.s3Key)
                 }
diff --git a/Tests/AppTests/IngestionTests.swift b/Tests/AppTests/IngestionTests.swift
@@ -466,6 +466,7 @@ class IngestionTests: AppTestCase {
 
     func test_ingest_storeS3Readme() async throws {
         let fetchCalls = QueueIsolated(0)
+        let storeCalls = QueueIsolated(0)
         try await withDependencies {
             $0.date.now = .now
             $0.github.fetchLicense = { @Sendable _, _ in nil }
@@ -484,13 +485,7 @@ class IngestionTests: AppTestCase {
                                  imagesToCache: [])
                 }
             }
-        } operation: {
-            // setup
-            let app = self.app!
-            let pkg = Package(url: "https://github.com/foo/bar".url, processingStage: .reconciliation)
-            try await pkg.save(on: app.db)
-            let storeCalls = QueueIsolated(0)
-            Current.storeS3Readme = { owner, repo, html in
+            $0.s3.storeReadme = { owner, repo, html in
                 storeCalls.increment()
                 XCTAssertEqual(owner, "foo")
                 XCTAssertEqual(repo, "bar")
@@ -501,6 +496,11 @@ class IngestionTests: AppTestCase {
                 }
                 return "objectUrl"
             }
+        } operation: {
+            // setup
+            let app = self.app!
+            let pkg = Package(url: "https://github.com/foo/bar".url, processingStage: .reconciliation)
+            try await pkg.save(on: app.db)
 
             do { // first ingestion, no readme has been saved
                  // MUT
@@ -553,13 +553,7 @@ class IngestionTests: AppTestCase {
         let pkg = Package(url: "https://github.com/foo/bar".url,
                           processingStage: .reconciliation)
         try await pkg.save(on: app.db)
-        Current.storeS3Readme = { _, _, _ in "objectUrl" }
         let storeS3ReadmeImagesCalls = QueueIsolated(0)
-        Current.storeS3ReadmeImages = { _, imagesToCache in
-            storeS3ReadmeImagesCalls.increment()
-
-            XCTAssertEqual(imagesToCache.count, 2)
-        }
 
         try await withDependencies {
             $0.date.now = .now
@@ -586,6 +580,11 @@ class IngestionTests: AppTestCase {
                                                    path: "/foo/bar/with-jwt-2.jpg"))
                              ])
             }
+            $0.s3.storeReadme = { _, _, _ in "objectUrl" }
+            $0.s3.storeReadmeImages = { imagesToCache in
+                storeS3ReadmeImagesCalls.increment()
+                XCTAssertEqual(imagesToCache.count, 2)
+            }
         } operation: {
             // MUT
             try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(1))
@@ -601,10 +600,6 @@ class IngestionTests: AppTestCase {
         let pkg = Package(url: "https://github.com/foo/bar".url, processingStage: .reconciliation)
         try await pkg.save(on: app.db)
         let storeCalls = QueueIsolated(0)
-        Current.storeS3Readme = { owner, repo, html throws(S3Readme.Error) in
-            storeCalls.increment()
-            throw .storeReadmeFailed
-        }
 
         do { // first ingestion, no readme has been saved
             try await withDependencies {
@@ -617,6 +612,10 @@ class IngestionTests: AppTestCase {
                                  htmlUrl: "readme url",
                                  imagesToCache: [])
                 }
+                $0.s3.storeReadme = { owner, repo, html throws(S3Readme.Error) in
+                    storeCalls.increment()
+                    throw .storeReadmeFailed
+                }
             } operation: {
                 // MUT
                 let app = self.app!
diff --git a/Tests/AppTests/Mocks/AppEnvironment+mock.swift b/Tests/AppTests/Mocks/AppEnvironment+mock.swift
@@ -22,7 +22,6 @@ import Vapor
 extension AppEnvironment {
     static func mock(eventLoop: EventLoop) -> Self {
         .init(
-            fetchS3Readme: { _, _, _ in "" },
             fileManager: .mock,
             getStatusCount: { _, _ in 100 },
             git: .mock,
@@ -32,8 +31,6 @@ extension AppEnvironment {
             logger: { logger },
             setLogger: { logger in Self.logger = logger },
             shell: .mock,
-            storeS3Readme: { _, _, _ in "s3ObjectUrl" },
-            storeS3ReadmeImages: { _, _ in },
             triggerBuild: { _, _, _, _, _, _, _, _ in .init(status: .ok, webUrl: "http://web_url") }
         )
     }
diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift

Original file line number	Diff line number	Diff line change
`@@ -306,7 +306,8 @@ enum PackageController {`
`306`	`306`	`}`
`307`	`307`
`308`	`308`	`do {`
`309`		`- let readme = try await Current.fetchS3Readme(req.client, owner, repository)`
	`309`	`+ @Dependency(\.s3) var s3`
	`310`	`+ let readme = try await s3.fetchReadme(owner, repository)`
`310`	`311`	`guard let branch = pkg.repository?.defaultBranch else {`
`311`	`312`	`return PackageReadme.View(model: .cacheLookupFailed(url: readmeHtmlUrl)).document()`
`312`	`313`	`}`
Original file line number	Diff line number	Diff line change
`@@ -66,6 +66,8 @@ extension HTTPClient: DependencyKey {`
`66`	`66`	`}`
`67`	`67`	`)`
`68`	`68`	`}`
	`69`	`+`
	`70`	`+ func get(url: String) async throws -> Response { try await get(url: url, headers: .init()) }`
`69`	`71`	`}`
`70`	`72`
`71`	`73`