U4-10274 - Umbraco.MemberHasAccess isn't cached

Created by Darren Ferguson 09 Aug 2017, 10:01:00 Updated by Sebastiaan Janssen 04 Sep 2017, 06:08:49

Subtask of: U4-9609

Umbraco.MemberHasAccess no longer appears to be cached. This is leading to performance issues for sites with members logged in as it runs multiple SQL queries.

Also - there is no request scoped caching, so if you use MemberHasAccess in multiple partial views, you get the 8 SQL queries within each partial.

Previously Umbraco used access.config - so member access was cached in memory.

This issue causes severe performance issues on large Umbraco sites as it is querying cmsPropertyData to get member properties - and in some cases querying the pre values for a datatype - if the data type is on the member type.

The desired fix would be to return to caching public access details in memory. If not, then at least an optimised SQL query could be made to retrieve the details required and not all of the member properties. This could also hopefully be request scope cached to avoid duplicate SQL queries.

To repeat the issue Add the following code to a view - Member must be logged in - child nodes must be protected.

@foreach(var child in Model.Content.Children()) {

    <li>@child.Id @child.Name @child.Path @Umbraco.MemberHasAccess(child.Id, child.Path)</li>
}

You can easily analyse the SQL that is being run by using Miniprofiler - add ?umbdebug=true to the query string.

I'm attaching the SQL that gets run on our environment here.

3 Attachments

Download member.sql.txt

Comments

Shannon Deminick 10 Aug 2017, 04:17:16

PR: https://github.com/umbraco/Umbraco-CMS/pull/2119

The problem isn't actually that before the access.config. This only cached the rules of what is public and what is not and that data is definitely cached by Umbraco. The problem is that the HasAccess methods end up calling a few extra things that have already been looked up: The member and the member's roles.

The 'expensive' query to get a member will not change and those same queries have always executed - we've never cached members because there could be millions. These 3 queries to retrieve a member actually perform much better than a very long time ago. So you'll always get the 3 queries for each page a member visits for the member lookup.

The changes I've made are:

  • The PCR looks up a member's roles to check for access - these are now cached in the PCR
  • The MembershipHelper wasn't in charge of IsProtected or MemberHasAccess but it is now and the UmbracoContext calls into this, this also makes this unit testable since they are virtual
  • MemberHasAccess is now request cached but each key will be on the path so this will only help in some cases but it will still help
  • HasAccess was really the underlying culprit since this would re-query both member data and role data. This is fixed now because HasAccess only needs the username and the list of roles, both of which we already have after the routing is done so we now just use that same data
  • GetMemberGroupsForMember(string username) was also executing 2x queries which I've refactored into a single query and tested the performance of it which is faster executing than the 2x - at least with the data I have.

See screenshots (this is based on your example code and in my install there are 2x children of the protected node)

So now there are 4 queries: 1 for the member roles (previously this was 2x and then it executed every time HasAccess was called), 3 are for the member lookup (previously these were executed more than once)


Sebastiaan Janssen 01 Sep 2017, 13:46:44

I've merged this now as this works as described in the comment above, we might need further improvements in the future but this is already much better than what we had!


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version: 7.6.6

Sprint: Sprint 66

Story Points: 2

Cycle: