From 0a0e34d8064b05a1489ece09a64569a08434b834 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 28 Jan 2015 16:15:34 +1100 Subject: [PATCH] Ensures that the cache is updated properly with cache refreshers, ensures that the GetAll method for public access repo doesn't make any db calls and uses a non validating cache since it gets called a ton. --- src/Umbraco.Core/Models/PublicAccessEntry.cs | 2 +- src/Umbraco.Core/Models/PublicAccessRule.cs | 3 +- src/Umbraco.Core/Models/Rdbms/AccessDto.cs | 2 +- .../Factories/PublicAccessEntryFactory.cs | 4 +- .../Persistence/Mappers/AccessMapper.cs | 36 +++++++++++++++ .../Interfaces/IPublicAccessRepository.cs | 1 - .../Repositories/PublicAccessRepository.cs | 34 ++++++++++---- .../Repositories/RepositoryBase.cs | 44 ++++++++++++++++--- .../Services/PublicAccessService.cs | 14 +++--- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../PublicAccessRepositoryTest.cs | 35 --------------- .../Cache/DistributedCacheExtensions.cs | 1 + src/Umbraco.Web/Cache/PageCacheRefresher.cs | 3 ++ .../Cache/UnpublishedPageCacheRefresher.cs | 7 +++ src/umbraco.cms/businesslogic/web/Access.cs | 30 +++---------- 15 files changed, 129 insertions(+), 88 deletions(-) create mode 100644 src/Umbraco.Core/Persistence/Mappers/AccessMapper.cs diff --git a/src/Umbraco.Core/Models/PublicAccessEntry.cs b/src/Umbraco.Core/Models/PublicAccessEntry.cs index 258974db6d..9522710c53 100644 --- a/src/Umbraco.Core/Models/PublicAccessEntry.cs +++ b/src/Umbraco.Core/Models/PublicAccessEntry.cs @@ -127,7 +127,7 @@ namespace Umbraco.Core.Models set { base.Key = value; - HasIdentity = true; + Id = value.GetHashCode(); } } diff --git a/src/Umbraco.Core/Models/PublicAccessRule.cs b/src/Umbraco.Core/Models/PublicAccessRule.cs index 5ecbb8c396..584d6761db 100644 --- a/src/Umbraco.Core/Models/PublicAccessRule.cs +++ b/src/Umbraco.Core/Models/PublicAccessRule.cs @@ -16,7 +16,6 @@ namespace Umbraco.Core.Models { AccessEntryId = accessEntryId; Key = id; - Id = Key.GetHashCode(); } public PublicAccessRule() @@ -32,7 +31,7 @@ namespace Umbraco.Core.Models set { base.Key = value; - HasIdentity = true; + Id = value.GetHashCode(); } } diff --git a/src/Umbraco.Core/Models/Rdbms/AccessDto.cs b/src/Umbraco.Core/Models/Rdbms/AccessDto.cs index 5b4bed1e10..ba2cb6d767 100644 --- a/src/Umbraco.Core/Models/Rdbms/AccessDto.cs +++ b/src/Umbraco.Core/Models/Rdbms/AccessDto.cs @@ -27,7 +27,7 @@ namespace Umbraco.Core.Models.Rdbms [Column("noAccessNodeId")] [ForeignKey(typeof(NodeDto), Name = "FK_umbracoAccess_umbracoNode_id2")] - public int AccessDeniedNodeId { get; set; } + public int NoAccessNodeId { get; set; } [Column("createDate")] [Constraint(Default = "getdate()")] diff --git a/src/Umbraco.Core/Persistence/Factories/PublicAccessEntryFactory.cs b/src/Umbraco.Core/Persistence/Factories/PublicAccessEntryFactory.cs index 05e9dd4ce4..a576ef31c4 100644 --- a/src/Umbraco.Core/Persistence/Factories/PublicAccessEntryFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/PublicAccessEntryFactory.cs @@ -9,7 +9,7 @@ namespace Umbraco.Core.Persistence.Factories { public PublicAccessEntry BuildEntity(AccessDto dto) { - var entity = new PublicAccessEntry(dto.Id, dto.NodeId, dto.LoginNodeId, dto.AccessDeniedNodeId, + var entity = new PublicAccessEntry(dto.Id, dto.NodeId, dto.LoginNodeId, dto.NoAccessNodeId, dto.Rules.Select(x => new PublicAccessRule(x.Id, x.AccessId) { RuleValue = x.RuleValue, @@ -33,7 +33,7 @@ namespace Umbraco.Core.Persistence.Factories var dto = new AccessDto { Id = entity.Key, - AccessDeniedNodeId = entity.NoAccessNodeId, + NoAccessNodeId = entity.NoAccessNodeId, LoginNodeId = entity.LoginNodeId, NodeId = entity.ProtectedNodeId, CreateDate = entity.CreateDate, diff --git a/src/Umbraco.Core/Persistence/Mappers/AccessMapper.cs b/src/Umbraco.Core/Persistence/Mappers/AccessMapper.cs new file mode 100644 index 0000000000..170a8f0fc6 --- /dev/null +++ b/src/Umbraco.Core/Persistence/Mappers/AccessMapper.cs @@ -0,0 +1,36 @@ +using System.Collections.Concurrent; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Rdbms; + +namespace Umbraco.Core.Persistence.Mappers +{ + [MapperFor(typeof(PublicAccessEntry))] + public sealed class AccessMapper : BaseMapper + { + private static readonly ConcurrentDictionary PropertyInfoCacheInstance = new ConcurrentDictionary(); + + public AccessMapper() + { + BuildMap(); + } + + #region Overrides of BaseMapper + + internal override ConcurrentDictionary PropertyInfoCache + { + get { return PropertyInfoCacheInstance; } + } + + internal override void BuildMap() + { + CacheMap(src => src.Key, dto => dto.Id); + CacheMap(src => src.LoginNodeId, dto => dto.LoginNodeId); + CacheMap(src => src.NoAccessNodeId, dto => dto.NoAccessNodeId); + CacheMap(src => src.ProtectedNodeId, dto => dto.NodeId); + CacheMap(src => src.CreateDate, dto => dto.CreateDate); + CacheMap(src => src.UpdateDate, dto => dto.UpdateDate); + } + + #endregion + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPublicAccessRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPublicAccessRepository.cs index 23ddb58106..86a2c0739b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPublicAccessRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IPublicAccessRepository.cs @@ -7,6 +7,5 @@ namespace Umbraco.Core.Persistence.Repositories { public interface IPublicAccessRepository : IRepositoryQueryable { - IEnumerable GetEntriesForProtectedContent(params int[] protectedContentIds); } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/PublicAccessRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PublicAccessRepository.cs index 6df10dfaf9..e111451e84 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PublicAccessRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PublicAccessRepository.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.Rdbms; @@ -89,6 +90,29 @@ namespace Umbraco.Core.Persistence.Repositories get { throw new NotImplementedException(); } } + /// + /// The threshold entity count for which the GetAll method will cache entities + /// + /// + /// Set to 1000 just to ensure that all of them are cached, The GetAll on this repository gets called *A lot*, we want max performance + /// + protected override int GetAllThresholdCacheLimit + { + get { return 1000; } + } + + /// + /// Override to false so that a Count check against the db is NOT performed when doing a GetAll without params, we just want to + /// return the raw cache without validation. + /// + /// + /// The GetAll on this repository gets called *A lot*, we want max performance + /// + protected override bool GetAllValidateCount + { + get { return false; } + } + protected override void PersistNewItem(PublicAccessEntry entity) { entity.AddingEntity(); @@ -150,14 +174,6 @@ namespace Umbraco.Core.Persistence.Repositories return entity.Key; } - public IEnumerable GetEntriesForProtectedContent(params int[] protectedContentIds) - { - var sql = GetBaseQuery(false); - sql.Where("umbracoAccess.nodeId in (@nodeIds)", new {nodeIds = protectedContentIds}); - - var factory = new PublicAccessEntryFactory(); - var dtos = Database.Fetch(new AccessRulesRelator().Map, sql); - return dtos.Select(factory.BuildEntity); - } + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs index a6bbff353a..880756bf6c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs @@ -160,7 +160,7 @@ namespace Umbraco.Core.Persistence.Repositories var entities = ids.Select(x => RuntimeCache.GetCacheItem(GetCacheIdKey(x))).ToArray(); if (ids.Count().Equals(entities.Count()) && entities.Any(x => x == null) == false) - return entities.Select(x => (TEntity)x); + return entities; } else { @@ -168,12 +168,20 @@ namespace Umbraco.Core.Persistence.Repositories if (allEntities.Any()) { - //Get count of all entities of current type (TEntity) to ensure cached result is correct - var query = Query.Builder.Where(x => x.Id != 0); - int totalCount = PerformCount(query); - if (allEntities.Count() == totalCount) - return allEntities.Select(x => (TEntity)x); + if (GetAllValidateCount) + { + //Get count of all entities of current type (TEntity) to ensure cached result is correct + var query = Query.Builder.Where(x => x.Id != 0); + int totalCount = PerformCount(query); + + if (allEntities.Count() == totalCount) + return allEntities; + } + else + { + return allEntities; + } } } @@ -186,7 +194,7 @@ namespace Umbraco.Core.Persistence.Repositories // coming back here we don't want to chuck it all into memory, this added cache here // is more for convenience when paging stuff temporarily - if (entityCollection.Length > 100) return entityCollection; + if (entityCollection.Length > GetAllThresholdCacheLimit) return entityCollection; foreach (var entity in entityCollection) { @@ -200,6 +208,28 @@ namespace Umbraco.Core.Persistence.Repositories return entityCollection; } + /// + /// True/false as to validate the total item count when all items are returned from cache, the default is true but this + /// means that a db lookup will occur - though that lookup will probably be significantly less expensive than the normal + /// GetAll method. + /// + /// + /// Overriding this to return false will improve performance of GetAll cache with no params but should only be used + /// for specific circumstances + /// + protected virtual bool GetAllValidateCount + { + get { return true; } + } + + /// + /// The threshold entity count for which the GetAll method will cache entities + /// + protected virtual int GetAllThresholdCacheLimit + { + get { return 100; } + } + protected abstract IEnumerable PerformGetByQuery(IQuery query); /// /// Gets a list of entities by the passed in query diff --git a/src/Umbraco.Core/Services/PublicAccessService.cs b/src/Umbraco.Core/Services/PublicAccessService.cs index aad0919b2f..a24f8db0c4 100644 --- a/src/Umbraco.Core/Services/PublicAccessService.cs +++ b/src/Umbraco.Core/Services/PublicAccessService.cs @@ -44,6 +44,9 @@ namespace Umbraco.Core.Services /// /// /// Returns null if no entry is found + /// + /// NOTE: This method get's called *very* often! This will return the results from cache + /// public PublicAccessEntry GetEntryForContent(string contentPath) { //Get all ids in the path for the content item and ensure they all @@ -65,9 +68,10 @@ namespace Umbraco.Core.Services ids.Reverse(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(UowProvider.GetUnitOfWork())) - { - var entries = repo.GetEntriesForProtectedContent(ids.ToArray()).ToArray(); - //return the entry with the deepest id in the path + { + //This will retrieve from cache! + var entries = repo.GetAll().ToArray(); + foreach (var id in ids) { var found = entries.FirstOrDefault(x => x.ProtectedNodeId == id); @@ -110,7 +114,7 @@ namespace Umbraco.Core.Services var uow = UowProvider.GetUnitOfWork(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(uow)) { - var entry = repo.GetEntriesForProtectedContent(content.Id).FirstOrDefault(); + var entry = repo.GetAll().FirstOrDefault(x => x.ProtectedNodeId == content.Id); if (entry == null) return null; var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType); @@ -143,7 +147,7 @@ namespace Umbraco.Core.Services var uow = UowProvider.GetUnitOfWork(); using (var repo = RepositoryFactory.CreatePublicAccessRepository(uow)) { - var entry = repo.GetEntriesForProtectedContent(content.Id).FirstOrDefault(); + var entry = repo.GetAll().FirstOrDefault(x => x.ProtectedNodeId == content.Id); if (entry == null) return; var existingRule = entry.Rules.FirstOrDefault(x => x.RuleType == ruleType && x.RuleValue == ruleValue); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 9f4519c120..82417d889a 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -330,6 +330,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs index af8b9255d5..77a252a129 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/PublicAccessRepositoryTest.cs @@ -224,41 +224,6 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] - public void Get_All_By_Content_Id() - { - var content = CreateTestData(3).ToArray(); - - var provider = new PetaPocoUnitOfWorkProvider(Logger); - var unitOfWork = provider.GetUnitOfWork(); - using (var repo = new PublicAccessRepository(unitOfWork, CacheHelper, Logger, SqlSyntax)) - { - var entry1 = new PublicAccessEntry(content[0], content[1], content[2], new[] - { - new PublicAccessRule - { - RuleValue = "test", - RuleType = "RoleName" - }, - }); - repo.AddOrUpdate(entry1); - - var entry2 = new PublicAccessEntry(content[1], content[0], content[2], new[] - { - new PublicAccessRule - { - RuleValue = "test", - RuleType = "RoleName" - }, - }); - repo.AddOrUpdate(entry2); - - unitOfWork.Commit(); - - var found = repo.GetEntriesForProtectedContent(content[0].Id, content[1].Id); - Assert.AreEqual(2, found.Count()); - } - } private ContentRepository CreateRepository(IDatabaseUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository) { diff --git a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs index 86a47eeaa8..3e053fcd18 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs @@ -22,6 +22,7 @@ namespace Umbraco.Web.Cache dc.RefreshAll(new Guid(DistributedCache.PublicAccessCacheRefresherId)); } + #endregion #region Application tree cache diff --git a/src/Umbraco.Web/Cache/PageCacheRefresher.cs b/src/Umbraco.Web/Cache/PageCacheRefresher.cs index ca20d5c2de..73f525383e 100644 --- a/src/Umbraco.Web/Cache/PageCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/PageCacheRefresher.cs @@ -76,6 +76,7 @@ namespace Umbraco.Web.Cache content.Instance.ClearDocumentCache(id); DistributedCache.Instance.ClearAllMacroCacheOnCurrentServer(); DistributedCache.Instance.ClearXsltCacheOnCurrentServer(); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); base.Remove(id); } @@ -85,6 +86,7 @@ namespace Umbraco.Web.Cache content.Instance.UpdateDocumentCache(new Document(instance)); DistributedCache.Instance.ClearAllMacroCacheOnCurrentServer(); DistributedCache.Instance.ClearXsltCacheOnCurrentServer(); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); base.Refresh(instance); } @@ -94,6 +96,7 @@ namespace Umbraco.Web.Cache content.Instance.ClearDocumentCache(new Document(instance)); DistributedCache.Instance.ClearAllMacroCacheOnCurrentServer(); DistributedCache.Instance.ClearXsltCacheOnCurrentServer(); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); base.Remove(instance); } } diff --git a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs index 1cf661a78f..47a2d73bdb 100644 --- a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs @@ -78,12 +78,14 @@ namespace Umbraco.Web.Cache public override void RefreshAll() { ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); base.RefreshAll(); } public override void Refresh(int id) { ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheItem(RepositoryBase.GetCacheIdKey(id)); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); content.Instance.UpdateSortOrder(id); base.Refresh(id); } @@ -91,6 +93,7 @@ namespace Umbraco.Web.Cache public override void Remove(int id) { ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheItem(RepositoryBase.GetCacheIdKey(id)); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); base.Remove(id); } @@ -98,6 +101,7 @@ namespace Umbraco.Web.Cache public override void Refresh(IContent instance) { ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheItem(RepositoryBase.GetCacheIdKey(instance.Id)); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); content.Instance.UpdateSortOrder(instance); base.Refresh(instance); } @@ -105,6 +109,7 @@ namespace Umbraco.Web.Cache public override void Remove(IContent instance) { ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheItem(RepositoryBase.GetCacheIdKey(instance.Id)); + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); base.Remove(instance); } @@ -114,6 +119,8 @@ namespace Umbraco.Web.Cache /// public void Refresh(string jsonPayload) { + ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheObjectTypes(); + foreach (var payload in DeserializeFromJsonPayload(jsonPayload)) { ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheItem(RepositoryBase.GetCacheIdKey(payload.Id)); diff --git a/src/umbraco.cms/businesslogic/web/Access.cs b/src/umbraco.cms/businesslogic/web/Access.cs index 8f84faa0bf..dd90e8ae97 100644 --- a/src/umbraco.cms/businesslogic/web/Access.cs +++ b/src/umbraco.cms/businesslogic/web/Access.cs @@ -302,14 +302,8 @@ namespace umbraco.cms.businesslogic.web public static string[] GetAccessingMembershipRoles(int documentId, string path) { - var entry = ApplicationContext.Current.Services.PublicAccessService.GetEntryForContent(path); - if (entry == null) - { - var content = ApplicationContext.Current.Services.ContentService.GetById(documentId); - if (content == null) return new string[] { }; - entry = ApplicationContext.Current.Services.PublicAccessService.GetEntryForContent(content); - if (entry == null) return new string[] { }; - } + var entry = ApplicationContext.Current.Services.PublicAccessService.GetEntryForContent(path.EnsureEndsWith("," + documentId)); + if (entry == null) return new string[] { }; var memberGroupRoleRules = entry.Rules.Where(x => x.RuleType == Constants.Conventions.PublicAccess.MemberRoleRuleType); return memberGroupRoleRules.Select(x => x.RuleValue).ToArray(); @@ -416,14 +410,8 @@ namespace umbraco.cms.businesslogic.web { //var hasAccess = false; - var entry = ApplicationContext.Current.Services.PublicAccessService.GetEntryForContent(path); - if (entry == null) - { - var content = ApplicationContext.Current.Services.ContentService.GetById(documentId); - if (content == null) return true; - entry = ApplicationContext.Current.Services.PublicAccessService.GetEntryForContent(content); - if (entry == null) return true; - } + var entry = ApplicationContext.Current.Services.PublicAccessService.GetEntryForContent(path.EnsureEndsWith("," + documentId)); + if (entry == null) return true; var roles = Roles.GetRolesForUser(member.UserName); return entry.Rules.Any(x => x.RuleType == Constants.Conventions.PublicAccess.MemberRoleRuleType @@ -449,15 +437,7 @@ namespace umbraco.cms.businesslogic.web public static bool IsProtected(int DocumentId, string Path) { - var isProtected = ApplicationContext.Current.Services.PublicAccessService.IsProtected(Path); - if (isProtected == false) - { - var content = ApplicationContext.Current.Services.ContentService.GetById(DocumentId); - if (content == null) return false; - isProtected = ApplicationContext.Current.Services.PublicAccessService.IsProtected(content); - } - return isProtected; - + return ApplicationContext.Current.Services.PublicAccessService.IsProtected(Path.EnsureEndsWith("," + DocumentId)); } public static int GetErrorPage(string Path)