From c6feb2a27a6feeea11d085887e5478ee734b2d90 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Tue, 8 Jan 2013 09:49:13 -0100 Subject: [PATCH 1/3] Correcting an issue in the ContentTypeBaseRepository where removing a PropertyType would cause an error if a Property was referencing that PropertyType. Correcting an issue in the ContentRepository that would occur if a new PropertyType had been added after Content is being used. Fixes U4-1402 --- .../Repositories/ContentRepository.cs | 2 + .../Repositories/ContentTypeBaseRepository.cs | 14 +- .../Repositories/ContentTypeRepositoryTest.cs | 122 ++++++++++++++++++ .../businesslogic/web/DocumentType.cs | 3 +- 4 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 8bc0b6d79f..87faaf70df 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -365,6 +365,8 @@ namespace Umbraco.Core.Persistence.Repositories { foreach (var property in entity.Properties) { + if(keyDictionary.ContainsKey(property.PropertyTypeId) == false) continue; + property.Id = keyDictionary[property.PropertyTypeId]; } } diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs index 1292db1ae1..68f9a4ad3b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeBaseRepository.cs @@ -168,11 +168,19 @@ namespace Umbraco.Core.Persistence.Repositories if (((ICanBeDirty)entity).IsPropertyDirty("PropertyGroups") || entity.PropertyGroups.Any(x => x.IsDirty())) { //Delete PropertyTypes by excepting entries from db with entries from collections - var dbPropertyTypes = Database.Fetch("WHERE contentTypeId = @Id", new { Id = entity.Id }).Select(x => x.Alias); - var entityPropertyTypes = entity.PropertyTypes.Select(x => x.Alias); - var aliases = dbPropertyTypes.Except(entityPropertyTypes); + var dbPropertyTypes = Database.Fetch("WHERE contentTypeId = @Id", new { Id = entity.Id }); + var dbPropertyTypeAlias = dbPropertyTypes.Select(x => x.Alias.ToLowerInvariant()); + var entityPropertyTypes = entity.PropertyTypes.Select(x => x.Alias.ToLowerInvariant()); + var aliases = dbPropertyTypeAlias.Except(entityPropertyTypes); foreach (var alias in aliases) { + //Before a PropertyType can be deleted, all Properties based on that PropertyType should be deleted. + var propertyType = dbPropertyTypes.FirstOrDefault(x => x.Alias.ToLowerInvariant() == alias); + if (propertyType != null) + { + Database.Delete("WHERE propertytypeid = @Id", new { Id = propertyType.Id }); + } + Database.Delete("WHERE contentTypeId = @Id AND Alias = @Alias", new { Id = entity.Id, Alias = alias }); } //Delete Tabs/Groups by excepting entries from db with entries from collections diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs index 8e1ca74866..28487509b4 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentTypeRepositoryTest.cs @@ -270,6 +270,128 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(updated.AllowedContentTypes.Any(x => x.Alias == simpleSubpageContentType.Alias), Is.True); } + [Test] + public void Can_Verify_Removal_Of_Used_PropertyType_From_ContentType() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentRepository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentType = repository.Get(1046); + + var subpage = MockedContent.CreateTextpageContent(contentType, "Text Page 1", contentType.Id); + contentRepository.AddOrUpdate(subpage); + unitOfWork.Commit(); + + // Act + contentType.RemovePropertyType("keywords"); + repository.AddOrUpdate(contentType); + unitOfWork.Commit(); + + // Assert + Assert.That(contentType.PropertyTypes.Count(), Is.EqualTo(3)); + Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "keywords"), Is.False); + Assert.That(subpage.Properties.First(x => x.Alias == "metaDescription").Value, Is.EqualTo("This is the meta description for a textpage")); + } + + [Test] + public void Can_Verify_Addition_Of_PropertyType_After_ContentType_Is_Used() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentRepository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentType = repository.Get(1046); + + var subpage = MockedContent.CreateTextpageContent(contentType, "Text Page 1", contentType.Id); + contentRepository.AddOrUpdate(subpage); + unitOfWork.Commit(); + + // Act + var propertyGroup = contentType.PropertyGroups.First(x => x.Name == "Meta"); + propertyGroup.PropertyTypes.Add(new PropertyType(new Guid(), DataTypeDatabaseType.Ntext) { Alias = "metaAuthor", Name = "Meta Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }); + repository.AddOrUpdate(contentType); + unitOfWork.Commit(); + + // Assert + Assert.That(contentType.PropertyTypes.Count(), Is.EqualTo(5)); + Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "metaAuthor"), Is.True); + } + + [Test] + public void Can_Verify_Usage_Of_New_PropertyType_On_Content() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentRepository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentType = repository.Get(1046); + + var subpage = MockedContent.CreateTextpageContent(contentType, "Text Page 1", contentType.Id); + contentRepository.AddOrUpdate(subpage); + unitOfWork.Commit(); + + var propertyGroup = contentType.PropertyGroups.First(x => x.Name == "Meta"); + propertyGroup.PropertyTypes.Add(new PropertyType(new Guid(), DataTypeDatabaseType.Ntext) { Alias = "metaAuthor", Name = "Meta Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }); + repository.AddOrUpdate(contentType); + unitOfWork.Commit(); + + // Act + var content = contentRepository.Get(subpage.Id); + content.SetValue("metaAuthor", "John Doe"); + contentRepository.AddOrUpdate(content); + unitOfWork.Commit(); + + //Assert + var updated = contentRepository.Get(subpage.Id); + Assert.That(updated.GetValue("metaAuthor").ToString(), Is.EqualTo("John Doe")); + Assert.That(contentType.PropertyTypes.Count(), Is.EqualTo(5)); + Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "metaAuthor"), Is.True); + } + + [Test] + public void + Can_Verify_That_A_Combination_Of_Adding_And_Deleting_PropertyTypes_Doesnt_Cause_Issues_For_Content_And_ContentType + () + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(); + var unitOfWork = provider.GetUnitOfWork(); + var repository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentRepository = RepositoryResolver.Current.ResolveByType(unitOfWork); + var contentType = repository.Get(1046); + + var subpage = MockedContent.CreateTextpageContent(contentType, "Text Page 1", contentType.Id); + contentRepository.AddOrUpdate(subpage); + unitOfWork.Commit(); + + //Remove PropertyType + contentType.RemovePropertyType("keywords"); + //Add PropertyType + var propertyGroup = contentType.PropertyGroups.First(x => x.Name == "Meta"); + propertyGroup.PropertyTypes.Add(new PropertyType(new Guid(), DataTypeDatabaseType.Ntext) { Alias = "metaAuthor", Name = "Meta Author", Description = "", HelpText = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88 }); + repository.AddOrUpdate(contentType); + unitOfWork.Commit(); + + // Act + var content = contentRepository.Get(subpage.Id); + content.SetValue("metaAuthor", "John Doe"); + contentRepository.AddOrUpdate(content); + unitOfWork.Commit(); + + //Assert + var updated = contentRepository.Get(subpage.Id); + Assert.That(updated.GetValue("metaAuthor").ToString(), Is.EqualTo("John Doe")); + Assert.That(updated.Properties.First(x => x.Alias == "metaDescription").Value, Is.EqualTo("This is the meta description for a textpage")); + + Assert.That(contentType.PropertyTypes.Count(), Is.EqualTo(4)); + Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "metaAuthor"), Is.True); + Assert.That(contentType.PropertyTypes.Any(x => x.Alias == "keywords"), Is.False); + } + public void CreateTestData() { //Create and Save ContentType "umbTextpage" -> 1045 diff --git a/src/umbraco.cms/businesslogic/web/DocumentType.cs b/src/umbraco.cms/businesslogic/web/DocumentType.cs index 02bd51b4ba..bb91b66323 100644 --- a/src/umbraco.cms/businesslogic/web/DocumentType.cs +++ b/src/umbraco.cms/businesslogic/web/DocumentType.cs @@ -457,7 +457,8 @@ namespace umbraco.cms.businesslogic.web ApplicationContext.Current.Services.ContentTypeService.Save(_contentType); - //Ensure that DocumentTypes are reloaded from db by clearing cache + //Ensure that DocumentTypes are reloaded from db by clearing cache. + //NOTE Would be nice if we could clear cache by type instead of emptying the entire cache. InMemoryCacheProvider.Current.Clear(); base.Save(); From dd61432d9f2e81682f2935f0750c34237d364669 Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Tue, 8 Jan 2013 10:58:06 -0100 Subject: [PATCH 2/3] More work on U4-1402 in order to clear cache in all the right places. --- .../Persistence/Caching/RuntimeCacheProvider.cs | 14 +++++++++++++- .../businesslogic/propertytype/propertytype.cs | 9 +++++---- src/umbraco.cms/businesslogic/web/DocumentType.cs | 1 + 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Caching/RuntimeCacheProvider.cs b/src/Umbraco.Core/Persistence/Caching/RuntimeCacheProvider.cs index fa5d5e421e..bb2b80a766 100644 --- a/src/Umbraco.Core/Persistence/Caching/RuntimeCacheProvider.cs +++ b/src/Umbraco.Core/Persistence/Caching/RuntimeCacheProvider.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Runtime.Caching; +using System.Threading; using Umbraco.Core.Models.EntityBase; namespace Umbraco.Core.Persistence.Caching @@ -23,9 +24,10 @@ namespace Umbraco.Core.Persistence.Caching #endregion - private readonly ObjectCache _memoryCache = new MemoryCache("in-memory"); //TODO Save this in cache as well, so its not limited to a single server usage private ConcurrentDictionary _keyTracker = new ConcurrentDictionary(); + private ObjectCache _memoryCache = new MemoryCache("in-memory"); + private static readonly ReaderWriterLockSlim ClearLock = new ReaderWriterLockSlim(); public IEntity GetById(Type type, Guid id) { @@ -77,6 +79,16 @@ namespace Umbraco.Core.Persistence.Caching _keyTracker.TryRemove(key, out throwaway); } + public void Clear() + { + using (new ReadLock(ClearLock)) + { + _keyTracker.Clear(); + _memoryCache.DisposeIfDisposable(); + _memoryCache = new MemoryCache("in-memory"); + } + } + private string GetCompositeId(Type type, Guid id) { return string.Format("{0}-{1}", type.Name, id.ToString()); diff --git a/src/umbraco.cms/businesslogic/propertytype/propertytype.cs b/src/umbraco.cms/businesslogic/propertytype/propertytype.cs index 2de760e13a..d72a1d3eb3 100644 --- a/src/umbraco.cms/businesslogic/propertytype/propertytype.cs +++ b/src/umbraco.cms/businesslogic/propertytype/propertytype.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Runtime.CompilerServices; using System.Threading; +using Umbraco.Core.Persistence.Caching; using umbraco.BusinessLogic; using umbraco.cms.businesslogic.cache; using umbraco.cms.businesslogic.datatype; @@ -461,10 +462,10 @@ namespace umbraco.cms.businesslogic.propertytype // clear cache in contentype Cache.ClearCacheItem("ContentType_PropertyTypes_Content:" + _contenttypeid); - // clear cache in tab - // zb-00040 #29889 : clear the right cache! t.ContentType is the ctype which _defines_ the tab, not the current one. - // foreach (ContentType.TabI t in new ContentType(ContentTypeId).getVirtualTabs) - // ContentType.FlushTabCache(t.Id, ContentTypeId); + //Ensure that DocumentTypes are reloaded from db by clearing cache - this similar to the Save method on DocumentType. + //NOTE Would be nice if we could clear cache by type instead of emptying the entire cache. + InMemoryCacheProvider.Current.Clear(); + RuntimeCacheProvider.Current.Clear(); } public static PropertyType GetPropertyType(int id) diff --git a/src/umbraco.cms/businesslogic/web/DocumentType.cs b/src/umbraco.cms/businesslogic/web/DocumentType.cs index bb91b66323..6ebca1c12a 100644 --- a/src/umbraco.cms/businesslogic/web/DocumentType.cs +++ b/src/umbraco.cms/businesslogic/web/DocumentType.cs @@ -460,6 +460,7 @@ namespace umbraco.cms.businesslogic.web //Ensure that DocumentTypes are reloaded from db by clearing cache. //NOTE Would be nice if we could clear cache by type instead of emptying the entire cache. InMemoryCacheProvider.Current.Clear(); + RuntimeCacheProvider.Current.Clear();//Runtime cache is used for Content, so we clear that as well base.Save(); FireAfterSave(e); From afb251d5b7eaa28a52554e2313aafca027eeaa1c Mon Sep 17 00:00:00 2001 From: Morten Christensen Date: Tue, 8 Jan 2013 11:38:35 -0100 Subject: [PATCH 3/3] Fixes U4-1394 so the DefaultTemplate is not completely removed from the list of allowed templates when changed. --- src/Umbraco.Core/Models/ContentType.cs | 6 ++++++ .../umbraco/settings/EditNodeTypeNew.aspx.cs | 2 ++ src/umbraco.cms/businesslogic/web/DocumentType.cs | 7 ++----- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index 9d747e4c55..9211e1ac98 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -72,6 +72,12 @@ namespace Umbraco.Core.Models /// Default public void SetDefaultTemplate(ITemplate template) { + if (template == null) + { + DefaultTemplateId = 0; + return; + } + DefaultTemplateId = template.Id; if(_allowedTemplates.Any(x => x != null && x.Id == template.Id) == false) { diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/settings/EditNodeTypeNew.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/settings/EditNodeTypeNew.aspx.cs index 0c3be42e00..170d2324c5 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/settings/EditNodeTypeNew.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/settings/EditNodeTypeNew.aspx.cs @@ -128,6 +128,8 @@ namespace umbraco.settings else dt.RemoveDefaultTemplate(); + dt.Save(); + bindTemplates(); } else diff --git a/src/umbraco.cms/businesslogic/web/DocumentType.cs b/src/umbraco.cms/businesslogic/web/DocumentType.cs index 6ebca1c12a..715a045c37 100644 --- a/src/umbraco.cms/businesslogic/web/DocumentType.cs +++ b/src/umbraco.cms/businesslogic/web/DocumentType.cs @@ -420,14 +420,11 @@ namespace umbraco.cms.businesslogic.web return doc; } - [Obsolete("Deprecated, Use RemoveTemplate() on Umbraco.Core.Models.ContentType", false)] + [Obsolete("Deprecated, Use SetDefaultTemplate(null) on Umbraco.Core.Models.ContentType", false)] public void RemoveDefaultTemplate() { _defaultTemplate = 0; - - var template = _contentType.DefaultTemplate; - if(template != null) - _contentType.RemoveTemplate(template); + _contentType.SetDefaultTemplate(null); } public bool HasTemplate()