Skip to content

Commit

Permalink
Resolve more request changes and warnings (#5)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@karlsb
karlsb authored Mar 6, 2024
1 parent ecc7a90 commit 9d43c8d
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 58 deletions.
15 changes: 3 additions & 12 deletions src/main/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFiles.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,14 @@
import java.util.Objects;
import java.util.Optional;

import org.jabref.logic.externalfiles.LinkedFileHandler;
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;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class RemoveLinksToNotExistentFiles implements CleanupJob {

private static final Logger LOGGER = LoggerFactory.getLogger(RemoveLinksToNotExistentFiles.class);

private final BibDatabaseContext databaseContext;
private final FilePreferences filePreferences;

Expand All @@ -36,18 +29,16 @@ public List<FieldChange> cleanup(BibEntry entry) {
List<LinkedFile> cleanedUpFiles = new ArrayList<>();
boolean changed = false;
for (LinkedFile file : files) {
LinkedFileHandler fileHandler = new LinkedFileHandler(file, entry, databaseContext, filePreferences);

if (!file.isOnlineLink()) {
if (file.isOnlineLink()) {
cleanedUpFiles.add(file);
} else {
Optional<Path> oldFile = file.findIn(databaseContext, filePreferences);

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

Expand Down
90 changes: 44 additions & 46 deletions src/test/java/org/jabref/logic/cleanup/RemoveLinksToNotExistentFilesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
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.logic.bibtex.FileFieldWriter;
Expand All @@ -22,35 +21,32 @@
import org.junit.jupiter.api.io.TempDir;

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;

public class RemoveLinksToNotExistentFilesTest {
private Path defaultFileFolder;
private Path fileBefore;
private BibEntry entry;
private FilePreferences filePreferences;
private BibDatabaseContext databaseContext;
private RemoveLinksToNotExistentFiles removeLinks;

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

// The folder where the files are located originally
Path fileFolder = bibFolder.resolve("files");
Files.createDirectory(fileFolder);
fileBefore = fileFolder.resolve("test.pdf");
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(defaultFileFolder.toAbsolutePath().toString());
databaseContext = new BibDatabaseContext(new BibDatabase(), metaData);
Files.createFile(bibFolder.resolve("test.bib"));
databaseContext.setDatabasePath(bibFolder.resolve("test.bib"));
metaData.setDefaultFileDirectory(newFileFolder.toAbsolutePath().toString());

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

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

Expand All @@ -60,8 +56,8 @@ void setUp(@TempDir Path bibFolder) throws IOException {
.withField(StandardField.DATE, "April 2020")
.withField(StandardField.YEAR, "2020")
.withField(StandardField.DOI, "10.1109/TII.2019.2935531")
.withField(StandardField.FILE, FileFieldWriter.getStringRepresentation(Arrays.asList(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912:PDF", ""),
.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")
Expand All @@ -72,40 +68,40 @@ void setUp(@TempDir Path bibFolder) throws IOException {
.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 = mock(FilePreferences.class);
FilePreferences filePreferences = mock(FilePreferences.class);
when(filePreferences.shouldStoreFilesRelativeToBibFile()).thenReturn(false);
removeLinks = new RemoveLinksToNotExistentFiles(databaseContext, filePreferences);
}

@Test
void deleteFileInMultipleLinkedEntry() {
void deleteFileInEntryWithMultipleFileLinks() throws IOException {
LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");
FieldChange expectedChange = new FieldChange(entry, StandardField.FILE,
FileFieldWriter.getStringRepresentation(Arrays.asList(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912:PDF", ""),
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", ""))
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)");

fileBefore.toFile().delete();
.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(expectedChange, changes.getFirst());
assertEquals(List.of(expectedChange), changes);
assertEquals(expectedEntry, entry);
}

Expand All @@ -117,8 +113,8 @@ void keepLinksToExistingFiles() {
.withField(StandardField.DATE, "April 2020")
.withField(StandardField.YEAR, "2020")
.withField(StandardField.DOI, "10.1109/TII.2019.2935531")
.withField(StandardField.FILE, FileFieldWriter.getStringRepresentation(Arrays.asList(
new LinkedFile("", "https://ieeexplore.ieee.org/stamp/stamp.jsp?tp=&arnumber=8801912:PDF", ""),
.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")
Expand All @@ -131,14 +127,16 @@ void keepLinksToExistingFiles() {

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

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

@Test
void deleteLinkedFile() {
void deleteLinkedFile() throws IOException {
LinkedFile fileField = new LinkedFile("", fileBefore.toAbsolutePath(), "");
entry.setField(StandardField.FILE, FileFieldWriter.getStringRepresentation(fileField)); // There is only one linked file in entry

// 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);
Expand All @@ -156,10 +154,10 @@ void deleteLinkedFile() {
.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)");

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

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

0 comments on commit 9d43c8d

Please sign in to comment.