Fix Authorization class to be compatible with using clustered cache

Change-Id: I25b5a588648afcb79c56a26545ecb785afd0d3a3
Signed-off-by: Yevgeny Khodorkovsky <ykhodork@cisco.com>

diff --git a/opendaylight/appauth/src/main/java/org/opendaylight/controller/appauth/authorization/Authorization.java b/opendaylight/appauth/src/main/java/org/opendaylight/controller/appauth/authorization/Authorization.java
index acfc7252ef2158deeaebae0210584b75179cf4fd..fd8799f4f31fdac7f49576f2b49fbb0d8e034867 100644 (file)
--- a/opendaylight/appauth/src/main/java/org/opendaylight/controller/appauth/authorization/Authorization.java
+++ b/opendaylight/appauth/src/main/java/org/opendaylight/controller/appauth/authorization/Authorization.java
@@ -26,7 +26,6 @@ import org.opendaylight.controller.sal.utils.ServiceHelper;
 import org.opendaylight.controller.sal.utils.Status;
 import org.opendaylight.controller.sal.utils.StatusCode;
 import org.opendaylight.controller.usermanager.IUserManager;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -85,7 +84,7 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
     protected Status createRoleInternal(String role, AppRoleLevel level) {
         roles.put(role, level);
         groupsAuthorizations.put(role, new HashSet<ResourceGroup>());
-        return new Status(StatusCode.SUCCESS, null);
+        return new Status(StatusCode.SUCCESS);
     }
 
     @Override
@@ -104,7 +103,7 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
     protected Status removeRoleInternal(String role) {
         groupsAuthorizations.remove(role);
         roles.remove(role);
-        return new Status(StatusCode.SUCCESS, null);
+        return new Status(StatusCode.SUCCESS);
     }
 
     @Override
@@ -126,7 +125,7 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
         }
         //verify group name is unique
         if (resourceGroups.containsKey(groupName)) {
-        return new Status(StatusCode.CONFLICT, "Group name already exists");
+            return new Status(StatusCode.CONFLICT, "Group name already exists");
         }
 
         //try adding resources, discard if not of type T
@@ -139,9 +138,6 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
                 allAdded = false;
             }
         }
-        /*if (toBeAdded.size() == 0){ // TODO andrekim - we should have the ability to create a group with no resources (so we can add and delete entries)
-           return new Status(StatusCode.NOTACCEPTABLE, "Group not created. No valid resources specified");
-        }*/
         resourceGroups.put(groupName, toBeAdded);
         return (allAdded ? new Status(StatusCode.SUCCESS, "All resources added succesfully") :
             new Status(StatusCode.SUCCESS, "One or more resources couldn't be added"));
@@ -155,6 +151,8 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
         Set<T> group = resourceGroups.get(groupName);
         if (group != null && resource != null) {
             group.add(resource);
+            // Update cluster
+            resourceGroups.put(groupName, group);
             return new Status(StatusCode.SUCCESS, "Resource added successfully");
         }
 
@@ -164,30 +162,29 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
     public Status removeRoleResourceGroupMapping(String groupName) {
         List<String> affectedRoles = new ArrayList<String>();
         Status result;
-        for (Entry<String, Set<ResourceGroup>> pairs : groupsAuthorizations
-                .entrySet()) {
+        for (Entry<String, Set<ResourceGroup>> pairs : groupsAuthorizations.entrySet()) {
             String role = pairs.getKey();
             Set<ResourceGroup> groups = pairs.getValue();
             for (ResourceGroup group : groups) {
-            if (group.getGroupName().equals(groupName)) {
-            affectedRoles.add(role);
-            break;
-            }
+                if (group.getGroupName().equals(groupName)) {
+                    affectedRoles.add(role);
+                    break;
+                }
             }
         }
         StringBuffer msg = new StringBuffer();
         for (String role : affectedRoles) {
-             result = unassignResourceGroupFromRole(groupName, role);
-             if (!result.isSuccess()) {
+            result = unassignResourceGroupFromRole(groupName, role);
+            if (!result.isSuccess()) {
                 msg.append(result.getDescription());
                 msg.append(' ');
-             }
-         }
+            }
+        }
 
         if (msg.length() != 0) {
-                return new Status(StatusCode.BADREQUEST, msg.toString());
+            return new Status(StatusCode.BADREQUEST, msg.toString());
         } else {
-                return new Status(StatusCode.SUCCESS);
+            return new Status(StatusCode.SUCCESS);
         }
     }
 
@@ -201,16 +198,11 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
             return new Status(StatusCode.NOTALLOWED,
                     "All resource group cannot be removed");
         }
-
-        Status result = removeRoleResourceGroupMapping(groupName);
-
         resourceGroups.remove(groupName);
+        Status result = removeRoleResourceGroupMapping(groupName);
 
-        if (!result.isSuccess()) {
-            return result;
-        }
-
-        return new Status(StatusCode.SUCCESS, null);
+        return result.isSuccess() ? result :
+            new Status(StatusCode.SUCCESS, "Failed removing group from: " + result.getDescription());
     }
 
 
@@ -221,6 +213,8 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
 
         Set<T> group = resourceGroups.get(groupName);
         if (group != null && group.remove(resource)) {
+            // Update cluster
+            resourceGroups.put(groupName, group);
             return new Status(StatusCode.SUCCESS, "Resource removed successfully");
         }
 
@@ -440,10 +434,12 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
         return assignResourceGroupToRoleInternal(group, privilege, role);
     }
 
-    protected Status assignResourceGroupToRoleInternal(String group,
-            Privilege privilege, String role) {
-        groupsAuthorizations.get(role).add(new ResourceGroup(group, privilege));
-        return new Status(StatusCode.SUCCESS, null);
+    protected Status assignResourceGroupToRoleInternal(String group, Privilege privilege, String role) {
+        Set<ResourceGroup> roleGroups = groupsAuthorizations.get(role);
+        roleGroups.add(new ResourceGroup(group, privilege));
+        // Update cluster
+        groupsAuthorizations.put(role, roleGroups);
+        return new Status(StatusCode.SUCCESS);
     }
 
     @Override
@@ -489,8 +485,7 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
         return unassignResourceGroupFromRoleInternal(group, role);
     }
 
-    protected Status unassignResourceGroupFromRoleInternal(String group,
-            String role) {
+    protected Status unassignResourceGroupFromRoleInternal(String group, String role) {
         ResourceGroup target = null;
         for (ResourceGroup rGroup : groupsAuthorizations.get(role)) {
             if (rGroup.getGroupName().equals(group)) {
@@ -499,11 +494,13 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
             }
         }
         if (target == null) {
-            return new Status(StatusCode.SUCCESS, "Group " + group
-                    + " was not assigned to " + role);
+            return new Status(StatusCode.SUCCESS, "Group " + group + " was not assigned to " + role);
         } else {
-        groupsAuthorizations.get(role).remove(target);
-        return new Status(StatusCode.SUCCESS);
+            Set<ResourceGroup> groups = groupsAuthorizations.get(role);
+            groups.remove(target);
+            // Update cluster
+            groupsAuthorizations.put(role, groups);
+            return new Status(StatusCode.SUCCESS);
 
         }
     }
@@ -518,7 +515,7 @@ private static final Logger logger = LoggerFactory.getLogger(Authorization.class
     @Override
     public List<Object> getResources(String groupName) {
         return (resourceGroups.containsKey(groupName)) ? new ArrayList<Object>(
-                resourceGroups.get(groupName)) : new ArrayList<Object>();
+                resourceGroups.get(groupName)) : new ArrayList<Object>(0);
     }
 
     @Override

diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/authorization/IResourceAuthorization.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/authorization/IResourceAuthorization.java
index c88c53decf36d7e6cdcf3e55ecd46ce1ff830032..088f9da9a86d4d5b0f39530e97657dbefea47007 100644 (file)
--- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/authorization/IResourceAuthorization.java
+++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/authorization/IResourceAuthorization.java
@@ -68,10 +68,15 @@ public interface IResourceAuthorization {
     public boolean isApplicationRole(String roleName);
 
     /**
-     * Create a resource group for application
+     * Create a resource group for application.
      *
-     * @param groupName the name for the resource group
-     * @param resources the list of resources for the group
+     * NOTE: Resource addition is "best effort", if an object is not of correct type,
+     * it is discarded.
+     *
+     * @param groupName
+     *            the name for the resource group
+     * @param resources
+     *            the list of resources for the group
      * @return the status of the request
      */
     public Status createResourceGroup(String groupName, List<Object> resources);