Merge lp:~danilo/launchpad/devel-bug-720826-delete-race into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Данило Шеган |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12513 |
Proposed branch: | lp:~danilo/launchpad/devel-bug-720826-delete-race |
Merge into: | lp:launchpad |
Diff against target: |
92 lines (+31/-5) 2 files modified
lib/lp/bugs/model/bugsubscriptionfilter.py (+17/-4) lib/lp/bugs/model/tests/test_bugsubscriptionfilter.py (+14/-1) |
To merge this branch: | bzr merge lp:~danilo/launchpad/devel-bug-720826-delete-race |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Aaron Bentley (community) | Approve | ||
Review via email: mp+51890@code.launchpad.net |
Commit message
[incr] [r=abentley][bug=720826] Make BSF.delete() method set all the data back to default values for the final one and add race-condition guard.
Description of the change
= Improvements to BugSubscription
BugSubscription
all the fields to their default values.
There is also a potential for a race-condition which we guard against by
locking all the rows that must not change when we do the check and delete
using SELECT FOR UPDATE. I have not provided a test-case because that's
likely to be pretty complex.
If we wanted to guard on the database level, we could add a trigger
that stops removal using something like
https:/
However, Stuart believes that's too complex for very little benefit.
I agree that what we want to guard against is malicious users, and not
developers who can shoot themselves in the foot (because we've got so
many ways to do it) if they don't use the recommended API for deletion.
So, the trigger is left to the side for now.
== Pre-implementation notes ==
Discussed this with Stuart. He recommends against the trigger and just guarding the Python code (though, he's partial to that as well). To quote his example:
"If you have a form """( ) bugsubscription a (x) bugsubscription b""", it is reasonable to assume someone clicks 'delete', thinks 'oh shit', changes the check box and clicks submit again. So catching this in Python is reasonable."
== Tests ==
bin/test -cvvt BugSubscription
== Demo and Q/A ==
Go to https:/
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/