8889 file-level PIDs configuration in individual collections #9614

landreev · 2023-05-24T14:53:47Z

What this PR does / why we need it:
See issue. Curation team feature request, allows enabling file PIDs in specific collections. Enabling/disabling is via API only.

Which issue(s) this PR closes:

Closes Feature Request: Enable file PIDs at Dataverse Collection Level #8889

Special notes for your reviewer:

The implementation is straightforward. I added a dedicated boolean flag to the Dataverse entity for this. I don't have any reservations about extending that table, especially since there should be relatively few entries in it.

Note that I copy-and-pasted/stole the new "collection attributes" API from Oliver's #9462, he's adding it in order to be able to configure version PIDs in collections, makes perfect sense to use it for this as well.

I added a simple integration test for the behavior.

One extra thing: there is a Flyway script in the PR, I numbered it 5.13.0.2, under the assumption that it is going to make it into 5.14... but it may need to be renamed as needed.

Suggestions on how to test this:

Straightforward. Have file PIDs disabled instance-wide. Enable them for a specific collection via the API: /api/dataverses/${ALIAS}/attribute/filePIDsEnabled?value=true. Create a dataset with files in the collection, publish, confirm that the files have got PIDs.
(Or the other way around - have it enabled for the instance, then disable it for a specific collection).
Important: needs to be tested on an instance using a real Datacite account. Because "FAKE" does not create file-level DOIs.

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

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

Yes, included.

Additional documentation:

…nter #8889

…a collection (#8889)

… v5.14. #8889

pdurbin

I didn't test this but the code looks good and excited about the main functionality AND the new ability to modify collections via API for alias, name, description, and affiliation. A nice bonus feature.

I suggested some tiny stuff (typo fixes, maybe more tests) so I'll assign this to @landreev to take a look.

DownloadFilesIT.downloadAllFilesRestricted was failing when I got to this PR: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-9614/5/testReport/edu.harvard.iq.dataverse.api/DownloadFilesIT/downloadAllFilesRestricted/

I must have bumped something accidentally triggering a new test run. 😅 I guess we'll see how it ends: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-9614/6/

We should be mindful of the SQL script name, of course. It seems likely we can get this into 5.13 but we'll see.

pdurbin · 2023-05-26T20:09:12Z

doc/sphinx-guides/source/api/native-api.rst

+The following attributes are supported:
+
+* ``alias``  Collection alias
+* ``name`` Name


This should be useful for @Kris-LIBIS.

In the containerization meetings he has expressed the need to change the name of a collection after installation via API.

pdurbin · 2023-05-26T20:12:14Z

src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java

@@ -2091,6 +2091,12 @@ static Response setDataverseLogo(String dataverseAlias, String pathToImageFile,
                .multiPart("file", new File(pathToImageFile))
                .put("/api/dataverses/" + dataverseAlias + "/logo");
    }
+
+    static Response setCollectionAttribute(String dataverseAlias, String attribute, String value, String apiToken) {


I'm excited to have this method and functionality. In future test cases, we could use it in DataversesIT to test changing the name, description, and affiliation of collections.

We COULD do that now, in this PR, if we feel like it. It's new functionality that should probably be exercised.

As I mentioned in the description, I'm copy-and-pasting/stealing this API from Oliver's pr that's still in draft (#9462). This by itself is going to introduce at least a minor merge conflict with his branch. @poikilotherm are you ok with me expanding on it in this branch, like adding an integration test? - or do you want me to leave it to you?

(I added a simple test that uses the API to change the alias then makes sure the change has taken effect)

Hooray for the extra tests! 🎉

Please go ahead! I'm happy to resolve any merge conflicts that might appear.

doc/sphinx-guides/source/api/native-api.rst

doc/sphinx-guides/source/installation/config.rst

Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>

poikilotherm · 2023-05-31T14:48:28Z

src/main/java/edu/harvard/iq/dataverse/Dataverse.java


+    public Boolean getFilePIDsEnabled() {
+        return filePIDsEnabled;


Shouldn't this code return a default if null? If this should be optional (so this is handled as business logic in service layer), maybe return Optional.ofNullable()?

At least the Javadoc should state what can be expected as return values.

I specifically wanted it to be nullable, and to be null by default; and I wanted the method above to return the actual value (or lack thereof) in the database table. Null value in my implementation has a meaning, of "not specifically configured" one way or another, indicating that the setting of the ancestral collection needs to be consulted instead (or the system-wide default, if none defined on the root collection).
So yes, that was a long way of saying "business logic is elsewhere". I will add a javadoc entry.

…n the Dataverse class. #8889

github-actions · 2023-05-31T20:12:28Z

📦 Pushed preview application image as

ghcr.io/gdcc/dataverse:8889-filepids-in-collections

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

landreev · 2023-06-02T21:35:08Z

Is this PR ready now, or are you still waiting for me to add anything else? - I may have missed something.

pdurbin

Looks good!

kcondon · 2023-06-14T19:14:32Z

Issues found:

Getting 405 when calling the api endpoint: API endpoint does not support this method.
-This is a doc issue. Shows as a GET but it is a POST
Need to confirm the current flyway script name is appropriate for this 5.14 release, per note in pr
-Yes, this is fine as long as we're releasing in 5.14
Question: what is the expectation if system setting and collection setting are the same, true/true, false/false, seems obvious but just asking.
-Collection level setting always wins is the general rule.
Question: how does an admin register file pids for datasets created before file pids was enabled? orig issues mentioned an api endpoint.

landreev · 2023-06-15T17:17:00Z

As to 4. above, it appears that I missed that part. Rather, I just assumed that no new API was needed, that we'd be able to register the file PIDs as needed using the existing API once the registration was enabled... But the only APIs for file level PIDs we have is "register all (unregistered)" and "register an individual file (by id)". While registering all the files in a specific collection can be achieved with the above, it's not practical and we do need an API for registering all the unregistered files in a specific collection.

…8889

landreev · 2023-06-22T14:28:26Z

There's now an API for minting PIDs for files within a specific collection. GET /api/admin/registerDataFiles/{collection_alias}, described in the "Managing Datasets and Dataverse Collections" guide, with the other file-level registration methods.

github-actions · 2023-06-27T17:36:48Z

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:8889-filepids-in-collections

ghcr.io/gdcc/configbaker:8889-filepids-in-collections

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

landreev added 9 commits May 22, 2023 17:14

File pids in individual collections (#8889)

9e77561

change to the configuration logic. #8889

bc84ed5

Guide entry change for the instance-wide setting. #8889

4d9e9b1

more guide entries #8889

e757a77

release note for #8889

0ec9c73

Added the handling for the file pids attribute to the json parser/pri…

ff34b46

…nter #8889

a simple restassured test for disabling file pid registration within …

82e91f2

…a collection (#8889)

a couple of extra fixes. #8889

60c1031

cosmetic. #8889

14ce446

A bug in the restassured test. #8889

78d68e2

This comment has been minimized.

Sign in to view

landreev added 2 commits May 24, 2023 13:32

Merge branch 'develop' into 8889-filepids-in-collections

2fd36ad

A better test for the persistent id. #8889

ae8b438

This comment has been minimized.

Sign in to view

reverting the test check. #8889

6d06e32

This comment has been minimized.

Sign in to view

renaming the flyway script 5.13.0.2, hopefully this will make it into…

a556b69

… v5.14. #8889

This comment has been minimized.

Sign in to view

pdurbin self-assigned this May 26, 2023

pdurbin reviewed May 26, 2023

View reviewed changes

pdurbin assigned landreev May 26, 2023

landreev and others added 2 commits May 31, 2023 10:40

Update doc/sphinx-guides/source/installation/config.rst

d2325fa

Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>

Update doc/sphinx-guides/source/api/native-api.rst

0704ae2

Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>

poikilotherm reviewed May 31, 2023

View reviewed changes

landreev mentioned this pull request May 31, 2023

4499 - dataset version pids #9462

Draft

27 tasks

This comment has been minimized.

Sign in to view

A simple test for the new collection attributes api; added comments i…

e337871

…n the Dataverse class. #8889

pdurbin approved these changes Jun 6, 2023

View reviewed changes

pdurbin unassigned pdurbin and landreev Jun 6, 2023

kcondon self-assigned this Jun 14, 2023

landreev self-assigned this Jun 20, 2023

landreev added 2 commits June 20, 2023 14:00

Merge branch 'develop' into 8889-filepids-in-collections

d902fed

Added an API for registering within a collection. More doc changes. #…

4f1afd3

…8889

This comment has been minimized.

Sign in to view

typo. #8889

0e89865

This comment has been minimized.

Sign in to view

landreev removed their assignment Jun 22, 2023

typo. #8889

59cd7ab

kcondon merged commit 8b4100d into develop Jun 27, 2023

pdurbin added this to the 5.14 milestone Jul 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8889 file-level PIDs configuration in individual collections #9614

8889 file-level PIDs configuration in individual collections #9614

landreev commented May 24, 2023 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pdurbin left a comment

pdurbin May 26, 2023

pdurbin May 26, 2023

landreev May 31, 2023

landreev May 31, 2023

pdurbin May 31, 2023

poikilotherm May 31, 2023 •

edited

Loading

poikilotherm May 31, 2023 •

edited

Loading

landreev May 31, 2023

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 31, 2023

landreev commented Jun 2, 2023

pdurbin left a comment

kcondon commented Jun 14, 2023 •

edited

Loading

landreev commented Jun 15, 2023

This comment has been minimized.

This comment has been minimized.

landreev commented Jun 22, 2023

github-actions bot commented Jun 27, 2023


		public Boolean getFilePIDsEnabled() {
		return filePIDsEnabled;

8889 file-level PIDs configuration in individual collections #9614

8889 file-level PIDs configuration in individual collections #9614

Conversation

landreev commented May 24, 2023 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pdurbin left a comment

Choose a reason for hiding this comment

pdurbin May 26, 2023

Choose a reason for hiding this comment

pdurbin May 26, 2023

Choose a reason for hiding this comment

landreev May 31, 2023

Choose a reason for hiding this comment

landreev May 31, 2023

Choose a reason for hiding this comment

pdurbin May 31, 2023

Choose a reason for hiding this comment

poikilotherm May 31, 2023 • edited Loading

Choose a reason for hiding this comment

poikilotherm May 31, 2023 • edited Loading

Choose a reason for hiding this comment

landreev May 31, 2023

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

github-actions bot commented May 31, 2023

landreev commented Jun 2, 2023

pdurbin left a comment

Choose a reason for hiding this comment

kcondon commented Jun 14, 2023 • edited Loading

landreev commented Jun 15, 2023

This comment has been minimized.

This comment has been minimized.

landreev commented Jun 22, 2023

github-actions bot commented Jun 27, 2023

landreev commented May 24, 2023 •

edited

Loading

poikilotherm May 31, 2023 •

edited

Loading

poikilotherm May 31, 2023 •

edited

Loading

kcondon commented Jun 14, 2023 •

edited

Loading