From e8fd6a6ecee4be9a486406b14b62c019482a7cd9 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 17 Sep 2013 10:18:26 +0200 Subject: [PATCH] Take care of FIXMEs --- src/Umbraco.Core/ApplicationContext.cs | 2 +- src/Umbraco.Core/Dynamics/DynamicNull.cs | 2 +- .../Dynamics/ExtensionMethodFinder.cs | 21 ++++++- .../PublishedContentModelFactoryImpl.cs | 7 +-- .../PublishedContentOrderedSet.cs | 37 +----------- .../PublishedContent/PublishedContentSet.cs | 28 ++++----- .../Models/DynamicPublishedContent.cs | 2 +- .../Models/PublishedContentTypeCaching.cs | 2 +- .../XmlPublishedCache/PublishedMediaCache.cs | 1 - src/Umbraco.Web/PublishedContentExtensions.cs | 58 ++----------------- 10 files changed, 44 insertions(+), 116 deletions(-) diff --git a/src/Umbraco.Core/ApplicationContext.cs b/src/Umbraco.Core/ApplicationContext.cs index 78ed2c755a..4f44af016c 100644 --- a/src/Umbraco.Core/ApplicationContext.cs +++ b/src/Umbraco.Core/ApplicationContext.cs @@ -116,7 +116,7 @@ namespace Umbraco.Core // public bool IsConfigured { - // fixme - we should not do this - ok for now + // todo - we should not do this - ok for now get { return Configured; diff --git a/src/Umbraco.Core/Dynamics/DynamicNull.cs b/src/Umbraco.Core/Dynamics/DynamicNull.cs index 473df3d203..91d50ce545 100644 --- a/src/Umbraco.Core/Dynamics/DynamicNull.cs +++ b/src/Umbraco.Core/Dynamics/DynamicNull.cs @@ -15,7 +15,7 @@ namespace Umbraco.Core.Dynamics // returned when TryGetMember fails on a DynamicPublishedContent // // so if user does @CurrentPage.TextPages it will get something that is enumerable (but empty) - // fixme - not sure I understand the stuff about .Where, though + // note - not sure I understand the stuff about .Where, though public class DynamicNull : DynamicObject, IEnumerable, IHtmlString { diff --git a/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs b/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs index 215d94e566..cfc8a92ddf 100644 --- a/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs +++ b/src/Umbraco.Core/Dynamics/ExtensionMethodFinder.cs @@ -32,20 +32,28 @@ namespace Umbraco.Core.Dynamics .ToArray(); } + // ORIGINAL CODE IS NOT COMPLETE, DOES NOT HANDLE GENERICS, ETC... + + // so this is an attempt at fixing things, but it's not done yet + // and do we really want to do this? extension methods are not supported on dynamics, period + // we should use strongly typed content instead of dynamics. + + /* + // get all extension methods for type thisType, with name name, // accepting argsCount arguments (not counting the instance of thisType). private static IEnumerable GetExtensionMethods(Type thisType, string name, int argsCount) { var key = string.Format("{0}.{1}::{2}", thisType.FullName, name, argsCount); - var types = thisType.GetBaseTypes(true); // either do this OR have MatchFirstParameter handle the stuff... FIXME? + var types = thisType.GetBaseTypes(true); // either do this OR have MatchFirstParameter handle the stuff... F*XME var methods = AllExtensionMethods .Where(m => m.Name == name) .Where(m => m.GetParameters().Length == argsCount) .Where(m => MatchFirstParameter(thisType, m.GetParameters()[0].ParameterType)); - // fixme - is this what we should cache? + // f*xme - is this what we should cache? return methods; } @@ -88,8 +96,10 @@ namespace Umbraco.Core.Dynamics { // public static int DoSomething(Foo foo, T t1, T t2) // DoSomething(foo, t1, t2) => how can we match?! - return parameterType == argumentType; // fixme of course! + return parameterType == argumentType; // f*xme of course! } + * + */ // BELOW IS THE ORIGINAL CODE... @@ -106,6 +116,10 @@ namespace Umbraco.Core.Dynamics /// private static IEnumerable GetAllExtensionMethods(Type thisType, string name, int argumentCount, bool argsContainsThis) { + // at *least* we can cache the extension methods discovery + var candidates = AllExtensionMethods; + + /* //only scan assemblies we know to contain extension methods (user assemblies) var assembliesToScan = TypeFinder.GetAssembliesWithKnownExclusions(); @@ -124,6 +138,7 @@ namespace Umbraco.Core.Dynamics //add the extension methods defined in IEnumerable candidates = candidates.Concat(typeof(Enumerable).GetMethods(BindingFlags.Static | BindingFlags.Public)); + */ //filter by name var methodsByName = candidates.Where(m => m.Name == name); diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentModelFactoryImpl.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentModelFactoryImpl.cs index ecff5ef0ca..af2bdd6859 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentModelFactoryImpl.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentModelFactoryImpl.cs @@ -34,14 +34,13 @@ namespace Umbraco.Core.Models.PublishedContent if (_constructors.ContainsKey(typeName)) throw new InvalidOperationException(string.Format("More that one type want to be a model for content type {0}.", typeName)); - // should work everywhere, potentially slow? + // should work everywhere, but slow //_constructors[typeName] = constructor; - // note: would it be even faster with a dynamic method? + // much faster with a dynamic method but potential MediumTrust issues // here http://stackoverflow.com/questions/16363838/how-do-you-call-a-constructor-via-an-expression-tree-on-an-existing-object - // but MediumTrust issue? - // fixme - must make sure that works in medium trust + // fast enough and works in MediumTrust // read http://boxbinary.com/2011/10/how-to-run-a-unit-test-in-medium-trust-with-nunitpart-three-umbraco-framework-testing/ var exprArg = Expression.Parameter(typeof(IPublishedContent), "content"); var exprNew = Expression.New(constructor, exprArg); diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentOrderedSet.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentOrderedSet.cs index 4770051649..ffb67876e7 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentOrderedSet.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentOrderedSet.cs @@ -17,6 +17,9 @@ namespace Umbraco.Core.Models.PublishedContent : base(content) { } + // note: because we implement IOrderedEnumerable, we don't need to implement the ThenBy nor + // ThenByDescending methods here, only CreateOrderedEnumerable and that does it. + #region IOrderedEnumerable public IOrderedEnumerable CreateOrderedEnumerable(Func keySelector, IComparer comparer, bool descending) @@ -25,39 +28,5 @@ namespace Umbraco.Core.Models.PublishedContent } #endregion - - // fixme wtf?! -#if IMPLEMENT_LINQ_EXTENSIONS - - // BEWARE! - // here, Source.Whatever() will invoke the System.Linq.Enumerable extension method - // and not the extension methods that we may have defined on IEnumerable or - // IOrderedEnumerable, provided that they are NOT within the scope at compile time. - - #region Wrap methods returning IOrderedEnumerable - - public PublishedContentOrderedSet ThenBy(Func keySelector) - { - return new PublishedContentOrderedSet(((IOrderedEnumerable)Source).ThenBy(keySelector)); - } - - public PublishedContentOrderedSet ThenBy(Func keySelector, IComparer comparer) - { - return new PublishedContentOrderedSet(((IOrderedEnumerable)Source).ThenBy(keySelector, comparer)); - } - - public PublishedContentOrderedSet ThenByDescending(Func keySelector) - { - return new PublishedContentOrderedSet(((IOrderedEnumerable)Source).ThenByDescending(keySelector)); - } - - public PublishedContentOrderedSet ThenByDescending(Func keySelector, IComparer comparer) - { - return new PublishedContentOrderedSet(((IOrderedEnumerable)Source).ThenByDescending(keySelector, comparer)); - } - - #endregion - -#endif } } diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentSet.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentSet.cs index 94f54d03aa..c4b49d84cf 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentSet.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentSet.cs @@ -48,26 +48,20 @@ namespace Umbraco.Core.Models.PublishedContent // wrap an item, ie create the actual clone for this set private T MapContentAsT(T t) { - // fixme - cleanup - return MapContent(t) /*.Content*/ as T; + return MapContent(t) as T; } - // fixme - cleanup - internal IPublishedContentExtended /*Handle*/ MapContent(T t) + internal IPublishedContentExtended MapContent(T t) { IPublishedContentExtended extend; - if (_xContent.TryGetValue(t, out extend) == false) - { - // fixme - cleanup - extend = PublishedContentExtended.Extend(t, this); - //extend = t.Extend(this); - var asT = extend as T; - //var asT = extend.Content as T; - if (asT == null) - throw new InvalidOperationException(string.Format("Failed extend a published content of type {0}." - + "Got {1} when expecting {2}.", t.GetType().FullName, extend /*.Content*/ .GetType().FullName, typeof(T).FullName)); - _xContent[t] = extend; - } + if (_xContent.TryGetValue(t, out extend)) return extend; + + extend = PublishedContentExtended.Extend(t, this); + var asT = extend as T; + if (asT == null) + throw new InvalidOperationException(string.Format("Failed extend a published content of type {0}." + + "Got {1} when expecting {2}.", t.GetType().FullName, extend.GetType().FullName, typeof(T).FullName)); + _xContent[t] = extend; return extend; } @@ -82,7 +76,7 @@ namespace Umbraco.Core.Models.PublishedContent { var extend = MapContent(t); extend.SetIndex(index++); - return extend /*.Content*/ as T; // fixme - cleanup + return extend as T; }).ToArray()); } } diff --git a/src/Umbraco.Web/Models/DynamicPublishedContent.cs b/src/Umbraco.Web/Models/DynamicPublishedContent.cs index 1dbc960253..6d0dc6a012 100644 --- a/src/Umbraco.Web/Models/DynamicPublishedContent.cs +++ b/src/Umbraco.Web/Models/DynamicPublishedContent.cs @@ -1,4 +1,4 @@ -// TODO in v7, #define FIX_GET_PROPERTY_VALUE (see GetPropertyValue region) +// fixme - should #define #undef FIX_GET_PROPERTY_VALUE using System; diff --git a/src/Umbraco.Web/Models/PublishedContentTypeCaching.cs b/src/Umbraco.Web/Models/PublishedContentTypeCaching.cs index 2b6ea34610..36624704f5 100644 --- a/src/Umbraco.Web/Models/PublishedContentTypeCaching.cs +++ b/src/Umbraco.Web/Models/PublishedContentTypeCaching.cs @@ -16,7 +16,7 @@ namespace Umbraco.Web.Models // TODO refactor this when the refresher is ready // FIXME should use the right syntax NOW - class PublishedContentTypeCaching2 : ApplicationEventHandler + class PublishedContentTypeCaching : ApplicationEventHandler { protected override void ApplicationInitialized(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) { diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs index a251c3d428..5090466d06 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/PublishedMediaCache.cs @@ -28,7 +28,6 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache /// /// NOTE: In the future if we want to properly cache all media this class can be extended or replaced when these classes/interfaces are exposed publicly. /// - // fixme - does not implement the content model factory internal class PublishedMediaCache : IPublishedMediaCache { public PublishedMediaCache() diff --git a/src/Umbraco.Web/PublishedContentExtensions.cs b/src/Umbraco.Web/PublishedContentExtensions.cs index 1476faca35..87c809532d 100644 --- a/src/Umbraco.Web/PublishedContentExtensions.cs +++ b/src/Umbraco.Web/PublishedContentExtensions.cs @@ -1,4 +1,4 @@ -// fixme - should define - ok for now +// fixme - should #define // axes navigation is broken in many ways... but fixes would not be 100% // backward compatible... so keep them for v7 or whenever appropriate. #undef FIX_AXES @@ -13,7 +13,6 @@ using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; using Umbraco.Web.Models; using Umbraco.Core; -using Umbraco.Web.PropertyEditors; using ContentType = umbraco.cms.businesslogic.ContentType; namespace Umbraco.Web @@ -120,9 +119,7 @@ namespace Umbraco.Web /// The content may have a property, and that property may not have a value. public static bool HasProperty(this IPublishedContent content, string alias) { - // FIXME that is very wrong, we want the TYPE that was used when creating the IPublishedContent else caching issues!!!! - var contentType = PublishedContentType.Get(content.ItemType, content.DocumentTypeAlias); - return contentType.GetPropertyType(alias) != null; + return content.ContentType.GetPropertyType(alias) != null; } #endregion @@ -295,28 +292,6 @@ namespace Umbraco.Web #region GetPropertyValue - /// - /// Provides a shortcut to GetPropertyValue{T}. - /// - /// The content. - /// The property alias. - /// The value of the content's property identified by the alias. - public static T V(this IPublishedContent content, string alias) - { - return content.GetPropertyValue(alias); - } - - /// - /// Provides a shortcut to GetPropertyValue{T} with recursion. - /// - /// The content. - /// The property alias. - /// The value of the content's property identified by the alias. - public static T Vr(this IPublishedContent content, string alias) - { - return content.GetPropertyValue(alias, true); - } - /// /// Gets the value of a content's property identified by its alias, converted to a specified type. /// @@ -584,27 +559,6 @@ namespace Umbraco.Web return index; } - // fixme - remove - now IPublishedContent.Index() is native - //public static int Index(this IPublishedContent content) - //{ - // // fast: check if content knows its index - // var withIndex = content as IPublishedContentWithIndex; - // if (withIndex != null && withIndex.Index.HasValue) return withIndex.Index.Value; - - // // slow: find content in the content set - // var index = content.Index(content.ContentSet); - // if (withIndex != null) withIndex.Index = index; - // return index; - //} - - //private static int Index(this IPublishedContent content, IEnumerable set) - //{ - // var index = set.FindIndex(n => n.Id == content.Id); - // if (index >= 0) return index; - - // throw new IndexOutOfRangeException("Could not find content in the content set."); - //} - #endregion #region IsSomething: misc. @@ -621,11 +575,9 @@ namespace Umbraco.Web // note: would be better to ensure we have an IPropertyEditorValueConverter for booleans // and then treat the umbracoNaviHide property as a boolean - vs. the hard-coded "1". - var umbracoNaviHide = content.GetProperty(Constants.Conventions.Content.NaviHide); - - // fixme - works but not using the proper converters? - if (umbracoNaviHide == null || umbracoNaviHide.HasValue == false) return true; - return umbracoNaviHide.GetValue() == false; + // rely on the property converter - will return default bool value, ie false, if property + // is not defined, or has no value, else will return its value. + return content.GetPropertyValue(Constants.Conventions.Content.NaviHide) == false; } public static bool IsDocumentType(this IPublishedContent content, string docTypeAlias)