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

Permission/Group-Caching (possible enhancement) #222

Closed
chland opened this issue Apr 7, 2018 · 4 comments
Closed

Permission/Group-Caching (possible enhancement) #222

chland opened this issue Apr 7, 2018 · 4 comments

Comments

@chland
Copy link
Contributor

chland commented Apr 7, 2018

I'm currently playing around with AAuth and one thing i noticed is that it fires a huge amount of SELECT-queries to read single values.

Lets say you've got code that outputs a fancy pdf and that checks at 20 different positions if the user has a certain permission. Lets say the permission is called "member.vip". In this case the code will query the database 20 times with the same SQL-Query (in the get_perm_id()-function):

SELECT *FROM aauth_permsWHEREname = 'member.vip'

Thats a massive waste of resources.

You could just add the ID to an array once you read it and add a check to the get_perm_id() function which checks this array first. So basically the first time you use is_allowed('member.vip') it would query the DB and add the ID to a simple array. This array would use the name of the permission as the key and the ID as the value (basically: $this->cache_permission_id['member.vip'] = 1234;)

That way you wouldn't query the DB over and over again for the same information.

You could even pre-populate the $this->cache_permission_id with all IDs in the constructor (usually there aren't THAT many permissions in a system). That way, you would query the DB only once.

In my experience you trade some additional memory-usage for a huge performance-gain.

The same is true for the get_group_id()-function.

@REJack
Copy link
Collaborator

REJack commented Apr 9, 2018

That's a great idea, we could use CI's Database Caching Class for this 😄 I'll take look over it this week.

@chland
Copy link
Contributor Author

chland commented Apr 9, 2018

I added a PR with a simple implementation. It uses a very simple internal cache instead of using CI's db-caching (which would be an overkill to use IMHO).

I did a few quick tests and it seems to work. The only thing that might cause issues is the way i "normalize" the permission/group-names before using them as array-keys. IDK how well this works if you use special chars like öäüßèá, etc. in names.

@REJack
Copy link
Collaborator

REJack commented Apr 10, 2018

Your PR is good, I've got no errors and anything works. Only the CamelCase doesn't fit into the code.

Your way to "normalize" is good, I would not do it differently.
Special chars works we also have users using Aauth with russian letters, important is that the encoding is UTF-8.

@REJack
Copy link
Collaborator

REJack commented Nov 11, 2018

Issue closed, changes already in master repo

@REJack REJack closed this as completed Nov 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants