packages/moxxmpp/CHANGELOG.md

@@ -1,7 +1,3 @@
-## 0.3.2
-
-- **BREAKING**: Remove `lastResource` from `XmppConnection`'s `connect` method. Instead, set the `StreamManagementNegotiator`'s `resource` attribute instead. Since the resource can only really be restored by stream management, this is no issue.
-
 ## 0.3.1
 
 - Fix some issues with running moxxmpp as a component

packages/moxxmpp/lib/src/connection.dart

@@ -10,7 +10,6 @@ import 'package:moxxmpp/src/errors.dart';
 import 'package:moxxmpp/src/events.dart';
 import 'package:moxxmpp/src/handlers/base.dart';
 import 'package:moxxmpp/src/iq.dart';
-import 'package:moxxmpp/src/jid.dart';
 import 'package:moxxmpp/src/managers/attributes.dart';
 import 'package:moxxmpp/src/managers/base.dart';
 import 'package:moxxmpp/src/managers/data.dart';
@@ -141,8 +140,6 @@ class XmppConnection {
   RoutingState _routingState = RoutingState.preConnection;
 
   /// The currently bound resource or '' if none has been bound yet.
-  /// NOTE: A Using the empty string is okay since RFC7622 says that
-  ///       the resource MUST NOT be zero octets.
   String _resource = '';
   String get resource => _resource;
 
@@ -173,13 +170,6 @@ class XmppConnection {
 
   bool get isAuthenticated => _isAuthenticated;
 
-  /// Returns the JID we authenticate with and add the resource that we have bound.
-  JID _getJidWithResource() {
-    assert(_resource.isNotEmpty, 'The resource must not be empty');
-
-    return connectionSettings.jid.withResource(_resource);
-  }
-
   /// Registers a list of [XmppManagerBase] sub-classes as managers on this connection.
   Future<void> registerManagers(List<XmppManagerBase> managers) async {
     for (final manager in managers) {
@@ -191,7 +181,7 @@ class XmppConnection {
           sendEvent: _sendEvent,
           getConnectionSettings: () => connectionSettings,
           getManagerById: getManagerById,
-          getFullJID: _getJidWithResource,
+          getFullJID: () => connectionSettings.jid.withResource(_resource),
           getSocket: () => _socket,
           getConnection: () => this,
           getNegotiatorById: _negotiationsHandler.getNegotiatorById,
@@ -242,7 +232,7 @@ class XmppConnection {
           _sendEvent,
           _negotiationsHandler.getNegotiatorById,
           getManagerById,
-          _getJidWithResource,
+          () => connectionSettings.jid.withResource(_resource),
           () => _socket,
           () => _isAuthenticated,
           _setAuthenticated,
@@ -305,11 +295,8 @@ class XmppConnection {
     // Connect again
     // ignore: cascade_invocations
     _log.finest('Calling _connectImpl() from _attemptReconnection');
-    unawaited(
+    await _connectImpl(
-      _connectImpl(
       waitForConnection: true,
-        shouldReconnect: false,
-      ),
     );
   }
 
@@ -444,7 +431,7 @@ class XmppConnection {
         case StanzaFromType.full:
           {
             stanza_ = stanza_.copyWith(
-              from: _getJidWithResource().toString(),
+              from: connectionSettings.jid.withResource(_resource).toString(),
             );
           }
           break;
@@ -853,6 +840,7 @@ class XmppConnection {
   /// The private implementation for [XmppConnection.connect]. The parameters have
   /// the same meaning as with [XmppConnection.connect].
   Future<Result<bool, XmppError>> _connectImpl({
+    String? lastResource,
     bool waitForConnection = false,
     bool shouldReconnect = true,
     bool waitUntilLogin = false,
@@ -875,9 +863,11 @@ class XmppConnection {
       _connectionCompleter = Completer();
     }
 
-    // Reset the resource. If we use stream resumption from XEP-0198, then the
+    if (lastResource != null) {
-    // manager will set it on successful resumption.
+      setResource(lastResource, triggerEvent: false);
+    } else {
       setResource('', triggerEvent: false);
+    }
 
     // If requested, wait until we have a network connection
     if (waitForConnection) {
@@ -924,6 +914,9 @@ class XmppConnection {
 
   /// Start the connection process using the provided connection settings.
   ///
+  /// If [lastResource] is set, then its value is used as the connection's resource.
+  /// Useful for stream resumption.
+  ///
   /// [shouldReconnect] indicates whether the reconnection attempts should be
   /// automatically performed after a fatal failure of any kind occurs.
   ///
@@ -938,12 +931,14 @@ class XmppConnection {
   /// [enableReconnectOnSuccess] indicates that automatic reconnection is to be
   /// enabled once the connection has been successfully established.
   Future<Result<bool, XmppError>> connect({
+    String? lastResource,
     bool? shouldReconnect,
     bool waitForConnection = false,
     bool waitUntilLogin = false,
     bool enableReconnectOnSuccess = true,
   }) async {
     final result = _connectImpl(
+      lastResource: lastResource,
       shouldReconnect: shouldReconnect ?? !waitUntilLogin,
       waitForConnection: waitForConnection,
       waitUntilLogin: waitUntilLogin,

packages/moxxmpp/lib/src/reconnect.dart

@@ -111,47 +111,31 @@ class RandomBackoffReconnectionPolicy extends ReconnectionPolicy {
   /// Called when the backoff expired
   @visibleForTesting
   Future<void> onTimerElapsed() async {
-    _log.finest('Timer elapsed. Waiting for lock...');
+    _log.fine('Timer elapsed. Waiting for lock...');
-    final shouldContinue = await _timerLock.synchronized(() async {
+    await _timerLock.synchronized(() async {
-      _log.finest('Timer lock aquired');
-      if (_timer == null) {
-        _log.finest(
-          'The timer is already set to null. Doing nothing.',
-        );
-        return false;
-      }
-
       if (!(await getIsReconnecting())) {
-        return false;
+        return;
       }
 
       if (!(await getShouldReconnect())) {
-        _log.finest(
+        _log.fine(
           'Should not reconnect. Stopping here.',
         );
-        return false;
-      }
-
-      _timer?.cancel();
-      _timer = null;
-      return true;
-    });
-
-    if (!shouldContinue) {
         return;
       }
 
       _log.fine('Triggering reconnect');
+      _timer?.cancel();
+      _timer = null;
       await performReconnect!();
+    });
   }
 
   @override
   Future<void> reset() async {
     _log.finest('Resetting internal state');
-    await _timerLock.synchronized(() {
     _timer?.cancel();
     _timer = null;
-    });
     await super.reset();
   }
 
@@ -160,11 +144,9 @@ class RandomBackoffReconnectionPolicy extends ReconnectionPolicy {
     final seconds =
         Random().nextInt(_maxBackoffTime - _minBackoffTime) + _minBackoffTime;
     _log.finest('Failure occured. Starting random backoff with ${seconds}s');
-
-    await _timerLock.synchronized(() {
     _timer?.cancel();
+
     _timer = Timer(Duration(seconds: seconds), onTimerElapsed);
-    });
   }
 
   @override

packages/moxxmpp/lib/src/xeps/xep_0198/negotiator.dart

@@ -1,5 +1,6 @@
 import 'package:collection/collection.dart';
 import 'package:logging/logging.dart';
+import 'package:meta/meta.dart';
 import 'package:moxxmpp/src/events.dart';
 import 'package:moxxmpp/src/managers/namespaces.dart';
 import 'package:moxxmpp/src/namespaces.dart';
@@ -59,7 +60,11 @@ class StreamManagementNegotiator extends Sasl2FeatureNegotiator
   bool _inlineStreamEnablementRequested = false;
 
   /// Cached resource for stream resumption
-  String resource = '';
+  String _resource = '';
+  @visibleForTesting
+  void setResource(String resource) {
+    _resource = resource;
+  }
 
   @override
   bool canInlineFeature(List<XMLNode> features) {
@@ -85,7 +90,7 @@ class StreamManagementNegotiator extends Sasl2FeatureNegotiator
   @override
   Future<void> onXmppEvent(XmppEvent event) async {
     if (event is ResourceBoundEvent) {
-      resource = event.resource;
+      _resource = event.resource;
     }
   }
 
@@ -95,9 +100,7 @@ class StreamManagementNegotiator extends Sasl2FeatureNegotiator
 
     if (sm.state.streamResumptionId != null && !_resumeFailed) {
       // We could do Stream resumption
-      return super.matchesFeature(features) &&
+      return super.matchesFeature(features) && attributes.isAuthenticated();
-          attributes.isAuthenticated() &&
-          resource.isNotEmpty;
     } else {
       // We cannot do a stream resumption
       return super.matchesFeature(features) &&
@@ -119,23 +122,23 @@ class StreamManagementNegotiator extends Sasl2FeatureNegotiator
     _resumeFailed = true;
     _isResumed = false;
     _state = _StreamManagementNegotiatorState.ready;
-    resource = '';
-    attributes.setResource('', triggerEvent: false);
   }
 
   Future<void> _onStreamResumptionSuccessful(XMLNode resumed) async {
     assert(resumed.tag == 'resumed', 'The correct element must be passed');
-    assert(
-      resource.isNotEmpty,
-      'The Stream Management Negotiator must know of the previous resource',
-    );
 
     final h = int.parse(resumed.attributes['h']! as String);
     await attributes.sendEvent(StreamResumedEvent(h: h));
 
     _resumeFailed = false;
     _isResumed = true;
-    attributes.setResource(resource);
+
+    if (attributes.getConnection().resource.isEmpty && _resource.isNotEmpty) {
+      attributes.setResource(_resource);
+    } else if (attributes.getConnection().resource.isNotEmpty &&
+        _resource.isEmpty) {
+      _resource = attributes.getConnection().resource;
+    }
   }
 
   Future<void> _onStreamEnablementSuccessful(XMLNode enabled) async {
@@ -149,7 +152,7 @@ class StreamManagementNegotiator extends Sasl2FeatureNegotiator
 
     await attributes.sendEvent(
       StreamManagementEnabledEvent(
-        resource: resource,
+        resource: attributes.getFullJID().resource,
         id: id,
         location: enabled.attributes['location'] as String?,
       ),
@@ -194,12 +197,10 @@ class StreamManagementNegotiator extends Sasl2FeatureNegotiator
           _log.finest('Stream Management resumption successful');
 
           assert(
-            resource.isNotEmpty,
+            attributes.getFullJID().resource != '',
             'Resume only works when we already have a resource bound and know about it',
           );
 
-          // TODO(Unknown): Don't do this here. We trigger an event that the CSIManager
-          //                can consume.
           final csi = attributes.getManagerById(csiManager) as CSIManager?;
           if (csi != null) {
             csi.restoreCSIState();

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

@@ -42,6 +42,7 @@ const _doNotEncryptList = [
   DoNotEncrypt('stanza-id', stableIdXmlns),
 ];
 
+@mustCallSuper
 abstract class BaseOmemoManager extends XmppManagerBase {
   BaseOmemoManager() : super(omemoManager);
 

packages/moxxmpp/pubspec.yaml

@@ -1,6 +1,6 @@
 name: moxxmpp
 description: A pure-Dart XMPP library
-version: 0.3.2
+version: 0.3.1
 homepage: https://codeberg.org/moxxy/moxxmpp
 publish_to: https://git.polynom.me/api/packages/Moxxy/pub
 

packages/moxxmpp/test/reconnection_test.dart

@@ -1,4 +1,3 @@
-import 'dart:async';
 import 'package:moxxmpp/moxxmpp.dart';
 import 'package:test/test.dart';
 import 'helpers/logging.dart';
@@ -64,8 +63,6 @@ void main() {
       counter++;
     });
     await policy.setShouldReconnect(true);
-    // ignore: avoid_print
-    print('policy.setShouldReconnect(true) done');
 
     // We have a failure
     expect(
@@ -73,13 +70,9 @@ void main() {
       true,
     );
     await policy.onFailure();
-    // ignore: avoid_print
-    print('policy.onFailure() done');
 
-    unawaited(policy.onTimerElapsed());
+    await policy.onTimerElapsed();
-    unawaited(policy.onTimerElapsed());
+    await policy.onTimerElapsed();
-
-    await Future<void>.delayed(const Duration(seconds: 3));
     expect(counter, 1);
   });
 }

packages/moxxmpp/test/xeps/xep_0198_test.dart

@@ -514,7 +514,7 @@ void main() {
       await conn.registerFeatureNegotiators([
         SaslPlainNegotiator(),
         ResourceBindingNegotiator(),
-        StreamManagementNegotiator()..resource = 'test-resource',
+        StreamManagementNegotiator(),
       ]);
       await conn.getManagerById<StreamManagementManager>(smManager)!.setState(
             StreamManagementState(
@@ -536,15 +536,6 @@ void main() {
             .isStreamManagementEnabled(),
         true,
       );
-      expect(conn.resource, 'MU29eEZn');
-      expect(
-        conn
-            .getNegotiatorById<StreamManagementNegotiator>(
-              streamManagementNegotiator,
-            )!
-            .resource,
-        'MU29eEZn',
-      );
     });
 
     test('Test a successful stream resumption', () async {
@@ -613,7 +604,7 @@ void main() {
       await conn.registerFeatureNegotiators([
         SaslPlainNegotiator(),
         ResourceBindingNegotiator(),
-        StreamManagementNegotiator()..resource = 'abc123',
+        StreamManagementNegotiator(),
       ]);
       await conn.getManagerById<StreamManagementManager>(smManager)!.setState(
             StreamManagementState(
@@ -624,6 +615,7 @@ void main() {
           );
 
       await conn.connect(
+        lastResource: 'abc123',
         waitUntilLogin: true,
       );
       expect(fakeSocket.getState(), 4);
@@ -698,7 +690,7 @@ void main() {
     await conn.registerFeatureNegotiators([
       SaslPlainNegotiator(),
       ResourceBindingNegotiator(),
-      StreamManagementNegotiator()..resource = 'test-resource',
+      StreamManagementNegotiator()..setResource('test-resource'),
       Sasl2Negotiator(
         userAgent: const UserAgent(
           id: 'd4565fa7-4d72-4749-b3d3-740edbf87770',
@@ -795,7 +787,7 @@ void main() {
     await conn.registerFeatureNegotiators([
       SaslPlainNegotiator(),
       ResourceBindingNegotiator(),
-      StreamManagementNegotiator()..resource = 'test-resource',
+      StreamManagementNegotiator()..setResource('test-resource'),
       Bind2Negotiator(),
       Sasl2Negotiator(
         userAgent: const UserAgent(