This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new d27acc8 Clarify which `LogRecord` properties are initialised when an
error is reported.
d27acc8 is described below
commit d27acc8bc457a664fd33e547d74585082e0f4caf
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Thu Feb 4 12:54:08 2021 +0100
Clarify which `LogRecord` properties are initialised when an error is
reported.
---
.../java/org/apache/sis/image/AnnotatedImage.java | 73 ++++++++++---------
.../java/org/apache/sis/image/ErrorAction.java | 1 +
.../java/org/apache/sis/image/ErrorHandler.java | 82 +++++++++++++++++-----
.../sis/internal/coverage/j2d/TileOpExecutor.java | 32 +++++++--
4 files changed, 134 insertions(+), 54 deletions(-)
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
b/core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
index 7d38278..8bbae85 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
@@ -326,39 +326,39 @@ abstract class AnnotatedImage extends ImageAdapter {
*/
final Object key = getCacheKey(property);
value = cache.peek(key);
- if (value == null) try {
+ if (value == null) {
boolean success = false;
final Cache.Handler<Object> handler = cache.lock(key);
try {
value = handler.peek();
- if (value == null) {
+ if (value == null) try {
value = computeProperty();
if (value == null) value = NULL;
success = (errors == null);
+ } catch (Exception e) {
+ /*
+ * Stores the given exception in a log record. We use
a log record in order to initialize
+ * the timestamp and thread ID to the values they had
at the time the first error occurred.
+ */
+ if (failOnException) {
+ throw (ImagingOpException) new ImagingOpException(
+ Errors.format(Errors.Keys.CanNotCompute_1,
property)).initCause(e);
+ }
+ synchronized (this) {
+ ErrorHandler.Report report = errors;
+ final boolean create = (report == null);
+ if (create) {
+ report = new ErrorHandler.Report();
+ }
+ report.addPropertyError(e, property);
+ if (create) setError(report);
+ }
}
} finally {
handler.putAndUnlock(success ? value : null); //
Cache only if no error occurred.
}
if (value == NULL) value = null;
else value = cloneProperty(property, value);
- } catch (Exception e) {
- /*
- * Stores the given exception in a log record. We use a log
record in order to initialize
- * the timestamp and thread ID to the values they had at the
time the first error occurred.
- */
- if (failOnException) {
- throw (ImagingOpException) new ImagingOpException(
- Errors.format(Errors.Keys.CanNotCompute_1,
property)).initCause(e);
- }
- synchronized (this) {
- ErrorHandler.Report report = errors;
- final boolean create = (report == null);
- if (create) {
- report = new ErrorHandler.Report();
- }
- report.addPropertyError(e, property);
- if (create) setError(report);
- }
}
} else if (isErrorProperty(property, name)) {
value = errors;
@@ -371,7 +371,8 @@ abstract class AnnotatedImage extends ImageAdapter {
/**
* Invoked by {@link TileOpExecutor} if an error occurred during
calculation on a tiles.
* Can also be invoked by {@link #getProperty(String)} directly if the
error occurred
- * outside {@link TileOpExecutor}. This method shall be invoked at most
once.
+ * outside {@link TileOpExecutor}. This method should be invoked at most
once, unless
+ * the calculation is attempted again.
*
* @param report a description of the error that occurred.
*/
@@ -388,9 +389,16 @@ abstract class AnnotatedImage extends ImageAdapter {
}
/**
- * If an error occurred, logs the message. The log record is cleared by
this method call
+ * If an error occurred, logs the message with the specified class and
method as the source.
+ * The {@code classe} and {@code method} arguments overwrite the {@link
LogRecord#getSourceClassName()}
+ * and {@link LogRecord#getSourceMethodName()} values. The log record is
cleared by this method call
* and will no longer be reported, unless the property is recomputed.
*
+ * <h4>Context of use</h4>
+ * This method should be invoked only on images that are going to be
disposed after the caller extracted
+ * the computed property value. This method should not be invoked on image
accessible by the user,
+ * because clearing the error may be surprising.
+ *
* @param classe the class to report as the source of the logging
message.
* @param method the method to report as the source of the logging
message.
* @param handler where to send the log message.
@@ -399,16 +407,15 @@ abstract class AnnotatedImage extends ImageAdapter {
final ErrorHandler.Report report;
synchronized (this) {
report = errors;
- errors = null;
- }
- if (report != null) {
- final LogRecord record = report.getDescription();
- if (record != null) {
- record.setSourceClassName(classe.getCanonicalName());
- record.setSourceMethodName(method);
- handler.handle(report);
+ if (report == null || report.isEmpty()) {
+ return;
}
+ final LogRecord record = report.getDescription();
+ record.setSourceClassName(classe.getCanonicalName());
+ record.setSourceMethodName(method);
+ errors = null; // Make sure that no other thread will use
that `Report` instance.
}
+ handler.handle(report);
}
/**
@@ -431,12 +438,12 @@ abstract class AnnotatedImage extends ImageAdapter {
protected Object computeProperty() throws Exception {
if (parallel) {
final TileOpExecutor executor = new TileOpExecutor(source,
boundsOfInterest);
- if (!failOnException) {
- executor.setErrorHandler(this::setError);
- }
if (executor.isMultiTiled()) {
final Collector<? super Raster,?,?> collector = collector();
if (collector != null) {
+ if (!failOnException) {
+ executor.setErrorHandler(this::setError);
+ }
return executor.executeOnReadable(source, collector());
}
}
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/image/ErrorAction.java
b/core/sis-feature/src/main/java/org/apache/sis/image/ErrorAction.java
index b2abedd..d441f00 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/ErrorAction.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ErrorAction.java
@@ -62,6 +62,7 @@ enum ErrorAction implements ErrorHandler {
String logger = record.getLoggerName();
if (logger == null) {
logger = Modules.RASTER;
+ record.setLoggerName(logger);
}
Logging.getLogger(logger).log(record);
} else {
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/image/ErrorHandler.java
b/core/sis-feature/src/main/java/org/apache/sis/image/ErrorHandler.java
index ba20674..0c940b6 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/ErrorHandler.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ErrorHandler.java
@@ -19,6 +19,7 @@ package org.apache.sis.image;
import java.awt.Point;
import java.util.Arrays;
import java.util.Locale;
+import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.awt.image.ImagingOpException;
@@ -99,13 +100,48 @@ public interface ErrorHandler {
}
/**
+ * Returns {@code true} if the given exceptions are of the same class,
have the same message
+ * and the same stack trace. The cause and the suppressed exceptions
are ignored.
+ */
+ private static boolean equals(final Throwable t1, final Throwable t2) {
+ return t1.getClass() == t2.getClass()
+ && Objects.equals(t1.getMessage(), t2.getMessage())
+ && Arrays.equals(t1.getStackTrace(), t2.getStackTrace());
+ }
+
+ /**
+ * Adds the {@code more} exception to the list if suppressed
exceptions if not already present.
+ */
+ private static void addSuppressed(final Throwable error, final
Throwable more) {
+ if (equals(error, more)) return;
+ for (final Throwable s : error.getSuppressed()) {
+ if (equals(s, more)) return;
+ }
+ error.addSuppressed(more);
+ }
+
+ /**
* Reports an error that occurred while computing an image property.
* This method can be invoked many times on the same {@code Report}
instance.
*
- * @param error the error that occurred.
- * @param property name of the property which was computed, or {@code
null} if none.
+ * <h4>Logging information</h4>
+ * {@code Report} creates a {@link LogRecord} the first time that an
{@code add(…)} method is invoked.
+ * The record will have its
+ * {@linkplain LogRecord#getLevel() level},
+ * {@linkplain LogRecord#getMessage() message} and
+ * {@linkplain LogRecord#getThrown() exception} properties
initialized. But the
+ * {@linkplain LogRecord#getSourceClassName() source class name},
+ * {@linkplain LogRecord#getSourceMethodName() source method name} and
+ * {@linkplain LogRecord#getLoggerName() logger name} may be undefined;
+ * they may need to be completed by the caller.
+ *
+ * @param error the error that occurred.
+ * @param property name of the property which was computed, or
{@code null} if none.
+ * @return {@code true} if this is the first time that an error is
reported
+ * (in which case a {@link LogRecord} instance has been
created),
+ * or {@code false} if a {@link LogRecord} already exists.
*/
- public void addPropertyError(final Throwable error, final String
property) {
+ public boolean addPropertyError(final Throwable error, final String
property) {
ArgumentChecks.ensureNonNull("error", error);
if (description == null) {
if (property != null) {
@@ -115,8 +151,10 @@ public interface ErrorHandler {
description = new LogRecord(Level.WARNING,
error.toString());
}
description.setThrown(error);
+ return true;
} else {
- description.getThrown().addSuppressed(error);
+ addSuppressed(description.getThrown(), error);
+ return false;
}
}
@@ -124,11 +162,25 @@ public interface ErrorHandler {
* Reports an error that occurred while computing an image tile.
* This method can be invoked many times on the same {@code Report}
instance.
*
- * @param error the error that occurred.
- * @param tx column index of the tile where the error occurred.
- * @param ty row index of the tile where the error occurred.
+ * <h4>Logging information</h4>
+ * {@code Report} creates a {@link LogRecord} the first time that an
{@code add(…)} method is invoked.
+ * The record will have its
+ * {@linkplain LogRecord#getLevel() level},
+ * {@linkplain LogRecord#getMessage() message} and
+ * {@linkplain LogRecord#getThrown() exception} properties
initialized. But the
+ * {@linkplain LogRecord#getSourceClassName() source class name},
+ * {@linkplain LogRecord#getSourceMethodName() source method name} and
+ * {@linkplain LogRecord#getLoggerName() logger name} may be undefined;
+ * they may need to be completed by the caller.
+ *
+ * @param error the error that occurred.
+ * @param tx column index of the tile where the error occurred.
+ * @param ty row index of the tile where the error occurred.
+ * @return {@code true} if this is the first time that an error is
reported
+ * (in which case a {@link LogRecord} instance has been
created),
+ * or {@code false} if a {@link LogRecord} already exists.
*/
- public void addTileError(final Throwable error, final int tx, final
int ty) {
+ public boolean addTileError(final Throwable error, final int tx, final
int ty) {
ArgumentChecks.ensureNonNull("error", error);
if (indices == null) {
indices = new int[8];
@@ -141,8 +193,10 @@ public interface ErrorHandler {
description = Resources.forLocale(null)
.getLogRecord(Level.WARNING,
Resources.Keys.CanNotProcessTile_2, tx, ty);
description.setThrown(error);
+ return true;
} else {
- description.getThrown().addSuppressed(error);
+ addSuppressed(description.getThrown(), error);
+ return false;
}
}
@@ -160,14 +214,8 @@ public interface ErrorHandler {
}
/**
- * Returns a description of errors as a log record. The exception can
be obtained by
- * {@link LogRecord#getThrown()}. In addition the {@code LogRecord}
has the
- * {@linkplain LogRecord#getLevel() level} and
- * {@linkplain LogRecord#getMessage() message} properties set. But the
- * {@linkplain LogRecord#getSourceClassName() source class name},
- * {@linkplain LogRecord#getSourceMethodName() source method name} and
- * {@linkplain LogRecord#getLoggerName() logger name} may be undefined;
- * they should be set by the {@link ErrorHandler}.
+ * Returns a description of errors as a log record.
+ * The exception can be obtained by {@link LogRecord#getThrown()}.
* The return value is never null unless this report {@linkplain
#isEmpty() is empty}.
*
* @return errors description, or {@code null} if this report is empty.
diff --git
a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
index bed6e80..c81ca58 100644
---
a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
+++
b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
@@ -81,8 +81,10 @@ public class TileOpExecutor {
/**
* Where to report exceptions, or {@link ErrorHandler#THROW} for throwing
them.
- * In current implementation this is used only during parallel computation.
- * A future version may need to use it for sequential computations as well
for consistency.
+ * If at least one error occurred, then this handler will receive the
{@link Cursor#errors} report
+ * after all computation {@linkplain Cursor#finish finished}.
+ *
+ * @see #setErrorHandler(ErrorHandler)
*/
private ErrorHandler errorHandler;
@@ -116,6 +118,17 @@ public class TileOpExecutor {
/**
* Sets the handler where to report exceptions.
+ * The exception can be obtained by {@link LogRecord#getThrown()}
+ * on the value returned by {@link ErrorHandler.Report#getDescription()}.
+ * In addition the {@code LogRecord} will have the
+ * {@linkplain LogRecord#getLevel() level} and
+ * {@linkplain LogRecord#getMessage() message} properties set. But the
+ * {@linkplain LogRecord#getSourceClassName() source class name},
+ * {@linkplain LogRecord#getSourceMethodName() source method name} and
+ * {@linkplain LogRecord#getLoggerName() logger name} will be undefined;
+ * they should be set by the given {@link ErrorHandler}.
+ *
+ * <h4>Limitation</h4>
* In current implementation this is used only during parallel computation.
* A future version may need to use it for sequential computations as well
for consistency.
*
@@ -537,7 +550,18 @@ public class TileOpExecutor {
/**
* The errors that occurred while computing a tile.
- * Will be ignored if {@linkplain ErrorHandler.Report#isEmpty() empty}.
+ * If this report {@linkplain ErrorHandler.Report#isEmpty() is empty},
+ * then it will be ignored. Otherwise it contains a log record with
+ * {@linkplain LogRecord#getLevel() level},
+ * {@linkplain LogRecord#getMessage() message} and
+ * {@linkplain LogRecord#getThrown() exception} properties set. But the
+ * {@linkplain LogRecord#getSourceClassName() source class name},
+ * {@linkplain LogRecord#getSourceMethodName() source method name} and
+ * {@linkplain LogRecord#getLoggerName() logger name} will be
undefined.
+ *
+ * <p>If the report is non-empty, it will be given to the
+ * {@linkplain TileOpExecutor#errorHandler error handler}
+ * after all computation {@linkplain #finish finished}.</p>
*
* @see #stopOnError
* @see #recordError(Worker, Throwable)
@@ -871,7 +895,7 @@ public class TileOpExecutor {
static <A,R> R execute(final TileOpExecutor executor, final
RenderedImage source,
final Collector<? super Raster, A, R> collector, final
ErrorHandler errorHandler)
{
- final Cursor<RenderedImage,A> cursor = executor.new
Cursor<>(source, collector, errorHandler == null);
+ final Cursor<RenderedImage,A> cursor = executor.new
Cursor<>(source, collector, errorHandler == ErrorHandler.THROW);
final Future<?>[] workers = new Future<?>[cursor.getNumWorkers()];
for (int i=0; i<workers.length; i++) {
workers[i] = CommonExecutor.instance().submit(new
ReadWork<>(cursor, collector));
