Unnecessary fetching of remote member ETags

0

The logic for "Phase 2A" of SyncManager.synchronize() seems a bit off and its result goes unused:

https://github.com/rfc2822/davdroid/blob/master/src/at/bitfire/davdroid/syncadapter/SyncManager.java#L53

Assuming I've parsed this right, something like this should work (assuming the manualSync parameter is renamed to pullRemoteCollection)

pullRemoteCollection = pullRemoteCollection || syncResult.stats.numEntries > 0;

// PHASE 2A: check if there's a reason to do a sync with remote (= forced sync or remote CTag changed)
if (!pullRemoteCollection) {
  String remoteCTag    = remoteCollection.getCTag();
  String localCTag     = localCollection.getCTag();
  pullRemoteCollection = (remoteCTag != null && localCTag != null) && remoteCTag.equals(localCTag);
}

if (!pullRemoteCollection) {
  Log.i(TAG, "No local changes made and ctags match, will not pull from remote.");
  return;
}

Also, might have something to do with #147 ?

0

Also, using != on String objects @ line 75:

https://github.com/rfc2822/davdroid/blob/master/src/at/bitfire/davdroid/syncadapter/SyncManager.java#L75

Fix...

if (localResource.getETag() == null || !localResource.getETag().equals(remoteResource.getETag())) {

I think I spotted a couple more of these while browsing through the code base. Would you prefer pull request(s) or filing issues for such small edits?

0

@rhodey: Seems like the return got lost in 8a651f135b95d3c534201142690ed7eed07ece85…

Issues are OK.

Temporal relations are not necessarily causal relations.

Log in to reply

Looks like your connection to Bitfire App Forums was lost, please wait while we try to reconnect.