This patch converts the user module please look over it closely for mistakes or misinterpretations i might have made concerning the point of this antipattern resolution and ill do the rest this week.
This patch:
* replaces a number pf plain statics with the new function based approach for consistency
* Removes the $reset argument from the user load functions and moves the reset_cache operation under the user save operation.
* Removes the $reset argument from user_access which called user_role_permissions to support dynamic roles. This was otherwise not called in core to actually reset but we will need contrib module upgrade notes for modules that want to support dynamic roles.
Questions:
drupal_cache_reset is called twice under user_save() but could probably be called only once above the "if" assuming we are ok invalidating the cache this far into a user save, i didnt want to move it as there might be an consequence i dont forsee.
Comment | File | Size | Author |
---|---|---|---|
#16 | 423806-16.patch | 1.12 KB | JamesAn |
#14 | 423806-14.patch | 14.49 KB | JamesAn |
#12 | 423806-12.patch | 14.57 KB | JamesAn |
#10 | 423806-10.patch | 14.79 KB | JamesAn |
#9 | 423806-9.patch | 14.48 KB | JamesAn |
Comments
Comment #1
michaelfavia CreditAttribution: michaelfavia commentedupdated patch to use -up. anyone know how to make eclipse show function names in patches?
Comment #3
JamesAn CreditAttribution: JamesAn commentedTwo wrapper functions, user_load_reset() and user_access_reset(), were created to reset the static vars, as per #422362-20: convert form.inc to use new static caching API.
Upgrade notes should be written somewhere to track the removal of these $reset params. I don't know where that would go though.. ^^"
Comment #5
JamesAn CreditAttribution: JamesAn commentedComment #7
JamesAn CreditAttribution: JamesAn commentedSilly me. I missed all the user.test changes. ^^"
Comment #8
JamesAn CreditAttribution: JamesAn commentedUgh. Forgot a semi-colon. -_-
Comment #9
JamesAn CreditAttribution: JamesAn commentedLol. Uploaded the wrong patch.. -_-"
Comment #10
JamesAn CreditAttribution: JamesAn commentedJust can't seem to get away from this issue!
I found a typo. Not my fault this time! XP
'User admininstration'
corrected to'User administration'
Comment #12
JamesAn CreditAttribution: JamesAn commentedRerolled against #486274: Typo in user.test, which fixed typos.
Comment #14
JamesAn CreditAttribution: JamesAn commentedRerolled.
Comment #16
JamesAn CreditAttribution: JamesAn commentedRerolled. A lot of the changes are no longer necessary as user_load uses entity_load that doesn't use static vars.
Comment #17
catchWhen a single function has two statics, we should have a single $cache = &drupal_static(array());
then $cache['accounts'] and $cache['categories']
This reduces total calls to drupal_static() during the request, which can be a performance hit if a function using it gets called more than a couple of times (which anything using a static variable should be.
Comment #18
Pasqualle#16: 423806-16.patch queued for re-testing.