docs(comments): strip internal task/milestone/bundle bookkeeping from code comments
Remove project bookkeeping citations from shipped code comments across the solution: hyphenated task IDs (WP-14, StoreAndForward-025), milestone/task/ issue refs (M3, Task 4, Audit Log #23, #21), Bundle X task-bundle labels, and C/D/K/S/T phase labels. Comment text only — no code logic, string/log literals, or XML-doc structure changed. Genuine descriptions are preserved (only the citation is stripped), and technical lookalikes are retained (UTF-8, SHA-256, T00:00:00, M365, UTC-5, pre-C4/pre-C5 schema versions). Flagged by the new CommentChecker TaskReferenceInComment / TrackingReferenceInComment checks plus targeted grep passes; full solution builds clean, append-only guard tests pass.
This commit is contained in:
@@ -13,25 +13,25 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types;
|
||||
namespace ZB.MOM.WW.ScadaBridge.InboundAPI;
|
||||
|
||||
/// <summary>
|
||||
/// WP-3: Executes the C# script associated with an inbound API method.
|
||||
/// Executes the C# script associated with an inbound API method.
|
||||
/// Compiles method scripts via Roslyn and caches compiled delegates.
|
||||
/// </summary>
|
||||
public class InboundScriptExecutor
|
||||
{
|
||||
private readonly ILogger<InboundScriptExecutor> _logger;
|
||||
|
||||
// InboundAPI-001: this executor is registered as a singleton and its handler cache
|
||||
// This executor is registered as a singleton and its handler cache
|
||||
// is read and written from concurrent ASP.NET request threads. A plain Dictionary is
|
||||
// not safe for concurrent read/write, so a ConcurrentDictionary is used throughout.
|
||||
private readonly ConcurrentDictionary<string, Func<InboundScriptContext, Task<object?>>> _scriptHandlers = new();
|
||||
|
||||
// InboundAPI-009: a script that fails to compile (or violates the trust model)
|
||||
// A script that fails to compile (or violates the trust model)
|
||||
// is recorded here so it is compiled at most once. Without this, every subsequent
|
||||
// request for a broken method re-runs the expensive Roslyn compilation — a CPU
|
||||
// amplification vector since the inbound API has no rate limiting. The entry is
|
||||
// cleared whenever the method is (re)compiled via CompileAndRegister.
|
||||
//
|
||||
// InboundAPI-024: bound the cache so a spam attack of unique method names cannot
|
||||
// Bound the cache so a spam attack of unique method names cannot
|
||||
// grow it without bound. Once the cap is reached new bad-method records are
|
||||
// dropped — the cache is just a fast-fail optimisation; the per-request DB
|
||||
// lookup remains the correctness path.
|
||||
@@ -39,14 +39,14 @@ public class InboundScriptExecutor
|
||||
private readonly ConcurrentDictionary<string, byte> _knownBadMethods = new();
|
||||
|
||||
/// <summary>
|
||||
/// InboundAPI-024 diagnostic helper — returns the current size of the
|
||||
/// Diagnostic helper — returns the current size of the
|
||||
/// known-bad-methods cache so tests can assert the cap is honoured. Internal
|
||||
/// so the cache itself stays an implementation detail.
|
||||
/// </summary>
|
||||
internal int KnownBadMethodCount => _knownBadMethods.Count;
|
||||
|
||||
/// <summary>
|
||||
/// InboundAPI-024: records <paramref name="methodName"/> in the known-bad-methods
|
||||
/// Records <paramref name="methodName"/> in the known-bad-methods
|
||||
/// cache only if the cache has not reached <see cref="KnownBadMethodsCap"/>.
|
||||
/// Once full, new records are dropped (paying the cheap recompile next time
|
||||
/// rather than leaking memory under a unique-name flood). Existing entries are
|
||||
@@ -120,9 +120,9 @@ public class InboundScriptExecutor
|
||||
var (handler, compileErrors) = Compile(method);
|
||||
if (handler == null)
|
||||
{
|
||||
// InboundAPI-009: record the failure so the lazy-compile path does not
|
||||
// keep recompiling a broken script on every request. InboundAPI-024:
|
||||
// routed through the capped TryRecordBadMethod helper so the cache
|
||||
// Record the failure so the lazy-compile path does not
|
||||
// keep recompiling a broken script on every request. Routed
|
||||
// through the capped TryRecordBadMethod helper so the cache
|
||||
// cannot grow without bound under a flood of unique method names.
|
||||
errors = compileErrors;
|
||||
TryRecordBadMethod(method.Name);
|
||||
@@ -145,7 +145,7 @@ public class InboundScriptExecutor
|
||||
/// <summary>
|
||||
/// Compiles a single API method script into an executable handler. Returns
|
||||
/// <c>null</c> when the script is missing, fails to compile, or violates the
|
||||
/// script trust model (InboundAPI-005). Does not mutate the handler cache.
|
||||
/// script trust model. Does not mutate the handler cache.
|
||||
/// </summary>
|
||||
private (Func<InboundScriptContext, Task<object?>>? Handler, IReadOnlyList<string> Errors) Compile(ApiMethod method)
|
||||
{
|
||||
@@ -155,7 +155,7 @@ public class InboundScriptExecutor
|
||||
return (null, new[] { "Method has no script code." });
|
||||
}
|
||||
|
||||
// InboundAPI-005: enforce the script trust model before compiling. Roslyn
|
||||
// Enforce the script trust model before compiling. Roslyn
|
||||
// scripting performs no API allow/deny-listing, so forbidden namespaces must
|
||||
// be rejected statically or the script could reach the host process.
|
||||
var violations = ForbiddenApiChecker.FindViolations(method.Script);
|
||||
@@ -225,7 +225,7 @@ public class InboundScriptExecutor
|
||||
/// <param name="timeout">Timeout duration for script execution.</param>
|
||||
/// <param name="cancellationToken">Cancellation token.</param>
|
||||
/// <param name="parentExecutionId">
|
||||
/// Audit Log #23 (ParentExecutionId): the inbound API request's per-request
|
||||
/// ParentExecutionId: the inbound API request's per-request
|
||||
/// <c>ExecutionId</c> (minted early by <c>AuditWriteMiddleware</c> and stashed
|
||||
/// on <c>HttpContext.Items</c>). When supplied, a routed
|
||||
/// <c>Route.To(...).Call(...)</c> inside the script carries it as
|
||||
@@ -245,7 +245,7 @@ public class InboundScriptExecutor
|
||||
// Every call site passes it by named argument (parentExecutionId:).
|
||||
Guid? parentExecutionId = null)
|
||||
{
|
||||
// InboundAPI-004: keep the timeout source and the request-abort source
|
||||
// Keep the timeout source and the request-abort source
|
||||
// separable. A single linked CTS makes a genuine client disconnect
|
||||
// indistinguishable from a method timeout, so a normal disconnect would be
|
||||
// logged and reported as "Script execution timed out". Use a dedicated
|
||||
@@ -255,7 +255,7 @@ public class InboundScriptExecutor
|
||||
cancellationToken, timeoutCts.Token);
|
||||
|
||||
// IpsenMES MoveIn: expose the scoped, parameterized Database helper to the script
|
||||
// (reads + writes — InboundAPI-026). Resolve the gateway from a fresh DI scope
|
||||
// (reads + writes). Resolve the gateway from a fresh DI scope
|
||||
// (declared outside the try so it lives until after the script handler runs and is
|
||||
// disposed here) so a scoped IDatabaseGateway is honoured. GetService (not
|
||||
// GetRequiredService) so a method that never touches Database still runs even when
|
||||
@@ -283,22 +283,22 @@ public class InboundScriptExecutor
|
||||
try
|
||||
{
|
||||
var gateway = scope?.ServiceProvider.GetService<IDatabaseGateway>();
|
||||
// InboundAPI-027: pass the method timeout so the helper derives a
|
||||
// Pass the method timeout so the helper derives a
|
||||
// CommandTimeout backstop and forwards cts.Token to every async DB call —
|
||||
// a slow query is then bounded by the method deadline.
|
||||
var dbHelper = new InboundDatabaseHelper(gateway, cts.Token, timeout);
|
||||
|
||||
// InboundAPI-016: bind the route helper to the method deadline so a
|
||||
// Bind the route helper to the method deadline so a
|
||||
// routed Route.To(...).Call(...) inherits the method-level timeout
|
||||
// without the script having to thread the context token by hand.
|
||||
//
|
||||
// InboundAPI-029: also pass the raw request-abort token separately so
|
||||
// Also pass the raw request-abort token separately so
|
||||
// Route.To(...).WaitForAttribute(...) can be bounded by its WAIT timeout
|
||||
// (not the generic method deadline) while still being cancelled by a
|
||||
// client disconnect — see RouteTarget.WaitForAttribute.
|
||||
//
|
||||
// Audit Log #23 (ParentExecutionId): also bind the inbound request's
|
||||
// ExecutionId so a routed call carries it as ParentExecutionId — the
|
||||
// Also bind the inbound request's ExecutionId (ParentExecutionId) so a
|
||||
// routed call carries it as ParentExecutionId — the
|
||||
// spawned site script execution points back at this inbound request.
|
||||
var context = new InboundScriptContext(
|
||||
parameters,
|
||||
@@ -310,7 +310,7 @@ public class InboundScriptExecutor
|
||||
|
||||
if (!_scriptHandlers.TryGetValue(method.Name, out var handler))
|
||||
{
|
||||
// InboundAPI-009: a method already known to fail compilation must not
|
||||
// A method already known to fail compilation must not
|
||||
// be recompiled on every request — short-circuit before Roslyn runs.
|
||||
if (_knownBadMethods.ContainsKey(method.Name))
|
||||
return new InboundScriptResult(false, null, "Script compilation failed for this method");
|
||||
@@ -322,7 +322,7 @@ public class InboundScriptExecutor
|
||||
if (compiled == null)
|
||||
{
|
||||
// Cache the failure so the next request short-circuits above.
|
||||
// InboundAPI-024: routed through TryRecordBadMethod so the
|
||||
// Routed through TryRecordBadMethod so the
|
||||
// cache is bounded under a flood of unique method names.
|
||||
TryRecordBadMethod(method.Name);
|
||||
return new InboundScriptResult(false, null, "Script compilation failed for this method");
|
||||
@@ -336,7 +336,7 @@ public class InboundScriptExecutor
|
||||
? JsonSerializer.Serialize(result)
|
||||
: null;
|
||||
|
||||
// M9-T32b: thread the JSON-Schema $ref resolution seam into return
|
||||
// Thread the JSON-Schema $ref resolution seam into return
|
||||
// validation so a method whose ReturnDefinition uses a {"$ref":"lib:Name"}
|
||||
// resolves the reference at RUNTIME (not just at deploy time). The
|
||||
// shared-schema library is pre-loaded ONCE from the per-execution DI scope
|
||||
@@ -349,7 +349,7 @@ public class InboundScriptExecutor
|
||||
var resolveRef = await SchemaRefResolver.BuildAsync(
|
||||
sharedSchemaRepo, [method.ReturnDefinition], cts.Token);
|
||||
|
||||
// InboundAPI-014: validate the script's return value against the
|
||||
// Validate the script's return value against the
|
||||
// method's declared ReturnDefinition. A method whose script returns a
|
||||
// shape inconsistent with its definition must not silently emit a
|
||||
// malformed 200 — surface it as a script failure (500) and log.
|
||||
@@ -366,7 +366,7 @@ public class InboundScriptExecutor
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
// InboundAPI-004: distinguish a genuine method timeout from a client
|
||||
// Distinguish a genuine method timeout from a client
|
||||
// abort. Only the timeout CTS firing is a real timeout; if the caller's
|
||||
// request token fired, the client disconnected — do not pollute the
|
||||
// timeout log (reserved for genuine script-execution timeouts).
|
||||
@@ -382,7 +382,7 @@ public class InboundScriptExecutor
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogError(ex, "Script execution failed for method {Method}", method.Name);
|
||||
// WP-5: Safe error message, no internal details
|
||||
// Safe error message, no internal details
|
||||
return new InboundScriptResult(false, null, "Internal script error");
|
||||
}
|
||||
finally
|
||||
|
||||
Reference in New Issue
Block a user