From 717be0178aca0e3988e6e6d2490b49156cbd5665 Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Tue, 30 Jun 2020 14:05:46 +0100
Subject: [PATCH] Allow local transport properties to be removed by setting
 empty values.

---
 .../TransportPropertyManagerImpl.java          | 14 ++++++++++++--
 .../TransportPropertyManagerImplTest.java      | 18 +++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
index f0bcde0e6..b1557e526 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java
@@ -30,6 +30,7 @@ import org.briarproject.bramble.api.versioning.ClientVersioningManager;
 import org.briarproject.bramble.api.versioning.ClientVersioningManager.ClientVersioningHook;
 
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
 
@@ -41,6 +42,7 @@ import static org.briarproject.bramble.api.properties.TransportPropertyConstants
 import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_LOCAL;
 import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_TRANSPORT_ID;
 import static org.briarproject.bramble.api.properties.TransportPropertyConstants.MSG_KEY_VERSION;
+import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty;
 
 @Immutable
 @NotNullByDefault
@@ -272,14 +274,22 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 				LatestUpdate latest = findLatest(txn, localGroup.getId(), t,
 						true);
 				if (latest == null) {
-					merged = p;
+					merged = new TransportProperties(p);
+					Iterator<String> it = merged.values().iterator();
+					while (it.hasNext()) {
+						if (isNullOrEmpty(it.next())) it.remove();
+					}
 					changed = true;
 				} else {
 					BdfList message = clientHelper.getMessageAsList(txn,
 							latest.messageId);
 					TransportProperties old = parseProperties(message);
 					merged = new TransportProperties(old);
-					merged.putAll(p);
+					for (Entry<String, String> e : p.entrySet()) {
+						String key = e.getKey(), value = e.getValue();
+						if (isNullOrEmpty(value)) merged.remove(key);
+						else merged.put(key, value);
+					}
 					changed = !merged.equals(old);
 				}
 				if (changed) {
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
index 6b11df71e..b0602b5ad 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java
@@ -566,6 +566,10 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		Contact contact = getContact();
 		Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION);
 
+		// Property with an empty value should be discarded
+		TransportProperties properties = new TransportProperties(fooProperties);
+		properties.put("fooKey3", "");
+
 		context.checking(new DbExpectations() {{
 			oneOf(db).transaction(with(false), withDbRunnable(txn));
 			// There are no existing properties to merge with
@@ -589,7 +593,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		}});
 
 		TransportPropertyManagerImpl t = createInstance();
-		t.mergeLocalProperties(new TransportId("foo"), fooProperties);
+		t.mergeLocalProperties(new TransportId("foo"), properties);
 	}
 
 	@Test
@@ -605,16 +609,24 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		MessageId localGroupUpdateId = new MessageId(getRandomId());
 		Map<MessageId, BdfDictionary> localGroupMessageMetadata =
 				singletonMap(localGroupUpdateId, oldMetadata);
+
 		MessageId contactGroupUpdateId = new MessageId(getRandomId());
 		Map<MessageId, BdfDictionary> contactGroupMessageMetadata =
 				singletonMap(contactGroupUpdateId, oldMetadata);
+
 		TransportProperties oldProperties = new TransportProperties();
 		oldProperties.put("fooKey1", "oldFooValue1");
+		oldProperties.put("fooKey3", "oldFooValue3");
 		BdfDictionary oldPropertiesDict = BdfDictionary.of(
-				new BdfEntry("fooKey1", "oldFooValue1")
+				new BdfEntry("fooKey1", "oldFooValue1"),
+				new BdfEntry("fooKey3", "oldFooValue3")
 		);
 		BdfList oldUpdate = BdfList.of("foo", 1, oldPropertiesDict);
 
+		// Property assigned an empty value should be removed
+		TransportProperties properties = new TransportProperties(fooProperties);
+		properties.put("fooKey3", "");
+
 		context.checking(new DbExpectations() {{
 			oneOf(db).transaction(with(false), withDbRunnable(txn));
 			// Merge the new properties with the existing properties
@@ -647,7 +659,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 		}});
 
 		TransportPropertyManagerImpl t = createInstance();
-		t.mergeLocalProperties(new TransportId("foo"), fooProperties);
+		t.mergeLocalProperties(new TransportId("foo"), properties);
 	}
 
 	private void expectGetLocalProperties(Transaction txn) throws Exception {