groue / GRDB.swift

A toolkit for SQLite databases, with a focus on application development
MIT License
6.79k stars 702 forks source link

Error while Encoding Generic Records using singleValueContainer #1565

Closed sroebert closed 2 months ago

sroebert commented 3 months ago

What did you do?

I have some tables with columns I'm hiding from the actual public use (for syncing purposes). To still use the fields internally, I have a generic struct that wraps these types. I have done this before in several other projects for JSON parsing. The general way of implementing decoding is to use singleValueContainer, which is how I have it implemented.

struct Wrapper<Model: Codable>: Codable {
    var model: Model
    var otherValue: String

    func encode(to encoder: any Encoder) throws {
        var modelContainer = encoder.singleValueContainer()
        try modelContainer.encode(model)

        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(otherValue, forKey: . otherValue)
    }
}

What did you expect to happen?

I expected encoding to work correctly.

What happened instead?

I got an error single value encoding is not supported.

Looking into the code it contains the following comment:

        // @itaiferber on https://forums.swift.org/t/how-to-encode-objects-of-unknown-type/12253/11
        //
        // > Encoding a value into a single-value container is equivalent to
        // > encoding the value directly into the encoder, with the primary
        // > difference being the above: encoding into the encoder writes the
        // > contents of a type into the encoder, while encoding to a
        // > single-value container gives the encoder a chance to intercept the
        // > type as a whole.
        //
        // Wait for somebody hitting this fatal error so that we can write a
        // meaningful regression test.

Looking at the forum post, it seems like I am doing the right thing. It can also be solved with try model.encode(to: encoder), but this does not seem correct. As the comments seem to suggest it might be something that still has to be implemented, I'm wondering how and if I can help.

Environment

GRDB flavor(s): GRDB GRDB version: 6.27.0 Installation method: SPM Xcode version: 16.0 beta 2 Swift version: 6.0 Platform(s) running GRDB: iOS, macOS macOS version running Xcode: 14.5

Demo Project

I don't have one, but can add if needed.

groue commented 3 months ago

Hello @sroebert,

Sorry you met an area of the library that is not implemented. As you could see, this case was foreseen, but not handled because well it did not look like it would be ever needed.

Then you came 😉

Adding the missing implementation would be nice, for sure. It's been a long time since this code was written, so I don't quite remember why it did not ship in a complete form. Do you think you could have a look? Otherwise I'll investigate. I hope we won't discover any serious difficulty.

groue commented 2 months ago

@sroebert,

Actually it wasn't difficult to do: #1570

Please hold on a little bit until I ship a new version of the library.

sroebert commented 2 months ago

That was quick, amazing!

groue commented 2 months ago

It's OK. That was not "amazing", just the reaction of a maintainer who cares about the library users. We can still act as regular European people, don't you agree? No need to over-react. Don't misinterpret the nature of the help from other people. Your issue was spot-on. You provided the use case that was missing. Thank you for that.

sroebert commented 2 months ago

That’s just the way I say thank you, no over-reaction to be honest, just my regular European reaction 😅. Glad it was taken care of so promptly.

groue commented 2 months ago

Yeah, sorry, I was the one who was over the edge in my expression. 😬

I'll amend the PR a little bit, so that if the wrapped type conforms to EncodableRecord, this conformance takes precedence over Encodable.

groue commented 2 months ago

Shipped in v6.28.0! Thanks for the feature request, @sroebert.