feat(service): Remove shared media entry on message retraction

Also: Fix some ordering oddities with the shared media widget and page.

lib/service/database/creation.dart

@@ -84,7 +84,9 @@ Future<void> createDatabase(Database db, int version) async {
       mime TEXT,
       timestamp INTEGER NOT NULL,
       conversation_id INTEGER NOT NULL,
-      FOREIGN KEY (conversation_id) REFERENCES $conversationsTable (id)
+      message_id INTEGER,
+      FOREIGN KEY (conversation_id) REFERENCES $conversationsTable (id),
+      FOREIGN KEY (message_id) REFERENCES $messsagesTable (id)
     )''',
   );
 

lib/service/database/database.dart

@@ -11,6 +11,7 @@ import 'package:moxxyv2/service/database/helpers.dart';
 import 'package:moxxyv2/service/database/migrations/0000_language.dart';
 import 'package:moxxyv2/service/database/migrations/0000_retraction.dart';
 import 'package:moxxyv2/service/database/migrations/0000_retraction_conversation.dart';
+import 'package:moxxyv2/service/database/migrations/0000_shared_media.dart';
 import 'package:moxxyv2/service/database/migrations/0000_xmpp_state.dart';
 import 'package:moxxyv2/service/not_specified.dart';
 import 'package:moxxyv2/service/omemo/omemo.dart';
@@ -57,7 +58,7 @@ class DatabaseService {
     _db = await openDatabase(
       dbPath,
       password: key,
-      version: 5,
+      version: 6,
       onCreate: createDatabase,
       onConfigure: configureDatabase,
       onUpgrade: (db, oldVersion, newVersion) async {
@@ -77,6 +78,10 @@ class DatabaseService {
           _log.finest('Running migration for database version 5');
           await upgradeFromV4ToV5(db);
         }
+        if (oldVersion < 6) {
+          _log.finest('Running migration for database version 6');
+          await upgradeFromV5ToV6(db);
+        }
       },
     );
 
@@ -257,18 +262,29 @@ class DatabaseService {
   }
 
   /// Like [addConversationFromData] but for [SharedMedium].
-  Future<SharedMedium> addSharedMediumFromData(String path, int timestamp, int conversationId, { String? mime }) async {
+  Future<SharedMedium> addSharedMediumFromData(String path, int timestamp, int conversationId, int messageId, { String? mime }) async {
     final s = SharedMedium(
       -1,
       path,
       timestamp,
       mime: mime,
+      messageId: messageId,
     );
 
     return s.copyWith(
       id: await _db.insert('SharedMedia', s.toDatabaseJson(conversationId)),
     );
   }
+
+  /// Remove a SharedMedium from the database based on the message it
+  /// references [messageId].
+  Future<void> removeSharedMediumByMessageId(int messageId) async {
+    await _db.delete(
+      mediaTable,
+      where: 'message_id = ?',
+      whereArgs: [messageId],
+    );
+  }
   
   /// Same as [addConversationFromData] but for a [Message].
   Future<Message> addMessageFromData(

lib/service/database/migrations/0000_shared_media.dart

@@ -0,0 +1,10 @@
+import 'package:moxxyv2/service/database/constants.dart';
+import 'package:moxxyv2/shared/models/preference.dart';
+import 'package:sqflite_sqlcipher/sqflite.dart';
+
+Future<void> upgradeFromV5ToV6(Database db) async {
+  // Allow shared media to reference a message
+  await db.execute(
+    'ALTER TABLE $mediaTable ADD COLUMN message_id INTEGER REFERENCES $messsagesTable (id);',
+  );
+}

lib/service/events.dart

@@ -579,16 +579,13 @@ Future<void> performRegenerateOwnDevice(RegenerateOwnDeviceCommand command, { dy
 }
 
 Future<void> performMessageRetraction(RetractMessageComment command, { dynamic extra }) async {
-  final msg = await GetIt.I.get<DatabaseService>().getMessageByOriginId(
-    command.originId,
+  await GetIt.I.get<MessageService>().retractMessage( 
     command.conversationJid,
+    command.originId,
+    '',
+    true,
   );
 
-  if (msg == null) {
-    GetIt.I.get<Logger>().warning('Failed to find message ${command.conversationJid}#${command.originId} for message retraction');
-    return;
-  }
-
   // Send the retraction
   (GetIt.I.get<XmppConnection>().getManagerById(messageManager)! as MessageManager)
     .sendMessage(
@@ -600,37 +597,4 @@ Future<void> performMessageRetraction(RetractMessageComment command, { dynamic e
         ),
       ),
     );
-  
-  // Update the database
-  final retractedMessage = await GetIt.I.get<MessageService>().updateMessage(
-    msg.id,
-    isMedia: false,
-    mediaUrl: null,
-    mediaType: null,
-    warningType: null,
-    errorType: null,
-    srcUrl: null,
-    key: null,
-    iv: null,
-    encryptionScheme: null,
-    mediaWidth: null,
-    mediaHeight: null,
-    mediaSize: null,
-    isRetracted: true,
-    thumbnailData: null,
-  );
-  sendEvent(MessageUpdatedEvent(message: retractedMessage));
-
-  final cs = GetIt.I.get<ConversationService>();
-  final conversation = await cs.getConversationByJid(
-    command.conversationJid,
-  );
-  if (conversation != null && conversation.lastMessageId == msg.id) {
-    final newConversation = await cs.updateConversation(
-      conversation.id,
-      lastMessageBody: '',
-      lastMessageRetracted: true,
-    );
-    sendEvent(ConversationUpdatedEvent(conversation: newConversation));
-  }
 }

lib/service/httpfiletransfer/httpfiletransfer.dart

@@ -24,7 +24,6 @@ import 'package:moxxyv2/service/notifications.dart';
 import 'package:moxxyv2/service/service.dart';
 import 'package:moxxyv2/shared/error_types.dart';
 import 'package:moxxyv2/shared/events.dart';
-import 'package:moxxyv2/shared/models/media.dart';
 import 'package:moxxyv2/shared/warning_types.dart';
 import 'package:path/path.dart' as pathlib;
 import 'package:path_provider/path_provider.dart';
@@ -485,11 +484,16 @@ class HttpFileTransferService {
         downloadedPath,
         msg.timestamp,
         conv.id,
+        job.mId,
         mime: mime,
       );
       final newConv = conv.copyWith(
-        sharedMedia: List<SharedMedium>.from(conv.sharedMedia)..add(sharedMedium),
+        sharedMedia: [
+          sharedMedium,
+          ...conv.sharedMedia,
+        ],
       );
+      GetIt.I.get<ConversationService>().setConversation(newConv);
       sendEvent(ConversationUpdatedEvent(conversation: newConv));
     }
 

lib/service/message.dart

@@ -2,8 +2,13 @@ import 'dart:collection';
 import 'package:get_it/get_it.dart';
 import 'package:logging/logging.dart';
 import 'package:moxlib/moxlib.dart';
+import 'package:moxxmpp/moxxmpp.dart';
+import 'package:moxxyv2/service/conversation.dart';
 import 'package:moxxyv2/service/database/database.dart';
 import 'package:moxxyv2/service/not_specified.dart';
+import 'package:moxxyv2/service/service.dart';
+import 'package:moxxyv2/shared/events.dart';
+import 'package:moxxyv2/shared/models/media.dart';
 import 'package:moxxyv2/shared/models/message.dart';
 
 class MessageService {
@@ -182,4 +187,88 @@ class MessageService {
     
     return newMessage;
   }
+
+  /// Helper function that manages everything related to retracting a message. It
+  /// - Replaces all metadata of the message with null values and marks it as retracted
+  /// - Modified the conversation, if the retracted message was the newest message
+  /// - Remove the SharedMedium from the database, if one referenced the retracted message
+  /// - Update the UI
+  ///
+  /// [conversationJid] is the bare JID of the conversation this message belongs to.
+  /// [originId] is the origin Id of the message that is to be retracted.
+  /// [bareSender] is the bare JID of the sender of the retraction message.
+  /// [selfRetract] indicates whether the message retraction came from the UI. If true,
+  /// then the sender check (see security considerations of XEP-0424) is skipped as
+  /// the UI already verifies it.
+  Future<void> retractMessage(String conversationJid, String originId, String bareSender, bool selfRetract) async {
+    final msg = await GetIt.I.get<DatabaseService>().getMessageByOriginId(
+      originId,
+      conversationJid,
+    );
+
+    if (msg == null) {
+      _log.finest('Got message retraction for origin Id $originId, but did not find the message');
+      return;
+    }
+
+    // Check if the retraction was sent by the original sender
+    if (!selfRetract) {
+      if (JID.fromString(msg.sender).toBare().toString() != bareSender) {
+        _log.warning('Received invalid message retraction from $bareSender but its original sender is ${msg.sender}');
+        return;
+      }
+    }
+
+    final isMedia = msg.isMedia;
+    final retractedMessage = await updateMessage(
+      msg.id,
+      isMedia: false,
+      mediaUrl: null,
+      mediaType: null,
+      warningType: null,
+      errorType: null,
+      srcUrl: null,
+      key: null,
+      iv: null,
+      encryptionScheme: null,
+      mediaWidth: null,
+      mediaHeight: null,
+      mediaSize: null,
+      isRetracted: true,
+      thumbnailData: null,
+    );
+    sendEvent(MessageUpdatedEvent(message: retractedMessage));
+
+    final cs = GetIt.I.get<ConversationService>();
+    final conversation = await cs.getConversationByJid(conversationJid);
+    if (conversation != null) {
+      if (conversation.lastMessageId == msg.id) {
+        var newConversation = await cs.updateConversation(
+          conversation.id,
+          lastMessageBody: '',
+          lastMessageRetracted: true,
+        );
+
+        if (isMedia) {
+          // TODO(PapaTutuWawa): Delete the file referenced by the shared media entry
+          await GetIt.I.get<DatabaseService>().removeSharedMediumByMessageId(msg.id);
+
+          newConversation = newConversation.copyWith(
+            sharedMedia: newConversation.sharedMedia.where((SharedMedium medium) {
+              return medium.messageId != msg.id;
+            }).toList(),
+          );
+          GetIt.I.get<ConversationService>().setConversation(newConversation);
+        }
+
+        sendEvent(
+          ConversationUpdatedEvent(
+            conversation: newConversation,
+          ),
+        );
+      }      
+    } else {
+      _log.warning('Failed to find conversation with conversationJid $conversationJid');
+    }
+  }
 }

lib/service/xmpp.dart

@@ -306,7 +306,14 @@ class XmppService {
     return GetIt.I.get<XmppConnection>().connectAwaitable(lastResource: lastResource);
   }
 
-  Future<List<SharedMedium>> _createSharedMedia(List<String> paths, int conversationId) async {
+  /// Wrapper function for creating shared media entries for the given paths.
+  /// [messages] is the mapping of "File path -> Recipient -> Message" required for
+  /// setting the single shared medium's message Id attribute.
+  /// [paths] is the list of paths to create shared media entries for.
+  /// [recipient] is the bare string JID that the messages will be sent to.
+  /// [conversationId] is the database id of the conversation these shared media entries
+  /// belong to.
+  Future<List<SharedMedium>> _createSharedMedia(Map<String, Map<String, Message>> messages, List<String> paths, String recipient, int conversationId) async {
     final sharedMedia = List<SharedMedium>.empty(growable: true);
     for (final path in paths) {
       sharedMedia.add(
@@ -314,6 +321,7 @@ class XmppService {
           path,
           DateTime.now().millisecondsSinceEpoch,
           conversationId,
+          messages[path]![recipient]!.id,
           mime: lookupMimeType(path),
         ),
       );
@@ -400,7 +408,7 @@ class XmppService {
       final conversation = await cs.getConversationByJid(recipient);
       if (conversation != null) {
         // Update conversation
-        final updatedConversation = await cs.updateConversation(
+        var updatedConversation = await cs.updateConversation(
           conversation.id,
           lastMessageBody: mimeTypeToEmoji(lastFileMime),
           lastMessageId: lastMessageIds[recipient],
@@ -408,21 +416,20 @@ class XmppService {
           open: true,
         );
 
-        sharedMediaMap[recipient] = await _createSharedMedia(paths, conversation.id);
-        sendEvent(
-          ConversationUpdatedEvent(
-            conversation: updatedConversation.copyWith(
-              sharedMedia: [
-                ...sharedMediaMap[recipient]!,
-                ...conversation.sharedMedia,
-              ],
-            ),
-          ),
+        sharedMediaMap[recipient] = await _createSharedMedia(messages, paths, recipient, conversation.id);
+
+        updatedConversation = updatedConversation.copyWith(
+          sharedMedia: [
+            ...sharedMediaMap[recipient]!,
+            ...conversation.sharedMedia,
+          ],
         );
+        cs.setConversation(updatedConversation);
+        sendEvent(ConversationUpdatedEvent(conversation: updatedConversation));
       } else {
         // Create conversation
         final rosterItem = await rs.getRosterItemByJid(recipient);
-        final newConversation = await cs.addConversationFromData(
+        var newConversation = await cs.addConversationFromData(
           // TODO(Unknown): Should we use the JID parser?
           rosterItem?.title ?? recipient.split('@').first,
           lastMessageIds[recipient]!,
@@ -437,15 +444,14 @@ class XmppService {
           prefs.enableOmemoByDefault,
         );
 
-        // Notify the UI
-        sharedMediaMap[recipient] = await _createSharedMedia(paths, newConversation.id);
-        sendEvent(
-          ConversationAddedEvent(
-            conversation: newConversation.copyWith(
-              sharedMedia: sharedMediaMap[recipient]!,
-            ),
-          ),
+        sharedMediaMap[recipient] = await _createSharedMedia(messages, paths, recipient, newConversation.id);
+        newConversation = newConversation.copyWith(
+          sharedMedia: sharedMediaMap[recipient]!,
         );
+        cs.setConversation(newConversation);
+
+        // Notify the UI
+        sendEvent(ConversationAddedEvent(conversation: newConversation));
       }
     }
 
@@ -754,55 +760,12 @@ class XmppService {
 
   /// Handle a message retraction given the MessageEvent [event].
   Future<void> _handleMessageRetraction(MessageEvent event, String conversationJid) async {
-    final msg = await GetIt.I.get<DatabaseService>().getMessageByOriginId(
-      event.messageRetraction!.id,
+    await GetIt.I.get<MessageService>().retractMessage(
       conversationJid,
+      event.messageRetraction!.id,
+      event.fromJid.toBare().toString(),
+      false,
     );
-
-    if (msg == null) {
-      _log.finest('Got message retraction for origin Id ${event.messageRetraction!.id}, but did not find the message');
-      return;
-    }
-
-    // Check if the retraction was sent by the original sender
-    if (JID.fromString(msg.sender).toBare().toString() != event.fromJid.toBare().toString()) {
-      _log.warning('Received invalid message retraction from ${event.fromJid.toBare().toString()} but its original sender is ${msg.sender}');
-      return;
-    }
-    
-    final retractedMessage = await GetIt.I.get<MessageService>().updateMessage(
-      msg.id,
-      isMedia: false,
-      mediaUrl: null,
-      mediaType: null,
-      warningType: null,
-      errorType: null,
-      srcUrl: null,
-      key: null,
-      iv: null,
-      encryptionScheme: null,
-      mediaWidth: null,
-      mediaHeight: null,
-      mediaSize: null,
-      isRetracted: true,
-      thumbnailData: null,
-    );
-    sendEvent(MessageUpdatedEvent(message: retractedMessage));
-
-    final cs = GetIt.I.get<ConversationService>();
-    final conversation = await cs.getConversationByJid(conversationJid);
-    if (conversation != null) {
-      if (conversation.lastMessageId == msg.id) {
-        final newConversation = await cs.updateConversation(
-          conversation.id,
-          lastMessageBody: '',
-          lastMessageRetracted: true,
-        );
-        sendEvent(ConversationUpdatedEvent(conversation: newConversation));
-      }
-    } else {
-      _log.warning('Failed to find conversation with conversationJid $conversationJid');
-    }
   }
   
   /// Returns true if a file should be automatically downloaded. If it should not, it

lib/shared/cache.dart

@@ -53,6 +53,12 @@ class LRUCache<K, V> extends Cache<K, V> {
     if (_cache.length + 1 <= _maxSize) {
       // Fall through
     } else {
+      if (inCache(key)) {
+        _cache[key] = _LRUCacheEntry<V>(value, _t);
+        _t++;
+        return;
+      }
+
       var lowestKey = _cache.keys.first;
       var t = _cache[lowestKey]!.t;
       _cache

lib/shared/models/media.dart

@@ -9,7 +9,10 @@ class SharedMedium with _$SharedMedium {
     int id,
     String path,
     int timestamp,
-    { String? mime, }
+    {
+      String? mime,
+      int? messageId,
+    }
   ) = _SharedMedia;
 
   const SharedMedium._();
@@ -18,14 +21,19 @@ class SharedMedium with _$SharedMedium {
   factory SharedMedium.fromJson(Map<String, dynamic> json) => _$SharedMediumFromJson(json);
 
   factory SharedMedium.fromDatabaseJson(Map<String, dynamic> json) {
-    return SharedMedium.fromJson(json);
+    return SharedMedium.fromJson({
+      ...json,
+      'messageId': json['message_id'] as int?,
+    });
   }
 
   Map<String, dynamic> toDatabaseJson(int conversationId) {
     return {
       ...toJson()
-        ..remove('id'),
+        ..remove('id')
+        ..remove('messageId'),
       'conversation_id': conversationId,
+      'message_id': messageId,
     };
   }
 }

lib/ui/pages/conversation/topbar.dart

@@ -52,7 +52,8 @@ class ConversationTopbar extends StatelessWidget implements PreferredSizeWidget
       || prev.conversation?.avatarUrl != next.conversation?.avatarUrl
       || prev.conversation?.chatState != next.conversation?.chatState
       || prev.conversation?.jid != next.conversation?.jid
-      || prev.conversation?.encrypted != next.conversation?.encrypted;
+      || prev.conversation?.encrypted != next.conversation?.encrypted
+      || prev.conversation?.sharedMedia != next.conversation?.sharedMedia;
   }
   
   Widget _buildChatState(ChatState state) {

lib/ui/pages/profile/selfheader.dart

@@ -27,7 +27,7 @@ class SelfProfileHeader extends StatelessWidget {
       context: context,
       builder: (BuildContext context) => Center(
         child: ClipRRect(
-          borderRadius: BorderRadius.all(radiusLarge),
+          borderRadius: const BorderRadius.all(radiusLarge),
           child: SizedBox(
             width: 220,
             height: 220,

lib/ui/pages/sharedmedia.dart

@@ -28,11 +28,11 @@ class SharedMediaPage extends StatelessWidget {
 
         final rows = List<List<SharedMedium>>.empty(growable: true);
         var currentRow = List<SharedMedium>.empty(growable: true);
-        for (var i = state.sharedMedia.length - 1; i >= 0; i--) {
+        for (var i = 0; i <= state.sharedMedia.length - 1; i++) {
           final item = state.sharedMedia[i];
           final thisMediaDateTime = DateTime.fromMillisecondsSinceEpoch(item.timestamp);
-          final lastMediaDateTime = i < state.sharedMedia.length - 1 ?
-            DateTime.fromMillisecondsSinceEpoch(state.sharedMedia[i + 1].timestamp) :
+          final lastMediaDateTime = i > 0 ?
+            DateTime.fromMillisecondsSinceEpoch(state.sharedMedia[i - 1].timestamp) :
             null;
 
           final newDay = lastMediaDateTime != null && (

lib/ui/widgets/chat/shared/media.dart

@@ -11,10 +11,10 @@ class SharedMediaDisplay extends StatelessWidget {
   List<Widget> _renderItems() {
     final tmp = List<Widget>.empty(growable: true);
 
-    final clampedStartIndex = sharedMedia.length >= 8 ? sharedMedia.length - 7 : 0;
-    final clampedEndIndex = sharedMedia.length - 1;
-
-    for (var i = clampedEndIndex; i >= clampedStartIndex; i--) {
+    // NOTE: 6, since that lets us iterate from 0 to 6 (7 elements), thus leaving
+    //       one space for the summary button
+    final clampedEndIndex = sharedMedia.length >= 8 ? 6 : sharedMedia.length - 1;
+    for (var i = 0; i <= clampedEndIndex; i++) {
       tmp.add(buildSharedMediaWidget(sharedMedia[i], jid));
     }
 

test/cache_test.dart

@@ -15,5 +15,10 @@ void main() {
       expect(cache.inCache('a'), false);
       expect(cache.inCache('b'), true);
       expect(cache.inCache('c'), true);
+
+      cache.cache('c', 4);
+      expect(cache.inCache('b'), true);
+      expect(cache.inCache('c'), true);
+      expect(cache.getValue('c'), 4);
   });
 }