From 1db50f245b419a8d1ce59e4cf4964098a97b5f22 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Fri, 3 Jan 2025 19:10:42 +0100 Subject: [PATCH 01/17] First stab at CurrentReferenceCacheClient --- .../CurrentReferenceCacheClient.swift | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift new file mode 100644 index 000000000..f412212ec --- /dev/null +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -0,0 +1,70 @@ +// 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 DependenciesMacros +import NIOCore +import Redis + + +@DependencyClient +struct CurrentReferenceCacheClient { + var set: @Sendable (_ owner: String, _ repository: String, _ reference: String) async -> Void + var get: @Sendable (_ owner: String, _ repository: String) async -> String? +} + + +extension CurrentReferenceCacheClient: DependencyKey { + static var liveValue: CurrentReferenceCacheClient { + .init( + set: { owner, repository, reference async in + await Self.redis?.set(owner: owner, repository: repository, reference: reference) + }, + get: { owner, repository in + await Self.redis?.get(owner: owner, repository: repository) + } + ) + } +} + + +extension CurrentReferenceCacheClient { + @MainActor + static var redis: Redis? + + @MainActor + static func setRedis(_ client: sending RedisClient) { redis = .init(client: client) } + + actor Redis { + var client: RedisClient + init(client: RedisClient) { + self.client = client + } + + static let expirationInSeconds = 5*60 + + func set(owner: String, repository: String, reference: String) async -> Void { + let key = "\(owner)/\(repository)".lowercased() + let buffer = ByteBuffer(string: reference) + try? await client.setex(.init(key), + to: RESPValue.bulkString(buffer), + expirationInSeconds: Self.expirationInSeconds).get() + } + + func get(owner: String, repository: String) async -> String? { + let key = "\(owner)/\(repository)".lowercased() + return try? await client.get(.init(key)).map(\.string).get() + } + } +} From 65f83354c79cde74b003f01407a751b44857caf7 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 15:48:30 +0100 Subject: [PATCH 02/17] Tweak setup --- .../CurrentReferenceCacheClient.swift | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index f412212ec..9e33c4e41 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -39,17 +39,28 @@ extension CurrentReferenceCacheClient: DependencyKey { } +@preconcurrency import RediStack + + extension CurrentReferenceCacheClient { @MainActor static var redis: Redis? @MainActor - static func setRedis(_ client: sending RedisClient) { redis = .init(client: client) } + static func bootstrap(hostname: String) async throws { +#warning("Add retry") + redis = try await Redis(hostname: hostname) + } actor Redis { var client: RedisClient - init(client: RedisClient) { - self.client = client + + init(hostname: String) async throws { + let connection = RedisConnection.make( + configuration: try .init(hostname: hostname), + boundEventLoop: NIOSingletons.posixEventLoopGroup.any() + ) + self.client = try await connection.get() } static let expirationInSeconds = 5*60 From 87bdbf8b0410f3b1daa667d6f26dac2f4c1908db Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:00:35 +0100 Subject: [PATCH 03/17] More tweaks --- .../CurrentReferenceCacheClient.swift | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 9e33c4e41..329b14d49 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -15,7 +15,7 @@ import Dependencies import DependenciesMacros import NIOCore -import Redis +@preconcurrency import RediStack @DependencyClient @@ -29,41 +29,45 @@ extension CurrentReferenceCacheClient: DependencyKey { static var liveValue: CurrentReferenceCacheClient { .init( set: { owner, repository, reference async in - await Self.redis?.set(owner: owner, repository: repository, reference: reference) + await Redis.shared.set(owner: owner, repository: repository, reference: reference) }, get: { owner, repository in - await Self.redis?.get(owner: owner, repository: repository) + await Redis.shared.get(owner: owner, repository: repository) } ) } } -@preconcurrency import RediStack - - extension CurrentReferenceCacheClient { - @MainActor - static var redis: Redis? - - @MainActor - static func bootstrap(hostname: String) async throws { -#warning("Add retry") - redis = try await Redis(hostname: hostname) - } - actor Redis { var client: RedisClient + static private var task: Task? + + static var shared: Redis { + get async { + if let task { + return await task.value + } + let task = Task { +#warning("Add retry") + return try! await Redis() + } + self.task = task + return await task.value + } + } - init(hostname: String) async throws { + private init() async throws { let connection = RedisConnection.make( - configuration: try .init(hostname: hostname), + configuration: try .init(hostname: Redis.hostname), boundEventLoop: NIOSingletons.posixEventLoopGroup.any() ) self.client = try await connection.get() } static let expirationInSeconds = 5*60 + static let hostname = "redis" func set(owner: String, repository: String, reference: String) async -> Void { let key = "\(owner)/\(repository)".lowercased() From a95aba989aab8ea3da7b6da28f97045b0bbe6181 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:24:49 +0100 Subject: [PATCH 04/17] Add connection retry --- .../CurrentReferenceCacheClient.swift | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 329b14d49..7990e8e9e 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -29,10 +29,10 @@ extension CurrentReferenceCacheClient: DependencyKey { static var liveValue: CurrentReferenceCacheClient { .init( set: { owner, repository, reference async in - await Redis.shared.set(owner: owner, repository: repository, reference: reference) + await Redis.shared?.set(owner: owner, repository: repository, reference: reference) }, get: { owner, repository in - await Redis.shared.get(owner: owner, repository: repository) + await Redis.shared?.get(owner: owner, repository: repository) } ) } @@ -42,16 +42,25 @@ extension CurrentReferenceCacheClient: DependencyKey { extension CurrentReferenceCacheClient { actor Redis { var client: RedisClient - static private var task: Task? + static private var task: Task? - static var shared: Redis { + static var shared: Redis? { get async { if let task { return await task.value } - let task = Task { -#warning("Add retry") - return try! await Redis() + let task = Task { + var attemptsLeft = maxConnectionAttempts + while attemptsLeft > 0 { + do { + return try await Redis() + } catch { + attemptsLeft -= 1 + print("Redis connection failed, \(attemptsLeft) attempts left. Error: \(error)") + try? await Task.sleep(for: .milliseconds(500)) + } + } + return nil } self.task = task return await task.value @@ -68,6 +77,7 @@ extension CurrentReferenceCacheClient { static let expirationInSeconds = 5*60 static let hostname = "redis" + static let maxConnectionAttempts = 3 func set(owner: String, repository: String, reference: String) async -> Void { let key = "\(owner)/\(repository)".lowercased() From 1c5ca6d0a9c4e5caf1fe15fd9621761f5e3bfce3 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:25:55 +0100 Subject: [PATCH 05/17] Log error --- Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 7990e8e9e..0c2373e14 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -56,7 +56,7 @@ extension CurrentReferenceCacheClient { return try await Redis() } catch { attemptsLeft -= 1 - print("Redis connection failed, \(attemptsLeft) attempts left. Error: \(error)") + Current.logger().warning("Redis connection failed, \(attemptsLeft) attempts left. Error: \(error)") try? await Task.sleep(for: .milliseconds(500)) } } From 1aa5b2143ec5372f0c56818985d95a0ee5db0360 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:29:08 +0100 Subject: [PATCH 06/17] Finish dependency system setup --- .../Dependencies/CurrentReferenceCacheClient.swift | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 0c2373e14..069db041a 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -39,6 +39,19 @@ extension CurrentReferenceCacheClient: DependencyKey { } +extension CurrentReferenceCacheClient: TestDependencyKey { + static var testValue: Self { Self() } +} + + +extension DependencyValues { + var currentReferenceCache: CurrentReferenceCacheClient { + get { self[CurrentReferenceCacheClient.self] } + set { self[CurrentReferenceCacheClient.self] = newValue } + } +} + + extension CurrentReferenceCacheClient { actor Redis { var client: RedisClient From d8f22caeb98655bf08b7901ba26593018ae51c76 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:46:51 +0100 Subject: [PATCH 07/17] Accept `nil` reference, deleting the key if set --- .../CurrentReferenceCacheClient.swift | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 069db041a..ffb4a5dd2 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -20,7 +20,7 @@ import NIOCore @DependencyClient struct CurrentReferenceCacheClient { - var set: @Sendable (_ owner: String, _ repository: String, _ reference: String) async -> Void + var set: @Sendable (_ owner: String, _ repository: String, _ reference: String?) async -> Void var get: @Sendable (_ owner: String, _ repository: String) async -> String? } @@ -92,12 +92,16 @@ extension CurrentReferenceCacheClient { static let hostname = "redis" static let maxConnectionAttempts = 3 - func set(owner: String, repository: String, reference: String) async -> Void { + func set(owner: String, repository: String, reference: String?) async -> Void { let key = "\(owner)/\(repository)".lowercased() - let buffer = ByteBuffer(string: reference) - try? await client.setex(.init(key), - to: RESPValue.bulkString(buffer), - expirationInSeconds: Self.expirationInSeconds).get() + if let reference { + let buffer = ByteBuffer(string: reference) + try? await client.setex(.init(key), + to: RESPValue.bulkString(buffer), + expirationInSeconds: Self.expirationInSeconds).get() + } else { + _ = try? await client.delete([.init(key)]).get() + } } func get(owner: String, repository: String) async -> String? { From fbcb232609891be1f819009cde280966765c84b5 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:47:24 +0100 Subject: [PATCH 08/17] Update current cache to also write key/value pair to redis --- Sources/App/Core/CurrentReferenceCache.swift | 21 +++++++++++-------- Sources/App/routes+documentation.swift | 4 ++-- .../PackageController+routesTests.swift | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/Sources/App/Core/CurrentReferenceCache.swift b/Sources/App/Core/CurrentReferenceCache.swift index f7de4336d..37b2316e3 100644 --- a/Sources/App/Core/CurrentReferenceCache.swift +++ b/Sources/App/Core/CurrentReferenceCache.swift @@ -13,20 +13,23 @@ // limitations under the License. @preconcurrency import Cache +import Dependencies + typealias CurrentReferenceCache = ExpiringCache extension CurrentReferenceCache { static let live = CurrentReferenceCache(duration: .minutes(5)) - subscript(owner owner: String, repository repository: String) -> String? { - get { - let key = "\(owner)/\(repository)".lowercased() - return self[key] - } - set { - let key = "\(owner)/\(repository)".lowercased() - self[key] = newValue - } + func get(owner: String, repository: String) -> String? { + let key = "\(owner)/\(repository)".lowercased() + return self[key] + } + + func set(owner: String, repository: String, reference: String?) async { + let key = "\(owner)/\(repository)".lowercased() + self[key] = reference + @Dependency(\.currentReferenceCache) var currentReferenceCache + await currentReferenceCache.set(owner: owner, repository: repository, reference: reference) } } diff --git a/Sources/App/routes+documentation.swift b/Sources/App/routes+documentation.swift index aef2fa664..af28a7be5 100644 --- a/Sources/App/routes+documentation.swift +++ b/Sources/App/routes+documentation.swift @@ -178,14 +178,14 @@ extension Request { let docVersion = try await { () -> DocVersion in if reference == String.current { - if let ref = environment.currentReferenceCache()?[owner: owner, repository: repository] { + if let ref = environment.currentReferenceCache()?.get(owner: owner, repository: repository) { return .current(referencing: ref) } guard let params = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)?.internal else { throw Abort(.notFound) } - environment.currentReferenceCache()?[owner: owner, repository: repository] = "\(params.docVersion)" + await environment.currentReferenceCache()?.set(owner: owner, repository: repository, reference: "\(params.docVersion)") return .current(referencing: "\(params.docVersion)") } else { return .reference(reference) diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift index eed792c9e..e7fe01f6e 100644 --- a/Tests/AppTests/PackageController+routesTests.swift +++ b/Tests/AppTests/PackageController+routesTests.swift @@ -1576,7 +1576,7 @@ class PackageController_routesTests: SnapshotTestCase { XCTFail("unexpected error: \(error)") } - cache[owner: "owner", repository: "repo"] = "1.2.3" + await cache.set(owner: "owner", repository: "repo", reference: "1.2.3") do { // Now with the cache in place this resolves let route = try await req.getDocRoute(fragment: .documentation) From a61478ab76ffd998f14359da4011fa248fa2e92d Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sat, 4 Jan 2025 16:59:21 +0100 Subject: [PATCH 09/17] Update tests --- .../Core/Dependencies/CurrentReferenceCacheClient.swift | 9 +++++++++ Tests/AppTests/PackageController+routesTests.swift | 2 ++ 2 files changed, 11 insertions(+) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index ffb4a5dd2..4d595472c 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -110,3 +110,12 @@ extension CurrentReferenceCacheClient { } } } + + +#if DEBUG +extension CurrentReferenceCacheClient { + static var disabled: Self { + .init(set: { _, _, _ in }, get: { _, _ in nil }) + } +} +#endif diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift index e7fe01f6e..de40f4214 100644 --- a/Tests/AppTests/PackageController+routesTests.swift +++ b/Tests/AppTests/PackageController+routesTests.swift @@ -1479,6 +1479,7 @@ class PackageController_routesTests: SnapshotTestCase { // Ensures default branch updates don't introduce a "documentation gap" // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/2288 try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } $0.environment.currentReferenceCache = { .live } $0.httpClient.fetchDocumentation = { @Sendable _ in .init(status: .ok, body: .mockIndexHTML()) } @@ -1558,6 +1559,7 @@ class PackageController_routesTests: SnapshotTestCase { func test_getDocRoute_documentation_current() async throws { nonisolated(unsafe) let cache = CurrentReferenceCache() try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.currentReferenceCache = { cache } } operation: { // owner/repo/~/documentation/archive From 3edf3e54e8b820f0ec4ef0cb59590ab3861cfa06 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 11:06:34 +0100 Subject: [PATCH 10/17] Move Redis to own file --- .../CurrentReferenceCacheClient.swift | 62 --------------- .../App/Core/Dependencies/RedisClient.swift | 75 +++++++++++++++++++ 2 files changed, 75 insertions(+), 62 deletions(-) create mode 100644 Sources/App/Core/Dependencies/RedisClient.swift diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 4d595472c..1752f055c 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -14,8 +14,6 @@ import Dependencies import DependenciesMacros -import NIOCore -@preconcurrency import RediStack @DependencyClient @@ -52,66 +50,6 @@ extension DependencyValues { } -extension CurrentReferenceCacheClient { - actor Redis { - var client: RedisClient - static private var task: Task? - - static var shared: Redis? { - get async { - if let task { - return await task.value - } - let task = Task { - var attemptsLeft = maxConnectionAttempts - while attemptsLeft > 0 { - do { - return try await Redis() - } catch { - attemptsLeft -= 1 - Current.logger().warning("Redis connection failed, \(attemptsLeft) attempts left. Error: \(error)") - try? await Task.sleep(for: .milliseconds(500)) - } - } - return nil - } - self.task = task - return await task.value - } - } - - private init() async throws { - let connection = RedisConnection.make( - configuration: try .init(hostname: Redis.hostname), - boundEventLoop: NIOSingletons.posixEventLoopGroup.any() - ) - self.client = try await connection.get() - } - - static let expirationInSeconds = 5*60 - static let hostname = "redis" - static let maxConnectionAttempts = 3 - - func set(owner: String, repository: String, reference: String?) async -> Void { - let key = "\(owner)/\(repository)".lowercased() - if let reference { - let buffer = ByteBuffer(string: reference) - try? await client.setex(.init(key), - to: RESPValue.bulkString(buffer), - expirationInSeconds: Self.expirationInSeconds).get() - } else { - _ = try? await client.delete([.init(key)]).get() - } - } - - func get(owner: String, repository: String) async -> String? { - let key = "\(owner)/\(repository)".lowercased() - return try? await client.get(.init(key)).map(\.string).get() - } - } -} - - #if DEBUG extension CurrentReferenceCacheClient { static var disabled: Self { diff --git a/Sources/App/Core/Dependencies/RedisClient.swift b/Sources/App/Core/Dependencies/RedisClient.swift new file mode 100644 index 000000000..12d1a186a --- /dev/null +++ b/Sources/App/Core/Dependencies/RedisClient.swift @@ -0,0 +1,75 @@ +// 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 NIOCore +@preconcurrency import RediStack + + +actor Redis { + var client: RedisClient + static private var task: Task? + + static var shared: Redis? { + get async { + if let task { + return await task.value + } + let task = Task { + var attemptsLeft = maxConnectionAttempts + while attemptsLeft > 0 { + do { + return try await Redis() + } catch { + attemptsLeft -= 1 + Current.logger().warning("Redis connection failed, \(attemptsLeft) attempts left. Error: \(error)") + try? await Task.sleep(for: .milliseconds(500)) + } + } + return nil + } + self.task = task + return await task.value + } + } + + private init() async throws { + let connection = RedisConnection.make( + configuration: try .init(hostname: Redis.hostname), + boundEventLoop: NIOSingletons.posixEventLoopGroup.any() + ) + self.client = try await connection.get() + } + + static let expirationInSeconds = 5*60 + static let hostname = "redis" + static let maxConnectionAttempts = 3 + + func set(owner: String, repository: String, reference: String?) async -> Void { + let key = "\(owner)/\(repository)".lowercased() + if let reference { + let buffer = ByteBuffer(string: reference) + try? await client.setex(.init(key), + to: RESPValue.bulkString(buffer), + expirationInSeconds: Self.expirationInSeconds).get() + } else { + _ = try? await client.delete([.init(key)]).get() + } + } + + func get(owner: String, repository: String) async -> String? { + let key = "\(owner)/\(repository)".lowercased() + return try? await client.get(.init(key)).map(\.string).get() + } +} + From 90e3a501de8b39ad7c9e0c3d02ec8975107f29cf Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 11:22:04 +0100 Subject: [PATCH 11/17] Make Redis actor private --- .../CurrentReferenceCacheClient.swift | 10 +++- .../App/Core/Dependencies/RedisClient.swift | 47 +++++++++++++++---- 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 1752f055c..0725dbc5d 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -27,13 +27,19 @@ extension CurrentReferenceCacheClient: DependencyKey { static var liveValue: CurrentReferenceCacheClient { .init( set: { owner, repository, reference async in - await Redis.shared?.set(owner: owner, repository: repository, reference: reference) + @Dependency(\.redis) var redis + await redis.set(key: getKey(owner: owner, repository: repository), value: reference) }, get: { owner, repository in - await Redis.shared?.get(owner: owner, repository: repository) + @Dependency(\.redis) var redis + return await redis.get(key: getKey(owner: owner, repository: repository)) } ) } + + static func getKey(owner: String, repository: String) -> String { + "\(owner)/\(repository)".lowercased() + } } diff --git a/Sources/App/Core/Dependencies/RedisClient.swift b/Sources/App/Core/Dependencies/RedisClient.swift index 12d1a186a..888064576 100644 --- a/Sources/App/Core/Dependencies/RedisClient.swift +++ b/Sources/App/Core/Dependencies/RedisClient.swift @@ -14,10 +14,42 @@ import NIOCore @preconcurrency import RediStack +import Dependencies +import DependenciesMacros -actor Redis { - var client: RedisClient +@DependencyClient +struct RedisClient { + var set: @Sendable (_ key: String, _ value: String?) async -> Void + var get: @Sendable (_ key: String) async -> String? +} + + +extension RedisClient: DependencyKey { + static var liveValue: RedisClient { + .init( + set: { key, value in await Redis.shared?.set(key: key, value: value) }, + get: { key in await Redis.shared?.get(key: key) } + ) + } +} + + +extension RedisClient: TestDependencyKey { + static var testValue: Self { Self() } +} + + +extension DependencyValues { + var redis: RedisClient { + get { self[RedisClient.self] } + set { self[RedisClient.self] = newValue } + } +} + + +private actor Redis { + var client: RediStack.RedisClient static private var task: Task? static var shared: Redis? { @@ -51,14 +83,14 @@ actor Redis { self.client = try await connection.get() } +#warning("move expiry to interface") static let expirationInSeconds = 5*60 static let hostname = "redis" static let maxConnectionAttempts = 3 - func set(owner: String, repository: String, reference: String?) async -> Void { - let key = "\(owner)/\(repository)".lowercased() - if let reference { - let buffer = ByteBuffer(string: reference) + func set(key: String, value: String?) async -> Void { + if let value { + let buffer = ByteBuffer(string: value) try? await client.setex(.init(key), to: RESPValue.bulkString(buffer), expirationInSeconds: Self.expirationInSeconds).get() @@ -67,8 +99,7 @@ actor Redis { } } - func get(owner: String, repository: String) async -> String? { - let key = "\(owner)/\(repository)".lowercased() + func get(key: String) async -> String? { return try? await client.get(.init(key)).map(\.string).get() } } From 1a975b2184435d13981200ef67ed676bcf9696a9 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 11:28:36 +0100 Subject: [PATCH 12/17] Add static `RedisClient.disabled` --- Sources/App/Core/Dependencies/RedisClient.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Sources/App/Core/Dependencies/RedisClient.swift b/Sources/App/Core/Dependencies/RedisClient.swift index 888064576..988676cd3 100644 --- a/Sources/App/Core/Dependencies/RedisClient.swift +++ b/Sources/App/Core/Dependencies/RedisClient.swift @@ -48,6 +48,15 @@ extension DependencyValues { } +#if DEBUG +extension RedisClient { + static var disabled: Self { + .init(set: { _, _ in }, get: { _ in nil }) + } +} +#endif + + private actor Redis { var client: RediStack.RedisClient static private var task: Task? From 519b256caf5c76743897c006b781a415a20f6a52 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 11:28:53 +0100 Subject: [PATCH 13/17] Remove outdated RedisConfiguration setup --- Sources/App/configure.swift | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index e1846a1b9..06828ecdf 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -78,13 +78,6 @@ public func configure(_ app: Application) async throws -> String { sqlLogLevel: .debug), as: .psql) - // Setup Redis connection - do { - app.redis.configuration = try RedisConfiguration(hostname: "redis") - } catch { - app.logger.warning("Failed to configure Redis, caching disabled. Error: \(error)") - } - do { // Migration 001 - schema 1.0 app.migrations.add(CreatePackage()) app.migrations.add(CreateRepository()) From 8efbace52bb74b54ca2c87cf13299171c819f6cd Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 12:08:37 +0100 Subject: [PATCH 14/17] Remove old cache --- Package.resolved | 11 +----- Package.swift | 2 -- Sources/App/Core/CurrentReferenceCache.swift | 35 ------------------- .../CurrentReferenceCacheClient.swift | 17 +++++++++ .../Core/Dependencies/EnvironmentClient.swift | 2 -- Sources/App/routes+documentation.swift | 5 +-- .../PackageController+routesTests.swift | 20 +++++------ 7 files changed, 30 insertions(+), 62 deletions(-) delete mode 100644 Sources/App/Core/CurrentReferenceCache.swift diff --git a/Package.resolved b/Package.resolved index c1d729ef6..be2aefffa 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "522f8d8b891d1a65e94cabb612d629f37de0bf6025993255871335482d355051", + "originHash" : "c68e60d7c703a8fa9fb0a3a11024cf6b2304fb294ba9f124de1d41f024b11cc9", "pins" : [ { "identity" : "async-http-client", @@ -28,15 +28,6 @@ "version" : "2.0.2" } }, - { - "identity" : "cache", - "kind" : "remoteSourceControl", - "location" : "https://github.com/0xLeif/Cache.git", - "state" : { - "revision" : "a535c68aab7bf0b42bda7a5de66b04f86e48c6e6", - "version" : "2.1.0" - } - }, { "identity" : "canonicalpackageurl", "kind" : "remoteSourceControl", diff --git a/Package.swift b/Package.swift index bc6225c79..9adebaf27 100644 --- a/Package.swift +++ b/Package.swift @@ -27,7 +27,6 @@ let package = Package( .library(name: "S3Store", targets: ["S3Store"]), ], dependencies: [ - .package(url: "https://github.com/0xLeif/Cache.git", from: "2.1.0"), .package(url: "https://github.com/JohnSundell/Ink.git", from: "0.5.1"), .package(url: "https://github.com/swift-server/swift-prometheus.git", from: "1.0.0"), .package(url: "https://github.com/SwiftPackageIndex/Plot.git", branch: "main"), @@ -63,7 +62,6 @@ let package = Package( .product(name: "SPIManifest", package: "SPIManifest"), .product(name: "SemanticVersion", package: "SemanticVersion"), .product(name: "SwiftSoup", package: "SwiftSoup"), - .product(name: "Cache", package: "cache"), .product(name: "CanonicalPackageURL", package: "CanonicalPackageURL"), .product(name: "CustomDump", package: "swift-custom-dump"), .product(name: "Dependencies", package: "swift-dependencies"), diff --git a/Sources/App/Core/CurrentReferenceCache.swift b/Sources/App/Core/CurrentReferenceCache.swift deleted file mode 100644 index 37b2316e3..000000000 --- a/Sources/App/Core/CurrentReferenceCache.swift +++ /dev/null @@ -1,35 +0,0 @@ -// 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. - -@preconcurrency import Cache -import Dependencies - - -typealias CurrentReferenceCache = ExpiringCache - -extension CurrentReferenceCache { - static let live = CurrentReferenceCache(duration: .minutes(5)) - - func get(owner: String, repository: String) -> String? { - let key = "\(owner)/\(repository)".lowercased() - return self[key] - } - - func set(owner: String, repository: String, reference: String?) async { - let key = "\(owner)/\(repository)".lowercased() - self[key] = reference - @Dependency(\.currentReferenceCache) var currentReferenceCache - await currentReferenceCache.set(owner: owner, repository: repository, reference: reference) - } -} diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 0725dbc5d..721ac2350 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -61,5 +61,22 @@ extension CurrentReferenceCacheClient { static var disabled: Self { .init(set: { _, _, _ in }, get: { _, _ in nil }) } + + static var inMemory: Self { + .init( + set: { owner, repository, reference in + _cache.withValue { + $0[getKey(owner: owner, repository: repository)] = reference + } + }, + get: { owner, repository in + _cache.withValue { + $0[getKey(owner: owner, repository: repository)] + } + } + ) + } + + nonisolated(unsafe) static var _cache = LockIsolated<[String: String]>([:]) } #endif diff --git a/Sources/App/Core/Dependencies/EnvironmentClient.swift b/Sources/App/Core/Dependencies/EnvironmentClient.swift index 67ee40016..371b570bf 100644 --- a/Sources/App/Core/Dependencies/EnvironmentClient.swift +++ b/Sources/App/Core/Dependencies/EnvironmentClient.swift @@ -39,7 +39,6 @@ struct EnvironmentClient { var collectionSigningCertificateChain: @Sendable () -> [URL] = { XCTFail("collectionSigningCertificateChain"); return [] } var collectionSigningPrivateKey: @Sendable () -> Data? var current: @Sendable () -> Environment = { XCTFail("current"); return .development } - var currentReferenceCache: @Sendable () -> CurrentReferenceCache? var dbId: @Sendable () -> String? var mastodonCredentials: @Sendable () -> Mastodon.Credentials? var random: @Sendable (_ range: ClosedRange) -> Double = { XCTFail("random"); return Double.random(in: $0) } @@ -102,7 +101,6 @@ extension EnvironmentClient: DependencyKey { Environment.get("COLLECTION_SIGNING_PRIVATE_KEY").map { Data($0.utf8) } }, current: { (try? Environment.detect()) ?? .development }, - currentReferenceCache: { .live }, dbId: { Environment.get("DATABASE_ID") }, mastodonCredentials: { Environment.get("MASTODON_ACCESS_TOKEN") diff --git a/Sources/App/routes+documentation.swift b/Sources/App/routes+documentation.swift index af28a7be5..595d4171f 100644 --- a/Sources/App/routes+documentation.swift +++ b/Sources/App/routes+documentation.swift @@ -178,14 +178,15 @@ extension Request { let docVersion = try await { () -> DocVersion in if reference == String.current { - if let ref = environment.currentReferenceCache()?.get(owner: owner, repository: repository) { + @Dependency(\.currentReferenceCache) var currentReferenceCache + if let ref = await currentReferenceCache.get(owner: owner, repository: repository) { return .current(referencing: ref) } guard let params = try await DocumentationTarget.query(on: db, owner: owner, repository: repository)?.internal else { throw Abort(.notFound) } - await environment.currentReferenceCache()?.set(owner: owner, repository: repository, reference: "\(params.docVersion)") + await currentReferenceCache.set(owner: owner, repository: repository, reference: "\(params.docVersion)") return .current(referencing: "\(params.docVersion)") } else { return .reference(reference) diff --git a/Tests/AppTests/PackageController+routesTests.swift b/Tests/AppTests/PackageController+routesTests.swift index de40f4214..4d0d8c672 100644 --- a/Tests/AppTests/PackageController+routesTests.swift +++ b/Tests/AppTests/PackageController+routesTests.swift @@ -566,8 +566,8 @@ class PackageController_routesTests: SnapshotTestCase { // Test the current (~) documentation routes: // /owner/package/documentation/~ + various path elements try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.httpClient.fetchDocumentation = { @Sendable _ in .init(status: .ok, body: .mockIndexHTML()) } } operation: { // setup @@ -649,8 +649,8 @@ class PackageController_routesTests: SnapshotTestCase { // Test the current (~) documentation routes with baseURL rewriting: // /owner/package/documentation/~ + various path elements try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.httpClient.fetchDocumentation = { @Sendable _ in .init(status: .ok, body: .mockIndexHTML(baseURL: "/owner/package/1.0.0")) } } operation: { // setup @@ -941,8 +941,8 @@ class PackageController_routesTests: SnapshotTestCase { func test_documentation_current_css() async throws { try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.httpClient.fetchDocumentation = App.HTTPClient.echoURL() } operation: { // setup @@ -996,8 +996,8 @@ class PackageController_routesTests: SnapshotTestCase { func test_documentation_current_js() async throws { try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.httpClient.fetchDocumentation = App.HTTPClient.echoURL() } operation: { // setup @@ -1051,8 +1051,8 @@ class PackageController_routesTests: SnapshotTestCase { func test_documentation_current_data() async throws { try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.httpClient.fetchDocumentation = App.HTTPClient.echoURL() } operation: { // setup @@ -1156,8 +1156,8 @@ class PackageController_routesTests: SnapshotTestCase { // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/2287 // Ensure references are path encoded try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.httpClient.fetchDocumentation = { @Sendable _ in .init(status: .ok, body: .mockIndexHTML()) } } operation: { // setup @@ -1220,8 +1220,8 @@ class PackageController_routesTests: SnapshotTestCase { func test_documentation_routes_tutorials() async throws { try await withDependencies { + $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { nil } $0.environment.dbId = { nil } $0.httpClient.fetchDocumentation = { @Sendable _ in .init(status: .ok, body: .mockIndexHTML()) } } operation: { @@ -1481,7 +1481,6 @@ class PackageController_routesTests: SnapshotTestCase { try await withDependencies { $0.currentReferenceCache = .disabled $0.environment.awsDocsBucket = { "docs-bucket" } - $0.environment.currentReferenceCache = { .live } $0.httpClient.fetchDocumentation = { @Sendable _ in .init(status: .ok, body: .mockIndexHTML()) } } operation: { // setup @@ -1557,10 +1556,8 @@ class PackageController_routesTests: SnapshotTestCase { } func test_getDocRoute_documentation_current() async throws { - nonisolated(unsafe) let cache = CurrentReferenceCache() try await withDependencies { - $0.currentReferenceCache = .disabled - $0.environment.currentReferenceCache = { cache } + $0.currentReferenceCache = .inMemory } operation: { // owner/repo/~/documentation/archive let req = Request(application: app, url: "", on: app.eventLoopGroup.next()) @@ -1578,6 +1575,7 @@ class PackageController_routesTests: SnapshotTestCase { XCTFail("unexpected error: \(error)") } + @Dependency(\.currentReferenceCache) var cache await cache.set(owner: "owner", repository: "repo", reference: "1.2.3") do { // Now with the cache in place this resolves From 6ba350c2027a175081c18d7258a81c80a6b62a48 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 12:28:03 +0100 Subject: [PATCH 15/17] Move hard coded expiry from Redis to CurrentReferenceCacheClient --- .../CurrentReferenceCacheClient.swift | 6 +++- .../App/Core/Dependencies/RedisClient.swift | 29 +++++++++++++------ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift index 721ac2350..793a65f8b 100644 --- a/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift +++ b/Sources/App/Core/Dependencies/CurrentReferenceCacheClient.swift @@ -24,11 +24,15 @@ struct CurrentReferenceCacheClient { extension CurrentReferenceCacheClient: DependencyKey { + static let timeToLive: Duration = .seconds(5*60) + static var liveValue: CurrentReferenceCacheClient { .init( set: { owner, repository, reference async in @Dependency(\.redis) var redis - await redis.set(key: getKey(owner: owner, repository: repository), value: reference) + await redis.set(key: getKey(owner: owner, repository: repository), + value: reference, + expiresIn: timeToLive) }, get: { owner, repository in @Dependency(\.redis) var redis diff --git a/Sources/App/Core/Dependencies/RedisClient.swift b/Sources/App/Core/Dependencies/RedisClient.swift index 988676cd3..35c704a18 100644 --- a/Sources/App/Core/Dependencies/RedisClient.swift +++ b/Sources/App/Core/Dependencies/RedisClient.swift @@ -20,15 +20,24 @@ import DependenciesMacros @DependencyClient struct RedisClient { - var set: @Sendable (_ key: String, _ value: String?) async -> Void + var set: @Sendable (_ key: String, _ value: String?, Duration?) async -> Void var get: @Sendable (_ key: String) async -> String? } +extension RedisClient { + func set(key: String, value: String?, expiresIn: Duration? = nil) async { + await set(key: key, value: value, expiresIn) + } +} + + extension RedisClient: DependencyKey { static var liveValue: RedisClient { .init( - set: { key, value in await Redis.shared?.set(key: key, value: value) }, + set: { key, value, expiresIn in + await Redis.shared?.set(key: key, value: value, expiresIn: expiresIn) + }, get: { key in await Redis.shared?.get(key: key) } ) } @@ -51,7 +60,7 @@ extension DependencyValues { #if DEBUG extension RedisClient { static var disabled: Self { - .init(set: { _, _ in }, get: { _ in nil }) + .init(set: { _, _, _ in }, get: { _ in nil }) } } #endif @@ -92,17 +101,19 @@ private actor Redis { self.client = try await connection.get() } -#warning("move expiry to interface") - static let expirationInSeconds = 5*60 static let hostname = "redis" static let maxConnectionAttempts = 3 - func set(key: String, value: String?) async -> Void { + func set(key: String, value: String?, expiresIn: Duration?) async -> Void { if let value { let buffer = ByteBuffer(string: value) - try? await client.setex(.init(key), - to: RESPValue.bulkString(buffer), - expirationInSeconds: Self.expirationInSeconds).get() + let value = RESPValue.bulkString(buffer) + if let expiresIn { + let ttl = Int(expiresIn.components.seconds) + try? await client.setex(.init(key), to: value, expirationInSeconds: ttl).get() + } else { + try? await client.set(.init(key), to: value).get() + } } else { _ = try? await client.delete([.init(key)]).get() } From 8eead6b1c76c2d4454f08496763d327846456e66 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 14:39:02 +0100 Subject: [PATCH 16/17] Add comment regarding hostname --- Sources/App/Core/Dependencies/RedisClient.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/App/Core/Dependencies/RedisClient.swift b/Sources/App/Core/Dependencies/RedisClient.swift index 35c704a18..c4582b4f6 100644 --- a/Sources/App/Core/Dependencies/RedisClient.swift +++ b/Sources/App/Core/Dependencies/RedisClient.swift @@ -101,6 +101,7 @@ private actor Redis { self.client = try await connection.get() } + // This hostname has to match the redir service name in app.yml. static let hostname = "redis" static let maxConnectionAttempts = 3 From 1e40561bbbe0cb275be822fabda1f50927373472 Mon Sep 17 00:00:00 2001 From: "Sven A. Schmidt" Date: Sun, 5 Jan 2025 14:55:49 +0100 Subject: [PATCH 17/17] Remove import --- Sources/App/configure.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index 06828ecdf..c8a65988a 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -14,7 +14,6 @@ import Fluent import FluentPostgresDriver -import Redis import Vapor