From 71e3b54917af0cce0f710ca5e55709521912db39 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:38:14 -0500 Subject: [PATCH 01/11] Remove numerous Sendable warning workarounds that are no longer needed --- .../Concurrency/AsyncModelMiddleware.swift | 12 +++---- .../Concurrency/Model+Concurrency.swift | 2 +- .../ModelResponder+Concurrency.swift | 6 ++-- Sources/FluentKit/Model/Model+CRUD.swift | 36 ++++++++----------- .../Properties/BooleanPropertyFormat.swift | 16 ++++----- Sources/FluentKit/Properties/Children.swift | 8 ++--- .../Properties/CompositeChildren.swift | 6 ++-- .../FluentKit/Properties/CompositeID.swift | 4 +-- .../Properties/CompositeOptionalChild.swift | 8 ++--- .../Properties/CompositeOptionalParent.swift | 15 ++++---- .../Properties/CompositeParent.swift | 11 +++--- Sources/FluentKit/Properties/Group.swift | 2 +- Sources/FluentKit/Properties/ID.swift | 22 ++++-------- .../FluentKit/Properties/OptionalChild.swift | 11 +++--- .../FluentKit/Properties/OptionalField.swift | 8 ++--- .../FluentKit/Properties/OptionalParent.swift | 22 ++++++------ Sources/FluentKit/Properties/Parent.swift | 13 ++++--- Sources/FluentKit/Properties/Property.swift | 2 +- Sources/FluentKit/Properties/Siblings.swift | 15 ++++---- Sources/FluentKit/Properties/Timestamp.swift | 4 +-- .../Query/Builder/QueryBuilder.swift | 8 ++--- Sources/FluentKit/Schema/DatabaseSchema.swift | 1 - Sources/FluentKit/Schema/SchemaBuilder.swift | 2 +- .../FluentKit/Utilities/OptionalType.swift | 2 +- .../Utilities/UnsafeMutableTransferBox.swift | 4 --- 25 files changed, 99 insertions(+), 141 deletions(-) diff --git a/Sources/FluentKit/Concurrency/AsyncModelMiddleware.swift b/Sources/FluentKit/Concurrency/AsyncModelMiddleware.swift index 28bbb707..e9838610 100644 --- a/Sources/FluentKit/Concurrency/AsyncModelMiddleware.swift +++ b/Sources/FluentKit/Concurrency/AsyncModelMiddleware.swift @@ -17,7 +17,7 @@ extension AsyncModelMiddleware { on db: any Database, chainingTo next: any AnyModelResponder ) -> EventLoopFuture { - guard let modelType = (model as? Model).map({ UnsafeTransfer(wrappedValue: $0) }) else { + guard let modelType = (model as? Model) else { return next.handle(event, model, on: db) } @@ -28,15 +28,15 @@ extension AsyncModelMiddleware { switch event { case .create: - try await self.create(model: modelType.wrappedValue, on: db, next: responder) + try await self.create(model: modelType, on: db, next: responder) case .update: - try await self.update(model: modelType.wrappedValue, on: db, next: responder) + try await self.update(model: modelType, on: db, next: responder) case .delete(let force): - try await self.delete(model: modelType.wrappedValue, force: force, on: db, next: responder) + try await self.delete(model: modelType, force: force, on: db, next: responder) case .softDelete: - try await self.softDelete(model: modelType.wrappedValue, on: db, next: responder) + try await self.softDelete(model: modelType, on: db, next: responder) case .restore: - try await self.restore(model: modelType.wrappedValue, on: db, next: responder) + try await self.restore(model: modelType, on: db, next: responder) } } } diff --git a/Sources/FluentKit/Concurrency/Model+Concurrency.swift b/Sources/FluentKit/Concurrency/Model+Concurrency.swift index dcf8d239..4bf3ae34 100644 --- a/Sources/FluentKit/Concurrency/Model+Concurrency.swift +++ b/Sources/FluentKit/Concurrency/Model+Concurrency.swift @@ -30,7 +30,7 @@ public extension Model { } } -public extension Collection where Element: FluentKit.Model { +public extension Collection where Element: FluentKit.Model, Self: Sendable { func delete(force: Bool = false, on database: any Database) async throws { try await self.delete(force: force, on: database).get() } diff --git a/Sources/FluentKit/Concurrency/ModelResponder+Concurrency.swift b/Sources/FluentKit/Concurrency/ModelResponder+Concurrency.swift index e2898ed9..dff70113 100644 --- a/Sources/FluentKit/Concurrency/ModelResponder+Concurrency.swift +++ b/Sources/FluentKit/Concurrency/ModelResponder+Concurrency.swift @@ -10,10 +10,8 @@ public protocol AnyAsyncModelResponder: AnyModelResponder { extension AnyAsyncModelResponder { func handle(_ event: ModelEvent, _ model: any AnyModel, on db: any Database) -> EventLoopFuture { - let model = UnsafeTransfer(wrappedValue: model) - - return db.eventLoop.makeFutureWithTask { - try await self.handle(event, model.wrappedValue, on: db) + db.eventLoop.makeFutureWithTask { + try await self.handle(event, model, on: db) } } } diff --git a/Sources/FluentKit/Model/Model+CRUD.swift b/Sources/FluentKit/Model/Model+CRUD.swift index 524a8465..37202915 100644 --- a/Sources/FluentKit/Model/Model+CRUD.swift +++ b/Sources/FluentKit/Model/Model+CRUD.swift @@ -17,7 +17,6 @@ extension Model { } private func _create(on database: any Database) -> EventLoopFuture { - let transfer = UnsafeTransfer(wrappedValue: self) precondition(!self._$idExists) self.touchTimestamps(.create, .update) if self.anyID is any AnyQueryableProperty { @@ -29,12 +28,12 @@ extension Model { .run { promise.succeed($0) } .cascadeFailure(to: promise) return promise.futureResult.flatMapThrowing { output in - var input = transfer.wrappedValue.collectInput() - if case .default = transfer.wrappedValue._$id.inputValue { + var input = self.collectInput() + if case .default = self._$id.inputValue { let idKey = Self()._$id.key input[idKey] = try .bind(output.decode(idKey, as: Self.IDValue.self)) } - try transfer.wrappedValue.output(from: SavedInput(input)) + try self.output(from: SavedInput(input)) } } else { return Self.query(on: database) @@ -42,7 +41,7 @@ extension Model { .action(.create) .run() .flatMapThrowing { - try transfer.wrappedValue.output(from: SavedInput(transfer.wrappedValue.collectInput())) + try self.output(from: SavedInput(self.collectInput())) } } } @@ -61,14 +60,13 @@ extension Model { self.touchTimestamps(.update) let input = self.collectInput() guard let id = self.id else { throw FluentError.idRequired } - let transfer = UnsafeTransfer(wrappedValue: self) return Self.query(on: database) .filter(id: id) .set(input) .update() .flatMapThrowing { - try transfer.wrappedValue.output(from: SavedInput(input)) + try self.output(from: SavedInput(input)) } } @@ -87,14 +85,13 @@ extension Model { private func _delete(force: Bool = false, on database: any Database) throws -> EventLoopFuture { guard let id = self.id else { throw FluentError.idRequired } - let transfer = UnsafeTransfer(wrappedValue: self) return Self.query(on: database) .filter(id: id) .delete(force: force) .map { - if force || transfer.wrappedValue.deletedTimestamp == nil { - transfer.wrappedValue._$idExists = false + if force || self.deletedTimestamp == nil { + self._$idExists = false } } } @@ -112,7 +109,6 @@ extension Model { timestamp.touch(date: nil) precondition(self._$idExists) guard let id = self.id else { throw FluentError.idRequired } - let transfer = UnsafeTransfer(wrappedValue: self) return Self.query(on: database) .withDeleted() .filter(id: id) @@ -121,8 +117,8 @@ extension Model { .run() .flatMapThrowing { - try transfer.wrappedValue.output(from: SavedInput(transfer.wrappedValue.collectInput())) - transfer.wrappedValue._$idExists = true + try self.output(from: SavedInput(self.collectInput())) + self._$idExists = true } } @@ -142,7 +138,7 @@ extension Model { } } -extension Collection where Element: FluentKit.Model { +extension Collection where Element: FluentKit.Model, Self: Sendable { public func delete(force: Bool = false, on database: any Database) -> EventLoopFuture { guard !self.isEmpty else { return database.eventLoop.makeSucceededFuture(()) @@ -150,20 +146,18 @@ extension Collection where Element: FluentKit.Model { precondition(self.allSatisfy { $0._$idExists }) - let transfer = UnsafeTransfer(wrappedValue: self) // ouch, the retains... - return EventLoopFuture.andAllSucceed(self.map { model in database.configuration.middleware.chainingTo(Element.self) { event, model, db in db.eventLoop.makeSucceededFuture(()) }.delete(model, force: force, on: database) }, on: database.eventLoop).flatMap { Element.query(on: database) - .filter(ids: transfer.wrappedValue.map { $0.id! }) + .filter(ids: self.map { $0.id! }) .delete(force: force) }.map { guard force else { return } - for model in transfer.wrappedValue where model.deletedTimestamp == nil { + for model in self where model.deletedTimestamp == nil { model._$idExists = false } } @@ -176,8 +170,6 @@ extension Collection where Element: FluentKit.Model { precondition(self.allSatisfy { !$0._$idExists }) - let transfer = UnsafeTransfer(wrappedValue: self) // ouch, the retains... - return EventLoopFuture.andAllSucceed(self.enumerated().map { idx, model in database.configuration.middleware.chainingTo(Element.self) { event, model, db in if model.anyID is any AnyQueryableProperty { @@ -188,10 +180,10 @@ extension Collection where Element: FluentKit.Model { }.create(model, on: database) }, on: database.eventLoop).flatMap { Element.query(on: database) - .set(transfer.wrappedValue.map { $0.collectInput(withDefaultedValues: database is any SQLDatabase) }) + .set(self.map { $0.collectInput(withDefaultedValues: database is any SQLDatabase) }) .create() }.map { - for model in transfer.wrappedValue { + for model in self { model._$idExists = true } } diff --git a/Sources/FluentKit/Properties/BooleanPropertyFormat.swift b/Sources/FluentKit/Properties/BooleanPropertyFormat.swift index 83bee2b7..8e7e1fb8 100644 --- a/Sources/FluentKit/Properties/BooleanPropertyFormat.swift +++ b/Sources/FluentKit/Properties/BooleanPropertyFormat.swift @@ -13,11 +13,11 @@ public struct DefaultBooleanPropertyFormat: BooleanPropertyFormat { public init() {} public func parse(_ value: Bool) -> Bool? { - return value + value } public func serialize(_ bool: Bool) -> Bool { - return bool + bool } } @@ -42,7 +42,7 @@ public struct IntegerBooleanPropertyFormat T { - return .zero.advanced(by: bool ? 1 : 0) + .zero.advanced(by: bool ? 1 : 0) } } @@ -63,7 +63,7 @@ public struct OneZeroBooleanPropertyFormat: BooleanPropertyFormat { } public func serialize(_ bool: Bool) -> String { - return bool ? "1" : "0" + bool ? "1" : "0" } } @@ -84,7 +84,7 @@ public struct YNBooleanPropertyFormat: BooleanPropertyFormat { } public func serialize(_ bool: Bool) -> String { - return bool ? "Y" : "N" + bool ? "Y" : "N" } } @@ -106,7 +106,7 @@ public struct YesNoBooleanPropertyFormat: BooleanPropertyFormat { } public func serialize(_ bool: Bool) -> String { - return bool ? "YES" : "NO" + bool ? "YES" : "NO" } } @@ -127,7 +127,7 @@ public struct OnOffBooleanPropertyFormat: BooleanPropertyFormat { } public func serialize(_ bool: Bool) -> String { - return bool ? "ON" : "OFF" + bool ? "ON" : "OFF" } } @@ -148,7 +148,7 @@ public struct TrueFalseBooleanPropertyFormat: BooleanPropertyFormat { } public func serialize(_ bool: Bool) -> String { - return bool ? "true" : "false" + bool ? "true" : "false" } } diff --git a/Sources/FluentKit/Properties/Children.swift b/Sources/FluentKit/Properties/Children.swift index 56a7cc43..f8dacb62 100644 --- a/Sources/FluentKit/Properties/Children.swift +++ b/Sources/FluentKit/Properties/Children.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers extension Model { public typealias Children = ChildrenProperty @@ -44,11 +43,11 @@ public final class ChildrenProperty: @unchecked Sendable } public var projectedValue: ChildrenProperty { - return self + self } public var fromId: From.IDValue? { - get { return self.idValue } + get { self.idValue } set { self.idValue = newValue } } @@ -221,9 +220,8 @@ private struct ChildrenEagerLoader: EagerLoader if (self.withDeleted) { builder.withDeleted() } - let models = UnsafeTransfer(wrappedValue: models) return builder.all().map { - for model in models.wrappedValue { + for model in models { let id = model[keyPath: self.relationKey].idValue! model[keyPath: self.relationKey].value = $0.filter { child in switch parentKey { diff --git a/Sources/FluentKit/Properties/CompositeChildren.swift b/Sources/FluentKit/Properties/CompositeChildren.swift index 1d62014d..793d7a1b 100644 --- a/Sources/FluentKit/Properties/CompositeChildren.swift +++ b/Sources/FluentKit/Properties/CompositeChildren.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers extension Model { /// A convenience alias for ``CompositeChildrenProperty``. It is strongly recommended that callers use this @@ -107,7 +106,7 @@ public final class CompositeChildrenProperty: @unchecked Sendable public var projectedValue: CompositeChildrenProperty { self } public var fromId: From.IDValue? { - get { return self.idValue } + get { self.idValue } set { self.idValue = newValue } } @@ -201,11 +200,10 @@ private struct CompositeChildrenEagerLoader: EagerLoader _ = parentKey.queryFilterIds(ids, in: query) } - let models = UnsafeTransfer(wrappedValue: models) return builder.all().map { let indexedResults = Dictionary(grouping: $0, by: { parentKey.referencedId(in: $0)! }) - for model in models.wrappedValue { + for model in models { model[keyPath: self.relationKey].value = indexedResults[model[keyPath: self.relationKey].idValue!] ?? [] } } diff --git a/Sources/FluentKit/Properties/CompositeID.swift b/Sources/FluentKit/Properties/CompositeID.swift index 28db8878..b9d91344 100644 --- a/Sources/FluentKit/Properties/CompositeID.swift +++ b/Sources/FluentKit/Properties/CompositeID.swift @@ -18,7 +18,7 @@ public final class CompositeIDProperty: @unchecked Sendable public var projectedValue: CompositeIDProperty { self } public var wrappedValue: Value? { - get { return self.value } + get { self.value } set { self.value = newValue } } @@ -29,7 +29,7 @@ public final class CompositeIDProperty: @unchecked Sendable ) -> Nested where Nested: Property { - return self.value![keyPath: keyPath] + self.value![keyPath: keyPath] } } diff --git a/Sources/FluentKit/Properties/CompositeOptionalChild.swift b/Sources/FluentKit/Properties/CompositeOptionalChild.swift index 4e8cdebc..7ff6a62f 100644 --- a/Sources/FluentKit/Properties/CompositeOptionalChild.swift +++ b/Sources/FluentKit/Properties/CompositeOptionalChild.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers extension Model { /// A convenience alias for ``CompositeOptionalChildProperty``. It is strongly recommended that callers use this @@ -92,7 +91,7 @@ public final class CompositeOptionalChildProperty: @unchecked Sendable public var projectedValue: CompositeOptionalChildProperty { self } public var fromId: From.IDValue? { - get { return self.idValue } + get { self.idValue } set { self.idValue = newValue } } @@ -185,14 +184,13 @@ private struct CompositeOptionalChildEagerLoader: EagerLoader builder.group(.or) { query in _ = parentKey.queryFilterIds(ids, in: query) } - if (self.withDeleted) { + if self.withDeleted { builder.withDeleted() } - let models = UnsafeTransfer(wrappedValue: models) return builder.all().map { let indexedResults = Dictionary(grouping: $0, by: { parentKey.referencedId(in: $0)! }) - for model in models.wrappedValue { + for model in models { model[keyPath: self.relationKey].value = indexedResults[model[keyPath: self.relationKey].idValue!]?.first } } diff --git a/Sources/FluentKit/Properties/CompositeOptionalParent.swift b/Sources/FluentKit/Properties/CompositeOptionalParent.swift index a20bf958..6297ccd5 100644 --- a/Sources/FluentKit/Properties/CompositeOptionalParent.swift +++ b/Sources/FluentKit/Properties/CompositeOptionalParent.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers import struct SQLKit.SomeCodingKey extension Model { @@ -104,7 +103,7 @@ public final class CompositeOptionalParentProperty: @unchecked Sendabl } public func query(on database: any Database) -> QueryBuilder { - return To.query(on: database).group(.and) { + To.query(on: database).group(.and) { self.id?.input(to: QueryFilterInput(builder: $0)) ?? To.IDValue().input(to: QueryFilterInput(builder: $0).nullValueOveridden()) } } @@ -220,19 +219,19 @@ private struct CompositeOptionalParentEagerLoader: EagerLoader func run(models: [From], on database: any Database) -> EventLoopFuture { var _sets = Dictionary(grouping: models, by: { $0[keyPath: self.relationKey].id }) - let nilParentModels = UnsafeTransfer(wrappedValue: _sets.removeValue(forKey: nil) ?? []) - let sets = UnsafeTransfer(wrappedValue: _sets) + let nilParentModels = _sets.removeValue(forKey: nil) ?? [] + let sets = _sets let builder = To.query(on: database) - .group(.or) { _ = sets.wrappedValue.keys.reduce($0) { query, id in query.group(.and) { id!.input(to: QueryFilterInput(builder: $0)) } } } - if (self.withDeleted) { + .group(.or) { _ = sets.keys.reduce($0) { query, id in query.group(.and) { id!.input(to: QueryFilterInput(builder: $0)) } } } + if self.withDeleted { builder.withDeleted() } return builder.all().flatMapThrowing { let parents = Dictionary(uniqueKeysWithValues: $0.map { ($0.id!, $0) }) - for (parentId, models) in sets.wrappedValue { + for (parentId, models) in sets { guard let parent = parents[parentId!] else { database.logger.debug( "Missing parent model in eager-load lookup results.", @@ -242,7 +241,7 @@ private struct CompositeOptionalParentEagerLoader: EagerLoader } models.forEach { $0[keyPath: self.relationKey].value = .some(.some(parent)) } } - nilParentModels.wrappedValue.forEach { $0[keyPath: self.relationKey].value = .some(.none) } + nilParentModels.forEach { $0[keyPath: self.relationKey].value = .some(.none) } } } } diff --git a/Sources/FluentKit/Properties/CompositeParent.swift b/Sources/FluentKit/Properties/CompositeParent.swift index 9c9b4d81..45c8bda9 100644 --- a/Sources/FluentKit/Properties/CompositeParent.swift +++ b/Sources/FluentKit/Properties/CompositeParent.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers import struct SQLKit.SomeCodingKey extension Model { @@ -98,7 +97,7 @@ public final class CompositeParentProperty: @unchecked Sendable } public func query(on database: any Database) -> QueryBuilder { - return To.query(on: database).group(.and) { self.id.input(to: QueryFilterInput(builder: $0)) } + To.query(on: database).group(.and) { self.id.input(to: QueryFilterInput(builder: $0)) } } public subscript(dynamicMember keyPath: KeyPath) -> Nested @@ -195,20 +194,20 @@ private struct CompositeParentEagerLoader: EagerLoader let withDeleted: Bool func run(models: [From], on database: any Database) -> EventLoopFuture { - let sets = UnsafeTransfer(wrappedValue: Dictionary(grouping: models, by: { $0[keyPath: self.relationKey].id })) + let sets = Dictionary(grouping: models, by: { $0[keyPath: self.relationKey].id }) let builder = To.query(on: database) .group(.or) { - _ = sets.wrappedValue.keys.reduce($0) { query, id in query.group(.and) { id.input(to: QueryFilterInput(builder: $0)) } } + _ = sets.keys.reduce($0) { query, id in query.group(.and) { id.input(to: QueryFilterInput(builder: $0)) } } } - if (self.withDeleted) { + if self.withDeleted { builder.withDeleted() } return builder.all() .flatMapThrowing { let parents = Dictionary(uniqueKeysWithValues: $0.map { ($0.id!, $0) }) - for (parentId, models) in sets.wrappedValue { + for (parentId, models) in sets { guard let parent = parents[parentId] else { database.logger.debug( "Missing parent model in eager-load lookup results.", diff --git a/Sources/FluentKit/Properties/Group.swift b/Sources/FluentKit/Properties/Group.swift index 977c16f2..1b326880 100644 --- a/Sources/FluentKit/Properties/Group.swift +++ b/Sources/FluentKit/Properties/Group.swift @@ -15,7 +15,7 @@ public final class GroupProperty: @unchecked Sendable public var value: Value? public var projectedValue: GroupProperty { - return self + self } public var wrappedValue: Value { diff --git a/Sources/FluentKit/Properties/ID.swift b/Sources/FluentKit/Properties/ID.swift index b3e9624f..97eb96dc 100644 --- a/Sources/FluentKit/Properties/ID.swift +++ b/Sources/FluentKit/Properties/ID.swift @@ -33,29 +33,21 @@ public final class IDProperty: @unchecked Sendable var cachedOutput: (any DatabaseOutput)? public var key: FieldKey { - return self.field.key + self.field.key } var inputValue: DatabaseQuery.Value? { - get { - return self.field.inputValue - } - set { - self.field.inputValue = newValue - } + get { self.field.inputValue } + set { self.field.inputValue = newValue } } public var projectedValue: IDProperty { - return self + self } public var wrappedValue: Value? { - get { - return self.value - } - set { - self.value = newValue - } + get { self.value } + set { self.value = newValue } } /// Initializes an `ID` property with the key `.id` and type `UUID`. @@ -131,7 +123,7 @@ extension IDProperty: AnyProperty { } extension IDProperty: Property { public var value: Value? { get { - return self.field.value ?? nil + self.field.value ?? nil } set { self.field.value = newValue diff --git a/Sources/FluentKit/Properties/OptionalChild.swift b/Sources/FluentKit/Properties/OptionalChild.swift index 372f8f05..ea19915a 100644 --- a/Sources/FluentKit/Properties/OptionalChild.swift +++ b/Sources/FluentKit/Properties/OptionalChild.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers extension Model { public typealias OptionalChild = OptionalChildProperty @@ -44,11 +43,11 @@ public final class OptionalChildProperty: @unchecked Sendable } public var projectedValue: OptionalChildProperty { - return self + self } public var fromId: From.IDValue? { - get { return self.idValue } + get { self.idValue } set { self.idValue = newValue } } @@ -203,12 +202,12 @@ private struct OptionalChildEagerLoader: EagerLoader case .required(let required): builder.filter(required.appending(path: \.$id) ~~ Set(ids)) } - if (self.withDeleted) { + if self.withDeleted { builder.withDeleted() } - let models = UnsafeTransfer(wrappedValue: models) + let models = models return builder.all().map { - for model in models.wrappedValue { + for model in models { let id = model[keyPath: self.relationKey].idValue! let children = $0.filter { child in switch parentKey { diff --git a/Sources/FluentKit/Properties/OptionalField.swift b/Sources/FluentKit/Properties/OptionalField.swift index 4b0c6b6a..ddbfe5fc 100644 --- a/Sources/FluentKit/Properties/OptionalField.swift +++ b/Sources/FluentKit/Properties/OptionalField.swift @@ -20,12 +20,8 @@ public final class OptionalFieldProperty: @unchecked Sendab } public var wrappedValue: WrappedValue? { - get { - self.value ?? nil - } - set { - self.value = .some(newValue) - } + get { self.value ?? nil } + set { self.value = .some(newValue) } } public init(key: FieldKey) { diff --git a/Sources/FluentKit/Properties/OptionalParent.swift b/Sources/FluentKit/Properties/OptionalParent.swift index 4dff02a4..df2232b2 100644 --- a/Sources/FluentKit/Properties/OptionalParent.swift +++ b/Sources/FluentKit/Properties/OptionalParent.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers import struct SQLKit.SomeCodingKey extension Model { @@ -26,7 +25,7 @@ public final class OptionalParentProperty: @unchecked Sendable } public var projectedValue: OptionalParentProperty { - return self + self } public var value: To?? @@ -130,8 +129,7 @@ extension OptionalParentProperty: AnyCodableProperty { extension OptionalParentProperty: EagerLoadable { public static func eagerLoad( - _ relationKey: KeyPath>, + _ relationKey: KeyPath>, to builder: Builder ) where Builder : EagerLoadBuilder, From == Builder.Model @@ -173,23 +171,23 @@ private struct OptionalParentEagerLoader: EagerLoader func run(models: [From], on database: any Database) -> EventLoopFuture { var _sets = Dictionary(grouping: models, by: { $0[keyPath: self.relationKey].id }) - let nilParentModels = UnsafeTransfer(wrappedValue: _sets.removeValue(forKey: nil) ?? []) - let sets = UnsafeTransfer(wrappedValue: _sets) + let nilParentModels = _sets.removeValue(forKey: nil) ?? [] + let sets = _sets - if sets.wrappedValue.isEmpty { + if sets.isEmpty { // Fetching "To" objects is unnecessary when no models have an id for "To". - nilParentModels.wrappedValue.forEach { $0[keyPath: self.relationKey].value = .some(.none) } + nilParentModels.forEach { $0[keyPath: self.relationKey].value = .some(.none) } return database.eventLoop.makeSucceededVoidFuture() } - let builder = To.query(on: database).filter(\._$id ~~ Set(sets.wrappedValue.keys.compactMap { $0 })) - if (self.withDeleted) { + let builder = To.query(on: database).filter(\._$id ~~ Set(sets.keys.compactMap { $0 })) + if self.withDeleted { builder.withDeleted() } return builder.all().flatMapThrowing { let parents = Dictionary(uniqueKeysWithValues: $0.map { ($0.id!, $0) }) - for (parentId, models) in sets.wrappedValue { + for (parentId, models) in sets { guard let parent = parents[parentId!] else { database.logger.debug( "Missing parent model in eager-load lookup results.", @@ -199,7 +197,7 @@ private struct OptionalParentEagerLoader: EagerLoader } models.forEach { $0[keyPath: self.relationKey].value = .some(.some(parent)) } } - nilParentModels.wrappedValue.forEach { $0[keyPath: self.relationKey].value = .some(.none) } + nilParentModels.forEach { $0[keyPath: self.relationKey].value = .some(.none) } } } } diff --git a/Sources/FluentKit/Properties/Parent.swift b/Sources/FluentKit/Properties/Parent.swift index cb01cc95..310fe2eb 100644 --- a/Sources/FluentKit/Properties/Parent.swift +++ b/Sources/FluentKit/Properties/Parent.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers import struct SQLKit.SomeCodingKey extension Model { @@ -27,7 +26,7 @@ public final class ParentProperty: @unchecked Sendable } public var projectedValue: ParentProperty { - return self + self } public var value: To? @@ -41,7 +40,7 @@ public final class ParentProperty: @unchecked Sendable } public func query(on database: any Database) -> QueryBuilder { - return To.query(on: database) + To.query(on: database) .filter(\._$id == self.id) } } @@ -166,15 +165,15 @@ private struct ParentEagerLoader: EagerLoader let withDeleted: Bool func run(models: [From], on database: any Database) -> EventLoopFuture { - let sets = UnsafeTransfer(wrappedValue: Dictionary(grouping: models, by: { $0[keyPath: self.relationKey].id })) - let builder = To.query(on: database).filter(\._$id ~~ Set(sets.wrappedValue.keys)) - if (self.withDeleted) { + let sets = Dictionary(grouping: models, by: { $0[keyPath: self.relationKey].id }) + let builder = To.query(on: database).filter(\._$id ~~ Set(sets.keys)) + if self.withDeleted { builder.withDeleted() } return builder.all().flatMapThrowing { let parents = Dictionary(uniqueKeysWithValues: $0.map { ($0.id!, $0) }) - for (parentId, models) in sets.wrappedValue { + for (parentId, models) in sets { guard let parent = parents[parentId] else { database.logger.debug( "Missing parent model in eager-load lookup results.", diff --git a/Sources/FluentKit/Properties/Property.swift b/Sources/FluentKit/Properties/Property.swift index 41a8c614..7e88790a 100644 --- a/Sources/FluentKit/Properties/Property.swift +++ b/Sources/FluentKit/Properties/Property.swift @@ -131,7 +131,7 @@ extension AnyQueryableProperty where Self: QueryableProperty { /// "identical encoding for identical property types" rule (see /// ``QueryableProperty/queryValue(_:)-5df0n``). public func queryableValue() -> DatabaseQuery.Value? { - return self.value.map { Self.queryValue($0) } + self.value.map { Self.queryValue($0) } } } diff --git a/Sources/FluentKit/Properties/Siblings.swift b/Sources/FluentKit/Properties/Siblings.swift index e1c618ca..be3c160b 100644 --- a/Sources/FluentKit/Properties/Siblings.swift +++ b/Sources/FluentKit/Properties/Siblings.swift @@ -1,5 +1,4 @@ import NIOCore -import NIOConcurrencyHelpers extension Model { public typealias Siblings = SiblingsProperty @@ -64,11 +63,11 @@ public final class SiblingsProperty: @unchecked Sendable } public var projectedValue: SiblingsProperty { - return self + self } public var fromId: From.IDValue? { - get { return self.idValue } + get { self.idValue } set { self.idValue = newValue } } @@ -155,13 +154,12 @@ public final class SiblingsProperty: @unchecked Sendable case .always: return self.attach(to, on: database, edit) case .ifNotExists: - let to = UnsafeTransfer(wrappedValue: to) - return self.isAttached(to: to.wrappedValue, on: database).flatMap { alreadyAttached in + return self.isAttached(to: to, on: database).flatMap { alreadyAttached in if alreadyAttached { return database.eventLoop.makeSucceededFuture(()) } - return self.attach(to.wrappedValue, on: database, edit) + return self.attach(to, on: database, edit) } } } @@ -387,17 +385,16 @@ private struct SiblingsEagerLoader: EagerLoader let builder = To.query(on: database) .join(Through.self, on: \To._$id == to.appending(path: \.$id)) .filter(Through.self, from.appending(path: \.$id) ~~ Set(ids)) - if (self.withDeleted) { + if self.withDeleted { builder.withDeleted() } - let models = UnsafeTransfer(wrappedValue: models) return builder.all().flatMapThrowing { var map: [From.IDValue: [To]] = [:] for to in $0 { let fromID = try to.joined(Through.self)[keyPath: from].id map[fromID, default: []].append(to) } - for model in models.wrappedValue { + for model in models { guard let id = model.id else { throw FluentError.idRequired } model[keyPath: self.relationKey].value = map[id] ?? [] } diff --git a/Sources/FluentKit/Properties/Timestamp.swift b/Sources/FluentKit/Properties/Timestamp.swift index 040b01d0..780353fd 100644 --- a/Sources/FluentKit/Properties/Timestamp.swift +++ b/Sources/FluentKit/Properties/Timestamp.swift @@ -27,7 +27,7 @@ public final class TimestampProperty let format: Format public var projectedValue: TimestampProperty { - return self + self } public var wrappedValue: Date? { @@ -190,7 +190,7 @@ extension Fields { } func touchTimestamps(_ triggers: TimestampTrigger...) { - return self.touchTimestamps(triggers) + self.touchTimestamps(triggers) } private func touchTimestamps(_ triggers: [TimestampTrigger]) { diff --git a/Sources/FluentKit/Query/Builder/QueryBuilder.swift b/Sources/FluentKit/Query/Builder/QueryBuilder.swift index 251ccb9b..eaa89ec2 100644 --- a/Sources/FluentKit/Query/Builder/QueryBuilder.swift +++ b/Sources/FluentKit/Query/Builder/QueryBuilder.swift @@ -162,18 +162,18 @@ public final class QueryBuilder } } #else - nonisolated(unsafe) var partial: [Result, any Error>] = [] + nonisolated(unsafe) var partial: [Result] = [] partial.reserveCapacity(max) return self.all { row in - partial.append(row.map { .init(wrappedValue: $0) }) + partial.append(row) if partial.count >= max { - closure(partial.map { $0.map { $0.wrappedValue } }) + closure(partial) partial.removeAll(keepingCapacity: true) } }.flatMapThrowing { if !partial.isEmpty { - closure(partial.map { $0.map { $0.wrappedValue } }) + closure(partial) } } #endif diff --git a/Sources/FluentKit/Schema/DatabaseSchema.swift b/Sources/FluentKit/Schema/DatabaseSchema.swift index 3a6ad271..4d27baea 100644 --- a/Sources/FluentKit/Schema/DatabaseSchema.swift +++ b/Sources/FluentKit/Schema/DatabaseSchema.swift @@ -25,7 +25,6 @@ public struct DatabaseSchema: Sendable { case uint32 case uint64 - case bool public struct Enum: Sendable { diff --git a/Sources/FluentKit/Schema/SchemaBuilder.swift b/Sources/FluentKit/Schema/SchemaBuilder.swift index 3ed6eb0a..07e04042 100644 --- a/Sources/FluentKit/Schema/SchemaBuilder.swift +++ b/Sources/FluentKit/Schema/SchemaBuilder.swift @@ -1,6 +1,6 @@ extension Database { public func schema(_ schema: String, space: String? = nil) -> SchemaBuilder { - return .init(database: self, schema: schema, space: space) + .init(database: self, schema: schema, space: space) } } diff --git a/Sources/FluentKit/Utilities/OptionalType.swift b/Sources/FluentKit/Utilities/OptionalType.swift index 207ddc26..a301e1c2 100644 --- a/Sources/FluentKit/Utilities/OptionalType.swift +++ b/Sources/FluentKit/Utilities/OptionalType.swift @@ -11,7 +11,7 @@ public protocol OptionalType: AnyOptionalType { extension OptionalType { public static var wrappedType: Any.Type { - return Wrapped.self + Wrapped.self } } diff --git a/Sources/FluentKit/Utilities/UnsafeMutableTransferBox.swift b/Sources/FluentKit/Utilities/UnsafeMutableTransferBox.swift index 4c48c3ef..40352602 100644 --- a/Sources/FluentKit/Utilities/UnsafeMutableTransferBox.swift +++ b/Sources/FluentKit/Utilities/UnsafeMutableTransferBox.swift @@ -1,7 +1,3 @@ -struct UnsafeTransfer: @unchecked Sendable { - var wrappedValue: Wrapped -} - @usableFromInline final class UnsafeMutableTransferBox: @unchecked Sendable { @usableFromInline From e0cf83c69f8401991b5e4d6c50278dd98f57bc6a Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:38:53 -0500 Subject: [PATCH 02/11] Make `.with(\.$foo, withDeleted: true)` work correctly when used with @CompositeChildren --- Sources/FluentKit/Properties/CompositeChildren.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Sources/FluentKit/Properties/CompositeChildren.swift b/Sources/FluentKit/Properties/CompositeChildren.swift index 793d7a1b..d9dd329c 100644 --- a/Sources/FluentKit/Properties/CompositeChildren.swift +++ b/Sources/FluentKit/Properties/CompositeChildren.swift @@ -199,6 +199,9 @@ private struct CompositeChildrenEagerLoader: EagerLoader builder.group(.or) { query in _ = parentKey.queryFilterIds(ids, in: query) } + if self.withDeleted { + builder.withDeleted() + } return builder.all().map { let indexedResults = Dictionary(grouping: $0, by: { parentKey.referencedId(in: $0)! }) From 1a8fa664ac92ab4f22188778c1a0679de1acac57 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:39:50 -0500 Subject: [PATCH 03/11] Support the full set of SQL utilities when a custom aggregate query is specified --- Sources/FluentSQL/SQLQueryConverter.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/FluentSQL/SQLQueryConverter.swift b/Sources/FluentSQL/SQLQueryConverter.swift index 6b1d25fa..006f2f8b 100644 --- a/Sources/FluentSQL/SQLQueryConverter.swift +++ b/Sources/FluentSQL/SQLQueryConverter.swift @@ -232,7 +232,7 @@ public struct SQLQueryConverter { private func aggregate(_ aggregate: DatabaseQuery.Aggregate, isUnique: Bool) -> any SQLExpression { switch aggregate { case .custom(let any): - return any as! any SQLExpression + return custom(any) case .field(let field, let method): let name: String From bb09eb2840c4635db117818ced85ab366f8664af Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:40:18 -0500 Subject: [PATCH 04/11] Fix DocC warnings --- Sources/FluentSQL/SQLJSONColumnPath+Deprecated.swift | 2 +- Sources/FluentSQL/SQLList+Deprecated.swift | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/FluentSQL/SQLJSONColumnPath+Deprecated.swift b/Sources/FluentSQL/SQLJSONColumnPath+Deprecated.swift index dd8a5dfc..65ad2f36 100644 --- a/Sources/FluentSQL/SQLJSONColumnPath+Deprecated.swift +++ b/Sources/FluentSQL/SQLJSONColumnPath+Deprecated.swift @@ -1,7 +1,7 @@ import SQLKit import FluentKit -/// A thin deprecated wrapper around ``SQLKit/SQLNestedSubpathExpression``. +/// A thin deprecated wrapper around `SQLNestedSubpathExpression`. @available(*, deprecated, message: "Replaced by `SQLNestedSubpathExpression` in SQLKit") public struct SQLJSONColumnPath: SQLExpression { private var realExpression: SQLNestedSubpathExpression diff --git a/Sources/FluentSQL/SQLList+Deprecated.swift b/Sources/FluentSQL/SQLList+Deprecated.swift index 92fab4c8..07e1048a 100644 --- a/Sources/FluentSQL/SQLList+Deprecated.swift +++ b/Sources/FluentSQL/SQLList+Deprecated.swift @@ -1,6 +1,6 @@ import SQLKit -/// This file provides a few extensions to SQLKit's ``SQLList`` which have the effect of mimicking +/// This file provides a few extensions to SQLKit's `SQLList` which have the effect of mimicking /// the public API which was previously provided by a nearly-identical type of the same name in /// this module. The slightly differing behavior of the Fluent version had a tendency to cause /// confusion when both `FluentSQL` and `SQLKit` were imported in the same context; as such, the @@ -9,7 +9,7 @@ import SQLKit /// Fluent implementation available. Whether the original or alternate serialization behavior is used /// is based on which initializer is used. The original SQLKit initializer, ``init(_:separator:)`` (or /// ``init(_:)``, taking the default value for the separator), gives the original and intended behavior -/// (see ``SQLKit/SQLList`` for further details). The convenience intializer, ``init(items:separator:)``, +/// (see `SQLList` for further details). The convenience intializer, ``init(items:separator:)``, /// enables the deprecated alternate behavior, which adds a space character before and after the separator. /// /// Examples: @@ -25,7 +25,7 @@ import SQLKit /// Alternate serialization: 1 , 2 , 3 , 4 , 5 /// /// > Warning: These extensions are not recommended, as it was never intended for this behavior to be -/// > public. Convert code using these extensions to invoke the original ``SQLKit/SQLList`` directly. +/// > public. Convert code using these extensions to invoke the original `SQLList` directly. extension SQLKit.SQLList { @available(*, deprecated, message: "Use `expressions` instead.") public var items: [any SQLExpression] { From 8c6dd7e764e88af31eb0881ff1a1f8ee975c04d5 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:44:34 -0500 Subject: [PATCH 05/11] In the FluentBenchmarks, convert some migrations to async and change "Sun" to "Sol" because I'm pointlessly pedantic that way. --- .../FluentBenchmark/FluentBenchmarker.swift | 6 +- .../FluentBenchmark/SolarSystem/Galaxy.swift | 35 ++++++------ .../FluentBenchmark/SolarSystem/Planet.swift | 2 +- .../FluentBenchmark/SolarSystem/Star.swift | 57 ++++++++++--------- .../Tests/EagerLoadTests.swift | 16 +++--- .../FluentBenchmark/Tests/GroupTests.swift | 16 +++--- Sources/FluentBenchmark/Tests/JoinTests.swift | 10 ++-- .../Tests/MiddlewareTests.swift | 10 ++-- .../FluentBenchmark/Tests/ParentTests.swift | 8 +-- .../FluentBenchmark/Tests/SchemaTests.swift | 2 +- .../Tests/TransactionTests.swift | 2 +- .../QueryBuilder+Join+DirectRelations.swift | 8 +-- .../AsyncTests/AsyncQueryBuilderTests.swift | 4 +- Tests/FluentKitTests/QueryBuilderTests.swift | 8 +-- 14 files changed, 94 insertions(+), 90 deletions(-) diff --git a/Sources/FluentBenchmark/FluentBenchmarker.swift b/Sources/FluentBenchmark/FluentBenchmarker.swift index 4282acf9..b1536a24 100644 --- a/Sources/FluentBenchmark/FluentBenchmarker.swift +++ b/Sources/FluentBenchmark/FluentBenchmarker.swift @@ -53,7 +53,7 @@ public final class FluentBenchmarker { // MARK: Utilities - internal func runTest( + func runTest( _ name: String, _ migrations: [any Migration], _ test: () throws -> () @@ -61,7 +61,7 @@ public final class FluentBenchmarker { try self.runTest(name, migrations, { _ in try test() }) } - internal func runTest( + func runTest( _ name: String, _ migrations: [any Migration], _ test: (any Database) throws -> () @@ -74,7 +74,7 @@ public final class FluentBenchmarker { try self.runTest(name, migrations, on: self.database, test) } - internal func runTest( + func runTest( _ name: String, _ migrations: [any Migration], on database: any Database, diff --git a/Sources/FluentBenchmark/SolarSystem/Galaxy.swift b/Sources/FluentBenchmark/SolarSystem/Galaxy.swift index 1b937ed9..3bba210a 100644 --- a/Sources/FluentBenchmark/SolarSystem/Galaxy.swift +++ b/Sources/FluentBenchmark/SolarSystem/Galaxy.swift @@ -6,7 +6,7 @@ import XCTest public final class Galaxy: Model, @unchecked Sendable { public static let schema = "galaxies" - @ID(key: .id) + @ID public var id: UUID? @Field(key: "name") @@ -18,7 +18,7 @@ public final class Galaxy: Model, @unchecked Sendable { @Siblings(through: GalacticJurisdiction.self, from: \.$id.$galaxy, to: \.$id.$jurisdiction) public var jurisdictions: [Jurisdiction] - public init() { } + public init() {} public init(id: UUID? = nil, name: String) { self.id = id @@ -26,37 +26,36 @@ public final class Galaxy: Model, @unchecked Sendable { } } -public struct GalaxyMigration: Migration { +public struct GalaxyMigration: AsyncMigration { public init() {} - public func prepare(on database: any Database) -> EventLoopFuture { - database.schema("galaxies") - .field("id", .uuid, .identifier(auto: false)) + public func prepare(on database: any Database) async throws { + try await database.schema("galaxies") + .id() .field("name", .string, .required) .create() } - public func revert(on database: any Database) -> EventLoopFuture { - database.schema("galaxies").delete() + public func revert(on database: any Database) async throws { + try await database.schema("galaxies").delete() } } -public struct GalaxySeed: Migration { - public init() { } +public struct GalaxySeed: AsyncMigration { + public init() {} - public func prepare(on database: any Database) -> EventLoopFuture { - .andAllSucceed([ + public func prepare(on database: any Database) async throws { + try await [ "Andromeda", "Milky Way", "Pinwheel Galaxy", "Messier 82" - ].map { - Galaxy(name: $0) - .create(on: database) - }, on: database.eventLoop) + ] + .map { Galaxy(name: $0) } + .create(on: database) } - public func revert(on database: any Database) -> EventLoopFuture { - Galaxy.query(on: database).delete() + public func revert(on database: any Database) async throws { + try await Galaxy.query(on: database).delete() } } diff --git a/Sources/FluentBenchmark/SolarSystem/Planet.swift b/Sources/FluentBenchmark/SolarSystem/Planet.swift index 01c97467..f9474d7e 100644 --- a/Sources/FluentBenchmark/SolarSystem/Planet.swift +++ b/Sources/FluentBenchmark/SolarSystem/Planet.swift @@ -68,7 +68,7 @@ public struct PlanetSeed: Migration { .andAllSucceed(stars.map { star in let planets: [Planet] switch star.name { - case "Sun": + case "Sol": planets = [ .init(name: "Mercury"), .init(name: "Venus"), diff --git a/Sources/FluentBenchmark/SolarSystem/Star.swift b/Sources/FluentBenchmark/SolarSystem/Star.swift index 9ef087e6..c687fc4c 100644 --- a/Sources/FluentBenchmark/SolarSystem/Star.swift +++ b/Sources/FluentBenchmark/SolarSystem/Star.swift @@ -6,7 +6,7 @@ import XCTest public final class Star: Model, @unchecked Sendable { public static let schema = "stars" - @ID(key: .id) + @ID public var id: UUID? @Field(key: "name") @@ -23,48 +23,53 @@ public final class Star: Model, @unchecked Sendable { public init() { } - public init(id: IDValue? = nil, name: String) { + public init(id: IDValue? = nil, name: String, galaxyId: Galaxy.IDValue? = nil) { self.id = id self.name = name + if let galaxyId { + self.$galaxy.id = galaxyId + } } } -public struct StarMigration: Migration { - public func prepare(on database: any Database) -> EventLoopFuture { - database.schema("stars") - .field("id", .uuid, .identifier(auto: false)) +public struct StarMigration: AsyncMigration { + public func prepare(on database: any Database) async throws { + try await database.schema("stars") + .id() .field("name", .string, .required) .field("galaxy_id", .uuid, .required, .references("galaxies", "id")) .field("deleted_at", .datetime) .create() } - public func revert(on database: any Database) -> EventLoopFuture { - database.schema("stars").delete() + public func revert(on database: any Database) async throws { + try await database.schema("stars").delete() } } -public final class StarSeed: Migration { - public init() { } +public final class StarSeed: AsyncMigration { + public init() {} - public func prepare(on database: any Database) -> EventLoopFuture { - Galaxy.query(on: database).all().flatMap { galaxies in - .andAllSucceed(galaxies.map { galaxy in - let stars: [Star] - switch galaxy.name { - case "Milky Way": - stars = [.init(name: "Sun"), .init(name: "Alpha Centauri")] - case "Andromeda": - stars = [.init(name: "Alpheratz")] - default: - stars = [] - } - return galaxy.$stars.create(stars, on: database) - }, on: database.eventLoop) + public func prepare(on database: any Database) async throws { + var stars: [Star] = [] + + for galaxy in try await Galaxy.query(on: database).all() { + switch galaxy.name { + case "Milky Way": + stars.append(contentsOf: [ + .init(name: "Sol", galaxyId: galaxy.id!), + .init(name: "Alpha Centauri", galaxyId: galaxy.id!) + ]) + case "Andromeda": + stars.append(.init(name: "Alpheratz", galaxyId: galaxy.id!)) + default: + break + } } + try await stars.create(on: database) } - public func revert(on database: any Database) -> EventLoopFuture { - Star.query(on: database).delete(force: true) + public func revert(on database: any Database) async throws { + try await Star.query(on: database).delete(force: true) } } diff --git a/Sources/FluentBenchmark/Tests/EagerLoadTests.swift b/Sources/FluentBenchmark/Tests/EagerLoadTests.swift index 318cda72..b245313c 100644 --- a/Sources/FluentBenchmark/Tests/EagerLoadTests.swift +++ b/Sources/FluentBenchmark/Tests/EagerLoadTests.swift @@ -48,7 +48,7 @@ extension FluentBenchmarker { switch galaxy.name { case "Milky Way": XCTAssertEqual( - galaxy.stars.contains { $0.name == "Sun" }, + galaxy.stars.contains { $0.name == "Sol" }, true ) XCTAssertEqual( @@ -68,7 +68,7 @@ extension FluentBenchmarker { try Planet.query(on: self.database).filter(\.$name == "Jupiter").delete().wait() let sun1 = try XCTUnwrap(Star.query(on: self.database) - .filter(\.$name == "Sun") + .filter(\.$name == "Sol") .with(\.$planets, withDeleted: true) .first().wait() ) @@ -76,7 +76,7 @@ extension FluentBenchmarker { XCTAssertTrue(sun1.planets.contains { $0.name == "Jupiter" }) let sun2 = try XCTUnwrap(Star.query(on: self.database) - .filter(\.$name == "Sun") + .filter(\.$name == "Sol") .with(\.$planets) .first().wait() ) @@ -96,7 +96,7 @@ extension FluentBenchmarker { for planet in planets { switch planet.name { case "Earth": - XCTAssertEqual(planet.star.name, "Sun") + XCTAssertEqual(planet.star.name, "Sol") case "Proxima Centauri b": XCTAssertEqual(planet.star.name, "Alpha Centauri") default: break @@ -109,14 +109,14 @@ extension FluentBenchmarker { try self.runTest(#function, [ SolarSystem() ]) { - try Star.query(on: self.database).filter(\.$name == "Sun").delete().wait() + try Star.query(on: self.database).filter(\.$name == "Sol").delete().wait() let planet = try XCTUnwrap(Planet.query(on: self.database) .filter(\.$name == "Earth") .with(\.$star, withDeleted: true) .first().wait() ) - XCTAssertEqual(planet.star.name, "Sun") + XCTAssertEqual(planet.star.name, "Sol") XCTAssertThrowsError( try Planet.query(on: self.database) @@ -145,13 +145,13 @@ extension FluentBenchmarker { for planet in planets { switch planet.name { case "Earth": - XCTAssertEqual(planet.star.name, "Sun") + XCTAssertEqual(planet.star.name, "Sol") XCTAssertEqual(planet.tags.map { $0.name }.sorted(), ["Inhabited", "Small Rocky"]) case "Proxima Centauri b": XCTAssertEqual(planet.star.name, "Alpha Centauri") XCTAssertEqual(planet.tags.map { $0.name }, ["Small Rocky"]) case "Jupiter": - XCTAssertEqual(planet.star.name, "Sun") + XCTAssertEqual(planet.star.name, "Sol") XCTAssertEqual(planet.tags.map { $0.name }, ["Gas Giant"]) default: break } diff --git a/Sources/FluentBenchmark/Tests/GroupTests.swift b/Sources/FluentBenchmark/Tests/GroupTests.swift index e29a2418..678c512d 100644 --- a/Sources/FluentBenchmark/Tests/GroupTests.swift +++ b/Sources/FluentBenchmark/Tests/GroupTests.swift @@ -26,7 +26,7 @@ extension FluentBenchmarker { XCTAssertEqual(moon.name, "Moon") XCTAssertEqual(moon.planet.name, "Earth") XCTAssertEqual(moon.planet.type, .smallRocky) - XCTAssertEqual(moon.planet.star.name, "Sun") + XCTAssertEqual(moon.planet.star.name, "Sol") XCTAssertEqual(moon.planet.star.galaxy.name, "Milky Way") // Test JSON @@ -36,7 +36,7 @@ extension FluentBenchmarker { XCTAssertEqual(decoded.name, "Moon") XCTAssertEqual(decoded.planet.name, "Earth") XCTAssertEqual(decoded.planet.type, .smallRocky) - XCTAssertEqual(decoded.planet.star.name, "Sun") + XCTAssertEqual(decoded.planet.star.name, "Sol") XCTAssertEqual(decoded.planet.star.galaxy.name, "Milky Way") // Test deeper filter @@ -66,7 +66,7 @@ extension FluentBenchmarker { // XCTAssertEqual(moon.name, "Moon") // XCTAssertEqual(moon.planet.name, "Earth") // XCTAssertEqual(moon.planet.type, .smallRocky) -// XCTAssertEqual(moon.planet.star.name, "Sun") +// XCTAssertEqual(moon.planet.star.name, "Sol") // XCTAssertEqual(moon.planet.star.galaxy.name, "Milky Way") // // // Test JSON @@ -75,7 +75,7 @@ extension FluentBenchmarker { // XCTAssertEqual(decoded.name, "Moon") // XCTAssertEqual(decoded.planet.name, "Earth") // XCTAssertEqual(decoded.planet.type, .smallRocky) -// XCTAssertEqual(decoded.planet.star.name, "Sun") +// XCTAssertEqual(decoded.planet.star.name, "Sol") // XCTAssertEqual(decoded.planet.star.galaxy.name, "Milky Way") // // // Test deeper filter @@ -189,7 +189,7 @@ private struct FlatMoonSeed: Migration { name: "Earth", type: .smallRocky, star: .init( - name: "Sun", + name: "Sol", galaxy: .init(name: "Milky Way") ) ) @@ -200,7 +200,7 @@ private struct FlatMoonSeed: Migration { name: "Jupiter", type: .gasGiant, star: .init( - name: "Sun", + name: "Sol", galaxy: .init(name: "Milky Way") ) ) @@ -313,7 +313,7 @@ private struct FlatMoonSeed: Migration { // name: "Earth", // type: .smallRocky, // star: .init( -// name: "Sun", +// name: "Sol", // galaxy: .init(name: "Milky Way") // ) // ) @@ -324,7 +324,7 @@ private struct FlatMoonSeed: Migration { // name: "Jupiter", // type: .gasGiant, // star: .init( -// name: "Sun", +// name: "Sol", // galaxy: .init(name: "Milky Way") // ) // ) diff --git a/Sources/FluentBenchmark/Tests/JoinTests.swift b/Sources/FluentBenchmark/Tests/JoinTests.swift index ead55465..d2b6fcc5 100644 --- a/Sources/FluentBenchmark/Tests/JoinTests.swift +++ b/Sources/FluentBenchmark/Tests/JoinTests.swift @@ -27,7 +27,7 @@ extension FluentBenchmarker { let star = try planet.joined(Star.self) switch planet.name { case "Earth": - XCTAssertEqual(star.name, "Sun") + XCTAssertEqual(star.name, "Sol") case "Proxima Centauri b": XCTAssertEqual(star.name, "Alpha Centauri") default: break @@ -42,7 +42,7 @@ extension FluentBenchmarker { for galaxy in galaxies { let star = try galaxy.joined(Star.self) switch star.name { - case "Sun", "Alpha Centauri": + case "Sol", "Alpha Centauri": XCTAssertEqual(galaxy.name, "Milky Way") case "Alpheratz": XCTAssertEqual(galaxy.name, "Andromeda") @@ -193,7 +193,7 @@ extension FluentBenchmarker { let planets = try Planet.query(on: self.database) .field(\.$name) .join(Star.self, on: \Planet.$star.$id == \Star.$id) - .filter(Star.self, \.$name ~~ ["Sun", "Alpha Centauri"]) + .filter(Star.self, \.$name ~~ ["Sol", "Alpha Centauri"]) .field(Star.self, \.$name) .all().wait() @@ -203,7 +203,7 @@ extension FluentBenchmarker { XCTAssertNil(star.$id.value) switch planet.name { case "Earth": - XCTAssertEqual(star.name, "Sun") + XCTAssertEqual(star.name, "Sol") case "Proxima Centauri b": XCTAssertEqual(star.name, "Alpha Centauri") default: break @@ -226,7 +226,7 @@ extension FluentBenchmarker { XCTAssertFalse(planets.isEmpty) let morePlanets = try Planet.query(on: self.database) - .join(Star.self, on: \Planet.$star.$id == \Star.$id && \Star.$name != "Sun") + .join(Star.self, on: \Planet.$star.$id == \Star.$id && \Star.$name != "Sol") .all().wait() XCTAssertEqual(morePlanets.count, 1) diff --git a/Sources/FluentBenchmark/Tests/MiddlewareTests.swift b/Sources/FluentBenchmark/Tests/MiddlewareTests.swift index aace231d..f2efae66 100644 --- a/Sources/FluentBenchmark/Tests/MiddlewareTests.swift +++ b/Sources/FluentBenchmark/Tests/MiddlewareTests.swift @@ -216,7 +216,7 @@ private struct UserMiddleware: ModelMiddleware { model.name = "B" return next.create(model, on: db).flatMap { - return db.eventLoop.makeFailedFuture(TestError(string: "didCreate")) + db.eventLoop.makeFailedFuture(TestError(string: "didCreate")) } } @@ -224,7 +224,7 @@ private struct UserMiddleware: ModelMiddleware { model.name = "D" return next.update(model, on: db).flatMap { - return db.eventLoop.makeFailedFuture(TestError(string: "didUpdate")) + db.eventLoop.makeFailedFuture(TestError(string: "didUpdate")) } } @@ -232,7 +232,7 @@ private struct UserMiddleware: ModelMiddleware { model.name = "E" return next.softDelete(model, on: db).flatMap { - return db.eventLoop.makeFailedFuture(TestError(string: "didSoftDelete")) + db.eventLoop.makeFailedFuture(TestError(string: "didSoftDelete")) } } @@ -240,7 +240,7 @@ private struct UserMiddleware: ModelMiddleware { model.name = "F" return next.restore(model , on: db).flatMap { - return db.eventLoop.makeFailedFuture(TestError(string: "didRestore")) + db.eventLoop.makeFailedFuture(TestError(string: "didRestore")) } } @@ -248,7 +248,7 @@ private struct UserMiddleware: ModelMiddleware { model.name = "G" return next.delete(model, force: force, on: db).flatMap { - return db.eventLoop.makeFailedFuture(TestError(string: "didDelete")) + db.eventLoop.makeFailedFuture(TestError(string: "didDelete")) } } } diff --git a/Sources/FluentBenchmark/Tests/ParentTests.swift b/Sources/FluentBenchmark/Tests/ParentTests.swift index 8f2d88e8..2f50477c 100644 --- a/Sources/FluentBenchmark/Tests/ParentTests.swift +++ b/Sources/FluentBenchmark/Tests/ParentTests.swift @@ -36,7 +36,7 @@ extension FluentBenchmarker { let star = try planet.$star.get(on: self.database).wait() switch planet.name { case "Earth", "Jupiter": - XCTAssertEqual(star.name, "Sun") + XCTAssertEqual(star.name, "Sol") case "Proxima Centauri b": XCTAssertEqual(star.name, "Alpha Centauri") default: break @@ -61,7 +61,7 @@ extension FluentBenchmarker { XCTAssertNil(earth.$star.value) try earth.$star.load(on: self.database).wait() XCTAssertNotNil(earth.$star.value) - XCTAssertEqual(earth.star.name, "Sun") + XCTAssertEqual(earth.star.name, "Sol") let test = Star(name: "Foo") earth.$star.value = test @@ -69,14 +69,14 @@ extension FluentBenchmarker { // test get uses cached value try XCTAssertEqual(earth.$star.get(on: self.database).wait().name, "Foo") // test get can reload relation - try XCTAssertEqual(earth.$star.get(reload: true, on: self.database).wait().name, "Sun") + try XCTAssertEqual(earth.$star.get(reload: true, on: self.database).wait().name, "Sol") // test clearing loaded relation earth.$star.value = nil XCTAssertNil(earth.$star.value) // test get loads relation if nil - try XCTAssertEqual(earth.$star.get(on: self.database).wait().name, "Sun") + try XCTAssertEqual(earth.$star.get(on: self.database).wait().name, "Sol") } } } diff --git a/Sources/FluentBenchmark/Tests/SchemaTests.swift b/Sources/FluentBenchmark/Tests/SchemaTests.swift index 09062e62..d5bddd25 100644 --- a/Sources/FluentBenchmark/Tests/SchemaTests.swift +++ b/Sources/FluentBenchmark/Tests/SchemaTests.swift @@ -77,7 +77,7 @@ extension FluentBenchmarker { ]) { XCTAssertThrowsError( try Star.query(on: self.database) - .filter(\.$name == "Sun") + .filter(\.$name == "Sol") .delete(force: true).wait() ) } diff --git a/Sources/FluentBenchmark/Tests/TransactionTests.swift b/Sources/FluentBenchmark/Tests/TransactionTests.swift index 7663dfa9..0e7a26ec 100644 --- a/Sources/FluentBenchmark/Tests/TransactionTests.swift +++ b/Sources/FluentBenchmark/Tests/TransactionTests.swift @@ -14,7 +14,7 @@ extension FluentBenchmarker { ]) { let result = self.database.transaction { transaction in Star.query(on: transaction) - .filter(\.$name == "Sun") + .filter(\.$name == "Sol") .first() .flatMap { sun -> EventLoopFuture in diff --git a/Sources/FluentKit/Query/Builder/QueryBuilder+Join+DirectRelations.swift b/Sources/FluentKit/Query/Builder/QueryBuilder+Join+DirectRelations.swift index 0c74aec8..16dedada 100644 --- a/Sources/FluentKit/Query/Builder/QueryBuilder+Join+DirectRelations.swift +++ b/Sources/FluentKit/Query/Builder/QueryBuilder+Join+DirectRelations.swift @@ -7,7 +7,7 @@ extension QueryBuilder { /// /// Planet.query(on: db) /// .join(from: Planet.self, parent: \.$star) - /// .filter(Star.self, \Star.$name == "Sun") + /// .filter(Star.self, \Star.$name == "Sol") /// /// - Parameters: /// - model: The `Model` to join from @@ -29,7 +29,7 @@ extension QueryBuilder { /// /// Planet.query(on: db) /// .join(parent: \.$star) - /// .filter(Star.self, \Star.$name == "Sun") + /// .filter(Star.self, \Star.$name == "Sol") /// /// - Parameters: /// - parent: The `ParentProperty` to join @@ -49,7 +49,7 @@ extension QueryBuilder { /// /// Planet.query(on: db) /// .join(from: Planet.self, parent: \.$star) - /// .filter(Star.self, \Star.$name == "Sun") + /// .filter(Star.self, \Star.$name == "Sol") /// /// - Parameters: /// - model: The `Model` to join from @@ -71,7 +71,7 @@ extension QueryBuilder { /// /// Planet.query(on: db) /// .join(parent: \.$star) - /// .filter(Star.self, \Star.$name == "Sun") + /// .filter(Star.self, \Star.$name == "Sol") /// /// - Parameters: /// - parent: The `OptionalParentProperty` to join diff --git a/Tests/FluentKitTests/AsyncTests/AsyncQueryBuilderTests.swift b/Tests/FluentKitTests/AsyncTests/AsyncQueryBuilderTests.swift index f5b74c60..d9da3716 100644 --- a/Tests/FluentKitTests/AsyncTests/AsyncQueryBuilderTests.swift +++ b/Tests/FluentKitTests/AsyncTests/AsyncQueryBuilderTests.swift @@ -263,7 +263,7 @@ final class AsyncQueryBuilderTests: XCTestCase { let planets = try await Planet.query(on: test.db) .join(Star.self, on: \Star.$id == \Planet.$star.$id) .filter(\.$name, .custom("ilike"), "earth") - .filter(Star.self, \.$name, .custom("ilike"), "sun") + .filter(Star.self, \.$name, .custom("ilike"), "Sol") .all() XCTAssertEqual(planets.count, 0) XCTAssertNotNil(query.wrappedValue?.filters[1]) @@ -288,7 +288,7 @@ final class AsyncQueryBuilderTests: XCTestCase { } switch value { case .bind(let any as String): - XCTAssertEqual(any, "sun") + XCTAssertEqual(any, "Sol") default: XCTFail("\(value)") } diff --git a/Tests/FluentKitTests/QueryBuilderTests.swift b/Tests/FluentKitTests/QueryBuilderTests.swift index 16e395c5..437cd9a7 100644 --- a/Tests/FluentKitTests/QueryBuilderTests.swift +++ b/Tests/FluentKitTests/QueryBuilderTests.swift @@ -155,7 +155,7 @@ final class QueryBuilderTests: XCTestCase { let planets = try Planet.query(on: test.db) .join(Star.self, on: \Star.$id == \Planet.$star.$id) .filter(\.$name, .custom("ilike"), "earth") - .filter(Star.self, \.$name, .custom("ilike"), "sun") + .filter(Star.self, \.$name, .custom("ilike"), "Sol") .all().wait() XCTAssertEqual(planets.count, 0) XCTAssertNotNil(query.wrappedValue?.filters[1]) @@ -180,7 +180,7 @@ final class QueryBuilderTests: XCTestCase { } switch value { case .bind(let any as String): - XCTAssertEqual(any, "sun") + XCTAssertEqual(any, "Sol") default: XCTFail("\(value)") } @@ -193,10 +193,10 @@ final class QueryBuilderTests: XCTestCase { let db = DummyDatabaseForTestSQLSerializer() _ = try Planet.query(on: db) .join(Planet.self, Star.self, - on: .custom(#"LEFT JOIN "stars" ON "stars"."id" = "planets"."id" AND "stars"."name" = 'Sun'"#)) + on: .custom(#"LEFT JOIN "stars" ON "stars"."id" = "planets"."id" AND "stars"."name" = 'Sol'"#)) .all().wait() XCTAssertEqual(db.sqlSerializers.count, 1) - XCTAssertEqual(db.sqlSerializers.first?.sql, #"SELECT "planets"."id" AS "planets_id", "planets"."name" AS "planets_name", "planets"."star_id" AS "planets_star_id", "planets"."possible_star_id" AS "planets_possible_star_id", "planets"."deleted_at" AS "planets_deleted_at", "stars"."id" AS "stars_id", "stars"."name" AS "stars_name", "stars"."galaxy_id" AS "stars_galaxy_id", "stars"."deleted_at" AS "stars_deleted_at" FROM "planets" LEFT JOIN "stars" ON "stars"."id" = "planets"."id" AND "stars"."name" = 'Sun' WHERE ("planets"."deleted_at" IS NULL OR "planets"."deleted_at" > $1) AND ("stars"."deleted_at" IS NULL OR "stars"."deleted_at" > $2)"#) + XCTAssertEqual(db.sqlSerializers.first?.sql, #"SELECT "planets"."id" AS "planets_id", "planets"."name" AS "planets_name", "planets"."star_id" AS "planets_star_id", "planets"."possible_star_id" AS "planets_possible_star_id", "planets"."deleted_at" AS "planets_deleted_at", "stars"."id" AS "stars_id", "stars"."name" AS "stars_name", "stars"."galaxy_id" AS "stars_galaxy_id", "stars"."deleted_at" AS "stars_deleted_at" FROM "planets" LEFT JOIN "stars" ON "stars"."id" = "planets"."id" AND "stars"."name" = 'Sol' WHERE ("planets"."deleted_at" IS NULL OR "planets"."deleted_at" > $1) AND ("stars"."deleted_at" IS NULL OR "stars"."deleted_at" > $2)"#) } func testComplexJoinOperators() throws { From 5d58b757dd5b1b5157c0389a91d98647cef0fb91 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:45:15 -0500 Subject: [PATCH 06/11] Make various FluentBenchmarks tests not crash when simple things go wrong --- .../FluentBenchmark/Tests/PaginationTests.swift | 14 +++++++------- Sources/FluentBenchmark/Tests/RangeTests.swift | 2 +- Sources/FluentBenchmark/Tests/SiblingsTests.swift | 15 +++++++++------ .../FluentBenchmark/Tests/TransactionTests.swift | 7 ++++--- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/Sources/FluentBenchmark/Tests/PaginationTests.swift b/Sources/FluentBenchmark/Tests/PaginationTests.swift index fda61098..3f7c08e6 100644 --- a/Sources/FluentBenchmark/Tests/PaginationTests.swift +++ b/Sources/FluentBenchmark/Tests/PaginationTests.swift @@ -17,8 +17,8 @@ extension FluentBenchmarker { XCTAssertEqual(planetsPage1.metadata.total, 9) XCTAssertEqual(planetsPage1.metadata.pageCount, 5) XCTAssertEqual(planetsPage1.items.count, 2) - XCTAssertEqual(planetsPage1.items[0].name, "Earth") - XCTAssertEqual(planetsPage1.items[1].name, "Jupiter") + XCTAssertEqual(planetsPage1.items.dropFirst(0).first?.name, "Earth") + XCTAssertEqual(planetsPage1.items.dropFirst(1).first?.name, "Jupiter") } do { let planetsPage2 = try Planet.query(on: self.database) @@ -31,8 +31,8 @@ extension FluentBenchmarker { XCTAssertEqual(planetsPage2.metadata.total, 9) XCTAssertEqual(planetsPage2.metadata.pageCount, 5) XCTAssertEqual(planetsPage2.items.count, 2) - XCTAssertEqual(planetsPage2.items[0].name, "Mars") - XCTAssertEqual(planetsPage2.items[1].name, "Mercury") + XCTAssertEqual(planetsPage2.items.dropFirst(0).first?.name, "Mars") + XCTAssertEqual(planetsPage2.items.dropFirst(1).first?.name, "Mercury") } do { let galaxiesPage = try Galaxy.query(on: self.database) @@ -45,9 +45,9 @@ extension FluentBenchmarker { XCTAssertEqual(galaxiesPage.metadata.page, 1) XCTAssertEqual(galaxiesPage.metadata.per, 1) - let milkyWay = galaxiesPage.items[0] - XCTAssertEqual(milkyWay.name, "Milky Way") - XCTAssertEqual(milkyWay.stars.count, 2) + let milkyWay = galaxiesPage.items.first + XCTAssertEqual(milkyWay?.name, "Milky Way") + XCTAssertEqual(milkyWay?.stars.count, 2) } } } diff --git a/Sources/FluentBenchmark/Tests/RangeTests.swift b/Sources/FluentBenchmark/Tests/RangeTests.swift index dbc53df7..809a51e4 100644 --- a/Sources/FluentBenchmark/Tests/RangeTests.swift +++ b/Sources/FluentBenchmark/Tests/RangeTests.swift @@ -16,7 +16,7 @@ extension FluentBenchmarker { .sort(\.$name) .all().wait() XCTAssertEqual(planets.count, 3) - XCTAssertEqual(planets[0].name, "Mars") + XCTAssertEqual(planets.first?.name, "Mars") } do { let planets = try Planet.query(on: self.database) diff --git a/Sources/FluentBenchmark/Tests/SiblingsTests.swift b/Sources/FluentBenchmark/Tests/SiblingsTests.swift index 97bf3ef1..ea41f3cb 100644 --- a/Sources/FluentBenchmark/Tests/SiblingsTests.swift +++ b/Sources/FluentBenchmark/Tests/SiblingsTests.swift @@ -13,15 +13,18 @@ extension FluentBenchmarker { try self.runTest(#function, [ SolarSystem() ]) { - let inhabited = try Tag.query(on: self.database) + let inhabited = try XCTUnwrap(try Tag.query(on: self.database) .filter(\.$name == "Inhabited") - .first().wait()! - let smallRocky = try Tag.query(on: self.database) + .first().wait() + ) + let smallRocky = try XCTUnwrap(try Tag.query(on: self.database) .filter(\.$name == "Small Rocky") - .first().wait()! - let earth = try Planet.query(on: self.database) + .first().wait() + ) + let earth = try XCTUnwrap(try Planet.query(on: self.database) .filter(\.$name == "Earth") - .first().wait()! + .first().wait() + ) // check tag has expected planet do { diff --git a/Sources/FluentBenchmark/Tests/TransactionTests.swift b/Sources/FluentBenchmark/Tests/TransactionTests.swift index 0e7a26ec..0fe21d14 100644 --- a/Sources/FluentBenchmark/Tests/TransactionTests.swift +++ b/Sources/FluentBenchmark/Tests/TransactionTests.swift @@ -16,10 +16,11 @@ extension FluentBenchmarker { Star.query(on: transaction) .filter(\.$name == "Sol") .first() - .flatMap - { sun -> EventLoopFuture in + .flatMapWithEventLoop + { sun, eventLoop -> EventLoopFuture in + guard let sun else { return eventLoop.makeFailedFuture(FluentError.missingField(name: "Sol")) } let pluto = Planet(name: "Pluto") - return sun!.$planets.create(pluto, on: transaction).map { + return sun.$planets.create(pluto, on: transaction).map { pluto } }.flatMap { pluto -> EventLoopFuture<(Planet, Tag)> in From c284c2ea751ab4f610e105470afdb959d7040ead Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 03:45:56 -0500 Subject: [PATCH 07/11] Make testParent_serialization() actually test something related to a parent property. --- .../FluentBenchmark/Tests/ParentTests.swift | 44 +++++-------------- 1 file changed, 10 insertions(+), 34 deletions(-) diff --git a/Sources/FluentBenchmark/Tests/ParentTests.swift b/Sources/FluentBenchmark/Tests/ParentTests.swift index 2f50477c..73d48b4c 100644 --- a/Sources/FluentBenchmark/Tests/ParentTests.swift +++ b/Sources/FluentBenchmark/Tests/ParentTests.swift @@ -14,14 +14,14 @@ extension FluentBenchmarker { try self.runTest(#function, [ SolarSystem() ]) { - let galaxies = try Galaxy.query(on: self.database) - .all().wait() + let stars = try Star.query(on: self.database).all().wait() - let encoded = try JSONEncoder().encode(galaxies) - self.database.logger.debug("\(String(decoding: encoded, as: UTF8.self)))") - let decoded = try JSONDecoder().decode([GalaxyJSON].self, from: encoded) - XCTAssertEqual(galaxies.map { $0.id }, decoded.map { $0.id }) - XCTAssertEqual(galaxies.map { $0.name }, decoded.map { $0.name }) + let encoded = try JSONEncoder().encode(stars) + self.database.logger.trace("\(String(decoding: encoded, as: UTF8.self)))") + let decoded = try JSONDecoder().decode([StarJSON].self, from: encoded) + XCTAssertEqual(stars.map { $0.id }, decoded.map { $0.id }) + XCTAssertEqual(stars.map { $0.name }, decoded.map { $0.name }) + XCTAssertEqual(stars.map { $0.$galaxy.id }, decoded.map { $0.galaxy.id }) } } @@ -81,33 +81,9 @@ extension FluentBenchmarker { } } -private struct GalaxyKey: CodingKey, ExpressibleByStringLiteral { - var stringValue: String - var intValue: Int? { - return Int(self.stringValue) - } - - init(stringLiteral value: String) { - self.stringValue = value - } - - init?(stringValue: String) { - self.stringValue = stringValue - } - - init?(intValue: Int) { - self.stringValue = intValue.description - } -} - -private struct GalaxyJSON: Codable { +private struct StarJSON: Codable { var id: UUID var name: String - - init(from decoder: any Decoder) throws { - let keyed = try decoder.container(keyedBy: GalaxyKey.self) - self.id = try keyed.decode(UUID.self, forKey: "id") - self.name = try keyed.decode(String.self, forKey: "name") - XCTAssertEqual(keyed.allKeys.count, 2) - } + struct GalaxyJSON: Codable { var id: UUID } + var galaxy: GalaxyJSON } From ba5bcab38a093ac1bfb32880a8bdfa322f23fbb5 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 14:07:48 -0500 Subject: [PATCH 08/11] Use structured logging when we log things --- Sources/FluentKit/Migration/Migrator.swift | 6 ++--- .../Query/Builder/QueryBuilder.swift | 4 ++- .../Query/Database/DatabaseQuery+Filter.swift | 11 +------- .../Query/Database/DatabaseQuery+Value.swift | 8 +++--- .../Query/Database/DatabaseQuery.swift | 26 +++++++++++++++++-- 5 files changed, 35 insertions(+), 20 deletions(-) diff --git a/Sources/FluentKit/Migration/Migrator.swift b/Sources/FluentKit/Migration/Migrator.swift index 11091ab9..ebbe9e9e 100644 --- a/Sources/FluentKit/Migration/Migrator.swift +++ b/Sources/FluentKit/Migration/Migrator.swift @@ -141,7 +141,7 @@ private final class DatabaseMigrator: Sendable { self.database.logger.critical("The migration at \(migration.name) is in a private context. Either explicitly give it a name by adding the `var name: String` property or make the migration `internal` or `public` instead of `private`.") fatalError("Private migrations not allowed") } - self.database.logger.error("The migration at \(migration.name) has an unexpected default name. Consider giving it an explicit name by adding a `var name: String` property before applying these migrations.") + self.database.logger.error("The migration has an unexpected default name. Consider giving it an explicit name by adding a `var name: String` property before applying these migrations.", metadata: ["migration": .string(migration.name)]) } } @@ -197,7 +197,7 @@ private final class DatabaseMigrator: Sendable { return MigrationLog(name: migration.name, batch: batch).save(on: self.database) }.flatMapErrorThrowing { - self.database.logger.error("[Migrator] Failed prepare: \(String(reflecting: $0))", metadata: ["migration": .string(migration.name)]) + self.database.logger.error("[Migrator] Failed prepare", metadata: ["migration": .string(migration.name), "error": .string(String(reflecting: $0))]) throw $0 } @@ -211,7 +211,7 @@ private final class DatabaseMigrator: Sendable { return MigrationLog.query(on: self.database).filter(\.$name == migration.name).delete() }.flatMapErrorThrowing { - self.database.logger.error("[Migrator] Failed revert: \(String(reflecting: $0))", metadata: ["migration": .string(migration.name)]) + self.database.logger.error("[Migrator] Failed revert", metadata: ["migration": .string(migration.name), "error": .string(String(reflecting: $0))]) throw $0 } diff --git a/Sources/FluentKit/Query/Builder/QueryBuilder.swift b/Sources/FluentKit/Query/Builder/QueryBuilder.swift index eaa89ec2..1791d728 100644 --- a/Sources/FluentKit/Query/Builder/QueryBuilder.swift +++ b/Sources/FluentKit/Query/Builder/QueryBuilder.swift @@ -330,7 +330,9 @@ public final class QueryBuilder break } - self.database.logger.debug("\(self.query)") + // N.B.: We use `self.query` here instead of `query` so that the logging reflects the query the user requested, + // without having to deal with the noise of us having added default fields, or doing deletedAt checks, etc. + self.database.logger.debug("Running query", metadata: self.query.describedByLoggingMetadata) self.database.history?.add(self.query) let loop = self.database.eventLoop diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift index a447a0e5..4e164408 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift @@ -65,13 +65,10 @@ extension DatabaseQuery.Filter: CustomStringConvertible { switch self { case .value(let field, let method, let value): return "\(field) \(method) \(value)" - case .field(let fieldA, let method, let fieldB): return "\(fieldA) \(method) \(fieldB)" - case .group(let filters, let relation): - return filters.map{ "(\($0.description))" }.joined(separator: " \(relation) ") - + return filters.map { "(\($0))" }.joined(separator: " \(relation) ") case .custom(let any): return "custom(\(any))" } @@ -83,20 +80,16 @@ extension DatabaseQuery.Filter.Method: CustomStringConvertible { switch self { case .equality(let inverse): return inverse ? "!=" : "=" - case .order(let inverse, let equality): if equality { return inverse ? "<=" : ">=" } else { return inverse ? "<" : ">" } - case .subset(let inverse): return inverse ? "!~~" : "~~" - case .contains(let inverse, let contains): return inverse ? "!\(contains)" : "\(contains)" - case .custom(let any): return "custom(\(any))" } @@ -108,10 +101,8 @@ extension DatabaseQuery.Filter.Method.Contains: CustomStringConvertible { switch self { case .prefix: return "startswith" - case .suffix: return "endswith" - case .anywhere: return "contains" } diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift index 5917f906..6dbdfbb3 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift @@ -15,14 +15,14 @@ extension DatabaseQuery.Value: CustomStringConvertible { switch self { case .bind(let encodable): if let convertible = encodable as? any CustomDebugStringConvertible { - return convertible.debugDescription + return String(reflecting: convertible) } else { - return "\(encodable)" + return String(describing: encodable) } case .dictionary(let dictionary): - return dictionary.description + return String(describing: dictionary) case .array(let array): - return array.description + return String(describing: array) case .enumCase(let string): return string case .null: diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery.swift b/Sources/FluentKit/Query/Database/DatabaseQuery.swift index ecc24de0..510c875d 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery.swift @@ -34,9 +34,10 @@ extension DatabaseQuery: CustomStringConvertible { "\(self.action)", ] if let space = self.space { - parts.append(space) + parts.append("\(space).\(self.schema)") + } else { + parts.append(self.schema) } - parts.append(self.schema) if self.isUnique { parts.append("unique") } @@ -57,4 +58,25 @@ extension DatabaseQuery: CustomStringConvertible { } return parts.joined(separator: " ") } + + var describedByLoggingMetadata: Logger.Metadata { + func valueMetadata(_ input: DatabaseQuery.Value) -> Logger.MetadataValue { + switch input { + case .dictionary(let d): return .dictionary(.init(uniqueKeysWithValues: d.map { ($0.description, valueMetadata($1)) })) + case .array(let a): return .array(a.map { valueMetadata($0) }) + default: return .stringConvertible(input) + } + } + + return [ + "action": "\(self.action)", + "schema": "\(self.space.map { "\($0)." } ?? "")\(self.schema)", + "unique": "\(self.isUnique)", + "fields": .array(self.fields.map { .stringConvertible($0) }), + "filters": .array(self.filters.map { .stringConvertible($0) }), + "input": self.input.count == 1 ? valueMetadata(self.input.first!) : .array(self.input.map { valueMetadata($0) }), + "limits": .array(self.limits.map { .stringConvertible($0) }), + "offsets": .array(self.offsets.map { .stringConvertible($0) }), + ] + } } From d2a4f38e3fa2b59b7062291f9a113c317ac1964b Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 14:49:19 -0500 Subject: [PATCH 09/11] Fix Sendable warnings, and in the process also the behavior of ISO8601TimestampFormat when both fractional and non-fractional formats are used in the same project. --- .../Tests/PerformanceTests.swift | 4 ++++ .../Properties/TimestampFormat.swift | 22 +++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/Sources/FluentBenchmark/Tests/PerformanceTests.swift b/Sources/FluentBenchmark/Tests/PerformanceTests.swift index d05a09d5..d0b818a3 100644 --- a/Sources/FluentBenchmark/Tests/PerformanceTests.swift +++ b/Sources/FluentBenchmark/Tests/PerformanceTests.swift @@ -1,5 +1,9 @@ import FluentKit +#if !canImport(Darwin) +@preconcurrency import Foundation +#else import Foundation +#endif import NIOCore import XCTest diff --git a/Sources/FluentKit/Properties/TimestampFormat.swift b/Sources/FluentKit/Properties/TimestampFormat.swift index a32043bd..deb642ff 100644 --- a/Sources/FluentKit/Properties/TimestampFormat.swift +++ b/Sources/FluentKit/Properties/TimestampFormat.swift @@ -53,18 +53,26 @@ extension TimestampFormatFactory { withMilliseconds: Bool ) -> TimestampFormatFactory { .init { - ISO8601DateFormatter.shared.withLockedValue { - if withMilliseconds { - $0.formatOptions.insert(.withFractionalSeconds) - } - return ISO8601TimestampFormat(formatter: $0) - } + ISO8601TimestampFormat(formatter: (withMilliseconds ? + ISO8601DateFormatter.sharedWithMs : + ISO8601DateFormatter.sharedWithoutMs + ).value) } } } extension ISO8601DateFormatter { - fileprivate static let shared: NIOLockedValueBox = .init(.init()) + // We use this to suppress warnings about ISO8601DateFormatter not being Sendable. It's safe to do so because we + // know that in reality, the formatter is safe to use simultaneously from multiple threads as long as the options + // are not changed, and we never change the options after the formatter is first created. + fileprivate struct FakeSendable: @unchecked Sendable { let value: T } + + fileprivate static let sharedWithoutMs: FakeSendable = .init(value: .init()) + fileprivate static let sharedWithMs: FakeSendable = { + let formatter = ISO8601DateFormatter() + formatter.formatOptions.insert(.withFractionalSeconds) + return .init(value: formatter) + }() } public struct ISO8601TimestampFormat: TimestampFormat, @unchecked Sendable { From ec655d7710862eec17e0c9d8c4c7410bf3f17fb8 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 29 May 2024 15:04:33 -0500 Subject: [PATCH 10/11] Another Sendable warning --- Sources/FluentKit/Query/Builder/QueryBuilder.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/FluentKit/Query/Builder/QueryBuilder.swift b/Sources/FluentKit/Query/Builder/QueryBuilder.swift index 1791d728..c1b8d9ca 100644 --- a/Sources/FluentKit/Query/Builder/QueryBuilder.swift +++ b/Sources/FluentKit/Query/Builder/QueryBuilder.swift @@ -368,5 +368,5 @@ public final class QueryBuilder } #if swift(<6) || !$InferSendableFromCaptures -extension Swift.KeyPath: @unchecked Sendable {} +extension Swift.KeyPath: @unchecked Swift.Sendable {} #endif From 6554e352effe4c50aa81410f19b66f11e0b58170 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Thu, 30 May 2024 09:40:35 -0500 Subject: [PATCH 11/11] Further flesh out the structured logging with better recursion and not omitting some of the query's information. --- .../Query/Database/DatabaseQuery+Field.swift | 11 +++++ .../Query/Database/DatabaseQuery+Filter.swift | 13 ++++++ .../Query/Database/DatabaseQuery+Join.swift | 24 +++++++++- .../Query/Database/DatabaseQuery+Sort.swift | 9 ++++ .../Query/Database/DatabaseQuery+Value.swift | 9 ++++ .../Query/Database/DatabaseQuery.swift | 46 +++++++++++-------- 6 files changed, 91 insertions(+), 21 deletions(-) diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Field.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Field.swift index 41de04bc..f5244459 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Field.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Field.swift @@ -17,4 +17,15 @@ extension DatabaseQuery.Field: CustomStringConvertible { return "custom(\(custom))" } } + + var describedByLoggingMetadata: Logger.MetadataValue { + switch self { + case .path(let array, let schema): + return "\(schema).\(array.map(\.description).joined(separator: "->"))" + case .extendedPath(let array, let schema, let space): + return "\(space.map { "\($0)." } ?? "")\(schema).\(array.map(\.description).joined(separator: "->"))" + case .custom: + return .stringConvertible(self) + } + } } diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift index 4e164408..3703df2d 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Filter.swift @@ -73,6 +73,19 @@ extension DatabaseQuery.Filter: CustomStringConvertible { return "custom(\(any))" } } + + var describedByLoggingMetadata: Logger.MetadataValue { + switch self { + case .value(let field, let method, let value): + return ["field": field.describedByLoggingMetadata, "method": "\(method)", "value": value.describedByLoggingMetadata] + case .field(let field, let method, let field2): + return ["field1": field.describedByLoggingMetadata, "method": "\(method)", "field2": field2.describedByLoggingMetadata] + case .group(let array, let relation): + return ["group": .array(array.map(\.describedByLoggingMetadata)), "relation": "\(relation)"] + case .custom: + return .stringConvertible(self) + } + } } extension DatabaseQuery.Filter.Method: CustomStringConvertible { diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Join.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Join.swift index e6080626..9cae3b08 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Join.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Join.swift @@ -51,7 +51,29 @@ extension DatabaseQuery.Join: CustomStringConvertible { return "custom(\(custom))" } } - + + var describedByLoggingMetadata: Logger.MetadataValue { + switch self { + case .join(let schema, let alias, let method, let foreign, let local): + return .dictionary([ + "schema": "\(schema)", "alias": alias.map { "\($0)" }, "method": "\(method)", + "foreign": foreign.describedByLoggingMetadata, "local": local.describedByLoggingMetadata + ].compactMapValues { $0 }) + case .extendedJoin(let schema, let space, let alias, let method, let foreign, let local): + return .dictionary([ + "schema": "\(schema)", "space": space.map { "\($0)" }, "alias": alias.map { "\($0)" }, "method": "\(method)", + "foreign": foreign.describedByLoggingMetadata, "local": local.describedByLoggingMetadata + ].compactMapValues { $0 }) + case .advancedJoin(let schema, let space, let alias, let method, let filters): + return .dictionary([ + "schema": "\(schema)", "space": space.map { "\($0)" }, "alias": alias.map { "\($0)" }, "method": "\(method)", + "filters": .array(filters.map(\.describedByLoggingMetadata)) + ].compactMapValues { $0 }) + case .custom: + return .stringConvertible(self) + } + } + private func schemaDescription(space: String? = nil, schema: String, alias: String?) -> String { [space, schema].compactMap({ $0 }).joined(separator: ".") + (alias.map { " as \($0)" } ?? "") } diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Sort.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Sort.swift index 467fbeb9..318fa085 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Sort.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Sort.swift @@ -19,6 +19,15 @@ extension DatabaseQuery.Sort: CustomStringConvertible { return "custom(\(custom))" } } + + var describedByLoggingMetadata: Logger.MetadataValue { + switch self { + case .sort(let field, let direction): + return ["field": field.describedByLoggingMetadata, "direction": "\(direction)"] + case .custom: + return .stringConvertible(self) + } + } } extension DatabaseQuery.Sort.Direction: CustomStringConvertible { diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift b/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift index 6dbdfbb3..d4e7cdc5 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery+Value.swift @@ -33,4 +33,13 @@ extension DatabaseQuery.Value: CustomStringConvertible { return "custom(\(custom))" } } + + var describedByLoggingMetadata: Logger.MetadataValue { + switch self { + case .dictionary(let d): return .dictionary(.init(uniqueKeysWithValues: d.map { ($0.description, $1.describedByLoggingMetadata) })) + case .array(let a): return .array(a.map(\.describedByLoggingMetadata)) + default: return .stringConvertible(self) + } + } + } diff --git a/Sources/FluentKit/Query/Database/DatabaseQuery.swift b/Sources/FluentKit/Query/Database/DatabaseQuery.swift index 510c875d..5f49eece 100644 --- a/Sources/FluentKit/Query/Database/DatabaseQuery.swift +++ b/Sources/FluentKit/Query/Database/DatabaseQuery.swift @@ -32,24 +32,26 @@ extension DatabaseQuery: CustomStringConvertible { var parts = [ "query", "\(self.action)", + "\(self.space.map { "\($0)." } ?? "")\(self.schema)", ] - if let space = self.space { - parts.append("\(space).\(self.schema)") - } else { - parts.append(self.schema) - } if self.isUnique { parts.append("unique") } if !self.fields.isEmpty { parts.append("fields=\(self.fields)") } + if !self.joins.isEmpty { + parts.append("joins=\(self.joins)") + } if !self.filters.isEmpty { parts.append("filters=\(self.filters)") } if !self.input.isEmpty { parts.append("input=\(self.input)") } + if !self.sorts.isEmpty { + parts.append("sorts=\(self.sorts)") + } if !self.limits.isEmpty { parts.append("limits=\(self.limits)") } @@ -60,23 +62,27 @@ extension DatabaseQuery: CustomStringConvertible { } var describedByLoggingMetadata: Logger.Metadata { - func valueMetadata(_ input: DatabaseQuery.Value) -> Logger.MetadataValue { - switch input { - case .dictionary(let d): return .dictionary(.init(uniqueKeysWithValues: d.map { ($0.description, valueMetadata($1)) })) - case .array(let a): return .array(a.map { valueMetadata($0) }) - default: return .stringConvertible(input) - } - } - - return [ + var result: Logger.Metadata = [ "action": "\(self.action)", "schema": "\(self.space.map { "\($0)." } ?? "")\(self.schema)", - "unique": "\(self.isUnique)", - "fields": .array(self.fields.map { .stringConvertible($0) }), - "filters": .array(self.filters.map { .stringConvertible($0) }), - "input": self.input.count == 1 ? valueMetadata(self.input.first!) : .array(self.input.map { valueMetadata($0) }), - "limits": .array(self.limits.map { .stringConvertible($0) }), - "offsets": .array(self.offsets.map { .stringConvertible($0) }), ] + switch self.action { + case .create, .update, .custom: result["input"] = .array(self.input.map(\.describedByLoggingMetadata)) + default: break + } + switch self.action { + case .read, .aggregate, .custom: + result["unique"] = "\(self.isUnique)" + result["fields"] = .array(self.fields.map(\.describedByLoggingMetadata)) + result["joins"] = .array(self.joins.map(\.describedByLoggingMetadata)) + fallthrough + case .update, .delete: + result["filters"] = .array(self.filters.map(\.describedByLoggingMetadata)) + result["sorts"] = .array(self.sorts.map(\.describedByLoggingMetadata)) + result["limits"] = .array(self.limits.map { .stringConvertible($0) }) + result["offsets"] = .array(self.offsets.map { .stringConvertible($0) }) + default: break + } + return result } }