Implemented OfferWriter and RequestWriter, made all the writers

reusable (though not thread-safe), and guiced the readers.

api/net/sf/briar/api/protocol/Tags.java

@@ -16,6 +16,8 @@ public interface Tags {
 	static final int GROUP_ID = 6;
 	static final int MESSAGE = 7;
 	static final int MESSAGE_ID = 8;
-	static final int SUBSCRIPTIONS = 9;
-	static final int TRANSPORTS = 10;
+	static final int OFFER = 9;
+	static final int REQUEST = 10;
+	static final int SUBSCRIPTIONS = 11;
+	static final int TRANSPORTS = 12;
 }

api/net/sf/briar/api/protocol/writers/PacketWriterFactory.java

@@ -8,6 +8,10 @@ public interface PacketWriterFactory {
 
 	BatchWriter createBatchWriter(OutputStream out);
 
+	OfferWriter createOfferWriter(OutputStream out);
+
+	RequestWriter createRequestWriter(OutputStream out);
+
 	SubscriptionWriter createSubscriptionWriter(OutputStream out);
 
 	TransportWriter createTransportWriter(OutputStream out);

api/net/sf/briar/api/protocol/writers/RequestWriter.java

@@ -7,5 +7,5 @@ import java.util.BitSet;
 public interface RequestWriter {
 
 	/** Writes the contents of the request. */
-	void writeBitmap(BitSet b) throws IOException;
+	void writeBitmap(BitSet b, int length) throws IOException;
 }

components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java

@@ -676,11 +676,10 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 				try {
 					subscriptionLock.readLock().lock();
 					try {
-						BitSet request;
+						Collection<MessageId> offered = o.getMessages();
+						BitSet request = new BitSet(offered.size());
 						Txn txn = db.startTransaction();
 						try {
-							Collection<MessageId> offered = o.getMessages();
-							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 if
@@ -694,7 +693,7 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 							db.abortTransaction(txn);
 							throw e;
 						}
-						r.writeBitmap(request);
+						r.writeBitmap(request, offered.size());
 					} finally {
 						subscriptionLock.readLock().unlock();
 					}

components/net/sf/briar/db/SynchronizedDatabaseComponent.java

@@ -497,11 +497,10 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 			synchronized(messageLock) {
 				synchronized(messageStatusLock) {
 					synchronized(subscriptionLock) {
-						BitSet request;
+						Collection<MessageId> offered = o.getMessages();
+						BitSet request = new BitSet(offered.size());
 						Txn txn = db.startTransaction();
 						try {
-							Collection<MessageId> offered = o.getMessages();
-							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 if
@@ -515,7 +514,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
 							db.abortTransaction(txn);
 							throw e;
 						}
-						r.writeBitmap(request);
+						r.writeBitmap(request, offered.size());
 					}
 				}
 			}

components/net/sf/briar/protocol/AckReader.java

@@ -9,11 +9,14 @@ import net.sf.briar.api.protocol.Tags;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class AckReader implements ObjectReader<Ack> {
 
 	private final ObjectReader<BatchId> batchIdReader;
 	private final AckFactory ackFactory;
 
+	@Inject
 	AckReader(ObjectReader<BatchId> batchIdReader, AckFactory ackFactory) {
 		this.batchIdReader = batchIdReader;
 		this.ackFactory = ackFactory;

components/net/sf/briar/protocol/AuthorReader.java

@@ -11,11 +11,14 @@ import net.sf.briar.api.protocol.Tags;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class AuthorReader implements ObjectReader<Author> {
 
 	private final MessageDigest messageDigest;
 	private final AuthorFactory authorFactory;
 
+	@Inject
 	AuthorReader(CryptoComponent crypto, AuthorFactory authorFactory) {
 		messageDigest = crypto.getMessageDigest();
 		this.authorFactory = authorFactory;

components/net/sf/briar/protocol/BatchReader.java

@@ -12,12 +12,15 @@ import net.sf.briar.api.protocol.Tags;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class BatchReader implements ObjectReader<Batch> {
 
 	private final MessageDigest messageDigest;
 	private final ObjectReader<Message> messageReader;
 	private final BatchFactory batchFactory;
 
+	@Inject
 	BatchReader(CryptoComponent crypto, ObjectReader<Message> messageReader,
 			BatchFactory batchFactory) {
 		messageDigest = crypto.getMessageDigest();

components/net/sf/briar/protocol/GroupReader.java

@@ -11,11 +11,14 @@ import net.sf.briar.api.protocol.Tags;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class GroupReader implements ObjectReader<Group> {
 
 	private final MessageDigest messageDigest;
 	private final GroupFactory groupFactory;
 
+	@Inject
 	GroupReader(CryptoComponent crypto, GroupFactory groupFactory) {
 		messageDigest = crypto.getMessageDigest();
 		this.groupFactory = groupFactory;

components/net/sf/briar/protocol/MessageReader.java

@@ -19,6 +19,8 @@ import net.sf.briar.api.serial.FormatException;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class MessageReader implements ObjectReader<Message> {
 
 	private final ObjectReader<Group> groupReader;
@@ -27,6 +29,7 @@ class MessageReader implements ObjectReader<Message> {
 	private final Signature signature;
 	private final MessageDigest messageDigest;
 
+	@Inject
 	MessageReader(CryptoComponent crypto, ObjectReader<Group> groupReader,
 			ObjectReader<Author> authorReader) {
 		this.groupReader = groupReader;

components/net/sf/briar/protocol/ProtocolModule.java

@@ -1,10 +1,17 @@
 package net.sf.briar.protocol;
 
+import net.sf.briar.api.crypto.CryptoComponent;
+import net.sf.briar.api.protocol.Author;
 import net.sf.briar.api.protocol.AuthorFactory;
+import net.sf.briar.api.protocol.BatchId;
+import net.sf.briar.api.protocol.Group;
 import net.sf.briar.api.protocol.GroupFactory;
+import net.sf.briar.api.protocol.Message;
 import net.sf.briar.api.protocol.MessageEncoder;
+import net.sf.briar.api.serial.ObjectReader;
 
 import com.google.inject.AbstractModule;
+import com.google.inject.Provides;
 
 public class ProtocolModule extends AbstractModule {
 
@@ -14,6 +21,32 @@ public class ProtocolModule extends AbstractModule {
 		bind(AuthorFactory.class).to(AuthorFactoryImpl.class);
 		bind(BatchFactory.class).to(BatchFactoryImpl.class);
 		bind(GroupFactory.class).to(GroupFactoryImpl.class);
+		bind(SubscriptionFactory.class).to(SubscriptionFactoryImpl.class);
+		bind(TransportFactory.class).to(TransportFactoryImpl.class);
 		bind(MessageEncoder.class).to(MessageEncoderImpl.class);
 	}
+
+	@Provides
+	ObjectReader<BatchId> getBatchIdReader() {
+		return new BatchIdReader();
+	}
+
+	@Provides
+	ObjectReader<Group> getGroupReader(CryptoComponent crypto,
+			GroupFactory groupFactory) {
+		return new GroupReader(crypto, groupFactory);
+	}
+
+	@Provides
+	ObjectReader<Author> getAuthorReader(CryptoComponent crypto,
+			AuthorFactory authorFactory) {
+		return new AuthorReader(crypto, authorFactory);
+	}
+
+	@Provides
+	ObjectReader<Message> getMessageReader(CryptoComponent crypto,
+			ObjectReader<Group> groupReader,
+			ObjectReader<Author> authorReader) {
+		return new MessageReader(crypto, groupReader, authorReader);
+	}
 }

components/net/sf/briar/protocol/SubscriptionReader.java

@@ -9,11 +9,14 @@ import net.sf.briar.api.protocol.Tags;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class SubscriptionReader implements ObjectReader<Subscriptions> {
 
 	private final ObjectReader<Group> groupReader;
 	private final SubscriptionFactory subscriptionFactory;
 
+	@Inject
 	SubscriptionReader(ObjectReader<Group> groupReader,
 			SubscriptionFactory subscriptionFactory) {
 		this.groupReader = groupReader;

components/net/sf/briar/protocol/TransportReader.java

@@ -8,10 +8,13 @@ import net.sf.briar.api.protocol.Transports;
 import net.sf.briar.api.serial.ObjectReader;
 import net.sf.briar.api.serial.Reader;
 
+import com.google.inject.Inject;
+
 class TransportReader implements ObjectReader<Transports> {
 
 	private final TransportFactory transportFactory;
 
+	@Inject
 	TransportReader(TransportFactory transportFactory) {
 		this.transportFactory = transportFactory;
 	}

components/net/sf/briar/protocol/writers/AckWriterImpl.java

@@ -15,7 +15,7 @@ class AckWriterImpl implements AckWriter {
 	private final OutputStream out;
 	private final Writer w;
 
-	private boolean started = false, finished = false;
+	private boolean started = false;
 
 	AckWriterImpl(OutputStream out, WriterFactory writerFactory) {
 		this.out = out;
@@ -23,7 +23,6 @@ class AckWriterImpl implements AckWriter {
 	}
 
 	public boolean writeBatchId(BatchId b) throws IOException {
-		if(finished) throw new IllegalStateException();
 		if(!started) {
 			w.writeUserDefinedTag(Tags.ACK);
 			w.writeListStart();
@@ -36,7 +35,6 @@ class AckWriterImpl implements AckWriter {
 	}
 
 	public void finish() throws IOException {
-		if(finished) throw new IllegalStateException();
 		if(!started) {
 			w.writeUserDefinedTag(Tags.ACK);
 			w.writeListStart();
@@ -44,6 +42,6 @@ class AckWriterImpl implements AckWriter {
 		}
 		w.writeListEnd();
 		out.flush();
-		finished = true;
+		started = false;
 	}
 }

components/net/sf/briar/protocol/writers/BatchWriterImpl.java

@@ -18,7 +18,7 @@ class BatchWriterImpl implements BatchWriter {
 	private final Writer w;
 	private final MessageDigest messageDigest;
 
-	private boolean started = false, finished = false;
+	private boolean started = false;
 
 	BatchWriterImpl(OutputStream out, WriterFactory writerFactory,
 			MessageDigest messageDigest) {
@@ -32,7 +32,6 @@ class BatchWriterImpl implements BatchWriter {
 	}
 
 	public boolean writeMessage(byte[] message) throws IOException {
-		if(finished) throw new IllegalStateException();
 		if(!started) {
 			messageDigest.reset();
 			w.writeUserDefinedTag(Tags.BATCH);
@@ -47,7 +46,6 @@ class BatchWriterImpl implements BatchWriter {
 	}
 
 	public BatchId finish() throws IOException {
-		if(finished) throw new IllegalStateException();
 		if(!started) {
 			messageDigest.reset();
 			w.writeUserDefinedTag(Tags.BATCH);
@@ -56,7 +54,7 @@ class BatchWriterImpl implements BatchWriter {
 		}
 		w.writeListEnd();
 		out.flush();
-		finished = true;
+		started = false;
 		return new BatchId(messageDigest.digest());
 	}
 }

components/net/sf/briar/protocol/writers/OfferWriterImpl.java

@@ -0,0 +1,49 @@
+package net.sf.briar.protocol.writers;
+
+import java.io.IOException;
+import java.io.OutputStream;
+
+import net.sf.briar.api.protocol.MessageId;
+import net.sf.briar.api.protocol.Offer;
+import net.sf.briar.api.protocol.Tags;
+import net.sf.briar.api.protocol.writers.OfferWriter;
+import net.sf.briar.api.serial.Writer;
+import net.sf.briar.api.serial.WriterFactory;
+
+class OfferWriterImpl implements OfferWriter {
+
+	private final OutputStream out;
+	private final Writer w;
+
+	private boolean started = false, finished = false;
+
+	OfferWriterImpl(OutputStream out, WriterFactory writerFactory) {
+		this.out = out;
+		w = writerFactory.createWriter(out);
+	}
+
+	public boolean writeMessageId(MessageId m) throws IOException {
+		if(finished) throw new IllegalStateException();
+		if(!started) {
+			w.writeUserDefinedTag(Tags.OFFER);
+			w.writeListStart();
+			started = true;
+		}
+		int capacity = Offer.MAX_SIZE - (int) w.getBytesWritten() - 1;
+		if(capacity < MessageId.SERIALISED_LENGTH) return false;
+		m.writeTo(w);
+		return true;
+	}
+
+	public void finish() throws IOException {
+		if(finished) throw new IllegalStateException();
+		if(!started) {
+			w.writeUserDefinedTag(Tags.OFFER);
+			w.writeListStart();
+			started = true;
+		}
+		w.writeListEnd();
+		out.flush();
+		finished = true;
+	}
+}

components/net/sf/briar/protocol/writers/PacketWriterFactoryImpl.java

@@ -6,7 +6,9 @@ import java.security.MessageDigest;
 import net.sf.briar.api.crypto.CryptoComponent;
 import net.sf.briar.api.protocol.writers.AckWriter;
 import net.sf.briar.api.protocol.writers.BatchWriter;
+import net.sf.briar.api.protocol.writers.OfferWriter;
 import net.sf.briar.api.protocol.writers.PacketWriterFactory;
+import net.sf.briar.api.protocol.writers.RequestWriter;
 import net.sf.briar.api.protocol.writers.SubscriptionWriter;
 import net.sf.briar.api.protocol.writers.TransportWriter;
 import net.sf.briar.api.serial.WriterFactory;
@@ -33,6 +35,14 @@ class PacketWriterFactoryImpl implements PacketWriterFactory {
 		return new BatchWriterImpl(out, writerFactory, messageDigest);
 	}
 
+	public OfferWriter createOfferWriter(OutputStream out) {
+		return new OfferWriterImpl(out, writerFactory);
+	}
+
+	public RequestWriter createRequestWriter(OutputStream out) {
+		return new RequestWriterImpl(out, writerFactory);
+	}
+
 	public SubscriptionWriter createSubscriptionWriter(OutputStream out) {
 		return new SubscriptionWriterImpl(out, writerFactory);
 	}

components/net/sf/briar/protocol/writers/RequestWriterImpl.java

@@ -0,0 +1,38 @@
+package net.sf.briar.protocol.writers;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.BitSet;
+
+import net.sf.briar.api.protocol.Tags;
+import net.sf.briar.api.protocol.writers.RequestWriter;
+import net.sf.briar.api.serial.Writer;
+import net.sf.briar.api.serial.WriterFactory;
+
+class RequestWriterImpl implements RequestWriter {
+
+	private final OutputStream out;
+	private final Writer w;
+
+	RequestWriterImpl(OutputStream out, WriterFactory writerFactory) {
+		this.out = out;
+		w = writerFactory.createWriter(out);
+	}
+
+	public void writeBitmap(BitSet b, int length) throws IOException {
+		w.writeUserDefinedTag(Tags.REQUEST);
+		// 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.writeBytes(bitmap);
+		out.flush();
+	}
+}

components/net/sf/briar/protocol/writers/SubscriptionWriterImpl.java

@@ -15,19 +15,15 @@ class SubscriptionWriterImpl implements SubscriptionWriter {
 	private final OutputStream out;
 	private final Writer w;
 
-	private boolean used = false;
-
 	SubscriptionWriterImpl(OutputStream out, WriterFactory writerFactory) {
 		this.out = out;
 		w = writerFactory.createWriter(out);
 	}
 
 	public void writeSubscriptions(Collection<Group> subs) throws IOException {
-		if(used) throw new IllegalStateException();
 		w.writeUserDefinedTag(Tags.SUBSCRIPTIONS);
 		w.writeList(subs);
 		w.writeInt64(System.currentTimeMillis());
 		out.flush();
-		used = true;
 	}
 }

components/net/sf/briar/protocol/writers/TransportWriterImpl.java

@@ -14,8 +14,6 @@ class TransportWriterImpl implements TransportWriter {
 	private final OutputStream out;
 	private final Writer w;
 
-	private boolean used = false;
-
 	TransportWriterImpl(OutputStream out, WriterFactory writerFactory) {
 		this.out = out;
 		w = writerFactory.createWriter(out);
@@ -23,11 +21,9 @@ class TransportWriterImpl implements TransportWriter {
 
 	public void writeTransports(Map<String, String> transports)
 	throws IOException {
-		if(used) throw new IllegalStateException();
 		w.writeUserDefinedTag(Tags.TRANSPORTS);
 		w.writeMap(transports);
 		w.writeInt64(System.currentTimeMillis());
 		out.flush();
-		used = true;
 	}
 }

test/net/sf/briar/db/DatabaseComponentTest.java

@@ -948,7 +948,7 @@ public abstract class DatabaseComponentTest extends TestCase {
 			will(returnValue(true)); // Visible - do not request message # 1
 			oneOf(database).setStatusSeenIfVisible(txn, contactId, messageId2);
 			will(returnValue(false)); // Not visible - request message # 2
-			oneOf(requestWriter).writeBitmap(expectedRequest);
+			oneOf(requestWriter).writeBitmap(expectedRequest, 3);
 		}});
 		DatabaseComponent db = createDatabaseComponent(database, cleaner);
 

test/net/sf/briar/protocol/FileReadWriteTest.java

@@ -34,7 +34,6 @@ import net.sf.briar.api.protocol.writers.SubscriptionWriter;
 import net.sf.briar.api.protocol.writers.TransportWriter;
 import net.sf.briar.api.serial.Reader;
 import net.sf.briar.api.serial.ReaderFactory;
-import net.sf.briar.api.serial.WriterFactory;
 import net.sf.briar.crypto.CryptoModule;
 import net.sf.briar.protocol.writers.WritersModule;
 import net.sf.briar.serial.SerialModule;
@@ -55,9 +54,12 @@ public class FileReadWriteTest extends TestCase {
 	private final long start = System.currentTimeMillis();
 
 	private final ReaderFactory readerFactory;
-	private final WriterFactory writerFactory;
 	private final PacketWriterFactory packetWriterFactory;
 	private final CryptoComponent crypto;
+	private final AckReader ackReader;
+	private final BatchReader batchReader;
+	private final SubscriptionReader subscriptionReader;
+	private final TransportReader transportReader;
 	private final Author author;
 	private final Group group, group1;
 	private final Message message, message1, message2, message3;
@@ -70,11 +72,14 @@ public class FileReadWriteTest extends TestCase {
 				new ProtocolModule(), new SerialModule(),
 				new WritersModule());
 		readerFactory = i.getInstance(ReaderFactory.class);
-		writerFactory = i.getInstance(WriterFactory.class);
 		packetWriterFactory = i.getInstance(PacketWriterFactory.class);
 		crypto = i.getInstance(CryptoComponent.class);
 		assertEquals(crypto.getMessageDigest().getDigestLength(),
 				UniqueId.LENGTH);
+		ackReader = i.getInstance(AckReader.class);
+		batchReader = i.getInstance(BatchReader.class);
+		subscriptionReader = i.getInstance(SubscriptionReader.class);
+		transportReader = i.getInstance(TransportReader.class);
 		// Create two groups: one restricted, one unrestricted
 		GroupFactory groupFactory = i.getInstance(GroupFactory.class);
 		group = groupFactory.createGroup("Unrestricted group", null);
@@ -139,21 +144,6 @@ public class FileReadWriteTest extends TestCase {
 
 		testWriteFile();
 
-		GroupReader groupReader = new GroupReader(crypto,
-				new GroupFactoryImpl(crypto, writerFactory));
-		AuthorReader authorReader = new AuthorReader(crypto,
-				new AuthorFactoryImpl(crypto, writerFactory));
-		MessageReader messageReader = new MessageReader(crypto, groupReader,
-				authorReader);
-		AckReader ackReader = new AckReader(new BatchIdReader(),
-				new AckFactoryImpl());
-		BatchReader batchReader = new BatchReader(crypto, messageReader,
-				new BatchFactoryImpl());
-		SubscriptionReader subscriptionReader =
-			new SubscriptionReader(groupReader, new SubscriptionFactoryImpl());
-		TransportReader transportReader =
-			new TransportReader(new TransportFactoryImpl());
-
 		FileInputStream in = new FileInputStream(file);
 		Reader reader = readerFactory.createReader(in);
 		reader.addObjectReader(Tags.ACK, ackReader);

test/net/sf/briar/protocol/writers/RequestWriterImplTest.java

@@ -0,0 +1,70 @@
+package net.sf.briar.protocol.writers;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.BitSet;
+
+import junit.framework.TestCase;
+import net.sf.briar.api.protocol.writers.RequestWriter;
+import net.sf.briar.api.serial.WriterFactory;
+import net.sf.briar.serial.SerialModule;
+import net.sf.briar.util.StringUtils;
+
+import org.junit.Test;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+
+public class RequestWriterImplTest extends TestCase {
+
+	private final WriterFactory writerFactory;
+
+	public RequestWriterImplTest() {
+		super();
+		Injector i = Guice.createInjector(new SerialModule());
+		writerFactory = i.getInstance(WriterFactory.class);
+	}
+
+	@Test
+	public void testWriteBitmapNoPadding() throws IOException {
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		RequestWriter r = new RequestWriterImpl(out, writerFactory);
+		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);
+		r.writeBitmap(b, 16);
+		// Short user tag 10, short bytes with length 2, 0xD959
+		byte[] output = out.toByteArray();
+		assertEquals("CA" + "92" + "D959", StringUtils.toHexString(output));
+	}
+
+	@Test
+	public void testWriteBitmapWithPadding() throws IOException {
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		RequestWriter r = new RequestWriterImpl(out, writerFactory);
+		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);
+		r.writeBitmap(b, 13);
+		// Short user tag 10, short bytes with length 2, 0x59D8
+		byte[] output = out.toByteArray();
+		assertEquals("CA" + "92" + "59D8", StringUtils.toHexString(output));
+	}
+}