Skip to content

Commit

Permalink
TEIIDES-2299: Improve performance bottle necks of LDAP wizard
Browse files Browse the repository at this point in the history
* LdapImportWizardManager
 * Sets a synchronise flag in Import Manager to reduce the number of
   notifyChanged calls that are passed to the page listeners.
 * setSynchronise(false) resets the flag and calls notifyChanged itself
 * notifyError method to pass error messages from tree content providers
   up to the pages. Errors are currently logged but only in the console.

* *Page
 * Fixes theme issues with the backgrounds of components

* Ldap[Columns|Table]Page
 * Reduces bottlenecks by settings a dirty flag in the modifyTextListeners
   and not calling notifyChanged (which calls setPageStatus)
 * Adds validate buttons for a single call to notifyChanged/setPageStatus

* Refresh icon appears to be missing from eclipse TP so adds it to icons
  folder

* LdapPageUtils
 * Make the wizard colours consistent across gtk themes since Group
   components appear to have a white background in some themes and not
   the colour of its parent composite
  • Loading branch information
Paul Richardson authored and blafond committed Dec 1, 2014
1 parent 53ddacc commit 9aa09c4
Show file tree
Hide file tree
Showing 11 changed files with 257 additions and 59 deletions.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ interface Images {
String WIZARD_BANNER = WIZBAN + "ldap_banner_icon.png"; //$NON-NLS-1$
String CHECKED_CHECKBOX = CTOOL16 + "checkbox-checked.png"; //$NON-NLS-1$
String UNCHECKED_CHECKBOX = CTOOL16 + "checkbox-unchecked.png"; //$NON-NLS-1$
String LDAP_REFRESH_ICON = CTOOL16 + "refresh.gif"; //$NON-NLS-1$
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ LdapTablesPage_detailTableSourceSuffixLabel=Table Source Name Suffix
LdapTablesPage_detailTableSourceSuffixToolTip=The source entry can be further filtered using scoping criteria, in the form '?search_scope?objectClass_name', eg. ?SUBTREE_SCOPE?inetOrgPerson. The '?'s are required if a value is being entered in this field
LdapTablesPage_noSourceModelTables=No source model tables have been selected
LdapTablesPage_invalidSourceNameSuffix=A table's source name suffix is invalid. If populated at all, it must contain 2 '?'s in the form '?search_scope?objectClass_name', eg. ?SUBTREE_SCOPE?inetOrgPerson
LdapTablesPage_needsValidating=Entries require validation. Please click the validate button.
LdapTablesPage_validateButtonLabel=Validate

#=================================================================================================================================
# LdapColumnsPage
Expand All @@ -104,6 +106,8 @@ LdapColumnsPage_detailColumnDVCountLabel=Column Distinct Value Count
LdapColumnsPage_detailColumnNVCountLabel=Column Null Value Count
LdapColumnsPage_detailMaxValueLabel=Column Length
LdapColumnsPage_sourceColumnsIncomplete=Not all selected source model tables contain columns
LdapColumnsPage_needsValidating=Attributes require validation. Please click the validate button.
LdapColumnsPage_validateButtonLabel=Validate

#=================================================================================================================================
# LdapImportWizardManager
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ public class LdapImportWizardManager implements IChangeNotifier {

private Collection<IChangeListener> listeners;

// Transient field for communicating exceptions from tree providers
// to the pages on which they are resident
private Exception error = null;

// Flag to determine whether wizard is synchronising
// and manager should avoid notifying pages too frequently
private boolean synchronising;

/**
* The set of {@link LdapEntryNode}s selected to be tables
* in the source model
Expand Down Expand Up @@ -386,6 +394,15 @@ void createModel() {
* Notify listeners of a change of state
*/
public void notifyChanged() {
if (isSynchronising()) {
/*
* In middle of major operation so protect performance
* by notifying of state needlessly. When synchronising
* is switched back on then a notify change should be called.
*/
return;
}

for( IChangeListener listener: this.listeners ) {
listener.stateChanged(this);
}
Expand All @@ -400,4 +417,43 @@ public void addChangeListener(IChangeListener listener) {
public void removeChangeListener(IChangeListener listener) {
this.listeners.remove(listener);
}

/**
* @return the error
*/
public Exception getError() {
return this.error;
}

/**
* Notify listeners of any generated errors.
* Note, the error field is transient in that its
* immediately nullified after calling this.
*
* @param error
*/
public void notifyError(Exception error) {
this.error = error;
notifyChanged();
this.error = null;
}

/**
* @return synchronising
*/
public boolean isSynchronising() {
return this.synchronising;
}

/**
* @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();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* JBoss, Home of Professional Open Source.
*
* See the LEGAL.txt file distributed with this work for information regarding copyright ownership and licensing.
*
* See the AUTHORS.txt file distributed with this work for a full listing of individual contributors.
*/
package org.teiid.designer.modelgenerator.ldap.ui.wizards;

import org.eclipse.swt.SWT;
import org.eclipse.swt.widgets.Composite;
import org.eclipse.swt.widgets.Control;

/**
* Random utility methods for use with LDAP Definition Wizard pages
*/
public class LdapPageUtils {

/**
* Set the background colour of the control to the same as the base
*
* @param control
* @param base
*/
public static void setBackground(Control control, Composite base) {
if (control == null || base == null)
return;

control.setBackground(base.getBackground());
}

/**
* Set the background colour of the control to grey
*
* @param control
*/
public static void greyBackground(Control control) {
if (control == null)
return;

control.setBackground(control.getDisplay().getSystemColor(SWT.COLOR_WIDGET_LIGHT_SHADOW));
}

/**
* Set the foreground colour of the control to blue
*
* @param control
*/
public static void blueForeground(Control control) {
if (control == null)
return;

control.setForeground(control.getDisplay().getSystemColor(SWT.COLOR_DARK_BLUE));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@
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.Display;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Text;
Expand All @@ -47,6 +47,7 @@
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;
Expand Down Expand Up @@ -82,7 +83,9 @@ public class LdapColumnsPage extends WizardPage

private Text columnMaxValueText;

private boolean synchronising;
private Button validateButton;

private boolean dirty;

/**
* Constructs the page with the provided import manager
Expand All @@ -105,11 +108,12 @@ private static String getString(String key, Object... properties) {
return ModelGeneratorLdapUiConstants.UTIL.getString(LdapColumnsPage.class.getSimpleName() + "_" + key, properties); //$NON-NLS-1$
}

/**
* @return the synchronising
*/
private boolean isSynchronising() {
return this.synchronising;
private boolean isDirty() {
return dirty;
}

private void setDirty(boolean dirty) {
this.dirty = dirty;
}

private void nodeSelected( final Object node ) {
Expand Down Expand Up @@ -179,9 +183,8 @@ private void setNonEditable(Text control) {
if (control == null)
return;

Display display = control.getDisplay();
control.setForeground(display.getSystemColor(SWT.COLOR_DARK_BLUE));
control.setBackground(display.getSystemColor(SWT.COLOR_WIDGET_LIGHT_SHADOW));
LdapPageUtils.blueForeground(control);
LdapPageUtils.greyBackground(control);
control.setEditable(false);
}

Expand Down Expand Up @@ -247,20 +250,27 @@ public void mouseDown(MouseEvent e) {
if (selectedItems.length == 0)
return;

for (TreeItem treeItem : selectedItems) {
if(treeItem.getImage() == null)
continue;
importManager.setSynchronising(true);

try {
for (TreeItem treeItem : selectedItems) {
if (treeItem.getImage() == null)
continue;

if (treeItem.getData() instanceof ILdapAttributeNode) {
ILdapAttributeNode attributeNode = (ILdapAttributeNode) treeItem.getData();
Rectangle imageRec = treeItem.getImageBounds(0);
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));
if (imageRec.contains(e.x, e.y)) {
treeItemChecked(treeItem, !importManager.attributeSelected(attributeNode));
}
}
}

nodeSelected(treeItem.getData());
nodeSelected(treeItem.getData());
}
} finally {
// Turns off synchronising and calls state changed
importManager.setSynchronising(false);
}
}
});
Expand Down Expand Up @@ -291,6 +301,7 @@ public void treeExpanded(TreeExpansionEvent e) {
ViewForm detailsView = new ViewForm(this.splitter, SWT.BORDER);
Group detailsGroup = WidgetFactory.createGroup(detailsView, getString("columnAttributesTitle"), SWT.NONE, 2); //$NON-NLS-1$
GridLayoutFactory.fillDefaults().numColumns(2).margins(10, 10).applyTo(detailsGroup);
LdapPageUtils.setBackground(detailsGroup, this.splitter);
GridDataFactory.fillDefaults().grab(true, false).applyTo(detailsGroup);

detailsView.setContent(detailsGroup);
Expand All @@ -303,8 +314,7 @@ public void treeExpanded(TreeExpansionEvent e) {
columnNameText.addModifyListener(new ModifyListener() {
@Override
public void modifyText(ModifyEvent e) {
if (isSynchronising())
return;
setDirty(true);

IStructuredSelection selection = (IStructuredSelection) treeViewer.getSelection();
if (selection.isEmpty())
Expand All @@ -316,7 +326,6 @@ public void modifyText(ModifyEvent e) {
String colNameText = columnNameText.getText();
if (! colNameText.equals(node.getLabel())) {
node.setLabel(colNameText);
notifyChanged();
}
}
}
Expand Down Expand Up @@ -350,6 +359,18 @@ public void modifyText(ModifyEvent e) {
GridDataFactory.fillDefaults().grab(true, false).applyTo(columnMaxValueText);
setNonEditable(columnMaxValueText);

validateButton = new Button(detailsGroup, SWT.PUSH);
validateButton.setText(getString("validateButtonLabel")); //$NON-NLS-1$
GridDataFactory.swtDefaults().span(2, 1).align(GridData.END, GridData.CENTER).applyTo(validateButton);
validateButton.addSelectionListener(new SelectionAdapter() {

@Override
public void widgetSelected(final SelectionEvent event) {
setDirty(false);
notifyChanged();
}
});

this.splitter.setWeights(SPLITTER_WEIGHTS);

final Button deselectAllButton = WidgetFactory.createButton(pg, InternalUiConstants.Widgets.DESELECT_ALL_BUTTON);
Expand All @@ -366,6 +387,20 @@ public void widgetSelected(final SelectionEvent event) {
* Performs validation and sets the page status.
*/
private void setPageStatus() {
if (getControl() != null && !getControl().isVisible())
return;

if (isDirty()) {
WizardUtil.setPageComplete(this, getString("needsValidating"), IMessageProvider.ERROR); //$NON-NLS-1$
return;
}

if (this.importManager.getError() != null) {
ModelGeneratorLdapUiConstants.UTIL.log(this.importManager.getError());
WizardUtil.setPageComplete(this, this.importManager.getError().getLocalizedMessage(), IMessageProvider.ERROR);
return;
}

if (! this.importManager.hasAttributesForEachSelectedEntry()) {
WizardUtil.setPageComplete(this, getString("sourceColumnsIncomplete"), IMessageProvider.ERROR); //$NON-NLS-1$
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public Object[] getChildren(Object parentElement) {
return childAttributes.toArray();

} catch (NamingException ex) {
getImportManager().notifyError(ex);
ModelGeneratorLdapUiConstants.UTIL.log(ex);
return EMPTY_ARRAY;
}
Expand Down

0 comments on commit 9aa09c4

Please sign in to comment.