yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Adds test to show that the content repo can deal with corrupted data, this fix actually simplifies a bunch of logic too and should consume slightly less memory, have added some TODOs in the code and need to then write more tests and code to deal with this in the EntityRepository.

Browse Source
This commit is contained in:
Shannon
2017-02-16 18:54:44 +11:00
parent 759e444624
commit af287c387e
7 changed files with 211 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/Umbraco.Core/Models/PropertyTypeCollection.cs
View File

@@ -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))

12
src/Umbraco.Core/Persistence/Factories/MemberFactory.cs
View File

@@ -28,17 +28,17 @@ namespace Umbraco.Core.Persistence.Factories
#region Implementation of IEntityFactory<IMedia,ContentVersionDto>
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

60
src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs
View File

@@ -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<DocumentDefinition>();
var content = new List<IContent>();
var defs = new DocumentDefinitionCollection();
var templateIds = new List<int>();
//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<int, IContentType>();
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<IContent>(GetCacheIdKey<IContent>(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.
/// </summary>
/// <param name="dto"></param>
/// <param name="versionId"></param>
/// <param name="docSql"></param>
/// <returns></returns>
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 });

47
src/Umbraco.Core/Persistence/Repositories/MediaRepository.cs
View File

@@ -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<ContentVersionDto, ContentDto, NodeDto>(sqlFull);
var content = new IMedia[dtos.Count];
var defs = new List<DocumentDefinition>();
var content = new List<IMedia>();
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<int, IMediaType>();
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<IMedia>(GetCacheIdKey<IMedia>(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
/// </summary>
/// <param name="dto"></param>
/// <param name="versionId"></param>
/// <param name="docSql"></param>
/// <returns></returns>
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 });

53
src/Umbraco.Core/Persistence/Repositories/MemberRepository.cs
View File

@@ -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<MemberDto, ContentVersionDto, ContentDto, NodeDto>(sqlFull);
var content = new IMember[dtos.Count];
var defs = new List<DocumentDefinition>();
var content = new List<IMember>();
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<IMember>(GetCacheIdKey<IMember>(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
/// </summary>
/// <param name="dto"></param>
/// <param name="versionId"></param>
/// <param name="docSql"></param>
/// <returns></returns>
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 });

96
src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs
View File

@@ -773,24 +773,102 @@ ORDER BY contentNodeId, propertytypeid
/// <returns></returns>
protected abstract Sql GetBaseQuery(BaseQueryType queryType);
internal class DocumentDefinitionCollection : KeyedCollection<int, DocumentDefinition>
{
protected override int GetKeyForItem(DocumentDefinition item)
{
return item.Id;
}
/// <summary>
/// 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
/// </summary>
/// <param name="item"></param>
/// <returns></returns>
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
{
/// <summary>
/// Initializes a new instance of the <see cref="T:System.Object"/> class.
/// </summary>
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; }
}

52
src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs
View File

@@ -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<IContent>().Where(c => c.Id == content1.Id));
Assert.AreEqual(1, content.Count());
}
}
/// <summary>
/// This tests the regression issue of U4-9438
/// </summary>