U4-7063 - Umbraco.TypedContent(IEnumerable) returns NULLs

Created by Dan Booth 03 Sep 2015, 13:08:38 Updated by Dallas Taylor 21 Oct 2015, 13:38:07

There is an overload for the Umbraco Helper method Umbraco.TypeContent() that you can pass in an array of Ids and it returns an IEnumerable<IPublishedContent>. However, if any of those Ids doesn't lookup to a published node then you get NULLs in the returned IEnumerable (using 7.2.8). This seems pretty bad practice, since it will break all LINQ queries or iterations you do using the results.

Example

Assume that pages 1153 and 1659 are the Ids of published pages but 12 doesn't exist. The following code will throw a NullReferenceException as soon as you try to iterate the results. ` var ids = new int[] { 1153, 1659, 12 };

var nodes = Umbraco.TypedContent(ids);

var pages = nodes.Where(x => x.DocumentTypeAlias == "TextPage");

foreach (var node in pages)
{
    <p>
        Name: @node.Name
    </p>
}

` I'd expect nulls to be removed.

Comments

Sebastiaan Janssen 03 Sep 2015, 13:13:48

I would argue that now you know that they don't exist. If you were just note getting them returned you'd be wondering why you only got 2 items when you were expecting 3!

This should work, no? nodes.Where(x => x != null && x.DocumentTypeAlias == "TextPage");


Dan Booth 03 Sep 2015, 14:36:27

@sebastiaan Yeah, I can see an argument for both ways. However, based on how most APIs work, I still wouldn't have to expect to have to perform null checks in queries. I can't think of any other API I've used that would return a list that contains null items - it breaks the fluent nature of LINQ etc.

Also, it's the kind of thing that can easily bite you if you aren't aware that the list can potentially include nulls - especially if it rarely happens. It might be once in a blue-moon that you find an id that doesn't exist (it's literally just been unpublished) and then your query blows up and throws an exception.

However, if it's intended behaviour I'll work around it :)


Sebastiaan Janssen 03 Sep 2015, 15:06:33

For now, you'll have to work around it as this is the way it works! ;-)


Stephan 04 Sep 2015, 07:42:44

Er... @sebastiaan would it be considered a breaking change for 7.3? Because I must say... it makes much more sense to ''not'' return nulls and I'd consider this a bug. And fixing is super cheap.


Sebastiaan Janssen 04 Sep 2015, 08:08:17

Go for it! Doesn't break anything as people who already have null checks shouldn't see anything different.


Sebastiaan Janssen 04 Sep 2015, 08:08:57

Make sure to update the comment on that method then to say: will return all nodes, if they exist.


Stephan 04 Sep 2015, 08:13:12

Comment, what comment? That method has no comment.


Sebastiaan Janssen 04 Sep 2015, 08:20:39

Time to create one then ;-)


Stephan 04 Sep 2015, 08:44:56

Pushed a542cf56d46d3f77177564426a2e6b47fd43aea5 - fixed


Priority: Normal

Type: Bug

State: Fixed

Assignee: Stephan

Difficulty:

Category: Architecture

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.1.4, 7.2.8

Due in version: 7.3.0

Sprint:

Story Points:

Cycle: