From 48933637d80665710153fb69977f6e8e4e82d479 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 24 Aug 2018 16:47:53 +0100 Subject: [PATCH] Remove raw messages from DB interface. --- .../bramble/api/db/DatabaseComponent.java | 12 ++++----- .../org/briarproject/bramble/db/Database.java | 9 ------- .../bramble/db/DatabaseComponentImpl.java | 12 ++++----- .../briarproject/bramble/db/JdbcDatabase.java | 24 ----------------- .../bramble/sync/DuplexOutgoingSession.java | 9 ++++--- .../bramble/sync/SimplexOutgoingSession.java | 9 ++++--- .../bramble/db/DatabaseComponentImplTest.java | 27 +++++++++---------- .../bramble/db/DatabasePerformanceTest.java | 6 ++--- .../bramble/db/JdbcDatabaseTest.java | 16 +++-------- .../sync/SimplexOutgoingSessionTest.java | 17 ++++++------ 10 files changed, 49 insertions(+), 92 deletions(-) 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 372421364..aec77c5aa 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 @@ -151,13 +151,13 @@ public interface DatabaseComponent { throws DbException; /** - * Returns a batch of raw messages for the given contact, with a total - * length less than or equal to the given length, for transmission over a + * Returns a batch of messages for the given contact, with a total length + * less than or equal to the given length, for transmission over a * transport with the given maximum latency. Returns null if there are no * sendable messages that fit in the given length. */ @Nullable - Collection generateBatch(Transaction txn, ContactId c, + Collection generateBatch(Transaction txn, ContactId c, int maxLength, int maxLatency) throws DbException; /** @@ -178,14 +178,14 @@ public interface DatabaseComponent { throws DbException; /** - * Returns a batch of raw messages for the given contact, with a total - * length less than or equal to the given length, for transmission over a + * Returns a batch of messages for the given contact, with a total length + * less than or equal to the given length, for transmission over a * transport with the given maximum latency. Only messages that have been * requested by the contact are returned. Returns null if there are no * sendable messages that fit in the given length. */ @Nullable - Collection generateRequestedBatch(Transaction txn, ContactId c, + Collection generateRequestedBatch(Transaction txn, ContactId c, int maxLength, int maxLatency) throws DbException; /** diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java index 1ad252584..d62bd6d38 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/Database.java @@ -474,15 +474,6 @@ interface Database { */ long getNextSendTime(T txn, ContactId c) throws DbException; - /** - * Returns the message with the given ID, in serialised form. - *

- * Read-only. - * - * @throws MessageDeletedException if the message has been deleted - */ - byte[] getRawMessage(T txn, MessageId m) throws DbException; - /** * Returns the IDs of some messages that are eligible to be sent to the * given contact and have been requested by the contact, up to the given 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 edeade4ee..3e4bed967 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 @@ -307,16 +307,16 @@ class DatabaseComponentImpl implements DatabaseComponent { @Nullable @Override - public Collection generateBatch(Transaction transaction, + public Collection generateBatch(Transaction transaction, ContactId c, int maxLength, int maxLatency) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); if (!db.containsContact(txn, c)) throw new NoSuchContactException(); Collection ids = db.getMessagesToSend(txn, c, maxLength); - List messages = new ArrayList<>(ids.size()); + List messages = new ArrayList<>(ids.size()); for (MessageId m : ids) { - messages.add(db.getRawMessage(txn, m)); + messages.add(db.getMessage(txn, m)); db.updateExpiryTime(txn, c, m, maxLatency); } if (ids.isEmpty()) return null; @@ -356,7 +356,7 @@ class DatabaseComponentImpl implements DatabaseComponent { @Nullable @Override - public Collection generateRequestedBatch(Transaction transaction, + public Collection generateRequestedBatch(Transaction transaction, ContactId c, int maxLength, int maxLatency) throws DbException { if (transaction.isReadOnly()) throw new IllegalArgumentException(); T txn = unbox(transaction); @@ -364,9 +364,9 @@ class DatabaseComponentImpl implements DatabaseComponent { throw new NoSuchContactException(); Collection ids = db.getRequestedMessagesToSend(txn, c, maxLength); - List messages = new ArrayList<>(ids.size()); + List messages = new ArrayList<>(ids.size()); for (MessageId m : ids) { - messages.add(db.getRawMessage(txn, m)); + messages.add(db.getMessage(txn, m)); db.updateExpiryTime(txn, c, m, maxLatency); } if (ids.isEmpty()) return null; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java index 36087b2ff..c0096d651 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/db/JdbcDatabase.java @@ -2045,30 +2045,6 @@ abstract class JdbcDatabase implements Database { } } - @Override - public byte[] getRawMessage(Connection txn, MessageId m) - throws DbException { - PreparedStatement ps = null; - ResultSet rs = null; - try { - String sql = "SELECT raw FROM messages WHERE messageId = ?"; - ps = txn.prepareStatement(sql); - ps.setBytes(1, m.getBytes()); - rs = ps.executeQuery(); - if (!rs.next()) throw new DbStateException(); - byte[] raw = rs.getBytes(1); - if (rs.next()) throw new DbStateException(); - rs.close(); - ps.close(); - if (raw == null) throw new MessageDeletedException(); - return raw; - } catch (SQLException e) { - tryToClose(rs); - tryToClose(ps); - throw new DbException(e); - } - } - @Override public Collection getRequestedMessagesToSend(Connection txn, ContactId c, int maxLength) throws DbException { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/DuplexOutgoingSession.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/DuplexOutgoingSession.java index f4109a488..3031116b5 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/sync/DuplexOutgoingSession.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/DuplexOutgoingSession.java @@ -13,6 +13,7 @@ import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.lifecycle.event.LifecycleEvent; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.Ack; +import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.Offer; import org.briarproject.bramble.api.sync.Request; import org.briarproject.bramble.api.sync.SyncRecordWriter; @@ -274,7 +275,7 @@ class DuplexOutgoingSession implements SyncSession, EventListener { if (!generateBatchQueued.getAndSet(false)) throw new AssertionError(); try { - Collection b; + Collection b; Transaction txn = db.startTransaction(false); try { b = db.generateRequestedBatch(txn, contactId, @@ -296,9 +297,9 @@ class DuplexOutgoingSession implements SyncSession, EventListener { private class WriteBatch implements ThrowingRunnable { - private final Collection batch; + private final Collection batch; - private WriteBatch(Collection batch) { + private WriteBatch(Collection batch) { this.batch = batch; } @@ -306,7 +307,7 @@ class DuplexOutgoingSession implements SyncSession, EventListener { @Override public void run() throws IOException { if (interrupted) return; - for (byte[] raw : batch) recordWriter.writeMessage(raw); + for (Message m : batch) recordWriter.writeMessage(m.getRaw()); LOG.info("Sent batch"); generateBatch(); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/sync/SimplexOutgoingSession.java b/bramble-core/src/main/java/org/briarproject/bramble/sync/SimplexOutgoingSession.java index 8fbd63cc8..259939e25 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/sync/SimplexOutgoingSession.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/sync/SimplexOutgoingSession.java @@ -13,6 +13,7 @@ import org.briarproject.bramble.api.lifecycle.IoExecutor; import org.briarproject.bramble.api.lifecycle.event.LifecycleEvent; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.Ack; +import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.SyncRecordWriter; import org.briarproject.bramble.api.sync.SyncSession; import org.briarproject.bramble.api.transport.StreamWriter; @@ -171,7 +172,7 @@ class SimplexOutgoingSession implements SyncSession, EventListener { public void run() { if (interrupted) return; try { - Collection b; + Collection b; Transaction txn = db.startTransaction(false); try { b = db.generateBatch(txn, contactId, @@ -193,9 +194,9 @@ class SimplexOutgoingSession implements SyncSession, EventListener { private class WriteBatch implements ThrowingRunnable { - private final Collection batch; + private final Collection batch; - private WriteBatch(Collection batch) { + private WriteBatch(Collection batch) { this.batch = batch; } @@ -203,7 +204,7 @@ class SimplexOutgoingSession implements SyncSession, EventListener { @Override public void run() throws IOException { if (interrupted) return; - for (byte[] raw : batch) recordWriter.writeMessage(raw); + for (Message m : batch) recordWriter.writeMessage(m.getRaw()); LOG.info("Sent batch"); dbExecutor.execute(new GenerateBatch()); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java index ee2a1a91c..ed3780026 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabaseComponentImplTest.java @@ -99,9 +99,8 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { private final Group group; private final Author author; private final LocalAuthor localAuthor; - private final Message message; + private final Message message, message1; private final MessageId messageId, messageId1; - private final byte[] raw, raw1; private final Metadata metadata; private final TransportId transportId; private final int maxLatency; @@ -117,11 +116,9 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { author = getAuthor(); localAuthor = getLocalAuthor(); message = getMessage(groupId); - Message message1 = getMessage(groupId); + message1 = getMessage(groupId); messageId = message.getId(); messageId1 = message1.getId(); - raw = message.getRaw(); - raw1 = message1.getRaw(); metadata = new Metadata(); metadata.put("foo", new byte[] {'b', 'a', 'r'}); transportId = getTransportId(); @@ -867,7 +864,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { @Test public void testGenerateBatch() throws Exception { Collection ids = Arrays.asList(messageId, messageId1); - Collection messages = Arrays.asList(raw, raw1); + Collection messages = Arrays.asList(message, message1); context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); @@ -876,12 +873,12 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { oneOf(database).getMessagesToSend(txn, contactId, MAX_MESSAGE_LENGTH * 2); will(returnValue(ids)); - oneOf(database).getRawMessage(txn, messageId); - will(returnValue(raw)); + oneOf(database).getMessage(txn, messageId); + will(returnValue(message)); oneOf(database).updateExpiryTime(txn, contactId, messageId, maxLatency); - oneOf(database).getRawMessage(txn, messageId1); - will(returnValue(raw1)); + oneOf(database).getMessage(txn, messageId1); + will(returnValue(message1)); oneOf(database).updateExpiryTime(txn, contactId, messageId1, maxLatency); oneOf(database).lowerRequestedFlag(txn, contactId, ids); @@ -963,7 +960,7 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { @Test public void testGenerateRequestedBatch() throws Exception { Collection ids = Arrays.asList(messageId, messageId1); - Collection messages = Arrays.asList(raw, raw1); + Collection messages = Arrays.asList(message, message1); context.checking(new Expectations() {{ oneOf(database).startTransaction(); will(returnValue(txn)); @@ -972,12 +969,12 @@ public class DatabaseComponentImplTest extends BrambleMockTestCase { oneOf(database).getRequestedMessagesToSend(txn, contactId, MAX_MESSAGE_LENGTH * 2); will(returnValue(ids)); - oneOf(database).getRawMessage(txn, messageId); - will(returnValue(raw)); + oneOf(database).getMessage(txn, messageId); + will(returnValue(message)); oneOf(database).updateExpiryTime(txn, contactId, messageId, maxLatency); - oneOf(database).getRawMessage(txn, messageId1); - will(returnValue(raw1)); + oneOf(database).getMessage(txn, messageId1); + will(returnValue(message1)); oneOf(database).updateExpiryTime(txn, contactId, messageId1, maxLatency); oneOf(database).lowerRequestedFlag(txn, contactId, ids); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java index e46a75b68..f20454bb6 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/DatabasePerformanceTest.java @@ -506,11 +506,11 @@ public abstract class DatabasePerformanceTest extends BrambleTestCase { } @Test - public void testGetRawMessage() throws Exception { - String name = "getRawMessage(T, MessageId)"; + public void testGetMessage() throws Exception { + String name = "getMessage(T, MessageId)"; benchmark(name, db -> { Connection txn = db.startTransaction(); - db.getRawMessage(txn, pickRandom(messages).getId()); + db.getMessage(txn, pickRandom(messages).getId()); db.commitTransaction(txn); }); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java index 57825b3ee..01ca372ec 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/db/JdbcDatabaseTest.java @@ -144,7 +144,8 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertTrue(db.containsContact(txn, contactId)); assertTrue(db.containsGroup(txn, groupId)); assertTrue(db.containsMessage(txn, messageId)); - assertArrayEquals(message.getRaw(), db.getRawMessage(txn, messageId)); + assertArrayEquals(message.getRaw(), + db.getMessage(txn, messageId).getRaw()); // Delete the records db.removeMessage(txn, messageId); @@ -1645,9 +1646,6 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { assertEquals(message.getTimestamp(), m.getTimestamp()); assertArrayEquals(message.getRaw(), m.getRaw()); - // The raw message should be available - assertArrayEquals(message.getRaw(), db.getRawMessage(txn, messageId)); - // Delete the message db.deleteMessage(txn, messageId); @@ -1668,14 +1666,6 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { // Expected } - // Requesting the raw message should throw an exception - try { - db.getRawMessage(txn, messageId); - fail(); - } catch (MessageDeletedException expected) { - // Expected - } - db.commitTransaction(txn); db.close(); } @@ -1806,7 +1796,7 @@ public abstract class JdbcDatabaseTest extends BrambleTestCase { Connection txn = db.startTransaction(); try { // Ask for a nonexistent message - an exception should be thrown - db.getRawMessage(txn, messageId); + db.getMessage(txn, messageId); fail(); } catch (DbException expected) { // It should be possible to abort the transaction without error 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 f91b5fe7a..6bb7e77a1 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 @@ -5,6 +5,8 @@ import org.briarproject.bramble.api.db.DatabaseComponent; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.event.EventBus; import org.briarproject.bramble.api.sync.Ack; +import org.briarproject.bramble.api.sync.GroupId; +import org.briarproject.bramble.api.sync.Message; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.SyncRecordWriter; import org.briarproject.bramble.api.transport.StreamWriter; @@ -13,12 +15,11 @@ import org.briarproject.bramble.test.ImmediateExecutor; import org.jmock.Expectations; import org.junit.Test; -import java.util.Arrays; -import java.util.Collections; import java.util.concurrent.Executor; +import static java.util.Collections.singletonList; import static org.briarproject.bramble.api.sync.SyncConstants.MAX_MESSAGE_IDS; -import static org.briarproject.bramble.test.TestUtils.getRandomBytes; +import static org.briarproject.bramble.test.TestUtils.getMessage; import static org.briarproject.bramble.test.TestUtils.getRandomId; public class SimplexOutgoingSessionTest extends BrambleMockTestCase { @@ -33,7 +34,8 @@ public class SimplexOutgoingSessionTest extends BrambleMockTestCase { private final Executor dbExecutor = new ImmediateExecutor(); private final ContactId contactId = new ContactId(234); - private final MessageId messageId = new MessageId(getRandomId()); + private final Message message = getMessage(new GroupId(getRandomId())); + private final MessageId messageId = message.getId(); @Test public void testNothingToSend() throws Exception { @@ -72,8 +74,7 @@ public class SimplexOutgoingSessionTest extends BrambleMockTestCase { @Test public void testSomethingToSend() throws Exception { - Ack ack = new Ack(Collections.singletonList(messageId)); - byte[] raw = getRandomBytes(1234); + Ack ack = new Ack(singletonList(messageId)); SimplexOutgoingSession session = new SimplexOutgoingSession(db, dbExecutor, eventBus, contactId, MAX_LATENCY, streamWriter, recordWriter); @@ -98,10 +99,10 @@ public class SimplexOutgoingSessionTest extends BrambleMockTestCase { will(returnValue(msgTxn)); oneOf(db).generateBatch(with(msgTxn), with(contactId), with(any(int.class)), with(MAX_LATENCY)); - will(returnValue(Arrays.asList(raw))); + will(returnValue(singletonList(message))); oneOf(db).commitTransaction(msgTxn); oneOf(db).endTransaction(msgTxn); - oneOf(recordWriter).writeMessage(raw); + oneOf(recordWriter).writeMessage(message.getRaw()); // No more acks oneOf(db).startTransaction(false); will(returnValue(noAckTxn));