From 814a20bc20978b62a8b6d951ba6fd7d2193e5d5c Mon Sep 17 00:00:00 2001 From: Patrick Schmitz Date: Tue, 22 Nov 2011 20:00:22 +0000 Subject: [PATCH] CSPACE-2818, CSPACE-3537 Refactored logic that handles authorityRefs, refObjs, and displayName update, so that it can handle authRefs in deeply nested structures. Still awaiting latest code from Nuxeo to integrate their updated search, but this change at least fixes 2818, and makes us ready for the fix for 3537 and 2323. Also cleaned up handling of authorityRefs for garbage values - now produces a log error and does not produce an authorityRef item (was producing an empty one). Also added a partial fix and notes to remove the hard-coded resource paths for authority types. --- .../xmlreplay/authrefs/authrefsComplex.xml | 24 ++ .../xmlreplay/authrefs/authrefsSimple.xml | 98 +++++ .../test-data/xmlreplay/authrefs/intake1.xml | 53 +++ .../authrefs/organizations_common.xml | 32 ++ .../authrefs/orgauthorities_common.xml | 11 + .../test-data/xmlreplay/xml-replay-master.xml | 2 + .../common/vocabulary/AuthorityResource.java | 7 +- .../tenants/tenant-bindings-proto.xml | 238 +++++------ .../services/common/ResourceBase.java | 9 +- .../common/config/PropertyItemUtils.java | 1 + .../common/document/DocumentUtils.java | 3 + .../vocabulary/RefNameServiceUtils.java | 401 +++++++++++++----- .../common/vocabulary/RefNameUtils.java | 9 +- .../client/java/DocumentModelHandler.java | 4 +- .../java/RemoteDocumentModelHandlerImpl.java | 140 ++---- .../client/test/OrgAuthorityAuthRefsTest.java | 2 +- 16 files changed, 687 insertions(+), 347 deletions(-) create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsComplex.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsSimple.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/intake1.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/organizations_common.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/orgauthorities_common.xml rename services/{authority/service => common}/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java (55%) diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsComplex.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsComplex.xml new file mode 100644 index 000000000..6100d1feb --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsComplex.xml @@ -0,0 +1,24 @@ + + + + + + POST + /cspace-services/intakes/ + authrefs/intake1.xml + + + + GET + /cspace-services/intakes/${intake1.CSID}/authorityrefs + + + authrefs/res/intakesAuthRefs.res.xml + + + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsSimple.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsSimple.xml new file mode 100644 index 000000000..721c67ca3 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/authrefsSimple.xml @@ -0,0 +1,98 @@ + + + + + + POST + /cspace-services/personauthorities/ + authrefs/newPersonAuthority.xml + + + POST + /cspace-services/personauthorities/${PersonAuth1.CSID}/items/ + authrefs/newPerson1.xml + + + POST + /cspace-services/personauthorities/${PersonAuth1.CSID}/items/ + authrefs/newPerson2.xml + + + + GET + /cspace-services/personauthorities/${PersonAuth1.CSID}/items/${Person1.CSID} + + + + GET + /cspace-services/personauthorities/${PersonAuth1.CSID}/items/${Person2.CSID} + + + + POST + /cspace-services/loansout/ + authrefs/loanout.xml + + 42 + ${GetPerson1.got("//refName")} + + + + + POST + /cspace-services/loansout/ + authrefs/loanout.xml + + 102 + ${GetPerson2.got("//refName")} + + + + + GET + /cspace-services/loansout/${loanout1.CSID}/authorityrefs + + + authrefs/res/foo.res.xml + + + + + GET + /cspace-services/personauthorities/${PersonAuth1.CSID}/items/${Person1.CSID}/refObjs + + + authrefs/res/foo.res.xml + + + + + POST + /cspace-services/orgauthorities/ + authrefs/orgauthorities_common.xml + + + + POST + /cspace-services/orgauthorities/${OrganizationAuth1.CSID}/items/ + authrefs/organizations_common.xml + + ${GetPerson1.got("//refName")} + ${GetPerson2.got("//refName")} + + + + + GET + /cspace-services/orgauthorities/${OrganizationAuth1.CSID}/items/${Org1.CSID}/authorityrefs + + + authrefs/res/foo.res.xml + + + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/intake1.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/intake1.xml new file mode 100644 index 000000000..1b6142430 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/intake1.xml @@ -0,0 +1,53 @@ + + + + CompleteIntake001 + urn:cspace:core.collectionspace.org:locationauthorities:name(location):item:name(MOMA1321979112557)'MOMA' + BH-90210 + Must wear suit and tie + commission + 1234-5678-90 + 2010-07-14T04:00:00Z + urn:cspace:org.collectionspace.demo:orgauthority:name(organization):organization:name(twentiethcentury-foxfilmcorporation)'Twentieth+Century-Fox+Film+Corporation' + We got the governator himself to travel back in time, and bring some + robot spiders, so the condition can be proper checked + TERM-II + + found-on-doorstep + + Dont break it - we cannot afford it + 911-101-404-411 + + + + + + urn:cspace:core.collectionspace.org:vocabularies:name(conditionfitness):item:name(suitable)'Suitable' + Where is this? + urn:cspace:core.collectionspace.org:locationauthorities:name(location):item:name(TheVault1321979077879)'The Vault' + + + urn:cspace:core.collectionspace.org:vocabularies:name(conditionfitness):item:name(reasonable)'Reasonable' + Not in plain sight + urn:cspace:core.collectionspace.org:locationauthorities:name(location):item:name(PatricksCube1321979033052)'Patricks Cube' + + + + + + + urn:cspace:org.collectionspace.demo:personauthority:name(person):person:name(arnoldschwarzenegger)'Arnold Schwarzenegger' + + Packed with bubble-wrap and duct-tape - no one will ever be able to open + it + + urn:cspace:org.collectionspace.demo:personauthority:name(person):person:name(tommyjones)'Tommy+Jones' + + 2010-07-16T04:00:00Z + 2010-07-05T04:00:00Z + urn:cspace:org.collectionspace.demo:personauthority:name(person):person:name(seanbean)'Sean+Bean' + 2009-07-15T04:00:00Z + urn:cspace:org.collectionspace.demo:personauthority:name(person):person:name(tommyjones)'Tommy+Jones' + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/organizations_common.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/organizations_common.xml new file mode 100644 index 000000000..0172a1131 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/organizations_common.xml @@ -0,0 +1,32 @@ + + + + 1288047801161 + true + true + Test Organization-1288047801161 + Test Organization Name + + ${person1} + ${person2} + + Anytown, USA + + urn:cspace:org.collectionspace.demo:orgauthority:name(1288047801161):organization:name(1288047803708) + + + My new function + My second function + + + My new group + My second group + My third group + + This is a test Authority item + Some mythical book + Let's say page 39 + + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/orgauthorities_common.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/orgauthorities_common.xml new file mode 100644 index 000000000..0fef4cf3e --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/authrefs/orgauthorities_common.xml @@ -0,0 +1,11 @@ + + + + 1288047801161 + TestOrgAuth-1288047801161 + OrgAuthority + This is a test authority + Some mythical book + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml index 686cb68e3..dd18060ba 100755 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml @@ -54,6 +54,8 @@ + + diff --git a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/AuthorityResource.java b/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/AuthorityResource.java index f72d750c5..61ecbed59 100644 --- a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/AuthorityResource.java +++ b/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/AuthorityResource.java @@ -51,6 +51,7 @@ import org.collectionspace.services.common.document.DocumentNotFoundException; import org.collectionspace.services.common.document.DocumentWrapper; import org.collectionspace.services.common.query.QueryManager; import org.collectionspace.services.common.repository.RepositoryClient; +import org.collectionspace.services.common.vocabulary.RefNameServiceUtils; import org.collectionspace.services.common.vocabulary.nuxeo.AuthorityDocumentModelHandler; import org.collectionspace.services.common.vocabulary.nuxeo.AuthorityItemDocumentModelHandler; import org.collectionspace.services.common.workflow.service.nuxeo.WorkflowDocumentModelHandler; @@ -806,10 +807,8 @@ public abstract class AuthorityResource String itemcsid = lookupItemCSID(itemspecifier, parentcsid, "getAuthorityItemAuthRefs(item)", "GET_ITEM_AUTH_REFS", ctx); DocumentWrapper docWrapper = getRepositoryClient(ctx).getDoc(ctx, itemcsid); - List authRefFields = - ((MultipartServiceContextImpl) ctx).getCommonPartPropertyValues( - ServiceBindingUtils.AUTH_REF_PROP, ServiceBindingUtils.QUALIFIED_PROP_NAMES); - authRefList = handler.getAuthorityRefs(docWrapper, authRefFields); + List authRefsInfo = RefNameServiceUtils.getConfiguredAuthorityRefs(ctx); + authRefList = handler.getAuthorityRefs(docWrapper, authRefsInfo); } catch (Exception e) { throw bigReThrow(e, ServiceMessages.GET_FAILED + " parentspecifier: " + parentspecifier + " itemspecifier:" + itemspecifier); } diff --git a/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto.xml b/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto.xml index 756036359..788ab606b 100644 --- a/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto.xml +++ b/services/common/src/main/cspace/config/services/tenants/tenant-bindings-proto.xml @@ -191,125 +191,119 @@ authRef fieldColEventNames|fieldColEventName - - - - - - @@ -319,33 +313,32 @@ - termRef - responsibleDepartment|responsibleDepartments + responsibleDepartments|responsibleDepartment termRef @@ -395,63 +388,63 @@ @@ -459,7 +452,7 @@ @@ -467,15 +460,15 @@ @@ -484,35 +477,35 @@ @@ -522,23 +515,23 @@ @@ -701,13 +694,10 @@ authRef conditionCheckersOrAssessors|conditionCheckerOrAssessor - - termRef @@ -733,7 +723,7 @@ @@ -857,29 +847,22 @@ authRef borrowersContact - - - - @@ -954,7 +937,7 @@ @@ -1241,16 +1224,14 @@ authRef - subjectList|subject + subjectList|subject - - - termRef languageList|language @@ -1265,15 +1246,15 @@ @@ -1691,10 +1672,10 @@ - @@ -1713,7 +1694,7 @@ @@ -1895,14 +1876,14 @@ - - - @@ -2178,14 +2159,10 @@ - - - termRef @@ -2211,7 +2188,7 @@ @@ -2288,13 +2265,10 @@ authRef fieldCollectionEventNames|fieldCollectionEventName - - termRef @@ -2324,7 +2298,7 @@ @@ -2539,52 +2513,40 @@ - - - - - - - - diff --git a/services/common/src/main/java/org/collectionspace/services/common/ResourceBase.java b/services/common/src/main/java/org/collectionspace/services/common/ResourceBase.java index 3d348503a..d2c933810 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/ResourceBase.java +++ b/services/common/src/main/java/org/collectionspace/services/common/ResourceBase.java @@ -37,6 +37,8 @@ import org.collectionspace.services.common.document.DocumentNotFoundException; import org.collectionspace.services.common.document.DocumentWrapper; import org.collectionspace.services.common.query.QueryManager; import org.collectionspace.services.common.security.UnauthorizedException; +import org.collectionspace.services.common.vocabulary.RefNameServiceUtils; +import org.collectionspace.services.common.vocabulary.RefNameServiceUtils.AuthRefConfigInfo; import org.collectionspace.services.jaxb.AbstractCommonList; import org.collectionspace.services.nuxeo.client.java.DocumentModelHandler; import org.jboss.resteasy.plugins.providers.multipart.MultipartInput; @@ -48,6 +50,7 @@ import javax.ws.rs.*; import javax.ws.rs.core.*; import java.util.List; +import java.util.Map; /** * $LastChangedRevision: $ @@ -331,10 +334,8 @@ public abstract class ResourceBase ServiceContext ctx = createServiceContext(queryParams); DocumentWrapper docWrapper = getRepositoryClient(ctx).getDoc(ctx, csid); DocumentModelHandler handler = (DocumentModelHandler) createDocumentHandler(ctx); - List authRefFields = - ((MultipartServiceContextImpl) ctx).getCommonPartPropertyValues( - ServiceBindingUtils.AUTH_REF_PROP, ServiceBindingUtils.QUALIFIED_PROP_NAMES); - authRefList = handler.getAuthorityRefs(docWrapper, authRefFields); + List authRefsInfo = RefNameServiceUtils.getConfiguredAuthorityRefs(ctx); + authRefList = handler.getAuthorityRefs(docWrapper, authRefsInfo); } catch (Exception e) { throw bigReThrow(e, ServiceMessages.AUTH_REFS_FAILED, csid); } diff --git a/services/common/src/main/java/org/collectionspace/services/common/config/PropertyItemUtils.java b/services/common/src/main/java/org/collectionspace/services/common/config/PropertyItemUtils.java index 892b3103e..dd15f33c8 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/config/PropertyItemUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/config/PropertyItemUtils.java @@ -95,6 +95,7 @@ public class PropertyItemUtils { values = new ArrayList(); for(PropertyItemType propItem:propItems) { if(propName.equals(propItem.getKey())) { + // TODO - the trim() belongs here, not down a few lines. String value = propItem.getValue(); if(value!=null) { values.add((qualPrefix!=null)?(qualPrefix+value):value.trim()); diff --git a/services/common/src/main/java/org/collectionspace/services/common/document/DocumentUtils.java b/services/common/src/main/java/org/collectionspace/services/common/document/DocumentUtils.java index 00e0ce46e..a1c5cc54b 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/document/DocumentUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/document/DocumentUtils.java @@ -817,6 +817,7 @@ public class DocumentUtils { * @return true, if is list type */ public static boolean isListType(Property prop) { + // TODO simplify this to return (prop!=null && prop.getType().isListType()); boolean isList = false; if (prop == null) { return isList; @@ -834,6 +835,7 @@ public class DocumentUtils { * @return true, if is complex type */ public static boolean isComplexType(Property prop) { + // TODO simplify this to return (prop!=null && prop.getType().isComplexType()); boolean isComplex = false; if (prop == null) { return isComplex; @@ -853,6 +855,7 @@ public class DocumentUtils { * false, if it is not a date type. */ private static boolean isDateType(Type type) { + // TODO simplify this to return ((SimpleType)type).getPrimitiveType() instanceof DateType; SimpleType st = (SimpleType) type; if (st.getPrimitiveType() instanceof DateType) { return true; diff --git a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java b/services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java similarity index 55% rename from services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java rename to services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java index 5e22beb2a..781f43e06 100644 --- a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameServiceUtils.java @@ -35,12 +35,14 @@ import org.nuxeo.ecm.core.api.DocumentModel; import org.nuxeo.ecm.core.api.DocumentModelList; import org.nuxeo.ecm.core.api.model.Property; import org.nuxeo.ecm.core.api.model.PropertyException; +import org.nuxeo.ecm.core.api.model.PropertyNotFoundException; import org.nuxeo.ecm.core.api.model.impl.primitives.StringProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.collectionspace.services.common.ServiceMain; import org.collectionspace.services.common.context.ServiceContext; +import org.collectionspace.services.common.context.AbstractServiceContextImpl; import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.authorityref.AuthorityRefDocList; import org.collectionspace.services.common.authorityref.AuthorityRefList; @@ -57,6 +59,8 @@ import org.collectionspace.services.common.service.ServiceBindingType; import org.collectionspace.services.jaxb.AbstractCommonList; import org.collectionspace.services.nuxeo.util.NuxeoUtils; +import com.sun.xml.bind.v2.runtime.unmarshaller.XsiNilLoader.Array; + /** * RefNameServiceUtils is a collection of services utilities related to refName usage. * @@ -65,9 +69,137 @@ import org.collectionspace.services.nuxeo.util.NuxeoUtils; */ public class RefNameServiceUtils { + public static class AuthRefConfigInfo { + public String getQualifiedDisplayName() { + return(Tools.isBlank(schema))? + displayName:DocumentUtils.appendSchemaName(schema, displayName); + } + public String getDisplayName() { + return displayName; + } + public void setDisplayName(String displayName) { + this.displayName = displayName; + } + String displayName; + String schema; + public String getSchema() { + return schema; + } + public void setSchema(String schema) { + this.schema = schema; + } + public String getFullPath() { + return fullPath; + } + public void setFullPath(String fullPath) { + this.fullPath = fullPath; + } + String fullPath; + protected String[] pathEls; + public AuthRefConfigInfo(AuthRefConfigInfo arci) { + this.displayName = arci.displayName; + this.schema = arci.schema; + this.fullPath = arci.fullPath; + this.pathEls = arci.pathEls; + // Skip the pathElse check, since we are creatign from another (presumably valid) arci. + } + + public AuthRefConfigInfo(String displayName, String schema, String fullPath, String[] pathEls) { + this.displayName = displayName; + this.schema = schema; + this.fullPath = fullPath; + this.pathEls = pathEls; + checkPathEls(); + } + + // Split a config value string like "intakes_common:collector", or + // "collectionobjects_common:contentPeoples|contentPeople" + // "collectionobjects_common:assocEventGroupList/*/assocEventPlace" + // If has a pipe ('|') second part is a displayLabel, and first is path + // Otherwise, entry is a path, and can use the last pathElement as displayName + // Should be schema qualified. + public AuthRefConfigInfo(String configString) { + String[] pair = configString.split("\\|", 2); + String[] pathEls; + String displayName, fullPath; + if(pair.length == 1) { + // no label specifier, so we'll defer getting label + fullPath = pair[0]; + pathEls = pair[0].split("/"); + displayName = pathEls[pathEls.length-1]; + } else { + fullPath = pair[0]; + pathEls = pair[0].split("/"); + displayName = pair[1]; + } + String[] schemaSplit = pathEls[0].split(":",2); + String schema; + if(schemaSplit.length==1) { // schema not specified + schema = null; + } else { + schema = schemaSplit[0]; + if(pair.length == 1 && pathEls.length == 1) { // simplest case of field in top level schema, no labelll + displayName = schemaSplit[1]; // Have to fix up displayName to have no schema + } + } + this.displayName = displayName; + this.schema = schema; + this.fullPath = fullPath; + this.pathEls = pathEls; + checkPathEls(); + } + + protected void checkPathEls() { + int len = pathEls.length; + if(len<1) + throw new InternalError("Bad values in authRef info - caller screwed up:"+fullPath); + // Handle case of them putting a leading slash on the path + if(len>1 && pathEls[0].endsWith(":")) { + len--; + String[] newArray = new String[len]; + newArray[0] = pathEls[0]+pathEls[1]; + if(len>=2) { + System.arraycopy(pathEls, 2, newArray, 1, len-1); + } + pathEls = newArray; + } + } + } + + public static class AuthRefInfo extends AuthRefConfigInfo { + public Property getProperty() { + return property; + } + public void setProperty(Property property) { + this.property = property; + } + Property property; + public AuthRefInfo(String displayName, String schema, String fullPath, String[] pathEls, Property prop) { + super(displayName, schema, fullPath, pathEls); + this.property = prop; + } + public AuthRefInfo(AuthRefConfigInfo arci, Property prop) { + super(arci); + this.property = prop; + } + } + private static final Logger logger = LoggerFactory.getLogger(RefNameServiceUtils.class); private static ArrayList refNameServiceTypes = null; + + public static List getConfiguredAuthorityRefs(ServiceContext ctx) { + List authRefFields = + ((AbstractServiceContextImpl) ctx).getAllPartsPropertyValues( + ServiceBindingUtils.AUTH_REF_PROP, ServiceBindingUtils.QUALIFIED_PROP_NAMES); + ArrayList authRefsInfo = new ArrayList(authRefFields.size()); + for(String spec:authRefFields) { + AuthRefConfigInfo arci = new AuthRefConfigInfo(spec); + authRefsInfo.add(arci); + } + return authRefsInfo; + } + public static AuthorityRefDocList getAuthorityRefDocs(ServiceContext ctx, RepositoryClient repoClient, @@ -83,7 +215,7 @@ public class RefNameServiceUtils { wrapperList.getAuthorityRefDocItem(); Map queriedServiceBindings = new HashMap(); - Map> authRefFieldsByService = new HashMap>(); + Map> authRefFieldsByService = new HashMap>(); DocumentModelList docList = findAuthorityRefDocs(ctx, repoClient, serviceTypes, refName, refPropName, queriedServiceBindings, authRefFieldsByService, pageSize, pageNum, computeTotal); @@ -114,24 +246,28 @@ public class RefNameServiceUtils { return refNameServiceTypes; } + // Seems like a good value - no real data to set this well. + private static final int N_OBJS_TO_UPDATE_PER_LOOP = 100; + public static int updateAuthorityRefDocs(ServiceContext ctx, RepositoryClient repoClient, String oldRefName, String newRefName, String refPropName ) { Map queriedServiceBindings = new HashMap(); - Map> authRefFieldsByService = new HashMap>(); + Map> authRefFieldsByService = new HashMap>(); int nRefsFound = 0; if(!(repoClient instanceof RepositoryJavaClientImpl)) { throw new InternalError("updateAuthorityRefDocs() called with unknown repoClient type!"); } try { - final int pageSize = 100; // Seems like a good value - no real data to set this well. + final int pageSize = N_OBJS_TO_UPDATE_PER_LOOP; int pageNumProcessed = 1; while(true) { // Keep looping until we find all the refs. logger.debug("updateAuthorityRefDocs working on page: "+pageNumProcessed); // Note that we always ask the Repo for the first page, since each page we process - // should not be found in successive searches. + // should not be found in successive searches. Slightly inefficient, but more + // reliable (stateless). DocumentModelList docList = findAuthorityRefDocs(ctx, repoClient, getRefNameServiceTypes(), oldRefName, refPropName, queriedServiceBindings, authRefFieldsByService, pageSize, 0, false); @@ -163,7 +299,7 @@ public class RefNameServiceUtils { String refName, String refPropName, Map queriedServiceBindings, - Map> authRefFieldsByService, + Map> authRefFieldsByService, int pageSize, int pageNum, boolean computeTotal) throws DocumentException, DocumentNotFoundException { // Get the service bindings for this tenant @@ -194,16 +330,18 @@ public class RefNameServiceUtils { return docList; } + private static final boolean READY_FOR_COMPLEX_QUERY = false; + private static String computeWhereClauseForAuthorityRefDocs( String escapedRefName, String refPropName, ArrayList docTypes, List servicebindings, Map queriedServiceBindings, - Map> authRefFieldsByService ) { + Map> authRefFieldsByService ) { StringBuilder whereClause = new StringBuilder(); boolean fFirst = true; - List authRefFieldPaths = new ArrayList(); + List authRefFieldPaths; for (ServiceBindingType sb : servicebindings) { // Gets the property names for each part, qualified with the part label (which // is also the table name, the way that the repository works). @@ -213,33 +351,29 @@ public class RefNameServiceUtils { if (authRefFieldPaths.isEmpty()) { continue; } - String authRefPath = ""; - String ancestorAuthRefFieldName = ""; - Map authRefFields = new HashMap(); - for (int i = 0; i < authRefFieldPaths.size(); i++) { - // fieldName = DocumentUtils.getDescendantOrAncestor(authRefFields.get(i)); - // For simple field values, we just search on the item. - // For simple repeating scalars, we just search the group field - // For repeating complex types, we will need to do more. - authRefPath = authRefFieldPaths.get(i); - ancestorAuthRefFieldName = DocumentUtils.getAncestorAuthRefFieldName(authRefFieldPaths.get(i)); - authRefFields.put(authRefPath, ancestorAuthRefFieldName); - } + ArrayList authRefsInfo = new ArrayList(); + for(String spec:authRefFieldPaths) { + AuthRefConfigInfo arci = new AuthRefConfigInfo(spec); + authRefsInfo.add(arci); + } String docType = sb.getObject().getName(); queriedServiceBindings.put(docType, sb); - authRefFieldsByService.put(docType, authRefFields); + authRefFieldsByService.put(docType, authRefsInfo); docTypes.add(docType); - Collection fields = authRefFields.values(); - for (String field : fields) { + for (AuthRefConfigInfo arci : authRefsInfo) { // Build up the where clause for each authRef field + if(!READY_FOR_COMPLEX_QUERY) { // filter complex field references + if(arci.pathEls.length>1) + continue; // skip this one + } if (fFirst) { fFirst = false; } else { whereClause.append(" OR "); } //whereClause.append(prefix); - whereClause.append(field); + whereClause.append(arci.getFullPath()); whereClause.append("='"); whereClause.append(escapedRefName); whereClause.append("'"); @@ -265,17 +399,9 @@ public class RefNameServiceUtils { DocumentModelList docList, String refName, Map queriedServiceBindings, - Map> authRefFieldsByService, + Map> authRefFieldsByService, List list, String newAuthorityRefName) { - if(newAuthorityRefName==null) { - if(list==null) { - throw new InternalError("processRefObjsDocList() called with neither an itemList nor a new RefName!"); - } - } else if(list!=null) { - throw new InternalError("processRefObjsDocList() called with both an itemList and a new RefName!"); - } - Iterator iter = docList.iterator(); int nRefsFoundTotal = 0; while (iter.hasNext()) { @@ -290,9 +416,15 @@ public class RefNameServiceUtils { } String serviceContextPath = "/" + sb.getName().toLowerCase() + "/"; - if(list == null) { + if(list == null) { // no list - should be update refName case. + if(newAuthorityRefName==null) { + throw new InternalError("processRefObjsDocList() called with neither an itemList nor a new RefName!"); + } ilistItem = null; - } else { + } else { // Have a list - refObjs case + if(newAuthorityRefName!=null) { + throw new InternalError("processRefObjsDocList() called with both an itemList and a new RefName!"); + } ilistItem = new AuthorityRefDocList.AuthorityRefDocItem(); String csid = NuxeoUtils.getCsid(docModel);//NuxeoUtils.extractId(docModel.getPathAsString()); ilistItem.setDocId(csid); @@ -311,71 +443,37 @@ public class RefNameServiceUtils { } // Now, we have to loop over the authRefFieldsByService to figure // out which field(s) matched this. - Map matchingAuthRefFields = authRefFieldsByService.get(docType); + List matchingAuthRefFields = authRefFieldsByService.get(docType); if (matchingAuthRefFields == null || matchingAuthRefFields.isEmpty()) { throw new RuntimeException( "getAuthorityRefDocs: internal logic error: can't fetch authRefFields for DocType."); } - String authRefAncestorField = ""; - String authRefDescendantField = ""; - String sourceField = ""; + //String authRefAncestorField = ""; + //String authRefDescendantField = ""; + //String sourceField = ""; int nRefsFoundInDoc = 0; - // Use this if we go to qualified field names - for (String path : matchingAuthRefFields.keySet()) { - try { - // This is the field name we show in the return info - // Returned as a schema-qualified property path - authRefAncestorField = (String) matchingAuthRefFields.get(path); - // This is the qualified field we have to get from the doc model - authRefDescendantField = DocumentUtils.getDescendantOrAncestor(path); - // The ancestor field is part-schema (tablename) qualified - //String[] strings = authRefAncestorField.split(":"); - //if (strings.length != 2) { - // throw new RuntimeException( - // "getAuthorityRefDocs: Bad configuration of path to authority reference field."); - //} - // strings[0] holds a schema name, such as "intakes_common" - // - // strings[1] holds: - // * The name of an authority reference field, such as "depositor"; - // or - // * The name of an ancestor (e.g. parent, grandparent ...) field, - // such as "fieldCollectors", of a repeatable authority reference - // field, such as "fieldCollector". - // TODO - if the value is not simple, or repeating scalar, need a more - // sophisticated fetch. - // Change this to an XPath model - //Object fieldValue = docModel.getProperty(strings[0], strings[1]); - // This will have to handle repeating complex fields by iterating over the possibilities - // and finding the one that matches. - Property fieldValue = docModel.getProperty(authRefAncestorField); - // We know this doc should have a match somewhere, but it may not be in this field - // If we are just building up the refItems, then it is enough to know we found a match. - // If we are patching refName values, then we have to replace each match. - int nRefsMatchedInField = refNameFoundInField(refName, fieldValue, newAuthorityRefName); - if (nRefsMatchedInField > 0) { - sourceField = authRefDescendantField; - // Handle multiple fields matching in one Doc. See CSPACE-2863. - if(nRefsFoundInDoc > 0) { - // We already added ilistItem, so we need to clone that and add again - if(ilistItem != null) { - ilistItem = cloneAuthRefDocItem(ilistItem, sourceField); - } - } else { - if(ilistItem != null) { - ilistItem.setSourceField(sourceField); - } - } - if(ilistItem != null) { - list.add(ilistItem); - } - nRefsFoundInDoc += nRefsMatchedInField; - } - - } catch (ClientException ce) { - throw new RuntimeException( - "getAuthorityRefDocs: Problem fetching: " + sourceField, ce); - } + + ArrayList foundProps + = new ArrayList(); + try { + findAuthRefPropertiesInDoc(docModel, matchingAuthRefFields, refName, foundProps); + for(RefNameServiceUtils.AuthRefInfo ari:foundProps) { + if(ilistItem != null) { + if(nRefsFoundInDoc == 0) { // First one? + ilistItem.setSourceField(ari.getQualifiedDisplayName()); + } else { // duplicates from one object + ilistItem = cloneAuthRefDocItem(ilistItem, ari.getQualifiedDisplayName()); + } + list.add(ilistItem); + } else { // update refName case + Property propToUpdate = ari.getProperty(); + propToUpdate.setValue(newAuthorityRefName); + } + nRefsFoundInDoc++; + } + } catch (ClientException ce) { + throw new RuntimeException( + "getAuthorityRefDocs: Problem fetching values from repo: " + ce.getLocalizedMessage()); } if (nRefsFoundInDoc == 0) { throw new RuntimeException( @@ -398,6 +496,119 @@ public class RefNameServiceUtils { newlistItem.setSourceField(sourceField); return newlistItem; } + + public static List findAuthRefPropertiesInDoc( + DocumentModel docModel, + List authRefFieldInfo, + String refNameToMatch, + List foundProps + ) { + // Assume that authRefFieldInfo is keyed by the field name (possibly mapped for UI) + // and the values are elPaths to the field, where intervening group structures in + // lists of complex structures are replaced with "*". Thus, valid paths include + // the following (note that the ServiceBindingUtils prepend schema names to configured values): + // "schemaname:fieldname" + // "schemaname:scalarlistname" + // "schemaname:complexfieldname/fieldname" + // "schemaname:complexlistname/*/fieldname" + // "schemaname:complexlistname/*/scalarlistname" + // "schemaname:complexlistname/*/complexfieldname/fieldname" + // "schemaname:complexlistname/*/complexlistname/*/fieldname" + // etc. + for (AuthRefConfigInfo arci : authRefFieldInfo) { + try { + // Get first property and work down as needed. + Property prop = docModel.getProperty(arci.pathEls[0]); + findAuthRefPropertiesInProperty(foundProps, prop, arci, 0, refNameToMatch); + } catch(Exception e) { + logger.error("Problem fetching property: "+arci.pathEls[0]); + } + } + return foundProps; + } + + public static List findAuthRefPropertiesInProperty( + List foundProps, + Property prop, + AuthRefConfigInfo arci, + int pathStartIndex, // Supports recursion and we work down the path + String refNameToMatch + ) { + if(pathStartIndex >= arci.pathEls.length) { + throw new ArrayIndexOutOfBoundsException("Index = "+pathStartIndex+" for path: " + +arci.pathEls.toString()); + } + AuthRefInfo ari = null; + if(prop == null) + return foundProps; + if(prop instanceof StringProperty) { // scalar string + addARIifMatches(refNameToMatch, arci, prop, foundProps); + } else if(prop instanceof List) { + List propList = (List)prop; + // run through list. Must either be list of Strings, or Complex + for (Property listItemProp : propList) { + if(listItemProp instanceof StringProperty) { + if(arci.pathEls.length-pathStartIndex != 1) { + logger.error("Configuration for authRefs does not match schema structure: " + +arci.pathEls.toString()); + break; + } else { + addARIifMatches(refNameToMatch, arci, listItemProp, foundProps); + } + } else if(listItemProp.isComplex()) { + // Just recurse to handle this. Note that since this is a list of complex, + // which should look like listName/*/... we add 2 to the path start index + findAuthRefPropertiesInProperty(foundProps, listItemProp, arci, + pathStartIndex+2, refNameToMatch); + } else { + logger.error("Configuration for authRefs does not match schema structure: " + +arci.pathEls.toString()); + break; + } + } + } else if(prop.isComplex()) { + String localPropName = arci.pathEls[pathStartIndex]; + try { + Property localProp = prop.get(localPropName); + // Now just recurse, pushing down the path 1 step + findAuthRefPropertiesInProperty(foundProps, localProp, arci, + pathStartIndex, refNameToMatch); + } catch(PropertyNotFoundException pnfe) { + logger.error("Could not find property: ["+localPropName+"] in path: "+ + arci.getFullPath()); + // Fall through - ari will be null and we will continue... + } + } else { + logger.error("Configuration for authRefs does not match schema structure: " + +arci.pathEls.toString()); + } + + if(ari != null) { + foundProps.add(ari); + } + return foundProps; + } + + private static void addARIifMatches( + String refNameToMatch, + AuthRefConfigInfo arci, + Property prop, + List foundProps) { + // Need to either match a passed refName + // OR have no refName to match but be non-empty + try { + String value = (String)prop.getValue(); + if(((refNameToMatch!=null) && refNameToMatch.equals(value)) + || ((refNameToMatch==null) && Tools.notBlank(value))) { + // Found a match + logger.debug("Found a match on property: "+prop.getPath()+" with value: ["+value+"]"); + AuthRefInfo ari = new AuthRefInfo(arci, prop); + foundProps.add(ari); + } + } catch(PropertyException pe) { + logger.debug("PropertyException on: "+prop.getPath()+pe.getLocalizedMessage()); + } + } /* * Identifies whether the refName was found in the supplied field. @@ -410,7 +621,6 @@ public class RefNameServiceUtils { * Does not work for: * * Structured fields (complexTypes) * * Repeatable structured fields (repeatable complexTypes) - */ private static int refNameFoundInField(String oldRefName, Property fieldValue, String newRefName) { int nFound = 0; if (fieldValue instanceof List) { @@ -443,5 +653,6 @@ public class RefNameServiceUtils { } return nFound; } + */ } diff --git a/services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameUtils.java b/services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameUtils.java index ae255a054..981acd848 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/vocabulary/RefNameUtils.java @@ -113,7 +113,14 @@ public class RefNameUtils { } else if(resource.equals("orgauthority")) { uri.append("/orgauthorities/"); } else { - throw new RuntimeException("Illegal Authority Type: " + resource); + if(!(resource.equals("orgauthorities") + || resource.equals("personauthorities") + || resource.equals("locationauthorities") + || resource.equals("placeauthorities") + || resource.equals("vocabularies"))) { + logger.error("Unrecognized Authority Type: " + resource); + } + uri.append("/"+resource+"/"); } if(csid!=null) { uri.append(csid); diff --git a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/DocumentModelHandler.java b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/DocumentModelHandler.java index 91c7a2f0c..02e3e9a06 100644 --- a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/DocumentModelHandler.java +++ b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/DocumentModelHandler.java @@ -24,6 +24,7 @@ package org.collectionspace.services.nuxeo.client.java; import java.util.List; +import java.util.Map; import org.collectionspace.services.client.PoxPayloadIn; import org.collectionspace.services.client.PoxPayloadOut; @@ -39,6 +40,7 @@ import org.collectionspace.services.nuxeo.util.NuxeoUtils; import org.collectionspace.services.common.profile.Profiler; import org.collectionspace.services.common.repository.RepositoryClient; import org.collectionspace.services.common.repository.RepositoryClientFactory; +import org.collectionspace.services.common.vocabulary.RefNameServiceUtils.AuthRefConfigInfo; import org.nuxeo.ecm.core.api.ClientException; import org.nuxeo.ecm.core.api.DocumentModel; @@ -184,7 +186,7 @@ public abstract class DocumentModelHandler */ abstract public AuthorityRefList getAuthorityRefs( DocumentWrapper docWrapper, - List authRefFields) throws PropertyException; + List authRefsInfo) throws PropertyException; private void handleCoreValues(DocumentWrapper docWrapper, Action action) throws ClientException { diff --git a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RemoteDocumentModelHandlerImpl.java b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RemoteDocumentModelHandlerImpl.java index 5c365d056..60d96f3d1 100644 --- a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RemoteDocumentModelHandlerImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RemoteDocumentModelHandlerImpl.java @@ -23,6 +23,7 @@ */ package org.collectionspace.services.nuxeo.client.java; +import java.util.ArrayList; import java.util.Collection; import java.util.GregorianCalendar; import java.util.HashMap; @@ -42,6 +43,7 @@ import org.collectionspace.services.client.PayloadOutputPart; import org.collectionspace.services.client.PoxPayloadIn; import org.collectionspace.services.client.PoxPayloadOut; import org.collectionspace.services.client.workflow.WorkflowClient; +import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.authorityref.AuthorityRefList; import org.collectionspace.services.common.context.JaxRsContext; import org.collectionspace.services.common.context.MultipartServiceContext; @@ -56,6 +58,8 @@ import org.collectionspace.services.common.security.SecurityUtils; import org.collectionspace.services.common.service.ObjectPartType; import org.collectionspace.services.common.storage.jpa.JpaStorageUtils; import org.collectionspace.services.common.vocabulary.RefNameUtils; +import org.collectionspace.services.common.vocabulary.RefNameServiceUtils; +import org.collectionspace.services.common.vocabulary.RefNameServiceUtils.AuthRefConfigInfo; import org.dom4j.Element; import org.nuxeo.ecm.core.api.DocumentModel; @@ -381,7 +385,7 @@ public abstract class RemoteDocumentModelHandlerImpl @Override public AuthorityRefList getAuthorityRefs( DocumentWrapper docWrapper, - List authRefFieldNames) throws PropertyException { + List authRefsInfo) throws PropertyException { AuthorityRefList authRefList = new AuthorityRefList(); AbstractCommonList commonList = (AbstractCommonList) authRefList; @@ -400,101 +404,22 @@ public abstract class RemoteDocumentModelHandlerImpl int iFirstToUse = (int)(pageSize*pageNum); int nFoundInPage = 0; int nFoundTotal = 0; - for (String authRefFieldName : authRefFieldNames) { - - // FIXME: Can use the schema to validate field existence, - // to help avoid encountering PropertyExceptions. - String schemaName = DocumentUtils.getSchemaNamePart(authRefFieldName); - Schema schema = DocumentUtils.getSchemaFromName(schemaName); - - String descendantAuthRefFieldName = DocumentUtils.getDescendantAuthRefFieldName(authRefFieldName); - if (descendantAuthRefFieldName != null && !descendantAuthRefFieldName.trim().isEmpty()) { - authRefFieldName = DocumentUtils.getAncestorAuthRefFieldName(authRefFieldName); - } - - String xpath = "//" + authRefFieldName; - Property prop = docModel.getProperty(xpath); - if (prop == null) { - continue; - } - - // If this is a single scalar field, with no children, - // add an item with its values to the authRefs list. - if (DocumentUtils.isSimpleType(prop)) { - String refName = prop.getValue(String.class); - if (refName == null) { - continue; - } - refName = refName.trim(); - if (refName.isEmpty()) { - continue; - } - if((nFoundTotal < iFirstToUse) - || (nFoundInPage >= pageSize)) { - nFoundTotal++; - continue; - } - nFoundTotal++; - nFoundInPage++; - appendToAuthRefsList(refName, schemaName, authRefFieldName, list); - - // Otherwise, if this field has children, cycle through each child. - // - // Whenever we find instances of the descendant field among - // these children, add an item with its values to the authRefs list. - // - // FIXME: When we increase maximum repeatability depth, that is, the depth - // between ancestor and descendant, we'll need to use recursion here, - // rather than making fixed assumptions about hierarchical depth. - } else if ((DocumentUtils.isListType(prop) || DocumentUtils.isComplexType(prop)) - && prop.size() > 0) { - - Collection childProp = prop.getChildren(); - for (Property cProp : childProp) { - if (DocumentUtils.isSimpleType(cProp) && cProp.getName().equals(descendantAuthRefFieldName)) { - String refName = cProp.getValue(String.class); - if (refName == null) { - continue; - } - refName = refName.trim(); - if (refName.isEmpty()) { - continue; - } - if((nFoundTotal < iFirstToUse) - || (nFoundInPage >= pageSize)) { - nFoundTotal++; - continue; - } - nFoundTotal++; - nFoundInPage++; - appendToAuthRefsList(refName, schemaName, descendantAuthRefFieldName, list); - } else if ((DocumentUtils.isListType(cProp) || DocumentUtils.isComplexType(cProp)) - && prop.size() > 0) { - Collection grandChildProp = cProp.getChildren(); - for (Property gProp : grandChildProp) { - if (DocumentUtils.isSimpleType(gProp) && gProp.getName().equals(descendantAuthRefFieldName)) { - String refName = gProp.getValue(String.class); - if (refName == null) { - continue; - } - refName = refName.trim(); - if (refName.isEmpty()) { - continue; - } - if((nFoundTotal < iFirstToUse) - || (nFoundInPage >= pageSize)) { - nFoundTotal++; - continue; - } - nFoundTotal++; - nFoundInPage++; - appendToAuthRefsList(refName, schemaName, descendantAuthRefFieldName, list); - } - } - } - } - } - } + + ArrayList foundProps + = new ArrayList(); + RefNameServiceUtils.findAuthRefPropertiesInDoc(docModel, authRefsInfo, null, foundProps); + // Slightly goofy pagination support - how many refs do we expect from one object? + for(RefNameServiceUtils.AuthRefInfo ari:foundProps) { + if((nFoundTotal >= iFirstToUse) && (nFoundInPage < pageSize)) { + if(appendToAuthRefsList(ari, list)) { + nFoundInPage++; + nFoundTotal++; + } + } else { + nFoundTotal++; + } + } + // Set num of items in list. this is useful to our testing framework. commonList.setItemsInPage(nFoundInPage); // set the total result size @@ -519,13 +444,21 @@ public abstract class RemoteDocumentModelHandlerImpl return authRefList; } - private void appendToAuthRefsList(String refName, String schemaName, - String fieldName, List list) + private boolean appendToAuthRefsList(RefNameServiceUtils.AuthRefInfo ari, + List list) throws Exception { - if (DocumentUtils.getSchemaNamePart(fieldName).isEmpty()) { - fieldName = DocumentUtils.appendSchemaName(schemaName, fieldName); - } - list.add(authorityRefListItem(fieldName, refName)); + String fieldName = ari.getQualifiedDisplayName(); + try { + String refNameValue = (String)ari.getProperty().getValue(); + AuthorityRefList.AuthorityRefItem item = authorityRefListItem(fieldName, refNameValue); + if(item!=null) { // ignore garbage values. + list.add(item); + return true; + } + } catch(PropertyException pe) { + logger.debug("PropertyException on: "+ari.getProperty().getPath()+pe.getLocalizedMessage()); + } + return false; } private AuthorityRefList.AuthorityRefItem authorityRefListItem(String authRefFieldName, String refName) { @@ -539,7 +472,8 @@ public abstract class RemoteDocumentModelHandlerImpl ilistItem.setSourceField(authRefFieldName); ilistItem.setUri(termInfo.getRelativeUri()); } catch (Exception e) { - // Do nothing upon encountering an Exception here. + logger.error("Trouble parsing refName from value: "+refName+" in field: "+authRefFieldName+e.getLocalizedMessage()); + ilistItem = null; } return ilistItem; } 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 ef155886b..776498d24 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 @@ -109,7 +109,7 @@ public class OrgAuthorityAuthRefsTest extends BaseServiceTest { private String subBodyRefName = null; /** The number of authorityreferences expected. */ - private final int NUM_AUTH_REFS_EXPECTED = 3; + private final int NUM_AUTH_REFS_EXPECTED = 2; // Place authRef not legal, should not be returned. protected void setKnownResource( String id, String refName ) { knownResourceId = id; -- 2.47.3