Concurrency updates and database logging APIs (#775)

* Bump dependency versions and clean up language flags * Make it easier to override the logger on a database when requesting it (since `.logging(to:)` doesn't usually work as intended) * Use the `willBootAsync` lifecycle callback to avoid calling `.wait()` when auto-migration is requested on the commandline. * Code style, improve the console output in the migration command * Make all the tests fully async and update them to use Application.make() etc. * Remove pointless Dependabot config
vapor · May 30, 2024 · dfcbeba · dfcbeba
1 parent d831ac5
commit dfcbeba
Show file tree

Hide file tree

Showing 12 changed files with 270 additions and 288 deletions.
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
@@ -1,5 +1,4 @@
 version: 2
-enable-beta-ecosystems: true
 updates:
   - package-ecosystem: "github-actions"
     directory: "/"
@@ -11,14 +10,3 @@ updates:
       dependencies:
         patterns:
           - "*"
-  - package-ecosystem: "swift"
-    directory: "/"
-    schedule:
-      interval: "daily"
-    open-pull-requests-limit: 6
-    allow:
-      - dependency-type: all
-    groups:
-      all-dependencies:
-        patterns:
-          - "*"
diff --git a/Package.swift b/Package.swift
@@ -13,8 +13,8 @@ let package = Package(
         .library(name: "Fluent", targets: ["Fluent"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/vapor/fluent-kit.git", from: "1.48.0"),
-        .package(url: "https://github.com/vapor/vapor.git", from: "4.94.1"),
+        .package(url: "https://github.com/vapor/fluent-kit.git", from: "1.48.4"),
+        .package(url: "https://github.com/vapor/vapor.git", from: "4.101.0"),
     ],
     targets: [
         .target(
@@ -40,10 +40,4 @@ let package = Package(
 var swiftSettings: [SwiftSetting] { [
     .enableUpcomingFeature("ConciseMagicFile"),
     .enableUpcomingFeature("ForwardTrailingClosures"),
-    .enableUpcomingFeature("ImportObjcForwardDeclarations"),
-    .enableUpcomingFeature("DisableOutwardActorInference"),
-    .enableUpcomingFeature("IsolatedDefaultValues"),
-    .enableUpcomingFeature("GlobalConcurrency"),
-    .enableUpcomingFeature("StrictConcurrency"),
-    .enableExperimentalFeature("StrictConcurrency=complete"),
 ] }
diff --git a/[email protected] b/[email protected]
@@ -13,8 +13,8 @@ let package = Package(
         .library(name: "Fluent", targets: ["Fluent"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/vapor/fluent-kit.git", from: "1.48.0"),
-        .package(url: "https://github.com/vapor/vapor.git", from: "4.94.1"),
+        .package(url: "https://github.com/vapor/fluent-kit.git", from: "1.48.4"),
+        .package(url: "https://github.com/vapor/vapor.git", from: "4.101.0"),
     ],
     targets: [
         .target(
@@ -43,8 +43,5 @@ var swiftSettings: [SwiftSetting] { [
     .enableUpcomingFeature("ForwardTrailingClosures"),
     .enableUpcomingFeature("ImportObjcForwardDeclarations"),
     .enableUpcomingFeature("DisableOutwardActorInference"),
-    .enableUpcomingFeature("IsolatedDefaultValues"),
-    .enableUpcomingFeature("GlobalConcurrency"),
-    .enableUpcomingFeature("StrictConcurrency"),
     .enableExperimentalFeature("StrictConcurrency=complete"),
 ] }
diff --git a/Sources/Fluent/FluentProvider.swift b/Sources/Fluent/FluentProvider.swift
@@ -12,15 +12,20 @@ extension Request {
     }
 
     public func db(_ id: DatabaseID?) -> any Database {
-        self.application
-            .databases
-            .database(
-                id,
-                logger: self.logger,
-                on: self.eventLoop,
-                history: self.fluent.history.historyEnabled ? self.fluent.history.history : nil,
-                pageSizeLimit: self.fluent.pagination.pageSizeLimit != nil ? self.fluent.pagination.pageSizeLimit?.value : self.application.fluent.pagination.pageSizeLimit
-            )!
+        self.db(id, logger: self.logger)
+    }
+
+    public func db(_ id: DatabaseID?, logger: Logger) -> any Database {
+        self.application.databases.database(
+            id,
+            logger: logger,
+            on: self.eventLoop,
+            history: self.fluent.history.historyEnabled ? self.fluent.history.history : nil,
+            // Use map() (not flatMap()) so if pageSizeLimit is non-nil but the value is nil
+            // the request's "no limit" setting overrides the app's setting.
+            pageSizeLimit: self.fluent.pagination.pageSizeLimit.map(\.value) ??
+                           self.application.fluent.pagination.pageSizeLimit
+        )!
     }
 
     public var fluent: Fluent {
@@ -34,14 +39,17 @@ extension Application {
     }
 
     public func db(_ id: DatabaseID?) -> any Database {
-        self.databases
-            .database(
-                id,
-                logger: self.logger,
-                on: self.eventLoopGroup.any(),
-                history: self.fluent.history.historyEnabled ? self.fluent.history.history : nil,
-                pageSizeLimit: self.fluent.pagination.pageSizeLimit
-            )!
+        self.db(id, logger: self.logger)
+    }
+
+    public func db(_ id: DatabaseID?, logger: Logger) -> any Database {
+        self.databases.database(
+            id,
+            logger: logger,
+            on: self.eventLoopGroup.any(),
+            history: self.fluent.history.historyEnabled ? self.fluent.history.history : nil,
+            pageSizeLimit: self.fluent.pagination.pageSizeLimit
+        )!
     }
 
     public var databases: Databases {
@@ -53,7 +61,7 @@ extension Application {
     }
 
     public var migrator: Migrator {
-        Migrator(
+        .init(
             databases: self.databases,
             migrations: self.migrations,
             logger: self.logger,
@@ -85,10 +93,7 @@ extension Application {
             let migrationLogLevel: NIOLockedValueBox<Logger.Level>
 
             init(threadPool: NIOThreadPool, on eventLoopGroup: any EventLoopGroup, migrationLogLevel: Logger.Level) {
-                self.databases = Databases(
-                    threadPool: threadPool,
-                    on: eventLoopGroup
-                )
+                self.databases = Databases(threadPool: threadPool, on: eventLoopGroup)
                 self.migrations = .init()
                 self.migrationLogLevel = .init(migrationLogLevel)
             }
@@ -99,25 +104,35 @@ extension Application {
         }
 
         struct Lifecycle: LifecycleHandler {
-            func willBoot(_ application: Application) throws {
-                struct Signature: CommandSignature {
-                    @Flag(name: "auto-migrate", help: "If true, Fluent will automatically migrate your database on boot")
-                    var autoMigrate: Bool
-
-                    @Flag(name: "auto-revert", help: "If true, Fluent will automatically revert your database on boot")
-                    var autoRevert: Bool
+            struct Signature: CommandSignature {
+                @Flag(name: "auto-migrate", help: "If true, Fluent will automatically migrate your database on boot")
+                var autoMigrate: Bool
 
-                    init() {}
-                }
+                @Flag(name: "auto-revert", help: "If true, Fluent will automatically revert your database on boot")
+                var autoRevert: Bool
+            }
 
+            func willBoot(_ application: Application) throws {
                 let signature = try Signature(from: &application.environment.commandInput)
+
                 if signature.autoRevert {
                     try application.autoRevert().wait()
                 }
                 if signature.autoMigrate {
                     try application.autoMigrate().wait()
                 }
             }
+
+            func willBootAsync(_ application: Application) async throws {
+                let signature = try Signature(from: &application.environment.commandInput)
+
+                if signature.autoRevert {
+                    try await application.autoRevert()
+                }
+                if signature.autoMigrate {
+                    try await application.autoMigrate()
+                }
+            }
 
             func shutdown(_ application: Application) {
                 application.databases.shutdown()
@@ -148,21 +163,11 @@ extension Application {
             nonmutating set { self.storage.migrationLogLevel.withLockedValue { $0 = newValue } }
         }
 
-        public var history: History {
-            .init(fluent: self)
-        }
-
-        public struct History {
-            let fluent: Fluent
-        }
+        public struct History { let fluent: Fluent }
+        public var history: History { .init(fluent: self) }
 
-        public var pagination: Pagination {
-            .init(fluent: self)
-        }
-
-        public struct Pagination {
-            let fluent: Fluent
-        }
+        public struct Pagination { let fluent: Fluent }
+        public var pagination: Pagination { .init(fluent: self) }
     }
 
     public var fluent: Fluent {

diff --git a/Sources/Fluent/MigrateCommand.swift b/Sources/Fluent/MigrateCommand.swift
@@ -30,16 +30,12 @@ public final class MigrateCommand: AsyncCommand {
 
     private func revert(using context: CommandContext) async throws {
         let migrations = try await context.application.migrator.previewRevertLastBatch().get()
-        guard migrations.count > 0 else {
-            context.console.print("No migrations to revert.")
-            return
+        guard !migrations.isEmpty else {
+            return context.console.print("No migrations to revert.")
         }
         context.console.print("The following migration(s) will be reverted:")
         for (migration, dbid) in migrations {
-            context.console.print("- ", newLine: false)
-            context.console.error(migration.name, newLine: false)
-            context.console.print(" on ", newLine: false)
-            context.console.print(dbid?.string ?? "default")
+            context.console.output("- \(migration.name, color: .red) on \(dbid?.string ?? "<default>", style: .info)")
         }
         if context.console.confirm("Would you like to continue?".consoleText(.warning)) {
             try await context.application.migrator.revertLastBatch().get()
@@ -51,16 +47,12 @@ public final class MigrateCommand: AsyncCommand {
 
     private func prepare(using context: CommandContext) async throws {
         let migrations = try await context.application.migrator.previewPrepareBatch().get()
-        guard migrations.count > 0 else {
-            context.console.print("No new migrations.")
-            return
+        guard !migrations.isEmpty else {
+            return context.console.print("No new migrations.")
         }
         context.console.print("The following migration(s) will be prepared:")
         for (migration, dbid) in migrations {
-            context.console.print("+ ", newLine: false)
-            context.console.success(migration.name, newLine: false)
-            context.console.print(" on ", newLine: false)
-            context.console.print(dbid?.string ?? "default")
+            context.console.output("+ \(migration.name, color: .green) on \(dbid?.string ?? "<default>", style: .info)")
         }
         if context.console.confirm("Would you like to continue?".consoleText(.warning)) {
             try await context.application.migrator.prepareBatch().get()

diff --git a/Tests/FluentTests/CacheTests.swift b/Tests/FluentTests/CacheTests.swift
@@ -4,26 +4,34 @@ import Fluent
 import Vapor
 
 final class CacheTests: XCTestCase {
+    var app: Application!
+
+    override func setUp() async throws {
+        self.app = try await Application.make(.testing)
+    }
+
+    override func tearDown() async throws {
+        try await self.app.asyncShutdown()
+        self.app = nil
+    }
+
     func testCacheMigrationName() {
         XCTAssertEqual(CacheEntry.migration.name, "Fluent.CacheEntry.Create")
     }
 
     func testCacheGet() async throws {
-        let app = Application(.testing)
-        defer { app.shutdown() }
-
         // Setup test db.
         let test = ArrayTestDatabase()
-        app.databases.use(test.configuration, as: .test)
-        app.migrations.add(CacheEntry.migration)
+        self.app.databases.use(test.configuration, as: .test)
+        self.app.migrations.add(CacheEntry.migration)
 
         // Configure cache.
-        app.caches.use(.fluent)
+        self.app.caches.use(.fluent)
 
         // simulate cache miss
         test.append([])
         do {
-            let foo = try await app.cache.get("foo", as: String.self)
+            let foo = try await self.app.cache.get("foo", as: String.self)
             XCTAssertNil(foo)
         }
 
@@ -33,15 +41,12 @@ final class CacheTests: XCTestCase {
             "value": "\"bar\""
         ])])
         do {
-            let foo = try await app.cache.get("foo", as: String.self)
+            let foo = try await self.app.cache.get("foo", as: String.self)
             XCTAssertEqual(foo, "bar")
         }
     }
 
     func testCacheSet() async throws {
-        let app = Application(.testing)
-        defer { app.shutdown() }
-
         // Setup test db.
         let test = CallbackTestDatabase { query in
             switch query.input[0] {
@@ -51,19 +56,16 @@ final class CacheTests: XCTestCase {
                     XCTAssertEqual(value, "\"bar\"")
                 default: XCTFail("unexpected value")
                 }
-
             default: XCTFail("unexpected input")
             }
-            return [
-                TestOutput(["id": UUID()])
-            ]
+            return [TestOutput(["id": UUID()])]
         }
-        app.databases.use(test.configuration, as: .test)
-        app.migrations.add(CacheEntry.migration)
+        self.app.databases.use(test.configuration, as: .test)
+        self.app.migrations.add(CacheEntry.migration)
 
         // Configure cache.
-        app.caches.use(.fluent)
+        self.app.caches.use(.fluent)
 
-        try await app.cache.set("foo", to: "bar")
+        try await self.app.cache.set("foo", to: "bar")
     }
 }