Merge branch 'parse-transport-properties' into 'master'

Helper method for parsing transport properties

See merge request akwizgran/briar!764

bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java

@@ -7,6 +7,7 @@ import org.briarproject.bramble.api.db.DbException;
 import org.briarproject.bramble.api.db.Transaction;
 import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageId;
@@ -103,4 +104,7 @@ public interface ClientHelper {
 			BdfList signed) throws FormatException, GeneralSecurityException;
 
 	Author parseAndValidateAuthor(BdfList author) throws FormatException;
+
+	TransportProperties parseAndValidateTransportProperties(
+			BdfDictionary properties) throws FormatException;
 }

bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java

@@ -18,6 +18,7 @@ import org.briarproject.bramble.api.db.Transaction;
 import org.briarproject.bramble.api.identity.Author;
 import org.briarproject.bramble.api.identity.AuthorFactory;
 import org.briarproject.bramble.api.nullsafety.NotNullByDefault;
+import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.api.sync.GroupId;
 import org.briarproject.bramble.api.sync.Message;
 import org.briarproject.bramble.api.sync.MessageFactory;
@@ -37,6 +38,8 @@ import javax.inject.Inject;
 import static org.briarproject.bramble.api.identity.Author.FORMAT_VERSION;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH;
 import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH;
+import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MAX_PROPERTIES_PER_TRANSPORT;
+import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MAX_PROPERTY_LENGTH;
 import static org.briarproject.bramble.api.sync.SyncConstants.MESSAGE_HEADER_LENGTH;
 import static org.briarproject.bramble.util.ValidationUtils.checkLength;
 import static org.briarproject.bramble.util.ValidationUtils.checkSize;
@@ -382,4 +385,18 @@ class ClientHelperImpl implements ClientHelper {
 		checkLength(publicKey, 1, MAX_PUBLIC_KEY_LENGTH);
 		return authorFactory.createAuthor(formatVersion, name, publicKey);
 	}
+
+	@Override
+	public TransportProperties parseAndValidateTransportProperties(
+			BdfDictionary properties) throws FormatException {
+		checkSize(properties, 0, MAX_PROPERTIES_PER_TRANSPORT);
+		TransportProperties p = new TransportProperties();
+		for (String key : properties.keySet()) {
+			checkLength(key, 1, MAX_PROPERTY_LENGTH);
+			String value = properties.getString(key);
+			checkLength(value, 1, MAX_PROPERTY_LENGTH);
+			p.put(key, value);
+		}
+		return p;
+	}
 }

bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java

@@ -347,10 +347,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 			throws FormatException {
 		// Transport ID, version, properties
 		BdfDictionary dictionary = message.getDictionary(2);
-		TransportProperties p = new TransportProperties();
-		for (String key : dictionary.keySet())
-			p.put(key, dictionary.getString(key));
-		return p;
+		return clientHelper.parseAndValidateTransportProperties(dictionary);
 	}
 
 	private static class LatestUpdate {

bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyValidator.java

@@ -15,8 +15,6 @@ import org.briarproject.bramble.api.system.Clock;
 import javax.annotation.concurrent.Immutable;
 
 import static org.briarproject.bramble.api.plugin.TransportId.MAX_TRANSPORT_ID_LENGTH;
-import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MAX_PROPERTIES_PER_TRANSPORT;
-import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MAX_PROPERTY_LENGTH;
 import static org.briarproject.bramble.util.ValidationUtils.checkLength;
 import static org.briarproject.bramble.util.ValidationUtils.checkSize;
 
@@ -42,12 +40,7 @@ class TransportPropertyValidator extends BdfMessageValidator {
 		if (version < 0) throw new FormatException();
 		// Properties
 		BdfDictionary dictionary = body.getDictionary(2);
-		checkSize(dictionary, 0, MAX_PROPERTIES_PER_TRANSPORT);
-		for (String key : dictionary.keySet()) {
-			checkLength(key, 0, MAX_PROPERTY_LENGTH);
-			String value = dictionary.getString(key);
-			checkLength(value, 0, MAX_PROPERTY_LENGTH);
-		}
+		clientHelper.parseAndValidateTransportProperties(dictionary);
 		// Return the metadata
 		BdfDictionary meta = new BdfDictionary();
 		meta.put("transportId", transportId);

bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java

@@ -400,6 +400,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			will(returnValue(messageMetadata));
 			oneOf(clientHelper).getMessageAsList(txn, fooUpdateId);
 			will(returnValue(fooUpdate));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					fooPropertiesDict);
+			will(returnValue(fooProperties));
 			oneOf(db).commitTransaction(txn);
 			oneOf(db).endTransaction(txn);
 		}});
@@ -467,6 +470,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			will(returnValue(messageMetadata3));
 			oneOf(clientHelper).getMessageAsList(txn, fooUpdateId);
 			will(returnValue(fooUpdate));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					fooPropertiesDict);
+			will(returnValue(fooProperties));
 			oneOf(db).commitTransaction(txn);
 			oneOf(db).endTransaction(txn);
 		}});
@@ -502,6 +508,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			will(returnValue(messageMetadata));
 			oneOf(clientHelper).getMessageAsList(txn, updateId);
 			will(returnValue(update));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					fooPropertiesDict);
+			will(returnValue(fooProperties));
 			// Properties are unchanged so we're done
 			oneOf(db).commitTransaction(txn);
 			oneOf(db).endTransaction(txn);
@@ -562,9 +571,12 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		MessageId contactGroupUpdateId = new MessageId(getRandomId());
 		Map<MessageId, BdfDictionary> contactGroupMessageMetadata =
 				Collections.singletonMap(contactGroupUpdateId, oldMetadata);
-		BdfList oldUpdate = BdfList.of("foo", 1, BdfDictionary.of(
+		TransportProperties oldProperties = new TransportProperties();
+		oldProperties.put("fooKey1", "oldFooValue1");
+		BdfDictionary oldPropertiesDict = BdfDictionary.of(
 				new BdfEntry("fooKey1", "oldFooValue1")
-		));
+		);
+		BdfList oldUpdate = BdfList.of("foo", 1, oldPropertiesDict);
 
 		context.checking(new Expectations() {{
 			oneOf(db).startTransaction(false);
@@ -575,6 +587,9 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			will(returnValue(localGroupMessageMetadata));
 			oneOf(clientHelper).getMessageAsList(txn, localGroupUpdateId);
 			will(returnValue(oldUpdate));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					oldPropertiesDict);
+			will(returnValue(oldProperties));
 			// Store the merged properties in the local group, version 2
 			expectStoreMessage(txn, localGroup.getId(), "foo",
 					fooPropertiesDict, 2, true, false);
@@ -638,8 +653,14 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			// Retrieve and parse the latest local properties
 			oneOf(clientHelper).getMessageAsList(txn, fooVersion999);
 			will(returnValue(fooUpdate));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					fooPropertiesDict);
+			will(returnValue(fooProperties));
 			oneOf(clientHelper).getMessageAsList(txn, barVersion3);
 			will(returnValue(barUpdate));
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					barPropertiesDict);
+			will(returnValue(barProperties));
 		}});
 	}
 

bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyValidatorTest.java

@@ -3,78 +3,78 @@ package org.briarproject.bramble.properties;
 import org.briarproject.bramble.api.FormatException;
 import org.briarproject.bramble.api.client.ClientHelper;
 import org.briarproject.bramble.api.data.BdfDictionary;
+import org.briarproject.bramble.api.data.BdfEntry;
 import org.briarproject.bramble.api.data.BdfList;
 import org.briarproject.bramble.api.data.MetadataEncoder;
 import org.briarproject.bramble.api.plugin.TransportId;
+import org.briarproject.bramble.api.properties.TransportProperties;
 import org.briarproject.bramble.api.sync.Group;
 import org.briarproject.bramble.api.sync.Message;
-import org.briarproject.bramble.api.sync.MessageId;
 import org.briarproject.bramble.api.system.Clock;
-import org.briarproject.bramble.test.BrambleTestCase;
-import org.briarproject.bramble.util.StringUtils;
-import org.jmock.Mockery;
+import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.jmock.Expectations;
 import org.junit.Test;
 
 import java.io.IOException;
 
 import static org.briarproject.bramble.api.plugin.TransportId.MAX_TRANSPORT_ID_LENGTH;
-import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MAX_PROPERTIES_PER_TRANSPORT;
 import static org.briarproject.bramble.test.TestUtils.getClientId;
 import static org.briarproject.bramble.test.TestUtils.getGroup;
-import static org.briarproject.bramble.test.TestUtils.getRandomBytes;
-import static org.briarproject.bramble.test.TestUtils.getRandomId;
+import static org.briarproject.bramble.test.TestUtils.getMessage;
 import static org.briarproject.bramble.test.TestUtils.getTransportId;
+import static org.briarproject.bramble.util.StringUtils.getRandomString;
 import static org.junit.Assert.assertEquals;
 
-public class TransportPropertyValidatorTest extends BrambleTestCase {
+public class TransportPropertyValidatorTest extends BrambleMockTestCase {
+
+	private final ClientHelper clientHelper = context.mock(ClientHelper.class);
 
 	private final TransportId transportId;
 	private final BdfDictionary bdfDictionary;
+	private final TransportProperties transportProperties;
 	private final Group group;
 	private final Message message;
 	private final TransportPropertyValidator tpv;
 
 	public TransportPropertyValidatorTest() {
 		transportId = getTransportId();
-		bdfDictionary = new BdfDictionary();
+		bdfDictionary = BdfDictionary.of(new BdfEntry("foo", "bar"));
+		transportProperties = new TransportProperties();
+		transportProperties.put("foo", "bar");
 
 		group = getGroup(getClientId());
-		MessageId messageId = new MessageId(getRandomId());
-		long timestamp = System.currentTimeMillis();
-		byte[] body = getRandomBytes(123);
-		message = new Message(messageId, group.getId(), timestamp, body);
+		message = getMessage(group.getId());
 
-		Mockery context = new Mockery();
-		ClientHelper clientHelper = context.mock(ClientHelper.class);
 		MetadataEncoder metadataEncoder = context.mock(MetadataEncoder.class);
 		Clock clock = context.mock(Clock.class);
-
 		tpv = new TransportPropertyValidator(clientHelper, metadataEncoder,
 				clock);
 	}
 
 	@Test
 	public void testValidateProperMessage() throws IOException {
-
 		BdfList body = BdfList.of(transportId.getString(), 4, bdfDictionary);
 
-		BdfDictionary result = tpv.validateMessage(message, group, body)
-				.getDictionary();
+		context.checking(new Expectations() {{
+			oneOf(clientHelper).parseAndValidateTransportProperties(
+					bdfDictionary);
+			will(returnValue(transportProperties));
+		}});
 
+		BdfDictionary result =
+				tpv.validateMessage(message, group, body).getDictionary();
 		assertEquals(transportId.getString(), result.getString("transportId"));
 		assertEquals(4, result.getLong("version").longValue());
 	}
 
 	@Test(expected = FormatException.class)
 	public void testValidateWrongVersionValue() throws IOException {
-
 		BdfList body = BdfList.of(transportId.getString(), -1, bdfDictionary);
 		tpv.validateMessage(message, group, body);
 	}
 
 	@Test(expected = FormatException.class)
 	public void testValidateWrongVersionType() throws IOException {
-
 		BdfList body = BdfList.of(transportId.getString(), bdfDictionary,
 				bdfDictionary);
 		tpv.validateMessage(message, group, body);
@@ -82,27 +82,15 @@ public class TransportPropertyValidatorTest extends BrambleTestCase {
 
 	@Test(expected = FormatException.class)
 	public void testValidateLongTransportId() throws IOException {
-
 		String wrongTransportIdString =
-				StringUtils.getRandomString(MAX_TRANSPORT_ID_LENGTH + 1);
+				getRandomString(MAX_TRANSPORT_ID_LENGTH + 1);
 		BdfList body = BdfList.of(wrongTransportIdString, 4, bdfDictionary);
 		tpv.validateMessage(message, group, body);
 	}
 
 	@Test(expected = FormatException.class)
 	public void testValidateEmptyTransportId() throws IOException {
-
 		BdfList body = BdfList.of("", 4, bdfDictionary);
 		tpv.validateMessage(message, group, body);
 	}
-
-	@Test(expected = FormatException.class)
-	public void testValidateTooManyProperties() throws IOException {
-
-		BdfDictionary d = new BdfDictionary();
-		for (int i = 0; i < MAX_PROPERTIES_PER_TRANSPORT + 1; i++)
-			d.put(String.valueOf(i), i);
-		BdfList body = BdfList.of(transportId.getString(), 4, d);
-		tpv.validateMessage(message, group, body);
-	}
 }