Uploaded image for project: 'IGB'
  1. IGB
  2. IGBF-3905

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

    Details

    • Type: Task
    • Status: In Progress (View Workflow)
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None

      Description

      The logic that is expected to remove a dataProvider whose defaultDataProviderId isn't present in the new igbDefaultPrefs.json needs to be corrected. In the code, the URL is the key, so even though we change the defaultDataProviderId to disable it, when we load the igbDefaultPrefs it just overwrites the old stored providerId in the user preferences, and again when we load the user preferences it overwrites the status of it. Also, we can't keep the defaultDataProviderId as a key as it is null for the custom user added providers. Have to investigate this and implement a simple way to make this work.

        Attachments

          Issue Links

            Activity

            Hide
            jsirigin Jaya Sravani Sirigineedi added a comment -

            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.

            Show
            jsirigin Jaya Sravani Sirigineedi added a comment - 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.

              People

              • Assignee:
                jsirigin Jaya Sravani Sirigineedi
                Reporter:
                jsirigin Jaya Sravani Sirigineedi
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated: