fix(historian): #668 review follow-ups (explicit modified source time, paging tests, ACL test, QuestDB orphan warning, docs)

- HistoryModifiedValue gains explicit SourceTimeUtc (SqlServer + QuestDB populate it); the node manager's resume cursor uses it instead of the prior value's nullable source timestamp (MinValue for inserts). - ACL test now uses a trail-returning data source and asserts Good + data on the allow leg. - Node-manager tests: end-only backward open-ended paging, reverse resume cursor, null-prior cursor. - QuestDB updater logs a Warning (tag + time range) when a sample write fails after its trail rows landed. - docs/Historian.md: rolling-upgrade window, IDENTITY-vs-commit ordering, QuestDB cross-node ModifiedId clock ordering.
2026-10-11 09:37:17 -04:00
parent 062086f9a6
commit 7c80d36eb6
9 changed files with 138 additions and 15 deletions
@@ -313,7 +313,7 @@ public sealed class HistoryUpdateOverTheWireTests
        private void Record(DateTime t, double? prior, HistoryModificationType type, HistoryUpdateContext who) =>
            _trail.Add(new HistoryModifiedValue(
                new DataValueSnapshot(prior, prior is null ? StatusCodes.BadNoData : StatusCodes.Good, t, t),
-                DateTime.UtcNow, type, who.Principal, _trail.Count + 1));
+                DateTime.UtcNow, type, who.Principal, _trail.Count + 1, t));

        public int UpdateCalls => Volatile.Read(ref _updateCalls);

@@ -101,7 +101,7 @@ public sealed class NodeManagerAclEnforcementTests : IDisposable
        try
        {
            var nm = server.NodeManager!;
-            nm.HistorianDataSource = NullHistorianDataSource.Instance;
+            nm.HistorianDataSource = new OneModificationTrail();
            var nodeId = Historized(nm, "Plant/Modbus/dev1/Speed");

            nm.Authorizer = Authorizer(enabled: true, GrantOn(NodePermissions.Read));
@@ -110,8 +110,14 @@ public sealed class NodeManagerAclEnforcementTests : IDisposable
            deniedResults[0].StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied);

            nm.Authorizer = Authorizer(enabled: true, GrantOn(NodePermissions.HistoryRead));
-            var (_, allowedErrors) = ReadRaw(nm, Identity(), modified: true, nodeId);
-            allowedErrors[0].StatusCode.Code.ShouldBe(StatusCodes.BadHistoryOperationUnsupported);
+            var (allowedResults, allowedErrors) = ReadRaw(nm, Identity(), modified: true, nodeId);
+            // A real trail entry must come back: Null's BadHistoryOperationUnsupported is also what the SDK pre-seeds,
+            // so asserting it would pass even if the grant never reached the modified branch.
+            allowedErrors[0].StatusCode.Code.ShouldBe(StatusCodes.Good);
+            allowedResults[0].StatusCode.Code.ShouldBe(StatusCodes.Good);
+            var data = ExtensionObject.ToEncodeable(allowedResults[0].HistoryData).ShouldBeOfType<HistoryModifiedData>();
+            data.DataValues.Count.ShouldBe(1);
+            data.ModificationInfos[0].UserName.ShouldBe("alice");
        }
        finally { await host.DisposeAsync(); }
    }
@@ -327,6 +333,32 @@ public sealed class NodeManagerAclEnforcementTests : IDisposable
        }
    }

+    /// <summary>A backend whose modified-value trail holds exactly one entry; every other read is empty.</summary>
+    private sealed class OneModificationTrail : IHistorianDataSource
+    {
+        public Task<HistoryModifiedReadResult> ReadModifiedAsync(
+            string fullReference, DateTime startUtc, DateTime endUtc, uint maxValues, HistoryModifiedCursor? resumeAfter,
+            CancellationToken cancellationToken)
+        {
+            var t = DateTime.UtcNow.AddMinutes(-5);
+            var prior = new DataValueSnapshot(1.5, StatusCodes.Good, t, t);
+            return Task.FromResult(new HistoryModifiedReadResult(
+                [new HistoryModifiedValue(prior, DateTime.UtcNow, HistoryModificationType.Replace, "alice", 1, t)], false));
+        }
+
+        public Task<ZB.MOM.WW.OtOpcUa.Core.Abstractions.HistoryReadResult> ReadRawAsync(string r, DateTime s, DateTime e, uint m, CancellationToken ct) =>
+            NullHistorianDataSource.Instance.ReadRawAsync(r, s, e, m, ct);
+        public Task<ZB.MOM.WW.OtOpcUa.Core.Abstractions.HistoryReadResult> ReadProcessedAsync(
+            string r, DateTime s, DateTime e, TimeSpan i, HistoryAggregateType a, CancellationToken ct) =>
+            NullHistorianDataSource.Instance.ReadProcessedAsync(r, s, e, i, a, ct);
+        public Task<ZB.MOM.WW.OtOpcUa.Core.Abstractions.HistoryReadResult> ReadAtTimeAsync(string r, IReadOnlyList<DateTime> t, CancellationToken ct) =>
+            NullHistorianDataSource.Instance.ReadAtTimeAsync(r, t, ct);
+        public Task<HistoricalEventsResult> ReadEventsAsync(string? n, DateTime s, DateTime e, int m, CancellationToken ct) =>
+            NullHistorianDataSource.Instance.ReadEventsAsync(n, s, e, m, ct);
+        public HistorianHealthSnapshot GetHealthSnapshot() => NullHistorianDataSource.Instance.GetHealthSnapshot();
+        public void Dispose() { }
+    }
+
    private static NodeId Historized(OtOpcUaNodeManager nm, string rawPath)
    {
        nm.EnsureVariable(rawPath, parentFolderNodeId: null, displayName: rawPath, dataType: "Float",
@@ -105,6 +105,56 @@ public sealed class NodeManagerHistoryReadModifiedTests : IDisposable
        await host.DisposeAsync();
    }

+    [Fact]
+    public async Task An_end_only_modified_read_pages_backward_from_the_end()
+    {
+        var trail = Enumerable.Range(1, 9).Select(i => Mod(i, seconds: i, prior: i, HistoryModificationType.Update, "u", Epoch)).ToList();
+        var (host, nm, node, src) = await SetupAsync(trail);
+
+        // StartTime unset + EndTime set + a cap = "the newest N, walking backward".
+        Drain(nm, node, DateTime.MinValue, Epoch.AddHours(1), max: 4).ShouldBe(src.Ordered(reverse: true).Select(m => m.ModifiedId));
+        src.LastStart.ShouldBe(Epoch.AddHours(1));
+        src.LastEnd.ShouldBe(new DateTime(0, DateTimeKind.Utc));
+        await host.DisposeAsync();
+    }
+
+    [Fact]
+    public async Task A_reverse_resume_hands_the_backend_the_last_returned_source_time_and_id()
+    {
+        // Ids 3 and 4 share a source time; page 1 (reverse, cap 3) ends on id 4, then resumes mid-run.
+        var trail = new List<HistoryModifiedValue>
+        {
+            Mod(1, 1, 1, HistoryModificationType.Update, "u", Epoch), Mod(2, 2, 2, HistoryModificationType.Update, "u", Epoch),
+            Mod(3, 3, 3, HistoryModificationType.Update, "u", Epoch), Mod(4, 3, 4, HistoryModificationType.Update, "u", Epoch),
+            Mod(5, 5, 5, HistoryModificationType.Update, "u", Epoch), Mod(6, 6, 6, HistoryModificationType.Update, "u", Epoch),
+        };
+        var (host, nm, node, src) = await SetupAsync(trail);
+
+        var p1 = Read(nm, node, Epoch.AddHours(1), Epoch.AddSeconds(-1), max: 3, cp: null);
+        p1.Ids.ShouldBe([6, 5, 4]);
+        p1.Cp.ShouldNotBeNull();
+        var p2 = Read(nm, node, Epoch.AddHours(1), Epoch.AddSeconds(-1), max: 3, cp: p1.Cp);
+        src.LastCursor.ShouldBe(new HistoryModifiedCursor(Epoch.AddSeconds(3), 4));
+        p2.Ids.ShouldBe([3, 2, 1]);
+        await host.DisposeAsync();
+    }
+
+    [Fact]
+    public async Task The_resume_cursor_uses_the_explicit_source_time_even_when_the_prior_value_carries_none()
+    {
+        // Inserts have no prior value; their snapshot has no source timestamp. The cursor must not fall back to MinValue.
+        var trail = Enumerable.Range(1, 6).Select(i => new HistoryModifiedValue(
+            new DataValueSnapshot(null, StatusCodes.BadNoData, null, Epoch), Epoch, HistoryModificationType.Insert, "u", i,
+            Epoch.AddSeconds(i))).ToList();
+        var (host, nm, node, src) = await SetupAsync(trail);
+
+        var p1 = Read(nm, node, Epoch, Epoch.AddHours(1), max: 3, cp: null);
+        p1.Cp.ShouldNotBeNull();
+        Read(nm, node, Epoch, Epoch.AddHours(1), max: 3, cp: p1.Cp);
+        src.LastCursor.ShouldBe(new HistoryModifiedCursor(Epoch.AddSeconds(3), 3));
+        await host.DisposeAsync();
+    }
+
    [Fact]
    public async Task An_open_ended_forward_modified_read_pages_to_the_end()
    {
@@ -172,7 +222,7 @@ public sealed class NodeManagerHistoryReadModifiedTests : IDisposable
        var value = prior is null
            ? new DataValueSnapshot(null, StatusCodes.BadNoData, t, modifiedAt)
            : new DataValueSnapshot(prior.Value, StatusCodes.Good, t, t);
-        return new HistoryModifiedValue(value, modifiedAt, type, user, id);
+        return new HistoryModifiedValue(value, modifiedAt, type, user, id, t);
    }

    private static List<long> Drain(OtOpcUaNodeManager nm, NodeId node, DateTime start, DateTime end, uint max)
@@ -258,8 +308,8 @@ public sealed class NodeManagerHistoryReadModifiedTests : IDisposable
        public static long IdOf(DataValue v) => v.ServerTimestamp.Ticks - Epoch.Ticks;

        public IEnumerable<HistoryModifiedValue> Ordered(bool reverse) => reverse
-            ? trail.OrderByDescending(m => m.Value.SourceTimestampUtc).ThenByDescending(m => m.ModifiedId)
-            : trail.OrderBy(m => m.Value.SourceTimestampUtc).ThenBy(m => m.ModifiedId);
+            ? trail.OrderByDescending(m => m.SourceTimeUtc).ThenByDescending(m => m.ModifiedId)
+            : trail.OrderBy(m => m.SourceTimeUtc).ThenBy(m => m.ModifiedId);

        public async Task<HistoryModifiedReadResult> ReadModifiedAsync(
            string fullReference, DateTime startUtc, DateTime endUtc, uint maxValues, HistoryModifiedCursor? resumeAfter,
@@ -273,7 +323,7 @@ public sealed class NodeManagerHistoryReadModifiedTests : IDisposable
            var (lo, hi) = reverse ? (endUtc, startUtc) : (startUtc, endUtc);
            var rows = Ordered(reverse).Where(m =>
            {
-                var t = m.Value.SourceTimestampUtc!.Value;
+                var t = m.SourceTimeUtc;
                var inWindow = startUtc == endUtc ? t == lo : reverse ? t > lo && t <= hi : t >= lo && t < hi;
                if (!inWindow) return false;
                if (resumeAfter is null) return true;