From 827f7a7bc89d052ae128e145c74f152b4f8d1e37 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 6 Jan 2016 18:34:07 +0100 Subject: [PATCH] Updates DictionaryRepository(s) to have custom GetAll caching options - to basically disable it since there could be tons of these and currently an IDictionaryItem is a massive entity (need to fix that) --- .../Repositories/DictionaryRepository.cs | 91 ++++++++++++++++--- .../Repositories/LanguageRepository.cs | 4 +- .../Services/LocalizationService.cs | 8 -- .../Dictionary/UmbracoCultureDictionary.cs | 6 ++ 4 files changed, 86 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs index 1985875717..4f588f42db 100644 --- a/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/DictionaryRepository.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Umbraco.Core.Cache; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; @@ -23,16 +24,40 @@ namespace Umbraco.Core.Persistence.Repositories public DictionaryRepository(IDatabaseUnitOfWork work, CacheHelper cache, ILogger logger, ISqlSyntaxProvider syntax, ILanguageRepository languageRepository) : base(work, cache, logger, syntax) - { - _languageRepository = languageRepository; - } + { + _languageRepository = languageRepository; + } + + /// + /// Returns the repository cache options + /// + /// + /// The dictionary repository is also backed by two sub repositories, the main one that will be used is the DictionaryByKeyRepository + /// since the queries from DefaultCultureDictionary will use this. That repositories will manage it's own caches by keys. + /// + protected override RepositoryCacheOptions RepositoryCacheOptions + { + get + { + return new RepositoryCacheOptions + { + //If there is zero, we can cache it + GetAllCacheAllowZeroCount = true, + GetAllCacheAsCollection = false, + GetAllCacheValidateCount = false, + //dont' cache any result with GetAll - since there could be a ton + // of dictionary items. + GetAllCacheThresholdLimit = 0 + }; + } + } #region Overrides of RepositoryBase protected override IDictionaryItem PerformGet(int id) { var sql = GetBaseQuery(false) - .Where(GetBaseWhereClause(), new {Id = id}) + .Where(GetBaseWhereClause(), new { Id = id }) .OrderBy(x => x.UniqueId, SqlSyntax); var dto = Database.Fetch(new DictionaryLanguageTextRelator().Map, sql).FirstOrDefault(); @@ -56,7 +81,7 @@ 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 }); } //This will be cached @@ -87,7 +112,7 @@ namespace Umbraco.Core.Persistence.Repositories protected override Sql GetBaseQuery(bool isCount) { var sql = new Sql(); - if(isCount) + if (isCount) { sql.Select("COUNT(*)") .From(SqlSyntax); @@ -161,7 +186,7 @@ namespace Umbraco.Core.Persistence.Repositories foreach (var translation in entity.Translations) { var textDto = translationFactory.BuildDto(translation); - if(translation.HasIdentity) + if (translation.HasIdentity) { Database.Update(textDto); } @@ -183,7 +208,7 @@ namespace Umbraco.Core.Persistence.Repositories { RecursiveDelete(entity.Key); - Database.Delete("WHERE UniqueId = @Id", new { Id = entity.Key}); + Database.Delete("WHERE UniqueId = @Id", new { Id = entity.Key }); Database.Delete("WHERE id = @Id", new { Id = entity.Key }); //Clear the cache entries that exist by uniqueid/item key @@ -193,7 +218,7 @@ namespace Umbraco.Core.Persistence.Repositories private void RecursiveDelete(Guid parentId) { - var list = Database.Fetch("WHERE parent = @ParentId", new {ParentId = parentId}); + var list = Database.Fetch("WHERE parent = @ParentId", new { ParentId = parentId }); foreach (var dto in list) { RecursiveDelete(dto.UniqueId); @@ -234,7 +259,7 @@ namespace Umbraco.Core.Persistence.Repositories { using (var uniqueIdRepo = new DictionaryByUniqueIdRepository(this, UnitOfWork, RepositoryCache, Logger, SqlSyntax)) { - return uniqueIdRepo.Get(uniqueId); + return uniqueIdRepo.Get(uniqueId); } } @@ -242,7 +267,7 @@ namespace Umbraco.Core.Persistence.Repositories { using (var keyRepo = new DictionaryByKeyRepository(this, UnitOfWork, RepositoryCache, Logger, SqlSyntax)) { - return keyRepo.Get(key); + return keyRepo.Get(key); } } @@ -284,7 +309,7 @@ namespace Umbraco.Core.Persistence.Repositories : getItemsFromParents(new[] { parentId.Value }); return childItems.SelectRecursive(items => getItemsFromParents(items.Select(x => x.Key).ToArray())).SelectMany(items => items); - + } private class DictionaryByUniqueIdRepository : SimpleGetRepository @@ -321,13 +346,33 @@ namespace Umbraco.Core.Persistence.Repositories protected override object GetBaseWhereClauseArguments(Guid id) { - return new {Id = id}; + return new { Id = id }; } protected override string GetWhereInClauseForGetAll() { return "cmsDictionary." + SqlSyntax.GetQuotedColumnName("id") + " in (@ids)"; } + + /// + /// Returns the repository cache options + /// + protected override RepositoryCacheOptions RepositoryCacheOptions + { + get + { + return new RepositoryCacheOptions + { + //If there is zero, we can cache it + GetAllCacheAllowZeroCount = true, + GetAllCacheAsCollection = false, + GetAllCacheValidateCount = false, + //dont' cache any result with GetAll - since there could be a ton + // of dictionary items. + GetAllCacheThresholdLimit = 0 + }; + } + } } private class DictionaryByKeyRepository : SimpleGetRepository @@ -371,6 +416,26 @@ namespace Umbraco.Core.Persistence.Repositories { return "cmsDictionary." + SqlSyntax.GetQuotedColumnName("key") + " in (@ids)"; } + + /// + /// Returns the repository cache options + /// + protected override RepositoryCacheOptions RepositoryCacheOptions + { + get + { + return new RepositoryCacheOptions + { + //If there is zero, we can cache it + GetAllCacheAllowZeroCount = true, + GetAllCacheAsCollection = false, + GetAllCacheValidateCount = false, + //dont' cache any result with GetAll - since there could be a ton + // of dictionary items. + GetAllCacheThresholdLimit = 0 + }; + } + } } /// diff --git a/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs b/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs index d1a28b7b4d..53bf422d99 100644 --- a/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/LanguageRepository.cs @@ -160,13 +160,13 @@ namespace Umbraco.Core.Persistence.Repositories public ILanguage GetByCultureName(string cultureName) { - //use the underlying GetAll which will force cache all domains + //use the underlying GetAll which will force cache all languages return GetAll().FirstOrDefault(x => x.CultureName.InvariantEquals(cultureName)); } public ILanguage GetByIsoCode(string isoCode) { - //use the underlying GetAll which will force cache all domains + //use the underlying GetAll which will force cache all languages return GetAll().FirstOrDefault(x => x.IsoCode.InvariantEquals(isoCode)); } diff --git a/src/Umbraco.Core/Services/LocalizationService.cs b/src/Umbraco.Core/Services/LocalizationService.cs index 6d6cf0c101..8ff7d9166c 100644 --- a/src/Umbraco.Core/Services/LocalizationService.cs +++ b/src/Umbraco.Core/Services/LocalizationService.cs @@ -265,10 +265,6 @@ namespace Umbraco.Core.Services using (var repository = RepositoryFactory.CreateLanguageRepository(UowProvider.GetUnitOfWork())) { return repository.GetByCultureName(cultureName); - //var query = Query.Builder.Where(x => x.CultureName == cultureName); - //var items = repository.GetByQuery(query); - - //return items.FirstOrDefault(); } } @@ -282,10 +278,6 @@ namespace Umbraco.Core.Services using (var repository = RepositoryFactory.CreateLanguageRepository(UowProvider.GetUnitOfWork())) { return repository.GetByIsoCode(isoCode); - //var query = Query.Builder.Where(x => x.IsoCode == isoCode); - //var items = repository.GetByQuery(query); - - //return items.FirstOrDefault(); } } diff --git a/src/Umbraco.Web/Dictionary/UmbracoCultureDictionary.cs b/src/Umbraco.Web/Dictionary/UmbracoCultureDictionary.cs index 4087d26bea..fdb4145160 100644 --- a/src/Umbraco.Web/Dictionary/UmbracoCultureDictionary.cs +++ b/src/Umbraco.Web/Dictionary/UmbracoCultureDictionary.cs @@ -98,6 +98,11 @@ namespace Umbraco.Web.Dictionary /// /// /// + /// + /// NOTE: The result of this is not cached anywhere - the underlying repository does not cache + /// the child lookups because that is done by a query lookup. This method isn't used in our codebase + /// so I don't think this is a performance issue but if devs are using this it could be optimized here. + /// public IDictionary GetChildren(string key) { var result = new Dictionary(); @@ -131,6 +136,7 @@ namespace Umbraco.Web.Dictionary get { //ensure it's stored/retrieved from request cache + //NOTE: This is no longer necessary since these are cached at the runtime level, but we can leave it here for now. return _requestCacheProvider.GetCacheItem(typeof (DefaultCultureDictionary).Name + "Culture", () => _localizationService.GetLanguageByIsoCode(Culture.Name)); }