Avoid hitting the database when rendering MNTP

[kjac]

EntityService.GetObjectType() is used in the property editor value converter for MNTP. This method explicitly hits the database with zero caching and should thus be avoided in frontend rendering.

I can see you've made a few changes since 2.1.4 (which is used in the screenshots below) that will limit the number of DB hits to one per MNTP on a page. That's definitively better than one hit per selected node :) but still I think it's worth going an extra mile to avoid hitting the DB altogether.

[Jeavon]

Awesome! I only changed to the method looking up each type due to issue #1 as there was no workaround. However I fixed the underlying issue in Umbraco for v7.2, hence why I just reworked it again. I will give your PR a good test with various situations and let you know.

[kjac]

Cool. I tested it with all three node types and it seems to work.
It's just a workaround to avoid the DB lookups. It might look quirky but it
was the best I could think of. If you know of a better one, by all means
implement that instead :-)
On Jan 14, 2015 8:00 PM, "Jeavon" notifications@github.com wrote:

Awesome! I only changed to the method looking up each type due to issue #1
#1
as there was no workaround. However I fixed the underlying issue in Umbraco
for v7.2, hence why I just reworked it again. I will give your PR a good
test with various situations and let you know.

—
Reply to this email directly or view it on GitHub
#8 (comment)
.

[Jeavon]

Thanks for this, it works great. I made a small change to ensure that the return type is System.Linq.Enumerable for typed. The only difference with this version is that they now return "System.Linq.Enumerable+WhereListIterator" as previously they returned "System.Linq.Enumerable+WhereEnumerableIterator" I don't think this is a issue, any thoughts?

[kjac]

Cool And nah, that shouldn't be an issue. I assume people will just use the return value as IEnumerable<IPublishedContent> anyways, so it won't make any difference usage wise.

[tomfulton]

Nice work! 💯

[JimBobSquarePants]

That is some truly clever optimization! 💯

One thing I would note, which should solve your issue @Jeavon, is that you are returning a List<IPublishedContent> instead of the enumerable which is frowned upon as that collection can now be modified.

If you returned it using this little trick.

multiNodeTreePicker.Where(x => x != null).Skip(0);

Then this is better because we are not returning a collection that can be cast back to List<IPublishedContent> and are iterating over the original collection.

p.s I'm totally stealing this. 😄

[kjac]

Steal away 😃

[Jeavon]

@JimBobSquarePants That's an interesting idea, I tried all sorts of things trying to get it to return System.Linq.Enumerable+WhereEnumerableIterator, with the skip it returns

System.Linq.Enumerable+<SkipIterator>d__4d`1[Umbraco.Core.Models.IPublishedContent]

[Jeavon]

@JimBobSquarePants p.s. I do have it returning Enumerable already, it's just the +ListIterator so I don't think the collection is modifiable. See here https://github.com/Jeavon/Umbraco-Core-Property-Value-Converters/blob/v2/Our.Umbraco.PropertyConverters/MultiNodeTreePickerPropertyConverter.cs#L136

[JimBobSquarePants]

@Jeavon Bugger. I didn't realise.

Yeah, looks like that looks like it might work. You can't cast it back certainly.

You could use an extension method if you want the exact same signature. Something like...

    public static IEnumerable<T> Yield<T>(this IEnumerable<T> source)
    {
        foreach (T element in source)
        {
            yield return element;
        }
    }

You should then be able to use the following.

multiNodeTreePicker.Yield().Where(x => x != null)

[Jeavon]

That works, thanks @JimBobSquarePants !

[JimBobSquarePants]

No worries 😄