From 3d948ed4610ac8d83b5111a704e9b0602eb87edd Mon Sep 17 00:00:00 2001 From: akwizgran <akwizgran@users.sourceforge.net> Date: Mon, 8 Feb 2016 11:17:45 +0000 Subject: [PATCH] Don't broadcast an event unless settings have changed. --- .../invitation/AddContactActivity.java | 2 - .../db/DatabaseComponentImpl.java | 12 ++++- .../db/DatabaseComponentImplTest.java | 45 +++++++++++++++++++ 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java b/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java index dc594a4f6f..3ae2ae4bce 100644 --- a/briar-android/src/org/briarproject/android/invitation/AddContactActivity.java +++ b/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 diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index 676b642041..505a288cd8 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -930,11 +930,19 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { } public void mergeSettings(Settings s, String namespace) throws DbException { + boolean changed = false; lock.writeLock().lock(); try { T txn = db.startTransaction(); try { - db.mergeSettings(txn, s, 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; + } db.commitTransaction(txn); } catch (DbException e) { db.abortTransaction(txn); @@ -943,7 +951,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { } finally { lock.writeLock().unlock(); } - eventBus.broadcast(new SettingsUpdatedEvent(namespace)); + if (changed) eventBus.broadcast(new SettingsUpdatedEvent(namespace)); } public void receiveAck(ContactId c, Ack a) throws DbException { diff --git a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java index eb0d8315c0..abc95eda18 100644 --- a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java +++ b/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(); + } } -- GitLab