Skip to content

Commit

Permalink
avoid creating duplicate LTIContext records
Browse files Browse the repository at this point in the history
NumbasLTIResourceMiddleware was doing two separate queries to get an
existing LTIContext object, or create one if it doesn't exist.
This can lead to multiple records being created by separate threads.

This commit changes that code to use the `get_or_create` method in one
atomic transaction, and adds a `unique_together` constraint on
LTIContext so a duplicate can't be created.
  • Loading branch information
christianp committed Oct 13, 2022
1 parent 9539309 commit 01e1762
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 15 deletions.
33 changes: 19 additions & 14 deletions numbas_lti/middleware.py
Expand Up @@ -20,20 +20,25 @@ def __call__(self,request):
label = request.LTI.get('context_label')
label = label if label is not None else name
instance_guid = request.LTI.get('tool_consumer_instance_guid')
try:
context = LTIContext.objects.get(context_id=context_id,instance_guid=instance_guid)
if (name,label) != (context.name,context.label):
context.name = name
context.label = label
context.save()
except LTIContext.DoesNotExist:
context = LTIContext.objects.create(
consumer = LTIConsumer.objects.get(key=request.POST.get('oauth_consumer_key')),
context_id=context_id,
name=name,
label=label,
instance_guid=instance_guid
)

consumer_key = request.LTI.get('oauth_consumer_key')
consumer = LTIConsumer.objects.get(key=consumer_key)

context, _ = LTIContext.objects.get_or_create(
context_id=context_id,
instance_guid=instance_guid,
defaults = {
'consumer': consumer,
'name': name,
'label': label,
}
)
if (name,label) != (context.name,context.label):
context.update(
name = name,
label = label
)

title = request.LTI.get('resource_link_title')
if title is None:
title = ''
Expand Down
17 changes: 17 additions & 0 deletions numbas_lti/migrations/0074_alter_lticontext_unique_together.py
@@ -0,0 +1,17 @@
# Generated by Django 3.2.14 on 2022-10-13 07:19

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('numbas_lti', '0073_resource_require_lockdown_app'),
]

operations = [
migrations.AlterUniqueTogether(
name='lticontext',
unique_together={('context_id', 'instance_guid')},
),
]
1 change: 1 addition & 0 deletions numbas_lti/models.py
Expand Up @@ -227,6 +227,7 @@ class LTIContext(models.Model):
class Meta:
verbose_name = _('LTI context')
verbose_name_plural = _('LTI contexts')
unique_together = (('context_id', 'instance_guid'),)

def __str__(self):
if self.name == self.label:
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Expand Up @@ -2,7 +2,7 @@ Django==3.2.14
oauthlib==3.0.1
lxml==4.9.1
channels==3.0.4
git+https://github.com/christianp/django-auth-lti@v3.1.1#egg=django_auth_lti
git+https://github.com/christianp/django-auth-lti@v3.1.2#egg=django_auth_lti
requests==2.26.0
requests-oauthlib==1.3.0
django-bootstrap-datepicker-plus==3.0.5
Expand Down

0 comments on commit 01e1762

Please sign in to comment.