From 488972acf3db3a9d8e3ed7c172dd15d216926d11 Mon Sep 17 00:00:00 2001 From: remillet Date: Mon, 31 Aug 2015 09:00:04 -0700 Subject: [PATCH] CSPACE-6802: To prevent creating more than one vocab/authority with the same shortid, added code to sychronize the creation of vocabs/authorities. --- .../6.0/config/proto-repo-config.xml | 1 + .../nuxeo-server/6.0/config/vcsconfig.sql.txt | 14 +++ .../src/main/resources/log4j.properties | 1 + .../client/test/AcquisitionAuthRefsTest.java | 2 +- .../common/vocabulary/AuthorityResource.java | 85 +++++++++++++++---- .../nuxeo/AuthorityDocumentModelHandler.java | 48 ++++++++++- .../client/test/AbstractServiceTestImpl.java | 3 +- .../services/client/test/BaseServiceTest.java | 45 +++++++++- .../common/storage/StorageClient.java | 11 ++- .../storage/jpa/JpaStorageClientImpl.java | 11 ++- .../nuxeo/client/java/CoreSessionWrapper.java | 25 ++++++ .../client/java/NuxeoClientEmbedded.java | 80 ++++++++++++++--- .../client/java/RepositoryJavaClientImpl.java | 30 ++++++- .../test/ConditioncheckAuthRefsTest.java | 2 +- .../client/test/MovementAuthRefsTest.java | 2 +- .../client/test/OrgAuthorityAuthRefsTest.java | 2 +- .../client/test/RelationServiceTest.java | 2 +- .../client/test/VocabularyServiceTest.java | 55 +++++++++++- 18 files changed, 372 insertions(+), 47 deletions(-) create mode 100644 3rdparty/nuxeo/nuxeo-server/6.0/config/vcsconfig.sql.txt diff --git a/3rdparty/nuxeo/nuxeo-server/6.0/config/proto-repo-config.xml b/3rdparty/nuxeo/nuxeo-server/6.0/config/proto-repo-config.xml index a6f7e3a46..6b7ec696e 100644 --- a/3rdparty/nuxeo/nuxeo-server/6.0/config/proto-repo-config.xml +++ b/3rdparty/nuxeo/nuxeo-server/6.0/config/proto-repo-config.xml @@ -26,6 +26,7 @@ @XA_DATASOURCE@ false + vcsconfig.sql.txt varchar diff --git a/3rdparty/nuxeo/nuxeo-server/6.0/config/vcsconfig.sql.txt b/3rdparty/nuxeo/nuxeo-server/6.0/config/vcsconfig.sql.txt new file mode 100644 index 000000000..c28de5738 --- /dev/null +++ b/3rdparty/nuxeo/nuxeo-server/6.0/config/vcsconfig.sql.txt @@ -0,0 +1,14 @@ +# +# A place to modify the Nuxeo database with SQL statements. +# See https://doc.nuxeo.com/display/ADMINDOC/VCS+Configuration#VCSConfiguration-DatabaseCreationOption +# + + +#CATEGORY: afterTableCreation + + +# +# Add a unique constraint to the shortidentifier column of the vocabularies_common table. +# +LOG.INFO Adding a unique constraint to the shortidentifier column of the vocabularies_common table +ALTER TABLE vocabularies_common add CONSTRAINT shortid_unique UNIQUE (shortidentifier); \ No newline at end of file diff --git a/services/JaxRsServiceProvider/src/main/resources/log4j.properties b/services/JaxRsServiceProvider/src/main/resources/log4j.properties index e0a0a5378..350c73c56 100644 --- a/services/JaxRsServiceProvider/src/main/resources/log4j.properties +++ b/services/JaxRsServiceProvider/src/main/resources/log4j.properties @@ -53,6 +53,7 @@ log4j.additivity.perf.collectionspace=false # CollectionSpace loggers and default levels - all loggers using the rootLogger if not otherwise specified # log4j.logger.org.collectionspace=WARN +log4j.logger.org.collectionspace.services.nuxeo.client.java.NuxeoClientEmbedded=TRACE #log4j.logger.org.collectionspace.services.nuxeo.client.java=ERROR #log4j.logger.org.collectionspace.services.common.storage.JDBCTools=ERROR #log4j.logger.org.collectionspace.services.common.profile.CSpaceFilter=ERROR diff --git a/services/acquisition/client/src/test/java/org/collectionspace/services/client/test/AcquisitionAuthRefsTest.java b/services/acquisition/client/src/test/java/org/collectionspace/services/client/test/AcquisitionAuthRefsTest.java index 8e70ee2e6..9f54a4309 100644 --- a/services/acquisition/client/src/test/java/org/collectionspace/services/client/test/AcquisitionAuthRefsTest.java +++ b/services/acquisition/client/src/test/java/org/collectionspace/services/client/test/AcquisitionAuthRefsTest.java @@ -70,7 +70,7 @@ public class AcquisitionAuthRefsTest extends BaseServiceTest // Instance variables specific to this test. // final String SERVICE_PATH_COMPONENT = AcquisitionClient.SERVICE_PATH_COMPONENT;//"acquisitions"; - final String PERSON_AUTHORITY_NAME = "TestPersonAuth"; + final String PERSON_AUTHORITY_NAME = "TestPersonAuthForAquisitionTest"; private String knownResourceId = null; private List acquisitionIdsCreated = new ArrayList(); private List personIdsCreated = new ArrayList(); 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 c32d17efa..867c53c11 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 @@ -347,20 +347,29 @@ public abstract class AuthorityResource } - @POST //FIXME: REM - 5/1/2012 - We can probably remove this method. - public Response createAuthority(String xmlPayload) { //REM - This method is never reached by the JAX-RS client -instead the "create" method in ResourceBase.java is getting called. - try { - PoxPayloadIn input = new PoxPayloadIn(xmlPayload); - ServiceContext ctx = createServiceContext(input); - DocumentHandler handler = createDocumentHandler(ctx); - String csid = getRepositoryClient(ctx).create(ctx, handler); - UriBuilder path = UriBuilder.fromResource(resourceClass); - path.path("" + csid); - Response response = Response.created(path.build()).build(); - return response; - } catch (Exception e) { - throw bigReThrow(e, ServiceMessages.CREATE_FAILED); - } + @POST + public Response createAuthority(String xmlPayload) { + // + // Requests to create new authorities come in on new threads. Unfortunately, we need to synchronize those threads on this block because, as of 8/27/2015, we can't seem to get Nuxeo + // transaction code to deal with a database level UNIQUE constraint violations on the 'shortidentifier' column of the vocabularies_common table. + // Therefore, to prevent having multiple authorities with the same shortid, we need to synchronize + // the code that creates new authorities. The authority document model handler will first check for authorities with the same short id before + // trying to create a new authority. + // + synchronized(AuthorityResource.class) { + try { + PoxPayloadIn input = new PoxPayloadIn(xmlPayload); + ServiceContext ctx = createServiceContext(input); + DocumentHandler handler = createDocumentHandler(ctx); + String csid = getRepositoryClient(ctx).create(ctx, handler); + UriBuilder path = UriBuilder.fromResource(resourceClass); + path.path("" + csid); + Response response = Response.created(path.build()).build(); + return response; + } catch (Exception e) { + throw bigReThrow(e, ServiceMessages.CREATE_FAILED); + } + } } protected String buildWhereForAuthByName(String name) { @@ -504,9 +513,10 @@ public abstract class AuthorityResource * * @return the response */ - @DELETE + @Deprecated +// @DELETE @Path("{csid}") - public Response deleteAuthority(@PathParam("csid") String csid) { + public Response old_deleteAuthority(@PathParam("csid") String csid) { if (logger.isDebugEnabled()) { logger.debug("deleteAuthority with csid=" + csid); } @@ -520,6 +530,49 @@ public abstract class AuthorityResource throw bigReThrow(e, ServiceMessages.DELETE_FAILED, csid); } } + + /** + * Delete authority + * + * @param csid the csid or a URN specifier form -e.g., urn:cspace:name(OurMuseumPersonAuthority) + * + * @return the response + */ + @DELETE + @Path("{csid}") + public Response deleteAuthority( + @Context Request request, + @Context UriInfo ui, + @PathParam("csid") String specifier) { + if (logger.isDebugEnabled()) { + logger.debug("deleteAuthority with specifier=" + specifier); + } + + try { + ServiceContext ctx = createServiceContext(ui); + DocumentHandler handler = createDocumentHandler(ctx); + + Specifier spec = getSpecifier(specifier, "getAuthority", "GET"); + if (spec.form == SpecifierForm.CSID) { + if (logger.isDebugEnabled()) { + logger.debug("deleteAuthority with csid=" + spec.value); + } + ensureCSID(spec.value, ServiceMessages.DELETE_FAILED, "Authority.csid"); + getRepositoryClient(ctx).delete(ctx, spec.value, handler); + } else { + if (logger.isDebugEnabled()) { + logger.debug("deleteAuthority with specifier=" + spec.value); + } + String whereClause = buildWhereForAuthByName(spec.value); + getRepositoryClient(ctx).deleteWithWhereClause(ctx, whereClause, handler); + } + + return Response.status(HttpResponseCodes.SC_OK).build(); + } catch (Exception e) { + throw bigReThrow(e, ServiceMessages.DELETE_FAILED, specifier); + } + } + /************************************************************************* * Create an AuthorityItem - this is a sub-resource of Authority diff --git a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityDocumentModelHandler.java b/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityDocumentModelHandler.java index 6a5656a23..137bd11b3 100644 --- a/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityDocumentModelHandler.java +++ b/services/authority/service/src/main/java/org/collectionspace/services/common/vocabulary/nuxeo/AuthorityDocumentModelHandler.java @@ -31,6 +31,8 @@ import org.collectionspace.services.common.api.RefName; import org.collectionspace.services.common.api.RefName.Authority; import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.context.ServiceContext; +import org.collectionspace.services.common.document.DocumentException; +import org.collectionspace.services.common.document.DocumentNotFoundException; import org.collectionspace.services.common.document.DocumentWrapper; import org.collectionspace.services.common.vocabulary.AuthorityJAXBSchema; import org.collectionspace.services.config.service.ObjectPartType; @@ -86,18 +88,58 @@ public abstract class AuthorityDocumentModelHandler handleDisplayNameAsShortIdentifier(wrapDoc.getWrappedObject(), authorityCommonSchemaName); updateRefnameForAuthority(wrapDoc, authorityCommonSchemaName);//CSPACE-3178 } + + protected String buildWhereForShortId(String name) { + return authorityCommonSchemaName + + ":" + AuthorityJAXBSchema.SHORT_IDENTIFIER + + "='" + name + "'"; + } + + private boolean isUnique(DocumentModel docModel, String schemaName) throws DocumentException { + boolean result = true; + + ServiceContext ctx = this.getServiceContext(); + String shortIdentifier = (String) docModel.getProperty(schemaName, AuthorityJAXBSchema.SHORT_IDENTIFIER); + String nxqlWhereClause = buildWhereForShortId(shortIdentifier); + try { + DocumentWrapper searchResultWrapper = getRepositoryClient(ctx).findDoc(ctx, nxqlWhereClause); + if (searchResultWrapper != null) { + result = false; + if (logger.isInfoEnabled() == true) { + DocumentModel searchResult = searchResultWrapper.getWrappedObject(); + String debugMsg = String.format("Could not create a new authority with a short identifier of '%s', because one already exists with the same short identifer: CSID = '%s'", + shortIdentifier, searchResult.getName()); + logger.trace(debugMsg); + } + } + } catch (DocumentNotFoundException e) { + // Not a problem, just means we couldn't find another authority with that short ID + } + + return result; + } /** * If no short identifier was provided in the input payload, - * generate a short identifier from the display name. + * generate a short identifier from the display name. Either way though, + * the short identifier needs to be unique. */ private void handleDisplayNameAsShortIdentifier(DocumentModel docModel, String schemaName) throws Exception { String shortIdentifier = (String) docModel.getProperty(schemaName, AuthorityJAXBSchema.SHORT_IDENTIFIER); String displayName = (String) docModel.getProperty(schemaName, AuthorityJAXBSchema.DISPLAY_NAME); String shortDisplayName = ""; + String generateShortIdentifier = null; if (Tools.isEmpty(shortIdentifier)) { - String generatedShortIdentifier = AuthorityIdentifierUtils.generateShortIdentifierFromDisplayName(displayName, shortDisplayName); - docModel.setProperty(schemaName, AuthorityJAXBSchema.SHORT_IDENTIFIER, generatedShortIdentifier); + generateShortIdentifier = AuthorityIdentifierUtils.generateShortIdentifierFromDisplayName(displayName, shortDisplayName); + docModel.setProperty(schemaName, AuthorityJAXBSchema.SHORT_IDENTIFIER, shortIdentifier); + } + + if (isUnique(docModel, schemaName) == false) { + String shortId = generateShortIdentifier == null ? shortIdentifier : generateShortIdentifier; + String errMsgVerb = generateShortIdentifier == null ? "supplied" : "generated"; + String errMsg = String.format("The %s short identifier '%s' was not unique, so the new authority could not be created.", + errMsgVerb, shortId); + throw new DocumentException(errMsg); } } diff --git a/services/client/src/main/java/org/collectionspace/services/client/test/AbstractServiceTestImpl.java b/services/client/src/main/java/org/collectionspace/services/client/test/AbstractServiceTestImpl.java index f1f693164..8f1d058ab 100644 --- a/services/client/src/main/java/org/collectionspace/services/client/test/AbstractServiceTestImpl.java +++ b/services/client/src/main/java/org/collectionspace/services/client/test/AbstractServiceTestImpl.java @@ -1117,7 +1117,8 @@ public abstract class AbstractServiceTestImpl { Response.Status.OK.getStatusCode(); protected static final int STATUS_FORBIDDEN = Response.Status.FORBIDDEN.getStatusCode(); + + // + // "Global flag to cancel cleanup() method + // + private static boolean cancelCleanup = false; + + // + // Decide if cleanup should happen + // + protected boolean cleanupCancelled() { + if (cancelCleanup == false) { + String noTestCleanup = System.getProperty(NO_TEST_CLEANUP); + if (Boolean.TRUE.toString().equalsIgnoreCase(noTestCleanup)) { + cancelCleanup = true; + } + } + + return cancelCleanup; + } + + protected void cancelCleanup() { + cancelCleanup = true; + } /** * Instantiates a new base service test. @@ -253,6 +274,22 @@ public abstract class BaseServiceTest { testExpectedStatusCode = expectedStatusCode; testRequestType = reqType; } + + protected long randomPause(Random randomGenerator, long maxPauseMillis) { + long result = 0; + + if (maxPauseMillis != 0) { + try { + Thread.sleep(result = randomGenerator.nextInt(500)); + } catch (InterruptedException e) { + // TODO Auto-generated catch block + e.printStackTrace(); + result = -1; + } + } + + return result; + } /** * Returns an error message indicating that the status code returned by a @@ -700,13 +737,13 @@ public abstract class BaseServiceTest { */ @AfterClass(alwaysRun = true) public void cleanUp() { - String noTestCleanup = System.getProperty(NO_TEST_CLEANUP); - if (Boolean.TRUE.toString().equalsIgnoreCase(noTestCleanup)) { + if (cleanupCancelled() == true) { if (logger.isDebugEnabled()) { logger.debug("Skipping Cleanup phase ..."); } return; } + if (logger.isDebugEnabled()) { logger.debug("Cleaning up temporary resources created for testing ..."); } diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/StorageClient.java b/services/common/src/main/java/org/collectionspace/services/common/storage/StorageClient.java index 8c72ed8fa..9df4fbffc 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/StorageClient.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/StorageClient.java @@ -50,7 +50,16 @@ public interface StorageClient { * @throws DocumentException */ void delete(ServiceContext ctx, String id) throws DocumentNotFoundException, DocumentException; - + + /** + * Delete with an NXQL 'WHERE' clause. If multiple documents match the clause, this method delete just + * the first document it finds. + * @param ctx + * @param specifier + * @throws DocumentNotFoundException + * @throws DocumentException + */ + void deleteWithWhereClause(ServiceContext ctx, String whereClause, DocumentHandler handler) throws DocumentNotFoundException, DocumentException; /** * delete a entity from the persistence store diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java index 457f7ab11..9e52abd0a 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java @@ -19,7 +19,9 @@ package org.collectionspace.services.common.storage.jpa; import java.util.Date; import java.util.List; + import javax.persistence.RollbackException; + import java.sql.BatchUpdateException; import javax.persistence.EntityManager; @@ -35,13 +37,11 @@ import org.collectionspace.services.common.document.DocumentHandler.Action; import org.collectionspace.services.common.document.DocumentWrapper; import org.collectionspace.services.common.document.DocumentWrapperImpl; import org.collectionspace.services.common.document.JaxbUtils; - import org.collectionspace.services.common.storage.StorageClient; import org.collectionspace.services.common.context.ServiceContextProperties; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.common.query.QueryContext; import org.collectionspace.services.lifecycle.TransitionDef; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -586,4 +586,11 @@ public class JpaStorageClientImpl implements StorageClient { DocumentException { // Do nothing. JPA services do not support workflow. } + + @Override + public void deleteWithWhereClause(ServiceContext ctx, String whereClause, + DocumentHandler handler) throws DocumentNotFoundException, + DocumentException { + throw new UnsupportedOperationException(); + } } diff --git a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/CoreSessionWrapper.java b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/CoreSessionWrapper.java index 95ccdf1ce..cbfaeef69 100644 --- a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/CoreSessionWrapper.java +++ b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/CoreSessionWrapper.java @@ -2,6 +2,7 @@ package org.collectionspace.services.nuxeo.client.java; import java.security.Principal; +import org.collectionspace.services.nuxeo.util.NuxeoUtils; import org.nuxeo.ecm.core.api.ClientException; import org.nuxeo.ecm.core.api.DocumentModel; import org.nuxeo.ecm.core.api.DocumentModelList; @@ -12,11 +13,31 @@ import org.nuxeo.ecm.core.api.NoRollbackOnException; import org.nuxeo.ecm.core.api.event.DocumentEventTypes; import org.nuxeo.ecm.core.api.impl.LifeCycleFilter; import org.nuxeo.ecm.core.api.CoreSession; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class CoreSessionWrapper implements CoreSessionInterface { private CoreSession repoSession; + /** The logger. */ + private static Logger logger = LoggerFactory.getLogger(CoreSessionWrapper.class); + + private void logQuery(String query) { + logger.debug(String.format("NXQL: %s", query)); + } + + private void logQuery(String query, String queryType) { + logger.debug(String.format("Query Type: '%s' NXQL: %s", queryType, query)); + } + + private void logQuery(String query, Filter filter, long limit, + long offset, boolean countTotal) { + logger.debug(String.format("Filter: '%s', Limit: '%d', Offset: '%d', Count Total?: %b, NXQL: %s", + filter != null ? filter.toString() : "none", limit, offset, countTotal, query)); + } + + public CoreSessionWrapper(CoreSession repoSession) { this.repoSession = repoSession; } @@ -71,22 +92,26 @@ public class CoreSessionWrapper implements CoreSessionInterface { @Override public IterableQueryResult queryAndFetch(String query, String queryType, Object... params) throws ClientException { + logQuery(query, queryType); return repoSession.queryAndFetch(query, queryType, params); } @Override public DocumentModelList query(String query, Filter filter, long limit, long offset, boolean countTotal) throws ClientException { + logQuery(query, filter, limit, offset, countTotal); return repoSession.query(query, filter, limit, offset, countTotal); } @Override public DocumentModelList query(String query, int max) throws ClientException { + logQuery(query); return repoSession.query(query, max); } @Override public DocumentModelList query(String query) throws ClientException { + logQuery(query); return repoSession.query(query); } diff --git a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/NuxeoClientEmbedded.java b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/NuxeoClientEmbedded.java index b6db6f339..b3904c0aa 100644 --- a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/NuxeoClientEmbedded.java +++ b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/NuxeoClientEmbedded.java @@ -58,6 +58,8 @@ public final class NuxeoClientEmbedded { private RepositoryManager repositoryMgr; private static final NuxeoClientEmbedded instance = new NuxeoClientEmbedded(); + + private static final int MAX_CREATE_TRANSACTION_ATTEMPTS = 5; /** * Constructs a new NuxeoClient. NOTE: Using {@link #getInstance()} instead @@ -128,7 +130,54 @@ public final class NuxeoClientEmbedded { */ public CoreSessionInterface openRepository(String repoName) throws Exception { return openRepository(repoName, ServiceContext.DEFAULT_TX_TIMEOUT); - } + } + + private boolean startTransaction() { + boolean startedTransaction = false; + int attempts = 0; + + if (TransactionHelper.isTransactionActive() == false) { + while (startedTransaction == false && attempts <= MAX_CREATE_TRANSACTION_ATTEMPTS) { + try { + startedTransaction = TransactionHelper.startTransaction(); + } catch (Exception e) { + String traceMsg = String.format("Could not start a new transaction on thread '%d'", Thread.currentThread().getId()); + logger.trace(traceMsg); + boolean txState = TransactionHelper.isTransactionActive(); + txState = TransactionHelper.isNoTransaction(); + txState = TransactionHelper.isTransactionActiveOrMarkedRollback(); + txState = TransactionHelper.isTransactionMarkedRollback(); + } + + if (startedTransaction == false) { + long currentThreadId = Thread.currentThread().getId(); + boolean txState = TransactionHelper.isTransactionActive(); + txState = TransactionHelper.isNoTransaction(); + txState = TransactionHelper.isTransactionActiveOrMarkedRollback(); + txState = TransactionHelper.isTransactionMarkedRollback(); + + if (TransactionHelper.isTransactionActiveOrMarkedRollback() == true) { + try { + TransactionHelper.commitOrRollbackTransaction(); + } catch (Exception e) { + logger.error("Could not commit or rollback transaction.", e); + } + } + } + attempts++; + } + } else { + logger.warn("A request to start a new transaction was made, but a transaction is already open."); + startedTransaction = true; + } + + if (startedTransaction == false) { + String errMsg = String.format("Attempted %d time(s) to start a new transaction, but failed.", attempts); + logger.error(errMsg); + } + + return startedTransaction; + } public CoreSessionInterface openRepository(String repoName, int timeoutSeconds) throws Exception { CoreSessionInterface result = null; @@ -157,9 +206,10 @@ public final class NuxeoClientEmbedded { // boolean startedTransaction = false; if (TransactionHelper.isTransactionActive() == false) { - startedTransaction = TransactionHelper.startTransaction(); + startedTransaction = startTransaction(); if (startedTransaction == false) { - String errMsg = "Could not start a Nuxeo transaction with the TransactionHelper class."; + String errMsg = String.format("Could not start a Nuxeo transaction with the TransactionHelper class on thread '%d'.", + Thread.currentThread().getId()); logger.error(errMsg); throw new Exception(errMsg); } @@ -190,12 +240,17 @@ public final class NuxeoClientEmbedded { logger.trace(String.format("Added a new repository instance to our repo list. Current count is now: %d", repositoryInstances.size())); } else { + // + // If we couldn't open a repo session, we need to close the transaction we started. + // + if (startedTransaction == true) { + TransactionHelper.commitOrRollbackTransaction(); + } String errMsg = String.format("Could not open a session to the Nuxeo repository='%s'", repoName); logger.error(errMsg); throw new Exception(errMsg); } - return result; } @@ -259,15 +314,18 @@ public final class NuxeoClientEmbedded { String key = repoSession.getSessionId(); String name = repoSession.getRepositoryName(); + // + // The caller should have already called the .save() method, but just in + // case they didn't, let's try calling it again. + // try { repoSession.save(); - repoSession.close(); } catch (Exception e) { - String errMsg = String.format("Possible data loss. Could not save and/or close the Nuxeo repository name = '%s'.", - name); - logger.error(errMsg, e); + String errMsg = String.format("Possible data loss. Could not save and/or close the Nuxeo repository name = '%s'.", name); + logger.trace(errMsg, e); throw e; } finally { + repoSession.close(); CoreSessionInterface wasRemoved = repositoryInstances.remove(key); if (logger.isTraceEnabled()) { if (wasRemoved != null) { @@ -283,9 +341,11 @@ public final class NuxeoClientEmbedded { // if (TransactionHelper.isTransactionActiveOrMarkedRollback() == true) { TransactionHelper.commitOrRollbackTransaction(); - UserTransaction ut = TransactionHelper.lookupUserTransaction(); - TransactionManager tm = TransactionHelper.lookupTransactionManager(); logger.trace(String.format("Transaction closed on thread '%d'", Thread.currentThread().getId())); + } else { + String warnMsg = String.format("Closed a Nuxeo repository session on thread '%d' without closing the containing transaction.", + Thread.currentThread().getId()); + logger.warn(warnMsg); } } } diff --git a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RepositoryJavaClientImpl.java b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RepositoryJavaClientImpl.java index ad8a81569..1330b01ad 100644 --- a/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RepositoryJavaClientImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/nuxeo/client/java/RepositoryJavaClientImpl.java @@ -461,8 +461,16 @@ public class RepositoryJavaClientImpl implements RepositoryClient conditioncheckIdsCreated = new ArrayList(); private List personIdsCreated = new ArrayList(); diff --git a/services/movement/client/src/test/java/org/collectionspace/services/client/test/MovementAuthRefsTest.java b/services/movement/client/src/test/java/org/collectionspace/services/client/test/MovementAuthRefsTest.java index bea5e5446..3d80cd468 100644 --- a/services/movement/client/src/test/java/org/collectionspace/services/client/test/MovementAuthRefsTest.java +++ b/services/movement/client/src/test/java/org/collectionspace/services/client/test/MovementAuthRefsTest.java @@ -69,7 +69,7 @@ public class MovementAuthRefsTest extends BaseServiceTest { final String SERVICE_PATH_COMPONENT = "movements"; // Instance variables specific to this test. - final String PERSON_AUTHORITY_NAME = "TestPersonAuth"; + final String PERSON_AUTHORITY_NAME = "TestPersonAuthForMovementTest"; private List movementIdsCreated = new ArrayList(); private List personIdsCreated = new ArrayList(); private String personAuthCSID = null; 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 58f7e5866..87fd68120 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 @@ -67,7 +67,7 @@ public class OrgAuthorityAuthRefsTest extends BaseServiceTest personIdsCreated = new ArrayList(); 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 bad57f61a..6e4c4a9f3 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 @@ -23,7 +23,6 @@ package org.collectionspace.services.client.test; import java.util.HashMap; -import javax.ws.rs.core.Response; import org.collectionspace.services.common.vocabulary.AuthorityItemJAXBSchema; import org.collectionspace.services.client.CollectionSpaceClient; @@ -34,14 +33,13 @@ import org.collectionspace.services.client.VocabularyClient; import org.collectionspace.services.client.VocabularyClientUtils; import org.collectionspace.services.vocabulary.VocabulariesCommon; import org.collectionspace.services.vocabulary.VocabularyitemsCommon; - -import org.jboss.resteasy.client.ClientResponse; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.Assert; import org.testng.annotations.Test; +import javax.ws.rs.core.Response; + /** * VocabularyServiceTest, carries out tests against a * deployed and running Vocabulary Service. @@ -101,6 +99,55 @@ public class VocabularyServiceTest extends AbstractAuthorityServiceTest