From 56a462833ed90849bf464c7e365c1a7b66fe78d8 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 13 Feb 2023 10:45:13 +0100 Subject: [PATCH] in group chats corrections and reactions use different ids. we need to merge stubs --- .../android/xmpp/TransformationTest.java | 185 +++++++++++++++++- .../android/database/dao/MessageDao.java | 100 ++++++++-- .../android/database/model/Modification.java | 2 +- .../android/transformer/Transformer.java | 2 +- 4 files changed, 274 insertions(+), 15 deletions(-) diff --git a/src/androidTest/java/im/conversations/android/xmpp/TransformationTest.java b/src/androidTest/java/im/conversations/android/xmpp/TransformationTest.java index 7d8b778cd..a891a8d55 100644 --- a/src/androidTest/java/im/conversations/android/xmpp/TransformationTest.java +++ b/src/androidTest/java/im/conversations/android/xmpp/TransformationTest.java @@ -153,7 +153,7 @@ public class TransformationTest { final var message = Iterables.getOnlyElement(messages); final var onlyContent = Iterables.getOnlyElement(message.contents); - Assert.assertEquals(Modification.EDIT, message.modification); + Assert.assertEquals(Modification.CORRECTION, message.modification); Assert.assertEquals("Hi example!", onlyContent.body); } @@ -187,7 +187,188 @@ public class TransformationTest { final var message = Iterables.getOnlyElement(messages); final var onlyContent = Iterables.getOnlyElement(message.contents); - Assert.assertEquals(Modification.EDIT, message.modification); + Assert.assertEquals(Modification.CORRECTION, message.modification); Assert.assertEquals("Hi example!", onlyContent.body); } + + @Test + public void replacingReactions() { + final var group = Jid.ofEscaped("a@group.example.com"); + final var message = new Message(Message.Type.GROUPCHAT); + message.addExtension(new Body("Please give me a thumbs up")); + message.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of(message, Instant.now(), REMOTE, "stanza-a", "id-user-a")); + + final var reactionA = new Message(Message.Type.GROUPCHAT); + reactionA.setFrom(group.withResource("user-b")); + reactionA.addExtension(Reactions.to("stanza-a")).addExtension(new Reaction("N")); + this.transformer.transform( + Transformation.of(reactionA, Instant.now(), REMOTE, "stanza-b", "id-user-b")); + + final var reactionB = new Message(Message.Type.GROUPCHAT); + reactionB.setFrom(group.withResource("user-b")); + reactionB.addExtension(Reactions.to("stanza-a")).addExtension(new Reaction("Y")); + this.transformer.transform( + Transformation.of(reactionB, Instant.now(), REMOTE, "stanza-c", "id-user-b")); + + final var messages = database.messageDao().getMessages(1L); + Assert.assertEquals(1, messages.size()); + final var dbMessage = Iterables.getOnlyElement(messages); + Assert.assertEquals(1, dbMessage.reactions.size()); + } + + @Test + public void twoCorrectionsOneReactionBeforeOriginalInGroupChat() { + final var group = Jid.ofEscaped("a@group.example.com"); + final var ogStanzaId = "og-stanza-id"; + final var ogMessageId = "og-message-id"; + + // first correction + final var m1 = new Message(Message.Type.GROUPCHAT); + // m1.setId(ogMessageId); + m1.addExtension(new Body("Please give me an thumbs up")); + m1.addExtension(new Replace()).setId(ogMessageId); + m1.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of( + m1, + Instant.ofEpochMilli(2000), + REMOTE, + "irrelevant-stanza-id1", + "id-user-a")); + + // second correction + final var m2 = new Message(Message.Type.GROUPCHAT); + // m2.setId(ogMessageId); + m2.addExtension(new Body("Please give me a thumbs up")); + m2.addExtension(new Replace()).setId(ogMessageId); + m2.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of( + m2, + Instant.ofEpochMilli(3000), + REMOTE, + "irrelevant-stanza-id2", + "id-user-a")); + + // a reaction + final var reactionB = new Message(Message.Type.GROUPCHAT); + reactionB.setFrom(group.withResource("user-b")); + reactionB.addExtension(Reactions.to(ogStanzaId)).addExtension(new Reaction("Y")); + this.transformer.transform( + Transformation.of( + reactionB, Instant.now(), REMOTE, "irrelevant-stanza-id3", "id-user-b")); + + // the original message + final var m4 = new Message(Message.Type.GROUPCHAT); + m4.setId(ogMessageId); + m4.addExtension(new Body("Please give me thumbs up")); + m4.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of(m4, Instant.ofEpochMilli(1000), REMOTE, ogStanzaId, "id-user-a")); + + final var messages = database.messageDao().getMessages(1L); + Assert.assertEquals(1, messages.size()); + final var dbMessage = Iterables.getOnlyElement(messages); + Assert.assertEquals(1, dbMessage.reactions.size()); + Assert.assertEquals(Modification.CORRECTION, dbMessage.modification); + Assert.assertEquals( + "Please give me a thumbs up", Iterables.getOnlyElement(dbMessage.contents).body); + } + + @Test + public void twoReactionsOneCorrectionBeforeOriginalInGroupChat() { + final var group = Jid.ofEscaped("a@group.example.com"); + final var ogStanzaId = "og-stanza-id"; + final var ogMessageId = "og-message-id"; + + // first reaction + final var reactionA = new Message(Message.Type.GROUPCHAT); + reactionA.setFrom(group.withResource("user-b")); + reactionA.addExtension(Reactions.to(ogStanzaId)).addExtension(new Reaction("Y")); + this.transformer.transform( + Transformation.of( + reactionA, Instant.now(), REMOTE, "irrelevant-stanza-id1", "id-user-b")); + + // second reaction + final var reactionB = new Message(Message.Type.GROUPCHAT); + reactionB.setFrom(group.withResource("user-c")); + reactionB.addExtension(Reactions.to(ogStanzaId)).addExtension(new Reaction("Y")); + this.transformer.transform( + Transformation.of( + reactionB, Instant.now(), REMOTE, "irrelevant-stanza-id2", "id-user-c")); + + // a correction + final var m1 = new Message(Message.Type.GROUPCHAT); + m1.addExtension(new Body("Please give me a thumbs up")); + m1.addExtension(new Replace()).setId(ogMessageId); + m1.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of( + m1, + Instant.ofEpochMilli(2000), + REMOTE, + "irrelevant-stanza-id3", + "id-user-a")); + + // the original message + final var m4 = new Message(Message.Type.GROUPCHAT); + m4.setId(ogMessageId); + m4.addExtension(new Body("Please give me thumbs up (Typo)")); + m4.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of(m4, Instant.ofEpochMilli(1000), REMOTE, ogStanzaId, "id-user-a")); + + final var messages = database.messageDao().getMessages(1L); + Assert.assertEquals(1, messages.size()); + final var dbMessage = Iterables.getOnlyElement(messages); + Assert.assertEquals(2, dbMessage.reactions.size()); + final var onlyReaction = Iterables.getOnlyElement(dbMessage.getAggregatedReactions()); + Assert.assertEquals(2L, (long) onlyReaction.getValue()); + Assert.assertEquals(Modification.CORRECTION, dbMessage.modification); + Assert.assertEquals( + "Please give me a thumbs up", Iterables.getOnlyElement(dbMessage.contents).body); + } + + @Test + public void twoReactionsInGroupChat() { + final var group = Jid.ofEscaped("a@group.example.com"); + final var ogStanzaId = "og-stanza-id"; + final var ogMessageId = "og-message-id"; + + // the original message + final var m4 = new Message(Message.Type.GROUPCHAT); + m4.setId(ogMessageId); + m4.addExtension(new Body("Please give me a thumbs up")); + m4.setFrom(group.withResource("user-a")); + this.transformer.transform( + Transformation.of(m4, Instant.ofEpochMilli(1000), REMOTE, ogStanzaId, "id-user-a")); + + // first reaction + final var reactionA = new Message(Message.Type.GROUPCHAT); + reactionA.setFrom(group.withResource("user-b")); + reactionA.addExtension(Reactions.to(ogStanzaId)).addExtension(new Reaction("Y")); + this.transformer.transform( + Transformation.of( + reactionA, Instant.now(), REMOTE, "irrelevant-stanza-id1", "id-user-b")); + + // second reaction + final var reactionB = new Message(Message.Type.GROUPCHAT); + reactionB.setFrom(group.withResource("user-c")); + reactionB.addExtension(Reactions.to(ogStanzaId)).addExtension(new Reaction("Y")); + this.transformer.transform( + Transformation.of( + reactionB, Instant.now(), REMOTE, "irrelevant-stanza-id2", "id-user-c")); + + final var messages = database.messageDao().getMessages(1L); + Assert.assertEquals(1, messages.size()); + final var dbMessage = Iterables.getOnlyElement(messages); + Assert.assertEquals(2, dbMessage.reactions.size()); + final var onlyReaction = Iterables.getOnlyElement(dbMessage.getAggregatedReactions()); + Assert.assertEquals(2L, (long) onlyReaction.getValue()); + Assert.assertEquals(Modification.ORIGINAL, dbMessage.modification); + Assert.assertEquals( + "Please give me a thumbs up", Iterables.getOnlyElement(dbMessage.contents).body); + } } diff --git a/src/main/java/im/conversations/android/database/dao/MessageDao.java b/src/main/java/im/conversations/android/database/dao/MessageDao.java index e414f642b..d5d5b0cce 100644 --- a/src/main/java/im/conversations/android/database/dao/MessageDao.java +++ b/src/main/java/im/conversations/android/database/dao/MessageDao.java @@ -74,10 +74,14 @@ public abstract class MessageDao { get( chatIdentifier.id, transformation.fromBare(), + transformation.occupantId, transformation.stanzaId, transformation.messageId); if (messageIdentifier != null) { if (messageIdentifier.isStub()) { + if (transformation.type == Message.Type.GROUPCHAT) { + mergeMessageStubs(chatIdentifier, messageIdentifier, transformation); + } LOGGER.info( "Found stub for stanzaId '{}' and messageId '{}'", transformation.stanzaId, @@ -93,9 +97,11 @@ public abstract class MessageDao { updatedEntity.id = messageIdentifier.id; updatedEntity.latestVersion = getLatestVersion(messageIdentifier.id); LOGGER.info( - "Created original version {} and updated latest version to {}", + "Created original version {} and updated latest version to {} for" + + " messageEntityId {}", originalVersionId, - updatedEntity.latestVersion); + updatedEntity.latestVersion, + messageIdentifier.id); update(updatedEntity); return new MessageIdentifier( updatedEntity.id, @@ -128,6 +134,52 @@ public abstract class MessageDao { messageVersionId); } + private void mergeMessageStubs( + ChatIdentifier chatIdentifier, + MessageIdentifier messageIdentifier, + final Transformation transformation) { + final Long stub; + if (messageIdentifier.messageId == null && transformation.messageId != null) { + stub = getMessageStubByMessageId(chatIdentifier.id, transformation.messageId); + } else if (messageIdentifier.stanzaId == null && transformation.stanzaId != null) { + stub = getMessageStubByStanzaId(chatIdentifier.id, transformation.stanzaId); + } else { + return; + } + if (stub == null) { + return; + } + LOGGER.info("Updating message.id in dangling stub {} => {}", stub, messageIdentifier.id); + updateMessageEntityIdInReactions(stub, messageIdentifier.id); + updateMessageEntityIdInVersions(stub, messageIdentifier.id); + deleteMessageEntity(stub); + } + + @Query( + "UPDATE message_reaction SET messageEntityId=:newMessageEntityId WHERE" + + " messageEntityId=:oldMessageEntityId") + protected abstract void updateMessageEntityIdInReactions( + long oldMessageEntityId, Long newMessageEntityId); + + @Query( + "UPDATE message_version SET messageEntityId=:newMessageEntityId WHERE" + + " messageEntityId=:oldMessageEntityId") + protected abstract void updateMessageEntityIdInVersions( + long oldMessageEntityId, Long newMessageEntityId); + + @Query("DELETE FROM message WHERE id=:messageEntityId") + protected abstract void deleteMessageEntity(final long messageEntityId); + + @Query( + "SELECT id FROM message WHERE chatId=:chatId AND messageId=:messageId AND stanzaId IS" + + " NULL AND latestVersion IS NULL") + protected abstract Long getMessageStubByMessageId(long chatId, String messageId); + + @Query( + "SELECT id FROM message WHERE chatId=:chatId AND stanzaId=:stanzaId AND messageId IS" + + " NULL AND latestVersion IS NULL") + protected abstract Long getMessageStubByStanzaId(long chatId, String stanzaId); + // this gets either a message or a stub. // stubs are recognized by latestVersion=NULL // when found by stanzaId the stanzaId must either by verified or belonging to a stub @@ -135,11 +187,12 @@ public abstract class MessageDao { // we only look up stubs @Query( "SELECT id,stanzaId,messageId,fromBare,latestVersion as version FROM message WHERE" - + " chatId=:chatId AND (fromBare=:fromBare OR fromBare IS NULL) AND ((stanzaId IS" - + " NOT NULL AND stanzaId=:stanzaId AND (stanzaIdVerified=1 OR latestVersion IS" - + " NULL)) OR (stanzaId IS NULL AND messageId=:messageId AND latestVersion IS" - + " NULL))") - abstract MessageIdentifier get(long chatId, Jid fromBare, String stanzaId, String messageId); + + " chatId=:chatId AND (fromBare=:fromBare OR fromBare IS NULL) AND" + + " (occupantId=:occupantId OR occupantId IS NULL) AND ((stanzaId IS NOT NULL AND" + + " stanzaId=:stanzaId AND (stanzaIdVerified=1 OR latestVersion IS NULL)) OR" + + " (stanzaId IS NULL AND messageId=:messageId AND latestVersion IS NULL))") + abstract MessageIdentifier get( + long chatId, Jid fromBare, String occupantId, String stanzaId, String messageId); public MessageIdentifier getOrCreateVersion( ChatIdentifier chat, @@ -149,15 +202,19 @@ public abstract class MessageDao { Preconditions.checkArgument( messageId != null, "A modification must reference a message id"); final MessageIdentifier messageIdentifier; - if (transformation.occupantId == null) { - messageIdentifier = getByMessageId(chat.id, transformation.fromBare(), messageId); - } else { + // TODO use type for condition and then null check occupantID + if (transformation.type == Message.Type.GROUPCHAT) { + Preconditions.checkNotNull( + transformation.occupantId, + "To create a version of a group chat message occupant id must be set"); messageIdentifier = getByOccupantIdAndMessageId( chat.id, transformation.fromBare(), transformation.occupantId, messageId); + } else { + messageIdentifier = getByMessageId(chat.id, transformation.fromBare(), messageId); } if (messageIdentifier == null) { LOGGER.info( @@ -310,13 +367,34 @@ public abstract class MessageDao { final Message.Type messageType = transformation.type; final MessageIdentifier messageIdentifier = getOrCreateStub(chat, messageType, reactions.getId()); - // TODO delete old reactions + if (messageType == Message.Type.GROUPCHAT) { + Preconditions.checkNotNull( + transformation.occupantId, + "OccupantId must not be null when processing reactions in group chats"); + deleteReactionsByOccupantId(messageIdentifier.id, transformation.occupantId); + } else { + deleteReactionsByFromBare(messageIdentifier.id, transformation.fromBare()); + } + LOGGER.info( + "Inserting reaction from {} to messageEntityId {}", + transformation.from, + messageIdentifier.id); insertReactions( Collections2.transform( reactions.getReactions(), r -> MessageReactionEntity.of(messageIdentifier.id, r, transformation))); } + @Query( + "DELETE FROM message_reaction WHERE messageEntityId=:messageEntityId AND" + + " occupantId=:occupantId") + protected abstract void deleteReactionsByOccupantId(long messageEntityId, String occupantId); + + @Query( + "DELETE FROM message_reaction WHERE messageEntityId=:messageEntityId AND" + + " reactionBy=:fromBare") + protected abstract void deleteReactionsByFromBare(long messageEntityId, Jid fromBare); + @Transaction @Query( "SELECT message.id as" diff --git a/src/main/java/im/conversations/android/database/model/Modification.java b/src/main/java/im/conversations/android/database/model/Modification.java index e5bc2709f..207e11c4b 100644 --- a/src/main/java/im/conversations/android/database/model/Modification.java +++ b/src/main/java/im/conversations/android/database/model/Modification.java @@ -2,7 +2,7 @@ package im.conversations.android.database.model; public enum Modification { ORIGINAL, - EDIT, // XEP-0308: Last Message Correction + CORRECTION, // XEP-0308: Last Message Correction RETRACTION, // XEP-0424: Message Retraction MODERATION // XEP-0425: Message Moderation } diff --git a/src/main/java/im/conversations/android/transformer/Transformer.java b/src/main/java/im/conversations/android/transformer/Transformer.java index 80ab3844b..92707b409 100644 --- a/src/main/java/im/conversations/android/transformer/Transformer.java +++ b/src/main/java/im/conversations/android/transformer/Transformer.java @@ -102,7 +102,7 @@ public class Transformer { chat, transformation, messageCorrection.getId(), - Modification.EDIT); + Modification.CORRECTION); } else { messageIdentifier =