diff --git a/backend/src/Squidex.Domain.Apps.Entities/Tags/TagService.cs b/backend/src/Squidex.Domain.Apps.Entities/Tags/TagService.cs index b33ae7e9d..6b8111694 100644 --- a/backend/src/Squidex.Domain.Apps.Entities/Tags/TagService.cs +++ b/backend/src/Squidex.Domain.Apps.Entities/Tags/TagService.cs @@ -22,15 +22,11 @@ namespace Squidex.Domain.Apps.Entities.Tags { public ValueTask OnReadAsync() { - if (Tags == null) - { - Tags = new Dictionary(); - } + // Tags should never be null, but it might happen due of bugs. + Tags ??= new Dictionary(); - if (Alias == null) - { - Alias = new Dictionary(); - } + // Alias can be null, because it was not part of the initial release. + Alias ??= new Dictionary(); return default; } @@ -54,6 +50,7 @@ namespace Squidex.Domain.Apps.Entities.Tags { var isChanged = false; + // Clear only resets the counts to zero, because we have no other source for tag names. foreach (var (_, tag) in Tags) { isChanged = tag.Count > 0; @@ -68,7 +65,7 @@ namespace Squidex.Domain.Apps.Entities.Tags { name = NormalizeName(name); - if (!TryGetTag(name, out var tag)) + if (!TryGetTag(name, out var tag, false)) { return false; } @@ -81,7 +78,7 @@ namespace Squidex.Domain.Apps.Entities.Tags return false; } - if (TryGetTag(newName, out var newTag)) + if (TryGetTag(newName, out var newTag, false)) { // Merge both tags by adding up the count. newTag.Info.Count += tag.Info.Count; @@ -96,14 +93,19 @@ namespace Squidex.Domain.Apps.Entities.Tags foreach (var alias in Alias.Where(x => x.Value == name).ToList()) { + // Remove the mapping to the old name. Alias.Remove(alias.Key); - if (alias.Key != name) + if (alias.Key != newName) { - Alias[alias.Key] = name; + // Create a new mapping to the new name. + Alias[alias.Key] = newName; } } + // Create a new mapping to the new name. + Alias[name] = newName; + return true; } @@ -139,15 +141,18 @@ namespace Squidex.Domain.Apps.Entities.Tags { if (TryGetTag(name, out var tag)) { + // If the tag exists, return the ID. tagIds[name] = tag.Id; } else { + // If the tag does not exist create a new one with a random ID. var id = Guid.NewGuid().ToString(); Tags[id] = new Tag { Name = name }; tagIds[name] = id; + // Track that something has changed and the state needs to be written. isChanged = true; } } @@ -191,11 +196,12 @@ namespace Squidex.Domain.Apps.Entities.Tags return name.TrimNonLetterOrDigit().ToLowerInvariant(); } - private bool TryGetTag(string name, out (string Id, Tag Info)result) + private bool TryGetTag(string name, out (string Id, Tag Info)result, bool useAlias = true) { result = default!; - if (Alias.TryGetValue(name, out var newName)) + // If the tag has been renamed we create a mapping from the old name to the new name. + if (useAlias && Alias.TryGetValue(name, out var newName)) { name = newName; } diff --git a/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/Field.cs b/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/Field.cs index 7b73cc054..ef0a283ea 100644 --- a/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/Field.cs +++ b/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/Field.cs @@ -15,7 +15,23 @@ namespace Squidex.Infrastructure.MongoDb { var name = mapper(default!); - return BsonClassMap.LookupClassMap(typeof(T)).GetMemberMap(name).ElementName; + var classMap = BsonClassMap.LookupClassMap(typeof(T)); + + // The class map does not contain all inherited members, therefore we have to loop over the hierarchy. + while (classMap != null) + { + var member = classMap.GetMemberMap(name); + + if (member != null) + { + return member.ElementName; + } + + classMap = classMap.BaseClassMap; + } + + ThrowHelper.InvalidOperationException($"Cannot find member '{name}' in type '{typeof(T)}."); + return null!; } } } diff --git a/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/MongoExtensions.cs b/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/MongoExtensions.cs index 0326fdb5c..a759e9e0e 100644 --- a/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/MongoExtensions.cs +++ b/backend/src/Squidex.Infrastructure.MongoDb/MongoDb/MongoExtensions.cs @@ -123,6 +123,8 @@ namespace Squidex.Infrastructure.MongoDb CancellationToken ct = default) where T : IVersionedEntity { + var field2 = Field.Of(x => nameof(x.Version)); + var (key, snapshot, newVersion, oldVersion) = job; try { @@ -161,6 +163,8 @@ namespace Squidex.Infrastructure.MongoDb CancellationToken ct = default) where T : IVersionedEntity { + var field2 = Field.Of(x => nameof(x.Version)); + var (key, snapshot, newVersion, oldVersion) = job; try { diff --git a/backend/src/Squidex/Squidex.csproj b/backend/src/Squidex/Squidex.csproj index 83f6b6df4..516807666 100644 --- a/backend/src/Squidex/Squidex.csproj +++ b/backend/src/Squidex/Squidex.csproj @@ -42,7 +42,6 @@ all runtime; build; native; contentfiles; analyzers; buildtransitive - diff --git a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Tags/TagServiceTests.cs b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Tags/TagServiceTests.cs index 6db90c0a7..6d2e0fffd 100644 --- a/backend/tests/Squidex.Domain.Apps.Entities.Tests/Tags/TagServiceTests.cs +++ b/backend/tests/Squidex.Domain.Apps.Entities.Tests/Tags/TagServiceTests.cs @@ -12,8 +12,6 @@ using Squidex.Infrastructure; using Squidex.Infrastructure.TestHelpers; using Xunit; -#pragma warning disable IDE0059 // Unnecessary assignment of a value - namespace Squidex.Domain.Apps.Entities.Tags { public class TagServiceTests @@ -81,48 +79,73 @@ namespace Squidex.Domain.Apps.Entities.Tags [Fact] public async Task Should_rename_tag() { - var ids_0 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag1"), ct); + var ids_0 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_0"), ct); - await sut.RenameTagAsync(appId, group, "tag1", "tag1_new", ct); + await sut.RenameTagAsync(appId, group, "tag_0", "tag_1", ct); - // Forward the old name to the new name. - var ids_1 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag1_new"), ct); + // Both names should map to the same tag. + var ids_1 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_0"), ct); + var ids_2 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_1"), ct); + + Assert.Equal(ids_0.Values, ids_1.Values); + Assert.Equal(ids_0.Values, ids_2.Values); var allTags = await sut.GetTagsAsync(appId, group, ct); - Assert.Equal(ids_0.Values, ids_1.Values); + Assert.Equal(new Dictionary + { + ["tag_1"] = 0 + }, allTags); } [Fact] public async Task Should_rename_tag_twice() { - var ids_0 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag1"), ct); - - await sut.RenameTagAsync(appId, group, "tag1", "tag2", ct); - await sut.RenameTagAsync(appId, group, "tag2", "tag3", ct); + var ids_0 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_0"), ct); // Forward the old name to the new name. - var ids_1 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag2"), ct); - var ids_2 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag3"), ct); + await sut.RenameTagAsync(appId, group, "tag_0", "tag_1", ct); + await sut.RenameTagAsync(appId, group, "tag_1", "tag_2", ct); - // Assert.Equal(ids_0.Values, ids_1.Values); + // All names should map to the same tag. + var ids_1 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_0"), ct); + var ids_2 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_1"), ct); + var ids_3 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_2"), ct); + + Assert.Equal(ids_0.Values, ids_1.Values); Assert.Equal(ids_0.Values, ids_2.Values); + Assert.Equal(ids_0.Values, ids_3.Values); + + var allTags = await sut.GetTagsAsync(appId, group, ct); + + Assert.Equal(new Dictionary + { + ["tag_2"] = 0 + }, allTags); } [Fact] public async Task Should_rename_tag_back() { - var ids_0 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag1"), ct); - - await sut.RenameTagAsync(appId, group, "tag1", "tag2", ct); - - // Rename back. - await sut.RenameTagAsync(appId, group, "tag2", "tag1", ct); + var ids_0 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_0"), ct); // Forward the old name to the new name. - var ids_1 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag1"), ct); + await sut.RenameTagAsync(appId, group, "tag_0", "tag_1", ct); + await sut.RenameTagAsync(appId, group, "tag_1", "tag_0", ct); + + // All names should map to the same tag. + var ids_1 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_0"), ct); + var ids_2 = await sut.GetTagIdsAsync(appId, group, HashSet.Of("tag_1"), ct); Assert.Equal(ids_0.Values, ids_1.Values); + Assert.Equal(ids_0.Values, ids_2.Values); + + var allTags = await sut.GetTagsAsync(appId, group, ct); + + Assert.Equal(new Dictionary + { + ["tag_0"] = 0 + }, allTags); } [Fact] diff --git a/backend/tests/Squidex.Infrastructure.Tests/MongoDb/MongoFieldTests.cs b/backend/tests/Squidex.Infrastructure.Tests/MongoDb/MongoFieldTests.cs index 492fd766a..5487156bd 100644 --- a/backend/tests/Squidex.Infrastructure.Tests/MongoDb/MongoFieldTests.cs +++ b/backend/tests/Squidex.Infrastructure.Tests/MongoDb/MongoFieldTests.cs @@ -12,7 +12,7 @@ namespace Squidex.Infrastructure.MongoDb { public class MongoFieldTests { - public sealed class Entity + public class Entity { public string Id { get; set; } @@ -22,6 +22,10 @@ namespace Squidex.Infrastructure.MongoDb public string Custom { get; set; } } + public sealed class Inherited : Entity + { + } + [Fact] public void Should_resolve_id_field() { @@ -30,6 +34,14 @@ namespace Squidex.Infrastructure.MongoDb Assert.Equal("_id", name); } + [Fact] + public void Should_resolve_id_field_from_base_class() + { + var name = Field.Of(x => nameof(x.Id)); + + Assert.Equal("_id", name); + } + [Fact] public void Should_resolve_default_field() { @@ -38,6 +50,14 @@ namespace Squidex.Infrastructure.MongoDb Assert.Equal("Default", name); } + [Fact] + public void Should_resolve_default_field_from_base_class() + { + var name = Field.Of(x => nameof(x.Default)); + + Assert.Equal("Default", name); + } + [Fact] public void Should_resolve_custom_field() { @@ -45,5 +65,19 @@ namespace Squidex.Infrastructure.MongoDb Assert.Equal("_c", name); } + + [Fact] + public void Should_resolve_custom_field_from_base_class() + { + var name = Field.Of(x => nameof(x.Custom)); + + Assert.Equal("_c", name); + } + + [Fact] + public void Should_throw_exception_if_field_not_found() + { + Assert.Throws(() => Field.Of(x => "invalid")); + } } }