JSONRequiredError while Decoding Generic Records using singleValueContainer

[sroebert]

What did you do?

After #1565 was fixed, I put back my code to use the singleValueContainer. Unfortunately now I get an error when decoding a row in the same way.

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

    init(from decoder: any Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        otherValue = try container.decode(String.self, forKey: . otherValue)

        let singleValueContainer = try decoder.singleValueContainer()
        model = try singleValueContainer.decode(Model.self)
    }
}

For now the workaround is to simply do:

model = try Model(from: decoder)

What did you expect to happen?

I expected decoding to work correctly.

What happened instead?

Trying to decode a generic FetchableRecord using singleValueContainer results in a JSONRequiredError.

Looking at the code in GRDB, the issue happens in _RowDecoder in FetchableRecord+Decodable.swift.

Here it returns a ColumnDecoder, apparently because there is a case where someone is trying to "Decoding an array of scalars". Instead it should most likely return a specific version of SingleValueDecodingContainer that would use a _RowDecoder when trying to decode a Decodable, similar to what happens when encoding.

Environment

GRDB flavor(s): GRDB GRDB version: 6.28.0 Installation method: SPM Xcode version: 16 beta 3 Swift version: 6 Platform(s) running GRDB: All macOS version running Xcode: 14.5

Demo Project

[groue]

Hello @sroebert,

I understand. Do you think it would be possible for you to validate this wrapper model with plain JSON encoding?

I mean: is Wrapper, as it is, able to encode and decode a flat JSON object that contains all properties of Model, plus otherValue?

[sroebert]

Hey, yes it is working normally with JSON. I have used this technique on several projects before, so it was quite common for me to do this.

I can have a look at making a PR for fixing this, doesn't seem too difficult. I was just wondering if you have an example of the case that is mentioned in the comments Decoding an array of scalars?

[groue]

Hey, yes it is working normally with JSON. I have used this technique on several projects before, so it was quite common for me to do this.

👍

I can have a look at making a PR for fixing this, doesn't seem too difficult.

That would be appreciated 😊

I was just wondering if you have an example of the case that is mentioned in the comments Decoding an array of scalars?

Yes. When I throw an error instead of returning a ColumnDecoder, one test fails: AssociationPrefetchingCodableRecordTests.testIncludingAllHasManyScalar. It is the last use case in the doc of including(all:)

[groue]

Instead it should most likely return a specific version of SingleValueDecodingContainer that would use a _RowDecoder when trying to decode a Decodable, similar to what happens when encoding.

Your acute look is very welcome as well. Please don't hesitate spotting the flaws in the current implementation 🙏 I just wish we would not introduce any breaking change (decoding more values is great, but breaking user code that currently works would not be nice).

[sroebert]

Yes totally understand and agree, that's why I was asking about the existing comment on the scalars. But if there is an existing test that fails, that will make it a lot easier for me to figure out what to do. I will have a look at creating a PR.

[groue]

Thanks @sroebert. CONTRIBUTING will help you getting started.

EDIT: you can actually directly open Package.swift. This is enough for the topic we're discussing.

[groue]

Fixed by #1574, resolved in 6.29.0