I've been upgrading a site from 6.1.6 to 6.2.0 and noticed that the signature for the Where extension on IEnumerable<IPublishedContent> method in Umbraco.Web.PublishedContentExtensions has changed.
In 6.1.6 there were three, including ones that took a delegate:
public static IEnumerable<IPublishedContent> Where(this IEnumerable<IPublishedContent> source, Func<IPublishedContent, bool> predicate);
public static IEnumerable<IPublishedContent> Where(this IEnumerable<IPublishedContent> source, Func<IPublishedContent, int, bool> predicate);
public static IQueryable<IPublishedContent> Where(this IEnumerable<IPublishedContent> list, string predicate);
But in 6.2.0 it is now only:
public static IQueryable<IPublishedContent> Where(this IEnumerable<IPublishedContent> list, string predicate);
If you use Where in a view then it uses the one from System.Linq.Enumerable instead.
Is this expected behaviour? And isn't this a breaking change?
OK... both methods are gone in 6.2 because they do not need to be published content extension methods, as they are already native Linq extensions. So if you use System.Linq, both methods should work. Now... might require to recompile the code. Damn. Can share more details on what happens on your side?
I noticed it as I had an .ashx handler that dealt with IPublishedContent. In it I had some code like this (where "root" is the homepage of a site):
var pages = root.Descendants().Where(x => x.DocumentTypeAlias.EndsWith("EventsSection"));
I noticed VS was underlining the Where part of the query saying something like, "The best overloaded method match for 'Where' has some invalid arguments". In this code I didn't happen to have a reference to System.Linq, so noticed the issue. I then realised that the overload for Where in PublishedContentExtensions was missing the overloads that take Func<IPublishedContent, bool> as an argument and wasn't sure if this was intentional or an oversight.
Interestingly it never happened in any of the Views or Partials, presumably because the compiler already adds System.Linq namespace, so methods that use "Where" would just fall back to using the standard Linq implementation of Where. And probably most compiled code would also already have a reference to "using System.Linq" as, I think, VS adds this as a default for all new classes. But I guess there could be other cases where code will break if this reference is not there.
Can I ask: Will the performance of Where queries remain the same using the "native" Linq implementation? I always presumed Umbraco had its own extension methods as these were optimised when dealing with IPublishedContent? Is this not the case? Also, I presume the native Linq methods will produce identical results?
About breaking code: if some code has been compiled (in a DLL) agains the old extension method and you try to run it in 6.2 it will fail and report that the method has not been found - switching to the new method is a compile-time thing that cannot be fixed by the runtime. I hope we won't see too many reports else we'd have to re-introduce the methods in 6.
As for performances... in fact they should be improved. The native Linq is as fast as it can be and does not add any Umbraco "sugar" on top of it. Which the Umbraco methods did. Mainly to manage "collections of nodes" in order to get things such as content.IsOdd() or content.IsEven() to work. However they were broken anyway. So now we're on bare Linq, and if you need to convert an IEnumerable<IPublishedContent> into a "collection of nodes" where .IsOdd() and .IsEven() works, you need to convert it to a "content set" using .ToContentSet().
Making sense? Or maybe I need to go into more details?
Thanks, Stephen - that makes sense. I'm reassured the "pure" Linq methods are at least as fast, as I didn't want performance to drop. Personally I don't mind recompiling code, but I guess this might need to be made clear in the release notes?
What is slightly more worrying is that methods such as IsOdd() and IsEven() will stop working. Presumably also IsLast() too (without changing code)? I've got a feeling I may have used these in the past, so that is something to look out for. Again, does sound rather like a breaking change that should be in the release notes? Not a "biggy", though. Thanks :)
Ah... IsOdd(), IsEven(), IsLast() and anything that relies on Index() do not really "stop working". They were working by accident - and soon as you did anything more complex than enumerating a node's children, they were broken. If you use them outside a ContentSet, they should still work, the way they worked, which is more or less broken. If you use them within a ContentSet, they should work.
Details: they all use the "index" of the node within the "collection of nodes" to figure out whether the node is odd, even, first or last. The old extension methods tried to maintain a proper "collection of nodes" anytime you filtered it (using Where() etc). This is one of the reason why we had custom extension methods: to try to wrap everything in our own "collection". Was not so good for perfs + wasn't working on complex cases.
What's done now is, there's no magic to handle a "collection of nodes". The collection a node belongs to is its siblings, ie its parent's children. The "index" always relates to that collection. Unless you convert the enumeration to a content set using .ToContentSet() and then the content set becomes the reference collection.
To make it short: IsFirst() etc will still work outside of content sets, the way they (sort of) worked. But if you want to be sure they work properly, use a content set.
On a different matter, I've found another major breaking change in 6.2.0 which is really weird.
Take the following simple code where you have a parent page and want to use InGroupsOf() to put the children of the parent page into groups of 2:
var groups = Model.Content.Children().InGroupsOf(2);
Now if groups.Count() returned 2 then what would you expect groups.Any() to return? Obviously I would expect it to return true. Prior to 6.2.0 this is what happened. However, now I've found that groups.Any() returns false.
This is counter-intuitive and a pretty major change as I rely on using groups.Any() to determine whether to show a list of grouped children in a lot of code.
To get it to work as expected you need to use:
var groups = Model.Content.Children().InGroupsOf(2).ToList();
Presumably this forces it into memory, but should this be necessary? Regardless, I think it should work as previously (which is more intuitive).
InGroupsOf is not a native Linq extension. It is a method of DynamicPublishedContentList... so I wonder... what exactly is the type of Model.Content in your examples? Is it a dynamic, or an IPublishedContent? What is the type of the Children collection? In fact, unless I miss something, working with pure IPublishedContent then the InGroupsOf extension does just not exist?
Yes, InGroupsOf is an extension method in Umbraco.Core.EnumerableExtensions
The script I have is extremely simple on a fresh Umbraco 6.2.0 install using the standard starter kit:
The entire script is an MVC partial that looks like this:
@inherits Umbraco.Web.Mvc.UmbracoTemplatePage
@{
var groups = Model.Content.Children().InGroupsOf(2);
<h1>Count: @groups.Count() Any?: @groups.Any()</h1>
<p>Model is @Model.GetType()</p>
<p>Model.Content is @Model.Content.GetType()</p>
<p>Model.Content.Children() is @Model.Content.Children().GetType()</p>
<p>groups is @groups.GetType()</p>
}
This outputs:
Count: 2 Any?: False
Model is Umbraco.Web.Models.RenderModel
Model.Content is Umbraco.Web.PublishedCache.XmlPublishedCache.XmlPublishedContent
Model.Content.Children() is System.Linq.OrderedEnumerable`2[Umbraco.Core.Models.IPublishedContent,System.Int32]
groups is Umbraco.Core.EnumerableExtensions+InGroupsEnumerator`1+<Groups>d__17[Umbraco.Core.Models.IPublishedContent]
@Dan - sounds like it. Mr Project Manager himself says a 6.2.1 is a possible thing. Specifically mentionned you and your missing stuff ;-) So make sure to point me to anything that we thought we could break but after all we accept not to break :-)
Umbraco 6.2.0 PublishedContentExtensions
I've been upgrading a site from 6.1.6 to 6.2.0 and noticed that the signature for the Where extension on IEnumerable<IPublishedContent> method in Umbraco.Web.PublishedContentExtensions has changed.
In 6.1.6 there were three, including ones that took a delegate:
But in 6.2.0 it is now only:
If you use Where in a view then it uses the one from System.Linq.Enumerable instead.
Is this expected behaviour? And isn't this a breaking change?
Irks. Should not be the case and would be a breaking change. Having a look.
OK... both methods are gone in 6.2 because they do not need to be published content extension methods, as they are already native Linq extensions. So if you use System.Linq, both methods should work. Now... might require to recompile the code. Damn. Can share more details on what happens on your side?
Thanks, Stephen.
I noticed it as I had an .ashx handler that dealt with IPublishedContent. In it I had some code like this (where "root" is the homepage of a site):
var pages = root.Descendants().Where(x => x.DocumentTypeAlias.EndsWith("EventsSection"));I noticed VS was underlining the Where part of the query saying something like, "The best overloaded method match for 'Where' has some invalid arguments". In this code I didn't happen to have a reference to System.Linq, so noticed the issue. I then realised that the overload for Where in PublishedContentExtensions was missing the overloads that take Func<IPublishedContent, bool> as an argument and wasn't sure if this was intentional or an oversight.
Interestingly it never happened in any of the Views or Partials, presumably because the compiler already adds System.Linq namespace, so methods that use "Where" would just fall back to using the standard Linq implementation of Where. And probably most compiled code would also already have a reference to "using System.Linq" as, I think, VS adds this as a default for all new classes. But I guess there could be other cases where code will break if this reference is not there.
Can I ask: Will the performance of Where queries remain the same using the "native" Linq implementation? I always presumed Umbraco had its own extension methods as these were optimised when dealing with IPublishedContent? Is this not the case? Also, I presume the native Linq methods will produce identical results?
About breaking code: if some code has been compiled (in a DLL) agains the old extension method and you try to run it in 6.2 it will fail and report that the method has not been found - switching to the new method is a compile-time thing that cannot be fixed by the runtime. I hope we won't see too many reports else we'd have to re-introduce the methods in 6.
As for performances... in fact they should be improved. The native Linq is as fast as it can be and does not add any Umbraco "sugar" on top of it. Which the Umbraco methods did. Mainly to manage "collections of nodes" in order to get things such as content.IsOdd() or content.IsEven() to work. However they were broken anyway. So now we're on bare Linq, and if you need to convert an IEnumerable<IPublishedContent> into a "collection of nodes" where .IsOdd() and .IsEven() works, you need to convert it to a "content set" using .ToContentSet().
Making sense? Or maybe I need to go into more details?
Thanks, Stephen - that makes sense. I'm reassured the "pure" Linq methods are at least as fast, as I didn't want performance to drop. Personally I don't mind recompiling code, but I guess this might need to be made clear in the release notes?
What is slightly more worrying is that methods such as IsOdd() and IsEven() will stop working. Presumably also IsLast() too (without changing code)? I've got a feeling I may have used these in the past, so that is something to look out for. Again, does sound rather like a breaking change that should be in the release notes? Not a "biggy", though. Thanks :)
Ah... IsOdd(), IsEven(), IsLast() and anything that relies on Index() do not really "stop working". They were working by accident - and soon as you did anything more complex than enumerating a node's children, they were broken. If you use them outside a ContentSet, they should still work, the way they worked, which is more or less broken. If you use them within a ContentSet, they should work.
Details: they all use the "index" of the node within the "collection of nodes" to figure out whether the node is odd, even, first or last. The old extension methods tried to maintain a proper "collection of nodes" anytime you filtered it (using Where() etc). This is one of the reason why we had custom extension methods: to try to wrap everything in our own "collection". Was not so good for perfs + wasn't working on complex cases.
What's done now is, there's no magic to handle a "collection of nodes". The collection a node belongs to is its siblings, ie its parent's children. The "index" always relates to that collection. Unless you convert the enumeration to a content set using .ToContentSet() and then the content set becomes the reference collection.
To make it short: IsFirst() etc will still work outside of content sets, the way they (sort of) worked. But if you want to be sure they work properly, use a content set.
Thanks, Stpehen, makes sense.
On a different matter, I've found another major breaking change in 6.2.0 which is really weird.
Take the following simple code where you have a parent page and want to use InGroupsOf() to put the children of the parent page into groups of 2:
Now if groups.Count() returned 2 then what would you expect groups.Any() to return? Obviously I would expect it to return true. Prior to 6.2.0 this is what happened. However, now I've found that groups.Any() returns false.
This is counter-intuitive and a pretty major change as I rely on using groups.Any() to determine whether to show a list of grouped children in a lot of code.
To get it to work as expected you need to use:
Presumably this forces it into memory, but should this be necessary? Regardless, I think it should work as previously (which is more intuitive).
Mmm.... this is weird. If groups.Count() returns 2 then groups.Any() should be true of course. Looking into it.
InGroupsOf is not a native Linq extension. It is a method of DynamicPublishedContentList... so I wonder... what exactly is the type of Model.Content in your examples? Is it a dynamic, or an IPublishedContent? What is the type of the Children collection? In fact, unless I miss something, working with pure IPublishedContent then the InGroupsOf extension does just not exist?
Yes, InGroupsOf is an extension method in Umbraco.Core.EnumerableExtensions
The script I have is extremely simple on a fresh Umbraco 6.2.0 install using the standard starter kit:
The entire script is an MVC partial that looks like this:
@inherits Umbraco.Web.Mvc.UmbracoTemplatePage @{ var groups = Model.Content.Children().InGroupsOf(2); <h1>Count: @groups.Count() Any?: @groups.Any()</h1> <p>Model is @Model.GetType()</p> <p>Model.Content is @Model.Content.GetType()</p> <p>Model.Content.Children() is @Model.Content.Children().GetType()</p> <p>groups is @groups.GetType()</p> }Though it would be fairly easy to add the method to IPublishedContent extensions if needed.
Sorry, my bad. Found the extension method. Looking into it now.
Reproduced. Bug. Will log into the bug tracked and fix.
edit: http://issues.umbraco.org/issue/U4-4837
Thanks, voted for!
Does this mean there will be further 6.2.x patch releases? :)
@Dan - sounds like it. Mr Project Manager himself says a 6.2.1 is a possible thing. Specifically mentionned you and your missing stuff ;-) So make sure to point me to anything that we thought we could break but after all we accept not to break :-)
is working on a reply...