custom Codable `init(from:)` method causing incorrect reflection

[tanner0101]

Moving vapor/fluent#524 here.

[tanner0101]

After digging into this a bit more, the issue seems unavoidable.

The problem is that false happens to be the value that the reflection decoder sets when a Bool property is active. (Bool has two cases which is the minimum to represent a "hi" and "lo" value. This is how the reflection decoder determines a hit or miss. Ignore the fact that false is a somewhat unintuitive "hi" value--they just need to be distinct).

The problem is that false is being set as a default value in the init(from:) method in question. When active is set to false, the reflection decoder gets a false positive that it has correctly identified the coding path for the property even though it hasn't. You can easily prove this by setting the default value to true instead of false. Reflection works normally.

TL;DR is that the algorithm used to power Codable reflection is not compatible with setting default values in a custom init(from:) decoder method. At least where those default values are the reflection decoder's "hi" value--which should be considered opaque.

Fortunately Codable reflection is protocol based and you can work around this issue by simply implementing the reflectProperty method manually. See https://api.vapor.codes/core/latest/Core/Protocols/Reflectable.html#/s:4Core11ReflectableP15reflectProperty6forKeyAA09ReflectedD0VSgs0F4PathCyxqd__G_tKlFZ.

Until Swift gets better reflection, this is really our best option. The alternative to using Codable-based reflection is having every model implement reflectProperty and reflectProperties (something the very early Fluent 3 alphas required) or using Strings for for query filtering. Both of those options suck worse...for lack of a better word.

Also worth noting that the init(from:) method can be done without the annoying do / catch by using contains. Although this still doesn't reflect the correct property.

init(from decoder: Decoder)throws {
    let container = try decoder.container(keyedBy: CodingKeys.self)
    self.email = try container.decode(String.self, forKey: .email)
    if container.contains(.active) {
        self.active = try container.decode(Bool.self, forKey: .active)
    } else {
        self.active = false
    }
}

[tanner0101]

Here's an example of how to implement the custom reflection methods for desired effect.

struct User: Reflectable, Codable {
    var email: String
    var active: Bool
    
    static func reflectProperty<T>(forKey keyPath: KeyPath<User, T>) throws -> ReflectedProperty? {
        let name: String
        switch keyPath {
        case \User.email: name = "email"
        case \User.active: name = "active"
        default: return nil
        }
        return .init(T.self, at: [name])
    }
    
    init(from decoder: Decoder)throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        self.email = try container.decode(String.self, forKey: .email)
        self.active = try container.decodeIfPresent(Bool.self, forKey: .active) ?? false
    }
}

try XCTAssertEqual(User.reflectProperties().description, "[email: String, active: Bool]")
try XCTAssertEqual(User.reflectProperty(forKey: \.email)?.description ?? "n/a", "email: String")
try XCTAssertEqual(User.reflectProperty(forKey: \.active)?.description ?? "n/a", "active: Bool")

[proggeramlug]

Thanks @tanner0101 !

The custom implementation of reflectProperty seems to be the most appropriate way to deal with it, though I like the contain approach as well.

I think I'll go with the contain approach for now, that seems most natural.