From 514452ddb9cb954b7d32a5a391024f480dc1591d Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 18 Mar 2019 17:51:04 +0100 Subject: [PATCH 01/24] Better ServerMessenger configuration --- src/Umbraco.Core/CompositionExtensions.cs | 22 +++++ .../BatchedDatabaseServerMessenger.cs | 5 +- ...aseServerRegistrarAndMessengerComponent.cs | 80 ++++++++----------- 3 files changed, 59 insertions(+), 48 deletions(-) diff --git a/src/Umbraco.Core/CompositionExtensions.cs b/src/Umbraco.Core/CompositionExtensions.cs index e1c7ad4467..828a577c34 100644 --- a/src/Umbraco.Core/CompositionExtensions.cs +++ b/src/Umbraco.Core/CompositionExtensions.cs @@ -203,6 +203,28 @@ namespace Umbraco.Core composition.RegisterUnique(_ => registrar); } + /// + /// Sets the database server messenger options. + /// + /// The composition. + /// A function creating the options. + /// Use DatabaseServerRegistrarAndMessengerComposer.GetDefaultOptions to get the options that Umbraco would use by default. + public static void SetDatabaseServerMessengerOptions(this Composition composition, Func factory) + { + composition.RegisterUnique(factory); + } + + /// + /// Sets the database server messenger options. + /// + /// The composition. + /// Options. + /// Use DatabaseServerRegistrarAndMessengerComposer.GetDefaultOptions to get the options that Umbraco would use by default. + public static void SetDatabaseServerMessengerOptions(this Composition composition, DatabaseServerMessengerOptions options) + { + composition.RegisterUnique(_ => options); + } + /// /// Sets the short string helper. /// diff --git a/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs b/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs index 76d7565862..818e8ecf77 100644 --- a/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs +++ b/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs @@ -27,9 +27,8 @@ namespace Umbraco.Web private readonly IUmbracoDatabaseFactory _databaseFactory; public BatchedDatabaseServerMessenger( - IRuntimeState runtime, IUmbracoDatabaseFactory databaseFactory, IScopeProvider scopeProvider, ISqlContext sqlContext, IProfilingLogger proflog, IGlobalSettings globalSettings, - bool enableDistCalls, DatabaseServerMessengerOptions options) - : base(runtime, scopeProvider, sqlContext, proflog, globalSettings, enableDistCalls, options) + IRuntimeState runtime, IUmbracoDatabaseFactory databaseFactory, IScopeProvider scopeProvider, ISqlContext sqlContext, IProfilingLogger proflog, IGlobalSettings globalSettings, DatabaseServerMessengerOptions options) + : base(runtime, scopeProvider, sqlContext, proflog, globalSettings, true, options) { _databaseFactory = databaseFactory; } diff --git a/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs b/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs index 086aa9b197..a68e137665 100644 --- a/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs +++ b/src/Umbraco.Web/Compose/DatabaseServerRegistrarAndMessengerComponent.cs @@ -38,54 +38,44 @@ namespace Umbraco.Web.Compose public sealed class DatabaseServerRegistrarAndMessengerComposer : ComponentComposer, ICoreComposer { + public static DatabaseServerMessengerOptions GetDefaultOptions(IFactory factory) + { + var logger = factory.GetInstance(); + var indexRebuilder = factory.GetInstance(); + + return new DatabaseServerMessengerOptions + { + //These callbacks will be executed if the server has not been synced + // (i.e. it is a new server or the lastsynced.txt file has been removed) + InitializingCallbacks = new Action[] + { + //rebuild the xml cache file if the server is not synced + () => + { + // rebuild the published snapshot caches entirely, if the server is not synced + // this is equivalent to DistributedCache RefreshAll... but local only + // (we really should have a way to reuse RefreshAll... locally) + // note: refresh all content & media caches does refresh content types too + var svc = Current.PublishedSnapshotService; + svc.Notify(new[] { new DomainCacheRefresher.JsonPayload(0, DomainChangeTypes.RefreshAll) }); + svc.Notify(new[] { new ContentCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out _, out _); + svc.Notify(new[] { new MediaCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out _); + }, + + //rebuild indexes if the server is not synced + // NOTE: This will rebuild ALL indexes including the members, if developers want to target specific + // indexes then they can adjust this logic themselves. + () => { ExamineComponent.RebuildIndexes(indexRebuilder, logger, false, 5000); } + } + }; + } + public override void Compose(Composition composition) { base.Compose(composition); - composition.SetServerMessenger(factory => - { - var runtime = factory.GetInstance(); - var databaseFactory = factory.GetInstance(); - var globalSettings = factory.GetInstance(); - var proflog = factory.GetInstance(); - var scopeProvider = factory.GetInstance(); - var sqlContext = factory.GetInstance(); - var logger = factory.GetInstance(); - var indexRebuilder = factory.GetInstance(); - - return new BatchedDatabaseServerMessenger( - runtime, databaseFactory, scopeProvider, sqlContext, proflog, globalSettings, - true, - //Default options for web including the required callbacks to build caches - new DatabaseServerMessengerOptions - { - //These callbacks will be executed if the server has not been synced - // (i.e. it is a new server or the lastsynced.txt file has been removed) - InitializingCallbacks = new Action[] - { - //rebuild the xml cache file if the server is not synced - () => - { - // rebuild the published snapshot caches entirely, if the server is not synced - // this is equivalent to DistributedCache RefreshAll... but local only - // (we really should have a way to reuse RefreshAll... locally) - // note: refresh all content & media caches does refresh content types too - var svc = Current.PublishedSnapshotService; - svc.Notify(new[] { new DomainCacheRefresher.JsonPayload(0, DomainChangeTypes.RefreshAll) }); - svc.Notify(new[] { new ContentCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out _, out _); - svc.Notify(new[] { new MediaCacheRefresher.JsonPayload(0, TreeChangeTypes.RefreshAll) }, out _); - }, - - //rebuild indexes if the server is not synced - // NOTE: This will rebuild ALL indexes including the members, if developers want to target specific - // indexes then they can adjust this logic themselves. - () => - { - ExamineComponent.RebuildIndexes(indexRebuilder, logger, false, 5000); - } - } - }); - }); + composition.SetDatabaseServerMessengerOptions(GetDefaultOptions); + composition.SetServerMessenger(); } } @@ -128,7 +118,7 @@ namespace Umbraco.Web.Compose } public void Initialize() - { + { //We will start the whole process when a successful request is made if (_registrar != null || _messenger != null) UmbracoModule.RouteAttempt += RegisterBackgroundTasksOnce; From 7dcf2b1abb168e98cd1c190b9d7794043fb39901 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 1 Apr 2019 17:05:13 +1100 Subject: [PATCH 02/24] infinity editing dim-layer for umb-modelcolumn --- src/Umbraco.Web.UI.Client/src/less/modals.less | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Umbraco.Web.UI.Client/src/less/modals.less b/src/Umbraco.Web.UI.Client/src/less/modals.less index 771be1bc2a..8e88774613 100644 --- a/src/Umbraco.Web.UI.Client/src/less/modals.less +++ b/src/Umbraco.Web.UI.Client/src/less/modals.less @@ -55,6 +55,16 @@ position: absolute;; } +.--notInFront .umb-modalcolumn::after { + content: ''; + position: absolute; + top: 0; + bottom: 0; + right: 0; + left: 0; + background: rgba(0,0,0,.4); +} + /* re-align loader */ .umb-modal .umb-loader-wrapper, .umb-modalcolumn .umb-loader-wrapper, .umb-dialog .umb-loader-wrapper{ position:relative; From a8265db5f0e4f61adac4fbc27980bed39a5117cc Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Apr 2019 10:45:04 +1100 Subject: [PATCH 03/24] Reload listview after moving items --- .../src/views/propertyeditors/listview/listview.controller.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js index 44c0c4ae7b..e52edd6e92 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js @@ -583,6 +583,8 @@ function listViewController($scope, $routeParams, $injector, $timeout, currentUs .then(function () { //executes if all is successful, let's sync the tree if (newPath) { + // reload the current view so the moved items are no longer shown + $scope.reloadView($scope.contentId); //we need to do a double sync here: first refresh the node where the content was moved, // then refresh the node where the content was moved from From eb529783c46475b720309291efb438ecb35e224a Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 3 Apr 2019 16:22:13 +1100 Subject: [PATCH 04/24] Don't allow move in members listview --- .../src/views/propertyeditors/listview/listview.controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js index e52edd6e92..8e233646bf 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/listview/listview.controller.js @@ -159,7 +159,7 @@ function listViewController($scope, $routeParams, $injector, $timeout, currentUs allowBulkPublish: $scope.entityType === 'content' && $scope.model.config.bulkActionPermissions.allowBulkPublish, allowBulkUnpublish: $scope.entityType === 'content' && $scope.model.config.bulkActionPermissions.allowBulkUnpublish, allowBulkCopy: $scope.entityType === 'content' && $scope.model.config.bulkActionPermissions.allowBulkCopy, - allowBulkMove: $scope.model.config.bulkActionPermissions.allowBulkMove, + allowBulkMove: $scope.entityType !== 'member' && $scope.model.config.bulkActionPermissions.allowBulkMove, allowBulkDelete: $scope.model.config.bulkActionPermissions.allowBulkDelete, cultureName: $routeParams.cculture ? $routeParams.cculture : $routeParams.mculture }; From 63fb8f59330c5a34f62eb871c512acb24af2def5 Mon Sep 17 00:00:00 2001 From: Andrew Deans Date: Tue, 2 Apr 2019 23:00:08 -0400 Subject: [PATCH 05/24] Fix case of query parameter --- .../Repositories/Implement/ExternalLoginRepository.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs index 7cb78f4c9f..0fa48e5521 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ExternalLoginRepository.cs @@ -46,7 +46,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override IIdentityUserLogin PerformGet(int id) { var sql = GetBaseQuery(false); - sql.Where(GetBaseWhereClause(), new { Id = id }); + sql.Where(GetBaseWhereClause(), new { id = id }); var dto = Database.Fetch(SqlSyntax.SelectTop(sql, 1)).FirstOrDefault(); if (dto == null) From f985c437b5a55c614490eebf8323411b87c52558 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 29 Mar 2019 08:26:36 +0100 Subject: [PATCH 06/24] Add content.SetValue overload for posted files --- src/Umbraco.Core/ContentExtensions.cs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index f802c2d0d9..a7d40b0b7d 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -7,6 +7,7 @@ using System.Web; using System.Xml.Linq; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using NPoco.Expressions; using Umbraco.Core.Composing; using Umbraco.Core.IO; using Umbraco.Core.Models; @@ -52,8 +53,8 @@ namespace Umbraco.Core return ContentStatus.Unpublished; } - - + + #endregion /// @@ -134,9 +135,14 @@ namespace Umbraco.Core /// /// Sets the posted file value of a property. /// - /// This really is for FileUpload fields only, and should be obsoleted. For anything else, - /// you need to store the file by yourself using Store and then figure out - /// how to deal with auto-fill properties (if any) and thumbnails (if any) by yourself. + public static void SetValue(this IContentBase content, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, string propertyTypeAlias, string filename, HttpPostedFileBase postedFile, string culture = null, string segment = null) + { + content.SetValue(contentTypeBaseServiceProvider, propertyTypeAlias, postedFile.FileName, postedFile.InputStream, culture, segment); + } + + /// + /// Sets the posted file value of a property. + /// public static void SetValue(this IContentBase content, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, string propertyTypeAlias, string filename, Stream filestream, string culture = null, string segment = null) { if (filename == null || filestream == null) return; From b5796ad23763f4a333c90dce8a9348f6baf5214d Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Apr 2019 10:44:36 +1100 Subject: [PATCH 07/24] Enable configuration of NuCache BTree block size #5114 --- .../NuCache/DataSource/BTree.cs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.cs b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.cs index 1b17e0c124..910c0ca737 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/DataSource/BTree.cs @@ -1,4 +1,5 @@ -using CSharpTest.Net.Collections; +using System.Configuration; +using CSharpTest.Net.Collections; using CSharpTest.Net.Serialization; namespace Umbraco.Web.PublishedCache.NuCache.DataSource @@ -14,6 +15,12 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource CreateFile = exists ? CreatePolicy.IfNeeded : CreatePolicy.Always, FileName = filepath, + // read or write but do *not* keep in memory + CachePolicy = CachePolicy.None, + + // default is 4096, min 2^9 = 512, max 2^16 = 64K + FileBlockSize = GetBlockSize(), + // other options? }; @@ -25,6 +32,28 @@ namespace Umbraco.Web.PublishedCache.NuCache.DataSource return tree; } + private static int GetBlockSize() + { + var blockSize = 4096; + + var appSetting = ConfigurationManager.AppSettings["Umbraco.Web.PublishedCache.NuCache.BTree.BlockSize"]; + if (appSetting == null) + return blockSize; + + if (!int.TryParse(appSetting, out blockSize)) + throw new ConfigurationErrorsException($"Invalid block size value \"{appSetting}\": not a number."); + + var bit = 0; + for (var i = blockSize; i != 1; i >>= 1) + bit++; + if (1 << bit != blockSize) + throw new ConfigurationErrorsException($"Invalid block size value \"{blockSize}\": must be a power of two."); + if (blockSize < 512 || blockSize > 65536) + throw new ConfigurationErrorsException($"Invalid block size value \"{blockSize}\": must be >= 512 and <= 65536."); + + return blockSize; + } + /* class ListOfIntSerializer : ISerializer> { From e38095b72796221741e8d0d1c6269bfe216d66b3 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 5 Apr 2019 10:49:19 +1100 Subject: [PATCH 08/24] NuCache: fix loading the media cache from local files, was missing media --- src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs | 2 ++ .../PublishedCache/NuCache/PublishedSnapshotService.cs | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 7ab4a64f31..331ec37248 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -503,6 +503,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } } + // IMPORTANT kits must be sorted out by LEVEL public void SetAll(IEnumerable kits) { var lockInfo = new WriteLockInfo(); @@ -533,6 +534,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } } + // IMPORTANT kits must be sorted out by LEVEL public void SetBranch(int rootContentId, IEnumerable kits) { var lockInfo = new WriteLockInfo(); diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 9c5587fbd5..7fb51463bf 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -356,6 +356,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Loading content from database..."); var sw = Stopwatch.StartNew(); + // IMPORTANT GetAllContentSources sorts kits by level var kits = _dataSource.GetAllContentSources(scope); _contentStore.SetAll(kits); sw.Stop(); @@ -370,7 +371,8 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Loading content from local db..."); var sw = Stopwatch.StartNew(); - var kits = _localContentDb.Select(x => x.Value).OrderBy(x => x.Node.Level); + var kits = _localContentDb.Select(x => x.Value) + .OrderBy(x => x.Node.Level); // IMPORTANT sort by level _contentStore.SetAll(kits); sw.Stop(); _logger.Debug("Loaded content from local db ({Duration}ms)", sw.ElapsedMilliseconds); @@ -422,6 +424,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Loading media from database..."); var sw = Stopwatch.StartNew(); + // IMPORTANT GetAllMediaSources sorts kits by level var kits = _dataSource.GetAllMediaSources(scope); _mediaStore.SetAll(kits); sw.Stop(); @@ -436,7 +439,8 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Loading media from local db..."); var sw = Stopwatch.StartNew(); - var kits = _localMediaDb.Select(x => x.Value); + var kits = _localMediaDb.Select(x => x.Value) + .OrderBy(x => x.Node.Level); // IMPORTANT sort by level _mediaStore.SetAll(kits); sw.Stop(); _logger.Debug("Loaded media from local db ({Duration}ms)", sw.ElapsedMilliseconds); @@ -647,6 +651,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (capture.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch)) { // ?? should we do some RV check here? + // IMPORTANT GetbranchContentSources sorts kits by level var kits = _dataSource.GetBranchContentSources(scope, capture.Id); _contentStore.SetBranch(capture.Id, kits); } @@ -738,6 +743,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (capture.ChangeTypes.HasType(TreeChangeTypes.RefreshBranch)) { // ?? should we do some RV check here? + // IMPORTANT GetbranchContentSources sorts kits by level var kits = _dataSource.GetBranchMediaSources(scope, capture.Id); _mediaStore.SetBranch(capture.Id, kits); } From 1c9b449c46d3327f7be8d128cb2026ba2cb9e494 Mon Sep 17 00:00:00 2001 From: Stephan Date: Sun, 7 Apr 2019 16:24:35 +0200 Subject: [PATCH 09/24] Log more details about boot environment --- src/Umbraco.Core/Runtime/CoreRuntime.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Runtime/CoreRuntime.cs b/src/Umbraco.Core/Runtime/CoreRuntime.cs index f00365496a..9c33f1fcfe 100644 --- a/src/Umbraco.Core/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Core/Runtime/CoreRuntime.cs @@ -5,6 +5,7 @@ using System.Linq; using System.Reflection; using System.Threading; using System.Web; +using System.Web.Hosting; using Umbraco.Core.Cache; using Umbraco.Core.Composing; using Umbraco.Core.Configuration; @@ -70,10 +71,15 @@ namespace Umbraco.Core.Runtime // objects. using (var timer = profilingLogger.TraceDuration( - $"Booting Umbraco {UmbracoVersion.SemanticVersion.ToSemanticString()} on {NetworkHelper.MachineName}.", + $"Booting Umbraco {UmbracoVersion.SemanticVersion.ToSemanticString()}.", "Booted.", "Boot failed.")) { + logger.Info("Booting site '{HostingSiteName}', app '{HostingApplicationID}', path '{HostingPhysicalPath}', server '{MachineName}'.", + HostingEnvironment.SiteName, + HostingEnvironment.ApplicationID, + HostingEnvironment.ApplicationPhysicalPath, + NetworkHelper.MachineName); logger.Debug("Runtime: {Runtime}", GetType().FullName); // application environment From 7e1ee26dee591d1ea9faf6166e8cbdab694c2cad Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 9 Apr 2019 10:34:59 +1000 Subject: [PATCH 10/24] Fix spelling mistake --- src/Umbraco.Web.UI.Client/src/views/packages/views/repo.html | 2 +- src/Umbraco.Web.UI/Umbraco/config/lang/en.xml | 2 +- src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/packages/views/repo.html b/src/Umbraco.Web.UI.Client/src/views/packages/views/repo.html index 93c2ce05ed..4767f6fdcc 100644 --- a/src/Umbraco.Web.UI.Client/src/views/packages/views/repo.html +++ b/src/Umbraco.Web.UI.Client/src/views/packages/views/repo.html @@ -250,7 +250,7 @@
Compatibility
-
This package is compatible with the following versions of Umbraco, as reported by community members. Full compatability cannot be gauranteed for versions reported below 100%
+
This package is compatible with the following versions of Umbraco, as reported by community members. Full compatability cannot be guaranteed for versions reported below 100%
{{compatibility.version}} diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml index 1840883c34..5e0d959b61 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en.xml @@ -1164,7 +1164,7 @@ To manage your website, simply open the Umbraco back office and start adding con Downloads Likes Compatibility - This package is compatible with the following versions of Umbraco, as reported by community members. Full compatability cannot be gauranteed for versions reported below 100% + This package is compatible with the following versions of Umbraco, as reported by community members. Full compatability cannot be guaranteed for versions reported below 100% External sources Author Documentation diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index 8700daff03..92780b304e 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -1163,7 +1163,7 @@ To manage your website, simply open the Umbraco back office and start adding con Downloads Likes Compatibility - This package is compatible with the following versions of Umbraco, as reported by community members. Full compatability cannot be gauranteed for versions reported below 100% + This package is compatible with the following versions of Umbraco, as reported by community members. Full compatability cannot be guaranteed for versions reported below 100% External sources Author Documentation From be7f42c454adf437692071c6bb990ed882b08bb9 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 10 Apr 2019 09:52:06 +1000 Subject: [PATCH 11/24] Don't show the recycle bin in tree pickers/dialogs --- src/Umbraco.Web/Trees/TreeControllerBase.cs | 2 +- src/Umbraco.Web/Trees/TreeQueryStringParameters.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/Trees/TreeControllerBase.cs b/src/Umbraco.Web/Trees/TreeControllerBase.cs index 4e43e6c093..a815364982 100644 --- a/src/Umbraco.Web/Trees/TreeControllerBase.cs +++ b/src/Umbraco.Web/Trees/TreeControllerBase.cs @@ -366,7 +366,7 @@ namespace Umbraco.Web.Trees /// protected bool IsDialog(FormDataCollection queryStrings) { - return queryStrings.GetValue(TreeQueryStringParameters.IsDialog); + return queryStrings.GetValue(TreeQueryStringParameters.Use) == "dialog"; } /// diff --git a/src/Umbraco.Web/Trees/TreeQueryStringParameters.cs b/src/Umbraco.Web/Trees/TreeQueryStringParameters.cs index 205871ca20..466aff5a1f 100644 --- a/src/Umbraco.Web/Trees/TreeQueryStringParameters.cs +++ b/src/Umbraco.Web/Trees/TreeQueryStringParameters.cs @@ -5,7 +5,7 @@ /// internal struct TreeQueryStringParameters { - public const string IsDialog = "isDialog"; + public const string Use = "use"; public const string Application = "application"; public const string StartNodeId = "startNodeId"; //public const string OnNodeClick = "OnNodeClick"; From 179afe7e0d1e716db47ec3b50471c7206eb967b1 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Thu, 21 Mar 2019 15:01:46 +0100 Subject: [PATCH 12/24] Add IsVisible extension on IPublishedElement --- src/Umbraco.Web/PublishedElementExtensions.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/Umbraco.Web/PublishedElementExtensions.cs b/src/Umbraco.Web/PublishedElementExtensions.cs index f2a49f7f60..de7c72d21a 100644 --- a/src/Umbraco.Web/PublishedElementExtensions.cs +++ b/src/Umbraco.Web/PublishedElementExtensions.cs @@ -170,5 +170,23 @@ namespace Umbraco.Web } #endregion + + #region IsSomething + + /// + /// Gets a value indicating whether the content is visible. + /// + /// The content. + /// A value indicating whether the content is visible. + /// A content is not visible if it has an umbracoNaviHide property with a value of "1". Otherwise, + /// the content is visible. + public static bool IsVisible(this IPublishedElement content) + { + // rely on the property converter - will return default bool value, ie false, if property + // is not defined, or has no value, else will return its value. + return content.Value(Constants.Conventions.Content.NaviHide) == false; + } + + #endregion } } From 576c10cd50029577ddbe2683c6ef05fd029f6763 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 15 Apr 2019 12:56:45 +1000 Subject: [PATCH 13/24] NuCache: fix vanishing content when refreshing content types --- .../PublishedCache/NuCache/ContentStore.cs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 331ec37248..48c68ab9bf 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -313,6 +313,8 @@ namespace Umbraco.Web.PublishedCache.NuCache var removedContentTypeNodes = new List(); var refreshedContentTypeNodes = new List(); + // find all the nodes that are either refreshed or removed, + // because of their content type being either refreshed or removed foreach (var link in _contentNodes.Values) { var node = link.Value; @@ -322,39 +324,49 @@ namespace Umbraco.Web.PublishedCache.NuCache if (refreshedIdsA.Contains(contentTypeId)) refreshedContentTypeNodes.Add(node.Id); } - // all content should have been deleted - but + // perform deletion of content with removed content type + // removing content types should have removed their content already + // but just to be 100% sure, clear again here foreach (var node in removedContentTypeNodes) ClearBranchLocked(node); + // perform deletion of removed content types foreach (var id in removedIdsA) { - if (_contentTypesById.TryGetValue(id, out LinkedNode link) == false || link.Value == null) + if (_contentTypesById.TryGetValue(id, out var link) == false || link.Value == null) continue; SetValueLocked(_contentTypesById, id, null); SetValueLocked(_contentTypesByAlias, link.Value.Alias, null); } + // perform update of refreshed content types foreach (var type in refreshedTypesA) { SetValueLocked(_contentTypesById, type.Id, type); SetValueLocked(_contentTypesByAlias, type.Alias, type); } - // skip missing type - // skip missing parents & unbuildable kits - what else could we do? + // perform update of content with refreshed content type - from the kits + // skip missing type, skip missing parents & unbuildable kits - what else could we do? + // kits are ordered by level, so ParentExits is ok here var visited = new List(); foreach (var kit in kits.Where(x => refreshedIdsA.Contains(x.ContentTypeId) && ParentExistsLocked(x) && BuildKit(x))) { + // replacing the node: must preserve the parents + var node = GetHead(_contentNodes, kit.Node.Id)?.Value; + if (node != null) + kit.Node.ChildContentIds = node.ChildContentIds; + SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); + visited.Add(kit.Node.Id); if (_localDb != null) RegisterChange(kit.Node.Id, kit); } // all content should have been refreshed - but... - var orphans = refreshedContentTypeNodes.Except(visited); foreach (var id in orphans) ClearBranchLocked(id); From fa655b812ce958fb25a283fcf27534cbd1dbfec6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 15 Apr 2019 17:55:22 +1000 Subject: [PATCH 14/24] Fix composers ordering --- src/Umbraco.Core/Composing/Composers.cs | 133 +++++++++++++----- .../Components/ComponentTests.cs | 28 ++++ src/Umbraco.Web/Runtime/WebFinalComposer.cs | 2 + 3 files changed, 125 insertions(+), 38 deletions(-) diff --git a/src/Umbraco.Core/Composing/Composers.cs b/src/Umbraco.Core/Composing/Composers.cs index 14cb0dce8e..0510740e42 100644 --- a/src/Umbraco.Core/Composing/Composers.cs +++ b/src/Umbraco.Core/Composing/Composers.cs @@ -70,7 +70,23 @@ namespace Umbraco.Core.Composing } } - private IEnumerable PrepareComposerTypes() + internal IEnumerable PrepareComposerTypes() + { + var requirements = GetRequirements(); + + // only for debugging, this is verbose + //_logger.Debug(GetComposersReport(requirements)); + + var sortedComposerTypes = SortComposers(requirements); + + // bit verbose but should help for troubleshooting + //var text = "Ordered Composers: " + Environment.NewLine + string.Join(Environment.NewLine, sortedComposerTypes) + Environment.NewLine; + _logger.Debug("Ordered Composers: {SortedComposerTypes}", sortedComposerTypes); + + return sortedComposerTypes; + } + + internal Dictionary> GetRequirements(bool throwOnMissing = true) { // create a list, remove those that cannot be enabled due to runtime level var composerTypeList = _composerTypes @@ -89,25 +105,69 @@ namespace Umbraco.Core.Composing // enable or disable composers EnableDisableComposers(composerTypeList); - // sort the composers according to their dependencies - var requirements = new Dictionary>(); - foreach (var type in composerTypeList) requirements[type] = null; - foreach (var type in composerTypeList) + void GatherInterfaces(Type type, Func getTypeInAttribute, HashSet iset, List set2) + where TAttribute : Attribute { - GatherRequirementsFromRequireAttribute(type, composerTypeList, requirements); - GatherRequirementsFromRequiredByAttribute(type, composerTypeList, requirements); + foreach (var attribute in type.GetCustomAttributes()) + { + var typeInAttribute = getTypeInAttribute(attribute); + if (typeInAttribute != null && // if the attribute references a type ... + typeInAttribute.IsInterface && // ... which is an interface ... + typeof(IComposer).IsAssignableFrom(typeInAttribute) && // ... which implements IComposer ... + !iset.Contains(typeInAttribute)) // ... which is not already in the list + { + // add it to the new list + iset.Add(typeInAttribute); + set2.Add(typeInAttribute); + + // add all its interfaces implementing IComposer + foreach (var i in typeInAttribute.GetInterfaces().Where(x => typeof(IComposer).IsAssignableFrom(x))) + { + iset.Add(i); + set2.Add(i); + } + } + } } - // only for debugging, this is verbose - //_logger.Debug(GetComposersReport(requirements)); + // gather interfaces too + var interfaces = new HashSet(composerTypeList.SelectMany(x => x.GetInterfaces().Where(y => typeof(IComposer).IsAssignableFrom(y)))); + composerTypeList.AddRange(interfaces); + var list1 = composerTypeList; + while (list1.Count > 0) + { + var list2 = new List(); + foreach (var t in list1) + { + GatherInterfaces(t, a => a.RequiredType, interfaces, list2); + GatherInterfaces(t, a => a.RequiringType, interfaces, list2); + } + composerTypeList.AddRange(list2); + list1 = list2; + } + // sort the composers according to their dependencies + var requirements = new Dictionary>(); + foreach (var type in composerTypeList) + requirements[type] = null; + foreach (var type in composerTypeList) + { + GatherRequirementsFromAfterAttribute(type, composerTypeList, requirements, throwOnMissing); + GatherRequirementsFromBeforeAttribute(type, composerTypeList, requirements); + } + + return requirements; + } + + internal IEnumerable SortComposers(Dictionary> requirements) + { // sort composers var graph = new TopoGraph>>(kvp => kvp.Key, kvp => kvp.Value); graph.AddItems(requirements); List sortedComposerTypes; try { - sortedComposerTypes = graph.GetSortedItems().Select(x => x.Key).ToList(); + sortedComposerTypes = graph.GetSortedItems().Select(x => x.Key).Where(x => !x.IsInterface).ToList(); } catch (Exception e) { @@ -117,40 +177,37 @@ namespace Umbraco.Core.Composing throw; } - // bit verbose but should help for troubleshooting - //var text = "Ordered Composers: " + Environment.NewLine + string.Join(Environment.NewLine, sortedComposerTypes) + Environment.NewLine; - _logger.Debug("Ordered Composers: {SortedComposerTypes}", sortedComposerTypes); - return sortedComposerTypes; } - private static string GetComposersReport(Dictionary> requirements) + internal static string GetComposersReport(Dictionary> requirements) { var text = new StringBuilder(); text.AppendLine("Composers & Dependencies:"); + text.AppendLine(" < compose before"); + text.AppendLine(" > compose after"); + text.AppendLine(" : implements"); + text.AppendLine(" = depends"); text.AppendLine(); + bool HasReq(IEnumerable types, Type type) + => types.Any(x => type.IsAssignableFrom(x) && !x.IsInterface); + foreach (var kvp in requirements) { var type = kvp.Key; text.AppendLine(type.FullName); foreach (var attribute in type.GetCustomAttributes()) - text.AppendLine(" -> " + attribute.RequiredType + (attribute.Weak.HasValue - ? (attribute.Weak.Value ? " (weak)" : (" (strong" + (requirements.ContainsKey(attribute.RequiredType) ? ", missing" : "") + ")")) - : "")); - foreach (var attribute in type.GetCustomAttributes()) - text.AppendLine(" -< " + attribute.RequiringType); - foreach (var i in type.GetInterfaces()) { - text.AppendLine(" : " + i.FullName); - foreach (var attribute in i.GetCustomAttributes()) - text.AppendLine(" -> " + attribute.RequiredType + (attribute.Weak.HasValue - ? (attribute.Weak.Value ? " (weak)" : (" (strong" + (requirements.ContainsKey(attribute.RequiredType) ? ", missing" : "") + ")")) - : "")); - foreach (var attribute in i.GetCustomAttributes()) - text.AppendLine(" -< " + attribute.RequiringType); + var weak = !(attribute.RequiredType.IsInterface ? attribute.Weak == false : attribute.Weak != true); + text.AppendLine(" > " + attribute.RequiredType + + (weak ? " (weak" : " (strong") + (HasReq(requirements.Keys, attribute.RequiredType) ? ", found" : ", missing") + ")"); } + foreach (var attribute in type.GetCustomAttributes()) + text.AppendLine(" < " + attribute.RequiringType); + foreach (var i in type.GetInterfaces()) + text.AppendLine(" : " + i.FullName); if (kvp.Value != null) foreach (var t in kvp.Value) text.AppendLine(" = " + t); @@ -221,16 +278,16 @@ namespace Umbraco.Core.Composing types.Remove(kvp.Key); } - private static void GatherRequirementsFromRequireAttribute(Type type, ICollection types, IDictionary> requirements) + private static void GatherRequirementsFromAfterAttribute(Type type, ICollection types, IDictionary> requirements, bool throwOnMissing = true) { // get 'require' attributes // these attributes are *not* inherited because we want to "custom-inherit" for interfaces only - var requireAttributes = type + var afterAttributes = type .GetInterfaces().SelectMany(x => x.GetCustomAttributes()) // those marking interfaces .Concat(type.GetCustomAttributes()); // those marking the composer // what happens in case of conflicting attributes (different strong/weak for same type) is not specified. - foreach (var attr in requireAttributes) + foreach (var attr in afterAttributes) { if (attr.RequiredType == type) continue; // ignore self-requirements (+ exclude in implems, below) @@ -238,13 +295,13 @@ namespace Umbraco.Core.Composing // unless strong, and then require at least one enabled composer implementing that interface if (attr.RequiredType.IsInterface) { - var implems = types.Where(x => x != type && attr.RequiredType.IsAssignableFrom(x)).ToList(); + var implems = types.Where(x => x != type && attr.RequiredType.IsAssignableFrom(x) && !x.IsInterface).ToList(); if (implems.Count > 0) { if (requirements[type] == null) requirements[type] = new List(); requirements[type].AddRange(implems); } - else if (attr.Weak == false) // if explicitly set to !weak, is strong, else is weak + else if (attr.Weak == false && throwOnMissing) // if explicitly set to !weak, is strong, else is weak throw new Exception($"Broken composer dependency: {type.FullName} -> {attr.RequiredType.FullName}."); } // requiring a class = require that the composer is enabled @@ -256,28 +313,28 @@ namespace Umbraco.Core.Composing if (requirements[type] == null) requirements[type] = new List(); requirements[type].Add(attr.RequiredType); } - else if (attr.Weak != true) // if not explicitly set to weak, is strong + else if (attr.Weak != true && throwOnMissing) // if not explicitly set to weak, is strong throw new Exception($"Broken composer dependency: {type.FullName} -> {attr.RequiredType.FullName}."); } } } - private static void GatherRequirementsFromRequiredByAttribute(Type type, ICollection types, IDictionary> requirements) + private static void GatherRequirementsFromBeforeAttribute(Type type, ICollection types, IDictionary> requirements) { // get 'required' attributes // these attributes are *not* inherited because we want to "custom-inherit" for interfaces only - var requiredAttributes = type + var beforeAttributes = type .GetInterfaces().SelectMany(x => x.GetCustomAttributes()) // those marking interfaces .Concat(type.GetCustomAttributes()); // those marking the composer - foreach (var attr in requiredAttributes) + foreach (var attr in beforeAttributes) { if (attr.RequiringType == type) continue; // ignore self-requirements (+ exclude in implems, below) // required by an interface = by any enabled composer implementing this that interface if (attr.RequiringType.IsInterface) { - var implems = types.Where(x => x != type && attr.RequiringType.IsAssignableFrom(x)).ToList(); + var implems = types.Where(x => x != type && attr.RequiringType.IsAssignableFrom(x) && !x.IsInterface).ToList(); foreach (var implem in implems) { if (requirements[implem] == null) requirements[implem] = new List(); diff --git a/src/Umbraco.Tests/Components/ComponentTests.cs b/src/Umbraco.Tests/Components/ComponentTests.cs index c026e5a157..2ba94d8c78 100644 --- a/src/Umbraco.Tests/Components/ComponentTests.cs +++ b/src/Umbraco.Tests/Components/ComponentTests.cs @@ -4,6 +4,7 @@ using System.Linq; using Moq; using NUnit.Framework; using Umbraco.Core; +using Umbraco.Core.Cache; using Umbraco.Core.Compose; using Umbraco.Core.Composing; using Umbraco.Core.IO; @@ -299,11 +300,19 @@ namespace Umbraco.Tests.Components composers = new Composers(composition, types, Mock.Of()); Composed.Clear(); Assert.Throws(() => composers.Compose()); + Console.WriteLine("throws:"); + composers = new Composers(composition, types, Mock.Of()); + var requirements = composers.GetRequirements(false); + Console.WriteLine(Composers.GetComposersReport(requirements)); types = new[] { typeof(Composer2) }; composers = new Composers(composition, types, Mock.Of()); Composed.Clear(); Assert.Throws(() => composers.Compose()); + Console.WriteLine("throws:"); + composers = new Composers(composition, types, Mock.Of()); + requirements = composers.GetRequirements(false); + Console.WriteLine(Composers.GetComposersReport(requirements)); types = new[] { typeof(Composer12) }; composers = new Composers(composition, types, Mock.Of()); @@ -349,6 +358,25 @@ namespace Umbraco.Tests.Components Assert.AreEqual(typeof(Composer27), Composed[1]); } + [Test] + public void AllComposers() + { + var typeLoader = new TypeLoader(AppCaches.Disabled.RuntimeCache, IOHelper.MapPath("~/App_Data/TEMP"), Mock.Of()); + + var register = MockRegister(); + var composition = new Composition(register, typeLoader, Mock.Of(), MockRuntimeState(RuntimeLevel.Run)); + + var types = typeLoader.GetTypes().Where(x => x.FullName.StartsWith("Umbraco.Core.") || x.FullName.StartsWith("Umbraco.Web")); + var composers = new Composers(composition, types, Mock.Of()); + var requirements = composers.GetRequirements(); + var report = Composers.GetComposersReport(requirements); + Console.WriteLine(report); + var composerTypes = composers.SortComposers(requirements); + + foreach (var type in composerTypes) + Console.WriteLine(type); + } + #region Compothings public class TestComposerBase : IComposer diff --git a/src/Umbraco.Web/Runtime/WebFinalComposer.cs b/src/Umbraco.Web/Runtime/WebFinalComposer.cs index 64d7725848..c69ae1af1a 100644 --- a/src/Umbraco.Web/Runtime/WebFinalComposer.cs +++ b/src/Umbraco.Web/Runtime/WebFinalComposer.cs @@ -3,7 +3,9 @@ namespace Umbraco.Web.Runtime { // web's final composer composes after all user composers + // and *also* after ICoreComposer (in case IUserComposer is disabled) [ComposeAfter(typeof(IUserComposer))] + [ComposeAfter(typeof(ICoreComposer))] public class WebFinalComposer : ComponentComposer { } } From 723d9eddd139a2a11cab9fc0f40abe586a6ef280 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 15 Apr 2019 18:15:52 +1000 Subject: [PATCH 15/24] unbreaks a ctor change preventing 8.0.2 from being dropped into the bin --- src/Umbraco.Web/BatchedDatabaseServerMessenger.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs b/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs index 818e8ecf77..f6d0bfd36a 100644 --- a/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs +++ b/src/Umbraco.Web/BatchedDatabaseServerMessenger.cs @@ -13,6 +13,7 @@ using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.Dtos; using Umbraco.Core.Scoping; using Umbraco.Web.Composing; +using System.ComponentModel; namespace Umbraco.Web { @@ -26,6 +27,14 @@ namespace Umbraco.Web { private readonly IUmbracoDatabaseFactory _databaseFactory; + [Obsolete("This overload should not be used, enableDistCalls has no effect")] + [EditorBrowsable(EditorBrowsableState.Never)] + public BatchedDatabaseServerMessenger( + IRuntimeState runtime, IUmbracoDatabaseFactory databaseFactory, IScopeProvider scopeProvider, ISqlContext sqlContext, IProfilingLogger proflog, IGlobalSettings globalSettings, bool enableDistCalls, DatabaseServerMessengerOptions options) + : this(runtime, databaseFactory, scopeProvider, sqlContext, proflog, globalSettings, options) + { + } + public BatchedDatabaseServerMessenger( IRuntimeState runtime, IUmbracoDatabaseFactory databaseFactory, IScopeProvider scopeProvider, ISqlContext sqlContext, IProfilingLogger proflog, IGlobalSettings globalSettings, DatabaseServerMessengerOptions options) : base(runtime, scopeProvider, sqlContext, proflog, globalSettings, true, options) From 02ddf80014093fa4dc4b60c8dd01f954519c9b61 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 16 Apr 2019 15:39:28 +1000 Subject: [PATCH 16/24] Adds a unit test --- .../Repositories/MemberTypeRepositoryTest.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs index 0c2314fd47..561822bbbe 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -232,6 +232,32 @@ namespace Umbraco.Tests.Persistence.Repositories } } + /// + /// This demonstates an issue found: https://github.com/umbraco/Umbraco-CMS/issues/4963#issuecomment-483516698 + /// + [Test] + public void Bug_Changing_Built_In_Member_Type_Property_Type_Aliases_Results_In_Exception() + { + //TODO: Fix this bug and then change this test + + var provider = TestObjects.GetScopeProvider(Logger); + using (var scope = provider.CreateScope()) + { + var repository = CreateRepository(provider); + + IMemberType memberType = MockedContentTypes.CreateSimpleMemberType(); + repository.Save(memberType); + + foreach(var stub in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) + { + var prop = memberType.PropertyTypes.First(x => x.Alias == stub.Key); + prop.Alias = prop.Alias + "__0000"; + } + + Assert.Throws(() => repository.Save(memberType)); + } + } + [Test] public void Built_In_Member_Type_Properties_Are_Automatically_Added_When_Creating() { From 6f44e4fd09775c594f0d5ef051a84e77e5e19573 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 16 Apr 2019 17:37:42 +1000 Subject: [PATCH 17/24] Fixes INotifyCollectionChanged collections to raise the correct events, ensures that no duplicate property type aliases can be added/updated to --- src/Umbraco.Core/EnumerableExtensions.cs | 17 +++++++ src/Umbraco.Core/Models/ContentTypeBase.cs | 12 +++++ src/Umbraco.Core/Models/PropertyCollection.cs | 13 ++--- .../Models/PropertyGroupCollection.cs | 17 +++---- .../Models/PropertyTypeCollection.cs | 37 ++++++++++---- src/Umbraco.Tests/Models/ContentTypeTests.cs | 50 +++++++++++++++---- .../Repositories/MemberTypeRepositoryTest.cs | 2 +- 7 files changed, 112 insertions(+), 36 deletions(-) diff --git a/src/Umbraco.Core/EnumerableExtensions.cs b/src/Umbraco.Core/EnumerableExtensions.cs index d71ccb04b9..00bb783486 100644 --- a/src/Umbraco.Core/EnumerableExtensions.cs +++ b/src/Umbraco.Core/EnumerableExtensions.cs @@ -10,6 +10,23 @@ namespace Umbraco.Core ///
 public static class EnumerableExtensions { + internal static bool HasDuplicates(this IEnumerable items, bool includeNull) + { + var hs = new HashSet(); + foreach (var item in items) + { + if (item != null || includeNull) + { + if (!hs.Add(item)) + { + return true; + } + } + } + return false; + } + + /// /// Wraps this object instance into an IEnumerable{T} consisting of a single item. /// diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index ed8a098299..04fac1a855 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -95,6 +95,18 @@ namespace Umbraco.Core.Models protected void PropertyTypesChanged(object sender, NotifyCollectionChangedEventArgs e) { + //detect if there are any duplicate aliases - this cannot be allowed + if (e.Action == NotifyCollectionChangedAction.Add + || e.Action == NotifyCollectionChangedAction.Replace) + { + var allAliases = _noGroupPropertyTypes.Concat(PropertyGroups.SelectMany(x => x.PropertyTypes)).Select(x => x.Alias); + if (allAliases.HasDuplicates(false)) + { + var newAliases = string.Join(", ", e.NewItems.Cast().Select(x => x.Alias)); + throw new InvalidOperationException($"Other property types already exist with the aliases: {newAliases}"); + } + } + OnPropertyChanged(nameof(PropertyTypes)); } diff --git a/src/Umbraco.Core/Models/PropertyCollection.cs b/src/Umbraco.Core/Models/PropertyCollection.cs index 977600a2f7..c587a45424 100644 --- a/src/Umbraco.Core/Models/PropertyCollection.cs +++ b/src/Umbraco.Core/Models/PropertyCollection.cs @@ -15,7 +15,7 @@ namespace Umbraco.Core.Models public class PropertyCollection : KeyedCollection, INotifyCollectionChanged, IDeepCloneable { private readonly object _addLocker = new object(); - internal Action OnAdd; + internal Func AdditionValidator { get; set; } /// @@ -49,10 +49,12 @@ namespace Umbraco.Core.Models /// internal void Reset(IEnumerable properties) { + //collection events will be raised in each of these calls Clear(); + + //collection events will be raised in each of these calls foreach (var property in properties) Add(property); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } /// @@ -60,8 +62,9 @@ namespace Umbraco.Core.Models /// protected override void SetItem(int index, Property property) { + var oldItem = index >= 0 ? this[index] : property; base.SetItem(index, property); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, property, index)); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, property, oldItem)); } /// @@ -120,10 +123,8 @@ namespace Umbraco.Core.Models } } + //collection events will be raised in InsertItem with Add base.Add(property); - - OnAdd?.Invoke(); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, property)); } } diff --git a/src/Umbraco.Core/Models/PropertyGroupCollection.cs b/src/Umbraco.Core/Models/PropertyGroupCollection.cs index 26e0fef178..5422dfb792 100644 --- a/src/Umbraco.Core/Models/PropertyGroupCollection.cs +++ b/src/Umbraco.Core/Models/PropertyGroupCollection.cs @@ -19,9 +19,6 @@ namespace Umbraco.Core.Models { private readonly ReaderWriterLockSlim _addLocker = new ReaderWriterLockSlim(); - // TODO: this doesn't seem to be used anywhere - internal Action OnAdd; - internal PropertyGroupCollection() { } @@ -37,16 +34,19 @@ namespace Umbraco.Core.Models /// internal void Reset(IEnumerable groups) { + //collection events will be raised in each of these calls Clear(); + + //collection events will be raised in each of these calls foreach (var group in groups) Add(group); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } protected override void SetItem(int index, PropertyGroup item) { + var oldItem = index >= 0 ? this[index] : item; base.SetItem(index, item); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, item, oldItem)); } protected override void RemoveItem(int index) @@ -84,6 +84,7 @@ namespace Umbraco.Core.Models if (keyExists) throw new Exception($"Naming conflict: Changing the name of PropertyGroup '{item.Name}' would result in duplicates"); + //collection events will be raised in SetItem SetItem(IndexOfKey(item.Id), item); return; } @@ -96,16 +97,14 @@ namespace Umbraco.Core.Models var exists = Contains(key); if (exists) { + //collection events will be raised in SetItem SetItem(IndexOfKey(key), item); return; } } } - + //collection events will be raised in InsertItem base.Add(item); - OnAdd?.Invoke(); - - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); } finally { diff --git a/src/Umbraco.Core/Models/PropertyTypeCollection.cs b/src/Umbraco.Core/Models/PropertyTypeCollection.cs index 6181ee078b..6e41f0d12b 100644 --- a/src/Umbraco.Core/Models/PropertyTypeCollection.cs +++ b/src/Umbraco.Core/Models/PropertyTypeCollection.cs @@ -19,9 +19,6 @@ namespace Umbraco.Core.Models [IgnoreDataMember] private readonly ReaderWriterLockSlim _addLocker = new ReaderWriterLockSlim(); - // TODO: This doesn't seem to be used - [IgnoreDataMember] - internal Action OnAdd; internal PropertyTypeCollection(bool supportsPublishing) { @@ -43,36 +40,44 @@ namespace Umbraco.Core.Models /// internal void Reset(IEnumerable properties) { + //collection events will be raised in each of these calls Clear(); + + //collection events will be raised in each of these calls foreach (var property in properties) - Add(property); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + Add(property); } protected override void SetItem(int index, PropertyType item) { item.SupportsPublishing = SupportsPublishing; - base.SetItem(index, item); - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item, index)); + var oldItem = index >= 0 ? this[index] : item; + base.SetItem(index, item); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, item, oldItem)); + item.PropertyChanged += Item_PropertyChanged; } protected override void RemoveItem(int index) { var removed = this[index]; base.RemoveItem(index); + removed.PropertyChanged -= Item_PropertyChanged; OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removed)); } protected override void InsertItem(int index, PropertyType item) { item.SupportsPublishing = SupportsPublishing; - base.InsertItem(index, item); + base.InsertItem(index, item); OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); + item.PropertyChanged += Item_PropertyChanged; } protected override void ClearItems() { base.ClearItems(); + foreach (var item in this) + item.PropertyChanged -= Item_PropertyChanged; OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); } @@ -91,6 +96,7 @@ namespace Umbraco.Core.Models var exists = Contains(key); if (exists) { + //collection events will be raised in SetItem SetItem(IndexOfKey(key), item); return; } @@ -103,10 +109,8 @@ namespace Umbraco.Core.Models item.SortOrder = this.Max(x => x.SortOrder) + 1; } + //collection events will be raised in InsertItem base.Add(item); - OnAdd?.Invoke(); - - OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, item)); } finally { @@ -115,6 +119,17 @@ namespace Umbraco.Core.Models } } + /// + /// Occurs when a property changes on a PropertyType that exists in this collection + /// + /// + /// + private void Item_PropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e) + { + var propType = (PropertyType)sender; + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, propType, propType)); + } + /// /// Determines whether this collection contains a whose alias matches the specified PropertyType. /// diff --git a/src/Umbraco.Tests/Models/ContentTypeTests.cs b/src/Umbraco.Tests/Models/ContentTypeTests.cs index d9e65ba6c6..3b35bb2dfc 100644 --- a/src/Umbraco.Tests/Models/ContentTypeTests.cs +++ b/src/Umbraco.Tests/Models/ContentTypeTests.cs @@ -15,13 +15,45 @@ namespace Umbraco.Tests.Models [TestFixture] public class ContentTypeTests : UmbracoTestBase { + [Test] + public void Cannot_Add_Duplicate_Property_Aliases() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + contentType.PropertyGroups.Add(new PropertyGroup(new PropertyTypeCollection(false, new[] + { + new PropertyType("testPropertyEditor", ValueStorageType.Nvarchar){ Alias = "myPropertyType" } + }))); + + Assert.Throws(() => + contentType.PropertyTypeCollection.Add( + new PropertyType("testPropertyEditor", ValueStorageType.Nvarchar) { Alias = "myPropertyType" })); + + } + + [Test] + public void Cannot_Update_Duplicate_Property_Aliases() + { + var contentType = MockedContentTypes.CreateBasicContentType(); + + contentType.PropertyGroups.Add(new PropertyGroup(new PropertyTypeCollection(false, new[] + { + new PropertyType("testPropertyEditor", ValueStorageType.Nvarchar){ Alias = "myPropertyType" } + }))); + + contentType.PropertyTypeCollection.Add(new PropertyType("testPropertyEditor", ValueStorageType.Nvarchar) { Alias = "myPropertyType2" }); + + var toUpdate = contentType.PropertyTypeCollection["myPropertyType2"]; + + Assert.Throws(() => toUpdate.Alias = "myPropertyType"); + + } [Test] public void Can_Deep_Clone_Content_Type_Sort() { var contentType = new ContentTypeSort(new Lazy(() => 3), 4, "test"); - var clone = (ContentTypeSort) contentType.DeepClone(); + var clone = (ContentTypeSort)contentType.DeepClone(); Assert.AreNotSame(clone, contentType); Assert.AreEqual(clone, contentType); Assert.AreEqual(clone.Id.Value, contentType.Id.Value); @@ -54,7 +86,7 @@ namespace Umbraco.Tests.Models contentType.Id = 10; contentType.CreateDate = DateTime.Now; contentType.CreatorId = 22; - contentType.SetDefaultTemplate(new Template((string) "Test Template", (string) "testTemplate") + contentType.SetDefaultTemplate(new Template((string)"Test Template", (string)"testTemplate") { Id = 88 }); @@ -117,12 +149,12 @@ namespace Umbraco.Tests.Models { group.Id = ++i; } - contentType.AllowedTemplates = new[] { new Template((string) "Name", (string) "name") { Id = 200 }, new Template((string) "Name2", (string) "name2") { Id = 201 } }; + contentType.AllowedTemplates = new[] { new Template((string)"Name", (string)"name") { Id = 200 }, new Template((string)"Name2", (string)"name2") { Id = 201 } }; contentType.AllowedContentTypes = new[] { new ContentTypeSort(new Lazy(() => 888), 8, "sub"), new ContentTypeSort(new Lazy(() => 889), 9, "sub2") }; contentType.Id = 10; contentType.CreateDate = DateTime.Now; contentType.CreatorId = 22; - contentType.SetDefaultTemplate(new Template((string) "Test Template", (string) "testTemplate") + contentType.SetDefaultTemplate(new Template((string)"Test Template", (string)"testTemplate") { Id = 88 }); @@ -167,12 +199,12 @@ namespace Umbraco.Tests.Models { group.Id = ++i; } - contentType.AllowedTemplates = new[] { new Template((string) "Name", (string) "name") { Id = 200 }, new Template((string) "Name2", (string) "name2") { Id = 201 } }; - contentType.AllowedContentTypes = new[] {new ContentTypeSort(new Lazy(() => 888), 8, "sub"), new ContentTypeSort(new Lazy(() => 889), 9, "sub2")}; + contentType.AllowedTemplates = new[] { new Template((string)"Name", (string)"name") { Id = 200 }, new Template((string)"Name2", (string)"name2") { Id = 201 } }; + contentType.AllowedContentTypes = new[] { new ContentTypeSort(new Lazy(() => 888), 8, "sub"), new ContentTypeSort(new Lazy(() => 889), 9, "sub2") }; contentType.Id = 10; contentType.CreateDate = DateTime.Now; contentType.CreatorId = 22; - contentType.SetDefaultTemplate(new Template((string) "Test Template", (string) "testTemplate") + contentType.SetDefaultTemplate(new Template((string)"Test Template", (string)"testTemplate") { Id = 88 }); @@ -264,12 +296,12 @@ namespace Umbraco.Tests.Models { propertyType.Id = ++i; } - contentType.AllowedTemplates = new[] { new Template((string) "Name", (string) "name") { Id = 200 }, new Template((string) "Name2", (string) "name2") { Id = 201 } }; + contentType.AllowedTemplates = new[] { new Template((string)"Name", (string)"name") { Id = 200 }, new Template((string)"Name2", (string)"name2") { Id = 201 } }; contentType.AllowedContentTypes = new[] { new ContentTypeSort(new Lazy(() => 888), 8, "sub"), new ContentTypeSort(new Lazy(() => 889), 9, "sub2") }; contentType.Id = 10; contentType.CreateDate = DateTime.Now; contentType.CreatorId = 22; - contentType.SetDefaultTemplate(new Template((string) "Test Template", (string) "testTemplate") + contentType.SetDefaultTemplate(new Template((string)"Test Template", (string)"testTemplate") { Id = 88 }); diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs index 561822bbbe..0b655004da 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -248,7 +248,7 @@ namespace Umbraco.Tests.Persistence.Repositories IMemberType memberType = MockedContentTypes.CreateSimpleMemberType(); repository.Save(memberType); - foreach(var stub in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) + foreach (var stub in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) { var prop = memberType.PropertyTypes.First(x => x.Alias == stub.Key); prop.Alias = prop.Alias + "__0000"; From 4d8d10f07a5efdae27d11cf99d928c208c5af707 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 17 Apr 2019 16:03:36 +1000 Subject: [PATCH 18/24] adds comments, ignores test for now --- .../Persistence/Factories/MemberTypeReadOnlyFactory.cs | 7 +++++++ .../Persistence/Repositories/MemberTypeRepositoryTest.cs | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index c7ce98a89c..9beeddde63 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -54,6 +54,12 @@ namespace Umbraco.Core.Persistence.Factories // no id, no dataTypeDefinitionId - ouch! - better notify caller of the situation needsSaving = true; + //fixme - this is wrong, this will add the standard prop without a group, yet by default it is attempted to be added to the + // group with the `Membership` alias just like we do in the MemberTypeRepository.PersistNewItem: + // //By Convention we add 9 standard PropertyTypes to an Umbraco MemberType + // entity.AddPropertyGroup(Constants.Conventions.Member.StandardPropertiesGroupName); + // that's exactly how it would need to be done here too, but... need to decide where/how this is done properly. + //Add the standard PropertyType to the current list propertyTypes.Add(standardPropertyType.Value); @@ -154,6 +160,7 @@ namespace Umbraco.Core.Persistence.Factories foreach (var typeDto in dto.PropertyTypes.Where(x => (x.PropertyTypeGroupId.HasValue == false || x.PropertyTypeGroupId.Value == 0) && x.Id.HasValue)) { //Internal dictionary for adding "MemberCanEdit" and "VisibleOnProfile" properties to each PropertyType + memberType.MemberTypePropertyTypes.Add(typeDto.Alias, new MemberTypePropertyProfileAccess(typeDto.ViewOnProfile, typeDto.CanEdit, typeDto.IsSensitive)); diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs index 0b655004da..f84c477fa9 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -236,6 +236,7 @@ namespace Umbraco.Tests.Persistence.Repositories /// This demonstates an issue found: https://github.com/umbraco/Umbraco-CMS/issues/4963#issuecomment-483516698 /// [Test] + [Ignore("Still testing")] public void Bug_Changing_Built_In_Member_Type_Property_Type_Aliases_Results_In_Exception() { //TODO: Fix this bug and then change this test @@ -254,7 +255,9 @@ namespace Umbraco.Tests.Persistence.Repositories prop.Alias = prop.Alias + "__0000"; } - Assert.Throws(() => repository.Save(memberType)); + repository.Save(memberType); + + //Assert.Throws(() => ); } } From 522fdefb8c0535594be34d61c6b2b2be1056be1c Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 17 Apr 2019 16:42:17 +1000 Subject: [PATCH 19/24] Ensures that ALL membership properties are created by the installer, removes the logic that updates a membertype while fetching it, fixes the logic of mapping membership properties when fetching and they don't exist. --- .../Migrations/Install/DatabaseDataCreator.cs | 2 + .../Factories/MemberTypeReadOnlyFactory.cs | 45 +++++++++---------- .../Implement/MemberTypeRepository.cs | 8 +--- 3 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs index f0e6dd2e5b..1de983636b 100644 --- a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs @@ -226,6 +226,8 @@ namespace Umbraco.Core.Migrations.Install _database.Insert(Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 32, UniqueId = 32.ToGuid(), DataTypeId = Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Constants.Conventions.Member.LastLockoutDate, Name = Constants.Conventions.Member.LastLockoutDateLabel, SortOrder = 4, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte) ContentVariation.Nothing }); _database.Insert(Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 33, UniqueId = 33.ToGuid(), DataTypeId = Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Constants.Conventions.Member.LastLoginDate, Name = Constants.Conventions.Member.LastLoginDateLabel, SortOrder = 5, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte) ContentVariation.Nothing }); _database.Insert(Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 34, UniqueId = 34.ToGuid(), DataTypeId = Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = 11, Alias = Constants.Conventions.Member.LastPasswordChangeDate, Name = Constants.Conventions.Member.LastPasswordChangeDateLabel, SortOrder = 6, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte) ContentVariation.Nothing }); + _database.Insert(Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 35, UniqueId = 35.ToGuid(), DataTypeId = Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = null, Alias = Constants.Conventions.Member.PasswordQuestion, Name = Constants.Conventions.Member.PasswordQuestionLabel, SortOrder = 7, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); + _database.Insert(Constants.DatabaseSchema.Tables.PropertyType, "id", false, new PropertyTypeDto { Id = 36, UniqueId = 36.ToGuid(), DataTypeId = Constants.DataTypes.LabelDateTime, ContentTypeId = 1044, PropertyTypeGroupId = null, Alias = Constants.Conventions.Member.PasswordAnswer, Name = Constants.Conventions.Member.PasswordAnswerLabel, SortOrder = 8, Mandatory = false, ValidationRegExp = null, Description = null, Variations = (byte)ContentVariation.Nothing }); } diff --git a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs index 9beeddde63..bafd73067f 100644 --- a/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/MemberTypeReadOnlyFactory.cs @@ -9,11 +9,10 @@ namespace Umbraco.Core.Persistence.Factories { internal static class MemberTypeReadOnlyFactory { - public static IMemberType BuildEntity(MemberTypeReadOnlyDto dto, out bool needsSaving) + public static IMemberType BuildEntity(MemberTypeReadOnlyDto dto) { var standardPropertyTypes = Constants.Conventions.Member.GetStandardPropertyTypeStubs(); - needsSaving = false; - + var memberType = new MemberType(dto.ParentId); try @@ -41,33 +40,29 @@ namespace Umbraco.Core.Persistence.Factories var propertyTypeGroupCollection = GetPropertyTypeGroupCollection(dto, memberType, standardPropertyTypes); memberType.PropertyGroups = propertyTypeGroupCollection; - var propertyTypes = GetPropertyTypes(dto, memberType, standardPropertyTypes); + memberType.NoGroupPropertyTypes = GetNoGroupPropertyTypes(dto, memberType, standardPropertyTypes); - //By Convention we add 9 standard PropertyTypes - This is only here to support loading of types that didn't have these conventions before. + // By Convention we add 9 standard PropertyTypes - This is only here to support loading of types that didn't have these conventions before. + // In theory this should not happen! The only reason this did happen was because: + // A) we didn't install all of the default membership properties by default + // B) the existing data is super old when we didn't store membership properties at all (very very old) + // So what to do? We absolutely do not want to update the database when only a read was requested, this will cause problems + // so we should just add these virtual properties, they will have no ids but they will have aliases and this should be perfectly + // fine for any membership provider logic to work since neither membership providers or asp.net identity care about whether a property + // has an ID or not. + // When the member type is saved, all the properties will correctly be added. + + //This will add this group if it doesn't exist, no need to error check here + memberType.AddPropertyGroup(Constants.Conventions.Member.StandardPropertiesGroupName); foreach (var standardPropertyType in standardPropertyTypes) { - if (dto.PropertyTypes.Any(x => x.Alias.Equals(standardPropertyType.Key))) continue; + //This will add the property if it doesn't exist, no need to error check here + memberType.AddPropertyType(standardPropertyType.Value, Constants.Conventions.Member.StandardPropertiesGroupName); - // beware! - // means that we can return a memberType "from database" that has some property types - // that do *not* come from the database and therefore are incomplete eg have no key, - // no id, no dataTypeDefinitionId - ouch! - better notify caller of the situation - needsSaving = true; - - //fixme - this is wrong, this will add the standard prop without a group, yet by default it is attempted to be added to the - // group with the `Membership` alias just like we do in the MemberTypeRepository.PersistNewItem: - // //By Convention we add 9 standard PropertyTypes to an Umbraco MemberType - // entity.AddPropertyGroup(Constants.Conventions.Member.StandardPropertiesGroupName); - // that's exactly how it would need to be done here too, but... need to decide where/how this is done properly. - - //Add the standard PropertyType to the current list - propertyTypes.Add(standardPropertyType.Value); - //Internal dictionary for adding "MemberCanEdit", "VisibleOnProfile", "IsSensitive" properties to each PropertyType - memberType.MemberTypePropertyTypes.Add(standardPropertyType.Key, - new MemberTypePropertyProfileAccess(false, false, false)); + if (!memberType.MemberTypePropertyTypes.TryGetValue(standardPropertyType.Key, out var memberTypePropertyProfile)) + memberType.MemberTypePropertyTypes[standardPropertyType.Key] = new MemberTypePropertyProfileAccess(false, false, false); } - memberType.NoGroupPropertyTypes = propertyTypes; return memberType; } @@ -153,7 +148,7 @@ namespace Umbraco.Core.Persistence.Factories return propertyGroups; } - private static List GetPropertyTypes(MemberTypeReadOnlyDto dto, MemberType memberType, Dictionary standardProps) + private static List GetNoGroupPropertyTypes(MemberTypeReadOnlyDto dto, MemberType memberType, Dictionary standardProps) { //Find PropertyTypes that does not belong to a PropertyTypeGroup var propertyTypes = new List(); diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs index afb6ac8b43..ffeff13207 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberTypeRepository.cs @@ -308,13 +308,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (dtos == null || dtos.Any() == false) return Enumerable.Empty(); - return dtos.Select(x => - { - bool needsSaving; - var memberType = MemberTypeReadOnlyFactory.BuildEntity(x, out needsSaving); - if (needsSaving) PersistUpdatedItem(memberType); - return memberType; - }).ToList(); + return dtos.Select(MemberTypeReadOnlyFactory.BuildEntity).ToList(); } /// From 0b6a369047cc301574f403e6a8d96791dacbc0ed Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 17 Apr 2019 17:12:34 +1000 Subject: [PATCH 20/24] When removing a property group, we need to first remove the group and then re-assign the property types so that duplicates are not added --- src/Umbraco.Core/Models/ContentTypeBase.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 04fac1a855..23e83e0e21 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -409,15 +409,16 @@ namespace Umbraco.Core.Models var group = PropertyGroups[propertyGroupName]; if (group == null) return; - // re-assign the group's properties to no group + // first remove the group + PropertyGroups.RemoveItem(propertyGroupName); + + // Then re-assign the group's properties to no group foreach (var property in group.PropertyTypes) { property.PropertyGroupId = null; _noGroupPropertyTypes.Add(property); } - // actually remove the group - PropertyGroups.RemoveItem(propertyGroupName); OnPropertyChanged(nameof(PropertyGroups)); } From a2a247b41337ca4b1d70e7188757dd47a7448f65 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 17 Apr 2019 21:00:15 +1000 Subject: [PATCH 21/24] removes the enforcement of non duplicate property type aliases for now --- src/Umbraco.Core/Models/ContentTypeBase.cs | 25 +++++++++++--------- src/Umbraco.Tests/Models/ContentTypeTests.cs | 2 ++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 23e83e0e21..725ebe1555 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -95,17 +95,20 @@ namespace Umbraco.Core.Models protected void PropertyTypesChanged(object sender, NotifyCollectionChangedEventArgs e) { - //detect if there are any duplicate aliases - this cannot be allowed - if (e.Action == NotifyCollectionChangedAction.Add - || e.Action == NotifyCollectionChangedAction.Replace) - { - var allAliases = _noGroupPropertyTypes.Concat(PropertyGroups.SelectMany(x => x.PropertyTypes)).Select(x => x.Alias); - if (allAliases.HasDuplicates(false)) - { - var newAliases = string.Join(", ", e.NewItems.Cast().Select(x => x.Alias)); - throw new InvalidOperationException($"Other property types already exist with the aliases: {newAliases}"); - } - } + //enable this to detect duplicate property aliases. We do want this, however making this change in a + //patch release might be a little dangerous + + ////detect if there are any duplicate aliases - this cannot be allowed + //if (e.Action == NotifyCollectionChangedAction.Add + // || e.Action == NotifyCollectionChangedAction.Replace) + //{ + // var allAliases = _noGroupPropertyTypes.Concat(PropertyGroups.SelectMany(x => x.PropertyTypes)).Select(x => x.Alias); + // if (allAliases.HasDuplicates(false)) + // { + // var newAliases = string.Join(", ", e.NewItems.Cast().Select(x => x.Alias)); + // throw new InvalidOperationException($"Other property types already exist with the aliases: {newAliases}"); + // } + //} OnPropertyChanged(nameof(PropertyTypes)); } diff --git a/src/Umbraco.Tests/Models/ContentTypeTests.cs b/src/Umbraco.Tests/Models/ContentTypeTests.cs index 3b35bb2dfc..9c3b976bf3 100644 --- a/src/Umbraco.Tests/Models/ContentTypeTests.cs +++ b/src/Umbraco.Tests/Models/ContentTypeTests.cs @@ -16,6 +16,7 @@ namespace Umbraco.Tests.Models public class ContentTypeTests : UmbracoTestBase { [Test] + [Ignore("Ignoring this test until we actually enforce this, see comments in ContentTypeBase.PropertyTypesChanged")] public void Cannot_Add_Duplicate_Property_Aliases() { var contentType = MockedContentTypes.CreateBasicContentType(); @@ -32,6 +33,7 @@ namespace Umbraco.Tests.Models } [Test] + [Ignore("Ignoring this test until we actually enforce this, see comments in ContentTypeBase.PropertyTypesChanged")] public void Cannot_Update_Duplicate_Property_Aliases() { var contentType = MockedContentTypes.CreateBasicContentType(); From 0cf857e8bcfbc7d7a7297a88b6174e844c4ebb8c Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 23 Apr 2019 12:09:13 +0200 Subject: [PATCH 22/24] Cleanup --- src/Umbraco.Core/EnumerableExtensions.cs | 11 +++------- .../src/less/modals.less | 22 +++++-------------- 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Core/EnumerableExtensions.cs b/src/Umbraco.Core/EnumerableExtensions.cs index 00bb783486..3719bb0750 100644 --- a/src/Umbraco.Core/EnumerableExtensions.cs +++ b/src/Umbraco.Core/EnumerableExtensions.cs @@ -15,13 +15,8 @@ namespace Umbraco.Core var hs = new HashSet(); foreach (var item in items) { - if (item != null || includeNull) - { - if (!hs.Add(item)) - { - return true; - } - } + if ((item != null || includeNull) && !hs.Add(item)) + return true; } return false; } @@ -117,7 +112,7 @@ namespace Umbraco.Core } } - + /// /// Returns true if all items in the other collection exist in this collection /// diff --git a/src/Umbraco.Web.UI.Client/src/less/modals.less b/src/Umbraco.Web.UI.Client/src/less/modals.less index c86ca7e2d7..52a573d8c4 100644 --- a/src/Umbraco.Web.UI.Client/src/less/modals.less +++ b/src/Umbraco.Web.UI.Client/src/less/modals.less @@ -52,17 +52,7 @@ bottom: 0px; left: 0px; right: 0px; - position: absolute; -} - -.--notInFront .umb-modalcolumn::after { - content: ''; - position: absolute; - top: 0; - bottom: 0; - right: 0; - left: 0; - background: rgba(0,0,0,.4); + position: absolute; } .--notInFront .umb-modalcolumn::after { @@ -77,7 +67,7 @@ /* re-align loader */ .umb-modal .umb-loader-wrapper, .umb-modalcolumn .umb-loader-wrapper, .umb-dialog .umb-loader-wrapper{ - position:relative; + position:relative; margin: 20px -20px; } @@ -116,7 +106,7 @@ .umb-dialog-footer{ position: absolute; overflow:auto; - text-align: right; + text-align: right; height: 32px; left: 0px; right: 0px; @@ -136,7 +126,7 @@ .umbracoDialog form{height: 100%;} /*ensures dialogs doesnt have side-by-side labels*/ -.umbracoDialog .controls-row, +.umbracoDialog .controls-row, .umb-modal .controls-row{margin-left: 0px !important;} /* modal and umb-modal are used for right.hand dialogs */ @@ -184,7 +174,7 @@ padding: 20px; background: @white; border: none; - height: auto; + height: auto; } .umb-modal .umb-panel-body{ padding: 0px 20px 0px 20px; @@ -196,7 +186,7 @@ } .umb-modal i { font-size: 20px; -} +} .umb-modal .breadcrumb { background: none; padding: 0 From 509b1151a2afc17b13cd00bd71a248c6f51c78bd Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 23 Apr 2019 12:09:22 +0200 Subject: [PATCH 23/24] Fix tests --- src/Umbraco.Tests/Composing/TypeLoaderTests.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Umbraco.Tests/Composing/TypeLoaderTests.cs b/src/Umbraco.Tests/Composing/TypeLoaderTests.cs index 7b7574ce47..7459ae848b 100644 --- a/src/Umbraco.Tests/Composing/TypeLoaderTests.cs +++ b/src/Umbraco.Tests/Composing/TypeLoaderTests.cs @@ -27,10 +27,7 @@ namespace Umbraco.Tests.Composing public void Initialize() { // this ensures it's reset - _typeLoader = new TypeLoader(NoAppCache.Instance, IOHelper.MapPath("~/App_Data/TEMP"), new ProfilingLogger(Mock.Of(), Mock.Of())); - - foreach (var file in Directory.GetFiles(IOHelper.MapPath(SystemDirectories.TempData.EnsureEndsWith('/') + "TypesCache"))) - File.Delete(file); + _typeLoader = new TypeLoader(NoAppCache.Instance, IOHelper.MapPath("~/App_Data/TEMP"), new ProfilingLogger(Mock.Of(), Mock.Of()), false); // for testing, we'll specify which assemblies are scanned for the PluginTypeResolver // TODO: Should probably update this so it only searches this assembly and add custom types to be found From 3185f7ae97723056b8fa1045f476297ed3eb2034 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 23 Apr 2019 13:07:57 +0200 Subject: [PATCH 24/24] Fix member type builtin properties handling --- .../Implement/ContentTypeCommonRepository.cs | 21 ++++- .../Repositories/MemberTypeRepositoryTest.cs | 89 ++++++++++++++++--- 2 files changed, 95 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs index 194a00b7f2..645ab9f924 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeCommonRepository.cs @@ -240,6 +240,25 @@ namespace Umbraco.Core.Persistence.Repositories.Implement propertyIx++; } contentType.NoGroupPropertyTypes = noGroupPropertyTypes; + + // ensure builtin properties + if (contentType is MemberType memberType) + { + // ensure that the group exists (ok if it already exists) + memberType.AddPropertyGroup(Constants.Conventions.Member.StandardPropertiesGroupName); + + // ensure that property types exist (ok if they already exist) + foreach (var (alias, propertyType) in builtinProperties) + { + var added = memberType.AddPropertyType(propertyType, Constants.Conventions.Member.StandardPropertiesGroupName); + + if (added) + { + var access = new MemberTypePropertyProfileAccess(false, false, false); + memberType.MemberTypePropertyTypes[alias] = access; + } + } + } } } @@ -264,7 +283,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (contentType is MemberType memberType) { var access = new MemberTypePropertyProfileAccess(dto.ViewOnProfile, dto.CanEdit, dto.IsSensitive); - memberType.MemberTypePropertyTypes.Add(dto.Alias, access); + memberType.MemberTypePropertyTypes[dto.Alias] = access; } return new PropertyType(dto.DataTypeDto.EditorAlias, storageType, readonlyStorageType, dto.Alias) diff --git a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs index 81d3b219ce..79e8e43804 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MemberTypeRepositoryTest.cs @@ -171,7 +171,6 @@ namespace Umbraco.Tests.Persistence.Repositories } } - //NOTE: This tests for left join logic (rev 7b14e8eacc65f82d4f184ef46c23340c09569052) [Test] public void Can_Get_All_Members_When_No_Properties_Assigned() @@ -200,7 +199,6 @@ namespace Umbraco.Tests.Persistence.Repositories } } - [Test] public void Can_Get_Member_Type_By_Id() { @@ -233,51 +231,114 @@ namespace Umbraco.Tests.Persistence.Repositories } } - /// - /// This demonstates an issue found: https://github.com/umbraco/Umbraco-CMS/issues/4963#issuecomment-483516698 - /// + // See: https://github.com/umbraco/Umbraco-CMS/issues/4963#issuecomment-483516698 [Test] - [Ignore("Still testing")] public void Bug_Changing_Built_In_Member_Type_Property_Type_Aliases_Results_In_Exception() { - //TODO: Fix this bug and then change this test + var stubs = Constants.Conventions.Member.GetStandardPropertyTypeStubs(); var provider = TestObjects.GetScopeProvider(Logger); - using (var scope = provider.CreateScope()) + using (provider.CreateScope()) { var repository = CreateRepository(provider); - IMemberType memberType = MockedContentTypes.CreateSimpleMemberType(); + IMemberType memberType = MockedContentTypes.CreateSimpleMemberType("mtype"); + + // created without the stub properties + Assert.AreEqual(1, memberType.PropertyGroups.Count); + Assert.AreEqual(3, memberType.PropertyTypes.Count()); + + // saving *new* member type adds the stub properties repository.Save(memberType); - foreach (var stub in Constants.Conventions.Member.GetStandardPropertyTypeStubs()) + // saving has added (and saved) the stub properties + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count, memberType.PropertyTypes.Count()); + + foreach (var stub in stubs) { var prop = memberType.PropertyTypes.First(x => x.Alias == stub.Key); prop.Alias = prop.Alias + "__0000"; } + // saving *existing* member type does *not* ensure stub properties repository.Save(memberType); - //Assert.Throws(() => ); + // therefore, nothing has changed + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count, memberType.PropertyTypes.Count()); + + // fetching ensures that the stub properties are there + memberType = repository.Get("mtype"); + Assert.IsNotNull(memberType); + + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count * 2, memberType.PropertyTypes.Count()); } } [Test] public void Built_In_Member_Type_Properties_Are_Automatically_Added_When_Creating() { + var stubs = Constants.Conventions.Member.GetStandardPropertyTypeStubs(); + var provider = TestObjects.GetScopeProvider(Logger); - using (var scope = provider.CreateScope()) + using (provider.CreateScope()) { var repository = CreateRepository(provider); IMemberType memberType = MockedContentTypes.CreateSimpleMemberType(); + + // created without the stub properties + Assert.AreEqual(1, memberType.PropertyGroups.Count); + Assert.AreEqual(3, memberType.PropertyTypes.Count()); + + // saving *new* member type adds the stub properties repository.Save(memberType); + // saving has added (and saved) the stub properties + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count, memberType.PropertyTypes.Count()); + // getting with stub properties memberType = repository.Get(memberType.Id); - Assert.That(memberType.PropertyTypes.Count(), Is.EqualTo(3 + Constants.Conventions.Member.GetStandardPropertyTypeStubs().Count)); - Assert.That(memberType.PropertyGroups.Count(), Is.EqualTo(2)); + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count, memberType.PropertyTypes.Count()); + } + } + + [Test] + public void Built_In_Member_Type_Properties_Missing_Are_Automatically_Added_When_Creating() + { + var stubs = Constants.Conventions.Member.GetStandardPropertyTypeStubs(); + + var provider = TestObjects.GetScopeProvider(Logger); + using (provider.CreateScope()) + { + var repository = CreateRepository(provider); + + IMemberType memberType = MockedContentTypes.CreateSimpleMemberType(); + + // created without the stub properties + Assert.AreEqual(1, memberType.PropertyGroups.Count); + Assert.AreEqual(3, memberType.PropertyTypes.Count()); + + // add one stub property, others are still missing + memberType.AddPropertyType(stubs.First().Value, Constants.Conventions.Member.StandardPropertiesGroupName); + + // saving *new* member type adds the (missing) stub properties + repository.Save(memberType); + + // saving has added (and saved) the (missing) stub properties + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count, memberType.PropertyTypes.Count()); + + // getting with stub properties + memberType = repository.Get(memberType.Id); + + Assert.AreEqual(2, memberType.PropertyGroups.Count); + Assert.AreEqual(3 + stubs.Count, memberType.PropertyTypes.Count()); } }