fix(transport): script-artifact change publisher covers Add resolutions — delete-then-reimport no longer starves non-self-healing subscribers (plan R2-05 T5)

2026-07-13 09:50:32 -04:00
parent 31fa5eafa4
commit ab77094a26
2 changed files with 65 additions and 7 deletions
@@ -1767,12 +1767,15 @@ public sealed class BundleImporter : IBundleImporter
    /// </summary>
    /// <summary>
    /// Publishes one <see cref="ScriptArtifactsChanged"/> per script-bearing
-    /// artifact kind (ApiMethod / SharedScript / Template) whose resolution was an
-    /// Overwrite or Rename, using the post-resolution (renamed) names. Adds are
-    /// excluded — nothing is cached under a brand-new name yet. Over-approximation is
-    /// safe (an Overwrite that actually fell through to Add just triggers a harmless
-    /// consumer re-check), which matches the bus's advisory, at-least-once contract.
-    /// Fully guarded: never throws (it runs after the transaction has committed).
+    /// artifact kind (ApiMethod / SharedScript / Template) for every non-Skip
+    /// resolution — Overwrite, Rename, AND Add — using the post-resolution (renamed)
+    /// names. An Add is NOT "nothing cached yet": an artifact deleted on the target and
+    /// re-imported as Add under the same name can still have a stale
+    /// compiled-handler/<c>_knownBadMethods</c> entry on any node, so it must notify too.
+    /// Over-approximation stays safe (the bus is advisory, at-least-once; consumers
+    /// self-heal), which matches its stated contract. Only Skip resolutions are excluded
+    /// (nothing was written). Fully guarded: never throws (it runs after the transaction
+    /// has committed).
    /// </summary>
    private void PublishScriptArtifactChanges(IReadOnlyList<ImportResolution> resolutions)
    {
@@ -1799,7 +1802,7 @@ public sealed class BundleImporter : IBundleImporter
        {
            var names = all
                .Where(r => string.Equals(r.EntityType, entityType, StringComparison.Ordinal)
-                    && (r.Action is ResolutionAction.Overwrite or ResolutionAction.Rename))
+                    && r.Action is not ResolutionAction.Skip)
                .Select(r => r.Action == ResolutionAction.Rename ? r.RenameTo ?? r.Name : r.Name)
                .Distinct(StringComparer.Ordinal)
                .ToList();
@@ -1397,6 +1397,61 @@ public sealed class BundleImporterApplyTests : IDisposable
        Assert.All(_artifactBus.Received, n => Assert.Equal("BundleImport", n.Source));
    }

+    [Fact]
+    public async Task ApplyAsync_publishes_ScriptArtifactsChanged_for_Add_resolutions()
+    {
+        // ApiMethod imported as Add into a target where the name does not exist.
+        // Delete-then-reimport-as-Add is the N3 edge: a node can still hold a
+        // _knownBadMethods / compiled-handler entry under that very name, so Adds
+        // must notify too (over-approximation is safe per the bus contract).
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+            ctx.ApiMethods.Add(new ApiMethod("DelmiaRecipeDownload", "return 1;") { TimeoutSeconds = 30 });
+            await ctx.SaveChangesAsync();
+        }
+
+        Guid sessionId;
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            var exporter = scope.ServiceProvider.GetRequiredService<IBundleExporter>();
+            var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
+            var selection = new ExportSelection(
+                TemplateIds: Array.Empty<int>(),
+                SharedScriptIds: Array.Empty<int>(),
+                ExternalSystemIds: Array.Empty<int>(),
+                DatabaseConnectionIds: Array.Empty<int>(),
+                NotificationListIds: Array.Empty<int>(),
+                SmtpConfigurationIds: Array.Empty<int>(),
+                ApiMethodIds: await ctx.ApiMethods.Select(m => m.Id).ToListAsync(),
+                IncludeDependencies: false);
+            var stream = await exporter.ExportAsync(selection, user: "alice", sourceEnvironment: "dev",
+                passphrase: null, cancellationToken: CancellationToken.None);
+            using var ms = new MemoryStream();
+            await stream.CopyToAsync(ms);
+            ms.Position = 0;
+
+            // Delete the target method so the resolution is a genuine Add.
+            ctx.ApiMethods.RemoveRange(await ctx.ApiMethods.ToListAsync());
+            await ctx.SaveChangesAsync();
+
+            var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
+            sessionId = (await importer.LoadAsync(ms, passphrase: null)).SessionId;
+        }
+
+        await using (var scope = _provider.CreateAsyncScope())
+        {
+            var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
+            await importer.ApplyAsync(sessionId,
+                new List<ImportResolution> { new("ApiMethod", "DelmiaRecipeDownload", ResolutionAction.Add, null) },
+                user: "bob");
+        }
+
+        var notification = Assert.Single(_artifactBus.Received,
+            n => n.ArtifactKind == ScriptArtifactKinds.ApiMethod);
+        Assert.Contains("DelmiaRecipeDownload", notification.Names);   // FAILS today: Adds are filtered out
+    }
+
    [Fact]
    public async Task ApplyAsync_failed_apply_publishes_nothing()
    {