Add MachineAlreadyExistsException and resolve race conditions in tests

psibr · Apr 4, 2018 · 1766644 · 1766644
1 parent d9e226c
commit 1766644
Show file tree

Hide file tree

Showing 26 changed files with 300 additions and 267 deletions.
diff --git a/...ate.Engine.Repositories.EntityFrameworkCore/EntityFrameworkCoreEngineRepositoryContext.cs b/...ate.Engine.Repositories.EntityFrameworkCore/EntityFrameworkCoreEngineRepositoryContext.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Data.SqlClient;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -151,7 +152,15 @@ public async Task<MachineStatus<TState, TInput>> CreateMachineAsync(
 
             DbContext.Machines.Add(record);
 
-            await DbContext.SaveChangesAsync(cancellationToken);
+            try
+            {
+                await DbContext.SaveChangesAsync(cancellationToken);
+            }
+            catch(DbUpdateException dbEx) 
+            when (dbEx.InnerException is SqlException sqlEx && sqlEx.Number == 2627)
+            {
+                throw new MachineAlreadyExistException(record.MachineId);
+            }
 
             return new MachineStatus<TState, TInput>
             {

diff --git a/...e.Repositories.EntityFrameworkCore/REstate.Engine.Repositories.EntityFrameworkCore.csproj b/...e.Repositories.EntityFrameworkCore/REstate.Engine.Repositories.EntityFrameworkCore.csproj
@@ -30,4 +30,10 @@
     <ProjectReference Include="..\REstate\REstate.csproj" />
   </ItemGroup>
 
+  <ItemGroup>
+    <Reference Include="System.Data.SqlClient">
+      <HintPath>..\..\..\..\..\..\..\Program Files\dotnet\sdk\NuGetFallbackFolder\system.data.sqlclient\4.4.0\ref\netstandard2.0\System.Data.SqlClient.dll</HintPath>
+    </Reference>
+  </ItemGroup>
+
 </Project>
diff --git a/src/REstate.Engine.Repositories.Redis/RedisEngineRepository.cs b/src/REstate.Engine.Repositories.Redis/RedisEngineRepository.cs
@@ -136,7 +136,7 @@ public async Task<MachineStatus<TState, TInput>> CreateMachineAsync(
 
             var recordBytes = MessagePackSerializer.Serialize(record);
 
-            await _restateDatabase.StringSetAsync($"{MachinesKeyPrefix}/{machineId}", recordBytes).ConfigureAwait(false);
+            await _restateDatabase.StringSetAsync($"{MachinesKeyPrefix}/{machineId}", recordBytes, null, When.NotExists).ConfigureAwait(false);
 
             return new MachineStatus<TState, TInput>
             {

diff --git a/src/REstate/Engine/EventListeners/LoggingEventListener.cs b/src/REstate/Engine/EventListeners/LoggingEventListener.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;

diff --git a/src/REstate/Engine/MachineAlreadyExistException.cs b/src/REstate/Engine/MachineAlreadyExistException.cs
@@ -0,0 +1,30 @@
+using System;
+
+namespace REstate.Engine
+{
+    /// <summary>
+    /// Indicates a request for a machine was made with an existing machineId
+    /// </summary>
+    public class MachineAlreadyExistException
+        : Exception
+    {
+        public string RequestedMachineId { get; }
+
+        public MachineAlreadyExistException(string machineId)
+            : this(machineId, $"A Machine already exists with MachineId matching {machineId}.")
+        {
+        }
+
+        public MachineAlreadyExistException(string machineId, string message)
+            : base(message)
+        {
+            RequestedMachineId = machineId;
+        }
+
+        public MachineAlreadyExistException(string machineId, string message, Exception innerException)
+            : base(message, innerException)
+        {
+            RequestedMachineId = machineId;
+        }
+    }
+}
diff --git a/src/REstate/Engine/Repositories/InMemory/EngineRepository.cs b/src/REstate/Engine/Repositories/InMemory/EngineRepository.cs
@@ -1,5 +1,7 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -13,10 +15,9 @@ public class EngineRepository<TState, TInput>
         : ISchematicRepository<TState, TInput>
         , IMachineRepository<TState, TInput>
     {
-        private IDictionary<string, Schematic<TState, TInput>> Schematics { get; } =
-            new Dictionary<string, Schematic<TState, TInput>>();
-        private IDictionary<string, (MachineStatus<TState, TInput> MachineStatus, Metadata Metadata)> Machines { get; } =
-            new Dictionary<string, (MachineStatus<TState, TInput>, Metadata)>();
+        private ConcurrentDictionary<string, Schematic<TState, TInput>> Schematics { get; } = new ConcurrentDictionary<string, Schematic<TState, TInput>>();
+        private ConcurrentDictionary<string, (MachineStatus<TState, TInput> MachineStatus, Metadata Metadata)> Machines { get; } =
+            new ConcurrentDictionary<string, (MachineStatus<TState, TInput>, Metadata)>();
 
         /// <summary>
         /// Retrieves a previously stored Schematic by name.
@@ -51,7 +52,7 @@ public Task<Schematic<TState, TInput>> StoreSchematicAsync(
             if (schematic == null) throw new ArgumentNullException(nameof(schematic));
             if (schematic.SchematicName == null) throw new ArgumentException("Schematic must have a name to be stored.", nameof(schematic));
 
-            Schematics.Add(schematic.SchematicName, schematic);
+            Schematics.AddOrUpdate(schematic.SchematicName, (key) => schematic, (key, old) => schematic);
 
             var storedSchematic = Schematics[schematic.SchematicName];
 
@@ -109,16 +110,21 @@ public Task<MachineStatus<TState, TInput>> CreateMachineAsync(
                 Metadata = metadata
             };
 
-            Machines.Add(id, (record, metadata));
+            if(Machines.TryAdd(id, (record, metadata)))
+                return Task.FromResult(record);
 
-            return Task.FromResult(record);
+            throw new MachineAlreadyExistException(id);
         }
 
         public Task<ICollection<MachineStatus<TState, TInput>>> BulkCreateMachinesAsync(
             Schematic<TState, TInput> schematic,
             IEnumerable<Metadata> metadata,
             CancellationToken cancellationToken = new CancellationToken())
         {
+            var exceptions = new Queue<Exception>();
+
+            var aggregateException = new AggregateException(exceptions);
+
             List<(MachineStatus<TState, TInput> MachineStatus, IDictionary<string, string> Metadata)> machineRecords =
                 metadata.Select(meta =>
                     (new MachineStatus<TState, TInput>
@@ -134,9 +140,13 @@ public Task<ICollection<MachineStatus<TState, TInput>>> BulkCreateMachinesAsync(
 
             foreach (var machineRecord in machineRecords)
             {
-                Machines.Add(machineRecord.MachineStatus.MachineId, machineRecord);
+                if (!Machines.TryAdd(machineRecord.MachineStatus.MachineId, machineRecord))
+                    exceptions.Enqueue(new MachineAlreadyExistException(machineRecord.MachineStatus.MachineId));
             }
 
+            if(exceptions.Count > 0)
+                throw aggregateException;
+
             return Task.FromResult<ICollection<MachineStatus<TState, TInput>>>(
                 machineRecords.Select(r => r.MachineStatus).ToList());
         }
@@ -166,7 +176,7 @@ public Task DeleteMachineAsync(
         {
             if (machineId == null) throw new ArgumentNullException(nameof(machineId));
 
-            Machines.Remove(machineId);
+            Machines.TryRemove(machineId, out var _);
 
 #if NET45
             return Task.FromResult(0);

diff --git a/src/REstate/Engine/Repositories/InMemory/RepositoryContextFactory.cs b/src/REstate/Engine/Repositories/InMemory/RepositoryContextFactory.cs
@@ -9,7 +9,7 @@ public class InMemoryRepositoryContextFactory<TState, TInput>
     {
         private Lazy<IEngineRepositoryContext<TState, TInput>> repositoryContextLazy 
             = new Lazy<IEngineRepositoryContext<TState, TInput>>(() 
-                => new EngineRepositoryContext<TState, TInput>());
+                => new EngineRepositoryContext<TState, TInput>(), true);
 
         public Task<IEngineRepositoryContext<TState, TInput>> OpenContextAsync(CancellationToken cancellationToken = default)
         {

diff --git a/test/REstate.Engine.Repositories.EntityFrameworkCore.Tests/Features/MachineCreation.cs b/test/REstate.Engine.Repositories.EntityFrameworkCore.Tests/Features/MachineCreation.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Engine.Repositories.EntityFrameworkCore.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.Engine.Repositories.EntityFrameworkCore.Tests/Features/MachineDeletion.cs b/test/REstate.Engine.Repositories.EntityFrameworkCore.Tests/Features/MachineDeletion.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Engine.Repositories.EntityFrameworkCore.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.Engine.Repositories.EntityFrameworkCore.Tests/Features/MachineRetrieval.cs b/test/REstate.Engine.Repositories.EntityFrameworkCore.Tests/Features/MachineRetrieval.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Engine.Repositories.EntityFrameworkCore.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.IoC.Ninject.Tests/Features/MachineCreation.cs b/test/REstate.IoC.Ninject.Tests/Features/MachineCreation.cs
@@ -4,7 +4,7 @@
 using LightBDD.Framework.Scenarios.Extended;
 using Ninject;
 using REstate.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.IoC.Ninject.Tests/Features/MachineDeletion.cs b/test/REstate.IoC.Ninject.Tests/Features/MachineDeletion.cs
@@ -5,7 +5,7 @@
 using Ninject;
 using REstate.IoC.Ninject;
 using REstate.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.IoC.Ninject.Tests/Features/MachineRetrieval.cs b/test/REstate.IoC.Ninject.Tests/Features/MachineRetrieval.cs
@@ -5,7 +5,7 @@
 using Ninject;
 using REstate.IoC.Ninject;
 using REstate.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.Remote.Tests/Features/GrpcServer.cs b/test/REstate.Remote.Tests/Features/GrpcServer.cs
@@ -4,7 +4,7 @@
 using LightBDD.Framework.Scenarios.Extended;
 using LightBDD.XUnit2;
 using REstate.Remote.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 using Xunit;
 
 namespace REstate.Remote.Tests.Features

diff --git a/test/REstate.Remote.Tests/Features/MachineCreation.cs b/test/REstate.Remote.Tests/Features/MachineCreation.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Remote.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.Remote.Tests/Features/MachineDeletion.cs b/test/REstate.Remote.Tests/Features/MachineDeletion.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Remote.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.Remote.Tests/Features/MachineRetrieval.cs b/test/REstate.Remote.Tests/Features/MachineRetrieval.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Remote.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 

diff --git a/test/REstate.Remote.Tests/Features/Schematics.cs b/test/REstate.Remote.Tests/Features/Schematics.cs
@@ -3,7 +3,7 @@
 using LightBDD.Framework.Scenarios.Contextual;
 using LightBDD.Framework.Scenarios.Extended;
 using REstate.Remote.Tests.Features.Context;
-using REstate.Tests.Features.Templates;
+using REstate.Tests.Features;
 
 // ReSharper disable InconsistentNaming
 
@@ -17,7 +17,7 @@ As a developer
     [ScenarioCategory("Remote")]
     [ScenarioCategory("gRPC")]
     public class Schematics
-        : SchematicScenarios<REstateRemoteContext<string, string>>
+        : SchematicsScenarios<REstateRemoteContext<string, string>>
     {
         protected override Task<CompositeStep> Given_host_configuration_is_applied()
         {