diff --git a/doc/release-notes/7000-mpconfig-support.md b/doc/release-notes/7000-mpconfig-support.md new file mode 100644 index 00000000000..01f21caf37a --- /dev/null +++ b/doc/release-notes/7000-mpconfig-support.md @@ -0,0 +1,8 @@ +# Broader MicroProfile Config Support for Developers + +As of this release, many [JVM options](https://guides.dataverse.org/en/latest/installation/config.html#jvm-options) +can be set using any [MicroProfile Config Source](https://docs.payara.fish/community/docs/Technical%20Documentation/MicroProfile/Config/Overview.html#config-sources). + +Currently this change is only relevant to developers but as settings are migrated to the new "lookup" pattern documented in the [Consuming Configuration](https://guides.dataverse.org/en/latest/developers/configuration.html) section of the Developer Guide, anyone installing the Dataverse software will have much greater flexibility when configuring those settings, especially within containers. These changes will be announced in future releases. + +Please note that an upgrade to Payara 5.2021.8 or higher is required to make use of this. Payara 5.2021.5 threw exceptions, as explained in PR #8823. diff --git a/doc/sphinx-guides/source/developers/configuration.rst b/doc/sphinx-guides/source/developers/configuration.rst index 0eac7de3134..fb15fea7900 100644 --- a/doc/sphinx-guides/source/developers/configuration.rst +++ b/doc/sphinx-guides/source/developers/configuration.rst @@ -18,12 +18,14 @@ authentication providers, harvesters and others. Simple Configuration Options ---------------------------- -Developers have accessed the simple properties via +Developers can access simple properties via: -1. ``System.getProperty(...)`` for JVM system property settings -2. ``SettingsServiceBean.get(...)`` for database settings and +1. ``JvmSettings..lookup(...)`` for JVM system property settings. +2. ``SettingsServiceBean.get(...)`` for database settings. 3. ``SystemConfig.xxx()`` for specially treated settings, maybe mixed from 1 and 2 and other sources. -4. ``SettingsWrapper`` must be used to obtain settings from 2 and 3 in frontend JSF (xhtml) pages. Please see the note on how to :ref:`avoid common efficiency issues with JSF render logic expressions `. +4. ``SettingsWrapper`` for use in frontend JSF (xhtml) pages to obtain settings from 2 and 3. Using the wrapper is a must for performance as explained in :ref:`avoid common efficiency issues with JSF render logic expressions + `. +5. ``System.getProperty()`` only for very special use cases not covered by ``JvmSettings``. As of Dataverse Software 5.3, we start to streamline our efforts into using a more consistent approach, also bringing joy and happiness to all the system administrators out there. This will be done by adopting the use of @@ -49,6 +51,7 @@ Developers benefit from: - Config API is also pushing for validation of configuration, as it's typesafe and converters for non-standard types can be added within our codebase. - Defaults in code or bundled in ``META-INF/microprofile-config.properties`` allow for optional values without much hassle. +- A single place to lookup any existing JVM setting in code, easier to keep in sync with the documentation. System administrators benefit from: @@ -57,9 +60,9 @@ System administrators benefit from: - Running a Dataverse installation in containers gets much easier when configuration can be provisioned in a streamlined fashion, mitigating the need for scripting glue and distinguishing between setting types. - Classic installations have a profit, too: we can enable using a single config file, e.g. living in - ``/etc/dataverse/config.properties``. + ``/etc/dataverse/config.properties`` by adding our own, hot-reload config source. - Features for monitoring resources and others are easier to use with this streamlined configuration, as we can - avoid people having to deal with ``asadmin`` commands and change a setting comfortably instead. + avoid people having to deal with ``asadmin`` commands and change a setting with comfort instead. Adopting MicroProfile Config API --------------------------------- @@ -68,33 +71,41 @@ This technology is introduced on a step-by-step basis. There will not be a big s Instead, we will provide backward compatibility by deprecating renamed or moved config options, while still supporting the old way of setting them. -- Introducing a new setting or moving and old one should result in a key ``dataverse..``. - That way we enable sys admins to recognize the meaning of an option and avoid name conflicts. +- Introducing a new setting or moving an old one should result in a scoped key + ``dataverse..``. That way we enable sys admins to recognize the meaning of an option + and avoid name conflicts. Starting with ``dataverse`` makes it perfectly clear that this is a setting meant for this application, which is important when using environment variables, system properties or other MPCONFIG sources. -- Replace ``System.getProperty()`` calls with either injected configs or retrieve programmatically if more complex - handling is necessary. If you rename the property, you should provide an alias. See below. -- Database settings need to be refactored in multiple steps. First you need to change the code retrieving it to use - MicroProfile Config API instead (just like above). Then you should provide an alias to retain backward compatibility. - See below. +- Replace ``System.getProperty()`` calls with ``JvmSettings..lookup(...)``, adding the setting there first. + This might be paired with renaming and providing backward-compatible aliases. +- Database settings need to be refactored in multiple steps and it is not yet clear how this will be done. + Many Database settings are of very static nature and might be moved to JVM settings (in backward compatible ways). -Moving or Replacing a JVM Setting -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Adding a JVM Setting +^^^^^^^^^^^^^^^^^^^^ -When moving an old key to a new (especially when doing so with a former JVM system property setting), you should -add an alias to ``src/main/resources/META-INF/microprofile-aliases.properties`` to enable backward compatibility. -The format is always like ``dataverse..newname...=old.property.name``. +Whenever a new option gets added or an existing configuration gets migrated to +``edu.harvard.iq.dataverse.settings.JvmSettings``, you will attach the setting to an existing scope or create new +sub-scopes first. -Details can be found in ``edu.harvard.iq.dataverse.settings.source.AliasConfigSource`` +- Scopes and settings are organised in a tree-like structure within a single enum ``JvmSettings``. +- The root scope is "dataverse". +- All sub-scopes are below that. +- Scopes are separated by dots (periods). +- A scope may be a placeholder, filled with a variable during lookup. (Named object mapping.) -Aliasing Database Setting -^^^^^^^^^^^^^^^^^^^^^^^^^ +Any consumer of the setting can choose to use one of the fluent ``lookup()`` methods, which hides away alias handling, +conversion etc from consuming code. See also the detailed Javadoc for these methods. -When moving a database setting (``:ExampleSetting``), configure an alias -``dataverse.my.example.setting=dataverse.settings.fromdb.ExampleSetting`` in -``src/main/resources/META-INF/microprofile-aliases.properties``. This will enable backward compatibility. +Moving or Replacing a JVM Setting +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +When moving an old key to a new (especially when doing so with a former JVM system property setting), you should +add an alias to the ``JvmSettings`` definition to enable backward compatibility. Old names given there are capable of +being used with patterned lookups. -A database setting with an i18n attribute using *lang* will have available language codes appended to the name. -Example: ``dataverse.settings.fromdb.ExampleI18nSetting.en``, ``dataverse.settings.fromdb.ExampleI18nSetting.de`` +Another option is to add the alias in ``src/main/resources/META-INF/microprofile-aliases.properties``. The format is +always like ``dataverse..newname...=old.property.name``. Note this doesn't provide support for patterned +aliases. -More details in ``edu.harvard.iq.dataverse.settings.source.DbSettingConfigSource`` +Details can be found in ``edu.harvard.iq.dataverse.settings.source.AliasConfigSource`` diff --git a/doc/sphinx-guides/source/developers/testing.rst b/doc/sphinx-guides/source/developers/testing.rst index 7bde4055e33..4b3d5fd0a55 100755 --- a/doc/sphinx-guides/source/developers/testing.rst +++ b/doc/sphinx-guides/source/developers/testing.rst @@ -79,6 +79,22 @@ greatly extended parameterized testing. Some guidance how to write those: - https://blog.codefx.org/libraries/junit-5-parameterized-tests/ - See also some examples in our codebase. +JUnit 5 Test Helper Extensions +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Our codebase provides little helpers to ease dealing with state during tests. +Some tests might need to change something which should be restored after the test ran. + +For unit tests, the most interesting part is to set a JVM setting just for the current test. +Please use the ``@JvmSetting(key = JvmSettings.XXX, value = "")`` annotation on a test method or +a test class to set and clear the property automatically. + +To set arbitrary system properties for the current test, a similar extension +``@SystemProperty(key = "", value = "")`` has been added. + +Both extensions will ensure the global state of system properties is non-interfering for +test executions. Tests using these extensions will be executed in serial. + Observing Changes to Code Coverage ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/modules/dataverse-parent/pom.xml b/modules/dataverse-parent/pom.xml index 22ea30795ba..14b84f80279 100644 --- a/modules/dataverse-parent/pom.xml +++ b/modules/dataverse-parent/pom.xml @@ -164,7 +164,7 @@ 1.15.0 - 0.4.1 + 2.10.1 4.13.1 5.7.0 diff --git a/pom.xml b/pom.xml index 7f8b3ffa258..585b7ee30a9 100644 --- a/pom.xml +++ b/pom.xml @@ -613,9 +613,9 @@ test - org.microbean - microbean-microprofile-config - ${microbean-mpconfig.version} + io.smallrye.config + smallrye-config + ${smallrye-mpconfig.version} test @@ -653,10 +653,17 @@ **/*.xml **/firstNames/*.* **/*.xsl - **/*.properties **/services/* + + src/main/resources + + true + + **/*.properties + + diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java new file mode 100644 index 00000000000..223e4b86da9 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java @@ -0,0 +1,352 @@ +package edu.harvard.iq.dataverse.settings; + +import org.eclipse.microprofile.config.ConfigProvider; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +/** + * Enum to store each and every JVM-based setting as a reference, + * much like the enum {@link SettingsServiceBean.Key} for DB settings. + * + * To be able to have more control over JVM settings names, + * avoid typos, maybe create lists of settings and so on, + * this enum will provide the place to add any old and new + * settings that are destined to be made at the JVM level. + * + * Further future extensions of this enum class include + * - adding predicates for validation and + * - adding data manipulation for aliased config names. + * + * To create a setting, simply add it within a scope: + * {@link JvmSettings#JvmSettings(JvmSettings, String)} + * + * Settings that might get renamed may provide their old names as aliases: + * {@link JvmSettings#JvmSettings(JvmSettings, String, String...)} + * + * Some scopes or settings may need one or more placeholders, simply don't give + * a key in these cases: + * {@link JvmSettings#JvmSettings(JvmSettings)} + * + */ +public enum JvmSettings { + // the upmost root scope - every setting shall start with it. + PREFIX("dataverse"), + + // GENERAL SETTINGS + VERSION(PREFIX, "version"), + BUILD(PREFIX, "build"), + + ; + + private static final String SCOPE_SEPARATOR = "."; + public static final String PLACEHOLDER_KEY = "%s"; + private static final Pattern OLD_NAME_PLACEHOLDER_PATTERN = Pattern.compile("%(\\d\\$)?s"); + + private final String key; + private final String scopedKey; + private final JvmSettings parent; + private final List oldNames; + private final int placeholders; + + /** + * Create a root scope. + * @param key The scopes name. + */ + JvmSettings(String key) { + this.key = key; + this.scopedKey = key; + this.parent = null; + this.oldNames = List.of(); + this.placeholders = 0; + } + + /** + * Create a scope or setting with a placeholder for a variable argument in it. + * Used to create "configurable objects" with certain attributes using dynamic, programmatic lookup. + * + * Any placeholder present in a settings full scoped key will be replaced when looked up + * via {@link #lookup(Class, String...)}. + * + * @param scope The parent scope. + */ + JvmSettings(JvmSettings scope) { + this.key = PLACEHOLDER_KEY; + this.scopedKey = scope.scopedKey + SCOPE_SEPARATOR + this.key; + this.parent = scope; + this.oldNames = List.of(); + this.placeholders = scope.placeholders + 1; + } + + /** + * Create a scope or setting with name it and associate with a parent scope. + * @param scope The parent scope. + * @param key The name of this scope or setting. + */ + JvmSettings(JvmSettings scope, String key) { + this.key = key; + this.scopedKey = scope.scopedKey + SCOPE_SEPARATOR + key; + this.parent = scope; + this.oldNames = List.of(); + this.placeholders = scope.placeholders; + } + + /** + * Create a setting with name it and associate with a parent scope. + * (Could also be a scope, but old names for scopes aren't the way this is designed.) + * + * When old names are given, these need to be given as fully scoped setting names! (Otherwise + * it would not be possible to switch between completely different scopes.) + * + * @param scope The parent scope of this setting. + * @param key The name of the setting. + * @param oldNames Any previous names this setting was known as. + * Must be given as fully scopes names, not just the old unscoped key/name. + * Used by {@link edu.harvard.iq.dataverse.settings.source.AliasConfigSource} to allow backward + * compatible, non-breaking deprecation and switching to new setting names. + */ + JvmSettings(JvmSettings scope, String key, String... oldNames) { + this.key = key; + this.scopedKey = scope.scopedKey + SCOPE_SEPARATOR + key; + this.parent = scope; + this.oldNames = Arrays.stream(oldNames).collect(Collectors.toUnmodifiableList()); + this.placeholders = scope.placeholders; + } + + private static final List aliased = new ArrayList<>(); + static { + for (JvmSettings setting : JvmSettings.values()) { + if (!setting.oldNames.isEmpty()) { + aliased.add(setting); + } + } + } + + /** + * Get all settings having old names to include them in {@link edu.harvard.iq.dataverse.settings.source.AliasConfigSource} + * @return List of settings with old alias names. Can be empty, but will not be null. + */ + public static List getAliasedSettings() { + return Collections.unmodifiableList(aliased); + } + + /** + * Return a list of old names to be used as aliases for backward compatibility. + * Will return empty list if no old names present. + * + * This method should only be used by {@link edu.harvard.iq.dataverse.settings.source.AliasConfigSource}. + * In case of a setting containing placeholder(s), it will check any old names given in the definition + * for presence of at least one placeholder plus it doesn't use more placeholders than available. + * (Old names containing placeholders for settings without any are checked, too.) + * + * Violations will result in a {@link IllegalArgumentException} and will be noticed during any test execution. + * A developer must fix the old name definition before shipping the code. + * + * @return List of old names, may be empty, but never null. + * @throws IllegalArgumentException When an old name has no or too many placeholders for this setting. + */ + public List getOldNames() { + if (needsVarArgs()) { + for (String name : oldNames) { + long matches = OLD_NAME_PLACEHOLDER_PATTERN.matcher(name).results().count(); + + if (matches == 0) { + throw new IllegalArgumentException("JvmSettings." + this.name() + "'s old name '" + + name + "' needs at least one placeholder"); + } else if (matches > this.placeholders) { + throw new IllegalArgumentException("JvmSettings." + this.name() + "'s old name '" + + name + "' has more placeholders than the current name"); + } + } + } else if (! this.oldNames.stream().noneMatch(OLD_NAME_PLACEHOLDER_PATTERN.asPredicate())) { + throw new IllegalArgumentException("JvmSettings." + this.name() + " has no placeholder but old name requires it"); + } + + return oldNames; + } + + /** + * Retrieve the scoped key for this setting. Scopes are separated by dots. + * If the setting contains placeholders, these will be represented as {@link #PLACEHOLDER_KEY}. + * + * @return The scoped key (or the key if no scope). Example: dataverse.subscope.subsubscope.key + */ + public String getScopedKey() { + return this.scopedKey; + } + + public Pattern getPatternizedKey() { + return Pattern.compile( + getScopedKey() + .replace(SCOPE_SEPARATOR, "\\.") + .replace(PLACEHOLDER_KEY, "(.+?)")); + } + + + /** + * Does this setting carry and placeholders for variable arguments? + * @return True if so, False otherwise. + */ + public boolean needsVarArgs() { + return this.placeholders > 0; + } + + /** + * Return the number of placeholders / variable arguments are necessary to lookup this setting. + * An exact match in the number of arguments will be necessary for a successful lookup. + * @return Number of placeholders for this scoped setting. + */ + public int numberOfVarArgs() { + return placeholders; + } + + /** + * Lookup this setting via MicroProfile Config as a required option (it will fail if not present). + * @throws java.util.NoSuchElementException - if the property is not defined or is defined as an empty string + * @return The setting as a String + */ + public String lookup() { + return lookup(String.class); + } + + /** + * Lookup this setting via MicroProfile Config as an optional setting. + * @return The setting as String wrapped in a (potentially empty) Optional + */ + public Optional lookupOptional() { + return lookupOptional(String.class); + } + + /** + * Lookup this setting via MicroProfile Config as a required option (it will fail if not present). + * + * @param klass The target type class to convert the setting to if found and not null + * @return The setting as an instance of {@link T} + * @param Target type to convert the setting to (you can create custom converters) + * + * @throws java.util.NoSuchElementException When the property is not defined or is defined as an empty string. + * @throws IllegalArgumentException When the settings value could not be converted to target type. + */ + public T lookup(Class klass) { + if (needsVarArgs()) { + throw new IllegalArgumentException("Cannot lookup a setting containing placeholders with this method."); + } + + // This must be done with the full-fledged lookup, as we cannot store the config in an instance or static + // variable, as the alias config source depends on this enum (circular dependency). This is easiest + // avoided by looking up the static cached config at the cost of a method invocation. + return ConfigProvider.getConfig().getValue(this.getScopedKey(), klass); + } + + /** + * Lookup this setting via MicroProfile Config as an optional setting. + * + * @param klass The target type class to convert the setting to if found and not null + * @param Target type to convert the setting to (you can create custom converters) + * @return The setting as an instance of {@link Optional} or an empty Optional + * + * @throws IllegalArgumentException When the settings value could not be converted to target type. + */ + public Optional lookupOptional(Class klass) { + if (needsVarArgs()) { + throw new IllegalArgumentException("Cannot lookup a setting containing variable arguments with this method."); + } + + // This must be done with the full-fledged lookup, as we cannot store the config in an instance or static + // variable, as the alias config source depends on this enum (circular dependency). This is easiest + // avoided by looking up the static cached config at the cost of a method invocation. + return ConfigProvider.getConfig().getOptionalValue(this.getScopedKey(), klass); + } + + /** + * Lookup a required setting containing placeholders for arguments like a name and return as plain String. + * To use type conversion, use {@link #lookup(Class, String...)}. + * + * @param arguments The var args to replace the placeholders of this setting. + * @return The value of the setting. + * + * @throws java.util.NoSuchElementException When the setting has not been set in any MPCONFIG source or is an empty string. + * @throws IllegalArgumentException When using it on a setting without placeholders. + * @throws IllegalArgumentException When not providing as many arguments as there are placeholders. + */ + public String lookup(String... arguments) { + return lookup(String.class, arguments); + } + + /** + * Lookup an optional setting containing placeholders for arguments like a name and return as plain String. + * To use type conversion, use {@link #lookupOptional(Class, String...)}. + * + * @param arguments The var args to replace the placeholders of this setting. + * @return The value as an instance of {@link Optional} or an empty Optional + * + * @throws IllegalArgumentException When using it on a setting without placeholders. + * @throws IllegalArgumentException When not providing as many arguments as there are placeholders. + */ + public Optional lookupOptional(String... arguments) { + return lookupOptional(String.class, arguments); + } + + /** + * Lookup a required setting containing placeholders for arguments like a name and return as converted type. + * To avoid type conversion, use {@link #lookup(String...)}. + * + * @param klass The target type class. + * @param arguments The var args to replace the placeholders of this setting. + * @param Target type to convert the setting to (you can create custom converters) + * @return The value of the setting, converted to the given type. + * + * @throws java.util.NoSuchElementException When the setting has not been set in any MPCONFIG source or is an empty string. + * @throws IllegalArgumentException When using it on a setting without placeholders. + * @throws IllegalArgumentException When not providing as many arguments as there are placeholders. + * @throws IllegalArgumentException When the settings value could not be converted to the target type. + */ + public T lookup(Class klass, String... arguments) { + if (needsVarArgs()) { + if (arguments == null || arguments.length != placeholders) { + throw new IllegalArgumentException("You must specify " + placeholders + " placeholder lookup arguments."); + } + return ConfigProvider.getConfig().getValue(this.insert(arguments), klass); + } + throw new IllegalArgumentException("Cannot lookup a setting without variable arguments with this method."); + } + + /** + * Lookup an optional setting containing placeholders for arguments like a name and return as converted type. + * To avoid type conversion, use {@link #lookupOptional(String...)}. + * + * @param klass The target type class. + * @param arguments The var args to replace the placeholders of this setting. + * @param Target type to convert the setting to (you can create custom converters) + * @return The value as an instance of {@link Optional} or an empty Optional + * + * @throws IllegalArgumentException When using it on a setting without placeholders. + * @throws IllegalArgumentException When not providing as many arguments as there are placeholders. + * @throws IllegalArgumentException When the settings value could not be converted to the target type. + */ + public Optional lookupOptional(Class klass, String... arguments) { + if (needsVarArgs()) { + if (arguments == null || arguments.length != placeholders) { + throw new IllegalArgumentException("You must specify " + placeholders + " placeholder lookup arguments."); + } + return ConfigProvider.getConfig().getOptionalValue(this.insert(arguments), klass); + } + throw new IllegalArgumentException("Cannot lookup a setting without variable arguments with this method."); + } + + /** + * Inject arguments into the placeholders of this setting. Will not do anything when no placeholders present. + * + * @param arguments The variable arguments to be inserted for the placeholders. + * @return The formatted setting name. + */ + public String insert(String... arguments) { + return String.format(this.getScopedKey(), (Object[]) arguments); + } + +} diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSource.java b/src/main/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSource.java index fbdbd982085..407f39ce0f9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSource.java +++ b/src/main/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSource.java @@ -1,31 +1,37 @@ package edu.harvard.iq.dataverse.settings.source; +import edu.harvard.iq.dataverse.settings.JvmSettings; import org.eclipse.microprofile.config.ConfigProvider; import org.eclipse.microprofile.config.spi.ConfigSource; import java.io.IOException; +import java.io.InputStream; import java.net.URL; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Properties; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.logging.Level; import java.util.logging.Logger; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Enable using an old name for a new config name. * Usages will be logged and this source will ALWAYS stand back if the new name is used anywhere. - * - * By using a DbSettingConfigSource value (dataverse.settings.fromdb.XXX) as old name, we can - * alias a new name to an old db setting, enabling backward compatibility. */ public final class AliasConfigSource implements ConfigSource { private static final Logger logger = Logger.getLogger(AliasConfigSource.class.getName()); + private static final String ALIASES_PROP_FILE = "META-INF/microprofile-aliases.properties"; - private final ConcurrentHashMap aliases = new ConcurrentHashMap<>(); - private final String ALIASES_PROP_FILE = "META-INF/microprofile-aliases.properties"; + private final ConcurrentHashMap> aliases = new ConcurrentHashMap<>(); + private final ConcurrentHashMap> varArgAliases = new ConcurrentHashMap<>(); public AliasConfigSource() { try { @@ -34,33 +40,63 @@ public AliasConfigSource() { // store in our aliases map importAliases(aliasProps); } catch (IOException e) { - logger.info("Could not read from "+ALIASES_PROP_FILE+". Skipping MPCONFIG alias setup."); + // Usually it's an anti-pattern to catch the exception here, but skipping the file + // should be fine here, as it's optional. + logger.log(Level.INFO, "Could not read from "+ALIASES_PROP_FILE+". Skipping MPCONFIG alias setup.", e); } + + // Store all old names from JvmSettings + importJvmSettings(JvmSettings.getAliasedSettings()); + } + + private void importJvmSettings(List aliasedSettings) { + // First add all simple aliases not containing placeholders + aliasedSettings.stream() + .filter(s -> ! s.needsVarArgs()) + .forEach(setting -> aliases.put(setting.getScopedKey(), setting.getOldNames())); + + // Aliases with placeholders need to be compiled into a regex + aliasedSettings.stream() + .filter(JvmSettings::needsVarArgs) + .forEach(setting -> varArgAliases.put(setting.getPatternizedKey(), setting.getOldNames())); } - Properties readAliases(String filePath) throws IOException { + + private Properties readAliases(String filePath) throws IOException { // get resource from classpath ClassLoader classLoader = this.getClass().getClassLoader(); URL aliasesResource = classLoader.getResource(filePath); + + // Prevent errors if file not found or could not be loaded + if (aliasesResource == null) { + throw new IOException("Could not find or load, class loader returned null"); + } // load properties from file resource (parsing included) Properties aliasProps = new Properties(); - try { - aliasProps.load(aliasesResource.openStream()); - } catch (NullPointerException e) { - throw new IOException(e.getMessage()); + try (InputStream propStream = aliasesResource.openStream()) { + aliasProps.load(propStream); } return aliasProps; } - void importAliases(Properties aliasProps) { - aliasProps.forEach((key, value) -> aliases.put(key.toString(), value.toString())); + private void importAliases(Properties aliasProps) { + aliasProps.forEach((key, value) -> aliases.put(key.toString(), List.of(value.toString()))); + } + + // Has visibility "package" to be usable from test class! + void addAlias(String newName, List oldNames) { + this.aliases.put(newName, oldNames); + } + + // Has visibility "package" to be usable from test class! + void addAlias(Pattern newNamePattern, List oldNamePatterns) { + this.varArgAliases.put(newNamePattern, oldNamePatterns); } @Override public Map getProperties() { - // No, just no. We're not going to drop a list of stuff. We're only - // dealiasing on getValue(); + // No, just no. We're not going to drop a list of stuff. We're only de-aliasing on calls to getValue() return new HashMap<>(); } @@ -79,16 +115,63 @@ public int getOrdinal() { @Override public String getValue(String key) { - String value = null; + + // If the key is null or not starting with the prefix ("dataverse"), we are not going to jump through loops, + // avoiding computation overhead + if (key == null || ! key.startsWith(JvmSettings.PREFIX.getScopedKey())) { + return null; + } + + List oldNames = new ArrayList<>(); + + // Retrieve simple cases without placeholders if (this.aliases.containsKey(key)) { - String oldKey = this.aliases.get(key); - value = ConfigProvider.getConfig().getOptionalValue(oldKey, String.class).orElse(null); + oldNames.addAll(this.aliases.get(key)); + // Or try with regex patterns + } else { + // Seek for the given key within all the patterns for placeholder containing settings, + // returning a Matcher to extract the variable arguments as regex match groups. + Optional foundMatcher = varArgAliases.keySet().stream() + .map(pattern -> pattern.matcher(key)) + .filter(Matcher::matches) + .findFirst(); - if (value != null) { - logger.warning("Detected deprecated config option '"+oldKey+"' in use. Please update your config to use '"+key+"'."); + // Extract the matched groups and construct all old setting names with them + if (foundMatcher.isPresent()) { + Matcher matcher = foundMatcher.get(); + + List varArgs = new ArrayList<>(); + for (int i = 1; i <= matcher.groupCount(); i++) { + varArgs.add(matcher.group(i)); + } + Object[] args = varArgs.toArray(); + + this.varArgAliases + .get(matcher.pattern()) + .forEach(oldNamePattern -> oldNames.add(String.format(oldNamePattern, args))); + } + } + + // Return the first non-empty result + // NOTE: When there are multiple old names in use, they would conflict anyway. Upon deletion of one of the + // old settings the other becomes visible and triggers the warning again. There might even be different + // old settings in different sources, which might conflict, too (see ordinal value). + // NOTE: As the default is an empty oldNames array, loop will only be executed if anything was found before. + for (String oldName : oldNames) { + Optional value = ConfigProvider.getConfig().getOptionalValue(oldName, String.class); + + if (value.isPresent()) { + logger.log( + Level.WARNING, + "Detected deprecated config option {0} in use. Please update your config to use {1}.", + new String[]{oldName, key} + ); + return value.get(); } } - return value; + + // Sane default: nothing found. + return null; } @Override diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigHelper.java b/src/main/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigHelper.java deleted file mode 100644 index 7b9783dee06..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigHelper.java +++ /dev/null @@ -1,27 +0,0 @@ -package edu.harvard.iq.dataverse.settings.source; - -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; - -import javax.annotation.PostConstruct; -import javax.ejb.EJB; -import javax.ejb.Singleton; -import javax.ejb.Startup; - -/** - * This is a small helper bean for the MPCONFIG DbSettingConfigSource. - * As it is a singleton and built at application start (=deployment), it will inject the (stateless) - * settings service into the MPCONFIG POJO once it's ready. - * - * MPCONFIG requires it's sources to be POJOs. No direct dependency injection possible. - */ -@Singleton -@Startup -public class DbSettingConfigHelper { - @EJB - SettingsServiceBean settingsSvc; - - @PostConstruct - public void injectService() { - DbSettingConfigSource.injectSettingsService(settingsSvc); - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigSource.java b/src/main/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigSource.java deleted file mode 100644 index 838cd415819..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigSource.java +++ /dev/null @@ -1,83 +0,0 @@ -package edu.harvard.iq.dataverse.settings.source; - -import edu.harvard.iq.dataverse.settings.Setting; -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; -import org.eclipse.microprofile.config.spi.ConfigSource; - -import java.time.Duration; -import java.time.Instant; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.logging.Logger; - -/** - * A caching wrapper around SettingServiceBean to provide database settings to MicroProfile Config API. - * Please be aware that this class relies on dependency injection during the application startup. - * Values will not be available before and a severe message will be logged to allow monitoring (potential race conditions) - * The settings will be cached for at least one minute, avoiding unnecessary database calls. - */ -public class DbSettingConfigSource implements ConfigSource { - - private static final Logger logger = Logger.getLogger(DbSettingConfigSource.class.getCanonicalName()); - private static final ConcurrentHashMap properties = new ConcurrentHashMap<>(); - private static Instant lastUpdate; - private static SettingsServiceBean settingsSvc; - public static final String PREFIX = "dataverse.settings.fromdb"; - - /** - * Let the SettingsServiceBean be injected by DbSettingConfigHelper with PostConstruct - * @param injected - */ - public static void injectSettingsService(SettingsServiceBean injected) { - settingsSvc = injected; - updateProperties(); - } - - /** - * Retrieve settings from the database via service and update cache. - */ - public static void updateProperties() { - // skip if the service has not been injected yet - if (settingsSvc == null) { - return; - } - properties.clear(); - Set dbSettings = settingsSvc.listAll(); - dbSettings.forEach(s -> properties.put(PREFIX+"."+s.getName().substring(1) + (s.getLang() == null ? "" : "."+s.getLang()), s.getContent())); - lastUpdate = Instant.now(); - } - - @Override - public Map getProperties() { - // if the cache is at least XX number of seconds old, update before serving data. - if (lastUpdate == null || Instant.now().minus(Duration.ofSeconds(60)).isAfter(lastUpdate)) { - updateProperties(); - } - return properties; - } - - @Override - public Set getPropertyNames() { - return getProperties().keySet(); - } - - @Override - public int getOrdinal() { - return 50; - } - - @Override - public String getValue(String key) { - // log usages for which this has been designed, but not yet ready to serve... - if (settingsSvc == null && key.startsWith(PREFIX)) { - logger.severe("MPCONFIG DbSettingConfigSource not ready yet, but requested for '"+key+"'."); - } - return getProperties().getOrDefault(key, null); - } - - @Override - public String getName() { - return "DataverseDB"; - } -} diff --git a/src/main/java/edu/harvard/iq/dataverse/settings/spi/DbSettingConfigSourceProvider.java b/src/main/java/edu/harvard/iq/dataverse/settings/spi/DbSettingConfigSourceProvider.java deleted file mode 100644 index 856a2c64a01..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/settings/spi/DbSettingConfigSourceProvider.java +++ /dev/null @@ -1,14 +0,0 @@ -package edu.harvard.iq.dataverse.settings.spi; - -import edu.harvard.iq.dataverse.settings.source.DbSettingConfigSource; -import org.eclipse.microprofile.config.spi.ConfigSource; -import org.eclipse.microprofile.config.spi.ConfigSourceProvider; - -import java.util.Arrays; - -public class DbSettingConfigSourceProvider implements ConfigSourceProvider { - @Override - public Iterable getConfigSources(ClassLoader forClassLoader) { - return Arrays.asList(new DbSettingConfigSource()); - } -} \ No newline at end of file diff --git a/src/main/resources/META-INF/microprofile-config.properties b/src/main/resources/META-INF/microprofile-config.properties index 09d71dfbf3a..16298d83118 100644 --- a/src/main/resources/META-INF/microprofile-config.properties +++ b/src/main/resources/META-INF/microprofile-config.properties @@ -1,3 +1,8 @@ +# GENERAL +# Will be replaced by Maven property in /target via filtering (see ) +dataverse.version=${project.version} +dataverse.build= + # DATABASE dataverse.db.host=localhost dataverse.db.port=5432 diff --git a/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProvider b/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProvider index f2e23ca1b4e..796f03d7ce3 100644 --- a/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProvider +++ b/src/main/resources/META-INF/services/org.eclipse.microprofile.config.spi.ConfigSourceProvider @@ -1,2 +1 @@ edu.harvard.iq.dataverse.settings.spi.AliasConfigSourceProvider -edu.harvard.iq.dataverse.settings.spi.DbSettingConfigSourceProvider diff --git a/src/test/java/edu/harvard/iq/dataverse/settings/JvmSettingsTest.java b/src/test/java/edu/harvard/iq/dataverse/settings/JvmSettingsTest.java new file mode 100644 index 00000000000..68458f6623c --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/settings/JvmSettingsTest.java @@ -0,0 +1,20 @@ +package edu.harvard.iq.dataverse.settings; + +import edu.harvard.iq.dataverse.util.testing.JvmSetting; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class JvmSettingsTest { + @Test + @JvmSetting(key = JvmSettings.VERSION, value = "foobar") + void lookupSetting() { + assertEquals("foobar", JvmSettings.VERSION.lookup()); + assertEquals("foobar", JvmSettings.VERSION.lookupOptional().orElse("")); + } + + /* + * TODO: add more tests here for features like old names, patterned settings etc when adding + * these in other pull requests adding new settings making use of these features. + */ +} \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSourceTest.java b/src/test/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSourceTest.java index 36c4d99f743..621423b3121 100644 --- a/src/test/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSourceTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/settings/source/AliasConfigSourceTest.java @@ -1,41 +1,95 @@ package edu.harvard.iq.dataverse.settings.source; +import edu.harvard.iq.dataverse.util.testing.SystemProperty; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import java.io.IOException; -import java.util.Properties; +import java.util.List; +import java.util.regex.Pattern; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; class AliasConfigSourceTest { - AliasConfigSource source = new AliasConfigSource(); + static AliasConfigSource source = new AliasConfigSource(); + + static final String VALUE = "test1234"; + + static final String SIMPLE_NEW = "dataverse.test.foobar"; + static final String SIMPLE_OLD = "dataverse.former.hello"; + static final String SIMPLE_MULTI_NEW = "dataverse.test.multi"; + static final String SIMPLE_MULTI_OLD = "former.2"; + + static final Pattern PATTERNED_NEW_PATTERN = Pattern.compile("dataverse\\.(.+?)\\.next\\.test"); + static final String PATTERNED_NEW_NAME = "dataverse.single.next.test"; + static final String PATTERNED_OLD_PATTERN = "dataverse.%s.test.foobar"; + static final String PATTERNED_OLD_NAME = "dataverse.single.test.foobar"; + + static final Pattern DUAL_PATTERNED_NEW_PATTERN = Pattern.compile("dataverse\\.(.+?)\\.two\\.(.+?)\\.foobar"); + static final String DUAL_PATTERNED_NEW_NAME = "dataverse.single.two.test.foobar"; + static final String DUAL_PATTERNED_OLD_PATTERN = "dataverse.%s.test.%s.foobar"; + static final String DUAL_PATTERNED_OLD_NAME = "dataverse.single.test.test.foobar"; + + static final Pattern PATTERNED_MULTI_NEW_PATTERN = Pattern.compile("dataverse\\.(.+?)\\.next\\.test2"); + static final String PATTERNED_MULTI_NEW_NAME = "dataverse.multi.next.test2"; + static final String PATTERNED_MULTI_OLD_PATTERN = "dataverse.%s.test.foobar2"; + static final String PATTERNED_MULTI_OLD_NAME = "dataverse.multi.test.foobar2"; + + static final Pattern DUALSINGLE_PATTERNED_NEW_PATTERN = Pattern.compile("dataverse\\.(.+?)\\.(.+?)\\.test2"); + static final String DUALSINGLE_PATTERNED_NEW_NAME = "dataverse.multi.next.test2"; + static final String DUALSINGLE_PATTERNED_OLD_PATTERN = "dataverse.%s.test.foobar2"; + static final String DUALSINGLE_PATTERNED_OLD_NAME = "dataverse.multi.test.foobar2"; + + @BeforeAll + static void setUp() { + source.addAlias(SIMPLE_NEW, List.of(SIMPLE_OLD)); + source.addAlias(SIMPLE_MULTI_NEW, List.of("former.1", SIMPLE_MULTI_OLD, "former.3")); + source.addAlias(PATTERNED_NEW_PATTERN, List.of(PATTERNED_OLD_PATTERN)); + source.addAlias(DUAL_PATTERNED_NEW_PATTERN, List.of(DUAL_PATTERNED_OLD_PATTERN)); + source.addAlias(PATTERNED_MULTI_NEW_PATTERN, List.of("dataverse.%s.test1.foobar1", PATTERNED_MULTI_OLD_PATTERN, "dataverse.test.%s.test")); + source.addAlias(DUALSINGLE_PATTERNED_NEW_PATTERN, List.of(DUALSINGLE_PATTERNED_OLD_PATTERN)); + } + + @Test + void testNullIfNotInScope() { + assertNull(source.getValue(null)); + assertNull(source.getValue("test.out.of.scope")); + } + + @Test + @SystemProperty(key = SIMPLE_OLD, value = VALUE) + void testSimpleAlias() { + assertEquals(VALUE, source.getValue(SIMPLE_NEW)); + } + + @Test + @SystemProperty(key = SIMPLE_MULTI_OLD, value = VALUE) + void testSimpleMultipleAlias() { + assertEquals(VALUE, source.getValue(SIMPLE_MULTI_NEW)); + } + + @Test + @SystemProperty(key = PATTERNED_OLD_NAME, value = VALUE) + void testPatternedAlias() { + assertEquals(VALUE, source.getValue(PATTERNED_NEW_NAME)); + } + + @Test + @SystemProperty(key = DUAL_PATTERNED_OLD_NAME, value = VALUE) + void testDualPatternedAlias() { + assertEquals(VALUE, source.getValue(DUAL_PATTERNED_NEW_NAME)); + } @Test - void getValue() { - // given - System.setProperty("dataverse.hello.foobar", "test"); - Properties aliases = new Properties(); - aliases.setProperty("dataverse.goodbye.foobar", "dataverse.hello.foobar"); - - // when - source.importAliases(aliases); - - // then - assertEquals("test", source.getValue("dataverse.goodbye.foobar")); + @SystemProperty(key = PATTERNED_MULTI_OLD_NAME, value = VALUE) + void testPatternedMultipleAlias() { + assertEquals(VALUE, source.getValue(PATTERNED_MULTI_NEW_NAME)); } @Test - void readImportTestAliasesFromFile() throws IOException { - // given - System.setProperty("dataverse.old.example", "test"); - String filePath = "test-microprofile-aliases.properties"; - - // when - Properties aliases = source.readAliases(filePath); - source.importAliases(aliases); - - // then - assertEquals("test", source.getValue("dataverse.new.example")); + @SystemProperty(key = DUALSINGLE_PATTERNED_OLD_NAME, value = VALUE) + void testDualSinglePatternedAlias() { + assertEquals(VALUE, source.getValue(DUALSINGLE_PATTERNED_NEW_NAME)); } } \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigSourceTest.java b/src/test/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigSourceTest.java deleted file mode 100644 index 9ceca24aadf..00000000000 --- a/src/test/java/edu/harvard/iq/dataverse/settings/source/DbSettingConfigSourceTest.java +++ /dev/null @@ -1,48 +0,0 @@ -package edu.harvard.iq.dataverse.settings.source; - -import edu.harvard.iq.dataverse.settings.Setting; -import edu.harvard.iq.dataverse.settings.SettingsServiceBean; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.MethodOrderer.OrderAnnotation; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.TestMethodOrder; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.junit.jupiter.MockitoExtension; - -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; - -import static org.junit.jupiter.api.Assertions.*; - -@ExtendWith(MockitoExtension.class) -@TestMethodOrder(OrderAnnotation.class) -class DbSettingConfigSourceTest { - - DbSettingConfigSource dbSource = new DbSettingConfigSource(); - @Mock - SettingsServiceBean settingsSvc; - - @Test - @Order(1) - void testEmptyIfNoSettingsService() { - assertEquals(null, dbSource.getValue("foobar")); - assertDoesNotThrow(DbSettingConfigSource::updateProperties); - } - - @Test - @Order(2) - void testDataRetrieval() { - Set settings = new HashSet<>(Arrays.asList(new Setting(":FooBar", "hello"), new Setting(":FooBarI18N", "de", "hallo"))); - Mockito.when(settingsSvc.listAll()).thenReturn(settings); - - DbSettingConfigSource.injectSettingsService(settingsSvc); - - assertEquals("hello", dbSource.getValue("dataverse.settings.fromdb.FooBar")); - assertEquals("hallo", dbSource.getValue("dataverse.settings.fromdb.FooBarI18N.de")); - } - -} \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/util/testing/JvmSetting.java b/src/test/java/edu/harvard/iq/dataverse/util/testing/JvmSetting.java new file mode 100644 index 00000000000..f54cadaf253 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/testing/JvmSetting.java @@ -0,0 +1,64 @@ +package edu.harvard.iq.dataverse.util.testing; + +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.Resources; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + + +/** + * {@code @SetJvmSetting} is a JUnit Jupiter extension to set the value of a + * JVM setting (internally a system property) for a test execution. + * + *

The key and value of the JVM setting to be set must be specified via + * {@link #key()} and {@link #value()}. After the annotated method has been + * executed, the initial default value is restored.

+ * + *

{@code SetJvmSetting} can be used on the method and on the class level. + * It is repeatable and inherited from higher-level containers. If a class is + * annotated, the configured property will be set before every test inside that + * class. Any method level configurations will override the class level + * configurations.

+ * + * Parallel execution of tests using this extension is prohibited by using + * resource locking provided by JUnit5 - system properties are a global state, + * these tests NEED to be done serial. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.METHOD, ElementType.TYPE }) +@Inherited +@Repeatable(JvmSetting.JvmSettings.class) +@ExtendWith(JvmSettingExtension.class) +@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ_WRITE) +public @interface JvmSetting { + + /** + * The key of the system property to be set. + */ + edu.harvard.iq.dataverse.settings.JvmSettings key(); + + /** + * The value of the system property to be set. + */ + String value(); + + String[] varArgs() default {}; + + /** + * Containing annotation of repeatable {@code @SetSystemProperty}. + */ + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.METHOD, ElementType.TYPE }) + @Inherited + @ExtendWith(JvmSettingExtension.class) + @interface JvmSettings { + JvmSetting[] value(); + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/util/testing/JvmSettingExtension.java b/src/test/java/edu/harvard/iq/dataverse/util/testing/JvmSettingExtension.java new file mode 100644 index 00000000000..56e87589139 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/testing/JvmSettingExtension.java @@ -0,0 +1,75 @@ +package edu.harvard.iq.dataverse.util.testing; + +import edu.harvard.iq.dataverse.settings.JvmSettings; +import org.junit.jupiter.api.extension.AfterTestExecutionCallback; +import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +public class JvmSettingExtension implements BeforeTestExecutionCallback, AfterTestExecutionCallback { + + private ExtensionContext.Store getStore(ExtensionContext context) { + return context.getStore(ExtensionContext.Namespace.create(getClass(), context.getRequiredTestClass(), context.getRequiredTestMethod())); + } + + @Override + public void beforeTestExecution(ExtensionContext extensionContext) throws Exception { + extensionContext.getTestMethod().ifPresent(method -> { + JvmSetting[] settings = method.getAnnotationsByType(JvmSetting.class); + for (JvmSetting setting : settings) { + // get the setting name (might need var args substitution) + String settingName = getSettingName(setting); + + // get the setting ... + String oldSetting = System.getProperty(settingName); + + // if present - store in context to restore later + if (oldSetting != null) { + getStore(extensionContext).put(settingName, oldSetting); + } + + // set to new value + System.setProperty(settingName, setting.value()); + } + }); + } + + @Override + public void afterTestExecution(ExtensionContext extensionContext) throws Exception { + extensionContext.getTestMethod().ifPresent(method -> { + JvmSetting[] settings = method.getAnnotationsByType(JvmSetting.class); + for (JvmSetting setting : settings) { + // get the setting name (might need var args substitution) + String settingName = getSettingName(setting); + + // get a stored setting from context + String oldSetting = getStore(extensionContext).remove(settingName, String.class); + + // if present before, restore + if (oldSetting != null) { + System.setProperty(settingName, oldSetting); + // if NOT present before, delete + } else { + System.clearProperty(settingName); + } + } + }); + } + + private String getSettingName(JvmSetting setting) { + JvmSettings target = setting.key(); + + if (target.needsVarArgs()) { + String[] variableArguments = setting.varArgs(); + + if (variableArguments == null || variableArguments.length != target.numberOfVarArgs()) { + throw new IllegalArgumentException("You must provide " + target.numberOfVarArgs() + + " variable arguments via varArgs = {...} for setting " + target + + " (\"" + target.getScopedKey() + "\")"); + } + + return target.insert(variableArguments); + } + + return target.getScopedKey(); + } +} diff --git a/src/test/java/edu/harvard/iq/dataverse/util/testing/SystemProperty.java b/src/test/java/edu/harvard/iq/dataverse/util/testing/SystemProperty.java new file mode 100644 index 00000000000..2770831296f --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/testing/SystemProperty.java @@ -0,0 +1,60 @@ +package edu.harvard.iq.dataverse.util.testing; + +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.api.parallel.Resources; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Repeatable; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * {@code @SystemProperty} is a JUnit Jupiter extension to set the value of an + * arbitrary system property for a test execution. + * + *

The key and value of the property to be set must be specified via + * {@link #key()} and {@link #value()}. After the annotated method has been + * executed, the initial default value is restored.

+ * + *

{@code SetJvmSetting} can be used on the method and on the class level. + * It is repeatable and inherited from higher-level containers. If a class is + * annotated, the configured property will be set before every test inside that + * class. Any method level configurations will override the class level + * configurations.

+ * + * Parallel execution of tests using this extension is prohibited by using + * resource locking provided by JUnit5 - system properties are a global state, + * these tests NEED to be done serial. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.METHOD, ElementType.TYPE }) +@Inherited +@Repeatable(SystemProperty.SystemProperties.class) +@ExtendWith(SystemPropertyExtension.class) +@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ_WRITE) +public @interface SystemProperty { + /** + * The key of the system property to be set. + */ + String key(); + + /** + * The value of the system property to be set. + */ + String value(); + + /** + * Containing annotation of repeatable {@code @SystemProperty}. + */ + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.METHOD, ElementType.TYPE }) + @Inherited + @ExtendWith(SystemPropertyExtension.class) + @interface SystemProperties { + SystemProperty[] value(); + } +} \ No newline at end of file diff --git a/src/test/java/edu/harvard/iq/dataverse/util/testing/SystemPropertyExtension.java b/src/test/java/edu/harvard/iq/dataverse/util/testing/SystemPropertyExtension.java new file mode 100644 index 00000000000..146b7c63713 --- /dev/null +++ b/src/test/java/edu/harvard/iq/dataverse/util/testing/SystemPropertyExtension.java @@ -0,0 +1,56 @@ +package edu.harvard.iq.dataverse.util.testing; + +import org.junit.jupiter.api.extension.AfterTestExecutionCallback; +import org.junit.jupiter.api.extension.BeforeTestExecutionCallback; +import org.junit.jupiter.api.extension.ExtensionContext; + +public class SystemPropertyExtension implements BeforeTestExecutionCallback, AfterTestExecutionCallback { + + private ExtensionContext.Store getStore(ExtensionContext context) { + return context.getStore(ExtensionContext.Namespace.create(getClass(), context.getRequiredTestClass(), context.getRequiredTestMethod())); + } + + @Override + public void beforeTestExecution(ExtensionContext extensionContext) throws Exception { + extensionContext.getTestMethod().ifPresent(method -> { + SystemProperty[] settings = method.getAnnotationsByType(SystemProperty.class); + for (SystemProperty setting : settings) { + // get the property name + String settingName = setting.key(); + + // get the setting ... + String oldSetting = System.getProperty(settingName); + + // if present - store in context to restore later + if (oldSetting != null) { + getStore(extensionContext).put(settingName, oldSetting); + } + + // set to new value + System.setProperty(settingName, setting.value()); + } + }); + } + + @Override + public void afterTestExecution(ExtensionContext extensionContext) throws Exception { + extensionContext.getTestMethod().ifPresent(method -> { + SystemProperty[] settings = method.getAnnotationsByType(SystemProperty.class); + for (SystemProperty setting : settings) { + /// get the property name + String settingName = setting.key(); + + // get a stored setting from context + String oldSetting = getStore(extensionContext).remove(settingName, String.class); + + // if present before, restore + if (oldSetting != null) { + System.setProperty(settingName, oldSetting); + // if NOT present before, delete + } else { + System.clearProperty(settingName); + } + } + }); + } +}