From faec8f7d9468481cce53a6443aae006b8dd4f10a Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 27 Jun 2017 16:09:33 +0200 Subject: [PATCH] Fixing tests - all green or ignored --- build/NuSpecs/UmbracoCms.Core.nuspec | 22 +- build/NuSpecs/UmbracoCms.nuspec | 2 +- src/Umbraco.Core/Composing/TypeLoader.cs | 3 +- .../Repositories/ContentRepository.cs | 25 +- .../Repositories/ContentTypeRepositoryBase.cs | 147 +++--- .../UnitOfWork/IScopeUnitOfWorkProvider.cs | 5 +- .../Persistence/UnitOfWork/ScopeUnitOfWork.cs | 5 +- .../UnitOfWork/ScopeUnitOfWorkProvider.cs | 16 +- .../Persistence/UnitOfWork/UnitOfWorkBase.cs | 62 ++- src/Umbraco.Core/Scoping/Scope.cs | 2 +- src/Umbraco.Core/Services/ContentService.cs | 21 +- src/Umbraco.Core/Services/DataTypeService.cs | 5 + src/Umbraco.Core/Services/MediaService.cs | 28 +- src/Umbraco.Core/Services/MemberService.cs | 24 +- .../Plugins/PluginManagerTests.cs | 7 +- .../Routing/NiceUrlRoutesTests.cs | 6 +- src/Umbraco.Tests/Scoping/ScopedXmlTests.cs | 447 ++++++++++-------- .../TestHelpers/TestWithDatabaseBase.cs | 3 +- .../Web/TemplateUtilitiesTests.cs | 2 + .../PublishedContentCache.cs | 4 +- .../XmlPublishedCache/XmlStore.cs | 37 +- 21 files changed, 478 insertions(+), 395 deletions(-) diff --git a/build/NuSpecs/UmbracoCms.Core.nuspec b/build/NuSpecs/UmbracoCms.Core.nuspec index 89170aa492..23210d5c19 100644 --- a/build/NuSpecs/UmbracoCms.Core.nuspec +++ b/build/NuSpecs/UmbracoCms.Core.nuspec @@ -18,31 +18,31 @@ - - - + + + - + - + - + - - - - + + + + - + diff --git a/build/NuSpecs/UmbracoCms.nuspec b/build/NuSpecs/UmbracoCms.nuspec index f376ec6b24..295f2f94d6 100644 --- a/build/NuSpecs/UmbracoCms.nuspec +++ b/build/NuSpecs/UmbracoCms.nuspec @@ -17,7 +17,7 @@ - + diff --git a/src/Umbraco.Core/Composing/TypeLoader.cs b/src/Umbraco.Core/Composing/TypeLoader.cs index 5a82730664..8954d0dad5 100644 --- a/src/Umbraco.Core/Composing/TypeLoader.cs +++ b/src/Umbraco.Core/Composing/TypeLoader.cs @@ -364,7 +364,8 @@ namespace Umbraco.Core.Composing return cache; } - private string GeTypesListFilePath() + // internal for tests + internal string GeTypesListFilePath() { var filename = "umbraco-types." + NetworkHelper.FileSafeMachineName + ".list"; return Path.Combine(_tempFolder, filename); diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 7cbd3f7570..b823d99b83 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -785,7 +785,8 @@ WHERE (@path LIKE {5})", return base.GetDatabaseFieldNameForOrderBy(orderBy); } - // fixme - missing the 'include all versions' thing here - see 7.6 ProcessQuery + // "many" corresponds to 7.6 "includeAllVersions" + // fixme - we are not implementing the double-query thing for pagination from 7.6? // private IEnumerable MapQueryDtos(List dtos, bool withCache = false, bool many = false) { @@ -794,16 +795,32 @@ WHERE (@path LIKE {5})", var contentTypes = new Dictionary(); var templateIds = new List(); + // in case of data corruption we may have more than 1 "newest" - cleanup + var ix = new Dictionary(); + foreach (var dto in dtos) + { + if (ix.TryGetValue(dto.NodeId, out DocumentDto ixDto) == false || ixDto.UpdateDate < dto.UpdateDate) + ix[dto.NodeId] = dto; + } + dtos = ix.Values.ToList(); + // populate published data if (many) { - var publishedDtoIndex = Database.FetchByGroups(dtos.Select(x => x.NodeId), 2000, batch + var roDtos = Database.FetchByGroups(dtos.Select(x => x.NodeId), 2000, batch => Sql() .Select() .From() .WhereIn(x => x.NodeId, batch) - .Where(x => x.Published)) - .ToDictionary(x => x.NodeId, x => x); + .Where(x => x.Published)); + + // in case of data corruption we may have more than 1 "published" - cleanup + var publishedDtoIndex = new Dictionary(); + foreach (var roDto in roDtos) + { + if (publishedDtoIndex.TryGetValue(roDto.NodeId, out DocumentPublishedReadOnlyDto ixDto) == false || ixDto.VersionDate < roDto.VersionDate) + publishedDtoIndex[roDto.NodeId] = roDto; + } foreach (var dto in dtos) { diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepositoryBase.cs index 1e652d829e..08073ba6ca 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentTypeRepositoryBase.cs @@ -1079,103 +1079,92 @@ AND umbracoNode.id <> @id", // query below is not safe + pointless if array is empty if (contentTypeIds.Length == 0) return; - var sql = @"SELECT - pt.contentTypeId as contentTypeId, pt.uniqueID as ptUniqueID, pt.id as ptId, - pt.Alias as ptAlias, pt." + sqlSyntax.GetQuotedColumnName("Description") + @" as ptDesc, pt.mandatory as ptMandatory, - pt.Name as ptName, pt.sortOrder as ptSortOrder, pt.validationRegExp as ptRegExp, + var sqlGroups = @"SELECT + pg.contenttypeNodeId AS contentTypeId, + pg.id AS id, pg.uniqueID AS " + sqlSyntax.GetQuotedColumnName("key") + @", + pg.sortOrder AS sortOrder, pg." + sqlSyntax.GetQuotedColumnName("text") + @" AS text +FROM cmsPropertyTypeGroup pg +WHERE pg.contenttypeNodeId IN (@ids) +ORDER BY contentTypeId, id"; - dt.nodeId as dtId, dt.dbType as dtDbType, dt.propertyEditorAlias as dtPropEdAlias, - - pg.id as pgId, pg.uniqueID as pgKey, pg.sortorder as pgSortOrder, pg." + sqlSyntax.GetQuotedColumnName("text") + @" as pgText - -FROM cmsPropertyType as pt + var sqlProps = @"SELECT + pt.contentTypeId AS contentTypeId, + pt.id AS id, pt.uniqueID AS " + sqlSyntax.GetQuotedColumnName("key") + @", + pt.propertyTypeGroupId AS groupId, + pt.Alias AS alias, pt." + sqlSyntax.GetQuotedColumnName("Description") + @" AS " + sqlSyntax.GetQuotedColumnName("desc") + @", pt.mandatory AS mandatory, + pt.Name AS name, pt.sortOrder AS sortOrder, pt.validationRegExp AS regexp, + dt.nodeId as dataTypeId, dt.dbType as dbType, dt.propertyEditorAlias as editorAlias +FROM cmsPropertyType pt INNER JOIN cmsDataType as dt ON pt.dataTypeId = dt.nodeId -LEFT JOIN cmsPropertyTypeGroup as pg ON pg.id = pt.propertyTypeGroupId +WHERE pt.contentTypeId IN (@ids) +ORDER BY contentTypeId, groupId, id"; -WHERE pt.contentTypeId IN (@contentTypeIds) - -ORDER BY pgId -"; - - //NOTE: we are going to assume there's not going to be more than 2100 content type ids since that is the max SQL param count! - // Since there are 2 groups of params, it will be half! - if (((contentTypeIds.Length / 2) - 1) > 2000) + if (contentTypeIds.Length > 2000) throw new InvalidOperationException("Cannot perform this lookup, too many sql parameters"); - var result = db.Fetch(sql, new { contentTypeIds = contentTypeIds }); + var groups = db.Fetch(sqlGroups, new { ids = contentTypeIds }); + var groupsEnumerator = groups.GetEnumerator(); + var group = groupsEnumerator.MoveNext() ? groupsEnumerator.Current : null; + + var props = db.Fetch(sqlProps, new { ids = contentTypeIds }); + var propsEnumerator = props.GetEnumerator(); + var prop = propsEnumerator.MoveNext() ? propsEnumerator.Current : null; + + // groups are ordered by content type, group id + // props are ordered by content type, group id, prop id foreach (var contentTypeId in contentTypeIds) { - //from this we need to make : - // * PropertyGroupCollection - Contains all property groups along with all property types associated with a group - // * PropertyTypeCollection - Contains all property types that do not belong to a group + var propertyTypeCollection = allPropertyTypeCollection[contentTypeId] = new PropertyTypeCollection(); + var propertyGroupCollection = allPropertyGroupCollection[contentTypeId] = new PropertyGroupCollection(); - //create the property group collection first, this means all groups (even empty ones) and all groups with properties + while (prop != null && prop.contentTypeId == contentTypeId && prop.groupId == null) + { + AddPropertyType(propertyTypeCollection, prop); + prop = propsEnumerator.MoveNext() ? propsEnumerator.Current : null; + } - int currId = contentTypeId; - - var propertyGroupCollection = new PropertyGroupCollection(result - //get all rows that have a group id - .Where(x => x.pgId != null) - //filter based on the current content type - .Where(x => x.contentTypeId == currId) - //turn that into a custom object containing only the group info - .Select(x => new { GroupId = x.pgId, SortOrder = x.pgSortOrder, Text = x.pgText, Key = x.pgKey }) - //get distinct data by id - .DistinctBy(x => (int)x.GroupId) - //for each of these groups, create a group object with it's associated properties - .Select(group => new PropertyGroup(new PropertyTypeCollection( - result - .Where(row => row.pgId == group.GroupId && row.ptId != null) - .Select(row => new PropertyType(row.dtPropEdAlias, Enum.Parse(row.dtDbType), row.ptAlias) - { - //fill in the rest of the property type properties - Description = row.ptDesc, - DataTypeDefinitionId = row.dtId, - Id = row.ptId, - Key = row.ptUniqueID, - Mandatory = Convert.ToBoolean(row.ptMandatory), - Name = row.ptName, - PropertyGroupId = new Lazy(() => group.GroupId, false), - SortOrder = row.ptSortOrder, - ValidationRegExp = row.ptRegExp - }))) + while (group != null && group.contentTypeId == contentTypeId) + { + var propertyGroup = new PropertyGroup(new PropertyTypeCollection()) { - //fill in the rest of the group properties - Id = group.GroupId, - Name = group.Text, - SortOrder = group.SortOrder, - Key = group.Key - }).ToArray()); + Id = group.id, + Name = group.text, + SortOrder = group.sortOrder, + Key = group.key + }; + propertyGroupCollection.Add(propertyGroup); - allPropertyGroupCollection[currId] = propertyGroupCollection; - - //Create the property type collection now (that don't have groups) - - var propertyTypeCollection = new PropertyTypeCollection(result - .Where(x => x.pgId == null) - //filter based on the current content type - .Where(x => x.contentTypeId == currId) - .Select(row => new PropertyType(row.dtPropEdAlias, Enum.Parse(row.dtDbType), row.ptAlias) + while (prop != null && prop.groupId == group.id) { - //fill in the rest of the property type properties - Description = row.ptDesc, - DataTypeDefinitionId = row.dtId, - Id = row.ptId, - Key = row.ptUniqueID, - Mandatory = Convert.ToBoolean(row.ptMandatory), - Name = row.ptName, - PropertyGroupId = null, - SortOrder = row.ptSortOrder, - ValidationRegExp = row.ptRegExp - }).ToArray()); + AddPropertyType(propertyGroup.PropertyTypes, prop, propertyGroup); + prop = propsEnumerator.MoveNext() ? propsEnumerator.Current : null; + } - allPropertyTypeCollection[currId] = propertyTypeCollection; + group = groupsEnumerator.MoveNext() ? groupsEnumerator.Current : null; + } } - + propsEnumerator.Dispose(); + groupsEnumerator.Dispose(); } + private static void AddPropertyType(PropertyTypeCollection propertyTypes, dynamic prop, PropertyGroup propertyGroup = null) + { + var propertyType = new PropertyType(prop.editorAlias, Enum.Parse(prop.dbType), prop.alias) + { + Description = prop.desc, + DataTypeDefinitionId = prop.dataTypeId, + Id = prop.id, + Key = prop.key, + Mandatory = Convert.ToBoolean(prop.mandatory), + Name = prop.name, + PropertyGroupId = propertyGroup == null ? null : new Lazy(() => propertyGroup.Id), + SortOrder = prop.sortOrder, + ValidationRegExp = prop.regexp + }; + propertyTypes.Add(propertyType); + } } protected abstract TEntity PerformGet(Guid id); diff --git a/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWorkProvider.cs b/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWorkProvider.cs index 76e6cdd798..c6771eb355 100644 --- a/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWorkProvider.cs +++ b/src/Umbraco.Core/Persistence/UnitOfWork/IScopeUnitOfWorkProvider.cs @@ -14,14 +14,13 @@ namespace Umbraco.Core.Persistence.UnitOfWork // creates a unit of work // redefine the method to indicate it returns an IScopeUnitOfWork and // not anymore only an IDatabaseUnitOfWork as IDatabaseUnitOfWorkProvider does + // fixme - merge IScope... and IDatabase... new IScopeUnitOfWork CreateUnitOfWork(); // creates a unit of work // support specifying an isolation level // support auto-commit - but beware! it will be committed, whatever happens - // fixme in v8 this should all be merged as one single method with optional args - IScopeUnitOfWork CreateUnitOfWork(bool readOnly); - IScopeUnitOfWork CreateUnitOfWork(IsolationLevel isolationLevel, bool readOnly = false); + IScopeUnitOfWork CreateUnitOfWork(IsolationLevel isolationLevel = IsolationLevel.Unspecified, bool readOnly = false, bool immediate = false); // fixme explain IDatabaseContext DatabaseContext { get; } diff --git a/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs b/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs index 5a201a7dcc..a20865e77d 100644 --- a/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs +++ b/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWork.cs @@ -31,11 +31,12 @@ namespace Umbraco.Core.Persistence.UnitOfWork /// /// /// + /// /// /// This should normally not be used directly and should be created with the UnitOfWorkProvider /// - internal ScopeUnitOfWork(IScopeProvider scopeProvider, RepositoryFactory repositoryFactory, IsolationLevel isolationLevel = IsolationLevel.Unspecified, bool readOnly = false) - : base(repositoryFactory, readOnly) + internal ScopeUnitOfWork(IScopeProvider scopeProvider, RepositoryFactory repositoryFactory, IsolationLevel isolationLevel = IsolationLevel.Unspecified, bool readOnly = false, bool immediate = false) + : base(repositoryFactory, readOnly, immediate) { _scopeProvider = scopeProvider; _isolationLevel = isolationLevel; diff --git a/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWorkProvider.cs b/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWorkProvider.cs index 8a7c6971d4..a1c5efe72c 100644 --- a/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWorkProvider.cs +++ b/src/Umbraco.Core/Persistence/UnitOfWork/ScopeUnitOfWorkProvider.cs @@ -37,21 +37,9 @@ namespace Umbraco.Core.Persistence.UnitOfWork } /// - public IScopeUnitOfWork CreateUnitOfWork(IsolationLevel isolationLevel) + public IScopeUnitOfWork CreateUnitOfWork(IsolationLevel isolationLevel = IsolationLevel.Unspecified, bool readOnly = false, bool immediate = false) { - return new ScopeUnitOfWork(ScopeProvider, _repositoryFactory, isolationLevel); - } - - /// - public IScopeUnitOfWork CreateUnitOfWork(bool readOnly) - { - return new ScopeUnitOfWork(ScopeProvider, _repositoryFactory, readOnly: readOnly); - } - - /// - public IScopeUnitOfWork CreateUnitOfWork(IsolationLevel isolationLevel, bool readOnly) - { - return new ScopeUnitOfWork(ScopeProvider, _repositoryFactory, isolationLevel, readOnly: readOnly); + return new ScopeUnitOfWork(ScopeProvider, _repositoryFactory, isolationLevel, readOnly, immediate); } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/UnitOfWork/UnitOfWorkBase.cs b/src/Umbraco.Core/Persistence/UnitOfWork/UnitOfWorkBase.cs index 80a19e6e19..3af5fe9a70 100644 --- a/src/Umbraco.Core/Persistence/UnitOfWork/UnitOfWorkBase.cs +++ b/src/Umbraco.Core/Persistence/UnitOfWork/UnitOfWorkBase.cs @@ -7,20 +7,28 @@ namespace Umbraco.Core.Persistence.UnitOfWork { public abstract class UnitOfWorkBase : DisposableObject, IUnitOfWork { - private readonly Queue _operations = new Queue(); + private Queue _operations; // fixme - explain readonly // it means that the unit of work *will* complete no matter what // but if an exception is thrown from within the 'using' block // the calling code still can deal with it and cancel everything + // + // fixme - explain immediate + // do not queue things - save allocations + in some complex operations + // that are transactional queueing makes no sense (or we keep flushing) - protected UnitOfWorkBase(RepositoryFactory repositoryFactory, bool readOnly = false) + protected UnitOfWorkBase(RepositoryFactory repositoryFactory, bool readOnly = false, bool immediate = false) { RepositoryFactory = repositoryFactory; ReadOnly = readOnly; + Immediate = immediate; } + private Queue Operations => _operations ?? (_operations = new Queue()); + protected bool ReadOnly { get; } + protected bool Immediate { get; } protected RepositoryFactory RepositoryFactory { get; } @@ -39,12 +47,15 @@ namespace Umbraco.Core.Persistence.UnitOfWork Completed = false; - _operations.Enqueue(new Operation - { - Entity = entity, - Repository = repository, - Type = OperationType.Insert - }); + if (Immediate) + repository.PersistNewItem(entity); + else + Operations.Enqueue(new Operation + { + Entity = entity, + Repository = repository, + Type = OperationType.Insert + }); } /// @@ -59,12 +70,15 @@ namespace Umbraco.Core.Persistence.UnitOfWork Completed = false; - _operations.Enqueue(new Operation - { - Entity = entity, - Repository = repository, - Type = OperationType.Update - }); + if (Immediate) + repository.PersistUpdatedItem(entity); + else + Operations.Enqueue(new Operation + { + Entity = entity, + Repository = repository, + Type = OperationType.Update + }); } /// @@ -79,12 +93,15 @@ namespace Umbraco.Core.Persistence.UnitOfWork Completed = false; - _operations.Enqueue(new Operation - { - Entity = entity, - Repository = repository, - Type = OperationType.Delete - }); + if (Immediate) + repository.PersistDeletedItem(entity); + else + Operations.Enqueue(new Operation + { + Entity = entity, + Repository = repository, + Type = OperationType.Delete + }); } // fixme - we don't need Begin, really, or do we? @@ -98,6 +115,9 @@ namespace Umbraco.Core.Persistence.UnitOfWork Begin(); + if (_operations == null) + return; + while (_operations.Count > 0) { var operation = _operations.Dequeue(); @@ -137,7 +157,7 @@ namespace Umbraco.Core.Persistence.UnitOfWork { // whatever hasn't been commited is lost // not sure we need this as we are being disposed... - _operations.Clear(); + _operations?.Clear(); } /// diff --git a/src/Umbraco.Core/Scoping/Scope.cs b/src/Umbraco.Core/Scoping/Scope.cs index 0074af6ac7..cec84f0368 100644 --- a/src/Umbraco.Core/Scoping/Scope.cs +++ b/src/Umbraco.Core/Scoping/Scope.cs @@ -367,7 +367,7 @@ namespace Umbraco.Core.Scoping { // figure out completed var completed = _completed.HasValue && _completed.Value; - + // deal with database var databaseException = false; if (_database != null) diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 7267d1d420..84b0957992 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -1536,19 +1536,20 @@ namespace Umbraco.Core.Services moves.Add(Tuple.Create(content, content.Path)); // capture original path + // get before moving, in case uow is immediate + var descendants = GetDescendants(content); + // these will be updated by the repo because we changed parentId //content.Path = (parent == null ? "-1" : parent.Path) + "," + content.Id; //content.SortOrder = ((ContentRepository) repository).NextChildSortOrder(parentId); //content.Level += levelDelta; PerformMoveContentLocked(repository, content, userId, trash); - // BUT content.Path will be updated only when the UOW commits, and - // because we want it now, we have to calculate it by ourselves + // if uow is not immediate, content.Path will be updated only when the UOW commits, + // and because we want it now, we have to calculate it by ourselves //paths[content.Id] = content.Path; paths[content.Id] = (parent == null ? (parentId == Constants.System.RecycleBinContent ? "-1,-20" : "-1") : parent.Path) + "," + content.Id; - Console.WriteLine("path " + content.Id + " = " + paths[content.Id]); - var descendants = GetDescendants(content); foreach (var descendant in descendants) { moves.Add(Tuple.Create(descendant, descendant.Path)); // capture original path @@ -2505,7 +2506,11 @@ namespace Umbraco.Core.Services var moves = new List>(); var contentTypeIdsA = contentTypeIds.ToArray(); - using (var uow = UowProvider.CreateUnitOfWork()) + // using an immediate uow here because we keep making changes with + // PerformMoveLocked and DeleteLocked that must be applied immediately, + // no point queuing operations + // + using (var uow = UowProvider.CreateUnitOfWork(immediate: true)) { uow.WriteLock(Constants.Locks.ContentTree); var repository = uow.CreateRepository(); @@ -2531,9 +2536,9 @@ namespace Umbraco.Core.Services // if current content has children, move them to trash var c = content; - var childQuery = repository.QueryT.Where(x => x.Path.StartsWith(c.Path)); + var childQuery = repository.QueryT.Where(x => x.ParentId == c.Id); var children = repository.GetByQuery(childQuery); - foreach (var child in children.Where(x => contentTypeIdsA.Contains(x.ContentTypeId) == false)) + foreach (var child in children) { // see MoveToRecycleBin PerformMoveLocked(repository, child, Constants.System.RecycleBinContent, null, userId, moves, true); @@ -2550,7 +2555,7 @@ namespace Umbraco.Core.Services .Select(x => new MoveEventInfo(x.Item1, x.Item2, x.Item1.ParentId)) .ToArray(); if (moveInfos.Length > 0) - uow.Events.Dispatch(Trashed, this, new MoveEventArgs(false, moveInfos)); + uow.Events.Dispatch(Trashed, this, new MoveEventArgs(false, moveInfos), "Trashed"); uow.Events.Dispatch(TreeChanged, this, changes.ToEventArgs()); Audit(uow, AuditType.Delete, $"Delete Content of Type {string.Join(",", contentTypeIdsA)} performed by user", userId, Constants.System.Root); diff --git a/src/Umbraco.Core/Services/DataTypeService.cs b/src/Umbraco.Core/Services/DataTypeService.cs index 4964a20ff4..d02a90d1c5 100644 --- a/src/Umbraco.Core/Services/DataTypeService.cs +++ b/src/Umbraco.Core/Services/DataTypeService.cs @@ -351,6 +351,11 @@ namespace Umbraco.Core.Services return; } + if (string.IsNullOrWhiteSpace(dataTypeDefinition.Name)) + { + throw new ArgumentException("Cannot save datatype with empty name."); + } + var repository = uow.CreateRepository(); repository.AddOrUpdate(dataTypeDefinition); diff --git a/src/Umbraco.Core/Services/MediaService.cs b/src/Umbraco.Core/Services/MediaService.cs index 06b4449fa7..64f5d5648f 100644 --- a/src/Umbraco.Core/Services/MediaService.cs +++ b/src/Umbraco.Core/Services/MediaService.cs @@ -114,7 +114,11 @@ namespace Umbraco.Core.Services throw new ArgumentException("No media with that id.", nameof(parentId)); var media = new Models.Media(name, parentId, mediaType); - CreateMedia(null, media, parent, userId, false); + using (var uow = UowProvider.CreateUnitOfWork()) + { + CreateMedia(uow, media, parent, userId, false); + uow.Complete(); + } return media; } @@ -139,7 +143,11 @@ namespace Umbraco.Core.Services throw new ArgumentException("No media type with that alias.", nameof(mediaTypeAlias)); var media = new Models.Media(name, -1, mediaType); - CreateMedia(null, media, null, userId, false); + using (var uow = UowProvider.CreateUnitOfWork()) + { + CreateMedia(uow, media, null, userId, false); + uow.Complete(); + } return media; } @@ -1122,18 +1130,20 @@ namespace Umbraco.Core.Services moves.Add(Tuple.Create(media, media.Path)); // capture original path + // get before moving, in case uow is immediate + var descendants = GetDescendants(media); + // these will be updated by the repo because we changed parentId //media.Path = (parent == null ? "-1" : parent.Path) + "," + media.Id; //media.SortOrder = ((MediaRepository) repository).NextChildSortOrder(parentId); //media.Level += levelDelta; PerformMoveMediaLocked(repository, media, userId, trash); - // BUT media.Path will be updated only when the UOW commits, and - // because we want it now, we have to calculate it by ourselves + // if uow is not immediate, content.Path will be updated only when the UOW commits, + // and because we want it now, we have to calculate it by ourselves //paths[media.Id] = media.Path; paths[media.Id] = (parent == null ? (parentId == Constants.System.RecycleBinMedia ? "-1,-21" : "-1") : parent.Path) + "," + media.Id; - var descendants = GetDescendants(media); foreach (var descendant in descendants) { moves.Add(Tuple.Create(descendant, descendant.Path)); // capture original path @@ -1407,7 +1417,11 @@ namespace Umbraco.Core.Services var moves = new List>(); var mediaTypeIdsA = mediaTypeIds.ToArray(); - using (var uow = UowProvider.CreateUnitOfWork()) + // using an immediate uow here because we keep making changes with + // PerformMoveLocked and DeleteLocked that must be applied immediately, + // no point queuing operations + // + using (var uow = UowProvider.CreateUnitOfWork(immediate: true)) { uow.WriteLock(Constants.Locks.MediaTree); var repository = uow.CreateRepository(); @@ -1446,7 +1460,7 @@ namespace Umbraco.Core.Services .Select(x => new MoveEventInfo(x.Item1, x.Item2, x.Item1.ParentId)) .ToArray(); if (moveInfos.Length > 0) - uow.Events.Dispatch(Trashed, this, new MoveEventArgs(false, moveInfos)); + uow.Events.Dispatch(Trashed, this, new MoveEventArgs(false, moveInfos), "Trashed"); uow.Events.Dispatch(TreeChanged, this, changes.ToEventArgs()); Audit(uow, AuditType.Delete, $"Delete Media of types {string.Join(",", mediaTypeIdsA)} performed by user", userId, Constants.System.Root); diff --git a/src/Umbraco.Core/Services/MemberService.cs b/src/Umbraco.Core/Services/MemberService.cs index 78a75fc2af..a6393d5d8b 100644 --- a/src/Umbraco.Core/Services/MemberService.cs +++ b/src/Umbraco.Core/Services/MemberService.cs @@ -122,7 +122,11 @@ namespace Umbraco.Core.Services throw new ArgumentException("No member type with that alias.", nameof(memberTypeAlias)); var member = new Member(name, email.ToLower().Trim(), username, memberType); - CreateMember(null, member, 0, false); + using (var uow = UowProvider.CreateUnitOfWork()) + { + CreateMember(uow, member, 0, false); + uow.Complete(); + } return member; } @@ -143,7 +147,11 @@ namespace Umbraco.Core.Services if (memberType == null) throw new ArgumentNullException(nameof(memberType)); var member = new Member(name, email.ToLower().Trim(), username, memberType); - CreateMember(null, member, 0, false); + using (var uow = UowProvider.CreateUnitOfWork()) + { + CreateMember(uow, member, 0, false); + uow.Complete(); + } return member; } @@ -348,7 +356,7 @@ namespace Umbraco.Core.Services } } - // fixme get rid of string filter, ignored here = bad! + // fixme get rid of string filter? public IEnumerable GetAll(long pageIndex, int pageSize, out long totalRecords, string orderBy, Direction orderDirection, string memberTypeAlias = null, string filter = "") @@ -363,8 +371,9 @@ namespace Umbraco.Core.Services { uow.ReadLock(Constants.Locks.MemberTree); var repository = uow.CreateRepository(); - var query = memberTypeAlias == null ? null : repository.QueryT.Where(x => x.ContentTypeAlias == memberTypeAlias); - return repository.GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, orderBy, orderDirection, orderBySystemField /*, filter*/); + var query1 = memberTypeAlias == null ? null : repository.QueryT.Where(x => x.ContentTypeAlias == memberTypeAlias); + var query2 = filter == null ? null : repository.QueryT.Where(x => x.Name.Contains(filter) || x.Username.Contains(filter)); + return repository.GetPagedResultsByQuery(query1, pageIndex, pageSize, out totalRecords, orderBy, orderDirection, orderBySystemField, query2); } } @@ -832,6 +841,11 @@ namespace Umbraco.Core.Services return; } + if (string.IsNullOrWhiteSpace(member.Name)) + { + throw new ArgumentException("Cannot save member with empty name."); + } + uow.WriteLock(Constants.Locks.MemberTree); var repository = uow.CreateRepository(); diff --git a/src/Umbraco.Tests/Plugins/PluginManagerTests.cs b/src/Umbraco.Tests/Plugins/PluginManagerTests.cs index 5d3185fd16..35b4ad998c 100644 --- a/src/Umbraco.Tests/Plugins/PluginManagerTests.cs +++ b/src/Umbraco.Tests/Plugins/PluginManagerTests.cs @@ -146,10 +146,9 @@ namespace Umbraco.Tests.Plugins [Test] public void Detect_Legacy_Plugin_File_List() { - var tempFolder = IOHelper.MapPath("~/App_Data/TEMP/PluginCache"); - Directory.CreateDirectory(tempFolder); - - var filePath= Path.Combine(tempFolder, string.Format("umbraco-plugins.{0}.list", NetworkHelper.FileSafeMachineName)); + var filePath = _manager.GeTypesListFilePath(); + var fileDir = Path.GetDirectoryName(filePath); + Directory.CreateDirectory(fileDir); File.WriteAllText(filePath, @" diff --git a/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs b/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs index 343968419c..aff999a183 100644 --- a/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs +++ b/src/Umbraco.Tests/Routing/NiceUrlRoutesTests.cs @@ -198,8 +198,7 @@ DetermineRouteById(id): SettingsForTests.HideTopLevelNodeFromPath = hide; - const bool preview = true; // make sure we don't cache - var route = cache.GetRouteById(preview, id); + var route = cache.GetRouteById(false, id); Assert.AreEqual(expected, route); } @@ -222,8 +221,7 @@ DetermineRouteById(id): SettingsForTests.HideTopLevelNodeFromPath = hide; - const bool preview = true; // make sure we don't cache - var route = cache.GetRouteById(preview, id); + var route = cache.GetRouteById(false, id); Assert.AreEqual(expected, route); } diff --git a/src/Umbraco.Tests/Scoping/ScopedXmlTests.cs b/src/Umbraco.Tests/Scoping/ScopedXmlTests.cs index d2f1ee2684..91f071cc0a 100644 --- a/src/Umbraco.Tests/Scoping/ScopedXmlTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopedXmlTests.cs @@ -1,274 +1,303 @@ using System; -using System.Collections; +using System.Collections.Generic; +using System.Xml; using Moq; using NUnit.Framework; -using Umbraco.Core; +using LightInject; +using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Events; -using Umbraco.Core.Logging; using Umbraco.Core.Models; -using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Core.Sync; -using Umbraco.Tests.Cache.DistributedCache; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.Testing; using Umbraco.Web.Cache; +using Umbraco.Web.PublishedCache; +using Umbraco.Web.PublishedCache.XmlPublishedCache; namespace Umbraco.Tests.Scoping { [TestFixture] - [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] + [UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest, FacadeServiceRepositoryEvents = true)] public class ScopedXmlTests : TestWithDatabaseBase { - [Test] - public void Fail() - { - // fixme - // need to rewrite these tests entirely using the XmlStore and such - // need to create an equivalent test for NuCache once we understand it + private CacheRefresherComponent _cacheRefresher; - Assert.Fail("need to rewrite these tests entirely for XmlStore"); + protected override void Compose() + { + base.Compose(); + + // the cache refresher component needs to trigger to refresh caches + // but then, it requires a lot of plumbing ;( + // fixme - and we cannot inject a DistributedCache yet + // so doing all this mess + Container.RegisterSingleton(); + Container.RegisterSingleton(f => Mock.Of()); + Container.RegisterCollectionBuilder() + .Add(f => f.TryGetInstance().GetCacheRefreshers()); } - //private CacheRefresherComponent _cacheRefresher; + [TearDown] + public void Teardown() + { + _cacheRefresher?.Unbind(); + _cacheRefresher = null; - //// fixme ? - ////protected override void FreezeResolution() - ////{ - //// ServerRegistrarResolver.Current = new ServerRegistrarResolver( - //// new DistributedCacheTests.TestServerRegistrar()); - //// ServerMessengerResolver.Current = new ServerMessengerResolver( - //// new DatabaseServerMessenger(ApplicationContext, false, new DatabaseServerMessengerOptions())); - //// CacheRefreshersResolver.Current = new CacheRefreshersResolver( - //// new ActivatorServiceProvider(), Mock.Of(), () => new[] - //// { - //// typeof(PageCacheRefresher), - //// typeof(UnpublishedPageCacheRefresher), + _onPublishedAssertAction = null; + ContentService.Published -= OnPublishedAssert; + SafeXmlReaderWriter.Cloning = null; + } - //// typeof(DomainCacheRefresher), - //// typeof(MacroCacheRefresher) - //// }); + private void OnPublishedAssert(IContentService sender, PublishEventArgs args) + { + _onPublishedAssertAction?.Invoke(); + } - //// base.FreezeResolution(); - ////} + private Action _onPublishedAssertAction; - //[TearDown] - //public void Teardown() - //{ - // _cacheRefresher?.Unbind(); - // _cacheRefresher = null; + // in 7.6, content.Instance + // .XmlContent - comes from .XmlContentInternal and is cached in context items for current request + // .XmlContentInternal - the actual main xml document + // become in 8 + // xmlStore.Xml - the actual main xml document + // publishedContentCache.GetXml() - the captured xml - // _onPublishedAssertAction = null; - // content.HttpContextItemsGetter = null; - // ContentService.Published -= OnPublishedAssert; - // SafeXmlReaderWriter.Cloning = null; + private static XmlStore XmlStore => (Current.Container.GetInstance() as FacadeService).XmlStore; + private static XmlDocument XmlMaster => XmlStore.Xml; + private static XmlDocument XmlInContext => ((PublishedContentCache) Umbraco.Web.Composing.Current.UmbracoContext.ContentCache).GetXml(false); - // ServerRegistrarResolver.Reset(); - // ServerMessengerResolver.Reset(); - // CacheRefreshersResolver.Reset(); - //} + [TestCase(true)] + [TestCase(false)] + public void TestScope(bool complete) + { + var umbracoContext = GetUmbracoContext("http://example.com/", setSingleton: true); - //private void OnPublishedAssert(IContentService sender, PublishEventArgs args) - //{ - // _onPublishedAssertAction?.Invoke(); - //} + // sanity checks + Assert.AreSame(umbracoContext, Umbraco.Web.Composing.Current.UmbracoContext); + Assert.AreSame(XmlStore, ((PublishedContentCache) umbracoContext.ContentCache).XmlStore); - //private Action _onPublishedAssertAction; + // settings + var settings = SettingsForTests.GenerateMockSettings(); + var contentMock = Mock.Get(settings.Content); + contentMock.Setup(x => x.XmlCacheEnabled).Returns(false); + SettingsForTests.ConfigureSettings(settings); - //[TestCase(true)] - //[TestCase(false)] - //public void TestScope(bool complete) - //{ - // content.TestingUpdateSitemapProvider = false; + // create document type, document + var contentType = new ContentType(-1) { Alias = "CustomDocument", Name = "Custom Document" }; + Current.Services.ContentTypeService.Save(contentType); + var item = new Content("name", -1, contentType); - // var httpContextItems = new Hashtable(); - // content.HttpContextItemsGetter = () => httpContextItems; + // wire cache refresher + _cacheRefresher = new CacheRefresherComponent(true); + _cacheRefresher.Initialize(); - // var settings = SettingsForTests.GenerateMockSettings(); - // var contentMock = Mock.Get(settings.Content); - // contentMock.Setup(x => x.XmlCacheEnabled).Returns(false); - // SettingsForTests.ConfigureSettings(settings); + // check xml in context = "before" + var xml = XmlInContext; + var beforeXml = xml; + var beforeOuterXml = beforeXml.OuterXml; + Console.WriteLine("Xml Before:"); + Console.WriteLine(xml.OuterXml); - // var contentType = new ContentType(-1) { Alias = "contenttype", Name = "test"}; - // Current.Services.ContentTypeService.Save(contentType); + // event handler + var evented = 0; + _onPublishedAssertAction = () => + { + evented++; - // _cacheRefresher = new CacheRefresherComponent(true); - // _cacheRefresher.Initialize(); + // should see the changes in context, not in master + xml = XmlInContext; + Assert.AreNotSame(beforeXml, xml); + Console.WriteLine("Xml Event:"); + Console.WriteLine(xml.OuterXml); + var node = xml.GetElementById(item.Id.ToString()); + Assert.IsNotNull(node); - // var xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Assert.AreSame(xml, httpContextItems[content.XmlContextContentItemKey]); - // var beforeXml = xml; - // var beforeOuterXml = beforeXml.OuterXml; - // var item = new Content("name", -1, contentType); + xml = XmlMaster; + Assert.AreSame(beforeXml, xml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); + }; - // Console.WriteLine("Xml Before:"); - // Console.WriteLine(xml.OuterXml); + ContentService.Published += OnPublishedAssert; - // var evented = 0; - // _onPublishedAssertAction = () => - // { - // evented++; - // xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Assert.AreNotSame(beforeXml, xml); - // Console.WriteLine("Xml Event:"); - // Console.WriteLine(xml.OuterXml); - // var node = xml.GetElementById(item.Id.ToString()); - // Assert.IsNotNull(node); - // }; + using (var scope = ScopeProvider.CreateScope()) + { + Current.Services.ContentService.SaveAndPublishWithStatus(item); // should create an xml clone + item.Name = "changed"; + Current.Services.ContentService.SaveAndPublishWithStatus(item); // should re-use the xml clone - // ContentService.Published += OnPublishedAssert; + // this should never change + Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); - // using (var scope = ScopeProvider.CreateScope()) - // { - // Current.Services.ContentService.SaveAndPublishWithStatus(item); // should create an xml clone - // item.Name = "changed"; - // Current.Services.ContentService.SaveAndPublishWithStatus(item); // should re-use the xml clone + // this does not change, other thread don't see the changes + xml = XmlMaster; + Assert.AreSame(beforeXml, xml); + Console.WriteLine("XmlInternal During:"); + Console.WriteLine(xml.OuterXml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); - // // this should never change - // Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); + // this does not change during the scope (only in events) + // because it is the events that trigger the changes + xml = XmlInContext; + Assert.IsNotNull(xml); + Assert.AreSame(beforeXml, xml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); - // // this does not change, other thread don't see the changes - // xml = content.Instance.XmlContentInternal; - // Assert.IsNotNull(xml); - // Assert.AreSame(beforeXml, xml); - // Console.WriteLine("XmlInternal During:"); - // Console.WriteLine(xml.OuterXml); - // Assert.AreEqual(beforeOuterXml, xml.OuterXml); + // note + // this means that, as long as ppl don't create scopes, they'll see their + // changes right after SaveAndPublishWithStatus, but if they create scopes, + // they will have to wail until the scope is completed, ie wait until the + // events trigger, to use eg GetUrl etc - // // this does not change during the scope (only in events) - // // because it is the events that trigger the changes - // xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Assert.AreSame(beforeXml, xml); - - // // note - // // this means that, as long as ppl don't create scopes, they'll see their - // // changes right after SaveAndPublishWithStatus, but if they create scopes, - // // they will have to wail until the scope is completed, ie wait until the - // // events trigger, to use eg GetUrl etc - - // if (complete) - // scope.Complete(); - // } + if (complete) + scope.Complete(); + } //The reason why there is only 1 event occuring is because we are publishing twice for the same event for the same - //object and the scope deduplicates the events (uses the latest) - // Assert.AreEqual(complete ? 1 : 0, evented); + //object and the scope deduplicates the events(uses the latest) + Assert.AreEqual(complete? 1 : 0, evented); - // // this should never change - // Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); + // this should never change + Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); - // xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Console.WriteLine("Xml After:"); - // Console.WriteLine(xml.OuterXml); - // if (complete) - // { - // var node = xml.GetElementById(item.Id.ToString()); - // Assert.IsNotNull(node); - // } - // else - // { - // Assert.AreSame(beforeXml, xml); - // Assert.AreEqual(beforeOuterXml, xml.OuterXml); // nothing has changed! - // } - //} + xml = XmlInContext; + Console.WriteLine("Xml After:"); + Console.WriteLine(xml.OuterXml); + if (complete) + { + var node = xml.GetElementById(item.Id.ToString()); + Assert.IsNotNull(node); + } + else + { + Assert.AreSame(beforeXml, xml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); // nothing has changed! + } - //[TestCase(true)] - //[TestCase(false)] - //public void TestScopeMany(bool complete) - //{ - // content.TestingUpdateSitemapProvider = false; + xml = XmlMaster; + if (complete) + { + var node = xml.GetElementById(item.Id.ToString()); + Assert.IsNotNull(node); + } + else + { + Assert.AreSame(beforeXml, xml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); // nothing has changed! + } + } - // var settings = SettingsForTests.GenerateMockSettings(); - // var contentMock = Mock.Get(settings.Content); - // contentMock.Setup(x => x.XmlCacheEnabled).Returns(false); - // SettingsForTests.ConfigureSettings(settings); + [TestCase(true)] + [TestCase(false)] + public void TestScopeMany(bool complete) + { + var umbracoContext = GetUmbracoContext("http://example.com/", setSingleton: true); - // var contentType = new ContentType(-1) { Alias = "contenttype", Name = "test" }; - // Current.Services.ContentTypeService.Save(contentType); + // sanity checks + Assert.AreSame(umbracoContext, Umbraco.Web.Composing.Current.UmbracoContext); + Assert.AreSame(XmlStore, ((PublishedContentCache)umbracoContext.ContentCache).XmlStore); - // _cacheRefresher = new CacheRefresherComponent(true); - // _cacheRefresher.Initialize(); + // settings + var settings = SettingsForTests.GenerateMockSettings(); + var contentMock = Mock.Get(settings.Content); + contentMock.Setup(x => x.XmlCacheEnabled).Returns(false); + SettingsForTests.ConfigureSettings(settings); - // var xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // var beforeXml = xml; - // var beforeOuterXml = beforeXml.OuterXml; - // var item = new Content("name", -1, contentType); - // const int count = 10; - // var ids = new int[count]; - // var clones = 0; + // create document type + var contentType = new ContentType(-1) { Alias = "CustomDocument", Name = "Custom Document" }; + Current.Services.ContentTypeService.Save(contentType); - // SafeXmlReaderWriter.Cloning = () => { clones++; }; + // wire cache refresher + _cacheRefresher = new CacheRefresherComponent(true); + _cacheRefresher.Initialize(); - // Console.WriteLine("Xml Before:"); - // Console.WriteLine(xml.OuterXml); + // check xml in context = "before" + var xml = XmlInContext; + var beforeXml = xml; + var beforeOuterXml = beforeXml.OuterXml; + var item = new Content("name", -1, contentType); + const int count = 10; + var ids = new int[count]; + var clones = 0; - // using (var scope = ScopeProvider.CreateScope()) - // { - // Current.Services.ContentService.SaveAndPublishWithStatus(item); + SafeXmlReaderWriter.Cloning = () => { clones++; }; - // for (var i = 0; i < count; i++) - // { - // var temp = new Content("content_" + i, -1, contentType); - // Current.Services.ContentService.SaveAndPublishWithStatus(temp); - // ids[i] = temp.Id; - // } + Console.WriteLine("Xml Before:"); + Console.WriteLine(xml.OuterXml); - // // this should never change - // Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); + using (var scope = ScopeProvider.CreateScope()) + { + Current.Services.ContentService.SaveAndPublishWithStatus(item); - // xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Console.WriteLine("Xml InScope (before complete):"); - // Console.WriteLine(xml.OuterXml); - // Assert.AreEqual(beforeOuterXml, xml.OuterXml); + for (var i = 0; i < count; i++) + { + var temp = new Content("content_" + i, -1, contentType); + Current.Services.ContentService.SaveAndPublishWithStatus(temp); + ids[i] = temp.Id; + } - // if (complete) - // scope.Complete(); + // this should never change + Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); - // xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Console.WriteLine("Xml InScope (after complete):"); - // Console.WriteLine(xml.OuterXml); - // Assert.AreEqual(beforeOuterXml, xml.OuterXml); - // } + xml = XmlMaster; + Assert.IsNotNull(xml); + Console.WriteLine("Xml InScope (before complete):"); + Console.WriteLine(xml.OuterXml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); - // var scopeProvider = ScopeProvider; - // Assert.IsNotNull(scopeProvider); - // // ambient scope may be null, or maybe not, depending on whether the code that - // // was called did proper scoped work, or some direct (NoScope) use of the database - // Assert.IsNull(scopeProvider.AmbientContext); + if (complete) + scope.Complete(); - // // limited number of clones! - // Assert.AreEqual(complete ? 1 : 0, clones); + xml = XmlMaster; + Assert.IsNotNull(xml); + Console.WriteLine("Xml InScope (after complete):"); + Console.WriteLine(xml.OuterXml); + Assert.AreEqual(beforeOuterXml, xml.OuterXml); + } - // // this should never change - // Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); + var scopeProvider = ScopeProvider; + Assert.IsNotNull(scopeProvider); + // ambient scope may be null, or maybe not, depending on whether the code that + // was called did proper scoped work, or some direct (NoScope) use of the database + Assert.IsNull(scopeProvider.AmbientContext); - // xml = content.Instance.XmlContent; - // Assert.IsNotNull(xml); - // Console.WriteLine("Xml After:"); - // Console.WriteLine(xml.OuterXml); - // if (complete) - // { - // var node = xml.GetElementById(item.Id.ToString()); - // Assert.IsNotNull(node); + // limited number of clones! + Assert.AreEqual(complete ? 1 : 0, clones); - // for (var i = 0; i < 10; i++) - // { - // node = xml.GetElementById(ids[i].ToString()); - // Assert.IsNotNull(node); - // } - // } - // else - // { - // Assert.AreEqual(beforeOuterXml, xml.OuterXml); // nothing has changed! - // } - //} + // this should never change + Assert.AreEqual(beforeOuterXml, beforeXml.OuterXml); + + xml = XmlMaster; + Assert.IsNotNull(xml); + Console.WriteLine("Xml After:"); + Console.WriteLine(xml.OuterXml); + if (complete) + { + var node = xml.GetElementById(item.Id.ToString()); + Assert.IsNotNull(node); + + for (var i = 0; i < 10; i++) + { + node = xml.GetElementById(ids[i].ToString()); + Assert.IsNotNull(node); + } + } + else + { + Assert.AreEqual(beforeOuterXml, xml.OuterXml); // nothing has changed! + } + } + + public class LocalServerMessenger : ServerMessengerBase + { + public LocalServerMessenger() + : base(false) + { } + + protected override void DeliverRemote(IEnumerable servers, ICacheRefresher refresher, MessageType messageType, IEnumerable ids = null, string json = null) + { + throw new NotImplementedException(); + } + } } } diff --git a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs index 69ebf7158f..15bda286bd 100644 --- a/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs +++ b/src/Umbraco.Tests/TestHelpers/TestWithDatabaseBase.cs @@ -245,7 +245,8 @@ namespace Umbraco.Tests.TestHelpers Current.Logger); // testing=true so XmlStore will not use the file nor the database - var facadeAccessor = new TestFacadeAccessor(); + //var facadeAccessor = new TestFacadeAccessor(); + var facadeAccessor = new UmbracoContextFacadeAccessor(Umbraco.Web.Composing.Current.UmbracoContextAccessor); var service = new FacadeService( Current.Services, (ScopeProvider) Current.ScopeProvider, diff --git a/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs b/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs index 95becdab8f..e86b866f11 100644 --- a/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs +++ b/src/Umbraco.Tests/Web/TemplateUtilitiesTests.cs @@ -26,6 +26,8 @@ namespace Umbraco.Tests.Web [SetUp] public void SetUp() { + Current.Reset(); + // fixme - now UrlProvider depends on EntityService for GetUrl(guid) - this is bad // should not depend on more than IdkMap maybe - fix this! var entityService = new Mock(); diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs index e6771e638c..7233f36645 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedContentCache.cs @@ -498,9 +498,9 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache return previewXml ?? _xml; } - internal void Resync() + internal void Resync(XmlDocument xml) { - _xml = _xmlStore.Xml; // re-capture + _xml = xml; // re-capture // note: we're not resyncing "preview" because that would mean re-building the whole // preview set which is costly, so basically when previewing, there will be no resync. diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs index 2380418c9b..1336fbf129 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlStore.cs @@ -301,7 +301,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache /// public Func GetXmlDocument { get; set; } - private readonly XmlDocument _xmlDocument; // supplied xml document (for tests) + private XmlDocument _xmlDocument; // supplied xml document (for tests) private volatile XmlDocument _xml; // master xml document private readonly AsyncLock _xmlLock = new AsyncLock(); // protects _xml @@ -311,10 +311,18 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache { get { + if (_xml != null) + return _xml; + if (_xmlDocument != null) - return _xmlDocument; + { + _xml = _xmlDocument; + _xmlDocument = null; + return _xml; + } + if (GetXmlDocument != null) - return GetXmlDocument(); + return _xml = GetXmlDocument(); LazyInitializeContent(); ReloadXmlFromFileIfChanged(); @@ -590,24 +598,24 @@ AND (umbracoNode.id=@id)"; // gets a locked safe read access to the main xml private SafeXmlReaderWriter GetSafeXmlReader() { - return SafeXmlReaderWriter.Get(_scopeProvider, _xmlLock, _xmlDocument, - _ => ResyncCurrentFacade(), + return SafeXmlReaderWriter.Get(_scopeProvider, _xmlLock, _xml, + ResyncCurrentFacade, (xml, registerXmlChange) => { SetXmlLocked(xml, registerXmlChange); - ResyncCurrentFacade(); + ResyncCurrentFacade(xml); }, false); } // gets a locked safe write access to the main xml (cloned) private SafeXmlReaderWriter GetSafeXmlWriter() { - return SafeXmlReaderWriter.Get(_scopeProvider, _xmlLock, _xmlDocument, - _ => ResyncCurrentFacade(), + return SafeXmlReaderWriter.Get(_scopeProvider, _xmlLock, _xml, + ResyncCurrentFacade, (xml, registerXmlChange) => { SetXmlLocked(xml, registerXmlChange); - ResyncCurrentFacade(); + ResyncCurrentFacade(xml); }, true); } @@ -1184,10 +1192,7 @@ ORDER BY umbracoNode.level, umbracoNode.sortOrder"; } if (publishedChanged) - { safeXml.AcceptChanges(); - ResyncCurrentFacade(); - } } } @@ -1216,8 +1221,6 @@ ORDER BY umbracoNode.level, umbracoNode.sortOrder"; RefreshContentTypes(ids); // ignore media and member types - we're not caching them - - ResyncCurrentFacade(); } public void Notify(DataTypeCacheRefresher.JsonPayload[] payloads) @@ -1236,15 +1239,13 @@ ORDER BY umbracoNode.level, umbracoNode.sortOrder"; // that's all we need to do as the changes have NO impact whatsoever on the Xml content // ignore media and member types - we're not caching them - - ResyncCurrentFacade(); } - private void ResyncCurrentFacade() + private void ResyncCurrentFacade(XmlDocument xml) { var facade = (Facade) _facadeAccessor.Facade; if (facade == null) return; - ((PublishedContentCache) facade.ContentCache).Resync(); + ((PublishedContentCache) facade.ContentCache).Resync(xml); ((PublishedMediaCache) facade.MediaCache).Resync(); // not trying to resync members or domains, which are not cached really