Bug 508705 - Don't try to browse a container that isn't using TTY

- add check for usingTTY inside ContainerCopyFrom and ContainerCopyTo
  wizards (use Container info)
- change isRunning flag in ContainerCopyFromPage and
  ContainerCopyToPage to be named canBrowseContainer
- call ContainerCopyFromPage and ContainerCopyToPage constructors
  with and'ing of isRunning and usingTTY booleans to form the
  canBrowseContainer flag
- fix CopyFromContainerCommandHandler to only show the DockerException
  cause when it is non-null and do the same for
  CopyToContainerCommandHandler

Change-Id: I8a0a8c243d07923e7289fa58988c728aee6889fc
Reviewed-on: https://git.eclipse.org/r/86421
Tested-by: Hudson CI
Reviewed-by: Jeff Johnston <jjohnstn@redhat.com>

6 files changed, 53 insertions, 24 deletions

diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java
index d1c479ee68..9a28890bd9 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyFromContainerCommandHandler.java
@@ -151,8 +151,9 @@ public class CopyFromContainerCommandHandler extends AbstractHandler {
 													ERROR_COPYING_FROM_CONTAINER,
 													proxy.getLink(),
 													container.name()),
-											e.getCause().getMessage()));
-							// for now
+											e.getCause() != null
+													? e.getCause().getMessage()
+													: e.getMessage()));
 						}
 					}
 				} catch (InterruptedException e) {
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java
index 5dd0b12b68..67f682773f 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/commands/CopyToContainerCommandHandler.java
@@ -191,7 +191,6 @@ public class CopyToContainerCommandHandler extends AbstractHandler {
 								tmpDir.toString(), container.id(), target);
 						deleteTmpDir(tmpDir);
 					} catch (final DockerException | IOException e) {
-						final String tmpDirName = tmpDir.toString();
 						Display.getDefault()
 								.syncExec(() -> MessageDialog.openError(
 										PlatformUI.getWorkbench()
@@ -200,7 +199,9 @@ public class CopyToContainerCommandHandler extends AbstractHandler {
 										CommandMessages.getFormattedString(
 												ERROR_COPYING_TO_CONTAINER,
 												target, container.name()),
-										e.getCause().getMessage()));
+										e.getCause() != null
+												? e.getCause().getMessage()
+												: e.getMessage()));
 
 					}
 				} catch (InterruptedException e) {
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFrom.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFrom.java
index 698652da86..020baefdda 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFrom.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFrom.java
@@ -20,6 +20,7 @@ import org.eclipse.jface.wizard.Wizard;
 import org.eclipse.linuxtools.docker.core.EnumDockerStatus;
 import org.eclipse.linuxtools.docker.core.IDockerConnection;
 import org.eclipse.linuxtools.docker.core.IDockerContainer;
+import org.eclipse.linuxtools.docker.core.IDockerContainerInfo;
 import org.eclipse.linuxtools.docker.ui.Activator;
 import org.eclipse.linuxtools.internal.docker.core.ContainerFileProxy;
 import org.eclipse.linuxtools.internal.docker.core.DockerConnection;
@@ -68,8 +69,19 @@ public class ContainerCopyFrom extends Wizard {
 		boolean isRunning = EnumDockerStatus.fromStatusMessage(
 				container.status()) == EnumDockerStatus.RUNNING;
 
+		// we only want to browse a Container's file system if
+		// it is running and it has specified to use a tty
+		boolean usingTTY = false;
+		if (isRunning) {
+			IDockerContainerInfo info = connection
+					.getContainerInfo(container.id());
+			if (info != null) {
+				usingTTY = info.config().tty();
+			}
+		}
+
 		mainPage = new ContainerCopyFromPage(sfo.getResult(), provider,
-				container.name(), isRunning);
+				container.name(), isRunning && usingTTY);
 		addPage(mainPage);
 	}
 
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFromPage.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFromPage.java
index ae6172e3f1..cc41322b15 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFromPage.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyFromPage.java
@@ -77,7 +77,7 @@ public class ContainerCopyFromPage extends WizardPage {
 
 	private IImportStructureProvider structureProvider;
 
-	private boolean isRunning;
+	private boolean canBrowseContainer;
 
     // the visual selection widget group
     CheckboxTreeAndListGroup selectionGroup;
@@ -104,10 +104,12 @@ public class ContainerCopyFromPage extends WizardPage {
 	 *            container
 	 * @param containerName
 	 *            - name of container
+	 * @param canBrowseContainer
+	 *            - whether we can browse the Container's file system
 	 */
 	public ContainerCopyFromPage(FileSystemElement fileSystemElement,
 			IImportStructureProvider structureProvider, String containerName,
-			boolean isRunning) {
+			boolean canBrowseContainer) {
 		super(WizardMessages.getString(NAME));
 		setDescription(WizardMessages.getFormattedString(DESC,
 				containerName));
@@ -115,7 +117,7 @@ public class ContainerCopyFromPage extends WizardPage {
 		setImageDescriptor(SWTImagesFactory.DESC_WIZARD);
         root = fileSystemElement;
 		this.structureProvider = structureProvider;
-		this.isRunning = isRunning;
+		this.canBrowseContainer = canBrowseContainer;
     }
 
 
@@ -131,7 +133,7 @@ public class ContainerCopyFromPage extends WizardPage {
 	 */
 	@SuppressWarnings({ "rawtypes", "unchecked" })
 	public Iterator getValueIterator() {
-		if (isRunning) {
+		if (canBrowseContainer) {
 			return selectionGroup.getAllWhiteCheckedItems();
 		} else {
 			// We need to fake a collection of MinimizedFileSystemElement with
@@ -166,7 +168,7 @@ public class ContainerCopyFromPage extends WizardPage {
 		}
 
 		if (!error) {
-			if (isRunning) {
+			if (canBrowseContainer) {
 				if (selectionGroup.getCheckedElementCount() == 0) {
 					error = true;
 					setErrorMessage(
@@ -226,7 +228,7 @@ public class ContainerCopyFromPage extends WizardPage {
 
 		Composite selectionComposite = null;
 
-		if (isRunning) {
+		if (canBrowseContainer) {
 			selectionComposite = new Composite(composite, SWT.NULL);
 			GridLayout selectionLayout = new GridLayout();
 			selectionComposite.setLayout(selectionLayout);
@@ -259,7 +261,7 @@ public class ContainerCopyFromPage extends WizardPage {
 
 		}
 
-		if (isRunning) {
+		if (canBrowseContainer) {
 			WorkbenchViewerComparator comparator = new WorkbenchViewerComparator();
 			selectionGroup.setTreeComparator(comparator);
 			selectionGroup.setListComparator(comparator);
@@ -270,7 +272,7 @@ public class ContainerCopyFromPage extends WizardPage {
 		validate();
 		setPageComplete(false);
 
-		if (isRunning) {
+		if (canBrowseContainer) {
 			selectionGroup.aboutToOpen();
 		}
     }
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyTo.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyTo.java
index 08665ceced..d6b54ee18f 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyTo.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyTo.java
@@ -65,9 +65,15 @@ public class ContainerCopyTo extends Wizard {
 		boolean isRunning = EnumDockerStatus.fromStatusMessage(
 				container.status()) == EnumDockerStatus.RUNNING;
 
+		// we only want to browse a Container's file system if
+		// it is running and it has specified to use a tty
+		boolean usingTTY = false;
+		if (isRunning) {
+			usingTTY = container.info().config().tty();
+		}
 
 		mainPage = new ContainerCopyToPage(sfo.getResult(), provider,
-				container.name(), isRunning);
+				container.name(), isRunning && usingTTY);
 		addPage(mainPage);
 	}
 
diff --git a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyToPage.java b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyToPage.java
index 10092a0d38..31173f8d29 100644
--- a/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyToPage.java
+++ b/containers/org.eclipse.linuxtools.docker.ui/src/org/eclipse/linuxtools/internal/docker/ui/wizards/ContainerCopyToPage.java
@@ -116,7 +116,7 @@ public class ContainerCopyToPage
 
 	private FileSystemElement root;
 
-	private boolean isRunning;
+	private boolean canBrowseContainer;
 
 	private ContainerFileSystemProvider provider;
 
@@ -129,19 +129,26 @@ public class ContainerCopyToPage
 	private IPath destination;
 
 	/**
-     *	Creates an instance of this class
-     *
-     * @param aWorkbench IWorkbench
-     * @param selection IStructuredSelection
-     */
+	 * Constructor
+	 * 
+	 * @param root
+	 *            - file system element root of the Container file system ("/")
+	 * @param provider
+	 *            - file system provider that can dynamically traverse Container
+	 *            directories
+	 * @param containerName
+	 *            - name of Container
+	 * @param canBrowseContainer
+	 *            - whether we can browse Container's file system or not
+	 */
 	public ContainerCopyToPage(FileSystemElement root,
 			ContainerFileSystemProvider provider, String containerName,
-			boolean isRunning) {
+			boolean canBrowseContainer) {
 		super("ContainerCopyToPage1", StructuredSelection.EMPTY);//$NON-NLS-1$
 		this.root = root;
 		this.provider = provider;
 		this.containerName = containerName;
-		this.isRunning = isRunning;
+		this.canBrowseContainer = canBrowseContainer;
 		setTitle(NLS.bind(CopyToContainerMessages.ContainerCopyTo_title,
 				this.containerName));
 		setDescription(
@@ -270,7 +277,7 @@ public class ContainerCopyToPage
 
 	@Override
 	protected void createDestinationGroup(Composite parent) {
-		if (isRunning) {
+		if (canBrowseContainer) {
 			super.createDestinationGroup(parent);
 		} else {
 			// container specification group
@@ -499,7 +506,7 @@ public class ContainerCopyToPage
 
 	@Override
 	protected IPath getResourcePath() {
-		if (isRunning) {
+		if (canBrowseContainer) {
 			return super.getResourcePath();
 		}
 		return new Path(containerNameField.getText());