fix(deployment): drop dead ITemplateGraphWatermark ctor param and no-op ClearMemos from StaleInstanceProbe (review 2026-10-21 04/U2)

This commit is contained in:
Joseph Doherty
2026-10-22 06:35:02 -04:00
parent 7975699ac4
commit 971a9886b8
4 changed files with 24 additions and 36 deletions
@@ -1,21 +1,19 @@
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Transport;
namespace ZB.MOM.WW.ScadaBridge.DeploymentManager; namespace ZB.MOM.WW.ScadaBridge.DeploymentManager;
/// <summary>Recomputes deployment revisions from authoritative configuration on every probe. /// <summary>Recomputes deployment revisions from authoritative configuration on every probe.
/// Process-local watermarks cannot prove freshness after another central changes the shared store.</summary> /// Process-local watermarks cannot prove freshness after another central changes the shared store,
/// so nothing is memoised across calls.</summary>
public sealed class StaleInstanceProbe : IStaleInstanceProbe public sealed class StaleInstanceProbe : IStaleInstanceProbe
{ {
private readonly IFlatteningPipeline _flatteningPipeline; private readonly IFlatteningPipeline _flatteningPipeline;
/// <summary>Initializes the authoritative probe. Watermark remains a constructor compatibility dependency.</summary> /// <summary>Initializes the authoritative probe.</summary>
/// <param name="flatteningPipeline">The pipeline used to flatten instances for staleness comparison.</param> /// <param name="flatteningPipeline">The pipeline used to flatten instances for staleness comparison.</param>
/// <param name="watermark">The template graph watermark; retained only for constructor compatibility.</param> public StaleInstanceProbe(IFlatteningPipeline flatteningPipeline)
public StaleInstanceProbe(IFlatteningPipeline flatteningPipeline, ITemplateGraphWatermark watermark)
{ {
_flatteningPipeline = flatteningPipeline ?? throw new ArgumentNullException(nameof(flatteningPipeline)); _flatteningPipeline = flatteningPipeline ?? throw new ArgumentNullException(nameof(flatteningPipeline));
ArgumentNullException.ThrowIfNull(watermark);
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -26,7 +24,4 @@ public sealed class StaleInstanceProbe : IStaleInstanceProbe
validateScripts: false, session).ConfigureAwait(false); validateScripts: false, session).ConfigureAwait(false);
return result.IsSuccess ? result.Value.RevisionHash : null; return result.IsSuccess ? result.Value.RevisionHash : null;
} }
/// <summary>Compatibility hook: probes no longer retain process-global results.</summary>
public static void ClearMemos() { }
} }
@@ -717,16 +717,12 @@ public class DeploymentServiceTests : TestKit
public async Task StaleInstanceProbe_RequestsValidateScriptsFalse() public async Task StaleInstanceProbe_RequestsValidateScriptsFalse()
{ {
// The Transport stale-instance probe also only needs the hash — same skip. // The Transport stale-instance probe also only needs the hash — same skip.
// The probe memoises per-instance against the graph watermark, so start
// from a clean memo or a sibling test's entry would satisfy the probe
// without a flatten at all.
StaleInstanceProbe.ClearMemos();
_pipeline.CreateSession().Returns(_ => new FlattenSession(new TemplateGraphWatermark())); _pipeline.CreateSession().Returns(_ => new FlattenSession(new TemplateGraphWatermark()));
_pipeline.FlattenAndValidateAsync(5, Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<FlattenSession?>()) _pipeline.FlattenAndValidateAsync(5, Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<FlattenSession?>())
.Returns(Result<FlatteningPipelineResult>.Success( .Returns(Result<FlatteningPipelineResult>.Success(
new FlatteningPipelineResult(new FlattenedConfiguration(), "sha256:probe", ValidationResult.Success()))); new FlatteningPipelineResult(new FlattenedConfiguration(), "sha256:probe", ValidationResult.Success())));
var probe = new StaleInstanceProbe(_pipeline, new TemplateGraphWatermark()); var probe = new StaleInstanceProbe(_pipeline);
var hash = await probe.GetCurrentRevisionHashAsync(5); var hash = await probe.GetCurrentRevisionHashAsync(5);
@@ -8,20 +8,11 @@ using ZB.MOM.WW.ScadaBridge.TemplateEngine.Flattening;
namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests; namespace ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests;
/// <summary> /// <summary>
/// The staleness fast path: <see cref="StaleInstanceProbe"/> memoises a computed /// <see cref="StaleInstanceProbe"/> is authoritative: it re-flattens on EVERY probe and
/// revision hash against the <see cref="ITemplateGraphWatermark"/> readings it was /// memoises nothing across calls. It used to memoise a revision hash against the
/// computed under, and the memo is PROCESS-STATIC. That makes the watermark the /// process-local <see cref="ITemplateGraphWatermark"/>, which a peer central node writing
/// only thing standing between a cached hash and a wrong answer — so these tests /// the shared database could never bump (MG-3) — these tests pin that every probe sees
/// pin that the memo is served only while the watermark has not moved, and is /// the current configuration whatever the local watermark says.
/// dropped the moment it has.
///
/// <para>
/// The failure this guards against is a real one: a data-connection edit, or a
/// bundle import committing through the raw <c>DbContext</c>, changes the flattened
/// output without going through the repository path that derives bumps from the
/// change tracker. If that write does not bump, the memo below keeps answering with
/// the old hash forever.
/// </para>
/// </summary> /// </summary>
public class StaleInstanceProbeTests public class StaleInstanceProbeTests
{ {
@@ -35,9 +26,6 @@ public class StaleInstanceProbeTests
/// <summary>Initializes a new instance of the <c>StaleInstanceProbeTests</c> class.</summary> /// <summary>Initializes a new instance of the <c>StaleInstanceProbeTests</c> class.</summary>
public StaleInstanceProbeTests() public StaleInstanceProbeTests()
{ {
// Process-static memo: start every test from a clean slate.
StaleInstanceProbe.ClearMemos();
_pipeline.CreateSession().Returns(_ => new FlattenSession(_watermark)); _pipeline.CreateSession().Returns(_ => new FlattenSession(_watermark));
_pipeline _pipeline
.FlattenAndValidateAsync(InstanceId, Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<FlattenSession?>()) .FlattenAndValidateAsync(InstanceId, Arg.Any<CancellationToken>(), Arg.Any<bool>(), Arg.Any<FlattenSession?>())
@@ -50,7 +38,20 @@ public class StaleInstanceProbeTests
}); });
} }
private StaleInstanceProbe CreateProbe() => new(_pipeline, _watermark); private StaleInstanceProbe CreateProbe() => new(_pipeline);
/// <summary>
/// review 2026-10-21 04/U2: with the memo gone the probe has no use for the watermark, so
/// its only dependency is the pipeline and the no-op <c>ClearMemos</c> hook is gone.
/// </summary>
[Fact]
public void Probe_DependsOnlyOnThePipeline_AndHasNoMemoHook()
{
var ctor = Assert.Single(typeof(StaleInstanceProbe).GetConstructors());
var parameter = Assert.Single(ctor.GetParameters());
Assert.Equal(typeof(IFlatteningPipeline), parameter.ParameterType);
Assert.Null(typeof(StaleInstanceProbe).GetMethod("ClearMemos"));
}
/// <summary>Verifies that peer edit with independent watermark is observed by warm active probe.</summary> /// <summary>Verifies that peer edit with independent watermark is observed by warm active probe.</summary>
/// <returns>A task that represents the asynchronous operation.</returns> /// <returns>A task that represents the asynchronous operation.</returns>
@@ -1595,10 +1595,6 @@ public sealed class BundleImporterApplyTests : IDisposable
[Fact] [Fact]
public async Task ApplyAsync_second_overwrite_of_same_template_still_marks_instance_stale() public async Task ApplyAsync_second_overwrite_of_same_template_still_marks_instance_stale()
{ {
// The memo is process-static and keyed by instance id alone; start clean so
// this test does not inherit another class's entry.
StaleInstanceProbe.ClearMemos();
await SeedSiteAsync(); await SeedSiteAsync();
// Shape A → bundle A. // Shape A → bundle A.