From 07f2295dfa92b5f0bb959065a6684e7d214489b9 Mon Sep 17 00:00:00 2001 From: j2blake Date: Mon, 26 Sep 2011 16:41:46 +0000 Subject: [PATCH] NIHVIVO-336 StartupStatus logs the messages it receives (with appropriate severity level) so the ServletContextListener doesn't need to do both. --- webapp/config/default.log4j.properties | 1 + .../permissions/PermissionSetsLoader.java | 1 - .../webapp/auth/policy/RootUserPolicy.java | 5 +- .../bean/PropertyRestrictionPolicyHelper.java | 5 +- .../policy/setup/CommonPolicyFamilySetup.java | 10 +--- .../webapp/config/RevisionInfoSetup.java | 4 +- .../webapp/email/FreemarkerEmailFactory.java | 1 - .../vitro/webapp/search/solr/SolrSetup.java | 1 - .../servlet/setup/JenaDataSourceSetup.java | 7 --- .../vitro/webapp/startup/StartupManager.java | 46 +++++-------------- .../vitro/webapp/startup/StartupStatus.java | 27 ++++++++++- .../webapp/startup/StartupManagerTest.java | 29 +++++------- 12 files changed, 55 insertions(+), 82 deletions(-) diff --git a/webapp/config/default.log4j.properties b/webapp/config/default.log4j.properties index cd7fcc39a..e88f12687 100644 --- a/webapp/config/default.log4j.properties +++ b/webapp/config/default.log4j.properties @@ -33,6 +33,7 @@ log4j.appender.AllAppender.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss,SSS} log4j.rootLogger=INFO, AllAppender +log4j.logger.edu.cornell.mannlib.vitro.webapp.startup.StartupStatus=WARN log4j.logger.edu.cornell.mannlib.vitro.webapp.controller.freemarker.BrowseController=WARN log4j.logger.edu.cornell.mannlib.vitro.webapp.dao.jena.pellet.PelletListener=WARN log4j.logger.edu.cornell.mannlib.vitro.webapp.dao.jena.RDBGraphGenerator=WARN diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/permissions/PermissionSetsLoader.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/permissions/PermissionSetsLoader.java index 0e0723180..7e8fb1d50 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/permissions/PermissionSetsLoader.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/permissions/PermissionSetsLoader.java @@ -53,7 +53,6 @@ public class PermissionSetsLoader implements ServletContextListener { wrapper.createPermissionSet(URI_CURATOR, "Curator"); wrapper.createPermissionSet(URI_DBA, "Site Admin"); } catch (Exception e) { - log.error("could not run PermissionSetsLoader" + e); ss.fatal(this, "could not run PermissionSetsLoader" + e); } } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RootUserPolicy.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RootUserPolicy.java index c5c0d533c..11b75160c 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RootUserPolicy.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/RootUserPolicy.java @@ -77,10 +77,7 @@ public class RootUserPolicy implements PolicyIface { ServletPolicyList.addPolicy(ctx, new RootUserPolicy()); } catch (Exception e) { - log.error("could not run " + this.getClass().getSimpleName() - + ": " + e); - ss.fatal(this, "could not run " + this.getClass().getSimpleName() - + ": " + e); + ss.fatal(this, "Failed to set up the RootUserPolicy", e); } } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/bean/PropertyRestrictionPolicyHelper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/bean/PropertyRestrictionPolicyHelper.java index 1daa2a900..273b9baa0 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/bean/PropertyRestrictionPolicyHelper.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/bean/PropertyRestrictionPolicyHelper.java @@ -355,10 +355,7 @@ public class PropertyRestrictionPolicyHelper { .createBean(model); PropertyRestrictionPolicyHelper.setBean(ctx, bean); } catch (Exception e) { - log.error("could not run PropertyRestrictionPolicyHelper$Setup: " - + e); - ss.fatal(this, "could not run PropertyRestrictionPolicyHelper$Setup: " - + e); + ss.fatal(this, "could not set up PropertyRestrictionPolicyHelper", e); } } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/setup/CommonPolicyFamilySetup.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/setup/CommonPolicyFamilySetup.java index 670146044..8f21eb5ef 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/setup/CommonPolicyFamilySetup.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/auth/policy/setup/CommonPolicyFamilySetup.java @@ -6,9 +6,6 @@ import javax.servlet.ServletContext; import javax.servlet.ServletContextEvent; import javax.servlet.ServletContextListener; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; - import edu.cornell.mannlib.vitro.webapp.auth.identifier.ActiveIdentifierBundleFactories; import edu.cornell.mannlib.vitro.webapp.auth.identifier.common.CommonIdentifierBundleFactory; import edu.cornell.mannlib.vitro.webapp.auth.policy.DisplayRestrictedDataByRoleLevelPolicy; @@ -23,8 +20,6 @@ import edu.cornell.mannlib.vitro.webapp.startup.StartupStatus; * Set up the common policy family, with Identifier factory. */ public class CommonPolicyFamilySetup implements ServletContextListener { - private static final Log log = LogFactory - .getLog(CommonPolicyFamilySetup.class); @Override public void contextInitialized(ServletContextEvent sce) { @@ -49,10 +44,7 @@ public class CommonPolicyFamilySetup implements ServletContextListener { ActiveIdentifierBundleFactories.addFactory(sce, factory); } catch (Exception e) { - log.error("could not run " + this.getClass().getSimpleName() + ": " - + e); - ss.fatal(this, "could not run " + this.getClass().getSimpleName() + ": " - + e); + ss.fatal(this, "could not run CommonPolicyFamilySetup", e); } } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/config/RevisionInfoSetup.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/config/RevisionInfoSetup.java index 5220e39d8..e84d491c0 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/config/RevisionInfoSetup.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/config/RevisionInfoSetup.java @@ -72,8 +72,8 @@ public class RevisionInfoSetup implements ServletContextListener { List lines = readRevisionInfo(context); bean = parseRevisionInformation(lines); } catch (Exception e) { - log.error(e, e); - StartupStatus.getBean(context).warning(this, e.getMessage(), e); + StartupStatus.getBean(context).warning(this, + "Failed to load the revision info", e); bean = DUMMY_BEAN; } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/email/FreemarkerEmailFactory.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/email/FreemarkerEmailFactory.java index f440e3978..36cc996c8 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/email/FreemarkerEmailFactory.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/email/FreemarkerEmailFactory.java @@ -168,7 +168,6 @@ public class FreemarkerEmailFactory { + "the system will not send mail to users."); } } catch (Exception e) { - log.error(e, e); ss.warning(this, "Failed to initialize FreemarkerEmailFactory. " + "The system will not be able to send email " diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/solr/SolrSetup.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/solr/SolrSetup.java index a85c28df6..bf3bbcec5 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/search/solr/SolrSetup.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/search/solr/SolrSetup.java @@ -122,7 +122,6 @@ public class SolrSetup implements javax.servlet.ServletContextListener{ log.info("Setup of Solr index completed."); ss.info(this, "Setup of Solr index completed."); } catch (Throwable e) { - log.error("could not setup local solr server",e); ss.fatal(this, "could not setup local solr server",e); } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/servlet/setup/JenaDataSourceSetup.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/servlet/setup/JenaDataSourceSetup.java index e827b52be..4659e6f4b 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/servlet/setup/JenaDataSourceSetup.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/servlet/setup/JenaDataSourceSetup.java @@ -311,13 +311,6 @@ public class JenaDataSourceSetup extends JenaDataSourceSetupBase implements java if( portalURIs.size() > 0 ){ for( String portalUri : portalURIs){ if( portalUri != null && ! portalUri.startsWith(defaultNamespaceFromDeployProperites)){ - log.error("Namespace mismatch between db and deploy.properties."); - log.error("Vivo will not start up correctly because the default namespace specified in deploy.properties does not match the namespace of " + - "a portal in the database. Namespace from deploy.properties: \"" + defaultNamespaceFromDeployProperites + - "\" Namespace from an existing portal: \"" + portalUri + "\" To get the application to start with this " + - "database change the default namespace in deploy.properties " + portalUri.substring(0, portalUri.lastIndexOf("/")+1) + - " Another possibility is that deploy.properties does not specify the intended database."); - StartupStatus ss = StartupStatus.getBean(ctx); ss.warning(this, "Namespace mismatch between db and deploy.properties."); ss.warning(this, "Vivo will not start up correctly because the default namespace specified in deploy.properties does not match the namespace of " + diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupManager.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupManager.java index 6c7310b9f..5ce315c28 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupManager.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupManager.java @@ -66,7 +66,8 @@ public class StartupManager implements ServletContextListener { } } } catch (Exception e) { - recordFatal("Startup threw an unexpected exception.", e); + ss.fatal(this, "Startup threw an unexpected exception.", e); + log.error("Startup threw an unexpected exception.", e); } } @@ -110,10 +111,10 @@ public class StartupManager implements ServletContextListener { } } } catch (NullPointerException e) { - recordFatal("Unable to locate the list of startup listeners: " + ss.fatal(this, "Unable to locate the list of startup listeners: " + FILE_OF_STARTUP_LISTENERS); } catch (IOException e) { - recordFatal( + ss.fatal(this, "Failed while processing the list of startup listeners: " + FILE_OF_STARTUP_LISTENERS, e); } finally { @@ -147,29 +148,17 @@ public class StartupManager implements ServletContextListener { Class c = Class.forName(className); Object o = c.newInstance(); return (ServletContextListener) o; - } catch (ClassNotFoundException e) { - recordFatal("Failed to instantiate listener: '" + className + "'", - e); - return null; - } catch (InstantiationException e) { - recordFatal("Failed to instantiate listener: '" + className + "'", - e); - return null; - } catch (IllegalAccessException e) { - recordFatal("Failed to instantiate listener: '" + className + "'", - e); - return null; } catch (ClassCastException e) { - recordFatal("Instance of '" + className + ss.fatal(this, "Instance of '" + className + "' is not a ServletContextListener", e); return null; } catch (Exception e) { - recordFatal("Failed to instantiate listener: '" + className + "'", - e); + ss.fatal(this, "Failed to instantiate listener: '" + className + + "'", e); return null; } catch (ExceptionInInitializerError e) { - recordFatal("Failed to instantiate listener: '" + className + "'", - e); + ss.fatal(this, "Failed to instantiate listener: '" + className + + "'", e); return null; } } @@ -187,8 +176,6 @@ public class StartupManager implements ServletContextListener { ss.listenerExecuted(listener); } catch (Exception e) { ss.fatal(listener, "Threw unexpected exception", e); - log.error("Listener threw fatal exception: '" - + listener.getClass().getName() + "'", e); } } @@ -201,23 +188,14 @@ public class StartupManager implements ServletContextListener { ServletContextListener iListener = initializeList.get(i); ServletContextListener jListener = initializeList.get(j); if (iListener.getClass().equals(jListener.getClass())) { - recordFatal("File contains duplicate listener classes: '" - + iListener.getClass().getName() + "'"); + ss.fatal(this, + ("File contains duplicate listener classes: '" + + iListener.getClass().getName() + "'")); } } } } - private void recordFatal(String message) { - ss.fatal(this, message); - log.error(message); - } - - private void recordFatal(String message, Throwable cause) { - ss.fatal(this, message, cause); - log.error(message, cause); - } - /** * Notify the listeners that the context is being destroyed, in the reverse * order from how they were notified at initialization. diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupStatus.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupStatus.java index 5f0c3e644..f84c25b03 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupStatus.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/startup/StartupStatus.java @@ -42,7 +42,7 @@ public class StartupStatus { } // ---------------------------------------------------------------------- - // methods to set status + // methods to set status - note that these write to the log also. // ---------------------------------------------------------------------- private List itemList = new ArrayList(); @@ -94,7 +94,30 @@ public class StartupStatus { private void addItem(StatusItem.Level level, ServletContextListener source, String message, Throwable cause) { - itemList.add(new StatusItem(level, source, message, cause)); + StatusItem item = new StatusItem(level, source, message, cause); + itemList.add(item); + + String logMessage = "From " + item.getShortSourceName() + ": " + + item.getMessage(); + if (item.getLevel() == StatusItem.Level.FATAL) { + if (cause == null) { + log.fatal(logMessage); + } else { + log.fatal(logMessage, cause); + } + } else if (item.getLevel() == StatusItem.Level.WARNING) { + if (cause == null) { + log.warn(logMessage); + } else { + log.warn(logMessage, cause); + } + } else { + if (cause == null) { + log.info(logMessage); + } else { + log.info(logMessage, cause); + } + } } // ---------------------------------------------------------------------- diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/startup/StartupManagerTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/startup/StartupManagerTest.java index e77420c00..931fca289 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/startup/StartupManagerTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/startup/StartupManagerTest.java @@ -43,8 +43,9 @@ public class StartupManagerTest extends AbstractTestClass { sm = new StartupManager(); ss = StartupStatus.getBean(ctx); -// setLoggerLevel(this.getClass(), Level.DEBUG); -// setLoggerLevel(StartupManager.class, Level.DEBUG); + // setLoggerLevel(this.getClass(), Level.DEBUG); + setLoggerLevel(StartupStatus.class, Level.OFF); + setLoggerLevel(StartupManager.class, Level.OFF); } @After @@ -56,7 +57,6 @@ public class StartupManagerTest extends AbstractTestClass { @Test public void noSuchFile() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails((String) null); } @@ -78,49 +78,41 @@ public class StartupManagerTest extends AbstractTestClass { @Test public void classDoesNotExist() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails("no.such.class\n"); } @Test public void classThrowsExceptionWhenLoading() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(ThrowsExceptionWhenLoading.class); } @Test public void classIsPrivate() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(PrivateClass.class); } @Test public void noDefaultConstructor() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(NoDefaultConstructor.class); } @Test public void constructorIsPrivate() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(PrivateConstructor.class); } @Test public void constructorThrowsException() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(ConstructorThrowsException.class); } @Test public void notAServletContextListener() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(NotAListener.class); } @Test public void listenerThrowsException() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(InitThrowsException.class); } @@ -139,28 +131,30 @@ public class StartupManagerTest extends AbstractTestClass { // Did they initialize in the correct order? List items = ss.getStatusItems(); assertEquals("how many", 2, items.size()); - assertEquals("init order 1", listener1Name, items.get(0).getSourceName()); - assertEquals("init order 2", listener2Name, items.get(1).getSourceName()); + assertEquals("init order 1", listener1Name, items.get(0) + .getSourceName()); + assertEquals("init order 2", listener2Name, items.get(1) + .getSourceName()); sm.contextDestroyed(sce); // Did they destroy in reverse order? items = ss.getStatusItems(); assertEquals("how many", 4, items.size()); - assertEquals("destroy order 1", listener2Name, items.get(2).getSourceName()); - assertEquals("destroy order 2", listener1Name, items.get(3).getSourceName()); + assertEquals("destroy order 1", listener2Name, items.get(2) + .getSourceName()); + assertEquals("destroy order 2", listener1Name, items.get(3) + .getSourceName()); } @Test public void duplicateListeners() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(SucceedsWithInfo.class, SucceedsWithWarning.class, SucceedsWithInfo.class); } @Test public void dontExecuteAfterFailure() { - setLoggerLevel(StartupManager.class, Level.OFF); assertStartupFails(InitThrowsException.class, SucceedsWithInfo.class); for (StatusItem item : ss.getStatusItems()) { @@ -321,4 +315,5 @@ public class StartupManagerTest extends AbstractTestClass { item.getSourceName(), item.getMessage(), item.getCause())); } } + }