From 8e3bbf91fb319b1eb1051a05f09fe1696d4f1712 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:31:54 -0400 Subject: [PATCH 1/3] fix(adminui): script analysis reuses one compilation, honours cancellation, indexes the catalog (D2) R04:P2 - every analysis derives from one process-wide bound base compilation via ReplaceSyntaxTree (the scoped service keeps its current compilation); every entry point takes the request token, the endpoints bind RequestAborted and answer 499 on abort, and the Monaco providers abort their fetch when Monaco cancels the request. R04:P3 - the catalog snapshot carries a sorted, de-duplicated string[] (binary-search prefix completion) and an Ordinal path->info dictionary, built once per snapshot. R04:S1 - Diagnose emits an OTSCRIPT_SETVTAG_INERT warning for each ctx.SetVirtualTag call. It is a warning, so editor-accepts == publish-accepts still holds. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../ScriptAnalysis/IScriptTagCatalog.cs | 21 +-- .../ScriptAnalysis/ScriptAnalysisEndpoints.cs | 26 ++- .../ScriptAnalysis/ScriptAnalysisService.cs | 149 +++++++++++++----- .../ScriptAnalysis/ScriptTagCatalogIndex.cs | 82 ++++++++++ .../ScriptTagCatalogSnapshotCache.cs | 25 ++- .../wwwroot/js/monaco-init.js | 77 +++++---- .../AnalysisCancellationTests.cs | 134 ++++++++++++++++ .../AnalysisCompilationReuseTests.cs | 58 +++++++ .../ScriptTagCatalogIndexTests.cs | 79 ++++++++++ .../SetVirtualTagInertDiagnosticTests.cs | 58 +++++++ 10 files changed, 612 insertions(+), 97 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogIndex.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCancellationTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCompilationReuseTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ScriptTagCatalogIndexTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/SetVirtualTagInertDiagnosticTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs index f4b652e2..18370bc3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs @@ -105,31 +105,20 @@ public sealed class ScriptTagCatalog( /// public async Task> GetPathsAsync(string? filter, CancellationToken ct) { - var entries = await cache.GetOrBuildAsync(BuildEntriesAsync, ct); - - var paths = new HashSet(entries.Select(e => e.Path), StringComparer.OrdinalIgnoreCase); - - IEnumerable query = paths; - if (!string.IsNullOrWhiteSpace(filter)) - { - query = query.Where(p => p.StartsWith(filter, StringComparison.OrdinalIgnoreCase)); - } - - return query - .OrderBy(p => p, StringComparer.OrdinalIgnoreCase) - .Take(MaxResults) - .ToList(); + // The sorted, de-duplicated path array is built once per snapshot; a keystroke is a binary search. + var index = await cache.GetOrBuildIndexAsync(BuildEntriesAsync, ct); + return index.FindByPrefix(filter, MaxResults); } /// public async Task GetTagInfoAsync(string path, CancellationToken ct) { - var entries = await cache.GetOrBuildAsync(BuildEntriesAsync, ct); + var index = await cache.GetOrBuildIndexAsync(BuildEntriesAsync, ct); // Exact, case-SENSITIVE (Ordinal) match: the live runtime resolves a ctx.GetTag("X") literal // via DependencyMuxActor keyed with StringComparer.Ordinal, so a case-mismatched path would // not resolve at runtime — hover must report it as "not a known configured tag path". - return entries.FirstOrDefault(e => string.Equals(e.Path, path, StringComparison.Ordinal)); + return index.TryGet(path); } /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs index 6783f56f..ad598c1f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs @@ -8,20 +8,38 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; public static class ScriptAnalysisEndpoints { + /// Status returned when the editor abandoned the request (Monaco cancelled it or the fetch was + /// aborted). Nobody reads the body; it only keeps a routine cancellation out of the error logs. + private const int ClientClosedRequest = 499; + /// Maps the /api/script-analysis endpoint group (diagnostics, completions, hover, signature help, format), gated by the ConfigEditor policy (Administrator or Designer). /// The route builder to map the endpoints onto. /// The same route builder, for chaining. + /// Every analysis handler binds the request's (HttpContext.RequestAborted) + /// and passes it to the analysis, so a request the editor abandoned stops compiling instead of running to completion. public static IEndpointRouteBuilder MapScriptAnalysisEndpoints(this IEndpointRouteBuilder endpoints) { // ConfigEditor policy (Administrator or Designer) — matches the Script page gate and the /deployments gate. var group = endpoints.MapGroup("/api/script-analysis") .RequireAuthorization(AdminUiPolicies.ConfigEditor); - group.MapPost("/diagnostics", async (DiagnoseRequest r, ScriptAnalysisService s) => Results.Ok(await s.DiagnoseAsync(r))); - group.MapPost("/completions", async (CompletionsRequest r, ScriptAnalysisService s) => Results.Ok(await s.CompleteAsync(r))); - group.MapPost("/hover", async (HoverRequest r, ScriptAnalysisService s) => Results.Ok(await s.Hover(r))); - group.MapPost("/signature-help", (SignatureHelpRequest r, ScriptAnalysisService s) => Results.Ok(s.SignatureHelp(r))); + group.MapPost("/diagnostics", (DiagnoseRequest r, ScriptAnalysisService s, CancellationToken ct) => Run(() => s.DiagnoseAsync(r, ct), ct)); + group.MapPost("/completions", (CompletionsRequest r, ScriptAnalysisService s, CancellationToken ct) => Run(() => s.CompleteAsync(r, ct), ct)); + group.MapPost("/hover", (HoverRequest r, ScriptAnalysisService s, CancellationToken ct) => Run(() => s.Hover(r, ct), ct)); + group.MapPost("/signature-help", (SignatureHelpRequest r, ScriptAnalysisService s, CancellationToken ct) => Run(() => Task.FromResult(s.SignatureHelp(r, ct)), ct)); group.MapPost("/format", (FormatRequest r, ScriptAnalysisService s) => Results.Ok(s.Format(r))); group.MapPost("/inlay-hints", (InlayHintsRequest r, ScriptAnalysisService s) => Results.Ok(s.InlayHints(r))); return endpoints; } + + private static async Task Run(Func> analyse, CancellationToken ct) + { + try + { + return Results.Ok(await analyse()); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + return Results.StatusCode(ClientClosedRequest); + } + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs index 23d610b7..9a98267d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -18,10 +18,12 @@ namespace ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; /// context. /// /// -/// All capabilities are fully implemented: Diagnose, Complete, Hover, SignatureHelp, and Format -/// are wired and backed by the real Roslyn document/analysis pipeline. Only InlayHints is -/// intentionally a no-op (empty list) — the endpoint exists for future use but the feature is -/// not implemented. +/// All capabilities are fully implemented: Diagnose, Complete, Hover, SignatureHelp, and Format +/// are wired and backed by the real Roslyn document/analysis pipeline. +/// Cost model. Every analysis derives from one process-wide +/// via , so the sandbox reference set is bound once rather +/// than per keystroke, and every entry point takes the request's +/// (HttpContext.RequestAborted) so an abandoned request stops instead of running to completion. /// public sealed class ScriptAnalysisService { @@ -56,16 +58,44 @@ public sealed class ScriptAnalysisService + " var ctx = globals.ctx;\n#line 1\n"; } - // userSource is appended after Preamble, then the method/class are closed. - private static (SyntaxTree Tree, CSharpCompilation Compilation, SemanticModel Model, int PreambleLength) Analyze(string userSource) + private static SyntaxTree WrapAndParse(string userSource, CancellationToken ct) + => CSharpSyntaxTree.ParseText(Preamble + (userSource ?? "") + "\n }\n}\n", cancellationToken: ct); + + /// + /// The process-wide compilation every analysis derives from. Its sandbox reference set is bound ONCE + /// (forced here by resolving System.Object); keeps + /// that binding, so a request only re-binds the user's tree instead of the whole reference set. It is + /// immutable, so sharing it across circuits and requests is thread-safe. + /// + internal static readonly CSharpCompilation BaseCompilation = CreateBaseCompilation(); + + private static CSharpCompilation CreateBaseCompilation() { - var prefix = Preamble; - var full = prefix + (userSource ?? "") + "\n }\n}\n"; - var tree = CSharpSyntaxTree.ParseText(full); - var compilation = CSharpCompilation.Create("ScriptAnalysis", new[] { tree }, Sandbox.References, CompileOptions); - return (tree, compilation, compilation.GetSemanticModel(tree), prefix.Length); + var compilation = CSharpCompilation.Create( + "ScriptAnalysis", new[] { WrapAndParse("", CancellationToken.None) }, Sandbox.References, CompileOptions); + _ = compilation.GetSpecialType(SpecialType.System_Object); + return compilation; } + // This scope's current compilation (one per scoped service). Each analysis swaps its tree in with + // ReplaceSyntaxTree; a lost race between two concurrent calls on one scope is harmless because every + // compilation in the lineage is a complete, valid single-tree compilation. + private CSharpCompilation _compilation = BaseCompilation; + + // userSource is appended after Preamble, then the method/class are closed. + private (SyntaxTree Tree, CSharpCompilation Compilation, SemanticModel Model, int PreambleLength) Analyze( + string userSource, CancellationToken ct) + { + var tree = WrapAndParse(userSource, ct); + var current = _compilation; + var compilation = current.ReplaceSyntaxTree(current.SyntaxTrees.Single(), tree); + _compilation = compilation; + return (tree, compilation, compilation.GetSemanticModel(tree), Preamble.Length); + } + + /// Test seam: the compilation one analysis of runs against. + internal CSharpCompilation CompilationFor(string userSource) => Analyze(Normalize(userSource), CancellationToken.None).Compilation; + // editor (line,col) in USER-source coords -> physical offset in the wrapped document private static int OffsetInWrapped(string userSource, int line, int column, int preambleLength) => preambleLength + Math.Clamp(PositionToOffset(userSource ?? "", line, column), 0, (userSource ?? "").Length); @@ -87,14 +117,16 @@ public sealed class ScriptAnalysisService /// Compiles the script and returns compile-error diagnostics plus sandbox-policy violations /// (forbidden types, disallowed dynamic tag paths). /// The diagnose request carrying the script source. + /// The request's cancellation token; a cancelled request throws . /// The list of diagnostic markers, empty when the script is clean or analysis fails. - public DiagnoseResponse Diagnose(DiagnoseRequest req) + public DiagnoseResponse Diagnose(DiagnoseRequest req, CancellationToken ct = default) { if (string.IsNullOrEmpty(req.Code)) return new DiagnoseResponse(Array.Empty()); try { + ct.ThrowIfCancellationRequested(); var code = Normalize(req.Code); - var (tree, compilation, _, preambleLength) = Analyze(code); + var (tree, compilation, _, preambleLength) = Analyze(code, ct); var markers = new List(); // (1) Roslyn — surface ONLY compile errors, the same severity that actually blocks a @@ -107,7 +139,7 @@ public sealed class ScriptAnalysisService // makes GetMappedLineSpan report user-source coords (GetLineSpan would return the // physical wrapper line and be wrong); the preambleLength guard also drops phantom // wrapper diagnostics like CS0161 on Run() when the user hasn't typed `return` yet. - foreach (var d in compilation.GetDiagnostics()) + foreach (var d in compilation.GetDiagnostics(ct)) { if (d.Severity != DiagnosticSeverity.Error || !d.Location.IsInSource) continue; if (d.Location.SourceSpan.Start < preambleLength) continue; @@ -115,14 +147,24 @@ public sealed class ScriptAnalysisService markers.Add(ToMarker(span, Sev(d.Severity), d.GetMessage(), d.Id)); } // (2) ForbiddenTypeAnalyzer — spans are in the wrapped tree; map via #line. + ct.ThrowIfCancellationRequested(); foreach (var r in ForbiddenTypeAnalyzer.Analyze(compilation)) markers.Add(ToMarker(tree.GetMappedLineSpan(r.Span), 8, r.Message, "OTSCRIPT_FORBIDDEN")); + ct.ThrowIfCancellationRequested(); // (3) DependencyExtractor — spans are offsets in the normalized user source; map directly. foreach (var r in DependencyExtractor.Extract(code).Rejections) markers.Add(ToUserMarker(code, r.Span, r.Message, "OTSCRIPT_DYNPATH")); + // (4) ctx.SetVirtualTag(...) is inert at runtime — a WARNING (severity 4), never an error: publish + // accepts the script, so the editor must too; the marker just puts the hover's caveat in the Problems list. + foreach (var inv in SetVirtualTagCalls(tree.GetRoot(ct), preambleLength)) + markers.Add(ToMarker(tree.GetMappedLineSpan(inv.Span), 4, SetVirtualTagInertMessage, "OTSCRIPT_SETVTAG_INERT")); return new DiagnoseResponse(markers.Distinct().ToList()); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } catch (Exception ex) { _logger?.LogWarning(ex, "Script diagnostics failed; returning no markers."); @@ -144,10 +186,11 @@ public sealed class ScriptAnalysisService /// (DraftValidator.ValidateEquipReferenceResolution) rejects, so the editor accepts ⇔ publish /// accepts. Inert (base markers only) on the shared ScriptEdit page where EquipmentId is null. /// The diagnose request (its EquipmentId carries the owning-equipment context). + /// The request's cancellation token; a cancelled request throws . /// The base diagnostics plus any unresolved-reference markers. - public async Task DiagnoseAsync(DiagnoseRequest req) + public async Task DiagnoseAsync(DiagnoseRequest req, CancellationToken ct = default) { - var baseResp = Diagnose(req); + var baseResp = Diagnose(req, ct); if (_catalog is null || string.IsNullOrEmpty(req.EquipmentId) || string.IsNullOrEmpty(req.Code)) return baseResp; try @@ -156,7 +199,7 @@ public sealed class ScriptAnalysisService if (!code.Contains(EquipmentScriptPaths.EquipTokenPrefix, StringComparison.Ordinal)) return baseResp; - var names = await _catalog.GetEquipmentReferenceNamesAsync(req.EquipmentId, null, CancellationToken.None); + var names = await _catalog.GetEquipmentReferenceNamesAsync(req.EquipmentId, null, ct); var resolvable = new HashSet(names, StringComparer.Ordinal); var markers = new List(baseResp.Markers); @@ -175,6 +218,10 @@ public sealed class ScriptAnalysisService } return new DiagnoseResponse(markers.Distinct().ToList()); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } catch (Exception ex) { _logger?.LogWarning(ex, "Script equip-ref diagnostics failed; returning base markers."); @@ -182,6 +229,23 @@ public sealed class ScriptAnalysisService } } + private const string SetVirtualTagInertMessage = + "ctx.SetVirtualTag(...) has no effect at runtime: cross-tag writes are dropped in single-tag mode (the " + + "cascade engine is not wired into the host). The script compiles and publishes, but this write will not " + + "take effect."; + + // Every ctx.SetVirtualTag(...) invocation in the user source — the same `ctx`-anchored receiver guard as the + // tag-path completion (TryGetTagPathLiteral), so comments, string literals and other receivers are ignored. + private static IEnumerable SetVirtualTagCalls(SyntaxNode root, int preambleLength) + => root.DescendantNodes() + .OfType() + .Where(inv => inv.SpanStart >= preambleLength + && inv.Expression is MemberAccessExpressionSyntax + { + Expression: IdentifierNameSyntax { Identifier.ValueText: "ctx" }, + Name.Identifier.ValueText: nameof(ScriptContext.SetVirtualTag), + }); + private static int Sev(DiagnosticSeverity s) => s switch { DiagnosticSeverity.Error => 8, @@ -210,16 +274,18 @@ public sealed class ScriptAnalysisService } /// Computes completion items (tag-path, dot-member, or in-scope symbol completions) at the caret position. /// The completions request carrying the script source and caret position. + /// The request's cancellation token; a cancelled request throws . /// The resolved list of completion items, empty when none apply. - public async Task CompleteAsync(CompletionsRequest req) + public async Task CompleteAsync(CompletionsRequest req, CancellationToken ct = default) { if (string.IsNullOrEmpty(req.CodeText)) return new CompletionsResponse(Array.Empty()); try { + ct.ThrowIfCancellationRequested(); var code = Normalize(req.CodeText); - var (tree, _, model, preambleLength) = Analyze(code); + var (tree, _, model, preambleLength) = Analyze(code, ct); var position = OffsetInWrapped(code, req.Line, req.Column, preambleLength); - var root = await tree.GetRootAsync(); + var root = await tree.GetRootAsync(ct); // position is the offset just AFTER the caret; -1 finds the token the caret sits at/just-after (e.g. the dot in "ctx."). var token = root.FindToken(Math.Max(0, position - 1)); @@ -233,26 +299,31 @@ public sealed class ScriptAnalysisService if (string.IsNullOrEmpty(req.EquipmentId)) return new CompletionsResponse(Array.Empty()); var names = await _catalog.GetEquipmentReferenceNamesAsync( - req.EquipmentId, pathPrefix.Substring(equipPrefix.Length), CancellationToken.None); + req.EquipmentId, pathPrefix.Substring(equipPrefix.Length), ct); return new CompletionsResponse(names .Select(n => new CompletionItem(equipPrefix + n, equipPrefix + n, "tag path", "Field")) .ToList()); } - var paths = await _catalog.GetPathsAsync(pathPrefix, CancellationToken.None); + var paths = await _catalog.GetPathsAsync(pathPrefix, ct); return new CompletionsResponse( paths.Select(p => new CompletionItem(p, p, "tag path", "Field")).ToList()); } - var dot = TryGetDotMembers(token, model); + var dot = TryGetDotMembers(token, model, ct); if (dot != null) return new CompletionsResponse(dot); + ct.ThrowIfCancellationRequested(); var scoped = model.LookupSymbols(position) .Where(s => !s.IsImplicitlyDeclared && !string.IsNullOrEmpty(s.Name)) .GroupBy(s => s.Name).Select(g => g.First()) .Select(ToCompletionItem).Take(200).ToList(); return new CompletionsResponse(scoped); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + throw; + } catch (Exception ex) { _logger?.LogWarning(ex, "Script completion failed; returning none."); @@ -260,12 +331,12 @@ public sealed class ScriptAnalysisService } } - private static List? TryGetDotMembers(SyntaxToken token, SemanticModel model) + private static List? TryGetDotMembers(SyntaxToken token, SemanticModel model, CancellationToken ct) { var memberAccess = token.Parent as MemberAccessExpressionSyntax ?? token.GetPreviousToken().Parent as MemberAccessExpressionSyntax; if (memberAccess == null) return null; - var typeInfo = model.GetTypeInfo(memberAccess.Expression); + var typeInfo = model.GetTypeInfo(memberAccess.Expression, ct); var type = typeInfo.Type ?? typeInfo.ConvertedType; if (type == null) return null; // The type's own members, then the INSTANCE members it inherits (stopping before System.Object, whose @@ -322,16 +393,18 @@ public sealed class ScriptAnalysisService } /// Computes hover information (tag-path info or C# symbol info) for the position under the caret. /// The hover request carrying the script source and caret position. + /// The request's cancellation token; a cancelled request throws . /// The resolved hover markdown, or a response with a null value when nothing resolves. - public async Task Hover(HoverRequest req) + public async Task Hover(HoverRequest req, CancellationToken ct = default) { if (string.IsNullOrWhiteSpace(req.CodeText)) return new HoverResponse(null); try { + ct.ThrowIfCancellationRequested(); var code = Normalize(req.CodeText); - var (tree, _, model, preambleLength) = Analyze(code); + var (tree, _, model, preambleLength) = Analyze(code, ct); var position = OffsetInWrapped(code, req.Line, req.Column, preambleLength); - var token = tree.GetRoot().FindToken(Math.Max(0, position - 1)); + var token = tree.GetRoot(ct).FindToken(Math.Max(0, position - 1)); // Tag-path hover takes priority over C# symbol resolution: when the caret sits on a // ctx.GetTag("…")/ctx.SetVirtualTag("…") path literal, show the resolved tag's info @@ -350,7 +423,7 @@ public sealed class ScriptAnalysisService "(by effective name) to the backing raw tag's path when the VirtualTag is deployed."); } - var info = await _catalog.GetTagInfoAsync(tagPath, CancellationToken.None); + var info = await _catalog.GetTagInfoAsync(tagPath, ct); var tagMd = info is null ? $"**Tag path** `{Code(tagPath)}`\n\n⚠ Not a known configured tag path." : $"**Tag path** `{Code(info.Path)}`\n\n{info.Kind} · Type **{info.DataType}**" @@ -368,21 +441,22 @@ public sealed class ScriptAnalysisService // Resolve the token's node; if that yields nothing, climb to the enclosing // member-access (e.g. hovering the `GetTag` identifier — the IdentifierName itself // carries no symbol info, but its parent member-access binds to the method). - var symbol = ResolveSymbol(model, node); + var symbol = ResolveSymbol(model, node, ct); if (symbol is null) return new HoverResponse(null); var display = symbol.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat); var summary = TryGetXmlSummary(symbol); var md = "```csharp\n" + display + "\n```" + (summary is null ? "" : "\n\n" + summary); return new HoverResponse(md); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } catch (Exception ex) { _logger?.LogWarning(ex, "Script hover failed."); return new HoverResponse(null); } } - private static ISymbol? ResolveSymbol(SemanticModel model, SyntaxNode node) + private static ISymbol? ResolveSymbol(SemanticModel model, SyntaxNode node, CancellationToken ct) { foreach (var candidate in ResolutionCandidates(node)) { - var info = model.GetSymbolInfo(candidate); + var info = model.GetSymbolInfo(candidate, ct); var symbol = info.Symbol ?? info.CandidateSymbols.FirstOrDefault(); if (symbol is not null) return symbol; } @@ -420,24 +494,26 @@ public sealed class ScriptAnalysisService /// Computes signature help for the method invocation at the caret position. /// The signature-help request carrying the script source and caret position. + /// The request's cancellation token; a cancelled request throws . /// The resolved signature and active-parameter index, or an empty response when none applies. - public SignatureHelpResponse SignatureHelp(SignatureHelpRequest req) + public SignatureHelpResponse SignatureHelp(SignatureHelpRequest req, CancellationToken ct = default) { var empty = new SignatureHelpResponse(null, null, 0); if (string.IsNullOrWhiteSpace(req.CodeText)) return empty; try { + ct.ThrowIfCancellationRequested(); var code = Normalize(req.CodeText); - var (tree, _, model, preambleLength) = Analyze(code); + var (tree, _, model, preambleLength) = Analyze(code, ct); var position = OffsetInWrapped(code, req.Line, req.Column, preambleLength); - var token = tree.GetRoot().FindToken(Math.Max(0, position - 1)); + var token = tree.GetRoot(ct).FindToken(Math.Max(0, position - 1)); InvocationExpressionSyntax? inv = null; for (var n = token.Parent; n is not null; n = n.Parent) if (n is InvocationExpressionSyntax found) { inv = found; break; } if (inv is null) return empty; - var info = model.GetSymbolInfo(inv); + var info = model.GetSymbolInfo(inv, ct); var method = (info.Symbol ?? info.CandidateSymbols.FirstOrDefault()) as IMethodSymbol; if (method is null) return empty; @@ -453,6 +529,7 @@ public sealed class ScriptAnalysisService return new SignatureHelpResponse(label, ps, active); } + catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } catch (Exception ex) { _logger?.LogWarning(ex, "Script signature help failed."); return empty; } } /// Formats a script's source code using Roslyn's syntax-tree whitespace normalization. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogIndex.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogIndex.cs new file mode 100644 index 00000000..2f4016ed --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogIndex.cs @@ -0,0 +1,82 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +/// +/// The query-ready form of one snapshot: the raw entries, a sorted +/// de-duplicated path array for prefix completion, and an Ordinal path→info dictionary for hover. +/// +/// +/// Built ONCE per snapshot (at most every 5 s) instead of per keystroke. Completion used to build a +/// of every fleet path and sort it on each request, and hover scanned the entries +/// linearly; on a 50k-tag fleet that was tens of MB of garbage per typed character. The semantics are +/// unchanged: completion is case-insensitive StartsWith, sorted , +/// with case-only duplicates collapsed to the first-seen casing; hover is an exact Ordinal match (the runtime +/// dependency mux is Ordinal) returning the first entry for a path. +/// +public sealed class ScriptTagCatalogIndex +{ + private static readonly StringComparer SortComparer = StringComparer.OrdinalIgnoreCase; + + private readonly Dictionary _byPath; + private readonly string[] _sortedPaths; + + private ScriptTagCatalogIndex(IReadOnlyList entries, string[] sortedPaths, Dictionary byPath) + { + Entries = entries; + _sortedPaths = sortedPaths; + _byPath = byPath; + } + + /// The projection the index was built from. + public IReadOnlyList Entries { get; } + + /// Distinct (case-insensitively) paths, sorted . + public IReadOnlyList SortedPaths => _sortedPaths; + + /// Builds the index over a catalog projection. + /// The catalog entries, in projection order (first occurrence wins on duplicates). + /// The built index. + public static ScriptTagCatalogIndex Build(IReadOnlyList entries) + { + // A stable sort keeps projection order among case-only duplicates, so keeping the first of each run + // reproduces the first-added casing a HashSet(OrdinalIgnoreCase) kept. + var sorted = new List(entries.Count); + foreach (var path in entries.Select(e => e.Path).OrderBy(p => p, SortComparer)) + { + if (sorted.Count > 0 && SortComparer.Equals(sorted[^1], path)) continue; + sorted.Add(path); + } + + var byPath = new Dictionary(entries.Count, StringComparer.Ordinal); + foreach (var e in entries) byPath.TryAdd(e.Path, e); + + return new ScriptTagCatalogIndex(entries, sorted.ToArray(), byPath); + } + + /// Paths starting (case-insensitively) with , in sorted order. + /// The literal prefix; null/whitespace returns the first paths. + /// Upper bound on the returned count. + /// At most matching paths. + public IReadOnlyList FindByPrefix(string? prefix, int max) + { + var paths = _sortedPaths; + if (string.IsNullOrWhiteSpace(prefix)) return paths.Take(max).ToArray(); + + // Under an ordinal-style (char-by-char) comparer every string with prefix P sorts at or after P and the + // matches form one contiguous run, so a lower-bound binary search plus a forward scan is exact. + var start = Array.BinarySearch(paths, prefix, SortComparer); + if (start < 0) start = ~start; + + var result = new List(Math.Min(max, 32)); + for (var i = start; i < paths.Length && result.Count < max; i++) + { + if (!paths[i].StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) break; + result.Add(paths[i]); + } + return result; + } + + /// Exact (Ordinal) lookup of a path. + /// The path to look up. + /// The first entry for that path, or . + public ScriptTagInfo? TryGet(string path) => _byPath.GetValueOrDefault(path); +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogSnapshotCache.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogSnapshotCache.cs index c6597fae..1369c52e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogSnapshotCache.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptTagCatalogSnapshotCache.cs @@ -55,24 +55,37 @@ public sealed class ScriptTagCatalogSnapshotCache(TimeProvider timeProvider) public async Task> GetOrBuildAsync( Func>> build, CancellationToken ct) + => (await GetOrBuildIndexAsync(build, ct)).Entries; + + /// + /// As , but returns the snapshot's query-ready + /// (sorted paths + path dictionary), which is built once per rebuild + /// rather than once per completion keystroke. + /// + /// Builds a fresh projection; invoked at most once per TTL window. + /// Cancellation token. + /// The index over a fresh snapshot, or over one just built. + public async Task GetOrBuildIndexAsync( + Func>> build, + CancellationToken ct) { var current = _snapshot; - if (current is not null && !IsStale(current)) return current.Entries; + if (current is not null && !IsStale(current)) return current.Index; await _rebuildGate.WaitAsync(ct); try { // Double-check: while we queued, the caller ahead of us may have rebuilt already. current = _snapshot; - if (current is not null && !IsStale(current)) return current.Entries; + if (current is not null && !IsStale(current)) return current.Index; // A throw here leaves _snapshot untouched (the previous one, if any, keeps serving) and // propagates to this caller only; the finally still releases the gate. - var entries = await build(ct); + var index = ScriptTagCatalogIndex.Build(await build(ct)); // Stamped AFTER the build so a slow scan does not hand back an already-expired snapshot. - _snapshot = new Snapshot(entries, timeProvider.GetUtcNow()); - return entries; + _snapshot = new Snapshot(index, timeProvider.GetUtcNow()); + return index; } finally { @@ -82,5 +95,5 @@ public sealed class ScriptTagCatalogSnapshotCache(TimeProvider timeProvider) private bool IsStale(Snapshot snapshot) => timeProvider.GetUtcNow() - snapshot.TakenAt >= Ttl; - private sealed record Snapshot(IReadOnlyList Entries, DateTimeOffset TakenAt); + private sealed record Snapshot(ScriptTagCatalogIndex Index, DateTimeOffset TakenAt); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js index e90c63bc..5900b6a2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js @@ -23,6 +23,30 @@ const KIND_MAP = { Method: 0, Field: 3, Property: 9, Event: 10, Class: 5, Module: 8, Variable: 4, Text: 18 }; + // POSTs to a script-analysis endpoint, honouring Monaco's CancellationToken: when Monaco abandons a request + // (the caret moved, the user kept typing) the fetch is aborted, which cancels HttpContext.RequestAborted + // server-side so the Roslyn analysis stops instead of running to completion. Resolves to null when the + // request was cancelled or failed, so a provider never applies a superseded answer. + async function postAnalysis(path, body, token) { + if (token && token.isCancellationRequested) return null; + const ac = new AbortController(); + const sub = token && token.onCancellationRequested ? token.onCancellationRequested(function () { ac.abort(); }) : null; + try { + const resp = await fetch("/api/script-analysis/" + path, { + method: "POST", credentials: "same-origin", signal: ac.signal, + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body) + }); + if (!resp.ok) return null; + const data = await resp.json(); + return (token && token.isCancellationRequested) ? null : data; + } catch (e) { + return null; + } finally { + if (sub && sub.dispose) sub.dispose(); + } + } + // The owning-equipment id is stamped onto each editor's model at createEditor time so the globally // registered csharp providers (which only receive the model) can thread it into the {{equip}}/ // completion + diagnostics. Null on the shared ScriptEdit page (equip-ref features inert there). @@ -31,15 +55,12 @@ function registerCSharpProviders() { monaco.languages.registerCompletionItemProvider("csharp", { triggerCharacters: [".", "(", "\""], - provideCompletionItems: async function (model, position) { + provideCompletionItems: async function (model, position, context, token) { try { - const resp = await fetch("/api/script-analysis/completions", { - method: "POST", credentials: "same-origin", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ codeText: model.getValue(), line: position.lineNumber, column: position.column, equipmentId: equipmentIdOf(model) }) - }); - if (!resp.ok) return { suggestions: [] }; - const data = await resp.json(); + const data = await postAnalysis("completions", + { codeText: model.getValue(), line: position.lineNumber, column: position.column, equipmentId: equipmentIdOf(model) }, + token); + if (!data) return { suggestions: [] }; const word = model.getWordUntilPosition(position); const wordRange = { startLineNumber: position.lineNumber, endLineNumber: position.lineNumber, @@ -72,31 +93,21 @@ } }); monaco.languages.registerHoverProvider("csharp", { - provideHover: async function (model, position) { + provideHover: async function (model, position, token) { try { - const resp = await fetch("/api/script-analysis/hover", { - method: "POST", credentials: "same-origin", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ codeText: model.getValue(), line: position.lineNumber, column: position.column, equipmentId: equipmentIdOf(model) }) - }); - if (!resp.ok) return null; - const data = await resp.json(); - if (!data.markdown) return null; + const data = await postAnalysis("hover", + { codeText: model.getValue(), line: position.lineNumber, column: position.column, equipmentId: equipmentIdOf(model) }, + token); + if (!data || !data.markdown) return null; return { contents: [{ value: data.markdown }] }; } catch (e) { return null; } } }); monaco.languages.registerDocumentFormattingEditProvider("csharp", { - provideDocumentFormattingEdits: async function (model) { + provideDocumentFormattingEdits: async function (model, options, token) { try { - const resp = await fetch("/api/script-analysis/format", { - method: "POST", credentials: "same-origin", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ code: model.getValue() }) - }); - if (!resp.ok) return []; - const data = await resp.json(); - if (typeof data.code !== "string" || data.code === model.getValue()) return []; + const data = await postAnalysis("format", { code: model.getValue() }, token); + if (!data || typeof data.code !== "string" || data.code === model.getValue()) return []; return [{ range: model.getFullModelRange(), text: data.code }]; } catch (e) { return []; } } @@ -123,16 +134,12 @@ monaco.languages.registerSignatureHelpProvider("csharp", { signatureHelpTriggerCharacters: ["(", ","], signatureHelpRetriggerCharacters: [","], - provideSignatureHelp: async function (model, position) { + provideSignatureHelp: async function (model, position, token) { try { - const resp = await fetch("/api/script-analysis/signature-help", { - method: "POST", credentials: "same-origin", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ codeText: model.getValue(), line: position.lineNumber, column: position.column }) - }); - if (!resp.ok) return null; - const data = await resp.json(); - if (!data.label) return null; + const data = await postAnalysis("signature-help", + { codeText: model.getValue(), line: position.lineNumber, column: position.column }, + token); + if (!data || !data.label) return null; return { value: { signatures: [{ diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCancellationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCancellationTests.cs new file mode 100644 index 00000000..2253d8f6 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCancellationTests.cs @@ -0,0 +1,134 @@ +using System.Text; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// R04:P2 — an abandoned editor request (Monaco cancelled it, or the browser aborted the fetch) used to run +/// its full compile to completion. The request's token now reaches the service, the Roslyn calls and the +/// catalog, and the endpoints bind HttpContext.RequestAborted. +/// +public sealed class AnalysisCancellationTests +{ + internal sealed class CapturingCatalog : IScriptTagCatalog + { + public readonly List Seen = new(); + + public Task> GetPathsAsync(string? filter, CancellationToken ct) + { + Seen.Add(ct); + return Task.FromResult>(new[] { "Plant/Dev/Speed" }); + } + + public Task GetTagInfoAsync(string path, CancellationToken ct) + { + Seen.Add(ct); + return Task.FromResult(new ScriptTagInfo(path, "Tag", "Float", "DRV-1")); + } + + public Task> GetEquipmentReferenceNamesAsync(string equipmentId, string? filter, CancellationToken ct) + { + Seen.Add(ct); + return Task.FromResult>(new[] { "Speed" }); + } + } + + private static CancellationToken Cancelled() => new(canceled: true); + + [Fact] + public async Task Diagnose_with_a_cancelled_token_throws_instead_of_returning_empty_markers() + { + var svc = new ScriptAnalysisService(); + // Positive control: the same request with a live token produces the error marker. + (await svc.DiagnoseAsync(new DiagnoseRequest("return ctx.Nope();"), CancellationToken.None)) + .Markers.ShouldContain(m => m.Severity == 8); + + await Should.ThrowAsync( + svc.DiagnoseAsync(new DiagnoseRequest("return ctx.Nope();"), Cancelled())); + } + + [Fact] + public async Task Completions_hover_and_signature_help_honour_a_cancelled_token() + { + var svc = new ScriptAnalysisService(new CapturingCatalog()); + const string code = "return ctx.GetTag(\"\");"; + + // Positive controls. + (await svc.CompleteAsync(new CompletionsRequest(code, 1, 20), CancellationToken.None)).Items.ShouldNotBeEmpty(); + (await svc.Hover(new HoverRequest("return ctx.GetTag(\"Plant/Dev/Speed\");", 1, 22), CancellationToken.None)) + .Markdown.ShouldNotBeNull(); + svc.SignatureHelp(new SignatureHelpRequest("ctx.GetTag(", 1, 12), CancellationToken.None).Label.ShouldNotBeNull(); + + await Should.ThrowAsync(svc.CompleteAsync(new CompletionsRequest(code, 1, 20), Cancelled())); + await Should.ThrowAsync( + svc.Hover(new HoverRequest("return ctx.GetTag(\"Plant/Dev/Speed\");", 1, 22), Cancelled())); + Should.Throw(() => svc.SignatureHelp(new SignatureHelpRequest("ctx.GetTag(", 1, 12), Cancelled())); + } + + [Fact] + public async Task The_request_token_reaches_the_catalog() + { + var catalog = new CapturingCatalog(); + var svc = new ScriptAnalysisService(catalog); + using var cts = new CancellationTokenSource(); + + await svc.CompleteAsync(new CompletionsRequest("return ctx.GetTag(\"\");", 1, 20), cts.Token); + await svc.Hover(new HoverRequest("return ctx.GetTag(\"Plant/Dev/Speed\");", 1, 22), cts.Token); + await svc.DiagnoseAsync(new DiagnoseRequest("return ctx.GetTag(\"{{equip}}/Nope\").Value;", "EQ-1"), cts.Token); + + catalog.Seen.Count.ShouldBe(3); + catalog.Seen.ShouldAllBe(t => t == cts.Token); + } + + // ---------------------------------------------------------------- endpoints + + // A bare DefaultHttpContext reports "no body"; Kestrel sets this feature for a real POST. + private sealed class HasBody : Microsoft.AspNetCore.Http.Features.IHttpRequestBodyDetectionFeature + { + public bool CanHaveBody => true; + } + + internal static (WebApplication App, CapturingCatalog Catalog) BuildApp() + { + var builder = WebApplication.CreateBuilder(); + var catalog = new CapturingCatalog(); + builder.Services.AddSingleton(catalog); + builder.Services.AddScoped(sp => new ScriptAnalysisService(sp.GetRequiredService())); + builder.Services.AddAuthorization(); + var app = builder.Build(); + app.MapScriptAnalysisEndpoints(); + return (app, catalog); + } + + internal static IReadOnlyList Endpoints(WebApplication app) + => ((IEndpointRouteBuilder)app).DataSources.SelectMany(d => d.Endpoints).OfType().ToList(); + + [Fact] + public async Task The_endpoint_binds_RequestAborted_through_to_the_catalog() + { + var (app, catalog) = BuildApp(); + var endpoint = Endpoints(app).Single(e => e.RoutePattern.RawText!.EndsWith("/completions")); + + using var scope = app.Services.CreateScope(); + using var aborted = new CancellationTokenSource(); + var ctx = new DefaultHttpContext { RequestServices = scope.ServiceProvider, RequestAborted = aborted.Token }; + ctx.Request.Method = "POST"; + ctx.Request.ContentType = "application/json"; + var body = Encoding.UTF8.GetBytes("""{"codeText":"return ctx.GetTag(\"\");","line":1,"column":20}"""); + ctx.Request.Body = new MemoryStream(body); + ctx.Request.ContentLength = body.Length; + ctx.Response.Body = new MemoryStream(); + ctx.Features.Set(new HasBody()); + + await endpoint.RequestDelegate!(ctx); + + ctx.Response.StatusCode.ShouldBe(200); + catalog.Seen.ShouldHaveSingleItem().ShouldBe(aborted.Token); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCompilationReuseTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCompilationReuseTests.cs new file mode 100644 index 00000000..d6eaca6a --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/AnalysisCompilationReuseTests.cs @@ -0,0 +1,58 @@ +using Microsoft.CodeAnalysis; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// R04:P2 — the editor used to build a brand-new CSharpCompilation (and re-bind the whole sandbox +/// reference set) on every completion / hover / signature-help / diagnostics request. Each analysis now +/// derives from ONE process-wide base compilation via ReplaceSyntaxTree, so the referenced-assembly +/// binding is done once and shared. +/// +public sealed class AnalysisCompilationReuseTests +{ + [Fact] + public void Two_services_share_the_bound_reference_set() + { + // Two separate scopes (two HTTP requests) analysing different sources. + var c1 = new ScriptAnalysisService().CompilationFor("return 1;"); + var c2 = new ScriptAnalysisService().CompilationFor("return 2;"); + + // Positive control: the two compilations really carry different user sources. + c1.SyntaxTrees.Single().ToString().ShouldContain("return 1;"); + c2.SyntaxTrees.Single().ToString().ShouldContain("return 2;"); + c1.ShouldNotBeSameAs(c2); + + // Derived from one bound base: the corlib symbol is the SAME instance, not a re-bound copy. + c1.GetSpecialType(SpecialType.System_Object) + .ShouldBeSameAs(c2.GetSpecialType(SpecialType.System_Object)); + c1.GetSpecialType(SpecialType.System_Object) + .ShouldBeSameAs(ScriptAnalysisService.BaseCompilation.GetSpecialType(SpecialType.System_Object)); + } + + [Fact] + public void Repeated_analysis_in_one_scope_replaces_the_tree_rather_than_adding_one() + { + var svc = new ScriptAnalysisService(); + svc.CompilationFor("return 1;"); + var second = svc.CompilationFor("return 2;"); + + second.SyntaxTrees.Count().ShouldBe(1); + second.SyntaxTrees.Single().ToString().ShouldContain("return 2;"); + second.SyntaxTrees.Single().ToString().ShouldNotContain("return 1;"); + } + + [Fact] + public void Diagnostics_are_unchanged_by_reuse() + { + // A clean script stays clean and an error stays an error across interleaved analyses on one service + // — a stale tree leaking from the previous call would surface here. + var svc = new ScriptAnalysisService(); + svc.Diagnose(new DiagnoseRequest("return ctx.Nope();")).Markers.ShouldContain(m => m.Severity == 8); + svc.Diagnose(new DiagnoseRequest("""return (double)ctx.GetTag("A").Value * 2.0;""")).Markers.ShouldBeEmpty(); + svc.Diagnose(new DiagnoseRequest("var a = 1;\nreturn ctx.Nope();")).Markers + .ShouldContain(m => m.Severity == 8 && m.StartLineNumber == 2); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ScriptTagCatalogIndexTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ScriptTagCatalogIndexTests.cs new file mode 100644 index 00000000..84ec3070 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ScriptTagCatalogIndexTests.cs @@ -0,0 +1,79 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// R04:P3 — the catalog used to build a HashSet of every fleet path and sort it on every completion keystroke, +/// and hover did a linear scan. The snapshot now carries a sorted, de-duplicated string[] (prefix-searched +/// with a binary search) and an Ordinal path→info dictionary, built once per snapshot. +/// +public sealed class ScriptTagCatalogIndexTests +{ + private const int Max = 200; + + // The previous per-keystroke implementation, kept here as the oracle the index must match exactly. + private static IReadOnlyList Naive(IReadOnlyList entries, string? filter) + { + IEnumerable q = new HashSet(entries.Select(e => e.Path), StringComparer.OrdinalIgnoreCase); + if (!string.IsNullOrWhiteSpace(filter)) + q = q.Where(p => p.StartsWith(filter, StringComparison.OrdinalIgnoreCase)); + return q.OrderBy(p => p, StringComparer.OrdinalIgnoreCase).Take(Max).ToList(); + } + + private static ScriptTagInfo Tag(string path) => new(path, "Tag", "Float", "DRV-1"); + + [Fact] + public void Prefix_search_is_case_insensitive_sorted_and_bounded() + { + var index = ScriptTagCatalogIndex.Build(new[] + { + Tag("Plant/b/Speed"), Tag("plant/A/Temp"), Tag("Other/X"), Tag("Plant/a/Flow"), + }); + + index.FindByPrefix("PLANT/", Max).ShouldBe(new[] { "Plant/a/Flow", "plant/A/Temp", "Plant/b/Speed" }); + index.FindByPrefix("other", Max).ShouldBe(new[] { "Other/X" }); + index.FindByPrefix("zzz", Max).ShouldBeEmpty(); + index.FindByPrefix(null, 2).Count.ShouldBe(2); + index.FindByPrefix(" ", Max).Count.ShouldBe(4); // whitespace filter = unfiltered, as before + } + + [Fact] + public void Case_only_duplicates_collapse_to_the_first_seen_casing() + { + // HashSet(OrdinalIgnoreCase) kept the FIRST-added casing; the index must too. + var index = ScriptTagCatalogIndex.Build(new[] { Tag("A/speed"), Tag("A/Speed"), Tag("A/SPEED") }); + index.SortedPaths.ShouldBe(new[] { "A/speed" }); + } + + [Fact] + public void Exact_lookup_is_ordinal_and_keeps_the_first_entry() + { + var first = new ScriptTagInfo("A/B", "Tag", "Float", "DRV-1"); + var index = ScriptTagCatalogIndex.Build(new[] { first, new ScriptTagInfo("A/B", "Virtual tag", "Int32", null) }); + + index.TryGet("A/B").ShouldBeSameAs(first); + index.TryGet("a/b").ShouldBeNull(); // case-mismatched path does not resolve at runtime + index.TryGet("A/C").ShouldBeNull(); + } + + [Fact] + public void Matches_the_previous_implementation_on_a_generated_fleet() + { + // Deterministic seed — a mixed-case fleet with case-only duplicates and shared prefixes. + var rng = new Random(745); + string Seg() => new string(Enumerable.Range(0, rng.Next(1, 4)).Select(_ => "abAB_/1"[rng.Next(7)]).ToArray()); + var entries = Enumerable.Range(0, 3000).Select(_ => Tag(Seg() + "/" + Seg() + "/" + Seg())).ToList(); + var index = ScriptTagCatalogIndex.Build(entries); + + var filters = new List { null, "", "a", "A", "ab", "aB/", "b/", "_", "1", "/", "zz" }; + filters.AddRange(entries.Take(50).Select(e => e.Path[..Math.Min(e.Path.Length, rng.Next(1, 6))])); + + foreach (var f in filters) + index.FindByPrefix(f, Max).ShouldBe(Naive(entries, f), $"filter '{f}'"); + // Positive control: the generated fleet really exercises truncation and filtering. + Naive(entries, null).Count.ShouldBe(Max); + Naive(entries, "ab").Count.ShouldBeGreaterThan(0); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/SetVirtualTagInertDiagnosticTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/SetVirtualTagInertDiagnosticTests.cs new file mode 100644 index 00000000..69cd2b57 --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/SetVirtualTagInertDiagnosticTests.cs @@ -0,0 +1,58 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; +using ZB.MOM.WW.OtOpcUa.Core.Scripting; +using ZB.MOM.WW.OtOpcUa.Core.VirtualTags; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// R04:S1 — ctx.SetVirtualTag(...) compiles and publishes but is dropped at runtime (single-tag mode). +/// Only a hover used to say so; Diagnose now emits an OTSCRIPT_SETVTAG_INERT WARNING marker so the +/// Problems list shows it. A warning, never an error: publish accepts the script, so the editor must too. +/// +public sealed class SetVirtualTagInertDiagnosticTests +{ + private const string InertCode = "OTSCRIPT_SETVTAG_INERT"; + private static readonly ScriptAnalysisService Svc = new(); + private static IReadOnlyList Diag(string code) => Svc.Diagnose(new DiagnoseRequest(code)).Markers; + + [Fact] + public void SetVirtualTag_call_gets_a_warning_marker_on_its_line() + { + var markers = Diag("var a = 1;\nvar b = 2;\nctx.SetVirtualTag(\"Other\", a + b);\nreturn 0;"); + + var m = markers.ShouldHaveSingleItem(); + m.Code.ShouldBe(InertCode); + m.Severity.ShouldBe(4); // Monaco warning + m.StartLineNumber.ShouldBe(3); + m.StartColumn.ShouldBe(1); + m.Message.ShouldContain("SetVirtualTag"); + } + + [Fact] + public void Every_SetVirtualTag_call_is_marked() + => Diag("ctx.SetVirtualTag(\"A\", 1);\nctx.SetVirtualTag(\"B\", 2);\nreturn 0;") + .Count(m => m.Code == InertCode).ShouldBe(2); + + [Theory] + [InlineData("return ctx.GetTag(\"A\").Value;")] // a different ctx member + [InlineData("return 0; // ctx.SetVirtualTag(\"A\", 1)")] // a comment + [InlineData("var s = \"ctx.SetVirtualTag(\\\"A\\\", 1)\"; return s;")] // a string literal + public void Non_calls_are_not_marked(string code) + { + var markers = Diag(code); + markers.ShouldNotContain(m => m.Code == InertCode); + markers.ShouldNotContain(m => m.Severity == 8); // the probe scripts themselves are valid + } + + [Fact] + public void The_warning_does_not_block_and_publish_accepts_the_script() + { + // Editor accepts ⇔ publish accepts: no error-severity marker, and the real publish compiler accepts it. + const string source = "ctx.SetVirtualTag(\"Other\", 1);\nreturn 0;"; + Diag(source).ShouldNotContain(m => m.Severity == 8); + using var evaluator = ScriptEvaluator.Compile(source); + evaluator.ShouldNotBeNull(); + } +} From da2578833dad86f5fcb148c8d1cb0bd2000e9793 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:38:56 -0400 Subject: [PATCH 2/3] fix(adminui): Monaco text syncs on demand, diagnostics are sequenced, Problems list (D1) R04:P1 - the editor no longer ships its full text JS->.NET on every keystroke. It reports at blur, and ScriptEdit / the VirtualTag modal read the authoritative value with MonacoEditor.GetValueAsync at Save (validation runs after the pull). The Blazor hub's MaximumReceiveMessageSize is raised from 32 KB to 1 MB. The JS refuses a value that would not fit instead of sending an oversize frame, and Save surfaces that refusal rather than persisting stale source. R04:P9 - diagnostics carry a generation counter and an AbortController. Every edit aborts the in-flight request, and only the response for the current text is applied. R04:S2 - the always-empty inlay-hints provider, endpoint, service method and contracts are removed. R04:S3 - MarkersChanged is typed (DiagnosticMarker) and feeds a new ScriptProblemsList. Clicking a problem calls RevealLineAsync. Markers cross to .NET only when a list subscribes. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Components/Pages/ScriptEdit.razor | 28 ++++- .../Components/Shared/MonacoEditor.razor | 61 ++++++++-- .../Shared/ScriptProblemsList.razor | 57 +++++++++ .../Shared/Uns/VirtualTagModal.razor | 34 +++++- .../EndpointRouteBuilderExtensions.cs | 2 +- .../ScriptAnalysis/ScriptAnalysisContracts.cs | 4 - .../ScriptAnalysis/ScriptAnalysisEndpoints.cs | 1 - .../ScriptAnalysis/ScriptAnalysisService.cs | 5 - .../ScriptAnalysis/ScriptEditorLimits.cs | 19 +++ .../ScriptAnalysis/ScriptProblems.cs | 54 ++++++++ .../wwwroot/js/monaco-init.js | 115 ++++++++++++------ .../MonacoInteropSourceTests.cs | 80 ++++++++++++ .../ScriptAnalysis/MonacoSyncTests.cs | 56 +++++++++ .../ScriptAnalysis/ProblemsListTests.cs | 59 +++++++++ 14 files changed, 514 insertions(+), 61 deletions(-) create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs create mode 100644 src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoSyncTests.cs create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ProblemsListTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor index e0a1d8af..e6fa4123 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor @@ -11,6 +11,7 @@ @using System.Text @using ZB.MOM.WW.OtOpcUa.Configuration @using ZB.MOM.WW.OtOpcUa.Configuration.Entities +@using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis @inject IDbContextFactory DbFactory @inject NavigationManager Nav @@ -30,7 +31,9 @@ else if (!IsNew && _existing is null) } else { - + @* OnSubmit (not OnValidSubmit): the editor text is pulled from Monaco FIRST, then validated — the bound + SourceCode is only refreshed at blur, so validating before the pull could judge stale text. *@ +
@@ -57,7 +60,9 @@ else
Source
- + +
SHA-256 hash is computed automatically on save.
@@ -84,6 +89,13 @@ else private bool _loaded; private bool _busy; private string? _error; + private MonacoEditor? _editor; + private IReadOnlyList _markers = Array.Empty(); + + private void OnMarkersChanged(IReadOnlyList markers) => _markers = markers; + + private Task RevealProblemAsync(DiagnosticMarker m) + => _editor is null ? Task.CompletedTask : _editor.RevealLineAsync(m.StartLineNumber, m.StartColumn); protected override async Task OnInitializedAsync() { @@ -105,11 +117,21 @@ else _loaded = true; } - private async Task SubmitAsync() + private async Task SubmitAsync(EditContext editContext) { _busy = true; _error = null; try { + // The editor reports its text only at blur, so read the authoritative current value now. A read + // failure (e.g. the script is too large to send) aborts the save rather than persisting stale source. + if (_editor is not null) + { + var read = await _editor.GetValueAsync(); + if (!read.Ok) { _error = $"Not saved: {read.Error}"; return; } + _form.SourceCode = read.Value!; + } + if (!editContext.Validate()) return; + var sourceHash = HashSource(_form.SourceCode); await using var db = await DbFactory.CreateDbContextAsync(); if (IsNew) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor index d4c73da1..82aab4a3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor @@ -1,5 +1,6 @@ @namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared @using Microsoft.Extensions.Logging +@using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis @implements IAsyncDisposable @inject IJSRuntime JS @inject Microsoft.Extensions.Logging.ILogger Logger @@ -22,7 +23,13 @@ style="height: @Height; border: 1px solid var(--bs-border-color); border-radius: 0.25rem; overflow: hidden;"> @code { + /// + /// The editor text. Two-way bound, but the editor reports edits back only when it loses focus — never per + /// keystroke (each report is one circuit message carrying the whole script). A page that persists the text + /// must read the authoritative current value with at save time. + /// [Parameter] public string Value { get; set; } = ""; + /// Raised with the editor text when the editor loses focus after an edit (and from ). [Parameter] public EventCallback ValueChanged { get; set; } [Parameter] public string Language { get; set; } = "csharp"; [Parameter] public string Height { get; set; } = "320px"; @@ -37,11 +44,11 @@ [Parameter] public string? EquipmentId { get; set; } /// - /// Fires whenever Monaco's marker set updates (after the 500 ms diagnostic - /// debounce). The marker DTO is not modelled yet — typed as object[] until a - /// later task wires the Roslyn-backed diagnostics. + /// Fires whenever Monaco's diagnostic marker set updates (after the 500 ms debounce, and only for the response + /// to the CURRENT text — superseded responses are dropped in the JS). Feeds ScriptProblemsList; the + /// markers are sent from JS only when this callback has a subscriber. /// - [Parameter] public EventCallback MarkersChanged { get; set; } + [Parameter] public EventCallback> MarkersChanged { get; set; } private ElementReference _hostRef; private DotNetObjectReference? _dotNetRef; @@ -69,7 +76,9 @@ value = Value ?? "", language = Language, readOnly = ReadOnly, - equipmentId = EquipmentId + equipmentId = EquipmentId, + maxSyncBytes = ScriptEditorLimits.MaxEditorSyncBytes, + reportMarkers = MarkersChanged.HasDelegate }, _dotNetRef); _initialized = true; @@ -126,9 +135,47 @@ return ValueChanged.InvokeAsync(_lastSentValue); } + /// + /// Reads the editor's current text on demand (call it at Save), raises when it + /// differs from the last value reported, and returns it. Before the editor is live (prerender, failed init) + /// the bound is authoritative and is returned as-is. + /// + /// The current text, or an error (e.g. the script is too large to send) — never a stale value. + public async Task GetValueAsync() + { + if (!_initialized) return new EditorReadResult(Value ?? "", null); + string? current; + try + { + current = await JS.InvokeAsync("MonacoBlazor.getValue", _id); + } + catch (JSDisconnectedException) + { + return new EditorReadResult(null, "The connection to the editor was lost; reload the page and try again."); + } + catch (JSException ex) + { + Logger.LogWarning(ex, "Monaco editor {EditorId}: failed to read the editor text.", _id); + // JSException.Message carries the JS stack after the first line; the author needs only the reason. + return new EditorReadResult(null, ex.Message.Split('\n', 2)[0].Trim()); + } + if (current is null) return new EditorReadResult(Value ?? "", null); // editor already disposed + await OnValueChanged(current); + return new EditorReadResult(current, null); + } + + /// The outcome of : the text, or why it could not be read. + /// The editor text; null when is set. + /// Why the text could not be read; null on success. + public sealed record EditorReadResult(string? Value, string? Error) + { + /// True when holds the editor text. + public bool Ok => Error is null; + } + [JSInvokable] - public Task OnMarkersChanged(object[] markers) => - MarkersChanged.InvokeAsync(markers ?? Array.Empty()); + public Task OnMarkersChanged(DiagnosticMarker[]? markers) => + MarkersChanged.InvokeAsync(markers ?? Array.Empty()); /// Programmatic scroll-to-line (called by the problems panel). public async Task RevealLineAsync(int line, int column = 1) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor new file mode 100644 index 00000000..c868a46a --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor @@ -0,0 +1,57 @@ +@namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared +@using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis +@* The script editor's Problems list. Fed by MonacoEditor.MarkersChanged; clicking a row calls OnSelect, which the + host wires to MonacoEditor.RevealLineAsync so the caret jumps to the problem. Ordering and wording live in + ScriptProblems (unit-tested). *@ + +@{ + var ordered = ScriptProblems.Order(Markers); +} +
+
+ Problems + @ScriptProblems.Summary(ordered) +
+ @if (ordered.Count > 0) + { +
    + @foreach (var m in ordered.Take(MaxRows)) + { +
  • + @ScriptProblems.SeverityLabel(m.Severity) + @m.StartLineNumber:@m.StartColumn + @m.Message + @m.Code +
    • + } +
+ @if (ordered.Count > MaxRows) + { +
… and @(ordered.Count - MaxRows) more
+ } + } +
+ +@code { + private const int MaxRows = 200; + + /// The markers the editor last reported (null before the first diagnostics pass). + [Parameter] public IReadOnlyList? Markers { get; set; } + + /// Raised when the author picks a problem; wire it to MonacoEditor.RevealLineAsync. + [Parameter] public EventCallback OnSelect { get; set; } + + private Task OnKeyAsync(KeyboardEventArgs e, DiagnosticMarker m) + => e.Key is "Enter" or " " ? OnSelect.InvokeAsync(m) : Task.CompletedTask; + + private static string BadgeClass(int severity) => severity switch + { + ScriptProblems.Error => "text-bg-danger", + ScriptProblems.Warning => "text-bg-warning", + _ => "text-bg-secondary", + }; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor index b28e4642..cbd7c0d4 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor @@ -7,6 +7,7 @@ @using System.ComponentModel.DataAnnotations @using Microsoft.AspNetCore.Components.Forms @using ZB.MOM.WW.OtOpcUa.AdminUI.Uns +@using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis @inject IUnsTreeService Svc @if (Visible) @@ -99,7 +100,9 @@ Editing shared script "@_scriptName" — used by @_scriptUsageCount virtual tag(s). Changes affect all of them. - + + @if (!string.IsNullOrWhiteSpace(_scriptError)) {
@_scriptError
@@ -217,6 +220,24 @@ private bool _scriptSaving; private bool _scriptSaved; private string? _scriptError; + private MonacoEditor? _scriptEditor; + private IReadOnlyList _scriptMarkers = Array.Empty(); + + private void OnScriptMarkersChanged(IReadOnlyList markers) => _scriptMarkers = markers; + + private Task RevealScriptProblemAsync(DiagnosticMarker m) + => _scriptEditor is null ? Task.CompletedTask : _scriptEditor.RevealLineAsync(m.StartLineNumber, m.StartColumn); + + // The inline editor reports its text only at blur; pull the authoritative current value before acting on it. + // Returns false (with _scriptError set) when the text could not be read, so nothing stale is saved. + private async Task PullScriptSourceAsync() + { + if (_scriptEditor is null) return true; + var read = await _scriptEditor.GetValueAsync(); + if (!read.Ok) { _scriptError = $"Not saved: {read.Error}"; return false; } + _scriptSource = read.Value!; + return true; + } // "New script" action state (shown only when no script is bound yet). private bool _scriptCreating; @@ -358,8 +379,16 @@ /// Toggles the inline script-source panel. Clears the stale "Script saved." banner when /// (re)expanding so a collapse/expand cycle doesn't re-show a confirmation from an earlier save. /// - private void ToggleScriptPanel() + private async Task ToggleScriptPanel() { + if (_scriptExpanded) + { + // Collapsing disposes the editor: keep its unsaved text in _scriptSource so re-opening shows it. + // A failed read keeps the panel open (with the error) rather than discarding the edits. + if (!await PullScriptSourceAsync()) return; + _scriptEditor = null; + _scriptMarkers = Array.Empty(); + } _scriptExpanded = !_scriptExpanded; if (_scriptExpanded) { @@ -384,6 +413,7 @@ _scriptError = null; try { + if (!await PullScriptSourceAsync()) return; var result = await Svc.UpdateScriptSourceAsync(_form.ScriptId, _scriptSource, _scriptRowVersion); if (result.Ok) { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs index cfb8063e..1240a033 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs @@ -53,7 +53,7 @@ public static class EndpointRouteBuilderExtensions /// The same service collection, for chaining. public static IServiceCollection AddAdminUI(this IServiceCollection services) { - services.AddRazorComponents().AddInteractiveServerComponents(); + services.AddRazorComponents().AddInteractiveServerComponents().AddHubOptions(o => o.MaximumReceiveMessageSize = ScriptAnalysis.ScriptEditorLimits.HubMaximumReceiveMessageSize); services.AddOtOpcUaDriverStatusServices(); // Secrets-management UI (ZB.MOM.WW.Secrets.Ui RCL, mounted at /admin/secrets). Register its diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisContracts.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisContracts.cs index 2c71031c..1227957a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisContracts.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisContracts.cs @@ -22,7 +22,3 @@ public record SignatureHelpParameter(string Label, string? Documentation); public record FormatRequest(string Code); public record FormatResponse(string Code); - -public record InlayHintsRequest(string Code); -public record InlayHintsResponse(IReadOnlyList Hints); -public record InlayHint(int Line, int Column, string Label); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs index ad598c1f..50739298 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisEndpoints.cs @@ -27,7 +27,6 @@ public static class ScriptAnalysisEndpoints group.MapPost("/hover", (HoverRequest r, ScriptAnalysisService s, CancellationToken ct) => Run(() => s.Hover(r, ct), ct)); group.MapPost("/signature-help", (SignatureHelpRequest r, ScriptAnalysisService s, CancellationToken ct) => Run(() => Task.FromResult(s.SignatureHelp(r, ct)), ct)); group.MapPost("/format", (FormatRequest r, ScriptAnalysisService s) => Results.Ok(s.Format(r))); - group.MapPost("/inlay-hints", (InlayHintsRequest r, ScriptAnalysisService s) => Results.Ok(s.InlayHints(r))); return endpoints; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs index 9a98267d..6945c32f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -551,9 +551,4 @@ public sealed class ScriptAnalysisService return new FormatResponse(req.Code); } } - /// Returns editor inlay hints for the script. Currently a no-op (always empty) — the endpoint - /// exists for future use but the feature is not implemented. - /// The inlay-hints request (unused). - /// An always-empty inlay-hints response. - public InlayHintsResponse InlayHints(InlayHintsRequest req) => new(Array.Empty()); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs new file mode 100644 index 00000000..e2b89893 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs @@ -0,0 +1,19 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +/// Size limits for moving Monaco editor text across the Blazor Server circuit. +/// +/// The editor text crosses JS → .NET only on demand (at blur and when a page saves), never per keystroke. Each +/// crossing is ONE SignalR message, bounded by the Blazor hub's MaximumReceiveMessageSize. The framework +/// default is 32 KB, which silently lost any larger script (the oversize invoke failed and Save persisted the +/// stale source), so AddAdminUI raises it to . The editor JS +/// refuses to send a value whose serialized form exceeds — an oversize frame +/// would tear the circuit down — and the save surfaces that refusal to the author instead. +/// +public static class ScriptEditorLimits +{ + /// The Blazor hub's receive limit, in bytes (1 MB). + public const long HubMaximumReceiveMessageSize = 1024 * 1024; + + /// The largest serialized editor value the JS will send, leaving 64 KB of frame headroom. + public const int MaxEditorSyncBytes = (int)HubMaximumReceiveMessageSize - 64 * 1024; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs new file mode 100644 index 00000000..6bd3ee05 --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs @@ -0,0 +1,54 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +/// +/// Presentation rules for the script editor's Problems list — the markers Monaco reports through +/// MonacoEditor.MarkersChanged (the same shape the diagnostics endpoint +/// returns), rendered by ScriptProblemsList. +/// +public static class ScriptProblems +{ + /// Monaco's error severity. + public const int Error = 8; + + /// Monaco's warning severity. + public const int Warning = 4; + + /// Errors first, then warnings, then the rest; within a severity, by position. + /// The reported markers (null treated as none). + /// The markers in display order. + public static IReadOnlyList Order(IEnumerable? markers) + => (markers ?? Array.Empty()) + .OrderByDescending(m => m.Severity) + .ThenBy(m => m.StartLineNumber) + .ThenBy(m => m.StartColumn) + .ToList(); + + /// The label for a Monaco severity value. + /// 8 = error, 4 = warning, 2 = info, otherwise hint. + /// The display label. + public static string SeverityLabel(int severity) => severity switch + { + Error => "Error", + Warning => "Warning", + 2 => "Info", + _ => "Hint", + }; + + /// A one-line count, e.g. "2 errors, 1 warning". + /// The reported markers. + /// The summary, or "No problems". + public static string Summary(IReadOnlyCollection markers) + { + if (markers.Count == 0) return "No problems"; + var errors = markers.Count(m => m.Severity == Error); + var warnings = markers.Count(m => m.Severity == Warning); + var other = markers.Count - errors - warnings; + var parts = new List(3); + if (errors > 0) parts.Add(Plural(errors, "error")); + if (warnings > 0) parts.Add(Plural(warnings, "warning")); + if (other > 0) parts.Add($"{other} other"); + return string.Join(", ", parts); + } + + private static string Plural(int n, string noun) => n == 1 ? $"1 {noun}" : $"{n} {noun}s"; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js index 5900b6a2..8a47002c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js @@ -112,25 +112,6 @@ } catch (e) { return []; } } }); - monaco.languages.registerInlayHintsProvider("csharp", { - provideInlayHints: async function (model) { - try { - const resp = await fetch("/api/script-analysis/inlay-hints", { - method: "POST", credentials: "same-origin", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ code: model.getValue() }) - }); - if (!resp.ok) return { hints: [], dispose: function () {} }; - const data = await resp.json(); - return { - hints: (data.hints || []).map(function (h) { - return { position: { lineNumber: h.line, column: h.column }, label: h.label, kind: 2, paddingRight: true }; - }), - dispose: function () {} - }; - } catch (e) { return { hints: [], dispose: function () {} }; } - } - }); monaco.languages.registerSignatureHelpProvider("csharp", { signatureHelpTriggerCharacters: ["(", ","], signatureHelpRetriggerCharacters: [","], @@ -156,17 +137,41 @@ }); } - async function fetchDiagnostics(model) { + // Resolves to the marker array, or null when the request was aborted (superseded by a newer edit) — the + // caller must then leave the markers alone. A server/network failure still resolves to [] (clears markers). + async function fetchDiagnostics(model, signal) { try { const resp = await fetch("/api/script-analysis/diagnostics", { - method: "POST", credentials: "same-origin", + method: "POST", credentials: "same-origin", signal: signal, headers: { "Content-Type": "application/json" }, body: JSON.stringify({ code: model.getValue(), equipmentId: equipmentIdOf(model) }) }); if (!resp.ok) return []; const data = await resp.json(); return data.markers || []; - } catch (e) { return []; } + } catch (e) { return (signal && signal.aborted) ? null : []; } + } + + // The editor text crosses the Blazor circuit (JS -> .NET) only on demand — at blur and when the page saves — + // never per keystroke. Each crossing is one SignalR message, bounded by the hub's MaximumReceiveMessageSize; + // an oversize message would tear the circuit down, so a value whose serialized form would not fit is refused + // here instead (getValue throws a descriptive error that .NET surfaces to the author). + // Blazor carries an interop value as a JSON string inside the hub frame; measuring it escaped twice is a + // conservative upper bound whichever hub protocol (BlazorPack or JSON) the circuit negotiated. + function serializedBytes(v) { return new TextEncoder().encode(JSON.stringify(JSON.stringify(v))).length; } + function tooLargeMessage(e, v) { + if (!e.maxSyncBytes) return null; + const bytes = serializedBytes(v); + return bytes > e.maxSyncBytes + ? "The script is too large for the editor to send to the server (" + bytes + " bytes; the limit is " + e.maxSyncBytes + " bytes)." + : null; + } + function syncValue(e) { + if (!e.dirty) return; + const v = e.editor.getValue(); + if (tooLargeMessage(e, v)) return; // stays dirty; the explicit save read reports the error + e.dirty = false; + e.dotNetRef.invokeMethodAsync("OnValueChanged", v).catch(function () { e.dirty = true; }); } // A single body-level node for Monaco's overflow widgets (suggest / hover / parameter hints). @@ -207,22 +212,38 @@ // Stamp the owning-equipment id on the model so the global csharp providers can thread it into the // {{equip}}/ completion + diagnostics (null on the shared ScriptEdit page). try { const m0 = editor.getModel(); if (m0) m0.__otEquipmentId = options.equipmentId || null; } catch (x) {} - let diagTimer = null; - const scheduleDiagnostics = function () { - if (diagTimer) clearTimeout(diagTimer); - diagTimer = setTimeout(async function () { - const model = editor.getModel(); if (!model) return; - const markers = await fetchDiagnostics(model); - monaco.editor.setModelMarkers(model, "otopcua", markers); - dotNetRef.invokeMethodAsync("OnMarkersChanged", markers).catch(function () {}); - }, 500); + const e = { + editor: editor, dotNetRef: dotNetRef, dirty: false, + maxSyncBytes: options.maxSyncBytes || 0, reportMarkers: !!options.reportMarkers, + diagTimer: null, diagGeneration: 0, diagAbort: null + }; + // Diagnostics: the 500 ms debounce delays the START of a request; the generation counter + AbortController + // make sure only the response for the CURRENT text is applied. Every edit bumps the generation and aborts + // the in-flight request, so a slow older response can never overwrite newer markers. + const runDiagnostics = async function () { + const model = editor.getModel(); if (!model) return; + const generation = ++e.diagGeneration; + const ac = new AbortController(); + e.diagAbort = ac; + const markers = await fetchDiagnostics(model, ac.signal); + if (markers === null || generation !== e.diagGeneration || editors[id] !== e) return; // superseded + e.diagAbort = null; + monaco.editor.setModelMarkers(model, "otopcua", markers); + if (e.reportMarkers) dotNetRef.invokeMethodAsync("OnMarkersChanged", markers).catch(function () {}); + }; + e.scheduleDiagnostics = function () { + e.diagGeneration++; + if (e.diagAbort) { e.diagAbort.abort(); e.diagAbort = null; } + if (e.diagTimer) clearTimeout(e.diagTimer); + e.diagTimer = setTimeout(runDiagnostics, 500); }; editor.onDidChangeModelContent(function () { - dotNetRef.invokeMethodAsync("OnValueChanged", editor.getValue()).catch(function () {}); - if (options.language === "csharp") scheduleDiagnostics(); + e.dirty = true; + if (options.language === "csharp") e.scheduleDiagnostics(); }); - editors[id] = { editor: editor, dotNetRef: dotNetRef }; - if (options.language === "csharp") scheduleDiagnostics(); + editor.onDidBlurEditorText(function () { syncValue(e); }); + editors[id] = e; + if (options.language === "csharp") e.scheduleDiagnostics(); } function setEditorOption(id, name, value) { const e = editors[id]; if (!e) return; @@ -235,10 +256,28 @@ const e = editors[id]; if (!e) return; e.editor.revealLineInCenter(line); e.editor.setPosition({ lineNumber: line, column: col || 1 }); e.editor.focus(); } - function setValue(id, v) { const e = editors[id]; if (e && e.editor.getValue() !== v) e.editor.setValue(v || ""); } - function getValue(id) { const e = editors[id]; return e ? e.editor.getValue() : null; } + function setValue(id, v) { + const e = editors[id]; if (!e) return; + if (e.editor.getValue() !== v) e.editor.setValue(v || ""); + e.dirty = false; // the text now equals what .NET holds + } + // Reads the current text on demand (Save). Throws when it would not fit in one circuit message. + function getValue(id) { + const e = editors[id]; if (!e) return null; + const v = e.editor.getValue(); + const err = tooLargeMessage(e, v); + if (err) throw new Error(err); + e.dirty = false; + return v; + } function setMarkers(id, m) { const e = editors[id]; const model = e && e.editor.getModel(); if (model) monaco.editor.setModelMarkers(model, "otopcua", m || []); } - function dispose(id) { const e = editors[id]; if (!e) return; try { e.editor.dispose(); } catch (x) {} delete editors[id]; } + function dispose(id) { + const e = editors[id]; if (!e) return; + if (e.diagTimer) clearTimeout(e.diagTimer); + if (e.diagAbort) e.diagAbort.abort(); + try { e.editor.dispose(); } catch (x) {} + delete editors[id]; + } window.MonacoBlazor = { createEditor, setValue, getValue, setMarkers, setEditorOption, format, revealLine, dispose }; })(); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs new file mode 100644 index 00000000..0e7356cc --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs @@ -0,0 +1,80 @@ +using System.Text.RegularExpressions; +using Shouldly; +using Xunit; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// Source pins for the Monaco interop (AdminUI has no bUnit and no JS test runner). These do not prove the +/// browser behaviour — the docker-dev live check does — they stop the specific regressions R04:P1/P9/S2/S3 fixed +/// from quietly coming back: per-keystroke text sync, unsequenced diagnostics, the inlay-hints POST, and an +/// unconsumed marker round-trip. +/// +[Trait("Category", "Unit")] +public sealed class MonacoInteropSourceTests +{ + private static string Read(params string[] relative) + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir is not null && !File.Exists(Path.Combine(dir.FullName, "ZB.MOM.WW.OtOpcUa.slnx"))) + dir = dir.Parent; + dir.ShouldNotBeNull("could not locate the repository root"); + return File.ReadAllText(Path.Combine(new[] { dir!.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.AdminUI" }.Concat(relative).ToArray())); + } + + private static string Js() => Read("wwwroot", "js", "monaco-init.js"); + + [Fact] + public void The_content_change_handler_does_not_ship_the_text_to_dotnet() + { + var js = Js(); + var handler = Regex.Match(js, @"onDidChangeModelContent\(function \(\) \{(?.*?)\n \}\);", RegexOptions.Singleline); + handler.Success.ShouldBeTrue("the content-change handler moved; update this pin"); + handler.Groups["body"].Value.ShouldNotContain("OnValueChanged"); + handler.Groups["body"].Value.ShouldNotContain("getValue"); + // Positive control: the text IS still reported — at blur. + js.ShouldContain("onDidBlurEditorText"); + js.ShouldContain("\"OnValueChanged\""); + } + + [Fact] + public void Diagnostics_are_generation_guarded_and_abortable() + { + var js = Js(); + js.ShouldContain("diagGeneration"); + js.ShouldContain("generation !== e.diagGeneration"); + js.ShouldContain("new AbortController()"); + js.ShouldContain("signal: signal"); + } + + [Fact] + public void Providers_honour_the_monaco_cancellation_token() + { + var js = Js(); + js.ShouldContain("token.onCancellationRequested"); + foreach (var path in new[] { "\"completions\"", "\"hover\"", "\"format\"", "\"signature-help\"" }) + js.ShouldContain("postAnalysis(" + path); + } + + [Fact] + public void The_inlay_hints_provider_is_not_registered() => Js().ShouldNotContain("registerInlayHintsProvider"); + + [Fact] + public void Markers_cross_to_dotnet_only_for_a_subscribed_problems_list() + { + Js().ShouldContain("if (e.reportMarkers) dotNetRef.invokeMethodAsync(\"OnMarkersChanged\""); + Read("Components", "Shared", "MonacoEditor.razor").ShouldContain("reportMarkers = MarkersChanged.HasDelegate"); + } + + [Theory] + [InlineData("Components", "Pages", "ScriptEdit.razor")] + [InlineData("Components", "Shared", "Uns", "VirtualTagModal.razor")] + public void Script_hosts_read_the_editor_on_save_and_render_the_problems_list(params string[] path) + { + var src = Read(path); + src.ShouldContain("GetValueAsync()"); + src.ShouldContain(").MakeGenericType(typeof(HubOptions<>).MakeGenericType(hubType)); + var options = sp.GetRequiredService(optionsType); + return (HubOptions)optionsType.GetProperty("Value")!.GetValue(options)!; + } + + [Fact] + public void The_blazor_hub_accepts_a_1_MB_message() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddAdminUI(); + using var sp = services.BuildServiceProvider(); + + ComponentHubOptions(sp).MaximumReceiveMessageSize.ShouldBe(1024 * 1024); + ScriptEditorLimits.HubMaximumReceiveMessageSize.ShouldBe(1024 * 1024); + } + + [Fact] + public void Without_AddAdminUI_the_default_limit_is_the_32_KB_that_truncated_large_scripts() + { + // Positive control for the test above: the 1 MB comes from AddAdminUI, not from a framework default. + var services = new ServiceCollection(); + services.AddLogging(); + services.AddRazorComponents().AddInteractiveServerComponents(); + using var sp = services.BuildServiceProvider(); + + (ComponentHubOptions(sp).MaximumReceiveMessageSize ?? 32 * 1024).ShouldBe(32 * 1024); + } + + [Fact] + public void The_editor_sync_budget_leaves_headroom_inside_the_hub_limit_and_fits_a_100_KB_script() + { + ScriptEditorLimits.MaxEditorSyncBytes.ShouldBeLessThan((int)ScriptEditorLimits.HubMaximumReceiveMessageSize); + ScriptEditorLimits.MaxEditorSyncBytes.ShouldBeGreaterThan(4 * 100 * 1024); // 100 KB even after escaping twice + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ProblemsListTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ProblemsListTests.cs new file mode 100644 index 00000000..84c4deaf --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ProblemsListTests.cs @@ -0,0 +1,59 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// R04:S3 / S2 — the marker round-trip now feeds a real Problems list (MarkersChanged → list → RevealLineAsync), +/// and the dead inlay-hints endpoint is gone. The Razor wiring is live-verified; the ordering and wording it +/// renders are pinned here. +/// +public sealed class ProblemsListTests +{ + private static DiagnosticMarker M(int sev, int line, int col, string code = "X") => new(sev, line, col, line, col + 1, "msg", code); + + [Fact] + public void Problems_are_ordered_errors_first_then_by_position() + { + var ordered = ScriptProblems.Order(new[] + { + M(4, 1, 1, "W1"), M(8, 9, 2, "E3"), M(8, 2, 5, "E2"), M(8, 2, 1, "E1"), M(2, 1, 1, "I1"), + }); + + ordered.Select(m => m.Code).ShouldBe(new[] { "E1", "E2", "E3", "W1", "I1" }); + } + + [Fact] + public void Null_or_empty_markers_order_to_an_empty_list() => ScriptProblems.Order(null).ShouldBeEmpty(); + + [Theory] + [InlineData(8, "Error")] + [InlineData(4, "Warning")] + [InlineData(2, "Info")] + [InlineData(1, "Hint")] + public void Severity_labels_follow_monaco_values(int severity, string label) + => ScriptProblems.SeverityLabel(severity).ShouldBe(label); + + [Fact] + public void Summary_counts_errors_and_warnings() + { + ScriptProblems.Summary(Array.Empty()).ShouldBe("No problems"); + ScriptProblems.Summary(new[] { M(8, 1, 1) }).ShouldBe("1 error"); + ScriptProblems.Summary(new[] { M(8, 1, 1), M(8, 2, 1), M(4, 3, 1) }).ShouldBe("2 errors, 1 warning"); + ScriptProblems.Summary(new[] { M(4, 1, 1), M(4, 2, 1), M(2, 3, 1) }).ShouldBe("2 warnings, 1 other"); + } + + [Fact] + public void The_inlay_hints_endpoint_is_gone() + { + // The always-empty inlay-hints endpoint (and the JS provider that POSTed the full source to it on every + // edit) were removed. Positive control: the other five routes are still mapped. + var (app, _) = AnalysisCancellationTests.BuildApp(); + var routes = AnalysisCancellationTests.Endpoints(app).Select(e => e.RoutePattern.RawText!).ToList(); + + routes.ShouldNotContain(r => r.Contains("inlay", StringComparison.OrdinalIgnoreCase)); + foreach (var r in new[] { "/diagnostics", "/completions", "/hover", "/signature-help", "/format" }) + routes.ShouldContain(x => x.EndsWith(r)); + } +} From a9e8f5abfee0721ba5e0cae20abe00c953a68477 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 17:51:35 -0400 Subject: [PATCH 3/3] fix(adminui): bound the diagnostics marker payload on both the endpoint and the circuit (D-a review r1) I-1 - a non-C# paste produced thousands of Roslyn markers. The Problems-list report sent them all as one JS->.NET message, which could exceed the 1 MB hub limit and kill the circuit. The JS now sends at most MaxReportedMarkers (200) markers, errors first, each message cut to 300 chars, plus the total. ScriptProblemReport.FromEditor clamps again on arrival, and the Problems list shows "showing N of Total" / "+N more". The diagnostics endpoint also bounds its response: Distinct, errors first, MaxDiagnosticMarkers (1000), messages cut to 1000 chars. An error is always kept, so editor-accepts == publish-accepts. M-1 - drop the per-instance compilation lineage; analyses derive from BaseCompilation. M-2 - the VirtualTag modal clears its problems (and editor ref) on close and on a script reload. M-5 - an over-limit text is announced at blur (OnSyncRefused), not only at Save. Refs #745 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Components/Pages/ScriptEdit.razor | 6 +- .../Components/Shared/MonacoEditor.razor | 33 ++++++- .../Shared/ScriptProblemsList.razor | 17 ++-- .../Shared/Uns/VirtualTagModal.razor | 14 ++- .../ScriptAnalysis/ScriptAnalysisService.cs | 31 +++--- .../ScriptAnalysis/ScriptEditorLimits.cs | 22 +++++ .../ScriptAnalysis/ScriptProblems.cs | 49 +++++++++ .../wwwroot/js/monaco-init.js | 42 +++++++- .../ScriptAnalysis/MarkerPayloadBoundTests.cs | 99 +++++++++++++++++++ .../MonacoInteropSourceTests.cs | 26 ++++- 10 files changed, 305 insertions(+), 34 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MarkerPayloadBoundTests.cs diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor index e6fa4123..20190df1 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/ScriptEdit.razor @@ -62,7 +62,7 @@ else
- +
SHA-256 hash is computed automatically on save.
@@ -90,9 +90,9 @@ else private bool _busy; private string? _error; private MonacoEditor? _editor; - private IReadOnlyList _markers = Array.Empty(); + private ScriptProblemReport _markers = ScriptProblemReport.Empty; - private void OnMarkersChanged(IReadOnlyList markers) => _markers = markers; + private void OnMarkersChanged(ScriptProblemReport report) => _markers = report; private Task RevealProblemAsync(DiagnosticMarker m) => _editor is null ? Task.CompletedTask : _editor.RevealLineAsync(m.StartLineNumber, m.StartColumn); diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor index 82aab4a3..9c763b57 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/MonacoEditor.razor @@ -21,6 +21,14 @@
+@if (_tooLargeBytes > 0) +{ + @* Non-blocking: the author keeps editing, but learns now (not at Save) that this text cannot be sent. *@ +
+ This script is too large to send to the server (@_tooLargeBytes bytes; the limit is + @ScriptEditorLimits.MaxEditorSyncBytes bytes). It cannot be saved until it is shortened. +
+} @code { /// @@ -46,15 +54,18 @@ /// /// Fires whenever Monaco's diagnostic marker set updates (after the 500 ms debounce, and only for the response /// to the CURRENT text — superseded responses are dropped in the JS). Feeds ScriptProblemsList; the - /// markers are sent from JS only when this callback has a subscriber. + /// markers are sent from JS only when this callback has a subscriber, and at most + /// of them (plus the total) so the report always fits in one + /// circuit message. /// - [Parameter] public EventCallback> MarkersChanged { get; set; } + [Parameter] public EventCallback MarkersChanged { get; set; } private ElementReference _hostRef; private DotNetObjectReference? _dotNetRef; private readonly string _id = Guid.NewGuid().ToString("N"); private string _lastSentValue = ""; private bool _initialized; + private int _tooLargeBytes; private bool _wrap; private bool _minimap; private bool _dark; @@ -78,7 +89,9 @@ readOnly = ReadOnly, equipmentId = EquipmentId, maxSyncBytes = ScriptEditorLimits.MaxEditorSyncBytes, - reportMarkers = MarkersChanged.HasDelegate + reportMarkers = MarkersChanged.HasDelegate, + maxReportedMarkers = ScriptEditorLimits.MaxReportedMarkers, + maxReportedMessageChars = ScriptEditorLimits.MaxReportedMessageChars }, _dotNetRef); _initialized = true; @@ -160,6 +173,7 @@ return new EditorReadResult(null, ex.Message.Split('\n', 2)[0].Trim()); } if (current is null) return new EditorReadResult(Value ?? "", null); // editor already disposed + _tooLargeBytes = 0; // it was read, so it fits await OnValueChanged(current); return new EditorReadResult(current, null); } @@ -174,8 +188,17 @@ } [JSInvokable] - public Task OnMarkersChanged(DiagnosticMarker[]? markers) => - MarkersChanged.InvokeAsync(markers ?? Array.Empty()); + public Task OnMarkersChanged(DiagnosticMarker[]? markers, int total) => + MarkersChanged.InvokeAsync(ScriptProblemReport.FromEditor(markers, total)); + + /// The JS reports, at blur, that the text has grown past (bytes > 0) or back under (0) the sync limit. + [JSInvokable] + public void OnSyncRefused(int bytes) + { + if (_tooLargeBytes == bytes) return; + _tooLargeBytes = bytes; + StateHasChanged(); + } /// Programmatic scroll-to-line (called by the problems panel). public async Task RevealLineAsync(int line, int column = 1) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor index c868a46a..7ebd9e4a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/ScriptProblemsList.razor @@ -5,12 +5,13 @@ ScriptProblems (unit-tested). *@ @{ - var ordered = ScriptProblems.Order(Markers); + var report = Report ?? ScriptProblemReport.Empty; + var ordered = ScriptProblems.Order(report.Markers); }
Problems - @ScriptProblems.Summary(ordered) + @ScriptProblems.Summary(report with { Markers = ordered })
@if (ordered.Count > 0) { @@ -29,18 +30,20 @@ } - @if (ordered.Count > MaxRows) + @if (report.Total > Math.Min(ordered.Count, MaxRows)) { -
… and @(ordered.Count - MaxRows) more
+ @* The editor reports at most ScriptEditorLimits.MaxReportedMarkers problems (one circuit message); the + rest are still squiggled in the editor. *@ +
+@(report.Total - Math.Min(ordered.Count, MaxRows)) more — fix the problems above, or scroll the editor to see them all.
} }
@code { - private const int MaxRows = 200; + private const int MaxRows = ScriptEditorLimits.MaxReportedMarkers; - /// The markers the editor last reported (null before the first diagnostics pass). - [Parameter] public IReadOnlyList? Markers { get; set; } + /// The report the editor last sent (null before the first diagnostics pass). + [Parameter] public ScriptProblemReport? Report { get; set; } /// Raised when the author picks a problem; wire it to MonacoEditor.RevealLineAsync. [Parameter] public EventCallback OnSelect { get; set; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor index cbd7c0d4..789d6ccd 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/VirtualTagModal.razor @@ -102,7 +102,7 @@ - + @if (!string.IsNullOrWhiteSpace(_scriptError)) {
@_scriptError
@@ -221,9 +221,9 @@ private bool _scriptSaved; private string? _scriptError; private MonacoEditor? _scriptEditor; - private IReadOnlyList _scriptMarkers = Array.Empty(); + private ScriptProblemReport _scriptMarkers = ScriptProblemReport.Empty; - private void OnScriptMarkersChanged(IReadOnlyList markers) => _scriptMarkers = markers; + private void OnScriptMarkersChanged(ScriptProblemReport report) => _scriptMarkers = report; private Task RevealScriptProblemAsync(DiagnosticMarker m) => _scriptEditor is null ? Task.CompletedTask : _scriptEditor.RevealLineAsync(m.StartLineNumber, m.StartColumn); @@ -248,6 +248,9 @@ if (!Visible) { _loadedKey = null; // closed → next open reloads fresh + // Closing removes the editor component: drop its reference and its problems. + _scriptEditor = null; + _scriptMarkers = ScriptProblemReport.Empty; return; } @@ -299,6 +302,9 @@ { _scriptSaved = false; _scriptError = null; + // A different script (or a reload) — drop the previous script's problems. The editor instance itself stays: + // while the panel is open the same MonacoEditor component receives the new source via its Value. + _scriptMarkers = ScriptProblemReport.Empty; if (string.IsNullOrEmpty(_form.ScriptId)) { @@ -387,7 +393,7 @@ // A failed read keeps the panel open (with the error) rather than discarding the edits. if (!await PullScriptSourceAsync()) return; _scriptEditor = null; - _scriptMarkers = Array.Empty(); + _scriptMarkers = ScriptProblemReport.Empty; } _scriptExpanded = !_scriptExpanded; if (_scriptExpanded) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs index 6945c32f..7182324c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptAnalysisService.cs @@ -77,19 +77,14 @@ public sealed class ScriptAnalysisService return compilation; } - // This scope's current compilation (one per scoped service). Each analysis swaps its tree in with - // ReplaceSyntaxTree; a lost race between two concurrent calls on one scope is harmless because every - // compilation in the lineage is a complete, valid single-tree compilation. - private CSharpCompilation _compilation = BaseCompilation; - - // userSource is appended after Preamble, then the method/class are closed. - private (SyntaxTree Tree, CSharpCompilation Compilation, SemanticModel Model, int PreambleLength) Analyze( + // userSource is appended after Preamble, then the method/class are closed. Every analysis derives straight + // from the shared BaseCompilation: the analysis endpoints are minimal-API handlers, so a scoped service lives + // for one HTTP request and a per-instance lineage would never be more than one step long. + private static (SyntaxTree Tree, CSharpCompilation Compilation, SemanticModel Model, int PreambleLength) Analyze( string userSource, CancellationToken ct) { var tree = WrapAndParse(userSource, ct); - var current = _compilation; - var compilation = current.ReplaceSyntaxTree(current.SyntaxTrees.Single(), tree); - _compilation = compilation; + var compilation = BaseCompilation.ReplaceSyntaxTree(BaseCompilation.SyntaxTrees[0], tree); return (tree, compilation, compilation.GetSemanticModel(tree), Preamble.Length); } @@ -159,7 +154,7 @@ public sealed class ScriptAnalysisService foreach (var inv in SetVirtualTagCalls(tree.GetRoot(ct), preambleLength)) markers.Add(ToMarker(tree.GetMappedLineSpan(inv.Span), 4, SetVirtualTagInertMessage, "OTSCRIPT_SETVTAG_INERT")); - return new DiagnoseResponse(markers.Distinct().ToList()); + return new DiagnoseResponse(Bound(markers)); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -216,7 +211,7 @@ public sealed class ScriptAnalysisService $"reference named '{refName}'. Add a matching reference or correct the path.", "OTSCRIPT_EQUIPREF")); } - return new DiagnoseResponse(markers.Distinct().ToList()); + return new DiagnoseResponse(Bound(markers)); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -229,6 +224,18 @@ public sealed class ScriptAnalysisService } } + // Bounds what the diagnostics endpoint returns (and so what Monaco renders and the Problems list is fed from): + // de-duplicated, errors first then by position, at most MaxDiagnosticMarkers, each message cut to + // MaxDiagnosticMessageChars. Ordering before the cut keeps an error whenever one exists, so the editor still + // rejects exactly what publish rejects even for a pathological (e.g. non-C#) paste. + private static IReadOnlyList Bound(IEnumerable markers) + => ScriptProblems.Order(markers.Distinct()) + .Take(ScriptEditorLimits.MaxDiagnosticMarkers) + .Select(m => m.Message.Length <= ScriptEditorLimits.MaxDiagnosticMessageChars + ? m + : m with { Message = ScriptProblems.Truncate(m.Message, ScriptEditorLimits.MaxDiagnosticMessageChars) }) + .ToList(); + private const string SetVirtualTagInertMessage = "ctx.SetVirtualTag(...) has no effect at runtime: cross-tag writes are dropped in single-tag mode (the " + "cascade engine is not wired into the host). The script compiles and publishes, but this write will not " + diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs index e2b89893..96fcbe66 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptEditorLimits.cs @@ -16,4 +16,26 @@ public static class ScriptEditorLimits /// The largest serialized editor value the JS will send, leaving 64 KB of frame headroom. public const int MaxEditorSyncBytes = (int)HubMaximumReceiveMessageSize - 64 * 1024; + + /// + /// The most diagnostic markers the editor reports to .NET (for the Problems list) in one circuit message. + /// A non-C# paste produces several Roslyn errors per line; uncapped, a few thousand markers exceed the hub + /// limit and tear the circuit down. The JS sends the first N in display order (errors first) plus the total. + /// Monaco's own in-editor squiggles are local and stay uncapped (up to ). + /// + public const int MaxReportedMarkers = 200; + + /// Each reported marker's message is truncated to this many characters before it crosses the circuit. + public const int MaxReportedMessageChars = 300; + + /// + /// The most markers the diagnostics endpoint returns (errors first). Bounds the analysis response and the + /// marker set Monaco renders for pathological input; an error is always kept when one exists, so the + /// editor still rejects exactly what publish rejects. + /// + public const int MaxDiagnosticMarkers = 1000; + + /// Each diagnostics-endpoint marker message is truncated to this many characters (a Roslyn message + /// can quote an arbitrarily long pasted token). + public const int MaxDiagnosticMessageChars = 1000; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs index 6bd3ee05..5d3969ae 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/ScriptProblems.cs @@ -50,5 +50,54 @@ public static class ScriptProblems return string.Join(", ", parts); } + /// A one-line count for a (possibly capped) report, e.g. "200 errors (showing 200 of 4321)". + /// The report the editor sent. + /// The summary of the shown markers, with the shown/total suffix when some were left out. + public static string Summary(ScriptProblemReport report) + { + var summary = Summary(report.Markers.ToList()); + return report.Omitted > 0 ? $"{summary} (showing {report.Markers.Count} of {report.Total})" : summary; + } + + /// Cuts to at most characters, ending in an ellipsis. + /// The text. + /// The maximum length, ellipsis included (at least 1). + /// The text, unchanged when it already fits. + public static string Truncate(string text, int maxChars) + => text.Length <= maxChars ? text : string.Concat(text.AsSpan(0, Math.Max(0, maxChars - 1)), "…"); + private static string Plural(int n, string noun) => n == 1 ? $"1 {noun}" : $"{n} {noun}s"; } + +/// +/// What the editor reported for the Problems list: at most +/// markers in display order (errors first) plus the the diagnostics pass produced. The JS caps +/// the report before it crosses the circuit — one hub message, bounded by the 1 MB receive limit — and +/// clamps it again on arrival. +/// +/// The markers shown. +/// How many markers the diagnostics pass produced (≥ Markers.Count). +public sealed record ScriptProblemReport(IReadOnlyList Markers, int Total) +{ + /// No problems. + public static readonly ScriptProblemReport Empty = new(Array.Empty(), 0); + + /// How many markers were left out of . + public int Omitted => Math.Max(0, Total - Markers.Count); + + /// Builds a report from what the editor JS sent, re-applying the circuit caps defensively. + /// The markers sent (already capped by the JS). + /// The total the JS counted. + /// The clamped report. + public static ScriptProblemReport FromEditor(IReadOnlyList? markers, int total) + { + if (markers is null || markers.Count == 0) return total <= 0 ? Empty : new(Array.Empty(), total); + var shown = ScriptProblems.Order(markers) + .Take(ScriptEditorLimits.MaxReportedMarkers) + .Select(m => m.Message.Length <= ScriptEditorLimits.MaxReportedMessageChars + ? m + : m with { Message = ScriptProblems.Truncate(m.Message, ScriptEditorLimits.MaxReportedMessageChars) }) + .ToList(); + return new ScriptProblemReport(shown, Math.Max(total, markers.Count)); + } +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js index 8a47002c..e5fc3f0e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/wwwroot/js/monaco-init.js @@ -169,11 +169,44 @@ function syncValue(e) { if (!e.dirty) return; const v = e.editor.getValue(); - if (tooLargeMessage(e, v)) return; // stays dirty; the explicit save read reports the error + if (tooLargeMessage(e, v)) { + // Stays dirty; the explicit save read reports the error. Tell .NET once (a tiny message) so the + // author sees a non-blocking notice now rather than only at Save. + if (!e.tooLargeNotified) { + e.tooLargeNotified = true; + e.dotNetRef.invokeMethodAsync("OnSyncRefused", serializedBytes(v)).catch(function () {}); + } + return; + } + if (e.tooLargeNotified) { + e.tooLargeNotified = false; + e.dotNetRef.invokeMethodAsync("OnSyncRefused", 0).catch(function () {}); + } e.dirty = false; e.dotNetRef.invokeMethodAsync("OnValueChanged", v).catch(function () { e.dirty = true; }); } + // The Problems-list report is ONE circuit message too. A non-C# paste yields thousands of markers, so only the + // first maxReportedMarkers in display order (errors first, then by position — ScriptProblems.Order) cross, each + // message cut to maxReportedMessageChars, plus the total so the list can say how many were left out. + // Monaco's own squiggles (setModelMarkers) are local and stay uncapped. + function severityOrder(a, b) { + return (b.severity - a.severity) || (a.startLineNumber - b.startLineNumber) || (a.startColumn - b.startColumn); + } + function markerReport(e, markers) { + const max = e.maxReportedMarkers || 200, maxChars = e.maxReportedMessageChars || 300; + const shown = markers.slice().sort(severityOrder).slice(0, max).map(function (m) { + const msg = typeof m.message === "string" && m.message.length > maxChars + ? m.message.substring(0, maxChars - 1) + "\u2026" : m.message; + return { + severity: m.severity, startLineNumber: m.startLineNumber, startColumn: m.startColumn, + endLineNumber: m.endLineNumber, endColumn: m.endColumn, message: msg, + code: typeof m.code === "string" ? m.code.substring(0, 64) : m.code + }; + }); + return { markers: shown, total: markers.length }; + } + // A single body-level node for Monaco's overflow widgets (suggest / hover / parameter hints). // Rendering them here — outside any ancestor with a CSS transform (e.g. the theme's `.rise` // entrance animation) — keeps the position:fixed widgets viewport-correct. Without it, the @@ -215,6 +248,8 @@ const e = { editor: editor, dotNetRef: dotNetRef, dirty: false, maxSyncBytes: options.maxSyncBytes || 0, reportMarkers: !!options.reportMarkers, + maxReportedMarkers: options.maxReportedMarkers || 200, maxReportedMessageChars: options.maxReportedMessageChars || 300, + tooLargeNotified: false, diagTimer: null, diagGeneration: 0, diagAbort: null }; // Diagnostics: the 500 ms debounce delays the START of a request; the generation counter + AbortController @@ -229,7 +264,10 @@ if (markers === null || generation !== e.diagGeneration || editors[id] !== e) return; // superseded e.diagAbort = null; monaco.editor.setModelMarkers(model, "otopcua", markers); - if (e.reportMarkers) dotNetRef.invokeMethodAsync("OnMarkersChanged", markers).catch(function () {}); + if (e.reportMarkers) { + const report = markerReport(e, markers); + dotNetRef.invokeMethodAsync("OnMarkersChanged", report.markers, report.total).catch(function () {}); + } }; e.scheduleDiagnostics = function () { e.diagGeneration++; diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MarkerPayloadBoundTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MarkerPayloadBoundTests.cs new file mode 100644 index 00000000..478395aa --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MarkerPayloadBoundTests.cs @@ -0,0 +1,99 @@ +using System.Text; +using System.Text.Json; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.AdminUI.ScriptAnalysis; + +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.ScriptAnalysis; + +/// +/// Review D-a I-1 — the diagnostics markers cross the Blazor circuit (JS → .NET, for the Problems list) as ONE +/// hub message. A non-C# paste yields several Roslyn errors per line; uncapped, a few thousand markers exceed +/// the 1 MB hub limit and kill the circuit with every unsaved edit. Both the endpoint response and the circuit +/// report are now bounded; these tests pin the bounds on the C# side of the contract. +/// +public sealed class MarkerPayloadBoundTests +{ + // ~150 KB of log-ish text: every line is several C# errors. + private static string NonCSharpPaste() + { + var sb = new StringBuilder(); + for (var i = 0; sb.Length < 150 * 1024; i++) + sb.Append($"2026-10-22 12:00:{i % 60:00} INFO [worker-{i}] value={i}, state=ok; next -> {i + 1} @@ ##\n"); + return sb.ToString(); + } + + [Fact] + public void A_non_csharp_paste_returns_a_bounded_marker_set_that_still_holds_the_errors() + { + var markers = new ScriptAnalysisService().Diagnose(new DiagnoseRequest(NonCSharpPaste())).Markers; + + markers.Count.ShouldBe(ScriptEditorLimits.MaxDiagnosticMarkers); // positive control: the cap was hit + markers.ShouldAllBe(m => m.Message.Length <= ScriptEditorLimits.MaxDiagnosticMessageChars); + markers[0].Severity.ShouldBe(8); // errors first — publish rejects, so must the editor + markers.Select(m => (m.Severity, m.StartLineNumber, m.StartColumn)) + .ShouldBe(ScriptProblems.Order(markers).Select(m => (m.Severity, m.StartLineNumber, m.StartColumn))); + } + + [Fact] + public void A_long_quoted_token_has_its_message_truncated() + { + var giant = new string('x', 20_000); + var markers = new ScriptAnalysisService().Diagnose(new DiagnoseRequest($"return {giant};")).Markers; + + markers.ShouldNotBeEmpty(); + markers.ShouldAllBe(m => m.Message.Length <= ScriptEditorLimits.MaxDiagnosticMessageChars); + markers.ShouldContain(m => m.Message.EndsWith("…")); + } + + [Fact] + public void A_small_script_is_not_truncated() + { + // Positive control for the cap: ordinary diagnostics pass through untouched. + var markers = new ScriptAnalysisService().Diagnose(new DiagnoseRequest("return ctx.Nope();")).Markers; + markers.ShouldContain(m => m.Severity == 8 && !m.Message.EndsWith("…")); + } + + [Fact] + public void The_worst_case_circuit_report_fits_inside_the_sync_budget() + { + // The JS sends at most MaxReportedMarkers markers, each message cut to MaxReportedMessageChars. Worst case: + // control characters (\u0001 → 6 JSON bytes, 7 when escaped again) and the largest line/column numbers. + var message = new string('\u0001', ScriptEditorLimits.MaxReportedMessageChars); + var marker = new DiagnosticMarker(8, int.MaxValue, int.MaxValue, int.MaxValue, int.MaxValue, message, "OTSCRIPT_SETVTAG_INERT"); + var report = Enumerable.Repeat(marker, ScriptEditorLimits.MaxReportedMarkers).ToArray(); + + var once = JsonSerializer.Serialize(new object[] { report, int.MaxValue }); + var twice = Encoding.UTF8.GetByteCount(JsonSerializer.Serialize(once)); + + twice.ShouldBeLessThan(ScriptEditorLimits.MaxEditorSyncBytes); + ScriptEditorLimits.MaxEditorSyncBytes.ShouldBeLessThan((int)ScriptEditorLimits.HubMaximumReceiveMessageSize); + } + + [Fact] + public void A_truncated_report_says_how_many_were_left_out() + { + var shown = Enumerable.Range(1, 3).Select(i => new DiagnosticMarker(8, i, 1, i, 2, "m", "CS1002")).ToArray(); + + var report = new ScriptProblemReport(shown, 4321); + report.Omitted.ShouldBe(4318); + ScriptProblems.Summary(report).ShouldBe("3 errors (showing 3 of 4321)"); + + // Positive control: an untruncated report carries no suffix. + ScriptProblems.Summary(new ScriptProblemReport(shown, 3)).ShouldBe("3 errors"); + ScriptProblemReport.Empty.Omitted.ShouldBe(0); + } + + [Fact] + public void A_report_from_js_is_clamped_even_if_the_js_cap_regressed() + { + var flood = Enumerable.Range(1, 5000).Select(i => new DiagnosticMarker(4, i, 1, i, 2, new string('m', 2000), "W")).ToArray(); + + var report = ScriptProblemReport.FromEditor(flood, total: 5000); + + report.Markers.Count.ShouldBe(ScriptEditorLimits.MaxReportedMarkers); + report.Total.ShouldBe(5000); + report.Markers.ShouldAllBe(m => m.Message.Length <= ScriptEditorLimits.MaxReportedMessageChars); + ScriptProblemReport.FromEditor(null, 0).ShouldBe(ScriptProblemReport.Empty); + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs index 0e7356cc..179ec723 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/MonacoInteropSourceTests.cs @@ -62,10 +62,34 @@ public sealed class MonacoInteropSourceTests [Fact] public void Markers_cross_to_dotnet_only_for_a_subscribed_problems_list() { - Js().ShouldContain("if (e.reportMarkers) dotNetRef.invokeMethodAsync(\"OnMarkersChanged\""); + Js().ShouldContain("if (e.reportMarkers) {"); Read("Components", "Shared", "MonacoEditor.razor").ShouldContain("reportMarkers = MarkersChanged.HasDelegate"); } + [Fact] + public void The_markers_report_is_capped_before_it_crosses_the_circuit() + { + // Review I-1: the only OnMarkersChanged invoke sends the capped report, never the raw marker array. + var js = Js(); + Regex.Matches(js, "\"OnMarkersChanged\"").Count.ShouldBe(1); + js.ShouldContain("dotNetRef.invokeMethodAsync(\"OnMarkersChanged\", report.markers, report.total)"); + js.ShouldContain("const report = markerReport(e, markers);"); + js.ShouldContain(".slice(0, max)"); + js.ShouldContain("m.message.substring(0, maxChars - 1)"); + // The caps come from the C# constants, so the JS and ScriptProblemReport.FromEditor agree. + var razor = Read("Components", "Shared", "MonacoEditor.razor"); + razor.ShouldContain("maxReportedMarkers = ScriptEditorLimits.MaxReportedMarkers"); + razor.ShouldContain("maxReportedMessageChars = ScriptEditorLimits.MaxReportedMessageChars"); + razor.ShouldContain("ScriptProblemReport.FromEditor(markers, total)"); + } + + [Fact] + public void An_oversize_text_is_announced_at_blur_not_only_at_save() + { + Js().ShouldContain("invokeMethodAsync(\"OnSyncRefused\", serializedBytes(v))"); + Read("Components", "Shared", "MonacoEditor.razor").ShouldContain("public void OnSyncRefused(int bytes)"); + } + [Theory] [InlineData("Components", "Pages", "ScriptEdit.razor")] [InlineData("Components", "Shared", "Uns", "VirtualTagModal.razor")]