diff --git a/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/FailedMessageIdGenerator.cs b/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/FailedMessageIdGenerator.cs index 06f242dfd4..4935378ec9 100644 --- a/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/FailedMessageIdGenerator.cs +++ b/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/FailedMessageIdGenerator.cs @@ -2,13 +2,10 @@ { public const string CollectionName = "FailedMessages"; - //[Obsolete("Use Guid.Parse")] TODO: As these are all guids... we don't need these generators.. Unless these are MessageIdentifiers as these can have any string value public static string MakeDocumentId(string messageUniqueId) { - Guard.Assert(!HasPrefix(messageUniqueId), $"value has {CollectionName}/ prefix"); // TODO: Could potentially be removed when all tests are green but no harm as its only included on Debug builds return $"{CollectionName}/{messageUniqueId}"; } public static string GetMessageIdFromDocumentId(string failedMessageDocumentId) => failedMessageDocumentId.Substring(CollectionName.Length + 1); - static bool HasPrefix(string value) => value.StartsWith(CollectionName + "/"); } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/MessageBodyIdGenerator.cs b/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/MessageBodyIdGenerator.cs index 3c693e749a..9891ffdc14 100644 --- a/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/MessageBodyIdGenerator.cs +++ b/src/ServiceControl.Persistence.RavenDb/DocumentIdGenerators/MessageBodyIdGenerator.cs @@ -4,9 +4,6 @@ public static string MakeDocumentId(string messageUniqueId) { - Guard.Assert(!HasPrefix(messageUniqueId), $"value has {CollectionName}/ prefix"); // TODO: Could potentially be removed when all tests are green but no harm as its only included on Debug builds return $"{CollectionName}/{messageUniqueId}"; } - - static bool HasPrefix(string value) => value.StartsWith(CollectionName + "/"); } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDb/Editing/NotificationsManager.cs b/src/ServiceControl.Persistence.RavenDb/Editing/NotificationsManager.cs index 0a4177e6b9..dd4d8fc4b3 100644 --- a/src/ServiceControl.Persistence.RavenDb/Editing/NotificationsManager.cs +++ b/src/ServiceControl.Persistence.RavenDb/Editing/NotificationsManager.cs @@ -7,7 +7,7 @@ class NotificationsManager : AbstractSessionManager, INotificationsManager { - static readonly TimeSpan CacheTimeoutDefault = TimeSpan.FromMinutes(5); // TODO: Copied value from SendEmailNotificationHandler.cacheTimeout, Raven requires this to be at least 1 second + static readonly TimeSpan CacheTimeoutDefault = TimeSpan.FromMinutes(5); // Raven requires this to be at least 1 second public NotificationsManager(IAsyncDocumentSession session) : base(session) { diff --git a/src/ServiceControl.Persistence.RavenDb/ErrorMessagesDataStore.cs b/src/ServiceControl.Persistence.RavenDb/ErrorMessagesDataStore.cs index 8eb216e327..2de540abd0 100644 --- a/src/ServiceControl.Persistence.RavenDb/ErrorMessagesDataStore.cs +++ b/src/ServiceControl.Persistence.RavenDb/ErrorMessagesDataStore.cs @@ -217,7 +217,7 @@ public async Task> GetFailureGroupView(string grou .WhereEquals(group => group.Id, groupId) .FilterByStatusWhere(status) .FilterByLastModifiedRange(modified) - .FirstOrDefaultAsync(); // TODO: Was previously a to list with a linq to object FirstOrDefault, not sure if this works; + .FirstOrDefaultAsync(); return new QueryResult(document, stats.ToQueryStatsInfo()); } @@ -343,8 +343,6 @@ public async Task> ErrorsSummary() x => (object)x.Value ); - Guard.Assert(false, "TODO: Check how to convert dictionary item VALUES, currently return object which must be typed"); - return results; } } @@ -667,7 +665,8 @@ public Task RemoveFailedMessageRetryDocument(string uniqueMessageId) return documentStore.AsyncDatabaseCommands.DeleteAsync(FailedMessageRetry.MakeDocumentId(uniqueMessageId), null); } - public async Task GetRetryPendingMessages(DateTime from, DateTime to, string queueAddress) // TODO: Could we use IAsyncEnumerable here as this is an unbounded query? + // TODO: Once using .NET, consider using IAsyncEnumerable here as this is an unbounded query + public async Task GetRetryPendingMessages(DateTime from, DateTime to, string queueAddress) { var ids = new List(); @@ -692,10 +691,9 @@ public async Task GetRetryPendingMessages(DateTime from, DateTime to, } } - return ids.ToArray(); // TODO: Currently returning array as all other API's return arrays and not IEnumerable + return ids.ToArray(); } - // TODO: How is this different than what RavenAttachmentBodyStorage.TryFetch is doing? Is this implemented twice? public async Task FetchFromFailedMessage(string uniqueMessageId) { string documentId = FailedMessageIdGenerator.MakeDocumentId(uniqueMessageId); @@ -722,7 +720,7 @@ public async Task StoreEventLogItem(EventLogItem logItem) } } - public async Task StoreFailedMessages(params FailedMessage[] failedMessages) + public async Task StoreFailedMessagesForTestsOnly(params FailedMessage[] failedMessages) { using (var session = documentStore.OpenAsyncSession()) { diff --git a/src/ServiceControl.Persistence.RavenDb/FailedMessageViewIndexNotifications.cs b/src/ServiceControl.Persistence.RavenDb/FailedMessageViewIndexNotifications.cs index 0d0bf814e5..1ab4388de0 100644 --- a/src/ServiceControl.Persistence.RavenDb/FailedMessageViewIndexNotifications.cs +++ b/src/ServiceControl.Persistence.RavenDb/FailedMessageViewIndexNotifications.cs @@ -9,7 +9,7 @@ using Persistence; using Raven.Client; - class FailedMessageViewIndexNotifications // TODO: Must be registered as single and as hosted service + class FailedMessageViewIndexNotifications : IFailedMessageViewIndexNotifications , IDisposable , IHostedService diff --git a/src/ServiceControl.Persistence.RavenDb/Guard.cs b/src/ServiceControl.Persistence.RavenDb/Guard.cs deleted file mode 100644 index a74daf1fd9..0000000000 --- a/src/ServiceControl.Persistence.RavenDb/Guard.cs +++ /dev/null @@ -1,20 +0,0 @@ -using System; -using System.Diagnostics; - -static class Guard -{ - /// - /// Checks for a condition; if the condition is false, raise exception with message - /// - public static void Assert(bool condition, string message) - { - if (!condition) - { - if (Debugger.IsAttached) - { - Debugger.Break(); - } - throw new InvalidOperationException(message); - } - } -} \ No newline at end of file diff --git a/src/ServiceControl.Persistence.RavenDb/RavenDbInstaller.cs b/src/ServiceControl.Persistence.RavenDb/RavenDbInstaller.cs index 7a4c4562ee..c940a0448d 100644 --- a/src/ServiceControl.Persistence.RavenDb/RavenDbInstaller.cs +++ b/src/ServiceControl.Persistence.RavenDb/RavenDbInstaller.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using NServiceBus.Logging; using Raven.Client.Embedded; + using ServiceControl.Infrastructure.RavenDB; class RavenDbInstaller : IPersistenceInstaller { @@ -19,22 +20,13 @@ public async Task Install(CancellationToken cancellationToken = default) documentStore.Initialize(); Logger.Info("Database initialization complete"); + Logger.Info("Index creation started"); await ravenStartup.CreateIndexesAsync(documentStore); + Logger.Info("Index creation complete"); - Logger.Info("Data migrations starting **TODO NOT IMPLEMENTED YET**"); - - // TODO: Figure out migrations - // This was copied from audit code: - ////var endpointMigrations = new MigrateKnownEndpoints(documentStore); - ////await endpointMigrations.Migrate(cancellationToken: cancellationToken) - //// ; - // While this was copied from EmbeddedRavenDbHostedService in main ServiceControl project, but list was injected - ////foreach (var migration in dataMigrations) - ////{ - //// await migration.Migrate(documentStore) - //// ; - ////} - + Logger.Info("Data migrations starting"); + var purgeTempIdKnownEndpoints = new PurgeKnownEndpointsWithTemporaryIdsThatAreDuplicateDataMigration(); + await purgeTempIdKnownEndpoints.Migrate(documentStore); Logger.Info("Data migrations complete"); } diff --git a/src/ServiceControl.Persistence.RavenDb/RavenDbPersistence.cs b/src/ServiceControl.Persistence.RavenDb/RavenDbPersistence.cs index 008e4f748c..8be713ddf6 100644 --- a/src/ServiceControl.Persistence.RavenDb/RavenDbPersistence.cs +++ b/src/ServiceControl.Persistence.RavenDb/RavenDbPersistence.cs @@ -78,21 +78,6 @@ public IPersistenceLifecycle CreateLifecycle() return new RavenDbPersistenceLifecycle(ravenStartup, documentStore); } - // TODO: Make sure this stuff from PersistenceHostBuilderExtensions is accounted for here - - //var documentStore = new EmbeddableDocumentStore(); - //RavenBootstrapper.Configure(documentStore, settings); - - //hostBuilder.ConfigureServices(serviceCollection => - //{ - // serviceCollection.AddSingleton(documentStore); - // serviceCollection.AddHostedService(); - // serviceCollection.AddCustomCheck(); - // serviceCollection.AddCustomCheck(); - - // serviceCollection.AddServiceControlPersistence(settings.DataStoreType); - //}); - public IPersistenceInstaller CreateInstaller() { return new RavenDbInstaller(documentStore, ravenStartup); diff --git a/src/ServiceControl.Persistence.Tests.RavenDb/Expiration/ProcessedMessageExpirationTests.cs b/src/ServiceControl.Persistence.Tests.RavenDb/Expiration/ProcessedMessageExpirationTests.cs index c62acc37c9..1425366ee4 100644 --- a/src/ServiceControl.Persistence.Tests.RavenDb/Expiration/ProcessedMessageExpirationTests.cs +++ b/src/ServiceControl.Persistence.Tests.RavenDb/Expiration/ProcessedMessageExpirationTests.cs @@ -18,7 +18,7 @@ sealed class ProcessedMessageExpirationTests : PersistenceTestBase { - IDocumentStore DocumentStore => GetRequiredService(); // TODO: Should primary use abstractions and only have native code for triggering the cleanup + IDocumentStore DocumentStore => GetRequiredService(); [Test] public void Old_documents_are_being_expired() diff --git a/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/RetryConfirmationProcessorTests.cs b/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/RetryConfirmationProcessorTests.cs index d40e64937f..ee092c8b3f 100644 --- a/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/RetryConfirmationProcessorTests.cs +++ b/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/RetryConfirmationProcessorTests.cs @@ -11,10 +11,9 @@ using NServiceBus.Testing; using NServiceBus.Transport; using NUnit.Framework; - using ServiceControl.Operations; using PersistenceTests; + using ServiceControl.Operations; - // TODO: Moved by Ramon to RavenDB specific tests, has a lot of RavenDB dependencies class RetryConfirmationProcessorTests : PersistenceTestBase { RetryConfirmationProcessor Processor { get; set; } @@ -28,7 +27,7 @@ class RetryConfirmationProcessorTests : PersistenceTestBase Handler = new LegacyMessageFailureResolvedHandler(ErrorMessageDataStore, domainEvents); - await ErrorMessageDataStore.StoreFailedMessages( + await ErrorMessageDataStore.StoreFailedMessagesForTestsOnly( new FailedMessage { Id = MessageId, diff --git a/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/ReturnToSenderDequeuerTests.cs b/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/ReturnToSenderDequeuerTests.cs index 181e16cf58..0ac1e9d507 100644 --- a/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/ReturnToSenderDequeuerTests.cs +++ b/src/ServiceControl.Persistence.Tests.RavenDb/Recoverability/ReturnToSenderDequeuerTests.cs @@ -101,7 +101,7 @@ public async Task It_fetches_the_body_from_index_if_provided() } }; - await ErrorMessageDataStore.StoreFailedMessages(failedMessage); + await ErrorMessageDataStore.StoreFailedMessagesForTestsOnly(failedMessage); var transformer = new MessagesBodyTransformer(); await transformer.ExecuteAsync(documentStore); diff --git a/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs b/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs index bb9bdd0f50..57763eda30 100644 --- a/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs +++ b/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs @@ -39,7 +39,6 @@ protected Task CompleteDatabaseOperation() return testPersistence.CompleteDatabaseOperation(); } - // TODO: Evaluate if BlockToInspectDatabase() concept should stay in codebase long-term [Conditional("DEBUG")] protected void BlockToInspectDatabase() => testPersistence.BlockToInspectDatabase(); diff --git a/src/ServiceControl.Persistence.Tests/Recoverability/EditMessageTests.cs b/src/ServiceControl.Persistence.Tests/Recoverability/EditMessageTests.cs index 09f1079969..3c38181c58 100644 --- a/src/ServiceControl.Persistence.Tests/Recoverability/EditMessageTests.cs +++ b/src/ServiceControl.Persistence.Tests/Recoverability/EditMessageTests.cs @@ -241,7 +241,7 @@ async Task CreateAndStoreFailedMessage(string failedMessageId = n } } }; - await ErrorMessageDataStore.StoreFailedMessages(new[] { failedMessage }); + await ErrorMessageDataStore.StoreFailedMessagesForTestsOnly(new[] { failedMessage }); return failedMessage; } } diff --git a/src/ServiceControl.Persistence.Tests/RetryStateTests.cs b/src/ServiceControl.Persistence.Tests/RetryStateTests.cs index 7d2d1f91b3..20742f525b 100644 --- a/src/ServiceControl.Persistence.Tests/RetryStateTests.cs +++ b/src/ServiceControl.Persistence.Tests/RetryStateTests.cs @@ -203,7 +203,7 @@ async Task CreateAFailedMessageAndMarkAsPartOfRetryBatch(RetryingManager retryMa } }).ToArray(); - await ErrorStore.StoreFailedMessages(messages); + await ErrorStore.StoreFailedMessagesForTestsOnly(messages); // Needs index FailedMessages_ByGroup // Needs index FailedMessages_UniqueMessageIdAndTimeOfFailures @@ -212,7 +212,6 @@ async Task CreateAFailedMessageAndMarkAsPartOfRetryBatch(RetryingManager retryMa var documentManager = new CustomRetryDocumentManager(progressToStaged, RetryStore, retryManager); var gateway = new CustomRetriesGateway(progressToStaged, RetryStore, retryManager); - // TODO: groupType appears to be the same as classifier, which was null in the previous StartRetryForIndex call - make sure that's true gateway.EnqueueRetryForFailureGroup(new RetriesGateway.RetryForFailureGroup(groupId, "Test-Context", groupType: null, DateTime.UtcNow)); await CompleteDatabaseOperation(); diff --git a/src/ServiceControl.Persistence/IErrorMessageDatastore.cs b/src/ServiceControl.Persistence/IErrorMessageDatastore.cs index d204753742..910783554e 100644 --- a/src/ServiceControl.Persistence/IErrorMessageDatastore.cs +++ b/src/ServiceControl.Persistence/IErrorMessageDatastore.cs @@ -76,7 +76,6 @@ public interface IErrorMessageDataStore Task>> SearchEndpointMessages(string endpointName, string searchKeyword, PagingInfo pagingInfo, SortInfo sortInfo); - // TODO: So far only used in a persistence test: RetryStateTests - Task StoreFailedMessages(params FailedMessage[] failedMessages); + Task StoreFailedMessagesForTestsOnly(params FailedMessage[] failedMessages); } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence/RetryBatchGroup.cs b/src/ServiceControl.Persistence/RetryBatchGroup.cs index 8504bbb7d9..cb53629017 100644 --- a/src/ServiceControl.Persistence/RetryBatchGroup.cs +++ b/src/ServiceControl.Persistence/RetryBatchGroup.cs @@ -8,6 +8,7 @@ public class RetryBatchGroup // [Raven.Imports.Newtonsoft.Json.JsonProperty(NullValueHandling = NullValueHandling.Ignore)] //default to RetryType.Unknown for backwards compatability // TODO: Need to fix ethe JsonProperty, maybe RavenDB has a method to specify metatdata or use a mapper/transformation + // THEORY: RetryType.Unknown is value 0 so it should default to that anyway public RetryType RetryType { get; set; } public bool HasStagingBatches { get; set; } diff --git a/src/ServiceControl.UnitTests/API/APIApprovals.cs b/src/ServiceControl.UnitTests/API/APIApprovals.cs index 72888d0e97..d85d09c760 100644 --- a/src/ServiceControl.UnitTests/API/APIApprovals.cs +++ b/src/ServiceControl.UnitTests/API/APIApprovals.cs @@ -6,11 +6,9 @@ using System.Net.Http; using System.Reflection; using System.Text; - using System.Text.Json; using System.Web.Http.Controllers; using System.Web.Http.Hosting; using System.Web.Http.Routing; - using Newtonsoft.Json.Linq; using NServiceBus.CustomChecks; using NUnit.Framework; using Particular.Approvals; @@ -134,7 +132,7 @@ public void TransportNames() Approver.Verify(publicTransportNames); } - [Test, Ignore("TODO: Deal with this once persister settings are properly managed")] // + [Test, Ignore("TODO: Deal with this once persister settings are properly managed")] public void PlatformSampleSettings() { //HINT: Particular.PlatformSample includes a parameterized version of the ServiceControl.exe.config file. diff --git a/src/ServiceControl/Bootstrapper.cs b/src/ServiceControl/Bootstrapper.cs index 29573fcdba..87cb5ab53b 100644 --- a/src/ServiceControl/Bootstrapper.cs +++ b/src/ServiceControl/Bootstrapper.cs @@ -93,11 +93,6 @@ void CreateHost() HostBuilder = new HostBuilder(); HostBuilder - .UseDefaultServiceProvider(c => // TODO: Remove when done testing - { - c.ValidateOnBuild = false; - c.ValidateScopes = false; - }) .ConfigureLogging(builder => { builder.ClearProviders(); diff --git a/src/ServiceControl/MessageFailures/InternalMessages/UnArchiveMessages.cs b/src/ServiceControl/MessageFailures/InternalMessages/UnArchiveMessages.cs index 588160ad8d..c4e01da278 100644 --- a/src/ServiceControl/MessageFailures/InternalMessages/UnArchiveMessages.cs +++ b/src/ServiceControl/MessageFailures/InternalMessages/UnArchiveMessages.cs @@ -5,6 +5,6 @@ class UnArchiveMessages : ICommand { - public List FailedMessageIds { get; set; } // TODO: + public List FailedMessageIds { get; set; } } } \ No newline at end of file diff --git a/src/ServiceControl/Operations/ErrorProcessor.cs b/src/ServiceControl/Operations/ErrorProcessor.cs index b53670cbdc..2edc47ada4 100644 --- a/src/ServiceControl/Operations/ErrorProcessor.cs +++ b/src/ServiceControl/Operations/ErrorProcessor.cs @@ -117,7 +117,7 @@ async Task ProcessMessage(MessageContext context, IIngestionUnitOfWork unitOfWor var processingAttempt = failedMessageFactory.CreateProcessingAttempt( context.Headers, - new Dictionary(metadata), // TODO: metadata is already a dictionary, it this really needed? + new Dictionary(metadata), failureDetails); await bodyStorageEnricher.StoreErrorMessageBody(context.Body, processingAttempt); diff --git a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs index 5eb8a99ea1..80377b5fec 100644 --- a/src/ServiceControl/Recoverability/API/FailureGroupsController.cs +++ b/src/ServiceControl/Recoverability/API/FailureGroupsController.cs @@ -85,7 +85,7 @@ public async Task GetAllGroups(string classifier = "Excepti classifierFilter = null; } - var results = await groupFetcher.GetGroups(classifier, classifierFilter); // TODO: Analyze what to do with the GroupFetcher dependency + var results = await groupFetcher.GetGroups(classifier, classifierFilter); return Negotiator.FromModel(Request, results) .WithDeterministicEtag(EtagHelper.CalculateEtag(results)); @@ -137,7 +137,6 @@ public async Task GetGroup(string groupId) string status = Request.GetStatus(); string modified = Request.GetModified(); - // TODO: Migrated as previous behavior but can be optimized as http api will return at most 1 item var result = await dataStore.GetGroup(groupId, status, modified); return Negotiator diff --git a/src/ServiceControl/Recoverability/Retrying/RetryDocumentManager.cs b/src/ServiceControl/Recoverability/Retrying/RetryDocumentManager.cs index ff798af01b..ee4c8b6a3a 100644 --- a/src/ServiceControl/Recoverability/Retrying/RetryDocumentManager.cs +++ b/src/ServiceControl/Recoverability/Retrying/RetryDocumentManager.cs @@ -71,7 +71,8 @@ public async Task RebuildRetryOperationState() readonly RetryingManager operationManager; readonly IRetryDocumentDataStore store; bool abort; - public static string RetrySessionId = Guid.NewGuid().ToString(); // TODO: Uplift this into DI? + // TODO: Uplift this into DI? Meant to differentiate between ServiceControl.exe process runs, so likely doesn't matter. + public static string RetrySessionId = Guid.NewGuid().ToString(); static readonly ILog log = LogManager.GetLogger(typeof(RetryDocumentManager)); } diff --git a/src/ServiceControl/SagaAudit/SagaAuditComponent.cs b/src/ServiceControl/SagaAudit/SagaAuditComponent.cs index d47027dc51..1380b5470f 100644 --- a/src/ServiceControl/SagaAudit/SagaAuditComponent.cs +++ b/src/ServiceControl/SagaAudit/SagaAuditComponent.cs @@ -9,6 +9,7 @@ class SagaAuditComponent : ServiceControlComponent public override void Configure(Settings settings, IHostBuilder hostBuilder) { // TODO: If this component doesn't do anything, should it even exist? + // THEORY: Remove in V5, since then there will be no audit capabilities left in the primary instance } public override void Setup(Settings settings, IComponentInstallationContext context)