Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix select for update #906

Merged
merged 5 commits into from
Feb 7, 2021
Merged

Fix select for update #906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d08dc04
removed limit on select for update
dag18e Dec 16, 2020
fecee27
added my name to authors
dag18e Dec 16, 2020
c7f1b95
Merge remote-tracking branch 'upstream/master' into fix-select-for-up…
dag18e Jan 12, 2021
ebc297c
Use filter and wrap in a list
dag18e Jan 13, 2021
fde3b61
documentation
dag18e Jan 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,5 @@ Jun Zhou
David Smith
Łukasz Skarżyński
Tom Evans
Dylan Giesler
Spencer Carroll
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
* #884 Added support for Python 3.9

### Fixed
* made token revocation not apply a limit to the `select_for_update` statement #866

## [1.3.3] 2020-10-16

### Added
Expand Down
14 changes: 8 additions & 6 deletions oauth2_provider/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,15 @@ def revoke(self):
access_token_model = get_access_token_model()
refresh_token_model = get_refresh_token_model()
with transaction.atomic():
self = (
refresh_token_model.objects.filter(pk=self.pk, revoked__isnull=True)
.select_for_update()
.first()
)
if not self:
try:
token = refresh_token_model.objects.select_for_update().filter(
pk=self.pk, revoked__isnull=True
)
except refresh_token_model.DoesNotExist:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dag18e @n2ygk I think we added some things here that are not really needed:

  1. AFAIK .filter() can't raise DoesNotExist
  2. L412 casts the Queryset to a list and then slicing, I do no think this is needed and this slows down things a bit

I feel these things should be addressed in a follow up PR

return
if not token:
return
self = list(token)[0]

try:
access_token_model.objects.get(id=self.access_token_id).revoke()
Expand Down