Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-19934 improvement on acl cache. #9807

Merged
merged 1 commit into from
Feb 16, 2017

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Feb 9, 2017

This change means that when we are checking the allow function we do not add the harmful
OR to the query.

The 'view my contact' & 'edit my contact' permissions have already been checked for.

Adding them into the ACL query with an OR causes serious performance issues, as
elsewhere commented. We can & should avoid it here


This change means that when we are checking the allow function we do not add the harmful
OR to the query.

The 'view my contact' & 'edit my contact' permissions have already been checked for.

Adding them into the ACL query with an OR causes serious performance issues, as
elsewhere commented. We can & should avoid it here
@eileenmcnaughton
Copy link
Contributor Author

@systopia Bjorn, this is a pretty minor change - I'm hoping you remember the code well enough to review & approve it

@bjendres
Copy link
Contributor

@systopia Bjorn, this is a pretty minor change - I'm hoping you remember the code well enough to review & approve it

No, sorry :-/ I can't even remember what these parameters do. If you want, I could dig into that, but it'll be a while - I'm really swamped atm.

@eileenmcnaughton
Copy link
Contributor Author

The last one has changed - it sets 'do not add the view my contact part on because we've already checked that here & it's really really slow & sucky'

Copy link
Contributor

@seamuslee001 seamuslee001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good improvement to system stability

@eileenmcnaughton
Copy link
Contributor Author

thanks for the review @seamuslee001

@eileenmcnaughton eileenmcnaughton merged commit d1c80ef into civicrm:master Feb 16, 2017
@eileenmcnaughton eileenmcnaughton deleted the perf branch February 16, 2017 04:40
monishdeb pushed a commit to monishdeb/civicrm-core that referenced this pull request May 2, 2017
CRM-19934 improvement on acl cache.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants