Rejecting updates without a version

[andybak]

I haven't looked into the internals to see if this is difficult or even possible - but I'd like to start a discussion on the subject.

I'd like to be able to raise an exception if a versioned instance is modified without supplying a version value. This would close a loophole where there is some method or API call that I've forgotten to update to handle concurrency checks.

Our use case is a large complex codebase where we want to quickly add baseline protection against concurrent updates - a strict 'fail fast' in any situation where there is a potential for conflicts.

[saxix]

should not be difficult to implement, I do not have time to make proper tests, anyway take a look at concurrency.core._select_lock() at line 44. value should be None (or 0 if first save).
It's possible to create a flag to enable this check to do not create backward incompatibility. I like the idea and I see the benefits, anyway do not think I have time now to work on that. If you want to create a proper pull request I'll be happy to merge it.

[claytondaley]

+1

Just ran into this issue. A pre-concurrency UI was sending REST requests without a version; no errors were generated and data was overwritten. It seems like a pretty serious bug if you can defeat the entire system through omission.

This strikes me as so serious that I'd recommend a major version change to make the opposite behavior "default". If you insist on persisting a very dangerous condition in the name of backwards compatibility, maybe a setting to override the default?

[claytondaley]

Turns out I had an issue at two different levels:

1. Django REST Framework (DRF) was not requiring the version (even though it was required in the model) and then was "filling it in" with the value from the database. Obviously, this broke the concurrency check.

I've submitted a ticket at DRF and worked around this issue in my local copy. The logical fix from the perspective of the REST framework was to, at minimum, use the default value (if available) for any value absent in the PUT. The default for these fields is version=0. Unfortunately, that leads to the second issue:

2. django-concurrency sees the default value and skips version checks here

I'm about to submit a PR that:

• Creates a settings flag IGNORE_DEFAULT=True
• Adapts the field logic to check the version unless this flag is enabled

I also had to account for an edge case (documented inline) that can occur during a create. This required a change in the way the logic flowed.

I added a test that failed under the old code. All of the old tests pass except one. As I'll reiterate in the PR, I'm not clear why the test should fail to fix the code. If you can explain, I'll update the PR accordingly.