U4-3591 - Creating PropertyValueConverter for a v7 prop editor (created with manifest) ends up with YSOD when outputting value on frontend

Created by Tim Geyssens 19 Nov 2013, 14:29:08 Updated by Stephan 21 Nov 2013, 07:30:19

Example prop editor and value convertor https://github.com/TimGeyssens/MatrixPropEditor

The error:

More than one converter for property type test.matrix

[InvalidOperationException: More than one converter for property type test.matrix] Umbraco.Core.Models.PublishedContent.PublishedPropertyType.InitializeConverters() in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:95 Umbraco.Core.Models.PublishedContent.PublishedContentType.<.ctor>b__0(PropertyType x) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:31 System.Linq.WhereSelectEnumerableIterator2.MoveNext() +248 System.Linq.Buffer1..ctor(IEnumerable1 source) +520 System.Linq.Enumerable.ToArray(IEnumerable1 source) +103 Umbraco.Core.Models.PublishedContent.PublishedContentType..ctor(IContentTypeComposition contentType) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:30 Umbraco.Core.Models.PublishedContent.PublishedContentType.CreatePublishedContentType(PublishedItemType itemType, String alias) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:152 Umbraco.Core.Cache.<>c__DisplayClass81.<GetCacheItem>b__7() in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Cache\CacheProviderExtensions.cs:56 Umbraco.Core.Cache.<>c__DisplayClass15.<GetCacheItem>b__14(String key) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Cache\StaticCacheProvider.cs:70 System.Collections.Concurrent.ConcurrentDictionary2.GetOrAdd(TKey key, Func2 valueFactory) +83 Umbraco.Core.Cache.StaticCacheProvider.GetCacheItem(String cacheKey, Func1 getCacheItem) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Cache\StaticCacheProvider.cs:70 Umbraco.Core.Cache.CacheProviderExtensions.GetCacheItem(ICacheProvider provider, String cacheKey, Func1 getCacheItem) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Cache\CacheProviderExtensions.cs:56 Umbraco.Core.Models.PublishedContent.PublishedContentType.Get(PublishedItemType itemType, String alias) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:133 Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent.Initialize() +2198 Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.ConvertToDocument(XmlNode xmlNode, Boolean isPreviewing) +178 Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.DetermineIdByRoute(UmbracoContext umbracoContext, Boolean preview, String route, Boolean hideTopLevelNode) +281 Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.GetByRoute(UmbracoContext umbracoContext, Boolean preview, String route, Nullable1 hideTopLevelNode) +228 Umbraco.Web.Routing.ContentFinderByNiceUrl.FindContent(PublishedContentRequest docreq, String route) +236 Umbraco.Web.Routing.ContentFinderByNiceUrl.TryFindContent(PublishedContentRequest docRequest) +228 System.Linq.Enumerable.Any(IEnumerable1 source, Func2 predicate) +149 Umbraco.Web.Routing.PublishedContentRequestEngine.FindPublishedContent() +298 Umbraco.Web.Routing.PublishedContentRequestEngine.FindPublishedContentAndTemplate() +226 Umbraco.Web.Routing.PublishedContentRequestEngine.PrepareRequest() +103 Umbraco.Web.UmbracoModule.ProcessRequest(HttpContextBase httpContext) +509 System.Web.SyncEventExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +80 System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +165

Comments

Stephan 19 Nov 2013, 17:15:33

There can only be one converter per data type. According to the exception message, more than one converter wants to be the converter for property type "test.matrix". You should try to find out why. Usually this is because either you do have two converters for the same property, or because you have a rogue DLL in bin, containing a converter.


Tim Geyssens 19 Nov 2013, 19:41:15

Just tried with a fresh install, just added the prop editor and the convertor dll, getting the same message...


Tim Geyssens 19 Nov 2013, 19:42:04

Now i also get the error in the backoffice content page doesn't load, get a 500 with

{"Message":"An error has occurred.","ExceptionMessage":"More than one converter for property type umbHomePage.matrix","ExceptionType":"System.InvalidOperationException","StackTrace":" at Umbraco.Core.Models.PublishedContent.PublishedPropertyType.InitializeConverters() in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:line 95\r\n at Umbraco.Core.Models.PublishedContent.PublishedPropertyType..ctor(PublishedContentType contentType, PropertyType propertyType) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedPropertyType.cs:line 31\r\n at Umbraco.Core.Models.PublishedContent.PublishedContentType.<.ctor>b__0(PropertyType x) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:line 31\r\n at System.Linq.Enumerable.WhereSelectEnumerableIterator2.MoveNext()\r\n at System.Linq.Buffer1..ctor(IEnumerable1 source)\r\n at System.Linq.Enumerable.ToArray[TSource](IEnumerable1 source)\r\n at Umbraco.Core.Models.PublishedContent.PublishedContentType..ctor(IContentTypeComposition contentType) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:line 29\r\n at Umbraco.Core.Models.PublishedContent.PublishedContentType.CreatePublishedContentType(PublishedItemType itemType, String alias) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:line 152\r\n at Umbraco.Core.Models.PublishedContent.PublishedContentType.<>c__DisplayClassc.b__b() in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Models\PublishedContent\PublishedContentType.cs:line 134\r\n at Umbraco.Core.Cache.CacheProviderExtensions.<>c__DisplayClass81.<GetCacheItem>b__7() in f:\\TeamCity\\buildAgent\\work\\e1a0ddf7a1349eaf\\src\\Umbraco.Core\\Cache\\CacheProviderExtensions.cs:line 56\r\n at Umbraco.Core.Cache.StaticCacheProvider.<>c__DisplayClass15.<GetCacheItem>b__14(String key) in f:\\TeamCity\\buildAgent\\work\\e1a0ddf7a1349eaf\\src\\Umbraco.Core\\Cache\\StaticCacheProvider.cs:line 70\r\n at System.Collections.Concurrent.ConcurrentDictionary2.GetOrAdd(TKey key, Func2 valueFactory)\r\n at Umbraco.Core.Cache.StaticCacheProvider.GetCacheItem(String cacheKey, Func1 getCacheItem) in f:\TeamCity\buildAgent\work\e1a0ddf7a1349eaf\src\Umbraco.Core\Cache\StaticCacheProvider.cs:line 70\r\n at Umbraco.Core.Cache.CacheProviderExtensions.GetCacheItem[T](ICacheProvider provider, String cacheKey, Func1 getCacheItem) in f:\\TeamCity\\buildAgent\\work\\e1a0ddf7a1349eaf\\src\\Umbraco.Core\\Cache\\CacheProviderExtensions.cs:line 56\r\n at Umbraco.Core.Models.PublishedContent.PublishedContentType.Get(PublishedItemType itemType, String alias) in f:\\TeamCity\\buildAgent\\work\\e1a0ddf7a1349eaf\\src\\Umbraco.Core\\Models\\PublishedContent\\PublishedContentType.cs:line 0\r\n at Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent.Initialize()\r\n at Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent..ctor(XmlNode xmlNode, Boolean isPreviewing)\r\n at Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.ConvertToDocument(XmlNode xmlNode, Boolean isPreviewing)\r\n at Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.GetById(UmbracoContext umbracoContext, Boolean preview, Int32 nodeId)\r\n at Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.DetermineRouteById(UmbracoContext umbracoContext, Boolean preview, Int32 contentId)\r\n at Umbraco.Web.PublishedCache.XmlPublishedCache.PublishedContentCache.GetRouteById(UmbracoContext umbracoContext, Boolean preview, Int32 contentId)\r\n at Umbraco.Web.Routing.DefaultUrlProvider.GetUrl(UmbracoContext umbracoContext, Int32 id, Uri current, UrlProviderMode mode)\r\n at Umbraco.Web.Routing.UrlProvider.<>c__DisplayClass3.<GetUrl>b__0(IUrlProvider provider)\r\n at System.Linq.Enumerable.WhereSelectArrayIterator2.MoveNext()\r\n at System.Linq.Enumerable.FirstOrDefault[TSource](IEnumerable1 source, Func2 predicate)\r\n at Umbraco.Web.Routing.UrlProvider.GetUrl(Int32 id, Uri current, UrlProviderMode mode)\r\n at Umbraco.Web.Routing.UrlProvider.GetUrl(Int32 id)\r\n at Umbraco.Web.Routing.UrlProviderExtensions.GetContentUrls(IContent content)\r\n at lambda_method(Closure , IContent )\r\n at AutoMapper.DelegateBasedResolver2.Resolve(ResolutionResult source)\r\n at AutoMapper.PropertyMap.<ResolveValue>b__6(ResolutionResult current, IValueResolver resolver)\r\n at System.Linq.Enumerable.Aggregate[TSource,TAccumulate](IEnumerable1 source, TAccumulate seed, Func`3 func)\r\n at AutoMapper.PropertyMap.ResolveValue(ResolutionContext context)\r\n at AutoMapper.Mappers.TypeMapObjectMapperRegistry.PropertyMapMappingStrategy.MapPropertyValue(ResolutionContext context, IMappingEngineRunner mapper, Object mappedObject, PropertyMap propertyMap)"}


Tim Geyssens 19 Nov 2013, 19:46:15

So I'm pretty sure there are no rogue DLLs in bin, it's using a fresh nightly build


Stephan 19 Nov 2013, 19:46:54

Well if there are two converters for the same data type it will throw in the front-end and the back-end as well. You need to find out all the converters and find those that pretend to convert your property.


Stephan 19 Nov 2013, 19:47:30

"test.matrix" is a custom datatype with a custom alias, right?


Stephan 19 Nov 2013, 19:48:53

Also, you could have PropertyEditorValueConverter and a PropertyValueConverter both pretending to convert the same datatype?


Tim Geyssens 19 Nov 2013, 19:51:01

test.matrix is the doctype alias test + the property alias matrix


Tim Geyssens 19 Nov 2013, 19:51:30

Nope don't have that only have https://github.com/TimGeyssens/MatrixPropEditor/blob/master/SamplePropertyValueConverter/SamplePropertyValueConverter/MatrixValueConverter.cs


Tim Geyssens 19 Nov 2013, 19:52:18

Could there be a core one that is used by default?


Stephan 19 Nov 2013, 19:55:17

Your code look line... No Core converter should trigger... unless your datatype (with alias Nibble.MatrixEditor) is reusing a Core's GUID? If everything fails I'll add more debugging infos to the exception so it tells you exactly what's wrong. Later tonight or more prob. tomorrow morning, have to go for family dinner.


Tim Geyssens 19 Nov 2013, 19:58:38

Yeah only thing I can think of is that the JsonValueConverter is getting in the way, will try to debug tomorrow morning, enjoy the dinner!


Stephan 19 Nov 2013, 21:36:29

Mmm... I don't know how the JSON value converter has been wired, I'd better check this tomorrow. It should not get in the way because no converter should be "by default". Will check.


Shannon Deminick 19 Nov 2013, 21:40:01

@Tim - yes the JSON value converter will get in the way. If you want a custom value converter for your JSON stored value - you must not make it a JSON type, instead leave out the type or make it a String type. Then you can implement your own converter. Since you probably want the functionality that the JSON converter exposes, you might inherit from it.


Shannon Deminick 19 Nov 2013, 21:40:21

the type JSON is a special type - it exists purely so that the JSON value converter kicks in.


Tim Geyssens 20 Nov 2013, 08:10:35

But when I look at the code for the JSON value converter it looks like

var propertyEditor = PropertyEditorResolver.Current.GetByAlias(propertyType.PropertyEditorAlias); if (propertyEditor == null) return false; return propertyEditor.ValueEditor.ValueType.InvariantEquals("json");

So I would expect that it first looks for a converter for the specific prop editor alias, if that doesn't exist, use the JSON one

Would love if that works, would avoid some confusion


Tim Geyssens 20 Nov 2013, 08:12:33

Would like to avoid this:

So person A a frontend dev creates new prop editor, doesn't care about strongly typed Person b a .net dev uses this on his site and creates a convertor, runs into the YSOD so removes the value type Person b upgrades the prop editor and runs into the ysod again...


Stephan 20 Nov 2013, 08:18:24

I'm currently looking at how it's been done. If the JSON converter automatically registers itself for all data types that are configured as JSON... then that's sort-of an issue as there is no "fallback" of converters: we don't take the "first one" that was found, there has to be one and only one.

If two converters exist... say because package XXX was shipped without a converter so you created one, then the next version of package XXX ships with a converter -- you will get an exception. I'm currently looking at the code so that the exception can be much more explicit, telling you which converters are conflicting.

But then, it would be the dev's task to either remove his converter from his code, or to disable one of the two converters (via an ApplicationEventHandler).


Stephan 20 Nov 2013, 08:44:09

@Shan: so if I understand correctly, the JSON converter acts as a "catchall" converter for every data type where ValueType is "json". The list of valid value types seems to be in PropertyValueEditor.GetDatabaseType(). Intuitively, looking at that list, I'd say I'd pick "json" anytime my editor stores JSON, and would indeed be surprised that the built-in converter kicks in and I have to store as "text" to avoid that.

In addition Tim's point is valid. Changing from "json" to "text" a data type that comes from a package just because you want to extend the converter... is bound to cause issues.

Two possibilities:

1/ we decide that a converter cannot be "by default" and has to explicitely convert some well defined data types. So the json converter cannot be "catchall". It means that when one creates a datatype using json, one also has to create a converter, prob. by subclassing the json converter.

2/ we keep it as it is today but then... I think it'd make sense for the JSON converter to give way to any other converter, so that if you create your own converter and we detect that it conflicts with the json converter, we don't throw but ignore the JSON converter?

Thoughts?


Shannon Deminick 20 Nov 2013, 08:50:49

I'll fix it up and figure something out tomorrow, no worries :-)


Tim Geyssens 20 Nov 2013, 08:55:46

Sweet :)


Stephan 20 Nov 2013, 08:56:06

OK no worries ;-) Just write here if you want to discuss. I think a user should be able to leave the value type as json and create his own converter. Now whether we achieve this via (1) or (2)... no idea yet.


Tim Geyssens 20 Nov 2013, 08:56:19

Would vote for option 2


Tim Geyssens 20 Nov 2013, 08:56:51

So don't use the json one if there is a specific one for the prop editor alias


Shannon Deminick 20 Nov 2013, 09:00:37

It'll be a solution where you specify Json type but it can be overridden. Will create a new internal attribute that we check for on converters, so we can have default converters for DB type. Something along those lines.


Stephan 20 Nov 2013, 09:00:57

In the meantime commit a626eef should at least make sure that you have a super-explicit exception message in 7.0 telling you which converter is conflicting with which.


Stephan 20 Nov 2013, 09:01:42

@Shannon - trusting you!


Shannon Deminick 21 Nov 2013, 05:40:21

Fixed in 6fd5eee4f6a573a6ec6c75ffdee4046bfb8c1e14

We have an internal attribute: [DefaultPropertyValueConverter] that can be put on converters that will be treated as a 'default' and excluded from the found converter list if 1 or more others are found.


Stephan 21 Nov 2013, 07:30:19

Nice one, I like it!


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version: 7.0.0

Sprint:

Story Points:

Cycle: