Skip to content

Commit

Permalink
Fix integrity error on SQL Server
Browse files Browse the repository at this point in the history
Currently we use the unique_together constraint to ensure that if a label has a
shortcut key combination, no other label has the same combination.

However, on SQL Server, the unique_together constraint also looks at null
values which breaks the unique_together constraint since we now can no longer
have two labels without shortcut keys. The error message is as follows:

```
Violation of UNIQUE KEY constraint 'api_label_project_id_prefix_key_suffix_key_1b3d8f77_uniq'.
Cannot insert duplicate key in object 'dbo.api_label'.
The duplicate key value is (32, <NULL>, <NULL>).
```

Moving the validation logic from the database into Django fixes the issue and
makes the constraint clearer.
  • Loading branch information
c-w committed Jul 8, 2019
1 parent 95a32ca commit 55b686f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 10 deletions.
4 changes: 4 additions & 0 deletions app/api/migrations/0003_support_sql_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,8 @@ class Migration(migrations.Migration):
name='seq2seqannotation',
unique_together={('document', 'user', 'text')},
),
migrations.AlterUniqueTogether(
name='label',
unique_together={('project', 'text')},
),
]
15 changes: 7 additions & 8 deletions app/api/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,19 +163,18 @@ def clean(self):
# Don't allow shortcut key not to have a suffix key.
if self.prefix_key and not self.suffix_key:
raise ValidationError('Shortcut key may not have a suffix key.')
super().clean()

def validate_unique(self, exclude=None):
# Don't allow to save same shortcut key when prefix_key is null.
if Label.objects.exclude(id=self.id).filter(suffix_key=self.suffix_key,
prefix_key__isnull=True).exists():
raise ValidationError('Duplicate key.')
super().validate_unique(exclude)
# each shortcut (prefix key + suffix key) can only be assigned to one label
if self.suffix_key or self.prefix_key:
other_labels = self.project.labels.exclude(id=self.id)
if other_labels.filter(suffix_key=self.suffix_key, prefix_key=self.prefix_key).exists():
raise ValidationError('A label with this shortcut already exists in the project')

super().clean()

class Meta:
unique_together = (
('project', 'text'),
('project', 'prefix_key', 'suffix_key')
)


Expand Down
4 changes: 2 additions & 2 deletions app/api/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ def test_text_uniqueness(self):

def test_keys_uniqueness(self):
label = mommy.make('Label', prefix_key='ctrl', suffix_key='a')
with self.assertRaises(IntegrityError):
with self.assertRaises(ValidationError):
Label(project=label.project,
text='example',
prefix_key=label.prefix_key,
suffix_key=label.suffix_key).save()
suffix_key=label.suffix_key).full_clean()

def test_suffix_key_uniqueness(self):
label = mommy.make('Label', prefix_key=None, suffix_key='a')
Expand Down

0 comments on commit 55b686f

Please sign in to comment.