Fix event consumer and restore state. (#898)

backend/src/Squidex.Domain.Apps.Entities/Backup/BackupService.cs

@@ -15,19 +15,19 @@ namespace Squidex.Domain.Apps.Entities.Backup
 {
     public sealed class BackupService : IBackupService, IDeleter
     {
-        private readonly SimpleState<RestoreJob> restoreState;
+        private readonly SimpleState<BackupRestoreState> restoreState;
         private readonly IPersistenceFactory<BackupState> persistenceFactoryBackup;
         private readonly IMessageBus messaging;
 
         public BackupService(
-            IPersistenceFactory<RestoreJob> persistenceFactoryRestore,
+            IPersistenceFactory<BackupRestoreState> persistenceFactoryRestore,
             IPersistenceFactory<BackupState> persistenceFactoryBackup,
             IMessageBus messaging)
         {
             this.persistenceFactoryBackup = persistenceFactoryBackup;
             this.messaging = messaging;
 
-            restoreState = new SimpleState<RestoreJob>(persistenceFactoryRestore, GetType(), "Default");
+            restoreState = new SimpleState<BackupRestoreState>(persistenceFactoryRestore, GetType(), "Default");
         }
 
         Task IDeleter.DeleteAppAsync(IAppEntity app,
@@ -51,7 +51,7 @@ namespace Squidex.Domain.Apps.Entities.Backup
         {
             await restoreState.LoadAsync(ct);
 
-            restoreState.Value.EnsureCanStart();
+            restoreState.Value.Job?.EnsureCanStart();
 
             await messaging.PublishAsync(new BackupRestore(actor, url, newAppName), ct: ct);
         }
@@ -67,7 +67,7 @@ namespace Squidex.Domain.Apps.Entities.Backup
         {
             await restoreState.LoadAsync(ct);
 
-            return restoreState.Value;
+            return restoreState.Value.Job ?? new RestoreJob();
         }
 
         public async Task<List<IBackupJob>> GetBackupsAsync(DomainId appId,

backend/src/Squidex.Infrastructure/EventSourcing/Consume/BatchSubscription.cs

@@ -23,8 +23,6 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
             IEventSubscriber<ParsedEvents> eventSubscriber,
             EventSubscriptionSource<ParsedEvent> eventSource)
         {
-            eventSubscription = eventSource(this);
-
             var batchSize = Math.Max(1, eventConsumer.BatchSize);
             var batchDelay = Math.Max(100, eventConsumer.BatchDelay);
 
@@ -44,6 +42,9 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
             batchQueue.Batch<ParsedEvent>(taskQueue, batchSize, batchDelay, completed.Token);
 
             handleTask = Run(eventSubscriber);
+
+            // Run last to subscribe after everything is configured.
+            eventSubscription = eventSource(this);
         }
 
         private async Task Run(IEventSubscriber<ParsedEvents> eventSink)

backend/src/Squidex.Infrastructure/EventSourcing/Consume/EventConsumerProcessor.cs

@@ -51,11 +51,12 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
 
         public virtual async Task CompleteAsync()
         {
-            if (currentSubscription is BatchSubscription batchSubscriber)
+            // This is only needed for tests to wait for all asynchronous tasks inside the subscriptions.
+            if (currentSubscription != null)
             {
                 try
                 {
-                    await batchSubscriber.CompleteAsync();
+                    await currentSubscription.CompleteAsync();
                 }
                 catch (Exception ex)
                 {
@@ -250,12 +251,14 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
 
         protected IEventSubscription CreatePipeline(IEventSubscriber<ParsedEvents> subscriber)
         {
+            // Create a pipline of subscription inside a retry.
             return new BatchSubscription(eventConsumer, subscriber,
                 x => new ParseSubscription(eventConsumer, eventFormatter, x, CreateSubscription));
         }
 
         protected virtual IEventSubscription CreateRetrySubscription(IEventSubscriber<ParsedEvents> subscriber)
         {
+            // It is very important to have the retry subscription as outer subscription, because we also need to cancel the batching in case of errors.
             return new RetrySubscription<ParsedEvents>(subscriber, CreatePipeline);
         }
 

backend/src/Squidex.Infrastructure/EventSourcing/Consume/EventConsumerWorker.cs

@@ -15,7 +15,7 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
         IMessageHandler<EventConsumerStart>,
         IMessageHandler<EventConsumerStop>,
         IMessageHandler<EventConsumerReset>,
-        IInitializable
+        IBackgroundProcess
     {
         private readonly Dictionary<string, EventConsumerProcessor> processors = new Dictionary<string, EventConsumerProcessor>();
         private CompletionTimer? timer;
@@ -29,7 +29,7 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
             }
         }
 
-        public async Task InitializeAsync(
+        public async Task StartAsync(
             CancellationToken ct)
         {
             foreach (var (_, processor) in processors)
@@ -47,7 +47,7 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
             });
         }
 
-        public Task ReleaseAsync(
+        public Task StopAsync(
             CancellationToken ct)
         {
             return timer?.StopAsync() ?? Task.CompletedTask;

backend/src/Squidex.Infrastructure/EventSourcing/Consume/ParseSubscription.cs

@@ -22,8 +22,6 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
             IEventSubscriber<ParsedEvent> eventSubscriber,
             EventSubscriptionSource<StoredEvent> eventSource)
         {
-            eventSubscription = eventSource(this);
-
             deserializeQueue = Channel.CreateBounded<object>(new BoundedChannelOptions(2)
             {
                 AllowSynchronousContinuations = true,
@@ -89,6 +87,9 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
                     }
                 }
             }).ContinueWith(x => deserializeQueue.Writer.TryComplete(x.Exception));
+
+            // Run last to subscribe after everything is configured.
+            eventSubscription = eventSource(this);
         }
 
         public void Dispose()

backend/src/Squidex.Infrastructure/EventSourcing/RetrySubscription.cs

@@ -15,11 +15,30 @@ namespace Squidex.Infrastructure.EventSourcing
         private readonly AsyncLock lockObject = new AsyncLock();
         private readonly IEventSubscriber<T> eventSubscriber;
         private readonly EventSubscriptionSource<T> eventSource;
-        private CancellationTokenSource timerCancellation = new CancellationTokenSource();
+        private SubscriptionHolder? currentSubscription;
-        private IEventSubscription? currentSubscription;
 
         public int ReconnectWaitMs { get; set; } = 5000;
 
+        // Holds all information for a current subscription. Therefore we only have to maintain one reference.
+        private sealed class SubscriptionHolder : IDisposable
+        {
+            public CancellationTokenSource Cancellation { get; } = new CancellationTokenSource();
+
+            public IEventSubscription Subscription { get; }
+
+            public SubscriptionHolder(IEventSubscription subscription)
+            {
+                Subscription = subscription;
+            }
+
+            public void Dispose()
+            {
+                Cancellation.Cancel();
+
+                Subscription.Dispose();
+            }
+        }
+
         public RetrySubscription(IEventSubscriber<T> eventSubscriber,
             EventSubscriptionSource<T> eventSource)
         {
@@ -49,7 +68,7 @@ namespace Squidex.Infrastructure.EventSourcing
                 return;
             }
 
-            currentSubscription = eventSource(this);
+            currentSubscription = new SubscriptionHolder(eventSource(this));
         }
 
         private void Unsubscribe()
@@ -59,30 +78,26 @@ namespace Squidex.Infrastructure.EventSourcing
                 return;
             }
 
-            timerCancellation.Cancel();
-            timerCancellation.Dispose();
-
             currentSubscription.Dispose();
             currentSubscription = null;
-
-            timerCancellation = new CancellationTokenSource();
         }
 
         public void WakeUp()
         {
-            currentSubscription?.WakeUp();
+            currentSubscription?.Subscription.WakeUp();
         }
 
         public ValueTask CompleteAsync()
         {
-            return currentSubscription?.CompleteAsync() ?? default;
+            return currentSubscription?.Subscription.CompleteAsync() ?? default;
         }
 
         async ValueTask IEventSubscriber<T>.OnNextAsync(IEventSubscription subscription, T @event)
         {
+            // It is not entirely sure, if the lock is needed, but it seems to work so far.
             using (await lockObject.EnterAsync(default))
             {
-                if (!ReferenceEquals(subscription, currentSubscription))
+                if (!ReferenceEquals(subscription, currentSubscription?.Subscription))
                 {
                     return;
                 }
@@ -100,11 +115,12 @@ namespace Squidex.Infrastructure.EventSourcing
 
             using (await lockObject.EnterAsync(default))
             {
-                if (!ReferenceEquals(subscription, currentSubscription))
+                if (!ReferenceEquals(subscription, currentSubscription?.Subscription))
                 {
                     return;
                 }
 
+                // Unsubscribing is not an atomar operation, therefore the lock.
                 Unsubscribe();
 
                 if (!retryWindow.CanRetryAfterFailure())
@@ -116,7 +132,7 @@ namespace Squidex.Infrastructure.EventSourcing
 
             try
             {
-                await Task.Delay(ReconnectWaitMs, timerCancellation.Token);
+                await Task.Delay(ReconnectWaitMs, currentSubscription?.Cancellation?.Token ?? default);
             }
             catch (OperationCanceledException)
             {
@@ -125,6 +141,7 @@ namespace Squidex.Infrastructure.EventSourcing
 
             using (await lockObject.EnterAsync(default))
             {
+                // Subscribing is not an atomar operation, therefore the lock.
                 Subscribe();
             }
         }

backend/src/Squidex.Infrastructure/States/NameReservationState.cs

@@ -9,6 +9,7 @@ namespace Squidex.Infrastructure.States
 {
     public sealed class NameReservationState : SimpleState<NameReservationState.State>
     {
+        [CollectionName("Names")]
         public sealed class State
         {
             public List<NameReservation> Reservations { get; set; } = new List<NameReservation>();

backend/tests/Squidex.Domain.Apps.Entities.Tests/Backup/BackupServiceTests.cs

@@ -20,7 +20,7 @@ namespace Squidex.Domain.Apps.Entities.Backup
     public class BackupServiceTests
     {
         private readonly TestState<BackupState> stateBackup;
-        private readonly TestState<RestoreJob> stateRestore;
+        private readonly TestState<BackupRestoreState> stateRestore;
         private readonly IMessageBus messaging = A.Fake<IMessageBus>();
         private readonly DomainId appId = DomainId.NewGuid();
         private readonly DomainId backupId = DomainId.NewGuid();
@@ -29,7 +29,7 @@ namespace Squidex.Domain.Apps.Entities.Backup
 
         public BackupServiceTests()
         {
-            stateRestore = new TestState<RestoreJob>("Default");
+            stateRestore = new TestState<BackupRestoreState>("Default");
             stateBackup = new TestState<BackupState>(appId);
 
             sut = new BackupService(
@@ -79,9 +79,12 @@ namespace Squidex.Domain.Apps.Entities.Backup
         [Fact]
         public async Task Should_throw_exception_when_restore_already_running()
         {
-            stateRestore.Snapshot = new RestoreJob
+            stateRestore.Snapshot = new BackupRestoreState
             {
-                Status = JobStatus.Started
+                Job = new RestoreJob
+                {
+                    Status = JobStatus.Started
+                }
             };
 
             var restoreUrl = new Uri("http://squidex.io");
@@ -114,14 +117,17 @@ namespace Squidex.Domain.Apps.Entities.Backup
         [Fact]
         public async Task Should_get_restore_state_from_store()
         {
-            stateRestore.Snapshot = new RestoreJob
+            stateRestore.Snapshot = new BackupRestoreState
             {
-                Stopped = SystemClock.Instance.GetCurrentInstant()
+                Job = new RestoreJob
+                {
+                    Stopped = SystemClock.Instance.GetCurrentInstant()
+                }
             };
 
             var result = await sut.GetRestoreAsync();
 
-            result.Should().BeEquivalentTo(stateRestore.Snapshot);
+            result.Should().BeEquivalentTo(stateRestore.Snapshot.Job);
         }
 
         [Fact]

backend/tests/Squidex.Infrastructure.Tests/EventSourcing/Consume/EventConsumerWorkerTests.cs

@@ -32,15 +32,15 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
         }
 
         [Fact]
-        public async Task Should_release_without_initialize()
+        public async Task Should_stop_without_start()
         {
-            await sut.ReleaseAsync(default);
+            await sut.StopAsync(default);
         }
 
         [Fact]
         public async Task Should_initialize_all_processors_on_initialize()
         {
-            await sut.InitializeAsync(default);
+            await sut.StartAsync(default);
 
             A.CallTo(() => processor1.InitializeAsync(default))
                 .MustHaveHappenedOnceExactly();
@@ -52,7 +52,7 @@ namespace Squidex.Infrastructure.EventSourcing.Consume
         [Fact]
         public async Task Should_activate_all_processors_on_initialize()
         {
-            await sut.InitializeAsync(default);
+            await sut.StartAsync(default);
 
             A.CallTo(() => processor1.ActivateAsync())
                 .MustHaveHappened();