From 0940e8852e654d7260c7b793fa55ee0601e5347a Mon Sep 17 00:00:00 2001 From: Richard Millet Date: Thu, 15 Dec 2011 22:56:39 +0000 Subject: [PATCH] CSPACE-4745: DocumentNotFoundException stack traces should only appear in the logs when logging level is set to TRACE --- .../client/test/AccountRoleServiceTest.java | 9 ++- .../AbstractCollectionSpaceResourceImpl.java | 41 ++++++++---- .../services/common/document/JaxbUtils.java | 8 +-- .../common/security/SecurityInterceptor.java | 19 ++++-- .../jpa/JpaRelationshipStorageClient.java | 3 + .../client/java/RepositoryJavaClientImpl.java | 66 ++++++++++--------- 6 files changed, 91 insertions(+), 55 deletions(-) diff --git a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java index a23530cb8..ba1401788 100644 --- a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java +++ b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java @@ -513,6 +513,8 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { AccountRole toDelete = null; try { toDelete = readResponse.getEntity(); + } catch (Throwable e) { + e.printStackTrace(); } finally { readResponse.releaseConnection(); } @@ -531,7 +533,9 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { // // recreate 'acc-role-user1' account and roles // - create(testName); +// create(testName); + + /* setupDelete(); // @@ -542,6 +546,8 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { toDelete = null; try { toDelete = readResponse.getEntity(); + } catch (Throwable e) { + e.printStackTrace(); } finally { readResponse.releaseConnection(); } @@ -555,6 +561,7 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { } finally { res.releaseConnection(); } + */ } diff --git a/services/common/src/main/java/org/collectionspace/services/common/AbstractCollectionSpaceResourceImpl.java b/services/common/src/main/java/org/collectionspace/services/common/AbstractCollectionSpaceResourceImpl.java index cbb61c928..7c50cf524 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/AbstractCollectionSpaceResourceImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/common/AbstractCollectionSpaceResourceImpl.java @@ -382,33 +382,35 @@ public abstract class AbstractCollectionSpaceResourceImpl } protected WebApplicationException bigReThrow(Exception e, String serviceMsg, String csid) throws WebApplicationException { + boolean logException = true; + WebApplicationException result = null; Response response; - if (logger.isDebugEnabled()) { - logger.debug(getClass().getName(), e); - } + String detail = Tools.errorToString(e, true); String detailNoTrace = Tools.errorToString(e, true, 3); - logger.error(getClass().getName()+" detail: "+detailNoTrace, e); - if (e instanceof UnauthorizedException) { response = Response.status(Response.Status.UNAUTHORIZED).entity(serviceMsg + e.getMessage()).type("text/plain").build(); - return new WebApplicationException(response); + result = new WebApplicationException(response); } else if (e instanceof DocumentNotFoundException) { + // + // Don't log this error unless we're in 'trace' mode + // + logException = false; response = Response.status(Response.Status.NOT_FOUND).entity(serviceMsg + " on " + getClass().getName() + " csid=" + csid).type("text/plain").build(); - return new WebApplicationException(response); + result = new WebApplicationException(response); } else if (e instanceof BadRequestException) { int code = ((BadRequestException) e).getErrorCode(); - if (code == 0){ + if (code == 0) { code = Response.Status.BAD_REQUEST.getStatusCode(); } // CSPACE-1110 response = Response.status(code).entity(serviceMsg + e.getMessage()).type("text/plain").build(); // return new WebApplicationException(e, code); - return new WebApplicationException(response); + result = new WebApplicationException(response); - } else if (e instanceof DocumentException){ + } else if (e instanceof DocumentException) { int code = ((DocumentException) e).getErrorCode(); if (code == 0){ code = Response.Status.BAD_REQUEST.getStatusCode(); @@ -416,16 +418,29 @@ public abstract class AbstractCollectionSpaceResourceImpl // CSPACE-1110 response = Response.status(code).entity(serviceMsg + e.getMessage()).type("text/plain").build(); // return new WebApplicationException(e, code); - return new WebApplicationException(response); + result = new WebApplicationException(response); } else if (e instanceof WebApplicationException) { // subresource may have already thrown this exception // so just pass it on - return (WebApplicationException) e; + result = (WebApplicationException) e; } else { // e is now instanceof Exception response = Response.status(Response.Status.INTERNAL_SERVER_ERROR).entity(serviceMsg + " detail: " + detailNoTrace).type("text/plain").build(); - return new WebApplicationException(response); + result = new WebApplicationException(response); + } + // + // Some exceptions like DocumentNotFoundException won't be logged unless we're in 'trace' mode + // + boolean traceEnabled = logger.isTraceEnabled(); + if (logException == true || traceEnabled == true) { + if (traceEnabled == true) { + logger.error(getClass().getName() + " detail: " + detail, e); + } else { + logger.error(getClass().getName() + " detail: " + detailNoTrace); + } } + + return result; } } diff --git a/services/common/src/main/java/org/collectionspace/services/common/document/JaxbUtils.java b/services/common/src/main/java/org/collectionspace/services/common/document/JaxbUtils.java index 38a87161f..26fa88513 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/document/JaxbUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/document/JaxbUtils.java @@ -154,10 +154,10 @@ public class JaxbUtils { Class c = o.getClass(); Method m = c.getMethod(methodName, argType); Object r = m.invoke(o, argValue); -// if (logger.isDebugEnabled()) { -// logger.debug("completed invocation of " + methodName -// + " for " + c.getName()); -// } + if (logger.isTraceEnabled() == true) { + logger.trace("Completed invocation of " + methodName + + " for " + c.getName() + "with value=" + argValue.toString()); + } return r; } } diff --git a/services/common/src/main/java/org/collectionspace/services/common/security/SecurityInterceptor.java b/services/common/src/main/java/org/collectionspace/services/common/security/SecurityInterceptor.java index 2c2148e70..c1b4b9ed0 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/security/SecurityInterceptor.java +++ b/services/common/src/main/java/org/collectionspace/services/common/security/SecurityInterceptor.java @@ -236,7 +236,7 @@ public class SecurityInterceptor implements PreProcessInterceptor, PostProcessIn try { nuxeoLogout(); } catch (LoginException e) { - String msg = "Unable to logout of the Nuxeo framework"; + String msg = "Unable to logout of the Nuxeo framework."; logger.error(msg, e); } } @@ -244,11 +244,12 @@ public class SecurityInterceptor implements PreProcessInterceptor, PostProcessIn private synchronized void nuxeoLogin() throws LoginException { // // Login as the Nuxeo system/admin user + // nuxeoLogin(null); } private void logLoginContext(LoginContext loginContext) { - logger.info("CollectionSpace services now logged in to Nuxeo with LoginContext: " + logger.trace("CollectionSpace services now logged in to Nuxeo with LoginContext: " + loginContext); Subject subject = loginContext.getSubject(); Set principals = subject.getPrincipals(); @@ -267,20 +268,24 @@ public class SecurityInterceptor implements PreProcessInterceptor, PostProcessIn // if (threadLocalLoginContext == null) { threadLocalLoginContext = new ThreadLocal(); - System.err.println("Created ThreadLocal instance: " + if (logger.isTraceEnabled() == true) { + logger.trace("Created ThreadLocal instance: " + threadLocalLoginContext.getClass().getCanonicalName() + " - " + threadLocalLoginContext.get()); + } } LoginContext loginContext = threadLocalLoginContext.get(); if (loginContext == null) { loginContext = Framework.loginAs(user); frameworkLogins++; threadLocalLoginContext.set(loginContext); - System.err.println("Setting ThreadLocal instance: " - + threadLocalLoginContext.getClass().getCanonicalName() - + " - " - + threadLocalLoginContext.get()); + if (logger.isTraceEnabled() == true) { + logger.trace("Setting ThreadLocal instance: " + + threadLocalLoginContext.getClass().getCanonicalName() + + " - " + + threadLocalLoginContext.get()); + } // // Debug message // diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java index 2918f652a..e10113d10 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java @@ -347,6 +347,9 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { q.setParameter("objectId", id); int rcount = 0; em.getTransaction().begin(); + if (logger.isDebugEnabled() == true) { + logger.debug(q.toString()); + } rcount = q.executeUpdate(); if (logger.isDebugEnabled()) { logger.debug("deleted " + rcount + " relationships for entity " + entityName 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 f94da1362..0972066ea 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 @@ -19,24 +19,17 @@ package org.collectionspace.services.nuxeo.client.java; import java.util.Hashtable; import java.util.List; -import java.util.Set; import java.util.UUID; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import java.util.regex.PatternSyntaxException; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.MultivaluedMap; -import org.collectionspace.services.client.IQueryManager; import org.collectionspace.services.client.PoxPayloadIn; import org.collectionspace.services.client.PoxPayloadOut; import org.collectionspace.services.client.workflow.WorkflowClient; import org.collectionspace.services.common.context.ServiceContext; -import org.collectionspace.services.common.datetime.GregorianCalendarDateTimeUtils; import org.collectionspace.services.common.query.QueryContext; import org.collectionspace.services.common.repository.RepositoryClient; -import org.collectionspace.services.common.workflow.service.nuxeo.WorkflowDocumentModelHandler; import org.collectionspace.services.common.profile.Profiler; import org.collectionspace.services.nuxeo.util.NuxeoUtils; @@ -58,9 +51,6 @@ import org.nuxeo.ecm.core.api.DocumentRef; import org.nuxeo.ecm.core.api.IdRef; import org.nuxeo.ecm.core.api.PathRef; import org.nuxeo.ecm.core.api.repository.RepositoryInstance; -//import org.nuxeo.ecm.core.client.NuxeoClient; -import org.nuxeo.ecm.core.schema.SchemaManager; -import org.nuxeo.runtime.api.Framework; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -106,7 +96,7 @@ public class RepositoryJavaClientImpl implements RepositoryClient(doc); @@ -476,7 +466,7 @@ public class RepositoryJavaClientImpl implements RepositoryClient"; + result = msg = msg + ". Caught exception:" + exceptionMessage; + + if (logger.isTraceEnabled() == true) { + logger.error(msg, e); + } else { + logger.error(msg); + } + + return result; + } + /** * update given document in the Nuxeo repository * @@ -769,28 +775,29 @@ public class RepositoryJavaClientImpl implements RepositoryClient