From 1a351535bec7add493224890dbb2ab31b8309f58 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Tue, 19 Nov 2013 22:13:26 +0000
Subject: [PATCH] The response to a BMP Offer is now an Ack and/or a Request.

The Request packet now contains a list of message IDs, rather than a
bitmap referring to the list of messages IDs in the Offer. This allows
the Request to be understood out of context, e.g. if the Offer and
Request are sent over separate connections or a connection is replayed.
---
 .../net/sf/briar/api/db/AckAndRequest.java    | 27 ++++++
 .../sf/briar/api/db/DatabaseComponent.java    | 24 ++---
 .../net/sf/briar/api/messaging/Request.java   | 25 ++---
 .../sf/briar/db/DatabaseComponentImpl.java    | 25 +++--
 .../sf/briar/messaging/PacketReaderImpl.java  | 37 ++++----
 .../sf/briar/messaging/PacketWriterImpl.java  | 19 +---
 .../messaging/duplex/DuplexConnection.java    | 87 ++++-------------
 briar-tests/build.xml                         |  1 -
 .../net/sf/briar/ProtocolIntegrationTest.java | 11 +--
 .../sf/briar/db/DatabaseComponentTest.java    | 13 ++-
 .../briar/messaging/PacketReaderImplTest.java | 68 +++++---------
 .../briar/messaging/PacketWriterImplTest.java | 93 -------------------
 12 files changed, 128 insertions(+), 302 deletions(-)
 create mode 100644 briar-api/src/net/sf/briar/api/db/AckAndRequest.java
 delete mode 100644 briar-tests/src/net/sf/briar/messaging/PacketWriterImplTest.java

diff --git a/briar-api/src/net/sf/briar/api/db/AckAndRequest.java b/briar-api/src/net/sf/briar/api/db/AckAndRequest.java
new file mode 100644
index 0000000000..11959a17d9
--- /dev/null
+++ b/briar-api/src/net/sf/briar/api/db/AckAndRequest.java
@@ -0,0 +1,27 @@
+package net.sf.briar.api.db;
+
+import net.sf.briar.api.messaging.Ack;
+import net.sf.briar.api.messaging.Request;
+
+/**
+ * A tuple of an {@link net.sf.briar.api.messaging.Ack} and a
+ * {@link net.sf.briar.api.messaging.Request}.
+ */
+public class AckAndRequest {
+
+	private final Ack ack;
+	private final Request request;
+
+	public AckAndRequest(Ack ack, Request request) {
+		this.ack = ack;
+		this.request = request;
+	}
+
+	public Ack getAck() {
+		return ack;
+	}
+
+	public Request getRequest() {
+		return request;
+	}
+}
diff --git a/briar-api/src/net/sf/briar/api/db/DatabaseComponent.java b/briar-api/src/net/sf/briar/api/db/DatabaseComponent.java
index 5dcf089862..312dfd18ec 100644
--- a/briar-api/src/net/sf/briar/api/db/DatabaseComponent.java
+++ b/briar-api/src/net/sf/briar/api/db/DatabaseComponent.java
@@ -20,7 +20,6 @@ import net.sf.briar.api.messaging.GroupStatus;
 import net.sf.briar.api.messaging.Message;
 import net.sf.briar.api.messaging.MessageId;
 import net.sf.briar.api.messaging.Offer;
-import net.sf.briar.api.messaging.Request;
 import net.sf.briar.api.messaging.RetentionAck;
 import net.sf.briar.api.messaging.RetentionUpdate;
 import net.sf.briar.api.messaging.SubscriptionAck;
@@ -98,9 +97,9 @@ public interface DatabaseComponent {
 	 * collection of requested messages, with a total length less than or equal
 	 * to the given length, for transmission over a transport with the given
 	 * maximum latency. Any messages that were either added to the batch, or
-	 * were considered but are no longer sendable to the contact, are removed
-	 * from the collection of requested messages before returning. Returns null
-	 * if there are no sendable messages that fit in the given length.
+	 * were considered but are not sendable to the contact, are removed from
+	 * the collection of requested messages before returning. Returns null if
+	 * there are no sendable messages that fit in the given length.
 	 */
 	Collection<byte[]> generateBatch(ContactId c, int maxLength,
 			long maxLatency, Collection<MessageId> requested)
@@ -259,14 +258,17 @@ public interface DatabaseComponent {
 	void receiveMessage(ContactId c, Message m) throws DbException;
 
 	/**
-	 * Processes an offer from the given contact and generates a request for
-	 * any messages in the offer that the contact should send. To prevent
-	 * contacts from using offers to test for subscriptions that are not
-	 * visible to them, any messages belonging to groups that are not visible
-	 * to the contact are requested just as though they were not present in the
-	 * database.
+	 * Processes an offer from the given contact and generates an ack for any
+	 * messages in the offer that are present in the database, and a request
+	 * for any messages that are not. The ack or the request may be null if no
+	 * messages meet the respective criteria.
+	 * <p>
+	 * To prevent contacts from using offers to test for subscriptions that are
+	 * not visible to them, any messages belonging to groups that are not
+	 * visible to the contact are requested just as though they were not
+	 * present in the database.
 	 */
-	Request receiveOffer(ContactId c, Offer o) throws DbException;
+	AckAndRequest receiveOffer(ContactId c, Offer o) throws DbException;
 
 	/** Processes a retention ack from the given contact. */
 	void receiveRetentionAck(ContactId c, RetentionAck a) throws DbException;
diff --git a/briar-api/src/net/sf/briar/api/messaging/Request.java b/briar-api/src/net/sf/briar/api/messaging/Request.java
index e789b4f353..ec22cb8e5f 100644
--- a/briar-api/src/net/sf/briar/api/messaging/Request.java
+++ b/briar-api/src/net/sf/briar/api/messaging/Request.java
@@ -1,31 +1,18 @@
 package net.sf.briar.api.messaging;
 
-import java.util.BitSet;
+import java.util.Collection;
 
-/**
- * A packet requesting some or all of the {@link Message}s from an
- * {@link Offer}.
- */
+/** A packet requesting one or more {@link Message}s from the recipient. */
 public class Request {
 
-	private final BitSet requested;
-	private final int length;
+	private final Collection<MessageId> requested;
 
-	public Request(BitSet requested, int length) {
+	public Request(Collection<MessageId> requested) {
 		this.requested = requested;
-		this.length = length;
 	}
 
-	/**
-	 * Returns a sequence of bits corresponding to the sequence of messages in
-	 * the offer, where the i^th bit is set if the i^th message should be sent.
-	 */
-	public BitSet getBitmap() {
+	/** Returns the identifiers of the requested messages. */
+	public Collection<MessageId> getMessageIds() {
 		return requested;
 	}
-
-	/** Returns the length of the bitmap in bits. */
-	public int getLength() {
-		return length;
-	}
 }
diff --git a/briar-core/src/net/sf/briar/db/DatabaseComponentImpl.java b/briar-core/src/net/sf/briar/db/DatabaseComponentImpl.java
index 7c43a3a48e..75778cc595 100644
--- a/briar-core/src/net/sf/briar/db/DatabaseComponentImpl.java
+++ b/briar-core/src/net/sf/briar/db/DatabaseComponentImpl.java
@@ -11,7 +11,6 @@ import static net.sf.briar.db.DatabaseConstants.MIN_FREE_SPACE;
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -32,6 +31,7 @@ import net.sf.briar.api.TransportConfig;
 import net.sf.briar.api.TransportId;
 import net.sf.briar.api.TransportProperties;
 import net.sf.briar.api.clock.Clock;
+import net.sf.briar.api.db.AckAndRequest;
 import net.sf.briar.api.db.ContactExistsException;
 import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.DbException;
@@ -1402,9 +1402,9 @@ DatabaseCleaner.Callback {
 		return storeGroupMessage(txn, m, c);
 	}
 
-	public Request receiveOffer(ContactId c, Offer o) throws DbException {
-		Collection<MessageId> offered;
-		BitSet request;
+	public AckAndRequest receiveOffer(ContactId c, Offer o) throws DbException {
+		List<MessageId> ack = new ArrayList<MessageId>();
+		List<MessageId> request = new ArrayList<MessageId>();
 		contactLock.readLock().lock();
 		try {
 			messageLock.writeLock().lock();
@@ -1415,15 +1415,10 @@ DatabaseCleaner.Callback {
 					try {
 						if(!db.containsContact(txn, c))
 							throw new NoSuchContactException();
-						offered = o.getMessageIds();
-						request = new BitSet(offered.size());
-						Iterator<MessageId> it = offered.iterator();
-						for(int i = 0; it.hasNext(); i++) {
-							// If the message is not in the database, or not
-							// visible to the contact, request it
-							MessageId m = it.next();
-							if(!db.setStatusSeenIfVisible(txn, c, m))
-								request.set(i);
+						for(MessageId m : o.getMessageIds()) {
+							// If the message is present and visible, ack it
+							if(db.setStatusSeenIfVisible(txn, c, m)) ack.add(m);
+							else request.add(m);
 						}
 						db.commitTransaction(txn);
 					} catch(DbException e) {
@@ -1439,7 +1434,9 @@ DatabaseCleaner.Callback {
 		} finally {
 			contactLock.readLock().unlock();
 		}
-		return new Request(request, offered.size());
+		Ack a = ack.isEmpty() ? null : new Ack(ack);
+		Request r = request.isEmpty() ? null : new Request(request);
+		return new AckAndRequest(a, r);
 	}
 
 	public void receiveRetentionAck(ContactId c, RetentionAck a)
diff --git a/briar-core/src/net/sf/briar/messaging/PacketReaderImpl.java b/briar-core/src/net/sf/briar/messaging/PacketReaderImpl.java
index ff30595e40..bb57923713 100644
--- a/briar-core/src/net/sf/briar/messaging/PacketReaderImpl.java
+++ b/briar-core/src/net/sf/briar/messaging/PacketReaderImpl.java
@@ -17,7 +17,6 @@ import static net.sf.briar.api.messaging.Types.TRANSPORT_UPDATE;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
-import java.util.BitSet;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -113,22 +112,22 @@ class PacketReaderImpl implements PacketReader {
 		// Read the start of the struct
 		r.readStructStart(OFFER);
 		// Read the message IDs
-		List<MessageId> messages = new ArrayList<MessageId>();
+		List<MessageId> offered = new ArrayList<MessageId>();
 		r.readListStart();
 		while(!r.hasListEnd()) {
 			byte[] b = r.readBytes(UniqueId.LENGTH);
 			if(b.length != UniqueId.LENGTH)
 				throw new FormatException();
-			messages.add(new MessageId(b));
+			offered.add(new MessageId(b));
 		}
-		if(messages.isEmpty()) throw new FormatException();
+		if(offered.isEmpty()) throw new FormatException();
 		r.readListEnd();
 		// Read the end of the struct
 		r.readStructEnd();
 		// Reset the reader
 		r.removeConsumer(counting);
 		// Build and return the offer
-		return new Offer(Collections.unmodifiableList(messages));
+		return new Offer(Collections.unmodifiableList(offered));
 	}
 
 	public boolean hasRequest() throws IOException {
@@ -141,25 +140,23 @@ class PacketReaderImpl implements PacketReader {
 		r.addConsumer(counting);
 		// Read the start of the struct
 		r.readStructStart(REQUEST);
-		// There may be up to 7 bits of padding at the end of the bitmap
-		int padding = r.readUint7();
-		if(padding > 7) throw new FormatException();
-		// Read the bitmap
-		byte[] bitmap = r.readBytes(MAX_PACKET_LENGTH);
+		// Read the message IDs
+		List<MessageId> requested = new ArrayList<MessageId>();
+		r.readListStart();
+		while(!r.hasListEnd()) {
+			byte[] b = r.readBytes(UniqueId.LENGTH);
+			if(b.length != UniqueId.LENGTH)
+				throw new FormatException();
+			requested.add(new MessageId(b));
+		}
+		if(requested.isEmpty()) throw new FormatException();
+		r.readListEnd();
 		// Read the end of the struct
 		r.readStructEnd();
 		// Reset the reader
 		r.removeConsumer(counting);
-		// Convert the bitmap into a BitSet
-		int length = bitmap.length * 8 - padding;
-		BitSet b = new BitSet(length);
-		for(int i = 0; i < bitmap.length; i++) {
-			for(int j = 0; j < 8 && i * 8 + j < length; j++) {
-				byte bit = (byte) (128 >> j);
-				if((bitmap[i] & bit) != 0) b.set(i * 8 + j);
-			}
-		}
-		return new Request(b, length);
+		// Build and return the request
+		return new Request(Collections.unmodifiableList(requested));
 	}
 
 	public boolean hasRetentionAck() throws IOException {
diff --git a/briar-core/src/net/sf/briar/messaging/PacketWriterImpl.java b/briar-core/src/net/sf/briar/messaging/PacketWriterImpl.java
index 9b2607f8dd..281d5a36b9 100644
--- a/briar-core/src/net/sf/briar/messaging/PacketWriterImpl.java
+++ b/briar-core/src/net/sf/briar/messaging/PacketWriterImpl.java
@@ -14,7 +14,6 @@ import static net.sf.briar.api.messaging.Types.TRANSPORT_UPDATE;
 
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.BitSet;
 
 import net.sf.briar.api.messaging.Ack;
 import net.sf.briar.api.messaging.Group;
@@ -92,22 +91,10 @@ class PacketWriterImpl implements PacketWriter {
 	}
 
 	public void writeRequest(Request r) throws IOException {
-		BitSet b = r.getBitmap();
-		int length = r.getLength();
-		// If the number of bits isn't a multiple of 8, round up to a byte
-		int bytes = length % 8 == 0 ? length / 8 : length / 8 + 1;
-		byte[] bitmap = new byte[bytes];
-		// I'm kind of surprised BitSet doesn't have a method for this
-		for(int i = 0; i < length; i++) {
-			if(b.get(i)) {
-				int offset = i / 8;
-				byte bit = (byte) (128 >> i % 8);
-				bitmap[offset] |= bit;
-			}
-		}
 		w.writeStructStart(REQUEST);
-		w.writeUint7((byte) (bytes * 8 - length));
-		w.writeBytes(bitmap);
+		w.writeListStart();
+		for(MessageId m : r.getMessageIds()) w.writeBytes(m.getBytes());
+		w.writeListEnd();
 		w.writeStructEnd();
 		if(flush) out.flush();
 	}
diff --git a/briar-core/src/net/sf/briar/messaging/duplex/DuplexConnection.java b/briar-core/src/net/sf/briar/messaging/duplex/DuplexConnection.java
index d671b09072..97b7ad4158 100644
--- a/briar-core/src/net/sf/briar/messaging/duplex/DuplexConnection.java
+++ b/briar-core/src/net/sf/briar/messaging/duplex/DuplexConnection.java
@@ -8,12 +8,7 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.security.GeneralSecurityException;
-import java.util.ArrayList;
-import java.util.BitSet;
 import java.util.Collection;
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Executor;
 import java.util.concurrent.LinkedBlockingQueue;
@@ -23,6 +18,7 @@ import java.util.logging.Logger;
 import net.sf.briar.api.ContactId;
 import net.sf.briar.api.FormatException;
 import net.sf.briar.api.TransportId;
+import net.sf.briar.api.db.AckAndRequest;
 import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.DbException;
 import net.sf.briar.api.db.event.ContactRemovedEvent;
@@ -93,8 +89,6 @@ abstract class DuplexConnection implements DatabaseListener {
 	private final AtomicBoolean canSendOffer, disposed;
 	private final BlockingQueue<Runnable> writerTasks;
 
-	private Collection<MessageId> offered = null; // Locking: this
-
 	private volatile PacketWriter writer = null;
 
 	DuplexConnection(Executor dbExecutor, Executor cryptoExecutor,
@@ -142,8 +136,11 @@ abstract class DuplexConnection implements DatabaseListener {
 		} else if(e instanceof LocalSubscriptionsUpdatedEvent) {
 			LocalSubscriptionsUpdatedEvent l =
 					(LocalSubscriptionsUpdatedEvent) e;
-			if(l.getAffectedContacts().contains(contactId))
+			if(l.getAffectedContacts().contains(contactId)) {
 				dbExecutor.execute(new GenerateSubscriptionUpdate());
+				if(canSendOffer.getAndSet(false))
+					dbExecutor.execute(new GenerateOffer());
+			}
 		} else if(e instanceof LocalTransportsUpdatedEvent) {
 			dbExecutor.execute(new GenerateTransportUpdates());
 		} else if(e instanceof MessageReceivedEvent) {
@@ -159,6 +156,8 @@ abstract class DuplexConnection implements DatabaseListener {
 			dbExecutor.execute(new GenerateRetentionAck());
 		} else if(e instanceof RemoteSubscriptionsUpdatedEvent) {
 			dbExecutor.execute(new GenerateSubscriptionAck());
+			if(canSendOffer.getAndSet(false))
+				dbExecutor.execute(new GenerateOffer());
 		} else if(e instanceof RemoteTransportsUpdatedEvent) {
 			dbExecutor.execute(new GenerateTransportAcks());
 		}
@@ -185,24 +184,7 @@ abstract class DuplexConnection implements DatabaseListener {
 				} else if(reader.hasRequest()) {
 					Request r = reader.readRequest();
 					if(LOG.isLoggable(INFO)) LOG.info("Received request");
-					// Retrieve the offered message IDs
-					Collection<MessageId> offered = getOfferedMessageIds();
-					if(offered == null) throw new FormatException();
-					// Work out which messages were requested
-					BitSet b = r.getBitmap();
-					List<MessageId> requested = new LinkedList<MessageId>();
-					List<MessageId> seen = new ArrayList<MessageId>();
-					int i = 0;
-					for(MessageId m : offered) {
-						if(b.get(i++)) requested.add(m);
-						else seen.add(m);
-					}
-					requested = Collections.synchronizedList(requested);
-					seen = Collections.unmodifiableList(seen);
-					// Mark the unrequested messages as seen
-					dbExecutor.execute(new SetSeen(seen));
-					// Start sending the requested messages
-					dbExecutor.execute(new GenerateBatches(requested));
+					dbExecutor.execute(new GenerateBatches(r.getMessageIds()));
 				} else if(reader.hasRetentionAck()) {
 					RetentionAck a = reader.readRetentionAck();
 					if(LOG.isLoggable(INFO)) LOG.info("Received retention ack");
@@ -244,17 +226,6 @@ abstract class DuplexConnection implements DatabaseListener {
 		}
 	}
 
-	private synchronized Collection<MessageId> getOfferedMessageIds() {
-		Collection<MessageId> ids = offered;
-		offered = null;
-		return ids;
-	}
-
-	private synchronized void setOfferedMessageIds(Collection<MessageId> ids) {
-		assert offered == null;
-		offered = ids;
-	}
-
 	void write() {
 		connRegistry.registerConnection(contactId, transportId);
 		db.addListener(this);
@@ -383,9 +354,15 @@ abstract class DuplexConnection implements DatabaseListener {
 
 		public void run() {
 			try {
-				Request r = db.receiveOffer(contactId, offer);
-				if(LOG.isLoggable(INFO)) LOG.info("DB received offer");
-				writerTasks.add(new WriteRequest(r));
+				AckAndRequest ar = db.receiveOffer(contactId, offer);
+				Ack a = ar.getAck();
+				Request r = ar.getRequest();
+				if(LOG.isLoggable(INFO)) {
+					LOG.info("DB received offer: " + (a != null)
+							+ " " + (r != null));
+				}
+				if(a != null) writerTasks.add(new WriteAck(a));
+				if(r != null) writerTasks.add(new WriteRequest(r));
 			} catch(DbException e) {
 				if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
@@ -413,25 +390,6 @@ abstract class DuplexConnection implements DatabaseListener {
 		}
 	}
 
-	// This task runs on a database thread
-	private class SetSeen implements Runnable {
-
-		private final Collection<MessageId> seen;
-
-		private SetSeen(Collection<MessageId> seen) {
-			this.seen = seen;
-		}
-
-		public void run() {
-			try {
-				db.setSeen(contactId, seen);
-				if(LOG.isLoggable(INFO)) LOG.info("DB set seen");
-			} catch(DbException e) {
-				if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-			}
-		}
-	}
-
 	// This task runs on a database thread
 	private class ReceiveRetentionAck implements Runnable {
 
@@ -649,15 +607,8 @@ abstract class DuplexConnection implements DatabaseListener {
 				Offer o = db.generateOffer(contactId, maxMessages);
 				if(LOG.isLoggable(INFO))
 					LOG.info("Generated offer: " + (o != null));
-				if(o == null) {
-					// No messages to offer - wait for some to be added
-					canSendOffer.set(true);
-				} else {
-					// Store the offered message IDs
-					setOfferedMessageIds(o.getMessageIds());
-					// Write the offer on the writer thread
-					writerTasks.add(new WriteOffer(o));
-				}
+				if(o == null) canSendOffer.set(true);
+				else writerTasks.add(new WriteOffer(o));
 			} catch(DbException e) {
 				if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
 			}
diff --git a/briar-tests/build.xml b/briar-tests/build.xml
index c316b4c8d0..40ca67c1b5 100644
--- a/briar-tests/build.xml
+++ b/briar-tests/build.xml
@@ -107,7 +107,6 @@
 			<test name='net.sf.briar.messaging.ConstantsTest'/>
 			<test name='net.sf.briar.messaging.ConsumersTest'/>
 			<test name='net.sf.briar.messaging.PacketReaderImplTest'/>
-			<test name='net.sf.briar.messaging.PacketWriterImplTest'/>
 			<test name='net.sf.briar.messaging.simplex.OutgoingSimplexConnectionTest'/>
 			<test name='net.sf.briar.messaging.simplex.SimplexMessagingIntegrationTest'/>
 			<test name='net.sf.briar.plugins.PluginManagerImplTest'/>
diff --git a/briar-tests/src/net/sf/briar/ProtocolIntegrationTest.java b/briar-tests/src/net/sf/briar/ProtocolIntegrationTest.java
index ad98a62b18..2d4cbcb950 100644
--- a/briar-tests/src/net/sf/briar/ProtocolIntegrationTest.java
+++ b/briar-tests/src/net/sf/briar/ProtocolIntegrationTest.java
@@ -9,7 +9,6 @@ import java.io.ByteArrayOutputStream;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Arrays;
-import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Random;
@@ -138,9 +137,7 @@ public class ProtocolIntegrationTest extends BriarTestCase {
 
 		writer.writeOffer(new Offer(messageIds));
 
-		BitSet requested = new BitSet(2);
-		requested.set(1);
-		writer.writeRequest(new Request(requested, 2));
+		writer.writeRequest(new Request(messageIds));
 
 		SubscriptionUpdate su = new SubscriptionUpdate(Arrays.asList(group), 1);
 		writer.writeSubscriptionUpdate(su);
@@ -187,11 +184,7 @@ public class ProtocolIntegrationTest extends BriarTestCase {
 		// Read the request
 		assertTrue(reader.hasRequest());
 		Request req = reader.readRequest();
-		BitSet requested = req.getBitmap();
-		assertFalse(requested.get(0));
-		assertTrue(requested.get(1));
-		// If there are any padding bits, they should all be zero
-		assertEquals(1, requested.cardinality());
+		assertEquals(messageIds, req.getMessageIds());
 
 		// Read the subscription update
 		assertTrue(reader.hasSubscriptionUpdate());
diff --git a/briar-tests/src/net/sf/briar/db/DatabaseComponentTest.java b/briar-tests/src/net/sf/briar/db/DatabaseComponentTest.java
index 34b9c6e6f1..a35816527e 100644
--- a/briar-tests/src/net/sf/briar/db/DatabaseComponentTest.java
+++ b/briar-tests/src/net/sf/briar/db/DatabaseComponentTest.java
@@ -22,6 +22,7 @@ import net.sf.briar.api.LocalAuthor;
 import net.sf.briar.api.TransportConfig;
 import net.sf.briar.api.TransportId;
 import net.sf.briar.api.TransportProperties;
+import net.sf.briar.api.db.AckAndRequest;
 import net.sf.briar.api.db.DatabaseComponent;
 import net.sf.briar.api.db.NoSuchContactException;
 import net.sf.briar.api.db.NoSuchSubscriptionException;
@@ -1077,7 +1078,7 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 			oneOf(database).setStatusSeenIfVisible(txn, contactId, messageId);
 			will(returnValue(false)); // Not visible - request message # 0
 			oneOf(database).setStatusSeenIfVisible(txn, contactId, messageId1);
-			will(returnValue(true)); // Visible - do not request message # 1
+			will(returnValue(true)); // Visible - ack message # 1
 			oneOf(database).setStatusSeenIfVisible(txn, contactId, messageId2);
 			will(returnValue(false)); // Not visible - request message # 2
 		}});
@@ -1085,9 +1086,13 @@ public abstract class DatabaseComponentTest extends BriarTestCase {
 				shutdown);
 
 		Offer o = new Offer(Arrays.asList(messageId, messageId1, messageId2));
-		Request r = db.receiveOffer(contactId, o);
-		assertEquals(expectedRequest, r.getBitmap());
-		assertEquals(3, r.getLength());
+		AckAndRequest ar = db.receiveOffer(contactId, o);
+		Ack a = ar.getAck();
+		assertNotNull(a);
+		assertEquals(Arrays.asList(messageId1), a.getMessageIds());
+		Request r = ar.getRequest();
+		assertNotNull(r);
+		assertEquals(Arrays.asList(messageId, messageId2), r.getMessageIds());
 
 		context.assertIsSatisfied();
 	}
diff --git a/briar-tests/src/net/sf/briar/messaging/PacketReaderImplTest.java b/briar-tests/src/net/sf/briar/messaging/PacketReaderImplTest.java
index 43a2f4427c..982391208e 100644
--- a/briar-tests/src/net/sf/briar/messaging/PacketReaderImplTest.java
+++ b/briar-tests/src/net/sf/briar/messaging/PacketReaderImplTest.java
@@ -7,12 +7,10 @@ import static net.sf.briar.api.messaging.Types.REQUEST;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
-import java.util.BitSet;
 
 import net.sf.briar.BriarTestCase;
 import net.sf.briar.TestUtils;
 import net.sf.briar.api.FormatException;
-import net.sf.briar.api.messaging.Request;
 import net.sf.briar.api.serial.ReaderFactory;
 import net.sf.briar.api.serial.SerialComponent;
 import net.sf.briar.api.serial.Writer;
@@ -127,39 +125,15 @@ public class PacketReaderImplTest extends BriarTestCase {
 	}
 
 	@Test
-	public void testBitmapDecoding() throws Exception {
-		// Test sizes from 0 to 1000 bits
-		for(int i = 0; i < 1000; i++) {
-			// Create a BitSet of size i with one in ten bits set (on average)
-			BitSet requested = new BitSet(i);
-			for(int j = 0; j < i; j++) if(Math.random() < 0.1) requested.set(j);
-			// Encode the BitSet as a bitmap
-			int bytes = i % 8 == 0 ? i / 8 : i / 8 + 1;
-			byte[] bitmap = new byte[bytes];
-			for(int j = 0; j < i; j++) {
-				if(requested.get(j)) {
-					int offset = j / 8;
-					byte bit = (byte) (128 >> j % 8);
-					bitmap[offset] |= bit;
-				}
-			}
-			// Create a serialised request containing the bitmap
-			byte[] b = createRequest(bitmap);
-			// Deserialise the request
-			ByteArrayInputStream in = new ByteArrayInputStream(b);
-			PacketReaderImpl reader = new PacketReaderImpl(readerFactory,
-					null, null, in);
-			Request request = reader.readRequest();
-			BitSet decoded = request.getBitmap();
-			// Check that the decoded BitSet matches the original - we can't
-			// use equals() because of padding, but the first i bits should
-			// match and the cardinalities should be equal, indicating that no
-			// padding bits are set
-			for(int j = 0; j < i; j++) {
-				assertEquals(requested.get(j), decoded.get(j));
-			}
-			assertEquals(requested.cardinality(), decoded.cardinality());
-		}
+	public void testEmptyRequest() throws Exception {
+		byte[] b = createEmptyRequest();
+		ByteArrayInputStream in = new ByteArrayInputStream(b);
+		PacketReaderImpl reader = new PacketReaderImpl(readerFactory, null,
+				null, in);
+		try {
+			reader.readRequest();
+			fail();
+		} catch(FormatException expected) {}
 	}
 
 	private byte[] createAck(boolean tooBig) throws Exception {
@@ -222,26 +196,26 @@ public class PacketReaderImplTest extends BriarTestCase {
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		Writer w = writerFactory.createWriter(out);
 		w.writeStructStart(REQUEST);
-		// Allow one byte for the STRUCT tag, one byte for the struct ID,
-		// one byte for the padding length as a uint7, one byte for the BYTES
-		// tag, five bytes for the length of the byte array as an int32, and
-		// one byte for the END tag
-		int size = MAX_PACKET_LENGTH - 10;
-		if(tooBig) size++;
-		assertTrue(size > Short.MAX_VALUE);
-		w.writeUint7((byte) 0);
-		w.writeBytes(new byte[size]);
+		w.writeListStart();
+		while(out.size() + serial.getSerialisedUniqueIdLength()
+				+ serial.getSerialisedListEndLength()
+				+ serial.getSerialisedStructEndLength()
+				< MAX_PACKET_LENGTH) {
+			w.writeBytes(TestUtils.getRandomId());
+		}
+		if(tooBig) w.writeBytes(TestUtils.getRandomId());
+		w.writeListEnd();
 		w.writeStructEnd();
 		assertEquals(tooBig, out.size() > MAX_PACKET_LENGTH);
 		return out.toByteArray();
 	}
 
-	private byte[] createRequest(byte[] bitmap) throws Exception {
+	private byte[] createEmptyRequest() throws Exception {
 		ByteArrayOutputStream out = new ByteArrayOutputStream();
 		Writer w = writerFactory.createWriter(out);
 		w.writeStructStart(REQUEST);
-		w.writeUint7((byte) 0);
-		w.writeBytes(bitmap);
+		w.writeListStart();
+		w.writeListEnd();
 		w.writeStructEnd();
 		return out.toByteArray();
 	}
diff --git a/briar-tests/src/net/sf/briar/messaging/PacketWriterImplTest.java b/briar-tests/src/net/sf/briar/messaging/PacketWriterImplTest.java
deleted file mode 100644
index 197bed53e7..0000000000
--- a/briar-tests/src/net/sf/briar/messaging/PacketWriterImplTest.java
+++ /dev/null
@@ -1,93 +0,0 @@
-package net.sf.briar.messaging;
-
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.util.BitSet;
-
-import net.sf.briar.BriarTestCase;
-import net.sf.briar.TestDatabaseModule;
-import net.sf.briar.TestLifecycleModule;
-import net.sf.briar.api.messaging.PacketWriter;
-import net.sf.briar.api.messaging.Request;
-import net.sf.briar.api.serial.SerialComponent;
-import net.sf.briar.api.serial.WriterFactory;
-import net.sf.briar.clock.ClockModule;
-import net.sf.briar.crypto.CryptoModule;
-import net.sf.briar.db.DatabaseModule;
-import net.sf.briar.messaging.duplex.DuplexMessagingModule;
-import net.sf.briar.messaging.simplex.SimplexMessagingModule;
-import net.sf.briar.serial.SerialModule;
-import net.sf.briar.transport.TransportModule;
-import net.sf.briar.util.StringUtils;
-
-import org.junit.Test;
-
-import com.google.inject.Guice;
-import com.google.inject.Injector;
-
-public class PacketWriterImplTest extends BriarTestCase {
-
-	// FIXME: This is an integration test, not a unit test
-
-	private final SerialComponent serial;
-	private final WriterFactory writerFactory;
-
-	public PacketWriterImplTest() {
-		Injector i = Guice.createInjector(new TestDatabaseModule(),
-				new TestLifecycleModule(), new ClockModule(),
-				new CryptoModule(), new DatabaseModule(), new MessagingModule(),
-				new DuplexMessagingModule(), new SimplexMessagingModule(),
-				new SerialModule(), new TransportModule());
-		serial = i.getInstance(SerialComponent.class);
-		writerFactory = i.getInstance(WriterFactory.class);
-	}
-
-	@Test
-	public void testWriteBitmapNoPadding() throws IOException {
-		ByteArrayOutputStream out = new ByteArrayOutputStream();
-		PacketWriter w = new PacketWriterImpl(serial, writerFactory, out,
-				true);
-		BitSet b = new BitSet();
-		// 11011001 = 0xD9
-		b.set(0);
-		b.set(1);
-		b.set(3);
-		b.set(4);
-		b.set(7);
-		// 01011001 = 0x59
-		b.set(9);
-		b.set(11);
-		b.set(12);
-		b.set(15);
-		w.writeRequest(new Request(b, 16));
-		// STRUCT tag, struct ID 5, 0 as uint7, BYTES tag, length 2 as uint7,
-		// bitmap 0xD959, END tag
-		byte[] output = out.toByteArray();
-		assertEquals("F3" + "05" + "00" + "F6" + "02" + "D959" + "F2",
-				StringUtils.toHexString(output));
-	}
-
-	@Test
-	public void testWriteBitmapWithPadding() throws IOException {
-		ByteArrayOutputStream out = new ByteArrayOutputStream();
-		PacketWriter w = new PacketWriterImpl(serial, writerFactory, out,
-				true);
-		BitSet b = new BitSet();
-		// 01011001 = 0x59
-		b.set(1);
-		b.set(3);
-		b.set(4);
-		b.set(7);
-		// 11011xxx = 0xD8, after padding
-		b.set(8);
-		b.set(9);
-		b.set(11);
-		b.set(12);
-		w.writeRequest(new Request(b, 13));
-		// STRUCT tag, struct ID 5, 3 as uint7, BYTES tag, length 2 as uint7,
-		// bitmap 0x59D8, END tag
-		byte[] output = out.toByteArray();
-		assertEquals("F3" + "05" + "03" + "F6" + "02" + "59D8" + "F2",
-				StringUtils.toHexString(output));
-	}
-}
-- 
GitLab