fix(deployment): instance delete fully removes the record

Deleting an instance only undeployed it from the site and set the state
to NotDeployed, leaving an orphan record that could never be removed —
the state-transition matrix rejected delete from NotDeployed.

Delete now removes the instance record entirely (deployment history,
snapshot, attribute/alarm overrides, and connection bindings go with
it), and is permitted from any state.
This commit is contained in:
Joseph Doherty
2026-05-15 12:05:13 -04:00
parent 17e24ddd20
commit 1d5465f31c
6 changed files with 45 additions and 17 deletions

View File

@@ -34,5 +34,12 @@ public interface IDeploymentManagerRepository
Task<Instance?> GetInstanceByUniqueNameAsync(string uniqueName, CancellationToken cancellationToken = default); Task<Instance?> GetInstanceByUniqueNameAsync(string uniqueName, CancellationToken cancellationToken = default);
Task UpdateInstanceAsync(Instance instance, CancellationToken cancellationToken = default); Task UpdateInstanceAsync(Instance instance, CancellationToken cancellationToken = default);
/// <summary>
/// Removes an instance and everything that depends on it: deployment
/// records, deployed config snapshot, attribute/alarm overrides, and
/// connection bindings.
/// </summary>
Task DeleteInstanceAsync(int instanceId, CancellationToken cancellationToken = default);
Task<int> SaveChangesAsync(CancellationToken cancellationToken = default); Task<int> SaveChangesAsync(CancellationToken cancellationToken = default);
} }

View File

@@ -189,6 +189,27 @@ public class DeploymentManagerRepository : IDeploymentManagerRepository
return Task.CompletedTask; return Task.CompletedTask;
} }
public async Task DeleteInstanceAsync(int instanceId, CancellationToken cancellationToken = default)
{
// DeploymentRecords have a Restrict FK to Instance — remove them
// explicitly first. The snapshot, overrides, and connection bindings
// are configured with cascade delete and go with the instance.
var records = await _dbContext.DeploymentRecords
.Where(d => d.InstanceId == instanceId)
.ToListAsync(cancellationToken);
if (records.Count > 0)
{
_dbContext.DeploymentRecords.RemoveRange(records);
}
var instance = await _dbContext.Set<Instance>()
.FirstOrDefaultAsync(i => i.Id == instanceId, cancellationToken);
if (instance != null)
{
_dbContext.Set<Instance>().Remove(instance);
}
}
public async Task<int> SaveChangesAsync(CancellationToken cancellationToken = default) public async Task<int> SaveChangesAsync(CancellationToken cancellationToken = default)
{ {
return await _dbContext.SaveChangesAsync(cancellationToken); return await _dbContext.SaveChangesAsync(cancellationToken);

View File

@@ -282,7 +282,9 @@ public class DeploymentService
} }
/// <summary> /// <summary>
/// WP-6: Delete an instance. Stops actor, removes config. S&amp;F NOT cleared. /// WP-6: Delete an instance. Stops the site actor, removes site config, and
/// removes the central instance record (deployment history, snapshot,
/// overrides, and connection bindings go with it). S&amp;F NOT cleared.
/// Delete fails if site unreachable (30s timeout via CommunicationOptions). /// Delete fails if site unreachable (30s timeout via CommunicationOptions).
/// </summary> /// </summary>
public async Task<Result<InstanceLifecycleResponse>> DeleteInstanceAsync( public async Task<Result<InstanceLifecycleResponse>> DeleteInstanceAsync(
@@ -309,12 +311,10 @@ public class DeploymentService
if (response.Success) if (response.Success)
{ {
// Remove deployed snapshot // Delete means delete: remove the instance record entirely.
await _repository.DeleteDeployedSnapshotAsync(instanceId, cancellationToken); // Deployment records, snapshot, overrides, and connection bindings
// are removed with it (see repository implementation).
// Set state to NotDeployed (or the instance record could be deleted entirely by higher layers) await _repository.DeleteInstanceAsync(instanceId, cancellationToken);
instance.State = InstanceState.NotDeployed;
await _repository.UpdateInstanceAsync(instance, cancellationToken);
await _repository.SaveChangesAsync(cancellationToken); await _repository.SaveChangesAsync(cancellationToken);
} }

View File

@@ -7,11 +7,12 @@ namespace ScadaLink.DeploymentManager;
/// ///
/// State | Deploy | Disable | Enable | Delete /// State | Deploy | Disable | Enable | Delete
/// ----------|--------|---------|--------|------- /// ----------|--------|---------|--------|-------
/// NotDeploy | OK | NO | NO | NO /// NotDeploy | OK | NO | NO | OK
/// Enabled | OK | OK | NO | OK /// Enabled | OK | OK | NO | OK
/// Disabled | OK* | NO | OK | OK /// Disabled | OK* | NO | OK | OK
/// ///
/// * Deploy on a Disabled instance also enables it. /// * Deploy on a Disabled instance also enables it.
/// Delete removes the instance record entirely; it is valid from any state.
/// </summary> /// </summary>
public static class StateTransitionValidator public static class StateTransitionValidator
{ {
@@ -25,7 +26,7 @@ public static class StateTransitionValidator
currentState == InstanceState.Disabled; currentState == InstanceState.Disabled;
public static bool CanDelete(InstanceState currentState) => public static bool CanDelete(InstanceState currentState) =>
currentState is InstanceState.Enabled or InstanceState.Disabled; currentState is InstanceState.NotDeployed or InstanceState.Enabled or InstanceState.Disabled;
/// <summary> /// <summary>
/// Returns a human-readable error message if the transition is invalid, or null if valid. /// Returns a human-readable error message if the transition is invalid, or null if valid.

View File

@@ -188,15 +188,14 @@ public class DeploymentServiceTests
} }
[Fact] [Fact]
public async Task DeleteInstanceAsync_WhenNotDeployed_ReturnsTransitionError() public async Task DeleteInstanceAsync_InstanceNotFound_ReturnsFailure()
{ {
var instance = new Instance("TestInst") { Id = 1, SiteId = 1, State = InstanceState.NotDeployed }; _repo.GetInstanceByIdAsync(1).Returns((Instance?)null);
_repo.GetInstanceByIdAsync(1).Returns(instance);
var result = await _service.DeleteInstanceAsync(1, "admin"); var result = await _service.DeleteInstanceAsync(1, "admin");
Assert.True(result.IsFailure); Assert.True(result.IsFailure);
Assert.Contains("not allowed", result.Error); Assert.Contains("not found", result.Error);
} }
// ── WP-8: Deployment comparison ── // ── WP-8: Deployment comparison ──

View File

@@ -73,9 +73,9 @@ public class StateTransitionValidatorTests
} }
[Fact] [Fact]
public void CanDelete_WhenNotDeployed_ReturnsFalse() public void CanDelete_WhenNotDeployed_ReturnsTrue()
{ {
Assert.False(StateTransitionValidator.CanDelete(InstanceState.NotDeployed)); Assert.True(StateTransitionValidator.CanDelete(InstanceState.NotDeployed));
} }
// ── ValidateTransition ── // ── ValidateTransition ──
@@ -103,10 +103,10 @@ public class StateTransitionValidatorTests
} }
[Fact] [Fact]
public void ValidateTransition_InvalidDeleteOnNotDeployed_ReturnsError() public void ValidateTransition_ValidDeleteOnNotDeployed_ReturnsNull()
{ {
var error = StateTransitionValidator.ValidateTransition(InstanceState.NotDeployed, "delete"); var error = StateTransitionValidator.ValidateTransition(InstanceState.NotDeployed, "delete");
Assert.NotNull(error); Assert.Null(error);
} }
[Fact] [Fact]