Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error while Encoding Generic Records using singleValueContainer #1565

Closed
sroebert opened this issue Jul 6, 2024 · 7 comments
Closed

Error while Encoding Generic Records using singleValueContainer #1565

sroebert opened this issue Jul 6, 2024 · 7 comments

Comments

@sroebert
Copy link
Contributor

sroebert commented Jul 6, 2024

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
Copy link
Owner

groue commented Jul 8, 2024

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 added a commit that referenced this issue Jul 9, 2024
groue added a commit that referenced this issue Jul 9, 2024
@groue
Copy link
Owner

groue commented Jul 9, 2024

@sroebert,

Actually it wasn't difficult to do: #1570

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

groue added a commit that referenced this issue Jul 9, 2024
@sroebert
Copy link
Contributor Author

sroebert commented Jul 9, 2024

That was quick, amazing!

@groue
Copy link
Owner

groue commented Jul 9, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

groue commented Jul 10, 2024

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
Copy link
Owner

groue commented Jul 11, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants