Skip to content

Commit 1340576

Browse files
author
Martes G Wigglesworth
committed
[JBIDE-15766]:Remove refresh of all IEnvironmentVariable on add/remove actions
Please reference the following change log: Removed update calls from EnvironmentVariableResource.destroy, ApplicationResource.addEnvironmentVariable,ApplicationResource.addEnvironmentVariables,and ApplicationResource.removeEnvironmentVariable. Added ApplicationResource.removeEnvironmentVariable(IEnvironmentVariable) Renamed environmentVariableByName to environmentVariableMap Updated getOrLoadEnvironmentVariable to work check for emptiness of environmentVariableMap Updated loadEnvironmentVariables to work on environmentVariablesMap, and return it, in contrast to a locally-scoped version that did not persist. Corrected test failures due to omitted changes to instances methods for EnvironmentVariableResouce and ApplicationResource. Updated remove methods to use remove..(IEnvironmentVariable) and ApplicationResourceIntegrationTest to use Application.removeEnvironmentVariable(IEnvironmentVariable) in testing. Added integration test for removeEnvironmentVariable(IEnvironmentVariable) Updated ApplicationResource.addEnvironmentVariables(Map<String,String>) to filter the incoming map key/value pairs if they are duplicate keys, without different values. This update should resolve unecessary update DTO requests being generated. (Rebased up to Nov 12 since the original PR got behind.) Corrected possible logical error that was accidentally included in addEnvinronmentVariables(Map<String,String>), with the original PR. [if(testVar!=null)...else....] (Rebased again, to support merge with most recent version of upstream master: 13-NOV-2013) Corrected ApplicationResource.removeEnvironmentVariableByInstance(IEnvironmentVariable) and ApplicationResource.removeEnvironmentVariable(String) to correct failing test(s).
1 parent 072de97 commit 1340576

7 files changed

Lines changed: 202 additions & 126 deletions

File tree

‎src/main/java/com/openshift/client/IApplication.java‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
/**
2727
* @author André Dietisheim
2828
* @author Syed Iqbal
29+
* @author Martes G Wigglesworth
2930
*/
3031
public interface IApplication extends IOpenShiftResource {
3132

@@ -423,38 +424,34 @@ public IEmbeddedCartridge getEmbeddedCartridge(IEmbeddableCartridge cartridge)
423424
/**
424425
* Retrieves the map of environment variables
425426
*
426-
* @return the list of environment variables
427+
* @return the Map<String,IEnvironmentVariable> of environment variables
427428
* @throws OpenShiftSSHOperationException
428429
*/
429430
public Map<String, IEnvironmentVariable> getEnvironmentVariables() throws OpenShiftSSHOperationException;
430431

431432
/**
432433
* Checks if the environment variable is present in the application.
433434
*
434-
* @param name
435-
* Name of the environment variable
436-
* @return
435+
* @param name Name of the environment variable
436+
* @return <code>true</code> if the current instance has IEnvironmentVariables to return <br>
437+
* <code>false</code> if the current instance has no IEnvironmentVariables to return
437438
* @throws OpenShiftSSHOperationException
438439
*/
439440
public boolean hasEnvironmentVariable(String name) throws OpenShiftException;
440441

441442
/**
442-
* Adds an environment variable to this application. If the environment
443-
* variable exists already, then an OpenShift exception is thrown.
443+
* Adds an environment variable to this application.
444444
*
445-
* @param name
446-
* name of the variable to add
447-
* @param value
448-
* value of the new variable
449-
* @throws OpenShiftSSHOperationException
445+
* @param name key associated with the variable to add
446+
* @param value value of the new variable
447+
* @throws OpenShiftSSHOperationException - if the variable already exists
450448
*/
451449
public IEnvironmentVariable addEnvironmentVariable(String name, String value) throws OpenShiftException;
452450

453451
/**
454452
* Adds a map of environment variables to the application
455453
*
456-
* @param environmentVariables
457-
* map of environment variables
454+
* @param environmentVariables Map<String,String> of environment variables
458455
* @throws OpenShiftSSHOperationException
459456
*/
460457
public Map<String, IEnvironmentVariable> addEnvironmentVariables(Map<String, String> environmentVariables)
@@ -463,42 +460,47 @@ public Map<String, IEnvironmentVariable> addEnvironmentVariables(Map<String, Str
463460
/**
464461
* Return the environment variable for the specified name
465462
*
466-
* @param name
467-
* Name of the environment variable
468-
* @return environment variable
469-
* @throws OpenShiftSSHOperationException
463+
* @param name key to be used to locate the environment variable
464+
* @return IEnvironmentVariable associated with the provided key
465+
* @throws OpenShiftSSHOperationException - thrown if the key does not exist
470466
*/
471467
public IEnvironmentVariable getEnvironmentVariable(String name) throws OpenShiftException;
472468

473469
/**
474470
* Removes the environment variables with the given name from this application.
475471
*
476-
* @param name
477-
* @return
478-
* @throws OpenShiftException
472+
* @param name key associated with the IEnvironmentVariable to be removed
473+
* @return
474+
* @throws OpenShiftException - thrown if there is no IEnvironmentVariable associated with the explicit parameter.
479475
*/
480476
public void removeEnvironmentVariable(String name) throws OpenShiftException;
477+
478+
/**
479+
* Removes the environment variables with the given name from this application.
480+
*
481+
* @param environmentVariable IEnvironmentVariable instance which should be removed
482+
* @return
483+
* @throws OpenShiftException - thrown if there is no instance of IEnvironmentVariable available to be removed
484+
*/
485+
public void removeEnvironmentVariable(IEnvironmentVariable environmentVariable);
481486

482487
/**
483-
* Returns <code>true</code> if this application can list its environment
484-
* variables. Returns <code>false</code> if it cant. Internally this
485-
* translates to the presence of the link to list environment variables.
488+
* Used to determine if environment variables exist and are available to be retrieved
486489
*
487-
* @return true if this application can list its envirnoment variables
490+
* @return Returns <code>true</code> if this application can list its environment variables. <br>
491+
* Returns <code>false</code> if it cannot. Internally this translates to the presence of the link to list environment variables.
488492
*
489-
* @see #getEnvironmentVariables()
493+
* @see #getEnvironmentVariablesMap()
490494
* @see #getEnvironmentVariable(String)
491495
* @see ApplicationResource#LINK_LIST_ENVIRONMENT_VARIABLES
492496
*/
493497
public boolean canGetEnvironmentVariables();
494498

495499
/**
496-
* Returns <code>true</code> if this application can update (set or unset)
497-
* its environment variables. Returns <code>false</code> if it cannot.
498-
* Internally this translates to the presence of the link to set and unset
499-
* environment variables.
500+
* Used to determine if the current instance is able to update environment variables.
500501
*
501-
* @return true if this application can set/unset its environment variables
502+
* @return Returns <code>true</code> if this application can augment its environment variables.<br>
503+
* Returns <code>false</code> if it cannot. <br>
502504
*
503505
* @see #addEnvironmentVariable(String, String)
504506
* @see #addEnvironmentVariables(Map)

‎src/main/java/com/openshift/internal/client/ApplicationResource.java‎

Lines changed: 85 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,12 @@
6767
import com.openshift.internal.client.utils.StringUtils;
6868

6969
/**
70-
* The Class Application.
70+
* The ApplicationResource object is an implementation of com.openshift.client.IApplication, and provides
71+
* a runtime model for the real application that resides on the OpenShift platform being accessed.
7172
*
7273
* @author André Dietisheim
7374
* @author Syed Iqbal
75+
* @author Martes G Wigglesworth
7476
*/
7577
public class ApplicationResource extends AbstractOpenShiftResource implements IApplication {
7678

@@ -149,7 +151,7 @@ public class ApplicationResource extends AbstractOpenShiftResource implements IA
149151
/**
150152
* The environment variables for this application
151153
*/
152-
private Map<String, IEnvironmentVariable> environmentVariableByName;
154+
private Map<String, IEnvironmentVariable> environmentVariablesMap;
153155

154156

155157
protected ApplicationResource(ApplicationResourceDTO dto, DomainResource domain) {
@@ -203,6 +205,7 @@ protected ApplicationResource(final String name, final String uuid, final String
203205
this.domain = domain;
204206
this.aliases = aliases;
205207
updateCartridges(cartridgesByName);
208+
environmentVariablesMap = new HashMap<String, IEnvironmentVariable>();
206209
}
207210

208211
public String getName() {
@@ -611,11 +614,11 @@ public Map<String, IEnvironmentVariable> getEnvironmentVariables() throws OpenSh
611614
return Collections.unmodifiableMap(new LinkedHashMap<String, IEnvironmentVariable>(getOrLoadEnvironmentVariables()));
612615
}
613616

614-
protected Map<String, IEnvironmentVariable> getOrLoadEnvironmentVariables() throws OpenShiftException {
615-
if (environmentVariableByName == null) {
616-
this.environmentVariableByName = loadEnvironmentVariables();
617-
}
618-
return environmentVariableByName;
617+
618+
protected Map<String, IEnvironmentVariable> getOrLoadEnvironmentVariables() throws OpenShiftException {
619+
if(environmentVariablesMap.isEmpty())
620+
environmentVariablesMap = loadEnvironmentVariables();
621+
return environmentVariablesMap;
619622
}
620623

621624
private Map<String, IEnvironmentVariable> loadEnvironmentVariables() throws OpenShiftException {
@@ -624,13 +627,14 @@ private Map<String, IEnvironmentVariable> loadEnvironmentVariables() throws Open
624627
return new LinkedHashMap<String, IEnvironmentVariable>();
625628
}
626629

627-
Map<String, IEnvironmentVariable> environmentVariablesByName = new LinkedHashMap<String, IEnvironmentVariable>();
628630
for (EnvironmentVariableResourceDTO environmentVariableResourceDTO : environmentVariableDTOs) {
629631
final IEnvironmentVariable environmentVariable =
630632
new EnvironmentVariableResource(environmentVariableResourceDTO, this);
631-
environmentVariablesByName.put(environmentVariable.getName(), environmentVariable);
633+
634+
environmentVariablesMap.put(environmentVariable.getName(),environmentVariable);
635+
632636
}
633-
return environmentVariablesByName;
637+
return environmentVariablesMap;
634638
}
635639

636640
@Override
@@ -643,42 +647,67 @@ public IEnvironmentVariable addEnvironmentVariable(String name, String value) th
643647
}
644648
if (hasEnvironmentVariable(name)) {
645649
throw new OpenShiftException("Environment variable with name \"{0}\" already exists.", name);
646-
}
647-
650+
}
651+
648652
EnvironmentVariableResourceDTO environmentVariableResourceDTO =
649-
new AddEnvironmentVariableRequest().execute(name, value);
653+
new AddEnvironmentVariableRequest().execute(name, value);
650654
IEnvironmentVariable environmentVariable = new EnvironmentVariableResource(environmentVariableResourceDTO, this);
651-
updateEnvironmentVariables();
655+
656+
environmentVariablesMap.put(environmentVariable.getName(), environmentVariable);
657+
652658
return environmentVariable;
653659
}
654660

655661
@Override
656-
public Map<String, IEnvironmentVariable> addEnvironmentVariables(Map<String, String> environmentVariablesMap)
662+
public Map<String, IEnvironmentVariable> addEnvironmentVariables(Map<String, String> environmentVariables)
657663
throws OpenShiftException {
658-
659-
List<EnvironmentVariableResourceDTO> environmentVariableResourceDTOs = new AddEnvironmentVariablesRequest()
660-
.execute(environmentVariablesMap);
661-
Map<String, IEnvironmentVariable> environmentVariables = new HashMap<String, IEnvironmentVariable>();
664+
665+
Map<String,String>variablesCandidateMap = new HashMap<String,String>();
666+
for(String varCandidateName:environmentVariables.keySet()){
667+
IEnvironmentVariable tempVar = environmentVariablesMap.get(varCandidateName);
668+
if(tempVar != null)
669+
{ if(tempVar.getValue() == environmentVariables.get(varCandidateName))
670+
variablesCandidateMap.put(varCandidateName,environmentVariables.get(varCandidateName));
671+
}
672+
else
673+
variablesCandidateMap.put(varCandidateName, environmentVariables.get(varCandidateName));
674+
}
675+
List<EnvironmentVariableResourceDTO> environmentVariableResourceDTOs = new AddEnvironmentVariablesRequest()
676+
.execute(variablesCandidateMap);
677+
662678
for (EnvironmentVariableResourceDTO dto : environmentVariableResourceDTOs) {
663679
IEnvironmentVariable environmentVariable = new EnvironmentVariableResource(dto, this);
664-
environmentVariables.put(environmentVariable.getName(), environmentVariable);
680+
environmentVariablesMap.put(environmentVariable.getName(), environmentVariable);
665681
}
666-
667-
updateEnvironmentVariables();
668-
return environmentVariables;
682+
683+
return environmentVariablesMap;
669684
}
670-
685+
/*
686+
* (non-Javadoc)
687+
* @see com.openshift.client.IApplication#removeEnvironmentVariable(java.lang.String)
688+
*/
671689
@Override
672-
public void removeEnvironmentVariable(String name) {
673-
IEnvironmentVariable environmentVariable = getEnvironmentVariable(name);
674-
if (environmentVariable == null) {
675-
return;
676-
}
677-
678-
environmentVariable.destroy();
679-
updateEnvironmentVariables();
690+
public void removeEnvironmentVariable(String targetName) {
691+
removeEnvironmentVariable(getEnvironmentVariable(targetName));
680692
}
681693

694+
/* (non-Javadoc)
695+
* @see com.openshift.client.IApplication#removeEnvironmentVariable(com.openshift.client.IEnvironmentVariable)
696+
*/
697+
@Override
698+
public void removeEnvironmentVariable(IEnvironmentVariable environmentVariable){
699+
if(getEnvironmentVariable(environmentVariable.getName()) == null)
700+
throw new OpenShiftException("IEnvironmentVariable with supplied name does not exist.");
701+
environmentVariable.destroy();
702+
environmentVariablesMap.remove(environmentVariable.getName());
703+
704+
}
705+
706+
707+
/*
708+
* (non-Javadoc)
709+
* @see com.openshift.client.IApplication#hasEnvironmentVariable(java.lang.String)
710+
*/
682711
@Override
683712
public boolean hasEnvironmentVariable(String name) throws OpenShiftException {
684713
if (StringUtils.isEmpty(name)) {
@@ -689,17 +718,20 @@ public boolean hasEnvironmentVariable(String name) throws OpenShiftException {
689718
}
690719

691720
protected void updateEnvironmentVariables() throws OpenShiftException {
692-
if (!canGetEnvironmentVariables()) {
721+
if (!canGetEnvironmentVariables())
693722
return;
723+
else
724+
{
725+
environmentVariablesMap.clear();
726+
environmentVariablesMap = loadEnvironmentVariables();
694727
}
695-
if (environmentVariableByName == null) {
696-
environmentVariableByName = loadEnvironmentVariables();
697-
} else {
698-
environmentVariableByName.clear();
699-
environmentVariableByName.putAll(loadEnvironmentVariables());
700-
}
728+
701729
}
702730

731+
/*
732+
* (non-Javadoc)
733+
* @see com.openshift.client.IApplication#getEnvironmentVariable(java.lang.String)
734+
*/
703735
@Override
704736
public IEnvironmentVariable getEnvironmentVariable(String name) {
705737
return getEnvironmentVariables().get(name);
@@ -714,6 +746,10 @@ public boolean canGetEnvironmentVariables() {
714746
}
715747
}
716748

749+
/*
750+
* (non-Javadoc)
751+
* @see com.openshift.client.IApplication#canUpdateEnvironmentVariables()
752+
*/
717753
@Override
718754
public boolean canUpdateEnvironmentVariables() {
719755
try {
@@ -820,9 +856,11 @@ public List<IApplicationPortForwarding> startPortForwarding() throws OpenShiftSS
820856
try {
821857
port.start(session);
822858
} catch (OpenShiftSSHOperationException oss) {
823-
// ignore for now
824-
// FIXME: should store this error on the forward to let user
825-
// know why it could not start/stop
859+
/*
860+
* ignore for now
861+
* FIXME: should store this error on the forward to let user
862+
* know why it could not start/stop
863+
*/
826864
}
827865
}
828866
return ports;
@@ -833,9 +871,10 @@ public List<IApplicationPortForwarding> stopPortForwarding() throws OpenShiftSSH
833871
try {
834872
port.stop(session);
835873
} catch (OpenShiftSSHOperationException oss) {
836-
// ignore for now
837-
// should store this error on the forward to let user know why
838-
// it could not start/stop
874+
/* ignore for now
875+
* should store this error on the forward to let user know why
876+
* it could not start/stop
877+
*/
839878
}
840879
}
841880
// make sure port forwarding is stopped by closing session...
@@ -1094,5 +1133,4 @@ protected List<EnvironmentVariableResourceDTO> execute(Map<String, String> envir
10941133
}
10951134
}
10961135

1097-
1098-
}
1136+
}

‎src/main/java/com/openshift/internal/client/EnvironmentVariableResource.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,22 @@ public String getValue() {
6161
}
6262

6363
@Override
64-
public void update(String value) throws OpenShiftException {
65-
if (value == null) {
64+
public void update(String newValue) throws OpenShiftException {
65+
if (newValue == null) {
6666
throw new OpenShiftException("Value for environment variable \"{0}\" not given.", name);
6767
}
6868
EnvironmentVariableResourceDTO environmentVariableResourceDTO =
69-
new UpdateEnvironmentVariableRequest().execute(value);
69+
new UpdateEnvironmentVariableRequest().execute(newValue);
7070
updateEnvironmentVariable(environmentVariableResourceDTO);
71+
/*
72+
* This should be done in the IApplication, to break up this dependency
73+
* on the entity, i.e. IEnvironmentVariable, on something that is
74+
* outside of itself, such as the implementation of IApplication.
75+
* @author Martes G Wigglesworth
76+
*/
7177
application.updateEnvironmentVariables();
78+
79+
7280
}
7381

7482
private void updateEnvironmentVariable(EnvironmentVariableResourceDTO dto) {
@@ -81,7 +89,7 @@ private void updateEnvironmentVariable(EnvironmentVariableResourceDTO dto) {
8189
@Override
8290
public void destroy() throws OpenShiftException {
8391
new DeleteEnvironmentVariableRequest().execute();
84-
application.updateEnvironmentVariables();
92+
8593
}
8694

8795
@Override
@@ -118,5 +126,10 @@ protected DeleteEnvironmentVariableRequest() {
118126
public IApplication getApplication() {
119127
return application;
120128
}
121-
129+
130+
public String toString(){
131+
return new String(
132+
"Name:"+this.name+",Value:"+value
133+
);
134+
}
122135
}

0 commit comments

Comments
 (0)