U4-7731 - Models Builder - Handle content type name collisions better

Created by Shannon Deminick 14 Jan 2016, 09:49:41 Updated by Shannon Deminick 20 Jan 2016, 16:47:16

Tags: ModelsBuilder

Relates to: U4-7734

Relates to: U4-7633

Currently I get a YSOD ( I think because I have a Document Type named 'test' and a Member Type named 'test' ... it could be another issue though, not sure ). This YSOD prevents me from visiting any page including the back office pages:

Server Error in '/' Application.

Type name "Test" is used for Content:"test", Member:"test". Should be used for one type only.

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code. 

Exception Details: System.InvalidOperationException: Type name "Test" is used for Content:"test", Member:"test". Should be used for one type only.

Source Error: 


Line 24:             // if factory returns nothing, throw
Line 25:             // if factory just returns what it got, return
Line 26:             var model = PublishedContentModelFactoryResolver.Current.Factory.CreateModel(content);
Line 27:             if (model == null)
Line 28:                 throw new Exception("IPublishedContentFactory returned null.");

Stack trace:

[InvalidOperationException: Type name "Test" is used for Content:"test", Member:"test". Should be used for one type only.]
   Zbu.ModelsBuilder.Building.Builder.Prepare() +2488
   Zbu.ModelsBuilder.Umbraco.PureLiveModelFactory.GenerateModelsCode() +586
   Zbu.ModelsBuilder.Umbraco.PureLiveModelFactory.EnsureModels() +380
   Zbu.ModelsBuilder.Umbraco.PureLiveModelFactory.CreateModel(IPublishedContent content) +21
   Umbraco.Core.Models.PublishedContent.PublishedContentExtensionsForModels.CreateModel(IPublishedContent content) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\Models\PublishedContent\PublishedContentExtensionsForModels.cs:26
   Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent.InitializeStructure() in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Web\PublishedCache\XmlPublishedCache\XmlPublishedContent.cs:352
   Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent..ctor(XmlNode xmlNode, Boolean isPreviewing, Boolean lazyInitialize) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Web\PublishedCache\XmlPublishedCache\XmlPublishedContent.cs:50
   Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent.InitializeChildren() in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Web\PublishedCache\XmlPublishedCache\XmlPublishedContent.cs:455
   Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent..ctor(XmlNode xmlNode, Boolean isPreviewing) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Web\PublishedCache\XmlPublishedCache\XmlPublishedContent.cs:35
   Umbraco.Web.PublishedCache.XmlPublishedCache.<>c__DisplayClass14_0.<ConvertToDocuments>b__0(XmlNode xmlNode) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Web\PublishedCache\XmlPublishedCache\PublishedContentCache.cs:255
   System.Linq.WhereSelectEnumerableIterator`2.MoveNext() +223
   System.Linq.Buffer`1..ctor(IEnumerable`1 source) +153
   System.Linq.Enumerable.ToArray(IEnumerable`1 source) +106
   Articulate.ArticulateRoutes.MapRoutes(RouteCollection routes, ContextualPublishedCache umbracoCache) in X:\Projects\Articulate\Articulate.Project\src\Articulate\ArticulateRoutes.cs:22
   Articulate.UmbracoEventHandler.ApplicationStarted(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) in X:\Projects\Articulate\Articulate.Project\src\Articulate\UmbracoEventHandler.cs:52
   Umbraco.Core.ApplicationEventHandler.OnApplicationStarted(UmbracoApplicationBase umbracoApplication, ApplicationContext applicationContext) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\ApplicationEventHandler.cs:35
   Umbraco.Core.CoreBootManager.<Complete>b__38_0(IApplicationEventHandler x) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\CoreBootManager.cs:358
   Umbraco.Core.EnumerableExtensions.ForEach(IEnumerable`1 items, Action`1 action) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\EnumerableExtensions.cs:92
   Umbraco.Core.CoreBootManager.Complete(Action`1 afterComplete) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\CoreBootManager.cs:348
   Umbraco.Web.WebBootManager.Complete(Action`1 afterComplete) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Web\WebBootManager.cs:202
   Umbraco.Core.UmbracoApplicationBase.StartApplication(Object sender, EventArgs e) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\UmbracoApplicationBase.cs:52
   Umbraco.Core.UmbracoApplicationBase.Application_Start(Object sender, EventArgs e) in X:\Projects\Umbraco\Umbraco_7.4\src\Umbraco.Core\UmbracoApplicationBase.cs:69

[HttpException (0x80004005): Type name "Test" is used for Content:"test", Member:"test". Should be used for one type only.]
   System.Web.HttpApplicationFactory.EnsureAppStartCalledForIntegratedMode(HttpContext context, HttpApplication app) +544
   System.Web.HttpApplication.RegisterEventSubscriptionsWithIIS(IntPtr appContext, HttpContext context, MethodInfo[] handlers) +186
   System.Web.HttpApplication.InitSpecial(HttpApplicationState state, MethodInfo[] handlers, IntPtr appContext, HttpContext context) +172
   System.Web.HttpApplicationFactory.GetSpecialApplicationInstance(IntPtr appContext, HttpContext context) +402
   System.Web.Hosting.PipelineRuntime.InitializeApplication(IntPtr appContext) +343

[HttpException (0x80004005): Type name "Test" is used for Content:"test", Member:"test". Should be used for one type only.]
   System.Web.HttpRuntime.FirstRequestInit(HttpContext context) +579
   System.Web.HttpRuntime.EnsureFirstRequestInit(HttpContext context) +112
   System.Web.HttpRuntime.ProcessRequestNotificationPrivate(IIS7WorkerRequest wr, HttpContext context) +716

One thing I noticed about the stack trace is that the EnsureModels is called eagerly it would seem when they might not actually be totally required at that moment. It would be more ideal to only ensure the models (do the processing) when absolutely required.

Comments

Stephan 14 Jan 2016, 09:58:51

In the stack trace above EnsureModels is called from within CreateModel(content) = we are creating a model, and therefore we have to ensure that models are indeed there.

In addition, at the moment the models builder does not support duplicates in model names, has never. Looooong time ago in Umbraco you could not create a media type & a content type with the same name.

Because, really, we would have class Models.Test for your content type, and Models.Test for your member type = that cannot work. We need aliases to be unique. At the moment we want aliases to be unique, and class names to be unique.

Though of course we could eventually support your scenario...


Shannon Deminick 14 Jan 2016, 10:01:02

I have scheduled looking at this next sprint - we'll need to fix something somewhere regarding this.


Stephan 14 Jan 2016, 10:05:24

Note that the models factory is part of Core already (and has been for a long time) and uses the alias as a key to identify models, so that factory would need to be refactored too. Extending the factory + the models builder to support aliases shared by content/media/member would not be that difficult.

OTOH we'd need to figure out what to generate, because obviously we cannot generate two classes Models.Test, one for the content type and one for the member type. So either the user has to use one of the (already existing) attributes to indicate that eg the member type should use Models.TestMember class name (already supported today, requires an attribute) OR we'd need... don't know... separate namespaces for media/member/content models? Not sure I'd like it.

OK - next sprint then.


Stephan 20 Jan 2016, 14:22:04

Notes: *we've basically decided that aliases should be unique accross content, media and member types *in various places I am pretty sure that Core assumes they are unique *ModelsBuilder uses aliases as keys in many places & assumes they are unique

Supporting duplicate aliases would mean a complete Core code review + a large refactoring of ModelsBuilder. It seems pointless to do it only for backward compatibility with people who would have duplicates today (and are ''not'' using ModelsBuilder anyways).

Decision = improve the exception message to be more explicit and point to this issue. Should anyone be hit by the exception after an upgrade, the instructions are: *edit web.config appSettings to set Umbraco.ModelsBuilder.Enable to false *if you want to re-enable the ModelsBuilder, sort-out the aliases to remove the duplicates


Stephan 20 Jan 2016, 14:22:10

PR : https://github.com/zpqrtbnk/Zbu.ModelsBuilder/pull/80


Priority: Task - Pri 2

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.4.0

Due in version: 7.4.0

Sprint: Sprint 7

Story Points:

Cycle: