Skip to content

Commit

Permalink
Resolve requested "nitpicks" and warnings. (#7) (#10960)
Browse files Browse the repository at this point in the history
* Adds feature CleanupJob for removing non-existent files (#2)

* feat: RemoveLinksToNotExistentFiles.java

* feat: added option to cleanup entries

* feat: improved cleanup entry dialogue

* Test: implements test for linked files which exist

Implementation of a test which calls the cleanup functionality of
RemoveLinksToNotExistentFiles with a entry that is linked to an existing
file. The test makes sure that no changes are made since the linked
file exists.

* Refactor: refactored test to match the standard of the project

Removed the JavaDoc comment from the test, since other tests in the
project don't have JavaDoc:s. Also removed test from the name of the
test function, as it seems that the standard way of naming tests
is to not have test in the testfunctions name.

* Refactor: removed system.out.print()

Removed an print from the tests.

* fix: non english character deleted

* Docs: Added information about changes

Added information about the changes made for solving issue#10929
to the CHANGES.md file.

---------



* Fix: Removes unused imports and solves other errors

Removes unused imports and solves the error of language keys
that were not in the english laguage file.

* Fix: Removes empty line and solves other issues

Removes an empty line where 2 empty lines appear in after each
other. Also adds brackets to URL and removes an key which is
not in the english language file and was missed in the last commit.

* Docs: Removes technicality from changelog

Removes technical details for the change made in changelog and also
follows the suggestion for removing the space and brackets between
the link and the issue.

* test: enhanced RemoveLinksToNotExistentFilesTest (#3)

* test: enhanced RemoveLinksToNotExistentFilesTest

* test: add real content

* test: change structure

* test: added expectedChanges

* test: added a third test to RemoveLinksToNotExistentFilesTest

* refactor: removed unnecessary toString

* style: removed a whitespace

* refactor: checkstyle formatting.

formatted files according to checkstyle plugin.

---------



* Resolve request changes (#4)

* test: enhanced RemoveLinksToNotExistentFilesTest

* test: add real content

* test: change structure

* test: added expectedChanges

* test: added a third test to RemoveLinksToNotExistentFilesTest

* refactor: removed unnecessary toString

* style: removed a whitespace

* refactor: checkstyle formatting.

formatted files according to checkstyle plugin.

* refactor: replaced .get(0) with .getFirst() for first accessing element of List.

Changed was required from ./gradlew rewriteDryRun

---------



* Resolve more request changes and warnings (#5)

* Reverse the condition and the content in RemoveLinksToNotExistentFiles

Resolves comment
 "Reverse the condition and the content.

Reason: You have BOTH (true and false) cases covered. The true case should normally come first."

* Fix: Store bibFolder.resolve("test.bib") in a variable

* Refactor: Rename variable fileFolder to originalFileFolder and remove comment.

* Refactor: Rename defaultFileFolder to newFileFolder

* Refactor: Fix indentation in RemoveLinksToNotExistentFilesTest

* Refactor: Replaced Arrays.asList() with List.of()

* Refactor: Replaced Arrays.asList() with List.of()

* Refactor: Change to use java.nio for file deletion.

* Refactor: Move comment to line above.

* Refactor: Added "PDF" as third argument to LinkedFile for OnlineLink files

* Refactor: Removed unused variables in RemoveLinksToNotExistentFiles

* Refactor: Throw IOException in RemoveLinksToNotExistentFilesTest functions

* Refactor: Tests assert using List.of() in RemoveLinksToNotExistentFilesTest

* Refactor: Corrected Arguments for LinkedFile in RemoveLinksToNotExistentFilesTest

* Refactor: Class fields into local variables

This refactor fixed 3 warnings.

* docs: updated docs to remove merge conflict

---------

Co-authored-by: karlsb <36365664+karlsb@users.noreply.github.com>
Co-authored-by: burcukilic <94201593+burcukilic@users.noreply.github.com>
  • Loading branch information
3 people committed Mar 6, 2024
1 parent f5efb34 commit 985b40b
Show file tree
Hide file tree
Showing 7 changed files with 257 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We added ability to export in CFF (Citation File Format) [#10661](https://github.com/JabRef/jabref/issues/10661).
- We added ability to push entries to TeXworks. [#3197](https://github.com/JabRef/jabref/issues/3197)
- We added the ability to zoom in and out in the document viewer using <kbd>Ctrl</kbd> + <kbd>Scroll</kbd>. [#10964](https://github.com/JabRef/jabref/pull/10964)
- We added a Cleanup for removing non-existent files and grouped the related options [#10929](https://github.com/JabRef/jabref/issues/10929)

### Changed

Expand Down
51 changes: 32 additions & 19 deletions src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,39 @@
<ToggleGroup fx:id="timestampCleanup"/>
</fx:define>


<CheckBox fx:id="cleanUpDOI" text="%Move DOIs from note and URL field to DOI field and remove http prefix" />
<CheckBox fx:id="cleanUpEprint" text="%Move preprint information from 'URL' and 'journal' field to the 'eprint' field" />
<CheckBox fx:id="cleanUpURL" text="%Move URL in note field to url field" />
<CheckBox fx:id="cleanUpISSN" text="%Reformat ISSN" />
<CheckBox fx:id="cleanUpUpgradeExternalLinks" />
<CheckBox fx:id="cleanUpMovePDF" />
<CheckBox fx:id="cleanUpMakePathsRelative" text="%Make paths of linked files relative (if possible)" />
<CheckBox fx:id="cleanUpRenamePDF" text="%Rename PDFs to given filename format pattern" />
<VBox prefHeight="40.0" prefWidth="451.0" spacing="10.0">
<Label fx:id="cleanupRenamePDFLabel" />
<CheckBox fx:id="cleanUpRenamePDFonlyRelativePaths" text="%Rename only PDFs having a relative path" />
<VBox.margin>
<Insets left="20.0" />
</VBox.margin>
<Label text="Miscellanous" />
<VBox>
<VBox.margin>
<Insets left="20.0" />
</VBox.margin>
<CheckBox fx:id="cleanUpDOI" text="%Move DOIs from note and URL field to DOI field and remove http prefix" />
<CheckBox fx:id="cleanUpEprint" text="%Move preprint information from 'URL' and 'journal' field to the 'eprint' field" />
<CheckBox fx:id="cleanUpURL" text="%Move URL in note field to url field" />
<CheckBox fx:id="cleanUpISSN" text="%Reformat ISSN" />
<CheckBox fx:id="cleanUpBiblatex" text="%Convert to biblatex format (e.g., store publication date in date field)" />
<CheckBox fx:id="cleanUpBibtex" text="%Convert to BibTeX format (e.g., store publication date in year and month fields)" />
<CheckBox fx:id="cleanUpTimestampToCreationDate" text="%Convert timestamp field to field 'creationdate'" />
<CheckBox fx:id="cleanUpTimestampToModificationDate" text="%Convert timestamp field to field 'modificationdate'" />
</VBox>

<Label text="File Related" />
<VBox>
<VBox.margin>
<Insets left="20.0" />
</VBox.margin>
<CheckBox fx:id="cleanUpUpgradeExternalLinks" />
<CheckBox fx:id="cleanUpMovePDF" />
<CheckBox fx:id="cleanUpMakePathsRelative" text="%Make paths of linked files relative (if possible)" />
<CheckBox fx:id="cleanUpRenamePDF" text="%Rename PDFs to given filename format pattern" />
<VBox prefHeight="40.0" prefWidth="451.0" spacing="10.0">
<Label fx:id="cleanupRenamePDFLabel" />
<CheckBox fx:id="cleanUpRenamePDFonlyRelativePaths" text="%Rename only PDFs having a relative path" />
<VBox.margin>
<Insets left="20.0" />
</VBox.margin>
</VBox>
<CheckBox fx:id="cleanUpDeletedFiles" text="Remove links to non existent files" />
</VBox>
<CheckBox fx:id="cleanUpBiblatex" text="%Convert to biblatex format (e.g., store publication date in date field)" />
<CheckBox fx:id="cleanUpBibtex" text="%Convert to BibTeX format (e.g., store publication date in year and month fields)" />
<CheckBox fx:id="cleanUpTimestampToCreationDate" text="%Convert timestamp field to field 'creationdate'" />
<CheckBox fx:id="cleanUpTimestampToModificationDate" text="%Convert timestamp field to field 'modificationdate'" />
<FieldFormatterCleanupsPanel fx:id="formatterCleanupsPanel" />
<padding>
<Insets bottom="10.0" left="15.0" right="10.0" top="10.0" />
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/org/jabref/gui/cleanup/CleanupPresetPanel.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class CleanupPresetPanel extends VBox {
@FXML private CheckBox cleanUpMakePathsRelative;
@FXML private CheckBox cleanUpRenamePDF;
@FXML private CheckBox cleanUpRenamePDFonlyRelativePaths;
@FXML private CheckBox cleanUpDeletedFiles;
@FXML private CheckBox cleanUpUpgradeExternalLinks;
@FXML private CheckBox cleanUpBiblatex;
@FXML private CheckBox cleanUpBibtex;
Expand Down Expand Up @@ -71,6 +72,7 @@ private void init(CleanupPreferences cleanupPreferences, FilePreferences filePre
.concat(": ")
.concat(filePreferences.getFileNamePattern());
cleanupRenamePDFLabel.setText(currentPattern);

cleanUpBibtex.selectedProperty().addListener(
(observable, oldValue, newValue) -> {
if (newValue) {
Expand Down Expand Up @@ -109,6 +111,7 @@ private void updateDisplay(CleanupPreferences preset) {
cleanUpRenamePDF.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF));
cleanUpRenamePDFonlyRelativePaths.setSelected(preset.isActive(CleanupPreferences.CleanupStep.RENAME_PDF_ONLY_RELATIVE_PATHS));
cleanUpUpgradeExternalLinks.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS));
cleanUpDeletedFiles.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES));
cleanUpBiblatex.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX));
cleanUpBibtex.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TO_BIBTEX));
cleanUpTimestampToCreationDate.setSelected(preset.isActive(CleanupPreferences.CleanupStep.CONVERT_TIMESTAMP_TO_CREATIONDATE));
Expand Down Expand Up @@ -150,6 +153,9 @@ public CleanupPreferences getCleanupPreset() {
if (cleanUpUpgradeExternalLinks.isSelected()) {
activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_UPGRADE_EXTERNAL_LINKS);
}
if (cleanUpDeletedFiles.isSelected()) {
activeJobs.add(CleanupPreferences.CleanupStep.CLEAN_UP_DELETED_LINKED_FILES);
}
if (cleanUpBiblatex.isSelected()) {
activeJobs.add(CleanupPreferences.CleanupStep.CONVERT_TO_BIBLATEX);
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/jabref/logic/cleanup/CleanupWorker.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ private CleanupJob toJob(CleanupPreferences.CleanupStep action) {
new RenamePdfCleanup(true, databaseContext, filePreferences);
case CLEAN_UP_UPGRADE_EXTERNAL_LINKS ->
new UpgradePdfPsToFileCleanup();
case CLEAN_UP_DELETED_LINKED_FILES ->
new RemoveLinksToNotExistentFiles(databaseContext, filePreferences);
case CONVERT_TO_BIBLATEX ->
new ConvertToBiblatexCleanup();
case CONVERT_TO_BIBTEX ->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package org.jabref.logic.cleanup;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;

import org.jabref.model.FieldChange;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.LinkedFile;
import org.jabref.model.util.OptionalUtil;
import org.jabref.preferences.FilePreferences;

public class RemoveLinksToNotExistentFiles implements CleanupJob {
private final BibDatabaseContext databaseContext;
private final FilePreferences filePreferences;

public RemoveLinksToNotExistentFiles(BibDatabaseContext databaseContext, FilePreferences filePreferences) {
this.databaseContext = Objects.requireNonNull(databaseContext);
this.filePreferences = Objects.requireNonNull(filePreferences);
}

@Override
public List<FieldChange> cleanup(BibEntry entry) {
List<LinkedFile> files = entry.getFiles();
List<LinkedFile> cleanedUpFiles = new ArrayList<>();
boolean changed = false;
for (LinkedFile file : files) {
if (file.isOnlineLink()) {
cleanedUpFiles.add(file);
} else {
Optional<Path> oldFile = file.findIn(databaseContext, filePreferences);

if (oldFile.isEmpty()) {
changed = true;
} else {
cleanedUpFiles.add(file);
}
}
}

if (changed) {
Optional<FieldChange> changes = entry.setFiles(cleanedUpFiles);
return OptionalUtil.toList(changes);
}

return Collections.emptyList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public enum CleanupStep {
* Collects file links from the pdf or ps field, and adds them to the list contained in the file field.
*/
CLEAN_UP_UPGRADE_EXTERNAL_LINKS,
CLEAN_UP_DELETED_LINKED_FILES,
/**
* Converts to biblatex format
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
package org.jabref.logic.cleanup;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;

import org.jabref.logic.bibtex.FileFieldWriter;
import org.jabref.model.FieldChange;
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.jabref.preferences.FilePreferences;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class RemoveLinksToNotExistentFilesTest {
private Path fileBefore;
private BibEntry entry;
private RemoveLinksToNotExistentFiles removeLinks;

@BeforeEach
void setUp(@TempDir Path bibFolder) throws IOException {
// The folder where the files should be moved to
Path newFileFolder = bibFolder.resolve("pdf");
Files.createDirectory(newFileFolder);

Path originalFileFolder = bibFolder.resolve("files");
Path testBibFolder = bibFolder.resolve("test.bib");
Files.createDirectory(originalFileFolder);
fileBefore = originalFileFolder.resolve("test.pdf");
Files.createFile(fileBefore);

MetaData metaData = new MetaData();
metaData.setDefaultFileDirectory(newFileFolder.toAbsolutePath().toString());

BibDatabaseContext databaseContext = new BibDatabaseContext(new BibDatabase(), metaData);
Files.createFile(testBibFolder);
databaseContext.setDatabasePath(testBibFolder);

LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");

// Entry with one online and one normal linked file
entry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.AUTHOR, "Shatakshi Sharma and Bhim Singh and Sukumar Mishra")
.withField(StandardField.DATE, "April 2020")
.withField(StandardField.YEAR, "2020")
.withField(StandardField.DOI, "10.1109/TII.2019.2935531")
.withField(StandardField.FILE, FileFieldWriter.getStringRepresentation(List.of(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912", "PDF"),
fileField)))
.withField(StandardField.ISSUE, "4")
.withField(StandardField.ISSN, "1941-0050")
.withField(StandardField.JOURNALTITLE, "IEEE Transactions on Industrial Informatics")
.withField(StandardField.PAGES, "2346--2356")
.withField(StandardField.PUBLISHER, "IEEE")
.withField(StandardField.TITLE, "Economic Operation and Quality Control in PV-BES-DG-Based Autonomous System")
.withField(StandardField.VOLUME, "16")
.withField(StandardField.KEYWORDS, "Batteries, Generators, Economics, Power quality, State of charge, Harmonic analysis, Control systems, Battery, diesel generator (DG), distributed generation, power quality, photovoltaic (PV), voltage source converter (VSC)");

FilePreferences filePreferences = mock(FilePreferences.class);
when(filePreferences.shouldStoreFilesRelativeToBibFile()).thenReturn(false);
removeLinks = new RemoveLinksToNotExistentFiles(databaseContext, filePreferences);
}

@Test
void deleteFileInEntryWithMultipleFileLinks() throws IOException {
LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");
FieldChange expectedChange = new FieldChange(entry, StandardField.FILE,
FileFieldWriter.getStringRepresentation(List.of(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912", "PDF"),
fileField)),
FileFieldWriter.getStringRepresentation(new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912", "PDF"))
);
BibEntry expectedEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.AUTHOR, "Shatakshi Sharma and Bhim Singh and Sukumar Mishra")
.withField(StandardField.DATE, "April 2020")
.withField(StandardField.YEAR, "2020")
.withField(StandardField.DOI, "10.1109/TII.2019.2935531")
.withField(StandardField.FILE, FileFieldWriter.getStringRepresentation(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912", "PDF")))
.withField(StandardField.ISSUE, "4")
.withField(StandardField.ISSN, "1941-0050")
.withField(StandardField.JOURNALTITLE, "IEEE Transactions on Industrial Informatics")
.withField(StandardField.PAGES, "2346--2356")
.withField(StandardField.PUBLISHER, "IEEE")
.withField(StandardField.TITLE, "Economic Operation and Quality Control in PV-BES-DG-Based Autonomous System")
.withField(StandardField.VOLUME, "16")
.withField(StandardField.KEYWORDS, "Batteries, Generators, Economics, Power quality, State of charge, Harmonic analysis, Control systems, Battery, diesel generator (DG), distributed generation, power quality, photovoltaic (PV), voltage source converter (VSC)");

Files.delete(fileBefore);
List<FieldChange> changes = removeLinks.cleanup(entry);

assertEquals(List.of(expectedChange), changes);
assertEquals(expectedEntry, entry);
}

@Test
void keepLinksToExistingFiles() {
LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");
BibEntry expectedEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.AUTHOR, "Shatakshi Sharma and Bhim Singh and Sukumar Mishra")
.withField(StandardField.DATE, "April 2020")
.withField(StandardField.YEAR, "2020")
.withField(StandardField.DOI, "10.1109/TII.2019.2935531")
.withField(StandardField.FILE, FileFieldWriter.getStringRepresentation(List.of(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912", "PDF"),
fileField)))
.withField(StandardField.ISSUE, "4")
.withField(StandardField.ISSN, "1941-0050")
.withField(StandardField.JOURNALTITLE, "IEEE Transactions on Industrial Informatics")
.withField(StandardField.PAGES, "2346--2356")
.withField(StandardField.PUBLISHER, "IEEE")
.withField(StandardField.TITLE, "Economic Operation and Quality Control in PV-BES-DG-Based Autonomous System")
.withField(StandardField.VOLUME, "16")
.withField(StandardField.KEYWORDS, "Batteries, Generators, Economics, Power quality, State of charge, Harmonic analysis, Control systems, Battery, diesel generator (DG), distributed generation, power quality, photovoltaic (PV), voltage source converter (VSC)");

List<FieldChange> changes = removeLinks.cleanup(entry);

assertEquals(List.of(), changes);
assertEquals(expectedEntry, entry);
}

@Test
void deleteLinkedFile() throws IOException {
LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");

// There is only one linked file in entry
entry.setField(StandardField.FILE, FileFieldWriter.getStringRepresentation(fileField));
FieldChange expectedChange = new FieldChange(entry, StandardField.FILE,
FileFieldWriter.getStringRepresentation(fileField),
null);
BibEntry expectedEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.AUTHOR, "Shatakshi Sharma and Bhim Singh and Sukumar Mishra")
.withField(StandardField.DATE, "April 2020")
.withField(StandardField.YEAR, "2020")
.withField(StandardField.DOI, "10.1109/TII.2019.2935531")
.withField(StandardField.ISSUE, "4")
.withField(StandardField.ISSN, "1941-0050")
.withField(StandardField.JOURNALTITLE, "IEEE Transactions on Industrial Informatics")
.withField(StandardField.PAGES, "2346--2356")
.withField(StandardField.PUBLISHER, "IEEE")
.withField(StandardField.TITLE, "Economic Operation and Quality Control in PV-BES-DG-Based Autonomous System")
.withField(StandardField.VOLUME, "16")
.withField(StandardField.KEYWORDS, "Batteries, Generators, Economics, Power quality, State of charge, Harmonic analysis, Control systems, Battery, diesel generator (DG), distributed generation, power quality, photovoltaic (PV), voltage source converter (VSC)");

Files.delete(fileBefore);
List<FieldChange> changes = removeLinks.cleanup(entry);

assertEquals(List.of(expectedChange), changes);
assertEquals(expectedEntry, entry);
}
}

0 comments on commit 985b40b

Please sign in to comment.