Tidy up FluentKit (#609)

* Bump dependency version requirements, add missing target deps, remove useless Swift feature flags * Code style cleanups * Add missing async versions of methods and missing SQLDatabase declarations, remove unneeded uses of UnsafeTransfer, use SQLKit's version of SQLQUalifiedTable * Rename the various versions of `.sql(raw:)` to `.sql(unsafeRaw:)`, deprecating the old name. * Don't reimplement `FieldKey.description` in SQLSchemaConverter, just use it * Run the SQLKit benchmarks as part of running the Fluent benchmarks. * Update tests for deprecations
vapor · May 14, 2024 · cb91bf9 · cb91bf9
1 parent ed4cfa9
commit cb91bf9
Show file tree

Hide file tree

Showing 26 changed files with 254 additions and 182 deletions.
diff --git a/Package.swift b/Package.swift
@@ -16,10 +16,10 @@ let package = Package(
         .library(name: "XCTFluent", targets: ["XCTFluent"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.62.0"),
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.65.0"),
         .package(url: "https://github.com/apple/swift-log.git", from: "1.5.4"),
-        .package(url: "https://github.com/vapor/sql-kit.git", from: "3.29.2"),
-        .package(url: "https://github.com/vapor/async-kit.git", from: "1.17.0"),
+        .package(url: "https://github.com/vapor/sql-kit.git", from: "3.29.3"),
+        .package(url: "https://github.com/vapor/async-kit.git", from: "1.19.0"),
     ],
     targets: [
         .target(
@@ -38,6 +38,8 @@ let package = Package(
             dependencies: [
                 .target(name: "FluentKit"),
                 .target(name: "FluentSQL"),
+                .product(name: "SQLKit", package: "sql-kit"),
+                .product(name: "SQLKitBenchmark", package: "sql-kit"),
             ],
             swiftSettings: swiftSettings
         ),
@@ -72,10 +74,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]
@@ -16,10 +16,10 @@ let package = Package(
         .library(name: "XCTFluent", targets: ["XCTFluent"]),
     ],
     dependencies: [
-        .package(url: "https://github.com/apple/swift-nio.git", from: "2.62.0"),
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.65.0"),
         .package(url: "https://github.com/apple/swift-log.git", from: "1.5.4"),
-        .package(url: "https://github.com/vapor/sql-kit.git", from: "3.29.2"),
-        .package(url: "https://github.com/vapor/async-kit.git", from: "1.17.0"),
+        .package(url: "https://github.com/vapor/sql-kit.git", from: "3.29.3"),
+        .package(url: "https://github.com/vapor/async-kit.git", from: "1.19.0"),
     ],
     targets: [
         .target(
@@ -38,6 +38,8 @@ let package = Package(
             dependencies: [
                 .target(name: "FluentKit"),
                 .target(name: "FluentSQL"),
+                .product(name: "SQLKit", package: "sql-kit"),
+                .product(name: "SQLKitBenchmark", package: "sql-kit"),
             ],
             swiftSettings: swiftSettings
         ),
@@ -73,10 +75,6 @@ var swiftSettings: [SwiftSetting] { [
     .enableUpcomingFeature("ExistentialAny"),
     .enableUpcomingFeature("ConciseMagicFile"),
     .enableUpcomingFeature("ForwardTrailingClosures"),
-    .enableUpcomingFeature("ImportObjcForwardDeclarations"),
     .enableUpcomingFeature("DisableOutwardActorInference"),
-    .enableUpcomingFeature("IsolatedDefaultValues"),
-    .enableUpcomingFeature("GlobalConcurrency"),
-    .enableUpcomingFeature("StrictConcurrency"),
     .enableExperimentalFeature("StrictConcurrency=complete"),
 ] }
diff --git a/Sources/FluentBenchmark/Exports.swift b/Sources/FluentBenchmark/Exports.swift
@@ -1,11 +1,2 @@
-#if swift(>=5.8)
-
 @_documentation(visibility: internal) @_exported import FluentKit
 @_documentation(visibility: internal) @_exported import XCTest
-
-#else
-
-@_exported import FluentKit
-@_exported import XCTest
-
-#endif
diff --git a/Sources/FluentBenchmark/FluentBenchmarker.swift b/Sources/FluentBenchmark/FluentBenchmarker.swift
@@ -8,6 +8,7 @@ public final class FluentBenchmarker {
 
     public init(databases: Databases) {
         precondition(databases.ids().count >= 2, "FluentBenchmarker Databases instance must have 2 or more registered databases")
+
         self.databases = databases
         self.database = self.databases.database(
             logger: .init(label: "codes.vapor.fluent.benchmarker"),

diff --git a/Sources/FluentBenchmark/Tests/ChildTests.swift b/Sources/FluentBenchmark/Tests/ChildTests.swift
@@ -267,7 +267,9 @@ private final class Player: Model, @unchecked Sendable {
 
     init(
         id: Int? = nil,
-        name: String, gameID: Game.IDValue) {
+        name: String,
+        gameID: Game.IDValue
+    ) {
         self.id = id
         self.name = name
         self.$game.id = gameID

diff --git a/Sources/FluentBenchmark/Tests/FilterTests.swift b/Sources/FluentBenchmark/Tests/FilterTests.swift
@@ -45,7 +45,7 @@ extension FluentBenchmarker {
             SolarSystem()
         ]) {
             let moon = try Moon.query(on: self.database)
-                .filter(\.$name == .sql(raw: "'Moon'"))
+                .filter(\.$name == .sql(unsafeRaw: "'Moon'"))
                 .first()
                 .wait()
 

diff --git a/Sources/FluentBenchmark/Tests/SQLTests.swift b/Sources/FluentBenchmark/Tests/SQLTests.swift
@@ -1,15 +1,20 @@
 import FluentKit
 import Foundation
 import NIOCore
+import NIOPosix
 import XCTest
 import SQLKit
+import SQLKitBenchmark
 
 extension FluentBenchmarker {
     public func testSQL() throws {
         guard let sql = self.database as? any SQLDatabase else {
             return
         }
         try self.testSQL_rawDecode(sql)
+        try MultiThreadedEventLoopGroup.singleton.any().makeFutureWithTask {
+            try await SQLBenchmarker(on: sql).runAllTests()
+        }.wait()
     }
 
     private func testSQL_rawDecode(_ sql: any SQLDatabase) throws {

diff --git a/Sources/FluentBenchmark/Tests/SchemaTests.swift b/Sources/FluentBenchmark/Tests/SchemaTests.swift
@@ -96,15 +96,15 @@ extension FluentBenchmarker {
                 .constraint(.sql(embed: "CONSTRAINT \(normalized1) UNIQUE (\(ident: "id"))"))
 
                 // Test raw SQL for table constraint definitions (but not names):
-                .constraint(.constraint(.sql(raw: "UNIQUE (id)"), name: "id_unq_2"))
+                .constraint(.constraint(.sql(unsafeRaw: "UNIQUE (id)"), name: "id_unq_2"))
                 .constraint(.constraint(.sql(embed: "UNIQUE (\(ident: "id"))"), name: "id_unq_3"))
 
                 .create().wait()
 
             if (self.database as! any SQLDatabase).dialect.alterTableSyntax.allowsBatch {
                 try self.database.schema("custom_constraints")
                     // Test raw SQL for dropping constraints:
-                    .deleteConstraint(.sql(embed: "\(SQLDropTypedConstraint(name: SQLIdentifier("id_unq_1"), algorithm: .sql(raw: "")))"))
+                    .deleteConstraint(.sql(embed: "\(SQLDropTypedConstraint(name: SQLIdentifier("id_unq_1"), algorithm: .sql(unsafeRaw: "")))"))
                     .update().wait()
             }
         }
@@ -127,7 +127,7 @@ extension FluentBenchmarker {
                 .field("neverbeid", .string, .sql(embed: "NOT NULL"))
 
                 // Test raw SQL for entire field definitions:
-                .field(.sql(raw: "idnah INTEGER NOT NULL"))
+                .field(.sql(unsafeRaw: "idnah INTEGER NOT NULL"))
                 .field(.sql(embed: "\(ident: "notid") INTEGER"))
 
                 .create().wait()

diff --git a/Sources/FluentKit/Concurrency/Database+Concurrency.swift b/Sources/FluentKit/Concurrency/Database+Concurrency.swift
@@ -1,16 +1,35 @@
 import NIOCore
 
 public extension Database {
+    func execute(
+        query: DatabaseQuery,
+        onOutput: @escaping @Sendable (any DatabaseOutput) -> ()
+    ) async throws {
+        try await self.execute(query: query, onOutput: onOutput).get()
+    }
+
+    func execute(
+        schema: DatabaseSchema
+    ) async throws {
+        try await self.execute(schema: schema).get()
+    }
+
+    func execute(
+        enum: DatabaseEnum
+    ) async throws {
+        try await self.execute(enum: `enum`).get()
+    }
+
     func transaction<T>(_ closure: @escaping @Sendable (any Database) async throws -> T) async throws -> T {
-        try await self.transaction { db -> EventLoopFuture<T> in
+        try await self.transaction { db in
             self.eventLoop.makeFutureWithTask {
                 try await closure(db)
             }
         }.get()
     }
 
     func withConnection<T>(_ closure: @escaping @Sendable (any Database) async throws -> T) async throws -> T {
-        try await self.withConnection { db -> EventLoopFuture<T> in
+        try await self.withConnection { db in
             self.eventLoop.makeFutureWithTask {
                 try await closure(db)
             }

diff --git a/Sources/FluentKit/Database/Database+Logging.swift b/Sources/FluentKit/Database/Database+Logging.swift
@@ -33,7 +33,6 @@ extension LoggingOverrideDatabase: Database {
     func execute(
         schema: DatabaseSchema
     ) -> EventLoopFuture<Void> {
-
         self.database.execute(schema: schema)
     }
 
@@ -60,6 +59,13 @@ extension LoggingOverrideDatabase: SQLDatabase where D: SQLDatabase {
     func execute(sql query: any SQLExpression, _ onRow: @escaping @Sendable (any SQLRow) -> ()) -> EventLoopFuture<Void> {
         self.database.execute(sql: query, onRow)
     }
+    func execute(sql query: any SQLExpression, _ onRow: @escaping @Sendable (any SQLRow) -> ()) async throws {
+        try await self.database.execute(sql: query, onRow)
+    }
+    func withSession<R>(_ closure: @escaping @Sendable (any SQLDatabase) async throws -> R) async throws -> R {
+        try await self.database.withSession(closure)
+    }
     var dialect: any SQLDialect { self.database.dialect }
     var version: (any SQLDatabaseReportedVersion)? { self.database.version }
+    var queryLogLevel: Logger.Level? { self.database.queryLogLevel }
 }
diff --git a/Sources/FluentKit/Database/DatabaseID.swift b/Sources/FluentKit/Database/DatabaseID.swift
@@ -1,5 +1,6 @@
 public struct DatabaseID: Hashable, Codable, Sendable {
     public let string: String
+
     public init(string: String) {
         self.string = string
     }

diff --git a/Sources/FluentKit/Database/TransactionControlDatabase.swift b/Sources/FluentKit/Database/TransactionControlDatabase.swift
@@ -3,12 +3,12 @@ import NIOCore
 /// Protocol for describing a database that allows fine-grained control over transcactions
 /// when you need more control than provided by ``Database/transaction(_:)-1x3ds``
 ///
-/// ⚠️ **WARNING**: it is the developer's responsiblity to get hold of a ``Database``,
-/// execute the transaction functions on that connection, and ensure that the functions aren't called across
-/// different conenctions. You are also responsible for ensuring that you commit or rollback queries
-/// when you're ready.
+/// > Warning: ⚠️ It is the developer's responsiblity to get hold of a ``Database``, execute the
+/// > transaction functions on that connection, and ensure that the functions aren't called across
+/// > different conenctions. You are also responsible for ensuring that you commit or rollback
+/// > queries when you're ready.
 ///
-/// Do not mix these functions and `Database.transaction(_:)`.
+/// Do not mix these functions and ``Database/transaction(_:)-1x3ds``.
 public protocol TransactionControlDatabase: Database {
     /// Start the transaction on the current connection. This is equivalent to an SQL `BEGIN`
     /// - Returns: future `Void` when the transaction has been started

diff --git a/Sources/FluentKit/FluentError.swift b/Sources/FluentKit/FluentError.swift
@@ -96,8 +96,8 @@ extension FluentError {
 /// An error describing a failure during an an operation on an ``SiblingsProperty``.
 ///
 /// > Note: This should just be another case on ``FluentError``, not a separate error type, but at the time
-///   of this writing, non-frozen enums are still not available to non-stdlib packages, so to avoid source
-///   breakage we chose this as the least annoying of the several annoying workarounds.
+/// > of this writing, non-frozen enums are still not available to non-stdlib packages, so to avoid source
+/// > breakage we chose this as the least annoying of the several annoying workarounds.
 public enum SiblingsPropertyError: Error, LocalizedError, CustomStringConvertible, CustomDebugStringConvertible {
     /// An attempt was made to query, attach to, or detach from a siblings property whose owning model's ID
     /// is not currently known (usually because that model has not yet been saved to the database).

diff --git a/Sources/FluentKit/Middleware/ModelMiddleware.swift b/Sources/FluentKit/Middleware/ModelMiddleware.swift
@@ -40,29 +40,29 @@ extension ModelMiddleware {
     }
 
     public func create(model: Model, on db: any Database, next: any AnyModelResponder) -> EventLoopFuture<Void> {
-        return next.create(model, on: db)
+        next.create(model, on: db)
     }
 
     public func update(model: Model, on db: any Database, next: any AnyModelResponder) -> EventLoopFuture<Void> {
-        return next.update(model, on: db)
+        next.update(model, on: db)
     }
 
     public func delete(model: Model, force: Bool, on db: any Database, next: any AnyModelResponder) -> EventLoopFuture<Void> {
-        return next.delete(model, force: force, on: db)
+        next.delete(model, force: force, on: db)
     }
 
     public func softDelete(model: Model, on db: any Database, next: any AnyModelResponder) -> EventLoopFuture<Void> {
-        return next.softDelete(model, on: db)
+        next.softDelete(model, on: db)
     }
 
     public func restore(model: Model, on db: any Database, next: any AnyModelResponder) -> EventLoopFuture<Void> {
-        return next.restore(model, on: db)
+        next.restore(model, on: db)
     }
 }
 
 extension AnyModelMiddleware {
     func makeResponder(chainingTo responder: any AnyModelResponder) -> any AnyModelResponder {
-        return ModelMiddlewareResponder(middleware: self, responder: responder)
+        ModelMiddlewareResponder(middleware: self, responder: responder)
     }
 }
 
@@ -84,7 +84,7 @@ private struct ModelMiddlewareResponder: AnyModelResponder {
     var responder: any AnyModelResponder
 
     func handle(_ event: ModelEvent, _ model: any AnyModel, on db: any Database) -> EventLoopFuture<Void> {
-        return self.middleware.handle(event, model, on: db, chainingTo: responder)
+        self.middleware.handle(event, model, on: db, chainingTo: responder)
     }
 }
 

diff --git a/Sources/FluentKit/Middleware/ModelResponder.swift b/Sources/FluentKit/Middleware/ModelResponder.swift
@@ -10,23 +10,23 @@ public protocol AnyModelResponder: Sendable {
 
 extension AnyModelResponder {
     public func create(_ model: any AnyModel, on db: any Database) -> EventLoopFuture<Void> {
-        return handle(.create, model, on: db)
+        self.handle(.create, model, on: db)
     }
 
     public func update(_ model: any AnyModel, on db: any Database) -> EventLoopFuture<Void> {
-        return handle(.update, model, on: db)
+        self.handle(.update, model, on: db)
     }
 
     public func restore(_ model: any AnyModel, on db: any Database) -> EventLoopFuture<Void> {
-        return handle(.restore, model, on: db)
+        self.handle(.restore, model, on: db)
     }
 
     public func softDelete(_ model: any AnyModel, on db: any Database) -> EventLoopFuture<Void> {
-        return handle(.softDelete, model, on: db)
+        self.handle(.softDelete, model, on: db)
     }
 
     public func delete(_ model: any AnyModel, force: Bool, on db: any Database) -> EventLoopFuture<Void> {
-        return handle(.delete(force), model, on: db)
+        self.handle(.delete(force), model, on: db)
     }
 }
 
@@ -39,7 +39,7 @@ internal struct BasicModelResponder<Model>: AnyModelResponder where Model: Fluen
         }
 
         do {
-            return try _handle(event, modelType, db)
+            return try self._handle(event, modelType, db)
         } catch {
             return db.eventLoop.makeFailedFuture(error)
         }

diff --git a/Sources/FluentKit/Migration/Migration.swift b/Sources/FluentKit/Migration/Migration.swift
@@ -28,7 +28,7 @@ extension Migration {
     }
 
     internal var defaultName: String {
-#if compiler(>=5.3) && compiler(<6)
+        #if compiler(<6)
         /// `String.init(reflecting:)` creates a `Mirror` unconditionally, but
         /// when the parameter is a metatype (such as is the case here), that
         /// mirror is never actually used for anything. Unfortunately, just
@@ -39,9 +39,9 @@ extension Migration {
         /// runtime function directly instead of taking the huge speed hit just
         /// because the leading underscore makes it harder to ignore the
         /// fragility of the usage.
-        return Swift._typeName(Self.self, qualified: true)
-#else
-        return String(reflecting: Self.self)
-#endif
+        Swift._typeName(Self.self, qualified: true)
+        #else
+        String(reflecting: Self.self)
+        #endif
     }
 }