From 2f6e465f50b9d444bc6719459f28e9963a26b8cd Mon Sep 17 00:00:00 2001 From: Richard Millet Date: Mon, 14 Nov 2011 20:37:57 +0000 Subject: [PATCH] CSPACE-4526: Cleaning up some of the logging messages in the AuthZ import process. --- .../jaxrs/CSpaceResteasyBootstrap.java | 4 +- .../java/org/collectionspace/ImportAuthz.java | 140 ++++++++++++------ .../driver/AuthorizationSeedDriver.java | 18 +-- .../config/TenantBindingConfigReaderImpl.java | 53 ++++--- 4 files changed, 132 insertions(+), 83 deletions(-) diff --git a/services/JaxRsServiceProvider/src/main/java/org/collectionspace/services/jaxrs/CSpaceResteasyBootstrap.java b/services/JaxRsServiceProvider/src/main/java/org/collectionspace/services/jaxrs/CSpaceResteasyBootstrap.java index 9a1d87478..c7fcf7790 100644 --- a/services/JaxRsServiceProvider/src/main/java/org/collectionspace/services/jaxrs/CSpaceResteasyBootstrap.java +++ b/services/JaxRsServiceProvider/src/main/java/org/collectionspace/services/jaxrs/CSpaceResteasyBootstrap.java @@ -13,10 +13,10 @@ public class CSpaceResteasyBootstrap extends ResteasyBootstrap { // private static final String public void contextInitialized(ServletContextEvent event) { if (true) { - System.out.print("Pausing 10 seconds in RESTEasy bootstrap for you to attached the debugger"); + System.out.print("Pausing 1 seconds in RESTEasy bootstrap for you to attached the debugger"); long startTime, currentTime; currentTime = startTime = System.currentTimeMillis(); - long stopTime = startTime + 10 * 1000; //5 seconds + long stopTime = startTime + 1 * 1000; //5 seconds do { if (currentTime % 1000 == 0) { System.out.print("."); diff --git a/services/authorization-mgt/import/src/main/java/org/collectionspace/ImportAuthz.java b/services/authorization-mgt/import/src/main/java/org/collectionspace/ImportAuthz.java index 72bf14356..0b348e3ed 100644 --- a/services/authorization-mgt/import/src/main/java/org/collectionspace/ImportAuthz.java +++ b/services/authorization-mgt/import/src/main/java/org/collectionspace/ImportAuthz.java @@ -27,6 +27,8 @@ */ package org.collectionspace; +import java.io.PrintStream; + import net.sf.ehcache.CacheException; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.CommandLineParser; @@ -50,6 +52,10 @@ public class ImportAuthz { final private static String OPTIONS_HELP = "help"; final private static String MSG_SEPARATOR = "--"; + final private static String LOGGING_SEPARATOR_HEAD = ">>>>>>>>>>>>>>>>>>>>>>>>>>>>"; + final private static String LOGGING_SEPARATOR_TAIL = "<<<<<<<<<<<<<<<<<<<<<<<<<<<<"; + final private static String LOGGING_INFO_PREFIX = "[INFO] "; + final private static String LOGGING_ERROR_PREFIX = "[ERROR] "; final private static boolean generateOnly(String param) { boolean result = false; @@ -58,6 +64,79 @@ public class ImportAuthz { } return result; } + + // + // Private logging methods. We should try to get this code to use a logging utility like Log4j, Slf4j, etc. + // I'm not sure why we are not using a logging util? But at least we're consolidating all calls to System.out and Sytem.err. + // + private static void logError(String errMessage) { + System.out.println(LOGGING_ERROR_PREFIX + errMessage); + } + + private static void logInfo(PrintStream outStream, String infoMessage) { + outStream.println(LOGGING_INFO_PREFIX + infoMessage); + } + + private static void logInfo(String infoMessage) { + logInfo(System.out, infoMessage); + } + + private static void logConfiguration(String user, + String password, + String tenantBinding, + String exportDir) { + logInfo(LOGGING_SEPARATOR_HEAD); + logInfo("Creating CollectionSpace authorization metadata using the following settings:"); + logInfo("\tuser=" + user); + logInfo("\tpassword=" + password); + logInfo("\ttenantBinding=" + tenantBinding); + logInfo("\texportDir=" + exportDir); + logInfo(LOGGING_SEPARATOR_TAIL); + } + + private static void printUsage(PrintStream outStream) { + StringBuilder sb = new StringBuilder(); + sb.append("\nUsage : java -cp " + ImportAuthz.class.getName() + " "); + sb.append("\nOptions :"); + sb.append("\n -g <" + OPTIONS_GENERATE_ONLY + "> generate only, do not seed AuthZ values in the security tables"); + sb.append("\n -u <" + OPTIONS_USERNAME + "> cspace username"); + sb.append("\n -p <" + OPTIONS_PASSWORD + "> password"); + sb.append("\n -b <" + OPTIONS_TENANT_BINDING + "> tenant binding file (fully qualified path)"); + sb.append("\n -edir <" + OPTIONS_EXPORT_DIR + "> directory to export authz data into"); + logInfo(sb.toString()); + } + + private static void printUsage() { + printUsage(System.out); + } + + private static void logInitialErrorCauseMsg(Throwable t) { + if (t != null) { + if (t.getCause() != null) { + logInitialErrorCauseMsg(t.getCause()); + } else { + logError(t.getMessage()); + } + } + } + + private static Options createOptions() { + Options options = new Options(); + options.addOption("g", true, OPTIONS_GENERATE_ONLY); + options.addOption("u", true, OPTIONS_USERNAME); + options.addOption("p", true, OPTIONS_PASSWORD); + options.addOption("b", true, OPTIONS_TENANT_BINDING); + options.addOption("edir", true, OPTIONS_EXPORT_DIR); + options.addOption("h", true, OPTIONS_HELP); + return options; + } + // + // End of logging methods. + // + + // + // Create our AuthZ metadata + // public static void main(String[] args) { Options options = createOptions(); @@ -75,10 +154,10 @@ public class ImportAuthz { String password = line.getOptionValue("p"); String tenantBinding = line.getOptionValue("b"); String exportDir = line.getOptionValue("edir"); - System.out.println("user=" + user - + " password=" + password - + " tenantBinding=" + tenantBinding - + " exportDir=" + exportDir); + logConfiguration(user, password, tenantBinding, exportDir); + // + // Instantiate an AuthZ seed driver and ask it to generate our AuthZ metadata + // AuthorizationSeedDriver driver = new AuthorizationSeedDriver( user, password, tenantBinding, exportDir); driver.generate(); @@ -88,54 +167,19 @@ public class ImportAuthz { // if (generateOnly(generate_only) == false) { driver.seed(); - } { - System.out.println("WARNING: '-g' was set to 'true' so AuthZ tables were not seeded."); + } else { + logError("WARNING: '-g' was set to 'true' so AuthZ tables were ***NOT*** seeded."); } } catch (ParseException exp) { - // oops, something went wrong - System.err.println("Parsing failed. Reason: " + exp.getMessage()); + logError("Parsing failed. Reason: " + exp.getMessage()); } catch (Exception e) { - System.out.println("Error : " + e.getMessage()); - System.out.println(MSG_SEPARATOR); - printUsage(); - System.out.println(MSG_SEPARATOR); - System.out.println("Import failed: "); - printInitialErrorCauseMsg(e); + logError("Error : " + e.getMessage()); + logError(MSG_SEPARATOR); + printUsage(System.err); + logError(MSG_SEPARATOR); + logError("Import failed: "); + logInitialErrorCauseMsg(e); System.exit(1); } - - } - - private static void printInitialErrorCauseMsg(Throwable t) { - if (t != null) { - if (t.getCause() != null) { - printInitialErrorCauseMsg(t.getCause()); - } else { - System.out.println(t.getMessage()); - } - } - } - - private static Options createOptions() { - Options options = new Options(); - options.addOption("g", true, OPTIONS_GENERATE_ONLY); - options.addOption("u", true, OPTIONS_USERNAME); - options.addOption("p", true, OPTIONS_PASSWORD); - options.addOption("b", true, OPTIONS_TENANT_BINDING); - options.addOption("edir", true, OPTIONS_EXPORT_DIR); - options.addOption("h", true, OPTIONS_HELP); - return options; - } - - private static void printUsage() { - StringBuilder sb = new StringBuilder(); - sb.append("\nUsage : java -cp " + ImportAuthz.class.getName() + " "); - sb.append("\nOptions :"); - sb.append("\n -g <" + OPTIONS_GENERATE_ONLY + "> generate only, do not seed AuthZ values in the security tables"); - sb.append("\n -u <" + OPTIONS_USERNAME + "> cspace username"); - sb.append("\n -p <" + OPTIONS_PASSWORD + "> password"); - sb.append("\n -b <" + OPTIONS_TENANT_BINDING + "> tenant binding file (fully qualified path)"); - sb.append("\n -edir <" + OPTIONS_EXPORT_DIR + "> directory to export authz data into"); - System.out.println(sb.toString()); } } diff --git a/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/driver/AuthorizationSeedDriver.java b/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/driver/AuthorizationSeedDriver.java index 1273605ef..56a42a279 100644 --- a/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/driver/AuthorizationSeedDriver.java +++ b/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/driver/AuthorizationSeedDriver.java @@ -111,12 +111,10 @@ public class AuthorizationSeedDriver { authzGen.exportDefaultPermissions(exportDir + File.separator + PERMISSION_FILE); authzGen.exportDefaultPermissionRoles(exportDir + File.separator + PERMISSION_ROLE_FILE); if (logger.isDebugEnabled()) { - logger.debug("authorization generation completed "); + logger.debug("Authorization generation completed but not yet persisted."); } } catch (Exception ex) { - if (logger.isDebugEnabled()) { - ex.printStackTrace(); - } + logger.error("AuthorizationSeedDriver caught an exception: ", ex); throw new RuntimeException(ex); } } @@ -157,10 +155,11 @@ public class AuthorizationSeedDriver { new String[]{SPRING_SECURITY_METADATA}); login(); System.setProperty("spring-beans-config", SPRING_SECURITY_METADATA); + // authZ local not used but call to AuthZ.get() has side-effect of initializing our Spring Security context AuthZ authZ = AuthZ.get(); txManager = (org.springframework.jdbc.datasource.DataSourceTransactionManager) appContext.getBean("transactionManager"); if (logger.isDebugEnabled()) { - logger.debug("spring setup complete"); + logger.debug("Spring Security setup complete."); } } @@ -173,14 +172,14 @@ public class AuthorizationSeedDriver { Authentication authRequest = new UsernamePasswordAuthenticationToken(user, password, gauths); SecurityContextHolder.getContext().setAuthentication(authRequest); if (logger.isDebugEnabled()) { - logger.debug("login successful for user=" + user); + logger.debug("Spring Security login successful for user=" + user); } } private void logout() { SecurityContextHolder.getContext().setAuthentication(null); if (logger.isDebugEnabled()) { - logger.debug("logged out user=" + user); + logger.debug("Spring Security logged out user=" + user); } } @@ -202,10 +201,9 @@ public class AuthorizationSeedDriver { authzStore.store(permRoleRel); } - if (logger.isDebugEnabled()) { - logger.debug("authroization storage completed "); + if (logger.isInfoEnabled()) { + logger.info("Authroization metata persisted."); } - } private TransactionStatus beginTransaction(String name) { diff --git a/services/common/src/main/java/org/collectionspace/services/common/config/TenantBindingConfigReaderImpl.java b/services/common/src/main/java/org/collectionspace/services/common/config/TenantBindingConfigReaderImpl.java index 8bf40e44b..c3a7c8deb 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/config/TenantBindingConfigReaderImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/common/config/TenantBindingConfigReaderImpl.java @@ -39,25 +39,13 @@ import org.collectionspace.services.common.tenant.TenantBindingType; import org.collectionspace.services.common.tenant.TenantBindingConfig; import org.collectionspace.services.common.types.PropertyItemType; -import ch.elca.el4j.util.codingsupport.Reject; -import ch.elca.el4j.services.xmlmerge.AbstractXmlMergeException; -import ch.elca.el4j.services.xmlmerge.ConfigurationException; import ch.elca.el4j.services.xmlmerge.Configurer; -import ch.elca.el4j.services.xmlmerge.XmlMerge; import ch.elca.el4j.services.xmlmerge.config.AttributeMergeConfigurer; import ch.elca.el4j.services.xmlmerge.config.ConfigurableXmlMerge; -import ch.elca.el4j.services.xmlmerge.config.PropertyXPathConfigurer; -import ch.elca.el4j.services.xmlmerge.merge.DefaultXmlMerge; - -import org.apache.commons.io.FileUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import ch.elca.el4j.services.xmlmerge.Configurer; -import ch.elca.el4j.services.xmlmerge.config.AttributeMergeConfigurer; -import ch.elca.el4j.services.xmlmerge.config.ConfigurableXmlMerge; - /** * ServicesConfigReader reads service layer specific configuration * @@ -101,16 +89,35 @@ public class TenantBindingConfigReaderImpl protected File getTenantsRootDir() { File result = null; - String tenantsRootPath = getConfigRootDir() + File.separator + TENANT_BINDINGS_ROOTDIRNAME; - File tenantsRootDir = new File(tenantsRootPath); - if (tenantsRootDir.exists() == true) { - result = tenantsRootDir; - logger.debug("Tenants home directory is: " + tenantsRootDir.getAbsolutePath()); //FIXME: REM - Add proper if (logger.isDebug() == true) check - } else { - logger.error("Tenants home directory is missing. Can't find: " + tenantsRootDir.getAbsolutePath()); //FIXME: REM - Add proper if (logger.isError() == true) check + String errMessage = null; + try { + String tenantsRootPath = getConfigRootDir() + File.separator + TENANT_BINDINGS_ROOTDIRNAME; + File tenantsRootDir = new File(tenantsRootPath); + if (tenantsRootDir.exists() == true) { + result = tenantsRootDir; + if (logger.isDebugEnabled() == true) { + logger.debug("The home directory for all tenants is at: " + result.getCanonicalPath()); + } + } else { + errMessage = "The home directory for all tenants is missing or inaccesible: "; + try { + errMessage = errMessage + tenantsRootDir.getCanonicalPath(); + } catch (IOException ioException) { + errMessage = errMessage + tenantsRootDir.getAbsolutePath(); + } + } + } catch (IOException e) { + // Log this exception, but continue anyway. Caller should handle the null result gracefully. + logger.equals(e); } + + if (errMessage != null) { + logger.error(errMessage); + } + return result; } + /* * Take the directory of the prototype bindings and the directory of the delta bindings. Merge the two and create (replace) a file * named "tenant-bindings.xml" @@ -127,7 +134,7 @@ public class TenantBindingConfigReaderImpl result = new ConfigurableXmlMerge(configurer).merge(inputStreamArray); } catch (Exception e) { logger.error("Could not merge tenant configuration delta file: " + - deltaFile.getAbsolutePath(), e); + deltaFile.getCanonicalPath(), e); } // // Try to save the merge output to a file that is suffixed with ".merged.xml" in the same directory @@ -136,7 +143,7 @@ public class TenantBindingConfigReaderImpl if (result != null) { File outputDir = deltaFile.getParentFile(); String mergedFileName = outputDir.getAbsolutePath() + File.separator + - this.TENANT_BINDINGS_FILENAME_PREFIX + MERGED_SUFFIX; + TenantBindingConfigReaderImpl.TENANT_BINDINGS_FILENAME_PREFIX + MERGED_SUFFIX; File mergedOutFile = new File(mergedFileName); try { FileUtils.copyInputStreamToFile(result, mergedOutFile); @@ -289,8 +296,8 @@ public class TenantBindingConfigReaderImpl docTypes.put(docTypeKey, serviceBinding); } } - if (logger.isDebugEnabled()) { - logger.debug("readServiceBindings() added service " + if (logger.isTraceEnabled()) { + logger.trace("readServiceBindings() added service " + " name=" + key + " workspace=" + serviceBinding.getName()); } -- 2.47.3