From f460e5ebe984fc693cd8b33a0bc69e4df890a937 Mon Sep 17 00:00:00 2001 From: "Alexander \"PapaTutuWawa" Date: Sat, 1 Apr 2023 12:28:11 +0200 Subject: [PATCH] feat(core): Handle less resource binding in the core connection class --- packages/moxxmpp/CHANGELOG.md | 2 ++ packages/moxxmpp/lib/src/connection.dart | 27 ++++++------------- packages/moxxmpp/lib/src/events.dart | 6 +++-- .../moxxmpp/lib/src/managers/attributes.dart | 4 --- packages/moxxmpp/lib/src/managers/base.dart | 24 +++++++++++++++++ .../lib/src/negotiators/negotiator.dart | 5 ++++ .../lib/src/negotiators/resource_binding.dart | 10 +++---- packages/moxxmpp/test/helpers/manager.dart | 1 - packages/moxxmpp/test/sasl/scram_test.dart | 5 ++++ packages/moxxmpp/test/xeps/xep_0198_test.dart | 1 - packages/moxxmpp/test/xeps/xep_0280_test.dart | 1 - packages/moxxmpp/test/xeps/xep_0352_test.dart | 2 -- packages/moxxmpp/test/xmpp_test.dart | 3 +-- 13 files changed, 53 insertions(+), 38 deletions(-) diff --git a/packages/moxxmpp/CHANGELOG.md b/packages/moxxmpp/CHANGELOG.md index 4bf403d..34fa8c7 100644 --- a/packages/moxxmpp/CHANGELOG.md +++ b/packages/moxxmpp/CHANGELOG.md @@ -3,6 +3,8 @@ - **BREAKING**: Removed `connectAwaitable` and merged it with `connect`. - **BREAKING**: Removed `allowPlainAuth` from `ConnectionSettings`. If you don't want to use SASL PLAIN, don't register the negotiator. If you want to only conditionally use SASL PLAIN, extend the `SaslPlainNegotiator` and override its `matchesFeature` method to only call the super method when SASL PLAIN should be used. - **BREAKING**: The user avatar's `subscribe` and `unsubscribe` no longer subscribe to the `:data` PubSub nodes +- Renamed `ResourceBindingSuccessEvent` to `ResourceBoundEvent` +- **BREAKING**: Removed `isFeatureSupported` from the manager attributes. The managers now all have a method `isFeatureSupported` that works the same ## 0.1.6+1 diff --git a/packages/moxxmpp/lib/src/connection.dart b/packages/moxxmpp/lib/src/connection.dart index f2ff348..0cef5c0 100644 --- a/packages/moxxmpp/lib/src/connection.dart +++ b/packages/moxxmpp/lib/src/connection.dart @@ -133,9 +133,6 @@ class XmppConnection { StreamController.broadcast(); final Map _xmppManagers = {}; - /// Disco info we got after binding a resource (xmlns) - final List _serverFeatures = List.empty(growable: true); - /// The buffer object to keep split up stanzas together final XmlStreamBuffer _streamBuffer = XmlStreamBuffer(); @@ -150,6 +147,7 @@ class XmppConnection { /// The currently bound resource or '' if none has been bound yet. String _resource = ''; + String get resource => _resource; /// True if we are authenticated. False if not. bool _isAuthenticated = false; @@ -201,8 +199,6 @@ class XmppConnection { ReconnectionPolicy get reconnectionPolicy => _reconnectionPolicy; - List get serverFeatures => _serverFeatures; - bool get isAuthenticated => _isAuthenticated; /// Return the registered feature negotiator that has id [id]. Returns null if @@ -221,7 +217,6 @@ class XmppConnection { sendEvent: _sendEvent, getConnectionSettings: () => _connectionSettings, getManagerById: getManagerById, - isFeatureSupported: _serverFeatures.contains, getFullJID: () => _connectionSettings.jid.withResource(_resource), getSocket: () => _socket, getConnection: () => this, @@ -283,6 +278,7 @@ class XmppConnection { () => _socket, () => _isAuthenticated, _setAuthenticated, + _setResource, _removeNegotiatingFeature, ), ); @@ -678,9 +674,13 @@ class XmppConnection { } /// Sets the resource of the connection - void setResource(String resource) { + void _setResource(String resource, {bool triggerEvent = true}) { _log.finest('Updating _resource to $resource'); _resource = resource; + + if (triggerEvent) { + _sendEvent(ResourceBoundEvent(resource)); + } } /// Returns the connection's events as a stream. @@ -1028,17 +1028,6 @@ class XmppConnection { Future _sendEvent(XmppEvent event) async { _log.finest('Event: ${event.toString()}'); - // Specific event handling - if (event is ResourceBindingSuccessEvent) { - _log.finest( - 'Received ResourceBindingSuccessEvent. Setting _resource to ${event.resource}', - ); - setResource(event.resource); - - _log.finest('Resetting _serverFeatures'); - _serverFeatures.clear(); - } - for (final manager in _xmppManagers.values) { await manager.onXmppEvent(event); } @@ -1151,7 +1140,7 @@ class XmppConnection { } if (lastResource != null) { - setResource(lastResource); + _setResource(lastResource, triggerEvent: false); } _enableReconnectOnSuccess = enableReconnectOnSuccess; diff --git a/packages/moxxmpp/lib/src/events.dart b/packages/moxxmpp/lib/src/events.dart index 9a8f9ea..44bbc87 100644 --- a/packages/moxxmpp/lib/src/events.dart +++ b/packages/moxxmpp/lib/src/events.dart @@ -160,8 +160,10 @@ class StreamManagementEnabledEvent extends XmppEvent { } /// Triggered when we bound a resource -class ResourceBindingSuccessEvent extends XmppEvent { - ResourceBindingSuccessEvent({required this.resource}); +class ResourceBoundEvent extends XmppEvent { + ResourceBoundEvent(this.resource); + + /// The resource that was just bound. final String resource; } diff --git a/packages/moxxmpp/lib/src/managers/attributes.dart b/packages/moxxmpp/lib/src/managers/attributes.dart index 9143d58..508c7a8 100644 --- a/packages/moxxmpp/lib/src/managers/attributes.dart +++ b/packages/moxxmpp/lib/src/managers/attributes.dart @@ -16,7 +16,6 @@ class XmppManagerAttributes { required this.getManagerById, required this.sendEvent, required this.getConnectionSettings, - required this.isFeatureSupported, required this.getFullJID, required this.getSocket, required this.getConnection, @@ -45,9 +44,6 @@ class XmppManagerAttributes { /// (Maybe) Get a Manager attached to the connection by its Id. final T? Function(String) getManagerById; - /// Returns true if a server feature is supported - final bool Function(String) isFeatureSupported; - /// Returns the full JID of the current account final JID Function() getFullJID; diff --git a/packages/moxxmpp/lib/src/managers/base.dart b/packages/moxxmpp/lib/src/managers/base.dart index 8d0ebb1..8170675 100644 --- a/packages/moxxmpp/lib/src/managers/base.dart +++ b/packages/moxxmpp/lib/src/managers/base.dart @@ -6,6 +6,7 @@ import 'package:moxxmpp/src/managers/data.dart'; import 'package:moxxmpp/src/managers/handlers.dart'; import 'package:moxxmpp/src/managers/namespaces.dart'; import 'package:moxxmpp/src/stringxml.dart'; +import 'package:moxxmpp/src/xeps/xep_0030/errors.dart'; import 'package:moxxmpp/src/xeps/xep_0030/types.dart'; import 'package:moxxmpp/src/xeps/xep_0030/xep_0030.dart'; import 'package:moxxmpp/src/xeps/xep_0198/xep_0198.dart'; @@ -31,6 +32,29 @@ abstract class XmppManagerBase { return _managerAttributes; } + /// Resolves to true when the server supports the disco feature [xmlns]. Resolves + /// to false when either the disco request fails or the server does not + /// support [xmlns]. + /// Note that this function requires a registered DiscoManager. + @protected + Future isFeatureSupported(String xmlns) async { + final dm = _managerAttributes.getManagerById(discoManager); + assert( + dm != null, + 'The DiscoManager must be registered for isFeatureSupported to work', + ); + + final result = await dm!.discoInfoQuery( + _managerAttributes.getConnectionSettings().jid.domain, + shouldEncrypt: false, + ); + if (result.isType()) { + return false; + } + + return result.get().features.contains(xmlns); + } + /// Return the StanzaHandlers associated with this manager that deal with stanzas we /// send. These are run before the stanza is sent. The higher the value of the /// handler's priority, the earlier it is run. diff --git a/packages/moxxmpp/lib/src/negotiators/negotiator.dart b/packages/moxxmpp/lib/src/negotiators/negotiator.dart index d60c4a6..9658e95 100644 --- a/packages/moxxmpp/lib/src/negotiators/negotiator.dart +++ b/packages/moxxmpp/lib/src/negotiators/negotiator.dart @@ -36,6 +36,7 @@ class NegotiatorAttributes { this.getSocket, this.isAuthenticated, this.setAuthenticated, + this.setResource, this.removeNegotiatingFeature, ); @@ -64,6 +65,10 @@ class NegotiatorAttributes { /// Returns true if the stream is authenticated. Returns false if not. final bool Function() isAuthenticated; + /// Sets the resource of the connection. If triggerEvent is true, then a + /// [ResourceBoundEvent] is triggered. + final void Function(String, {bool triggerEvent}) setResource; + /// Sets the authentication state of the connection to true. final void Function() setAuthenticated; diff --git a/packages/moxxmpp/lib/src/negotiators/resource_binding.dart b/packages/moxxmpp/lib/src/negotiators/resource_binding.dart index e60eee0..6c6ffd2 100644 --- a/packages/moxxmpp/lib/src/negotiators/resource_binding.dart +++ b/packages/moxxmpp/lib/src/negotiators/resource_binding.dart @@ -1,4 +1,4 @@ -import 'package:moxxmpp/src/events.dart'; +import 'package:moxxmpp/src/jid.dart'; import 'package:moxxmpp/src/managers/namespaces.dart'; import 'package:moxxmpp/src/namespaces.dart'; import 'package:moxxmpp/src/negotiators/namespaces.dart'; @@ -65,11 +65,9 @@ class ResourceBindingNegotiator extends XmppFeatureNegotiatorBase { } final bind = nonza.firstTag('bind')!; - final jid = bind.firstTag('jid')!; - final resource = jid.innerText().split('/')[1]; - - await attributes - .sendEvent(ResourceBindingSuccessEvent(resource: resource)); + final rawJid = bind.firstTag('jid')!.innerText(); + final resource = JID.fromString(rawJid).resource; + attributes.setResource(resource); return const Result(NegotiatorState.done); } } diff --git a/packages/moxxmpp/test/helpers/manager.dart b/packages/moxxmpp/test/helpers/manager.dart index 17706ea..a530cce 100644 --- a/packages/moxxmpp/test/helpers/manager.dart +++ b/packages/moxxmpp/test/helpers/manager.dart @@ -56,7 +56,6 @@ class TestingManagerHolder { sendNonza: (_) {}, sendEvent: (_) {}, getSocket: () => _socket, - isFeatureSupported: (_) => false, getNegotiatorById: getNegotiatorNullStub, getFullJID: () => jid, getManagerById: _getManagerById, diff --git a/packages/moxxmpp/test/sasl/scram_test.dart b/packages/moxxmpp/test/sasl/scram_test.dart index d3552f7..44596de 100644 --- a/packages/moxxmpp/test/sasl/scram_test.dart +++ b/packages/moxxmpp/test/sasl/scram_test.dart @@ -58,6 +58,7 @@ void main() { () => fakeSocket, () => false, () {}, + (_, {bool triggerEvent = true}) {}, (_) {}, ), ); @@ -153,6 +154,7 @@ void main() { () => fakeSocket, () => false, () {}, + (_, {bool triggerEvent = true}) {}, (_) {}, ), ); @@ -203,6 +205,7 @@ void main() { () => fakeSocket, () => false, () {}, + (_, {bool triggerEvent = true}) {}, (_) {}, ), ); @@ -243,6 +246,7 @@ void main() { () => fakeSocket, () => false, () {}, + (_, {bool triggerEvent = true}) {}, (_) {}, ), ); @@ -286,6 +290,7 @@ void main() { () => fakeSocket, () => false, () {}, + (_, {bool triggerEvent = true}) {}, (_) {}, ), ); diff --git a/packages/moxxmpp/test/xeps/xep_0198_test.dart b/packages/moxxmpp/test/xeps/xep_0198_test.dart index bce1f96..fc985ef 100644 --- a/packages/moxxmpp/test/xeps/xep_0198_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0198_test.dart @@ -63,7 +63,6 @@ XmppManagerAttributes mkAttributes(void Function(Stanza) callback) { password: 'password', useDirectTLS: true, ), - isFeatureSupported: (_) => false, getFullJID: () => JID.fromString('hallo@example.server/uwu'), getSocket: () => StubTCPSocket([]), getConnection: () => XmppConnection( diff --git a/packages/moxxmpp/test/xeps/xep_0280_test.dart b/packages/moxxmpp/test/xeps/xep_0280_test.dart index f22333e..282317e 100644 --- a/packages/moxxmpp/test/xeps/xep_0280_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0280_test.dart @@ -27,7 +27,6 @@ void main() { password: 'password', useDirectTLS: true, ), - isFeatureSupported: (_) => false, getFullJID: () => JID.fromString('bob@xmpp.example/uwu'), getSocket: () => StubTCPSocket([]), getConnection: () => XmppConnection( diff --git a/packages/moxxmpp/test/xeps/xep_0352_test.dart b/packages/moxxmpp/test/xeps/xep_0352_test.dart index da89079..3981296 100644 --- a/packages/moxxmpp/test/xeps/xep_0352_test.dart +++ b/packages/moxxmpp/test/xeps/xep_0352_test.dart @@ -55,7 +55,6 @@ void main() { ), getManagerById: getManagerNullStub, getNegotiatorById: getUnsupportedCSINegotiator, - isFeatureSupported: (_) => false, getFullJID: () => JID.fromString('some.user@example.server/aaaaa'), getSocket: () => StubTCPSocket([]), getConnection: () => XmppConnection( @@ -99,7 +98,6 @@ void main() { ), getManagerById: getManagerNullStub, getNegotiatorById: getSupportedCSINegotiator, - isFeatureSupported: (_) => false, getFullJID: () => JID.fromString('some.user@example.server/aaaaa'), getSocket: () => StubTCPSocket([]), getConnection: () => XmppConnection( diff --git a/packages/moxxmpp/test/xmpp_test.dart b/packages/moxxmpp/test/xmpp_test.dart index 829f101..501932d 100644 --- a/packages/moxxmpp/test/xmpp_test.dart +++ b/packages/moxxmpp/test/xmpp_test.dart @@ -35,7 +35,6 @@ Future testRosterManager( ), getManagerById: getManagerNullStub, getNegotiatorById: getNegotiatorNullStub, - isFeatureSupported: (_) => false, getFullJID: () => JID.fromString('$bareJid/$resource'), getSocket: () => StubTCPSocket([]), getConnection: () => XmppConnection( @@ -151,6 +150,7 @@ void main() { waitUntilLogin: true, ); expect(fakeSocket.getState(), /*6*/ 5); + expect(conn.resource, 'MU29eEZn'); }); test('Test a failed SASL auth', () async { @@ -296,7 +296,6 @@ void main() { ), getManagerById: getManagerNullStub, getNegotiatorById: getNegotiatorNullStub, - isFeatureSupported: (_) => false, getFullJID: () => JID.fromString('some.user@example.server/aaaaa'), getSocket: () => StubTCPSocket([]), getConnection: () => XmppConnection(