[JBIDE-23210] deploy docker wizard: dont load projects twice (#1333)

jbosstools · Oct 3, 2016 · 18ddb66 · 18ddb66
1 parent bdff52e
commit 18ddb66
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 17 deletions.
diff --git a/...hift.ui/src/org/jboss/tools/openshift/internal/ui/wizard/deployimage/DeployImagePage.java b/...hift.ui/src/org/jboss/tools/openshift/internal/ui/wizard/deployimage/DeployImagePage.java
@@ -178,9 +178,11 @@ protected void onPageWillGetActivated(final Direction progress, final PageChangi
 		if(imageNameProposalAdapter != null) {
 			imageNameProposalAdapter.setEnabled(true);
 		}
-		if (Direction.FORWARDS == progress) {
-			loadResources(dbc);
-		}
+	}
+
+	@Override
+	protected void onPageActivated(DataBindingContext dbc) {
+		loadResources(dbc);
 	}
 
 	private void loadResources(DataBindingContext dbc) {

diff --git a/.../src/org/jboss/tools/openshift/internal/ui/wizard/deployimage/DeployImageWizardModel.java b/.../src/org/jboss/tools/openshift/internal/ui/wizard/deployimage/DeployImageWizardModel.java
@@ -23,6 +23,7 @@
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.apache.commons.lang.ObjectUtils;
 import org.apache.commons.lang.StringUtils;
 import org.eclipse.linuxtools.docker.core.DockerConnectionManager;
 import org.eclipse.linuxtools.docker.core.IDockerConnection;
@@ -57,27 +58,21 @@ public class DeployImageWizardModel
 		implements IDeployImageParameters, IDockerConnectionManagerListener2, PropertyChangeListener {
 
 	private static final int DEFAULT_REPLICA_COUNT = 1;
+	private static final DockerImage2OpenshiftResourceConverter dockerImage2OpenshiftResourceConverter = new DockerImage2OpenshiftResourceConverter();
 
 	private Connection connection;
 	private IProject project;
+	private List<IProject> projects = new ArrayList<>();
 	private String resourceName;
 	private String imageName;
-	private List<IProject> projects = new ArrayList<>();
-
 	private EnvironmentVariablesPageModel envModel = new EnvironmentVariablesPageModel();
-
 	private List<String> volumes = Collections.emptyList();
 	private String selectedVolume;
-
 	private List<IPort> portSpecs = Collections.emptyList();
-
 	private int replicas;
-
 	private boolean addRoute = true;
-
 	List<IServicePort> servicePorts = new ArrayList<>();
 	IServicePort selectedServicePort = null;
-
 	private IDockerConnection dockerConnection;
 	private ArrayList<IServicePort> imagePorts;
 	private boolean originatedFromDockerExplorer;
@@ -87,13 +82,10 @@ public class DeployImageWizardModel
 	private String targetRegistryLocation;
 	private String targetRegistryUsername;
 	private String targetRegistryPassword;
-
-
-
-	private static final DockerImage2OpenshiftResourceConverter dockerImage2OpenshiftResourceConverter = new DockerImage2OpenshiftResourceConverter();
 	private final List<String> imageNames = new ArrayList<>();
     private List<IDockerConnection> dockerConnections = Arrays.asList(DockerConnectionManager.getInstance().getConnections());
     private Comparator<IProject> projectsComparator;
+	protected boolean resourcesLoaded = false;
 
 	public DeployImageWizardModel() {
 		envModel.addPropertyChangeListener(PROPERTY_ENVIRONMENT_VARIABLES, this);
@@ -188,14 +180,23 @@ private void initImageRegistry(Connection connection) {
 
 	@Override
 	public void setConnection(final Connection connection) {
+		Connection oldConnection = this.connection;
 		firePropertyChange(PROPERTY_CONNECTION, this.connection, this.connection = connection);
 		initImageRegistry(connection);
+		this.resourcesLoaded = ObjectUtils.equals(oldConnection, connection);
 	}
 
+	@Override
 	public void loadResources() {
+		if (resourcesLoaded) {
+			return;
+		}
+
 		List<IProject> projects = connection.getResources(ResourceKind.PROJECT);
 		setProjects(projects);
 		setProjectOrDefault(project);
+
+		this.resourcesLoaded = true;
 	}
 
 	protected void setProjects(List<IProject> projects) {

diff --git a/.../src/org/jboss/tools/openshift/test/ui/wizard/deployimage/DeployImageWizardModelTest.java b/.../src/org/jboss/tools/openshift/test/ui/wizard/deployimage/DeployImageWizardModelTest.java
@@ -13,6 +13,10 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.io.IOException;
@@ -63,14 +67,14 @@ public class DeployImageWizardModelTest {
     private IDockerConnection dockerConnection;
 	private Connection connection;
 	private IProject project;
-    private DeployImageWizardModel model;
+    private TestableDeployImageWizardModel model;
 
     @Before
     public void setUp() {
     	this.dockerConnection = mock(IDockerConnection.class);
     	this.connection = createConnection();
     	this.project = mock(IProject.class);
-    	this.model = new DeployImageWizardModel();
+    	this.model = new TestableDeployImageWizardModel();
     	model.setConnection(connection);
     	createModelProjects(connection, project, mock(IProject.class));
     	model.setDockerConnection(dockerConnection);
@@ -237,6 +241,7 @@ public void loadResources_should_load_projects() {
         IProject project1 = mock(IProject.class);
         IProject project2 = mock(IProject.class);
     	createModelProjects(this.connection, project1, project2) ;
+    	model.setResourcesLoaded(false);
     	// when
     	model.loadResources();
     	// then
@@ -251,6 +256,7 @@ public void loadResources_should_reset_project_if_not_contained_in_loaded_projec
     	IProject project1 = mock(IProject.class);
     	IProject project2 = mock(IProject.class);
     	createModelProjects(this.connection, project1, project2) ;
+    	model.setResourcesLoaded(false);
     	// when
     	model.loadResources();
     	// then
@@ -267,6 +273,27 @@ public void addProject_should_not_reset_project() {
         assertThat(model.getProject()).isEqualTo(project);
     }
 
+    @Test
+    public void should_not_load_resources_twice() {
+    	// given
+    	verify(connection, times(1)).getResources(ResourceKind.PROJECT); // loaded in #setUp
+    	reset(connection);
+    	// when
+    	model.loadResources();
+    	verify(connection, never()).getResources(ResourceKind.PROJECT);
+    }
+
+    @Test
+    public void should_reload_resources_if_connection_was_changed() {
+    	// given
+    	Connection connection2 = mock(Connection.class);
+    	model.setConnection(connection2);
+    	// when
+    	model.loadResources();
+    	// then
+    	verify(connection2, times(1)).getResources(ResourceKind.PROJECT);
+    }
+
     @Test
     public void checkThatRemoveAnExistingServicePortIsEffective() {
         // assume Docker image is on local
@@ -414,4 +441,13 @@ private static IDockerImage mockSingleImage(IDockerConnection dockerConnection,
         when(dockerConnection.getImages()).thenReturn(Collections.singletonList(image));
         return image;
     }    
+
+    private static final class TestableDeployImageWizardModel extends DeployImageWizardModel {
+
+    	public void setResourcesLoaded(boolean loaded) {
+    		this.resourcesLoaded = loaded;
+    	}
+
+    }
+
 }