fix(site-runtime,dcl): low-severity cleanups — observed adapter dispose (S9), alarm-filter overwrite warning (S10), bounded-channel comment (C2), invariant-culture condition fallback (C6)
This commit is contained in:
@@ -229,7 +229,12 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers
|
|||||||
{
|
{
|
||||||
_log.Info("DataConnectionActor [{0}] stopping — disposing adapter", _connectionName);
|
_log.Info("DataConnectionActor [{0}] stopping — disposing adapter", _connectionName);
|
||||||
_adapter.Disconnected -= OnAdapterDisconnected;
|
_adapter.Disconnected -= OnAdapterDisconnected;
|
||||||
_ = _adapter.DisposeAsync().AsTask();
|
// S9: fire-and-forget, but observe a faulted dispose so a failing adapter
|
||||||
|
// teardown is logged rather than swallowed into an unobserved task exception.
|
||||||
|
_adapter.DisposeAsync().AsTask().ContinueWith(
|
||||||
|
t => _log.Warning("[{0}] Adapter dispose faulted on stop: {1}",
|
||||||
|
_connectionName, t.Exception?.GetBaseException().Message),
|
||||||
|
TaskContinuationOptions.OnlyOnFaulted);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
@@ -1829,6 +1834,17 @@ public class DataConnectionActor : UntypedActor, IWithStash, IWithTimers
|
|||||||
_alarmSourceSubscribers[request.SourceReference] = subs;
|
_alarmSourceSubscribers[request.SourceReference] = subs;
|
||||||
}
|
}
|
||||||
subs.Add(subscriber);
|
subs.Add(subscriber);
|
||||||
|
// S10: the adapter feed carries a single shared condition filter per source.
|
||||||
|
// A second subscriber with a different filter silently overwrites the first
|
||||||
|
// (last-writer-wins). Surface it so a co-subscriber mismatch is diagnosable —
|
||||||
|
// co-subscribers to one source are expected to agree on the filter.
|
||||||
|
if (_alarmSourceFilter.TryGetValue(request.SourceReference, out var existingFilter)
|
||||||
|
&& !string.Equals(existingFilter, request.ConditionFilter, StringComparison.Ordinal))
|
||||||
|
{
|
||||||
|
_log.Warning(
|
||||||
|
"[{0}] Alarm condition filter for source {1} overwritten: '{2}' -> '{3}' (last subscriber wins; co-subscribers must agree)",
|
||||||
|
_connectionName, request.SourceReference, existingFilter, request.ConditionFilter);
|
||||||
|
}
|
||||||
_alarmSourceFilter[request.SourceReference] = request.ConditionFilter;
|
_alarmSourceFilter[request.SourceReference] = request.ConditionFilter;
|
||||||
// Parse the type-name filter once; this is the authoritative client-side
|
// Parse the type-name filter once; this is the authoritative client-side
|
||||||
// gate consulted on every routed transition.
|
// gate consulted on every routed transition.
|
||||||
|
|||||||
@@ -212,11 +212,11 @@ public class SiteEventLogger : ISiteEventLogger, IDisposable
|
|||||||
// on disk I/O or on contention for the write lock.
|
// on disk I/O or on contention for the write lock.
|
||||||
if (!_writeQueue.Writer.TryWrite(pending))
|
if (!_writeQueue.Writer.TryWrite(pending))
|
||||||
{
|
{
|
||||||
// The channel is unbounded, so the only way TryWrite fails is that the
|
// The channel is bounded with DropOldest, so TryWrite only returns false
|
||||||
// writer has been completed (logger disposed). The event cannot be
|
// once the channel is completed (logger disposed) — full-buffer pressure
|
||||||
// persisted — fault the Task rather than
|
// evicts the oldest pending event instead of failing the write. The event
|
||||||
// reporting false success, so a caller that awaits a critical audit
|
// cannot be persisted — fault the Task rather than reporting false success,
|
||||||
// event can tell it was dropped.
|
// so a caller that awaits a critical audit event can tell it was dropped.
|
||||||
pending.Completion.TrySetException(
|
pending.Completion.TrySetException(
|
||||||
new ObjectDisposedException(nameof(SiteEventLogger),
|
new ObjectDisposedException(nameof(SiteEventLogger),
|
||||||
"Event could not be recorded: the event logger has been disposed."));
|
"Event could not be recorded: the event logger has been disposed."));
|
||||||
|
|||||||
@@ -475,7 +475,10 @@ public class ScriptActor : ReceiveActor, IWithTimers
|
|||||||
_scriptName, _instanceName, msg.Success);
|
_scriptName, _instanceName, msg.Success);
|
||||||
}
|
}
|
||||||
|
|
||||||
private static bool EvaluateCondition(ConditionalTriggerConfig config, object? value)
|
// internal (not private) so the culture-invariance of the non-numeric fallback
|
||||||
|
// can be unit-tested directly on the test thread — the live path evaluates on a
|
||||||
|
// dispatcher thread whose CurrentCulture the test cannot deterministically set.
|
||||||
|
internal static bool EvaluateCondition(ConditionalTriggerConfig config, object? value)
|
||||||
{
|
{
|
||||||
if (value == null) return false;
|
if (value == null) return false;
|
||||||
|
|
||||||
@@ -500,7 +503,10 @@ public class ScriptActor : ReceiveActor, IWithTimers
|
|||||||
}
|
}
|
||||||
catch
|
catch
|
||||||
{
|
{
|
||||||
return string.Equals(value.ToString(), config.Threshold.ToString(), StringComparison.Ordinal);
|
// Render the threshold with InvariantCulture to match the invariant numeric
|
||||||
|
// parse two lines up — otherwise a de-DE host renders 1.5 as "1,5" and the
|
||||||
|
// string fallback fires spuriously on locale-formatted values (C6).
|
||||||
|
return string.Equals(value.ToString(), config.Threshold.ToString(CultureInfo.InvariantCulture), StringComparison.Ordinal);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+23
@@ -81,6 +81,29 @@ public class DataConnectionActorAlarmTests : TestKit
|
|||||||
ExpectMsg<SubscribeAlarmsResponse>(m => !m.Success && m.ErrorMessage != null);
|
ExpectMsg<SubscribeAlarmsResponse>(m => !m.Success && m.ErrorMessage != null);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void SecondAlarmSubscriber_WithDifferentFilter_LogsOverwriteWarning()
|
||||||
|
{
|
||||||
|
// The adapter alarm feed carries a single shared condition filter per source;
|
||||||
|
// a second subscriber with a different filter overwrites it (last-writer-wins).
|
||||||
|
// That silent overwrite must be surfaced as a warning so a co-subscriber
|
||||||
|
// mismatch is diagnosable (S10).
|
||||||
|
var (adapter, _) = BuildAlarmAdapter();
|
||||||
|
var actor = Sys.ActorOf(Props.Create(() => new DataConnectionActor(
|
||||||
|
"conn", adapter, _options, _health, _factory, "OpcUa")));
|
||||||
|
|
||||||
|
actor.Tell(new SubscribeAlarmsRequest(
|
||||||
|
"c1", "instA", "conn", "Tank01", "AnalogLimit.Hi", DateTimeOffset.UtcNow));
|
||||||
|
ExpectMsg<SubscribeAlarmsResponse>(m => m.Success);
|
||||||
|
|
||||||
|
EventFilter.Warning(contains: "condition filter").ExpectOne(() =>
|
||||||
|
{
|
||||||
|
actor.Tell(new SubscribeAlarmsRequest(
|
||||||
|
"c2", "instB", "conn", "Tank01", "AnalogLimit.Lo", DateTimeOffset.UtcNow));
|
||||||
|
ExpectMsg<SubscribeAlarmsResponse>(m => m.Success);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// ── M2.4 (#8): conditionFilter is now applied client-side in the actor ──
|
// ── M2.4 (#8): conditionFilter is now applied client-side in the actor ──
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
@@ -1,3 +1,4 @@
|
|||||||
|
using System.Globalization;
|
||||||
using Akka.Actor;
|
using Akka.Actor;
|
||||||
using Akka.TestKit;
|
using Akka.TestKit;
|
||||||
using Akka.TestKit.Xunit2;
|
using Akka.TestKit.Xunit2;
|
||||||
@@ -300,6 +301,36 @@ public class ScriptActorTests : TestKit, IDisposable
|
|||||||
private AttributeValueChanged Change(string attribute, object? value) =>
|
private AttributeValueChanged Change(string attribute, object? value) =>
|
||||||
new("TestInstance", attribute, attribute, value, "Good", DateTimeOffset.UtcNow);
|
new("TestInstance", attribute, attribute, value, "Good", DateTimeOffset.UtcNow);
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ConditionalTrigger_NonNumericFallback_IsCultureInvariant()
|
||||||
|
{
|
||||||
|
// EvaluateCondition runs on a dispatcher thread in the live path, whose
|
||||||
|
// CurrentCulture a test cannot deterministically set — so the culture-
|
||||||
|
// invariance of the string fallback is verified by calling the pure helper
|
||||||
|
// directly under a forced de-DE culture.
|
||||||
|
var original = CultureInfo.CurrentCulture;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = new CultureInfo("de-DE"); // 1234.5 -> "1.234,5"
|
||||||
|
|
||||||
|
// "1.234,5" is the de-DE rendering of 1234.5: it FAILS the invariant
|
||||||
|
// numeric parse (decimal point then group separator) and falls through to
|
||||||
|
// the string comparison. The old fallback compared against the culture-
|
||||||
|
// sensitive Threshold.ToString() ("1.234,5" under de-DE) and matched (fired);
|
||||||
|
// the invariant fallback compares against "1234.5" and must NOT match.
|
||||||
|
var config = new ConditionalTriggerConfig("Temp", "==", 1234.5, TriggerMode.OnTrue);
|
||||||
|
Assert.False(ScriptActor.EvaluateCondition(config, "1.234,5"));
|
||||||
|
|
||||||
|
// Sanity: a value that genuinely equals the invariant rendering still matches.
|
||||||
|
Assert.True(ScriptActor.EvaluateCondition(
|
||||||
|
new ConditionalTriggerConfig("Temp", "==", 1234.5, TriggerMode.OnTrue), "1234.5"));
|
||||||
|
}
|
||||||
|
finally
|
||||||
|
{
|
||||||
|
CultureInfo.CurrentCulture = original;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private Script<object?> CompileTriggerExpression(string expression) =>
|
private Script<object?> CompileTriggerExpression(string expression) =>
|
||||||
_compilationService.CompileTriggerExpression("trigger-expr", expression).CompiledScript!;
|
_compilationService.CompileTriggerExpression("trigger-expr", expression).CompiledScript!;
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user