Add getZipDownloadLimit and getMaxEmbargoDurationInMonths API info endpoints

[GPortas]

What this PR does / why we need it:

The PR includes the following new endpoints:

• getZipDownloadLimit (/api/info/zipDownloadLimit): Get the configured zip file download limit. The response contains the long value of the limit in bytes.

• getMaxEmbargoDurationInMonths (/api/info/settings/:MaxEmbargoDurationInMonths): Get the max embargo duration in months, if available.

Which issue(s) this PR closes:

• Closes [Spike - API] Extend the info API to return zip download limit and embargo enabled/disabled #9880

Special notes for your reviewer:

These new endpoints are required by IQSS/dataverse-client-javascript#84

Suggestions on how to test this:

Via curl:

curl "$SERVER_URL/api/info/settings/incompleteMetadataViaApi
curl "$SERVER_URL/api/info/settings/:MaxEmbargoDurationInMonths

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

No

Is there a release notes update needed for this change?:

Yes

Additional documentation:

None

[pdurbin]

Overall, this looks great. I'm glad to see tests (good to delete the seeing before and after) and docs.

I did ask a couple questions.

[pdurbin]

Something feels off to me...

We should probably make an active decision about two things:

• Should we continue putting "settings" in the path?
• Should we match the name of the setting including the colon?

Another way to ask this is which of the following we want.

/api/info/zipDownloadLimit
/api/info/settings/zipDownloadLimit
/api/info/settings/:ZipDownloadLimit

For reference here is a sorted list of "info" endpoints as of this PR:

/api/info/apiTermsOfUse
/api/info/embargoEnabled
/api/info/server
/api/info/settings/:DatasetPublishPopupCustomText
/api/info/settings/incompleteMetadataViaApi
/api/info/version
/api/info/zipDownloadLimit

[GPortas]

It seems appropriate to me to add the setting subpath, and even include the colon if it is more understandable for the setting name, as long as the endpoint only reads the setting value, with no other operations around it.

For example, the /admin/settings/{name} endpoint returns the plain value of the setting if it exists, otherwise it returns a not found error.

On the other hand, the info/zipDownloadLimitendpoint does more than read the setting, since if the associated setting is not found it assigns a default value. So I find appropriate to exclude the "settings" endpoint subpath/naming here.

Same for the info/embargoEnabled endpoint, that after reading the :MaxEmbargoDurationInMonths setting, based on its possible values, checks whether the embargo functionality is enabled or not.

[pdurbin]

@GPortas I wrote some docs to merge into this PR once we are happy with them:

• stub out page on API design, especially paths #9890

[pdurbin]

In the future, will the SPA (or some other system) need to know the value of :MaxEmbargoDurationInMonths? If so, should we return it now, as part of the endpoint we're adding? Or would we add a second endpoint to get the number of months?

[GPortas]

To be honest, I'm not entirely sure, but I don't think so, considering that the embargo duration is something that is managed internally by the setting, and that for now, I've only seen that the UI needs to know if the feature is enable (To show / not show related UI) or if a file is embargoed, but not the configured embargo duration.

In any case, if necessary in the future, we can extend the endpoint to include the setting value or create a separate endpoint. For now I have preferred to keep it simple.

[pdurbin]

I'm assuming the SPA will need to perform some validation when we get to the EDIT side of the dataset page. I can imagine an error like this:

"Please an embargo date before September 8, 2025" (for example)

[GPortas]

You are right. When creating an embargo, there is a date picker that I was overlooking where dates within the range determined by the :MaxEmbargoDurationInMonths setting are displayed. So the SPA has to know the value of the setting beforehand.

I think then it is better to directly expose the setting. Something like: /api/info/settings/:MaxEmbargoDurationInMonths

Then the SPA/consumer will know if the feature is enabled (by having a value set different than 0) and the embargo duration in months.

What do you think? @pdurbin

[pdurbin]

Yes, directly exposing the setting sounds good.

[GPortas]

Updated! @pdurbin

[pdurbin]

@GPortas great, I just updated this PR to reflect the embargo change:

• stub out page on API design, especially paths #9890

Can you please review and merge if you're happy with it?

[GPortas]

Thanks @pdurbin, I just reviewed it.

Looks great, just one minor request.

[pdurbin]

I just wanted to mention @landreev 's idea at standup to someday expose more database settings automatically fix via API instead of exposing them here and there. Roughly:

• add a column to the setting table to indicate if it's safe to expose the database setting publicly
• expose the database setting publicly programmatically via a predictable path, but only if it's safe to do so

I sort of assume one could get a dump of public settings at once as well. Not sure. I'm not sure about JVM options and MicroProfile settings either. Needs more discussion and an issue.

[poikilotherm]

A few thoughts:

• DB settings are hacky right now. Should be refactored.
• Information about safe exposure can be held in the enum instead of the table
• Exposing MPCONFIG JvmSettings is easy to do, also with making sure secrets are kept secret

[pdurbin]

I kicked of a run of the API tests here: https://github.com/gdcc/api-test-runner/actions/runs/6148788525

Soon we'll have Jenkins/Ansible back and testing the post 6.0 world:

• bump to 6.0 release gdcc/dataverse-ansible#316

[GPortas]

@pdurbin

I like the idea.

I can foresee that more settings exposed via API will be needed for the development of the SPA.

Considering this argument: "Information about safe exposure can be held in the enum instead of the table", we can add public settings to the enum in an emerging way as we need their exposure. @poikilotherm

For the next setting to be exposed we can evaluate this design in more depth and implement it at that time.

[pdurbin]

https://github.com/gdcc/api-test-runner/actions/runs/6148788525 passed so I'm going to go ahead and approve this.

[pdurbin]

Looks good.

[kcondon]

Please refresh from develop to update version to 6.0, thanks!

[GPortas]

Done, thanks @kcondon !

[github-actions]

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:9880-info-zip-download-limit-embargo

ghcr.io/gdcc/configbaker:9880-info-zip-download-limit-embargo

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

[kcondon]

@GPortas, when I'm doing a smoke test (create collection, dataset, publish) I'm getting a 500 error when publishing:

publish_err.txt

Update: I've retested this a few times and am no longer seeing this. Not sure what caused it.

[GPortas]

In a quick glance, the error seems related to faces UI, I'm not sure if we should give it much importance. In any case it should have no relation to the changes implemented in this PR.

Thanks, @kcondon.