U4-5084 - DynamicPublishedContentList extension methods confusion

Created by tuan 13 Jun 2014, 04:52:30 Updated by Ed Parry 24 Nov 2014, 13:04:20

after upgraded to 6.2 version, my code doesn't work anymore.

The problem is with this where condition:

DynamicPublishedContent node = Umbraco.Content(1212); DynamicPublishedContent nodeT = node.Children.Where(x => x.DocumentTypeAlias == "Indice").FirstOrDefault() as DynamicPublishedContent;

Cannot convert lambda expression to type 'string' because it is not a delegate type

http://our.umbraco.org/forum/developers/razor/50510-Umbraco-62-and-lambda-expression

Comments

Stephan 28 Jun 2014, 10:58:30

Change issue title (was ''Umbraco 6.2 and lambda expression doesn't work anymore #Show-stopper''). See also discussion on Our: http://our.umbraco.org/forum/umbraco-7/using-umbraco-7/53022-UmbracoWebModelsDynamicPublishedContentList-does-not-contain-a-definition-for-Any - this is a general issue with DynamicPublishedContentList extension methods.


Stephan 28 Jun 2014, 10:59:48

Note: since 010893a73a08e8f2f8cb3ef45920e0079f56d05f in March 2013, DynamicPublishedContentList implements ''both'' IEnumerable and IEnumerable which confuses both compile-time and run-time binders.


Stephan 28 Jun 2014, 11:33:57

About the reported issue: nodeT is a DynamicPublishedContent (not a dynamic) and nodeT is a DynamicPublishedContentList (again, not a dynamic). Then, nodeT.Children.Where(...) can either be the builtin DynamicPublishedContentList Where(string) method, or an extension method.

Because DynamicPublishedContentList implements both IEnumerable and IEnumerable the binder knows about two extension methods, Where() and Where(), which both accept a lambda. Because it doesn't know which to pick, it excludes both, falls back to the builtin method, and fails and reports that that method does not accept a lambda but a string.


Stephan 28 Jun 2014, 11:42:33

About the issue discussed in the "''list does not contain Any()''" thread on Our: same cause, binder fails to infer in all extension methods and rule them out.


Stephan 28 Jun 2014, 11:44:44

Removing the IEnumerable on DynamicPublishedContentList solves most of these issues but I'm not sure of the consequences yet. It was added by @Shandem so reassigning the issue. Shan, what do you think? Any particular reason why we have it in the first place?


Shannon Deminick 30 Jun 2014, 02:45:27

It's been this way for quite a long time so I'm having trouble remembering exactly why. It probably perhaps due to us also allowing any extension methods on IEnumerable to be available dynamically on the dynamic objects. But considering DynamicPublishedContent is IPublishedContent I'm not sure it matters. I've tested removing the implementation of IEnumerable and then tested using dynamics and calling into custom extension methods of IEnumerable and our FindAndExecuteExtensionMethod still works - so perhaps it was implemented due to some legacy logic of making this work... not sure but since it implements IEnumrable which implements IPublishedContent and custom extension methods still work on dynamics then I don't see much need to have it also implement IEnumerable.

However...

My first question with the above query is why is dynamic not being used? i.e. why is this using the strongly typed dynamic implementation? There was never an intention to have these classes used directly, these are purely for dynamic purposes and if dynamic is not being used then using IPublishedContent should be used.

The above should be using real strongly typed queries like:

var pNode = Umbraco.TypedContent(1064);
var pNodeT = pNode.Children.Where(x => x.DocumentTypeAlias == "Indice").FirstOrDefault();

or using dynamic queries like:

var dNode = Umbraco.Content(1064);
var dNodeT = dNode.Children.Where("DocumentTypeAlias=\"Indice\"").FirstOrDefault();

There shouldn't really ever be direct casting to DynamicPublishedContent objects. If there's some strange reason this is necessary, like any other ambiguous extension methods, you'd need to call the extension methods directly, so a work around if you require casing to DynamicPublishedContent is:

DynamicPublishedContent node = Umbraco.Content(1064);
DynamicPublishedContent nodeT = Enumerable.Where<IPublishedContent>(
    node.Children, x => x.DocumentTypeAlias == "Indice").FirstOrDefault() as DynamicPublishedContent;

I'll think more about this for 6.2.2, and whether we remove the IEnumerable implementation or not, i don't want to unbreak anything and this is a pretty edge case issue.


Stephan 30 Jun 2014, 07:39:27

You're right, the source of all confusion is people using DynamicPublishedContent in a non-dynamic way (ie explicitely typing variables as DynamicPublishedContent instead of using the dynamic keyword). It ''may'' be that the correct answer is "don't do it". That's why I put ''confusion'' in the issue title.

Will run more tests to check whether it all works "as expected" if everything is true dynamic.


Stephan 13 Aug 2014, 10:21:36

By my tests, it should be fine to remove the IEnumerable implementation and that should remove the most obvious ambiguities with extension methods.


Jeavon Leopold 14 Aug 2014, 08:17:27

@zpqrtbnk did you remove it in v7.1.5? I've been trying to recreate a issue with .Count missing in with v7.1.5 source but can't however it's easy to recreate in v7.1.4...


Stephan 14 Aug 2014, 18:40:07

@crumpled_jeavon not at the time you wrote your question.


Stephan 14 Aug 2014, 18:40:20

Just pushed 648cf69d455106e326ef911855f1fd4c24653371 to 7.1.5 which does remove the IEnumerable (still need to do it for 6.2.x).


Stephan 14 Aug 2014, 18:53:19

Pushed bdc234b18ac71accb1552fe4613f91d516645bc0 to 6.2.2.


Jeavon Leopold 14 Aug 2014, 19:57:47

Interesting, I have theory on why sometimes .Count() .Any() randomly disappear for dynamics users, it's here http://our.umbraco.org/forum/developers/razor/55558-Adding-ServiceStack-reference-breaks-extension-methods?p=0#comment190962


Shannon Deminick 15 Aug 2014, 18:04:15

@zpqrtbnk I'll close this issue as you've fixed it... unless I'm missing something else that is required?


Ed Parry 21 Nov 2014, 11:11:26

We're still experiencing this issue in 7.1.8. Unable to run code past "CurrentPage.Children.Where("Visisble").Any()", and the same for .Count().

Able to work around the issue by changing "CurrentPage" to "Model.Content".


Sebastiaan Janssen 23 Nov 2014, 14:02:40

@edparry Have you installed Analytics for Umbraco? See the thread above, it might break these extension methods: http://our.umbraco.org/forum/developers/razor/55558-Adding-ServiceStack-reference-breaks-extension-methods?p=0#comment190962


Ed Parry 24 Nov 2014, 13:04:20

@sebastiaan We haven't installed that, no. We have managed to fix the issue by copying the bin and umbraco folders from a working machine across to the machine which had the problem, so it's possible that a dll was out of sync with TFS somewhere along the line.


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 6.2.0, 6.2.1, 7.1.4

Due in version: 7.1.5, 6.2.2

Sprint:

Story Points:

Cycle: