diff --git a/src/Umbraco.Core/Models/Property.cs b/src/Umbraco.Core/Models/Property.cs index 7eb065751f..0ff1c534c4 100644 --- a/src/Umbraco.Core/Models/Property.cs +++ b/src/Umbraco.Core/Models/Property.cs @@ -246,7 +246,7 @@ namespace Umbraco.Core.Models if (!PropertyType.IsPublishing) throw new NotSupportedException("Property type does not support publishing."); var origValue = pvalue.PublishedValue; - pvalue.PublishedValue = ConvertSetValue(pvalue.EditedValue); + pvalue.PublishedValue = PropertyType.ConvertAssignedValue(pvalue.EditedValue); DetectChanges(pvalue.EditedValue, origValue, Ps.Value.ValuesSelector, Ps.Value.PropertyValueComparer, false); } @@ -257,7 +257,7 @@ namespace Umbraco.Core.Models if (!PropertyType.IsPublishing) throw new NotSupportedException("Property type does not support publishing."); var origValue = pvalue.PublishedValue; - pvalue.PublishedValue = ConvertSetValue(null); + pvalue.PublishedValue = PropertyType.ConvertAssignedValue(null); DetectChanges(pvalue.EditedValue, origValue, Ps.Value.ValuesSelector, Ps.Value.PropertyValueComparer, false); } @@ -270,7 +270,7 @@ namespace Umbraco.Core.Models (var pvalue, var change) = GetPValue(languageId, segment, true); var origValue = pvalue.EditedValue; - var setValue = ConvertSetValue(value); + var setValue = PropertyType.ConvertAssignedValue(value); pvalue.EditedValue = setValue; @@ -326,57 +326,6 @@ namespace Umbraco.Core.Models return (pvalue, change); } - private object ConvertSetValue(object value) - { - var isOfExpectedType = PropertyType.IsPropertyTypeValid(value); - - if (isOfExpectedType) - return value; - - // isOfExpectedType is true if value is null - so if false, value is *not* null - // "garbage-in", accept what we can & convert - // throw only if conversion is not possible - - var s = value.ToString(); - - switch (PropertyType.ValueStorageType) - { - case ValueStorageType.Nvarchar: - case ValueStorageType.Ntext: - return s; - - case ValueStorageType.Integer: - if (s.IsNullOrWhiteSpace()) - return null; // assume empty means null - var convInt = value.TryConvertTo(); - if (convInt == false) ThrowTypeException(value, typeof(int), PropertyType.Alias); - return convInt.Result; - - case ValueStorageType.Decimal: - if (s.IsNullOrWhiteSpace()) - return null; // assume empty means null - var convDecimal = value.TryConvertTo(); - if (convDecimal == false) ThrowTypeException(value, typeof(decimal), PropertyType.Alias); - // need to normalize the value (change the scaling factor and remove trailing zeroes) - // because the underlying database is going to mess with the scaling factor anyways. - return convDecimal.Result.Normalize(); - - case ValueStorageType.Date: - if (s.IsNullOrWhiteSpace()) - return null; // assume empty means null - var convDateTime = value.TryConvertTo(); - if (convDateTime == false) ThrowTypeException(value, typeof(DateTime), PropertyType.Alias); - return convDateTime.Result; - } - - return value; - } - - private static void ThrowTypeException(object value, Type expected, string alias) - { - throw new InvalidOperationException($"Cannot assign value \"{value}\" of type \"{value.GetType()}\" to property \"{alias}\" expecting type \"{expected}\"."); - } - /// /// Gets a value indicating whether everything is valid. /// @@ -444,7 +393,7 @@ namespace Umbraco.Core.Models /// True is property value is valid, otherwise false private bool IsValidValue(object value) { - return PropertyType.IsValidPropertyValue(value); + return PropertyType.IsPropertyValueValid(value); } public override object DeepClone() diff --git a/src/Umbraco.Core/Models/PropertyType.cs b/src/Umbraco.Core/Models/PropertyType.cs index 7603496abd..62853ac26a 100644 --- a/src/Umbraco.Core/Models/PropertyType.cs +++ b/src/Umbraco.Core/Models/PropertyType.cs @@ -18,7 +18,7 @@ namespace Umbraco.Core.Models { private static PropertySelectors _selectors; - private readonly bool _isExplicitDbType; + private readonly bool _forceValueStorageType; private string _name; private string _alias; private string _description; @@ -31,6 +31,9 @@ namespace Umbraco.Core.Models private string _validationRegExp; private ContentVariation _variations; + /// + /// Initializes a new instance of the class. + /// public PropertyType(IDataType dataType) { if (dataType == null) throw new ArgumentNullException(nameof(dataType)); @@ -52,43 +55,31 @@ namespace Umbraco.Core.Models _alias = SanitizeAlias(propertyTypeAlias); } + /// + /// Initializes a new instance of the class. + /// public PropertyType(string propertyEditorAlias, ValueStorageType valueStorageType) : this(propertyEditorAlias, valueStorageType, false) { } + /// + /// Initializes a new instance of the class. + /// public PropertyType(string propertyEditorAlias, ValueStorageType valueStorageType, string propertyTypeAlias) : this(propertyEditorAlias, valueStorageType, false, propertyTypeAlias) { } - // fixme - need to explain and understand this explicitDbType thing here - /// - /// Used internally to assign an explicity database type for this property type regardless of what the underlying data type/property editor is. + /// Initializes a new instance of the class. /// - /// - /// - /// - internal PropertyType(string propertyEditorAlias, ValueStorageType valueStorageType, bool isExplicitDbType) + /// Set to true to force the value storage type. Values assigned to + /// the property, eg from the underlying datatype, will be ignored. + internal PropertyType(string propertyEditorAlias, ValueStorageType valueStorageType, bool forceValueStorageType, string propertyTypeAlias = null) { - _isExplicitDbType = isExplicitDbType; _propertyEditorAlias = propertyEditorAlias; _valueStorageType = valueStorageType; - _variations = ContentVariation.InvariantNeutral; - } - - /// - /// Used internally to assign an explicity database type for this property type regardless of what the underlying data type/property editor is. - /// - /// - /// - /// - /// - internal PropertyType(string propertyEditorAlias, ValueStorageType valueStorageType, bool isExplicitDbType, string propertyTypeAlias) - { - _isExplicitDbType = isExplicitDbType; - _propertyEditorAlias = propertyEditorAlias; - _valueStorageType = valueStorageType; - _alias = SanitizeAlias(propertyTypeAlias); + _forceValueStorageType = forceValueStorageType; + _alias = propertyTypeAlias == null ? null : SanitizeAlias(propertyTypeAlias); _variations = ContentVariation.InvariantNeutral; } @@ -173,8 +164,7 @@ namespace Umbraco.Core.Models get => _valueStorageType; set { - //don't allow setting this if an explicit declaration has been made in the ctor - if (_isExplicitDbType) return; + if (_forceValueStorageType) return; // ignore changes SetPropertyValueAndDetectChanges(value, ref _valueStorageType, Selectors.ValueStorageType); } } @@ -211,7 +201,7 @@ namespace Umbraco.Core.Models } /// - /// Gets or sets the regular expression for validation of legacy DataTypes fixme?? + /// Gets or sets the regular expression validating the property values. /// [DataMember] public string ValidationRegExp @@ -273,93 +263,160 @@ namespace Umbraco.Core.Models /// If the value is of the expected type, it can be directly assigned to the property. /// Otherwise, some conversion is required. /// - public bool IsPropertyTypeValid(object value) + public bool IsOfExpectedPropertyType(object value) { // null values are assumed to be ok if (value == null) return true; // check if the type of the value matches the type from the DataType/PropertyEditor + // then it can be directly assigned, anything else requires conversion var valueType = value.GetType(); - - //TODO Add PropertyEditor Type validation when its relevant to introduce - /*bool isEditorModel = value is IEditorModel; - if (isEditorModel && DataTypeControlId != Guid.Empty) + switch (ValueStorageType) { - //Find PropertyEditor by Id - var propertyEditor = PropertyEditorResolver.Current.GetById(DataTypeControlId); + case ValueStorageType.Integer: + return valueType == typeof(int); + case ValueStorageType.Decimal: + return valueType == typeof(decimal); + case ValueStorageType.Date: + return valueType == typeof(DateTime); + case ValueStorageType.Nvarchar: + return valueType == typeof(string); + case ValueStorageType.Ntext: + return valueType == typeof(string); + default: + throw new NotSupportedException($"Not supported storage type \"{ValueStorageType}\"."); + } + } - if (propertyEditor == null) - return false;//Throw exception instead? + /// + /// Determines whether a value can be assigned to a property. + /// + public bool IsValueAssignable(object value) => TryConvertAssignedValue(value, false, out _); - //Get the generic parameter of the PropertyEditor and check it against the type of the passed in (object) value - Type argument = propertyEditor.GetType().BaseType.GetGenericArguments()[0]; - return argument == type; - }*/ + /// + /// Converts a value assigned to a property. + /// + /// + /// The input value can be pretty much anything, and is converted to the actual Clr type + /// expected by the property (eg an integer if the property values are integers). + /// Throws if the value cannot be converted. + /// + public object ConvertAssignedValue(object value) => TryConvertAssignedValue(value, true, out var converted) ? converted : null; - if (PropertyEditorAlias.IsNullOrWhiteSpace() == false) // fixme - always true? + /// + /// Tries to convert a value assigned to a property. + /// + /// + /// + /// + public bool TryConvertAssignedValue(object value, out object converted) => TryConvertAssignedValue(value, false, out converted); + + private bool TryConvertAssignedValue(object value, bool throwOnError, out object converted) + { + var isOfExpectedType = IsOfExpectedPropertyType(value); + if (isOfExpectedType) { - // simple validation using the DatabaseType from the DataTypeDefinition - // and the Type of the passed in value - switch (ValueStorageType) - { - // fixme breaking! - case ValueStorageType.Integer: - return valueType == typeof(int); - case ValueStorageType.Decimal: - return valueType == typeof(decimal); - case ValueStorageType.Date: - return valueType == typeof(DateTime); - case ValueStorageType.Nvarchar: - return valueType == typeof(string); - case ValueStorageType.Ntext: - return valueType == typeof(string); - } + converted = value; + return true; } - // fixme - never reached + makes no sense? - // fallback for simple value types when no Control Id or Database Type is set - if (valueType.IsPrimitive || value is string) - return true; + // isOfExpectedType is true if value is null - so if false, value is *not* null + // "garbage-in", accept what we can & convert + // throw only if conversion is not possible - return false; + var s = value.ToString(); + converted = null; + + switch (ValueStorageType) + { + case ValueStorageType.Nvarchar: + case ValueStorageType.Ntext: + { + converted = s; + return true; + } + + case ValueStorageType.Integer: + if (s.IsNullOrWhiteSpace()) + return true; // assume empty means null + var convInt = value.TryConvertTo(); + if (convInt) + { + converted = convInt.Result; + return true; + } + if (throwOnError) + ThrowTypeException(value, typeof(int), Alias); + return false; + + case ValueStorageType.Decimal: + if (s.IsNullOrWhiteSpace()) + return true; // assume empty means null + var convDecimal = value.TryConvertTo(); + if (convDecimal) + { + // need to normalize the value (change the scaling factor and remove trailing zeroes) + // because the underlying database is going to mess with the scaling factor anyways. + converted = convDecimal.Result.Normalize(); + return true; + } + if (throwOnError) + ThrowTypeException(value, typeof(decimal), Alias); + return false; + + case ValueStorageType.Date: + if (s.IsNullOrWhiteSpace()) + return true; // assume empty means null + var convDateTime = value.TryConvertTo(); + if (convDateTime) + { + converted = convDateTime.Result; + return true; + } + if (throwOnError) + ThrowTypeException(value, typeof(DateTime), Alias); + return false; + + default: + throw new NotSupportedException($"Not supported storage type \"{ValueStorageType}\"."); + } + } + + private static void ThrowTypeException(object value, Type expected, string alias) + { + throw new InvalidOperationException($"Cannot assign value \"{value}\" of type \"{value.GetType()}\" to property \"{alias}\" expecting type \"{expected}\"."); } /// /// Determines whether a value is valid for this property type. /// - public bool IsValidPropertyValue(object value) + public bool IsPropertyValueValid(object value) { - //If the Property is mandatory and value is null or empty, return false as the validation failed - if (Mandatory && (value == null || string.IsNullOrEmpty(value.ToString()))) + var stringValue = Mandatory || !string.IsNullOrWhiteSpace(ValidationRegExp) + ? value?.ToString() + : null; + + // validate mandatory property value + if (Mandatory && string.IsNullOrWhiteSpace(stringValue)) return false; - //Check against Regular Expression for Legacy DataTypes - Validation exists and value is not null: - if(string.IsNullOrEmpty(ValidationRegExp) == false && (value != null && string.IsNullOrEmpty(value.ToString()) == false)) + // validate regular expression if appropriate (have a regex and a string value) + if (!string.IsNullOrWhiteSpace(ValidationRegExp) && !string.IsNullOrWhiteSpace(stringValue)) { try { - var regexPattern = new Regex(ValidationRegExp); - return regexPattern.IsMatch(value.ToString()); + return new Regex(ValidationRegExp).IsMatch(stringValue); } catch { - throw new Exception($"Invalid validation expression on property {Alias}"); + throw new Exception($"Invalid validation expression on property {Alias}."); } - } - //TODO: We must ensure that the property value can actually be saved based on the specified database type - - //TODO Add PropertyEditor validation when its relevant to introduce - /*if (value is IEditorModel && DataTypeControlId != Guid.Empty) - { - //Find PropertyEditor by Id - var propertyEditor = PropertyEditorResolver.Current.GetById(DataTypeControlId); - - //TODO Get the validation from the PropertyEditor if a validation attribute exists - //Will probably need to reflect the PropertyEditor in order to apply the validation - }*/ + // fixme - todo + // ensure that the property value complies with the value storage type, ie can be saved + // plug PropertyEditor validation - when it's a thing return true; } diff --git a/src/Umbraco.Core/Models/PublishedContent/IPublishedContentTypeFactory.cs b/src/Umbraco.Core/Models/PublishedContent/IPublishedContentTypeFactory.cs index ae7fee734e..e75e8a4eb9 100644 --- a/src/Umbraco.Core/Models/PublishedContent/IPublishedContentTypeFactory.cs +++ b/src/Umbraco.Core/Models/PublishedContent/IPublishedContentTypeFactory.cs @@ -38,7 +38,10 @@ /// /// Notifies the factory of datatype changes. /// - /// This is so the factory can flush its caches. - void NotifyDataTypeChanges(); // fixme never invoked! + /// + /// This is so the factory can flush its caches. + /// Invoked by the IPublishedSnapshotService. + /// + void NotifyDataTypeChanges(int[] ids); } } diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeFactory.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeFactory.cs index 1e349969a6..1387b00ae8 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeFactory.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentTypeFactory.cs @@ -92,5 +92,18 @@ namespace Umbraco.Core.Models.PublishedContent _publishedDataTypes = null; } } + + /// + public void NotifyDataTypeChanges(int[] ids) + { + lock (_publishedDataTypesLocker) + { + foreach (var id in ids) + _publishedDataTypes.Remove(id); + var dataTypes = _dataTypeService.GetAll(ids); + foreach (var dataType in dataTypes) + _publishedDataTypes[dataType.Id] = new PublishedDataType(dataType.Id, dataType.EditorAlias, dataType is DataType d ? d.GetLazyConfiguration() : new Lazy(() => dataType.Configuration))); + } + } } } diff --git a/src/Umbraco.Core/PropertyEditors/ValueConverters/JsonValueConverter.cs b/src/Umbraco.Core/PropertyEditors/ValueConverters/JsonValueConverter.cs index 42c128dca5..d9a56df9e2 100644 --- a/src/Umbraco.Core/PropertyEditors/ValueConverters/JsonValueConverter.cs +++ b/src/Umbraco.Core/PropertyEditors/ValueConverters/JsonValueConverter.cs @@ -16,6 +16,16 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters [DefaultPropertyValueConverter] public class JsonValueConverter : PropertyValueConverterBase { + private readonly PropertyEditorCollection _propertyEditors; + + /// + /// Initializes a new instance of the class. + /// + public JsonValueConverter(PropertyEditorCollection propertyEditors) + { + _propertyEditors = propertyEditors; + } + /// /// It is a converter for any value type that is "JSON" /// @@ -23,9 +33,8 @@ namespace Umbraco.Core.PropertyEditors.ValueConverters /// public override bool IsConverter(PublishedPropertyType propertyType) { - var propertyEditor = Current.PropertyEditors[propertyType.EditorAlias]; // fixme inject! - if (propertyEditor == null) return false; - return propertyEditor.ValueEditor.ValueType.InvariantEquals(ValueTypes.Json); + return _propertyEditors.TryGet(propertyType.EditorAlias, out var editor) + && editor.ValueEditor.ValueType.InvariantEquals(ValueTypes.Json); } public override Type GetPropertyValueType(PublishedPropertyType propertyType) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 86599e7a65..224a35f3c3 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -757,6 +757,12 @@ namespace Umbraco.Web.PublishedCache.NuCache using (_contentStore.GetWriter(_scopeProvider)) using (_mediaStore.GetWriter(_scopeProvider)) { + // fixme - datatype lock + // this is triggering datatypes reload in the factory, and right after we create some + // content types by loading them ... there's a race condition here, which would require + // some locking on datatypes + _publishedContentTypeFactory.NotifyDataTypeChanges(idsA); + if (!(_serviceContext.ContentService is ContentService)) throw new Exception("oops"); diff --git a/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs b/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs index b2bc3424b6..675b3e4d24 100644 --- a/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs +++ b/src/Umbraco.Web/PublishedCache/PublishedContentTypeCache.cs @@ -41,6 +41,8 @@ namespace Umbraco.Web.PublishedCache _publishedContentTypeFactory = publishedContentTypeFactory; } + // note: cache clearing is performed by XmlStore + /// /// Clears all cached content types. /// diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedSnapshotService.cs index 7e0868dbe8..35b3d1f873 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedSnapshotService.cs @@ -22,6 +22,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache { private readonly XmlStore _xmlStore; private readonly RoutesCache _routesCache; + private readonly IPublishedContentTypeFactory _publishedContentTypeFactory; private readonly PublishedContentTypeCache _contentTypeCache; private readonly IDomainService _domainService; private readonly IMemberService _memberService; @@ -77,6 +78,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache : base(publishedSnapshotAccessor) { _routesCache = new RoutesCache(); + _publishedContentTypeFactory = publishedContentTypeFactory; _contentTypeCache = contentTypeCache ?? new PublishedContentTypeCache(serviceContext.ContentTypeService, serviceContext.MediaTypeService, serviceContext.MemberTypeService, publishedContentTypeFactory, logger); @@ -234,6 +236,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache public override void Notify(DataTypeCacheRefresher.JsonPayload[] payloads) { + _publishedContentTypeFactory.NotifyDataTypeChanges(payloads.Select(x => x.Id).ToArray()); _xmlStore.Notify(payloads); }