Fix for issue 6877: Allow users to customize the API Key

[ruanych]

Fixes #6877

• Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
• Tests created for changes (if applicable)
• Manually tested changed features in running JabRef (always required)
• Screenshots added in PR description (for UI changes)
• Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Changed:

1. Add feature: allows the user to customize the Springer API Key in the customization tab of the preferences and check this key.
2. Update API_URL's protocol of SpringerFetcher to HTTPS.

Initial default value, do not use custom API key:

Check success with a valid API key:

Check failed with an invalid API key:

Search using a custom Springer API key:

[ruanych]

@calixtus Now I have some difficulties with the preferences, can you give me some help?

I don't know how to get the content of preferences in SpringerFetcher. org.jabref.gui.Globals is deprecated, org.jabref.preferences.JabRefPreferences.getInstance() is also deprecated, but I have not found a way to replace it.

My current idea is to use the constructor to pass in org.jabref.preferences.PreferencesService to construct org.jabref.logic.importer.fetcher.SpringerFetcher, which also means that org.jabref.logic.importer.WebFetchers#getSearchBasedFetchers needs PreferencesService, but its parameter is only ImportFormatPreferences and it is dependent on multiple places.

How should I find the singleton of org.jabref.preferences.JabRefPreferences(The only implementation class of PreferencesService)?
Should I change the org.jabref.logic.importer.WebFetchers#getSearchBasedFetchers's parameter list or add a member attribute in WebFetchers?

[Siedlerchr]

Overall looks good to me, I just think the prefs can be made more generic to hold other custom api keys as well

[Siedlerchr]

You can simply add the CustomApiKeyPreferences to the ImportFormatPreferences, then you can acess it from the importFormatPreferences and don't need to pass the whole PreferencesService, see:

jabref/src/main/java/org/jabref/logic/importer/ImportFormatPreferences.java

Lines 21 to 31 in bb011c9

	public ImportFormatPreferences(Set<CustomImporter> customImportList, Charset encoding, Character keywordSeparator,
	CitationKeyPatternPreferences citationKeyPatternPreferences,
	FieldContentFormatterPreferences fieldContentFormatterPreferences, XmpPreferences xmpPreferences, boolean keywordSyncEnabled) {
	this.customImportList = customImportList;
	this.encoding = encoding;
	this.keywordSeparator = keywordSeparator;
	this.citationKeyPatternPreferences = citationKeyPatternPreferences;
	this.fieldContentFormatterPreferences = fieldContentFormatterPreferences;
	this.xmpPreferences = xmpPreferences;
	this.keywordSyncEnabled = keywordSyncEnabled;
	}

[calixtus]

Yes, PreferencesService should be injected in the constructor (= passed to the constructor).
See WebFetchers::getSearchBasedFetchers for examples.

One thing I believe that could be an issue: Currently only a created ImportFormatPreferences object is passed through. But the list of the WebFetchers is created when the GUI starts. So either the list of WebFetchers must be recreated each time the preferences have changed or (what I like way more) you refactor the WebFetchers class to use just the PreferencesService object which should be passed through to those fetchers that needs them.

[calixtus]

Do not use ImportFormatPreferences to pass the API keys as this class should be disbanded soon. You can mark it as deprecated if you want.

[ruanych]

Do not use ImportFormatPreferences to pass the API keys as this class should be disbanded soon. You can mark it as deprecated if you want.

Perhaps ImportFormatPreferences can be turned into CustomPreferences so that it can carry more user custom content without affecting the original dependencies.

[ruanych]

The new version of the code allows users to customize more API Keys. Currently, Springer and IEEE are added.

Developers can easily add more options here:
https://github.com/Ryyyc/jabref/blob/91cbb1b99492eb6ef2547362bd179f4f00b85693/src/main/java/org/jabref/gui/preferences/customization/CustomizationTabViewModel.java#L53-L57

Just simply add the name and test URL(remove the value of API key from query parameters) corresponding to the API Key.

[calixtus]

Do not use ImportFormatPreferences to pass the API keys as this class should be disbanded soon. You can mark it as deprecated if you want.

Perhaps ImportFormatPreferences can be turned into CustomPreferences so that it can carry more user custom content without affecting the original dependencies.

As for now ImportFormatPreferences is not a real Preferences object, since it simply collects other preferences objects and works as rucksack, frozen in time of creation. See JabRefPreferences::getImportGormatPreferences.

What should instead happen is to create a preferences object like ApiKeyPreferences that includes all the ApiKeys and related preferences, that is being constructed by JabRefPreferences and is being stored in JabRefPreferences.

[calixtus]

I like your approach to construct a CustomApiKeyPreferences object for each key. I would just ask to create the list of the available keys not in the viewmodel of the preferences tab, but maybe to return it out of a static method in CustomApiKeyPreferences, maybe called getAvailableKeys, - or - to introduce a new method in the fetcher classes that returns if this fetcher uses a customizable API key and how it is called.

[tobiasdiez]

I think it would also make sense to rename the "customization" tab to "Web search" (Fetcher is our internal notion) since everything in the preferences is about customization.

[ruanych]

How about CustomApiKeyPreferences ( rename to ApiKeyPreferences) designed as a class that manages custom API keys?

public class ApiKeyItem {
    private final String name;
    private boolean useCustom;
    private String customApiKey;
    private String defaultApiKey;
    private String testUrlWithoutApiKey;

    // ...
}

public class ApiKeyPreferences {
    private final PreferencesService preferences;
    private final TreeMap<String, ApiKeyItem> apiKeyItems;

    // It is created in JabRefPreferences, and the JabRefPreferences object is passed to the constructor
    // In this way it can use some of the services provided by JabRefPreferences,
    //    at the same time proxy some of the work of JabRefPreferences
    public CustomApiKeyPreferences(PreferencesService preferences) {
          this.preferences = preferences;
    }

    public static void registerApiKey(String name, String defaultApiKey, String testUrlWithoutApiKey) {
              // Only useCustom and customApiKey attributes are valid 
	      ApiKeyItem item = preferences.getCustomApiKeyPreferences(name); 
	      item.setDefaultApiKey(defaultApiKey);
	      item.setTestUrlWithoutApiKey(testUrlWithoutApiKey);
	      apiKeyItems.put(name, item);
    }

    public static String getApiKey(String name) {
	      preferences.getCustomApiKeyPreferences(name);
	      ApiKeyItem item = apiKeyItems.get(name);
	      String apiKey = "";
	      if (item!=null) {
	          if (item.useCustom) {
	              apiKey = item.getCustomApiKey();
	          } else {
	              apiKey = item.getDefaultApiKey();
	          }
	      }
	      return apiKey;
    }

    // Usually CustomizationTabViewModel uses it
    public static void updateApiKeyItem(ApiKeyItem item){
	      apiKeyItems.put(item.getName(), item);
	      preferences.storeCustomApiKeyPreferences(item);
    }

    // Usually CustomizationTabViewModel uses it
    public static ArrayList<ApiKeyItem> getAvailableApiKey() {
              return (ArrayList<String>) apiKeyItems.values();
    }
}

This design is more compatible with the previous model.
When constructing Fetcher, according to needs choose whether to provide users with customizable API key services.

In addition, I also noticed that org.jabref.logic.util.BuildInfo obtains some API keys from environment variables. Perhaps we can discuss a more complete solution.

[calixtus]

Some API keys are deliberately stored in the GitHub secrets to hide them from public access, since we use private keys just for the ci tests, that should work, even when the public keys are locked out.

[ruanych]

If the design of ApiKeyItem and ApiKeyPreferences is feasible, I will start to modify and optimize the code.

[calixtus]

Sorry had no time yet to look into detail, will comment tonight.

[calixtus]

Just on a first look: why do you want to introduce this class ApiKeyPreferences? This seems to be way more complicated, than it has to be. It looks like you just put another layer around the keys. Just put the get and update and store methods in JabRefPreferences. If you have to, put the API keys into cache on lazy loading.

[ruanych]

Just on a first look: why do you want to introduce this class ApiKeyPreferences? This seems to be way more complicated, than it has to be. It looks like you just put another layer around the keys. Just put the get and update and store methods in JabRefPreferences. If you have to, put the API keys into cache on lazy loading.

This makes the logic of management API Key more centralized.

I like your approach to construct a CustomApiKeyPreferences object for each key. I would just ask to create the list of the available keys not in the viewmodel of the preferences tab, but maybe to return it out of a static method in CustomApiKeyPreferences, maybe called getAvailableKeys, - or - to introduce a new method in the fetcher classes that returns if this fetcher uses a customizable API key and how it is called.

Now the custom API Key is only loaded when the preference tab dialog is opened (CustomizationTabViewModel#setValues()), and cannot be accessed directly through a static method.

If ApiKeyPreferences is not needed, compare initApiKeyNameUrl and registerApiKey in CustomizationTabViewModel.

private void initApiKeyNameUrl() {
    // Springer query using the parameter 'q=doi:10.1007/s11276-008-0131-4s=1' will respond faster
    apiKeyNameUrl.put("Springer", "https://api.springernature.com/meta/v1/json?q=doi:10.1007/s11276-008-0131-4s=1&p=1&api_key=");
    apiKeyNameUrl.put("IEEEXplore", "https://ieeexploreapi.ieee.org/api/v1/search/articles?max_records=0&apikey=");
}

public static void registerApiKey(String name, String testUrlWithoutApiKey) {
    apiKeyNameUrl.put(name, testUrlWithoutApiKey);
}

.

[Siedlerchr]

Not sure how, but I think this should take the custom SSL store into account

[calixtus]

I am not completely sure, but I strongly believe that SSL is handled by URLDownload or in the DefaultSSlSocketFactory internally. At least I have no place found in our codebase that handles the custom SSL store directly besides the custom SSL preferences. Also the pr that introduced custom SSL certificates did not introduce any custom SSL handling anywhere, but just removed the infamous bypassSSLVerification.

[koppor]

@ryyyc Thank you for your effort. We took the liberty to finalize the PR and include it so that it is available on our next release.