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

Add file description to gui and fix sync bug #3210

Merged
merged 9 commits into from
Sep 20, 2017
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public void acceptAsLinked() {
}

public Observable[] getObservables() {
return new Observable[] {this.downloadProgress, this.isAutomaticallyFound};
return linkedFile.getObservables();
}

public void open() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import javafx.scene.control.ListView;
import javafx.scene.control.MenuItem;
import javafx.scene.control.ProgressBar;
import javafx.scene.control.Separator;
import javafx.scene.control.SeparatorMenuItem;
import javafx.scene.control.Tooltip;
import javafx.scene.input.ClipboardContent;
Expand Down Expand Up @@ -56,7 +57,6 @@ public LinkedFilesEditor(String fieldName, DialogService dialogService, BibDatab
ControlHelper.loadFXMLForControl(this);

ViewModelListCellFactory<LinkedFileViewModel> cellFactory = new ViewModelListCellFactory<LinkedFileViewModel>()
.withTooltip(LinkedFileViewModel::getDescription)
.withGraphic(LinkedFilesEditor::createFileDisplay)
.withContextMenu(this::createContextMenuForFile)
.withOnMouseClickedEvent(this::handleItemMouseClick)
Expand Down Expand Up @@ -149,6 +149,8 @@ private void handleOnDragDropped(LinkedFileViewModel originalItem, DragEvent eve
private static Node createFileDisplay(LinkedFileViewModel linkedFile) {
Text icon = MaterialDesignIconFactory.get().createIcon(linkedFile.getTypeIcon());
Text text = new Text(linkedFile.getLink());
Text desc = new Text(linkedFile.getDescription());

ProgressBar progressIndicator = new ProgressBar();
progressIndicator.progressProperty().bind(linkedFile.downloadProgressProperty());
progressIndicator.visibleProperty().bind(linkedFile.downloadOngoingProperty());
Expand All @@ -161,7 +163,8 @@ private static Node createFileDisplay(LinkedFileViewModel linkedFile) {

HBox container = new HBox(10);
container.setPrefHeight(Double.NEGATIVE_INFINITY);
container.getChildren().addAll(icon, text, progressIndicator, acceptAutoLinkedFile);
Separator separator = new Separator();
container.getChildren().addAll(icon, text, separator, desc, progressIndicator, acceptAutoLinkedFile);
return container;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public LinkedFilesEditorViewModel(String fieldName, AutoCompleteSuggestionProvid
text,
LinkedFilesEditorViewModel::getStringRepresentation,
this::parseToFileViewModel);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor, but as far as I can see this blank line is unnecessary.

}

private static String getStringRepresentation(List<LinkedFileViewModel> files) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/model/entry/FileFieldWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

public class FileFieldWriter {

private FileFieldWriter() {
FileFieldWriter() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this constructor not private anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was a relict from testing. Didn't see that it had static method on the first try

}

public static String getStringRepresentation(List<LinkedFile> fields) {
Expand Down
45 changes: 30 additions & 15 deletions src/main/java/org/jabref/model/entry/LinkedFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
import java.util.Objects;
import java.util.Optional;

import javafx.beans.Observable;
import javafx.beans.property.BooleanProperty;
import javafx.beans.property.DoubleProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleDoubleProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;

import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.metadata.FileDirectoryPreferences;
import org.jabref.model.util.FileHelper;
Expand All @@ -19,42 +27,49 @@
public class LinkedFile implements Serializable {

private static final LinkedFile NULL_OBJECT = new LinkedFile("", "", "");
private String description;
private String link;
private String fileType;
private final StringProperty description = new SimpleStringProperty();
private final StringProperty link = new SimpleStringProperty();
private final StringProperty fileType = new SimpleStringProperty();
private final DoubleProperty downloadProgress = new SimpleDoubleProperty(-1);
private final BooleanProperty isAutomaticallyFound = new SimpleBooleanProperty(false);

public LinkedFile(String description, String link, String fileType) {
this.description = Objects.requireNonNull(description);
this.link = Objects.requireNonNull(link);
this.fileType = Objects.requireNonNull(fileType);
this.description.setValue(Objects.requireNonNull(description));
this.link.setValue(Objects.requireNonNull(link));
this.fileType.setValue(Objects.requireNonNull(fileType));
}

public LinkedFile(String description, URL link, String fileType) {
this(description, Objects.requireNonNull(link).toString(), fileType);
}

public String getFileType() {
return fileType;
return fileType.get();
}

public void setFileType(String fileType) {
this.fileType = fileType;
this.fileType.setValue(fileType);
}

public String getDescription() {
return description;
return description.get();
}

public void setDescription(String description) {
this.description = description;
this.description.setValue(description);

}

public String getLink() {
return link;
return link.get();
}

public void setLink(String link) {
this.link = link;
this.link.setValue(link);
}

public Observable[] getObservables() {
return new Observable[] {this.downloadProgress, this.isAutomaticallyFound};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to return description, link, fileType here (and move downloadProgress and isAutomaticallyFound back to the viewmodel).

The observable tell javafx which properties mark the object as changed, i.e. if description is changed this should mark the LinkedFile as changed and thus also LinkedFileViewModel and thus finally the list

private final ListProperty<LinkedFileViewModel> files = new SimpleListProperty<>(FXCollections.observableArrayList(LinkedFileViewModel::getObservables));
.
For more details see https://docs.oracle.com/javase/8/javafx/api/javafx/collections/FXCollections.html#observableList-java.util.List-javafx.util.Callback- or https://stackoverflow.com/questions/31687642/callback-and-extractors-for-javafx-observablelist

}

@Override
Expand Down Expand Up @@ -96,7 +111,7 @@ public boolean isEmpty() {
}

public boolean isOnlineLink() {
return link.startsWith("http://") || link.startsWith("https://") || link.contains("www.");
return link.get().startsWith("http://") || link.get().startsWith("https://") || link.get().contains("www.");
}

public Optional<Path> findIn(BibDatabaseContext databaseContext, FileDirectoryPreferences fileDirectoryPreferences) {
Expand All @@ -105,11 +120,11 @@ public Optional<Path> findIn(BibDatabaseContext databaseContext, FileDirectoryPr
}

public Optional<Path> findIn(List<Path> directories) {
Path file = Paths.get(link);
Path file = Paths.get(link.get());
if (file.isAbsolute() || directories.isEmpty()) {
return Optional.of(file);
} else {
return FileHelper.expandFilenameAsPath(link, directories);
return FileHelper.expandFilenameAsPath(link.get(), directories);
}
}
}
61 changes: 61 additions & 0 deletions src/test/java/org/jabref/model/entry/FileFieldBibEntryTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package org.jabref.model.entry;

import org.jabref.logic.exporter.BibtexDatabaseWriter;
import org.jabref.logic.exporter.SaveException;
import org.jabref.logic.exporter.SavePreferences;
import org.jabref.logic.exporter.StringSaveSession;
import org.jabref.logic.util.OS;
import org.jabref.model.Defaults;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.metadata.MetaData;

import org.junit.Before;
import org.junit.Test;

import static org.junit.Assert.assertEquals;

public class FileFieldBibEntryTest {

private BibEntry emptyEntry;

@Before
public void setUp() {
emptyEntry = new BibEntry();
emptyEntry.setType("article");
emptyEntry.setChanged(false);
}

@Test
public void testFileFieldSerialization() {
LinkedFile file = new LinkedFile("test", "/home/uers/test.pdf", "PDF");
emptyEntry.addFile(file);

assertEquals("@article{,\n" +
" file = {test:/home/uers/test.pdf:PDF}\n" +
"}", emptyEntry.toString());
}

@Test
public void testFileFieldSerializationDatabase() throws SaveException {
BibDatabase database = new BibDatabase();

LinkedFile file = new LinkedFile("test", "/home/uers/test.pdf", "PDF");
emptyEntry.addFile(file);
database.insertEntries(emptyEntry);

BibtexDatabaseWriter<StringSaveSession> databaseWriter = new BibtexDatabaseWriter<>(StringSaveSession::new);
StringSaveSession saveSession = databaseWriter.savePartOfDatabase(
new BibDatabaseContext(database, new MetaData(), new Defaults()), database.getEntries(),
new SavePreferences());

assertEquals(OS.NEWLINE +
"@Article{,"
+ OS.NEWLINE
+ " file = {test:/home/uers/test.pdf:PDF},"
+ OS.NEWLINE
+ "}" + OS.NEWLINE
+ OS.NEWLINE
+ "@Comment{jabref-meta: databaseType:bibtex;}" + OS.NEWLINE, saveSession.getStringValue());
}
}
16 changes: 12 additions & 4 deletions src/test/java/org/jabref/model/entry/FileFieldWriterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,17 @@ public void testQuoteNull() {

@Test
public void testEncodeStringArray() {
assertEquals("a:b;c:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", "b"}, {"c", "d"}}));
assertEquals("a:;c:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", ""}, {"c", "d"}}));
assertEquals("a:" + null + ";c:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", null}, {"c", "d"}}));
assertEquals("a:\\:b;c\\;:d", FileFieldWriter.encodeStringArray(new String[][]{{"a", ":b"}, {"c;", "d"}}));
assertEquals("a:b;c:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", "b"}, {"c", "d"}}));
assertEquals("a:;c:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", ""}, {"c", "d"}}));
assertEquals("a:" + null + ";c:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", null}, {"c", "d"}}));
assertEquals("a:\\:b;c\\;:d", FileFieldWriter.encodeStringArray(new String[][] {{"a", ":b"}, {"c;", "d"}}));
}

@Test
public void testFileFieldWriterGetStringRepresentation() {
LinkedFile file = new LinkedFile("test", "X:\\Users\\abc.pdf", "PDF");
assertEquals("test:X\\:\\\\Users\\\\abc.pdf:PDF", FileFieldWriter.getStringRepresentation(file));
}


}