From 495baf8c7095d915f920b626c77843501706a884 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Wed, 7 Dec 2011 00:38:14 +0000
Subject: [PATCH] BATCH_ID and MESSAGE_ID don't need to be structs.

---
 api/net/sf/briar/api/protocol/Types.java      | 17 ++++------
 .../sf/briar/api/serial/SerialComponent.java  |  2 +-
 .../net/sf/briar/protocol/AckReader.java      | 33 +++++++++----------
 .../sf/briar/protocol/MessageIdReader.java    | 20 -----------
 .../net/sf/briar/protocol/MessageReader.java  | 15 ++++-----
 .../net/sf/briar/protocol/OfferReader.java    | 27 ++++++++++-----
 .../net/sf/briar/protocol/ProtocolModule.java | 14 ++------
 .../sf/briar/protocol/ProtocolWriterImpl.java | 14 +++-----
 .../sf/briar/serial/SerialComponentImpl.java  |  8 ++---
 test/net/sf/briar/protocol/AckReaderTest.java |  2 --
 .../protocol/ProtocolWriterImplTest.java      |  8 ++---
 11 files changed, 65 insertions(+), 95 deletions(-)
 delete mode 100644 components/net/sf/briar/protocol/MessageIdReader.java

diff --git a/api/net/sf/briar/api/protocol/Types.java b/api/net/sf/briar/api/protocol/Types.java
index a3a5919d41..33749c45f2 100644
--- a/api/net/sf/briar/api/protocol/Types.java
+++ b/api/net/sf/briar/api/protocol/Types.java
@@ -3,17 +3,14 @@ package net.sf.briar.api.protocol;
 /** Struct identifiers for encoding and decoding protocol objects. */
 public interface Types {
 
-	// FIXME: Batch ID, message ID don't need to be structs
 	static final int ACK = 0;
 	static final int AUTHOR = 1;
 	static final int BATCH = 2;
-	static final int BATCH_ID = 3;
-	static final int GROUP = 4;
-	static final int MESSAGE = 5;
-	static final int MESSAGE_ID = 6;
-	static final int OFFER = 7;
-	static final int REQUEST = 8;
-	static final int SUBSCRIPTION_UPDATE = 9;
-	static final int TRANSPORT = 10;
-	static final int TRANSPORT_UPDATE = 11;
+	static final int GROUP = 3;
+	static final int MESSAGE = 4;
+	static final int OFFER = 5;
+	static final int REQUEST = 6;
+	static final int SUBSCRIPTION_UPDATE = 7;
+	static final int TRANSPORT = 8;
+	static final int TRANSPORT_UPDATE = 9;
 }
diff --git a/api/net/sf/briar/api/serial/SerialComponent.java b/api/net/sf/briar/api/serial/SerialComponent.java
index f880083b9e..ae0c2f106a 100644
--- a/api/net/sf/briar/api/serial/SerialComponent.java
+++ b/api/net/sf/briar/api/serial/SerialComponent.java
@@ -8,5 +8,5 @@ public interface SerialComponent {
 
 	int getSerialisedStructIdLength(int id);
 
-	int getSerialisedUniqueIdLength(int id);
+	int getSerialisedUniqueIdLength();
 }
diff --git a/components/net/sf/briar/protocol/AckReader.java b/components/net/sf/briar/protocol/AckReader.java
index 51bc713fe7..f2c4a899f4 100644
--- a/components/net/sf/briar/protocol/AckReader.java
+++ b/components/net/sf/briar/protocol/AckReader.java
@@ -1,8 +1,12 @@
 package net.sf.briar.protocol;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
+import net.sf.briar.api.Bytes;
 import net.sf.briar.api.FormatException;
 import net.sf.briar.api.protocol.Ack;
 import net.sf.briar.api.protocol.BatchId;
@@ -18,35 +22,30 @@ import net.sf.briar.api.serial.Reader;
 class AckReader implements ObjectReader<Ack> {
 
 	private final PacketFactory packetFactory;
-	private final ObjectReader<BatchId> batchIdReader;
 
 	AckReader(PacketFactory packetFactory) {
 		this.packetFactory = packetFactory;
-		batchIdReader = new BatchIdReader();
 	}
 
 	public Ack readObject(Reader r) throws IOException {
 		// Initialise the consumer
 		Consumer counting =
 			new CountingConsumer(ProtocolConstants.MAX_PACKET_LENGTH);
-		// Read and digest the data
+		// Read the data
 		r.addConsumer(counting);
 		r.readStructId(Types.ACK);
-		r.addObjectReader(Types.BATCH_ID, batchIdReader);
-		Collection<BatchId> batches = r.readList(BatchId.class);
-		r.removeObjectReader(Types.BATCH_ID);
+		r.setMaxBytesLength(UniqueId.LENGTH);
+		Collection<Bytes> raw = r.readList(Bytes.class);
+		r.resetMaxBytesLength();
 		r.removeConsumer(counting);
-		// Build and return the ack
-		return packetFactory.createAck(batches);
-	}
-
-	private static class BatchIdReader implements ObjectReader<BatchId> {
-
-		public BatchId readObject(Reader r) throws IOException {
-			r.readStructId(Types.BATCH_ID);
-			byte[] b = r.readBytes(UniqueId.LENGTH);
-			if(b.length != UniqueId.LENGTH) throw new FormatException();
-			return new BatchId(b);
+		// Convert the byte arrays to batch IDs
+		List<BatchId> batches = new ArrayList<BatchId>();
+		for(Bytes b : raw) {
+			if(b.getBytes().length != UniqueId.LENGTH)
+				throw new FormatException();
+			batches.add(new BatchId(b.getBytes()));
 		}
+		// Build and return the ack
+		return packetFactory.createAck(Collections.unmodifiableList(batches));
 	}
 }
diff --git a/components/net/sf/briar/protocol/MessageIdReader.java b/components/net/sf/briar/protocol/MessageIdReader.java
deleted file mode 100644
index 7204af2e74..0000000000
--- a/components/net/sf/briar/protocol/MessageIdReader.java
+++ /dev/null
@@ -1,20 +0,0 @@
-package net.sf.briar.protocol;
-
-import java.io.IOException;
-
-import net.sf.briar.api.FormatException;
-import net.sf.briar.api.protocol.MessageId;
-import net.sf.briar.api.protocol.Types;
-import net.sf.briar.api.protocol.UniqueId;
-import net.sf.briar.api.serial.ObjectReader;
-import net.sf.briar.api.serial.Reader;
-
-class MessageIdReader implements ObjectReader<MessageId> {
-
-	public MessageId readObject(Reader r) throws IOException {
-		r.readStructId(Types.MESSAGE_ID);
-		byte[] b = r.readBytes(UniqueId.LENGTH);
-		if(b.length != UniqueId.LENGTH) throw new FormatException();
-		return new MessageId(b);
-	}
-}
diff --git a/components/net/sf/briar/protocol/MessageReader.java b/components/net/sf/briar/protocol/MessageReader.java
index 0409daf0b0..4c7b8a1a6d 100644
--- a/components/net/sf/briar/protocol/MessageReader.java
+++ b/components/net/sf/briar/protocol/MessageReader.java
@@ -8,6 +8,7 @@ import net.sf.briar.api.protocol.Group;
 import net.sf.briar.api.protocol.MessageId;
 import net.sf.briar.api.protocol.ProtocolConstants;
 import net.sf.briar.api.protocol.Types;
+import net.sf.briar.api.protocol.UniqueId;
 import net.sf.briar.api.serial.CopyingConsumer;
 import net.sf.briar.api.serial.CountingConsumer;
 import net.sf.briar.api.serial.ObjectReader;
@@ -15,14 +16,11 @@ import net.sf.briar.api.serial.Reader;
 
 class MessageReader implements ObjectReader<UnverifiedMessage> {
 
-	private final ObjectReader<MessageId> messageIdReader;
 	private final ObjectReader<Group> groupReader;
 	private final ObjectReader<Author> authorReader;
 
-	MessageReader(ObjectReader<MessageId> messageIdReader,
-			ObjectReader<Group> groupReader,
+	MessageReader(ObjectReader<Group> groupReader,
 			ObjectReader<Author> authorReader) {
-		this.messageIdReader = messageIdReader;
 		this.groupReader = groupReader;
 		this.authorReader = authorReader;
 	}
@@ -40,9 +38,9 @@ class MessageReader implements ObjectReader<UnverifiedMessage> {
 		if(r.hasNull()) {
 			r.readNull();
 		} else {
-			r.addObjectReader(Types.MESSAGE_ID, messageIdReader);
-			parent = r.readStruct(Types.MESSAGE_ID, MessageId.class);
-			r.removeObjectReader(Types.MESSAGE_ID);
+			byte[] b = r.readBytes(UniqueId.LENGTH);
+			if(b.length != UniqueId.LENGTH) throw new FormatException();
+			parent = new MessageId(b);
 		}
 		// Read the group, if there is one
 		Group group = null;
@@ -69,7 +67,8 @@ class MessageReader implements ObjectReader<UnverifiedMessage> {
 		if(timestamp < 0L) throw new FormatException();
 		// Read the salt
 		byte[] salt = r.readBytes(ProtocolConstants.SALT_LENGTH);
-		if(salt.length != ProtocolConstants.SALT_LENGTH) throw new FormatException();
+		if(salt.length != ProtocolConstants.SALT_LENGTH)
+			throw new FormatException();
 		// Read the message body
 		byte[] body = r.readBytes(ProtocolConstants.MAX_BODY_LENGTH);
 		// Record the offset of the body within the message
diff --git a/components/net/sf/briar/protocol/OfferReader.java b/components/net/sf/briar/protocol/OfferReader.java
index a476886df7..f87247dda4 100644
--- a/components/net/sf/briar/protocol/OfferReader.java
+++ b/components/net/sf/briar/protocol/OfferReader.java
@@ -1,13 +1,19 @@
 package net.sf.briar.protocol;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
+import net.sf.briar.api.Bytes;
+import net.sf.briar.api.FormatException;
 import net.sf.briar.api.protocol.MessageId;
 import net.sf.briar.api.protocol.Offer;
 import net.sf.briar.api.protocol.PacketFactory;
 import net.sf.briar.api.protocol.ProtocolConstants;
 import net.sf.briar.api.protocol.Types;
+import net.sf.briar.api.protocol.UniqueId;
 import net.sf.briar.api.serial.Consumer;
 import net.sf.briar.api.serial.CountingConsumer;
 import net.sf.briar.api.serial.ObjectReader;
@@ -15,12 +21,9 @@ import net.sf.briar.api.serial.Reader;
 
 class OfferReader implements ObjectReader<Offer> {
 
-	private final ObjectReader<MessageId> messageIdReader;
 	private final PacketFactory packetFactory;
 
-	OfferReader(ObjectReader<MessageId> messageIdReader,
-			PacketFactory packetFactory) {
-		this.messageIdReader = messageIdReader;
+	OfferReader(PacketFactory packetFactory) {
 		this.packetFactory = packetFactory;
 	}
 
@@ -31,11 +34,19 @@ class OfferReader implements ObjectReader<Offer> {
 		// Read the data
 		r.addConsumer(counting);
 		r.readStructId(Types.OFFER);
-		r.addObjectReader(Types.MESSAGE_ID, messageIdReader);
-		Collection<MessageId> messages = r.readList(MessageId.class);
-		r.removeObjectReader(Types.MESSAGE_ID);
+		r.setMaxBytesLength(UniqueId.LENGTH);
+		Collection<Bytes> raw = r.readList(Bytes.class);
+		r.resetMaxBytesLength();
 		r.removeConsumer(counting);
+		// Convert the byte arrays to message IDs
+		List<MessageId> messages = new ArrayList<MessageId>();
+		for(Bytes b : raw) {
+			if(b.getBytes().length != UniqueId.LENGTH)
+				throw new FormatException();
+			messages.add(new MessageId(b.getBytes()));
+		}
 		// Build and return the offer
-		return packetFactory.createOffer(messages);
+		return packetFactory.createOffer(Collections.unmodifiableList(
+				messages));
 	}
 }
diff --git a/components/net/sf/briar/protocol/ProtocolModule.java b/components/net/sf/briar/protocol/ProtocolModule.java
index 9527f4d86d..cf83575c93 100644
--- a/components/net/sf/briar/protocol/ProtocolModule.java
+++ b/components/net/sf/briar/protocol/ProtocolModule.java
@@ -7,7 +7,6 @@ import net.sf.briar.api.protocol.AuthorFactory;
 import net.sf.briar.api.protocol.Group;
 import net.sf.briar.api.protocol.GroupFactory;
 import net.sf.briar.api.protocol.MessageFactory;
-import net.sf.briar.api.protocol.MessageId;
 import net.sf.briar.api.protocol.Offer;
 import net.sf.briar.api.protocol.PacketFactory;
 import net.sf.briar.api.protocol.ProtocolReaderFactory;
@@ -58,23 +57,16 @@ public class ProtocolModule extends AbstractModule {
 		return new GroupReader(crypto, groupFactory);
 	}
 
-	@Provides
-	ObjectReader<MessageId> getMessageIdReader() {
-		return new MessageIdReader();
-	}
-
 	@Provides
 	ObjectReader<UnverifiedMessage> getMessageReader(
-			ObjectReader<MessageId> messageIdReader,
 			ObjectReader<Group> groupReader,
 			ObjectReader<Author> authorReader) {
-		return new MessageReader(messageIdReader, groupReader, authorReader);
+		return new MessageReader(groupReader, authorReader);
 	}
 
 	@Provides
-	ObjectReader<Offer> getOfferReader(ObjectReader<MessageId> messageIdReader,
-			PacketFactory packetFactory) {
-		return new OfferReader(messageIdReader, packetFactory);
+	ObjectReader<Offer> getOfferReader(PacketFactory packetFactory) {
+		return new OfferReader(packetFactory);
 	}
 
 	@Provides
diff --git a/components/net/sf/briar/protocol/ProtocolWriterImpl.java b/components/net/sf/briar/protocol/ProtocolWriterImpl.java
index 9cff78109d..4dc1d44ece 100644
--- a/components/net/sf/briar/protocol/ProtocolWriterImpl.java
+++ b/components/net/sf/briar/protocol/ProtocolWriterImpl.java
@@ -41,7 +41,7 @@ class ProtocolWriterImpl implements ProtocolWriter {
 		int overhead = serial.getSerialisedStructIdLength(Types.ACK)
 		+ serial.getSerialisedListStartLength()
 		+ serial.getSerialisedListEndLength();
-		int idLength = serial.getSerialisedUniqueIdLength(Types.BATCH_ID);
+		int idLength = serial.getSerialisedUniqueIdLength();
 		return (packet - overhead) / idLength;
 	}
 
@@ -50,7 +50,7 @@ class ProtocolWriterImpl implements ProtocolWriter {
 		int overhead = serial.getSerialisedStructIdLength(Types.OFFER)
 		+ serial.getSerialisedListStartLength()
 		+ serial.getSerialisedListEndLength();
-		int idLength = serial.getSerialisedUniqueIdLength(Types.MESSAGE_ID);
+		int idLength = serial.getSerialisedUniqueIdLength();
 		return (packet - overhead) / idLength;
 	}
 
@@ -65,10 +65,7 @@ class ProtocolWriterImpl implements ProtocolWriter {
 	public void writeAck(Ack a) throws IOException {
 		w.writeStructId(Types.ACK);
 		w.writeListStart();
-		for(BatchId b : a.getBatchIds()) {
-			w.writeStructId(Types.BATCH_ID);
-			w.writeBytes(b.getBytes());
-		}
+		for(BatchId b : a.getBatchIds()) w.writeBytes(b.getBytes());
 		w.writeListEnd();
 	}
 
@@ -82,10 +79,7 @@ class ProtocolWriterImpl implements ProtocolWriter {
 	public void writeOffer(Offer o) throws IOException {
 		w.writeStructId(Types.OFFER);
 		w.writeListStart();
-		for(MessageId m : o.getMessageIds()) {
-			w.writeStructId(Types.MESSAGE_ID);
-			w.writeBytes(m.getBytes());
-		}
+		for(MessageId m : o.getMessageIds()) w.writeBytes(m.getBytes());
 		w.writeListEnd();
 	}
 
diff --git a/components/net/sf/briar/serial/SerialComponentImpl.java b/components/net/sf/briar/serial/SerialComponentImpl.java
index 934e4b071f..67c898272b 100644
--- a/components/net/sf/briar/serial/SerialComponentImpl.java
+++ b/components/net/sf/briar/serial/SerialComponentImpl.java
@@ -20,10 +20,10 @@ class SerialComponentImpl implements SerialComponent {
 		return id < 32 ? 1 : 2;
 	}
 
-	public int getSerialisedUniqueIdLength(int id) {
-		// Struct ID, BYTES tag, length spec, bytes
-		return getSerialisedStructIdLength(id) + 1
-		+ getSerialisedLengthSpecLength(UniqueId.LENGTH) + UniqueId.LENGTH;
+	public int getSerialisedUniqueIdLength() {
+		// BYTES tag, length spec, bytes
+		return 1 + getSerialisedLengthSpecLength(UniqueId.LENGTH)
+		+ UniqueId.LENGTH;
 	}
 
 	private int getSerialisedLengthSpecLength(int length) {
diff --git a/test/net/sf/briar/protocol/AckReaderTest.java b/test/net/sf/briar/protocol/AckReaderTest.java
index 54017dff93..3ff8ab8e0d 100644
--- a/test/net/sf/briar/protocol/AckReaderTest.java
+++ b/test/net/sf/briar/protocol/AckReaderTest.java
@@ -107,12 +107,10 @@ public class AckReaderTest extends TestCase {
 		Random random = new Random();
 		while(out.size() + BatchId.LENGTH + 3
 				< ProtocolConstants.MAX_PACKET_LENGTH) {
-			w.writeStructId(Types.BATCH_ID);
 			random.nextBytes(b);
 			w.writeBytes(b);
 		}
 		if(tooBig) {
-			w.writeStructId(Types.BATCH_ID);
 			random.nextBytes(b);
 			w.writeBytes(b);
 		}
diff --git a/test/net/sf/briar/protocol/ProtocolWriterImplTest.java b/test/net/sf/briar/protocol/ProtocolWriterImplTest.java
index 07fe5377ba..c558bda32e 100644
--- a/test/net/sf/briar/protocol/ProtocolWriterImplTest.java
+++ b/test/net/sf/briar/protocol/ProtocolWriterImplTest.java
@@ -52,9 +52,9 @@ public class ProtocolWriterImplTest extends TestCase {
 		b.set(15);
 		Request r = packetFactory.createRequest(b, 16);
 		w.writeRequest(r);
-		// Short user tag 8, 0 as uint7, short bytes with length 2, 0xD959
+		// Short user tag 6, 0 as uint7, short bytes with length 2, 0xD959
 		byte[] output = out.toByteArray();
-		assertEquals("C8" + "00" + "92" + "D959",
+		assertEquals("C6" + "00" + "92" + "D959",
 				StringUtils.toHexString(output));
 	}
 
@@ -75,9 +75,9 @@ public class ProtocolWriterImplTest extends TestCase {
 		b.set(12);
 		Request r = packetFactory.createRequest(b, 13);
 		w.writeRequest(r);
-		// Short user tag 8, 3 as uint7, short bytes with length 2, 0x59D8
+		// Short user tag 6, 3 as uint7, short bytes with length 2, 0x59D8
 		byte[] output = out.toByteArray();
-		assertEquals("C8" + "03" + "92" + "59D8",
+		assertEquals("C6" + "03" + "92" + "59D8",
 				StringUtils.toHexString(output));
 	}
 }
-- 
GitLab