diff --git a/backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsCacheGrain.cs b/backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsCacheGrain.cs index 1da1adce0..bb89c2a7d 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsCacheGrain.cs +++ b/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 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> GetAppIdsAsync(string[] names) { var result = new List(); diff --git a/backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsIndex.cs b/backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsIndex.cs index d4c10a566..c0a6be9e5 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Apps/Indexes/AppsIndex.cs +++ b/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 ReserveAsync(DomainId id, string name, + public async Task ReserveAsync(DomainId id, string name, CancellationToken ct = default) { - return Cache().ReserveAsync(id, name); + return await Cache().ReserveAsync(id, name); } public async Task> GetAppsForUserAsync(string userId, PermissionSet permissions, @@ -183,7 +183,7 @@ namespace Squidex.Domain.Apps.Entities.Apps.Indexes } } - private async Task CheckAppAsync(IAppsCacheGrain cache, CreateApp command) + private static async Task 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; } diff --git a/backend/src/Squidex.Domain.Apps.Entities/Schemas/Indexes/SchemasCacheGrain.cs b/backend/src/Squidex.Domain.Apps.Entities/Schemas/Indexes/SchemasCacheGrain.cs index e8016e1a4..a72909037 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Schemas/Indexes/SchemasCacheGrain.cs +++ b/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 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> GetSchemaIdsAsync() { var ids = await GetIdsAsync(); diff --git a/backend/src/Squidex.Infrastructure/Orleans/Indexes/UniqueNameGrain.cs b/backend/src/Squidex.Infrastructure/Orleans/Indexes/UniqueNameGrain.cs index 49b08847b..517dd2d90 100644 --- a/backend/src/Squidex.Infrastructure/Orleans/Indexes/UniqueNameGrain.cs +++ b/backend/src/Squidex.Infrastructure/Orleans/Indexes/UniqueNameGrain.cs @@ -11,7 +11,7 @@ namespace Squidex.Infrastructure.Orleans.Indexes { private readonly Dictionary reservations = new Dictionary(); - public Task ReserveAsync(T id, string name) + public virtual Task ReserveAsync(T id, string name) { string? token = null; diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsCacheGrainTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsCacheGrainTests.cs index 6666d619f..f39e26e48 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsCacheGrainTests.cs +++ b/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 + { + ["name1"] = DomainId.NewGuid() + }; + + A.CallTo(() => appRepository.QueryIdsAsync(A>.That.Is("name1"), default)) + .Returns(ids1); + + A.CallTo(() => appRepository.QueryIdsAsync(A>.That.Is("name2"), default)) + .Returns(new Dictionary()); + + 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() { diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsIndexTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsIndexTests.cs index aaee61937..db5df2433 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Apps/Indexes/AppsIndexTests.cs +++ b/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.That.Is(appId.Name))) - .Returns(new List { appId.Id }); - - var command = Create(appId.Name); - - var context = - new CommandContext(command, commandBus) - .Complete(); - - await Assert.ThrowsAsync(() => sut.HandleAsync(context)); - - A.CallTo(() => cache.AddAsync(A._, A._)) - .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(); diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Schemas/Indexes/SchemasCacheGrainTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Schemas/Indexes/SchemasCacheGrainTests.cs index 371f2280c..2b9792853 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Schemas/Indexes/SchemasCacheGrainTests.cs +++ b/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 + { + ["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() { diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/AppContributorsTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/AppContributorsTests.cs index 855acde79..cda5d7c8a 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/AppContributorsTests.cs +++ b/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(() => + var ex = await Assert.ThrowsAnyAsync(() => { return _.Apps.PostContributorAsync(appName, createRequest); }); diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/AppCreationTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/AppCreationTests.cs index 0d3e18f99..88c2ad87b 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/AppCreationTests.cs +++ b/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(() => _.Apps.PostAppAsync(createRequest)); + + Assert.Equal(400, ex.StatusCode); + } + [Fact] public async Task Should_archive_app() { diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/AppRolesTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/AppRolesTests.cs index 663d63c68..7ffa57c6d 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/AppRolesTests.cs +++ b/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>(() => + var ex = await Assert.ThrowsAnyAsync(() => { return _.Apps.DeleteRoleAsync(_.AppName, roleName); }); @@ -123,7 +123,7 @@ namespace TestSuite.ApiTests // STEP 4: Try to delete role. - var ex = await Assert.ThrowsAsync>(() => + var ex = await Assert.ThrowsAnyAsync(() => { return _.Apps.DeleteRoleAsync(_.AppName, roleName); }); diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/AppTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/AppTests.cs index 332da4031..eae2d7e75 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/AppTests.cs +++ b/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>(() => - { - return _.Apps.DeleteRoleAsync(_.AppName, roleName); - }); + var ex = await Assert.ThrowsAnyAsync(() => _.Apps.DeleteRoleAsync(_.AppName, roleName)); Assert.Equal(400, ex.StatusCode); diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/AssetTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/AssetTests.cs index 06e790026..c549e11ad 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/AssetTests.cs +++ b/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(() => _.UploadFileAsync("Assets/logo-squared.png", "image/png", id: id)); + var ex = await Assert.ThrowsAnyAsync(() => _.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(() => _.DownloadAsync(asset_1)); + var ex = await Assert.ThrowsAnyAsync(() => _.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(() => _.DownloadAsync(asset_1, 0)); + var ex = await Assert.ThrowsAnyAsync(() => _.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(() => _.Assets.GetAssetAsync(_.AppName, asset.Id)); + var ex = await Assert.ThrowsAnyAsync(() => _.Assets.GetAssetAsync(_.AppName, asset.Id)); Assert.Equal(404, ex.StatusCode); diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/ContentReferencesTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/ContentReferencesTests.cs index f45b80865..e88c99ee8 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/ContentReferencesTests.cs +++ b/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(() => _.Contents.DeleteAsync(contentA_1.Id, checkReferrers: true)); + await Assert.ThrowsAnyAsync(() => _.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. - await Assert.ThrowsAsync(() => _.Contents.ChangeStatusAsync(contentA_1.Id, new ChangeStatus + // STEP 3: Try to ThrowsAnyAsync with referrer check. + await Assert.ThrowsAnyAsync(() => _.Contents.ChangeStatusAsync(contentA_1.Id, new ChangeStatus { Status = "Draft", CheckReferrers = true diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/ContentUpdateTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/ContentUpdateTests.cs index 830e98e83..42a0933c5 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/ContentUpdateTests.cs +++ b/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(() => _.Contents.GetAsync(content.Id)); + await Assert.ThrowsAnyAsync(() => _.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(() => _.Contents.GetAsync(content.Id)); + await Assert.ThrowsAnyAsync(() => _.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(() => _.Contents.GetAsync(content.Id)); + await Assert.ThrowsAnyAsync(() => _.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(() => _.Contents.CreateAsync(new TestEntityData { Number1 = 1 }, id, true)); + var ex = await Assert.ThrowsAnyAsync(() => _.Contents.CreateAsync(new TestEntityData { Number1 = 1 }, id, true)); Assert.Contains("\"statusCode\":409", ex.Message, StringComparison.Ordinal); } diff --git a/backend/tools/TestSuite/TestSuite.ApiTests/SchemaTests.cs b/backend/tools/TestSuite/TestSuite.ApiTests/SchemaTests.cs index 4571c2b61..6121e7098 100644 --- a/backend/tools/TestSuite/TestSuite.ApiTests/SchemaTests.cs +++ b/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(() => _.Schemas.PostSchemaAsync(_.AppName, createRequest)); + + Assert.Equal(400, ex.StatusCode); + } + [Fact] public async Task Should_create_singleton_schema() {