From 3dd3742a962ce717837f2dbabf6be61ee5a90433 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Mon, 19 Sep 2011 18:45:19 +0100
Subject: [PATCH] Folded findLostBatches() into receiveAck().

---
 .../sf/briar/api/db/DatabaseComponent.java    |  6 --
 .../sf/briar/db/DatabaseComponentImpl.java    | 80 ++++++-------------
 .../sf/briar/db/DatabaseComponentTest.java    | 16 ++--
 3 files changed, 31 insertions(+), 71 deletions(-)

diff --git a/api/net/sf/briar/api/db/DatabaseComponent.java b/api/net/sf/briar/api/db/DatabaseComponent.java
index b07d49a008..8c1dd2488b 100644
--- a/api/net/sf/briar/api/db/DatabaseComponent.java
+++ b/api/net/sf/briar/api/db/DatabaseComponent.java
@@ -69,12 +69,6 @@ public interface DatabaseComponent {
 	/** Adds a locally generated private message to the database. */
 	void addLocalPrivateMessage(Message m, ContactId c) throws DbException;
 
-	/**
-	 * Finds any lost batches that were sent to the given contact, and marks any
-	 * messages in the batches that are still outstanding for retransmission.
-	 */
-	void findLostBatches(ContactId c) throws DbException;
-
 	/** Generates an acknowledgement for the given contact. */
 	void generateAck(ContactId c, AckWriter a) throws DbException,
 	IOException;
diff --git a/components/net/sf/briar/db/DatabaseComponentImpl.java b/components/net/sf/briar/db/DatabaseComponentImpl.java
index e08425e7de..526141eaa1 100644
--- a/components/net/sf/briar/db/DatabaseComponentImpl.java
+++ b/components/net/sf/briar/db/DatabaseComponentImpl.java
@@ -376,63 +376,6 @@ DatabaseCleaner.Callback {
 		}
 	}
 
-	public void findLostBatches(ContactId c) throws DbException {
-		// Find any lost batches that need to be retransmitted
-		Collection<BatchId> lost;
-		contactLock.readLock().lock();
-		try {
-			if(!containsContact(c)) throw new NoSuchContactException();
-			messageLock.readLock().lock();
-			try {
-				messageStatusLock.writeLock().lock();
-				try {
-					T txn = db.startTransaction();
-					try {
-						lost = db.getLostBatches(txn, c);
-						db.commitTransaction(txn);
-					} catch(DbException e) {
-						db.abortTransaction(txn);
-						throw e;
-					}
-				} finally {
-					messageStatusLock.writeLock().unlock();
-				}
-			} finally {
-				messageLock.readLock().unlock();
-			}
-		} finally {
-			contactLock.readLock().unlock();
-		}
-		for(BatchId batch : lost) {
-			contactLock.readLock().lock();
-			try {
-				if(!containsContact(c)) throw new NoSuchContactException();
-				messageLock.readLock().lock();
-				try {
-					messageStatusLock.writeLock().lock();
-					try {
-						T txn = db.startTransaction();
-						try {
-							if(LOG.isLoggable(Level.FINE))
-								LOG.fine("Removing lost batch");
-							db.removeLostBatch(txn, c, batch);
-							db.commitTransaction(txn);
-						} catch(DbException e) {
-							db.abortTransaction(txn);
-							throw e;
-						}
-					} finally {
-						messageStatusLock.writeLock().unlock();
-					}
-				} finally {
-					messageLock.readLock().unlock();
-				}
-			} finally {
-				contactLock.readLock().unlock();
-			}
-		}
-	}
-
 	public void generateAck(ContactId c, AckWriter a) throws DbException,
 	IOException {
 		contactLock.readLock().lock();
@@ -916,6 +859,7 @@ DatabaseCleaner.Callback {
 			try {
 				messageStatusLock.writeLock().lock();
 				try {
+					// Remove the acked batches' outstanding batch records
 					Collection<BatchId> acks = a.getBatchIds();
 					for(BatchId ack : acks) {
 						T txn = db.startTransaction();
@@ -929,6 +873,28 @@ DatabaseCleaner.Callback {
 					}
 					if(LOG.isLoggable(Level.FINE))
 						LOG.fine("Received " + acks.size() + " acks");
+					// Find any lost batches that need to be retransmitted
+					Collection<BatchId> lost;
+					T txn = db.startTransaction();
+					try {
+						lost = db.getLostBatches(txn, c);
+						db.commitTransaction(txn);
+					} catch(DbException e) {
+						db.abortTransaction(txn);
+						throw e;
+					}
+					for(BatchId batch : lost) {
+						txn = db.startTransaction();
+						try {
+							db.removeLostBatch(txn, c, batch);
+							db.commitTransaction(txn);
+						} catch(DbException e) {
+							db.abortTransaction(txn);
+							throw e;
+						}
+					}
+					if(LOG.isLoggable(Level.FINE))
+						LOG.fine("Removed " + lost.size() + " lost batches");
 				} finally {
 					messageStatusLock.writeLock().unlock();
 				}
diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java
index 886b2d7da4..c7b0ba20c1 100644
--- a/test/net/sf/briar/db/DatabaseComponentTest.java
+++ b/test/net/sf/briar/db/DatabaseComponentTest.java
@@ -587,11 +587,11 @@ public abstract class DatabaseComponentTest extends TestCase {
 		final TransportUpdate transportsUpdate = context.mock(TransportUpdate.class);
 		context.checking(new Expectations() {{
 			// Check whether the contact is still in the DB (which it's not)
-			exactly(18).of(database).startTransaction();
+			exactly(17).of(database).startTransaction();
 			will(returnValue(txn));
-			exactly(18).of(database).containsContact(txn, contactId);
+			exactly(17).of(database).containsContact(txn, contactId);
 			will(returnValue(false));
-			exactly(18).of(database).commitTransaction(txn);
+			exactly(17).of(database).commitTransaction(txn);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner);
 
@@ -600,11 +600,6 @@ public abstract class DatabaseComponentTest extends TestCase {
 			fail();
 		} catch(NoSuchContactException expected) {}
 
-		try {
-			db.findLostBatches(contactId);
-			fail();
-		} catch(NoSuchContactException expected) {}
-
 		try {
 			db.generateAck(contactId, ackWriter);
 			fail();
@@ -918,6 +913,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 
 	@Test
 	public void testReceiveAck() throws Exception {
+		final BatchId batchId1 = new BatchId(TestUtils.getRandomId());
 		Mockery context = new Mockery();
 		@SuppressWarnings("unchecked")
 		final Database<Object> database = context.mock(Database.class);
@@ -933,6 +929,10 @@ public abstract class DatabaseComponentTest extends TestCase {
 			oneOf(ack).getBatchIds();
 			will(returnValue(Collections.singletonList(batchId)));
 			oneOf(database).removeAckedBatch(txn, contactId, batchId);
+			// Find lost batches
+			oneOf(database).getLostBatches(txn, contactId);
+			will(returnValue(Collections.singletonList(batchId1)));
+			oneOf(database).removeLostBatch(txn, contactId, batchId1);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner);
 
-- 
GitLab