Remove delete() call from CompressorFileStorage.get_available_name

[cuu508]

This PR is an alternative to #1105 and #1100, and intends to fix #1099. It changes CompressorFileStorage:

• drops the deletion logic in CompressorFileStorage.get_available_name()
• this means that get_available_name will sometimes return filenames with an underscore and a random string appended at the end to ensure uniqueness
• overrides the save() method to add one additional last step: if the filename contains an underscore then move it into the correct location.

So, instead of deleting a file and then writing new data in the same filename, compressor would write the data in a temporary file, then move it to the correct place. Python docs say os.replace is an atomic operation, so this would avoid the brief moment of file not existing between delete and write.

Notes:

• I didn't add any new test cases. In theory there could be a test case which creates compressed files with clashing filenames, and then tests if the produced files contain any underscores (they shouldn't). But not sure if it's worth testing that.
• GzipCompressorFileStorage and BrotliCompressorFileStorage override the save method again and do some more file IO. I'm not sure if we could run into race conditions there too.

[diox]

I didn't add any new test cases. In theory there could be a test case which creates compressed files with clashing filenames, and then tests if the produced files contain any underscores (they shouldn't). But not sure if it's worth testing that.

I think it's worth testing (I know the current CompressorFileStorage implementation doesn't have any tests - we should change that). Something along the lines of what you suggest should be enough. Let me know if you don't have the motivation to do that (you already done a lot of work, I appreciate the time spent on this!) and I'll step in if needed.

GzipCompressorFileStorage and BrotliCompressorFileStorage override the save method again and do some more file IO. I'm not sure if we could run into race conditions there too.

They do this with the low-level primitives directly though, so they might run into different issues. I think it's fine to ignore them for this.

[cuu508]

No problem – added a small testcase which saves the same file and tests if the filename returned by save() contains any underscores or not.

[diox]

👍 let's merge and get some people experiencing issues with this to test it!