Refactoring

* add overflow marker "…" to LoggingInputStream
* split the populate() methods
* always close cursors (resolves #120)
* introduce LocalStorageException and its subclass RecordNotFoundException and catch RemoteException earlier
* split synchronize() method into phases
* commit after every added/modified record (obsoletes MAX_UPDATES_BEFORE_COMMIT)
* read contact photos from (high-res) asset instead of the thumbnail blob (resolves #121)

src/at/bitfire/davdroid/LoggingInputStream.java

@@ -14,12 +14,15 @@ public class LoggingInputStream extends FilterInputStream {
 
 	ByteArrayOutputStream log = new ByteArrayOutputStream(MAX_LENGTH);
 	int logSize = 0;
+	boolean overflow = false;
+	
 
 	public LoggingInputStream(String tag, InputStream proxy) {
 		super(proxy);
 		this.tag = tag;
 	}
 	
+
 	@Override
 	public boolean markSupported() {
 		return false;
@@ -31,7 +34,8 @@ public class LoggingInputStream extends FilterInputStream {
 		if (logSize < MAX_LENGTH) {
 			log.write(b);
 			logSize++;
-		}
+		} else
+			overflow = true;
 		return b;
 	}
 
@@ -40,8 +44,10 @@ public class LoggingInputStream extends FilterInputStream {
 			throws IOException {
 		int read = super.read(buffer, byteOffset, byteCount);
 		int bytesToLog = read;
-		if (bytesToLog + logSize > MAX_LENGTH)
+		if (bytesToLog + logSize > MAX_LENGTH) {
 			bytesToLog = MAX_LENGTH - logSize;
+			overflow = true;
+		}
 		if (bytesToLog > 0) {
 			log.write(buffer, byteOffset, bytesToLog);
 			logSize += bytesToLog;
@@ -51,7 +57,7 @@ public class LoggingInputStream extends FilterInputStream {
 
 	@Override
 	public void close() throws IOException {
-		Log.d(tag, "Content: " + log.toString());
+		Log.d(tag, "Content: " + log.toString() + (overflow ? "…" : ""));
 		super.close();
 	}
 

src/at/bitfire/davdroid/resource/LocalAddressBook.java

@@ -7,6 +7,8 @@
  ******************************************************************************/
 package at.bitfire.davdroid.resource;
 
+import java.io.IOException;
+import java.io.InputStream;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.Date;
@@ -15,6 +17,9 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
+import lombok.Cleanup;
+
+import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.WordUtils;
 
@@ -24,6 +29,7 @@ import android.content.ContentProviderClient;
 import android.content.ContentProviderOperation;
 import android.content.ContentProviderOperation.Builder;
 import android.content.ContentUris;
+import android.content.res.AssetFileDescriptor;
 import android.database.Cursor;
 import android.database.DatabaseUtils;
 import android.net.Uri;
@@ -107,28 +113,68 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 	}
 
 	
-	/* content provider (= database) querying */
+	/* create/update/delete */
+	
+	public Contact newResource(long localID, String resourceName, String eTag) {
+		return new Contact(localID, resourceName, eTag);
+	}
+	
+	public void deleteAllExceptRemoteNames(Resource[] remoteResources) {
+		String where;
+		
+		if (remoteResources.length != 0) {
+			List<String> sqlFileNames = new LinkedList<String>();
+			for (Resource res : remoteResources)
+				sqlFileNames.add(DatabaseUtils.sqlEscapeString(res.getName()));
+			where = entryColumnRemoteName() + " NOT IN (" + StringUtils.join(sqlFileNames, ",") + ")";
+		} else
+			where = entryColumnRemoteName() + " IS NOT NULL";
+			
+		Builder builder = ContentProviderOperation.newDelete(entriesURI()).withSelection(where, null);
+		pendingOperations.add(builder
+				.withYieldAllowed(true)
+				.build());
+	}
+	
+	
+	/* methods for populating the data object from the content provider */
 
 	@Override
-	public void populate(Resource res) throws RemoteException {
+	public void populate(Resource res) throws LocalStorageException {
 		Contact c = (Contact)res;
-		if (c.isPopulated())
-			return;
 		
-		Cursor cursor = providerClient.query(ContentUris.withAppendedId(entriesURI(), c.getLocalID()),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(ContentUris.withAppendedId(entriesURI(), c.getLocalID()),
 				new String[] { entryColumnUID(), RawContacts.STARRED }, null, null, null);
 			if (cursor != null && cursor.moveToNext()) {
 				c.setUid(cursor.getString(0));
 				c.setStarred(cursor.getInt(1) != 0);
+			} else
+				throw new RecordNotFoundException();
+		
+			populateStructuredName(c);
+			populatePhoneNumbers(c);
+			populateEmailAddresses(c);
+			populatePhoto(c);
+			populateOrganization(c);
+			populateIMPPs(c);
+			populateNickname(c);
+			populateNote(c);
+			populatePostalAddresses(c);
+			populateURLs(c);
+			populateEvents(c);
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
 	}
 
-		// structured name
+	private void populateStructuredName(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] {
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] {
 				/* 0 */ StructuredName.DISPLAY_NAME, StructuredName.PREFIX, StructuredName.GIVEN_NAME,
 				/* 3 */ StructuredName.MIDDLE_NAME,	StructuredName.FAMILY_NAME, StructuredName.SUFFIX,
 				/* 6 */ StructuredName.PHONETIC_GIVEN_NAME, StructuredName.PHONETIC_MIDDLE_NAME, StructuredName.PHONETIC_FAMILY_NAME
 			}, StructuredName.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
-			new String[] { String.valueOf(res.getLocalID()), StructuredName.CONTENT_ITEM_TYPE }, null);
+			new String[] { String.valueOf(c.getLocalID()), StructuredName.CONTENT_ITEM_TYPE }, null);
 		if (cursor != null && cursor.moveToNext()) {
 			c.setDisplayName(cursor.getString(0));
 			
@@ -142,9 +188,10 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 			c.setPhoneticMiddleName(cursor.getString(7));
 			c.setPhoneticFamilyName(cursor.getString(8));
 		}
+	}
 	
-		// phone numbers
+	protected void populatePhoneNumbers(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Phone.TYPE, Phone.LABEL, Phone.NUMBER },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Phone.TYPE, Phone.LABEL, Phone.NUMBER },
 				Phone.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Phone.CONTENT_ITEM_TYPE }, null);
 		while (cursor != null && cursor.moveToNext()) {
@@ -218,9 +265,10 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 			}
 			c.getPhoneNumbers().add(number);
 		}
+	}
 	
-		// email addresses
+	protected void populateEmailAddresses(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Email.TYPE, Email.ADDRESS, Email.LABEL },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Email.TYPE, Email.ADDRESS, Email.LABEL },
 				Email.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Email.CONTENT_ITEM_TYPE }, null);
 		while (cursor != null && cursor.moveToNext()) {
@@ -239,20 +287,26 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 				String customType = cursor.getString(2);
 				if (customType != null && !customType.isEmpty())
 					email.addType(EmailType.get(labelToXName(customType)));
-				break;
 			}
 			c.getEmails().add(email);
 		}
+	}
 	
-		// photo
+	protected void populatePhoto(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Photo.PHOTO },
+		Uri photoUri = Uri.withAppendedPath(
-				Photo.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
+	             ContentUris.withAppendedId(RawContacts.CONTENT_URI, c.getLocalID()),
-				new String[] { String.valueOf(c.getLocalID()), Photo.CONTENT_ITEM_TYPE }, null);
+	             RawContacts.DisplayPhoto.CONTENT_DIRECTORY);
-		if (cursor != null && cursor.moveToNext())
+		try {
-			c.setPhoto(cursor.getBlob(0));
+			@Cleanup AssetFileDescriptor fd = providerClient.openAssetFile(photoUri, "r");
+			@Cleanup InputStream is = fd.createInputStream();
+			c.setPhoto(IOUtils.toByteArray(is));
+		} catch(IOException ex) {
+			Log.w(TAG, "Couldn't read contact photo", ex);
+		}
+	}
 	
-		// organization
+	protected void populateOrganization(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Organization.COMPANY, Organization.TITLE },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Organization.COMPANY, Organization.TITLE },
 				Photo.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Organization.CONTENT_ITEM_TYPE }, null);
 		if (cursor != null && cursor.moveToNext()) {
@@ -263,9 +317,10 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 			if (role != null && !role.isEmpty())
 				c.setRole(role);
 		}
+	}
 	
-		// IMPPs
+	protected void populateIMPPs(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Im.DATA, Im.TYPE, Im.LABEL, Im.PROTOCOL, Im.CUSTOM_PROTOCOL },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Im.DATA, Im.TYPE, Im.LABEL, Im.PROTOCOL, Im.CUSTOM_PROTOCOL },
 				Photo.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Im.CONTENT_ITEM_TYPE }, null);
 		while (cursor != null && cursor.moveToNext()) {
@@ -302,7 +357,6 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 				break;
 			case Im.PROTOCOL_CUSTOM:
 				impp = new Impp(cursor.getString(4), handle);
-				break;
 			}
 			
 			if (impp != null) {
@@ -317,28 +371,30 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 					String customType = cursor.getString(2);
 					if (customType != null && !customType.isEmpty())
 						impp.addType(ImppType.get(labelToXName(customType)));
-					break;
 				}
 				c.getImpps().add(impp);
 			}
 		}
+	}
 
-		// nick name (max. 1)
+	protected void populateNickname(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Nickname.NAME },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Nickname.NAME },
 				Nickname.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Nickname.CONTENT_ITEM_TYPE }, null);
 		if (cursor != null && cursor.moveToNext())
 			c.setNickName(cursor.getString(0));
+	}
 	
-		// note (max. 1)
+	protected void populateNote(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Note.NOTE },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Note.NOTE },
 				Website.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Note.CONTENT_ITEM_TYPE }, null);
 		if (cursor != null && cursor.moveToNext())
 			c.setNote(cursor.getString(0));
+	}
 	
-		// postal addresses
+	protected void populatePostalAddresses(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] {
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] {
 				/* 0 */ StructuredPostal.FORMATTED_ADDRESS, StructuredPostal.TYPE, StructuredPostal.LABEL,
 				/* 3 */ StructuredPostal.STREET, StructuredPostal.POBOX, StructuredPostal.NEIGHBORHOOD,
 				/* 6 */ StructuredPostal.CITY, StructuredPostal.REGION, StructuredPostal.POSTCODE,
@@ -371,16 +427,18 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 			address.setCountry(cursor.getString(9));
 			c.getAddresses().add(address);
 		}
+	}
 	
-		// URL
+	protected void populateURLs(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { Website.URL },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { Website.URL },
 				Website.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), Website.CONTENT_ITEM_TYPE }, null);
 		if (cursor != null && cursor.moveToNext())
 			c.getURLs().add(cursor.getString(0));
+	}
 	
-		// events
+	protected void populateEvents(Contact c) throws RemoteException {
-		cursor = providerClient.query(dataURI(), new String[] { CommonDataKinds.Event.TYPE, CommonDataKinds.Event.START_DATE },
+		@Cleanup("close") Cursor cursor = providerClient.query(dataURI(), new String[] { CommonDataKinds.Event.TYPE, CommonDataKinds.Event.START_DATE },
 				Photo.RAW_CONTACT_ID + "=? AND " + Data.MIMETYPE + "=?",
 				new String[] { String.valueOf(c.getLocalID()), CommonDataKinds.Event.CONTENT_ITEM_TYPE }, null);
 		while (cursor != null && cursor.moveToNext()) {
@@ -399,63 +457,6 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 				Log.w(TAG, "Couldn't parse local birthday/anniversary date", e);
 			}
 		}
-		
-		c.populated = true;
-		return;
-	}
-	
-	
-	public void deleteAllExceptRemoteNames(Resource[] remoteResources) {
-		String where;
-		
-		if (remoteResources.length != 0) {
-			List<String> sqlFileNames = new LinkedList<String>();
-			for (Resource res : remoteResources)
-				sqlFileNames.add(DatabaseUtils.sqlEscapeString(res.getName()));
-			where = entryColumnRemoteName() + " NOT IN (" + StringUtils.join(sqlFileNames, ",") + ")";
-		} else
-			where = entryColumnRemoteName() + " IS NOT NULL";
-			
-		Builder builder = ContentProviderOperation.newDelete(entriesURI()).withSelection(where, null);
-		pendingOperations.add(builder
-				.withYieldAllowed(true)
-				.build());
-	}
-	
-	
-	/* create/update/delete */
-	
-	public Contact newResource(long localID, String resourceName, String eTag) {
-		return new Contact(localID, resourceName, eTag);
-	}
-
-	
-	/* private helper methods */
-	
-	protected Uri dataURI() {
-		return syncAdapterURI(Data.CONTENT_URI);
-	}
-	
-	protected String labelToXName(String label) {
-		if (label == null)
-			return null;
-		String xName = "X-" + label.replaceAll(" ","_").replaceAll("[^\\p{L}\\p{Nd}\\-_]", "").toUpperCase(Locale.US);
-		return xName;
-	}
-	
-	private Builder newDataInsertBuilder(long raw_contact_id, Integer backrefIdx) {
-		return newDataInsertBuilder(dataURI(), Data.RAW_CONTACT_ID, raw_contact_id, backrefIdx);
-	}
-
-	protected String xNameToLabel(String xname) {
-		if (xname == null)
-			return null;
-		// "x-my_property"
-		// 1. ensure lower case -> "x-my_property"
-		// 2. remove x- from beginning -> "my_property"
-		// 3. replace "_" by " " -> "my property"
-		// 4. capitalize -> "My Property"
-		return WordUtils.capitalize(StringUtils.removeStart(xname.toLowerCase(Locale.US), "x-").replaceAll("_"," "));
 	}
 
 	
@@ -788,4 +789,32 @@ public class LocalAddressBook extends LocalCollection<Contact> {
 			.withValue(CommonDataKinds.Event.TYPE, type) 
 			.withValue(CommonDataKinds.Event.START_DATE, formatter.format(date.getDate()));
 	}
+	
+
+	
+	/* helper methods */
+	
+	protected Uri dataURI() {
+		return syncAdapterURI(Data.CONTENT_URI);
+	}
+	
+	protected String labelToXName(String label) {
+		return "X-" + label.replaceAll(" ","_").replaceAll("[^\\p{L}\\p{Nd}\\-_]", "").toUpperCase(Locale.US);
+	}
+	
+	private Builder newDataInsertBuilder(long raw_contact_id, Integer backrefIdx) {
+		return newDataInsertBuilder(dataURI(), Data.RAW_CONTACT_ID, raw_contact_id, backrefIdx);
+	}
+
+	protected String xNameToLabel(String xname) {
+		if (xname == null)
+			return null;
+		// "x-my_property"
+		// 1. ensure lower case -> "x-my_property"
+		// 2. remove x- from beginning -> "my_property"
+		// 3. replace "_" by " " -> "my property"
+		// 4. capitalize -> "My Property"
+		return WordUtils.capitalize(StringUtils.removeStart(xname.toLowerCase(Locale.US), "x-").replaceAll("_"," "));
+	}
+
 }

src/at/bitfire/davdroid/resource/LocalCalendar.java

@@ -16,6 +16,7 @@ import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import lombok.Cleanup;
 import lombok.Getter;
 import net.fortuna.ical4j.model.Dur;
 import net.fortuna.ical4j.model.Parameter;
@@ -137,7 +138,7 @@ public class LocalCalendar extends LocalCollection<Event> {
 	}
 	
 	public static LocalCalendar[] findAll(Account account, ContentProviderClient providerClient) throws RemoteException {
-		Cursor cursor = providerClient.query(calendarsURI(account),
+		@Cleanup("close") Cursor cursor = providerClient.query(calendarsURI(account),
 				new String[] { Calendars._ID, Calendars.NAME, COLLECTION_COLUMN_CTAG },
 				Calendars.DELETED + "=0 AND " + Calendars.SYNC_EVENTS + "=1", null, null);
 		
@@ -165,15 +166,40 @@ public class LocalCalendar extends LocalCollection<Event> {
 	}
 
 
-	/* content provider (= database) querying */
+	/* create/update/delete */
+	
+	public Event newResource(long localID, String resourceName, String eTag) {
+		return new Event(localID, resourceName, eTag);
+	}
+	
+	public void deleteAllExceptRemoteNames(Resource[] remoteResources) {
+		String where;
+		
+		if (remoteResources.length != 0) {
+			List<String> sqlFileNames = new LinkedList<String>();
+			for (Resource res : remoteResources)
+				sqlFileNames.add(DatabaseUtils.sqlEscapeString(res.getName()));
+			where = entryColumnRemoteName() + " NOT IN (" + StringUtils.join(sqlFileNames, ",") + ")";
+		} else
+			where = entryColumnRemoteName() + " IS NOT NULL";
+		
+		Builder builder = ContentProviderOperation.newDelete(entriesURI())
+				.withSelection(entryColumnParentID() + "=? AND (" + where + ")", new String[] { String.valueOf(id) });
+		pendingOperations.add(builder
+				.withYieldAllowed(true)
+				.build());
+	}
+	
+	
+	/* methods for populating the data object from the content provider */
+	
 
 	@Override
-	public void populate(Resource resource) throws RemoteException {
+	public void populate(Resource resource) throws LocalStorageException {
 		Event e = (Event)resource;
-		if (e.isPopulated())
-			return;
 		
-		Cursor cursor = providerClient.query(ContentUris.withAppendedId(entriesURI(), e.getLocalID()),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(ContentUris.withAppendedId(entriesURI(), e.getLocalID()),
 				new String[] {
 					/*  0 */ Events.TITLE, Events.EVENT_LOCATION, Events.DESCRIPTION,
 					/*  3 */ Events.DTSTART, Events.DTEND, Events.EVENT_TIMEZONE, Events.EVENT_END_TIMEZONE, Events.ALL_DAY,
@@ -265,11 +291,33 @@ public class LocalCalendar extends LocalCollection<Event> {
 					} catch (URISyntaxException ex) {
 						Log.e(TAG, "Error when creating ORGANIZER URI, ignoring", ex);
 					}
+					populateAttendees(e);
+				}
 				
+				// classification
+				switch (cursor.getInt(9)) {
+				case Events.ACCESS_CONFIDENTIAL:
+				case Events.ACCESS_PRIVATE:
+					e.setForPublic(false);
+					break;
+				case Events.ACCESS_PUBLIC:
+					e.setForPublic(true);
+				}
+				
+				populateReminders(e);
+			} else
+				throw new RecordNotFoundException();
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
+	}
+
+	
+	void populateAttendees(Event e) throws RemoteException {
 		Uri attendeesUri = Attendees.CONTENT_URI.buildUpon()
 				.appendQueryParameter(ContactsContract.CALLER_IS_SYNCADAPTER, "true")
 				.build();
-				Cursor c = providerClient.query(attendeesUri, new String[] {
+		@Cleanup("close") Cursor c = providerClient.query(attendeesUri, new String[] {
 				/* 0 */ Attendees.ATTENDEE_EMAIL, Attendees.ATTENDEE_NAME, Attendees.ATTENDEE_TYPE,
 				/* 3 */ Attendees.ATTENDEE_RELATIONSHIP, Attendees.STATUS
 			}, Attendees.EVENT_ID + "=?", new String[] { String.valueOf(e.getLocalID()) }, null);
@@ -324,21 +372,12 @@ public class LocalCalendar extends LocalCollection<Event> {
 		}
 	}
 	
-			// classification
+	void populateReminders(Event e) throws RemoteException {
-			switch (cursor.getInt(9)) {
-			case Events.ACCESS_CONFIDENTIAL:
-			case Events.ACCESS_PRIVATE:
-				e.setForPublic(false);
-				break;
-			case Events.ACCESS_PUBLIC:
-				e.setForPublic(true);
-			}
-			
 		// reminders
 		Uri remindersUri = Reminders.CONTENT_URI.buildUpon()
 				.appendQueryParameter(ContactsContract.CALLER_IS_SYNCADAPTER, "true")
 				.build();
-			Cursor c = providerClient.query(remindersUri, new String[] {
+		@Cleanup("close") Cursor c = providerClient.query(remindersUri, new String[] {
 				/* 0 */ Reminders.MINUTES, Reminders.METHOD
 			}, Reminders.EVENT_ID + "=?", new String[] { String.valueOf(e.getLocalID()) }, null);
 		while (c != null && c.moveToNext()) {
@@ -356,47 +395,6 @@ public class LocalCalendar extends LocalCollection<Event> {
 			e.addAlarm(alarm);
 		}
 	}
-	}
-
-	
-	public void deleteAllExceptRemoteNames(Resource[] remoteResources) {
-		String where;
-		
-		if (remoteResources.length != 0) {
-			List<String> sqlFileNames = new LinkedList<String>();
-			for (Resource res : remoteResources)
-				sqlFileNames.add(DatabaseUtils.sqlEscapeString(res.getName()));
-			where = entryColumnRemoteName() + " NOT IN (" + StringUtils.join(sqlFileNames, ",") + ")";
-		} else
-			where = entryColumnRemoteName() + " IS NOT NULL";
-		
-		Builder builder = ContentProviderOperation.newDelete(entriesURI())
-				.withSelection(entryColumnParentID() + "=? AND (" + where + ")", new String[] { String.valueOf(id) });
-		pendingOperations.add(builder
-				.withYieldAllowed(true)
-				.build());
-	}
-
-	
-	/* create/update/delete */
-	
-	public Event newResource(long localID, String resourceName, String eTag) {
-		return new Event(localID, resourceName, eTag);
-	}
-	
-	
-	/* private helper methods */
-	
-	protected static Uri calendarsURI(Account account) {
-		return Calendars.CONTENT_URI.buildUpon().appendQueryParameter(Calendars.ACCOUNT_NAME, account.name)
-				.appendQueryParameter(Calendars.ACCOUNT_TYPE, account.type)
-				.appendQueryParameter(CalendarContract.CALLER_IS_SYNCADAPTER, "true").build();
-	}
-
-	protected Uri calendarsURI() {
-		return calendarsURI(account);
-	}
-	
 	
 	
 	/* content builder methods */
@@ -570,4 +568,19 @@ public class LocalCalendar extends LocalCollection<Event> {
 				.withValue(Reminders.METHOD, Reminders.METHOD_ALERT)
 				.withValue(Reminders.MINUTES, minutes);
 	}
+	
+	
+	
+	/* private helper methods */
+	
+	protected static Uri calendarsURI(Account account) {
+		return Calendars.CONTENT_URI.buildUpon().appendQueryParameter(Calendars.ACCOUNT_NAME, account.name)
+				.appendQueryParameter(Calendars.ACCOUNT_TYPE, account.type)
+				.appendQueryParameter(CalendarContract.CALLER_IS_SYNCADAPTER, "true").build();
+	}
+
+	protected Uri calendarsURI() {
+		return calendarsURI(account);
+	}
+
 }

src/at/bitfire/davdroid/resource/LocalCollection.java

@@ -10,6 +10,7 @@ package at.bitfire.davdroid.resource;
 import java.util.ArrayList;
 import java.util.LinkedList;
 
+import lombok.Cleanup;
 import android.accounts.Account;
 import android.content.ContentProviderClient;
 import android.content.ContentProviderOperation;
@@ -63,11 +64,12 @@ public abstract class LocalCollection<T extends Resource> {
 	
 	// content provider (= database) querying
 	
-	public Resource[] findDirty() throws RemoteException {
+	public Resource[] findDirty() throws LocalStorageException {
 		String where = entryColumnDirty() + "=1";
 		if (entryColumnParentID() != null)
 			where += " AND " + entryColumnParentID() + "=" + String.valueOf(getId());
-		Cursor cursor = providerClient.query(entriesURI(),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID(), entryColumnRemoteName(), entryColumnETag() },
 					where, null, null);
 			LinkedList<T> dirty = new LinkedList<T>();
@@ -77,13 +79,17 @@ public abstract class LocalCollection<T extends Resource> {
 					dirty.add(resource);
 			}
 			return dirty.toArray(new Resource[0]);
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
 	}
 
-	public Resource[] findDeleted() throws RemoteException {
+	public Resource[] findDeleted() throws LocalStorageException {
 		String where = entryColumnDeleted() + "=1";
 		if (entryColumnParentID() != null)
 			where += " AND " + entryColumnParentID() + "=" + String.valueOf(getId());
-		Cursor cursor = providerClient.query(entriesURI(),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID(), entryColumnRemoteName(), entryColumnETag() },
 					where, null, null);
 			LinkedList<T> deleted = new LinkedList<T>();
@@ -93,13 +99,17 @@ public abstract class LocalCollection<T extends Resource> {
 					deleted.add(resource);
 			}
 			return deleted.toArray(new Resource[0]);
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
 	}
 
-	public Resource[] findNew() throws RemoteException {
+	public Resource[] findNew() throws LocalStorageException {
 		String where = entryColumnDirty() + "=1 AND " + entryColumnRemoteName() + " IS NULL";
 		if (entryColumnParentID() != null)
 			where += " AND " + entryColumnParentID() + "=" + String.valueOf(getId());
-		Cursor cursor = providerClient.query(entriesURI(),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID() },
 					where, null, null);
 			LinkedList<T> fresh = new LinkedList<T>();
@@ -119,10 +129,14 @@ public abstract class LocalCollection<T extends Resource> {
 				}
 			}
 			return fresh.toArray(new Resource[0]);
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
 	}
 	
-	public T findById(long localID, boolean populate) throws RemoteException {
+	public T findById(long localID, boolean populate) throws LocalStorageException {
-		Cursor cursor = providerClient.query(ContentUris.withAppendedId(entriesURI(), localID),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(ContentUris.withAppendedId(entriesURI(), localID),
 					new String[] { entryColumnRemoteName(), entryColumnETag() }, null, null, null);
 			if (cursor != null && cursor.moveToNext()) {
 				T resource = newResource(localID, cursor.getString(0), cursor.getString(1));
@@ -130,11 +144,15 @@ public abstract class LocalCollection<T extends Resource> {
 					populate(resource);
 				return resource;
 			} else
-			return null;
+				throw new RecordNotFoundException();
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
 	}
 	
-	public T findByRemoteName(String remoteName, boolean populate) throws RemoteException {
+	public T findByRemoteName(String remoteName, boolean populate) throws LocalStorageException {
-		Cursor cursor = providerClient.query(entriesURI(),
+		try {
+			@Cleanup("close") Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID(), entryColumnRemoteName(), entryColumnETag() },
 					entryColumnRemoteName() + "=?", new String[] { remoteName }, null);
 			if (cursor != null && cursor.moveToNext()) {
@@ -143,11 +161,14 @@ public abstract class LocalCollection<T extends Resource> {
 					populate(resource);
 				return resource;
 			} else
-			return null;
+				throw new RecordNotFoundException();
+		} catch(RemoteException ex) {
+			throw new LocalStorageException(ex);
+		}
 	}
 
 
-	public abstract void populate(Resource record) throws RemoteException;
+	public abstract void populate(Resource record) throws LocalStorageException;
 	
 	protected void queueOperation(Builder builder) {
 		if (builder != null)
@@ -169,9 +190,8 @@ public abstract class LocalCollection<T extends Resource> {
 		addDataRows(resource, -1, idx);
 	}
 	
-	public void updateByRemoteName(Resource remoteResource) throws RemoteException {
+	public void updateByRemoteName(Resource remoteResource) throws LocalStorageException {
 		T localResource = findByRemoteName(remoteResource.getName(), false);
-		if (localResource != null) {		
 		pendingOperations.add(
 				buildEntry(ContentProviderOperation.newUpdate(ContentUris.withAppendedId(entriesURI(), localResource.getLocalID())), remoteResource)
 				.withValue(entryColumnETag(), remoteResource.getETag())
@@ -181,7 +201,6 @@ public abstract class LocalCollection<T extends Resource> {
 		removeDataRows(localResource);
 		addDataRows(remoteResource, localResource.getLocalID(), -1);
 	}
-	}
 
 	public void delete(Resource resource) {
 		pendingOperations.add(ContentProviderOperation
@@ -198,11 +217,16 @@ public abstract class LocalCollection<T extends Resource> {
 				.withValue(entryColumnDirty(), 0).build());
 	}
 
-	public void commit() throws RemoteException, OperationApplicationException {
+	public void commit() throws LocalStorageException {
-		if (!pendingOperations.isEmpty()) {
+		if (!pendingOperations.isEmpty())
+			try {
 				Log.i(TAG, "Committing " + pendingOperations.size() + " operations");
 				providerClient.applyBatch(pendingOperations);
 				pendingOperations.clear();
+			} catch (RemoteException ex) {
+				throw new LocalStorageException(ex);
+			} catch(OperationApplicationException ex) {
+				throw new LocalStorageException(ex);
 			}
 	}
 

src/at/bitfire/davdroid/resource/LocalStorageException.java

@@ -0,0 +1,24 @@
+package at.bitfire.davdroid.resource;
+
+public class LocalStorageException extends Exception {
+	private static final long serialVersionUID = -7787658815291629529L;
+	
+	private static final String detailMessage = "Couldn't access local content provider";
+	
+
+	public LocalStorageException(String detailMessage, Throwable throwable) {
+		super(detailMessage, throwable);
+	}
+
+	public LocalStorageException(String detailMessage) {
+		super(detailMessage);
+	}
+	
+	public LocalStorageException(Throwable throwable) {
+		super(detailMessage, throwable);
+	}
+
+	public LocalStorageException() {
+		super(detailMessage);
+	}
+}

src/at/bitfire/davdroid/resource/RecordNotFoundException.java

@@ -0,0 +1,17 @@
+package at.bitfire.davdroid.resource;
+
+public class RecordNotFoundException extends LocalStorageException {
+	private static final long serialVersionUID = 4961024282198632578L;
+	
+	private static final String detailMessage = "Record not found in local content provider"; 
+	
+	
+	RecordNotFoundException(Throwable ex) {
+		super(detailMessage, ex);
+	}
+	
+	RecordNotFoundException() {
+		super(detailMessage);
+	}
+
+}

src/at/bitfire/davdroid/resource/Resource.java

@@ -23,8 +23,6 @@ public abstract class Resource {
 	@Getter @Setter protected String uid;
 	@Getter protected long localID;
 	
-	@Getter protected boolean populated = false;
-	
 	
 	public Resource(String name, String ETag) {
 		this.name = name;

src/at/bitfire/davdroid/syncadapter/DavSyncAdapter.java

@@ -15,13 +15,12 @@ import android.content.AbstractThreadedSyncAdapter;
 import android.content.ContentProviderClient;
 import android.content.ContentResolver;
 import android.content.Context;
-import android.content.OperationApplicationException;
 import android.content.SyncResult;
 import android.os.Bundle;
-import android.os.RemoteException;
 import android.provider.Settings;
 import android.util.Log;
 import at.bitfire.davdroid.resource.LocalCollection;
+import at.bitfire.davdroid.resource.LocalStorageException;
 import at.bitfire.davdroid.resource.RemoteCollection;
 import at.bitfire.davdroid.webdav.DAVException;
 
@@ -71,19 +70,16 @@ public abstract class DavSyncAdapter extends AbstractThreadedSyncAdapter {
 				
 			} catch (AuthenticationException ex) {
 				syncResult.stats.numAuthExceptions++;
-				Log.e(TAG, "HTTP authorization error", ex);
+				Log.e(TAG, "HTTP authentication failed", ex);
 			} catch (DAVException ex) {
 				syncResult.stats.numParseExceptions++;
 				Log.e(TAG, "Invalid DAV response", ex);
 			} catch (HttpException ex) {
 				syncResult.stats.numIoExceptions++;
 				Log.e(TAG, "HTTP error", ex);
-			} catch (OperationApplicationException ex) {
+			} catch (LocalStorageException ex) {
 				syncResult.databaseError = true;
-				Log.e(TAG, "Content provider operation error", ex);
+				Log.e(TAG, "Local storage (content provider) exception", ex);
-			} catch (RemoteException ex) {
-				syncResult.databaseError = true;
-				Log.e(TAG, "Remote process (content provider?) died", ex);
 			} catch (IOException ex) {
 				syncResult.stats.numIoExceptions++;
 				Log.e(TAG, "I/O error", ex);

src/at/bitfire/davdroid/syncadapter/SyncManager.java

@@ -17,11 +17,11 @@ import org.apache.http.HttpException;
 
 import android.accounts.Account;
 import android.accounts.AccountManager;
-import android.content.OperationApplicationException;
 import android.content.SyncResult;
-import android.os.RemoteException;
 import android.util.Log;
 import at.bitfire.davdroid.resource.LocalCollection;
+import at.bitfire.davdroid.resource.LocalStorageException;
+import at.bitfire.davdroid.resource.RecordNotFoundException;
 import at.bitfire.davdroid.resource.RemoteCollection;
 import at.bitfire.davdroid.resource.Resource;
 import at.bitfire.davdroid.webdav.NotFoundException;
@@ -29,7 +29,6 @@ import at.bitfire.davdroid.webdav.PreconditionFailedException;
 
 public class SyncManager {
 	private static final String TAG = "davdroid.SyncManager";
-	private static final int MAX_UPDATES_BEFORE_COMMIT = 25;
 	
 	protected Account account;
 	protected AccountManager accountManager;
@@ -40,132 +39,149 @@ public class SyncManager {
 		this.accountManager = accountManager;
 	}
 
-	public void synchronize(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> dav, boolean manualSync, SyncResult syncResult) throws RemoteException, OperationApplicationException, IOException, HttpException {
+	public void synchronize(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, boolean manualSync, SyncResult syncResult) throws LocalStorageException, IOException, HttpException {
-		boolean fetchCollection = false;
+		// PHASE 1: push local changes to server
+		int	deletedRemotely = pushDeleted(local, remote),
+			addedRemotely = pushNew(local, remote),
+			updatedRemotely = pushDirty(local, remote);
 		
-		// PHASE 1: UPLOAD LOCALLY-CHANGED RESOURCES
+		syncResult.stats.numEntries = deletedRemotely + addedRemotely + updatedRemotely;
-		// remove deleted resources from remote
+		
+		// PHASE 2A: check if there's a reason to do a sync with remote (= forced sync or remote CTag changed)
+		boolean fetchCollection = syncResult.stats.numEntries > 0;
+		if (manualSync) {
+			Log.i(TAG, "Synchronization forced");
+			fetchCollection = true;
+		}
+		if (!fetchCollection) {
+			String	currentCTag = remote.getCTag(),
+					lastCTag = local.getCTag();
+			if (currentCTag == null || !currentCTag.equals(lastCTag))
+				fetchCollection = true;
+		}
+		
+		// PHASE 2B: detect details of remote changes
+		Log.i(TAG, "Fetching remote resource list");
+		Set<Resource>	remotelyAdded = new HashSet<Resource>(),
+						remotelyUpdated = new HashSet<Resource>();
+		
+		Resource[] remoteResources = remote.getMemberETags();
+		if (remoteResources != null) {
+			for (Resource remoteResource : remoteResources) {
+				try {
+					Resource localResource = local.findByRemoteName(remoteResource.getName(), false);
+					if (localResource.getETag() == null || !localResource.getETag().equals(remoteResource.getETag()))
+						remotelyUpdated.add(remoteResource);
+				} catch(RecordNotFoundException e) {
+					remotelyAdded.add(remoteResource);
+				}
+			}
+		}
+		
+		// PHASE 3: pull remote changes from server
+		syncResult.stats.numInserts = pullNew(local, remote, remotelyAdded);
+		syncResult.stats.numUpdates = pullChanged(local, remote, remotelyUpdated);
+		
+		Log.i(TAG, "Removing non-dirty resources that are not present remotely anymore");
+		local.deleteAllExceptRemoteNames(remoteResources);
+		local.commit();
+
+		// update collection CTag
+		Log.i(TAG, "Sync complete, fetching new CTag");
+		local.setCTag(remote.getCTag());
+		local.commit();
+	}
+	
+	
+	private int pushDeleted(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote) throws LocalStorageException, IOException, HttpException {
+		int count = 0;
 		Resource[] deletedResources = local.findDeleted();
 		if (deletedResources != null) {
 			Log.i(TAG, "Remotely removing " + deletedResources.length + " deleted resource(s) (if not changed)");
 			for (Resource res : deletedResources) {
 				try {
 					if (res.getName() != null)	// is this resource even present remotely?
-						dav.delete(res);
+						remote.delete(res);
 				} catch(NotFoundException e) {
 					Log.i(TAG, "Locally-deleted resource has already been removed from server");
 				} catch(PreconditionFailedException e) {
 					Log.i(TAG, "Locally-deleted resource has been changed on the server in the meanwhile");
 				}
-				fetchCollection = true;
 				local.delete(res);
+				count++;
 			}
 			local.commit();
 		}
+		return count;
+	}
 	
-		// upload new resources
+	private int pushNew(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote) throws LocalStorageException, IOException, HttpException {
+		int count = 0;
 		Resource[] newResources = local.findNew();
 		if (newResources != null) {
 			Log.i(TAG, "Uploading " + newResources.length + " new resource(s) (if not existing)");
 			for (Resource res : newResources) {
 				try {
-					dav.add(res);
+					remote.add(res);
 				} catch(PreconditionFailedException e) {
 					Log.i(TAG, "Didn't overwrite existing resource with other content");
 				} catch (ValidationException e) {
 					Log.e(TAG, "Couldn't create entity for adding: " + e.toString());
 				}
-				fetchCollection = true;
 				local.clearDirty(res);
+				count++;
 			}
 			local.commit();
 		}
+		return count;
+	}
 	
-		// upload modified resources
+	private int pushDirty(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote) throws LocalStorageException, IOException, HttpException {
+		int count = 0;
 		Resource[] dirtyResources = local.findDirty();
 		if (dirtyResources != null) {
 			Log.i(TAG, "Uploading " + dirtyResources.length + " modified resource(s) (if not changed)");
 			for (Resource res : dirtyResources) {
 				try {
-					dav.update(res);
+					remote.update(res);
 				} catch(PreconditionFailedException e) {
 					Log.i(TAG, "Locally changed resource has been changed on the server in the meanwhile");
 				} catch (ValidationException e) {
 					Log.e(TAG, "Couldn't create entity for updating: " + e.toString());
 				}
-				fetchCollection = true;
 				local.clearDirty(res);
+				count++;
 			}
 			local.commit();
 		}
-		
+		return count;
-		
-		// PHASE 2A: FETCH REMOTE COLLECTION STATUS
-		// has collection changed -> fetch resources?
-		if (manualSync) {
-			Log.i(TAG, "Synchronization forced");
-			fetchCollection = true;
-		}
-		if (!fetchCollection) {
-			String	currentCTag = dav.getCTag(),
-					lastCTag = local.getCTag();
-			if (currentCTag == null || !currentCTag.equals(lastCTag))
-				fetchCollection = true;
 	}
 	
-		if (!fetchCollection)
+	private int pullNew(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, Set<Resource> resourcesToAdd) throws LocalStorageException, IOException, HttpException {
-			return;
+		int count = 0;
-
+		Log.i(TAG, "Fetching " + resourcesToAdd.size() + " new remote resource(s)");
-		// PHASE 2B: FETCH REMOTE COLLECTION SUMMARY
-		// fetch remote resources -> add/overwrite local resources
-		Log.i(TAG, "Fetching remote resource list");
-		
-		Set<Resource> resourcesToAdd = new HashSet<Resource>(),
-				resourcesToUpdate = new HashSet<Resource>();
-		
-		Resource[] remoteResources = dav.getMemberETags();
-		if (remoteResources == null)	// failure
-			return;
-		
-		for (Resource remoteResource : remoteResources) {
-			Resource localResource = local.findByRemoteName(remoteResource.getName(), true);
-			if (localResource == null)
-				resourcesToAdd.add(remoteResource);
-			else if (localResource.getETag() == null || !localResource.getETag().equals(remoteResource.getETag()))
-				resourcesToUpdate.add(remoteResource);
-		}
-		
-		// PHASE 3: DOWNLOAD NEW/REMOTELY-CHANGED RESOURCES
-		Log.i(TAG, "Adding " + resourcesToAdd.size() + " remote resource(s)");
 		if (!resourcesToAdd.isEmpty()) {
-			for (Resource res : dav.multiGet(resourcesToAdd.toArray(new Resource[0]))) {
+			for (Resource res : remote.multiGet(resourcesToAdd.toArray(new Resource[0]))) {
 				Log.i(TAG, "Adding " + res.getName());
 				local.add(res);
-				
-				if (++syncResult.stats.numInserts % MAX_UPDATES_BEFORE_COMMIT == 0)	// avoid TransactionTooLargeException
 				local.commit();
+				count++;
 			}
-			local.commit();
+		}
+		return count;
 	}
 	
-		Log.i(TAG, "Updating from " + resourcesToUpdate.size() + " remote resource(s)");
+	private int pullChanged(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, Set<Resource> resourcesToUpdate) throws LocalStorageException, IOException, HttpException {
+		int count = 0;
+		Log.i(TAG, "Fetching " + resourcesToUpdate.size() + " updated remote resource(s)");
 		if (!resourcesToUpdate.isEmpty())
-			for (Resource res : dav.multiGet(resourcesToUpdate.toArray(new Resource[0]))) {
+			for (Resource res : remote.multiGet(resourcesToUpdate.toArray(new Resource[0]))) {
 				Log.i(TAG, "Updating " + res.getName());
 				local.updateByRemoteName(res);
-				
-				if (++syncResult.stats.numUpdates % MAX_UPDATES_BEFORE_COMMIT == 0)	// avoid TransactionTooLargeException
 				local.commit();
+				count++;
 			}
-		local.commit();
+		return count;
-
-		// delete remotely removed resources
-		Log.i(TAG, "Removing resources that are missing remotely");
-		local.deleteAllExceptRemoteNames(remoteResources);
-		local.commit();
-
-		// update collection CTag
-		local.setCTag(dav.getCTag());
-		local.commit();
 	}
+
 }