Skip to content

Commit

Permalink
TEIIDDES-2299: Improve the performance of ldap wizard checkbox selection
Browse files Browse the repository at this point in the history 
* LdapImportWizardManager
 * Stop setSynchronising from notifying that it has changed as this is
   calling update in the middle of user-selection operations. Not least
   it is simply a flag that is observed via isSynchronising()

* Ldap[Columns|Tables]Page
 * Refactor the tree viewers of these pages to be CheckboxTreeViewers as
   this avoids the expensive method of WidgetUtil.findTreeItem().
 * With the change to setSynchronising the performance of the trees is much
   improved
  • Loading branch information
@blafond
Paul Richardson authored and blafond committed Jan 27, 2015
1 parent 9b2a8fb commit 84de795
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 197 deletions.
15 changes: 4 additions & 11 deletions ...dap.ui/src/org/teiid/designer/modelgenerator/ldap/ui/wizards/LdapImportWizardManager.java
View file
Expand Up @@ -263,15 +263,14 @@ public ILdapAttributeNode newAttribute(ILdapEntryNode contextNode, Attribute att
* Add an attribute to the collection of selected attributes
*
* @param attribute
* @throws Exception
*/
public void addAttribute(ILdapAttributeNode attribute) throws Exception {
public void addAttribute(ILdapAttributeNode attribute) {
ILdapEntryNode associatedEntry = attribute.getAssociatedEntry();

// Prefer the version already in the import manager
associatedEntry = ldapEntryNodes.get(associatedEntry.hashCode());
if (associatedEntry == null)
throw new Exception(getString("noEntryForAttribute")); //$NON-NLS-1$
return;

associatedEntry.addAttribute(attribute);
notifyChanged();
Expand All @@ -281,15 +280,14 @@ public void addAttribute(ILdapAttributeNode attribute) throws Exception {
* Removes an attribute from the set of selected attributes
*
* @param attribute
* @throws Exception
*/
public void removeAttribute(ILdapAttributeNode attribute) throws Exception {
public void removeAttribute(ILdapAttributeNode attribute) {
ILdapEntryNode associatedEntry = attribute.getAssociatedEntry();

// Prefer the version already in the import manager
associatedEntry = ldapEntryNodes.get(associatedEntry.hashCode());
if (associatedEntry == null)
throw new Exception(getString("noEntryForAttribute")); //$NON-NLS-1$
return;

if (associatedEntry.removeAttribute(attribute))
notifyChanged();
Expand Down Expand Up @@ -449,11 +447,6 @@ public boolean isSynchronising() {
* @param synchronising
*/
public void setSynchronising(boolean synchronising) {
boolean oldSync = this.synchronising;
this.synchronising = synchronising;

// Only notify if sync value has actually changed
if (!synchronising && oldSync)
notifyChanged();
}
}
137 changes: 49 additions & 88 deletions .../src/org/teiid/designer/modelgenerator/ldap/ui/wizards/pages/columns/LdapColumnsPage.java
View file
Expand Up @@ -9,15 +9,19 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import org.eclipse.jface.dialogs.IMessageProvider;
import org.eclipse.jface.layout.GridDataFactory;
import org.eclipse.jface.layout.GridLayoutFactory;
import org.eclipse.jface.viewers.CheckStateChangedEvent;
import org.eclipse.jface.viewers.CheckboxTreeViewer;
import org.eclipse.jface.viewers.ICheckStateListener;
import org.eclipse.jface.viewers.IContentProvider;
import org.eclipse.jface.viewers.ILabelProvider;
import org.eclipse.jface.viewers.ISelection;
import org.eclipse.jface.viewers.ISelectionChangedListener;
import org.eclipse.jface.viewers.IStructuredSelection;
import org.eclipse.jface.viewers.ITreeViewerListener;
import org.eclipse.jface.viewers.TreeExpansionEvent;
import org.eclipse.jface.viewers.TreeViewer;
import org.eclipse.jface.viewers.SelectionChangedEvent;
import org.eclipse.jface.wizard.WizardPage;
import org.eclipse.swt.SWT;
import org.eclipse.swt.custom.CLabel;
Expand All @@ -30,27 +34,23 @@
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.graphics.Rectangle;
import org.eclipse.swt.layout.GridData;
import org.eclipse.swt.widgets.Button;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Text;
import org.eclipse.swt.widgets.Tree;
import org.eclipse.swt.widgets.TreeItem;
import org.teiid.core.designer.event.IChangeListener;
import org.teiid.core.designer.event.IChangeNotifier;
import org.teiid.designer.modelgenerator.ldap.ui.ModelGeneratorLdapUiConstants;
import org.teiid.designer.modelgenerator.ldap.ui.ModelGeneratorLdapUiPlugin;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.ILdapAttributeNode;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.ILdapEntryNode;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.LdapImportWizard;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.LdapImportWizardManager;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.LdapPageUtils;
import org.teiid.designer.ui.common.InternalUiConstants;
import org.teiid.designer.ui.common.util.WidgetFactory;
import org.teiid.designer.ui.common.util.WidgetUtil;
import org.teiid.designer.ui.common.util.WizardUtil;

/**
Expand All @@ -71,7 +71,7 @@ public class LdapColumnsPage extends WizardPage

private SashForm splitter;
private ViewForm objsView;
private TreeViewer treeViewer;
private CheckboxTreeViewer treeViewer;

private Text columnNameText;

Expand Down Expand Up @@ -135,47 +135,27 @@ private void nodeSelected( final Object node ) {
}
}

private void treeItemChecked(TreeItem item, boolean selected ) {
// Update check boxes of item
if (item == null) {
return;
}

if (! (item.getData() instanceof ILdapAttributeNode))
private void nodeChecked(ILdapAttributeNode attrNode, boolean selected) {
if (attrNode == null)
return;

final ILdapAttributeNode attributeNode = (ILdapAttributeNode) item.getData();

try {
if (selected) {
importManager.addAttribute(attributeNode);
} else {
importManager.removeAttribute(attributeNode);
}
importManager.setSynchronising(true);

// Ensure the treeviewer has completed all events before updating the node
treeViewer.getControl().getDisplay().asyncExec(new Runnable() {
@Override
public void run() {
treeViewer.update(attributeNode, null);
}
});
} catch(Exception ex) {
ModelGeneratorLdapUiConstants.UTIL.log(ex);
WizardUtil.setPageComplete(this, ex.getLocalizedMessage(), IMessageProvider.ERROR);
if (selected) {
importManager.addAttribute(attrNode);
} else {
importManager.removeAttribute(attrNode);
}

importManager.setSynchronising(false);
}

private void deselectAllButtonSelected() {
Collection<ILdapAttributeNode> oldSelection = new ArrayList<ILdapAttributeNode>();
oldSelection.addAll(importManager.getSelectedAttributes());

for (ILdapAttributeNode attributeNode : oldSelection) {
TreeItem treeItem = WidgetUtil.findTreeItem(attributeNode, treeViewer);
if (treeItem == null)
continue;

treeItemChecked(treeItem, false);
for (ILdapAttributeNode node : oldSelection) {
treeViewer.setChecked(node, false);
}
}

Expand Down Expand Up @@ -218,55 +198,48 @@ public Point computeSize( final int widthHint,
this.objsView.setTopLeft(ldapLabel);

// Add contents to view form
this.treeViewer = new TreeViewer(this.objsView, SWT.SINGLE | SWT.BORDER);
this.treeViewer = new CheckboxTreeViewer(this.objsView, SWT.SINGLE | SWT.BORDER);
this.treeViewer.setUseHashlookup(true);
final Tree tree = this.treeViewer.getTree();
this.objsView.setContent(tree);

this.treeViewer.setContentProvider(contentProvider);
this.treeViewer.setLabelProvider(labelProvider);

/*
* Mouse down listener for simulating the checkbox selection.
*
* This is used instead of the SWT checkbox tree viewer which has really
* awful performance on item selection.
*/
this.treeViewer.getTree().addMouseListener(new MouseAdapter() {

this.treeViewer.addCheckStateListener(new ICheckStateListener() {
@Override
public void mouseDoubleClick(MouseEvent e) {
IStructuredSelection selection = (IStructuredSelection) treeViewer.getSelection();
if (selection.isEmpty())
public void checkStateChanged(CheckStateChangedEvent event) {
Object element = event.getElement();
if (! (element instanceof ILdapAttributeNode))
return;

Object node = selection.getFirstElement();
treeViewer.setExpandedState(node, ! treeViewer.getExpandedState(node));
ILdapAttributeNode node = (ILdapAttributeNode) element;
nodeChecked(node, event.getChecked());
}
});

this.treeViewer.addSelectionChangedListener(new ISelectionChangedListener() {

@Override
public void mouseDown(MouseEvent e) {
TreeItem[] selectedItems = treeViewer.getTree().getSelection();
if (selectedItems.length == 0)
public void selectionChanged(SelectionChangedEvent event) {
ISelection selection = event.getSelection();
if (selection.isEmpty())
return;

if (! (selection instanceof IStructuredSelection))
return;

IStructuredSelection sselection = (IStructuredSelection) selection;
importManager.setSynchronising(true);

try {
for (TreeItem treeItem : selectedItems) {
if (treeItem.getImage() == null)
Iterator iterator = sselection.iterator();
while(iterator.hasNext()) {
Object object = iterator.next();
if (! (object instanceof ILdapAttributeNode))
continue;

if (treeItem.getData() instanceof ILdapAttributeNode) {
ILdapAttributeNode attributeNode = (ILdapAttributeNode)treeItem.getData();
Rectangle imageRec = treeItem.getImageBounds(0);

if (imageRec.contains(e.x, e.y)) {
treeItemChecked(treeItem, !importManager.attributeSelected(attributeNode));
}
}

nodeSelected(treeItem.getData());
nodeSelected(object);
}
} finally {
// Turns off synchronising and calls state changed
Expand All @@ -275,26 +248,16 @@ public void mouseDown(MouseEvent e) {
}
});

// Add listener to select node when expanded/collapsed
this.treeViewer.addTreeListener(new ITreeViewerListener() {

@Override
public void treeCollapsed(TreeExpansionEvent e) {
// Nothing to do
}
this.treeViewer.getTree().addMouseListener(new MouseAdapter() {

@Override
public void treeExpanded(TreeExpansionEvent e) {
Object element = e.getElement();
TreeItem treeItem = WidgetUtil.findTreeItem(element, treeViewer);
if (treeItem == null)
return;

if (treeItem.getData() instanceof ILdapEntryNode)
public void mouseDoubleClick(MouseEvent e) {
IStructuredSelection selection = (IStructuredSelection) treeViewer.getSelection();
if (selection.isEmpty())
return;

ILdapAttributeNode attributeNode = (ILdapAttributeNode) treeItem.getData();
treeItemChecked(treeItem, importManager.attributeSelected(attributeNode));
Object node = selection.getFirstElement();
treeViewer.setExpandedState(node, ! treeViewer.getExpandedState(node));
}
});

Expand Down Expand Up @@ -419,10 +382,8 @@ public void setVisible(boolean visible) {
if (this.importManager.getConnectionProfile() == null)
return;

if (this.treeViewer.getInput() == null) {
this.treeViewer.setInput(importManager);
this.treeViewer.expandToLevel(2);
}
this.treeViewer.setInput(importManager);
this.treeViewer.expandToLevel(2);

setPageStatus();
}
Expand Down
14 changes: 0 additions & 14 deletions ...g/teiid/designer/modelgenerator/ldap/ui/wizards/pages/columns/LdapEntryLabelProvider.java
View file
Expand Up @@ -7,7 +7,6 @@
*/
package org.teiid.designer.modelgenerator.ldap.ui.wizards.pages.columns;

import org.eclipse.swt.graphics.Image;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.AbstractLdapLabelProvider;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.ILdapAttributeNode;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.ILdapEntryNode;
Expand Down Expand Up @@ -37,17 +36,4 @@ public String getText(Object element) {

return null;
}

@Override
public Image getImage(Object element) {
if (element instanceof ILdapAttributeNode) {
ILdapAttributeNode attribute = (ILdapAttributeNode) element;
if (getImportManager().attributeSelected(attribute))
return CHECKED_IMAGE;
else
return UNCHECKED_IMAGE;
}

return null;
}
}
16 changes: 0 additions & 16 deletions ...eiid/designer/modelgenerator/ldap/ui/wizards/pages/table/LdapConnectionLabelProvider.java
View file
Expand Up @@ -7,7 +7,6 @@
*/
package org.teiid.designer.modelgenerator.ldap.ui.wizards.pages.table;

import org.eclipse.swt.graphics.Image;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.AbstractLdapLabelProvider;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.ILdapEntryNode;
import org.teiid.designer.modelgenerator.ldap.ui.wizards.LdapImportWizardManager;
Expand Down Expand Up @@ -39,19 +38,4 @@ public String getText(Object element) {

return null;
}

@Override
public Image getImage(Object element) {
if (! (element instanceof ILdapEntryNode))
return null;

ILdapEntryNode node = (ILdapEntryNode) element;
if (node.isRoot())
return null;

if (getImportManager().entrySelected(node))
return CHECKED_IMAGE;
else
return UNCHECKED_IMAGE;
}
}

0 comments on commit 84de795

Please sign in to comment.