diff --git a/src/Umbraco.Core/Models/PropertyTypeCollection.cs b/src/Umbraco.Core/Models/PropertyTypeCollection.cs index 38abd0c57d..a06e6d737d 100644 --- a/src/Umbraco.Core/Models/PropertyTypeCollection.cs +++ b/src/Umbraco.Core/Models/PropertyTypeCollection.cs @@ -69,6 +69,7 @@ namespace Umbraco.Core.Models OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } + //TODO: Instead of 'new' this should explicitly implement one of the collection interfaces members internal new void Add(PropertyType item) { using (new WriteLock(_addLocker)) diff --git a/src/Umbraco.Core/Persistence/Factories/MemberFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberFactory.cs index 2901f48539..7b28808429 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberFactory.cs @@ -28,17 +28,17 @@ namespace Umbraco.Core.Persistence.Factories #region Implementation of IEntityFactory - public IMember BuildEntity(MemberDto dto) + public static IMember BuildEntity(MemberDto dto, IMemberType contentType) { var member = new Member( dto.ContentVersionDto.ContentDto.NodeDto.Text, - dto.Email, dto.LoginName, dto.Password, _contentType); + dto.Email, dto.LoginName, dto.Password, contentType); try { member.DisableChangeTracking(); - member.Id = _id; + member.Id = dto.NodeId; member.Key = dto.ContentVersionDto.ContentDto.NodeDto.UniqueId; member.Path = dto.ContentVersionDto.ContentDto.NodeDto.Path; member.CreatorId = dto.ContentVersionDto.ContentDto.NodeDto.UserId.Value; @@ -62,6 +62,12 @@ namespace Umbraco.Core.Persistence.Factories } } + [Obsolete("Use the static BuildEntity instead so we don't have to allocate one of these objects everytime we want to map values")] + public IMember BuildEntity(MemberDto dto) + { + return BuildEntity(dto, _contentType); + } + public MemberDto BuildDto(IMember entity) { var dto = new MemberDto diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index ff466ea84e..0e1228960e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -64,7 +64,7 @@ namespace Umbraco.Core.Persistence.Repositories if (dto == null) return null; - var content = CreateContentFromDto(dto, dto.ContentVersionDto.VersionId, sql); + var content = CreateContentFromDto(dto, sql); return content; } @@ -287,7 +287,7 @@ namespace Umbraco.Core.Persistence.Repositories if (dto == null) return null; - var content = CreateContentFromDto(dto, versionId, sql); + var content = CreateContentFromDto(dto, sql); return content; } @@ -933,13 +933,14 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", parsedOriginalSql = parsedOriginalSql.Substring(0, parsedOriginalSql.LastIndexOf("ORDER BY ", StringComparison.Ordinal)); } + //order by update date DESC, if there is corrupted published flags we only want the latest! var publishedSql = new Sql(@"SELECT * FROM cmsDocument AS doc2 INNER JOIN (" + parsedOriginalSql + @") as docData ON doc2.nodeId = docData.nodeId WHERE doc2.published = 1 -ORDER BY doc2.nodeId +ORDER BY doc2.updateDate DESC ", sqlFull.Arguments); //go and get the published version data, we do a Query here and not a Fetch so we are @@ -955,8 +956,8 @@ ORDER BY doc2.nodeId } - var content = new IContent[dtos.Count]; - var defs = new List(); + var content = new List(); + var defs = new DocumentDefinitionCollection(); var templateIds = new List(); //track the looked up content types, even though the content types are cached @@ -964,9 +965,8 @@ ORDER BY doc2.nodeId // the overhead of deep cloning them on every item in this loop var contentTypes = new Dictionary(); - for (var i = 0; i < dtos.Count; i++) + foreach (var dto in dtos) { - var dto = dtos[i]; DocumentPublishedReadOnlyDto publishedDto; publishedDataCollection.TryGetValue(dto.NodeId, out publishedDto); @@ -975,9 +975,10 @@ ORDER BY doc2.nodeId { var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); //only use this cached version if the dto returned is also the publish version, they must match + //TODO: Shouldn't this also match on version!? if (cached != null && cached.Published && dto.Published) { - content[i] = cached; + content.Add(cached); continue; } } @@ -996,20 +997,15 @@ ORDER BY doc2.nodeId contentTypes[dto.ContentVersionDto.ContentDto.ContentTypeId] = contentType; } - content[i] = ContentFactory.BuildEntity(dto, contentType, publishedDto); + // track the definition and if it's successfully added or updated then processed + if (defs.AddOrUpdate(new DocumentDefinition(dto, contentType))) + { + // need template + if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) + templateIds.Add(dto.TemplateId.Value); - // need template - if (dto.TemplateId.HasValue && dto.TemplateId.Value > 0) - templateIds.Add(dto.TemplateId.Value); - - // need properties - defs.Add(new DocumentDefinition( - dto.NodeId, - dto.VersionId, - dto.ContentVersionDto.VersionDate, - dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, - contentType - )); + content.Add(ContentFactory.BuildEntity(dto, contentType, publishedDto)); + } } // load all required templates in 1 query @@ -1019,26 +1015,21 @@ ORDER BY doc2.nodeId // load all properties for all documents from database in 1 query var propertyData = GetPropertyCollection(pagingSqlQuery, defs); - // assign - var dtoIndex = 0; - foreach (var def in defs) + // assign template and property data + foreach (var cc in content) { - // move to corresponding item (which has to exist) - while (dtos[dtoIndex].NodeId != def.Id) dtoIndex++; + var def = defs[cc.Id]; - // complete the item - var cc = content[dtoIndex]; - var dto = dtos[dtoIndex]; ITemplate template = null; - if (dto.TemplateId.HasValue) - templates.TryGetValue(dto.TemplateId.Value, out template); // else null + if (def.DocumentDto.TemplateId.HasValue) + templates.TryGetValue(def.DocumentDto.TemplateId.Value, out template); // else null cc.Template = template; cc.Properties = propertyData[cc.Id]; //on initial construction we don't want to have dirty properties tracked // http://issues.umbraco.org/issue/U4-1946 cc.ResetDirtyProperties(false); - } + } return content; } @@ -1047,10 +1038,9 @@ ORDER BY doc2.nodeId /// Private method to create a content object from a DocumentDto, which is used by Get and GetByVersion. /// /// - /// /// /// - private IContent CreateContentFromDto(DocumentDto dto, Guid versionId, Sql docSql) + private IContent CreateContentFromDto(DocumentDto dto, Sql docSql) { var contentType = _contentTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); @@ -1062,7 +1052,7 @@ ORDER BY doc2.nodeId content.Template = _templateRepository.Get(dto.TemplateId.Value); } - var docDef = new DocumentDefinition(dto.NodeId, versionId, content.UpdateDate, content.CreateDate, contentType); + var docDef = new DocumentDefinition(dto, contentType); var properties = GetPropertyCollection(docSql, new[] { docDef }); diff --git a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs index 53b04e1322..3f0d1d2486 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs @@ -55,7 +55,7 @@ namespace Umbraco.Core.Persistence.Repositories if (dto == null) return null; - var content = CreateMediaFromDto(dto, dto.VersionId, sql); + var content = CreateMediaFromDto(dto, sql); return content; } @@ -161,25 +161,24 @@ namespace Umbraco.Core.Persistence.Repositories { // fetch returns a list so it's ok to iterate it in this method var dtos = Database.Fetch(sqlFull); - var content = new IMedia[dtos.Count]; - var defs = new List(); + var content = new List(); + var defs = new DocumentDefinitionCollection(); //track the looked up content types, even though the content types are cached // they still need to be deep cloned out of the cache and we don't want to add // the overhead of deep cloning them on every item in this loop var contentTypes = new Dictionary(); - for (var i = 0; i < dtos.Count; i++) + foreach (var dto in dtos) { - var dto = dtos[i]; - // if the cache contains the item, use it if (withCache) { var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); + //TODO: Shouldn't this also match on version!? if (cached != null) { - content[i] = cached; + content.Add(cached); continue; } } @@ -197,37 +196,26 @@ namespace Umbraco.Core.Persistence.Repositories contentType = _mediaTypeRepository.Get(dto.ContentDto.ContentTypeId); contentTypes[dto.ContentDto.ContentTypeId] = contentType; } - - content[i] = MediaFactory.BuildEntity(dto, contentType); - // need properties - defs.Add(new DocumentDefinition( - dto.NodeId, - dto.VersionId, - dto.VersionDate, - dto.ContentDto.NodeDto.CreateDate, - contentType - )); + // track the definition and if it's successfully added or updated then processed + if (defs.AddOrUpdate(new DocumentDefinition(dto, contentType))) + { + content.Add(MediaFactory.BuildEntity(dto, contentType)); + } } // load all properties for all documents from database in 1 query var propertyData = GetPropertyCollection(pagingSqlQuery, defs); - // assign - var dtoIndex = 0; - foreach (var def in defs) + // assign property data + foreach (var cc in content) { - // move to corresponding item (which has to exist) - while (dtos[dtoIndex].NodeId != def.Id) dtoIndex++; - - // complete the item - var cc = content[dtoIndex]; cc.Properties = propertyData[cc.Id]; //on initial construction we don't want to have dirty properties tracked // http://issues.umbraco.org/issue/U4-1946 cc.ResetDirtyProperties(false); - } + } return content; } @@ -243,7 +231,7 @@ namespace Umbraco.Core.Persistence.Repositories if (dto == null) return null; - var content = CreateMediaFromDto(dto, versionId, sql); + var content = CreateMediaFromDto(dto, sql); return content; } @@ -528,16 +516,15 @@ namespace Umbraco.Core.Persistence.Repositories /// Private method to create a media object from a ContentDto /// /// - /// /// /// - private IMedia CreateMediaFromDto(ContentVersionDto dto, Guid versionId, Sql docSql) + private IMedia CreateMediaFromDto(ContentVersionDto dto, Sql docSql) { var contentType = _mediaTypeRepository.Get(dto.ContentDto.ContentTypeId); var media = MediaFactory.BuildEntity(dto, contentType); - var docDef = new DocumentDefinition(dto.NodeId, versionId, media.UpdateDate, media.CreateDate, contentType); + var docDef = new DocumentDefinition(dto, contentType); var properties = GetPropertyCollection(new PagingSqlQuery(docSql), new[] { docDef }); diff --git a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs index ae8cb08255..23b42108bc 100644 --- a/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs @@ -54,7 +54,7 @@ namespace Umbraco.Core.Persistence.Repositories if (dto == null) return null; - var content = CreateMemberFromDto(dto, dto.ContentVersionDto.VersionId, sql); + var content = CreateMemberFromDto(dto, sql); return content; @@ -448,16 +448,16 @@ namespace Umbraco.Core.Persistence.Repositories var memberType = _memberTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); var factory = new MemberFactory(memberType, NodeObjectTypeId, dto.NodeId); - var media = factory.BuildEntity(dto); + var member = factory.BuildEntity(dto); - var properties = GetPropertyCollection(new PagingSqlQuery(sql), new[] { new DocumentDefinition(dto.NodeId, dto.ContentVersionDto.VersionId, media.UpdateDate, media.CreateDate, memberType) }); + var properties = GetPropertyCollection(new PagingSqlQuery(sql), new[] { new DocumentDefinition(dto.ContentVersionDto, memberType) }); - media.Properties = properties[dto.NodeId]; + member.Properties = properties[dto.NodeId]; //on initial construction we don't want to have dirty properties tracked // http://issues.umbraco.org/issue/U4-1946 - ((Entity)media).ResetDirtyProperties(false); - return media; + ((Entity)member).ResetDirtyProperties(false); + return member; } @@ -681,20 +681,19 @@ namespace Umbraco.Core.Persistence.Repositories // fetch returns a list so it's ok to iterate it in this method var dtos = Database.Fetch(sqlFull); - var content = new IMember[dtos.Count]; - var defs = new List(); + var content = new List(); + var defs = new DocumentDefinitionCollection(); - for (var i = 0; i < dtos.Count; i++) + foreach (var dto in dtos) { - var dto = dtos[i]; - // if the cache contains the item, use it if (withCache) { var cached = RuntimeCache.GetCacheItem(GetCacheIdKey(dto.NodeId)); + //TODO: Shouldn't this also match on version!? if (cached != null) { - content[i] = cached; + content.Add(cached); continue; } } @@ -702,36 +701,25 @@ namespace Umbraco.Core.Persistence.Repositories // else, need to fetch from the database // content type repository is full-cache so OK to get each one independently var contentType = _memberTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); - var factory = new MemberFactory(contentType, NodeObjectTypeId, dto.NodeId); - content[i] = factory.BuildEntity(dto); // need properties - defs.Add(new DocumentDefinition( - dto.NodeId, - dto.ContentVersionDto.VersionId, - dto.ContentVersionDto.VersionDate, - dto.ContentVersionDto.ContentDto.NodeDto.CreateDate, - contentType - )); + if (defs.AddOrUpdate(new DocumentDefinition(dto.ContentVersionDto, contentType))) + { + content.Add(MemberFactory.BuildEntity(dto, contentType)); + } } // load all properties for all documents from database in 1 query var propertyData = GetPropertyCollection(pagingSqlQuery, defs); - // assign - var dtoIndex = 0; - foreach (var def in defs) + // assign property data + foreach (var cc in content) { - // move to corresponding item (which has to exist) - while (dtos[dtoIndex].NodeId != def.Id) dtoIndex++; - - // complete the item - var cc = content[dtoIndex]; cc.Properties = propertyData[cc.Id]; //on initial construction we don't want to have dirty properties tracked // http://issues.umbraco.org/issue/U4-1946 - ((Entity)cc).ResetDirtyProperties(false); + cc.ResetDirtyProperties(false); } return content; @@ -741,17 +729,16 @@ namespace Umbraco.Core.Persistence.Repositories /// Private method to create a member object from a MemberDto /// /// - /// /// /// - private IMember CreateMemberFromDto(MemberDto dto, Guid versionId, Sql docSql) + private IMember CreateMemberFromDto(MemberDto dto, Sql docSql) { var memberType = _memberTypeRepository.Get(dto.ContentVersionDto.ContentDto.ContentTypeId); var factory = new MemberFactory(memberType, NodeObjectTypeId, dto.ContentVersionDto.NodeId); var member = factory.BuildEntity(dto); - var docDef = new DocumentDefinition(dto.ContentVersionDto.NodeId, versionId, member.UpdateDate, member.CreateDate, memberType); + var docDef = new DocumentDefinition(dto.ContentVersionDto, memberType); var properties = GetPropertyCollection(docSql, new[] { docDef }); diff --git a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs index df96112a30..c2c8d3e096 100644 --- a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs @@ -773,24 +773,102 @@ ORDER BY contentNodeId, propertytypeid /// protected abstract Sql GetBaseQuery(BaseQueryType queryType); + internal class DocumentDefinitionCollection : KeyedCollection + { + protected override int GetKeyForItem(DocumentDefinition item) + { + return item.Id; + } + + /// + /// if this key already exists if it does then we need to check + /// if the existing item is 'older' than the new item and if that is the case we'll replace the older one + /// + /// + /// + public bool AddOrUpdate(DocumentDefinition item) + { + if (Dictionary == null) + { + base.Add(item); + return true; + } + + var key = GetKeyForItem(item); + DocumentDefinition found; + if (TryGetValue(key, out found)) + { + //it already exists and it's older so we need to replace it + if (item.VersionDate > found.VersionDate) + { + var currIndex = Items.IndexOf(found); + if (currIndex == -1) + throw new IndexOutOfRangeException("Could not find the item in the list: " + found.Version); + + //replace the current one with the newer one + SetItem(currIndex, item); + return true; + } + //could not add or update + return false; + } + + base.Add(item); + return true; + } + + public bool TryGetValue(int key, out DocumentDefinition val) + { + if (Dictionary == null) + { + val = null; + return false; + } + return Dictionary.TryGetValue(key, out val); + } + } + internal class DocumentDefinition { /// /// Initializes a new instance of the class. /// - public DocumentDefinition(int id, Guid version, DateTime versionDate, DateTime createDate, IContentTypeComposition composition) + public DocumentDefinition(DocumentDto dto, IContentTypeComposition composition) { - Id = id; - Version = version; - VersionDate = versionDate; - CreateDate = createDate; + DocumentDto = dto; + ContentVersionDto = dto.ContentVersionDto; Composition = composition; } - public int Id { get; set; } - public Guid Version { get; set; } - public DateTime VersionDate { get; set; } - public DateTime CreateDate { get; set; } + public DocumentDefinition(ContentVersionDto dto, IContentTypeComposition composition) + { + ContentVersionDto = dto; + Composition = composition; + } + + public DocumentDto DocumentDto { get; private set; } + public ContentVersionDto ContentVersionDto { get; private set; } + + public int Id + { + get { return ContentVersionDto.NodeId; } + } + + public Guid Version + { + get { return DocumentDto != null ? DocumentDto.VersionId : ContentVersionDto.VersionId; } + } + + public DateTime VersionDate + { + get { return ContentVersionDto.VersionDate; } + } + + public DateTime CreateDate + { + get { return ContentVersionDto.ContentDto.NodeDto.CreateDate; } + } + public IContentTypeComposition Composition { get; set; } } diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index a561af08a2..c6f9ed21ba 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -67,6 +67,58 @@ namespace Umbraco.Tests.Persistence.Repositories return repository; } + //TODO: We need to write this same test and fix the EntityRepository! + [Test] + public void Deal_With_Corrupt_Duplicate_Newest_Published_Flags() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + IContent content1; + + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + var hasPropertiesContentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); + content1 = MockedContent.CreateSimpleContent(hasPropertiesContentType); + + contentTypeRepository.AddOrUpdate(hasPropertiesContentType); + repository.AddOrUpdate(content1); + unitOfWork.Commit(); + } + + //Now manually corrupt the data + for (var index = 0; index < new[] {Guid.NewGuid(), Guid.NewGuid()}.Length; index++) + { + var version = new[] {Guid.NewGuid(), Guid.NewGuid()}[index]; + var versionDate = DateTime.Now.AddMinutes(index); + this.DatabaseContext.Database.Insert(new ContentVersionDto + { + NodeId = content1.Id, + VersionDate = versionDate, + VersionId = version + }); + this.DatabaseContext.Database.Insert(new DocumentDto + { + Newest = true, + NodeId = content1.Id, + Published = true, + Text = content1.Name, + VersionId = version, + WriterUserId = 0, + UpdateDate = versionDate, + TemplateId = content1.Template == null || content1.Template.Id <= 0 ? null : (int?) content1.Template.Id + }); + } + + // Assert + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + var content = repository.GetByQuery(new Query().Where(c => c.Id == content1.Id)); + Assert.AreEqual(1, content.Count()); + } + } + /// /// This tests the regression issue of U4-9438 ///