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

Adds logic to validate an entity's path when it is used to GetDescendants, adds logic to fix an entity's path when it's state is going to be persisted when GetDescendants is used (i.e. MoveToRecycleBin), added this logic to Media too, adds unit tests to support

Browse Source
This commit is contained in:
Shannon
2017-01-05 10:29:03 +11:00
parent f200ab423d
commit 8be0f68390
5 changed files with 318 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

72
src/Umbraco.Core/Models/UmbracoEntityExtensions.cs
View File

@@ -4,12 +4,84 @@ using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Web.UI.WebControls;
using Umbraco.Core.Logging;
using Umbraco.Core.Models.EntityBase;
namespace Umbraco.Core.Models
{
internal static class UmbracoEntityExtensions
{
/// <summary>
/// Does a quick check on the entity's set path to ensure that it's valid and consistent
/// </summary>
/// <param name="entity"></param>
/// <returns></returns>
public static bool ValidatePath(this IUmbracoEntity entity)
{
//don't validate if it's empty and it has no id
if (entity.HasIdentity == false && entity.Path.IsNullOrWhiteSpace())
return true;
if (entity.Path.IsNullOrWhiteSpace())
return false;
var pathParts = entity.Path.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
if (pathParts.Length < 2)
{
//a path cannot be less than 2 parts, at a minimum it must be root (-1) and it's own id
return false;
}
if (entity.ParentId != default(int) && pathParts[pathParts.Length - 2] != entity.ParentId.ToInvariantString())
{
//the 2nd last id in the path must be it's parent id
return false;
}
return true;
}
/// <summary>
/// This will validate the entity's path and if it's invalid it will fix it, if fixing is required it will recursively
/// check and fix all ancestors if required.
/// </summary>
/// <param name="entity"></param>
/// <param name="logger"></param>
/// <param name="getParent">A callback specified to retrieve the parent entity of the entity</param>
/// <param name="update">A callback specified to update a fixed entity</param>
public static void EnsureValidPath<T>(this T entity,
ILogger logger,
Func<T, T> getParent,
Action<T> update)
where T: IUmbracoEntity
{
if (entity.HasIdentity == false)
throw new InvalidOperationException("Could not ensure the entity path, the entity has not been assigned an identity");
if (entity.ValidatePath() == false)
{
logger.Warn(typeof(UmbracoEntityExtensions), string.Format("The content item {0} has an invalid path: {1} with parentID: {2}", entity.Id, entity.Path, entity.ParentId));
if (entity.ParentId == -1)
{
entity.Path = string.Concat("-1,", entity.Id);
//path changed, update it
update(entity);
}
else
{
var parent = getParent(entity);
if (parent == null)
throw new NullReferenceException("Could not ensure path for entity " + entity.Id + " could not resolve it's parent " + entity.ParentId);
//the parent must also be valid!
parent.EnsureValidPath(logger, getParent, update);
entity.Path = string.Concat(parent.Path, ",", entity.Id);
//path changed, update it
update(entity);
}
}
}
public static bool HasChildren(this IUmbracoEntity entity)
{

40
src/Umbraco.Core/Services/ContentService.cs
View File

@@ -2,6 +2,7 @@ using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Threading;
using System.Xml;
@@ -11,6 +12,7 @@ using Umbraco.Core.Configuration;
using Umbraco.Core.Events;
using Umbraco.Core.Logging;
using Umbraco.Core.Models;
using Umbraco.Core.Models.EntityBase;
using Umbraco.Core.Models.Membership;
using Umbraco.Core.Models.Rdbms;
using Umbraco.Core.Persistence;
@@ -714,7 +716,12 @@ namespace Umbraco.Core.Services
/// <returns>An Enumerable list of <see cref="IContent"/> objects</returns>
public IEnumerable<IContent> GetDescendants(IContent content)
{
using (var repository = RepositoryFactory.CreateContentRepository(UowProvider.GetUnitOfWork()))
//This is a check to ensure that the path is correct for this entity to avoid problems like: http://issues.umbraco.org/issue/U4-9336 due to data corruption
if (content.ValidatePath() == false)
throw new InvalidDataException(string.Format("The content item {0} has an invalid path: {1} with parentID: {2}", content.Id, content.Path, content.ParentId));
var uow = UowProvider.GetUnitOfWork();
using (var repository = RepositoryFactory.CreateContentRepository(uow))
{
var pathMatch = content.Path + ",";
var query = Query<IContent>.Builder.Where(x => x.Path.StartsWith(pathMatch) && x.Id != content.Id);
@@ -996,6 +1003,10 @@ namespace Umbraco.Core.Services
using (new WriteLock(Locker))
{
//Hack: this ensures that the entity's path is valid and if not it fixes/persists it
//see: http://issues.umbraco.org/issue/U4-9336
content.EnsureValidPath(Logger, entity => GetById(entity.ParentId), QuickUpdate);
var originalPath = content.Path;
if (Trashing.IsRaisedEventCancelled(
@@ -1683,16 +1694,16 @@ namespace Umbraco.Core.Services
/// <returns>True if sorting succeeded, otherwise False</returns>
public bool Sort(IEnumerable<IContent> items, int userId = 0, bool raiseEvents = true)
{
var asArray = items.ToArray();
if (raiseEvents)
{
if (Saving.IsRaisedEventCancelled(new SaveEventArgs<IContent>(items), this))
if (Saving.IsRaisedEventCancelled(new SaveEventArgs<IContent>(asArray), this))
return false;
}
var shouldBePublished = new List<IContent>();
var shouldBeSaved = new List<IContent>();
var asArray = items.ToArray();
using (new WriteLock(Locker))
{
var uow = UowProvider.GetUnitOfWork();
@@ -1812,6 +1823,23 @@ namespace Umbraco.Core.Services
#region Private Methods
/// <summary>
/// Hack: This is used to fix some data if an entity's properties are invalid/corrupt
/// </summary>
/// <param name="content"></param>
private void QuickUpdate(IContent content)
{
if (content == null) throw new ArgumentNullException("content");
if (content.HasIdentity == false) throw new InvalidOperationException("Cannot update an entity without an Identity");
var uow = UowProvider.GetUnitOfWork();
using (var repository = RepositoryFactory.CreateContentRepository(uow))
{
repository.AddOrUpdate(content);
uow.Commit();
}
}
private void Audit(AuditType type, string message, int userId, int objectId)
{
var uow = UowProvider.GetUnitOfWork();
@@ -1919,6 +1947,10 @@ namespace Umbraco.Core.Services
using (new WriteLock(Locker))
{
//Hack: this ensures that the entity's path is valid and if not it fixes/persists it
//see: http://issues.umbraco.org/issue/U4-9336
content.EnsureValidPath(Logger, entity => GetById(entity.ParentId), QuickUpdate);
var result = new List<Attempt<PublishStatus>>();
//Check if parent is published (although not if its a root node) - if parent isn't published this Content cannot be published

116
src/Umbraco.Core/Services/MediaService.cs
View File

@@ -2,6 +2,7 @@ using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
@@ -527,6 +528,10 @@ namespace Umbraco.Core.Services
/// <returns>An Enumerable flat list of <see cref="IMedia"/> objects</returns>
public IEnumerable<IMedia> GetDescendants(IMedia media)
{
//This is a check to ensure that the path is correct for this entity to avoid problems like: http://issues.umbraco.org/issue/U4-9336 due to data corruption
if (media.ValidatePath() == false)
throw new InvalidDataException(string.Format("The content item {0} has an invalid path: {1} with parentID: {2}", media.Id, media.Path, media.ParentId));
var uow = UowProvider.GetUnitOfWork();
using (var repository = RepositoryFactory.CreateMediaRepository(uow))
{
@@ -619,7 +624,7 @@ namespace Umbraco.Core.Services
public IMedia GetMediaByPath(string mediaPath)
{
var umbracoFileValue = mediaPath;
const string Pattern = ".*[_][0-9]+[x][0-9]+[.].*";
var isResized = Regex.IsMatch(mediaPath, Pattern);
@@ -998,56 +1003,63 @@ namespace Umbraco.Core.Services
{
if (media == null) throw new ArgumentNullException("media");
var originalPath = media.Path;
var evtMsgs = EventMessagesFactory.Get();
if (Trashing.IsRaisedEventCancelled(
new MoveEventArgs<IMedia>(new MoveEventInfo<IMedia>(media, originalPath, Constants.System.RecycleBinMedia)), this))
using (new WriteLock(Locker))
{
return OperationStatus.Cancelled(evtMsgs);
}
//Hack: this ensures that the entity's path is valid and if not it fixes/persists it
//see: http://issues.umbraco.org/issue/U4-9336
media.EnsureValidPath(Logger, entity => GetById(entity.ParentId), QuickUpdate);
var moveInfo = new List<MoveEventInfo<IMedia>>
{
new MoveEventInfo<IMedia>(media, originalPath, Constants.System.RecycleBinMedia)
};
var originalPath = media.Path;
//Find Descendants, which will be moved to the recycle bin along with the parent/grandparent.
var descendants = GetDescendants(media).OrderBy(x => x.Level).ToList();
var uow = UowProvider.GetUnitOfWork();
using (var repository = RepositoryFactory.CreateMediaRepository(uow))
{
//TODO: This should be part of the repo!
//Remove 'published' xml from the cmsContentXml table for the unpublished media
uow.Database.Delete<ContentXmlDto>("WHERE nodeId = @Id", new { Id = media.Id });
media.ChangeTrashedState(true, Constants.System.RecycleBinMedia);
repository.AddOrUpdate(media);
//Loop through descendants to update their trash state, but ensuring structure by keeping the ParentId
foreach (var descendant in descendants)
if (Trashing.IsRaisedEventCancelled(
new MoveEventArgs<IMedia>(new MoveEventInfo<IMedia>(media, originalPath, Constants.System.RecycleBinMedia)), this))
{
//Remove 'published' xml from the cmsContentXml table for the unpublished media
uow.Database.Delete<ContentXmlDto>("WHERE nodeId = @Id", new { Id = descendant.Id });
descendant.ChangeTrashedState(true, descendant.ParentId);
repository.AddOrUpdate(descendant);
moveInfo.Add(new MoveEventInfo<IMedia>(descendant, descendant.Path, descendant.ParentId));
return OperationStatus.Cancelled(evtMsgs);
}
uow.Commit();
var moveInfo = new List<MoveEventInfo<IMedia>>
{
new MoveEventInfo<IMedia>(media, originalPath, Constants.System.RecycleBinMedia)
};
//Find Descendants, which will be moved to the recycle bin along with the parent/grandparent.
var descendants = GetDescendants(media).OrderBy(x => x.Level).ToList();
var uow = UowProvider.GetUnitOfWork();
using (var repository = RepositoryFactory.CreateMediaRepository(uow))
{
//TODO: This should be part of the repo!
//Remove 'published' xml from the cmsContentXml table for the unpublished media
uow.Database.Delete<ContentXmlDto>("WHERE nodeId = @Id", new { Id = media.Id });
media.ChangeTrashedState(true, Constants.System.RecycleBinMedia);
repository.AddOrUpdate(media);
//Loop through descendants to update their trash state, but ensuring structure by keeping the ParentId
foreach (var descendant in descendants)
{
//Remove 'published' xml from the cmsContentXml table for the unpublished media
uow.Database.Delete<ContentXmlDto>("WHERE nodeId = @Id", new { Id = descendant.Id });
descendant.ChangeTrashedState(true, descendant.ParentId);
repository.AddOrUpdate(descendant);
moveInfo.Add(new MoveEventInfo<IMedia>(descendant, descendant.Path, descendant.ParentId));
}
uow.Commit();
}
Trashed.RaiseEvent(
new MoveEventArgs<IMedia>(false, evtMsgs, moveInfo.ToArray()), this);
Audit(AuditType.Move, "Move Media to Recycle Bin performed by user", userId, media.Id);
return OperationStatus.Success(evtMsgs);
}
Trashed.RaiseEvent(
new MoveEventArgs<IMedia>(false, evtMsgs, moveInfo.ToArray()), this);
Audit(AuditType.Move, "Move Media to Recycle Bin performed by user", userId, media.Id);
return OperationStatus.Success(evtMsgs);
}
/// <summary>
@@ -1064,8 +1076,6 @@ namespace Umbraco.Core.Services
((IMediaServiceOperations)this).Delete(media, userId);
}
/// <summary>
/// Permanently deletes versions from an <see cref="IMedia"/> object prior to a specific date.
/// This method will never delete the latest version of a content item.
@@ -1154,7 +1164,6 @@ namespace Umbraco.Core.Services
public bool Sort(IEnumerable<IMedia> items, int userId = 0, bool raiseEvents = true)
{
var asArray = items.ToArray();
if (raiseEvents)
{
if (Saving.IsRaisedEventCancelled(new SaveEventArgs<IMedia>(asArray), this))
@@ -1316,6 +1325,23 @@ namespace Umbraco.Core.Services
}
}
/// <summary>
/// Hack: This is used to fix some data if an entity's properties are invalid/corrupt
/// </summary>
/// <param name="media"></param>
private void QuickUpdate(IMedia media)
{
if (media == null) throw new ArgumentNullException("media");
if (media.HasIdentity == false) throw new InvalidOperationException("Cannot update an entity without an Identity");
var uow = UowProvider.GetUnitOfWork();
using (var repository = RepositoryFactory.CreateMediaRepository(uow))
{
repository.AddOrUpdate(media);
uow.Commit();
}
}
#region Event Handlers
/// <summary>

129
src/Umbraco.Tests/Models/UmbracoEntityTests.cs
View File

@@ -1,7 +1,10 @@
using System;
using System.Diagnostics;
using Moq;
using NUnit.Framework;
using Umbraco.Core.Logging;
using Umbraco.Core.Models;
using Umbraco.Core.Models.EntityBase;
using Umbraco.Core.Serialization;
namespace Umbraco.Tests.Models
@@ -9,6 +12,132 @@ namespace Umbraco.Tests.Models
[TestFixture]
public class UmbracoEntityTests
{
[Test]
public void Validate_Path()
{
var entity = new UmbracoEntity();
//it's empty with no id so we need to allow it
Assert.IsTrue(entity.ValidatePath());
entity.Id = 1234;
//it has an id but no path, so we can't allow it
Assert.IsFalse(entity.ValidatePath());
entity.Path = "-1";
//invalid path
Assert.IsFalse(entity.ValidatePath());
entity.Path = string.Concat("-1,", entity.Id);
//valid path
Assert.IsTrue(entity.ValidatePath());
}
[Test]
public void Ensure_Path_Throws_Without_Id()
{
var entity = new UmbracoEntity();
//no id assigned
Assert.Throws<InvalidOperationException>(() => entity.EnsureValidPath(Mock.Of<ILogger>(), umbracoEntity => new UmbracoEntity(), umbracoEntity => { }));
}
[Test]
public void Ensure_Path_Throws_Without_Parent()
{
var entity = new UmbracoEntity {Id = 1234};
//no parent found
Assert.Throws<NullReferenceException>(() => entity.EnsureValidPath(Mock.Of<ILogger>(), umbracoEntity => null, umbracoEntity => { }));
}
[Test]
public void Ensure_Path_Entity_At_Root()
{
var entity = new UmbracoEntity
{
Id = 1234,
ParentId = -1
};
entity.EnsureValidPath(Mock.Of<ILogger>(), umbracoEntity => null, umbracoEntity => { });
//works because it's under the root
Assert.AreEqual("-1,1234", entity.Path);
}
[Test]
public void Ensure_Path_Entity_Valid_Parent()
{
var entity = new UmbracoEntity
{
Id = 1234,
ParentId = 888
};
entity.EnsureValidPath(Mock.Of<ILogger>(), umbracoEntity => umbracoEntity.ParentId == 888 ? new UmbracoEntity{Id = 888, Path = "-1,888"} : null, umbracoEntity => { });
//works because the parent was found
Assert.AreEqual("-1,888,1234", entity.Path);
}
[Test]
public void Ensure_Path_Entity_Valid_Recursive_Parent()
{
var parentA = new UmbracoEntity
{
Id = 999,
ParentId = -1
};
var parentB = new UmbracoEntity
{
Id = 888,
ParentId = 999
};
var parentC = new UmbracoEntity
{
Id = 777,
ParentId = 888
};
var entity = new UmbracoEntity
{
Id = 1234,
ParentId = 777
};
Func<IUmbracoEntity, IUmbracoEntity> getParent = umbracoEntity =>
{
switch (umbracoEntity.ParentId)
{
case 999:
return parentA;
case 888:
return parentB;
case 777:
return parentC;
case 1234:
return entity;
default:
return null;
}
};
//this will recursively fix all paths
entity.EnsureValidPath(Mock.Of<ILogger>(), getParent, umbracoEntity => { });
Assert.AreEqual("-1,999", parentA.Path);
Assert.AreEqual("-1,999,888", parentB.Path);
Assert.AreEqual("-1,999,888,777", parentC.Path);
Assert.AreEqual("-1,999,888,777,1234", entity.Path);
}
[Test]
public void UmbracoEntity_Can_Be_Initialized_From_Dynamic()
{

16
src/Umbraco.Tests/Services/ContentServiceTests.cs
View File

@@ -55,7 +55,7 @@ namespace Umbraco.Tests.Services
/// Regression test: http://issues.umbraco.org/issue/U4-9336
/// </summary>
[Test]
public void Deleting_Node_With_Invalid_Path()
public void Moving_Node_To_Recycle_Bin_With_Invalid_Path()
{
var contentService = ServiceContext.ContentService;
var root = ServiceContext.ContentService.GetById(NodeDto.NodeIdSeed + 1);
@@ -72,15 +72,19 @@ namespace Umbraco.Tests.Services
//now make the data corrupted :/
DatabaseContext.Database.Execute("UPDATE umbracoNode SET path = '-1' WHERE id = @id", new {id = content.Id});
// need to clear the caches otherwise the ContentService will just serve is a cached version with the non-updated path from db.
ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearAllCache();
//re-get with the corrupt path
content = contentService.GetById(content.Id);
// Note to Shan: put a breakpoint at ContentService.cs line: 1021
// here we get all descendants by the path of the node being moved to bin, and unpublish all of them.
ServiceContext.ContentService.MoveToRecycleBin(content);
// since the path is invalid, there's logic in here to fix that if it's possible and re-persist the entity.
var moveResult = ServiceContext.ContentService.WithResult().MoveToRecycleBin(content);
Assert.IsTrue(moveResult.Success);
//re-get with the fixed/moved path
content = contentService.GetById(content.Id);
Assert.AreEqual("-1,-20," + content.Id, content.Path);
//re-get
hierarchy = contentService.GetByIds(hierarchy.Select(x => x.Id).ToArray()).OrderBy(x => x.Level).ToArray();