GDCC/9005 replace files api call #9018

qqmyers · 2022-10-03T15:24:49Z

What this PR does / why we need it: This PR adds a /replaceFiles api call to allow bulk direct upload/out-of-band upload replace operations. It also include

some minor cleanup,
a bug fix for /addFiles where an error with one file would stop processing of further files and falsely report the same error for all the skipped files
minor improvement to report the bulk edit operations in the edit-drafts logs (as the single file versions do).

Which issue(s) this PR closes:

Closes #9005
Closes #5924

Special notes for your reviewer: No new tests - while this could be tested within an overall direct upload test (only on S3 with upload-redirect=true), it is tricky to test as a single call since one has to be able to place files in storage directly prior to running the /replaceFiles call. If that can be done in an IT test, then this and /addFiles could be tested this way.

Suggestions on how to test this: @janvanmansum /DANS should be able to test this as part of their migration activity so their review would be useful. Otherwise, manual testing can be done - easiest on an S3 store with direct-upload enabled. (One can temporarily add upload-redirect=true on a file store but this breaks upload in the UI). The examples in the docs should give a good idea of how to test. W.r.t. to regression, this does make minor changes to the overall add/replace mechanism so retesting the single file add/replace apis would be useful.

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?: added a small one

Additional documentation: included.

refactor to make multifile a separate boolean remove unused LicenseBean from constructor updated /addFiles logic to use clone refactored steps 70/80 to work for multi-replace. i.e. by tracking filesToDelete and the physical files to delete. replace local Json readers with JsonUtil method move sanity check on file deletes to DataFileServiceBean

hasError is not cleared where it was being used causing one error to skip all further add/replace calls and report that error for all subsequent files

the title Add File Metadata has been misunderstood to mean the call can change the metadata for existing files which it can't. The entry was also in the File section when it is a dataset-level call

…es_api_call

coveralls · 2022-10-05T17:00:55Z

Coverage decreased (-0.03%) to 19.948% when pulling 54871d1 on GlobalDataverseCommunityConsortium:GDCC/9005-replaceFiles_api_call into 11abccf on IQSS:develop.

…es_api_call

typo

mreekie · 2022-12-13T16:48:11Z

The crux is that we can now replace multiple files at once.
It is functionality that we have, just in a different API call.

Discussion:
DANS wanted this and may be that they have tested this already.
If we rely on their work then we would only need to do regression tests.
It would make us feel more confident if we can still do some QA
There are 9 files, but some are documentation which makes review easier
Jim will resolve the merge conflicts as part of the review.
Bulk add is already existing. DVUploader supports this using the DirectUpload API.
This is not a bulk add. It is a direct upload.
For bulk add, you can do it with a direct upload.
This does a bulk replace. It only works with a direct upload.
Will need S3 setup to test this. The test can be built but it will need to be built out to do 3 things.

It looks like this may close #5924. It may not as they might not be using direct upload. Does Amber using S3?

This gets a 33.

Next step:

@qqmyers will check in Is there an option to batch file replace? #5924 to see if this addresses their issue.

GDCC/9005-replaceFiles_api_call

…GlobalDataverseCommunityConsortium/dataverse.git into GDCC/9005-replaceFiles_api_call

mreekie · 2022-12-14T21:18:55Z

added to sprint Dec 15, 2022

sekmiller · 2022-12-20T15:27:51Z

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

@@ -2366,48 +2373,6 @@ The fully expanded example above (without environment variables) looks like this
 Note: The ``id`` returned in the json response is the id of the file metadata version.


-
-Adding File Metadata


Not sure why you're removing this doc since the endpoint still exists and it seems like we would want to have the ability to add files' metadata, not just replace

This endpoint doesn't work as ~implied by this section and this placement in the guides. You can't change the metadata of existing files with it. It is a way to add new files that have been uploaded by direct S3, out-of-band means, or when using the remoteOverlayStore. This call is documented already at https://guides.dataverse.org/en/latest/developers/s3-direct-upload-api.html?highlight=addfiles#to-add-multiple-uploaded-files-to-the-dataset.

OK. thanks for the explanation

src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java

…es_api_call

sekmiller

a couple of old methods in the add replace helper need to be updated. also a question about addFiles.

sekmiller

Looks good, thanks!

kcondon · 2023-01-03T17:50:55Z

Issues found:
[x] 1. extra buttons on upload page appear during replace file workflow in UI if file types differ and you select delete.
[x] 2. example json for multiple file upload needs different filetoreplace ids.
[x] 3. JSON_DATA for multiple file replace needs to switch all " with ' and vice versa
4. Replacing 2 files removes first file but not replaced, correctly replaces the second file, no server.log error. Command line result says 2 files replaced: Result":{"Total number of files":2,"Number of files successfully replaced":2}

In the UI doing a file replace and, in the case where the mimetype does not match the original, selecting delete could result in a broken upload tab. This only occured with direct upload enabled and was due to the delete call not reinitializing the upload tab for direct upload.

The replaceFiles call keeps one dataset/draft version open for all file replacements and then calls the update version command. At some point prior to this commit changes started causing the dataset being used to be replaced by a fresh copy from the database for each file, losing changes for all but the last file.

qqmyers · 2023-01-05T20:25:17Z

@kcondon - hopefully ready for QA again. 1) was not related to the PR, just to replacing when using direct upload, but I included a fix. Doc changes for 2),3) have been made, and I fixed 4) - looks like a change along the way broke things and made only the last file in the list be changed (with earlier ones being deleted with no replacement).

qqmyers added 7 commits September 30, 2022 09:21

todo is done

3ea4e92

add getjsonarray

cb5007a

docs

e6bd5b3

release note

088cf8a

fix replaceFiles and remove hasError checks that block further changes

4ffccdb

hasError is not cleared where it was being used causing one error to skip all further add/replace calls and report that error for all subsequent files

relocate/rename entry for the /addFiles, /replaceFiles in native-api

9d2fc05

the title Add File Metadata has been misunderstood to mean the call can change the metadata for existing files which it can't. The entry was also in the File section when it is a dataset-level call

qqmyers added the GDCC: DANS related to GDCC work for DANS label Oct 3, 2022

qqmyers added this to On Deck 🛎 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) via automation Oct 3, 2022

Merge remote-tracking branch 'IQSS/develop' into GDCC/9005-replaceFil…

fc5bcf8

…es_api_call

pdurbin mentioned this pull request Oct 14, 2022

Is there an option to batch file replace? #5924

Closed

mreekie added the bk2211 label Nov 1, 2022

qqmyers and others added 2 commits November 7, 2022 09:17

Merge remote-tracking branch 'IQSS/develop' into GDCC/9005-replaceFil…

54871d1

…es_api_call

Update 9005-replaceFiles-api-call

76abd34

typo

qqmyers added 2 commits December 13, 2022 12:48

Merge remote-tracking branch 'IQSS/develop' into

233926e

GDCC/9005-replaceFiles_api_call

Merge branch 'GDCC/9005-replaceFiles_api_call' of https://github.com/…

135eb22

…GlobalDataverseCommunityConsortium/dataverse.git into GDCC/9005-replaceFiles_api_call

mreekie added the Size: 30 A percentage of a sprint. 21 hours. (formerly size:33) label Dec 13, 2022

mreekie added this to Ready for Review in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) via automation Dec 14, 2022

sekmiller self-assigned this Dec 19, 2022

sekmiller moved this from Ready for Review to Review 🔎 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Dec 19, 2022

sekmiller reviewed Dec 20, 2022

View reviewed changes

src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java Outdated Show resolved Hide resolved

sekmiller reviewed Dec 20, 2022

View reviewed changes

src/main/java/edu/harvard/iq/dataverse/datasetutility/AddReplaceFileHelper.java Outdated Show resolved Hide resolved

qqmyers added 2 commits December 20, 2022 10:40

Merge remote-tracking branch 'IQSS/develop' into GDCC/9005-replaceFil…

8a39a3b

…es_api_call

Fix merge issues per review

0956ba1

sekmiller requested changes Dec 20, 2022

View reviewed changes

sekmiller approved these changes Dec 20, 2022

View reviewed changes

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🔎 to QA ✅ Dec 20, 2022

sekmiller removed their assignment Dec 20, 2022

kcondon self-assigned this Dec 21, 2022

kcondon assigned qqmyers and unassigned kcondon Jan 3, 2023

kcondon moved this from QA ✅ to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jan 4, 2023

qqmyers added 4 commits January 5, 2023 15:03

doc/example updates per QA

edf7919

add example response to docs

0da9f8c

typo

d7fdb1b

qqmyers removed their assignment Jan 5, 2023

qqmyers moved this from IQSS Team - In Progress 💻 to QA ✅ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jan 6, 2023

flip quotes in examples per QA

3c35baa

kcondon self-assigned this Jan 9, 2023

qqmyers added 3 commits January 10, 2023 10:51

debug logging

7bf4026

use equals!

64af6f4

Only do physcal delete when deleteFileId has been set

c22545b

kcondon merged commit f63f0e8 into IQSS:develop Jan 10, 2023

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA ✅ to Done 🚀 Jan 10, 2023

pdurbin added this to the 5.13 milestone Jan 23, 2023

qqmyers mentioned this pull request Aug 2, 2023

9002 allow direct upload setting #9003

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GDCC/9005 replace files api call #9018

GDCC/9005 replace files api call #9018

qqmyers commented Oct 3, 2022 •

edited

coveralls commented Oct 5, 2022 •

edited

mreekie commented Dec 13, 2022

mreekie commented Dec 14, 2022

sekmiller Dec 20, 2022

qqmyers Dec 20, 2022

sekmiller Dec 20, 2022

sekmiller left a comment

sekmiller left a comment

kcondon commented Jan 3, 2023 •

edited

qqmyers commented Jan 5, 2023

		@@ -2366,48 +2373,6 @@ The fully expanded example above (without environment variables) looks like this
		Note: The ``id`` returned in the json response is the id of the file metadata version.



		Adding File Metadata

GDCC/9005 replace files api call #9018

GDCC/9005 replace files api call #9018

Conversation

qqmyers commented Oct 3, 2022 • edited

coveralls commented Oct 5, 2022 • edited

mreekie commented Dec 13, 2022

mreekie commented Dec 14, 2022

sekmiller Dec 20, 2022

Choose a reason for hiding this comment

qqmyers Dec 20, 2022

Choose a reason for hiding this comment

sekmiller Dec 20, 2022

Choose a reason for hiding this comment

sekmiller left a comment

Choose a reason for hiding this comment

sekmiller left a comment

Choose a reason for hiding this comment

kcondon commented Jan 3, 2023 • edited

qqmyers commented Jan 5, 2023

qqmyers commented Oct 3, 2022 •

edited

coveralls commented Oct 5, 2022 •

edited

kcondon commented Jan 3, 2023 •

edited