This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to annotated tag 0.8 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 52848c0de5250c77a0c7c619ce8d6bce14b474e3 Author: Martin Desruisseaux <[email protected]> AuthorDate: Fri Nov 3 15:10:10 2017 +0000 Try to be more informative in some logging messages related to SIS_DATA environment variable. git-svn-id: https://svn.apache.org/repos/asf/sis/branches/0.8@1814200 13f79535-47bb-0310-9956-ffa450edef68 --- .../java/org/apache/sis/console/AboutCommand.java | 2 + .../sis/internal/metadata/sql/Initializer.java | 2 +- .../apache/sis/internal/system/DataDirectory.java | 34 ++-- .../java/org/apache/sis/util/logging/Logging.java | 173 +++++++++++---------- .../apache/sis/util/logging/WarningListeners.java | 18 ++- .../sis/util/logging/WarningListenersTest.java | 1 + .../internal/storage/DocumentedStoreProvider.java | 13 +- .../org/apache/sis/internal/storage/Resources.java | 5 + .../sis/internal/storage/Resources.properties | 1 + .../sis/internal/storage/Resources_fr.properties | 1 + 10 files changed, 143 insertions(+), 107 deletions(-) diff --git a/application/sis-console/src/main/java/org/apache/sis/console/AboutCommand.java b/application/sis-console/src/main/java/org/apache/sis/console/AboutCommand.java index 85aa69a..5497e95 100644 --- a/application/sis-console/src/main/java/org/apache/sis/console/AboutCommand.java +++ b/application/sis-console/src/main/java/org/apache/sis/console/AboutCommand.java @@ -39,6 +39,7 @@ import org.apache.sis.util.collection.TableColumn; import org.apache.sis.internal.system.Loggers; import org.apache.sis.internal.system.Supervisor; import org.apache.sis.internal.system.SupervisorMBean; +import org.apache.sis.internal.system.DataDirectory; import org.apache.sis.internal.util.StandardDateFormat; import org.apache.sis.internal.util.X364; @@ -84,6 +85,7 @@ final class AboutCommand extends CommandRunner { */ @Override public int run() throws Exception { + DataDirectory.quiet(); /* * Check the number of arguments, which can be 0 or 1. If present, * the argument is the name and port number of a remote machine. diff --git a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/sql/Initializer.java b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/sql/Initializer.java index 1f26cdf..5546619 100644 --- a/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/sql/Initializer.java +++ b/core/sis-metadata/src/main/java/org/apache/sis/internal/metadata/sql/Initializer.java @@ -267,7 +267,7 @@ public abstract class Initializer { final LogRecord record = Messages.getResources(null).getLogRecord( Level.CONFIG, Messages.Keys.JNDINotSpecified_1, JNDI); record.setLoggerName(Loggers.SQL); - Logging.log(Initializer.class, "getDataSource", record); + Logging.log(null, null, record); // Let Logging.log(…) infers the public caller. } /* * At this point we determined that there is no JNDI context or no object binded to "jdbc/SpatialMetadata". diff --git a/core/sis-utility/src/main/java/org/apache/sis/internal/system/DataDirectory.java b/core/sis-utility/src/main/java/org/apache/sis/internal/system/DataDirectory.java index 6e1d043..56d635b 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/internal/system/DataDirectory.java +++ b/core/sis-utility/src/main/java/org/apache/sis/internal/system/DataDirectory.java @@ -84,25 +84,33 @@ public enum DataDirectory { private Path directory; /** + * Prevents the log message about {@code SIS_DATA} environment variable not set. + * This is used for the "About" command line action only. + */ + public static void quiet() { + lastWarning = Messages.Keys.DataDirectoryNotSpecified_1; + } + + /** * Logs a message to the {@code "org.apache.sis.system"} logger only if different than the last warning. */ - private static void warning(final String method, final Exception e, final short key, final Object... parameters) { + private static void warning(final Exception e, final short key, final Object... parameters) { if (key != lastWarning) { lastWarning = key; - log(Level.WARNING, method, e, key, parameters); + log(Level.WARNING, e, key, parameters); } } /** * Logs a message to the {@code "org.apache.sis.system"} logger. */ - private static void log(final Level level, final String method, final Exception e, final short key, final Object... parameters) { + private static void log(final Level level, final Exception e, final short key, final Object... parameters) { final LogRecord record = Messages.getResources(null).getLogRecord(level, key, parameters); record.setLoggerName(Loggers.SYSTEM); if (e != null) { record.setThrown(e); } - Logging.log(DataDirectory.class, method, record); + Logging.log(null, null, record); // Let Logging.log(…) infers the public caller. } /** @@ -153,22 +161,22 @@ public enum DataDirectory { if (rootDirectory == null) try { final String dir = getenv(); if (dir == null || dir.isEmpty()) { - warning("getRootDirectory", null, Messages.Keys.DataDirectoryNotSpecified_1, ENV); + warning(null, Messages.Keys.DataDirectoryNotSpecified_1, ENV); } else try { final Path path = Paths.get(dir); if (!Files.isDirectory(path)) { - warning("getRootDirectory", null, Messages.Keys.DataDirectoryDoesNotExist_2, ENV, path); + warning(null, Messages.Keys.DataDirectoryDoesNotExist_2, ENV, path); } else if (!Files.isReadable(path)) { - warning("getRootDirectory", null, Messages.Keys.DataDirectoryNotReadable_2, ENV, path); + warning(null, Messages.Keys.DataDirectoryNotReadable_2, ENV, path); } else { - log(Level.CONFIG, "getRootDirectory", null, Messages.Keys.DataDirectory_2, ENV, path); + log(Level.CONFIG, null, Messages.Keys.DataDirectory_2, ENV, path); rootDirectory = path; } } catch (InvalidPathException e) { - warning("getRootDirectory", e, Messages.Keys.DataDirectoryDoesNotExist_2, ENV, dir); + warning(e, Messages.Keys.DataDirectoryDoesNotExist_2, ENV, dir); } } catch (SecurityException e) { - warning("getRootDirectory", e, Messages.Keys.DataDirectoryNotAuthorized_1, ENV); + warning(e, Messages.Keys.DataDirectoryNotAuthorized_1, ENV); } return rootDirectory; } @@ -198,12 +206,12 @@ public enum DataDirectory { } else if (Files.isWritable(root)) try { directory = Files.createDirectory(dir); } catch (IOException e) { - warning("getDirectory", e, Messages.Keys.DataDirectoryNotWritable_2, ENV, root); + warning(e, Messages.Keys.DataDirectoryNotWritable_2, ENV, root); } else { - warning("getDirectory", null, Messages.Keys.DataDirectoryNotWritable_2, ENV, root); + warning(null, Messages.Keys.DataDirectoryNotWritable_2, ENV, root); } } catch (SecurityException e) { - warning("getDirectory", e, Messages.Keys.DataDirectoryNotAccessible_2, ENV, name); + warning(e, Messages.Keys.DataDirectoryNotAccessible_2, ENV, name); } } } diff --git a/core/sis-utility/src/main/java/org/apache/sis/util/logging/Logging.java b/core/sis-utility/src/main/java/org/apache/sis/util/logging/Logging.java index d587c8d..f9f7934 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/util/logging/Logging.java +++ b/core/sis-utility/src/main/java/org/apache/sis/util/logging/Logging.java @@ -21,6 +21,7 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.logging.LogRecord; +import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.Configuration; import org.apache.sis.util.Static; import org.apache.sis.util.Exceptions; @@ -47,7 +48,7 @@ import org.apache.sis.internal.system.Modules; * </ul> * * @author Martin Desruisseaux (Geomatys) - * @version 0.6 + * @version 0.8 * @since 0.3 * @module */ @@ -204,21 +205,98 @@ public final class Logging extends Static { * @param method the name of the method which is logging a record. * @param record the record to log. */ - public static void log(final Class<?> classe, final String method, final LogRecord record) { - record.setSourceClassName(classe.getCanonicalName()); - record.setSourceMethodName(method); + public static void log(final Class<?> classe, String method, final LogRecord record) { + ArgumentChecks.ensureNonNull("record", record); final String loggerName = record.getLoggerName(); - final Logger logger; + Logger logger; if (loggerName == null) { logger = getLogger(classe); record.setLoggerName(logger.getName()); } else { logger = getLogger(loggerName); } + if (classe != null && method != null) { + record.setSourceClassName(classe.getCanonicalName()); + record.setSourceMethodName(method); + } else { + /* + * If the given class or method is null, infer them from stack trace. We do not document this feature + * in public API because the rules applied here are heuristic and may change in any future SIS version. + */ + logger = inferCaller(logger, (classe != null) ? classe.getCanonicalName() : null, + method, Thread.currentThread().getStackTrace(), record); + } logger.log(record); } /** + * Sets the {@code LogRecord} source class and method names according values inferred from the given stack trace. + * This method inspects the given stack trace, skips what looks like internal API based on heuristic rules, then + * if some arguments are non-null tries to match them. + * + * @param logger where the log record will be sent after this method call, or {@code null} if unknown. + * @param classe the name of the class to report in the log record, or {@code null} if unknown. + * @param method the name of the method to report in the log record, or {@code null} if unknown. + * @param trace the stack trace to use for inferring the class and method names. + * @param record the record where to set the class and method names. + * @return the record to use for logging the record. + */ + static Logger inferCaller(Logger logger, String classe, String method, + final StackTraceElement[] trace, final LogRecord record) + { + for (final StackTraceElement element : trace) { + /* + * Search for the first stack trace element with a classname matching the expected one. + * We compare against the name of the class given in argument if it was non-null. + */ + final String classname = element.getClassName(); + if (classe != null) { + if (!classname.equals(classe)) { + continue; + } + } else if (!WarningListeners.isPublic(element)) { + continue; + } + /* + * Now that we have a stack trace element from the expected class (or any + * element if we don't know the class), make sure that we have the right method. + */ + final String methodName = element.getMethodName(); + if (method != null && !methodName.equals(method)) { + continue; + } + /* + * Now computes every values that are null, and stop the loop. + */ + if (logger == null) { + final int separator = classname.lastIndexOf('.'); + logger = getLogger((separator >= 1) ? classname.substring(0, separator-1) : ""); + } + if (classe == null) { + classe = classname; + } + if (method == null) { + method = methodName; + } + break; + } + /* + * The logger may stay null if we have been unable to find a suitable stack trace. + * Fallback on the global logger. + */ + if (logger == null) { + logger = getLogger(Logger.GLOBAL_LOGGER_NAME); + } + if (classe != null) { + record.setSourceClassName(classe); + } + if (method != null) { + record.setSourceMethodName(method); + } + return logger; + } + + /** * Invoked when an unexpected error occurred. This method logs a message at {@link Level#WARNING} * to the specified logger. The originating class name and method name can optionally be specified. * If any of them is {@code null}, then it will be inferred from the error stack trace as described below. @@ -290,79 +368,6 @@ public final class Logging extends Static { return false; } /* - * Loggeable, so complete the null argument from the stack trace if we can. - */ - if (logger == null || classe == null || method == null) { - String paquet = (logger != null) ? logger.getName() : null; - for (final StackTraceElement element : error.getStackTrace()) { - /* - * Searches for the first stack trace element with a classname matching the - * expected one. We compare preferably against the name of the class given - * in argument, or against the logger name (taken as the package name) otherwise. - */ - final String classname = element.getClassName(); - if (classe != null) { - if (!classname.equals(classe)) { - continue; - } - } else if (paquet != null) { - if (!classname.startsWith(paquet)) { - continue; - } - final int length = paquet.length(); - if (classname.length() > length) { - /* - * We expect '.' but we accept also '$' or end of string. - */ - final char separator = classname.charAt(length); - if (Character.isJavaIdentifierPart(separator)) { - continue; - } - } - } - /* - * Now that we have a stack trace element from the expected class (or any - * element if we don't know the class), make sure that we have the right method. - */ - final String methodName = element.getMethodName(); - if (method != null && !methodName.equals(method)) { - continue; - } - /* - * Now computes every values that are null, and stop the loop. - */ - if (paquet == null) { - final int separator = classname.lastIndexOf('.'); - paquet = (separator >= 1) ? classname.substring(0, separator-1) : ""; - logger = getLogger(paquet); - if (!logger.isLoggable(level)) { - return false; - } - } - if (classe == null) { - classe = classname; - } - if (method == null) { - method = methodName; - } - break; - } - /* - * The logger may stay null if we have been unable to find a suitable - * stack trace. Fallback on the global logger. - */ - if (logger == null) { - logger = getLogger(Logger.GLOBAL_LOGGER_NAME); - if (!logger.isLoggable(level)) { - return false; - } - } - } - /* - * Now prepare the log message. If we have been unable to figure out a source class and - * method name, we will fallback on JDK logging default mechanism, which may return a - * less relevant name than our attempt to use the logger name as the package name. - * * The message is fetched using Exception.getMessage() instead than getLocalizedMessage() * because in a client-server architecture, we want the locale on the server-side instead * than the locale on the client side. See LocalizedException policy. @@ -375,15 +380,15 @@ public final class Logging extends Static { message = buffer.toString(); message = Exceptions.formatChainedMessages(null, message, error); final LogRecord record = new LogRecord(level, message); - if (classe != null) { - record.setSourceClassName(classe); - } - if (method != null) { - record.setSourceMethodName(method); - } if (level.intValue() >= LEVEL_THRESHOLD_FOR_STACKTRACE) { record.setThrown(error); } + if (logger == null || classe == null || method == null) { + logger = inferCaller(logger, classe, method, error.getStackTrace(), record); + } else { + record.setSourceClassName(classe); + record.setSourceMethodName(method); + } record.setLoggerName(logger.getName()); logger.log(record); return true; diff --git a/core/sis-utility/src/main/java/org/apache/sis/util/logging/WarningListeners.java b/core/sis-utility/src/main/java/org/apache/sis/util/logging/WarningListeners.java index 2f7c54d..f3b4a80 100644 --- a/core/sis-utility/src/main/java/org/apache/sis/util/logging/WarningListeners.java +++ b/core/sis-utility/src/main/java/org/apache/sis/util/logging/WarningListeners.java @@ -27,6 +27,7 @@ import org.apache.sis.util.Localized; import org.apache.sis.util.Exceptions; import org.apache.sis.util.ArgumentChecks; import org.apache.sis.util.resources.Errors; +import org.apache.sis.internal.system.Modules; import org.apache.sis.internal.util.UnmodifiableArrayList; @@ -267,6 +268,7 @@ public class WarningListeners<S> implements Localized { /** * Returns {@code true} if the given stack trace element describes a method considered part of public API. * This method is invoked in order to infer the class and method names to declare in a {@link LogRecord}. + * We do not document this feature in public Javadoc because it is based on heuristic rules that may change. * * <p>The current implementation compares the class name against a hard-coded list of classes to hide. * This implementation may change in any future SIS version.</p> @@ -274,11 +276,17 @@ public class WarningListeners<S> implements Localized { * @param e a stack trace element. * @return {@code true} if the class and method specified by the given element can be considered public API. */ - private static boolean isPublic(final StackTraceElement e) { - final String classname = e.getClassName(); - return !classname.equals("org.apache.sis.util.logging.WarningListeners") && - !classname.contains(".internal.") && !classname.startsWith("java") && - classname.indexOf('$') < 0 && e.getMethodName().indexOf('$') < 0; + static boolean isPublic(final StackTraceElement e) { + final String classname = e.getClassName(); + if (classname.startsWith("java") || classname.contains(".internal.") || + classname.indexOf('$') >= 0 || e.getMethodName().indexOf('$') >= 0) + { + return false; + } + if (classname.startsWith(Modules.CLASSNAME_PREFIX + "util.logging.")) { + return classname.endsWith("Test"); // Consider JUnit tests as public. + } + return true; // TODO: with StackWalker on JDK9, check if the class is public. } /** diff --git a/core/sis-utility/src/test/java/org/apache/sis/util/logging/WarningListenersTest.java b/core/sis-utility/src/test/java/org/apache/sis/util/logging/WarningListenersTest.java index e81f926..c2ff86f 100644 --- a/core/sis-utility/src/test/java/org/apache/sis/util/logging/WarningListenersTest.java +++ b/core/sis-utility/src/test/java/org/apache/sis/util/logging/WarningListenersTest.java @@ -110,6 +110,7 @@ public final strictfp class WarningListenersTest extends TestCase implements War listeners.warning("The message", null); listeners.removeWarningListener(this); assertNotNull("Listener has not been notified.", warning); + assertEquals(getClass().getName(), warning.getSourceClassName()); assertEquals("testWarning", warning.getSourceMethodName()); assertEquals("The message", warning.getMessage()); } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/DocumentedStoreProvider.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/DocumentedStoreProvider.java index ffca89f..e20be3c 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/DocumentedStoreProvider.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/DocumentedStoreProvider.java @@ -16,7 +16,8 @@ */ package org.apache.sis.internal.storage; -import java.util.logging.Logger; +import java.util.logging.Level; +import java.util.logging.LogRecord; import org.opengis.metadata.distribution.Format; import org.apache.sis.metadata.sql.MetadataSource; import org.apache.sis.metadata.sql.MetadataStoreException; @@ -98,13 +99,17 @@ public abstract class DocumentedStoreProvider extends URIDataStore.Provider { if (listeners != null) { listeners.warning(null, e); } else { - final Logger logger = Logging.getLogger(Modules.STORAGE); + final Level level; if (!logged) { logged = true; // Not atomic - not a big deal if we use warning level twice. - Logging.unexpectedException(logger, getClass(), "getFormat", e); + level = Level.WARNING; } else { - Logging.recoverableException(logger, getClass(), "getFormat", e); + level = Level.FINE; } + final LogRecord record = Resources.forLocale(null).getLogRecord(level, + Resources.Keys.CanNotGetCommonMetadata_2, getShortName(), e.getLocalizedMessage()); + record.setLoggerName(Modules.STORAGE); + Logging.log(getClass(), "getFormat", record); } } return super.getFormat(); diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java index 5774c73..e9dedd4 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.java @@ -67,6 +67,11 @@ public final class Resources extends IndexedResourceBundle { public static final short AmbiguousName_4 = 15; /** + * Can not get metadata common to “{0}” files. Reason: {1} + */ + public static final short CanNotGetCommonMetadata_2 = 39; + + /** * Can not read the Coordinate Reference System (CRS) Well Known Text (WKT) in “{0}”. */ public static final short CanNotReadCRS_WKT_1 = 37; diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties index 78d4297..42f2571 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources.properties @@ -20,6 +20,7 @@ # For resources shared by all modules in the Apache SIS project, see "org.apache.sis.util.resources" package. # AmbiguousName_4 = Name \u201c{3}\u201d is ambiguous because it can be understood as either \u201c{1}\u201d or \u201c{2}\u201d in the context of \u201c{0}\u201d data. +CanNotGetCommonMetadata_2 = Can not get metadata common to \u201c{0}\u201d files. Reason: {1} CanNotReadCRS_WKT_1 = Can not read the Coordinate Reference System (CRS) Well Known Text (WKT) in \u201c{0}\u201d. CanNotReadDirectory_1 = Can not read \u201c{0}\u201d directory. CanNotReadFile_2 = Can not read \u201c{1}\u201d as a file in the {0} format. diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties index 9801ddc..45f1d8c 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/Resources_fr.properties @@ -25,6 +25,7 @@ # U+00A0 NO-BREAK SPACE before : # AmbiguousName_4 = Le nom \u00ab\u202f{3}\u202f\u00bb est ambigu\u00eb car il peut \u00eatre interpr\u00e9t\u00e9 aussi bien comme \u00ab\u202f{1}\u202f\u00bb ou \u00ab\u202f{2}\u202f\u00bb dans le contexte des donn\u00e9es de \u00ab\u202f{0}\u202f\u00bb. +CanNotGetCommonMetadata_2 = Ne peut pas obtenir les m\u00e9ta-donn\u00e9es communes aux fichiers \u00ab\u202f{0}\u202f\u00bb. Raison\u2008: {1} CanNotReadCRS_WKT_1 = Ne peut pas lire le syst\u00e8me de r\u00e9f\u00e9rence spatial dans le \u00ab\u202fWell Known Text\u202f\u00bb (WKT) de \u00ab\u202f{0}\u202f\u00bb. CanNotReadDirectory_1 = Ne peut pas lire le r\u00e9pertoire \u00ab\u202f{0}\u202f\u00bb. CanNotReadFile_2 = Ne peut pas lire \u00ab\u202f{1}\u202f\u00bb comme un fichier au format {0}.
