Merge branch 'merge-settings' into 'master'

Fix a couple of bugs in settings

1. SettingsFragment wasn't receiving events because it extended BaseFragment rather than BaseEventFragment
2. Removed broken logic for deciding whether to broadcast a SettingsUpdatedEvent
3. Added the namespace to SettingsUpdatedEvent so listeners can decide whether to react

See merge request !97

briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java

@@ -13,7 +13,6 @@ import org.briarproject.android.contact.ConversationActivity;
 import org.briarproject.android.forum.ForumActivity;
 import org.briarproject.api.android.AndroidExecutor;
 import org.briarproject.api.android.AndroidNotificationManager;
-import org.briarproject.api.db.DatabaseComponent;
 import org.briarproject.api.db.DatabaseExecutor;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.event.Event;
@@ -24,6 +23,7 @@ import org.briarproject.api.forum.ForumManager;
 import org.briarproject.api.lifecycle.Service;
 import org.briarproject.api.messaging.MessagingManager;
 import org.briarproject.api.settings.Settings;
+import org.briarproject.api.settings.SettingsManager;
 import org.briarproject.api.sync.ClientId;
 import org.briarproject.api.sync.GroupId;
 import org.briarproject.util.StringUtils;
@@ -42,6 +42,7 @@ import static android.content.Context.NOTIFICATION_SERVICE;
 import static android.content.Intent.FLAG_ACTIVITY_CLEAR_TOP;
 import static android.content.Intent.FLAG_ACTIVITY_SINGLE_TOP;
 import static java.util.logging.Level.WARNING;
+import static org.briarproject.android.fragment.SettingsFragment.SETTINGS_NAMESPACE;
 
 class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 		Service, EventListener {
@@ -56,8 +57,8 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 	private static final Logger LOG =
 			Logger.getLogger(AndroidNotificationManagerImpl.class.getName());
 
-	private final DatabaseComponent db;
 	private final Executor dbExecutor;
+	private final SettingsManager settingsManager;
 	private final MessagingManager messagingManager;
 	private final ForumManager forumManager;
 	private final AndroidExecutor androidExecutor;
@@ -75,12 +76,12 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 	private volatile Settings settings = new Settings();
 
 	@Inject
-	public AndroidNotificationManagerImpl(DatabaseComponent db,
-			@DatabaseExecutor Executor dbExecutor,
-			MessagingManager messagingManager, ForumManager forumManager,
-			AndroidExecutor androidExecutor, Application app) {
-		this.db = db;
+	public AndroidNotificationManagerImpl(@DatabaseExecutor Executor dbExecutor,
+			SettingsManager settingsManager, MessagingManager messagingManager,
+			ForumManager forumManager, AndroidExecutor androidExecutor,
+			Application app) {
 		this.dbExecutor = dbExecutor;
+		this.settingsManager = settingsManager;
 		this.messagingManager = messagingManager;
 		this.forumManager = forumManager;
 		this.androidExecutor = androidExecutor;
@@ -97,7 +98,7 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 		dbExecutor.execute(new Runnable() {
 			public void run() {
 				try {
-					settings = db.getSettings("settings-activity");
+					settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
 				} catch (DbException e) {
 					if (LOG.isLoggable(WARNING))
 						LOG.log(WARNING, e.toString(), e);
@@ -135,7 +136,8 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
 
 	public void eventOccurred(Event e) {
 		if (e instanceof SettingsUpdatedEvent) {
-			loadSettings();
+			SettingsUpdatedEvent s = (SettingsUpdatedEvent) e;
+			if (s.getNamespace().equals(SETTINGS_NAMESPACE)) loadSettings();
 		} else if (e instanceof MessageValidatedEvent) {
 			MessageValidatedEvent m = (MessageValidatedEvent) e;
 			if (m.isValid() && !m.isLocal()) {

briar-android/src/org/briarproject/android/fragment/SettingsFragment.java

@@ -25,10 +25,8 @@ import org.briarproject.android.util.FixedVerticalSpace;
 import org.briarproject.android.util.HorizontalBorder;
 import org.briarproject.android.util.LayoutUtils;
 import org.briarproject.android.util.ListLoadingProgressBar;
-import org.briarproject.api.db.DatabaseComponent;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.event.Event;
-import org.briarproject.api.event.EventBus;
 import org.briarproject.api.event.SettingsUpdatedEvent;
 import org.briarproject.api.settings.Settings;
 import org.briarproject.api.settings.SettingsManager;
@@ -38,6 +36,7 @@ import java.util.logging.Logger;
 
 import javax.inject.Inject;
 
+import static android.app.Activity.RESULT_OK;
 import static android.graphics.Typeface.DEFAULT_BOLD;
 import static android.media.RingtoneManager.ACTION_RINGTONE_PICKER;
 import static android.media.RingtoneManager.EXTRA_RINGTONE_DEFAULT_URI;
@@ -55,16 +54,15 @@ import static android.widget.LinearLayout.VERTICAL;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 import static org.briarproject.android.TestingConstants.SHOW_TESTING_ACTIVITY;
-import static android.app.Activity.RESULT_OK;
 import static org.briarproject.android.util.CommonLayoutParams.MATCH_WRAP;
 import static org.briarproject.android.util.CommonLayoutParams.MATCH_WRAP_1;
 
-public class SettingsFragment extends BaseFragment implements
+public class SettingsFragment extends BaseEventFragment implements
 		View.OnClickListener {
 
-	public final static String TAG = "SettingsFragment";
-
+	public static final String TAG = "SettingsFragment";
 	public static final int REQUEST_RINGTONE = 2;
+	public static final String SETTINGS_NAMESPACE = "android-ui";
 
 	private static final Logger LOG =
 			Logger.getLogger(SettingsFragment.class.getName());
@@ -80,11 +78,9 @@ public class SettingsFragment extends BaseFragment implements
 	private ImageButton testingButton = null;
 
 	// Fields that are accessed from background threads must be volatile
-	@Inject private volatile DatabaseComponent db;
 	@Inject private volatile SettingsManager settingsManager;
-	@Inject private volatile EventBus eventBus;
 	private volatile Settings settings;
-	private volatile boolean bluetoothSetting = true, torSetting = false;
+	private volatile boolean bluetoothSetting = false, torSetting = false;
 
 	public static SettingsFragment newInstance() {
 
@@ -282,7 +278,7 @@ public class SettingsFragment extends BaseFragment implements
 			public void run() {
 				try {
 					long now = System.currentTimeMillis();
-					settings = settingsManager.getSettings("settings-activity");
+					settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
 					Settings btSettings = settingsManager.getSettings("bt");
 					Settings torSettings = settingsManager.getSettings("tor");
 					long duration = System.currentTimeMillis() - now;
@@ -360,7 +356,8 @@ public class SettingsFragment extends BaseFragment implements
 					notifyPrivateMessages.isChecked());
 			storeSettings(s);
 		} else if (view == panicSettings || view == panicSettingsHint) {
-			startActivity(new Intent(getActivity(), PanicPreferencesActivity.class));
+			startActivity(new Intent(getActivity(),
+					PanicPreferencesActivity.class));
 		} else if (view == notifyForumPosts) {
 			Settings s = new Settings();
 			s.putBoolean("notifyForumPosts", notifyForumPosts.isChecked());
@@ -431,8 +428,7 @@ public class SettingsFragment extends BaseFragment implements
 			public void run() {
 				try {
 					long now = System.currentTimeMillis();
-					settingsManager
-							.mergeSettings(settings, "settings-activity");
+					settingsManager.mergeSettings(settings, SETTINGS_NAMESPACE);
 					long duration = System.currentTimeMillis() - now;
 					if (LOG.isLoggable(INFO))
 						LOG.info("Merging settings took " + duration + " ms");
@@ -475,10 +471,15 @@ public class SettingsFragment extends BaseFragment implements
 		}
 	}
 
+	@Override
 	public void eventOccurred(Event e) {
 		if (e instanceof SettingsUpdatedEvent) {
-			LOG.info("Settings updated");
-			loadSettings();
+			String namespace = ((SettingsUpdatedEvent) e).getNamespace();
+			if (namespace.equals("bt") || namespace.equals("tor")
+					|| namespace.equals(SETTINGS_NAMESPACE)) {
+				LOG.info("Settings updated");
+				loadSettings();
+			}
 		}
 	}
 }

briar-android/src/org/briarproject/android/invitation/AddContactActivity.java

@@ -8,7 +8,6 @@ import org.briarproject.R;
 import org.briarproject.android.BriarActivity;
 import org.briarproject.api.android.ReferenceManager;
 import org.briarproject.api.crypto.CryptoComponent;
-import org.briarproject.api.db.DatabaseComponent;
 import org.briarproject.api.db.DbException;
 import org.briarproject.api.identity.AuthorId;
 import org.briarproject.api.identity.IdentityManager;
@@ -54,7 +53,6 @@ implements InvitationListener {
 	private String contactName = null;
 
 	// Fields that are accessed from background threads must be volatile
-	@Inject private volatile DatabaseComponent db;
 	@Inject private volatile IdentityManager identityManager;
 
 	@Override

briar-android/src/org/briarproject/plugins/tor/TorPlugin.java

@@ -596,8 +596,10 @@ class TorPlugin implements DuplexPlugin, EventHandler,
 
 	public void eventOccurred(Event e) {
 		if (e instanceof SettingsUpdatedEvent) {
-			// Wifi setting may have been updated
-			updateConnectionStatus();
+			if (((SettingsUpdatedEvent) e).getNamespace().equals("tor")) {
+				// Wifi setting may have been updated
+				updateConnectionStatus();
+			}
 		}
 	}
 

briar-api/src/org/briarproject/api/event/SettingsUpdatedEvent.java

@@ -3,4 +3,13 @@ package org.briarproject.api.event;
 /** An event that is broadcast when one or more settings are updated. */
 public class SettingsUpdatedEvent extends Event {
 
+	private final String namespace;
+
+	public SettingsUpdatedEvent(String namespace) {
+		this.namespace = namespace;
+	}
+
+	public String getNamespace() {
+		return namespace;
+	}
 }

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

@@ -935,7 +935,11 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		try {
 			T txn = db.startTransaction();
 			try {
-				if (!s.equals(db.getSettings(txn, namespace))) {
+				Settings old = db.getSettings(txn, namespace);
+				Settings merged = new Settings();
+				merged.putAll(old);
+				merged.putAll(s);
+				if (!merged.equals(old)) {
 					db.mergeSettings(txn, s, namespace);
 					changed = true;
 				}
@@ -947,7 +951,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
 		} finally {
 			lock.writeLock().unlock();
 		}
-		if (changed) eventBus.broadcast(new SettingsUpdatedEvent());
+		if (changed) eventBus.broadcast(new SettingsUpdatedEvent(namespace));
 	}
 
 	public void receiveAck(ContactId c, Ack a) throws DbException {

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

@@ -24,12 +24,14 @@ import org.briarproject.api.event.MessageToRequestEvent;
 import org.briarproject.api.event.MessageValidatedEvent;
 import org.briarproject.api.event.MessagesAckedEvent;
 import org.briarproject.api.event.MessagesSentEvent;
+import org.briarproject.api.event.SettingsUpdatedEvent;
 import org.briarproject.api.event.SubscriptionAddedEvent;
 import org.briarproject.api.event.SubscriptionRemovedEvent;
 import org.briarproject.api.identity.Author;
 import org.briarproject.api.identity.AuthorId;
 import org.briarproject.api.identity.LocalAuthor;
 import org.briarproject.api.lifecycle.ShutdownManager;
+import org.briarproject.api.settings.Settings;
 import org.briarproject.api.sync.Ack;
 import org.briarproject.api.sync.ClientId;
 import org.briarproject.api.sync.Group;
@@ -1319,4 +1321,47 @@ public class DatabaseComponentImplTest extends BriarTestCase {
 				2, 456);
 		return new TransportKeys(transportId, inPrev, inCurr, inNext, outCurr);
 	}
+
+	@Test
+	public void testMergeSettings() throws Exception {
+		final Settings before = new Settings();
+		before.put("foo", "bar");
+		before.put("baz", "bam");
+		final Settings update = new Settings();
+		update.put("baz", "qux");
+		final Settings merged = new Settings();
+		merged.put("foo", "bar");
+		merged.put("baz", "qux");
+		Mockery context = new Mockery();
+		@SuppressWarnings("unchecked")
+		final Database<Object> database = context.mock(Database.class);
+		final ShutdownManager shutdown = context.mock(ShutdownManager.class);
+		final EventBus eventBus = context.mock(EventBus.class);
+		context.checking(new Expectations() {{
+			// mergeSettings()
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).getSettings(txn, "namespace");
+			will(returnValue(before));
+			oneOf(database).mergeSettings(txn, update, "namespace");
+			oneOf(database).commitTransaction(txn);
+			oneOf(eventBus).broadcast(with(any(SettingsUpdatedEvent.class)));
+			// mergeSettings() again
+			oneOf(database).startTransaction();
+			will(returnValue(txn));
+			oneOf(database).getSettings(txn, "namespace");
+			will(returnValue(merged));
+			oneOf(database).commitTransaction(txn);
+		}});
+
+		DatabaseComponent db = createDatabaseComponent(database, eventBus,
+				shutdown);
+
+		// First merge should broadcast an event
+		db.mergeSettings(update, "namespace");
+		// Second merge should not broadcast an event
+		db.mergeSettings(update, "namespace");
+
+		context.assertIsSatisfied();
+	}
 }