From 11c9462b25a36fa6a4f0f5bcd080d4b700886137 Mon Sep 17 00:00:00 2001 From: Matthew Khouzam Date: Wed, 15 May 2013 15:03:46 -0400 Subject: [PATCH] tmf: Bug fixes for batch import Remove non-standard non-modal wizard. Add cancelability to the directory add. clean up some code, remove " " strings merge a job that only calls a job into the top job Change-Id: I18beb4a6054092388b180f42e91be6b8e598d728 Signed-off-by: Matthew Khouzam Reviewed-on: https://git.eclipse.org/r/13246 --- .../handlers/BatchImportTraceHandler.java | 3 +- .../importtrace/BatchImportTraceWizard.java | 103 ++++++++----- .../ImportTraceWizardScanPage.java | 100 ++++++------ .../importtrace/NonModalWizardDialog.java | 143 ------------------ 4 files changed, 119 insertions(+), 230 deletions(-) delete mode 100644 org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/NonModalWizardDialog.java diff --git a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/BatchImportTraceHandler.java b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/BatchImportTraceHandler.java index 461e175e09..362f869ad9 100644 --- a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/BatchImportTraceHandler.java +++ b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/internal/tmf/ui/project/handlers/BatchImportTraceHandler.java @@ -18,7 +18,6 @@ import org.eclipse.jface.viewers.StructuredSelection; import org.eclipse.jface.wizard.WizardDialog; import org.eclipse.linuxtools.tmf.ui.project.model.TmfTraceFolder; import org.eclipse.linuxtools.tmf.ui.project.wizards.importtrace.BatchImportTraceWizard; -import org.eclipse.linuxtools.tmf.ui.project.wizards.importtrace.NonModalWizardDialog; import org.eclipse.swt.widgets.Shell; import org.eclipse.ui.IWorkbench; import org.eclipse.ui.IWorkbenchWindow; @@ -48,7 +47,7 @@ public class BatchImportTraceHandler extends ImportTraceHandler { BatchImportTraceWizard wizard = new BatchImportTraceWizard(); wizard.init(PlatformUI.getWorkbench(), new StructuredSelection(traceFolder)); - WizardDialog dialog = new NonModalWizardDialog(shell, wizard); + WizardDialog dialog = new WizardDialog(shell, wizard); dialog.open(); traceFolder.refresh(); diff --git a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/BatchImportTraceWizard.java b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/BatchImportTraceWizard.java index 1163c27b35..1715311e54 100644 --- a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/BatchImportTraceWizard.java +++ b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/BatchImportTraceWizard.java @@ -16,6 +16,7 @@ import java.io.File; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -62,6 +63,9 @@ import org.eclipse.ui.wizards.datatransfer.ImportOperation; */ public class BatchImportTraceWizard extends ImportTraceWizard { + private static final int WIN_HEIGHT = 400; + private static final int WIN_WIDTH = 800; + private static final Status CANCEL_STATUS = new Status(IStatus.CANCEL, Activator.PLUGIN_ID, ""); //$NON-NLS-1$ private static final int TOTALWORK = 65536; // ----------------- // Constants @@ -78,10 +82,9 @@ public class BatchImportTraceWizard extends ImportTraceWizard { private IWizardPage fSelectDirectoriesPage; private IWizardPage fScanPage; private IWizardPage fSelectTypePage; - private NonModalWizardDialog fNonModalWizard = null; private final List fTraceTypesToScan = new ArrayList(); - private final Set fParentFilesToScan = new TreeSet(); + private final Set fParentFilesToScan = new HashSet(); private ImportTraceContentProvider fScannedTraces = new ImportTraceContentProvider(); @@ -91,6 +94,9 @@ public class BatchImportTraceWizard extends ImportTraceWizard { private BlockingQueue fTracesToScan; private final Set fTraces = new TreeSet(); + + private Map> fParentFiles = new HashMap>(); + // Target import directory ('Traces' folder) private IFolder fTargetFolder; @@ -131,25 +137,11 @@ public class BatchImportTraceWizard extends ImportTraceWizard { addPage(fSelectTypePage); addPage(fSelectDirectoriesPage); addPage(fScanPage); - final WizardDialog container = (WizardDialog)getContainer(); - container.setPageSize(800, 400); - container.updateSize(); - } - - /** - * A non-modal wizard container - * - * @return a non-modal wizard container - */ - public NonModalWizardDialog getNMContainer() { - if (!(super.getContainer() instanceof NonModalWizardDialog)) { - if (fNonModalWizard == null) { - WizardDialog dlg = (WizardDialog) super.getContainer(); - fNonModalWizard = new NonModalWizardDialog(dlg); - } - return fNonModalWizard; + final WizardDialog container = (WizardDialog) getContainer(); + if (container != null) { + container.setPageSize(WIN_WIDTH, WIN_HEIGHT); + container.updateSize(); } - return (NonModalWizardDialog) super.getContainer(); } /** @@ -159,8 +151,12 @@ public class BatchImportTraceWizard extends ImportTraceWizard { * the file to scan */ public void addFileToScan(final String fileName) { - fParentFilesToScan.add(fileName); - startUpdateTask(Messages.BatchImportTraceWizard_add + " " + fileName); //$NON-NLS-1$ + if (!fParentFiles.containsKey(fileName)) { + fParentFiles.put(fileName, new HashSet()); + startUpdateTask(Messages.BatchImportTraceWizard_add + ' ' + fileName, fileName); + + } + } /** @@ -170,13 +166,14 @@ public class BatchImportTraceWizard extends ImportTraceWizard { * the name of the file to remove */ public void removeFile(final String fileName) { + fParentFiles.remove(fileName); fParentFilesToScan.remove(fileName); - startUpdateTask(Messages.BatchImportTraceWizard_remove + " " + fileName);//$NON-NLS-1$ + startUpdateTask(Messages.BatchImportTraceWizard_remove + ' ' + fileName, null); } - private void startUpdateTask(final String taskName) { + private void startUpdateTask(final String taskName, final String fileName) { try { - this.getContainer().run(true, false, new IRunnableWithProgress() { + this.getContainer().run(true, true, new IRunnableWithProgress() { @Override public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException { @@ -187,15 +184,13 @@ public class BatchImportTraceWizard extends ImportTraceWizard { sm = SubMonitor.convert(monitor); sm.setTaskName(taskName); sm.setWorkRemaining(TOTALWORK); - updateFiles(sm); + updateFiles(sm, fileName); sm.done(); } } }); } catch (InvocationTargetException e) { } catch (InterruptedException e) { - } finally { - } } @@ -349,6 +344,7 @@ public class BatchImportTraceWizard extends ImportTraceWizard { /** * Returns if a trace to import is selected + * * @return if there are traces to import */ public boolean hasTracesToImport() { @@ -537,12 +533,10 @@ public class BatchImportTraceWizard extends ImportTraceWizard { /* * I am a job. Make me work */ - private synchronized IStatus updateFiles(IProgressMonitor monitor) { + private synchronized IStatus updateFiles(IProgressMonitor monitor, String traceToScan) { final Set filesToScan = new TreeSet(); - final String[] parentFiles = fParentFilesToScan.toArray(new String[0]); - final String[] traceTypes = fTraceTypesToScan.toArray(new String[0]); int workToDo = 1; - for (String name : parentFiles) { + for (String name : fParentFiles.keySet()) { final File file = new File(name); final File[] listFiles = file.listFiles(); @@ -552,19 +546,34 @@ public class BatchImportTraceWizard extends ImportTraceWizard { } int step = TOTALWORK / workToDo; try { - for (String name : parentFiles) { + for (String name : fParentFiles.keySet()) { final File fileToAdd = new File(name); - recurse(filesToScan, fileToAdd, monitor, step); + final Set parentFilesToScan = fParentFiles.get(fileToAdd.getAbsolutePath()); + recurse(parentFilesToScan, fileToAdd, monitor, step); + if (monitor.isCanceled()) { + fParentFilesToScan.remove(traceToScan); + fParentFiles.remove(traceToScan); + return CANCEL_STATUS; + } + } + filesToScan.clear(); + for (String name : fParentFiles.keySet()) { + filesToScan.addAll(fParentFiles.get(name)); + fParentFilesToScan.add(name); } - for (String fileToScan : filesToScan) { - for (String traceCat : traceTypes) { + for (String traceCat : fTraceTypesToScan) { TraceValidationHelper tv = new TraceValidationHelper(fileToScan, traceCat); // for thread safety, keep checks in this order. if (!fResults.containsKey(tv)) { if (!fTracesToScan.contains(tv)) { fTracesToScan.put(tv); monitor.subTask(tv.getTraceToScan()); + if (monitor.isCanceled()) { + fParentFilesToScan.remove(traceToScan); + fParentFiles.remove(traceToScan); + return CANCEL_STATUS; + } } } } @@ -578,7 +587,7 @@ public class BatchImportTraceWizard extends ImportTraceWizard { return Status.OK_STATUS; } - private void recurse(Set filesToScan, File fileToAdd, IProgressMonitor monitor, int step) { + private IStatus recurse(Set filesToScan, File fileToAdd, IProgressMonitor monitor, int step) { final String absolutePath = fileToAdd.getAbsolutePath(); if (!filesToScan.contains(absolutePath) && (filesToScan.size() < MAX_FILES)) { filesToScan.add(absolutePath); @@ -586,24 +595,38 @@ public class BatchImportTraceWizard extends ImportTraceWizard { if (null != listFiles) { for (File child : listFiles) { monitor.subTask(child.getName()); - recurse(filesToScan, child); + if (monitor.isCanceled()) { + return CANCEL_STATUS; + } + IStatus retVal = recurse(filesToScan, child, monitor); + if (retVal.getSeverity() == IStatus.CANCEL) { + return retVal; + } monitor.worked(step); } } } + return Status.OK_STATUS; } - private void recurse(Set filesToScan, File fileToAdd) { + private IStatus recurse(Set filesToScan, File fileToAdd, IProgressMonitor monitor) { final String absolutePath = fileToAdd.getAbsolutePath(); if (!filesToScan.contains(absolutePath) && (filesToScan.size() < MAX_FILES)) { filesToScan.add(absolutePath); final File[] listFiles = fileToAdd.listFiles(); if (null != listFiles) { for (File child : listFiles) { - recurse(filesToScan, child); + if (monitor.isCanceled()) { + return CANCEL_STATUS; + } + IStatus retVal = recurse(filesToScan, child, monitor); + if (retVal.getSeverity() == IStatus.CANCEL) { + return retVal; + } } } } + return Status.OK_STATUS; } /** diff --git a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardScanPage.java b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardScanPage.java index 514c18b914..b5af0943a4 100644 --- a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardScanPage.java +++ b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/ImportTraceWizardScanPage.java @@ -13,13 +13,14 @@ package org.eclipse.linuxtools.tmf.ui.project.wizards.importtrace; import java.io.File; -import java.lang.reflect.InvocationTargetException; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.BlockingQueue; import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; import org.eclipse.core.runtime.SubMonitor; -import org.eclipse.jface.operation.IRunnableWithProgress; +import org.eclipse.core.runtime.jobs.Job; import org.eclipse.jface.viewers.CellEditor; import org.eclipse.jface.viewers.CheckStateChangedEvent; import org.eclipse.jface.viewers.CheckboxTreeViewer; @@ -60,11 +61,12 @@ import org.eclipse.ui.IWorkbench; */ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { + private static final int COL_WIDTH = 200; private static final int MAX_TRACES = 65536; private CheckboxTreeViewer traceTypeViewer; // private int position = 0; - final ScanRunnable fRunnable = new ScanRunnable(); + final ScanRunnable fRunnable = new ScanRunnable("Scan job"); //$NON-NLS-1$ final private BlockingQueue fTracesToScan = new ArrayBlockingQueue(MAX_TRACES); private volatile boolean fCanRun = true; @@ -99,6 +101,7 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { @Override public void dispose() { fCanRun = false; + fRunnable.done(Status.OK_STATUS); super.dispose(); } @@ -129,7 +132,7 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { // Column 1 // -------------------- TreeViewerColumn column = new TreeViewerColumn(traceTypeViewer, SWT.NONE); - column.getColumn().setWidth(200); + column.getColumn().setWidth(COL_WIDTH); column.getColumn().setText(Messages.ImportTraceWizardImportCaption); column.setLabelProvider(new FirstColumnLabelProvider()); column.setEditingSupport(new ColumnEditorSupport(traceTypeViewer, textCellEditor)); @@ -139,7 +142,7 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { // -------------------- column = new TreeViewerColumn(traceTypeViewer, SWT.NONE); - column.getColumn().setWidth(200); + column.getColumn().setWidth(COL_WIDTH); column.getColumn().setText(Messages.ImportTraceWizardTraceDisplayName); column.setLabelProvider(new ColumnLabelProvider() { @Override @@ -155,7 +158,7 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { init(); getBatchWizard().setTracesToScan(fTracesToScan); getBatchWizard().setTraceFolder(fTargetFolder); - getBatchWizard().getNMContainer().backgroundRun(this, fRunnable); + fRunnable.schedule(); setErrorMessage(Messages.ImportTraceWizardScanPage_SelectAtleastOne); } @@ -216,7 +219,6 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { @Override public void widgetDefaultSelected(SelectionEvent e) { - // TODO Auto-generated method stub } } @@ -350,7 +352,12 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { } } - private final class ScanRunnable implements IRunnableWithProgress { + private final class ScanRunnable extends Job { + + public ScanRunnable(String name) { + super(name); + } + private IProgressMonitor fMonitor; private synchronized IProgressMonitor getMonitor() { @@ -358,7 +365,7 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { } @Override - public void run(IProgressMonitor monitor) throws InvocationTargetException, InterruptedException { + public IStatus run(IProgressMonitor monitor) { fMonitor = monitor; final Control control = traceTypeViewer.getControl(); control.getDisplay().syncExec(new Runnable() { @@ -385,54 +392,57 @@ public class ImportTraceWizardScanPage extends AbstractImportTraceWizardPage { } }); } - final TraceValidationHelper traceToScan = fTracesToScan.take(); + try { + final TraceValidationHelper traceToScan = fTracesToScan.take(); - if (!getBatchWizard().hasScanned(traceToScan)) { - getBatchWizard().addResult(traceToScan, TmfTraceType.getInstance().validate(traceToScan)); - } - validCombo = getBatchWizard().getResult(traceToScan); - if (validCombo) { - // Synched on it's parent + if (!getBatchWizard().hasScanned(traceToScan)) { + getBatchWizard().addResult(traceToScan, TmfTraceType.getInstance().validate(traceToScan)); + } + validCombo = getBatchWizard().getResult(traceToScan); + if (validCombo) { + // Synched on it's parent - getBatchWizard().getScannedTraces().addCandidate(traceToScan.getTraceType(), new File(traceToScan.getTraceToScan())); - updated = true; - } - // position++; - - if (updated) { - if (!control.isDisposed()) { - control.getDisplay().asyncExec(new Runnable() { - @Override - public void run() { - if (!control.isDisposed()) { - getMonitor().setTaskName(Messages.ImportTraceWizardPageScan_scanning + " "); //$NON-NLS-1$ - getMonitor().subTask(traceToScan.getTraceToScan()); - getMonitor().worked(1); + getBatchWizard().getScannedTraces().addCandidate(traceToScan.getTraceType(), new File(traceToScan.getTraceToScan())); + updated = true; + } + + if (updated) { + if (!control.isDisposed()) { + control.getDisplay().asyncExec(new Runnable() { + @Override + public void run() { + if (!control.isDisposed()) { + getMonitor().setTaskName(Messages.ImportTraceWizardPageScan_scanning + ' '); + getMonitor().subTask(traceToScan.getTraceToScan()); + getMonitor().worked(1); + } } } + ); } - ); } - } - final boolean editing = traceTypeViewer.isCellEditorActive(); - if (updated && !editing) - { - if (!control.isDisposed()) { - control.getDisplay().asyncExec(new Runnable() { - - @Override - public void run() { - if (!control.isDisposed()) { - if (!traceTypeViewer.isCellEditorActive()) { - traceTypeViewer.refresh(); + final boolean editing = traceTypeViewer.isCellEditorActive(); + if (updated && !editing) { + if (!control.isDisposed()) { + control.getDisplay().asyncExec(new Runnable() { + + @Override + public void run() { + if (!control.isDisposed()) { + if (!traceTypeViewer.isCellEditorActive()) { + traceTypeViewer.refresh(); + } } } - } - }); + }); + } } + } catch (InterruptedException e) { + return new Status(IStatus.CANCEL, Activator.PLUGIN_ID, new String()); } } + return Status.OK_STATUS; } } } diff --git a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/NonModalWizardDialog.java b/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/NonModalWizardDialog.java deleted file mode 100644 index 923096189c..0000000000 --- a/org.eclipse.linuxtools.tmf.ui/src/org/eclipse/linuxtools/tmf/ui/project/wizards/importtrace/NonModalWizardDialog.java +++ /dev/null @@ -1,143 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2013 Ericsson - * - * All rights reserved. This program and the accompanying materials are - * made available under the terms of the Eclipse Public License v1.0 which - * accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * Matthew Khouzam - Initial API and implementation - *******************************************************************************/ - -package org.eclipse.linuxtools.tmf.ui.project.wizards.importtrace; - -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.List; - -import org.eclipse.core.runtime.IProgressMonitor; -import org.eclipse.core.runtime.SubMonitor; -import org.eclipse.jface.operation.IRunnableWithProgress; -import org.eclipse.jface.wizard.IWizard; -import org.eclipse.jface.wizard.IWizardPage; -import org.eclipse.jface.wizard.ProgressMonitorPart; -import org.eclipse.jface.wizard.WizardDialog; -import org.eclipse.swt.layout.GridLayout; -import org.eclipse.swt.widgets.Composite; -import org.eclipse.swt.widgets.Shell; - -/** - * Non-modal wizard allows background jobs to work in tandem with modal jobs. - * - * @author Matthew Khouzam - * @since 2.0 - */ -public class NonModalWizardDialog extends WizardDialog { - - private ProgressMonitorPart fProgressMonitor; - private IWizardPage fWizardPage; - private List fHelpers = new ArrayList(); - - /** - * Creates a new non modal wizard dialog for the given wizard. - * - * @param parentShell - * the parent shell - * @param newWizard - * the wizard this dialog is working on - */ - public NonModalWizardDialog(Shell parentShell, IWizard newWizard) { - super(parentShell, newWizard); - } - - /** - * Copy constructor (be careful) - * - * @param wiz - * the Wizard to clone - */ - public NonModalWizardDialog(WizardDialog wiz) { - super(wiz.getShell(), wiz.getCurrentPage().getWizard()); - } - - @Override - public boolean close() { - for(HelperThread t : fHelpers){ - t.interrupt(); - } - for(HelperThread t : fHelpers){ - try { - t.join(100); - } catch (InterruptedException e) { - } - } - return super.close(); - } - /** - * Gets the progress monitor, can be null - * - * @return a progress monitor that can be null - */ - public IProgressMonitor getMonitor() { - return super.getProgressMonitor(); - } - - @Override - public void updateButtons() { - super.updateButtons(); - if (fProgressMonitor != null) { - fProgressMonitor.setVisible(this.getCurrentPage().equals(fWizardPage)); - fProgressMonitor.getMonitor(); - } - } - - /** - * Background job to run - * - * @param pageForJob - * the page to display the job on - * @param runnable - * the runnable that is a background job - */ - public void backgroundRun(IWizardPage pageForJob, IRunnableWithProgress runnable) { - // create progress monitor; - fWizardPage = pageForJob; - IProgressMonitor x = getProgressMonitor(); - SubMonitor y = SubMonitor.convert(x); - // make a new thread with the runnable - HelperThread h = new HelperThread(runnable, y); - - h.start(); - fHelpers.add(h); - - } - - @Override - protected ProgressMonitorPart createProgressMonitorPart(Composite composite, GridLayout pmlayout) { - fProgressMonitor = new ProgressMonitorPart(composite, pmlayout); - return fProgressMonitor; - } - - private class HelperThread extends Thread { - private IRunnableWithProgress runnable; - private IProgressMonitor monitor; - - public HelperThread(IRunnableWithProgress i, IProgressMonitor s) { - runnable = i; - monitor = s; - } - - @Override - public void run() { - try { - runnable.run(monitor); - } catch (InvocationTargetException e) { - // skip over me - } catch (InterruptedException e) { - // skip over me - } - } - } - -} -- 2.34.1