From 00275e260fcb69db3010717a41484a27e190f185 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 9 Feb 2016 11:14:52 +0000 Subject: [PATCH 1/2] Allow messages to be deleted. --- .../api/db/DatabaseComponent.java | 6 +++ .../src/org/briarproject/db/Database.java | 10 ++++ .../db/DatabaseComponentImpl.java | 17 +++++++ .../src/org/briarproject/db/JdbcDatabase.java | 26 ++++++++-- .../org/briarproject/db/H2DatabaseTest.java | 47 +++++++++++++++++++ 5 files changed, 101 insertions(+), 5 deletions(-) diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java index 465a886bd..80fd14a7e 100644 --- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java +++ b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java @@ -63,6 +63,12 @@ public interface DatabaseComponent { */ void addTransportKeys(ContactId c, TransportKeys k) throws DbException; + /** + * Deletes the message with the given ID. The message ID and any other + * associated data are not deleted. + */ + void deleteMessage(MessageId m) throws DbException; + /** * Returns an acknowledgement for the given contact, or null if there are * no messages to acknowledge. diff --git a/briar-core/src/org/briarproject/db/Database.java b/briar-core/src/org/briarproject/db/Database.java index a5a22b135..d21e6c349 100644 --- a/briar-core/src/org/briarproject/db/Database.java +++ b/briar-core/src/org/briarproject/db/Database.java @@ -215,6 +215,16 @@ interface Database { */ int countOfferedMessages(T txn, ContactId c) throws DbException; + /** + * Deletes the message with the given ID. Unlike + * {@link #removeMessage(Object, MessageId)}, the message ID and any other + * associated data are not deleted, and + * {@link #containsMessage(Object, MessageId)} will continue to return true. + *

+ * Locking: write. + */ + void deleteMessage(T txn, MessageId m) throws DbException; + /** * Returns the contact with the given ID. *

diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index 9e929d86c..d55501a10 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -293,6 +293,23 @@ class DatabaseComponentImpl implements DatabaseComponent { } } + public void deleteMessage(MessageId m) throws DbException { + lock.writeLock().lock(); + try { + T txn = db.startTransaction(); + try { + if (!db.containsMessage(txn, m)) + throw new NoSuchMessageException(); + db.deleteMessage(txn, m); + } catch (DbException e) { + db.abortTransaction(txn); + throw e; + } + } finally { + lock.writeLock().unlock(); + } + } + public Ack generateAck(ContactId c, int maxMessages) throws DbException { Collection ids; lock.writeLock().lock(); diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index 79c07da03..2f32e8fd8 100644 --- a/briar-core/src/org/briarproject/db/JdbcDatabase.java +++ b/briar-core/src/org/briarproject/db/JdbcDatabase.java @@ -136,7 +136,7 @@ abstract class JdbcDatabase implements Database { + " valid INT NOT NULL," + " shared BOOLEAN NOT NULL," + " length INT NOT NULL," - + " raw BLOB NOT NULL," + + " raw BLOB," // Null if message has been deleted + " PRIMARY KEY (messageId)," + " FOREIGN KEY (groupId)" + " REFERENCES groups (groupId)" @@ -933,6 +933,22 @@ abstract class JdbcDatabase implements Database { } } + public void deleteMessage(Connection txn, MessageId m) throws DbException { + PreparedStatement ps = null; + try { + String sql = "UPDATE messages SET raw = NULL WHERE messageId = ?"; + ps = txn.prepareStatement(sql); + ps.setBytes(1, m.getBytes()); + int affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + if (affected > 1) throw new DbStateException(); + ps.close(); + } catch (SQLException e) { + tryToClose(ps); + throw new DbException(e); + } + } + public Contact getContact(Connection txn, ContactId c) throws DbException { PreparedStatement ps = null; ResultSet rs = null; @@ -1308,7 +1324,7 @@ abstract class JdbcDatabase implements Database { + " ON m.messageId = s.messageId" + " AND gv.contactId = s.contactId" + " WHERE gv.contactId = ?" - + " AND valid = ? AND shared = TRUE" + + " AND valid = ? AND shared = TRUE AND raw IS NOT NULL" + " AND seen = FALSE AND requested = FALSE" + " AND s.expiry < ?" + " ORDER BY timestamp DESC LIMIT ?"; @@ -1367,7 +1383,7 @@ abstract class JdbcDatabase implements Database { + " ON m.messageId = s.messageId" + " AND gv.contactId = s.contactId" + " WHERE gv.contactId = ?" - + " AND valid = ? AND shared = TRUE" + + " AND valid = ? AND shared = TRUE AND raw IS NOT NULL" + " AND seen = FALSE" + " AND s.expiry < ?" + " ORDER BY timestamp DESC"; @@ -1401,7 +1417,7 @@ abstract class JdbcDatabase implements Database { try { String sql = "SELECT messageId FROM messages AS m" + " JOIN groups AS g ON m.groupId = g.groupId" - + " WHERE valid = ? AND clientId = ?"; + + " WHERE valid = ? AND clientId = ? AND raw IS NOT NULL"; ps = txn.prepareStatement(sql); ps.setInt(1, UNKNOWN.getValue()); ps.setBytes(2, c.getBytes()); @@ -1453,7 +1469,7 @@ abstract class JdbcDatabase implements Database { + " ON m.messageId = s.messageId" + " AND gv.contactId = s.contactId" + " WHERE gv.contactId = ?" - + " AND valid = ? AND shared = TRUE" + + " AND valid = ? AND shared = TRUE AND raw IS NOT NULL" + " AND seen = FALSE AND requested = TRUE" + " AND s.expiry < ?" + " ORDER BY timestamp DESC"; diff --git a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java index 2fe48c6c3..ec0674e03 100644 --- a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java +++ b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java @@ -52,6 +52,8 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -1071,6 +1073,51 @@ public class H2DatabaseTest extends BriarTestCase { db.close(); } + @Test + public void testDeleteMessage() throws Exception { + Database db = open(false); + Connection txn = db.startTransaction(); + + // Add a contact, a group and a message + db.addLocalAuthor(txn, localAuthor); + assertEquals(contactId, db.addContact(txn, author, localAuthorId)); + db.addGroup(txn, group); + db.addVisibility(txn, contactId, groupId); + db.addMessage(txn, message, VALID, true); + db.addStatus(txn, contactId, messageId, false, false); + + // The message should be visible to the contact + assertTrue(db.containsVisibleMessage(txn, contactId, messageId)); + + // The message should be sendable + Collection ids = db.getMessagesToSend(txn, contactId, + ONE_MEGABYTE); + assertEquals(Collections.singletonList(messageId), ids); + ids = db.getMessagesToOffer(txn, contactId, 100); + assertEquals(Collections.singletonList(messageId), ids); + + // The raw message should not be null + assertNotNull(db.getRawMessage(txn, messageId)); + + // Delete the message + db.deleteMessage(txn, messageId); + + // The message should be visible to the contact + assertTrue(db.containsVisibleMessage(txn, contactId, messageId)); + + // The message should not be sendable + ids = db.getMessagesToSend(txn, contactId, ONE_MEGABYTE); + assertTrue(ids.isEmpty()); + ids = db.getMessagesToOffer(txn, contactId, 100); + assertTrue(ids.isEmpty()); + + // The raw message should be null + assertNull(db.getRawMessage(txn, messageId)); + + db.commitTransaction(txn); + db.close(); + } + @Test public void testExceptionHandling() throws Exception { Database db = open(false); From a91d500263fbe417ecc2a2fcef6f49185809dcfa Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 10 Feb 2016 13:58:46 +0000 Subject: [PATCH 2/2] Added method for deleting metadata. --- .../api/db/DatabaseComponent.java | 3 +++ .../src/org/briarproject/db/Database.java | 7 +++++++ .../db/DatabaseComponentImpl.java | 17 ++++++++++++++++ .../src/org/briarproject/db/JdbcDatabase.java | 16 +++++++++++++++ .../db/DatabaseComponentImplTest.java | 20 ++++++++++++++++--- .../org/briarproject/db/H2DatabaseTest.java | 11 ++++++++++ 6 files changed, 71 insertions(+), 3 deletions(-) diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java index 80fd14a7e..5609844f4 100644 --- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java +++ b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java @@ -69,6 +69,9 @@ public interface DatabaseComponent { */ void deleteMessage(MessageId m) throws DbException; + /** Deletes any metadata associated with the given message. */ + void deleteMessageMetadata(MessageId m) throws DbException; + /** * Returns an acknowledgement for the given contact, or null if there are * no messages to acknowledge. diff --git a/briar-core/src/org/briarproject/db/Database.java b/briar-core/src/org/briarproject/db/Database.java index d21e6c349..934156308 100644 --- a/briar-core/src/org/briarproject/db/Database.java +++ b/briar-core/src/org/briarproject/db/Database.java @@ -225,6 +225,13 @@ interface Database { */ void deleteMessage(T txn, MessageId m) throws DbException; + /** + * Deletes any metadata associated with the given message. + *

+ * Locking: write. + */ + void deleteMessageMetadata(T txn, MessageId m) throws DbException; + /** * Returns the contact with the given ID. *

diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index d55501a10..0a974ed8e 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -310,6 +310,23 @@ class DatabaseComponentImpl implements DatabaseComponent { } } + public void deleteMessageMetadata(MessageId m) throws DbException { + lock.writeLock().lock(); + try { + T txn = db.startTransaction(); + try { + if (!db.containsMessage(txn, m)) + throw new NoSuchMessageException(); + db.deleteMessageMetadata(txn, m); + } catch (DbException e) { + db.abortTransaction(txn); + throw e; + } + } finally { + lock.writeLock().unlock(); + } + } + public Ack generateAck(ContactId c, int maxMessages) throws DbException { Collection ids; lock.writeLock().lock(); diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index 2f32e8fd8..ac583594b 100644 --- a/briar-core/src/org/briarproject/db/JdbcDatabase.java +++ b/briar-core/src/org/briarproject/db/JdbcDatabase.java @@ -949,6 +949,22 @@ abstract class JdbcDatabase implements Database { } } + public void deleteMessageMetadata(Connection txn, MessageId m) + throws DbException { + PreparedStatement ps = null; + try { + String sql = "DELETE FROM messageMetadata WHERE messageId = ?"; + ps = txn.prepareStatement(sql); + ps.setBytes(1, m.getBytes()); + int affected = ps.executeUpdate(); + if (affected < 0) throw new DbStateException(); + ps.close(); + } catch (SQLException e) { + tryToClose(ps); + throw new DbException(e); + } + } + public Contact getContact(Connection txn, ContactId c) throws DbException { PreparedStatement ps = null; ResultSet rs = null; diff --git a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java index b0930805f..70a9113f2 100644 --- a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java +++ b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java @@ -543,11 +543,11 @@ public class DatabaseComponentImplTest extends BriarTestCase { final EventBus eventBus = context.mock(EventBus.class); context.checking(new Expectations() {{ // Check whether the message is in the DB (which it's not) - exactly(6).of(database).startTransaction(); + exactly(8).of(database).startTransaction(); will(returnValue(txn)); - exactly(6).of(database).containsMessage(txn, messageId); + exactly(8).of(database).containsMessage(txn, messageId); will(returnValue(false)); - exactly(6).of(database).abortTransaction(txn); + exactly(8).of(database).abortTransaction(txn); // This is needed for getMessageStatus() to proceed exactly(1).of(database).containsContact(txn, contactId); will(returnValue(true)); @@ -555,6 +555,20 @@ public class DatabaseComponentImplTest extends BriarTestCase { DatabaseComponent db = createDatabaseComponent(database, eventBus, shutdown); + try { + db.deleteMessage(messageId); + fail(); + } catch (NoSuchMessageException expected) { + // Expected + } + + try { + db.deleteMessageMetadata(messageId); + fail(); + } catch (NoSuchMessageException expected) { + // Expected + } + try { db.getRawMessage(messageId); fail(); diff --git a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java index ec0674e03..1e880356b 100644 --- a/briar-tests/src/org/briarproject/db/H2DatabaseTest.java +++ b/briar-tests/src/org/briarproject/db/H2DatabaseTest.java @@ -939,6 +939,17 @@ public class H2DatabaseTest extends BriarTestCase { assertTrue(retrieved.containsKey("baz")); assertArrayEquals(metadata.get("baz"), retrieved.get("baz")); + // Delete the metadata + db.deleteMessageMetadata(txn, messageId); + + // Retrieve the metadata again + retrieved = db.getMessageMetadata(txn, messageId); + assertTrue(retrieved.isEmpty()); + + // Retrieve the metadata for the group again + all = db.getMessageMetadata(txn, groupId); + assertTrue(all.isEmpty()); + db.commitTransaction(txn); db.close(); }