Skip to content

Commit

Permalink
feat,doc(setting): fixate that old names cannot change order of place…
Browse files Browse the repository at this point in the history
…holders

For the sake of keeping semantics intact (and make it much easier to search for config bits),
no longer allow to use positional string replacements. As we are using String.format() in
AliasConfigSource, this was possible, but it doesn't make much sense.

Usually, the placeholders are going to be used to express some kind of object or a list of options.
Changing the order of that seems unlikely to be useful, as it would invert the semantic.

Also straightened the docs on old names some more, including current limitations of 'em.
  • Loading branch information
poikilotherm committed Oct 11, 2023
1 parent 0ff7ed3 commit a963180
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 7 deletions.
10 changes: 9 additions & 1 deletion doc/sphinx-guides/source/developers/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,15 @@ 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.
being used with patterned lookups, but must follow some rules:

- Must be given as fully scopes names, not just the old unscoped key/name.
(Otherwise it would not be possible to switch between completely different scopes.)
- Must contain "%s" as replaceable substring for settings using placeholders.
- The order of these placeholders cannot be changed between old and new names to keep semantics intact.
(Usually placeholders are used to describe some sort of object.)

Note: old names are currently limited to keyed settings and cannot be defined for scopes or placeholder settings.

Another option is to add the alias in ``src/main/resources/META-INF/microprofile-aliases.properties``. The format is
always like ``dataverse.<scope/....>.newname...=old.property.name``. Note this doesn't provide support for patterned
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/edu/harvard/iq/dataverse/settings/JvmSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public enum JvmSettings {

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 static final Pattern OLD_NAME_PLACEHOLDER_PATTERN = Pattern.compile("%s");

private final String key;
private final String scopedKey;
Expand Down Expand Up @@ -206,15 +206,20 @@ public enum JvmSettings {
* 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.)
* Old names are picked up by {@link edu.harvard.iq.dataverse.settings.source.AliasConfigSource} to allow backward
* compatible, non-breaking deprecation and switching to new setting names.
*
* @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.
* <ul>
* <li>Must be given as fully scopes names, not just the old unscoped key/name.
* (Otherwise it would not be possible to switch between completely different scopes.)</li>
* <li>Must contain "%s" as replaceable substring for settings using placeholders.</li>
* <li>The order of these placeholders cannot be changed between old and new names to keep
* semantics intact. (Usually placeholders are used to describe some sort of object.)</li>
* </ul>
*
*/
JvmSettings(JvmSettings scope, String key, String... oldNames) {
this.key = key;
Expand Down

0 comments on commit a963180

Please sign in to comment.