diff --git a/NuGet.Config b/NuGet.Config index 7d786702f4..31fd84e456 100644 --- a/NuGet.Config +++ b/NuGet.Config @@ -7,6 +7,5 @@ --> - 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 3edad0c963..7bce23e98e 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -60,6 +60,19 @@ namespace Umbraco.Core #endregion + 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.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/Models/RelationType.cs b/src/Umbraco.Core/Models/RelationType.cs index 1085ecdcdd..25156cdb8a 100644 --- a/src/Umbraco.Core/Models/RelationType.cs +++ b/src/Umbraco.Core/Models/RelationType.cs @@ -1,5 +1,6 @@ using System; using System.Runtime.Serialization; +using Umbraco.Core.Exceptions; using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Models @@ -13,27 +14,41 @@ namespace Umbraco.Core.Models { private string _name; private string _alias; - private bool _isBidrectional; + private bool _isBidirectional; private Guid? _parentObjectType; private Guid? _childObjectType; - //TODO: Should we put back the broken ctors with obsolete attributes? - + [Obsolete("This constructor is no longer used and will be removed in future versions, use one of the other constructors instead")] public RelationType(string alias, string name) - : this(name, alias, false, null, null) + : this(name: name, alias: alias, false, null, null) { } public RelationType(string name, string alias, bool isBidrectional, Guid? parentObjectType, Guid? childObjectType) { + if (name == null) throw new ArgumentNullException(nameof(name)); + if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(name)); + if (alias == null) throw new ArgumentNullException(nameof(alias)); + if (string.IsNullOrWhiteSpace(alias)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(alias)); + _name = name; _alias = alias; - _isBidrectional = isBidrectional; + _isBidirectional = isBidrectional; _parentObjectType = parentObjectType; _childObjectType = childObjectType; } + [Obsolete("This constructor is no longer used and will be removed in future versions, use one of the other constructors instead")] + public RelationType(Guid childObjectType, Guid parentObjectType, string alias) + : this(name: alias, alias: alias, isBidrectional: false, parentObjectType: parentObjectType, childObjectType: childObjectType) + { + } + [Obsolete("This constructor is no longer used and will be removed in future versions, use one of the other constructors instead")] + public RelationType(Guid childObjectType, Guid parentObjectType, string alias, string name) + : this(name: name, alias: alias, isBidrectional: false, parentObjectType: parentObjectType, childObjectType: childObjectType) + { + } /// /// Gets or sets the Name of the RelationType @@ -61,8 +76,8 @@ namespace Umbraco.Core.Models [DataMember] public bool IsBidirectional { - get => _isBidrectional; - set => SetPropertyValueAndDetectChanges(value, ref _isBidrectional, nameof(IsBidirectional)); + get => _isBidirectional; + set => SetPropertyValueAndDetectChanges(value, ref _isBidirectional, nameof(IsBidirectional)); } /// diff --git a/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs b/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs index edd87fec68..177a0494a2 100644 --- a/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/RelationTypeFactory.cs @@ -9,7 +9,7 @@ namespace Umbraco.Core.Persistence.Factories public static IRelationType BuildEntity(RelationTypeDto dto) { - var entity = new RelationType(dto.Name, dto.Alias, dto.Dual, dto.ChildObjectType, dto.ParentObjectType); + var entity = new RelationType(dto.Name, dto.Alias, dto.Dual, dto.ParentObjectType, dto.ChildObjectType); try { diff --git a/src/Umbraco.Core/Persistence/NPocoDatabaseExtensions.cs b/src/Umbraco.Core/Persistence/NPocoDatabaseExtensions.cs index acfa51f895..152dcbe6d3 100644 --- a/src/Umbraco.Core/Persistence/NPocoDatabaseExtensions.cs +++ b/src/Umbraco.Core/Persistence/NPocoDatabaseExtensions.cs @@ -18,6 +18,48 @@ namespace Umbraco.Core.Persistence /// public static partial class NPocoDatabaseExtensions { + /// + /// Iterates over the result of a paged data set with a db reader + /// + /// + /// + /// + /// The number of rows to load per page + /// + /// + /// + /// + /// NPoco's normal Page returns a List{T} but sometimes we don't want all that in memory and instead want to + /// iterate over each row with a reader using Query vs Fetch. + /// + internal static IEnumerable QueryPaged(this IDatabase database, long pageSize, Sql sql) + { + var sqlString = sql.SQL; + var sqlArgs = sql.Arguments; + + int? itemCount = null; + long pageIndex = 0; + do + { + // Get the paged queries + database.BuildPageQueries(pageIndex * pageSize, pageSize, sqlString, ref sqlArgs, out var sqlCount, out var sqlPage); + + // get the item count once + if (itemCount == null) + { + itemCount = database.ExecuteScalar(sqlCount, sqlArgs); + } + pageIndex++; + + // iterate over rows without allocating all items to memory (Query vs Fetch) + foreach (var row in database.Query(sqlPage, sqlArgs)) + { + yield return row; + } + + } while ((pageIndex * pageSize) < itemCount); + } + // NOTE // // proper way to do it with TSQL and SQLCE diff --git a/src/Umbraco.Core/Persistence/Repositories/IContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IContentRepository.cs index 217719e144..ad9e2d27c1 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IContentRepository.cs +++ b/src/Umbraco.Core/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.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs index 13b687eb4e..845006891d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs @@ -83,7 +83,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")) @@ -99,7 +99,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(); @@ -121,7 +121,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 })); @@ -403,7 +403,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"); @@ -477,6 +477,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, @@ -770,7 +887,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"))); @@ -783,7 +900,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) ); @@ -792,7 +909,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")) ); @@ -801,7 +918,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.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index ccfa8209fb..db1e2b350d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -321,7 +321,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) @@ -519,8 +519,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) @@ -535,29 +534,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 @@ -568,146 +579,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(); @@ -1183,7 +1200,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (temp.Template2Id.HasValue && templates.ContainsKey(temp.Template2Id.Value)) temp.Content.PublishTemplateId = temp.Template2Id; } - + // set properties if (loadProperties) @@ -1216,7 +1233,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement SetVariations(temp.Content, contentVariations, documentVariations); } } - + foreach (var c in content) @@ -1430,7 +1447,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement yield return dto; } - + } private class ContentVariation diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs index 081efcfdf6..242f21c749 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MediaRepository.cs @@ -219,7 +219,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 @@ -274,15 +273,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); @@ -298,26 +297,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 @@ -328,26 +333,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.Core/Runtime/SqlMainDomLock.cs b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs index 4433a8e307..f3bfe4eefc 100644 --- a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs @@ -32,6 +32,7 @@ namespace Umbraco.Core.Runtime // unique id for our appdomain, this is more unique than the appdomain id which is just an INT counter to its safer _lockId = Guid.NewGuid().ToString(); _logger = logger; + _dbFactory = new UmbracoDatabaseFactory( Constants.System.UmbracoConnectionName, _logger, @@ -40,6 +41,12 @@ namespace Umbraco.Core.Runtime public async Task AcquireLockAsync(int millisecondsTimeout) { + if (!_dbFactory.Configured) + { + // if we aren't configured, then we're in an install state, in which case we have no choice but to assume we can acquire + return true; + } + if (!(_dbFactory.SqlContext.SqlSyntax is SqlServerSyntaxProvider sqlServerSyntaxProvider)) throw new NotSupportedException("SqlMainDomLock is only supported for Sql Server"); @@ -126,6 +133,12 @@ namespace Umbraco.Core.Runtime // poll every 1 second Thread.Sleep(1000); + if (!_dbFactory.Configured) + { + // if we aren't configured, we just keep looping since we can't query the db + continue; + } + lock (_locker) { // If cancellation has been requested we will just exit. Depending on timing of the shutdown, @@ -358,41 +371,44 @@ namespace Umbraco.Core.Runtime _cancellationTokenSource.Cancel(); _cancellationTokenSource.Dispose(); - var db = GetDatabase(); - try + if (_dbFactory.Configured) { - db.BeginTransaction(IsolationLevel.ReadCommitted); - - // get a write lock - _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); - - // When we are disposed, it means we have released the MainDom lock - // and called all MainDom release callbacks, in this case - // if another maindom is actually coming online we need - // to signal to the MainDom coming online that we have shutdown. - // To do that, we update the existing main dom DB record with a suffixed "_updated" string. - // Otherwise, if we are just shutting down, we want to just delete the row. - if (_mainDomChanging) + var db = GetDatabase(); + try { - _logger.Debug("Releasing MainDom, updating row, new application is booting."); - db.Execute($"UPDATE umbracoKeyValue SET [value] = [value] + '{UpdatedSuffix}' WHERE [key] = @key", new { key = MainDomKey }); + db.BeginTransaction(IsolationLevel.ReadCommitted); + + // get a write lock + _sqlServerSyntax.WriteLock(db, Constants.Locks.MainDom); + + // When we are disposed, it means we have released the MainDom lock + // and called all MainDom release callbacks, in this case + // if another maindom is actually coming online we need + // to signal to the MainDom coming online that we have shutdown. + // To do that, we update the existing main dom DB record with a suffixed "_updated" string. + // Otherwise, if we are just shutting down, we want to just delete the row. + if (_mainDomChanging) + { + _logger.Debug("Releasing MainDom, updating row, new application is booting."); + db.Execute($"UPDATE umbracoKeyValue SET [value] = [value] + '{UpdatedSuffix}' WHERE [key] = @key", new { key = MainDomKey }); + } + else + { + _logger.Debug("Releasing MainDom, deleting row, application is shutting down."); + db.Execute("DELETE FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); + } } - else + catch (Exception ex) { - _logger.Debug("Releasing MainDom, deleting row, application is shutting down."); - db.Execute("DELETE FROM umbracoKeyValue WHERE [key] = @key", new { key = MainDomKey }); + ResetDatabase(); + _logger.Error(ex, "Unexpected error during dipsose."); + _hasError = true; + } + finally + { + db?.CompleteTransaction(); + ResetDatabase(); } - } - catch (Exception ex) - { - ResetDatabase(); - _logger.Error(ex, "Unexpected error during dipsose."); - _hasError = true; - } - finally - { - db?.CompleteTransaction(); - ResetDatabase(); } } } diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 6f9ca58821..58279fb4da 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -526,6 +526,6 @@ namespace Umbraco.Core.Services OperationResult Rollback(int id, int versionId, string culture = "*", int userId = Constants.Security.SuperUserId); #endregion - + } } 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.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index 1558b0170b..068864a558 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -601,23 +601,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); } @@ -1866,7 +1870,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()) { @@ -1925,7 +1929,7 @@ namespace Umbraco.Core.Services.Implement return; } - var moves = new List>(); + var moves = new List<(IContent, string)>(); using (var scope = ScopeProvider.CreateScope()) { @@ -1978,7 +1982,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; @@ -1990,7 +1994,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; @@ -2007,20 +2011,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); } @@ -2375,6 +2383,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(-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 @@ -2812,7 +2839,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.Core/Services/Implement/MediaService.cs b/src/Umbraco.Core/Services/Implement/MediaService.cs index 528d0a0bf9..ecd4cccc8d 100644 --- a/src/Umbraco.Core/Services/Implement/MediaService.cs +++ b/src/Umbraco.Core/Services/Implement/MediaService.cs @@ -530,23 +530,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 +892,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 +944,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 +983,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 +993,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 +1010,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) @@ -1139,6 +1147,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(-1)) { Id = -1, Key = Guid.Empty }; + scope.Events.Dispatch(TreeChanged, this, new TreeChange.EventArgs(new TreeChange(root, TreeChangeTypes.RefreshAll))); + } + + return report; + } } #endregion @@ -1277,7 +1305,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()) @@ -1358,5 +1386,7 @@ namespace Umbraco.Core.Services.Implement } #endregion + + } } diff --git a/src/Umbraco.Core/Services/UserServiceExtensions.cs b/src/Umbraco.Core/Services/UserServiceExtensions.cs index 82cab07b25..c365f1ccc2 100644 --- a/src/Umbraco.Core/Services/UserServiceExtensions.cs +++ b/src/Umbraco.Core/Services/UserServiceExtensions.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Web.Security; using Umbraco.Core.Models.Membership; @@ -116,5 +117,17 @@ namespace Umbraco.Core.Services var permissionCollection = userService.GetPermissions(user, nodeId); return permissionCollection.SelectMany(c => c.AssignedPermissions).Distinct().ToArray(); } + + internal static IEnumerable GetProfilesById(this IUserService userService, params int[] ids) + { + var fullUsers = userService.GetUsersById(ids); + + return fullUsers.Select(user => + { + var asProfile = user as IProfile; + return asProfile ?? new UserProfile(user.Id, user.Name); + }); + + } } } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 24954e316c..f15b098473 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -128,9 +128,13 @@ --> + + + + diff --git a/src/Umbraco.Examine/ContentValueSetBuilder.cs b/src/Umbraco.Examine/ContentValueSetBuilder.cs index 9cbc311639..b8477a9047 100644 --- a/src/Umbraco.Examine/ContentValueSetBuilder.cs +++ b/src/Umbraco.Examine/ContentValueSetBuilder.cs @@ -1,9 +1,13 @@ using Examine; +using System; using System.Collections.Generic; using System.Linq; using Umbraco.Core; +using Umbraco.Core.Composing; using Umbraco.Core.Models; +using Umbraco.Core.Models.Membership; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Strings; @@ -16,20 +20,46 @@ namespace Umbraco.Examine { private readonly UrlSegmentProviderCollection _urlSegmentProviders; private readonly IUserService _userService; + private readonly IScopeProvider _scopeProvider; + + [Obsolete("Use the other ctor instead")] + public ContentValueSetBuilder(PropertyEditorCollection propertyEditors, + UrlSegmentProviderCollection urlSegmentProviders, + IUserService userService, + bool publishedValuesOnly) + : this(propertyEditors, urlSegmentProviders, userService, Current.ScopeProvider, publishedValuesOnly) + { + } public ContentValueSetBuilder(PropertyEditorCollection propertyEditors, UrlSegmentProviderCollection urlSegmentProviders, IUserService userService, + IScopeProvider scopeProvider, bool publishedValuesOnly) : base(propertyEditors, publishedValuesOnly) { _urlSegmentProviders = urlSegmentProviders; _userService = userService; + _scopeProvider = scopeProvider; } /// public override IEnumerable GetValueSets(params IContent[] content) { + Dictionary creatorIds; + Dictionary writerIds; + + // We can lookup all of the creator/writer names at once which can save some + // processing below instead of one by one. + using (var scope = _scopeProvider.CreateScope()) + { + creatorIds = _userService.GetProfilesById(content.Select(x => x.CreatorId).ToArray()) + .ToDictionary(x => x.Id, x => x); + writerIds = _userService.GetProfilesById(content.Select(x => x.WriterId).ToArray()) + .ToDictionary(x => x.Id, x => x); + scope.Complete(); + } + // TODO: There is a lot of boxing going on here and ultimately all values will be boxed by Lucene anyways // but I wonder if there's a way to reduce the boxing that we have to do or if it will matter in the end since // Lucene will do it no matter what? One idea was to create a `FieldValue` struct which would contain `object`, `object[]`, `ValueType` and `ValueType[]` @@ -58,8 +88,8 @@ namespace Umbraco.Examine {"urlName", urlValue?.Yield() ?? Enumerable.Empty()}, //Always add invariant urlName {"path", c.Path?.Yield() ?? Enumerable.Empty()}, {"nodeType", c.ContentType.Id.ToString().Yield() ?? Enumerable.Empty()}, - {"creatorName", (c.GetCreatorProfile(_userService)?.Name ?? "??").Yield() }, - {"writerName",(c.GetWriterProfile(_userService)?.Name ?? "??").Yield() }, + {"creatorName", (creatorIds.TryGetValue(c.CreatorId, out var creatorProfile) ? creatorProfile.Name : "??").Yield() }, + {"writerName", (writerIds.TryGetValue(c.WriterId, out var writerProfile) ? writerProfile.Name : "??").Yield() }, {"writerID", new object[] {c.WriterId}}, {"templateID", new object[] {c.TemplateId ?? 0}}, {UmbracoContentIndex.VariesByCultureFieldName, new object[] {"n"}}, diff --git a/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs index 962737e1dc..edde8e8f81 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/RelationTypeRepositoryTest.cs @@ -107,13 +107,16 @@ namespace Umbraco.Tests.Persistence.Repositories var repository = CreateRepository(provider); // Act - var relationType = repository.Get(RelationTypeDto.NodeIdSeed); + var relationType = repository.Get(RelationTypeDto.NodeIdSeed + 2); // Assert Assert.That(relationType, Is.Not.Null); Assert.That(relationType.HasIdentity, Is.True); - Assert.That(relationType.Alias, Is.EqualTo("relateContentOnCopy")); - Assert.That(relationType.Name, Is.EqualTo("Relate Content on Copy")); + Assert.That(relationType.IsBidirectional, Is.True); + Assert.That(relationType.Alias, Is.EqualTo("relateContentToMedia")); + Assert.That(relationType.Name, Is.EqualTo("Relate Content to Media")); + Assert.That(relationType.ChildObjectType, Is.EqualTo(Constants.ObjectTypes.Media)); + Assert.That(relationType.ParentObjectType, Is.EqualTo(Constants.ObjectTypes.Document)); } } @@ -133,7 +136,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.That(relationTypes, Is.Not.Null); Assert.That(relationTypes.Any(), Is.True); Assert.That(relationTypes.Any(x => x == null), Is.False); - Assert.That(relationTypes.Count(), Is.EqualTo(7)); + Assert.That(relationTypes.Count(), Is.EqualTo(8)); } } @@ -190,7 +193,7 @@ namespace Umbraco.Tests.Persistence.Repositories int count = repository.Count(query); // Assert - Assert.That(count, Is.EqualTo(5)); + Assert.That(count, Is.EqualTo(6)); } } @@ -224,8 +227,9 @@ namespace Umbraco.Tests.Persistence.Repositories public void CreateTestData() { - var relateContent = new RelationType("Relate Content on Copy", "relateContentOnCopy", true, Constants.ObjectTypes.Document, new Guid("C66BA18E-EAF3-4CFF-8A22-41B16D66A972")); - var relateContentType = new RelationType("Relate ContentType on Copy", "relateContentTypeOnCopy", true, Constants.ObjectTypes.DocumentType, new Guid("A2CB7800-F571-4787-9638-BC48539A0EFB")); + var relateContent = new RelationType("Relate Content on Copy", "relateContentOnCopy", true, Constants.ObjectTypes.Document, Constants.ObjectTypes.Document); + var relateContentType = new RelationType("Relate ContentType on Copy", "relateContentTypeOnCopy", true, Constants.ObjectTypes.DocumentType, Constants.ObjectTypes.DocumentType); + var relateContentMedia = new RelationType("Relate Content to Media", "relateContentToMedia", true, Constants.ObjectTypes.Document, Constants.ObjectTypes.Media); var provider = TestObjects.GetScopeProvider(Logger); using (var scope = ScopeProvider.CreateScope()) @@ -234,6 +238,7 @@ namespace Umbraco.Tests.Persistence.Repositories repository.Save(relateContent);//Id 2 repository.Save(relateContentType);//Id 3 + repository.Save(relateContentMedia);//Id 4 scope.Complete(); } } diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs index c124582730..834d211994 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs @@ -1200,7 +1200,7 @@ namespace Umbraco.Tests.PublishedContent /// 2) Save and publish it /// 3) Publish it with descendants /// 4) Repeat steps 2 and 3 - /// + /// /// Which has caused an exception. To replicate this test: /// 1) RefreshBranch with kits for a branch where the top most node is unpublished /// 2) RefreshBranch with kits for the branch where the top most node is published @@ -1370,7 +1370,6 @@ namespace Umbraco.Tests.PublishedContent var items = snapshot.Content.GetByXPath("/root/itype"); Assert.AreEqual(items.Count(), items.Count()); } - private void AssertLinkedNode(ContentNode node, int parent, int prevSibling, int nextSibling, int firstChild, int lastChild) { Assert.AreEqual(parent, node.ParentContentId); diff --git a/src/Umbraco.Tests/Services/RelationServiceTests.cs b/src/Umbraco.Tests/Services/RelationServiceTests.cs index 2ec10811b7..06de405cec 100644 --- a/src/Umbraco.Tests/Services/RelationServiceTests.cs +++ b/src/Umbraco.Tests/Services/RelationServiceTests.cs @@ -110,8 +110,8 @@ namespace Umbraco.Tests.Services Assert.AreEqual("Test", rt.Name); Assert.AreEqual("repeatedEventOccurence", rt.Alias); Assert.AreEqual(false, rt.IsBidirectional); - Assert.AreEqual(Constants.ObjectTypes.Document, rt.ChildObjectType.Value); - Assert.AreEqual(Constants.ObjectTypes.Media, rt.ParentObjectType.Value); + Assert.AreEqual(Constants.ObjectTypes.Document, rt.ParentObjectType.Value); + Assert.AreEqual(Constants.ObjectTypes.Media, rt.ChildObjectType.Value); } [Test] diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index a96385a923..016085c352 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -924,6 +924,24 @@ namespace Umbraco.Tests.Services Assert.AreEqual(user.Id, profile.Id); } + [Test] + public void Get_By_Profile_Id_Must_return_null_if_user_not_exists() + { + var profile = ServiceContext.UserService.GetProfileById(42); + + // Assert + Assert.IsNull(profile); + } + + [Test] + public void GetProfilesById_Must_empty_if_users_not_exists() + { + var profiles = ServiceContext.UserService.GetProfilesById(42); + + // Assert + CollectionAssert.IsEmpty(profiles); + } + [Test] public void Get_User_By_Username() { diff --git a/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs b/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs index 1653de827d..e9f18d8947 100644 --- a/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs +++ b/src/Umbraco.Tests/UmbracoExamine/IndexInitializer.cs @@ -30,16 +30,22 @@ namespace Umbraco.Tests.UmbracoExamine /// internal static class IndexInitializer { - public static ContentValueSetBuilder GetContentValueSetBuilder(PropertyEditorCollection propertyEditors, bool publishedValuesOnly) + public static ContentValueSetBuilder GetContentValueSetBuilder(PropertyEditorCollection propertyEditors, IScopeProvider scopeProvider, bool publishedValuesOnly) { - var contentValueSetBuilder = new ContentValueSetBuilder(propertyEditors, new UrlSegmentProviderCollection(new[] { new DefaultUrlSegmentProvider() }), GetMockUserService(), publishedValuesOnly); + var contentValueSetBuilder = new ContentValueSetBuilder( + propertyEditors, + new UrlSegmentProviderCollection(new[] { new DefaultUrlSegmentProvider() }), + GetMockUserService(), + scopeProvider, + publishedValuesOnly); + return contentValueSetBuilder; } - public static ContentIndexPopulator GetContentIndexRebuilder(PropertyEditorCollection propertyEditors, IContentService contentService, ISqlContext sqlContext, bool publishedValuesOnly) + public static ContentIndexPopulator GetContentIndexRebuilder(PropertyEditorCollection propertyEditors, IContentService contentService, IScopeProvider scopeProvider, bool publishedValuesOnly) { - var contentValueSetBuilder = GetContentValueSetBuilder(propertyEditors, publishedValuesOnly); - var contentIndexDataSource = new ContentIndexPopulator(true, null, contentService, sqlContext, contentValueSetBuilder); + var contentValueSetBuilder = GetContentValueSetBuilder(propertyEditors, scopeProvider, publishedValuesOnly); + var contentIndexDataSource = new ContentIndexPopulator(true, null, contentService, scopeProvider.SqlContext, contentValueSetBuilder); return contentIndexDataSource; } diff --git a/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs b/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs index 9e59422310..acb26fb8f6 100644 --- a/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs +++ b/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs @@ -29,7 +29,7 @@ namespace Umbraco.Tests.UmbracoExamine [Test] public void Index_Property_Data_With_Value_Indexer() { - var contentValueSetBuilder = IndexInitializer.GetContentValueSetBuilder(Factory.GetInstance(), false); + var contentValueSetBuilder = IndexInitializer.GetContentValueSetBuilder(Factory.GetInstance(), ScopeProvider, false); using (var luceneDir = new RandomIdRamDirectory()) using (var indexer = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir, @@ -121,7 +121,7 @@ namespace Umbraco.Tests.UmbracoExamine [Test] public void Rebuild_Index() { - var contentRebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider.SqlContext, false); + var contentRebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider, false); var mediaRebuilder = IndexInitializer.GetMediaIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockMediaService()); using (var luceneDir = new RandomIdRamDirectory()) @@ -149,7 +149,7 @@ namespace Umbraco.Tests.UmbracoExamine [Test] public void Index_Protected_Content_Not_Indexed() { - var rebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider.SqlContext, false); + var rebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider, false); using (var luceneDir = new RandomIdRamDirectory()) @@ -274,7 +274,7 @@ namespace Umbraco.Tests.UmbracoExamine [Test] public void Index_Reindex_Content() { - var rebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider.SqlContext, false); + var rebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider, false); using (var luceneDir = new RandomIdRamDirectory()) using (var indexer = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir, validator: new ContentValueSetValidator(false))) @@ -315,7 +315,7 @@ namespace Umbraco.Tests.UmbracoExamine public void Index_Delete_Index_Item_Ensure_Heirarchy_Removed() { - var rebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider.SqlContext, false); + var rebuilder = IndexInitializer.GetContentIndexRebuilder(Factory.GetInstance(), IndexInitializer.GetMockContentService(), ScopeProvider, false); using (var luceneDir = new RandomIdRamDirectory()) using (var indexer = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir)) diff --git a/src/Umbraco.Tests/UmbracoExamine/SearchTests.cs b/src/Umbraco.Tests/UmbracoExamine/SearchTests.cs index a45a33ec00..96e8892cd1 100644 --- a/src/Umbraco.Tests/UmbracoExamine/SearchTests.cs +++ b/src/Umbraco.Tests/UmbracoExamine/SearchTests.cs @@ -55,7 +55,7 @@ namespace Umbraco.Tests.UmbracoExamine allRecs); var propertyEditors = Factory.GetInstance(); - var rebuilder = IndexInitializer.GetContentIndexRebuilder(propertyEditors, contentService, ScopeProvider.SqlContext, true); + var rebuilder = IndexInitializer.GetContentIndexRebuilder(propertyEditors, contentService, ScopeProvider, true); using (var luceneDir = new RandomIdRamDirectory()) using (var indexer = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir)) diff --git a/src/Umbraco.Web.UI.Client/src/common/services/events.service.js b/src/Umbraco.Web.UI.Client/src/common/services/events.service.js index 51f63e6787..965ac3d635 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/events.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/events.service.js @@ -1,7 +1,7 @@ /** Used to broadcast and listen for global events and allow the ability to add async listeners to the callbacks */ /* - Core app events: + Core app events: app.ready app.authenticated @@ -12,9 +12,9 @@ */ function eventsService($q, $rootScope) { - + return { - + /** raise an event with a given name */ emit: function (name, args) { diff --git a/src/Umbraco.Web.UI.Client/src/common/services/user.service.js b/src/Umbraco.Web.UI.Client/src/common/services/user.service.js index 91dc08ebe2..de6fbaf782 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/user.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/user.service.js @@ -185,7 +185,19 @@ angular.module('umbraco.services') authenticate: function (login, password) { return authResource.performLogin(login, password) - .then(this.setAuthenticationSuccessful); + .then(function(data) { + + // Check if user has a start node set. + if(data.startContentIds.length === 0 && data.startMediaIds.length === 0){ + var errorMsg = "User has no start-nodes"; + var result = { errorMsg: errorMsg, user: data, authenticated: false, lastUserId: lastUserId, loginType: "credentials" }; + eventsService.emit("app.notAuthenticated", result); + throw result; + } + + return data; + + }).then(this.setAuthenticationSuccessful); }, setAuthenticationSuccessful: function (data) { diff --git a/src/Umbraco.Web.UI.Client/src/less/components/umb-content-grid.less b/src/Umbraco.Web.UI.Client/src/less/components/umb-content-grid.less index 622dcb8b0a..47fc8a10b9 100644 --- a/src/Umbraco.Web.UI.Client/src/less/components/umb-content-grid.less +++ b/src/Umbraco.Web.UI.Client/src/less/components/umb-content-grid.less @@ -11,7 +11,7 @@ cursor: pointer; position: relative; user-select: none; - box-shadow: 0 1px 1px 0 rgba(0,0,0,0.16); + box-shadow: 0 1px 2px 0 rgba(0,0,0,0.16); border-radius: 3px; } diff --git a/src/Umbraco.Web.UI.Client/src/main.controller.js b/src/Umbraco.Web.UI.Client/src/main.controller.js index 198ac132c1..81eadf150f 100644 --- a/src/Umbraco.Web.UI.Client/src/main.controller.js +++ b/src/Umbraco.Web.UI.Client/src/main.controller.js @@ -72,6 +72,7 @@ function MainController($scope, $location, appState, treeService, notificationsS $scope.authenticated = null; $scope.user = null; const isTimedOut = data && data.isTimedOut ? true : false; + $scope.showLoginScreen(isTimedOut); // Remove the localstorage items for tours shown diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index e7b4223424..1d77505fc6 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -347,7 +347,7 @@ True 8700 / - http://localhost:8700 + http://localhost:8700 False False @@ -429,4 +429,4 @@ - \ No newline at end of file + 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/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 34d21497a2..a3f918c92c 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -502,6 +502,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 @@ -512,6 +520,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) { @@ -800,7 +817,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); } @@ -865,6 +882,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); @@ -1032,6 +1053,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.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs index 19aab7ea65..694dac04df 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/DatabaseDataSource.cs @@ -20,6 +20,8 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource // provides efficient database access for NuCache internal class DatabaseDataSource : IDataSource { + private const int PageSize = 500; + // we want arrays, we want them all loaded, not an enumerable private Sql ContentSourcesSelect(IScope scope, Func, Sql> joins = null) @@ -79,33 +81,43 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource .Where(x => x.NodeObjectType == Constants.ObjectTypes.Document && !x.Trashed) .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - return scope.Database.Query(sql).Select(CreateContentNodeKit); + // We need to page here. We don't want to iterate over every single row in one connection cuz this can cause an SQL Timeout. + // We also want to read with a db reader and not load everything into memory, QueryPaged lets us do that. + + foreach (var row in scope.Database.QueryPaged(PageSize, sql)) + yield return CreateContentNodeKit(row); } public IEnumerable GetBranchContentSources(IScope scope, int id) { var syntax = scope.SqlContext.SqlSyntax; - var sql = ContentSourcesSelect(scope, s => s + var sql = ContentSourcesSelect(scope, + s => s.InnerJoin("x").On((left, right) => left.NodeId == right.NodeId || SqlText(left.Path, right.Path, (lp, rp) => $"({lp} LIKE {syntax.GetConcat(rp, "',%'")})"), aliasRight: "x")) + .Where(x => x.NodeObjectType == Constants.ObjectTypes.Document && !x.Trashed) + .Where(x => x.NodeId == id, "x") + .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - .InnerJoin("x").On((left, right) => left.NodeId == right.NodeId || SqlText(left.Path, right.Path, (lp, rp) => $"({lp} LIKE {syntax.GetConcat(rp, "',%'")})"), aliasRight: "x")) + // We need to page here. We don't want to iterate over every single row in one connection cuz this can cause an SQL Timeout. + // We also want to read with a db reader and not load everything into memory, QueryPaged lets us do that. - .Where(x => x.NodeObjectType == Constants.ObjectTypes.Document && !x.Trashed) - .Where(x => x.NodeId == id, "x") - .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - - return scope.Database.Query(sql).Select(CreateContentNodeKit); + foreach (var row in scope.Database.QueryPaged(PageSize, sql)) + yield return CreateContentNodeKit(row); } public IEnumerable GetTypeContentSources(IScope scope, IEnumerable ids) { - if (!ids.Any()) return Enumerable.Empty(); + if (!ids.Any()) yield break; var sql = ContentSourcesSelect(scope) .Where(x => x.NodeObjectType == Constants.ObjectTypes.Document && !x.Trashed) .WhereIn(x => x.ContentTypeId, ids) .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - return scope.Database.Query(sql).Select(CreateContentNodeKit); + // We need to page here. We don't want to iterate over every single row in one connection cuz this can cause an SQL Timeout. + // We also want to read with a db reader and not load everything into memory, QueryPaged lets us do that. + + foreach (var row in scope.Database.QueryPaged(PageSize, sql)) + yield return CreateContentNodeKit(row); } private Sql MediaSourcesSelect(IScope scope, Func, Sql> joins = null) @@ -116,11 +128,8 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource x => Alias(x.Level, "Level"), x => Alias(x.Path, "Path"), x => Alias(x.SortOrder, "SortOrder"), x => Alias(x.ParentId, "ParentId"), x => Alias(x.CreateDate, "CreateDate"), x => Alias(x.UserId, "CreatorId")) .AndSelect(x => Alias(x.ContentTypeId, "ContentTypeId")) - .AndSelect(x => Alias(x.Id, "VersionId"), x => Alias(x.Text, "EditName"), x => Alias(x.VersionDate, "EditVersionDate"), x => Alias(x.UserId, "EditWriterId")) - .AndSelect("nuEdit", x => Alias(x.Data, "EditData")) - .From(); if (joins != null) @@ -128,9 +137,7 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource sql = sql .InnerJoin().On((left, right) => left.NodeId == right.NodeId) - .InnerJoin().On((left, right) => left.NodeId == right.NodeId && right.Current) - .LeftJoin("nuEdit").On((left, right) => left.NodeId == right.NodeId && !right.Published, aliasRight: "nuEdit"); return sql; @@ -152,33 +159,43 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource .Where(x => x.NodeObjectType == Constants.ObjectTypes.Media && !x.Trashed) .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - return scope.Database.Query(sql).Select(CreateMediaNodeKit); + // We need to page here. We don't want to iterate over every single row in one connection cuz this can cause an SQL Timeout. + // We also want to read with a db reader and not load everything into memory, QueryPaged lets us do that. + + foreach (var row in scope.Database.QueryPaged(PageSize, sql)) + yield return CreateMediaNodeKit(row); } public IEnumerable GetBranchMediaSources(IScope scope, int id) { var syntax = scope.SqlContext.SqlSyntax; - var sql = MediaSourcesSelect(scope, s => s - - .InnerJoin("x").On((left, right) => left.NodeId == right.NodeId || SqlText(left.Path, right.Path, (lp, rp) => $"({lp} LIKE {syntax.GetConcat(rp, "',%'")})"), aliasRight: "x")) - + var sql = MediaSourcesSelect(scope, + s => s.InnerJoin("x").On((left, right) => left.NodeId == right.NodeId || SqlText(left.Path, right.Path, (lp, rp) => $"({lp} LIKE {syntax.GetConcat(rp, "',%'")})"), aliasRight: "x")) .Where(x => x.NodeObjectType == Constants.ObjectTypes.Media && !x.Trashed) .Where(x => x.NodeId == id, "x") .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - return scope.Database.Query(sql).Select(CreateMediaNodeKit); + // We need to page here. We don't want to iterate over every single row in one connection cuz this can cause an SQL Timeout. + // We also want to read with a db reader and not load everything into memory, QueryPaged lets us do that. + + foreach (var row in scope.Database.QueryPaged(PageSize, sql)) + yield return CreateMediaNodeKit(row); } public IEnumerable GetTypeMediaSources(IScope scope, IEnumerable ids) { - if (!ids.Any()) return Enumerable.Empty(); + if (!ids.Any()) yield break; var sql = MediaSourcesSelect(scope) .Where(x => x.NodeObjectType == Constants.ObjectTypes.Media && !x.Trashed) .WhereIn(x => x.ContentTypeId, ids) .OrderBy(x => x.Level, x => x.ParentId, x => x.SortOrder); - return scope.Database.Query(sql).Select(CreateMediaNodeKit); + // We need to page here. We don't want to iterate over every single row in one connection cuz this can cause an SQL Timeout. + // We also want to read with a db reader and not load everything into memory, QueryPaged lets us do that. + + foreach (var row in scope.Database.QueryPaged(PageSize, sql)) + yield return CreateMediaNodeKit(row); } private static ContentNodeKit CreateContentNodeKit(ContentSourceDto dto) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index a33d9ee427..6866878484 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -5,6 +5,7 @@ using System.Globalization; using System.IO; using System.Linq; using System.Threading; +using System.Threading.Tasks; using CSharpTest.Net.Collections; using Newtonsoft.Json; using Umbraco.Core; @@ -866,12 +867,37 @@ namespace Umbraco.Web.PublishedCache.NuCache //into a new DLL for the application which includes both content types and media types. //Since the models in the cache are based on these actual classes, all of the objects in the cache need to be updated //to use the newest version of the class. - using (_contentStore.GetScopedWriteLock(_scopeProvider)) - using (_mediaStore.GetScopedWriteLock(_scopeProvider)) - { - NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, null, TreeChangeTypes.RefreshAll) }, out var draftChanged, out var publishedChanged); - NotifyLocked(new[] { new MediaCacheRefresher.JsonPayload(0, null, TreeChangeTypes.RefreshAll) }, out var anythingChanged); - } + + // NOTE: Ideally this can be run on background threads here which would prevent blocking the UI + // as is the case when saving a content type. Intially one would think that it won't be any different + // between running this here or in another background thread immediately after with regards to how the + // UI will respond because we already know between calling `WithSafeLiveFactoryReset` to reset the PureLive models + // and this code here, that many front-end requests could be attempted to be processed. If that is the case, those pages are going to get a + // model binding error and our ModelBindingExceptionFilter is going to to its magic to reload those pages so the end user is none the wiser. + // So whether or not this executes 'here' or on a background thread immediately wouldn't seem to make any difference except that we can return + // execution to the UI sooner. + // BUT!... there is a difference IIRC. There is still execution logic that continues after this call on this thread with the cache refreshers + // and those cache refreshers need to have the up-to-date data since other user cache refreshers will be expecting the data to be 'live'. If + // we ran this on a background thread then those cache refreshers are going to not get 'live' data when they query the content cache which + // they require. + + // These can be run side by side in parallel. + + Parallel.Invoke( + () => + { + using (_contentStore.GetScopedWriteLock(_scopeProvider)) + { + NotifyLocked(new[] { new ContentCacheRefresher.JsonPayload(0, null, TreeChangeTypes.RefreshAll) }, out _, out _); + } + }, + () => + { + using (_mediaStore.GetScopedWriteLock(_scopeProvider)) + { + NotifyLocked(new[] { new MediaCacheRefresher.JsonPayload(0, null, TreeChangeTypes.RefreshAll) }, out _); + } + }); } ((PublishedSnapshot)CurrentPublishedSnapshot)?.Resync(); diff --git a/src/Umbraco.Web/PublishedCache/NuCache/Snap/LinkedNode.cs b/src/Umbraco.Web/PublishedCache/NuCache/Snap/LinkedNode.cs index d187996df8..94f83ac4e5 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/Snap/LinkedNode.cs +++ b/src/Umbraco.Web/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.Web/Search/ExamineComposer.cs b/src/Umbraco.Web/Search/ExamineComposer.cs index b30f0cbe03..64eeb6978a 100644 --- a/src/Umbraco.Web/Search/ExamineComposer.cs +++ b/src/Umbraco.Web/Search/ExamineComposer.cs @@ -4,6 +4,7 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models; using Umbraco.Core.PropertyEditors; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Strings; using Umbraco.Examine; @@ -36,12 +37,14 @@ namespace Umbraco.Web.Search factory.GetInstance(), factory.GetInstance(), factory.GetInstance(), + factory.GetInstance(), true)); composition.RegisterUnique(factory => new ContentValueSetBuilder( factory.GetInstance(), factory.GetInstance(), factory.GetInstance(), + factory.GetInstance(), false)); composition.RegisterUnique, MediaValueSetBuilder>(); composition.RegisterUnique, MemberValueSetBuilder>(); diff --git a/src/Umbraco.Web/Security/BackOfficeSignInManager.cs b/src/Umbraco.Web/Security/BackOfficeSignInManager.cs index 8e5e532731..5f1c1012f3 100644 --- a/src/Umbraco.Web/Security/BackOfficeSignInManager.cs +++ b/src/Umbraco.Web/Security/BackOfficeSignInManager.cs @@ -121,7 +121,9 @@ namespace Umbraco.Web.Security { _logger.WriteCore(TraceEventType.Information, 0, $"Login attempt failed for username {userName} from IP address {_request.RemoteIpAddress}, no content and/or media start nodes could be found for any of the user's groups", null, null); - return SignInStatus.Failure; + + // We will say its a sucessful login which it is, but they have no node access + return SignInStatus.Success; } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index c3024f63ae..e39687bed8 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -156,6 +156,7 @@ +