-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add auto-renaming of linked files on entry data change #13295
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
package org.jabref.gui.externalfiles; | ||
|
||
import java.util.HashSet; | ||
InAnYan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import java.util.Set; | ||
|
||
import org.jabref.gui.preferences.GuiPreferences; | ||
import org.jabref.logic.FilePreferences; | ||
import org.jabref.logic.citationkeypattern.CitationKeyGenerator; | ||
import org.jabref.logic.cleanup.RenamePdfCleanup; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
import org.jabref.model.entry.BibEntry; | ||
import org.jabref.model.entry.event.FieldChangedEvent; | ||
|
||
import com.google.common.eventbus.Subscribe; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import static org.jabref.logic.citationkeypattern.BracketedPattern.expandBrackets; | ||
|
||
public class AutoRenameFileOnEntryChange { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(AutoRenameFileOnEntryChange.class); | ||
|
||
private final GuiPreferences preferences; | ||
private final BibDatabaseContext bibDatabaseContext; | ||
private final RenamePdfCleanup renamePdfCleanup; | ||
|
||
public AutoRenameFileOnEntryChange(BibDatabaseContext bibDatabaseContext, GuiPreferences preferences) { | ||
this.bibDatabaseContext = bibDatabaseContext; | ||
this.preferences = preferences; | ||
bibDatabaseContext.getDatabase().registerListener(this); | ||
renamePdfCleanup = new RenamePdfCleanup(false, () -> bibDatabaseContext, preferences.getFilePreferences()); | ||
} | ||
|
||
@Subscribe | ||
public void listen(FieldChangedEvent event) { | ||
FilePreferences filePreferences = preferences.getFilePreferences(); | ||
|
||
if (!filePreferences.shouldAutoRenameFilesOnChange() | ||
|| filePreferences.getFileNamePattern().isEmpty() | ||
|| filePreferences.getFileNamePattern() == null | ||
InAnYan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|| !relatesToFilePattern(filePreferences.getFileNamePattern(), event)) { | ||
return; | ||
} | ||
|
||
BibEntry entry = event.getBibEntry(); | ||
if (entry.getFiles().isEmpty()) { | ||
return; | ||
} | ||
new CitationKeyGenerator(bibDatabaseContext, preferences.getCitationKeyPatternPreferences()).generateAndSetKey(entry); | ||
renamePdfCleanup.cleanup(entry); | ||
|
||
LOGGER.info("Field changed for entry {}: {}", entry.getCitationKey().orElse("defaultCitationKey"), event.getField().getName()); | ||
} | ||
|
||
private boolean relatesToFilePattern(String fileNamePattern, FieldChangedEvent event) { | ||
Set<String> extractedFields = new HashSet<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using 'new HashSet<>()' is not optimal. Consider using 'Set.of()' for better readability and performance as per modern Java practices. |
||
|
||
expandBrackets(fileNamePattern, bracketContent -> { | ||
extractedFields.add(bracketContent); | ||
return bracketContent; | ||
}); | ||
|
||
return extractedFields.contains("bibtexkey") | ||
|| extractedFields.contains(event.getField().getName()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
package org.jabref.gui.externalfiles; | ||
|
||
import java.io.IOException; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
import org.jabref.gui.preferences.GuiPreferences; | ||
import org.jabref.logic.FilePreferences; | ||
import org.jabref.logic.citationkeypattern.CitationKeyPatternPreferences; | ||
import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns; | ||
import org.jabref.model.database.BibDatabase; | ||
import org.jabref.model.database.BibDatabaseContext; | ||
import org.jabref.model.entry.BibEntry; | ||
import org.jabref.model.entry.LinkedFile; | ||
import org.jabref.model.entry.field.StandardField; | ||
import org.jabref.model.entry.types.StandardEntryType; | ||
import org.jabref.model.metadata.MetaData; | ||
|
||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
import org.junit.jupiter.api.io.TempDir; | ||
|
||
import static org.jabref.logic.citationkeypattern.CitationKeyGenerator.DEFAULT_UNWANTED_CHARACTERS; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
import static org.junit.jupiter.api.Assertions.assertTrue; | ||
import static org.mockito.Mockito.mock; | ||
import static org.mockito.Mockito.when; | ||
|
||
class AutoRenameFileOnEntryChangeTest { | ||
private FilePreferences filePreferences; | ||
private BibEntry entry; | ||
private Path tempDir; | ||
|
||
@BeforeEach | ||
void setUp(@TempDir Path tempDir) { | ||
this.tempDir = tempDir; | ||
MetaData metaData = new MetaData(); | ||
metaData.setLibrarySpecificFileDirectory(tempDir.toString()); | ||
BibDatabaseContext bibDatabaseContext = new BibDatabaseContext(new BibDatabase(), metaData); | ||
GlobalCitationKeyPatterns keyPattern = GlobalCitationKeyPatterns.fromPattern("[auth][year]"); | ||
GuiPreferences guiPreferences = mock(GuiPreferences.class); | ||
filePreferences = mock(FilePreferences.class); | ||
CitationKeyPatternPreferences patternPreferences = new CitationKeyPatternPreferences( | ||
false, | ||
true, | ||
false, | ||
CitationKeyPatternPreferences.KeySuffix.SECOND_WITH_A, | ||
"", | ||
"", | ||
DEFAULT_UNWANTED_CHARACTERS, | ||
keyPattern, | ||
"", | ||
','); | ||
|
||
when(guiPreferences.getCitationKeyPatternPreferences()).thenReturn(patternPreferences); | ||
when(guiPreferences.getFilePreferences()).thenReturn(filePreferences); | ||
when(filePreferences.shouldStoreFilesRelativeToBibFile()).thenReturn(true); | ||
when(filePreferences.getFileNamePattern()).thenReturn("[bibtexkey]"); | ||
|
||
entry = new BibEntry(StandardEntryType.Article).withCitationKey("oldKey2081") | ||
.withField(StandardField.AUTHOR, "oldKey") | ||
.withField(StandardField.YEAR, "2081"); | ||
|
||
bibDatabaseContext.getDatabase().insertEntry(entry); | ||
new AutoRenameFileOnEntryChange(bibDatabaseContext, guiPreferences); | ||
} | ||
|
||
@Test | ||
void noFileRenameByDefault() throws IOException { | ||
Files.createFile(tempDir.resolve("oldKey2081.pdf")); | ||
entry.setFiles(List.of(new LinkedFile("", "oldKey2081.pdf", "PDF"))); | ||
entry.setField(StandardField.AUTHOR, "newKey"); | ||
|
||
assertEquals("oldKey2081.pdf", entry.getFiles().getFirst().getLink()); | ||
assertTrue(Files.exists(tempDir.resolve("oldKey2081.pdf"))); | ||
} | ||
|
||
@Test | ||
void noFileRenameOnEmptyFilePattern() throws IOException { | ||
Files.createFile(tempDir.resolve("oldKey2081.pdf")); | ||
entry.setFiles(List.of(new LinkedFile("", "oldKey2081.pdf", "PDF"))); | ||
when(filePreferences.getFileNamePattern()).thenReturn(""); | ||
when(filePreferences.shouldAutoRenameFilesOnChange()).thenReturn(true); | ||
entry.setField(StandardField.AUTHOR, "newKey"); | ||
|
||
assertEquals("oldKey2081.pdf", entry.getFiles().getFirst().getLink()); | ||
assertTrue(Files.exists(tempDir.resolve("oldKey2081.pdf"))); | ||
} | ||
|
||
@Test | ||
void singleFileRenameOnEntryChange() throws IOException { | ||
Files.createFile(tempDir.resolve("oldKey2081.pdf")); | ||
entry.setFiles(List.of(new LinkedFile("", "oldKey2081.pdf", "PDF"))); | ||
when(filePreferences.shouldAutoRenameFilesOnChange()).thenReturn(true); | ||
|
||
// change author only | ||
entry.setField(StandardField.AUTHOR, "newKey"); | ||
assertEquals("newKey2081.pdf", entry.getFiles().getFirst().getLink()); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2081.pdf"))); | ||
InAnYan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// change year only | ||
entry.setField(StandardField.YEAR, "2082"); | ||
assertEquals("newKey2082.pdf", entry.getFiles().getFirst().getLink()); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2082.pdf"))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assertion checks for a boolean condition instead of asserting the expected content of the object. Use assertEquals to compare expected and actual values. |
||
} | ||
|
||
@Test | ||
void multipleFilesRenameOnEntryChange() throws IOException { | ||
// create multiple entries | ||
List<String> fileNames = Arrays.asList( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Modern Java data structures should be used. Instead of Arrays.asList, consider using List.of for better performance and readability. |
||
"oldKey2081.pdf", | ||
"oldKey2081.jpg", | ||
"oldKey2081.csv", | ||
"oldKey2081.doc", | ||
"oldKey2081.docx" | ||
); | ||
|
||
for (String fileName : fileNames) { | ||
Path filePath = tempDir.resolve(fileName); | ||
Files.createFile(filePath); | ||
} | ||
|
||
LinkedFile pdfLinkedFile = new LinkedFile("", "oldKey2081.pdf", "PDF"); | ||
LinkedFile jpgLinkedFile = new LinkedFile("", "oldKey2081.jpg", "JPG"); | ||
LinkedFile csvLinkedFile = new LinkedFile("", "oldKey2081.csv", "CSV"); | ||
LinkedFile docLinkedFile = new LinkedFile("", "oldKey2081.doc", "DOC"); | ||
LinkedFile docxLinkedFile = new LinkedFile("", "oldKey2081.docx", "DOCX"); | ||
|
||
entry.setFiles(List.of(pdfLinkedFile, jpgLinkedFile, csvLinkedFile, docLinkedFile, docxLinkedFile)); | ||
when(filePreferences.shouldAutoRenameFilesOnChange()).thenReturn(true); | ||
|
||
// Change author only | ||
entry.setField(StandardField.AUTHOR, "newKey"); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2081.pdf"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2081.jpg"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2081.csv"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2081.doc"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2081.docx"))); | ||
|
||
// change year only | ||
entry.setField(StandardField.YEAR, "2082"); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2082.pdf"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2082.jpg"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2082.csv"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2082.doc"))); | ||
assertTrue(Files.exists(tempDir.resolve("newKey2082.docx"))); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ public class FilePreferences { | |
private final StringProperty userAndHost = new SimpleStringProperty(); | ||
private final SimpleStringProperty mainFileDirectory = new SimpleStringProperty(); | ||
private final BooleanProperty storeFilesRelativeToBibFile = new SimpleBooleanProperty(); | ||
private final BooleanProperty autoRenameFilesOnChange = new SimpleBooleanProperty(); | ||
private final StringProperty fileNamePattern = new SimpleStringProperty(); | ||
private final StringProperty fileDirectoryPattern = new SimpleStringProperty(); | ||
private final BooleanProperty downloadLinkedFiles = new SimpleBooleanProperty(); | ||
|
@@ -39,6 +40,7 @@ public class FilePreferences { | |
public FilePreferences(String userAndHost, | ||
String mainFileDirectory, | ||
boolean storeFilesRelativeToBibFile, | ||
boolean autoRenameFilesOnChange, | ||
String fileNamePattern, | ||
String fileDirectoryPattern, | ||
boolean downloadLinkedFiles, | ||
|
@@ -55,6 +57,7 @@ public FilePreferences(String userAndHost, | |
this.userAndHost.setValue(userAndHost); | ||
this.mainFileDirectory.setValue(mainFileDirectory); | ||
this.storeFilesRelativeToBibFile.setValue(storeFilesRelativeToBibFile); | ||
this.autoRenameFilesOnChange.setValue(autoRenameFilesOnChange); | ||
this.fileNamePattern.setValue(fileNamePattern); | ||
this.fileDirectoryPattern.setValue(fileDirectoryPattern); | ||
this.downloadLinkedFiles.setValue(downloadLinkedFiles); | ||
|
@@ -106,6 +109,18 @@ public void setStoreFilesRelativeToBibFile(boolean shouldStoreFilesRelativeToBib | |
this.storeFilesRelativeToBibFile.set(shouldStoreFilesRelativeToBibFile); | ||
} | ||
|
||
public boolean shouldAutoRenameFilesOnChange() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method should return an Optional instead of a primitive boolean to avoid returning null and to align with modern Java practices. |
||
return autoRenameFilesOnChange.get(); | ||
} | ||
|
||
public BooleanProperty autoRenameFilesOnChangeProperty() { | ||
return autoRenameFilesOnChange; | ||
} | ||
|
||
public void setAutoRenameFilesOnChange(boolean autoRenameFilesOnChange) { | ||
this.autoRenameFilesOnChange.set(autoRenameFilesOnChange); | ||
} | ||
|
||
public String getFileNamePattern() { | ||
return fileNamePattern.get(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The instantiation of AutoRenameFileOnEntryChange should be moved to org.jabref.logic package as it involves non-GUI operations, adhering to the layered architecture principle.