Fix unique indexes. (#829)

* Fix unique indexes.

* Tests for uniqueness.

* Improved tests.

* Save all files.

backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsCacheGrain.cs

@@ -23,6 +23,26 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
             this.appRepository = appRepository;
         }
 
+        public override async Task<string?> ReserveAsync(DomainId id, string name)
+        {
+            var token = await base.ReserveAsync(id, name);
+
+            if (token == null)
+            {
+                return null;
+            }
+
+            var ids = await GetAppIdsAsync(new[] { name });
+
+            if (ids.Any())
+            {
+                await RemoveReservationAsync(token);
+                return null;
+            }
+
+            return token;
+        }
+
         public async Task<IReadOnlyCollection<DomainId>> GetAppIdsAsync(string[] names)
         {
             var result = new List<DomainId>();

backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsIndex.cs

@@ -40,10 +40,10 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
             return Cache().RemoveReservationAsync(token);
         }
 
-        public Task<string?> ReserveAsync(DomainId id, string name,
+        public async Task<string?> ReserveAsync(DomainId id, string name,
             CancellationToken ct = default)
         {
-            return Cache().ReserveAsync(id, name);
+            return await Cache().ReserveAsync(id, name);
         }
 
         public async Task<List<IAppEntity>> GetAppsForUserAsync(string userId, PermissionSet permissions,
@@ -183,7 +183,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
             }
         }
 
-        private async Task<string?> CheckAppAsync(IAppsCacheGrain cache, CreateApp command)
+        private static async Task<string?> CheckAppAsync(IAppsCacheGrain cache, CreateApp command)
         {
             var token = await cache.ReserveAsync(command.AppId, command.Name);
 
@@ -192,22 +192,6 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
                 throw new ValidationException(T.Get("apps.nameAlreadyExists"));
             }
 
-            try
-            {
-                var existingId = await GetAppIdAsync(command.Name);
-
-                if (existingId != default)
-                {
-                    throw new ValidationException(T.Get("apps.nameAlreadyExists"));
-                }
-            }
-            catch
-            {
-                // Catch our own exception, just in case something went wrong before.
-                await cache.RemoveReservationAsync(token);
-                throw;
-            }
-
             return token;
         }
 

backend/src/Squidex.Domain.Apps.Entities/Schemas/Indexes/SchemasCacheGrain.cs

@@ -25,6 +25,26 @@ namespace Squidex.Domain.Apps.Entities.Schemas.Indexes
             this.schemaRepository = schemaRepository;
         }
 
+        public override async Task<string?> ReserveAsync(DomainId id, string name)
+        {
+            var token = await base.ReserveAsync(id, name);
+
+            if (token == null)
+            {
+                return null;
+            }
+
+            var ids = await GetIdsAsync();
+
+            if (ids.ContainsKey(name))
+            {
+                await RemoveReservationAsync(token);
+                return null;
+            }
+
+            return token;
+        }
+
         public async Task<IReadOnlyCollection<DomainId>> GetSchemaIdsAsync()
         {
             var ids = await GetIdsAsync();

backend/src/Squidex.Infrastructure/Orleans/Indexes/UniqueNameGrain.cs

@@ -11,7 +11,7 @@ namespace Squidex.Infrastructure.Orleans.Indexes
     {
         private readonly Dictionary<string, (string Name, T Id)> reservations = new Dictionary<string, (string Name, T Id)>();
 
-        public Task<string?> ReserveAsync(T id, string name)
+        public virtual Task<string?> ReserveAsync(T id, string name)
         {
             string? token = null;
 

backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsCacheGrainTests.cs

@@ -25,6 +25,27 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
             sut.ActivateAsync(appId.ToString()).Wait();
         }
 
+        [Fact]
+        public async Task Should_not_reserve_name_if_already_used()
+        {
+            var ids1 = new Dictionary<string, DomainId>
+            {
+                ["name1"] = DomainId.NewGuid()
+            };
+
+            A.CallTo(() => appRepository.QueryIdsAsync(A<IEnumerable<string>>.That.Is("name1"), default))
+                .Returns(ids1);
+
+            A.CallTo(() => appRepository.QueryIdsAsync(A<IEnumerable<string>>.That.Is("name2"), default))
+                .Returns(new Dictionary<string, DomainId>());
+
+            var token1 = await sut.ReserveAsync(DomainId.NewGuid(), "name1");
+            var token2 = await sut.ReserveAsync(DomainId.NewGuid(), "name2");
+
+            Assert.Null(token1);
+            Assert.NotNull(token2);
+        }
+
         [Fact]
         public async Task Should_provide_app_ids_from_repository_once()
         {

backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsIndexTests.cs

@@ -263,32 +263,6 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
                 .MustNotHaveHappened();
         }
 
-        [Fact]
-        public async Task Should_not_add_to_indexes_if_name_is_taken()
-        {
-            var token = RandomHash.Simple();
-
-            A.CallTo(() => cache.ReserveAsync(appId.Id, appId.Name))
-                .Returns(token);
-
-            A.CallTo(() => cache.GetAppIdsAsync(A<string[]>.That.Is(appId.Name)))
-                .Returns(new List<DomainId> { appId.Id });
-
-            var command = Create(appId.Name);
-
-            var context =
-                new CommandContext(command, commandBus)
-                    .Complete();
-
-            await Assert.ThrowsAsync<ValidationException>(() => sut.HandleAsync(context));
-
-            A.CallTo(() => cache.AddAsync(A<DomainId>._, A<string>._))
-                .MustNotHaveHappened();
-
-            A.CallTo(() => cache.RemoveReservationAsync(token))
-                .MustHaveHappened();
-        }
-
         [Fact]
         public async Task Should_update_index_with_result_if_app_is_updated()
         {
@@ -326,7 +300,12 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes
         [Fact]
         public async Task Should_forward_reserveration()
         {
-            await sut.ReserveAsync(appId.Id, appId.Name);
+            A.CallTo(() => cache.ReserveAsync(appId.Id, appId.Name))
+                .Returns("token");
+
+            var token = await sut.ReserveAsync(appId.Id, appId.Name);
+
+            Assert.Equal("token", token);
 
             A.CallTo(() => cache.ReserveAsync(appId.Id, appId.Name))
                 .MustHaveHappened();

backend/tests/Squidex.Domain.Apps.Entities.Tests/Schemas/Indexes/SchemasCacheGrainTests.cs

@@ -24,6 +24,24 @@ namespace Squidex.Domain.Apps.Entities.Schemas.Indexes
             sut.ActivateAsync(appId.ToString()).Wait();
         }
 
+        [Fact]
+        public async Task Should_not_reserve_name_if_already_used()
+        {
+            var ids = new Dictionary<string, DomainId>
+            {
+                ["name1"] = DomainId.NewGuid()
+            };
+
+            A.CallTo(() => schemaRepository.QueryIdsAsync(appId, default))
+                .Returns(ids);
+
+            var token1 = await sut.ReserveAsync(DomainId.NewGuid(), "name1");
+            var token2 = await sut.ReserveAsync(DomainId.NewGuid(), "name2");
+
+            Assert.Null(token1);
+            Assert.NotNull(token2);
+        }
+
         [Fact]
         public async Task Should_provide_schema_ids_from_repository_once()
         {

backend/tools/TestSuite/TestSuite.ApiTests/AppContributorsTests.cs

@@ -36,7 +36,7 @@ namespace TestSuite.ApiTests
             // STEP 1:  Do not invite contributors when flag is false.
             var createRequest = new AssignContributorDto { ContributorId = "test@squidex.io" };
 
-            var ex = await Assert.ThrowsAsync<SquidexManagementException>(() =>
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() =>
             {
                 return _.Apps.PostContributorAsync(appName, createRequest);
             });

backend/tools/TestSuite/TestSuite.ApiTests/AppCreationTests.cs

@@ -58,6 +58,23 @@ namespace TestSuite.ApiTests
             Assert.Contains(clients.Items, x => x.Id == "default");
         }
 
+        [Fact]
+        public async Task Should_not_allow_creation_if_name_used()
+        {
+            var appName = Guid.NewGuid().ToString();
+
+            // STEP 1: Create app
+            var createRequest = new CreateAppDto { Name = appName };
+
+            await _.Apps.PostAppAsync(createRequest);
+
+
+            // STEP 2: Create again and fail
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() => _.Apps.PostAppAsync(createRequest));
+
+            Assert.Equal(400, ex.StatusCode);
+        }
+
         [Fact]
         public async Task Should_archive_app()
         {

backend/tools/TestSuite/TestSuite.ApiTests/AppRolesTests.cs

@@ -88,7 +88,7 @@ namespace TestSuite.ApiTests
 
 
             // STEP 4: Try to delete role.
-            var ex = await Assert.ThrowsAsync<SquidexManagementException<ErrorDto>>(() =>
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() =>
             {
                 return _.Apps.DeleteRoleAsync(_.AppName, roleName);
             });
@@ -123,7 +123,7 @@ namespace TestSuite.ApiTests
 
 
             // STEP 4: Try to delete role.
-            var ex = await Assert.ThrowsAsync<SquidexManagementException<ErrorDto>>(() =>
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() =>
             {
                 return _.Apps.DeleteRoleAsync(_.AppName, roleName);
             });

backend/tools/TestSuite/TestSuite.ApiTests/AppTests.cs

@@ -160,10 +160,7 @@ namespace TestSuite.ApiTests
 
 
             // STEP 4:  Try to delete role.
-            var ex = await Assert.ThrowsAsync<SquidexManagementException<ErrorDto>>(() =>
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() => _.Apps.DeleteRoleAsync(_.AppName, roleName));
-            {
-                return _.Apps.DeleteRoleAsync(_.AppName, roleName);
-            });
 
             Assert.Equal(400, ex.StatusCode);
 

backend/tools/TestSuite/TestSuite.ApiTests/AssetTests.cs

@@ -60,7 +60,7 @@ namespace TestSuite.ApiTests
 
 
             // STEP 2: Create a new item with a custom id.
-            var ex = await Assert.ThrowsAsync<SquidexManagementException>(() => _.UploadFileAsync("Assets/logo-squared.png", "image/png", id: id));
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() => _.UploadFileAsync("Assets/logo-squared.png", "image/png", id: id));
 
             Assert.Equal(409, ex.StatusCode);
         }
@@ -182,7 +182,7 @@ namespace TestSuite.ApiTests
             // STEP 5: Download asset without key.
             await using (var stream = new FileStream("Assets/logo-squared.png", FileMode.Open))
             {
-                var ex = await Assert.ThrowsAsync<HttpRequestException>(() => _.DownloadAsync(asset_1));
+                var ex = await Assert.ThrowsAnyAsync<HttpRequestException>(() => _.DownloadAsync(asset_1));
 
                 // Should return 403 when not authenticated.
                 Assert.Contains("403", ex.Message, StringComparison.Ordinal);
@@ -192,7 +192,7 @@ namespace TestSuite.ApiTests
             // STEP 6: Download asset without key and version.
             await using (var stream = new FileStream("Assets/logo-squared.png", FileMode.Open))
             {
-                var ex = await Assert.ThrowsAsync<HttpRequestException>(() => _.DownloadAsync(asset_1, 0));
+                var ex = await Assert.ThrowsAnyAsync<HttpRequestException>(() => _.DownloadAsync(asset_1, 0));
 
                 // Should return 403 when not authenticated.
                 Assert.Contains("403", ex.Message, StringComparison.Ordinal);
@@ -318,7 +318,7 @@ namespace TestSuite.ApiTests
             await _.Assets.DeleteAssetAsync(_.AppName, asset.Id, permanent: permanent);
 
             // Should return 404 when asset deleted.
-            var ex = await Assert.ThrowsAsync<SquidexManagementException>(() => _.Assets.GetAssetAsync(_.AppName, asset.Id));
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() => _.Assets.GetAssetAsync(_.AppName, asset.Id));
 
             Assert.Equal(404, ex.StatusCode);
 

backend/tools/TestSuite/TestSuite.ApiTests/ContentReferencesTests.cs

@@ -71,7 +71,7 @@ namespace TestSuite.ApiTests
 
 
             // STEP 3: Try to delete with referrer check.
-            await Assert.ThrowsAsync<SquidexException>(() => _.Contents.DeleteAsync(contentA_1.Id, checkReferrers: true));
+            await Assert.ThrowsAnyAsync<SquidexException>(() => _.Contents.DeleteAsync(contentA_1.Id, checkReferrers: true));
 
 
             // STEP 4: Delete without referrer check
@@ -93,8 +93,8 @@ namespace TestSuite.ApiTests
             await _.Contents.CreateAsync(dataB, true);
 
 
-            // STEP 3: Try to delete with referrer check.
+            // STEP 3: Try to ThrowsAnyAsync with referrer check.
-            await Assert.ThrowsAsync<SquidexException>(() => _.Contents.ChangeStatusAsync(contentA_1.Id, new ChangeStatus
+            await Assert.ThrowsAnyAsync<SquidexException>(() => _.Contents.ChangeStatusAsync(contentA_1.Id, new ChangeStatus
             {
                 Status = "Draft",
                 CheckReferrers = true

backend/tools/TestSuite/TestSuite.ApiTests/ContentUpdateTests.cs

@@ -68,7 +68,7 @@ namespace TestSuite.ApiTests
 
 
                 // STEP 3. Get a 404 for the item because it is not published anymore.
-                await Assert.ThrowsAsync<SquidexException>(() => _.Contents.GetAsync(content.Id));
+                await Assert.ThrowsAnyAsync<SquidexException>(() => _.Contents.GetAsync(content.Id));
             }
             finally
             {
@@ -95,7 +95,7 @@ namespace TestSuite.ApiTests
 
 
                 // STEP 3. Get a 404 for the item because it is not published anymore.
-                await Assert.ThrowsAsync<SquidexException>(() => _.Contents.GetAsync(content.Id));
+                await Assert.ThrowsAnyAsync<SquidexException>(() => _.Contents.GetAsync(content.Id));
             }
             finally
             {
@@ -309,7 +309,7 @@ namespace TestSuite.ApiTests
 
 
                 // STEP 2. Get a 404 for the item because it is not published.
-                await Assert.ThrowsAsync<SquidexException>(() => _.Contents.GetAsync(content.Id));
+                await Assert.ThrowsAnyAsync<SquidexException>(() => _.Contents.GetAsync(content.Id));
             }
             finally
             {
@@ -379,7 +379,7 @@ namespace TestSuite.ApiTests
 
 
                 // STEP 2: Create a new item with a custom id.
-                var ex = await Assert.ThrowsAsync<SquidexException>(() => _.Contents.CreateAsync(new TestEntityData { Number1 = 1 }, id, true));
+                var ex = await Assert.ThrowsAnyAsync<SquidexException>(() => _.Contents.CreateAsync(new TestEntityData { Number1 = 1 }, id, true));
 
                 Assert.Contains("\"statusCode\":409", ex.Message, StringComparison.Ordinal);
             }

backend/tools/TestSuite/TestSuite.ApiTests/SchemaTests.cs

@@ -45,6 +45,23 @@ namespace TestSuite.ApiTests
             Assert.Contains(schemas.Items, x => x.Name == schemaName);
         }
 
+        [Fact]
+        public async Task Should_not_allow_creation_if_name_used()
+        {
+            var schemaName = $"schema-{Guid.NewGuid()}";
+
+            // STEP 1: Create schema
+            var createRequest = new CreateSchemaDto { Name = schemaName };
+
+            var schema = await _.Schemas.PostSchemaAsync(_.AppName, createRequest);
+
+
+            // STEP 2: Create again and fail
+            var ex = await Assert.ThrowsAnyAsync<SquidexManagementException>(() => _.Schemas.PostSchemaAsync(_.AppName, createRequest));
+
+            Assert.Equal(400, ex.StatusCode);
+        }
+
         [Fact]
         public async Task Should_create_singleton_schema()
         {