diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtended.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtended.cs index a85ccda48c..8c0eeef86f 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtended.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtended.cs @@ -43,6 +43,11 @@ namespace Umbraco.Core.Models.PublishedContent internal static IPublishedContentExtended Extend(IPublishedContent content, IEnumerable contentSet) { + // first unwrap content down to the lowest possible level, ie either the deepest inner + // IPublishedContent or the first extended that has added properties. this is to avoid + // nesting extended objects as much as possible, so we try to re-extend that lowest + // object. + var wrapped = content as PublishedContentExtended; while (wrapped != null && ((IPublishedContentExtended)wrapped).HasAddedProperties == false) wrapped = (content = wrapped.Unwrap()) as PublishedContentExtended; @@ -51,14 +56,38 @@ namespace Umbraco.Core.Models.PublishedContent // a model, and then that model has to inherit from PublishedContentExtended, // => implements the internal IPublishedContentExtended. + // here we assume that either the factory just created a model that implements + // IPublishedContentExtended and therefore does not need to be extended again, + // because it can carry the extra property - or that it did *not* create a + // model and therefore returned the original content unchanged. + var model = content.CreateModel(); var extended = model == content // == means the factory did not create a model ? new PublishedContentExtended(content) // so we have to extend : model; // else we can use what the factory returned + // so extended should always implement IPublishedContentExtended, however if + // by mistake the factory returned a different object that does not implement + // IPublishedContentExtended (which would be an error), throw. + // + // see also PublishedContentExtensionsForModels.CreateModel + + // NOTE + // could we lift that constraint and accept that models just be IPublishedContent? + // would then mean that we cannot assume a model is IPublishedContentExtended, so + // either it is, or we need to wrap it. so instead of having + // (Model:IPublishedContentExtended (IPublishedContent)) + // we'd have + // (PublishedContentExtended (Model (IPublishedContent))) + // and it is that bad? any other consequences? + // + // would also allow the factory to cache the model (though that should really + // be done by the content cache, not by the factory). + var extended2 = extended as IPublishedContentExtended; - if (extended2 != null) // always true, but keeps Resharper happy - extended2.SetContentSet(contentSet); + if (extended2 == null) + throw new Exception("Extended does not implement IPublishedContentExtended."); + extended2.SetContentSet(contentSet); return extended2; } diff --git a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs index effc7918d5..f0b8ad92d6 100644 --- a/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs +++ b/src/Umbraco.Core/Models/PublishedContent/PublishedContentExtensionsForModels.cs @@ -1,4 +1,6 @@ -namespace Umbraco.Core.Models.PublishedContent +using System; + +namespace Umbraco.Core.Models.PublishedContent { /// /// Provides strongly typed published content models services. @@ -12,9 +14,32 @@ /// The strongly typed published content model. public static IPublishedContent CreateModel(this IPublishedContent content) { - return PublishedContentModelFactoryResolver.Current.HasValue - ? PublishedContentModelFactoryResolver.Current.Factory.CreateModel(content) - : content; + if (content == null) + return null; + + if (PublishedContentModelFactoryResolver.Current.HasValue == false) + return content; + + // get model + // if factory returns nothing, throw + // if factory just returns what it got, return + var model = PublishedContentModelFactoryResolver.Current.Factory.CreateModel(content); + if (model == null) + throw new Exception("IPublishedContentFactory returned null."); + if (ReferenceEquals(model, content)) + return content; + + // at the moment, other parts of our code assume that all models will + // somehow implement IPublishedContentExtended and not just be IPublishedContent, + // so we'd better check this here to fail as soon as we can. + // + // see also PublishedContentExtended.Extend + + var extended = model as IPublishedContentExtended; + if (extended == null) + throw new Exception("IPublishedContentFactory created an object that does not implement IPublishedContentModelExtended."); + + return model; } } }