diff --git a/Sources/App/Commands/Alerting.swift b/Sources/App/Commands/Alerting.swift index e9633322b..65a9e97dd 100644 --- a/Sources/App/Commands/Alerting.swift +++ b/Sources/App/Commands/Alerting.swift @@ -42,8 +42,10 @@ enum Alerting { } func run(using context: CommandContext, signature: Signature) async throws { + prepareDependencies { + $0.logger = Logger(component: "alerting") + } @Dependency(\.logger) var logger - logger.set(to: Logger(component: "alerting")) logger.info("Running alerting...") diff --git a/Sources/App/Commands/Analyze.swift b/Sources/App/Commands/Analyze.swift index 39b449954..990e6f765 100644 --- a/Sources/App/Commands/Analyze.swift +++ b/Sources/App/Commands/Analyze.swift @@ -28,10 +28,13 @@ enum Analyze { var help: String { "Run package analysis (fetching git repository and inspecting content)" } func run(using context: CommandContext, signature: SPICommand.Signature) async throws { + prepareDependencies { + $0.logger = Logger(component: "analyze") + } + @Dependency(\.logger) var logger + let client = context.application.client let db = context.application.db - @Dependency(\.logger) var logger - logger.set(to: Logger(component: "analyze")) Analyze.resetMetrics() @@ -251,7 +254,7 @@ extension Analyze { attributes: nil) } catch { let error = AppError.genericError(nil, "Failed to create checkouts directory: \(error.localizedDescription)") - logger.logger.report(error: error) + logger.report(error: error) } } diff --git a/Sources/App/Commands/Ingestion.swift b/Sources/App/Commands/Ingestion.swift index 47e8996a4..fddf6cf97 100644 --- a/Sources/App/Commands/Ingestion.swift +++ b/Sources/App/Commands/Ingestion.swift @@ -82,10 +82,13 @@ enum Ingestion { var help: String { "Run package ingestion (fetching repository metadata)" } func run(using context: CommandContext, signature: SPICommand.Signature) async { + prepareDependencies { + $0.logger = Logger(component: "ingest") + } + @Dependency(\.logger) var logger + let client = context.application.client let db = context.application.db - @Dependency(\.logger) var logger - logger.set(to: Logger(component: "ingest")) Self.resetMetrics() diff --git a/Sources/App/Commands/ReAnalyzeVersions.swift b/Sources/App/Commands/ReAnalyzeVersions.swift index 5741908cf..6d9771c42 100644 --- a/Sources/App/Commands/ReAnalyzeVersions.swift +++ b/Sources/App/Commands/ReAnalyzeVersions.swift @@ -43,14 +43,16 @@ enum ReAnalyzeVersions { var help: String { "Run version re-analysis" } func run(using context: CommandContext, signature: Signature) async throws { - let limit = signature.limit ?? defaultLimit + prepareDependencies { + $0.logger = Logger(component: "re-analyze-versions") + } + @Dependency(\.logger) var logger + @Dependency(\.date.now) var now + let limit = signature.limit ?? defaultLimit let client = context.application.client let db = context.application.db - @Dependency(\.logger) var logger - logger.set(to: Logger(component: "re-analyze-versions")) - @Dependency(\.date.now) var now if let id = signature.packageId { logger.info("Re-analyzing versions (id: \(id)) ...") do { diff --git a/Sources/App/Commands/Reconcile.swift b/Sources/App/Commands/Reconcile.swift index 90f31195b..14c9aa647 100644 --- a/Sources/App/Commands/Reconcile.swift +++ b/Sources/App/Commands/Reconcile.swift @@ -23,14 +23,15 @@ struct ReconcileCommand: AsyncCommand { var help: String { "Reconcile package list with server" } func run(using context: CommandContext, signature: Signature) async throws { + prepareDependencies{ + $0.logger = Logger(component: "reconcile") + } @Dependency(\.logger) var logger - logger.set(to: Logger(component: "reconcile")) logger.info("Reconciling...") do { - try await reconcile(client: context.application.client, - database: context.application.db) + try await reconcile(client: context.application.client, database: context.application.db) } catch { logger.error("\(error)") } @@ -38,8 +39,7 @@ struct ReconcileCommand: AsyncCommand { logger.info("done.") do { - try await AppMetrics.push(client: context.application.client, - jobName: "reconcile") + try await AppMetrics.push(client: context.application.client, jobName: "reconcile") } catch { logger.warning("\(error)") } diff --git a/Sources/App/Commands/TriggerBuilds.swift b/Sources/App/Commands/TriggerBuilds.swift index 2fc9ce4aa..352bb54b1 100644 --- a/Sources/App/Commands/TriggerBuilds.swift +++ b/Sources/App/Commands/TriggerBuilds.swift @@ -55,8 +55,10 @@ struct TriggerBuildsCommand: AsyncCommand { } func run(using context: CommandContext, signature: Signature) async throws { + prepareDependencies { + $0.logger = Logger(component: "trigger-builds") + } @Dependency(\.logger) var logger - logger.set(to: Logger(component: "trigger-builds")) Self.resetMetrics() diff --git a/Sources/App/Core/Dependencies/LoggerClient.swift b/Sources/App/Core/Dependencies/LoggerClient.swift index 65a320adc..fe28c00be 100644 --- a/Sources/App/Core/Dependencies/LoggerClient.swift +++ b/Sources/App/Core/Dependencies/LoggerClient.swift @@ -13,72 +13,39 @@ // limitations under the License. import Dependencies -import DependenciesMacros +import IssueReporting import Logging -import Synchronization -@DependencyClient -struct LoggerClient { - var log: @Sendable (_ level: Logging.Logger.Level, Logging.Logger.Message) -> Void - var set: @Sendable (_ to: Logging.Logger) -> Void -} - - -extension LoggerClient { - func critical(_ message: Logging.Logger.Message) { log(.critical, message) } - func debug(_ message: Logging.Logger.Message) { log(.debug, message) } - func error(_ message: Logging.Logger.Message) { log(.error, message) } - func info(_ message: Logging.Logger.Message) { log(.info, message) } - func warning(_ message: Logging.Logger.Message) { log(.warning, message) } - func trace(_ message: Logging.Logger.Message) { log(.trace, message) } - func report(error: Error, file: String = #fileID, function: String = #function, line: UInt = #line) { - logger.report(error: error, file: file, function: function, line: line) +private enum LoggerClient: DependencyKey { + static var liveValue: Logger { + reportIssue("The default logger is being used. Override this dependency in the entry point of your app.") + return Logging.Logger(label: "default") } - var logger: Logging.Logger { Self._logger.withLock { $0 } } } -#if DEBUG -extension LoggerClient { - func set(to handler: LogHandler?) { - if let handler { - let logger = Logger(label: "test", factory: { _ in handler }) - set(to: logger) - } - } - - static var noop: Self { - .init(log: { _, _ in }, set: { _ in }) +extension LoggerClient: TestDependencyKey { + static var testValue: Logger { + unimplemented("testValue"); return .init(label: "test") } } -#endif -extension LoggerClient: DependencyKey { - static var liveValue: Self { - .init( - log: { level, message in - _logger.withLock { $0.log(level: level, message) } - }, - set: { logger in - _logger.withLock { $0 = logger } - } - ) +extension DependencyValues { + public var logger: Logger { + get { self[LoggerClient.self] } + set { self[LoggerClient.self] = newValue } } - - private static let _logger = Mutex(Logging.Logger(component: "default")) } -extension LoggerClient: TestDependencyKey { - static var testValue: Self { liveValue } -} - +#if DEBUG +extension Logger { + static var noop: Self { .init(label: "noop") { _ in SwiftLogNoOpLogHandler() } } -extension DependencyValues { - var logger: LoggerClient { - get { self[LoggerClient.self] } - set { self[LoggerClient.self] = newValue } + static func testLogger(_ handler: LogHandler) -> Self { + .init(label: "test", factory: { _ in handler }) } } +#endif diff --git a/Sources/App/Core/Dependencies/ShellClient.swift b/Sources/App/Core/Dependencies/ShellClient.swift index 4efa5a510..7556be544 100644 --- a/Sources/App/Core/Dependencies/ShellClient.swift +++ b/Sources/App/Core/Dependencies/ShellClient.swift @@ -42,7 +42,7 @@ extension ShellClient: DependencyKey { run: { command, path in @Dependency(\.logger) var logger do { - let res = try await ShellOut.shellOut(to: command, at: path, logger: logger.logger) + let res = try await ShellOut.shellOut(to: command, at: path, logger: logger) if !res.stderr.isEmpty { logger.warning("stderr: \(res.stderr)") } diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index 666ebc758..e3b3f0716 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -27,9 +27,7 @@ public func configure(_ app: Application, databasePort: Int? = nil) async throws let _ = Bundle(path: "/Applications/InjectionIII.app/Contents/Resources/macOSInjection.bundle")?.load() #endif - @Dependency(\.logger) var logger app.logger.component = "server" - logger.set(to: app.logger) // It will be tempting to uncomment/re-add these lines in the future. We should not enable // server-side compression as long as we pass requests through Cloudflare, which compresses diff --git a/Tests/AppTests/AnalyzeErrorTests.swift b/Tests/AppTests/AnalyzeErrorTests.swift index 1ec6ece13..1a7761d4b 100644 --- a/Tests/AppTests/AnalyzeErrorTests.swift +++ b/Tests/AppTests/AnalyzeErrorTests.swift @@ -66,7 +66,7 @@ extension AllTests.AnalyzeErrorTests { try await withDependencies { $0.environment.loadSPIManifest = { _ in nil } $0.fileManager.fileExists = { @Sendable _ in true } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) $0.shell.run = { @Sendable cmd, path in switch cmd { case _ where cmd.description.contains("git clone https://github.com/foo/1"): @@ -100,7 +100,7 @@ extension AllTests.AnalyzeErrorTests { try await withDependencies { $0.environment.loadSPIManifest = { _ in nil } $0.fileManager.fileExists = { @Sendable _ in true } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) } operation: { // setup let pkg = try await Package.find(badPackageID, on: app.db).unwrap() @@ -130,7 +130,7 @@ extension AllTests.AnalyzeErrorTests { try await withDependencies { $0.environment.loadSPIManifest = { _ in nil } $0.fileManager.fileExists = { @Sendable _ in true } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) $0.shell.run = { @Sendable cmd, path in switch cmd { case .gitCheckout(branch: "main", quiet: true) where path.hasSuffix("foo-1"): @@ -166,7 +166,7 @@ extension AllTests.AnalyzeErrorTests { } return true } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) } operation: { // MUT try await Analyze.analyze(client: app.client, diff --git a/Tests/AppTests/AnalyzerTests.swift b/Tests/AppTests/AnalyzerTests.swift index 0afde6554..28db4db9a 100644 --- a/Tests/AppTests/AnalyzerTests.swift +++ b/Tests/AppTests/AnalyzerTests.swift @@ -1059,6 +1059,7 @@ extension AllTests.AnalyzerTests { // points to the correct version of Xcode! try await withDependencies { $0.fileManager.fileExists = FileManagerClient.liveValue.fileExists(atPath:) + $0.logger = .noop $0.shell = .liveValue } operation: { // setup @@ -1080,6 +1081,7 @@ extension AllTests.AnalyzerTests { // See also https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/1441 try await withDependencies { $0.fileManager.fileExists = FileManagerClient.liveValue.fileExists(atPath:) + $0.logger = .noop $0.shell = .liveValue } operation: { // setup @@ -1100,6 +1102,7 @@ extension AllTests.AnalyzerTests { // points to the correct version of Xcode! try await withDependencies { $0.fileManager.fileExists = FileManagerClient.liveValue.fileExists(atPath:) + $0.logger = .noop $0.shell = .liveValue } operation: { // setup @@ -1126,6 +1129,9 @@ extension AllTests.AnalyzerTests { // NB: If this test fails on macOS make sure xcode-select -p // points to the correct version of Xcode! // setup + try await withDependencies { + $0.logger = .noop + } operation: { try await withTempDir { @Sendable tempDir in let fixture = fixturesDirectory() .appendingPathComponent("5.9-Package-swift").path @@ -1149,6 +1155,7 @@ extension AllTests.AnalyzerTests { #endif } } + } @Test func issue_577() async throws { // Duplicate "latest release" versions @@ -1456,7 +1463,7 @@ extension AllTests.AnalyzerTests { 1\tPerson 2 """ } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) $0.shell.run = { @Sendable _, _ in return "" } } operation: { let pkgId = UUID() diff --git a/Tests/AppTests/ErrorReportingTests.swift b/Tests/AppTests/ErrorReportingTests.swift index a05b1e60c..2b040f1e9 100644 --- a/Tests/AppTests/ErrorReportingTests.swift +++ b/Tests/AppTests/ErrorReportingTests.swift @@ -44,7 +44,7 @@ extension AllTests.ErrorReportingTests { try await withDependencies { $0.date.now = .now $0.github.fetchMetadata = { @Sendable _, _ throws(Github.Error) in throw Github.Error.invalidURL("1") } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10)) @@ -63,7 +63,7 @@ extension AllTests.ErrorReportingTests { let capturingLogger = CapturingLogger() try await withDependencies { $0.fileManager.fileExists = { @Sendable _ in true } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) $0.shell.run = { @Sendable cmd, _ in if cmd.description == "git tag" { return "1.0.0" } // returning a blank string will cause an exception when trying to diff --git a/Tests/AppTests/GithubTests.swift b/Tests/AppTests/GithubTests.swift index 4e8ee5b03..fa12e4d09 100644 --- a/Tests/AppTests/GithubTests.swift +++ b/Tests/AppTests/GithubTests.swift @@ -178,6 +178,7 @@ extension AllTests.GithubTests { await withDependencies { $0.github.token = { "secr3t" } $0.httpClient.post = { @Sendable _, _, _ in .badRequest } + $0.logger = .noop } operation: { do { _ = try await Github.fetchMetadata(owner: "alamofire", @@ -219,6 +220,7 @@ extension AllTests.GithubTests { await withDependencies { $0.github.token = { "secr3t" } $0.httpClient.post = { @Sendable _, _, _ in .tooManyRequests } + $0.logger = .noop } operation: { // MUT do { @@ -273,7 +275,7 @@ extension AllTests.GithubTests { $0.httpClient.post = { @Sendable _, _, _ in .init(status: .forbidden, headers: ["X-RateLimit-Remaining": "0"]) } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) } operation: { // MUT do { @@ -367,6 +369,7 @@ extension AllTests.GithubTests { await withDependencies { $0.github.token = { "secr3t" } $0.httpClient.get = { @Sendable _, headers in .notFound } + $0.logger = .noop } operation: { // MUT let res = await Github.fetchReadme(owner: "foo", repository: "bar") diff --git a/Tests/AppTests/GitlabBuilderTests.swift b/Tests/AppTests/GitlabBuilderTests.swift index dc04f74e6..1cf299319 100644 --- a/Tests/AppTests/GitlabBuilderTests.swift +++ b/Tests/AppTests/GitlabBuilderTests.swift @@ -82,6 +82,7 @@ extension AllTests.GitlabBuilderTests { ) return try .created(jsonEncode: Gitlab.Builder.Response(webUrl: "http://web_url")) } + $0.logger = .noop } operation: { // MUT _ = try await Gitlab.Builder.triggerBuild(buildId: buildId, @@ -112,6 +113,7 @@ extension AllTests.GitlabBuilderTests { #expect(swiftVersion == "6.0") return try .created(jsonEncode: Gitlab.Builder.Response(webUrl: "http://web_url")) } + $0.logger = .noop } operation: { // MUT _ = try await Gitlab.Builder.triggerBuild(buildId: .id0, diff --git a/Tests/AppTests/Helpers/TestSupport.swift b/Tests/AppTests/Helpers/TestSupport.swift index d4128a133..670fdd48d 100644 --- a/Tests/AppTests/Helpers/TestSupport.swift +++ b/Tests/AppTests/Helpers/TestSupport.swift @@ -23,6 +23,10 @@ func withApp(_ setup: (Application) async throws -> Void = { _ in }, _ updateValuesForOperation: (inout DependencyValues) async throws -> Void = { _ in }, environment: Environment = .testing, _ test: (Application) async throws -> Void) async throws { + prepareDependencies { + $0.logger = .noop + } + try await TestSupport.setupDb(environment) let app = try await TestSupport.setupApp(environment) @@ -47,10 +51,6 @@ enum TestSupport { static func setupApp(_ environment: Environment, databasePort: Int? = nil) async throws -> Application { let app = try await Application.make(environment) try await configure(app, databasePort: databasePort) - - // Silence app logging - app.logger = .init(label: "noop") { _ in SwiftLogNoOpLogHandler() } - return app } diff --git a/Tests/AppTests/IngestionTests.swift b/Tests/AppTests/IngestionTests.swift index c56e239a5..7b29e009a 100644 --- a/Tests/AppTests/IngestionTests.swift +++ b/Tests/AppTests/IngestionTests.swift @@ -413,7 +413,7 @@ extension AllTests.IngestionTests { summary: "desc") } $0.github.fetchReadme = { @Sendable _, _ in nil } - $0.logger.set(to: capturingLogger) + $0.logger = .testLogger(capturingLogger) } operation: { // MUT try await Ingestion.ingest(client: app.client, database: app.db, mode: .limit(10))