Create new fetcher infrastructure

[tobiasdiez]

The aim of this PR is to refactor the fetcher interface, in particular separate logic and GUI code as well as implement #383 and #550.

I propose the following class hierarchy for fetchers:

• WebFetcher: search web resources for matching BibEntries
  • SearchBasedFetcher: based on a free-text query (may return multiple results)
  • IdBasedFetcher: based on an ID (single item as result)
• IdFetcher: looks for an ID (arXiv, DOI,...) based on an existing BibEntry
• FulltextFetcher: find fulltext of an existing BibEntry (i.e. rebranding of FullTextFinder)

• Change in CHANGELOG.md described
• Tests created for changes
• Screenshots added (for bigger UI changes)

[oscargus]

Are these classes logic or gui? What is the complete work flow? ImportInspector? Merging? (Adding empty fields may not be enough.)

[tobiasdiez]

They are logic and try to extract the non-gui part from the current EntryFetcher. Thus the workflow is not changed (at least not with this PR).

[matthiasgeiger]

Is generally fine for me.

One remark: There are currently two different workflows for the "SearchBasedFetchers".
Normally one hits the "Search" button after inserting some search query and a list of found entries is presented, after selecting the desired entries they are imported.
However, at least for the GoogleScholar fetcher there is another dialog presenting more details to "preselect" the entries which should be shown in the list of found entries (I think the intention is to reduce the amount of queries).

Is the proposed "shallow" search method used in this case? Or is this difference in the workflow currently not really covered, yet?

[koppor]

I think the workflow is supported by the two methods performShallowSearch and performDeepSearch. I assume that the dialog is presented if performDeepSearch is present. I think that one needs an additional class having only performShallowSearch and a second one containing both methods.

My concern is that the intermediate results might not be available as bibtex entries, but as HTML only. I think, the ImportInspectionDialog was used for that, wasn't it.

What about error messages? I assume the aim is to use LOGGER only and to use the error console? I used to like the GUI dialogs showing errors, but OK for me to remove them to improve the code. (Until now, this was done by the OutputPrinter status, wasn't

@zellerdev will try to port GVKFetcher and the ISBNToBibTeX fetcher to the new infrastructure. He will do a PR on your repo, is that OK, @tobiasdiez?

[Siedlerchr]

For the FulltextFetcher it would be awesome if the FullText finding could be run as part of a Cleanup operation, e.g. globally finding fulltext for all entries.
This should be kept in mind

[tobiasdiez]

Thanks for the feedback.
@matthiasgeiger, @koppor yes I thought about covering the two-stage search by the deep and shallow methods. Maybe this is not the best idea and one needs a different approach for the Google search. Thus for now I would concentrate on the simple fetchers which get all the metadata in one query.

@koppor The result from the server is of course HTML (or XML, JSON,...). It is the task of the fetcher to translate the response to something meaningful, i.e. to BibEntries. I expanded the arXiv fetcher to outline how this works (in fact the arXiv fetcher now understands free text queries and not just ids). I also included a way to notify the caller about errors...by, well, throwing exceptions (with meaningful error messages). Maybe the fetchers also need to post some events to let the GUI know about the progress.

@Siedlerchr Good idea. However a bit out of the scope of this PR. Could you file a new issue for it? Thanks!

So the plan for this PR is now to use the new interfaces parallel to the old code (so that I don't have to rewrite every fetcher).

[tobiasdiez]

So PR is ready for review.
The new fetcher can now be included via a wrapper. In this way the fetcher can be transformed to the new interface step-by-step without the need to refactor everything at once.

If I get the ok for merge, then I (or @zellerdev) will move HelpFiles to logic and thereby fix the failing architecture test.

[oscargus]

Many of the current fetchers will only fetch a limited number of entries. Typically based on user input. How is that going to be handled here?

[oscargus]

gui?

[stefan-kolb]

Whereas I think the interface redesign is a good thing, I'd like to throw a few things for discussion.

1. The interface hierarchy is too complex in my opinion why not just use:
   • WebFetcher: Has two methods for SearchBasedFetcher & IdBasedFetcher
   • IdFetcher
   • FulltextFetcher

The web fetcher hierarchy seems artificial to me. Why do we ultimately need one method interfaces?

1. I really don't like the effect that we merge all fetchers into one class, e.g. your ArxiV.java class.
   Some fetcher are complex enough on their own so why create new 1000 lines classes being capable of searching, id fetching and full text retrieval. Let us just use 3 classes for the three types of fetching.

[stefan-kolb]

On another note: I don't think that HelpFile.java belongs into the logic package.

[tobiasdiez]

@oscargus Right now every parameter has to be passed to the fetcher via the constructor. Maybe it would make sense to include the number of results as a method parameter. Right now I don't have a feeling on how many fetcher actually support this (or if a common fixed value of say 30 would also work).

@stefan-kolb The idea to distinct between ID-based and text-based fetchers came from #383 and #550. Right now the interfaces only have one method but this might change in the future (for example, a method to check validity of a given ID would make sense for the ID-based fetcher).
If a fetcher is complex enough, then one of course still decompose the functionality in different classes. For the arXiv fetcher it felt like all the functionality is essentially given by one method (callApi) and the other methods are just different wrappers around it.
Do you have a good idea how to handle for the help files in fetchers (logic) if HelpFiles stays in gui?

[stefan-kolb]

I still don't get why these two types of fetcher methods cannot live in one interface.
Maybe this is more of a problem of our implementations than of the architecture abstraction?
I haven't checked the usage of the HelpFiles yet, tho I think this has to be managed by the GUI somehow.

[Siedlerchr]

The helpfiles could be under the logic,, as they do not contain any gui related methods or any other related thing.
They just serve as a kind of Container class, they simply store the name of the HelpPage-Markdown file in an Enum.
The assembling of the url and opening is in the gui in HelpAction.

[oscargus]

I think a two-stage search is preferred in many situations. All is not really an option and sometimes one would really like to go for more than 100 (which is a more reasonable number). Personally, I often use JabRef as a search engine to find interesting papers (or rather, I search for a specific paper and then find related ones so it makes sense to not have a too narrow query, also some publishers has a logic or on the search terms so you get more hits the more terms you add). Just try searching Springer or DOAJ for some common term, "efficient" or "Smith" or something. I think this can be solved though by adding another call so query -> number of hits -> number to fetch -> list of entries.

[matthiasgeiger]

Thanks Oscar for bringing up this aspect.

I think the main reason for introducing this step was to reduce the number of requests fired to the different providers as some of them are rather quick with blocking IPs crawling to much information in a short time.
Thus the number of matches to be shown is not only dependent on the intention of the user (find a single paper vs. search for all possible papers matching the search) but also might have technical reasons...
However, personally I don't like the workflow "Enter term" > "Start search" > "Determine number of hits and ask user how much hits should be displayed" > "List chosen number of hits"
I think "Enter term" > "Start search" >"List first X hits" + "Label: Showing first X of Y hits" + Button "get next X hits" would be better... but this would require some more methods in the Interfaces to start fetching from/to a given offset...

[stefan-kolb]

@simonharrer and @matthiasgeiger have persuaded me that I'm dumb, please merge as intended. Maybe the naming can be improved a little bit but this is NP-hard.

[oscargus]

OK, so how should blocking be avoided? I agree that @matthiasgeiger's approach is preferred, but there must be some support for fetching an arbitrary number of items.