From b24914408dd188af2fba0ce4ae69caebb89eeb44 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 12 Nov 2018 11:31:59 +0000 Subject: [PATCH 1/4] Stream context may be null. --- .../org/briarproject/bramble/transport/KeyManagerImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java index 363034c3e..624c8b2b8 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/transport/KeyManagerImpl.java @@ -137,7 +137,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); return null; } - return db.transactionWithResult(false, txn -> + return db.transactionWithNullableResult(false, txn -> m.getStreamContext(txn, c)); } @@ -149,7 +149,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener { if (LOG.isLoggable(INFO)) LOG.info("No key manager for " + t); return null; } - return db.transactionWithResult(false, txn -> + return db.transactionWithNullableResult(false, txn -> m.getStreamContext(txn, tag)); } From ecb63d1acb8f82345eb43a40fa8cc32b2ba6ed23 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 12 Nov 2018 12:13:26 +0000 Subject: [PATCH 2/4] Add interface for DB tasks will nullable results. --- .../bramble/api/db/DatabaseComponent.java | 2 +- .../briarproject/bramble/api/db/DbCallable.java | 3 --- .../bramble/api/db/NullableDbCallable.java | 12 ++++++++++++ .../bramble/db/DatabaseComponentImpl.java | 15 +++++++++++---- .../bramble/sync/SimplexOutgoingSessionTest.java | 12 ++++++------ .../briarproject/bramble/test/DbExpectations.java | 8 ++++++++ 6 files changed, 38 insertions(+), 14 deletions(-) create mode 100644 bramble-api/src/main/java/org/briarproject/bramble/api/db/NullableDbCallable.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java index 73e150542..5e6aeb299 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DatabaseComponent.java @@ -95,7 +95,7 @@ public interface DatabaseComponent { */ @Nullable R transactionWithNullableResult(boolean readOnly, - DbCallable task) throws DbException, E; + NullableDbCallable task) throws DbException, E; /** * Stores a contact associated with the given local and remote pseudonyms, diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DbCallable.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DbCallable.java index bb558a5d9..1f024b0dd 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/db/DbCallable.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/DbCallable.java @@ -2,11 +2,8 @@ package org.briarproject.bramble.api.db; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; -import javax.annotation.Nullable; - @NotNullByDefault public interface DbCallable { - @Nullable R call(Transaction txn) throws DbException, E; } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/db/NullableDbCallable.java b/bramble-api/src/main/java/org/briarproject/bramble/api/db/NullableDbCallable.java new file mode 100644 index 000000000..4ed459118 --- /dev/null +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/db/NullableDbCallable.java @@ -0,0 +1,12 @@ +package org.briarproject.bramble.api.db; + +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; + +@NotNullByDefault +public interface NullableDbCallable { + + @Nullable + R call(Transaction txn) throws DbException, E; +} diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java index e12e694a3..68f5afd12 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/DatabaseComponentImpl.java @@ -19,6 +19,7 @@ import org.briarproject.bramble.api.db.NoSuchGroupException; import org.briarproject.bramble.api.db.NoSuchLocalAuthorException; import org.briarproject.bramble.api.db.NoSuchMessageException; import org.briarproject.bramble.api.db.NoSuchTransportException; +import org.briarproject.bramble.api.db.NullableDbCallable; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventBus; @@ -183,14 +184,20 @@ class DatabaseComponentImpl implements DatabaseComponent { @Override public R transactionWithResult(boolean readOnly, DbCallable task) throws DbException, E { - R result = transactionWithNullableResult(readOnly, task); - if (result == null) throw new NullPointerException(); - return result; + Transaction txn = startTransaction(readOnly); + try { + R result = task.call(txn); + commitTransaction(txn); + return result; + } finally { + endTransaction(txn); + } } @Override public R transactionWithNullableResult( - boolean readOnly, DbCallable task) throws DbException, E { + boolean readOnly, NullableDbCallable task) + throws DbException, E { Transaction txn = startTransaction(readOnly); try { R result = task.call(txn); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/sync/SimplexOutgoingSessionTest.java b/bramble-core/src/test/java/org/briarproject/bramble/sync/SimplexOutgoingSessionTest.java index c1a61a8eb..e050fae54 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/sync/SimplexOutgoingSessionTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/sync/SimplexOutgoingSessionTest.java @@ -50,12 +50,12 @@ public class SimplexOutgoingSessionTest extends BrambleMockTestCase { oneOf(eventBus).addListener(session); // No acks to send oneOf(db).transactionWithNullableResult(with(false), - withDbCallable(noAckTxn)); + withNullableDbCallable(noAckTxn)); oneOf(db).generateAck(noAckTxn, contactId, MAX_MESSAGE_IDS); will(returnValue(null)); // No messages to send oneOf(db).transactionWithNullableResult(with(false), - withDbCallable(noMsgTxn)); + withNullableDbCallable(noMsgTxn)); oneOf(db).generateBatch(with(noMsgTxn), with(contactId), with(any(int.class)), with(MAX_LATENCY)); will(returnValue(null)); @@ -84,25 +84,25 @@ public class SimplexOutgoingSessionTest extends BrambleMockTestCase { oneOf(eventBus).addListener(session); // One ack to send oneOf(db).transactionWithNullableResult(with(false), - withDbCallable(ackTxn)); + withNullableDbCallable(ackTxn)); oneOf(db).generateAck(ackTxn, contactId, MAX_MESSAGE_IDS); will(returnValue(ack)); oneOf(recordWriter).writeAck(ack); // One message to send oneOf(db).transactionWithNullableResult(with(false), - withDbCallable(msgTxn)); + withNullableDbCallable(msgTxn)); oneOf(db).generateBatch(with(msgTxn), with(contactId), with(any(int.class)), with(MAX_LATENCY)); will(returnValue(singletonList(message))); oneOf(recordWriter).writeMessage(message); // No more acks oneOf(db).transactionWithNullableResult(with(false), - withDbCallable(noAckTxn)); + withNullableDbCallable(noAckTxn)); oneOf(db).generateAck(noAckTxn, contactId, MAX_MESSAGE_IDS); will(returnValue(null)); // No more messages oneOf(db).transactionWithNullableResult(with(false), - withDbCallable(noMsgTxn)); + withNullableDbCallable(noMsgTxn)); oneOf(db).generateBatch(with(noMsgTxn), with(contactId), with(any(int.class)), with(MAX_LATENCY)); will(returnValue(null)); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java b/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java index 8c5fa8f28..17682e78d 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java @@ -2,6 +2,7 @@ package org.briarproject.bramble.test; import org.briarproject.bramble.api.db.DbCallable; import org.briarproject.bramble.api.db.DbRunnable; +import org.briarproject.bramble.api.db.NullableDbCallable; import org.briarproject.bramble.api.db.Transaction; import org.jmock.Expectations; @@ -21,4 +22,11 @@ public class DbExpectations extends Expectations { return null; } + protected NullableDbCallable withNullableDbCallable( + Transaction txn) { + addParameterMatcher(any(NullableDbCallable.class)); + currentBuilder().setAction(new RunTransactionWithResultAction(txn)); + return null; + } + } From 3bfedfdc3dbe548ee93f08ff87e93a86d2a0bfb8 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 12 Nov 2018 12:16:42 +0000 Subject: [PATCH 3/4] Add action for nullable DB callables. --- .../bramble/test/DbExpectations.java | 3 +- ...unTransactionWithNullableResultAction.java | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/test/RunTransactionWithNullableResultAction.java diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java b/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java index 17682e78d..6f2a0c104 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/DbExpectations.java @@ -25,7 +25,8 @@ public class DbExpectations extends Expectations { protected NullableDbCallable withNullableDbCallable( Transaction txn) { addParameterMatcher(any(NullableDbCallable.class)); - currentBuilder().setAction(new RunTransactionWithResultAction(txn)); + currentBuilder().setAction( + new RunTransactionWithNullableResultAction(txn)); return null; } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/RunTransactionWithNullableResultAction.java b/bramble-core/src/test/java/org/briarproject/bramble/test/RunTransactionWithNullableResultAction.java new file mode 100644 index 000000000..9ed991c92 --- /dev/null +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/RunTransactionWithNullableResultAction.java @@ -0,0 +1,28 @@ +package org.briarproject.bramble.test; + +import org.briarproject.bramble.api.db.NullableDbCallable; +import org.briarproject.bramble.api.db.Transaction; +import org.hamcrest.Description; +import org.jmock.api.Action; +import org.jmock.api.Invocation; + +public class RunTransactionWithNullableResultAction implements Action { + + private final Transaction txn; + + public RunTransactionWithNullableResultAction(Transaction txn) { + this.txn = txn; + } + + @Override + public Object invoke(Invocation invocation) throws Throwable { + NullableDbCallable task = + (NullableDbCallable) invocation.getParameter(1); + return task.call(txn); + } + + @Override + public void describeTo(Description description) { + description.appendText("runs a task inside a database transaction"); + } +} From c00ee80f0f94d9326d0cca464000a74e3bbbc4d7 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 12 Nov 2018 12:20:04 +0000 Subject: [PATCH 4/4] Update test expectations. --- .../bramble/transport/KeyManagerImplTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java index 336d09275..c09185f65 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/transport/KeyManagerImplTest.java @@ -131,7 +131,8 @@ public class KeyManagerImplTest extends BrambleMockTestCase { @Test public void testGetStreamContextForContact() throws Exception { context.checking(new DbExpectations() {{ - oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); + oneOf(db).transactionWithNullableResult(with(false), + withNullableDbCallable(txn)); oneOf(transportKeyManager).getStreamContext(txn, contactId); will(returnValue(streamContext)); }}); @@ -149,7 +150,8 @@ public class KeyManagerImplTest extends BrambleMockTestCase { @Test public void testGetStreamContextForTag() throws Exception { context.checking(new DbExpectations() {{ - oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); + oneOf(db).transactionWithNullableResult(with(false), + withNullableDbCallable(txn)); oneOf(transportKeyManager).getStreamContext(txn, tag); will(returnValue(streamContext)); }}); @@ -177,7 +179,8 @@ public class KeyManagerImplTest extends BrambleMockTestCase { new ContactStatusChangedEvent(inactiveContactId, true); context.checking(new DbExpectations() {{ - oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); + oneOf(db).transactionWithNullableResult(with(false), + withNullableDbCallable(txn)); oneOf(transportKeyManager).getStreamContext(txn, inactiveContactId); will(returnValue(streamContext)); }});