User tests: Successful: Unsuccessful:
This adds caching to the authorise method of JUser. Instead of doing the whole check each time JUser::authorise() is called, the checks are done once per session and that is it. This is quite a performance improvement.
To prevent the session from being overrun by the access cache, the number of access results to cache is limited to 250 entries. That should be equivalent to around 10-14kb of data.
I did not make scientific tests with this, just some manual comparisons and I did get positive results in terms of performance gain.
There is a potential issue when an extension creates its own login/logout code that does not use JApplicationCMS::login/logout and which does not correctly reset the JUser object. In that case however, that extension has quite large security issues anyway.
Anyway, I made the caching variable private, so that we can remove it again in case it turns out to be a failure. There is no backwards compatibility issue there.
Labels |
Added:
?
|
Easy | No | ⇒ | Yes |
Easy | Yes | ⇒ | No |
Category | ⇒ | Libraries |
@Hackwar can we have test instructions and can you answer Roland his question.
We have a PBF now and want to be able to test this.
With the change in how the JUser
object is serialized to the session, is this even still valid? At best it would cache the lookups for a single request cycle based on current staging.
can we have test instructions
They are not interested in performance improvements, just 'code poetry'. ;)
Sorry we don't intend to rewrite Joomla as a non-PHP application to follow your scoping and performance preferences
This PR was meant for when the serialized user object was stored in the session. Since that has changed, I have no idea about the implications that this has on the usefullness of this PR. A cache for the access data still seems to be usefull, but as @mbabker wrote, in the current situation its value is greatly diminished.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-07-31 20:45:05 |
Closed_By | ⇒ | roland-d |
@hackwar Closing this as the usability has been greatly diminished. If you wish to pursue this further feel free to re-open the pull request. Thank you for your contribution.
@Hackwar
What are the test instructions to make sure this is properly tested?
If I am not mistaken this requires a user to logout and login again to get the correct permissions once it has been changed. How does this relate to the PR #8805? Does this have any effect on this PR?