Changeset 5990

Timestamp:

Mar 5, 2012, 8:50:19 AM (11 years ago)

Author:

Nicklas Nordborg

Message:

Fixes #1667: Delete of items in trash is aborted if user logs off

The trashcan now uses a cloned session control and should not be affected if something happens (such as logout) with the original session control.

Location:

branches/3.0-stable/src/core/net/sf/basedb/core

Files:

• SessionControl.java (modified) (3 diffs)
• Trashcan.java (modified) (3 diffs)

branches/3.0-stable/src/core/net/sf/basedb/core/SessionControl.java

@@ -514 +514 @@
 
   /**
-    Log in as another user. If this call is successful, you will get a new
-    <code>SessionControl</code> object which is equivalent to a <code>SessionControl</code>
-    where that user logged in by normal means. This method requires that the logged
-    in user has {@link Permission#ACT_AS_ANOTHER_USER} permission.
+    Log in as another user or create a clone of the currently logged in user's session.
+    If this call is successful, you will get a new <code>SessionControl</code> object which 
+    is equivalent to a <code>SessionControl</code> where that user logged in by normal means. 
+    This method requires that the logged in user has {@link Permission#ACT_AS_ANOTHER_USER} 
+    permission or that the userId is the same as the currently logged in user.
 
     @param userId The id of the user to login as
@@ -524 +525 @@
     @throws ItemNotFoundException If no user with the specified id exists
     @throws PermissionDeniedException If the logged in user doesn't
-      have {@link Permission#ACT_AS_ANOTHER_USER} permission
+      have {@link Permission#ACT_AS_ANOTHER_USER} permission and the userId
+      is a different user from the currently logged in user
     @throws BaseException If there is another error
   */
@@ -530 +532 @@
     throws ItemNotFoundException, PermissionDeniedException, BaseException
   {
-    if (!hasSystemPermission(Permission.ACT_AS_ANOTHER_USER))
+    if (!hasSystemPermission(Permission.ACT_AS_ANOTHER_USER) && userId != getLoggedInUserId())
     {
       throw new PermissionDeniedException("No permission to act as another user.");

branches/3.0-stable/src/core/net/sf/basedb/core/Trashcan.java

@@ -23 +23 @@
 package net.sf.basedb.core;
 
+import net.sf.basedb.core.Application.Pinger;
 import net.sf.basedb.core.data.BasicData;
 import net.sf.basedb.core.data.RemovableData;
@@ -172 +173 @@
     and some may not.
     
-    @param sc The session control to use for connecting to the database
+    @param sc The session control to use for connecting to the database. To
+      avoid multithread-related problems the session control is cloned 
+      before being used by this method. 
     @param items The items to remove
     @param ignoreFlag TRUE to also remove items that hasn't been flagged for removal
@@ -211 +214 @@
 
     DbControl dc = null;
-    // We try to delete items from the set as long as there are items
-    // left and at least one item was deleted in the last iteration
-    do
-    {
-      ThreadSignalHandler.checkInterrupted();
-      dc = sc.newDbControl();
-      log.debug("Start new delete transaction");
-      try
-      {
-        removedInTransaction = 0;
-        Iterator<Identifiable> iterator = itemsToRemove.iterator();
-        deleteTransaction:
-        while (iterator.hasNext())
+    Pinger pinger = null;
+    SessionControl sc2 = null;
+    try
+    {
+      // We create an impersonated session control to 
+      // avoid issues where the original session control is closed,
+      // for example, due to the user logging out
+      sc2 = sc.impersonateLogin(sc.getLoggedInUserId(), "Deleting items from trashcan");
+      pinger = Application.newPinger(sc2);
+      // We try to delete items from the set as long as there are items
+      // left and at least one item was deleted in the last iteration
+      do
+      {
+        ThreadSignalHandler.checkInterrupted();
+        dc = sc2.newDbControl();
+        log.debug("Start new delete transaction");
+        try
         {
-          ThreadSignalHandler.checkInterrupted();
-          Identifiable item = iterator.next();
+          removedInTransaction = 0;
+          Iterator<Identifiable> iterator = itemsToRemove.iterator();
+          deleteTransaction:
+          while (iterator.hasNext())
+          {
+            ThreadSignalHandler.checkInterrupted();
+            Identifiable item = iterator.next();
+            if (progress != null)
+            {
+              String name = item instanceof Nameable ? ((Nameable)item).getName() : item.toString();
+              progress.display((int)(numRemoved * progressFactor), "Remove " +
+                  item.getType() + ": " + name + 
+                  " (" + (numRemoved + 1) + " of " + numToRemove + ")");
+            }
+            BasicItem basicItem = item.getType().getById(dc, item.getId());
+            if (!basicItem.isUsed()) 
+            {
+              if (isDebug) log.debug("Deleting item " + basicItem);
+              dc.deleteItem(basicItem);
+              numRemoved++;
+              removedInTransaction++;
+              iterator.remove();
+            }
+            else 
+            {
+              if (isDebug) log.debug("Item is used: " + basicItem);
+              if (progress != null) progress.append(" [USED]");
+              if (removedInTransaction > 0)
+              {
+                // Commit, start new transaction and try again
+                break deleteTransaction;
+              }
+            }
+          }
+          log.debug("Committing transaction; " + removedInTransaction + 
+            " items scheduled for deletion; " + numRemoved + " total so far; " +
+            itemsToRemove.size() + " remains in list");
           if (progress != null)
           {
-            String name = item instanceof Nameable ? ((Nameable)item).getName() : item.toString();
-            progress.display((int)(numRemoved * progressFactor), "Remove " +
-                item.getType() + ": " + name + 
-                " (" + (numRemoved + 1) + " of " + numToRemove + ")");
+            progress.display((int)(numRemoved * progressFactor), 
+              numRemoved + " of " + numToRemove + " items removed; flushing to database");
           }
-          BasicItem basicItem = item.getType().getById(dc, item.getId());
-          if (!basicItem.isUsed()) 
-          {
-            if (isDebug) log.debug("Deleting item " + basicItem);
-            dc.deleteItem(basicItem);
-            numRemoved++;
-            removedInTransaction++;
-            iterator.remove();
-          }
-          else 
-          {
-            if (isDebug) log.debug("Item is used: " + basicItem);
-            if (progress != null) progress.append(" [USED]");
-            if (removedInTransaction > 0)
-            {
-              // Commit, start new transaction and try again
-              break deleteTransaction;
-            }
-          }
+          dc.commit();
         }
-        log.debug("Committing transaction; " + removedInTransaction + 
-          " items scheduled for deletion; " + numRemoved + " total so far; " +
-          itemsToRemove.size() + " remains in list");
-        if (progress != null)
+        finally
         {
-          progress.display((int)(numRemoved * progressFactor), 
-            numRemoved + " of " + numToRemove + " items removed; flushing to database");
+          if (dc != null) dc.close();
         }
-        dc.commit();
-      }
-      finally
-      {
-        if (dc != null) dc.close();
-      }
-    } while (removedInTransaction > 0 && itemsToRemove.size() > 0);
+      } while (removedInTransaction > 0 && itemsToRemove.size() > 0);
+    }
+    finally
+    {
+      if (pinger != null) pinger.stop();
+      if (sc2 != null)
+      {
+        sc2.logout();
+        sc2.close();
+      }
+    }
     
     if (progress != null)