Improve responsiveness of library loading

[calixtus]

When JabRef opens a library, it opens a file, runs the database parser, and after everything is done, creates a new tab in the frame with the contents of the open database.

An improvement would be to first create a tab in the frame, display a loading animation, and after the parser has finished, display all entries (or display every entry as soon as it is parsed).

[sustcstar]

I am gonna check this.

[sustcstar]

This is too complex for me...Sorry.

[sustcstar]

It seems that the function "open a library" involves many classes and files without a top level file controlling them.

[calixtus]

There should be two entry points for this: JabRefGUI::openDatabases for autoloading in the startup process, and OpenDatabaseAction::openFiles for the menu command.
The basic idea would probably to move e.g. the OpenDatabaseAction::loadDatabase method into the BasePanel class, maybe called as a background process by a constructor, and bind the workDonePercentageProperty of the Task to some loading animation in the BasePanel...

[sustcstar]

Thanks for your suggestion, I will try it.

[Hollyqqqqq]

@sustcstar Hi, do you still work on this issue? My teamate and I want to try this issue if you are not doing it now. ^_^

[sustcstar]

Yeah, I have been doing this for two days.
If finally I give up, I will tell you.

[Hollyqqqqq]

OK, best wishes!

[sustcstar]

I give up on this issue, everyone please feel free to take it

[tmrd993]

Is anyone actively working on this? I'd like to work on it but I don't want to step on anyones toes.

[calixtus]

Feel free to take this issue.
Be aware that recently part of our architecture changed.
BasePanel is now Library tab.
Have fun coding and don't hesitate to ask for help in the gitter chat, if you don't understand something.

[HoussemNasri]

I'd like to work on this, @tmrd993 If you're still working on it tell me

[calixtus]

Hi @HoussemNasri , have fun coding. Maybe @tmrd993 can show what he already did and you both can join forces?
Don't forget to ask, if you encounter a problem or need help in our gitter jabref developer chat https://gitter.im/JabRef/jabref

[tmrd993]

@tmrd993 If you're still working on it tell me

No, you can go ahead and work on it.

I only skimmed over the files but I am pretty sure you need to look at

jabref/src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java

Line 39 in 1dc91ca

public class OpenDatabaseAction extends SimpleCommand {

This is the action that gets executed when the user loads a database from the menu.

When a database wasn't closed, the program opens it on startup from here

jabref/src/main/java/org/jabref/gui/JabRefGUI.java

Line 121 in 1dc91ca

private void openDatabases() {

[HoussemNasri]

For now, I am trying to deal with opening the database using action and these are the steps I am following

1. open a empty tab placeholder tab without starting the loading process yet
2. start loading the data on a background thread and with it a progress bar
3. when data loaded stop the progress bar and load data into the tab we opened close the placeholder tab and start the real tab

My current issue is the following, with the current LibraryTab, opening an empty tab is challenging because you need to pass a BibDatabaseContext at initialization and it can't be null, I can create a BibDatabaseContext with no data by just passing an empty list of entries to Database and pass the context to LibraryTab, but this will leave us with no information about the database we are trying to open, I tried to change the title of the tab using setText() but for some reason, it gets back to "untitled"
I created an instance of BibDatabaseContext then i setDatabasePath() and it worked

My current task is to refactor LibraryTab so it's easy to create an empty tab while keeping the information of the database we're trying to open(name, path...) and define a method inside LibraryTab of the name for example loadData() that will load a list of entries into the tab and update the UI
Refactoring LibraryTab wasn't an easy job as i thought, I tried refactoring it so i can create a tab then feed data later but there was some edge cases where the sidebar doesn't update and random crashes.

• Some Progress
  

• There was an issue when opening multiple databases simultaneously they will appear sorted by who loaded first, for example when you open database1 then database2, if database2 loaded before database1 you'll notice that database2 tab is before database1

• I fixed it with setting the position of each tab to their placeholder position, this way it doesn't matter when tab finishes loading it will reposition itself in the correct position.
  
  Next step is to apply the same behavior on startup.

[HoussemNasri]

Can anyone explain to me what this code is doing

jabref/src/main/java/org/jabref/gui/JabRefGUI.java

Lines 149 to 171 in 0412684

	if (pr.getDatabase().isShared()) {
	try {
	new SharedDatabaseUIManager(mainFrame).openSharedDatabaseFromParserResult(pr);
	} catch (SQLException | DatabaseNotSupportedException | InvalidDBMSConnectionPropertiesException |
	NotASharedDatabaseException e) {
	pr.getDatabaseContext().clearDatabasePath(); // do not open the original file
	pr.getDatabase().clearSharedDatabaseID();
	LOGGER.error("Connection error", e);
	mainFrame.getDialogService().showErrorDialogAndWait(
	Localization.lang("Connection error"),
	Localization.lang("A local copy will be opened."),
	e);
	}
	toOpenTab.add(pr);
	} else if (pr.toOpenTab()) {
	// things to be appended to an opened tab should be done after opening all tabs
	// add them to the list
	toOpenTab.add(pr);
	} else {
	mainFrame.addParserResult(pr, first);
	first = false;
	}

[calixtus]

This code should autoload the databases on startup.
First if block handles the shared databases (like Overleaf or some sql database), second if block handles, if someone uses the command line to merge some other database into the currently opened database, third block is the very simple autoload last opened files on startup.

[HoussemNasri]

@calixtus Thank you for responding, but I have another question, my code will close tabs automatically so to achieve this I copied this method from JabRefFrame and called it whenever I close a tab

jabref/src/main/java/org/jabref/gui/JabRefFrame.java

Lines 1157 to 1162 in b0af521

	private void removeTab(LibraryTab libraryTab) {
	DefaultTaskExecutor.runInJavaFXThread(() -> {
	libraryTab.cleanUp();
	tabbedPane.getTabs().remove(libraryTab);
	});
	}

The tab disappear indeed, The problem is when I close the app it keeps running in the background, After some investigation, I found out this was happening because AutosaveManager and BackupManager still attached to the tab so to solve this I need to call

AutosaveManager.shutdown(context);
BackupManager.shutdown(context);

My question is it ok if I added this snippet into the cleanup() method in LibraryTab

jabref/src/main/java/org/jabref/gui/LibraryTab.java

Lines 586 to 588 in b0af521

	public void cleanUp() {
	changeMonitor.ifPresent(DatabaseChangeMonitor::unregister);
	}

[calixtus]

Can you make a pull request with your changes so far? Then we can discuss this more easily. About autosave: @Siedlerchr is most experienced with this part of the code.

[Siedlerchr]

@HoussemNasri The autosave / tab shutdown closing seems plausible and should be done.A tab has an attach bibdatabasecontext so this is correct

[HoussemNasri]

I am trying a new approach by adding a new tab with 0 entries then when data is ready I call a method from LibraryTab to update BibdatabaseContext but the problem is that Groups Pane doesn't update and keep showing 0 entries although when I navigate away from the tab and come back it updates just fine.
This is the method in LibraryTab to update BibdatabaseContext

public void feedData(BibDatabaseContext bibDatabaseContext){
        cleanUp();

        this.bibDatabaseContext = Objects.requireNonNull(bibDatabaseContext);

        bibDatabaseContext.getDatabase().registerListener(this);
        bibDatabaseContext.getMetaData().registerListener(this);

        this.sidePaneManager = frame.getSidePaneManager();

        this.tableModel = new MainTableDataModel(getBibDatabaseContext(), preferencesService, Globals.stateManager);

        citationStyleCache = new CitationStyleCache(bibDatabaseContext);
        annotationCache = new FileAnnotationCache(bibDatabaseContext, preferencesService.getFilePreferences());

        setupMainPanel();
        setupAutoCompletion();

        this.getDatabase().registerListener(new SearchListener());
        this.getDatabase().registerListener(new EntriesRemovedListener());

        // ensure that at each addition of a new entry, the entry is added to the groups interface
        this.bibDatabaseContext.getDatabase().registerListener(new GroupTreeListener());
        // ensure that all entry changes mark the panel as changed
        this.bibDatabaseContext.getDatabase().registerListener(this);

        this.getDatabase().registerListener(new UpdateTimestampListener(preferencesService));

        this.entryEditor = new EntryEditor(this, externalFileTypes);

        Platform.runLater(() -> {
            EasyBind.subscribe(changedProperty, this::updateTabTitle);
            Globals.stateManager.getOpenDatabases().addListener((ListChangeListener<BibDatabaseContext>) c ->
                    updateTabTitle(changedProperty.getValue()));
        });

         //I took some code from add() method in JabRefFrame to reinitialize Backup and Autosave.
        if (readyForAutosave(bibDatabaseContext)) {
            AutosaveManager autosaver = AutosaveManager.start(bibDatabaseContext);
            autosaver.registerListener(new AutosaveUiManager(this));
        }

        BackupManager.start(this.bibDatabaseContext, Globals.entryTypesManager, Globals.prefs);

        trackOpenNewDatabase(this);

    }