From: Sanjay Dalal Date: Thu, 3 Jun 2010 19:44:26 +0000 (+0000) Subject: CSPACE-1937 if a role/permission/account does not have any relationship (to permissio... X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=57b42bfde59287fddf2888d67f33e113a70d3113;p=tmp%2Fjakarta-migration.git CSPACE-1937 if a role/permission/account does not have any relationship (to permission, role, role, resp.), return empty result instead of 404 test: added tests for this case in accountrole, permissionrole and rolepermission M authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java M authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RolePermissionServiceTest.java M account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java M account/service/src/main/java/org/collectionspace/services/account/storage/AccountRoleDocumentHandler.java M authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java M common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java --- diff --git a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java index e18c9ab33..dfb538e0f 100644 --- a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java +++ b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java @@ -66,17 +66,13 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { /** The Constant logger. */ private final static String CLASS_NAME = AccountRoleServiceTest.class.getName(); private final static Logger logger = LoggerFactory.getLogger(CLASS_NAME); - // Instance variables specific to this test. /** The known resource id. */ private String knownResourceId = null; - /** The all resource ids created. */ private List allResourceIdsCreated = new ArrayList(); - /** The acc values. */ private Hashtable accValues = new Hashtable(); - /** The role values. */ private Hashtable roleValues = new Hashtable(); /* @@ -97,7 +93,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { @BeforeClass(alwaysRun = true) public void seedData() { String userId = "acc-role-user1"; - String accId = createAccount(userId, "acc-role-test@cspace.org"); + String accId = createAccount(userId, "acc-role-user1-test@cspace.org"); AccountValue ava = new AccountValue(); ava.setScreenName(userId); ava.setUserId(userId); @@ -105,21 +101,13 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { accValues.put(ava.getScreenName(), ava); String userId2 = "acc-role-user2"; - String coAccId = createAccount(userId2, "acc-role-test@cspace.org"); + String coAccId = createAccount(userId2, "acc-role-user2-test@cspace.org"); AccountValue avc = new AccountValue(); avc.setScreenName(userId2); avc.setUserId(userId2); avc.setAccountId(coAccId); accValues.put(avc.getScreenName(), avc); - String ri = "acc-role-user3"; - String iAccId = createAccount(ri, "acc-role-test@cspace.org"); - AccountValue avi = new AccountValue(); - avi.setScreenName(ri); - avi.setUserId(ri); - avi.setAccountId(iAccId); - accValues.put(avi.getScreenName(), avi); - String rn1 = "ROLE_CO1"; String r1RoleId = createRole(rn1); RoleValue rv1 = new RoleValue(); @@ -140,28 +128,28 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override protected CollectionSpaceClient getClientInstance() { - return new AccountRoleClient(); + return new AccountRoleClient(); } - + /* (non-Javadoc) * @see org.collectionspace.services.client.test.BaseServiceTest#getAbstractCommonList(org.jboss.resteasy.client.ClientResponse) */ @Override - protected AbstractCommonList getAbstractCommonList( - ClientResponse response) { - //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 - throw new UnsupportedOperationException(); + protected AbstractCommonList getAbstractCommonList( + ClientResponse response) { + //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 + throw new UnsupportedOperationException(); } - - /* (non-Javadoc) - * @see org.collectionspace.services.client.test.AbstractServiceTestImpl#readPaginatedList(java.lang.String) - */ - @Test(dataProvider = "testName") - @Override + + /* (non-Javadoc) + * @see org.collectionspace.services.client.test.AbstractServiceTestImpl#readPaginatedList(java.lang.String) + */ + @Test(dataProvider = "testName") + @Override public void readPaginatedList(String testName) throws Exception { - //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 - } - + //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 + } + // --------------------------------------------------------------- // CRUD tests : CREATE tests // --------------------------------------------------------------- @@ -188,24 +176,24 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { AccountRoleClient client = new AccountRoleClient(); ClientResponse res = client.create(pv.getAccountId(), accRole); try { - int statusCode = res.getStatus(); - - if (logger.isDebugEnabled()) { - logger.debug(testName + ": status = " + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); - - // Store the ID returned from this create operation - // for additional tests below. - //this is is not important in case of this relationship - knownResourceId = extractId(res); - if (logger.isDebugEnabled()) { - logger.debug(testName + ": knownResourceId=" + knownResourceId); - } + int statusCode = res.getStatus(); + + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + // Store the ID returned from this create operation + // for additional tests below. + //this is is not important in case of this relationship + knownResourceId = extractId(res); + if (logger.isDebugEnabled()) { + logger.debug(testName + ": knownResourceId=" + knownResourceId); + } } finally { - res.releaseConnection(); + res.releaseConnection(); } } @@ -217,7 +205,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { dependsOnMethods = {"create"}) @Override public void createList(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } // Failure outcomes @@ -228,7 +216,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override public void createWithEmptyEntityBody(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } /* (non-Javadoc) @@ -236,7 +224,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override public void createWithMalformedXml(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } /* (non-Javadoc) @@ -244,7 +232,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override public void createWithWrongXmlSchema(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } // --------------------------------------------------------------- @@ -262,7 +250,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { testBanner(testName, CLASS_NAME); } - + // Perform setup. setupRead(); @@ -272,19 +260,19 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { accValues.get("acc-role-user1").getAccountId(), "123"); int statusCode = res.getStatus(); try { - // Check the status code of the response: does it match - // the expected response(s)? - if (logger.isDebugEnabled()) { - logger.debug(testName + ": status = " + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); - - AccountRole output = (AccountRole) res.getEntity(); - Assert.assertNotNull(output); + // Check the status code of the response: does it match + // the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + AccountRole output = (AccountRole) res.getEntity(); + Assert.assertNotNull(output); } finally { - res.releaseConnection(); + res.releaseConnection(); } } @@ -299,7 +287,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { testBanner(testName, CLASS_NAME); } - + // Perform setup. setupReadNonExistent(); @@ -308,16 +296,52 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { ClientResponse res = client.read(this.NON_EXISTENT_ID, "123"); int statusCode = res.getStatus(); try { - // Check the status code of the response: does it match - // the expected response(s)? - if (logger.isDebugEnabled()) { - logger.debug(testName + ": status = " + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + // Check the status code of the response: does it match + // the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); } finally { - res.releaseConnection(); + res.releaseConnection(); + } + } + + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, + dependsOnMethods = {"create"}) + public void readNoRelationship(String testName) throws Exception { + + if (logger.isDebugEnabled()) { + testBanner(testName, CLASS_NAME); + } + + // Perform setup. + setupRead(); + + // Submit the request to the service and store the response. + AccountRoleClient client = new AccountRoleClient(); + ClientResponse res = client.read( + accValues.get("acc-role-user2").getAccountId(), "123"); + int statusCode = res.getStatus(); + try { + // Check the status code of the response: does it match + // the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, Response.Status.OK.getStatusCode()); + AccountRole output = (AccountRole) res.getEntity(); + + String sOutput = objectAsXmlString(output, AccountRole.class); + if(logger.isDebugEnabled()) { + logger.debug(testName + " received " + sOutput); + } + } finally { + res.releaseConnection(); } } @@ -332,7 +356,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, dependsOnMethods = {"createList", "read"}) public void readList(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } // Failure outcomes @@ -348,7 +372,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, dependsOnMethods = {"read", "readList", "readNonExistent"}) public void update(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } // Failure outcomes @@ -359,7 +383,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override public void updateWithEmptyEntityBody(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } /* (non-Javadoc) @@ -367,7 +391,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override public void updateWithMalformedXml(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } /* (non-Javadoc) @@ -375,7 +399,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { */ @Override public void updateWithWrongXmlSchema(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } /* (non-Javadoc) @@ -385,7 +409,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, dependsOnMethods = {"readNonExistent", "testSubmitRequest"}) public void updateNonExistent(String testName) throws Exception { - //FIXME: Should this test really be empty? If so, please comment accordingly. + //FIXME: Should this test really be empty? If so, please comment accordingly. } // --------------------------------------------------------------- @@ -399,11 +423,11 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, dependsOnMethods = {"read"}) public void delete(String testName) throws Exception { - + if (logger.isDebugEnabled()) { testBanner(testName, CLASS_NAME); } - + // Perform setup. setupDelete(); @@ -413,16 +437,16 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { accValues.get("acc-role-user1").getAccountId(), "123"); int statusCode = res.getStatus(); try { - // Check the status code of the response: does it match - // the expected response(s)? - if (logger.isDebugEnabled()) { - logger.debug(testName + ": status = " + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + // Check the status code of the response: does it match + // the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); } finally { - res.releaseConnection(); + res.releaseConnection(); } } @@ -499,34 +523,34 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { @AfterClass(alwaysRun = true) @Override public void cleanUp() { - + setupDelete(); String noTest = System.getProperty("noTestCleanup"); - if(Boolean.TRUE.toString().equalsIgnoreCase(noTest)) { + if (Boolean.TRUE.toString().equalsIgnoreCase(noTest)) { if (logger.isDebugEnabled()) { logger.debug("Skipping Cleanup phase ..."); } return; - } + } if (logger.isDebugEnabled()) { logger.debug("Cleaning up temporary resources created for testing ..."); } - + AccountRoleClient client = new AccountRoleClient(); for (String resourceId : allResourceIdsCreated) { ClientResponse res = client.delete(resourceId, "123"); try { - int statusCode = res.getStatus(); - if (logger.isDebugEnabled()) { - logger.debug("clenaup: delete relationships for accission id=" - + resourceId + " status=" + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + int statusCode = res.getStatus(); + if (logger.isDebugEnabled()) { + logger.debug("clenaup: delete relationships for accission id=" + + resourceId + " status=" + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); } finally { - res.releaseConnection(); + res.releaseConnection(); } } @@ -547,13 +571,13 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { * @return the string */ private String createAccount(String userName, String email) { - + if (logger.isDebugEnabled()) { testBanner("createAccount"); } - + setupCreate(); - + AccountClient accClient = new AccountClient(); AccountsCommon account = AccountFactory.createAccountInstance( userName, userName, userName, email, @@ -577,26 +601,26 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { * @param accId the acc id */ private void deleteAccount(String accId) { - + if (logger.isDebugEnabled()) { testBanner("deleteAccount"); } - + setupDelete(); - + AccountClient accClient = new AccountClient(); ClientResponse res = accClient.delete(accId); int statusCode = res.getStatus(); try { - if (logger.isDebugEnabled()) { - logger.debug("deleteAccount: delete account id=" - + accId + " status=" + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + if (logger.isDebugEnabled()) { + logger.debug("deleteAccount: delete account id=" + + accId + " status=" + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); } finally { - res.releaseConnection(); + res.releaseConnection(); } } @@ -642,15 +666,15 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { ClientResponse res = roleClient.delete(roleId); int statusCode = res.getStatus(); try { - if (logger.isDebugEnabled()) { - logger.debug("deleteRole: delete role id=" + roleId - + " status=" + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + if (logger.isDebugEnabled()) { + logger.debug("deleteRole: delete role id=" + roleId + + " status=" + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); } finally { - res.releaseConnection(); + res.releaseConnection(); } } } diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountRoleDocumentHandler.java b/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountRoleDocumentHandler.java index 7acb87f90..7d2c0ceb1 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountRoleDocumentHandler.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountRoleDocumentHandler.java @@ -85,6 +85,9 @@ public class AccountRoleDocumentHandler List arrl = wrapDoc.getWrappedObject(); AccountRole ar = new AccountRole(); SubjectType subject = getSubject(getServiceContext()); + if (arrl.size() == 0) { + return ar; + } AccountRoleRel ar0 = arrl.get(0); if (SubjectType.ROLE.equals(subject)) { @@ -130,6 +133,7 @@ public class AccountRoleDocumentHandler //subject mismatch should have been checked during validation } if (subject.equals(SubjectType.ROLE)) { + //FIXME: potential index out of bounds exception...negative test needed AccountValue av = ar.getAccounts().get(0); for (RoleValue rv : ar.getRoles()) { @@ -137,6 +141,7 @@ public class AccountRoleDocumentHandler arrl.add(arr); } } else if (SubjectType.ACCOUNT.equals(subject)) { + //FIXME: potential index out of bounds exception...negative test needed RoleValue rv = ar.getRoles().get(0); for (AccountValue av : ar.getAccounts()) { AccountRoleRel arr = buildAccountRoleRel(av, rv); @@ -213,8 +218,8 @@ public class AccountRoleDocumentHandler static SubjectType getSubject(ServiceContext ctx) { Object o = ctx.getProperty(ServiceContextProperties.SUBJECT); if (o == null) { - throw new IllegalArgumentException(ServiceContextProperties.SUBJECT + - " property is missing in context " + throw new IllegalArgumentException(ServiceContextProperties.SUBJECT + + " property is missing in context " + ctx.toString()); } return (SubjectType) o; diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java index 00e3d0421..41fc32752 100644 --- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java +++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java @@ -67,7 +67,6 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { /** The Constant logger. */ private final static String CLASS_NAME = PermissionRoleServiceTest.class.getName(); private final static Logger logger = LoggerFactory.getLogger(CLASS_NAME); - // Instance variables specific to this test. /** The known resource id. */ private String knownResourceId = null; @@ -75,6 +74,7 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { private List allResourceIdsCreated = new ArrayList(); final private static String TEST_MARKER = "_PermissionRoleServiceTest"; final private static String TEST_SERVICE_NAME = "fakeservice"; + final private static String NO_REL_SUFFIX = "-no-rel"; /** The perm values. */ private Hashtable permValues = new Hashtable(); /** The role values. */ @@ -103,12 +103,12 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { pva.setPermissionId(accPermId); permValues.put(pva.getResourceName(), pva); -// String rc = "collectionobjects"; -// String coPermId = createPermission(rc, EffectType.DENY); -// PermissionValue pvc = new PermissionValue(); -// pvc.setResourceName(rc); -// pvc.setPermissionId(coPermId); -// permValues.put(pvc.getResourceName(), pvc); + String rc = TEST_SERVICE_NAME + TEST_MARKER + NO_REL_SUFFIX; + String coPermId = createPermission(rc, EffectType.DENY); + PermissionValue pvc = new PermissionValue(); + pvc.setResourceName(rc); + pvc.setPermissionId(coPermId); + permValues.put(pvc.getResourceName(), pvc); // // String ri = "intakes"; // String iPermId = createPermission(ri, EffectType.DENY); @@ -173,7 +173,7 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { logger.debug(testBanner(testName, CLASS_NAME)); } - + // Perform setup, such as initializing the type of service request // (e.g. CREATE, DELETE), its valid and expected status codes, and // its associated HTTP method name (e.g. POST, DELETE). @@ -329,6 +329,46 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { } } + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, + dependsOnMethods = {"create"}) + public void readNoRelationship(String testName) throws Exception { + + if (logger.isDebugEnabled()) { + logger.debug(testBanner(testName, CLASS_NAME)); + } + // Perform setup. + setupRead(); + + // Submit the request to the service and store the response. + PermissionRoleClient client = new PermissionRoleClient(); + ClientResponse res = null; + try { + res = client.read( + permValues.get(TEST_SERVICE_NAME + TEST_MARKER + NO_REL_SUFFIX).getPermissionId(), "123"); + int statusCode = res.getStatus(); + + // Check the status code of the response: does it match + // the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, Response.Status.OK.getStatusCode()); + + PermissionRole output = (PermissionRole) res.getEntity(); + + String sOutput = objectAsXmlString(output, PermissionRole.class); + if (logger.isDebugEnabled()) { + logger.debug(testName + " received " + sOutput); + } + } finally { + if (res != null) { + res.releaseConnection(); + } + } + + } // --------------------------------------------------------------- // CRUD tests : READ_LIST tests // --------------------------------------------------------------- @@ -336,6 +376,7 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { /* (non-Javadoc) * @see org.collectionspace.services.client.test.AbstractServiceTestImpl#readList(java.lang.String) */ + @Override @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, dependsOnMethods = {"createList", "read"}) diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RolePermissionServiceTest.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RolePermissionServiceTest.java index 9bc8714c5..bb33ec820 100644 --- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RolePermissionServiceTest.java +++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RolePermissionServiceTest.java @@ -68,7 +68,6 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { /** The Constant logger. */ private final static String CLASS_NAME = RolePermissionServiceTest.class.getName(); private final static Logger logger = LoggerFactory.getLogger(CLASS_NAME); - // Instance variables specific to this test. /** The known resource id. */ private String knownResourceId = null; @@ -76,11 +75,11 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { private List allResourceIdsCreated = new ArrayList(); final private static String TEST_MARKER = "_RolePermissionServiceTest"; final private static String TEST_ROLE_NAME = "ROLE"; + final private static String NO_REL_SUFFIX = "-no-rel"; /** The perm values. */ private Hashtable permValues = new Hashtable(); /** The role values. */ private Hashtable roleValues = new Hashtable(); - private Date now = new Date(); /* * This method is called only by the parent class, AbstractServiceTestImpl @@ -97,6 +96,7 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { private String getRoleName() { return TEST_ROLE_NAME + TEST_MARKER + now.toString(); } + /** * Seed data. */ @@ -110,6 +110,13 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { rv1.setRoleName(rn1); roleValues.put(rv1.getRoleName(), rv1); + String rn2 = getRoleName() + NO_REL_SUFFIX; + String r2RoleId = createRole(rn2); + RoleValue rv2 = new RoleValue(); + rv2.setRoleId(r2RoleId); + rv2.setRoleName(rn2); + roleValues.put(rv2.getRoleName(), rv2); + String ra1 = "fooService" + TEST_MARKER; String permId1 = createPermission(ra1, EffectType.PERMIT); PermissionValue pva1 = new PermissionValue(); @@ -165,7 +172,8 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { logger.debug(testBanner(testName, CLASS_NAME)); - }; + } + ; // Perform setup, such as initializing the type of service request // (e.g. CREATE, DELETE), its valid and expected status codes, and // its associated HTTP method name (e.g. POST, DELETE). @@ -254,7 +262,8 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { logger.debug(testBanner(testName, CLASS_NAME)); - }; + } + ; // Perform setup. setupRead(); @@ -294,7 +303,8 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { logger.debug(testBanner(testName, CLASS_NAME)); - }; + } + ; // Perform setup. setupReadNonExistent(); @@ -321,6 +331,43 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { } } + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class) + public void readNoRelationship(String testName) throws Exception { + + if (logger.isDebugEnabled()) { + logger.debug(testBanner(testName, CLASS_NAME)); + } + ; + setupRead(); + // Submit the request to the service and store the response. + RolePermissionClient client = new RolePermissionClient(); + ClientResponse res = null; + try { + + res = client.read(roleValues.get(getRoleName() + NO_REL_SUFFIX).getRoleId(), "123"); + int statusCode = res.getStatus(); + + // Check the status code of the response: does it match + // the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, Response.Status.OK.getStatusCode()); + PermissionRole output = (PermissionRole) res.getEntity(); + + String sOutput = objectAsXmlString(output, PermissionRole.class); + if(logger.isDebugEnabled()) { + logger.debug(testName + " received " + sOutput); + } + } finally { + if (res != null) { + res.releaseConnection(); + } + } + } + // --------------------------------------------------------------- // CRUD tests : READ_LIST tests // --------------------------------------------------------------- @@ -402,7 +449,8 @@ public class RolePermissionServiceTest extends AbstractServiceTestImpl { if (logger.isDebugEnabled()) { logger.debug(testBanner(testName, CLASS_NAME)); - }; + } + ; // Perform setup. setupDelete(); diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java index 87640f908..ae4303a80 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java @@ -82,7 +82,6 @@ public class PermissionRoleDocumentHandler throw new UnsupportedOperationException("operation not relevant for PermissionRoleDocumentHandler"); } - @Override public void completeDelete(DocumentWrapper> wrapDoc) throws Exception { // @@ -95,6 +94,9 @@ public class PermissionRoleDocumentHandler List prrl = wrapDoc.getWrappedObject(); PermissionRole pr = new PermissionRole(); SubjectType subject = PermissionRoleUtil.getRelationSubject(getServiceContext()); + if (prrl.size() == 0) { + return pr; + } PermissionRoleRel prr0 = prrl.get(0); if (SubjectType.ROLE.equals(subject)) { @@ -140,12 +142,14 @@ public class PermissionRoleDocumentHandler //subject mismatch should have been checked during validation } if (subject.equals(SubjectType.ROLE)) { + //FIXME: potential index out of bounds exception...negative test needed PermissionValue pv = pr.getPermissions().get(0); for (RoleValue rv : pr.getRoles()) { PermissionRoleRel prr = buildPermissonRoleRel(pv, rv); prrl.add(prr); } } else if (SubjectType.PERMISSION.equals(subject)) { + //FIXME: potential index out of bounds exception...negative test needed RoleValue rv = pr.getRoles().get(0); for (PermissionValue pv : pr.getPermissions()) { PermissionRoleRel prr = buildPermissonRoleRel(pv, rv); diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java index 264bb173d..8787ab6b1 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java @@ -145,10 +145,18 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { } if (getObject(ctx, id) == null) { String msg = "get: " - + "could not find the object entity with id=" + id; + + "could not find the object with id=" + id; logger.error(msg); throw new DocumentNotFoundException(msg); } + String objectId = getObjectId(ctx); + if (logger.isDebugEnabled()) { + logger.debug("get: using objectId=" + objectId); + } + Class objectClass = getObjectClass(ctx); + if (logger.isDebugEnabled()) { + logger.debug("get: using object class=" + objectClass.getName()); + } DocumentFilter docFilter = handler.getDocumentFilter(); if (docFilter == null) { docFilter = handler.createDocumentFilter(); @@ -160,10 +168,7 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { StringBuilder queryStrBldr = new StringBuilder("SELECT a FROM "); queryStrBldr.append(getEntityName(ctx)); queryStrBldr.append(" a"); - String objectId = getObjectId(ctx); - if (logger.isDebugEnabled()) { - logger.debug("get: using objectId=" + objectId); - } + queryStrBldr.append(" WHERE " + objectId + " = :objectId"); String where = docFilter.getWhereClause(); if ((null != where) && (where.length() > 0)) { @@ -188,16 +193,20 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { if (em != null && em.getTransaction().isActive()) { em.getTransaction().rollback(); } - String msg = "get: " - + " could not find entity with id=" + id; - logger.error(msg, nre); - throw new DocumentNotFoundException(msg, nre); + String msg = "get(1): " + + " could not find relationships for object class=" + + objectClass.getName() + " id=" + id; + if (logger.isDebugEnabled()) { + logger.debug(msg, nre); + } } if (rl.size() == 0) { - String msg = "get: " - + " could not find entity with id=" + id; - logger.error(msg); - throw new DocumentNotFoundException(msg); + String msg = "get(2): " + + " could not find relationships for object class=" + + objectClass.getName() + " id=" + id; + if (logger.isDebugEnabled()) { + logger.debug(msg); + } } DocumentWrapper> wrapDoc = new DocumentWrapperImpl>(rl); @@ -236,20 +245,24 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { } if (getObject(ctx, id) == null) { String msg = "delete : " - + "could not find the object entity with id=" + id; + + "could not find the object with id=" + id; logger.error(msg); throw new DocumentNotFoundException(msg); } + String objectId = getObjectId(ctx); + if (logger.isDebugEnabled()) { + logger.debug("delete: using objectId=" + objectId); + } + Class objectClass = getObjectClass(ctx); + if (logger.isDebugEnabled()) { + logger.debug("delete: using object class=" + objectClass.getName()); + } EntityManagerFactory emf = null; EntityManager em = null; try { StringBuilder deleteStr = new StringBuilder("DELETE FROM "); String entityName = getEntityName(ctx); deleteStr.append(entityName); - String objectId = getObjectId(ctx); - if (logger.isDebugEnabled()) { - logger.debug("delete: using objectId=" + objectId); - } deleteStr.append(" WHERE " + objectId + " = :objectId"); emf = JpaStorageUtils.getEntityManagerFactory(); em = emf.createEntityManager(); @@ -290,8 +303,8 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { protected String getObjectId(ServiceContext ctx) { String objectId = (String) ctx.getProperty(ServiceContextProperties.OBJECT_ID); if (objectId == null) { - String msg = ServiceContextProperties.OBJECT_ID + - " property is missing in the context"; + String msg = ServiceContextProperties.OBJECT_ID + + " property is missing in the context"; logger.error(msg); throw new IllegalArgumentException(msg); } @@ -300,19 +313,29 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { } /** - * getObject returns the object in the relationship + * getObjectClass returns the class of the object in a relationship * @param ctx - * @param id * @return */ - protected Object getObject(ServiceContext ctx, String id) { + protected Class getObjectClass(ServiceContext ctx) { Class objectClass = (Class) ctx.getProperty(ServiceContextProperties.OBJECT_CLASS); if (objectClass == null) { - String msg = ServiceContextProperties.OBJECT_CLASS + - " property is missing in the context"; + String msg = ServiceContextProperties.OBJECT_CLASS + + " property is missing in the context"; logger.error(msg); throw new IllegalArgumentException(msg); } + return objectClass; + } + + /** + * getObject returns the object in the relationship + * @param ctx + * @param id + * @return + */ + protected Object getObject(ServiceContext ctx, String id) { + Class objectClass = getObjectClass(ctx); return JpaStorageUtils.getEntity(id, objectClass); } }