U4-949 - Calling library.HasAccess is slow

Created by Markus Johansson 04 Oct 2012, 06:10:15 Updated by Shannon Deminick 26 Jun 2017, 05:40:42

Relates to: U4-5376

In Umbraco v4.8, I'm running this piece of XSLT:

$currentPage/ancestor-or-self::[@isDoc][@level = 1]//[@isDoc][umbraco.library:HasAccess(@id, @path) = true()]

The library "HasAccess"-method will be called for each node in the scope. The current implementation of the HasAccess-method looks like this:

..yada..

if (Member.IsLoggedOn()) return Access.HasAccess(NodeId, Path, Membership.GetUser());

..yada..

This means that the call will fetch the get user from the Membership-provider once for each node in the tree. I've solved this by adding caching to the membership provider, but would'nt it be better to store the current user in a private field and reuse in some way?

Comments

Shannon Deminick 20 Aug 2014, 17:36:45

This is one of the problems with these legacy APIs and the use of all of these static classes - you cannot persist any state in a private field in a static class because it is global.

That said, we could add some logic to these legacy classes to store the result in HttpContext.Items for the current request.


Markus Johansson 21 Aug 2014, 08:12:38

@Shandem True story. Hmm that could be a good solution - does the new api (Umbraco.MemberHasAccess(id, path)) call the membershipprovider in the same way? I'm not sure if this is a problem for other people and my issues was solved with the the cache in the membership-provider - so it's really up to you if you think we need to update this legacy API. Cheers!


Shannon Deminick 21 Aug 2014, 18:50:50

The problem with caching of any sort (apart from request based caching) is that you need to ensure that it get's invalidated when the data is changed, and then you need to make sure that it is also invalidated on all other servers if you have a load balanced environment. Caching always needs to be handled in a very particular way. There's actually some docs written (though I'm not sure why there's no link to it, will fix!): http://our.umbraco.org/documentation/reference/cache/

In 7.1.5, I've added request based caching on the UmbracoHelper's GetCurrentMember() which is used by all calls for HasAccess (including the usage of library:HasAccess) - therefore if you call HasAccess multiple times in a single request, there will only be one lookup for the current member. Since it's request based caching we don't have to worry about cache invalidation.

I've also added this task: http://issues.umbraco.org/issue/U4-5377

Which is the correct place to apply a global cache for members instead of at the membership provider level.


Shannon Deminick 26 Jun 2017, 05:40:43

Closing issue due to inactivity - see blog post for details https://umbraco.com/blog/issue-tracker-cleanup/


Priority: Normal

Type: Bug

State: Closed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version:

Sprint:

Story Points:

Cycle: