NOJIRA

incorporate some code review changes. started refactoring seed test in order to convert it to generator and import for all services

M    services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationSeedTest.java
M    services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationGen.java
M    services/authorization-mgt/client/src/test/resources/test-data/test-permissions-roles.xml
M    services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java
M    services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java
M    services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java

diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationGen.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationGen.java
index 6cf2bd0fc471cedb3a300bb3b421037a8a724890..d1b2acfed056d6f045a2a32bec174e6f4242103c 100644 (file)
--- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationGen.java
+++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationGen.java
@@ -15,28 +15,6 @@
 
  *  https://source.collectionspace.org/collection-space/LICENSE.txt
 
- *  Unless required by applicable law or agreed to in writing, software
- *  distributed under the License is distributed on an "AS IS" BASIS,
- *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- *  See the License for the specific language governing permissions and
- *  limitations under the License.
- *//**
- *  This document is a part of the source code and related artifacts
- *  for CollectionSpace, an open source collections management system
- *  for museums and related institutions:
-
- *  http://www.collectionspace.org
- *  http://wiki.collectionspace.org
-
- *  Copyright 2009 University of California at Berkeley
-
- *  Licensed under the Educational Community License (ECL), Version 2.0.
- *  You may not use this file except in compliance with this License.
-
- *  You may obtain a copy of the ECL 2.0 License at
-
- *  https://source.collectionspace.org/collection-space/LICENSE.txt
-
  *  Unless required by applicable law or agreed to in writing, software
  *  distributed under the License is distributed on an "AS IS" BASIS,
  *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -49,6 +27,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.UUID;
 import org.collectionspace.services.authorization.ActionType;
 import org.collectionspace.services.authorization.Permission;
 import org.collectionspace.services.authorization.EffectType;
@@ -67,29 +46,35 @@ import org.collectionspace.services.authorization.SubjectType;
 public class AuthorizationGen {
 
     final Logger logger = LoggerFactory.getLogger(AuthorizationGen.class);
+    private PermissionsList pcList = new PermissionsList();
+    PermissionsRolesList psrsl = new PermissionsRolesList();
 
-    public void genPermissions() {
-        PermissionsList pcList = new PermissionsList();
+    public PermissionsList genPermissions() {
         ArrayList<Permission> apcList = new ArrayList<Permission>();
         pcList.setPermissions(apcList);
 
-        Permission accPerm = buildCommonPermission("1", "accounts");
+        Permission accPerm = buildCommonPermission("1", "1", "accounts");
         apcList.add(accPerm);
-        Permission dimPerm = buildCommonPermission("2", "dimensions");
+        Permission dimPerm = buildCommonPermission("1", "2", "dimensions");
         apcList.add(dimPerm);
+        return pcList;
+
+    }
+
+    public void writePermissions(PermissionsList pcList, String fileName) {
         AbstractAuthorizationTestImpl.toFile(pcList, PermissionsList.class,
-                AbstractAuthorizationTestImpl.testDataDir + "test-permissions.xml");
+                AbstractAuthorizationTestImpl.testDataDir + fileName);
         logger.info("generated permissions to "
-                + AbstractAuthorizationTestImpl.testDataDir + "test-permissions.xml");
-
+                + AbstractAuthorizationTestImpl.testDataDir + fileName);
     }
 
-    private Permission buildCommonPermission(String id, String resourceName) {
+    private Permission buildCommonPermission(String tenantId, String permId, String resourceName) {
+        //String id = UUID.randomUUID().toString();
         Permission perm = new Permission();
-        perm.setCsid(id);
+        perm.setCsid(permId);
         perm.setResourceName(resourceName);
         perm.setEffect(EffectType.PERMIT);
-        perm.setTenantId("1");
+        perm.setTenantId(tenantId);
         ArrayList<PermissionAction> pas = new ArrayList<PermissionAction>();
         perm.setActions(pas);
 
@@ -108,39 +93,38 @@ public class AuthorizationGen {
         return perm;
     }
 
-    public void genPermissionsRoles() {
-        PermissionsRolesList psrsl = new PermissionsRolesList();
+    public PermissionsRolesList genPermissionsRoles(PermissionsList pcList) {
         ArrayList<PermissionRole> prl = new ArrayList<PermissionRole>();
-        prl.add(buildCommonPermissionRoles("1", "accounts"));
-        prl.add(buildCommonPermissionRoles("2", "dimensions"));
+        prl.add(buildCommonPermissionRoles("1", "1", "accounts"));
+        prl.add(buildCommonPermissionRoles("1", "2", "dimensions"));
         psrsl.setPermissionRoles(prl);
+        return psrsl;
+    }
+
+    public void writePermissionRoles(PermissionsRolesList psrsl, String fileName) {
         AbstractAuthorizationTestImpl.toFile(psrsl, PermissionsRolesList.class,
-                AbstractAuthorizationTestImpl.testDataDir + "test-permissions-roles.xml");
+                AbstractAuthorizationTestImpl.testDataDir + fileName);
         logger.info("generated permissions-roles to "
-                + AbstractAuthorizationTestImpl.testDataDir + "test-permissions-roles.xml");
+                + AbstractAuthorizationTestImpl.testDataDir + fileName);
     }
 
-    private PermissionRole buildCommonPermissionRoles(String id, String resName) {
+    private PermissionRole buildCommonPermissionRoles(String tenantId, String permissionId,
+            String resName) {
 
         PermissionRole pr = new PermissionRole();
         pr.setSubject(SubjectType.ROLE);
-
         List<PermissionValue> permValues = new ArrayList<PermissionValue>();
         pr.setPermissions(permValues);
         PermissionValue permValue = new PermissionValue();
-        permValue.setPermissionId(id);
+        permValue.setPermissionId(permissionId);
         permValue.setResourceName(resName);
         permValues.add(permValue);
 
         List<RoleValue> roleValues = new ArrayList<RoleValue>();
-        RoleValue rv1 = new RoleValue();
-        rv1.setRoleName("ROLE_USERS");
-        rv1.setRoleId("1");
-        roleValues.add(rv1);
-        RoleValue rv2 = new RoleValue();
-        rv2.setRoleName("ROLE_ADMINISTRATOR");
-        rv2.setRoleId("2");
-        roleValues.add(rv2);
+        RoleValue radmin = new RoleValue();
+        radmin.setRoleName("ROLE_ADMINISTRATOR");
+        radmin.setRoleId(tenantId);
+        roleValues.add(radmin);
         pr.setRoles(roleValues);
 
         return pr;

diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationSeedTest.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationSeedTest.java
index ea650f0141dca57efaa4476782c3f4bd4c3924d5..c22344991e8c964cf5950266a472e6475003ed62 100644 (file)
--- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationSeedTest.java
+++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/test/AuthorizationSeedTest.java
@@ -51,6 +51,8 @@ import org.testng.annotations.BeforeClass;
 public class AuthorizationSeedTest extends AbstractAuthorizationTestImpl {
 
     final Logger logger = LoggerFactory.getLogger(AuthorizationSeedTest.class);
+    final static String PERMISSION_FILE = "test-permissions.xml";
+    final static String PERMISSION_ROLE_FILE = "test-permissions-roles.xml";
 
     @BeforeClass(alwaysRun = true)
     public void seedData() {
@@ -58,8 +60,10 @@ public class AuthorizationSeedTest extends AbstractAuthorizationTestImpl {
         TransactionStatus status = beginTransaction("seedData");
         try {
             AuthorizationGen authzGen = new AuthorizationGen();
-            authzGen.genPermissions();
-            authzGen.genPermissionsRoles();
+            PermissionsList pl = authzGen.genPermissions();
+            authzGen.writePermissions(pl, PERMISSION_FILE);
+            PermissionsRolesList prl = authzGen.genPermissionsRoles(pl);
+            authzGen.writePermissionRoles(prl, PERMISSION_ROLE_FILE);
             seedRoles();
             seedPermissions();
         } catch (Exception ex) {
@@ -71,20 +75,20 @@ public class AuthorizationSeedTest extends AbstractAuthorizationTestImpl {
     }
 
     public void seedRoles() throws Exception {
-       //Should this test really be empty?
+        //Should this test really be empty?
     }
 
-    public void seedPermissions() throws Exception {           
+    public void seedPermissions() throws Exception {
         PermissionsList pcList =
-                (PermissionsList) fromFile(PermissionsList.class, baseDir +
-                AbstractAuthorizationTestImpl.testDataDir + "test-permissions.xml");
+                (PermissionsList) fromFile(PermissionsList.class, baseDir
+                + AbstractAuthorizationTestImpl.testDataDir + PERMISSION_FILE);
         logger.info("read permissions from "
-                + baseDir + AbstractAuthorizationTestImpl.testDataDir + "test-permissions.xml");
+                + baseDir + AbstractAuthorizationTestImpl.testDataDir +  PERMISSION_FILE);
         PermissionsRolesList pcrList =
-                (PermissionsRolesList) fromFile(PermissionsRolesList.class, baseDir +
-                AbstractAuthorizationTestImpl.testDataDir + "test-permissions-roles.xml");
+                (PermissionsRolesList) fromFile(PermissionsRolesList.class, baseDir
+                + AbstractAuthorizationTestImpl.testDataDir + PERMISSION_ROLE_FILE);
         logger.info("read permissions-roles from "
-                + baseDir + AbstractAuthorizationTestImpl.testDataDir + "test-permissions.xml");
+                + baseDir + AbstractAuthorizationTestImpl.testDataDir +  PERMISSION_ROLE_FILE);
         AuthZ authZ = AuthZ.get();
         for (Permission p : pcList.getPermissions()) {
             if (logger.isDebugEnabled()) {
@@ -98,7 +102,7 @@ public class AuthorizationSeedTest extends AbstractAuthorizationTestImpl {
         }
     }
 
-        /**
+    /**
      * addPermissionsForUri add permissions from given permission configuration
      * with assumption that resource is of type URI
      * @param permission configuration
@@ -125,7 +129,6 @@ public class AuthorizationSeedTest extends AbstractAuthorizationTestImpl {
         }
     }
 
-
     /**
      * getAction is a convenience method to get corresponding action for
      * given ActionType

diff --git a/services/authorization-mgt/client/src/test/resources/test-data/test-permissions-roles.xml b/services/authorization-mgt/client/src/test/resources/test-data/test-permissions-roles.xml
index ea59f111552ef776a27b3fc3158b2019df927b9c..ef35f5a0f4c4a887016c338c691442539817182d 100644 (file)
--- a/services/authorization-mgt/client/src/test/resources/test-data/test-permissions-roles.xml
+++ b/services/authorization-mgt/client/src/test/resources/test-data/test-permissions-roles.xml
@@ -8,10 +8,6 @@
         </permission>
         <role>
             <roleId>1</roleId>
-            <roleName>ROLE_USERS</roleName>
-        </role>
-        <role>
-            <roleId>2</roleId>
             <roleName>ROLE_ADMINISTRATOR</roleName>
         </role>
     </permissionRole>
@@ -23,10 +19,6 @@
         </permission>
         <role>
             <roleId>1</roleId>
-            <roleName>ROLE_USERS</roleName>
-        </role>
-        <role>
-            <roleId>2</roleId>
             <roleName>ROLE_ADMINISTRATOR</roleName>
         </role>
     </permissionRole>

diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java
index 3bf90ebb3a1b1dd7e6b1047f7ad8a93796bebc9c..be2c8afac3302bc6acbee0df034f05de626cf0cc 100644 (file)
--- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java
+++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java
@@ -122,7 +122,7 @@ public class SpringAuthorizationProvider implements CSpaceAuthorizationProvider
         return sids.toArray(new Sid[0]);
     }
 
-    static Permission mapPermission(CSpaceAction perm) {
+    static Permission mapAction(CSpaceAction perm) {
         switch (perm) {
             case ADMIN:
                 return BasePermission.ADMINISTRATION;

diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java
index 18b6f40d853e81d69aac9f592a34bd63e8851ba9..3f238693dc4fe48a9891f8c99f4083ee66c3103c 100644 (file)
--- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java
+++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java
@@ -50,7 +50,7 @@ public class SpringPermissionEvaluator implements CSpacePermissionEvaluator {
     @Override
     public boolean hasPermission(CSpaceResource res, CSpaceAction perm) {
         PermissionEvaluator eval = provider.getProviderPermissionEvaluator();
-        Permission p = SpringAuthorizationProvider.mapPermission(perm);
+        Permission p = SpringAuthorizationProvider.mapAction(perm);
         Authentication authToken = SecurityContextHolder.getContext().getAuthentication();
         return eval.hasPermission(authToken,
                 Long.valueOf(res.getId().hashCode()),

diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java
index 8b942352b091d648c9f7835f80a310d6b558c5d7..aa1462a828273afca6547d1d6f4538db5ab05a48 100644 (file)
--- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java
+++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java
@@ -61,7 +61,7 @@ public class SpringPermissionManager implements CSpacePermissionManager {
             throws PermissionException {
         ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res);
         Sid[] sids = SpringAuthorizationProvider.mapPrincipal(principals);
-        Permission p = SpringAuthorizationProvider.mapPermission(action);
+        Permission p = SpringAuthorizationProvider.mapAction(action);
         TransactionStatus status = provider.beginTransaction("addPermssions");
 
         //add permission for each sid
@@ -121,7 +121,7 @@ public class SpringPermissionManager implements CSpacePermissionManager {
             throws PermissionNotFoundException, PermissionException {
         ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res);
         Sid[] sids = SpringAuthorizationProvider.mapPrincipal(principals);
-        Permission p = SpringAuthorizationProvider.mapPermission(action);
+        Permission p = SpringAuthorizationProvider.mapAction(action);
         TransactionStatus status = provider.beginTransaction("deletePermssions");
         //delete permission for each sid
         for (Sid sid : sids) {
@@ -178,10 +178,18 @@ public class SpringPermissionManager implements CSpacePermissionManager {
     public void deletePermissions(CSpaceResource res, CSpaceAction action)
             throws PermissionNotFoundException, PermissionException {
         ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res);
-        Permission p = SpringAuthorizationProvider.mapPermission(action);
+        Permission p = SpringAuthorizationProvider.mapAction(action);
         TransactionStatus status = provider.beginTransaction("deletePermssions");
         try {
             deletePermissions(oid, p, null);
+            provider.commitTransaction(status);
+            if (log.isDebugEnabled()) {
+                log.debug("deletepermissions(res,action) success, "
+                        + " res=" + res.toString()
+                        + " action=" + action.toString()
+                        + " oid=" + oid.toString()
+                        + " perm=" + p.toString());
+            }
         } catch (AclDataAccessException aex) {
             provider.rollbackTransaction(status);
             log.debug("deletepermissions(res,action) failed,"
@@ -207,14 +215,6 @@ public class SpringPermissionManager implements CSpacePermissionManager {
             }
             throw new PermissionException(msg, ex);
         }
-        provider.commitTransaction(status);
-        if (log.isDebugEnabled()) {
-            log.debug("deletepermissions(res,action) success, "
-                    + " res=" + res.toString()
-                    + " action=" + action.toString()
-                    + " oid=" + oid.toString()
-                    + " perm=" + p.toString());
-        }
 
     }
 
@@ -222,9 +222,15 @@ public class SpringPermissionManager implements CSpacePermissionManager {
     public void deletePermissions(CSpaceResource res)
             throws PermissionNotFoundException, PermissionException {
         ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res);
-        TransactionStatus status = provider.beginTransaction("addPermssion");
+        TransactionStatus status = provider.beginTransaction("deletePermssion");
         try {
             provider.getProviderAclService().deleteAcl(oid, true);
+            provider.commitTransaction(status);
+            if (log.isDebugEnabled()) {
+                log.debug("deletepermissions(res) success, "
+                        + " res=" + res.toString()
+                        + " oid=" + oid.toString());
+            }
         } catch (AclDataAccessException aex) {
             provider.rollbackTransaction(status);
             log.debug("deletepermissions(res) failed,"
@@ -246,13 +252,6 @@ public class SpringPermissionManager implements CSpacePermissionManager {
             }
             throw new PermissionException(msg, ex);
         }
-        provider.commitTransaction(status);
-
-        if (log.isDebugEnabled()) {
-            log.debug("deletepermissions(res) success, "
-                    + " res=" + res.toString()
-                    + " oid=" + oid.toString());
-        }
     }
 
     private void addPermission(ObjectIdentity oid, Permission permission,
@@ -293,8 +292,8 @@ public class SpringPermissionManager implements CSpacePermissionManager {
         ArrayList<Integer> foundAces = new ArrayList<Integer>();
         Iterator iter = acel.listIterator();
         //not possible to delete while iterating
-        while(iter.hasNext()) {
-            AccessControlEntry ace = (AccessControlEntry)iter.next();
+        while (iter.hasNext()) {
+            AccessControlEntry ace = (AccessControlEntry) iter.next();
             if (sid != null) {
                 if (ace.getSid().equals(sid)
                         && ace.getPermission().equals(permission)) {