From e62a3442a65c8a19472abadd118a2beb04c35050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krasi=C5=84ski-Sroka?= Date: Mon, 22 Jul 2024 16:48:03 +0200 Subject: [PATCH 1/2] Double-check whether connection is possible after _buildMountPoint --- lib/src/socket.dart | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/lib/src/socket.dart b/lib/src/socket.dart index 82a7690..7d495a4 100644 --- a/lib/src/socket.dart +++ b/lib/src/socket.dart @@ -170,10 +170,19 @@ class PhoenixSocket { /// Whether the underlying socket is connected of not. bool get isConnected => _ws != null && _socketState == SocketState.connected; + bool get _isConnectingOrConnected => + _ws != null && + (_socketState == SocketState.connected || + _socketState == SocketState.connecting); + + void _checkNotDisposed() { + if (_disposed) { + throw StateError('PhoenixSocket cannot connect after being disposed.'); + } + } + void _connect(Completer completer) async { - if (_ws != null && - (_socketState == SocketState.connected || - _socketState == SocketState.connecting)) { + if (_isConnectingOrConnected) { _logger.warning( 'Calling connect() on already connected or connecting socket.'); completer.complete(this); @@ -182,12 +191,20 @@ class PhoenixSocket { _shouldReconnect = true; - if (_disposed) { - throw StateError('PhoenixSocket cannot connect after being disposed.'); - } + _checkNotDisposed(); _mountPoint = await _buildMountPoint(_endpoint, _options); + // Double-check in case something changed during the async call above. + if (_isConnectingOrConnected) { + _logger.warning( + 'Calling connect() on already connected or connecting socket.'); + completer.complete(this); + return; + } + + _checkNotDisposed(); + // workaround to check the existing bearer token final token = _mountPoint.queryParameters["token"]; From 785434294be5c3b5c336a6b4afdaccb7783e2fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krasi=C5=84ski-Sroka?= Date: Mon, 22 Jul 2024 18:01:41 +0200 Subject: [PATCH 2/2] Adds a test case. --- test/mocks.dart | 1 + test/mocks.mocks.dart | 247 ++++++++++++++++++++++++++++-------------- test/socket_test.dart | 61 +++++++++++ 3 files changed, 226 insertions(+), 83 deletions(-) diff --git a/test/mocks.dart b/test/mocks.dart index 87d5409..a6d4aae 100644 --- a/test/mocks.dart +++ b/test/mocks.dart @@ -11,6 +11,7 @@ export 'mocks.mocks.dart'; MockSpec(), MockSpec(), MockSpec(), + MockSpec(), ], ) class MockTest extends Mock { diff --git a/test/mocks.mocks.dart b/test/mocks.mocks.dart index 4341c83..010aa7c 100644 --- a/test/mocks.mocks.dart +++ b/test/mocks.mocks.dart @@ -3,17 +3,19 @@ // Do not manually edit this file. // ignore_for_file: no_leading_underscores_for_library_prefixes -import 'dart:async' as _i9; +import 'dart:async' as _i10; -import 'package:async/async.dart' as _i12; +import 'package:async/async.dart' as _i13; import 'package:mockito/mockito.dart' as _i1; -import 'package:mockito/src/dummies.dart' as _i8; +import 'package:mockito/src/dummies.dart' as _i9; import 'package:phoenix_socket/src/channel.dart' as _i5; -import 'package:phoenix_socket/src/events.dart' as _i10; -import 'package:phoenix_socket/src/exceptions.dart' as _i11; +import 'package:phoenix_socket/src/events.dart' as _i11; +import 'package:phoenix_socket/src/exceptions.dart' as _i12; import 'package:phoenix_socket/src/message.dart' as _i3; +import 'package:phoenix_socket/src/message_serializer.dart' as _i8; import 'package:phoenix_socket/src/push.dart' as _i4; import 'package:phoenix_socket/src/socket.dart' as _i2; +import 'package:phoenix_socket/src/socket_options.dart' as _i14; import 'package:stream_channel/stream_channel.dart' as _i7; import 'package:web_socket_channel/web_socket_channel.dart' as _i6; @@ -112,6 +114,17 @@ class _FakeStreamChannel_7 extends _i1.SmartFake ); } +class _FakeMessageSerializer_8 extends _i1.SmartFake + implements _i8.MessageSerializer { + _FakeMessageSerializer_8( + Object parent, + Invocation parentInvocation, + ) : super( + parent, + parentInvocation, + ); +} + /// A class which mocks [PhoenixChannel]. /// /// See the documentation for Mockito's code generation for more information. @@ -139,11 +152,11 @@ class MockPhoenixChannel extends _i1.Mock implements _i5.PhoenixChannel { @override String get topic => (super.noSuchMethod( Invocation.getter(#topic), - returnValue: _i8.dummyValue( + returnValue: _i9.dummyValue( this, Invocation.getter(#topic), ), - returnValueForMissingStub: _i8.dummyValue( + returnValueForMissingStub: _i9.dummyValue( this, Invocation.getter(#topic), ), @@ -157,20 +170,20 @@ class MockPhoenixChannel extends _i1.Mock implements _i5.PhoenixChannel { ) as List<_i4.Push>); @override - _i9.Stream<_i3.Message> get messages => (super.noSuchMethod( + _i10.Stream<_i3.Message> get messages => (super.noSuchMethod( Invocation.getter(#messages), - returnValue: _i9.Stream<_i3.Message>.empty(), - returnValueForMissingStub: _i9.Stream<_i3.Message>.empty(), - ) as _i9.Stream<_i3.Message>); + returnValue: _i10.Stream<_i3.Message>.empty(), + returnValueForMissingStub: _i10.Stream<_i3.Message>.empty(), + ) as _i10.Stream<_i3.Message>); @override String get joinRef => (super.noSuchMethod( Invocation.getter(#joinRef), - returnValue: _i8.dummyValue( + returnValue: _i9.dummyValue( this, Invocation.getter(#joinRef), ), - returnValueForMissingStub: _i8.dummyValue( + returnValueForMissingStub: _i9.dummyValue( this, Invocation.getter(#joinRef), ), @@ -193,11 +206,11 @@ class MockPhoenixChannel extends _i1.Mock implements _i5.PhoenixChannel { @override String get loggerName => (super.noSuchMethod( Invocation.getter(#loggerName), - returnValue: _i8.dummyValue( + returnValue: _i9.dummyValue( this, Invocation.getter(#loggerName), ), - returnValueForMissingStub: _i8.dummyValue( + returnValueForMissingStub: _i9.dummyValue( this, Invocation.getter(#loggerName), ), @@ -206,38 +219,39 @@ class MockPhoenixChannel extends _i1.Mock implements _i5.PhoenixChannel { @override String get reference => (super.noSuchMethod( Invocation.getter(#reference), - returnValue: _i8.dummyValue( + returnValue: _i9.dummyValue( this, Invocation.getter(#reference), ), - returnValueForMissingStub: _i8.dummyValue( + returnValueForMissingStub: _i9.dummyValue( this, Invocation.getter(#reference), ), ) as String); @override - _i9.Future<_i3.Message> onPushReply(_i10.PhoenixChannelEvent? replyEvent) => + _i10.Future<_i3.Message> onPushReply(_i11.PhoenixChannelEvent? replyEvent) => (super.noSuchMethod( Invocation.method( #onPushReply, [replyEvent], ), - returnValue: _i9.Future<_i3.Message>.value(_FakeMessage_1( + returnValue: _i10.Future<_i3.Message>.value(_FakeMessage_1( this, Invocation.method( #onPushReply, [replyEvent], ), )), - returnValueForMissingStub: _i9.Future<_i3.Message>.value(_FakeMessage_1( + returnValueForMissingStub: + _i10.Future<_i3.Message>.value(_FakeMessage_1( this, Invocation.method( #onPushReply, [replyEvent], ), )), - ) as _i9.Future<_i3.Message>); + ) as _i10.Future<_i3.Message>); @override void close() => super.noSuchMethod( @@ -258,7 +272,7 @@ class MockPhoenixChannel extends _i1.Mock implements _i5.PhoenixChannel { ); @override - void triggerError(_i11.PhoenixException? error) => super.noSuchMethod( + void triggerError(_i12.PhoenixException? error) => super.noSuchMethod( Invocation.method( #triggerError, [error], @@ -354,7 +368,7 @@ class MockPhoenixChannel extends _i1.Mock implements _i5.PhoenixChannel { @override _i4.Push pushEvent( - _i10.PhoenixChannelEvent? event, + _i11.PhoenixChannelEvent? event, Map? payload, [ Duration? newTimeout, ]) => @@ -414,46 +428,47 @@ class MockPhoenixSocket extends _i1.Mock implements _i2.PhoenixSocket { ); @override - _i9.Stream<_i10.PhoenixSocketOpenEvent> get openStream => (super.noSuchMethod( + _i10.Stream<_i11.PhoenixSocketOpenEvent> get openStream => + (super.noSuchMethod( Invocation.getter(#openStream), - returnValue: _i9.Stream<_i10.PhoenixSocketOpenEvent>.empty(), + returnValue: _i10.Stream<_i11.PhoenixSocketOpenEvent>.empty(), returnValueForMissingStub: - _i9.Stream<_i10.PhoenixSocketOpenEvent>.empty(), - ) as _i9.Stream<_i10.PhoenixSocketOpenEvent>); + _i10.Stream<_i11.PhoenixSocketOpenEvent>.empty(), + ) as _i10.Stream<_i11.PhoenixSocketOpenEvent>); @override - _i9.Stream<_i10.PhoenixSocketCloseEvent> get closeStream => + _i10.Stream<_i11.PhoenixSocketCloseEvent> get closeStream => (super.noSuchMethod( Invocation.getter(#closeStream), - returnValue: _i9.Stream<_i10.PhoenixSocketCloseEvent>.empty(), + returnValue: _i10.Stream<_i11.PhoenixSocketCloseEvent>.empty(), returnValueForMissingStub: - _i9.Stream<_i10.PhoenixSocketCloseEvent>.empty(), - ) as _i9.Stream<_i10.PhoenixSocketCloseEvent>); + _i10.Stream<_i11.PhoenixSocketCloseEvent>.empty(), + ) as _i10.Stream<_i11.PhoenixSocketCloseEvent>); @override - _i9.Stream<_i10.PhoenixSocketErrorEvent> get errorStream => + _i10.Stream<_i11.PhoenixSocketErrorEvent> get errorStream => (super.noSuchMethod( Invocation.getter(#errorStream), - returnValue: _i9.Stream<_i10.PhoenixSocketErrorEvent>.empty(), + returnValue: _i10.Stream<_i11.PhoenixSocketErrorEvent>.empty(), returnValueForMissingStub: - _i9.Stream<_i10.PhoenixSocketErrorEvent>.empty(), - ) as _i9.Stream<_i10.PhoenixSocketErrorEvent>); + _i10.Stream<_i11.PhoenixSocketErrorEvent>.empty(), + ) as _i10.Stream<_i11.PhoenixSocketErrorEvent>); @override - _i9.Stream<_i3.Message> get messageStream => (super.noSuchMethod( + _i10.Stream<_i3.Message> get messageStream => (super.noSuchMethod( Invocation.getter(#messageStream), - returnValue: _i9.Stream<_i3.Message>.empty(), - returnValueForMissingStub: _i9.Stream<_i3.Message>.empty(), - ) as _i9.Stream<_i3.Message>); + returnValue: _i10.Stream<_i3.Message>.empty(), + returnValueForMissingStub: _i10.Stream<_i3.Message>.empty(), + ) as _i10.Stream<_i3.Message>); @override String get nextRef => (super.noSuchMethod( Invocation.getter(#nextRef), - returnValue: _i8.dummyValue( + returnValue: _i9.dummyValue( this, Invocation.getter(#nextRef), ), - returnValueForMissingStub: _i8.dummyValue( + returnValueForMissingStub: _i9.dummyValue( this, Invocation.getter(#nextRef), ), @@ -475,11 +490,11 @@ class MockPhoenixSocket extends _i1.Mock implements _i2.PhoenixSocket { @override String get endpoint => (super.noSuchMethod( Invocation.getter(#endpoint), - returnValue: _i8.dummyValue( + returnValue: _i9.dummyValue( this, Invocation.getter(#endpoint), ), - returnValueForMissingStub: _i8.dummyValue( + returnValueForMissingStub: _i9.dummyValue( this, Invocation.getter(#endpoint), ), @@ -506,24 +521,24 @@ class MockPhoenixSocket extends _i1.Mock implements _i2.PhoenixSocket { ) as bool); @override - _i9.Stream<_i3.Message> streamForTopic(String? topic) => (super.noSuchMethod( + _i10.Stream<_i3.Message> streamForTopic(String? topic) => (super.noSuchMethod( Invocation.method( #streamForTopic, [topic], ), - returnValue: _i9.Stream<_i3.Message>.empty(), - returnValueForMissingStub: _i9.Stream<_i3.Message>.empty(), - ) as _i9.Stream<_i3.Message>); + returnValue: _i10.Stream<_i3.Message>.empty(), + returnValueForMissingStub: _i10.Stream<_i3.Message>.empty(), + ) as _i10.Stream<_i3.Message>); @override - _i9.Future<_i2.PhoenixSocket?> connect() => (super.noSuchMethod( + _i10.Future<_i2.PhoenixSocket?> connect() => (super.noSuchMethod( Invocation.method( #connect, [], ), - returnValue: _i9.Future<_i2.PhoenixSocket?>.value(), - returnValueForMissingStub: _i9.Future<_i2.PhoenixSocket?>.value(), - ) as _i9.Future<_i2.PhoenixSocket?>); + returnValue: _i10.Future<_i2.PhoenixSocket?>.value(), + returnValueForMissingStub: _i10.Future<_i2.PhoenixSocket?>.value(), + ) as _i10.Future<_i2.PhoenixSocket?>); @override void close([ @@ -553,50 +568,52 @@ class MockPhoenixSocket extends _i1.Mock implements _i2.PhoenixSocket { ); @override - _i9.Future<_i3.Message> waitForMessage(_i3.Message? message) => + _i10.Future<_i3.Message> waitForMessage(_i3.Message? message) => (super.noSuchMethod( Invocation.method( #waitForMessage, [message], ), - returnValue: _i9.Future<_i3.Message>.value(_FakeMessage_1( + returnValue: _i10.Future<_i3.Message>.value(_FakeMessage_1( this, Invocation.method( #waitForMessage, [message], ), )), - returnValueForMissingStub: _i9.Future<_i3.Message>.value(_FakeMessage_1( + returnValueForMissingStub: + _i10.Future<_i3.Message>.value(_FakeMessage_1( this, Invocation.method( #waitForMessage, [message], ), )), - ) as _i9.Future<_i3.Message>); + ) as _i10.Future<_i3.Message>); @override - _i9.Future<_i3.Message> sendMessage(_i3.Message? message) => + _i10.Future<_i3.Message> sendMessage(_i3.Message? message) => (super.noSuchMethod( Invocation.method( #sendMessage, [message], ), - returnValue: _i9.Future<_i3.Message>.value(_FakeMessage_1( + returnValue: _i10.Future<_i3.Message>.value(_FakeMessage_1( this, Invocation.method( #sendMessage, [message], ), )), - returnValueForMissingStub: _i9.Future<_i3.Message>.value(_FakeMessage_1( + returnValueForMissingStub: + _i10.Future<_i3.Message>.value(_FakeMessage_1( this, Invocation.method( #sendMessage, [message], ), )), - ) as _i9.Future<_i3.Message>); + ) as _i10.Future<_i3.Message>); @override _i5.PhoenixChannel addChannel({ @@ -655,18 +672,18 @@ class MockPhoenixSocket extends _i1.Mock implements _i2.PhoenixSocket { /// See the documentation for Mockito's code generation for more information. class MockWebSocketChannel extends _i1.Mock implements _i6.WebSocketChannel { @override - _i9.Future get ready => (super.noSuchMethod( + _i10.Future get ready => (super.noSuchMethod( Invocation.getter(#ready), - returnValue: _i9.Future.value(), - returnValueForMissingStub: _i9.Future.value(), - ) as _i9.Future); + returnValue: _i10.Future.value(), + returnValueForMissingStub: _i10.Future.value(), + ) as _i10.Future); @override - _i9.Stream get stream => (super.noSuchMethod( + _i10.Stream get stream => (super.noSuchMethod( Invocation.getter(#stream), - returnValue: _i9.Stream.empty(), - returnValueForMissingStub: _i9.Stream.empty(), - ) as _i9.Stream); + returnValue: _i10.Stream.empty(), + returnValueForMissingStub: _i10.Stream.empty(), + ) as _i10.Stream); @override _i6.WebSocketSink get sink => (super.noSuchMethod( @@ -716,7 +733,7 @@ class MockWebSocketChannel extends _i1.Mock implements _i6.WebSocketChannel { @override _i7.StreamChannel transformStream( - _i9.StreamTransformer? transformer) => + _i10.StreamTransformer? transformer) => (super.noSuchMethod( Invocation.method( #transformStream, @@ -740,7 +757,7 @@ class MockWebSocketChannel extends _i1.Mock implements _i6.WebSocketChannel { @override _i7.StreamChannel transformSink( - _i12.StreamSinkTransformer? transformer) => + _i13.StreamSinkTransformer? transformer) => (super.noSuchMethod( Invocation.method( #transformSink, @@ -764,7 +781,7 @@ class MockWebSocketChannel extends _i1.Mock implements _i6.WebSocketChannel { @override _i7.StreamChannel changeStream( - _i9.Stream Function(_i9.Stream)? change) => + _i10.Stream Function(_i10.Stream)? change) => (super.noSuchMethod( Invocation.method( #changeStream, @@ -788,7 +805,8 @@ class MockWebSocketChannel extends _i1.Mock implements _i6.WebSocketChannel { @override _i7.StreamChannel changeSink( - _i9.StreamSink Function(_i9.StreamSink)? change) => + _i10.StreamSink Function(_i10.StreamSink)? + change) => (super.noSuchMethod( Invocation.method( #changeSink, @@ -838,14 +856,14 @@ class MockWebSocketChannel extends _i1.Mock implements _i6.WebSocketChannel { /// See the documentation for Mockito's code generation for more information. class MockWebSocketSink extends _i1.Mock implements _i6.WebSocketSink { @override - _i9.Future get done => (super.noSuchMethod( + _i10.Future get done => (super.noSuchMethod( Invocation.getter(#done), - returnValue: _i9.Future.value(), - returnValueForMissingStub: _i9.Future.value(), - ) as _i9.Future); + returnValue: _i10.Future.value(), + returnValueForMissingStub: _i10.Future.value(), + ) as _i10.Future); @override - _i9.Future close([ + _i10.Future close([ int? closeCode, String? closeReason, ]) => @@ -857,9 +875,9 @@ class MockWebSocketSink extends _i1.Mock implements _i6.WebSocketSink { closeReason, ], ), - returnValue: _i9.Future.value(), - returnValueForMissingStub: _i9.Future.value(), - ) as _i9.Future); + returnValue: _i10.Future.value(), + returnValueForMissingStub: _i10.Future.value(), + ) as _i10.Future); @override void add(dynamic data) => super.noSuchMethod( @@ -887,13 +905,76 @@ class MockWebSocketSink extends _i1.Mock implements _i6.WebSocketSink { ); @override - _i9.Future addStream(_i9.Stream? stream) => + _i10.Future addStream(_i10.Stream? stream) => (super.noSuchMethod( Invocation.method( #addStream, [stream], ), - returnValue: _i9.Future.value(), - returnValueForMissingStub: _i9.Future.value(), - ) as _i9.Future); + returnValue: _i10.Future.value(), + returnValueForMissingStub: _i10.Future.value(), + ) as _i10.Future); +} + +/// A class which mocks [PhoenixSocketOptions]. +/// +/// See the documentation for Mockito's code generation for more information. +class MockPhoenixSocketOptions extends _i1.Mock + implements _i14.PhoenixSocketOptions { + @override + _i8.MessageSerializer get serializer => (super.noSuchMethod( + Invocation.getter(#serializer), + returnValue: _FakeMessageSerializer_8( + this, + Invocation.getter(#serializer), + ), + returnValueForMissingStub: _FakeMessageSerializer_8( + this, + Invocation.getter(#serializer), + ), + ) as _i8.MessageSerializer); + + @override + List get reconnectDelays => (super.noSuchMethod( + Invocation.getter(#reconnectDelays), + returnValue: [], + returnValueForMissingStub: [], + ) as List); + + @override + Duration get timeout => (super.noSuchMethod( + Invocation.getter(#timeout), + returnValue: _FakeDuration_3( + this, + Invocation.getter(#timeout), + ), + returnValueForMissingStub: _FakeDuration_3( + this, + Invocation.getter(#timeout), + ), + ) as Duration); + + @override + Duration get heartbeat => (super.noSuchMethod( + Invocation.getter(#heartbeat), + returnValue: _FakeDuration_3( + this, + Invocation.getter(#heartbeat), + ), + returnValueForMissingStub: _FakeDuration_3( + this, + Invocation.getter(#heartbeat), + ), + ) as Duration); + + @override + _i10.Future> getParams() => (super.noSuchMethod( + Invocation.method( + #getParams, + [], + ), + returnValue: _i10.Future>.value({}), + returnValueForMissingStub: + _i10.Future>.value({}), + ) as _i10.Future>); } diff --git a/test/socket_test.dart b/test/socket_test.dart index 763353c..a614c4e 100644 --- a/test/socket_test.dart +++ b/test/socket_test.dart @@ -5,6 +5,7 @@ import 'package:mockito/mockito.dart'; import 'package:phoenix_socket/phoenix_socket.dart'; import 'package:rxdart/rxdart.dart'; import 'package:test/test.dart'; +import 'package:web_socket_channel/web_socket_channel.dart'; import 'mocks.dart'; @@ -50,4 +51,64 @@ void main() { // Expect the first two unexpected failures to be retried verify(sink.add(any)).called(3); }); + + test('socket connect does not create new socket if one is already connected', + () async { + final optionsCompleter = Completer>(); + final mockPhoenixSocketOptions = MockPhoenixSocketOptions(); + when(mockPhoenixSocketOptions.getParams()) + .thenAnswer((_) => optionsCompleter.future); + when(mockPhoenixSocketOptions.heartbeat).thenReturn(Duration(days: 1)); + + final sentRefs = []; + when(mockPhoenixSocketOptions.serializer).thenReturn(MessageSerializer( + encoder: (object) { + if (object is List) { + final message = Message.fromJson(object); + sentRefs.add(message.ref!); + return message.ref!; + } + return 'ignored'; + }, + decoder: (ref) => Message.heartbeat(ref).encode(), + )); + + int factoryCalls = 0; + WebSocketChannel stubWebSocketChannelFactory(Uri uri) { + ++factoryCalls; + final mockWebSocketChannel = MockWebSocketChannel(); + when(mockWebSocketChannel.stream).thenAnswer((_) => NeverStream()); + when(mockWebSocketChannel.ready) + .thenAnswer((_) => Future.sync(() => null)); + when(mockWebSocketChannel.sink).thenReturn(MockWebSocketSink()); + return mockWebSocketChannel; + } + + final phoenixSocket = PhoenixSocket( + 'ws://endpoint', + webSocketChannelFactory: stubWebSocketChannelFactory, + socketOptions: mockPhoenixSocketOptions, + ); + + // Connect to the socket + final connectFutures = [ + phoenixSocket.connect(), + phoenixSocket.connect(), + phoenixSocket.connect(), + ]; + + expect(factoryCalls, 0); + + optionsCompleter.complete({'token': 'fakeUserToken'}); + + await Future.delayed(Duration.zero); + + for (final ref in sentRefs) { + phoenixSocket.onSocketDataCallback(ref); + } + + await Future.wait(connectFutures); + + expect(factoryCalls, 1); + }); }