Refactor cleanup code

[tobiasdiez]

This PR (again) refactors the cleanup department. The motivation was to extract as much logic as possible from the cleanup action to make it easier to use cleanup operations also at different points (like on save).

Biggest changes:

• Move preferences controlling the cleanup process to own data class CleanupPreset and the execution of the cleanup process to CleanupWorker. Thus starting a cleanup process is reduced to setting up a proper preset and passing it to the worker.
• Move code displaying and changing a preset to a separate class. Hence it is now easy to show and change a preset at a different place (like in the preferences).
• Move preferences code to JabRefPreferences. Well, this is the place where it belongs.
• Add tests for the CleanupWorker. More detailed tests of the different cleanup processes are still missing.

Todo's/Breaking changes: These are all related to the fact that there is no nice interface to get/set the file field of an entry (all the code is in the gui class FileListTableModel).

• The counting of unsuccessful renames does not work anymore (I vote for removing this completely)
• Remove CleanupUtil and incorperate the code in the respective cleanup classes. Currently this code relies on gui code (FileListTableModel) and thus cannot easily be moved to logic.
• Test the file related cleanups.

[tobiasdiez]

By the way, does somebody know why the tests fail on the ci server but pass locally? Is there a secretly hidden preference controlling whether the file type is added?

[tobiasdiez]

Any comments on this one? Should I rebase?

[simonharrer]

During the run method, NO swing class must be accessed. You use panels and option panes.

[tobiasdiez]

The method contains nearly the same code as the previous run method. The code somehow needs to get the selected cleanup actions. What would be the proper way to do this?

[simonharrer]

I would move some of the code in the init part and some of it in the update part. You can have a look at the SearchWorker, which separate this correctly. The issue is that the run method is always executed outside of the EDT, and the update method always on the EDT. And swing only works correctly if all methods on all swing classes are executed on the EDT.

This change may require a lot of work. If you think this is too much, we can leave it as is and improve that in another PR.

[tobiasdiez]

I'll have a look at it tomorrow.

[tobiasdiez]

The run method asks the user if he wants to continue with autogenerating pdf names. I'm not sure how to implement this properly with the Worker interface. Maybe the problem disappears automatically after migration to javafx?

[simonharrer]

• model.FileField.parse
• Preferences rework

[tobiasdiez]

I implemented everything we had discussed yesterday. In particular:

• Remove CleanupUtil class by moving its methods to appropriate cleanup classes (and use FileField instead of FileListTableModel)
• Reimplement counting of unsuccessful renames
• Add tests for rename files and make relative cleanups
• Rework cleanup preferences

However, I didn't make CleanupPreset stateless (i.e. it still contains setter) because I don't want to create a constructor accepting 19 booleans...Any idea how to proceed here?

[simonharrer]

Why not use a list of enums, each enum stands for a cleanup that is active aka true.

[tobiasdiez]

Because this would be too simple 😃 . Nice idea 👍

[simonharrer]

Awesome PR. Thanks for the contribution!