Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More conservative write #8750

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
242e799
Write to final file only of no exception during write happened
koppor May 2, 2022
125e09e
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor May 12, 2022
69ba879
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor May 12, 2022
048729b
Add log on disk
koppor May 12, 2022
2fb0657
Add writing to a local tmp file (instead of somewhere on a network dr…
koppor May 12, 2022
1671496
Fix logger
koppor May 12, 2022
977e2e2
Change file handling during writing
koppor May 12, 2022
292de7a
Initial AtomicFileWriterTest
koppor May 12, 2022
3e5f117
Add test case
koppor May 12, 2022
902b52e
Revert "Add log on disk"
koppor May 13, 2022
a9172e6
Merge branch 'main' into more-conservative-save
koppor Aug 8, 2022
e948789
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor Aug 10, 2022
67e181c
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor Aug 10, 2022
84d09d2
Fix checkstyle
koppor Aug 10, 2022
1c76e6d
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor Aug 10, 2022
36f4cf1
Remove "keepBackup" from AtomicFileOutputStream
koppor Aug 11, 2022
2d21fe5
Add ADR-0026
koppor Aug 11, 2022
be4d336
Add links to ADR
koppor Aug 11, 2022
75e0aed
Use more clear method
koppor Aug 11, 2022
40157de
Fix typos
koppor Aug 11, 2022
9054614
Add getUniqueFilePrefix(Path)
koppor Aug 11, 2022
7bbf120
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor Aug 11, 2022
45d01ac
Fix checkstyle
koppor Aug 11, 2022
b8fc4f6
Fix org.jabref.logic.autosaveandbackup.BackupManager#performBackup to…
koppor Aug 11, 2022
ba392b3
Fix typos
koppor Aug 11, 2022
4becafb
Move path determination method to FileUtil (will be reused at BackupM…
koppor Aug 11, 2022
a35a6de
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor Aug 13, 2022
ee078fd
Introduce BackupFileType
koppor Aug 13, 2022
bc7853c
Rename method
koppor Aug 13, 2022
d1dc0f4
Add test for getPathOfBackupFileAndCreateDirectory()
koppor Aug 13, 2022
8a08df7
Start to implement multiple backups
koppor Aug 13, 2022
34b9dd9
Merge remote-tracking branch 'origin/main' into more-conservative-save
koppor Aug 16, 2022
60ad4d1
Fix AtomicFileOutputStreamTest
koppor Aug 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Changed

- We changed the writing method of a `.bib` file: Existing files should have been modified instead of being replaced.
- When JabRef writes a `.bib` file, it makes first a backup (`.bak`) of the `.bib`.
- When JabRef writes a `.bib` file, it first attempts to write into a separate local directory.
- We improved the Citavi Importer to also import so called Knowledge-items into the field `comment` of the corresponding entry [#9025](https://github.com/JabRef/jabref/issues/9025)
- We removed wrapping of string constants when writing to a `.bib` file.
- We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action.
Expand Down
6 changes: 6 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,12 @@ dependencies {
testImplementation "org.testfx:testfx-junit5:4.0.16-alpha"
testImplementation "org.hamcrest:hamcrest-library:2.2"

testImplementation ('com.google.jimfs:jimfs:1.2') {
exclude group: "com.google.auto.service"
exclude group: "com.google.code.findbugs"
exclude group: "org.checkerframework"
}

checkstyle 'com.puppycrawl.tools:checkstyle:10.3.2'
// xjc needs the runtime as well for the ant task, otherwise it fails
xjc group: 'org.glassfish.jaxb', name: 'jaxb-xjc', version: '3.0.2'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
parent: Decision Records
nav_order: 26
---
# Safe File Writing by Writing to a Temporary File in Another Folder

## Context and Problem Statement

JabRef needs to write to the .bib file. A .bib file of a user should never be damaged. JabRef needs to provide a way to do "safe" writing of a bib file.

## Considered Options

* Create a temporary file in a local folder and copy after successful write
* Create temporary file next to bib file and atomically move it to the original file

## Decision Outcome

Chosen option: "Create a temporary file in a local folder and copy after successful write", because good usage of Dropbox outweighs potential recovery scenarios

## Pros and Cons of the Options

### Create a temporary file in a local folder and copy after successful write

* Good, because Keeps directory of .bib file clean
* Good, because Keeping file access rights is simple as the file content is replaced, not the file itself
* Bad, because Error recovery is hard

### Create temporary file next to bib file and atomically move it to the original file

This implementation is available at [Marty Lamb's AtomicFileOutputStream](https://github.com/martylamb/atomicfileoutputstream/blob/master/src/main/java/com/martiansoftware/io/AtomicFileOutputStream.java)
and [Apache Zookeepr's AtomicFileOutputStream](https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/common/AtomicFileOutputStream.java).

* Good, because Atomic move is an all-or-nothing move
* Bad, because Makes issues with Dropbox, OneDrive, ...
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ private void openWindow(Stage mainStage) {
if (guiPreferences.isWindowMaximised()) {
mainStage.setMaximized(true);
} else if ((Screen.getScreens().size() == 1) && isWindowPositionOutOfBounds()) {
// corrects the Window, if its outside of the mainscreen
LOGGER.debug("The Jabref Window is outside the Main Monitor\n");
// corrects the Window, if it is outside the mainscreen
LOGGER.debug("The Jabref window is outside the main screen\n");
mainStage.setX(0);
mainStage.setY(0);
mainStage.setWidth(1024);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ private boolean saveDatabase(Path file, boolean selectedOnly, Charset encoding,
GeneralPreferences generalPreferences = this.preferences.getGeneralPreferences();
SavePreferences savePreferences = this.preferences.getSavePreferences()
.withSaveType(saveType);
try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, encoding, savePreferences.shouldMakeBackup())) {
try (AtomicFileWriter fileWriter = new AtomicFileWriter(file, encoding)) {
BibDatabaseContext bibDatabaseContext = libraryTab.getBibDatabaseContext();
BibWriter bibWriter = new BibWriter(fileWriter, bibDatabaseContext.getDatabase().getNewLineSeparator());
BibtexDatabaseWriter databaseWriter = new BibtexDatabaseWriter(bibWriter, generalPreferences, savePreferences, entryTypesManager);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public static boolean backupFileDiffers(Path originalPath) {
}

try {
return Files.mismatch(originalPath, backupPath) != -1L;
return !Files.isSameFile(originalPath, backupPath);
} catch (IOException e) {
LOGGER.debug("Could not compare original file and backup file.", e);
// User has to investigate in this case
Expand Down