From 6808a63e7320a6edab5c7ef2db421afbe07da53d Mon Sep 17 00:00:00 2001 From: Mole Date: Tue, 7 Feb 2023 08:30:33 +0100 Subject: [PATCH] New backoffice: Add GUIDS to user groups (#13672) * Add key to UserGroupDto * Fix renaming table in sqlite The SqliteSyntaxProvider needed an overload to use the correct query * Start work on user group GUID migration * Add key index to UserGroupDto * Copy over data when migrating sqlite * Make sqlite column migration work * Remove PostMigrations These should be replaced with Notification usage * Remove outer scope from Upgrader * Remove unececary null check * Add marker base class for migrations * Enable scopeless migrations * Remove unnecessary state check The final state of the migration is no longer necessarily the final state of the plan. * Extend ExecutedMigrationPlan * Ensure that MigrationPlanExecutor.Execute always returns a result. * Always save final state, regardless of errors * Remove obsolete Execute * Add Umbraco specific migration notification * Publish notification after umbraco migration * Throw the exception that failed a migration after publishing notification * Handle notification publishing in DatabaseBuilder * Fix tests * Remember to complete scope * Clean up MigrationPlanExecutor * Run each package migration in a separate scope * Add PartialMigrationsTests * Add unhappy path test * Fix bug shown by test * Move PartialMigrationsTests into the correct folder * Comment out refresh cache in data type migration Need to add this back again as a notification handler or something. * Start working on a notification test * Allow migrations to request a cache rebuild * Set RebuildCache from MigrateDataTypeConfigurations * Clean MigrationPlanExecutor * Add comment explaining the need to partial migration success * Fix tests * Allow overriding DefinePlan of UmbracoPlan This is needed to test the DatabaseBuilder * Fix notification test * Don't throw exception to be immediately re-caught * Assert that scopes notification are always published * Ensure that scopes are created when requested * Make test classes internal. It doesn't really matter, but this way it doesn't show up in intellisense * Add notification handler for clearing cookies * Add CompatibilitySuppressions * Use unscoped migration for adding GUID to user group * Make sqlite migration work It's really not pretty, square peg, round hole. * Don't re-enable foreign keys This will happen automatically next time a connection is started. * Scope database when using SQLServer * Don't call complete transaction * Tidy up a couple of comment * Only allow scoping the database from UnscopedMigrationBase * Fix comment * Remove remark in UnscopedMigrationBase as it's no longer true * Add keys when creating default user groups * Map database value from DTO to entity * Fix migration Rename also renamed the foreign keys, making it not work * Make migration idempotent * Fix unit test * Update CompatibilitySuppressions.xml * Add UniqueId to AppendGroupBy Otherwise MSSQL grenades * Cleanup * Update CompatibilitySuppressions * Rename UniqueId to Key * Cleanup --------- Co-authored-by: Elitsa Marinovska --- .../Services/SqliteSyntaxProvider.cs | 2 + .../CompatibilitySuppressions.xml | 30 ++- .../Models/Membership/IReadOnlyUserGroup.cs | 2 + .../Models/Membership/ReadOnlyUserGroup.cs | 18 +- .../Models/Membership/UserGroupExtensions.cs | 13 +- .../Migrations/Install/DatabaseDataCreator.cs | 30 ++- .../Migrations/MigrationContext.cs | 2 +- .../Migrations/UnscopedMigrationBase.cs | 29 ++- .../Migrations/Upgrade/UmbracoPlan.cs | 1 + .../Upgrade/V_13_0_0/AddGuidsToUserGroups.cs | 180 ++++++++++++++++++ .../Persistence/Dtos/UserGroupDto.cs | 6 + .../Persistence/Factories/UserFactory.cs | 23 ++- .../Persistence/Factories/UserGroupFactory.cs | 2 + .../Persistence/Mappers/UserGroupMapper.cs | 1 + .../Implement/UserGroupRepository.cs | 3 +- .../UserEditorAuthorizationHelperTests.cs | 2 +- 16 files changed, 301 insertions(+), 43 deletions(-) create mode 100644 src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/AddGuidsToUserGroups.cs diff --git a/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteSyntaxProvider.cs b/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteSyntaxProvider.cs index 3803d96490..e00af97909 100644 --- a/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteSyntaxProvider.cs +++ b/src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteSyntaxProvider.cs @@ -160,6 +160,8 @@ public class SqliteSyntaxProvider : SqlSyntaxProviderBase public override string ConvertDateToOrderableString => "{0}"; + public override string RenameTable => "ALTER TABLE {0} RENAME TO {1}"; + /// public override string GetSpecialDbType(SpecialDbType dbType) => "TEXT COLLATE NOCASE"; diff --git a/src/Umbraco.Core/CompatibilitySuppressions.xml b/src/Umbraco.Core/CompatibilitySuppressions.xml index 47d170282c..400de0b096 100644 --- a/src/Umbraco.Core/CompatibilitySuppressions.xml +++ b/src/Umbraco.Core/CompatibilitySuppressions.xml @@ -203,6 +203,20 @@ lib/net7.0/Umbraco.Core.dll true + + CP0002 + M:Umbraco.Cms.Core.Models.Membership.ReadOnlyUserGroup.#ctor(System.Int32,System.String,System.String,System.Nullable{System.Int32},System.Nullable{System.Int32},System.String,System.Collections.Generic.IEnumerable{System.Int32},System.Collections.Generic.IEnumerable{System.String},System.Collections.Generic.IEnumerable{System.String},System.Boolean) + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + + + CP0002 + M:Umbraco.Cms.Core.Models.Membership.ReadOnlyUserGroup.#ctor(System.Int32,System.String,System.String,System.Nullable{System.Int32},System.Nullable{System.Int32},System.String,System.Collections.Generic.IEnumerable{System.String},System.Collections.Generic.IEnumerable{System.String}) + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + CP0002 M:Umbraco.Cms.Core.PropertyEditors.ConfigurationEditor.FromConfigurationEditor(System.Collections.Generic.IDictionary{System.String,System.Object},System.Object) @@ -329,6 +343,13 @@ lib/net7.0/Umbraco.Core.dll true + + CP0002 + M:Umbraco.Cms.Core.Services.Implement.DataTypeService.#ctor(Umbraco.Cms.Core.PropertyEditors.IDataValueEditorFactory,Umbraco.Cms.Core.Scoping.ICoreScopeProvider,Microsoft.Extensions.Logging.ILoggerFactory,Umbraco.Cms.Core.Events.IEventMessagesFactory,Umbraco.Cms.Core.Persistence.Repositories.IDataTypeRepository,Umbraco.Cms.Core.Persistence.Repositories.IDataTypeContainerRepository,Umbraco.Cms.Core.Persistence.Repositories.IAuditRepository,Umbraco.Cms.Core.Persistence.Repositories.IEntityRepository,Umbraco.Cms.Core.Persistence.Repositories.IContentTypeRepository,Umbraco.Cms.Core.IO.IIOHelper,Umbraco.Cms.Core.Services.ILocalizedTextService,Umbraco.Cms.Core.Services.ILocalizationService,Umbraco.Cms.Core.Strings.IShortStringHelper,Umbraco.Cms.Core.Serialization.IJsonSerializer) + lib/net7.0/Umbraco.Core.dll + lib/net7.0/Umbraco.Core.dll + true + CP0002 M:Umbraco.Extensions.DictionaryItemExtensions.GetDefaultValue(Umbraco.Cms.Core.Models.IDictionaryItem) @@ -343,13 +364,6 @@ lib/net7.0/Umbraco.Core.dll true - - CP0002 - M:Umbraco.Cms.Core.Services.Implement.DataTypeService.#ctor(Umbraco.Cms.Core.PropertyEditors.IDataValueEditorFactory,Umbraco.Cms.Core.Scoping.ICoreScopeProvider,Microsoft.Extensions.Logging.ILoggerFactory,Umbraco.Cms.Core.Events.IEventMessagesFactory,Umbraco.Cms.Core.Persistence.Repositories.IDataTypeRepository,Umbraco.Cms.Core.Persistence.Repositories.IDataTypeContainerRepository,Umbraco.Cms.Core.Persistence.Repositories.IAuditRepository,Umbraco.Cms.Core.Persistence.Repositories.IEntityRepository,Umbraco.Cms.Core.Persistence.Repositories.IContentTypeRepository,Umbraco.Cms.Core.IO.IIOHelper,Umbraco.Cms.Core.Services.ILocalizedTextService,Umbraco.Cms.Core.Services.ILocalizationService,Umbraco.Cms.Core.Strings.IShortStringHelper,Umbraco.Cms.Core.Serialization.IJsonSerializer) - lib/net7.0/Umbraco.Core.dll - lib/net7.0/Umbraco.Core.dll - true - CP0006 M:Umbraco.Cms.Core.Deploy.IDataTypeConfigurationConnector.FromArtifact(Umbraco.Cms.Core.Models.IDataType,System.String,Umbraco.Cms.Core.Deploy.IContextCache) @@ -574,4 +588,4 @@ lib/net7.0/Umbraco.Core.dll true - + \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs b/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs index df34964954..0064a7f454 100644 --- a/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs @@ -11,6 +11,8 @@ public interface IReadOnlyUserGroup int Id { get; } + Guid Key => Guid.Empty; + int? StartContentId { get; } int? StartMediaId { get; } diff --git a/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs b/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs index 09865a61bb..f7bc83d94d 100644 --- a/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs @@ -4,6 +4,7 @@ public class ReadOnlyUserGroup : IReadOnlyUserGroup, IEquatable allowedSections, - IEnumerable? permissions) - : this(id, name, icon, startContentId, startMediaId, alias, Enumerable.Empty(), allowedSections, permissions, true) - { - } - public int Id { get; } + public Guid Key { get; } + public bool Equals(ReadOnlyUserGroup? other) { if (ReferenceEquals(null, other)) diff --git a/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs b/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs index dbfbb2f935..a9f3c70610 100644 --- a/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs +++ b/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs @@ -14,7 +14,18 @@ public static class UserGroupExtensions } // otherwise create one - return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, group.StartContentId, group.StartMediaId, group.Alias, group.AllowedLanguages, group.AllowedSections, group.Permissions, group.HasAccessToAllLanguages); + return new ReadOnlyUserGroup( + group.Id, + group.Key, + group.Name, + group.Icon, + group.StartContentId, + group.StartMediaId, + group.Alias, + group.AllowedLanguages, + group.AllowedSections, + group.Permissions, + group.HasAccessToAllLanguages); } public static bool IsSystemUserGroup(this IUserGroup group) => diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs index cc2e20fa02..e77d26f088 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs @@ -1165,10 +1165,14 @@ internal class DatabaseDataCreator private void CreateUserGroupData() { - _database.Insert(Constants.DatabaseSchema.Tables.UserGroup, "id", false, + _database.Insert( + Constants.DatabaseSchema.Tables.UserGroup, + "id", + false, new UserGroupDto { Id = 1, + Key = Guid.NewGuid(), StartMediaId = -1, StartContentId = -1, Alias = Constants.Security.AdminGroupAlias, @@ -1179,10 +1183,14 @@ internal class DatabaseDataCreator Icon = "icon-medal", HasAccessToAllLanguages = true, }); - _database.Insert(Constants.DatabaseSchema.Tables.UserGroup, "id", false, + _database.Insert( + Constants.DatabaseSchema.Tables.UserGroup, + "id", + false, new UserGroupDto { Id = 2, + Key = Guid.NewGuid(), StartMediaId = -1, StartContentId = -1, Alias = Constants.Security.WriterGroupAlias, @@ -1193,10 +1201,14 @@ internal class DatabaseDataCreator Icon = "icon-edit", HasAccessToAllLanguages = true, }); - _database.Insert(Constants.DatabaseSchema.Tables.UserGroup, "id", false, + _database.Insert( + Constants.DatabaseSchema.Tables.UserGroup, + "id", + false, new UserGroupDto { Id = 3, + Key = Guid.NewGuid(), StartMediaId = -1, StartContentId = -1, Alias = Constants.Security.EditorGroupAlias, @@ -1207,10 +1219,14 @@ internal class DatabaseDataCreator Icon = "icon-tools", HasAccessToAllLanguages = true, }); - _database.Insert(Constants.DatabaseSchema.Tables.UserGroup, "id", false, + _database.Insert( + Constants.DatabaseSchema.Tables.UserGroup, + "id", + false, new UserGroupDto { Id = 4, + Key = Guid.NewGuid(), StartMediaId = -1, StartContentId = -1, Alias = Constants.Security.TranslatorGroupAlias, @@ -1221,10 +1237,14 @@ internal class DatabaseDataCreator Icon = "icon-globe", HasAccessToAllLanguages = true, }); - _database.Insert(Constants.DatabaseSchema.Tables.UserGroup, "id", false, + _database.Insert( + Constants.DatabaseSchema.Tables.UserGroup, + "id", + false, new UserGroupDto { Id = 5, + Key = Guid.NewGuid(), Alias = Constants.Security.SensitiveDataGroupAlias, Name = "Sensitive data", DefaultPermissions = string.Empty, diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs index 4b1900c27c..a5a44a8d8d 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs @@ -24,7 +24,7 @@ internal class MigrationContext : IMigrationContext public MigrationPlan Plan { get; } /// - public IUmbracoDatabase Database { get; } + public IUmbracoDatabase Database { get; internal set; } /// public ISqlContext SqlContext => Database.SqlContext; diff --git a/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs b/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs index 79a7f2e8ac..ca52be6d9d 100644 --- a/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs +++ b/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs @@ -1,14 +1,31 @@ -namespace Umbraco.Cms.Infrastructure.Migrations; +using Umbraco.Cms.Infrastructure.Scoping; + +namespace Umbraco.Cms.Infrastructure.Migrations; /// -/// Base class for creating a migration that does not have a scope provided for it +/// Base class for creating a migration that does not have a scope provided for it. /// -/// -/// This is just a marker class, and has all the same functionality as the underlying MigrationBase -/// public abstract class UnscopedMigrationBase : MigrationBase { - protected UnscopedMigrationBase(IMigrationContext context) : base(context) + protected UnscopedMigrationBase(IMigrationContext context) + : base(context) { } + + /// + /// Scope the database used by the migration builder. + /// This is used with when you need to execute something before the scope is created + /// but later need to have your queries scoped in a transaction. + /// + /// The scope to get the database from. + /// If the migration is missing or has a malformed MigrationContext, this exception is thrown. + protected void ScopeDatabase(IScope scope) + { + if (Context is not MigrationContext context) + { + throw new InvalidOperationException("Cannot scope database because context is not a MigrationContext"); + } + + context.Database = scope.Database; + } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 0ae119b8e7..bccc949002 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -96,5 +96,6 @@ public class UmbracoPlan : MigrationPlan // To 13.0.0 To("{419827A0-4FCE-464B-A8F3-247C6092AF55}"); To("{5F15A1CC-353D-4889-8C7E-F303B4766196}"); + To("{69E12556-D9B3-493A-8E8A-65EC89FB658D}"); } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/AddGuidsToUserGroups.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/AddGuidsToUserGroups.cs new file mode 100644 index 0000000000..3d06a07ce4 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/AddGuidsToUserGroups.cs @@ -0,0 +1,180 @@ +using NPoco; +using Umbraco.Cms.Core; +using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations; +using Umbraco.Cms.Infrastructure.Persistence.DatabaseModelDefinitions; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Infrastructure.Scoping; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_13_0_0; + +public class AddGuidsToUserGroups : UnscopedMigrationBase +{ + private const string NewColumnName = "key"; + private readonly IScopeProvider _scopeProvider; + + public AddGuidsToUserGroups(IMigrationContext context, IScopeProvider scopeProvider) + : base(context) + { + _scopeProvider = scopeProvider; + } + + protected override void Migrate() + { + // SQL server can simply add the column, but for SQLite this won't work, + // so we'll have to create a new table and copy over data. + if (DatabaseType != DatabaseType.SQLite) + { + MigrateSqlServer(); + return; + } + + MigrateSqlite(); + } + + private void MigrateSqlServer() + { + using IScope scope = _scopeProvider.CreateScope(); + using IDisposable notificationSuppression = scope.Notifications.Suppress(); + ScopeDatabase(scope); + + var columns = SqlSyntax.GetColumnsInSchema(Context.Database).ToList(); + AddColumnIfNotExists(columns, NewColumnName); + scope.Complete(); + } + + private void MigrateSqlite() + { + using IScope scope = _scopeProvider.CreateScope(); + using IDisposable notificationSuppression = scope.Notifications.Suppress(); + ScopeDatabase(scope); + + // If the new column already exists we'll do nothing. + if (ColumnExists(Constants.DatabaseSchema.Tables.UserGroup, NewColumnName)) + { + return; + } + + // This isn't pretty, + // But since you cannot alter columns, we have to copy the data over and delete the old table. + // However we cannot do this due to foreign keys, so temporarily disable these keys while migrating. + // This leads to an interesting chicken and egg issue, we have to do this before a transaction + // but in the same connection, since foreign keys will default to "ON" next time we open a connection. + // This means we have to just execute the DB command to end the transaction, + // instead of using CompleteTransaction, since this will end the connection. + Database.Execute("COMMIT;"); + + // We don't have to worry about re-enabling this since it happens automatically. + Database.Execute("PRAGMA foreign_keys=off;"); + Database.Execute("BEGIN TRANSACTION;"); + + // Now that keys are disabled and we have a transaction, we'll do our migration. + MigrateColumnSqlite(); + scope.Complete(); + } + + private void MigrateColumnSqlite() + { + IEnumerable groups = Database.Fetch().Select(x => new UserGroupDto + { + Id = x.Id, + Key = Guid.NewGuid(), + Alias = x.Alias, + Name = x.Name, + DefaultPermissions = x.DefaultPermissions, + CreateDate = x.CreateDate, + UpdateDate = x.UpdateDate, + Icon = x.Icon, + HasAccessToAllLanguages = x.HasAccessToAllLanguages, + StartContentId = x.StartContentId, + StartMediaId = x.StartMediaId, + UserGroup2AppDtos = x.UserGroup2AppDtos, + UserGroup2LanguageDtos = x.UserGroup2LanguageDtos + }); + + // I realize that this may seem a bit drastic, + // however, since SQLite cannot generate GUIDs we have to load all the user groups into memory to generate it. + // So instead of going through the trouble of creating a new table, copying over data, and then deleting + // We can just drop the table directly and re-create it to add the new column. + Delete.Table(Constants.DatabaseSchema.Tables.UserGroup).Do(); + Create.Table().Do(); + + // We have to insert one at a time to be able to not auto increment the id. + foreach (UserGroupDto group in groups) + { + Database.Insert(Constants.DatabaseSchema.Tables.UserGroup, "id", false, group); + } + } + + [TableName(Constants.DatabaseSchema.Tables.UserGroup)] + [PrimaryKey("id")] + [ExplicitColumns] + private class OldUserGroupDto + { + public OldUserGroupDto() + { + UserGroup2AppDtos = new List(); + UserGroup2LanguageDtos = new List(); + } + + [Column("id")] + [PrimaryKeyColumn(IdentitySeed = 6)] + public int Id { get; set; } + + [Column("userGroupAlias")] + [Length(200)] + [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoUserGroup_userGroupAlias")] + public string? Alias { get; set; } + + [Column("userGroupName")] + [Length(200)] + [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoUserGroup_userGroupName")] + public string? Name { get; set; } + + [Column("userGroupDefaultPermissions")] + [Length(50)] + [NullSetting(NullSetting = NullSettings.Null)] + public string? DefaultPermissions { get; set; } + + [Column("createDate")] + [NullSetting(NullSetting = NullSettings.NotNull)] + [Constraint(Default = SystemMethods.CurrentDateTime)] + public DateTime CreateDate { get; set; } + + [Column("updateDate")] + [NullSetting(NullSetting = NullSettings.NotNull)] + [Constraint(Default = SystemMethods.CurrentDateTime)] + public DateTime UpdateDate { get; set; } + + [Column("icon")] + [NullSetting(NullSetting = NullSettings.Null)] + public string? Icon { get; set; } + + [Column("hasAccessToAllLanguages")] + [NullSetting(NullSetting = NullSettings.NotNull)] + public bool HasAccessToAllLanguages { get; set; } + + [Column("startContentId")] + [NullSetting(NullSetting = NullSettings.Null)] + [ForeignKey(typeof(NodeDto), Name = "FK_startContentId_umbracoNode_id")] + public int? StartContentId { get; set; } + + [Column("startMediaId")] + [NullSetting(NullSetting = NullSettings.Null)] + [ForeignKey(typeof(NodeDto), Name = "FK_startMediaId_umbracoNode_id")] + public int? StartMediaId { get; set; } + + [ResultColumn] + [Reference(ReferenceType.Many, ReferenceMemberName = "UserGroupId")] + public List UserGroup2AppDtos { get; set; } + + [ResultColumn] + [Reference(ReferenceType.Many, ReferenceMemberName = "UserGroupId")] + public List UserGroup2LanguageDtos { get; set; } + + /// + /// This is only relevant when this column is included in the results (i.e. GetUserGroupsWithUserCounts). + /// + [ResultColumn] + public int UserCount { get; set; } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroupDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroupDto.cs index 62cdad9e26..e8699221d7 100644 --- a/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroupDto.cs +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroupDto.cs @@ -20,6 +20,12 @@ public class UserGroupDto [PrimaryKeyColumn(IdentitySeed = 6)] public int Id { get; set; } + [Column("key")] + [NullSetting(NullSetting = NullSettings.NotNull)] + [Constraint(Default = SystemMethods.NewGuid)] + [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoUserGroup_userGroupKey")] + public Guid Key { get; set; } + [Column("userGroupAlias")] [Length(200)] [Index(IndexTypes.UniqueNonClustered, Name = "IX_umbracoUserGroup_userGroupAlias")] diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs index 11e2479691..17c445576d 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/UserFactory.cs @@ -110,12 +110,23 @@ internal static class UserFactory return dto; } - private static IReadOnlyUserGroup ToReadOnlyGroup(UserGroupDto group) => - new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, - group.StartContentId, group.StartMediaId, group.Alias, group.UserGroup2LanguageDtos.Select(x => x.LanguageId), + private static IReadOnlyUserGroup ToReadOnlyGroup(UserGroupDto group) + { + IEnumerable permissions = group.DefaultPermissions is null + ? Enumerable.Empty() + : group.DefaultPermissions.ToCharArray().Select(x => x.ToString()); + + return new ReadOnlyUserGroup( + group.Id, + group.Key, + group.Name, + group.Icon, + group.StartContentId, + group.StartMediaId, + group.Alias, + group.UserGroup2LanguageDtos.Select(x => x.LanguageId), group.UserGroup2AppDtos.Select(x => x.AppAlias).WhereNotNull().ToArray(), - group.DefaultPermissions == null - ? Enumerable.Empty() - : group.DefaultPermissions.ToCharArray().Select(x => x.ToString()), + permissions, group.HasAccessToAllLanguages); + } } diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/UserGroupFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/UserGroupFactory.cs index 3c4546da04..e48d1cd17f 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/UserGroupFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/UserGroupFactory.cs @@ -22,6 +22,7 @@ internal static class UserGroupFactory { userGroup.DisableChangeTracking(); userGroup.Id = dto.Id; + userGroup.Key = dto.Key; userGroup.CreateDate = dto.CreateDate; userGroup.UpdateDate = dto.UpdateDate; userGroup.StartContentId = dto.StartContentId; @@ -53,6 +54,7 @@ internal static class UserGroupFactory { var dto = new UserGroupDto { + Key = entity.Key, Alias = entity.Alias, DefaultPermissions = entity.Permissions == null ? string.Empty : string.Join(string.Empty, entity.Permissions), Name = entity.Name, diff --git a/src/Umbraco.Infrastructure/Persistence/Mappers/UserGroupMapper.cs b/src/Umbraco.Infrastructure/Persistence/Mappers/UserGroupMapper.cs index 6c6ef11d35..885981453e 100644 --- a/src/Umbraco.Infrastructure/Persistence/Mappers/UserGroupMapper.cs +++ b/src/Umbraco.Infrastructure/Persistence/Mappers/UserGroupMapper.cs @@ -19,6 +19,7 @@ public sealed class UserGroupMapper : BaseMapper protected override void DefineMaps() { DefineMap(nameof(UserGroup.Id), nameof(UserGroupDto.Id)); + DefineMap(nameof(UserGroup.Key), nameof(UserGroupDto.Key)); DefineMap(nameof(UserGroup.Alias), nameof(UserGroupDto.Alias)); DefineMap(nameof(UserGroup.Name), nameof(UserGroupDto.Name)); DefineMap(nameof(UserGroup.Icon), nameof(UserGroupDto.Icon)); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs index f95e86110b..33d666dfe4 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs @@ -403,7 +403,8 @@ public class UserGroupRepository : EntityRepositoryBase, IUserG x => x.Alias, x => x.DefaultPermissions, x => x.Name, - x => x.HasAccessToAllLanguages) + x => x.HasAccessToAllLanguages, + x => x.Key) .AndBy(x => x.AppAlias, x => x.UserGroupId); protected override string GetBaseWhereClause() => $"{Constants.DatabaseSchema.Tables.UserGroup}.id = @id"; diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs index 7751dee263..fd9fd1ca05 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs @@ -116,7 +116,7 @@ public class UserEditorAuthorizationHelperTests { var currentUser = Mock.Of(user => user.Groups == new[] { - new ReadOnlyUserGroup(1, "CurrentUser", "icon-user", null, null, groupAlias, new int[0], new string[0], new string[0], true), + new ReadOnlyUserGroup(1, Guid.NewGuid(), "CurrentUser", "icon-user", null, null, groupAlias, new int[0], new string[0], new string[0], true), }); IUser savingUser = null; // This means it is a new created user