From 2e96de54499427c977bacd03784519ef8ddc47fa Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 28 Apr 2014 18:53:21 +1000 Subject: [PATCH] Moves the CRUD logic from the media service to the media repo for dealing with content xml items and ensures it's done in the same transaction, streamlines how this process is done between the content, media, member services, adds test for it. --- src/Umbraco.Core/Models/ContentXmlEntity.cs | 13 ++--- .../Repositories/ContentRepository.cs | 21 +++++---- .../Repositories/ContentXmlRepository.cs | 15 +++--- .../Interfaces/IContentRepository.cs | 5 +- .../Interfaces/IMediaRepository.cs | 12 ++++- .../Repositories/MediaRepository.cs | 11 +++++ src/Umbraco.Core/Services/ContentService.cs | 16 +++---- .../Services/EntityXmlSerializer.cs | 2 + src/Umbraco.Core/Services/MediaService.cs | 47 +++++++------------ .../Services/MediaServiceTests.cs | 23 +++++++++ 10 files changed, 100 insertions(+), 65 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentXmlEntity.cs b/src/Umbraco.Core/Models/ContentXmlEntity.cs index ffa3bd2ff8..6ba9427073 100644 --- a/src/Umbraco.Core/Models/ContentXmlEntity.cs +++ b/src/Umbraco.Core/Models/ContentXmlEntity.cs @@ -8,12 +8,13 @@ namespace Umbraco.Core.Models /// Used in content/media/member repositories in order to add this type of entity to the persisted collection to be saved /// in a single transaction during saving an entity /// - internal class ContentXmlEntity : IAggregateRoot + internal class ContentXmlEntity : IAggregateRoot + where TContent : IContentBase { private readonly bool _entityExists; - private readonly Func _xml; + private readonly Func _xml; - public ContentXmlEntity(bool entityExists, IContentBase content, Func xml) + public ContentXmlEntity(bool entityExists, TContent content, Func xml) { if (content == null) throw new ArgumentNullException("content"); _entityExists = entityExists; @@ -23,9 +24,9 @@ namespace Umbraco.Core.Models public XElement Xml { - get { return _xml(); } + get { return _xml(Content); } } - public IContentBase Content { get; private set; } + public TContent Content { get; private set; } public int Id { @@ -44,7 +45,7 @@ namespace Umbraco.Core.Models public object DeepClone() { - var clone = (ContentXmlEntity)MemberwiseClone(); + var clone = (ContentXmlEntity)MemberwiseClone(); //Automatically deep clone ref properties that are IDeepCloneable DeepCloneHelper.DeepCloneRefProperties(this, clone); return clone; diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 19b730dd07..9b2d728ab5 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -13,6 +13,7 @@ using Umbraco.Core.Models.Rdbms; using Umbraco.Core.Persistence.Caching; using Umbraco.Core.Persistence.Factories; using Umbraco.Core.Persistence.Querying; +using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; namespace Umbraco.Core.Persistence.Repositories @@ -26,7 +27,7 @@ namespace Umbraco.Core.Persistence.Repositories private readonly ITemplateRepository _templateRepository; private readonly CacheHelper _cacheHelper; private readonly ContentPreviewRepository _contentPreviewRepository; - private readonly ContentXmlRepository _contentXmlRepository; + private readonly ContentXmlRepository _contentXmlRepository; public ContentRepository(IDatabaseUnitOfWork work, IContentTypeRepository contentTypeRepository, ITemplateRepository templateRepository, CacheHelper cacheHelper) : base(work) @@ -35,7 +36,7 @@ namespace Umbraco.Core.Persistence.Repositories _templateRepository = templateRepository; _cacheHelper = cacheHelper; _contentPreviewRepository = new ContentPreviewRepository(work, NullCacheProvider.Current); - _contentXmlRepository = new ContentXmlRepository(work, NullCacheProvider.Current); + _contentXmlRepository = new ContentXmlRepository(work, NullCacheProvider.Current); EnsureUniqueNaming = true; } @@ -47,7 +48,7 @@ namespace Umbraco.Core.Persistence.Repositories _templateRepository = templateRepository; _cacheHelper = cacheHelper; _contentPreviewRepository = new ContentPreviewRepository(work, NullCacheProvider.Current); - _contentXmlRepository = new ContentXmlRepository(work, NullCacheProvider.Current); + _contentXmlRepository = new ContentXmlRepository(work, NullCacheProvider.Current); EnsureUniqueNaming = true; } @@ -566,18 +567,20 @@ namespace Umbraco.Core.Persistence.Repositories /// Adds/updates content/published xml /// /// - public void AddOrUpdateContentXml(IContent content, Func xml) + /// + public void AddOrUpdateContentXml(IContent content, Func xml) { var contentExists = Database.ExecuteScalar("SELECT COUNT(nodeId) FROM cmsContentXml WHERE nodeId = @Id", new { Id = content.Id }) != 0; - _contentXmlRepository.AddOrUpdate(new ContentXmlEntity(contentExists, content, xml)); + _contentXmlRepository.AddOrUpdate(new ContentXmlEntity(contentExists, content, xml)); } /// /// Adds/updates preview xml /// /// - public void AddOrUpdatePreviewXml(IContent content, Func xml) + /// + public void AddOrUpdatePreviewXml(IContent content, Func xml) { var previewExists = Database.ExecuteScalar("SELECT COUNT(nodeId) FROM cmsPreviewXml WHERE nodeId = @Id AND versionId = @Version", @@ -585,7 +588,7 @@ namespace Umbraco.Core.Persistence.Repositories _contentPreviewRepository.AddOrUpdate(new ContentPreviewEntity(previewExists, content, xml)); } - + #endregion /// @@ -653,9 +656,9 @@ namespace Umbraco.Core.Persistence.Repositories /// Used content repository in order to add an entity to the persisted collection to be saved /// in a single transaction during saving an entity /// - private class ContentPreviewEntity : ContentXmlEntity + private class ContentPreviewEntity : ContentXmlEntity { - public ContentPreviewEntity(bool previewExists, IContentBase content, Func xml) + public ContentPreviewEntity(bool previewExists, IContent content, Func xml) : base(previewExists, content, xml) { Version = content.Version; diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentXmlRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentXmlRepository.cs index 3590ad8815..93cf67ba49 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentXmlRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentXmlRepository.cs @@ -12,7 +12,8 @@ namespace Umbraco.Core.Persistence.Repositories /// /// Internal class to handle content/published xml insert/update based on standard principles and units of work with transactions /// - internal class ContentXmlRepository : PetaPocoRepositoryBase + internal class ContentXmlRepository : PetaPocoRepositoryBase> + where TContent : IContentBase { public ContentXmlRepository(IDatabaseUnitOfWork work, IRepositoryCacheProvider cache) : base(work, cache) @@ -20,17 +21,17 @@ namespace Umbraco.Core.Persistence.Repositories } #region Not implemented (don't need to for the purposes of this repo) - protected override ContentXmlEntity PerformGet(int id) + protected override ContentXmlEntity PerformGet(int id) { throw new NotImplementedException(); } - protected override IEnumerable PerformGetAll(params int[] ids) + protected override IEnumerable> PerformGetAll(params int[] ids) { throw new NotImplementedException(); } - protected override IEnumerable PerformGetByQuery(IQuery query) + protected override IEnumerable> PerformGetByQuery(IQuery> query) { throw new NotImplementedException(); } @@ -55,13 +56,13 @@ namespace Umbraco.Core.Persistence.Repositories get { throw new NotImplementedException(); } } - protected override void PersistDeletedItem(ContentXmlEntity entity) + protected override void PersistDeletedItem(ContentXmlEntity entity) { throw new NotImplementedException(); } #endregion - protected override void PersistNewItem(ContentXmlEntity entity) + protected override void PersistNewItem(ContentXmlEntity entity) { if (entity.Content.HasIdentity == false) { @@ -72,7 +73,7 @@ namespace Umbraco.Core.Persistence.Repositories Database.Insert(poco); } - protected override void PersistUpdatedItem(ContentXmlEntity entity) + protected override void PersistUpdatedItem(ContentXmlEntity entity) { if (entity.Content.HasIdentity == false) { diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs index a47999f5c4..9c39f6bed3 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs @@ -44,13 +44,14 @@ namespace Umbraco.Core.Persistence.Repositories /// /// /// - void AddOrUpdateContentXml(IContent content, Func xml); + void AddOrUpdateContentXml(IContent content, Func xml); /// /// Used to add/update preview xml for the content item /// /// /// - void AddOrUpdatePreviewXml(IContent content, Func xml); + void AddOrUpdatePreviewXml(IContent content, Func xml); + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IMediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IMediaRepository.cs index 5d0e088d39..7573bac60c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IMediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IMediaRepository.cs @@ -1,8 +1,18 @@ -using Umbraco.Core.Models; +using System; +using System.Xml.Linq; +using Umbraco.Core.Models; namespace Umbraco.Core.Persistence.Repositories { public interface IMediaRepository : IRepositoryVersionable { + + /// + /// Used to add/update published xml for the media item + /// + /// + /// + void AddOrUpdateContentXml(IMedia content, Func xml); + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs index 47f202e34e..0464917d18 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Xml.Linq; using Umbraco.Core.Configuration; using Umbraco.Core.IO; using Umbraco.Core.Models; @@ -20,11 +21,13 @@ namespace Umbraco.Core.Persistence.Repositories internal class MediaRepository : VersionableRepositoryBase, IMediaRepository { private readonly IMediaTypeRepository _mediaTypeRepository; + private readonly ContentXmlRepository _contentXmlRepository; public MediaRepository(IDatabaseUnitOfWork work, IMediaTypeRepository mediaTypeRepository) : base(work) { _mediaTypeRepository = mediaTypeRepository; + _contentXmlRepository = new ContentXmlRepository(work, NullCacheProvider.Current); EnsureUniqueNaming = true; } @@ -33,6 +36,7 @@ namespace Umbraco.Core.Persistence.Repositories : base(work, cache) { _mediaTypeRepository = mediaTypeRepository; + _contentXmlRepository = new ContentXmlRepository(work, NullCacheProvider.Current); EnsureUniqueNaming = true; } @@ -174,6 +178,13 @@ namespace Umbraco.Core.Persistence.Repositories return media; } + public void AddOrUpdateContentXml(IMedia content, Func xml) + { + var contentExists = Database.ExecuteScalar("SELECT COUNT(nodeId) FROM cmsContentXml WHERE nodeId = @Id", new { Id = content.Id }) != 0; + + _contentXmlRepository.AddOrUpdate(new ContentXmlEntity(contentExists, content, xml)); + } + protected override void PerformDeleteVersion(int id, Guid versionId) { Database.Delete("WHERE nodeId = @Id AND versionId = @VersionId", new { Id = id, VersionId = versionId }); diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 5336d70a42..77837595aa 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1350,15 +1350,13 @@ namespace Umbraco.Core.Services repository.AddOrUpdate(content); //Generate a new preview - var local = content; - repository.AddOrUpdatePreviewXml(content, () => _entitySerializer.Serialize(this, _dataTypeService, local)); + repository.AddOrUpdatePreviewXml(content, c => _entitySerializer.Serialize(this, _dataTypeService, c)); } foreach (var content in shouldBePublished) { //Create and Save ContentXml DTO - var local = content; - repository.AddOrUpdateContentXml(content, () => _entitySerializer.Serialize(this, _dataTypeService, local)); + repository.AddOrUpdateContentXml(content, c => _entitySerializer.Serialize(this, _dataTypeService, c)); } uow.Commit(); @@ -1511,6 +1509,8 @@ namespace Umbraco.Core.Services //TODO: WE should make a base class for ContentService and MediaService to share! // currently we have this logic duplicated (nearly the same) for media types and soon to be member types + //TODO: This needs to be put into the ContentRepository, all CUD logic! + /// /// Rebuilds all xml content in the cmsContentXml table for all documents /// @@ -1776,13 +1776,12 @@ namespace Umbraco.Core.Services repository.AddOrUpdate(content); //Generate a new preview - var local = content; - repository.AddOrUpdatePreviewXml(content, () => _entitySerializer.Serialize(this, _dataTypeService, local)); + repository.AddOrUpdatePreviewXml(content, c => _entitySerializer.Serialize(this, _dataTypeService, c)); if (published) { //Content Xml - repository.AddOrUpdateContentXml(content, () => _entitySerializer.Serialize(this, _dataTypeService, local)); + repository.AddOrUpdateContentXml(content, c => _entitySerializer.Serialize(this, _dataTypeService, c)); } uow.Commit(); @@ -1844,8 +1843,7 @@ namespace Umbraco.Core.Services repository.AddOrUpdate(content); //Generate a new preview - var local = content; - repository.AddOrUpdatePreviewXml(content, () => _entitySerializer.Serialize(this, _dataTypeService, local)); + repository.AddOrUpdatePreviewXml(content, c => _entitySerializer.Serialize(this, _dataTypeService, c)); uow.Commit(); } diff --git a/src/Umbraco.Core/Services/EntityXmlSerializer.cs b/src/Umbraco.Core/Services/EntityXmlSerializer.cs index b16e74df4d..cddea75375 100644 --- a/src/Umbraco.Core/Services/EntityXmlSerializer.cs +++ b/src/Umbraco.Core/Services/EntityXmlSerializer.cs @@ -10,6 +10,8 @@ using umbraco.interfaces; namespace Umbraco.Core.Services { + //TODO: Move the rest of the logic for the PackageService.Export methods to here! + /// /// A helper class to serialize entities to XML /// diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 1e8f0c5380..28cbe7da1c 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -156,10 +156,10 @@ namespace Umbraco.Core.Services { media.CreatorId = userId; repository.AddOrUpdate(media); - uow.Commit(); - var xml = _entitySerializer.Serialize(this, _dataTypeService, media); - CreateAndSaveMediaXml(xml, media.Id, uow.Database); + repository.AddOrUpdateContentXml(media, m => _entitySerializer.Serialize(this, _dataTypeService, m)); + + uow.Commit(); } } @@ -211,10 +211,8 @@ namespace Umbraco.Core.Services { media.CreatorId = userId; repository.AddOrUpdate(media); + repository.AddOrUpdateContentXml(media, m => _entitySerializer.Serialize(this, _dataTypeService, m)); uow.Commit(); - - var xml = _entitySerializer.Serialize(this, _dataTypeService, media); - CreateAndSaveMediaXml(xml, media.Id, uow.Database); } } @@ -809,10 +807,9 @@ namespace Umbraco.Core.Services { media.CreatorId = userId; repository.AddOrUpdate(media); - uow.Commit(); + repository.AddOrUpdateContentXml(media, m => _entitySerializer.Serialize(this, _dataTypeService, m)); - var xml = _entitySerializer.Serialize(this, _dataTypeService, media); - CreateAndSaveMediaXml(xml, media.Id, uow.Database); + uow.Commit(); } } @@ -847,16 +844,11 @@ namespace Umbraco.Core.Services { media.CreatorId = userId; repository.AddOrUpdate(media); + repository.AddOrUpdateContentXml(media, m => _entitySerializer.Serialize(this, _dataTypeService, m)); } //commit the whole lot in one go uow.Commit(); - - foreach (var media in asArray) - { - var xml = _entitySerializer.Serialize(this, _dataTypeService, media); - CreateAndSaveMediaXml(xml, media.Id, uow.Database); - } } if (raiseEvents) @@ -884,8 +876,6 @@ namespace Umbraco.Core.Services return false; } - var shouldBeCached = new List(); - using (new WriteLock(Locker)) { var uow = _uowProvider.GetUnitOfWork(); @@ -907,17 +897,10 @@ namespace Umbraco.Core.Services i++; repository.AddOrUpdate(media); - shouldBeCached.Add(media); + repository.AddOrUpdateContentXml(media, m => _entitySerializer.Serialize(this, _dataTypeService, m)); } uow.Commit(); - - foreach (var content in shouldBeCached) - { - //Create and Save ContentXml DTO - var xml = _entitySerializer.Serialize(this, _dataTypeService, content); - CreateAndSaveMediaXml(xml, content.Id, uow.Database); - } } } @@ -929,6 +912,8 @@ namespace Umbraco.Core.Services return true; } + //TODO: This needs to be put into the MediaRepository, all CUD logic! + /// /// Rebuilds all xml content in the cmsContentXml table for all media /// @@ -1052,12 +1037,12 @@ namespace Umbraco.Core.Services return list; } - private void CreateAndSaveMediaXml(XElement xml, int id, UmbracoDatabase db) - { - var poco = new ContentXmlDto { NodeId = id, Xml = xml.ToString(SaveOptions.None) }; - var exists = db.FirstOrDefault("WHERE nodeId = @Id", new { Id = id }) != null; - int result = exists ? db.Update(poco) : Convert.ToInt32(db.Insert(poco)); - } + //private void CreateAndSaveMediaXml(XElement xml, int id, UmbracoDatabase db) + //{ + // var poco = new ContentXmlDto { NodeId = id, Xml = xml.ToString(SaveOptions.None) }; + // var exists = db.FirstOrDefault("WHERE nodeId = @Id", new { Id = id }) != null; + // int result = exists ? db.Update(poco) : Convert.ToInt32(db.Insert(poco)); + //} private IMediaType FindMediaTypeByAlias(string mediaTypeAlias) { diff --git a/src/Umbraco.Tests/Services/MediaServiceTests.cs b/src/Umbraco.Tests/Services/MediaServiceTests.cs index 10dfc6790b..4f76f4f7f8 100644 --- a/src/Umbraco.Tests/Services/MediaServiceTests.cs +++ b/src/Umbraco.Tests/Services/MediaServiceTests.cs @@ -4,6 +4,10 @@ using System.Linq; using System.Text; using NUnit.Framework; using Umbraco.Core.Models; +using Umbraco.Core.Models.Rdbms; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Repositories; +using Umbraco.Core.Persistence.UnitOfWork; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; @@ -76,6 +80,25 @@ namespace Umbraco.Tests.Services Assert.That(mediaChild.Trashed, Is.False); } + [Test] + public void Ensure_Content_Xml_Created() + { + var mediaService = ServiceContext.MediaService; + var mediaType = MockedContentTypes.CreateVideoMediaType(); + ServiceContext.ContentTypeService.Save(mediaType); + var media = mediaService.CreateMedia("Test", -1, "video"); + + mediaService.Save(media); + + var provider = new PetaPocoUnitOfWorkProvider(); + var uow = provider.GetUnitOfWork(); + using (RepositoryResolver.Current.ResolveByType(uow)) + { + Assert.IsTrue(uow.Database.Exists(media.Id)); + } + + } + private Tuple CreateTrashedTestMedia() { //Create and Save folder-Media -> 1050