Skip to content

Commit

Permalink
When performing an insert query ("create" action), explicitly set def…
Browse files Browse the repository at this point in the history 
…aults for all fields to avoid column count mismatches in model arrays. Fixes #594
  • Loading branch information
@gwynne
gwynne committed Feb 16, 2024
1 parent 3ae5187 commit 73c1966
Show file tree
Hide file tree
Showing 4 changed files with 59 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
6 changes: 3 additions & 3 deletions Sources/FluentKit/Model/Model+CRUD.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,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: true))
.action(.create)
.run { promise.succeed($0) }
.cascadeFailure(to: promise)
Expand All @@ -36,7 +36,7 @@ extension Model {
}
} else {
return Self.query(on: database)
.set(self.collectInput())
.set(self.collectInput(withDefaultedValues: true))
.action(.create)
.run()
.flatMapThrowing {
Expand Down Expand Up @@ -179,7 +179,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: true) })
.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 73c1966

Please sign in to comment.