From 4f0718ef3f24b128be37f33d7a821ab26e5c76a2 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 3 May 2016 15:50:10 +0200 Subject: [PATCH] U4-6147 - cleanup --- .../Repositories/DictionaryRepository.cs | 105 +++++++----------- .../Interfaces/IPartialViewMacroRepository.cs | 6 +- .../Services/ServerRegistrationService.cs | 6 +- .../Repositories/DictionaryRepositoryTest.cs | 36 +++--- .../TestHelpers/BaseUmbracoApplicationTest.cs | 1 - 5 files changed, 64 insertions(+), 90 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs index 37ce649af6..00f4c3af2f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs @@ -11,7 +11,6 @@ using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Mappers; using Umbraco.Core.Persistence.Querying; -using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; namespace Umbraco.Core.Persistence.Repositories @@ -30,20 +29,15 @@ namespace Umbraco.Core.Persistence.Repositories } private IRepositoryCachePolicyFactory _cachePolicyFactory; - protected override IRepositoryCachePolicyFactory CachePolicyFactory - { - get - { - //custom cache policy which will not cache any results for GetAll - return _cachePolicyFactory ?? (_cachePolicyFactory = new OnlySingleItemsRepositoryCachePolicyFactory( - RuntimeCache, - new RepositoryCachePolicyOptions - { - //allow zero to be cached - GetAllCacheAllowZeroCount = true - })); - } - } + protected override IRepositoryCachePolicyFactory CachePolicyFactory => _cachePolicyFactory ?? + // custom cache policy which will not cache any results for GetAll + (_cachePolicyFactory = new OnlySingleItemsRepositoryCachePolicyFactory( + RuntimeCache, + new RepositoryCachePolicyOptions + { + //allow zero to be cached + GetAllCacheAllowZeroCount = true + })); #region Overrides of RepositoryBase @@ -59,7 +53,7 @@ namespace Umbraco.Core.Persistence.Repositories if (dto == null) return null; - + var entity = ConvertFromDto(dto); //on initial construction we don't want to have dirty properties tracked @@ -74,12 +68,12 @@ namespace Umbraco.Core.Persistence.Repositories var sql = GetBaseQuery(false).Where("cmsDictionary.pk > 0"); if (ids.Any()) { - sql.Where("cmsDictionary.pk in (@ids)", new { ids = ids }); + sql.Where("cmsDictionary.pk in (@ids)", new { /*ids =*/ ids }); } return Database .FetchOneToMany(x => x.LanguageTextDtos, sql) - .Select(dto => ConvertFromDto(dto)); + .Select(ConvertFromDto); } protected override IEnumerable PerformGetByQuery(IQuery query) @@ -88,10 +82,10 @@ namespace Umbraco.Core.Persistence.Repositories var translator = new SqlTranslator(sqlClause, query); var sql = translator.Translate(); sql.OrderBy(x => x.UniqueId); - + return Database .FetchOneToMany(x => x.LanguageTextDtos, sql) - .Select(x => ConvertFromDto(x)); + .Select(ConvertFromDto); } #endregion @@ -158,7 +152,7 @@ namespace Umbraco.Core.Persistence.Repositories translation.Key = dictionaryItem.Key; } - dictionaryItem.ResetDirtyProperties(); + dictionaryItem.ResetDirtyProperties(); } protected override void PersistUpdatedItem(IDictionaryItem entity) @@ -230,34 +224,27 @@ namespace Umbraco.Core.Persistence.Repositories var factory = new DictionaryItemFactory(); var entity = factory.BuildEntity(dto); - var list = new List(); - foreach (var textDto in dto.LanguageTextDtos.EmptyNull()) - { - if (textDto.LanguageId <= 0) - continue; - - var translationFactory = new DictionaryTranslationFactory(dto.UniqueId); - list.Add(translationFactory.BuildEntity(textDto)); - } - entity.Translations = list; + var f = new DictionaryTranslationFactory(dto.UniqueId); + entity.Translations = dto.LanguageTextDtos.EmptyNull() + .Where(x => x.LanguageId > 0) + .Select(x => f.BuildEntity(x)) + .ToList(); return entity; } public IDictionaryItem Get(Guid uniqueId) { - // fixme - this is ugly var uniqueIdRepo = new DictionaryByUniqueIdRepository(this, UnitOfWork, RepositoryCache, Logger, _mappingResolver); return uniqueIdRepo.Get(uniqueId); } public IDictionaryItem Get(string key) { - // fixme - this is ugly var keyRepo = new DictionaryByKeyRepository(this, UnitOfWork, RepositoryCache, Logger, _mappingResolver); return keyRepo.Get(key); } - + private IEnumerable GetRootDictionaryItems() { var query = Query.Where(x => x.ParentId == null); @@ -277,7 +264,7 @@ namespace Umbraco.Core.Persistence.Repositories { var sqlClause = GetBaseQuery(false) .Where(x => x.Parent != null) - .Where(string.Format("{0} IN (@parentIds)", SqlSyntax.GetQuotedColumnName("parent")), new { parentIds = @group }); + .Where($"{SqlSyntax.GetQuotedColumnName("parent")} IN (@parentIds)", new { parentIds = @group }); var translator = new SqlTranslator(sqlClause, Query); var sql = translator.Translate(); @@ -339,20 +326,15 @@ namespace Umbraco.Core.Persistence.Repositories } private IRepositoryCachePolicyFactory _cachePolicyFactory; - protected override IRepositoryCachePolicyFactory CachePolicyFactory - { - get - { - //custom cache policy which will not cache any results for GetAll - return _cachePolicyFactory ?? (_cachePolicyFactory = new OnlySingleItemsRepositoryCachePolicyFactory( - RuntimeCache, - new RepositoryCachePolicyOptions - { - //allow zero to be cached - GetAllCacheAllowZeroCount = true - })); - } - } + protected override IRepositoryCachePolicyFactory CachePolicyFactory => _cachePolicyFactory ?? + // custom cache policy which will not cache any results for GetAll + (_cachePolicyFactory = new OnlySingleItemsRepositoryCachePolicyFactory( + RuntimeCache, + new RepositoryCachePolicyOptions + { + //allow zero to be cached + GetAllCacheAllowZeroCount = true + })); } private class DictionaryByKeyRepository : SimpleGetRepository @@ -397,22 +379,15 @@ namespace Umbraco.Core.Persistence.Repositories } private IRepositoryCachePolicyFactory _cachePolicyFactory; - protected override IRepositoryCachePolicyFactory CachePolicyFactory - { - get - { - //custom cache policy which will not cache any results for GetAll - return _cachePolicyFactory ?? (_cachePolicyFactory = new OnlySingleItemsRepositoryCachePolicyFactory( - RuntimeCache, - new RepositoryCachePolicyOptions - { - //allow zero to be cached - GetAllCacheAllowZeroCount = true - })); - } - } + protected override IRepositoryCachePolicyFactory CachePolicyFactory => _cachePolicyFactory ?? + // custom cache policy which will not cache any results for GetAll + (_cachePolicyFactory = new OnlySingleItemsRepositoryCachePolicyFactory( + RuntimeCache, + new RepositoryCachePolicyOptions + { + //allow zero to be cached + GetAllCacheAllowZeroCount = true + })); } - - } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewMacroRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewMacroRepository.cs index fca0d61d4e..0f5475e7b6 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewMacroRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPartialViewMacroRepository.cs @@ -1,9 +1,9 @@ namespace Umbraco.Core.Persistence.Repositories { // this only exists to differenciate with IPartialViewRepository in IoC - // without resorting to constants, names, whatever - both interfaces are - // implemented by PartialViewRepository anyway - // fixme - what about file systems?! + // without resorting to constants, names, whatever - and IPartialViewRepository + // is implemented by PartialViewRepository and IPartialViewMacroRepository by + // PartialViewMacroRepository - just to inject the proper filesystem. internal interface IPartialViewMacroRepository : IPartialViewRepository { } } diff --git a/src/Umbraco.Core/Services/ServerRegistrationService.cs b/src/Umbraco.Core/Services/ServerRegistrationService.cs index d1af7ad645..b0d9076b2b 100644 --- a/src/Umbraco.Core/Services/ServerRegistrationService.cs +++ b/src/Umbraco.Core/Services/ServerRegistrationService.cs @@ -67,7 +67,6 @@ namespace Umbraco.Core.Services repo.AddOrUpdate(server); uow.Flush(); // triggers a cache reload - // fixme - this will release the log since we commited the uow?! repo.DeactiveStaleServers(staleTimeout); // triggers a cache reload // reload - cheap, cached @@ -139,8 +138,7 @@ namespace Umbraco.Core.Services /// public IEnumerable GetActiveServers() { - // fixme - oops?! - // fixme - do we want to ensure that the repository does NOT cache? + // fixme - this needs to be refactored entirely now that we have repeatable read everywhere //return _lrepo.WithReadLocked(xr => //{ @@ -166,7 +164,7 @@ namespace Umbraco.Core.Services // repositories works at all in a LB environment - TODO: figure it out using (var uow = UowProvider.CreateUnitOfWork()) - { + { var repo = uow.CreateRepository(); repo.ReadLockServers(); diff --git a/src/Umbraco.Tests/Persistence/Repositories/DictionaryRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/DictionaryRepositoryTest.cs index eb245c947d..598eceb71c 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/DictionaryRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/DictionaryRepositoryTest.cs @@ -297,27 +297,29 @@ namespace Umbraco.Tests.Persistence.Repositories { // Arrange var provider = new NPocoUnitOfWorkProvider(Logger); - var unitOfWork = provider.CreateUnitOfWork(); - var repository = CreateRepository(unitOfWork); + using (var unitOfWork = provider.CreateUnitOfWork()) + { + var repository = CreateRepository(unitOfWork); - var languageNo = new Language("nb-NO") { CultureName = "nb-NO" }; - ServiceContext.LocalizationService.Save(languageNo); + var languageNo = new Language("nb-NO") { CultureName = "nb-NO" }; + ServiceContext.LocalizationService.Save(languageNo); - // Act - var item = repository.Get(1); - var translations = item.Translations.ToList(); - translations.Add(new DictionaryTranslation(languageNo, "Les mer")); - item.Translations = translations; + // Act + var item = repository.Get(1); + var translations = item.Translations.ToList(); + translations.Add(new DictionaryTranslation(languageNo, "Les mer")); + item.Translations = translations; - repository.AddOrUpdate(item); - unitOfWork.Flush(); + repository.AddOrUpdate(item); + unitOfWork.Flush(); - var dictionaryItem = (DictionaryItem)repository.Get(1); - - // Assert - Assert.That(dictionaryItem, Is.Not.Null); - Assert.That(dictionaryItem.Translations.Count(), Is.EqualTo(3)); - Assert.That(dictionaryItem.Translations.Single(t => t.LanguageId == languageNo.Id).Value, Is.EqualTo("Les mer")); + var dictionaryItem = (DictionaryItem) repository.Get(1); + + // Assert + Assert.That(dictionaryItem, Is.Not.Null); + Assert.That(dictionaryItem.Translations.Count(), Is.EqualTo(3)); + Assert.That(dictionaryItem.Translations.Single(t => t.LanguageId == languageNo.Id).Value, Is.EqualTo("Les mer")); + } } [Test] diff --git a/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs b/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs index f08458d671..e1966b7a5c 100644 --- a/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs +++ b/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs @@ -126,7 +126,6 @@ namespace Umbraco.Tests.TestHelpers Container.Register(factory => new MediaFileSystem(Mock.Of())); //replace some stuff - Container.Register(factory => SqlSyntax); // fixme kill? Container.RegisterSingleton(factory => Mock.Of(), "ScriptFileSystem"); Container.RegisterSingleton(factory => Mock.Of(), "PartialViewFileSystem"); Container.RegisterSingleton(factory => Mock.Of(), "PartialViewMacroFileSystem");