Fix Fluent model encoding for SQLKit and several Sendable warnings (#624)

* Require Swift 5.9, remove obsolete workaround

* Resolve new Sendable warnings produced by Swift 6 compiler

* Make Fluent model encoding for SQLKit stable

* Fix misuse of EmbeddedEventLoop in tests and disable unfixable test

Author: gwynne

Package.swift

@@ -1,4 +1,4 @@
-// swift-tools-version:5.8
+// swift-tools-version:5.9
 import PackageDescription
 
 let package = Package(
@@ -16,9 +16,9 @@ let package = Package(
         .library(name: "XCTFluent", targets: ["XCTFluent"]),
     ],
     dependencies: [
-        .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.3"),
+        .package(url: "https://github.com/apple/swift-nio.git", from: "2.79.0"),
+        .package(url: "https://github.com/apple/swift-log.git", from: "1.6.2"),
+        .package(url: "https://github.com/vapor/sql-kit.git", from: "3.32.0"),
         .package(url: "https://github.com/vapor/async-kit.git", from: "1.20.0"),
     ],
     targets: [
@@ -72,6 +72,9 @@ let package = Package(
 )
 
 var swiftSettings: [SwiftSetting] { [
+    .enableUpcomingFeature("ExistentialAny"),
     .enableUpcomingFeature("ConciseMagicFile"),
     .enableUpcomingFeature("ForwardTrailingClosures"),
+    .enableUpcomingFeature("DisableOutwardActorInference"),
+    .enableExperimentalFeature("StrictConcurrency=complete"),
 ] }

[email protected]

@@ -20,15 +20,15 @@ public extension Database {
         try await self.execute(enum: `enum`).get()
     }
 
-    func transaction<T>(_ closure: @escaping @Sendable (any Database) async throws -> T) async throws -> T {
+    func transaction<T: Sendable>(_ closure: @escaping @Sendable (any Database) async throws -> T) async throws -> T {
         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 {
+    func withConnection<T: Sendable>(_ closure: @escaping @Sendable (any Database) async throws -> T) async throws -> T {
         try await self.withConnection { db in
             self.eventLoop.makeFutureWithTask {
                 try await closure(db)

Sources/FluentKit/Concurrency/Database+Concurrency.swift

@@ -7,11 +7,7 @@ extension Fields {
         let container = try decoder.container(keyedBy: SomeCodingKey.self)
 
         for (key, property) in self.codableProperties {
-#if swift(<5.7.1)
-            let propDecoder = WorkaroundSuperDecoder(container: container, key: key)
-#else
             let propDecoder = try container.superDecoder(forKey: key)
-#endif
             do {
                 try property.decode(from: propDecoder)
             } catch {
@@ -40,29 +36,3 @@ extension Fields {
         }
     }
 }
-
-#if swift(<5.7.1)
-/// This ``Decoder`` compensates for a bug in `KeyedDecodingContainerProtocol.superDecoder(forKey:)` on Linux
-/// which first appeared in Swift 5.5 and was fixed in Swift 5.7.1.
-///
-/// When a given key is not present in the input JSON, `.superDecoder(forKey:)` is expected to return a valid
-/// ``Decoder`` that will only decode a nil value. However, in affected versions of Swift, the method instead
-/// throws a ``DecodingError/keyNotFound``.
-///
-/// As a workaround, instead of calling `.superDecoder(forKey:)`, an instance of this type is created and
-/// provided with the decoding container; the apporiate decoding methods are intercepted to provide the
-/// desired semantics, with everything else being forwarded directly to the container. This has a minor but
-/// nonzero impact on performance, but was determined to be the best and cleanest option.
-private struct WorkaroundSuperDecoder<K: CodingKey>: Decoder, SingleValueDecodingContainer {
-    var codingPath: [CodingKey] { self.container.codingPath }
-    var userInfo: [CodingUserInfoKey: Any] { [:] }
-    let container: KeyedDecodingContainer<K>
-    let key: K
-
-    func container<NK: CodingKey>(keyedBy: NK.Type) throws -> KeyedDecodingContainer<NK> { try self.container.nestedContainer(keyedBy: NK.self, forKey: self.key) }
-    func unkeyedContainer() throws -> UnkeyedDecodingContainer { try self.container.nestedUnkeyedContainer(forKey: self.key) }
-    func singleValueContainer() throws -> SingleValueDecodingContainer { self }
-    func decode<T: Decodable>(_: T.Type) throws -> T { try self.container.decode(T.self, forKey: self.key) }
-    func decodeNil() -> Bool { self.container.contains(self.key) ? try! self.container.decodeNil(forKey: self.key) : true }
-}
-#endif

Sources/FluentKit/Model/Fields+Codable.swift

@@ -6,7 +6,7 @@ import NIOCore
 /// > Note: This protocol should probably require conformance to ``Property``, but adding that requirement
 /// > wouldn't have enough value to be worth having to hand-wave a technically semver-major change.
 public protocol Relation: Sendable {
-    associatedtype RelatedValue
+    associatedtype RelatedValue: Sendable
     var name: String { get }
     var value: RelatedValue? { get set }
     func load(on database: any Database) -> EventLoopFuture<Void>

Sources/FluentKit/Properties/Relation.swift

@@ -50,7 +50,7 @@ extension QueryBuilder {
 }
 
 /// A single section of a larger, traversable result set.
-public struct Page<T> {
+public struct Page<T: Sendable>: Sendable {
     /// The page's items. Usually models.
     public let items: [T]
 
@@ -76,7 +76,7 @@ extension Page: Encodable where T: Encodable {}
 extension Page: Decodable where T: Decodable {}
 
 /// Metadata for a given `Page`.
-public struct PageMetadata: Codable {
+public struct PageMetadata: Codable, Sendable {
     /// Current page number. Starts at `1`.
     public let page: Int
 

Sources/FluentKit/Query/Builder/QueryBuilder+Paginate.swift

@@ -83,7 +83,7 @@ extension DatabaseQuery.Value {
 
 extension Model {
     fileprivate func encodeForSQL(withDefaultedValues: Bool) -> [(String, any SQLExpression)] {
-        self.collectInput(withDefaultedValues: withDefaultedValues).map { ($0.description, $1.asSQLExpression) }
+        self.collectInput(withDefaultedValues: withDefaultedValues).map { ($0.description, $1.asSQLExpression) }.sorted(by: { $0.0 < $1.0 })
     }
 }
 

Sources/FluentSQL/SQLDatabase+Model.swift

@@ -1,6 +1,6 @@
 import FluentKit
 import FluentSQL
-import NIOEmbedded
+import NIOCore
 import SQLKit
 import XCTFluent
 import NIOConcurrencyHelpers
@@ -60,7 +60,7 @@ final class DummyDatabaseForTestSQLSerializer: Database, SQLDatabase {
         self.context = .init(
             configuration: Configuration(),
             logger: .init(label: "test"),
-            eventLoop: EmbeddedEventLoop()
+            eventLoop: NIOSingletons.posixEventLoopGroup.any()
         )
     }
 
@@ -81,12 +81,14 @@ final class DummyDatabaseForTestSQLSerializer: Database, SQLDatabase {
         var sqlSerializer = SQLSerializer(database: self)
         query.serialize(to: &sqlSerializer)
         self._sqlSerializers.withLockedValue { $0.append(sqlSerializer) }
-        if !self.fakedRows.isEmpty {
-            for row in self._fakedRows.withLockedValue({ $0.removeFirst() }) {
+        return self.eventLoop.submit {
+            let rows = self._fakedRows.withLockedValue {
+                $0.isEmpty ? [] : $0.removeFirst()
+            }
+            for row in rows {
                 onRow(row)
             }
         }
-        return self.eventLoop.makeSucceededVoidFuture()
     }
 
     func transaction<T>(_ closure: @escaping @Sendable (any Database) -> EventLoopFuture<T>) -> EventLoopFuture<T> {

Tests/FluentKitTests/DummyDatabaseForTestSQLSerializer.swift

@@ -23,6 +23,10 @@ final class FluentKitTests: XCTestCase {
     /// Since no part of Fluent or any of its drivers currently relies, or ever will rely, on
     /// the format in question, it is desirable to enforce that it should never change, just in
     /// case someone actually is relying on it for some hopefully very good reason.
+    ///
+    /// Update: Ignore all of the above. This test is not reliable due to the instability of serializing
+    /// dictionaries as strings, and adding sorting changes the output, so the whole point is mooted.
+    /*
     func testAnyModelDescriptionFormatHasNotChanged() throws {
         final class Foo: Model, @unchecked Sendable {
             static let schema = "foos"
@@ -47,7 +51,8 @@ final class FluentKitTests: XCTestCase {
         XCTAssertEqual(modelOutputDesc, "Foo(output: [num: 42, name: \"Test\", id: \(model.id!)])")
         XCTAssertEqual(modelBothDesc,   "Foo(output: [num: 42, name: \"Test\", id: \(model.id!)], input: [num: 43])")
     }
-    
+    */
+
     func testMigrationLogNames() throws {
         XCTAssertEqual(MigrationLog.path(for: \.$id), [.id])
         XCTAssertEqual(MigrationLog.path(for: \.$name), ["name"])

Tests/FluentKitTests/FluentKitTests.swift

@@ -158,13 +158,13 @@ final class SQLTests: DbQueryTestCase {
         try await self.db.insert(into: FromPivot.schema).fluentModels([fromPivot1, fromPivot2]).run()
 
         XCTAssertEqual(self.db.sqlSerializers.count, 4)
-        XCTAssertEqual(self.db.sqlSerializers.dropFirst(0).first?.sql, #"INSERT INTO "model1s" ("optfield", "id", "bool", "optbool", "group_groupfield2", "group_groupfield1", "created_at", "enum", "field", "optenum") VALUES ($1, DEFAULT, $2, $3, $4, $5, $6, 'foo', $7, 'bar'), (NULL, $8, $9, NULL, $10, $11, $12, 'foo', $13, NULL)"#)
+        XCTAssertEqual(self.db.sqlSerializers.dropFirst(0).first?.sql, #"INSERT INTO "model1s" ("bool", "created_at", "enum", "field", "group_groupfield1", "group_groupfield2", "id", "optbool", "optenum", "optfield") VALUES ($1, $2, 'foo', $3, $4, $5, DEFAULT, $6, 'bar', $7), ($8, $9, 'foo', $10, $11, $12, $13, NULL, NULL, NULL)"#)
         XCTAssertEqual(self.db.sqlSerializers.dropFirst(0).first?.binds.count, 13)
-        XCTAssertEqual(self.db.sqlSerializers.dropFirst(1).first?.sql, #"INSERT INTO "model2s" ("id", "model1_id", "field", "othermodel1_id") VALUES (DEFAULT, $1, $2, $3), ($4, $5, $6, NULL)"#)
+        XCTAssertEqual(self.db.sqlSerializers.dropFirst(1).first?.sql, #"INSERT INTO "model2s" ("field", "id", "model1_id", "othermodel1_id") VALUES ($1, DEFAULT, $2, $3), ($4, $5, $6, NULL)"#)
         XCTAssertEqual(self.db.sqlSerializers.dropFirst(1).first?.binds.count, 6)
-        XCTAssertEqual(self.db.sqlSerializers.dropFirst(2).first?.sql, #"INSERT INTO "pivots" ("model2_id", "model1_id") VALUES ($1, $2)"#)
+        XCTAssertEqual(self.db.sqlSerializers.dropFirst(2).first?.sql, #"INSERT INTO "pivots" ("model1_id", "model2_id") VALUES ($1, $2)"#)
         XCTAssertEqual(self.db.sqlSerializers.dropFirst(2).first?.binds.count, 2)
-        XCTAssertEqual(self.db.sqlSerializers.dropFirst(3).first?.sql, #"INSERT INTO "from_pivots" ("pivot_model2_id", "optpivot_model1_id", "pivot_model1_id", "field2", "optpivot_model2_id", "field1") VALUES ($1, $2, $3, $4, $5, $6), ($7, NULL, $8, $9, NULL, $10)"#)
+        XCTAssertEqual(self.db.sqlSerializers.dropFirst(3).first?.sql, #"INSERT INTO "from_pivots" ("field1", "field2", "optpivot_model1_id", "optpivot_model2_id", "pivot_model1_id", "pivot_model2_id") VALUES ($1, $2, $3, $4, $5, $6), ($7, $8, NULL, NULL, $9, $10)"#)
         XCTAssertEqual(self.db.sqlSerializers.dropFirst(3).first?.binds.count, 10)
     }
 }