Add Codable support to DatabaseDateComponents

[mtancock]

Add Codable conformance to DatabaseDateComponents as per discussion in #764

Embarrassingly, after *cough* years of development, this is my first open source PR so apologies for anything I've done wrong.

What if any tests would be needed for this? I had a look but couldn't figure out what to test as there doesn't appear to be anything much equivalent in the library that I could steal from.

It also didn't seem like there was anything in the Readme to update? But happy to add something if you point me to what needs updating.

Of course if this is embarrassingly bad, let's just delete it and pretend it never happened?

Pull Request Checklist

• This pull request is submitted against the development branch.
• Inline documentation has been updated.
• README.md or another dedicated guide has been updated.
• Changes are tested.

[groue]

Hello, @mtancock, I'm happy you could not resist ;-)

So now, DatabaseDateComponents is Codable, and the code below now compiles:

struct Activity: Codable, FetchableRecord, PersistableRecord {
    var id: String
    var name: String
    var date: DatabaseDateComponents
}

Funnily, This Codable conformance is not used at runtime, when this type roundtrips a database. Indeed, we won't see the two coding keys introduced by this pull request, dateComponents and format, anywhere in the database content! We just see regular YYYY-MM-DD formatted strings, depending on the format property of the DatabaseDateComponents value.

The reason is the DatabaseValueConvertible protocol. It is this protocol which allows DatabaseDateComponents to go into the database (it was the case even before this pull request was opened). And this protocol has priority over Codable, when a type conforms to both. This allows Date to encode itself in a YYYY-MM-DD HH:MM.SS.SSS string, instead of the numerical timestamp used by the Codable implementation of Date.

So can we do whatever we want in the Codable implementation of DatabaseDateComponents, since it is never used at runtime, in the context of Codable records?

Well, not really. Now we have a public Codable conformance, so people may use it for other purposes. The Activity type above may end up in JSON, for example. In the proposed implementation, we get:

// Proposed
{
  "id": "12345",
  "name": "Lockdown",
  "date": {
    "format": "yyyy-MM-dd",
    "dateComponents": {
      "hour" : 10,
      "minute" : 0,
      "year" : 2020,
      "month" : 4,
      "day" : 19
    }
  }
}

(Yeah, see also how nothing prevents extra hour and minute to sneak in).

This is not wrong, because it does the job. But it won't ship as it is, because I would not want to use it in my projects.

I suggest an alternative:

// Alternative
{
  "id": "12345",
  "name": "Lockdown",
  "date": "2020-04-19"
}

The conversion of DatabaseDateComponents to and from a String is provided by DatabaseValueConvertible, as below:

let encoded = DatabaseDateComponents(
    DateComponents(year: 2020, month: 4, day: 19, hour: 10, minute: 00),
    format: .YMD)

let string = String.fromDatabaseValue(encoded.databaseValue)! // never nil with DatabaseDateComponents
print("string: \(string)") // "2020-04-19"

if let decoded = DatabaseDateComponents.fromDatabaseValue(string.databaseValue) {
    print("decoded: \(decoded)")
} else {
    print("decoding error!")
}

Would you be OK with amending your pull request in this direction?

[groue]

What if any tests would be needed for this? I had a look but couldn't figure out what to test as there doesn't appear to be anything much equivalent in the library that I could steal from.

Yes we need at least one test. FoundationDateComponentsTests.swift is a good recipient. There is no "test encoder" in the stdlib or Foundation, so just perform a JSON encoding and check the resulting output: this will be quite enough.

It also didn't seem like there was anything in the Readme to update? But happy to add something if you point me to what needs updating.

It's OK, there's nothing to add in the README. As we've seen together in #764, this conformance is perfectly expectable, and does not need any explicit mention.

Of course if this is embarrassingly bad, let's just delete it and pretend it never happened?

I'd much prefer that we complete it :-) You are really helping, and not only on the coding side of things!

It can be very fast.

[mtancock]

Sure - I'll look at those changes later on today. Thanks for the gentle review!

[mtancock]

I've updated. I'm not sure what I should be doing (rather than force unwrapping) in the initialiser as there's no error to re-throw.

[groue]

I hope I do not write my review too soon... I can't accept this pull request, I'm sorry. I though I had been clear on the desired handling of DatabaseDateComponents coding, but I see surprising mistakes, here. Please tell me if you need more time, or if you prefer that I handle it myself.

[groue]

I've updated. I'm not sure what I should be doing (rather than force unwrapping) in the initialiser as there's no error to re-throw.

You have to throw a DecodingError: https://developer.apple.com/documentation/swift/decodingerror

And you need a single-value-container: https://developer.apple.com/documentation/swift/encoder/2894221-singlevaluecontainer

[mtancock]

I did suggest you might want to delete it. ;-) Sorry I misread your comments. Update incoming

[groue]

This looks perfect, now 👍

[groue]

Hey, thank you very much @mtancock! Now we have a useful Codable implementation :-) I'll ship it shortly

[groue]

@mtancock, welcome to the GRDB contributors! Would you like to be granted a push access to the repository? I'll invite you right away. You don't have to be shy, because you know how things work in this repo, now. You won't be bothered unless you want to. And it's good for the library that I'm not the only one able to push here.

[mtancock]

Thanks. I feel completely unqualified but always happy to help out any way that I’m able to.

[groue]

Invitation sent :-) You know, I was just as unqualified when this started 😅 All it takes is a desire to enhance our common daily tasks, and a little taste in order to remember that other people may use our code in unexpected ways, and that this is the beauty of our job :-)

[groue]

Shipped in v4.14.0 🤝