From bcbd4289ea3500e8815b0d98d8c927c05da24a6d Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 29 Sep 2017 15:51:33 +0200 Subject: [PATCH] Facade cleanup and refactoring --- src/Umbraco.Compat7/Umbraco.Compat7.csproj | 2 + .../Cache/DictionaryCacheProvider.cs | 29 +- .../PublishedContent/IPublishedElement.cs | 2 +- .../PublishedContent/PublishedModelFactory.cs | 10 +- src/Umbraco.Core/ReflectionUtilities.cs | 51 +- src/Umbraco.Core/Umbraco.Core.csproj | 4 + src/Umbraco.Examine/Umbraco.Examine.csproj | 2 + src/Umbraco.Tests/Facade/ConvertersTests.cs | 261 +++++++ src/Umbraco.Tests/Facade/FacadeTestObjects.cs | 113 +++ src/Umbraco.Tests/Facade/ModelTypeTests.cs | 55 ++ .../Facade/NestedContentTests.cs | 35 +- .../Facade/PropertyCacheLevelTests.cs | 231 ++++++ .../ModelsAndConvertersTests.cs | 659 ------------------ .../PublishedContentDataTableTests.cs | 33 +- .../PublishedContentTestElements.cs | 5 + .../PublishedContent/PublishedContentTests.cs | 2 +- src/Umbraco.Tests/Umbraco.Tests.csproj | 7 +- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 2 + .../PropertyEditors/NestedContentHelper.cs | 11 +- .../NestedContentManyValueConverter.cs | 12 +- .../NestedContentSingleValueConverter.cs | 4 +- .../NestedContentValueConverterBase.cs | 15 +- .../PublishedCache/FacadeServiceBase.cs | 17 +- src/Umbraco.Web/PublishedCache/IFacade.cs | 11 + .../PublishedCache/IFacadeService.cs | 26 - .../PublishedCache/NuCache/FacadeService.cs | 11 - .../NuCache/PublishedElementProperty.cs | 55 -- .../PublishedCache/PublishedElement.cs | 38 +- .../PublishedElementProperty.cs | 23 - .../PublishedElementPropertyBase.cs | 70 +- .../XmlPublishedCache/Facade.cs | 24 +- .../XmlPublishedCache/FacadeService.cs | 10 - src/Umbraco.Web/Umbraco.Web.csproj | 4 +- src/umbraco.sln.DotSettings | 2 +- 34 files changed, 882 insertions(+), 954 deletions(-) create mode 100644 src/Umbraco.Tests/Facade/ConvertersTests.cs create mode 100644 src/Umbraco.Tests/Facade/FacadeTestObjects.cs create mode 100644 src/Umbraco.Tests/Facade/ModelTypeTests.cs create mode 100644 src/Umbraco.Tests/Facade/PropertyCacheLevelTests.cs delete mode 100644 src/Umbraco.Tests/PublishedContent/ModelsAndConvertersTests.cs delete mode 100644 src/Umbraco.Web/PublishedCache/NuCache/PublishedElementProperty.cs delete mode 100644 src/Umbraco.Web/PublishedCache/PublishedElementProperty.cs diff --git a/src/Umbraco.Compat7/Umbraco.Compat7.csproj b/src/Umbraco.Compat7/Umbraco.Compat7.csproj index 254f3f53d5..f0f819b176 100644 --- a/src/Umbraco.Compat7/Umbraco.Compat7.csproj +++ b/src/Umbraco.Compat7/Umbraco.Compat7.csproj @@ -17,6 +17,7 @@ TRACE;DEBUG;COMPAT7 prompt 4 + latest pdbonly @@ -25,6 +26,7 @@ TRACE;COMPAT7 prompt 4 + latest diff --git a/src/Umbraco.Core/Cache/DictionaryCacheProvider.cs b/src/Umbraco.Core/Cache/DictionaryCacheProvider.cs index 5c8a44b404..22f9209acc 100644 --- a/src/Umbraco.Core/Cache/DictionaryCacheProvider.cs +++ b/src/Umbraco.Core/Cache/DictionaryCacheProvider.cs @@ -7,11 +7,14 @@ using Umbraco.Core.Composing; namespace Umbraco.Core.Cache { - class DictionaryCacheProvider : ICacheProvider + internal class DictionaryCacheProvider : ICacheProvider { private readonly ConcurrentDictionary> _items = new ConcurrentDictionary>(); + // for tests + internal ConcurrentDictionary> Items => _items; + public void ClearAllCache() { _items.Clear(); @@ -19,8 +22,7 @@ namespace Umbraco.Core.Cache public void ClearCacheItem(string key) { - Lazy item; - _items.TryRemove(key, out item); + _items.TryRemove(key, out _); } public void ClearCacheObjectTypes(string typeName) @@ -29,7 +31,6 @@ namespace Umbraco.Core.Cache if (type == null) return; var isInterface = type.IsInterface; - Lazy item; foreach (var kvp in _items .Where(x => { @@ -42,7 +43,7 @@ namespace Umbraco.Core.Cache // otherwise remove exact types (not inherited types) return value == null || (isInterface ? (type.IsInstanceOfType(value)) : (value.GetType() == type)); })) - _items.TryRemove(kvp.Key, out item); + _items.TryRemove(kvp.Key, out _); } public void ClearCacheObjectTypes() @@ -50,7 +51,6 @@ namespace Umbraco.Core.Cache var typeOfT = typeof(T); var isInterface = typeOfT.IsInterface; - Lazy item; foreach (var kvp in _items .Where(x => { @@ -64,7 +64,7 @@ namespace Umbraco.Core.Cache // otherwise remove exact types (not inherited types) return value == null || (isInterface ? (value is T) : (value.GetType() == typeOfT)); })) - _items.TryRemove(kvp.Key, out item); + _items.TryRemove(kvp.Key, out _); } public void ClearCacheObjectTypes(Func predicate) @@ -72,7 +72,6 @@ namespace Umbraco.Core.Cache var typeOfT = typeof(T); var isInterface = typeOfT.IsInterface; - Lazy item; foreach (var kvp in _items .Where(x => { @@ -89,23 +88,21 @@ namespace Umbraco.Core.Cache // run predicate on the 'public key' part only, ie without prefix && predicate(x.Key, (T)value); })) - _items.TryRemove(kvp.Key, out item); + _items.TryRemove(kvp.Key, out _); } public void ClearCacheByKeySearch(string keyStartsWith) { - Lazy item; foreach (var ikvp in _items .Where(kvp => kvp.Key.InvariantStartsWith(keyStartsWith))) - _items.TryRemove(ikvp.Key, out item); + _items.TryRemove(ikvp.Key, out _); } public void ClearCacheByKeyExpression(string regexString) { - Lazy item; foreach (var ikvp in _items .Where(kvp => Regex.IsMatch(kvp.Key, regexString))) - _items.TryRemove(ikvp.Key, out item); + _items.TryRemove(ikvp.Key, out _); } public IEnumerable GetCacheItemsByKeySearch(string keyStartsWith) @@ -126,8 +123,7 @@ namespace Umbraco.Core.Cache public object GetCacheItem(string cacheKey) { - Lazy result; - _items.TryGetValue(cacheKey, out result); // else null + _items.TryGetValue(cacheKey, out var result); // else null return result == null ? null : DictionaryCacheProviderBase.GetSafeLazyValue(result); // return exceptions as null } @@ -136,8 +132,7 @@ namespace Umbraco.Core.Cache var result = _items.GetOrAdd(cacheKey, k => DictionaryCacheProviderBase.GetSafeLazy(getCacheItem)); var value = result.Value; // will not throw (safe lazy) - var eh = value as DictionaryCacheProviderBase.ExceptionHolder; - if (eh == null) + if (!(value is DictionaryCacheProviderBase.ExceptionHolder eh)) return value; // and... it's in the cache anyway - so contrary to other cache providers, diff --git a/src/Umbraco.Core/Models/PublishedContent/IPublishedElement.cs b/src/Umbraco.Core/Models/PublishedContent/IPublishedElement.cs index 45dcbd7c78..31325e8c3a 100644 --- a/src/Umbraco.Core/Models/PublishedContent/IPublishedElement.cs +++ b/src/Umbraco.Core/Models/PublishedContent/IPublishedElement.cs @@ -29,7 +29,7 @@ namespace Umbraco.Core.Models.PublishedContent #region Properties /// - /// Gets the properties of the set. + /// Gets the properties of the element. /// /// Contains one IPublishedProperty for each property defined for the content type, including /// inherited properties. Some properties may have no value. diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs index c09d2a7a38..392508f591 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedModelFactory.cs @@ -14,7 +14,7 @@ namespace Umbraco.Core.Models.PublishedContent private class ModelInfo { public Type ParameterType { get; set; } - public Func Ctor { get; set; } + public Func Ctor { get; set; } public Type ModelType { get; set; } } @@ -69,8 +69,9 @@ namespace Umbraco.Core.Models.PublishedContent if (modelInfos.TryGetValue(typeName, out var modelInfo)) throw new InvalidOperationException($"Both types {type.FullName} and {modelInfo.ModelType.FullName} want to be a model type for content type with alias \"{typeName}\"."); - var ctorFunc = ReflectionUtilities.EmitCtor>(constructor); - modelInfos[typeName] = new ModelInfo { ParameterType = parameterType, ModelType = type, Ctor = ctorFunc }; + // have to use an unsafe ctor because we don't know the types, really + var modelCtor = ReflectionUtilities.EmitCtorUnsafe>(constructor); + modelInfos[typeName] = new ModelInfo { ParameterType = parameterType, ModelType = type, Ctor = modelCtor }; ModelTypeMap[typeName] = type; } @@ -90,7 +91,8 @@ namespace Umbraco.Core.Models.PublishedContent if (modelInfo.ParameterType.IsAssignableFrom(element.GetType()) == false) throw new InvalidOperationException($"Model {modelInfo.ModelType} expects argument of type {modelInfo.ParameterType.FullName}, but got {element.GetType().FullName}."); - return modelInfo.Ctor(element); + // can cast, because we checked when creating the ctor + return (IPublishedElement) modelInfo.Ctor(element); } } } diff --git a/src/Umbraco.Core/ReflectionUtilities.cs b/src/Umbraco.Core/ReflectionUtilities.cs index 382955f34e..002ed257cf 100644 --- a/src/Umbraco.Core/ReflectionUtilities.cs +++ b/src/Umbraco.Core/ReflectionUtilities.cs @@ -214,7 +214,7 @@ namespace Umbraco.Core } // emit - return EmitCtor(declaring, args, returned, ctor); + return EmitCtor(declaring, args, ctor); } /// @@ -249,7 +249,50 @@ namespace Umbraco.Core ThrowInvalidLambda("ctor", declaring, args); // emit - return EmitCtor(declaring, args, declaring, ctor); + return EmitCtor(declaring, args, ctor); + } + + /// + /// Emits a constructor. + /// + /// A lambda representing the constructor. + /// The constructor info. + /// A constructor function. + /// + /// The constructor is emitted in an unsafe way, using the lambda arguments without verifying + /// them at all. This assumes that the calling code is taking care of all verifications, in order + /// to avoid cast errors. + /// + /// Occurs when is not a Func or when its generic + /// arguments do not match those of . + /// Occurs when is null. + public static TLambda EmitCtorUnsafe(ConstructorInfo ctor) + { + if (ctor == null) + throw new ArgumentNullException(nameof(ctor)); + + // get type and args + var declaring = ctor.DeclaringType; + var module = declaring?.Module; + if (module == null) + throw new ArgumentException("Failed to get ctor's declaring type module.", nameof(ctor)); + + // validate lambda type + ValidateCtorLambda(); + + // unsafe - use lambda's args and assume they are correct + //var args = ctor.GetParameters().Select(x => x.ParameterType).ToArray(); + var genArgs = typeof(TLambda).GetGenericArguments(); + var args = new Type[genArgs.Length - 1]; + Array.Copy(genArgs, 0, args, 0, args.Length); + + // emit + var dm = new DynamicMethod(string.Empty, declaring, args, module, true); + var ilgen = dm.GetILGenerator(); + EmitLdargs(ilgen, args.Length); + ilgen.Emit(OpCodes.Newobj, ctor); // ok to just return, it's only objects + ilgen.Emit(OpCodes.Ret); + return (TLambda) (object) dm.CreateDelegate(typeof(TLambda)); } private static void ValidateCtorLambda() @@ -260,9 +303,9 @@ namespace Umbraco.Core throw new ArgumentException($"Lambda {typeLambda} is not a Func.", nameof(TLambda)); } - private static TLambda EmitCtor(Type declaring, Type[] args, Type returned, ConstructorInfo ctor) + private static TLambda EmitCtor(Type declaring, Type[] args, ConstructorInfo ctor) { - var dm = new DynamicMethod(string.Empty, returned, args, declaring.Module, true); + var dm = new DynamicMethod(string.Empty, declaring, args, declaring.Module, true); var ilgen = dm.GetILGenerator(); EmitLdargs(ilgen, args.Length); ilgen.Emit(OpCodes.Newobj, ctor); // ok to just return, it's only objects diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 7a7664f5c4..26042fb478 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -18,6 +18,7 @@ prompt 4 false + latest pdbonly @@ -79,6 +80,9 @@ + + 4.4.0 +