Code cleanup.

briar-core/src/org/briarproject/db/DatabaseComponentImpl.java

@@ -245,25 +245,22 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public Ack generateAck(Transaction transaction, ContactId c,
 			int maxMessages) throws DbException {
-		Collection<MessageId> ids;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		ids = db.getMessagesToAck(txn, c, maxMessages);
+		Collection<MessageId> ids = db.getMessagesToAck(txn, c, maxMessages);
-		if (!ids.isEmpty()) db.lowerAckFlag(txn, c, ids);
 		if (ids.isEmpty()) return null;
+		db.lowerAckFlag(txn, c, ids);
 		return new Ack(ids);
 	}
 
 	public Collection<byte[]> generateBatch(Transaction transaction,
 			ContactId c, int maxLength, int maxLatency) throws DbException {
-		Collection<MessageId> ids;
-		List<byte[]> messages;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		ids = db.getMessagesToSend(txn, c, maxLength);
+		Collection<MessageId> ids = db.getMessagesToSend(txn, c, maxLength);
-		messages = new ArrayList<byte[]>(ids.size());
+		List<byte[]> messages = new ArrayList<byte[]>(ids.size());
 		for (MessageId m : ids) {
 			messages.add(db.getRawMessage(txn, m));
 			db.updateExpiryTime(txn, c, m, maxLatency);
@@ -276,37 +273,35 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public Offer generateOffer(Transaction transaction, ContactId c,
 			int maxMessages, int maxLatency) throws DbException {
-		Collection<MessageId> ids;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		ids = db.getMessagesToOffer(txn, c, maxMessages);
+		Collection<MessageId> ids = db.getMessagesToOffer(txn, c, maxMessages);
-		for (MessageId m : ids) db.updateExpiryTime(txn, c, m, maxLatency);
 		if (ids.isEmpty()) return null;
+		for (MessageId m : ids) db.updateExpiryTime(txn, c, m, maxLatency);
 		return new Offer(ids);
 	}
 
 	public Request generateRequest(Transaction transaction, ContactId c,
 			int maxMessages) throws DbException {
-		Collection<MessageId> ids;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		ids = db.getMessagesToRequest(txn, c, maxMessages);
+		Collection<MessageId> ids = db.getMessagesToRequest(txn, c,
-		if (!ids.isEmpty()) db.removeOfferedMessages(txn, c, ids);
+				maxMessages);
 		if (ids.isEmpty()) return null;
+		db.removeOfferedMessages(txn, c, ids);
 		return new Request(ids);
 	}
 
 	public Collection<byte[]> generateRequestedBatch(Transaction transaction,
 			ContactId c, int maxLength, int maxLatency) throws DbException {
-		Collection<MessageId> ids;
-		List<byte[]> messages;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
-		ids = db.getRequestedMessagesToSend(txn, c, maxLength);
+		Collection<MessageId> ids = db.getRequestedMessagesToSend(txn, c,
-		messages = new ArrayList<byte[]>(ids.size());
+				maxLength);
+		List<byte[]> messages = new ArrayList<byte[]>(ids.size());
 		for (MessageId m : ids) {
 			messages.add(db.getRawMessage(txn, m));
 			db.updateExpiryTime(txn, c, m, maxLatency);
@@ -501,10 +496,10 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void receiveAck(Transaction transaction, ContactId c, Ack a)
 			throws DbException {
-		Collection<MessageId> acked = new ArrayList<MessageId>();
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
+		Collection<MessageId> acked = new ArrayList<MessageId>();
 		for (MessageId m : a.getMessageIds()) {
 			if (db.containsVisibleMessage(txn, c, m)) {
 				db.raiseSeenFlag(txn, c, m);
@@ -531,11 +526,11 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void receiveOffer(Transaction transaction, ContactId c, Offer o)
 			throws DbException {
-		boolean ack = false, request = false;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
 		int count = db.countOfferedMessages(txn, c);
+		boolean ack = false, request = false;
 		for (MessageId m : o.getMessageIds()) {
 			if (db.containsVisibleMessage(txn, c, m)) {
 				db.raiseSeenFlag(txn, c, m);
@@ -553,10 +548,10 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void receiveRequest(Transaction transaction, ContactId c, Request r)
 			throws DbException {
-		boolean requested = false;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
+		boolean requested = false;
 		for (MessageId m : r.getMessageIds()) {
 			if (db.containsVisibleMessage(txn, c, m)) {
 				db.raiseRequestedFlag(txn, c, m);
@@ -578,12 +573,11 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void removeGroup(Transaction transaction, Group g)
 			throws DbException {
-		Collection<ContactId> affected;
 		T txn = unbox(transaction);
 		GroupId id = g.getId();
 		if (!db.containsGroup(txn, id))
 			throw new NoSuchGroupException();
-		affected = db.getVisibility(txn, id);
+		Collection<ContactId> affected = db.getVisibility(txn, id);
 		db.removeGroup(txn, id);
 		transaction.attach(new GroupRemovedEvent(g));
 		transaction.attach(new GroupVisibilityUpdatedEvent(affected));
@@ -638,13 +632,12 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void setVisibleToContact(Transaction transaction, ContactId c,
 			GroupId g, boolean visible) throws DbException {
-		boolean wasVisible;
 		T txn = unbox(transaction);
 		if (!db.containsContact(txn, c))
 			throw new NoSuchContactException();
 		if (!db.containsGroup(txn, g))
 			throw new NoSuchGroupException();
-		wasVisible = db.containsVisibleGroup(txn, c, g);
+		boolean wasVisible = db.containsVisibleGroup(txn, c, g);
 		if (visible && !wasVisible) db.addVisibility(txn, c, g);
 		else if (!visible && wasVisible) db.removeVisibility(txn, c, g);
 		if (visible != wasVisible) {
@@ -655,9 +648,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 
 	public void updateTransportKeys(Transaction transaction,
 			Map<ContactId, TransportKeys> keys) throws DbException {
+		T txn = unbox(transaction);
 		Map<ContactId, TransportKeys> filtered =
 				new HashMap<ContactId, TransportKeys>();
-		T txn = unbox(transaction);
 		for (Entry<ContactId, TransportKeys> e : keys.entrySet()) {
 			ContactId c = e.getKey();
 			TransportKeys k = e.getValue();

briar-core/src/org/briarproject/forum/ForumSharingManagerImpl.java

@@ -45,7 +45,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import java.util.logging.Logger;
 
 import static org.briarproject.api.forum.ForumConstants.FORUM_SALT_LENGTH;
 import static org.briarproject.api.forum.ForumConstants.MAX_FORUM_NAME_LENGTH;
@@ -376,8 +375,8 @@ class ForumSharingManagerImpl implements ForumSharingManager, AddContactHook,
 			BdfDictionary d = new BdfDictionary();
 			d.put("version", version);
 			d.put("local", true);
-			db.addLocalMessage(txn, m, CLIENT_ID, metadataEncoder.encode(d),
+			Metadata meta = metadataEncoder.encode(d);
-					true);
+			db.addLocalMessage(txn, m, CLIENT_ID, meta, true);
 		} catch (FormatException e) {
 			throw new RuntimeException(e);
 		}

briar-core/src/org/briarproject/messaging/MessagingManagerImpl.java

@@ -154,11 +154,11 @@ class MessagingManagerImpl implements MessagingManager, AddContactHook,
 	@Override
 	public Collection<PrivateMessageHeader> getMessageHeaders(ContactId c)
 			throws DbException {
-		GroupId g = getConversationId(c);
 		Map<MessageId, Metadata> metadata;
 		Collection<MessageStatus> statuses;
 		Transaction txn = db.startTransaction();
 		try {
+			GroupId g = getContactGroup(db.getContact(txn, c)).getId();
 			metadata = db.getMessageMetadata(txn, g);
 			statuses = db.getMessageStatus(txn, c, g);
 			txn.setComplete();

briar-core/src/org/briarproject/properties/TransportPropertyManagerImpl.java

@@ -91,7 +91,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 		db.setVisibleToContact(txn, c.getId(), g.getId(), true);
 		// Copy the latest local properties into the group
 		DeviceId dev = db.getDeviceId(txn);
-		Map<TransportId, TransportProperties> local = getLocalProperties();
+		Map<TransportId, TransportProperties> local = getLocalProperties(txn);
 		for (Entry<TransportId, TransportProperties> e : local.entrySet()) {
 			storeMessage(txn, g.getId(), dev, e.getKey(), e.getValue(), 1,
 					true, true);
@@ -122,30 +122,15 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 	@Override
 	public Map<TransportId, TransportProperties> getLocalProperties()
 			throws DbException {
+		Map<TransportId, TransportProperties> local;
+		Transaction txn = db.startTransaction();
 		try {
-			Map<TransportId, TransportProperties> local =
+			local = getLocalProperties(txn);
-					new HashMap<TransportId, TransportProperties>();
+			txn.setComplete();
-			Transaction txn = db.startTransaction();
+		} finally {
-			try {
+			db.endTransaction(txn);
-				// Find the latest local update for each transport
-				Map<TransportId, LatestUpdate> latest = findLatest(txn,
-						localGroup.getId(), true);
-				// Retrieve and parse the latest local properties
-				for (Entry<TransportId, LatestUpdate> e : latest.entrySet()) {
-					byte[] raw = db.getRawMessage(txn, e.getValue().messageId);
-					local.put(e.getKey(), parseProperties(raw));
-				}
-				txn.setComplete();
-			} finally {
-				db.endTransaction(txn);
-			}
-			return Collections.unmodifiableMap(local);
-		} catch (NoSuchGroupException e) {
-			// Local group doesn't exist - there are no local properties
-			return Collections.emptyMap();
-		} catch (FormatException e) {
-			throw new DbException(e);
 		}
+		return Collections.unmodifiableMap(local);
 	}
 
 	@Override
@@ -255,6 +240,28 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 		return privateGroupFactory.createPrivateGroup(CLIENT_ID, c);
 	}
 
+	private Map<TransportId, TransportProperties> getLocalProperties(
+			Transaction txn) throws DbException {
+		try {
+			Map<TransportId, TransportProperties> local =
+					new HashMap<TransportId, TransportProperties>();
+			// Find the latest local update for each transport
+			Map<TransportId, LatestUpdate> latest = findLatest(txn,
+					localGroup.getId(), true);
+			// Retrieve and parse the latest local properties
+			for (Entry<TransportId, LatestUpdate> e : latest.entrySet()) {
+				byte[] raw = db.getRawMessage(txn, e.getValue().messageId);
+				local.put(e.getKey(), parseProperties(raw));
+			}
+			return local;
+		} catch (NoSuchGroupException e) {
+			// Local group doesn't exist - there are no local properties
+			return Collections.emptyMap();
+		} catch (FormatException e) {
+			throw new DbException(e);
+		}
+	}
+
 	private void storeMessage(Transaction txn, GroupId g, DeviceId dev,
 			TransportId t, TransportProperties p, long version, boolean local,
 			boolean shared) throws DbException {

briar-core/src/org/briarproject/settings/SettingsManagerImpl.java

@@ -19,14 +19,15 @@ class SettingsManagerImpl implements SettingsManager {
 
 	@Override
 	public Settings getSettings(String namespace) throws DbException {
+		Settings s;
 		Transaction txn = db.startTransaction();
 		try {
-			Settings s = db.getSettings(txn, namespace);
+			s = db.getSettings(txn, namespace);
 			txn.setComplete();
-			return s;
 		} finally {
 			db.endTransaction(txn);
 		}
+		return s;
 	}
 
 	@Override

briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java

@@ -190,20 +190,23 @@ public class DatabaseComponentImplTest extends BriarTestCase {
 
 		assertFalse(db.open());
 		Transaction transaction = db.startTransaction();
-		db.addLocalAuthor(transaction, localAuthor);
+		try {
-		assertEquals(contactId,
+			db.addLocalAuthor(transaction, localAuthor);
-				db.addContact(transaction, author, localAuthorId));
+			assertEquals(contactId,
-		assertEquals(Collections.singletonList(contact),
+					db.addContact(transaction, author, localAuthorId));
-				db.getContacts(transaction));
+			assertEquals(Collections.singletonList(contact),
-		db.addGroup(transaction, group); // First time - listeners called
+					db.getContacts(transaction));
-		db.addGroup(transaction, group); // Second time - not called
+			db.addGroup(transaction, group); // First time - listeners called
-		assertEquals(Collections.singletonList(group),
+			db.addGroup(transaction, group); // Second time - not called
-				db.getGroups(transaction, clientId));
+			assertEquals(Collections.singletonList(group),
-		db.removeGroup(transaction, group);
+					db.getGroups(transaction, clientId));
-		db.removeContact(transaction, contactId);
+			db.removeGroup(transaction, group);
-		db.removeLocalAuthor(transaction, localAuthorId);
+			db.removeContact(transaction, contactId);
-		transaction.setComplete();
+			db.removeLocalAuthor(transaction, localAuthorId);
-		db.endTransaction(transaction);
+			transaction.setComplete();
+		} finally {
+			db.endTransaction(transaction);
+		}
 		db.close();
 
 		context.assertIsSatisfied();