From c7200910c9e69c731f4e54c79fc492fb5a9fe7fc Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 28 Oct 2019 09:45:41 -0300 Subject: [PATCH] [core] address feedback for selective conversation message deletion --- .../introduction/IntroductionManagerImpl.java | 20 +++++++++---------- .../GroupInvitationManagerImpl.java | 14 ++++++------- .../briar/sharing/SharingManagerImpl.java | 12 +++++------ .../IntroductionIntegrationTest.java | 3 ++- .../MessagingManagerIntegrationTest.java | 2 +- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java index 3677faf9e..0fdb774ca 100644 --- a/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/introduction/IntroductionManagerImpl.java @@ -561,8 +561,8 @@ class IntroductionManagerImpl extends ConversationClientImpl @FunctionalInterface private interface MessageRetriever { - Map getMessages(GroupId contactGroup) - throws DbException; + Map getMessages(Transaction txn, + GroupId contactGroup) throws DbException; } @FunctionalInterface @@ -572,17 +572,17 @@ class IntroductionManagerImpl extends ConversationClientImpl * It returns true if the given {@link MessageId} causes a problem * so that the session can not be deleted. */ - boolean causesProblem(MessageId messageId) throws DbException; + boolean causesProblem(MessageId messageId); } @Override public boolean deleteAllMessages(Transaction txn, ContactId c) throws DbException { - return deleteMessages(txn, c, g -> { + return deleteMessages(txn, c, (txn1, g) -> { // get metadata for all messages in the group Map messages; try { - messages = clientHelper.getMessageMetadataAsDictionary(txn, g); + messages = clientHelper.getMessageMetadataAsDictionary(txn1, g); } catch (FormatException e) { throw new DbException(e); } @@ -593,14 +593,14 @@ class IntroductionManagerImpl extends ConversationClientImpl @Override public boolean deleteMessages(Transaction txn, ContactId c, Set messageIds) throws DbException { - return deleteMessages(txn, c, g -> { + return deleteMessages(txn, c, (txn1, g) -> { // get metadata for messages that shall be deleted Map messages = new HashMap<>(messageIds.size()); for (MessageId m : messageIds) { BdfDictionary d; try { - d = clientHelper.getMessageMetadataAsDictionary(txn, m); + d = clientHelper.getMessageMetadataAsDictionary(txn1, m); } catch (FormatException e) { throw new DbException(e); } @@ -620,16 +620,14 @@ class IntroductionManagerImpl extends ConversationClientImpl GroupId g = getContactGroup(db.getContact(txn, c)).getId(); // get messages to be deleted - Map messages = retriever.getMessages(g); + Map messages = retriever.getMessages(txn, g); // assign protocol messages to their sessions Map sessions = new HashMap<>(); for (Entry entry : messages.entrySet()) { - BdfDictionary d = entry.getValue(); - if (d == null) continue; // throw new NoSuchMessageException() MessageMetadata m; try { - m = messageParser.parseMetadata(d); + m = messageParser.parseMetadata(entry.getValue()); } catch (FormatException e) { throw new DbException(e); } diff --git a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java index 39472e684..fdacd865e 100644 --- a/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/privategroup/invitation/GroupInvitationManagerImpl.java @@ -620,7 +620,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl @FunctionalInterface private interface DeletableSessionRetriever { - Map getDeletableSessions( + Map getDeletableSessions(Transaction txn, GroupId contactGroup, Map metadata) throws DbException; } @@ -632,13 +632,13 @@ class GroupInvitationManagerImpl extends ConversationClientImpl * It returns true if the given {@link MessageId} causes a problem * so that the session can not be deleted. */ - boolean causesProblem(MessageId messageId) throws DbException; + boolean causesProblem(MessageId messageId); } @Override public boolean deleteAllMessages(Transaction txn, ContactId c) throws DbException { - return deleteMessages(txn, c, (g, metadata) -> { + return deleteMessages(txn, c, (txn1, g, metadata) -> { // get all sessions and their states Map sessions = new HashMap<>(); for (BdfDictionary d : metadata.values()) { @@ -659,7 +659,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl @Override public boolean deleteMessages(Transaction txn, ContactId c, Set messageIds) throws DbException { - return deleteMessages(txn, c, (g, metadata) -> { + return deleteMessages(txn, c, (txn1, g, metadata) -> { // get only sessions from given messageIds Map sessions = new HashMap<>(); for (MessageId messageId : messageIds) { @@ -670,7 +670,7 @@ class GroupInvitationManagerImpl extends ConversationClientImpl messageParser.parseMetadata(d); SessionId sessionId = getSessionId(messageMetadata.getPrivateGroupId()); - StoredSession ss = getSession(txn, g, sessionId); + StoredSession ss = getSession(txn1, g, sessionId); if (ss == null) throw new DbException(); Session session = sessionParser .parseSession(g, metadata.get(ss.storageId)); @@ -700,9 +700,9 @@ class GroupInvitationManagerImpl extends ConversationClientImpl throw new DbException(e); } - // get all sessions and their states + // get sessions and their states from retriever Map sessions = - retriever.getDeletableSessions(g, metadata); + retriever.getDeletableSessions(txn, g, metadata); // assign protocol messages to their sessions for (Entry entry : metadata.entrySet()) { diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java index b998d57b3..4f7ce604a 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/SharingManagerImpl.java @@ -541,7 +541,7 @@ abstract class SharingManagerImpl @FunctionalInterface private interface DeletableSessionRetriever { - Map getDeletableSessions( + Map getDeletableSessions(Transaction txn, GroupId contactGroup, Map metadata) throws DbException; } @@ -553,13 +553,13 @@ abstract class SharingManagerImpl * It returns true if the given {@link MessageId} causes a problem * so that the session can not be deleted. */ - boolean causesProblem(MessageId messageId) throws DbException; + boolean causesProblem(MessageId messageId); } @Override public boolean deleteAllMessages(Transaction txn, ContactId c) throws DbException { - return deleteMessages(txn, c, (contactGroup, metadata) -> { + return deleteMessages(txn, c, (txn1, contactGroup, metadata) -> { // get all sessions and their states Map sessions = new HashMap<>(); for (BdfDictionary d : metadata.values()) { @@ -580,7 +580,7 @@ abstract class SharingManagerImpl @Override public boolean deleteMessages(Transaction txn, ContactId c, Set messageIds) throws DbException { - return deleteMessages(txn, c, (g, metadata) -> { + return deleteMessages(txn, c, (txn1, g, metadata) -> { // get only sessions from given messageIds Map sessions = new HashMap<>(); for (MessageId messageId : messageIds) { @@ -591,7 +591,7 @@ abstract class SharingManagerImpl messageParser.parseMetadata(d); SessionId sessionId = getSessionId(messageMetadata.getShareableId()); - StoredSession ss = getSession(txn, g, sessionId); + StoredSession ss = getSession(txn1, g, sessionId); if (ss == null) throw new DbException(); Session session = sessionParser .parseSession(g, metadata.get(ss.storageId)); @@ -623,7 +623,7 @@ abstract class SharingManagerImpl // get all sessions and their states Map sessions = - retriever.getDeletableSessions(g, metadata); + retriever.getDeletableSessions(txn, g, metadata); // assign protocol messages to their sessions for (Entry entry : metadata.entrySet()) { diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java index beff1dac0..b53b6465d 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/IntroductionIntegrationTest.java @@ -1470,7 +1470,8 @@ public class IntroductionIntegrationTest sync0To2(1, true); eventWaiter.await(TIMEOUT, 1); - // deleting introduction also fails for introducees + // deleting introduction fails for introducees, + // because response is not yet selected for deletion assertFalse(deleteMessages0From1(toDelete1)); assertFalse(deleteMessages0From2(toDelete2)); diff --git a/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java index f23bd3334..aae06972c 100644 --- a/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/messaging/MessagingManagerIntegrationTest.java @@ -56,7 +56,7 @@ public class MessagingManagerIntegrationTest messagingManager0 = c0.getMessagingManager(); messagingManager1 = c1.getMessagingManager(); messageFactory = c0.getPrivateMessageFactory(); - assertEquals(contact0From1, contact1From0); + assertEquals(contactId0From1, contactId1From0); contactId = contactId0From1; }