fix: Fix message quote generation

Also fixes an invalid test case in the XEP-0198 tests, where
the IQ reply sets the "to" instead of the "from".

packages/moxxmpp/lib/src/connection.dart

@@ -1,6 +1,7 @@
 import 'dart:async';
 import 'package:logging/logging.dart';
 import 'package:meta/meta.dart';
+import 'package:moxlib/moxlib.dart';
 import 'package:moxxmpp/src/buffer.dart';
 import 'package:moxxmpp/src/errors.dart';
 import 'package:moxxmpp/src/events.dart';
@@ -72,6 +73,23 @@ class XmppConnectionResult {
   final XmppError? error;
 }
 
+@immutable
+class _StanzaAwaitableData {
+  const _StanzaAwaitableData(this.sentTo, this.id);
+  final String sentTo;
+  final String id;
+
+  @override
+  int get hashCode => sentTo.hashCode ^ id.hashCode;
+
+  @override
+  bool operator==(Object other) {
+    return other is _StanzaAwaitableData &&
+           other.sentTo == sentTo &&
+           other.id == id;
+  }
+}
+
 class XmppConnection {
   /// [_socket] is for debugging purposes.
   /// [connectionPingDuration] is the duration after which a ping will be sent to keep
@@ -128,7 +146,7 @@ class XmppConnection {
   /// A policy on how to reconnect 
   final ReconnectionPolicy _reconnectionPolicy;
   /// A list of stanzas we are tracking with its corresponding critical section
-  final Map<String, Completer<XMLNode>> _awaitingResponse;
+  final Map<_StanzaAwaitableData, Completer<XMLNode>> _awaitingResponse;
   final Lock _awaitingResponseLock;
   
   /// Helpers
@@ -428,9 +446,10 @@ class XmppConnection {
   /// none.
   // TODO(Unknown): if addId = false, the function crashes.
   Future<XMLNode> sendStanza(Stanza stanza, { StanzaFromType addFrom = StanzaFromType.full, bool addId = true, bool awaitable = true, bool encrypted = false }) async {
-    var stanza_ = stanza;
-    
+    assert(implies(addId == false && stanza.id == null, !awaitable), 'Cannot await a stanza with no id');
+
     // Add extra data in case it was not set
+    var stanza_ = stanza;
     if (addId && (stanza_.id == null || stanza_.id == '')) {
       stanza_ = stanza.copyWith(id: generateId());
     }
@@ -448,8 +467,6 @@ class XmppConnection {
       }
     }
 
-    final id = stanza_.id!;
-
     _log.fine('Running pre stanza handlers..');
     final data = await _runOutgoingPreStanzaHandlers(
       stanza_,
@@ -487,42 +504,45 @@ class XmppConnection {
     final stanzaString = data.stanza.toXml();
 
     // ignore: cascade_invocations
-    _log.fine('Attempting to acquire lock for $id...');
+    _log.fine('Attempting to acquire lock for ${data.stanza.id}...');
     // TODO(PapaTutuWawa): Handle this much more graceful
     var future = Future.value(XMLNode(tag: 'not-used'));
     await _awaitingResponseLock.synchronized(() async {
-        _log.fine('Lock acquired for $id');
-        if (awaitable) {
-          _awaitingResponse[id] = Completer();
-        }
+      _log.fine('Lock acquired for ${data.stanza.id}');
 
-        // This uses the StreamManager to behave like a send queue
-        if (await _canSendData()) {
-          _socket.write(stanzaString);
+      _StanzaAwaitableData? key;
+      if (awaitable) {
+        key = _StanzaAwaitableData(data.stanza.to!, data.stanza.id!);
+        _awaitingResponse[key] = Completer();
+      }
 
-          // Try to ack every stanza
-          // NOTE: Here we have send an Ack request nonza. This is now done by StreamManagementManager when receiving the StanzaSentEvent
-        } else {
-          _log.fine('_canSendData() returned false.');
-        }
+      // This uses the StreamManager to behave like a send queue
+      if (await _canSendData()) {
+        _socket.write(stanzaString);
 
-        _log.fine('Running post stanza handlers..');
-        await _runOutgoingPostStanzaHandlers(
+        // Try to ack every stanza
+        // NOTE: Here we have send an Ack request nonza. This is now done by StreamManagementManager when receiving the StanzaSentEvent
+      } else {
+        _log.fine('_canSendData() returned false.');
+      }
+
+      _log.fine('Running post stanza handlers..');
+      await _runOutgoingPostStanzaHandlers(
+        stanza_,
+        initial: StanzaHandlerData(
+          false,
+          false,
+          null,
           stanza_,
-          initial: StanzaHandlerData(
-            false,
-            false,
-            null,
-            stanza_,
-          ),
-        );
-        _log.fine('Done');
+        ),
+      );
+      _log.fine('Done');
 
-        if (awaitable) {
-          future = _awaitingResponse[id]!.future;
-        }
+      if (awaitable) {
+        future = _awaitingResponse[key]!.future;
+      }
 
-        _log.fine('Releasing lock for $id');
+      _log.fine('Releasing lock for ${data.stanza.id}');
     });
 
     return future;
@@ -691,10 +711,14 @@ class XmppConnection {
     final id = incomingPreHandlers.stanza.attributes['id'] as String?;
     var awaited = false;
     await _awaitingResponseLock.synchronized(() async {
-      if (id != null && _awaitingResponse.containsKey(id)) {
-        _awaitingResponse[id]!.complete(incomingPreHandlers.stanza);
-        _awaitingResponse.remove(id);
-        awaited = true;
+      if (id != null && incomingPreHandlers.stanza.from != null) {
+        final key = _StanzaAwaitableData(incomingPreHandlers.stanza.from!, id);
+        final comp = _awaitingResponse[key];
+        if (comp != null) {
+          comp.complete(incomingPreHandlers.stanza);
+          _awaitingResponse.remove(key);
+          awaited = true;
+        }
       }
     });
 

packages/moxxmpp/lib/src/message.dart

@@ -1,3 +1,4 @@
+import 'package:moxlib/moxlib.dart';
 import 'package:moxxmpp/src/events.dart';
 import 'package:moxxmpp/src/jid.dart';
 import 'package:moxxmpp/src/managers/base.dart';
@@ -20,6 +21,7 @@ import 'package:moxxmpp/src/xeps/xep_0444.dart';
 import 'package:moxxmpp/src/xeps/xep_0446.dart';
 import 'package:moxxmpp/src/xeps/xep_0447.dart';
 import 'package:moxxmpp/src/xeps/xep_0448.dart';
+import 'package:moxxmpp/src/xeps/xep_0461.dart';
 
 /// Data used to build a message stanza.
 ///
@@ -140,6 +142,11 @@ class MessageManager extends XmppManagerBase {
   /// element to this id. If originId is non-null, then it will create an "origin-id"
   /// child in the message stanza and set its id to originId.
   void sendMessage(MessageDetails details) {
+    assert(
+      implies(details.quoteBody != null, details.quoteFrom != null && details.quoteId != null),
+      'When quoting a message, then quoteFrom and quoteId must also be non-null',
+    );
+
     final stanza = Stanza.message(
       to: details.to,
       type: 'chat',
@@ -148,11 +155,11 @@ class MessageManager extends XmppManagerBase {
     );
 
     if (details.quoteBody != null) {
-      final fallback = '> ${details.quoteBody!}';
-
+      final quote = QuoteData.fromBodies(details.quoteBody!, details.body!);
+        
       stanza
         ..addChild(
-          XMLNode(tag: 'body', text: '$fallback\n${details.body}'),
+          XMLNode(tag: 'body', text: quote.body),
         )
         ..addChild(
           XMLNode.xmlns(
@@ -176,7 +183,7 @@ class MessageManager extends XmppManagerBase {
                 tag: 'body',
                 attributes: <String, String>{
                   'start': '0',
-                  'end': '${fallback.length}'
+                  'end': '${quote.fallbackLength}',
                 },
               )
             ],

packages/moxxmpp/lib/src/xeps/xep_0461.dart

@@ -1,3 +1,4 @@
+import 'package:meta/meta.dart';
 import 'package:moxxmpp/src/managers/base.dart';
 import 'package:moxxmpp/src/managers/data.dart';
 import 'package:moxxmpp/src/managers/handlers.dart';
@@ -5,6 +6,7 @@ import 'package:moxxmpp/src/managers/namespaces.dart';
 import 'package:moxxmpp/src/namespaces.dart';
 import 'package:moxxmpp/src/stanza.dart';
 
+/// Data summarizing the XEP-0461 data.
 class ReplyData {
   const ReplyData({
     required this.to,
@@ -12,12 +14,57 @@ class ReplyData {
     this.start,
     this.end,
   });
+
+  /// The bare JID to whom the reply applies to
   final String to;
+
+  /// The stanza ID of the message that is replied to
   final String id;
+
+  /// The start of the fallback body (inclusive)
   final int? start;
+
+  /// The end of the fallback body (exclusive)
   final int? end;
+
+  /// Applies the metadata to the received body [body] in order to remove the fallback.
+  /// If either [ReplyData.start] or [ReplyData.end] are null, then body is returned as
+  /// is.
+  String removeFallback(String body) {
+    if (start == null || end == null) return body;
+
+    return body.replaceRange(start!, end, '');
+  }
 }
 
+/// Internal class describing how to build a message with a quote fallback body.
+@visibleForTesting
+class QuoteData {
+  const QuoteData(this.body, this.fallbackLength);
+
+  /// Takes the body of the message we want to quote [quoteBody] and the content of
+  /// the reply [body] and computes the fallback body and its length.
+  factory QuoteData.fromBodies(String quoteBody, String body) {
+    final fallback = quoteBody
+      .split('\n')
+      .map((line) => '> $line\n')
+      .join();
+
+    return QuoteData(
+      '$fallback$body',
+      fallback.length,
+    );
+  }
+  
+  /// The new body with fallback data at the beginning
+  final String body;
+
+  /// The length of the fallback data
+  final int fallbackLength;
+}
+
+/// A manager implementing support for parsing XEP-0461 metadata. The
+/// MessageRepliesManager itself does not modify the body of the message.
 class MessageRepliesManager extends XmppManagerBase {
   @override
   String getName() => 'MessageRepliesManager';

packages/moxxmpp/test/xeps/xep_0198_test.dart

@@ -348,7 +348,7 @@ void main() {
           ),
           StanzaExpectation(
             "<iq to='user@example.com' type='get' id='a' xmlns='jabber:client' />",
-            "<iq to='user@example.com' type='result' id='a' />",
+            "<iq from='user@example.com' type='result' id='a' />",
             ignoreId: true,
             adjustId: true,
           ),

packages/moxxmpp/test/xeps/xep_0461_test.dart

@@ -0,0 +1,44 @@
+import 'package:moxxmpp/moxxmpp.dart';
+import 'package:test/test.dart';
+
+void main() {
+  test('Test building a singleline quote', () {
+    final quote = QuoteData.fromBodies('Hallo Welt', 'Hello Earth!');
+
+    expect(quote.body, '> Hallo Welt\nHello Earth!');
+    expect(quote.fallbackLength, 13);
+  });
+
+  test('Test building a multiline quote', () {
+    final quote = QuoteData.fromBodies('Hallo Welt\nHallo Erde', 'How are you?');
+
+    expect(quote.body, '> Hallo Welt\n> Hallo Erde\nHow are you?');
+    expect(quote.fallbackLength, 26);
+  });
+
+  test('Applying a singleline quote', () {
+    final body = '> Hallo Welt\nHello right back!';
+    final reply = ReplyData(
+      to: '',
+      id: '',
+      start: 0,
+      end: 13,
+    );
+
+    final bodyWithoutFallback = reply.removeFallback(body);
+    expect(bodyWithoutFallback, 'Hello right back!');
+  });
+
+  test('Applying a multiline quote', () {
+    final body = "> Hallo Welt\n> How are you?\nI'm fine.\nThank you!";
+    final reply = ReplyData(
+      to: '',
+      id: '',
+      start: 0,
+      end: 28,
+    );
+
+    final bodyWithoutFallback = reply.removeFallback(body);
+    expect(bodyWithoutFallback, "I'm fine.\nThank you!");
+  });
+}