Change the implementation logic to disable the dataprovider when defaultDataProviderId is changed

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.

 //In DataProviderManager class
    public void initializeDataProvider(Preferences node) {
        boolean isEditable = node.getBoolean(IS_EDITABLE, true);
        String defaultDataProviderId = node.get(DEFAULT_PROVIDER_ID, null);
        // If it has a defaultDataProviderId, handle it as a default data provider,
        // otherwise, it must be a user-supplied data provider or a legacy default
        if (defaultDataProviderId != null) {
            // node DOES have a defaultDataProviderId
            // Is there a registered default data provider with that id?
            DataProvider dataProvider = getDataProviderById(defaultDataProviderId);
            if (dataProvider != null) {
                integrateUserPrefsToDefaultDataProvider(node, dataProvider);
            }// else - this is a depricated default, do not initiate it.
        } else {
            if (isEditable) {
                //User-supplied data provider: initiate it .
                //IMPORTANT NOTE: About field isEditable: It has nothing to do with the "Edit" option of a Data Provider.
                //It is not used in deciding if a Data Provider is editable or not.
                //Here, isEditable is going to tell us if a Data Provider is a default or user provided. This check is
                //STRICTLY for IGB versions 9.0.0 or before. It has been depricated from the defaults(JSON) since 9.0.1.
                initializeCustomDataProvider(node);
            } //else - this is a legacy default, do not initiate it.
        }

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.

Updated the code to disable the data provider when defaultDataProviderId is changed. Removed duplicate code and corrected some parts of code. Here is the branch: https://bitbucket.org/jaya-sravani/integrated-genome-browser/branch/IGBF-3905. Below are a few scenarios that should be tested as part of the ticket: (you can include any other test scenarios that you think might get affected by this code, especially if there is any other way to add a data provider, please test that too.)

Scenario-1:

1. Reset the preferences of your IGB before going forward.
2. Clone the above branch and build the jar.
3. Start IGB.
4. See whether the default data providers match the igbDefaultPrefs.json file or not. They should match.

Scenario-2: (Continuation)

1. Add a custom data provider.
2. Try disabling and enabling it and check whether it's working as expected.
3. Remember the current status of all the data providers and exit the application.
4. Change the igbDefaultPrefs.json file like this: add below variable for two data providers, but for one change the defaultDataProviderId to a different one (should be unique) and for the other keep it the same.

   "status": "disabled"
   

1. Start IGB.
2. See whether the first one, for which you changed the defaultDataProviderId is disabled now, which means the factory reset is working. The other one isn't disabled, which means user preferences are also considered. Lastly, the custom data provider also has the same status as before exiting the application.

Please review and let me know if there are any issues or need any help while testing it.

Testing whether new IGB branch allows setting status to "disabled" as noted in instructions:

• Reset IGB preferences to defaults
• Built the new branch IGBF-3905
• Observed data provider "Blueberry" status is not disabled, as per the IGB preferences
• Edited data provider "Blueberry" to be disabled upon startup as follows:

            {
                "factoryName": "quickload",
                "name": "Blueberry",
                "url": "http://lorainelab-quickload.scidas.org/blueberry",
                "loadPriority": "3",
                "mirror": "http://igbquickload.org/blueberry",
                "status": "disabled",
                "defaultDataProviderId": "IgbDefaultDataProviderId:3"
            },

• Rebuild IGB by running "mvn clean install"
• Ran IGB
• Opened Data Provider Preferences screen
• Observed that data provider "Blueberry" was not disabled, even though this new build of IGB's preferences file contained "status": "disabled" in its entry
• Unpacked the newly built jar file and confirmed that the preferences file contained the "status":"disabled" entry.

Because the data provider id has not changed, this means that the previous setting (enabled) persisted into this new build of IGB. This is because (I guess?) there is no way for IGB to tell if a data provider's enabled or disabled setting is coming from the user's preference or from a factory setting in a previous version of IGB.

However, when I reset IGB's preferences to defaults, the new setting (disabled) appeared to take effect.

I believe this is the expected behavior. Again, the only setting that a user can change for a data provider is the "status" setting. All other settings are not editable by the user. Therefore, a new version of IGB is unable to determine if the "status" setting (enabled or disabled) was set by the user or instead set by the previous IGB version's default preference for that data provider. It could be very confusing and possibly annoying if a new version of IGB changed a setting that its user deliberately set themselves, using the interface.

Testing what happens when I disable a factory-provided data provider and, at the same time, change its provider identifier, thus appearing to delete it from IGB.

Tested scenario 1 and 2 from Sravani's comment above.

I modified igbDefaultPrefs.json so that the RNA-Seq data provider had "status": "disabled" (status defaulted to enabled before) and I changed its "defaultDataProviderId": to a new number (18 from 2). When I started IGB the RNA-Seq data provider was disabled.

Thank you Nowlan Freese!

Ready for pull request.

cc: Jaya Sravani Sirigineedi

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.