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

Fix hang when exporting at the CLI #10383

Merged
merged 17 commits into from
Sep 16, 2023
Merged
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv

- We added the possibility to find (and add) papers that cite or are cited by a given paper. [#6187](https://github.com/JabRef/jabref/issues/6187)
- We added an error-specific message for when a download from a URL fails. [#9826](https://github.com/JabRef/jabref/issues/9826)
- We added support for customizating the citation command (e.g., `[@key1,@key2]`) when [pushing to external applications](https://docs.jabref.org/cite/pushtoapplications). [#10133](https://github.com/JabRef/jabref/issues/10133)
- We added support for customizing the citation command (e.g., `[@key1,@key2]`) when [pushing to external applications](https://docs.jabref.org/cite/pushtoapplications). [#10133](https://github.com/JabRef/jabref/issues/10133)
- We added an integrity check for more special characters. [#8712](https://github.com/JabRef/jabref/issues/8712)
- We added protected terms described as "Computer science". [#10222](https://github.com/JabRef/jabref/pull/10222)

### Changed

- In the exports listrefs, tablerefs, tablerefsabsbib, use ISO date format in the footer.

### Fixed

- It is possible again to use "current table sort order" for the order of entries when saving. [#9869](https://github.com/JabRef/jabref/issues/9869)
Expand All @@ -28,6 +30,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We fixed an issue where the ISBN fetcher returned the entrytype `misc` for certain ISBN numbers [#10348](https://github.com/JabRef/jabref/issues/10348)
- We fixed a bug where an exception was raised when saving less than three export save orders in the preference. [#10157](https://github.com/JabRef/jabref/issues/10157)
- We fixed an issue where it was possible to create a group with no name or with a group separator inside the name [#9776](https://github.com/JabRef/jabref/issues/9776)
- JabRef does not hang anymore when exporting via CLI. [#10380](https://github.com/JabRef/jabref/issues/10380)

### Removed

Expand Down
2 changes: 1 addition & 1 deletion buildres/abbrv.jabref.org
62 changes: 31 additions & 31 deletions src/main/java/org/jabref/cli/ArgumentProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,20 @@ public class ArgumentProcessor {

private static final Logger LOGGER = LoggerFactory.getLogger(ArgumentProcessor.class);
private final JabRefCLI cli;
private final List<ParserResult> parserResults;

// Written once by processArguments()
private List<ParserResult> parserResults = List.of();

private final Mode startupMode;
private final PreferencesService preferencesService;
private final FileUpdateMonitor fileUpdateMonitor;
private final BibEntryTypesManager entryTypesManager;
private boolean noGUINeeded;

/**
* First call the constructor, then call {@link #processArguments()}.
* Afterward, you can access the {@link #getParserResults()} and other getters.
*/
public ArgumentProcessor(String[] args,
Mode startupMode,
PreferencesService preferencesService,
Expand All @@ -84,8 +91,6 @@ public ArgumentProcessor(String[] args,
this.preferencesService = preferencesService;
this.fileUpdateMonitor = fileUpdateMonitor;
this.entryTypesManager = entryTypesManager;

this.parserResults = processArguments();
}

/**
Expand Down Expand Up @@ -145,8 +150,8 @@ private Optional<ParserResult> importFile(String argument) {

Optional<ParserResult> importResult = importFile(file, importFormat);
importResult.ifPresent(result -> {
OutputPrinter printer = new SystemOutputPrinter();
if (result.hasWarnings()) {
OutputPrinter printer = new SystemOutputPrinter();
printer.showMessage(result.getErrorMessage());
}
});
Expand All @@ -161,22 +166,21 @@ private Optional<ParserResult> importFile(Path file, String importFormat) {
fileUpdateMonitor);

if (!"*".equals(importFormat)) {
System.out.println(Localization.lang("Importing") + ": " + file);
System.out.println(Localization.lang("Importing %0", file));
ParserResult result = importFormatReader.importFromFile(importFormat, file);
return Optional.of(result);
} else {
// * means "guess the format":
System.out.println(Localization.lang("Importing in unknown format") + ": " + file);
System.out.println(Localization.lang("Importing file %0 as unknown format", file));

ImportFormatReader.UnknownFormatImport importResult =
importFormatReader.importUnknownFormat(file, new DummyFileUpdateMonitor());

System.out.println(Localization.lang("Format used") + ": " + importResult.format());
System.out.println(Localization.lang("Format used: %0", importResult.format()));
return Optional.of(importResult.parserResult());
}
} catch (ImportException ex) {
System.err
.println(Localization.lang("Error opening file") + " '" + file + "': " + ex.getLocalizedMessage());
System.err.println(Localization.lang("Error opening file '%0'", file) + "\n" + ex.getLocalizedMessage());
return Optional.empty();
}
}
Expand All @@ -185,19 +189,16 @@ public List<ParserResult> getParserResults() {
return parserResults;
}

public boolean hasParserResults() {
return !parserResults.isEmpty();
}

private List<ParserResult> processArguments() {
public void processArguments() {
if ((startupMode == Mode.INITIAL_START) && cli.isShowVersion()) {
cli.displayVersion();
}

if ((startupMode == Mode.INITIAL_START) && cli.isHelp()) {
JabRefCLI.printUsage(preferencesService);
noGUINeeded = true;
return Collections.emptyList();
this.parserResults = Collections.emptyList();
return;
}

// Check if we should reset all preferences to default values:
Expand All @@ -220,7 +221,8 @@ private List<ParserResult> processArguments() {
if (cli.isExportMatches()) {
if (!loaded.isEmpty()) {
if (!exportMatches(loaded)) {
return Collections.emptyList();
this.parserResults = Collections.emptyList();
return;
}
} else {
System.err.println(Localization.lang("The output option depends on a valid input option."));
Expand Down Expand Up @@ -275,7 +277,7 @@ private List<ParserResult> processArguments() {
doAuxImport(loaded);
}

return loaded;
this.parserResults = loaded;
}

private void writeMetadataToPdf(List<ParserResult> loaded,
Expand Down Expand Up @@ -475,15 +477,14 @@ private boolean exportMatches(List<ParserResult> loaded) {
Globals.entryTypesManager);
Optional<Exporter> exporter = exporterFactory.getExporterByName(formatName);
if (exporter.isEmpty()) {
System.err.println(Localization.lang("Unknown export format") + ": " + formatName);
System.err.println(Localization.lang("Unknown export format %0", formatName));
} else {
// We have an TemplateExporter instance:
try {
System.out.println(Localization.lang("Exporting") + ": " + data[1]);
System.out.println(Localization.lang("Exporting %0", data[1]));
exporter.get().export(databaseContext, Path.of(data[1]), matches, Collections.emptyList(), Globals.journalAbbreviationRepository);
} catch (Exception ex) {
System.err.println(Localization.lang("Could not export file") + " '" + data[1] + "': "
+ Throwables.getStackTraceAsString(ex));
System.err.println(Localization.lang("Could not export file '%0' (reason: %1)", data[1], Throwables.getStackTraceAsString(ex)));
}
}
}
Expand All @@ -503,7 +504,7 @@ private void doAuxImport(List<ParserResult> loaded) {
}

if (usageMsg) {
System.out.println(Localization.lang("no base-BibTeX-file specified") + "!");
System.out.println(Localization.lang("no base-BibTeX-file specified!"));
System.out.println(Localization.lang("usage") + " :");
System.out.println("jabref --aux infile[.aux],outfile[.bib] base-BibTeX-file");
}
Expand Down Expand Up @@ -634,32 +635,31 @@ private void exportFile(List<ParserResult> loaded, String[] data) {
} else if (data.length == 2) {
// This signals that the latest import should be stored in the given
// format to the given file.
ParserResult pr = loaded.get(loaded.size() - 1);
ParserResult parserResult = loaded.get(loaded.size() - 1);

Path path = pr.getPath().get().toAbsolutePath();
BibDatabaseContext databaseContext = pr.getDatabaseContext();
Path path = parserResult.getPath().get().toAbsolutePath();
BibDatabaseContext databaseContext = parserResult.getDatabaseContext();
databaseContext.setDatabasePath(path);
List<Path> fileDirForDatabase = databaseContext
.getFileDirectories(preferencesService.getFilePreferences());
System.out.println(Localization.lang("Exporting") + ": " + data[0]);
System.out.println(Localization.lang("Exporting %0", data[0]));
ExporterFactory exporterFactory = ExporterFactory.create(
preferencesService,
Globals.entryTypesManager);
Optional<Exporter> exporter = exporterFactory.getExporterByName(data[1]);
if (exporter.isEmpty()) {
System.err.println(Localization.lang("Unknown export format") + ": " + data[1]);
System.err.println(Localization.lang("Unknown export format %0", data[1]));
} else {
// We have an exporter:
try {
exporter.get().export(
pr.getDatabaseContext(),
parserResult.getDatabaseContext(),
Path.of(data[0]),
pr.getDatabaseContext().getDatabase().getEntries(),
parserResult.getDatabaseContext().getDatabase().getEntries(),
fileDirForDatabase,
Globals.journalAbbreviationRepository);
} catch (Exception ex) {
System.err.println(Localization.lang("Could not export file") + " '" + data[0] + "': "
+ Throwables.getStackTraceAsString(ex));
System.err.println(Localization.lang("Could not export file '%0' (reason: %1)", data[0], Throwables.getStackTraceAsString(ex)));
}
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/main/java/org/jabref/cli/Launcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,18 @@ public static void main(String[] args) {
BibEntryTypesManager entryTypesManager = new BibEntryTypesManager();
Globals.entryTypesManager = entryTypesManager;

// Init preferences
// Initialize preferences
final JabRefPreferences preferences = JabRefPreferences.getInstance();
Globals.prefs = preferences;
PreferencesMigrations.runMigrations(preferences, entryTypesManager);

// Early exit in case another instance is already running
if (!handleMultipleAppInstances(ARGUMENTS, preferences)) {
if (!handleMultipleAppInstances(ARGUMENTS, preferences.getRemotePreferences())) {
return;
}

// Init rest of preferences
Globals.prefs = preferences;
PreferencesMigrations.runMigrations(preferences, entryTypesManager);

// Initialize rest of preferences
configureProxy(preferences.getProxyPreferences());
configureSSL(preferences.getSSLPreferences());
initGlobals(preferences);
Expand All @@ -86,13 +87,17 @@ public static void main(String[] args) {

// Process arguments
ArgumentProcessor argumentProcessor = new ArgumentProcessor(
ARGUMENTS, ArgumentProcessor.Mode.INITIAL_START,
ARGUMENTS,
ArgumentProcessor.Mode.INITIAL_START,
preferences,
fileUpdateMonitor,
entryTypesManager);
argumentProcessor.processArguments();
if (argumentProcessor.shouldShutDown()) {
LOGGER.debug("JabRef shut down after processing command line arguments");
return;
// A clean shutdown takes 60s time
// We don't need the clean shutdown here
System.exit(0);
}

MainApplication.main(argumentProcessor.getParserResults(), argumentProcessor.isBlank(), preferences, fileUpdateMonitor, ARGUMENTS);
Expand Down Expand Up @@ -141,8 +146,7 @@ private static void initializeLogger() {
LOGGER = LoggerFactory.getLogger(MainApplication.class);
}

private static boolean handleMultipleAppInstances(String[] args, PreferencesService preferences) {
RemotePreferences remotePreferences = preferences.getRemotePreferences();
private static boolean handleMultipleAppInstances(String[] args, RemotePreferences remotePreferences) {
if (remotePreferences.useRemoteServer()) {
// Try to contact already running JabRef
RemoteClient remoteClient = new RemoteClient(remotePreferences.getPort());
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.jabref.logic.util.BuildInfo;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.util.FileUpdateMonitor;
import org.jabref.preferences.JabRefPreferences;
import org.jabref.preferences.PreferencesService;

import kong.unirest.Unirest;

Expand Down Expand Up @@ -43,7 +43,7 @@ public class Globals {
/**
* Each test case initializes this field if required
*/
public static JabRefPreferences prefs;
public static PreferencesService prefs;

/**
* This field is initialized upon startup.
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/JabRefGUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ private void openDatabases() {
}

for (ParserResult pr : failed) {
String message = Localization.lang("Error opening file '%0'.",
String message = Localization.lang("Error opening file '%0'",
pr.getPath().map(Path::toString).orElse("(File name unknown)")) + "\n" +
pr.getErrorMessage();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public void open() {
dialogService.showErrorDialogAndWait(Localization.lang("File not found"), Localization.lang("Could not find file '%0'.", linkedFile.getLink()));
}
} catch (IOException e) {
dialogService.showErrorDialogAndWait(Localization.lang("Error opening file '%0'.", linkedFile.getLink()), e);
dialogService.showErrorDialogAndWait(Localization.lang("Error opening file '%0'", linkedFile.getLink()), e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/importer/ImportCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private ParserResult doImport(List<Path> files, Importer importFormat) throws IO
if (FileUtil.isPDFFile(filename) && GrobidOptInDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferencesService.getGrobidPreferences())) {
importFormatReader.reset();
}
dialogService.notify(Localization.lang("Importing in unknown format") + "...");
dialogService.notify(Localization.lang("Importing file %0 as unknown format", filename.getFileName().toString()));
});
// This import method never throws an IOException
imports.add(importFormatReader.importUnknownFormat(filename, fileUpdateMonitor));
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/remote/CLIMessageHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void handleCommandLineArguments(String[] message) {
preferencesService,
fileUpdateMonitor,
entryTypesManager);

argumentProcessor.processArguments();
List<ParserResult> loaded = argumentProcessor.getParserResults();
for (int i = 0; i < loaded.size(); i++) {
ParserResult pr = loaded.get(i);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/logic/bst/BstPreviewLayout.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ public BstPreviewLayout(Path path) {
name = path.getFileName().toString();
if (!Files.exists(path)) {
LOGGER.error("File {} not found", path.toAbsolutePath());
error = Localization.lang("Error opening file '%0'.", path.toString());
error = Localization.lang("Error opening file '%0'", path.toString());
return;
}
try {
bstVM = new BstVM(path);
} catch (Exception e) {
LOGGER.error("Could not read {}.", path.toAbsolutePath(), e);
error = Localization.lang("Error opening file '%0'.", path.toString());
error = Localization.lang("Error opening file '%0'", path.toString());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,6 @@ public void export(final BibDatabaseContext databaseContext,
missingFormatters.addAll(endLayout.getMissingFormatters());
}

// Clear custom name formatters:
layoutPreferences.clearCustomExportNameFormatters();

if (!missingFormatters.isEmpty() && LOGGER.isWarnEnabled()) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/logic/layout/Layout.java
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public String doLayout(BibEntry bibtex, BibDatabase database) {

/**
* Returns the processed text. If the database argument is
* null, no string references will be resolved. Otherwise all valid
* null, no string references will be resolved. Otherwise, all valid
* string references will be replaced by the strings' contents. Even
* recursive string references are resolved.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public ExportPreferences(String lastExportExtension,
Path exportWorkingDirectory,
SaveOrder exportSaveOrder,
List<TemplateExporter> customExporters) {

this.lastExportExtension = new SimpleStringProperty(lastExportExtension);
this.exportWorkingDirectory = new SimpleObjectProperty<>(exportWorkingDirectory);
this.exportSaveOrder = new SimpleObjectProperty<>(exportSaveOrder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2287,7 +2287,7 @@ private void storeExportSaveOrder(SaveOrder saveOrder) {
* For the export configuration, generates the SelfContainedSaveOrder having the reference to TABLE resolved.
*/
public SelfContainedSaveOrder getSelfContainedTableSaveOrder() {
List<MainTableColumnModel> sortOrder = mainTableColumnPreferences.getColumnSortOrder();
List<MainTableColumnModel> sortOrder = getMainTableColumnPreferences().getColumnSortOrder();
return new SelfContainedSaveOrder(
SaveOrder.OrderType.SPECIFIED,
sortOrder.stream().flatMap(model -> model.getSortCriteria().stream()).toList());
Expand Down
Loading