diff --git a/src/Umbraco.Core/Constants-SqlTemplates.cs b/src/Umbraco.Core/Constants-SqlTemplates.cs new file mode 100644 index 0000000000..984bc495b0 --- /dev/null +++ b/src/Umbraco.Core/Constants-SqlTemplates.cs @@ -0,0 +1,20 @@ +namespace Umbraco.Core +{ + public static partial class Constants + { + public static class SqlTemplates + { + public static class VersionableRepository + { + public const string GetVersionIds = "Umbraco.Core.VersionableRepository.GetVersionIds"; + public const string GetVersion = "Umbraco.Core.VersionableRepository.GetVersion"; + public const string GetVersions = "Umbraco.Core.VersionableRepository.GetVersions"; + public const string EnsureUniqueNodeName = "Umbraco.Core.VersionableRepository.EnsureUniqueNodeName"; + public const string GetSortOrder = "Umbraco.Core.VersionableRepository.GetSortOrder"; + public const string GetParentNode = "Umbraco.Core.VersionableRepository.GetParentNode"; + public const string GetReservedId = "Umbraco.Core.VersionableRepository.GetReservedId"; + } + + } + } +} diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index b5d5915137..3e70bcda53 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -6,6 +6,7 @@ namespace Umbraco.Core { public static class ContentExtensions { + #region XML methods /// diff --git a/src/Umbraco.Core/Models/ContentDataIntegrityReport.cs b/src/Umbraco.Core/Models/ContentDataIntegrityReport.cs new file mode 100644 index 0000000000..9f0f147083 --- /dev/null +++ b/src/Umbraco.Core/Models/ContentDataIntegrityReport.cs @@ -0,0 +1,48 @@ +using System.Collections.Generic; +using System.Linq; + +namespace Umbraco.Core.Models +{ + public class ContentDataIntegrityReport + { + public ContentDataIntegrityReport(IReadOnlyDictionary detectedIssues) + { + DetectedIssues = detectedIssues; + } + + public bool Ok => DetectedIssues.Count == 0 || DetectedIssues.Count == DetectedIssues.Values.Count(x => x.Fixed); + + public IReadOnlyDictionary DetectedIssues { get; } + + public IReadOnlyDictionary FixedIssues + => DetectedIssues.Where(x => x.Value.Fixed).ToDictionary(x => x.Key, x => x.Value); + + public enum IssueType + { + /// + /// The item's level and path are inconsistent with it's parent's path and level + /// + InvalidPathAndLevelByParentId, + + /// + /// The item's path doesn't contain all required parts + /// + InvalidPathEmpty, + + /// + /// The item's path parts are inconsistent with it's level value + /// + InvalidPathLevelMismatch, + + /// + /// The item's path does not end with it's own ID + /// + InvalidPathById, + + /// + /// The item's path does not have it's parent Id as the 2nd last entry + /// + InvalidPathByParentId, + } + } +} diff --git a/src/Umbraco.Core/Models/ContentDataIntegrityReportEntry.cs b/src/Umbraco.Core/Models/ContentDataIntegrityReportEntry.cs new file mode 100644 index 0000000000..517b9e80dc --- /dev/null +++ b/src/Umbraco.Core/Models/ContentDataIntegrityReportEntry.cs @@ -0,0 +1,13 @@ +namespace Umbraco.Core.Models +{ + public class ContentDataIntegrityReportEntry + { + public ContentDataIntegrityReportEntry(ContentDataIntegrityReport.IssueType issueType) + { + IssueType = issueType; + } + + public ContentDataIntegrityReport.IssueType IssueType { get; } + public bool Fixed { get; set; } + } +} diff --git a/src/Umbraco.Core/Models/ContentDataIntegrityReportOptions.cs b/src/Umbraco.Core/Models/ContentDataIntegrityReportOptions.cs new file mode 100644 index 0000000000..c4689467c1 --- /dev/null +++ b/src/Umbraco.Core/Models/ContentDataIntegrityReportOptions.cs @@ -0,0 +1,13 @@ +namespace Umbraco.Core.Models +{ + public class ContentDataIntegrityReportOptions + { + /// + /// Set to true to try to automatically resolve data integrity issues + /// + public bool FixIssues { get; set; } + + // TODO: We could define all sorts of options for the data integrity check like what to check for, what to fix, etc... + // things like Tag data consistency, etc... + } +} diff --git a/src/Umbraco.Core/Services/IContentServiceBase.cs b/src/Umbraco.Core/Services/IContentServiceBase.cs index 439c55d0d0..c40f49347f 100644 --- a/src/Umbraco.Core/Services/IContentServiceBase.cs +++ b/src/Umbraco.Core/Services/IContentServiceBase.cs @@ -1,9 +1,16 @@ -namespace Umbraco.Core.Services +using Umbraco.Core.Models; + +namespace Umbraco.Core.Services { /// /// Placeholder for sharing logic between the content, media (and member) services /// TODO: Start sharing the logic! /// public interface IContentServiceBase : IService - { } + { + /// + /// Checks/fixes the data integrity of node paths/levels stored in the database + /// + ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options); + } } diff --git a/src/Umbraco.Infrastructure/ContentExtensions.cs b/src/Umbraco.Infrastructure/ContentExtensions.cs index 158e365958..d8d39cc984 100644 --- a/src/Umbraco.Infrastructure/ContentExtensions.cs +++ b/src/Umbraco.Infrastructure/ContentExtensions.cs @@ -4,11 +4,9 @@ using System.IO; using System.Linq; using Newtonsoft.Json; using Newtonsoft.Json.Linq; -using Umbraco.Composing; using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; -using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Core.Strings; @@ -16,6 +14,20 @@ namespace Umbraco.Core { public static class ContentExtensions { + + internal static bool IsMoving(this IContentBase entity) + { + // Check if this entity is being moved as a descendant as part of a bulk moving operations. + // When this occurs, only Path + Level + UpdateDate are being changed. In this case we can bypass a lot of the below + // operations which will make this whole operation go much faster. When moving we don't need to create + // new versions, etc... because we cannot roll this operation back anyways. + var isMoving = entity.IsPropertyDirty(nameof(entity.Path)) + && entity.IsPropertyDirty(nameof(entity.Level)) + && entity.IsPropertyDirty(nameof(entity.UpdateDate)); + + return isMoving; + } + /// /// Removes characters that are not valid XML characters from all entity properties /// of type string. See: http://stackoverflow.com/a/961504/5018 diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/IContentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/IContentRepository.cs index 217719e144..ad9e2d27c1 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/IContentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/IContentRepository.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using Umbraco.Core.Models; using Umbraco.Core.Models.Entities; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Querying; @@ -77,5 +78,7 @@ namespace Umbraco.Core.Persistence.Repositories /// Here, can be null but cannot. IEnumerable GetPage(IQuery query, long pageIndex, int pageSize, out long totalRecords, IQuery filter, Ordering ordering); + + ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options); } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 90f8d454ac..6c216e938f 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -91,7 +91,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // gets all version ids, current first public virtual IEnumerable GetVersionIds(int nodeId, int maxRows) { - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.GetVersionIds", tsql => + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.GetVersionIds, tsql => tsql.Select(x => x.Id) .From() .Where(x => x.NodeId == SqlTemplate.Arg("nodeId")) @@ -107,7 +107,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // TODO: test object node type? // get the version we want to delete - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.GetVersion", tsql => + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.GetVersion, tsql => tsql.Select().From().Where(x => x.Id == SqlTemplate.Arg("versionId")) ); var versionDto = Database.Fetch(template.Sql(new { versionId })).FirstOrDefault(); @@ -129,7 +129,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // TODO: test object node type? // get the versions we want to delete, excluding the current one - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.GetVersions", tsql => + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.GetVersions, tsql => tsql.Select().From().Where(x => x.NodeId == SqlTemplate.Arg("nodeId") && !x.Current && x.VersionDate < SqlTemplate.Arg("versionDate")) ); var versionDtos = Database.Fetch(template.Sql(new { nodeId, versionDate })); @@ -411,7 +411,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } // content type alias is invariant - if(ordering.OrderBy.InvariantEquals("contentTypeAlias")) + if (ordering.OrderBy.InvariantEquals("contentTypeAlias")) { var joins = Sql() .InnerJoin("ctype").On((content, contentType) => content.ContentTypeId == contentType.NodeId, aliasRight: "ctype"); @@ -485,6 +485,123 @@ namespace Umbraco.Core.Persistence.Repositories.Implement IQuery filter, Ordering ordering); + public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options) + { + var report = new Dictionary(); + + var sql = SqlContext.Sql() + .Select() + .From() + .Where(x => x.NodeObjectType == NodeObjectTypeId) + .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); + + var nodesToRebuild = new Dictionary>(); + var validNodes = new Dictionary(); + var rootIds = new[] {Constants.System.Root, Constants.System.RecycleBinContent, Constants.System.RecycleBinMedia}; + var currentParentIds = new HashSet(rootIds); + var prevParentIds = currentParentIds; + var lastLevel = -1; + + // use a forward cursor (query) + foreach (var node in Database.Query(sql)) + { + if (node.Level != lastLevel) + { + // changing levels + prevParentIds = currentParentIds; + currentParentIds = null; + lastLevel = node.Level; + } + + if (currentParentIds == null) + { + // we're reset + currentParentIds = new HashSet(); + } + + currentParentIds.Add(node.NodeId); + + // paths parts without the roots + var pathParts = node.Path.Split(',').Where(x => !rootIds.Contains(int.Parse(x))).ToArray(); + + if (!prevParentIds.Contains(node.ParentId)) + { + // invalid, this will be because the level is wrong (which prob means path is wrong too) + report.Add(node.NodeId, new ContentDataIntegrityReportEntry(ContentDataIntegrityReport.IssueType.InvalidPathAndLevelByParentId)); + AppendNodeToFix(nodesToRebuild, node); + } + else if (pathParts.Length == 0) + { + // invalid path + report.Add(node.NodeId, new ContentDataIntegrityReportEntry(ContentDataIntegrityReport.IssueType.InvalidPathEmpty)); + AppendNodeToFix(nodesToRebuild, node); + } + else if (pathParts.Length != node.Level) + { + // invalid, either path or level is wrong + report.Add(node.NodeId, new ContentDataIntegrityReportEntry(ContentDataIntegrityReport.IssueType.InvalidPathLevelMismatch)); + AppendNodeToFix(nodesToRebuild, node); + } + else if (pathParts[pathParts.Length - 1] != node.NodeId.ToString()) + { + // invalid path + report.Add(node.NodeId, new ContentDataIntegrityReportEntry(ContentDataIntegrityReport.IssueType.InvalidPathById)); + AppendNodeToFix(nodesToRebuild, node); + } + else if (!rootIds.Contains(node.ParentId) && pathParts[pathParts.Length - 2] != node.ParentId.ToString()) + { + // invalid path + report.Add(node.NodeId, new ContentDataIntegrityReportEntry(ContentDataIntegrityReport.IssueType.InvalidPathByParentId)); + AppendNodeToFix(nodesToRebuild, node); + } + else + { + // it's valid! + + // don't track unless we are configured to fix + if (options.FixIssues) + validNodes.Add(node.NodeId, node); + } + } + + var updated = new List(); + + if (options.FixIssues) + { + // iterate all valid nodes to see if these are parents for invalid nodes + foreach (var (nodeId, node) in validNodes) + { + if (!nodesToRebuild.TryGetValue(nodeId, out var invalidNodes)) continue; + + // now we can try to rebuild the invalid paths. + + foreach (var invalidNode in invalidNodes) + { + invalidNode.Level = (short)(node.Level + 1); + invalidNode.Path = node.Path + "," + invalidNode.NodeId; + updated.Add(invalidNode); + } + } + + foreach (var node in updated) + { + Database.Update(node); + if (report.TryGetValue(node.NodeId, out var entry)) + entry.Fixed = true; + } + } + + return new ContentDataIntegrityReport(report); + } + + private static void AppendNodeToFix(IDictionary> nodesToRebuild, NodeDto node) + { + if (nodesToRebuild.TryGetValue(node.ParentId, out var childIds)) + childIds.Add(node); + else + nodesToRebuild[node.ParentId] = new List { node }; + } + // here, filter can be null and ordering cannot protected IEnumerable GetPage(IQuery query, long pageIndex, int pageSize, out long totalRecords, @@ -778,7 +895,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected virtual string EnsureUniqueNodeName(int parentId, string nodeName, int id = 0) { - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.EnsureUniqueNodeName", tsql => tsql + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.EnsureUniqueNodeName, tsql => tsql .Select(x => Alias(x.NodeId, "id"), x => Alias(x.Text, "name")) .From() .Where(x => x.NodeObjectType == SqlTemplate.Arg("nodeObjectType") && x.ParentId == SqlTemplate.Arg("parentId"))); @@ -791,7 +908,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected virtual int GetNewChildSortOrder(int parentId, int first) { - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.GetSortOrder", tsql => + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.GetSortOrder, tsql => tsql.Select($"COALESCE(MAX(sortOrder),{first - 1})").From().Where(x => x.ParentId == SqlTemplate.Arg("parentId") && x.NodeObjectType == NodeObjectTypeId) ); @@ -800,7 +917,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected virtual NodeDto GetParentNodeDto(int parentId) { - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.GetParentNode", tsql => + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.GetParentNode, tsql => tsql.Select().From().Where(x => x.NodeId == SqlTemplate.Arg("parentId")) ); @@ -809,7 +926,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected virtual int GetReservedId(Guid uniqueId) { - var template = SqlContext.Templates.Get("Umbraco.Core.VersionableRepository.GetReservedId", tsql => + var template = SqlContext.Templates.Get(Constants.SqlTemplates.VersionableRepository.GetReservedId, tsql => tsql.Select(x => x.NodeId).From().Where(x => x.UniqueId == SqlTemplate.Arg("uniqueId") && x.NodeObjectType == Constants.ObjectTypes.IdReservation) ); var id = Database.ExecuteScalar(template.Sql(new { uniqueId = uniqueId })); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs index ff4ac4e4dd..e02844f562 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs @@ -331,7 +331,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement .InnerJoin() .On((c, d) => c.Id == d.Id) .Where(x => x.NodeId == SqlTemplate.Arg("nodeId") && !x.Current && x.VersionDate < SqlTemplate.Arg("versionDate")) - .Where( x => !x.Published) + .Where(x => !x.Published) ); var versionDtos = Database.Fetch(template.Sql(new { nodeId, versionDate })); foreach (var versionDto in versionDtos) @@ -529,8 +529,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistUpdatedItem(IContent entity) { - var entityBase = entity as EntityBase; - var isEntityDirty = entityBase != null && entityBase.IsDirty(); + var isEntityDirty = entity.IsDirty(); // check if we need to make any database changes at all if ((entity.PublishedState == PublishedState.Published || entity.PublishedState == PublishedState.Unpublished) @@ -545,29 +544,41 @@ namespace Umbraco.Core.Persistence.Repositories.Implement // update entity.UpdatingEntity(); + // Check if this entity is being moved as a descendant as part of a bulk moving operations. + // In this case we can bypass a lot of the below operations which will make this whole operation go much faster. + // When moving we don't need to create new versions, etc... because we cannot roll this operation back anyways. + var isMoving = entity.IsMoving(); + // TODO: I'm sure we can also detect a "Copy" (of a descendant) operation and probably perform similar checks below. + // There is probably more stuff that would be required for copying but I'm sure not all of this logic would be, we could more than likely boost + // copy performance by 95% just like we did for Move + + var publishing = entity.PublishedState == PublishedState.Publishing; - // check if we need to create a new version - if (publishing && entity.PublishedVersionId > 0) + if (!isMoving) { - // published version is not published anymore - Database.Execute(Sql().Update(u => u.Set(x => x.Published, false)).Where(x => x.Id == entity.PublishedVersionId)); - } + // check if we need to create a new version + if (publishing && entity.PublishedVersionId > 0) + { + // published version is not published anymore + Database.Execute(Sql().Update(u => u.Set(x => x.Published, false)).Where(x => x.Id == entity.PublishedVersionId)); + } - // sanitize names - SanitizeNames(entity, publishing); + // sanitize names + SanitizeNames(entity, publishing); - // ensure that strings don't contain characters that are invalid in xml - // TODO: do we really want to keep doing this here? - entity.SanitizeEntityPropertiesForXmlStorage(); + // ensure that strings don't contain characters that are invalid in xml + // TODO: do we really want to keep doing this here? + entity.SanitizeEntityPropertiesForXmlStorage(); - // if parent has changed, get path, level and sort order - if (entity.IsPropertyDirty("ParentId")) - { - var parent = GetParentNodeDto(entity.ParentId); - entity.Path = string.Concat(parent.Path, ",", entity.Id); - entity.Level = parent.Level + 1; - entity.SortOrder = GetNewChildSortOrder(entity.ParentId, 0); + // if parent has changed, get path, level and sort order + if (entity.IsPropertyDirty("ParentId")) + { + var parent = GetParentNodeDto(entity.ParentId); + entity.Path = string.Concat(parent.Path, ",", entity.Id); + entity.Level = parent.Level + 1; + entity.SortOrder = GetNewChildSortOrder(entity.ParentId, 0); + } } // create the dto @@ -578,146 +589,152 @@ namespace Umbraco.Core.Persistence.Repositories.Implement nodeDto.ValidatePathWithException(); Database.Update(nodeDto); - // update the content dto - Database.Update(dto.ContentDto); - - // update the content & document version dtos - var contentVersionDto = dto.DocumentVersionDto.ContentVersionDto; - var documentVersionDto = dto.DocumentVersionDto; - if (publishing) + if (!isMoving) { - documentVersionDto.Published = true; // now published - contentVersionDto.Current = false; // no more current - } - Database.Update(contentVersionDto); - Database.Update(documentVersionDto); + // update the content dto + Database.Update(dto.ContentDto); - // and, if publishing, insert new content & document version dtos - if (publishing) - { - entity.PublishedVersionId = entity.VersionId; - - contentVersionDto.Id = 0; // want a new id - contentVersionDto.Current = true; // current version - contentVersionDto.Text = entity.Name; - Database.Insert(contentVersionDto); - entity.VersionId = documentVersionDto.Id = contentVersionDto.Id; // get the new id - - documentVersionDto.Published = false; // non-published version - Database.Insert(documentVersionDto); - } - - // replace the property data (rather than updating) - // only need to delete for the version that existed, the new version (if any) has no property data yet - var versionToDelete = publishing ? entity.PublishedVersionId : entity.VersionId; - var deletePropertyDataSql = Sql().Delete().Where(x => x.VersionId == versionToDelete); - Database.Execute(deletePropertyDataSql); - - // insert property data - var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishing ? entity.PublishedVersionId : 0, - entity.Properties, LanguageRepository, out var edited, out var editedCultures); - foreach (var propertyDataDto in propertyDataDtos) - Database.Insert(propertyDataDto); - - // if !publishing, we may have a new name != current publish name, - // also impacts 'edited' - if (!publishing && entity.PublishName != entity.Name) - edited = true; - - if (entity.ContentType.VariesByCulture()) - { - // bump dates to align cultures to version + // update the content & document version dtos + var contentVersionDto = dto.DocumentVersionDto.ContentVersionDto; + var documentVersionDto = dto.DocumentVersionDto; if (publishing) - entity.AdjustDates(contentVersionDto.VersionDate); + { + documentVersionDto.Published = true; // now published + contentVersionDto.Current = false; // no more current + } + Database.Update(contentVersionDto); + Database.Update(documentVersionDto); - // names also impact 'edited' - // ReSharper disable once UseDeconstruction - foreach (var cultureInfo in entity.CultureInfos) - if (cultureInfo.Name != entity.GetPublishName(cultureInfo.Culture)) - { - edited = true; - (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(cultureInfo.Culture); + // and, if publishing, insert new content & document version dtos + if (publishing) + { + entity.PublishedVersionId = entity.VersionId; - // TODO: change tracking - // at the moment, we don't do any dirty tracking on property values, so we don't know whether the - // culture has just been edited or not, so we don't update its update date - that date only changes - // when the name is set, and it all works because the controller does it - but, if someone uses a - // service to change a property value and save (without setting name), the update date does not change. - } + contentVersionDto.Id = 0; // want a new id + contentVersionDto.Current = true; // current version + contentVersionDto.Text = entity.Name; + Database.Insert(contentVersionDto); + entity.VersionId = documentVersionDto.Id = contentVersionDto.Id; // get the new id - // replace the content version variations (rather than updating) + documentVersionDto.Published = false; // non-published version + Database.Insert(documentVersionDto); + } + + // replace the property data (rather than updating) // only need to delete for the version that existed, the new version (if any) has no property data yet - var deleteContentVariations = Sql().Delete().Where(x => x.VersionId == versionToDelete); - Database.Execute(deleteContentVariations); + var versionToDelete = publishing ? entity.PublishedVersionId : entity.VersionId; + var deletePropertyDataSql = Sql().Delete().Where(x => x.VersionId == versionToDelete); + Database.Execute(deletePropertyDataSql); - // replace the document version variations (rather than updating) - var deleteDocumentVariations = Sql().Delete().Where(x => x.NodeId == entity.Id); - Database.Execute(deleteDocumentVariations); + // insert property data + var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, publishing ? entity.PublishedVersionId : 0, + entity.Properties, LanguageRepository, out var edited, out var editedCultures); + foreach (var propertyDataDto in propertyDataDtos) + Database.Insert(propertyDataDto); - // TODO: NPoco InsertBulk issue? - // we should use the native NPoco InsertBulk here but it causes problems (not sure exactly all scenarios) - // but by using SQL Server and updating a variants name will cause: Unable to cast object of type - // 'Umbraco.Core.Persistence.FaultHandling.RetryDbConnection' to type 'System.Data.SqlClient.SqlConnection'. - // (same in PersistNewItem above) + // if !publishing, we may have a new name != current publish name, + // also impacts 'edited' + if (!publishing && entity.PublishName != entity.Name) + edited = true; - // insert content variations - Database.BulkInsertRecords(GetContentVariationDtos(entity, publishing)); + if (entity.ContentType.VariesByCulture()) + { + // bump dates to align cultures to version + if (publishing) + entity.AdjustDates(contentVersionDto.VersionDate); - // insert document variations - Database.BulkInsertRecords(GetDocumentVariationDtos(entity, editedCultures)); + // names also impact 'edited' + // ReSharper disable once UseDeconstruction + foreach (var cultureInfo in entity.CultureInfos) + if (cultureInfo.Name != entity.GetPublishName(cultureInfo.Culture)) + { + edited = true; + (editedCultures ?? (editedCultures = new HashSet(StringComparer.OrdinalIgnoreCase))).Add(cultureInfo.Culture); + + // TODO: change tracking + // at the moment, we don't do any dirty tracking on property values, so we don't know whether the + // culture has just been edited or not, so we don't update its update date - that date only changes + // when the name is set, and it all works because the controller does it - but, if someone uses a + // service to change a property value and save (without setting name), the update date does not change. + } + + // replace the content version variations (rather than updating) + // only need to delete for the version that existed, the new version (if any) has no property data yet + var deleteContentVariations = Sql().Delete().Where(x => x.VersionId == versionToDelete); + Database.Execute(deleteContentVariations); + + // replace the document version variations (rather than updating) + var deleteDocumentVariations = Sql().Delete().Where(x => x.NodeId == entity.Id); + Database.Execute(deleteDocumentVariations); + + // TODO: NPoco InsertBulk issue? + // we should use the native NPoco InsertBulk here but it causes problems (not sure exactly all scenarios) + // but by using SQL Server and updating a variants name will cause: Unable to cast object of type + // 'Umbraco.Core.Persistence.FaultHandling.RetryDbConnection' to type 'System.Data.SqlClient.SqlConnection'. + // (same in PersistNewItem above) + + // insert content variations + Database.BulkInsertRecords(GetContentVariationDtos(entity, publishing)); + + // insert document variations + Database.BulkInsertRecords(GetDocumentVariationDtos(entity, editedCultures)); + } + + // refresh content + entity.SetCultureEdited(editedCultures); + + // update the document dto + // at that point, when un/publishing, the entity still has its old Published value + // so we need to explicitly update the dto to persist the correct value + if (entity.PublishedState == PublishedState.Publishing) + dto.Published = true; + else if (entity.PublishedState == PublishedState.Unpublishing) + dto.Published = false; + entity.Edited = dto.Edited = !dto.Published || edited; // if not published, always edited + Database.Update(dto); + + //update the schedule + if (entity.IsPropertyDirty("ContentSchedule")) + PersistContentSchedule(entity, true); + + // if entity is publishing, update tags, else leave tags there + // means that implicitly unpublished, or trashed, entities *still* have tags in db + if (entity.PublishedState == PublishedState.Publishing) + SetEntityTags(entity, _tagRepository); } - // refresh content - entity.SetCultureEdited(editedCultures); - - // update the document dto - // at that point, when un/publishing, the entity still has its old Published value - // so we need to explicitly update the dto to persist the correct value - if (entity.PublishedState == PublishedState.Publishing) - dto.Published = true; - else if (entity.PublishedState == PublishedState.Unpublishing) - dto.Published = false; - entity.Edited = dto.Edited = !dto.Published || edited; // if not published, always edited - Database.Update(dto); - - //update the schedule - if (entity.IsPropertyDirty("ContentSchedule")) - PersistContentSchedule(entity, true); - - // if entity is publishing, update tags, else leave tags there - // means that implicitly unpublished, or trashed, entities *still* have tags in db - if (entity.PublishedState == PublishedState.Publishing) - SetEntityTags(entity, _tagRepository); - // trigger here, before we reset Published etc OnUowRefreshedEntity(new ScopedEntityEventArgs(AmbientScope, entity)); - // flip the entity's published property - // this also flips its published state - if (entity.PublishedState == PublishedState.Publishing) + if (!isMoving) { - entity.Published = true; - entity.PublishTemplateId = entity.TemplateId; - entity.PublisherId = entity.WriterId; - entity.PublishName = entity.Name; - entity.PublishDate = entity.UpdateDate; + // flip the entity's published property + // this also flips its published state + if (entity.PublishedState == PublishedState.Publishing) + { + entity.Published = true; + entity.PublishTemplateId = entity.TemplateId; + entity.PublisherId = entity.WriterId; + entity.PublishName = entity.Name; + entity.PublishDate = entity.UpdateDate; - SetEntityTags(entity, _tagRepository); + SetEntityTags(entity, _tagRepository); + } + else if (entity.PublishedState == PublishedState.Unpublishing) + { + entity.Published = false; + entity.PublishTemplateId = null; + entity.PublisherId = null; + entity.PublishName = null; + entity.PublishDate = null; + + ClearEntityTags(entity, _tagRepository); + } + + PersistRelations(entity); + + // TODO: note re. tags: explicitly unpublished entities have cleared tags, but masked or trashed entities *still* have tags in the db - so what? } - else if (entity.PublishedState == PublishedState.Unpublishing) - { - entity.Published = false; - entity.PublishTemplateId = null; - entity.PublisherId = null; - entity.PublishName = null; - entity.PublishDate = null; - - ClearEntityTags(entity, _tagRepository); - } - - PersistRelations(entity); - - // TODO: note re. tags: explicitly unpublished entities have cleared tags, but masked or trashed entities *still* have tags in the db - so what? entity.ResetDirtyProperties(); diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs index 11f8c4d696..83088de9bd 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs @@ -231,7 +231,6 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistNewItem(IMedia entity) { - var media = (Models.Media) entity; entity.AddingEntity(); // ensure unique name on the same level @@ -286,15 +285,15 @@ namespace Umbraco.Core.Persistence.Repositories.Implement contentVersionDto.NodeId = nodeDto.NodeId; contentVersionDto.Current = true; Database.Insert(contentVersionDto); - media.VersionId = contentVersionDto.Id; + entity.VersionId = contentVersionDto.Id; // persist the media version dto var mediaVersionDto = dto.MediaVersionDto; - mediaVersionDto.Id = media.VersionId; + mediaVersionDto.Id = entity.VersionId; Database.Insert(mediaVersionDto); // persist the property data - var propertyDataDtos = PropertyFactory.BuildDtos(media.ContentType.Variations, media.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); + var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); foreach (var propertyDataDto in propertyDataDtos) Database.Insert(propertyDataDto); @@ -310,26 +309,32 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistUpdatedItem(IMedia entity) { - var media = (Models.Media) entity; - // update - media.UpdatingEntity(); + entity.UpdatingEntity(); - // ensure unique name on the same level - entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name, entity.Id); + // Check if this entity is being moved as a descendant as part of a bulk moving operations. + // In this case we can bypass a lot of the below operations which will make this whole operation go much faster. + // When moving we don't need to create new versions, etc... because we cannot roll this operation back anyways. + var isMoving = entity.IsMoving(); - // ensure that strings don't contain characters that are invalid in xml - // TODO: do we really want to keep doing this here? - entity.SanitizeEntityPropertiesForXmlStorage(); - - // if parent has changed, get path, level and sort order - if (entity.IsPropertyDirty("ParentId")) + if (!isMoving) { - var parent = GetParentNodeDto(entity.ParentId); + // ensure unique name on the same level + entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name, entity.Id); - entity.Path = string.Concat(parent.Path, ",", entity.Id); - entity.Level = parent.Level + 1; - entity.SortOrder = GetNewChildSortOrder(entity.ParentId, 0); + // ensure that strings don't contain characters that are invalid in xml + // TODO: do we really want to keep doing this here? + entity.SanitizeEntityPropertiesForXmlStorage(); + + // if parent has changed, get path, level and sort order + if (entity.IsPropertyDirty(nameof(entity.ParentId))) + { + var parent = GetParentNodeDto(entity.ParentId); + + entity.Path = string.Concat(parent.Path, ",", entity.Id); + entity.Level = parent.Level + 1; + entity.SortOrder = GetNewChildSortOrder(entity.ParentId, 0); + } } // create the dto @@ -340,26 +345,29 @@ namespace Umbraco.Core.Persistence.Repositories.Implement nodeDto.ValidatePathWithException(); Database.Update(nodeDto); - // update the content dto - Database.Update(dto.ContentDto); + if (!isMoving) + { + // update the content dto + Database.Update(dto.ContentDto); - // update the content & media version dtos - var contentVersionDto = dto.MediaVersionDto.ContentVersionDto; - var mediaVersionDto = dto.MediaVersionDto; - contentVersionDto.Current = true; - Database.Update(contentVersionDto); - Database.Update(mediaVersionDto); + // update the content & media version dtos + var contentVersionDto = dto.MediaVersionDto.ContentVersionDto; + var mediaVersionDto = dto.MediaVersionDto; + contentVersionDto.Current = true; + Database.Update(contentVersionDto); + Database.Update(mediaVersionDto); - // replace the property data - var deletePropertyDataSql = SqlContext.Sql().Delete().Where(x => x.VersionId == media.VersionId); - Database.Execute(deletePropertyDataSql); - var propertyDataDtos = PropertyFactory.BuildDtos(media.ContentType.Variations, media.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); - foreach (var propertyDataDto in propertyDataDtos) - Database.Insert(propertyDataDto); + // replace the property data + var deletePropertyDataSql = SqlContext.Sql().Delete().Where(x => x.VersionId == entity.VersionId); + Database.Execute(deletePropertyDataSql); + var propertyDataDtos = PropertyFactory.BuildDtos(entity.ContentType.Variations, entity.VersionId, 0, entity.Properties, LanguageRepository, out _, out _); + foreach (var propertyDataDto in propertyDataDtos) + Database.Insert(propertyDataDto); - SetEntityTags(entity, _tagRepository); + SetEntityTags(entity, _tagRepository); - PersistRelations(entity); + PersistRelations(entity); + } OnUowRefreshedEntity(new ScopedEntityEventArgs(AmbientScope, entity)); diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs index d02f84d294..ec28614905 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentService.cs @@ -12,6 +12,7 @@ using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Scoping; using Umbraco.Core.Services.Changes; +using Umbraco.Core.Strings; namespace Umbraco.Core.Services.Implement { @@ -27,6 +28,7 @@ namespace Umbraco.Core.Services.Implement private readonly IDocumentBlueprintRepository _documentBlueprintRepository; private readonly ILanguageRepository _languageRepository; private readonly Lazy _propertyValidationService; + private readonly IShortStringHelper _shortStringHelper; private IQuery _queryNotTrashed; #region Constructors @@ -35,7 +37,7 @@ namespace Umbraco.Core.Services.Implement IEventMessagesFactory eventMessagesFactory, IDocumentRepository documentRepository, IEntityRepository entityRepository, IAuditRepository auditRepository, IContentTypeRepository contentTypeRepository, IDocumentBlueprintRepository documentBlueprintRepository, ILanguageRepository languageRepository, - Lazy propertyValidationService) + Lazy propertyValidationService, IShortStringHelper shortStringHelper) : base(provider, logger, eventMessagesFactory) { _documentRepository = documentRepository; @@ -45,6 +47,7 @@ namespace Umbraco.Core.Services.Implement _documentBlueprintRepository = documentBlueprintRepository; _languageRepository = languageRepository; _propertyValidationService = propertyValidationService; + _shortStringHelper = shortStringHelper; } #endregion @@ -600,23 +603,27 @@ namespace Umbraco.Core.Services.Implement totalChildren = 0; return Enumerable.Empty(); } - return GetPagedDescendantsLocked(contentPath[0].Path, pageIndex, pageSize, out totalChildren, filter, ordering); + return GetPagedLocked(GetPagedDescendantQuery(contentPath[0].Path), pageIndex, pageSize, out totalChildren, filter, ordering); } - return GetPagedDescendantsLocked(null, pageIndex, pageSize, out totalChildren, filter, ordering); + return GetPagedLocked(null, pageIndex, pageSize, out totalChildren, filter, ordering); } } - private IEnumerable GetPagedDescendantsLocked(string contentPath, long pageIndex, int pageSize, out long totalChildren, + private IQuery GetPagedDescendantQuery(string contentPath) + { + var query = Query(); + if (!contentPath.IsNullOrWhiteSpace()) + query.Where(x => x.Path.SqlStartsWith($"{contentPath},", TextColumnType.NVarchar)); + return query; + } + + private IEnumerable GetPagedLocked(IQuery query, long pageIndex, int pageSize, out long totalChildren, IQuery filter, Ordering ordering) { if (pageIndex < 0) throw new ArgumentOutOfRangeException(nameof(pageIndex)); if (pageSize <= 0) throw new ArgumentOutOfRangeException(nameof(pageSize)); if (ordering == null) throw new ArgumentNullException(nameof(ordering)); - var query = Query(); - if (!contentPath.IsNullOrWhiteSpace()) - query.Where(x => x.Path.SqlStartsWith($"{contentPath},", TextColumnType.NVarchar)); - return _documentRepository.GetPage(query, pageIndex, pageSize, out totalChildren, filter, ordering); } @@ -1865,7 +1872,7 @@ namespace Umbraco.Core.Services.Implement public OperationResult MoveToRecycleBin(IContent content, int userId) { var evtMsgs = EventMessagesFactory.Get(); - var moves = new List>(); + var moves = new List<(IContent, string)>(); using (var scope = ScopeProvider.CreateScope()) { @@ -1924,7 +1931,7 @@ namespace Umbraco.Core.Services.Implement return; } - var moves = new List>(); + var moves = new List<(IContent, string)>(); using (var scope = ScopeProvider.CreateScope()) { @@ -1977,7 +1984,7 @@ namespace Umbraco.Core.Services.Implement // MUST be called from within WriteLock // trash indicates whether we are trashing, un-trashing, or not changing anything private void PerformMoveLocked(IContent content, int parentId, IContent parent, int userId, - ICollection> moves, + ICollection<(IContent, string)> moves, bool? trash) { content.WriterId = userId; @@ -1989,7 +1996,7 @@ namespace Umbraco.Core.Services.Implement var paths = new Dictionary(); - moves.Add(Tuple.Create(content, content.Path)); // capture original path + moves.Add((content, content.Path)); // capture original path //need to store the original path to lookup descendants based on it below var originalPath = content.Path; @@ -2006,20 +2013,24 @@ namespace Umbraco.Core.Services.Implement paths[content.Id] = (parent == null ? (parentId == Constants.System.RecycleBinContent ? "-1,-20" : Constants.System.RootString) : parent.Path) + "," + content.Id; const int pageSize = 500; - var total = long.MaxValue; - while (total > 0) + var query = GetPagedDescendantQuery(originalPath); + long total; + do { - var descendants = GetPagedDescendantsLocked(originalPath, 0, pageSize, out total, null, Ordering.By("Path", Direction.Ascending)); + // We always page a page 0 because for each page, we are moving the result so the resulting total will be reduced + var descendants = GetPagedLocked(query, 0, pageSize, out total, null, Ordering.By("Path", Direction.Ascending)); + foreach (var descendant in descendants) { - moves.Add(Tuple.Create(descendant, descendant.Path)); // capture original path + moves.Add((descendant, descendant.Path)); // capture original path // update path and level since we do not update parentId descendant.Path = paths[descendant.Id] = paths[descendant.ParentId] + "," + descendant.Id; descendant.Level += levelDelta; PerformMoveContentLocked(descendant, userId, trash); } - } + + } while (total > pageSize); } @@ -2367,6 +2378,25 @@ namespace Umbraco.Core.Services.Implement return OperationResult.Succeed(evtMsgs); } + public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options) + { + using (var scope = ScopeProvider.CreateScope(autoComplete: true)) + { + scope.WriteLock(Constants.Locks.ContentTree); + + var report = _documentRepository.CheckDataIntegrity(options); + + if (report.FixedIssues.Count > 0) + { + //The event args needs a content item so we'll make a fake one with enough properties to not cause a null ref + var root = new Content("root", -1, new ContentType(_shortStringHelper, -1)) {Id = -1, Key = Guid.Empty}; + scope.Events.Dispatch(TreeChanged, this, new TreeChange.EventArgs(new TreeChange(root, TreeChangeTypes.RefreshAll))); + } + + return report; + } + } + #endregion #region Internal Methods @@ -2804,7 +2834,7 @@ namespace Umbraco.Core.Services.Implement // which we need for many things like keeping caches in sync, but we can surely do this MUCH better. var changes = new List>(); - var moves = new List>(); + var moves = new List<(IContent, string)>(); var contentTypeIdsA = contentTypeIds.ToArray(); // using an immediate uow here because we keep making changes with diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs index 14bebc4eb8..5e9854ad9e 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaService.cs @@ -13,6 +13,7 @@ using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Scoping; using Umbraco.Core.Services.Changes; +using Umbraco.Core.Strings; namespace Umbraco.Core.Services.Implement { @@ -25,6 +26,7 @@ namespace Umbraco.Core.Services.Implement private readonly IMediaTypeRepository _mediaTypeRepository; private readonly IAuditRepository _auditRepository; private readonly IEntityRepository _entityRepository; + private readonly IShortStringHelper _shortStringHelper; private readonly IMediaFileSystem _mediaFileSystem; @@ -32,7 +34,7 @@ namespace Umbraco.Core.Services.Implement public MediaService(IScopeProvider provider, IMediaFileSystem mediaFileSystem, ILogger logger, IEventMessagesFactory eventMessagesFactory, IMediaRepository mediaRepository, IAuditRepository auditRepository, IMediaTypeRepository mediaTypeRepository, - IEntityRepository entityRepository) + IEntityRepository entityRepository, IShortStringHelper shortStringHelper) : base(provider, logger, eventMessagesFactory) { _mediaFileSystem = mediaFileSystem; @@ -40,6 +42,7 @@ namespace Umbraco.Core.Services.Implement _auditRepository = auditRepository; _mediaTypeRepository = mediaTypeRepository; _entityRepository = entityRepository; + _shortStringHelper = shortStringHelper; } #endregion @@ -530,23 +533,27 @@ namespace Umbraco.Core.Services.Implement totalChildren = 0; return Enumerable.Empty(); } - return GetPagedDescendantsLocked(mediaPath[0].Path, pageIndex, pageSize, out totalChildren, filter, ordering); + return GetPagedLocked(GetPagedDescendantQuery(mediaPath[0].Path), pageIndex, pageSize, out totalChildren, filter, ordering); } - return GetPagedDescendantsLocked(null, pageIndex, pageSize, out totalChildren, filter, ordering); + return GetPagedLocked(GetPagedDescendantQuery(null), pageIndex, pageSize, out totalChildren, filter, ordering); } } - private IEnumerable GetPagedDescendantsLocked(string mediaPath, long pageIndex, int pageSize, out long totalChildren, + private IQuery GetPagedDescendantQuery(string mediaPath) + { + var query = Query(); + if (!mediaPath.IsNullOrWhiteSpace()) + query.Where(x => x.Path.SqlStartsWith(mediaPath + ",", TextColumnType.NVarchar)); + return query; + } + + private IEnumerable GetPagedLocked(IQuery query, long pageIndex, int pageSize, out long totalChildren, IQuery filter, Ordering ordering) { if (pageIndex < 0) throw new ArgumentOutOfRangeException(nameof(pageIndex)); if (pageSize <= 0) throw new ArgumentOutOfRangeException(nameof(pageSize)); if (ordering == null) throw new ArgumentNullException(nameof(ordering)); - var query = Query(); - if (!mediaPath.IsNullOrWhiteSpace()) - query.Where(x => x.Path.SqlStartsWith(mediaPath + ",", TextColumnType.NVarchar)); - return _mediaRepository.GetPage(query, pageIndex, pageSize, out totalChildren, filter, ordering); } @@ -888,7 +895,7 @@ namespace Umbraco.Core.Services.Implement public Attempt MoveToRecycleBin(IMedia media, int userId = Constants.Security.SuperUserId) { var evtMsgs = EventMessagesFactory.Get(); - var moves = new List>(); + var moves = new List<(IMedia, string)>(); using (var scope = ScopeProvider.CreateScope()) { @@ -940,7 +947,7 @@ namespace Umbraco.Core.Services.Implement return OperationResult.Attempt.Succeed(evtMsgs); } - var moves = new List>(); + var moves = new List<(IMedia, string)>(); using (var scope = ScopeProvider.CreateScope()) { @@ -979,7 +986,7 @@ namespace Umbraco.Core.Services.Implement // MUST be called from within WriteLock // trash indicates whether we are trashing, un-trashing, or not changing anything - private void PerformMoveLocked(IMedia media, int parentId, IMedia parent, int userId, ICollection> moves, bool? trash) + private void PerformMoveLocked(IMedia media, int parentId, IMedia parent, int userId, ICollection<(IMedia, string)> moves, bool? trash) { media.ParentId = parentId; @@ -989,7 +996,7 @@ namespace Umbraco.Core.Services.Implement var paths = new Dictionary(); - moves.Add(Tuple.Create(media, media.Path)); // capture original path + moves.Add((media, media.Path)); // capture original path //need to store the original path to lookup descendants based on it below var originalPath = media.Path; @@ -1006,21 +1013,25 @@ namespace Umbraco.Core.Services.Implement paths[media.Id] = (parent == null ? (parentId == Constants.System.RecycleBinMedia ? "-1,-21" : Constants.System.RootString) : parent.Path) + "," + media.Id; const int pageSize = 500; - var page = 0; - var total = long.MaxValue; - while (page * pageSize < total) + var query = GetPagedDescendantQuery(originalPath); + long total; + do { - var descendants = GetPagedDescendantsLocked(originalPath, page++, pageSize, out total, null, Ordering.By("Path", Direction.Ascending)); + // We always page a page 0 because for each page, we are moving the result so the resulting total will be reduced + var descendants = GetPagedLocked(query, 0, pageSize, out total, null, Ordering.By("Path", Direction.Ascending)); + foreach (var descendant in descendants) { - moves.Add(Tuple.Create(descendant, descendant.Path)); // capture original path + moves.Add((descendant, descendant.Path)); // capture original path // update path and level since we do not update parentId descendant.Path = paths[descendant.Id] = paths[descendant.ParentId] + "," + descendant.Id; descendant.Level += levelDelta; PerformMoveMediaLocked(descendant, userId, trash); } - } + + } while (total > pageSize); + } private void PerformMoveMediaLocked(IMedia media, int userId, bool? trash) @@ -1132,6 +1143,26 @@ namespace Umbraco.Core.Services.Implement } return true; + + } + + public ContentDataIntegrityReport CheckDataIntegrity(ContentDataIntegrityReportOptions options) + { + using (var scope = ScopeProvider.CreateScope(autoComplete: true)) + { + scope.WriteLock(Constants.Locks.MediaTree); + + var report = _mediaRepository.CheckDataIntegrity(options); + + if (report.FixedIssues.Count > 0) + { + //The event args needs a content item so we'll make a fake one with enough properties to not cause a null ref + var root = new Models.Media("root", -1, new MediaType(_shortStringHelper, -1)) { Id = -1, Key = Guid.Empty }; + scope.Events.Dispatch(TreeChanged, this, new TreeChange.EventArgs(new TreeChange(root, TreeChangeTypes.RefreshAll))); + } + + return report; + } } #endregion @@ -1270,7 +1301,7 @@ namespace Umbraco.Core.Services.Implement // which we need for many things like keeping caches in sync, but we can surely do this MUCH better. var changes = new List>(); - var moves = new List>(); + var moves = new List<(IMedia, string)>(); var mediaTypeIdsA = mediaTypeIds.ToArray(); using (var scope = ScopeProvider.CreateScope()) @@ -1351,5 +1382,7 @@ namespace Umbraco.Core.Services.Implement } #endregion + + } } diff --git a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs index 9611ecb653..ae3f9ae472 100644 --- a/src/Umbraco.PublishedCache.NuCache/ContentStore.cs +++ b/src/Umbraco.PublishedCache.NuCache/ContentStore.cs @@ -505,6 +505,14 @@ namespace Umbraco.Web.PublishedCache.NuCache } } + /// + /// Validate the and try to create a parent + /// + /// + /// + /// + /// Returns false if the parent was not found or if the kit validation failed + /// private bool BuildKit(ContentNodeKit kit, out LinkedNode parent) { // make sure parent exists @@ -515,6 +523,15 @@ namespace Umbraco.Web.PublishedCache.NuCache return false; } + // We cannot continue if there's no value. This shouldn't happen but it can happen if the database umbracoNode.path + // data is invalid/corrupt. If that is the case, the parentId might be ok but not the Path which can result in null + // because the data sort operation is by path. + if (parent.Value == null) + { + _logger.Warn($"Skip item id={kit.Node.Id}, no Data assigned for linked node with path {kit.Node.Path} and parent id {kit.Node.ParentContentId}. This can indicate data corruption for the Path value for node {kit.Node.Id}. See the Health Check dashboard in Settings to resolve data integrity issues."); + return false; + } + // make sure the kit is valid if (kit.DraftData == null && kit.PublishedData == null) { @@ -803,7 +820,7 @@ namespace Umbraco.Web.PublishedCache.NuCache { //this zero's out the branch (recursively), if we're in a new gen this will add a NULL placeholder for the gen ClearBranchLocked(existing); - //TODO: This removes the current GEN from the tree - do we really want to do that? + //TODO: This removes the current GEN from the tree - do we really want to do that? (not sure if this is still an issue....) RemoveTreeNodeLocked(existing); } @@ -868,6 +885,10 @@ namespace Umbraco.Web.PublishedCache.NuCache private void ClearBranchLocked(ContentNode content) { + // This should never be null, all code that calls this method is null checking but we've seen + // issues of null ref exceptions in issue reports so we'll double check here + if (content == null) throw new ArgumentNullException(nameof(content)); + SetValueLocked(_contentNodes, content.Id, null); if (_localDb != null) RegisterChange(content.Id, ContentNodeKit.Null); @@ -1035,6 +1056,12 @@ namespace Umbraco.Web.PublishedCache.NuCache var parent = parentLink.Value; + // We are doing a null check here but this should no longer be possible because we have a null check in BuildKit + // for the parent.Value property and we'll output a warning. However I'll leave this additional null check in place. + // see https://github.com/umbraco/Umbraco-CMS/issues/7868 + if (parent == null) + throw new PanicException($"A null Value was returned on the {nameof(parentLink)} LinkedNode with id={content.ParentContentId}, potentially your database paths are corrupted."); + // if parent has no children, clone parent + add as first child if (parent.FirstChildContentId < 0) { diff --git a/src/Umbraco.PublishedCache.NuCache/Snap/LinkedNode.cs b/src/Umbraco.PublishedCache.NuCache/Snap/LinkedNode.cs index d187996df8..94f83ac4e5 100644 --- a/src/Umbraco.PublishedCache.NuCache/Snap/LinkedNode.cs +++ b/src/Umbraco.PublishedCache.NuCache/Snap/LinkedNode.cs @@ -11,7 +11,7 @@ { public LinkedNode(TValue value, long gen, LinkedNode next = null) { - Value = value; + Value = value; // This is allowed to be null, we actually explicitly set this to null in ClearLocked Gen = gen; Next = next; } diff --git a/src/Umbraco.Tests/TestHelpers/TestObjects.cs b/src/Umbraco.Tests/TestHelpers/TestObjects.cs index c27bd046c8..78c442f688 100644 --- a/src/Umbraco.Tests/TestHelpers/TestObjects.cs +++ b/src/Umbraco.Tests/TestHelpers/TestObjects.cs @@ -158,12 +158,12 @@ namespace Umbraco.Tests.TestHelpers var userService = GetLazyService(factory, c => new UserService(scopeProvider, logger, eventMessagesFactory, runtimeState, GetRepo(c), GetRepo(c),globalSettings)); var dataTypeService = GetLazyService(factory, c => new DataTypeService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), ioHelper, localizedTextService.Value, localizationService.Value, TestHelper.ShortStringHelper)); var propertyValidationService = new Lazy(() => new PropertyValidationService(propertyEditorCollection, dataTypeService.Value)); - var contentService = GetLazyService(factory, c => new ContentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), propertyValidationService)); + var contentService = GetLazyService(factory, c => new ContentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), propertyValidationService, TestHelper.ShortStringHelper)); var notificationService = GetLazyService(factory, c => new NotificationService(scopeProvider, userService.Value, contentService.Value, localizationService.Value, logger, ioHelper, GetRepo(c), globalSettings, contentSettings)); var serverRegistrationService = GetLazyService(factory, c => new ServerRegistrationService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), TestHelper.GetHostingEnvironment())); var memberGroupService = GetLazyService(factory, c => new MemberGroupService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var memberService = GetLazyService(factory, c => new MemberService(scopeProvider, logger, eventMessagesFactory, memberGroupService.Value, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); - var mediaService = GetLazyService(factory, c => new MediaService(scopeProvider, mediaFileSystem, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); + var mediaService = GetLazyService(factory, c => new MediaService(scopeProvider, mediaFileSystem, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), TestHelper.ShortStringHelper)); var contentTypeService = GetLazyService(factory, c => new ContentTypeService(scopeProvider, logger, eventMessagesFactory, contentService.Value, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); var mediaTypeService = GetLazyService(factory, c => new MediaTypeService(scopeProvider, logger, eventMessagesFactory, mediaService.Value, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c))); var fileService = GetLazyService(factory, c => new FileService(scopeProvider, ioHelper, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), TestHelper.ShortStringHelper, globalSettings)); diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index f450c2300c..9d2602ad6b 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -346,6 +346,7 @@ 8610 / http://localhost:8610 + http://localhost:8700 False False diff --git a/src/Umbraco.Web/HealthCheck/Checks/Data/DatabaseIntegrityCheck.cs b/src/Umbraco.Web/HealthCheck/Checks/Data/DatabaseIntegrityCheck.cs new file mode 100644 index 0000000000..0c3e2f3d91 --- /dev/null +++ b/src/Umbraco.Web/HealthCheck/Checks/Data/DatabaseIntegrityCheck.cs @@ -0,0 +1,124 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Serilog.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Services; + +namespace Umbraco.Web.HealthCheck.Checks.Data +{ + [HealthCheck( + "73DD0C1C-E0CA-4C31-9564-1DCA509788AF", + "Database data integrity check", + Description = "Checks for various data integrity issues in the Umbraco database.", + Group = "Data Integrity")] + public class DatabaseIntegrityCheck : HealthCheck + { + private readonly IContentService _contentService; + private readonly IMediaService _mediaService; + private const string _fixMediaPaths = "fixMediaPaths"; + private const string _fixContentPaths = "fixContentPaths"; + private const string _fixMediaPathsTitle = "Fix media paths"; + private const string _fixContentPathsTitle = "Fix content paths"; + + public DatabaseIntegrityCheck(IContentService contentService, IMediaService mediaService) + { + _contentService = contentService; + _mediaService = mediaService; + } + + /// + /// Get the status for this health check + /// + /// + public override IEnumerable GetStatus() + { + //return the statuses + return new[] + { + CheckDocuments(false), + CheckMedia(false) + }; + } + + private HealthCheckStatus CheckMedia(bool fix) + { + return CheckPaths(_fixMediaPaths, _fixMediaPathsTitle, Core.Constants.UdiEntityType.Media, fix, + () => _mediaService.CheckDataIntegrity(new ContentDataIntegrityReportOptions {FixIssues = fix})); + } + + private HealthCheckStatus CheckDocuments(bool fix) + { + return CheckPaths(_fixContentPaths, _fixContentPathsTitle, Core.Constants.UdiEntityType.Document, fix, + () => _contentService.CheckDataIntegrity(new ContentDataIntegrityReportOptions {FixIssues = fix})); + } + + private HealthCheckStatus CheckPaths(string actionAlias, string actionName, string entityType, bool detailedReport, Func doCheck) + { + var report = doCheck(); + + var actions = new List(); + if (!report.Ok) + { + actions.Add(new HealthCheckAction(actionAlias, Id) + { + Name = actionName + }); + } + + return new HealthCheckStatus(GetReport(report, entityType, detailedReport)) + { + ResultType = report.Ok ? StatusResultType.Success : StatusResultType.Error, + Actions = actions + }; + } + + private static string GetReport(ContentDataIntegrityReport report, string entityType, bool detailed) + { + var sb = new StringBuilder(); + + if (report.Ok) + { + sb.AppendLine($"

All {entityType} paths are valid

"); + + if (!detailed) + return sb.ToString(); + } + else + { + sb.AppendLine($"

{report.DetectedIssues.Count} invalid {entityType} paths detected.

"); + } + + if (detailed && report.DetectedIssues.Count > 0) + { + sb.AppendLine("
    "); + foreach (var issueGroup in report.DetectedIssues.GroupBy(x => x.Value.IssueType)) + { + var countByGroup = issueGroup.Count(); + var fixedByGroup = issueGroup.Count(x => x.Value.Fixed); + sb.AppendLine("
  • "); + sb.AppendLine($"{countByGroup} issues of type {issueGroup.Key} ... {fixedByGroup} fixed"); + sb.AppendLine("
    • "); + } + sb.AppendLine("
"); + } + + return sb.ToString(); + } + + public override HealthCheckStatus ExecuteAction(HealthCheckAction action) + { + switch (action.Alias) + { + case _fixContentPaths: + return CheckDocuments(true); + case _fixMediaPaths: + return CheckMedia(true); + default: + throw new InvalidOperationException("Action not supported"); + } + } + } +} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 9e1179a8f4..be2c25123f 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -160,6 +160,7 @@ +