From 1f8d4b1db7226c2ced9e944f0e4648773e35c77e Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 15 Feb 2020 09:20:59 +0100 Subject: [PATCH 01/11] Moved implementation of check for existance of username on creating user from user service to repository. --- .../Persistence/Repositories/IUserRepository.cs | 16 ++++++++++++++++ .../Repositories/Implement/UserRepository.cs | 15 +++++++++++++++ .../Services/Implement/UserService.cs | 4 ++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Abstractions/Persistence/Repositories/IUserRepository.cs b/src/Umbraco.Abstractions/Persistence/Repositories/IUserRepository.cs index 29ae581459..0d1a8764ae 100644 --- a/src/Umbraco.Abstractions/Persistence/Repositories/IUserRepository.cs +++ b/src/Umbraco.Abstractions/Persistence/Repositories/IUserRepository.cs @@ -20,8 +20,24 @@ namespace Umbraco.Core.Persistence.Repositories /// /// /// + [Obsolete("This method will be removed in future versions. Please use ExistsByUserName instead.")] bool Exists(string username); + /// + /// Checks if a user with the username exists + /// + /// + /// + bool ExistsByUserName(string username); + + + /// + /// Checks if a user with the login exists + /// + /// + /// + bool ExistsByLogin(string login); + /// /// Gets a list of objects associated with a given group /// diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs index f3db5c5906..f35e820042 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs @@ -626,6 +626,11 @@ ORDER BY colName"; } public bool Exists(string username) + { + return ExistsByUserName(username); + } + + public bool ExistsByUserName(string username) { var sql = SqlContext.Sql() .SelectCount() @@ -635,6 +640,16 @@ ORDER BY colName"; return Database.ExecuteScalar(sql) > 0; } + public bool ExistsByLogin(string login) + { + var sql = SqlContext.Sql() + .SelectCount() + .From() + .Where(x => x.Login == login); + + return Database.ExecuteScalar(sql) > 0; + } + /// /// Gets a list of objects associated with a given group /// diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs index f09e917e94..4dbc87dd4f 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs @@ -109,9 +109,9 @@ namespace Umbraco.Core.Services.Implement User user; using (var scope = ScopeProvider.CreateScope()) { - var loginExists = scope.Database.ExecuteScalar("SELECT COUNT(id) FROM umbracoUser WHERE userLogin = @Login", new { Login = username }) != 0; + var loginExists = _userRepository.ExistsByLogin(username); if (loginExists) - throw new ArgumentException("Login already exists"); // causes rollback // causes rollback + throw new ArgumentException("Login already exists"); // causes rollback user = new User(_globalSettings) { From 0b49c2d84e812adf9fd3a0092d38014df3d74204 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 15 Feb 2020 09:24:13 +0100 Subject: [PATCH 02/11] Removed a few duplicated comments. --- src/Umbraco.Infrastructure/Services/Implement/MemberService.cs | 2 -- .../Services/Implement/PublicAccessService.cs | 1 - .../Services/Implement/ServerRegistrationService.cs | 1 - src/Umbraco.Infrastructure/Services/Implement/UserService.cs | 2 -- 4 files changed, 6 deletions(-) diff --git a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs index 4876772c86..fc0dd8a554 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs @@ -756,7 +756,6 @@ namespace Umbraco.Core.Services.Implement throw new ArgumentOutOfRangeException(nameof(matchType)); // causes rollback // causes rollback } - // TODO: Since this is by property value, we need a GetByPropertyQuery on the repo! // TODO: Since this is by property value, we need a GetByPropertyQuery on the repo! return _memberRepository.Get(query); } @@ -1196,7 +1195,6 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.MemberTree); - // TODO: What about content that has the contenttype as part of its composition? // TODO: What about content that has the contenttype as part of its composition? var query = Query().Where(x => x.ContentTypeId == memberTypeId); diff --git a/src/Umbraco.Infrastructure/Services/Implement/PublicAccessService.cs b/src/Umbraco.Infrastructure/Services/Implement/PublicAccessService.cs index ab9ea64292..f2af6cf424 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/PublicAccessService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/PublicAccessService.cs @@ -124,7 +124,6 @@ namespace Umbraco.Core.Services.Implement } else { - //If they are both the same already then there's nothing to update, exit //If they are both the same already then there's nothing to update, exit return OperationResult.Attempt.Succeed(evtMsgs, entry); } diff --git a/src/Umbraco.Infrastructure/Services/Implement/ServerRegistrationService.cs b/src/Umbraco.Infrastructure/Services/Implement/ServerRegistrationService.cs index 97bf76e672..6a092b159f 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ServerRegistrationService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ServerRegistrationService.cs @@ -72,7 +72,6 @@ namespace Umbraco.Core.Services.Implement _serverRegistrationRepository.Save(server); _serverRegistrationRepository.DeactiveStaleServers(staleTimeout); // triggers a cache reload - // reload - cheap, cached // reload - cheap, cached // default role is single server, but if registrations contain more diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs index 4dbc87dd4f..7b7c36d2ea 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs @@ -340,7 +340,6 @@ namespace Umbraco.Core.Services.Implement _userRepository.Save(user); - //Now we have to check for backwards compat hacks //Now we have to check for backwards compat hacks var explicitUser = user as User; if (explicitUser != null && explicitUser.GroupsToSave.Count > 0) @@ -358,7 +357,6 @@ namespace Umbraco.Core.Services.Implement scope.Events.Dispatch(SavedUser, this, saveEventArgs); } - //commit the whole lot in one go //commit the whole lot in one go scope.Complete(); } From 40d7baca7909038f809da936289892f70b0e95a0 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 15 Feb 2020 11:08:32 +0100 Subject: [PATCH 03/11] Migrated database concerns from KeyValueService to new repository. --- src/Umbraco.Abstractions/Models/IKeyValue.cs | 14 ++ src/Umbraco.Abstractions/Models/KeyValue.cs | 39 +++ .../Repositories/IKeyValueRepository.cs | 13 + .../Migrations/Install/DatabaseDataCreator.cs | 2 +- .../Persistence/Dtos/KeyValueDto.cs | 2 +- .../Implement/KeyValueRepository.cs | 233 ++++++++++++++++++ .../Runtime/SqlMainDomLock.cs | 2 +- src/Umbraco.Infrastructure/RuntimeState.cs | 4 +- .../Services/Implement/KeyValueService.cs | 125 +--------- 9 files changed, 316 insertions(+), 118 deletions(-) create mode 100644 src/Umbraco.Abstractions/Models/IKeyValue.cs create mode 100644 src/Umbraco.Abstractions/Models/KeyValue.cs create mode 100644 src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs create mode 100644 src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs diff --git a/src/Umbraco.Abstractions/Models/IKeyValue.cs b/src/Umbraco.Abstractions/Models/IKeyValue.cs new file mode 100644 index 0000000000..7396ae1b68 --- /dev/null +++ b/src/Umbraco.Abstractions/Models/IKeyValue.cs @@ -0,0 +1,14 @@ +using System; +using Umbraco.Core.Models.Entities; + +namespace Umbraco.Core.Models +{ + public interface IKeyValue : IEntity + { + string Identifier { get; set; } + + string Value { get; set; } + + DateTime UpdateDate { get; set; } + } +} diff --git a/src/Umbraco.Abstractions/Models/KeyValue.cs b/src/Umbraco.Abstractions/Models/KeyValue.cs new file mode 100644 index 0000000000..4ad906d91e --- /dev/null +++ b/src/Umbraco.Abstractions/Models/KeyValue.cs @@ -0,0 +1,39 @@ +using System; +using System.Runtime.Serialization; +using Umbraco.Core.Models.Entities; + +namespace Umbraco.Core.Models +{ + /// + /// Implements . + /// + [Serializable] + [DataContract(IsReference = true)] + public class KeyValue : EntityBase, IKeyValue + { + private string _identifier; + private string _value; + private DateTime _updateDate; + + /// + public string Identifier + { + get => _identifier; + set => SetPropertyValueAndDetectChanges(value, ref _identifier, nameof(Identifier)); + } + + /// + public string Value + { + get => _value; + set => SetPropertyValueAndDetectChanges(value, ref _value, nameof(Value)); + } + + /// + public DateTime UpdateDate + { + get => _updateDate; + set => SetPropertyValueAndDetectChanges(value, ref _updateDate, nameof(UpdateDate)); + } + } +} diff --git a/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs b/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs new file mode 100644 index 0000000000..baae2a1724 --- /dev/null +++ b/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs @@ -0,0 +1,13 @@ +namespace Umbraco.Infrastructure.Persistence.Repositories +{ + public interface IKeyValueRepository + { + void Initialize(); + + string GetValue(string key); + + void SetValue(string key, string value); + + bool TrySetValue(string key, string originalValue, string newValue); + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs index da6574670b..b4328c973d 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs @@ -344,7 +344,7 @@ namespace Umbraco.Core.Migrations.Install var stateValueKey = upgrader.StateValueKey; var finalState = upgrader.Plan.FinalState; - _database.Insert(Constants.DatabaseSchema.Tables.KeyValue, "key", false, new KeyValueDto { Key = stateValueKey, Value = finalState, Updated = DateTime.Now }); + _database.Insert(Constants.DatabaseSchema.Tables.KeyValue, "key", false, new KeyValueDto { Key = stateValueKey, Value = finalState, UpdateDate = DateTime.Now }); } } } diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/KeyValueDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/KeyValueDto.cs index b74039c388..5ead6d0d26 100644 --- a/src/Umbraco.Infrastructure/Persistence/Dtos/KeyValueDto.cs +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/KeyValueDto.cs @@ -21,6 +21,6 @@ namespace Umbraco.Core.Persistence.Dtos [Column("updated")] [Constraint(Default = SystemMethods.CurrentDateTime)] - public DateTime Updated { get; set; } + public DateTime UpdateDate { get; set; } } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs new file mode 100644 index 0000000000..ebe101bbee --- /dev/null +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs @@ -0,0 +1,233 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using NPoco; +using Umbraco.Core.Cache; +using Umbraco.Core.Logging; +using Umbraco.Core.Migrations; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Dtos; +using Umbraco.Core.Persistence.Querying; +using Umbraco.Core.Scoping; +using Umbraco.Infrastructure.Persistence.Repositories; + +namespace Umbraco.Core.Persistence.Repositories.Implement +{ + internal class KeyValueRepository : NPocoRepositoryBase, IKeyValueRepository + { + public KeyValueRepository(IScopeAccessor scopeAccessor, ILogger logger) + : base(scopeAccessor, AppCaches.NoCache, logger) + { } + + public void Initialize() + { + var context = new MigrationContext(Database, Logger); + var initMigration = new InitialMigration(context); + initMigration.Migrate(); + } + + public string GetValue(string key) + { + return GetDtoByKey(key)?.Value; + } + + public void SetValue(string key, string value) + { + var dto = GetDtoByKey(key); + if (dto == null) + { + dto = new KeyValueDto + { + Key = key, + Value = value, + UpdateDate = DateTime.Now + }; + + Database.Insert(dto); + } + else + { + UpdateDtoValue(dto, value); + } + } + + public bool TrySetValue(string key, string originalValue, string newValue) + { + var dto = GetDtoByKey(key); + + if (dto == null || dto.Value != originalValue) + return false; + + UpdateDtoValue(dto, newValue); + return true; + } + + private void UpdateDtoValue(KeyValueDto dto, string value) + { + dto.Value = value; + dto.UpdateDate = DateTime.Now; + Database.Update(dto); + } + + /// + /// Gets a value directly from the database, no scope, nothing. + /// + /// Used by to determine the runtime state. + internal static string GetValue(IUmbracoDatabase database, string key) + { + if (database is null) return null; + + var sql = database.SqlContext.Sql() + .Select() + .From() + .Where(x => x.Key == key); + return database.FirstOrDefault(sql)?.Value; + } + + #region Overrides of NPocoRepositoryBase + + protected override Guid NodeObjectTypeId => throw new NotImplementedException(); + + protected override Sql GetBaseQuery(bool isCount) + { + var sql = SqlContext.Sql(); + + sql = isCount + ? sql.SelectCount() + : sql.Select(); + + sql + .From(); + + return sql; + } + + protected override string GetBaseWhereClause() + { + return Constants.DatabaseSchema.Tables.KeyValue + ".key = @id"; + } + + protected override IEnumerable GetDeleteClauses() + { + return new List(); + } + + protected override IKeyValue PerformGet(string id) + { + var dto = GetDtoByKey(id); + return dto == null ? null : Map(dto); + } + + private KeyValueDto GetDtoByKey(string key) + { + var sql = GetBaseQuery(false).Where(x => x.Key == key); + return Database.Fetch(sql).FirstOrDefault(); + } + + protected override IEnumerable PerformGetAll(params string[] ids) + { + var sql = GetBaseQuery(false).WhereIn(x => x.Key, ids); + var dtos = Database.Fetch(sql); + return dtos.WhereNotNull().Select(Map); + } + + protected override IEnumerable PerformGetByQuery(IQuery query) + { + throw new NotImplementedException(); + } + + protected override void PersistNewItem(IKeyValue entity) + { + var dto = Map(entity); + Database.Insert(dto); + } + + protected override void PersistUpdatedItem(IKeyValue entity) + { + var dto = Map(entity); + Database.Update(dto); + } + + private static KeyValueDto Map(IKeyValue keyValue) + { + if (keyValue == null) return null; + + return new KeyValueDto + { + Key = keyValue.Identifier, + Value = keyValue.Value, + UpdateDate = keyValue.UpdateDate, + }; + } + + private static IKeyValue Map(KeyValueDto dto) + { + if (dto == null) return null; + + return new KeyValue + { + Identifier = dto.Key, + Value = dto.Value, + UpdateDate = dto.UpdateDate, + }; + } + + #endregion + + + /// + /// A custom migration that executes standalone during the Initialize phase of the KeyValueService. + /// + internal class InitialMigration : MigrationBase + { + public InitialMigration(IMigrationContext context) + : base(context) + { } + + public override void Migrate() + { + // as long as we are still running 7 this migration will be invoked, + // but due to multiple restarts during upgrades, maybe the table + // exists already + if (TableExists(Constants.DatabaseSchema.Tables.KeyValue)) + return; + + Logger.Info("Creating KeyValue structure."); + + // the locks table was initially created with an identity (auto-increment) primary key, + // but we don't want this, especially as we are about to insert a new row into the table, + // so here we drop that identity + DropLockTableIdentity(); + + // insert the lock object for key/value + Insert.IntoTable(Constants.DatabaseSchema.Tables.Lock).Row(new { id = Constants.Locks.KeyValues, name = "KeyValues", value = 1 }).Do(); + + // create the key-value table + Create.Table().Do(); + } + + private void DropLockTableIdentity() + { + // one cannot simply drop an identity, that requires a bit of work + + // create a temp. id column and copy values + Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("nid").AsInt32().Nullable().Do(); + Execute.Sql("update umbracoLock set nid = id").Do(); + + // drop the id column entirely (cannot just drop identity) + Delete.PrimaryKey("PK_umbracoLock").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); + Delete.Column("id").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); + + // recreate the id column without identity and copy values + Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("id").AsInt32().Nullable().Do(); + Execute.Sql("update umbracoLock set id = nid").Do(); + + // drop the temp. id column + Delete.Column("nid").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); + + // complete the primary key + Alter.Table(Constants.DatabaseSchema.Tables.Lock).AlterColumn("id").AsInt32().NotNullable().PrimaryKey("PK_umbracoLock").Do(); + } + } + } +} diff --git a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs index 4e1feb221a..d8a66b4198 100644 --- a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs @@ -324,7 +324,7 @@ namespace Umbraco.Core.Runtime { Key = MainDomKey, Value = id, - Updated = DateTime.Now + UpdateDate = DateTime.Now }); } diff --git a/src/Umbraco.Infrastructure/RuntimeState.cs b/src/Umbraco.Infrastructure/RuntimeState.cs index 5952e73e62..b6ffecbb0d 100644 --- a/src/Umbraco.Infrastructure/RuntimeState.cs +++ b/src/Umbraco.Infrastructure/RuntimeState.cs @@ -9,7 +9,7 @@ using Umbraco.Core.Hosting; using Umbraco.Core.Logging; using Umbraco.Core.Migrations.Upgrade; using Umbraco.Core.Persistence; -using Umbraco.Core.Services.Implement; +using Umbraco.Core.Persistence.Repositories.Implement; using Umbraco.Core.Sync; namespace Umbraco.Core @@ -263,7 +263,7 @@ namespace Umbraco.Core // no scope, no service - just directly accessing the database using (var database = databaseFactory.CreateDatabase()) { - CurrentMigrationState = KeyValueService.GetValue(database, stateValueKey); + CurrentMigrationState = KeyValueRepository.GetValue(database, stateValueKey); FinalMigrationState = upgrader.Plan.FinalState; } diff --git a/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs b/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs index d5d8e66525..bdb65091de 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs @@ -1,12 +1,8 @@ using System; -using System.Linq; -using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; using Umbraco.Core.Scoping; -using Umbraco.Core.Persistence; -using Umbraco.Core.Persistence.Dtos; +using Umbraco.Infrastructure.Persistence.Repositories; namespace Umbraco.Core.Services.Implement { @@ -14,13 +10,15 @@ namespace Umbraco.Core.Services.Implement { private readonly object _initialock = new object(); private readonly IScopeProvider _scopeProvider; + private readonly IKeyValueRepository _repository; private readonly ILogger _logger; private readonly IUmbracoVersion _umbracoVersion; private bool _initialized; - public KeyValueService(IScopeProvider scopeProvider, ILogger logger, IUmbracoVersion umbracoVersion) + public KeyValueService(IScopeProvider scopeProvider, IKeyValueRepository repository, ILogger logger, IUmbracoVersion umbracoVersion) { _scopeProvider = scopeProvider; + _repository = repository; _logger = logger; _umbracoVersion = umbracoVersion; } @@ -53,71 +51,14 @@ namespace Umbraco.Core.Services.Implement using (var scope = _scopeProvider.CreateScope()) { - var context = new MigrationContext(scope.Database, _logger); - var initMigration = new InitializeMigration(context); - initMigration.Migrate(); + _repository.Initialize(); scope.Complete(); } // but don't assume we are initializing // we are upgrading from v7 and if anything goes wrong, // the table and everything will be rolled back - } - - /// - /// A custom migration that executes standalone during the Initialize phase of this service. - /// - internal class InitializeMigration : MigrationBase - { - public InitializeMigration(IMigrationContext context) - : base(context) - { } - - public override void Migrate() - { - // as long as we are still running 7 this migration will be invoked, - // but due to multiple restarts during upgrades, maybe the table - // exists already - if (TableExists(Constants.DatabaseSchema.Tables.KeyValue)) - return; - - Logger.Info("Creating KeyValue structure."); - - // the locks table was initially created with an identity (auto-increment) primary key, - // but we don't want this, especially as we are about to insert a new row into the table, - // so here we drop that identity - DropLockTableIdentity(); - - // insert the lock object for key/value - Insert.IntoTable(Constants.DatabaseSchema.Tables.Lock).Row(new {id = Constants.Locks.KeyValues, name = "KeyValues", value = 1}).Do(); - - // create the key-value table - Create.Table().Do(); - } - - private void DropLockTableIdentity() - { - // one cannot simply drop an identity, that requires a bit of work - - // create a temp. id column and copy values - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("nid").AsInt32().Nullable().Do(); - Execute.Sql("update umbracoLock set nid = id").Do(); - - // drop the id column entirely (cannot just drop identity) - Delete.PrimaryKey("PK_umbracoLock").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - Delete.Column("id").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - - // recreate the id column without identity and copy values - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("id").AsInt32().Nullable().Do(); - Execute.Sql("update umbracoLock set id = nid").Do(); - - // drop the temp. id column - Delete.Column("nid").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - - // complete the primary key - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AlterColumn("id").AsInt32().NotNullable().PrimaryKey("PK_umbracoLock").Do(); - } - } + } /// public string GetValue(string key) @@ -126,10 +67,7 @@ namespace Umbraco.Core.Services.Implement using (var scope = _scopeProvider.CreateScope()) { - var sql = scope.SqlContext.Sql().Select().From().Where(x => x.Key == key); - var dto = scope.Database.Fetch(sql).FirstOrDefault(); - scope.Complete(); - return dto?.Value; + return _repository.GetValue(key); } } @@ -142,26 +80,7 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.KeyValues); - var sql = scope.SqlContext.Sql().Select().From().Where(x => x.Key == key); - var dto = scope.Database.Fetch(sql).FirstOrDefault(); - - if (dto == null) - { - dto = new KeyValueDto - { - Key = key, - Value = value, - Updated = DateTime.Now - }; - - scope.Database.Insert(dto); - } - else - { - dto.Value = value; - dto.Updated = DateTime.Now; - scope.Database.Update(dto); - } + _repository.SetValue(key, value); scope.Complete(); } @@ -175,7 +94,7 @@ namespace Umbraco.Core.Services.Implement } /// - public bool TrySetValue(string key, string originValue, string newValue) + public bool TrySetValue(string key, string originalValue, string newValue) { EnsureInitialized(); @@ -183,35 +102,15 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.KeyValues); - var sql = scope.SqlContext.Sql().Select().From().Where(x => x.Key == key); - var dto = scope.Database.Fetch(sql).FirstOrDefault(); - - if (dto == null || dto.Value != originValue) + if (!_repository.TrySetValue(key, originalValue, newValue)) + { return false; - - dto.Value = newValue; - dto.Updated = DateTime.Now; - scope.Database.Update(dto); + } scope.Complete(); } return true; } - - /// - /// Gets a value directly from the database, no scope, nothing. - /// - /// Used by to determine the runtime state. - internal static string GetValue(IUmbracoDatabase database, string key) - { - if (database is null) return null; - - var sql = database.SqlContext.Sql() - .Select() - .From() - .Where(x => x.Key == key); - return database.FirstOrDefault(sql)?.Value; - } } } From ac91b9ef5e1e587272613bf033969cfe7b7d7240 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 15 Feb 2020 11:10:25 +0100 Subject: [PATCH 04/11] Removed usage of obsoleteted method in UserService. --- src/Umbraco.Infrastructure/Services/Implement/UserService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs index 7b7c36d2ea..c6cf6c01c3 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs @@ -46,7 +46,7 @@ namespace Umbraco.Core.Services.Implement { using (var scope = ScopeProvider.CreateScope(autoComplete: true)) { - return _userRepository.Exists(username); + return _userRepository.ExistsByUserName(username); } } From f92b43260af235a39e5fce9f3a153cc9b6dca532 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 15 Feb 2020 12:17:48 +0100 Subject: [PATCH 05/11] Added unit tests for new KeyValueRepository. --- .../Repositories/KeyValueRepositoryTests.cs | 61 +++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 2 files changed, 62 insertions(+) create mode 100644 src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs diff --git a/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs b/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs new file mode 100644 index 0000000000..92ebb03a32 --- /dev/null +++ b/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs @@ -0,0 +1,61 @@ +using NUnit.Framework; +using Umbraco.Core.Persistence.Repositories.Implement; +using Umbraco.Core.Scoping; +using Umbraco.Infrastructure.Persistence.Repositories; +using Umbraco.Tests.TestHelpers; +using Umbraco.Tests.Testing; + +namespace Umbraco.Tests.Persistence.Repositories +{ + [TestFixture] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] + public class KeyValueRepositoryTests : TestWithDatabaseBase + { + [Test] + public void CanSetAndGet() + { + var provider = TestObjects.GetScopeProvider(Logger); + + // Insert new key/value + using (var scope = provider.CreateScope()) + { + var repo = CreateRepository(provider); + repo.SetValue("foo", "bar"); + scope.Complete(); + } + + // Retrieve key/value + using (var scope = provider.CreateScope()) + { + var repo = CreateRepository(provider); + var value = repo.GetValue("foo"); + scope.Complete(); + + Assert.AreEqual("bar", value); + } + + // Update new key/value + using (var scope = provider.CreateScope()) + { + var repo = CreateRepository(provider); + repo.SetValue("foo", "buzz"); + scope.Complete(); + } + + // Retrieve key/value again + using (var scope = provider.CreateScope()) + { + var repo = CreateRepository(provider); + var value = repo.GetValue("foo"); + scope.Complete(); + + Assert.AreEqual("buzz", value); + } + } + + private IKeyValueRepository CreateRepository(IScopeProvider provider) + { + return new KeyValueRepository((IScopeAccessor) provider, Logger); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 16e4ca2c11..1527a8d4cc 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -148,6 +148,7 @@ + From 0bba4755339ae186ef194cde9799ff933d629579 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Mon, 17 Feb 2020 22:12:42 +0100 Subject: [PATCH 06/11] Amends to key value service/repository to better match existing repository patterns. --- src/Umbraco.Abstractions/Models/KeyValue.cs | 7 - .../Repositories/IKeyValueRepository.cs | 13 +- .../Services/ServiceContext.cs | 11 +- .../CompositionExtensions/Repositories.cs | 1 + .../Custom/IKeyValueServiceInitialization.cs | 9 ++ .../Custom/KeyValueServiceInitialization.cs | 80 +++++++++++ .../Implement/KeyValueRepository.cs | 134 +++--------------- .../Implement/RepositoryBaseOfTIdTEntity.cs | 2 +- .../Runtime/CoreInitialComposer.cs | 2 + .../Services/Implement/KeyValueService.cs | 37 ++++- .../Repositories/KeyValueRepositoryTests.cs | 29 ++-- .../Services/KeyValueServiceTests.cs | 93 ++++++++++++ src/Umbraco.Tests/TestHelpers/TestObjects.cs | 3 + src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 14 files changed, 272 insertions(+), 150 deletions(-) create mode 100644 src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs create mode 100644 src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs create mode 100644 src/Umbraco.Tests/Services/KeyValueServiceTests.cs diff --git a/src/Umbraco.Abstractions/Models/KeyValue.cs b/src/Umbraco.Abstractions/Models/KeyValue.cs index 4ad906d91e..ba98b577a4 100644 --- a/src/Umbraco.Abstractions/Models/KeyValue.cs +++ b/src/Umbraco.Abstractions/Models/KeyValue.cs @@ -28,12 +28,5 @@ namespace Umbraco.Core.Models get => _value; set => SetPropertyValueAndDetectChanges(value, ref _value, nameof(Value)); } - - /// - public DateTime UpdateDate - { - get => _updateDate; - set => SetPropertyValueAndDetectChanges(value, ref _updateDate, nameof(UpdateDate)); - } } } diff --git a/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs b/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs index baae2a1724..0eb2b27eb0 100644 --- a/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs +++ b/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs @@ -1,13 +1,8 @@ -namespace Umbraco.Infrastructure.Persistence.Repositories +using Umbraco.Core.Models; + +namespace Umbraco.Core.Persistence.Repositories { - public interface IKeyValueRepository + public interface IKeyValueRepository : IReadRepository, IWriteRepository { - void Initialize(); - - string GetValue(string key); - - void SetValue(string key, string value); - - bool TrySetValue(string key, string originalValue, string newValue); } } diff --git a/src/Umbraco.Abstractions/Services/ServiceContext.cs b/src/Umbraco.Abstractions/Services/ServiceContext.cs index f3c95b07f9..c9bdc6d924 100644 --- a/src/Umbraco.Abstractions/Services/ServiceContext.cs +++ b/src/Umbraco.Abstractions/Services/ServiceContext.cs @@ -32,12 +32,13 @@ namespace Umbraco.Core.Services private readonly Lazy _externalLoginService; private readonly Lazy _redirectUrlService; private readonly Lazy _consentService; + private readonly Lazy _keyValueService; private readonly Lazy _contentTypeBaseServiceProvider; /// /// Initializes a new instance of the class with lazy services. /// - public ServiceContext(Lazy publicAccessService, Lazy domainService, Lazy auditService, Lazy localizedTextService, Lazy tagService, Lazy contentService, Lazy userService, Lazy memberService, Lazy mediaService, Lazy contentTypeService, Lazy mediaTypeService, Lazy dataTypeService, Lazy fileService, Lazy localizationService, Lazy packagingService, Lazy serverRegistrationService, Lazy entityService, Lazy relationService, Lazy macroService, Lazy memberTypeService, Lazy memberGroupService, Lazy notificationService, Lazy externalLoginService, Lazy redirectUrlService, Lazy consentService, Lazy contentTypeBaseServiceProvider) + public ServiceContext(Lazy publicAccessService, Lazy domainService, Lazy auditService, Lazy localizedTextService, Lazy tagService, Lazy contentService, Lazy userService, Lazy memberService, Lazy mediaService, Lazy contentTypeService, Lazy mediaTypeService, Lazy dataTypeService, Lazy fileService, Lazy localizationService, Lazy packagingService, Lazy serverRegistrationService, Lazy entityService, Lazy relationService, Lazy macroService, Lazy memberTypeService, Lazy memberGroupService, Lazy notificationService, Lazy externalLoginService, Lazy redirectUrlService, Lazy consentService, Lazy keyValueService, Lazy contentTypeBaseServiceProvider) { _publicAccessService = publicAccessService; _domainService = domainService; @@ -64,6 +65,7 @@ namespace Umbraco.Core.Services _externalLoginService = externalLoginService; _redirectUrlService = redirectUrlService; _consentService = consentService; + _keyValueService = keyValueService; _contentTypeBaseServiceProvider = contentTypeBaseServiceProvider; } @@ -99,6 +101,7 @@ namespace Umbraco.Core.Services IServerRegistrationService serverRegistrationService = null, IRedirectUrlService redirectUrlService = null, IConsentService consentService = null, + IKeyValueService keyValueService = null, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider = null) { Lazy Lazy(T service) => service == null ? null : new Lazy(() => service); @@ -129,6 +132,7 @@ namespace Umbraco.Core.Services Lazy(externalLoginService), Lazy(redirectUrlService), Lazy(consentService), + Lazy(keyValueService), Lazy(contentTypeBaseServiceProvider) ); } @@ -258,6 +262,11 @@ namespace Umbraco.Core.Services /// public IConsentService ConsentService => _consentService.Value; + /// + /// Gets the KeyValueService. + /// + public IKeyValueService KeyValueService => _keyValueService.Value; + /// /// Gets the ContentTypeServiceBaseFactory. /// diff --git a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs index 0939dd0f71..5318c47a40 100644 --- a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs +++ b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs @@ -47,6 +47,7 @@ namespace Umbraco.Core.Composing.CompositionExtensions composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); + composition.RegisterUnique(); return composition; } diff --git a/src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs b/src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs new file mode 100644 index 0000000000..a863905dc7 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs @@ -0,0 +1,9 @@ +using Umbraco.Core.Persistence; + +namespace Umbraco.Infrastructure.Migrations.Custom +{ + public interface IKeyValueServiceInitialization + { + void PerformInitialMigration(IUmbracoDatabase database); + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs b/src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs new file mode 100644 index 0000000000..488acf507a --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs @@ -0,0 +1,80 @@ +using Umbraco.Core; +using Umbraco.Core.Logging; +using Umbraco.Core.Migrations; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Dtos; + +namespace Umbraco.Infrastructure.Migrations.Custom +{ + public class KeyValueServiceInitialization : IKeyValueServiceInitialization + { + private readonly ILogger _logger; + + public KeyValueServiceInitialization(ILogger logger) + { + _logger = logger; + } + + public void PerformInitialMigration(IUmbracoDatabase database) + { + var context = new MigrationContext(database, _logger); + var initMigration = new InitialMigration(context); + initMigration.Migrate(); + } + + /// + /// A custom migration that executes standalone during the Initialize phase of the KeyValueService. + /// + internal class InitialMigration : MigrationBase + { + public InitialMigration(IMigrationContext context) + : base(context) + { } + + public override void Migrate() + { + // as long as we are still running 7 this migration will be invoked, + // but due to multiple restarts during upgrades, maybe the table + // exists already + if (TableExists(Constants.DatabaseSchema.Tables.KeyValue)) + return; + + Logger.Info("Creating KeyValue structure."); + + // the locks table was initially created with an identity (auto-increment) primary key, + // but we don't want this, especially as we are about to insert a new row into the table, + // so here we drop that identity + DropLockTableIdentity(); + + // insert the lock object for key/value + Insert.IntoTable(Constants.DatabaseSchema.Tables.Lock).Row(new { id = Constants.Locks.KeyValues, name = "KeyValues", value = 1 }).Do(); + + // create the key-value table + Create.Table().Do(); + } + + private void DropLockTableIdentity() + { + // one cannot simply drop an identity, that requires a bit of work + + // create a temp. id column and copy values + Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("nid").AsInt32().Nullable().Do(); + Execute.Sql("update umbracoLock set nid = id").Do(); + + // drop the id column entirely (cannot just drop identity) + Delete.PrimaryKey("PK_umbracoLock").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); + Delete.Column("id").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); + + // recreate the id column without identity and copy values + Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("id").AsInt32().Nullable().Do(); + Execute.Sql("update umbracoLock set id = nid").Do(); + + // drop the temp. id column + Delete.Column("nid").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); + + // complete the primary key + Alter.Table(Constants.DatabaseSchema.Tables.Lock).AlterColumn("id").AsInt32().NotNullable().PrimaryKey("PK_umbracoLock").Do(); + } + } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs index ebe101bbee..eab4049ec9 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs @@ -4,12 +4,11 @@ using System.Linq; using NPoco; using Umbraco.Core.Cache; using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; using Umbraco.Core.Models; +using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Dtos; using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Scoping; -using Umbraco.Infrastructure.Persistence.Repositories; namespace Umbraco.Core.Persistence.Repositories.Implement { @@ -19,56 +18,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement : base(scopeAccessor, AppCaches.NoCache, logger) { } - public void Initialize() - { - var context = new MigrationContext(Database, Logger); - var initMigration = new InitialMigration(context); - initMigration.Migrate(); - } - - public string GetValue(string key) - { - return GetDtoByKey(key)?.Value; - } - - public void SetValue(string key, string value) - { - var dto = GetDtoByKey(key); - if (dto == null) - { - dto = new KeyValueDto - { - Key = key, - Value = value, - UpdateDate = DateTime.Now - }; - - Database.Insert(dto); - } - else - { - UpdateDtoValue(dto, value); - } - } - - public bool TrySetValue(string key, string originalValue, string newValue) - { - var dto = GetDtoByKey(key); - - if (dto == null || dto.Value != originalValue) - return false; - - UpdateDtoValue(dto, newValue); - return true; - } - - private void UpdateDtoValue(KeyValueDto dto, string value) - { - dto.Value = value; - dto.UpdateDate = DateTime.Now; - Database.Update(dto); - } - /// /// Gets a value directly from the database, no scope, nothing. /// @@ -84,6 +33,18 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return database.FirstOrDefault(sql)?.Value; } + #region Overrides of IReadWriteQueryRepository + + public override void Save(IKeyValue entity) + { + if (Get(entity.Identifier) == null) + PersistNewItem(entity); + else + PersistUpdatedItem(entity); + } + + #endregion + #region Overrides of NPocoRepositoryBase protected override Guid NodeObjectTypeId => throw new NotImplementedException(); @@ -114,16 +75,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override IKeyValue PerformGet(string id) { - var dto = GetDtoByKey(id); + var sql = GetBaseQuery(false).Where(x => x.Key == id); + var dto = Database.Fetch(sql).FirstOrDefault(); return dto == null ? null : Map(dto); } - private KeyValueDto GetDtoByKey(string key) - { - var sql = GetBaseQuery(false).Where(x => x.Key == key); - return Database.Fetch(sql).FirstOrDefault(); - } - protected override IEnumerable PerformGetAll(params string[] ids) { var sql = GetBaseQuery(false).WhereIn(x => x.Key, ids); @@ -170,64 +126,8 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Value = dto.Value, UpdateDate = dto.UpdateDate, }; - } + } - #endregion - - - /// - /// A custom migration that executes standalone during the Initialize phase of the KeyValueService. - /// - internal class InitialMigration : MigrationBase - { - public InitialMigration(IMigrationContext context) - : base(context) - { } - - public override void Migrate() - { - // as long as we are still running 7 this migration will be invoked, - // but due to multiple restarts during upgrades, maybe the table - // exists already - if (TableExists(Constants.DatabaseSchema.Tables.KeyValue)) - return; - - Logger.Info("Creating KeyValue structure."); - - // the locks table was initially created with an identity (auto-increment) primary key, - // but we don't want this, especially as we are about to insert a new row into the table, - // so here we drop that identity - DropLockTableIdentity(); - - // insert the lock object for key/value - Insert.IntoTable(Constants.DatabaseSchema.Tables.Lock).Row(new { id = Constants.Locks.KeyValues, name = "KeyValues", value = 1 }).Do(); - - // create the key-value table - Create.Table().Do(); - } - - private void DropLockTableIdentity() - { - // one cannot simply drop an identity, that requires a bit of work - - // create a temp. id column and copy values - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("nid").AsInt32().Nullable().Do(); - Execute.Sql("update umbracoLock set nid = id").Do(); - - // drop the id column entirely (cannot just drop identity) - Delete.PrimaryKey("PK_umbracoLock").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - Delete.Column("id").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - - // recreate the id column without identity and copy values - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("id").AsInt32().Nullable().Do(); - Execute.Sql("update umbracoLock set id = nid").Do(); - - // drop the temp. id column - Delete.Column("nid").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - - // complete the primary key - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AlterColumn("id").AsInt32().NotNullable().PrimaryKey("PK_umbracoLock").Do(); - } - } + #endregion } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs index e8397ba22a..6985bf78da 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs @@ -132,7 +132,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// /// This method is backed by an cache /// - public void Save(TEntity entity) + public virtual void Save(TEntity entity) { if (entity.HasIdentity == false) CachePolicy.Create(entity, PersistNewItem); diff --git a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs index dbecd56d18..109f307c98 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs @@ -21,6 +21,7 @@ using Umbraco.Core.Services; using Umbraco.Core.Strings; using Umbraco.Core.Sync; using IntegerValidator = Umbraco.Core.PropertyEditors.Validators.IntegerValidator; +using Umbraco.Infrastructure.Migrations.Custom; namespace Umbraco.Core.Runtime { @@ -128,6 +129,7 @@ namespace Umbraco.Core.Runtime .Append(); composition.RegisterUnique(factory => new MigrationBuilder(factory)); + composition.RegisterUnique(); // by default, register a noop factory composition.RegisterUnique(); diff --git a/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs b/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs index bdb65091de..6835457f64 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs @@ -1,8 +1,10 @@ using System; using Umbraco.Core.Configuration; using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Scoping; -using Umbraco.Infrastructure.Persistence.Repositories; +using Umbraco.Infrastructure.Migrations.Custom; namespace Umbraco.Core.Services.Implement { @@ -11,14 +13,16 @@ namespace Umbraco.Core.Services.Implement private readonly object _initialock = new object(); private readonly IScopeProvider _scopeProvider; private readonly IKeyValueRepository _repository; + private readonly IKeyValueServiceInitialization _initialization; private readonly ILogger _logger; private readonly IUmbracoVersion _umbracoVersion; private bool _initialized; - public KeyValueService(IScopeProvider scopeProvider, IKeyValueRepository repository, ILogger logger, IUmbracoVersion umbracoVersion) + public KeyValueService(IScopeProvider scopeProvider, IKeyValueRepository repository, IKeyValueServiceInitialization initialization, ILogger logger, IUmbracoVersion umbracoVersion) { _scopeProvider = scopeProvider; _repository = repository; + _initialization = initialization; _logger = logger; _umbracoVersion = umbracoVersion; } @@ -51,7 +55,7 @@ namespace Umbraco.Core.Services.Implement using (var scope = _scopeProvider.CreateScope()) { - _repository.Initialize(); + _initialization.PerformInitialMigration(scope.Database); scope.Complete(); } @@ -67,7 +71,7 @@ namespace Umbraco.Core.Services.Implement using (var scope = _scopeProvider.CreateScope()) { - return _repository.GetValue(key); + return _repository.Get(key)?.Value; } } @@ -80,7 +84,23 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.KeyValues); - _repository.SetValue(key, value); + var keyValue = _repository.Get(key); + if (keyValue == null) + { + keyValue = new KeyValue + { + Identifier = key, + Value = value, + UpdateDate = DateTime.Now, + }; + } + else + { + keyValue.Value = value; + keyValue.UpdateDate = DateTime.Now; + } + + _repository.Save(keyValue); scope.Complete(); } @@ -102,11 +122,16 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.KeyValues); - if (!_repository.TrySetValue(key, originalValue, newValue)) + var keyValue = _repository.Get(key); + if (keyValue == null || keyValue.Value != originalValue) { return false; } + keyValue.Value = newValue; + keyValue.UpdateDate = DateTime.Now; + _repository.Save(keyValue); + scope.Complete(); } diff --git a/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs b/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs index 92ebb03a32..50ebc7ff5d 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/KeyValueRepositoryTests.cs @@ -1,7 +1,9 @@ -using NUnit.Framework; +using System; +using NUnit.Framework; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.Repositories.Implement; using Umbraco.Core.Scoping; -using Umbraco.Infrastructure.Persistence.Repositories; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.Testing; @@ -19,8 +21,14 @@ namespace Umbraco.Tests.Persistence.Repositories // Insert new key/value using (var scope = provider.CreateScope()) { + var keyValue = new KeyValue + { + Identifier = "foo", + Value = "bar", + UpdateDate = DateTime.Now, + }; var repo = CreateRepository(provider); - repo.SetValue("foo", "bar"); + repo.Save(keyValue); scope.Complete(); } @@ -28,17 +36,20 @@ namespace Umbraco.Tests.Persistence.Repositories using (var scope = provider.CreateScope()) { var repo = CreateRepository(provider); - var value = repo.GetValue("foo"); + var keyValue = repo.Get("foo"); scope.Complete(); - Assert.AreEqual("bar", value); + Assert.AreEqual("bar", keyValue.Value); } - // Update new key/value + // Update value using (var scope = provider.CreateScope()) { var repo = CreateRepository(provider); - repo.SetValue("foo", "buzz"); + var keyValue = repo.Get("foo"); + keyValue.Value = "buzz"; + keyValue.UpdateDate = DateTime.Now; + repo.Save(keyValue); scope.Complete(); } @@ -46,10 +57,10 @@ namespace Umbraco.Tests.Persistence.Repositories using (var scope = provider.CreateScope()) { var repo = CreateRepository(provider); - var value = repo.GetValue("foo"); + var keyValue = repo.Get("foo"); scope.Complete(); - Assert.AreEqual("buzz", value); + Assert.AreEqual("buzz", keyValue.Value); } } diff --git a/src/Umbraco.Tests/Services/KeyValueServiceTests.cs b/src/Umbraco.Tests/Services/KeyValueServiceTests.cs new file mode 100644 index 0000000000..5ab29258f5 --- /dev/null +++ b/src/Umbraco.Tests/Services/KeyValueServiceTests.cs @@ -0,0 +1,93 @@ +using System.Threading; +using NUnit.Framework; +using NUnit.Framework.Internal; +using Umbraco.Core.Services; +using Umbraco.Tests.Testing; + +namespace Umbraco.Tests.Services +{ + /// + /// Tests covering methods in the KeyValueService class. + /// This is more of an integration test as it involves multiple layers + /// as well as configuration. + /// + [TestFixture] + [Apartment(ApartmentState.STA)] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] + public class KeyValueServiceTests : TestWithSomeContentBase + { + [Test] + public void GetValue_ForMissingKey_ReturnsNull() + { + // Arrange + var keyValueService = ServiceContext.KeyValueService; + + // Act + var value = keyValueService.GetValue("foo"); + + // Assert + Assert.IsNull(value); + } + + [Test] + public void GetValue_ForExistingKey_ReturnsValue() + { + // Arrange + var keyValueService = ServiceContext.KeyValueService; + keyValueService.SetValue("foo", "bar"); + + // Act + var value = keyValueService.GetValue("foo"); + + // Assert + Assert.AreEqual("bar", value); + } + + [Test] + public void SetValue_ForExistingKey_SavesValue() + { + // Arrange + var keyValueService = ServiceContext.KeyValueService; + keyValueService.SetValue("foo", "bar"); + + // Act + keyValueService.SetValue("foo", "buzz"); + var value = keyValueService.GetValue("foo"); + + // Assert + Assert.AreEqual("buzz", value); + } + + [Test] + public void TrySetValue_ForExistingKeyWithProvidedValue_ReturnsTrueAndSetsValue() + { + // Arrange + var keyValueService = ServiceContext.KeyValueService; + keyValueService.SetValue("foo", "bar"); + + // Act + var result = keyValueService.TrySetValue("foo", "bar", "buzz"); + var value = keyValueService.GetValue("foo"); + + // Assert + Assert.IsTrue(result); + Assert.AreEqual("buzz", value); + } + + [Test] + public void TrySetValue_ForExistingKeyWithoutProvidedValue_ReturnsFalseAndDoesNotSetValue() + { + // Arrange + var keyValueService = ServiceContext.KeyValueService; + keyValueService.SetValue("foo", "bar"); + + // Act + var result = keyValueService.TrySetValue("foo", "bang", "buzz"); + var value = keyValueService.GetValue("foo"); + + // Assert + Assert.IsFalse(result); + Assert.AreEqual("bar", value); + } + } +} diff --git a/src/Umbraco.Tests/TestHelpers/TestObjects.cs b/src/Umbraco.Tests/TestHelpers/TestObjects.cs index 92b9dd0ad2..8e884a5937 100644 --- a/src/Umbraco.Tests/TestHelpers/TestObjects.cs +++ b/src/Umbraco.Tests/TestHelpers/TestObjects.cs @@ -21,6 +21,7 @@ using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Core.Strings; +using Umbraco.Infrastructure.Migrations.Custom; using Umbraco.Tests.TestHelpers.Stubs; using Current = Umbraco.Web.Composing.Current; @@ -193,6 +194,7 @@ namespace Umbraco.Tests.TestHelpers var tagService = GetLazyService(factory, c => new TagService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var redirectUrlService = GetLazyService(factory, c => new RedirectUrlService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var consentService = GetLazyService(factory, c => new ConsentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); + var keyValueService = GetLazyService(factory, c => new KeyValueService(scopeProvider, GetRepo(c), Mock.Of(), logger, umbracoVersion)); var contentTypeServiceBaseFactory = GetLazyService(factory, c => new ContentTypeBaseServiceProvider(factory.GetInstance(),factory.GetInstance(),factory.GetInstance())); return new ServiceContext( @@ -221,6 +223,7 @@ namespace Umbraco.Tests.TestHelpers externalLoginService, redirectUrlService, consentService, + keyValueService, contentTypeServiceBaseFactory); } diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 1527a8d4cc..46baf94644 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -149,6 +149,7 @@ + From 3d206533101a5f23c04327dc5bb70128e8749938 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Thu, 5 Mar 2020 07:53:27 +0100 Subject: [PATCH 07/11] Aligned files with new project structure and fixed failing unit tests. --- src/{Umbraco.Abstractions => Umbraco.Core}/Models/IKeyValue.cs | 0 src/{Umbraco.Abstractions => Umbraco.Core}/Models/KeyValue.cs | 0 .../Persistence/Repositories/IKeyValueRepository.cs | 0 .../Composing/CompositionExtensions/Repositories.cs | 2 ++ 4 files changed, 2 insertions(+) rename src/{Umbraco.Abstractions => Umbraco.Core}/Models/IKeyValue.cs (100%) rename src/{Umbraco.Abstractions => Umbraco.Core}/Models/KeyValue.cs (100%) rename src/{Umbraco.Abstractions => Umbraco.Core}/Persistence/Repositories/IKeyValueRepository.cs (100%) diff --git a/src/Umbraco.Abstractions/Models/IKeyValue.cs b/src/Umbraco.Core/Models/IKeyValue.cs similarity index 100% rename from src/Umbraco.Abstractions/Models/IKeyValue.cs rename to src/Umbraco.Core/Models/IKeyValue.cs diff --git a/src/Umbraco.Abstractions/Models/KeyValue.cs b/src/Umbraco.Core/Models/KeyValue.cs similarity index 100% rename from src/Umbraco.Abstractions/Models/KeyValue.cs rename to src/Umbraco.Core/Models/KeyValue.cs diff --git a/src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IKeyValueRepository.cs similarity index 100% rename from src/Umbraco.Abstractions/Persistence/Repositories/IKeyValueRepository.cs rename to src/Umbraco.Core/Persistence/Repositories/IKeyValueRepository.cs diff --git a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs index 9a23255cc4..b8231241a1 100644 --- a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs +++ b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs @@ -1,5 +1,6 @@ using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.Repositories.Implement; +using Umbraco.Infrastructure.Migrations.Custom; namespace Umbraco.Core.Composing.CompositionExtensions { @@ -48,6 +49,7 @@ namespace Umbraco.Core.Composing.CompositionExtensions composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); + composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); From 3e40389819c57a5edc98f592bf250ed0a53d10bc Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 28 Mar 2020 14:55:03 +0100 Subject: [PATCH 08/11] PR review amends on key value model and repository classes. --- src/Umbraco.Core/Models/IKeyValue.cs | 2 -- src/Umbraco.Core/Models/KeyValue.cs | 5 +++-- .../Implement/KeyValueRepository.cs | 18 +++--------------- .../Implement/RepositoryBaseOfTIdTEntity.cs | 2 +- 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/Umbraco.Core/Models/IKeyValue.cs b/src/Umbraco.Core/Models/IKeyValue.cs index 7396ae1b68..6025d4d37b 100644 --- a/src/Umbraco.Core/Models/IKeyValue.cs +++ b/src/Umbraco.Core/Models/IKeyValue.cs @@ -8,7 +8,5 @@ namespace Umbraco.Core.Models string Identifier { get; set; } string Value { get; set; } - - DateTime UpdateDate { get; set; } } } diff --git a/src/Umbraco.Core/Models/KeyValue.cs b/src/Umbraco.Core/Models/KeyValue.cs index ba98b577a4..2d47fcbfb3 100644 --- a/src/Umbraco.Core/Models/KeyValue.cs +++ b/src/Umbraco.Core/Models/KeyValue.cs @@ -9,11 +9,10 @@ namespace Umbraco.Core.Models /// [Serializable] [DataContract(IsReference = true)] - public class KeyValue : EntityBase, IKeyValue + public class KeyValue : EntityBase, IKeyValue, IEntity { private string _identifier; private string _value; - private DateTime _updateDate; /// public string Identifier @@ -28,5 +27,7 @@ namespace Umbraco.Core.Models get => _value; set => SetPropertyValueAndDetectChanges(value, ref _value, nameof(Value)); } + + bool IEntity.HasIdentity => !string.IsNullOrEmpty(Identifier); } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs index eab4049ec9..094c83b05f 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs @@ -33,21 +33,9 @@ namespace Umbraco.Core.Persistence.Repositories.Implement return database.FirstOrDefault(sql)?.Value; } - #region Overrides of IReadWriteQueryRepository - - public override void Save(IKeyValue entity) - { - if (Get(entity.Identifier) == null) - PersistNewItem(entity); - else - PersistUpdatedItem(entity); - } - - #endregion - #region Overrides of NPocoRepositoryBase - protected override Guid NodeObjectTypeId => throw new NotImplementedException(); + protected override Guid NodeObjectTypeId => throw new NotSupportedException(); protected override Sql GetBaseQuery(bool isCount) { @@ -70,7 +58,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override IEnumerable GetDeleteClauses() { - return new List(); + return Enumerable.Empty(); } protected override IKeyValue PerformGet(string id) @@ -89,7 +77,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override IEnumerable PerformGetByQuery(IQuery query) { - throw new NotImplementedException(); + throw new NotSupportedException(); } protected override void PersistNewItem(IKeyValue entity) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs index 6985bf78da..e8397ba22a 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs @@ -132,7 +132,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// /// This method is backed by an cache /// - public virtual void Save(TEntity entity) + public void Save(TEntity entity) { if (entity.HasIdentity == false) CachePolicy.Create(entity, PersistNewItem); From f01ccb49dd8d181a4ba33293163f7f83e6485da8 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 28 Mar 2020 15:04:36 +0100 Subject: [PATCH 09/11] Removed key value service initialization logic. --- .../CompositionExtensions/Repositories.cs | 2 - .../Custom/IKeyValueServiceInitialization.cs | 9 --- .../Custom/KeyValueServiceInitialization.cs | 80 ------------------- .../Runtime/CoreInitialComposer.cs | 14 ++-- .../Services/Implement/KeyValueService.cs | 56 +------------ src/Umbraco.Tests/TestHelpers/TestObjects.cs | 3 +- 6 files changed, 7 insertions(+), 157 deletions(-) delete mode 100644 src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs delete mode 100644 src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs diff --git a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs index b8231241a1..9a23255cc4 100644 --- a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs +++ b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/Repositories.cs @@ -1,6 +1,5 @@ using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.Repositories.Implement; -using Umbraco.Infrastructure.Migrations.Custom; namespace Umbraco.Core.Composing.CompositionExtensions { @@ -49,7 +48,6 @@ namespace Umbraco.Core.Composing.CompositionExtensions composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); - composition.RegisterUnique(); composition.RegisterUnique(); composition.RegisterUnique(); diff --git a/src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs b/src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs deleted file mode 100644 index a863905dc7..0000000000 --- a/src/Umbraco.Infrastructure/Migrations/Custom/IKeyValueServiceInitialization.cs +++ /dev/null @@ -1,9 +0,0 @@ -using Umbraco.Core.Persistence; - -namespace Umbraco.Infrastructure.Migrations.Custom -{ - public interface IKeyValueServiceInitialization - { - void PerformInitialMigration(IUmbracoDatabase database); - } -} diff --git a/src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs b/src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs deleted file mode 100644 index 488acf507a..0000000000 --- a/src/Umbraco.Infrastructure/Migrations/Custom/KeyValueServiceInitialization.cs +++ /dev/null @@ -1,80 +0,0 @@ -using Umbraco.Core; -using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; -using Umbraco.Core.Persistence; -using Umbraco.Core.Persistence.Dtos; - -namespace Umbraco.Infrastructure.Migrations.Custom -{ - public class KeyValueServiceInitialization : IKeyValueServiceInitialization - { - private readonly ILogger _logger; - - public KeyValueServiceInitialization(ILogger logger) - { - _logger = logger; - } - - public void PerformInitialMigration(IUmbracoDatabase database) - { - var context = new MigrationContext(database, _logger); - var initMigration = new InitialMigration(context); - initMigration.Migrate(); - } - - /// - /// A custom migration that executes standalone during the Initialize phase of the KeyValueService. - /// - internal class InitialMigration : MigrationBase - { - public InitialMigration(IMigrationContext context) - : base(context) - { } - - public override void Migrate() - { - // as long as we are still running 7 this migration will be invoked, - // but due to multiple restarts during upgrades, maybe the table - // exists already - if (TableExists(Constants.DatabaseSchema.Tables.KeyValue)) - return; - - Logger.Info("Creating KeyValue structure."); - - // the locks table was initially created with an identity (auto-increment) primary key, - // but we don't want this, especially as we are about to insert a new row into the table, - // so here we drop that identity - DropLockTableIdentity(); - - // insert the lock object for key/value - Insert.IntoTable(Constants.DatabaseSchema.Tables.Lock).Row(new { id = Constants.Locks.KeyValues, name = "KeyValues", value = 1 }).Do(); - - // create the key-value table - Create.Table().Do(); - } - - private void DropLockTableIdentity() - { - // one cannot simply drop an identity, that requires a bit of work - - // create a temp. id column and copy values - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("nid").AsInt32().Nullable().Do(); - Execute.Sql("update umbracoLock set nid = id").Do(); - - // drop the id column entirely (cannot just drop identity) - Delete.PrimaryKey("PK_umbracoLock").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - Delete.Column("id").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - - // recreate the id column without identity and copy values - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AddColumn("id").AsInt32().Nullable().Do(); - Execute.Sql("update umbracoLock set id = nid").Do(); - - // drop the temp. id column - Delete.Column("nid").FromTable(Constants.DatabaseSchema.Tables.Lock).Do(); - - // complete the primary key - Alter.Table(Constants.DatabaseSchema.Tables.Lock).AlterColumn("id").AsInt32().NotNullable().PrimaryKey("PK_umbracoLock").Do(); - } - } - } -} diff --git a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs index f1f96a9f4a..aa9ccb958f 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs @@ -1,5 +1,4 @@ using System; -using System.IO; using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Composing.CompositionExtensions; @@ -7,9 +6,8 @@ using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Grid; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Dashboards; -using Umbraco.Core.Hosting; using Umbraco.Core.Dictionary; -using Umbraco.Core.IO; +using Umbraco.Core.Hosting; using Umbraco.Core.Logging; using Umbraco.Core.Manifest; using Umbraco.Core.Migrations; @@ -25,16 +23,15 @@ using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Core.Strings; using Umbraco.Core.Sync; -using Umbraco.Web.Models.PublishedContent; -using Umbraco.Web.PublishedCache; using Umbraco.Web; -using Umbraco.Web.Migrations.PostMigrations; using Umbraco.Web.Install; -using Umbraco.Web.Trees; +using Umbraco.Web.Migrations.PostMigrations; +using Umbraco.Web.Models.PublishedContent; using Umbraco.Web.PropertyEditors; +using Umbraco.Web.PublishedCache; using Umbraco.Web.Services; +using Umbraco.Web.Trees; using IntegerValidator = Umbraco.Core.PropertyEditors.Validators.IntegerValidator; -using Umbraco.Infrastructure.Migrations.Custom; namespace Umbraco.Core.Runtime { @@ -149,7 +146,6 @@ namespace Umbraco.Core.Runtime .Append(); composition.RegisterUnique(factory => new MigrationBuilder(factory)); - composition.RegisterUnique(); // by default, register a noop factory composition.RegisterUnique(); diff --git a/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs b/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs index 6835457f64..12ca34b020 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/KeyValueService.cs @@ -1,74 +1,24 @@ using System; -using Umbraco.Core.Configuration; -using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Scoping; -using Umbraco.Infrastructure.Migrations.Custom; namespace Umbraco.Core.Services.Implement { internal class KeyValueService : IKeyValueService { - private readonly object _initialock = new object(); private readonly IScopeProvider _scopeProvider; private readonly IKeyValueRepository _repository; - private readonly IKeyValueServiceInitialization _initialization; - private readonly ILogger _logger; - private readonly IUmbracoVersion _umbracoVersion; - private bool _initialized; - public KeyValueService(IScopeProvider scopeProvider, IKeyValueRepository repository, IKeyValueServiceInitialization initialization, ILogger logger, IUmbracoVersion umbracoVersion) + public KeyValueService(IScopeProvider scopeProvider, IKeyValueRepository repository) { _scopeProvider = scopeProvider; _repository = repository; - _initialization = initialization; - _logger = logger; - _umbracoVersion = umbracoVersion; } - private void EnsureInitialized() - { - lock (_initialock) - { - if (_initialized) return; - Initialize(); - } - } - - private void Initialize() - { - // the key/value service is entirely self-managed, because it is used by the - // upgrader and anything we might change need to happen before everything else - - // if already running 8, either following an upgrade or an install, - // then everything should be ok (the table should exist, etc) - - if (_umbracoVersion.LocalVersion != null && _umbracoVersion.LocalVersion.Major >= 8) - { - _initialized = true; - return; - } - - // else we are upgrading from 7, we can assume that the locks table - // exists, but we need to create everything for key/value - - using (var scope = _scopeProvider.CreateScope()) - { - _initialization.PerformInitialMigration(scope.Database); - scope.Complete(); - } - - // but don't assume we are initializing - // we are upgrading from v7 and if anything goes wrong, - // the table and everything will be rolled back - } - /// public string GetValue(string key) { - EnsureInitialized(); - using (var scope = _scopeProvider.CreateScope()) { return _repository.Get(key)?.Value; @@ -78,8 +28,6 @@ namespace Umbraco.Core.Services.Implement /// public void SetValue(string key, string value) { - EnsureInitialized(); - using (var scope = _scopeProvider.CreateScope()) { scope.WriteLock(Constants.Locks.KeyValues); @@ -116,8 +64,6 @@ namespace Umbraco.Core.Services.Implement /// public bool TrySetValue(string key, string originalValue, string newValue) { - EnsureInitialized(); - using (var scope = _scopeProvider.CreateScope()) { scope.WriteLock(Constants.Locks.KeyValues); diff --git a/src/Umbraco.Tests/TestHelpers/TestObjects.cs b/src/Umbraco.Tests/TestHelpers/TestObjects.cs index ccbea01d0b..13d5cc0328 100644 --- a/src/Umbraco.Tests/TestHelpers/TestObjects.cs +++ b/src/Umbraco.Tests/TestHelpers/TestObjects.cs @@ -21,7 +21,6 @@ using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Core.Strings; -using Umbraco.Infrastructure.Migrations.Custom; using Umbraco.Tests.TestHelpers.Stubs; using Current = Umbraco.Web.Composing.Current; @@ -194,7 +193,7 @@ namespace Umbraco.Tests.TestHelpers var tagService = GetLazyService(factory, c => new TagService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var redirectUrlService = GetLazyService(factory, c => new RedirectUrlService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var consentService = GetLazyService(factory, c => new ConsentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); - var keyValueService = GetLazyService(factory, c => new KeyValueService(scopeProvider, GetRepo(c), Mock.Of(), logger, umbracoVersion)); + var keyValueService = GetLazyService(factory, c => new KeyValueService(scopeProvider, GetRepo(c))); var contentTypeServiceBaseFactory = GetLazyService(factory, c => new ContentTypeBaseServiceProvider(factory.GetInstance(),factory.GetInstance(),factory.GetInstance())); return new ServiceContext( From 8958cf90b978cbebc72a7f523ad61308dc31e189 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 28 Mar 2020 15:13:50 +0100 Subject: [PATCH 10/11] Moved non-scoped retrieval of key value into an extension method in IUmbracoDatabase. --- .../Repositories/Implement/KeyValueRepository.cs | 15 --------------- .../Persistence/UmbracoDatabaseExtensions.cs | 16 ++++++++++++++++ src/Umbraco.Infrastructure/RuntimeState.cs | 2 +- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs index 094c83b05f..2b318b2056 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs @@ -18,21 +18,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement : base(scopeAccessor, AppCaches.NoCache, logger) { } - /// - /// Gets a value directly from the database, no scope, nothing. - /// - /// Used by to determine the runtime state. - internal static string GetValue(IUmbracoDatabase database, string key) - { - if (database is null) return null; - - var sql = database.SqlContext.Sql() - .Select() - .From() - .Where(x => x.Key == key); - return database.FirstOrDefault(sql)?.Value; - } - #region Overrides of NPocoRepositoryBase protected override Guid NodeObjectTypeId => throw new NotSupportedException(); diff --git a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseExtensions.cs b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseExtensions.cs index 249dd3dc73..4e5c3a113a 100644 --- a/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseExtensions.cs +++ b/src/Umbraco.Infrastructure/Persistence/UmbracoDatabaseExtensions.cs @@ -1,4 +1,5 @@ using System; +using Umbraco.Core.Persistence.Dtos; namespace Umbraco.Core.Persistence { @@ -10,5 +11,20 @@ namespace Umbraco.Core.Persistence if (asDatabase == null) throw new Exception("oops: database."); return asDatabase; } + + /// + /// Gets a key/value directly from the database, no scope, nothing. + /// + /// Used by to determine the runtime state. + public static string GetFromKeyValueTable(this IUmbracoDatabase database, string key) + { + if (database is null) return null; + + var sql = database.SqlContext.Sql() + .Select() + .From() + .Where(x => x.Key == key); + return database.FirstOrDefault(sql)?.Value; + } } } diff --git a/src/Umbraco.Infrastructure/RuntimeState.cs b/src/Umbraco.Infrastructure/RuntimeState.cs index 9e5cf96906..d6f674f028 100644 --- a/src/Umbraco.Infrastructure/RuntimeState.cs +++ b/src/Umbraco.Infrastructure/RuntimeState.cs @@ -225,7 +225,7 @@ namespace Umbraco.Core // no scope, no service - just directly accessing the database using (var database = databaseFactory.CreateDatabase()) { - CurrentMigrationState = KeyValueRepository.GetValue(database, stateValueKey); + CurrentMigrationState = database.GetFromKeyValueTable(stateValueKey); FinalMigrationState = upgrader.Plan.FinalState; } From 5a29365063d1bd783137690edb86f6a7d96260bc Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 28 Mar 2020 15:20:17 +0100 Subject: [PATCH 11/11] Revert change to overridden save method on key value repository. --- .../Repositories/Implement/KeyValueRepository.cs | 12 ++++++++++++ .../Implement/RepositoryBaseOfTIdTEntity.cs | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs index 2b318b2056..3c2da4d5b7 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/KeyValueRepository.cs @@ -18,6 +18,18 @@ namespace Umbraco.Core.Persistence.Repositories.Implement : base(scopeAccessor, AppCaches.NoCache, logger) { } + #region Overrides of IReadWriteQueryRepository + + public override void Save(IKeyValue entity) + { + if (Get(entity.Identifier) == null) + PersistNewItem(entity); + else + PersistUpdatedItem(entity); + } + + #endregion + #region Overrides of NPocoRepositoryBase protected override Guid NodeObjectTypeId => throw new NotSupportedException(); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs index e8397ba22a..6985bf78da 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs @@ -132,7 +132,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// /// This method is backed by an cache /// - public void Save(TEntity entity) + public virtual void Save(TEntity entity) { if (entity.HasIdentity == false) CachePolicy.Create(entity, PersistNewItem);