U4-1272 - Razor Dynamicnode opens connection to SQL server if Property doesn’t exist

Created by Tomislav Ivankovic 28 Nov 2012, 10:48:22 Updated by Tomislav Ivankovic 14 Dec 2012, 15:38:55

Relates to: U4-814

When you use @Model.SomeProperty and the property doesn’t exist then it makes a query to the database to check the master doc type. This doesn’t happened if you do @{if(Model.HasProperty(“SomeProperty”))} Tested on Umbraco 4.9.0.

Comments

Sebastiaan Janssen 12 Dec 2012, 10:53:09

Shannon, Tomislav has submitted a fix for this, in umbraco.MacroEngines.DynamicNode TryGetMember()

            //check if the alias is that of a child type

            var typeChildren = n.ChildrenAsList;
            if (typeChildren != null && propertyExists)

Notice the added "&& propertyExists".

I can't judge if this is the right thing to do and you did tweet to Darren Ferguson that there are fixes pending: https://twitter.com/Shazwazza/status/272754646823612420

Could you update this issue with your thoughts, is this going to be fixed for 6.0.0? (in which case please update the Due in version field as well).

Thanks!


Tomislav Ivankovic 12 Dec 2012, 11:53:21

I would like to add that SQL is also being hit if @Model.MediaById(nonExistingId) or @Library.MediaById(nonExistingId).


Shannon Deminick 14 Dec 2012, 03:41:58

@Tomislav, I haven't looked yet but I'm pretty sure the reason db calls are made for any media is because currently there is no 'real' cache for media. With v4.10 we now have support to be able to cache the media property like we do with content but it is not there. Media has always been a bit of a pain in the arse because the implementation has not been there to properly cache it. Pretty sure early version of 4.x all calls to get media would hit the database. Over a few minor versions media is now cached in various and not so pleasant ways to not hit the databaes but the correct implementation is not there yet.


Shannon Deminick 14 Dec 2012, 04:10:37

The proposed fix will get around the underlying problem but not fix the underlying problem. I had found this issue when creating DynamicPublishedContent and it is due to a 'hidden' feature of razor macros that has never worked. We actually have a unit test failing for it for DynamicNode but passing for DynamicPublishedContent. The feature is that you can get the children of an item by using its children's doc type alias. For example, if I have this tree

  • Home -- ContentPage -- ContentPage -- News --- NewsArticle --- NewsArticle

and lets say that @Model = the first none of type 'Home'. I could then do this:

foreach(var c in @Model.ContentPages)

and the system will automatically pluralize your children by alias. Another example would be if @Model = the 'News' page. You could do:

foreach(var n in @Model.NewsArticles)

This feature also works if you don't properly case things or forget the plural, so these would also work:

@Model.newsArticles @Model.newsArticle @Model.nEWsaRtiClE ec...

When i discovered this non-working feature I found out about the database calls but didn't want to change anything just in case i didn't fully understand. I did port this feature over to DynamicPublishedContent though :) The database calls that it makes are completely redundant and in fact the script that makes the call would never work (as per my //NOTE: in the DynamicNode codebase prior to this fix).

Anyways, I've removed all reasons for this database call, fixed how the Linq statement is supposed to work and now even that old unit test is passing :)


Tomislav Ivankovic 14 Dec 2012, 15:38:55

@Shannon Thanks


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category: Architecture

Backwards Compatible: False

Fix Submitted:

Affected versions: 4.8.0

Due in version: 6.0.0

Sprint:

Story Points:

Cycle: