From b9991a98ccf0399e23f124545c55e84189cc19ba Mon Sep 17 00:00:00 2001 From: Patrick Schmitz Date: Sat, 28 Aug 2010 03:01:02 +0000 Subject: [PATCH] CSPACE-2542 Bug was a side effect of an improperly constructed Person item (passing in a payload with inAuthority set to something different from the authority the Person was actually being added to). This is turn was not detected by services code which should have forced this to be correct. Changed services code for authorities to ensure that this cannot happen. inAuthority values in payloads will be ignored. Made analogous changes to contact service, which has parent references. --- .../test/CollectionObjectAuthRefsTest.java | 2 +- .../AuthorityItemDocumentModelHandler.java | 23 +++++++++ .../AuthorityResourceWithContacts.java | 48 ++++++++++++++----- .../nuxeo/ContactDocumentModelHandler.java | 44 +++++++++++++++++ .../test/OrganizationAuthRefDocsTest.java | 2 +- .../client/LocationAuthorityClientUtils.java | 6 +-- .../test/LocationAuthorityServiceTest.java | 2 +- .../client/OrgAuthorityClientUtils.java | 13 ++--- .../client/test/OrgAuthorityAuthRefsTest.java | 2 +- .../client/test/OrgAuthorityServiceTest.java | 2 +- .../client/VocabularyClientUtils.java | 5 +- .../client/test/VocabularyServiceTest.java | 4 +- 12 files changed, 119 insertions(+), 34 deletions(-) diff --git a/services/collectionobject/client/src/test/java/org/collectionspace/services/client/test/CollectionObjectAuthRefsTest.java b/services/collectionobject/client/src/test/java/org/collectionspace/services/client/test/CollectionObjectAuthRefsTest.java index a00afc0aa..c3148a3d0 100644 --- a/services/collectionobject/client/src/test/java/org/collectionspace/services/client/test/CollectionObjectAuthRefsTest.java +++ b/services/collectionobject/client/src/test/java/org/collectionspace/services/client/test/CollectionObjectAuthRefsTest.java @@ -349,7 +349,7 @@ public class CollectionObjectAuthRefsTest extends BaseServiceTest { orgInfo.put(OrganizationJAXBSchema.SHORT_IDENTIFIER, shortIdentifier); OrgAuthorityClient orgAuthClient = new OrgAuthorityClient(); MultipartOutput multipart = - OrgAuthorityClientUtils.createOrganizationInstance(orgAuthCSID, + OrgAuthorityClientUtils.createOrganizationInstance( orgAuthRefName, orgInfo, orgAuthClient.getItemCommonPartName()); ClientResponse res = orgAuthClient.createItem(orgAuthCSID, multipart); int statusCode = res.getStatus(); diff --git a/services/common/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java b/services/common/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java index 8e6b51d31..8a648a25f 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java +++ b/services/common/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java @@ -72,6 +72,29 @@ public abstract class AuthorityItemDocumentModelHandler this.inAuthority = inAuthority; } + /* (non-Javadoc) + * @see org.collectionspace.services.nuxeo.client.java.DocumentModelHandler#handleCreate(org.collectionspace.services.common.document.DocumentWrapper) + */ + @Override + public void handleCreate(DocumentWrapper wrapDoc) throws Exception { + // first fill all the parts of the document + super.handleCreate(wrapDoc); + handleInAuthority(wrapDoc.getWrappedObject()); + } + + /** + * Check the logic around the parent pointer. Note that we only need do this on + * create, since we have logic to make this read-only on update. + * + * @param docModel + * + * @throws Exception the exception + */ + private void handleInAuthority(DocumentModel docModel) throws Exception { + docModel.setProperty(authorityItemCommonSchemaName, + AuthorityItemJAXBSchema.IN_AUTHORITY, inAuthority); + } + /** * getCommonPart get associated item diff --git a/services/contact/service/src/main/java/org/collectionspace/services/contact/AuthorityResourceWithContacts.java b/services/contact/service/src/main/java/org/collectionspace/services/contact/AuthorityResourceWithContacts.java index a1218f11f..19971c8ee 100644 --- a/services/contact/service/src/main/java/org/collectionspace/services/contact/AuthorityResourceWithContacts.java +++ b/services/contact/service/src/main/java/org/collectionspace/services/contact/AuthorityResourceWithContacts.java @@ -153,7 +153,7 @@ public abstract class AuthorityResourceWithContacts ctx = null; String parentcsid; @@ -263,6 +272,15 @@ public abstract class AuthorityResourceWithContacts ctx = null; String parentcsid; @@ -411,9 +431,11 @@ public abstract class AuthorityResourceWithContacts ctx = null; String parentcsid; @@ -476,7 +498,9 @@ public abstract class AuthorityResourceWithContacts wrapDoc) throws Exception { + // first fill all the parts of the document + super.handleCreate(wrapDoc); + handleInAuthority(wrapDoc.getWrappedObject()); + } + + /** + * Check the logic around the parent pointer. Note that we only need do this on + * create, since we have logic to make this read-only on update. + * + * @param docModel + * + * @throws Exception the exception + */ + private void handleInAuthority(DocumentModel docModel) throws Exception { + String commonPartLabel = getServiceContext().getCommonPartLabel("contacts"); + docModel.setProperty(commonPartLabel, + ContactJAXBSchema.IN_AUTHORITY, inAuthority); + docModel.setProperty(commonPartLabel, + ContactJAXBSchema.IN_ITEM, inItem); + } + /* (non-Javadoc) * @see org.collectionspace.services.nuxeo.client.java.DocumentModelHandler#getCommonPart() */ @@ -178,5 +207,20 @@ public class ContactDocumentModelHandler public String getQProperty(String prop) { return ContactConstants.NUXEO_SCHEMA_NAME + ":" + prop; } + + /** + * Filters out ContactJAXBSchema.IN_AUTHORITY, and IN_ITEM, to ensure that + * the parent links remains untouched. + * @param objectProps the properties parsed from the update payload + * @param partMeta metadata for the object to fill + */ + @Override + public void filterReadOnlyPropertiesForPart( + Map objectProps, ObjectPartType partMeta) { + super.filterReadOnlyPropertiesForPart(objectProps, partMeta); + objectProps.remove(ContactJAXBSchema.IN_AUTHORITY); + objectProps.remove(ContactJAXBSchema.IN_ITEM); + } + } diff --git a/services/intake/client/src/test/java/org/collectionspace/services/client/test/OrganizationAuthRefDocsTest.java b/services/intake/client/src/test/java/org/collectionspace/services/client/test/OrganizationAuthRefDocsTest.java index e32874106..cd949690f 100644 --- a/services/intake/client/src/test/java/org/collectionspace/services/client/test/OrganizationAuthRefDocsTest.java +++ b/services/intake/client/src/test/java/org/collectionspace/services/client/test/OrganizationAuthRefDocsTest.java @@ -209,7 +209,7 @@ public class OrganizationAuthRefDocsTest extends BaseServiceTest { orgInfo.put(OrganizationJAXBSchema.SHORT_NAME, shortName); orgInfo.put(OrganizationJAXBSchema.LONG_NAME, longName); MultipartOutput multipart = - OrgAuthorityClientUtils.createOrganizationInstance(orgAuthCSID, orgAuthRefName, + OrgAuthorityClientUtils.createOrganizationInstance(orgAuthRefName, orgInfo, orgAuthClient.getItemCommonPartName()); ClientResponse res = orgAuthClient.createItem(orgAuthCSID, multipart); int statusCode = res.getStatus(); diff --git a/services/location/client/src/main/java/org/collectionspace/services/client/LocationAuthorityClientUtils.java b/services/location/client/src/main/java/org/collectionspace/services/client/LocationAuthorityClientUtils.java index fac62e1d5..0c00e49f2 100644 --- a/services/location/client/src/main/java/org/collectionspace/services/client/LocationAuthorityClientUtils.java +++ b/services/location/client/src/main/java/org/collectionspace/services/client/LocationAuthorityClientUtils.java @@ -54,19 +54,17 @@ public class LocationAuthorityClientUtils { } /** - * @param inAuthority CSID of the authority to create a new location in * @param locationRefName The proper refName for this authority * @param locationInfo the properties for the new Location * @param headerLabel The common part label * @return The MultipartOutput payload for the create call */ - public static MultipartOutput createLocationInstance(String inAuthority, + public static MultipartOutput createLocationInstance( String locationAuthRefName, Map locationInfo, String headerLabel){ LocationsCommon location = new LocationsCommon(); String shortId = locationInfo.get(LocationJAXBSchema.SHORT_IDENTIFIER); String displayName = locationInfo.get(LocationJAXBSchema.DISPLAY_NAME); location.setShortIdentifier(shortId); - location.setInAuthority(inAuthority); String locationRefName = createLocationRefName(locationAuthRefName, shortId, displayName); location.setRefName(locationRefName); String value = null; @@ -134,7 +132,7 @@ public class LocationAuthorityClientUtils { +"\" in locationAuthority: \"" + locationAuthorityRefName +"\""); } MultipartOutput multipart = - createLocationInstance( vcsid, locationAuthorityRefName, + createLocationInstance( locationAuthorityRefName, locationMap, client.getItemCommonPartName() ); String newID = null; ClientResponse res = client.createItem(vcsid, multipart); 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 1d941d32b..246930bb1 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 @@ -1114,7 +1114,7 @@ public class LocationAuthorityServiceTest extends AbstractServiceTestImpl { nonexMap.put(LocationJAXBSchema.LOCATION_TYPE, TEST_LOCATION_TYPE); nonexMap.put(LocationJAXBSchema.TERM_STATUS, TEST_STATUS); MultipartOutput multipart = - LocationAuthorityClientUtils.createLocationInstance(NON_EXISTENT_ID, + LocationAuthorityClientUtils.createLocationInstance( LocationAuthorityClientUtils.createLocationRefName(knownResourceRefName, "nonEx", "Non Existent"), nonexMap, client.getItemCommonPartName() ); ClientResponse res = diff --git a/services/organization/client/src/main/java/org/collectionspace/services/client/OrgAuthorityClientUtils.java b/services/organization/client/src/main/java/org/collectionspace/services/client/OrgAuthorityClientUtils.java index c0e19b4f7..cb143100a 100644 --- a/services/organization/client/src/main/java/org/collectionspace/services/client/OrgAuthorityClientUtils.java +++ b/services/organization/client/src/main/java/org/collectionspace/services/client/OrgAuthorityClientUtils.java @@ -168,7 +168,7 @@ public class OrgAuthorityClientUtils { * @param client the client * @return the string */ - public static String createItemInAuthority(String inAuthority, + public static String createItemInAuthority( String inAuthority, String orgAuthorityRefName, Map orgInfo, Map> orgRepeatablesInfo, OrgAuthorityClient client) { // Expected status code: 201 Created @@ -182,7 +182,7 @@ public class OrgAuthorityClientUtils { +"\" in orgAuthority: \"" + orgAuthorityRefName +"\""); } MultipartOutput multipart = - createOrganizationInstance(inAuthority, orgAuthorityRefName, + createOrganizationInstance(orgAuthorityRefName, orgInfo, orgRepeatablesInfo, client.getItemCommonPartName()); ClientResponse res = client.createItem(inAuthority, multipart); @@ -211,17 +211,16 @@ public class OrgAuthorityClientUtils { /** * Creates the organization instance. * - * @param inAuthority the in authority * @param orgAuthRefName the owning Authority ref name * @param orgInfo the org info * @param headerLabel the header label * @return the multipart output */ - public static MultipartOutput createOrganizationInstance(String inAuthority, + public static MultipartOutput createOrganizationInstance( String orgAuthRefName, Map orgInfo, String headerLabel){ final Map> EMPTY_ORG_REPEATABLES_INFO = new HashMap>(); - return createOrganizationInstance(inAuthority, orgAuthRefName, + return createOrganizationInstance(orgAuthRefName, orgInfo, EMPTY_ORG_REPEATABLES_INFO, headerLabel); } @@ -229,7 +228,6 @@ public class OrgAuthorityClientUtils { /** * Creates the organization instance. * - * @param inAuthority the in authority * @param orgAuthRefName the owning Authority ref name * @param orgInfo the org info * @param orgRepeatablesInfo names and values of repeatable scalar @@ -237,11 +235,10 @@ public class OrgAuthorityClientUtils { * @param headerLabel the header label * @return the multipart output */ - public static MultipartOutput createOrganizationInstance(String inAuthority, + public static MultipartOutput createOrganizationInstance( String orgAuthRefName, Map orgInfo, Map> orgRepeatablesInfo, String headerLabel){ OrganizationsCommon organization = new OrganizationsCommon(); - organization.setInAuthority(inAuthority); String shortId = orgInfo.get(OrganizationJAXBSchema.SHORT_IDENTIFIER); if (shortId == null || shortId.isEmpty()) { throw new IllegalArgumentException("shortIdentifier cannot be null or empty"); diff --git a/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityAuthRefsTest.java b/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityAuthRefsTest.java index eeb1325f6..090e75d30 100644 --- a/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityAuthRefsTest.java +++ b/services/organization/client/src/test/java/org/collectionspace/services/client/test/OrgAuthorityAuthRefsTest.java @@ -302,7 +302,7 @@ public class OrgAuthorityAuthRefsTest extends BaseServiceTest { subBodyOrgMap.put(OrganizationJAXBSchema.LONG_NAME, subBodyName + " Long Name"); subBodyOrgMap.put(OrganizationJAXBSchema.FOUNDING_PLACE, subBodyName + " Founding Place"); MultipartOutput multipart = - OrgAuthorityClientUtils.createOrganizationInstance(knownResourceId, + OrgAuthorityClientUtils.createOrganizationInstance( knownResourceRefName, subBodyOrgMap, orgAuthClient.getItemCommonPartName()); String result = null; 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 1c74846e0..0aa1b2ad7 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 @@ -1664,7 +1664,7 @@ public class OrgAuthorityServiceTest extends AbstractServiceTestImpl { nonexOrgMap.put(OrganizationJAXBSchema.SHORT_NAME, "Non-existent"); MultipartOutput multipart = OrgAuthorityClientUtils.createOrganizationInstance( - NON_EXISTENT_ID, knownResourceRefName, + knownResourceRefName, nonexOrgMap, client.getItemCommonPartName() ); ClientResponse res = client.updateItem(knownResourceId, NON_EXISTENT_ID, multipart); diff --git a/services/vocabulary/client/src/main/java/org/collectionspace/services/client/VocabularyClientUtils.java b/services/vocabulary/client/src/main/java/org/collectionspace/services/client/VocabularyClientUtils.java index ec6c338ab..f07c21d8a 100644 --- a/services/vocabulary/client/src/main/java/org/collectionspace/services/client/VocabularyClientUtils.java +++ b/services/vocabulary/client/src/main/java/org/collectionspace/services/client/VocabularyClientUtils.java @@ -43,10 +43,9 @@ public class VocabularyClientUtils { // Note that we do not use the map, but we will once we add more info to the // items - public static MultipartOutput createVocabularyItemInstance(String inAuthority, + public static MultipartOutput createVocabularyItemInstance( String vocabularyRefName, Map vocabItemInfo, String headerLabel){ VocabularyitemsCommon vocabularyItem = new VocabularyitemsCommon(); - vocabularyItem.setInAuthority(inAuthority); String shortId = vocabItemInfo.get(AuthorityItemJAXBSchema.SHORT_IDENTIFIER); String displayName = vocabItemInfo.get(AuthorityItemJAXBSchema.DISPLAY_NAME); vocabularyItem.setShortIdentifier(shortId); @@ -78,7 +77,7 @@ public class VocabularyClientUtils { +"\" in personAuthority: \"" + vcsid +"\""); } MultipartOutput multipart = - createVocabularyItemInstance( vcsid, vocabularyRefName, + createVocabularyItemInstance( vocabularyRefName, itemMap, client.getItemCommonPartName() ); ClientResponse res = client.createItem(vcsid, multipart); diff --git a/services/vocabulary/client/src/test/java/org/collectionspace/services/client/test/VocabularyServiceTest.java b/services/vocabulary/client/src/test/java/org/collectionspace/services/client/test/VocabularyServiceTest.java index d244e7b9d..f68e33f1c 100644 --- a/services/vocabulary/client/src/test/java/org/collectionspace/services/client/test/VocabularyServiceTest.java +++ b/services/vocabulary/client/src/test/java/org/collectionspace/services/client/test/VocabularyServiceTest.java @@ -261,7 +261,7 @@ public class VocabularyServiceTest extends AbstractServiceTestImpl { itemInfo.put(AuthorityItemJAXBSchema.SHORT_IDENTIFIER, "Bad Item Short Id!"); itemInfo.put(AuthorityItemJAXBSchema.DISPLAY_NAME, "Bad Item!"); MultipartOutput multipart = - VocabularyClientUtils.createVocabularyItemInstance( knownResourceId, knownResourceRefName, + VocabularyClientUtils.createVocabularyItemInstance( knownResourceRefName, itemInfo, client.getItemCommonPartName() ); ClientResponse res = client.createItem(knownResourceId, multipart); @@ -1109,7 +1109,7 @@ public class VocabularyServiceTest extends AbstractServiceTestImpl { itemInfo.put(AuthorityItemJAXBSchema.SHORT_IDENTIFIER, "nonex"); itemInfo.put(AuthorityItemJAXBSchema.DISPLAY_NAME, "display-nonex"); MultipartOutput multipart = - VocabularyClientUtils.createVocabularyItemInstance(knownResourceId, + VocabularyClientUtils.createVocabularyItemInstance( VocabularyClientUtils.createVocabularyRefName(NON_EXISTENT_ID, null), itemInfo, client.getItemCommonPartName()); ClientResponse res = -- 2.47.3