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.

This commit is contained in:
Shannon
2020-02-19 16:37:00 +11:00
parent 04c620d76e
commit 13a56eb085
20 changed files with 122 additions and 100 deletions

View File

@@ -29,8 +29,8 @@ namespace Umbraco.Core
/// </summary>
/// <param name="composition">The composition.</param>
/// <returns></returns>
public static DataEditorWithMediaPathCollectionBuilder DataEditorsWithMediaPath(this Composition composition)
=> composition.WithCollectionBuilder<DataEditorWithMediaPathCollectionBuilder>();
public static MediaUrlGeneratorCollectionBuilder MediaUrlGenerators(this Composition composition)
=> composition.WithCollectionBuilder<MediaUrlGeneratorCollectionBuilder>();
#endregion
}

View File

@@ -1,21 +0,0 @@
namespace Umbraco.Core.PropertyEditors
{
/// <summary>
/// Must be implemented by property editors that store media and return media paths
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
public interface IDataEditorWithMediaPath
{
string Alias { get; }
/// <summary>
/// Returns the media path for the value stored for a property
/// </summary>
/// <param name="value"></param>
/// <returns></returns>
string GetMediaPath(object value);
}
}

View File

@@ -0,0 +1,18 @@
namespace Umbraco.Core.PropertyEditors
{
/// <summary>
/// Used to generate paths to media items for a specified property editor alias
/// </summary>
public interface IMediaUrlGenerator
{
/// <summary>
/// Tries to get a media path for a given property editor alias
/// </summary>
/// <param name="alias">The property editor alias</param>
/// <param name="value">The value of the property</param>
/// <returns>
/// True if a media path was returned
/// </returns>
bool TryGetMediaPath(string alias, object value, out string mediaPath);
}
}

View File

@@ -1,21 +0,0 @@
using System.Collections.Generic;
using System.Linq;
using Umbraco.Core.Composing;
namespace Umbraco.Core.PropertyEditors
{
public class DataEditorWithMediaPathCollection : BuilderCollectionBase<IDataEditorWithMediaPath>
{
public DataEditorWithMediaPathCollection(IEnumerable<IDataEditorWithMediaPath> items) : base(items)
{
}
public bool TryGet(string alias, out IDataEditorWithMediaPath editor)
{
editor = this.FirstOrDefault(x => x.Alias == alias);
return editor != null;
}
}
}

View File

@@ -1,9 +0,0 @@
using Umbraco.Core.Composing;
namespace Umbraco.Core.PropertyEditors
{
public class DataEditorWithMediaPathCollectionBuilder : LazyCollectionBuilderBase<DataEditorWithMediaPathCollectionBuilder, DataEditorWithMediaPathCollection, IDataEditorWithMediaPath>
{
protected override DataEditorWithMediaPathCollectionBuilder This => this;
}
}

View File

@@ -0,0 +1,28 @@
using System.Collections.Generic;
using Umbraco.Core.Composing;
namespace Umbraco.Core.PropertyEditors
{
public class MediaUrlGeneratorCollection : BuilderCollectionBase<IMediaUrlGenerator>
{
public MediaUrlGeneratorCollection(IEnumerable<IMediaUrlGenerator> 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;
}
}
}

View File

@@ -0,0 +1,9 @@
using Umbraco.Core.Composing;
namespace Umbraco.Core.PropertyEditors
{
public class MediaUrlGeneratorCollectionBuilder : LazyCollectionBuilderBase<MediaUrlGeneratorCollectionBuilder, MediaUrlGeneratorCollection, IMediaUrlGenerator>
{
protected override MediaUrlGeneratorCollectionBuilder This => this;
}
}

View File

@@ -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;

View File

@@ -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
/// <summary>
/// Gets the url of a media item.
/// </summary>
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
/// <summary>
/// Gets the urls of a media item.
/// </summary>
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();
}

View File

@@ -185,7 +185,7 @@ namespace Umbraco.Core.Persistence.Factories
/// <summary>
/// Builds a dto from an IMedia item.
/// </summary>
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

View File

@@ -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> 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;

View File

@@ -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<UmbracoEntityReference> GetAllReferences(IPropertyCollection properties, PropertyEditorCollection propertyEditors)
{
var trackedRelations = new List<UmbracoEntityReference>();

View File

@@ -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;
}
/// <summary>
/// Gets a value indicating whether a property is an upload field.

View File

@@ -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;
}
/// <summary>
/// Creates the corresponding property value editor.

View File

@@ -81,7 +81,7 @@ namespace Umbraco.Core.Runtime
composition.DataEditors()
.Add(() => composition.TypeLoader.GetDataEditors());
composition.DataEditorsWithMediaPath()
composition.MediaUrlGenerators()
.Add<FileUploadPropertyEditor>()
.Add<ImageCropperPropertyEditor>();

View File

@@ -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<PropertyEditorCollection>(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty<IDataEditor>())));
var mediaUrlGenerators = new MediaUrlGeneratorCollection(Enumerable.Empty<IMediaUrlGenerator>());
var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty<IDataValueReferenceFactory>());
var repository = new MediaRepository(scopeAccessor, appCaches, Logger, mediaTypeRepository, tagRepository, Mock.Of<ILanguageRepository>(), relationRepository, relationTypeRepository, propertyEditors, dataValueReferences, DataTypeService);
var repository = new MediaRepository(scopeAccessor, appCaches, Logger, mediaTypeRepository, tagRepository, Mock.Of<ILanguageRepository>(), relationRepository, relationTypeRepository, propertyEditors, mediaUrlGenerators, dataValueReferences, DataTypeService);
return repository;
}

View File

@@ -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<PropertyEditorCollection>(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty<IDataEditor>())));
var mediaUrlGenerators = new MediaUrlGeneratorCollection(Enumerable.Empty<IMediaUrlGenerator>());
var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty<IDataValueReferenceFactory>());
var repository = new MediaRepository(accessor, AppCaches.Disabled, Logger, mediaTypeRepository, tagRepository, Mock.Of<ILanguageRepository>(), relationRepository, relationTypeRepository, propertyEditors, dataValueReferences, DataTypeService);
var repository = new MediaRepository(accessor, AppCaches.Disabled, Logger, mediaTypeRepository, tagRepository, Mock.Of<ILanguageRepository>(), relationRepository, relationTypeRepository, propertyEditors, mediaUrlGenerators, dataValueReferences, DataTypeService);
return repository;
}
}

View File

@@ -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<PropertyEditorCollection>(() => new PropertyEditorCollection(new DataEditorCollection(Enumerable.Empty<IDataEditor>())));
var mediaUrlGenerators = new MediaUrlGeneratorCollection(Enumerable.Empty<IMediaUrlGenerator>());
var dataValueReferences = new DataValueReferenceFactoryCollection(Enumerable.Empty<IDataValueReferenceFactory>());
var repository = new MediaRepository(accessor, AppCaches, Mock.Of<ILogger>(), mediaTypeRepository, tagRepository, Mock.Of<ILanguageRepository>(), relationRepository, relationTypeRepository, propertyEditors, dataValueReferences, DataTypeService);
var repository = new MediaRepository(accessor, AppCaches, Mock.Of<ILogger>(), mediaTypeRepository, tagRepository, Mock.Of<ILanguageRepository>(), relationRepository, relationTypeRepository, propertyEditors, mediaUrlGenerators, dataValueReferences, DataTypeService);
return repository;
}

View File

@@ -38,7 +38,7 @@ namespace Umbraco.Tests.Routing
var dataTypeService = Mock.Of<IDataTypeService>();
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),

View File

@@ -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<IMedia> _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<IMedia>(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;