Here is a detailed explanation of the assumed role of the defaultDataProviderId (both in code and requirement) and how it is working:
Below is the part of the code that should be responsible for the expected behavior of defaultDataProviderId, i.e., If we change the defaultDataProviderId of a default data provider in the igbDefaultPrefs.json file and change any of its other variables (like status, load priority; except URL) it should also change in the preferences, why - as we changed the id it should be treated as a new data provider and not the old one and shouldn't override them with the user preferences but create a new preference node for this new provider, similarly if we didn't change the id and made any changes to any of the mentioned variables, it will override the variables with user preferences as it is an old one and the user had kept some preferences for it we have to obey them.
public void initializeDataProvider(Preferences node) {
boolean isEditable = node.getBoolean(IS_EDITABLE, true);
String defaultDataProviderId = node.get(DEFAULT_PROVIDER_ID, null);
if (defaultDataProviderId != null) {
DataProvider dataProvider = getDataProviderById(defaultDataProviderId);
if (dataProvider != null) {
integrateUserPrefsToDefaultDataProvider(node, dataProvider);
} } else {
if (isEditable) {
initializeCustomDataProvider(node);
} }
The code logic is correct here, but the way each preference is stored, this code won't work as expected. The preferences node is stored by keeping the hash of the URL as the key and all the variables in a map, like below:
<node name="dataProviders">
<map/>
<node name="15168ebbd4885f6b911ebae5aacfa59a"> //this is the hash of the URL and not the id.
<map>
<entry key="status" value="Initialized"/>
<entry key="defaultDataProviderId" value="IgbDefaultDataProviderId:7"/>
<entry key="mirrorUrl" value="http://igbquickload.org/chipseq/"/>
<entry key="isEditable" value="false"/>
<entry key="loadPriority" value="1"/>
<entry key="factoryName" value="Quickload"/>
<entry key="name" value="ChIP-Seq"/>
<entry key="url" value="http://lorainelab-quickload.scidas.org/chipseq/"/>
</map>
</node>
</node>
Here is the code that stores the preferences nodes:
The get itself creates a preference node if it's not present before. This is the place where each default data provider gets initialized.
public BaseDataProvider(String url, String name, int loadPriority, String id) {
this.url = checkNotNull(url);
this.name = checkNotNull(name);
this.loadPriority = loadPriority;
this.defaultDataProviderId = id;
encrypter = new StringEncrypter(StringEncrypter.DESEDE_ENCRYPTION_SCHEME);
preferencesNode = PreferenceUtils.getDataProviderNode(url);
loadPersistedConfiguration();
initializePreferences();
}
and in the loadPersistedConfiguration() method it checks for already stored variable values and if they are present, it overrides them with these as the key for the preference nodes is the hash of the URL even if we change the defaultDataProviderId, it's gonna find values for previous preferences unless we change the URL. That's why it worked when we changed the URL in before scenarios.
Also, not sure why we are checking all the variables in the stored preferences as we don't allow users to change anything except load priority and status.
private void loadPersistedConfiguration() {
Optional.ofNullable(preferencesNode.get(PROVIDER_NAME, null)).ifPresent(preferenceValue -> name = preferenceValue);
Optional.ofNullable(preferencesNode.get(LOAD_PRIORITY, null)).ifPresent(preferenceValue -> loadPriority = Integer.parseInt(preferenceValue));
Optional.ofNullable(preferencesNode.get(MIRROR_URL, null)).ifPresent(preferenceValue -> mirrorUrl = preferenceValue);
Optional.ofNullable(preferencesNode.get(LOGIN, null)).ifPresent(preferenceValue -> {
if (!Strings.isNullOrEmpty(preferenceValue)) {
login = preferenceValue;
}
});
Optional.ofNullable(preferencesNode.get(PASSWORD, null)).ifPresent(preferenceValue -> {
if (!Strings.isNullOrEmpty(preferenceValue)) {
password = encrypter.decrypt(preferenceValue);
}
});
String value = preferencesNode.get(STATUS, null);
Optional.ofNullable(value).ifPresent(preferenceValue -> {
ResourceStatus.fromName(preferenceValue).ifPresent(matchingStatus -> status = matchingStatus);
if (status == Initialized) {
status = NotInitialized;
}
});
}
In order to resolve this, we have to change how preference nodes are being created. We can't use defaultDataProviderId for all, as the custom data provider won't have them. So, we can follow this approach: use defaultDataProviderId as the key for the default data providers and URL as the key for custom data providers. Tested this approach by changing this part of the code for Ensembl alone and it worked.
Changed the name of the provider from "Ensembl REST" to "Ensembl" and the defaultDataProviderId of Ensembl
Before code change:
user preferences node overrides it even though the id is changed.
After code change:
user preferences node doesn't override it as the id is changed and it is being used SSD the key for the preferences node, so it didn't find any previously stored preferences for that key in the user preferences.
Ann Loraine and Nowlan Freese Please review this and let me know your opinions. We can schedule a call to discuss this as well.
Tested the scenario discussed with Nowlan Freese: The scenario is to check what happens if we give a DataProvider with an existing dataProviderId of an old DataProvider that's only present in user_preferncecs and not in igbDefaultPrefs anymore. For example, I have two data providers, A and B, A with a defaultDataProviderId of 2 and B with 3. First, we changed the defaultDataProviderId of A to 4, status to disabled in igbDefaultPrefs, and deployed it. Users will now have two preference nodes for the DataProvider A one with the defaultDataProviderId of 2, status not disabled, and another with 4, status disabled but IGB loads only the disabled one and considers the previous one as a legacy data provider and doesn't initiate it. Later, if we change the defaultDataProviderId of B to 2, status to disabled in igbDefaultPrefs, and deploy it, it won't be disabled as we will be integrating the legacy data provider stored in the user preferences to this and it's not disabled.
To avoid this, we have to keep track of the defaultDataProviderId and shouldn't reuse the ids that were used before, we can simply keep on incrementing it. Also, while going through the code again, I remembered considering the option that we discussed Nowlan Freese, where we don't use the defaultDataProviderId at all and use a boolean flag like integrateUSerPrefs (setting it to true or false based on our scenarios) but there is some complexity around it too, which I can discuss with you. For now, I feel this approach is good enough if we don't reuse the defaultDataProviderId again.
Moving this ticket back to review and will move it accordingly based on our next discussion.