Fetch remote records in chunks of max. 50 to avoid server and memory problems

* performance optimisation
* refactoring

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

@@ -97,7 +97,7 @@ public class Contact extends Resource {
 	}
 
 	@Override
-	public void initialize() {
+	public void initRemoteFields() {
 		uid = UUID.randomUUID().toString();
 		name = uid + ".vcf";
 	}
@@ -113,7 +113,7 @@ public class Contact extends Resource {
 		
 		Uid uid = vcard.getUid();
 		if (uid == null) {
-			Log.w(TAG, "Received VCONTACT without UID, generating new one");
+			Log.w(TAG, "Received VCard without UID, generating new one");
 			uid = new Uid(UUID.randomUUID().toString());
 		}
 		this.uid = uid.getValue();

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

@@ -103,7 +103,7 @@ public class Event extends Resource {
 	}
 	
 	@Override
-	public void initialize() {
+	public void initRemoteFields() {
 		uid = DavSyncAdapter.generateUID();
 		name = uid.replace("@", "_") + ".ics";
 	}

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

@@ -8,9 +8,6 @@
 package at.bitfire.davdroid.resource;
 
 import java.util.ArrayList;
-import java.util.LinkedList;
-
-import org.apache.commons.lang.ArrayUtils;
 
 import lombok.Cleanup;
 import android.accounts.Account;
@@ -75,10 +72,13 @@ public abstract class LocalCollection<T extends Resource> {
 			@Cleanup Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID(), entryColumnRemoteName(), entryColumnETag() },
 					where, null, null);
-			LinkedList<Long> dirty = new LinkedList<Long>();
+			if (cursor == null)
-			while (cursor != null && cursor.moveToNext())
+				throw new LocalStorageException("Couldn't query dirty records");
-				dirty.add(cursor.getLong(0));
+			
-			return ArrayUtils.toPrimitive(dirty.toArray(new Long[0]), -1);
+			long[] dirty = new long[cursor.getCount()];
+			for (int idx = 0; cursor.moveToNext(); idx++)
+				dirty[idx] = cursor.getLong(0);
+			return dirty;
 		} catch(RemoteException ex) {
 			throw new LocalStorageException(ex);
 		}
@@ -92,10 +92,13 @@ public abstract class LocalCollection<T extends Resource> {
 			@Cleanup Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID(), entryColumnRemoteName(), entryColumnETag() },
 					where, null, null);
-			LinkedList<Long> deleted = new LinkedList<Long>();
+			if (cursor == null)
-			while (cursor != null && cursor.moveToNext())
+				throw new LocalStorageException("Couldn't query dirty records");
-				deleted.add(cursor.getLong(0));
+			
-			return ArrayUtils.toPrimitive(deleted.toArray(new Long[0]), -1);
+			long deleted[] = new long[cursor.getCount()];
+			for (int idx = 0; cursor.moveToNext(); idx++)
+				deleted[idx] = cursor.getLong(0);
+			return deleted;
 		} catch(RemoteException ex) {
 			throw new LocalStorageException(ex);
 		}
@@ -110,22 +113,25 @@ public abstract class LocalCollection<T extends Resource> {
 			@Cleanup Cursor cursor = providerClient.query(entriesURI(),
 					new String[] { entryColumnID() },
 					where, null, null);
-			LinkedList<Long> fresh = new LinkedList<Long>();
+			if (cursor == null)
-			while (cursor != null && cursor.moveToNext()) {
+				throw new LocalStorageException("Couldn't query new records");
+			
+			long[] fresh = new long[cursor.getCount()];
+			for (int idx = 0; cursor.moveToNext(); idx++) {
 				long id = cursor.getLong(0);
 				
 				// new record: generate UID + remote file name so that we can upload
 				T resource = findById(id, false);
-				resource.initialize();
+				resource.initRemoteFields();
 				// write generated UID + remote file name into database
 				ContentValues values = new ContentValues(2);
 				values.put(entryColumnUID(), resource.getUid());
 				values.put(entryColumnRemoteName(), resource.getName());
 				providerClient.update(ContentUris.withAppendedId(entriesURI(), id), values, null, null);
 				
-				fresh.add(id);
+				fresh[idx] = id;
 			}
-			return ArrayUtils.toPrimitive(fresh.toArray(new Long[0]), -1);
+			return fresh;
 		} catch(RemoteException ex) {
 			throw new LocalStorageException(ex);
 		}
@@ -217,7 +223,7 @@ public abstract class LocalCollection<T extends Resource> {
 	public void commit() throws LocalStorageException {
 		if (!pendingOperations.isEmpty())
 			try {
-				Log.i(TAG, "Committing " + pendingOperations.size() + " operations");
+				Log.d(TAG, "Committing " + pendingOperations.size() + " operations");
 				providerClient.applyBatch(pendingOperations);
 				pendingOperations.clear();
 			} catch (RemoteException ex) {

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

@@ -76,6 +76,8 @@ public abstract class RemoteCollection<T extends Resource> {
 			if (resources.length == 1)
 				return (T[]) new Resource[] { get(resources[0]) };
 			
+			Log.i(TAG, "Multi-getting " + resources.length + " remote resource(s)");
+			
 			LinkedList<String> names = new LinkedList<String>();
 			for (Resource resource : resources)
 				names.add(resource.getName());

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

@@ -36,7 +36,7 @@ public abstract class Resource {
 	}
 	
 	// sets resource name and UID
-	public abstract void initialize();
+	public abstract void initRemoteFields();
 	
 	public abstract void parseEntity(InputStream entity) throws IOException, ParserException, VCardException;
 	public abstract ByteArrayOutputStream toEntity() throws IOException, ValidationException;

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

@@ -58,7 +58,7 @@ public abstract class DavSyncAdapter extends AbstractThreadedSyncAdapter {
 		// set class loader for iCal4j ResourceLoader
 		Thread.currentThread().setContextClassLoader(getContext().getClassLoader());
 		
-		SyncManager syncManager = new SyncManager(account, accountManager);
+		SyncManager syncManager = new SyncManager();
 		
 		Map<LocalCollection<?>, RemoteCollection<?>> syncCollections = getSyncPairs(account, provider);
 		if (syncCollections == null)

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

@@ -15,10 +15,9 @@ import net.fortuna.ical4j.model.ValidationException;
 
 import org.apache.http.HttpException;
 
-import android.accounts.Account;
-import android.accounts.AccountManager;
 import android.content.SyncResult;
 import android.util.Log;
+import at.bitfire.davdroid.ArrayUtils;
 import at.bitfire.davdroid.resource.LocalCollection;
 import at.bitfire.davdroid.resource.LocalStorageException;
 import at.bitfire.davdroid.resource.RecordNotFoundException;
@@ -30,15 +29,9 @@ import at.bitfire.davdroid.webdav.PreconditionFailedException;
 public class SyncManager {
 	private static final String TAG = "davdroid.SyncManager";
 	
-	protected Account account;
+	private static final int MAX_MULTIGET_RESOURCES = 50;
-	protected AccountManager accountManager;
 	
 
-	public SyncManager(Account account, AccountManager accountManager) {
-		this.account = account;
-		this.accountManager = accountManager;
-	}
-
 	public void synchronize(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, boolean manualSync, SyncResult syncResult) throws LocalStorageException, IOException, HttpException {
 		// PHASE 1: push local changes to server
 		int	deletedRemotely = pushDeleted(local, remote),
@@ -79,8 +72,8 @@ public class SyncManager {
 		}
 		
 		// PHASE 3: pull remote changes from server
-		syncResult.stats.numInserts = pullNew(local, remote, remotelyAdded);
+		syncResult.stats.numInserts = pullNew(local, remote, remotelyAdded.toArray(new Resource[0]));
-		syncResult.stats.numUpdates = pullChanged(local, remote, remotelyUpdated);
+		syncResult.stats.numUpdates = pullChanged(local, remote, remotelyUpdated.toArray(new Resource[0]));
 		
 		Log.i(TAG, "Removing non-dirty resources that are not present remotely anymore");
 		local.deleteAllExceptRemoteNames(remoteResources);
@@ -96,9 +89,10 @@ public class SyncManager {
 	private int pushDeleted(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote) throws LocalStorageException, IOException, HttpException {
 		int count = 0;
 		long[] deletedIDs = local.findDeleted();
-		if (deletedIDs != null) {
+		
+		try {
 			Log.i(TAG, "Remotely removing " + deletedIDs.length + " deleted resource(s) (if not changed)");
-			for (long id : deletedIDs) {
+			for (long id : deletedIDs)
 				try {
 					Resource res = local.findById(id, false);
 					if (res.getName() != null)	// is this resource even present remotely?
@@ -109,8 +103,10 @@ public class SyncManager {
 					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");
+				} catch (RecordNotFoundException e) {
+					Log.e(TAG, "Couldn't read locally-deleted record", e);
 				}
-			}
+		} finally {
 			local.commit();
 		}
 		return count;
@@ -119,21 +115,22 @@ public class SyncManager {
 	private int pushNew(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote) throws LocalStorageException, IOException, HttpException {
 		int count = 0;
 		long[] newIDs = local.findNew();
-		if (newIDs != null) {
 		Log.i(TAG, "Uploading " + newIDs.length + " new resource(s) (if not existing)");
-			for (long id : newIDs) {
+		try {
+			for (long id : newIDs)
 				try {
 					Resource res = local.findById(id, true);
-					Log.d(TAG, "Uploading " + res.toString());
 					remote.add(res);
 					local.clearDirty(res);
+					count++;
 				} 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());
+				} catch (RecordNotFoundException e) {
+					Log.e(TAG, "Couldn't read new record", e);
 				}
-				count++;
+		} finally {
-			}
 			local.commit();
 		}
 		return count;
@@ -142,49 +139,59 @@ public class SyncManager {
 	private int pushDirty(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote) throws LocalStorageException, IOException, HttpException {
 		int count = 0;
 		long[] dirtyIDs = local.findDirty();
-		if (dirtyIDs != null) {
 		Log.i(TAG, "Uploading " + dirtyIDs.length + " modified resource(s) (if not changed)");
+		try {
 			for (long id : dirtyIDs) {
 				try {
 					Resource res = local.findById(id, true);
 					remote.update(res);
 					local.clearDirty(res);
+					count++;
 				} 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());
+				} catch (RecordNotFoundException e) {
+					Log.e(TAG, "Couldn't read dirty record", e);
 				}
-				count++;
 			}
+		} finally {
 			local.commit();
 		}
 		return count;
 	}
 	
-	private int pullNew(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, Set<Resource> resourcesToAdd) throws LocalStorageException, IOException, HttpException {
+	private int pullNew(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, Resource[] resourcesToAdd) throws LocalStorageException, IOException, HttpException {
 		int count = 0;
-		Log.i(TAG, "Fetching " + resourcesToAdd.size() + " new remote resource(s)");
+		Log.i(TAG, "Fetching " + resourcesToAdd.length + " new remote resource(s)");
-		if (!resourcesToAdd.isEmpty()) {
+		
-			for (Resource res : remote.multiGet(resourcesToAdd.toArray(new Resource[0]))) {
+		for (Resource[] resources : ArrayUtils.partition(resourcesToAdd, MAX_MULTIGET_RESOURCES))
-				Log.i(TAG, "Adding " + res.getName());
+			try {
+				for (Resource res : remote.multiGet(resources)) {
+					Log.d(TAG, "Adding " + res.getName());
 					local.add(res);
-				local.commit();
 					count++;
 				}
+			} finally {
+				local.commit();
 			}
 		return count;
 	}
 	
-	private int pullChanged(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, Set<Resource> resourcesToUpdate) throws LocalStorageException, IOException, HttpException {
+	private int pullChanged(LocalCollection<? extends Resource> local, RemoteCollection<? extends Resource> remote, Resource[] resourcesToUpdate) throws LocalStorageException, IOException, HttpException {
 		int count = 0;
-		Log.i(TAG, "Fetching " + resourcesToUpdate.size() + " updated remote resource(s)");
+		Log.i(TAG, "Fetching " + resourcesToUpdate.length + " updated remote resource(s)");
-		if (!resourcesToUpdate.isEmpty())
+		
-			for (Resource res : remote.multiGet(resourcesToUpdate.toArray(new Resource[0]))) {
+		for (Resource[] resources : ArrayUtils.partition(resourcesToUpdate, MAX_MULTIGET_RESOURCES))
+			try {
+				for (Resource res : remote.multiGet(resources)) {
 					Log.i(TAG, "Updating " + res.getName());
 					local.updateByRemoteName(res);
-				local.commit();
 					count++;
 				}
+			} finally {
+				local.commit();
+			}
 		return count;
 	}
 

src/at/bitfire/davdroid/webdav/WebDavResource.java

@@ -236,7 +236,7 @@ public class WebDavResource {
 		if (response.getStatusLine().getStatusCode() != HttpStatus.SC_MULTI_STATUS)
 			throw new DavNoMultiStatusException();
 
-		@Cleanup("consumeContent") HttpEntity entity = response.getEntity();
+		HttpEntity entity = response.getEntity();
 		if (entity == null)
 			throw new DavNoContentException();
 		
@@ -277,11 +277,11 @@ public class WebDavResource {
 		if (response.getStatusLine().getStatusCode() != HttpStatus.SC_MULTI_STATUS)
 			throw new DavNoMultiStatusException();
 		
-		@Cleanup("consumeContent") HttpEntity entity = response.getEntity();
+		HttpEntity entity = response.getEntity();
 		if (entity == null)
 			throw new DavNoContentException();
 		
-		InputStream rawContent = entity.getContent();
+		@Cleanup InputStream rawContent = entity.getContent();
 		if (rawContent == null)
 			throw new DavNoContentException();
 		@Cleanup LoggingInputStream content = new LoggingInputStream(TAG, rawContent);
@@ -303,7 +303,7 @@ public class WebDavResource {
 		HttpResponse response = client.execute(get);
 		checkResponse(response);
 		
-		@Cleanup("consumeContent") HttpEntity entity = response.getEntity();
+		HttpEntity entity = response.getEntity();
 		if (entity == null)
 			throw new DavNoContentException();
 		

test/src/at/bitfire/davdroid/webdav/test/WebDavResourceTest.java

@@ -1,16 +1,19 @@
 package at.bitfire.davdroid.webdav.test;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.List;
 
+import lombok.Cleanup;
+
 import org.apache.commons.io.IOUtils;
 import org.apache.http.HttpException;
 
 import android.content.res.AssetManager;
 import android.test.InstrumentationTestCase;
-import at.bitfire.davdroid.webdav.DAVException;
+import at.bitfire.davdroid.webdav.DavException;
 import at.bitfire.davdroid.webdav.DavMultiget;
 import at.bitfire.davdroid.webdav.HttpPropfind;
 import at.bitfire.davdroid.webdav.NotFoundException;
@@ -88,7 +91,7 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 		try {
 			simpleFile.propfind(HttpPropfind.Mode.CURRENT_USER_PRINCIPAL);
 			fail();
-		} catch(DAVException ex) {
+		} catch(DavException ex) {
 		}
 		assertNull(simpleFile.getCurrentUserPrincipal());
 	}
@@ -141,13 +144,12 @@ public class WebDavResourceTest extends InstrumentationTestCase {
 	
 	public void testGet() throws URISyntaxException, IOException, HttpException {
 		simpleFile.get();
-		assertTrue(IOUtils.contentEquals(
+		@Cleanup InputStream is = assetMgr.open("test.random", AssetManager.ACCESS_STREAMING);
-			assetMgr.open("test.random", AssetManager.ACCESS_STREAMING),
+		byte[] expected = IOUtils.toByteArray(is);
-			simpleFile.getContent()
+		assertEquals(expected, simpleFile.getContent());
-		));
 	}
 	
-	public void testMultiGet() throws DAVException, IOException, HttpException {
+	public void testMultiGet() throws DavException, IOException, HttpException {
 		WebDavResource davAddressBook = new WebDavResource(davCollection, "addressbooks/default.vcf", true);
 		davAddressBook.multiGet(DavMultiget.Type.ADDRESS_BOOK, new String[] { "1.vcf", "2.vcf" });
 		assertEquals(2, davAddressBook.getMembers().size());