diff --git a/src/ZB.MOM.WW.OtOpcUa.Admin/Services/EquipmentImportBatchService.cs b/src/ZB.MOM.WW.OtOpcUa.Admin/Services/EquipmentImportBatchService.cs index d5bb7dd..eb2c9ba 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Admin/Services/EquipmentImportBatchService.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Admin/Services/EquipmentImportBatchService.cs @@ -2,6 +2,7 @@ using Microsoft.EntityFrameworkCore; using ZB.MOM.WW.OtOpcUa.Admin.Services; using ZB.MOM.WW.OtOpcUa.Configuration; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; +using ZB.MOM.WW.OtOpcUa.Configuration.Enums; namespace ZB.MOM.WW.OtOpcUa.Admin.Services; @@ -152,14 +153,37 @@ public sealed class EquipmentImportBatchService(OtOpcUaConfigDbContext db) try { - foreach (var row in batch.Rows.Where(r => r.IsAccepted)) + // Snapshot active reservations that overlap this batch's ZTag + SAPID set — one + // round-trip instead of N. Released rows (ReleasedAt IS NOT NULL) are ignored so + // an explicitly-released value can be reused. + var accepted = batch.Rows.Where(r => r.IsAccepted).ToList(); + var zTags = accepted.Where(r => !string.IsNullOrWhiteSpace(r.ZTag)) + .Select(r => r.ZTag).Distinct(StringComparer.OrdinalIgnoreCase).ToList(); + var sapIds = accepted.Where(r => !string.IsNullOrWhiteSpace(r.SAPID)) + .Select(r => r.SAPID).Distinct(StringComparer.OrdinalIgnoreCase).ToList(); + + var existingReservations = await db.ExternalIdReservations + .Where(r => r.ReleasedAt == null && + ((r.Kind == ReservationKind.ZTag && zTags.Contains(r.Value)) || + (r.Kind == ReservationKind.SAPID && sapIds.Contains(r.Value)))) + .ToListAsync(ct).ConfigureAwait(false); + var resByKey = existingReservations.ToDictionary( + r => (r.Kind, r.Value.ToLowerInvariant()), + r => r); + + var nowUtc = DateTime.UtcNow; + var firstPublishedBy = batch.CreatedBy; + + foreach (var row in accepted) { + var equipmentUuid = Guid.TryParse(row.EquipmentUuid, out var u) ? u : Guid.NewGuid(); + db.Equipment.Add(new Equipment { EquipmentRowId = Guid.NewGuid(), GenerationId = generationId, EquipmentId = row.EquipmentId, - EquipmentUuid = Guid.TryParse(row.EquipmentUuid, out var u) ? u : Guid.NewGuid(), + EquipmentUuid = equipmentUuid, DriverInstanceId = driverInstanceIdForRows, UnsLineId = unsLineIdForRows, Name = row.Name, @@ -176,10 +200,25 @@ public sealed class EquipmentImportBatchService(OtOpcUaConfigDbContext db) ManufacturerUri = row.ManufacturerUri, DeviceManualUri = row.DeviceManualUri, }); + + MergeReservation(row.ZTag, ReservationKind.ZTag, equipmentUuid, batch.ClusterId, + firstPublishedBy, nowUtc, resByKey); + MergeReservation(row.SAPID, ReservationKind.SAPID, equipmentUuid, batch.ClusterId, + firstPublishedBy, nowUtc, resByKey); } - batch.FinalisedAtUtc = DateTime.UtcNow; - await db.SaveChangesAsync(ct).ConfigureAwait(false); + batch.FinalisedAtUtc = nowUtc; + try + { + await db.SaveChangesAsync(ct).ConfigureAwait(false); + } + catch (DbUpdateException ex) when (IsReservationUniquenessViolation(ex)) + { + throw new ExternalIdReservationConflictException( + "Finalise rejected: one or more ZTag/SAPID values were reserved by another operator " + + "between batch preview and commit. Inspect active reservations + retry after resolving the conflict.", + ex); + } if (tx is not null) await tx.CommitAsync(ct).ConfigureAwait(false); } catch @@ -193,6 +232,71 @@ public sealed class EquipmentImportBatchService(OtOpcUaConfigDbContext db) } } + /// + /// Merge one external-ID reservation for an equipment row. Three outcomes: + /// (1) value is empty → skip; (2) reservation exists for same + /// → bump LastPublishedAt; (3) reservation exists for a different EquipmentUuid + /// → throw with the conflicting UUID + /// so the caller sees which equipment already owns the value; (4) no reservation → create new. + /// + private void MergeReservation( + string? value, + ReservationKind kind, + Guid equipmentUuid, + string clusterId, + string firstPublishedBy, + DateTime nowUtc, + Dictionary<(ReservationKind, string), ExternalIdReservation> cache) + { + if (string.IsNullOrWhiteSpace(value)) return; + + var key = (kind, value.ToLowerInvariant()); + if (cache.TryGetValue(key, out var existing)) + { + if (existing.EquipmentUuid != equipmentUuid) + throw new ExternalIdReservationConflictException( + $"{kind} '{value}' is already reserved by EquipmentUuid {existing.EquipmentUuid} " + + $"(first published {existing.FirstPublishedAt:u} on cluster '{existing.ClusterId}'). " + + $"Refusing to re-assign to {equipmentUuid}."); + + existing.LastPublishedAt = nowUtc; + return; + } + + var fresh = new ExternalIdReservation + { + ReservationId = Guid.NewGuid(), + Kind = kind, + Value = value, + EquipmentUuid = equipmentUuid, + ClusterId = clusterId, + FirstPublishedAt = nowUtc, + FirstPublishedBy = firstPublishedBy, + LastPublishedAt = nowUtc, + }; + db.ExternalIdReservations.Add(fresh); + cache[key] = fresh; + } + + /// + /// True when the root-cause was the filtered-unique + /// index UX_ExternalIdReservation_KindValue_Active — i.e. another transaction + /// won the race between our cache-load + commit. SQL Server surfaces this as 2601 / 2627. + /// + private static bool IsReservationUniquenessViolation(DbUpdateException ex) + { + for (Exception? inner = ex; inner is not null; inner = inner.InnerException) + { + if (inner is Microsoft.Data.SqlClient.SqlException sql && + (sql.Number == 2601 || sql.Number == 2627) && + sql.Message.Contains("UX_ExternalIdReservation_KindValue_Active", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + return false; + } + /// List batches created by the given user. Finalised batches are archived; include them on demand. public async Task> ListByUserAsync(string createdBy, bool includeFinalised, CancellationToken ct) { @@ -205,3 +309,16 @@ public sealed class EquipmentImportBatchService(OtOpcUaConfigDbContext db) public sealed class ImportBatchNotFoundException(string message) : Exception(message); public sealed class ImportBatchAlreadyFinalisedException(string message) : Exception(message); + +/// +/// Thrown when a FinaliseBatchAsync call detects that one of its ZTag/SAPID values is +/// already reserved by a different EquipmentUuid — either from a prior published generation +/// or a concurrent finalise that won the race. The operator sees the message + the conflicting +/// equipment ownership so they can resolve the conflict (pick a new ZTag, release the existing +/// reservation via sp_ReleaseExternalIdReservation, etc.) and retry the finalise. +/// +public sealed class ExternalIdReservationConflictException : Exception +{ + public ExternalIdReservationConflictException(string message) : base(message) { } + public ExternalIdReservationConflictException(string message, Exception inner) : base(message, inner) { } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/EquipmentImportBatchServiceTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/EquipmentImportBatchServiceTests.cs index efcee02..d28db99 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/EquipmentImportBatchServiceTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Admin.Tests/EquipmentImportBatchServiceTests.cs @@ -23,11 +23,13 @@ public sealed class EquipmentImportBatchServiceTests : IDisposable public void Dispose() => _db.Dispose(); + // Unique SAPID per row — FinaliseBatch reserves ZTag + SAPID via filtered-unique index, so + // two rows sharing a SAPID under different EquipmentUuids collide as intended. private static EquipmentCsvRow Row(string zTag, string name = "eq-1") => new() { ZTag = zTag, MachineCode = "mc", - SAPID = "sap", + SAPID = $"sap-{zTag}", EquipmentId = "eq-id", EquipmentUuid = Guid.NewGuid().ToString(), Name = name, @@ -162,4 +164,93 @@ public sealed class EquipmentImportBatchServiceTests : IDisposable await _svc.DropBatchAsync(Guid.NewGuid(), CancellationToken.None); // no throw } + + [Fact] + public async Task FinaliseBatch_Creates_ExternalIdReservations_ForZTagAndSAPID() + { + var batch = await _svc.CreateBatchAsync("c1", "alice", CancellationToken.None); + await _svc.StageRowsAsync(batch.Id, [Row("z-new-1")], [], CancellationToken.None); + + await _svc.FinaliseBatchAsync(batch.Id, 1, "drv", "line", CancellationToken.None); + + var active = await _db.ExternalIdReservations.AsNoTracking() + .Where(r => r.ReleasedAt == null) + .ToListAsync(); + active.Count.ShouldBe(2); + active.ShouldContain(r => r.Kind == ZB.MOM.WW.OtOpcUa.Configuration.Enums.ReservationKind.ZTag && r.Value == "z-new-1"); + active.ShouldContain(r => r.Kind == ZB.MOM.WW.OtOpcUa.Configuration.Enums.ReservationKind.SAPID && r.Value == "sap-z-new-1"); + } + + [Fact] + public async Task FinaliseBatch_SameEquipmentUuid_ReusesExistingReservation() + { + var batch1 = await _svc.CreateBatchAsync("c1", "alice", CancellationToken.None); + var sharedUuid = Guid.NewGuid(); + var row = new EquipmentCsvRow + { + ZTag = "z-shared", MachineCode = "mc", SAPID = "sap-shared", + EquipmentId = "eq-1", EquipmentUuid = sharedUuid.ToString(), + Name = "eq-1", UnsAreaName = "a", UnsLineName = "l", + }; + await _svc.StageRowsAsync(batch1.Id, [row], [], CancellationToken.None); + await _svc.FinaliseBatchAsync(batch1.Id, 1, "drv", "line", CancellationToken.None); + + var countAfterFirst = _db.ExternalIdReservations.Count(r => r.ReleasedAt == null); + + // Second finalise with same EquipmentUuid + same ZTag — should NOT create a duplicate. + var batch2 = await _svc.CreateBatchAsync("c1", "alice", CancellationToken.None); + await _svc.StageRowsAsync(batch2.Id, [row], [], CancellationToken.None); + await _svc.FinaliseBatchAsync(batch2.Id, 2, "drv", "line", CancellationToken.None); + + _db.ExternalIdReservations.Count(r => r.ReleasedAt == null).ShouldBe(countAfterFirst); + } + + [Fact] + public async Task FinaliseBatch_DifferentEquipmentUuid_SameZTag_Throws_Conflict() + { + var batchA = await _svc.CreateBatchAsync("c1", "alice", CancellationToken.None); + var rowA = new EquipmentCsvRow + { + ZTag = "z-collide", MachineCode = "mc-a", SAPID = "sap-a", + EquipmentId = "eq-a", EquipmentUuid = Guid.NewGuid().ToString(), + Name = "a", UnsAreaName = "ar", UnsLineName = "ln", + }; + await _svc.StageRowsAsync(batchA.Id, [rowA], [], CancellationToken.None); + await _svc.FinaliseBatchAsync(batchA.Id, 1, "drv", "line", CancellationToken.None); + + var batchB = await _svc.CreateBatchAsync("c1", "bob", CancellationToken.None); + var rowB = new EquipmentCsvRow + { + ZTag = "z-collide", MachineCode = "mc-b", SAPID = "sap-b", // same ZTag, different EquipmentUuid + EquipmentId = "eq-b", EquipmentUuid = Guid.NewGuid().ToString(), + Name = "b", UnsAreaName = "ar", UnsLineName = "ln", + }; + await _svc.StageRowsAsync(batchB.Id, [rowB], [], CancellationToken.None); + + var ex = await Should.ThrowAsync(() => + _svc.FinaliseBatchAsync(batchB.Id, 2, "drv", "line", CancellationToken.None)); + ex.Message.ShouldContain("z-collide"); + + // Second finalise must have rolled back — no partial Equipment row for batch B. + var equipmentB = await _db.Equipment.AsNoTracking() + .Where(e => e.EquipmentId == "eq-b") + .ToListAsync(); + equipmentB.ShouldBeEmpty(); + } + + [Fact] + public async Task FinaliseBatch_EmptyZTagAndSAPID_SkipsReservation() + { + var batch = await _svc.CreateBatchAsync("c1", "alice", CancellationToken.None); + var row = new EquipmentCsvRow + { + ZTag = "", MachineCode = "mc", SAPID = "", + EquipmentId = "eq-nil", EquipmentUuid = Guid.NewGuid().ToString(), + Name = "nil", UnsAreaName = "ar", UnsLineName = "ln", + }; + await _svc.StageRowsAsync(batch.Id, [row], [], CancellationToken.None); + await _svc.FinaliseBatchAsync(batch.Id, 1, "drv", "line", CancellationToken.None); + + _db.ExternalIdReservations.Count().ShouldBe(0); + } }