sghpjuikit / player

Audio player and management application.
22 stars 2 forks source link

Implement safe file saving #149

Closed sghpjuikit closed 5 years ago

sghpjuikit commented 5 years ago

Implements file saving using the tmp file strategy & uses it across the project where necessary.

The saving method, as is (IMPL1), remains problematic, due to the fact that data can still be lost, however the advised method (in #132) of saving to tmp first and then renaming (IMPL2) is also problematic and I do not know which is worse.

Currently the issue is that if file saving succeeds, we need to delete the tmp file, but this can fail and if it does the application has to move on, which means next time the same operation is called, we either always fail (which absolutely necessitates user solving this manually, which is bad) or we proceed by first removing the tmp file, which can lead to disastrous loss of data if the previous operation actually failed in the middle of the write (causing tmp now contain broken data) and this operation failed also - leading to no data and no backup. This could seemingly be solved by deleting the save file on error to guarantee it never contains corrupt data, unfortunately this can never be avoided as the delete can fail too.

So we either force user to take care of it in order to prevent app never being able to save again (this is how Intellij Idea does it with commit fails - user must delete .git/.lock file) or we allow the (very rare) possibility of data loss.

Another problem is the cleanup logic in case writing fails - we have no way to provide the error back to the caller as we now have two errors we want to report and this is an odd situation to be in.

The IMPL2 suggested in the previous PR also has both these issues (two consequent failures may lead to complete data loss).

So what now?

The idea of save file, or backup file containing corrupt data does not sit well with me in the first place. Perhaps the writing could save into a .tmp.write file, which would be renamed upon success, this would guarantee no corrupt data. Any ideas?

sghpjuikit commented 5 years ago

I think solving both IMPL1 and IMPL2 with the idea of temporary write file leads to the exact same solution.

sghpjuikit commented 5 years ago

Implemented as described, using two temporary files. Algorithm described in Javadoc. I consider this solved/usable.