fix(adminui): dedup + chunk the rename script-scan OR-chain

2026-08-16 04:36:03 -04:00
parent 485f4ea1e8
commit ca87329a83
2 changed files with 109 additions and 8 deletions
@@ -1415,6 +1415,12 @@ public sealed class RawTreeService(IDbContextFactory<OtOpcUaConfigDbContext> dbF
        return warnings;
    }

+    /// <summary>
+    /// Max <c>OR</c> terms in one script-scan predicate — bounds both EF's recursive expression-visitor
+    /// depth over the left-nested <c>OrElse</c> chain and SQL Server's boolean-expression complexity limit.
+    /// </summary>
+    private const int ScriptScanBatchSize = 200;
+
    /// <summary>
    /// Scans every <see cref="Script"/> body for the OLD RawPath of any affected tag as a plain substring
    /// (no AST — false positives tolerated by design; <c>{{equip}}</c>-relative refs are NOT scanned, they
@@ -1423,7 +1429,9 @@ public sealed class RawTreeService(IDbContextFactory<OtOpcUaConfigDbContext> dbF
    /// <para>
    /// The substring match runs in the DATABASE (see <see cref="AnyPathContained"/>), so unmatched script
    /// bodies — the overwhelming majority, and the largest text column in the config DB — never cross the
-    /// wire; only the matched scripts' names are projected.
+    /// wire; only the matched scripts' names are projected. The paths are deduplicated and scanned in
+    /// batches of <see cref="ScriptScanBatchSize"/>, so the disjunction stays bounded whatever the size of
+    /// the renamed subtree.
    /// </para>
    /// </summary>
    private static async Task<string?> BuildScriptReferenceWarningAsync(
@@ -1432,16 +1440,30 @@ public sealed class RawTreeService(IDbContextFactory<OtOpcUaConfigDbContext> dbF
        var oldPaths = await ComputeOldRawPathsAsync(db, tags, ct);
        if (oldPaths.Count == 0) return null;

-        var matched = await db.Scripts.AsNoTracking()
-            .Where(AnyPathContained(oldPaths))
-            .Select(s => s.Name)
-            .ToListAsync(ct);
+        // One path per affected TAG, so an ancestor rename yields thousands of them, many identical (a
+        // tag name repeated under sibling devices). Deduplicating first shortens the disjunction without
+        // narrowing the match — the SQL comparison runs under the column collation either way.
+        var distinctPaths = oldPaths.Distinct(StringComparer.Ordinal).ToArray();
+
+        // ScriptId, not Name: Name carries no unique index, and collapsing two same-named scripts into one
+        // would change the count this warning reports.
+        var matched = new Dictionary<string, string>(StringComparer.Ordinal);
+        for (var offset = 0; offset < distinctPaths.Length; offset += ScriptScanBatchSize)
+        {
+            var batch = distinctPaths[offset..Math.Min(offset + ScriptScanBatchSize, distinctPaths.Length)];
+            var rows = await db.Scripts.AsNoTracking()
+                .Where(AnyPathContained(batch))
+                .Select(s => new { s.ScriptId, s.Name })
+                .ToListAsync(ct);
+            foreach (var row in rows) matched[row.ScriptId] = row.Name;
+        }
        if (matched.Count == 0) return null;

        // Ordered in memory: SQL's ORDER BY would sort under the column collation, not ordinally.
-        matched.Sort(StringComparer.Ordinal);
+        var matchedNames = matched.Values.ToList();
+        matchedNames.Sort(StringComparer.Ordinal);

-        var names = string.Join(", ", matched.Select(n => $"'{n}'"));
+        var names = string.Join(", ", matchedNames.Select(n => $"'{n}'"));
        return matched.Count == 1
            ? $"1 script ({names}) references a tag beneath this node by its raw path; the reference will break unless the script is updated."
            : $"{matched.Count} scripts ({names}) reference tags beneath this node by their raw paths; the references will break unless the scripts are updated.";
@@ -43,11 +43,12 @@ public sealed class RawTreeServiceRenameWarningTests
    private const string EquipmentName = "packer-1";
    private const string ScriptName = "AreaCalc";

-    private static (RawTreeService Service, string DbName) Seeded()
+    private static (RawTreeService Service, string DbName) Seeded(Action<OtOpcUaConfigDbContext>? extra = null)
    {
        var name = $"rawwarn-{Guid.NewGuid():N}";
        using var db = CreateNamed(name);
        Seed(db);
+        extra?.Invoke(db);
        return (new RawTreeService(new NamedFactory(name)), name);
    }

@@ -124,6 +125,84 @@ public sealed class RawTreeServiceRenameWarningTests
        result.Warnings.ShouldContain(w => w.Contains("script") && w.Contains(ScriptName));
    }

+    [Fact]
+    public async Task Duplicate_old_paths_are_deduplicated_before_the_scan()
+    {
+        // Two tags share a name under the same device, so both resolve to the SAME old RawPath — the scan
+        // must not carry the duplicate into its predicate, and the warning must still name the script once.
+        var (service, dbName) = Seeded(db =>
+        {
+            db.Tags.Add(new Tag
+            {
+                TagId = "TAG-flow-dup",
+                DeviceId = DevAllId,
+                TagGroupId = null,
+                Name = "flow",
+                DataType = "Float",
+                AccessLevel = TagAccessLevel.Read,
+                TagConfig = "{}",
+            });
+            db.SaveChanges();
+        });
+        var rv = DeviceRowVersion(dbName, DevAllId);
+
+        var result = await service.RenameDeviceAsync(DevAllId, "Dev-renamed", rv);
+
+        result.Ok.ShouldBeTrue();
+        var scriptWarnings = result.Warnings.Where(w => w.Contains("script")).ToList();
+        scriptWarnings.Count.ShouldBe(1);
+        scriptWarnings[0].ShouldBe(
+            $"1 script ('{ScriptName}') references a tag beneath this node by its raw path; " +
+            "the reference will break unless the script is updated.");
+    }
+
+    [Fact]
+    public async Task A_rename_with_more_paths_than_one_batch_still_finds_matches_in_every_batch()
+    {
+        // 401 affected tags → three scan batches at the 200-term cap. One script names a path in the first
+        // batch, one in the middle, one in the last: a loop that drops any batch loses a name.
+        const int bulkTags = 401;
+        var (service, dbName) = Seeded(db =>
+        {
+            db.Devices.Add(new Device { DeviceId = "DEV-bulk", DriverInstanceId = DriverId, Name = "Dev-bulk", DeviceConfig = "{}" });
+            for (var i = 0; i < bulkTags; i++)
+            {
+                db.Tags.Add(new Tag
+                {
+                    TagId = $"TAG-bulk-{i:D3}",
+                    DeviceId = "DEV-bulk",
+                    TagGroupId = null,
+                    Name = $"bulk-{i:D3}",
+                    DataType = "Float",
+                    AccessLevel = TagAccessLevel.Read,
+                    TagConfig = "{}",
+                });
+            }
+
+            foreach (var (scriptName, index) in new[] { ("S-first", 0), ("S-middle", 200), ("S-last", bulkTags - 1) })
+            {
+                db.Scripts.Add(new Script
+                {
+                    ScriptId = $"SC-{scriptName}",
+                    Name = scriptName,
+                    SourceCode = $"var v = ctx.GetTag(\"Plant/modbus-1/Dev-bulk/bulk-{index:D3}\"); return v;",
+                    SourceHash = $"hash-{scriptName}",
+                });
+            }
+
+            db.SaveChanges();
+        });
+        var rv = DeviceRowVersion(dbName, "DEV-bulk");
+
+        var result = await service.RenameDeviceAsync("DEV-bulk", "Dev-bulk2", rv);
+
+        result.Ok.ShouldBeTrue();
+        // Ordinal sort: 'S-first' < 'S-last' < 'S-middle'.
+        result.Warnings.ShouldContain(
+            "3 scripts ('S-first', 'S-last', 'S-middle') reference tags beneath this node by their raw " +
+            "paths; the references will break unless the scripts are updated.");
+    }
+
    // ------------------------------------------------------------------------------------------ Seed

    private static void Seed(OtOpcUaConfigDbContext db)