From 1d00d5f469e08c29ec1ee2766ab26992ae39fbd0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:32:40 -0400 Subject: [PATCH] fix(configdb): deterministic config-audit paging (review 2026-10-21 08/P5) --- .../Repositories/CentralUiRepository.cs | 27 +------- .../RepositoryTests.cs | 65 ++++--------------- 2 files changed, 17 insertions(+), 75 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/CentralUiRepository.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/CentralUiRepository.cs index 2ddff48c..3bdee77b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/CentralUiRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/CentralUiRepository.cs @@ -49,20 +49,6 @@ public class CentralUiRepository : ICentralUiRepository .ToListAsync(cancellationToken); } - /// - public async Task> GetTemplateTreeAsync(CancellationToken cancellationToken = default) - { - return await _context.Templates - .AsNoTracking() - .Include(t => t.Attributes) - .Include(t => t.Alarms) - .Include(t => t.Scripts) - .Include(t => t.Compositions) - .AsSplitQuery() - .OrderBy(t => t.Name) - .ToListAsync(cancellationToken); - } - /// public async Task> GetInstancesFilteredAsync( int? siteId = null, @@ -118,16 +104,6 @@ public class CentralUiRepository : ICentralUiRepository return (rows, total); } - /// - public async Task> GetRecentDeploymentsAsync(int count, CancellationToken cancellationToken = default) - { - return await _context.DeploymentRecords - .AsNoTracking() - .OrderByDescending(d => d.DeployedAt) - .Take(count) - .ToListAsync(cancellationToken); - } - /// public async Task> GetAreaTreeBySiteIdAsync(int siteId, CancellationToken cancellationToken = default) { @@ -182,7 +158,10 @@ public class CentralUiRepository : ICentralUiRepository var totalCount = await query.CountAsync(cancellationToken); var entries = await query + // Id tie-break: a bundle import writes hundreds of rows within the same few + // milliseconds, so Timestamp alone makes offset pages repeat/drop rows. .OrderByDescending(a => a.Timestamp) + .ThenByDescending(a => a.Id) .Skip((page - 1) * pageSize) .Take(pageSize) .ToListAsync(cancellationToken); diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/RepositoryTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/RepositoryTests.cs index 44888f79..f8a6b628 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/RepositoryTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/RepositoryTests.cs @@ -66,21 +66,6 @@ public class SecurityRepositoryTests : IDisposable Assert.True(all.Count >= 2); } - /// Verifies that getting mappings by role returns only the matching mappings. - /// A task that represents the asynchronous operation. - [Fact] - public async Task GetMappingsByRole_FiltersCorrectly() - { - await _repository.AddMappingAsync(new LdapGroupMapping("Designers", "Designer")); - await _repository.AddMappingAsync(new LdapGroupMapping("Deployers", "Deployer")); - await _repository.SaveChangesAsync(); - - var designMappings = await _repository.GetMappingsByRoleAsync("Designer"); - // Seed data includes "SCADA-Designers" with role "Designer", plus the one we added - Assert.Equal(2, designMappings.Count); - Assert.Contains(designMappings, m => m.LdapGroupName == "Designers"); - } - /// Verifies that updating a mapping persists the change. /// A task that represents the asynchronous operation. [Fact] @@ -312,33 +297,6 @@ public class CentralUiRepositoryTests : IDisposable Assert.Equal(0, noneTotal); } - /// Verifies that recent deployments are returned in reverse chronological order. - /// A task that represents the asynchronous operation. - [Fact] - public async Task GetRecentDeployments_ReturnsInReverseChronological() - { - var site = new Site("Site1", "S-001"); - var template = new Template("T1"); - _context.Sites.Add(site); - _context.Templates.Add(template); - await _context.SaveChangesAsync(); - - var instance = new Instance("I1") { SiteId = site.Id, TemplateId = template.Id }; - _context.Instances.Add(instance); - await _context.SaveChangesAsync(); - - _context.DeploymentRecords.AddRange( - new DeploymentRecord("d-001", "admin") { InstanceId = instance.Id, DeployedAt = DateTimeOffset.UtcNow.AddHours(-2) }, - new DeploymentRecord("d-002", "admin") { InstanceId = instance.Id, DeployedAt = DateTimeOffset.UtcNow.AddHours(-1) }, - new DeploymentRecord("d-003", "admin") { InstanceId = instance.Id, DeployedAt = DateTimeOffset.UtcNow }); - await _context.SaveChangesAsync(); - - var recent = await _repository.GetRecentDeploymentsAsync(2); - Assert.Equal(2, recent.Count); - Assert.Equal("d-003", recent[0].DeploymentId); - Assert.Equal("d-002", recent[1].DeploymentId); - } - /// Verifies that audit log entries can be filtered by user. /// A task that represents the asynchronous operation. [Fact] @@ -477,20 +435,25 @@ public class CentralUiRepositoryTests : IDisposable Assert.Equal("T6", page2[0].EntityName); } - /// Verifies that the template tree includes child entities. + /// Entries sharing one Timestamp page deterministically (Id tie-break) with no repeats or gaps (review 08/P5). /// A task that represents the asynchronous operation. [Fact] - public async Task GetTemplateTree_IncludesChildren() + public async Task GetAuditLogEntries_SameTimestamp_PagesDeterministicallyWithoutOverlap() { - var template = new Template("TestTemplate"); - template.Attributes.Add(new TemplateAttribute("Attr1") { DataType = DataType.Int32 }); - _context.Templates.Add(template); + var ts = DateTimeOffset.UtcNow; + for (var i = 0; i < 10; i++) + _context.AuditLogEntries.Add(new AuditLogEntry("admin", "Create", "Template", i.ToString(), $"T{i}") { Timestamp = ts }); await _context.SaveChangesAsync(); - var tree = await _repository.GetTemplateTreeAsync(); - Assert.NotEmpty(tree); - var loaded = tree.First(t => t.Name == "TestTemplate"); - Assert.Single(loaded.Attributes); + var seen = new List(); + for (var page = 1; page <= 4; page++) + { + var (entries, _) = await _repository.GetAuditLogEntriesAsync(page: page, pageSize: 3); + seen.AddRange(entries.Select(e => e.Id)); + } + + Assert.Equal(10, seen.Distinct().Count()); + Assert.Equal(seen.OrderByDescending(x => x), seen); } /// Verifies that the area tree returns the area hierarchy.