fix(tests): Fix tests

This commit is contained in:
PapaTutuWawa 2023-09-29 20:24:58 +02:00
parent 3a94dd9634
commit 49d3c6411b
8 changed files with 77 additions and 62 deletions

View File

@ -1,6 +1,5 @@
import 'dart:async'; import 'dart:async';
import 'package:meta/meta.dart'; import 'package:meta/meta.dart';
import 'package:moxxmpp/src/jid.dart';
import 'package:moxxmpp/src/stringxml.dart'; import 'package:moxxmpp/src/stringxml.dart';
import 'package:synchronized/synchronized.dart'; import 'package:synchronized/synchronized.dart';
@ -11,7 +10,7 @@ class _StanzaSurrogateKey {
/// The JID the original stanza was sent to. We expect the result to come from the /// The JID the original stanza was sent to. We expect the result to come from the
/// same JID. /// same JID.
final String sentTo; final String? sentTo;
/// The ID of the original stanza. We expect the result to have the same ID. /// The ID of the original stanza. We expect the result to have the same ID.
final String id; final String id;
@ -52,7 +51,7 @@ class StanzaAwaiter {
/// [tag] is the stanza's tag name. /// [tag] is the stanza's tag name.
/// ///
/// Returns a future that might resolve to the response to the stanza. /// Returns a future that might resolve to the response to the stanza.
Future<Future<XMLNode>> addPending(String to, String id, String tag) async { Future<Future<XMLNode>> addPending(String? to, String id, String tag) async {
final completer = await _lock.synchronized(() { final completer = await _lock.synchronized(() {
final completer = Completer<XMLNode>(); final completer = Completer<XMLNode>();
_pending[_StanzaSurrogateKey(to, id, tag)] = completer; _pending[_StanzaSurrogateKey(to, id, tag)] = completer;
@ -62,20 +61,15 @@ class StanzaAwaiter {
return completer.future; return completer.future;
} }
/// Checks if the stanza [stanza] is being awaited. [bareJid] is the bare JID of /// Checks if the stanza [stanza] is being awaited.
/// the connection.
/// If [stanza] is awaited, resolves the future and returns true. If not, returns /// If [stanza] is awaited, resolves the future and returns true. If not, returns
/// false. /// false.
Future<bool> onData(XMLNode stanza, JID bareJid) async { Future<bool> onData(XMLNode stanza) async {
assert(bareJid.isBare(), 'bareJid must be bare');
final id = stanza.attributes['id'] as String?; final id = stanza.attributes['id'] as String?;
if (id == null) return false; if (id == null) return false;
final key = _StanzaSurrogateKey( final key = _StanzaSurrogateKey(
// Section 8.1.2.1 § 3 of RFC 6120 says that an empty "from" indicates that the stanza.attributes['from'] as String?,
// attribute is implicitly from our own bare JID.
stanza.attributes['from'] as String? ?? bareJid.toString(),
id, id,
stanza.tag, stanza.tag,
); );
@ -91,4 +85,19 @@ class StanzaAwaiter {
return false; return false;
}); });
} }
/// Checks if [stanza] represents a stanza that is awaited. Returns true, if [stanza]
/// is awaited. False, if not.
Future<bool> isAwaited(XMLNode stanza) async {
final id = stanza.attributes['id'] as String?;
if (id == null) return false;
final key = _StanzaSurrogateKey(
stanza.attributes['from'] as String?,
id,
stanza.tag,
);
return _lock.synchronized(() => _pending.containsKey(key));
}
} }

View File

@ -90,7 +90,7 @@ class XmppConnection {
}, },
); );
_incomingStanzaQueue = IncomingStanzaQueue(handleXmlStream); _incomingStanzaQueue = IncomingStanzaQueue(handleXmlStream, _stanzaAwaiter);
_socketStream = _socket.getDataStream(); _socketStream = _socket.getDataStream();
// TODO(Unknown): Handle on done // TODO(Unknown): Handle on done
_socketStream _socketStream
@ -531,12 +531,15 @@ class XmppConnection {
_log.finest('==> $prefix${newStanza.toXml()}'); _log.finest('==> $prefix${newStanza.toXml()}');
if (details.awaitable) { if (details.awaitable) {
final isOwnJid =
data.stanza.to == connectionSettings.jid.toBare().toString();
await _stanzaAwaiter await _stanzaAwaiter
.addPending( .addPending(
// A stanza with no to attribute is for direct processing by the server. As such, // A stanza with no to attribute is for direct processing by the server. As such,
// we can correlate it by just *assuming* we have that attribute // we can correlate it by just *assuming* we have that attribute
// (RFC 6120 Section 8.1.1.1) // (RFC 6120 Section 8.1.1.1)
data.stanza.to ?? connectionSettings.jid.toBare().toString(), isOwnJid ? null : data.stanza.to,
data.stanza.id!, data.stanza.id!,
data.stanza.tag, data.stanza.tag,
) )
@ -773,9 +776,15 @@ class XmppConnection {
return; return;
} }
// In case the stanza came from our own bare Jid, remove it so that the stanza
// awaiter works correctly.
final isOwnJid = incomingPreHandlers.stanza.from ==
connectionSettings.jid.toBare().toString();
final ownJidStanza = isOwnJid
? incomingPreHandlers.stanza.copyWith(from: null)
: incomingPreHandlers.stanza;
final awaited = await _stanzaAwaiter.onData( final awaited = await _stanzaAwaiter.onData(
incomingPreHandlers.stanza, ownJidStanza,
connectionSettings.jid.toBare(),
); );
if (awaited) { if (awaited) {
return; return;

View File

@ -102,6 +102,8 @@ class RemoteServerTimeoutError extends StanzaError {
/// An unknown error. /// An unknown error.
class UnknownStanzaError extends StanzaError {} class UnknownStanzaError extends StanzaError {}
const _stanzaNotDefined = Object();
class Stanza extends XMLNode { class Stanza extends XMLNode {
// ignore: use_super_parameters // ignore: use_super_parameters
Stanza({ Stanza({
@ -216,7 +218,7 @@ class Stanza extends XMLNode {
Stanza copyWith({ Stanza copyWith({
String? id, String? id,
String? from, Object? from = _stanzaNotDefined,
String? to, String? to,
String? type, String? type,
List<XMLNode>? children, List<XMLNode>? children,
@ -225,7 +227,7 @@ class Stanza extends XMLNode {
return Stanza( return Stanza(
tag: tag, tag: tag,
to: to ?? this.to, to: to ?? this.to,
from: from ?? this.from, from: from != _stanzaNotDefined ? from as String? : this.from,
id: id ?? this.id, id: id ?? this.id,
type: type ?? this.type, type: type ?? this.type,
children: children ?? this.children, children: children ?? this.children,

View File

@ -1,24 +1,37 @@
import 'dart:async'; import 'dart:async';
import 'dart:collection'; import 'dart:collection';
import 'package:logging/logging.dart'; import 'package:logging/logging.dart';
import 'package:moxxmpp/src/awaiter.dart';
import 'package:moxxmpp/src/parser.dart'; import 'package:moxxmpp/src/parser.dart';
import 'package:synchronized/synchronized.dart'; import 'package:synchronized/synchronized.dart';
typedef LockResult = (Completer<void>?, XMPPStreamObject);
class IncomingStanzaQueue { class IncomingStanzaQueue {
IncomingStanzaQueue(this._callback); IncomingStanzaQueue(this._callback, this._stanzaAwaiter);
/// The queue for storing the completer of each
/// incoming stanza (or stream object to be precise).
/// Only access while holding the lock [_lock].
final Queue<Completer<void>> _queue = Queue(); final Queue<Completer<void>> _queue = Queue();
final Future<void> Function(XMPPStreamObject) _callback; /// Flag indicating whether a callback is already running (true)
/// or not. "a callback" and not "the callback" because awaited stanzas
/// are allowed to bypass the queue.
/// Only access while holding the lock [_lock].
bool _isRunning = false; bool _isRunning = false;
/// The function to call to process an incoming stream object.
final Future<void> Function(XMPPStreamObject) _callback;
/// Lock guarding both [_queue] and [_isRunning].
final Lock _lock = Lock(); final Lock _lock = Lock();
// TODO: Remove once we can await stanzas (or can we).
bool negotiationsDone = false;
/// Logger.
final Logger _log = Logger('IncomingStanzaQueue'); final Logger _log = Logger('IncomingStanzaQueue');
bool negotiationsDone = false; final StanzaAwaiter _stanzaAwaiter;
Future<void> _processStreamObject( Future<void> _processStreamObject(
Future<void>? future, Future<void>? future,
@ -37,37 +50,22 @@ class IncomingStanzaQueue {
await future; await future;
// Run the callback. // Run the callback.
if (object is XMPPStreamElement) {
_log.finest('Running callback for ${object.node.toXml()}');
}
await _callback(object); await _callback(object);
if (object is XMPPStreamElement) {
_log.finest(
'Callback for ${object.node.tag} (${object.node.attributes["id"]}) done',
);
}
// Run the next entry. // Run the next entry.
_log.finest('Entering second lock...');
await _lock.synchronized(() { await _lock.synchronized(() {
_log.finest('Second lock entered...');
if (_queue.isNotEmpty) { if (_queue.isNotEmpty) {
_log.finest('New queue size: ${_queue.length - 1}');
_queue.removeFirst().complete(); _queue.removeFirst().complete();
} else { } else {
_isRunning = false; _isRunning = false;
_log.finest('New queue size: 0');
} }
}); });
} }
Future<void> addStanza(List<XMPPStreamObject> objects) async { Future<void> addStanza(List<XMPPStreamObject> objects) async {
_log.finest('Entering initial lock...'); await _lock.synchronized(() async {
await _lock.synchronized(() {
_log.finest('Lock entered...');
for (final object in objects) { for (final object in objects) {
if (canBypassQueue(object)) { if (await canBypassQueue(object)) {
unawaited( unawaited(
_processStreamObject(null, object), _processStreamObject(null, object),
); );
@ -89,11 +87,12 @@ class IncomingStanzaQueue {
}); });
} }
bool canBypassQueue(XMPPStreamObject object) { Future<bool> canBypassQueue(XMPPStreamObject object) async {
// TODO: Ask the StanzaAwaiter if the stanza is awaited if (object is XMPPStreamHeader) {
return object is XMPPStreamElement && return false;
negotiationsDone && }
object.node.tag == 'iq' &&
['result', 'error'].contains(object.node.attributes['type'] as String?); object as XMPPStreamElement;
return _stanzaAwaiter.isAwaited(object.node);
} }
} }

View File

@ -16,7 +16,6 @@ import 'package:moxxmpp/src/xeps/xep_0045/errors.dart';
import 'package:moxxmpp/src/xeps/xep_0045/events.dart'; import 'package:moxxmpp/src/xeps/xep_0045/events.dart';
import 'package:moxxmpp/src/xeps/xep_0045/types.dart'; import 'package:moxxmpp/src/xeps/xep_0045/types.dart';
import 'package:moxxmpp/src/xeps/xep_0359.dart'; import 'package:moxxmpp/src/xeps/xep_0359.dart';
import 'package:synchronized/extension.dart';
import 'package:synchronized/synchronized.dart'; import 'package:synchronized/synchronized.dart';
/// (Room JID, nickname) /// (Room JID, nickname)

View File

@ -173,8 +173,13 @@ class EntityCapabilitiesManager extends XmppManagerBase {
}); });
} }
Future<void> _performQuery(Stanza presence, String ver, Future<void> _performQuery(
String hashFunctionName, String capabilityNode, JID from) async { Stanza presence,
String ver,
String hashFunctionName,
String capabilityNode,
JID from,
) async {
final dm = getAttributes().getManagerById<DiscoManager>(discoManager)!; final dm = getAttributes().getManagerById<DiscoManager>(discoManager)!;
final discoRequest = await dm.discoInfoQuery( final discoRequest = await dm.discoInfoQuery(
from, from,

View File

@ -3,8 +3,6 @@ import 'package:moxxmpp/src/awaiter.dart';
import 'package:test/test.dart'; import 'package:test/test.dart';
void main() { void main() {
const bareJid = JID('moxxmpp', 'server3.example', '');
test('Test awaiting an awaited stanza with a from attribute', () async { test('Test awaiting an awaited stanza with a from attribute', () async {
final awaiter = StanzaAwaiter(); final awaiter = StanzaAwaiter();
@ -20,14 +18,12 @@ void main() {
XMLNode.fromString( XMLNode.fromString(
'<iq from="user3@server.example" id="abc123" type="result" />', '<iq from="user3@server.example" id="abc123" type="result" />',
), ),
bareJid,
); );
expect(result1, false); expect(result1, false);
final result2 = await awaiter.onData( final result2 = await awaiter.onData(
XMLNode.fromString( XMLNode.fromString(
'<iq from="user1@server.example" id="lol" type="result" />', '<iq from="user1@server.example" id="lol" type="result" />',
), ),
bareJid,
); );
expect(result2, false); expect(result2, false);
@ -37,7 +33,6 @@ void main() {
); );
final result3 = await awaiter.onData( final result3 = await awaiter.onData(
stanza, stanza,
bareJid,
); );
expect(result3, true); expect(result3, true);
expect(await future, stanza); expect(await future, stanza);
@ -47,12 +42,11 @@ void main() {
final awaiter = StanzaAwaiter(); final awaiter = StanzaAwaiter();
// "Send" a stanza // "Send" a stanza
final future = await awaiter.addPending(bareJid.toString(), 'abc123', 'iq'); final future = await awaiter.addPending(null, 'abc123', 'iq');
// Receive the wrong answer // Receive the wrong answer
final result1 = await awaiter.onData( final result1 = await awaiter.onData(
XMLNode.fromString('<iq id="lol" type="result" />'), XMLNode.fromString('<iq id="lol" type="result" />'),
bareJid,
); );
expect(result1, false); expect(result1, false);
@ -60,7 +54,6 @@ void main() {
final stanza = XMLNode.fromString('<iq id="abc123" type="result" />'); final stanza = XMLNode.fromString('<iq id="abc123" type="result" />');
final result2 = await awaiter.onData( final result2 = await awaiter.onData(
stanza, stanza,
bareJid,
); );
expect(result2, true); expect(result2, true);
expect(await future, stanza); expect(await future, stanza);
@ -70,13 +63,12 @@ void main() {
final awaiter = StanzaAwaiter(); final awaiter = StanzaAwaiter();
// "Send" a stanza // "Send" a stanza
final future = await awaiter.addPending(bareJid.toString(), 'abc123', 'iq'); final future = await awaiter.addPending(null, 'abc123', 'iq');
// Receive the correct answer // Receive the correct answer
final stanza = XMLNode.fromString('<iq id="abc123" type="result" />'); final stanza = XMLNode.fromString('<iq id="abc123" type="result" />');
final result1 = await awaiter.onData( final result1 = await awaiter.onData(
stanza, stanza,
bareJid,
); );
expect(result1, true); expect(result1, true);
expect(await future, stanza); expect(await future, stanza);
@ -84,7 +76,6 @@ void main() {
// Receive it again // Receive it again
final result2 = await awaiter.onData( final result2 = await awaiter.onData(
stanza, stanza,
bareJid,
); );
expect(result2, false); expect(result2, false);
}); });
@ -93,20 +84,18 @@ void main() {
final awaiter = StanzaAwaiter(); final awaiter = StanzaAwaiter();
// "Send" a stanza // "Send" a stanza
final future = await awaiter.addPending(bareJid.toString(), 'abc123', 'iq'); final future = await awaiter.addPending(null, 'abc123', 'iq');
// Receive the wrong answer // Receive the wrong answer
final stanza = XMLNode.fromString('<iq id="abc123" type="result" />'); final stanza = XMLNode.fromString('<iq id="abc123" type="result" />');
final result1 = await awaiter.onData( final result1 = await awaiter.onData(
XMLNode.fromString('<message id="abc123" type="result" />'), XMLNode.fromString('<message id="abc123" type="result" />'),
bareJid,
); );
expect(result1, false); expect(result1, false);
// Receive the correct answer // Receive the correct answer
final result2 = await awaiter.onData( final result2 = await awaiter.onData(
stanza, stanza,
bareJid,
); );
expect(result2, true); expect(result2, true);
expect(await future, stanza); expect(await future, stanza);

View File

@ -343,6 +343,7 @@ void main() {
stanza, stanza,
StanzaHandlerData(false, false, stanza, TypedMap()), StanzaHandlerData(false, false, stanza, TypedMap()),
); );
await Future<void>.delayed(const Duration(seconds: 2));
expect( expect(
await manager.getCachedDiscoInfoFromJid(aliceJid) != null, await manager.getCachedDiscoInfoFromJid(aliceJid) != null,
@ -513,6 +514,7 @@ void main() {
stanza, stanza,
StanzaHandlerData(false, false, stanza, TypedMap()), StanzaHandlerData(false, false, stanza, TypedMap()),
); );
await Future<void>.delayed(const Duration(seconds: 2));
final cachedItem = await manager.getCachedDiscoInfoFromJid(aliceJid); final cachedItem = await manager.getCachedDiscoInfoFromJid(aliceJid);
expect( expect(
@ -549,6 +551,7 @@ void main() {
stanza, stanza,
StanzaHandlerData(false, false, stanza, TypedMap()), StanzaHandlerData(false, false, stanza, TypedMap()),
); );
await Future<void>.delayed(const Duration(seconds: 2));
final cachedItem = await manager.getCachedDiscoInfoFromJid(aliceJid); final cachedItem = await manager.getCachedDiscoInfoFromJid(aliceJid);
expect( expect(