From a97a5dc3fdcf85c3e791530a0fd2d225e74fd002 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 10 Aug 2017 10:52:31 +1000 Subject: [PATCH] Removes need for the IdkMap for content/media services since we have a wrapping repo now, adds tests to verify that caching is active for both INT and GUID --- src/Umbraco.Core/Services/ContentService.cs | 32 ++++----- src/Umbraco.Core/Services/MediaService.cs | 18 ++--- src/Umbraco.Core/Services/ServiceContext.cs | 4 +- .../Repositories/ContentRepositoryTest.cs | 67 ++++++++++++++++--- .../Repositories/MediaRepositoryTest.cs | 57 ++++++++++++++-- 5 files changed, 131 insertions(+), 47 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index e6f402de46..49d745f0cf 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -32,7 +32,6 @@ namespace Umbraco.Core.Services private readonly EntityXmlSerializer _entitySerializer = new EntityXmlSerializer(); private readonly IDataTypeService _dataTypeService; private readonly IUserService _userService; - private readonly IdkMap _idkMap; //Support recursive locks because some of the methods that require locking call other methods that require locking. //for example, the Move method needs to be locked but this calls the Save method which also needs to be locked. @@ -44,8 +43,7 @@ namespace Umbraco.Core.Services ILogger logger, IEventMessagesFactory eventMessagesFactory, IDataTypeService dataTypeService, - IUserService userService, - IdkMap idkMap) + IUserService userService) : base(provider, repositoryFactory, logger, eventMessagesFactory) { if (dataTypeService == null) throw new ArgumentNullException("dataTypeService"); @@ -53,7 +51,6 @@ namespace Umbraco.Core.Services _publishingStrategy = new PublishingStrategy(UowProvider.ScopeProvider, eventMessagesFactory, logger); _dataTypeService = dataTypeService; _userService = userService; - _idkMap = idkMap; } #region Static Queries @@ -431,13 +428,11 @@ namespace Umbraco.Core.Services /// public IContent GetById(Guid key) { - // the repository implements a cache policy on int identifiers, not guids, - // and we are not changing it now, but we still would like to rely on caching - // instead of running a full query against the database, so relying on the - // id-key map, which is fast. - - var a = _idkMap.GetIdForKey(key, UmbracoObjectTypes.Document); - return a.Success ? GetById(a.Result) : null; + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateContentRepository(uow); + return repository.Get(key); + } } /// @@ -1218,13 +1213,14 @@ namespace Umbraco.Core.Services public IContent GetBlueprintById(Guid id) { - // the repository implements a cache policy on int identifiers, not guids, - // and we are not changing it now, but we still would like to rely on caching - // instead of running a full query against the database, so relying on the - // id-key map, which is fast. - - var a = _idkMap.GetIdForKey(id, UmbracoObjectTypes.DocumentBlueprint); - return a.Success ? GetBlueprintById(a.Result) : null; + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateContentBlueprintRepository(uow); + var blueprint = repository.Get(id); + if (blueprint != null) + ((Content)blueprint).IsBlueprint = true; + return blueprint; + } } public void SaveBlueprint(IContent content, int userId = 0) diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 28b1275823..ee4cddb382 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -35,16 +35,14 @@ namespace Umbraco.Core.Services private readonly IDataTypeService _dataTypeService; private readonly IUserService _userService; private readonly MediaFileSystem _mediaFileSystem = FileSystemProviderManager.Current.MediaFileSystem; - private readonly IdkMap _idkMap; - public MediaService(IDatabaseUnitOfWorkProvider provider, RepositoryFactory repositoryFactory, ILogger logger, IEventMessagesFactory eventMessagesFactory, IDataTypeService dataTypeService, IUserService userService, IdkMap idkMap) + public MediaService(IDatabaseUnitOfWorkProvider provider, RepositoryFactory repositoryFactory, ILogger logger, IEventMessagesFactory eventMessagesFactory, IDataTypeService dataTypeService, IUserService userService) : base(provider, repositoryFactory, logger, eventMessagesFactory) { if (dataTypeService == null) throw new ArgumentNullException("dataTypeService"); if (userService == null) throw new ArgumentNullException("userService"); _dataTypeService = dataTypeService; _userService = userService; - _idkMap = idkMap; } /// @@ -365,15 +363,11 @@ namespace Umbraco.Core.Services /// public IMedia GetById(Guid key) { - // the repository implements a cache policy on int identifiers, not guids, - // and we are not changing it now, but we still would like to rely on caching - // instead of running a full query against the database, so relying on the - // id-key map, which is fast. - // - // we should inject the id-key map but ... breaking changes ... yada - - var a = _idkMap.GetIdForKey(key, UmbracoObjectTypes.Media); - return a.Success ? GetById(a.Result) : null; + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateMediaRepository(uow); + return repository.Get(key); + } } /// diff --git a/src/Umbraco.Core/Services/ServiceContext.cs b/src/Umbraco.Core/Services/ServiceContext.cs index 3da10e132b..144d548b78 100644 --- a/src/Umbraco.Core/Services/ServiceContext.cs +++ b/src/Umbraco.Core/Services/ServiceContext.cs @@ -261,10 +261,10 @@ namespace Umbraco.Core.Services _memberService = new Lazy(() => new MemberService(provider, repositoryFactory, logger, eventMessagesFactory, _memberGroupService.Value, _dataTypeService.Value)); if (_contentService == null) - _contentService = new Lazy(() => new ContentService(provider, repositoryFactory, logger, eventMessagesFactory, _dataTypeService.Value, _userService.Value, idkMap)); + _contentService = new Lazy(() => new ContentService(provider, repositoryFactory, logger, eventMessagesFactory, _dataTypeService.Value, _userService.Value)); if (_mediaService == null) - _mediaService = new Lazy(() => new MediaService(provider, repositoryFactory, logger, eventMessagesFactory, _dataTypeService.Value, _userService.Value, idkMap)); + _mediaService = new Lazy(() => new MediaService(provider, repositoryFactory, logger, eventMessagesFactory, _dataTypeService.Value, _userService.Value)); if (_contentTypeService == null) _contentTypeService = new Lazy(() => new ContentTypeService(provider, repositoryFactory, logger, eventMessagesFactory, _contentService.Value, _mediaService.Value)); diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 9301efa68a..8814ef229e 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -2,10 +2,12 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Web; using System.Xml.Linq; using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Cache; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.IO; using Umbraco.Core.Models; @@ -41,32 +43,75 @@ namespace Umbraco.Tests.Persistence.Repositories base.TearDown(); } - private ContentRepository CreateRepository(IScopeUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository, out DataTypeDefinitionRepository dtdRepository) + private ContentRepository CreateRepository(IScopeUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository, out DataTypeDefinitionRepository dtdRepository, CacheHelper cacheHelper = null) { TemplateRepository tr; var ctRepository = CreateRepository(unitOfWork, out contentTypeRepository, out tr); - dtdRepository = new DataTypeDefinitionRepository(unitOfWork, CacheHelper, Logger, SqlSyntax, contentTypeRepository); + dtdRepository = new DataTypeDefinitionRepository(unitOfWork, cacheHelper, Logger, SqlSyntax, contentTypeRepository); return ctRepository; } - private ContentRepository CreateRepository(IScopeUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository) + private ContentRepository CreateRepository(IScopeUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository, CacheHelper cacheHelper = null) { TemplateRepository tr; - return CreateRepository(unitOfWork, out contentTypeRepository, out tr); + return CreateRepository(unitOfWork, out contentTypeRepository, out tr, cacheHelper); } - private ContentRepository CreateRepository(IScopeUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository, out TemplateRepository templateRepository) + private ContentRepository CreateRepository(IScopeUnitOfWork unitOfWork, out ContentTypeRepository contentTypeRepository, out TemplateRepository templateRepository, CacheHelper cacheHelper = null) { - templateRepository = new TemplateRepository(unitOfWork, CacheHelper, Logger, SqlSyntax, Mock.Of(), Mock.Of(), Mock.Of()); - var tagRepository = new TagRepository(unitOfWork, CacheHelper, Logger, SqlSyntax); - contentTypeRepository = new ContentTypeRepository(unitOfWork, CacheHelper, Logger, SqlSyntax, templateRepository); - var repository = new ContentRepository(unitOfWork, CacheHelper, Logger, SqlSyntax, contentTypeRepository, templateRepository, tagRepository, Mock.Of()); + cacheHelper = cacheHelper ?? CacheHelper; + + templateRepository = new TemplateRepository(unitOfWork, cacheHelper, Logger, SqlSyntax, Mock.Of(), Mock.Of(), Mock.Of()); + var tagRepository = new TagRepository(unitOfWork, cacheHelper, Logger, SqlSyntax); + contentTypeRepository = new ContentTypeRepository(unitOfWork, cacheHelper, Logger, SqlSyntax, templateRepository); + var repository = new ContentRepository(unitOfWork, cacheHelper, Logger, SqlSyntax, contentTypeRepository, templateRepository, tagRepository, Mock.Of()); return repository; } - private UserGroupRepository CreateUserGroupRepository(IScopeUnitOfWork unitOfWork) + [Test] + public void Cache_Active_By_Int_And_Guid() { - return new UserGroupRepository(unitOfWork, CacheHelper, Logger, SqlSyntax); + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + + var realCache = new CacheHelper( + new ObjectCacheRuntimeCacheProvider(), + new StaticCacheProvider(), + new StaticCacheProvider(), + new IsolatedRuntimeCache(t => new ObjectCacheRuntimeCacheProvider())); + + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository, cacheHelper: realCache)) + { + DatabaseContext.Database.DisableSqlCount(); + + var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); + var content = MockedContent.CreateSimpleContent(contentType); + contentTypeRepository.AddOrUpdate(contentType); + repository.AddOrUpdate(content); + unitOfWork.Commit(); + + DatabaseContext.Database.EnableSqlCount(); + + //go get it, this should already be cached since the default repository key is the INT + var found = repository.Get(content.Id); + Assert.AreEqual(0, DatabaseContext.Database.SqlCount); + //retrieve again, this should use cache + found = repository.Get(content.Id); + Assert.AreEqual(0, DatabaseContext.Database.SqlCount); + + //reset counter + DatabaseContext.Database.DisableSqlCount(); + DatabaseContext.Database.EnableSqlCount(); + + //now get by GUID, this won't be cached yet because the default repo key is not a GUID + found = repository.Get(content.Key); + var sqlCount = DatabaseContext.Database.SqlCount; + Assert.Greater(sqlCount, 0); + //retrieve again, this should use cache now + found = repository.Get(content.Key); + Assert.AreEqual(sqlCount, DatabaseContext.Database.SqlCount); + } } [Test] diff --git a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs index 4dfaccd7ab..a2f82b0ad0 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs @@ -5,6 +5,7 @@ using System.Xml.Linq; using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Cache; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; using Umbraco.Core.Models; @@ -35,14 +36,62 @@ namespace Umbraco.Tests.Persistence.Repositories CreateTestData(); } - private MediaRepository CreateRepository(IScopeUnitOfWork unitOfWork, out MediaTypeRepository mediaTypeRepository) + private MediaRepository CreateRepository(IScopeUnitOfWork unitOfWork, out MediaTypeRepository mediaTypeRepository, CacheHelper cacheHelper = null) { - mediaTypeRepository = new MediaTypeRepository(unitOfWork, CacheHelper, Mock.Of(), SqlSyntax); - var tagRepository = new TagRepository(unitOfWork, CacheHelper, Mock.Of(), SqlSyntax); - var repository = new MediaRepository(unitOfWork, CacheHelper, Mock.Of(), SqlSyntax, mediaTypeRepository, tagRepository, Mock.Of()); + cacheHelper = cacheHelper ?? CacheHelper; + + mediaTypeRepository = new MediaTypeRepository(unitOfWork, cacheHelper, Mock.Of(), SqlSyntax); + var tagRepository = new TagRepository(unitOfWork, cacheHelper, Mock.Of(), SqlSyntax); + var repository = new MediaRepository(unitOfWork, cacheHelper, Mock.Of(), SqlSyntax, mediaTypeRepository, tagRepository, Mock.Of()); return repository; } + [Test] + public void Cache_Active_By_Int_And_Guid() + { + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + MediaTypeRepository mediaTypeRepository; + + var realCache = new CacheHelper( + new ObjectCacheRuntimeCacheProvider(), + new StaticCacheProvider(), + new StaticCacheProvider(), + new IsolatedRuntimeCache(t => new ObjectCacheRuntimeCacheProvider())); + + using (var repository = CreateRepository(unitOfWork, out mediaTypeRepository, cacheHelper: realCache)) + { + DatabaseContext.Database.DisableSqlCount(); + + var mediaType = MockedContentTypes.CreateSimpleMediaType("umbTextpage1", "Textpage"); + var media = MockedMedia.CreateSimpleMedia(mediaType, "hello", -1); + mediaTypeRepository.AddOrUpdate(mediaType); + repository.AddOrUpdate(media); + unitOfWork.Commit(); + + DatabaseContext.Database.EnableSqlCount(); + + //go get it, this should already be cached since the default repository key is the INT + var found = repository.Get(media.Id); + Assert.AreEqual(0, DatabaseContext.Database.SqlCount); + //retrieve again, this should use cache + found = repository.Get(media.Id); + Assert.AreEqual(0, DatabaseContext.Database.SqlCount); + + //reset counter + DatabaseContext.Database.DisableSqlCount(); + DatabaseContext.Database.EnableSqlCount(); + + //now get by GUID, this won't be cached yet because the default repo key is not a GUID + found = repository.Get(media.Key); + var sqlCount = DatabaseContext.Database.SqlCount; + Assert.Greater(sqlCount, 0); + //retrieve again, this should use cache now + found = repository.Get(media.Key); + Assert.AreEqual(sqlCount, DatabaseContext.Database.SqlCount); + } + } + [Test] public void Rebuild_All_Xml_Structures() {