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
commit ea5179e8dd16e63096b5b7256a804fd057594a87 Author: Martin Desruisseaux <[email protected]> AuthorDate: Fri Sep 16 18:23:00 2022 +0200 Partial fix of logging records that were lost: * In components of folder store (they need to declare their parent). * In JavaFX log listeners (need to take in account nested listeners). It does not fix all losts. We still have logging records lost when ConcatenatedGridCoverage read data from a resource. --- .../org/apache/sis/gui/dataset/ResourceItem.java | 1 + .../org/apache/sis/internal/gui/LogHandler.java | 51 ++++++++++++++++++++-- .../apache/sis/internal/netcdf/RasterResource.java | 8 ++++ .../apache/sis/internal/storage/folder/Store.java | 21 ++------- .../sis/internal/storage/io/InternalOptionKey.java | 3 ++ .../java/org/apache/sis/storage/DataOptionKey.java | 13 +++++- .../java/org/apache/sis/storage/DataStore.java | 4 +- .../apache/sis/storage/event/StoreListeners.java | 27 ++++++++++-- 8 files changed, 100 insertions(+), 28 deletions(-) diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceItem.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceItem.java index f0dd403600..b9e8753315 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceItem.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/dataset/ResourceItem.java @@ -397,6 +397,7 @@ final class ResourceItem extends TreeItem<Resource> { } // More cases may be added in the future. } + LogHandler.redirect(result, resource); final ResourceItem item = new ResourceItem(result); item.label = DataStoreOpener.findLabel(resource, locale, false); item.isLoading = false; diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/LogHandler.java b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/LogHandler.java index a6e7c2f4af..a1f7e174c2 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/LogHandler.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/LogHandler.java @@ -16,6 +16,8 @@ */ package org.apache.sis.internal.gui; +import java.util.List; +import java.util.ArrayList; import java.util.TreeMap; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentMap; @@ -40,7 +42,7 @@ import org.apache.sis.util.CharSequences; * This class maintains both a global (system) list and a list of log records specific to each resource. * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.3 * @since 1.1 * @module */ @@ -90,12 +92,37 @@ public final class LogHandler extends Handler implements StoreListener<WarningEv */ private TreeItem<String> loggers; + /** + * Previous values of {@link #inProgress} in case of nested calls to {@link #loadingStart(Resource)}. + */ + private final List<Destination> nested; + /** * Creates a new list of records. */ Destination() { queue = FXCollections.observableArrayList(); records = FXCollections.unmodifiableObservableList(queue); + nested = new ArrayList<>(3); + } + + /** + * Invoked when a nested call to {@link #loadingStart(Resource)} is detected. + * + * @param newer the new destination of log records. + * @return {@code newer}. + */ + final Destination startNested(final Destination newer) { + newer.nested.add(this); + return newer; + } + + /** + * Invoked for potentially nested call to {@link #loadingStop(Long)}. + */ + final Destination stopNested() { + final int n = nested.size(); + return (n != 0) ? nested.remove(n-1) : null; } /** @@ -217,6 +244,24 @@ public final class LogHandler extends Handler implements StoreListener<WarningEv } } + /** + * Redirects log records from the given source to the given target. + * This is used when the source is a view derived from the target. + * Invoking this method causes the logs from the two resources to go into a single list of log records, + * which can be viewed from the "Log" pane of any of those two resources. + * + * @param source the source of logs, typically a view derived from {@code target}. + * @param target the resource where to add logs, typically the original resource + * for which a window is provided. + */ + public static void redirect(final Resource source, final Resource target) { + if (source != target && source != null && target != null) { + synchronized (INSTANCE.resourceLogs) { + INSTANCE.resourceLogs.putIfAbsent(target, getRecords(source)); + } + } + } + /** * Notifies this {@code LogHandler} that an operation is about to start on the given resource. * Call to this method must be followed by call to {@link #loadingStop(Long)} in a {@code finally} block. @@ -227,7 +272,7 @@ public final class LogHandler extends Handler implements StoreListener<WarningEv public static Long loadingStart(final Resource source) { if (source == null) return null; final Long id = Thread.currentThread().getId(); - INSTANCE.inProgress.put(id, INSTANCE.getRecordsNonNull(source)); + INSTANCE.inProgress.merge(id, getRecords(source), Destination::startNested); return id; } @@ -239,7 +284,7 @@ public final class LogHandler extends Handler implements StoreListener<WarningEv */ public static void loadingStop(final Long id) { if (id != null) { - INSTANCE.inProgress.remove(id); + INSTANCE.inProgress.computeIfPresent(id, (k,v) -> v.stopNested()); } } diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/RasterResource.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/RasterResource.java index cf8b4e26fd..7250fa9369 100644 --- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/RasterResource.java +++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/RasterResource.java @@ -774,4 +774,12 @@ public final class RasterResource extends AbstractGridCoverageResource implement */ return lock; } + + /** + * Returns a string representation of this resource for debuging purposes. + */ + @Override + public String toString() { + return Strings.toString(getClass(), "identifier", identifier); + } } diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java index 6433b929dd..16a8b4aa5c 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/folder/Store.java @@ -42,12 +42,14 @@ import org.opengis.parameter.ParameterValueGroup; import org.apache.sis.setup.OptionKey; import org.apache.sis.storage.Resource; import org.apache.sis.storage.Aggregate; +import org.apache.sis.storage.DataOptionKey; import org.apache.sis.storage.DataStore; import org.apache.sis.storage.DataStores; import org.apache.sis.storage.DataStoreProvider; import org.apache.sis.storage.StorageConnector; import org.apache.sis.storage.DataStoreException; import org.apache.sis.storage.UnsupportedStorageException; +import org.apache.sis.storage.aggregate.CoverageAggregator; import org.apache.sis.util.collection.BackingStoreException; import org.apache.sis.internal.util.UnmodifiableArrayList; import org.apache.sis.internal.system.DefaultFactories; @@ -55,10 +57,6 @@ import org.apache.sis.internal.storage.MetadataBuilder; import org.apache.sis.internal.storage.StoreUtilities; import org.apache.sis.internal.storage.StoreResource; import org.apache.sis.internal.storage.Resources; -import org.apache.sis.storage.event.StoreEvent; -import org.apache.sis.storage.event.StoreListener; -import org.apache.sis.storage.event.WarningEvent; -import org.apache.sis.storage.aggregate.CoverageAggregator; /** @@ -187,7 +185,6 @@ class Store extends DataStore implements StoreResource, UnstructuredAggregate, D children = new ConcurrentHashMap<>(); children.put(path.toRealPath(), this); componentProvider = format; - listeners.useReadOnlyEvents(); } /** @@ -331,6 +328,7 @@ class Store extends DataStore implements StoreResource, UnstructuredAggregate, D connector.setOption(OptionKey.LOCALE, locale); connector.setOption(OptionKey.TIMEZONE, timezone); connector.setOption(OptionKey.ENCODING, encoding); + connector.setOption(DataOptionKey.PARENT_LISTENERS, listeners); try { if (componentProvider == null) { next = DataStores.open(connector); // May throw UnsupportedStorageException. @@ -438,19 +436,6 @@ class Store extends DataStore implements StoreResource, UnstructuredAggregate, D return messages().getString(key, value); } - /** - * Registers a listener to notify when the specified kind of event occurs in this data store. - * The current implementation of this data store can emit only {@link WarningEvent}s; - * any listener specified for another kind of events will be ignored. - */ - @Override - public <T extends StoreEvent> void addListener(Class<T> eventType, StoreListener<? super T> listener) { - // If an argument is null, we let the parent class throws (indirectly) NullArgumentException. - if (listener == null || eventType == null || eventType.isAssignableFrom(WarningEvent.class)) { - super.addListener(eventType, listener); - } - } - /** * Closes all children resources. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InternalOptionKey.java b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InternalOptionKey.java index 6789d214a7..83c6627beb 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InternalOptionKey.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/internal/storage/io/InternalOptionKey.java @@ -49,6 +49,9 @@ public final class InternalOptionKey<T> extends OptionKey<T> { /** * Creates a new key of the given name. + * + * @param name the key name. + * @param type the type of values. */ public InternalOptionKey(final String name, final Class<T> type) { super(name, type); diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataOptionKey.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataOptionKey.java index 208f62c78b..80ef28b1ce 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataOptionKey.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataOptionKey.java @@ -17,6 +17,7 @@ package org.apache.sis.storage; import org.apache.sis.setup.OptionKey; +import org.apache.sis.storage.event.StoreListeners; import org.apache.sis.feature.FoliationRepresentation; import org.opengis.referencing.crs.CoordinateReferenceSystem; @@ -27,7 +28,7 @@ import org.opengis.referencing.crs.CoordinateReferenceSystem; * or other kinds of structure that are specific to some data formats. * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.3 * * @param <T> the type of option values. * @@ -60,6 +61,16 @@ public final class DataOptionKey<T> extends OptionKey<T> { public static final OptionKey<FoliationRepresentation> FOLIATION_REPRESENTATION = new DataOptionKey<>("FOLIATION_REPRESENTATION", FoliationRepresentation.class); + /** + * The listeners to declare as the parent of the data store listeners. + * This option can be used when the {@link DataStore} to open is itself + * a child of an {@link Aggregate}. + * + * @since 1.3 + */ + public static final OptionKey<StoreListeners> PARENT_LISTENERS = + new DataOptionKey<>("PARENT_LISTENERS", StoreListeners.class); + /** * Creates a new key of the given name. */ diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java index 5b8f2e8801..5703e735e2 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/DataStore.java @@ -66,7 +66,7 @@ import org.apache.sis.storage.event.StoreListeners; * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) - * @version 1.1 + * @version 1.3 * * @see DataStores#open(Object) * @@ -133,7 +133,7 @@ public abstract class DataStore implements Resource, Localized, AutoCloseable { this.provider = provider; this.name = connector.getStorageName(); this.locale = Locale.getDefault(Locale.Category.DISPLAY); - this.listeners = new StoreListeners(null, this); + this.listeners = new StoreListeners(connector.getOption(DataOptionKey.PARENT_LISTENERS), this); /* * Above locale is NOT OptionKey.LOCALE because we are not talking about the same locale. * The one in this DataStore is for warning and exception messages, not for parsing data. diff --git a/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java b/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java index 91bb6d4f76..e149b7228e 100644 --- a/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java +++ b/storage/sis-storage/src/main/java/org/apache/sis/storage/event/StoreListeners.java @@ -28,6 +28,7 @@ import java.util.logging.LogRecord; import java.util.logging.Filter; import java.util.concurrent.ExecutionException; import java.lang.reflect.Method; +import org.apache.sis.util.Classes; import org.apache.sis.util.ArraysExt; import org.apache.sis.util.Localized; import org.apache.sis.util.Exceptions; @@ -41,6 +42,7 @@ import org.apache.sis.internal.storage.Resources; import org.apache.sis.internal.storage.StoreResource; import org.apache.sis.internal.storage.StoreUtilities; import org.apache.sis.internal.util.CollectionsExt; +import org.apache.sis.internal.util.Strings; import org.apache.sis.storage.DataStoreProvider; import org.apache.sis.storage.DataStore; import org.apache.sis.storage.Resource; @@ -236,10 +238,10 @@ public class StoreListeners implements Localized { } /** - * Returns {@code true} if this element has at least one listener. + * Returns the number of listeners. */ - final boolean hasListeners() { - return listeners != null; + final int count() { + return (listeners != null) ? listeners.length : 0; } /** @@ -865,7 +867,7 @@ public class StoreListeners implements Localized { do { for (ForType<?> e = m.listeners; e != null; e = e.next) { if (eventType.isAssignableFrom(e.type)) { - if (e.hasListeners()) { + if (e.count() != 0) { return true; } } @@ -982,4 +984,21 @@ public class StoreListeners implements Localized { * become no-op) and the objects are probably going to be garbage collected soon anyway. */ } + + /** + * Returns a string representation for debugging purposes. + * + * @return a debug string. + */ + @Override + public String toString() { + int count = 0; + for (ForType<?> e = listeners; e != null; e = e.next) { + count += e.count(); + } + return Strings.toString(getClass(), + "parent", (parent != null) ? Classes.getShortClassName(parent.source) : null, + "source", Classes.getShortClassName(source), + "count", count); + } }
