diff --git a/lib/xmpp/connection.dart b/lib/xmpp/connection.dart index ad33d094..cc253687 100644 --- a/lib/xmpp/connection.dart +++ b/lib/xmpp/connection.dart @@ -23,6 +23,7 @@ import "package:moxxyv2/xmpp/xeps/xep_0030/cachemanager.dart"; import "package:moxxyv2/xmpp/xeps/xep_0352.dart"; import "package:moxxyv2/xmpp/xeps/xep_0198/xep_0198.dart"; +import "package:meta/meta.dart"; import "package:uuid/uuid.dart"; import "package:synchronized/synchronized.dart"; import "package:logging/logging.dart"; @@ -165,6 +166,11 @@ class XmppConnection { } List get serverFeatures => _serverFeatures; + + /// Return the registered feature negotiator that has id [id]. Returns null if + /// none can be found. + @visibleForTesting + XmppFeatureNegotiatorBase? getNegotiatorById(String id) => _featureNegotiators[id]; /// Registers an [XmppManagerBase] sub-class as a manager on this connection. /// [sortHandlers] should NOT be touched. It specified if the handler priorities @@ -183,7 +189,7 @@ class XmppConnection { getFullJID: () => _connectionSettings.jid.withResource(_resource), getSocket: () => _socket, getConnection: () => this, - getNegotiatorById: (id) => _featureNegotiators[id], + getNegotiatorById: getNegotiatorById, )); final id = manager.getId(); @@ -458,7 +464,7 @@ class XmppConnection { } } } - + /// Sets the routing state and logs the change void _updateRoutingState(RoutingState state) { _log.finest("Updating _routingState from $_routingState to $state"); @@ -571,12 +577,13 @@ class XmppConnection { /// Returns true if we can still negotiate. Returns false if no negotiator is /// matching and ready. bool _isNegotiationPossible(List features) { - return _getNextNegotiator(features, log: false) != null; + return getNextNegotiator(features, log: false) != null; } /// Returns the next negotiator that matches [features]. Returns null if none can be /// picked. If [log] is true, then the list of matching negotiators will be logged. - XmppFeatureNegotiatorBase? _getNextNegotiator(List features, {bool log = true}) { + @visibleForTesting + XmppFeatureNegotiatorBase? getNextNegotiator(List features, {bool log = true}) { final matchingNegotiators = _featureNegotiators.values .where( (XmppFeatureNegotiatorBase negotiator) { @@ -627,7 +634,7 @@ class XmppConnection { await _onNegotiationsDone(); } else { - _currentNegotiator = _getNextNegotiator(_streamFeatures); + _currentNegotiator = getNextNegotiator(_streamFeatures); _log.finest("Chose ${_currentNegotiator!.id} as next negotiator"); final fakeStanza = XMLNode( @@ -650,7 +657,7 @@ class XmppConnection { await _onNegotiationsDone(); } else { _log.finest('Picking new negotiator...'); - _currentNegotiator = _getNextNegotiator(_streamFeatures); + _currentNegotiator = getNextNegotiator(_streamFeatures); _log.finest("Chose $_currentNegotiator as next negotiator"); final fakeStanza = XMLNode( tag: "stream:features", @@ -663,7 +670,7 @@ class XmppConnection { } /// Called whenever we receive data that has been parsed as XML. - void handleXmlStream(XMLNode node) async { + Future handleXmlStream(XMLNode node) async { switch (_routingState) { case RoutingState.negotiating: if (_currentNegotiator != null) { @@ -682,7 +689,7 @@ class XmppConnection { // Mandatory features are done but can we still negotiate more? if (_isNegotiationPossible(node.children)) {// We can still negotiate features, so do that. _log.finest('All required stream features done! Continuing negotiation'); - _currentNegotiator = _getNextNegotiator(node.children); + _currentNegotiator = getNextNegotiator(node.children); _log.finest("Chose $_currentNegotiator as next negotiator"); await _currentNegotiator!.negotiate(node); await _checkCurrentNegotiator(); @@ -697,7 +704,7 @@ class XmppConnection { return; } - _currentNegotiator = _getNextNegotiator(node.children); + _currentNegotiator = getNextNegotiator(node.children); _log.finest("Chose $_currentNegotiator as next negotiator"); await _currentNegotiator!.negotiate(node); await _checkCurrentNegotiator(); diff --git a/test/negotiator_test.dart b/test/negotiator_test.dart new file mode 100644 index 00000000..0acb6a60 --- /dev/null +++ b/test/negotiator_test.dart @@ -0,0 +1,68 @@ +import "package:test/test.dart"; +import "package:moxxyv2/xmpp/stringxml.dart"; +import "package:moxxyv2/xmpp/connection.dart"; +import "package:moxxyv2/xmpp/reconnect.dart"; +import "package:moxxyv2/xmpp/presence.dart"; +import "package:moxxyv2/xmpp/negotiators/negotiator.dart"; + +import "helpers/logging.dart"; + +const exampleXmlns1 = "im:moxxy:example1"; +const exampleNamespace1 = "im.moxxy.test.example1"; +const exampleXmlns2 = "im:moxxy:example2"; +const exampleNamespace2 = "im.moxxy.test.example2"; + +class StubNegotiator1 extends XmppFeatureNegotiatorBase { + StubNegotiator1() : called = false, super(1, false, exampleXmlns1, exampleNamespace1); + + bool called; + + @override + Future negotiate(XMLNode nonza) async { + state = NegotiatorState.done; + } +} + +class StubNegotiator2 extends XmppFeatureNegotiatorBase { + StubNegotiator2() : called = false, super(10, false, exampleXmlns2, exampleNamespace2); + + bool called; + + @override + Future negotiate(XMLNode nonza) async { + called = true; + state = NegotiatorState.done; + } +} + +void main() { + initLogger(); + + final connection = XmppConnection(TestingReconnectionPolicy()) + ..registerFeatureNegotiators([ + StubNegotiator1(), + StubNegotiator2(), + ]) + ..registerManagers([ + PresenceManager(), + ]); + final features = [ + XMLNode.xmlns(tag: "example1", xmlns: exampleXmlns1), + XMLNode.xmlns(tag: "example2", xmlns: exampleXmlns2), + ]; + + test("Test the priority system", () { + expect(connection.getNextNegotiator(features)?.id, exampleNamespace2); + }); + + test("Test negotiating features with no stream restarts", () async { + // TODO: Use a simple connection setup to test this + final streamFeatures = XMLNode(tag: "stream:features", children: features); + await connection.handleXmlStream(streamFeatures); + + final negotiator1 = connection.getNegotiatorById(exampleNamespace1) as StubNegotiator1?; + final negotiator2 = connection.getNegotiatorById(exampleNamespace2) as StubNegotiator2?; + expect(negotiator1?.called, true); + expect(negotiator2?.called, true); + }); +}