From cd6380689549ee8ea89683ef14696fa34509d771 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 6 Mar 2018 16:11:27 +0100 Subject: [PATCH] U4-10756 - id/guid cache --- src/Umbraco.Core/Services/IdkMap.cs | 164 +++++++++++++++--- src/Umbraco.Web/Cache/MediaCacheRefresher.cs | 5 +- .../Cache/UnpublishedPageCacheRefresher.cs | 4 +- src/Umbraco.Web/PublishedContentQuery.cs | 26 +-- 4 files changed, 155 insertions(+), 44 deletions(-) diff --git a/src/Umbraco.Core/Services/IdkMap.cs b/src/Umbraco.Core/Services/IdkMap.cs index 91367a6316..42bbc8a146 100644 --- a/src/Umbraco.Core/Services/IdkMap.cs +++ b/src/Umbraco.Core/Services/IdkMap.cs @@ -22,19 +22,111 @@ namespace Umbraco.Core.Services // note - no need for uow, scope would be enough, but a pain to wire // note - for pure read-only we might want to *not* enforce a transaction? - public Attempt GetIdForKey(Guid key, UmbracoObjectTypes umbracoObjectType) + // notes + // + // - this class assumes that the id/guid map is unique; that is, if an id and a guid map + // to each other, then the id will never map to another guid, and the guid will never map + // to another id + // + // - LeeK's solution in 7.7 was to look for the id/guid in the content cache "on demand" via + // XPath, which is probably fast enough but cannot deal with media ids + // see https://github.com/umbraco/Umbraco-CMS/pull/2398 + // + // - Andy's solution in a package was to prefetch all by sql; it cannot prefecth reserved ids + // as we don't know the corresponding object type, but that's not a big issue + // see https://github.com/AndyButland/UmbracoUdiToIdCache + // + // - the original IdkMap implementation that was used by services, did a database lookup on + // each cache miss, which is fine enough for services, but would be really slow at content + // cache level - so this implementation prefetches all document and media ids + // + // - and this implementation works for document and media ids + // + // - cache is cleared by MediaCacheRefresher, UnpublishedPageCacheRefresher, and other + // refreshers - because id/guid map is unique, we only clear to avoid leaking memory, 'cos + // we don't risk caching obsolete values - and only when actually deleting + + private void PopulateLocked() + { + // don't if not empty + if (_key2Id.Count > 0) return; + + using (var uow = _uowProvider.GetUnitOfWork()) + { + // populate content and media items + var types = new[] { Constants.ObjectTypes.Document, Constants.ObjectTypes.Media }; + var values = uow.Database.Fetch("SELECT id, uniqueId, nodeObjectType FROM umbracoNode WHERE nodeObjectType IN @types", new { types }); + foreach (var value in values) + { + UmbracoObjectTypes umbracoObjectType = UmbracoObjectTypesExtensions.GetUmbracoObjectType(value.NodeObjectType); + _id2Key.Add(value.Id, new TypedId(value.UniqueId, umbracoObjectType)); + _key2Id.Add(value.UniqueId, new TypedId(value.Id, umbracoObjectType)); + } + } + } + + private Attempt PopulateAndGetIdForKey(Guid key, UmbracoObjectTypes umbracoObjectType) { - TypedId id; try { - _locker.EnterReadLock(); - if (_key2Id.TryGetValue(key, out id) && id.UmbracoObjectType == umbracoObjectType) return Attempt.Succeed(id.Id); + _locker.EnterWriteLock(); + + PopulateLocked(); + + return _key2Id.TryGetValue(key, out var id) && id.UmbracoObjectType == umbracoObjectType + ? Attempt.Succeed(id.Id) + : Attempt.Fail(); + } finally { if (_locker.IsReadLockHeld) _locker.ExitReadLock(); } + } + + private Attempt PopulateAndGetKeyForId(int id, UmbracoObjectTypes umbracoObjectType) + { + try + { + _locker.EnterWriteLock(); + + PopulateLocked(); + + return _id2Key.TryGetValue(id, out var key) && key.UmbracoObjectType == umbracoObjectType + ? Attempt.Succeed(key.Id) + : Attempt.Fail(); + } + finally + { + if (_locker.IsReadLockHeld) + _locker.ExitReadLock(); + } + } + + public Attempt GetIdForKey(Guid key, UmbracoObjectTypes umbracoObjectType) + { + bool empty; + + try + { + _locker.EnterReadLock(); + if (_key2Id.TryGetValue(key, out var id) && id.UmbracoObjectType == umbracoObjectType) return Attempt.Succeed(id.Id); + empty = _key2Id.Count == 0; + } + finally + { + if (_locker.IsReadLockHeld) + _locker.ExitReadLock(); + } + + // if cache is empty and looking for a document or a media, + // populate the cache at once and return what we found + if (empty && (umbracoObjectType == UmbracoObjectTypes.Document || umbracoObjectType == UmbracoObjectTypes.Media)) + return PopulateAndGetIdForKey(key, umbracoObjectType); + + // optimize for read speed: reading database outside a lock means that we could read + // multiple times, but we don't lock the cache while accessing the database = better int? val; using (var uow = _uowProvider.GetUnitOfWork()) @@ -83,13 +175,23 @@ namespace Umbraco.Core.Services return GetIdForKey(guidUdi.Guid, umbracoType); } + public Attempt GetUdiForId(int id, UmbracoObjectTypes umbracoObjectType) + { + var keyAttempt = GetKeyForId(id, umbracoObjectType); + return keyAttempt + ? Attempt.Succeed(new GuidUdi(Constants.UdiEntityType.FromUmbracoObjectType(umbracoObjectType), keyAttempt.Result)) + : Attempt.Fail(); + } + public Attempt GetKeyForId(int id, UmbracoObjectTypes umbracoObjectType) { - TypedId key; + bool empty; + try { _locker.EnterReadLock(); - if (_id2Key.TryGetValue(id, out key) && key.UmbracoObjectType == umbracoObjectType) return Attempt.Succeed(key.Id); + if (_id2Key.TryGetValue(id, out var key) && key.UmbracoObjectType == umbracoObjectType) return Attempt.Succeed(key.Id); + empty = _id2Key.Count == 0; } finally { @@ -97,6 +199,14 @@ namespace Umbraco.Core.Services _locker.ExitReadLock(); } + // if cache is empty and looking for a document or a media, + // populate the cache at once and return what we found + if (empty && (umbracoObjectType == UmbracoObjectTypes.Document || umbracoObjectType == UmbracoObjectTypes.Media)) + return PopulateAndGetKeyForId(id, umbracoObjectType); + + // optimize for read speed: reading database outside a lock means that we could read + // multiple times, but we don't lock the cache while accessing the database = better + Guid? val; using (var uow = _uowProvider.GetUnitOfWork()) { @@ -142,6 +252,8 @@ namespace Umbraco.Core.Services return guid; } + // invoked on UnpublishedPageCacheRefresher.RefreshAll + // anything else will use the id-specific overloads public void ClearCache() { try @@ -162,8 +274,7 @@ namespace Umbraco.Core.Services try { _locker.EnterWriteLock(); - TypedId key; - if (_id2Key.TryGetValue(id, out key) == false) return; + if (_id2Key.TryGetValue(id, out var key) == false) return; _id2Key.Remove(id); _key2Id.Remove(key.Id); } @@ -179,8 +290,7 @@ namespace Umbraco.Core.Services try { _locker.EnterWriteLock(); - TypedId id; - if (_key2Id.TryGetValue(key, out id) == false) return; + if (_key2Id.TryGetValue(key, out var id) == false) return; _id2Key.Remove(id.Id); _key2Id.Remove(key); } @@ -191,26 +301,28 @@ namespace Umbraco.Core.Services } } + // ReSharper disable ClassNeverInstantiated.Local + // ReSharper disable UnusedAutoPropertyAccessor.Local + private class TypedIdDto + { + public int Id { get; set; } + public Guid UniqueId { get; set; } + public Guid NodeObjectType { get; set; } + } + // ReSharper restore ClassNeverInstantiated.Local + // ReSharper restore UnusedAutoPropertyAccessor.Local + private struct TypedId { - private readonly T _id; - private readonly UmbracoObjectTypes _umbracoObjectType; - - public T Id - { - get { return _id; } - } - - public UmbracoObjectTypes UmbracoObjectType - { - get { return _umbracoObjectType; } - } - public TypedId(T id, UmbracoObjectTypes umbracoObjectType) { - _umbracoObjectType = umbracoObjectType; - _id = id; + UmbracoObjectType = umbracoObjectType; + Id = id; } + + public UmbracoObjectTypes UmbracoObjectType { get; } + + public T Id { get; } } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Cache/MediaCacheRefresher.cs b/src/Umbraco.Web/Cache/MediaCacheRefresher.cs index 3613da426d..783ca95841 100644 --- a/src/Umbraco.Web/Cache/MediaCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/MediaCacheRefresher.cs @@ -155,7 +155,8 @@ namespace Umbraco.Web.Cache foreach (var payload in payloads) { - ApplicationContext.Current.Services.IdkMap.ClearCache(payload.Id); + if (payload.Operation == OperationType.Deleted) + ApplicationContext.Current.Services.IdkMap.ClearCache(payload.Id); var mediaCache = ApplicationContext.Current.ApplicationCache.IsolatedRuntimeCache.GetCache(); @@ -190,4 +191,4 @@ namespace Umbraco.Web.Cache } } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs index 2941357f19..6404d60b40 100644 --- a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs @@ -84,7 +84,6 @@ namespace Umbraco.Web.Cache public override void Refresh(int id) { - ApplicationContext.Current.Services.IdkMap.ClearCache(id); ClearRepositoryCacheItemById(id); ClearAllIsolatedCacheByEntityType(); content.Instance.UpdateSortOrder(id); @@ -104,7 +103,6 @@ namespace Umbraco.Web.Cache public override void Refresh(IContent instance) { - ApplicationContext.Current.Services.IdkMap.ClearCache(instance.Id); ClearRepositoryCacheItemById(instance.Id); ClearAllIsolatedCacheByEntityType(); content.Instance.UpdateSortOrder(instance); @@ -150,4 +148,4 @@ namespace Umbraco.Web.Cache } } } -} \ No newline at end of file +} diff --git a/src/Umbraco.Web/PublishedContentQuery.cs b/src/Umbraco.Web/PublishedContentQuery.cs index 8cbbfd3fda..d231cd04dd 100644 --- a/src/Umbraco.Web/PublishedContentQuery.cs +++ b/src/Umbraco.Web/PublishedContentQuery.cs @@ -13,6 +13,7 @@ using Umbraco.Core; using Umbraco.Core.Configuration; using Umbraco.Core.Dynamics; using Umbraco.Core.Models; +using Umbraco.Core.Services; using Umbraco.Core.Xml; using Umbraco.Web.Models; using Umbraco.Web.PublishedCache; @@ -55,6 +56,9 @@ namespace Umbraco.Web _dynamicContentQuery = dynamicContentQuery; } + // TODO use this to implement media-by-GUID but is a breaking change? + private IdkMap IdkMap => ApplicationContext.Current.Services.IdkMap; // fixme inject - v8 + #region Content public IPublishedContent TypedContent(int id) @@ -67,7 +71,7 @@ namespace Umbraco.Web public IPublishedContent TypedContent(Guid id) { return _typedContentQuery == null - ? TypedDocumentById(id, _contentCache) + ? TypedDocumentById(id, _contentCache, UmbracoObjectTypes.Document) : _typedContentQuery.TypedContent(id); } @@ -88,7 +92,7 @@ namespace Umbraco.Web public IEnumerable TypedContent(IEnumerable ids) { return _typedContentQuery == null - ? TypedDocumentsByIds(_contentCache, ids) + ? TypedDocumentsByIds(_contentCache, ids, UmbracoObjectTypes.Document) : _typedContentQuery.TypedContent(ids); } @@ -232,13 +236,10 @@ namespace Umbraco.Web return doc; } - private IPublishedContent TypedDocumentById(Guid id, ContextualPublishedCache cache) + private IPublishedContent TypedDocumentById(Guid key, ContextualPublishedCache cache, UmbracoObjectTypes umbracoObjectType) { - // todo: in v8, implement in a more efficient way - var legacyXml = UmbracoConfig.For.UmbracoSettings().Content.UseLegacyXmlSchema; - var xpath = legacyXml ? "//node [@key=$guid]" : "//* [@key=$guid]"; - var doc = cache.GetSingleByXPath(xpath, new XPathVariable("guid", id.ToString())); - return doc; + var idAttempt = IdkMap.GetIdForKey(key, umbracoObjectType); + return idAttempt ? TypedDocumentById(idAttempt.Result, cache) : null; } private IPublishedContent TypedDocumentByXPath(string xpath, XPathVariable[] vars, ContextualPublishedContentCache cache) @@ -259,10 +260,9 @@ namespace Umbraco.Web return ids.Select(eachId => TypedDocumentById(eachId, cache)).WhereNotNull(); } - private IEnumerable TypedDocumentsByIds(ContextualPublishedCache cache, IEnumerable ids) + private IEnumerable TypedDocumentsByIds(ContextualPublishedCache cache, IEnumerable ids, UmbracoObjectTypes umbracoObjectType) { - // todo: in v8, implement in a more efficient way - return ids.Select(eachId => TypedDocumentById(eachId, cache)).WhereNotNull(); + return ids.Select(eachId => TypedDocumentById(eachId, cache, umbracoObjectType)).WhereNotNull(); } private IEnumerable TypedDocumentsByXPath(string xpath, XPathVariable[] vars, ContextualPublishedContentCache cache) @@ -292,7 +292,7 @@ namespace Umbraco.Web private dynamic DocumentById(Guid id, ContextualPublishedCache cache, object ifNotFound) { - var doc = TypedDocumentById(id, cache); + var doc = TypedDocumentById(id, cache, UmbracoObjectTypes.Document); return doc == null ? ifNotFound : new DynamicPublishedContent(doc).AsDynamic(); @@ -548,4 +548,4 @@ namespace Umbraco.Web #endregion } -} \ No newline at end of file +}