Opened 17 years ago
Closed 17 years ago
#7201 closed defect (fixed)
PermissionCache doesn't cache when created with __call__
| Reported by: | Owned by: | Christian Boos | |
|---|---|---|---|
| Priority: | highest | Milestone: | 0.11 |
| Component: | general | Version: | 0.11rc1 |
| Severity: | minor | Keywords: | permission patch |
| Cc: | Branch: | ||
| Release Notes: | |||
| API Changes: | |||
| Internal Changes: | |||
Description
Discovered in my logs after a user report:
2008-05-06 19:37:35,544 Trac[perm] DEBUG: No policy allowed <redacted> performing TICKET_MODIFY on <Resource 'ticket'> (repeat 18 times with different timestamps)
I have an IPermissionPolicy that requests actions on perm('ticket') to find them for perm('ticket', id), and then performs an expensive test when that answers false, hence creating the performance issue reported by my user for viewing a single ticket (besides being called multiple times in the first place).
As I see it, the issue is:
>>> cache = {}
>>> cache is not None and cache or 0
0
(i.e. {} is false) and is fixed in the attached patch.
Attachments (1)
Change History (3)
by , 17 years ago
| Attachment: | perm-share-empty-cache.diff added |
|---|
comment:1 by , 17 years ago
| Keywords: | permission added |
|---|---|
| Milestone: | → 0.11 |
| Owner: | changed from to |
| Priority: | normal → highest |
| Status: | new → assigned |
Oops, right, good catch!
comment:2 by , 17 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Patch applied in r7022, with some unit-tests.
Getting the unit-tests to work implied disabling the cache at the level of the DefaultPermissionPolicy, which makes me think that cache could eventually lead to transient permission issues (like an admin complaining that the removal of a permission for anonymous didn't take effect immediately - ok, it's a matter of waiting 5 seconds, so nothing critical).
patch against r7021 (tried on 0.11rc1)