From aaf58ea7598a63d72595cd6bc9e8c00dc96ffaca Mon Sep 17 00:00:00 2001 From: Aron Roberts Date: Fri, 1 Feb 2013 15:03:32 -0800 Subject: [PATCH] CSPACE-5845: First minor changes toward storing collectionspace_core:refName values with imported records. CSPACE-5845: Created initial (failing) test of refName value in collectionspace_core. CSPACE-5845: Placeholder method added for inserting refName values into collectionspace_core on import. CSPACE-5845: Plain old id-style refNames now generated by Imports service and added to collectionspace_core:refName. CSPACE-5845: Added (as yet untested) code to insert a provided refName, if any, into collectionspace_core:refName, and to add a display name to the ID form of refNames if the relevant service supports hierarchy. CSPACE-5845: Generation of display name for refNames for record types with hierarchy is now working, with its own unit test. CSPACE-5845: Add null checks. Add tests for extracting refName value from an authority record and adding that value to collectionspace_core:refName. CSPACE-5845: Remove currently-irrelevant comments. CSPACE-5845: Removed call in (partially-implemented) service test class that was triggering dependency-related issues. CSPACE-5845: Added code (untested) to generate short identifier-based refNames for authority records, and to skip generating refNames for authority item records (rather than defaulting to giving them id-based refNames). CSPACE-5845: Added test for generated authority record refName. --- .../import-collectionobject-varexpansion.xml | 12 ++ .../import-locationauthority-varexpansion.xml | 11 ++ .../test-data/xmlreplay/imports/imports.xml | 160 +++++++++++++++- ...port-collectionobject-varexpansion.res.xml | 21 +++ .../res/import-collectionobject.res.xml | 9 + ...ort-locationauthority-varexpansion.res.xml | 16 ++ .../res/import-locationauthority.res.xml | 9 + .../res/import-objectexit-dollarsign.res.xml | 3 +- .../import-objectexit-varexpansion.res.xml | 5 +- .../services/imports/TemplateExpander.java | 172 +++++++++++++++++- .../resources/templates/service-document.xml | 1 + .../services/test/ImportsServiceTest.java | 14 +- 12 files changed, 416 insertions(+), 17 deletions(-) create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-collectionobject-varexpansion.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-locationauthority-varexpansion.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-collectionobject-varexpansion.res.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-collectionobject.res.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority-varexpansion.res.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority.res.xml diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-collectionobject-varexpansion.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-collectionobject-varexpansion.xml new file mode 100644 index 000000000..93db3c447 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-collectionobject-varexpansion.xml @@ -0,0 +1,12 @@ + + + + + ${objectNumberValue} + + ${briefDescriptionValue} + + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-locationauthority-varexpansion.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-locationauthority-varexpansion.xml new file mode 100644 index 000000000..bedeb4429 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/import-locationauthority-varexpansion.xml @@ -0,0 +1,11 @@ + + + + + ${displayNameValue} + ${shortIdentifierValue} + ${refNameValue} + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/imports.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/imports.xml index ba529c290..716a6961d 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/imports.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/imports.xml @@ -5,8 +5,8 @@ YWRtaW5AY29yZS5jb2xsZWN0aW9uc3BhY2Uub3JnOkFkbWluaXN0cmF0b3I= - - + + @@ -81,9 +81,9 @@ * Insert the value of that CSID into a field via expansion of the ${docID} variable. * Verify that variable has been expanded in the imported record. - * Verify that a server-side variable has also been expanded in the + * Verify that server-side variables have also been expanded in the wrapper template for imported records; specifically, that an - expected value is is present in the collectionspace_core part. + expected values are present in the collectionspace_core part. (See "Variables supported in expansion of request" in http://wiki.collectionspace.org/display/collectionspace/Imports+Service+Home) --> @@ -115,6 +115,10 @@ + + /objectexit/${importObjectExitWithVarExpansion.recordCSID} + urn:cspace:core.collectionspace.org:objectexit:id(${importObjectExitWithVarExpansion.recordCSID}) + @@ -123,6 +127,154 @@ /cspace-services/objectexit/${importObjectExitWithVarExpansion.recordCSID} + + + + + + + 200 + POST + /cspace-services/imports + imports/import-collectionobject-varexpansion.xml + + 715dc8f1-9846-4da7-90ab-b4224936e9e2 + COLLECTIONOBJECT-IMPORT-TEST-1999.7 + Photograph of a Kordofan Giraffe near an Acacia shrub. + + + + imports/res/import-collectionobject.res.xml + + + + 200 + GET + /cspace-services/collectionobjects/${importCollectionObjectWithVarExpansion.recordCSID} + + imports/res/import-collectionobject-varexpansion.res.xml + + + + + + + + + + + /collectionobjects/${importCollectionObjectWithVarExpansion.recordCSID} + + + + urn:cspace:core.collectionspace.org:collectionobjects:id(${importCollectionObjectWithVarExpansion.recordCSID})'${importCollectionObjectWithVarExpansion.objectNumberValue}' + + + + + 200 + DELETE + /cspace-services/collectionobjects/${importCollectionObjectWithVarExpansion.recordCSID} + + + + + + + 200 + POST + /cspace-services/imports + imports/import-locationauthority-varexpansion.xml + + 88b7753c-f63b-4254-80a2-4dfcb3342c63 + Outbuilding Locations + outbuildinglocations + urn:cspace:core.collectionspace.org:locationauthorities:name(outbuildinglocations)'Outbuilding Locations' + + + + imports/res/import-locationauthority.res.xml + + + + 200 + GET + /cspace-services/locationauthorities/${importLocationAuthorityWithVarExpansion.recordCSID} + + imports/res/import-locationauthority-varexpansion.res.xml + + + + + + + + + + + ${importLocationAuthorityWithVarExpansion.displayNameValue} + ${importLocationAuthorityWithVarExpansion.shortIdentifierValue} + /locationauthorities/${importLocationAuthorityWithVarExpansion.recordCSID} + ${importLocationAuthorityWithVarExpansion.refNameValue} + + + + + 200 + DELETE + /cspace-services/locationauthorities/${importLocationAuthorityWithVarExpansion.recordCSID} + + + + + + + + 200 + POST + /cspace-services/imports + imports/import-locationauthority-varexpansion.xml + + c0589d54-64cb-4c9a-9fa2-6ef1e7ab75db + Hill Locations + hilllocations + + + + + + imports/res/import-locationauthority.res.xml + + + + 200 + GET + /cspace-services/locationauthorities/${importLocationAuthorityWithGeneratedRefName.recordCSID} + + imports/res/import-locationauthority-varexpansion.res.xml + + + + + + + + + + + ${importLocationAuthorityWithGeneratedRefName.displayNameValue} + ${importLocationAuthorityWithGeneratedRefName.shortIdentifierValue} + /locationauthorities/${importLocationAuthorityWithGeneratedRefName.recordCSID} + urn:cspace:core.collectionspace.org:locationauthorities:name(hilllocations)'Hill Locations' + + + + + 200 + DELETE + /cspace-services/locationauthorities/${importLocationAuthorityWithGeneratedRefName.recordCSID} + + + 1 + ${uriValue} + + + + ${refNameValue} + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-collectionobject.res.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-collectionobject.res.xml new file mode 100644 index 000000000..73808bb5a --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-collectionobject.res.xml @@ -0,0 +1,9 @@ + + + 1 + + CollectionObject + 1 + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority-varexpansion.res.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority-varexpansion.res.xml new file mode 100644 index 000000000..7d3407e91 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority-varexpansion.res.xml @@ -0,0 +1,16 @@ + + + + ${displayNameValue} + ${shortIdentifierValue} + + + + 1 + ${uriValue} + ${refNameValue} + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority.res.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority.res.xml new file mode 100644 index 000000000..fa8c2619c --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-locationauthority.res.xml @@ -0,0 +1,9 @@ + + + 1 + + LocationAuthority + 1 + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-dollarsign.res.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-dollarsign.res.xml index 30dbe662e..aea8fa960 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-dollarsign.res.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-dollarsign.res.xml @@ -1,6 +1,7 @@ - + A dollar sign followed by '29.00': $29.00 And a backslash: \ OE-IMPORT-TEST-1999.10 diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-varexpansion.res.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-varexpansion.res.xml index d007a8b73..1d4966913 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-varexpansion.res.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/imports/res/import-objectexit-varexpansion.res.xml @@ -8,6 +8,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"> 1 + ${uriValue} + ${refNameValue} - - + \ No newline at end of file diff --git a/services/imports/service/src/main/java/org/collectionspace/services/imports/TemplateExpander.java b/services/imports/service/src/main/java/org/collectionspace/services/imports/TemplateExpander.java index 36a8dcd59..d172814d2 100644 --- a/services/imports/service/src/main/java/org/collectionspace/services/imports/TemplateExpander.java +++ b/services/imports/service/src/main/java/org/collectionspace/services/imports/TemplateExpander.java @@ -40,7 +40,14 @@ import org.collectionspace.services.common.XmlSaxFragmenter; import org.collectionspace.services.common.XmlTools; import org.collectionspace.services.common.api.FileTools; import org.collectionspace.services.common.api.GregorianCalendarDateTimeUtils; +import org.collectionspace.services.common.api.RefName; +import org.collectionspace.services.common.api.RefNameUtils; import org.collectionspace.services.common.api.Tools; +import org.collectionspace.services.common.config.TenantBindingConfigReaderImpl; +import org.collectionspace.services.config.service.DocHandlerParams; +import org.collectionspace.services.config.service.ListResultField; +import org.collectionspace.services.config.service.ServiceBindingType; +import org.collectionspace.services.config.tenant.TenantBindingType; import org.collectionspace.services.nuxeo.util.NuxeoUtils; import org.dom4j.Attribute; import org.dom4j.Document; @@ -69,12 +76,20 @@ public class TemplateExpander { // non-namespace-qualified elements. private static final String IN_AUTHORITY_NAMESPACE_XPATH = "//*[local-name()='inAuthority']"; private static final String IN_AUTHORITY_NO_NAMESPACE_XPATH = "//inAuthority"; + private static final String REFNAME_NAMESPACE_XPATH = "//*[local-name()='refName']"; + private static final String REFNAME_NO_NAMESPACE_XPATH = "//refName"; + private static final String SHORT_IDENTIFIER_NAMESPACE_XPATH = "//*[local-name()='shortIdentifier']"; + private static final String SHORT_IDENTIFIER_NO_NAMESPACE_XPATH = "//shortIdentifier"; + private static final String DISPLAYNAME_NAMESPACE_XPATH = "//*[local-name()='displayName']"; + private static final String DISPLAYNAME_NO_NAMESPACE_XPATH = "//displayName"; private static final String SERVICE_ATTRIBUTE = "service"; private static final String TYPE_ATTRIBUTE = "type"; private static final String CREATED_AT_ATTRIBUTE = "createdAt"; private static final String CREATED_BY_ATTRIBUTE = "createdBy"; private static final String UPDATED_AT_ATTRIBUTE = "updatedAt"; private static final String UPDATED_BY_ATTRIBUTE = "updatedBy"; + private static TenantBindingConfigReaderImpl tReader = + ServiceMain.getInstance().getTenantBindingConfigReader(); protected static String var(String theVar) { return "\\$\\{" + theVar + "\\}"; @@ -160,6 +175,9 @@ public class TemplateExpander { getAttributeValue(perRecordAttributes, UPDATED_BY_ATTRIBUTE)); wrapperTmpl = Tools.searchAndReplace(wrapperTmpl, var("uri"), getDocUri(tenantId, SERVICE_TYPE, docID, partTmpl)); + wrapperTmpl = Tools.searchAndReplace(wrapperTmpl, var("refName"), + getRefName(tenantId, SERVICE_TYPE, docID, partTmpl)); + String serviceDir = outDir + '/' + docID; FileTools.saveFile(serviceDir, "document.xml", wrapperTmpl, FileTools.FORCE_CREATE_PARENT_DIRS); @@ -257,16 +275,40 @@ public class TemplateExpander { // their uniqueness against those already present in a running system. // - ADR 2012-05-24 private static String getInAuthorityValue(String xmlFragment) { - String inAuthorityValue = ""; - // Check in two ways for the inAuthority value: one intended for records with - // namespace-qualified elements, the second for unqualified elements. + return extractNamespacedOrNonNamespacedValue(xmlFragment, + IN_AUTHORITY_NAMESPACE_XPATH, IN_AUTHORITY_NO_NAMESPACE_XPATH); + } + + private static String getRefNameValue(String xmlFragment) { + return extractNamespacedOrNonNamespacedValue(xmlFragment, + REFNAME_NAMESPACE_XPATH, REFNAME_NO_NAMESPACE_XPATH); + } + + private static String getShortIdentifierValue(String xmlFragment) { + return extractNamespacedOrNonNamespacedValue(xmlFragment, + SHORT_IDENTIFIER_NAMESPACE_XPATH, SHORT_IDENTIFIER_NO_NAMESPACE_XPATH); + } + + private static String getDisplayNameValue(String xmlFragment) { + return extractNamespacedOrNonNamespacedValue(xmlFragment, + DISPLAYNAME_NAMESPACE_XPATH, DISPLAYNAME_NO_NAMESPACE_XPATH); + } + + private static String extractNamespacedOrNonNamespacedValue(String xmlFragment, + String namespacedXpath, String nonNamespacedXpath) { + String value = ""; + if (Tools.isBlank(namespacedXpath) || Tools.isBlank(nonNamespacedXpath)) { + return value; + } + // Extract the value using two XPath expressions: one intended for records + // with namespace-qualified elements, the second for unqualified elements. // (There may be a more elegant way to do this with a single XPath expression, // via an OR operator or the like.) - inAuthorityValue = extractValueFromXmlFragment(IN_AUTHORITY_NAMESPACE_XPATH, xmlFragment); - if (Tools.isBlank(inAuthorityValue)) { - inAuthorityValue = extractValueFromXmlFragment(IN_AUTHORITY_NO_NAMESPACE_XPATH, xmlFragment); + value = extractValueFromXmlFragment(namespacedXpath, xmlFragment); + if (Tools.isBlank(value)) { + value = extractValueFromXmlFragment(nonNamespacedXpath, xmlFragment); } - return inAuthorityValue; + return value; } // FIXME: Need to handle cases here where the xmlFragment may contain more @@ -276,8 +318,11 @@ public class TemplateExpander { private static String extractValueFromXmlFragment(String xpathExpr, String xmlFragment) { String value = ""; try { - // FIXME: Cruelly ugly hack; at this point for imported records - // with more than one child, we have a non-well-formed fragment. + // FIXME: This '' wrapper is a crudely ugly hack. This hack is + // used because, at this point, when handling imported records that + // contain more than one child, we have only a non-well-formed + // fragment to work with, and thus need we to wrap that fragment in a + // container element. String xmlFragmentWrapped = "" + xmlFragment + ""; InputSource input = new InputSource(new StringReader(xmlFragmentWrapped)); value = xpath.evaluate(xpathExpr, input); @@ -296,6 +341,115 @@ public class TemplateExpander { return attributeVal; } + private static String getRefName(String tenantId, String serviceType, String docID, String partTmpl) { + String refName = ""; + String refNameDisplayName = ""; + String shortIdentifier = ""; + + // If a valid refName value is provided within the imported record, + // use that value + refName = getRefNameValue(partTmpl); + if (Tools.notBlank(refName)) { + // Validate the provided refName by attempting to parse it + if (RefNameUtils.parseAuthorityInfo(refName) != null + || RefNameUtils.parseAuthorityTermInfo(refName) != null) { + return refName; + } else { + // If the provided refName wasn't valid, reset the refName value + refName = ""; + } + } + + shortIdentifier = getShortIdentifierValue(partTmpl); + if (Tools.notBlank(shortIdentifier)) { + // By inference, if a record contains values for both shortIdentifier + // and inAuthority, it is an authority item (term) record + if (Tools.notBlank(getInAuthorityValue(partTmpl))) { + // At present, we're not attempting to automatically generate + // refNames for authority item records. Their 'parent' authority + // records don't necessarily exist at the time the item records are + // being imported, and thus we might not have a good way of identifying + // parents and obtaining their data for building part of this refName. + // + // Until and unless this is changed, imported authority item records + // should include refName values. + // + // FIXME: We could conceivably do a CSID-based lookup for an + // authority item's parent here, using the inAuthority value, + // and might then obtain the values necessary for building the + // refName, if that parent does exist. + return refName; + // Otherwise, by inference, if a record contains a value for + // only a shortIdentifier, it is an authority record + } else { + refNameDisplayName = getDisplayNameValue(partTmpl); + RefName.RefNameInterface authorityRefName = + RefName.Authority.buildAuthority(getTenantName(tenantId), + getServiceName(tenantId, serviceType), null, shortIdentifier, refNameDisplayName); + return authorityRefName.toString(); + } + } + + // If this service supports hierarchy, get the refName display name from + // the appropriate field in the record, so it can be added to the refName + if (serviceSupportsHierarchy(tenantId, serviceType)) { + String displayNameFieldXpath = getRefNameDisplayNameXpath(tenantId, serviceType); + refNameDisplayName = extractValueFromXmlFragment(displayNameFieldXpath, partTmpl); + } + // Generate an 'ID' form of refName + RefName.RefNameInterface refname = + RefName.Authority.buildAuthority(getTenantName(tenantId), + getServiceName(tenantId, serviceType), docID, null, refNameDisplayName); + refName = refname.toString(); + + return refName; + } + + private static String getTenantName(String tenantId) { + TenantBindingType tenantBinding = tReader.getTenantBinding(tenantId); + return tenantBinding.getName(); + } + + private static String getServiceName(String tenantId, String serviceType) { + ServiceBindingType serviceBinding = tReader.getServiceBindingForDocType(tenantId, serviceType); + return serviceBinding.getName(); + } + + private static boolean serviceSupportsHierarchy(String tenantId, String serviceType) { + ServiceBindingType serviceBinding = tReader.getServiceBindingForDocType(tenantId, serviceType); + DocHandlerParams params = serviceBinding.getDocHandlerParams(); + if (params == null || params.getParams() == null || params.getParams().isSupportsHierarchy() == null) { + return false; + } else { + return params.getParams().isSupportsHierarchy(); + } + } + + private static String getRefNameDisplayNameXpath(String tenantId, String serviceType) { + String displayNameXPath = ""; + ServiceBindingType serviceBinding = tReader.getServiceBindingForDocType(tenantId, serviceType); + DocHandlerParams params = serviceBinding.getDocHandlerParams(); + if (params == null || params.getParams() == null) { + return displayNameXPath; + } + ListResultField displayNameField = params.getParams().getRefnameDisplayNameField(); + if (displayNameField == null) { + return displayNameXPath; + + } + // Based on the two sample Xpath values for the RefnameDisplayNameField + // value, it appears these values require prefacing with the 'double slash' + // abbreviation, signifying the axis that selects all matching descendants + // of the document root. If that assumption proves incorrect in the future, + // we will need to revise this line. - ADR 2013-03-07 + if (Tools.isBlank(displayNameField.getXpath())) { + return displayNameXPath; + } else { + displayNameXPath = "//" + displayNameField.getXpath(); + } + return displayNameXPath; + } + /** * This inner class is the callback target for calls to XmlSaxFragmenter, * for example: FragmentHandlerImpl callback = new FragmentHandlerImpl(); diff --git a/services/imports/service/src/main/resources/templates/service-document.xml b/services/imports/service/src/main/resources/templates/service-document.xml index 1941651a8..5c6419681 100644 --- a/services/imports/service/src/main/resources/templates/service-document.xml +++ b/services/imports/service/src/main/resources/templates/service-document.xml @@ -42,6 +42,7 @@ ${updatedBy} ${tenantID} ${uri} + ${refName} ${Schema} diff --git a/services/imports/service/src/test/java/org/collectionspace/services/test/ImportsServiceTest.java b/services/imports/service/src/test/java/org/collectionspace/services/test/ImportsServiceTest.java index ec9832010..de421fd6c 100644 --- a/services/imports/service/src/test/java/org/collectionspace/services/test/ImportsServiceTest.java +++ b/services/imports/service/src/test/java/org/collectionspace/services/test/ImportsServiceTest.java @@ -59,8 +59,20 @@ public class ImportsServiceTest { String xmlPayload = FileTools.readFile(REQUESTS_DIR,"authority-request.xml"); InputSource inputSource = ImportsResource.payloadToInputSource(xmlPayload); - ImportsResource.expandXmlPayloadToDir(BOGUS_TENANT_ID, inputSource, TEMPLATE_DIR, outputDir); + + // Following changes made to TemplateExpander in CSPACE-5845 to read from + // Services tenant bindings via an instance of a tenant binding reader + // obtained from ServiceMain, the following call will yield dependency- + // related error. Some significant re-working, to pass in a tenant reader + // obtained by directly reading a tenant bindings file, in the manner of + // org.collectionspace.services.authorization.importer.AuthorizationGen.initialize(), + // may be needed to re-enable this call. + + // ImportsResource.expandXmlPayloadToDir(BOGUS_TENANT_ID, inputSource, TEMPLATE_DIR, outputDir); //TODO: inspect dir, then *cleanup*!! + + // This test class is incomplete; it does not (yet) have any asserts, + // as per an examination by Richard and Aron on 2013-03-08. } } -- 2.47.3