From aee6156a096efec60bcd9dc0459b5cd7317bfe9c Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 6 Dec 2018 10:02:23 +0100 Subject: [PATCH 1/8] Refactored the ManifestContentAppDefinition to have a factory and a seperate data class. --- .../Manifest/ContentAppDefinitionConverter.cs | 6 +- .../Manifest/ManifestContentAppDefinition.cs | 131 +------------ .../Manifest/ManifestContentAppFactory.cs | 172 ++++++++++++++++++ src/Umbraco.Core/Manifest/ManifestParser.cs | 2 +- src/Umbraco.Core/Manifest/PackageManifest.cs | 2 +- ...AppDefinition.cs => IContentAppFactory.cs} | 4 +- src/Umbraco.Core/Umbraco.Core.csproj | 3 +- .../Manifest/ManifestContentAppTests.cs | 3 +- .../ContentAppDefinitionCollection.cs | 11 +- .../ContentAppDefinitionCollectionBuilder.cs | 10 +- ...n.cs => ContentEditorContentAppFactory.cs} | 2 +- ...ion.cs => ContentInfoContentAppFactory.cs} | 2 +- ...nition.cs => ListViewContentAppFactory.cs} | 4 +- src/Umbraco.Web/Editors/ContentController.cs | 2 +- src/Umbraco.Web/Editors/MediaController.cs | 2 +- src/Umbraco.Web/Editors/MemberController.cs | 2 +- .../Runtime/WebRuntimeComponent.cs | 6 +- src/Umbraco.Web/Umbraco.Web.csproj | 6 +- 18 files changed, 212 insertions(+), 158 deletions(-) create mode 100644 src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs rename src/Umbraco.Core/Models/ContentEditing/{IContentAppDefinition.cs => IContentAppFactory.cs} (92%) rename src/Umbraco.Web/ContentApps/{ContentEditorContentAppDefinition.cs => ContentEditorContentAppFactory.cs} (95%) rename src/Umbraco.Web/ContentApps/{ContentInfoContentAppDefinition.cs => ContentInfoContentAppFactory.cs} (95%) rename src/Umbraco.Web/ContentApps/{ListViewContentAppDefinition.cs => ListViewContentAppFactory.cs} (96%) diff --git a/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs b/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs index 87f104d90e..dd167e06df 100644 --- a/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs +++ b/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs @@ -6,11 +6,11 @@ using Umbraco.Core.Serialization; namespace Umbraco.Core.Manifest { /// - /// Implements a json read converter for . + /// Implements a json read converter for . /// - internal class ContentAppDefinitionConverter : JsonReadConverter + internal class ContentAppDefinitionConverter : JsonReadConverter { - protected override IContentAppDefinition Create(Type objectType, string path, JObject jObject) + protected override ManifestContentAppDefinition Create(Type objectType, string path, JObject jObject) => new ManifestContentAppDefinition(); } } diff --git a/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs b/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs index d5f6c2b8c4..0667f11aab 100644 --- a/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs +++ b/src/Umbraco.Core/Manifest/ManifestContentAppDefinition.cs @@ -31,11 +31,9 @@ namespace Umbraco.Core.Manifest /// Represents a content app definition, parsed from a manifest. /// [DataContract(Name = "appdef", Namespace = "")] - public class ManifestContentAppDefinition : IContentAppDefinition + public class ManifestContentAppDefinition { private string _view; - private ContentApp _app; - private ShowRule[] _showRules; /// /// Gets or sets the name of the content app. @@ -83,132 +81,5 @@ namespace Umbraco.Core.Manifest [DataMember(Name = "show")] public string[] Show { get; set; } = Array.Empty(); - /// - public ContentApp GetContentAppFor(object o, IEnumerable userGroups) - { - string partA, partB; - - switch (o) - { - case IContent content: - partA = "content"; - partB = content.ContentType.Alias; - break; - - case IMedia media: - partA = "media"; - partB = media.ContentType.Alias; - break; - - default: - return null; - } - - var rules = _showRules ?? (_showRules = ShowRule.Parse(Show).ToArray()); - var userGroupsList = userGroups.ToList(); - - var okRole = false; - var hasRole = false; - var okType = false; - var hasType = false; - - foreach (var rule in rules) - { - if (rule.PartA.InvariantEquals("role")) - { - // if roles have been ok-ed already, skip the rule - if (okRole) - continue; - - // remember we have role rules - hasRole = true; - - foreach (var group in userGroupsList) - { - // if the entry does not apply, skip - if (!rule.Matches("role", group.Alias)) - continue; - - // if the entry applies, - // if it's an exclude entry, exit, do not display the content app - if (!rule.Show) - return null; - - // else ok to display, remember roles are ok, break from userGroupsList - okRole = rule.Show; - break; - } - } - else // it is a type rule - { - // if type has been ok-ed already, skip the rule - if (okType) - continue; - - // remember we have type rules - hasType = true; - - // if the entry does not apply, skip it - if (!rule.Matches(partA, partB)) - continue; - - // if the entry applies, - // if it's an exclude entry, exit, do not display the content app - if (!rule.Show) - return null; - - // else ok to display, remember type rules are ok - okType = true; - } - } - - // if roles rules are specified but not ok, - // or if type roles are specified but not ok, - // cannot display the content app - if ((hasRole && !okRole) || (hasType && !okType)) - return null; - - // else - // content app can be displayed - return _app ?? (_app = new ContentApp - { - Alias = Alias, - Name = Name, - Icon = Icon, - View = View, - Weight = Weight - }); - } - - private class ShowRule - { - private static readonly Regex ShowRegex = new Regex("^([+-])?([a-z]+)/([a-z0-9_]+|\\*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase); - - public bool Show { get; private set; } - public string PartA { get; private set; } - public string PartB { get; private set; } - - public bool Matches(string partA, string partB) - { - return (PartA == "*" || PartA.InvariantEquals(partA)) && (PartB == "*" || PartB.InvariantEquals(partB)); - } - - public static IEnumerable Parse(string[] rules) - { - foreach (var rule in rules) - { - var match = ShowRegex.Match(rule); - if (!match.Success) - throw new FormatException($"Illegal 'show' entry \"{rule}\" in manifest."); - - yield return new ShowRule - { - Show = match.Groups[1].Value != "-", - PartA = match.Groups[2].Value, - PartB = match.Groups[3].Value - }; - } - } - } } } diff --git a/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs b/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs new file mode 100644 index 0000000000..b44d31f0a5 --- /dev/null +++ b/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs @@ -0,0 +1,172 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; +using Umbraco.Core.Models; +using Umbraco.Core.Models.ContentEditing; +using Umbraco.Core.Models.Membership; + +namespace Umbraco.Core.Manifest +{ + // contentApps: [ + // { + // name: 'App Name', // required + // alias: 'appAlias', // required + // weight: 0, // optional, default is 0, use values between -99 and +99 + // icon: 'icon.app', // required + // view: 'path/view.htm', // required + // show: [ // optional, default is always show + // '-content/foo', // hide for content type 'foo' + // '+content/*', // show for all other content types + // '+media/*', // show for all media types + // '+role/admin' // show for admin users. Role based permissions will override others. + // ] + // }, + // ... + // ] + + /// + /// Represents a content app definition, parsed from a manifest. + /// + public class ManifestContentAppFactory : IContentAppFactory + { + private readonly ManifestContentAppDefinition _definition; + + + public ManifestContentAppFactory(ManifestContentAppDefinition definition) + { + _definition = definition; + } + + private ContentApp _app; + private ShowRule[] _showRules; + + /// + public ContentApp GetContentAppFor(object o,IEnumerable userGroups) + { + string partA, partB; + + switch (o) + { + case IContent content: + partA = "content"; + partB = content.ContentType.Alias; + break; + + case IMedia media: + partA = "media"; + partB = media.ContentType.Alias; + break; + + default: + return null; + } + + var rules = _showRules ?? (_showRules = ShowRule.Parse(_definition.Show).ToArray()); + var userGroupsList = userGroups.ToList(); + + var okRole = false; + var hasRole = false; + var okType = false; + var hasType = false; + + foreach (var rule in rules) + { + if (rule.PartA.InvariantEquals("role")) + { + // if roles have been ok-ed already, skip the rule + if (okRole) + continue; + + // remember we have role rules + hasRole = true; + + foreach (var group in userGroupsList) + { + // if the entry does not apply, skip + if (!rule.Matches("role", group.Alias)) + continue; + + // if the entry applies, + // if it's an exclude entry, exit, do not display the content app + if (!rule.Show) + return null; + + // else ok to display, remember roles are ok, break from userGroupsList + okRole = rule.Show; + break; + } + } + else // it is a type rule + { + // if type has been ok-ed already, skip the rule + if (okType) + continue; + + // remember we have type rules + hasType = true; + + // if the entry does not apply, skip it + if (!rule.Matches(partA, partB)) + continue; + + // if the entry applies, + // if it's an exclude entry, exit, do not display the content app + if (!rule.Show) + return null; + + // else ok to display, remember type rules are ok + okType = true; + } + } + + // if roles rules are specified but not ok, + // or if type roles are specified but not ok, + // cannot display the content app + if ((hasRole && !okRole) || (hasType && !okType)) + return null; + + // else + // content app can be displayed + return _app ?? (_app = new ContentApp + { + Alias = _definition.Alias, + Name = _definition.Name, + Icon = _definition.Icon, + View = _definition.View, + Weight = _definition.Weight + }); + } + + private class ShowRule + { + private static readonly Regex ShowRegex = new Regex("^([+-])?([a-z]+)/([a-z0-9_]+|\\*)$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + + public bool Show { get; private set; } + public string PartA { get; private set; } + public string PartB { get; private set; } + + public bool Matches(string partA, string partB) + { + return (PartA == "*" || PartA.InvariantEquals(partA)) && (PartB == "*" || PartB.InvariantEquals(partB)); + } + + public static IEnumerable Parse(string[] rules) + { + foreach (var rule in rules) + { + var match = ShowRegex.Match(rule); + if (!match.Success) + throw new FormatException($"Illegal 'show' entry \"{rule}\" in manifest."); + + yield return new ShowRule + { + Show = match.Groups[1].Value != "-", + PartA = match.Groups[2].Value, + PartB = match.Groups[3].Value + }; + } + } + } + } +} diff --git a/src/Umbraco.Core/Manifest/ManifestParser.cs b/src/Umbraco.Core/Manifest/ManifestParser.cs index fe021fae5b..2d6ec93e14 100644 --- a/src/Umbraco.Core/Manifest/ManifestParser.cs +++ b/src/Umbraco.Core/Manifest/ManifestParser.cs @@ -99,7 +99,7 @@ namespace Umbraco.Core.Manifest var propertyEditors = new List(); var parameterEditors = new List(); var gridEditors = new List(); - var contentApps = new List(); + var contentApps = new List(); var dashboards = new List(); foreach (var manifest in manifests) diff --git a/src/Umbraco.Core/Manifest/PackageManifest.cs b/src/Umbraco.Core/Manifest/PackageManifest.cs index 95a5c01b6a..cd806ac847 100644 --- a/src/Umbraco.Core/Manifest/PackageManifest.cs +++ b/src/Umbraco.Core/Manifest/PackageManifest.cs @@ -26,7 +26,7 @@ namespace Umbraco.Core.Manifest public GridEditor[] GridEditors { get; set; } = Array.Empty(); [JsonProperty("contentApps")] - public IContentAppDefinition[] ContentApps { get; set; } = Array.Empty(); + public ManifestContentAppDefinition[] ContentApps { get; set; } = Array.Empty(); [JsonProperty("dashboards")] public ManifestDashboardDefinition[] Dashboards { get; set; } = Array.Empty(); diff --git a/src/Umbraco.Core/Models/ContentEditing/IContentAppDefinition.cs b/src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs similarity index 92% rename from src/Umbraco.Core/Models/ContentEditing/IContentAppDefinition.cs rename to src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs index af83c5a2f5..144f1c4f84 100644 --- a/src/Umbraco.Core/Models/ContentEditing/IContentAppDefinition.cs +++ b/src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs @@ -1,13 +1,15 @@ using System.Collections.Generic; +using Umbraco.Core.Manifest; using Umbraco.Core.Models.Membership; namespace Umbraco.Core.Models.ContentEditing { + /// /// Represents a content app definition. /// - public interface IContentAppDefinition + public interface IContentAppFactory { /// /// Gets the content app for an object. diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 047f3f8cdd..c2b16f590a 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -336,6 +336,7 @@ + @@ -380,7 +381,7 @@ - + diff --git a/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs b/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs index eed0919149..016eb4113a 100644 --- a/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs +++ b/src/Umbraco.Tests/Manifest/ManifestContentAppTests.cs @@ -67,7 +67,8 @@ namespace Umbraco.Tests.Manifest private void AssertDefinition(object source, bool expected, string[] show, IReadOnlyUserGroup[] groups) { var definition = JsonConvert.DeserializeObject("{" + (show.Length == 0 ? "" : " \"show\": [" + string.Join(",", show.Select(x => "\"" + x + "\"")) + "] ") + "}"); - var app = definition.GetContentAppFor(source, groups); + var factory = new ManifestContentAppFactory(definition); + var app = factory.GetContentAppFor(source, groups); if (expected) Assert.IsNotNull(app); else diff --git a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs b/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs index 8b33fd1447..1d94c3f381 100644 --- a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs +++ b/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs @@ -5,15 +5,17 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.Logging; +using Umbraco.Core.Manifest; using Umbraco.Core.Models.Membership; namespace Umbraco.Web.ContentApps { - public class ContentAppDefinitionCollection : BuilderCollectionBase + public class ContentAppDefinitionCollection : BuilderCollectionBase { private readonly ILogger _logger; + private readonly IContentAppFactory _factory; - public ContentAppDefinitionCollection(IEnumerable items, ILogger logger) + public ContentAppDefinitionCollection(IEnumerable items, ILogger logger) : base(items) { _logger = logger; @@ -32,7 +34,10 @@ namespace Umbraco.Web.ContentApps public IEnumerable GetContentAppsFor(object o, IEnumerable userGroups=null) { var roles = GetCurrentUserGroups(); - var apps = this.Select(x => x.GetContentAppFor(o, roles)).WhereNotNull().OrderBy(x => x.Weight).ToList(); + + + var apps = Enumerable.Empty();// this.Select(x => x.GetContentAppFor(o, roles)).WhereNotNull().OrderBy(x => x.Weight).ToList(); + var aliases = new HashSet(); List dups = null; diff --git a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs b/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs index 267dd2d0e7..744785bacd 100644 --- a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs +++ b/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs @@ -5,14 +5,16 @@ using Umbraco.Core.Composing; using Umbraco.Core.Logging; using Umbraco.Core.Manifest; using Umbraco.Core.Models.ContentEditing; +using Umbraco.Core.Services; namespace Umbraco.Web.ContentApps { - public class ContentAppDefinitionCollectionBuilder : OrderedCollectionBuilderBase + public class ContentAppDefinitionCollectionBuilder : OrderedCollectionBuilderBase { public ContentAppDefinitionCollectionBuilder(IServiceContainer container) : base(container) - { } + { + } protected override ContentAppDefinitionCollectionBuilder This => this; @@ -25,14 +27,14 @@ namespace Umbraco.Web.ContentApps return new ContentAppDefinitionCollection(CreateItems(), logger); } - protected override IEnumerable CreateItems(params object[] args) + protected override IEnumerable CreateItems(params object[] args) { // get the manifest parser just-in-time - injecting it in the ctor would mean that // simply getting the builder in order to configure the collection, would require // its dependencies too, and that can create cycles or other oddities var manifestParser = Container.GetInstance(); - return base.CreateItems(args).Concat(manifestParser.Manifest.ContentApps); + return base.CreateItems(args).Concat(manifestParser.Manifest.ContentApps.Select(x=>new ManifestContentAppFactory(x))); } } } diff --git a/src/Umbraco.Web/ContentApps/ContentEditorContentAppDefinition.cs b/src/Umbraco.Web/ContentApps/ContentEditorContentAppFactory.cs similarity index 95% rename from src/Umbraco.Web/ContentApps/ContentEditorContentAppDefinition.cs rename to src/Umbraco.Web/ContentApps/ContentEditorContentAppFactory.cs index d54d1a44d4..b1d5d373c0 100644 --- a/src/Umbraco.Web/ContentApps/ContentEditorContentAppDefinition.cs +++ b/src/Umbraco.Web/ContentApps/ContentEditorContentAppFactory.cs @@ -6,7 +6,7 @@ using Umbraco.Core.Models.Membership; namespace Umbraco.Web.ContentApps { - internal class ContentEditorContentAppDefinition : IContentAppDefinition + internal class ContentEditorContentAppFactory : IContentAppFactory { // see note on ContentApp private const int Weight = -100; diff --git a/src/Umbraco.Web/ContentApps/ContentInfoContentAppDefinition.cs b/src/Umbraco.Web/ContentApps/ContentInfoContentAppFactory.cs similarity index 95% rename from src/Umbraco.Web/ContentApps/ContentInfoContentAppDefinition.cs rename to src/Umbraco.Web/ContentApps/ContentInfoContentAppFactory.cs index de490439ba..49be194349 100644 --- a/src/Umbraco.Web/ContentApps/ContentInfoContentAppDefinition.cs +++ b/src/Umbraco.Web/ContentApps/ContentInfoContentAppFactory.cs @@ -6,7 +6,7 @@ using Umbraco.Core.Models.Membership; namespace Umbraco.Web.ContentApps { - public class ContentInfoContentAppDefinition : IContentAppDefinition + public class ContentInfoContentAppFactory : IContentAppFactory { // see note on ContentApp private const int Weight = +100; diff --git a/src/Umbraco.Web/ContentApps/ListViewContentAppDefinition.cs b/src/Umbraco.Web/ContentApps/ListViewContentAppFactory.cs similarity index 96% rename from src/Umbraco.Web/ContentApps/ListViewContentAppDefinition.cs rename to src/Umbraco.Web/ContentApps/ListViewContentAppFactory.cs index 0e4c7a04b8..7421a55907 100644 --- a/src/Umbraco.Web/ContentApps/ListViewContentAppDefinition.cs +++ b/src/Umbraco.Web/ContentApps/ListViewContentAppFactory.cs @@ -9,7 +9,7 @@ using Umbraco.Web.Models.ContentEditing; namespace Umbraco.Web.ContentApps { - internal class ListViewContentAppDefinition : IContentAppDefinition + internal class ListViewContentAppFactory : IContentAppFactory { // see note on ContentApp private const int Weight = -666; @@ -17,7 +17,7 @@ namespace Umbraco.Web.ContentApps private readonly IDataTypeService _dataTypeService; private readonly PropertyEditorCollection _propertyEditors; - public ListViewContentAppDefinition(IDataTypeService dataTypeService, PropertyEditorCollection propertyEditors) + public ListViewContentAppFactory(IDataTypeService dataTypeService, PropertyEditorCollection propertyEditors) { _dataTypeService = dataTypeService; _propertyEditors = propertyEditors; diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 53ecddd015..1e7e24e903 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -234,7 +234,7 @@ namespace Umbraco.Web.Editors public ContentItemDisplay GetRecycleBin() { var apps = new List(); - apps.Add(ListViewContentAppDefinition.CreateContentApp(Services.DataTypeService, _propertyEditors, "recycleBin", "content", Core.Constants.DataTypes.DefaultMembersListView)); + apps.Add(ListViewContentAppFactory.CreateContentApp(Services.DataTypeService, _propertyEditors, "recycleBin", "content", Core.Constants.DataTypes.DefaultMembersListView)); apps[0].Active = true; var display = new ContentItemDisplay { diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index 9aebc11dc6..dd224dc551 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -99,7 +99,7 @@ namespace Umbraco.Web.Editors public MediaItemDisplay GetRecycleBin() { var apps = new List(); - apps.Add(ListViewContentAppDefinition.CreateContentApp(Services.DataTypeService, _propertyEditors, "recycleBin", "media", Core.Constants.DataTypes.DefaultMediaListView)); + apps.Add(ListViewContentAppFactory.CreateContentApp(Services.DataTypeService, _propertyEditors, "recycleBin", "media", Core.Constants.DataTypes.DefaultMediaListView)); apps[0].Active = true; var display = new MediaItemDisplay { diff --git a/src/Umbraco.Web/Editors/MemberController.cs b/src/Umbraco.Web/Editors/MemberController.cs index 6117db8857..ae02645afa 100644 --- a/src/Umbraco.Web/Editors/MemberController.cs +++ b/src/Umbraco.Web/Editors/MemberController.cs @@ -139,7 +139,7 @@ namespace Umbraco.Web.Editors var name = foundType != null ? foundType.Name : listName; var apps = new List(); - apps.Add(ListViewContentAppDefinition.CreateContentApp(Services.DataTypeService, _propertyEditors, listName, "member", Core.Constants.DataTypes.DefaultMembersListView)); + apps.Add(ListViewContentAppFactory.CreateContentApp(Services.DataTypeService, _propertyEditors, listName, "member", Core.Constants.DataTypes.DefaultMembersListView)); apps[0].Active = true; var display = new MemberListDisplay diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs index 97cebdb076..421b807c26 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs @@ -207,9 +207,9 @@ namespace Umbraco.Web.Runtime // register known content apps composition.Container.RegisterCollectionBuilder() - .Append() - .Append() - .Append(); + .Append() + .Append() + .Append(); } internal void Initialize( diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index c17536ab4d..371b34d71d 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -110,6 +110,7 @@ + @@ -155,9 +156,8 @@ - - - + + From 1aaa4a353ebfe902504832276ca247f1412a7d43 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 6 Dec 2018 11:02:35 +0100 Subject: [PATCH 2/8] Bugfix --- src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs b/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs index 1d94c3f381..484e8641a9 100644 --- a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs +++ b/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs @@ -36,7 +36,7 @@ namespace Umbraco.Web.ContentApps var roles = GetCurrentUserGroups(); - var apps = Enumerable.Empty();// this.Select(x => x.GetContentAppFor(o, roles)).WhereNotNull().OrderBy(x => x.Weight).ToList(); + var apps = this.Select(x => x.GetContentAppFor(o, roles)).WhereNotNull().OrderBy(x => x.Weight).ToList(); var aliases = new HashSet(); From d1ab98004859e9e13abd2c23dc2dbef21d4a861f Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 7 Dec 2018 13:44:41 +0100 Subject: [PATCH 3/8] Cleanup --- .../Web/Controllers/ContentControllerTests.cs | 18 +++++--------- src/Umbraco.Web/Editors/ContentController.cs | 24 ++++++------------- 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs index 26a7403dac..e28d11dc4e 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerTests.cs @@ -211,9 +211,8 @@ namespace Umbraco.Tests.Web.Controllers var contentServiceMock = Mock.Get(Current.Services.ContentService); contentServiceMock.Setup(x => x.GetById(123)).Returns(() => null); //do not find it - var publishedSnapshot = Mock.Of(); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); - var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + var usersController = new ContentController(propertyEditorCollection); Container.InjectProperties(usersController); return usersController; } @@ -239,9 +238,8 @@ namespace Umbraco.Tests.Web.Controllers var contentServiceMock = Mock.Get(Current.Services.ContentService); contentServiceMock.Setup(x => x.GetById(123)).Returns(() => GetMockedContent()); - var publishedSnapshot = Mock.Of(); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); - var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + var usersController = new ContentController(propertyEditorCollection); Container.InjectProperties(usersController); return usersController; } @@ -272,9 +270,8 @@ namespace Umbraco.Tests.Web.Controllers var contentServiceMock = Mock.Get(Current.Services.ContentService); contentServiceMock.Setup(x => x.GetById(123)).Returns(() => GetMockedContent()); - var publishedSnapshot = Mock.Of(); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); - var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + var usersController = new ContentController(propertyEditorCollection); Container.InjectProperties(usersController); return usersController; } @@ -311,9 +308,8 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.Save(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new OperationResult(OperationResultType.Success, new Core.Events.EventMessages())); //success - var publishedSnapshot = Mock.Of(); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); - var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + var usersController = new ContentController(propertyEditorCollection); Container.InjectProperties(usersController); return usersController; } @@ -344,9 +340,8 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.Save(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new OperationResult(OperationResultType.Success, new Core.Events.EventMessages())); //success - var publishedSnapshot = Mock.Of(); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); - var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + var usersController = new ContentController(propertyEditorCollection); Container.InjectProperties(usersController); return usersController; } @@ -381,9 +376,8 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.Save(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new OperationResult(OperationResultType.Success, new Core.Events.EventMessages())); //success - var publishedSnapshot = Mock.Of(); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty())); - var usersController = new ContentController(publishedSnapshot, propertyEditorCollection); + var usersController = new ContentController(propertyEditorCollection); Container.InjectProperties(usersController); return usersController; } diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index e330df1c45..892ecab557 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using System.Net; @@ -9,7 +8,6 @@ using System.Text; using System.Web.Http; using System.Web.Http.Controllers; using System.Web.Http.ModelBinding; -using System.Web.Http.ValueProviders; using AutoMapper; using Umbraco.Core; using Umbraco.Core.Logging; @@ -23,16 +21,11 @@ using Umbraco.Web.Mvc; using Umbraco.Web.WebApi; using Umbraco.Web.WebApi.Filters; using Umbraco.Core.Persistence.Querying; -using Umbraco.Web.PublishedCache; using Umbraco.Core.Events; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.Models.Validation; using Umbraco.Web.Composing; -using Umbraco.Web.Models; -using Umbraco.Web.WebServices; - using Constants = Umbraco.Core.Constants; -using Language = Umbraco.Web.Models.ContentEditing.Language; using Umbraco.Core.PropertyEditors; using Umbraco.Web.Actions; using Umbraco.Web.ContentApps; @@ -55,16 +48,13 @@ namespace Umbraco.Web.Editors [ContentControllerConfiguration] public class ContentController : ContentControllerBase { - private readonly IPublishedSnapshotService _publishedSnapshotService; private readonly PropertyEditorCollection _propertyEditors; private readonly Lazy> _allLangs; public object Domains { get; private set; } - public ContentController(IPublishedSnapshotService publishedSnapshotService, PropertyEditorCollection propertyEditors) + public ContentController(PropertyEditorCollection propertyEditors) { - if (publishedSnapshotService == null) throw new ArgumentNullException(nameof(publishedSnapshotService)); - _publishedSnapshotService = publishedSnapshotService; _propertyEditors = propertyEditors ?? throw new ArgumentNullException(nameof(propertyEditors)); _allLangs = new Lazy>(() => Services.LocalizationService.GetAllLanguages().ToDictionary(x => x.IsoCode, x => x, StringComparer.InvariantCultureIgnoreCase)); } @@ -352,7 +342,7 @@ namespace Umbraco.Web.Editors /// Gets an empty content item for the /// /// - /// + /// [OutgoingEditorModelEvent] public ContentItemDisplay GetEmpty(string contentTypeAlias, int parentId) { @@ -1083,7 +1073,7 @@ namespace Umbraco.Web.Editors ActionPublish.ActionLetter) == ContentPermissionsHelper.ContentAccess.Denied)) { denied.Add(c); - } + } } } noAccess = denied; @@ -1241,11 +1231,11 @@ namespace Umbraco.Web.Editors //Check if a mandatory language is missing from being published var mandatoryVariant = cultureVariants.First(x => x.Culture.InvariantEquals(culture)); - + var isPublished = contentItem.PersistedContent.Published && contentItem.PersistedContent.IsCulturePublished(culture); result.Add((mandatoryVariant, isPublished)); - var isPublishing = isPublished ? true : publishingCheck(mandatoryVariant); + var isPublishing = isPublished ? true : publishingCheck(mandatoryVariant); if (isPublished || isPublishing) continue; @@ -1880,7 +1870,7 @@ namespace Umbraco.Web.Editors case PublishResultType.SuccessPublish: case PublishResultType.SuccessPublishCulture: //these 2 belong to a single group - return PublishResultType.SuccessPublish; + return PublishResultType.SuccessPublish; case PublishResultType.FailedPublishAwaitingRelease: case PublishResultType.FailedPublishCultureAwaitingRelease: //these 2 belong to a single group @@ -1903,7 +1893,7 @@ namespace Umbraco.Web.Editors }); foreach (var status in statusGroup) - { + { switch (status.Key) { case PublishResultType.SuccessPublishAlready: From 1eee6dfb2fbeb8cfd174d46fe8e5f8c37918f1fc Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 7 Dec 2018 14:05:47 +0100 Subject: [PATCH 4/8] Cleanup --- .../Manifest/ContentAppDefinitionConverter.cs | 16 ---------------- .../Manifest/ManifestContentAppFactory.cs | 3 +-- src/Umbraco.Core/Manifest/ManifestParser.cs | 1 - .../Models/ContentEditing/IContentAppFactory.cs | 5 +---- src/Umbraco.Core/Umbraco.Core.csproj | 1 - src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 2 +- src/Umbraco.Web/CompositionExtensions.cs | 4 ++-- ...lection.cs => ContentAppFactoryCollection.cs} | 13 ++++--------- ....cs => ContentAppFactoryCollectionBuilder.cs} | 16 +++++++--------- .../Models/Mapping/ContentAppResolver.cs | 4 ++-- .../Models/Mapping/MediaAppResolver.cs | 4 ++-- src/Umbraco.Web/Runtime/WebRuntimeComponent.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 4 ++-- 13 files changed, 23 insertions(+), 52 deletions(-) delete mode 100644 src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs rename src/Umbraco.Web/ContentApps/{ContentAppDefinitionCollection.cs => ContentAppFactoryCollection.cs} (77%) rename src/Umbraco.Web/ContentApps/{ContentAppDefinitionCollectionBuilder.cs => ContentAppFactoryCollectionBuilder.cs} (63%) diff --git a/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs b/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs deleted file mode 100644 index dd167e06df..0000000000 --- a/src/Umbraco.Core/Manifest/ContentAppDefinitionConverter.cs +++ /dev/null @@ -1,16 +0,0 @@ -using System; -using Newtonsoft.Json.Linq; -using Umbraco.Core.Models.ContentEditing; -using Umbraco.Core.Serialization; - -namespace Umbraco.Core.Manifest -{ - /// - /// Implements a json read converter for . - /// - internal class ContentAppDefinitionConverter : JsonReadConverter - { - protected override ManifestContentAppDefinition Create(Type objectType, string path, JObject jObject) - => new ManifestContentAppDefinition(); - } -} diff --git a/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs b/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs index b44d31f0a5..1c50a4b895 100644 --- a/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs +++ b/src/Umbraco.Core/Manifest/ManifestContentAppFactory.cs @@ -26,13 +26,12 @@ namespace Umbraco.Core.Manifest // ] /// - /// Represents a content app definition, parsed from a manifest. + /// Represents a content app factory, for content apps parsed from the manifest. /// public class ManifestContentAppFactory : IContentAppFactory { private readonly ManifestContentAppDefinition _definition; - public ManifestContentAppFactory(ManifestContentAppDefinition definition) { _definition = definition; diff --git a/src/Umbraco.Core/Manifest/ManifestParser.cs b/src/Umbraco.Core/Manifest/ManifestParser.cs index 2d6ec93e14..59753df66a 100644 --- a/src/Umbraco.Core/Manifest/ManifestParser.cs +++ b/src/Umbraco.Core/Manifest/ManifestParser.cs @@ -153,7 +153,6 @@ namespace Umbraco.Core.Manifest var manifest = JsonConvert.DeserializeObject(text, new DataEditorConverter(_logger), new ValueValidatorConverter(_validators), - new ContentAppDefinitionConverter(), new DashboardAccessRuleConverter()); // scripts and stylesheets are raw string, must process here diff --git a/src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs b/src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs index 144f1c4f84..6b8d90d418 100644 --- a/src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs +++ b/src/Umbraco.Core/Models/ContentEditing/IContentAppFactory.cs @@ -1,13 +1,10 @@ using System.Collections.Generic; -using Umbraco.Core.Manifest; using Umbraco.Core.Models.Membership; namespace Umbraco.Core.Models.ContentEditing { - - /// - /// Represents a content app definition. + /// Represents a content app factory. /// public interface IContentAppFactory { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 6acda13392..a8361a237a 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -334,7 +334,6 @@ - diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index d80802b1cf..1eb222cd6a 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -213,7 +213,7 @@ namespace Umbraco.Tests.Testing Container.RegisterSingleton(); // register empty content apps collection - Container.RegisterCollectionBuilder(); + Container.RegisterCollectionBuilder(); } protected virtual void ComposeCacheHelper() diff --git a/src/Umbraco.Web/CompositionExtensions.cs b/src/Umbraco.Web/CompositionExtensions.cs index fe2ea4b8a7..3031410f19 100644 --- a/src/Umbraco.Web/CompositionExtensions.cs +++ b/src/Umbraco.Web/CompositionExtensions.cs @@ -40,8 +40,8 @@ namespace Umbraco.Core.Components /// /// The composition. /// - public static ContentAppDefinitionCollectionBuilder ContentApps(this Composition composition) - => composition.Container.GetInstance(); + public static ContentAppFactoryCollectionBuilder ContentApps(this Composition composition) + => composition.Container.GetInstance(); /// /// Gets the content finders collection builder. diff --git a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs b/src/Umbraco.Web/ContentApps/ContentAppFactoryCollection.cs similarity index 77% rename from src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs rename to src/Umbraco.Web/ContentApps/ContentAppFactoryCollection.cs index 484e8641a9..07987aea3e 100644 --- a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollection.cs +++ b/src/Umbraco.Web/ContentApps/ContentAppFactoryCollection.cs @@ -1,21 +1,18 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.Logging; -using Umbraco.Core.Manifest; using Umbraco.Core.Models.Membership; namespace Umbraco.Web.ContentApps { - public class ContentAppDefinitionCollection : BuilderCollectionBase + public class ContentAppFactoryCollection : BuilderCollectionBase { private readonly ILogger _logger; - private readonly IContentAppFactory _factory; - public ContentAppDefinitionCollection(IEnumerable items, ILogger logger) + public ContentAppFactoryCollection(IEnumerable items, ILogger logger) : base(items) { _logger = logger; @@ -35,10 +32,8 @@ namespace Umbraco.Web.ContentApps { var roles = GetCurrentUserGroups(); - var apps = this.Select(x => x.GetContentAppFor(o, roles)).WhereNotNull().OrderBy(x => x.Weight).ToList(); - var aliases = new HashSet(); List dups = null; @@ -55,7 +50,7 @@ namespace Umbraco.Web.ContentApps // dying is not user-friendly, so let's write to log instead, and wish people read logs... //throw new InvalidOperationException($"Duplicate content app aliases found: {string.Join(",", dups)}"); - _logger.Warn("Duplicate content app aliases found: {DuplicateAliases}", string.Join(",", dups)); + _logger.Warn("Duplicate content app aliases found: {DuplicateAliases}", string.Join(",", dups)); } return apps; diff --git a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs b/src/Umbraco.Web/ContentApps/ContentAppFactoryCollectionBuilder.cs similarity index 63% rename from src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs rename to src/Umbraco.Web/ContentApps/ContentAppFactoryCollectionBuilder.cs index 744785bacd..8dd3b6bc98 100644 --- a/src/Umbraco.Web/ContentApps/ContentAppDefinitionCollectionBuilder.cs +++ b/src/Umbraco.Web/ContentApps/ContentAppFactoryCollectionBuilder.cs @@ -5,26 +5,24 @@ using Umbraco.Core.Composing; using Umbraco.Core.Logging; using Umbraco.Core.Manifest; using Umbraco.Core.Models.ContentEditing; -using Umbraco.Core.Services; namespace Umbraco.Web.ContentApps { - public class ContentAppDefinitionCollectionBuilder : OrderedCollectionBuilderBase + public class ContentAppFactoryCollectionBuilder : OrderedCollectionBuilderBase { - public ContentAppDefinitionCollectionBuilder(IServiceContainer container) + public ContentAppFactoryCollectionBuilder(IServiceContainer container) : base(container) - { - } + { } - protected override ContentAppDefinitionCollectionBuilder This => this; + protected override ContentAppFactoryCollectionBuilder This => this; // need to inject dependencies in the collection, so override creation - public override ContentAppDefinitionCollection CreateCollection() + public override ContentAppFactoryCollection CreateCollection() { // get the logger just-in-time - see note below for manifest parser var logger = Container.GetInstance(); - return new ContentAppDefinitionCollection(CreateItems(), logger); + return new ContentAppFactoryCollection(CreateItems(), logger); } protected override IEnumerable CreateItems(params object[] args) @@ -34,7 +32,7 @@ namespace Umbraco.Web.ContentApps // its dependencies too, and that can create cycles or other oddities var manifestParser = Container.GetInstance(); - return base.CreateItems(args).Concat(manifestParser.Manifest.ContentApps.Select(x=>new ManifestContentAppFactory(x))); + return base.CreateItems(args).Concat(manifestParser.Manifest.ContentApps.Select(x => new ManifestContentAppFactory(x))); } } } diff --git a/src/Umbraco.Web/Models/Mapping/ContentAppResolver.cs b/src/Umbraco.Web/Models/Mapping/ContentAppResolver.cs index a199c7e60e..ee3e991f89 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentAppResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentAppResolver.cs @@ -11,9 +11,9 @@ namespace Umbraco.Web.Models.Mapping // maps ContentApps when mapping IContent to ContentItemDisplay internal class ContentAppResolver : IValueResolver> { - private readonly ContentAppDefinitionCollection _contentAppDefinitions; + private readonly ContentAppFactoryCollection _contentAppDefinitions; - public ContentAppResolver(ContentAppDefinitionCollection contentAppDefinitions) + public ContentAppResolver(ContentAppFactoryCollection contentAppDefinitions) { _contentAppDefinitions = contentAppDefinitions; } diff --git a/src/Umbraco.Web/Models/Mapping/MediaAppResolver.cs b/src/Umbraco.Web/Models/Mapping/MediaAppResolver.cs index caaaacc5f2..2e3afac549 100644 --- a/src/Umbraco.Web/Models/Mapping/MediaAppResolver.cs +++ b/src/Umbraco.Web/Models/Mapping/MediaAppResolver.cs @@ -11,9 +11,9 @@ namespace Umbraco.Web.Models.Mapping // maps ContentApps when mapping IMedia to MediaItemDisplay internal class MediaAppResolver : IValueResolver> { - private readonly ContentAppDefinitionCollection _contentAppDefinitions; + private readonly ContentAppFactoryCollection _contentAppDefinitions; - public MediaAppResolver(ContentAppDefinitionCollection contentAppDefinitions) + public MediaAppResolver(ContentAppFactoryCollection contentAppDefinitions) { _contentAppDefinitions = contentAppDefinitions; } diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs index f6d6d98050..2e1c934bc0 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs @@ -208,7 +208,7 @@ namespace Umbraco.Web.Runtime composition.Container.RegisterSingleton(); // register known content apps - composition.Container.RegisterCollectionBuilder() + composition.Container.RegisterCollectionBuilder() .Append() .Append() .Append(); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 67e34e313c..a00a8c69a6 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -156,8 +156,8 @@ - - + + From 6d898128d43a077c6cff0f060de978052fce5258 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 7 Dec 2018 15:32:02 +0100 Subject: [PATCH 5/8] Fix nuspec for Examine --- build/NuSpecs/UmbracoCms.Web.nuspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/NuSpecs/UmbracoCms.Web.nuspec b/build/NuSpecs/UmbracoCms.Web.nuspec index e9bd8ca6ea..adf090c69b 100644 --- a/build/NuSpecs/UmbracoCms.Web.nuspec +++ b/build/NuSpecs/UmbracoCms.Web.nuspec @@ -25,7 +25,7 @@ - + From 02617885b43747641aa01e9988e31fe4ac4d0fe4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 7 Dec 2018 12:06:25 +0100 Subject: [PATCH 6/8] Disable media tests that use Examine (no point) --- src/Umbraco.Tests/PublishedContent/PublishedMediaTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Umbraco.Tests/PublishedContent/PublishedMediaTests.cs b/src/Umbraco.Tests/PublishedContent/PublishedMediaTests.cs index 784f534af4..508a005663 100644 --- a/src/Umbraco.Tests/PublishedContent/PublishedMediaTests.cs +++ b/src/Umbraco.Tests/PublishedContent/PublishedMediaTests.cs @@ -111,6 +111,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void Ensure_Children_Sorted_With_Examine() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); @@ -138,6 +139,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void Do_Not_Find_In_Recycle_Bin() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); @@ -185,6 +187,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void Children_With_Examine() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); @@ -212,6 +215,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void Descendants_With_Examine() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); @@ -239,6 +243,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void DescendantsOrSelf_With_Examine() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); @@ -266,6 +271,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void Ancestors_With_Examine() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); @@ -291,6 +297,7 @@ namespace Umbraco.Tests.PublishedContent } [Test] + [Ignore("No point testing with Examine, should refactor this test.")] public void AncestorsOrSelf_With_Examine() { var rebuilder = IndexInitializer.GetMediaIndexRebuilder(Container.GetInstance(), IndexInitializer.GetMockMediaService()); From 0d83da93274a104426196b0904a9d118a6444d2d Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 7 Dec 2018 12:44:54 +0100 Subject: [PATCH 7/8] Fix some tests --- .../TestControllerActivatorBase.cs | 3 --- .../Testing/TestingTests/MockTests.cs | 1 - .../Web/Mvc/SurfaceControllerTests.cs | 10 +++++++--- src/Umbraco.Web/UmbracoHelper.cs | 14 +++----------- 4 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs index c65faf76c9..2cf64f04d1 100644 --- a/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs +++ b/src/Umbraco.Tests/TestHelpers/ControllerTesting/TestControllerActivatorBase.cs @@ -163,11 +163,8 @@ namespace Umbraco.Tests.TestHelpers.ControllerTesting var membershipHelper = new MembershipHelper(umbCtx, Mock.Of(), Mock.Of()); - var mockedTypedContent = Mock.Of(); - var umbHelper = new UmbracoHelper(umbCtx, Mock.Of(), - mockedTypedContent, Mock.Of(), Mock.Of(), Mock.Of(), diff --git a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs index d5f5778d1a..51855f7e19 100644 --- a/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs +++ b/src/Umbraco.Tests/Testing/TestingTests/MockTests.cs @@ -60,7 +60,6 @@ namespace Umbraco.Tests.Testing.TestingTests // ReSharper disable once UnusedVariable var helper = new UmbracoHelper(umbracoContext, Mock.Of(), - Mock.Of(), Mock.Of(), Mock.Of(), Mock.Of(), diff --git a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs index 81f338da87..dce975d0c4 100644 --- a/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/SurfaceControllerTests.cs @@ -103,6 +103,13 @@ namespace Umbraco.Tests.Web.Mvc { var publishedSnapshot = new Mock(); publishedSnapshot.Setup(x => x.Members).Returns(Mock.Of()); + var contentCache = new Mock(); + var content = new Mock(); + content.Setup(x => x.Id).Returns(2); + contentCache.Setup(x => x.GetById(It.IsAny())).Returns(content.Object); + var mediaCache = new Mock(); + publishedSnapshot.Setup(x => x.Content).Returns(contentCache.Object); + publishedSnapshot.Setup(x => x.Media).Returns(mediaCache.Object); var publishedSnapshotService = new Mock(); publishedSnapshotService.Setup(x => x.CreatePublishedSnapshot(It.IsAny())).Returns(publishedSnapshot.Object); var globalSettings = TestObjects.GetGlobalSettings(); @@ -121,9 +128,6 @@ namespace Umbraco.Tests.Web.Mvc var helper = new UmbracoHelper( umbracoContext, Mock.Of(), - Mock.Of(query => query.Content(It.IsAny()) == - //return mock of IPublishedContent for any call to GetById - Mock.Of(content => content.Id == 2)), Mock.Of(), Mock.Of(), Mock.Of(), diff --git a/src/Umbraco.Web/UmbracoHelper.cs b/src/Umbraco.Web/UmbracoHelper.cs index fbb739b5c2..3bfa433987 100644 --- a/src/Umbraco.Web/UmbracoHelper.cs +++ b/src/Umbraco.Web/UmbracoHelper.cs @@ -11,7 +11,6 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Services; using Umbraco.Core.Xml; using Umbraco.Web.Composing; -using Umbraco.Core.Cache; using Umbraco.Web.Routing; using Umbraco.Web.Security; @@ -28,7 +27,6 @@ namespace Umbraco.Web private readonly UmbracoContext _umbracoContext; private readonly IPublishedContent _currentPage; - private readonly IPublishedContentQuery _iQuery; private readonly ServiceContext _services; private IUmbracoComponentRenderer _componentRenderer; @@ -44,22 +42,18 @@ namespace Umbraco.Web /// /// For tests. internal UmbracoHelper(UmbracoContext umbracoContext, IPublishedContent content, - IPublishedContentQuery query, ITagQuery tagQuery, ICultureDictionary cultureDictionary, IUmbracoComponentRenderer componentRenderer, MembershipHelper membershipHelper, ServiceContext services) { - if (tagQuery == null) throw new ArgumentNullException(nameof(tagQuery)); - _umbracoContext = umbracoContext ?? throw new ArgumentNullException(nameof(umbracoContext)); - _tag = new TagQuery(tagQuery); + _tag = tagQuery ?? throw new ArgumentNullException(nameof(tagQuery)); _cultureDictionary = cultureDictionary ?? throw new ArgumentNullException(nameof(cultureDictionary)); _componentRenderer = componentRenderer ?? throw new ArgumentNullException(nameof(componentRenderer)); _membershipHelper = membershipHelper ?? throw new ArgumentNullException(nameof(membershipHelper)); _currentPage = content ?? throw new ArgumentNullException(nameof(content)); - _iQuery = query ?? throw new ArgumentNullException(nameof(query)); _services = services ?? throw new ArgumentNullException(nameof(services)); } @@ -105,15 +99,13 @@ namespace Umbraco.Web /// Gets the tag context. /// public ITagQuery TagQuery => _tag ?? - (_tag = new TagQuery(_services.TagService, _iQuery ?? ContentQuery)); + (_tag = new TagQuery(_services.TagService, ContentQuery)); /// /// Gets the query context. /// public IPublishedContentQuery ContentQuery => _query ?? - (_query = _iQuery != null - ? new PublishedContentQuery(_iQuery) - : new PublishedContentQuery(UmbracoContext.ContentCache, UmbracoContext.MediaCache)); + (_query = new PublishedContentQuery(UmbracoContext.ContentCache, UmbracoContext.MediaCache)); /// /// Gets the Umbraco context. From bd91706f898d6b5a24964741fee094980a936f69 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 7 Dec 2018 13:01:18 +0100 Subject: [PATCH 8/8] Cleanup test --- src/Umbraco.Tests/UmbracoExamine/IndexTest.cs | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs b/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs index 78bdb37cae..29abfb9234 100644 --- a/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs +++ b/src/Umbraco.Tests/UmbracoExamine/IndexTest.cs @@ -187,21 +187,21 @@ namespace Umbraco.Tests.UmbracoExamine [Test] public void Index_Move_Media_From_Non_Indexable_To_Indexable_ParentID() { + // create a validator with + // publishedValuesOnly false + // parentId 1116 (only content under that parent will be indexed) + var validator = new ContentValueSetValidator(false, 1116); using (var luceneDir = new RandomIdRamDirectory()) - using (var indexer = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir, - //make parent id 1116 - validator: new ContentValueSetValidator(false, 1116))) + using (var indexer = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir, validator: validator)) using (indexer.ProcessNonAsync()) { var searcher = indexer.GetSearcher(); //get a node from the data repo (this one exists underneath 2222) var node = _mediaService.GetLatestMediaByXpath("//*[string-length(@id)>0 and number(@id)>0]") - .Root - .Elements() - .Where(x => (int)x.Attribute("id") == 2112) - .First(); + .Root.Elements() + .First(x => (int) x.Attribute("id") == 2112); var currPath = (string)node.Attribute("path"); //should be : -1,1111,2222,2112 Assert.AreEqual("-1,1111,2222,2112", currPath); @@ -230,20 +230,21 @@ namespace Umbraco.Tests.UmbracoExamine [Test] public void Index_Move_Media_To_Non_Indexable_ParentID() { + // create a validator with + // publishedValuesOnly false + // parentId 2222 (only content under that parent will be indexed) + var validator = new ContentValueSetValidator(false, 2222); + using (var luceneDir = new RandomIdRamDirectory()) - using (var indexer1 = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir, - //make parent id 2222 - validator: new ContentValueSetValidator(false, 2222))) + using (var indexer1 = IndexInitializer.GetUmbracoIndexer(ProfilingLogger, luceneDir, validator: validator)) using (indexer1.ProcessNonAsync()) { var searcher = indexer1.GetSearcher(); //get a node from the data repo (this one exists underneath 2222) var node = _mediaService.GetLatestMediaByXpath("//*[string-length(@id)>0 and number(@id)>0]") - .Root - .Elements() - .Where(x => (int)x.Attribute("id") == 2112) - .First(); + .Root.Elements() + .First(x => (int) x.Attribute("id") == 2112); var currPath = (string)node.Attribute("path"); //should be : -1,1111,2222,2112 Assert.AreEqual("-1,1111,2222,2112", currPath); @@ -251,8 +252,6 @@ namespace Umbraco.Tests.UmbracoExamine //ensure it's indexed indexer1.IndexItem(node.ConvertToValueSet(IndexTypes.Media)); - - //it will exist because it exists under 2222 var results = searcher.Search(searcher.CreateCriteria().Id(2112).Compile()); Assert.AreEqual(1, results.Count()); @@ -264,8 +263,6 @@ namespace Umbraco.Tests.UmbracoExamine //now reindex the node, this should first delete it and then NOT add it because of the parent id constraint indexer1.IndexItems(new[] { node.ConvertToValueSet(IndexTypes.Media) }); - - //now ensure it's deleted results = searcher.Search(searcher.CreateCriteria().Id(2112).Compile()); Assert.AreEqual(0, results.Count());