Adding a task to a contest can spawn an IntegrityError

[wil93]

~~This happens very rarely.~~ Sometimes, when you delete ~~(or better "unassign")~~ a task from an existing contest, the Task.num fields will be left corrupted. For example, if (after the deletion) you have 3 tasks, you could find that the Task.num values are now 0, 1, and 3, instead of 0, 1, and 2. When you'll try to add a fourth task, AWS will create it with Task.num = len(Contest.tasks) = 3, and that will spawn this error:

(psycopg2.IntegrityError) duplicate key value violates unique constraint "tasks_contest_id_num_key" DETAIL: Key (contest_id, num)=(1, 3) already exists. [SQL: 'UPDATE tasks SET num=%(num)s, contest_id=%(contest_id)s WHERE tasks.id = %(tasks_id)s'] [parameters: {'contest_id': 1, 'num': 3, 'tasks_id': 10}]

A possible solution is to remove the offending task (the one with Task.num = 3) and readding it (the task will now correctly have Task.num = 2). Once you do this, the IntegrityError won't show up if you try to add a fourth task.

~~I think this is probably related to this not being executed, for some odd reason.~~

~~Any idea is appreciated (maybe creating a transaction, doing all the updates, and then committing the transaction? I'm not sure if SQLAlchemy already does it, though).~~

[wil93]

I just realized that this was just a communication problem between me and the person who found the bug :smile:

Actually, to reproduce this, you must actually delete the task (not just unassign it from AWS). So... steps to reproduce:

1. Add some tasks with cmsAddTask <taskname>
2. Assign two tasks to a new contest (using AWS) in order: first task A, then task B.
3. Delete the task A (the first task you added) by calling cmsRemoveTask <taskname>
4. Try to assign another task to the contest, you will get an IntegrityError.

[lw]

OK, so I'd say that there are two "culprits": the code that removes the task (that leaves the nums in a non-contiguous state) and the one that adds it (which doesn't check if the number it's about to assign is already taken). Therefore there are two ways to fix it.

I remember having already encountered this problem quite a while ago. And, as the link you posted [1] shows, AWS already has some code to address the first of the issues above. The simplest fix thus would be to add some code like that also to cmsRemoveTask.

As a plus, it would be nice to change the code that adds tasks as well, to have it take Task.num = max(t.num for t in Contest.tasks) + 1. This change would need to be made both in AWS and in cmsAddTask.

Would you mind writing such a PR?

[1] https://github.com/cms-dev/cms/blob/608a8e4ff4be0a73601e441e761ae13e7639376e/cms/server/admin/handlers/contesttask.py#L85

[wil93]

The simplest fix thus would be to add some code like that also to cmsRemoveTask.

Maybe it's better to put that code in some single place that cmsRemoveTask can call as well (instead of duplicating it). Or we could just disallow removing a task if it's assigned to some contest (for now). Or we could just do the max thing you suggested (also: removing the "decrease subsequent task numbers by 1" part from AWS), I think it would be a more than acceptable solution.

This change would need to be made both in AWS and in cmsAddTask.

Actually cmsAddTask does not tie the task to a contest, so the max part would only end up in AWS.