From 1dd657db742915a4b0d471e9cf399c9478af69de Mon Sep 17 00:00:00 2001 From: Aron Roberts Date: Wed, 2 May 2012 17:33:14 -0700 Subject: [PATCH] CSPACE-5119,CSPACE-5135,CSPACE-5138: All Person tests now pass. Validation that each authority item contains at least one term, whose term name or term display name is non-blank, added to validator handler. Cleanup of some commented-out code. --- .../common/vocabulary/AuthorityResource.java | 5 - .../AuthorityItemDocumentModelHandler.java | 5 +- .../test/PersonAuthorityServiceTest.java | 12 +- .../person/nuxeo/PersonValidatorHandler.java | 159 +++++++++++------- 4 files changed, 106 insertions(+), 75 deletions(-) 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 38031921f..8f60e0359 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 @@ -675,11 +675,6 @@ public abstract class AuthorityResource String qualifiedDisplayNameField = NuxeoUtils.getPrimaryElPathPropertyName(authorityItemCommonSchemaName, getItemTermInfoGroupXPathBase(), AuthorityItemJAXBSchema.TERM_DISPLAY_NAME); - // authorityItemCommonSchemaName + ":" + getItemTermInfoGroupXPathBase() - // + "/0/" + AuthorityItemJAXBSchema.TERM_DISPLAY_NAME; - - // NuxeoUtils.getPrimaryXPathPropertyName(authorityItemCommonSchemaName, - // getItemTermInfoGroupXPathBase(), AuthorityItemJAXBSchema.TERM_DISPLAY_NAME); // Note that docType defaults to the ServiceName, so we're fine with that. ServiceContext ctx = null; diff --git a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java b/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java index ad85f2a2e..c440b5a09 100644 --- a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java +++ b/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityItemDocumentModelHandler.java @@ -170,7 +170,6 @@ public abstract class AuthorityItemDocumentModelHandler field.setElement(AuthorityItemJAXBSchema.TERM_DISPLAY_NAME); field.setXpath(NuxeoUtils.getPrimaryXPathPropertyName( authorityItemCommonSchemaName, getItemTermInfoGroupXPathBase(), AuthorityItemJAXBSchema.TERM_DISPLAY_NAME)); - // field.setXpath(AuthorityItemJAXBSchema.DISPLAY_NAME); list.add(field); } if (!hasShortId) { @@ -190,7 +189,6 @@ public abstract class AuthorityItemDocumentModelHandler field.setElement(AuthorityItemJAXBSchema.TERM_STATUS); field.setXpath(NuxeoUtils.getPrimaryXPathPropertyName( authorityItemCommonSchemaName, getItemTermInfoGroupXPathBase(), AuthorityItemJAXBSchema.TERM_STATUS)); - // field.setXpath(AuthorityItemJAXBSchema.TERM_STATUS); list.add(field); } return list; @@ -217,7 +215,8 @@ public abstract class AuthorityItemDocumentModelHandler logger.warn("Creating Authority Item with no displayName!"); } * - */ + */ + // CSPACE-3178: // handleDisplayNameAsShortIdentifier(wrapDoc.getWrappedObject(), authorityItemCommonSchemaName); // refName includes displayName, so we force a correct value here. diff --git a/services/person/client/src/test/java/org/collectionspace/services/client/test/PersonAuthorityServiceTest.java b/services/person/client/src/test/java/org/collectionspace/services/client/test/PersonAuthorityServiceTest.java index 392721e8e..a85a86160 100644 --- a/services/person/client/src/test/java/org/collectionspace/services/client/test/PersonAuthorityServiceTest.java +++ b/services/person/client/src/test/java/org/collectionspace/services/client/test/PersonAuthorityServiceTest.java @@ -72,11 +72,6 @@ public class PersonAuthorityServiceTest extends AbstractAuthorityServiceTest terms = termList.getPersonTermGroup(); Assert.assertNotNull(terms); terms.get(0).setTermDisplayName(null); + terms.get(0).setTermName(null); // Submit the updated resource to the service and store the response. PoxPayloadOut output = new PoxPayloadOut(PersonAuthorityClient.SERVICE_ITEM_PAYLOAD_NAME); @@ -825,11 +821,11 @@ public class PersonAuthorityServiceTest extends AbstractAuthorityServiceTest terms = new ArrayList(); PersonTermGroup term = new PersonTermGroup(); + term.setTermDisplayName("John Wayne"); + term.setTermName("John Wayne"); term.setForeName("John"); term.setSurName("Wayne"); terms.add(term); diff --git a/services/person/service/src/main/java/org/collectionspace/services/person/nuxeo/PersonValidatorHandler.java b/services/person/service/src/main/java/org/collectionspace/services/person/nuxeo/PersonValidatorHandler.java index fe36ecd5f..0bfb7897b 100644 --- a/services/person/service/src/main/java/org/collectionspace/services/person/nuxeo/PersonValidatorHandler.java +++ b/services/person/service/src/main/java/org/collectionspace/services/person/nuxeo/PersonValidatorHandler.java @@ -1,45 +1,48 @@ /** - * 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 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. */ package org.collectionspace.services.person.nuxeo; +import java.util.List; import java.util.regex.Pattern; +import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.person.PersonsCommon; import org.collectionspace.services.common.context.MultipartServiceContext; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.common.document.DocumentHandler.Action; import org.collectionspace.services.common.document.InvalidDocumentException; import org.collectionspace.services.common.document.ValidatorHandler; +import org.collectionspace.services.person.PersonTermGroup; +import org.collectionspace.services.person.PersonTermGroupList; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** * PersonValidatorHandler - * - * Validates data supplied when attempting to create and/or update Person records. - * - * $LastChangedRevision: $ - * $LastChangedDate: $ + * + * Validates data supplied when attempting to create and/or update Person + * records. + * + * $LastChangedRevision: $ $LastChangedDate: $ */ public class PersonValidatorHandler implements ValidatorHandler { @@ -52,49 +55,61 @@ public class PersonValidatorHandler implements ValidatorHandler { if (logger.isDebugEnabled()) { logger.debug("validate() action=" + action.name()); } - + // Bail out if the validation action is for delete. if (action.equals(Action.DELETE)) { - return; + return; } - + try { MultipartServiceContext mctx = (MultipartServiceContext) ctx; PersonsCommon person = (PersonsCommon) mctx.getInputPart(mctx.getCommonPartLabel(), PersonsCommon.class); String msg = ""; boolean invalid = false; - - if(person != null) { // No guarantee that there is a common part in every post/update. - - // Validation occurring on both creates and updates - - // FIXME Add validation logic here to ensure that every term info group must contain - // any required data elements. (Potential example: every term info group must contain - // a non-null, non-whitespace-only value in either of the term or displayName fields.) - - /* - String displayName = person.getDisplayName(); - if (!person.isDisplayNameComputed() && ((displayName == null) || displayName.trim().isEmpty())) { - invalid = true; - msg += "displayName must be non-null and non-blank if displayNameComputed is false!"; - } - * - */ - - // Validation specific to creates or updates - if (action.equals(Action.CREATE)) { - String shortId = person.getShortIdentifier(); - // Per CSPACE-2215, shortIdentifier values that are null (missing) - // oe the empty string are now legally accepted in create payloads. - // In either of those cases, a short identifier will be synthesized from - // a display name or supplied in another manner. - if ((shortId != null) && (shortIdBadPattern.matcher(shortId).find())) { - invalid = true; - msg += "shortIdentifier must only contain standard word characters"; - } - } else if (action.equals(Action.UPDATE)) { - } + + if (person != null) { // No guarantee that there is a common part in every post/update. + + // Validation occurring on both creates and updates + + /* + * String displayName = person.getDisplayName(); if + * (!person.isDisplayNameComputed() && ((displayName == null) || + * displayName.trim().isEmpty())) { invalid = true; msg += + * "displayName must be non-null and non-blank if + * displayNameComputed is false!"; } + * + */ + + if (!containsAtLeastOneTerm(person)) { + invalid = true; + msg += "Authority items must contain at least one term."; + } + + if (!allTermsContainNameOrDisplayName(person)) { + invalid = true; + msg += "Each term group in an authority item must contain " + + "a non-empty term name or " + + "a non-empty term display name."; + } + + // Validation specific to creates or updates + if (action.equals(Action.CREATE)) { + + // shortIdentifier value must contain only word characters + String shortId = person.getShortIdentifier(); + if ((shortId != null) && (shortIdBadPattern.matcher(shortId).find())) { + invalid = true; + msg += "shortIdentifier must only contain standard word characters"; + } + + // Note: Per CSPACE-2215, shortIdentifier values that are null (missing) + // or the empty string are now legally accepted in create payloads. + // In either of those cases, a short identifier will be synthesized from + // a display name or supplied in another manner. + + } else if (action.equals(Action.UPDATE)) { + } } if (invalid) { @@ -107,4 +122,28 @@ public class PersonValidatorHandler implements ValidatorHandler { throw new InvalidDocumentException(e); } } + + private boolean containsAtLeastOneTerm(PersonsCommon person) { + PersonTermGroupList termGroupList = person.getPersonTermGroupList(); + if (termGroupList == null) { + return false; + } + List termGroups = termGroupList.getPersonTermGroup(); + if ((termGroups == null) || (termGroups.size() == 0)) { + return false; + } + return true; + } + + private boolean allTermsContainNameOrDisplayName(PersonsCommon person) { + PersonTermGroupList termGroupList = person.getPersonTermGroupList(); + List termGroups = termGroupList.getPersonTermGroup(); + for (PersonTermGroup termGroup : termGroups) { + if (Tools.isBlank(termGroup.getTermName()) || Tools.isBlank(termGroup.getTermDisplayName()) ){ + return false; + } + } + return true; + } + } -- 2.47.3