Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavier Coulon2016-01-29 15:56:46 +0000
committerJeff Johnston2016-02-05 19:38:39 +0000
commit085fe4fca3ee785586fb51612182f5c25afa177d (patch)
tree5cde3e02d647ea76b42354b47e0546e3a8755816
parent3414343c299e65a33a928987879869c99863d08d (diff)
downloadorg.eclipse.linuxtools-085fe4fca3ee785586fb51612182f5c25afa177d.tar.gz
org.eclipse.linuxtools-085fe4fca3ee785586fb51612182f5c25afa177d.tar.xz
org.eclipse.linuxtools-085fe4fca3ee785586fb51612182f5c25afa177d.zip
Bug 486463 - SWTException below DockerImagesView$8.run
Unregister the Docker Images View as an image listener when the view is disposed. Preventing refreshing the view when the table widget is disposed Preventing NPE when iterating on images and containers listeners Added SWTBot tests Change-Id: I45355965a236bddfb8720e7872391a4f50701fdc Signed-off-by: Xavier Coulon <xcoulon@redhat.com> Reviewed-on: https://git.eclipse.org/r/65567 Tested-by: Hudson CI Reviewed-by: Jeff Johnston <jjohnstn@redhat.com> Reviewed-on: https://git.eclipse.org/r/66040
-rw-r--r--containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java12
-rw-r--r--containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersViewSWTBotTest.java8
-rw-r--r--containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesViewSWTBotTest.java66
-rw-r--r--containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersView.java3
-rw-r--r--containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesView.java17
5 files changed, 82 insertions, 24 deletions
diff --git a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
index 9bd6dbe463..d4b1eae364 100644
--- a/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
+++ b/containers/org.eclipse.linuxtools.docker.core/src/org/eclipse/linuxtools/internal/docker/core/DockerConnection.java
@@ -342,8 +342,9 @@ public class DockerConnection implements IDockerConnection, Closeable {
@Override
public void removeContainerListener(IDockerContainerListener listener) {
- if (containerListeners != null)
+ if (containerListeners != null) {
containerListeners.remove(listener);
+ }
}
/**
@@ -385,6 +386,9 @@ public class DockerConnection implements IDockerConnection, Closeable {
*/
// TODO: include in IDockerConnection API
public List<IDockerContainerListener> getContainerListeners() {
+ if (this.containerListeners == null) {
+ return Collections.emptyList();
+ }
final IDockerContainerListener[] result = new IDockerContainerListener[this.containerListeners
.size()];
final Object[] listeners = containerListeners.getListeners();
@@ -682,8 +686,9 @@ public class DockerConnection implements IDockerConnection, Closeable {
@Override
public void removeImageListener(IDockerImageListener listener) {
- if (imageListeners != null)
+ if (imageListeners != null) {
imageListeners.remove(listener);
+ }
}
public void notifyImageListeners(List<IDockerImage> list) {
@@ -700,6 +705,9 @@ public class DockerConnection implements IDockerConnection, Closeable {
*/
// TODO: include in IDockerConnection API
public List<IDockerImageListener> getImageListeners() {
+ if (this.imageListeners == null) {
+ return Collections.emptyList();
+ }
final IDockerImageListener[] result = new IDockerImageListener[this.imageListeners
.size()];
final Object[] listeners = imageListeners.getListeners();
diff --git a/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersViewSWTBotTest.java b/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersViewSWTBotTest.java
index 2c8aa39a0c..ff7ad9c57c 100644
--- a/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersViewSWTBotTest.java
+++ b/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersViewSWTBotTest.java
@@ -58,6 +58,8 @@ public class DockerContainersViewSWTBotTest {
SWTUtils.asyncExec(() -> {try {
PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage()
.showView(DockerContainersView.VIEW_ID);
+ PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage()
+ .showView(DockerExplorerView.VIEW_ID);
} catch (Exception e) {
e.printStackTrace();
Assert.fail("Failed to open Docker Explorer view: " + e.getMessage());
@@ -76,9 +78,8 @@ public class DockerContainersViewSWTBotTest {
// remove the DockerContainerRefreshManager
dockerConnection.removeContainerListener(DockerContainerRefreshManager
.getInstance());
- // DockerExplorerView inner classes
assertThat(dockerConnection.getContainerListeners()).hasSize(2);
- // close the Docker Explorer View
+ // close the Docker Containers View
dockerContainersViewBot.close();
// there should be one listener left: DockerExplorerView
assertThat(dockerConnection.getContainerListeners()).hasSize(1);
@@ -95,9 +96,8 @@ public class DockerContainersViewSWTBotTest {
// remove the DockerContainerRefreshManager
dockerConnection.removeContainerListener(DockerContainerRefreshManager
.getInstance());
- // DockerExplorerView inner classes
assertThat(dockerConnection.getContainerListeners()).hasSize(0);
- // close the Docker Explorer View
+ // close the Docker Containers View
dockerContainersViewBot.close();
// there should be one listener left: DockerExplorerView
assertThat(dockerConnection.getContainerListeners()).hasSize(0);
diff --git a/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesViewSWTBotTest.java b/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesViewSWTBotTest.java
index 9c2bddfd32..2d9a2a7131 100644
--- a/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesViewSWTBotTest.java
+++ b/containers/org.eclipse.linuxtools.docker.ui.tests/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesViewSWTBotTest.java
@@ -14,8 +14,10 @@ package org.eclipse.linuxtools.internal.docker.ui.views;
import static org.assertj.core.api.Assertions.assertThat;
import org.eclipse.linuxtools.internal.docker.core.DockerConnection;
+import org.eclipse.linuxtools.internal.docker.core.DockerContainerRefreshManager;
import org.eclipse.linuxtools.internal.docker.ui.testutils.MockDockerClientFactory;
import org.eclipse.linuxtools.internal.docker.ui.testutils.MockDockerConnectionFactory;
+import org.eclipse.linuxtools.internal.docker.ui.testutils.MockDockerContainerFactory;
import org.eclipse.linuxtools.internal.docker.ui.testutils.MockDockerImageFactory;
import org.eclipse.linuxtools.internal.docker.ui.testutils.swt.ClearConnectionManagerRule;
import org.eclipse.linuxtools.internal.docker.ui.testutils.swt.CloseWelcomePageRule;
@@ -41,8 +43,9 @@ import com.spotify.docker.client.DockerClient;
public class DockerImagesViewSWTBotTest {
private SWTWorkbenchBot bot = new SWTWorkbenchBot();
- private SWTBotView dockerImagesViewBot;
+ private SWTBotView dockerImagesBotView;
private DockerImagesView dockerImagesView;
+ private SWTBotView dockerExplorerBotView;
@ClassRule
public static CloseWelcomePageRule closeWelcomePage = new CloseWelcomePageRule();
@@ -56,23 +59,26 @@ public class DockerImagesViewSWTBotTest {
@Before
public void setup() {
this.bot = new SWTWorkbenchBot();
- SWTUtils.asyncExec(() -> {try {
- PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage()
- .showView("org.eclipse.linuxtools.docker.ui.dockerImagesView");
- } catch (Exception e) {
- e.printStackTrace();
- Assert.fail("Failed to open Docker Images view: " + e.getMessage());
- }});
- this.dockerImagesViewBot = bot.viewById("org.eclipse.linuxtools.docker.ui.dockerImagesView");
- this.dockerImagesView = (DockerImagesView) (dockerImagesViewBot.getViewReference().getView(true));
+ SWTUtils.asyncExec(() -> {
+ try {
+ PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage()
+ .showView(DockerExplorerView.VIEW_ID);
+ PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage().showView(DockerImagesView.VIEW_ID);
+ } catch (Exception e) {
+ e.printStackTrace();
+ Assert.fail("Failed to open Docker Images view: " + e.getMessage());
+ }
+ });
+ this.dockerImagesBotView = bot.viewById("org.eclipse.linuxtools.docker.ui.dockerImagesView");
+ this.dockerImagesView = (DockerImagesView) (dockerImagesBotView.getViewReference().getView(true));
+ this.dockerExplorerBotView = bot.viewById(DockerExplorerView.VIEW_ID);
}
@Test
public void shouldShowAllImageVariants() {
// given
- final DockerClient client = MockDockerClientFactory
- .image(MockDockerImageFactory.id("1a2b3c4d5e6f7g").name("foo:1.0", "foo:latest", "bar:1.0", "bar:latest").build())
- .build();
+ final DockerClient client = MockDockerClientFactory.image(MockDockerImageFactory.id("1a2b3c4d5e6f7g")
+ .name("foo:1.0", "foo:latest", "bar:1.0", "bar:latest").build()).build();
final DockerConnection dockerConnection = MockDockerConnectionFactory.from("Test", client).get();
DockerConnectionManagerUtils.configureConnectionManager(dockerConnection);
// then 1 image with all repo/tags should be displayed
@@ -82,4 +88,38 @@ public class DockerImagesViewSWTBotTest {
assertThat(images[0].getText(1)).isEqualTo("bar:1.0\nbar:latest\nfoo:1.0\nfoo:latest");
});
}
+
+ @Test
+ public void shouldRemoveListenersWhenClosingView() {
+ // given
+ final DockerClient client = MockDockerClientFactory
+ .container(MockDockerContainerFactory.name("angry_bar").status("Stopped").build()).build();
+ final DockerConnection dockerConnection = MockDockerConnectionFactory.from("Test", client).get();
+ DockerConnectionManagerUtils.configureConnectionManager(dockerConnection);
+ // remove the DockerContainerRefreshManager
+ dockerConnection.removeContainerListener(DockerContainerRefreshManager.getInstance());
+ // DockerExplorerView inner classes
+ assertThat(dockerConnection.getImageListeners()).hasSize(2);
+ // close the Docker Images View
+ dockerImagesBotView.close();
+ // there should be one listener left: DockerExplorerView
+ assertThat(dockerConnection.getImageListeners()).hasSize(1);
+ }
+
+ @Test
+ public void shouldNotRemoveListenersWhenNoSelectedConnectionBeforeClosingView() {
+ // given
+ dockerExplorerBotView.close();
+ final DockerClient client = MockDockerClientFactory
+ .container(MockDockerContainerFactory.name("angry_bar").status("Stopped").build()).build();
+ final DockerConnection dockerConnection = MockDockerConnectionFactory.from("Test", client).get();
+ DockerConnectionManagerUtils.configureConnectionManager(dockerConnection);
+ // remove the DockerContainerRefreshManager
+ dockerConnection.removeContainerListener(DockerContainerRefreshManager.getInstance());
+ assertThat(dockerConnection.getImageListeners()).hasSize(0);
+ // close the Docker Images View
+ dockerImagesBotView.close();
+ // there should be one listener left: DockerExplorerView
+ assertThat(dockerConnection.getImageListeners()).hasSize(0);
+ }
}
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersView.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersView.java
index 69c0e785fa..3b39a0fdac 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersView.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerContainersView.java
@@ -282,7 +282,8 @@ public class DockerContainersView extends ViewPart implements
this.viewer.setComparator(comparator);
// apply search filter
this.viewer.addFilter(getContainersFilter());
- IDockerConnection[] connections = DockerConnectionManager.getInstance()
+ final IDockerConnection[] connections = DockerConnectionManager
+ .getInstance()
.getConnections();
if (connections.length > 0) {
setConnection(connections[0]);
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesView.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesView.java
index adfcdb4f24..5950d85671 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesView.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/views/DockerImagesView.java
@@ -91,6 +91,11 @@ public class DockerImagesView extends ViewPart implements IDockerImageListener,
@Override
public void dispose() {
+ // remove this listener instance registered on the Docker connection
+ if (connection != null) {
+ connection.removeImageListener(this);
+ }
+
// stop tracking selection changes in the Docker Explorer view (only)
getSite().getWorkbenchWindow().getSelectionService()
.removeSelectionListener(DockerExplorerView.VIEW_ID, this);
@@ -271,8 +276,8 @@ public class DockerImagesView extends ViewPart implements IDockerImageListener,
viewer.setComparator(comparator);
// apply search filter
this.viewer.addFilter(getImagesFilter());
-
- IDockerConnection[] connections = DockerConnectionManager.getInstance()
+ final IDockerConnection[] connections = DockerConnectionManager
+ .getInstance()
.getConnections();
if (connections.length > 0) {
setConnection(connections[0]);
@@ -367,8 +372,12 @@ public class DockerImagesView extends ViewPart implements IDockerImageListener,
Display.getDefault().asyncExec(new Runnable() {
@Override
public void run() {
- DockerImagesView.this.viewer.refresh();
- refreshViewTitle();
+ if (DockerImagesView.this.viewer != null
+ && !DockerImagesView.this.viewer.getTable()
+ .isDisposed()) {
+ DockerImagesView.this.viewer.refresh();
+ refreshViewTitle();
+ }
}
});
}

Back to the top