Skip to content

Commit

Permalink
Explicitly set all columns when creating arrays of models (#595)
Browse files Browse the repository at this point in the history 
* When performing an insert query ("create" action), explicitly set defaults for all fields to avoid column count mismatches in model arrays. Fixes #594

* Because Mongo is terrible, don't use the new defaulting behavior when the database isn't SQL.
  • Loading branch information
@gwynne
gwynne authored Feb 17, 2024
1 parent 3ae5187 commit bb47433
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 29 deletions.
9 changes: 7 additions & 2 deletions Sources/FluentKit/Model/Fields.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ private final class HasChangesInput: DatabaseInput {
extension Fields {
/// Returns a dictionary of field keys and associated values representing all "pending"
/// data - e.g. all fields (if any) which have been changed by something other than Fluent.
internal func collectInput() -> [FieldKey: DatabaseQuery.Value] {
let input = DictionaryInput()
internal func collectInput(withDefaultedValues defaultedValues: Bool = false) -> [FieldKey: DatabaseQuery.Value] {
let input = DictionaryInput(wantsUnmodifiedKeys: defaultedValues)
self.input(to: input)
return input.storage
}
Expand All @@ -166,6 +166,11 @@ extension Fields {
/// Helper type for the implementation of `Fields.collectInput()`.
private final class DictionaryInput: DatabaseInput {
var storage: [FieldKey: DatabaseQuery.Value] = [:]
let wantsUnmodifiedKeys: Bool

init(wantsUnmodifiedKeys: Bool = false) {
self.wantsUnmodifiedKeys = wantsUnmodifiedKeys
}

func set(_ value: DatabaseQuery.Value, at key: FieldKey) {
self.storage[key] = value
Expand Down
7 changes: 4 additions & 3 deletions Sources/FluentKit/Model/Model+CRUD.swift
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import NIOCore
import protocol SQLKit.SQLDatabase

extension Model {
public func save(on database: Database) -> EventLoopFuture<Void> {
Expand All @@ -22,7 +23,7 @@ extension Model {
self.anyID.generate()
let promise = database.eventLoop.makePromise(of: DatabaseOutput.self)
Self.query(on: database)
.set(self.collectInput())
.set(self.collectInput(withDefaultedValues: database is SQLDatabase))
.action(.create)
.run { promise.succeed($0) }
.cascadeFailure(to: promise)
Expand All @@ -36,7 +37,7 @@ extension Model {
}
} else {
return Self.query(on: database)
.set(self.collectInput())
.set(self.collectInput(withDefaultedValues: database is SQLDatabase))
.action(.create)
.run()
.flatMapThrowing {
Expand Down Expand Up @@ -179,7 +180,7 @@ extension Collection where Element: FluentKit.Model {
}.create(model, on: database)
}, on: database.eventLoop).flatMap {
Element.query(on: database)
.set(self.map { $0.collectInput() })
.set(self.map { $0.collectInput(withDefaultedValues: database is SQLDatabase) })
.create()
}.map {
for model in self {
Expand Down
46 changes: 24 additions & 22 deletions Tests/FluentKitTests/CompositeIDTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ final class CompositeIDTests: XCTestCase {
try model.delete(force: true, on: db).wait()

XCTAssertEqual(db.sqlSerializers.count, 6)
XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4, $5)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, $3, $4, $5, DEFAULT)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(1).sql, #"UPDATE "composite+planet+tag" SET "notation" = $1, "updatedAt" = $2 WHERE ("composite+planet+tag"."planet_id" = $3 AND "composite+planet+tag"."tag_id" = $4) AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $5)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(2).sql, #"UPDATE "composite+planet+tag" SET "planet_id" = $1, "updatedAt" = $2 WHERE ("composite+planet+tag"."planet_id" = $3 AND "composite+planet+tag"."tag_id" = $4) AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $5)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(3).sql, #"UPDATE "composite+planet+tag" SET "updatedAt" = $1, "deletedAt" = $2 WHERE ("composite+planet+tag"."planet_id" = $3 AND "composite+planet+tag"."tag_id" = $4) AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $5)"#)
Expand Down Expand Up @@ -112,8 +112,8 @@ final class CompositeIDTests: XCTestCase {
XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"SELECT "planets"."id" AS "planets_id", "planets"."name" AS "planets_name", "planets"."star_id" AS "planets_star_id" FROM "planets" WHERE "planets"."id" = $1 LIMIT 1"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(1).sql, #"SELECT "composite+planet+tag"."planet_id" AS "composite+planet+tag_planet_id", "composite+planet+tag"."tag_id" AS "composite+planet+tag_tag_id", "composite+planet+tag"."notation" AS "composite+planet+tag_notation", "composite+planet+tag"."createdAt" AS "composite+planet+tag_createdAt", "composite+planet+tag"."updatedAt" AS "composite+planet+tag_updatedAt", "composite+planet+tag"."deletedAt" AS "composite+planet+tag_deletedAt" FROM "composite+planet+tag" WHERE "composite+planet+tag"."planet_id" = $1 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $2)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(2).sql, #"SELECT "tags"."id" AS "tags_id", "tags"."name" AS "tags_name", "composite+planet+tag"."planet_id" AS "composite+planet+tag_planet_id", "composite+planet+tag"."tag_id" AS "composite+planet+tag_tag_id", "composite+planet+tag"."notation" AS "composite+planet+tag_notation", "composite+planet+tag"."createdAt" AS "composite+planet+tag_createdAt", "composite+planet+tag"."updatedAt" AS "composite+planet+tag_updatedAt", "composite+planet+tag"."deletedAt" AS "composite+planet+tag_deletedAt" FROM "tags" INNER JOIN "composite+planet+tag" ON "tags"."id" = "composite+planet+tag"."tag_id" WHERE "composite+planet+tag"."planet_id" = $1 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $2)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(3).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(4).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(3).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, $3, $4, DEFAULT)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(4).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, $3, $4, DEFAULT)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(5).sql, #"SELECT COUNT("composite+planet+tag"."planet_id") AS "aggregate" FROM "composite+planet+tag" WHERE "composite+planet+tag"."planet_id" = $1 AND "composite+planet+tag"."tag_id" = $2 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $3)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(6).sql, #"SELECT COUNT("composite+planet+tag"."planet_id") AS "aggregate" FROM "composite+planet+tag" WHERE "composite+planet+tag"."planet_id" = $1 AND "composite+planet+tag"."tag_id" = $2 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $3)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(7).sql, #"UPDATE "composite+planet+tag" SET "updatedAt" = $1, "deletedAt" = $2 WHERE "composite+planet+tag"."planet_id" = $3 AND "composite+planet+tag"."tag_id" = $4 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $5)"#)
Expand Down Expand Up @@ -147,11 +147,11 @@ final class CompositeIDTests: XCTestCase {
}

XCTAssertEqual(db.sqlSerializers.count, 6)
XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(0).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, $3, $4, DEFAULT)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(1).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" FROM "planets" WHERE ("planets"."deleted_at" IS NULL OR "planets"."deleted_at" > $1)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(2).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4, $5)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(2).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, $3, $4, $5, DEFAULT)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(3).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" FROM "planets" WHERE ("planets"."deleted_at" IS NULL OR "planets"."deleted_at" > $1)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(4).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4, $5)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(4).sql, #"INSERT INTO "composite+planet+tag" ("planet_id", "tag_id", "notation", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, $3, $4, $5, DEFAULT)"#)
XCTAssertEqual(try db.sqlSerializers.xctAt(5).sql, #"SELECT COUNT("composite+planet+tag"."planet_id") AS "aggregate" FROM "composite+planet+tag" WHERE "composite+planet+tag"."planet_id" = $1 AND "composite+planet+tag"."tag_id" = $2 AND ("composite+planet+tag"."deletedAt" IS NULL OR "composite+planet+tag"."deletedAt" > $3)"#)

planet.$tags.fromId = nil
Expand Down Expand Up @@ -234,26 +234,28 @@ final class CompositeIDTests: XCTestCase {
moon4.$planetoid.id = nil
try [moon1, moon2, moon3, moon4].forEach { try $0.update(on: db).wait() }

let moonCols = #""id", "name", "planet_system_id", "planet_nrm_ord""#, fourVals = "$1, $2, $3, $4", sixVals = "\(fourVals), $5, $6"
let expectedQueries: [(String, [Encodable])] = [
(#"INSERT INTO "composite+planet" ("system_id", "nrm_ord", "name") VALUES ($1, $2, $3)"#, [sysId, 1, "A"]),
(#"INSERT INTO "composite+moon" (\#(moonCols)) VALUES (\#(fourVals))"#, [moon1.id!, "B", sysId, 1]),
(#"INSERT INTO "composite+moon" (\#(moonCols), "progenitorSystem_id", "progenitorNrm_ord") VALUES (\#(sixVals))"#, [moon2.id!, "C", sysId, 1, sysId, 2]),
(#"INSERT INTO "composite+moon" (\#(moonCols)) VALUES (\#(fourVals))"#, [moon3.id!, "D", sysId, 1]),
(#"INSERT INTO "composite+moon" (\#(moonCols), "planetoid_system_id", "planetoid_nrm_ord") VALUES (\#(sixVals))"#, [moon4.id!, "E", sysId, 1, sysId, 3]),
(#"UPDATE "composite+planet" SET "name" = $1 WHERE ("composite+planet"."system_id" = $2 AND "composite+planet"."nrm_ord" = $3)"#, ["AA", sysId, 1]),
(#"UPDATE "composite+moon" SET "planet_system_id" = $1, "planet_nrm_ord" = $2 WHERE "composite+moon"."id" = $3"#, [sys2Id, 2, moon1.id!]),
(#"UPDATE "composite+moon" SET "progenitorSystem_id" = NULL, "progenitorNrm_ord" = NULL WHERE "composite+moon"."id" = $1"#, [moon2.id!]),
(#"UPDATE "composite+moon" SET "planetoid_system_id" = $1, "planetoid_nrm_ord" = $2 WHERE "composite+moon"."id" = $3"#, [sys2Id, 3, moon3.id!]),
(#"UPDATE "composite+moon" SET "planetoid_system_id" = NULL, "planetoid_nrm_ord" = NULL WHERE "composite+moon"."id" = $1"#, [moon4.id!]),
let moonCols = #""id", "name", "planet_system_id", "planet_nrm_ord", "progenitorSystem_id", "progenitorNrm_ord", "planetoid_system_id", "planetoid_nrm_ord""#
let fourVals = "$1, $2, $3, $4, NULL, NULL, NULL, NULL"
let sixVals1 = "$1, $2, $3, $4, $5, $6, NULL, NULL", sixVals2 = "$1, $2, $3, $4, NULL, NULL, $5, $6"
let expectedQueries: [(String, [Encodable], UInt)] = [
(#"INSERT INTO "composite+planet" ("system_id", "nrm_ord", "name") VALUES ($1, $2, $3)"#, [sysId, 1, "A"], #line),
(#"INSERT INTO "composite+moon" (\#(moonCols)) VALUES (\#(fourVals))"#, [moon1.id!, "B", sysId, 1], #line),
(#"INSERT INTO "composite+moon" (\#(moonCols)) VALUES (\#(sixVals1))"#, [moon2.id!, "C", sysId, 1, sysId, 2], #line),
(#"INSERT INTO "composite+moon" (\#(moonCols)) VALUES (\#(fourVals))"#, [moon3.id!, "D", sysId, 1], #line),
(#"INSERT INTO "composite+moon" (\#(moonCols)) VALUES (\#(sixVals2))"#, [moon4.id!, "E", sysId, 1, sysId, 3], #line),
(#"UPDATE "composite+planet" SET "name" = $1 WHERE ("composite+planet"."system_id" = $2 AND "composite+planet"."nrm_ord" = $3)"#, ["AA", sysId, 1], #line),
(#"UPDATE "composite+moon" SET "planet_system_id" = $1, "planet_nrm_ord" = $2 WHERE "composite+moon"."id" = $3"#, [sys2Id, 2, moon1.id!], #line),
(#"UPDATE "composite+moon" SET "progenitorSystem_id" = NULL, "progenitorNrm_ord" = NULL WHERE "composite+moon"."id" = $1"#, [moon2.id!], #line),
(#"UPDATE "composite+moon" SET "planetoid_system_id" = $1, "planetoid_nrm_ord" = $2 WHERE "composite+moon"."id" = $3"#, [sys2Id, 3, moon3.id!], #line),
(#"UPDATE "composite+moon" SET "planetoid_system_id" = NULL, "planetoid_nrm_ord" = NULL WHERE "composite+moon"."id" = $1"#, [moon4.id!], #line),
]

XCTAssertEqual(db.sqlSerializers.count, expectedQueries.count)
for ((query, binds), serializer) in zip(expectedQueries, db.sqlSerializers) {
XCTAssertEqual(serializer.sql, query)
XCTAssertEqual(serializer.binds.count, binds.count)
for ((query, binds, line), serializer) in zip(expectedQueries, db.sqlSerializers) {
XCTAssertEqual(serializer.sql, query, file: #filePath, line: line)
XCTAssertEqual(serializer.binds.count, binds.count, file: #filePath, line: line)
for (lBind, rBind) in zip(binds, serializer.binds) {
XCTAssertEqual("\(lBind)", "\(rBind)")
XCTAssertEqual("\(lBind)", "\(rBind)", file: #filePath, line: line)
}
}
}
Expand Down
27 changes: 25 additions & 2 deletions Tests/FluentKitTests/FluentKitTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ final class FluentKitTests: XCTestCase {
XCTAssertEqual(db.sqlSerializers.count, 6)
XCTAssertEqual(db.sqlSerializers.dropFirst(0).first?.sql, #"CREATE TABLE "mirror_universe"."planets"("id" UUID PRIMARY KEY, "name" TEXT NOT NULL, "star_id" UUID REFERENCES "stars" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION NOT NULL, "possible_star_id" UUID REFERENCES "stars" ("id") ON DELETE NO ACTION ON UPDATE NO ACTION, "createdAt" TIMESTAMPTZ, "updatedAt" TIMESTAMPTZ, "deletedAt" TIMESTAMPTZ DEFAULT NULL)"#)
XCTAssertEqual(db.sqlSerializers.dropFirst(1).first?.sql, #"SELECT "mirror_universe"."planets"."id" AS "mirror_universe_planets_id", "mirror_universe"."planets"."name" AS "mirror_universe_planets_name", "mirror_universe"."planets"."star_id" AS "mirror_universe_planets_star_id", "mirror_universe"."planets"."possible_star_id" AS "mirror_universe_planets_possible_star_id", "mirror_universe"."planets"."createdAt" AS "mirror_universe_planets_createdAt", "mirror_universe"."planets"."updatedAt" AS "mirror_universe_planets_updatedAt", "mirror_universe"."planets"."deletedAt" AS "mirror_universe_planets_deletedAt" FROM "mirror_universe"."planets" WHERE "mirror_universe"."planets"."name" = $1 AND ("mirror_universe"."planets"."deletedAt" IS NULL OR "mirror_universe"."planets"."deletedAt" > $2)"#)
XCTAssertEqual(db.sqlSerializers.dropFirst(2).first?.sql, #"INSERT INTO "mirror_universe"."planets" ("id", "name", "createdAt", "updatedAt") VALUES ($1, $2, $3, $4)"#)
XCTAssertEqual(db.sqlSerializers.dropFirst(2).first?.sql, #"INSERT INTO "mirror_universe"."planets" ("id", "name", "star_id", "possible_star_id", "createdAt", "updatedAt", "deletedAt") VALUES ($1, $2, DEFAULT, DEFAULT, $3, $4, DEFAULT)"#)
XCTAssertEqual(db.sqlSerializers.dropFirst(3).first?.sql, #"UPDATE "mirror_universe"."planets" SET "id" = $1, "name" = $2, "updatedAt" = $3 WHERE "mirror_universe"."planets"."id" = $4 AND ("mirror_universe"."planets"."deletedAt" IS NULL OR "mirror_universe"."planets"."deletedAt" > $5)"#)
XCTAssertEqual(db.sqlSerializers.dropFirst(4).first?.sql, #"DELETE FROM "mirror_universe"."planets" WHERE "mirror_universe"."planets"."name" <> $1"#)
XCTAssertEqual(db.sqlSerializers.dropFirst(5).first?.sql, #"SELECT "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 "stars" INNER JOIN "mirror_universe"."planets" ON "mirror_universe"."planets"."star_id" = "stars"."id" LIMIT 1"#)
Expand Down Expand Up @@ -765,10 +765,33 @@ final class FluentKitTests: XCTestCase {
XCTAssertEqual(KeyPrefixingStrategy.custom({ .prefix($0, .prefix("+", $1)) }).apply(prefix: "abc", to: "def").description, "abc+def")
}

func testCreatingModelArraysWithUnsetOptionalProperties() throws {
final class Foo: Model {
static let schema = "foos"

@ID var id: UUID?
@OptionalField(key: "opt") var opt: String?

init() {}
init(id: UUID? = nil, opt: String? = nil) { (self.id, self.opt) = (id, opt) }
}

let foos = [
Foo(),
Foo(opt: nil),
Foo(opt: "foo"),
]
let db = DummyDatabaseForTestSQLSerializer()

try foos.create(on: db).wait()
XCTAssertEqual(db.sqlSerializers.count, 1)
XCTAssertEqual(db.sqlSerializers.first?.sql, #"INSERT INTO "foos" ("id", "opt") VALUES ($1, DEFAULT), ($2, NULL), ($3, $4)"#)
}

func testFieldsPropertiesPerformance() throws {
measure {
let model = LotsOfFields()
for _ in 1 ... 10_000 {
for _ in 1 ... 5_000 {
XCTAssertEqual(model.properties.count, 21)
}
}
Expand Down

0 comments on commit bb47433

Please sign in to comment.