You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by se...@apache.org on 2020/03/16 09:27:03 UTC

[directory-studio] branch master updated: DIRSTUDIO-1247: Fix high memory usage in clipboard code

This is an automated email from the ASF dual-hosted git repository.

seelmann pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/directory-studio.git


The following commit(s) were added to refs/heads/master by this push:
     new 5e1754a  DIRSTUDIO-1247: Fix high memory usage in clipboard code
5e1754a is described below

commit 5e1754a28ebeac41925cff3c488ea589e427271b
Author: Stefan Seelmann <ma...@stefan-seelmann.de>
AuthorDate: Mon Mar 16 10:24:43 2020 +0100

    DIRSTUDIO-1247: Fix high memory usage in clipboard code
    
    * Remove inefficient and likely useless function to paste connection via an LDAP URL
    * Improve usage of clipboard API to enable/disable paste action and determine label, check for available types first before reading the actual content from clipboard
---
 .../directory/studio/common/ui/ClipboardUtils.java |  79 ++++++++++++++++
 .../studio/connection/ui/actions/PasteAction.java  | 101 ++-------------------
 .../ldapbrowser/common/actions/PasteAction.java    |  26 ------
 .../entryeditor/EntryEditorPasteAction.java        |   3 +-
 .../ldapbrowser/ui/actions/BrowserPasteAction.java |   7 +-
 .../ldapbrowser/ui/actions/GotoDnAction.java       |  27 +-----
 .../ui/actions/GotoDnNavigateMenuAction.java       |  32 +------
 .../SearchResultEditorPasteAction.java             |   3 +-
 8 files changed, 97 insertions(+), 181 deletions(-)

diff --git a/plugins/common.ui/src/main/java/org/apache/directory/studio/common/ui/ClipboardUtils.java b/plugins/common.ui/src/main/java/org/apache/directory/studio/common/ui/ClipboardUtils.java
new file mode 100644
index 0000000..98f129d
--- /dev/null
+++ b/plugins/common.ui/src/main/java/org/apache/directory/studio/common/ui/ClipboardUtils.java
@@ -0,0 +1,79 @@
+package org.apache.directory.studio.common.ui;
+
+
+import java.util.function.Function;
+
+import org.eclipse.swt.dnd.Clipboard;
+import org.eclipse.swt.dnd.Transfer;
+import org.eclipse.swt.widgets.Display;
+
+
+public class ClipboardUtils
+{
+
+    /**
+     * Retrieve the data of the specified type currently available on the system clipboard.
+     *
+     * @param transfer the transfer agent for the type of data being requested
+     * @return the data obtained from the clipboard or null if no data of this type is available
+     */
+    public static Object getFromClipboard( Transfer transfer )
+    {
+        return getFromClipboard( transfer, Object.class );
+    }
+
+
+    public static <T> T getFromClipboard( Transfer transfer, Class<T> type )
+    {
+        return withClipboard( clipboard -> {
+            if ( isAvailable( transfer, clipboard ) )
+            {
+                Object contents = clipboard.getContents( transfer );
+                if ( contents != null && type.isAssignableFrom( contents.getClass() ) )
+                {
+                    return type.cast( contents );
+                }
+            }
+            return null;
+        } );
+    }
+
+
+    public static boolean isAvailable( Transfer transfer )
+    {
+        return withClipboard( clipboard -> {
+            return isAvailable( transfer, clipboard );
+        } );
+    }
+
+
+    private static Boolean isAvailable( Transfer transfer, Clipboard clipboard )
+    {
+        for ( org.eclipse.swt.dnd.TransferData transferData : clipboard.getAvailableTypes() )
+        {
+            if ( transfer.isSupportedType( transferData ) )
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
+
+    private static <T> T withClipboard( Function<Clipboard, T> fn )
+    {
+        Clipboard clipboard = null;
+        try
+        {
+            clipboard = new Clipboard( Display.getCurrent() );
+            return fn.apply( clipboard );
+        }
+        finally
+        {
+            if ( clipboard != null )
+            {
+                clipboard.dispose();
+            }
+        }
+    }
+}
diff --git a/plugins/connection.ui/src/main/java/org/apache/directory/studio/connection/ui/actions/PasteAction.java b/plugins/connection.ui/src/main/java/org/apache/directory/studio/connection/ui/actions/PasteAction.java
index 238ea84..3be2ac0 100644
--- a/plugins/connection.ui/src/main/java/org/apache/directory/studio/connection/ui/actions/PasteAction.java
+++ b/plugins/connection.ui/src/main/java/org/apache/directory/studio/connection/ui/actions/PasteAction.java
@@ -24,24 +24,14 @@ package org.apache.directory.studio.connection.ui.actions;
 import java.util.ArrayList;
 import java.util.List;
 
-import org.apache.commons.lang3.StringUtils;
-import org.apache.directory.api.ldap.model.exception.LdapURLEncodingException;
-import org.apache.directory.api.ldap.model.url.LdapUrl;
+import org.apache.directory.studio.common.ui.ClipboardUtils;
 import org.apache.directory.studio.connection.core.Connection;
-import org.apache.directory.studio.connection.core.ConnectionCoreConstants;
 import org.apache.directory.studio.connection.core.ConnectionCorePlugin;
 import org.apache.directory.studio.connection.core.ConnectionFolder;
 import org.apache.directory.studio.connection.core.ConnectionFolderManager;
 import org.apache.directory.studio.connection.core.ConnectionManager;
-import org.apache.directory.studio.connection.core.ConnectionParameter;
-import org.apache.directory.studio.connection.ui.ConnectionParameterPage;
-import org.apache.directory.studio.connection.ui.ConnectionParameterPageManager;
 import org.apache.directory.studio.connection.ui.dnd.ConnectionTransfer;
 import org.eclipse.jface.resource.ImageDescriptor;
-import org.eclipse.swt.dnd.Clipboard;
-import org.eclipse.swt.dnd.TextTransfer;
-import org.eclipse.swt.dnd.Transfer;
-import org.eclipse.swt.widgets.Display;
 import org.eclipse.ui.ISharedImages;
 import org.eclipse.ui.IWorkbenchCommandConstants;
 import org.eclipse.ui.PlatformUI;
@@ -114,8 +104,7 @@ public class PasteAction extends StudioAction
      */
     public boolean isEnabled()
     {
-        return this.getFromClipboard( ConnectionTransfer.getInstance() ) != null
-            || !getConnectionsByLdapUrl().isEmpty();
+        return ClipboardUtils.isAvailable( ConnectionTransfer.getInstance() );
     }
 
 
@@ -177,9 +166,8 @@ public class PasteAction extends StudioAction
     {
         List<Connection> connections = new ArrayList<>();
 
-        // first check for Connection objects in the clipboard
-        Object content = getFromClipboard( ConnectionTransfer.getInstance() );
-        
+        Object content = ClipboardUtils.getFromClipboard( ConnectionTransfer.getInstance() );
+
         if ( content instanceof Object[] )
         {
             for ( Object object : ( Object[] )content )
@@ -191,58 +179,6 @@ public class PasteAction extends StudioAction
             }
         }
 
-        // if there are no Connection objects in the clipboard
-        // then check for LDAP URLs
-        if ( connections.isEmpty() )
-        {
-            List<Connection> connectionByLdapUrl = getConnectionsByLdapUrl();
-            connections.addAll( connectionByLdapUrl );
-        }
-
-        return connections;
-    }
-
-
-    private List<Connection> getConnectionsByLdapUrl()
-    {
-        List<Connection> connections = new ArrayList<>();
-
-        Object content = getFromClipboard( TextTransfer.getInstance() );
-        
-        if ( content instanceof String )
-        {
-            ConnectionParameterPage[] connectionParameterPages = ConnectionParameterPageManager
-                .getConnectionParameterPages();
-
-            String[] lines = ( ( String ) content ).split( ConnectionCoreConstants.LINE_SEPARATOR );
-            
-            for ( String line : lines )
-            {
-                line = line.trim();
-                
-                if ( StringUtils.isNotEmpty( line ) )
-                {
-                    try
-                    {
-                        LdapUrl ldapUrl = new LdapUrl( line );
-                        ConnectionParameter parameter = new ConnectionParameter();
-                        
-                        for ( ConnectionParameterPage connectionParameterPage : connectionParameterPages )
-                        {
-                            connectionParameterPage.mergeLdapUrlToParameters( ldapUrl, parameter );
-                        }
-                        
-                        Connection connection = new Connection( parameter );
-                        connections.add( connection );
-                    }
-                    catch ( LdapURLEncodingException e )
-                    {
-                        // this was a string that doesn't represent an LDAP URL, ignore
-                    }
-                }
-            }
-        }
-
         return connections;
     }
 
@@ -256,8 +192,8 @@ public class PasteAction extends StudioAction
     {
         List<ConnectionFolder> folders = new ArrayList<>();
 
-        Object content = this.getFromClipboard( ConnectionTransfer.getInstance() );
-        
+        Object content = ClipboardUtils.getFromClipboard( ConnectionTransfer.getInstance() );
+
         if ( content instanceof Object[] )
         {
             for ( Object object : ( Object[] ) content )
@@ -272,29 +208,4 @@ public class PasteAction extends StudioAction
         return folders;
     }
 
-
-    /**
-     * Retrieve the data of the specified type currently available on the system clipboard.
-     *
-     * @param dataType the transfer agent for the type of data being requested
-     * @return the data obtained from the clipboard or null if no data of this type is available
-     */
-    protected Object getFromClipboard( Transfer dataType )
-    {
-        Clipboard clipboard = null;
-        
-        try
-        {
-            clipboard = new Clipboard( Display.getCurrent() );
-            
-            return clipboard.getContents( dataType );
-        }
-        finally
-        {
-            if ( clipboard != null )
-            {
-                clipboard.dispose();
-            }
-        }
-    }
 }
diff --git a/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/actions/PasteAction.java b/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/actions/PasteAction.java
index 53a8c97..f121f11 100644
--- a/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/actions/PasteAction.java
+++ b/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/actions/PasteAction.java
@@ -22,9 +22,6 @@ package org.apache.directory.studio.ldapbrowser.common.actions;
 
 
 import org.eclipse.jface.resource.ImageDescriptor;
-import org.eclipse.swt.dnd.Clipboard;
-import org.eclipse.swt.dnd.Transfer;
-import org.eclipse.swt.widgets.Display;
 import org.eclipse.ui.ISharedImages;
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.texteditor.IWorkbenchActionDefinitionIds;
@@ -63,27 +60,4 @@ public abstract class PasteAction extends BrowserAction
         return IWorkbenchActionDefinitionIds.PASTE;
     }
 
-
-    /**
-     * Retrieve the data of the specified type currently available on the system clipboard.
-     *
-     * @param dataType
-     *      the transfer agent for the type of data being requested
-     * @return
-     *      the data obtained from the clipboard or null if no data of this type is available
-     */
-    protected Object getFromClipboard( Transfer dataType )
-    {
-        Clipboard clipboard = null;
-        try
-        {
-            clipboard = new Clipboard( Display.getCurrent() );
-            return clipboard.getContents( dataType );
-        }
-        finally
-        {
-            if ( clipboard != null )
-                clipboard.dispose();
-        }
-    }
 }
diff --git a/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/widgets/entryeditor/EntryEditorPasteAction.java b/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/widgets/entryeditor/EntryEditorPasteAction.java
index 7481137..d371ef9 100644
--- a/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/widgets/entryeditor/EntryEditorPasteAction.java
+++ b/plugins/ldapbrowser.common/src/main/java/org/apache/directory/studio/ldapbrowser/common/widgets/entryeditor/EntryEditorPasteAction.java
@@ -21,6 +21,7 @@
 package org.apache.directory.studio.ldapbrowser.common.widgets.entryeditor;
 
 
+import org.apache.directory.studio.common.ui.ClipboardUtils;
 import org.apache.directory.studio.ldapbrowser.common.actions.PasteAction;
 import org.apache.directory.studio.ldapbrowser.common.dnd.ValuesTransfer;
 import org.apache.directory.studio.ldapbrowser.core.model.AttributeHierarchy;
@@ -114,7 +115,7 @@ public class EntryEditorPasteAction extends PasteAction
     {
         if ( ( getInput() instanceof IEntry ) || ( getInput() instanceof AttributeHierarchy ) )
         {
-            Object content = getFromClipboard( ValuesTransfer.getInstance() );
+            Object content = ClipboardUtils.getFromClipboard( ValuesTransfer.getInstance() );
             
             if ( content instanceof IValue[] )
             {
diff --git a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/BrowserPasteAction.java b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/BrowserPasteAction.java
index 83bf2e8..0aa809a 100644
--- a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/BrowserPasteAction.java
+++ b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/BrowserPasteAction.java
@@ -22,6 +22,7 @@ package org.apache.directory.studio.ldapbrowser.ui.actions;
 
 
 import org.apache.directory.api.ldap.model.message.SearchScope;
+import org.apache.directory.studio.common.ui.ClipboardUtils;
 import org.apache.directory.studio.connection.core.Utils;
 import org.apache.directory.studio.ldapbrowser.common.BrowserCommonConstants;
 import org.apache.directory.studio.ldapbrowser.common.actions.PasteAction;
@@ -290,7 +291,7 @@ public class BrowserPasteAction extends PasteAction
             && getSelectedEntries().length == 1 )
         {
 
-            Object content = this.getFromClipboard( EntryTransfer.getInstance() );
+            Object content = ClipboardUtils.getFromClipboard( EntryTransfer.getInstance() );
             if ( content instanceof IEntry[] )
             {
                 IEntry[] entries = ( IEntry[] ) content;
@@ -315,7 +316,7 @@ public class BrowserPasteAction extends PasteAction
             + getSelectedConnections().length + getSelectedAttributes().length + getSelectedValues().length == 0
             && ( getSelectedSearches().length + getSelectedBrowserViewCategories().length > 0 ) )
         {
-            Object content = this.getFromClipboard( SearchTransfer.getInstance() );
+            Object content = ClipboardUtils.getFromClipboard( SearchTransfer.getInstance() );
             if ( content instanceof ISearch[] )
             {
                 ISearch[] searches = ( ISearch[] ) content;
@@ -343,7 +344,7 @@ public class BrowserPasteAction extends PasteAction
             || ( getSelectedAttributes().length + getSelectedValues().length + getSelectedEntries().length
                 + getSelectedBookmarks().length + getSelectedSearches().length + getSelectedConnections().length == 0 && ( getSelectedSearchResults().length == 1 ) ) )
         {
-            Object content = this.getFromClipboard( ValuesTransfer.getInstance() );
+            Object content = ClipboardUtils.getFromClipboard( ValuesTransfer.getInstance() );
             if ( content instanceof IValue[] )
             {
                 IValue[] values = ( IValue[] ) content;
diff --git a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnAction.java b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnAction.java
index b89807e..ac96e54 100644
--- a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnAction.java
+++ b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnAction.java
@@ -22,6 +22,7 @@ package org.apache.directory.studio.ldapbrowser.ui.actions;
 
 
 import org.apache.directory.api.ldap.model.name.Dn;
+import org.apache.directory.studio.common.ui.ClipboardUtils;
 import org.apache.directory.studio.connection.core.Utils;
 import org.apache.directory.studio.ldapbrowser.common.dialogs.DnDialog;
 import org.apache.directory.studio.ldapbrowser.common.dialogs.TextDialog;
@@ -29,9 +30,7 @@ import org.apache.directory.studio.ldapbrowser.core.model.IBrowserConnection;
 import org.apache.directory.studio.ldapbrowser.ui.BrowserUIConstants;
 import org.apache.directory.studio.ldapbrowser.ui.BrowserUIPlugin;
 import org.eclipse.jface.resource.ImageDescriptor;
-import org.eclipse.swt.dnd.Clipboard;
 import org.eclipse.swt.dnd.TextTransfer;
-import org.eclipse.swt.widgets.Display;
 
 
 /**
@@ -95,7 +94,7 @@ public class GotoDnAction extends LocateInDitAction
         {
             IBrowserConnection conn = ( IBrowserConnection ) getInput();
 
-            Dn dn = Utils.getLdapDn( getStringFromClipboard() );
+            Dn dn = Utils.getLdapDn( ClipboardUtils.getFromClipboard( TextTransfer.getInstance(), String.class ) );
 
             DnDialog dialog = new DnDialog(
                 getShell(),
@@ -110,26 +109,4 @@ public class GotoDnAction extends LocateInDitAction
         return null;
     }
 
-
-    private static String getStringFromClipboard()
-    {
-        Clipboard clipboard = null;
-        try
-        {
-            clipboard = new Clipboard( Display.getCurrent() );
-            Object contents = clipboard.getContents( TextTransfer.getInstance() );
-            if ( contents instanceof String )
-            {
-                return ( String ) contents;
-            }
-        }
-        finally
-        {
-            if ( clipboard != null )
-            {
-                clipboard.dispose();
-            }
-        }
-        return null;
-    }
 }
diff --git a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnNavigateMenuAction.java b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnNavigateMenuAction.java
index 7d15adf..0ac24a6 100644
--- a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnNavigateMenuAction.java
+++ b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/actions/GotoDnNavigateMenuAction.java
@@ -22,6 +22,7 @@ package org.apache.directory.studio.ldapbrowser.ui.actions;
 
 
 import org.apache.directory.api.ldap.model.name.Dn;
+import org.apache.directory.studio.common.ui.ClipboardUtils;
 import org.apache.directory.studio.connection.core.Connection;
 import org.apache.directory.studio.connection.core.Utils;
 import org.apache.directory.studio.ldapbrowser.common.dialogs.DnDialog;
@@ -33,9 +34,7 @@ import org.apache.directory.studio.ldapbrowser.ui.BrowserUIPlugin;
 import org.apache.directory.studio.ldapbrowser.ui.views.connection.ConnectionView;
 import org.eclipse.jface.resource.ImageDescriptor;
 import org.eclipse.jface.viewers.StructuredSelection;
-import org.eclipse.swt.dnd.Clipboard;
 import org.eclipse.swt.dnd.TextTransfer;
-import org.eclipse.swt.widgets.Display;
 import org.eclipse.ui.PlatformUI;
 
 
@@ -87,7 +86,7 @@ public class GotoDnNavigateMenuAction extends LocateInDitAction
                 .getBrowserConnection( selectedConnection );
 
             // Getting the DN from the clipboard (if any)
-            Dn dn = Utils.getLdapDn( getStringFromClipboard() );
+            Dn dn = Utils.getLdapDn( ClipboardUtils.getFromClipboard( TextTransfer.getInstance(), String.class ) );
 
             // Displaying the DN dialog
             DnDialog dialog = new DnDialog(
@@ -137,31 +136,4 @@ public class GotoDnNavigateMenuAction extends LocateInDitAction
         return null;
     }
 
-
-    /**
-     * Gets the string from the clipboard.
-     *
-     * @return the string from the clipboard
-     */
-    private String getStringFromClipboard()
-    {
-        Clipboard clipboard = null;
-        try
-        {
-            clipboard = new Clipboard( Display.getCurrent() );
-            Object contents = clipboard.getContents( TextTransfer.getInstance() );
-            if ( contents instanceof String )
-            {
-                return ( String ) contents;
-            }
-        }
-        finally
-        {
-            if ( clipboard != null )
-            {
-                clipboard.dispose();
-            }
-        }
-        return null;
-    }
 }
diff --git a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/editors/searchresult/SearchResultEditorPasteAction.java b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/editors/searchresult/SearchResultEditorPasteAction.java
index 74a09e4..717a3e2 100644
--- a/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/editors/searchresult/SearchResultEditorPasteAction.java
+++ b/plugins/ldapbrowser.ui/src/main/java/org/apache/directory/studio/ldapbrowser/ui/editors/searchresult/SearchResultEditorPasteAction.java
@@ -21,6 +21,7 @@
 package org.apache.directory.studio.ldapbrowser.ui.editors.searchresult;
 
 
+import org.apache.directory.studio.common.ui.ClipboardUtils;
 import org.apache.directory.studio.ldapbrowser.common.actions.PasteAction;
 import org.apache.directory.studio.ldapbrowser.common.dnd.ValuesTransfer;
 import org.apache.directory.studio.ldapbrowser.core.model.IAttribute;
@@ -116,7 +117,7 @@ public class SearchResultEditorPasteAction extends PasteAction
             && getSelectedAttributeHierarchies()[0].size() == 1 )
         {
 
-            Object content = this.getFromClipboard( ValuesTransfer.getInstance() );
+            Object content = ClipboardUtils.getFromClipboard( ValuesTransfer.getInstance() );
             if ( content instanceof IValue[] )
             {
                 IValue[] values = ( IValue[] ) content;