Modify

Opened 3 years ago

Closed 20 months ago

#8685 closed defect (fixed)

[patch] User deletion ordering breaks 'deleted' notification for SessionStore

Reported by: nathaniel@… Owned by: hasienda
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)

acct_mgr.patch (1.2 KB) - added by nathaniel@… 3 years ago.
patch to fix the problem

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by nathaniel@…

patch to fix the problem

comment:1 Changed 2 years ago by rjollos

  • Component changed from SELECT A HACK to AccountManagerPlugin
  • Owner set to hasienda

Yet another incorrectly filed ticket.

comment:2 Changed 2 years ago by hasienda

  • 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 2 years ago by hasienda

  • Status changed from new to assigned
  • Summary changed from [patch] User deletion ordering breaks deleted notification to [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 2 years ago by hasienda

(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 2 years ago by hasienda

(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.:

http://effbot.org/zone/python-getattr.htm

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 20 months ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to 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.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.