QPID-5138: Change preferences provider to allow deletion of preferences for multiple users

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1529362 13f79535-47bb-0310-9956-ffa450edef68

7 files changed, 117 insertions, 50 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/PreferencesProvider.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/PreferencesProvider.java
index 96d02b2ef6..fadd036a58 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/PreferencesProvider.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/PreferencesProvider.java
@@ -69,11 +69,11 @@ public interface PreferencesProvider extends ConfiguredObject
     Map<String, Object> setPreferences(String userId, Map<String, Object> preferences);
 
     /**
-     * Delete preferences for a given user ID
-     * @param userId user ID to delete preferences for
-     * @return user preferences before the deletion
+     * Delete preferences for given user IDs
+     * @param userIDs user IDs to delete preferences for
+     * @return user an array with user IDs having preferences deleted
      */
-    Map<String, Object> deletePreferences(String userId);
+    String[] deletePreferences(String... userIDs);
 
     /**
      * Returns set of the user IDs having preferences set
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/User.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/User.java
index f9d6f17ffd..cf1a688634 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/User.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/User.java
@@ -61,5 +61,5 @@ public interface User extends ConfiguredObject
 
     public Map<String, Object> setPreferences(Map<String, Object> preferences);
 
-    public Map<String, Object> replacePreferences(Map<String, Object> newPreferences);
+    public boolean deletePreferences();
 }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
index bb734ba5bd..ee2cd6a5fc 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java
@@ -803,16 +803,15 @@ public abstract class AuthenticationProviderAdapter<T extends AuthenticationMana
             }
 
             @Override
-            public Map<String, Object> replacePreferences(Map<String, Object> newPreferences)
+            public boolean deletePreferences()
             {
                 PreferencesProvider preferencesProvider = getPreferencesProvider();
                 if (preferencesProvider == null)
                 {
-                    return null;
+                    return false;
                 }
-                Map<String, Object> preferences = preferencesProvider.deletePreferences(this.getName());
-                preferencesProvider.setPreferences(this.getName(), newPreferences);
-                return preferences;
+                String[] deleted = preferencesProvider.deletePreferences(this.getName());
+                return deleted.length == 1;
             }
 
             private PreferencesProvider getPreferencesProvider()
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProvider.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProvider.java
index 365393ee8d..39f0017fc3 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProvider.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProvider.java
@@ -35,6 +35,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
@@ -301,9 +302,9 @@ public class FileSystemPreferencesProvider extends AbstractAdapter implements Pr
     }
 
     @Override
-    public Map<String, Object> deletePreferences(String userId)
+    public String[] deletePreferences(String... userIDs)
     {
-        return _store.deletePreferences(userId);
+        return _store.deletePreferences(userIDs);
     }
 
     @Override
@@ -566,19 +567,26 @@ public class FileSystemPreferencesProvider extends AbstractAdapter implements Pr
             return userPreferences;
         }
 
-        public Map<String, Object> deletePreferences(String userId)
+        public String[] deletePreferences(String... userIDs)
         {
             checkStoreOpened();
-            Map<String, Object> userPreferences = null;
+            Set<String> deletedUsers = new HashSet<String>();
             synchronized (_preferences)
             {
-                if (_preferences.containsKey(userId))
+                for (String id : userIDs)
+                {
+                    if (_preferences.containsKey(id))
+                    {
+                        _preferences.remove(id);
+                        deletedUsers.add(id);
+                    }
+                }
+                if (!deletedUsers.isEmpty())
                 {
-                    userPreferences = _preferences.remove(userId);
                     save();
                 }
             }
-            return userPreferences;
+            return deletedUsers.toArray(new String[deletedUsers.size()]);
         }
 
         public Set<String> listUserIDs()
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java
index aebea294d8..8b0847e0d2 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/adapter/FileSystemPreferencesProviderTest.java
@@ -210,7 +210,7 @@ public class FileSystemPreferencesProviderTest extends QpidTestCase
 
         String user3 = "user3";
         Map<String, Object> preferences3 = _preferencesProvider.getPreferences(user3);
-        assertTrue("No preference found for user3", preferences3.isEmpty());
+        assertTrue("Unexpected preferences found for user3", preferences3.isEmpty());
     }
 
     public void testDeletePrefernces()
@@ -218,6 +218,9 @@ public class FileSystemPreferencesProviderTest extends QpidTestCase
         _preferencesProvider = createPreferencesProvider();
         _preferencesProvider.setDesiredState(State.INITIALISING, State.ACTIVE);
 
+        assertUser1Preferences(_preferencesProvider.getPreferences(_user1));
+        assertUser2Preferences(_preferencesProvider.getPreferences(_user2));
+
         _preferencesProvider.deletePreferences(_user1);
         _preferencesProvider.setDesiredState(State.ACTIVE, State.STOPPED);
 
@@ -231,6 +234,30 @@ public class FileSystemPreferencesProviderTest extends QpidTestCase
 
         String user3 = "user3";
         Map<String, Object> preferences3 = _preferencesProvider.getPreferences(user3);
+        assertTrue("Unexpected preferences found for user3", preferences3.isEmpty());
+    }
+
+    public void testDeleteMultipleUsersPrefernces()
+    {
+        _preferencesProvider = createPreferencesProvider();
+        _preferencesProvider.setDesiredState(State.INITIALISING, State.ACTIVE);
+
+        assertUser1Preferences(_preferencesProvider.getPreferences(_user1));
+        assertUser2Preferences(_preferencesProvider.getPreferences(_user2));
+
+        _preferencesProvider.deletePreferences(_user1, _user2);
+        _preferencesProvider.setDesiredState(State.ACTIVE, State.STOPPED);
+
+        _preferencesProvider = createPreferencesProvider();
+        _preferencesProvider.setDesiredState(State.INITIALISING, State.ACTIVE);
+        Map<String, Object> preferences1 = _preferencesProvider.getPreferences(_user1);
+        assertTrue("Preferences should not be set for user 1", preferences1.isEmpty());
+
+        Map<String, Object> preferences2 = _preferencesProvider.getPreferences(_user2);
+        assertTrue("Preferences should not be set for user 2", preferences2.isEmpty());
+
+        String user3 = "user3";
+        Map<String, Object> preferences3 = _preferencesProvider.getPreferences(user3);
         assertTrue("No preference found for user3", preferences3.isEmpty());
     }
 
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java
index 56e83db4d2..36949e22e1 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/UserPreferencesServlet.java
@@ -25,7 +25,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.Iterator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -157,53 +157,75 @@ public class UserPreferencesServlet extends AbstractServlet
     @Override
     protected void doDeleteWithSubjectAndActor(HttpServletRequest request, HttpServletResponse response) throws IOException
     {
-        final List<String[]> userData = new ArrayList<String[]>();
-        for (String name : request.getParameterValues("user"))
+        Broker broker = getBroker();
+        Collection<AuthenticationProvider> authenticationProviders = broker.getAuthenticationProviders();
+        Map<String, Set<String>> providerUsers = new HashMap<String, Set<String>>();
+        Map<String, AuthenticationProvider> requestProviders = new HashMap<String, AuthenticationProvider>();
+        for (String path : request.getParameterValues("user"))
         {
-            String[] elements = name.split("/");
+            String[] elements = path.split("/");
             if (elements.length != 2)
             {
-                throw new IllegalArgumentException("Illegal parameter");
+                throw new IllegalArgumentException("Illegal user parameter " + path);
+            }
+
+            String userId = elements[1];
+
+            if (!userPreferencesOperationAuthorized(userId))
+            {
+                response.sendError(HttpServletResponse.SC_FORBIDDEN, "Deletion of preferences is not allowed");
+                return;
             }
-            userData.add(elements);
+            String providerName =  elements[0];
+            Set<String> users = providerUsers.get(providerName);
+
+            if (users == null)
+            {
+                AuthenticationProvider provider = findAuthenticationProviderByName(providerName, authenticationProviders);
+                if (provider == null)
+                {
+                    throw new IllegalArgumentException("Cannot find provider with name '" + providerName + "'");
+                }
+                users = new HashSet<String>();
+                providerUsers.put(providerName, users);
+                requestProviders.put(providerName, provider);
+            }
+            users.add(userId);
         }
 
-        if (!userData.isEmpty())
+        if (!providerUsers.isEmpty())
         {
-            Broker broker = getBroker();
-            Collection<AuthenticationProvider> authenticationProviders = broker.getAuthenticationProviders();
-            for (Iterator<String[]> it = userData.iterator(); it.hasNext();)
+            for (Map.Entry<String, Set<String>> entry : providerUsers.entrySet())
             {
-                String[] data = (String[]) it.next();
-                String authenticationProviderName = data[0];
-                String userId = data[1];
+                String providerName = entry.getKey();
+                AuthenticationProvider provider = requestProviders.get(providerName);
+                Set<String> usersToDelete = entry.getValue();
+                PreferencesProvider preferencesProvider = provider.getPreferencesProvider();
 
-                for (AuthenticationProvider authenticationProvider : authenticationProviders)
+                if (preferencesProvider != null && !usersToDelete.isEmpty())
                 {
-                    if (authenticationProviderName.equals(authenticationProvider.getName()))
-                    {
-                        PreferencesProvider preferencesProvider = authenticationProvider.getPreferencesProvider();
-                        if (preferencesProvider != null)
-                        {
-                            Set<String> usernames = preferencesProvider.listUserIDs();
-                            if (usernames.contains(userId))
-                            {
-                                if (!userPreferencesOperationAuthorized(userId))
-                                {
-                                    response.sendError(HttpServletResponse.SC_FORBIDDEN, "Deletion of preferences is not allowed");
-                                    return;
-                                }
-                                preferencesProvider.deletePreferences(userId);
-                            }
-                        }
-                        break;
-                    }
+                    String[] users = usersToDelete.toArray(new String[usersToDelete.size()]);
+                    preferencesProvider.deletePreferences(users);
                 }
             }
         }
 
     }
 
+    protected AuthenticationProvider findAuthenticationProviderByName(String providerName, Collection<AuthenticationProvider> authenticationProviders)
+    {
+        AuthenticationProvider provider = null;
+        for (AuthenticationProvider authenticationProvider : authenticationProviders)
+        {
+            if(authenticationProvider.getName().equals(providerName))
+            {
+                provider = authenticationProvider;
+                break;
+            }
+        }
+        return provider;
+    }
+
     private boolean userPreferencesOperationAuthorized(String userId)
     {
         return getBroker().getSecurityManager().authoriseUserOperation(Operation.UPDATE, userId);
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
index 7c0883ec84..9965587343 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/systest/rest/UserPreferencesRestTest.java
@@ -125,6 +125,17 @@ public class UserPreferencesRestTest extends QpidRestTestCase
         assertNull("User webadmin is found", findUser("webadmin", users));
     }
 
+    public void testDeleteMultipleUser() throws Exception
+    {
+        int status = getRestTestHelper().submitRequest("/rest/userpreferences?user="
+                + URLEncoder.encode(TestBrokerConfiguration.ENTRY_NAME_AUTHENTICATION_PROVIDER + "/webadmin", "UTF-8")
+                + "&user=" + URLEncoder.encode(TestBrokerConfiguration.ENTRY_NAME_AUTHENTICATION_PROVIDER + "/admin", "UTF-8"),
+                "DELETE", null);
+        assertEquals("Unexpected status ", 200, status);
+        List<Map<String, Object>> users = getRestTestHelper().getJsonAsList("/rest/userpreferences");
+        assertEquals("Unexpected number of users", 0, users.size());
+    }
+
     private Map<String, Object> findUser(String userName, List<Map<String, Object>> users)
     {
         for (Map<String, Object> map : users)