From cee4956b374607297b278726efbe52292959f6fa Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Thu, 28 Jul 2011 11:17:33 +0100
Subject: [PATCH] If no messages are added to a batch, don't call
 BatchWriter.finish() - this allows the caller to avoid creating an empty
 packet by delaying creation of the packet's header and trailer until
 something's written to the packet's body. Changed the return semantics of
 DatabaseComponent.generateBatch(ContactId, BatchWriter,
 Collection<MessageId>) so that the IDs of messages considered for inclusion
 in the batch but no longer sendable are also returned - this allows the
 caller to remove them from the set of requested IDs.

---
 .../sf/briar/api/db/DatabaseComponent.java    |  5 ++-
 .../db/ReadWriteLockDatabaseComponent.java    | 31 ++++++++------
 .../db/SynchronizedDatabaseComponent.java     | 42 ++++++++++++-------
 3 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/api/net/sf/briar/api/db/DatabaseComponent.java b/api/net/sf/briar/api/db/DatabaseComponent.java
index 5a1352e738..4a5da87dd8 100644
--- a/api/net/sf/briar/api/db/DatabaseComponent.java
+++ b/api/net/sf/briar/api/db/DatabaseComponent.java
@@ -79,8 +79,9 @@ public interface DatabaseComponent {
 
 	/**
 	 * Generates a batch of messages for the given contact from the given
-	 * collection of requested messages, and returns the IDs of the messages
-	 * added to the bacth.
+	 * collection of requested messages, and returns the IDs of any messages
+	 * that were either added to the batch, or were considered for inclusion
+	 * but are no longer sendable to the contact.
 	 */
 	Collection<MessageId> generateBatch(ContactId c, BatchWriter b,
 			Collection<MessageId> requested) throws DbException, IOException;
diff --git a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java
index a4cd24cac4..996d24e9aa 100644
--- a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java
+++ b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java
@@ -300,13 +300,11 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 					try {
 						Txn txn = db.startTransaction();
 						try {
+							sent = new ArrayList<MessageId>();
 							int capacity = b.getCapacity();
 							Collection<MessageId> sendable =
 								db.getSendableMessages(txn, c, capacity);
-							Iterator<MessageId> it = sendable.iterator();
-							sent = new ArrayList<MessageId>();
-							while(it.hasNext()) {
-								MessageId m = it.next();
+							for(MessageId m : sendable) {
 								byte[] raw = db.getMessage(txn, m);
 								if(!b.writeMessage(raw)) break;
 								bytesSent += raw.length;
@@ -326,9 +324,9 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 				} finally {
 					messageStatusLock.readLock().unlock();
 				}
-				BatchId id = b.finish();
 				// Record the contents of the batch, unless it's empty
 				if(sent.isEmpty()) return;
+				BatchId id = b.finish();
 				messageStatusLock.writeLock().lock();
 				try {
 					Txn txn = db.startTransaction();
@@ -357,7 +355,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 			if(!containsContact(c)) throw new NoSuchContactException();
 			messageLock.readLock().lock();
 			try {
-				Collection<MessageId> sent;
+				Collection<MessageId> sent, considered;
 				messageStatusLock.readLock().lock();
 				try{
 					subscriptionLock.readLock().lock();
@@ -365,13 +363,20 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 						Txn txn = db.startTransaction();
 						try {
 							sent = new ArrayList<MessageId>();
+							considered = new ArrayList<MessageId>();
 							int bytesSent = 0;
 							for(MessageId m : requested) {
 								byte[] raw = db.getMessageIfSendable(txn, c, m);
-								if(raw == null) continue;
-								if(!b.writeMessage(raw)) break;
-								bytesSent += raw.length;
-								sent.add(m);
+								// If the message is still sendable, try to add
+								// it to the batch. If the batch is full, don't
+								// treat the message as considered, and don't
+								// try to add any further messages.
+								if(raw != null) {
+									if(!b.writeMessage(raw)) break;
+									bytesSent += raw.length;
+									sent.add(m);
+								}
+								considered.add(m);
 							}
 							db.commitTransaction(txn);
 						} catch(DbException e) {
@@ -387,16 +392,16 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 				} finally {
 					messageStatusLock.readLock().unlock();
 				}
-				BatchId id = b.finish();
 				// Record the contents of the batch, unless it's empty
-				if(sent.isEmpty()) return sent;
+				if(sent.isEmpty()) return considered;
+				BatchId id = b.finish();
 				messageStatusLock.writeLock().lock();
 				try {
 					Txn txn = db.startTransaction();
 					try {
 						db.addOutstandingBatch(txn, c, id, sent);
 						db.commitTransaction(txn);
-						return sent;
+						return considered;
 					} catch(DbException e) {
 						db.abortTransaction(txn);
 						throw e;
diff --git a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java
index af8f67cd02..3c1aecd158 100644
--- a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java
+++ b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java
@@ -223,23 +223,24 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 					synchronized(subscriptionLock) {
 						Txn txn = db.startTransaction();
 						try {
-							int capacity = b.getCapacity();
-							Collection<MessageId> sendable =
-								db.getSendableMessages(txn, c, capacity);
-							Iterator<MessageId> it = sendable.iterator();
 							Collection<MessageId> sent =
 								new ArrayList<MessageId>();
 							int bytesSent = 0;
-							while(it.hasNext()) {
-								MessageId m = it.next();
+							int capacity = b.getCapacity();
+							Collection<MessageId> sendable =
+								db.getSendableMessages(txn, c, capacity);
+							for(MessageId m : sendable) {
 								byte[] raw = db.getMessage(txn, m);
 								if(!b.writeMessage(raw)) break;
 								bytesSent += raw.length;
 								sent.add(m);
 							}
-							BatchId id = b.finish();
-							if(!sent.isEmpty())
+							// If the batch is not empty, calculate its ID and
+							// record it as outstanding
+							if(!sent.isEmpty()) {
+								BatchId id = b.finish();
 								db.addOutstandingBatch(txn, c, id, sent);
+							}
 							db.commitTransaction(txn);
 						} catch(DbException e) {
 							db.abortTransaction(txn);
@@ -265,19 +266,30 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 						try {
 							Collection<MessageId> sent =
 								new ArrayList<MessageId>();
+							Collection<MessageId> considered =
+								new ArrayList<MessageId>();
 							int bytesSent = 0;
 							for(MessageId m : requested) {
 								byte[] raw = db.getMessageIfSendable(txn, c, m);
-								if(raw == null) continue;
-								if(!b.writeMessage(raw)) break;
-								bytesSent += raw.length;
-								sent.add(m);
+								// If the message is still sendable, try to add
+								// it to the batch. If the batch is full, don't
+								// treat the message as considered, and don't
+								// try to add any further messages.
+								if(raw != null) {
+									if(!b.writeMessage(raw)) break;
+									bytesSent += raw.length;
+									sent.add(m);
+								}
+								considered.add(m);
 							}
-							BatchId id = b.finish();
-							if(!sent.isEmpty())
+							// If the batch is not empty, calculate its ID and
+							// record it as outstanding
+							if(!sent.isEmpty()) {
+								BatchId id = b.finish();
 								db.addOutstandingBatch(txn, c, id, sent);
+							}
 							db.commitTransaction(txn);
-							return sent;
+							return considered;
 						} catch(DbException e) {
 							db.abortTransaction(txn);
 							throw e;
-- 
GitLab