Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Umbraco.Core/EmbeddedResources/Lang/da.xml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ Mange hilsner fra Umbraco robotten
<key alias="entriesShort"><![CDATA[Minimum %0% element(er), tilføj <strong>%1%</strong> mere.]]></key>
<key alias="entriesExceed"><![CDATA[Maksimum %0% element(er), <strong>%1%</strong> for mange.]]></key>
<key alias="entriesAreasMismatch">Ét eller flere områder lever ikke op til kravene for antal indholdselementer.</key>
<key alias="invalidMediaType">Den valgte medie type er ugyldig.</key>
<key alias="multipleMediaNotAllowed">Det er kun tilladt at vælge ét medie.</key>
<key alias="invalidStartNode">Valgt medie kommer fra en ugyldig mappe.</key>
</area>
<area alias="recycleBin">
<key alias="contentTrashed">Slettet indhold med Id: {0} Relateret til original "parent" med id: {1}</key>
Expand All @@ -124,4 +127,4 @@ Mange hilsner fra Umbraco robotten
<key alias="FileWriting">Filskrivning</key>
<key alias="MediaFolderCreation">Mediemappeoprettelse</key>
</area>
</language>
</language>
3 changes: 3 additions & 0 deletions src/Umbraco.Core/EmbeddedResources/Lang/en.xml
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,9 @@
<key alias="duplicateUserGroupName">User group name '%0%' is already taken</key>
<key alias="duplicateMemberGroupName">Member group name '%0%' is already taken</key>
<key alias="duplicateUsername">Username '%0%' is already taken</key>
<key alias="invalidMediaType">The chosen media type is invalid.</key>
<key alias="multipleMediaNotAllowed">Multiple selected media is not allowed.</key>
<key alias="invalidStartNode">The selected media is from the wrong folder.</key>
</area>
<area alias="healthcheck">
<!-- The following keys get these tokens passed in:
Expand Down
3 changes: 3 additions & 0 deletions src/Umbraco.Core/EmbeddedResources/Lang/en_us.xml
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,9 @@
<key alias="entriesShort"><![CDATA[Minimum %0% entries, requires <strong>%1%</strong> more.]]></key>
<key alias="entriesExceed"><![CDATA[Maximum %0% entries, <strong>%1%</strong> too many.]]></key>
<key alias="entriesAreasMismatch">The content amount requirements are not met for one or more areas.</key>
<key alias="invalidMediaType">The chosen media type is invalid.</key>
<key alias="multipleMediaNotAllowed">Multiple selected media is not allowed.</key>
<key alias="invalidStartNode">The selected media is from the wrong folder.</key>
</area>
<area alias="healthcheck">
<!-- The following keys get these tokens passed in:
Expand Down
19 changes: 19 additions & 0 deletions src/Umbraco.Core/PropertyEditors/Validation/ITypedJsonValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System.ComponentModel.DataAnnotations;
using Umbraco.Cms.Core.Models.Validation;

namespace Umbraco.Cms.Core.PropertyEditors.Validation;

/// <summary>
/// A specific validator used for JSON based value editors, to avoid doing multiple deserialization.
/// <remarks>Is used together with <see cref="TypedJsonValidatorRunner{TValue,TConfiguration}"/></remarks>
/// </summary>
/// <typeparam name="TValue">The type of the value consumed by the validator.</typeparam>
/// <typeparam name="TConfiguration">The type of the configuration consumed by validator.</typeparam>
public interface ITypedJsonValidator<TValue, TConfiguration>
{
public abstract IEnumerable<ValidationResult> Validate(
TValue? value,
TConfiguration? configuration,
string? valueType,
PropertyValidationContext validationContext);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System.ComponentModel.DataAnnotations;
using Umbraco.Cms.Core.Models.Validation;
using Umbraco.Cms.Core.Serialization;

namespace Umbraco.Cms.Core.PropertyEditors.Validation;

/// <summary>
/// <para>
/// An aggregate validator for JSON based value editors, to avoid doing multiple deserialization.
/// </para>
/// <para>
/// Will deserialize once, and cast the configuration once, and pass those values to each <see cref="ITypedJsonValidator{TValue,TConfiguration}"/>, aggregating the results.
/// </para>
/// </summary>
/// <typeparam name="TValue">The type of the expected value.</typeparam>
/// <typeparam name="TConfiguration">The type of the expected configuration</typeparam>
public class TypedJsonValidatorRunner<TValue, TConfiguration> : IValueValidator
where TValue : class
{
private readonly IJsonSerializer _jsonSerializer;
private readonly ITypedJsonValidator<TValue, TConfiguration>[] _validators;

public TypedJsonValidatorRunner(IJsonSerializer jsonSerializer, params ITypedJsonValidator<TValue, TConfiguration>[] validators)
{
_jsonSerializer = jsonSerializer;
_validators = validators;
}

public IEnumerable<ValidationResult> Validate(object? value, string? valueType, object? dataTypeConfiguration,
PropertyValidationContext validationContext)
{
var validationResults = new List<ValidationResult>();

if (dataTypeConfiguration is not TConfiguration configuration)
{
return validationResults;
}

if (value is null || _jsonSerializer.TryDeserialize(value, out TValue? deserializedValue) is false)
{
return validationResults;
}

foreach (ITypedJsonValidator<TValue, TConfiguration> validator in _validators)
{
validationResults.AddRange(validator.Validate(deserializedValue, configuration, valueType, validationContext));
}

return validationResults;
}
}
201 changes: 154 additions & 47 deletions src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
using System.ComponentModel.DataAnnotations;
using Microsoft.Extensions.DependencyInjection;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.DependencyInjection;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.Editors;
using Umbraco.Cms.Core.Models.TemporaryFile;
using Umbraco.Cms.Core.Models.Validation;
using Umbraco.Cms.Core.PropertyEditors.Validation;
using Umbraco.Cms.Core.PropertyEditors.ValueConverters;
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Navigation;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Extensions;
Expand Down Expand Up @@ -71,7 +71,9 @@
IScopeProvider scopeProvider,
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IDataTypeConfigurationCache dataTypeReadCache,
ILocalizedTextService localizedTextService)
ILocalizedTextService localizedTextService,
IMediaTypeService mediaTypeService,
IMediaNavigationQueryService mediaNavigationQueryService)
: base(shortStringHelper, jsonSerializer, ioHelper, attribute)
{
_jsonSerializer = jsonSerializer;
Expand All @@ -81,34 +83,13 @@
_scopeProvider = scopeProvider;
_backOfficeSecurityAccessor = backOfficeSecurityAccessor;
_dataTypeReadCache = dataTypeReadCache;
Validators.Add(new MinMaxValidator(jsonSerializer, localizedTextService));
}

[Obsolete("Use non obsoleted constructor instead. Scheduled for removal in v17")]
public MediaPicker3PropertyValueEditor(
IShortStringHelper shortStringHelper,
IJsonSerializer jsonSerializer,
IIOHelper ioHelper,
DataEditorAttribute attribute,
IMediaImportService mediaImportService,
IMediaService mediaService,
ITemporaryFileService temporaryFileService,
IScopeProvider scopeProvider,
IBackOfficeSecurityAccessor backOfficeSecurityAccessor,
IDataTypeConfigurationCache dataTypeReadCache)
: this(
shortStringHelper,
var validators = new TypedJsonValidatorRunner<List<MediaWithCropsDto>, MediaPicker3Configuration>(
jsonSerializer,
ioHelper,
attribute,
mediaImportService,
mediaService,
temporaryFileService,
scopeProvider,
backOfficeSecurityAccessor,
dataTypeReadCache,
StaticServiceProvider.Instance.GetRequiredService<ILocalizedTextService>())
{
new MinMaxValidator(localizedTextService),
new AllowedTypeValidator(localizedTextService, mediaTypeService),
new StartNodeValidator(localizedTextService, mediaNavigationQueryService));

Validators.Add(validators);

Check notice on line 92 in src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

✅ No longer an issue: Constructor Over-Injection

MediaPicker3PropertyValueEditor is no longer above the threshold for number of arguments. This constructor has too many arguments, indicating an object with low cohesion or missing function argument abstraction. Avoid adding more arguments.
}

/// <remarks>
Expand Down Expand Up @@ -328,60 +309,186 @@
}
}

private class MinMaxValidator : IValueValidator
internal class MinMaxValidator : ITypedJsonValidator<List<MediaWithCropsDto>, MediaPicker3Configuration>
{
private readonly IJsonSerializer _jsonSerializer;
private readonly ILocalizedTextService _localizedTextService;

public MinMaxValidator(IJsonSerializer jsonSerializer, ILocalizedTextService localizedTextService)
{
_jsonSerializer = jsonSerializer;
_localizedTextService = localizedTextService;
}
public MinMaxValidator(ILocalizedTextService localizedTextService) => _localizedTextService = localizedTextService;

public IEnumerable<ValidationResult> Validate(
object? value,
List<MediaWithCropsDto>? mediaWithCropsDtos,
MediaPicker3Configuration? mediaPickerConfiguration,
string? valueType,
object? dataTypeConfiguration,
PropertyValidationContext validationContext)
{
var validationResults = new List<ValidationResult>();

if (dataTypeConfiguration is not MediaPicker3Configuration mediaPickerConfiguration)
if (mediaWithCropsDtos is null || mediaPickerConfiguration is null)
{
return validationResults;
}

if (value is null ||
_jsonSerializer.TryDeserialize(value, out List<MediaWithCropsDto>? mediaWithCropsDtos) is false)
if (mediaPickerConfiguration.Multiple is false && mediaWithCropsDtos.Count > 1)
{
return validationResults;
validationResults.Add(new ValidationResult(
_localizedTextService.Localize("validation", "multipleMediaNotAllowed"),
["value"]));
}

if (mediaPickerConfiguration.ValidationLimit.Min is not null
&& mediaWithCropsDtos.Count < mediaPickerConfiguration.ValidationLimit.Min)
{
validationResults.Add(new ValidationResult(
_localizedTextService.Localize(
"validation",
"entriesShort",
new[] { mediaPickerConfiguration.ValidationLimit.Min.ToString(), (mediaPickerConfiguration.ValidationLimit.Min - mediaWithCropsDtos.Count).ToString(), }),
new[] { "validationLimit" }));
[mediaPickerConfiguration.ValidationLimit.Min.ToString(), (mediaPickerConfiguration.ValidationLimit.Min - mediaWithCropsDtos.Count).ToString()
]),
["value"]));
}

if (mediaPickerConfiguration.ValidationLimit.Max is not null
&& mediaWithCropsDtos.Count > mediaPickerConfiguration.ValidationLimit.Max)
{
validationResults.Add(new ValidationResult(
_localizedTextService.Localize(
"validation",
"entriesExceed",
new[] { mediaPickerConfiguration.ValidationLimit.Max.ToString(), (mediaWithCropsDtos.Count - mediaPickerConfiguration.ValidationLimit.Max).ToString(), }),
new[] { "validationLimit" }));
[mediaPickerConfiguration.ValidationLimit.Max.ToString(), (mediaWithCropsDtos.Count - mediaPickerConfiguration.ValidationLimit.Max).ToString()
]),
["value"]));

Check warning on line 359 in src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ New issue: Complex Method

Validate has a cyclomatic complexity of 9, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
}

return validationResults;
}
}

internal class AllowedTypeValidator : ITypedJsonValidator<List<MediaWithCropsDto>, MediaPicker3Configuration>
{
private readonly ILocalizedTextService _localizedTextService;
private readonly IMediaTypeService _mediaTypeService;

public AllowedTypeValidator(ILocalizedTextService localizedTextService, IMediaTypeService mediaTypeService)
{
_localizedTextService = localizedTextService;
_mediaTypeService = mediaTypeService;
}

public IEnumerable<ValidationResult> Validate(
List<MediaWithCropsDto>? value,
MediaPicker3Configuration? configuration,
string? valueType,
PropertyValidationContext validationContext)
{
if (value is null || configuration is null)
{
return [];
}

var allowedTypes = configuration.Filter?.Split(Constants.CharArrays.Comma, StringSplitOptions.RemoveEmptyEntries);

// No allowed types = all types are allowed
if (allowedTypes is null || allowedTypes.Length == 0)
{
return [];
}

IEnumerable<string> distinctTypeAliases = value.DistinctBy(x => x.MediaTypeAlias).Select(x => x.MediaTypeAlias);

foreach (var typeAlias in distinctTypeAliases)
{
IMediaType? type = _mediaTypeService.Get(typeAlias);

if (type is null || allowedTypes.Contains(type.Key.ToString()) is false)
{
return
[
new ValidationResult(
_localizedTextService.Localize("validation", "invalidMediaType"),
["value"])
];
}
}

return [];
}
}

internal class StartNodeValidator : ITypedJsonValidator<List<MediaWithCropsDto>, MediaPicker3Configuration>
{
private readonly ILocalizedTextService _localizedTextService;
private readonly IMediaNavigationQueryService _mediaNavigationQueryService;

public StartNodeValidator(
ILocalizedTextService localizedTextService,
IMediaNavigationQueryService mediaNavigationQueryService)
{
_localizedTextService = localizedTextService;
_mediaNavigationQueryService = mediaNavigationQueryService;
}

public IEnumerable<ValidationResult> Validate(
List<MediaWithCropsDto>? value,
MediaPicker3Configuration? configuration,
string? valueType,
PropertyValidationContext validationContext)
{
if (value is null || configuration?.StartNodeId is null)
{
return [];
}


List<Guid> uniqueParentKeys = [];
foreach (Guid distinctMediaKey in value.DistinctBy(x => x.MediaKey).Select(x => x.MediaKey))
{
if (_mediaNavigationQueryService.TryGetParentKey(distinctMediaKey, out Guid? parentKey) is false)
{
continue;
}

// If there is a start node, the media must have a parent.
if (parentKey is null)
{
return
[
new ValidationResult(
_localizedTextService.Localize("validation", "invalidStartNode"),
["value"])
];
}

uniqueParentKeys.Add(parentKey.Value);
}

IEnumerable<Guid> parentKeysNotInStartNode = uniqueParentKeys.Where(x => x != configuration.StartNodeId.Value);
foreach (Guid parentKey in parentKeysNotInStartNode)
{
if (_mediaNavigationQueryService.TryGetAncestorsKeys(parentKey, out IEnumerable<Guid> foundAncestorKeys) is false)
{
// We couldn't find the parent node, so we fail.
return
[
new ValidationResult(
_localizedTextService.Localize("validation", "invalidStartNode"),
["value"])
];
}

Guid[] ancestorKeys = foundAncestorKeys.ToArray();
if (ancestorKeys.Length == 0 || ancestorKeys.Contains(configuration.StartNodeId.Value) is false)
{
return
[
new ValidationResult(
_localizedTextService.Localize("validation", "invalidStartNode"),
["value"])
];
}
}

return [];
}

Check warning on line 491 in src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ New issue: Complex Method

Validate has a cyclomatic complexity of 11, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 491 in src/Umbraco.Infrastructure/PropertyEditors/MediaPicker3PropertyEditor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v15/dev)

❌ New issue: Bumpy Road Ahead

Validate has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Umbraco.Cms.Core.Security;
using Umbraco.Cms.Core.Serialization;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Navigation;
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Infrastructure.Scoping;
using Umbraco.Cms.Infrastructure.Serialization;
Expand All @@ -37,7 +38,9 @@ public class DataValueReferenceFactoryCollectionTests
Mock.Of<IScopeProvider>(),
Mock.Of<IBackOfficeSecurityAccessor>(),
Mock.Of<IDataTypeConfigurationCache>(),
Mock.Of<ILocalizedTextService>()));
Mock.Of<ILocalizedTextService>(),
Mock.Of<IMediaTypeService>(),
Mock.Of<IMediaNavigationQueryService>()));

private IIOHelper IOHelper { get; } = Mock.Of<IIOHelper>();

Expand Down
Loading
Loading