[jbosstools-issues] [JBoss JIRA] (JBIDE-15766) openshift-java-client: dont refresh env variables on each addition/removal

[Martes Wigglesworth (JIRA)]

    [ https://issues.jboss.org/browse/JBIDE-15766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12824498#comment-12824498 ] 

Martes Wigglesworth edited comment on JBIDE-15766 at 10/24/13 12:38 AM:
------------------------------------------------------------------------

Use Cases:

Add
1) add non-existing variable
2) add pre-existing variable

Remove
1) remove pre-existing variable
2) remove non-pre-existing variable

Solution analysis:
I. Simply remove the update operations from both the add and remove methods listed.
II. Remove update from remove, and leave it in add
III. Leave update in both aspects due to massive rework requirement.
IV. Alter the current behaviours of the add and remove operations as follows:
a) The update should be removed from add and remove because nothing is actually done with the updated list, other than to make sure that   the list is updated.  
   i)In the case that the add operation is successful, then your local variable list will simply reflect the remote list. The add method should then return boolean "true" based on the outcome of the operation.
   ii)In the case that the add operation is not successful, then the add operation should capture the exception and return a boolean indication. The add method should then return boolean "false" based on the outcome of the operation.
   iii)In the case for the remove operation, if the removal is successful, the list should already reflect the external list, and does not need to be updated.  The remove method should then return boolean "true" based on the outcome of the operation.
   iv) In the case for the remove operation that is not successful, a boolean of false should be rendered, and the list would still reflect the removed variable anyhow.  The remove method should then return boolean "false" based on the outcome of the operation.
   v) In all of the cases listed above, a side-effect update to the list is not really needed to complete the operations on the incident variable list, hence it should be removed in this solution option as well.

b) There may be a possibility of a race condition in 2.0, according to Jay, so we will need to build some further robustness into these operations.  

I propose that we change the add and remove operations to be dependent upon exceptions being generated for operations such as add, remove/destroy, such that indicators for the results on remove and add operations are returned to the user of the plugin. 

This may be further than the scope of this issue, however, I am not sure.  During initial testing for this functionality in the destroy() integration tests, I could not generate an exception based on reuse of an instance of IEnvironmentVariable retrieved from the IApplication instance, and using IEnvironmentVariable.destroy() twice.  No exception is generated when you call the operation.  

A lack of understanding of how the destroy() method is supposed to work may have been the reason that I was unable to generate the assumed to be available SSHOperationException.  

However, are the service objects supposed to generate an exception when you attempt to remove a variable that does not exist? (Answer: It currently does not due to the flow of the application processing.)
                
      was (Author: mwigglesworth-redhat):
    Use Cases:

Add
1) add non-existing variable
2) add pre-existing variable

Remove
1) remove pre-existing variable
2) remove non-pre-existing variable

Solution analysis:
I. Simply remove the update operations from both the add and remove methods listed.
II. Remove update from remove, and leave it in add
III. Alter the current behaviours of the add and remove operations as follows:
a) The update should be removed from add and remove because nothing is actually done with the updated list, other than to make sure that   the list is updated.  
   i)In the case that the add operation is successful, then your local variable list will simply reflect the remote list. The add method should then return boolean "true" based on the outcome of the operation.
   ii)In the case that the add operation is not successful, then the add operation should capture the exception and return a boolean indication. The add method should then return boolean "false" based on the outcome of the operation.
   iii)In the case for the remove operation, if the removal is successful, the list should already reflect the external list, and does not need to be updated.  The remove method should then return boolean "true" based on the outcome of the operation.
   iv) In the case for the remove operation that is not successful, a boolean of false should be rendered, and the list would still reflect the removed variable anyhow.  The remove method should then return boolean "false" based on the outcome of the operation.
   v) In all of the cases listed above, a side-effect update to the list is not really needed to complete the operations on the incident variable list, hence it should be removed in this solution option as well.

b) There may be a possibility of a race condition in 2.0, according to Jay, so we will need to build some further robustness into these operations.  

I propose that we change the add and remove operations to be dependent upon exceptions being generated for operations such as add, remove/destroy, such that indicators for the results on remove and add operations are returned to the user of the plugin. 

This may be further than the scope of this issue, however, I am not sure.  During initial testing for this functionality in the destroy() integration tests, I could not generate an exception based on reuse of an instance of IEnvironmentVariable retrieved from the IApplication instance, and using IEnvironmentVariable.destroy() twice.  No exception is generated when you call the operation.  

A lack of understanding of how the destroy() method is supposed to work may have been the reason that I was unable to generate the assumed to be available SSHOperationException.  

However, are the service objects supposed to generate an exception when you attempt to remove a variable that does not exist? 
                  
> openshift-java-client: dont refresh env variables on each addition/removal
> --------------------------------------------------------------------------
>
>                 Key: JBIDE-15766
>                 URL: https://issues.jboss.org/browse/JBIDE-15766
>             Project: Tools (JBoss Tools)
>          Issue Type: Bug
>          Components: openshift
>    Affects Versions: 4.1.1.Beta1, 4.2.0.Alpha1
>            Reporter: Andre Dietisheim
>            Assignee: Andre Dietisheim
>             Fix For: 4.1.1.Beta1, 4.2.0.Alpha1
>
>
> When adding/removing environment variables, the openshift-java-client would always request the backend for the full list. This should not be required and avoided
> {code:title=ApplicationResource#addEnvironmentVariable}
> 		EnvironmentVariableResourceDTO environmentVariableResourceDTO =
> 				new AddEnvironmentVariableRequest().execute(name, value);
> 		IEnvironmentVariable environmentVariable = new EnvironmentVariableResource(environmentVariableResourceDTO, this);
> 		updateEnvironmentVariables();
> 		return environmentVariable;
> {code}
> {code:title=ApplicationResource#updateEnvironmentVariables}
> 	protected void updateEnvironmentVariables() throws OpenShiftException {
> 		if (environmentVariableByName == null) {
> 			environmentVariableByName = loadEnvironmentVariables();
> 		} else {
> 			environmentVariableByName.clear();
> 			environmentVariableByName.putAll(loadEnvironmentVariables());
> 		}
> 	}
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira