mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-29 07:07:24 +08:00
Hey everyone! 👋 Just wanted to contribute! I learned about the OPC Foundation during my last internship and got introduced to open source coding, so here we are! I noticed issue #4863 while exploring the codebase and wanted to help address it. --- # Description ## Problem In `ReverseConnectServer.OnReverseConnect` (`src/Opc.Ua.Server/Server/ReverseConnectServer.cs`), reverse connection admission was evaluated using a combination of `MaxSessionCount` and `ServerInternal.SessionManager.GetSessions().Count`: ```csharp if (reverseConnection.Enabled && ( reverseConnection.MaxSessionCount == 0 || ( reverseConnection.MaxSessionCount == 1 && reverseConnection.LastState == ReverseConnectState.Closed ) || reverseConnection.MaxSessionCount > ServerInternal.SessionManager .GetSessions() .Count)) ``` As reported in #4863, this logic causes several issues: 1. **Redundant connection attempts:** When `MaxSessionCount == 0` (the default if omitted or unconfigured), `MaxSessionCount == 0` is always true. Every 15-second timer tick dispatches another `CreateConnection` call, even if the connection is already `Connected` or `Connecting`. 2. **Unrelated server sessions block reverse connect:** When `MaxSessionCount >= 2`, `ServerInternal.SessionManager.GetSessions().Count` queries the global session count across all endpoints (including local/direct OPC UA clients). Once direct sessions reach `MaxSessionCount`, reverse connections are permanently prevented from establishing even when in state `Closed`. 3. **Premature connections under `MaxSessionCount == 1`:** If a reverse connection is established (`Connected`) but the client has not yet activated an OPC UA session (`GetSessions().Count == 0`), `1 > 0` evaluates to true, triggering duplicate connection attempts. 4. **Stuck `Errored` state:** If `CreateConnection` throws an exception, `LastState` is set to `Errored`. If other sessions exist on the server, `1 > 1` is false, leaving the connection permanently stuck in `Errored` without retrying. --- ## Code Explanation Following maintainer guidance on #4863 to remove session-count coupling from connection admission: In `src/Opc.Ua.Server/Server/ReverseConnectServer.cs`: ```csharp // try the reverse connect if (reverseConnection.Enabled && (reverseConnection.LastState == ReverseConnectState.Closed || reverseConnection.LastState == ReverseConnectState.Errored)) ``` - **Connection state drives admission:** A reverse connection is only initiated if it is enabled and currently `Closed` (needs connection) or `Errored` (retrying after a previous failure). - **Prevents duplicate sockets:** When already `Connected` or `Connecting`, no redundant connection attempts are dispatched. - **Respects reject backoff:** When in state `Rejected`, the connection remains idle until `m_rejectTimeout` elapses and recharges the state to `Closed`. - **Decoupled from global sessions:** Unrelated client sessions on the server no longer interfere with reverse connection establishment. - **Zero API breakage:** Kept changes minimal and surgical—no public signatures were changed, ensuring full backward compatibility. --- ## Commits Breakdown The changes were structured into two focused commits: 1. **`31fc2fd95` — `test(server): add regression tests for reverse connect session accounting (#4863)`** - Adds unit regression tests in `tests/Opc.Ua.Server.Tests/ReverseConnectServerSessionAccountingTests.cs` covering all edge cases (`MaxSessionCount == 0`, `MaxSessionCount == 1`, `MaxSessionCount >= 2`, `Errored` retry, reject timeout expiry, and disabled connections) using `FakeTimeProvider`. 2. **`b820dbfed` — `fix(server): decouple reverse connect connection admission from global sessions (#4863)`** - Applies the surgical fix to `ReverseConnectServer.OnReverseConnect` to evaluate `Closed` and `Errored` states directly. --- ## Testing & Verification - **New regression suite:** All 7 tests in `ReverseConnectServerSessionAccountingTests` pass on `net10.0` (98 ms). - **Existing suite:** All 43 tests in category `ReverseConnect` pass on `net10.0` (12 s). - **Analyzers:** Zero compiler warnings and zero Roslynator analyzer warnings (`TreatWarningsAsErrors=true`). --- ## Related Issues Fixes #4863 --- ## Checklist - [] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran the affected tests locally against .NET 10 and all passed. - [] I have addressed all PR feedback received. --------- Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Marc <marcschier@hotmail.com>
This commit is contained in:
@@ -936,7 +936,6 @@ unchanged. The data binds directly from
|
||||
{
|
||||
"EndpointUrl": "opc.tcp://client.example.com:4841",
|
||||
"Timeout": 30000,
|
||||
"MaxSessionCount": 1,
|
||||
"Enabled": true
|
||||
}
|
||||
]
|
||||
|
||||
@@ -220,7 +220,6 @@ services.AddOpcUa()
|
||||
{
|
||||
EndpointUrl = "opc.tcp://client.example.com:65300",
|
||||
Timeout = 30000,
|
||||
MaxSessionCount = 1,
|
||||
Enabled = true
|
||||
});
|
||||
});
|
||||
|
||||
@@ -228,7 +228,6 @@
|
||||
<Clients>
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://localhost:65300</EndpointUrl>
|
||||
<MaxSessionCount>0</MaxSessionCount>
|
||||
<Enabled>true</Enabled>
|
||||
</ReverseConnectClient>
|
||||
</Clients>
|
||||
|
||||
@@ -371,7 +371,6 @@
|
||||
<Clients>
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://localhost:65300</EndpointUrl>
|
||||
<MaxSessionCount>0</MaxSessionCount>
|
||||
<Enabled>true</Enabled>
|
||||
</ReverseConnectClient>
|
||||
</Clients>
|
||||
|
||||
@@ -1623,18 +1623,15 @@ namespace Opc.Ua
|
||||
|
||||
/// <summary>
|
||||
/// The maximum count of active reverse connect sessions.
|
||||
/// 0 or undefined means unlimited number of sessions.
|
||||
/// 1 means a single connection is created at a time.
|
||||
/// n disables reverse hello once the total number of sessions
|
||||
/// in the server reaches n.
|
||||
/// This setting is ignored and is no longer serialized.
|
||||
/// </summary>
|
||||
[DataTypeField(Order = 2)]
|
||||
[Obsolete("MaxSessionCount is no longer supported and is ignored.")]
|
||||
public int MaxSessionCount { get; set; }
|
||||
|
||||
/// <summary>
|
||||
/// Specifies whether the sending of reverse connect attempts is enabled.
|
||||
/// </summary>
|
||||
[DataTypeField(Order = 3, DefaultValueHandling = DefaultValueHandling.Emit)]
|
||||
[DataTypeField(Order = 2, DefaultValueHandling = DefaultValueHandling.Emit)]
|
||||
public bool Enabled { get; set; } = true;
|
||||
}
|
||||
|
||||
|
||||
@@ -206,7 +206,6 @@
|
||||
<xs:sequence>
|
||||
<xs:element name="EndpointUrl" type="xs:string" minOccurs="0" nillable="true" />
|
||||
<xs:element name="Timeout" type="xs:int" minOccurs="0" />
|
||||
<xs:element name="MaxSessionCount" type="xs:int" minOccurs="0" />
|
||||
<xs:element name="Enabled" type="xs:boolean" minOccurs="0" />
|
||||
</xs:sequence>
|
||||
</xs:complexType>
|
||||
|
||||
@@ -638,7 +638,6 @@ namespace Opc.Ua.Gds.Server.Hosting
|
||||
{
|
||||
EndpointUrl = c.EndpointUrl,
|
||||
Timeout = c.Timeout,
|
||||
MaxSessionCount = c.MaxSessionCount,
|
||||
Enabled = c.Enabled
|
||||
};
|
||||
}
|
||||
|
||||
@@ -141,7 +141,6 @@ namespace Opc.Ua.Server.Hosting
|
||||
{
|
||||
EndpointUrl = client.EndpointUrl,
|
||||
Timeout = client.Timeout,
|
||||
MaxSessionCount = client.MaxSessionCount,
|
||||
Enabled = client.Enabled
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1928,8 +1928,6 @@ namespace Microsoft.Extensions.DependencyInjection
|
||||
BindString(child, nameof(ServerReverseConnectClientOptions.EndpointUrl),
|
||||
value => client.EndpointUrl = value);
|
||||
BindInt32(child, nameof(ServerReverseConnectClientOptions.Timeout), value => client.Timeout = value);
|
||||
BindInt32(child, nameof(ServerReverseConnectClientOptions.MaxSessionCount),
|
||||
value => client.MaxSessionCount = value);
|
||||
BindBoolean(child, nameof(ServerReverseConnectClientOptions.Enabled), value => client.Enabled = value);
|
||||
reverseConnect.Clients.Add(client);
|
||||
}
|
||||
|
||||
@@ -94,9 +94,9 @@ namespace Opc.Ua.Server.Hosting
|
||||
|
||||
/// <summary>
|
||||
/// Maximum simultaneous reverse-connect sessions for this client.
|
||||
/// Zero means unlimited; <c>1</c> permits a single connection at a
|
||||
/// time.
|
||||
/// This setting is ignored.
|
||||
/// </summary>
|
||||
[System.Obsolete("MaxSessionCount is no longer supported and is ignored.")]
|
||||
public int MaxSessionCount { get; set; }
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -74,6 +74,25 @@ namespace Opc.Ua.Server
|
||||
/// </summary>
|
||||
public class ReverseConnectProperty
|
||||
{
|
||||
/// <summary>
|
||||
/// Initialize a reverse connect server property.
|
||||
/// </summary>
|
||||
/// <param name="clientUrl">The Url of the reverse connect client.</param>
|
||||
/// <param name="timeout">The timeout to use for a reverse connect attempt.</param>
|
||||
/// <param name="configEntry">If this is an application configuration entry.</param>
|
||||
/// <param name="enabled">If the connection is enabled.</param>
|
||||
public ReverseConnectProperty(
|
||||
Uri clientUrl,
|
||||
int timeout,
|
||||
bool configEntry,
|
||||
bool enabled = true)
|
||||
{
|
||||
ClientUrl = clientUrl;
|
||||
Timeout = timeout > 0 ? timeout : ReverseConnectServer.DefaultReverseConnectTimeout;
|
||||
ConfigEntry = configEntry;
|
||||
Enabled = enabled;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Initialize a reverse connect server property.
|
||||
/// </summary>
|
||||
@@ -82,18 +101,18 @@ namespace Opc.Ua.Server
|
||||
/// <param name="maxSessionCount">The maximum number of sessions allowed to the client.</param>
|
||||
/// <param name="configEntry">If this is an application configuration entry.</param>
|
||||
/// <param name="enabled">If the connection is enabled.</param>
|
||||
[Obsolete("MaxSessionCount is no longer supported and has been removed from configuration.")]
|
||||
public ReverseConnectProperty(
|
||||
Uri clientUrl,
|
||||
int timeout,
|
||||
int maxSessionCount,
|
||||
bool configEntry,
|
||||
bool enabled = true)
|
||||
: this(clientUrl, timeout, configEntry, enabled)
|
||||
{
|
||||
ClientUrl = clientUrl;
|
||||
Timeout = timeout > 0 ? timeout : ReverseConnectServer.DefaultReverseConnectTimeout;
|
||||
#pragma warning disable CS0618
|
||||
MaxSessionCount = maxSessionCount;
|
||||
ConfigEntry = configEntry;
|
||||
Enabled = enabled;
|
||||
#pragma warning restore CS0618
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -124,6 +143,7 @@ namespace Opc.Ua.Server
|
||||
/// <summary>
|
||||
/// The maximum number of sessions allowed to the client.
|
||||
/// </summary>
|
||||
[Obsolete("MaxSessionCount is no longer supported and has been removed from configuration.")]
|
||||
public int MaxSessionCount;
|
||||
|
||||
/// <summary>
|
||||
@@ -232,12 +252,32 @@ namespace Opc.Ua.Server
|
||||
/// Add a reverse connection url.
|
||||
/// </summary>
|
||||
/// <exception cref="ArgumentException"></exception>
|
||||
[System.Runtime.CompilerServices.OverloadResolutionPriority(1)]
|
||||
public virtual void AddReverseConnection(
|
||||
Uri url,
|
||||
int timeout = 0,
|
||||
bool enabled = true)
|
||||
{
|
||||
#pragma warning disable CS0618
|
||||
AddReverseConnection(url, timeout, maxSessionCount: 0, enabled);
|
||||
#pragma warning restore CS0618
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Add a reverse connection url.
|
||||
/// </summary>
|
||||
/// <param name="url">The Url of the reverse connect client.</param>
|
||||
/// <param name="timeout">The timeout to use for a reverse connect attempt.</param>
|
||||
/// <param name="maxSessionCount">The maximum number of sessions allowed (deprecated and ignored).</param>
|
||||
/// <param name="enabled">If the connection is enabled.</param>
|
||||
[Obsolete("The maxSessionCount parameter is no longer supported and will be ignored.")]
|
||||
public virtual void AddReverseConnection(
|
||||
Uri url,
|
||||
int timeout = 0,
|
||||
int maxSessionCount = 0,
|
||||
bool enabled = true)
|
||||
{
|
||||
_ = maxSessionCount;
|
||||
if (m_connections.ContainsKey(url))
|
||||
{
|
||||
throw new ArgumentException(
|
||||
@@ -247,7 +287,6 @@ namespace Opc.Ua.Server
|
||||
var reverseConnection = new ReverseConnectProperty(
|
||||
url,
|
||||
timeout,
|
||||
maxSessionCount,
|
||||
false,
|
||||
enabled);
|
||||
|
||||
@@ -329,15 +368,8 @@ namespace Opc.Ua.Server
|
||||
|
||||
// try the reverse connect
|
||||
if (reverseConnection.Enabled &&
|
||||
(
|
||||
reverseConnection.MaxSessionCount == 0 ||
|
||||
(
|
||||
reverseConnection.MaxSessionCount == 1 &&
|
||||
reverseConnection.LastState == ReverseConnectState.Closed
|
||||
) ||
|
||||
reverseConnection.MaxSessionCount > ServerInternal.SessionManager
|
||||
.GetSessions()
|
||||
.Count))
|
||||
reverseConnection.LastState != ReverseConnectState.Connecting &&
|
||||
reverseConnection.LastState != ReverseConnectState.Rejected)
|
||||
{
|
||||
try
|
||||
{
|
||||
@@ -526,7 +558,6 @@ namespace Opc.Ua.Server
|
||||
{
|
||||
if (existing.ConfigEntry)
|
||||
{
|
||||
existing.MaxSessionCount = client.MaxSessionCount;
|
||||
existing.Enabled = client.Enabled;
|
||||
}
|
||||
else
|
||||
@@ -539,7 +570,6 @@ namespace Opc.Ua.Server
|
||||
m_connections[uri] = new ReverseConnectProperty(
|
||||
uri,
|
||||
client.Timeout,
|
||||
client.MaxSessionCount,
|
||||
true,
|
||||
client.Enabled);
|
||||
m_logger.ReverseConnectionAddedForEndpointUrlUri(uri);
|
||||
|
||||
@@ -289,7 +289,6 @@
|
||||
<Clients>
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://localhost:65300</EndpointUrl>
|
||||
<MaxSessionCount>0</MaxSessionCount>
|
||||
<Enabled>true</Enabled>
|
||||
</ReverseConnectClient>
|
||||
</Clients>
|
||||
|
||||
@@ -872,13 +872,11 @@ namespace Opc.Ua.Configuration.Tests
|
||||
rc.Clients[0].EndpointUrl,
|
||||
Is.EqualTo("opc.tcp://client1:65300"));
|
||||
Assert.That(rc.Clients[0].Timeout, Is.EqualTo(5000));
|
||||
Assert.That(rc.Clients[0].MaxSessionCount, Is.EqualTo(10));
|
||||
Assert.That(rc.Clients[0].Enabled, Is.True);
|
||||
Assert.That(
|
||||
rc.Clients[1].EndpointUrl,
|
||||
Is.EqualTo("opc.tcp://client2:65301"));
|
||||
Assert.That(rc.Clients[1].Timeout, Is.EqualTo(10000));
|
||||
Assert.That(rc.Clients[1].MaxSessionCount, Is.EqualTo(5));
|
||||
Assert.That(rc.Clients[1].Enabled, Is.False);
|
||||
Assert.That(rc.ConnectInterval, Is.EqualTo(20000));
|
||||
Assert.That(rc.ConnectTimeout, Is.EqualTo(45000));
|
||||
|
||||
@@ -107,13 +107,11 @@
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://client1:65300</EndpointUrl>
|
||||
<Timeout>5000</Timeout>
|
||||
<MaxSessionCount>10</MaxSessionCount>
|
||||
<Enabled>true</Enabled>
|
||||
</ReverseConnectClient>
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://client2:65301</EndpointUrl>
|
||||
<Timeout>10000</Timeout>
|
||||
<MaxSessionCount>5</MaxSessionCount>
|
||||
<Enabled>false</Enabled>
|
||||
</ReverseConnectClient>
|
||||
</Clients>
|
||||
|
||||
@@ -120,8 +120,7 @@ namespace UaLens.Tests.Samples
|
||||
options.ReverseConnect.Clients.Add(new ServerReverseConnectClientOptions
|
||||
{
|
||||
EndpointUrl = listenerUrl,
|
||||
Timeout = 10000,
|
||||
MaxSessionCount = 1
|
||||
Timeout = 10000
|
||||
});
|
||||
}
|
||||
}).AddCertificateManager(manager);
|
||||
|
||||
@@ -106,8 +106,7 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
options.ReverseConnect.Clients.Add(new ServerReverseConnectClientOptions
|
||||
{
|
||||
EndpointUrl = clientUrl.AbsoluteUri,
|
||||
Timeout = 10000,
|
||||
MaxSessionCount = 1
|
||||
Timeout = 10000
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -237,7 +236,6 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
{
|
||||
EndpointUrl = clientUrl.AbsoluteUri,
|
||||
Timeout = 10000,
|
||||
MaxSessionCount = 1,
|
||||
Enabled = true
|
||||
}
|
||||
]
|
||||
|
||||
@@ -178,7 +178,6 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
{
|
||||
EndpointUrl = "opc.tcp://reverse-client:4841",
|
||||
Timeout = 500,
|
||||
MaxSessionCount = 2,
|
||||
Enabled = true
|
||||
});
|
||||
options.EndpointUrls.Add("opc.tcp://localhost:0/ReverseConnect");
|
||||
@@ -195,7 +194,6 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
Assert.That(reverseConnect.Clients.Count, Is.EqualTo(1));
|
||||
Assert.That(reverseConnect.Clients[0].EndpointUrl, Is.EqualTo("opc.tcp://reverse-client:4841"));
|
||||
Assert.That(reverseConnect.Clients[0].Timeout, Is.EqualTo(500));
|
||||
Assert.That(reverseConnect.Clients[0].MaxSessionCount, Is.EqualTo(2));
|
||||
Assert.That(reverseConnect.Clients[0].Enabled, Is.True);
|
||||
}
|
||||
|
||||
|
||||
@@ -89,7 +89,6 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
{
|
||||
EndpointUrl = "opc.tcp://client.local:4841",
|
||||
Timeout = 7000,
|
||||
MaxSessionCount = 2,
|
||||
Enabled = true
|
||||
});
|
||||
});
|
||||
@@ -104,7 +103,6 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
ServerReverseConnectClientOptions client = opts.ReverseConnect.Clients[0];
|
||||
Assert.That(client.EndpointUrl, Is.EqualTo("opc.tcp://client.local:4841"));
|
||||
Assert.That(client.Timeout, Is.EqualTo(7000));
|
||||
Assert.That(client.MaxSessionCount, Is.EqualTo(2));
|
||||
Assert.That(client.Enabled, Is.True);
|
||||
}
|
||||
|
||||
|
||||
@@ -1424,8 +1424,7 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
options.ReverseConnect.Clients.Add(new ServerReverseConnectClientOptions
|
||||
{
|
||||
EndpointUrl = enabled.ToString(),
|
||||
Timeout = 3210,
|
||||
MaxSessionCount = 0
|
||||
Timeout = 3210
|
||||
});
|
||||
options.ReverseConnect.Clients.Add(new ServerReverseConnectClientOptions
|
||||
{
|
||||
@@ -2132,8 +2131,7 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
options.ReverseConnect.Clients.Add(new ServerReverseConnectClientOptions
|
||||
{
|
||||
EndpointUrl = client.ToString(),
|
||||
Timeout = 1456,
|
||||
MaxSessionCount = 0
|
||||
Timeout = 1456
|
||||
});
|
||||
});
|
||||
await using ConfiguredAsyncDisposable cleanup = fixture.ConfigureAwait(false);
|
||||
@@ -2236,13 +2234,11 @@ namespace Opc.Ua.Server.Tests.Hosting
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://localhost:0/xml-enabled</EndpointUrl>
|
||||
<Timeout>1234</Timeout>
|
||||
<MaxSessionCount>0</MaxSessionCount>
|
||||
<Enabled>true</Enabled>
|
||||
</ReverseConnectClient>
|
||||
<ReverseConnectClient>
|
||||
<EndpointUrl>opc.tcp://localhost:0/xml-disabled</EndpointUrl>
|
||||
<Timeout>4321</Timeout>
|
||||
<MaxSessionCount>0</MaxSessionCount>
|
||||
<Enabled>false</Enabled>
|
||||
</ReverseConnectClient>
|
||||
</Clients>
|
||||
|
||||
@@ -57,7 +57,7 @@ namespace Opc.Ua.Server.Tests
|
||||
CreateConfiguration("configured").ServerConfiguration!.ReverseConnect;
|
||||
ReloadableReverseConnectServer server = await fixture.StartAsync().ConfigureAwait(false);
|
||||
var dynamicUrl = new Uri("opc.tcp://localhost:4840/dynamic");
|
||||
server.AddReverseConnection(dynamicUrl, timeout: 1234, maxSessionCount: 2, enabled: false);
|
||||
server.AddReverseConnection(dynamicUrl, timeout: 1234, enabled: false);
|
||||
ReverseConnectProperty retained = server.GetReverseConnections()[dynamicUrl];
|
||||
Assert.That(server.GetReverseConnections(), Has.Count.EqualTo(2));
|
||||
|
||||
@@ -70,7 +70,6 @@ namespace Opc.Ua.Server.Tests
|
||||
Assert.That(server.GetReverseConnections()[dynamicUrl], Is.SameAs(retained));
|
||||
Assert.That(retained.ConfigEntry, Is.False);
|
||||
Assert.That(retained.Timeout, Is.EqualTo(1234));
|
||||
Assert.That(retained.MaxSessionCount, Is.EqualTo(2));
|
||||
Assert.That(retained.Enabled, Is.False);
|
||||
}
|
||||
finally
|
||||
@@ -88,7 +87,7 @@ namespace Opc.Ua.Server.Tests
|
||||
{
|
||||
ReloadableReverseConnectServer server = await fixture.StartAsync().ConfigureAwait(false);
|
||||
var dynamicUrl = new Uri("opc.tcp://localhost:4840/dynamic");
|
||||
server.AddReverseConnection(dynamicUrl, timeout: 1234, maxSessionCount: 2, enabled: false);
|
||||
server.AddReverseConnection(dynamicUrl, timeout: 1234, enabled: false);
|
||||
ReverseConnectProperty retained = server.GetReverseConnections()[dynamicUrl];
|
||||
fixture.Config.ServerConfiguration!.ReverseConnect = new ReverseConnectServerConfiguration
|
||||
{
|
||||
@@ -98,7 +97,6 @@ namespace Opc.Ua.Server.Tests
|
||||
{
|
||||
EndpointUrl = dynamicUrl.AbsoluteUri,
|
||||
Timeout = 9000,
|
||||
MaxSessionCount = 8,
|
||||
Enabled = true
|
||||
}
|
||||
]
|
||||
@@ -111,7 +109,6 @@ namespace Opc.Ua.Server.Tests
|
||||
Assert.That(server.GetReverseConnections()[dynamicUrl], Is.SameAs(retained));
|
||||
Assert.That(retained.ConfigEntry, Is.False);
|
||||
Assert.That(retained.Timeout, Is.EqualTo(1234));
|
||||
Assert.That(retained.MaxSessionCount, Is.EqualTo(2));
|
||||
Assert.That(retained.Enabled, Is.False);
|
||||
}
|
||||
finally
|
||||
@@ -128,7 +125,7 @@ namespace Opc.Ua.Server.Tests
|
||||
{
|
||||
using var server = new ReverseConnectServer(NUnitTelemetryContext.Create(), new FakeTimeProvider());
|
||||
var dynamicUrl = new Uri("opc.tcp://localhost:4840/dynamic");
|
||||
server.AddReverseConnection(dynamicUrl, timeout: 1234, maxSessionCount: 2, enabled: false);
|
||||
server.AddReverseConnection(dynamicUrl, timeout: 1234, enabled: false);
|
||||
MethodInfo method = typeof(ReverseConnectServer).GetMethod(
|
||||
"UpdateConfiguration", BindingFlags.Instance | BindingFlags.NonPublic)!;
|
||||
#if NET5_0_OR_GREATER
|
||||
@@ -148,7 +145,6 @@ namespace Opc.Ua.Server.Tests
|
||||
ReverseConnectProperty retained = server.GetReverseConnections()[dynamicUrl];
|
||||
Assert.That(retained.ConfigEntry, Is.False);
|
||||
Assert.That(retained.Timeout, Is.EqualTo(1234));
|
||||
Assert.That(retained.MaxSessionCount, Is.EqualTo(2));
|
||||
Assert.That(retained.Enabled, Is.False);
|
||||
}
|
||||
|
||||
|
||||
@@ -53,7 +53,6 @@ namespace Opc.Ua.Server.Tests
|
||||
var property = new ReverseConnectProperty(
|
||||
s_clientUrl,
|
||||
timeout: 5000,
|
||||
maxSessionCount: 3,
|
||||
configEntry: true,
|
||||
enabled: true);
|
||||
|
||||
@@ -61,7 +60,6 @@ namespace Opc.Ua.Server.Tests
|
||||
{
|
||||
Assert.That(property.ClientUrl, Is.EqualTo(s_clientUrl));
|
||||
Assert.That(property.Timeout, Is.EqualTo(5000));
|
||||
Assert.That(property.MaxSessionCount, Is.EqualTo(3));
|
||||
Assert.That(property.ConfigEntry, Is.True);
|
||||
Assert.That(property.Enabled, Is.True);
|
||||
Assert.That(property.LastState, Is.EqualTo(ReverseConnectState.Closed));
|
||||
@@ -76,7 +74,6 @@ namespace Opc.Ua.Server.Tests
|
||||
var property = new ReverseConnectProperty(
|
||||
s_clientUrl,
|
||||
timeout,
|
||||
maxSessionCount: 0,
|
||||
configEntry: false);
|
||||
|
||||
Assert.That(
|
||||
@@ -90,7 +87,6 @@ namespace Opc.Ua.Server.Tests
|
||||
var property = new ReverseConnectProperty(
|
||||
s_clientUrl,
|
||||
timeout: 1000,
|
||||
maxSessionCount: 0,
|
||||
configEntry: false);
|
||||
|
||||
Assert.That(property.Enabled, Is.True);
|
||||
@@ -102,7 +98,6 @@ namespace Opc.Ua.Server.Tests
|
||||
var property = new ReverseConnectProperty(
|
||||
s_clientUrl,
|
||||
timeout: 1000,
|
||||
maxSessionCount: 0,
|
||||
configEntry: false,
|
||||
enabled: false);
|
||||
|
||||
@@ -115,12 +110,10 @@ namespace Opc.Ua.Server.Tests
|
||||
var property = new ReverseConnectProperty(
|
||||
s_clientUrl,
|
||||
timeout: 1000,
|
||||
maxSessionCount: 0,
|
||||
configEntry: false);
|
||||
var rejectTime = new DateTime(2024, 1, 1, 0, 0, 0, DateTimeKind.Utc);
|
||||
|
||||
property.LastState = ReverseConnectState.Rejected;
|
||||
property.MaxSessionCount = 7;
|
||||
property.Enabled = false;
|
||||
property.RejectTime = rejectTime;
|
||||
property.ServiceResult = new ServiceResult(StatusCodes.BadTimeout);
|
||||
@@ -128,7 +121,6 @@ namespace Opc.Ua.Server.Tests
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(property.LastState, Is.EqualTo(ReverseConnectState.Rejected));
|
||||
Assert.That(property.MaxSessionCount, Is.EqualTo(7));
|
||||
Assert.That(property.Enabled, Is.False);
|
||||
Assert.That(property.RejectTime, Is.EqualTo(rejectTime));
|
||||
Assert.That(property.ServiceResult, Is.Not.Null);
|
||||
@@ -158,7 +150,6 @@ namespace Opc.Ua.Server.Tests
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(property.Timeout, Is.EqualTo(ReverseConnectServer.DefaultReverseConnectTimeout));
|
||||
Assert.That(property.MaxSessionCount, Is.Zero);
|
||||
Assert.That(property.Enabled, Is.True);
|
||||
Assert.That(property.ConfigEntry, Is.False);
|
||||
});
|
||||
@@ -172,14 +163,12 @@ namespace Opc.Ua.Server.Tests
|
||||
server.AddReverseConnection(
|
||||
s_clientUrl,
|
||||
timeout: 2500,
|
||||
maxSessionCount: 4,
|
||||
enabled: false);
|
||||
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_clientUrl];
|
||||
Assert.Multiple(() =>
|
||||
{
|
||||
Assert.That(property.Timeout, Is.EqualTo(2500));
|
||||
Assert.That(property.MaxSessionCount, Is.EqualTo(4));
|
||||
Assert.That(property.Enabled, Is.False);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -0,0 +1,326 @@
|
||||
/* ========================================================================
|
||||
* Copyright (c) 2005-2026 The OPC Foundation, Inc. All rights reserved.
|
||||
*
|
||||
* OPC Foundation MIT License 1.00
|
||||
*
|
||||
* Permission is hereby granted, free of charge, to any person
|
||||
* obtaining a copy of this software and associated documentation
|
||||
* files (the "Software"), to deal in the Software without
|
||||
* restriction, including without limitation the rights to use,
|
||||
* copy, modify, merge, publish, distribute, sublicense, and/or sell
|
||||
* copies of the Software, and to permit persons to whom the
|
||||
* Software is furnished to do so, subject to the following
|
||||
* conditions:
|
||||
*
|
||||
* The above copyright notice and this permission notice shall be
|
||||
* included in all copies or substantial portions of the Software.
|
||||
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
|
||||
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
|
||||
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
|
||||
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
|
||||
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
|
||||
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
|
||||
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
|
||||
* OTHER DEALINGS IN THE SOFTWARE.
|
||||
*
|
||||
* The complete license agreement can be found here:
|
||||
* http://opcfoundation.org/License/MIT/1.00/
|
||||
* ======================================================================*/
|
||||
|
||||
using System;
|
||||
using System.Reflection;
|
||||
using System.Threading.Tasks;
|
||||
using Microsoft.Extensions.Time.Testing;
|
||||
using NUnit.Framework;
|
||||
using Opc.Ua.Server.TestFramework;
|
||||
using Opc.Ua.Tests;
|
||||
|
||||
namespace Opc.Ua.Server.Tests
|
||||
{
|
||||
/// <summary>
|
||||
/// Regression tests for Issue #4863: reverse connect connection admission must not
|
||||
/// be blocked by unrelated server sessions or an existing connection.
|
||||
/// </summary>
|
||||
[TestFixture]
|
||||
[Category("ReverseConnect")]
|
||||
[NonParallelizable]
|
||||
public class ReverseConnectServerSessionAccountingTests
|
||||
{
|
||||
private static readonly Uri s_reverseClientUri = new("opc.tcp://reverseclient.local:4840");
|
||||
|
||||
private sealed class TestableReverseConnectServer : ReverseConnectServer
|
||||
{
|
||||
public TestableReverseConnectServer(ITelemetryContext telemetry)
|
||||
: this(telemetry, new FakeTimeProvider())
|
||||
{
|
||||
}
|
||||
|
||||
public TestableReverseConnectServer(ITelemetryContext telemetry, TimeProvider timeProvider)
|
||||
: base(telemetry, timeProvider)
|
||||
{
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Number of calls dispatched to the legacy overload.
|
||||
/// </summary>
|
||||
public int LegacyAddReverseConnectionCallCount { get; private set; }
|
||||
|
||||
[Obsolete]
|
||||
public override void AddReverseConnection(
|
||||
Uri url,
|
||||
int timeout = 0,
|
||||
int maxSessionCount = 0,
|
||||
bool enabled = true)
|
||||
{
|
||||
LegacyAddReverseConnectionCallCount++;
|
||||
base.AddReverseConnection(url, timeout, maxSessionCount, enabled);
|
||||
}
|
||||
|
||||
public void InvokeOnReverseConnect(long version)
|
||||
{
|
||||
MethodInfo method = typeof(ReverseConnectServer).GetMethod(
|
||||
"OnReverseConnect",
|
||||
BindingFlags.Instance | BindingFlags.NonPublic)!;
|
||||
method.Invoke(this, [version]);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that the new overload still dispatches through the legacy virtual overload.
|
||||
/// </summary>
|
||||
[Test]
|
||||
public void AddReverseConnectionPreservesLegacyVirtualDispatch()
|
||||
{
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create());
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri);
|
||||
|
||||
Assert.That(server.LegacyAddReverseConnectionCallCount, Is.EqualTo(1));
|
||||
Assert.That(server.GetReverseConnections(), Contains.Key(s_reverseClientUri));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ConnectedReverseConnection_CanBeReconnected()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create(), fakeTime);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
// Simulate the reverse connection has successfully established
|
||||
property.LastState = ReverseConnectState.Connected;
|
||||
|
||||
// Trigger the reverse connect tick
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
// A new connection is required to recreate a session without outbound discovery.
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Errored),
|
||||
"A Connected reverse connection must allow another connection attempt.");
|
||||
Assert.That(property.ServiceResult?.AdditionalInfo, Does.Contain("No suitable listener found"));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that an established reverse connection can be replaced when the server has no sessions.
|
||||
/// </summary>
|
||||
[Test]
|
||||
public async Task ConnectedReverseConnectionCanBeReplacedWhenStartedServerHasZeroSessionsAsync()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
var fixture = new ServerFixture<TestableReverseConnectServer>(
|
||||
telemetry => new TestableReverseConnectServer(telemetry, fakeTime));
|
||||
try
|
||||
{
|
||||
await fixture.LoadConfigurationAsync().ConfigureAwait(false);
|
||||
fixture.Config.ServerConfiguration!.ReverseConnect = new ReverseConnectServerConfiguration
|
||||
{
|
||||
Clients = []
|
||||
};
|
||||
TestableReverseConnectServer server = await fixture.StartAsync().ConfigureAwait(false);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
// Connection is already established, but server has 0 active sessions
|
||||
property.LastState = ReverseConnectState.Connected;
|
||||
|
||||
// Fire the server's current timer (initial delay is one second)
|
||||
fakeTime.Advance(TimeSpan.FromSeconds(2));
|
||||
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Connecting),
|
||||
"A Connected reverse connection must allow a replacement connection attempt.");
|
||||
}
|
||||
finally
|
||||
{
|
||||
await fixture.StopAsync().ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that reverse connections are attempted with a full legacy session threshold.
|
||||
/// </summary>
|
||||
[TestCase(ReverseConnectState.Closed)]
|
||||
[TestCase(ReverseConnectState.Errored)]
|
||||
[TestCase(ReverseConnectState.Connected)]
|
||||
public async Task ReverseConnectionIsAttemptedWhenLegacySessionThresholdIsReachedAsync(
|
||||
ReverseConnectState initialState)
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
var fixture = new ServerFixture<TestableReverseConnectServer>(
|
||||
telemetry => new TestableReverseConnectServer(telemetry, fakeTime));
|
||||
try
|
||||
{
|
||||
await fixture.LoadConfigurationAsync().ConfigureAwait(false);
|
||||
fixture.Config.ServerConfiguration!.ReverseConnect = new ReverseConnectServerConfiguration
|
||||
{
|
||||
Clients = []
|
||||
};
|
||||
TestableReverseConnectServer server = await fixture.StartAsync().ConfigureAwait(false);
|
||||
await server.CreateAndActivateSessionAsync("reverse-connect-session-1").ConfigureAwait(false);
|
||||
await server.CreateAndActivateSessionAsync("reverse-connect-session-2").ConfigureAwait(false);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
#pragma warning disable CS0618
|
||||
property.MaxSessionCount = 2;
|
||||
#pragma warning restore CS0618
|
||||
property.LastState = initialState;
|
||||
|
||||
Assert.That(server.CurrentInstance.SessionManager.GetSessions(), Has.Count.EqualTo(2));
|
||||
|
||||
// Fire the current timer after its one-second initial delay.
|
||||
fakeTime.Advance(TimeSpan.FromSeconds(2));
|
||||
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Connecting),
|
||||
$"A {initialState} connection must be attempted even when the legacy session threshold is reached.");
|
||||
}
|
||||
finally
|
||||
{
|
||||
await fixture.StopAsync().ConfigureAwait(false);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ConnectingReverseConnection_IsNotReconnected_WhileConnectionInProgress()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create(), fakeTime);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
// Simulate connection attempt in flight
|
||||
property.LastState = ReverseConnectState.Connecting;
|
||||
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Connecting),
|
||||
"A reverse connection that is currently Connecting must not trigger a redundant connection attempt.");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ClosedReverseConnection_InitiatesConnection_RegardlessOfServerSessions()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create(), fakeTime);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
Assert.That(property.LastState, Is.EqualTo(ReverseConnectState.Closed));
|
||||
|
||||
// Tick should attempt connection (which throws ArgumentException because no listener is configured in unit test)
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Errored),
|
||||
"A Closed connection must attempt connection on tick.");
|
||||
Assert.That(
|
||||
property.ServiceResult?.AdditionalInfo,
|
||||
Does.Contain("No suitable listener found"));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void ErroredReverseConnection_IsRetried_OnSubsequentTick()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create(), fakeTime);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
// Simulate that an earlier connection attempt errored
|
||||
property.LastState = ReverseConnectState.Errored;
|
||||
property.ServiceResult = new ServiceResult(StatusCodes.BadCommunicationError);
|
||||
|
||||
// On the next tick, it should attempt to reconnect (which updates ServiceResult)
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
Assert.That(property.ServiceResult, Is.Not.Null);
|
||||
Assert.That(
|
||||
property.ServiceResult!.AdditionalInfo,
|
||||
Does.Contain("No suitable listener found"),
|
||||
"An Errored reverse connection should be retried on subsequent ticks.");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void RejectedReverseConnection_DoesNotConnect_UntilRejectTimeoutExpires()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create(), fakeTime);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: true);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
property.LastState = ReverseConnectState.Rejected;
|
||||
property.RejectTime = fakeTime.GetUtcNow().UtcDateTime;
|
||||
|
||||
// Advance time only by 10s (default reject timeout is 60s)
|
||||
fakeTime.Advance(TimeSpan.FromSeconds(10));
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Rejected),
|
||||
"Rejected connection must not connect before reject timeout elapses.");
|
||||
|
||||
// Advance time past the 60s reject timeout
|
||||
fakeTime.Advance(TimeSpan.FromSeconds(51));
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
// It should now have recharged to Closed and attempted connect (becoming Errored due to no listener in test)
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Errored),
|
||||
"Rejected connection should attempt connect once reject timeout expires.");
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void DisabledReverseConnection_DoesNotConnect()
|
||||
{
|
||||
var fakeTime = new FakeTimeProvider();
|
||||
using var server = new TestableReverseConnectServer(NUnitTelemetryContext.Create(), fakeTime);
|
||||
|
||||
server.AddReverseConnection(s_reverseClientUri, timeout: 5000, enabled: false);
|
||||
ReverseConnectProperty property = server.GetReverseConnections()[s_reverseClientUri];
|
||||
|
||||
Assert.That(property.LastState, Is.EqualTo(ReverseConnectState.Closed));
|
||||
|
||||
server.InvokeOnReverseConnect(0);
|
||||
|
||||
Assert.That(
|
||||
property.LastState,
|
||||
Is.EqualTo(ReverseConnectState.Closed),
|
||||
"Disabled reverse connection must not attempt connection.");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user