Process Google Calendar-private extended property "iCalUid"

[ArnyminerZ]

Depends on https://github.com/bitfireAT/davx5-ose/issues/538 Depends on #128 Depends on #130

[ArnyminerZ]

Note: I've just added the tests, I've not checked whether it works with Google Calendar.

[rfc2822]

There's already

https://github.com/bitfireAT/ical4android/blob/93262f53c2f6e200f73ec9e0ca876e9b120841ae/lib/src/main/kotlin/at/bitfire/ical4android/AndroidEvent.kt#L143

to process extended properties. Can we just add the iCalUid property there and set the UID field accordingly?

[ArnyminerZ]

Yes, my bad, I've switched it to the populateExtended function. I've also added some utilities for testing extended properties, since they are used in some different places, and the syntax for adding them is a bit verbose.

[ArnyminerZ]

Everything should be ready here then @rfc2822. If you prefer to test the previous change before, we can wait to merge.

[github-actions[bot]]

This PR/issue depends on:

• bitfireAT/davx5-ose#538
• bitfireAT/ical4android#128
• bitfireAT/ical4android#130

[github-actions[bot]]

This PR/issue depends on:

• bitfireAT/davx5-ose#538
• bitfireAT/ical4android#128
• bitfireAT/ical4android#130

[ArnyminerZ]

I'd just remove the iCalUid property and replace it with UID_2445, so iCalUid is read-only, and we actually use UID_2445 when updating. AFAIK Google Calendar, which is the one adding this custom property, shouldn't have any problems with UID_2445, right? So I'd just get rid of it ASAP.

I think the quickest option would be, when updating the database, if uid is not null, it will be stored in UID_2445; if that's the case, remove the extended property. Otherwise do not update anything. This way we won't ever lose iCalUid, but if there's another valid uid, use that one.

[ArnyminerZ]

Well, I don't love the approach, but this is the simplest I've been able to achieve. I've split populateEvent into populateAndroidEvent because I needed direct access to AndroidEvent to test the extendedProperties variable. If you feel like it's not necessary to test that it's initialized correctly (since we are already kind of testing it in testPopulateEvent_Uid_UID_2445_and_iCalUid) we can just go back to what we had.

The main issue here is that if we just enqueue the removal of the extended property when there's an uid, it tries to remove it even when there's no iCalUid, so the query just fails. And the easiest way I have been able to find is by holding the properties available (in extendedProperties) and then checking whether it's present or not.

I'm using a MutableList<String>, but if you feel that it would be better to use a more "synchronized" method, for example with

@Volatile
var extendedProperties: List<String> = emptyList()

I think it may be useful.

[rfc2822]

I have added the delete operation for iCalUid in line 716 of update():

https://github.com/bitfireAT/ical4android/blob/350fff4c5454a32c52101289bfffc3e27734da0c/lib/src/main/kotlin/at/bitfire/ical4android/AndroidEvent.kt#L709-L720

So iCalUid is removed as soon as we update an event. (And if there was an UID, regardless from where, it's written to UID_2445.)

Regarding extendedProperties, I wouldn't introduce additional data fields that can become async etc. So if we'd like to test the extended properties, we should test the database rows directly I think. However for now I'd leave it as it is.

Is this PR now ok for you? Then I will have a last look and merge :)

[ArnyminerZ]

Looks good 👍🏼