Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -1,38 +1,27 @@
using Umbraco.Cms.Api.Management.Extensions;
using Umbraco.Cms.Api.Management.ViewModels.Content;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Models.ContentEditing;

namespace Umbraco.Cms.Api.Management.Factories;

internal abstract class ContentEditingPresentationFactory<TValueModel, TVariantModel>
where TValueModel : ValueModelBase
where TVariantModel : VariantModelBase
{
protected TContentEditingModel MapContentEditingModel<TContentEditingModel>(ContentModelBase<TValueModel, TVariantModel> contentModel)
protected TContentEditingModel MapContentEditingModel<TContentEditingModel>(
ContentModelBase<TValueModel, TVariantModel> contentModel)
where TContentEditingModel : ContentEditingModelBase, new()
{
TVariantModel? invariantVariant = contentModel.Variants.FirstOrDefault(variant => variant.DoesNotVaryByCulture() && variant.DoesNotVaryBySegment());
TValueModel[] invariantProperties = contentModel.Values.Where(value => value.DoesNotVaryByCulture() && value.DoesNotVaryBySegment()).ToArray();

PropertyValueModel ToPropertyValueModel(TValueModel valueModel)
=> new() { Alias = valueModel.Alias, Value = valueModel.Value };

return new TContentEditingModel
=> new()
{
InvariantName = invariantVariant?.Name,
InvariantProperties = invariantProperties.Select(ToPropertyValueModel).ToArray(),
Properties = contentModel
.Values
.Select(value => new PropertyValueModel
{
Alias = value.Alias, Value = value.Value, Culture = value.Culture, Segment = value.Segment
}).ToArray(),
Variants = contentModel
.Variants
.Select(variant => new VariantModel
{
Culture = variant.Culture,
Segment = variant.Segment,
Name = variant.Name,
Properties = contentModel
.Values
.Where(value => value.Culture == variant.Culture && value.Segment == variant.Segment)
.Select(ToPropertyValueModel).ToArray()
Culture = variant.Culture, Segment = variant.Segment, Name = variant.Name
})
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

public abstract class ContentEditingModelBase
{
public string? InvariantName { get; set; }

public IEnumerable<PropertyValueModel> InvariantProperties { get; set; } = Array.Empty<PropertyValueModel>();
public IEnumerable<PropertyValueModel> Properties { get; set; } = Array.Empty<PropertyValueModel>();

public IEnumerable<VariantModel> Variants { get; set; } = Array.Empty<VariantModel>();
}
4 changes: 4 additions & 0 deletions src/Umbraco.Core/Models/ContentEditing/PropertyValueModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ public class PropertyValueModel
public required string Alias { get; set; }

public required object? Value { get; set; }

public string? Culture { get; set; }

public string? Segment { get; set; }
}
2 changes: 0 additions & 2 deletions src/Umbraco.Core/Models/ContentEditing/VariantModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@ public class VariantModel
public string? Segment { get; set; }

public required string Name { get; set; }

public required IEnumerable<PropertyValueModel> Properties { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public sealed class PropertyValidationContext

public required IEnumerable<string> CulturesBeingValidated { get; init; }

public required IEnumerable<string> SegmentsBeingValidated { get; init; }
public required IEnumerable<string?> SegmentsBeingValidated { get; init; }

public static PropertyValidationContext Empty() => new()
{
Expand Down
18 changes: 16 additions & 2 deletions src/Umbraco.Core/Services/ContentBlueprintEditingService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.Extensions.Logging;

Check notice on line 1 in src/Umbraco.Core/Services/ContentBlueprintEditingService.cs

View check run for this annotation

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

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 58.00% to 55.77%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Events;
Expand Down Expand Up @@ -86,7 +86,7 @@

IContent blueprint = result.Result.Content!;

if (ValidateUniqueName(createModel.InvariantName ?? string.Empty, blueprint) is false)
if (ValidateUniqueNames(createModel.Variants, blueprint) is false)
{
return Attempt.FailWithStatus(ContentEditingOperationStatus.DuplicateName, new ContentCreateResult());
}
Expand Down Expand Up @@ -133,7 +133,7 @@
return Attempt.FailWithStatus(ContentEditingOperationStatus.NotFound, new ContentUpdateResult());
}

if (ValidateUniqueName(updateModel.InvariantName ?? string.Empty, blueprint) is false)
if (ValidateUniqueNames(updateModel.Variants, blueprint) is false)
{
return Attempt.FailWithStatus(ContentEditingOperationStatus.DuplicateName, new ContentUpdateResult());
}
Expand Down Expand Up @@ -249,4 +249,18 @@
IEnumerable<IContent> existing = ContentService.GetBlueprintsForContentTypes(content.ContentTypeId);
return existing.Any(c => c.Name == name && c.Id != content.Id) is false;
}

private bool ValidateUniqueNames(IEnumerable<VariantModel> variants, IContent content)
{
IContent[] existing = ContentService.GetBlueprintsForContentTypes(content.ContentTypeId).ToArray();
foreach (VariantModel variant in variants)
{
if (existing.Any(c => c.GetCultureName(variant.Culture) == variant.Name && c.Id != content.Id))
{
return false;
}
}

return true;
}
}
39 changes: 11 additions & 28 deletions src/Umbraco.Core/Services/ContentEditingServiceBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@
return null;
}

if (contentType.VariesByNothing() && contentEditingModelBase.InvariantName.IsNullOrWhiteSpace())
if (contentType.VariesByNothing() && contentEditingModelBase.Variants.Any(v => v.Culture is null && v.Segment is null) is false)

Check warning on line 321 in src/Umbraco.Core/Services/ContentEditingServiceBase.cs

View check run for this annotation

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

❌ New issue: Complex Conditional

TryGetAndValidateContentType has 1 complex conditionals with 2 branches, threshold = 2. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.
{
// does not vary by anything and is missing the invariant name = invalid
operationStatus = ContentEditingOperationStatus.ContentTypeCultureVarianceMismatch;
Expand All @@ -341,23 +341,13 @@

var propertyTypesByAlias = contentType.CompositionPropertyTypes.ToDictionary(pt => pt.Alias);
var propertyValuesAndVariance = contentEditingModelBase
.InvariantProperties
.Properties
.Select(pv => new
{
VariesByCulture = false,
VariesBySegment = false,
VariesByCulture = pv.Culture is not null,
VariesBySegment = pv.Segment is not null,
PropertyValue = pv
})
.Union(contentEditingModelBase
.Variants
.SelectMany(v => v
.Properties
.Select(vpv => new
{
VariesByCulture = contentType.VariesByCulture(),
VariesBySegment = v.Segment.IsNullOrWhiteSpace() == false,
PropertyValue = vpv
})))
.ToArray();

// verify that all property values are defined as property types
Expand Down Expand Up @@ -446,7 +436,7 @@
else
{
// this should be validated already so it's OK to throw an exception here
content.Name = contentEditingModelBase.InvariantName
content.Name = contentEditingModelBase.Variants.FirstOrDefault(v => v.Culture is null && v.Segment is null)?.Name
?? throw new ArgumentException("Could not find a culture invariant variant", nameof(contentEditingModelBase));
}
}
Expand All @@ -456,21 +446,14 @@
// create a mapping dictionary for all content type property types by their property aliases
Dictionary<string, IPropertyType> propertyTypesByAlias = GetPropertyTypesByAlias(contentType);

// flatten the invariant and variant property values from the model into one array, and remove any properties
// that do not exist on the content type
var propertyValues = contentEditingModelBase
.InvariantProperties
.Select(pv => new { Culture = (string?)null, Segment = (string?)null, Alias = pv.Alias, Value = pv.Value })
.Union(contentEditingModelBase
.Variants
.SelectMany(v => v
.Properties
.Select(vpv => new { Culture = v.Culture, Segment = v.Segment, Alias = vpv.Alias, Value = vpv.Value })))
// remove any properties that do not exist on the content type
PropertyValueModel[] propertyValues = contentEditingModelBase
.Properties
.Where(propertyValue => propertyTypesByAlias.ContainsKey(propertyValue.Alias))
.ToArray();

// update all properties on the content item
foreach (var propertyValue in propertyValues)
foreach (PropertyValueModel propertyValue in propertyValues)
{
// the following checks should already have been validated by now, so it's OK to throw exceptions here
if(propertyTypesByAlias.TryGetValue(propertyValue.Alias, out IPropertyType? propertyType) == false
Expand All @@ -491,8 +474,8 @@
// create a mapping dictionary for all content type property types by their property aliases
Dictionary<string, IPropertyType> propertyTypesByAlias = GetPropertyTypesByAlias(contentType);
var knownPropertyAliases = contentEditingModelBase
.InvariantProperties.Select(pv => pv.Alias)
.Union(contentEditingModelBase.Variants.SelectMany(v => v.Properties.Select(vpv => vpv.Alias)))
.Properties
.Select(pv => pv.Alias)
.Distinct()
.ToArray();

Expand Down
31 changes: 18 additions & 13 deletions src/Umbraco.Core/Services/ContentPublishingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,28 +238,33 @@ public async Task<Attempt<ContentPublishingResult, ContentPublishingOperationSta

private async Task<ContentValidationResult> ValidateCurrentContentAsync(IContent content, string[] cultures)
{
IEnumerable<string?> effectiveCultures = content.ContentType.VariesByCulture()
? cultures.Union([null])
: [null];

// Would be better to be able to use a mapper/factory, but currently all that functionality is very much presentation logic.
var model = new ContentUpdateModel()
{
InvariantName = content.Name,
// NOTE KJA: this needs redoing; we need to make an informed decision whether to include invariant properties, depending on if editing invariant properties is allowed on all variants, or if the default language is included in cultures
InvariantProperties = content.Properties.Where(x => x.PropertyType.VariesByCulture() is false).Select(x => new PropertyValueModel()
{
Alias = x.Alias,
Value = x.GetValue()
}),
Properties = effectiveCultures.SelectMany(culture =>
content.Properties.Select(property => property.PropertyType.VariesByCulture() == (culture is not null)
? new PropertyValueModel
{
Alias = property.Alias,
Value = property.GetValue(culture: culture, segment: null, published: false),
Culture = culture
}
: null)
.WhereNotNull())
.ToArray(),
Variants = cultures.Select(culture => new VariantModel()
{
Name = content.GetPublishName(culture) ?? string.Empty,
Culture = culture,
Segment = null,
Properties = content.Properties.Where(prop => prop.PropertyType.VariesByCulture()).Select(prop => new PropertyValueModel()
{
Alias = prop.Alias,
Value = prop.GetValue(culture: culture, segment: null, published: false)
})
})
Segment = null
}).ToArray()
};

IContentType? contentType = _contentTypeService.Get(content.ContentType.Key)!;
ContentValidationResult validationResult = await _contentValidationService.ValidatePropertiesAsync(model, contentType, cultures);
return validationResult;
Expand Down
66 changes: 52 additions & 14 deletions src/Umbraco.Core/Services/ContentValidationServiceBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,55 +31,93 @@

IPropertyType[] contentTypePropertyTypes = contentType.CompositionPropertyTypes.ToArray();
IPropertyType[] invariantPropertyTypes = contentTypePropertyTypes
.Where(propertyType => propertyType.VariesByNothing())
.Where(propertyType => propertyType.Variations == ContentVariation.Nothing)
.ToArray();
IPropertyType[] cultureVariantPropertyTypes = contentTypePropertyTypes
.Where(propertyType => propertyType.Variations == ContentVariation.Culture)
.ToArray();
IPropertyType[] segmentVariantPropertyTypes = contentTypePropertyTypes
.Where(propertyType => propertyType.Variations == ContentVariation.Segment)
.ToArray();
IPropertyType[] cultureAndSegmentVariantPropertyTypes = contentTypePropertyTypes
.Where(propertyType => propertyType.Variations == ContentVariation.CultureAndSegment)
.ToArray();
IPropertyType[] variantPropertyTypes = contentTypePropertyTypes.Except(invariantPropertyTypes).ToArray();

var cultures = culturesToValidate?.WhereNotNull().Except(["*"]).ToArray();
if (cultures?.Any() is not true)
{
cultures = await GetCultureCodes();
}

// we don't have any managed segments, so we have to make do with the ones passed in the model
var segments = contentEditingModelBase.Variants
.Where(variant => variant.Culture is null || cultures.Contains(variant.Culture))
.DistinctBy(variant => variant.Segment).Select(variant => variant.Segment)
.WhereNotNull()
.ToArray();
var segments =
new string?[] { null }
.Union(contentEditingModelBase.Variants
.Where(variant => variant.Culture is null || cultures.Contains(variant.Culture))
.DistinctBy(variant => variant.Segment).Select(variant => variant.Segment)
.WhereNotNull()
)
.ToArray();

foreach (IPropertyType propertyType in invariantPropertyTypes)
{
var validationContext = new PropertyValidationContext
{
Culture = null, Segment = null, CulturesBeingValidated = cultures, SegmentsBeingValidated = segments
};

PropertyValueModel? propertyValueModel = contentEditingModelBase.InvariantProperties.FirstOrDefault(propertyValue => propertyValue.Alias == propertyType.Alias);
PropertyValueModel? propertyValueModel = contentEditingModelBase
.Properties
.FirstOrDefault(propertyValue => propertyValue.Alias == propertyType.Alias && propertyValue.Culture is null && propertyValue.Segment is null);
validationErrors.AddRange(ValidateProperty(propertyType, propertyValueModel, validationContext));
}

if (variantPropertyTypes.Any() is false)
foreach (IPropertyType propertyType in cultureVariantPropertyTypes)
{
return new ContentValidationResult { ValidationErrors = validationErrors };
foreach (var culture in cultures)
{
var validationContext = new PropertyValidationContext
{
Culture = culture, Segment = null, CulturesBeingValidated = cultures, SegmentsBeingValidated = segments
};

PropertyValueModel? propertyValueModel = contentEditingModelBase
.Properties
.FirstOrDefault(propertyValue => propertyValue.Alias == propertyType.Alias && propertyValue.Culture.InvariantEquals(culture) && propertyValue.Segment is null);
validationErrors.AddRange(ValidateProperty(propertyType, propertyValueModel, validationContext));
}
}

foreach (IPropertyType propertyType in segmentVariantPropertyTypes)
{
foreach (var segment in segments)
{
var validationContext = new PropertyValidationContext
{
Culture = null, Segment = segment, CulturesBeingValidated = cultures, SegmentsBeingValidated = segments
};

PropertyValueModel? propertyValueModel = contentEditingModelBase
.Properties
.FirstOrDefault(propertyValue => propertyValue.Alias == propertyType.Alias && propertyValue.Culture is null && propertyValue.Segment.InvariantEquals(segment));
validationErrors.AddRange(ValidateProperty(propertyType, propertyValueModel, validationContext));
}
}

foreach (IPropertyType propertyType in variantPropertyTypes)
foreach (IPropertyType propertyType in cultureAndSegmentVariantPropertyTypes)
{
foreach (var culture in cultures)
{
foreach (var segment in segments.DefaultIfEmpty(null))
{
var validationContext = new PropertyValidationContext
{
Culture = culture, Segment = segment, CulturesBeingValidated = cultures, SegmentsBeingValidated = segments
};

PropertyValueModel? propertyValueModel = contentEditingModelBase
.Variants
.FirstOrDefault(variant => string.Equals(variant.Culture, culture, StringComparison.InvariantCultureIgnoreCase) && string.Equals(segment, variant.Segment, StringComparison.InvariantCultureIgnoreCase))?
.Properties
.FirstOrDefault(propertyValue => propertyValue.Alias == propertyType.Alias);
.FirstOrDefault(propertyValue => propertyValue.Alias == propertyType.Alias && propertyValue.Culture.InvariantEquals(culture) && propertyValue.Segment.InvariantEquals(segment));

Check warning on line 120 in src/Umbraco.Core/Services/ContentValidationServiceBase.cs

View check run for this annotation

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

❌ Getting worse: Complex Method

HandlePropertiesValidationAsync increases in cyclomatic complexity from 9 to 19, 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 120 in src/Umbraco.Core/Services/ContentValidationServiceBase.cs

View check run for this annotation

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

❌ New issue: Bumpy Road Ahead

HandlePropertiesValidationAsync has 3 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.
validationErrors.AddRange(ValidateProperty(propertyType, propertyValueModel, validationContext));
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/Umbraco.Core/Services/MemberContentEditingService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ private bool ValidateAccessToSensitiveProperties(IMember member, IMemberType mem

var sensitivePropertyAliases = memberType.GetSensitivePropertyTypeAliases().ToArray();
return updateModel
.InvariantProperties
.Union(updateModel.Variants.SelectMany(variant => variant.Properties))
.Properties
.Select(property => property.Alias)
.Intersect(sensitivePropertyAliases, StringComparer.OrdinalIgnoreCase)
.Any() is false;
Expand Down
Loading