Skip to content

Commit

Permalink
Fix setting nil on @OptionalField (vapor#267)
Browse files Browse the repository at this point in the history
* optional field null

* fix faulty tests
  • Loading branch information
tanner0101 authored May 6, 2020
1 parent 36d8ba6 commit c73570c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 11 deletions.
49 changes: 49 additions & 0 deletions Sources/FluentBenchmark/Tests/ModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ extension FluentBenchmarker {
try self.testModel_uuid()
try self.testModel_decode()
try self.testModel_nullField()
try self.testModel_nullField_2()
try self.testModel_idGeneration()
try self.testModel_jsonColumn()
try self.testModel_hasChanges()
Expand Down Expand Up @@ -68,6 +69,37 @@ extension FluentBenchmarker {
}
}

private func testModel_nullField_2() throws {
try runTest(#function, [
FooMigration(),
]) {
let foo = Foo_2(bar: "test")
try foo.save(on: self.database).wait()
guard foo.bar != nil else {
XCTFail("unexpected nil value")
return
}
foo.bar = nil
try foo.save(on: self.database).wait()
guard foo.bar == nil else {
XCTFail("unexpected non-nil value")
return
}

guard let fetched = try Foo_2.query(on: self.database)
.filter(\.$id == foo.id!)
.first().wait()
else {
XCTFail("no model returned")
return
}
guard fetched.bar == nil else {
XCTFail("unexpected non-nil value")
return
}
}
}

private func testModel_idGeneration() throws {
try runTest(#function, [
GalaxyMigration(),
Expand Down Expand Up @@ -169,6 +201,23 @@ private struct FooMigration: Migration {
}
}

private final class Foo_2: Model {
static let schema = "foos"

@ID(key: .id)
var id: UUID?

@OptionalField(key: "bar")
var bar: String?

init() { }

init(id: IDValue? = nil, bar: String?) {
self.id = id
self.bar = bar
}
}

private final class User: Model {
static let schema = "users"

Expand Down
11 changes: 1 addition & 10 deletions Sources/FluentBenchmark/Tests/TimestampTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,12 @@ extension FluentBenchmarker {
XCTAssertEqual(event.createdAt, event.updatedAt)

Thread.sleep(forTimeInterval: 0.001) // ensure update timestamp with millisecond precision increments

let eventWithSameId = Event(id: event.id, name: "D")
eventWithSameId.$id.exists = true
try eventWithSameId.update(on: self.database).wait()
XCTAssertNil(eventWithSameId.createdAt)
XCTAssertNotNil(eventWithSameId.updatedAt)
XCTAssertNotEqual(event.updatedAt, eventWithSameId.updatedAt)


let storedEvent = try Event.find(event.id, on: self.database).wait()
XCTAssertNotNil(storedEvent)
XCTAssertEqual(storedEvent?.name, eventWithSameId.name)
XCTAssertNotNil(storedEvent?.createdAt)
XCTAssertNotNil(storedEvent?.updatedAt)
XCTAssertEqual(storedEvent?.createdAt, event.createdAt)
XCTAssertEqual(storedEvent?.updatedAt, eventWithSameId.updatedAt)
}
}

Expand Down
6 changes: 6 additions & 0 deletions Sources/FluentKit/Model/Model+CRUD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,12 @@ private struct SavedInput: DatabaseOutput {
return encodable as! T
case .enumCase(let string):
return string as! T
case .null:
if let optionalType = T.self as? AnyOptionalType.Type {
return optionalType.nil as! T
} else {
fatalError("Null value for non-optional type: \(T.self)")
}
default:
fatalError("Invalid input type: \(value)")
}
Expand Down
4 changes: 3 additions & 1 deletion Sources/FluentKit/Properties/OptionalField.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ extension OptionalFieldProperty: PropertyProtocol {
return string as? Value
case .default:
fatalError("Cannot access default field for '\(Model.self).\(key)' before it is initialized or fetched")
case .null:
return nil
default:
fatalError("Unexpected input value type for '\(Model.self).\(key)': \(value)")
}
Expand All @@ -50,7 +52,7 @@ extension OptionalFieldProperty: PropertyProtocol {
}
}
set {
self.inputValue = newValue.map { .bind($0) }
self.inputValue = newValue.map { .bind($0) } ?? .null
}
}
}
Expand Down

0 comments on commit c73570c

Please sign in to comment.