From 435f7fd493095431e1c75828b86d56f555949edb Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 18:47:30 +0200 Subject: [PATCH 01/39] refactor(export): modularize ExportService by introducing ExporterRegistryBean #11405 - Moved exporter management logic into a dedicated `ExporterRegistryBean` singleton for improved modularity and maintainability. - Simplified `ExportService` to delegate exporter logic to the new registry. --- .../iq/dataverse/export/ExportService.java | 79 +------- .../export/service/ExporterRegistryBean.java | 176 ++++++++++++++++++ 2 files changed, 181 insertions(+), 74 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java diff --git a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java index 1a888610a9e..37d5fae357a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java @@ -9,11 +9,11 @@ import static edu.harvard.iq.dataverse.dataaccess.DataAccess.getStorageIO; import edu.harvard.iq.dataverse.dataaccess.DataAccessOption; import edu.harvard.iq.dataverse.dataaccess.StorageIO; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import io.gdcc.spi.export.ExportException; import io.gdcc.spi.export.Exporter; import io.gdcc.spi.export.XMLExporter; -import edu.harvard.iq.dataverse.settings.JvmSettings; -import edu.harvard.iq.dataverse.util.BundleUtil; +import jakarta.ejb.EJB; import java.io.BufferedReader; import java.io.File; @@ -22,8 +22,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; -import java.net.URL; -import java.net.URLClassLoader; import java.nio.channels.Channel; import java.nio.channels.Channels; import java.nio.channels.WritableByteChannel; @@ -42,7 +40,6 @@ import java.util.Map; import java.util.Optional; import java.util.ServiceConfigurationError; -import java.util.ServiceLoader; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -59,77 +56,11 @@ */ public class ExportService { - private static ExportService service; - private ServiceLoader loader; - private Map exporterMap = new HashMap<>(); - private static final Logger logger = Logger.getLogger(ExportService.class.getCanonicalName()); - - private ExportService() { - /* - * Step 1 - find the EXPORTERS dir and add all jar files there to a class loader - */ - List jarUrls = new ArrayList<>(); - Optional exportPathSetting = JvmSettings.EXPORTERS_DIRECTORY.lookupOptional(String.class); - if (exportPathSetting.isPresent()) { - Path exporterDir = Paths.get(exportPathSetting.get()); - // Get all JAR files from the configured directory - try (DirectoryStream stream = Files.newDirectoryStream(exporterDir, "*.jar")) { - // Using the foreach loop here to enable catching the URI/URL exceptions - for (Path path : stream) { - logger.log(Level.FINE, "Adding {0}", path.toUri().toURL()); - // This is the syntax required to indicate a jar file from which classes should - // be loaded (versus a class file). - jarUrls.add(new URL("jar:" + path.toUri().toURL() + "!/")); - } - } catch (IOException e) { - logger.warning("Problem accessing external Exporters: " + e.getLocalizedMessage()); - } - } - URLClassLoader cl = URLClassLoader.newInstance(jarUrls.toArray(new URL[0]), this.getClass().getClassLoader()); - - /* - * Step 2 - load all Exporters that can be found, using the jars as additional - * sources - */ - loader = ServiceLoader.load(Exporter.class, cl); - /* - * Step 3 - Fill exporterMap with providerName as the key, allow external - * exporters to replace internal ones for the same providerName. FWIW: From the - * logging it appears that ServiceLoader returns classes in ~ alphabetical order - * rather than by class loader, so internal classes handling a given - * providerName may be processed before or after external ones. - */ - loader.forEach(exp -> { - String formatName = exp.getFormatName(); - // If no entry for this providerName yet or if it is an external exporter - if (!exporterMap.containsKey(formatName) || exp.getClass().getClassLoader().equals(cl)) { - exporterMap.put(formatName, exp); - } - logger.log(Level.FINE, "SL: " + exp.getFormatName() + " from " + exp.getClass().getCanonicalName() - + " and classloader: " + exp.getClass().getClassLoader().getClass().getCanonicalName()); - }); - } - - public static synchronized ExportService getInstance() { - if (service == null) { - service = new ExportService(); - } - return service; - } - - public List getExportersLabels() { - List retList = new ArrayList<>(); - - exporterMap.values().forEach(exp -> { - String[] temp = new String[2]; - temp[0] = exp.getDisplayName(BundleUtil.getCurrentLocale()); - temp[1] = exp.getFormatName(); - retList.add(temp); - }); - return retList; - } + @EJB + ExporterRegistryBean exporterRegistry; + public InputStream getExport(DatasetVersion datasetVersion, String formatName) throws ExportException, IOException { Dataset dataset = datasetVersion.getDataset(); diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java new file mode 100644 index 00000000000..66eac0039af --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -0,0 +1,176 @@ +package edu.harvard.iq.dataverse.export.service; + +import edu.harvard.iq.dataverse.settings.JvmSettings; +import edu.harvard.iq.dataverse.util.BundleUtil; +import io.gdcc.spi.export.Exporter; +import jakarta.annotation.PostConstruct; +import jakarta.annotation.PreDestroy; +import jakarta.ejb.Lock; +import jakarta.ejb.LockType; +import jakarta.ejb.Singleton; +import jakarta.ejb.Startup; + +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; +import java.nio.file.DirectoryStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.ServiceLoader; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * ExporterRegistry is responsible for managing the registration, retrieval, and lifecycle of {@code Exporter}s. + * It dynamically loads exporters from external JAR files and provides access to those exporters via their format names. + *

+ * This class is designed as a Jakarta EJB Singleton and is initialized at application startup. + * It uses a non-modifiable {@link Map} internally to store exporters under their format name, ensuring the state of + * the map is always consistent and thread-safe. + *

+ * Key responsibilities: + *

    + *
  • Locates and loads exporter JAR files from a specified directory.
  • + *
  • Use {@code ServiceLoader} to discover and register {@code Exporter} implementations dynamically.
  • + *
  • Allows external exporters to replace internal ones for the same format name.
  • + *
  • Provides thread-safe access to registered exporters and their metadata.
  • + *
+ * @implNote

Note on Concurrency: EJB singletons use container-managed concurrency by default, where every business + * method implicitly runs under an exclusive {@code @Lock(LockType.WRITE)}, meaning only one caller at + * a time may use the bean. Since this registry is populated once in and is effectively immutable afterwards, + * that exclusivity is unnecessary.

+ *

The class-level {@code @Lock(LockType.READ)} instead allows any number of callers to read from the + * registry concurrently, avoiding an application-wide bottleneck on exporter lookups. If a method that + * mutates the registry is ever added (e.g. a reload operation), it must be annotated with + * {@code @Lock(LockType.WRITE)} to regain exclusive access for that method.

+ */ +@Singleton +@Startup +@Lock(LockType.READ) +public class ExporterRegistryBean { + + /** + * Represents a set of labels associated with an exporter. + */ + public record Labels( + String localizedDisplayName, + String formatName + ) {} + + private static final Logger logger = Logger.getLogger(ExporterRegistryBean.class.getCanonicalName()); + + // When the class is initialized, the exporter map is an empty, non-modifiable map (key = format name). + // Once the exporters have been located and loaded, the map is replaced, fully loaded, still unmodifiable. + // No half-initialized state is exposable this way. Future optimizations may use @Lock on it, too, for example, + // when implementing a reload mechanism. + private Map exporters = Map.of(); + // Caching the classloader used to load plugin JAR files, keeping it open, will allow reuse for reloads + // or loading more resources from plugin JARs. May be dropped later if not necessary. + private URLClassLoader exporterClassLoader; + + /** + * Retrieves an exporter associated with the specified format name. + * + * @param formatName the name of the format for which to retrieve the exporter + * @return an {@code Optional} containing the exporter if found, or + * an empty {@code Optional} if no exporter is associated with the given format name + */ + public Optional get(String formatName) { + return Optional.ofNullable(exporters.get(formatName)); + } + + /** + * Retrieves a list of all registered exporters in the system. + * @return an unmodifiable list of {@link Exporter} instances representing all the exporters currently available + */ + public List getAll() { + return List.copyOf(exporters.values()); + } + + /** + * Retrieves a list of {@link Labels} representing the exporters registered in the system. + * @return a list of {@code Labels} objects + */ + public List getLabels() { + return exporters.values().stream() + .map(exporter -> new Labels( + exporter.getDisplayName(BundleUtil.getCurrentLocale()), + exporter.getFormatName())) + .toList(); + } + + @PostConstruct + private void initialize() { + /* + * Step 1 - find the EXPORTERS dir and add all jar files there to a class loader + */ + List jarUrls = new ArrayList<>(); + Optional exportPathSetting = JvmSettings.EXPORTERS_DIRECTORY.lookupOptional(String.class); + if (exportPathSetting.isPresent()) { + Path exporterDir = Paths.get(exportPathSetting.get()); + // Get all JAR files from the configured directory + try (DirectoryStream stream = Files.newDirectoryStream(exporterDir, "*.jar")) { + // Using the foreach loop here to enable catching the URI/URL exceptions + for (Path path : stream) { + logger.log(Level.FINE, "Adding {0}", path.toUri().toURL()); + // This is the syntax required to indicate a jar file from which classes should + // be loaded (versus a class file). + jarUrls.add(new URL("jar:" + path.toUri().toURL() + "!/")); + } + } catch (IOException e) { + logger.warning("Problem accessing external Exporters: " + e.getLocalizedMessage()); + } + } + this.exporterClassLoader = URLClassLoader.newInstance(jarUrls.toArray(new URL[0]), this.getClass().getClassLoader()); + + /* + * Step 2 - load all Exporters that can be found, using the jars as additional sources + */ + ServiceLoader loader = ServiceLoader.load(Exporter.class, this.exporterClassLoader); + + /* + * Step 3 - Fill exporterMap with providerName as the key, allow external + * exporters to replace internal ones for the same providerName. FWIW: From the + * logging it appears that ServiceLoader returns classes in ~ alphabetical order + * rather than by class loader, so internal classes handling a given + * providerName may be processed before or after external ones. + */ + Map loadedExporters = new HashMap<>(); + loader.forEach(exp -> { + String formatName = exp.getFormatName(); + // If no entry for this providerName yet or if it is an external exporter + if (!exporters.containsKey(formatName) || exp.getClass().getClassLoader().equals(this.exporterClassLoader)) { + loadedExporters.put(formatName, exp); + } + logger.log( + Level.FINE, + "SL: {0} from {1} and classloader: {2}", + new Object[]{ + formatName, + exp.getClass().getCanonicalName(), + exp.getClass().getClassLoader().getClass().getCanonicalName() + }); + }); + this.exporters = loadedExporters; + + } + + @PreDestroy + private void tearDown() { + if (exporterClassLoader == null) { + return; + } + + try { + exporterClassLoader.close(); + } catch (IOException e) { + logger.log(Level.WARNING, "Could not close exporter classloader", e); + } + } +} From 5395ad3c69d49cda29856427e00b2ea3562e3481 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 18:50:10 +0200 Subject: [PATCH 02/39] refactor(export): make ExportService a @Stateless EJB bean - Enable injectingthe registry and other components - The export process itself is stateless. State is involved in potential write locks, the loaded plugins, etc. - A stateless coordinator bean scales better for multiple export requests coming in. --- .../java/edu/harvard/iq/dataverse/export/ExportService.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java index 37d5fae357a..c75163a46a2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java @@ -14,6 +14,7 @@ import io.gdcc.spi.export.Exporter; import io.gdcc.spi.export.XMLExporter; import jakarta.ejb.EJB; +import jakarta.ejb.Stateless; import java.io.BufferedReader; import java.io.File; @@ -50,10 +51,7 @@ import org.apache.commons.io.IOUtils; -/** - * - * @author skraffmi - */ +@Stateless public class ExportService { private static final Logger logger = Logger.getLogger(ExportService.class.getCanonicalName()); From 1c33930cf6e81f360a0b9e9dcf64d80e3f05c357 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 18:53:18 +0200 Subject: [PATCH 03/39] feat(export): introduce new ExportCache subsystem with cache key, invalidator, and storage abstraction #11405 The goal is removing the caching logic from the ExportService. At the same time, a distinct caching subsystem shall have policies about what gets cached, when it expires etc, all independent of a coordinating service like ExportService. This make cognitive loader smaller and allows extension without using more code branches. --- .../dataverse/export/service/ExportCache.java | 46 +++++++++++++++++++ .../service/ExportCacheInvalidator.java | 23 ++++++++++ .../export/service/ExportCacheKey.java | 21 +++++++++ 3 files changed, 90 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java new file mode 100644 index 00000000000..d4c2f2fe28e --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java @@ -0,0 +1,46 @@ +package edu.harvard.iq.dataverse.export.service; + +import edu.harvard.iq.dataverse.Dataset; +import io.gdcc.spi.export.ExportException; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.Optional; + +/** + * Storage abstraction for cached metadata exports. Implementations own all + * knowledge about where and under which names cached exports live; the export + * pipeline only ever deals in {@link ExportCacheKey}s and streams. + */ +public sealed interface ExportCache permits StorageIOCache { + + /** + * Looks up a cached export. + * @return the cached export stream, or empty if none is cached. Note: the caller is responsible for closing the stream. + * @throws IOException on actual storage failures (not on a cache miss) + */ + Optional read(ExportCacheKey key) throws IOException; + + /** + * Produces and stores an export. The {@code writer} callback receives the output stream to write to. + * Any implementations guarantee that a partially written export is never made visible under the cache key + * (i.e., a failed write leaves either the previous entry or no entry). + */ + void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException; + + /** Removes a cached export. Absence of the entry is not an error. */ + void evict(ExportCacheKey key) throws IOException; + + /** + * Removes all cached exports for a dataset, across all versions and formats, including legacy (pre-versioning) entries. + * Intended for publish/deaccession hooks and the admin "reexport" API. + */ + void evictAll(Dataset dataset) throws IOException; + + /** Callback that renders an export into the store-provided stream. */ + @FunctionalInterface + interface ExportStreamWriter { + void writeTo(OutputStream out) throws ExportException, IOException; + } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java new file mode 100644 index 00000000000..5208098285e --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java @@ -0,0 +1,23 @@ +package edu.harvard.iq.dataverse.export.service; + +import java.util.List; + +/** + * Represents an abstraction for determining whether a cached export needs to be invalidated and regenerated. + *

+ * This sealed interface is intended to enforce a controlled hierarchy of classes that implement the cache + * invalidation logic, ensuring behavior consistency across different implementations. If necessary, the contract + * may be altered to allow more dynamic discovery of invalidators. + */ +public sealed interface ExportCacheInvalidator permits FileEmbargoExpiryInvalidator { + + /** + * A collection of {@link ExportCacheInvalidator} instances. + * This list is intended to centralize all invalidation mechanisms for export cache entries. + * Any new implementations must be added here in addition to the "permits" on the interface seal. + */ + List invalidators = List.of(new FileEmbargoExpiryInvalidator()); + + /** Should a cached export for this key be discarded and regenerated? */ + boolean isStale(ExportCacheKey key); +} diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java new file mode 100644 index 00000000000..2b2f1265757 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java @@ -0,0 +1,21 @@ +package edu.harvard.iq.dataverse.export.service; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersion; + +/** + * This record encapsulates information related to the dataset, the version of the dataset, + * and the format name used for the export, enabling precise identification + * of cache entries for export operations. + */ +public record ExportCacheKey(Dataset dataset, DatasetVersion version, String formatName) { + + /** The one canonical, version-qualified aux tag. Always used to write. */ + public String auxTag() { + return "export_" + formatName + "_" + version.getFriendlyVersionNumber() + ".cached"; + } + + public boolean isLatestReleased() { + return version.equals(dataset.getReleasedVersion()); + } +} From 279db4071a46cea27b3c673dc7fafe167866d631 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 18:55:15 +0200 Subject: [PATCH 04/39] refactor(export): extract file embargo expiry logic from ExportService into FileEmbargoExpiryInvalidator #11405 --- .../iq/dataverse/export/ExportService.java | 54 --------------- .../service/FileEmbargoExpiryInvalidator.java | 68 +++++++++++++++++++ 2 files changed, 68 insertions(+), 54 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java diff --git a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java index c75163a46a2..2e3c90d8baa 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java @@ -90,60 +90,6 @@ public InputStream getExport(DatasetVersion datasetVersion, String formatName) t exportInputStream = getCachedExportFormat(dataset, formatName); } - // The DDI export is limited for restricted and actively embargoed files (no - // data/file description sections).and when an embargo ends, we need to refresh - // this export. - boolean clearCachedExport = false; - if (formatName.equals(DDIExporter.PROVIDER_NAME) && (exportInputStream != null)) { - // We want ddi and there was a cached version - LocalDate exportLocalDate = null; - Date lastExportDate = dataset.getLastExportTime(); - // if lastExportDate == null, assume it's not set because were exporting for the - // first time now (e.g. during publish) and therefore no changes are needed - if (lastExportDate != null) { - exportLocalDate = lastExportDate.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); - logger.fine("Last export date: " + exportLocalDate.toString()); - // Track which embargoes we've already checked - Set embargoIds = new HashSet(); - // Check for all files in the latest released version - for (FileMetadata fm : dataset.getLatestVersionForCopy().getFileMetadatas()) { - // ToDo? This loop is necessary because we have not stored the date when the - // next embargo in this datasetversion will end. If we knew that (another - // dataset/datasetversion column), we could make - // one check that nextembargoEnd exists and is after the last export and before - // now versus scanning through files until we potentially find such an embargo. - Embargo e = fm.getDataFile().getEmbargo(); - if (e != null) { - logger.fine("Datafile: " + fm.getDataFile().getId()); - logger.fine("Embargo end date: " + e.getFormattedDateAvailable()); - } - if (e != null && !embargoIds.contains(e.getId()) && e.getDateAvailable().isAfter(exportLocalDate) - && e.getDateAvailable().isBefore(LocalDate.now())) { - logger.fine("Request that the ddi export be cleared."); - // The file has been embargoed and the embargo ended after the last export and - // before the current date, so we need to remove the cached DDI export and make - // it refresh - clearCachedExport = true; - break; - } else if (e != null) { - logger.fine("adding embargo to checked list: " + e.getId()); - embargoIds.add(e.getId()); - } - } - } - if (clearCachedExport) { - try { - exportInputStream.close(); - clearCachedExport(dataset, formatName); - } catch (Exception ex) { - logger.warning("Failure deleting DDI export format for dataset id: " + dataset.getId() - + " after embargo expiration: " + ex.getLocalizedMessage()); - } finally { - exportInputStream = null; - } - } - } - if (exportInputStream != null) { return exportInputStream; } diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java b/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java new file mode 100644 index 00000000000..e9367f2b3ad --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java @@ -0,0 +1,68 @@ +package edu.harvard.iq.dataverse.export.service; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.Embargo; +import edu.harvard.iq.dataverse.FileMetadata; + +import java.time.LocalDate; +import java.time.ZoneId; +import java.util.Date; +import java.util.HashSet; +import java.util.Set; +import java.util.logging.Logger; + +/** + * The {@code FileEmbargoExpiryInvalidator} class implements the {@link ExportCacheInvalidator} interface to determine + * whether a cached export should be invalidated due to the expiration of an embargo on any file within a dataset. + * This invalidation ensures that stale cached exports do not persist beyond the embargo period. + *

+ * Note: This code was originally a part of {@code ExportService}, written mostly by qqmyers. + * Back there it was targeting DDI format only, but with pluggable exports, any format may export file metadata. + */ +public final class FileEmbargoExpiryInvalidator implements ExportCacheInvalidator { + + private static final Logger logger = Logger.getLogger(FileEmbargoExpiryInvalidator.class.getCanonicalName()); + + @Override + public boolean isStale(ExportCacheKey key) { + return isStaleDueToExpiredEmbargo(key.dataset()); + } + + /** + * Checks whether a cached export has been rendered stale because an embargo + * on one of the dataset's files ended after the last export ran. + */ + private boolean isStaleDueToExpiredEmbargo(Dataset dataset) { + Date lastExportDate = dataset.getLastExportTime(); + // if lastExportDate == null, assume it's not set because we're exporting for the + // first time now (e.g. during publish) and therefore no changes are needed + if (lastExportDate == null) { + return false; + } + LocalDate exportLocalDate = lastExportDate.toInstant().atZone(ZoneId.systemDefault()).toLocalDate(); + logger.fine("Last export date: " + exportLocalDate); + // Track which embargoes we've already checked + Set embargoIds = new HashSet<>(); + // Check for all files in the latest released version + for (FileMetadata fm : dataset.getLatestVersionForCopy().getFileMetadatas()) { + // ToDo? This loop is necessary because we have not stored the date when the + // next embargo in this datasetversion will end. If we knew that (another + // dataset/datasetversion column), we could make one check that nextembargoEnd + // exists and is after the last export and before now versus scanning through + // files until we potentially find such an embargo. + Embargo e = fm.getDataFile().getEmbargo(); + if (e == null || embargoIds.contains(e.getId())) { + continue; + } + logger.fine("Datafile: " + fm.getDataFile().getId() + ", embargo end date: " + e.getFormattedDateAvailable()); + if (e.getDateAvailable().isAfter(exportLocalDate) && e.getDateAvailable().isBefore(LocalDate.now(ZoneId.systemDefault()))) { + // The embargo ended after the last export and before the current date, + // so the cached export needs to be refreshed. + logger.fine("Request that the cached export be cleared."); + return true; + } + embargoIds.add(e.getId()); + } + return false; + } +} From 38cb8a9ab20aa4ef06562598cade3e879e11b740 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 19:04:29 +0200 Subject: [PATCH 05/39] refactor(export): move caching logic from ExportService to new StorageIOCache class #11405 - Reorganized export cache handling into a dedicated `StorageIOCache` service, improving modularity and reducing cognitive load in `ExportService`. - Streamlined caching operations with a unified approach across all storage drivers. - Deprecated legacy unversioned cache keys; introduced versioned aux tag schema for better cache qualification. - Enhanced write atomicity and cache eviction logic. - Remove stale code for size of exports --- .../iq/dataverse/export/ExportService.java | 157 +--------------- .../export/service/StorageIOCache.java | 175 ++++++++++++++++++ 2 files changed, 180 insertions(+), 152 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java diff --git a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java index 2e3c90d8baa..8dc6462d6f5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java @@ -2,54 +2,27 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetVersion; -import edu.harvard.iq.dataverse.Embargo; -import edu.harvard.iq.dataverse.FileMetadata; - -import edu.harvard.iq.dataverse.dataaccess.DataAccess; -import static edu.harvard.iq.dataverse.dataaccess.DataAccess.getStorageIO; -import edu.harvard.iq.dataverse.dataaccess.DataAccessOption; -import edu.harvard.iq.dataverse.dataaccess.StorageIO; import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import io.gdcc.spi.export.ExportException; import io.gdcc.spi.export.Exporter; import io.gdcc.spi.export.XMLExporter; import jakarta.ejb.EJB; import jakarta.ejb.Stateless; +import jakarta.ws.rs.core.MediaType; +import org.apache.commons.io.IOUtils; import java.io.BufferedReader; -import java.io.File; -import java.io.FileOutputStream; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.io.OutputStream; -import java.nio.channels.Channel; -import java.nio.channels.Channels; -import java.nio.channels.WritableByteChannel; -import java.nio.file.DirectoryStream; -import java.nio.file.Files; -import java.nio.file.Path; -import java.nio.file.Paths; import java.sql.Timestamp; -import java.time.LocalDate; -import java.time.ZoneId; -import java.util.ArrayList; import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; -import java.util.Map; -import java.util.Optional; import java.util.ServiceConfigurationError; -import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; -import jakarta.ws.rs.core.MediaType; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.FileInputStream; - -import org.apache.commons.io.IOUtils; @Stateless public class ExportService { @@ -305,127 +278,7 @@ public Exporter getExporter(String formatName) throws ExportException { } throw new ExportException("No such Exporter: " + formatName); } - - // This method runs the selected metadata exporter, caching the output - // in a file in the dataset directory / container based on its DOI: - private void cacheExport(Dataset dataset, InternalExportDataProvider dataProvider, String format, Exporter exporter) - throws ExportException { - - OutputStream outputStream = null; - try { - boolean tempFileUsed = false; - File tempFile = null; - StorageIO storageIO = null; - - // With some storage drivers, we can open a WritableChannel, or OutputStream - // to directly write the generated metadata export that we want to cache; - // Some drivers (like Swift) do not support that, and will give us an - // "operation not supported" exception. If that's the case, we'll have - // to save the output into a temp file, and then copy it over to the - // permanent storage using the IO "save" command: - try { - storageIO = DataAccess.getStorageIO(dataset); - Channel outputChannel = storageIO.openAuxChannel("export_" + format + ".cached", - DataAccessOption.WRITE_ACCESS); - outputStream = Channels.newOutputStream((WritableByteChannel) outputChannel); - } catch (IOException ioex) { - // A common case = an IOException in openAuxChannel which is not supported by S3 - // stores for WRITE_ACCESS - tempFileUsed = true; - tempFile = File.createTempFile("tempFileToExport", ".tmp"); - outputStream = new FileOutputStream(tempFile); - } - - try { - // Write the metadata export file to the outputStream, which may be the final - // location or a temp file - exporter.exportDataset(dataProvider, outputStream); - outputStream.flush(); - outputStream.close(); - if (tempFileUsed) { - logger.fine("Saving export_" + format + ".cached aux file from temp file: " - + Paths.get(tempFile.getAbsolutePath())); - storageIO.savePathAsAux(Paths.get(tempFile.getAbsolutePath()), "export_" + format + ".cached"); - boolean tempFileDeleted = tempFile.delete(); - logger.fine("tempFileDeleted: " + tempFileDeleted); - } - } catch (ExportException exex) { - /* - * This exception is from the particular exporter and may not affect other - * exporters (versus other exceptions in this method which are from the basic - * mechanism to create a file) So we'll catch it here and report so that loops - * over other exporters can continue. Todo: Might be better to create a new - * exception subtype and send it upward, but the callers currently just log and - * ignore beyond terminating any loop over exporters. - */ - logger.warning("Exception thrown while creating export_" + format + ".cached : " + exex.getMessage()); - } catch (IOException ioex) { - throw new ExportException("IO Exception thrown exporting as " + "export_" + format + ".cached"); - } - - } catch (IOException ioex) { - // This catches any problem creating a local temp file in the catch clause above - throw new ExportException("IO Exception thrown before exporting as " + "export_" + format + ".cached"); - } finally { - IOUtils.closeQuietly(outputStream); - } - - } - - private void clearCachedExport(Dataset dataset, String format) throws IOException { - try { - StorageIO storageIO = getStorageIO(dataset); - storageIO.deleteAuxObject("export_" + format + ".cached"); - - } catch (IOException ex) { - throw new IOException("IO Exception caught deleting export_" + format + ".cached"); - } - } - - // This method checks if the metadata has already been exported in this - // format and cached on disk. If it has, it'll open the file and retun - // the file input stream. If not, it'll return null. - private InputStream getCachedExportFormat(Dataset dataset, String formatName) throws ExportException, IOException { - - StorageIO dataAccess = null; - - try { - dataAccess = DataAccess.getStorageIO(dataset); - } catch (IOException ioex) { - throw new IOException("IO Exception thrown exporting as " + "export_" + formatName + ".cached", ioex); - } - - InputStream cachedExportInputStream = null; - - try { - cachedExportInputStream = dataAccess.getAuxFileAsInputStream("export_" + formatName + ".cached"); - return cachedExportInputStream; - } catch (IOException ioex) { - throw new IOException("IO Exception thrown exporting as " + "export_" + formatName + ".cached", ioex); - } - - } - - /* - * The below method, getCachedExportSize(), is not currently used. An exercise - * for the reader could be to refactor it if it's needed to be compatible with - * storage drivers other than local filesystem. Files.exists() would need to be - * discarded. -- L.A. 4.8 - */ -// public Long getCachedExportSize(Dataset dataset, String formatName) { -// try { -// if (dataset.getFileSystemDirectory() != null) { -// Path cachedMetadataFilePath = Paths.get(dataset.getFileSystemDirectory().toString(), "export_" + formatName + ".cached"); -// if (Files.exists(cachedMetadataFilePath)) { -// return cachedMetadataFilePath.toFile().length(); -// } -// } -// } catch (Exception ioex) { -// // don't do anything - we'll just return null -// } -// -// return null; -// } + public Boolean isXMLFormat(String provider) { Exporter e = exporterMap.get(provider); if (e != null) { diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java new file mode 100644 index 00000000000..9bae5ca5d82 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -0,0 +1,175 @@ +package edu.harvard.iq.dataverse.export.service; + +import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.dataaccess.DataAccess; +import edu.harvard.iq.dataverse.dataaccess.StorageIO; +import io.gdcc.spi.export.ExportException; +import jakarta.enterprise.context.ApplicationScoped; + +import java.io.BufferedOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; +import java.util.Optional; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * {@link ExportCache} backed by Dataverse's {@link StorageIO} layer, storing exports as auxiliary objects alongside the dataset. + *

+ * Naming Schema: The canonical "aux tag" is version-qualified ({@code export__.cached}, + * see {@link ExportCacheKey#auxTag()}) and is the only name ever written. + *

+ * The legacy, unqualified name ({@code export_.cached}) predates version qualification and only ever described + * the latest released version. It is therefore consulted as a read fallback exclusively for that version. + * It will be deleted alongside the canonical name on eviction, so a stale legacy entry can never resurrect an invalidated export. + *

+ * Write Atomicity: Exports are always rendered to a local temp file first. + * Then it gets persisted via {@link StorageIO#savePathAsAux(Path, String)} as an auxiliary dataset file. + *

+ * Note: This class replaces the former {@code ExportService.cacheExport()} method, mostly written by qqmyers. + * Instead of its "try openAuxChannel, fall back to temp file for S3/Swift" branching, there now is one code path for all drivers. + * Readers can never observe a half-written export under the cache key. The cost is one extra local write per export, + * which is negligible next to export generation itself. + *

+ * Note 2: This class is an application scoped CDI bean (single instance). The cache itself is stateless, + * and every operation operates on their own {@code StorageIO}. But: if we add a write lock later on to avoid race + * conditions during writes, we will require an instance wide single map to store these locks, which CDI gives us for free. + * In addition, one might use a Hazelcast-backed map to acquire multi-instance wide locks! + * And lastly, making this an injectable CDI bean makes mocking it in tests very easy. + */ +@ApplicationScoped +public final class StorageIOCache implements ExportCache { + + private static final Logger logger = Logger.getLogger(StorageIOCache.class.getCanonicalName()); + + private static final String TAG_PREFIX = "export_"; + private static final String TAG_SUFFIX = ".cached"; + + /** + * Reads an input stream associated with the given export cache key. + * + * @param key the export cache key containing dataset, format, and versioning information. + * @return an {@code Optional} containing the input stream if available, otherwise an empty {@code Optional}. + * @throws IOException if an I/O error occurs while attempting to read the data. + */ + @Override + public Optional read(ExportCacheKey key) throws IOException { + StorageIO storage = storageFor(key.dataset()); + + Optional versioned = tryRead(storage, key.auxTag()); + if (versioned.isPresent()) { + return versioned; + } + // Legacy fallback: pre-versioning cache entries carried no version identity and only ever described the + // latest released version. For any other version they are unattributable and must be ignored! + if (key.isLatestReleased()) { + return tryRead(storage, legacyLatestAuxTag(key.formatName())); + } + return Optional.empty(); + } + + /** + * Writes the export cache data to a temporary file and ensures it is properly persisted to the dataset's storage. + * Handles file cleanup to maintain system integrity. + * @param key The {@code ExportCacheKey} representing the metadata export about to be cached. + * @param writer The {@code ExportStreamWriter} functional interface implementation responsible for writing data + * to the output stream. This wraps the underlying exporter, writing the actual data format. + * @throws ExportException If an error occurs during the export process. + * @throws IOException If an I/O error occurs while creating, writing, or managing the temporary file. + */ + @Override + public void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException { + Path tempFile = Files.createTempFile("dataverse-export-", ".tmp"); + try { + try (OutputStream out = new BufferedOutputStream(Files.newOutputStream(tempFile))) { + writer.writeTo(out); + } + // Persist to storage only after the metadata export has been fully and successfully rendered. + // A failure above leaves the cache untouched. + storageFor(key.dataset()).savePathAsAux(tempFile, key.auxTag()); + logger.log(Level.FINE, key.version() + ": Cached export written: {0}", key.auxTag()); + } finally { + try { + Files.deleteIfExists(tempFile); + } catch (IOException e) { + // Warn, but do not fail if the temp file could not be deleted. (The main operation was a success) + logger.log(Level.WARNING, e, () -> key.version() + ": could not delete export temp file " + tempFile); + } + } + } + + @Override + public void evict(ExportCacheKey key) throws IOException { + StorageIO storage = storageFor(key.dataset()); + deleteQuietly(storage, key.auxTag()); + // Paired eviction: + // Without this, the next read would fall through to the stale legacy entry and resurrect what we just invalidated! + if (key.isLatestReleased()) { + deleteQuietly(storage, legacyLatestAuxTag(key.formatName())); + } + } + + @Override + public void evictAll(Dataset dataset) throws IOException { + StorageIO storage = storageFor(dataset); + List auxTags = storage.listAuxObjects(); + for (String tag : auxTags) { + if (tag.startsWith(TAG_PREFIX) && tag.endsWith(TAG_SUFFIX)) { + deleteQuietly(storage, tag); + } + } + } + + /** + * The pre-versioning aux tag, kept for reading and deleting existing caches only. + * + * @deprecated Never write under this name. + * Remove the fallback entirely once instances have had a release cycle to regenerate their caches. + * (Worst case on removal: one redundant re-export per dataset. Cache is fully derivable state). + */ + @Deprecated(forRemoval = true) + private static String legacyLatestAuxTag(String formatName) { + return TAG_PREFIX + formatName + TAG_SUFFIX; + } + + private static Optional tryRead(StorageIO storage, String auxTag) { + // Distinguish "not cached" (normal, frequent) from actual failures: only read if the aux object exists. + try { + if (!storage.isAuxObjectCached(auxTag)) { + return Optional.empty(); + } + } catch (IOException e) { + // Treat as a "cache miss" so the pipeline regenerates rather than failing over a cache IO issue. + logger.log(Level.FINE, e, () -> "Existence check failed for " + auxTag); + return Optional.empty(); + } + try { + return Optional.of(storage.getAuxFileAsInputStream(auxTag)); + } catch (IOException e) { + // Exists-then-vanished race, or a genuine storage problem. + // Treated as a "cache miss" so the pipeline regenerates rather than failing over a cache IO issue. + logger.log(Level.WARNING, e, () -> "Could not open cached export " + auxTag); + return Optional.empty(); + } + } + + private static void deleteQuietly(StorageIO storage, String auxTag) { + try { + storage.deleteAuxObject(auxTag); + } catch (IOException e) { + // Absence is the common case here and not an error. + // Real failures are logged but non-fatal, as the entry will be overwritten or ignored on the next pipeline run. + logger.log(Level.FINE, e, () -> "Could not delete aux object " + auxTag); + } + } + + // Extracted to static method to avoid repeating it in multiple places, allowing substituion + // and extension to a StorageProvider functional interface (which is mockable on its own). + private static StorageIO storageFor(Dataset dataset) throws IOException { + return DataAccess.getStorageIO(dataset); + } +} From 64700c4ac428f8d7551ec1e98232f9a60b81d833 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 19:29:25 +0200 Subject: [PATCH 06/39] refactor(export): remove legacy unversioned cache logic from StorageIOCache #11405 The legacy reading of cached exports is prone to produce bugs in production. When we rely on reading cached exports as prerequisites for other metadata formats, we might end up with stale data. Any export has no knowledge about whether and when an export of another format happened. We keep no provenance per format. Assuming there is a cached "latest" with the legacy file format, it would be read as a prerequisite format, but our invalidation mechanisms would not be able to tell if it's actually stale, because it was not yet re-exported. Any released version is immutable, thus if we rely in lookups on cached objects with the version present in the aux tag, we can be sure we get the latest data. --- .../export/service/ExportCacheKey.java | 4 --- .../export/service/StorageIOCache.java | 32 ++----------------- 2 files changed, 2 insertions(+), 34 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java index 2b2f1265757..9b6ddff0cc9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java @@ -14,8 +14,4 @@ public record ExportCacheKey(Dataset dataset, DatasetVersion version, String for public String auxTag() { return "export_" + formatName + "_" + version.getFriendlyVersionNumber() + ".cached"; } - - public boolean isLatestReleased() { - return version.equals(dataset.getReleasedVersion()); - } } diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java index 9bae5ca5d82..7ca5aaa33dc 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -59,17 +59,7 @@ public final class StorageIOCache implements ExportCache { @Override public Optional read(ExportCacheKey key) throws IOException { StorageIO storage = storageFor(key.dataset()); - - Optional versioned = tryRead(storage, key.auxTag()); - if (versioned.isPresent()) { - return versioned; - } - // Legacy fallback: pre-versioning cache entries carried no version identity and only ever described the - // latest released version. For any other version they are unattributable and must be ignored! - if (key.isLatestReleased()) { - return tryRead(storage, legacyLatestAuxTag(key.formatName())); - } - return Optional.empty(); + return tryRead(storage, key.auxTag()); } /** @@ -104,13 +94,7 @@ public void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportEx @Override public void evict(ExportCacheKey key) throws IOException { - StorageIO storage = storageFor(key.dataset()); - deleteQuietly(storage, key.auxTag()); - // Paired eviction: - // Without this, the next read would fall through to the stale legacy entry and resurrect what we just invalidated! - if (key.isLatestReleased()) { - deleteQuietly(storage, legacyLatestAuxTag(key.formatName())); - } + deleteQuietly(storageFor(key.dataset()), key.auxTag()); } @Override @@ -124,18 +108,6 @@ public void evictAll(Dataset dataset) throws IOException { } } - /** - * The pre-versioning aux tag, kept for reading and deleting existing caches only. - * - * @deprecated Never write under this name. - * Remove the fallback entirely once instances have had a release cycle to regenerate their caches. - * (Worst case on removal: one redundant re-export per dataset. Cache is fully derivable state). - */ - @Deprecated(forRemoval = true) - private static String legacyLatestAuxTag(String formatName) { - return TAG_PREFIX + formatName + TAG_SUFFIX; - } - private static Optional tryRead(StorageIO storage, String auxTag) { // Distinguish "not cached" (normal, frequent) from actual failures: only read if the aux object exists. try { From 234626f42ccdf37a83c14b5b2f40e09ede2b5ac1 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 19:50:58 +0200 Subject: [PATCH 07/39] feat(export): enhance ExportCacheKey with validation and convenience constructor - Added null and blank checks for dataset, version, and formatName to ensure robust usage. - Introduced a convenience constructor for creating cache keys directly from a dataset version and format. --- .../export/service/ExportCacheKey.java | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java index 9b6ddff0cc9..be1068f1a53 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java @@ -3,6 +3,8 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetVersion; +import java.util.Objects; + /** * This record encapsulates information related to the dataset, the version of the dataset, * and the format name used for the export, enabling precise identification @@ -10,7 +12,36 @@ */ public record ExportCacheKey(Dataset dataset, DatasetVersion version, String formatName) { - /** The one canonical, version-qualified aux tag. Always used to write. */ + /** + * Constructs an ExportCacheKey instance with the specified dataset, dataset version, and format name. + * @param dataset the dataset associated with this cache key; must not be null + * @param version the dataset version associated with this cache key; must not be null + * @param formatName the format name used for export operations; must not be null or blank + * @throws NullPointerException if the dataset, version, or formatName is null + * @throws IllegalArgumentException if the formatName is blank or empty + */ + public ExportCacheKey(Dataset dataset, DatasetVersion version, String formatName) { + this.dataset = Objects.requireNonNull(dataset); + this.version = Objects.requireNonNull(version); + if (Objects.requireNonNull(formatName).isBlank()) { + throw new IllegalArgumentException("formatName must not be blank or empty"); + } + this.formatName = formatName; + } + + /** + * Convenience wrapper to create a cache key fro ma version and format alone. + * Note: the entity object must have a reference to the dataset present! + * @param version the dataset version + * @param formatName the target format + * @throws NullPointerException if either version, the dataset in the version or the format are null + * @throws IllegalArgumentException if the format name is blank or empty + */ + public ExportCacheKey(DatasetVersion version, String formatName) { + this(Objects.requireNonNull(version).getDataset(), version, formatName); + } + + /** The one canonical, version-qualified aux tag. */ public String auxTag() { return "export_" + formatName + "_" + version.getFriendlyVersionNumber() + ".cached"; } From 121e617b653e5ad6fe88ab169d09687011a4b602 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 7 Aug 2026 23:26:12 +0200 Subject: [PATCH 08/39] refactor(export): relocate service and provider classes to `export.service` package and rename `ExportService` to `ExportServiceBean` - "ExportServiceBean" is more aligned with the codebase style where EJBs mostly have a "Bean" name suffix. - Also move test classes into the same package (under the test source tree) --- .../{ExportService.java => service/ExportServiceBean.java} | 7 +++---- .../export/{ => service}/InternalExportDataProvider.java | 2 +- .../{ => service}/HugeDatasetExportPerformanceIT.java | 2 +- .../export/{ => service}/InternalExportProviderTest.java | 2 +- .../export/{ => service}/TabularDataExportIT.java | 2 +- 5 files changed, 7 insertions(+), 8 deletions(-) rename src/main/java/edu/harvard/iq/dataverse/export/{ExportService.java => service/ExportServiceBean.java} (98%) rename src/main/java/edu/harvard/iq/dataverse/export/{ => service}/InternalExportDataProvider.java (99%) rename src/test/java/edu/harvard/iq/dataverse/export/{ => service}/HugeDatasetExportPerformanceIT.java (98%) rename src/test/java/edu/harvard/iq/dataverse/export/{ => service}/InternalExportProviderTest.java (97%) rename src/test/java/edu/harvard/iq/dataverse/export/{ => service}/TabularDataExportIT.java (99%) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java similarity index 98% rename from src/main/java/edu/harvard/iq/dataverse/export/ExportService.java rename to src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 8dc6462d6f5..9886c2614aa 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/ExportService.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -1,8 +1,7 @@ -package edu.harvard.iq.dataverse.export; +package edu.harvard.iq.dataverse.export.service; import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetVersion; -import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import io.gdcc.spi.export.ExportException; import io.gdcc.spi.export.Exporter; import io.gdcc.spi.export.XMLExporter; @@ -25,9 +24,9 @@ import java.util.logging.Logger; @Stateless -public class ExportService { +public class ExportServiceBean { - private static final Logger logger = Logger.getLogger(ExportService.class.getCanonicalName()); + private static final Logger logger = Logger.getLogger(ExportServiceBean.class.getCanonicalName()); @EJB ExporterRegistryBean exporterRegistry; diff --git a/src/main/java/edu/harvard/iq/dataverse/export/InternalExportDataProvider.java b/src/main/java/edu/harvard/iq/dataverse/export/service/InternalExportDataProvider.java similarity index 99% rename from src/main/java/edu/harvard/iq/dataverse/export/InternalExportDataProvider.java rename to src/main/java/edu/harvard/iq/dataverse/export/service/InternalExportDataProvider.java index 634416b949b..0f74c8f8e32 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/InternalExportDataProvider.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/InternalExportDataProvider.java @@ -1,4 +1,4 @@ -package edu.harvard.iq.dataverse.export; +package edu.harvard.iq.dataverse.export.service; import java.io.InputStream; import java.util.Optional; diff --git a/src/test/java/edu/harvard/iq/dataverse/export/HugeDatasetExportPerformanceIT.java b/src/test/java/edu/harvard/iq/dataverse/export/service/HugeDatasetExportPerformanceIT.java similarity index 98% rename from src/test/java/edu/harvard/iq/dataverse/export/HugeDatasetExportPerformanceIT.java rename to src/test/java/edu/harvard/iq/dataverse/export/service/HugeDatasetExportPerformanceIT.java index 63bf826167d..afd340a6613 100644 --- a/src/test/java/edu/harvard/iq/dataverse/export/HugeDatasetExportPerformanceIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/export/service/HugeDatasetExportPerformanceIT.java @@ -1,4 +1,4 @@ -package edu.harvard.iq.dataverse.export; +package edu.harvard.iq.dataverse.export.service; import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.Dataset; diff --git a/src/test/java/edu/harvard/iq/dataverse/export/InternalExportProviderTest.java b/src/test/java/edu/harvard/iq/dataverse/export/service/InternalExportProviderTest.java similarity index 97% rename from src/test/java/edu/harvard/iq/dataverse/export/InternalExportProviderTest.java rename to src/test/java/edu/harvard/iq/dataverse/export/service/InternalExportProviderTest.java index c072788735e..d794f626602 100644 --- a/src/test/java/edu/harvard/iq/dataverse/export/InternalExportProviderTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/export/service/InternalExportProviderTest.java @@ -1,4 +1,4 @@ -package edu.harvard.iq.dataverse.export; +package edu.harvard.iq.dataverse.export.service; import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.DataTable; diff --git a/src/test/java/edu/harvard/iq/dataverse/export/TabularDataExportIT.java b/src/test/java/edu/harvard/iq/dataverse/export/service/TabularDataExportIT.java similarity index 99% rename from src/test/java/edu/harvard/iq/dataverse/export/TabularDataExportIT.java rename to src/test/java/edu/harvard/iq/dataverse/export/service/TabularDataExportIT.java index a6f6562ed19..d73a2482ae0 100644 --- a/src/test/java/edu/harvard/iq/dataverse/export/TabularDataExportIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/export/service/TabularDataExportIT.java @@ -1,4 +1,4 @@ -package edu.harvard.iq.dataverse.export; +package edu.harvard.iq.dataverse.export.service; import edu.harvard.iq.dataverse.DataFile; import edu.harvard.iq.dataverse.Dataset; From b405d7d8d04814d1b1015860ff2aec03b3a8f6e0 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 18 Aug 2026 17:03:18 +0200 Subject: [PATCH 09/39] docs(export): add Javadoc to private helpers in StorageIOCache #11405 - Documented `tryRead`, `deleteQuietly`, and `storageFor` with proper Javadoc. - Clarified the stream-closing intent in `write` to make the leak-avoidance pattern explicit. --- .../export/service/StorageIOCache.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java index 7ca5aaa33dc..c81efa45a0c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -75,6 +75,7 @@ public Optional read(ExportCacheKey key) throws IOException { public void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException { Path tempFile = Files.createTempFile("dataverse-export-", ".tmp"); try { + // No catch here (checked exception), but closing the stream after use, avoiding leaks. try (OutputStream out = new BufferedOutputStream(Files.newOutputStream(tempFile))) { writer.writeTo(out); } @@ -108,6 +109,9 @@ public void evictAll(Dataset dataset) throws IOException { } } + /** + * Try reading a cached metadata export via StorageIO. Cache miss results in empty {@code Optional}. + */ private static Optional tryRead(StorageIO storage, String auxTag) { // Distinguish "not cached" (normal, frequent) from actual failures: only read if the aux object exists. try { @@ -122,13 +126,16 @@ private static Optional tryRead(StorageIO storage, String try { return Optional.of(storage.getAuxFileAsInputStream(auxTag)); } catch (IOException e) { - // Exists-then-vanished race, or a genuine storage problem. + // Maybe an exists-then-vanished race, or a genuine storage problem. // Treated as a "cache miss" so the pipeline regenerates rather than failing over a cache IO issue. logger.log(Level.WARNING, e, () -> "Could not open cached export " + auxTag); return Optional.empty(); } } + /** + * Try to delete, but do not fail on errors. Logging a warning instead. + */ private static void deleteQuietly(StorageIO storage, String auxTag) { try { storage.deleteAuxObject(auxTag); @@ -139,8 +146,12 @@ private static void deleteQuietly(StorageIO storage, String auxTag) { } } - // Extracted to static method to avoid repeating it in multiple places, allowing substituion - // and extension to a StorageProvider functional interface (which is mockable on its own). + /** + * Retrieve the storage interface for a given dataset. + *

+ * Extracted to a static method to avoid repeating it in multiple places, allowing substitution + * and extension to a StorageProvider functional interface (which is mockable on its own). + */ private static StorageIO storageFor(Dataset dataset) throws IOException { return DataAccess.getStorageIO(dataset); } From 88f90dadd0fa5e9586639525b2238f99da424d1f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 18 Aug 2026 17:04:40 +0200 Subject: [PATCH 10/39] refactor(export): move invalidators list from ExportCacheInvalidator to ExportServiceBean #11405 - Relocated the `invalidators` collection from the sealed interface to the service bean, where it logically belongs as a runtime dependency rather than a static on the contract. - Added a section marker for export data retrieval methods in `ExportServiceBean`. - Noted future plan to replace the static list with a registry pattern once plugins can supply their own invalidation logic. --- .../export/service/ExportCacheInvalidator.java | 13 +++---------- .../dataverse/export/service/ExportServiceBean.java | 12 ++++++++++++ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java index 5208098285e..208fe5316a0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java @@ -1,23 +1,16 @@ package edu.harvard.iq.dataverse.export.service; -import java.util.List; - /** * Represents an abstraction for determining whether a cached export needs to be invalidated and regenerated. *

* This sealed interface is intended to enforce a controlled hierarchy of classes that implement the cache * invalidation logic, ensuring behavior consistency across different implementations. If necessary, the contract * may be altered to allow more dynamic discovery of invalidators. + *

+ * If at a later point we want to enable export plugins to provide their own invalidation logic, + * this interface shall be unsealed and moved into the Exporter SPI codebase. */ public sealed interface ExportCacheInvalidator permits FileEmbargoExpiryInvalidator { - - /** - * A collection of {@link ExportCacheInvalidator} instances. - * This list is intended to centralize all invalidation mechanisms for export cache entries. - * Any new implementations must be added here in addition to the "permits" on the interface seal. - */ - List invalidators = List.of(new FileEmbargoExpiryInvalidator()); - /** Should a cached export for this key be discarded and regenerated? */ boolean isStale(ExportCacheKey key); } diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 9886c2614aa..9e10ccc5d6b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -31,6 +31,18 @@ public class ExportServiceBean { @EJB ExporterRegistryBean exporterRegistry; + /** + * A collection of {@link ExportCacheInvalidator} instances. + * This list is intended to centralize all invalidation mechanisms for export cache entries. + * Any new implementations must be added here in addition to the "permits" on the interface seal. + *

+ * Note: Once we allow plugins to provide their own invalidation logic, we must load them. + * This static, non-CDI list shall then be replaced by a registry pattern following implementation. + */ + List invalidators = List.of(new FileEmbargoExpiryInvalidator()); + + // METHODS TO RETRIEVE EXPORTED DATA + public InputStream getExport(DatasetVersion datasetVersion, String formatName) throws ExportException, IOException { Dataset dataset = datasetVersion.getDataset(); From 873b589209b9cdf1854ee03620808115a03499d5 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 18 Aug 2026 17:05:49 +0200 Subject: [PATCH 11/39] style(export): rename `exporterRegistry` field to `registry` in ExportServiceBean Making it simpler to read inline. --- .../harvard/iq/dataverse/export/service/ExportServiceBean.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 9e10ccc5d6b..55fbc20417f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -29,7 +29,7 @@ public class ExportServiceBean { private static final Logger logger = Logger.getLogger(ExportServiceBean.class.getCanonicalName()); @EJB - ExporterRegistryBean exporterRegistry; + ExporterRegistryBean registry; /** * A collection of {@link ExportCacheInvalidator} instances. From 6b124d7441b225708feaa77533b789f49c1551ee Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 18 Aug 2026 17:08:02 +0200 Subject: [PATCH 12/39] feat(export): make ExportCache instance available in service #11405 Added `ExportCache` as an CDI (not EJB) injected dependency in the service bean. --- .../iq/dataverse/export/service/ExportServiceBean.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 55fbc20417f..bd3e655f388 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -7,6 +7,7 @@ import io.gdcc.spi.export.XMLExporter; import jakarta.ejb.EJB; import jakarta.ejb.Stateless; +import jakarta.inject.Inject; import jakarta.ws.rs.core.MediaType; import org.apache.commons.io.IOUtils; @@ -31,6 +32,12 @@ public class ExportServiceBean { @EJB ExporterRegistryBean registry; + // We must use (frowned upon) field injection here, as EJB requires a no-args constructor. + // When the codebase transitions to use CDI only, this shall be changed to constructor injection. + @SuppressWarnings("java:S6813") + @Inject + ExportCache cache; + /** * A collection of {@link ExportCacheInvalidator} instances. * This list is intended to centralize all invalidation mechanisms for export cache entries. From a2ee3d1796fddb17386fb592d7644e2eddef3972 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 18 Aug 2026 17:18:40 +0200 Subject: [PATCH 13/39] refactor(export): make cache clearing version-aware #11405 - Introduced `clearCachedFormats(DatasetVersion, List)` as the version-specific clearing entry point, with the dataset-level overload delegating via a new `defaultVersion()` helper. - Added `clearCachedFormat(DatasetVersion, String)` to evict a single cache entry by key. - Added `requireExists` and `requireAllExist` validation methods to `ExporterRegistryBean` so format names are checked before eviction. --- .../export/service/ExportServiceBean.java | 101 +++++++++++++++--- .../export/service/ExporterRegistryBean.java | 39 +++++++ 2 files changed, 124 insertions(+), 16 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index bd3e655f388..d90a8dfdf8a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -17,10 +17,14 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.io.OutputStream; import java.sql.Timestamp; import java.util.Date; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Optional; import java.util.ServiceConfigurationError; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -215,31 +219,84 @@ public void exportFormats(Dataset dataset, List formatNames) throws Expo "Unknown runtime exception exporting metadata. " + (e.getMessage() == null ? "" : e.getMessage())); } } - - // A convenience wrapper method + + + + // ++++ ++++ ++++ METHODS FOR CACHE MANAGEMENT ++++ ++++ ++++ + + /** + * Clears all cached export formats for the given dataset. + * Because all formats are removed, the dataset's * "last exported" timestamp is also set to null, + * reflecting no cached exports remain. + *

+ * TODO: When this service is extended to support caching and retrieving arbitrary dataset versions, + * it needs to be decided what "all" means: does "all" include all versions? + * Maybe replace the method with one that takes a list of versions. + * TODO: The export timestamp should be moved to the individual versions. + * Not sure where else we may rely on this timestamp being on the dataset. + * + * @param dataset the dataset whose cached exports should all be cleared + * @throws IOException if an I/O error occurs while clearing the cached format entries + */ public void clearAllCachedFormats(Dataset dataset) throws IOException { clearCachedFormats(dataset, List.of()); + // Only if we clear *all* formats, reset the "last exported" time stamp. + // (Otherwise some formats still may exist in the cache.) dataset.setLastExportTime(null); } - public void clearCachedFormats(Dataset dataset, List formatNames) throws IOException { + /** + * Clears the cached formats for the given dataset. + * Delegates to the version-specific overload by resolving the default version of the dataset. + * + * @param dataset the dataset for which cached formats should be cleared; must not be null + * @param formatNames the list of format names to clear; may be null to clear all formats + * @throws ExportException if the dataset is null + */ + public void clearCachedFormats(Dataset dataset, List formatNames) throws ExportException { if (dataset == null) { - throw new ExportException("cleareCachedFormats called with null Dataset"); + throw new ExportException("Dataset may not be null"); } + // Let clearCachedFormats(DatasetVersion, List) handle verifying the formatNames - if (formatNames == null) { - throw new ExportException("clearCachedFormats called with null formatNames (use an empty List for \"all\""); + clearCachedFormats(defaultVersion(dataset), formatNames); + } + + /** + * Clears the cached formats for the specified dataset version. + * Validates that the dataset version is not null and that all provided format names exist in + * the registry before clearing each cached format. + * + * @param datasetVersion the dataset version whose cached formats should be cleared; must not be null + * @param formatNames the list of format names to clear from the cache + * @throws ExportException if the dataset version is null or any format name is invalid + */ + public void clearCachedFormats(DatasetVersion datasetVersion, List formatNames) { + if (datasetVersion == null) { + throw new ExportException("Dataset version may not be null"); } - - for (Exporter e : exporterMap.values()) { - String formatName = e.getFormatName(); - if (formatNames.isEmpty() || formatNames.contains(formatName)) { - try { - clearCachedExport(dataset, formatName); - } catch (IOException ex) { - // not fatal - } - } + try { + registry.requireAllExist(formatNames); + } catch (IllegalArgumentException ex) { + throw new ExportException("Invalid format names: " + ex.getMessage()); + } + + formatNames.forEach(formatName -> clearCachedFormat(datasetVersion, formatName)); + } + + void clearCachedFormat(DatasetVersion datasetVersion, String formatName) throws ExportException { + // Note: If this is ever changed to a "public" method, it will require parameter validation! + // (Which may duplicate checks when coming from other methods) + + // Build the cache key and evict it from the cache. + // NOTE: If the given version wasn't cacheable in the first place (as per isCacheable()), + // eviction should just succeed instead of failing (nothing was ever there, but this + // was the service's choice, not the cache's!). + ExportCacheKey key = new ExportCacheKey(datasetVersion, formatName); + try { + cache.evict(key); + } catch (IOException ex) { + throw new ExportException("Failed to clear cached format: " + ex.getMessage()); } } @@ -312,5 +369,17 @@ public String getMediaType(String provider) { } return MediaType.TEXT_PLAIN; } + + /** + * Export policy: determines the default dataset version to use for export operations. + * If the given dataset has been released, its released version is returned. + * Otherwise, the dataset's latest version is returned. + * + * @param dataset the dataset from which the default version should be resolved + * @return the released version if the dataset is released, otherwise the latest version (should be draft) + */ + static DatasetVersion defaultVersion(Dataset dataset) { + return dataset.isReleased() ? dataset.getReleasedVersion() : dataset.getLatestVersion(); + } } diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index 66eac0039af..5bfd72dc9ff 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -23,8 +23,10 @@ import java.util.Map; import java.util.Optional; import java.util.ServiceLoader; +import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * ExporterRegistry is responsible for managing the registration, retrieval, and lifecycle of {@code Exporter}s. @@ -105,6 +107,43 @@ public List getLabels() { .toList(); } + /** + * Validates that an exporter is registered for the given format name. + * Throws an exception if the format name is null or if no exporter has been registered under that name. + * + * @param formatName the name of the format to check; must not be null + * @throws IllegalArgumentException if formatName is null, or if no exporter is registered for the specified format name + */ + public void requireExists(String formatName) { + if (formatName == null) { + throw new IllegalArgumentException("format name may not be null"); + } + if (!exporters.containsKey(formatName)) { + throw new IllegalArgumentException("no exporter registered for format: " + formatName); + } + } + + /** + * Validates that every format in the provided list has a corresponding exporter registered in this registry. + * If one or more formats are not recognized, an exception is thrown listing all invalid formats. + * + * @param formats the list of format names that must each have a registered exporter; must not be null; + * an empty list is allowed (no formats are checked) + * @throws IllegalArgumentException if any format in the list does not have a corresponding registered exporter, + * with the message enumerating all invalid format names; or if the list is null + */ + public void requireAllExist(List formats) { + if (formats == null) { + throw new IllegalArgumentException("list must not be null (hint: use empty list to express 'all')"); + } + Set invalidFormats = formats.stream() + .filter(format -> !exporters.containsKey(format)) + .collect(Collectors.toUnmodifiableSet()); + if (!invalidFormats.isEmpty()) { + throw new IllegalArgumentException("no exporters available for " + String.join(", ", invalidFormats)); + } + } + @PostConstruct private void initialize() { /* From 148832cad806f0c6e7f2e4a260168fee84afcad2 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Tue, 18 Aug 2026 17:24:41 +0200 Subject: [PATCH 14/39] style(export): move export trigger service methods next to each other #11405 Align the related methods into one block, not divided by the cache handling stuff. --- .../export/service/ExportServiceBean.java | 171 ++++++++++-------- 1 file changed, 93 insertions(+), 78 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index d90a8dfdf8a..4ab09d675d2 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -141,84 +141,6 @@ public String getLatestPublishedAsString(Dataset dataset, String formatName) { return null; } - - // A convenience wrapper method; the actual implementation has been moved - // into exportFormats() below. - public void exportAllFormats(Dataset dataset) throws ExportException { - exportFormats(dataset, List.of()); - } - - /** - * This method is added to supplement the classic exportAllFormats() in order - * to allow the metadata export APIs to selectively re-export only the formats - * specified. This is to finally allow an instance admin to avoid running - * a complete, from-scratch reexport when only _some_, or just one of them - * actually needs to be refreshed. On a large instance this can waste a - * significant amount of time and CPU cycles. (new as of 6.12) - * This method calls the cacheExport() method for every valid/supported - * format name supplied, or for every Exporter available, if an empty List - * is passed. - * Only the latest published version is used for exports. - * exportAllFormats() above is now a convenience wrapper, with the - * implementation moved here. - * - * @param dataset - * @param formatNames - * @throws ExportException - */ - public void exportFormats(Dataset dataset, List formatNames) throws ExportException { - if (dataset == null) { - throw new ExportException("exportFormats called with null Dataset"); - } - - if (formatNames == null) { - throw new ExportException("exportFormats called with null formatNames (use an empty List for \"all\""); - } - - try { - clearCachedFormats(dataset, formatNames); - } catch (IOException ex) { - Logger.getLogger(ExportService.class.getName()).log(Level.SEVERE, null, ex); - } - - try { - DatasetVersion releasedVersion = dataset.getReleasedVersion(); - if (releasedVersion == null) { - throw new ExportException("No released version for dataset " + dataset.getGlobalId().toString()); - } - InternalExportDataProvider dataProvider = new InternalExportDataProvider(releasedVersion); - - for (Exporter e : exporterMap.values()) { - String formatName = e.getFormatName(); - if (formatNames.isEmpty() || formatNames.contains(formatName)) { - if (e.getPrerequisiteFormatName().isPresent()) { - String prereqFormatName = e.getPrerequisiteFormatName().get(); - try (InputStream preReqStream = getExport(dataset.getReleasedVersion(), prereqFormatName)) { - dataProvider.setPrerequisiteInputStream(preReqStream); - cacheExport(dataset, dataProvider, formatName, e); - dataProvider.setPrerequisiteInputStream(null); - } catch (IOException ioe) { - throw new ExportException("Could not get prerequisite " + e.getPrerequisiteFormatName() + " to create " + formatName + "export for dataset " + dataset.getId(), ioe); - } - } else { - cacheExport(dataset, dataProvider, formatName, e); - } - } - } - // Finally, if we have been able to successfully export in all available - // formats, we'll increment the "last exported" time stamp: - if (formatNames.isEmpty()) { - dataset.setLastExportTime(new Timestamp(new Date().getTime())); - } - - } catch (ServiceConfigurationError serviceError) { - throw new ExportException("Service configuration error during export. " + serviceError.getMessage()); - } catch (RuntimeException e) { - logger.log(Level.FINE, e.getMessage(), e); - throw new ExportException( - "Unknown runtime exception exporting metadata. " + (e.getMessage() == null ? "" : e.getMessage())); - } - } @@ -299,6 +221,99 @@ void clearCachedFormat(DatasetVersion datasetVersion, String formatName) throws throw new ExportException("Failed to clear cached format: " + ex.getMessage()); } } + + + + // ++++ ++++ ++++ METHODS TO TRIGGER DIFFERENT EXPORTS ++++ ++++ ++++ + + /** + * Exports the given dataset in all available supported formats. + *

+ * This is a convenience wrapper that delegates to {@link #exportFormats(Dataset, List)} with an empty list, + * causing every registered exporter to be invoked. + *

+ * Note: Currently, only the latest released version of the dataset is exported. + * This may change in future versions. + * + * @param dataset the dataset whose metadata should be re-exported in all formats + * @throws ExportException if any exporter fails to produce its output + */ + public void exportAllFormats(Dataset dataset) throws ExportException { + exportFormats(dataset, List.of()); + } + + /** + * This method is added to supplement the classic exportAllFormats() in order + * to allow the metadata export APIs to selectively re-export only the formats + * specified. This is to finally allow an instance admin to avoid running + * a complete, from-scratch reexport when only _some_, or just one of them + * actually needs to be refreshed. On a large instance this can waste a + * significant amount of time and CPU cycles. (new as of 6.12) + * This method calls the cacheExport() method for every valid/supported + * format name supplied, or for every Exporter available, if an empty List + * is passed. + * Only the latest published version is used for exports. + * exportAllFormats() above is now a convenience wrapper, with the + * implementation moved here. + * + * @param dataset + * @param formatNames + * @throws ExportException + */ + public void exportFormats(Dataset dataset, List formatNames) throws ExportException { + if (dataset == null) { + throw new ExportException("exportFormats called with null Dataset"); + } + try { + registry.requireAllExist(formatNames); + } catch (IllegalArgumentException ex) { + throw new ExportException("Invalid format names: " + ex.getMessage()); + } + + try { + clearCachedFormats(dataset, formatNames); + } catch (IOException ex) { + Logger.getLogger(ExportService.class.getName()).log(Level.SEVERE, null, ex); + } + + try { + DatasetVersion releasedVersion = dataset.getReleasedVersion(); + if (releasedVersion == null) { + throw new ExportException("No released version for dataset " + dataset.getGlobalId().toString()); + } + InternalExportDataProvider dataProvider = new InternalExportDataProvider(releasedVersion); + + for (Exporter e : exporterMap.values()) { + String formatName = e.getFormatName(); + if (formatNames.isEmpty() || formatNames.contains(formatName)) { + if (e.getPrerequisiteFormatName().isPresent()) { + String prereqFormatName = e.getPrerequisiteFormatName().get(); + try (InputStream preReqStream = getExport(dataset.getReleasedVersion(), prereqFormatName)) { + dataProvider.setPrerequisiteInputStream(preReqStream); + cacheExport(dataset, dataProvider, formatName, e); + dataProvider.setPrerequisiteInputStream(null); + } catch (IOException ioe) { + throw new ExportException("Could not get prerequisite " + e.getPrerequisiteFormatName() + " to create " + formatName + "export for dataset " + dataset.getId(), ioe); + } + } else { + cacheExport(dataset, dataProvider, formatName, e); + } + } + } + // Finally, if we have been able to successfully export in all available + // formats, we'll increment the "last exported" time stamp: + if (formatNames.isEmpty()) { + dataset.setLastExportTime(new Timestamp(new Date().getTime())); + } + + } catch (ServiceConfigurationError serviceError) { + throw new ExportException("Service configuration error during export. " + serviceError.getMessage()); + } catch (RuntimeException e) { + logger.log(Level.FINE, e.getMessage(), e); + throw new ExportException( + "Unknown runtime exception exporting metadata. " + (e.getMessage() == null ? "" : e.getMessage())); + } + } // This method finds the exporter for the format requested, // then produces the dataset metadata as a JsonObject, then calls From b6b6ef5cd9085b612f4c271830194df526a69502 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Aug 2026 14:51:24 +0200 Subject: [PATCH 15/39] feat(export): add prerequisite dependency verification to ExporterRegistryBean #11405 - Added `buildFormatRequiredByMap` to build a read-only map of prerequisite format names to the exporters that depend on them. - Added `buildAndVerifyRequirements` to validate registry integrity: all prerequisite formats must have a registered exporter, and no cyclic prerequisite chains may exist. - Integrated the check into initialization as Step 4, failing fast with `ExportException` on any integrity violation (missing prerequisite or cycle). --- .../export/service/ExporterRegistryBean.java | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index 5bfd72dc9ff..92243cbb119 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -2,6 +2,7 @@ import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.util.BundleUtil; +import io.gdcc.spi.export.ExportException; import io.gdcc.spi.export.Exporter; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; @@ -19,8 +20,10 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.ServiceLoader; import java.util.Set; @@ -196,6 +199,10 @@ private void initialize() { exp.getClass().getClassLoader().getClass().getCanonicalName() }); }); + + // Step 4 - Create prerequisite dependency graph and verify integrity + var requiredBy = buildAndVerifyRequirements(loadedExporters); + // All good, (more or less) atomic updates now. this.exporters = loadedExporters; } @@ -212,4 +219,85 @@ private void tearDown() { logger.log(Level.WARNING, "Could not close exporter classloader", e); } } + + /** + * Builds a map of prerequisite format names to the list of export formats that depend on them. + * For each registered exporter that declares a prerequisite format, the exporter's format name is collected + * under the prerequisite key. (Thus exporters without a prerequisite are not included.) + * + * @return a map where each key is a prerequisite format name and each value is the list of format names of + * exporters that require that prerequisite; an empty map if no exporter declares a prerequisite + */ + static Map> buildFormatRequiredByMap(Map exporters) { + Objects.requireNonNull(exporters); + Map> requiredByMap = new HashMap<>(); + + for (Exporter exporter : exporters.values()) { + exporter.getPrerequisiteFormatName().ifPresent(prereq -> + requiredByMap + // Create new list if necessary + .computeIfAbsent(prereq, k -> new ArrayList<>()) + // Put down exporter as depending on this format + .add(exporter.getFormatName())); + } + + // Make a deep, read-only copy before returning + return requiredByMap.entrySet().stream() + .collect(Collectors.toUnmodifiableMap( + Map.Entry::getKey, + entry -> List.copyOf(entry.getValue()) + )); + } + + /** + * Builds the prerequisite dependency map from the given exporters and verifies that every prerequisite format + * referenced by an exporter is itself backed by a registered exporter in the provided map. + * In addition, it verifies no prerequisite formats form a cyclic dependency. + * + * @return the built prerequisite dependency map, see {@link #buildFormatRequiredByMap(Map)}. + * @throws ExportException if one or more prerequisite format names in the dependency map + * do not have a corresponding entry in the provided exporters map + */ + static Map> buildAndVerifyRequirements(Map exporters) { + Map> formatRequiredBy = buildFormatRequiredByMap(exporters); + + // Check that all prerequisite formats have a registered exporter + if (!exporters.keySet().containsAll(formatRequiredBy.keySet())) { + Map> unsatisfied = formatRequiredBy.entrySet().stream() + .filter(e -> !exporters.containsKey(e.getKey())) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); + + logger.log(Level.SEVERE, "Exporter registry integrity check failed: the following exporters are missing prerequisites: {}", unsatisfied); + throw new ExportException("Exporter registry integrity check failed"); + } + + // Now that we know all exporters are present as required, check for cyclic dependencies! + // How: a cycle exists if we revisit a format already seen within the current chain. + // Checking against the whole chain, not just the starting format, is essential:a chain may merely lead + // *into* a cycle it is not part of, e.g., D -> A -> B -> A. + boolean cycleDetected = false; + for (String startFormat : exporters.keySet()) { + List chain = new ArrayList<>(); + // Using a set here to enable O(1) lookup for seen formats. + Set seen = new HashSet<>(); + + String current = startFormat; + while (current != null) { + chain.add(current); + if (!seen.add(current)) { + logger.log(Level.SEVERE, "Exporter registry integrity check failed due to cyclic format dependency chain: {0}", String.join(" -> ", chain)); + cycleDetected = true; + break; + } + // Existence was verified above, so the lookup cannot return null here. + // If no format is detected, break the loop by returning null. + current = exporters.get(current).getPrerequisiteFormatName().orElse(null); + } + } + if (cycleDetected) { + throw new ExportException("Exporter registry integrity check failed: cyclic dependencies detected."); + } + + return formatRequiredBy; + } } From 6cd4a99fd068662a61a0a2b9ebada1310904e75e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Aug 2026 14:58:28 +0200 Subject: [PATCH 16/39] feat(export): cache formatRequiredBy map in ExporterRegistryBean #11405 Added `formatRequiredBy` field to store the prerequisite format dependency map alongside the exporters map, populated during registry initialization. Will be reused during cascaded cache eviction or exporting of formats depending on a certain format. --- .../export/service/ExporterRegistryBean.java | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index 92243cbb119..38045e248da 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -19,6 +19,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -75,6 +76,11 @@ public record Labels( // No half-initialized state is exposable this way. Future optimizations may use @Lock on it, too, for example, // when implementing a reload mechanism. private Map exporters = Map.of(); + + // Caching the requirements as a map (key = format, value = list of formats that require this format). + // Managed the same way as the exporter map. + private Map> formatRequiredBy = Map.of(); + // Caching the classloader used to load plugin JAR files, keeping it open, will allow reuse for reloads // or loading more resources from plugin JARs. May be dropped later if not necessary. private URLClassLoader exporterClassLoader; @@ -147,6 +153,17 @@ public void requireAllExist(List formats) { } } + /** + * Retrieves the list of export format names that depend on the given format as a prerequisite. + * + * @param format the name of the format for which dependent formats are to be resolved. + * @return a list of format names of exporters that require the specified format as a prerequisite, + * or an empty list if no such dependencies exist + */ + public List getFormatsDependingOn(String format) { + return this.formatRequiredBy.getOrDefault(format, Collections.emptyList()); + } + @PostConstruct private void initialize() { /* @@ -204,7 +221,7 @@ private void initialize() { var requiredBy = buildAndVerifyRequirements(loadedExporters); // All good, (more or less) atomic updates now. this.exporters = loadedExporters; - + this.formatRequiredBy = requiredBy; } @PreDestroy From 1e142a9133948157451d8b8cd48197b80f7699eb Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Aug 2026 14:59:48 +0200 Subject: [PATCH 17/39] feat(export): add topological comparator to ExporterRegistryBean #11405 - Added `buildPrerequisitesChainDepth` to compute the prerequisite chain depth for each format (0 = no prerequisite, N = N levels deep). - Added `buildTopologicalComparator` to create an immutable comparator ordering exporters by depth, with format name as tiebreaker for deterministic results. - Exposed via `getTopologicalComparator()` so callers can sort the exporter list in a dependency-safe order. - Integrated as Step 5 in initialization, stored alongside the existing `formatRequiredBy` map. --- .../export/service/ExporterRegistryBean.java | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index 38045e248da..a4812b1abcb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -20,6 +20,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -81,6 +82,10 @@ public record Labels( // Managed the same way as the exporter map. private Map> formatRequiredBy = Map.of(); + // Comparator imposing a topologically consistent order on exporters, derived from prereqDepthByFormat. + // Managed the same way as the exporter map. Initialized with empty Map for consistency. + private Comparator topologicalComparator = buildTopologicalComparator(Map.of()); + // Caching the classloader used to load plugin JAR files, keeping it open, will allow reuse for reloads // or loading more resources from plugin JARs. May be dropped later if not necessary. private URLClassLoader exporterClassLoader; @@ -164,6 +169,37 @@ public List getFormatsDependingOn(String format) { return this.formatRequiredBy.getOrDefault(format, Collections.emptyList()); } + /** + * Returns a {@link Comparator} that orders {@link Exporter}s such that every prerequisite format sorts before + * all exporters depending on it (directly or transitively). + *

+ * The comparator sorts on the cached prerequisite chain depth (see {@link #buildPrerequisitesChainDepth(Map)}) + * rather than comparing prerequisite relations directly: the {@code Comparator} contract requires a total, + * transitive ordering, while "is a prerequisite of" is only a partial order - unrelated exporters would + * compare as equal, allowing a sort to place a transitive dependent before its prerequisite. + * Depth turns the partial order into a total order that still respects all prerequisite constraints. + * Ties are broken by format name for deterministic results. + *

+ * The returned comparator is immutable, thread-safe, and reflects the registry state (at startup or when refreshed). + *

+ * Please be aware that the comparator is not capable of preventing dependency cycles! It is the responsibility + * of the caller to ensure that the registry does not contain cyclic dependencies. + *

+ * Example usage: + *

{@code
+     * List ordered = registry.getAll()
+     *                              .stream()
+     *                              .sorted(registry.getTopologicalComparator())
+     *                              .toList();
+     * }
+ * + * @return a comparator imposing a topologically consistent total order on registered exporters + */ + public Comparator getTopologicalComparator() { + return topologicalComparator; + } + + @PostConstruct private void initialize() { /* @@ -219,9 +255,15 @@ private void initialize() { // Step 4 - Create prerequisite dependency graph and verify integrity var requiredBy = buildAndVerifyRequirements(loadedExporters); + + // Step 5 - Build map of prerequisite dependency graph depth per format and the comparator + var prerequisitesDepth = buildPrerequisitesChainDepth(loadedExporters); + var comparator = buildTopologicalComparator(prerequisitesDepth); + // All good, (more or less) atomic updates now. this.exporters = loadedExporters; this.formatRequiredBy = requiredBy; + this.topologicalComparator = comparator; } @PreDestroy @@ -317,4 +359,53 @@ static Map> buildAndVerifyRequirements(Map + *
  • a depth of 0 means the exporter has no prerequisite format,
  • + *
  • a depth of 1 means it depends on a format that itself has no prerequisite,
  • + *
  • and so on for longer chains.
  • + * + * + * @param exportersByFormat a map from format name to its associated {@link Exporter} instance; must not be null + * @return an unmodifiable map where each key is a format name and each value is the integer depth of the + * prerequisite chain for that format; the map contains one entry per export format in the input + */ + static Map buildPrerequisitesChainDepth(Map exportersByFormat) { + Objects.requireNonNull(exportersByFormat); + Map depthsByFormat = new HashMap<>(); + for (Exporter e : exportersByFormat.values()) { + depthOf(e, exportersByFormat, depthsByFormat); + } + return Map.copyOf(depthsByFormat); + } + + // Note: Make sure no cyclomatic format dependencies exist in exporters, otherwise infinite recursion may occur! + private static int depthOf(Exporter e, Map exportersByFormat, Map depthsByFormat) { + // If the depth map does not already contain the depth value, compute it recursively, then return it. + return depthsByFormat.computeIfAbsent( + e.getFormatName(), + // Note: the following operates on Optional.map(), not Stream.map()! + name -> e.getPrerequisiteFormatName() + .map(exportersByFormat::get) + .map(prereq -> depthOf(prereq, exportersByFormat, depthsByFormat) + 1) + // As no value could be found, return 0 = no prerequisite format + .orElse(0)); + } + + /** + * Creates a comparator ordering exporters by their prerequisite format chain depth, with format name as tiebreak. + * Exporters not present in the given depth map (which should not occur for registered exporters) are treated + * as having no prerequisite (depth 0). See {@link #getTopologicalComparator()} for the rationale. + * + * @param depthsByFormat map from format name to prerequisite chain depth; must not be null + * @return an immutable, thread-safe comparator + */ + static Comparator buildTopologicalComparator(Map depthsByFormat) { + Objects.requireNonNull(depthsByFormat); + return Comparator.comparingInt((Exporter e) -> depthsByFormat.getOrDefault(e.getFormatName(), 0)) + .thenComparing(Exporter::getFormatName); + } } From 8f1d92870e79fed6ab46d61d3cf08b3b37e9dda8 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Wed, 19 Aug 2026 23:11:31 +0200 Subject: [PATCH 18/39] feat(util,export): enforce owner-only permissions on export temp files #11405 - Added `SecureTempFiles` utility that creates temp files with `0600` permissions on POSIX systems; on Windows it relies on the per-user `%TEMP%` ACLs. - Replaced raw `Files.createTempFile` in `StorageIOCache.write` with `SecureTempFiles.createOwnerOnlyTempFile` so other local users can no longer read or tamper with export temp files. --- .../export/service/StorageIOCache.java | 3 +- .../iq/dataverse/util/SecureTempFiles.java | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/util/SecureTempFiles.java diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java index c81efa45a0c..21d0f6b2ebd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -3,6 +3,7 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.dataaccess.DataAccess; import edu.harvard.iq.dataverse.dataaccess.StorageIO; +import edu.harvard.iq.dataverse.util.SecureTempFiles; import io.gdcc.spi.export.ExportException; import jakarta.enterprise.context.ApplicationScoped; @@ -73,7 +74,7 @@ public Optional read(ExportCacheKey key) throws IOException { */ @Override public void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException { - Path tempFile = Files.createTempFile("dataverse-export-", ".tmp"); + Path tempFile = SecureTempFiles.createOwnerOnlyTempFile("dataverse-export-", ".tmp"); try { // No catch here (checked exception), but closing the stream after use, avoiding leaks. try (OutputStream out = new BufferedOutputStream(Files.newOutputStream(tempFile))) { diff --git a/src/main/java/edu/harvard/iq/dataverse/util/SecureTempFiles.java b/src/main/java/edu/harvard/iq/dataverse/util/SecureTempFiles.java new file mode 100644 index 00000000000..eed19cee86b --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/util/SecureTempFiles.java @@ -0,0 +1,31 @@ +package edu.harvard.iq.dataverse.util; + +import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Set; + +public final class SecureTempFiles { + + private SecureTempFiles() { + } + + @SuppressWarnings("java:S5443") // Make SonarQube stop warning about "raw" temp file generator on Windows. + public static Path createOwnerOnlyTempFile(String prefix, String suffix) throws IOException { + if (FileSystems.getDefault().supportedFileAttributeViews().contains("posix")) { + // POSIX (Linux, macOS): owner read/write only -> "rw-------" (0600) + Set perms = PosixFilePermissions.fromString("rw-------"); + FileAttribute> attr = + PosixFilePermissions.asFileAttribute(perms); + return Files.createTempFile(prefix, suffix, attr); + } else { + // Windows: the per-user temp directory (%TEMP%) is already + // ACL-protected so only the owner (and admins) can access it. + return Files.createTempFile(prefix, suffix); + } + } +} From 1b64d431c47ac8568512feb99acd56b085a86d73 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 20 Aug 2026 17:46:57 +0200 Subject: [PATCH 19/39] fix(export): decouple ExportCacheKey from JPA entities #11405 The cache key should not be responsible to carry the information about the "where" of an export, just about the "what". Changing dependent methods accordingly. Also, fixed ambiguity with the cache invalidator implementations: the invalidator should look for stale *versions* of dataset, not for the dataset as a whole being stale. The cache is treating versions individually, so they shall get stale individually, too. - Reduced `ExportCacheKey` to a single `auxTag` string, removing `Dataset`/`DatasetVersion` references for thread-safety and GC-friendliness. - Moved `TAG_PREFIX`/`TAG_SUFFIX` into `ExportCacheKey` as public constants. - Added explicit `Dataset` parameter to all `ExportCache` methods (`read`, `write`, `evict`) since the key no longer carries storage context. - Added explicit `DatasetVersion` parameter to `ExportCacheInvalidator.isStale`; updated `FileEmbargoExpiryInvalidator` with null-checks and released/archived status guard. - Updated `StorageIOCache` logging to use `dataset.getId()` instead of the version string. --- .../dataverse/export/service/ExportCache.java | 12 +++--- .../service/ExportCacheInvalidator.java | 12 +++++- .../export/service/ExportCacheKey.java | 41 ++++++++----------- .../service/FileEmbargoExpiryInvalidator.java | 34 +++++++++++---- .../export/service/StorageIOCache.java | 28 +++++++------ 5 files changed, 75 insertions(+), 52 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java index d4c2f2fe28e..922b1b0296a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCache.java @@ -9,9 +9,9 @@ import java.util.Optional; /** - * Storage abstraction for cached metadata exports. Implementations own all - * knowledge about where and under which names cached exports live; the export - * pipeline only ever deals in {@link ExportCacheKey}s and streams. + * Storage abstraction for cached metadata exports. + * Implementations own all knowledge about where and under which names cached exports live. + * The export pipeline only ever deals in {@link ExportCacheKey}s, datasets, and streams. */ public sealed interface ExportCache permits StorageIOCache { @@ -20,17 +20,17 @@ public sealed interface ExportCache permits StorageIOCache { * @return the cached export stream, or empty if none is cached. Note: the caller is responsible for closing the stream. * @throws IOException on actual storage failures (not on a cache miss) */ - Optional read(ExportCacheKey key) throws IOException; + Optional read(Dataset dataset, ExportCacheKey key) throws IOException; /** * Produces and stores an export. The {@code writer} callback receives the output stream to write to. * Any implementations guarantee that a partially written export is never made visible under the cache key * (i.e., a failed write leaves either the previous entry or no entry). */ - void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException; + void write(Dataset dataset, ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException; /** Removes a cached export. Absence of the entry is not an error. */ - void evict(ExportCacheKey key) throws IOException; + void evict(Dataset dataset, ExportCacheKey key) throws IOException; /** * Removes all cached exports for a dataset, across all versions and formats, including legacy (pre-versioning) entries. diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java index 208fe5316a0..1e11dc78abf 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheInvalidator.java @@ -1,5 +1,7 @@ package edu.harvard.iq.dataverse.export.service; +import edu.harvard.iq.dataverse.DatasetVersion; + /** * Represents an abstraction for determining whether a cached export needs to be invalidated and regenerated. *

    @@ -11,6 +13,12 @@ * this interface shall be unsealed and moved into the Exporter SPI codebase. */ public sealed interface ExportCacheInvalidator permits FileEmbargoExpiryInvalidator { - /** Should a cached export for this key be discarded and regenerated? */ - boolean isStale(ExportCacheKey key); + /** + * Should a cached export for this key be discarded and regenerated? + * + * @param datasetVersion the dataset version for which the export is being generated + * @param key the cache key associated with the export + * @throws IllegalArgumentException if any parameters are null or implementation expectations are not met + */ + boolean isStale(DatasetVersion datasetVersion, ExportCacheKey key); } diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java index be1068f1a53..042a6a4658a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java @@ -1,6 +1,5 @@ package edu.harvard.iq.dataverse.export.service; -import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetVersion; import java.util.Objects; @@ -9,40 +8,34 @@ * This record encapsulates information related to the dataset, the version of the dataset, * and the format name used for the export, enabling precise identification * of cache entries for export operations. + *

    + * Note: This cache key is thread-safe, as the JPA entities are not kept, but the read-only aux tag is + * derived at construction time. Even if the version entity is altered between usages, the cache key is stable. + * The cache itself derives the target auxiliary storage (dataset or datafile) at runtime. + * In addition, by not keeping an JPA entity reference, garbage collection is facilitated. */ -public record ExportCacheKey(Dataset dataset, DatasetVersion version, String formatName) { +public record ExportCacheKey(String auxTag) { + + public static final String TAG_PREFIX = "export_"; + public static final String TAG_SUFFIX = ".cached"; /** - * Constructs an ExportCacheKey instance with the specified dataset, dataset version, and format name. - * @param dataset the dataset associated with this cache key; must not be null + * Constructs an ExportCacheKey instance with the specified dataset version, and format name. * @param version the dataset version associated with this cache key; must not be null * @param formatName the format name used for export operations; must not be null or blank * @throws NullPointerException if the dataset, version, or formatName is null * @throws IllegalArgumentException if the formatName is blank or empty */ - public ExportCacheKey(Dataset dataset, DatasetVersion version, String formatName) { - this.dataset = Objects.requireNonNull(dataset); - this.version = Objects.requireNonNull(version); - if (Objects.requireNonNull(formatName).isBlank()) { - throw new IllegalArgumentException("formatName must not be blank or empty"); - } - this.formatName = formatName; - } - - /** - * Convenience wrapper to create a cache key fro ma version and format alone. - * Note: the entity object must have a reference to the dataset present! - * @param version the dataset version - * @param formatName the target format - * @throws NullPointerException if either version, the dataset in the version or the format are null - * @throws IllegalArgumentException if the format name is blank or empty - */ public ExportCacheKey(DatasetVersion version, String formatName) { - this(Objects.requireNonNull(version).getDataset(), version, formatName); + this(auxTag(version, formatName)); } /** The one canonical, version-qualified aux tag. */ - public String auxTag() { - return "export_" + formatName + "_" + version.getFriendlyVersionNumber() + ".cached"; + static String auxTag(DatasetVersion version, String formatName) { + Objects.requireNonNull(version); + if (Objects.requireNonNull(formatName).isBlank()) { + throw new IllegalArgumentException("formatName must not be blank or empty"); + } + return TAG_PREFIX + formatName + "_" + version.getFriendlyVersionNumber() + TAG_SUFFIX; } } diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java b/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java index e9367f2b3ad..2afd652e384 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/FileEmbargoExpiryInvalidator.java @@ -1,6 +1,6 @@ package edu.harvard.iq.dataverse.export.service; -import edu.harvard.iq.dataverse.Dataset; +import edu.harvard.iq.dataverse.DatasetVersion; import edu.harvard.iq.dataverse.Embargo; import edu.harvard.iq.dataverse.FileMetadata; @@ -24,16 +24,36 @@ public final class FileEmbargoExpiryInvalidator implements ExportCacheInvalidato private static final Logger logger = Logger.getLogger(FileEmbargoExpiryInvalidator.class.getCanonicalName()); @Override - public boolean isStale(ExportCacheKey key) { - return isStaleDueToExpiredEmbargo(key.dataset()); + public boolean isStale(DatasetVersion datasetVersion, ExportCacheKey key) { + if (datasetVersion == null) { + throw new IllegalArgumentException("datasetVersion cannot be null"); + } + if (key == null) { + throw new IllegalArgumentException("key cannot be null"); + } + + return isStaleDueToExpiredEmbargo(datasetVersion); } /** * Checks whether a cached export has been rendered stale because an embargo * on one of the dataset's files ended after the last export ran. */ - private boolean isStaleDueToExpiredEmbargo(Dataset dataset) { - Date lastExportDate = dataset.getLastExportTime(); + private boolean isStaleDueToExpiredEmbargo(DatasetVersion datasetVersion) { + if (datasetVersion.getDataset() == null) { + throw new IllegalArgumentException("datasetVersion must have a dataset associated and cannot be null"); + } + // Only released or archived versions can have expired embargoes + // (See also Dataset.getLatestVersionForCopy(), which was used before within the original code) + if (!datasetVersion.isReleased() && !datasetVersion.isArchived()) { + return false; + } + + // The following code was originally contained in ExportServiceBean and written by @landreev. + // Its limitation to the DDI format was lifted, as other formats supporting file metadata may benefit from it as well. + // Also, it now uses the given dataset version, no longer receiving it by itself from the dataset. + + Date lastExportDate = datasetVersion.getDataset().getLastExportTime(); // if lastExportDate == null, assume it's not set because we're exporting for the // first time now (e.g. during publish) and therefore no changes are needed if (lastExportDate == null) { @@ -43,8 +63,8 @@ private boolean isStaleDueToExpiredEmbargo(Dataset dataset) { logger.fine("Last export date: " + exportLocalDate); // Track which embargoes we've already checked Set embargoIds = new HashSet<>(); - // Check for all files in the latest released version - for (FileMetadata fm : dataset.getLatestVersionForCopy().getFileMetadatas()) { + // Check for all files in the given version + for (FileMetadata fm : datasetVersion.getFileMetadatas()) { // ToDo? This loop is necessary because we have not stored the date when the // next embargo in this datasetversion will end. If we knew that (another // dataset/datasetversion column), we could make one check that nextembargoEnd diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java index 21d0f6b2ebd..0d29d7899ba 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -30,6 +30,7 @@ *

    * Write Atomicity: Exports are always rendered to a local temp file first. * Then it gets persisted via {@link StorageIO#savePathAsAux(Path, String)} as an auxiliary dataset file. + * To make it thread-safe end-to-end, the underlying storage drivers must support atomic writes. *

    * Note: This class replaces the former {@code ExportService.cacheExport()} method, mostly written by qqmyers. * Instead of its "try openAuxChannel, fall back to temp file for S3/Swift" branching, there now is one code path for all drivers. @@ -47,25 +48,25 @@ public final class StorageIOCache implements ExportCache { private static final Logger logger = Logger.getLogger(StorageIOCache.class.getCanonicalName()); - private static final String TAG_PREFIX = "export_"; - private static final String TAG_SUFFIX = ".cached"; - /** * Reads an input stream associated with the given export cache key. * - * @param key the export cache key containing dataset, format, and versioning information. + * @param dataset The dataset associated with the export cache key, used to determine storage access. + * @param key The export cache key containing dataset, format, and versioning information. * @return an {@code Optional} containing the input stream if available, otherwise an empty {@code Optional}. * @throws IOException if an I/O error occurs while attempting to read the data. */ @Override - public Optional read(ExportCacheKey key) throws IOException { - StorageIO storage = storageFor(key.dataset()); + public Optional read(Dataset dataset, ExportCacheKey key) throws IOException { + StorageIO storage = storageFor(dataset); return tryRead(storage, key.auxTag()); } /** * Writes the export cache data to a temporary file and ensures it is properly persisted to the dataset's storage. * Handles file cleanup to maintain system integrity. + * + * @param dataset The dataset associated with the export cache key, used to determine storage access. * @param key The {@code ExportCacheKey} representing the metadata export about to be cached. * @param writer The {@code ExportStreamWriter} functional interface implementation responsible for writing data * to the output stream. This wraps the underlying exporter, writing the actual data format. @@ -73,7 +74,7 @@ public Optional read(ExportCacheKey key) throws IOException { * @throws IOException If an I/O error occurs while creating, writing, or managing the temporary file. */ @Override - public void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException { + public void write(Dataset dataset, ExportCacheKey key, ExportStreamWriter writer) throws ExportException, IOException { Path tempFile = SecureTempFiles.createOwnerOnlyTempFile("dataverse-export-", ".tmp"); try { // No catch here (checked exception), but closing the stream after use, avoiding leaks. @@ -82,21 +83,22 @@ public void write(ExportCacheKey key, ExportStreamWriter writer) throws ExportEx } // Persist to storage only after the metadata export has been fully and successfully rendered. // A failure above leaves the cache untouched. - storageFor(key.dataset()).savePathAsAux(tempFile, key.auxTag()); - logger.log(Level.FINE, key.version() + ": Cached export written: {0}", key.auxTag()); + // TODO: verify for all storage drivers that they support atomic writes. + storageFor(dataset).savePathAsAux(tempFile, key.auxTag()); + logger.log(Level.FINE, dataset.getId() + ": Cached export written: {0}", key.auxTag()); } finally { try { Files.deleteIfExists(tempFile); } catch (IOException e) { // Warn, but do not fail if the temp file could not be deleted. (The main operation was a success) - logger.log(Level.WARNING, e, () -> key.version() + ": could not delete export temp file " + tempFile); + logger.log(Level.WARNING, e, () -> dataset.getId() + ": could not delete export temp file " + tempFile); } } } @Override - public void evict(ExportCacheKey key) throws IOException { - deleteQuietly(storageFor(key.dataset()), key.auxTag()); + public void evict(Dataset dataset, ExportCacheKey key) throws IOException { + deleteQuietly(storageFor(dataset), key.auxTag()); } @Override @@ -104,7 +106,7 @@ public void evictAll(Dataset dataset) throws IOException { StorageIO storage = storageFor(dataset); List auxTags = storage.listAuxObjects(); for (String tag : auxTags) { - if (tag.startsWith(TAG_PREFIX) && tag.endsWith(TAG_SUFFIX)) { + if (tag.startsWith(ExportCacheKey.TAG_PREFIX) && tag.endsWith(ExportCacheKey.TAG_SUFFIX)) { deleteQuietly(storage, tag); } } From 01e1e9507584c4a1ec73632969c825977c332ca7 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Thu, 20 Aug 2026 17:51:13 +0200 Subject: [PATCH 20/39] refactor(export): replace depth-based comparator with transitive dependents set #11405 - Renamed `formatRequiredBy` to `transitiveDependents`, changing the value type from `List` to `Set` to capture all direct and transitive dependents per format. - Replaced `buildPrerequisitesChainDepth` with `buildTransitiveDependents`, which walks each exporter's prerequisite chain and registers it as a dependent of every ancestor format. - Updated `buildTopologicalComparator` to sort by new dependent-set - Merged `buildFormatRequiredByMap` into `verifyRequirements` as the former map is no longer stored for reuse - Moved `getFormatsDependingOn` to `getTransitiveDependents` to reflect the new semantics --- .../export/service/ExporterRegistryBean.java | 152 ++++++++---------- 1 file changed, 66 insertions(+), 86 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index a4812b1abcb..d5f5d88f335 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -78,9 +78,12 @@ public record Labels( // when implementing a reload mechanism. private Map exporters = Map.of(); - // Caching the requirements as a map (key = format, value = list of formats that require this format). + // Map of direct and transitive dependents per format. + // Serves eviction and export cascades and, via Set::size, the topological comparator. + // Format: Key = format, Value = all formats that directly or indirectly declare it as a prerequisite + // Rules: An empty set equals a leaf, self is never included in the set. // Managed the same way as the exporter map. - private Map> formatRequiredBy = Map.of(); + private Map> transitiveDependents = Map.of(); // Comparator imposing a topologically consistent order on exporters, derived from prereqDepthByFormat. // Managed the same way as the exporter map. Initialized with empty Map for consistency. @@ -159,26 +162,25 @@ public void requireAllExist(List formats) { } /** - * Retrieves the list of export format names that depend on the given format as a prerequisite. + * Retrieves all export formats that depend on the given format as a prerequisite, directly or transitively. * * @param format the name of the format for which dependent formats are to be resolved. - * @return a list of format names of exporters that require the specified format as a prerequisite, - * or an empty list if no such dependencies exist + * @return an unmodifiable set of format names of exporters requiring the specified format somewhere in their + * prerequisite chain, or an empty set if none do */ - public List getFormatsDependingOn(String format) { - return this.formatRequiredBy.getOrDefault(format, Collections.emptyList()); + public Set getTransitiveDependents(String format) { + return this.transitiveDependents.getOrDefault(format, Set.of()); } /** * Returns a {@link Comparator} that orders {@link Exporter}s such that every prerequisite format sorts before - * all exporters depending on it (directly or transitively). + * all export formats depending on it (directly or transitively). *

    - * The comparator sorts on the cached prerequisite chain depth (see {@link #buildPrerequisitesChainDepth(Map)}) - * rather than comparing prerequisite relations directly: the {@code Comparator} contract requires a total, - * transitive ordering, while "is a prerequisite of" is only a partial order - unrelated exporters would - * compare as equal, allowing a sort to place a transitive dependent before its prerequisite. - * Depth turns the partial order into a total order that still respects all prerequisite constraints. - * Ties are broken by format name for deterministic results. + * The comparator sorts on the cached number of transitive dependents rather than comparing prerequisite + * relations directly: the {@code Comparator} contract requires a total, transitive ordering, while "is a prerequisite of" + * is only a partial order. The dependent count induces a valid total order because a prerequisite's dependent set + * is always a strict superset of each of its dependents' sets (it contains at least the dependent itself), + * so it always sorts first. Ties (unrelated exporters) are broken by format name for deterministic results. *

    * The returned comparator is immutable, thread-safe, and reflects the registry state (at startup or when refreshed). *

    @@ -254,15 +256,15 @@ private void initialize() { }); // Step 4 - Create prerequisite dependency graph and verify integrity - var requiredBy = buildAndVerifyRequirements(loadedExporters); + verifyRequirements(loadedExporters); - // Step 5 - Build map of prerequisite dependency graph depth per format and the comparator - var prerequisitesDepth = buildPrerequisitesChainDepth(loadedExporters); - var comparator = buildTopologicalComparator(prerequisitesDepth); + // Step 5 - Build the transitive dependents map and derive the comparator from it + var dependents = buildTransitiveDependents(loadedExporters); + var comparator = buildTopologicalComparator(dependents); // All good, (more or less) atomic updates now. this.exporters = loadedExporters; - this.formatRequiredBy = requiredBy; + this.transitiveDependents = dependents; this.topologicalComparator = comparator; } @@ -280,45 +282,25 @@ private void tearDown() { } /** - * Builds a map of prerequisite format names to the list of export formats that depend on them. - * For each registered exporter that declares a prerequisite format, the exporter's format name is collected - * under the prerequisite key. (Thus exporters without a prerequisite are not included.) + * Builds the prerequisite dependency map from the given exporters and verifies that every prerequisite format + * referenced by an exporter is itself backed by a registered exporter in the provided map. + * In addition, it verifies no prerequisite formats form a cyclic dependency. * - * @return a map where each key is a prerequisite format name and each value is the list of format names of - * exporters that require that prerequisite; an empty map if no exporter declares a prerequisite + * @throws ExportException if one or more prerequisite format names in the dependency map + * do not have a corresponding entry in the provided exporters map */ - static Map> buildFormatRequiredByMap(Map exporters) { + static void verifyRequirements(Map exporters) { Objects.requireNonNull(exporters); - Map> requiredByMap = new HashMap<>(); + Map> formatRequiredBy = new HashMap<>(); for (Exporter exporter : exporters.values()) { exporter.getPrerequisiteFormatName().ifPresent(prereq -> - requiredByMap + formatRequiredBy // Create new list if necessary .computeIfAbsent(prereq, k -> new ArrayList<>()) // Put down exporter as depending on this format .add(exporter.getFormatName())); } - - // Make a deep, read-only copy before returning - return requiredByMap.entrySet().stream() - .collect(Collectors.toUnmodifiableMap( - Map.Entry::getKey, - entry -> List.copyOf(entry.getValue()) - )); - } - - /** - * Builds the prerequisite dependency map from the given exporters and verifies that every prerequisite format - * referenced by an exporter is itself backed by a registered exporter in the provided map. - * In addition, it verifies no prerequisite formats form a cyclic dependency. - * - * @return the built prerequisite dependency map, see {@link #buildFormatRequiredByMap(Map)}. - * @throws ExportException if one or more prerequisite format names in the dependency map - * do not have a corresponding entry in the provided exporters map - */ - static Map> buildAndVerifyRequirements(Map exporters) { - Map> formatRequiredBy = buildFormatRequiredByMap(exporters); // Check that all prerequisite formats have a registered exporter if (!exporters.keySet().containsAll(formatRequiredBy.keySet())) { @@ -356,56 +338,54 @@ static Map> buildAndVerifyRequirements(Map - *

  • a depth of 0 means the exporter has no prerequisite format,
  • - *
  • a depth of 1 means it depends on a format that itself has no prerequisite,
  • - *
  • and so on for longer chains.
  • - * - * - * @param exportersByFormat a map from format name to its associated {@link Exporter} instance; must not be null - * @return an unmodifiable map where each key is a format name and each value is the integer depth of the - * prerequisite chain for that format; the map contains one entry per export format in the input + * Builds a map from every format name to the set of formats that depend on it, directly or transitively. + * Every registered format has an entry (empty set for formats nothing depends on). + * In addition, a format is never a member of its own set. + *

    + * Precondition: {@code exporters} must have passed {@link #verifyRequirements(Map)}, as the chain walk + * assumes all prerequisites are registered and cycle-free. */ - static Map buildPrerequisitesChainDepth(Map exportersByFormat) { - Objects.requireNonNull(exportersByFormat); - Map depthsByFormat = new HashMap<>(); - for (Exporter e : exportersByFormat.values()) { - depthOf(e, exportersByFormat, depthsByFormat); + static Map> buildTransitiveDependents(Map exporters) { + Objects.requireNonNull(exporters); + Map> dependents = new HashMap<>(); + // Ensure an entry for every format, including leaves. + exporters.keySet().forEach(name -> dependents.put(name, new HashSet<>())); + + // Each exporter has at most one prerequisite, so its ancestors form a simple chain: + // register the exporter as a dependent of every format on that chain. + for (Exporter exporter : exporters.values()) { + String dependent = exporter.getFormatName(); + Optional prereq = exporter.getPrerequisiteFormatName(); + while (prereq.isPresent()) { + Exporter ancestor = exporters.get(prereq.get()); + dependents.get(ancestor.getFormatName()).add(dependent); + prereq = ancestor.getPrerequisiteFormatName(); + } } - return Map.copyOf(depthsByFormat); - } - - // Note: Make sure no cyclomatic format dependencies exist in exporters, otherwise infinite recursion may occur! - private static int depthOf(Exporter e, Map exportersByFormat, Map depthsByFormat) { - // If the depth map does not already contain the depth value, compute it recursively, then return it. - return depthsByFormat.computeIfAbsent( - e.getFormatName(), - // Note: the following operates on Optional.map(), not Stream.map()! - name -> e.getPrerequisiteFormatName() - .map(exportersByFormat::get) - .map(prereq -> depthOf(prereq, exportersByFormat, depthsByFormat) + 1) - // As no value could be found, return 0 = no prerequisite format - .orElse(0)); + + // Deep, read-only copy + return dependents.entrySet().stream() + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, e -> Set.copyOf(e.getValue()))); } /** - * Creates a comparator ordering exporters by their prerequisite format chain depth, with format name as tiebreak. - * Exporters not present in the given depth map (which should not occur for registered exporters) are treated - * as having no prerequisite (depth 0). See {@link #getTopologicalComparator()} for the rationale. + * Creates a comparator ordering exporters by their number of transitive dependents in descending order. + * (Prerequisites carry strictly more dependents than anything depending on them and thus sort first.) + * The format name is used as tiebreak. + * Formats absent from the map (which should not occur for registered exporters) are treated as having no + * dependents and sort last among ties. * - * @param depthsByFormat map from format name to prerequisite chain depth; must not be null + * @param dependentsByFormat map from format name to its transitive dependents; must not be null * @return an immutable, thread-safe comparator */ - static Comparator buildTopologicalComparator(Map depthsByFormat) { - Objects.requireNonNull(depthsByFormat); - return Comparator.comparingInt((Exporter e) -> depthsByFormat.getOrDefault(e.getFormatName(), 0)) + static Comparator buildTopologicalComparator(Map> dependentsByFormat) { + Objects.requireNonNull(dependentsByFormat); + return Comparator.comparingInt( + (Exporter e) -> dependentsByFormat.getOrDefault(e.getFormatName(), Set.of()).size()) + .reversed() // inversed order as the more transitive dependents, the earlier it needs to be processed! .thenComparing(Exporter::getFormatName); } } From 031a6a0386eeb75bc6b616c615e754c634042d59 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 02:10:42 +0200 Subject: [PATCH 21/39] refactor(export): replace Labels record with sealed Details interface #11405 - Introduced sealed `Details` interface exposing `localizedDisplayName`, `formatName`, `mediaType`, `isHarvestable`, and `isAvailableToUsers`, thus avoiding having to retrieve these details from the exporter, saving a roundtrip. - Made `ExporterDetails` record package-private to prevent external instantiation while allowing consumers to read via the interface. - Renamed `getLabels()` to `getDetails()`, returning `List

    ` with the expanded field set. - Added `get(Details)` lookup method to resolve an exporter by its details object. These can only be created and handed out by the registry, thus we can be sure a matching exporter exists. - Removed unused `Collections` import. --- .../export/service/ExporterRegistryBean.java | 47 +++++++++++++++---- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index d5f5d88f335..cd750d970ce 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -19,7 +19,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; -import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; @@ -65,10 +64,22 @@ public class ExporterRegistryBean { /** * Represents a set of labels associated with an exporter. */ - public record Labels( + public sealed interface Details permits ExporterDetails { + String localizedDisplayName(); + String formatName(); + String mediaType(); + boolean isHarvestable(); + boolean isAvailableToUsers(); + } + + // Package-private to disable creating details records from outside this class/package + record ExporterDetails ( String localizedDisplayName, - String formatName - ) {} + String formatName, + String mediaType, + boolean isHarvestable, + boolean isAvailableToUsers + ) implements Details {} private static final Logger logger = Logger.getLogger(ExporterRegistryBean.class.getCanonicalName()); @@ -104,6 +115,20 @@ public Optional get(String formatName) { return Optional.ofNullable(exporters.get(formatName)); } + /** + * Retrieves an exporter by the format name specified in the given details. + * + * @param detail the details containing the format name used to look up the exporter; must not be null + * @return the exporter associated with the format name from the provided details + * @throws IllegalArgumentException if the detail parameter is null + */ + public Exporter get(Details detail) { + if (detail == null) { + throw new IllegalArgumentException("Exporter details cannot be null"); + } + return exporters.get(detail.formatName()); + } + /** * Retrieves a list of all registered exporters in the system. * @return an unmodifiable list of {@link Exporter} instances representing all the exporters currently available @@ -113,14 +138,18 @@ public List getAll() { } /** - * Retrieves a list of {@link Labels} representing the exporters registered in the system. - * @return a list of {@code Labels} objects + * Retrieves a list of {@link Details} representing the exporters registered in the system. + * @return a list of {@code Details} objects */ - public List getLabels() { + public List
    getDetails() { return exporters.values().stream() - .map(exporter -> new Labels( + .
    map(exporter -> new ExporterDetails( exporter.getDisplayName(BundleUtil.getCurrentLocale()), - exporter.getFormatName())) + exporter.getFormatName(), + exporter.getMediaType(), + exporter.isHarvestable(), + exporter.isAvailableToUsers() + )) .toList(); } From fdac53eca37d1a166c1f7d2415d17c3d6f12c086 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 02:11:12 +0200 Subject: [PATCH 22/39] style(export): convert field comments to block comment style for readability --- .../export/service/ExporterRegistryBean.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index cd750d970ce..9fee54499dd 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -83,25 +83,29 @@ record ExporterDetails ( private static final Logger logger = Logger.getLogger(ExporterRegistryBean.class.getCanonicalName()); - // When the class is initialized, the exporter map is an empty, non-modifiable map (key = format name). - // Once the exporters have been located and loaded, the map is replaced, fully loaded, still unmodifiable. - // No half-initialized state is exposable this way. Future optimizations may use @Lock on it, too, for example, - // when implementing a reload mechanism. + /* When the class is initialized, the exporter map is an empty, non-modifiable map (key = format name). + * Once the exporters have been located and loaded, the map is replaced, fully loaded, still unmodifiable. + * No half-initialized state is exposable this way. Future optimizations may use @Lock on it, too, for example, + * when implementing a reload mechanism. + */ private Map exporters = Map.of(); - // Map of direct and transitive dependents per format. - // Serves eviction and export cascades and, via Set::size, the topological comparator. - // Format: Key = format, Value = all formats that directly or indirectly declare it as a prerequisite - // Rules: An empty set equals a leaf, self is never included in the set. - // Managed the same way as the exporter map. + /* Map of direct and transitive dependents per format. + * Serves eviction and export cascades and, via Set::size, the topological comparator. + * Format: Key = format, Value = all formats that directly or indirectly declare it as a prerequisite + * Rules: An empty set equals a leaf, self is never included in the set. + * Managed the same way as the exporter map. + */ private Map> transitiveDependents = Map.of(); - // Comparator imposing a topologically consistent order on exporters, derived from prereqDepthByFormat. - // Managed the same way as the exporter map. Initialized with empty Map for consistency. + /* Comparator imposing a topologically consistent order on exporters, derived from prereqDepthByFormat. + * Managed the same way as the exporter map. Initialized with empty Map for consistency. + */ private Comparator topologicalComparator = buildTopologicalComparator(Map.of()); - // Caching the classloader used to load plugin JAR files, keeping it open, will allow reuse for reloads - // or loading more resources from plugin JARs. May be dropped later if not necessary. + /* Caching the classloader used to load plugin JAR files, keeping it open, will allow reuse for reloads + * or loading more resources from plugin JARs. May be dropped later if not necessary. + */ private URLClassLoader exporterClassLoader; /** From 95d08f1253abd4db3b8a87d8786b5084ca12d6ee Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 02:30:18 +0200 Subject: [PATCH 23/39] refactor(export): store formatName and friendlyVersion again as ExportCacheKey components #11405 - Replaced the single `auxTag` field with `formatName` and `friendlyVersion` so the key exposes its meaningful parts directly. - Moved `auxTag()` from a static factory into an instance method derived from the record's fields. - Split validation into `checkFormatName` and `checkVersion` private helpers for clearer intent (and compatibility with the constructor needing to be called first thing). --- .../dataverse/export/service/ExportCacheKey.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java index 042a6a4658a..4fbbd600aa0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportCacheKey.java @@ -14,7 +14,7 @@ * The cache itself derives the target auxiliary storage (dataset or datafile) at runtime. * In addition, by not keeping an JPA entity reference, garbage collection is facilitated. */ -public record ExportCacheKey(String auxTag) { +public record ExportCacheKey(String formatName, String friendlyVersion) { public static final String TAG_PREFIX = "export_"; public static final String TAG_SUFFIX = ".cached"; @@ -27,15 +27,23 @@ public record ExportCacheKey(String auxTag) { * @throws IllegalArgumentException if the formatName is blank or empty */ public ExportCacheKey(DatasetVersion version, String formatName) { - this(auxTag(version, formatName)); + this(checkFormatName(formatName), checkVersion(version)); } /** The one canonical, version-qualified aux tag. */ - static String auxTag(DatasetVersion version, String formatName) { + public String auxTag() { + return TAG_PREFIX + formatName + "_" + friendlyVersion + TAG_SUFFIX; + } + + private static String checkVersion(DatasetVersion version) { Objects.requireNonNull(version); + return Objects.requireNonNull(version.getFriendlyVersionNumber()); + } + + private static String checkFormatName(String formatName) { if (Objects.requireNonNull(formatName).isBlank()) { throw new IllegalArgumentException("formatName must not be blank or empty"); } - return TAG_PREFIX + formatName + "_" + version.getFriendlyVersionNumber() + TAG_SUFFIX; + return formatName; } } From 77fd70efc28aa95a56ec7dfb8fe2d8483ab87a65 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 12:44:28 +0200 Subject: [PATCH 24/39] refactor(export): remove obsolete exporter lookup methods from ExportServiceBean #11405 These methods (`getExporter`, `isXMLFormat`, `getMediaType`) directly exposed the internal `exporterMap` and are no longer needed now that format details are resolved via the `Details` interface in the registry. --- .../export/service/ExportServiceBean.java | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 4ab09d675d2..234f99bf1b9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -360,29 +360,9 @@ public void exportFormat(Dataset dataset, String formatName) throws ExportExcept } } - - public Exporter getExporter(String formatName) throws ExportException { - Exporter e = exporterMap.get(formatName); - if (e != null) { - return e; } - throw new ExportException("No such Exporter: " + formatName); } - public Boolean isXMLFormat(String provider) { - Exporter e = exporterMap.get(provider); - if (e != null) { - return e instanceof XMLExporter; - } - return null; - } - - public String getMediaType(String provider) { - Exporter e = exporterMap.get(provider); - if (e != null) { - return e.getMediaType(); - } - return MediaType.TEXT_PLAIN; } /** From c59d2be1d8213222b532bf0da739a718b57f514e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 12:57:46 +0200 Subject: [PATCH 25/39] fix(export): guard against null formatName in ExporterRegistryBean#get #11405 Added null check in `get(String formatName)` to return `Optional.empty()` instead of throwing NPE when the underlying Map implementation does not permit null keys. --- .../iq/dataverse/export/service/ExporterRegistryBean.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java index 9fee54499dd..7e4feec1797 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExporterRegistryBean.java @@ -116,6 +116,10 @@ record ExporterDetails ( * an empty {@code Optional} if no exporter is associated with the given format name */ public Optional get(String formatName) { + // Avoid NPE being thrown from Map lookup when Map implementation does not permit null keys + if (formatName == null) { + return Optional.empty(); + } return Optional.ofNullable(exporters.get(formatName)); } @@ -129,7 +133,7 @@ public Optional get(String formatName) { public Exporter get(Details detail) { if (detail == null) { throw new IllegalArgumentException("Exporter details cannot be null"); - } + } return exporters.get(detail.formatName()); } From d1403aa6e073b8be5ff02d9bd9bfd419884c572b Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 15:02:04 +0200 Subject: [PATCH 26/39] feat(util): add FailureEscalation for threshold-based log level escalation - Tracks consecutive failures via an `AtomicInteger` streak; escalates from `FINE` to `WARNING` once the streak reaches the configured threshold. - A success resets the streak; a threshold of zero or negative deactivates escalation entirely. - Thread-safe and suitable for sharing across concurrent callers or use in `ConcurrentHashMap` contexts. - Warnings will not be flooding the log once threshold is reached via configurable repeat cycle. - To enable "all clear" messages once the threshold was met, the success recording may then return the number of failures. Using OptionalInt, the logging statement is a one-liner. --- .../util/logging/FailureEscalation.java | 86 ++++++++ .../util/logging/FailureEscalationTest.java | 200 ++++++++++++++++++ 2 files changed, 286 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/util/logging/FailureEscalation.java create mode 100644 src/test/java/edu/harvard/iq/dataverse/util/logging/FailureEscalationTest.java diff --git a/src/main/java/edu/harvard/iq/dataverse/util/logging/FailureEscalation.java b/src/main/java/edu/harvard/iq/dataverse/util/logging/FailureEscalation.java new file mode 100644 index 00000000000..2572f6236df --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/util/logging/FailureEscalation.java @@ -0,0 +1,86 @@ +package edu.harvard.iq.dataverse.util.logging; + +import java.util.OptionalInt; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Level; + +/** + * Tracks a streak of consecutive failures and escalates the logging level once a threshold is exceeded. + * A success resets the streak. + *

    + * Once escalated, only the first failure and every {@code repeatEvery}-th subsequent failure return + * {@link Level#WARNING}; failures in between are demoted to {@link Level#FINE} to avoid flooding the log + * (they remain visible at FINE for debugging). + *

    + * If the threshold is set to 0 or a negative value, escalation is deactivated. + *

    + * {@link #recordSuccess()} reports whether the cleared streak had been escalated, so the caller can log + * a recovery message — otherwise the log would show escalations without ever showing the recovery. + *

    + * Instances are thread-safe and may be shared across concurrent callers and used in other, + * thread-safe contexts like {@code ConcurrentHashMap}. + */ + +public final class FailureEscalation { + private final AtomicInteger streak = new AtomicInteger(); + private final int threshold; + private final int repeatEvery; + + /** + * @param threshold consecutive failures required before escalation; 0 or negative deactivates escalation; + * makes escalation repeat every this-many failures + */ + public FailureEscalation(int threshold) { + this.threshold = threshold; + this.repeatEvery = threshold; // we don't care about negative or 0, as escalation is deactivated anyway + } + + /** + * @param threshold consecutive failures required before escalation; 0 or negative deactivates escalation + * @param repeatEvery once escalated, log at WARNING only every this-many failures (minimum 1 = every failure) + */ + public FailureEscalation(int threshold, int repeatEvery) { + this.threshold = threshold; + this.repeatEvery = Math.max(1, repeatEvery); + } + + /** + * Record a failure and return the level to log it at. + */ + public Level incrementAndGetLevel() { + // Deactivated: skip all bookkeeping, no map entries are ever created. + if (threshold < 1) { + return Level.FINE; + } + // When repeatEvery is smaller than threshold, we must refrain from escalating, as the modulo operation would + // generate 0 for some failure counts smaller than threshold. + // Example: (1 - 5) % 4 = 0 (count=1, threshold=5, repeatEvery=4) + if (streak.incrementAndGet() < threshold) { + return Level.FINE; + } + // Escalated: warn on the first hit and every repeatEvery-th afterwards, demote the rest. + return (streak.get() - threshold) % repeatEvery == 0 ? Level.WARNING : Level.FINE; + } + + /** + * Record a success, resetting the streak. + * + * @return The length of the just-cleared streak, if it had reached the escalation threshold. + * The caller should log a recovery message in that case + * (e.g. via {@code recordSuccess().ifPresent(n -> logger.warning(...))}). + * Empty otherwise. + */ + public OptionalInt recordSuccess() { + int previous = streak.getAndSet(0); + return (threshold > 0 && previous >= threshold) + ? OptionalInt.of(previous) + : OptionalInt.empty(); + } + + /** + * Current streak length; intended for metrics gauges. + */ + public int currentStreak() { + return streak.get(); + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/util/logging/FailureEscalationTest.java b/src/test/java/edu/harvard/iq/dataverse/util/logging/FailureEscalationTest.java new file mode 100644 index 00000000000..eda7c9be6b0 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/logging/FailureEscalationTest.java @@ -0,0 +1,200 @@ +package edu.harvard.iq.dataverse.util.logging; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.OptionalInt; +import java.util.logging.Level; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class FailureEscalationTest { + + @Nested + class DeactivatedEscalation { + + @ParameterizedTest + @ValueSource(ints = {0, -5}) + void alwaysReturnsFine(int threshold) { + FailureEscalation escalation = new FailureEscalation(threshold); + + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); + } + + @Test + void recordSuccessNeverReportsRecovery() { + FailureEscalation escalation = new FailureEscalation(0); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + + assertTrue(escalation.recordSuccess().isEmpty()); + } + + @Test + void keepsNoBookkeeping() { + FailureEscalation escalation = new FailureEscalation(0); + escalation.incrementAndGetLevel(); + + assertEquals(0, escalation.currentStreak()); + } + } + + @Nested + class EscalationThreshold { + + @Test + void staysFineBelowThreshold() { + FailureEscalation escalation = new FailureEscalation(3); + + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); + } + + @Test + void warnsExactlyAtThreshold() { + FailureEscalation escalation = new FailureEscalation(3); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + } + + @Test + void thresholdOneWarnsOnFirstFailure() { + FailureEscalation escalation = new FailureEscalation(1); + + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + } + + @Test + void smallRepeatEveryMustNotWarnBelowThreshold() { + // Regression test: (count - threshold) % repeatEvery can be zero below the + // threshold; without the explicit guard this warned on the very first failure. + FailureEscalation escalation = new FailureEscalation(3, 1); + + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + } + } + + @Nested + class FloodSuppression { + + @Test + void demotesBetweenRepeatsAndWarnsOnEveryNth() { + FailureEscalation escalation = new FailureEscalation(2, 3); + + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); // 1: below threshold + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); // 2: threshold hit + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); // 3: suppressed + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); // 4: suppressed + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); // 5: repeat + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); // 6: suppressed + } + + @Test + void repeatEveryOneWarnsOnEveryEscalatedFailure() { + FailureEscalation escalation = new FailureEscalation(2, 1); + escalation.incrementAndGetLevel(); + + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + } + + @Test + void repeatEveryBelowOneIsClampedToOne() { + FailureEscalation escalation = new FailureEscalation(1, 0); + + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); + } + + @Test + void singleArgConstructorRepeatsEveryThresholdFailures() { + FailureEscalation escalation = new FailureEscalation(2); + escalation.incrementAndGetLevel(); + + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); // 2: threshold hit + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); // 3: suppressed + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); // 4: repeat + } + } + + @Nested + class Recovery { + + @Test + void successWithoutAnyFailuresReportsNothing() { + FailureEscalation escalation = new FailureEscalation(2); + + assertTrue(escalation.recordSuccess().isEmpty()); + } + + @Test + void successBelowThresholdReportsNothing() { + FailureEscalation escalation = new FailureEscalation(3); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + + assertTrue(escalation.recordSuccess().isEmpty()); + } + + @Test + void successAfterEscalationReportsClearedStreakLength() { + FailureEscalation escalation = new FailureEscalation(2); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + + assertEquals(OptionalInt.of(3), escalation.recordSuccess()); + } + + @Test + void successResetsTheStreak() { + FailureEscalation escalation = new FailureEscalation(2); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + escalation.recordSuccess(); + + assertEquals(Level.FINE, escalation.incrementAndGetLevel()); // streak restarted at 1 + assertEquals(Level.WARNING, escalation.incrementAndGetLevel()); // threshold applies anew + } + + @Test + void secondSuccessDoesNotReportRecoveryTwice() { + FailureEscalation escalation = new FailureEscalation(1); + escalation.incrementAndGetLevel(); + escalation.recordSuccess(); + + assertTrue(escalation.recordSuccess().isEmpty()); + } + } + + @Nested + class StreakGauge { + + @Test + void reflectsFailureCount() { + FailureEscalation escalation = new FailureEscalation(5); + escalation.incrementAndGetLevel(); + escalation.incrementAndGetLevel(); + + assertEquals(2, escalation.currentStreak()); + } + + @Test + void resetsToZeroOnSuccess() { + FailureEscalation escalation = new FailureEscalation(5); + escalation.incrementAndGetLevel(); + escalation.recordSuccess(); + + assertEquals(0, escalation.currentStreak()); + } + } +} \ No newline at end of file From 74680989ab2eb9734ecfc299b81731b60f333ebe Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 15:50:11 +0200 Subject: [PATCH 27/39] docs(export): correct legacy cache name behavior in StorageIOCache Javadoc #11405 - Clarified that the legacy unqualified name is ignored for read/write cycles and only purged via `evictAll`, rather than being a read fallback. - Fix typos --- .../harvard/iq/dataverse/export/service/StorageIOCache.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java index 0d29d7899ba..223283b3971 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -25,8 +25,8 @@ * see {@link ExportCacheKey#auxTag()}) and is the only name ever written. *

    * The legacy, unqualified name ({@code export_.cached}) predates version qualification and only ever described - * the latest released version. It is therefore consulted as a read fallback exclusively for that version. - * It will be deleted alongside the canonical name on eviction, so a stale legacy entry can never resurrect an invalidated export. + * the latest released version. It is ignored by this cache implementation for read/write cycles but may + * be purged using {@link #evictAll(Dataset)}. *

    * Write Atomicity: Exports are always rendered to a local temp file first. * Then it gets persisted via {@link StorageIO#savePathAsAux(Path, String)} as an auxiliary dataset file. @@ -37,7 +37,7 @@ * Readers can never observe a half-written export under the cache key. The cost is one extra local write per export, * which is negligible next to export generation itself. *

    - * Note 2: This class is an application scoped CDI bean (single instance). The cache itself is stateless, + * Note 2: This class is an application-scoped CDI bean (single instance). The cache itself is stateless, * and every operation operates on their own {@code StorageIO}. But: if we add a write lock later on to avoid race * conditions during writes, we will require an instance wide single map to store these locks, which CDI gives us for free. * In addition, one might use a Hazelcast-backed map to acquire multi-instance wide locks! From bbe8cd9f6c37a805c00c69d68e17a75a453bfb08 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 15:51:05 +0200 Subject: [PATCH 28/39] feat(export): apply FailureEscalation to StorageIOCache log levels #11405 - Replace static `FINE`-level logging in `tryRead` and `deleteQuietly` with threshold-based escalation via `FailureEscalation` instances (threshold: 256). - Log a recovery warning once consecutive failures drop below the threshold after previously exceeding it. - Include the current failure streak in the read-path log message for operational context. --- .../dataverse/export/service/StorageIOCache.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java index 223283b3971..9a782f765fb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/StorageIOCache.java @@ -4,6 +4,7 @@ import edu.harvard.iq.dataverse.dataaccess.DataAccess; import edu.harvard.iq.dataverse.dataaccess.StorageIO; import edu.harvard.iq.dataverse.util.SecureTempFiles; +import edu.harvard.iq.dataverse.util.logging.FailureEscalation; import io.gdcc.spi.export.ExportException; import jakarta.enterprise.context.ApplicationScoped; @@ -48,6 +49,10 @@ public final class StorageIOCache implements ExportCache { private static final Logger logger = Logger.getLogger(StorageIOCache.class.getCanonicalName()); + // TODO: these hard coded thresholds are arbitrarily high and should be configurable via JvmSettings + private static final FailureEscalation quietDeleteFails = new FailureEscalation(256); + private static final FailureEscalation tryReadFails = new FailureEscalation(256); + /** * Reads an input stream associated with the given export cache key. * @@ -121,9 +126,12 @@ private static Optional tryRead(StorageIO storage, String if (!storage.isAuxObjectCached(auxTag)) { return Optional.empty(); } + tryReadFails.recordSuccess().ifPresent(n -> logger.warning("Trying to read cached export recovered after " + n + " consecutive failures")); } catch (IOException e) { // Treat as a "cache miss" so the pipeline regenerates rather than failing over a cache IO issue. - logger.log(Level.FINE, e, () -> "Existence check failed for " + auxTag); + // Note: if necessary, elevate recording the failures per storage or even more fine-grained, including the tag. + logger.log(tryReadFails.incrementAndGetLevel(), e, + () -> "Existence check failed for " + auxTag + " (consecutive failures: " + tryReadFails.currentStreak() + ")"); return Optional.empty(); } try { @@ -142,10 +150,12 @@ private static Optional tryRead(StorageIO storage, String private static void deleteQuietly(StorageIO storage, String auxTag) { try { storage.deleteAuxObject(auxTag); + quietDeleteFails.recordSuccess().ifPresent(n -> logger.log(Level.FINE, "Quiet deletes from the cache recovered after " + n + " consecutive failures.")); } catch (IOException e) { // Absence is the common case here and not an error. // Real failures are logged but non-fatal, as the entry will be overwritten or ignored on the next pipeline run. - logger.log(Level.FINE, e, () -> "Could not delete aux object " + auxTag); + // Note: if necessary, elevate recording the failures per storage or even more fine-grained, including the tag. + logger.log(quietDeleteFails.incrementAndGetLevel(), e, () -> "Could not delete aux object " + auxTag); } } From 9aefcaabdea8b8f076ea4b4c94545c63e5bfe0ec Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 16:41:43 +0200 Subject: [PATCH 29/39] feat(export): add ExportPipelineBean as central export orchestration EJB #11405 - Introduces a `@Stateless` EJB that funnels all export data production (draft, cached, bulk) through a single path for uniform staleness validation, prerequisite resolution, and error wrapping. - Cached reads consult registered `ExportCacheInvalidator` instances; stale entries are evicted and reported as a miss. - Prerequisite formats are resolved recursively with circular-chain detection via an in-flight `LinkedHashSet`. - Non-cacheable (draft) versions are produced to `SecureTempFiles` with `DELETE_ON_CLOSE` to avoid in-memory retention of large exports. - `IllegalStateException` from exporters is wrapped in `ExportException` with dataset context for consistent reporting across all production paths. --- .../export/service/ExportPipelineBean.java | 337 ++++++++++++++++++ 1 file changed, 337 insertions(+) create mode 100644 src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java new file mode 100644 index 00000000000..2618a7e9ca2 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java @@ -0,0 +1,337 @@ +package edu.harvard.iq.dataverse.export.service; + +import edu.harvard.iq.dataverse.DatasetVersion; +import edu.harvard.iq.dataverse.util.SecureTempFiles; +import io.gdcc.spi.export.ExportException; +import io.gdcc.spi.export.Exporter; +import jakarta.ejb.EJB; +import jakarta.ejb.Stateless; +import jakarta.inject.Inject; + +import java.io.BufferedOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; + +/** + * Stateless EJB that orchestrates the end-to-end export pipeline for dataset versions. + *

    + * This bean acts as the central coordinator between the export cache, the exporter registry, + * and the individual format-specific exporters. Its responsibilities include: + *

      + *
    • Serving cached exports after verifying their freshness against all registered + * {@link ExportCacheInvalidator} instances. A stale entry is evicted and reported as + * a cache miss, ensuring that no consumer (prerequisite resolution or direct retrieval) + * ever receives outdated bytes.
    • + *
    • Producing a new export by looking up the appropriate {@link Exporter} in the + * {@link ExporterRegistryBean}, resolving any declared prerequisite format recursively, + * and writing the result into the cache atomically.
    • + *
    • Detecting and rejecting circular prerequisite chains via an in-flight format set + * passed through the recursive resolution calls.
    • + *
    + *

    + * All data production paths (draft, cached, bulk) funnel through this bean, which means + * that every export is subjected to the same staleness validation, prerequisite resolution, + * and error-wrapping logic. + *

    + * Field injection is used for the {@link ExportCache} dependency because EJB mandates a + * no-args constructor; this is expected to be replaced with constructor injection when the + * codebase transitions to CDI-only dependency management. + * + * @see ExporterRegistryBean + * @see ExportCache + * @see ExportCacheInvalidator + * @see ExportServiceBean + */ +@Stateless +class ExportPipelineBean { + + @EJB + ExporterRegistryBean registry; + + // We must use (frowned upon) field injection here, as EJB requires a no-args constructor. + // When the codebase transitions to use CDI only, this shall be changed to constructor injection. + @SuppressWarnings("java:S6813") + @Inject + ExportCache cache; + + /** + * A collection of {@link ExportCacheInvalidator} instances. + * This list is intended to centralize all invalidation mechanisms for export cache entries. + * Any new implementations must be added here in addition to the "permits" on the interface seal. + *

    + * Note: Once we allow plugins to provide their own invalidation logic, we must load them. + * This static, non-CDI list shall then be replaced by a registry pattern following implementation. + */ + static final List invalidators = List.of(new FileEmbargoExpiryInvalidator()); + + /** + * Attempts to read a cached export for the given dataset version and cache key, verifying freshness through + * registered invalidators before returning the stream. + *

    + * If the dataset version is not cacheable, this method returns {@link Optional#empty()} + * immediately without consulting the cache. + *

    + * When a cached entry is found, all registered invalidators are consulted. + * If any invalidator reports the entry as stale, a cache miss is signaled. + * + * @param datasetVersion the dataset version whose cached export is to be read; must not be null + * @param key the cache key identifying the target export format and cache location; must not be null + * @return an {@link Optional} containing an open {@link InputStream} to the cached export data, or + * {@link Optional#empty()} if the version is not cacheable, no entry exists, or the entry was determined to be stale and evicted + * @throws IllegalArgumentException if {@code datasetVersion} or {@code key} is null + * @throws IOException if an I/O error occurs while closing a stale stream or evicting the cache entry + */ + Optional readFreshCachedExport(DatasetVersion datasetVersion, ExportCacheKey key) throws IOException { + if (datasetVersion == null || key == null) { + throw new IllegalArgumentException("Dataset version and export cache key must not be null"); + } + + // Short-circuit if the version is not cacheable anyway + if (!ExportServiceBean.isCacheable(datasetVersion)) { + return Optional.empty(); + } + + Optional cached = cache.read(datasetVersion.getDataset(), key); + + if (cached.isPresent()) { + try { + // Apply all invalidators to see if the cache entry may be stale + if (invalidators.stream().anyMatch(inv -> inv.isStale(datasetVersion, key))) { + // If this in fact is stale, evict, close the stream, and report back cache miss + cache.evict(datasetVersion.getDataset(), key); + cached.get().close(); // First evict, then close, in case closing throws. + return Optional.empty(); + } + } catch (IOException | RuntimeException ex) { + // Avoid leaking the stream, but never let the close failure mask the original exception + try { + cached.get().close(); + } catch (IOException closeEx) { + ex.addSuppressed(closeEx); + } + throw ex; + } + } + + return cached; + } + + /** + * Produces an export for the given dataset version and writes the result through to the export cache. + * + * @param datasetVersion the dataset version whose metadata will be exported + * @param key the cache key identifying the target export format and cache location + * @throws IllegalArgumentException argument validation fails + * @throws ExportException if an error occurs during export in {@link #produce(String, DatasetVersion, OutputStream, Set)} + * @throws IOException if an I/O error occurs while writing the export to the cache + */ + void produceAndCache(DatasetVersion datasetVersion, ExportCacheKey key) throws IOException { + if (datasetVersion == null || key == null) { + throw new IllegalArgumentException("Neither dataset version nor cache key may be null"); + } + + cache.write( + datasetVersion.getDataset(), + key, + // The trick here: by creating a lambda, use the input from the functional interface the cache provides. + // This way, the cache owns all the I/O going on. + out -> produce(key.formatName(), datasetVersion, out, new LinkedHashSet<>()) + ); + } + + /** + * No caching variant to produce an export for the given dataset version in the requested format. + * Writes the result to the supplied output stream. + *

    + * The requested format name must be registered in the export registry. + * If the exporter declares a prerequisite format, it is resolved recursively before the export is produced. + * Circular prerequisite chains are detected and rejected. + *

    + * If the dataset version fulfills {@link ExportServiceBean#isCacheable(DatasetVersion)}, these formats will be + * read from the cache. If the prerequisites are not yet cached, they are going to be cached here. + *

    + * The caller is responsible for creating and closing the output stream. + * + * @param datasetVersion the dataset version whose metadata will be exported + * @param formatName the name of the export format to produce; must be a registered format + * @param out the output stream to write the produced export to + * @throws IllegalArgumentException if the dataset version or output stream is null, + * if no exporter is registered for the format, or + * if a prerequisite cycle is detected + * @throws ExportException if the prerequisite format resolution fails, or + * if the exporter throws an {@link IllegalStateException} + */ + void produceAndWriteOut(DatasetVersion datasetVersion, String formatName, OutputStream out) { + if (datasetVersion == null || out == null) { + throw new IllegalArgumentException("datasetVersion and out must not be null"); + } + registry.requireExists(formatName); + + produce(formatName, datasetVersion, out, new LinkedHashSet<>()); + } + + /** + * Produces a single export for the given dataset version by delegating to the registered exporter for the + * requested format, writing the result to the supplied output stream. + *

    + * If the exporter declares a prerequisite format, this method resolves that prerequisite recursively via + * {@link #resolvePrerequisite(String, DatasetVersion, Set)}, before invoking the exporter's export logic. + * The in-flight set is used to detect circular prerequisite chains and throws an {@link ExportException} if a cycle is found. + *

    + * The requested format name is added to the in-flight set at entry and removed in a "finally" block, ensuring the + * set is left in its original state regardless of whether the export succeeds or fails. + * + * @param formatName the name of the export format to produce + * @param version the dataset version whose metadata will be exported + * @param out the output stream to write the produced export to; the caller is + * responsible for closing it + * @param inFlight a set of format names currently being produced along the prerequisite + * resolution chain; used to detect and reject circular dependencies + * @throws IllegalArgumentException if no exporter is registered for the format, + * if a prerequisite cycle is detected, or + * if the output stream is null + * @throws ExportException if the exporter throws an {@link IllegalStateException} or + * if prerequisite format resolution fails + * + */ + private void produce(String formatName, DatasetVersion version, OutputStream out, Set inFlight) { + // version is null checked before, inFlight is injected by the caller. This is a private method, no additional checks necessary. + if (out == null) { + throw new IllegalArgumentException("Output stream may not be null"); + } + + // Try retrieving the exporter for the requested format + Exporter exporter = registry.get(formatName).orElseThrow(() -> new IllegalArgumentException("No such exporter available for format " + formatName)); + + // Add current requested format to the set of formats requested before for this dataset version. + if (!inFlight.add(formatName)) { + throw new IllegalArgumentException("Prerequisite cycle detected while exporting: " + + String.join(" -> ", inFlight) + + " -> " + formatName); + } + + try { + // Case A: No prerequisite format needed + Optional prereqFormatName = exporter.getPrerequisiteFormatName(); + if (prereqFormatName.isEmpty()) { + exporter.exportDataset(new InternalExportDataProvider(version), out); + return; + } + + // Case B: Prerequisite format needed, recursively resolve, then export + try (InputStream prereqStream = resolvePrerequisite(prereqFormatName.get(), version, inFlight)) { + exporter.exportDataset(new InternalExportDataProvider(version, prereqStream), out); + } catch (IOException ioe) { + throw new ExportException("Could not provide prerequisite " + prereqFormatName.get() + + " to create " + formatName + " export for dataset " + + version.getDataset().getId(), ioe); + } + } catch (IllegalStateException ise) { + /* @landreev 2023-04-23: + * IllegalStateException can potentially mean very different, and unexpected things. + * An exporter attempting to get a single primitive value from a fieldDTO that is, in fact, a multiple and + * contains a JSON vector will result in an IllegalStateException. + * This has happened, for example, when the code in the DDI exporter was not updated following a + * metadata field type change. + * Wrap it here so ALL data production paths (draft, cached, bulk) report it usefully. + */ + throw new ExportException("IllegalStateException caught when exporting " + + formatName + " for dataset " + + version.getDataset().getGlobalId().toString() + + "; may or may not be due to a mismatch between exporter code " + + "and a metadata block update. " + ise.getMessage(), ise); + } finally { + inFlight.remove(formatName); + } + } + + /** + * Provides the prerequisite export for a derived format. + *

    + * In case a complete chain of prereq formats are needed, a recursive stack is used to iterate through it, + * calling {@link #produce(String, DatasetVersion, OutputStream, Set)} on the prereq format. + *

    + * For cacheable versions the cached entry is used if present and fresh. + * On a miss the prerequisite is produced and written through to the cache. + * (The bytes a derived export was built from are the same bytes subsequently served for the prerequisite format). + *

    + * Non-cacheable versions (drafts) are always produced fresh, see cache policy at {@link ExportServiceBean#isCacheable(DatasetVersion)}. + * + * @param prereqFormatName the name of the export format to produce + * @param version the dataset version whose metadata will be exported + * @param inFlight a set of format names currently being produced along the prerequisite + * resolution chain; used to detect and reject circular dependencies + * @return open stream to the exported metadata, which the caller must close + */ + private InputStream resolvePrerequisite(String prereqFormatName, DatasetVersion version, Set inFlight) throws IOException { + // Note: Intentionally no checks for null parameters or writability of the set here. + // This is an internal method, and any calls are in this class, which hopefully provides enough control. + + // Non-cacheable versions are always created fresh + if (!ExportServiceBean.isCacheable(version)) { + return producePreReqToTempFile(prereqFormatName, version, inFlight); + } + + // If cacheable, try to read from the cache + ExportCacheKey key = new ExportCacheKey(version, prereqFormatName); + Optional cached = readFreshCachedExport(version, key); + if (cached.isPresent()) { + return cached.get(); + } + + // If not in cache, produce and cache, return resulting data stream + // TODO: This write-then-read is not atomic, which might lead to a race condition, also we already try to run exports in topological order. + // Consider adding a ExportCache.writeThenRead() function which ensures atomicity in the implementation. + // Alternatively, lock-by-key may be used inside the ExportCache. + cache.write(version.getDataset(), key, out -> produce(prereqFormatName, version, out, inFlight)); + return cache + .read(version.getDataset(), key) + .orElseThrow(() -> new ExportException("Prerequisite " + prereqFormatName + " was produced but could not be read back")); + } + + /** + * Produces an export for the given (non-cacheable) dataset version by writing the result to a secure temporary file, + * then returns an input stream over that file. This avoids huge blips in memory usage for drafts. + *

    + * The temporary file is created with owner-only permissions and opened with {@link StandardOpenOption#DELETE_ON_CLOSE}, + * so the file is automatically removed when the caller closes the returned stream. + *

    + * If an exception is thrown before the stream is handed back, the temporary file is deleted immediately to avoid + * leaving orphaned files on disk. + *

    + * TODO: Using temporary files will leave things behind when the JVM crashes. + * If we ever think this may become a problem (given that java.io.tmp dir should be cleaned up by the OS), + * we can always add something to an @Startup EJB. + * + * @return an open {@link InputStream} to the temporary file containing the produced export data; + * the caller is responsible for closing it, which also deletes the temporary file + */ + private InputStream producePreReqToTempFile(String formatName, DatasetVersion version, Set inFlight) throws IOException { + Path tempFile = SecureTempFiles.createOwnerOnlyTempFile("dataverse-export-draft-", ".tmp"); + try { + try (OutputStream out = new BufferedOutputStream(Files.newOutputStream(tempFile))) { + produce(formatName, version, out, inFlight); + } + // The returned stream deletes the file on close. + // Note: The only caller (produce(), Case B) already closes it via try-with-resources. + return Files.newInputStream(tempFile, StandardOpenOption.DELETE_ON_CLOSE); + } catch (IOException | RuntimeException e) { + // Export failed before the stream existed: nobody will ever close it, delete now. + try { + Files.deleteIfExists(tempFile); + } catch (IOException del) { + e.addSuppressed(del); + } + throw e; + } + } + +} From f318f8e2e89b128ca9910aa6f990b0a236f74121 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 17:05:13 +0200 Subject: [PATCH 30/39] refactor(export): introduce ExportPipelineBean to ExportServiceBean #11405 - Injecting `ExportPipelineBean` as an `@EJB` - Removed the static `invalidators` list and its associated Javadoc from `ExportServiceBean` - they are now owned by the pipeline. --- .../dataverse/export/service/ExportServiceBean.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 234f99bf1b9..ddf16919d2f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -42,15 +42,8 @@ public class ExportServiceBean { @Inject ExportCache cache; - /** - * A collection of {@link ExportCacheInvalidator} instances. - * This list is intended to centralize all invalidation mechanisms for export cache entries. - * Any new implementations must be added here in addition to the "permits" on the interface seal. - *

    - * Note: Once we allow plugins to provide their own invalidation logic, we must load them. - * This static, non-CDI list shall then be replaced by a registry pattern following implementation. - */ - List invalidators = List.of(new FileEmbargoExpiryInvalidator()); + @EJB + ExportPipelineBean pipeline; // METHODS TO RETRIEVE EXPORTED DATA From 2d12621deb72f34ce666bc8b9d1d3f8626b31886 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 17:06:35 +0200 Subject: [PATCH 31/39] feat(export): add isCacheable helper centralizing version cache policy #11405 - Introduces a static `isCacheable(DatasetVersion)` method so the "drafts are mutable, therefore never cached" rule lives in one place instead of being re-encoded at each call site. - The service owns the cache policy, it's mostly applied within ExportPipeline. - Javadoc documents the intent and flags the method as the extension point for future version states (e.g. deaccessioned). --- .../iq/dataverse/export/service/ExportServiceBean.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index ddf16919d2f..bf4e980d2b5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -356,6 +356,12 @@ public void exportFormat(Dataset dataset, String formatName) throws ExportExcept } } + /** + * Cache policy: drafts are mutable and therefore never cached; released versions are cacheable. + * Extend here (not at call sites) when caching of further version states (e.g. deaccessioned) needs an explicit decision. + */ + static boolean isCacheable(DatasetVersion version) { + return !version.isDraft(); } /** From cb0f91036f4f75726a7b56b4f940028f6bfd02aa Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 18:11:57 +0200 Subject: [PATCH 32/39] refactor(export): rework exportFormats to use pipeline and topological ordering #11405 - Replace the manual prerequisite-resolution loop with a pipeline-driven `exportFormats(DatasetVersion, List)` that resolves transitive dependents via the registry and sorts exporters topologically before executing `produceAndCache` in ExportPipeline. - Simplify `exportFormat(Dataset, String)` to a one-line delegate over `exportFormats` with a single-element list. - Update `lastExportTime` after any successful export, not only when the full format set was requested. - Collect per-format failures and throw a single `ExportException` at the end, logging each individual failure at WARNING level. --- .../export/service/ExportServiceBean.java | 212 +++++++++--------- 1 file changed, 108 insertions(+), 104 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index bf4e980d2b5..267d3182a4e 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -236,123 +236,127 @@ public void exportAllFormats(Dataset dataset) throws ExportException { } /** - * This method is added to supplement the classic exportAllFormats() in order - * to allow the metadata export APIs to selectively re-export only the formats - * specified. This is to finally allow an instance admin to avoid running - * a complete, from-scratch reexport when only _some_, or just one of them - * actually needs to be refreshed. On a large instance this can waste a - * significant amount of time and CPU cycles. (new as of 6.12) - * This method calls the cacheExport() method for every valid/supported - * format name supplied, or for every Exporter available, if an empty List - * is passed. - * Only the latest published version is used for exports. - * exportAllFormats() above is now a convenience wrapper, with the - * implementation moved here. + * Exports the given dataset in a single specified format. + * Delegate to the multi-format export method with a very short list. + * Be aware that this may cause multiple exporters to be invoked in case the format is a prerequisite for others. * - * @param dataset - * @param formatNames - * @throws ExportException + * @param dataset the dataset to export; must not be null + * @param formatName the name of the export format to use; must not be null + * @throws ExportException if the format name is null or if the underlying export operation fails + */ + public void exportFormat(Dataset dataset, String formatName) throws ExportException { + // Check here to avoid NPE from List.of() + if (formatName == null) { + throw new ExportException("Format name cannot be null"); + } + exportFormats(dataset, List.of(formatName)); + } + + /** + * Exports the given dataset selectively in the specified formats by resolving the dataset's {@link #defaultVersion} + * and delegating to the version-specific export method. Upon successful completion of all exports, the dataset's + * last export time is updated to the current timestamp. + *

    + * Be aware that this may cause more exporters to be invoked in case any format is a prerequisite for others. + * If the list is empty, this method will export all available formats. + * + * @param dataset the dataset to export; must not be null + * @param formatNames the list of format names to export in; an empty list means all formats + * @throws ExportException if the dataset is null or if any export operation fails */ public void exportFormats(Dataset dataset, List formatNames) throws ExportException { if (dataset == null) { - throw new ExportException("exportFormats called with null Dataset"); + throw new ExportException("Dataset must not be null"); + } + + exportFormats(defaultVersion(dataset), formatNames); + + // All exports done successfully, update last export time on the dataset + // TODO: Is it correct to update the last export time even if only some formats were exported? + dataset.setLastExportTime(Date.from(Instant.now())); + } + + /** + * Clears the cached exports for the specified formats (or all registered formats if the list is empty), + * resolves all transitive dependent formats, orders the required exporters topologically to guarantee + * that prerequisite formats are regenerated before their dependents, and then sequentially produces + * and caches the requested exports. + *

    + * If any of the requested formats has transitive dependents in the registry, those dependents are + * automatically included in the export process so that they are regenerated with fresh prerequisite + * data. + * + * @param datasetVersion the dataset version to export; must not be null + * @param formatNames the names of the export formats to produce; if empty, all formats registered in + * the registry will be exported + * @throws ExportException if datasetVersion is null or does not fullfill {@link #isCacheable(DatasetVersion)}, + * if any format name is invalid, or + * if one or more exports fail during execution + */ + public void exportFormats(DatasetVersion datasetVersion, List formatNames) throws ExportException { + if (datasetVersion == null) { + throw new ExportException("Dataset version must not be null"); + } + if (!isCacheable(datasetVersion)) { + throw new ExportException("Dataset version is not cacheable, thus it cannot be exported to cache"); } try { registry.requireAllExist(formatNames); - } catch (IllegalArgumentException ex) { - throw new ExportException("Invalid format names: " + ex.getMessage()); + } catch (IllegalArgumentException e) { + throw new ExportException("One or more format names are invalid: " + e.getMessage()); } - try { - clearCachedFormats(dataset, formatNames); - } catch (IOException ex) { - Logger.getLogger(ExportService.class.getName()).log(Level.SEVERE, null, ex); - } + // NOTE: Evict all formats at once before producing any new exports to improve cache consistency + // and force prerequisite formats to be renewed before use! + clearCachedFormats(datasetVersion, formatNames); - try { - DatasetVersion releasedVersion = dataset.getReleasedVersion(); - if (releasedVersion == null) { - throw new ExportException("No released version for dataset " + dataset.getGlobalId().toString()); - } - InternalExportDataProvider dataProvider = new InternalExportDataProvider(releasedVersion); - - for (Exporter e : exporterMap.values()) { - String formatName = e.getFormatName(); - if (formatNames.isEmpty() || formatNames.contains(formatName)) { - if (e.getPrerequisiteFormatName().isPresent()) { - String prereqFormatName = e.getPrerequisiteFormatName().get(); - try (InputStream preReqStream = getExport(dataset.getReleasedVersion(), prereqFormatName)) { - dataProvider.setPrerequisiteInputStream(preReqStream); - cacheExport(dataset, dataProvider, formatName, e); - dataProvider.setPrerequisiteInputStream(null); - } catch (IOException ioe) { - throw new ExportException("Could not get prerequisite " + e.getPrerequisiteFormatName() + " to create " + formatName + "export for dataset " + dataset.getId(), ioe); - } - } else { - cacheExport(dataset, dataProvider, formatName, e); - } - } - } - // Finally, if we have been able to successfully export in all available - // formats, we'll increment the "last exported" time stamp: - if (formatNames.isEmpty()) { - dataset.setLastExportTime(new Timestamp(new Date().getTime())); - } - - } catch (ServiceConfigurationError serviceError) { - throw new ExportException("Service configuration error during export. " + serviceError.getMessage()); - } catch (RuntimeException e) { - logger.log(Level.FINE, e.getMessage(), e); - throw new ExportException( - "Unknown runtime exception exporting metadata. " + (e.getMessage() == null ? "" : e.getMessage())); + // If the list of format names is empty, retrieve all format names from the registry and evict all. + if (formatNames.isEmpty()) { + formatNames = registry.getDetails().stream().map(ExporterRegistryBean.Details::formatName).toList(); + // Otherwise, make sure to add all formats relying on the requested ones, as they need to be regenerated, too. + } else { + formatNames = formatNames.stream() + // The flatMap replaces any stream element with the concatenated elements, + // thus re-adding the format itself to the list keeps it around. + .flatMap(format -> Stream.concat( + Stream.of(format), + registry.getTransitiveDependents(format).stream()) + ) + // Filter for duplicates (multiple formats may have the same dependents) + .distinct() + .toList(); } - } - - // This method finds the exporter for the format requested, - // then produces the dataset metadata as a JsonObject, then calls - // the "cacheExport()" method that will save the produced output - // in a file in the dataset directory. - public void exportFormat(Dataset dataset, String formatName) throws ExportException { - try { - - Exporter e = exporterMap.get(formatName); - if (e != null) { - DatasetVersion releasedVersion = dataset.getReleasedVersion(); - if (releasedVersion == null) { - throw new ExportException( - "No published version found during export. " + dataset.getGlobalId().toString()); - } - if(e.getPrerequisiteFormatName().isPresent()) { - String prereqFormatName = e.getPrerequisiteFormatName().get(); - try (InputStream preReqStream = getExport(releasedVersion, prereqFormatName)) { - InternalExportDataProvider dataProvider = new InternalExportDataProvider(releasedVersion, preReqStream); - cacheExport(dataset, dataProvider, formatName, e); - } catch (IOException ioe) { - throw new ExportException ("Could not get prerequisite " + e.getPrerequisiteFormatName() + " to create " + formatName + "export for dataset " + dataset.getId(), ioe); - } - } else { - InternalExportDataProvider dataProvider = new InternalExportDataProvider(releasedVersion); - cacheExport(dataset, dataProvider, formatName, e); - } - // As with exportAll, we should update the lastexporttime for the dataset - dataset.setLastExportTime(new Timestamp(new Date().getTime())); - } else { - throw new ExportException("Exporter not found"); + + // Retrieve the exporters for all formats, then order the list topologically, ensuring dependencies get done first + List exporters = formatNames.stream() + .map(registry::get) + .flatMap(Optional::stream) // safe: names were validated above! + .sorted(registry.getTopologicalComparator()) + .toList(); + + // THINK: What about the datacite export format? Any exporter may use it via the provider. + // Shouldn't all exports have this as an implicit dependency? Same goes for schema.org and ORE export! + // At the moment, the provider does a live conversion and does not read from a cached export, thus safe for now. + + // Now execute exports in sequential order + // Note: If parallelization of exports is to be achieved, use a different data structure (like a queue) and + // group by number of dependencies. All exports at a certain depth must be done before proceeding to + // avoid race conditions. + boolean allSucceeded = true; + for (Exporter exporter : exporters) { + String formatName = exporter.getFormatName(); + ExportCacheKey key = new ExportCacheKey(datasetVersion, formatName); + try { + pipeline.produceAndCache(datasetVersion, key); + // RuntimeEx also catches ExportException and NPEs + } catch (IOException | RuntimeException ex) { + allSucceeded = false; + logger.log(Level.WARNING, ex, () -> "Export of " + formatName + " failed for dataset version" + datasetVersion); } - } catch (IllegalStateException e) { - // IllegalStateException can potentially mean very different, and - // unexpected things. An exporter attempting to get a single primitive - // value from a fieldDTO that is in fact a Multiple and contains a - // json vector (this has happened, for example, when the code in the - // DDI exporter was not updated following a metadata fieldtype change), - // will result in IllegalStateException. - throw new ExportException("IllegalStateException caught when exporting " + formatName + " for dataset " - + dataset.getGlobalId().toString() - + "; may or may not be due to a mismatch between an exporter code and a metadata block update. " - + e.getMessage()); } - - } + + if (!allSucceeded) { + throw new ExportException("One or more exports failed, for details see logs"); } } From 101b9b4c3f2d5e6e400becd59c9018f0ecc42055 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:07:41 +0200 Subject: [PATCH 33/39] refactor(export): delegate ExportService.getExport to ExportPipeline #11405 - `ExportServiceBean#getExport` now simply attempts `readFreshCachedExport` and falls back to `readFreshExport`, eliminating the manual draft/published branching and the in-memory `ByteArrayOutputStream` round-trip. - Replaces `ExportPipelineBean#produceAndWriteOut` (caller-supplied `OutputStream`) with `readFreshExport`, which produces to a `SecureTempFiles` temp file and returns an `InputStream`, consistent with the existing temp-file strategy for drafts. - Renames `producePreReqToTempFile` to `produceToTempFile` since it now serves both prerequisite and primary format paths uniformly. --- .../export/service/ExportPipelineBean.java | 75 ++++++++++--------- .../export/service/ExportServiceBean.java | 70 ++++++----------- 2 files changed, 62 insertions(+), 83 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java index 2618a7e9ca2..dd2f8dabbe0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java @@ -124,6 +124,40 @@ Optional readFreshCachedExport(DatasetVersion datasetVersion, Expor return cached; } + /** + * No caching variant to produce an export for the given dataset version in the requested format. + * The produces metadata export will reside as a temporary file on disk, auto-deleted after consumption. + *

    + * The requested format name must be registered in the export registry. + * If the exporter declares a prerequisite format, it is resolved recursively before the export is produced. + * Circular prerequisite chains are detected and rejected. + *

    + * If the given dataset version does not satisfy {@link ExportServiceBean#isCacheable(DatasetVersion)}, + * the export and any prerequisite data formats will be generated on-the-fly. + * (Prerequisite formats will have their own temporary files, destroyed after consumption) + *

    + * If the dataset version is cacheable, it will still be written to a temporary file, but any prequisites + * will be read from the cache. If the prerequisites are not yet cached, they are going to be cached here. + *

    + * The caller is responsible for closing the returned input stream. + * + * @param datasetVersion the dataset version whose metadata will be exported + * @param formatName the name of the export format to produce; must be a registered format + * @throws IllegalArgumentException if the dataset version or output stream is null, + * if no exporter is registered for the format, or + * if a prerequisite cycle is detected + * @throws ExportException if the prerequisite format resolution fails, or + * if the exporter throws an {@link IllegalStateException} + */ + InputStream readFreshExport(DatasetVersion datasetVersion, String formatName) throws IOException { + if (datasetVersion == null) { + throw new IllegalArgumentException("datasetVersion must not be null"); + } + registry.requireExists(formatName); + + return produceToTempFile(formatName, datasetVersion, new LinkedHashSet<>()); + } + /** * Produces an export for the given dataset version and writes the result through to the export cache. * @@ -147,37 +181,6 @@ void produceAndCache(DatasetVersion datasetVersion, ExportCacheKey key) throws I ); } - /** - * No caching variant to produce an export for the given dataset version in the requested format. - * Writes the result to the supplied output stream. - *

    - * The requested format name must be registered in the export registry. - * If the exporter declares a prerequisite format, it is resolved recursively before the export is produced. - * Circular prerequisite chains are detected and rejected. - *

    - * If the dataset version fulfills {@link ExportServiceBean#isCacheable(DatasetVersion)}, these formats will be - * read from the cache. If the prerequisites are not yet cached, they are going to be cached here. - *

    - * The caller is responsible for creating and closing the output stream. - * - * @param datasetVersion the dataset version whose metadata will be exported - * @param formatName the name of the export format to produce; must be a registered format - * @param out the output stream to write the produced export to - * @throws IllegalArgumentException if the dataset version or output stream is null, - * if no exporter is registered for the format, or - * if a prerequisite cycle is detected - * @throws ExportException if the prerequisite format resolution fails, or - * if the exporter throws an {@link IllegalStateException} - */ - void produceAndWriteOut(DatasetVersion datasetVersion, String formatName, OutputStream out) { - if (datasetVersion == null || out == null) { - throw new IllegalArgumentException("datasetVersion and out must not be null"); - } - registry.requireExists(formatName); - - produce(formatName, datasetVersion, out, new LinkedHashSet<>()); - } - /** * Produces a single export for the given dataset version by delegating to the registered exporter for the * requested format, writing the result to the supplied output stream. @@ -277,10 +280,10 @@ private InputStream resolvePrerequisite(String prereqFormatName, DatasetVersion // Non-cacheable versions are always created fresh if (!ExportServiceBean.isCacheable(version)) { - return producePreReqToTempFile(prereqFormatName, version, inFlight); + return produceToTempFile(prereqFormatName, version, inFlight); } - // If cacheable, try to read from the cache + // If cacheable, try to read from the cache (will also trigger full invalidator chain!) ExportCacheKey key = new ExportCacheKey(version, prereqFormatName); Optional cached = readFreshCachedExport(version, key); if (cached.isPresent()) { @@ -299,7 +302,7 @@ private InputStream resolvePrerequisite(String prereqFormatName, DatasetVersion /** * Produces an export for the given (non-cacheable) dataset version by writing the result to a secure temporary file, - * then returns an input stream over that file. This avoids huge blips in memory usage for drafts. + * then returns an input stream over that file. This especially avoids huge blips in memory usage for drafts. *

    * The temporary file is created with owner-only permissions and opened with {@link StandardOpenOption#DELETE_ON_CLOSE}, * so the file is automatically removed when the caller closes the returned stream. @@ -314,10 +317,12 @@ private InputStream resolvePrerequisite(String prereqFormatName, DatasetVersion * @return an open {@link InputStream} to the temporary file containing the produced export data; * the caller is responsible for closing it, which also deletes the temporary file */ - private InputStream producePreReqToTempFile(String formatName, DatasetVersion version, Set inFlight) throws IOException { + private InputStream produceToTempFile(String formatName, DatasetVersion version, Set inFlight) throws IOException { Path tempFile = SecureTempFiles.createOwnerOnlyTempFile("dataverse-export-draft-", ".tmp"); try { try (OutputStream out = new BufferedOutputStream(Files.newOutputStream(tempFile))) { + // Note: Any prerequisites are recursively produced on demand, in addition to the original target format. + // If the dataset version can be cached, a read attempt for prerequisites will be made. produce(formatName, version, out, inFlight); } // The returned stream deletes the file on close. diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java index 267d3182a4e..416a24f7427 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportServiceBean.java @@ -47,55 +47,29 @@ public class ExportServiceBean { // METHODS TO RETRIEVE EXPORTED DATA - public InputStream getExport(DatasetVersion datasetVersion, String formatName) throws ExportException, IOException { - - Dataset dataset = datasetVersion.getDataset(); - InputStream exportInputStream = null; - - if (datasetVersion.isDraft()) { - // For drafts we create the export on the fly rather than caching. - Exporter exporter = exporterMap.get(formatName); - if (exporter != null) { - try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { - // getPrerequisiteFormatName logic copied from exportFormat() - if (exporter.getPrerequisiteFormatName().isPresent()) { - String prereqFormatName = exporter.getPrerequisiteFormatName().get(); - try (InputStream preReqStream = getExport(datasetVersion, prereqFormatName)) { - InternalExportDataProvider dataProvider = new InternalExportDataProvider(datasetVersion, preReqStream); - exporter.exportDataset(dataProvider, outputStream); - } catch (IOException ioe) { - throw new ExportException("Could not get prerequisite " + prereqFormatName + " to create " + formatName + " export for dataset " + dataset.getId(), ioe); - } - } else { - InternalExportDataProvider dataProvider = new InternalExportDataProvider(datasetVersion); - exporter.exportDataset(dataProvider, outputStream); - } - return new ByteArrayInputStream(outputStream.toByteArray()); - } - } - } else { - // for non-drafts (published versions) we try to locate an already existing, cached export - exportInputStream = getCachedExportFormat(dataset, formatName); - } - - if (exportInputStream != null) { - return exportInputStream; - } - - // if it doesn't exist, we'll try to run the export: - exportFormat(dataset, formatName); - - // and then try again: - exportInputStream = getCachedExportFormat(dataset, formatName); - - if (exportInputStream != null) { - return exportInputStream; + /** + * Retrieves a stream of the metadata export for the given dataset version in the specified format. + *

    + * First checks for a fresh, cached export. + * If none is available (usually because the dataset version is not able to be cached), + * generates a fresh export by invoking the export pipeline and writing to a temporary location. + *

    + * The caller is responsible for closing the returned {@link InputStream}. + * + * @param datasetVersion the dataset version to retrieve the export for; must not be null + * @param formatName the name of the export format to retrieve; must not be null + * @return an {@link InputStream} containing the export data for the requested format + * @throws ExportException if the input stream for the metadata export cannot be retrieved due to underlying errors + */ + public InputStream getExport(DatasetVersion datasetVersion, String formatName) throws ExportException { + // Note: we don't do validation here, as the lower layers will take care of it. + try { + ExportCacheKey key = new ExportCacheKey(datasetVersion, formatName); + return pipeline.readFreshCachedExport(datasetVersion, key) + .orElse(pipeline.readFreshExport(datasetVersion, formatName)); + } catch (IOException e) { + throw new ExportException("Failed to retrieve export", e); } - - // if there is no cached export still - we have to give up and throw - // an exception! - throw new ExportException("Failed to export the dataset as " + formatName); - } public String getLatestPublishedAsString(Dataset dataset, String formatName) { From e2894e386ec8b8417c56afeed5a69188e0d60b78 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:09:10 +0200 Subject: [PATCH 34/39] docs(export): note limitations of naive staleness invalidation #11405 Added TODO comments in `readFreshCachedExport` flagging that the per-invalidator staleness check is a naive approach that won't scale properly to longer prerequisite format chains. --- .../iq/dataverse/export/service/ExportPipelineBean.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java index dd2f8dabbe0..51a066551cf 100644 --- a/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/export/service/ExportPipelineBean.java @@ -104,6 +104,9 @@ Optional readFreshCachedExport(DatasetVersion datasetVersion, Expor if (cached.isPresent()) { try { // Apply all invalidators to see if the cache entry may be stale + // TODO: In case we ever have longer prerequisite format chains, this naive appraoch will need refinement. + // The staleness checks may be expensive and repeated execution is not helpful. + // For now, this pipeline is *stateless*, so changing the procedure needs careful consideration. if (invalidators.stream().anyMatch(inv -> inv.isStale(datasetVersion, key))) { // If this in fact is stale, evict, close the stream, and report back cache miss cache.evict(datasetVersion.getDataset(), key); From 10747266ba5cc26ac6a1052cfe1d935a7982611f Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:16:06 +0200 Subject: [PATCH 35/39] refactor(commands): replace ExportService.getInstance() with CommandContext injection #11405 - Add `exportService()` and `exporterRegistry()` to `CommandContext`, implemented via EJB lookup in `EjbDataverseEngine` and null stubs in `TestCommandContext`. - Replace all `ExportService.getInstance()` call sites in `CuratePublishedDatasetVersionCommand`, `RedetectFileTypeCommand`, `DeaccessionDatasetVersionCommand`, and `DestroyDatasetCommand` with `ctxt.exportService()`. - Remove now-unnecessary `ExportService` imports from the affected command classes. - Widen `clearAllCachedFormats` catch from `IOException` to `ExportException` and add WARNING-level logging for ignored export failures. --- .../iq/dataverse/EjbDataverseEngine.java | 20 ++++++++++++++++- .../engine/command/CommandContext.java | 6 +++++ .../CuratePublishedDatasetVersionCommand.java | 4 +--- .../DeaccessionDatasetVersionCommand.java | 22 ++++++------------- .../command/impl/DestroyDatasetCommand.java | 11 +++++----- .../impl/ReconcileDatasetPidCommand.java | 2 -- .../command/impl/RedetectFileTypeCommand.java | 4 +--- .../dataverse/engine/TestCommandContext.java | 14 +++++++++++- 8 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java b/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java index 4fa85a543d8..3b2c7163491 100644 --- a/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java +++ b/src/main/java/edu/harvard/iq/dataverse/EjbDataverseEngine.java @@ -6,6 +6,8 @@ import edu.harvard.iq.dataverse.authorization.AuthenticationServiceBean; import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinUserServiceBean; import edu.harvard.iq.dataverse.dataverse.featured.DataverseFeaturedItemServiceBean; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.license.LicenseServiceBean; import edu.harvard.iq.dataverse.util.cache.CacheFactoryBean; import edu.harvard.iq.dataverse.engine.DataverseEngine; @@ -209,6 +211,12 @@ public class EjbDataverseEngine { @EJB CacheFactoryBean cacheFactory; + @EJB + ExportServiceBean exportService; + + @EJB + ExporterRegistryBean exporterRegistry; + @Resource EJBContext ejbCtxt; @@ -664,7 +672,17 @@ public MetadataBlockServiceBean metadataBlocks() { public DatasetTypeServiceBean datasetTypes() { return datasetTypeService; } - + + @Override + public ExportServiceBean exportService() { + return exportService; + } + + @Override + public ExporterRegistryBean exporterRegistry() { + return exporterRegistry; + } + @Override public void beginCommandSequence() { this.commandsCalled = new Stack(); diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/CommandContext.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/CommandContext.java index 1945d44cd78..c481759f972 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/CommandContext.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/CommandContext.java @@ -4,6 +4,8 @@ import edu.harvard.iq.dataverse.dataset.DatasetFieldsValidator; import edu.harvard.iq.dataverse.authorization.providers.builtin.BuiltinUserServiceBean; import edu.harvard.iq.dataverse.dataverse.featured.DataverseFeaturedItemServiceBean; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.license.LicenseServiceBean; import edu.harvard.iq.dataverse.search.IndexServiceBean; import edu.harvard.iq.dataverse.search.SearchService; @@ -143,4 +145,8 @@ public interface CommandContext { public DatasetFieldsValidator datasetFieldsValidator(); public LicenseServiceBean licenses(); + + public ExportServiceBean exportService(); + + public ExporterRegistryBean exporterRegistry(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CuratePublishedDatasetVersionCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CuratePublishedDatasetVersionCommand.java index 1c57a9d4647..1b863115bdb 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CuratePublishedDatasetVersionCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/CuratePublishedDatasetVersionCommand.java @@ -6,7 +6,6 @@ import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; -import edu.harvard.iq.dataverse.export.ExportService; import io.gdcc.spi.export.ExportException; import edu.harvard.iq.dataverse.util.BundleUtil; import edu.harvard.iq.dataverse.util.DatasetFieldUtil; @@ -249,8 +248,7 @@ public boolean onSuccess(CommandContext ctxt, Object r) { // And the exported metadata files try { - ExportService instance = ExportService.getInstance(); - instance.exportAllFormats(d); + ctxt.exportService().exportAllFormats(d); } catch (ExportException ex) { // Just like with indexing, a failure to export is not a fatal condition. retVal = false; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeaccessionDatasetVersionCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeaccessionDatasetVersionCommand.java index 39306273b61..65863a86d28 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeaccessionDatasetVersionCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DeaccessionDatasetVersionCommand.java @@ -15,16 +15,10 @@ import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; -import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; -import edu.harvard.iq.dataverse.export.ExportService; import io.gdcc.spi.export.ExportException; -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; -import edu.harvard.iq.dataverse.util.BundleUtil; -import java.io.IOException; + +import java.util.logging.Level; import java.util.logging.Logger; -import edu.harvard.iq.dataverse.batch.util.LoggingUtil; -import java.util.concurrent.Future; -import org.apache.solr.client.solrj.SolrServerException; /** * @@ -74,23 +68,21 @@ public DatasetVersion execute(CommandContext ctxt) throws CommandException { boolean doNormalSolrDocCleanUp = true; - - ExportService instance = ExportService.getInstance(); - - if (managed.getDataset().getReleasedVersion() != null) { try { - instance.exportAllFormats(managed.getDataset()); + ctxt.exportService().exportAllFormats(managed.getDataset()); } catch (ExportException ex) { // Something went wrong! // But we're not going to treat it as a fatal condition. + logger.log(Level.WARNING,"Ignored failure to export all formats after deaccessioning", ex); } } else { try { // otherwise, we need to wipe clean the exports we may have cached: - instance.clearAllCachedFormats(managed.getDataset()); - } catch (IOException ex) { + ctxt.exportService().clearAllCachedFormats(managed.getDataset()); + } catch (ExportException ex) { //Try catch required due to original method for clearing cached metadata (non fatal) + logger.log(Level.WARNING,"Ignored failure to delete all formats after deaccessioning", ex); } } // And save the dataset, to get the "last exported" timestamp right: diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java index 49861e084b6..2b8c56683ac 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/DestroyDatasetCommand.java @@ -9,7 +9,6 @@ import edu.harvard.iq.dataverse.dataaccess.DataAccess; import edu.harvard.iq.dataverse.dataaccess.FileAccessIO; import edu.harvard.iq.dataverse.dataaccess.GlobusOverlayAccessIO; -import edu.harvard.iq.dataverse.export.ExportService; import edu.harvard.iq.dataverse.search.IndexServiceBean; import edu.harvard.iq.dataverse.RoleAssignment; import edu.harvard.iq.dataverse.authorization.Permission; @@ -38,6 +37,7 @@ import edu.harvard.iq.dataverse.batch.util.LoggingUtil; import java.io.IOException; +import io.gdcc.spi.export.ExportException; import org.apache.solr.client.solrj.SolrServerException; /** @@ -126,13 +126,12 @@ protected void executeImpl(CommandContext ctxt) throws CommandException { } // CACHED EXPORTS - var exportService = ExportService.getInstance(); try { - exportService.clearAllCachedFormats(managedDoomed); + ctxt.exportService().clearAllCachedFormats(managedDoomed); } - catch (IOException e) { - var msg = format("Failed to delete cached exports of {0}: {1} ", managedDoomed.getIdentifier(), e.getClass().getSimpleName()); - logger.log(Level.WARNING, msg, e.getMessage()); + catch (ExportException e) { + var msg = format("Ignored failure to delete cached exports of {0}: {1} ", managedDoomed.getIdentifier(), e.getClass().getSimpleName()); + logger.log(Level.WARNING, msg, e); } // DIRECTORY diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ReconcileDatasetPidCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ReconcileDatasetPidCommand.java index 18db587dcc4..616685a8f24 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ReconcileDatasetPidCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/ReconcileDatasetPidCommand.java @@ -3,14 +3,12 @@ import edu.harvard.iq.dataverse.*; import edu.harvard.iq.dataverse.authorization.Permission; import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; -import edu.harvard.iq.dataverse.dataaccess.DataAccess; import edu.harvard.iq.dataverse.engine.command.CommandContext; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; import edu.harvard.iq.dataverse.engine.command.exception.PermissionException; -import edu.harvard.iq.dataverse.export.ExportService; import edu.harvard.iq.dataverse.pidproviders.PidProvider; import edu.harvard.iq.dataverse.pidproviders.PidUtil; import edu.harvard.iq.dataverse.util.BundleUtil; diff --git a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RedetectFileTypeCommand.java b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RedetectFileTypeCommand.java index b9346a43af8..fa410a6acd6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RedetectFileTypeCommand.java +++ b/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/RedetectFileTypeCommand.java @@ -9,7 +9,6 @@ import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.RequiredPermissions; import edu.harvard.iq.dataverse.engine.command.exception.CommandException; -import edu.harvard.iq.dataverse.export.ExportService; import io.gdcc.spi.export.ExportException; import edu.harvard.iq.dataverse.util.EjbUtil; import edu.harvard.iq.dataverse.util.FileUtil; @@ -86,8 +85,7 @@ public DataFile execute(CommandContext ctxt) throws CommandException { boolean doNormalSolrDocCleanUp = true; ctxt.index().asyncIndexDataset(dataset, doNormalSolrDocCleanUp); try { - ExportService instance = ExportService.getInstance(); - instance.exportAllFormats(dataset); + ctxt.exportService().exportAllFormats(dataset); } catch (ExportException ex) { // Just like with indexing, a failure to export is not a fatal condition. logger.info("Exception while exporting metadata files during file type redetection: " + ex.getLocalizedMessage()); diff --git a/src/test/java/edu/harvard/iq/dataverse/engine/TestCommandContext.java b/src/test/java/edu/harvard/iq/dataverse/engine/TestCommandContext.java index 573c0f48a53..51844143f2c 100644 --- a/src/test/java/edu/harvard/iq/dataverse/engine/TestCommandContext.java +++ b/src/test/java/edu/harvard/iq/dataverse/engine/TestCommandContext.java @@ -13,6 +13,8 @@ import edu.harvard.iq.dataverse.dataverse.featured.DataverseFeaturedItemServiceBean; import edu.harvard.iq.dataverse.engine.command.Command; import edu.harvard.iq.dataverse.engine.command.CommandContext; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.ingest.IngestServiceBean; import edu.harvard.iq.dataverse.license.LicenseServiceBean; import edu.harvard.iq.dataverse.pidproviders.PidProviderFactoryBean; @@ -263,7 +265,17 @@ public DatasetFieldsValidator datasetFieldsValidator() { public LicenseServiceBean licenses() { return null; } - + + @Override + public ExportServiceBean exportService() { + return null; + } + + @Override + public ExporterRegistryBean exporterRegistry() { + return null; + } + @Override public void beginCommandSequence() { throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates. From 750fff462da24eaeeb8485351ab6aba9625ad44e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:21:36 +0200 Subject: [PATCH 36/39] refactor(oai): replace ExportService.getInstance() with EJB-injected beans #11405 - Inject `ExportServiceBean` and `ExporterRegistryBean` via `@EJB` in `OAIServlet` and `OAIRecordServiceBean`. - Replace all `ExportService.getInstance()` call sites in `OAIServlet`, `OAIRecordServiceBean`, and `DataverseXoaiItemRepository` with injected bean calls. - Add `exportService` as a constructor injection parameter to `DataverseXoaiItemRepository` (it's a POJO). - Simplify `addSupportedMetadataFormats` to iterate `exporterRegistryService.getAll()` directly, removing manual label lookup and null-checking. - Add TODO comments in `OAIRecordServiceBean#exportAllFormats` questioning silent exception swallowing. - Remove unused imports --- .../harvest/server/OAIRecordServiceBean.java | 26 +++++++------ .../server/web/servlet/OAIServlet.java | 37 +++++++------------ .../xoai/DataverseXoaiItemRepository.java | 12 +++--- 3 files changed, 36 insertions(+), 39 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/harvest/server/OAIRecordServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/harvest/server/OAIRecordServiceBean.java index 975f4397908..570fb9e1ac0 100644 --- a/src/main/java/edu/harvard/iq/dataverse/harvest/server/OAIRecordServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/harvest/server/OAIRecordServiceBean.java @@ -8,9 +8,8 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetServiceBean; import edu.harvard.iq.dataverse.DatasetVersion; -import edu.harvard.iq.dataverse.export.ExportService; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; import io.gdcc.spi.export.ExportException; -import edu.harvard.iq.dataverse.search.IndexServiceBean; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import java.time.Instant; import java.util.Collection; @@ -45,8 +44,8 @@ public class OAIRecordServiceBean implements java.io.Serializable { DatasetServiceBean datasetService; @EJB SettingsServiceBean settingsService; - //@EJB - //ExportService exportService; + @EJB + ExportServiceBean exportService; @PersistenceContext(unitName = "VDCNet-ejbPU") EntityManager em; @@ -250,12 +249,18 @@ public void markOaiRecordsAsRemoved(Collection records, Date updateTi public void exportAllFormats(Dataset dataset) { try { - ExportService exportServiceInstance = ExportService.getInstance(); logger.log(Level.FINE, "Attempting to run export on dataset {0}", dataset.getGlobalId()); - exportServiceInstance.exportAllFormats(dataset); - dataset = datasetService.merge(dataset); - } catch (ExportException ee) {logger.fine("Caught export exception while trying to export. (ignoring)");} - catch (Exception e) {logger.fine("Caught unknown exception while trying to export (ignoring)");} + exportService.exportAllFormats(dataset); + datasetService.merge(dataset); + } catch (ExportException ee) { + // TODO: Should this really be ignored? What if we at least have a failure escalation for this? + // At least the exception should be logged. + logger.fine("Caught export exception while trying to export. (ignoring)"); + } catch (Exception e) { + // TODO: Should this really be ignored? What if we at least have a failure escalation for this? + // At least the exception should be logged. + logger.fine("Caught unknown exception while trying to export (ignoring)"); + } } @TransactionAttribute(REQUIRES_NEW) @@ -266,8 +271,7 @@ public void exportAllFormatsInNewTransaction(Dataset dataset) throws ExportExcep @TransactionAttribute(REQUIRES_NEW) public void exportFormatsInNewTransaction(Dataset dataset, List formatNames) throws ExportException { try { - ExportService exportServiceInstance = ExportService.getInstance(); - exportServiceInstance.exportFormats(dataset, formatNames); + exportService.exportFormats(dataset, formatNames); datasetService.setLastExportTimeInNewTransaction(dataset.getId(), dataset.getLastExportTime()); } catch (OptimisticLockException ole) { datasetService.setLastExportTimeInNewTransaction(dataset.getId(), dataset.getLastExportTime()); diff --git a/src/main/java/edu/harvard/iq/dataverse/harvest/server/web/servlet/OAIServlet.java b/src/main/java/edu/harvard/iq/dataverse/harvest/server/web/servlet/OAIServlet.java index f9047e3ee5f..9a0e0fd2948 100644 --- a/src/main/java/edu/harvard/iq/dataverse/harvest/server/web/servlet/OAIServlet.java +++ b/src/main/java/edu/harvard/iq/dataverse/harvest/server/web/servlet/OAIServlet.java @@ -6,6 +6,7 @@ package edu.harvard.iq.dataverse.harvest.server.web.servlet; import edu.harvard.iq.dataverse.MailServiceBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import io.gdcc.xoai.dataprovider.DataProvider; import io.gdcc.xoai.dataprovider.repository.Repository; import io.gdcc.xoai.dataprovider.repository.RepositoryConfiguration; @@ -21,7 +22,7 @@ import io.gdcc.xoai.xml.XmlWriter; import edu.harvard.iq.dataverse.DatasetServiceBean; import edu.harvard.iq.dataverse.DataverseServiceBean; -import edu.harvard.iq.dataverse.export.ExportService; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; import io.gdcc.spi.export.ExportException; import io.gdcc.spi.export.Exporter; import io.gdcc.spi.export.XMLExporter; @@ -29,8 +30,6 @@ import edu.harvard.iq.dataverse.harvest.server.OAISetServiceBean; import edu.harvard.iq.dataverse.harvest.server.xoai.DataverseXoaiItemRepository; import edu.harvard.iq.dataverse.harvest.server.xoai.DataverseXoaiSetRepository; -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; -import edu.harvard.iq.dataverse.util.MailUtil; import edu.harvard.iq.dataverse.util.SystemConfig; import io.gdcc.xoai.exceptions.BadVerbException; import io.gdcc.xoai.exceptions.OAIException; @@ -72,6 +71,10 @@ public class OAIServlet extends HttpServlet { DataverseServiceBean dataverseService; @EJB DatasetServiceBean datasetService; + @EJB + ExportServiceBean exportService; + @EJB + ExporterRegistryBean exporterRegistryService; @EJB SystemConfig systemConfig; @@ -130,7 +133,7 @@ public void init(ServletConfig config) throws ServletException { } setRepository = new DataverseXoaiSetRepository(setService); - itemRepository = new DataverseXoaiItemRepository(recordService, datasetService, SystemConfig.getDataverseSiteUrlStatic()); + itemRepository = new DataverseXoaiItemRepository(recordService, datasetService, exportService, SystemConfig.getDataverseSiteUrlStatic()); repositoryConfiguration = createRepositoryConfiguration(); @@ -149,25 +152,13 @@ private Context createContext() { } private void addSupportedMetadataFormats(Context context) { - for (String[] provider : ExportService.getInstance().getExportersLabels()) { - String formatName = provider[1]; - Exporter exporter; - try { - exporter = ExportService.getInstance().getExporter(formatName); - } catch (ExportException ex) { - exporter = null; - } - - if (exporter != null && (exporter instanceof XMLExporter) && exporter.isHarvestable()) { - MetadataFormat metadataFormat; - - metadataFormat = MetadataFormat.metadataFormat(formatName); - metadataFormat.withNamespace(((XMLExporter) exporter).getXMLNameSpace()); - metadataFormat.withSchemaLocation(((XMLExporter) exporter).getXMLSchemaLocation()); - - if (metadataFormat != null) { - context.withMetadataFormat(metadataFormat); - } + // Keep in mind: since EJB 3.1 (JSR 318) the call to the EJB singleton will block until bean is initialized + for (Exporter exporter : exporterRegistryService.getAll()) { + if (exporter instanceof XMLExporter xmlExporter && Boolean.TRUE.equals(exporter.isHarvestable())) { + MetadataFormat metadataFormat = MetadataFormat.metadataFormat(exporter.getFormatName()); + metadataFormat.withNamespace(xmlExporter.getXMLNameSpace()); + metadataFormat.withSchemaLocation(xmlExporter.getXMLSchemaLocation()); + context.withMetadataFormat(metadataFormat); } } } diff --git a/src/main/java/edu/harvard/iq/dataverse/harvest/server/xoai/DataverseXoaiItemRepository.java b/src/main/java/edu/harvard/iq/dataverse/harvest/server/xoai/DataverseXoaiItemRepository.java index 93679c7812b..05c0322e646 100644 --- a/src/main/java/edu/harvard/iq/dataverse/harvest/server/xoai/DataverseXoaiItemRepository.java +++ b/src/main/java/edu/harvard/iq/dataverse/harvest/server/xoai/DataverseXoaiItemRepository.java @@ -9,7 +9,7 @@ import io.gdcc.xoai.dataprovider.repository.ItemRepository; import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetServiceBean; -import edu.harvard.iq.dataverse.export.ExportService; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; import io.gdcc.spi.export.ExportException; import edu.harvard.iq.dataverse.harvest.server.OAIRecord; import edu.harvard.iq.dataverse.harvest.server.OAIRecordServiceBean; @@ -40,12 +40,14 @@ public class DataverseXoaiItemRepository implements ItemRepository { private final OAIRecordServiceBean recordService; private final DatasetServiceBean datasetService; - private final String serverUrl; + private final String serverUrl; + private final ExportServiceBean exportService; - public DataverseXoaiItemRepository (OAIRecordServiceBean recordService, DatasetServiceBean datasetService, String serverUrl) { + public DataverseXoaiItemRepository (OAIRecordServiceBean recordService, DatasetServiceBean datasetService, ExportServiceBean exportService, String serverUrl) { this.recordService = recordService; this.datasetService = datasetService; - this.serverUrl = serverUrl; + this.serverUrl = serverUrl; + this.exportService = exportService; } @Override @@ -253,7 +255,7 @@ private Metadata getDatasetMetadata(Dataset dataset, String metadataPrefix) thro } else { InputStream pregeneratedMetadataStream; - pregeneratedMetadataStream = ExportService.getInstance().getExport(dataset.getReleasedVersion(), metadataPrefix); + pregeneratedMetadataStream = exportService.getExport(dataset.getReleasedVersion(), metadataPrefix); metadata = Metadata.copyFromStream(pregeneratedMetadataStream); } From e29f920caffca1a18eaf735c9ad8a375270ba772 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:23:48 +0200 Subject: [PATCH 37/39] refactor(api): replace ExportService.getInstance() with EJB-injected beans #11405 - Inject `ExportServiceBean` and `ExporterRegistryBean` via `@EJB` in `AbstractApiBean`. - Replace `ExportService.getInstance()` call in `Files#exportDatasetMetadata` with injected `exportSvc`. - Replace manual label-lookup validation in `Metadata#validateFormatNames` with `exporterRegistrySvc.requireAllExist(formatNames)`. - Reformat `Info#getExportFormats` to iterate `exporterRegistrySvc.getDetails()` instead of `ExportService.getInstance().getExportersLabels()`. - Remove unused imports. --- .../iq/dataverse/api/AbstractApiBean.java | 8 +++++ .../edu/harvard/iq/dataverse/api/Files.java | 5 +-- .../edu/harvard/iq/dataverse/api/Info.java | 36 ++++++++----------- .../harvard/iq/dataverse/api/Metadata.java | 21 +++++------ 4 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 4eccb16f2b3..525309b85b5 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -22,6 +22,8 @@ import edu.harvard.iq.dataverse.engine.command.impl.GetLatestAccessibleDatasetVersionCommand; import edu.harvard.iq.dataverse.engine.command.impl.GetLatestPublishedDatasetVersionCommand; import edu.harvard.iq.dataverse.engine.command.impl.GetSpecificPublishedDatasetVersionCommand; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean; import edu.harvard.iq.dataverse.license.LicenseServiceBean; import edu.harvard.iq.dataverse.makedatacount.DatasetMetricsServiceBean; @@ -246,6 +248,12 @@ String getWrappedMessageWhenJson() { @EJB TemplateServiceBean templateSvc; + + @EJB + ExportServiceBean exportSvc; + + @EJB + ExporterRegistryBean exporterRegistrySvc; @Inject FailedPIDResolutionLoggingServiceBean fprLogService; diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Files.java b/src/main/java/edu/harvard/iq/dataverse/api/Files.java index 1c865c236ab..8de975487ba 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Files.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Files.java @@ -18,7 +18,6 @@ import edu.harvard.iq.dataverse.engine.command.exception.CommandException; import edu.harvard.iq.dataverse.engine.command.exception.IllegalCommandException; import edu.harvard.iq.dataverse.engine.command.impl.*; -import edu.harvard.iq.dataverse.export.ExportService; import io.gdcc.spi.export.ExportException; import edu.harvard.iq.dataverse.externaltools.ExternalTool; import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; @@ -60,7 +59,6 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.Status; import static edu.harvard.iq.dataverse.util.json.JsonPrinter.*; import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; @@ -888,8 +886,7 @@ public Response extractNcml(@Context ContainerRequestContext crc, @Parameter(des private void exportDatasetMetadata(SettingsServiceBean settingsServiceBean, Dataset theDataset) { try { - ExportService instance = ExportService.getInstance(); - instance.exportAllFormats(theDataset); + exportSvc.exportAllFormats(theDataset); } catch (ExportException ex) { // Something went wrong! diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Info.java b/src/main/java/edu/harvard/iq/dataverse/api/Info.java index b3cc69837f8..91dcce99a09 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Info.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Info.java @@ -2,20 +2,17 @@ import java.util.logging.Logger; import edu.harvard.iq.dataverse.customization.CustomizationConstants; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.util.json.JsonUtil; import jakarta.ws.rs.*; import jakarta.ws.rs.client.Client; import jakarta.ws.rs.client.ClientBuilder; import jakarta.ws.rs.client.WebTarget; -import edu.harvard.iq.dataverse.export.ExportService; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.SystemConfig; -import io.gdcc.spi.export.Exporter; -import io.gdcc.spi.export.ExportException; import io.gdcc.spi.export.XMLExporter; import jakarta.ejb.EJB; -import jakarta.json.Json; import jakarta.json.JsonObjectBuilder; import jakarta.json.JsonValue; import jakarta.ws.rs.core.MediaType; @@ -149,24 +146,21 @@ public Response getZipDownloadLimit() { description = "Returns dataset export formats with display name, media type, harvestability, user-interface visibility, and XML metadata when available.") public Response getExportFormats() { JsonObjectBuilder responseModel = JsonUtil.createObjectBuilder(); - ExportService instance = ExportService.getInstance(); - for (String[] labels : instance.getExportersLabels()) { - try { - Exporter exporter = instance.getExporter(labels[1]); - JsonObjectBuilder exporterObject = JsonUtil.createObjectBuilder().add("displayName", labels[0]) - .add("mediaType", exporter.getMediaType()).add("isHarvestable", exporter.isHarvestable()) - .add("isVisibleInUserInterface", exporter.isAvailableToUsers()); - if (exporter instanceof XMLExporter xmlExporter) { - exporterObject.add("XMLNameSpace", xmlExporter.getXMLNameSpace()) - .add("XMLSchemaLocation", xmlExporter.getXMLSchemaLocation()) - .add("XMLSchemaVersion", xmlExporter.getXMLSchemaVersion()); - } - responseModel.add(labels[1], exporterObject); - } - catch (ExportException ex){ - logger.warning("Failed to get: " + labels[1]); - logger.warning(ex.getLocalizedMessage()); + + for (ExporterRegistryBean.Details exporterDetail : exporterRegistrySvc.getDetails()) { + JsonObjectBuilder exporterObject = JsonUtil.createObjectBuilder() + .add("displayName", exporterDetail.localizedDisplayName()) + .add("mediaType", exporterDetail.mediaType()) + .add("isHarvestable", exporterDetail.isHarvestable()) + .add("isVisibleInUserInterface", exporterDetail.isAvailableToUsers()); + + if (exporterRegistrySvc.get(exporterDetail) instanceof XMLExporter xmlExporter) { + exporterObject.add("XMLNameSpace", xmlExporter.getXMLNameSpace()) + .add("XMLSchemaLocation", xmlExporter.getXMLSchemaLocation()) + .add("XMLSchemaVersion", xmlExporter.getXMLSchemaVersion()); } + + responseModel.add(exporterDetail.formatName(), exporterObject); } return ok(responseModel); } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Metadata.java b/src/main/java/edu/harvard/iq/dataverse/api/Metadata.java index 8e7ed211974..d980bd2097c 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Metadata.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Metadata.java @@ -7,10 +7,11 @@ import edu.harvard.iq.dataverse.Dataset; import edu.harvard.iq.dataverse.DatasetServiceBean; -import edu.harvard.iq.dataverse.export.ExportService; import java.util.Date; import java.util.logging.Logger; + +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import jakarta.ejb.EJB; import jakarta.ws.rs.*; @@ -23,6 +24,8 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; + import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.parameters.Parameter; import org.eclipse.microprofile.openapi.annotations.tags.Tag; @@ -167,18 +170,10 @@ private List validateFormatNames(String formats) { List formatNames = new ArrayList<>(Arrays.asList(formats.split(","))); - Set supportedFormatNames = new HashSet<>(); - for (String[] providerLabels : ExportService.getInstance().getExportersLabels()) { - supportedFormatNames.add(providerLabels[1]); - } - - //for (String formatName : formatNames) { - // if (!supportedFormatNames.contains(formatName)) { - // throw new BadRequestException(formatName + " is not a supported format"); - // } - //} - if (!supportedFormatNames.containsAll(formatNames)) { - throw new BadRequestException("Invalid/unsupported format name(s)"); + try { + exporterRegistrySvc.requireAllExist(formatNames); + } catch (IllegalArgumentException ex) { + throw new BadRequestException("Invalid/unsupported format name(s)" + ex.getMessage()); } return formatNames; From b6c9f46b9f27df244336f717ba3f59fb8ab512d9 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:26:12 +0200 Subject: [PATCH 38/39] refactor(signposting): replace ExportService.getInstance() with EJB-injected beans #11405 - Inject `ExporterRegistryBean` via `@EJB` in `DatasetPage` and pass it to `SignpostingResources`. - Replace `ExportService.getInstance().getExportersLabels()` loops with `exporterRegistry.getDetails()` iteration in both the `describedby` header and the linkset JSON. - Simplify `describedby` construction using a shared template string and `StringBuilder`. - Replace `mediaTypes.toString().isBlank()` with `mediaTypes.build().isEmpty()` for a more accurate emptiness check. - Remove unused imports (`ExportService`, `Json`). --- .../edu/harvard/iq/dataverse/DatasetPage.java | 5 +- .../dataverse/util/SignpostingResources.java | 65 ++++++++----------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java index 2c78873fa71..e3d4fef8301 100644 --- a/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java +++ b/src/main/java/edu/harvard/iq/dataverse/DatasetPage.java @@ -39,6 +39,7 @@ import edu.harvard.iq.dataverse.engine.command.impl.PublishDataverseCommand; import edu.harvard.iq.dataverse.engine.command.impl.UpdateDatasetVersionCommand; import edu.harvard.iq.dataverse.export.ExportService; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.util.cache.CacheFactoryBean; import edu.harvard.iq.dataverse.util.json.JsonUtil; import io.gdcc.spi.export.ExportException; @@ -255,6 +256,8 @@ public enum DisplayMode { DvObjectServiceBean dvObjectService; @EJB CacheFactoryBean cacheFactory; + @EJB + ExporterRegistryBean exporterRegistryService; @Inject DataverseRequestServiceBean dvRequestService; @Inject @@ -7000,7 +7003,7 @@ public String getSignpostingLinkHeader() { return null; } if (signpostingLinkHeader == null) { - SignpostingResources sr = new SignpostingResources(systemConfig, workingVersion, + SignpostingResources sr = new SignpostingResources(systemConfig, exporterRegistryService, workingVersion, JvmSettings.SIGNPOSTING_LEVEL1_AUTHOR_LIMIT.lookupOptional().orElse(""), JvmSettings.SIGNPOSTING_LEVEL1_ITEM_LIMIT.lookupOptional().orElse("")); signpostingLinkHeader = sr.getLinks(); diff --git a/src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java b/src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java index e26549736c1..917e80f5f20 100644 --- a/src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java +++ b/src/main/java/edu/harvard/iq/dataverse/util/SignpostingResources.java @@ -16,9 +16,8 @@ Two configurable options allow changing the limit for the number of authors or d import edu.harvard.iq.dataverse.*; import edu.harvard.iq.dataverse.dataset.DatasetUtil; -import edu.harvard.iq.dataverse.export.ExportService; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; import edu.harvard.iq.dataverse.util.json.JsonUtil; -import jakarta.json.Json; import jakarta.json.JsonArrayBuilder; import jakarta.json.JsonObjectBuilder; import org.apache.commons.validator.routines.UrlValidator; @@ -36,14 +35,16 @@ Two configurable options allow changing the limit for the number of authors or d public class SignpostingResources { private static final Logger logger = Logger.getLogger(SignpostingResources.class.getCanonicalName()); SystemConfig systemConfig; + ExporterRegistryBean exporterRegistry; DatasetVersion workingDatasetVersion; static final String defaultFileTypeValue = "https://schema.org/Dataset"; static final int defaultMaxLinks = 5; int maxAuthors; int maxItems; - public SignpostingResources(SystemConfig systemConfig, DatasetVersion workingDatasetVersion, String authorLimitSetting, String itemLimitSetting) { + public SignpostingResources(SystemConfig systemConfig, ExporterRegistryBean exporterRegistry, DatasetVersion workingDatasetVersion, String authorLimitSetting, String itemLimitSetting) { this.systemConfig = systemConfig; + this.exporterRegistry = exporterRegistry; this.workingDatasetVersion = workingDatasetVersion; maxAuthors = SystemConfig.getIntLimitFromStringOrDefault(authorLimitSetting, defaultMaxLinks); maxItems = SystemConfig.getIntLimitFromStringOrDefault(itemLimitSetting, defaultMaxLinks); @@ -75,19 +76,17 @@ public String getLinks() { valueList.add(items); } - String describedby = "<" + ds.getGlobalId().asURL().toString() + ">;rel=\"describedby\"" + ";type=\"" + "application/vnd.citationstyles.csl+json\""; - ExportService instance = ExportService.getInstance(); - for (String[] labels : instance.getExportersLabels()) { - String formatName = labels[1]; - Exporter exporter; - try { - exporter = ExportService.getInstance().getExporter(formatName); - describedby += ",<" + getExporterUrl(formatName, ds) + ">;rel=\"describedby\"" + ";type=\"" + exporter.getMediaType() + "\""; - } catch (ExportException ex) { - logger.warning("Could not look up exporter based on " + formatName + ". Exception: " + ex); - } - } - valueList.add(describedby); + String describedByTemplate = "<%s>;rel=\"describedby\";type=\"%s\""; + + StringBuilder describedBy = new StringBuilder(); + describedBy.append(describedByTemplate.formatted(ds.getGlobalId().asURL(), "application/vnd.citationstyles.csl+json")); + exporterRegistry.getDetails() + .forEach(detail -> describedBy.append( + describedByTemplate.formatted( + getExporterUrl(detail.formatName(), ds), + detail.mediaType() + ))); + valueList.add(describedBy.toString()); String type = ";rel=\"type\""; type = ";rel=\"type\",<" + defaultFileTypeValue + ">;rel=\"type\""; @@ -124,25 +123,16 @@ public JsonArrayBuilder getJsonLinkset() { "application/vnd.citationstyles.csl+json" ) ); - - ExportService instance = ExportService.getInstance(); - for (String[] labels : instance.getExportersLabels()) { - String formatName = labels[1]; - Exporter exporter; - try { - exporter = ExportService.getInstance().getExporter(formatName); - mediaTypes.add( - jsonObjectBuilder().add( - "href", getExporterUrl(formatName, ds) - ).add( - "type", - exporter.getMediaType() - ) - ); - } catch (ExportException ex) { - logger.warning("Could not look up exporter based on " + formatName + ". Exception: " + ex); - } - } + exporterRegistry.getDetails().forEach(detail -> + mediaTypes.add( + jsonObjectBuilder().add( + "href", getExporterUrl(detail.formatName(), ds) + ).add( + "type", + detail.mediaType() + ) + )); + JsonArrayBuilder linksetJsonObj = JsonUtil.createArrayBuilder(); JsonObjectBuilder mandatory; @@ -158,8 +148,9 @@ public JsonArrayBuilder getJsonLinkset() { if (licenseString != null && !licenseString.isBlank()) { mandatory.add("license", jsonObjectBuilder().add("href", licenseString)); } - if (!mediaTypes.toString().isBlank()) { - mandatory.add("describedby", mediaTypes); + var mediaTypesArray = mediaTypes.build(); + if (!mediaTypesArray.isEmpty()) { + mandatory.add("describedby", mediaTypesArray); } if (items != null) { mandatory.add("item", items); From 48cdce9e6cb281241993ab92f1b37fc9bd665e9e Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 21 Aug 2026 19:28:24 +0200 Subject: [PATCH 39/39] refactor(ui): replace ExportService.getInstance() with EJB-injected beans in FilePage #11405 - Inject `ExportServiceBean` and `ExporterRegistryBean` via `@EJB` in `FilePage`. - Rewrite from using `getExporters()` to stream over `exporterRegistryService.getDetails()`, replacing the manual `ExportService.getInstance().getExportersLabels()` loop and per-exporter null-checking. - Replace `ExportService.getInstance().exportAllFormats()` with the injected `exportService.exportAllFormats()`. - Remove unused imports --- .../edu/harvard/iq/dataverse/FilePage.java | 54 ++++++++----------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/FilePage.java b/src/main/java/edu/harvard/iq/dataverse/FilePage.java index 09dc360e7be..494f5c195ba 100644 --- a/src/main/java/edu/harvard/iq/dataverse/FilePage.java +++ b/src/main/java/edu/harvard/iq/dataverse/FilePage.java @@ -24,9 +24,10 @@ import edu.harvard.iq.dataverse.engine.command.impl.RestrictFileCommand; import edu.harvard.iq.dataverse.engine.command.impl.UningestFileCommand; import edu.harvard.iq.dataverse.engine.command.impl.UpdateDatasetVersionCommand; -import edu.harvard.iq.dataverse.export.ExportService; +import edu.harvard.iq.dataverse.export.service.ExportServiceBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean; +import edu.harvard.iq.dataverse.export.service.ExporterRegistryBean.Details; import io.gdcc.spi.export.ExportException; -import io.gdcc.spi.export.Exporter; import edu.harvard.iq.dataverse.externaltools.ExternalTool; import edu.harvard.iq.dataverse.externaltools.ExternalToolHandler; import edu.harvard.iq.dataverse.externaltools.ExternalToolServiceBean; @@ -35,7 +36,6 @@ import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean; import edu.harvard.iq.dataverse.makedatacount.MakeDataCountLoggingServiceBean.MakeDataCountEntry; import edu.harvard.iq.dataverse.privateurl.PrivateUrlServiceBean; -import edu.harvard.iq.dataverse.settings.FeatureFlags; import edu.harvard.iq.dataverse.settings.JvmSettings; import edu.harvard.iq.dataverse.settings.SettingsServiceBean; import edu.harvard.iq.dataverse.util.BundleUtil; @@ -63,7 +63,6 @@ import jakarta.faces.application.FacesMessage; import jakarta.faces.component.UIComponent; import jakarta.faces.context.FacesContext; -import jakarta.faces.validator.ValidatorException; import jakarta.faces.view.ViewScoped; import jakarta.inject.Inject; import jakarta.inject.Named; @@ -128,6 +127,10 @@ public class FilePage implements java.io.Serializable { IngestServiceBean ingestService; @EJB SystemConfig systemConfig; + @EJB + ExportServiceBean exportService; + @EJB + ExporterRegistryBean exporterRegistryService; @Inject @@ -463,30 +466,19 @@ public void setVersion(String version) { this.version = version; } - public List< String[]> getExporters(){ - List retList = new ArrayList<>(); - String myHostURL = systemConfig.getDataverseSiteUrl(); - for (String [] provider : ExportService.getInstance().getExportersLabels() ){ - String formatName = provider[1]; - String formatDisplayName = provider[0]; - - Exporter exporter = null; - try { - exporter = ExportService.getInstance().getExporter(formatName); - } catch (ExportException ex) { - exporter = null; - } - if (exporter != null && exporter.isAvailableToUsers()) { - // Not all metadata exports should be presented to the web users! - // Some are only for harvesting clients. - - String[] temp = new String[2]; - temp[0] = formatDisplayName; - temp[1] = myHostURL + "/api/datasets/export?exporter=" + formatName + "&persistentId=" + fileMetadata.getDatasetVersion().getDataset().getGlobalId().asString(); - retList.add(temp); - } - } - return retList; + public List getExporters(){ + String urlTemplate = systemConfig.getDataverseSiteUrl() + "/api/datasets/export?exporter=%s&persistentId=%s"; + + return exporterRegistryService.getDetails().stream() + .filter(Details::isAvailableToUsers) + .map(details -> new String[]{ + details.localizedDisplayName(), + urlTemplate.formatted( + details.formatName(), + fileMetadata.getDatasetVersion().getDataset().getGlobalId().asString() + ) + }) + .toList(); } public String saveProvFreeform(String freeformTextInput, DataFile dataFileFromPopup) throws CommandException { @@ -637,15 +629,13 @@ public String uningestFile() throws CommandException { editDataset = file.getOwner(); if (editDataset.isReleased()) { try { - ExportService instance = ExportService.getInstance(); - instance.exportAllFormats(editDataset); - + exportService.exportAllFormats(editDataset); } catch (ExportException ex) { // Something went wrong! // Just like with indexing, a failure to export is not a fatal // condition. We'll just log the error as a warning and keep // going: - logger.log(Level.WARNING, "Uningest: Exception while exporting:{0}", ex.getMessage()); + logger.log(Level.WARNING, "Uningest: Exception while exporting: {0}", ex); } } datafileService.save(file);