More conservative write

[koppor]

Refs https://github.com/JabRef/jabref/issues/8746

This introduces a feature of our AtomicFileWriter to not "commit" changes if an error happened during write.

I currently don't know how to test. Maybe simulating a small virtual hard disk not having enough space for writing?

---

should fix:

• [ ] https://github.com/JabRef/jabref/issues/7718
• [x] #6102
• [x] #8484
• [ ] https://github.com/JabRef/jabref/issues/8746

---

open things:

• [x] https://github.com/koppor/jabref/issues/391

---

does not fix:

• [ ] #4877
• [ ] #9064
• [ ] #6694

---

• [ ] Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
• [ ] Tests created for changes (if applicable)
• [ ] Manually tested changed features in running JabRef (always required)
• [ ] Screenshots added in PR description (for UI changes)
• [ ] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
• [ ] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

[Siedlerchr]

Still does not fix one of the root issues that there is no Filelock and that this method can be executed in parallel

[koppor]

(outdated)

Sure, this DOES NOT fix following issues: #6694, #8484, #6102, #4877, https://github.com/JabRef/jabref/issues/8746. All of them are (currently) part of our next milestone v5.7: https://github.com/JabRef/jabref/milestone/26

This thing was one quick thought to implement following contract:

https://github.com/JabRef/jabref/blob/dd173e0f6bdc2428036cd2e5b33f69e274541405/src/main/java/org/jabref/logic/exporter/AtomicFileWriter.java#L14

[koppor]

Refs https://github.com/JabRef/jabref/issues/6688, because users don't want that directory to be cluttered.

[Siedlerchr]

Another more general thing I just came across as I looked into this saving issues again:

1. We call the AtomicOutputStream with a NIO-File-Channel based Stream (ChannelOutputstream) which wil fail the instanceof check and never create a File Lock => We would need to call the constructor with
   super(new FileOutputStream(getPathOfTemporaryFile(path).toAbsolutePath().toString()));
2. Second problem: Each saving creates a new SaveDatabaseAction object which itself creates a new Writer and Outputstream.
3. The fileLock variable is an object var. To have any effect it would ne need to be static
4. Saving and writing happens on the javafx thread...

[tobiasdiez]

I did a bit of research how other (code) editors are handling this and was a bit surprised that vscode for example uses a simple write (not an atomic write). However, there are quite a few issues in the vscode repo about lost or corrupt files, see https://github.com/microsoft/vscode/issues/98063 and linked issues. Other data points: visual studio uses atomic writer, sublime text has an option to toggle between them.

While going through vscode's code, I found a few really nice implementations of things that JabRef is struggling with. They might be handy to be used as reference. In general, the vscode is very readable and well-structured; so worth looking at.

• https://github.com/microsoft/vscode/blob/78397428676e15782e253261358b0398c2a1149e/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopy.ts#L812 - the actual safe operation of a file, and how to navigate all the different user scenarios that might lead to issues (auto save just triggered, multiple save operations triggered by user, save operation triggered by user although there are no changes etc). One important point is that the have a "version" attached to each state of the document, and use this to decide the correct order of save operations (or which to cancel). If I'm not mistaken, it is even written to the file as a metadata, and then used to compare the in memory representation to possible changes in the file.
• https://github.com/microsoft/vscode/blob/78397428676e15782e253261358b0398c2a1149e/src/vs/workbench/services/workingCopy/common/storedFileWorkingCopySaveParticipant.ts and https://github.com/microsoft/vscode/blob/4a130c40ed876644ed8af2943809d08221375408/src/vs/workbench/api/browser/mainThreadSaveParticipant.ts - handling of operations that run on save (e.g. our cleanup ops)
• https://github.com/microsoft/vscode/blob/main/src/vs/workbench/browser/parts/editor/editorAutoSave.ts - auto save (nicely separated from everything else, using listeners to be notified about relevant changes in focus etc)
• https://hackage.haskell.org/package/atomic-write - applies the permission update to the temporary file (that is then moved into place), if I remember correctly, we are updating the file after the move of the temporary file, which might lead to issues with file watchers like dropbox

[koppor]

We fixed the AtomicFileOutputStream in summer. The discussions at https://github.com/JabRef/jabref/pull/8750#issuecomment-1214455276 show that we should re-iterate on the idea again. - Especially https://github.com/JabRef/jabref/issues/7718 should be regarded.

We close this PR and need to go back. The ADR https://github.com/JabRef/jabref/pull/8750/files#diff-123ad108d1738a32004ec352c9156e0088de245e0cb840687b658725c40bbf96 can still from a good basis for capturing the implementation decisions.