Refactor SEQUENCE and UID handling on successful uploads (#1785)

* Define interfaces

* [WIP] Refactor sequence and UID handling in event uploads

- Refactor `generateUpload` method to return `GeneratedResource`.
- Update `SyncManager` to handle `GeneratedResource`.
- Implement sequence and UID management in `CalendarSyncManager`.
- Remove redundant `prepareForUpload` method from `LocalEvent`.

* Refactor sequence handling in uploads

- Move UID validation to `DavUtils.isGoodFileBaseName`
- Update sequence directly in iCalendar for group-scheduled events
- Rename `resourceBaseName` to `suggestedBaseName` for clarity

* Refactor sequence / UID handling in contact uploads

- Update `LocalAddress` interface to include `updateUid` method
- Modify `ContactsSyncManager` to handle UID generation and update
- Remove redundant UID handling in `LocalContact` and `LocalGroup`
- Adjust code style settings for right margin

* Remove redundant UID update from `ContactsSyncManager` and `CalendarSyncManager`

* Implement UID handling in `TasksSyncManager` for uploads

* Update JtxSyncManager

- Update `JtxSyncManager.kt` to implement the `generateUpload` function.
- Update `LocalJtxICalObject.kt` to include the `updateUid` method for updating UIDs in the collection.

* Remove deprecated `prepareForUpload` method from `LocalResource`

- Remove `prepareForUpload` implementations from `LocalContact`, `LocalEvent`, `LocalGroup`, and `LocalTask`

* Rename `isGoodFileBaseName` to `isGoodFileName` in `DavUtils`

* Fix tests

* Move UID generation logic to `DavUtils.generateUidIfNecessary`

- Use `DavUtils.fileNameFromUid` for generating file names
- Update `ContactsSyncManager`, `CalendarSyncManager`, `TasksSyncManager`, and `JtxSyncManager` to use new utility methods

* Add tests for DavUtils

* Some tests

* Refactor onSuccessfulUpload

* Update KDoc

* Logging

* Remove unnecessary LocalEvent method

* KDoc
This commit is contained in:
Ricki Hirner
2025-10-29 17:57:34 +01:00
committed by GitHub
parent b839cbfe7f
commit a8c8a8d2e0
19 changed files with 445 additions and 371 deletions

View File

@@ -29,7 +29,6 @@ import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import java.util.UUID
import javax.inject.Inject
@HiltAndroidTest
@@ -66,113 +65,6 @@ class LocalEventTest {
}
@Test
fun testPrepareForUpload_NoUid() {
// create event
val event = Event().apply {
dtStart = DtStart("20220120T010203Z")
summary = "Event without uid"
}
calendar.add(
event = event,
fileName = "filename.ics",
eTag = null,
scheduleTag = null,
flags = LocalResource.FLAG_REMOTELY_PRESENT
)
val localEvent = calendar.findByName("filename.ics")!!
// prepare for upload - this should generate a new random uuid, returned as filename
val fileNameWithSuffix = localEvent.prepareForUpload()
val fileName = fileNameWithSuffix.removeSuffix(".ics")
// throws an exception if fileName is not an UUID
UUID.fromString(fileName)
// UID in calendar storage should be the same as file name
client.query(
ContentUris.withAppendedId(Events.CONTENT_URI, localEvent.id!!).asSyncAdapter(account),
arrayOf(Events.UID_2445), null, null, null
)!!.use { cursor ->
cursor.moveToFirst()
assertEquals(fileName, cursor.getString(0))
}
}
@Test
fun testPrepareForUpload_NormalUid() {
// create event
val event = Event().apply {
dtStart = DtStart("20220120T010203Z")
summary = "Event with normal uid"
uid = "some-event@hostname.tld" // old UID format, UUID would be new format
}
calendar.add(
event = event,
fileName = "filename.ics",
eTag = null,
scheduleTag = null,
flags = LocalResource.FLAG_REMOTELY_PRESENT
)
val localEvent = calendar.findByName("filename.ics")!!
// prepare for upload - this should use the UID for the file name
val fileNameWithSuffix = localEvent.prepareForUpload()
val fileName = fileNameWithSuffix.removeSuffix(".ics")
assertEquals(event.uid, fileName)
// UID in calendar storage should still be set, too
client.query(
ContentUris.withAppendedId(Events.CONTENT_URI, localEvent.id!!).asSyncAdapter(account),
arrayOf(Events.UID_2445), null, null, null
)!!.use { cursor ->
cursor.moveToFirst()
assertEquals(fileName, cursor.getString(0))
}
}
@Test
fun testPrepareForUpload_UidHasDangerousChars() {
// create event
val event = Event().apply {
dtStart = DtStart("20220120T010203Z")
summary = "Event with funny uid"
uid = "https://www.example.com/events/asdfewfe-cxyb-ewrws-sadfrwerxyvser-asdfxye-"
}
calendar.add(
event = event,
fileName = "filename.ics",
eTag = null,
scheduleTag = null,
flags = LocalResource.FLAG_REMOTELY_PRESENT
)
val localEvent = calendar.findByName("filename.ics")!!
// prepare for upload - this should generate a new random uuid, returned as filename
val fileNameWithSuffix = localEvent.prepareForUpload()
val fileName = fileNameWithSuffix.removeSuffix(".ics")
// throws an exception if fileName is not an UUID
UUID.fromString(fileName)
// UID in calendar storage shouldn't have been changed
client.query(
ContentUris.withAppendedId(Events.CONTENT_URI, localEvent.id!!).asSyncAdapter(account),
arrayOf(Events.UID_2445), null, null, null
)!!.use { cursor ->
cursor.moveToFirst()
assertEquals(event.uid, cursor.getString(0))
}
}
@Test
fun testDeleteDirtyEventsWithoutInstances_NoInstances_Exdate() {
// TODO
}
@Test
fun testDeleteDirtyEventsWithoutInstances_NoInstances_CancelledExceptions() {
// create recurring event with only deleted/cancelled instances

View File

@@ -24,8 +24,6 @@ import dagger.hilt.android.testing.HiltAndroidTest
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
@@ -242,19 +240,6 @@ class LocalGroupTest {
}
}
@Test
fun testPrepareForUpload() {
localTestAddressBookProvider.provide(account, provider, GroupMethod.CATEGORIES) { ab ->
val group = newGroup(ab)
assertNull(group.getContact().uid)
val fileName = group.prepareForUpload()
val newUid = group.getContact().uid
assertNotNull(newUid)
assertEquals("$newUid.vcf", fileName)
}
}
@Test
fun testUpdate() {
localTestAddressBookProvider.provide(account, provider) { ab ->

View File

@@ -0,0 +1,103 @@
/*
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
*/
package at.bitfire.davdroid.sync
import android.accounts.Account
import android.content.Context
import at.bitfire.davdroid.sync.account.TestAccount
import at.bitfire.ical4android.Event
import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import io.mockk.every
import io.mockk.mockk
import net.fortuna.ical4j.model.property.DtStart
import okio.Buffer
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import javax.inject.Inject
@HiltAndroidTest
class CalendarSyncManagerTest {
@get:Rule
val hiltRule = HiltAndroidRule(this)
@Inject @ApplicationContext
lateinit var context: Context
@Inject
lateinit var syncManagerFactory: CalendarSyncManager.Factory
lateinit var account: Account
@Before
fun setUp() {
hiltRule.inject()
account = TestAccount.create()
}
@After
fun tearDown() {
TestAccount.remove(account)
}
@Test
fun generateUpload_existingUid() {
val result = syncManager().generateUpload(mockk(relaxed = true) {
every { getCachedEvent() } returns Event(uid = "existing-uid", dtStart = DtStart())
})
assertEquals("existing-uid.ics", result.suggestedFileName)
assertTrue(result.onSuccessContext.uid.isEmpty)
val iCal = Buffer().also {
result.requestBody.writeTo(it)
}.readString(Charsets.UTF_8)
assertTrue(iCal.contains("UID:existing-uid\r\n"))
}
@Test
fun generateUpload_noUid() {
val result = syncManager().generateUpload(mockk(relaxed = true) {
every { getCachedEvent() } returns Event(dtStart = DtStart())
})
assertTrue(result.suggestedFileName.matches(UUID_FILENAME_REGEX))
val uuid = result.suggestedFileName.removeSuffix(".ics")
assertEquals(uuid, result.onSuccessContext.uid.get())
val iCal = Buffer().also {
result.requestBody.writeTo(it)
}.readString(Charsets.UTF_8)
assertTrue(iCal.contains("UID:$uuid\r\n"))
}
// helpers
private fun syncManager() = syncManagerFactory.calendarSyncManager(
account = account,
httpClient = mockk(),
syncResult = mockk(),
localCalendar = mockk(),
collection = mockk(),
resync = mockk()
)
companion object {
val UUID_FILENAME_REGEX = "^[0-9a-fA-F]{8}\\b-[0-9a-fA-F]{4}\\b-[0-9a-fA-F]{4}\\b-[0-9a-fA-F]{4}\\b-[0-9a-fA-F]{12}\\.ics$".toRegex()
}
}

View File

@@ -19,8 +19,6 @@ class LocalTestResource: LocalResource {
var deleted = false
var dirty = false
override fun prepareForUpload() = "generated-file.txt"
override fun clearDirty(fileName: Optional<String>, eTag: String?, scheduleTag: String?) {
dirty = false
if (fileName.isPresent)
@@ -33,6 +31,9 @@ class LocalTestResource: LocalResource {
this.flags = flags
}
override fun updateUid(uid: String) { /* no-op */ }
override fun updateSequence(sequence: Int) = throw NotImplementedError()
override fun deleteLocal() = throw NotImplementedError()
override fun resetDeleted() = throw NotImplementedError()

View File

@@ -20,7 +20,6 @@ import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import kotlinx.coroutines.CoroutineDispatcher
import okhttp3.HttpUrl
import okhttp3.RequestBody
import okhttp3.RequestBody.Companion.toRequestBody
import org.junit.Assert.assertEquals
@@ -76,9 +75,13 @@ class TestSyncManager @AssistedInject constructor(
}
var didGenerateUpload = false
override fun generateUpload(resource: LocalTestResource): RequestBody {
override fun generateUpload(resource: LocalTestResource): GeneratedResource {
didGenerateUpload = true
return resource.toString().toRequestBody()
return GeneratedResource(
suggestedFileName = resource.fileName ?: "generated-file.txt",
requestBody = resource.toString().toRequestBody(),
onSuccessContext = GeneratedResource.OnSuccessContext()
)
}
override fun syncAlgorithm() = SyncAlgorithm.PROPFIND_REPORT

View File

@@ -31,7 +31,6 @@ import com.google.common.base.Ascii
import com.google.common.base.MoreObjects
import java.io.FileNotFoundException
import java.util.Optional
import java.util.UUID
import kotlin.jvm.optionals.getOrNull
class LocalContact: AndroidContact, LocalAddress {
@@ -70,25 +69,6 @@ class LocalContact: AndroidContact, LocalAddress {
}
override fun prepareForUpload(): String {
val contact = getContact()
val uid: String = contact.uid ?: run {
// generate new UID
val newUid = UUID.randomUUID().toString()
// update in contacts provider
val values = contentValuesOf(COLUMN_UID to newUid)
addressBook.provider!!.update(rawContactSyncURI(), values, null, null)
// update this event
contact.uid = newUid
newUid
}
return "$uid.vcf"
}
/**
* Clears cached [contact] so that the next read of [contact] will query the content provider again.
*/
@@ -137,6 +117,13 @@ class LocalContact: AndroidContact, LocalAddress {
this.flags = flags
}
override fun updateSequence(sequence: Int) = throw NotImplementedError()
override fun updateUid(uid: String) {
val values = contentValuesOf(COLUMN_UID to uid)
addressBook.provider!!.update(rawContactSyncURI(), values, null, null)
}
override fun deleteLocal() {
delete()
}

View File

@@ -18,7 +18,6 @@ import at.bitfire.synctools.storage.calendar.AndroidRecurringCalendar
import com.google.common.base.Ascii
import com.google.common.base.MoreObjects
import java.util.Optional
import java.util.UUID
class LocalEvent(
val recurringCalendar: AndroidRecurringCalendar,
@@ -75,80 +74,19 @@ class LocalEvent(
return event
}
/**
* Generates the [Event] that should actually be uploaded:
*
* 1. Takes the [getCachedEvent].
* 2. Calculates the new SEQUENCE.
*
* _Note: This method currently modifies the object returned by [getCachedEvent], but
* this may change in the future._
*
* @return data object that should be used for uploading
*/
fun eventToUpload(): Event {
val event = getCachedEvent()
val nonGroupScheduled = event.attendees.isEmpty()
val weAreOrganizer = event.isOrganizer == true
// Increase sequence (event.sequence null/non-null behavior is defined by the Event, see KDoc of event.sequence):
// - If it's null, the event has just been created in the database, so we can start with SEQUENCE:0 (default).
// - If it's non-null, the event already exists on the server, so increase by one.
val sequence = event.sequence
if (sequence != null && (nonGroupScheduled || weAreOrganizer))
event.sequence = sequence + 1
return event
}
/**
* Updates the SEQUENCE of the event in the content provider.
*
* @param sequence new sequence value
*/
fun updateSequence(sequence: Int?) {
override fun updateSequence(sequence: Int) {
androidEvent.update(contentValuesOf(
AndroidEvent2.COLUMN_SEQUENCE to sequence
))
}
/**
* Creates and sets a new UID in the calendar provider, if no UID is already set.
* It also returns the desired file name for the event for further processing in the sync algorithm.
*
* @return file name to use at upload
*/
override fun prepareForUpload(): String {
// make sure that UID is set
val uid: String = getCachedEvent().uid ?: run {
// generate new UID
val newUid = UUID.randomUUID().toString()
// persist to calendar provider
val values = contentValuesOf(Events.UID_2445 to newUid)
androidEvent.update(values)
// update in cached event data object
getCachedEvent().uid = newUid
newUid
}
val uidIsGoodFilename = uid.all { char ->
// see RFC 2396 2.2
char.isLetterOrDigit() || arrayOf( // allow letters and digits
';', ':', '@', '&', '=', '+', '$', ',', // allow reserved characters except '/' and '?'
'-', '_', '.', '!', '~', '*', '\'', '(', ')' // allow unreserved characters
).contains(char)
}
return if (uidIsGoodFilename)
"$uid.ics" // use UID as file name
else
"${UUID.randomUUID()}.ics" // UID would be dangerous as file name, use random UUID instead
override fun updateUid(uid: String) {
androidEvent.update(contentValuesOf(
Events.UID_2445 to uid
))
}
override fun clearDirty(fileName: Optional<String>, eTag: String?, scheduleTag: String?) {
val values = contentValuesOf(
Events.DIRTY to 0,

View File

@@ -16,7 +16,6 @@ import android.provider.ContactsContract.RawContacts
import android.provider.ContactsContract.RawContacts.Data
import androidx.core.content.contentValuesOf
import at.bitfire.davdroid.resource.LocalGroup.Companion.COLUMN_PENDING_MEMBERS
import at.bitfire.davdroid.util.trimToNull
import at.bitfire.synctools.storage.BatchOperation
import at.bitfire.synctools.storage.ContactsBatchOperation
import at.bitfire.vcard4android.AndroidAddressBook
@@ -28,7 +27,6 @@ import at.bitfire.vcard4android.Contact
import com.google.common.base.MoreObjects
import java.util.LinkedList
import java.util.Optional
import java.util.UUID
import java.util.logging.Logger
import kotlin.jvm.optionals.getOrNull
@@ -142,26 +140,6 @@ class LocalGroup: AndroidGroup, LocalAddress {
}
override fun prepareForUpload(): String {
var uid: String? = null
addressBook.provider!!.query(groupSyncUri(), arrayOf(AndroidContact.COLUMN_UID), null, null, null)?.use { cursor ->
if (cursor.moveToNext())
uid = cursor.getString(0).trimToNull()
}
if (uid == null) {
// generate new UID
uid = UUID.randomUUID().toString()
val values = contentValuesOf(AndroidContact.COLUMN_UID to uid)
addressBook.provider!!.update(groupSyncUri(), values, null, null)
_contact?.uid = uid
}
return "$uid.vcf"
}
override fun clearDirty(fileName: Optional<String>, eTag: String?, scheduleTag: String?) {
if (scheduleTag != null)
throw IllegalArgumentException("Contact groups must not have a Schedule-Tag")
@@ -229,6 +207,13 @@ class LocalGroup: AndroidGroup, LocalAddress {
this.flags = flags
}
override fun updateSequence(sequence: Int) = throw NotImplementedError()
override fun updateUid(uid: String) {
val values = contentValuesOf(AndroidContact.COLUMN_UID to uid)
addressBook.provider!!.update(groupSyncUri(), values, null, null)
}
override fun deleteLocal() {
delete()
}

View File

@@ -20,10 +20,7 @@ class LocalJtxICalObject(
eTag: String?,
scheduleTag: String?,
flags: Int
) :
JtxICalObject(collection),
LocalResource {
) : JtxICalObject(collection), LocalResource {
init {
this.fileName = fileName
@@ -62,6 +59,10 @@ class LocalJtxICalObject(
update(data)
}
override fun updateSequence(sequence: Int) = throw NotImplementedError()
override fun updateUid(uid: String) = throw NotImplementedError()
override fun clearDirty(fileName: Optional<String>, eTag: String?, scheduleTag: String?) {
clearDirty(fileName.getOrNull(), eTag, scheduleTag)
}

View File

@@ -47,18 +47,6 @@ interface LocalResource {
/** bitfield of flags; currently either [FLAG_REMOTELY_PRESENT] or 0 */
val flags: Int
/**
* Prepares the resource for uploading:
*
* 1. If the resource doesn't have an UID yet, this method generates one and writes it to the content provider.
* 2. The new file name which can be used for the upload is derived from the UID and returned, but not
* saved to the content provider. The sync manager is responsible for saving the file name that
* was actually used.
*
* @return suggestion for new file name of the resource (like "<uid>.vcf")
*/
fun prepareForUpload(): String
/**
* Unsets the _dirty_ field of the resource and updates other sync-related fields in the content provider.
* Does not affect `this` object itself (which is immutable).
@@ -78,6 +66,19 @@ interface LocalResource {
*/
fun updateFlags(flags: Int)
/**
* Updates the local UID of the resource in the content provider.
* Usually used to persist a UID that has been created during an upload of a locally created resource.
*/
fun updateUid(uid: String)
/**
* Updates the local SEQUENCE of the resource in the content provider.
*
* @throws NotImplementedError if SEQUENCE update is not supported
*/
fun updateSequence(sequence: Int)
/**
* Deletes the data object from the content provider.
*/

View File

@@ -19,7 +19,6 @@ import com.google.common.base.Ascii
import com.google.common.base.MoreObjects
import org.dmfs.tasks.contract.TaskContract.Tasks
import java.util.Optional
import java.util.UUID
class LocalTask: DmfsTask, LocalResource {
@@ -65,24 +64,6 @@ class LocalTask: DmfsTask, LocalResource {
/* custom queries */
override fun prepareForUpload(): String {
val uid: String = task!!.uid ?: run {
// generate new UID
val newUid = UUID.randomUUID().toString()
// update in tasks provider
val values = contentValuesOf(Tasks._UID to newUid)
taskList.provider.update(taskSyncURI(), values, null, null)
// update this task
task!!.uid = newUid
newUid
}
return "$uid.ics"
}
override fun clearDirty(fileName: Optional<String>, eTag: String?, scheduleTag: String?) {
if (scheduleTag != null)
logger.fine("Schedule-Tag for tasks not supported yet, won't save")
@@ -119,6 +100,13 @@ class LocalTask: DmfsTask, LocalResource {
this.flags = flags
}
override fun updateSequence(sequence: Int) = throw NotImplementedError()
override fun updateUid(uid: String) {
val values = contentValuesOf(Tasks._UID to uid)
taskList.provider.update(taskSyncURI(), values, null, null)
}
override fun deleteLocal() {
delete()
}

View File

@@ -27,6 +27,7 @@ import at.bitfire.davdroid.resource.LocalEvent
import at.bitfire.davdroid.resource.LocalResource
import at.bitfire.davdroid.resource.SyncState
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.davdroid.util.DavUtils.lastSegment
import at.bitfire.ical4android.Event
import at.bitfire.ical4android.EventReader
@@ -42,7 +43,6 @@ import net.fortuna.ical4j.model.Component
import net.fortuna.ical4j.model.component.VAlarm
import net.fortuna.ical4j.model.property.Action
import okhttp3.HttpUrl
import okhttp3.RequestBody
import okhttp3.RequestBody.Companion.toRequestBody
import java.io.Reader
import java.io.StringReader
@@ -178,24 +178,52 @@ class CalendarSyncManager @AssistedInject constructor(
return modified or superModified
}
override fun onSuccessfulUpload(local: LocalEvent, newFileName: String, eTag: String?, scheduleTag: String?) {
super.onSuccessfulUpload(local, newFileName, eTag, scheduleTag)
override fun generateUpload(resource: LocalEvent): GeneratedResource {
val event = resource.getCachedEvent()
logger.log(Level.FINE, "Preparing upload of iCalendar ${resource.fileName}", event)
// update local SEQUENCE to new value after successful upload
local.updateSequence(local.getCachedEvent().sequence)
}
// get/create UID
val (uid, uidIsGenerated) = DavUtils.generateUidIfNecessary(event.uid)
if (uidIsGenerated)
event.uid = uid
override fun generateUpload(resource: LocalEvent): RequestBody =
SyncException.wrapWithLocalResource(resource) {
val event = resource.eventToUpload()
logger.log(Level.FINE, "Preparing upload of event ${resource.fileName}", event)
// Increase sequence, if necessary:
// - If it's null, the event has just been created in the database, so we can start with SEQUENCE:0 (default).
// - If it's non-null, the event already exists on the server, so increase by one.
val groupScheduled = event.attendees.isNotEmpty()
val weAreOrganizer = event.isOrganizer == true
val sequence = event.sequence
val newSequence: Optional<Int> = when {
// first upload, set to 0 after upload
sequence == null ->
Optional.of(0)
// write iCalendar to string and convert to request body
val iCalWriter = StringWriter()
EventWriter(Constants.iCalProdId).write(event, iCalWriter)
iCalWriter.toString().toRequestBody(DavCalendar.MIME_ICALENDAR_UTF8)
// re-upload of group-scheduled event (and we're ORGANIZER), increase sequence in iCalendar and after upload
groupScheduled && weAreOrganizer -> {
event.sequence = sequence + 1
Optional.of(sequence + 1)
}
// standard re-upload, don't update sequence
else ->
Optional.empty()
}
// generate iCalendar and convert to request body
val iCalWriter = StringWriter()
EventWriter(Constants.iCalProdId).write(event, iCalWriter)
val requestBody = iCalWriter.toString().toRequestBody(DavCalendar.MIME_ICALENDAR_UTF8)
return GeneratedResource(
suggestedFileName = DavUtils.fileNameFromUid(uid, "ics"),
requestBody = requestBody,
onSuccessContext = GeneratedResource.OnSuccessContext(
uid = if (uidIsGenerated) Optional.of(uid) else Optional.empty(),
sequence = newSequence
)
)
}
override suspend fun listAllRemote(callback: MultiResponseCallback) {
// calculate time range limits
val limitStart = accountSettings.getTimeRangePastDays()?.let { pastDays ->

View File

@@ -51,7 +51,6 @@ import okhttp3.HttpUrl
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
import okhttp3.MediaType
import okhttp3.Request
import okhttp3.RequestBody
import okhttp3.RequestBody.Companion.toRequestBody
import java.io.ByteArrayOutputStream
import java.io.IOException
@@ -272,35 +271,45 @@ class ContactsSyncManager @AssistedInject constructor(
return modified or superModified
}
override fun generateUpload(resource: LocalAddress): RequestBody =
SyncException.wrapWithLocalResource(resource) {
val contact: Contact = when (resource) {
is LocalContact -> resource.getContact()
is LocalGroup -> resource.getContact()
else -> throw IllegalArgumentException("resource must be LocalContact or LocalGroup")
}
logger.log(Level.FINE, "Preparing upload of vCard ${resource.fileName}", contact)
val os = ByteArrayOutputStream()
val mimeType: MediaType
when {
hasJCard -> {
mimeType = DavAddressBook.MIME_JCARD
contact.writeJCard(os, Constants.vCardProdId)
}
hasVCard4 -> {
mimeType = DavAddressBook.MIME_VCARD4
contact.writeVCard(VCardVersion.V4_0, os, Constants.vCardProdId)
}
else -> {
mimeType = DavAddressBook.MIME_VCARD3_UTF8
contact.writeVCard(VCardVersion.V3_0, os, Constants.vCardProdId)
}
}
return@wrapWithLocalResource os.toByteArray().toRequestBody(mimeType)
override fun generateUpload(resource: LocalAddress): GeneratedResource {
val contact: Contact = when (resource) {
is LocalContact -> resource.getContact()
is LocalGroup -> resource.getContact()
else -> throw IllegalArgumentException("resource must be LocalContact or LocalGroup")
}
logger.log(Level.FINE, "Preparing upload of vCard ${resource.fileName}", contact)
// get/create UID
val (uid, uidIsGenerated) = DavUtils.generateUidIfNecessary(contact.uid)
if (uidIsGenerated)
contact.uid = uid
// generate vCard and convert to request body
val os = ByteArrayOutputStream()
val mimeType: MediaType
when {
hasJCard -> {
mimeType = DavAddressBook.MIME_JCARD
contact.writeJCard(os, Constants.vCardProdId)
}
hasVCard4 -> {
mimeType = DavAddressBook.MIME_VCARD4
contact.writeVCard(VCardVersion.V4_0, os, Constants.vCardProdId)
}
else -> {
mimeType = DavAddressBook.MIME_VCARD3_UTF8
contact.writeVCard(VCardVersion.V3_0, os, Constants.vCardProdId)
}
}
return GeneratedResource(
suggestedFileName = DavUtils.fileNameFromUid(uid, "vcf"),
requestBody = os.toByteArray().toRequestBody(mimeType),
onSuccessContext = GeneratedResource.OnSuccessContext(
uid = if (uidIsGenerated) Optional.of(uid) else Optional.empty()
)
)
}
override suspend fun listAllRemote(callback: MultiResponseCallback) =
SyncException.wrapWithRemoteResourceSuspending(collection.url) {

View File

@@ -0,0 +1,34 @@
/*
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
*/
package at.bitfire.davdroid.sync
import okhttp3.RequestBody
import java.util.Optional
/**
* Represents a resource that has been generated for the purpose of being uploaded.
*
* @param suggestedFileName file name that can be used for uploading if there's no existing name
* @param requestBody resource body (including MIME type)
* @param onSuccessContext context that must be passed to [SyncManager.onSuccessfulUpload] on successful upload
*/
class GeneratedResource(
val suggestedFileName: String,
val requestBody: RequestBody,
val onSuccessContext: OnSuccessContext
) {
/**
* Contains information that has been created for a [GeneratedResource], but has not been saved yet.
*
* @param uid new UID to persist on successful upload (empty: UID not modified)
* @param sequence new SEQUENCE to persist on successful upload (empty: SEQUENCE not modified)
*/
data class OnSuccessContext(
val uid: Optional<String> = Optional.empty(),
val sequence: Optional<Int> = Optional.empty()
)
}

View File

@@ -25,6 +25,7 @@ import at.bitfire.davdroid.resource.LocalJtxCollection
import at.bitfire.davdroid.resource.LocalJtxICalObject
import at.bitfire.davdroid.resource.LocalResource
import at.bitfire.davdroid.resource.SyncState
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.davdroid.util.DavUtils.lastSegment
import at.bitfire.ical4android.JtxICalObject
import at.bitfire.synctools.exception.InvalidICalendarException
@@ -34,7 +35,6 @@ import dagger.assisted.AssistedInject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.runInterruptible
import okhttp3.HttpUrl
import okhttp3.RequestBody
import okhttp3.RequestBody.Companion.toRequestBody
import java.io.ByteArrayOutputStream
import java.io.Reader
@@ -96,13 +96,18 @@ class JtxSyncManager @AssistedInject constructor(
syncState
}
override fun generateUpload(resource: LocalJtxICalObject): RequestBody =
SyncException.wrapWithLocalResource(resource) {
logger.log(Level.FINE, "Preparing upload of icalobject ${resource.fileName}", resource)
val os = ByteArrayOutputStream()
resource.write(os, Constants.iCalProdId)
os.toByteArray().toRequestBody(DavCalendar.MIME_ICALENDAR_UTF8)
}
override fun generateUpload(resource: LocalJtxICalObject): GeneratedResource {
logger.log(Level.FINE, "Preparing upload of icalobject ${resource.uid}")
val os = ByteArrayOutputStream()
resource.write(os, Constants.iCalProdId)
return GeneratedResource(
suggestedFileName = DavUtils.fileNameFromUid(resource.uid, "ics"),
requestBody = os.toByteArray().toRequestBody(DavCalendar.MIME_ICALENDAR_UTF8),
onSuccessContext = GeneratedResource.OnSuccessContext() // nothing special to update after upload
)
}
override fun syncAlgorithm() = SyncAlgorithm.PROPFIND_REPORT

View File

@@ -8,6 +8,7 @@ import android.accounts.Account
import android.content.Context
import android.os.DeadObjectException
import android.os.RemoteException
import androidx.annotation.VisibleForTesting
import at.bitfire.dav4jvm.DavCollection
import at.bitfire.dav4jvm.DavResource
import at.bitfire.dav4jvm.Error
@@ -46,7 +47,6 @@ import kotlinx.coroutines.launch
import kotlinx.coroutines.runInterruptible
import kotlinx.coroutines.withContext
import okhttp3.HttpUrl
import okhttp3.RequestBody
import java.io.IOException
import java.net.HttpURLConnection
import java.security.cert.CertificateException
@@ -388,15 +388,10 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
*/
protected open suspend fun uploadDirty(local: ResourceType, forceAsNew: Boolean = false) {
val existingFileName = local.fileName
val fileName = if (existingFileName != null) {
// prepare upload (for UID etc), but ignore returned file name suggestion
local.prepareForUpload()
existingFileName
} else {
// prepare upload and use returned file name suggestion as new file name
local.prepareForUpload()
}
val upload = generateUpload(local)
val fileName = existingFileName ?: upload.suggestedFileName
val uploadUrl = collection.url.newBuilder().addPathSegment(fileName).build()
val remote = DavResource(httpClient.okHttpClient, uploadUrl)
@@ -405,13 +400,12 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
if (existingFileName == null || forceAsNew) {
// create new resource on server
logger.info("Uploading new resource ${local.id} -> $fileName")
val bodyToUpload = generateUpload(local)
var newETag: String? = null
var newScheduleTag: String? = null
runInterruptible {
remote.put(
bodyToUpload,
upload.requestBody,
ifNoneMatch = true, // fails if there's already a resource with that name
callback = { response ->
newETag = GetETag.fromResponse(response)?.eTag
@@ -421,10 +415,8 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
)
}
logger.fine("Upload successful; new ETag=$newETag / Schedule-Tag=$newScheduleTag")
// success (no exception thrown)
onSuccessfulUpload(local, fileName, newETag, newScheduleTag)
onSuccessfulUpload(local, fileName, newETag, newScheduleTag, upload.onSuccessContext)
} else {
// update resource on server
@@ -432,13 +424,12 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
val ifETag = if (ifScheduleTag == null) local.eTag else null
logger.info("Uploading modified resource ${local.id} -> $fileName (if ETag=$ifETag / Schedule-Tag=$ifScheduleTag)")
val bodyToUpload = generateUpload(local)
var updatedETag: String? = null
var updatedScheduleTag: String? = null
runInterruptible {
remote.put(
bodyToUpload,
upload.requestBody,
ifETag = ifETag,
ifScheduleTag = ifScheduleTag,
callback = { response ->
@@ -449,10 +440,8 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
)
}
logger.fine("Upload successful; updated ETag=$updatedETag / Schedule-Tag=$updatedScheduleTag")
// success (no exception thrown)
onSuccessfulUpload(local, fileName, updatedETag, updatedScheduleTag)
onSuccessfulUpload(local, fileName, updatedETag, updatedScheduleTag, upload.onSuccessContext)
}
}
@@ -492,16 +481,6 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
}
}
/**
* Called after a successful upload (either of a new or an updated resource) so that the local
* _dirty_ state can be reset.
*
* Note: [CalendarSyncManager] overrides this method to additionally store the updated SEQUENCE.
*/
protected open fun onSuccessfulUpload(local: ResourceType, newFileName: String, eTag: String?, scheduleTag: String?) {
local.clearDirty(Optional.of(newFileName), eTag, scheduleTag)
}
/**
* Generates the request body (iCalendar or vCard) from a local resource.
*
@@ -509,7 +488,43 @@ abstract class SyncManager<ResourceType: LocalResource, out CollectionType: Loca
*
* @return iCalendar or vCard (content + Content-Type) that can be uploaded to the server
*/
protected abstract fun generateUpload(resource: ResourceType): RequestBody
@VisibleForTesting
internal abstract fun generateUpload(resource: ResourceType): GeneratedResource
/**
* Called after a successful upload (either of a new or an updated resource) so that the local
* _dirty_ state can be reset. Also updates some other local properties.
*
* @param local local resource that has been uploaded successfully
* @param newFileName file name that has been used for uploading
* @param eTag resulting `ETag` of the upload (from the server)
* @param scheduleTag resulting `Schedule-Tag` of the upload (from the server)
* @param context properties that have been generated before the upload and that shall be persisted by this method
*/
private fun onSuccessfulUpload(
local: ResourceType,
newFileName: String,
eTag: String?,
scheduleTag: String?,
context: GeneratedResource.OnSuccessContext
) {
logger.log(Level.FINE, "Upload successful", arrayOf(
"File name = $newFileName",
"ETag = $eTag",
"Schedule-Tag = $scheduleTag",
"context = $context"
))
// update local UID and SEQUENCE, if necessary
if (context.uid.isPresent)
local.updateUid(context.uid.get())
if (context.sequence.isPresent)
local.updateSequence(context.sequence.get())
// clear dirty flag and update ETag/Schedule-Tag
local.clearDirty(Optional.of(newFileName), eTag, scheduleTag)
}
/**

View File

@@ -24,6 +24,7 @@ import at.bitfire.davdroid.resource.LocalResource
import at.bitfire.davdroid.resource.LocalTask
import at.bitfire.davdroid.resource.LocalTaskList
import at.bitfire.davdroid.resource.SyncState
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.davdroid.util.DavUtils.lastSegment
import at.bitfire.ical4android.Task
import at.bitfire.synctools.exception.InvalidICalendarException
@@ -33,11 +34,11 @@ import dagger.assisted.AssistedInject
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.runInterruptible
import okhttp3.HttpUrl
import okhttp3.RequestBody
import okhttp3.RequestBody.Companion.toRequestBody
import java.io.ByteArrayOutputStream
import java.io.Reader
import java.io.StringReader
import java.util.Optional
import java.util.logging.Level
/**
@@ -101,16 +102,27 @@ class TasksSyncManager @AssistedInject constructor(
override fun syncAlgorithm() = SyncAlgorithm.PROPFIND_REPORT
override fun generateUpload(resource: LocalTask): RequestBody =
SyncException.wrapWithLocalResource(resource) {
val task = requireNotNull(resource.task)
logger.log(Level.FINE, "Preparing upload of task ${resource.fileName}", task)
override fun generateUpload(resource: LocalTask): GeneratedResource {
val task = requireNotNull(resource.task)
logger.log(Level.FINE, "Preparing upload of task ${resource.fileName}", task)
val os = ByteArrayOutputStream()
task.write(os, Constants.iCalProdId)
// get/create UID
val (uid, uidIsGenerated) = DavUtils.generateUidIfNecessary(task.uid)
if (uidIsGenerated)
task.uid = uid
os.toByteArray().toRequestBody(DavCalendar.MIME_ICALENDAR_UTF8)
}
// generate iCalendar and convert to request body
val os = ByteArrayOutputStream()
task.write(os, Constants.iCalProdId)
return GeneratedResource(
suggestedFileName = DavUtils.fileNameFromUid(uid, "ics"),
requestBody = os.toByteArray().toRequestBody(DavCalendar.MIME_ICALENDAR_UTF8),
onSuccessContext = GeneratedResource.OnSuccessContext(
uid = if (uidIsGenerated) Optional.of(uid) else Optional.empty()
)
)
}
override suspend fun listAllRemote(callback: MultiResponseCallback) {
SyncException.wrapWithRemoteResourceSuspending(collection.url) {

View File

@@ -4,12 +4,14 @@
package at.bitfire.davdroid.util
import at.bitfire.davdroid.util.DavUtils.generateUidIfNecessary
import okhttp3.HttpUrl
import okhttp3.MediaType
import okhttp3.MediaType.Companion.toMediaType
import java.net.URI
import java.net.URISyntaxException
import java.util.Locale
import java.util.UUID
/**
* Some WebDAV and HTTP network utility methods.
@@ -43,6 +45,68 @@ object DavUtils {
return String.format(Locale.ROOT, "#%06X%02X", color, alpha)
}
/**
* Generates a usable WebDAV resource name (file name) from an UID.
*
* If the UID contains only characters that are usually not problematic in file names,
* the returned value is `<uid>.<suffix>`. If there are problematic characters,
* the file name will be generated from a random UUID plus suffix instead.
*
* @param uid UID of the iCalendar or vCard
* @param suffix suffix to use (without dot, for instance `ics` for iCalendar files)
* @param generateUuid generator that generates a random UUID
*
* @return file name that can be used to upload the resource
*/
fun fileNameFromUid(
uid: String,
suffix: String,
generateUuid: () -> String = { UUID.randomUUID().toString() }
): String {
val uidIsGoodBaseName: Boolean = uid.all { char ->
// see RFC 2396 2.2
char.isLetterOrDigit() || arrayOf( // allow letters and digits
';', ':', '@', '&', '=', '+', '$', ',', // allow reserved characters except '/' and '?'
'-', '_', '.', '!', '~', '*', '\'', '(', ')' // allow unreserved characters
).contains(char)
}
val baseName = if (uidIsGoodBaseName)
uid
else
generateUuid()
return "$baseName.$suffix"
}
/**
* Result of [generateUidIfNecessary].
*
* @param uid resulting UID (either from existing or generated)
* @param generated *true*: [uid] was generated by [generateUidIfNecessary]; *false*: [uid] was taken from existing UID
*/
data class UidGenerationResult(
val uid: String,
val generated: Boolean
)
/**
* Generates a UID for an iCalendar/vCard if there is no existing UID.
*
* @param existingUid existing UID (may be null)
* @param generateUuid generator that generates a random UUID
*
* @return decomposable result that contains either the existing or the generated UID and whether it was generated
*/
fun generateUidIfNecessary(
existingUid: String?,
generateUuid: () -> String = { UUID.randomUUID().toString() }
): UidGenerationResult =
if (existingUid == null) {
// generate new UID
UidGenerationResult(generateUuid(), generated = true)
} else {
// use existing UID
UidGenerationResult(existingUid, generated = false)
}
// extension methods

View File

@@ -2,14 +2,12 @@
* Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
*/
package at.bitfire.davdroid
package at.bitfire.davdroid.util
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.davdroid.util.DavUtils.lastSegment
import at.bitfire.davdroid.util.DavUtils.parent
import okhttp3.HttpUrl.Companion.toHttpUrl
import okhttp3.MediaType.Companion.toMediaType
import org.junit.Assert
import org.junit.Assert.assertEquals
import org.junit.Test
@@ -28,14 +26,39 @@ class DavUtilsTest {
assertEquals("#000000FF", DavUtils.ARGBtoCalDAVColor(0xFF000000.toInt()))
}
@Test
fun `fileNameFromUid (good uid)`() {
assertEquals("good-uid.txt", DavUtils.fileNameFromUid("good-uid", "txt"))
}
@Test
fun `fileNameFromUid (bad uid)`() {
assertEquals("new-uuid.txt", DavUtils.fileNameFromUid("bad\\uid", "txt", generateUuid = { "new-uuid" }))
}
@Test
fun `generateUidIfNecessary (existing uid)`() {
assertEquals(
DavUtils.UidGenerationResult("existing", generated = false),
DavUtils.generateUidIfNecessary("existing")
)
}
@Test
fun `generateUidIfNecessary (no existing uid)`() {
assertEquals(
DavUtils.UidGenerationResult("new-uuid", generated = true),
DavUtils.generateUidIfNecessary(null, generateUuid = { "new-uuid" })
)
}
@Test
fun testHttpUrl_LastSegment() {
val exampleURL = "http://example.com/"
Assert.assertEquals("/", exampleURL.toHttpUrl().lastSegment)
Assert.assertEquals("dir", (exampleURL + "dir").toHttpUrl().lastSegment)
Assert.assertEquals("dir", (exampleURL + "dir/").toHttpUrl().lastSegment)
Assert.assertEquals("file.html", (exampleURL + "dir/file.html").toHttpUrl().lastSegment)
assertEquals("/", exampleURL.toHttpUrl().lastSegment)
assertEquals("dir", (exampleURL + "dir").toHttpUrl().lastSegment)
assertEquals("dir", (exampleURL + "dir/").toHttpUrl().lastSegment)
assertEquals("file.html", (exampleURL + "dir/file.html").toHttpUrl().lastSegment)
}
@Test
@@ -53,4 +76,4 @@ class DavUtilsTest {
assertEquals("http://example.com/".toHttpUrl(), "http://example.com".toHttpUrl().parent())
}
}
}