Opened 14 years ago
Closed 12 years ago
#8685 closed defect (fixed)
[patch] User deletion ordering breaks 'deleted' notification for SessionStore
Reported by: | Owned by: | Steffen Hoffmann | |
---|---|---|---|
Priority: | normal | Component: | AccountManagerPlugin |
Severity: | normal | Keywords: | user delete notification |
Cc: | Trac Release: | 0.12 |
Description
If you use the DB storage backend for users, the deleted event never fires. This is due to the fact that the ordering is wrong. The attached patch fixes this issue.
Attachments (1)
Change History (7)
Changed 14 years ago by
Attachment: | acct_mgr.patch added |
---|
comment:1 Changed 13 years ago by
Component: | SELECT A HACK → AccountManagerPlugin |
---|---|
Owner: | changed from anonymous to Steffen Hoffmann |
Yet another incorrectly filed ticket.
comment:2 Changed 13 years ago by
Keywords: | user delete notification added |
---|
Thank you for informing us about this issue. Notification via TracAnnouncer is currently broken, so I could hardly notice this myself.
From first glance I heartily recommend to only do what you declare. Removing comments shouldn’t be your business, at least unless you've proven before, that they are wrong.
I'll have a closer look at this later, within the next few days.
comment:3 Changed 13 years ago by
Status: | new → assigned |
---|---|
Summary: | [patch] User deletion ordering breaks deleted notification → [patch] User deletion ordering breaks 'deleted' notification for SessionStore |
Ok, the comments are gone in the current code anyway. Adapted the patch, and it looks good.
comment:4 Changed 13 years ago by
(In [11250]) AccountManagerPlugin: Change sequence of actions when deleting a user, refs #8685.
Nathaniel McCallum noticed, that with SessionStore
db storage backend
the 'user deleted' event never fires due to wrong ordering of actions.
The call to store.delete_user
is more elegant - pythonic - now, but a bit
more subtle to me too.
Thanks for spotting this and even more for providing a fix as well.
comment:5 Changed 13 years ago by
(In [11251]) AccountManagerPlugin: Split retrieval and call to delete_user
method, refs #8685.
I felt concerned enough about the change to the delete method call in [11250] to research it a bit more and found i.e.:
Afterwards I decided to use an alternative, more inflated pattern, that calls
the method retrieved by getattr
only if it exists and appears to be callable.
Rationale: Dealing with multiple, even concurrent AuthStores tends to become
complicated, so avoiding confusion about AttributeError exceptions raised by
getattr
with similar exceptions raised inside the called method itself seems
like a reasonable precaution.
comment:6 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
(In [12398]) AccountManagerPlugin: Releasing version 0.4, pushing development to acct_mgr-0.5dev.
Availability of that code as stable release closes #874, #3459, #4677, #5295, #5691, #6616, #7577, #8076, #8685, #8770, #8791, #8990, #9052, #9079, #9090, #9139, #9246, #9252, #9547, #9618, #9676, #9843, #9852, #9940, #10023, #10028, #10123, #10142, #10204, #10276, #10397, #10412, #10594, #10625 and #10644.
Some more issues have been worked-on, yet without confirmed resolution,
refs #5464 (for JiraToTracIntegration
), #8927 and #10134.
And finally there are some issues and enhancement requests showing progress, but known to require more work to resolve them satisfactorily, refs #843, #1600, #5964, #8217, #8933.
Thanks to all contributors and followers, that enabled and encouraged a good portion of this development work.
patch to fix the problem