Database Settings: idempotent mass operations, necessary cleanup and script using it for real use case

[poikilotherm]

What this PR does / why we need it:

Enable setting all the database settings in one go, using an atomic database operation.

Which issue(s) this PR closes:

• Closes Feature Request: refactor and cleanup Settings Admin API to allow idempotent updates #11639

Special notes for your reviewer:
Aside from the bulk operation itself, it required a lot more work to actually implement this properly. Unfortunately, this makes this PR a lot larger than anticipated, but hopefully it's worth it. (I suppose DB opts are not going to go away anytime soon...)

1. I had to migrate any non-aligned database setting names. Reshaped them into a form that makes it easier to do input validation in the code. But also makes it easier for sysadmins to use, as there are no longer three different naming schemes for them. (This affects the infamous BuiltinUsers.KEY and limits for tabular file ingests.)
2. All Admin API endpoints for settings didn't do any input validation at all. You could basically do whatever you want, even accidentally. This has been mitigated.
3. The way the Settingclass was setup it impeded performance and did not enforce any constraints to avoid duplicated settings. (There was a Flyway script to take care of that, but it wasn't at all in the code, not even mentioned as comment.)
4. The endpoint to retrieve all settings as JSON was faulty: it did not take the localization into account.
5. Most of the code around DB settings was untested. For bits I added, I added tests to make sure things work as intended.
6. Archive settings had to be moved to the enum. See 83589ba and Iqss/7140 google cloud archiver #7292 (comment) .

• Decide whether or not to document in the Native API docs which (two) settings are supporting localization (within this PR).

Suggestions on how to test this:

• Run the included tests
• Roundtrip! Get all settings as JSON and put them back in.
• Apply DB migrations on a DB dump from a real instance using Flyway Maven Plugin to verify before deploying
• Use the script in configbaker to try a real use case scenario
• Test the archive settings mentioned in the release note (some will be hard to test, unfortunately, because we may not have access to systems like DuraCloud, etc.) See 83589ba and Iqss/7140 google cloud archiver #7292 (comment) .

Please note: the Jenkins build / test run will always fail until gdcc/dataverse-ansible#399 is merged! You'll need to execute the tests locally, e.g. using a containerized approach, which also helps with trying the apply-db-settings.sh script. @donsizemore hooked us up with a dedicated Jenkins job for this PR. Check the status of this job rather than the normal one: https://jenkins.dataverse.org/job/IQSS-Dataverse-11639/

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope

Is there a release notes update needed for this change?:
To Be Done Included

Additional documentation:

Preview docs here:

• https://dataverse-guide--11654.org.readthedocs.build/en/11654/api/changelog.html

[github-actions]

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

shellcheck

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 56 in d0bb952

curl -s -X POST -d'[":guest","@anAuthUser"]' -H "Content-type:application/json" $ENDPOINT/dataverses/permissionsTestDv/groups/PTG/roleAssignees?key=$ROOT_KEY

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 62 in d0bb952

curl -s -X POST -d"$ASSIGNMENT" -H "Content-type:application/json" $ENDPOINT/dataverses/permissionsTestDv/assignments/?key=$ROOT_KEY

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 80 in d0bb952

echo @anAuthUser $AN_AUTH_USER_KEY

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 81 in d0bb952

echo @anotherAuthUser $ANOTHER_AUTH_USER_KEY

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 90 in d0bb952

curl -si $ENDPOINT/dataverses/permissionsTestDv?key=$AN_AUTH_USER_KEY | head -n 1

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 94 in d0bb952

curl -si $ENDPOINT/dataverses/permissionsTestDv?key=$ANOTHER_AUTH_USER_KEY | head -n 1

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 103 in d0bb952

curl -si -X POST -d@assignment.json -H "Content-type:application/json" $ENDPOINT/dataverses/permissionsTestDv/assignments/?key=$ANOTHER_AUTH_USER_KEY | head -n 1

---

📝 [shellcheck] _{reported by reviewdog 🐶}
Double quote to prevent globbing and word splitting. SC2086

dataverse/scripts/issues/2454/run-test.sh

Line 107 in d0bb952

curl -si -X POST -d@assignment.json -H "Content-type:application/json" $ENDPOINT/dataverses/permissionsTestDv/assignments/?key=$AN_AUTH_USER_KEY | head -n 1

[sekmiller]

@poikilotherm the AdminIT$SettingsAPI#testSettingsRoundTrip fails if you have a bad setting name in your table. In my case I had put in a manual update for ":publicInstall" instead of ":PublicInstall". When I deleted the offending setting name from my local table the test passed.

[sekmiller]

@poikilotherm deployment in Jenkins is failing. Here is one of the errors I'm seeing: TASK [dataverse : set user management quesadilla] ******************************
fatal: [localhost]: FAILED! => {"changed": false, "connection": "close", "content_length": "66", "content_type": "application/json;charset=UTF-8", "elapsed": 0, "json": {"message": "The name of the setting is invalid.", "status": "ERROR"}, "msg": "Status code was 400 and not [200]: HTTP Error 400: Bad Request", "redirected": false, "server": "Payara Server 6.2025.3 #badassfish", "status": 400, "url": "http://localhost:8080/api/admin/settings/BuiltinUsers.KEY", "x_frame_options": "SAMEORIGIN", "x_powered_by": "Servlet/6.0 JSP/3.1 (Payara Server 6.2025.3 #badassfish Java/Red Hat, Inc./17)"}

[poikilotherm]

@sekmiller thanks for looking into this!

In my case I had put in a manual update for ":publicInstall" instead of ":PublicInstall". When I deleted the offending setting name from my local table the test passed.

Is there anything you'd like me to do? Some migration, a startup check or similar to make things fail early?
No offense, but still: one of the points of this PR is to avoid such rogue setting names in the database going forward... 😉

deployment in Jenkins is failing.

Are we talking about this pipeline or that one? The later is expected to fail, as an update to dataverse-ansible is necessary. Don setup a distinct job that already has the patch applied for this PR, see the first link.

[sekmiller]

@poikilotherm Yes, that's my concern that a real installation running an upgrade will fail because of a rogue setting name. Is there anything we can do about that? (and that the failure of the test didn't give me anything in the response to help me fix it - I had to find it for myself)

[sekmiller]

It's the continuous integrations PR merge that is failing the Jenkins build - https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-11654/41/display/redirect

[pdurbin]

@sekmiller this is the one to look at for this PR: https://jenkins.dataverse.org/job/IQSS-Dataverse-11639/

@donsizemore set it up special for @poikilotherm

All tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-11639/30/testReport/

You can safely ignore https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-11654/41/display/redirect and https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-11654/ more generally, which will always fail.

We should definitely keep an eye on https://jenkins.dataverse.org/job/IQSS-dataverse-develop/ after merging but it should "just work" (famous last words).

[poikilotherm]

@sekmiller I added a Java based Flyway Callback in 886a9cf

This will take all the settings defined in the SettingsServiceBean.Key enum and check the settings in the database against it. Any invalid settings will be automatically purged from the database on every deployment.

This means for the future that we can drop settings from the enum and they will be cleaned up. I went this way because you can't delete an invalid setting using the API...

Let me know if you think this'll work. Good catch finding this edge case - didn't think about it upfront!

[qqmyers]

FWIW: There are settings like

dataverse/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GoogleCloudSubmitToArchiveCommand.java

Lines 38 to 39 in b78060a

	private static final String GOOGLECLOUD_BUCKET = ":GoogleCloudBucket";
	private static final String GOOGLECLOUD_PROJECT = ":GoogleCloudProject";

not in the SettingsServiceBean - the original idea being that things like archivers would eventually be packaged separately and manage their own settings.

[poikilotherm]

FWIW: There are settings like

dataverse/src/main/java/edu/harvard/iq/dataverse/engine/command/impl/GoogleCloudSubmitToArchiveCommand.java

Lines 38 to 39 in b78060a

	private static final String GOOGLECLOUD_BUCKET = ":GoogleCloudBucket";
	private static final String GOOGLECLOUD_PROJECT = ":GoogleCloudProject";

not in the SettingsServiceBean - the original idea being that things like archivers would eventually be packaged separately and manage their own settings.

For now, we added these settings to the enum. People should be safe.

If we extend the mechanism to discoverable settings, we can extend the Java based Flyway Callback, too. (Make it more selective in cleaning up or make them all available during cleanup etc)

[github-actions]

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:11639-db-opts-idempotency

ghcr.io/gdcc/configbaker:11639-db-opts-idempotency

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

[poikilotherm]

Thanks y'all @sekmiller @donsizemore @pdurbin @ofahimIQSS for bearing with me and your thorough work on this one! Glad it got merged! 😎🤗🤓🍻

[landreev]

Unfortunately, the flyway script V6.8.0.1.sql from this PR failed on my personal instance.
Specifically:

at org.flywaydb.core.internal.command.DbMigrate.doMigrateGroup(DbMigrate.java:391)
        ... 112 more
Caused by: org.postgresql.util.PSQLException: ERROR: there is no unique or exclusion constraint matching the ON CONFLICT
 specification
  Where: SQL statement "INSERT INTO setting (name, content, lang)
        VALUES (':TabularIngestSizeLimit', json_object::TEXT, NULL)
        ON CONFLICT (name) WHERE lang IS NULL
            DO UPDATE SET content = EXCLUDED.content"
PL/pgSQL function inline_code_block line 65 at SQL statement

I had both :TabularIngestSizeLimit and :TabularIngestSizeLimit:xlsx in my settings. It worked once I deleted the latter.

[landreev]

@poikilotherm @sekmiller
This really needs to be fixed. I'm sorry to be pushy, but we are very close to the 6.9 code freeze at this point.

Unfortunately, the flyway script V6.8.0.1.sql from this PR failed on my personal instance. Specifically:

at org.flywaydb.core.internal.command.DbMigrate.doMigrateGroup(DbMigrate.java:391)
        ... 112 more
Caused by: org.postgresql.util.PSQLException: ERROR: there is no unique or exclusion constraint matching the ON CONFLICT
 specification
  Where: SQL statement "INSERT INTO setting (name, content, lang)
        VALUES (':TabularIngestSizeLimit', json_object::TEXT, NULL)
        ON CONFLICT (name) WHERE lang IS NULL
            DO UPDATE SET content = EXCLUDED.content"
PL/pgSQL function inline_code_block line 65 at SQL statement

I had both :TabularIngestSizeLimit and :TabularIngestSizeLimit:xlsx in my settings. It worked once I deleted the latter.

[poikilotherm]

@landreev I have a reproducer going over here: #12002
Will continue to fix this. Sorry we didn't catch this earlier. Absolutely one of the reasons why I want to introduce migration testing with JUnit, DBunit and Testcontainers (see PR). Happy to talk about this during tech hours, too.

[landreev]

@poikilotherm
Great, glad to hear this is on your radar.
The script is definitely failing on the copy of our prod. db as well. Our own prod. instance is not my concern, since I can simply convert these settings by hand if I need to. But it would be a problem for every other installation that has format-specific ingest limits.

@landreev I have a reproducer going over here: #12002 Will continue to fix this. Sorry we didn't catch this earlier. Absolutely one of the reasons why I want to introduce migration testing with JUnit, DBunit and Testcontainers (see PR). Happy to talk about this during tech hours, too.