From 5cb55855747359e54eadfe23a7ced8657c6a2aa2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 19 Feb 2020 15:38:22 +1100 Subject: [PATCH 1/5] Removes ICurrentUserAccessor since we don't need it, fixes editorconfig (for now) --- .editorconfig | 2 +- .../ContentAppFactoryCollection.cs | 9 ++++---- .../ContentAppFactoryCollectionBuilder.cs | 4 ++-- .../Models/Identity/ICurrentUserAccessor.cs | 13 ------------ .../BackOfficeExamineSearcher.cs | 9 ++++---- .../Compose/NotificationsComponent.cs | 10 ++++----- src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 1 - src/Umbraco.Web/Editors/TourController.cs | 5 +---- .../Models/Mapping/CommonMapper.cs | 6 ++---- src/Umbraco.Web/Runtime/WebInitialComposer.cs | 1 - .../Security/CurrentUserAccessor.cs | 21 ------------------- src/Umbraco.Web/Umbraco.Web.csproj | 1 - 12 files changed, 19 insertions(+), 63 deletions(-) delete mode 100644 src/Umbraco.Abstractions/Models/Identity/ICurrentUserAccessor.cs delete mode 100644 src/Umbraco.Web/Security/CurrentUserAccessor.cs diff --git a/.editorconfig b/.editorconfig index 5d270b1db0..29ad74b4d6 100644 --- a/.editorconfig +++ b/.editorconfig @@ -25,7 +25,7 @@ dotnet_naming_rule.private_members_with_underscore.severity = suggestion dotnet_naming_symbols.private_fields.applicable_kinds = field dotnet_naming_symbols.private_fields.applicable_accessibilities = private -dotnet_naming_symbols.private_fields.required_modifiers = abstract,async,readonly,static # all except const +# dotnet_naming_symbols.private_fields.required_modifiers = abstract,async,readonly,static # all except const dotnet_naming_style.prefix_underscore.capitalization = camel_case dotnet_naming_style.prefix_underscore.required_prefix = _ diff --git a/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollection.cs b/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollection.cs index f8f57ce4fd..4e85e51af3 100644 --- a/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollection.cs +++ b/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollection.cs @@ -4,7 +4,6 @@ using Umbraco.Core; using Umbraco.Core.Composing; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.Logging; -using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; namespace Umbraco.Web.ContentApps @@ -12,18 +11,18 @@ namespace Umbraco.Web.ContentApps public class ContentAppFactoryCollection : BuilderCollectionBase { private readonly ILogger _logger; - private readonly ICurrentUserAccessor _currentUserAccessor; + private readonly IUmbracoContextAccessor _umbracoContextAccessor; - public ContentAppFactoryCollection(IEnumerable items, ILogger logger, ICurrentUserAccessor currentUserAccessor) + public ContentAppFactoryCollection(IEnumerable items, ILogger logger, IUmbracoContextAccessor umbracoContextAccessor) : base(items) { _logger = logger; - _currentUserAccessor = currentUserAccessor; + _umbracoContextAccessor = umbracoContextAccessor; } private IEnumerable GetCurrentUserGroups() { - var currentUser = _currentUserAccessor.TryGetCurrentUser(); + var currentUser = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser; return currentUser == null ? Enumerable.Empty() : currentUser.Groups; diff --git a/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollectionBuilder.cs b/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollectionBuilder.cs index 0e21823b90..f3d3a149d5 100644 --- a/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollectionBuilder.cs +++ b/src/Umbraco.Abstractions/ContentApps/ContentAppFactoryCollectionBuilder.cs @@ -19,8 +19,8 @@ namespace Umbraco.Web.ContentApps { // get the logger just-in-time - see note below for manifest parser var logger = factory.GetInstance(); - var currentUserAccessor = factory.GetInstance(); - return new ContentAppFactoryCollection(CreateItems(factory), logger, currentUserAccessor); + var umbracoContextAccessor = factory.GetInstance(); + return new ContentAppFactoryCollection(CreateItems(factory), logger, umbracoContextAccessor); } protected override IEnumerable CreateItems(IFactory factory) diff --git a/src/Umbraco.Abstractions/Models/Identity/ICurrentUserAccessor.cs b/src/Umbraco.Abstractions/Models/Identity/ICurrentUserAccessor.cs deleted file mode 100644 index 49b2c6d9f9..0000000000 --- a/src/Umbraco.Abstractions/Models/Identity/ICurrentUserAccessor.cs +++ /dev/null @@ -1,13 +0,0 @@ -using Umbraco.Core.Models.Membership; - -namespace Umbraco.Core.Models.Identity -{ - public interface ICurrentUserAccessor - { - /// - /// Returns the current user or null if no user is currently authenticated. - /// - /// The current user or null - IUser TryGetCurrentUser(); - } -} diff --git a/src/Umbraco.Examine.Lucene/BackOfficeExamineSearcher.cs b/src/Umbraco.Examine.Lucene/BackOfficeExamineSearcher.cs index cfa9de5fa6..d73dc89948 100644 --- a/src/Umbraco.Examine.Lucene/BackOfficeExamineSearcher.cs +++ b/src/Umbraco.Examine.Lucene/BackOfficeExamineSearcher.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Models.Identity; using Umbraco.Core.Persistence; using Umbraco.Core.Services; +using Umbraco.Web; using Umbraco.Web.Models.ContentEditing; namespace Umbraco.Examine @@ -17,19 +18,19 @@ namespace Umbraco.Examine { private readonly IExamineManager _examineManager; private readonly ILocalizationService _languageService; - private readonly ICurrentUserAccessor _currentUserAccessor; + private readonly IUmbracoContextAccessor _umbracoContextAccessor; private readonly IEntityService _entityService; private readonly IUmbracoTreeSearcherFields _treeSearcherFields; public BackOfficeExamineSearcher(IExamineManager examineManager, ILocalizationService languageService, - ICurrentUserAccessor currentUserAccessor, + IUmbracoContextAccessor umbracoContextAccessor, IEntityService entityService, IUmbracoTreeSearcherFields treeSearcherFields) { _examineManager = examineManager; _languageService = languageService; - _currentUserAccessor = currentUserAccessor; + _umbracoContextAccessor = umbracoContextAccessor; _entityService = entityService; _treeSearcherFields = treeSearcherFields; } @@ -48,7 +49,7 @@ namespace Umbraco.Examine query = "\"" + g.ToString() + "\""; } - var currentUser = _currentUserAccessor.TryGetCurrentUser(); + var currentUser = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser; switch (entityType) { diff --git a/src/Umbraco.Infrastructure/Compose/NotificationsComponent.cs b/src/Umbraco.Infrastructure/Compose/NotificationsComponent.cs index 41fc097ff3..b39e5c03e9 100644 --- a/src/Umbraco.Infrastructure/Compose/NotificationsComponent.cs +++ b/src/Umbraco.Infrastructure/Compose/NotificationsComponent.cs @@ -148,13 +148,12 @@ namespace Umbraco.Web.Compose /// public sealed class Notifier { - private readonly ICurrentUserAccessor _currentUserAccessor; + private readonly IUmbracoContextAccessor _umbracoContextAccessor; private readonly IRuntimeState _runtimeState; private readonly INotificationService _notificationService; private readonly IUserService _userService; private readonly ILocalizedTextService _textService; private readonly IGlobalSettings _globalSettings; - private readonly IContentSection _contentConfig; private readonly ILogger _logger; /// @@ -167,21 +166,20 @@ namespace Umbraco.Web.Compose /// /// /// - public Notifier(ICurrentUserAccessor currentUserAccessor, IRuntimeState runtimeState, INotificationService notificationService, IUserService userService, ILocalizedTextService textService, IGlobalSettings globalSettings, IContentSection contentConfig, ILogger logger) + public Notifier(IUmbracoContextAccessor umbracoContextAccessor, IRuntimeState runtimeState, INotificationService notificationService, IUserService userService, ILocalizedTextService textService, IGlobalSettings globalSettings, ILogger logger) { - _currentUserAccessor = currentUserAccessor; + _umbracoContextAccessor = umbracoContextAccessor; _runtimeState = runtimeState; _notificationService = notificationService; _userService = userService; _textService = textService; _globalSettings = globalSettings; - _contentConfig = contentConfig; _logger = logger; } public void Notify(IAction action, params IContent[] entities) { - var user = _currentUserAccessor.TryGetCurrentUser(); + var user = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser; //if there is no current user, then use the admin if (user == null) diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index f10c18492a..b0cde2b905 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -199,7 +199,6 @@ namespace Umbraco.Tests.Testing Composition.RegisterUnique(backOfficeInfo); Composition.RegisterUnique(ipResolver); Composition.RegisterUnique(); - Composition.RegisterUnique(); Composition.RegisterUnique(TestHelper.ShortStringHelper); diff --git a/src/Umbraco.Web/Editors/TourController.cs b/src/Umbraco.Web/Editors/TourController.cs index c5be44b809..c3b9732807 100644 --- a/src/Umbraco.Web/Editors/TourController.cs +++ b/src/Umbraco.Web/Editors/TourController.cs @@ -27,7 +27,6 @@ namespace Umbraco.Web.Editors private readonly TourFilterCollection _filters; private readonly IUmbracoSettingsSection _umbracoSettingsSection; private readonly IIOHelper _ioHelper; - private readonly ICurrentUserAccessor _currentUserAccessor; public TourController( IGlobalSettings globalSettings, @@ -43,14 +42,12 @@ namespace Umbraco.Web.Editors TourFilterCollection filters, IUmbracoSettingsSection umbracoSettingsSection, IIOHelper ioHelper, - ICurrentUserAccessor currentUserAccessor, IPublishedUrlProvider publishedUrlProvider) : base(globalSettings, umbracoContextAccessor, sqlContext, services, appCaches, logger, runtimeState, umbracoHelper, shortStringHelper, umbracoMapper, publishedUrlProvider) { _filters = filters; _umbracoSettingsSection = umbracoSettingsSection ?? throw new ArgumentNullException(nameof(umbracoSettingsSection)); _ioHelper = ioHelper; - _currentUserAccessor = currentUserAccessor; } public IEnumerable GetTours() @@ -60,7 +57,7 @@ namespace Umbraco.Web.Editors if (_umbracoSettingsSection.BackOffice.Tours.EnableTours == false) return result; - var user = _currentUserAccessor.TryGetCurrentUser(); + var user = UmbracoContext.Security.CurrentUser; if (user == null) return result; diff --git a/src/Umbraco.Web/Models/Mapping/CommonMapper.cs b/src/Umbraco.Web/Models/Mapping/CommonMapper.cs index f8b99a444c..748ffe07f9 100644 --- a/src/Umbraco.Web/Models/Mapping/CommonMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/CommonMapper.cs @@ -25,10 +25,9 @@ namespace Umbraco.Web.Models.Mapping private readonly ContentAppFactoryCollection _contentAppDefinitions; private readonly ILocalizedTextService _localizedTextService; private readonly IHttpContextAccessor _httpContextAccessor; - private readonly ICurrentUserAccessor _currentUserAccessor; public CommonMapper(IUserService userService, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, IUmbracoContextAccessor umbracoContextAccessor, - ContentAppFactoryCollection contentAppDefinitions, ILocalizedTextService localizedTextService, IHttpContextAccessor httpContextAccessor, ICurrentUserAccessor currentUserAccessor) + ContentAppFactoryCollection contentAppDefinitions, ILocalizedTextService localizedTextService, IHttpContextAccessor httpContextAccessor) { _userService = userService; _contentTypeBaseServiceProvider = contentTypeBaseServiceProvider; @@ -36,7 +35,6 @@ namespace Umbraco.Web.Models.Mapping _contentAppDefinitions = contentAppDefinitions; _localizedTextService = localizedTextService; _httpContextAccessor = httpContextAccessor; - _currentUserAccessor = currentUserAccessor; } public UserProfile GetOwner(IContentBase source, MapperContext context) @@ -54,7 +52,7 @@ namespace Umbraco.Web.Models.Mapping public ContentTypeBasic GetContentType(IContentBase source, MapperContext context) { - var user = _currentUserAccessor.TryGetCurrentUser(); + var user = _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser; if (user?.AllowedSections.Any(x => x.Equals(Constants.Applications.Settings)) ?? false) { var contentType = _contentTypeBaseServiceProvider.GetContentTypeOf(source); diff --git a/src/Umbraco.Web/Runtime/WebInitialComposer.cs b/src/Umbraco.Web/Runtime/WebInitialComposer.cs index c36e1fc597..e7ae1ad7ba 100644 --- a/src/Umbraco.Web/Runtime/WebInitialComposer.cs +++ b/src/Umbraco.Web/Runtime/WebInitialComposer.cs @@ -68,7 +68,6 @@ namespace Umbraco.Web.Runtime composition.Register(); composition.Register(); composition.Register(); - composition.Register(); composition.Register(Lifetime.Singleton); composition.RegisterUnique(); // required for hybrid accessors diff --git a/src/Umbraco.Web/Security/CurrentUserAccessor.cs b/src/Umbraco.Web/Security/CurrentUserAccessor.cs deleted file mode 100644 index 3d914be77f..0000000000 --- a/src/Umbraco.Web/Security/CurrentUserAccessor.cs +++ /dev/null @@ -1,21 +0,0 @@ -using Umbraco.Core.Models.Identity; -using Umbraco.Core.Models.Membership; - -namespace Umbraco.Web.Security -{ - internal class CurrentUserAccessor : ICurrentUserAccessor - { - private readonly IUmbracoContextAccessor _umbracoContextAccessor; - - public CurrentUserAccessor(IUmbracoContextAccessor umbracoContextAccessor) - { - _umbracoContextAccessor = umbracoContextAccessor; - } - - /// - public IUser TryGetCurrentUser() - { - return _umbracoContextAccessor.UmbracoContext?.Security?.CurrentUser; - } - } -} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 0b0e6b6fee..388b5b10dc 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -216,7 +216,6 @@ - From 04c620d76e40785ecb3e1ac4606bebe0d1fdd550 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 19 Feb 2020 15:46:54 +1100 Subject: [PATCH 2/5] Reverts how UmbracoContextReference works --- src/Umbraco.Abstractions/UmbracoContextReference.cs | 8 +++++--- src/Umbraco.Web/UmbracoContextFactory.cs | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Abstractions/UmbracoContextReference.cs b/src/Umbraco.Abstractions/UmbracoContextReference.cs index 3cd97cd77b..bd021494e6 100644 --- a/src/Umbraco.Abstractions/UmbracoContextReference.cs +++ b/src/Umbraco.Abstractions/UmbracoContextReference.cs @@ -14,22 +14,24 @@ namespace Umbraco.Web /// public class UmbracoContextReference : IDisposable //fixme - should we inherit from DisposableObjectSlim? { + private readonly IUmbracoContextAccessor _umbracoContextAccessor; private bool _disposed; /// /// Initializes a new instance of the class. /// - internal UmbracoContextReference(bool isRoot, IUmbracoContext umbracoContext) + internal UmbracoContextReference(IUmbracoContext umbracoContext, bool isRoot, IUmbracoContextAccessor umbracoContextAccessor) { IsRoot = isRoot; UmbracoContext = umbracoContext; + _umbracoContextAccessor = umbracoContextAccessor; } /// /// Gets the . /// - public IUmbracoContext UmbracoContext { get; private set; } + public IUmbracoContext UmbracoContext { get; } /// /// Gets a value indicating whether the reference is a root reference. @@ -49,7 +51,7 @@ namespace Umbraco.Web if (IsRoot) { UmbracoContext.Dispose(); - UmbracoContext = null; + _umbracoContextAccessor.UmbracoContext = null; } GC.SuppressFinalize(this); diff --git a/src/Umbraco.Web/UmbracoContextFactory.cs b/src/Umbraco.Web/UmbracoContextFactory.cs index d627d9e823..a84e51eda3 100644 --- a/src/Umbraco.Web/UmbracoContextFactory.cs +++ b/src/Umbraco.Web/UmbracoContextFactory.cs @@ -77,13 +77,13 @@ namespace Umbraco.Web { var currentUmbracoContext = _umbracoContextAccessor.UmbracoContext; if (currentUmbracoContext != null) - return new UmbracoContextReference(false, currentUmbracoContext); + return new UmbracoContextReference(currentUmbracoContext, false, _umbracoContextAccessor); var umbracoContext = CreateUmbracoContext(); _umbracoContextAccessor.UmbracoContext = umbracoContext; - return new UmbracoContextReference(true, umbracoContext); + return new UmbracoContextReference(umbracoContext, true, _umbracoContextAccessor); } // dummy TextWriter that does not write From 13a56eb08549f196eeb8039b656c24cf91c452aa Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 19 Feb 2020 16:37:00 +1100 Subject: [PATCH 3/5] Renames and refactors the MediaUrlGeneratorCollection and ensure that this collection is used wherever IMediaUrlGenerator is needed instead of relying property editors possibly being the implementation. --- .../CompositionExtensions.cs | 4 +-- .../Models/IDataEditorWithMediaPath.cs | 21 -------------- .../Models/IMediaUrlGenerator.cs | 18 ++++++++++++ .../DataEditorWithMediaPathCollection.cs | 21 -------------- ...ataEditorWithMediaPathCollectionBuilder.cs | 9 ------ .../MediaUrlGeneratorCollection.cs | 28 +++++++++++++++++++ .../MediaUrlGeneratorCollectionBuilder.cs | 9 ++++++ .../Routing/DefaultMediaUrlProvider.cs | 21 +++++++------- .../Models/MediaExtensions.cs | 18 +++++------- .../Factories/ContentBaseFactory.cs | 12 ++++---- .../Repositories/Implement/MediaRepository.cs | 7 +++-- .../DataValueReferenceFactoryCollection.cs | 3 ++ .../FileUploadPropertyEditor.cs | 13 +++++++-- .../ImageCropperPropertyEditor.cs | 13 +++++++-- .../Runtime/CoreInitialComposer.cs | 2 +- .../Repositories/MediaRepositoryTest.cs | 3 +- .../Repositories/TagRepositoryTest.cs | 3 +- .../Repositories/UserRepositoryTest.cs | 3 +- .../Routing/MediaUrlProviderTests.cs | 2 +- .../Models/Mapping/MediaMapDefinition.cs | 12 ++++---- 20 files changed, 122 insertions(+), 100 deletions(-) delete mode 100644 src/Umbraco.Abstractions/Models/IDataEditorWithMediaPath.cs create mode 100644 src/Umbraco.Abstractions/Models/IMediaUrlGenerator.cs delete mode 100644 src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollection.cs delete mode 100644 src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollectionBuilder.cs create mode 100644 src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollection.cs create mode 100644 src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollectionBuilder.cs diff --git a/src/Umbraco.Abstractions/CompositionExtensions.cs b/src/Umbraco.Abstractions/CompositionExtensions.cs index 24db8653d1..bea78f82ed 100644 --- a/src/Umbraco.Abstractions/CompositionExtensions.cs +++ b/src/Umbraco.Abstractions/CompositionExtensions.cs @@ -29,8 +29,8 @@ namespace Umbraco.Core /// /// The composition. /// - public static DataEditorWithMediaPathCollectionBuilder DataEditorsWithMediaPath(this Composition composition) - => composition.WithCollectionBuilder(); + public static MediaUrlGeneratorCollectionBuilder MediaUrlGenerators(this Composition composition) + => composition.WithCollectionBuilder(); #endregion } diff --git a/src/Umbraco.Abstractions/Models/IDataEditorWithMediaPath.cs b/src/Umbraco.Abstractions/Models/IDataEditorWithMediaPath.cs deleted file mode 100644 index c0ad0fe199..0000000000 --- a/src/Umbraco.Abstractions/Models/IDataEditorWithMediaPath.cs +++ /dev/null @@ -1,21 +0,0 @@ -namespace Umbraco.Core.PropertyEditors -{ - /// - /// Must be implemented by property editors that store media and return media paths - /// - /// - /// Currently there are only 2x core editors that do this: upload and image cropper. - /// It would be possible for developers to know implement their own media property editors whereas previously this was not possible. - /// - public interface IDataEditorWithMediaPath - { - string Alias { get; } - - /// - /// Returns the media path for the value stored for a property - /// - /// - /// - string GetMediaPath(object value); - } -} diff --git a/src/Umbraco.Abstractions/Models/IMediaUrlGenerator.cs b/src/Umbraco.Abstractions/Models/IMediaUrlGenerator.cs new file mode 100644 index 0000000000..41e1be8d6c --- /dev/null +++ b/src/Umbraco.Abstractions/Models/IMediaUrlGenerator.cs @@ -0,0 +1,18 @@ +namespace Umbraco.Core.PropertyEditors +{ + /// + /// Used to generate paths to media items for a specified property editor alias + /// + public interface IMediaUrlGenerator + { + /// + /// Tries to get a media path for a given property editor alias + /// + /// The property editor alias + /// The value of the property + /// + /// True if a media path was returned + /// + bool TryGetMediaPath(string alias, object value, out string mediaPath); + } +} diff --git a/src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollection.cs b/src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollection.cs deleted file mode 100644 index de93237661..0000000000 --- a/src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollection.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using Umbraco.Core.Composing; - -namespace Umbraco.Core.PropertyEditors -{ - public class DataEditorWithMediaPathCollection : BuilderCollectionBase - { - public DataEditorWithMediaPathCollection(IEnumerable items) : base(items) - { - } - - public bool TryGet(string alias, out IDataEditorWithMediaPath editor) - { - editor = this.FirstOrDefault(x => x.Alias == alias); - return editor != null; - } - - - } -} diff --git a/src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollectionBuilder.cs b/src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollectionBuilder.cs deleted file mode 100644 index ddb656720d..0000000000 --- a/src/Umbraco.Abstractions/PropertyEditors/DataEditorWithMediaPathCollectionBuilder.cs +++ /dev/null @@ -1,9 +0,0 @@ -using Umbraco.Core.Composing; - -namespace Umbraco.Core.PropertyEditors -{ - public class DataEditorWithMediaPathCollectionBuilder : LazyCollectionBuilderBase - { - protected override DataEditorWithMediaPathCollectionBuilder This => this; - } -} diff --git a/src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollection.cs b/src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollection.cs new file mode 100644 index 0000000000..0374ca5cd2 --- /dev/null +++ b/src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollection.cs @@ -0,0 +1,28 @@ +using System.Collections.Generic; +using Umbraco.Core.Composing; + +namespace Umbraco.Core.PropertyEditors +{ + public class MediaUrlGeneratorCollection : BuilderCollectionBase + { + public MediaUrlGeneratorCollection(IEnumerable items) : base(items) + { + } + + public bool TryGetMediaPath(string alias, object value, out string mediaPath) + { + foreach(var generator in this) + { + if (generator.TryGetMediaPath(alias, value, out var mp)) + { + mediaPath = mp; + return true; + } + } + mediaPath = null; + return false; + } + + + } +} diff --git a/src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollectionBuilder.cs b/src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollectionBuilder.cs new file mode 100644 index 0000000000..40a68fd4e3 --- /dev/null +++ b/src/Umbraco.Abstractions/PropertyEditors/MediaUrlGeneratorCollectionBuilder.cs @@ -0,0 +1,9 @@ +using Umbraco.Core.Composing; + +namespace Umbraco.Core.PropertyEditors +{ + public class MediaUrlGeneratorCollectionBuilder : LazyCollectionBuilderBase + { + protected override MediaUrlGeneratorCollectionBuilder This => this; + } +} diff --git a/src/Umbraco.Abstractions/Routing/DefaultMediaUrlProvider.cs b/src/Umbraco.Abstractions/Routing/DefaultMediaUrlProvider.cs index 72128c21f5..3e6413ff90 100644 --- a/src/Umbraco.Abstractions/Routing/DefaultMediaUrlProvider.cs +++ b/src/Umbraco.Abstractions/Routing/DefaultMediaUrlProvider.cs @@ -10,11 +10,11 @@ namespace Umbraco.Web.Routing public class DefaultMediaUrlProvider : IMediaUrlProvider { private readonly UriUtility _uriUtility; - private readonly DataEditorWithMediaPathCollection _dataEditors; + private readonly MediaUrlGeneratorCollection _mediaPathGenerators; - public DefaultMediaUrlProvider(DataEditorWithMediaPathCollection dataEditors, UriUtility uriUtility) + public DefaultMediaUrlProvider(MediaUrlGeneratorCollection mediaPathGenerators, UriUtility uriUtility) { - _dataEditors = dataEditors ?? throw new ArgumentNullException(nameof(dataEditors)); + _mediaPathGenerators = mediaPathGenerators ?? throw new ArgumentNullException(nameof(mediaPathGenerators)); _uriUtility = uriUtility; } @@ -32,24 +32,23 @@ namespace Umbraco.Web.Routing } var propType = prop.PropertyType; - string path = null; - if (_dataEditors.TryGet(propType.EditorAlias, out var dataEditor)) + if (_mediaPathGenerators.TryGetMediaPath(propType.EditorAlias, value, out var path)) { - path = dataEditor.GetMediaPath(value); + var url = AssembleUrl(path, current, mode); + return UrlInfo.Url(url.ToString(), culture); } - var url = AssembleUrl(path, current, mode); - return url == null ? null : UrlInfo.Url(url.ToString(), culture); + return null; } private Uri AssembleUrl(string path, Uri current, UrlMode mode) { - if (string.IsNullOrEmpty(path)) - return null; + if (string.IsNullOrWhiteSpace(path)) + throw new ArgumentException($"{nameof(path)} cannot be null or whitespace", nameof(path)); // the stored path is absolute so we just return it as is - if(Uri.IsWellFormedUriString(path, UriKind.Absolute)) + if (Uri.IsWellFormedUriString(path, UriKind.Absolute)) return new Uri(path); Uri uri; diff --git a/src/Umbraco.Infrastructure/Models/MediaExtensions.cs b/src/Umbraco.Infrastructure/Models/MediaExtensions.cs index 08612d2810..41bcfdbc88 100644 --- a/src/Umbraco.Infrastructure/Models/MediaExtensions.cs +++ b/src/Umbraco.Infrastructure/Models/MediaExtensions.cs @@ -1,7 +1,5 @@ using System.Linq; -using Umbraco.Core.Composing; using Umbraco.Core.Configuration.UmbracoSettings; -using Umbraco.Core.Logging; using Umbraco.Core.PropertyEditors; namespace Umbraco.Core.Models @@ -11,17 +9,15 @@ namespace Umbraco.Core.Models /// /// Gets the url of a media item. /// - public static string GetUrl(this IMedia media, string propertyAlias, ILogger logger, PropertyEditorCollection propertyEditors) + public static string GetUrl(this IMedia media, string propertyAlias, MediaUrlGeneratorCollection mediaUrlGenerators) { if (!media.Properties.TryGetValue(propertyAlias, out var property)) return string.Empty; - if (propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out var editor) - && editor is IDataEditorWithMediaPath dataEditor) - { - // TODO: would need to be adjusted to variations, when media become variants - var value = property.GetValue(); - return dataEditor.GetMediaPath(value); + // TODO: would need to be adjusted to variations, when media become variants + if (mediaUrlGenerators.TryGetMediaPath(property.PropertyType.PropertyEditorAlias, property.GetValue(), out var mediaUrl)) + { + return mediaUrl; } // Without knowing what it is, just adding a string here might not be very nice @@ -31,10 +27,10 @@ namespace Umbraco.Core.Models /// /// Gets the urls of a media item. /// - public static string[] GetUrls(this IMedia media, IContentSection contentSection, ILogger logger, PropertyEditorCollection propertyEditors) + public static string[] GetUrls(this IMedia media, IContentSection contentSection, MediaUrlGeneratorCollection mediaUrlGenerators) { return contentSection.ImageAutoFillProperties - .Select(field => media.GetUrl(field.Alias, logger, propertyEditors)) + .Select(field => media.GetUrl(field.Alias, mediaUrlGenerators)) .Where(link => string.IsNullOrWhiteSpace(link) == false) .ToArray(); } diff --git a/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs b/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs index 4ae859ba80..13f9d3ee5c 100644 --- a/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs +++ b/src/Umbraco.Infrastructure/Persistence/Factories/ContentBaseFactory.cs @@ -185,7 +185,7 @@ namespace Umbraco.Core.Persistence.Factories /// /// Builds a dto from an IMedia item. /// - public static MediaDto BuildDto(PropertyEditorCollection propertyEditors, IMedia entity) + public static MediaDto BuildDto(MediaUrlGeneratorCollection mediaUrlGenerators, IMedia entity) { var contentDto = BuildContentDto(entity, Constants.ObjectTypes.Media); @@ -193,7 +193,7 @@ namespace Umbraco.Core.Persistence.Factories { NodeId = entity.Id, ContentDto = contentDto, - MediaVersionDto = BuildMediaVersionDto(propertyEditors, entity, contentDto) + MediaVersionDto = BuildMediaVersionDto(mediaUrlGenerators, entity, contentDto) }; return dto; @@ -287,7 +287,7 @@ namespace Umbraco.Core.Persistence.Factories return dto; } - private static MediaVersionDto BuildMediaVersionDto(PropertyEditorCollection propertyEditors, IMedia entity, ContentDto contentDto) + private static MediaVersionDto BuildMediaVersionDto(MediaUrlGeneratorCollection mediaUrlGenerators, IMedia entity, ContentDto contentDto) { // try to get a path from the string being stored for media // TODO: only considering umbracoFile @@ -295,11 +295,9 @@ namespace Umbraco.Core.Persistence.Factories string path = null; if (entity.Properties.TryGetValue(Constants.Conventions.Media.File, out var property) - && propertyEditors.TryGet(property.PropertyType.PropertyEditorAlias, out var editor) - && editor is IDataEditorWithMediaPath dataEditor) + && mediaUrlGenerators.TryGetMediaPath(property.PropertyType.PropertyEditorAlias, property.GetValue(), out var mediaPath)) { - var value = property.GetValue(); - path = dataEditor.GetMediaPath(value); + path = mediaPath; } var dto = new MediaVersionDto diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs index b0afc3b5f8..11f8c4d696 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs @@ -25,6 +25,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement { private readonly IMediaTypeRepository _mediaTypeRepository; private readonly ITagRepository _tagRepository; + private readonly MediaUrlGeneratorCollection _mediaUrlGenerators; private readonly MediaByGuidReadRepository _mediaByGuidReadRepository; public MediaRepository( @@ -37,12 +38,14 @@ namespace Umbraco.Core.Persistence.Repositories.Implement IRelationRepository relationRepository, IRelationTypeRepository relationTypeRepository, Lazy propertyEditorCollection, + MediaUrlGeneratorCollection mediaUrlGenerators, DataValueReferenceFactoryCollection dataValueReferenceFactories, IDataTypeService dataTypeService) : base(scopeAccessor, cache, logger, languageRepository, relationRepository, relationTypeRepository, propertyEditorCollection, dataValueReferenceFactories, dataTypeService) { _mediaTypeRepository = mediaTypeRepository ?? throw new ArgumentNullException(nameof(mediaTypeRepository)); _tagRepository = tagRepository ?? throw new ArgumentNullException(nameof(tagRepository)); + _mediaUrlGenerators = mediaUrlGenerators; _mediaByGuidReadRepository = new MediaByGuidReadRepository(this, scopeAccessor, cache, logger); } @@ -239,7 +242,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement entity.SanitizeEntityPropertiesForXmlStorage(); // create the dto - var dto = ContentBaseFactory.BuildDto(PropertyEditors, entity); + var dto = ContentBaseFactory.BuildDto(_mediaUrlGenerators, entity); // derive path and level from parent var parent = GetParentNodeDto(entity.ParentId); @@ -330,7 +333,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement } // create the dto - var dto = ContentBaseFactory.BuildDto(PropertyEditors, entity); + var dto = ContentBaseFactory.BuildDto(_mediaUrlGenerators, entity); // update the node dto var nodeDto = dto.ContentDto.NodeDto; diff --git a/src/Umbraco.Infrastructure/PropertyEditors/DataValueReferenceFactoryCollection.cs b/src/Umbraco.Infrastructure/PropertyEditors/DataValueReferenceFactoryCollection.cs index 51dfe6c5c4..83f5badb9c 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/DataValueReferenceFactoryCollection.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/DataValueReferenceFactoryCollection.cs @@ -11,6 +11,9 @@ namespace Umbraco.Core.PropertyEditors : base(items) { } + // TODO: We could further reduce circular dependencies with PropertyEditorCollection by not having IDataValueReference implemented + // by property editors and instead just use the already built in IDataValueReferenceFactory and/or refactor that into a more normal collection + public IEnumerable GetAllReferences(IPropertyCollection properties, PropertyEditorCollection propertyEditors) { var trackedRelations = new List(); diff --git a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs index 715d4b0d36..8c820c497c 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/FileUploadPropertyEditor.cs @@ -19,7 +19,7 @@ namespace Umbraco.Web.PropertyEditors "fileupload", Group = Constants.PropertyEditors.Groups.Media, Icon = "icon-download-alt")] - public class FileUploadPropertyEditor : DataEditor, IDataEditorWithMediaPath + public class FileUploadPropertyEditor : DataEditor, IMediaUrlGenerator { private readonly IMediaFileSystem _mediaFileSystem; private readonly IContentSection _contentSection; @@ -52,7 +52,16 @@ namespace Umbraco.Web.PropertyEditors return editor; } - public string GetMediaPath(object value) => value?.ToString(); + public bool TryGetMediaPath(string alias, object value, out string mediaPath) + { + if (alias == Alias) + { + mediaPath = value?.ToString(); + return true; + } + mediaPath = null; + return false; + } /// /// Gets a value indicating whether a property is an upload field. diff --git a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs index 66a1ae68ef..045a0fb3d2 100644 --- a/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs +++ b/src/Umbraco.Infrastructure/PropertyEditors/ImageCropperPropertyEditor.cs @@ -26,7 +26,7 @@ namespace Umbraco.Web.PropertyEditors HideLabel = false, Group = Constants.PropertyEditors.Groups.Media, Icon = "icon-crop")] - public class ImageCropperPropertyEditor : DataEditor, IDataEditorWithMediaPath + public class ImageCropperPropertyEditor : DataEditor, IMediaUrlGenerator { private readonly IMediaFileSystem _mediaFileSystem; private readonly IContentSection _contentSettings; @@ -53,7 +53,16 @@ namespace Umbraco.Web.PropertyEditors _autoFillProperties = new UploadAutoFillProperties(_mediaFileSystem, logger, _contentSettings); } - public string GetMediaPath(object value) => GetFileSrcFromPropertyValue(value, out _, false); + public bool TryGetMediaPath(string alias, object value, out string mediaPath) + { + if (alias == Alias) + { + mediaPath = GetFileSrcFromPropertyValue(value, out _, false); + return true; + } + mediaPath = null; + return false; + } /// /// Creates the corresponding property value editor. diff --git a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs index a2d3bc4e45..e767ce5505 100644 --- a/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs +++ b/src/Umbraco.Infrastructure/Runtime/CoreInitialComposer.cs @@ -81,7 +81,7 @@ namespace Umbraco.Core.Runtime composition.DataEditors() .Add(() => composition.TypeLoader.GetDataEditors()); - composition.DataEditorsWithMediaPath() + composition.MediaUrlGenerators() .Add() .Add(); diff --git a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs index ca24b051d0..efabddb2cd 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/MediaRepositoryTest.cs @@ -44,8 +44,9 @@ namespace Umbraco.Tests.Persistence.Repositories var entityRepository = new EntityRepository(scopeAccessor); var relationRepository = new RelationRepository(scopeAccessor, Logger, relationTypeRepository, entityRepository); var propertyEditors = new Lazy(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty()))); + var mediaUrlGenerators = new MediaUrlGeneratorCollection(Enumerable.Empty()); var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty()); - var repository = new MediaRepository(scopeAccessor, appCaches, Logger, mediaTypeRepository, tagRepository, Mock.Of(), relationRepository, relationTypeRepository, propertyEditors, dataValueReferences, DataTypeService); + var repository = new MediaRepository(scopeAccessor, appCaches, Logger, mediaTypeRepository, tagRepository, Mock.Of(), relationRepository, relationTypeRepository, propertyEditors, mediaUrlGenerators, dataValueReferences, DataTypeService); return repository; } diff --git a/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs index 70c946dabe..d4341cd128 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/TagRepositoryTest.cs @@ -981,8 +981,9 @@ namespace Umbraco.Tests.Persistence.Repositories var entityRepository = new EntityRepository(accessor); var relationRepository = new RelationRepository(accessor, Logger, relationTypeRepository, entityRepository); var propertyEditors = new Lazy(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty()))); + var mediaUrlGenerators = new MediaUrlGeneratorCollection(Enumerable.Empty()); var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty()); - var repository = new MediaRepository(accessor, AppCaches.Disabled, Logger, mediaTypeRepository, tagRepository, Mock.Of(), relationRepository, relationTypeRepository, propertyEditors, dataValueReferences, DataTypeService); + var repository = new MediaRepository(accessor, AppCaches.Disabled, Logger, mediaTypeRepository, tagRepository, Mock.Of(), relationRepository, relationTypeRepository, propertyEditors, mediaUrlGenerators, dataValueReferences, DataTypeService); return repository; } } diff --git a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs index 909c6098fb..8b2514eec0 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs @@ -36,8 +36,9 @@ namespace Umbraco.Tests.Persistence.Repositories var entityRepository = new EntityRepository(accessor); var relationRepository = new RelationRepository(accessor, Logger, relationTypeRepository, entityRepository); var propertyEditors = new Lazy(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty()))); + var mediaUrlGenerators = new MediaUrlGeneratorCollection(Enumerable.Empty()); var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty()); - var repository = new MediaRepository(accessor, AppCaches, Mock.Of(), mediaTypeRepository, tagRepository, Mock.Of(), relationRepository, relationTypeRepository, propertyEditors, dataValueReferences, DataTypeService); + var repository = new MediaRepository(accessor, AppCaches, Mock.Of(), mediaTypeRepository, tagRepository, Mock.Of(), relationRepository, relationTypeRepository, propertyEditors, mediaUrlGenerators, dataValueReferences, DataTypeService); return repository; } diff --git a/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs b/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs index 519274b118..fb61fcffe8 100644 --- a/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs +++ b/src/Umbraco.Tests/Routing/MediaUrlProviderTests.cs @@ -38,7 +38,7 @@ namespace Umbraco.Tests.Routing var dataTypeService = Mock.Of(); var umbracoSettingsSection = TestObjects.GetUmbracoSettings(); - var propertyEditors = new DataEditorWithMediaPathCollection(new IDataEditorWithMediaPath[] + var propertyEditors = new MediaUrlGeneratorCollection(new IMediaUrlGenerator[] { new FileUploadPropertyEditor(logger, mediaFileSystemMock, contentSection, dataTypeService, LocalizationService, LocalizedTextService, ShortStringHelper, umbracoSettingsSection), new ImageCropperPropertyEditor(logger, mediaFileSystemMock, contentSection, dataTypeService, LocalizationService, IOHelper, ShortStringHelper, LocalizedTextService, umbracoSettingsSection), diff --git a/src/Umbraco.Web/Models/Mapping/MediaMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/MediaMapDefinition.cs index 3298f3bd1e..d02e88ef4b 100644 --- a/src/Umbraco.Web/Models/Mapping/MediaMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/MediaMapDefinition.cs @@ -18,21 +18,19 @@ namespace Umbraco.Web.Models.Mapping internal class MediaMapDefinition : IMapDefinition { private readonly CommonMapper _commonMapper; - private readonly ILogger _logger; private readonly IMediaService _mediaService; private readonly IMediaTypeService _mediaTypeService; - private readonly PropertyEditorCollection _propertyEditorCollection; + private readonly MediaUrlGeneratorCollection _mediaUrlGenerators; private readonly TabsAndPropertiesMapper _tabsAndPropertiesMapper; private readonly IUmbracoSettingsSection _umbracoSettingsSection; - public MediaMapDefinition(ICultureDictionary cultureDictionary, ILogger logger, CommonMapper commonMapper, IMediaService mediaService, IMediaTypeService mediaTypeService, - ILocalizedTextService localizedTextService, PropertyEditorCollection propertyEditorCollection, IUmbracoSettingsSection umbracoSettingsSection, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider) + public MediaMapDefinition(ICultureDictionary cultureDictionary, CommonMapper commonMapper, IMediaService mediaService, IMediaTypeService mediaTypeService, + ILocalizedTextService localizedTextService, MediaUrlGeneratorCollection mediaUrlGenerators, IUmbracoSettingsSection umbracoSettingsSection, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider) { - _logger = logger; _commonMapper = commonMapper; _mediaService = mediaService; _mediaTypeService = mediaTypeService; - _propertyEditorCollection = propertyEditorCollection; + _mediaUrlGenerators = mediaUrlGenerators; _umbracoSettingsSection = umbracoSettingsSection ?? throw new ArgumentNullException(nameof(umbracoSettingsSection)); _tabsAndPropertiesMapper = new TabsAndPropertiesMapper(cultureDictionary, localizedTextService, contentTypeBaseServiceProvider); @@ -64,7 +62,7 @@ namespace Umbraco.Web.Models.Mapping target.Id = source.Id; target.IsChildOfListView = DetermineIsChildOfListView(source); target.Key = source.Key; - target.MediaLink = string.Join(",", source.GetUrls(_umbracoSettingsSection.Content, _logger, _propertyEditorCollection)); + target.MediaLink = string.Join(",", source.GetUrls(_umbracoSettingsSection.Content, _mediaUrlGenerators)); target.Name = source.Name; target.Owner = _commonMapper.GetOwner(source, context); target.ParentId = source.ParentId; From 610d807006cf27dadb53d87a5999a0c15272fc63 Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 19 Feb 2020 10:17:49 +0100 Subject: [PATCH 4/5] Fixed integration tests by composing the MediaUrlGenerators --- src/Umbraco.Tests/Testing/UmbracoTestBase.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index b0cde2b905..bbc16b29ec 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -335,6 +335,9 @@ namespace Umbraco.Tests.Testing // manifest Composition.ManifestValueValidators(); Composition.ManifestFilters(); + Composition.MediaUrlGenerators() + .Add() + .Add(); } From b964ff1d3ebd83c4067fa1ac9c461697a6c5280f Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Wed, 19 Feb 2020 10:18:27 +0100 Subject: [PATCH 5/5] Introduced an extension method on IHttpContextAccessor that return the HttpContext or throws if null. --- .../Editors/BackOfficeServerVariables.cs | 4 +-- src/Umbraco.Web/HtmlHelperRenderExtensions.cs | 2 +- .../HttpContextAccessorExtensions.cs | 17 ++++++++++++ src/Umbraco.Web/Install/InstallHelper.cs | 11 ++++---- .../Install/InstallSteps/NewInstallStep.cs | 2 +- .../InstallSteps/StarterKitInstallStep.cs | 2 +- .../Net/AspNetHttpContextAccessor.cs | 7 +---- .../Routing/ContentFinderByIdPath.cs | 5 ++-- .../Routing/ContentFinderByPageIdQuery.cs | 2 +- src/Umbraco.Web/Routing/PublishedRouter.cs | 2 +- src/Umbraco.Web/Security/WebSecurity.cs | 26 ++++++++++--------- src/Umbraco.Web/Templates/TemplateRenderer.cs | 2 +- src/Umbraco.Web/Umbraco.Web.csproj | 1 + src/Umbraco.Web/UmbracoContext.cs | 4 +-- 14 files changed, 52 insertions(+), 35 deletions(-) create mode 100644 src/Umbraco.Web/HttpContextAccessorExtensions.cs diff --git a/src/Umbraco.Web/Editors/BackOfficeServerVariables.cs b/src/Umbraco.Web/Editors/BackOfficeServerVariables.cs index 4b5992ac23..48d739b172 100644 --- a/src/Umbraco.Web/Editors/BackOfficeServerVariables.cs +++ b/src/Umbraco.Web/Editors/BackOfficeServerVariables.cs @@ -378,7 +378,7 @@ namespace Umbraco.Web.Editors "externalLogins", new Dictionary { { - "providers", _httpContextAccessor.HttpContext.GetOwinContext().Authentication.GetExternalAuthenticationTypes() + "providers", _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().Authentication.GetExternalAuthenticationTypes() .Where(p => p.Properties.ContainsKey("UmbracoBackOffice")) .Select(p => new { @@ -465,7 +465,7 @@ namespace Umbraco.Web.Editors app.Add("cacheBuster", $"{version}.{_runtimeState.Level}.{ClientDependencySettings.Instance.Version}".GenerateHash()); //useful for dealing with virtual paths on the client side when hosted in virtual directories especially - app.Add("applicationPath", _httpContextAccessor.HttpContext.Request.ApplicationPath.EnsureEndsWith('/')); + app.Add("applicationPath", _httpContextAccessor.GetRequiredHttpContext().Request.ApplicationPath.EnsureEndsWith('/')); //add the server's GMT time offset in minutes app.Add("serverTimeOffset", Convert.ToInt32(DateTimeOffset.Now.Offset.TotalMinutes)); diff --git a/src/Umbraco.Web/HtmlHelperRenderExtensions.cs b/src/Umbraco.Web/HtmlHelperRenderExtensions.cs index d361ca0af2..796a3a0c4a 100644 --- a/src/Umbraco.Web/HtmlHelperRenderExtensions.cs +++ b/src/Umbraco.Web/HtmlHelperRenderExtensions.cs @@ -67,7 +67,7 @@ namespace Umbraco.Web var htmlBadge = String.Format(umbracoSettingsSection.Content.PreviewBadge, ioHelper.ResolveUrl(globalSettings.UmbracoPath), - WebUtility.UrlEncode(httpContextAccessor.HttpContext.Request.Path), + WebUtility.UrlEncode(httpContextAccessor.GetRequiredHttpContext().Request.Path), Current.UmbracoContext.PublishedRequest.PublishedContent.Id); return new MvcHtmlString(htmlBadge); } diff --git a/src/Umbraco.Web/HttpContextAccessorExtensions.cs b/src/Umbraco.Web/HttpContextAccessorExtensions.cs new file mode 100644 index 0000000000..4c2dcf0a0d --- /dev/null +++ b/src/Umbraco.Web/HttpContextAccessorExtensions.cs @@ -0,0 +1,17 @@ +using System.IO; +using System.Web; + +namespace Umbraco.Web +{ + public static class HttpContextAccessorExtensions + { + public static HttpContextBase GetRequiredHttpContext(this IHttpContextAccessor httpContextAccessor) + { + var httpContext = httpContextAccessor.HttpContext; + + if(httpContext is null) throw new IOException("HttpContext is null"); + + return httpContext; + } + } +} diff --git a/src/Umbraco.Web/Install/InstallHelper.cs b/src/Umbraco.Web/Install/InstallHelper.cs index fb809d1bfc..0c3e945029 100644 --- a/src/Umbraco.Web/Install/InstallHelper.cs +++ b/src/Umbraco.Web/Install/InstallHelper.cs @@ -18,7 +18,7 @@ namespace Umbraco.Web.Install { private static HttpClient _httpClient; private readonly DatabaseBuilder _databaseBuilder; - private readonly HttpContextBase _httpContext; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly ILogger _logger; private readonly IGlobalSettings _globalSettings; private readonly IUmbracoVersion _umbracoVersion; @@ -29,7 +29,7 @@ namespace Umbraco.Web.Install DatabaseBuilder databaseBuilder, ILogger logger, IGlobalSettings globalSettings, IUmbracoVersion umbracoVersion, IConnectionStrings connectionStrings) { - _httpContext = httpContextAccessor.HttpContext; + _httpContextAccessor = httpContextAccessor; _logger = logger; _globalSettings = globalSettings; _umbracoVersion = umbracoVersion; @@ -46,12 +46,13 @@ namespace Umbraco.Web.Install { try { - var userAgent = _httpContext.Request.UserAgent; + var httpContext = _httpContextAccessor.GetRequiredHttpContext(); + var userAgent = httpContext.Request.UserAgent; // Check for current install Id var installId = Guid.NewGuid(); - var installCookie = _httpContext.Request.GetCookieValue(Constants.Web.InstallerCookieName); + var installCookie = httpContext.Request.GetCookieValue(Constants.Web.InstallerCookieName); if (string.IsNullOrEmpty(installCookie) == false) { if (Guid.TryParse(installCookie, out installId)) @@ -61,7 +62,7 @@ namespace Umbraco.Web.Install installId = Guid.NewGuid(); } } - _httpContext.Response.Cookies.Set(new HttpCookie(Constants.Web.InstallerCookieName, "1")); + httpContext.Response.Cookies.Set(new HttpCookie(Constants.Web.InstallerCookieName, "1")); var dbProvider = string.Empty; if (IsBrandNewInstall == false) diff --git a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs index c898532116..e47a8f4b5f 100644 --- a/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/NewInstallStep.cs @@ -56,7 +56,7 @@ namespace Umbraco.Web.Install.InstallSteps throw new InvalidOperationException("Could not find the super user!"); } - var userManager = _httpContextAccessor.HttpContext.GetOwinContext().GetBackOfficeUserManager(); + var userManager = _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().GetBackOfficeUserManager(); var membershipUser = await userManager.FindByIdAsync(Constants.Security.SuperUserId); if (membershipUser == null) { diff --git a/src/Umbraco.Web/Install/InstallSteps/StarterKitInstallStep.cs b/src/Umbraco.Web/Install/InstallSteps/StarterKitInstallStep.cs index 37e514b370..c9688fed4c 100644 --- a/src/Umbraco.Web/Install/InstallSteps/StarterKitInstallStep.cs +++ b/src/Umbraco.Web/Install/InstallSteps/StarterKitInstallStep.cs @@ -34,7 +34,7 @@ namespace Umbraco.Web.Install.InstallSteps InstallBusinessLogic(packageId); - UmbracoApplication.Restart(_httContextAccessor.HttpContext); + UmbracoApplication.Restart(_httContextAccessor.GetRequiredHttpContext()); return Task.FromResult(null); } diff --git a/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs b/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs index 322e9f2d88..d222d18438 100644 --- a/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs +++ b/src/Umbraco.Web/Net/AspNetHttpContextAccessor.cs @@ -11,12 +11,7 @@ namespace Umbraco.Web { var httpContext = System.Web.HttpContext.Current; - if (httpContext is null) - { - throw new InvalidOperationException("HttpContext is not available"); - } - - return new HttpContextWrapper(httpContext); + return httpContext is null ? null : new HttpContextWrapper(httpContext); } set { diff --git a/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs b/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs index 52d07300ab..bf7d5ef7c4 100644 --- a/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs +++ b/src/Umbraco.Web/Routing/ContentFinderByIdPath.cs @@ -56,11 +56,12 @@ namespace Umbraco.Web.Routing if (node != null) { + var httpContext = _httpContextAccessor.GetRequiredHttpContext(); //if we have a node, check if we have a culture in the query string - if (_httpContextAccessor.HttpContext.Request.QueryString.ContainsKey("culture")) + if (httpContext.Request.QueryString.ContainsKey("culture")) { //we're assuming it will match a culture, if an invalid one is passed in, an exception will throw (there is no TryGetCultureInfo method), i think this is ok though - frequest.Culture = CultureInfo.GetCultureInfo(_httpContextAccessor.HttpContext.Request.QueryString["culture"]); + frequest.Culture = CultureInfo.GetCultureInfo(httpContext.Request.QueryString["culture"]); } frequest.PublishedContent = node; diff --git a/src/Umbraco.Web/Routing/ContentFinderByPageIdQuery.cs b/src/Umbraco.Web/Routing/ContentFinderByPageIdQuery.cs index 790da3cc89..fb79e13dbc 100644 --- a/src/Umbraco.Web/Routing/ContentFinderByPageIdQuery.cs +++ b/src/Umbraco.Web/Routing/ContentFinderByPageIdQuery.cs @@ -19,7 +19,7 @@ public bool TryFindContent(IPublishedRequest frequest) { int pageId; - if (int.TryParse(_httpContextAccessor.HttpContext.Request["umbPageID"], out pageId)) + if (int.TryParse(_httpContextAccessor.GetRequiredHttpContext().Request["umbPageID"], out pageId)) { var doc = frequest.UmbracoContext.Content.GetById(pageId); diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 7d827b935e..1a6048e4ec 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -637,7 +637,7 @@ namespace Umbraco.Web.Routing var useAltTemplate = request.IsInitialPublishedContent || (_webRoutingSection.InternalRedirectPreservesTemplate && request.IsInternalRedirectPublishedContent); var altTemplate = useAltTemplate - ? _httpContextAccessor.HttpContext.Request[Constants.Conventions.Url.AltTemplate] + ? _httpContextAccessor.GetRequiredHttpContext().Request[Constants.Conventions.Url.AltTemplate] : null; if (string.IsNullOrWhiteSpace(altTemplate)) diff --git a/src/Umbraco.Web/Security/WebSecurity.cs b/src/Umbraco.Web/Security/WebSecurity.cs index a1b360bb83..6f16eb3c52 100644 --- a/src/Umbraco.Web/Security/WebSecurity.cs +++ b/src/Umbraco.Web/Security/WebSecurity.cs @@ -61,7 +61,7 @@ namespace Umbraco.Web.Security { if (_signInManager == null) { - var mgr = _httpContextAccessor.HttpContext.GetOwinContext().Get(); + var mgr = _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().Get(); if (mgr == null) { throw new NullReferenceException("Could not resolve an instance of " + typeof(BackOfficeSignInManager) + " from the " + typeof(IOwinContext)); @@ -74,12 +74,13 @@ namespace Umbraco.Web.Security private BackOfficeUserManager _userManager; protected BackOfficeUserManager UserManager - => _userManager ?? (_userManager = _httpContextAccessor.HttpContext.GetOwinContext().GetBackOfficeUserManager()); + => _userManager ?? (_userManager = _httpContextAccessor.GetRequiredHttpContext().GetOwinContext().GetBackOfficeUserManager()); [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] public double PerformLogin(int userId) { - var owinCtx = _httpContextAccessor.HttpContext.GetOwinContext(); + var httpContext = _httpContextAccessor.GetRequiredHttpContext(); + var owinCtx = httpContext.GetOwinContext(); //ensure it's done for owin too owinCtx.Authentication.SignOut(Constants.Security.BackOfficeExternalAuthenticationType); @@ -87,7 +88,7 @@ namespace Umbraco.Web.Security SignInManager.SignInAsync(user, isPersistent: true, rememberBrowser: false).Wait(); - _httpContextAccessor.HttpContext.SetPrincipalForRequest(owinCtx.Request.User); + httpContext.SetPrincipalForRequest(owinCtx.Request.User); return TimeSpan.FromMinutes(_globalSettings.TimeOutInMinutes).TotalSeconds; } @@ -95,8 +96,9 @@ namespace Umbraco.Web.Security [Obsolete("This needs to be removed, ASP.NET Identity should always be used for this operation, this is currently only used in the installer which needs to be updated")] public void ClearCurrentLogin() { - _httpContextAccessor.HttpContext.UmbracoLogout(); - _httpContextAccessor.HttpContext.GetOwinContext().Authentication.SignOut( + var httpContext = _httpContextAccessor.GetRequiredHttpContext(); + httpContext.UmbracoLogout(); + httpContext.GetOwinContext().Authentication.SignOut( Core.Constants.Security.BackOfficeAuthenticationType, Core.Constants.Security.BackOfficeExternalAuthenticationType); } @@ -108,7 +110,7 @@ namespace Umbraco.Web.Security /// public Attempt GetUserId() { - var identity = _httpContextAccessor.HttpContext.GetCurrentIdentity(false); + var identity = _httpContextAccessor.GetRequiredHttpContext().GetCurrentIdentity(false); return identity == null ? Attempt.Fail() : Attempt.Succeed(Convert.ToInt32(identity.Id)); } @@ -143,7 +145,7 @@ namespace Umbraco.Web.Security var user = CurrentUser; // Check for console access - if (user == null || (requiresApproval && user.IsApproved == false) || (user.IsLockedOut && RequestIsInUmbracoApplication(_httpContextAccessor.HttpContext, _globalSettings, _ioHelper))) + if (user == null || (requiresApproval && user.IsApproved == false) || (user.IsLockedOut && RequestIsInUmbracoApplication(_httpContextAccessor, _globalSettings, _ioHelper))) { if (throwExceptions) throw new ArgumentException("You have no privileges to the umbraco console. Please contact your administrator"); return ValidateRequestAttempt.FailedNoPrivileges; @@ -152,9 +154,9 @@ namespace Umbraco.Web.Security } - private static bool RequestIsInUmbracoApplication(HttpContextBase context, IGlobalSettings globalSettings, IIOHelper ioHelper) + private static bool RequestIsInUmbracoApplication(IHttpContextAccessor httpContextAccessor, IGlobalSettings globalSettings, IIOHelper ioHelper) { - return context.Request.Path.ToLower().IndexOf(ioHelper.ResolveUrl(globalSettings.UmbracoPath).ToLower(), StringComparison.Ordinal) > -1; + return httpContextAccessor.GetRequiredHttpContext().Request.Path.ToLower().IndexOf(ioHelper.ResolveUrl(globalSettings.UmbracoPath).ToLower(), StringComparison.Ordinal) > -1; } /// @@ -165,7 +167,7 @@ namespace Umbraco.Web.Security public ValidateRequestAttempt AuthorizeRequest(bool throwExceptions = false) { // check for secure connection - if (_globalSettings.UseHttps && _httpContextAccessor.HttpContext.Request.IsSecureConnection == false) + if (_globalSettings.UseHttps && _httpContextAccessor.GetRequiredHttpContext().Request.IsSecureConnection == false) { if (throwExceptions) throw new SecurityException("This installation requires a secure connection (via SSL). Please update the URL to include https://"); return ValidateRequestAttempt.FailedNoSsl; @@ -191,7 +193,7 @@ namespace Umbraco.Web.Security public bool IsAuthenticated() { var httpContext = _httpContextAccessor.HttpContext; - return httpContext.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity(false) != null; + return httpContext?.User != null && httpContext.User.Identity.IsAuthenticated && httpContext.GetCurrentIdentity(false) != null; } } diff --git a/src/Umbraco.Web/Templates/TemplateRenderer.cs b/src/Umbraco.Web/Templates/TemplateRenderer.cs index da5de1055c..ac08258ee2 100644 --- a/src/Umbraco.Web/Templates/TemplateRenderer.cs +++ b/src/Umbraco.Web/Templates/TemplateRenderer.cs @@ -127,7 +127,7 @@ namespace Umbraco.Web.Templates //var queryString = _umbracoContext.HttpContext.Request.QueryString.AllKeys // .ToDictionary(key => key, key => context.Request.QueryString[key]); - var requestContext = new RequestContext(_httpContextAccessor.HttpContext, new RouteData() + var requestContext = new RequestContext(_httpContextAccessor.GetRequiredHttpContext(), new RouteData() { Route = RouteTable.Routes["Umbraco_default"] }); diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 388b5b10dc..fa0fe5eda8 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -156,6 +156,7 @@ + diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index 2dacc60e73..99d0b68d89 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -53,7 +53,7 @@ namespace Umbraco.Web // // all in all, this context may be disposed more than once, but DisposableObject ensures that // it is ok and it will be actually disposed only once. - httpContextAccessor.HttpContext.DisposeOnPipelineCompleted(this); + httpContextAccessor.GetRequiredHttpContext().DisposeOnPipelineCompleted(this); ObjectCreated = DateTime.Now; UmbracoRequestId = Guid.NewGuid(); @@ -203,7 +203,7 @@ namespace Umbraco.Web { try { - return _httpContextAccessor.HttpContext.Request; + return _httpContextAccessor.GetRequiredHttpContext().Request; } catch (HttpException) {