From 5c74e9732fb7d45fa912b75d3603325bcf3845b0 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 24 Jul 2026 13:29:16 +0800 Subject: [PATCH 1/2] move all ravendb specific ID generation into the ravenDB persistence layer --- .../Editing/NotificationsManager.cs | 5 ++-- .../EndpointSettingsStore.cs | 6 +++-- .../ErrorMessagesDataStore.cs | 14 +++++++---- .../MessageRedirectsDataStore.cs | 8 +++---- .../RavenMonitoringDataStore.cs | 4 +++- .../Recoverability/GroupsDataStore.cs | 8 ++++--- .../Recoverability/RetryHistoryDataStore.cs | 10 ++++---- .../RetryBatchesDataStore.cs | 2 +- .../RetryBatchesManager.cs | 7 +++--- .../RetryDocumentDataStore.cs | 11 +++++++-- .../TrialLicenseDataProvider.cs | 6 +++-- .../RavenMonitoringIngestionUnitOfWork.cs | 2 +- .../RavenRecoverabilityIngestionUnitOfWork.cs | 2 +- .../FailedErrorImportCustomCheckTests.cs | 2 +- .../Recoverability/FailedMessageRetryTests.cs | 5 ++-- .../EndpointSettings.cs | 2 -- .../EventLog/EventLogItem.cs | 5 +++- .../FailedAuditImport.cs | 10 -------- .../FailedErrorImport.cs | 2 -- .../FailedMessage.cs | 5 ---- .../FailedMessageRetry.cs | 7 ------ .../TransportMessageExtensions.cs | 10 ++------ .../KnownEndpoint.cs | 2 -- .../MessageRedirectsCollection.cs | 2 -- .../NotificationsSettings.cs | 2 -- .../Recoverability/ReclassifyErrorSettings.cs | 15 ------------ src/ServiceControl.Persistence/RetryBatch.cs | 2 -- .../RetryBatchNowForwarding.cs | 1 - .../RetryHistory.cs | 19 ++------------- .../TrialMetadata.cs | 2 -- .../UnitOfWorkServiceCollectionExtensions.cs | 23 +------------------ .../Operations/ErrorIngestionFaultPolicy.cs | 4 ++-- .../Recoverability/Retrying/RetriesGateway.cs | 2 +- 33 files changed, 70 insertions(+), 137 deletions(-) delete mode 100644 src/ServiceControl.Persistence/FailedAuditImport.cs delete mode 100644 src/ServiceControl.Persistence/Recoverability/ReclassifyErrorSettings.cs diff --git a/src/ServiceControl.Persistence.RavenDB/Editing/NotificationsManager.cs b/src/ServiceControl.Persistence.RavenDB/Editing/NotificationsManager.cs index bca77a18e3..52e41d6a44 100644 --- a/src/ServiceControl.Persistence.RavenDB/Editing/NotificationsManager.cs +++ b/src/ServiceControl.Persistence.RavenDB/Editing/NotificationsManager.cs @@ -7,13 +7,14 @@ class NotificationsManager(IAsyncDocumentSession session) : AbstractSessionManager(session), INotificationsManager { + const string SingleDocumentId = "NotificationsSettings/All"; static readonly TimeSpan CacheTimeoutDefault = TimeSpan.FromMinutes(5); // Raven requires this to be at least 1 second public async Task LoadSettings(TimeSpan? cacheTimeout = null) { using var aggressivelyCacheFor = await Session.Advanced.DocumentStore.AggressivelyCacheForAsync(cacheTimeout ?? CacheTimeoutDefault); var settings = await Session - .LoadAsync(NotificationsSettings.SingleDocumentId); + .LoadAsync(SingleDocumentId); if (settings != null) { @@ -22,7 +23,7 @@ public async Task LoadSettings(TimeSpan? cacheTimeout = n settings = new NotificationsSettings { - Id = NotificationsSettings.SingleDocumentId + Id = SingleDocumentId }; await Session.StoreAsync(settings); diff --git a/src/ServiceControl.Persistence.RavenDB/EndpointSettingsStore.cs b/src/ServiceControl.Persistence.RavenDB/EndpointSettingsStore.cs index 463a31efd0..455736495a 100644 --- a/src/ServiceControl.Persistence.RavenDB/EndpointSettingsStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/EndpointSettingsStore.cs @@ -11,14 +11,14 @@ class EndpointSettingsStore(IRavenSessionProvider sessionProvider) : IEndpointSettingsStore { static string MakeDocumentId(string name) => - $"{EndpointSettings.CollectionName}/{DeterministicGuid.MakeId(name)}"; + $"{CollectionName}/{DeterministicGuid.MakeId(name)}"; public async IAsyncEnumerable GetAllEndpointSettings([EnumeratorCancellation] CancellationToken cancellationToken) { using IAsyncDocumentSession session = await sessionProvider.OpenSession(cancellationToken: cancellationToken); await using IAsyncEnumerator> enumerator = await session .Advanced - .StreamAsync($"{EndpointSettings.CollectionName}/", token: cancellationToken); + .StreamAsync($"{CollectionName}/", token: cancellationToken); while (await enumerator.MoveNextAsync() && !cancellationToken.IsCancellationRequested) { @@ -46,4 +46,6 @@ public async Task UpdateEndpointSettings(EndpointSettings settings, Cancellation await session.SaveChangesAsync(cancellationToken); } + + const string CollectionName = "EndpointSettings"; } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs b/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs index df59b8fdf9..84d262936c 100644 --- a/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs @@ -15,6 +15,7 @@ using Raven.Client.Documents.Queries; using Raven.Client.Documents.Queries.Facets; using Raven.Client.Documents.Session; + using Recoverability; using ServiceControl.CompositeViews.Messages; using ServiceControl.EventLog; using ServiceControl.MessageFailures; @@ -172,6 +173,7 @@ public async Task FailedMessagesFetch(Guid[] ids) public async Task StoreFailedErrorImport(FailedErrorImport failure) { using var session = await sessionProvider.OpenSession(); + failure.Id = MakeDocumentId(failure.Id); await session.StoreAsync(failure); await session.SaveChangesAsync(); @@ -392,8 +394,8 @@ public async Task EditComment(string groupId, string comment) { using var session = await sessionProvider.OpenSession(); var groupComment = - await session.LoadAsync(GroupComment.MakeId(groupId)) - ?? new GroupComment { Id = GroupComment.MakeId(groupId) }; + await session.LoadAsync(GroupsDataStore.MakeId(groupId)) + ?? new GroupComment { Id = GroupsDataStore.MakeId(groupId) }; groupComment.Comment = comment; @@ -404,7 +406,7 @@ await session.LoadAsync(GroupComment.MakeId(groupId)) public async Task DeleteComment(string groupId) { using var session = await sessionProvider.OpenSession(); - session.Delete(GroupComment.MakeId(groupId)); + session.Delete(GroupsDataStore.MakeId(groupId)); await session.SaveChangesAsync(); } @@ -595,7 +597,7 @@ public async Task RevertRetry(string messageUniqueId) failedMessage?.Status = FailedMessageStatus.Unresolved; var failedMessageRetry = await session - .LoadAsync(FailedMessageRetry.MakeDocumentId(messageUniqueId)); + .LoadAsync(RetryDocumentDataStore.MakeFailedMessageRetriesDocumentId(messageUniqueId)); if (failedMessageRetry != null) { session.Delete(failedMessageRetry); @@ -607,7 +609,7 @@ public async Task RevertRetry(string messageUniqueId) public async Task RemoveFailedMessageRetryDocument(string uniqueMessageId) { using var session = await sessionProvider.OpenSession(); - await session.Advanced.RequestExecutor.ExecuteAsync(new DeleteDocumentCommand(FailedMessageRetry.MakeDocumentId(uniqueMessageId), null), session.Advanced.Context); + await session.Advanced.RequestExecutor.ExecuteAsync(new DeleteDocumentCommand(RetryDocumentDataStore.MakeFailedMessageRetriesDocumentId(uniqueMessageId), null), session.Advanced.Context); } public async Task GetRetryPendingMessages(DateTime from, DateTime to, string queueAddress) @@ -674,5 +676,7 @@ public async Task StoreFailedMessagesForTestsOnly(params FailedMessage[] failedM await session.SaveChangesAsync(); } + + public static string MakeDocumentId(string id) => $"FailedErrorImports/{id}"; } } diff --git a/src/ServiceControl.Persistence.RavenDB/MessageRedirects/MessageRedirectsDataStore.cs b/src/ServiceControl.Persistence.RavenDB/MessageRedirects/MessageRedirectsDataStore.cs index b7d52a6f8a..8df68a921f 100644 --- a/src/ServiceControl.Persistence.RavenDB/MessageRedirects/MessageRedirectsDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/MessageRedirects/MessageRedirectsDataStore.cs @@ -5,10 +5,12 @@ class MessageRedirectsDataStore(IRavenSessionProvider sessionProvider) : IMessageRedirectsDataStore { + public const string CollectionId = "messageredirects"; + public async Task GetOrCreate() { using var session = await sessionProvider.OpenSession(); - var redirects = await session.LoadAsync(DefaultId); + var redirects = await session.LoadAsync(CollectionId); if (redirects != null) { @@ -24,10 +26,8 @@ public async Task GetOrCreate() public async Task Save(MessageRedirectsCollection redirects) { using var session = await sessionProvider.OpenSession(); - await session.StoreAsync(redirects, redirects.ETag, DefaultId); + await session.StoreAsync(redirects, redirects.ETag, CollectionId); await session.SaveChangesAsync(); } - - const string DefaultId = "messageredirects"; } } diff --git a/src/ServiceControl.Persistence.RavenDB/RavenMonitoringDataStore.cs b/src/ServiceControl.Persistence.RavenDB/RavenMonitoringDataStore.cs index 494b92b788..333da3dabb 100644 --- a/src/ServiceControl.Persistence.RavenDB/RavenMonitoringDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/RavenMonitoringDataStore.cs @@ -9,7 +9,7 @@ class RavenMonitoringDataStore(IRavenSessionProvider sessionProvider) : IMonitoringDataStore { - public static string MakeDocumentId(Guid id) => $"{KnownEndpoint.CollectionName}/{id}"; + public static string MakeDocumentId(Guid id) => $"{KnownEndpointsCollectionName}/{id}"; public async Task CreateIfNotExists(EndpointDetails endpoint) { @@ -111,5 +111,7 @@ public async Task> GetAllKnownEndpoints() return knownEndpoints; } + + public const string KnownEndpointsCollectionName = "KnownEndpoints"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs b/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs index ca7a57bbcf..6ae5c7879f 100644 --- a/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/Recoverability/GroupsDataStore.cs @@ -25,13 +25,13 @@ public async Task> GetFailureGroupsByClassifier(string c .Take(200) .ToListAsync(); - var commentIds = groups.Select(x => GroupComment.MakeId(x.Id)).ToArray(); + var commentIds = groups.Select(x => MakeId(x.Id)).ToArray(); var comments = await session.Query().Where(x => x.Id.In(commentIds)) .ToListAsync(CancellationToken.None); foreach (var group in groups) { - group.Comment = comments.FirstOrDefault(x => x.Id == GroupComment.MakeId(group.Id))?.Comment; + group.Comment = comments.FirstOrDefault(x => x.Id == MakeId(group.Id))?.Comment; } return groups; @@ -41,9 +41,11 @@ public async Task GetCurrentForwardingBatch() { using var session = await sessionProvider.OpenSession(); var nowForwarding = await session.Include(r => r.RetryBatchId) - .LoadAsync(RetryBatchNowForwarding.Id); + .LoadAsync(RetryDocumentDataStore.NowForwardingDocumentId); return nowForwarding == null ? null : await session.LoadAsync(nowForwarding.RetryBatchId); } + + public static string MakeId(string groupId) => $"GroupComment/{groupId}"; } } diff --git a/src/ServiceControl.Persistence.RavenDB/Recoverability/RetryHistoryDataStore.cs b/src/ServiceControl.Persistence.RavenDB/Recoverability/RetryHistoryDataStore.cs index 08119b045e..054359b812 100644 --- a/src/ServiceControl.Persistence.RavenDB/Recoverability/RetryHistoryDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/Recoverability/RetryHistoryDataStore.cs @@ -6,13 +6,15 @@ class RetryHistoryDataStore(IRavenSessionProvider sessionProvider) : IRetryHistoryDataStore { + const string DocumentId = "RetryOperations/History"; + public async Task GetRetryHistory() { using var session = await sessionProvider.OpenSession(); - var id = RetryHistory.MakeId(); + var id = DocumentId; var retryHistory = await session.LoadAsync(id); - retryHistory ??= RetryHistory.CreateNew(); + retryHistory ??= new() { Id = DocumentId }; return retryHistory; } @@ -21,7 +23,7 @@ public async Task RecordRetryOperationCompleted(string requestId, RetryType retr string originator, string classifier, bool messageFailed, int numberOfMessagesProcessed, DateTime lastProcessed, int retryHistoryDepth) { using var session = await sessionProvider.OpenSession(); - var retryHistory = await session.LoadAsync(RetryHistory.MakeId()) ?? RetryHistory.CreateNew(); + var retryHistory = await session.LoadAsync(DocumentId) ?? new() { Id = DocumentId }; retryHistory.AddToUnacknowledged(new UnacknowledgedRetryOperation { @@ -54,7 +56,7 @@ public async Task RecordRetryOperationCompleted(string requestId, RetryType retr public async Task AcknowledgeRetryGroup(string groupId) { using var session = await sessionProvider.OpenSession(); - var retryHistory = await session.LoadAsync(RetryHistory.MakeId()); + var retryHistory = await session.LoadAsync(DocumentId); if (retryHistory != null) { if (retryHistory.Acknowledge(groupId, RetryType.FailureGroup)) diff --git a/src/ServiceControl.Persistence.RavenDB/RetryBatchesDataStore.cs b/src/ServiceControl.Persistence.RavenDB/RetryBatchesDataStore.cs index 4d951b9354..277e300826 100644 --- a/src/ServiceControl.Persistence.RavenDB/RetryBatchesDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/RetryBatchesDataStore.cs @@ -80,7 +80,7 @@ public async Task IncrementAttemptCounter(FailedMessageRetry message) public async Task DeleteFailedMessageRetry(string uniqueMessageId) { using var session = await sessionProvider.OpenSession(); - await session.Advanced.RequestExecutor.ExecuteAsync(new DeleteDocumentCommand(FailedMessageRetry.MakeDocumentId(uniqueMessageId), null), session.Advanced.Context); + await session.Advanced.RequestExecutor.ExecuteAsync(new DeleteDocumentCommand(RetryDocumentDataStore.MakeFailedMessageRetriesDocumentId(uniqueMessageId), null), session.Advanced.Context); } } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDB/RetryBatchesManager.cs b/src/ServiceControl.Persistence.RavenDB/RetryBatchesManager.cs index 55b0aca0f5..8192e07229 100644 --- a/src/ServiceControl.Persistence.RavenDB/RetryBatchesManager.cs +++ b/src/ServiceControl.Persistence.RavenDB/RetryBatchesManager.cs @@ -5,6 +5,7 @@ using System.Threading; using System.Threading.Tasks; using MessageFailures; + using MessageRedirects; using Persistence.MessageRedirects; using Raven.Client.Documents; using Raven.Client.Documents.Session; @@ -39,7 +40,7 @@ public async Task GetFailedMessages(Dictionary GetRetryBatchNowForwarding() => await Session.Include(r => r.RetryBatchId) - .LoadAsync(RetryBatchNowForwarding.Id); + .LoadAsync(RetryDocumentDataStore.NowForwardingDocumentId); public async Task GetRetryBatch(string retryBatchId, CancellationToken cancellationToken) => await Session.LoadAsync(retryBatchId, cancellationToken); @@ -52,11 +53,11 @@ public async Task GetStagingBatch() } public async Task Store(RetryBatchNowForwarding retryBatchNowForwarding) => - await Session.StoreAsync(retryBatchNowForwarding, RetryBatchNowForwarding.Id); + await Session.StoreAsync(retryBatchNowForwarding, RetryDocumentDataStore.NowForwardingDocumentId); public async Task GetOrCreateMessageRedirectsCollection() { - var redirects = await Session.LoadAsync(MessageRedirectsCollection.DefaultId); + var redirects = await Session.LoadAsync(MessageRedirectsDataStore.CollectionId); if (redirects != null) { diff --git a/src/ServiceControl.Persistence.RavenDB/RetryDocumentDataStore.cs b/src/ServiceControl.Persistence.RavenDB/RetryDocumentDataStore.cs index 999e80ee42..b2606ece2a 100644 --- a/src/ServiceControl.Persistence.RavenDB/RetryDocumentDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/RetryDocumentDataStore.cs @@ -57,7 +57,8 @@ public async Task CreateBatchDocument(string retrySessionId, string requ DateTime startTime, DateTime? last = null, string batchName = null, string classifier = null, string initiatedById = null, string initiatedByName = null, string operationId = null) { - var batchDocumentId = RetryBatch.MakeDocumentId(Guid.NewGuid().ToString()); + var batchDocumentId = MakeDocumentId(Guid.NewGuid().ToString()); + failedMessageRetryIds = failedMessageRetryIds.Select(MakeFailedMessageRetriesDocumentId).ToArray(); using var session = await sessionProvider.OpenSession(); await session.StoreAsync(new RetryBatch { @@ -117,7 +118,7 @@ static ICommandData CreateFailedMessageRetryDocument(string batchDocumentId, str } }; - return new PatchCommandData(FailedMessageRetry.MakeDocumentId(messageId), null, patch: new PatchRequest { Script = "" }, patchIfMissing: patchRequest); + return new PatchCommandData(MakeFailedMessageRetriesDocumentId(messageId), null, patch: new PatchRequest { Script = "" }, patchIfMissing: patchRequest); } public async Task GetBatchesForAll(DateTime cutoff, Func callback) @@ -206,5 +207,11 @@ public async Task QueryFailureGroupViewOnGroupId(string groupI .FirstOrDefaultAsync(x => x.Id == groupId); return group; } + + public static string MakeDocumentId(string messageUniqueId) => "RetryBatches/" + messageUniqueId; + + public static string MakeFailedMessageRetriesDocumentId(string messageUniqueId) => "FailedMessageRetries/" + messageUniqueId; + + public static readonly string NowForwardingDocumentId = MakeDocumentId("NowForwarding"); } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDB/TrialLicenseDataProvider.cs b/src/ServiceControl.Persistence.RavenDB/TrialLicenseDataProvider.cs index a161f759bd..af9c5105bc 100644 --- a/src/ServiceControl.Persistence.RavenDB/TrialLicenseDataProvider.cs +++ b/src/ServiceControl.Persistence.RavenDB/TrialLicenseDataProvider.cs @@ -10,7 +10,7 @@ class TrialLicenseDataProvider(IRavenSessionProvider sessionProvider) : ITrialLi { using var session = await sessionProvider.OpenSession(cancellationToken: cancellationToken); - var document = await session.LoadAsync(TrialMetadata.TrialMetadataId, cancellationToken); + var document = await session.LoadAsync(TrialMetadataId, cancellationToken); return document?.TrialEndDate; } @@ -19,8 +19,10 @@ public async Task StoreTrialEndDate(DateOnly trialEndDate, CancellationToken can { using var session = await sessionProvider.OpenSession(cancellationToken: cancellationToken); - await session.StoreAsync(new TrialMetadata { TrialEndDate = trialEndDate }, TrialMetadata.TrialMetadataId, cancellationToken); + await session.StoreAsync(new TrialMetadata { TrialEndDate = trialEndDate }, TrialMetadataId, cancellationToken); await session.SaveChangesAsync(cancellationToken); } + + static string TrialMetadataId = "metadata/trialinformation"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenMonitoringIngestionUnitOfWork.cs b/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenMonitoringIngestionUnitOfWork.cs index 996e9ce4f3..af690c8129 100644 --- a/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenMonitoringIngestionUnitOfWork.cs +++ b/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenMonitoringIngestionUnitOfWork.cs @@ -63,7 +63,7 @@ static RavenMonitoringIngestionUnitOfWork() { KnownEndpointMetadata = JObject.Parse($@" {{ - ""@collection"": ""{KnownEndpoint.CollectionName}"", + ""@collection"": ""{RavenMonitoringDataStore.KnownEndpointsCollectionName}"", ""Raven-Clr-Type"": ""{typeof(KnownEndpoint).AssemblyQualifiedName}"" }}"); } diff --git a/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenRecoverabilityIngestionUnitOfWork.cs b/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenRecoverabilityIngestionUnitOfWork.cs index 906b2e1f36..882bd07d58 100644 --- a/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenRecoverabilityIngestionUnitOfWork.cs +++ b/src/ServiceControl.Persistence.RavenDB/UnitOfWork/RavenRecoverabilityIngestionUnitOfWork.cs @@ -69,7 +69,7 @@ public Task RecordFailedProcessingAttempt( public Task RecordSuccessfulRetry(string retriedMessageUniqueId) { var failedMessageDocumentId = FailedMessageIdGenerator.MakeDocumentId(retriedMessageUniqueId); - var failedMessageRetryDocumentId = FailedMessageRetry.MakeDocumentId(retriedMessageUniqueId); + var failedMessageRetryDocumentId = RetryDocumentDataStore.MakeFailedMessageRetriesDocumentId(retriedMessageUniqueId); var patchRequest = new PatchRequest { diff --git a/src/ServiceControl.Persistence.Tests.RavenDB/Operations/FailedErrorImportCustomCheckTests.cs b/src/ServiceControl.Persistence.Tests.RavenDB/Operations/FailedErrorImportCustomCheckTests.cs index 3e5184d657..2f778c3ebe 100644 --- a/src/ServiceControl.Persistence.Tests.RavenDB/Operations/FailedErrorImportCustomCheckTests.cs +++ b/src/ServiceControl.Persistence.Tests.RavenDB/Operations/FailedErrorImportCustomCheckTests.cs @@ -37,7 +37,7 @@ public async Task Fail_if_failed_imports() { await session.StoreAsync(new FailedErrorImport { - Id = FailedErrorImport.MakeDocumentId(Guid.NewGuid()) + Id = ServiceControl.Persistence.RavenDB.ErrorMessagesDataStore.MakeDocumentId(Guid.NewGuid().ToString()) }); BlockToInspectDatabase(); diff --git a/src/ServiceControl.Persistence.Tests.RavenDB/Recoverability/FailedMessageRetryTests.cs b/src/ServiceControl.Persistence.Tests.RavenDB/Recoverability/FailedMessageRetryTests.cs index f0d4bacbdd..90ceb5a697 100644 --- a/src/ServiceControl.Persistence.Tests.RavenDB/Recoverability/FailedMessageRetryTests.cs +++ b/src/ServiceControl.Persistence.Tests.RavenDB/Recoverability/FailedMessageRetryTests.cs @@ -2,6 +2,7 @@ { using System; using NUnit.Framework; + using Persistence.RavenDB; using ServiceControl.Recoverability; [TestFixture] @@ -13,9 +14,9 @@ public void Should_Extract_Correct_FailedMessageRetryId_From_FailedMessageId() var messageId = Guid.NewGuid().ToString(); var failedMessageId = FailedMessageIdGenerator.MakeDocumentId(messageId); - var extractedFailedMessageRetryId = FailedMessageRetry.MakeDocumentId(FailedMessageIdGenerator.GetMessageIdFromDocumentId(failedMessageId)); + var extractedFailedMessageRetryId = RetryDocumentDataStore.MakeFailedMessageRetriesDocumentId(FailedMessageIdGenerator.GetMessageIdFromDocumentId(failedMessageId)); - Assert.That(extractedFailedMessageRetryId, Is.EqualTo(FailedMessageRetry.MakeDocumentId(messageId))); + Assert.That(extractedFailedMessageRetryId, Is.EqualTo(RetryDocumentDataStore.MakeFailedMessageRetriesDocumentId(messageId))); } } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/EndpointSettings.cs b/src/ServiceControl.Persistence/EndpointSettings.cs index 1c388de4ff..46f7dab685 100644 --- a/src/ServiceControl.Persistence/EndpointSettings.cs +++ b/src/ServiceControl.Persistence/EndpointSettings.cs @@ -4,6 +4,4 @@ public class EndpointSettings { public string Name { get; set; } public bool TrackInstances { get; set; } - - public const string CollectionName = "EndpointSettings"; } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/EventLog/EventLogItem.cs b/src/ServiceControl.Persistence/EventLog/EventLogItem.cs index cf75a8da22..c726ccab14 100644 --- a/src/ServiceControl.Persistence/EventLog/EventLogItem.cs +++ b/src/ServiceControl.Persistence/EventLog/EventLogItem.cs @@ -9,7 +9,10 @@ public class EventLogItem public string Description { get; set; } public Severity Severity { get; set; } public DateTime RaisedAt { get; set; } - public List RelatedTo { get; set; } // This could be the Id of a related document, such as the FailedMessage event, which will have more information regarding this alert. + /// + /// This could be the Id of a related document, such as the FailedMessage event, which will have more information regarding this alert. + /// + public List RelatedTo { get; set; } public string Category { get; set; } public string EventType { get; set; } } diff --git a/src/ServiceControl.Persistence/FailedAuditImport.cs b/src/ServiceControl.Persistence/FailedAuditImport.cs deleted file mode 100644 index 3e62245b28..0000000000 --- a/src/ServiceControl.Persistence/FailedAuditImport.cs +++ /dev/null @@ -1,10 +0,0 @@ -namespace ServiceControl.Operations -{ - using System; - - public class FailedAuditImport - { - public Guid Id { get; set; } - public FailedTransportMessage Message { get; set; } - } -} \ No newline at end of file diff --git a/src/ServiceControl.Persistence/FailedErrorImport.cs b/src/ServiceControl.Persistence/FailedErrorImport.cs index 751f6f3418..5ae3573cd2 100644 --- a/src/ServiceControl.Persistence/FailedErrorImport.cs +++ b/src/ServiceControl.Persistence/FailedErrorImport.cs @@ -7,7 +7,5 @@ public class FailedErrorImport public string Id { get; set; } public FailedTransportMessage Message { get; set; } public string ExceptionInfo { get; set; } - - public static string MakeDocumentId(Guid id) => $"FailedErrorImports/{id}"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/FailedMessage.cs b/src/ServiceControl.Persistence/FailedMessage.cs index eec457aa95..74c871b2c9 100644 --- a/src/ServiceControl.Persistence/FailedMessage.cs +++ b/src/ServiceControl.Persistence/FailedMessage.cs @@ -50,11 +50,6 @@ public class GroupComment { public string Id { get; set; } public string Comment { get; set; } - - public static string MakeId(string groupId) - { - return $"GroupComment/{groupId}"; - } } public enum FailedMessageStatus diff --git a/src/ServiceControl.Persistence/FailedMessageRetry.cs b/src/ServiceControl.Persistence/FailedMessageRetry.cs index 281679ecf0..dccbcacde0 100644 --- a/src/ServiceControl.Persistence/FailedMessageRetry.cs +++ b/src/ServiceControl.Persistence/FailedMessageRetry.cs @@ -6,12 +6,5 @@ public class FailedMessageRetry public string FailedMessageId { get; set; } public string RetryBatchId { get; set; } public int StageAttempts { get; set; } - - public static string MakeDocumentId(string messageUniqueId) - { - return CollectionName + "/" + messageUniqueId; - } - - public const string CollectionName = "FailedMessageRetries"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/Infrastructure/TransportMessageExtensions.cs b/src/ServiceControl.Persistence/Infrastructure/TransportMessageExtensions.cs index 9ed1567ab5..4aaef5131e 100644 --- a/src/ServiceControl.Persistence/Infrastructure/TransportMessageExtensions.cs +++ b/src/ServiceControl.Persistence/Infrastructure/TransportMessageExtensions.cs @@ -102,15 +102,9 @@ public static bool IsBinary(this IReadOnlyDictionary headers) return true; } - static string ReplyToAddress(this IReadOnlyDictionary headers) - { - return headers.TryGetValue(Headers.ReplyToAddress, out var destination) ? destination : null; - } + static string ReplyToAddress(this IReadOnlyDictionary headers) => headers.GetValueOrDefault(Headers.ReplyToAddress); - static string ProcessingStarted(this IReadOnlyDictionary headers) - { - return headers.TryGetValue(Headers.ProcessingStarted, out var processingStarted) ? processingStarted : null; - } + static string ProcessingStarted(this IReadOnlyDictionary headers) => headers.GetValueOrDefault(Headers.ProcessingStarted); static string ExtractQueue(string address) { diff --git a/src/ServiceControl.Persistence/KnownEndpoint.cs b/src/ServiceControl.Persistence/KnownEndpoint.cs index 1fd5f16ae7..ac024a5407 100644 --- a/src/ServiceControl.Persistence/KnownEndpoint.cs +++ b/src/ServiceControl.Persistence/KnownEndpoint.cs @@ -7,7 +7,5 @@ public class KnownEndpoint public string HostDisplayName { get; set; } public bool Monitored { get; set; } public EndpointDetails EndpointDetails { get; set; } - - public const string CollectionName = "KnownEndpoints"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/MessageRedirects/MessageRedirectsCollection.cs b/src/ServiceControl.Persistence/MessageRedirects/MessageRedirectsCollection.cs index fa6b8b6ea9..02678e72ec 100644 --- a/src/ServiceControl.Persistence/MessageRedirects/MessageRedirectsCollection.cs +++ b/src/ServiceControl.Persistence/MessageRedirects/MessageRedirectsCollection.cs @@ -15,7 +15,5 @@ public class MessageRedirectsCollection public MessageRedirect this[Guid id] => Redirects.SingleOrDefault(r => r.MessageRedirectId == id); public List Redirects { get; set; } = []; - - public const string DefaultId = "messageredirects"; } } diff --git a/src/ServiceControl.Persistence/NotificationsSettings.cs b/src/ServiceControl.Persistence/NotificationsSettings.cs index 2197b67124..97bc8c4f55 100644 --- a/src/ServiceControl.Persistence/NotificationsSettings.cs +++ b/src/ServiceControl.Persistence/NotificationsSettings.cs @@ -2,8 +2,6 @@ { public class NotificationsSettings { - public const string SingleDocumentId = "NotificationsSettings/All"; - public string Id { get; set; } public EmailNotifications Email { get; set; } = new EmailNotifications(); diff --git a/src/ServiceControl.Persistence/Recoverability/ReclassifyErrorSettings.cs b/src/ServiceControl.Persistence/Recoverability/ReclassifyErrorSettings.cs deleted file mode 100644 index 25b9235d3e..0000000000 --- a/src/ServiceControl.Persistence/Recoverability/ReclassifyErrorSettings.cs +++ /dev/null @@ -1,15 +0,0 @@ -namespace ServiceControl.Recoverability -{ - public class ReclassifyErrorSettings - { - public ReclassifyErrorSettings() - { - Id = IdentifierCase; - } - - public string Id { get; set; } - - public bool ReclassificationDone { get; set; } - public const string IdentifierCase = "ReclassifyErrorSettings/1"; - } -} \ No newline at end of file diff --git a/src/ServiceControl.Persistence/RetryBatch.cs b/src/ServiceControl.Persistence/RetryBatch.cs index 45f89dddd9..386db556dc 100644 --- a/src/ServiceControl.Persistence/RetryBatch.cs +++ b/src/ServiceControl.Persistence/RetryBatch.cs @@ -26,7 +26,5 @@ public class RetryBatch public string InitiatedById { get; set; } public string InitiatedByName { get; set; } public string OperationId { get; set; } - - public static string MakeDocumentId(string messageUniqueId) => "RetryBatches/" + messageUniqueId; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/RetryBatchNowForwarding.cs b/src/ServiceControl.Persistence/RetryBatchNowForwarding.cs index 820cb7e4c4..e0fb79e430 100644 --- a/src/ServiceControl.Persistence/RetryBatchNowForwarding.cs +++ b/src/ServiceControl.Persistence/RetryBatchNowForwarding.cs @@ -3,6 +3,5 @@ namespace ServiceControl.Persistence public class RetryBatchNowForwarding { public string RetryBatchId { get; set; } - public const string Id = "RetryBatches/NowForwarding"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/RetryHistory.cs b/src/ServiceControl.Persistence/RetryHistory.cs index 995c9875fb..f736333f67 100644 --- a/src/ServiceControl.Persistence/RetryHistory.cs +++ b/src/ServiceControl.Persistence/RetryHistory.cs @@ -7,23 +7,8 @@ public class RetryHistory { public string Id { get; set; } - public List HistoricOperations { get; set; } - public List UnacknowledgedOperations { get; set; } - - public static string MakeId() - { - return "RetryOperations/History"; - } - - public static RetryHistory CreateNew() - { - return new RetryHistory - { - HistoricOperations = [], - UnacknowledgedOperations = [], - Id = MakeId() - }; - } + public List HistoricOperations { get; set; } = []; + public List UnacknowledgedOperations { get; set; } = []; public void AddToHistory(HistoricRetryOperation historicOperation, int historyDepth) { diff --git a/src/ServiceControl.Persistence/TrialMetadata.cs b/src/ServiceControl.Persistence/TrialMetadata.cs index d7eb67c11d..d587890403 100644 --- a/src/ServiceControl.Persistence/TrialMetadata.cs +++ b/src/ServiceControl.Persistence/TrialMetadata.cs @@ -5,7 +5,5 @@ public class TrialMetadata { public DateOnly TrialEndDate { get; set; } - - public static string TrialMetadataId = "metadata/trialinformation"; } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/UnitOfWork/UnitOfWorkServiceCollectionExtensions.cs b/src/ServiceControl.Persistence/UnitOfWork/UnitOfWorkServiceCollectionExtensions.cs index c14f7ab2bb..b3b8abeb1a 100644 --- a/src/ServiceControl.Persistence/UnitOfWork/UnitOfWorkServiceCollectionExtensions.cs +++ b/src/ServiceControl.Persistence/UnitOfWork/UnitOfWorkServiceCollectionExtensions.cs @@ -6,31 +6,10 @@ public static class UnitOfWorkServiceCollectionExtensions { /// - /// Register an ingestion unit of work factory that implements all of the components. + /// Register an ingestion unit of work factory that implements all the components. /// public static void AddUnitOfWorkFactory(this IServiceCollection serviceCollection) where T : class, IIngestionUnitOfWorkFactory => serviceCollection.AddSingleton(); - - /// - /// Register an ingestion unit of work factory that only implements some of the components. - /// The rest will be handled by RavenDb. - /// - public static void AddPartialUnitOfWorkFactory(this IServiceCollection serviceCollection) - where T : class, IIngestionUnitOfWorkFactory - { - // HINT: Falls back to the Raven implementation for components not implemented - var ravenImplementation = Type.GetType("ServiceControl.Persistence.RavenDb.RavenDbIngestionUnitOfWorkFactory, ServiceControl.Persistence.RavenDb", true); - serviceCollection.AddSingleton(ravenImplementation); - serviceCollection.AddSingleton(); - //HINT: Custom check state dependency is needed by the RavenDB unit of work - serviceCollection.AddSingleton(); - serviceCollection.AddSingleton(sp => - new FallbackIngestionUnitOfWorkFactory( - sp.GetRequiredService(), - (IIngestionUnitOfWorkFactory)sp.GetRequiredService(ravenImplementation) - ) - ); - } } } \ No newline at end of file diff --git a/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs b/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs index d60dae5c63..535be0b41d 100644 --- a/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs +++ b/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs @@ -57,7 +57,7 @@ async Task Handle(ErrorContext errorContext, CancellationToken cancellationToken Body = errorContext.Body.ToArray() }, ExceptionInfo = errorContext.Exception.ToFriendlyString(), - Id = FailedErrorImport.MakeDocumentId(Guid.NewGuid()) + Id = Guid.NewGuid().ToString() }; try @@ -80,7 +80,7 @@ async Task DoLogging(Exception exception, FailedErrorImport failure, Cancellatio if (!AppEnvironment.RunningInContainer) { // Write to Log Path - string filePath = Path.Combine(logPath, $"{failure.Id.Replace("/", "_")}.txt"); + string filePath = Path.Combine(logPath, $"FailedErrorImports_{failure.Id.Replace("/", "_")}.txt"); await File.WriteAllTextAsync(filePath, failure.ExceptionInfo, cancellationToken); if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) diff --git a/src/ServiceControl/Recoverability/Retrying/RetriesGateway.cs b/src/ServiceControl/Recoverability/Retrying/RetriesGateway.cs index 793310a1c0..c3770b32f7 100644 --- a/src/ServiceControl/Recoverability/Retrying/RetriesGateway.cs +++ b/src/ServiceControl/Recoverability/Retrying/RetriesGateway.cs @@ -54,7 +54,7 @@ async Task StageRetryByUniqueMessageIds(string requestId, RetryType retryType, s return; } - var failedMessageRetryIds = messageIds.Select(FailedMessageRetry.MakeDocumentId).ToArray(); + var failedMessageRetryIds = messageIds.ToArray(); var batchDocumentId = await store.CreateBatchDocument(RetryDocumentManager.RetrySessionId, requestId, retryType, failedMessageRetryIds, originator, startTime, last, batchName, classifier, initiatedBy?.Id, initiatedBy?.Name, operationId); From a6007f6976ff99672aa77b8840c4400e4c697f8e Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Fri, 24 Jul 2026 14:33:10 +0800 Subject: [PATCH 2/2] improvements from review --- .../ErrorMessagesDataStore.cs | 14 ++++++++------ .../Operations/ErrorIngestionFaultPolicy.cs | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs b/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs index 84d262936c..43b37c2fd5 100644 --- a/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDB/ErrorMessagesDataStore.cs @@ -173,6 +173,12 @@ public async Task FailedMessagesFetch(Guid[] ids) public async Task StoreFailedErrorImport(FailedErrorImport failure) { using var session = await sessionProvider.OpenSession(); + // This object's ID is generated externally, but is not in the RavenDB format + // Check that's true to make sure that if it already is that it doesn't get double-formatted + if (!failure.Id.StartsWith(CollectionName)) + { + failure.Id = MakeDocumentId(failure.Id); + } failure.Id = MakeDocumentId(failure.Id); await session.StoreAsync(failure); @@ -514,11 +520,6 @@ public async Task ProcessPendingRetries(DateTime periodFrom, DateTime periodTo, } } - class DocumentPatchResult - { - public string Document { get; set; } - } - public async Task UnArchiveMessagesByRange(DateTime from, DateTime to) { const int Unresolved = (int)FailedMessageStatus.Unresolved; @@ -677,6 +678,7 @@ public async Task StoreFailedMessagesForTestsOnly(params FailedMessage[] failedM await session.SaveChangesAsync(); } - public static string MakeDocumentId(string id) => $"FailedErrorImports/{id}"; + public static string MakeDocumentId(string id) => string.Join("/", CollectionName, id); + public const string CollectionName = "FailedErrorImports"; } } diff --git a/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs b/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs index 535be0b41d..35eff1209a 100644 --- a/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs +++ b/src/ServiceControl/Operations/ErrorIngestionFaultPolicy.cs @@ -57,7 +57,7 @@ async Task Handle(ErrorContext errorContext, CancellationToken cancellationToken Body = errorContext.Body.ToArray() }, ExceptionInfo = errorContext.Exception.ToFriendlyString(), - Id = Guid.NewGuid().ToString() + Id = Guid.CreateVersion7().ToString() }; try