From 92f10311a55c52a3c5c679b56548ade7888aa11b Mon Sep 17 00:00:00 2001 From: Aron Roberts Date: Mon, 7 May 2012 14:02:52 -0700 Subject: [PATCH] CSPACE-5155: Fix to the fix for updateItemInstance in three authority item services: Person, Organization, and Storage Location. --- .../test/LocationAuthorityServiceTest.java | 195 ++++++------------ .../location/LocationAuthorityResource.java | 64 +++--- .../client/test/OrgAuthorityServiceTest.java | 6 +- .../test/PersonAuthorityServiceTest.java | 8 +- 4 files changed, 96 insertions(+), 177 deletions(-) diff --git a/services/location/client/src/test/java/org/collectionspace/services/client/test/LocationAuthorityServiceTest.java b/services/location/client/src/test/java/org/collectionspace/services/client/test/LocationAuthorityServiceTest.java index 9ff14e513..1898b9f32 100644 --- a/services/location/client/src/test/java/org/collectionspace/services/client/test/LocationAuthorityServiceTest.java +++ b/services/location/client/src/test/java/org/collectionspace/services/client/test/LocationAuthorityServiceTest.java @@ -22,6 +22,7 @@ */ package org.collectionspace.services.client.test; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -37,6 +38,8 @@ import org.collectionspace.services.common.datetime.GregorianCalendarDateTimeUti import org.collectionspace.services.client.LocationAuthorityClient; import org.collectionspace.services.client.LocationAuthorityClientUtils; import org.collectionspace.services.jaxb.AbstractCommonList; +import org.collectionspace.services.location.LocTermGroup; +import org.collectionspace.services.location.LocTermGroupList; import org.collectionspace.services.location.LocationauthoritiesCommon; import org.collectionspace.services.location.LocationsCommon; @@ -61,8 +64,6 @@ public class LocationAuthorityServiceTest extends AbstractAuthorityServiceTest shelf1Map = new HashMap(); // TODO Make loc type and status be controlled vocabs. - shelf1Map.put(LocationJAXBSchema.NAME, TEST_NAME); shelf1Map.put(LocationJAXBSchema.SHORT_IDENTIFIER, TEST_SHORTID); shelf1Map.put(LocationJAXBSchema.CONDITION_NOTE, TEST_CONDITION_NOTE); shelf1Map.put(LocationJAXBSchema.CONDITION_NOTE_DATE, TEST_CONDITION_NOTE_DATE); @@ -127,10 +127,17 @@ public class LocationAuthorityServiceTest extends AbstractAuthorityServiceTest shelf1Terms = new ArrayList(); + LocTermGroup term = new LocTermGroup(); + term.setTermDisplayName(TEST_NAME); + term.setTermName(TEST_NAME); + term.setTermStatus(TEST_STATUS); + shelf1Terms.add(term); + shelf1Map.put(LocationJAXBSchema.TERM_STATUS, TEST_STATUS); + String newID = LocationAuthorityClientUtils.createItemInAuthority(vcsid, - authRefName, shelf1Map, client ); + authRefName, shelf1Map, shelf1Terms, client ); // Store the ID returned from the first item resource created // for additional tests below. @@ -149,119 +156,13 @@ public class LocationAuthorityServiceTest extends AbstractAuthorityServiceTest res = client.readItem(knownResourceId, knownItemResourceId); - LocationsCommon location = null; - try { - assertStatusCode(res, testName); - // Check whether location has expected displayName. - PoxPayloadIn input = new PoxPayloadIn(res.getEntity()); - location = (LocationsCommon) extractPart(input, - client.getItemCommonPartName(), LocationsCommon.class); - Assert.assertNotNull(location); - } finally { - if (res != null) { - res.releaseConnection(); - } - } - // - // Now prepare an updated payload. - // - String displayName = location.getDisplayName(); - // Make sure displayName matches computed form - String expectedDisplayName = - LocationAuthorityClientUtils.prepareDefaultDisplayName(TEST_NAME); - Assert.assertNotNull(displayName, expectedDisplayName); - - // Update the shortName and verify the computed name is updated. - location.setCsid(null); - location.setDisplayNameComputed(true); - location.setName("updated-" + TEST_NAME); - expectedDisplayName = - LocationAuthorityClientUtils.prepareDefaultDisplayName("updated-" + TEST_NAME); - - // Submit the updated resource to the service and store the response. - PoxPayloadOut output = new PoxPayloadOut(LocationAuthorityClient.SERVICE_ITEM_PAYLOAD_NAME); - PayloadOutputPart commonPart = output.addPart(client.getItemCommonPartName(), location); - - setupUpdate(); - res = client.updateItem(knownResourceId, knownItemResourceId, output); - LocationsCommon updatedLocation = null; - try { - assertStatusCode(res, testName); - // Retrieve the updated resource and verify that its contents exist. - PoxPayloadIn input = new PoxPayloadIn(res.getEntity()); - updatedLocation = (LocationsCommon) extractPart(input, - client.getItemCommonPartName(), LocationsCommon.class); - Assert.assertNotNull(updatedLocation); - } finally { - if (res != null) { - res.releaseConnection(); - } - } - - // Verify that the updated resource received the correct data. - Assert.assertEquals(updatedLocation.getName(), location.getName(), - "Updated ForeName in Location did not match submitted data."); - // Verify that the updated resource computes the right displayName. - Assert.assertEquals(updatedLocation.getDisplayName(), expectedDisplayName, - "Updated ForeName in Location not reflected in computed DisplayName."); - // - // Now Update the displayName, not computed and verify the computed name is overriden. - // - location.setDisplayNameComputed(false); - expectedDisplayName = "TestName"; - location.setDisplayName(expectedDisplayName); - - // Submit the updated resource to the service and store the response. - output = new PoxPayloadOut(LocationAuthorityClient.SERVICE_ITEM_PAYLOAD_NAME); - commonPart = output.addPart(client.getItemCommonPartName(), location); - setupUpdate(); - res = client.updateItem(knownResourceId, knownItemResourceId, output); - try { - assertStatusCode(res, testName); - // Retrieve the updated resource and verify that its contents exist. - PoxPayloadIn input = new PoxPayloadIn(res.getEntity()); - updatedLocation = (LocationsCommon) extractPart(input, - client.getItemCommonPartName(), LocationsCommon.class); - Assert.assertNotNull(updatedLocation); - } finally { - if (res != null) { - res.releaseConnection(); - } - } - - // Verify that the updated resource received the correct data. - Assert.assertEquals(updatedLocation.isDisplayNameComputed(), false, - "Updated displayNameComputed in Location did not match submitted data."); - // Verify that the updated resource computes the right displayName. - Assert.assertEquals(updatedLocation.getDisplayName(), - expectedDisplayName, - "Updated DisplayName (not computed) in Location not stored."); - } - /** * Verify illegal item display name. * * @param testName the test name * @throws Exception the exception */ - @Test(dataProvider="testName", - dependsOnMethods = {"verifyItemDisplayName"}) + @Test(dataProvider="testName") public void verifyIllegalItemDisplayName(String testName) throws Exception { // Perform setup for read. setupRead(); @@ -282,9 +183,18 @@ public class LocationAuthorityServiceTest extends AbstractAuthorityServiceTest terms = termList.getLocTermGroup(); + Assert.assertNotNull(terms); + Assert.assertTrue(terms.size() > 0); + terms.get(0).setTermDisplayName(null); + terms.get(0).setTermName(null); + + setupUpdateWithInvalidBody(); // we expect a failure // Submit the updated resource to the service and store the response. PoxPayloadOut output = new PoxPayloadOut(LocationAuthorityClient.SERVICE_ITEM_PAYLOAD_NAME); @@ -367,10 +277,10 @@ public class LocationAuthorityServiceTest extends AbstractAuthorityServiceTest terms = termList.getLocTermGroup(); + Assert.assertNotNull(terms); + Assert.assertTrue(terms.size() > 0); + terms.get(0).setTermDisplayName("updated-" + terms.get(0).getTermDisplayName()); + terms.get(0).setTermName("updated-" + terms.get(0).getTermName()); + locationsCommon.setLocTermGroupList(termList); + + return locationsCommon; + } @Override protected void compareUpdatedItemInstances(LocationsCommon original, LocationsCommon updated) throws Exception { - Assert.assertEquals(updated.getName(), original.getName(), - "Data in updated Location did not match submitted data."); + LocTermGroupList originalTermList = original.getLocTermGroupList(); + Assert.assertNotNull(originalTermList); + List originalTerms = originalTermList.getLocTermGroup(); + Assert.assertNotNull(originalTerms); + Assert.assertTrue(originalTerms.size() > 0); + + LocTermGroupList updatedTermList = updated.getLocTermGroupList(); + Assert.assertNotNull(updatedTermList); + List updatedTerms = updatedTermList.getLocTermGroup(); + Assert.assertNotNull(updatedTerms); + Assert.assertTrue(updatedTerms.size() > 0); + + Assert.assertEquals(updatedTerms.get(0).getTermDisplayName(), + originalTerms.get(0).getTermDisplayName(), + "Value in updated record did not match submitted data."); } @Override @@ -578,15 +507,13 @@ public class LocationAuthorityServiceTest extends AbstractAuthorityServiceTest nonexMap = new HashMap(); - nonexMap.put(LocationJAXBSchema.NAME, TEST_NAME); - nonexMap.put(LocationJAXBSchema.SHORT_IDENTIFIER, "nonEx"); - nonexMap.put(LocationJAXBSchema.LOCATION_TYPE, TEST_LOCATION_TYPE); - nonexMap.put(LocationJAXBSchema.TERM_STATUS, TEST_STATUS); - final String EMPTY_REFNAME = ""; - PoxPayloadOut result = - LocationAuthorityClientUtils.createLocationInstance(EMPTY_REFNAME, - nonexMap, commonPartName); - return result; + Map nonexMap = new HashMap(); + nonexMap.put(LocationJAXBSchema.SHORT_IDENTIFIER, "nonExistent"); + final String EMPTY_REFNAME = ""; + PoxPayloadOut result = + LocationAuthorityClientUtils.createLocationInstance(EMPTY_REFNAME, nonexMap, + LocationAuthorityClientUtils.getTermGroupInstance(""), commonPartName); + return result; + } } diff --git a/services/location/service/src/main/java/org/collectionspace/services/location/LocationAuthorityResource.java b/services/location/service/src/main/java/org/collectionspace/services/location/LocationAuthorityResource.java index 343ace4c1..0345d378d 100644 --- a/services/location/service/src/main/java/org/collectionspace/services/location/LocationAuthorityResource.java +++ b/services/location/service/src/main/java/org/collectionspace/services/location/LocationAuthorityResource.java @@ -1,25 +1,24 @@ /** - * 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. - * 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, 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. See the + * License for the specific language governing permissions and limitations under + * the License. */ package org.collectionspace.services.location; @@ -36,21 +35,18 @@ import javax.ws.rs.Produces; @Path(LocationAuthorityClient.SERVICE_PATH) @Consumes("application/xml") @Produces("application/xml") -public class LocationAuthorityResource - extends AuthorityResource { +public class LocationAuthorityResource + extends AuthorityResource { private final static String locationAuthorityServiceName = "locationauthorities"; - private final static String LOCATIONAUTHORITIES_COMMON = "locationauthorities_common"; - + private final static String LOCATIONAUTHORITIES_COMMON = "locationauthorities_common"; private final static String locationServiceName = "locations"; - private final static String LOCATIONS_COMMON = "locations_common"; - + private final static String LOCATIONS_COMMON = "locations_common"; final Logger logger = LoggerFactory.getLogger(LocationAuthorityResource.class); public LocationAuthorityResource() { - super(LocationauthoritiesCommon.class, LocationAuthorityResource.class, - LOCATIONAUTHORITIES_COMMON, LOCATIONS_COMMON); + super(LocationauthoritiesCommon.class, LocationAuthorityResource.class, + LOCATIONAUTHORITIES_COMMON, LOCATIONS_COMMON); } @Override @@ -64,11 +60,11 @@ public class LocationAuthorityResource @Override public Class getCommonPartClass() { - return LocationauthoritiesCommon.class; + return LocationauthoritiesCommon.class; } - @Override - public String getItemTermInfoGroupXPathBase() { + @Override + public String getItemTermInfoGroupXPathBase() { return LocationAuthorityClient.TERM_INFO_GROUP_XPATH_BASE; - } + } } diff --git a/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityServiceTest.java b/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityServiceTest.java index b17acce13..011e5eb39 100644 --- a/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityServiceTest.java +++ b/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityServiceTest.java @@ -1072,9 +1072,7 @@ public class OrgAuthorityServiceTest extends AbstractAuthorityServiceTest terms = termList.getOrgTermGroup(); @@ -1084,7 +1082,7 @@ public class OrgAuthorityServiceTest extends AbstractAuthorityServiceTest terms = termList.getPersonTermGroup(); Assert.assertNotNull(terms); @@ -1396,7 +1394,7 @@ public class PersonAuthorityServiceTest extends AbstractAuthorityServiceTest