Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7000 mpconfig basic infra #8823

Merged
merged 35 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
de1f520
fix(settings): remove DbConfigSource #7680
poikilotherm Jun 20, 2022
aa65848
deps(settings): switch to Smallrye MP config #7000
poikilotherm Jun 20, 2022
33886c9
feat(settings): start new enum JvmSettings #7000
poikilotherm Jun 20, 2022
2dc970e
feat(settings): filter MPCONFIG for Maven properties #7000
poikilotherm Jun 20, 2022
4ff94f2
feat(tests): add JUnit5 JVM setting helper
poikilotherm Jun 21, 2022
0cfed10
docs(dev): add notes about @JvmSetting for unit tests
poikilotherm Jun 21, 2022
1b64393
feat(settings): add fluent JvmSettings.lookup() #7000
poikilotherm Jun 22, 2022
f8574d7
refactor(settings): store scoped key on construction for JvmSettings
poikilotherm Jun 23, 2022
58bb9aa
feat(settings): add old names / alias support in JvmSettings #7000
poikilotherm Jun 23, 2022
de5918c
feat(settings): add MPCONFIG converter usage in JvmSettings #7000
poikilotherm Jun 30, 2022
b2d3792
feat(settings): add Dataverse version to MPCONFIG #7000
poikilotherm Jun 20, 2022
3a4ea01
test(settings): add test for AliasConfigSource and JvmSettings #7000
poikilotherm Jun 30, 2022
645af18
docs(settings): add basic release note for #7000
poikilotherm Jul 4, 2022
0d792f3
refactor(settings): make AliasConfigSource() log exception
poikilotherm Jul 12, 2022
48afdf1
refactor(settings): make AliasConfigSource() always read JvmSettings …
poikilotherm Jul 12, 2022
daefd6d
refactor(settings): make AliasConfigSource.readAliases more robust
poikilotherm Jul 12, 2022
c392253
feat(settings): make AliasConfigSource use lists of old names and pat…
poikilotherm Jul 12, 2022
5e65996
feat(settings): let AliasConfigSource.getValue use patterned aliases
poikilotherm Jul 12, 2022
27387dc
feat(tests): add @SystemProperty JUnit5 extension
poikilotherm Jul 13, 2022
9bbed22
test(settings): add tests for AliasConfigSource.getValue
poikilotherm Jul 13, 2022
811de96
feat(settings): add patterned JvmSettings support #7000
poikilotherm Jul 13, 2022
51abbbf
feat(tests): add patterned JvmSettings support for @JvmSetting extens…
poikilotherm Jul 13, 2022
7b7edb7
test(settings): add simple setting test for JvmSettings #7000
poikilotherm Jul 13, 2022
c42230e
docs(dev): update for JvmSettings and deleted DB config source #7000
poikilotherm Jul 13, 2022
86526d5
Merge branch 'develop' into 7000-mpconfig-infra
poikilotherm Jul 19, 2022
78c3ef8
feat(settings): add simple String vararg lookups in JvmSettings #7000
poikilotherm Jul 22, 2022
90e1799
fix(settings): correct JvmSettings pattern for old names placeholders…
poikilotherm Jul 22, 2022
06f874d
Merge branch 'develop' into 7000-mpconfig-infra
poikilotherm Aug 5, 2022
22940ff
Merge branch 'develop' into 7000-mpconfig-infra #7000
pdurbin Sep 1, 2022
233bbda
Merge branch 'develop' into 7000-mpconfig-infra #7000
pdurbin Sep 6, 2022
a64c138
tweak docs and release note #7000
pdurbin Sep 6, 2022
dd27401
"IllegalStateException: Recursive update" went away with Payara upgra…
pdurbin Sep 7, 2022
1737923
Update doc/release-notes/7000-mpconfig-support.md
pdurbin Sep 8, 2022
154fc59
Merge branch 'develop' into 7000-mpconfig-infra
poikilotherm Sep 14, 2022
17fc0dc
Merge branch 'develop' into 7000-mpconfig-infra #7000
pdurbin Sep 14, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/release-notes/7000-mpconfig-support.md
Original file line number Diff line number Diff line change
@@ -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.2022.3 or higher may be required. Payara 5.2021.5 threw exceptions, as explained in PR #8823.
pdurbin marked this conversation as resolved.
Show resolved Hide resolved
65 changes: 38 additions & 27 deletions doc/sphinx-guides/source/developers/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.<SETTING NAME>.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 <avoid-efficiency-issues-with-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
<avoid-efficiency-issues-with-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
Expand All @@ -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:

Expand All @@ -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
---------------------------------
Expand All @@ -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.<scope/task/module/...>.<setting>``.
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.<scope/task/module/...>.<setting>``. 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.<SETTING NAME>.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.<scope/....>.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.<scope/....>.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``
16 changes: 16 additions & 0 deletions doc/sphinx-guides/source/developers/testing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
2 changes: 1 addition & 1 deletion modules/dataverse-parent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@

<!-- Testing dependencies -->
<testcontainers.version>1.15.0</testcontainers.version>
<microbean-mpconfig.version>0.4.1</microbean-mpconfig.version>
<smallrye-mpconfig.version>2.10.1</smallrye-mpconfig.version>

<junit.version>4.13.1</junit.version>
<junit.jupiter.version>5.7.0</junit.jupiter.version>
Expand Down
15 changes: 11 additions & 4 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -613,9 +613,9 @@
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.microbean</groupId>
<artifactId>microbean-microprofile-config</artifactId>
<version>${microbean-mpconfig.version}</version>
<groupId>io.smallrye.config</groupId>
<artifactId>smallrye-config</artifactId>
<version>${smallrye-mpconfig.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
Expand Down Expand Up @@ -653,10 +653,17 @@
<include>**/*.xml</include>
<include>**/firstNames/*.*</include>
<include>**/*.xsl</include>
<include>**/*.properties</include>
<include>**/services/*</include>
</includes>
</resource>
<resource>
<directory>src/main/resources</directory>
<!-- Filter files matching here for Maven properties and replace -->
<filtering>true</filtering>
<includes>
<include>**/*.properties</include>
</includes>
</resource>
</resources>
<plugins>
<plugin>
Expand Down
Loading